diff --git a/apps/desktop/src/main/services/ipc/registerIpc.ts b/apps/desktop/src/main/services/ipc/registerIpc.ts index c1e2321b2..caff5f132 100644 --- a/apps/desktop/src/main/services/ipc/registerIpc.ts +++ b/apps/desktop/src/main/services/ipc/registerIpc.ts @@ -11451,8 +11451,8 @@ export function registerIpc({ }); ipcMain.handle(IPC.updateCheckForUpdates, () => { - // Only reachable from the Settings button, so it always counts as - // user-initiated: it must run even when an update is already staged. + // Only reachable from the Settings button. Every entry point now runs the + // same check, so `userInitiated` only labels the log line. getCtx().autoUpdateService?.checkForUpdates({ userInitiated: true }); }); diff --git a/apps/desktop/src/main/services/updates/autoUpdateService.test.ts b/apps/desktop/src/main/services/updates/autoUpdateService.test.ts index a1f456d71..ccd78180a 100644 --- a/apps/desktop/src/main/services/updates/autoUpdateService.test.ts +++ b/apps/desktop/src/main/services/updates/autoUpdateService.test.ts @@ -2,7 +2,7 @@ import { EventEmitter } from "node:events"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi, type Mock } from "vitest"; import { createAutoUpdateService } from "./autoUpdateService"; import { buildGithubReleaseUrl, @@ -76,6 +76,80 @@ function plentyOfDisk(): { availableBytes: number; volumePath: string } { }; } +// The release the supersede tests answer the feed with. Every one of them +// needs the same shape, and a drifting `files` entry would silently change +// which preflight the download runs through. +const NEWER_UPDATE = { + version: "1.2.63", + files: [{ url: "ADE-1.2.63-mac.zip", size: 100 * 1024 * 1024, sha512: "new" }], +}; + +function newerUpdateArchivePath(updaterCacheDir: string): string { + return path.join(updaterCacheDir, "pending", "ADE-1.2.63-universal-mac.zip"); +} + +// electron-updater emits `update-downloaded` from inside downloadUpdate(), so +// the fake writes the archive and emits before it resolves. A mock that only +// resolves cannot see the supersede path at all. +function downloadsNewerUpdate( + updater: FakeAutoUpdater, + updaterCacheDir: string, +): () => Promise { + const archivePath = newerUpdateArchivePath(updaterCacheDir); + return async () => { + fs.mkdirSync(path.dirname(archivePath), { recursive: true }); + fs.writeFileSync(archivePath, "newer update", "utf8"); + updater.emit("update-downloaded", { ...NEWER_UPDATE, downloadedFile: archivePath }); + }; +} + +type StagedReadyUpdate = { + service: ReturnType; + updater: FakeAutoUpdater & { downloadUpdate: Mock<[], Promise> }; + logger: Logger; + updaterCacheDir: string; +}; + +/** + * Builds a service that already has 1.2.61 downloaded and waiting for a + * restart, which is the starting state of every "check while staged" test. + * `downloadUpdate` defaults to a fake that downloads nothing; pass + * `downloadsNewerUpdate` to cover the supersede path. + */ +function stageReadyUpdate( + overrides: Partial[0]> & { + downloadUpdate?: ( + updater: FakeAutoUpdater, + updaterCacheDir: string, + ) => () => Promise; + } = {}, +): StagedReadyUpdate { + const { downloadUpdate, ...serviceOverrides } = overrides; + const updaterCacheDir = makeUpdaterCacheDir(); + const logger = makeLogger(); + const updater = new FakeAutoUpdater() as StagedReadyUpdate["updater"]; + updater.downloadUpdate = vi.fn<[], Promise>( + downloadUpdate?.(updater, updaterCacheDir) ?? (async () => undefined), + ); + const service = createAutoUpdateService({ + logger, + currentVersion: "1.2.60", + globalStatePath: makeStatePath(), + updaterCacheDir, + getDiskSpace: plentyOfDisk, + autoCheckEnabled: false, + autoApplyEnabled: false, + updater, + ...serviceOverrides, + }); + + updater.emit("update-available", { version: "1.2.61" }); + updater.emit("update-downloaded", { version: "1.2.61" }); + expect(service.getSnapshot()).toMatchObject({ status: "ready", version: "1.2.61" }); + + return { service, updater, logger, updaterCacheDir }; +} + function readState(globalStatePath: string): Record { try { return JSON.parse(fs.readFileSync(globalStatePath, "utf8")); @@ -468,80 +542,242 @@ describe("createAutoUpdateService", () => { service.dispose(); }); - it("still checks when an update is already staged, but only when the user asked", async () => { - // The Settings button was a silent no-op in exactly this state: an update - // downloaded and waiting for a restart. The automatic timers must keep - // standing down so they cannot disturb the staged download. - const updater = new FakeAutoUpdater(); - const service = createAutoUpdateService({ - logger: makeLogger(), - currentVersion: "1.2.60", - globalStatePath: makeStatePath(), + // A staged update used to block discovery of every later release until the + // app was relaunched: the automatic checks returned early while `ready`, and + // a manual check recorded the newer version as metadata only. The pill kept + // offering the older release for hours. Every check now behaves the same way. + it("ignores a same-version feed answer on the periodic check while an update is staged", async () => { + vi.useFakeTimers(); + const { service, updater, logger, updaterCacheDir } = stageReadyUpdate({ startupDelayMs: 60_000, - periodicCheckMs: 60_000, - now: () => "2026-08-19T21:00:00.000Z", - updater, + periodicCheckMs: 1_000, + autoCheckEnabled: true, }); - updater.emit("update-available", { version: "1.2.61" }); - updater.emit("update-downloaded", { version: "1.2.61" }); - expect(service.getSnapshot()).toMatchObject({ status: "ready", version: "1.2.61" }); + updater.checkForUpdates.mockImplementationOnce(async () => { + updater.emit("checking-for-update"); + updater.emit("update-available", { version: "1.2.61" }); + return { updateInfo: { version: "1.2.61" } }; + }); - updater.checkForUpdates.mockClear(); - service.checkForUpdates(); - // Flush the microtask queue rather than waitFor: a `not.toHaveBeenCalled` - // inside waitFor passes on its first tick and would assert nothing. - await Promise.resolve(); - await Promise.resolve(); - expect(updater.checkForUpdates).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(1_000); + await vi.waitFor(() => expect(updater.checkForUpdates).toHaveBeenCalled()); - // electron-updater emits these BEFORE checkForUpdates() resolves. A mock - // that only resolves cannot see the bug this test exists for: the - // update-available handler is what supersedes the staged update, deletes - // the finished download, and frees the resolve path to start a new one. - const downloadUpdate = vi.fn(async () => null); - (updater as unknown as { downloadUpdate: unknown }).downloadUpdate = downloadUpdate; - updater.checkForUpdates.mockImplementation(async () => { + expect(service.getSnapshot()).toMatchObject({ + status: "ready", + version: "1.2.61", + latestKnownVersion: "1.2.61", + }); + expect(updater.downloadUpdate).not.toHaveBeenCalled(); + expect(fs.readdirSync(updaterCacheDir).sort()).toEqual(["pending", "update.zip"]); + expect(logger.info).not.toHaveBeenCalledWith( + "autoUpdate.cache_cleaned", + expect.objectContaining({ reason: "superseded_ready_update" }), + ); + expect(logger.info).toHaveBeenCalledWith( + "autoUpdate.update_available_ignored", + expect.objectContaining({ version: "1.2.61", reason: "same_ready_version" }), + ); + + service.dispose(); + }); + + it("supersedes a staged update with a newer release found by the periodic check", async () => { + vi.useFakeTimers(); + const { service, updater, logger, updaterCacheDir } = stageReadyUpdate({ + startupDelayMs: 60_000, + periodicCheckMs: 1_000, + autoCheckEnabled: true, + downloadUpdate: downloadsNewerUpdate, + }); + + updater.checkForUpdates.mockImplementationOnce(async () => { + updater.emit("checking-for-update"); + updater.emit("update-available", NEWER_UPDATE); + return { updateInfo: NEWER_UPDATE }; + }); + + await vi.advanceTimersByTimeAsync(1_000); + await vi.waitFor(() => { + expect(service.getSnapshot()).toMatchObject({ + status: "ready", + version: "1.2.63", + latestKnownVersion: "1.2.63", + }); + }); + + expect(updater.downloadUpdate).toHaveBeenCalledTimes(1); + expect(logger.info).toHaveBeenCalledWith( + "autoUpdate.cache_cleaned", + expect.objectContaining({ reason: "superseded_ready_update" }), + ); + expect(fs.readFileSync(newerUpdateArchivePath(updaterCacheDir), "utf8")).toBe("newer update"); + + service.dispose(); + }); + + it("supersedes a staged update with a newer release found by a user-initiated check", async () => { + const { service, updater, logger } = stageReadyUpdate({ + downloadUpdate: downloadsNewerUpdate, + }); + + // electron-updater emits these BEFORE checkForUpdates() resolves, so the + // supersede happens in the update-available handler. + updater.checkForUpdates.mockImplementationOnce(async () => { updater.emit("checking-for-update"); - updater.emit("update-available", { version: "1.2.63" }); - return { updateInfo: { version: "1.2.63" } }; + updater.emit("update-available", NEWER_UPDATE); + return { updateInfo: NEWER_UPDATE }; }); + service.checkForUpdates({ userInitiated: true }); await vi.waitFor(() => { - expect(updater.checkForUpdates).toHaveBeenCalledTimes(1); - // The newest version is reported, and the staged 1.2.61 is left alone. expect(service.getSnapshot()).toMatchObject({ status: "ready", - version: "1.2.61", + version: "1.2.63", latestKnownVersion: "1.2.63", }); }); - // Asking what the newest version is must not throw away the update the - // user already downloaded and is one restart away from installing. - expect(downloadUpdate).not.toHaveBeenCalled(); + expect(updater.checkForUpdates).toHaveBeenCalledTimes(1); + expect(updater.downloadUpdate).toHaveBeenCalledTimes(1); + expect(logger.info).toHaveBeenCalledWith( + "autoUpdate.cache_cleaned", + expect.objectContaining({ reason: "superseded_ready_update" }), + ); - // Let the first check's promise chain settle: while `checkPromise` is - // still set, a second call is swallowed by the in-flight guard and would - // assert nothing. - await new Promise((resolve) => setTimeout(resolve, 0)); + service.dispose(); + }); - // ...and neither must a check that fails. A feed error here means "nothing - // new to tell you", not "discard the finished download". - updater.checkForUpdates.mockImplementation(async () => { + it("leaves a staged update untouched when the check itself fails", async () => { + const { service, updater, logger, updaterCacheDir } = stageReadyUpdate(); + + // A feed error means "nothing new to report", not "discard the finished + // download". electron-updater emits `error` and rejects, so cover both. + updater.checkForUpdates.mockImplementationOnce(async () => { updater.emit("checking-for-update"); updater.emit("error", new Error("net::ERR_INTERNET_DISCONNECTED")); throw new Error("net::ERR_INTERNET_DISCONNECTED"); }); + + service.checkForUpdates(); + + await vi.waitFor(() => expect(updater.checkForUpdates).toHaveBeenCalledTimes(1)); + expect(service.getSnapshot()).toMatchObject({ + status: "ready", + version: "1.2.61", + latestKnownVersion: "1.2.61", + error: null, + errorDetails: null, + }); + expect(updater.downloadUpdate).not.toHaveBeenCalled(); + expect(fs.readdirSync(updaterCacheDir).sort()).toEqual(["pending", "update.zip"]); + expect(logger.info).not.toHaveBeenCalledWith( + "autoUpdate.cache_cleaned", + expect.anything(), + ); + expect(logger.warn).toHaveBeenCalledWith( + "autoUpdate.ready_check_failed", + expect.objectContaining({ readyVersion: "1.2.61" }), + ); + + service.dispose(); + }); + + it("refuses to check the feed while a quit-and-install transaction is running", async () => { + // The snapshot stays `ready` for the whole transaction, across the + // `beforeQuitAndInstall` service uninstall, and only flips to `installing` + // afterwards. A check started in that window would let a strictly newer + // feed answer supersede and delete the archive the install is one call away + // from handing to Squirrel/NSIS. + let releasePrepare!: () => void; + let signalPrepareStarted!: () => void; + const prepareStarted = new Promise((resolve) => { + signalPrepareStarted = resolve; + }); + const { service, updater, logger, updaterCacheDir } = stageReadyUpdate({ + downloadUpdate: downloadsNewerUpdate, + beforeQuitAndInstall: () => { + signalPrepareStarted(); + return new Promise((resolve) => { + releasePrepare = resolve; + }); + }, + }); + const cacheBefore = fs.readdirSync(updaterCacheDir).sort(); + + const installPromise = service.quitAndInstall(); + await prepareStarted; + + // The pre-install refresh already ran and finished; only the guard can + // stand between the next check and the staged archive now. + updater.checkForUpdates.mockClear(); + updater.checkForUpdates.mockImplementation(async () => { + updater.emit("checking-for-update"); + updater.emit("update-available", NEWER_UPDATE); + return { updateInfo: NEWER_UPDATE }; + }); + + service.checkForUpdates(); service.checkForUpdates({ userInitiated: true }); + await Promise.resolve(); + await Promise.resolve(); + + expect(updater.checkForUpdates).not.toHaveBeenCalled(); + expect(updater.downloadUpdate).not.toHaveBeenCalled(); + expect(fs.readdirSync(updaterCacheDir).sort()).toEqual(cacheBefore); + expect(fs.existsSync(newerUpdateArchivePath(updaterCacheDir))).toBe(false); + expect(logger.info).not.toHaveBeenCalledWith( + "autoUpdate.cache_cleaned", + expect.objectContaining({ reason: "superseded_ready_update" }), + ); + expect(service.getSnapshot()).toMatchObject({ status: "ready", version: "1.2.61" }); + + releasePrepare(); + await expect(installPromise).resolves.toBe(true); + expect(service.getSnapshot()).toMatchObject({ status: "installing", version: "1.2.61" }); + expect(updater.quitAndInstall).toHaveBeenCalledWith(false, true); - await vi.waitFor(() => expect(updater.checkForUpdates).toHaveBeenCalledTimes(2)); + service.dispose(); + }); + + it("aborts the install when the check it piggybacks on fails", async () => { + // The pre-install refresh reuses an in-flight periodic check instead of + // starting its own. The failure guards must test the refresh first, or the + // feed error is swallowed as "nothing new" and the install proceeds on a + // version it never managed to verify. + const { service, updater, logger, updaterCacheDir } = stageReadyUpdate({ + downloadUpdate: downloadsNewerUpdate, + }); + const cacheBefore = fs.readdirSync(updaterCacheDir).sort(); + + let rejectFeed!: (error: Error) => void; + updater.checkForUpdates.mockImplementationOnce(() => { + updater.emit("checking-for-update"); + return new Promise((_resolve, reject) => { + rejectFeed = reject; + }); + }); + + service.checkForUpdates(); + await vi.waitFor(() => expect(updater.checkForUpdates).toHaveBeenCalledTimes(1)); + const installPromise = service.quitAndInstall(); + + const feedError = new Error("net::ERR_INTERNET_DISCONNECTED"); + updater.emit("error", feedError); + rejectFeed(feedError); + + await expect(installPromise).resolves.toBe(false); + expect(updater.quitAndInstall).not.toHaveBeenCalled(); expect(service.getSnapshot()).toMatchObject({ status: "ready", version: "1.2.61", - latestKnownVersion: "1.2.63", + parked: { reason: "refresh_failed" }, }); - expect(downloadUpdate).not.toHaveBeenCalled(); + expect(fs.readdirSync(updaterCacheDir).sort()).toEqual(cacheBefore); + expect(logger.warn).toHaveBeenCalledWith( + "autoUpdate.refresh_ready_before_install_failed", + expect.objectContaining({ version: "1.2.61" }), + ); service.dispose(); }); diff --git a/apps/desktop/src/main/services/updates/autoUpdateService.ts b/apps/desktop/src/main/services/updates/autoUpdateService.ts index ddf22388a..ffa10bddb 100644 --- a/apps/desktop/src/main/services/updates/autoUpdateService.ts +++ b/apps/desktop/src/main/services/updates/autoUpdateService.ts @@ -527,11 +527,35 @@ export function createAutoUpdateService({ let ignoredDownloadVersion: string | null = null; let readyRefreshInProgress = false; /** - * A user-initiated check while an update is already staged. The answer they - * want is "what is the newest version" — throwing away the update they have - * already downloaded in order to answer it is not a service. + * A feed check that started while an update was already staged, from any + * entry point. A strictly newer release supersedes the staged one, so this + * flag only protects the failure path: a check that cannot answer must leave + * the staged update, its version and its archive exactly as it found them. + * It is never set for the pre-install refresh, which owns its own failure + * handling through `readyRefreshInProgress`. */ - let readyMetadataRefreshInProgress = false; + let readyCheckInProgress = false; + /** + * The shared failure guard for a check that started while an update was + * staged. The check asked what the newest version is and the feed did not + * answer. That is a reason to report nothing new, not a reason to throw away + * the update already downloaded: setErrorSnapshot would replace the `ready` + * snapshot and, with currentPhase already moved to "download" by + * `checking-for-update`, take the finished download with it. Once a supersede + * has started the status is no longer `ready`, so a failure of the new + * download flows through the normal error path instead. + * + * Returns true when the caller must return without touching the snapshot. + */ + function preserveStagedUpdateOnCheckFailure(err: unknown): boolean { + if (!readyCheckInProgress || snapshot.status !== "ready") return false; + logger.warn("autoUpdate.ready_check_failed", { + message: formatErrorMessage(err), + kind: classifyUpdateError(err, currentPhase).kind, + readyVersion: snapshot.version, + }); + return true; + } const readyRefreshFailure: { current: { error: unknown; phase: AutoUpdatePhase } | null; } = { current: null }; @@ -840,20 +864,10 @@ export function createAutoUpdateService({ }); return; } - if (readyMetadataRefreshInProgress) { - // electron-updater emits `update-available` BEFORE checkForUpdates() - // resolves, so without this the newer version would supersede the - // staged one here — deleting the finished download from the cache and - // leaving the resolve path free to start a fresh one — purely because - // someone pressed a button to ask a question. - ignoredDownloadVersion = info.version; - patchSnapshot({ latestKnownVersion: info.version }); - logger.info("autoUpdate.update_available_metadata_only", { - version: info.version, - readyVersion: snapshot.version, - }); - return; - } + // A strictly newer release supersedes the staged one, whichever check + // found it. Keeping the older archive would pin the app to a version the + // feed has already replaced until the next relaunch, which is the exact + // complaint this behavior exists to answer. if (!readyRefreshInProgress) { downloadedFilePath = null; cleanupUpdaterCacheDir({ @@ -1004,19 +1018,10 @@ export function createAutoUpdateService({ } ignoredDownloadVersion = null; if (staleHandoffRecoveryInProgress && isStaleHandoffError(err)) return; - if (readyMetadataRefreshInProgress) { - // The user asked what the newest version is and the feed did not answer. - // That is a reason to tell them nothing new, not a reason to throw away - // the update they already downloaded: setErrorSnapshot would replace the - // `ready` snapshot and, with currentPhase already moved to "download" by - // `checking-for-update`, take the finished download with it. - logger.warn("autoUpdate.metadata_refresh_failed", { - message, - kind: classified.kind, - readyVersion: snapshot.version, - }); - return; - } + // The pre-install refresh is tested first. It can piggyback on a periodic + // check that is already in flight, which leaves `readyCheckInProgress` set + // too. Recording the failure is what aborts the install; swallowing it here + // would let the install proceed on an unverified staged version. if (readyRefreshInProgress) { readyRefreshFailure.current = { error: err, @@ -1024,6 +1029,7 @@ export function createAutoUpdateService({ }; return; } + if (preserveStagedUpdateOnCheckFailure(err)) return; if (snapshot.status === "installing") { if ( isStaleHandoffError(err) @@ -1054,13 +1060,21 @@ export function createAutoUpdateService({ updater.on("update-cancelled", onUpdateCancelled); updater.on("error", onError); - async function runUpdateCheck( - args: { allowReady?: boolean; metadataOnlyWhenReady?: boolean } = {}, - ): Promise { + async function runUpdateCheck(): Promise { if (checkPromise) { await checkPromise; return; } + const isReadyCheck = snapshot.status === "ready" && !readyRefreshInProgress; + // An install transaction holds the staged archive until it flips the + // snapshot to `installing`, and the status stays `ready` across the whole + // `beforeQuitAndInstall` service uninstall. A check started in that window + // would let a strictly newer feed answer supersede and wipe the archive the + // install is about to hand to Squirrel/NSIS. The `!readyRefreshInProgress` + // term is required: when `restorePromise` is non-null, `run()` suspends at + // `await restorePromise` before the pre-install refresh, so + // `quitAndInstallPromise` is already assigned when that refresh runs. + if (isReadyCheck && (quitAndInstallPromise != null || installQuitArmed)) return; if ( snapshot.status === "checking" || snapshot.status === "downloading" @@ -1068,11 +1082,16 @@ export function createAutoUpdateService({ ) { return; } - if (!args.allowReady && snapshot.status === "ready") { + if (isReadyCheck) { + // A restore already owns the staged archive and drives the updater + // itself. Starting a feed check on top of it would race its download. + if (archiveRestoreInProgress) return; + // The archive is the thing the restart depends on. Put it back first and + // ask the feed on the next cycle, so one cycle never does both. if (!stagedArchiveStillPresent()) { void restoreStagedArchiveIfMissing("periodic_ready_check"); + return; } - return; } const reusableDownloadedVersion = snapshot.status === "error" && snapshot.errorDetails?.preservesDownload @@ -1081,8 +1100,7 @@ export function createAutoUpdateService({ preservedDownloadRetry = reusableDownloadedVersion ? { version: reusableDownloadedVersion, releaseNotesUrl: snapshot.releaseNotesUrl } : null; - readyMetadataRefreshInProgress = args.metadataOnlyWhenReady === true - && snapshot.status === "ready"; + readyCheckInProgress = isReadyCheck; checkPromise = updater.checkForUpdates() .then(async (result) => { const updateInfo = isUpdateCheckResultLike(result) ? result.updateInfo : undefined; @@ -1123,15 +1141,8 @@ export function createAutoUpdateService({ } }) .catch((error) => { - // Same reasoning as onError: a metadata-only refresh must leave the - // staged update exactly as it found it, however it fails. - if (readyMetadataRefreshInProgress) { - logger.warn("autoUpdate.metadata_refresh_failed", { - message: formatErrorMessage(error), - readyVersion: snapshot.version, - }); - return; - } + // Same ordering as onError: the pre-install refresh owns the failure + // first, then the guard that keeps a staged update intact. if (readyRefreshInProgress) { readyRefreshFailure.current = { error, @@ -1139,6 +1150,7 @@ export function createAutoUpdateService({ }; return; } + if (preserveStagedUpdateOnCheckFailure(error)) return; // electron-updater normally emits `error` as well as rejecting. Keep // this fallback so synchronous filesystem failures cannot disappear. if (snapshot.status !== "error") { @@ -1154,25 +1166,25 @@ export function createAutoUpdateService({ .finally(() => { checkPromise = null; preservedDownloadRetry = null; - readyMetadataRefreshInProgress = false; + readyCheckInProgress = false; }); await checkPromise; } /** - * `userInitiated` is what separates the Settings button from the startup and - * periodic timers. The automatic checks stay out of the way of an update that - * is already downloaded and waiting for a restart, but a person pressing - * "Check for updates" is asking a question, and answering it with an early - * return is indistinguishable from the button being broken — the version on - * screen just stays at whatever the last real check found. - * - * The `ready` branch of the check still returns before downloading, so this - * refreshes the newest known version without disturbing the staged download. + * Every entry point behaves the same way, whether it is the Settings button, + * the startup and periodic timers, the `ade update` CLI, or an ADE action. + * An update that is already staged does not stop the check: a strictly newer + * release supersedes the staged one and downloads in its place, a same or + * older release is ignored, and a check that fails leaves the staged update + * untouched. `userInitiated` is recorded for the logs only. */ function checkForUpdates(options: { userInitiated?: boolean } = {}): void { - const userInitiated = options.userInitiated === true; - void runUpdateCheck({ allowReady: userInitiated, metadataOnlyWhenReady: userInitiated }); + logger.info("autoUpdate.check_requested", { + userInitiated: options.userInitiated === true, + status: snapshot.status, + }); + void runUpdateCheck(); } async function refreshReadyUpdateBeforeInstall(): Promise { @@ -1182,7 +1194,7 @@ export function createAutoUpdateService({ readyRefreshInProgress = true; readyRefreshFailure.current = null; try { - await runUpdateCheck({ allowReady: true }); + await runUpdateCheck(); } finally { readyRefreshInProgress = false; } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index d1f18ab3a..5aed3ee9f 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1040,7 +1040,7 @@ Most services described here live under `apps/desktop/src/main/services/ | `state/` | `kvDb.ts`, `crsqliteExtension.ts`, `dbMaintenanceApi.ts`, `globalState.ts`, `projectState.ts`, `onConflictAudit.ts` | SQLite schema + open (WAL + `synchronous = NORMAL`), CRR extension loader, global state file, per-project state init. The desktop's Electron user-data `ade-state.json` holds machine-local shell state, including `AutoUpdatePreferences`; missing or malformed preference fields normalize to automatic installation off and idle-only safety on. `kvDb` also attaches the optional `maintenance` (`DbMaintenanceApi`) handle — retention prunes, zero-peers-only cr-sqlite compaction, and fragmentation-gated vacuum — whose interface and shared retention constants live in `dbMaintenanceApi.ts` and are invoked by the storage doctor. `globalState.upsertRecentProject` accepts `preserveRecentOrder` so reactivating an already-known project (by app focus, deep link, etc.) refreshes its `lastOpenedAt` in place instead of jumping it to the front of the recents list. Recent projects use stable keys: local rows are keyed by absolute root path, remote rows by `remote::`, so a remote path string never collides with a local project. Pinned rows are retained above normal recency ordering and survive beyond the cap. `model_picker_favorites` and `model_picker_recents` are per-project CRR tables shared by desktop, TUI, and iOS; they are primary-key-only so CRR can convert them, with the recents cap enforced in `modelPickerStore.ts`. `AdeDb.sync.discardUnpublishedChangesForTables(tableNames)` lets a service clear local CRR state for specific tables without leaking those clears to sync peers — it records the cleared tables and `through_db_version` in the local-only `local_crr_change_suppressions` table, and `exportChangesSince` filters local-site rows for those tables at or below that version on the way out. The local-only excluded set (still kept out of replication) includes that suppression table itself, the snapshot caches, `local_worktree_residual_cleanups`, `pr_auto_link_ignores`, `pull_request_ai_summaries`, and `runtime_processes`. `crsql_changes` DELETE statements run through a helper that swallows the read-only-table error the cr-sqlite extension raises when a CRR-managed table is wiped, with a `db.crr_changes_cleanup_skipped` warn log instead of failing the migration. | | `sync/` | `syncService.ts`, `syncHostService.ts`, `syncPeerService.ts`, `syncRemoteCommandService.ts`, `syncProtocol.ts`, `deviceRegistryService.ts`, `syncPairingStore.ts` | **Thin delegation to the ADE runtime's sync service.** The authoritative sync service now lives in `apps/ade-cli/src/services/sync/`; the desktop main-process instances default to a non-host viewer role for legacy state and tests. The old in-process host is disabled unless `ADE_ENABLE_DESKTOP_SYNC_HOST=1` (diagnostics only). Wire formats — WebSocket envelope, remote command routing, device registry, pairing secrets — are the same across both implementations. Viewer joins clear the local `devices` + `sync_cluster_state` rows and then call `db.sync.discardUnpublishedChangesForTables(["devices", "sync_cluster_state"])` so the resulting DELETE rows do not leak back to other peers; the peer client follows up with `syncPeerService.acknowledgeLocalDbVersion()` to advance the outbound cursor past the suppressed range. | | `tests/` | `testService.ts` | Test-suite execution + run history. | -| `updates/` | `autoUpdateService.ts`, `autoUpdateVersions.ts` | Electron auto-update wrapper around `electron-updater`. Owns the renderer-visible `AutoUpdateSnapshot` (`idle \| checking \| downloading \| ready \| installing \| error`, plus `currentVersion` / `latestKnownVersion` for the truthful-version surfaces), uses `compareUpdateVersions` (the SemVer-aware comparator in `autoUpdateVersions.ts`) to dedupe / supersede staged installers and to reconcile `pendingInstallUpdate` against the running version on next boot. Packaged builds schedule startup/periodic checks and downloads; source/dev launches construct the service without auto-check timers so missing `app-update.yml` never surfaces as a renderer error. ADE manually starts downloads after a cache-volume capacity preflight, checks the installed-app volume again before staging, classifies disk/quota/network/verification/permission/installer failures in the shared snapshot, preserves verified downloads when safe, and bounds the native installer handoff with a watchdog. A `ready` snapshot is not treated as proof the ZIP/EXE still exists: if the updater cache or `downloadedFile` is gone, the service re-downloads before uninstalling the runtime or calling native `quitAndInstall`, and it retries once when Squirrel reports a vanished-archive `network connection was lost`. The install is transactional: `quitAndInstall()` re-checks the staged version, and a consent that aborts before the native updater takes over lands in `snapshot.parked` (a typed `AutoUpdateInstallAbortReason`) so the exceptional shell banner offers a retry instead of silently losing the update; ordinary ready state remains in the top-right control. Restarting automatically is a separate machine-local `AutoUpdatePreferences` policy and defaults off. When enabled, its default-on idle safety waits for no active agent turns or work sessions (`RuntimeActivitySummary.idle`) before the grace period and renderer-visible countdown (`autoApplyPending`); users can opt into starting the countdown immediately instead. Cancel suppresses the next countdown (`autoApplySuppressedUntil`), disabling the preference clears it, and `ADE_DISABLE_AUTO_UPDATE_APPLY=1` is the process-level kill switch. `autoUpdateVersions.ts` also builds the changelog (`buildReleaseNotesUrl`) and GitHub release (`buildGithubReleaseUrl`) links. See [desktop auto-update disk-space behavior](./features/onboarding-and-settings/desktop-auto-update.md). | +| `updates/` | `autoUpdateService.ts`, `autoUpdateVersions.ts` | Electron auto-update wrapper around `electron-updater`. Owns the renderer-visible `AutoUpdateSnapshot` (`idle \| checking \| downloading \| ready \| installing \| error`, plus `currentVersion` / `latestKnownVersion` for the truthful-version surfaces), uses `compareUpdateVersions` (the SemVer-aware comparator in `autoUpdateVersions.ts`) to dedupe / supersede staged installers and to reconcile `pendingInstallUpdate` against the running version on next boot. A staged (`ready`) update does not pause those checks (an in-flight `quitAndInstall()` is the exception, because status stays `ready` until the native handoff): a strictly newer feed answer wipes the cached installer and downloads in its place, a same-or-older answer is ignored, and a failed check leaves the staged update untouched. Packaged builds schedule startup/periodic checks and downloads; source/dev launches construct the service without auto-check timers so missing `app-update.yml` never surfaces as a renderer error. ADE manually starts downloads after a cache-volume capacity preflight, checks the installed-app volume again before staging, classifies disk/quota/network/verification/permission/installer failures in the shared snapshot, preserves verified downloads when safe, and bounds the native installer handoff with a watchdog. A `ready` snapshot is not treated as proof the ZIP/EXE still exists: if the updater cache or `downloadedFile` is gone, the service re-downloads before uninstalling the runtime or calling native `quitAndInstall`, and it retries once when Squirrel reports a vanished-archive `network connection was lost`. The install is transactional: `quitAndInstall()` re-checks the staged version, and a consent that aborts before the native updater takes over lands in `snapshot.parked` (a typed `AutoUpdateInstallAbortReason`) so the exceptional shell banner offers a retry instead of silently losing the update; ordinary ready state remains in the top-right control. Restarting automatically is a separate machine-local `AutoUpdatePreferences` policy and defaults off. When enabled, its default-on idle safety waits for no active agent turns or work sessions (`RuntimeActivitySummary.idle`) before the grace period and renderer-visible countdown (`autoApplyPending`); users can opt into starting the countdown immediately instead. Cancel suppresses the next countdown (`autoApplySuppressedUntil`), disabling the preference clears it, and `ADE_DISABLE_AUTO_UPDATE_APPLY=1` is the process-level kill switch. `autoUpdateVersions.ts` also builds the changelog (`buildReleaseNotesUrl`) and GitHub release (`buildGithubReleaseUrl`) links. See [desktop auto-update disk-space behavior](./features/onboarding-and-settings/desktop-auto-update.md). | | `storage/` | `diskPressure.ts`, `volume.ts`, `storageInsightsService.ts`, `historyCompression.ts`, `storageLedger.ts`, `storageDbBreakdown.ts`, `storageMaintenanceJournal.ts` | Disk-full/recovery hardening + the storage doctor. `diskPressure` samples all ADE storage roots, classifies pressure with recovery hysteresis, and gates write-producing operation classes via `canPerform(kind)` (enforced at each start boundary in `agentChatService` / `ptyService` and the compressor). `storageInsightsService` builds the categorized Settings > Storage snapshot and preview-confirmed, link-safe cleanup, and runs the scheduled **storage doctor** maintenance sweep (`runMaintenanceNow` + post-boot/daily timers) that compresses history, reaps safe staging/backups/iOS build data, and invokes the kvDb DB-maintenance hooks. `storageLedger` is the declared bounding policy for every table/directory (with a CI coverage cross-check against `ADE_LAYOUT_DEFINITIONS`); `storageDbBreakdown` maps `dbstat` rows into the project-database breakdown; `storageMaintenanceJournal` reads/writes the 30-run doctor journal. `historyCompression` losslessly gzip-compresses inactive old transcripts/logs after byte-identity verification and exposes the transparent `.gz` read/reinflate helpers. Constructed in both `main.ts` and the `ade` runtime `bootstrap.ts`. See [features/storage-and-recovery/README.md](./features/storage-and-recovery/README.md). | | `usage/` | `usageTrackingService.ts`, `providerQuotaParsers.ts`, `usageStatsStore.ts`, `usageLedgerWorkerClient.ts`, `budgetCapService.ts`, `ledgers/localUsageLedgers.ts`, `usagePricing.ts`, `githubActivityStats.ts`, `accountUsageRollup.ts`, `accountUsageRollupStore.ts`, `accountUsageSource.ts`, `accountUsageLiveRefresh.ts` | Live provider quota/cost accounting, budget enforcement, and retrospective activity stats. `usageTrackingService.ts` owns polling, pacing, provider/GitHub cache orchestration, and `getAdeUsageStats`; `providerQuotaParsers.ts` normalizes Claude and Codex quota payload variants and classifies Codex windows by advertised duration rather than assuming the provider's primary/secondary positions. The stats read returns cached expensive sources plus live project-DB aggregates immediately, marks the result `refreshing` when stale, and revalidates provider ledgers / GitHub in the background. Expensive local-ledger aggregation runs through `usageLedgerWorkerClient.ts` in a separate process so it cannot block terminal input, project switching, or sync; packaged desktop/CLI builds ship a sidecar while the static runtime uses the equivalent embedded entrypoint. The worker streams NDJSON — a roster header then one line per provider as that provider finishes — so a timeout yields the providers that did land instead of discarding eight finished scans along with the ninth, and every budget in front of it (`USAGE_REFRESH_HISTORY_TIMEOUT_MS` for renderer IPC, `USAGE_REFRESH_HISTORY_REMOTE_TRANSPORT_TIMEOUT_MS` for the remote JSON-RPC leg) is derived from `LEDGER_WORKER_TIMEOUT_MS` so it outlives the worker rather than racing it. `usagePricing.ts` resolves per-model token rates, preferring the maintained public rate list over ADE's static table; `githubActivityStats.ts` owns the `gh` shell-outs behind the page's commit/PR/code-movement numbers and fails soft. `usageStatsStore.ts` aggregates AI calls, sessions, lanes, code movement, artifacts, automations, workers, streaks, and the local-only cross-client `usage_events` ledger. Local provider scanners live under `usage/ledgers/`. The `account*` modules add the account scope: each machine publishes day × provider × model aggregates (never a transcript record) into the CRR-replicated `usage_machine_rollups` / `usage_machine_rollup_meta`, `accountUsageLiveRefresh.ts` opportunistically pulls fresher rollups from reachable peers over `usage.getUsageRollup`, and `accountUsageSource.ts` counts two machines that read one shared transcript home only once, keyed on a `.ade-usage-source` marker id with digested roots as the fallback. Historical cost/tokens/code merge; live quota windows do not, because provider rate limits are per provider account rather than per machine. Budget caps can match a rule scope while `usd-per-run` evaluates usage records keyed to the active run id. For runtime-backed projects, the machine brain is the sole quota poller and the renderer consumes its pushed snapshot; `main.ts` does not start a competing project-context tracker. Threshold state remains shared at module level for the unbound/local contexts, and `main.ts` adds a final IPC-level dedup gate with a 10-minute TTL per `provider:threshold:resetCycle` key. | | `perf/` | `perfLog.ts`, `perfIpc.ts`, `metricsSampler.ts`, `chatTextProbe.ts`, `aggregator.ts` | Opt-in local performance harness. `ADE_PERF_RUN_ID` opens a JSONL event log, samples Electron process metrics (including main-process event-loop delay and V8 heap size), records IPC durations, accepts renderer perf marks/web-vitals/`streamSmoothness` windows, records `chatTextFlush` events from the assistant-text coalescer, and aggregates each run into `summary.json`. `perfLog.ts` holds the single `PERF_EVENT_KINDS` list that both the `PerfEventKind` type and the `isPerfEventKind` IPC guard derive from, so a new kind cannot be added to one and forgotten in the other. More than one process can append to the same log — Electron main and the `ade` runtime daemon both host chat sessions — so `appendEvent` must stay one `appendFileSync` of one already newline-terminated string (one `O_APPEND` write the kernel will not interleave). | diff --git a/docs/features/onboarding-and-settings/README.md b/docs/features/onboarding-and-settings/README.md index d0e52be70..3f7b62080 100644 --- a/docs/features/onboarding-and-settings/README.md +++ b/docs/features/onboarding-and-settings/README.md @@ -1099,15 +1099,20 @@ banner): periodic update checks, while dev/source launches leave those timers off to avoid surfacing missing-updater-config errors; if the new build is strictly newer, the cached installer dir is wiped and the snapshot - transitions back through `downloading`. `quitAndInstall()` is + transitions back through `downloading`. A staged update never stops a check; + see + [Checking while an update is staged](./desktop-auto-update.md#checking-while-an-update-is-staged) + for what each entry point does with a same, newer, or failed feed answer. + `userInitiated` only labels `autoUpdate.check_requested`; it does not + change whether the check runs. `quitAndInstall()` is transactional and asynchronous: it gates on the current snapshot being `ready`, - re-runs `updater.checkForUpdates()` with `allowReady: true` to + re-runs `updater.checkForUpdates()` to confirm the staged installer is still the latest, and only then flips the snapshot to `installing`, persists the `pendingInstallUpdate` global-state row, and calls - `updater.quitAndInstall(false, true)`. If the refresh check fails, - it surfaces the error, drops the cache, and clears the pending - install. A consent that aborts before the native updater takes over sets + `updater.quitAndInstall(false, true)`. If the refresh check fails, it parks as `refresh_failed`, returns + the snapshot to `ready` on the staged version, and keeps the + archive. A consent that aborts before the native updater takes over sets `snapshot.parked` with a typed `AutoUpdateInstallAbortReason` (`refresh_failed`, `install_preflight_failed`, `prepare_failed`, `prepare_timeout`, `handoff_failed`) so the shell banner can offer a retry. @@ -1730,19 +1735,23 @@ the previous two-way behaviour instead of reporting a state it cannot compute. exists — its settings moved into General + the top-bar Help menu. - **Auto-update install must refresh before quitting.** `quitAndInstall()` deliberately re-runs `updater.checkForUpdates()` - with `allowReady: true` before flipping to `installing`. Skipping + before flipping to `installing`. Skipping that step (e.g. a synchronous quitAndInstall) reintroduces the bug where ADE quits to install a stale download while a strictly newer build is available. Comparison goes through `compareUpdateVersions` — never `===` on the version string — because `v1.2.3` / `1.2.3-rc.1` / `1.2.3` all need consistent ordering on both the pending-install reconcile path and the supersede check. -- **`installing` is a sticky status.** While the snapshot is +- **`installing` is a sticky status; `ready` is not.** While the snapshot is `installing` the service ignores `update-not-available`, `checking-for-update`, and `error`, because the main process is in - the middle of quitAndInstall. New status checks should treat - `ready` and `installing` symmetrically when deciding whether to - cancel or override the staged update. + the middle of quitAndInstall. Checks keep running while status is `ready` + (same-or-older ignored, strictly newer supersedes, failed check leaves the + archive). Do not skip a check just because an update is staged. The one + window that *does* block a check is an already-running `quitAndInstall()`: + status stays `ready` until the native handoff, and a supersede in that + window would delete the archive about to be handed to Squirrel or NSIS. See + [Checking while an update is staged](./desktop-auto-update.md#checking-while-an-update-is-staged). - **A parked install is not a failure.** An aborted consent lands in `snapshot.parked`, not `error` — the download is still staged and the shell banner offers a **Restart now** retry. Keep parked distinct from the disk / diff --git a/docs/features/onboarding-and-settings/desktop-auto-update.md b/docs/features/onboarding-and-settings/desktop-auto-update.md index 4ec4f6b5b..d909febcf 100644 --- a/docs/features/onboarding-and-settings/desktop-auto-update.md +++ b/docs/features/onboarding-and-settings/desktop-auto-update.md @@ -34,7 +34,8 @@ differential cache copy — Squirrel pipes the pending ZIP recorded as service or calling native `quitAndInstall`, whenever the recorded `downloadedFile` or updater-cache ZIP/EXE is gone. - Re-downloads on the periodic ready check so a vanished cache does not sit - on the Install button until the user clicks it. + on the Install button until the user clicks it, and skips the feed check + for that cycle. - Treats install-phase `The network connection was lost` / `Cannot pipe` / `ENOENT` as a vanished local archive: restore the ZIP (which recreates the loopback server) and retry native handoff once. A second failure parks as @@ -213,6 +214,43 @@ bytes, and re-downloading the whole release on every retry is pure cost. A second consecutive failure on the same version stops trusting the archive and clears the updater cache. +## Checking while an update is staged + +A staged update does not pause update checks. The startup timer, the periodic +timer, the Settings **Check for updates** button, `ade update`, and the +`update.checkForUpdates` ADE action all reach the same code, and it behaves the +same way for all of them while the status is `ready`: + +| Feed answer | Result | +| --- | --- | +| Same or older than the staged version | Ignored (`autoUpdate.update_available_ignored`). The status, the staged version, and the cached archive do not change; `latestKnownVersion` still records what the feed reported. | +| Strictly newer | Supersedes. The recorded `downloadedFile` is dropped, the updater cache is wiped with reason `superseded_ready_update`, any auto-apply countdown for the old version is cancelled, and the snapshot runs `checking` → `downloading` → `ready` on the new version. The countdown re-arms on the new `ready`. | +| The check fails | Nothing changes. Both the `error` event and a rejected `checkForUpdates()` return early while the status is still `ready`, logging `autoUpdate.ready_check_failed`. | + +Every entry point first logs `autoUpdate.check_requested` with the current +status and a `userInitiated` label, so an operator can tell whether a check was +requested at all and what state it found. The flag does not change whether the +check runs. + +This is what stops the top-right pill from offering a release the feed has +already replaced. + +A failure *after* a supersede has started is an ordinary download failure: the +status is no longer `ready`, so it takes the normal error path. The old archive +is already gone at that point, which matches what a relaunch would have done. + +When the staged archive has vanished, that cycle restores the archive and skips +the feed check, so one cycle never runs a restore and a check at the same time. + +An install that is already running blocks the check entirely. The status stays +`ready` for the whole `quitAndInstall()` transaction, across the +`beforeQuitAndInstall` service uninstall, so a check started in that window +could supersede and delete the archive the install is about to hand to +Squirrel or NSIS. The pre-install refresh inside that transaction is the one +exception, and it keeps its own failure handling: a feed failure there aborts +the install with `parked.reason === "refresh_failed"` and returns the snapshot +to `ready` on the staged version. + ## Truthful version surfaces Every version surface reads from one shared snapshot so they can never disagree @@ -232,7 +270,7 @@ user sees the version they will get after the next restart rather than a stale ## Transactional install and exceptional recovery banners `quitAndInstall()` is transactional. Before flipping the snapshot to -`installing` it re-runs `updater.checkForUpdates({ allowReady: true })` to +`installing` it re-runs `updater.checkForUpdates()` to confirm the staged installer is still the latest, verifies the staged ZIP or NSIS installer is still on disk (and re-downloads it if macOS or Windows removed it), and only then uninstalls the background service, persists