fix(desktop): break renderer-led reinstall loop on transient backend stalls
Issue #74874. The renderer's 'Repair' button treated every transient backend GIL stall (event loop stalled ... ws ready frame send failed) as a fatal backend fault, asking the bootstrap to force-reinstall + restart, which then stalled again for the same reason — looping the user through 30+ minutes of reinstall cycles. Distinguish 'venv is genuinely broken' from 'backend is just transiently stalled' before honouring a repair request. Probe the live backend process (exitCode === null && signalCode === null) and an in-flight repair-attempt counter: attempt <= 3 AND primary alive → soft restart (skip installer) attempt <= 3 AND primary dead → soft restart (verify before reinstall) attempt > 3 → hard reinstall (escalate) Counter resets on a clean backend.ready so a later, unrelated failure episode starts at attempt 1. The guard is a pure helper (decideBootstrap Repair in electron/bootstrap-repair-guard.ts) so the decision logic is unit-tested in isolation; main.ts only wires the existing flag and counters to it. Refs #74874
This commit is contained in:
parent
0ee9723b52
commit
16e66e721f
|
|
@ -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)
|
||||
})
|
||||
|
|
@ -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`
|
||||
}
|
||||
}
|
||||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue