diff --git a/src/relay/pty-shell-utils.test.ts b/src/relay/pty-shell-utils.test.ts index 113b834c3bb..ab12c7440f7 100644 --- a/src/relay/pty-shell-utils.test.ts +++ b/src/relay/pty-shell-utils.test.ts @@ -472,6 +472,24 @@ describe('getForegroundProcessName', () => { }) }) + it('resolves a duplicated root pid to the LAST capture row', async () => { + // Why: the shared index is last-wins, replacing a first-wins `rows.find()`. + // One tie-break for every consumer of a malformed capture; here the later + // row owns the terminal foreground, so no background child can claim it. + await withProcessPlatform('linux', async () => { + mockExecFile((_command, args) => { + if (args[0] === '-axo') { + return { + stdout: ['100 1 Ss bash', '100 1 Ss+ bash', '101 100 S node /opt/codex'].join('\n') + } + } + return new Error('unexpected command') + }) + + await expect(getForegroundProcessName(100, 'bash')).resolves.toBe('bash') + }) + }) + 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 3a8218f164f..accccb9e702 100644 --- a/src/relay/pty-shell-utils.ts +++ b/src/relay/pty-shell-utils.ts @@ -11,8 +11,10 @@ import { } from '../shared/agent-process-recognition' import { getFirstCommandToken } from '../shared/command-token-scanner' import { + getProcessTableIndex, getProcessTableSnapshot, scoreForegroundCandidateRow, + type ProcessTableIndex, type ProcessTableRow } from '../shared/process-table-snapshot' import { @@ -198,22 +200,15 @@ export function isProcessAlive(pid: number): boolean { } function collectDescendants( - rows: ProcessTableRow[], + index: ProcessTableIndex, rootPid: number ): (ProcessTableRow & { depth: number })[] { - const childrenByParent = new Map() - for (const row of rows) { - const children = childrenByParent.get(row.ppid) ?? [] - children.push(row) - childrenByParent.set(row.ppid, children) - } - const descendants: (ProcessTableRow & { depth: number })[] = [] - const stack = (childrenByParent.get(rootPid) ?? []).map((row) => ({ row, depth: 1 })) + const stack = (index.childrenByPpid.get(rootPid) ?? []).map((row) => ({ row, depth: 1 })) while (stack.length > 0) { const { row, depth } = stack.pop()! descendants.push({ ...row, depth }) - for (const child of childrenByParent.get(row.pid) ?? []) { + for (const child of index.childrenByPpid.get(row.pid) ?? []) { stack.push({ row: child, depth: depth + 1 }) } } @@ -241,8 +236,11 @@ function getForegroundProcessNameFromProcessTable( pid: number, fallbackProcess?: string | null ): string | null { - const root = rows.find((row) => row.pid === pid) - const candidates = collectDescendants(rows, pid).sort( + // Why: one memoized index per capture, so N panes sharing the TTL-cached + // snapshot no longer each rebuild the parent/child map over every row. + const index = getProcessTableIndex(rows) + const root = index.byPid.get(pid) + const candidates = collectDescendants(index, pid).sort( (a, b) => scoreForegroundCandidateRow(b) - scoreForegroundCandidateRow(a) ) // Why: SSH relays do not have the daemon's async wrapper cache. Inspect the diff --git a/src/shared/process-table-snapshot.test.ts b/src/shared/process-table-snapshot.test.ts index bb9810bb633..334ef472ad1 100644 --- a/src/shared/process-table-snapshot.test.ts +++ b/src/shared/process-table-snapshot.test.ts @@ -9,12 +9,14 @@ vi.mock('node:child_process', () => ({ execFile: execFileMock })) import { buildProcessTableIndex, createProcessTableSnapshotReader, + getProcessTableIndex, getProcessTableSnapshot, getStrictProcessTableSnapshot, parseProcessTableRows, parseStrictProcessTableRows, ProcessTableCaptureError, - resetProcessTableSnapshotForTests + resetProcessTableSnapshotForTests, + type ProcessTableIndexStats } from './process-table-snapshot' function deferred(): { @@ -372,3 +374,47 @@ describe('parseStrictProcessTableRows', () => { ) }) }) + +describe('getProcessTableIndex', () => { + it('reuses one index for the same snapshot identity', () => { + const rows = parseProcessTableRows( + ['100 1 Ss bash', '101 100 S node codex', '102 100 S vim'].join('\n') + ) + + const first = getProcessTableIndex(rows) + const second = getProcessTableIndex(rows) + + expect(second).toBe(first) + expect(first.byPid.get(100)).toBe(rows[0]) + expect(first.childrenByPpid.get(100)).toEqual([rows[1], rows[2]]) + }) + + it('does not reuse an index across distinct snapshot arrays', () => { + const firstRows = parseProcessTableRows('100 1 Ss bash') + const secondRows = parseProcessTableRows('100 1 Ss bash') + + expect(getProcessTableIndex(secondRows)).not.toBe(getProcessTableIndex(firstRows)) + }) + + it('resolves a duplicated pid to the LAST row, matching the evidence resolver', () => { + // Why: the memo replaces a first-wins `rows.find()`, so pin the deliberate + // tie-break — one rule for every index consumer on a malformed capture. + const rows = parseProcessTableRows(['100 1 Ss bash', '100 1 Ss+ zsh'].join('\n')) + + expect(getProcessTableIndex(rows).byPid.get(100)).toBe(rows[1]) + }) + + it('keeps the memo out of measured builds so a cache hit cannot satisfy a perf gate', () => { + const rows = parseProcessTableRows('100 1 Ss bash') + const stats: ProcessTableIndexStats = { indexBuilds: 0, rowVisits: 0, indexLookups: 0 } + + const memoized = getProcessTableIndex(rows) + const measured = buildProcessTableIndex(rows, stats) + + expect(memoized.stats).toBeUndefined() + expect(measured).not.toBe(memoized) + expect(stats).toEqual({ indexBuilds: 1, rowVisits: 1, indexLookups: 0 }) + // The measured build must not evict or replace the shared memo. + expect(getProcessTableIndex(rows)).toBe(memoized) + }) +}) diff --git a/src/shared/process-table-snapshot.ts b/src/shared/process-table-snapshot.ts index 1d334972dca..933412f006b 100644 --- a/src/shared/process-table-snapshot.ts +++ b/src/shared/process-table-snapshot.ts @@ -166,6 +166,29 @@ export function scoreForegroundCandidateRow(row: ProcessTableRow & { depth: numb return (row.stat.includes('+') ? 10_000 : 0) + row.depth } +const processTableIndexes = new WeakMap() + +/** + * Memoize one index per snapshot identity, so the panes that share a TTL-cached + * capture walk its rows once instead of once each. Keyed weakly by the rows + * array, so an index dies with the snapshot that produced it. + * + * Deliberately stats-free: `buildProcessTableIndex` mutates the caller's counter + * bag and stores it on the index, so a shared index would hand one caller's bag + * to an unrelated later caller and let a cache hit satisfy an `indexBuilds` + * measurement without building anything. Measured callers keep calling + * `buildProcessTableIndex(rows, stats)` directly. + */ +export function getProcessTableIndex(rows: readonly ProcessTableRow[]): ProcessTableIndex { + const cached = processTableIndexes.get(rows) + if (cached) { + return cached + } + const index = buildProcessTableIndex(rows) + processTableIndexes.set(rows, index) + return index +} + export function lookupProcessTableIndex( index: ProcessTableIndex, lookup: (index: ProcessTableIndex) => T,