From 00001f1fd74ac9fac9323ccd49b5be7294eeb6af Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 3 Oct 2026 01:46:17 -0700 Subject: [PATCH 1/4] Move provider process supervision and stream reading out of Codex --- config/scripts/ci-shard-timings.json | 4 ++-- config/scripts/pr-code-change-scope.mjs | 2 +- config/scripts/pr-code-change-scope.test.mjs | 2 +- config/tsconfig.cli.json | 4 ++-- .../claude-agent-sdk-process-spawn.test.ts | 8 +++---- .../claude/claude-agent-sdk-process-spawn.ts | 2 +- .../claude-child-exit-proof-ladder.test.ts | 2 +- .../claude/claude-child-exit-proof-ladder.ts | 4 ++-- ...claude-supervised-stop.integration.test.ts | 2 +- .../codex/codex-app-server-connection.test.ts | 2 +- src/main/codex/codex-app-server-connection.ts | 24 +++++++------------ src/main/codex/codex-app-server-jsonl.ts | 12 ---------- .../codex/codex-app-server-record-dispatch.ts | 6 ++--- src/main/codex/codex-app-server-session.ts | 6 ++--- ...ctured-agent-session-host-teardown.test.ts | 2 +- .../structured-agent-session-host-teardown.ts | 2 +- ...-agent-session-recovery-resolution.test.ts | 2 +- ...tured-agent-session-recovery-resolution.ts | 2 +- .../provider-process/provider-json-record.ts | 3 +++ .../provider-process-exit-deadline.ts} | 2 +- .../provider-process-launch.ts | 10 ++++++++ ...er-process-supervisor.integration.test.ts} | 2 +- .../provider-process-supervisor.test.ts} | 6 ++--- .../provider-process-supervisor.ts} | 6 ++--- .../provider-process-teardown.test.ts} | 18 +++++++------- .../provider-process-teardown.ts} | 17 ++++++------- .../provider-record-reader.ts} | 6 ++--- .../child-process-import-allowlist.txt | 4 ++-- 28 files changed, 76 insertions(+), 86 deletions(-) delete mode 100644 src/main/codex/codex-app-server-jsonl.ts create mode 100644 src/main/provider-process/provider-json-record.ts rename src/main/{codex/codex-process-exit-deadline.ts => provider-process/provider-process-exit-deadline.ts} (86%) create mode 100644 src/main/provider-process/provider-process-launch.ts rename src/main/{codex/codex-app-server-posix-supervisor.integration.test.ts => provider-process/provider-process-supervisor.integration.test.ts} (99%) rename src/main/{codex/codex-app-server-posix-supervisor.test.ts => provider-process/provider-process-supervisor.test.ts} (94%) rename src/main/{codex/codex-app-server-posix-supervisor.ts => provider-process/provider-process-supervisor.ts} (98%) rename src/main/{codex/codex-app-server-process-teardown.test.ts => provider-process/provider-process-teardown.test.ts} (90%) rename src/main/{codex/codex-app-server-process-teardown.ts => provider-process/provider-process-teardown.ts} (90%) rename src/main/{codex/codex-app-server-record-reader.ts => provider-process/provider-record-reader.ts} (90%) diff --git a/config/scripts/ci-shard-timings.json b/config/scripts/ci-shard-timings.json index 73d0226943b8..a56932a44795 100644 --- a/config/scripts/ci-shard-timings.json +++ b/config/scripts/ci-shard-timings.json @@ -1211,8 +1211,8 @@ "src/main/codex/codex-app-server-capability-cache.test.ts": 64, "src/main/codex/codex-app-server-client.test.ts": 1502, "src/main/codex/codex-app-server-connection.test.ts": 5199, - "src/main/codex/codex-app-server-posix-supervisor.test.ts": 38, - "src/main/codex/codex-app-server-process-teardown.test.ts": 71, + "src/main/provider-process/provider-process-supervisor.test.ts": 38, + "src/main/provider-process/provider-process-teardown.test.ts": 71, "src/main/codex/codex-app-server-session.test.ts": 129, "src/main/codex/codex-app-server-teardown.integration.test.ts": 21677, "src/main/codex/codex-background-command-tracker.test.ts": 221, diff --git a/config/scripts/pr-code-change-scope.mjs b/config/scripts/pr-code-change-scope.mjs index 800854c8008f..6064ff38a2fb 100644 --- a/config/scripts/pr-code-change-scope.mjs +++ b/config/scripts/pr-code-change-scope.mjs @@ -70,7 +70,7 @@ const CODEX_INDEX_HEAL_CONTRACT_PREFIXES = [ 'src/main/codex/codex-state-db', 'src/main/sqlite/sync-database', 'src/main/codex/codex-app-server-capability-signal', - 'src/main/codex/codex-process-exit-deadline', + 'src/main/provider-process/provider-process-exit-deadline', 'src/main/codex/codex-session-backfill', 'src/main/codex/codex-session-index-heal-state', 'src/main/codex-cli/command', diff --git a/config/scripts/pr-code-change-scope.test.mjs b/config/scripts/pr-code-change-scope.test.mjs index 297d1f8e6713..28fd0194a3b5 100644 --- a/config/scripts/pr-code-change-scope.test.mjs +++ b/config/scripts/pr-code-change-scope.test.mjs @@ -169,7 +169,7 @@ describe('per-job path classification', () => { // Keep the real-binary gate live when a transport or launch dependency changes. for (const file of [ 'src/main/codex/codex-app-server-capability-signal.ts', - 'src/main/codex/codex-process-exit-deadline.ts', + 'src/main/provider-process/provider-process-exit-deadline.ts', 'src/main/codex/codex-session-backfill.ts', 'src/main/codex/codex-session-index-heal-state.ts', 'src/main/codex-cli/command.ts', diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index 44edcfd90b7e..0bd07d1b6c15 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -118,7 +118,7 @@ "../src/main/codex/codex-app-server-capability-signal.ts", "../src/main/codex/codex-app-server-client.ts", "../src/main/codex/codex-app-server-process-tree-kill.ts", - "../src/main/codex/codex-app-server-record-reader.ts", + "../src/main/provider-process/provider-record-reader.ts", "../src/main/codex/codex-app-server-session.ts", "../src/main/codex/codex-config-mirror.ts", "../src/main/codex/codex-config-mirror-merge.ts", @@ -149,7 +149,7 @@ "../src/main/codex/codex-hook-retired-commands.ts", "../src/main/codex/codex-hook-wsl-runtime.ts", "../src/main/codex/codex-managed-trust-reconciliation.ts", - "../src/main/codex/codex-process-exit-deadline.ts", + "../src/main/provider-process/provider-process-exit-deadline.ts", "../src/main/codex/codex-real-home-background-grant.ts", "../src/main/codex/codex-real-home-entry-trust.ts", "../src/main/codex/codex-real-home-hook-entry-plan.ts", diff --git a/src/main/claude/claude-agent-sdk-process-spawn.test.ts b/src/main/claude/claude-agent-sdk-process-spawn.test.ts index 1b6c192d433a..af1da7cd8bf0 100644 --- a/src/main/claude/claude-agent-sdk-process-spawn.test.ts +++ b/src/main/claude/claude-agent-sdk-process-spawn.test.ts @@ -4,12 +4,12 @@ import { describe, expect, it, vi } from 'vitest' import type { SpawnOptions as SdkSpawnOptions } from '@anthropic-ai/claude-agent-sdk' import { resolveSpawn, type spawnProcess } from '../../shared/child-process/run-process' import type { ProcessSpec } from '../../shared/child-process/process-spec' -import type * as ProviderSupervisor from '../codex/codex-app-server-posix-supervisor' -import { createProviderSpawnSpec } from '../codex/codex-app-server-posix-supervisor' +import type * as ProviderSupervisor from '../provider-process/provider-process-supervisor' +import { createProviderSpawnSpec } from '../provider-process/provider-process-supervisor' import { createClaudeCodeProcessSpawn } from './claude-agent-sdk-process-spawn' import { proveClaudeChildExitWithReaper } from './claude-child-exit-proof-ladder' -vi.mock('../codex/codex-app-server-posix-supervisor', async (importOriginal) => { +vi.mock('../provider-process/provider-process-supervisor', async (importOriginal) => { const actual = await importOriginal() return { ...actual, createProviderSpawnSpec: vi.fn(actual.createProviderSpawnSpec) } }) @@ -110,7 +110,7 @@ describe('claude agent SDK process spawn', () => { 'stops Claude by the spawn spec\u2019s supervision on $platform, never the platform', async ({ platform, specSupervised }) => { const actual = await vi.importActual( - '../codex/codex-app-server-posix-supervisor' + '../provider-process/provider-process-supervisor' ) vi.mocked(createProviderSpawnSpec).mockImplementationOnce((...args) => ({ ...actual.createProviderSpawnSpec(...args), diff --git a/src/main/claude/claude-agent-sdk-process-spawn.ts b/src/main/claude/claude-agent-sdk-process-spawn.ts index bbb4b295aade..96fefa0e275b 100644 --- a/src/main/claude/claude-agent-sdk-process-spawn.ts +++ b/src/main/claude/claude-agent-sdk-process-spawn.ts @@ -1,6 +1,6 @@ import type { SpawnOptions as ClaudeAgentSdkSpawnOptions } from '@anthropic-ai/claude-agent-sdk' import { spawnProcess } from '../../shared/child-process/run-process' -import { createProviderSpawnSpec } from '../codex/codex-app-server-posix-supervisor' +import { createProviderSpawnSpec } from '../provider-process/provider-process-supervisor' /** Derived rather than imported: only src/shared/child-process may name node:child_process. */ type ClaudeCodeChild = ReturnType diff --git a/src/main/claude/claude-child-exit-proof-ladder.test.ts b/src/main/claude/claude-child-exit-proof-ladder.test.ts index 2d8da7f629dc..6a74a0b0c6b6 100644 --- a/src/main/claude/claude-child-exit-proof-ladder.test.ts +++ b/src/main/claude/claude-child-exit-proof-ladder.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../codex/codex-app-server-posix-supervisor' +import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../provider-process/provider-process-supervisor' import type { ClaudeChildTreeReaper } from './claude-agent-sdk-exit-proof' import { proveClaudeChildExitWithReaper } from './claude-child-exit-proof-ladder' diff --git a/src/main/claude/claude-child-exit-proof-ladder.ts b/src/main/claude/claude-child-exit-proof-ladder.ts index 37d4e47508d7..41015b0192f0 100644 --- a/src/main/claude/claude-child-exit-proof-ladder.ts +++ b/src/main/claude/claude-child-exit-proof-ladder.ts @@ -1,6 +1,6 @@ import type { SpawnedProcess } from '../../shared/child-process/run-process' -import { waitForProcessExitUntil } from '../codex/codex-process-exit-deadline' -import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../codex/codex-app-server-posix-supervisor' +import { waitForProcessExitUntil } from '../provider-process/provider-process-exit-deadline' +import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../provider-process/provider-process-supervisor' import type { ClaudeChildTreeReaper } from './claude-agent-sdk-exit-proof' export const GRACEFUL_EXIT_MS = 1_500 diff --git a/src/main/claude/claude-supervised-stop.integration.test.ts b/src/main/claude/claude-supervised-stop.integration.test.ts index d0a5f9ea0cc4..6f0749e79e79 100644 --- a/src/main/claude/claude-supervised-stop.integration.test.ts +++ b/src/main/claude/claude-supervised-stop.integration.test.ts @@ -10,7 +10,7 @@ import { PROVIDER_SIGTERM_GRACE_MS, PROVIDER_STDIN_END_GRACE_MS, PROVIDER_SUPERVISOR_MAX_STOP_MS -} from '../codex/codex-app-server-posix-supervisor' +} from '../provider-process/provider-process-supervisor' import { proveClaudeChildExit } from './claude-agent-sdk-exit-proof' import { createClaudeCodeProcessSpawn } from './claude-agent-sdk-process-spawn' diff --git a/src/main/codex/codex-app-server-connection.test.ts b/src/main/codex/codex-app-server-connection.test.ts index d36cfac47a44..a5e279039830 100644 --- a/src/main/codex/codex-app-server-connection.test.ts +++ b/src/main/codex/codex-app-server-connection.test.ts @@ -11,7 +11,7 @@ import { type CodexAppServerConnection, type CodexAppServerConnectionHandlers } from './codex-app-server-connection' -import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from './codex-app-server-posix-supervisor' +import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../provider-process/provider-process-supervisor' import { isCodexAppServerUnsupportedError } from './codex-app-server-session' // close() waits out the supervisor's own stop before forcing the tree. diff --git a/src/main/codex/codex-app-server-connection.ts b/src/main/codex/codex-app-server-connection.ts index 3e7d16ea1bb0..cae5a81aa59d 100644 --- a/src/main/codex/codex-app-server-connection.ts +++ b/src/main/codex/codex-app-server-connection.ts @@ -1,20 +1,21 @@ import { spawnProcess } from '../../shared/child-process/run-process' import { RetryableProcessExitProof } from '../../shared/child-process/retryable-process-exit-proof' +import type { ProviderProcessLaunch } from '../provider-process/provider-process-launch' import { createProviderSpawnSpec, PROVIDER_SUPERVISOR_MAX_STOP_MS -} from './codex-app-server-posix-supervisor' +} from '../provider-process/provider-process-supervisor' import { buildCodexAppServerExitError } from './codex-app-server-exit-error' import { initializeCodexAppServerConnection } from './codex-app-server-handshake' import { CodexAppServerHandshakeExitUnprovenError } from './codex-app-server-handshake-exit-proof' -import { terminateCodexAppServerProcessTree } from './codex-app-server-process-teardown' -import { waitForProcessExitUntil } from './codex-process-exit-deadline' +import { terminateProviderProcessTree } from '../provider-process/provider-process-teardown' +import { waitForProcessExitUntil } from '../provider-process/provider-process-exit-deadline' import { CodexAppServerTimeoutError, CodexAppServerUnsupportedError } from './codex-app-server-session' import { createCodexAppServerRecordDispatcher } from './codex-app-server-record-dispatch' -import { createCodexAppServerRecordReader } from './codex-app-server-record-reader' +import { createProviderRecordReader } from '../provider-process/provider-record-reader' import type { CodexAppServerConnection, CodexAppServerConnectionHandlers @@ -34,16 +35,7 @@ export { CodexAppServerFrameSizeError } from './codex-app-server-frame-size-erro // Structured chat needs a persistent bidirectional child and per-request deadlines; // the request-scoped app-server runner cannot carry approvals or streamed turns. -export type CodexAppServerLaunch = { - command: string - args: string[] - /** Workspace directory used by the provider process itself. */ - cwd?: string - /** Overlay on the inherited environment — the pinned CODEX_HOME lives here. */ - env?: Record - /** Keys stripped after the overlay, matching `CodexAppServerInvocation`. */ - envToDelete?: readonly string[] -} +export type CodexAppServerLaunch = ProviderProcessLaunch const DEFAULT_REQUEST_TIMEOUT_MS = 30_000 export const GRACEFUL_EXIT_MS = 1_500 @@ -70,7 +62,7 @@ export async function openCodexAppServerConnection( function terminateProcessTree(): Promise { // The supervisor and provider own separate POSIX groups so the supervisor can prove the // provider group empty before relaying its exit. Forced wrapper teardown uses descendant proof. - return terminateCodexAppServerProcessTree(child) + return terminateProviderProcessTree(child) } let stderrTail = '' @@ -156,7 +148,7 @@ export async function openCodexAppServerConnection( void terminateProcessTree() }) - const recordReader = createCodexAppServerRecordReader({ + const recordReader = createProviderRecordReader({ stdout: child.stdout, onRecord: (parsed, line) => { if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { diff --git a/src/main/codex/codex-app-server-jsonl.ts b/src/main/codex/codex-app-server-jsonl.ts deleted file mode 100644 index 3a65e1fb1389..000000000000 --- a/src/main/codex/codex-app-server-jsonl.ts +++ /dev/null @@ -1,12 +0,0 @@ -export function isAppServerRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value) -} - -export function parseCodexAppServerJsonLine(line: string): Record | null { - try { - const parsed: unknown = JSON.parse(line) - return isAppServerRecord(parsed) ? parsed : null - } catch { - return null - } -} diff --git a/src/main/codex/codex-app-server-record-dispatch.ts b/src/main/codex/codex-app-server-record-dispatch.ts index cbac80f3643e..313fbda003cd 100644 --- a/src/main/codex/codex-app-server-record-dispatch.ts +++ b/src/main/codex/codex-app-server-record-dispatch.ts @@ -1,7 +1,7 @@ import type { NdjsonRejectedRecord } from '../../shared/main-process-ndjson-framer' import type { CodexAppServerConnectionHandlers } from './codex-app-server-connection-types' import { CodexAppServerFrameSizeError } from './codex-app-server-frame-size-error' -import { isAppServerRecord } from './codex-app-server-jsonl' +import { isProviderRecord } from '../provider-process/provider-json-record' import { CodexAppServerRequestError } from './codex-app-server-request-error' import { CodexAppServerUnsupportedError, @@ -92,7 +92,7 @@ export function createCodexAppServerRecordDispatcher(input: { // interpreted this reply, and it already reported its own outcome. const timedOutMethod = timedOutMethods.get(message.id) timedOutMethods.delete(message.id) - const error = isAppServerRecord(message.error) ? message.error.message : undefined + const error = isProviderRecord(message.error) ? message.error.message : undefined console.warn( timedOutMethod ? `[codex-app-server] late reply to ${timedOutMethod} after timeout (id ${message.id})` @@ -104,7 +104,7 @@ export function createCodexAppServerRecordDispatcher(input: { pending.delete(message.id) clearTimeout(waiter.timer) const error = message.error - if (isAppServerRecord(error)) { + if (isProviderRecord(error)) { const detail = typeof error.message === 'string' ? error.message : 'unknown error' waiter.reject( isCodexMethodNotFoundError(error) diff --git a/src/main/codex/codex-app-server-session.ts b/src/main/codex/codex-app-server-session.ts index 389cd20516cb..8f2bcbecad2b 100644 --- a/src/main/codex/codex-app-server-session.ts +++ b/src/main/codex/codex-app-server-session.ts @@ -1,5 +1,5 @@ import type { ChildProcessWithoutNullStreams } from 'node:child_process' -import { waitForProcessExitUntil } from './codex-process-exit-deadline' +import { waitForProcessExitUntil } from '../provider-process/provider-process-exit-deadline' import { stderrIndicatesMissingAppServer } from './codex-app-server-capability-signal' import { withCliRuntimeOnPath } from '../../shared/node-cli-command-resolution' import { @@ -7,7 +7,7 @@ import { spawnCodexAppServerProcess, type CodexAppServerSpawn } from './codex-app-server-process-tree-kill' -import { createCodexAppServerRecordReader } from './codex-app-server-record-reader' +import { createProviderRecordReader } from '../provider-process/provider-record-reader' // Why: `codex app-server` is Orca's sanctioned RPC surface into Codex-owned // state (hook trust hashes, the sqlite thread index). This module owns the @@ -154,7 +154,7 @@ export async function runCodexAppServerSession( failPending(error) }) - createCodexAppServerRecordReader({ + createProviderRecordReader({ stdout: child.stdout, onRecord: (parsed) => { if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.test.ts index e6f62d6d780c..dde5f82dae59 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.test.ts @@ -5,7 +5,7 @@ import { SUPERVISED_GRACEFUL_EXIT_MS } from '../../claude/claude-child-exit-proof-ladder' import { GRACEFUL_EXIT_MS as CODEX_WINDOWS_GRACEFUL_EXIT_MS } from '../../codex/codex-app-server-connection' -import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../../codex/codex-app-server-posix-supervisor' +import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../../provider-process/provider-process-supervisor' import { SNAPSHOT_DRAIN_TIMEOUT_MS } from './structured-agent-session-eviction' import { CHILD_EVICTION_TIMEOUT_MS, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.ts index 65bf219cf79d..bbae1b4db163 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-teardown.ts @@ -7,7 +7,7 @@ import type { AgentSessionResumeTrigger } from '../../../shared/agent-session-resume-marker' import { SUPERVISED_GRACEFUL_EXIT_MS } from '../../claude/claude-child-exit-proof-ladder' -import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../../codex/codex-app-server-posix-supervisor' +import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../../provider-process/provider-process-supervisor' import { SNAPSHOT_DRAIN_TIMEOUT_MS } from './structured-agent-session-eviction' import type { StructuredAgentSessionRestartResume } from './structured-agent-session-restart-resume-host' import { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.test.ts index 8e3be9d1f3d7..8d44f9708b5f 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.test.ts @@ -10,7 +10,7 @@ import { } from '../../runtime/agent-session-older-build-lease.test-fixture' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' import { openTestAgentSessionRecordStore } from '../../runtime/agent-session-record-store-test-harness' -import { supervisedPosixLaunch } from '../../codex/codex-app-server-posix-supervisor' +import { supervisedPosixLaunch } from '../../provider-process/provider-process-supervisor' import { resolveStructuredSessionRecovery, type StructuredSessionRecoveryResolutionDeps diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts index 385d438f7a04..5dbd9a4071b7 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts @@ -15,7 +15,7 @@ import { type AgentSessionOwnerProbe } from '../../../shared/agent-session-lease-adjudication' import type { AgentSessionRecord } from '../../../shared/agent-session-record' -import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../../codex/codex-app-server-posix-supervisor' +import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../../provider-process/provider-process-supervisor' import { releaseUnprovenAgentSessionOwner } from '../../runtime/agent-session-lease-transitions' import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store' diff --git a/src/main/provider-process/provider-json-record.ts b/src/main/provider-process/provider-json-record.ts new file mode 100644 index 000000000000..cc57d7a36dd3 --- /dev/null +++ b/src/main/provider-process/provider-json-record.ts @@ -0,0 +1,3 @@ +export function isProviderRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} diff --git a/src/main/codex/codex-process-exit-deadline.ts b/src/main/provider-process/provider-process-exit-deadline.ts similarity index 86% rename from src/main/codex/codex-process-exit-deadline.ts rename to src/main/provider-process/provider-process-exit-deadline.ts index d6eaf1eda2cf..40d099a736fd 100644 --- a/src/main/codex/codex-process-exit-deadline.ts +++ b/src/main/provider-process/provider-process-exit-deadline.ts @@ -10,7 +10,7 @@ export async function waitForProcessExitUntil( await Promise.race([exitPromise, timeout]) } finally { // Why: this runs in a short-lived entry; a live grace timer delays the - // parent spawnSync even after the app-server process has already exited. + // parent spawnSync even after the provider process has already exited. if (timer !== undefined) { clearTimeout(timer) } diff --git a/src/main/provider-process/provider-process-launch.ts b/src/main/provider-process/provider-process-launch.ts new file mode 100644 index 000000000000..326e1d17adcb --- /dev/null +++ b/src/main/provider-process/provider-process-launch.ts @@ -0,0 +1,10 @@ +export type ProviderProcessLaunch = { + command: string + args: string[] + /** Workspace directory used by the provider process itself. */ + cwd?: string + /** Overlay on the inherited environment. */ + env?: Record + /** Keys stripped after the overlay. */ + envToDelete?: readonly string[] +} diff --git a/src/main/codex/codex-app-server-posix-supervisor.integration.test.ts b/src/main/provider-process/provider-process-supervisor.integration.test.ts similarity index 99% rename from src/main/codex/codex-app-server-posix-supervisor.integration.test.ts rename to src/main/provider-process/provider-process-supervisor.integration.test.ts index 846afaa973f4..b3b7f068b7d3 100644 --- a/src/main/codex/codex-app-server-posix-supervisor.integration.test.ts +++ b/src/main/provider-process/provider-process-supervisor.integration.test.ts @@ -10,7 +10,7 @@ import { PROVIDER_SUPERVISOR_MAX_STOP_MS, supervisedPosixLaunch, type ProviderSupervisorOptions -} from './codex-app-server-posix-supervisor' +} from './provider-process-supervisor' // The provider leads its own group; its grandchild shares that group and ignores SIGTERM. const PROVIDER = String.raw` diff --git a/src/main/codex/codex-app-server-posix-supervisor.test.ts b/src/main/provider-process/provider-process-supervisor.test.ts similarity index 94% rename from src/main/codex/codex-app-server-posix-supervisor.test.ts rename to src/main/provider-process/provider-process-supervisor.test.ts index 99cbaf100088..c65f5518f3f3 100644 --- a/src/main/codex/codex-app-server-posix-supervisor.test.ts +++ b/src/main/provider-process/provider-process-supervisor.test.ts @@ -1,14 +1,14 @@ import { describe, expect, it } from 'vitest' -import type { CodexAppServerLaunch } from './codex-app-server-connection' +import type { ProviderProcessLaunch } from './provider-process-launch' import { createProviderSpawnSpec, POSIX_PROVIDER_SUPERVISOR_SCRIPT, PROVIDER_SIGTERM_GRACE_MS, PROVIDER_STDIN_END_GRACE_MS, supervisedPosixLaunch -} from './codex-app-server-posix-supervisor' +} from './provider-process-supervisor' -const launch: CodexAppServerLaunch = { +const launch: ProviderProcessLaunch = { command: '/opt/codex', args: ['app-server', '--flag'], cwd: '/work/repo', diff --git a/src/main/codex/codex-app-server-posix-supervisor.ts b/src/main/provider-process/provider-process-supervisor.ts similarity index 98% rename from src/main/codex/codex-app-server-posix-supervisor.ts rename to src/main/provider-process/provider-process-supervisor.ts index 21dd56aeb5b5..459c379b6881 100644 --- a/src/main/codex/codex-app-server-posix-supervisor.ts +++ b/src/main/provider-process/provider-process-supervisor.ts @@ -1,4 +1,4 @@ -import type { CodexAppServerLaunch } from './codex-app-server-connection' +import type { ProviderProcessLaunch } from './provider-process-launch' /** Time the provider gets to exit on its own after its stdin ends, before SIGTERM. */ export const PROVIDER_STDIN_END_GRACE_MS = 1_000 @@ -139,7 +139,7 @@ function assertGraceWithin(name: string, graceMs: number, maxMs: number): void { } export function supervisedPosixLaunch( - launch: CodexAppServerLaunch, + launch: ProviderProcessLaunch, childEnv: NodeJS.ProcessEnv, { cwd = launch.cwd ?? process.cwd(), @@ -174,7 +174,7 @@ export function supervisedPosixLaunch( } export function createProviderSpawnSpec( - launch: CodexAppServerLaunch, + launch: ProviderProcessLaunch, childEnv: NodeJS.ProcessEnv, platform: NodeJS.Platform ): { diff --git a/src/main/codex/codex-app-server-process-teardown.test.ts b/src/main/provider-process/provider-process-teardown.test.ts similarity index 90% rename from src/main/codex/codex-app-server-process-teardown.test.ts rename to src/main/provider-process/provider-process-teardown.test.ts index 295d780718d0..4c7d1dae86d2 100644 --- a/src/main/codex/codex-app-server-process-teardown.test.ts +++ b/src/main/provider-process/provider-process-teardown.test.ts @@ -4,7 +4,7 @@ import { findSelfInitiatedTreeKills, resetSelfInitiatedTreeKillLogForTest } from '../crash-reporting/self-initiated-tree-kill-log' -import { terminateCodexAppServerProcessTree } from './codex-app-server-process-teardown' +import { terminateProviderProcessTree } from './provider-process-teardown' /** Above pid_max on every supported POSIX host, so the group signal is a real ESRCH. */ const UNREACHABLE_PGID = 2_147_483_647 @@ -16,7 +16,7 @@ function child() { } } -describe('terminateCodexAppServerProcessTree', () => { +describe('terminateProviderProcessTree', () => { beforeEach(() => { resetSelfInitiatedTreeKillLogForTest() }) @@ -26,7 +26,7 @@ describe('terminateCodexAppServerProcessTree', () => { const release = Promise.withResolvers() const terminateWindowsTree = vi.fn(() => release.promise) - const teardown = terminateCodexAppServerProcessTree(target, { + const teardown = terminateProviderProcessTree(target, { platform: 'win32', terminateWindowsTree }) @@ -43,7 +43,7 @@ describe('terminateCodexAppServerProcessTree', () => { const snapshot = { rootPgid: 1234, descendants: [], capturedAtMs: 1 } const release = Promise.withResolvers() - const teardown = terminateCodexAppServerProcessTree(target, { + const teardown = terminateProviderProcessTree(target, { platform: 'darwin', captureDescendants: async () => snapshot, terminateDescendants: () => release.promise @@ -62,7 +62,7 @@ describe('terminateCodexAppServerProcessTree', () => { const signalProcessGroup = vi.fn() await expect( - terminateCodexAppServerProcessTree(target, { + terminateProviderProcessTree(target, { platform: 'darwin', dedicatedProcessGroup: true, captureDescendants, @@ -79,7 +79,7 @@ describe('terminateCodexAppServerProcessTree', () => { const target = child() await expect( - terminateCodexAppServerProcessTree(target, { + terminateProviderProcessTree(target, { platform: 'linux', dedicatedProcessGroup: true, signalProcessGroup: () => { @@ -102,7 +102,7 @@ describe('terminateCodexAppServerProcessTree', () => { const target = { pid: UNREACHABLE_PGID, kill: vi.fn(() => true) as ChildProcess['kill'] } await expect( - terminateCodexAppServerProcessTree(target, { + terminateProviderProcessTree(target, { platform: 'darwin', captureDescendants: async () => ({ rootPgid: UNREACHABLE_PGID, @@ -122,7 +122,7 @@ describe('terminateCodexAppServerProcessTree', () => { const signalProcessGroup = vi.fn() await expect( - terminateCodexAppServerProcessTree(target, { + terminateProviderProcessTree(target, { platform: 'darwin', captureDescendants: async () => ({ rootPgid: 1234, descendants: [], capturedAtMs: 1 }), terminateDescendants: async () => true, @@ -151,7 +151,7 @@ describe('terminateCodexAppServerProcessTree', () => { const results = await Promise.all( targets.map((target) => - terminateCodexAppServerProcessTree(target, { + terminateProviderProcessTree(target, { platform: 'linux', dedicatedProcessGroup: true, captureDescendants, diff --git a/src/main/codex/codex-app-server-process-teardown.ts b/src/main/provider-process/provider-process-teardown.ts similarity index 90% rename from src/main/codex/codex-app-server-process-teardown.ts rename to src/main/provider-process/provider-process-teardown.ts index 10e625bef7e8..c55531f9be0f 100644 --- a/src/main/codex/codex-app-server-process-teardown.ts +++ b/src/main/provider-process/provider-process-teardown.ts @@ -8,7 +8,7 @@ const activeTeardowns = new WeakMap>() type TeardownChild = Pick -export type CodexAppServerProcessTeardownDeps = { +export type ProviderProcessTeardownDeps = { platform?: NodeJS.Platform dedicatedProcessGroup?: boolean captureDescendants?: (rootPid: number) => Promise @@ -17,10 +17,7 @@ export type CodexAppServerProcessTeardownDeps = { signalProcessGroup?: (pgid: number, signal: NodeJS.Signals) => void } -function terminateDedicatedPosixGroup( - rootPid: number, - deps: CodexAppServerProcessTeardownDeps -): boolean { +function terminateDedicatedPosixGroup(rootPid: number, deps: ProviderProcessTeardownDeps): boolean { const signalGroup = deps.signalProcessGroup ?? ((pgid: number, signal: NodeJS.Signals) => process.kill(-pgid, signal)) @@ -41,7 +38,7 @@ function terminateDedicatedPosixGroup( async function terminatePosixTree( child: TeardownChild, rootPid: number, - deps: CodexAppServerProcessTeardownDeps + deps: ProviderProcessTeardownDeps ): Promise { child.kill('SIGSTOP') const capture = deps.captureDescendants ?? captureDescendantSnapshot @@ -87,10 +84,10 @@ async function terminatePosixTree( return true } -/** Stops every process owned by one app-server launch before releasing its wrapper. */ +/** Stops every process owned by one provider launch before releasing its wrapper. */ async function terminateOnce( child: TeardownChild, - deps: CodexAppServerProcessTeardownDeps + deps: ProviderProcessTeardownDeps ): Promise { const rootPid = child.pid if (!rootPid) { @@ -110,9 +107,9 @@ async function terminateOnce( return terminatePosixTree(child, rootPid, deps) } -export function terminateCodexAppServerProcessTree( +export function terminateProviderProcessTree( child: TeardownChild, - deps: CodexAppServerProcessTeardownDeps = {} + deps: ProviderProcessTeardownDeps = {} ): Promise { const key = child as object const active = activeTeardowns.get(key) diff --git a/src/main/codex/codex-app-server-record-reader.ts b/src/main/provider-process/provider-record-reader.ts similarity index 90% rename from src/main/codex/codex-app-server-record-reader.ts rename to src/main/provider-process/provider-record-reader.ts index bdaea79164b0..4c3608bc9994 100644 --- a/src/main/codex/codex-app-server-record-reader.ts +++ b/src/main/provider-process/provider-record-reader.ts @@ -6,17 +6,17 @@ import { type RecordReaderStream = Pick -export type CodexAppServerRecordReader = { +export type ProviderRecordReader = { pause: () => void resume: () => void } -export function createCodexAppServerRecordReader(input: { +export function createProviderRecordReader(input: { stdout: RecordReaderStream onRecord: (record: unknown, line: string) => void onRejected: (rejected: NdjsonRejectedRecord) => void onFatal: (error: Error) => void -}): CodexAppServerRecordReader { +}): ProviderRecordReader { let paused = false const framer = createIncrementalNdjsonFramer(input.onRecord, input.onRejected, { // The provider owns this local stdio stream, so valid agent payloads keep full fidelity. 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 2e086367d8b5..69f28aa8d7e8 100644 --- a/src/shared/child-process/__fixtures__/child-process-import-allowlist.txt +++ b/src/shared/child-process/__fixtures__/child-process-import-allowlist.txt @@ -14,7 +14,7 @@ # `detached: true` for a process-group kill, a synchronous taskkill, and an stdio shape other # than all-pipes (a long-lived `ssh -N` must not be handed a stdin pipe nobody closes). # Migrating them means teaching run-process.ts those options first. -# codex-app-server-posix-supervisor.ts requires child_process only inside its +# provider-process-supervisor.ts requires child_process only inside its # dependency-free POSIX supervisor script, never in Orca's process. # # The src/main/git/command-runner/* entries are the seven spawn sites that used @@ -56,7 +56,7 @@ src/main/codex-accounts/legacy-wsl-runtime-auth-drain-recovery-script-harness.ts src/main/codex-accounts/legacy-wsl-runtime-auth-drain-script-harness.ts src/main/codex-accounts/legacy-wsl-runtime-auth-drain-script-interference-shims.ts src/main/codex-accounts/service.ts -src/main/codex/codex-app-server-posix-supervisor.ts +src/main/provider-process/provider-process-supervisor.ts src/main/codex/codex-app-server-session.ts src/main/codex/codex-wsl-hook-install-plan.ts src/main/computer/desktop-script-provider-bridge.ts From ef2bb104a23eef29d3e6d46d66bf925f26a5ea97 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 3 Oct 2026 01:54:19 -0700 Subject: [PATCH 2/4] Preserve teardown behavior with checked mock types after move --- .../provider-process/provider-process-teardown.test.ts | 8 ++++---- src/main/provider-process/provider-process-teardown.ts | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/provider-process/provider-process-teardown.test.ts b/src/main/provider-process/provider-process-teardown.test.ts index 4c7d1dae86d2..60826a269c5f 100644 --- a/src/main/provider-process/provider-process-teardown.test.ts +++ b/src/main/provider-process/provider-process-teardown.test.ts @@ -12,7 +12,7 @@ const UNREACHABLE_PGID = 2_147_483_647 function child() { return { pid: 1234, - kill: vi.fn(() => true) as ChildProcess['kill'] + kill: vi.fn(() => true) } } @@ -99,7 +99,7 @@ describe('terminateProviderProcessTree', () => { * lives in the production default, not in an injectable seam. */ it('does not claim a snapshot group that was already gone', async () => { - const target = { pid: UNREACHABLE_PGID, kill: vi.fn(() => true) as ChildProcess['kill'] } + const target = { pid: UNREACHABLE_PGID, kill: vi.fn(() => true) } await expect( terminateProviderProcessTree(target, { @@ -141,10 +141,10 @@ describe('terminateProviderProcessTree', () => { }) it('tears down 40 dedicated groups without process-table scans or cross-group fanout', async () => { - const killMocks = Array.from({ length: 40 }, () => vi.fn(() => true)) + const killMocks = Array.from({ length: 40 }, () => vi.fn(() => true)) const targets = killMocks.map((kill, index) => ({ pid: 10_000 + index, - kill: kill as ChildProcess['kill'] + kill })) const captureDescendants = vi.fn() const signalProcessGroup = vi.fn() diff --git a/src/main/provider-process/provider-process-teardown.ts b/src/main/provider-process/provider-process-teardown.ts index c55531f9be0f..de55788ae5f2 100644 --- a/src/main/provider-process/provider-process-teardown.ts +++ b/src/main/provider-process/provider-process-teardown.ts @@ -24,6 +24,7 @@ function terminateDedicatedPosixGroup(rootPid: number, deps: ProviderProcessTear try { signalGroup(rootPid, 'SIGKILL') } catch (error) { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Node process.kill errors expose an optional errno code; only that field is read. return (error as NodeJS.ErrnoException).code === 'ESRCH' } // Outside the try: that catch is the ESRCH contract, not a breadcrumb handler. @@ -111,7 +112,7 @@ export function terminateProviderProcessTree( child: TeardownChild, deps: ProviderProcessTeardownDeps = {} ): Promise { - const key = child as object + const key = child const active = activeTeardowns.get(key) if (active) { return active From 0b95df2374f4e2729876711083c8cde486c55aab Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 3 Oct 2026 13:06:00 -0700 Subject: [PATCH 3/4] Apply provider launch environment and caller teardown labels --- src/main/codex/codex-app-server-connection.ts | 8 ++----- .../provider-process-supervisor.test.ts | 21 +++++++++++++++++- .../provider-process-supervisor.ts | 6 ++++- .../provider-process-teardown.test.ts | 22 +++++++++++++++++++ .../provider-process-teardown.ts | 9 ++++---- 5 files changed, 54 insertions(+), 12 deletions(-) diff --git a/src/main/codex/codex-app-server-connection.ts b/src/main/codex/codex-app-server-connection.ts index cae5a81aa59d..298fa4428324 100644 --- a/src/main/codex/codex-app-server-connection.ts +++ b/src/main/codex/codex-app-server-connection.ts @@ -52,17 +52,13 @@ export async function openCodexAppServerConnection( handlers: CodexAppServerConnectionHandlers = {}, spawnImpl: typeof spawnProcess = spawnProcess ): Promise { - const childEnv: NodeJS.ProcessEnv = { ...process.env, ...launch.env } - for (const key of launch.envToDelete ?? []) { - delete childEnv[key] - } - const spawnSpec = createProviderSpawnSpec(launch, childEnv, process.platform) + const spawnSpec = createProviderSpawnSpec(launch, process.env, process.platform) const child = spawnImpl(spawnSpec) function terminateProcessTree(): Promise { // The supervisor and provider own separate POSIX groups so the supervisor can prove the // provider group empty before relaying its exit. Forced wrapper teardown uses descendant proof. - return terminateProviderProcessTree(child) + return terminateProviderProcessTree(child, { site: 'codex-app-server-teardown' }) } let stderrTail = '' diff --git a/src/main/provider-process/provider-process-supervisor.test.ts b/src/main/provider-process/provider-process-supervisor.test.ts index c65f5518f3f3..dcca6feeec05 100644 --- a/src/main/provider-process/provider-process-supervisor.test.ts +++ b/src/main/provider-process/provider-process-supervisor.test.ts @@ -65,10 +65,29 @@ describe('structured provider supervision', () => { expect(createProviderSpawnSpec(launch, { PATH: '/bin' }, 'win32')).toEqual({ program: '/opt/codex', args: ['app-server', '--flag'], - env: { PATH: '/bin' }, + env: { PATH: '/bin', CODEX_HOME: '/tmp/codex' }, cwd: '/work/repo', detached: false, supervised: false }) }) + + it.each(['win32', 'darwin', 'linux'] as const)( + 'applies launch environment overrides and deletions on %s', + (platform) => { + const baseEnv = { PATH: '/bin', AGENT_HOME: '/inherited', PARENT_AGENT: 'parent' } + const overlay = { AGENT_HOME: '/pinned', LAUNCH_ONLY: 'added', PARENT_AGENT: 'overlay' } + const spec = createProviderSpawnSpec( + { command: 'provider', args: [], env: overlay, envToDelete: ['PARENT_AGENT'] }, + baseEnv, + platform + ) + + expect(spec.env).toMatchObject({ PATH: '/bin', AGENT_HOME: '/pinned', LAUNCH_ONLY: 'added' }) + expect(spec.env).not.toHaveProperty('PARENT_AGENT') + expect(baseEnv.PARENT_AGENT).toBe('parent') + expect(baseEnv.AGENT_HOME).toBe('/inherited') + expect(overlay.PARENT_AGENT).toBe('overlay') + } + ) }) diff --git a/src/main/provider-process/provider-process-supervisor.ts b/src/main/provider-process/provider-process-supervisor.ts index 459c379b6881..a446167b2f05 100644 --- a/src/main/provider-process/provider-process-supervisor.ts +++ b/src/main/provider-process/provider-process-supervisor.ts @@ -175,7 +175,7 @@ export function supervisedPosixLaunch( export function createProviderSpawnSpec( launch: ProviderProcessLaunch, - childEnv: NodeJS.ProcessEnv, + baseEnv: NodeJS.ProcessEnv, platform: NodeJS.Platform ): { program: string @@ -186,6 +186,10 @@ export function createProviderSpawnSpec( /** The child is the supervisor, whose SIGTERM stops the provider and then itself. */ supervised: boolean } { + const childEnv: NodeJS.ProcessEnv = { ...baseEnv, ...launch.env } + for (const key of launch.envToDelete ?? []) { + delete childEnv[key] + } const supervisor = platform === 'win32' ? null : supervisedPosixLaunch(launch, childEnv) return { program: supervisor?.command ?? launch.command, diff --git a/src/main/provider-process/provider-process-teardown.test.ts b/src/main/provider-process/provider-process-teardown.test.ts index 60826a269c5f..d5fdcce6afcf 100644 --- a/src/main/provider-process/provider-process-teardown.test.ts +++ b/src/main/provider-process/provider-process-teardown.test.ts @@ -27,6 +27,7 @@ describe('terminateProviderProcessTree', () => { const terminateWindowsTree = vi.fn(() => release.promise) const teardown = terminateProviderProcessTree(target, { + site: 'codex-app-server-teardown', platform: 'win32', terminateWindowsTree }) @@ -38,12 +39,25 @@ describe('terminateProviderProcessTree', () => { expect(target.kill).toHaveBeenCalledWith('SIGKILL') }) + it('passes a non-Codex diagnostic label to Windows teardown', async () => { + const terminateWindowsTree = vi.fn(async () => undefined) + + await terminateProviderProcessTree(child(), { + site: 'provider-test-teardown', + platform: 'win32', + terminateWindowsTree + }) + + expect(terminateWindowsTree).toHaveBeenCalledWith(1234, { site: 'provider-test-teardown' }) + }) + it('waits for an owned POSIX snapshot before killing the wrapper', async () => { const target = child() const snapshot = { rootPgid: 1234, descendants: [], capturedAtMs: 1 } const release = Promise.withResolvers() const teardown = terminateProviderProcessTree(target, { + site: 'codex-app-server-teardown', platform: 'darwin', captureDescendants: async () => snapshot, terminateDescendants: () => release.promise @@ -63,6 +77,7 @@ describe('terminateProviderProcessTree', () => { await expect( terminateProviderProcessTree(target, { + site: 'provider-test-teardown', platform: 'darwin', dedicatedProcessGroup: true, captureDescendants, @@ -73,6 +88,9 @@ describe('terminateProviderProcessTree', () => { expect(signalProcessGroup).toHaveBeenCalledWith(1234, 'SIGKILL') expect(captureDescendants).not.toHaveBeenCalled() expect(target.kill).not.toHaveBeenCalled() + expect(findSelfInitiatedTreeKills(Date.now())).toEqual([ + expect.objectContaining({ site: 'provider-test-teardown', scope: 'posix-process-group' }) + ]) }) it('keeps the dedicated-group wrapper reachable when signalling is unproven', async () => { @@ -80,6 +98,7 @@ describe('terminateProviderProcessTree', () => { await expect( terminateProviderProcessTree(target, { + site: 'codex-app-server-teardown', platform: 'linux', dedicatedProcessGroup: true, signalProcessGroup: () => { @@ -103,6 +122,7 @@ describe('terminateProviderProcessTree', () => { await expect( terminateProviderProcessTree(target, { + site: 'codex-app-server-teardown', platform: 'darwin', captureDescendants: async () => ({ rootPgid: UNREACHABLE_PGID, @@ -123,6 +143,7 @@ describe('terminateProviderProcessTree', () => { await expect( terminateProviderProcessTree(target, { + site: 'codex-app-server-teardown', platform: 'darwin', captureDescendants: async () => ({ rootPgid: 1234, descendants: [], capturedAtMs: 1 }), terminateDescendants: async () => true, @@ -152,6 +173,7 @@ describe('terminateProviderProcessTree', () => { const results = await Promise.all( targets.map((target) => terminateProviderProcessTree(target, { + site: 'codex-app-server-teardown', platform: 'linux', dedicatedProcessGroup: true, captureDescendants, diff --git a/src/main/provider-process/provider-process-teardown.ts b/src/main/provider-process/provider-process-teardown.ts index de55788ae5f2..19e5b9b967f3 100644 --- a/src/main/provider-process/provider-process-teardown.ts +++ b/src/main/provider-process/provider-process-teardown.ts @@ -9,6 +9,7 @@ const activeTeardowns = new WeakMap>() type TeardownChild = Pick export type ProviderProcessTeardownDeps = { + site: string platform?: NodeJS.Platform dedicatedProcessGroup?: boolean captureDescendants?: (rootPid: number) => Promise @@ -30,7 +31,7 @@ function terminateDedicatedPosixGroup(rootPid: number, deps: ProviderProcessTear // Outside the try: that catch is the ESRCH contract, not a breadcrumb handler. recordSelfInitiatedTreeKill({ pid: rootPid, - site: 'codex-app-server-teardown', + site: deps.site, scope: 'posix-process-group' }) return true @@ -72,7 +73,7 @@ async function terminatePosixTree( // already-gone contract, not a breadcrumb handler. recordSelfInitiatedTreeKill({ pid: snapshot.rootPgid, - site: 'codex-app-server-teardown', + site: deps.site, scope: 'posix-process-group' }) } @@ -97,7 +98,7 @@ async function terminateOnce( } if ((deps.platform ?? process.platform) === 'win32') { const terminate = deps.terminateWindowsTree ?? terminateWindowsProcessTree - await terminate(rootPid, { site: 'codex-app-server-teardown' }) + await terminate(rootPid, { site: deps.site }) // taskkill owns the tree; this preserves the prior direct-child fallback when it fails. child.kill('SIGKILL') return true @@ -110,7 +111,7 @@ async function terminateOnce( export function terminateProviderProcessTree( child: TeardownChild, - deps: ProviderProcessTeardownDeps = {} + deps: ProviderProcessTeardownDeps ): Promise { const key = child const active = activeTeardowns.get(key) From 987fb5307f22691c5e3d3c21c88a929af47ef24f Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 4 Oct 2026 23:42:01 -0700 Subject: [PATCH 4/4] Give provider child env one owner and gate Codex contract on the shared reader resolveProviderChildEnv is now the only place that overlays and strips a provider's environment; the spawn spec and the request-scoped Codex session both call it. supervisedPosixLaunch only accepts a launch without env fields, so an override can no longer be silently ignored there. Edits to the shared stream reader or the env rule now run the real-binary Codex contract job. --- config/scripts/pr-code-change-scope.mjs | 2 ++ config/scripts/pr-code-change-scope.test.mjs | 2 ++ config/tsconfig.cli.json | 1 + src/main/codex/codex-app-server-session.ts | 6 ++--- .../provider-process-launch.test.ts | 27 +++++++++++++++++++ .../provider-process-launch.ts | 12 +++++++++ .../provider-process-supervisor.test.ts | 14 +++++++--- .../provider-process-supervisor.ts | 23 +++++++++++----- 8 files changed, 73 insertions(+), 14 deletions(-) create mode 100644 src/main/provider-process/provider-process-launch.test.ts diff --git a/config/scripts/pr-code-change-scope.mjs b/config/scripts/pr-code-change-scope.mjs index 2d0b7ac2fc01..7f9005197722 100644 --- a/config/scripts/pr-code-change-scope.mjs +++ b/config/scripts/pr-code-change-scope.mjs @@ -71,6 +71,8 @@ const CODEX_INDEX_HEAL_CONTRACT_PREFIXES = [ 'src/main/sqlite/sync-database', 'src/main/codex/codex-app-server-capability-signal', 'src/main/provider-process/provider-process-exit-deadline', + 'src/main/provider-process/provider-process-launch', + 'src/main/provider-process/provider-record-reader', 'src/main/codex/codex-session-backfill', 'src/main/codex/codex-session-index-heal-state', 'src/main/codex-cli/command', diff --git a/config/scripts/pr-code-change-scope.test.mjs b/config/scripts/pr-code-change-scope.test.mjs index 65cc540c90f2..ffd19687528a 100644 --- a/config/scripts/pr-code-change-scope.test.mjs +++ b/config/scripts/pr-code-change-scope.test.mjs @@ -170,6 +170,8 @@ describe('per-job path classification', () => { for (const file of [ 'src/main/codex/codex-app-server-capability-signal.ts', 'src/main/provider-process/provider-process-exit-deadline.ts', + 'src/main/provider-process/provider-process-launch.ts', + 'src/main/provider-process/provider-record-reader.ts', 'src/main/codex/codex-session-backfill.ts', 'src/main/codex/codex-session-index-heal-state.ts', 'src/main/codex-cli/command.ts', diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index 910071f62a34..934423623384 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -150,6 +150,7 @@ "../src/main/codex/codex-hook-wsl-runtime.ts", "../src/main/codex/codex-managed-trust-reconciliation.ts", "../src/main/provider-process/provider-process-exit-deadline.ts", + "../src/main/provider-process/provider-process-launch.ts", "../src/main/codex/codex-real-home-background-grant.ts", "../src/main/codex/codex-real-home-entry-trust.ts", "../src/main/codex/codex-real-home-hook-entry-plan.ts", diff --git a/src/main/codex/codex-app-server-session.ts b/src/main/codex/codex-app-server-session.ts index 8f2bcbecad2b..e46be4303296 100644 --- a/src/main/codex/codex-app-server-session.ts +++ b/src/main/codex/codex-app-server-session.ts @@ -1,5 +1,6 @@ import type { ChildProcessWithoutNullStreams } from 'node:child_process' import { waitForProcessExitUntil } from '../provider-process/provider-process-exit-deadline' +import { resolveProviderChildEnv } from '../provider-process/provider-process-launch' import { stderrIndicatesMissingAppServer } from './codex-app-server-capability-signal' import { withCliRuntimeOnPath } from '../../shared/node-cli-command-resolution' import { @@ -102,10 +103,7 @@ export async function runCodexAppServerSession( ): Promise { // Why: a default-home grant must run against the real ~/.codex, so strip an // inherited CODEX_HOME (envToDelete) after applying the overlay, not before. - const childEnv: NodeJS.ProcessEnv = { ...process.env, ...invocation.env } - for (const key of invocation.envToDelete ?? []) { - delete childEnv[key] - } + const childEnv = resolveProviderChildEnv(invocation, process.env) const pairedEnv = invocation.cliPath ? withCliRuntimeOnPath(invocation.cliPath, childEnv) : childEnv diff --git a/src/main/provider-process/provider-process-launch.test.ts b/src/main/provider-process/provider-process-launch.test.ts new file mode 100644 index 000000000000..0eb40592a50a --- /dev/null +++ b/src/main/provider-process/provider-process-launch.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from 'vitest' +import { resolveProviderChildEnv } from './provider-process-launch' + +describe('resolveProviderChildEnv', () => { + it('overlays the launch env on the inherited env, then strips deleted keys', () => { + const baseEnv = { PATH: '/bin', AGENT_HOME: '/inherited', PARENT_AGENT: 'parent' } + const overlay = { AGENT_HOME: '/pinned', LAUNCH_ONLY: 'added', PARENT_AGENT: 'overlay' } + + const childEnv = resolveProviderChildEnv( + { env: overlay, envToDelete: ['PARENT_AGENT'] }, + baseEnv + ) + + expect(childEnv).toEqual({ PATH: '/bin', AGENT_HOME: '/pinned', LAUNCH_ONLY: 'added' }) + expect(baseEnv).toEqual({ PATH: '/bin', AGENT_HOME: '/inherited', PARENT_AGENT: 'parent' }) + expect(overlay.PARENT_AGENT).toBe('overlay') + }) + + it('copies the inherited env when the launch carries no env', () => { + const baseEnv = { PATH: '/bin' } + + const childEnv = resolveProviderChildEnv({}, baseEnv) + + expect(childEnv).toEqual(baseEnv) + expect(childEnv).not.toBe(baseEnv) + }) +}) diff --git a/src/main/provider-process/provider-process-launch.ts b/src/main/provider-process/provider-process-launch.ts index 326e1d17adcb..f7c527cd5824 100644 --- a/src/main/provider-process/provider-process-launch.ts +++ b/src/main/provider-process/provider-process-launch.ts @@ -8,3 +8,15 @@ export type ProviderProcessLaunch = { /** Keys stripped after the overlay. */ envToDelete?: readonly string[] } + +/** The one env rule for provider children: inherit, overlay, then strip (so a delete beats an overlay). */ +export function resolveProviderChildEnv( + launch: Pick, + baseEnv: NodeJS.ProcessEnv +): NodeJS.ProcessEnv { + const childEnv: NodeJS.ProcessEnv = { ...baseEnv, ...launch.env } + for (const key of launch.envToDelete ?? []) { + delete childEnv[key] + } + return childEnv +} diff --git a/src/main/provider-process/provider-process-supervisor.test.ts b/src/main/provider-process/provider-process-supervisor.test.ts index dcca6feeec05..a35f8102d6b3 100644 --- a/src/main/provider-process/provider-process-supervisor.test.ts +++ b/src/main/provider-process/provider-process-supervisor.test.ts @@ -14,11 +14,12 @@ const launch: ProviderProcessLaunch = { cwd: '/work/repo', env: { CODEX_HOME: '/tmp/codex' } } +const command = { command: launch.command, args: launch.args, cwd: launch.cwd } describe('structured provider supervision', () => { it('wraps POSIX launches in a detached supervisor and preserves the launch spec', () => { const childEnv = { PATH: '/bin', CODEX_HOME: '/tmp/codex' } - const spec = supervisedPosixLaunch(launch, childEnv) + const spec = supervisedPosixLaunch(command, childEnv) expect(spec.command).toBe(process.execPath) expect(spec.args).toEqual(['-e', POSIX_PROVIDER_SUPERVISOR_SCRIPT]) @@ -49,11 +50,18 @@ describe('structured provider supervision', () => { expect(POSIX_PROVIDER_SUPERVISOR_SCRIPT).not.toContain('process.ppid === 1') }) + it('only accepts a resolved env, never a launch whose env it would ignore', () => { + // @ts-expect-error env/envToDelete are resolved by createProviderSpawnSpec, not here. + const spec = supervisedPosixLaunch(launch, { PATH: '/bin' }) + + expect(spec.env).not.toHaveProperty('CODEX_HOME') + }) + it('refuses a grace longer than recovery waits before SIGKILL', () => { const stdinEnd = (stdinEndGraceMs: number) => () => - supervisedPosixLaunch(launch, {}, { stdinEndGraceMs }) + supervisedPosixLaunch(command, {}, { stdinEndGraceMs }) const sigterm = (sigtermGraceMs: number) => () => - supervisedPosixLaunch(launch, {}, { sigtermGraceMs }) + supervisedPosixLaunch(command, {}, { sigtermGraceMs }) expect(stdinEnd(PROVIDER_STDIN_END_GRACE_MS)).not.toThrow() expect(stdinEnd(PROVIDER_STDIN_END_GRACE_MS + 1)).toThrow(RangeError) diff --git a/src/main/provider-process/provider-process-supervisor.ts b/src/main/provider-process/provider-process-supervisor.ts index a446167b2f05..ac47a4a51120 100644 --- a/src/main/provider-process/provider-process-supervisor.ts +++ b/src/main/provider-process/provider-process-supervisor.ts @@ -1,4 +1,4 @@ -import type { ProviderProcessLaunch } from './provider-process-launch' +import { resolveProviderChildEnv, type ProviderProcessLaunch } from './provider-process-launch' /** Time the provider gets to exit on its own after its stdin ends, before SIGTERM. */ export const PROVIDER_STDIN_END_GRACE_MS = 1_000 @@ -138,8 +138,14 @@ function assertGraceWithin(name: string, graceMs: number, maxMs: number): void { } } +// `never` makes an env-bearing launch a type error here: env is resolved before supervision. +type ProviderSupervisedCommand = Pick & { + env?: never + envToDelete?: never +} + export function supervisedPosixLaunch( - launch: ProviderProcessLaunch, + launch: ProviderSupervisedCommand, childEnv: NodeJS.ProcessEnv, { cwd = launch.cwd ?? process.cwd(), @@ -186,11 +192,14 @@ export function createProviderSpawnSpec( /** The child is the supervisor, whose SIGTERM stops the provider and then itself. */ supervised: boolean } { - const childEnv: NodeJS.ProcessEnv = { ...baseEnv, ...launch.env } - for (const key of launch.envToDelete ?? []) { - delete childEnv[key] - } - const supervisor = platform === 'win32' ? null : supervisedPosixLaunch(launch, childEnv) + const childEnv = resolveProviderChildEnv(launch, baseEnv) + const supervisor = + platform === 'win32' + ? null + : supervisedPosixLaunch( + { command: launch.command, args: launch.args, cwd: launch.cwd }, + childEnv + ) return { program: supervisor?.command ?? launch.command, args: supervisor?.args ?? launch.args,