From 1d94ebee3f65940e1a6d5d608c6ffe84fc17733e Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 2 Sep 2026 12:49:15 -0700 Subject: [PATCH] fix(agents): stop a deeper vendor helper from stealing a pane's agent identity (#18062) * fix(agents): keep outer agent identity over vendor helpers * fix(agents): preserve outer identity across relay scans --------- Co-authored-by: Merge Sim --- .../__fixtures__/real-agent-rows.json.gz | Bin 0 -> 569 bytes .../agent-foreground-process-batch.ts | 27 ++---- ...agent-foreground-process-real-rows.test.ts | 37 ++++++++ .../providers/agent-foreground-process.ts | 36 ++++---- src/relay/pty-shell-utils.ts | 22 ++--- .../foreground-process-selection.test.ts | 51 +++++++++++ src/shared/foreground-process-selection.ts | 80 ++++++++++++++++++ 7 files changed, 201 insertions(+), 52 deletions(-) create mode 100644 src/main/providers/__fixtures__/real-agent-rows.json.gz create mode 100644 src/main/providers/agent-foreground-process-real-rows.test.ts create mode 100644 src/shared/foreground-process-selection.test.ts create mode 100644 src/shared/foreground-process-selection.ts diff --git a/src/main/providers/__fixtures__/real-agent-rows.json.gz b/src/main/providers/__fixtures__/real-agent-rows.json.gz new file mode 100644 index 0000000000000000000000000000000000000000..3bd62f9eda36e9bbf3a7e3c07ad27cca35f5c0e9 GIT binary patch literal 569 zcmV-90>=FxiwFP!000026U~>)PQx$|Mfd%RC|e3?V#g0wd;+375JIJ~N+F4(rUfL# zzvHBjlyspNq|xf^)ihxvG(Kbfg~>0f&OG@Yyx}Rau_b_54lB z=kOSo!&m}g&wjveJV5Yd$Y34^c(-TPWtw_8EO(6MIIM7t6+3MGeLvU;IG9SEsCQ^6 zNlZC*D4X0)L%G&sw~NFw0&p!A_F=^1IEa(c>A2%v(S^#Ze5f&$#uVF_Cbv^#c5>`y zQOY2*T0*R59T1QEHB;FI$5Qv46bHc z&r%hvC7(~zdGNT(lU?K@doB?^8?7;*wY3DR&pm0C64o03KdnLvQ0uoP;E&>}9(ITq z`UM(cCMkM^o7_$#TuZT=#U!}dTOzED{YJAKj9CF$B*N`=!ERBL>*fYk)kAnC!U*!J zrN(|R8Uqk8G7=yxf}W5!)+w&);jj{q9b>Q(g$+k|>@5Ntj!;+%JzQJfJG+J$9P1Y+ z`ovM@Vs%XqO+6e|;;}c~S1Ee!AqC&s{tU = null - for (const candidate of candidates) { - const recognized = recognizeAgentProcessFromCommandLine(candidate.command) - if ( - recognized && - (bestCandidate === null || - scoreForegroundCandidateRow(candidate) > scoreForegroundCandidateRow(bestCandidate)) - ) { - bestCandidate = candidate - bestName = recognized - } - } - if (bestCandidate && bestName) { + const selected = selectForegroundProcessCandidate(candidates, allCandidates) + if (selected) { return { available: true, - processName: resolveOuterWrapperForegroundProcess(bestName, bestCandidate, allCandidates) + processName: resolveOuterWrapperForegroundProcess( + selected.recognized, + selected.candidate, + allCandidates + ) } } return { available: true, processName: null } diff --git a/src/main/providers/agent-foreground-process-real-rows.test.ts b/src/main/providers/agent-foreground-process-real-rows.test.ts new file mode 100644 index 00000000000..fd86e207398 --- /dev/null +++ b/src/main/providers/agent-foreground-process-real-rows.test.ts @@ -0,0 +1,37 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { gunzipSync } from 'node:zlib' +import { describe, expect, it } from 'vitest' +import type { ProcessTableRow } from '../../shared/process-table-snapshot' +import { resolveAgentForegroundProcessFromPs } from './agent-foreground-process' + +type CapturedRun = { + agent: string + shellPid: number + rows: ProcessTableRow[] +} + +describe('real foreground process captures', () => { + it('resolves all six agents, including omp over its deeper vendor helpers', () => { + const captured = JSON.parse( + gunzipSync(readFileSync(join(__dirname, '__fixtures__', 'real-agent-rows.json.gz'))).toString( + 'utf8' + ) + ) as CapturedRun[] + + expect(captured).toHaveLength(6) + expect( + captured.map(({ agent, shellPid, rows }) => ({ + agent, + processName: resolveAgentForegroundProcessFromPs(rows, shellPid) + })) + ).toEqual([ + { agent: 'claude', processName: 'claude' }, + { agent: 'codex', processName: 'codex' }, + { agent: 'opencode', processName: 'opencode' }, + { agent: 'gemini', processName: 'gemini' }, + { agent: 'grok', processName: 'grok' }, + { agent: 'omp', processName: 'omp' } + ]) + }) +}) diff --git a/src/main/providers/agent-foreground-process.ts b/src/main/providers/agent-foreground-process.ts index d171e39a18e..d244e0100dc 100644 --- a/src/main/providers/agent-foreground-process.ts +++ b/src/main/providers/agent-foreground-process.ts @@ -11,6 +11,7 @@ import { type AgentForegroundResolutionOptions } from './windows-agent-foreground-process' import { isShellProcess } from '../../shared/shell-process-detection' +import { selectForegroundProcessCandidate } from '../../shared/foreground-process-selection' export type { AgentForegroundResolutionOptions } from './windows-agent-foreground-process' export { @@ -120,13 +121,6 @@ export async function confirmShellForegroundProcess( } } -function candidateScore(row: ProcessTableRow & { depth: number }): number { - // Why: foreground descendants carry `+` in `ps stat` on Unix PTYs. Prefer - // them, then prefer leaf/deeper wrappers so `node /path/bin/codex` beats the - // parent shell but still lets the native child confirm the same identity. - return (row.stat.includes('+') ? 10_000 : 0) + row.depth -} - export async function resolveAgentForegroundProcess( shellPid: number | null | undefined, fallbackProcess: string | null, @@ -191,30 +185,30 @@ export async function resolveAgentForegroundProcessWithAvailability( } } -function resolveAgentForegroundProcessFromPs( +export function resolveAgentForegroundProcessFromPs( rows: ProcessTableRow[], shellPid: number ): string | null { const shellRow = rows.find((row) => row.pid === shellPid) - const candidates = collectDescendants(rows, shellPid).sort( - (a, b) => candidateScore(b) - candidateScore(a) - ) + const candidates = collectDescendants(rows, shellPid) // Why: `+` in `ps stat` marks the process holding the terminal foreground. // The root shell can hold it after Ctrl-Z, so use the whole PTY tree as the // foreground gate; otherwise a stopped agent child still masquerades as live. const foregroundIsKnown = shellRow?.stat.includes('+') === true || candidates.some((candidate) => candidate.stat.includes('+')) - for (const candidate of candidates) { - if (foregroundIsKnown && !candidate.stat.includes('+')) { - continue - } - const recognized = recognizeAgentProcessFromCommandLine(candidate.command) - if (recognized) { - // Why: return the outer wrapper (omp) rather than the deeper wrapped child - // (pi) of a shell→omp→pi tree — see resolveOuterWrapperForegroundProcess. - return resolveOuterWrapperForegroundProcess(recognized, candidate, candidates) - } + const foregroundCandidates = foregroundIsKnown + ? candidates.filter((candidate) => candidate.stat.includes('+')) + : candidates + // Keep the complete process tree for ancestry checks. A recognized agent can + // sit above a non-foreground helper before another recognized process; the + // helper is filtered from selection but must remain traversable. + const ancestryCandidates = shellRow ? [{ ...shellRow, depth: 0 }, ...candidates] : candidates + const selected = selectForegroundProcessCandidate(foregroundCandidates, ancestryCandidates) + if (selected) { + // Why: return the outer wrapper (omp) rather than the deeper wrapped child + // (pi) of a shell→omp→pi tree — see resolveOuterWrapperForegroundProcess. + return resolveOuterWrapperForegroundProcess(selected.recognized, selected.candidate, candidates) } return null } diff --git a/src/relay/pty-shell-utils.ts b/src/relay/pty-shell-utils.ts index accccb9e702..e06edaeabc7 100644 --- a/src/relay/pty-shell-utils.ts +++ b/src/relay/pty-shell-utils.ts @@ -6,17 +6,16 @@ import { promisify } from 'node:util' import { isAgentForegroundWrapperProcess, isExpectedAgentProcess, - recognizeAgentProcess, - recognizeAgentProcessFromCommandLine + recognizeAgentProcess } 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 { selectForegroundProcessCandidate } from '../shared/foreground-process-selection' import { resolveOuterWrapperForegroundProcess, shouldInspectOuterWrapperForegroundProcess @@ -240,9 +239,7 @@ function getForegroundProcessNameFromProcessTable( // 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) - ) + const candidates = collectDescendants(index, pid) // 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. const foregroundIsKnown = @@ -264,13 +261,12 @@ function getForegroundProcessNameFromProcessTable( ) { return null } - for (const candidate of inspectionCandidates) { - const recognized = recognizeAgentProcessFromCommandLine(candidate.command) - if (recognized) { - // Why: return the outer wrapper (omp) rather than the deeper wrapped child - // (pi) of a shell→omp→pi tree — see resolveOuterWrapperForegroundProcess. - return resolveOuterWrapperForegroundProcess(recognized, candidate, candidates) - } + const ancestryCandidates = root ? [{ ...root, depth: 0 }, ...candidates] : candidates + const selected = selectForegroundProcessCandidate(inspectionCandidates, ancestryCandidates) + if (selected) { + // Why: return the outer wrapper (omp) rather than a deeper recognized helper + // in the same process lineage. + return resolveOuterWrapperForegroundProcess(selected.recognized, selected.candidate, candidates) } return null } diff --git a/src/shared/foreground-process-selection.test.ts b/src/shared/foreground-process-selection.test.ts new file mode 100644 index 00000000000..5fc45b186fb --- /dev/null +++ b/src/shared/foreground-process-selection.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from 'vitest' +import { selectForegroundProcessCandidate } from './foreground-process-selection' + +describe('selectForegroundProcessCandidate', () => { + it('keeps a recognized ancestor over a different agent helper below a non-agent', () => { + const candidates = [ + { pid: 101, ppid: 100, depth: 1, stat: 'S+', command: 'omp' }, + { pid: 102, ppid: 101, depth: 2, stat: 'S+', command: 'vendor-ui' }, + { pid: 103, ppid: 102, depth: 3, stat: 'S+', command: 'codex' } + ] + + expect(selectForegroundProcessCandidate(candidates)).toMatchObject({ + candidate: { pid: 101 }, + recognized: { agent: 'omp' } + }) + }) + + it('traverses non-foreground helpers when checking ancestry', () => { + const all = [ + { pid: 101, ppid: 100, depth: 1, stat: 'S+', command: 'omp' }, + { pid: 102, ppid: 101, depth: 2, stat: 'S', command: 'vendor-helper' }, + { pid: 103, ppid: 102, depth: 3, stat: 'S+', command: 'codex' } + ] + + expect(selectForegroundProcessCandidate([all[0], all[2]], all)).toMatchObject({ + candidate: { pid: 101 }, + recognized: { agent: 'omp' } + }) + }) + + it('refuses different recognized agents on sibling lineages', () => { + const candidates = [ + { pid: 101, ppid: 100, depth: 1, stat: 'S+', command: 'codex' }, + { pid: 102, ppid: 100, depth: 1, stat: 'S+', command: 'gemini' } + ] + + expect(selectForegroundProcessCandidate(candidates)).toBeNull() + }) + + it('keeps the deepest process when one recognized agent owns the lineage', () => { + const candidates = [ + { pid: 101, ppid: 100, depth: 1, stat: 'S+', command: 'node /opt/bin/codex' }, + { pid: 102, ppid: 101, depth: 2, stat: 'S+', command: '/opt/vendor/bin/codex' } + ] + + expect(selectForegroundProcessCandidate(candidates)).toMatchObject({ + candidate: { pid: 102 }, + recognized: { agent: 'codex' } + }) + }) +}) diff --git a/src/shared/foreground-process-selection.ts b/src/shared/foreground-process-selection.ts new file mode 100644 index 00000000000..294bb40e5d9 --- /dev/null +++ b/src/shared/foreground-process-selection.ts @@ -0,0 +1,80 @@ +import { + recognizeAgentProcessFromCommandLine, + type RecognizedAgentProcess +} from './agent-process-recognition' + +export type ForegroundProcessCandidate = { + pid: number + ppid: number + command: string + depth: number + stat?: string +} + +export type SelectedForegroundProcess = { + candidate: ForegroundProcessCandidate + recognized: RecognizedAgentProcess +} + +/** + * Select a foreground agent without letting a vendor helper steal an outer + * agent's identity when both names occur in one process lineage. + */ +export function selectForegroundProcessCandidate( + candidates: readonly ForegroundProcessCandidate[], + ancestryCandidates: readonly ForegroundProcessCandidate[] = candidates +): SelectedForegroundProcess | null { + const recognized = candidates.flatMap((candidate) => { + const agent = recognizeAgentProcessFromCommandLine(candidate.command) + return agent ? [{ candidate, recognized: agent }] : [] + }) + if (recognized.length === 0) { + return null + } + + const agentNames = new Set(recognized.map(({ recognized: agent }) => agent.agent)) + if (agentNames.size > 1) { + const candidatesByPid = new Map( + ancestryCandidates.map((candidate) => [candidate.pid, candidate]) + ) + const outer = [...recognized].sort( + (left, right) => left.candidate.depth - right.candidate.depth + )[0] + if ( + !outer || + !recognized.every((entry) => + isAncestorOrSelf(outer.candidate, entry.candidate, candidatesByPid) + ) + ) { + // Distinct sibling agents do not provide a trustworthy identity. + return null + } + return outer + } + + return recognized.reduce((best, current) => + foregroundCandidateScore(current.candidate) > foregroundCandidateScore(best.candidate) + ? current + : best + ) +} + +function foregroundCandidateScore(candidate: ForegroundProcessCandidate): number { + return (candidate.stat?.includes('+') ? 10_000 : 0) + candidate.depth +} + +function isAncestorOrSelf( + ancestor: ForegroundProcessCandidate, + descendant: ForegroundProcessCandidate, + candidatesByPid: ReadonlyMap +): boolean { + let currentPid = descendant.pid + while (currentPid !== ancestor.pid) { + const current = candidatesByPid.get(currentPid) + if (!current) { + return false + } + currentPid = current.ppid + } + return true +}