diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index b49fc5f827b..89164b09fca 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -10874,7 +10874,7 @@ "invariant": "Shift+Enter key recognition follows the client OS, while emitted bytes follow the active application and PTY host: active Kitty keyboard protocol authorizes CSI-u on every host; otherwise Windows hosts use Esc+CR except when allowlisted Droid ownership or fresh routing-trusted Droid process evidence authorizes CSI-u, and non-Windows hosts use Esc+CR. The active SSH connection, remote-runtime PTY owner, or live local session outranks later worktree ownership changes, and missing host-platform metadata falls back to the client. Every new command and PTY replacement revokes stale agent routing until current evidence settles; split or reused panes cannot inherit sibling or prior-PTY authority; unrelated keys perform no host, agent, protocol, or ConPTY lookup.", "oracle": "Renderer tests assert exact bytes across client/PTY-host combinations, including Kitty keyboard protocol active and inactive on Windows and non-Windows hosts, and prove SSH identity, the environment encoded in an active remote-runtime PTY id, and live local-session identity outrank current worktree ownership, while unavailable platform metadata falls back to the client. A live linux-arm64 SSH PTY independently captured Esc+CR with KKP inactive and CSI-u with KKP active. Fresh Windows process evidence authorizes Droid bytes only after a recognized global candidate is intersected with the exact ConPTY console process list; detached descendants, helper failure/timeout/root-only fallback, stale PTY exit/rebind results, typed text, and hook/OSC output fail closed. A new OSC 133;C immediately publishes a routing-neutral generation, including during D-to-C races. In cmd.exe/Git Bash/custom shells without OSC 133, accepted inferred commands start the same fresh generation; accepted submit/interrupt, title exit, focus, and visibility revoke trusted Droid bytes while one bounded confirmation runs. Detach preserves the live PTY's source shell override so WSL/native classification cannot change with the current default. Ordinary fast shell commands use cached/no-scan paths; routing-relevant confirmation bypasses cached process snapshots and keeps the bounded three-read ladder. Daemon protocol v21 persists only an allowlisted launchAgent; warm reattach uses it as a display/confirmation hint and restores routing only after current process proof. The Windows Electron test focuses the real xterm textarea and records exact renderer-to-main PTY writes.", "commands": [ - "pnpm exec vitest run --config config/vitest.config.ts src/main/daemon/daemon-foreground-confirmation-protocol.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/daemon-pty-router.test.ts src/main/daemon/daemon-server.test.ts src/main/daemon/degraded-daemon-pty-provider.test.ts src/main/daemon/pty-subprocess-foreground-scan-cadence.test.ts src/main/daemon/pty-subprocess.test.ts src/main/ipc/pty-runtime-kill-and-exit.test.ts src/main/providers/agent-foreground-process.test.ts src/main/providers/local-pty-provider-foreground-process.test.ts src/main/providers/windows-conpty-process-membership.test.ts src/renderer/src/components/terminal-pane/pane-foreground-agent-tracker.test.ts src/renderer/src/components/terminal-pane/pty-connection-command-finished-cleanup.test.ts src/renderer/src/components/terminal-pane/pty-connection-foreground-agent-routing.test.ts src/renderer/src/components/terminal-pane/pty-connection-foreground-agent-sampling.test.ts src/renderer/src/components/terminal-pane/pty-transport-reattach-admission.test.ts src/renderer/src/components/terminal-pane/terminal-pane-tab-detach.test.ts src/renderer/src/components/terminal-pane/terminal-shortcut-policy.test.ts src/renderer/src/components/terminal-pane/terminal-windows-shift-enter.test.ts src/renderer/src/store/slices/store-active-worktree-terminal-creation.test.ts src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts src/shared/agent-process-recognition.test.ts src/shared/process-table-snapshot.test.ts tests/e2e/terminal-foreground-confirmation.unit.test.ts", + "pnpm exec vitest run --config config/vitest.config.ts src/main/daemon/daemon-foreground-confirmation-protocol.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/daemon-pty-router.test.ts src/main/daemon/daemon-server.test.ts src/main/daemon/degraded-daemon-pty-provider.test.ts src/main/daemon/pty-subprocess-foreground-scan-cadence.test.ts src/main/daemon/pty-subprocess.test.ts src/main/ipc/pty-runtime-kill-and-exit.test.ts src/main/providers/agent-foreground-process.test.ts src/main/providers/local-pty-provider-foreground-process.test.ts src/main/daemon/pty-subprocess-foreground-degraded-scan.test.ts src/main/providers/windows-console-attached-processes.test.ts src/main/providers/windows-pty-job-membership.test.ts src/renderer/src/components/terminal-pane/pane-foreground-agent-tracker.test.ts src/renderer/src/components/terminal-pane/pty-connection-command-finished-cleanup.test.ts src/renderer/src/components/terminal-pane/pty-connection-foreground-agent-routing.test.ts src/renderer/src/components/terminal-pane/pty-connection-foreground-agent-sampling.test.ts src/renderer/src/components/terminal-pane/pty-transport-reattach-admission.test.ts src/renderer/src/components/terminal-pane/terminal-pane-tab-detach.test.ts src/renderer/src/components/terminal-pane/terminal-shortcut-policy.test.ts src/renderer/src/components/terminal-pane/terminal-windows-shift-enter.test.ts src/renderer/src/store/slices/store-active-worktree-terminal-creation.test.ts src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts src/shared/agent-process-recognition.test.ts src/shared/process-table-snapshot.test.ts tests/e2e/terminal-foreground-confirmation.unit.test.ts", "pnpm run test:e2e -- tests/e2e/terminal-shortcuts.spec.ts", "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane/keyboard-handlers.test.ts src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-pane-handle-resolution.test.ts src/renderer/src/components/terminal-pane/terminal-input-host-platform.test.ts" ], @@ -10889,7 +10889,9 @@ "src/main/ipc/pty-runtime-kill-and-exit.test.ts", "src/main/providers/agent-foreground-process.test.ts", "src/main/providers/local-pty-provider-foreground-process.test.ts", - "src/main/providers/windows-conpty-process-membership.test.ts", + "src/main/daemon/pty-subprocess-foreground-degraded-scan.test.ts", + "src/main/providers/windows-console-attached-processes.test.ts", + "src/main/providers/windows-pty-job-membership.test.ts", "src/renderer/src/components/terminal-pane/pane-foreground-agent-tracker.test.ts", "src/renderer/src/components/terminal-pane/keyboard-handlers.test.ts", "src/renderer/src/components/terminal-pane/pty-connection-command-finished-cleanup.test.ts", @@ -10928,13 +10930,30 @@ ] }, { - "file": "src/main/providers/windows-conpty-process-membership.test.ts", + "file": "src/main/providers/windows-console-attached-processes.test.ts", "assertions": [ "the fixed node-pty helper returns validated integer console membership containing the PTY root", "root-only fallback, malformed/missing-root messages, spawn errors, and a silent helper timeout all fail closed", "the bounded timeout kills the child helper exactly once" ] }, + { + "file": "src/main/providers/windows-pty-job-membership.test.ts", + "assertions": [ + "never spawns a child process to answer", + "refuses an answer that does not contain the shell", + "is asked with the pty handle, because a bare pid cannot find the job" + ] + }, + { + "file": "src/main/daemon/pty-subprocess-foreground-degraded-scan.test.ts", + "assertions": [ + "stops holding a dead agent once the job answer is only a superset", + "restores the idle refresh backoff once the dead identity is gone", + "never expires an identity while scans stay degraded", + "never expires an identity while the job answer is unverifiable" + ] + }, { "file": "src/main/providers/local-pty-provider-foreground-process.test.ts", "assertions": [ @@ -11082,13 +11101,13 @@ ], "evidenceRuns": [ { - "date": "2026-07-10", + "date": "2026-08-25", "runner": "local", "platform": "macos", - "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/daemon/daemon-foreground-confirmation-protocol.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/daemon-pty-router.test.ts src/main/daemon/daemon-server.test.ts src/main/daemon/degraded-daemon-pty-provider.test.ts src/main/daemon/pty-subprocess-foreground-scan-cadence.test.ts src/main/daemon/pty-subprocess.test.ts src/main/ipc/pty-runtime-kill-and-exit.test.ts src/main/providers/agent-foreground-process.test.ts src/main/providers/local-pty-provider-foreground-process.test.ts src/main/providers/windows-conpty-process-membership.test.ts src/renderer/src/components/terminal-pane/pane-foreground-agent-tracker.test.ts src/renderer/src/components/terminal-pane/pty-connection-command-finished-cleanup.test.ts src/renderer/src/components/terminal-pane/pty-connection-foreground-agent-routing.test.ts src/renderer/src/components/terminal-pane/pty-connection-foreground-agent-sampling.test.ts src/renderer/src/components/terminal-pane/pty-transport-reattach-admission.test.ts src/renderer/src/components/terminal-pane/terminal-pane-tab-detach.test.ts src/renderer/src/components/terminal-pane/terminal-shortcut-policy.test.ts src/renderer/src/components/terminal-pane/terminal-windows-shift-enter.test.ts src/renderer/src/store/slices/store-active-worktree-terminal-creation.test.ts src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts src/shared/agent-process-recognition.test.ts src/shared/process-table-snapshot.test.ts tests/e2e/terminal-foreground-confirmation.unit.test.ts", + "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/daemon/daemon-foreground-confirmation-protocol.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/daemon-pty-router.test.ts src/main/daemon/daemon-server.test.ts src/main/daemon/degraded-daemon-pty-provider.test.ts src/main/daemon/pty-subprocess-foreground-scan-cadence.test.ts src/main/daemon/pty-subprocess.test.ts src/main/ipc/pty-runtime-kill-and-exit.test.ts src/main/providers/agent-foreground-process.test.ts src/main/providers/local-pty-provider-foreground-process.test.ts src/main/daemon/pty-subprocess-foreground-degraded-scan.test.ts src/main/providers/windows-console-attached-processes.test.ts src/main/providers/windows-pty-job-membership.test.ts src/renderer/src/components/terminal-pane/pane-foreground-agent-tracker.test.ts src/renderer/src/components/terminal-pane/pty-connection-command-finished-cleanup.test.ts src/renderer/src/components/terminal-pane/pty-connection-foreground-agent-routing.test.ts src/renderer/src/components/terminal-pane/pty-connection-foreground-agent-sampling.test.ts src/renderer/src/components/terminal-pane/pty-transport-reattach-admission.test.ts src/renderer/src/components/terminal-pane/terminal-pane-tab-detach.test.ts src/renderer/src/components/terminal-pane/terminal-shortcut-policy.test.ts src/renderer/src/components/terminal-pane/terminal-windows-shift-enter.test.ts src/renderer/src/store/slices/store-active-worktree-terminal-creation.test.ts src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts src/shared/agent-process-recognition.test.ts src/shared/process-table-snapshot.test.ts tests/e2e/terminal-foreground-confirmation.unit.test.ts", "result": "passed", - "durationSeconds": 8.71, - "summary": "Twenty-two focused test files passed (1206 tests) on current main, covering fresh-scan ordering/deduplication, exact ConPTY membership and detached-child rejection, stale PTY guards, daemon v21 launch ownership with current-process confirmation, no-OSC command/exit recovery, preserved detach shell classification, unavailable evidence, routing-neutral command generations, exact shortcut bytes, and the composed daemon-plus-tracker contract; the Windows Electron byte test remains platform-gated and is not counted as local macOS evidence." + "durationSeconds": 3.88, + "summary": "Twenty-six focused test files passed (489 tests) on the PR head, covering fresh-scan ordering/deduplication, exact ConPTY membership and detached-child rejection, stale PTY guards, daemon v21 launch ownership with current-process confirmation, no-OSC command/exit recovery, preserved detach shell classification, unavailable evidence, routing-neutral command generations, exact shortcut bytes, and the composed daemon-plus-tracker contract; the Windows Electron byte test remains platform-gated and is not counted as local macOS evidence." } ], "runtimeBudget": { @@ -11147,7 +11166,7 @@ "oracle": "A quarter-circle Claude task title authorizes without a provider lookup only on the exact PTY incarnation carrying verified managed-Claude launch identity; the same title in a bare pane, with an unverified launch hint, or after PTY incarnation replacement is refused while current Claude activity remains a working signal. Fresh explicit state plus ordinary PowerShell plus confirmed recognized agent is sendable on the same PTY. Confirmed shell/non-agent, unavailable confirmation, PTY exit, handle rebind, or a callback PTY mismatch returns a refusal or not-writable result and writes zero bytes.", "commands": [ "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/quarter-circle-title-send-authorization.test.ts", - "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/runtime/rpc/terminal-send.test.ts src/main/ipc/pty-runtime-kill-and-exit.test.ts src/main/providers/agent-foreground-process.test.ts src/main/providers/local-pty-provider-foreground-process.test.ts src/main/providers/windows-conpty-process-membership.test.ts src/main/daemon/daemon-foreground-confirmation-protocol.test.ts src/main/daemon/pty-subprocess.test.ts src/renderer/src/lib/active-agent-note-send-explicit-target.test.ts src/renderer/src/components/browser-pane/annotate/BrowserAnnotationSendMenuContent.test.tsx" + "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/runtime/rpc/terminal-send.test.ts src/main/ipc/pty-runtime-kill-and-exit.test.ts src/main/providers/agent-foreground-process.test.ts src/main/providers/local-pty-provider-foreground-process.test.ts src/main/daemon/pty-subprocess-foreground-degraded-scan.test.ts src/main/providers/windows-console-attached-processes.test.ts src/main/providers/windows-pty-job-membership.test.ts src/main/daemon/daemon-foreground-confirmation-protocol.test.ts src/main/daemon/pty-subprocess.test.ts src/renderer/src/lib/active-agent-note-send-explicit-target.test.ts src/renderer/src/components/browser-pane/annotate/BrowserAnnotationSendMenuContent.test.tsx" ], "testFiles": [ "src/main/runtime/quarter-circle-title-send-authorization.test.ts", @@ -11156,7 +11175,9 @@ "src/main/ipc/pty-runtime-kill-and-exit.test.ts", "src/main/providers/agent-foreground-process.test.ts", "src/main/providers/local-pty-provider-foreground-process.test.ts", - "src/main/providers/windows-conpty-process-membership.test.ts", + "src/main/daemon/pty-subprocess-foreground-degraded-scan.test.ts", + "src/main/providers/windows-console-attached-processes.test.ts", + "src/main/providers/windows-pty-job-membership.test.ts", "src/main/daemon/daemon-foreground-confirmation-protocol.test.ts", "src/main/daemon/pty-subprocess.test.ts", "src/renderer/src/lib/active-agent-note-send-explicit-target.test.ts", @@ -11207,7 +11228,7 @@ "assertions": ["fresh confirmation is discarded when its owning local PTY exits"] }, { - "file": "src/main/providers/windows-conpty-process-membership.test.ts", + "file": "src/main/providers/windows-console-attached-processes.test.ts", "assertions": [ "exact ConPTY console membership comes from the fixed node-pty helper", "malformed, incomplete, timed-out, and spawn-error membership reads fail closed" @@ -11248,13 +11269,13 @@ "summary": "The focused file passed 9 tests, including current-incarnation managed-Claude authorization with zero foreground calls and fail-closed bare, unverified, and replacement-incarnation quarter-circle titles." }, { - "date": "2026-07-11", + "date": "2026-08-25", "runner": "local", "platform": "macos", - "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/runtime/rpc/terminal-send.test.ts src/main/ipc/pty-runtime-kill-and-exit.test.ts src/main/providers/agent-foreground-process.test.ts src/main/providers/local-pty-provider-foreground-process.test.ts src/main/providers/windows-conpty-process-membership.test.ts src/main/daemon/daemon-foreground-confirmation-protocol.test.ts src/main/daemon/pty-subprocess.test.ts src/renderer/src/lib/active-agent-note-send-explicit-target.test.ts src/renderer/src/components/browser-pane/annotate/BrowserAnnotationSendMenuContent.test.tsx", + "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/runtime/rpc/terminal-send.test.ts src/main/ipc/pty-runtime-kill-and-exit.test.ts src/main/providers/agent-foreground-process.test.ts src/main/providers/local-pty-provider-foreground-process.test.ts src/main/daemon/pty-subprocess-foreground-degraded-scan.test.ts src/main/providers/windows-console-attached-processes.test.ts src/main/providers/windows-pty-job-membership.test.ts src/main/daemon/daemon-foreground-confirmation-protocol.test.ts src/main/daemon/pty-subprocess.test.ts src/renderer/src/lib/active-agent-note-send-explicit-target.test.ts src/renderer/src/components/browser-pane/annotate/BrowserAnnotationSendMenuContent.test.tsx", "result": "passed", - "durationSeconds": 9.58, - "summary": "Ten focused test files passed (1183 tests), covering runtime confirmation and PTY revalidation, guarded RPC zero-write behavior, PTY controller routing, local/daemon fresh scans, exact ConPTY membership, and unchanged renderer note routing." + "durationSeconds": 13.74, + "summary": "Twelve focused test files passed (1351 tests, 1 skipped), covering runtime confirmation and PTY revalidation, guarded RPC zero-write behavior, PTY controller routing, local/daemon fresh scans, exact ConPTY membership, and unchanged renderer note routing." } ], "runtimeBudget": { diff --git a/src/main/daemon/pty-subprocess-cwd-cancel-identity.test.ts b/src/main/daemon/pty-subprocess-cwd-cancel-identity.test.ts index 44c70f75116..4ad763cdfdc 100644 --- a/src/main/daemon/pty-subprocess-cwd-cancel-identity.test.ts +++ b/src/main/daemon/pty-subprocess-cwd-cancel-identity.test.ts @@ -42,8 +42,8 @@ vi.mock('../providers/agent-foreground-process', () => ({ } })) -vi.mock('../providers/windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: () => Promise.resolve(new Set([12345])) +vi.mock('../providers/windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: () => new Set([12345]) })) import { createPtySubprocess } from './pty-subprocess' diff --git a/src/main/daemon/pty-subprocess-env-inheritance.test.ts b/src/main/daemon/pty-subprocess-env-inheritance.test.ts index 53846b5071b..8e3ca849f76 100644 --- a/src/main/daemon/pty-subprocess-env-inheritance.test.ts +++ b/src/main/daemon/pty-subprocess-env-inheritance.test.ts @@ -70,8 +70,8 @@ vi.mock('../providers/agent-foreground-process', () => ({ // fake timers; default to "shell-only" so the degraded-scan guard falls through // to its existing retirement logic (the degraded-scan behavior itself is // covered in pty-subprocess-foreground-degraded-scan.test.ts). -vi.mock('../providers/windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: () => Promise.resolve(new Set([12345])) +vi.mock('../providers/windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: () => new Set([12345]) })) import { createPtySubprocess } from './pty-subprocess' diff --git a/src/main/daemon/pty-subprocess-foreground-degraded-scan.test.ts b/src/main/daemon/pty-subprocess-foreground-degraded-scan.test.ts index c1b25d0750f..97ce175391c 100644 --- a/src/main/daemon/pty-subprocess-foreground-degraded-scan.test.ts +++ b/src/main/daemon/pty-subprocess-foreground-degraded-scan.test.ts @@ -37,8 +37,8 @@ vi.mock('../providers/agent-foreground-process', () => ({ resolveAgentForegroundProcessMock(...args) })) -vi.mock('../providers/windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: (...args: unknown[]) => readConptyMock(...args) +vi.mock('../providers/windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: (...args: unknown[]) => readConptyMock(...args) })) import { createPtySubprocess } from './pty-subprocess' @@ -84,7 +84,7 @@ describe('daemon pty foreground degraded-scan handling', () => { isPwshAvailableMock.mockReturnValue(false) resolveAgentForegroundProcessMock.mockReset() readConptyMock.mockReset() - readConptyMock.mockResolvedValue(null) + readConptyMock.mockReturnValue(null) previousUserDataPath = process.env.ORCA_USER_DATA_PATH userDataPath = mkdtempSync(join(tmpdir(), 'daemon-pty-degraded-scan-test-')) process.env.ORCA_USER_DATA_PATH = userDataPath @@ -127,11 +127,11 @@ describe('daemon pty foreground degraded-scan handling', () => { expect(readConptyMock).not.toHaveBeenCalled() }) - it('keeps a cached agent when a scan finds no agent but the console still has a child', async () => { + it('keeps a cached agent, for a bounded time, when the job still has a descendant', async () => { resolveAgentForegroundProcessMock .mockResolvedValueOnce({ available: true, processName: 'claude' }) .mockResolvedValue({ available: true, processName: null }) - readConptyMock.mockResolvedValue(new Set([12345, 999])) // child still attached + readConptyMock.mockReturnValue(new Set([12345, 999])) // child still attached const { handle } = await spawnWindowsShell() await readForegroundAt(handle, 0) @@ -139,11 +139,137 @@ describe('daemon pty foreground degraded-scan handling', () => { expect(await readForegroundAt(handle, 2_500)).toBe('claude') }) + it('stops holding a dead agent once the job answer is only a superset', async () => { + // The WSL shape: job [shell, detached plumbing] forever, so `size > 1` used + // to veto retirement outright and pin an exited agent's name for the life of + // the pane. Job membership is a SUPERSET of the console -- it cannot tell a + // working agent from a leftover -- so age decides instead. + resolveAgentForegroundProcessMock + .mockResolvedValueOnce({ available: true, processName: 'claude' }) + .mockResolvedValue({ available: true, processName: null }) + readConptyMock.mockReturnValue(new Set([12345, 999])) + const { handle } = await spawnWindowsShell() + + await readForegroundAt(handle, 0) + expect(await readForegroundAt(handle, 2_500)).toBe('claude') + await readForegroundAt(handle, 40_000) // this refresh clears the cache + expect(await readForegroundAt(handle, 40_100)).toBe('powershell.exe') + }) + + it('restores the idle refresh backoff once the dead identity is gone', async () => { + // A non-null cache makes idleNoEvidenceShell false, which pins retryMs at + // the 1s TTL. Never clearing the cache therefore also meant scanning the + // process table every second, forever, on an idle pane. + resolveAgentForegroundProcessMock + .mockResolvedValueOnce({ available: true, processName: 'claude' }) + .mockResolvedValue({ available: true, processName: null }) + readConptyMock.mockReturnValue(new Set([12345, 999])) + const { handle } = await spawnWindowsShell() + + await readForegroundAt(handle, 0) + await readForegroundAt(handle, 40_000) // identity expires here + const scansAfterExpiry = resolveAgentForegroundProcessMock.mock.calls.length + + await readForegroundAt(handle, 41_000) + expect(resolveAgentForegroundProcessMock.mock.calls.length).toBe(scansAfterExpiry) + + await readForegroundAt(handle, 60_000) + expect(resolveAgentForegroundProcessMock.mock.calls.length).toBeGreaterThan(scansAfterExpiry) + }) + + it('never expires an identity a scan keeps recognizing', async () => { + resolveAgentForegroundProcessMock.mockResolvedValue({ available: true, processName: 'claude' }) + readConptyMock.mockReturnValue(new Set([12345, 999])) + const { handle } = await spawnWindowsShell() + + await readForegroundAt(handle, 0) + expect(await readForegroundAt(handle, 40_000)).toBe('claude') + expect(await readForegroundAt(handle, 120_000)).toBe('claude') + }) + + it('never expires an identity while scans stay degraded', async () => { + // Age must only advance on positive "I looked and found no agent" evidence. + resolveAgentForegroundProcessMock + .mockResolvedValueOnce({ available: true, processName: 'claude' }) + .mockResolvedValue({ available: false, processName: null }) + readConptyMock.mockReturnValue(new Set([12345, 999])) + const { handle } = await spawnWindowsShell() + + await readForegroundAt(handle, 0) + expect(await readForegroundAt(handle, 120_000)).toBe('claude') + }) + + it('never expires an identity while the job answer is unverifiable', async () => { + // ssh-execution-boundary.md: loss of contact is not evidence of death. + resolveAgentForegroundProcessMock + .mockResolvedValueOnce({ available: true, processName: 'claude' }) + .mockResolvedValue({ available: true, processName: null }) + readConptyMock.mockReturnValue(null) + const { handle } = await spawnWindowsShell() + + await readForegroundAt(handle, 0) + expect(await readForegroundAt(handle, 120_000)).toBe('claude') + }) + + it('retires an anchored agent immediately when its pid leaves the job, despite a leftover', async () => { + // With an anchor pid the detached-leftover shape no longer pins a dead + // agent for the age bound: the anchor missing from a complete job read is + // proof of exit, leftovers notwithstanding. + resolveAgentForegroundProcessMock + .mockResolvedValueOnce({ + available: true, + processName: 'claude', + processId: 999 + }) + .mockResolvedValue({ available: true, processName: null }) + readConptyMock.mockReturnValue(new Set([12345, 999])) + const { handle } = await spawnWindowsShell() + + await readForegroundAt(handle, 0) + // Agent 999 exits; detached plumbing 777 keeps the job larger than the shell. + readConptyMock.mockReturnValue(new Set([12345, 777])) + await readForegroundAt(handle, 1_000) // refresh sees the anchor gone and clears + expect(await readForegroundAt(handle, 1_100)).toBe('powershell.exe') + }) + + it('never retires an anchored agent the job still holds, even when scans miss it', async () => { + // The anchor pid alive in the job is proof of life: agentless-but-available + // scans past the age bound restamp instead of retiring a working agent. + resolveAgentForegroundProcessMock + .mockResolvedValueOnce({ + available: true, + processName: 'claude', + processId: 999 + }) + .mockResolvedValue({ available: true, processName: null }) + readConptyMock.mockReturnValue(new Set([12345, 999])) + const { handle } = await spawnWindowsShell() + + await readForegroundAt(handle, 0) + await readForegroundAt(handle, 40_000) // pre-fix: this refresh cleared the cache + expect(await readForegroundAt(handle, 40_100)).toBe('claude') + expect(await readForegroundAt(handle, 120_000)).toBe('claude') + }) + + it('retires an anchored agent when the scan proves its pid was recycled', async () => { + // Squatter reuse: the pid survives in the job, but the scan shows it now + // runs a non-agent. Proof of life must yield to proof of a different process. + resolveAgentForegroundProcessMock + .mockResolvedValueOnce({ available: true, processName: 'claude', processId: 999 }) + .mockResolvedValue({ available: true, processName: null, anchorPidForeign: true }) + readConptyMock.mockReturnValue(new Set([12345, 999])) + const { handle } = await spawnWindowsShell() + + await readForegroundAt(handle, 0) + await readForegroundAt(handle, 1_000) // refresh sees the foreign anchor and clears + expect(await readForegroundAt(handle, 1_100)).toBe('powershell.exe') + }) + it('retires a cached agent when a scan finds no agent and the console is shell-only', async () => { resolveAgentForegroundProcessMock .mockResolvedValueOnce({ available: true, processName: 'claude' }) .mockResolvedValue({ available: true, processName: null }) - readConptyMock.mockResolvedValue(new Set([12345])) + readConptyMock.mockReturnValue(new Set([12345])) const { handle } = await spawnWindowsShell() await readForegroundAt(handle, 0) @@ -156,7 +282,7 @@ describe('daemon pty foreground degraded-scan handling', () => { resolveAgentForegroundProcessMock .mockResolvedValueOnce({ available: true, processName: 'claude' }) .mockResolvedValue({ available: true, processName: null }) - readConptyMock.mockResolvedValue(null) + readConptyMock.mockReturnValue(null) const { handle } = await spawnWindowsShell() await readForegroundAt(handle, 0) diff --git a/src/main/daemon/pty-subprocess-foreground-identity.test.ts b/src/main/daemon/pty-subprocess-foreground-identity.test.ts index 67967f02d5b..0e9d8c64c93 100644 --- a/src/main/daemon/pty-subprocess-foreground-identity.test.ts +++ b/src/main/daemon/pty-subprocess-foreground-identity.test.ts @@ -69,8 +69,8 @@ vi.mock('../providers/agent-foreground-process', () => ({ // fake timers; default to "shell-only" so the degraded-scan guard falls through // to its existing retirement logic (the degraded-scan behavior itself is // covered in pty-subprocess-foreground-degraded-scan.test.ts). -vi.mock('../providers/windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: () => Promise.resolve(new Set([12345])) +vi.mock('../providers/windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: () => new Set([12345]) })) import { createPtySubprocess } from './pty-subprocess' diff --git a/src/main/daemon/pty-subprocess-git-credential-guard.test.ts b/src/main/daemon/pty-subprocess-git-credential-guard.test.ts index 746db79287a..e3a4c5a503c 100644 --- a/src/main/daemon/pty-subprocess-git-credential-guard.test.ts +++ b/src/main/daemon/pty-subprocess-git-credential-guard.test.ts @@ -69,8 +69,8 @@ vi.mock('../providers/agent-foreground-process', () => ({ // fake timers; default to "shell-only" so the degraded-scan guard falls through // to its existing retirement logic (the degraded-scan behavior itself is // covered in pty-subprocess-foreground-degraded-scan.test.ts). -vi.mock('../providers/windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: () => Promise.resolve(new Set([12345])) +vi.mock('../providers/windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: () => new Set([12345]) })) import { createPtySubprocess } from './pty-subprocess' diff --git a/src/main/daemon/pty-subprocess-handle-lifecycle.test.ts b/src/main/daemon/pty-subprocess-handle-lifecycle.test.ts index 0a19bc55a06..a5d56d2b705 100644 --- a/src/main/daemon/pty-subprocess-handle-lifecycle.test.ts +++ b/src/main/daemon/pty-subprocess-handle-lifecycle.test.ts @@ -69,8 +69,8 @@ vi.mock('../providers/agent-foreground-process', () => ({ // fake timers; default to "shell-only" so the degraded-scan guard falls through // to its existing retirement logic (the degraded-scan behavior itself is // covered in pty-subprocess-foreground-degraded-scan.test.ts). -vi.mock('../providers/windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: () => Promise.resolve(new Set([12345])) +vi.mock('../providers/windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: () => new Set([12345]) })) import { createPtySubprocess } from './pty-subprocess' diff --git a/src/main/daemon/pty-subprocess-managed-agent-env.test.ts b/src/main/daemon/pty-subprocess-managed-agent-env.test.ts index ed63b84afec..120bb259594 100644 --- a/src/main/daemon/pty-subprocess-managed-agent-env.test.ts +++ b/src/main/daemon/pty-subprocess-managed-agent-env.test.ts @@ -69,8 +69,8 @@ vi.mock('../providers/agent-foreground-process', () => ({ // fake timers; default to "shell-only" so the degraded-scan guard falls through // to its existing retirement logic (the degraded-scan behavior itself is // covered in pty-subprocess-foreground-degraded-scan.test.ts). -vi.mock('../providers/windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: () => Promise.resolve(new Set([12345])) +vi.mock('../providers/windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: () => new Set([12345]) })) import { createPtySubprocess } from './pty-subprocess' diff --git a/src/main/daemon/pty-subprocess-windows-shell-launch.test.ts b/src/main/daemon/pty-subprocess-windows-shell-launch.test.ts index 8dd0b420008..e6ab1a3fca0 100644 --- a/src/main/daemon/pty-subprocess-windows-shell-launch.test.ts +++ b/src/main/daemon/pty-subprocess-windows-shell-launch.test.ts @@ -69,8 +69,8 @@ vi.mock('../providers/agent-foreground-process', () => ({ // fake timers; default to "shell-only" so the degraded-scan guard falls through // to its existing retirement logic (the degraded-scan behavior itself is // covered in pty-subprocess-foreground-degraded-scan.test.ts). -vi.mock('../providers/windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: () => Promise.resolve(new Set([12345])) +vi.mock('../providers/windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: () => new Set([12345]) })) import { createPtySubprocess } from './pty-subprocess' diff --git a/src/main/daemon/pty-subprocess-wsl-launch.test.ts b/src/main/daemon/pty-subprocess-wsl-launch.test.ts index ffc042d3509..4ec141a4492 100644 --- a/src/main/daemon/pty-subprocess-wsl-launch.test.ts +++ b/src/main/daemon/pty-subprocess-wsl-launch.test.ts @@ -72,8 +72,8 @@ vi.mock('../providers/agent-foreground-process', () => ({ // fake timers; default to "shell-only" so the degraded-scan guard falls through // to its existing retirement logic (the degraded-scan behavior itself is // covered in pty-subprocess-foreground-degraded-scan.test.ts). -vi.mock('../providers/windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: () => Promise.resolve(new Set([12345])) +vi.mock('../providers/windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: () => new Set([12345]) })) import { createPtySubprocess } from './pty-subprocess' diff --git a/src/main/daemon/pty-subprocess.test.ts b/src/main/daemon/pty-subprocess.test.ts index 0e05fd44041..b71696a7fc9 100644 --- a/src/main/daemon/pty-subprocess.test.ts +++ b/src/main/daemon/pty-subprocess.test.ts @@ -72,8 +72,8 @@ vi.mock('../providers/agent-foreground-process', () => ({ // fake timers; default to "shell-only" so the degraded-scan guard falls through // to its existing retirement logic (the degraded-scan behavior itself is // covered in pty-subprocess-foreground-degraded-scan.test.ts). -vi.mock('../providers/windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: () => Promise.resolve(new Set([12345])) +vi.mock('../providers/windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: () => new Set([12345]) })) import { createPtySubprocess, checkPtySpawnHealth } from './pty-subprocess' diff --git a/src/main/daemon/pty-subprocess/foreground-fallback-process.ts b/src/main/daemon/pty-subprocess/foreground-fallback-process.ts new file mode 100644 index 00000000000..b463ec9a136 --- /dev/null +++ b/src/main/daemon/pty-subprocess/foreground-fallback-process.ts @@ -0,0 +1,24 @@ +import { win32 as pathWin32 } from 'node:path' + +function normalizeForegroundProcessName(processName: string | null | undefined): string | null { + const trimmed = processName?.trim().replace(/^["']|["']$/g, '') ?? '' + if (!trimmed || trimmed === 'xterm-256color') { + return null + } + return trimmed.split(/[\\/]/).pop() || null +} + +/** + * node-pty's reported foreground name, normalized to a bare executable. + * Windows ConPTY can report nothing useful; fall back to the spawned shell. + */ +export function resolveFallbackForegroundProcess( + processName: string | null | undefined, + shellPath: string +): string | null { + const normalized = normalizeForegroundProcessName(processName) + if (normalized || process.platform !== 'win32') { + return normalized + } + return normalizeForegroundProcessName(pathWin32.basename(shellPath)) +} diff --git a/src/main/daemon/pty-subprocess/foreground-process-tracker.ts b/src/main/daemon/pty-subprocess/foreground-process-tracker.ts index ff905e0cdfb..adab8c6dca0 100644 --- a/src/main/daemon/pty-subprocess/foreground-process-tracker.ts +++ b/src/main/daemon/pty-subprocess/foreground-process-tracker.ts @@ -1,15 +1,23 @@ import type * as pty from 'node-pty' -import { win32 as pathWin32 } from 'node:path' import { getAgentForegroundContextPaths } from '../../providers/agent-foreground-context-paths' import { resolveAgentForegroundProcessWithAvailability } from '../../providers/agent-foreground-process' -import { readWindowsConptyProcessIds } from '../../providers/windows-conpty-process-membership' +import { + judgeCachedAgentJobEvidence, + WINDOWS_DETACHED_DESCENDANT_IDENTITY_MAX_AGE_MS +} from '../../providers/windows-cached-agent-revalidation' +import { readWindowsPtyJobProcessIds } from '../../providers/windows-pty-job-membership' +import { readWindowsConsoleAttachedProcessIds } from '../../providers/windows-console-attached-processes' import { isAgentForegroundWrapperProcess, recognizeAgentProcess, type RecognizedAgentProcess } from '../../../shared/agent-process-recognition' -import { shouldInspectOuterWrapperForegroundProcess } from '../../../shared/foreground-wrapper-agent' +import { + shouldInspectOuterWrapperForegroundName, + shouldInspectOuterWrapperForegroundProcess +} from '../../../shared/foreground-wrapper-agent' import { isShellProcess } from '../../../shared/shell-process-detection' +import { resolveFallbackForegroundProcess } from './foreground-fallback-process' import { parsePtySessionId } from '../pty-session-id' const FOREGROUND_AGENT_CACHE_TTL_MS = 1000 @@ -18,29 +26,7 @@ const WINDOWS_IDLE_SHELL_FOREGROUND_REFRESH_RETRY_MS = 15_000 const SHELL_FOREGROUND_OUTPUT_HOT_WINDOW_MS = 10_000 const STARTUP_AGENT_FOREGROUND_BOOTSTRAP_MS = 5_000 -function normalizeForegroundProcessName(processName: string | null | undefined): string | null { - const trimmed = processName?.trim().replace(/^["']|["']$/g, '') ?? '' - if (!trimmed || trimmed === 'xterm-256color') { - return null - } - return trimmed.split(/[\\/]/).pop() || null -} - -function resolveFallbackForegroundProcess( - processName: string | null | undefined, - shellPath: string -): string | null { - const normalized = normalizeForegroundProcessName(processName) - if (normalized || process.platform !== 'win32') { - return normalized - } - return normalizeForegroundProcessName(pathWin32.basename(shellPath)) -} - -function shouldInspectOuterWrapperFallback(processName: string | null): boolean { - const recognized = recognizeAgentProcess(processName) - return recognized !== null && shouldInspectOuterWrapperForegroundProcess(recognized) -} +type CachedAgentForeground = { processName: string; pid: number | null; refreshedAt: number } export type PtyForegroundProcessTracker = { recordOutput(data: string): void @@ -59,7 +45,8 @@ export function createPtyForegroundProcessTracker(args: { }): PtyForegroundProcessTracker { const proc = args.process let lastOutputAt = 0 - let cachedAgentForeground: { processName: string; refreshedAt: number } | null = null + // `pid` anchors the identity to the row that proved it (null when ambiguous). + let cachedAgentForeground: CachedAgentForeground | null = null const contextPaths = getAgentForegroundContextPaths({ cwd: args.cwd, worktreeId: parsePtySessionId(args.sessionId).worktreeId @@ -91,7 +78,7 @@ export function createPtyForegroundProcessTracker(args: { fallbackProcess !== null && (isShellProcess(fallbackProcess) || isAgentForegroundWrapperProcess(fallbackProcess) || - shouldInspectOuterWrapperFallback(fallbackProcess) || + shouldInspectOuterWrapperForegroundName(fallbackProcess) || process.platform !== 'win32') const scheduleRefresh = (fallbackProcess: string | null): void => { @@ -121,45 +108,76 @@ export function createPtyForegroundProcessTracker(args: { } foregroundRefreshInFlight = true lastForegroundRefreshStartedAt = now - const retireStaleForegroundIdentity = (): void => { + const identityOlderThan = (ms: number): boolean => + cachedAgentForeground !== null && Date.now() - cachedAgentForeground.refreshedAt > ms + const retireStaleForegroundIdentity = ({ onlyWhenAged = false } = {}): void => { const currentFallbackProcess = getFallbackProcess() if ( fallbackIsShell && !getActiveStartupAgent() && currentFallbackProcess !== null && - isShellProcess(currentFallbackProcess) + isShellProcess(currentFallbackProcess) && + (!onlyWhenAged || identityOlderThan(WINDOWS_DETACHED_DESCENDANT_IDENTITY_MAX_AGE_MS)) ) { cachedAgentForeground = null startupAgentForeground = null } else if ( - cachedAgentForeground !== null && - Date.now() - cachedAgentForeground.refreshedAt > FOREGROUND_AGENT_CACHE_TTL_MS && + identityOlderThan(FOREGROUND_AGENT_CACHE_TTL_MS) && currentFallbackProcess !== null && isAgentForegroundWrapperProcess(currentFallbackProcess) ) { cachedAgentForeground = null } } + const anchor = cachedAgentForeground void resolveAgentForegroundProcessWithAvailability(proc.pid, fallbackProcess, { - contextPaths + contextPaths, + ...(anchor?.pid != null + ? { anchorProcessId: anchor.pid, anchorProcessName: anchor.processName } + : {}) }) - .then(({ processName, available }) => { + .then(({ processName, processId, available, anchorPidForeign }) => { if (args.isDead() || !available) { return } if (!processName || !recognizeAgentProcess(processName)) { if (process.platform === 'win32' && fallbackIsShell && cachedAgentForeground !== null) { - return readWindowsConptyProcessIds(proc.pid).then((consoleProcessIds) => { - if (args.isDead() || consoleProcessIds === null || consoleProcessIds.size > 1) { + // Job, not console: needs no console attachment, so no fork (#10857). + const verdict = judgeCachedAgentJobEvidence({ + jobProcessIds: readWindowsPtyJobProcessIds(proc), + shellPid: proc.pid, + anchorProcessId: cachedAgentForeground.pid, + identityAgeMs: Date.now() - cachedAgentForeground.refreshedAt + }) + // Unverifiable is never exit proof (ssh-execution-boundary.md): hold. + if (verdict === 'unavailable') { + return + } + if (verdict === 'confirmed' || verdict === 'recheck') { + if (anchorPidForeign === true) { + // The scan proved the pid recycled to a non-agent: retire now. + retireStaleForegroundIdentity() return } + // The anchor pid is still in the job: the scan lost the row, not + // the agent. Restamp so a live agent never ages out (#9258). + cachedAgentForeground = { ...cachedAgentForeground, refreshedAt: Date.now() } + return + } + if (verdict === 'exited' || verdict === 'anchor-exited') { + // Safe mid-restart: an available scan already found no agent. retireStaleForegroundIdentity() - }) + return + } + // Unanchored superset evidence cannot tell a working agent from a + // leftover; the age bound settles it. + retireStaleForegroundIdentity({ onlyWhenAged: true }) + return } retireStaleForegroundIdentity() return } - cachedAgentForeground = { processName, refreshedAt: Date.now() } + cachedAgentForeground = { processName, pid: processId ?? null, refreshedAt: Date.now() } startupAgentForeground = null return processName }) @@ -192,7 +210,11 @@ export function createPtyForegroundProcessTracker(args: { fallbackRecognition !== null && shouldInspectOuterWrapperForegroundProcess(fallbackRecognition) if (fallbackProcess && fallbackRecognition && !inspectOuterWrapper) { - cachedAgentForeground = { processName: fallbackProcess, refreshedAt: Date.now() } + cachedAgentForeground = { + processName: fallbackProcess, + pid: null, + refreshedAt: Date.now() + } startupAgentForeground = null return fallbackProcess } @@ -247,7 +269,8 @@ export function createPtyForegroundProcessTracker(args: { ...(process.platform === 'win32' ? { forceProcessScan: true, - readWindowsConptyProcessIds: () => readWindowsConptyProcessIds(proc.pid) + readWindowsConsoleAttachedProcessIds: () => + readWindowsConsoleAttachedProcessIds(proc.pid) } : {}) } @@ -259,6 +282,7 @@ export function createPtyForegroundProcessTracker(args: { if (recognized) { cachedAgentForeground = { processName: recognized.processName, + pid: resolution.processId ?? null, refreshedAt: Date.now() } startupAgentForeground = null diff --git a/src/main/providers/agent-foreground-process-pi.test.ts b/src/main/providers/agent-foreground-process-pi.test.ts index 104d54bacfa..e5f92038f8d 100644 --- a/src/main/providers/agent-foreground-process-pi.test.ts +++ b/src/main/providers/agent-foreground-process-pi.test.ts @@ -52,14 +52,41 @@ describe('Pi Windows foreground recognition', () => { getAllProcessesMock.mockImplementation((cb: (snapshot: unknown) => void) => { cb(withSelf(rows)) }) - const readWindowsConptyProcessIds = vi.fn(async () => new Set([100, 101])) + const readWindowsConsoleAttachedProcessIds = vi.fn(async () => new Set([100, 101])) await expect( resolveAgentForegroundProcessWithAvailability(100, 'node.exe', { fresh: true, - readWindowsConptyProcessIds + readWindowsConsoleAttachedProcessIds }) - ).resolves.toEqual({ available: true, processName: 'pi' }) - expect(readWindowsConptyProcessIds).toHaveBeenCalledTimes(1) + ).resolves.toEqual({ available: true, processName: 'pi', processId: 101 }) + expect(readWindowsConsoleAttachedProcessIds).toHaveBeenCalledTimes(1) + }) + + it('anchors a collapsed omp name to the omp pid, not the embedded pi leaf', async () => { + // Pi restarts under a live OMP; an anchor on pi's pid would read that as + // OMP's exit and fire a false "agent done" when the next snapshot degrades. + const rows = [ + { pid: 100, ppid: 99, name: 'powershell.exe', commandLine: 'powershell.exe' }, + { pid: 101, ppid: 100, name: 'omp.exe', commandLine: 'omp' }, + { + pid: 102, + ppid: 101, + name: 'node.exe', + commandLine: + 'node.exe C:\\npm\\node_modules\\@earendil-works\\pi-coding-agent\\dist\\cli.js' + } + ] + getAllProcessesMock.mockImplementation((cb: (snapshot: unknown) => void) => { + cb(withSelf(rows)) + }) + const readWindowsConsoleAttachedProcessIds = vi.fn(async () => new Set([100, 101, 102])) + + await expect( + resolveAgentForegroundProcessWithAvailability(100, 'powershell.exe', { + fresh: true, + readWindowsConsoleAttachedProcessIds + }) + ).resolves.toEqual({ available: true, processName: 'omp', processId: 101 }) }) }) diff --git a/src/main/providers/agent-foreground-process.test.ts b/src/main/providers/agent-foreground-process.test.ts index 081a59a74eb..a1e02d02fc5 100644 --- a/src/main/providers/agent-foreground-process.test.ts +++ b/src/main/providers/agent-foreground-process.test.ts @@ -407,6 +407,96 @@ describe('resolveAgentForegroundProcess', () => { ).resolves.toEqual({ available: true, processName: null }) }) + it('reports a foreign anchor when its pid now runs an unrecognized command', async () => { + // Pid reuse inside the pane's job: the row proves a different process + // (command lines are immutable), so job membership must stop confirming it. + Object.defineProperty(process, 'platform', { value: 'win32' }) + mockWindowsRows([ + { pid: 100, ppid: 99, name: 'powershell.exe', commandLine: 'powershell.exe' }, + { pid: 999, ppid: 100, name: 'node.exe', commandLine: 'node server.js' } + ]) + + await expect( + resolveAgentForegroundProcessWithAvailability(100, 'powershell.exe', { + anchorProcessId: 999 + }) + ).resolves.toEqual({ + available: true, + processName: 'powershell.exe', + anchorPidForeign: true + }) + }) + + it('detects a foreign anchor even when the squatter is orphaned out of the descendant walk', async () => { + // The recycled pid's creator exited, so the row is not a ppid-descendant of + // the shell — but it can still be the job member holding the anchor pid. + Object.defineProperty(process, 'platform', { value: 'win32' }) + mockWindowsRows([ + { pid: 100, ppid: 99, name: 'powershell.exe', commandLine: 'powershell.exe' }, + { pid: 999, ppid: 500, name: 'node.exe', commandLine: 'node server.js' } + ]) + + await expect( + resolveAgentForegroundProcessWithAvailability(100, 'powershell.exe', { + anchorProcessId: 999 + }) + ).resolves.toEqual({ + available: true, + processName: 'powershell.exe', + anchorPidForeign: true + }) + }) + + it('flags an anchor recycled by a DIFFERENT agent as foreign', async () => { + // The squatter recognizes as an agent — just not the cached one. + Object.defineProperty(process, 'platform', { value: 'win32' }) + mockWindowsRows([ + { pid: 100, ppid: 99, name: 'powershell.exe', commandLine: 'powershell.exe' }, + { pid: 999, ppid: 500, name: 'node.exe', commandLine: 'node /usr/bin/codex' } + ]) + + await expect( + resolveAgentForegroundProcessWithAvailability(100, 'powershell.exe', { + anchorProcessId: 999, + anchorProcessName: 'claude' + }) + ).resolves.toEqual({ + available: true, + processName: 'powershell.exe', + anchorPidForeign: true + }) + }) + + it('does not flag an anchor whose row still recognizes as the cached agent', async () => { + Object.defineProperty(process, 'platform', { value: 'win32' }) + mockWindowsRows([ + { pid: 100, ppid: 99, name: 'powershell.exe', commandLine: 'powershell.exe' }, + { pid: 999, ppid: 500, name: 'node.exe', commandLine: 'node C:\\npm\\claude' } + ]) + + await expect( + resolveAgentForegroundProcessWithAvailability(100, 'powershell.exe', { + anchorProcessId: 999, + anchorProcessName: 'claude' + }) + ).resolves.toEqual({ available: true, processName: 'powershell.exe' }) + }) + + it('treats a query-denied anchor row as inconclusive, never foreign', async () => { + // A denied query yields command === name; the agent may just be unreadable. + Object.defineProperty(process, 'platform', { value: 'win32' }) + mockWindowsRows([ + { pid: 100, ppid: 99, name: 'powershell.exe', commandLine: 'powershell.exe' }, + { pid: 999, ppid: 100, name: 'node.exe', commandLine: '' } + ]) + + await expect( + resolveAgentForegroundProcessWithAvailability(100, 'powershell.exe', { + anchorProcessId: 999 + }) + ).resolves.toEqual({ available: true, processName: 'powershell.exe' }) + }) + it('treats a Windows snapshot missing the requested shell as unavailable', async () => { Object.defineProperty(process, 'platform', { value: 'win32' }) mockWindowsRows([ @@ -508,9 +598,39 @@ describe('resolveAgentForegroundProcess', () => { await expect( resolveAgentForegroundProcessWithAvailability(100, 'powershell.exe', { fresh: true, - readWindowsConptyProcessIds: async () => new Set([100, 101]) + readWindowsConsoleAttachedProcessIds: async () => new Set([100, 101]) }) - ).resolves.toEqual({ available: true, processName: 'droid' }) + ).resolves.toEqual({ + available: true, + processName: 'droid', + processId: 101 + }) + }) + + it('fails closed when console attachment cannot be read', async () => { + // The filter exists to DROP descendants that left the console. If it cannot + // tell which ones those are, publishing the unfiltered list would grant a + // detached process the pane's identity. #16419 briefly made this fall open + // and nothing caught it, because no test drove the null. + Object.defineProperty(process, 'platform', { value: 'win32' }) + mockWindowsRows([ + { pid: 200, ppid: 199, name: 'powershell.exe', commandLine: 'powershell.exe' }, + { pid: 201, ppid: 200, name: 'droid.exe', commandLine: 'droid' }, + { pid: 202, ppid: 200, name: 'agy.exe', commandLine: 'agy' } + ]) + + const reader = vi.fn(async () => null) + + const resolution = await resolveAgentForegroundProcessWithAvailability(200, 'powershell.exe', { + fresh: true, + readWindowsConsoleAttachedProcessIds: reader + }) + + expect(reader).toHaveBeenCalledTimes(1) + // available:false is the fail-closed signal; the wrapper then substitutes + // the shell fallback rather than publishing an unverified agent identity. + expect(resolution.available).toBe(false) + expect(resolution.processName).toBe('powershell.exe') }) it('recognizes a Windows shell-rooted agent when only one candidate matches the worktree path', async () => { @@ -655,15 +775,19 @@ describe('resolveAgentForegroundProcess', () => { commandLine: 'droid' } ]) - const readWindowsConptyProcessIds = vi.fn(async () => new Set([100, 101, 999])) + const readWindowsConsoleAttachedProcessIds = vi.fn(async () => new Set([100, 101, 999])) await expect( resolveAgentForegroundProcessWithAvailability(100, 'powershell.exe', { fresh: true, - readWindowsConptyProcessIds + readWindowsConsoleAttachedProcessIds }) - ).resolves.toEqual({ available: true, processName: 'droid' }) - expect(readWindowsConptyProcessIds).toHaveBeenCalledTimes(1) + ).resolves.toEqual({ + available: true, + processName: 'droid', + processId: 101 + }) + expect(readWindowsConsoleAttachedProcessIds).toHaveBeenCalledTimes(1) }) it('excludes a detached Windows Droid descendant from byte authority', async () => { @@ -686,7 +810,7 @@ describe('resolveAgentForegroundProcess', () => { await expect( resolveAgentForegroundProcessWithAvailability(100, 'powershell.exe', { fresh: true, - readWindowsConptyProcessIds: async () => new Set([100, 999]) + readWindowsConsoleAttachedProcessIds: async () => new Set([100, 999]) }) ).resolves.toEqual({ available: true, processName: 'powershell.exe' }) }) @@ -701,14 +825,14 @@ describe('resolveAgentForegroundProcess', () => { commandLine: 'powershell.exe' } ]) - const readWindowsConptyProcessIds = vi.fn(async () => new Set([100, 999])) + const readWindowsConsoleAttachedProcessIds = vi.fn(async () => new Set([100, 999])) await expect( resolveAgentForegroundProcessWithAvailability(100, 'powershell.exe', { fresh: true, - readWindowsConptyProcessIds + readWindowsConsoleAttachedProcessIds }) ).resolves.toEqual({ available: true, processName: 'powershell.exe' }) - expect(readWindowsConptyProcessIds).not.toHaveBeenCalled() + expect(readWindowsConsoleAttachedProcessIds).not.toHaveBeenCalled() }) }) diff --git a/src/main/providers/agent-foreground-process.ts b/src/main/providers/agent-foreground-process.ts index e428c945c32..5242bb2b911 100644 --- a/src/main/providers/agent-foreground-process.ts +++ b/src/main/providers/agent-foreground-process.ts @@ -16,6 +16,14 @@ export type { AgentForegroundResolutionOptions } from './windows-agent-foregroun export type AgentForegroundProcessResolution = { available: boolean processName: string | null + /** + * Windows: pid of the process a recognized name belongs to — a liveness + * anchor callers may check against the pane's job. Absent when the name is a + * fallback, ambiguous, or resolved on POSIX (where `+` already marks it). + */ + processId?: number + /** Windows: the scan proved the caller's `anchorProcessId` is now a non-agent. */ + anchorPidForeign?: boolean } function collectDescendants( @@ -86,7 +94,12 @@ export async function resolveAgentForegroundProcessWithAvailability( resolution.processName ?? (options.forceProcessScan && recognizeAgentProcessFromCommandLine(fallbackProcess) ? null - : fallbackProcess) + : fallbackProcess), + // The anchor only travels with the name it proved, never with a fallback. + ...(resolution.processName !== null && resolution.processId !== undefined + ? { processId: resolution.processId } + : {}), + ...(resolution.anchorPidForeign ? { anchorPidForeign: true } : {}) } } diff --git a/src/main/providers/local-pty-provider-foreground-process.test.ts b/src/main/providers/local-pty-provider-foreground-process.test.ts index 93c7a3e4db5..cbfb2cf5b0e 100644 --- a/src/main/providers/local-pty-provider-foreground-process.test.ts +++ b/src/main/providers/local-pty-provider-foreground-process.test.ts @@ -10,7 +10,7 @@ const { spawnMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, @@ -24,7 +24,7 @@ const { spawnMock: vi.fn(), prepareMacosTccLoginShellMock: vi.fn(), resolveAgentForegroundProcessMock: vi.fn(), - readWindowsConptyProcessIdsMock: vi.fn(), + readWindowsPtyJobProcessIdsMock: vi.fn(), killWithDescendantSweepMock: vi.fn(), isWslAvailableAsyncMock: vi.fn(), wslUncDirectoryExistsMock: vi.fn(), @@ -84,8 +84,8 @@ vi.mock('./agent-foreground-process', () => ({ resolveAgentForegroundProcessMock(...args) })) -vi.mock('./windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: (...args: unknown[]) => readWindowsConptyProcessIdsMock(...args) +vi.mock('./windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: (...args: unknown[]) => readWindowsPtyJobProcessIdsMock(...args) })) vi.mock('../wsl', () => ({ @@ -137,7 +137,7 @@ describe('LocalPtyProvider', () => { writeFileSyncMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, @@ -228,15 +228,15 @@ describe('LocalPtyProvider', () => { await expect(foreground).resolves.toBeNull() }) - it('confirms a still-active agent from ConPTY console presence without a whole-table scan', async () => { + it('confirms a still-active agent from job membership without a whole-table scan', async () => { Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) mockProc.process = 'powershell.exe' resolveAgentForegroundProcessMock.mockResolvedValue({ available: true, processName: 'claude' }) - // A child beyond the shell is still attached to this console. - readWindowsConptyProcessIdsMock.mockResolvedValue(new Set([12345, 999])) + // A descendant beyond the shell is still in the pane's job. + readWindowsPtyJobProcessIdsMock.mockReturnValue(new Set([12345, 999])) const { id } = await provider.spawn({ cols: 80, rows: 24 }) // First call establishes the agent identity via the scan. @@ -247,14 +247,111 @@ describe('LocalPtyProvider', () => { expect(resolveAgentForegroundProcessMock).toHaveBeenCalledTimes(1) }) - it('falls through to the scan when the ConPTY console shows only the shell', async () => { + it('stops trusting job membership once the cached agent goes stale', async () => { + // The daemon path is not the only one that short-circuited on `size > 1`. + // Here the early return skips the scan that is the ONLY thing able to + // clear ptyLastRecognizedForeground, so on a WSL pane -- whose job always + // holds console-detached plumbing -- the identity was pinned for good. Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) mockProc.process = 'powershell.exe' resolveAgentForegroundProcessMock.mockResolvedValue({ available: true, processName: 'claude' }) - readWindowsConptyProcessIdsMock.mockResolvedValue(new Set([12345])) + readWindowsPtyJobProcessIdsMock.mockReturnValue(new Set([12345, 999])) + vi.useFakeTimers({ toFake: ['Date'] }) + try { + vi.setSystemTime(1_000_000) + const { id } = await provider.spawn({ cols: 80, rows: 24 }) + + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledTimes(1) + + // Past the bound, the superset answer stops standing in for a scan. + vi.setSystemTime(1_000_000 + 40_000) + resolveAgentForegroundProcessMock.mockResolvedValue({ + available: true, + processName: 'powershell.exe' + }) + await expect(provider.getForegroundProcess(id)).resolves.toBe('powershell.exe') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + } + }) + + it('does not refresh the agent age after a degraded revalidation scan', async () => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + mockProc.process = 'powershell.exe' + resolveAgentForegroundProcessMock + .mockResolvedValueOnce({ available: true, processName: 'claude' }) + .mockResolvedValueOnce({ available: false, processName: null }) + .mockResolvedValue({ available: true, processName: 'powershell.exe' }) + readWindowsPtyJobProcessIdsMock.mockReturnValue(new Set([12345, 999])) + vi.useFakeTimers({ toFake: ['Date'] }) + try { + vi.setSystemTime(1_000_000) + const { id } = await provider.spawn({ cols: 80, rows: 24 }) + + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + + vi.setSystemTime(1_000_000 + 40_000) + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + + vi.setSystemTime(1_000_000 + 41_000) + await expect(provider.getForegroundProcess(id)).resolves.toBe('powershell.exe') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledTimes(3) + } finally { + vi.useRealTimers() + } + }) + + it('keeps short-circuiting a live agent instead of scanning on every call', async () => { + // The age bound must mean "time since we last confirmed the agent", not + // "how long the agent has run". Stamping only on a CHANGE of name makes a + // steadily-recognized agent age out permanently: the short-circuit dies + // after 30s, every call runs the whole-table scan, and one available-but- + // agentless snapshot then deletes a LIVE agent -- the false "agent done" + // #9258 exists to prevent. + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + mockProc.process = 'powershell.exe' + resolveAgentForegroundProcessMock.mockResolvedValue({ + available: true, + processName: 'claude' + }) + readWindowsPtyJobProcessIdsMock.mockReturnValue(new Set([12345, 999])) + vi.useFakeTimers({ toFake: ['Date'] }) + try { + vi.setSystemTime(1_000_000) + const { id } = await provider.spawn({ cols: 80, rows: 24 }) + + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledTimes(1) + + // Past the bound: one revalidating scan is expected, and it re-confirms. + vi.setSystemTime(1_000_000 + 40_000) + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledTimes(2) + + // That confirmation must restart the clock, so the short-circuit resumes. + vi.setSystemTime(1_000_000 + 45_000) + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + } + }) + + it('falls through to the scan when the job holds only the shell', async () => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + mockProc.process = 'powershell.exe' + resolveAgentForegroundProcessMock.mockResolvedValue({ + available: true, + processName: 'claude' + }) + readWindowsPtyJobProcessIdsMock.mockReturnValue(new Set([12345])) const { id } = await provider.spawn({ cols: 80, rows: 24 }) await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') @@ -268,7 +365,7 @@ describe('LocalPtyProvider', () => { resolveAgentForegroundProcessMock .mockResolvedValueOnce({ available: true, processName: 'claude' }) .mockResolvedValue({ available: true, processName: null }) - readWindowsConptyProcessIdsMock.mockResolvedValue(null) + readWindowsPtyJobProcessIdsMock.mockReturnValue(null) const { id } = await provider.spawn({ cols: 80, rows: 24 }) await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') @@ -276,13 +373,149 @@ describe('LocalPtyProvider', () => { expect(resolveAgentForegroundProcessMock).toHaveBeenCalledTimes(2) }) - it('retires the cached agent after verified shell-only membership and a no-agent scan', async () => { + it('retires an anchored agent immediately when its pid leaves the job, despite a leftover', async () => { + // Fix for stale-identity-behind-a-leftover: with an anchor pid, a detached + // descendant surviving in the job no longer stands in for the dead agent + // until the age bound -- the missing anchor is proof of exit right now. + Object.defineProperty(process, 'platform', { + configurable: true, + value: 'win32' + }) + mockProc.process = 'powershell.exe' + resolveAgentForegroundProcessMock + .mockResolvedValueOnce({ + available: true, + processName: 'claude', + processId: 999 + }) + .mockResolvedValue({ available: true, processName: null }) + readWindowsPtyJobProcessIdsMock.mockReturnValue(new Set([12345, 999])) + const { id } = await provider.spawn({ cols: 80, rows: 24 }) + + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + // Agent 999 exits; a detached leftover 777 keeps the job larger than the shell. + readWindowsPtyJobProcessIdsMock.mockReturnValue(new Set([12345, 777])) + await expect(provider.getForegroundProcess(id)).resolves.toBeNull() + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledTimes(2) + }) + + it('never expires an anchored agent the job still holds, even when scans miss it', async () => { + // Fix for false removal of a live agent: the anchor pid in the job is + // proof of life, so >30s of agentless-but-successful scans no longer + // retire a working agent -- and the confirmation restamps the clock. + Object.defineProperty(process, 'platform', { + configurable: true, + value: 'win32' + }) + mockProc.process = 'powershell.exe' + resolveAgentForegroundProcessMock + .mockResolvedValueOnce({ + available: true, + processName: 'claude', + processId: 999 + }) + .mockResolvedValue({ available: true, processName: null }) + readWindowsPtyJobProcessIdsMock.mockReturnValue(new Set([12345, 999])) + vi.useFakeTimers({ toFake: ['Date'] }) + try { + vi.setSystemTime(1_000_000) + const { id } = await provider.spawn({ cols: 80, rows: 24 }) + + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + + // Past the bound: one drift-recheck scan runs, finds nothing, and the + // live anchor outranks the incomplete snapshot. + vi.setSystemTime(1_000_000 + 40_000) + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledTimes(2) + + // The proof of life restamped the clock, so the short-circuit resumes. + vi.setSystemTime(1_000_000 + 45_000) + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledTimes(2) + + // Much later again: recheck, still alive, still claude. + vi.setSystemTime(1_000_000 + 80_000) + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledTimes(3) + } finally { + vi.useRealTimers() + } + }) + + it('holds a restarting agent across a degraded scan instead of reporting an exit', async () => { + // The anchor pid died but another job member remains -- possibly the + // agent's restarted successor under a new pid. A degraded scan at that + // instant must not fire a false "agent done"; the next available scan + // re-recognizes and re-anchors. Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) mockProc.process = 'powershell.exe' + resolveAgentForegroundProcessMock + .mockResolvedValueOnce({ available: true, processName: 'claude', processId: 999 }) + .mockResolvedValueOnce({ available: false, processName: null }) + .mockResolvedValue({ available: true, processName: 'claude', processId: 1000 }) + readWindowsPtyJobProcessIdsMock.mockReturnValue(new Set([12345, 999])) + vi.useFakeTimers({ toFake: ['Date'] }) + try { + vi.setSystemTime(1_000_000) + const { id } = await provider.spawn({ cols: 80, rows: 24 }) + + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + // Restart: 999 exits, successor 1000 joins the job; scan degrades. + readWindowsPtyJobProcessIdsMock.mockReturnValue(new Set([12345, 1000])) + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + // Downgraded evidence ages out; the recheck scan re-anchors the successor. + vi.setSystemTime(1_000_000 + 40_000) + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledTimes(3) + // Re-anchored on pid 1000: the short-circuit resumes. + vi.setSystemTime(1_000_000 + 45_000) + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledTimes(3) + } finally { + vi.useRealTimers() + } + }) + + it('drops an anchored agent when the drift recheck proves the pid was recycled', async () => { + // The anchor pid stays in the job (a squatter reused it) but the scan + // proves the pid now runs a non-agent: proof of life must not apply. + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + mockProc.process = 'powershell.exe' + resolveAgentForegroundProcessMock + .mockResolvedValueOnce({ available: true, processName: 'claude', processId: 999 }) + .mockResolvedValue({ available: true, processName: null, anchorPidForeign: true }) + readWindowsPtyJobProcessIdsMock.mockReturnValue(new Set([12345, 999])) + vi.useFakeTimers({ toFake: ['Date'] }) + try { + vi.setSystemTime(1_000_000) + const { id } = await provider.spawn({ cols: 80, rows: 24 }) + + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + + vi.setSystemTime(1_000_000 + 40_000) + await expect(provider.getForegroundProcess(id)).resolves.toBeNull() + // The recheck scan received the anchor to test against. + expect(resolveAgentForegroundProcessMock).toHaveBeenLastCalledWith( + mockProc.pid, + 'powershell.exe', + expect.objectContaining({ anchorProcessId: 999 }) + ) + } finally { + vi.useRealTimers() + } + }) + + it('retires the cached agent after verified shell-only membership and a no-agent scan', async () => { + Object.defineProperty(process, 'platform', { + configurable: true, + value: 'win32' + }) + mockProc.process = 'powershell.exe' resolveAgentForegroundProcessMock .mockResolvedValueOnce({ available: true, processName: 'claude' }) .mockResolvedValue({ available: true, processName: null }) - readWindowsConptyProcessIdsMock.mockResolvedValue(new Set([12345])) + readWindowsPtyJobProcessIdsMock.mockReturnValue(new Set([12345])) const { id } = await provider.spawn({ cols: 80, rows: 24 }) await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') diff --git a/src/main/providers/local-pty-provider-io-events.test.ts b/src/main/providers/local-pty-provider-io-events.test.ts index 83cf268a7f3..edc747ba694 100644 --- a/src/main/providers/local-pty-provider-io-events.test.ts +++ b/src/main/providers/local-pty-provider-io-events.test.ts @@ -10,7 +10,7 @@ const { spawnMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, @@ -24,7 +24,7 @@ const { spawnMock: vi.fn(), prepareMacosTccLoginShellMock: vi.fn(), resolveAgentForegroundProcessMock: vi.fn(), - readWindowsConptyProcessIdsMock: vi.fn(), + readWindowsPtyJobProcessIdsMock: vi.fn(), killWithDescendantSweepMock: vi.fn(), isWslAvailableAsyncMock: vi.fn(), wslUncDirectoryExistsMock: vi.fn(), @@ -84,8 +84,8 @@ vi.mock('./agent-foreground-process', () => ({ resolveAgentForegroundProcessMock(...args) })) -vi.mock('./windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: (...args: unknown[]) => readWindowsConptyProcessIdsMock(...args) +vi.mock('./windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: (...args: unknown[]) => readWindowsPtyJobProcessIdsMock(...args) })) vi.mock('../wsl', () => ({ @@ -137,7 +137,7 @@ describe('LocalPtyProvider', () => { writeFileSyncMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, diff --git a/src/main/providers/local-pty-provider-session-inventory.test.ts b/src/main/providers/local-pty-provider-session-inventory.test.ts index 2abe881354e..e94a3f98bfa 100644 --- a/src/main/providers/local-pty-provider-session-inventory.test.ts +++ b/src/main/providers/local-pty-provider-session-inventory.test.ts @@ -10,7 +10,7 @@ const { spawnMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, @@ -24,7 +24,7 @@ const { spawnMock: vi.fn(), prepareMacosTccLoginShellMock: vi.fn(), resolveAgentForegroundProcessMock: vi.fn(), - readWindowsConptyProcessIdsMock: vi.fn(), + readWindowsPtyJobProcessIdsMock: vi.fn(), killWithDescendantSweepMock: vi.fn(), isWslAvailableAsyncMock: vi.fn(), wslUncDirectoryExistsMock: vi.fn(), @@ -84,8 +84,8 @@ vi.mock('./agent-foreground-process', () => ({ resolveAgentForegroundProcessMock(...args) })) -vi.mock('./windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: (...args: unknown[]) => readWindowsConptyProcessIdsMock(...args) +vi.mock('./windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: (...args: unknown[]) => readWindowsPtyJobProcessIdsMock(...args) })) vi.mock('../wsl', () => ({ @@ -137,7 +137,7 @@ describe('LocalPtyProvider', () => { writeFileSyncMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, diff --git a/src/main/providers/local-pty-provider-shell-readiness.test.ts b/src/main/providers/local-pty-provider-shell-readiness.test.ts index c4621382e66..01b94816029 100644 --- a/src/main/providers/local-pty-provider-shell-readiness.test.ts +++ b/src/main/providers/local-pty-provider-shell-readiness.test.ts @@ -10,7 +10,7 @@ const { spawnMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, @@ -24,7 +24,7 @@ const { spawnMock: vi.fn(), prepareMacosTccLoginShellMock: vi.fn(), resolveAgentForegroundProcessMock: vi.fn(), - readWindowsConptyProcessIdsMock: vi.fn(), + readWindowsPtyJobProcessIdsMock: vi.fn(), killWithDescendantSweepMock: vi.fn(), isWslAvailableAsyncMock: vi.fn(), wslUncDirectoryExistsMock: vi.fn(), @@ -84,8 +84,8 @@ vi.mock('./agent-foreground-process', () => ({ resolveAgentForegroundProcessMock(...args) })) -vi.mock('./windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: (...args: unknown[]) => readWindowsConptyProcessIdsMock(...args) +vi.mock('./windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: (...args: unknown[]) => readWindowsPtyJobProcessIdsMock(...args) })) vi.mock('../wsl', () => ({ @@ -137,7 +137,7 @@ describe('LocalPtyProvider', () => { writeFileSyncMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, diff --git a/src/main/providers/local-pty-provider-shutdown.test.ts b/src/main/providers/local-pty-provider-shutdown.test.ts index 8e2f13bd1cd..902fa62aaf3 100644 --- a/src/main/providers/local-pty-provider-shutdown.test.ts +++ b/src/main/providers/local-pty-provider-shutdown.test.ts @@ -10,7 +10,7 @@ const { spawnMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, @@ -24,7 +24,7 @@ const { spawnMock: vi.fn(), prepareMacosTccLoginShellMock: vi.fn(), resolveAgentForegroundProcessMock: vi.fn(), - readWindowsConptyProcessIdsMock: vi.fn(), + readWindowsPtyJobProcessIdsMock: vi.fn(), killWithDescendantSweepMock: vi.fn(), isWslAvailableAsyncMock: vi.fn(), wslUncDirectoryExistsMock: vi.fn(), @@ -84,8 +84,8 @@ vi.mock('./agent-foreground-process', () => ({ resolveAgentForegroundProcessMock(...args) })) -vi.mock('./windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: (...args: unknown[]) => readWindowsConptyProcessIdsMock(...args) +vi.mock('./windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: (...args: unknown[]) => readWindowsPtyJobProcessIdsMock(...args) })) vi.mock('../wsl', () => ({ @@ -142,7 +142,7 @@ describe('LocalPtyProvider', () => { writeFileSyncMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, diff --git a/src/main/providers/local-pty-provider-spawn-cwd-safety.test.ts b/src/main/providers/local-pty-provider-spawn-cwd-safety.test.ts index 01aec7c7f4b..bfeb75e44d9 100644 --- a/src/main/providers/local-pty-provider-spawn-cwd-safety.test.ts +++ b/src/main/providers/local-pty-provider-spawn-cwd-safety.test.ts @@ -10,7 +10,7 @@ const { spawnMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, @@ -24,7 +24,7 @@ const { spawnMock: vi.fn(), prepareMacosTccLoginShellMock: vi.fn(), resolveAgentForegroundProcessMock: vi.fn(), - readWindowsConptyProcessIdsMock: vi.fn(), + readWindowsPtyJobProcessIdsMock: vi.fn(), killWithDescendantSweepMock: vi.fn(), isWslAvailableAsyncMock: vi.fn(), wslUncDirectoryExistsMock: vi.fn(), @@ -84,8 +84,8 @@ vi.mock('./agent-foreground-process', () => ({ resolveAgentForegroundProcessMock(...args) })) -vi.mock('./windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: (...args: unknown[]) => readWindowsConptyProcessIdsMock(...args) +vi.mock('./windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: (...args: unknown[]) => readWindowsPtyJobProcessIdsMock(...args) })) vi.mock('../wsl', () => ({ @@ -138,7 +138,7 @@ describe('LocalPtyProvider', () => { writeFileSyncMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, diff --git a/src/main/providers/local-pty-provider-spawn-env.test.ts b/src/main/providers/local-pty-provider-spawn-env.test.ts index f12de81360f..3ba1471410a 100644 --- a/src/main/providers/local-pty-provider-spawn-env.test.ts +++ b/src/main/providers/local-pty-provider-spawn-env.test.ts @@ -12,7 +12,7 @@ const { spawnMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, @@ -26,7 +26,7 @@ const { spawnMock: vi.fn(), prepareMacosTccLoginShellMock: vi.fn(), resolveAgentForegroundProcessMock: vi.fn(), - readWindowsConptyProcessIdsMock: vi.fn(), + readWindowsPtyJobProcessIdsMock: vi.fn(), killWithDescendantSweepMock: vi.fn(), isWslAvailableAsyncMock: vi.fn(), wslUncDirectoryExistsMock: vi.fn(), @@ -86,8 +86,8 @@ vi.mock('./agent-foreground-process', () => ({ resolveAgentForegroundProcessMock(...args) })) -vi.mock('./windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: (...args: unknown[]) => readWindowsConptyProcessIdsMock(...args) +vi.mock('./windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: (...args: unknown[]) => readWindowsPtyJobProcessIdsMock(...args) })) vi.mock('../wsl', () => ({ @@ -139,7 +139,7 @@ describe('LocalPtyProvider', () => { writeFileSyncMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, diff --git a/src/main/providers/local-pty-provider-spawn-session.test.ts b/src/main/providers/local-pty-provider-spawn-session.test.ts index 96340a8436c..0fce65a21b2 100644 --- a/src/main/providers/local-pty-provider-spawn-session.test.ts +++ b/src/main/providers/local-pty-provider-spawn-session.test.ts @@ -10,7 +10,7 @@ const { spawnMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, @@ -24,7 +24,7 @@ const { spawnMock: vi.fn(), prepareMacosTccLoginShellMock: vi.fn(), resolveAgentForegroundProcessMock: vi.fn(), - readWindowsConptyProcessIdsMock: vi.fn(), + readWindowsPtyJobProcessIdsMock: vi.fn(), killWithDescendantSweepMock: vi.fn(), isWslAvailableAsyncMock: vi.fn(), wslUncDirectoryExistsMock: vi.fn(), @@ -84,8 +84,8 @@ vi.mock('./agent-foreground-process', () => ({ resolveAgentForegroundProcessMock(...args) })) -vi.mock('./windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: (...args: unknown[]) => readWindowsConptyProcessIdsMock(...args) +vi.mock('./windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: (...args: unknown[]) => readWindowsPtyJobProcessIdsMock(...args) })) vi.mock('../wsl', () => ({ @@ -137,7 +137,7 @@ describe('LocalPtyProvider', () => { writeFileSyncMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, diff --git a/src/main/providers/local-pty-provider-test-harness.ts b/src/main/providers/local-pty-provider-test-harness.ts index 121b2e67e5a..cf95ae1d962 100644 --- a/src/main/providers/local-pty-provider-test-harness.ts +++ b/src/main/providers/local-pty-provider-test-harness.ts @@ -23,7 +23,7 @@ export type LocalPtyProviderMocks = { writeFileSyncMock: Mock prepareMacosTccLoginShellMock: Mock resolveAgentForegroundProcessMock: Mock - readWindowsConptyProcessIdsMock: Mock + readWindowsPtyJobProcessIdsMock: Mock killWithDescendantSweepMock: Mock isWslAvailableAsyncMock: Mock wslUncDirectoryExistsMock: Mock @@ -95,8 +95,8 @@ export function applyLocalPtyProviderMockDefaults(mocks: LocalPtyProviderMocks): processName: fallbackProcess }) ) - mocks.readWindowsConptyProcessIdsMock.mockReset() - mocks.readWindowsConptyProcessIdsMock.mockResolvedValue(null) + mocks.readWindowsPtyJobProcessIdsMock.mockReset() + mocks.readWindowsPtyJobProcessIdsMock.mockReturnValue(null) mocks.isWslAvailableAsyncMock.mockReset() mocks.isWslAvailableAsyncMock.mockResolvedValue(true) mocks.wslUncDirectoryExistsMock.mockReset() diff --git a/src/main/providers/local-pty-provider-windows-shell-launch.test.ts b/src/main/providers/local-pty-provider-windows-shell-launch.test.ts index 5f95812d5ae..4d524eb483e 100644 --- a/src/main/providers/local-pty-provider-windows-shell-launch.test.ts +++ b/src/main/providers/local-pty-provider-windows-shell-launch.test.ts @@ -10,7 +10,7 @@ const { spawnMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, @@ -24,7 +24,7 @@ const { spawnMock: vi.fn(), prepareMacosTccLoginShellMock: vi.fn(), resolveAgentForegroundProcessMock: vi.fn(), - readWindowsConptyProcessIdsMock: vi.fn(), + readWindowsPtyJobProcessIdsMock: vi.fn(), killWithDescendantSweepMock: vi.fn(), isWslAvailableAsyncMock: vi.fn(), wslUncDirectoryExistsMock: vi.fn(), @@ -85,8 +85,8 @@ vi.mock('./agent-foreground-process', () => ({ resolveAgentForegroundProcessMock(...args) })) -vi.mock('./windows-conpty-process-membership', () => ({ - readWindowsConptyProcessIds: (...args: unknown[]) => readWindowsConptyProcessIdsMock(...args) +vi.mock('./windows-pty-job-membership', () => ({ + readWindowsPtyJobProcessIds: (...args: unknown[]) => readWindowsPtyJobProcessIdsMock(...args) })) vi.mock('../wsl', () => ({ @@ -139,7 +139,7 @@ describe('LocalPtyProvider', () => { writeFileSyncMock, prepareMacosTccLoginShellMock, resolveAgentForegroundProcessMock, - readWindowsConptyProcessIdsMock, + readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, isWslAvailableAsyncMock, wslUncDirectoryExistsMock, diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index 6678e75c288..9d21acc7425 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -64,9 +64,13 @@ import { resolveStableForegroundProcess } from './stable-foreground-process' import { getAgentForegroundContextPaths } from './agent-foreground-context-paths' import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition' import { killWithDescendantSweep } from '../pty-descendant-termination' -import { readWindowsConptyProcessIds } from './windows-conpty-process-membership' +import { readWindowsPtyJobProcessIds } from './windows-pty-job-membership' +import { readWindowsConsoleAttachedProcessIds } from './windows-console-attached-processes' import { terminatePtyJob } from '../windows/windows-pty-job' -import { canConfirmAgentFromConsolePresence } from './windows-console-foreground' +import { + canRevalidateCachedAgentWithoutScan, + judgeCachedAgentJobEvidence +} from './windows-cached-agent-revalidation' import { forceKillPosixPtyProcessGroups } from '../pty/posix-pty-process-groups' import { shouldUseShellReadyStartupDelivery } from '../../shared/codex-startup-delivery' import { assertSafeAgentStartupCwd, resolveSafePtyDefaultCwd } from './pty-default-cwd' @@ -120,7 +124,12 @@ const pendingLocalPtySpawns = new Map>() const ptyShellName = new Map() const ptyAgentForegroundContextPaths = new Map() // Why: remember the last recognized agent foreground so a degraded scan doesn't report the shell and look like an exit. -const ptyLastRecognizedForeground = new Map() +// `pid` anchors the identity to the row that proved it (null when ambiguous); +// `at` is the last confirmation, so unanchored job evidence -- only a superset -- cannot hold it forever. +const ptyLastRecognizedForeground = new Map< + string, + { name: string; pid: number | null; at: number } +>() const ptyTerminalHandle = new Map() const ptyWorktreeId = new Map() const ptyInitialCwd = new Map() @@ -1406,24 +1415,47 @@ export class LocalPtyProvider implements IPtyProvider { proc.process || null, ptyShellName.get(id) ) - const cachedAgent = ptyLastRecognizedForeground.get(id) ?? null - let consoleMembershipUnavailable = false - // Why: console membership preserves a live cached agent without the whole-table scan (incomplete under Windows load). + const cachedEntry = ptyLastRecognizedForeground.get(id) + const cachedAgent = cachedEntry?.name ?? null + let paneMembershipUnavailable = false + let cachedAgentAliveInJob = false + // Why: job membership preserves a live cached agent without the whole-table + // scan (incomplete under Windows load). Job, not console: this asks "is + // anything besides the shell alive?", which needs no console attachment and + // so needs no forked helper (#10857). if ( process.platform === 'win32' && - canConfirmAgentFromConsolePresence(cachedAgent, fallbackProcess) + canRevalidateCachedAgentWithoutScan(cachedAgent, fallbackProcess) ) { try { - const consoleProcessIds = await readWindowsConptyProcessIds(proc.pid) + const paneProcessIds = readWindowsPtyJobProcessIds(proc) if (ptyProcesses.get(id) !== proc) { return null } - if (consoleProcessIds !== null && consoleProcessIds.size > 1 && cachedAgent !== null) { + const verdict = judgeCachedAgentJobEvidence({ + jobProcessIds: paneProcessIds, + shellPid: proc.pid, + anchorProcessId: cachedEntry?.pid ?? null, + identityAgeMs: Date.now() - (cachedEntry?.at ?? 0) + }) + if (verdict === 'confirmed' || verdict === 'unproven') { return cachedAgent } - consoleMembershipUnavailable = consoleProcessIds === null + if (verdict === 'exited') { + // The shell stands alone in a complete, inescapable job list: no + // successor is possible, so the identity retires before the scan. + ptyLastRecognizedForeground.delete(id) + } else if (verdict === 'anchor-exited' && cachedEntry) { + // The recognized process died but another member remains -- a + // leftover, or a restarted successor. Keep the name as unanchored, + // age-bounded evidence and let this cycle's scan decide: deleting + // here made a degraded scan read a mid-restart agent as an exit. + ptyLastRecognizedForeground.set(id, { ...cachedEntry, pid: null }) + } + cachedAgentAliveInJob = verdict === 'recheck' + paneMembershipUnavailable = verdict === 'unavailable' } catch { - consoleMembershipUnavailable = true + paneMembershipUnavailable = true } } try { @@ -1431,7 +1463,10 @@ export class LocalPtyProvider implements IPtyProvider { proc.pid, fallbackProcess, { - contextPaths: ptyAgentForegroundContextPaths.get(id) + contextPaths: ptyAgentForegroundContextPaths.get(id), + ...(cachedEntry?.pid != null + ? { anchorProcessId: cachedEntry.pid, anchorProcessName: cachedEntry.name } + : {}) } ) // Why: the scan can outlive PTY teardown/id reuse; stale results must not resurrect cache for a foreign id. @@ -1439,20 +1474,40 @@ export class LocalPtyProvider implements IPtyProvider { return null } // Why: a degraded scan reporting shell-as-foreground fires a false "agent done"; keep last recognized agent instead. - const lastRecognizedAgent = ptyLastRecognizedForeground.get(id) ?? null + const lastRecognizedAgent = ptyLastRecognizedForeground.get(id)?.name ?? null const resolvedAgent = resolution.processName ? recognizeAgentProcessFromCommandLine(resolution.processName) : null - // Why: incomplete snapshot + unavailable console probe isn't exit proof; only shell-only membership may clear the cache. - const stable = resolveStableForegroundProcess( - consoleMembershipUnavailable && resolvedAgent === null + // A recycled anchor pid keeps job membership truthful but the identity + // dead; the scan proving the pid now runs a non-agent settles it. + const anchorContradicted = resolution.anchorPidForeign === true + // Why: incomplete snapshot + unavailable job read isn't exit proof; and an + // anchor pid still alive in the job outranks a snapshot that lost its row. + const stableResolution = + (paneMembershipUnavailable || cachedAgentAliveInJob) && + !anchorContradicted && + resolvedAgent === null ? { ...resolution, available: false } - : resolution, - lastRecognizedAgent - ) - if (stable.lastRecognizedAgent) { - ptyLastRecognizedForeground.set(id, stable.lastRecognizedAgent) - } else { + : resolution + const stable = resolveStableForegroundProcess(stableResolution, lastRecognizedAgent) + if (stable.lastRecognizedAgent && stableResolution.available) { + // Only a positive recognition restarts the age bound. + ptyLastRecognizedForeground.set(id, { + name: stable.lastRecognizedAgent, + pid: + stable.lastRecognizedAgent === resolution.processName + ? (resolution.processId ?? null) + : null, + at: Date.now() + }) + } else if (stable.lastRecognizedAgent && cachedAgentAliveInJob && !anchorContradicted) { + // The anchor pid in the job is proof of life; restamp so the + // short-circuit resumes instead of scanning on every call. + const entry = ptyLastRecognizedForeground.get(id) + if (entry) { + ptyLastRecognizedForeground.set(id, { ...entry, at: Date.now() }) + } + } else if (!stable.lastRecognizedAgent) { ptyLastRecognizedForeground.delete(id) } return stable.processName @@ -1461,7 +1516,7 @@ export class LocalPtyProvider implements IPtyProvider { return null } // Why: an inspection error is a degraded read; fall back to last recognized agent (null reads as an exit). - return ptyLastRecognizedForeground.get(id) ?? null + return ptyLastRecognizedForeground.get(id)?.name ?? null } } @@ -1480,7 +1535,8 @@ export class LocalPtyProvider implements IPtyProvider { ...(process.platform === 'win32' ? { forceProcessScan: true, - readWindowsConptyProcessIds: () => readWindowsConptyProcessIds(proc.pid) + readWindowsConsoleAttachedProcessIds: () => + readWindowsConsoleAttachedProcessIds(proc.pid) } : {}) } diff --git a/src/main/providers/windows-agent-foreground-process.ts b/src/main/providers/windows-agent-foreground-process.ts index 0826406b160..8ffba73c24d 100644 --- a/src/main/providers/windows-agent-foreground-process.ts +++ b/src/main/providers/windows-agent-foreground-process.ts @@ -6,12 +6,12 @@ import { type RecognizedAgentProcess } from '../../shared/agent-process-recognition' import { - resolveOuterWrapperForegroundProcess, + resolveOuterWrapperForegroundIdentity, shouldInspectOuterWrapperForegroundProcess } from '../../shared/foreground-wrapper-agent' import { isShellProcess } from '../../shared/shell-process-detection' import { - queryWindowsProcessDescendants, + queryWindowsPaneProcessInventory, type WindowsProcessCandidate, type WindowsProcessRow } from './windows-foreground-process-rows' @@ -23,12 +23,34 @@ export type AgentForegroundResolutionOptions = { /** Force confirmation scans even when node-pty reports a recognized name. */ forceProcessScan?: boolean /** Lazily proves which global descendants still belong to this ConPTY. */ - readWindowsConptyProcessIds?: () => Promise | null> + readWindowsConsoleAttachedProcessIds?: () => Promise | null> + /** + * A caller's cached liveness anchor. When a scan row holds this pid but no + * longer recognizes as the cached agent, the pid was recycled by a different + * process (command lines are immutable): the resolution reports it foreign. + */ + anchorProcessId?: number + /** The cached agent name the anchor pid is supposed to prove. */ + anchorProcessName?: string } export type WindowsAgentForegroundResolution = { available: boolean processName: string | null + /** + * Pid of the process the name belongs to — the liveness anchor a caller may + * check against the pane's job. The OUTER wrapper's pid when the name + * collapsed onto one (its embedded leaf may exit first). Absent when the + * name came from a fallback or when sibling leaves left no single anchor. + */ + processId?: number + /** True when the scan proves `anchorProcessId` now belongs to a non-agent. */ + anchorPidForeign?: boolean +} + +type WindowsForegroundIdentity = { + processName: string | null + processId?: number } export function shouldInspectWindowsAgentForeground(fallbackProcess: string): boolean { @@ -55,13 +77,14 @@ export async function resolveWindowsAgentForegroundProcessWithAvailability( fallbackProcess: string, options: AgentForegroundResolutionOptions ): Promise { - const candidates = await queryWindowsProcessDescendants( - shellPid, - options.fresh === true ? { fresh: true } : {} - ) - if (!candidates) { + const inventory = await queryWindowsPaneProcessInventory(shellPid, { + ...(options.fresh === true ? { fresh: true } : {}), + ...(options.anchorProcessId !== undefined ? { anchorPid: options.anchorProcessId } : {}) + }) + if (!inventory) { return { available: false, processName: null } } + const candidates = inventory.candidates // Resolve membership before applying the global ambiguity rule. A detached // agent can otherwise make an attached Droid look ambiguous and suppress // the only identity that is actually able to receive this PTY's input. @@ -71,20 +94,35 @@ export async function resolveWindowsAgentForegroundProcessWithAvailability( options.contextPaths ) let filteredCandidates = candidates - if (hasRecognizedCandidate && options.readWindowsConptyProcessIds) { - const conptyProcessIds = await options.readWindowsConptyProcessIds() - if (!conptyProcessIds) { + if (hasRecognizedCandidate && options.readWindowsConsoleAttachedProcessIds) { + // Why console attachment and not the job: this filter exists to DROP a + // descendant that detached from the console, and the job still contains + // those by design. Answering it from the job would re-admit precisely what + // the filter is for -- granting byte authority to a detached `Start-Process + // droid`, or making an attached agent look ambiguous. + const consoleProcessIds = await options.readWindowsConsoleAttachedProcessIds() + if (!consoleProcessIds) { return { available: false, processName: null } } - filteredCandidates = candidates.filter((candidate) => conptyProcessIds.has(candidate.pid)) + filteredCandidates = candidates.filter((candidate) => consoleProcessIds.has(candidate.pid)) } + // From the FULL table, not the ppid projection: an orphaned job member (its + // creator exited) leaves the descendant walk yet can hold a recycled pid. + const anchorRow = inventory.anchorRow + const anchorRecognized = anchorRow === null ? null : recognizeWindowsProcessCandidate(anchorRow) + const anchorPidForeign = + anchorRow !== null && + (anchorRecognized !== null + ? // A recognized row is foreign when it names a DIFFERENT agent. + options.anchorProcessName !== undefined && + anchorRecognized.processName !== options.anchorProcessName + : // A query-denied row falls back to command === name; that is + // inconclusive (the agent may just be unreadable), never foreign. + anchorRow.command !== anchorRow.name) return { available: true, - processName: resolveWindowsProcessName( - filteredCandidates, - fallbackProcess, - options.contextPaths - ) + ...resolveWindowsForegroundIdentity(filteredCandidates, fallbackProcess, options.contextPaths), + ...(anchorPidForeign ? { anchorPidForeign: true } : {}) } } @@ -105,11 +143,11 @@ function windowsCandidatesContainRecognizedAgent( ) } -function resolveWindowsProcessName( +function resolveWindowsForegroundIdentity( candidates: readonly WindowsProcessCandidate[], fallbackProcess: string, contextPaths: readonly string[] | undefined -): string | null { +): WindowsForegroundIdentity { if (isShellProcess(fallbackProcess)) { return resolveShellForegroundProcessFromWindowsCandidates(candidates, contextPaths) } @@ -128,15 +166,15 @@ function resolveWindowsProcessName( recognizeAgentProcessFromCommandLine(candidate.command) ?? recognizeAgentProcessFromCommandLine(candidate.name) if (recognized) { - return resolveOuterWrapperForegroundProcess(recognized, candidate, candidates) + return resolveOuterWrapperForegroundIdentity(recognized, candidate, candidates) } - return null + return { processName: null } } function resolveShellForegroundProcessFromWindowsCandidates( candidates: readonly WindowsProcessCandidate[], contextPaths: readonly string[] | undefined -): string | null { +): WindowsForegroundIdentity { const recognizedCandidates = createRecognizedWindowsProcessCandidates(candidates, contextPaths) const contextCandidates = recognizedCandidates.filter((candidate) => candidate.contextMatch) if (contextCandidates.length > 0) { @@ -149,14 +187,14 @@ function resolveWrapperForegroundProcessFromWindowsCandidates( candidates: readonly WindowsProcessCandidate[], allCandidates: readonly WindowsProcessCandidate[], contextPaths: readonly string[] | undefined -): string | null { +): WindowsForegroundIdentity { const contextCandidates = createRecognizedWindowsProcessCandidates( candidates, contextPaths ).filter((candidate) => candidate.contextMatch) return contextCandidates.length > 0 ? resolveRecognizedWindowsProcessCandidates(contextCandidates, allCandidates) - : null + : { processName: null } } type RecognizedWindowsProcessCandidate = WindowsProcessRow & { @@ -190,9 +228,9 @@ function createRecognizedWindowsProcessCandidates( function resolveRecognizedWindowsProcessCandidates( recognizedCandidates: readonly RecognizedWindowsProcessCandidate[], allCandidates: readonly WindowsProcessCandidate[] -): string | null { +): WindowsForegroundIdentity { if (recognizedCandidates.length === 0) { - return null + return { processName: null } } const candidatesByPid = new Map(allCandidates.map((candidate) => [candidate.pid, candidate])) const leafCandidates = recognizedCandidates.filter( @@ -203,14 +241,24 @@ function resolveRecognizedWindowsProcessCandidates( windowsCandidateIsAncestor(candidate, other, candidatesByPid) ) ) - const leafProcessNames = new Set( - leafCandidates.map((candidate) => - resolveOuterWrapperForegroundProcess(candidate.recognized, candidate, allCandidates) - ) + const leafIdentities = leafCandidates.map((candidate) => + resolveOuterWrapperForegroundIdentity(candidate.recognized, candidate, allCandidates) ) + const leafProcessNames = new Set(leafIdentities.map((identity) => identity.processName)) // Why: Windows lacks a cheap PTY foreground marker like POSIX '+'. A single // recognized lineage leaf is strong enough; sibling agent leaves are not. - return leafProcessNames.size === 1 ? [...leafProcessNames][0] : null + if (leafProcessNames.size !== 1) { + return { processName: null } + } + // The anchor is the process the NAME belongs to — the outer wrapper when the + // leaf collapsed onto one, else the leaf itself. An embedded leaf can exit + // and restart under a live wrapper; its pid must not stand for the wrapper's. + const anchorProcessIds = new Set(leafIdentities.map((identity) => identity.processId)) + return { + processName: [...leafProcessNames][0], + // Distinct anchors agreeing on one name still leave no single liveness anchor. + ...(anchorProcessIds.size === 1 ? { processId: [...anchorProcessIds][0] } : {}) + } } function windowsCandidateIsAncestor( diff --git a/src/main/providers/windows-cached-agent-revalidation.test.ts b/src/main/providers/windows-cached-agent-revalidation.test.ts new file mode 100644 index 00000000000..55e52b37139 --- /dev/null +++ b/src/main/providers/windows-cached-agent-revalidation.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from 'vitest' +import { + canRevalidateCachedAgentWithoutScan, + judgeCachedAgentJobEvidence, + WINDOWS_DETACHED_DESCENDANT_IDENTITY_MAX_AGE_MS +} from './windows-cached-agent-revalidation' + +describe('canRevalidateCachedAgentWithoutScan', () => { + it('is true for a cached agent when node-pty only names the shell (a scan would run)', () => { + expect(canRevalidateCachedAgentWithoutScan('claude', 'powershell.exe')).toBe(true) + expect(canRevalidateCachedAgentWithoutScan('codex', 'cmd.exe')).toBe(true) + }) + + it('is false when node-pty already names a recognized agent (no scan needed)', () => { + // Nothing to save here — the fast no-scan path already returns the agent. + expect(canRevalidateCachedAgentWithoutScan('claude', 'claude')).toBe(false) + }) + + it('is false for a generic wrapper that may outlive the cached agent', () => { + expect(canRevalidateCachedAgentWithoutScan('claude', 'node.exe')).toBe(false) + }) + + it('is false when no agent has been recognized yet (identity must be established first)', () => { + expect(canRevalidateCachedAgentWithoutScan(null, 'powershell.exe')).toBe(false) + }) + + it('is false when there is no fallback process name', () => { + expect(canRevalidateCachedAgentWithoutScan('claude', null)).toBe(false) + }) +}) + +describe('judgeCachedAgentJobEvidence', () => { + const SHELL = 12345 + const AGENT = 999 + const FRESH = 1_000 + const AGED = WINDOWS_DETACHED_DESCENDANT_IDENTITY_MAX_AGE_MS + 1 + + const judge = ( + jobProcessIds: ReadonlySet | null, + anchorProcessId: number | null, + identityAgeMs: number + ) => + judgeCachedAgentJobEvidence({ + jobProcessIds, + shellPid: SHELL, + anchorProcessId, + identityAgeMs + }) + + it('is unavailable without a job answer, never exit proof', () => { + expect(judge(null, AGENT, FRESH)).toBe('unavailable') + expect(judge(null, null, AGED)).toBe('unavailable') + }) + + it('confirms a fresh anchored identity whose pid is still in the job', () => { + expect(judge(new Set([SHELL, AGENT]), AGENT, FRESH)).toBe('confirmed') + }) + + it('asks for a drift recheck once an anchored identity ages, without retiring it', () => { + expect(judge(new Set([SHELL, AGENT]), AGENT, AGED)).toBe('recheck') + }) + + it('downgrades an anchored identity when its pid leaves a job that still has members', () => { + // The survivor is a detached leftover OR the agent's restarted successor; + // only a scan can tell, so the verdict must not be a hard exit. + expect(judge(new Set([SHELL, 777]), AGENT, FRESH)).toBe('anchor-exited') + }) + + it('retires any identity when the shell stands alone', () => { + expect(judge(new Set([SHELL]), AGENT, FRESH)).toBe('exited') + expect(judge(new Set([SHELL]), null, FRESH)).toBe('exited') + }) + + it('bounds unanchored superset evidence by age', () => { + expect(judge(new Set([SHELL, 777]), null, FRESH)).toBe('unproven') + expect(judge(new Set([SHELL, 777]), null, AGED)).toBe('expired') + }) + + it('treats a shell-pid anchor as unanchored', () => { + // The shell being alive proves nothing about the agent. + expect(judge(new Set([SHELL, 777]), SHELL, FRESH)).toBe('unproven') + }) +}) diff --git a/src/main/providers/windows-cached-agent-revalidation.ts b/src/main/providers/windows-cached-agent-revalidation.ts new file mode 100644 index 00000000000..93325fb7518 --- /dev/null +++ b/src/main/providers/windows-cached-agent-revalidation.ts @@ -0,0 +1,86 @@ +import { isShellProcess } from '../../shared/shell-process-detection' + +/** + * How long a cached agent identity may survive on job evidence alone. + * + * The job is a SUPERSET of the console: it keeps console-detached descendants, + * so "something besides the shell is alive" cannot tell a working agent from a + * leftover. Any pane that keeps one -- and whose fallback name reads as a shell + * -- would otherwise pin a dead agent's name forever (#9258's bug, reached by a + * new route). Age is the tiebreak. + * + * The invariant both callers must preserve: every successful recognition resets + * the clock, so this can only expire an identity no scan has confirmed for this + * long. It is never a timeout on a live agent. + * + * 5x the renderer's 6s confirm ladder, and bounded above by the fact that a + * stale cache also pins the refresh at the 1s TTL until it clears. + */ +export const WINDOWS_DETACHED_DESCENDANT_IDENTITY_MAX_AGE_MS = 30_000 + +/** Whether job membership can revalidate a cached agent without a process scan. */ +export function canRevalidateCachedAgentWithoutScan( + cachedAgentName: string | null, + fallbackProcess: string | null +): boolean { + return ( + cachedAgentName !== null && + fallbackProcess !== null && + // Why: a generic wrapper may outlive the agent; only the shell fallback is + // the known unreliable Windows exit signal this cache is allowed to bridge. + isShellProcess(fallbackProcess) + ) +} + +export type WindowsCachedAgentJobVerdict = + /** Anchor pid alive in the job, recently confirmed: identity stands, no scan. */ + | 'confirmed' + /** Anchor pid alive but past the age bound: scan for drift; a silent scan keeps it. */ + | 'recheck' + /** The shell alone in a complete job read: no successor is possible, retire. */ + | 'exited' + /** + * The anchor pid left the job but another member remains — a leftover, or + * the agent's restarted successor. The name may only survive as unanchored, + * age-bounded evidence; an available scan settles it (a degraded one must + * not read as an exit). + */ + | 'anchor-exited' + /** No anchor; a non-shell member exists and the bound has not elapsed: identity stands. */ + | 'unproven' + /** No anchor and the bound elapsed: the superset answer stops standing in for a scan. */ + | 'expired' + /** No job answer: unverifiable per ssh-execution-boundary.md, never exit proof. */ + | 'unavailable' + +/** + * Weigh a cached agent identity against the pane's job membership. + * + * The job list is complete when non-null (the native read grows its buffer + * until every pid fits), so an anchor pid it lacks has provably exited — a job + * is inescapable once joined. Without an anchor the job is only a superset of + * the console (it keeps console-detached descendants), so `size > 1` cannot + * tell a working agent from a leftover and the age bound decides instead. + */ +export function judgeCachedAgentJobEvidence(args: { + jobProcessIds: ReadonlySet | null + shellPid: number + anchorProcessId: number | null + identityAgeMs: number +}): WindowsCachedAgentJobVerdict { + if (args.jobProcessIds === null) { + return 'unavailable' + } + const withinAgeBound = args.identityAgeMs <= WINDOWS_DETACHED_DESCENDANT_IDENTITY_MAX_AGE_MS + // A shell-pid "anchor" proves nothing about a child; treat it as unanchored. + if (args.anchorProcessId !== null && args.anchorProcessId !== args.shellPid) { + if (!args.jobProcessIds.has(args.anchorProcessId)) { + return args.jobProcessIds.size <= 1 ? 'exited' : 'anchor-exited' + } + return withinAgeBound ? 'confirmed' : 'recheck' + } + if (args.jobProcessIds.size <= 1) { + return 'exited' + } + return withinAgeBound ? 'unproven' : 'expired' +} diff --git a/src/main/providers/windows-conpty-process-membership.test.ts b/src/main/providers/windows-console-attached-processes.test.ts similarity index 70% rename from src/main/providers/windows-conpty-process-membership.test.ts rename to src/main/providers/windows-console-attached-processes.test.ts index 9ae23b6f379..e3c9e93d240 100644 --- a/src/main/providers/windows-conpty-process-membership.test.ts +++ b/src/main/providers/windows-console-attached-processes.test.ts @@ -1,6 +1,15 @@ +/* + * Coverage for the FORKING console-attachment reader. + * + * These assertions moved here with the code when the job-object reader took + * over the polling path (#16419). They cover the module that caused #10857 -- + * the bounded timeout, the single kill, spawn errors, malformed messages and + * helper-pid removal -- so none of it is left untested just because the file + * it used to live in now answers a different question. + */ import { EventEmitter } from 'node:events' import { describe, expect, it, vi } from 'vitest' -import { readWindowsConptyProcessIds } from './windows-conpty-process-membership' +import { readWindowsConsoleAttachedProcessIds } from './windows-console-attached-processes' function forkWith(event: 'message' | 'error' | 'none', value?: unknown, pid?: number) { const child = new EventEmitter() as EventEmitter & { @@ -24,12 +33,12 @@ function forkWith(event: 'message' | 'error' | 'none', value?: unknown, pid?: nu return { child, forkProcess: forkProcess as never } } -describe('readWindowsConptyProcessIds', () => { +describe('readWindowsConsoleAttachedProcessIds', () => { it('returns exact console membership from the fixed node-pty helper', async () => { const { forkProcess } = forkWith('message', [999, 101, 202, 303], 999) await expect( - readWindowsConptyProcessIds(101, { + readWindowsConsoleAttachedProcessIds(101, { forkProcess, resolveAgentPath: () => '/fixed/node-pty/conpty_console_list_agent.js' }) @@ -49,31 +58,33 @@ describe('readWindowsConptyProcessIds', () => { ['unavailable helper pid', [101, 202], undefined] ])('fails closed for %s', async (_label, processIds, helperPid) => { const { forkProcess } = forkWith('message', processIds, helperPid) - await expect(readWindowsConptyProcessIds(101, { forkProcess })).resolves.toBeNull() + await expect(readWindowsConsoleAttachedProcessIds(101, { forkProcess })).resolves.toBeNull() }) it('returns root-only membership when only the helper and shell are attached', async () => { const { forkProcess } = forkWith('message', [999, 101], 999) - await expect(readWindowsConptyProcessIds(101, { forkProcess })).resolves.toEqual(new Set([101])) + await expect(readWindowsConsoleAttachedProcessIds(101, { forkProcess })).resolves.toEqual( + new Set([101]) + ) }) it('reports membership excluding the helper when a real child is attached', async () => { const { forkProcess } = forkWith('message', [999, 101, 202], 999) - await expect(readWindowsConptyProcessIds(101, { forkProcess })).resolves.toEqual( + await expect(readWindowsConsoleAttachedProcessIds(101, { forkProcess })).resolves.toEqual( new Set([101, 202]) ) }) it('handles helper spawn errors without an unhandled child error', async () => { const { forkProcess } = forkWith('error') - await expect(readWindowsConptyProcessIds(101, { forkProcess })).resolves.toBeNull() + await expect(readWindowsConsoleAttachedProcessIds(101, { forkProcess })).resolves.toBeNull() }) it('kills a silent helper at the bounded timeout', async () => { vi.useFakeTimers() try { const { child, forkProcess } = forkWith('none') - const result = readWindowsConptyProcessIds(101, { forkProcess, timeoutMs: 10 }) + const result = readWindowsConsoleAttachedProcessIds(101, { forkProcess, timeoutMs: 10 }) await vi.advanceTimersByTimeAsync(10) await expect(result).resolves.toBeNull() expect(child.kill).toHaveBeenCalledTimes(1) @@ -89,7 +100,7 @@ describe('readWindowsConptyProcessIds', () => { child.kill.mockImplementation(() => { queueMicrotask(() => child.emit('error', new Error('kill failed'))) }) - const result = readWindowsConptyProcessIds(101, { forkProcess, timeoutMs: 10 }) + const result = readWindowsConsoleAttachedProcessIds(101, { forkProcess, timeoutMs: 10 }) await vi.advanceTimersByTimeAsync(10) await expect(result).resolves.toBeNull() expect(child.listenerCount('error')).toBe(0) diff --git a/src/main/providers/windows-conpty-process-membership.ts b/src/main/providers/windows-console-attached-processes.ts similarity index 69% rename from src/main/providers/windows-conpty-process-membership.ts rename to src/main/providers/windows-console-attached-processes.ts index 28e29ebbd97..8439156de9c 100644 --- a/src/main/providers/windows-conpty-process-membership.ts +++ b/src/main/providers/windows-console-attached-processes.ts @@ -4,7 +4,7 @@ const CONPTY_PROCESS_LIST_TIMEOUT_MS = 3_000 type ProcessListMessage = { consoleProcessList?: unknown } -type WindowsConptyMembershipDeps = { +type WindowsConsoleAttachedProcessDeps = { forkProcess?: typeof fork resolveAgentPath?: () => string timeoutMs?: number @@ -15,12 +15,25 @@ function resolveNodePtyConsoleListAgent(): string { } /** - * Returns normalized console membership, or null when the probe is unavailable. - * A root-only set proves the shell is alone because successful raw results include the helper. + * Processes ATTACHED TO THIS PANE'S CONSOLE, or null when unavailable. + * + * Distinct from job membership on purpose. `GetConsoleProcessList` must be + * called from a process attached to that console, and a process can hold only + * one console at a time -- which is why node-pty answers it from a separate + * process, and why this still forks. + * + * Only the candidate FILTER may use this. That filter exists to drop a + * descendant which detached from the console (`Start-Process`, a GUI child), and + * the job object deliberately still contains those, so the job cannot answer it + * -- see docs/windows-wsl-root-cause-plan.html, "Use B". + * + * This is not the fork storm in #10857: it runs only when a recognized agent + * candidate already exists, not on every foreground poll. Bounding it to one + * pooled, supervised helper is the remaining half of that fix. */ -export function readWindowsConptyProcessIds( +export function readWindowsConsoleAttachedProcessIds( rootPid: number, - deps: WindowsConptyMembershipDeps = {} + deps: WindowsConsoleAttachedProcessDeps = {} ): Promise | null> { if (!Number.isSafeInteger(rootPid) || rootPid <= 0) { return Promise.resolve(null) diff --git a/src/main/providers/windows-console-foreground.test.ts b/src/main/providers/windows-console-foreground.test.ts deleted file mode 100644 index f13ef2ae556..00000000000 --- a/src/main/providers/windows-console-foreground.test.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { canConfirmAgentFromConsolePresence } from './windows-console-foreground' - -describe('canConfirmAgentFromConsolePresence', () => { - it('is true for a cached agent when node-pty only names the shell (a scan would run)', () => { - expect(canConfirmAgentFromConsolePresence('claude', 'powershell.exe')).toBe(true) - expect(canConfirmAgentFromConsolePresence('codex', 'cmd.exe')).toBe(true) - }) - - it('is false when node-pty already names a recognized agent (no scan needed)', () => { - // Nothing to save here — the fast no-scan path already returns the agent. - expect(canConfirmAgentFromConsolePresence('claude', 'claude')).toBe(false) - }) - - it('is false for a generic wrapper that may outlive the cached agent', () => { - expect(canConfirmAgentFromConsolePresence('claude', 'node.exe')).toBe(false) - }) - - it('is false when no agent has been recognized yet (identity must be established first)', () => { - expect(canConfirmAgentFromConsolePresence(null, 'powershell.exe')).toBe(false) - }) - - it('is false when there is no fallback process name', () => { - expect(canConfirmAgentFromConsolePresence('claude', null)).toBe(false) - }) -}) diff --git a/src/main/providers/windows-console-foreground.ts b/src/main/providers/windows-console-foreground.ts deleted file mode 100644 index 77b5bfe4f6d..00000000000 --- a/src/main/providers/windows-console-foreground.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { isShellProcess } from '../../shared/shell-process-detection' - -/** Whether ConPTY membership can revalidate a cached agent without a process scan. */ -export function canConfirmAgentFromConsolePresence( - cachedAgentName: string | null, - fallbackProcess: string | null -): boolean { - return ( - cachedAgentName !== null && - fallbackProcess !== null && - // Why: a generic wrapper may outlive the agent; only the shell fallback is - // the known unreliable Windows exit signal this cache is allowed to bridge. - isShellProcess(fallbackProcess) - ) -} diff --git a/src/main/providers/windows-foreground-process-rows.ts b/src/main/providers/windows-foreground-process-rows.ts index 6c7533ecc56..f5a74bd0fe2 100644 --- a/src/main/providers/windows-foreground-process-rows.ts +++ b/src/main/providers/windows-foreground-process-rows.ts @@ -40,6 +40,23 @@ export async function queryWindowsProcessDescendants( rootPid: number, options: { fresh?: boolean } = {} ): Promise { + return (await queryWindowsPaneProcessInventory(rootPid, options))?.candidates ?? null +} + +export type WindowsPaneProcessInventory = { + candidates: WindowsProcessCandidate[] + /** + * Full-table row for `anchorPid`. From the whole snapshot, not the ppid + * projection: a pane-job member whose creator exited is orphaned out of the + * descendant walk yet can still hold a recycled anchor pid. + */ + anchorRow: WindowsProcessRow | null +} + +export async function queryWindowsPaneProcessInventory( + rootPid: number, + options: { fresh?: boolean; anchorPid?: number } = {} +): Promise { let rows: WindowsProcessRow[] try { const native = @@ -55,7 +72,13 @@ export async function queryWindowsProcessDescendants( if (!rows.some((row) => row.pid === rootPid)) { return null } - return collectDescendants(rows, rootPid).sort((a, b) => b.depth - a.depth) + return { + candidates: collectDescendants(rows, rootPid).sort((a, b) => b.depth - a.depth), + anchorRow: + options.anchorPid !== undefined + ? (rows.find((row) => row.pid === options.anchorPid) ?? null) + : null + } } /** Test-only: clear the shared snapshot so one case's rows never serve the next. */ diff --git a/src/main/providers/windows-pty-job-membership.test.ts b/src/main/providers/windows-pty-job-membership.test.ts new file mode 100644 index 00000000000..8c8ab42122e --- /dev/null +++ b/src/main/providers/windows-pty-job-membership.test.ts @@ -0,0 +1,107 @@ +import type * as ChildProcess from 'node:child_process' +import { describe, expect, it, vi } from 'vitest' +import type { IPty } from 'node-pty' + +// Module-level, so it intercepts the module's own import binding. A spyOn of a +// require()'d child_process does not: the first version of this test passed +// even with a fork() reintroduced, which is the failure it exists to catch. +const forkMock = vi.hoisted(() => vi.fn()) +vi.mock('node:child_process', async (importOriginal) => ({ + ...(await importOriginal()), + fork: forkMock +})) + +import { readWindowsPtyJobProcessIds } from './windows-pty-job-membership' + +const pty = (pid = 100): IPty => ({ pid }) as unknown as IPty + +describe('readWindowsPtyJobProcessIds', () => { + it('never spawns a child process to answer', () => { + // The whole point. node-pty answers console membership by FORKING a helper, + // and Orca asked on a foreground poll, per pane -- hundreds of hidden + // conpty_console_list_agent processes until the machine ran out of memory + // (#10857). Killing them changed nothing; the next poll spawned more. + // QueryInformationJobObject needs no console attachment, so this is one + // syscall and zero children. + const listJobProcessIds = vi.fn(() => [100, 200]) + forkMock.mockClear() + + for (let read = 0; read < 50; read += 1) { + readWindowsPtyJobProcessIds(pty(), listJobProcessIds) + } + + expect(forkMock).not.toHaveBeenCalled() + expect(listJobProcessIds).toHaveBeenCalledTimes(50) + }) + + it('reports the shell alone, which is what lets a stale agent be retired', () => { + const membership = readWindowsPtyJobProcessIds(pty(), () => [100]) + + expect(membership).toEqual(new Set([100])) + expect(membership?.size).toBe(1) + }) + + it('reports descendants, which is what keeps a live agent cached', () => { + const membership = readWindowsPtyJobProcessIds(pty(), () => [100, 200, 300]) + + expect(membership?.size).toBe(3) + }) + + it.each([ + ['no job support or an untracked tree', null], + ['an empty job, which is not the shell-alone case', []] + ])('reports unverifiable for %s', (_case, pids) => { + // null is never evidence that processes died + // (docs/reference/ssh-execution-boundary.md). An empty list means the tree + // is gone, which this function has never been the one to report. + expect(readWindowsPtyJobProcessIds(pty(), () => pids)).toBeNull() + }) + + it('drops nonsense pids rather than trusting the whole answer', () => { + const membership = readWindowsPtyJobProcessIds(pty(), () => [100, 0, -1, 1.5, 200]) + + expect(membership).toEqual(new Set([100, 200])) + }) +}) + +describe('why the filter does NOT use this', () => { + it('refuses an answer that does not contain the shell', () => { + // Shell exited, a descendant is still up. Size 1 -- but reading that as + // "the shell is alone, retire the agent" inverts the truth. The forked + // probe this replaced required the root in the raw list; so does this. + const membership = readWindowsPtyJobProcessIds(pty(100), () => [200]) + + expect(membership).toBeNull() + }) + + it('is asked with the pty handle, because a bare pid cannot find the job', () => { + // ptyJobTarget reads node-pty's private `_pty` id off the object and pairs + // it with proc.pid; the native side refuses on a mismatch. Passing proc.pid + // instead of proc types fine at some call sites but makes every pane report + // unverifiable, so pin the argument. + const listJobProcessIds = vi.fn(() => [100]) + const proc = pty(100) + + readWindowsPtyJobProcessIds(proc, listJobProcessIds) + + expect(listJobProcessIds).toHaveBeenCalledWith(proc) + expect(listJobProcessIds).not.toHaveBeenCalledWith(100) + }) + + it('documents that job membership keeps console-detached descendants', () => { + // The candidate filter in windows-agent-foreground-process.ts exists to DROP + // a descendant that left the console (`Start-Process droid`, a GUI child). + // The job still contains those by design, so answering that filter from the + // job would re-admit exactly what it is for -- granting byte authority to a + // pane no agent owns, or making an attached agent look ambiguous. + // docs/windows-wsl-root-cause-plan.html calls this out as "Use B". + // + // Measured on Windows 11 against a real WSL pane: job [40980,104068,4888,69908] + // vs console [69908,40980] -- the job is a superset. Harmless for the + // `size > 1` callers, wrong for the filter. + const detachedChild = 104068 + const membership = readWindowsPtyJobProcessIds(pty(40980), () => [40980, detachedChild]) + + expect(membership?.has(detachedChild)).toBe(true) + }) +}) diff --git a/src/main/providers/windows-pty-job-membership.ts b/src/main/providers/windows-pty-job-membership.ts new file mode 100644 index 00000000000..f812ad41312 --- /dev/null +++ b/src/main/providers/windows-pty-job-membership.ts @@ -0,0 +1,34 @@ +import type { IPty } from 'node-pty' +import { listPtyJobProcessIds } from '../windows/windows-pty-job' + +/** + * Processes still running under a pane, or null when there is no answer. + * + * Read from the pane's Win32 job object. `GetConsoleProcessList` would need + * console attachment, which is why node-pty answers it by forking a helper, and + * why asking on a foreground poll exhausted memory (#10857). + * `QueryInformationJobObject` has no such constraint: one syscall, no children. + * + * The job is a SUPERSET of the console -- it keeps console-detached descendants + * -- so `size > 1` is not proof of life, only absence of proof of absence. + * `size === 1` (the shell alone) is decisive, and so is membership of a KNOWN + * pid: the list is complete when non-null and a job is inescapable once joined, + * so an anchored identity can be confirmed or retired exactly + * (judgeCachedAgentJobEvidence). Callers must bound only the unanchored rest. + * + * Null means unverifiable per docs/reference/ssh-execution-boundary.md, never + * that processes died. + */ +export function readWindowsPtyJobProcessIds( + proc: IPty, + listJobProcessIds: (proc: IPty) => readonly number[] | null = listPtyJobProcessIds +): ReadonlySet | null { + const pids = listJobProcessIds(proc) + if (!pids) { + return null + } + const membership = new Set(pids.filter((pid) => Number.isSafeInteger(pid) && pid > 0)) + // Without the shell, a size-1 set would read as "shell alone, retire" when it + // means the opposite. The forked probe this replaced refused the same way. + return membership.has(proc.pid) ? membership : null +} diff --git a/src/shared/child-process/__fixtures__/child-process-import-allowlist.txt b/src/shared/child-process/__fixtures__/child-process-import-allowlist.txt index ba80b405cca..dd12763f3b8 100644 --- a/src/shared/child-process/__fixtures__/child-process-import-allowlist.txt +++ b/src/shared/child-process/__fixtures__/child-process-import-allowlist.txt @@ -118,7 +118,7 @@ src/main/plugins/plugin-host-process.ts src/main/ports/port-scan-command-execution.ts src/main/providers/macos-login-session-pty-probe.ts src/main/providers/process-cwd.ts -src/main/providers/windows-conpty-process-membership.ts +src/main/providers/windows-console-attached-processes.ts src/main/pty-descendant-termination.ts src/main/pty/posix-pty-foreground-group.ts src/main/pty/posix-pty-process-groups.ts diff --git a/src/shared/foreground-wrapper-agent.test.ts b/src/shared/foreground-wrapper-agent.test.ts index ab6e1147a6a..c06684e75af 100644 --- a/src/shared/foreground-wrapper-agent.test.ts +++ b/src/shared/foreground-wrapper-agent.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it } from 'vitest' -import { resolveOuterWrapperForegroundProcess } from './foreground-wrapper-agent' +import { + resolveOuterWrapperForegroundIdentity, + resolveOuterWrapperForegroundProcess +} from './foreground-wrapper-agent' describe('resolveOuterWrapperForegroundProcess', () => { const omp = { agent: 'omp' as const, processName: 'omp' } @@ -52,4 +55,22 @@ describe('resolveOuterWrapperForegroundProcess', () => { ]) ).toBe('pi') }) + + it('carries the wrapper pid with a collapsed name, so liveness tracks the wrapper', () => { + // Anchoring 'omp' to pi's pid would read a pi restart as omp's exit. + expect( + resolveOuterWrapperForegroundIdentity(pi, { pid: 102, ppid: 101, command: 'pi' }, [ + { pid: 102, ppid: 101, command: 'pi' }, + { pid: 101, ppid: 100, command: 'omp' } + ]) + ).toEqual({ processName: 'omp', processId: 101 }) + }) + + it('keeps the winner pid when nothing collapses', () => { + const barePi = { pid: 101, ppid: 100, command: 'pi' } + expect(resolveOuterWrapperForegroundIdentity(pi, barePi, [barePi])).toEqual({ + processName: 'pi', + processId: 101 + }) + }) }) diff --git a/src/shared/foreground-wrapper-agent.ts b/src/shared/foreground-wrapper-agent.ts index 9e3421d5526..583df2d4706 100644 --- a/src/shared/foreground-wrapper-agent.ts +++ b/src/shared/foreground-wrapper-agent.ts @@ -1,4 +1,5 @@ import { + recognizeAgentProcess, recognizeAgentProcessFromCommandLine, type RecognizedAgentProcess } from './agent-process-recognition' @@ -18,6 +19,12 @@ export function shouldInspectOuterWrapperForegroundProcess( return process.agent === 'pi' } +/** Same gate for a bare process name, recognizing it first. */ +export function shouldInspectOuterWrapperForegroundName(processName: string | null): boolean { + const recognized = recognizeAgentProcess(processName) + return recognized !== null && shouldInspectOuterWrapperForegroundProcess(recognized) +} + /** * Collapse a foreground read onto its outermost same-title-group ancestor. * Why: OMP embeds Pi, while depth alone cannot distinguish wrappers from sibling jobs. @@ -27,13 +34,28 @@ export function resolveOuterWrapperForegroundProcess( winnerCandidate: ForegroundAgentCandidate, descendants: readonly ForegroundAgentCandidate[] ): string { + return resolveOuterWrapperForegroundIdentity(winner, winnerCandidate, descendants).processName +} + +/** + * Same collapse, keeping the pid of the process the name belongs to. + * Why: a liveness anchor must follow the REPORTED process — anchoring the + * outer wrapper's name to the embedded leaf's pid reads the leaf's exit as + * the wrapper's. + */ +export function resolveOuterWrapperForegroundIdentity( + winner: RecognizedAgentProcess, + winnerCandidate: ForegroundAgentCandidate, + descendants: readonly ForegroundAgentCandidate[] +): { processName: string; processId: number } { const winnerGroup = getSyntheticAgentTitleProfile(winner.agent)?.titleIdentityGroup if (!winnerGroup) { - return winner.processName + return { processName: winner.processName, processId: winnerCandidate.pid } } const candidatesByPid = new Map(descendants.map((candidate) => [candidate.pid, candidate])) const seen = new Set([winnerCandidate.pid]) let outerProcessName = winner.processName + let outerProcessId = winnerCandidate.pid let parentPid = winnerCandidate.ppid while (!seen.has(parentPid)) { seen.add(parentPid) @@ -49,8 +71,9 @@ export function resolveOuterWrapperForegroundProcess( getSyntheticAgentTitleProfile(recognized.agent)?.titleIdentityGroup === winnerGroup ) { outerProcessName = recognized.processName + outerProcessId = candidate.pid } parentPid = candidate.ppid } - return outerProcessName + return { processName: outerProcessName, processId: outerProcessId } }