From cde86c4f58c6d914a014d55d117d69d7ec018a3f Mon Sep 17 00:00:00 2001 From: Andrew Mikofalvy <5668128+amikofalvy@users.noreply.github.com> Date: Wed, 1 Jul 2026 12:15:19 -0700 Subject: [PATCH] fix(desktop): fix cross-channel + unbounded install-failed notice (#2344) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(desktop): stop stale/cross-channel install-failed notice from nagging every boot The boot-time failed-install detector (PRD-7149) armed attemptedInstall at download and only cleared it once the running version reached it, with no channel check and no retry bound. Two failure modes resulted: - Cross-channel contamination: stable and beta builds share one state.json (same appId/productName), so a version armed by one channel poisoned the other channel's boot check. A beta build surfaced Update to didnt install for a version its update-available veto guarantees it can never install, and the record never cleared, so the card re-fired every boot. Now cleared silently when attemptedInstall and the running build are on different channels. - Unbounded nag: a genuinely stuck install re-surfaced the card every boot forever. Now capped at INSTALL_FAILURE_MAX_SURFACES (3) surfacings, after which the record is dropped; the 7-day stuck-hint stays the backstop. Adds attemptedInstallSurfacedCount to AppState (defensively coerced) and the attempted-install-cross-channel / install-failed-giveup dispatch tags. Genuine same-channel failures still surface as before. * address review: clear versionPendingInstall on install abandonment + test hardening Review (claude bot) flagged that the giveup branch cleared attemptedInstall but left versionPendingInstall armed. For a higher-MMP attempted version the MMP-only stale-pending reconciliation cannot clear it, leaving a phantom 'ready to install' banner (and dedup-blocking a genuine re-download). Fix: the giveup AND the cross-channel abandonment branches now also clear versionPendingInstall (the cross-channel branch has the identical latent issue — a stale stable pending marker on a beta build). Tests: assert versionPendingInstall is cleared in the giveup + cross-channel paths (higher-MMP versions so stale-pending does not pre-clear); add persist- failure tests for both new branches; add a same-version re-download budget- preserve test; assert the surface counter resets on success reconcile. GitOrigin-RevId: 32b8aeac91d9ab4a4223d9f2ad288919490e43de --- .changeset/fix-attempted-install-nag.md | 11 + packages/desktop/src/main/auto-updater.ts | 74 +++++-- packages/desktop/src/main/state-store.ts | 9 + .../tests/integration/auto-updater.test.ts | 195 ++++++++++++++++++ .../tests/unit/state-store-m3-fields.test.ts | 21 ++ 5 files changed, 297 insertions(+), 13 deletions(-) create mode 100644 .changeset/fix-attempted-install-nag.md diff --git a/.changeset/fix-attempted-install-nag.md b/.changeset/fix-attempted-install-nag.md new file mode 100644 index 000000000..83a6ebd0e --- /dev/null +++ b/.changeset/fix-attempted-install-nag.md @@ -0,0 +1,11 @@ +--- +"@inkeep/open-knowledge": patch +--- + +Stop OK Desktop's "Update to X didn't install" notice from firing spuriously and re-appearing on every launch. The boot-time failed-install detector now handles two cases it previously got wrong: + +1. Cross-channel state. The stable and beta builds share one settings file (same app id), so a version that one channel armed as its pending install could poison the other channel's boot check. A beta build would show "Update to 0.23.0 didn't install" for a stable version it can never install (cross-channel updates are blocked), and there was no way for it to clear, so the notice returned on every launch. That stale cross-channel record is now cleared silently. + +2. No retry bound. A genuinely stuck install (a persistently-failing installer, a pulled release) re-surfaced the notice on every boot forever. It is now shown at most 3 times per failed update, after which the record is dropped. The 7-day "updates paused" hint remains the backstop. + +Genuine same-channel install failures still surface as before, so a real failed update is not hidden. diff --git a/packages/desktop/src/main/auto-updater.ts b/packages/desktop/src/main/auto-updater.ts index 136209800..b2371ea3d 100644 --- a/packages/desktop/src/main/auto-updater.ts +++ b/packages/desktop/src/main/auto-updater.ts @@ -63,6 +63,8 @@ export type DispatchKind = | 'stale-pending-cleared' | 'attempted-install-reconciled' | 'install-failed-on-boot' + | 'install-failed-giveup' + | 'attempted-install-cross-channel' | 'cross-channel-blocked'; interface StartAutoUpdaterOpts { @@ -127,6 +129,8 @@ export const RELAUNCH_WATCHDOG_MS = 15_000; export const STUCK_HINT_THRESHOLD_MS = 7 * 24 * 60 * 60 * 1000; +export const INSTALL_FAILURE_MAX_SURFACES = 3; + export const STUCK_HINT_DOWNLOAD_URL = 'https://github.com/inkeep/open-knowledge/releases'; const WHATS_NEW_LIVE_WINDOW_MS = 60_000; @@ -526,7 +530,13 @@ export function startAutoUpdater(opts: StartAutoUpdaterOpts): StartAutoUpdaterHa } if ( !persistSafely( - { ...state, versionPendingInstall: version, attemptedInstall: version }, + { + ...state, + versionPendingInstall: version, + attemptedInstall: version, + attemptedInstallSurfacedCount: + state.attemptedInstall === version ? state.attemptedInstallSurfacedCount : 0, + }, 'update-downloaded', ) ) @@ -659,7 +669,7 @@ export function startAutoUpdater(opts: StartAutoUpdaterOpts): StartAutoUpdaterHa if (state.attemptedInstall) { const attempted = state.attemptedInstall; if (installReached(currentVersion, attempted)) { - const next = { ...state, attemptedInstall: null }; + const next = { ...state, attemptedInstall: null, attemptedInstallSurfacedCount: 0 }; if (persistSafely(next, 'attempted-install-reconciled')) { state = next; onDispatch?.('attempted-install-reconciled'); @@ -669,23 +679,61 @@ export function startAutoUpdater(opts: StartAutoUpdaterOpts): StartAutoUpdaterHa running: currentVersion, }); } - } else if (updatesEnabled) { - const next = { ...state, versionPendingInstall: attempted }; - if (persistSafely(next, 'install-failed-on-boot')) { + } else if (channelFromVersion(attempted) !== channelFromVersion(currentVersion)) { + const next = { + ...state, + attemptedInstall: null, + attemptedInstallSurfacedCount: 0, + versionPendingInstall: null, + }; + if (persistSafely(next, 'attempted-install-cross-channel')) { state = next; - logger.warn('attempted install did not take — surfacing failure notice', { + logger.info('cleared cross-channel attemptedInstall residue', { attempted, running: currentVersion, }); - const fireInstallFailed = (): void => { - broadcastToAllWindows('ok:update:relaunch-failed', { - version: attempted, - downloadUrl: STUCK_HINT_DOWNLOAD_URL, + onDispatch?.('attempted-install-cross-channel'); + } + } else if (updatesEnabled) { + if (state.attemptedInstallSurfacedCount >= INSTALL_FAILURE_MAX_SURFACES) { + const next = { + ...state, + attemptedInstall: null, + attemptedInstallSurfacedCount: 0, + versionPendingInstall: null, + }; + if (persistSafely(next, 'install-failed-giveup')) { + state = next; + logger.warn('attempted install exhausted its retry budget — clearing record', { + attempted, + running: currentVersion, + surfaced: INSTALL_FAILURE_MAX_SURFACES, }); + onDispatch?.('install-failed-giveup'); + } + } else { + const next = { + ...state, + versionPendingInstall: attempted, + attemptedInstallSurfacedCount: state.attemptedInstallSurfacedCount + 1, }; - if (whenRendererReady) whenRendererReady(fireInstallFailed); - else fireInstallFailed(); - onDispatch?.('install-failed-on-boot'); + if (persistSafely(next, 'install-failed-on-boot')) { + state = next; + logger.warn('attempted install did not take — surfacing failure notice', { + attempted, + running: currentVersion, + surfaced: next.attemptedInstallSurfacedCount, + }); + const fireInstallFailed = (): void => { + broadcastToAllWindows('ok:update:relaunch-failed', { + version: attempted, + downloadUrl: STUCK_HINT_DOWNLOAD_URL, + }); + }; + if (whenRendererReady) whenRendererReady(fireInstallFailed); + else fireInstallFailed(); + onDispatch?.('install-failed-on-boot'); + } } } } diff --git a/packages/desktop/src/main/state-store.ts b/packages/desktop/src/main/state-store.ts index ccdd29f35..a1a62f313 100644 --- a/packages/desktop/src/main/state-store.ts +++ b/packages/desktop/src/main/state-store.ts @@ -28,6 +28,7 @@ export interface AppState { lastOpenedProject: string | null; versionPendingInstall: string | null; attemptedInstall: string | null; + attemptedInstallSurfacedCount: number; lastSeenVersion: string | null; lastSuccessfulCheckAt: string | null; stuckHintShown: boolean; @@ -47,6 +48,7 @@ export function emptyState(): AppState { lastOpenedProject: null, versionPendingInstall: null, attemptedInstall: null, + attemptedInstallSurfacedCount: 0, lastSeenVersion: null, lastSuccessfulCheckAt: null, stuckHintShown: false, @@ -294,6 +296,12 @@ export function parseAppState(raw: unknown): AppState | null { const versionPendingInstall = typeof obj.versionPendingInstall === 'string' ? obj.versionPendingInstall : null; const attemptedInstall = typeof obj.attemptedInstall === 'string' ? obj.attemptedInstall : null; + const attemptedInstallSurfacedCount = + typeof obj.attemptedInstallSurfacedCount === 'number' && + Number.isInteger(obj.attemptedInstallSurfacedCount) && + obj.attemptedInstallSurfacedCount >= 0 + ? obj.attemptedInstallSurfacedCount + : 0; const lastSeenVersion = typeof obj.lastSeenVersion === 'string' ? obj.lastSeenVersion : null; const lastSuccessfulCheckAt = typeof obj.lastSuccessfulCheckAt === 'string' ? obj.lastSuccessfulCheckAt : null; @@ -319,6 +327,7 @@ export function parseAppState(raw: unknown): AppState | null { lastOpenedProject, versionPendingInstall, attemptedInstall, + attemptedInstallSurfacedCount, lastSeenVersion, lastSuccessfulCheckAt, stuckHintShown, diff --git a/packages/desktop/tests/integration/auto-updater.test.ts b/packages/desktop/tests/integration/auto-updater.test.ts index da7f8ef39..0477c9624 100644 --- a/packages/desktop/tests/integration/auto-updater.test.ts +++ b/packages/desktop/tests/integration/auto-updater.test.ts @@ -5,6 +5,7 @@ import { bootAutoUpdater, buildCheckNowResultFromError, type DispatchKind, + INSTALL_FAILURE_MAX_SURFACES, type IpcMainLike, installReached, isClassifiedUpdaterError, @@ -1002,10 +1003,12 @@ describe('boot-time failed-install detection', () => { const { rig } = makeRig({ attemptedInstall: '0.16.0-beta.3', appVersion: '0.16.0-beta.3', + attemptedInstallSurfacedCount: 2, }); const failed = rig.captured.filter((c) => c.channel === 'ok:update:relaunch-failed'); expect(failed).toHaveLength(0); expect(rig.state.attemptedInstall).toBeNull(); + expect(rig.state.attemptedInstallSurfacedCount).toBe(0); expect(rig.dispatches).toContain('attempted-install-reconciled' as DispatchKind); expect(rig.dispatches).not.toContain('install-failed-on-boot' as DispatchKind); }); @@ -1014,9 +1017,11 @@ describe('boot-time failed-install detection', () => { const { rig } = makeRig({ attemptedInstall: '0.16.0-beta.3', appVersion: '0.16.0', + attemptedInstallSurfacedCount: 2, }); expect(rig.captured.filter((c) => c.channel === 'ok:update:relaunch-failed')).toHaveLength(0); expect(rig.state.attemptedInstall).toBeNull(); + expect(rig.state.attemptedInstallSurfacedCount).toBe(0); expect(rig.dispatches).toContain('attempted-install-reconciled' as DispatchKind); }); @@ -1034,6 +1039,196 @@ describe('boot-time failed-install detection', () => { expect(rig.state.attemptedInstall).toBe('0.16.0-beta.3'); }); + test('cross-channel residue (stable attempted, beta running) → silently cleared, no notice', () => { + const { rig } = makeRig({ + attemptedInstall: '0.24.0', + versionPendingInstall: '0.24.0', + appVersion: '0.23.0-beta.1', + }); + expect(rig.captured.filter((c) => c.channel === 'ok:update:relaunch-failed')).toHaveLength(0); + expect(rig.state.attemptedInstall).toBeNull(); + expect(rig.state.versionPendingInstall).toBeNull(); + expect(rig.dispatches).toContain('attempted-install-cross-channel' as DispatchKind); + expect(rig.dispatches).not.toContain('install-failed-on-boot' as DispatchKind); + expect(rig.dispatches).not.toContain('attempted-install-reconciled' as DispatchKind); + }); + + test('cross-channel residue (beta attempted, older stable running) → silently cleared', () => { + const { rig } = makeRig({ + attemptedInstall: '0.23.0-beta.5', + appVersion: '0.22.0', + }); + expect(rig.captured.filter((c) => c.channel === 'ok:update:relaunch-failed')).toHaveLength(0); + expect(rig.state.attemptedInstall).toBeNull(); + expect(rig.dispatches).toContain('attempted-install-cross-channel' as DispatchKind); + expect(rig.dispatches).not.toContain('install-failed-on-boot' as DispatchKind); + }); + + test('same-channel failure below budget → surfaces AND increments the counter', () => { + const { rig } = makeRig({ + attemptedInstall: '0.16.0-beta.3', + appVersion: '0.16.0-beta.1', + attemptedInstallSurfacedCount: 1, + }); + expect(rig.captured.filter((c) => c.channel === 'ok:update:relaunch-failed')).toHaveLength(1); + expect(rig.state.attemptedInstallSurfacedCount).toBe(2); + expect(rig.dispatches).toContain('install-failed-on-boot' as DispatchKind); + }); + + test('retry budget exhausted → gives up, clears the record incl. pending marker', () => { + const { rig } = makeRig({ + attemptedInstall: '0.17.0-beta.1', + versionPendingInstall: '0.17.0-beta.1', + appVersion: '0.16.0-beta.1', + attemptedInstallSurfacedCount: INSTALL_FAILURE_MAX_SURFACES, + }); + expect(rig.captured.filter((c) => c.channel === 'ok:update:relaunch-failed')).toHaveLength(0); + expect(rig.state.attemptedInstall).toBeNull(); + expect(rig.state.attemptedInstallSurfacedCount).toBe(0); + expect(rig.state.versionPendingInstall).toBeNull(); + expect(rig.dispatches).toContain('install-failed-giveup' as DispatchKind); + expect(rig.dispatches).not.toContain('install-failed-on-boot' as DispatchKind); + }); + + test('surfaces exactly INSTALL_FAILURE_MAX_SURFACES times across reboots, then gives up', () => { + const state: AppState = { + ...emptyState(), + lastSeenVersion: '0.16.0-beta.1', + attemptedInstall: '0.16.0-beta.3', + }; + const boot = () => { + const captured: CapturedSend[] = []; + const dispatches: DispatchKind[] = []; + startAutoUpdater({ + updater: new FakeUpdater(), + ipcMain: makeFakeIpc(), + readState: () => state, + writeState: (next) => { + Object.assign(state, next); + }, + getPrimaryWindow: () => makeFakeWindow(captured), + getAppVersion: () => '0.16.0-beta.1', + isPackaged: true, + clock: makeFakeClock(), + now: () => new Date(), + onDispatch: (k) => dispatches.push(k), + logger: { + info: mock(() => {}), + warn: mock(() => {}), + error: mock(() => {}), + debug: mock(() => {}), + }, + }); + return { captured, dispatches }; + }; + for (let i = 0; i < INSTALL_FAILURE_MAX_SURFACES; i++) { + const b = boot(); + expect(b.captured.filter((c) => c.channel === 'ok:update:relaunch-failed')).toHaveLength(1); + expect(b.dispatches).toContain('install-failed-on-boot' as DispatchKind); + } + expect(state.attemptedInstallSurfacedCount).toBe(INSTALL_FAILURE_MAX_SURFACES); + const giveup = boot(); + expect(giveup.captured.filter((c) => c.channel === 'ok:update:relaunch-failed')).toHaveLength( + 0, + ); + expect(giveup.dispatches).toContain('install-failed-giveup' as DispatchKind); + expect(state.attemptedInstall).toBeNull(); + const after = boot(); + expect(after.dispatches).not.toContain('install-failed-on-boot' as DispatchKind); + expect(after.dispatches).not.toContain('install-failed-giveup' as DispatchKind); + }); + + test('update-downloaded of a NEW version resets the surface counter', () => { + const { rig } = makeRig({ + attemptedInstall: '0.16.0-beta.3', + appVersion: '0.16.0-beta.1', + attemptedInstallSurfacedCount: 1, + }); + expect(rig.state.attemptedInstallSurfacedCount).toBe(2); + rig.updater.emit('update-downloaded', { version: '0.16.0-beta.5' }); + expect(rig.state.attemptedInstall).toBe('0.16.0-beta.5'); + expect(rig.state.attemptedInstallSurfacedCount).toBe(0); + }); + + test('update-downloaded of the SAME version preserves the surface counter', () => { + const { rig } = makeRig({ + isPackaged: false, + attemptedInstall: '0.16.0-beta.3', + appVersion: '0.16.0-beta.1', + attemptedInstallSurfacedCount: 2, + }); + rig.updater.emit('update-downloaded', { version: '0.16.0-beta.3' }); + expect(rig.state.versionPendingInstall).toBe('0.16.0-beta.3'); + expect(rig.state.attemptedInstall).toBe('0.16.0-beta.3'); + expect(rig.state.attemptedInstallSurfacedCount).toBe(2); + }); + + test('persist failure on the cross-channel branch → no clear, no dispatch', () => { + const captured: CapturedSend[] = []; + const state: AppState = { + ...emptyState(), + lastSeenVersion: '0.23.0-beta.1', + attemptedInstall: '0.24.0', + }; + const dispatches: DispatchKind[] = []; + startAutoUpdater({ + updater: new FakeUpdater(), + ipcMain: makeFakeIpc(), + readState: () => state, + writeState: () => { + throw new Error('EACCES'); + }, + getPrimaryWindow: () => makeFakeWindow(captured), + getAppVersion: () => '0.23.0-beta.1', + isPackaged: true, + clock: makeFakeClock(), + now: () => new Date(), + onDispatch: (k) => dispatches.push(k), + logger: { + info: mock(() => {}), + warn: mock(() => {}), + error: mock(() => {}), + debug: mock(() => {}), + }, + }); + expect(dispatches).not.toContain('attempted-install-cross-channel' as DispatchKind); + expect(state.attemptedInstall).toBe('0.24.0'); + }); + + test('persist failure on the giveup branch → record stays armed, no dispatch', () => { + const captured: CapturedSend[] = []; + const state: AppState = { + ...emptyState(), + lastSeenVersion: '0.16.0-beta.1', + attemptedInstall: '0.16.0-beta.3', + attemptedInstallSurfacedCount: INSTALL_FAILURE_MAX_SURFACES, + }; + const dispatches: DispatchKind[] = []; + startAutoUpdater({ + updater: new FakeUpdater(), + ipcMain: makeFakeIpc(), + readState: () => state, + writeState: () => { + throw new Error('EACCES'); + }, + getPrimaryWindow: () => makeFakeWindow(captured), + getAppVersion: () => '0.16.0-beta.1', + isPackaged: true, + clock: makeFakeClock(), + now: () => new Date(), + onDispatch: (k) => dispatches.push(k), + logger: { + info: mock(() => {}), + warn: mock(() => {}), + error: mock(() => {}), + debug: mock(() => {}), + }, + }); + expect(dispatches).not.toContain('install-failed-giveup' as DispatchKind); + expect(state.attemptedInstall).toBe('0.16.0-beta.3'); + expect(state.attemptedInstallSurfacedCount).toBe(INSTALL_FAILURE_MAX_SURFACES); + }); + test('persist failure on the failure branch → no broadcast, no dispatch', () => { const updater = new FakeUpdater(); const ipc = makeFakeIpc(); diff --git a/packages/desktop/tests/unit/state-store-m3-fields.test.ts b/packages/desktop/tests/unit/state-store-m3-fields.test.ts index 21c869770..8d070aea7 100644 --- a/packages/desktop/tests/unit/state-store-m3-fields.test.ts +++ b/packages/desktop/tests/unit/state-store-m3-fields.test.ts @@ -39,6 +39,27 @@ describe('parseAppState M3 fields — coercion', () => { expect(parseAppState({ recentProjects: [] })?.attemptedInstall).toBeNull(); }); + test('attemptedInstallSurfacedCount: defaults to 0, round-trips a non-negative integer, coerces junk to 0', () => { + expect(emptyState().attemptedInstallSurfacedCount).toBe(0); + expect( + parseAppState({ recentProjects: [], attemptedInstallSurfacedCount: 2 }) + ?.attemptedInstallSurfacedCount, + ).toBe(2); + expect(parseAppState({ recentProjects: [] })?.attemptedInstallSurfacedCount).toBe(0); + expect( + parseAppState({ recentProjects: [], attemptedInstallSurfacedCount: -3 }) + ?.attemptedInstallSurfacedCount, + ).toBe(0); + expect( + parseAppState({ recentProjects: [], attemptedInstallSurfacedCount: 1.5 }) + ?.attemptedInstallSurfacedCount, + ).toBe(0); + expect( + parseAppState({ recentProjects: [], attemptedInstallSurfacedCount: '2' }) + ?.attemptedInstallSurfacedCount, + ).toBe(0); + }); + test('M1-forward-compat: pre-M3 blob without M3 keys returns valid state with defaults', () => { const raw = { recentProjects: [