diff --git a/apps/desktop/electron/bootstrap-repair-guard.test.ts b/apps/desktop/electron/bootstrap-repair-guard.test.ts new file mode 100644 index 0000000000000..e1a7a1ecf3d93 --- /dev/null +++ b/apps/desktop/electron/bootstrap-repair-guard.test.ts @@ -0,0 +1,109 @@ +import assert from 'node:assert/strict' + +import { test } from 'vitest' + +import { decideBootstrapRepair } from './bootstrap-repair-guard' + +test('first soft attempt with alive backend returns soft restart', () => { + const decision = decideBootstrapRepair({ + attempt: 1, + primaryBackendAlive: true + }) + + assert.equal(decision.hardReinstall, false) + assert.equal(decision.attempt, 1) + assert.match(decision.reason, /still alive/) + assert.match(decision.reason, /1\/3/) +}) + +test('first attempt with dead backend still returns soft restart', () => { + const decision = decideBootstrapRepair({ + attempt: 1, + primaryBackendAlive: false + }) + + assert.equal(decision.hardReinstall, false) + assert.match(decision.reason, /has exited/) +}) + +test('soft restart budget exhausts at maxSoftAttempts+1 and escalates', () => { + const decision = decideBootstrapRepair({ + attempt: 4, + maxSoftAttempts: 3, + primaryBackendAlive: true + }) + + assert.equal(decision.hardReinstall, true) + assert.equal(decision.attempt, 4) + assert.match(decision.reason, /exceeds soft-restart budget/) +}) + +test('attempt exactly at maxSoftAttempts is still soft', () => { + const decision = decideBootstrapRepair({ + attempt: 3, + maxSoftAttempts: 3, + primaryBackendAlive: true + }) + + assert.equal(decision.hardReinstall, false) + assert.equal(decision.attempt, 3) +}) + +test('custom maxSoftAttempts is honored', () => { + const soft = decideBootstrapRepair({ + attempt: 5, + maxSoftAttempts: 10, + primaryBackendAlive: true + }) + + assert.equal(soft.hardReinstall, false) + + const hard = decideBootstrapRepair({ + attempt: 11, + maxSoftAttempts: 10, + primaryBackendAlive: false + }) + + assert.equal(hard.hardReinstall, true) +}) + +test('default maxSoftAttempts is 3', () => { + // Probe the default indirectly: attempt 4 with no override must escalate. + const decision = decideBootstrapRepair({ + attempt: 4, + primaryBackendAlive: true + }) + + assert.equal(decision.hardReinstall, true) +}) + +test('fractional or zero attempts are clamped to 1', () => { + const zeroDecision = decideBootstrapRepair({ + attempt: 0, + primaryBackendAlive: true + }) + + assert.equal(zeroDecision.attempt, 1) + assert.equal(zeroDecision.hardReinstall, false) + + const fractionalDecision = decideBootstrapRepair({ + attempt: 2.7, + primaryBackendAlive: true + }) + + assert.equal(fractionalDecision.attempt, 2) + assert.equal(fractionalDecision.hardReinstall, false) +}) + +test('alive=false on a high attempt number still escalates (defense in depth)', () => { + // A dead backend should normally be handled by the renderer before it + // reaches the repair path, but if it does reach us with a high attempt + // count we still escalate — never silently keep soft-restarting. + const decision = decideBootstrapRepair({ + attempt: 5, + maxSoftAttempts: 3, + primaryBackendAlive: false + }) + + assert.equal(decision.hardReinstall, true) +}) diff --git a/apps/desktop/electron/bootstrap-repair-guard.ts b/apps/desktop/electron/bootstrap-repair-guard.ts new file mode 100644 index 0000000000000..96f383770ed8e --- /dev/null +++ b/apps/desktop/electron/bootstrap-repair-guard.ts @@ -0,0 +1,122 @@ +/** + * Repair-loop guard for the desktop bootstrap. + * + * Why this exists + * ─────────────── + * Hermes desktop can request a "repair" of its bundled backend when the + * renderer observes a transient backend failure (see issue #74874). The + * classic failure fingerprint: + * + * 1. Backend Python process hits a transient GIL stall (e.g. heavy + * import, MCP discovery, a long-running agent turn). + * 2. The renderer's WebSocket can't deliver the `gateway.ready` frame + * in time and treats the socket as dead. + * 3. Renderer calls `hermes:bootstrap:repair`. + * 4. Bootstrap unconditionally force-reinstalls the venv, restarting + * the backend — which stalls again for the same reason. + * 5. Renderer reports dead backend → another repair → infinite loop. + * + * The desktop should distinguish: + * - "the venv/install is genuinely broken" → hard reinstall is correct + * - "the runtime is healthy but temporarily stalled" → restart only, + * NOT a destructive reinstall that drops the venv + * + * What this module does + * ───────────────────── + * A pure decision helper. Given the current repair attempt count and a + * hint about whether the live backend process still looks alive, return + * whether the next repair should: + * - `hardReinstall: true` → run the installer, recreate the venv + * - `hardReinstall: false` → restart the existing backend, keep the venv + * + * Cap on soft restarts is bounded so an actually-corrupted install still + * eventually escalates to a hard reinstall after repeated stalls — the + * guard prevents the *unbounded* reinstall loop, not all reinstalls. + * + * The module is intentionally pure (no I/O, no logging, no global state) + * so it is unit-testable in isolation. Wiring into `main.ts` lives there. + */ + +export type RepairDecision = + | { + /** Run the installer (recreate venv). Caller bypasses the active runtime. */ + hardReinstall: true + /** Human-readable rationale for the desktop log. */ + reason: string + /** 1-indexed repair attempt number for diagnostics. */ + attempt: number + } + | { + /** Skip the installer; restart the existing backend only. */ + hardReinstall: false + reason: string + attempt: number + } + +export type RepairDecisionInput = { + /** + * 1-indexed count of how many repair attempts have happened in this + * failure episode. The first repair is `attempt === 1`; a successful + * boot resets the counter (see `main.ts`'s bootstrap completion path). + */ + attempt: number + /** + * Soft-restart budget before escalation to a hard reinstall. Defaults + * to 3: three "just restart" attempts, then a real reinstall. Bounded + * so a corrupt install still gets fixed; high enough that a GIL + * stall no longer loops the user into a 30-minute reinstall cycle. + */ + maxSoftAttempts?: number + /** + * Whether the live backend process (the one we are about to tear down + * to honour the repair request) still looks alive. A process whose + * `exitCode !== null` or `signalCode !== null` has actually exited; + * a process with both null is either still running or stalled — and a + * stall is exactly the case the soft-restart path is for. + */ + primaryBackendAlive: boolean +} + +/** + * Decide the next repair action. + * + * Decision matrix: + * attempt ≤ maxSoftAttempts AND alive → soft restart (don't reinstall) + * attempt ≤ maxSoftAttempts AND dead → soft restart (process exited, + * but we don't yet trust that + * the install is corrupt; restart + * once to confirm) + * attempt > maxSoftAttempts → hard reinstall (give up on the + * current install) + * + * "Alive" being true does NOT force a soft restart on every call: the + * attempt counter still increments, so an actually-broken install that + * keeps respawning a child but never announces READY still escalates + * after `maxSoftAttempts` cycles. + */ +export function decideBootstrapRepair(input: RepairDecisionInput): RepairDecision { + const maxSoftAttempts = input.maxSoftAttempts ?? 3 + const attempt = Math.max(1, Math.floor(input.attempt)) + const alive = Boolean(input.primaryBackendAlive) + + if (attempt > maxSoftAttempts) { + return { + hardReinstall: true, + attempt, + reason: + `repair attempt ${attempt} exceeds soft-restart budget ` + + `(${maxSoftAttempts}); escalating to hard reinstall` + } + } + + return { + hardReinstall: false, + attempt, + reason: alive + ? `repair attempt ${attempt}/${maxSoftAttempts}: primary backend process ` + + `still alive (likely transient stall, see #74874); restarting only, ` + + `skipping installer` + : `repair attempt ${attempt}/${maxSoftAttempts}: primary backend process ` + + `has exited; restarting before escalating to reinstall` + } +} diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 87ebe3a8c6f38..a55eb3a6a110b 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -47,6 +47,7 @@ import { import { waitForDashboardPortAnnouncement } from './backend-ready' import { shouldLatchBackendStartFailure, shouldLatchRemoteReauthFailure } from './backend-start-failure' import { detectRemoteDisplay, isWindowsBinaryPathInWsl, isWslEnvironment } from './bootstrap-platform' +import { decideBootstrapRepair } from './bootstrap-repair-guard' import { runBootstrap } from './bootstrap-runner' import { applyConnectionChange, resolveTerminalConnection } from './connection-apply' import { @@ -1099,6 +1100,14 @@ let bootstrapAbortController = null // repair can force the installer without destroying provenance about how the // install was created. Cleared once the reinstall is under way. let bootstrapRepairRequested = false +// Counter for in-flight repair attempts. Reset on a clean boot completion +// (see runBootstrap -> ensureRuntime resolve path). Each successive repair +// in the same failure episode increments this; once it crosses +// MAX_BOOTSTRAP_REPAIR_SOFT_ATTEMPTS the guard escalates from "soft restart" +// to "hard reinstall" so a transient backend stall (issue #74874) stops +// looping the user through a destructive venv reinstall. +let bootstrapRepairAttempt = 0 +const MAX_BOOTSTRAP_REPAIR_SOFT_ATTEMPTS = 3 let connectionConfigCache = null let connectionConfigCacheMtime = null const hermesLog = [] @@ -4037,6 +4046,7 @@ async function ensureRuntime(backend) { // The repair request has been honoured by reaching the installer; clear it // so a later boot isn't forced through bootstrap again. bootstrapRepairRequested = false + bootstrapRepairAttempt = 0 const bootstrapResult = await runBootstrap({ installStamp: backend.installStamp, @@ -8547,6 +8557,13 @@ async function startHermes() { error: null }) + // A successful boot (including a soft restart that the repair-guard + // chose over a hard reinstall, see #74874) means any in-flight repair + // attempt counter has been honoured — reset it so the next genuine + // failure starts fresh from attempt 1 instead of inheriting the + // accumulated count of the resolved episode. + bootstrapRepairAttempt = 0 + return { baseUrl, mode: 'local', @@ -9628,9 +9645,41 @@ ipcMain.handle('hermes:bootstrap:repair', async () => { // transient backend errors on a perfectly healthy install, and deleting the // marker in that case stranded the app in first-run setup with no way back // (#72166). The explicit flag carries the intent instead. - rememberLog('[bootstrap] repair requested by renderer; forcing reinstall + clearing latched failure') + bootstrapRepairAttempt += 1 - bootstrapRepairRequested = true + // Probe the live backend process so the guard can distinguish "venv is + // genuinely broken" (force reinstall) from "backend is just transiently + // stalled under GIL pressure" (#74874 — `event loop stalled` followed by + // `ws ready frame send failed`, then renderer keeps reporting dead). + const primaryProc = backendConnectionState.getProcess() + + const primaryBackendAlive = Boolean( + primaryProc && + (primaryProc as { exitCode?: number | null }).exitCode === null && + (primaryProc as { signalCode?: string | null }).signalCode === null + ) + + const repairDecision = decideBootstrapRepair({ + attempt: bootstrapRepairAttempt, + maxSoftAttempts: MAX_BOOTSTRAP_REPAIR_SOFT_ATTEMPTS, + primaryBackendAlive + }) + + rememberLog( + `[bootstrap] repair requested by renderer; forcing reinstall + clearing latched failure ` + + `(attempt=${repairDecision.attempt}/${MAX_BOOTSTRAP_REPAIR_SOFT_ATTEMPTS}, ` + + `primaryBackendAlive=${primaryBackendAlive}, ` + + `hardReinstall=${repairDecision.hardReinstall}): ${repairDecision.reason}` + ) + + // The guard may decide the install is healthy enough that a restart + // (without touching the venv) is the right answer. Translate that into + // the existing flag: if the guard said "soft restart", we skip the + // "bypass active runtime" path inside startHermes() and fall through + // to the normal restart branch, which just kills the current child + // and respawns it against the same venv. See #74874 — this is what + // breaks the infinite reinstall loop the user hit. + bootstrapRepairRequested = repairDecision.hardReinstall bootstrapFailure = null backendStartFailure = null remoteReauthFailure = null