diff --git a/apps/desktop/src/store/updates.test.ts b/apps/desktop/src/store/updates.test.ts index a0cb66cb63..5b8811c54e 100644 --- a/apps/desktop/src/store/updates.test.ts +++ b/apps/desktop/src/store/updates.test.ts @@ -1496,7 +1496,12 @@ describe('applyBackendUpdate recovery', () => { name: 'hermes-update', pid: null, running: false, - receipt: { outcome: 'success', followups: [{ step: 'dependencies', reason: 'sync failed' }], user_action: null } + receipt: { + action_id: 'e'.repeat(32), + outcome: 'success', + followups: [{ step: 'dependencies', reason: 'sync failed' }], + user_action: null + } }) notifySpy.mockClear() @@ -1511,6 +1516,43 @@ describe('applyBackendUpdate recovery', () => { ) }) + it("names a partial update's owed user action verbatim, only from this action's own receipt", async () => { + const instruction = 'Your local changes are parked in stash@{0}; run `git stash pop` in ~/.hermes/hermes-agent.' + + const status = (actionId: string) => ({ + exit_code: 1, + lines: ['update parked local changes'], + name: 'hermes-update', + pid: null, + running: false, + receipt: { + action_id: actionId, + outcome: 'partial', + followups: [], + user_action: { step: 'local_changes', reason: instruction } + } + }) + + updateHermesSpy.mockResolvedValue({ action_id: 'c'.repeat(32), ok: true, name: 'hermes-update', pid: 1 }) + getActionStatusSpy.mockResolvedValue(status('c'.repeat(32))) + const own = applyBackendUpdate() + await vi.advanceTimersByTimeAsync(1500) + const result = await own + + // exit 1 stays a failed apply, but the committed run's debt is named, never as "re-run hermes update". + expect(result).toMatchObject({ ok: false, error: 'apply-failed' }) + expect(result.message).toContain(`local_changes: ${instruction}`) + expect(result.message).not.toMatch(/Re-run `hermes update`/) + expect($backendUpdateApply.get()).toMatchObject({ stage: 'error', message: result.message }) + + // The status route attaches the latest receipt when this action has none: another run's debt is not ours. + getActionStatusSpy.mockResolvedValue(status('d'.repeat(32))) + const foreign = applyBackendUpdate() + await vi.advanceTimersByTimeAsync(1500) + + expect((await foreign).message).not.toContain('local_changes') + }) + it("never lets another action's receipt certify this update", async () => { const receipt = (actionId: string) => ({ exit_code: null, diff --git a/apps/desktop/src/store/updates.ts b/apps/desktop/src/store/updates.ts index b78ecf5ffa..4b39552ae5 100644 --- a/apps/desktop/src/store/updates.ts +++ b/apps/desktop/src/store/updates.ts @@ -3,6 +3,7 @@ * surfaces it as an ambient pill, and orchestrates the apply flow. */ +import { updateDebt } from '@hermes/shared' import { atom } from 'nanostores' import { connectionScoped, profileScoped } from '@/api/client' @@ -747,15 +748,43 @@ const BACKEND_ACTION_POLL_MS = 1500 const BACKEND_ACTION_MAX_MS = 6 * 60 * 1000 const BACKEND_RETURN_MAX_MS = 4 * 60 * 1000 -// C3: a committed update succeeds even when post-commit steps are still owed. -// Name every owed step (and its reason) instead of reporting plain success. -function owedSteps(receipt: UpdateReceiptSummary | undefined): string { - return [...(receipt?.followups ?? []), ...(receipt?.user_action ? [receipt.user_action] : [])] - .map(step => (step.reason ? `${step.step} (${step.reason})` : step.step)) - .join('; ') +// C3: a committed update succeeds even when post-commit steps are still owed, +// and a partial one (committed, but the user must act) still owes them. Name +// every owed step of THIS action's committed run whatever the exit: follow-ups +// with the rerun remedy, a user action with the producer's own instruction, +// verbatim (a rerun does not restore a parked stash). +function owedMessage(receipt: UpdateReceiptSummary | undefined, actionId: string | undefined): string | null { + const debt = updateDebt(receipt, actionId) + + if (!debt) { + return null + } + + return [debt.followups && translateNow('updates.applyStatus.owed', debt.followups), debt.userAction] + .filter(Boolean) + .join(' ') } -function finishBackendApply(returned: boolean, receipt?: UpdateReceiptSummary): DesktopUpdateApplyResult { +// A failed apply whose receipt still proves a committed partial run (exit 1) +// names what that run owes instead of only "failed" / "did not come back". +function failBackendApply( + fallback: { shown: string; result: string }, + status: { receipt?: UpdateReceiptSummary } | null, + actionId: string | undefined +): DesktopUpdateApplyResult { + const owed = owedMessage(status?.receipt, actionId) + const message = owed ? `${translateNow('updates.applyStatus.failed')} ${owed}` : fallback.shown + + $backendUpdateApply.set({ ...$backendUpdateApply.get(), applying: false, stage: 'error', error: 'apply-failed', message }) + + return { ok: false, error: 'apply-failed', message: owed ? message : fallback.result } +} + +function finishBackendApply( + returned: boolean, + receipt?: UpdateReceiptSummary, + actionId?: string +): DesktopUpdateApplyResult { if (returned) { $backendUpdateApply.set(IDLE) setUpdateOverlayOpen(false) @@ -773,27 +802,22 @@ function finishBackendApply(returned: boolean, receipt?: UpdateReceiptSummary): // affordance in remote mode targets the backend, so nothing ever told // them the app itself was stale). Nudge with a one-click client update. void maybeNudgeClientAfterBackendUpdate() - const owed = owedSteps(receipt) + const owed = owedMessage(receipt, actionId) if (owed) { - const message = translateNow('updates.applyStatus.owed', owed) - notify({ durationMs: 0, kind: 'warning', message }) + notify({ durationMs: 0, kind: 'warning', message: owed }) - return { ok: true, message } + return { ok: true, message: owed } } return { ok: true, message: 'Backend update applied.' } } - $backendUpdateApply.set({ - ...$backendUpdateApply.get(), - applying: false, - stage: 'error', - error: 'apply-failed', - message: translateNow('updates.applyStatus.noReturn') - }) - - return { ok: false, error: 'apply-failed', message: 'Backend did not come back online.' } + return failBackendApply( + { shown: translateNow('updates.applyStatus.noReturn'), result: 'Backend did not come back online.' }, + { receipt }, + actionId + ) } function ingestBackendActionStatus(status: Awaited>): void { @@ -955,7 +979,7 @@ async function runBackendUpdate(): Promise { } if (last.exit_code === 0 || (last.exit_code === null && completedAfterRestart(last, started.action_id))) { - return finishBackendApply(true, last.receipt) + return finishBackendApply(true, last.receipt, started.action_id) } // #91277 bullet 3: the backend now attaches the durable update @@ -963,7 +987,7 @@ async function runBackendUpdate(): Promise { // this update off is authoritative — read its outcome instead of // inferring from log markers or timing out across the restart gap. if (last.exit_code === null && receiptProvesOutcome(last, applyStartedAtMs, started.action_id)) { - return finishBackendApply(last.receipt!.outcome === 'success', last.receipt) + return finishBackendApply(last.receipt!.outcome === 'success', last.receipt, started.action_id) } if (!started.action_id && last.exit_code === null) { @@ -983,15 +1007,11 @@ async function runBackendUpdate(): Promise { } } - $backendUpdateApply.set({ - ...$backendUpdateApply.get(), - applying: false, - stage: 'error', - error: 'apply-failed', - message: translateNow('updates.applyStatus.failed') - }) - - return { ok: false, error: 'apply-failed', message: 'Backend update failed.' } + return failBackendApply( + { shown: translateNow('updates.applyStatus.failed'), result: 'Backend update failed.' }, + last, + started.action_id + ) } catch (error) { const message = error instanceof Error ? error.message : String(error) $backendUpdateApply.set({ diff --git a/apps/shared/src/index.ts b/apps/shared/src/index.ts index 00144fdb14..b67989b360 100644 --- a/apps/shared/src/index.ts +++ b/apps/shared/src/index.ts @@ -174,6 +174,7 @@ export { WINDOWS_GLASS_MIN_BUILD, type WindowsBackgroundMaterial } from './translucency' +export { type UpdateDebt, updateDebt, type UpdateDebtReceipt, type UpdateDebtStep } from './update-debt' export { buildHermesWebSocketUrl, type GatewayAuthMode, diff --git a/apps/shared/src/update-debt.ts b/apps/shared/src/update-debt.ts new file mode 100644 index 0000000000..3e03b162bd --- /dev/null +++ b/apps/shared/src/update-debt.ts @@ -0,0 +1,49 @@ +// Owed post-commit work of a dashboard-started `hermes update`, read from the +// receipt summary the action status route attaches. Shared by the web +// dashboard toast and the Desktop backend-apply store so both surfaces name +// the same debt with the same rules as the managed SSH result. + +export interface UpdateDebtStep { + step: string + reason?: string | null +} + +export interface UpdateDebtReceipt { + outcome?: string | null + action_id?: string | null + followups?: UpdateDebtStep[] | null + user_action?: UpdateDebtStep | null +} + +export interface UpdateDebt { + /** Follow-ups a rerun of `hermes update` finishes, as `step (reason); step`; '' when none. */ + followups: string + /** What only the user can do (e.g. re-apply a parked stash) as `step: `; + * '' when none. Never phrase this as "re-run hermes update": a rerun does not restore it. */ + userAction: string +} + +/** The debt of THIS action's committed run, or null when nothing is owed. + * + * Committed is `success` or `partial` (record_user_action turns a committed success into + * partial / exit 1), so the debt is read for every committed outcome, not only exit 0. The + * status route attaches the latest receipt when this action has none, so a receipt is only + * this action's when its `action_id` matches the id the start request returned. */ +export function updateDebt(receipt: UpdateDebtReceipt | null | undefined, actionId: string | undefined): UpdateDebt | null { + if (!receipt || !actionId || receipt.action_id !== actionId) { + return null + } + + if (receipt.outcome !== 'success' && receipt.outcome !== 'partial') { + return null + } + + const followups = (receipt.followups ?? []) + .map(step => (step.reason ? `${step.step} (${step.reason})` : step.step)) + .join('; ') + + const action = receipt.user_action + const userAction = action ? (action.reason ? `${action.step}: ${action.reason}` : action.step) : '' + + return followups || userAction ? { followups, userAction } : null +}