From 704167197a9660dfef8dd34c8ef591f2481b5bee Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 31 Aug 2026 17:45:55 -0700 Subject: [PATCH] perf(relay): serve one ps capture per window and pin the batched inventory path (#17763) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up defect fixes for the batched PTY-inventory evidence path (#17525), now on main. - One memoized `ps` capture serves both the lenient and strict views. The two readers ran byte-identical argv behind separate caches, so a relay serving both forked `ps` twice per 500ms window — the doubling issue #6288 removed. - Drop the `byPgid`/`byTpgid` indexes no resolver reads, plus the zero-caller `parseProcessTableRowsStrict` and `getFreshStrictProcessTableSnapshot`; the batch resolver now reuses the shared index lookup and candidate score instead of private copies. - Restore `getForegroundProcessName`'s ladder contract: the extracted table scan answers null again, so an unconfirmed wrapper fallback publishes the recognized (normalized) name rather than node-pty's raw one. - Pin the SHIPPED `pty.listProcesses` path: one capture and one linear row pass for N panes, and node-pty's own name (never "shell") when the capture cannot disambiguate a `node`/`python` wrapper. - Pin the hidden-pane cadence gate in the production option shape, and move the strict-parser coverage next to the parser it tests. --- .../agent-foreground-process-batch.test.ts | 39 +--- .../agent-foreground-process-batch.ts | 25 +-- ...handler-inventory-process-evidence.test.ts | 178 ++++++++++++++++++ src/relay/pty-shell-utils.test.ts | 15 ++ src/relay/pty-shell-utils.ts | 28 ++- .../agent-completion-coordinator-types.ts | 8 +- ...ent-completion-no-evidence-cadence.test.ts | 19 ++ src/shared/process-table-snapshot.test.ts | 109 ++++++++++- src/shared/process-table-snapshot.ts | 104 +++++----- 9 files changed, 404 insertions(+), 121 deletions(-) create mode 100644 src/relay/pty-handler-inventory-process-evidence.test.ts diff --git a/src/main/providers/agent-foreground-process-batch.test.ts b/src/main/providers/agent-foreground-process-batch.test.ts index 593754062b3..784a62c3376 100644 --- a/src/main/providers/agent-foreground-process-batch.test.ts +++ b/src/main/providers/agent-foreground-process-batch.test.ts @@ -2,49 +2,12 @@ import { describe, expect, it } from 'vitest' import { buildProcessTableIndex, parseStrictProcessTableRows, - ProcessTableCaptureError, type ProcessTableIndexStats } from '../../shared/process-table-snapshot' import { resolveAgentForegroundProcessesBatch, resolveAgentForegroundProcessesFromIndex -} from './agent-foreground-process' - -describe('strict process-table evidence parser', () => { - it('extracts pgid/tpgid while retaining command spacing', () => { - expect( - parseStrictProcessTableRows( - ' PID PPID PGID TPGID STAT COMMAND\r\n 100 1 100 101 Ss /bin/zsh -l\r\n 101 100 101 101 S+ node /opt/codex --flag value\r\n' - ) - ).toEqual([ - { pid: 100, ppid: 1, pgid: 100, tpgid: 101, stat: 'Ss', command: '/bin/zsh -l' }, - { - pid: 101, - ppid: 100, - pgid: 101, - tpgid: 101, - stat: 'S+', - command: 'node /opt/codex --flag value' - } - ]) - }) - - it.each(['101 100 101 S+ node /opt/codex', '101 100 -2 101 S+ node /opt/codex'])( - 'rejects malformed/truncated captures (%s)', - (capture) => { - expect(() => parseStrictProcessTableRows(capture)).toThrow(ProcessTableCaptureError) - } - ) - - it('accepts no-controlling-tty sentinels for later unverifiable classification', () => { - expect(parseStrictProcessTableRows('100 1 100 0 Ss /bin/zsh')).toEqual([ - { pid: 100, ppid: 1, pgid: 100, tpgid: 0, stat: 'Ss', command: '/bin/zsh' } - ]) - expect(parseStrictProcessTableRows('100 1 100 -1 Ss /bin/zsh')).toEqual([ - { pid: 100, ppid: 1, pgid: 100, tpgid: -1, stat: 'Ss', command: '/bin/zsh' } - ]) - }) -}) +} from './agent-foreground-process-batch' describe('batched foreground process correlation', () => { it('uses tpgid/pgid association instead of stat alone', () => { diff --git a/src/main/providers/agent-foreground-process-batch.ts b/src/main/providers/agent-foreground-process-batch.ts index 093ef003ddc..ea89005d87f 100644 --- a/src/main/providers/agent-foreground-process-batch.ts +++ b/src/main/providers/agent-foreground-process-batch.ts @@ -9,6 +9,8 @@ import type { ForegroundProcessEvidence } from '../../shared/foreground-process- import { buildProcessTableIndex, getStrictProcessTableSnapshot, + lookupProcessTableIndex, + scoreForegroundCandidateRow, type ProcessTableIndex, type ProcessTableIndexStats, type ProcessTableRow @@ -59,7 +61,7 @@ export function resolveAgentForegroundProcessesFromIndex( const rowsByOwner = new Map() const queue: { row: ProcessTableRow; owner: number; depth: number }[] = [] for (const rootPid of uniqueRoots) { - const root = lookupIndex(index, (value) => value.byPid.get(rootPid)) + const root = lookupProcessTableIndex(index, (value) => value.byPid.get(rootPid)) if (root) { depthByPid.set(root.pid, 0) queue.push({ row: root, owner: root.pid, depth: 0 }) @@ -72,7 +74,10 @@ export function resolveAgentForegroundProcessesFromIndex( owned.push({ ...current.row, depth: current.depth }) } rowsByOwner.set(current.owner, owned) - const children = lookupIndex(index, (value) => value.childrenByPpid.get(current.row.pid) ?? []) + const children = lookupProcessTableIndex( + index, + (value) => value.childrenByPpid.get(current.row.pid) ?? [] + ) for (const child of children) { const childOwner = rootsByPid.has(child.pid) ? child.pid : current.owner const childDepth = rootsByPid.has(child.pid) ? 0 : current.depth + 1 @@ -86,7 +91,7 @@ export function resolveAgentForegroundProcessesFromIndex( } return requests.map((request) => { - const root = lookupIndex(index, (value) => value.byPid.get(request.rootPid)) + const root = lookupProcessTableIndex(index, (value) => value.byPid.get(request.rootPid)) if (!root) { return { available: false, @@ -127,7 +132,8 @@ export function resolveAgentForegroundProcessesFromIndex( const recognized = recognizeAgentProcessFromCommandLine(candidate.command) if ( recognized && - (bestCandidate === null || candidateScore(candidate) > candidateScore(bestCandidate)) + (bestCandidate === null || + scoreForegroundCandidateRow(candidate) > scoreForegroundCandidateRow(bestCandidate)) ) { bestCandidate = candidate bestName = recognized @@ -143,17 +149,6 @@ export function resolveAgentForegroundProcessesFromIndex( }) } -function lookupIndex(index: ProcessTableIndex, lookup: (value: ProcessTableIndex) => T): T { - if (index.stats) { - index.stats.indexLookups += 1 - } - return lookup(index) -} - -function candidateScore(row: ProcessTableRow & { depth: number }): number { - return (row.stat.includes('+') ? 10_000 : 0) + row.depth -} - export function toForegroundProcessEvidence( result: BatchedForegroundProcessResult, metadata: { authorityGeneration: string; observationEpoch: number; capturedAgeMs: number } diff --git a/src/relay/pty-handler-inventory-process-evidence.test.ts b/src/relay/pty-handler-inventory-process-evidence.test.ts new file mode 100644 index 00000000000..17896f67c72 --- /dev/null +++ b/src/relay/pty-handler-inventory-process-evidence.test.ts @@ -0,0 +1,178 @@ +// Regression guard for the SHIPPED inventory path. `pty.listProcesses` resolves +// every managed pane's title from one batched host capture; a per-pane tree walk +// would restore the O(panes x rows) scan on the relay's single event-loop thread, +// and a batched result that cannot name the foreground process must fall back to +// node-pty's own name rather than relabelling a live pane "shell". +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest' + +const { + mockPtySpawn, + mockPtyInstance, + mockCreateShellPromptReadinessProbe, + mockGetStrictProcessTableSnapshot +} = vi.hoisted(() => ({ + mockPtySpawn: vi.fn(), + mockCreateShellPromptReadinessProbe: vi.fn(), + mockGetStrictProcessTableSnapshot: vi.fn(), + mockPtyInstance: { + pid: process.pid, + process: 'zsh', + onData: vi.fn(), + onExit: vi.fn(), + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn(), + clear: vi.fn(), + pause: vi.fn(), + resume: vi.fn() + } +})) + +vi.mock('node-pty', () => ({ + spawn: mockPtySpawn +})) + +vi.mock('../main/pty/posix-pty-process-groups', () => ({ + forceKillPosixPtyProcessGroups: vi.fn((_pid: number, fallback: () => void) => fallback()) +})) + +vi.mock('../main/shell-prompt-readiness-probe', () => ({ + createShellPromptReadinessProbe: mockCreateShellPromptReadinessProbe +})) + +vi.mock('../shared/process-table-snapshot', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, getStrictProcessTableSnapshot: mockGetStrictProcessTableSnapshot } +}) + +import type * as processTableSnapshotModule from '../shared/process-table-snapshot' +import type { ProcessTableRow } from '../shared/process-table-snapshot' + +type ProcessTableSnapshotModule = typeof processTableSnapshotModule +import * as ptyShellUtils from './pty-shell-utils' +import type { PtyHandler } from './pty-handler' +import { + beginPtyHandlerTest, + createPtyRequestHelpers, + endPtyHandlerTest +} from './pty-handler-test-harness' +import type { MockDispatcher } from './pty-handler-test-harness' + +type ProcessSummary = { id: string; title: string } + +/** A shell root plus its foreground children, as `ps` reports them. */ +function paneRows(rootPid: number, commands: string[]): ProcessTableRow[] { + const foregroundPgid = rootPid + 1 + return [ + { + pid: rootPid, + ppid: 1, + pgid: rootPid, + tpgid: foregroundPgid, + stat: 'Ss', + command: '/bin/zsh' + }, + ...commands.map((command, index) => ({ + pid: foregroundPgid + index, + ppid: index === 0 ? rootPid : foregroundPgid + index - 1, + pgid: foregroundPgid, + tpgid: foregroundPgid, + stat: 'S+', + command + })) + ] +} + +/** Counts element reads so a per-pane rescan of the table cannot pass unseen. */ +function countingRows(rows: ProcessTableRow[]): { + rows: readonly ProcessTableRow[] + reads: () => number +} { + let reads = 0 + const proxy = new Proxy(rows, { + get(target, key, receiver) { + if (typeof key === 'string' && /^\d+$/.test(key)) { + reads += 1 + } + return Reflect.get(target, key, receiver) + } + }) + return { rows: proxy, reads: () => reads } +} + +describe('PtyHandler inventory foreground evidence', () => { + let dispatcher: MockDispatcher + let handler: PtyHandler + let originalPlatform: PropertyDescriptor | undefined + + const { spawnPty } = createPtyRequestHelpers(() => dispatcher) + + async function spawnPane(pid: number, processName: string): Promise { + mockPtySpawn.mockReturnValue({ + ...mockPtyInstance, + pid, + process: processName, + onData: vi.fn(), + onExit: vi.fn(), + kill: vi.fn() + }) + return (await spawnPty()).id + } + + async function listProcesses(): Promise { + return (await dispatcher.callRequest('pty.listProcesses', {})) as ProcessSummary[] + } + + beforeEach(() => { + ;({ dispatcher, handler, originalPlatform } = beginPtyHandlerTest({ + mockPtySpawn, + mockPtyInstance, + mockCreateShellPromptReadinessProbe + })) + mockGetStrictProcessTableSnapshot.mockReset() + vi.spyOn(ptyShellUtils, 'isProcessAlive').mockReturnValue(true) + }) + + afterEach(async () => { + await endPtyHandlerTest(handler, originalPlatform) + }) + + it('names each pane from the batched capture', async () => { + const rows = [...paneRows(1000, ['node /opt/codex']), ...paneRows(2000, ['vim notes.txt'])] + mockGetStrictProcessTableSnapshot.mockResolvedValue(rows) + await spawnPane(1000, 'zsh') + await spawnPane(2000, 'vim') + + expect((await listProcesses()).map((entry) => entry.title)).toEqual(['codex', 'vim']) + }) + + it('keeps the node-pty name when the capture cannot disambiguate a wrapper', async () => { + // Two same-group `node` children (dev server + worker): the batch refuses to + // guess, and the pane must stay "node" rather than being relabelled a shell. + mockGetStrictProcessTableSnapshot.mockResolvedValue( + paneRows(3000, ['node /srv/app/server.js', 'node /srv/app/worker.js']) + ) + await spawnPane(3000, 'node') + + expect((await listProcesses())[0].title).toBe('node') + }) + + it.each([1, 8])('visits the host table exactly once for %s panes', async (paneCount) => { + const table = Array.from({ length: paneCount }, (_, index) => + paneRows(10_000 + index * 10, ['node /opt/codex']) + ).flat() + const { rows, reads } = countingRows(table) + mockGetStrictProcessTableSnapshot.mockResolvedValue(rows) + for (let index = 0; index < paneCount; index += 1) { + await spawnPane(10_000 + index * 10, 'zsh') + } + + const listed = await listProcesses() + + expect(listed).toHaveLength(paneCount) + expect(listed.every((entry) => entry.title === 'codex')).toBe(true) + expect(mockGetStrictProcessTableSnapshot).toHaveBeenCalledTimes(1) + // One linear index pass — NOT one full-table walk per pane. + expect(reads()).toBe(table.length) + }) +}) diff --git a/src/relay/pty-shell-utils.test.ts b/src/relay/pty-shell-utils.test.ts index ac913e689f0..113b834c3bb 100644 --- a/src/relay/pty-shell-utils.test.ts +++ b/src/relay/pty-shell-utils.test.ts @@ -457,6 +457,21 @@ describe('getForegroundProcessName', () => { }) }) + it('normalizes a wrapper fallback the process table cannot confirm', async () => { + // Why: the table scan must answer null, not the raw node-pty name, so the + // ladder still publishes the RECOGNIZED (normalized) identity. + await withProcessPlatform('linux', async () => { + mockExecFile((_command, args) => { + if (args[0] === '-axo') { + return { stdout: ['100 99 Ss bash -l', '101 100 S+ vim notes.txt'].join('\n') } + } + return new Error('unexpected command') + }) + + await expect(getForegroundProcessName(100, '/opt/homebrew/bin/pi')).resolves.toBe('pi') + }) + }) + it('falls back to the root process command when descendant inspection fails', async () => { mockExecFile((_command, args) => { if (args[0] === '-axo') { diff --git a/src/relay/pty-shell-utils.ts b/src/relay/pty-shell-utils.ts index 04fd9287c0e..3a8218f164f 100644 --- a/src/relay/pty-shell-utils.ts +++ b/src/relay/pty-shell-utils.ts @@ -10,7 +10,11 @@ import { recognizeAgentProcessFromCommandLine } from '../shared/agent-process-recognition' import { getFirstCommandToken } from '../shared/command-token-scanner' -import { getProcessTableSnapshot, type ProcessTableRow } from '../shared/process-table-snapshot' +import { + getProcessTableSnapshot, + scoreForegroundCandidateRow, + type ProcessTableRow +} from '../shared/process-table-snapshot' import { resolveOuterWrapperForegroundProcess, shouldInspectOuterWrapperForegroundProcess @@ -216,17 +220,6 @@ function collectDescendants( return descendants } -function candidateScore(row: ProcessTableRow & { depth: number }): number { - return (row.stat.includes('+') ? 10_000 : 0) + row.depth -} - -function candidateMatchesFallbackWrapper( - candidate: ProcessTableRow, - fallbackProcess: string -): boolean { - return isExpectedAgentProcess(getFirstCommandToken(candidate.command), fallbackProcess) -} - async function getRecognizedForegroundDescendant( pid: number, fallbackProcess?: string | null @@ -240,14 +233,17 @@ async function getRecognizedForegroundDescendant( return null } -export function getForegroundProcessNameFromProcessTable( +// Why: returns null (never the fallback) so `getForegroundProcessName` keeps +// owning the fallback ladder — its wrapper branch answers with the RECOGNIZED +// process name, which is normalized where node-pty's raw name is not. +function getForegroundProcessNameFromProcessTable( rows: ProcessTableRow[], pid: number, fallbackProcess?: string | null ): string | null { const root = rows.find((row) => row.pid === pid) const candidates = collectDescendants(rows, pid).sort( - (a, b) => candidateScore(b) - candidateScore(a) + (a, b) => scoreForegroundCandidateRow(b) - scoreForegroundCandidateRow(a) ) // Why: SSH relays do not have the daemon's async wrapper cache. Inspect the // remote process tree so node/python agent entrypoints become real agents. @@ -260,7 +256,7 @@ export function getForegroundProcessNameFromProcessTable( const inspectionCandidates = fallbackProcess && isAgentForegroundWrapperProcess(fallbackProcess) ? foregroundCandidates.filter((candidate) => - candidateMatchesFallbackWrapper(candidate, fallbackProcess) + isExpectedAgentProcess(getFirstCommandToken(candidate.command), fallbackProcess) ) : foregroundCandidates if ( @@ -278,7 +274,7 @@ export function getForegroundProcessNameFromProcessTable( return resolveOuterWrapperForegroundProcess(recognized, candidate, candidates) } } - return fallbackProcess ?? null + return null } /** diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts index bac5a6253b5..c65fec024ad 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts @@ -40,9 +40,11 @@ export type AgentCompletionCoordinatorOptions = { shouldSuppressConfirmedProcessExitCompletion?: (exited: RecognizedAgentProcess) => boolean isLive: () => boolean shouldPollProcessCadence?: () => boolean - // Why: direct SSH/remote authorities publish foreground evidence with their - // inventory, so a pane without agent evidence can stay push-driven instead - // of scheduling redundant host process-table reads while idle. + // Why: a host that publishes foreground evidence with its inventory lets a + // pane without agent evidence stay push-driven instead of scheduling + // redundant host process-table reads while idle. Wire a producer only once + // this renderer CONSUMES that evidence and can tell "no evidence published" + // from "host too old to publish it" — mixed-version hosts omit the field. shouldPollNoEvidenceProcessCadence?: () => boolean // Why: on hosts where one inspection forks a whole-process-table scan (local // Windows PowerShell/CIM), panes without agent evidence relax to a slow diff --git a/src/renderer/src/components/terminal-pane/agent-completion-no-evidence-cadence.test.ts b/src/renderer/src/components/terminal-pane/agent-completion-no-evidence-cadence.test.ts index fa7c564e069..a0a35c36057 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-no-evidence-cadence.test.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-no-evidence-cadence.test.ts @@ -138,6 +138,25 @@ describe('agent completion no-evidence inspection cadence', () => { expect(inspectProcess).not.toHaveBeenCalled() }) + it('leaves a hidden noisy pane fully unpolled in the shipped option shape', async () => { + // Why: production sets no `shouldPollNoEvidenceProcessCadence`, so the + // activity re-arm has to stay under the visibility/tracking gate — a + // background `npm run dev` pane must not resume 3s host scans (#6288). + const inspectProcess = vi.fn(async () => processResult(null, false)) + const { coordinator } = createCoordinator(inspectProcess, { + shouldPollProcessCadence: () => false, + shouldPollNoEvidenceProcessCadence: undefined + }) + + coordinator.startProcessTracking() + for (let tick = 0; tick < 12; tick += 1) { + coordinator.observeOutputActivity() + await vi.advanceTimersByTimeAsync(5_000) + } + + expect(inspectProcess).not.toHaveBeenCalled() + }) + it('escalates to the hot cadence when PTY output appears mid-interval', async () => { const inspectProcess = vi.fn(async () => processResult(null, false)) const { coordinator } = createCoordinator(inspectProcess) diff --git a/src/shared/process-table-snapshot.test.ts b/src/shared/process-table-snapshot.test.ts index c4011dd5fb3..bb9810bb633 100644 --- a/src/shared/process-table-snapshot.test.ts +++ b/src/shared/process-table-snapshot.test.ts @@ -1,11 +1,20 @@ import { readFileSync } from 'node:fs' import { join } from 'node:path' -import { describe, expect, it } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { execFileMock } = vi.hoisted(() => ({ execFileMock: vi.fn() })) + +vi.mock('node:child_process', () => ({ execFile: execFileMock })) + import { + buildProcessTableIndex, createProcessTableSnapshotReader, + getProcessTableSnapshot, + getStrictProcessTableSnapshot, parseProcessTableRows, parseStrictProcessTableRows, - ProcessTableCaptureError + ProcessTableCaptureError, + resetProcessTableSnapshotForTests } from './process-table-snapshot' function deferred(): { @@ -206,6 +215,75 @@ describe('process-table-snapshot reader', () => { }) }) +describe('shared process-table capture', () => { + beforeEach(() => { + execFileMock.mockReset() + resetProcessTableSnapshotForTests() + }) + + function mockPsCaptures(...stdouts: string[]): () => number { + let forks = 0 + execFileMock.mockImplementation( + (_command: string, _args: string[], _options: unknown, callback: unknown) => { + const stdout = stdouts[Math.min(forks, stdouts.length - 1)] ?? '' + forks += 1 + ;(callback as (err: unknown, result: { stdout: string; stderr: string }) => void)(null, { + stdout, + stderr: '' + }) + } + ) + return () => forks + } + + it('serves the strict and lenient views from ONE ps fork per TTL window', async () => { + // Why: both views run byte-identical argv, so separate memoizers would double + // the relay's idle fork rate — the regression issue #6288 removed. + const forks = mockPsCaptures('100 1 100 100 Ss+ /bin/zsh\n', '200 1 200 200 Ss+ /bin/bash\n') + + const [lenient, strict] = await Promise.all([ + getProcessTableSnapshot(), + getStrictProcessTableSnapshot() + ]) + + expect(forks()).toBe(1) + expect(lenient.map((row) => row.pid)).toEqual([100]) + expect(strict.map((row) => row.pid)).toEqual([100]) + }) + + it('reuses the cached capture for a later strict read inside the TTL', async () => { + const forks = mockPsCaptures('100 1 100 100 Ss+ /bin/zsh\n', '200 1 200 200 Ss+ /bin/bash\n') + + await getProcessTableSnapshot() + const strict = await getStrictProcessTableSnapshot() + + expect(forks()).toBe(1) + expect(strict).toEqual([ + { pid: 100, ppid: 1, pgid: 100, tpgid: 100, stat: 'Ss+', command: '/bin/zsh' } + ]) + }) + + it('builds only the indexes a resolver reads', () => { + // Why: an unread group index costs two maps plus a per-row array on every + // capture, on the exact path this reader exists to make cheap. + const index = buildProcessTableIndex( + parseStrictProcessTableRows('100 1 100 101 Ss /bin/zsh\n101 100 101 101 S+ node /opt/codex') + ) + + expect(Object.keys(index).sort()).toEqual(['byPid', 'childrenByPpid', 'rows', 'stats']) + }) + + it('keeps the lenient view readable when the same capture is strictly unreadable', async () => { + const forks = mockPsCaptures('100 1 Ss+ /bin/zsh\n') + + const lenient = await getProcessTableSnapshot() + await expect(getStrictProcessTableSnapshot()).rejects.toBeInstanceOf(ProcessTableCaptureError) + + expect(forks()).toBe(1) + expect(lenient).toEqual([{ pid: 100, ppid: 1, stat: 'Ss+', command: '/bin/zsh' }]) + }) +}) + describe('parseProcessTableRows', () => { it('parses pid/ppid/stat and keeps the full command (including spaces)', () => { const rows = parseProcessTableRows( @@ -248,6 +326,33 @@ describe('parseStrictProcessTableRows', () => { ]) }) + it('extracts pgid/tpgid across CRLF framing while retaining command spacing', () => { + expect( + parseStrictProcessTableRows( + ' PID PPID PGID TPGID STAT COMMAND\r\n 100 1 100 101 Ss /bin/zsh -l\r\n 101 100 101 101 S+ node /opt/codex --flag value\r\n' + ) + ).toEqual([ + { pid: 100, ppid: 1, pgid: 100, tpgid: 101, stat: 'Ss', command: '/bin/zsh -l' }, + { + pid: 101, + ppid: 100, + pgid: 101, + tpgid: 101, + stat: 'S+', + command: 'node /opt/codex --flag value' + } + ]) + }) + + it('accepts no-controlling-tty sentinels for later unverifiable classification', () => { + expect(parseStrictProcessTableRows('100 1 100 0 Ss /bin/zsh')).toEqual([ + { pid: 100, ppid: 1, pgid: 100, tpgid: 0, stat: 'Ss', command: '/bin/zsh' } + ]) + expect(parseStrictProcessTableRows('100 1 100 -1 Ss /bin/zsh')).toEqual([ + { pid: 100, ppid: 1, pgid: 100, tpgid: -1, stat: 'Ss', command: '/bin/zsh' } + ]) + }) + it('still rejects truncated captures as unreadable', () => { expect(() => parseStrictProcessTableRows('100 1 100 100 Ss+')).toThrow(ProcessTableCaptureError) }) diff --git a/src/shared/process-table-snapshot.ts b/src/shared/process-table-snapshot.ts index ba88afb595b..1d334972dca 100644 --- a/src/shared/process-table-snapshot.ts +++ b/src/shared/process-table-snapshot.ts @@ -117,9 +117,6 @@ export function parseStrictProcessTableRows(stdout: string): ProcessTableRow[] { return rows } -/** Alias retained for callers that prefer the adjective at the end. */ -export const parseProcessTableRowsStrict = parseStrictProcessTableRows - export type ProcessTableIndexStats = { captures?: number indexBuilds: number @@ -131,12 +128,15 @@ export type ProcessTableIndex = { rows: readonly ProcessTableRow[] byPid: ReadonlyMap childrenByPpid: ReadonlyMap - byPgid: ReadonlyMap - byTpgid: ReadonlyMap stats?: ProcessTableIndexStats } -/** Build all correlation indexes in one linear pass over a capture. */ +/** + * Build the correlation indexes in one linear pass over a capture. Only the + * indexes a resolver actually reads are materialized: group indexes would cost + * two more maps plus a per-row array allocation on every capture, and foreground + * membership is derived from each row's own `pgid` against the root's `tpgid`. + */ export function buildProcessTableIndex( rows: readonly ProcessTableRow[], stats?: ProcessTableIndexStats @@ -146,8 +146,6 @@ export function buildProcessTableIndex( } const byPid = new Map() const childrenByPpid = new Map() - const byPgid = new Map() - const byTpgid = new Map() for (const row of rows) { if (stats) { stats.rowVisits += 1 @@ -156,18 +154,16 @@ export function buildProcessTableIndex( const children = childrenByPpid.get(row.ppid) ?? [] children.push(row) childrenByPpid.set(row.ppid, children) - if (row.pgid !== undefined) { - const group = byPgid.get(row.pgid) ?? [] - group.push(row) - byPgid.set(row.pgid, group) - } - if (row.tpgid !== undefined) { - const foreground = byTpgid.get(row.tpgid) ?? [] - foreground.push(row) - byTpgid.set(row.tpgid, foreground) - } } - return { rows, byPid, childrenByPpid, byPgid, byTpgid, stats } + return { rows, byPid, childrenByPpid, stats } +} + +/** + * Rank a descendant row as a foreground candidate: a `+` (foreground process + * group) row always outranks a background one, then the deepest wins. + */ +export function scoreForegroundCandidateRow(row: ProcessTableRow & { depth: number }): number { + return (row.stat.includes('+') ? 10_000 : 0) + row.depth } export function lookupProcessTableIndex( @@ -296,27 +292,46 @@ export function createProcessTableSnapshotReader( } } -const defaultReader = createProcessTableSnapshotReader({ - runPs: async () => { - const { stdout } = await execFile('ps', [...PS_ARGS], { - encoding: 'utf-8', - timeout: PS_TIMEOUT_MS - }) - // Why: parse once inside the deduped scan so a burst of panes sharing the - // TTL window reuse one ProcessTableRow[] instead of each re-tokenizing the - // identical stdout — matches the Windows reader, which already caches rows. - return parseProcessTableRows(stdout) - }, - now: () => Date.now() -}) +/** + * One capture, two views. The lenient and strict readers issue byte-identical + * `ps` argv, so giving them separate memoizers would fork `ps` twice per TTL + * window on a relay that serves both — the exact doubling issue #6288 removed. + * Each parse is memoized per capture (including a strict failure) so a burst of + * panes sharing the window re-tokenizes nothing. + */ +type ProcessTableCapture = { + lenient: () => ProcessTableRow[] + strict: () => ProcessTableRow[] +} -const strictReader = createProcessTableSnapshotReader({ +function createProcessTableCapture(stdout: string): ProcessTableCapture { + let lenientRows: ProcessTableRow[] | null = null + let strictResult: { rows: ProcessTableRow[] } | { error: unknown } | null = null + return { + lenient: () => (lenientRows ??= parseProcessTableRows(stdout)), + strict: () => { + if (strictResult === null) { + try { + strictResult = { rows: parseStrictProcessTableRows(stdout) } + } catch (error) { + strictResult = { error } + } + } + if ('error' in strictResult) { + throw strictResult.error + } + return strictResult.rows + } + } +} + +const processTableReader = createProcessTableSnapshotReader({ runPs: async () => { const { stdout } = await execFile('ps', [...PS_ARGS], { encoding: 'utf-8', timeout: PS_TIMEOUT_MS }) - return parseStrictProcessTableRows(stdout) + return createProcessTableCapture(stdout) }, now: () => Date.now() }) @@ -326,22 +341,18 @@ const strictReader = createProcessTableSnapshotReader({ * its parsed rows. Per-process singleton: the relay and local main processes * each dedupe their own scans and share a single parse per TTL window. */ -export function getProcessTableSnapshot(): Promise { - return defaultReader.getSnapshot() +export async function getProcessTableSnapshot(): Promise { + return (await processTableReader.getSnapshot()).lenient() } /** Capture process rows from a scan that starts after this request. */ -export function getFreshProcessTableSnapshot(): Promise { - return defaultReader.getFreshSnapshot() +export async function getFreshProcessTableSnapshot(): Promise { + return (await processTableReader.getFreshSnapshot()).lenient() } -/** Run (or reuse) the strict evidence capture. */ -export function getStrictProcessTableSnapshot(): Promise { - return strictReader.getSnapshot() -} - -export function getFreshStrictProcessTableSnapshot(): Promise { - return strictReader.getFreshSnapshot() +/** Strict evidence view of the same deduplicated capture. */ +export async function getStrictProcessTableSnapshot(): Promise { + return (await processTableReader.getSnapshot()).strict() } /** @@ -349,6 +360,5 @@ export function getFreshStrictProcessTableSnapshot(): Promise * cases don't have one case's snapshot served to the next within the TTL. */ export function resetProcessTableSnapshotForTests(): void { - defaultReader.reset() - strictReader.reset() + processTableReader.reset() }