diff --git a/apps/desktop/src/store/updates.test.ts b/apps/desktop/src/store/updates.test.ts index 9dd1cbed6a..161f7480f1 100644 --- a/apps/desktop/src/store/updates.test.ts +++ b/apps/desktop/src/store/updates.test.ts @@ -290,7 +290,11 @@ describe('requestActiveUpdate', () => { vi.useRealTimers() }) - afterEach(() => { + afterEach(async () => { + // Drain any backend apply this suite kicked off: applyBackendUpdate() now + // memoizes the in-flight run, so a dangling promise here would be handed + // to the next suite's tests instead of a fresh run. + await vi.waitFor(() => expect($backendUpdateApply.get().applying).toBe(false), { timeout: 5000 }) setRemote(false) delete (globalThis as unknown as { window?: unknown }).window }) @@ -465,6 +469,7 @@ describe('applyBackendUpdate recovery', () => { checkHermesUpdateSpy.mockReset() updateHermesSpy.mockReset() getActionStatusSpy.mockReset() + $backendUpdateStatus.set(null) $backendUpdateApply.set({ applying: false, stage: 'idle', @@ -482,16 +487,14 @@ describe('applyBackendUpdate recovery', () => { }) it('waits for the backend to return after the restart drops the connection, then clears the overlay', async () => { - updateHermesSpy.mockResolvedValue({ ok: true, name: 'update', pid: 1 }) - getActionStatusSpy.mockRejectedValue(new Error('ECONNREFUSED')) - checkHermesUpdateSpy.mockResolvedValue({ - install_method: 'git', - current_version: '0.16.0', - behind: 0, - update_available: false, - can_apply: true, - update_command: 'hermes update', - message: null + const actionId = 'd'.repeat(32) + updateHermesSpy.mockResolvedValue({ action_id: actionId, ok: true, name: 'update', pid: 1 }) + getActionStatusSpy.mockRejectedValueOnce(new Error('ECONNREFUSED')).mockResolvedValueOnce({ + exit_code: null, + lines: [`=== hermes-update completed ${actionId} ===`], + name: 'update', + pid: null, + running: false }) const promise = applyBackendUpdate() @@ -504,7 +507,8 @@ describe('applyBackendUpdate recovery', () => { }) it('surfaces backend update action log lines while the action is running', async () => { - updateHermesSpy.mockResolvedValue({ ok: true, name: 'update', pid: 1 }) + const actionId = 'e'.repeat(32) + updateHermesSpy.mockResolvedValue({ action_id: actionId, ok: true, name: 'update', pid: 1 }) getActionStatusSpy .mockResolvedValueOnce({ exit_code: null, @@ -514,15 +518,13 @@ describe('applyBackendUpdate recovery', () => { running: true }) .mockRejectedValueOnce(new Error('ECONNREFUSED')) - checkHermesUpdateSpy.mockResolvedValue({ - install_method: 'git', - current_version: '0.16.0', - behind: 0, - update_available: false, - can_apply: true, - update_command: 'hermes update', - message: null - }) + .mockResolvedValueOnce({ + exit_code: null, + lines: [`=== hermes-update completed ${actionId} ===`], + name: 'update', + pid: null, + running: false + }) const promise = applyBackendUpdate() await vi.advanceTimersByTimeAsync(1500) @@ -537,18 +539,323 @@ describe('applyBackendUpdate recovery', () => { await promise }) + it('keeps waiting past the old 45-second cutoff while the update action is running', async () => { + const actionId = 'f'.repeat(32) + updateHermesSpy.mockResolvedValue({ action_id: actionId, ok: true, name: 'hermes-update', pid: 1 }) + + for (let attempt = 0; attempt < 31; attempt += 1) { + getActionStatusSpy.mockResolvedValueOnce({ + exit_code: null, + lines: ['=== hermes-update started now ===', `step ${attempt}`], + name: 'hermes-update', + pid: 1, + running: true + }) + } + + getActionStatusSpy.mockRejectedValueOnce(new Error('ECONNREFUSED')).mockResolvedValueOnce({ + exit_code: null, + lines: [`=== hermes-update completed ${actionId} ===`], + name: 'hermes-update', + pid: null, + running: false + }) + + const promise = applyBackendUpdate() + await vi.advanceTimersByTimeAsync(46500) + + expect($backendUpdateApply.get().applying).toBe(true) + expect($backendUpdateApply.get().stage).toBe('pull') + + await vi.advanceTimersByTimeAsync(5000) + await expect(promise).resolves.toMatchObject({ ok: true }) + }) + + it('treats a successful no-op as complete without waiting for a restart', async () => { + updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 }) + getActionStatusSpy.mockResolvedValue({ + exit_code: 0, + lines: ['stale output from another run', '=== hermes-update started now ===', '✓ Already up to date!'], + name: 'hermes-update', + pid: 1, + running: false + }) + + const promise = applyBackendUpdate() + await vi.advanceTimersByTimeAsync(1500) + const result = await promise + + expect(result.ok).toBe(true) + expect($backendUpdateApply.get().stage).toBe('idle') + }) + + it('treats a successful dependency repair as complete without waiting for a restart', async () => { + updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 }) + getActionStatusSpy.mockResolvedValue({ + exit_code: 0, + lines: ['=== hermes-update started now ===', '✓ Dependencies repaired!', '✓ Update complete!'], + name: 'hermes-update', + pid: 1, + running: false + }) + + const promise = applyBackendUpdate() + await vi.advanceTimersByTimeAsync(1500) + await expect(promise).resolves.toMatchObject({ ok: true }) + expect($backendUpdateApply.get().stage).toBe('idle') + }) + + it('trusts the current action exit code without parsing its output', async () => { + updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 }) + getActionStatusSpy.mockResolvedValue({ + exit_code: 0, + lines: ['✓ Already up to date!'], + name: 'hermes-update', + pid: 1, + running: false + }) + const promise = applyBackendUpdate() + await vi.advanceTimersByTimeAsync(1500) + await expect(promise).resolves.toMatchObject({ ok: true }) + expect(checkHermesUpdateSpy).not.toHaveBeenCalled() + }) + + it('waits for current-action completion proof after the backend restarts', async () => { + const actionId = 'a'.repeat(32) + updateHermesSpy.mockResolvedValue({ action_id: actionId, ok: true, name: 'hermes-update', pid: 1 }) + getActionStatusSpy + .mockRejectedValueOnce(new Error('ECONNREFUSED')) + .mockResolvedValueOnce({ + exit_code: null, + lines: ['Update complete!', `=== hermes-update completed ${'c'.repeat(32)} ===`], + name: 'hermes-update', + pid: null, + running: false + }) + .mockResolvedValueOnce({ + exit_code: null, + lines: ['Update complete!', `=== hermes-update completed ${actionId} ===`], + name: 'hermes-update', + pid: null, + running: false + }) + + const promise = applyBackendUpdate() + await vi.advanceTimersByTimeAsync(5000) + await expect(promise).resolves.toMatchObject({ ok: true }) + expect(checkHermesUpdateSpy).not.toHaveBeenCalled() + }) + + it('accepts its terminal receipt when a verbose update pushes the start marker out of the log tail', async () => { + const actionId = 'b'.repeat(32) + updateHermesSpy.mockResolvedValue({ action_id: actionId, ok: true, name: 'hermes-update', pid: 1 }) + getActionStatusSpy.mockRejectedValueOnce(new Error('ECONNREFUSED')).mockResolvedValueOnce({ + exit_code: null, + lines: ['final build output', 'Update complete!', `=== hermes-update completed ${actionId} ===`], + name: 'hermes-update', + pid: null, + running: false + }) + + const promise = applyBackendUpdate() + await vi.advanceTimersByTimeAsync(5000) + + await expect(promise).resolves.toMatchObject({ ok: true }) + expect(getActionStatusSpy).toHaveBeenCalledWith('hermes-update', 2000) + }) + + it('proves a pre-action-ID backend reached its requested commit after restart', async () => { + $backendUpdateStatus.set({ + behind: 2, + commits: [{ at: 1, author: 'Nous', sha: 'requested-target', summary: 'target' }], + fetchedAt: 1, + supported: true, + targetSha: 'backend:0.18.2', + updateAvailable: true + }) + updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 }) + getActionStatusSpy.mockRejectedValueOnce(new Error('ECONNREFUSED')).mockResolvedValue({ + exit_code: null, + lines: ['verbose output', 'Update complete!'], + name: 'hermes-update', + pid: null, + running: false + }) + checkHermesUpdateSpy + .mockResolvedValueOnce({ + behind: null, + can_apply: true, + commits: [], + current_version: '0.18.2', + install_method: 'git', + message: 'offline', + update_available: false, + update_command: 'hermes update' + }) + .mockResolvedValueOnce({ + behind: 1, + can_apply: true, + commits: [{ at: 2, author: 'Nous', sha: 'newer-commit', summary: 'newer' }], + current_version: '0.18.2', + install_method: 'git', + message: null, + update_available: true, + update_command: 'hermes update' + }) + + const promise = applyBackendUpdate() + await vi.advanceTimersByTimeAsync(5000) + + await expect(promise).resolves.toMatchObject({ ok: true }) + expect(checkHermesUpdateSpy).toHaveBeenCalledTimes(2) + }) + + it('proves a fast pre-action-ID packaged update by its changed version', async () => { + $backendUpdateStatus.set({ + behind: 1, + commits: [], + fetchedAt: 1, + supported: true, + targetSha: 'backend:0.18.2', + updateAvailable: true + }) + updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 }) + getActionStatusSpy.mockResolvedValue({ + exit_code: null, + lines: ['verbose output without a retained start marker'], + name: 'hermes-update', + pid: null, + running: false + }) + checkHermesUpdateSpy.mockResolvedValue({ + behind: -1, + can_apply: true, + commits: [], + current_version: '0.18.3', + install_method: 'pip', + message: null, + update_available: true, + update_command: 'hermes update' + }) + + const promise = applyBackendUpdate() + await vi.advanceTimersByTimeAsync(1500) + + await expect(promise).resolves.toMatchObject({ ok: true }) + expect(checkHermesUpdateSpy).toHaveBeenCalledWith(true) + }) + + it('resumes action polling after a transient status failure', async () => { + updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 }) + getActionStatusSpy + .mockRejectedValueOnce(new Error('ECONNRESET')) + .mockResolvedValueOnce({ + exit_code: null, + lines: ['=== hermes-update started now ===', 'still running'], + name: 'hermes-update', + pid: 1, + running: true + }) + .mockResolvedValueOnce({ + exit_code: 0, + lines: ['=== hermes-update started now ===', 'Update complete!'], + name: 'hermes-update', + pid: 1, + running: false + }) + + const promise = applyBackendUpdate() + await vi.advanceTimersByTimeAsync(5000) + await expect(promise).resolves.toMatchObject({ ok: true }) + expect(getActionStatusSpy).toHaveBeenCalledTimes(3) + }) + + it('restores the fixed action deadline after reconnecting', async () => { + updateHermesSpy.mockResolvedValue({ action_id: 'a'.repeat(32), ok: true, name: 'hermes-update', pid: 1 }) + const running = { + exit_code: null, + lines: ['still running'], + name: 'hermes-update', + pid: 1, + running: true + } + + for (let attempt = 0; attempt < 119; attempt += 1) { + getActionStatusSpy.mockResolvedValueOnce(running) + } + getActionStatusSpy.mockRejectedValueOnce(new Error('ECONNRESET')).mockResolvedValue(running) + + const promise = applyBackendUpdate() + await vi.advanceTimersByTimeAsync(6 * 60 * 1000 + 1500) + + await expect(promise).resolves.toMatchObject({ error: 'apply-failed', ok: false }) + expect($backendUpdateApply.get().stage).toBe('error') + }) + + it('shares one in-flight update between concurrent apply requests', async () => { + updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 }) + getActionStatusSpy.mockResolvedValue({ + exit_code: 0, + lines: ['=== hermes-update started now ===', '✓ Already up to date!'], + name: 'hermes-update', + pid: 1, + running: false + }) + + const first = applyBackendUpdate() + const second = applyBackendUpdate() + + expect(second).toBe(first) + await vi.advanceTimersByTimeAsync(1500) + await Promise.all([first, second]) + expect(updateHermesSpy).toHaveBeenCalledTimes(1) + }) + + it('fails closed when the update action never reaches a terminal state', async () => { + updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 }) + getActionStatusSpy.mockResolvedValue({ + exit_code: null, + lines: ['=== hermes-update started now ===', 'still running'], + name: 'hermes-update', + pid: 1, + running: true + }) + + const promise = applyBackendUpdate() + await vi.advanceTimersByTimeAsync(6 * 60 * 1000 + 1500) + await expect(promise).resolves.toMatchObject({ ok: false, error: 'apply-failed' }) + expect($backendUpdateApply.get().stage).toBe('error') + }) + + it('fails immediately when the update action exits nonzero', async () => { + updateHermesSpy.mockResolvedValue({ ok: true, name: 'hermes-update', pid: 1 }) + getActionStatusSpy.mockResolvedValue({ + exit_code: 1, + lines: ['=== hermes-update started now ===', 'update failed'], + name: 'hermes-update', + pid: 1, + running: false + }) + + const promise = applyBackendUpdate() + await vi.advanceTimersByTimeAsync(1500) + await expect(promise).resolves.toMatchObject({ ok: false, error: 'apply-failed' }) + expect(checkHermesUpdateSpy).not.toHaveBeenCalled() + expect($backendUpdateApply.get().stage).toBe('error') + }) + it('surfaces an error when the backend never comes back after the restart', async () => { updateHermesSpy.mockResolvedValue({ ok: true, name: 'update', pid: 1 }) getActionStatusSpy.mockRejectedValue(new Error('ECONNREFUSED')) checkHermesUpdateSpy.mockRejectedValue(new Error('ECONNREFUSED')) const promise = applyBackendUpdate() - await vi.advanceTimersByTimeAsync(70000) + await vi.advanceTimersByTimeAsync(250000) const result = await promise expect(result.ok).toBe(false) expect($backendUpdateApply.get().stage).toBe('error') - }) + }, 10000) }) describe('startUpdatePoller', () => { diff --git a/apps/desktop/src/store/updates.ts b/apps/desktop/src/store/updates.ts index 0b46920e30..1c99356030 100644 --- a/apps/desktop/src/store/updates.ts +++ b/apps/desktop/src/store/updates.ts @@ -487,24 +487,9 @@ export async function applyUpdates(opts: DesktopUpdateApplyOptions = {}): Promis } } -const BACKEND_RETURN_POLL_MS = 1500 -const BACKEND_RETURN_MAX_ATTEMPTS = 40 - -async function waitForBackendReturn(): Promise { - for (let attempt = 0; attempt < BACKEND_RETURN_MAX_ATTEMPTS; attempt += 1) { - await new Promise(resolve => globalThis.setTimeout(resolve, BACKEND_RETURN_POLL_MS)) - - try { - await checkHermesUpdate() - - return true - } catch { - continue - } - } - - return false -} +const BACKEND_ACTION_POLL_MS = 1500 +const BACKEND_ACTION_MAX_MS = 6 * 60 * 1000 +const BACKEND_RETURN_MAX_MS = 4 * 60 * 1000 function finishBackendApply(returned: boolean): DesktopUpdateApplyResult { if (returned) { @@ -547,7 +532,32 @@ function ingestBackendActionStatus(status: Awaited { +function completedAfterRestart( + status: Awaited>, + actionId: string | undefined +): boolean { + return !!actionId && status.lines.some(line => line === `=== hermes-update completed ${actionId} ===`) +} + +function legacyBackendReachedTarget( + status: BackendUpdateCheckResponse, + targetSha: string | undefined, + previousVersion: string | undefined +): boolean { + if (status.behind === 0) { + return true + } + + if (previousVersion && status.current_version !== previousVersion) { + return true + } + + return !!targetSha && !!status.commits?.length && !status.commits.some(commit => commit.sha === targetSha) +} + +let backendUpdateInFlight: Promise | null = null + +async function runBackendUpdate(): Promise { dismissNotification(UPDATE_TOAST_ID) $backendUpdateApply.set({ ...IDLE, @@ -557,6 +567,11 @@ export async function applyBackendUpdate(): Promise { }) try { + const previousStatus = $backendUpdateStatus.get() + const requestedTargetSha = previousStatus?.commits?.at(0)?.sha + const previousVersion = previousStatus?.targetSha?.startsWith('backend:') + ? previousStatus.targetSha.slice('backend:'.length) + : undefined const started = await updateHermes() if (!started.ok) { @@ -575,43 +590,69 @@ export async function applyBackendUpdate(): Promise { }) let last: Awaited> | null = null + // Backups, dependency repair, and builds can legitimately take several + // minutes. Keep the generous cap only as a guard against a stuck action. + const actionDeadline = Date.now() + BACKEND_ACTION_MAX_MS + let deadline = actionDeadline + let reconnecting = false - for (let attempt = 0; attempt < 30; attempt += 1) { - await new Promise(resolve => globalThis.setTimeout(resolve, 1500)) + while (Date.now() < deadline) { + await new Promise(resolve => globalThis.setTimeout(resolve, BACKEND_ACTION_POLL_MS)) try { - last = await getActionStatus(started.name, 200) + last = await getActionStatus(started.name, 2000) ingestBackendActionStatus(last) } catch { - // The dashboard restarts mid-update, dropping this connection — expected, not a failure. - $backendUpdateApply.set({ - ...$backendUpdateApply.get(), - applying: true, - stage: 'restart', - message: translateNow('updates.applyStatus.restarting') - }) + if (!reconnecting) { + reconnecting = true + deadline = Date.now() + BACKEND_RETURN_MAX_MS + $backendUpdateApply.set({ + ...$backendUpdateApply.get(), + applying: true, + stage: 'restart', + message: translateNow('updates.applyStatus.restarting') + }) + } - return finishBackendApply(await waitForBackendReturn()) + continue } - if (last && !last.running) { + if (last.running) { + if (reconnecting) { + reconnecting = false + deadline = actionDeadline + $backendUpdateApply.set({ + ...$backendUpdateApply.get(), + applying: true, + stage: 'pull', + message: translateNow('updates.applyStatus.pulling') + }) + } + + continue + } + + if (last.exit_code === 0 || (last.exit_code === null && completedAfterRestart(last, started.action_id))) { + return finishBackendApply(true) + } + + if (!started.action_id && last.exit_code === null) { + try { + const status = await checkHermesUpdate(true) + + if (legacyBackendReachedTarget(status, requestedTargetSha, previousVersion)) { + return finishBackendApply(true) + } + } catch { + continue + } + } + + if (last.exit_code !== null) { break } } - const ok = !!last && (last.exit_code ?? 1) === 0 - - if (ok) { - $backendUpdateApply.set({ - ...$backendUpdateApply.get(), - applying: true, - stage: 'restart', - message: translateNow('updates.applyStatus.restarting') - }) - - return finishBackendApply(await waitForBackendReturn()) - } - $backendUpdateApply.set({ ...$backendUpdateApply.get(), applying: false, @@ -635,6 +676,18 @@ export async function applyBackendUpdate(): Promise { } } +export function applyBackendUpdate(): Promise { + if (backendUpdateInFlight) { + return backendUpdateInFlight + } + + backendUpdateInFlight = runBackendUpdate().finally(() => { + backendUpdateInFlight = null + }) + + return backendUpdateInFlight +} + function ingestProgress(payload: DesktopUpdateProgress): void { const current = $updateApply.get() const log = [...current.log, { stage: payload.stage, message: payload.message, at: payload.at }].slice(-50) diff --git a/apps/desktop/src/types/hermes.ts b/apps/desktop/src/types/hermes.ts index e468d21d12..374bc4db9a 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/apps/desktop/src/types/hermes.ts @@ -1144,6 +1144,8 @@ export interface ActionResponse { name: string ok: boolean pid: number + action_id?: string + already_running?: boolean } export interface ActionStatusResponse {