S0-01: Prove signed-in Codex ACP and bounded cleanup

core/acp/README.md

@@ -12,6 +12,13 @@ for the implementation ([P1-01] — done).
 
 Adding a backend later = spawn a different ACP server. No new client code.
 
+Install the maintained Codex adapter as
+`@agentclientprotocol/codex-acp`. The older
+`@zed-industries/codex-acp` package is deprecated and embeds an older Codex
+runtime even when a newer standalone `codex` CLI is on `PATH`. Record the
+adapter package and version during live verification; executable name alone
+does not distinguish them.
+
 ## Dependency
 
 `@agentclientprotocol/sdk` (npm, verified on the registry 2026-07-21). This is
@@ -64,6 +71,13 @@ child adapter process is killed and the caller gets a thrown error
 hanging forever — this is what protects against a stalled adapter (e.g. an
 errored codex-acp turn, or any other hang mid-session).
 
+On POSIX, adapters start in their own process group. Session disposal and
+handshake/turn failures terminate the complete adapter process tree, wait up
+to one second, then escalate to `SIGKILL` if required. This matters for npm
+launcher scripts that spawn a native adapter child: killing only the launcher
+can otherwise orphan the native process. Windows terminates the direct child,
+waits with the same bound, and escalates before reporting cleanup failure.
+
 The handshake default is deliberately generous, not tight: measured against
 a real cold-start `claude-code-acp` (`CLAUDECODE` stripped), `initialize`
 returns in ~226ms but `session/new` alone legitimately takes ~16.2s — it
@@ -83,6 +97,18 @@ Requires the adapter CLI(s) actually installed and signed in to your
 Claude Pro/Max or ChatGPT plan. This is **not** part of `npm test` / CI — it's
 a separate, honest check of the real sanctioned-plan path. See `smoke.ts`.
 
