From 97dce8b1d4e815a060f4966d2bb5d7cd03c9e171 Mon Sep 17 00:00:00 2001 From: "buf0-bot[bot]" <252831055+buf0-bot[bot]@users.noreply.github.com> Date: Mon, 15 Jun 2026 00:03:20 -0700 Subject: [PATCH] fix: pr-bug-scan validated finding from #5283 (#5306) * fix: address pr-bug-scan validated finding from #5283 Added '+'-foreground filter in resolveAgentForegroundProcessFromPs so a suspended/backgrounded recognized agent is not returned when a non-agent holds the terminal foreground. * fix: handle shell foreground when agent is stopped --------- Co-authored-by: orca-bug-scan-bot Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com> --- .../agent-foreground-process.test.ts | 77 +++++++++++++++++++ .../providers/agent-foreground-process.ts | 13 +++- 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 src/main/providers/agent-foreground-process.test.ts diff --git a/src/main/providers/agent-foreground-process.test.ts b/src/main/providers/agent-foreground-process.test.ts new file mode 100644 index 00000000000..9694e0dac9c --- /dev/null +++ b/src/main/providers/agent-foreground-process.test.ts @@ -0,0 +1,77 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { execFileMock } = vi.hoisted(() => ({ + execFileMock: vi.fn() +})) + +vi.mock('child_process', () => ({ + execFile: execFileMock +})) + +import { resolveAgentForegroundProcess } from './agent-foreground-process' + +// Why: the module wraps execFile with promisify, so the mock must honor the +// Node callback contract — invoke the last arg with (err, { stdout, stderr }). +function mockPs(stdout: string): void { + execFileMock.mockImplementation((_cmd: string, _args: string[], _opts: unknown, cb: unknown) => { + const callback = cb as (err: unknown, result: { stdout: string; stderr: string }) => void + callback(null, { stdout, stderr: '' }) + }) +} + +describe('resolveAgentForegroundProcess', () => { + let platform: PropertyDescriptor | undefined + + beforeEach(() => { + execFileMock.mockReset() + platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'darwin' }) + }) + + afterEach(() => { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + }) + + it('does not report a suspended agent when a non-agent holds the foreground', async () => { + // shell pid 100. vim (pid 102) holds the terminal foreground ('+'); a + // suspended codex (pid 101, stat 'T', no '+') is a backgrounded descendant. + mockPs( + [ + '101 100 T node /Users/dev/.nvm/versions/node/bin/codex', + '102 100 S+ vim notes.txt' + ].join('\n') + ) + + await expect(resolveAgentForegroundProcess(100, 'vim')).resolves.toBe('vim') + }) + + it('still reports a foreground agent', async () => { + mockPs(['101 100 S+ node /Users/dev/.nvm/versions/node/bin/codex'].join('\n')) + + await expect(resolveAgentForegroundProcess(100, 'node')).resolves.toBe('codex') + }) + + it('does not report a stopped agent after the shell regains foreground', async () => { + mockPs( + ['100 99 Ss+ bash -i', '101 100 T node /Users/dev/.nvm/versions/node/bin/codex'].join( + '\n' + ) + ) + + await expect(resolveAgentForegroundProcess(100, 'bash')).resolves.toBe('bash') + }) + + it('falls back to recognized descendants when no process in the PTY tree holds foreground', async () => { + // No '+' marker at all (e.g. a detached/daemon descendant tree) — the + // recognized agent may still be the best available signal. + mockPs( + ['100 99 Ss bash -i', '101 100 S node /Users/dev/.nvm/versions/node/bin/codex'].join( + '\n' + ) + ) + + await expect(resolveAgentForegroundProcess(100, 'node')).resolves.toBe('codex') + }) +}) diff --git a/src/main/providers/agent-foreground-process.ts b/src/main/providers/agent-foreground-process.ts index 7d0d47f7c57..e2a9521e5c3 100644 --- a/src/main/providers/agent-foreground-process.ts +++ b/src/main/providers/agent-foreground-process.ts @@ -81,10 +81,21 @@ export async function resolveAgentForegroundProcess( } function resolveAgentForegroundProcessFromPs(stdout: string, shellPid: number): string | null { - const candidates = collectDescendants(parsePsRows(stdout), shellPid).sort( + const rows = parsePsRows(stdout) + const shellRow = rows.find((row) => row.pid === shellPid) + const candidates = collectDescendants(rows, shellPid).sort( (a, b) => candidateScore(b) - candidateScore(a) ) + // 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) { return recognized.processName