From cbf08f7d5bc82e929dcee6e39bae028a68db8f7c Mon Sep 17 00:00:00 2001 From: Taylor Dolezal <117500+onlydole@users.noreply.github.com> Date: Wed, 4 Nov 2026 13:09:36 -0800 Subject: [PATCH 1/2] fix(desktop): a waiting update is no longer re-announced on every poll While an update was available or downloaded, every background poll moved the state to "checking" and back, and logged "update available" again for the same version. The sidebar update button lost its Download or Install action for the length of each check. A check now leaves an available or downloaded update on screen, and an update-available event for the version already offered changes nothing. A newer version still replaces the offer. The Download and Install controls now stay enabled during a check. A download or install started then waits for the check to finish, and then proceeds or refuses based on the state the check left. It no longer gets refused with no message. Refs #28480 Co-Authored-By: Claude Opus 5.5 --- .../desktop/src/app/DesktopStartupRecovery.ts | 1 - .../src/updates/DesktopRemoteUpdates.test.ts | 3 +- .../src/updates/DesktopUpdates.test.ts | 53 +++++--- apps/desktop/src/updates/DesktopUpdates.ts | 123 +++++++++++------- 4 files changed, 113 insertions(+), 67 deletions(-) diff --git a/apps/desktop/src/app/DesktopStartupRecovery.ts b/apps/desktop/src/app/DesktopStartupRecovery.ts index c8228183aad91..e6c210043cf26 100644 --- a/apps/desktop/src/app/DesktopStartupRecovery.ts +++ b/apps/desktop/src/app/DesktopStartupRecovery.ts @@ -217,7 +217,6 @@ export const listen: Effect.Effect< const download = yield* updates.download; if (!download.completed) return; } - // Waits for a running background check; plain install would refuse. yield* updates.installPrepared(current.version); return; } diff --git a/apps/desktop/src/updates/DesktopRemoteUpdates.test.ts b/apps/desktop/src/updates/DesktopRemoteUpdates.test.ts index 1f5321148b77b..04c1fac8ca6aa 100644 --- a/apps/desktop/src/updates/DesktopRemoteUpdates.test.ts +++ b/apps/desktop/src/updates/DesktopRemoteUpdates.test.ts @@ -691,7 +691,7 @@ describe("DesktopRemoteUpdates", () => { const backgroundCheck = yield* updates.check("poll").pipe(Effect.forkChild); yield* Deferred.await(backgroundCheckStarted); - assert.equal((yield* updates.getState).status, "checking"); + assert.isTrue(yield* updates.isActionActive); yield* Queue.offer(commits, { version: 1, @@ -701,7 +701,6 @@ describe("DesktopRemoteUpdates", () => { yield* settle; assert.equal(harness.quitAndInstalls(), 0); - harness.emit("update-downloaded", { version: "1.2.4" }); yield* Deferred.succeed(releaseBackgroundCheck, undefined); yield* Fiber.join(backgroundCheck); yield* settle; diff --git a/apps/desktop/src/updates/DesktopUpdates.test.ts b/apps/desktop/src/updates/DesktopUpdates.test.ts index 3b45d80bbc6e2..6eeef4a043b22 100644 --- a/apps/desktop/src/updates/DesktopUpdates.test.ts +++ b/apps/desktop/src/updates/DesktopUpdates.test.ts @@ -65,8 +65,12 @@ describe("DesktopUpdates", () => { ).pipe(Effect.provide(harness.layer)); }); - it.effect("checks for newer releases after an update has been downloaded", () => { + it.effect("re-checks an offered update without announcing it again", () => { const harness = makeHarness(); + const announcements: unknown[] = []; + const logger = Logger.make(({ message }) => { + if ([message].flat().includes("update available")) announcements.push(message); + }); return Effect.scoped( Effect.gen(function* () { @@ -78,14 +82,24 @@ describe("DesktopUpdates", () => { releaseNotes: "## What's changed\n- fix: queued update", }); yield* flushCallbacks; + const offeredStates = harness.sentStates.length; + + yield* updates.check("poll"); + harness.emit("update-available", { version: "1.2.4" }); + yield* flushCallbacks; + assert.equal(harness.sentStates.length, offeredStates); + harness.emit("update-downloaded", { version: "1.2.4" }); yield* flushCallbacks; + const downloadedStates = harness.sentStates.length; const result = yield* updates.check("poll"); assert.isTrue(result.checked); harness.emit("update-available", { version: "1.2.4" }); yield* flushCallbacks; + assert.equal(harness.sentStates.length, downloadedStates); + assert.lengthOf(announcements, 1); const unchangedState = yield* updates.getState; assert.equal(unchangedState.status, "downloaded"); @@ -105,8 +119,17 @@ describe("DesktopUpdates", () => { assert.equal(state.status, "available"); assert.equal(state.availableVersion, "1.2.5"); assert.isNull(state.downloadedVersion); + assert.lengthOf(announcements, 2); }), - ).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer))); + ).pipe( + Effect.provide( + Layer.mergeAll( + TestClock.layer(), + harness.layer, + Logger.layer([logger], { mergeWithExisting: false }), + ), + ), + ); }); it.effect("preserves a queued installer when the feed has no update", () => { @@ -142,9 +165,12 @@ describe("DesktopUpdates", () => { ).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer))); }); - it.effect( - "rejects install while a refresh check is in progress and releases the reservation", - () => + it.effect.each([ + { action: "download", offer: "update-available" }, + { action: "install", offer: "update-downloaded" }, + ] as const)( + "waits out a running refresh check instead of refusing $action", + ({ action, offer }) => Effect.gen(function* () { const checkStarted = yield* Deferred.make(); const releaseCheck = yield* Deferred.make(); @@ -158,22 +184,17 @@ describe("DesktopUpdates", () => { Effect.gen(function* () { const updates = yield* DesktopUpdates.DesktopUpdates; yield* updates.configure; - harness.emit("update-downloaded", { version: "1.2.4" }); + harness.emit(offer, { version: "1.2.4" }); yield* flushCallbacks; - const checkFiber = yield* updates.check("manual").pipe(Effect.forkScoped); + const checkFiber = yield* updates.check("poll").pipe(Effect.forkScoped); yield* Deferred.await(checkStarted); - - const installResult = yield* updates.install; - assert.isFalse(installResult.accepted); + const actionFiber = yield* updates[action].pipe(Effect.forkScoped); + yield* flushCallbacks; yield* Deferred.succeed(releaseCheck, undefined); - const checkResult = yield* Fiber.join(checkFiber); - assert.isTrue(checkResult.checked); - - const followUpCheck = yield* updates.check("manual"); - assert.isTrue(followUpCheck.checked); - assert.equal(harness.checkCount(), 2); + assert.isTrue((yield* Fiber.join(checkFiber)).checked); + assert.isTrue((yield* Fiber.join(actionFiber)).accepted); }), ).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer))); }), diff --git a/apps/desktop/src/updates/DesktopUpdates.ts b/apps/desktop/src/updates/DesktopUpdates.ts index 6279e8b6141a8..f4240a2315e81 100644 --- a/apps/desktop/src/updates/DesktopUpdates.ts +++ b/apps/desktop/src/updates/DesktopUpdates.ts @@ -56,7 +56,7 @@ const CHANNEL_CHANGE_RETRY_DELAY = Duration.seconds(1); const CHANNEL_CHANGE_RETRIES = 30; const AUTO_UPDATE_STARTUP_DELAY = "15 seconds"; const AUTO_UPDATE_POLL_INTERVAL = "4 minutes"; -const PREPARED_INSTALL_CHECK_WAIT = Duration.seconds(90); +const UPDATE_CHECK_WAIT = Duration.seconds(90); type UpdateAction = "check" | "download" | "install" | "install-recovery" | "channel"; @@ -310,6 +310,10 @@ function getCanRetryFromState(state: DesktopUpdateState): boolean { return state.availableVersion !== null || state.downloadedVersion !== null; } +function isOffered(state: DesktopUpdateState): boolean { + return state.status === "available" || state.status === "downloaded"; +} + function shouldBroadcastDownloadProgress( currentState: DesktopUpdateState, nextPercent: number, @@ -509,6 +513,23 @@ export const make = Effect.gen(function* () { ), ); + // Re-runs admit after each finished action, so a caller waiting on a check decides on its result. + const admitAfterCheck = Effect.fn("desktop.updates.admitAfterCheck")(function* ( + admit: Effect.Effect<"admitted" | "refused" | "wait-for-check">, + ) { + const actionCompletions = yield* PubSub.subscribe(finishedUpdateActions); + let admission = yield* stateMutex.withPermits(1)(admit); + while (admission === "wait-for-check") { + const finishedAction = yield* PubSub.take(actionCompletions).pipe( + Effect.timeoutOption(UPDATE_CHECK_WAIT), + ); + admission = Option.isNone(finishedAction) + ? "refused" + : yield* stateMutex.withPermits(1)(admit); + } + return admission; + }); + const applyAutoUpdaterChannel = Effect.fn("desktop.updates.applyAutoUpdaterChannel")(function* ( channel: DesktopUpdateChannel, ) { @@ -550,8 +571,10 @@ export const make = Effect.gen(function* () { if (actionReservation === "acquire" && !(yield* tryStartUpdateAction("check"))) return false; const check = Effect.gen(function* () { - const checkedAt = yield* currentIsoTimestamp; - yield* setState(reduceDesktopUpdateStateOnCheckStart(state, checkedAt)); + // An offer stays on screen while re-checking, so finding it again changes nothing. + if (!isOffered(state)) { + yield* setState(reduceDesktopUpdateStateOnCheckStart(state, yield* currentIsoTimestamp)); + } yield* logUpdaterInfo("checking for updates", { reason }); // The flag is shared updater state, so it flips only while this check holds the reservation. @@ -588,15 +611,30 @@ export const make = Effect.gen(function* () { }); const downloadAvailableUpdate = Effect.gen(function* () { - const state = yield* Ref.get(updateStateRef); - if (!(yield* Ref.get(updaterConfiguredRef)) || state.status !== "available") { - return { accepted: false, completed: false }; - } - - if (!(yield* tryStartUpdateAction("download"))) { + const admission = yield* Effect.scoped( + admitAfterCheck( + Effect.gen(function* () { + const state = yield* Ref.get(updateStateRef); + if (!(yield* Ref.get(updaterConfiguredRef)) || state.status !== "available") { + return "refused" as const; + } + const activeAction = yield* Ref.get(activeUpdateActionRef); + if (Option.isSome(activeAction)) { + return activeAction.value === "check" + ? ("wait-for-check" as const) + : ("refused" as const); + } + return (yield* tryStartUpdateAction("download")) + ? ("admitted" as const) + : ("refused" as const); + }), + ), + ); + if (admission === "refused") { return { accepted: false, completed: false }; } + const state = yield* Ref.get(updateStateRef); return yield* Effect.gen(function* () { yield* setState(reduceDesktopUpdateStateOnDownloadStart(state)); yield* electronUpdater.setDisableDifferentialDownload( @@ -758,47 +796,35 @@ export const make = Effect.gen(function* () { const installDownloadedUpdate = (expectedVersion?: string) => Effect.scoped( Effect.gen(function* () { - const actionCompletions = yield* PubSub.subscribe(finishedUpdateActions); - let admission: "admitted" | "refused" | "wait-for-check" = "wait-for-check"; - while (admission === "wait-for-check") { - admission = yield* stateMutex.withPermits(1)( - Effect.gen(function* () { - const state = yield* Ref.get(updateStateRef); - const activeAction = yield* Ref.get(activeUpdateActionRef); - const hasExpectedDownload = - state.downloadedVersion !== null && - (expectedVersion === undefined || state.downloadedVersion === expectedVersion); - if ( - (yield* Ref.get(desktopState.quitting)) || - !(yield* Ref.get(updaterConfiguredRef)) || - !hasExpectedDownload - ) { - return "refused" as const; - } - if (Option.isSome(activeAction)) { - return activeAction.value === "check" && expectedVersion !== undefined - ? ("wait-for-check" as const) - : ("refused" as const); - } - const hasInstallableDownload = - state.status === "downloaded" || - (state.status === "error" && - (state.errorContext === null || state.errorContext === "install")); - if (!hasInstallableDownload) return "refused" as const; - return (yield* tryStartUpdateAction("install")) - ? ("admitted" as const) + const admission = yield* admitAfterCheck( + Effect.gen(function* () { + const state = yield* Ref.get(updateStateRef); + const activeAction = yield* Ref.get(activeUpdateActionRef); + const hasExpectedDownload = + state.downloadedVersion !== null && + (expectedVersion === undefined || state.downloadedVersion === expectedVersion); + if ( + (yield* Ref.get(desktopState.quitting)) || + !(yield* Ref.get(updaterConfiguredRef)) || + !hasExpectedDownload + ) { + return "refused" as const; + } + if (Option.isSome(activeAction)) { + return activeAction.value === "check" + ? ("wait-for-check" as const) : ("refused" as const); - }), - ); - if (admission === "wait-for-check") { - const finishedAction = yield* PubSub.take(actionCompletions).pipe( - Effect.timeoutOption(PREPARED_INSTALL_CHECK_WAIT), - ); - if (Option.isNone(finishedAction)) { - admission = "refused"; } - } - } + const hasInstallableDownload = + state.status === "downloaded" || + (state.status === "error" && + (state.errorContext === null || state.errorContext === "install")); + if (!hasInstallableDownload) return "refused" as const; + return (yield* tryStartUpdateAction("install")) + ? ("admitted" as const) + : ("refused" as const); + }), + ); if (admission === "refused") { return { accepted: false, completed: false, failed: false }; } @@ -934,6 +960,7 @@ export const make = Effect.gen(function* () { yield* Ref.set(lastLoggedDownloadMilestoneRef, -1); return; } + if (isOffered(state) && state.availableVersion === info.version) return; const checkedAt = yield* currentIsoTimestamp; const { releaseNotes, omittedReleaseCount } = normalizeDesktopUpdateReleaseNotes( From eb6a0a432cf753cdad86c741291d4a5fb09196a6 Mon Sep 17 00:00:00 2001 From: Taylor Dolezal <117500+onlydole@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:50:31 -0800 Subject: [PATCH 2/2] test(desktop): a waiting update action decides on the check's outcome A Download or Install that waits out a refresh check now has test rows where the check finds a newer version. The check emits the outcome before resolving, as electron-updater does, so the action is decided against the new offer. The rows fail if the outcome handlers start after the check resolves. Co-Authored-By: Claude Opus 5.5 --- .../src/updates/DesktopUpdates.test.ts | 21 ++++++++++++++----- apps/desktop/src/updates/DesktopUpdates.ts | 2 ++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/apps/desktop/src/updates/DesktopUpdates.test.ts b/apps/desktop/src/updates/DesktopUpdates.test.ts index 6eeef4a043b22..b890f04960eb1 100644 --- a/apps/desktop/src/updates/DesktopUpdates.test.ts +++ b/apps/desktop/src/updates/DesktopUpdates.test.ts @@ -166,17 +166,25 @@ describe("DesktopUpdates", () => { }); it.effect.each([ - { action: "download", offer: "update-available" }, - { action: "install", offer: "update-downloaded" }, + { action: "download", offer: "update-available", found: null, accepted: true }, + { action: "install", offer: "update-downloaded", found: null, accepted: true }, + { action: "download", offer: "update-available", found: "1.2.5", accepted: true }, + { action: "install", offer: "update-downloaded", found: "1.2.5", accepted: false }, ] as const)( - "waits out a running refresh check instead of refusing $action", - ({ action, offer }) => + "a $action waiting out a refresh check acts on what it found ($found)", + ({ action, offer, found, accepted }) => Effect.gen(function* () { const checkStarted = yield* Deferred.make(); const releaseCheck = yield* Deferred.make(); const harness = makeHarness({ checkForUpdates: Deferred.succeed(checkStarted, undefined).pipe( Effect.andThen(Deferred.await(releaseCheck)), + // electron-updater emits the check's outcome before its promise resolves. + Effect.andThen( + Effect.sync(() => { + if (found !== null) harness.emit("update-available", { version: found }); + }), + ), ), }); @@ -194,7 +202,10 @@ describe("DesktopUpdates", () => { yield* Deferred.succeed(releaseCheck, undefined); assert.isTrue((yield* Fiber.join(checkFiber)).checked); - assert.isTrue((yield* Fiber.join(actionFiber)).accepted); + assert.equal((yield* Fiber.join(actionFiber)).accepted, accepted); + const state = yield* updates.getState; + assert.equal(state.availableVersion, found ?? "1.2.4"); + if (action === "download") assert.equal(state.status, "downloading"); }), ).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer))); }), diff --git a/apps/desktop/src/updates/DesktopUpdates.ts b/apps/desktop/src/updates/DesktopUpdates.ts index f4240a2315e81..abc88a1ccd2a8 100644 --- a/apps/desktop/src/updates/DesktopUpdates.ts +++ b/apps/desktop/src/updates/DesktopUpdates.ts @@ -1140,6 +1140,8 @@ export const make = Effect.gen(function* () { ), configure: Effect.gen(function* () { const context = yield* Effect.context(); + // Runs synchronously up to the first await, so a check's outcome is applied before the + // check's promise resolves and wakes the actions waiting on it. const runEffect = (effect: Effect.Effect) => { void Effect.runPromiseWith(context)(effect); };