+Smoke output is sanitized and compact: UTC start/end times, streamed text,
+update kinds, and terminal stop reason. Adapter diagnostics remain on stderr.
+
+If a service rollout selects a model newer than the installed adapter runtime,
+set `VELLUM_CODEX_MODEL` to a model supported by that runtime while upgrading
+the adapter. The value goes directly to the first-party adapter as
+`-c model=<value>` without a shell or alternate authentication path:
+
+```bash
+VELLUM_CODEX_MODEL=gpt-5.5 npm run smoke:acp -- codex
+```
+
 Runs fine from inside a Claude Code terminal/agent now (the `CLAUDECODE`
 stripping above handles it automatically) and no longer hangs forever on a
 stalled adapter (the turn/handshake timeouts above make it fail loud

core/acp/smoke.ts

@@ -40,6 +40,7 @@ const PROMPT = 'Reply with exactly the word "pong" and nothing else.'
 
 async function smoke(backend: AcpBackend): Promise<void> {
   console.log(`\n=== ${backend} (${backend === 'claude' ? 'claude-code-acp' : 'codex-acp'}) ===`)
+  console.log(`startedAt=${new Date().toISOString()}`)
   const client = new StdioAcpClient()
   let session
   try {
@@ -49,22 +50,34 @@ async function smoke(backend: AcpBackend): Promise<void> {
     console.log(
       `  -> install/sign in the adapter, confirm it's on PATH, then re-run: npm run smoke:acp -- ${backend}`,
     )
+    process.exitCode = 1
     return
   }
 
   try {
     let sawDone = false
+    const updateKinds: string[] = []
     for await (const update of session.prompt({ text: PROMPT })) {
-      console.log(JSON.stringify(update))
+      updateKinds.push(update.kind)
+      if (update.kind === 'text' && typeof update.data === 'object' && update.data !== null) {
+        const text = 'text' in update.data ? update.data.text : undefined
+        if (typeof text === 'string') console.log(`text=${JSON.stringify(text)}`)
+      }
+      if (update.kind === 'done') console.log(`done=${JSON.stringify(update.data)}`)
       if (update.kind === 'done') sawDone = true
       if (update.kind === 'error') {
+        console.log(`error=${JSON.stringify(update.data)}`)
         console.log(`UNVERIFIED — adapter reported an error mid-turn.`)
+        process.exitCode = 1
         return
       }
     }
+    console.log(`updateKinds=${updateKinds.join(',')}`)
     console.log(sawDone ? 'VERIFIED — stream ended in a done update.' : 'UNVERIFIED — stream ended without done.')
+    if (!sawDone) process.exitCode = 1
   } finally {
     await session.dispose()
+    console.log(`endedAt=${new Date().toISOString()}`)
   }
 }
 

core/acp/stdio-client.test.ts

@@ -13,7 +13,13 @@ import type { Readable, Writable } from 'node:stream'
 import { describe, expect, it } from 'vitest'
 
 import type { AcpBackend, AcpUpdate } from './client.js'
-import { StdioAcpClient, buildAdapterEnv, mapSessionUpdate, type SpawnAdapter } from './stdio-client.js'
+import {
+  StdioAcpClient,
+  buildAdapterArgs,
+  buildAdapterEnv,
+  mapSessionUpdate,
+  type SpawnAdapter,
+} from './stdio-client.js'
 
 type AdapterChildProcess = ChildProcessByStdio<Writable, Readable, null>
 
@@ -22,6 +28,7 @@ type AdapterChildProcess = ChildProcessByStdio<Writable, Readable, null>
 // Behavior is selected via argv so one script covers every test scenario.
 const FAKE_AGENT_SCRIPT = String.raw`
 const readline = require('node:readline')
+const { spawn } = require('node:child_process')
 // argv[0] is the node binary; with \`node -e <script> <extra>\`, the first
 // extra arg lands at argv[1] (there is no "eval" placeholder entry).
 const mode = process.argv[1]
@@ -30,6 +37,12 @@ if (mode === 'exit-immediately') {
   process.exit(7)
 }
 
+if (mode === 'term-resistant-descendant') {
+  spawn(process.execPath, ['-e', 'process.on("SIGTERM", () => {}); setInterval(() => {}, 1000)'], {
+    stdio: 'ignore',
+  })
+}
+
 const rl = readline.createInterface({ input: process.stdin, terminal: false })
 function send(msg) {
   process.stdout.write(JSON.stringify(msg) + '\n')
@@ -70,6 +83,7 @@ function spawnFakeAgent(mode: string): SpawnAdapter {
   return (_backend: AcpBackend): AdapterChildProcess =>
     spawn(process.execPath, ['-e', FAKE_AGENT_SCRIPT, mode], {
       stdio: ['pipe', 'pipe', 'inherit'],
+      detached: process.platform !== 'win32',
     }) as AdapterChildProcess
 }
 
@@ -163,6 +177,38 @@ describe('StdioAcpClient', () => {
     expect(updates).toEqual([{ kind: 'error', data: { message: 'session disposed' } }])
   })
 
+  it('dispose() does not resolve until the adapter subprocess exits', async () => {
+    let child: AdapterChildProcess | undefined
+    const captureChild: SpawnAdapter = (backend) => {
+      child = spawnFakeAgent('happy')(backend)
+      return child
+    }
+    const client = new StdioAcpClient(captureChild)
+    const session = await client.newSession('codex')
+
+    await session.dispose()
+
+    expect(child?.exitCode !== null || child?.signalCode !== null).toBe(true)
+  })
+
+  it.runIf(process.platform !== 'win32')(
+    'dispose() removes a TERM-resistant adapter descendant and remains idempotent',
+    async () => {
+      let child: AdapterChildProcess | undefined
+      const captureChild: SpawnAdapter = (backend) => {
+        child = spawnFakeAgent('term-resistant-descendant')(backend)
+        return child
+      }
+      const client = new StdioAcpClient(captureChild)
+      const session = await client.newSession('codex')
+      const groupId = child!.pid!
+
+      await Promise.all([session.dispose(), session.dispose()])
+
+      expect(() => process.kill(-groupId, 0)).toThrow()
+    },
+  )
+
   it('fails a stalled prompt() via the turn timeout instead of hanging forever', async () => {
     const client = new StdioAcpClient(spawnFakeAgent('stall'), { turnMs: 50 })
     const session = await client.newSession('claude')
@@ -176,8 +222,14 @@ describe('StdioAcpClient', () => {
   })
 
   it('fails newSession() via the handshake timeout when the adapter never answers initialize', async () => {
-    const client = new StdioAcpClient(spawnFakeAgent('stall-handshake'), { handshakeMs: 50 })
+    let child: AdapterChildProcess | undefined
+    const captureChild: SpawnAdapter = (backend) => {
+      child = spawnFakeAgent('stall-handshake')(backend)
+      return child
+    }
+    const client = new StdioAcpClient(captureChild, { handshakeMs: 50 })
     await expect(client.newSession('claude')).rejects.toThrow('ACP handshake timed out after 50ms')
+    expect(child?.exitCode !== null || child?.signalCode !== null).toBe(true)
   })
 })
 
@@ -200,3 +252,17 @@ describe('buildAdapterEnv', () => {
     expect(buildAdapterEnv(sourceEnv)).toEqual({ PATH: '/usr/bin' })
   })
 })
+
+describe('buildAdapterArgs', () => {
+  it('passes an explicit Codex model override through the first-party adapter config interface', () => {
+    expect(buildAdapterArgs('codex', { VELLUM_CODEX_MODEL: 'gpt-5.5' })).toEqual([
+      '-c',
+      'model="gpt-5.5"',
+    ])
+  })
+
+  it('uses normal adapter configuration when no Codex model override is set', () => {
+    expect(buildAdapterArgs('codex', {})).toEqual([])
+    expect(buildAdapterArgs('claude', { VELLUM_CODEX_MODEL: 'gpt-5.5' })).toEqual([])
+  })
+})

core/acp/stdio-client.ts

@@ -42,9 +42,20 @@ type AdapterChildProcess = ChildProcessByStdio<Writable, Readable, null>
 
 /** First-party adapter subprocess per backend. Do not add others here without
  * a card — this is the whole point of the "no OAuth bridge" guardrail. */
-const ADAPTERS: Record<AcpBackend, { command: string; args: string[] }> = {
-  claude: { command: 'claude-code-acp', args: [] },
-  codex: { command: 'codex-acp', args: [] },
+const ADAPTERS: Record<AcpBackend, { command: string }> = {
+  claude: { command: 'claude-code-acp' },
+  codex: { command: 'codex-acp' },
+}
+
+/** Optional compatibility override for Codex service/CLI rollout skew. The
+ * adapter owns config parsing; JSON string encoding is valid TOML and keeps
+ * the model value in one argv entry without invoking a shell. */
+export function buildAdapterArgs(
+  backend: AcpBackend,
+  sourceEnv: NodeJS.ProcessEnv = process.env,
+): string[] {
+  const model = sourceEnv.VELLUM_CODEX_MODEL?.trim()
+  return backend === 'codex' && model ? ['-c', `model=${JSON.stringify(model)}`] : []
 }
 
 /** Spawns the adapter subprocess for a backend. Overridable for tests so unit
@@ -72,13 +83,79 @@ export function buildAdapterEnv(sourceEnv: NodeJS.ProcessEnv = process.env): Nod
 }
 
 const defaultSpawnAdapter: SpawnAdapter = (backend) => {
-  const { command, args } = ADAPTERS[backend]
-  return spawn(command, args, {
+  const { command } = ADAPTERS[backend]
+  return spawn(command, buildAdapterArgs(backend), {
     stdio: ['pipe', 'pipe', 'inherit'],
     env: buildAdapterEnv(),
+    detached: process.platform !== 'win32',
   }) as AdapterChildProcess
 }
 
+const PROCESS_EXIT_GRACE_MS = 1_000
+
+function processGroupExists(pid: number): boolean {
+  try {
+    process.kill(-pid, 0)
+    return true
+  } catch {
+    return false
+  }
+}
+
+async function waitForProcessGroupExit(pid: number, timeoutMs: number): Promise<boolean> {
+  const deadline = Date.now() + timeoutMs
+  while (processGroupExists(pid) && Date.now() < deadline) {
+    await new Promise((resolve) => setTimeout(resolve, 10))
+  }
+  return !processGroupExists(pid)
+}
+
+async function waitForChildExit(child: AdapterChildProcess, timeoutMs: number): Promise<boolean> {
+  if (child.exitCode !== null || child.signalCode !== null) return true
+  return new Promise((resolve) => {
+    const timer = setTimeout(() => resolve(false), timeoutMs)
+    const finish = (): void => {
+      clearTimeout(timer)
+      resolve(true)
+    }
+    child.once('exit', finish)
+    child.once('error', finish)
+  })
+}
+
+async function terminateAdapter(child: AdapterChildProcess): Promise<void> {
+  // A spawn failure has no pid and no process tree to terminate. Its tracked
+  // ENOENT/permission diagnostic must remain the error returned to callers.
+  if (child.pid === undefined) return
+
+  if (process.platform === 'win32') {
+    if (child.exitCode !== null || child.signalCode !== null) return
+    child.kill()
+    if (await waitForChildExit(child, PROCESS_EXIT_GRACE_MS)) return
+    child.kill('SIGKILL')
+    if (await waitForChildExit(child, PROCESS_EXIT_GRACE_MS)) return
+    throw new Error('ACP adapter process did not exit after forced termination')
+  }
+
+  const pid = child.pid
+  if (!processGroupExists(pid)) return
+  try {
+    process.kill(-pid, 'SIGTERM')
+  } catch {
+    child.kill()
+  }
+  if (await waitForProcessGroupExit(pid, PROCESS_EXIT_GRACE_MS)) return
+
+  try {
+    process.kill(-pid, 'SIGKILL')
+  } catch {
+    child.kill('SIGKILL')
+  }
+  if (!(await waitForProcessGroupExit(pid, PROCESS_EXIT_GRACE_MS))) {
+    throw new Error(`ACP adapter process group ${pid} did not exit after forced termination`)
+  }
+}
+
 /** Maps a raw ACP `SessionUpdate` onto Vellum's `AcpUpdate` contract.
  * Exported standalone so the mapping can be unit-tested without a subprocess. */
 export function mapSessionUpdate(update: SessionUpdate): AcpUpdate {
@@ -223,6 +300,7 @@ class StdioAcpSession implements AcpSession {
   // session, mirroring the ACP protocol (a session has one active turn).
   private queue: UpdateQueue | undefined
   private disposed = false
+  private disposePromise: Promise<void> | undefined
   // Set only while a prompt() call is in flight; cleared by dispose() too so
   // an external dispose mid-turn doesn't leave a dangling timer.
   private turnTimeout: { cancel(): void } | undefined
@@ -280,8 +358,7 @@ class StdioAcpSession implements AcpSession {
       settled = true
       queue.push({ kind: 'error', data: { message: err.message } })
       queue.close()
-      this.disposed = true
-      this.child.kill()
+      void this.dispose().catch(() => undefined)
     })
 
     try {
@@ -293,13 +370,14 @@ class StdioAcpSession implements AcpSession {
   }
 
   async dispose(): Promise<void> {
-    if (this.disposed) return
+    if (this.disposePromise) return this.disposePromise
     this.disposed = true
     this.turnTimeout?.cancel()
     this.turnTimeout = undefined
     this.queue?.close()
     this.queue = undefined
-    this.child.kill()
+    this.disposePromise = terminateAdapter(this.child)
+    return this.disposePromise
   }
 }
 
@@ -372,7 +450,7 @@ export class StdioAcpClient implements AcpClient {
       return session
     } catch (err) {
       handshake.cancel()
-      child.kill()
+      await terminateAdapter(child)
       // Whether the transport's generic "connection closed" error or the
       // child's own exit/spawn diagnostic wins the race above is a timing
       // accident. `failure` settles once the child actually exits/errors