diff --git a/src/relay/pty-handler-spawn-admission.test.ts b/src/relay/pty-handler-spawn-admission.test.ts index c25a623f406..7ba02f78587 100644 --- a/src/relay/pty-handler-spawn-admission.test.ts +++ b/src/relay/pty-handler-spawn-admission.test.ts @@ -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' diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index 42f8bdc0a77..18c79bc7f5d 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -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 =