From 62eefff697b33f0cb0c1332d2c9b64fe89164699 Mon Sep 17 00:00:00 2001 From: Aleks Clark Date: Thu, 13 Aug 2026 10:52:28 -0500 Subject: [PATCH] perf(desktop): bound long-running app resource use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Persist backend ownership for reliable cleanup, park inactive panes, and evict unreferenced transcripts so Desktop stays responsive over long sessions. 💘 Generated with Crush Assisted-by: Crush:gpt-5.6 --- .../electron/backend-ownership.test.ts | 223 ++++++++++ apps/desktop/electron/backend-ownership.ts | 227 +++++++++++ apps/desktop/electron/main.ts | 382 ++++++++++++++++-- apps/desktop/src/app/contrib/controller.tsx | 8 +- .../hooks/use-session-state-cache.test.tsx | 27 ++ .../session/hooks/use-session-state-cache.ts | 149 ++++--- .../app/session/session-state-cache.test.ts | 133 ++++++ .../src/app/session/session-state-cache.ts | 135 +++++++ .../assistant-ui/thread/list.test.ts | 12 +- .../components/assistant-ui/thread/list.tsx | 20 +- .../pane-shell/pane-lifecycle.test.ts | 72 ++++ .../components/pane-shell/pane-lifecycle.ts | 71 ++++ .../components/pane-shell/pane-visibility.ts | 8 + .../pane-shell/tree/renderer/track-model.ts | 4 + .../pane-shell/tree/renderer/tree-group.tsx | 58 ++- .../src/store/session-states-eviction.test.ts | 14 +- apps/desktop/src/store/session-states.test.ts | 23 ++ apps/desktop/src/store/session-states.ts | 43 +- hermes_cli/web_server.py | 151 ++++++- 19 files changed, 1593 insertions(+), 167 deletions(-) create mode 100644 apps/desktop/electron/backend-ownership.test.ts create mode 100644 apps/desktop/electron/backend-ownership.ts create mode 100644 apps/desktop/src/app/session/session-state-cache.test.ts create mode 100644 apps/desktop/src/app/session/session-state-cache.ts create mode 100644 apps/desktop/src/components/pane-shell/pane-lifecycle.test.ts create mode 100644 apps/desktop/src/components/pane-shell/pane-lifecycle.ts diff --git a/apps/desktop/electron/backend-ownership.test.ts b/apps/desktop/electron/backend-ownership.test.ts new file mode 100644 index 0000000000..b2afbae7b0 --- /dev/null +++ b/apps/desktop/electron/backend-ownership.test.ts @@ -0,0 +1,223 @@ +import assert from 'node:assert/strict' + +import { test, vi } from 'vitest' + +import { + backendCommandMatches, + type BackendIdentity, + createBackendOwnership, + createBackendShutdownCoordinator, + parseBackendOwnership +} from './backend-ownership' + +function memoryStore(initial = '') { + let contents = initial + + return { + read: () => contents, + value: () => contents, + write: (next: string) => { + contents = next + } + } +} + +function identity(overrides: Partial = {}): BackendIdentity { + return { + nonce: 'nonce-42', + pid: 42, + profile: 'default', + startMarker: 'os-start-123', + ...overrides + } +} + +function ownershipEntry(overrides: Partial = {}) { + return { command: 'hermes serve --port 0', ...identity(overrides) } +} + +function stored(entries: object[]): string { + return JSON.stringify({ backends: entries }) +} + +function deferred() { + let resolve!: () => void + + const promise = new Promise(done => { + resolve = done + }) + + return { promise, resolve } +} + +function createOwnership(store = memoryStore(), overrides: Partial[0]> = {}) { + return createBackendOwnership({ + matchesIdentity: async () => true, + stop: () => {}, + store, + ...overrides + }) +} + +test('claim persists the caller-supplied exact identity before resolving', async () => { + const store = memoryStore() + const ownership = createOwnership(store) + const claim = ownershipEntry() + + assert.deepEqual(await ownership.claim(claim), claim) + assert.deepEqual(parseBackendOwnership(store.value()), [claim]) +}) + +test('incomplete claims and persisted records are rejected', async () => { + const store = memoryStore( + stored([ + ownershipEntry(), + { ...ownershipEntry({ pid: 43 }), startMarker: '' }, + { ...ownershipEntry({ pid: 44 }), nonce: undefined }, + { ...ownershipEntry({ pid: 45 }), profile: undefined } + ]) + ) + + const ownership = createOwnership(store) + + await assert.rejects(ownership.claim({ ...ownershipEntry(), startMarker: '' }), /complete process identity/) + assert.deepEqual(parseBackendOwnership(store.value()), [ownershipEntry()]) +}) + +test('failed persistence awaits asynchronous cleanup of the exact identity', async () => { + const cleanup = deferred() + const stop = vi.fn(() => cleanup.promise) + const expected = new Error('disk full') + const claim = ownershipEntry({ pid: 43 }) + + const ownership = createOwnership(memoryStore(), { + stop, + store: { + read: () => null, + write: () => { + throw expected + } + } + }) + + let rejected = false + + const result = ownership.claim(claim).catch(error => { + rejected = true + throw error + }) + + await Promise.resolve() + assert.equal(rejected, false) + assert.deepEqual(stop.mock.calls, [[claim]]) + + cleanup.resolve() + await assert.rejects(result, expected) + assert.equal(rejected, true) +}) + +test('startup reap drops a confirmed PID reuse mismatch without stopping it', async () => { + const entry = ownershipEntry() + const store = memoryStore(stored([entry])) + const matchesIdentity = vi.fn(async () => false) + const stop = vi.fn() + const ownership = createOwnership(store, { matchesIdentity, stop }) + + assert.deepEqual(await ownership.reapOrphans(), []) + assert.deepEqual(matchesIdentity.mock.calls, [[entry]]) + assert.equal(stop.mock.calls.length, 0) + assert.deepEqual(parseBackendOwnership(store.value()), []) +}) + +test('startup reap preserves records when exact identity probing is uncertain or fails', async () => { + const uncertain = ownershipEntry({ pid: 50, nonce: 'uncertain' }) + const failed = ownershipEntry({ pid: 51, nonce: 'failed' }) + const store = memoryStore(stored([uncertain, failed])) + const stop = vi.fn() + + const ownership = createOwnership(store, { + matchesIdentity: async entry => { + if (entry.pid === failed.pid) { + throw new Error('process table unavailable') + } + + return undefined + }, + stop + }) + + assert.deepEqual(await ownership.reapOrphans(), []) + assert.equal(stop.mock.calls.length, 0) + assert.deepEqual(parseBackendOwnership(store.value()), [uncertain, failed]) +}) + +test('startup reap passes the full confirmed identity to stop', async () => { + const entry = ownershipEntry({ pid: 52 }) + const store = memoryStore(stored([entry])) + const stop = vi.fn() + const ownership = createOwnership(store, { stop }) + + assert.deepEqual(await ownership.reapOrphans(), [52]) + assert.deepEqual(stop.mock.calls, [[entry]]) + assert.deepEqual(parseBackendOwnership(store.value()), []) +}) + +test('startup reap preserves failed stops for the next launch', async () => { + const entry = ownershipEntry({ pid: 53 }) + const store = memoryStore(stored([entry])) + + const ownership = createOwnership(store, { + stop: () => { + throw new Error('permission denied') + } + }) + + assert.deepEqual(await ownership.reapOrphans(), []) + assert.deepEqual(parseBackendOwnership(store.value()), [entry]) +}) + +test('release removes only the exact identity rather than every record for its PID', () => { + const oldProcess = ownershipEntry({ nonce: 'old', startMarker: 'start-old' }) + const reusedPid = ownershipEntry({ nonce: 'new', startMarker: 'start-new' }) + const store = memoryStore(stored([oldProcess, reusedPid])) + const ownership = createOwnership(store) + + ownership.release(oldProcess) + + assert.deepEqual(parseBackendOwnership(store.value()), [reusedPid]) +}) + +test('backend identity check matches only serve and dashboard invocation shapes', () => { + assert.equal(backendCommandMatches('/venv/bin/hermes serve --port 0'), true) + assert.equal(backendCommandMatches('python -m hermes_cli.main dashboard --no-open'), true) + assert.equal(backendCommandMatches('/venv/bin/hermes --profile work serve --port 0'), true) + assert.equal(backendCommandMatches('"C:\\Hermes Runtime\\hermes.exe" dashboard --no-open'), true) + assert.equal(backendCommandMatches('hermes chat --query serve'), false) + assert.equal(backendCommandMatches('unrelated dashboard'), false) +}) + +test('shutdown coordinator returns one promise and awaits teardown exactly once', async () => { + const completion = deferred() + const teardown = vi.fn(() => completion.promise) + const coordinator = createBackendShutdownCoordinator(teardown) + + const first = coordinator.run() + const second = coordinator.run() + + assert.equal(first, second) + assert.equal(coordinator.hasStarted(), true) + await Promise.resolve() + assert.equal(teardown.mock.calls.length, 1) + + let finished = false + first.then(() => { + finished = true + }) + await Promise.resolve() + assert.equal(finished, false) + + completion.resolve() + await second + assert.equal(finished, true) + assert.equal(coordinator.run(), first) +}) diff --git a/apps/desktop/electron/backend-ownership.ts b/apps/desktop/electron/backend-ownership.ts new file mode 100644 index 0000000000..feb50fb8e6 --- /dev/null +++ b/apps/desktop/electron/backend-ownership.ts @@ -0,0 +1,227 @@ +export interface BackendIdentity { + nonce: string + pid: number + profile: string + startMarker: string +} + +export interface BackendOwnershipEntry extends BackendIdentity { + command?: string +} + +export interface BackendOwnershipStore { + read: () => string | null + write: (contents: string) => void +} + +export interface BackendOwnershipDeps { + matchesIdentity: (identity: BackendIdentity) => Promise + stop: (identity: BackendIdentity) => Promise | void + store: BackendOwnershipStore +} + +export interface BackendClaim extends BackendIdentity { + command?: string +} + +function isNonEmptyString(value: unknown): value is string { + return typeof value === 'string' && value.length > 0 +} + +function isCompleteIdentity(value: unknown): value is BackendIdentity { + if (!value || typeof value !== 'object') { + return false + } + + const candidate = value as Partial + + return ( + Number.isInteger(candidate.pid) && + Number(candidate.pid) > 0 && + isNonEmptyString(candidate.startMarker) && + isNonEmptyString(candidate.nonce) && + isNonEmptyString(candidate.profile) + ) +} + +function identitiesMatch(left: BackendIdentity, right: BackendIdentity): boolean { + return ( + left.pid === right.pid && + left.startMarker === right.startMarker && + left.nonce === right.nonce && + left.profile === right.profile + ) +} + +export function parseBackendOwnership(contents: unknown): BackendOwnershipEntry[] { + let parsed: unknown + + try { + parsed = JSON.parse(String(contents ?? '')) + } catch { + return [] + } + + const values = Array.isArray(parsed) + ? parsed + : parsed && typeof parsed === 'object' && Array.isArray((parsed as { backends?: unknown }).backends) + ? (parsed as { backends: unknown[] }).backends + : [] + + const entries: BackendOwnershipEntry[] = [] + + for (const value of values) { + if (!isCompleteIdentity(value)) { + continue + } + + const candidate = value as BackendOwnershipEntry + + const entry: BackendOwnershipEntry = { + nonce: candidate.nonce, + pid: candidate.pid, + profile: candidate.profile, + startMarker: candidate.startMarker + } + + if (typeof candidate.command === 'string') { + entry.command = candidate.command + } + + if (!entries.some(existing => identitiesMatch(existing, entry))) { + entries.push(entry) + } + } + + return entries +} + +export function serializeBackendOwnership(entries: BackendOwnershipEntry[]): string { + return `${JSON.stringify({ backends: entries }, null, 2)}\n` +} + +/** + * Persistent ownership for local backend roots. + * + * Claiming is asynchronous so a failed persistence transaction can await child + * cleanup before reporting failure to the caller. + */ +export function createBackendOwnership(deps: BackendOwnershipDeps) { + const read = () => parseBackendOwnership(deps.store.read()) + const write = (entries: BackendOwnershipEntry[]) => deps.store.write(serializeBackendOwnership(entries)) + + return { + async claim(claim: BackendClaim): Promise { + if (!isCompleteIdentity(claim)) { + throw new Error('Cannot own a backend without a complete process identity.') + } + + const entry: BackendOwnershipEntry = { + nonce: claim.nonce, + pid: claim.pid, + profile: claim.profile, + startMarker: claim.startMarker + } + + if (typeof claim.command === 'string') { + entry.command = claim.command + } + + try { + const entries = read().filter(candidate => candidate.pid !== entry.pid) + write([...entries, entry]) + } catch (error) { + try { + await deps.stop(entry) + } catch { + // Persistence remains the claim failure even if cleanup also fails. + } + + throw error + } + + return entry + }, + + release(identity: BackendIdentity): void { + if (!isCompleteIdentity(identity)) { + throw new Error('Cannot release a backend without a complete process identity.') + } + + const entries = read() + const next = entries.filter(entry => !identitiesMatch(entry, identity)) + + if (next.length !== entries.length) { + write(next) + } + }, + + async reapOrphans(): Promise { + const entries = read() + const survivors: BackendOwnershipEntry[] = [] + const reaped: number[] = [] + + for (const entry of entries) { + let matches: boolean | undefined + + try { + matches = await deps.matchesIdentity(entry) + } catch { + survivors.push(entry) + + continue + } + + if (matches === false) { + continue + } + + if (matches !== true) { + survivors.push(entry) + + continue + } + + try { + await deps.stop(entry) + reaped.push(entry.pid) + } catch { + // Preserve failed ownership so a later startup can retry it. + survivors.push(entry) + } + } + + write(survivors) + + return reaped + }, + + clear(): void { + write([]) + } + } +} + +export function backendCommandMatches(command: unknown): boolean { + return /(?:^|[\s/\\"])(?:hermes(?:\.exe)?|hermes_cli\.main|hermes_cli[/\\]main\.py)"?(?:\s+(?:--profile|-p)\s+\S+)?\s+(?:serve|dashboard)(?:\s|$)/i.test( + String(command ?? '') + ) +} + +/** Coordinates all quit paths so asynchronous backend teardown runs once. */ +export function createBackendShutdownCoordinator(teardown: () => Promise | void) { + let completion: Promise | undefined + + return { + run(): Promise { + if (!completion) { + completion = Promise.resolve().then(teardown) + } + + return completion + }, + hasStarted(): boolean { + return completion !== undefined + } + } +} diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index b6addda017..6fd8a0dba8 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -37,6 +37,11 @@ import { dashboardFallbackArgs, sourceDeclaresServe } from './backend-command' import { createBackendConnectionState } from './backend-connection-state' import { buildDesktopBackendEnv, hermesManagedNodePathEntries, normalizeHermesHomeRoot } from './backend-env' import { isReauthRequiredError, waitForHermesReady } from './backend-health' +import { + backendCommandMatches, + createBackendOwnership, + createBackendShutdownCoordinator +} from './backend-ownership' import { canImportHermesCli, execProbeSync, @@ -467,7 +472,7 @@ if (IS_WINDOWS) { try { app.relaunch({ args: buildNoSandboxRelaunchArgs(process.argv.slice(1)) }) - app.exit(0) + void exitAfterBackendShutdown(0) } catch (error) { console.error(`[hermes] --no-sandbox relaunch failed: ${error?.message || error}`) } @@ -655,6 +660,7 @@ const DESKTOP_CONNECTION_CONFIG_PATH = path.join(app.getPath('userData'), 'conne const DESKTOP_INSTALLATION_PATH = path.join(app.getPath('userData'), 'desktop-installation.json') const DESKTOP_UPDATE_CONFIG_PATH = path.join(app.getPath('userData'), 'updates.json') const DESKTOP_WINDOW_STATE_PATH = path.join(app.getPath('userData'), 'window-state.json') +const DESKTOP_BACKEND_OWNERSHIP_PATH = path.join(app.getPath('userData'), 'backend-ownership.json') // active-profile.json records which Hermes profile the desktop launches its // local backend as. When set, startHermes() passes `hermes --profile // dashboard …`, which deterministically pins HERMES_HOME (see @@ -1122,6 +1128,7 @@ const POOL_IDLE_MS = Math.max(60_000, Number(process.env.HERMES_DESKTOP_POOL_IDL // killing one to honor the soft cap would abort a running agent. const POOL_KEEPALIVE_FRESH_MS = 90_000 let poolIdleReaper = null +let backendOrphanReapPromise = null // Auto-reload budget for renderer crashes, shared by EVERY window (primary, // secondary session, instance) so a crash loop anywhere is suppressed after // the same budget instead of reloading per-window forever. A deterministic @@ -2885,6 +2892,226 @@ function forceKillProcessTree(pid) { } } +function writeBackendOwnership(contents) { + fs.mkdirSync(path.dirname(DESKTOP_BACKEND_OWNERSHIP_PATH), { recursive: true }) + const tempPath = `${DESKTOP_BACKEND_OWNERSHIP_PATH}.${process.pid}.tmp` + + try { + fs.writeFileSync(tempPath, contents, { encoding: 'utf8', mode: 0o600 }) + fs.renameSync(tempPath, DESKTOP_BACKEND_OWNERSHIP_PATH) + } finally { + try { + fs.rmSync(tempPath, { force: true }) + } catch { + void 0 + } + } +} + +function execText(command, args) { + return new Promise((resolve, reject) => { + execFile(command, args, hiddenWindowsChildOptions({ encoding: 'utf8', timeout: 3000 }), (error, stdout) => { + if (error) { + reject(error) + } else { + resolve(String(stdout || '').trim()) + } + }) + }) +} + +async function processStartMarker(pid) { + if (process.platform === 'linux') { + const stat = await fs.promises.readFile(`/proc/${pid}/stat`, 'utf8') + const fields = stat.slice(stat.lastIndexOf(')') + 1).trim().split(/\s+/) + + if (!/^\d+$/.test(fields[19] || '')) { + throw new Error(`Invalid /proc start marker for PID ${pid}`) + } + + return `linux:${fields[19]}` + } + + if (IS_WINDOWS) { + const ticks = await execText('powershell.exe', [ + '-NoProfile', + '-NonInteractive', + '-Command', + `$p = Get-Process -Id ${pid} -ErrorAction Stop; $p.StartTime.ToUniversalTime().Ticks` + ]) + + if (!/^\d+$/.test(ticks)) { + throw new Error(`Invalid Windows start marker for PID ${pid}`) + } + + return `win:${ticks}` + } + + const started = await execText('ps', ['-p', String(pid), '-o', 'lstart=']) + + if (!started) { + throw new Error(`Missing process start marker for PID ${pid}`) + } + + return `ps:${started}` +} + +async function backendCommandForPid(pid) { + try { + const command = IS_WINDOWS ? 'powershell.exe' : 'ps' + + const args = IS_WINDOWS + ? ['-NoProfile', '-NonInteractive', '-Command', `(Get-CimInstance Win32_Process -Filter 'ProcessId = ${pid}').CommandLine`] + : ['-p', String(pid), '-o', 'command='] + + return (await execText(command, args)) || null + } catch { + return null + } +} + +async function processIdentityMatches(identity) { + try { + return (await processStartMarker(identity.pid)) === identity.startMarker + } catch (error) { + return error?.code === 'ENOENT' || error?.code === 'ESRCH' ? false : undefined + } +} + +async function backendIdentityMatches(identity) { + const processMatches = await processIdentityMatches(identity) + + if (processMatches !== true) { + return processMatches + } + + const command = await backendCommandForPid(identity.pid) + + return command === null ? undefined : backendCommandMatches(command) +} + +async function stopOwnedBackend(identity) { + if ((await processIdentityMatches(identity)) !== true) { + return + } + + if (IS_WINDOWS) { + forceKillProcessTree(identity.pid) + } else { + try { + process.kill(-identity.pid, 'SIGTERM') + } catch { + try { + process.kill(identity.pid, 'SIGTERM') + } catch { + return + } + } + + const deadline = Date.now() + 1500 + + while (Date.now() < deadline) { + if ((await processIdentityMatches(identity)) !== true) { + return + } + + await new Promise(resolve => setTimeout(resolve, 50)) + } + + // Revalidate immediately before escalation so PID reuse cannot target a + // replacement process. + if ((await processIdentityMatches(identity)) === true) { + try { + process.kill(-identity.pid, 'SIGKILL') + } catch { + process.kill(identity.pid, 'SIGKILL') + } + } + } + + await new Promise(resolve => setTimeout(resolve, 50)) + const remaining = await processIdentityMatches(identity) + + if (remaining !== false) { + throw new Error(`Backend PID ${identity.pid} did not stop cleanly.`) + } +} + +const backendOwnership = createBackendOwnership({ + matchesIdentity: backendIdentityMatches, + stop: stopOwnedBackend, + store: { + read: () => { + try { + return fs.readFileSync(DESKTOP_BACKEND_OWNERSHIP_PATH, 'utf8') + } catch { + return null + } + }, + write: writeBackendOwnership + } +}) + +let desktopParentStartMarkerPromise = null + +function desktopParentStartMarker() { + desktopParentStartMarkerPromise ??= processStartMarker(process.pid) + + return desktopParentStartMarkerPromise +} + +async function claimBackendChild(child, command, profile, nonce) { + try { + const identity = await backendOwnership.claim({ + command, + nonce, + pid: child.pid, + profile, + startMarker: await processStartMarker(child.pid) + }) + + child.hermesBackendIdentity = identity + + return identity + } catch (error) { + stopBackendChild(child) + await waitForBackendExit(child) + throw new Error(`Could not persist ownership for the Hermes backend: ${error.message}`) + } +} + +function releaseBackendChild(child) { + const identity = child?.hermesBackendIdentity + + if (!identity) { + return + } + + try { + backendOwnership.release(identity) + } catch (error) { + rememberLog(`Could not release backend ownership for PID ${identity.pid}: ${error.message}`) + } +} + +function reapOrphanedBackendsOnce() { + if (!backendOrphanReapPromise) { + backendOrphanReapPromise = backendOwnership + .reapOrphans() + .then(pids => { + if (pids.length) { + rememberLog(`Reaped orphaned desktop backend PID(s): ${pids.join(', ')}`) + } + }) + .catch(error => { + backendOrphanReapPromise = null + throw error + }) + } + + return backendOrphanReapPromise +} + // Before handing off the update on Windows, the desktop MUST stop every backend // it spawned and WAIT for the venv shim to actually unlock. The old code did // `hermesProcess.kill('SIGTERM')` + `app.quit()` fire-and-forget: SIGTERM on @@ -7350,6 +7577,7 @@ const desktopInstallationId = loadOrCreateInstallationId(DESKTOP_INSTALLATION_PA const sshBootstrapCoordinator = createBootstrapCoordinator() let sshQuitTeardownDone = false +let backendQuitTeardownDone = false function sshScopeKey(profile) { return connectionScopeKey(profile) || '' @@ -8050,42 +8278,52 @@ function sendConnectionApplied() { } async function waitForBackendExit(child, timeoutMs = 5000) { - if (!child) { + if (!child || child.exitCode !== null || child.signalCode !== null) { return } - if (child.exitCode !== null || child.signalCode !== null) { - return - } + const exited = () => child.exitCode !== null || child.signalCode !== null - await new Promise(resolve => { - const timer = setTimeout(() => { - try { - if (IS_WINDOWS && Number.isInteger(child.pid)) { - forceKillProcessTree(child.pid) - } else if (Number.isInteger(child.pid)) { - // POSIX: SIGKILL the whole group (pgid==pid, start_new_session) so - // MCP grandchildren die with the backend. Fall back to the child. - try { - process.kill(-child.pid, 'SIGKILL') - } catch { - child.kill('SIGKILL') - } - } else { - child.kill('SIGKILL') - } - } catch { - // Already gone. + const wait = delay => + new Promise(resolve => { + if (exited()) { + resolve() + + return } - resolve() - }, timeoutMs) - - child.once('exit', () => { - clearTimeout(timer) - resolve() + const timer = setTimeout(resolve, delay) + child.once('exit', () => { + clearTimeout(timer) + resolve() + }) }) - }) + + await wait(timeoutMs) + + if (exited()) { + return + } + + try { + if (IS_WINDOWS && Number.isInteger(child.pid)) { + forceKillProcessTree(child.pid) + } else if (Number.isInteger(child.pid)) { + try { + process.kill(-child.pid, 'SIGKILL') + } catch { + child.kill('SIGKILL') + } + } else { + child.kill('SIGKILL') + } + } catch { + return + } + + // Await the escalation as well; do not let shutdown or failed adoption race + // a still-running backend. + await wait(1000) } // The profile the primary (window) backend runs as. readActiveDesktopProfile() @@ -8142,8 +8380,13 @@ async function ensureBackend(profile) { remoteBaseUrl: null } - entry.connectionPromise = spawnPoolBackend(key, entry).catch(error => { - backendPool.delete(key) + entry.connectionPromise = spawnPoolBackend(key, entry).catch(async error => { + if (backendPool.get(key) === entry) { + backendPool.delete(key) + } + + stopBackendChild(entry.process) + await waitForBackendExit(entry.process) throw error }) backendPool.set(key, entry) @@ -8227,6 +8470,7 @@ function startPoolIdleReaper() { // local-spawn portion of startHermes() but without the boot-progress UI, // bootstrap, or remote handling (those belong to the primary backend only). async function spawnPoolBackend(profile, entry) { + await reapOrphanedBackendsOnce() // A profile may point at its OWN remote backend (connection.json // `profiles[name]`), or inherit the app-wide remote (env / global settings). // In either case there is no local child to spawn — we just verify the @@ -8285,6 +8529,9 @@ async function spawnPoolBackend(profile, entry) { rememberLog(`Starting Hermes backend for profile "${profile}" via ${backend.label}`) + const parentStartMarker = await desktopParentStartMarker() + const backendNonce = crypto.randomBytes(16).toString('hex') + const child = spawn( backend.command, backend.args, @@ -8302,11 +8549,11 @@ async function spawnPoolBackend(profile, entry) { // Marks this dashboard backend as desktop-spawned so it runs the cron // scheduler tick loop (the gateway isn't running under the app). HERMES_DESKTOP: '1', - // Our PID so the backend's parent-death watchdog self-exits if we die - // uncleanly (crash / SIGKILL / update handoff) instead of leaking a - // serving backend + its MCP child subtree. See web_server.py - // _start_parent_death_watchdog. + // Exact parent identity lets the backend self-exit after an unclean + // Desktop death without mistaking a reused PID for its owner. HERMES_PARENT_PID: String(process.pid), + HERMES_PARENT_START_MARKER: parentStartMarker, + HERMES_PARENT_NONCE: backendNonce, HERMES_WEB_DIST: webDist, ...(readyFile ? { HERMES_DESKTOP_READY_FILE: readyFile } : {}) }, @@ -8317,6 +8564,7 @@ async function spawnPoolBackend(profile, entry) { entry.process = child entry.token = token + await claimBackendChild(child, `${backend.command} ${backend.args.join(' ')}`, profile, backendNonce) child.stdout.on('data', rememberLog) child.stderr.on('data', rememberLog) @@ -8330,11 +8578,13 @@ async function spawnPoolBackend(profile, entry) { child.once('error', error => { rememberLog(`Hermes backend for profile "${profile}" failed to start: ${error.message}`) + releaseBackendChild(child) backendPool.delete(profile) rejectStart?.(error) }) child.once('exit', (code, signal) => { rememberLog(`Hermes backend for profile "${profile}" exited (${signal || code})`) + releaseBackendChild(child) backendPool.delete(profile) if (!ready) { @@ -8420,6 +8670,26 @@ function stopAllPoolBackends() { } } +const backendShutdown = createBackendShutdownCoordinator(async () => { + const primary = backendConnectionState.invalidate() + const pooled = [...backendPool.values()].map(entry => entry.process).filter(Boolean) + + stopBackendChild(primary) + stopAllPoolBackends() + + if (poolIdleReaper) { + clearInterval(poolIdleReaper) + poolIdleReaper = null + } + + await Promise.all([waitForBackendExit(primary), ...pooled.map(child => waitForBackendExit(child))]) +}) + +async function exitAfterBackendShutdown(code) { + await backendShutdown.run() + app.exit(code) +} + // Returns the profile name whose backend was torn down, or null when the // request is not a profile-delete. The caller uses this to skip ensureBackend // for the just-torn-down profile — otherwise ensureBackend respawns a pool @@ -8455,6 +8725,8 @@ async function prepareProfileDeleteRequest(request) { } async function startHermes() { + await reapOrphanedBackendsOnce() + // Latched-failure short-circuit: once bootstrap has failed in this // process, every subsequent startHermes() call re-throws the same error // without re-running install.ps1. This prevents the renderer's @@ -8578,6 +8850,10 @@ async function startHermes() { await advanceBootProgress('backend.spawn', `Starting Hermes backend via ${backend.label}`, 84) rememberLog(`Starting Hermes backend via ${backend.label}`) + const profile = primaryProfileKey() + const parentStartMarker = await desktopParentStartMarker() + const backendNonce = crypto.randomBytes(16).toString('hex') + const hermesProcess = spawn( backend.command, backend.args, @@ -8600,11 +8876,11 @@ async function startHermes() { // Marks this dashboard backend as desktop-spawned so it runs the cron // scheduler tick loop (the gateway isn't running under the app). HERMES_DESKTOP: '1', - // Our PID so the backend's parent-death watchdog self-exits if we die - // uncleanly (crash / SIGKILL / update handoff) instead of leaking a - // serving backend + its MCP child subtree. See web_server.py - // _start_parent_death_watchdog. + // Exact parent identity lets the backend self-exit after an unclean + // Desktop death without mistaking a reused PID for its owner. HERMES_PARENT_PID: String(process.pid), + HERMES_PARENT_START_MARKER: parentStartMarker, + HERMES_PARENT_NONCE: backendNonce, HERMES_WEB_DIST: webDist, ...(readyFile ? { HERMES_DESKTOP_READY_FILE: readyFile } : {}) }, @@ -8613,10 +8889,13 @@ async function startHermes() { }) ) + await claimBackendChild(hermesProcess, `${backend.command} ${backend.args.join(' ')}`, profile, backendNonce) const processOwner = backendConnectionState.attachProcess(connectionAttempt, hermesProcess) if (!processOwner) { stopBackendChild(hermesProcess) + await waitForBackendExit(hermesProcess) + releaseBackendChild(hermesProcess) throw new Error('Hermes backend start was superseded by a newer connection attempt.') } @@ -8630,6 +8909,8 @@ async function startHermes() { }) hermesProcess.once('error', error => { + releaseBackendChild(hermesProcess) + if (!backendConnectionState.clearForCurrentProcess(processOwner)) { rememberLog(`Ignoring stale Hermes backend error: ${error.message}`) rejectBackendStart?.(new Error('Hermes backend start was superseded by a newer connection attempt.')) @@ -8651,6 +8932,8 @@ async function startHermes() { rejectBackendStart?.(error) }) hermesProcess.once('exit', (code, signal) => { + releaseBackendChild(hermesProcess) + if (!backendConnectionState.clearForCurrentProcess(processOwner)) { rememberLog(`Ignoring stale Hermes backend exit (${signal || code})`) @@ -8741,11 +9024,15 @@ async function startHermes() { logs: hermesLog.slice(-80), ...getWindowState() } - })().catch(error => { + })().catch(async error => { if (!backendConnectionState.clearPromiseForAttempt(connectionAttempt)) { throw error } + const failedProcess = backendConnectionState.invalidate() + stopBackendChild(failedProcess) + await waitForBackendExit(failedProcess) + if (error instanceof FirstRunSetupResetError) { throw error } @@ -9992,7 +10279,7 @@ function createWindow() { try { app.relaunch({ args: buildNoSandboxRelaunchArgs(process.argv.slice(1)) }) - app.exit(0) + void exitAfterBackendShutdown(0) } catch (err) { rememberLog(`[renderer] --no-sandbox relaunch failed: ${err?.message || err}`) } @@ -12750,6 +13037,14 @@ app.on('before-quit', event => { return } + if (!backendQuitTeardownDone) { + event.preventDefault() + void backendShutdown.run().finally(() => { + backendQuitTeardownDone = true + app.quit() + }) + } + if ((sshConnections.size > 0 || sshBootstrapCoordinator.promises().length > 0) && !sshQuitTeardownDone) { event.preventDefault() sshBootstrapCoordinator.cancelAll() @@ -12824,8 +13119,7 @@ app.on('before-quit', event => { disposeTerminalSession(id) } - stopBackendChild(backendConnectionState.getProcess()) - stopAllPoolBackends() + void backendShutdown.run() }) app.on('window-all-closed', () => { diff --git a/apps/desktop/src/app/contrib/controller.tsx b/apps/desktop/src/app/contrib/controller.tsx index a70efb6f65..80e16fa272 100644 --- a/apps/desktop/src/app/contrib/controller.tsx +++ b/apps/desktop/src/app/contrib/controller.tsx @@ -184,7 +184,13 @@ registry.registerMany([ // NO minHeight: a tool panel drags all the way down to its collapsed // header (the sash floors it at COLLAPSED_ZONE_PX and folds the zone to // its rail there). A real floor left a sliver of unusable terminal. - data: { placement: 'bottom', height: '20vh', maxHeight: '80vh', revealOnPreset: true }, + data: { + placement: 'bottom', + height: '20vh', + maxHeight: '80vh', + revealOnPreset: true, + lifecycleKeepAlive: true + }, render: () => }, { diff --git a/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx b/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx index f96307f361..bb6f4e74a8 100644 --- a/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx @@ -21,6 +21,7 @@ import { setCurrentServiceTier, setTurnStartedAt } from '@/store/session' +import { $sessionStates } from '@/store/session-states' import { useSessionStateCache } from './use-session-state-cache' @@ -377,6 +378,10 @@ function assistantError(id: string, error: string): ChatMessage { return { id, role: 'assistant', parts: [], error, pending: false } } +function transcriptForCache(id: string): ChatMessage[] { + return [userMessage(`${id}-user`, id), assistantText(`${id}-assistant`, `reply ${id}`)] +} + interface ViewHarnessProps { activeSessionId: string | null onReady: (cache: Cache) => void @@ -405,6 +410,7 @@ describe('useSessionStateCache — cross-thread error isolation', () => { afterEach(() => { cleanup() $messages.set([]) + $sessionStates.set({}) }) it('does not leak a failed turn into another thread on switch', () => { @@ -475,6 +481,27 @@ describe('useSessionStateCache — cross-thread error isolation', () => { expect($messages.get().some(message => message.error === 'OpenRouter 403')).toBe(true) }) + it('evicts the oldest warm transcript with its reverse ownership while retaining lightweight state', () => { + let cache!: Cache + render( (cache = value)} selectedStoredSessionId={null} />) + + act(() => { + for (let index = 0; index < 25; index += 1) { + cache.updateSessionState( + `runtime-${index}`, + state => ({ ...state, messages: transcriptForCache(`message-${index}`) }), + `stored-${index}` + ) + } + }) + + expect(cache.sessionStateByRuntimeIdRef.current.has('runtime-0')).toBe(false) + expect(cache.runtimeIdByStoredSessionIdRef.current.has('stored-0')).toBe(false) + expect($sessionStates.get()['runtime-0']).toMatchObject({ storedSessionId: 'stored-0', busy: false }) + expect($sessionStates.get()['runtime-0']?.messages).toEqual([]) + expect(cache.getRuntimeIdForStoredSession('stored-24')).toBe('runtime-24') + }) + it('only returns a runtime whose cached state owns the requested stored session', () => { let cache!: Cache render( (cache = value)} selectedStoredSessionId={null} />) diff --git a/apps/desktop/src/app/session/hooks/use-session-state-cache.ts b/apps/desktop/src/app/session/hooks/use-session-state-cache.ts index 04fe08513e..43dd15ea91 100644 --- a/apps/desktop/src/app/session/hooks/use-session-state-cache.ts +++ b/apps/desktop/src/app/session/hooks/use-session-state-cache.ts @@ -20,9 +20,10 @@ import { setTurnStartedAt, setYoloActive } from '@/store/session' -import { publishSessionState } from '@/store/session-states' +import { $sessionTiles, publishSessionState, releaseSessionTranscript } from '@/store/session-states' import type { ClientSessionState } from '../../types' +import { SessionStateCache } from '../session-state-cache' import { chatMessageArraysEquivalent } from './use-session-actions/utils' @@ -54,6 +55,7 @@ export function useSessionStateCache({ setMessages }: SessionStateCacheOptions) { const busy = useStore($busy) + const sessionTiles = useStore($sessionTiles) const activeSessionIdRef = useRef(activeSessionId) const selectedStoredSessionIdRef = useRef(selectedStoredSessionId) @@ -81,8 +83,35 @@ export function useSessionStateCache({ selectedStoredSessionIdRef.current = selectedStoredSessionId } - const sessionStateByRuntimeIdRef = useRef(new Map()) const runtimeIdByStoredSessionIdRef = useRef(new Map()) + const sessionStateByRuntimeIdRef = useRef(null!) + + if (sessionStateByRuntimeIdRef.current === null) { + sessionStateByRuntimeIdRef.current = new SessionStateCache({ + isReferenced: (runtimeId, state) => + runtimeId === activeSessionIdRef.current || + state.storedSessionId === selectedStoredSessionIdRef.current || + $sessionTiles + .get() + .some( + tile => + tile.runtimeId === runtimeId || + (state.storedSessionId !== null && tile.storedSessionId === state.storedSessionId) + ), + onEvict: (runtimeId, state) => { + // Ownership is removed with the transcript, but only if both sides still + // describe this exact binding. A recycled runtime must not erase its + // new owner's reverse entry. + if (state.storedSessionId && runtimeIdByStoredSessionIdRef.current.get(state.storedSessionId) === runtimeId) { + runtimeIdByStoredSessionIdRef.current.delete(state.storedSessionId) + } + + releaseSessionTranscript(runtimeId) + } + }) + } + + const sessionStateCache = sessionStateByRuntimeIdRef.current const pendingViewStateRef = useRef<{ sessionId: string; state: ClientSessionState } | null>(null) const viewSyncRafRef = useRef(null) // Runtime id whose transcript currently occupies `$messages` — lets the @@ -94,58 +123,62 @@ export function useSessionStateCache({ setMutableRef(busyRef, busy) }, [busy, busyRef]) - const ensureSessionState = useCallback((sessionId: string, storedSessionId?: string | null) => { - const existing = sessionStateByRuntimeIdRef.current.get(sessionId) + const ensureSessionState = useCallback( + (sessionId: string, storedSessionId?: string | null) => { + const existing = sessionStateCache.get(sessionId) - if (existing) { - if (storedSessionId !== undefined && storedSessionId !== existing.storedSessionId) { - // Stored id changed (e.g. auto-compression rotated it). Create a NEW - // state object rather than mutating in place — updateSessionState needs - // the PREVIOUS state to detect transitions (busy→idle, id rotation). - const updated = { ...existing, storedSessionId } + if (existing) { + if (storedSessionId !== undefined && storedSessionId !== existing.storedSessionId) { + // Stored id changed (e.g. auto-compression rotated it). Create a NEW + // state object rather than mutating in place — updateSessionState needs + // the PREVIOUS state to detect transitions (busy→idle, id rotation). + const updated = { ...existing, storedSessionId } - sessionStateByRuntimeIdRef.current.set(sessionId, updated) + // Drop the obsolete stored→runtime reverse mapping as soon as the id + // rotates (e.g. auto-compression forks a continuation). Leaving the + // stale key lets getRuntimeIdForStoredSession resolve the old stored id + // to this runtime, which the compression route-follow logic relies on + // being absent. The rotation signal was previously emitted centrally + // from handleTransition (session-states.ts), but updateSessionState + // now skips publishSessionState (and thus handleTransition) when the + // updater is a no-op — fire it here so the route-follow effect still + // tracks compression without needing a dummy state write. + if (existing.storedSessionId && existing.storedSessionId !== storedSessionId) { + runtimeIdByStoredSessionIdRef.current.delete(existing.storedSessionId) - // Drop the obsolete stored→runtime reverse mapping as soon as the id - // rotates (e.g. auto-compression forks a continuation). Leaving the - // stale key lets getRuntimeIdForStoredSession resolve the old stored id - // to this runtime, which the compression route-follow logic relies on - // being absent. The rotation signal was previously emitted centrally - // from handleTransition (session-states.ts), but updateSessionState - // now skips publishSessionState (and thus handleTransition) when the - // updater is a no-op — fire it here so the route-follow effect still - // tracks compression without needing a dummy state write. - if (existing.storedSessionId && existing.storedSessionId !== storedSessionId) { - runtimeIdByStoredSessionIdRef.current.delete(existing.storedSessionId) - - // A rotation event needs a real next id — a null/cleared stored id - // is a detach, not a rotation the route-follow effect should chase. - if (storedSessionId && sessionId === $activeSessionId.get()) { - setActiveSessionStoredIdRotation({ - nextStoredSessionId: storedSessionId, - previousStoredSessionId: existing.storedSessionId, - runtimeSessionId: sessionId - }) + // A rotation event needs a real next id — a null/cleared stored id + // is a detach, not a rotation the route-follow effect should chase. + if (storedSessionId && sessionId === $activeSessionId.get()) { + setActiveSessionStoredIdRotation({ + nextStoredSessionId: storedSessionId, + previousStoredSessionId: existing.storedSessionId, + runtimeSessionId: sessionId + }) + } } + + if (storedSessionId) { + runtimeIdByStoredSessionIdRef.current.set(storedSessionId, sessionId) + } + + sessionStateCache.set(sessionId, updated) } - if (storedSessionId) { - runtimeIdByStoredSessionIdRef.current.set(storedSessionId, sessionId) - } + return sessionStateCache.get(sessionId)! } - return sessionStateByRuntimeIdRef.current.get(sessionId)! - } + const created = createClientSessionState(storedSessionId ?? null) - const created = createClientSessionState(storedSessionId ?? null) - sessionStateByRuntimeIdRef.current.set(sessionId, created) + if (storedSessionId) { + runtimeIdByStoredSessionIdRef.current.set(storedSessionId, sessionId) + } - if (storedSessionId) { - runtimeIdByStoredSessionIdRef.current.set(storedSessionId, sessionId) - } + sessionStateCache.set(sessionId, created) - return created - }, []) + return created + }, + [sessionStateCache] + ) const resetViewSync = useCallback(() => { // Drop any RAF-pending transcript stage so a backgrounded turn cannot @@ -299,7 +332,7 @@ export function useSessionStateCache({ return previous } - sessionStateByRuntimeIdRef.current.set(sessionId, next) + sessionStateCache.set(sessionId, next) // Crash-survivable turn progress: journal the running turn's visible // tail (throttled localStorage write; cleared the moment the turn // settles) so a renderer/app death mid-turn can be recovered on resume. @@ -308,24 +341,32 @@ export function useSessionStateCache({ // (watchdog, settle grace, unread marker, compression id rotation) inside // publishSessionState — no manual transition call needed. publishSessionState(sessionId, next) + sessionStateCache.prune() syncSessionStateToView(sessionId, next) return next }, - [ensureSessionState, syncSessionStateToView] + [ensureSessionState, sessionStateCache, syncSessionStateToView] ) - const getRuntimeIdForStoredSession = useCallback((storedSessionId: string): string | null => { - const runtimeId = runtimeIdByStoredSessionIdRef.current.get(storedSessionId) + useEffect(() => { + sessionStateCache.prune() + }, [activeSessionId, selectedStoredSessionId, sessionStateCache, sessionTiles]) - if (!runtimeId) { - return null - } + const getRuntimeIdForStoredSession = useCallback( + (storedSessionId: string): string | null => { + const runtimeId = runtimeIdByStoredSessionIdRef.current.get(storedSessionId) - const runtimeState = sessionStateByRuntimeIdRef.current.get(runtimeId) + if (!runtimeId) { + return null + } - return runtimeState?.storedSessionId === storedSessionId ? runtimeId : null - }, []) + const runtimeState = sessionStateCache.get(runtimeId) + + return runtimeState?.storedSessionId === storedSessionId ? runtimeId : null + }, + [sessionStateCache] + ) return { activeSessionIdRef, @@ -334,7 +375,7 @@ export function useSessionStateCache({ resetViewSync, runtimeIdByStoredSessionIdRef, selectedStoredSessionIdRef, - sessionStateByRuntimeIdRef, + sessionStateByRuntimeIdRef: sessionStateByRuntimeIdRef as MutableRefObject>, syncSessionStateToView, updateSessionState } diff --git a/apps/desktop/src/app/session/session-state-cache.test.ts b/apps/desktop/src/app/session/session-state-cache.test.ts new file mode 100644 index 0000000000..a52ce3238e --- /dev/null +++ b/apps/desktop/src/app/session/session-state-cache.test.ts @@ -0,0 +1,133 @@ +import { beforeEach, describe, expect, it } from 'vitest' + +import type { ClientSessionState } from '@/app/types' +import type { ChatMessage } from '@/lib/chat-messages' +import { createClientSessionState } from '@/lib/chat-runtime' +import { $sessionStates, $sessionTiles, releaseSessionTranscript } from '@/store/session-states' + +import { SessionStateCache } from './session-state-cache' + +function transcript(id: string, text = id): ChatMessage[] { + return [ + { id: `${id}-user`, role: 'user', parts: [{ type: 'text', text }] }, + { id: `${id}-assistant`, role: 'assistant', parts: [{ type: 'text', text: `reply ${text}` }] } + ] +} + +function settled(storedSessionId: string, text = storedSessionId): ClientSessionState { + return { ...createClientSessionState(storedSessionId), messages: transcript(storedSessionId, text) } +} + +describe('SessionStateCache', () => { + beforeEach(() => { + $sessionStates.set({}) + $sessionTiles.set([]) + }) + + it('bounds warm settled transcripts by LRU count and cleans ownership atomically', () => { + const owners = new Map() + const evicted: string[] = [] + + const cache = new SessionStateCache( + { + isReferenced: () => false, + onEvict: (runtimeId, state) => { + if (state.storedSessionId && owners.get(state.storedSessionId) === runtimeId) { + owners.delete(state.storedSessionId) + } + + evicted.push(runtimeId) + } + }, + { maxBytes: Number.POSITIVE_INFINITY, maxCount: 2 } + ) + + for (const id of ['a', 'b', 'c']) { + owners.set(`stored-${id}`, `runtime-${id}`) + cache.set(`runtime-${id}`, settled(`stored-${id}`)) + } + + // A read makes A warmer than B, so B is the oldest when pruning. + cache.get('runtime-a') + cache.prune() + + expect([...cache.keys()].sort()).toEqual(['runtime-a', 'runtime-c']) + expect(evicted).toEqual(['runtime-b']) + expect(owners.has('stored-b')).toBe(false) + + // A recycled reverse mapping is not owned by the evicted runtime and must + // survive cleanup. + owners.set('stored-a', 'runtime-new-owner') + cache.set('runtime-d', settled('stored-d')) + owners.set('stored-d', 'runtime-d') + cache.prune() + expect(owners.get('stored-a')).toBe('runtime-new-owner') + }) + + it('uses transcript bytes as well as count', () => { + const evicted: string[] = [] + + const cache = new SessionStateCache( + { isReferenced: () => false, onEvict: runtimeId => evicted.push(runtimeId) }, + { maxBytes: 600, maxCount: 10 } + ) + + cache.set('small', settled('small', 'x')) + cache.set('large', settled('large', 'x'.repeat(500))) + cache.prune() + + expect(evicted).toEqual(['small', 'large']) + expect(cache.size).toBe(0) + }) + + it.each([ + ['active', (state: ClientSessionState) => state, true], + ['tiled', (state: ClientSessionState) => state, true], + ['busy', (state: ClientSessionState) => ({ ...state, busy: true }), false], + ['awaiting', (state: ClientSessionState) => ({ ...state, awaitingResponse: true }), false], + ['needs input', (state: ClientSessionState) => ({ ...state, needsInput: true }), false] + ])('never evicts %s transcripts', (_label, decorate, referenced) => { + const protectedState = decorate(settled('protected')) + + const cache = new SessionStateCache( + { + isReferenced: runtimeId => referenced && runtimeId === 'protected', + onEvict: () => undefined + }, + { maxBytes: 0, maxCount: 0 } + ) + + cache.set('protected', protectedState) + cache.prune() + + expect(cache.get('protected')).toBe(protectedState) + }) + + it('keeps unsaved drafts and pending messages out of the eviction pool', () => { + const draft = { ...createClientSessionState(null), messages: transcript('draft') } + const pending = settled('pending') + pending.messages = [{ id: 'pending-assistant', role: 'assistant', parts: [], pending: true }] + + const cache = new SessionStateCache( + { isReferenced: () => false, onEvict: () => undefined }, + { maxBytes: 0, maxCount: 0 } + ) + + cache.set('draft', draft) + cache.set('pending', pending) + cache.prune() + + expect(cache.has('draft')).toBe(true) + expect(cache.has('pending')).toBe(true) + }) + + it('retains lightweight status while releasing an evicted transcript', () => { + const state = { ...settled('stored'), needsInput: false } + $sessionStates.set({ runtime: state }) + + releaseSessionTranscript('runtime') + + expect($sessionStates.get().runtime).toMatchObject({ storedSessionId: 'stored', busy: false, needsInput: false }) + expect($sessionStates.get().runtime.messages).toEqual([]) + }) +}) diff --git a/apps/desktop/src/app/session/session-state-cache.ts b/apps/desktop/src/app/session/session-state-cache.ts new file mode 100644 index 0000000000..9c4197e6d2 --- /dev/null +++ b/apps/desktop/src/app/session/session-state-cache.ts @@ -0,0 +1,135 @@ +import type { ClientSessionState } from '../types' + +export const DEFAULT_WARM_SESSION_TRANSCRIPT_COUNT = 24 +export const DEFAULT_WARM_SESSION_TRANSCRIPT_BYTES = 32 * 1024 * 1024 + +interface SessionStateCacheLimits { + maxBytes?: number + maxCount?: number +} + +interface SessionStateCacheCallbacks { + isReferenced: (runtimeId: string, state: ClientSessionState) => boolean + onEvict: (runtimeId: string, state: ClientSessionState) => void +} + +function transcriptBytes(state: ClientSessionState): number { + if (state.messages.length === 0) { + return 0 + } + + // JS strings occupy two bytes per UTF-16 code unit. JSON also accounts for + // ids, part tags, tool payloads, attachment metadata, and error text without + // retaining a second serialized copy in the cache. + return JSON.stringify(state.messages).length * 2 +} + +function hasDraftOrInFlightMessage(state: ClientSessionState): boolean { + return state.messages.some(message => message.pending === true) +} + +/** + * Runtime state map whose settled, unreferenced transcripts form a weighted + * LRU. Live/visible states and unsaved drafts are outside both limits. + */ +export class SessionStateCache extends Map { + readonly #callbacks: SessionStateCacheCallbacks + readonly #maxBytes: number + readonly #maxCount: number + readonly #recency = new Map() + #clock = 0 + + constructor(callbacks: SessionStateCacheCallbacks, limits: SessionStateCacheLimits = {}) { + super() + this.#callbacks = callbacks + this.#maxBytes = limits.maxBytes ?? DEFAULT_WARM_SESSION_TRANSCRIPT_BYTES + this.#maxCount = limits.maxCount ?? DEFAULT_WARM_SESSION_TRANSCRIPT_COUNT + } + + override get(runtimeId: string): ClientSessionState | undefined { + const state = super.get(runtimeId) + + if (state) { + this.#touch(runtimeId) + } + + return state + } + + override set(runtimeId: string, state: ClientSessionState): this { + super.set(runtimeId, state) + this.#touch(runtimeId) + + return this + } + + override delete(runtimeId: string): boolean { + this.#recency.delete(runtimeId) + + return super.delete(runtimeId) + } + + override clear(): void { + this.#recency.clear() + super.clear() + } + + prune(): void { + const candidates: Array<{ bytes: number; runtimeId: string; state: ClientSessionState; touched: number }> = [] + let bytes = 0 + + for (const [runtimeId, state] of this.entries()) { + if (!this.#isWarmSettled(runtimeId, state)) { + continue + } + + const weight = transcriptBytes(state) + candidates.push({ bytes: weight, runtimeId, state, touched: this.#recency.get(runtimeId) ?? 0 }) + bytes += weight + } + + let count = candidates.length + + if (count <= this.#maxCount && bytes <= this.#maxBytes) { + return + } + + candidates.sort((a, b) => a.touched - b.touched) + + for (const candidate of candidates) { + if (count <= this.#maxCount && bytes <= this.#maxBytes) { + break + } + + // References and activity can change between insertion and pruning. + const current = super.get(candidate.runtimeId) + + if (current !== candidate.state || !this.#isWarmSettled(candidate.runtimeId, current)) { + continue + } + + super.delete(candidate.runtimeId) + this.#recency.delete(candidate.runtimeId) + count -= 1 + bytes -= candidate.bytes + this.#callbacks.onEvict(candidate.runtimeId, candidate.state) + } + } + + #isWarmSettled(runtimeId: string, state: ClientSessionState): boolean { + return ( + Boolean(state.storedSessionId) && + state.messages.length > 0 && + !state.busy && + !state.awaitingResponse && + !state.needsInput && + !hasDraftOrInFlightMessage(state) && + !this.#callbacks.isReferenced(runtimeId, state) + ) + } + + #touch(runtimeId: string): void { + this.#clock += 1 + this.#recency.set(runtimeId, this.#clock) + } +} diff --git a/apps/desktop/src/components/assistant-ui/thread/list.test.ts b/apps/desktop/src/components/assistant-ui/thread/list.test.ts index f2a3d66b4b..f6d6a77e02 100644 --- a/apps/desktop/src/components/assistant-ui/thread/list.test.ts +++ b/apps/desktop/src/components/assistant-ui/thread/list.test.ts @@ -3,11 +3,13 @@ import { describe, expect, it } from 'vitest' import { buildGroups, firstVisibleGroupIndex, + HIDDEN_TRANSCRIPT_RENDER_BUDGET, LIVE_TAIL_MIN_GROUPS, LIVE_TAIL_PARTS, liveTailStart, type MessageGroup, - resolveThreadScrollTarget + resolveThreadScrollTarget, + transcriptPaneBudget } from './list' // Signature rows are `${index}:${id}:${role}:${weight}` (see the useAuiState @@ -15,6 +17,14 @@ import { const signature = (rows: [string, string, number][]) => rows.map(([id, role, weight], index) => `${index}:${id}:${role}:${weight}`).join('\n') +describe('transcriptPaneBudget', () => { + it('uses a fixed live-tail budget while hidden instead of charging every mounted transcript', () => { + expect(transcriptPaneBudget(1, true)).toBe(HIDDEN_TRANSCRIPT_RENDER_BUDGET) + expect(transcriptPaneBudget(4, true)).toBe(HIDDEN_TRANSCRIPT_RENDER_BUDGET) + expect(transcriptPaneBudget(1, false)).toBeGreaterThan(HIDDEN_TRANSCRIPT_RENDER_BUDGET) + }) +}) + describe('buildGroups', () => { it('returns no groups for an empty signature', () => { expect(buildGroups('')).toEqual([]) diff --git a/apps/desktop/src/components/assistant-ui/thread/list.tsx b/apps/desktop/src/components/assistant-ui/thread/list.tsx index afede60385..b0d2c1cf3a 100644 --- a/apps/desktop/src/components/assistant-ui/thread/list.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/list.tsx @@ -17,6 +17,7 @@ import { } from 'react' import { type GetTargetScrollTop, useStickToBottom } from 'use-stick-to-bottom' +import { usePaneLifecycle } from '@/components/pane-shell/pane-visibility' import { useI18n } from '@/i18n' import { messagePaintWeight } from '@/lib/render-weight' import { cn } from '@/lib/utils' @@ -96,6 +97,15 @@ const MIN_VISIBLE_GROUPS = 8 // interruptibly, so the only thing a smaller budget changes is how much work // blocks the click-to-paint path. const FIRST_PAINT_BUDGET = 20 +// A hot-hidden transcript is retained for instant tab return, but keeping its +// full scrollback mounted defeats the bounded pane cache. Preserve only the +// live tail while hidden; revealing it resumes stepped backfill. +export const HIDDEN_TRANSCRIPT_RENDER_BUDGET = 40 + +export const transcriptPaneBudget = (mountedPanes: number, hidden: boolean): number => + hidden + ? HIDDEN_TRANSCRIPT_RENDER_BUDGET + : Math.max(Math.ceil(RENDER_BUDGET / Math.max(1, mountedPanes)), RENDER_BUDGET / 4) // Units the backfill adds per committed step (see the backfill effect). ~8-15 // ordinary turns or 1-2 tool-heavy ones per frame — big enough to fill a page // in ~10 frames, small enough that no single commit approaches a frame budget. @@ -354,8 +364,10 @@ const ThreadMessageListInner: FC = ({ }, []) const mountedPanes = useStore($mountedTranscriptPanes) - // This pane's share of the render budget — see $mountedTranscriptPanes. - const paneBudget = Math.max(Math.ceil(RENDER_BUDGET / Math.max(1, mountedPanes)), RENDER_BUDGET / 4) + const paneLifecycle = usePaneLifecycle() + // Hidden panes retain only a live-tail budget. Visible panes share the normal + // screen budget; a reveal backfills older rows in bounded transition steps. + const paneBudget = transcriptPaneBudget(mountedPanes, paneLifecycle === 'hot-hidden') const [renderBudget, setRenderBudget] = useState(FIRST_PAINT_BUDGET) @@ -379,6 +391,10 @@ const ThreadMessageListInner: FC = ({ setBudgetSessionKey(sessionKey) setHadGroups(hasGroups) setRenderBudget(FIRST_PAINT_BUDGET) + } else if (renderBudget > paneBudget) { + // Apply the hidden budget during render so React never first commits the + // stale full transcript after this pane moves to the background. + setRenderBudget(paneBudget) } else if (hadGroups !== hasGroups) { setHadGroups(hasGroups) diff --git a/apps/desktop/src/components/pane-shell/pane-lifecycle.test.ts b/apps/desktop/src/components/pane-shell/pane-lifecycle.test.ts new file mode 100644 index 0000000000..f8e2f5838c --- /dev/null +++ b/apps/desktop/src/components/pane-shell/pane-lifecycle.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from 'vitest' + +import { emptyPaneLifecycleState, reconcilePaneLifecycle } from './pane-lifecycle' + +const visit = (state: ReturnType, activeId: string, paneIds: string[]) => + reconcilePaneLifecycle(state, { activeId, paneIds }) + +describe('per-zone pane lifecycle', () => { + it('keeps a small recent hidden set and parks older panes', () => { + let state = emptyPaneLifecycleState() + + state = visit(state, 'a', ['a', 'b', 'c', 'd']) + state = visit(state, 'b', ['a', 'b', 'c', 'd']) + state = visit(state, 'c', ['a', 'b', 'c', 'd']) + state = visit(state, 'd', ['a', 'b', 'c', 'd']) + + expect(state.entries).toMatchObject({ + a: { lifecycle: 'parked' }, + b: { lifecycle: 'hot-hidden' }, + c: { lifecycle: 'hot-hidden' }, + d: { lifecycle: 'visible' } + }) + }) + + it('tracks recency independently for each zone state', () => { + const zoneA = visit(visit(emptyPaneLifecycleState(), 'a', ['a', 'b']), 'b', ['a', 'b']) + const zoneB = visit(emptyPaneLifecycleState(), 'x', ['x', 'y']) + + expect(zoneA.entries.a.lifecycle).toBe('hot-hidden') + expect(zoneA.entries.b.lifecycle).toBe('visible') + expect(zoneB.entries.x.lifecycle).toBe('visible') + expect(zoneB.entries.y).toBeUndefined() + }) + + it('keeps a hidden terminal alive outside the normal cap', () => { + let state = emptyPaneLifecycleState() + const paneIds = ['terminal', 'a', 'b', 'c'] + + const reconcile = (activeId: string) => { + state = reconcilePaneLifecycle(state, { + activeId, + hotHiddenCap: 1, + keepAlive: id => id === 'terminal', + paneIds + }) + } + + reconcile('terminal') + reconcile('a') + reconcile('b') + reconcile('c') + + expect(state.entries.terminal.lifecycle).toBe('hot-hidden') + expect(state.entries.b.lifecycle).toBe('hot-hidden') + expect(state.entries.a.lifecycle).toBe('parked') + }) + + it('forgets panes that leave a zone and remounts a parked pane when selected', () => { + let state = visit(emptyPaneLifecycleState(), 'a', ['a', 'b', 'c', 'd']) + + for (const active of ['b', 'c', 'd']) { + state = visit(state, active, ['a', 'b', 'c', 'd']) + } + + expect(state.entries.a.lifecycle).toBe('parked') + + state = visit(state, 'a', ['a', 'b', 'c']) + + expect(state.entries.a.lifecycle).toBe('visible') + expect(state.entries.d).toBeUndefined() + }) +}) diff --git a/apps/desktop/src/components/pane-shell/pane-lifecycle.ts b/apps/desktop/src/components/pane-shell/pane-lifecycle.ts new file mode 100644 index 0000000000..edac97d3dc --- /dev/null +++ b/apps/desktop/src/components/pane-shell/pane-lifecycle.ts @@ -0,0 +1,71 @@ +export type PaneLifecycle = 'visible' | 'hot-hidden' | 'parked' + +export const DEFAULT_HOT_HIDDEN_PANE_CAP = 2 + +interface PaneLifecycleEntry { + lifecycle: PaneLifecycle + lastVisible: number +} + +export interface PaneLifecycleState { + clock: number + entries: Record +} + +export const emptyPaneLifecycleState = (): PaneLifecycleState => ({ clock: 0, entries: {} }) + +interface ReconcilePaneLifecycleOptions { + activeId: string + hotHiddenCap?: number + keepAlive?: (id: string) => boolean + paneIds: readonly string[] +} + +/** + * Reconcile one zone's mounted pane cache. + * + * The foreground pane is visible, the most recently visible inactive panes stay + * hot up to a small cap, and the rest park (unmount). Explicit keep-alive panes + * such as the terminal remain hot outside that cap so hiding UI never kills the + * stateful resource they host. + */ +export function reconcilePaneLifecycle( + previous: PaneLifecycleState, + { activeId, hotHiddenCap = DEFAULT_HOT_HIDDEN_PANE_CAP, keepAlive = () => false, paneIds }: ReconcilePaneLifecycleOptions +): PaneLifecycleState { + const present = new Set(paneIds) + const entries: Record = {} + let clock = previous.clock + + for (const id of paneIds) { + const prior = previous.entries[id] + + if (prior) { + entries[id] = { ...prior, lifecycle: 'parked' } + } + } + + if (present.has(activeId)) { + const prior = previous.entries[activeId] + + if (!prior || prior.lifecycle !== 'visible') { + clock += 1 + } + + entries[activeId] = { lifecycle: 'visible', lastVisible: clock } + } + + const inactive = paneIds + .filter(id => id !== activeId && entries[id]) + .sort((a, b) => entries[b].lastVisible - entries[a].lastVisible) + + for (const id of inactive.filter(keepAlive)) { + entries[id] = { ...entries[id], lifecycle: 'hot-hidden' } + } + + for (const id of inactive.filter(id => !keepAlive(id)).slice(0, Math.max(0, hotHiddenCap))) { + entries[id] = { ...entries[id], lifecycle: 'hot-hidden' } + } + + return { clock, entries } +} diff --git a/apps/desktop/src/components/pane-shell/pane-visibility.ts b/apps/desktop/src/components/pane-shell/pane-visibility.ts index 0a7169e68b..52276270b2 100644 --- a/apps/desktop/src/components/pane-shell/pane-visibility.ts +++ b/apps/desktop/src/components/pane-shell/pane-visibility.ts @@ -12,6 +12,8 @@ import { createContext, useContext } from 'react' +import type { PaneLifecycle } from './pane-lifecycle' + /** Marks a mounted-but-hidden pane layer (an inactive tab in a stack). */ export const PANE_HIDDEN_ATTR = 'data-pane-hidden' @@ -28,6 +30,12 @@ export const PaneVisibleContext = createContext(true) export const usePaneVisible = (): boolean => useContext(PaneVisibleContext) +/** Lifecycle face for expensive descendants. Outside a pane tree the surface is + * visible; hot-hidden panes stay mounted but can lower their render budget. */ +export const PaneLifecycleContext = createContext('visible') + +export const usePaneLifecycle = (): PaneLifecycle => useContext(PaneLifecycleContext) + /** Fallback group key for a surface rendered outside the layout tree (secondary * windows, plain routes) — one bucket, since there are no sibling zones there * to tell apart. */ diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/track-model.ts b/apps/desktop/src/components/pane-shell/tree/renderer/track-model.ts index f70b8222ec..f1d27741e2 100644 --- a/apps/desktop/src/components/pane-shell/tree/renderer/track-model.ts +++ b/apps/desktop/src/components/pane-shell/tree/renderer/track-model.ts @@ -59,6 +59,10 @@ interface PaneChrome extends PaneSizing { /** Spawn corner for `placement: 'floating'` (default `'top-right'`). The * pane also TRACKS that corner's edges when the window resizes. */ anchor?: FloatingAnchor + /** Keep this pane mounted when hidden even after the zone's bounded hot + * cache fills. Reserved for stateful resources whose lifetime must not track + * tab visibility (for example terminal PTYs). */ + lifecycleKeepAlive?: boolean /** No Close in the tab menu — the one surface the app can't lose (the * main workspace). Session tiles share `placement: 'main'` but close. */ uncloseable?: boolean diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx b/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx index 3f237d1290..6e7abf05cc 100644 --- a/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx +++ b/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx @@ -10,7 +10,7 @@ */ import { useStore } from '@nanostores/react' -import { type CSSProperties, Fragment, type ReactNode, type RefObject, useEffect, useRef, useState } from 'react' +import { type CSSProperties, Fragment, type ReactNode, type RefObject, useRef, useState } from 'react' import { ActionsContextMenu, type MenuKit, renderActionItem } from '@/components/ui/actions-menu' import { Codicon } from '@/components/ui/codicon' @@ -32,7 +32,8 @@ import { cn } from '@/lib/utils' import { $layoutEditMode } from '../../edit-mode' import { useWindowControlsOverlap } from '../../geometry' -import { hiddenPaneProps, PaneGroupContext, PaneVisibleContext } from '../../pane-visibility' +import { emptyPaneLifecycleState, reconcilePaneLifecycle } from '../../pane-lifecycle' +import { hiddenPaneProps, PaneGroupContext, PaneLifecycleContext, PaneVisibleContext } from '../../pane-visibility' import type { DropPosition, GroupNode } from '../model' import { $dropHint, @@ -212,29 +213,24 @@ export function TreeGroup({ const active = paneFor(activeId) const isEmpty = node.panes.length === 0 - // KEEP-ALIVE: every pane that has been ACTIVE in this zone stays mounted — - // an inactive tab merely hides (visibility), it does not unmount. Remounting - // on every tab switch re-measured and re-scrolled the content from scratch - // (the thread visibly layout-shifted each time a session tab was revisited). - // Lazy on purpose: a pane first mounts when first activated, so a - // boot-restored tab stack doesn't resume every session up front. - const everActivePanesRef = useRef>(new Set()) + // BOUNDED KEEP-ALIVE: the active pane is visible, a small per-zone LRU stays + // hot-hidden, and older panes park (unmount). This preserves fast tab + // round-trips without letting a long-lived zone pin every transcript it has + // ever visited. Stateful resources can opt out of parking (the terminal keeps + // its PTY alive while hidden). Lazy remains deliberate: restored background + // tabs have no lifecycle entry and do not mount until first activation. + const lifecycleRef = useRef(emptyPaneLifecycleState()) - useEffect(() => { - if (!node.minimized && !isEmpty) { - everActivePanesRef.current.add(activeId) - } + if (!node.minimized && !isEmpty) { + lifecycleRef.current = reconcilePaneLifecycle(lifecycleRef.current, { + activeId, + keepAlive: id => Boolean(paneChrome(paneFor(id)).lifecycleKeepAlive), + paneIds: shown + }) + } - // Prune panes that left the zone (closed / moved to another group), so a - // long-lived zone doesn't pin stale ids forever. - for (const id of everActivePanesRef.current) { - if (!node.panes.includes(id)) { - everActivePanesRef.current.delete(id) - } - } - }) - - const keptPanes = shown.filter(id => id === activeId || everActivePanesRef.current.has(id)) + const paneLifecycle = lifecycleRef.current.entries + const keptPanes = shown.filter(id => paneLifecycle[id] && paneLifecycle[id].lifecycle !== 'parked') // ONE header style: the app's compact pane-header. DEFAULT is contextual — // a single pane isn't a "tab", so its header auto-hides; a stack shows its @@ -593,8 +589,8 @@ export function TreeGroup({ )} - {/* Body: the zone's pane content — every kept (ever-active) pane stays - mounted in an absolute layer; only the active one is visible. + {/* Body: the zone's pane content — the active pane and bounded hot-hidden + cache stay mounted in absolute layers; parked panes are unmounted. `visibility` (not display) keeps the hidden pane's layout box, so scroll positions and measurements survive the round-trip — which also makes a hidden layer's rect identical to the visible one's, hence the @@ -627,11 +623,13 @@ export function TreeGroup({ // Reload remounts the contribution (effects re-run, state // resets) while the layer — and every other tab — stays. - - - - - + + + + + + + ) : ( isActive && ( diff --git a/apps/desktop/src/store/session-states-eviction.test.ts b/apps/desktop/src/store/session-states-eviction.test.ts index 8958df05ee..b82aa79756 100644 --- a/apps/desktop/src/store/session-states-eviction.test.ts +++ b/apps/desktop/src/store/session-states-eviction.test.ts @@ -8,8 +8,8 @@ import { $sessionStates, $sessionTiles, closeSessionTile, publishSessionState } * The closed-tile leak: gateway events keep publishing for sessions whose * surface is gone, and every parked transcript taxes every later publish (map * spread + the status projections run per entry per message delta). A settled - * state nothing references must leave the map; everything a surface still - * needs must stay. + * state nothing references must release its transcript; lightweight status + * stays so sidebar projections remain available. */ const state = (storedId: string, patch: Partial> = {}) => ({ @@ -28,13 +28,14 @@ beforeEach(() => { }) describe('publish-time eviction', () => { - it('evicts a settling session no surface references, keeping its unread dot', () => { + it('releases an unreferenced settled transcript while keeping status and its unread dot', () => { publishSessionState('rt-1', state('stored-1', { busy: true })) expect($sessionStates.get()['rt-1']).toBeDefined() publishSessionState('rt-1', state('stored-1', { busy: false })) - expect($sessionStates.get()['rt-1']).toBeUndefined() + expect($sessionStates.get()['rt-1']?.messages).toEqual([]) + expect($sessionStates.get()['rt-1']).toMatchObject({ storedSessionId: 'stored-1', busy: false }) // The settle transition still fired: the sidebar's unread marker landed. expect($unreadFinishedSessionIds.get()).toContain('stored-1') }) @@ -100,8 +101,9 @@ describe('closeSessionTile eviction', () => { expect($sessionStates.get()['rt-1']).toBeDefined() - // ... and its settle publish is what evicts it. + // ... and its settle publish releases only the heavy transcript. publishSessionState('rt-1', state('stored-1', { busy: false })) - expect($sessionStates.get()['rt-1']).toBeUndefined() + expect($sessionStates.get()['rt-1']?.messages).toEqual([]) + expect($sessionStates.get()['rt-1']).toMatchObject({ storedSessionId: 'stored-1', busy: false }) }) }) diff --git a/apps/desktop/src/store/session-states.test.ts b/apps/desktop/src/store/session-states.test.ts index 3bff4ba520..0abc89bfd5 100644 --- a/apps/desktop/src/store/session-states.test.ts +++ b/apps/desktop/src/store/session-states.test.ts @@ -6,16 +6,39 @@ import { $layoutTree } from '@/components/pane-shell/tree/store' import { $selectedStoredSessionId } from '@/store/session' import type { SessionTile } from '@/store/session-states' import { + $sessionStates, blankDraftTile, focusedSessionNeedsRoute, markSelectionRestore, orderTilesByTree, + releaseSessionTranscript, selectionHomesToWorkspace } from '@/store/session-states' const tile = (storedSessionId: string): SessionTile => ({ storedSessionId }) const tilePane = (id: string) => `session-tile:${id}` +describe('releaseSessionTranscript', () => { + afterEach(() => { + $sessionStates.set({}) + }) + + it('normalizes legacy state whose messages field is undefined', () => { + const legacy = { busy: false, storedSessionId: 'stored' } as ClientSessionState + $sessionStates.set({ runtime: legacy }) + + expect(() => releaseSessionTranscript('runtime')).not.toThrow() + expect($sessionStates.get().runtime).toEqual({ ...legacy, messages: [] }) + }) + + it('ignores a legacy undefined state without throwing', () => { + $sessionStates.set({ runtime: undefined } as unknown as Record) + + expect(() => releaseSessionTranscript('runtime')).not.toThrow() + expect($sessionStates.get()).toHaveProperty('runtime', undefined) + }) +}) + describe('orderTilesByTree', () => { it('no-ops (null) without a tree or below two tiles', () => { expect(orderTilesByTree(null, [tile('a'), tile('b')])).toBeNull() diff --git a/apps/desktop/src/store/session-states.ts b/apps/desktop/src/store/session-states.ts index 1064b90d7a..79653baaad 100644 --- a/apps/desktop/src/store/session-states.ts +++ b/apps/desktop/src/store/session-states.ts @@ -228,15 +228,13 @@ function evictable(runtimeId: string, state: ClientSessionState): boolean { * is updated independently by the caller, so the visual path stays live * without the store churn. * - * A settled state nothing references is EVICTED instead of republished: - * gateway events keep flowing for sessions whose tile was closed mid-turn, - * and parking each one's full transcript here forever is the leak that made - * the app crawl after a day of tile use — every entry taxes every later - * publish (map spread + the status-set projections). Transition side effects - * still fire, so the closed session's settle keeps its unread dot. Only an - * entry already in the map is evicted — a FIRST publish always lands, because - * a resume can publish its idle state a beat before `$activeSessionId` / - * the tile's runtime binding points at it. */ + * A settled state nothing references releases its transcript instead of + * republishing it. Gateway events keep flowing for sessions whose tile was + * closed mid-turn, and parking each one's full transcript here forever is the + * leak that made the app crawl after a day of tile use. Transition side + * effects still fire, so lightweight status and the unread dot survive. A + * FIRST publish always lands in full because a resume can publish its idle + * state a beat before `$activeSessionId` / the tile binding points at it. */ export function publishSessionState(runtimeId: string, state: ClientSessionState) { const current = $sessionStates.get() const prev = current[runtimeId] ?? null @@ -247,8 +245,7 @@ export function publishSessionState(runtimeId: string, state: ClientSessionState if (prev && evictable(runtimeId, state)) { handleTransition(prev, state, runtimeId) - const { [runtimeId]: _dropped, ...rest } = current - $sessionStates.set(rest) + releaseSessionTranscript(runtimeId, state) return } @@ -257,6 +254,30 @@ export function publishSessionState(runtimeId: string, state: ClientSessionState handleTransition(prev, state, runtimeId) } +/** Keep the cheap status projection for a cold session while releasing its + * transcript. Unread completion is stored separately, so it survives too. */ +export function releaseSessionTranscript(runtimeId: string, state?: ClientSessionState) { + const current = $sessionStates.get() + + if (!(runtimeId in current)) { + return + } + + const retained = state ?? current[runtimeId] + + // Older persisted snapshots can contain an undefined state or omit the + // messages field. Treat either shape as already cold instead of throwing + // while memory pressure is being relieved. + if (!retained) { + return + } + + const lightweight = + Array.isArray(retained.messages) && retained.messages.length === 0 ? retained : { ...retained, messages: [] } + + $sessionStates.set({ ...current, [runtimeId]: lightweight }) +} + export function dropSessionState(runtimeId: string) { // Disarm the watchdog — a dropped runtime must not fire a stale clear later. // Settle-grace entries are keyed by stored id and self-expire; leave them so diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 7a42a601b4..e64e06d30d 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -137,6 +137,94 @@ except ImportError: WEB_DIST = Path(os.environ["HERMES_WEB_DIST"]) if "HERMES_WEB_DIST" in os.environ else Path(__file__).parent / "web_dist" _log = logging.getLogger(__name__) + +def _process_start_marker(pid: int) -> str: + """Return a cross-runtime marker for the current incarnation of ``pid``. + + ``ProcessLookupError`` means the process is absent. Other failures are left + distinct so callers can fail safe rather than killing a healthy backend. + """ + if sys.platform == "linux": + try: + stat_line = Path(f"/proc/{pid}/stat").read_text(encoding="utf-8") + except FileNotFoundError as exc: + raise ProcessLookupError(pid) from exc + + # The command in field 2 may contain spaces or parentheses. Splitting + # after its final ')' leaves field 3 at index zero and field 22 at 19. + fields = stat_line.rsplit(")", 1)[1].strip().split() + if len(fields) < 20 or not fields[19].isdigit(): + raise OSError(f"invalid /proc stat data for PID {pid}") + return f"linux:{fields[19]}" + + if os.name == "nt": + import ctypes + from ctypes import wintypes + + process_query_limited_information = 0x1000 + kernel32 = ctypes.WinDLL("kernel32", use_last_error=True) + kernel32.OpenProcess.argtypes = [wintypes.DWORD, wintypes.BOOL, wintypes.DWORD] + kernel32.OpenProcess.restype = wintypes.HANDLE + kernel32.GetProcessTimes.argtypes = [ + wintypes.HANDLE, + ctypes.POINTER(wintypes.FILETIME), + ctypes.POINTER(wintypes.FILETIME), + ctypes.POINTER(wintypes.FILETIME), + ctypes.POINTER(wintypes.FILETIME), + ] + kernel32.GetProcessTimes.restype = wintypes.BOOL + kernel32.CloseHandle.argtypes = [wintypes.HANDLE] + kernel32.CloseHandle.restype = wintypes.BOOL + handle = kernel32.OpenProcess(process_query_limited_information, False, pid) + if not handle: + error = ctypes.get_last_error() + if error in (87, 1168): # invalid parameter / not found + raise ProcessLookupError(pid) + raise OSError(error, f"OpenProcess failed for PID {pid}") + + creation = wintypes.FILETIME() + exit_time = wintypes.FILETIME() + kernel = wintypes.FILETIME() + user = wintypes.FILETIME() + try: + if not kernel32.GetProcessTimes( + handle, + ctypes.byref(creation), + ctypes.byref(exit_time), + ctypes.byref(kernel), + ctypes.byref(user), + ): + error = ctypes.get_last_error() + raise OSError(error, f"GetProcessTimes failed for PID {pid}") + finally: + kernel32.CloseHandle(handle) + + filetime = (creation.dwHighDateTime << 32) | creation.dwLowDateTime + return f"win:{filetime + 504911232000000000}" + + result = subprocess.run( + ["ps", "-p", str(pid), "-o", "lstart="], + capture_output=True, + text=True, + check=False, + ) + marker = result.stdout.strip() + if result.returncode == 0 and marker: + return f"ps:{marker}" + if result.returncode == 1 and not marker: + raise ProcessLookupError(pid) + raise OSError(f"ps could not inspect PID {pid}: {result.stderr.strip()}") + + +def _valid_parent_start_marker(marker: str) -> bool: + prefix, separator, value = marker.partition(":") + if not separator or not value or value != value.strip(): + return False + if prefix in ("linux", "win"): + return value.isdigit() + return prefix == "ps" + + # --------------------------------------------------------------------------- # Per-channel subscriber registry used by /api/pub (PTY-side gateway → dashboard) # and /api/events (dashboard → browser sidebar). Keyed by an opaque channel id @@ -17901,53 +17989,80 @@ def _maybe_open_browser( threading.Thread(target=_open, daemon=True).start() -def _is_serve_orphaned(desktop_pid: int, pid_exists=None) -> bool: - """True when the Desktop process that owns this serve backend is gone. +def _is_serve_orphaned( + desktop_pid: int, + expected_start_marker: Optional[str] = None, + *, + pid_exists=None, + process_start_marker=None, +) -> bool: + """True when the exact Desktop process that owns this backend is gone. ``HERMES_PARENT_PID`` is the Electron Desktop PID, not necessarily this Python process's immediate PPID. On Windows the venv ``hermes.exe`` launcher introduces one or more shim processes, so comparing ``os.getppid()`` to the Electron PID incorrectly treats a healthy backend as orphaned and exits 0. - Probe the recorded Desktop PID directly instead. - Any liveness-probe failure is fail-safe: keep serving rather than killing a - backend whose owner could not be conclusively shown to be dead. + New Desktop versions also provide the owner's process-start marker. This + prevents a recycled PID from keeping an orphan alive. Older versions remain + compatible through the PID-only probe. Any inconclusive probe failure is + fail-safe: keep serving rather than killing a backend whose owner could not + be conclusively shown to be dead. """ try: + if expected_start_marker is not None: + probe = process_start_marker or _process_start_marker + return probe(int(desktop_pid)) != expected_start_marker + if pid_exists is None: from gateway.status import _pid_exists pid_exists = _pid_exists return not bool(pid_exists(int(desktop_pid))) + except ProcessLookupError: + return True except Exception: return False def _start_parent_death_watchdog() -> None: - """Exit when the desktop parent that spawned this backend dies. + """Exit when the exact desktop parent that spawned this backend dies. - The desktop passes its own PID via HERMES_PARENT_PID. When that process - vanishes (crash, SIGKILL, update handoff exiting before it reaps us) this - orphaned backend would otherwise keep serving forever and leak its MCP - child subtree. os._exit propagates to the MCP watchdogs parented here. - - No-op for standalone `hermes serve` (env unset). Poll interval tunable via - HERMES_SERVE_WATCHDOG_POLL_S. + The desktop passes its PID and, in newer versions, its process-start marker + plus a per-spawn nonce. The marker distinguishes a live owner from PID reuse; + the nonce makes partial/mixed-version identity plumbing fail safe. Legacy + Desktop versions that provide only ``HERMES_PARENT_PID`` retain PID-only + tracking. """ - raw = os.environ.get("HERMES_PARENT_PID") - if not raw: - return + raw_pid = os.environ.get("HERMES_PARENT_PID") + start_marker = os.environ.get("HERMES_PARENT_START_MARKER") + nonce = os.environ.get("HERMES_PARENT_NONCE") + try: - desktop_pid = int(raw) + desktop_pid = int(raw_pid or "") except (TypeError, ValueError): return + if desktop_pid <= 0: + return + + has_marker = start_marker is not None + has_nonce = nonce is not None + if has_marker != has_nonce: + return + if has_marker and ( + not _valid_parent_start_marker(start_marker or "") + or not nonce + or nonce != nonce.strip() + ): + return + try: poll = max(0.5, float(os.environ.get("HERMES_SERVE_WATCHDOG_POLL_S", "2.0"))) except (TypeError, ValueError): poll = 2.0 def _loop() -> None: - while not _is_serve_orphaned(desktop_pid): + while not _is_serve_orphaned(desktop_pid, start_marker): time.sleep(poll) os._exit(0)