diff --git a/src/main/orca-chromium-process-pids.ts b/src/main/orca-chromium-process-pids.ts index f22babc6921..b2613e42b79 100644 --- a/src/main/orca-chromium-process-pids.ts +++ b/src/main/orca-chromium-process-pids.ts @@ -1,4 +1,5 @@ import { getAppEnvironment, hasAppEnvironment } from '../shared/app-environment' +import { recordCoalescedDurableCrashBreadcrumb } from './crash-reporting/durable-crash-breadcrumb' /** * PIDs of Orca's own Chromium processes — browser, renderers, GPU, utilities. @@ -11,6 +12,14 @@ import { getAppEnvironment, hasAppEnvironment } from '../shared/app-environment' * Empty on a Node host and empty on failure: that is "no refusal proven", never * "safe to kill" — callers must keep every other guard they already have. * + * Why failure stays open rather than refusing everything: a refusal is not free. + * `terminateWindowsProcessTree` resolves without killing, and + * `killSourceControlAgentProcess` returns that straight to a caller that then + * releases the managed-home lock, so failing closed would trade one unreadable + * metrics table for every PTY, git, codex and notebook tree in main leaking at + * once. The `own_chromium_pids_unreadable` crumb is the price of that choice: + * without it a throw is byte-identical to "no Chromium on this host". + * * Host coverage: only Electron main installs a Chromium-backed AppEnvironment * (main-process-preflight). The standalone daemon installs none and `orcad` * installs a Node one whose `getAppMetrics()` is `[]`, so this set is empty in @@ -30,7 +39,26 @@ export function readOrcaChromiumProcessPids(): ReadonlySet { .map((metric) => metric.pid) .filter((pid) => Number.isInteger(pid) && pid > 0) return new Set(pids) - } catch { + } catch (error) { + recordUnreadableOwnChromiumMetrics(error) return new Set() } } + +// Why coalesced: the gate reads this set on every tree kill, so a persistently +// broken metrics table would otherwise flood the 30-slot ring it shares. +const UNREADABLE_METRICS_COALESCE_MS = 60_000 + +function recordUnreadableOwnChromiumMetrics(error: unknown): void { + try { + recordCoalescedDurableCrashBreadcrumb({ + name: 'own_chromium_pids_unreadable', + data: { cause: error instanceof Error ? error.message : String(error) }, + coalesceKey: 'own-chromium-pids-unreadable', + minIntervalMs: UNREADABLE_METRICS_COALESCE_MS + }) + } catch { + // Diagnostics must never turn an admitted kill into a thrown one: callers + // read this set outside their own try. + } +} diff --git a/src/main/own-chromium-tree-kill-guard.test.ts b/src/main/own-chromium-tree-kill-guard.test.ts index 7e98661aca6..bd3b1674e18 100644 --- a/src/main/own-chromium-tree-kill-guard.test.ts +++ b/src/main/own-chromium-tree-kill-guard.test.ts @@ -143,6 +143,40 @@ describe('refusing to tree-kill our own Chromium processes', () => { ) }) + /** + * Fail-open is the deliberate choice — see `orca-chromium-process-pids.ts` for + * why refusing everything is worse — so the crumb is the only thing that keeps + * an unreadable metrics table distinguishable from a host that has no Chromium. + */ + it('leaves proof, and still admits the kill, when the Chromium metrics cannot be read', () => { + appMetricsMock.mockImplementation(() => { + throw new Error('getAppMetrics unavailable') + }) + + expect([...readOrcaChromiumProcessPids()]).toEqual([]) + // Coalesced: the gate reads this set on every kill, so a broken table must + // not evict the ring it shares with the refusal crumb. + expect([...readOrcaChromiumProcessPids()]).toEqual([]) + expect( + admitSelfInitiatedTreeKill({ + pid: RENDERER_PID, + site: 'pty-descendant-sweep', + scope: 'win-taskkill-tree' + }) + ).toBe(true) + + expect( + getCrashBreadcrumbSnapshot().filter( + (breadcrumb) => breadcrumb.name === 'own_chromium_pids_unreadable' + ) + ).toEqual([ + expect.objectContaining({ + name: 'own_chromium_pids_unreadable', + data: expect.objectContaining({ cause: 'getAppMetrics unavailable' }) + }) + ]) + }) + it('refuses an own-Chromium pid at the gate the account teardowns share', () => { expect( admitSelfInitiatedTreeKill({