From bc73ec33be1dd1552a4bfa4dfc69bbb62e97c0db Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 5 Oct 2026 22:30:57 -0700 Subject: [PATCH] fix: managed SSH results name owed work on restore-failed and partial updates managedUpdateMessage returned a generic update/restore message before reading the receipt's debt whenever the combined outcome was not `updated`. Two reachable outcomes lost necessary recovery data from the displayed result and the Update-all row: a committed success whose profile restoration failed (owed follow-ups dropped) and a `partial` run whose record_user_action carries the exact parked-local-change instruction (dropped; only "the remote update failed" shown). Compose the debt with every verdict: owed steps are listed for any terminal receipt, `owed` is returned, follow-ups get the rerun advice by name, and a user action is shown verbatim (never as "re-run hermes update", which does not restore a parked stash). updateOk/restoreOk stay independent. --- .../electron/managed-ssh-update.test.ts | 46 +++++++++++++++++-- apps/desktop/electron/managed-ssh-update.ts | 35 ++++++++------ 2 files changed, 63 insertions(+), 18 deletions(-) diff --git a/apps/desktop/electron/managed-ssh-update.test.ts b/apps/desktop/electron/managed-ssh-update.test.ts index 8196caf91f..348e83e1c7 100644 --- a/apps/desktop/electron/managed-ssh-update.test.ts +++ b/apps/desktop/electron/managed-ssh-update.test.ts @@ -512,18 +512,22 @@ test.runIf(process.platform !== 'win32')( async () => { const home = await mkdtemp(path.join(os.tmpdir(), 'hermes-managed-owed-')) - const run = async (followups: unknown[]) => { + const run = async ( + followups: unknown[], + { exitCode = 0, outcome = 'success', restoreFails = false, userAction = null as unknown } = {} + ) => { const receipts = path.join(home, 'logs', 'update_receipts') await mkdir(receipts, { recursive: true }) - await writeFile(path.join(home, `.update_exit_code.${CORRELATION}`), '0') + await writeFile(path.join(home, `.update_exit_code.${CORRELATION}`), String(exitCode)) await writeFile( path.join(receipts, 'update_x.json'), JSON.stringify({ correlation_id: CORRELATION, - outcome: 'success', + outcome, started_at: '2026-08-23T00:00:00Z', finished_at: '2026-08-23T00:01:00Z', - followups + followups, + user_action: userAction }) ) @@ -543,7 +547,11 @@ test.runIf(process.platform !== 'win32')( updateRemote: async () => ({ exitCode: observed.exitCode!, receipt: observed.receipt! }), awaitRestoreClearance: async () => {}, closeTransports: async () => {}, - restoreScope: async () => {}, + restoreScope: async () => { + if (restoreFails) { + throw new Error('restore refused') + } + }, releaseGate: () => {} }) } @@ -564,6 +572,34 @@ test.runIf(process.platform !== 'win32')( assert.equal(clean.ok, true) assert.equal(clean.owed, undefined) assert.equal(clean.message, 'Remote Hermes updated and every managed SSH profile is ready.') + + // The debt survives a failed restoration: update and restore verdicts stay independent. + const unrestored = await run([{ step: 'dependencies', reason: 'sync failed' }], { restoreFails: true }) + + assert.equal(unrestored.updateOk, true) + assert.equal(unrestored.restoreOk, false) + assert.deepEqual(unrestored.owed, [{ step: 'dependencies', reason: 'sync failed' }]) + assert.match( + managedSshUpdateAllRow({ id: 'home' }, unrestored).detail || '', + /still owed: dependencies \(sync failed\)/ + ) + + // record_user_action turns a success into partial/exit 1: its exact manual instruction is shown, + // and the rerun advice is never offered for a parked stash. + const instruction = 'Run `git stash apply stash@{0}` in /opt/hermes to restore your local changes.' + const partial = await run([], { + exitCode: 1, + outcome: 'partial', + userAction: { step: 'local_changes', reason: instruction } + }) + + assert.equal(partial.updateOk, false) + assert.equal(partial.restoreOk, true) + assert.deepEqual(partial.owed, [{ step: 'local_changes', reason: instruction }]) + const detail = managedSshUpdateAllRow({ id: 'home' }, partial).detail || '' + assert.ok(detail.includes(instruction), detail) + assert.match(detail, /local_changes/) + assert.doesNotMatch(detail, /Re-run `hermes update`/) } finally { await rm(home, { force: true, recursive: true }) } diff --git a/apps/desktop/electron/managed-ssh-update.ts b/apps/desktop/electron/managed-ssh-update.ts index 3e9b1bd07a..808ee04525 100644 --- a/apps/desktop/electron/managed-ssh-update.ts +++ b/apps/desktop/electron/managed-ssh-update.ts @@ -506,33 +506,42 @@ function withReceiptDebt(receipt: any): ManagedUpdateReceiptSummary { } // C3: a committed update is a success even when post-commit steps are owed; -// name each one and its remedy instead of claiming everything is ready. +// name each one and its remedy instead of claiming everything is ready. The +// debt rides on every terminal receipt, so it is composed with the update and +// restoration verdicts, never only on success: a committed run whose profile +// restore failed, and a partial run that parked local changes, still owe it. function managedUpdateMessage( outcome: ManagedUpdateOutcome, restoreOk: boolean, receipt: ManagedUpdateReceiptSummary | undefined ): { owed?: ManagedUpdateOwedStep[]; message: string } { - if (outcome !== 'updated') { - return { - message: restoreOk - ? 'The remote update failed, but every managed SSH profile was restored.' - : 'The remote update transaction could not restore every managed SSH profile.' - } - } - const followups = receipt?.followups ?? [] const userAction = receipt?.userAction const owed = [...followups, ...(userAction ? [userAction] : [])] + const status = + outcome === 'updated' + ? 'Remote Hermes updated' + : restoreOk + ? 'The remote update failed, but every managed SSH profile was restored.' + : 'The remote update transaction could not restore every managed SSH profile.' + if (!owed.length) { - return { message: 'Remote Hermes updated and every managed SSH profile is ready.' } + return { message: outcome === 'updated' ? `${status} and every managed SSH profile is ready.` : status } } const message = - 'Remote Hermes updated, but these steps are still owed: ' + - owed.map(step => (step.reason ? `${step.step} (${step.reason})` : step.step)).join('; ') + + (outcome === 'updated' ? `${status}, but these steps are still owed: ` : `${status} These steps are still owed: `) + + [ + ...followups.map(step => (step.reason ? `${step.step} (${step.reason})` : step.step)), + ...(userAction ? [userAction.step] : []) + ].join('; ') + '.' + - (followups.length ? ' Re-run `hermes update` on the remote to finish them.' : '') + + // Only follow-ups are finished by a rerun; a user action (e.g. a parked + // stash) is the producer's exact manual instruction, shown verbatim. + (followups.length + ? ` Re-run \`hermes update\` on the remote to finish ${followups.map(step => step.step).join(', ')}.` + : '') + (userAction?.reason ? ` ${userAction.reason}` : '') return { owed, message }