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: [