fix(crash-reporting): leave proof when the own-Chromium pid set cannot be read

`readOrcaChromiumProcessPids` returns an empty set when `getAppMetrics()`
throws, which is the right decision — refusing every kill would orphan every
PTY, git, codex and notebook tree main tears down, and on main a refusal from
`killSourceControlAgentProcess` releases the managed-home lock with the agent
still alive. But the empty set was byte-identical to "no Chromium on this
host", so the fail-open was invisible in a field bundle.

Keeps the decision, adds a coalesced durable `own_chromium_pids_unreadable`
crumb so the two cases are distinguishable. Coalesced because the gate reads
this set on every tree kill.
This commit is contained in:
Neil
2026-09-03 22:14:50 -07:00
parent 1ff4a3d30d
commit 43450231dd
2 changed files with 63 additions and 1 deletions
+29 -1
View File
@@ -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<number> {
.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.
}
}
@@ -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({