From fe278b48a8cfd20bcbd7c90ae2f133d987423a23 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 16 Jul 2026 01:19:48 -0700 Subject: [PATCH] fix(ai-vault): preserve execution boundaries and reap children --- .../ai-vault/codex-session-root-dedup.test.ts | 39 ++++++++++++- src/main/ai-vault/codex-session-root-dedup.ts | 17 +++++- .../codex/codex-app-server-client.test.ts | 56 ++++++++++++++++++- src/main/codex/codex-app-server-session.ts | 44 +++++++++++++-- 4 files changed, 146 insertions(+), 10 deletions(-) diff --git a/src/main/ai-vault/codex-session-root-dedup.test.ts b/src/main/ai-vault/codex-session-root-dedup.test.ts index dcff2f0143c..9121249ad5e 100644 --- a/src/main/ai-vault/codex-session-root-dedup.test.ts +++ b/src/main/ai-vault/codex-session-root-dedup.test.ts @@ -71,8 +71,8 @@ describe('dedupeCodexRolloutFileAliases', () => { it('prefers the managed runtime home over other non-default homes', () => { const managed = { agent: 'codex', - path: MANAGED_HOME_ROLLOUT, - codexHome: MANAGED_HOME, + path: `\\\\wsl$\\Ubuntu\\home\\ada\\.local\\share\\orca\\codex-runtime-home\\home\\sessions\\2026\\07\\01\\rollout-2026-07-01T10-00-00-019f0000-1111-7222-8333-444444444444.jsonl`, + codexHome: '\\\\wsl$\\Ubuntu\\home\\ada\\.local\\share\\orca\\codex-runtime-home\\home', hardlinkIdentity: '1:42' } const wslReal = { @@ -149,6 +149,24 @@ describe('dedupeCodexRolloutFileAliases', () => { unprovenCopy ]) }) + + it('never treats matching host and WSL inode tuples as one hardlink', () => { + const rolloutName = REAL_HOME_ROLLOUT.split('/').at(-1) + const host = { + agent: 'codex', + path: `C:\\Users\\ada\\.codex\\sessions\\${rolloutName}`, + codexHome: null, + hardlinkIdentity: '1:42' + } + const wsl = { + agent: 'codex', + path: `\\\\wsl$\\Ubuntu\\home\\ada\\.codex\\sessions\\${rolloutName}`, + codexHome: '\\\\wsl$\\Ubuntu\\home\\ada\\.codex', + hardlinkIdentity: '1:42' + } + + expect(dedupeCodexRolloutFileAliases([host, wsl], accessors)).toEqual([host, wsl]) + }) }) describe('dedupeCodexSessionsBySessionId', () => { @@ -247,4 +265,21 @@ describe('dedupeCodexSessionsBySessionId', () => { }) expect(dedupeCodexSessionsBySessionId([wslReal, wslManaged])).toEqual([wslManaged]) }) + + it('never collapses matching host and WSL session identities', () => { + const rolloutName = REAL_HOME_ROLLOUT.split('/').at(-1) + const host = codexSession({ + sessionId: 'shared-id', + filePath: `C:\\Users\\ada\\.codex\\sessions\\${rolloutName}`, + codexHome: null + }) + const wsl = codexSession({ + sessionId: 'shared-id', + filePath: `\\\\wsl.localhost\\Ubuntu\\home\\ada\\.local\\share\\orca\\codex-runtime-home\\home\\sessions\\${rolloutName}`, + codexHome: + '\\\\wsl.localhost\\Ubuntu\\home\\ada\\.local\\share\\orca\\codex-runtime-home\\home' + }) + + expect(dedupeCodexSessionsBySessionId([host, wsl])).toEqual([host, wsl]) + }) }) diff --git a/src/main/ai-vault/codex-session-root-dedup.ts b/src/main/ai-vault/codex-session-root-dedup.ts index b54434eb2d1..0f4a39cf8d2 100644 --- a/src/main/ai-vault/codex-session-root-dedup.ts +++ b/src/main/ai-vault/codex-session-root-dedup.ts @@ -1,4 +1,5 @@ import type { AiVaultSession } from '../../shared/ai-vault-types' +import { parseWslUncPath } from '../../shared/wsl-paths' import { sessionSortTime } from './session-scanner-accumulator' // Why: the session bridge and the real-home backfill hardlink one physical @@ -17,6 +18,14 @@ function lastPathSegment(filePath: string): string { return filePath.split(/[\\/]/).at(-1) ?? '' } +// Why: local Windows discovery scans both the host and every WSL distro under +// one `local` host id, even though hardlinks and resume identity cannot cross +// those execution boundaries. +function codexPathExecutionNamespace(filePath: string): string { + const wslPath = parseWslUncPath(filePath) + return wslPath ? `wsl:${wslPath.distro.toLowerCase()}` : 'native' +} + /** Returns a pre-parse alias key only when metadata proves a shared hardlink. */ export function codexRolloutHardlinkIdentity(file: { dev?: number @@ -84,7 +93,7 @@ export function dedupeCodexRolloutFileAliases( if (!hardlinkIdentity) { continue } - const aliasKey = `${fileName}\0${hardlinkIdentity}` + const aliasKey = `${codexPathExecutionNamespace(filePath)}\0${fileName}\0${hardlinkIdentity}` const rank = codexSessionRootRank(accessors.getCodexHome(candidate)) const best = bestByAlias.get(aliasKey) if (!best || rank < best.rank || (rank === best.rank && filePath < best.filePath)) { @@ -100,7 +109,9 @@ export function dedupeCodexRolloutFileAliases( if (!hardlinkIdentity) { return true } - const best = bestByAlias.get(`${fileName}\0${hardlinkIdentity}`) + const best = bestByAlias.get( + `${codexPathExecutionNamespace(accessors.getFilePath(candidate))}\0${fileName}\0${hardlinkIdentity}` + ) return !best || best.candidate === candidate }) } @@ -142,7 +153,7 @@ function codexSessionAliasKey(session: AiVaultSession): string | null { if (!CODEX_ROLLOUT_FILE_NAME_PATTERN.test(fileName)) { return null } - return `${session.executionHostId}\0${session.sessionId}\0${fileName}` + return `${session.executionHostId}\0${codexPathExecutionNamespace(session.filePath)}\0${session.sessionId}\0${fileName}` } function codexSessionAliasBeats(candidate: AiVaultSession, best: AiVaultSession): boolean { diff --git a/src/main/codex/codex-app-server-client.test.ts b/src/main/codex/codex-app-server-client.test.ts index 38f6e1405d6..879fcc5cab3 100644 --- a/src/main/codex/codex-app-server-client.test.ts +++ b/src/main/codex/codex-app-server-client.test.ts @@ -1,3 +1,5 @@ +import { EventEmitter } from 'node:events' +import type { ChildProcess, spawn } from 'node:child_process' import { afterEach, describe, expect, it, vi } from 'vitest' import { mkdtempSync, readFileSync, rmSync, writeFileSync, existsSync } from 'node:fs' import { tmpdir } from 'node:os' @@ -9,7 +11,7 @@ import { runCodexHookTrustGrantSession, type CodexHookTrustGrantRequest } from './codex-app-server-client' -import { runCodexAppServerSession } from './codex-app-server-session' +import { killCodexAppServerProcessTree, runCodexAppServerSession } from './codex-app-server-session' import { resolveCodexGrantEntryPath, runCodexHookTrustGrantSessionSync @@ -149,6 +151,58 @@ function managedHook(key: string, trustStatus = 'untrusted'): StubHook { return { key, command: MANAGED_COMMAND, currentHash: `sha256:hash-of-${key}`, trustStatus } } +describe('killCodexAppServerProcessTree', () => { + it('kills the Windows wrapper and all app-server descendants', () => { + const child = { + pid: 1234, + kill: vi.fn(() => true) as ChildProcess['kill'] + } + const killer = new EventEmitter() as EventEmitter & { unref: ReturnType } + killer.unref = vi.fn() + const spawnImpl = vi.fn(() => killer) as unknown as typeof spawn + + killCodexAppServerProcessTree(child, { platform: 'win32', spawnImpl }) + + expect(spawnImpl).toHaveBeenCalledWith('taskkill', ['/pid', '1234', '/t', '/f'], { + stdio: 'ignore', + windowsHide: true + }) + expect(killer.unref).toHaveBeenCalledOnce() + expect(child.kill).not.toHaveBeenCalled() + + killer.emit('error', new Error('taskkill unavailable')) + expect(child.kill).toHaveBeenCalledWith('SIGKILL') + }) + + it('falls back when taskkill starts but cannot terminate the process tree', () => { + const child = { + pid: 1234, + kill: vi.fn(() => true) as ChildProcess['kill'] + } + const killer = new EventEmitter() as EventEmitter & { unref: ReturnType } + killer.unref = vi.fn() + const spawnImpl = vi.fn(() => killer) as unknown as typeof spawn + + killCodexAppServerProcessTree(child, { platform: 'win32', spawnImpl }) + killer.emit('exit', 1) + + expect(child.kill).toHaveBeenCalledWith('SIGKILL') + }) + + it('kills the direct app-server process on non-Windows hosts', () => { + const child = { + pid: 1234, + kill: vi.fn(() => true) as ChildProcess['kill'] + } + const spawnImpl = vi.fn() as unknown as typeof spawn + + killCodexAppServerProcessTree(child, { platform: 'linux', spawnImpl }) + + expect(spawnImpl).not.toHaveBeenCalled() + expect(child.kill).toHaveBeenCalledWith('SIGKILL') + }) +}) + describe('runCodexHookTrustGrantSession', () => { it('grants and verifies exactly the expected managed entries', async () => { const setTimeoutSpy = vi.spyOn(globalThis, 'setTimeout') diff --git a/src/main/codex/codex-app-server-session.ts b/src/main/codex/codex-app-server-session.ts index 6fbf202ac28..9c332729958 100644 --- a/src/main/codex/codex-app-server-session.ts +++ b/src/main/codex/codex-app-server-session.ts @@ -1,4 +1,4 @@ -import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process' +import { spawn, type ChildProcess, type ChildProcessWithoutNullStreams } from 'node:child_process' import { waitForProcessExitUntil } from './codex-process-exit-deadline' // Why: `codex app-server` is Orca's sanctioned RPC surface into Codex-owned @@ -51,6 +51,42 @@ const JSON_RPC_METHOD_NOT_FOUND = -32601 const STDERR_TAIL_MAX_BYTES = 8192 const STDOUT_LINE_MAX_BYTES = 1024 * 1024 +export function killCodexAppServerProcessTree( + child: Pick, + options: { platform?: NodeJS.Platform; spawnImpl?: typeof spawn } = {} +): void { + const platform = options.platform ?? process.platform + const spawnImpl = options.spawnImpl ?? spawn + if (platform === 'win32' && child.pid) { + try { + // Why: npm-installed Codex runs behind cmd.exe; killing only that wrapper + // leaves the app-server child alive after a timeout or failed shutdown. + const killer = spawnImpl('taskkill', ['/pid', String(child.pid), '/t', '/f'], { + stdio: 'ignore', + windowsHide: true + }) + let fellBack = false + const killDirectChild = (): void => { + if (!fellBack) { + fellBack = true + child.kill('SIGKILL') + } + } + killer.on('error', killDirectChild) + killer.on('exit', (code) => { + if (code !== 0) { + killDirectChild() + } + }) + killer.unref() + return + } catch { + // Fall through to the direct-child best effort when taskkill cannot start. + } + } + child.kill('SIGKILL') +} + function isMethodNotFoundError(error: { code?: number; message?: string }): boolean { return error.code === JSON_RPC_METHOD_NOT_FOUND || /method not found/i.test(error.message ?? '') } @@ -120,7 +156,7 @@ export async function runCodexAppServerSession( child.stdout.setEncoding('utf8').on('data', (chunk: string) => { stdoutBuffer += chunk if (Buffer.byteLength(stdoutBuffer) > STDOUT_LINE_MAX_BYTES) { - child.kill('SIGKILL') + killCodexAppServerProcessTree(child) failPending(new Error('codex app-server emitted an oversized JSONL response')) return } @@ -161,7 +197,7 @@ export async function runCodexAppServerSession( const error = new CodexAppServerTimeoutError( `codex app-server session exceeded ${invocation.timeoutMs}ms (${invocation.command})` ) - child.kill('SIGKILL') + killCodexAppServerProcessTree(child) failPending(error) rejectDeadline(error) }, invocation.timeoutMs) @@ -264,7 +300,7 @@ export async function runCodexAppServerSession( // bounds a wedged child before the guaranteed SIGKILL reap. await waitForProcessExitUntil(exitPromise, 1500) if (!exited) { - child.kill('SIGKILL') + killCodexAppServerProcessTree(child) await waitForProcessExitUntil(exitPromise, 1000) } }