Keep the compatibility fields off the capture the budget just abandoned

inspectProcess falls back to processHasChildren and listProcesses to
getForegroundProcessName, and both read the same TTL-shared capture with
no budget of their own. On a slow host they joined the in-flight capture
the budgeted evidence read had just given up on, so the call still blocked
for the full 6-18s and the budget bought nothing -- once for inspectProcess
and once per managed PTY for listProcesses.

Use the degraded answers those helpers already give for an unreadable
table, reached promptly. pty.hasChildProcesses keeps its unbudgeted fresh
probe: it is a one-shot destructive gate that can afford to wait.
This commit is contained in:
Merge Sim
2026-09-04 13:56:34 -07:00
parent a3e07a88b1
commit 70ba3eca43
2 changed files with 59 additions and 2 deletions
@@ -125,6 +125,44 @@ describe('PtyHandler', () => {
expect(hasChildren).toHaveBeenLastCalledWith(mockPtyInstance.pid, { fresh: true })
})
it('does not re-enter the shared capture after the evidence read gave up on it', async () => {
// The budget is worthless if the compatibility fields answer by joining the very capture the
// evidence read just abandoned: `processHasChildren` and `getForegroundProcessName` read the
// same TTL-shared table with no budget of their own, so on a slow host this call would still
// block for the whole capture -- once, and then once per managed PTY in the listing.
const snapshot = vi
.spyOn(processTableSnapshotReader, 'getStrictProcessTableSnapshotWithAge')
.mockRejectedValue(new Error('process table unreadable: capture_over_budget'))
const hasChildren = vi.spyOn(ptyShellUtils, 'processHasChildren')
const foregroundName = vi.spyOn(ptyShellUtils, 'getForegroundProcessName')
const { id } = (await spawnPty({ cols: 80, rows: 24 })) as { id: string }
hasChildren.mockClear()
foregroundName.mockClear()
const inspection = (await dispatcher.callRequest('pty.inspectProcess', { id })) as {
hasChildProcesses: boolean
foregroundProcessEvidence?: { verdict: string; reason?: string }
}
expect(snapshot).toHaveBeenCalled()
expect(hasChildren).not.toHaveBeenCalled()
// The verdict the gates already handle, reached promptly instead of late.
expect(inspection.foregroundProcessEvidence?.verdict).toBe('unverifiable')
expect(inspection.foregroundProcessEvidence?.reason).toBe('process_table_unreadable')
// `false` is what the child probe itself answers for an unreadable table; this is that same
// degraded answer without the wait, not a new claim about the pane.
expect(inspection.hasChildProcesses).toBe(false)
const listing = (await dispatcher.callRequest('pty.listProcesses', {})) as {
id: string
title: string
}[]
expect(foregroundName).not.toHaveBeenCalled()
expect(listing.find((entry) => entry.id === id)?.title).toBeTruthy()
})
it('rejects strict process inspection for a missing relay PTY', async () => {
await expect(dispatcher.callRequest('pty.inspectProcess', { id: 'missing' })).rejects.toThrow(
'terminal_gone'
+21 -2
View File
@@ -2661,6 +2661,9 @@ export class PtyHandler {
}
}
let rows: readonly ProcessTableRow[] | null = null
// Set only when the budgeted evidence read gave up, so the compatibility fields below do not
// turn around and ask the same unreadable table again with no budget at all.
let tableUnavailable = false
let evidence: RemoteForegroundEvidence | undefined
if (process.platform === 'win32') {
// Why SSH-to-Windows is always unverifiable: POSIX has a real foreground primitive
@@ -2699,6 +2702,7 @@ export class PtyHandler {
rows
)
} catch {
tableUnavailable = true
evidence = {
authorityGeneration: this.ptyIdMintEpoch,
observationEpoch: ++this.foregroundEvidenceEpoch,
@@ -2721,9 +2725,19 @@ export class PtyHandler {
// Derive child liveness from the same capture; do not fork a second
// process-table probe for each field/pane in an event burst. Windows
// has no evidence capture, so preserve the compatibility child probe.
//
// Why the middle branch: `processHasChildren` reads the same shared capture with no budget
// of its own, so on a host slow enough to blow the evidence budget it would join the very
// capture this call just abandoned and block for all of it -- spending the whole latency
// the budget exists to avoid, on a compatibility field. `false` is what that helper already
// answers for an unreadable table, so this is the existing degraded answer reached promptly
// rather than a new one. The destructive gate is the separate `pty.hasChildProcesses` RPC,
// which keeps its unbudgeted fresh probe.
hasChildProcesses: rows
? rows.some((row) => row.ppid === managed.pty.pid)
: await processHasChildren(managed.pty.pid),
: tableUnavailable
? false
: await processHasChildren(managed.pty.pid),
...(evidence ? { foregroundProcessEvidence: evidence } : {})
}
}
@@ -2742,6 +2756,10 @@ export class PtyHandler {
// process-table work on the host.
const includeForegroundProcessEvidence = params.includeForegroundProcessEvidence !== false
let evidenceRows: readonly ProcessTableRow[] | null = null
// Same reason as `inspectProcess`: once the budgeted read has given up, the per-PTY title
// fallback below must not re-enter the same capture without a budget -- and here it would do
// so once per managed PTY.
let evidenceTableUnavailable = false
let evidenceResults: BatchedForegroundProcessResult[] = []
const evidenceEpoch = ++this.foregroundEvidenceEpoch
// Worst-case capture time for the snapshot below, not the instant its await settled: the
@@ -2767,6 +2785,7 @@ export class PtyHandler {
} catch {
// An unreadable capture is represented as unverifiable evidence below;
// existing inventory fields remain available for old clients.
evidenceTableUnavailable = true
}
}
for (const [entryIndex, [id, managed]] of managedEntries.entries()) {
@@ -2781,7 +2800,7 @@ export class PtyHandler {
const title =
(evidenceRows
? (evidenceResults[entryIndex]?.processName ?? managed.pty.process ?? null)
: includeForegroundProcessEvidence
: includeForegroundProcessEvidence && !evidenceTableUnavailable
? await getForegroundProcessName(managed.pty.pid, managed.pty.process || null)
: managed.pty.process || null) || 'shell'
const foregroundProcessEvidence =