fix(native-chat): require a proven exit before handing off the thread (#15162)

Native-to-TUI handoff called closeSession, but connection close stopped waiting
after a fixed forced-exit deadline even when the child never emitted 'exit' and
the kill request itself had failed. Handoff then durably recorded
`exit-observed`, advanced the fence, and launched a TUI against the same
provider thread while the app-server could still be alive and streaming — two
live writers on one Codex session.

`close()` now reports whether exit was actually observed, and the forward
handoff refuses on an unproven one rather than advancing the fence. The gate
lives at the forward call site, where the fence decision is made, so a caller
cannot bypass it by ignoring the returned proof.

Two supporting changes: on POSIX the kill reaped only the direct child, missing
the npm-shim grandchild the Windows branch's own comment warns about; and
recovered-owner termination sent SIGTERM without escalating. SIGTERM is a
request — it now escalates once and requires an independent absence probe
before the lease transition proceeds.

Verified: 2148 tests across ai-vault/codex/native-chat, node typecheck, oxlint
clean. Mutation: stubbing out the forward gate turns the exit-proof test red.

src/main/codex/codex-app-server-client.test.ts

@@ -195,16 +195,21 @@ describe('killCodexAppServerProcessTree', () => {
     expect(child.kill).toHaveBeenCalledWith('SIGKILL')
   })
 
-  it('kills the direct app-server process on non-Windows hosts', () => {
+  it('kills the launcher descendants before the direct 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
+    const descendants = { unref: vi.fn(), on: vi.fn() }
+    const spawnImpl = vi.fn(() => descendants) as unknown as typeof spawn
 
     killCodexAppServerProcessTree(child, { platform: 'linux', spawnImpl })
 
-    expect(spawnImpl).not.toHaveBeenCalled()
+    expect(spawnImpl).toHaveBeenCalledWith('pkill', ['-KILL', '-P', '1234'], { stdio: 'ignore' })
+    // A missing pkill arrives as an async 'error' event; unhandled, it would
+    // take down the main process.
+    expect(descendants.on).toHaveBeenCalledWith('error', expect.any(Function))
+    expect(descendants.unref).toHaveBeenCalledOnce()
     expect(child.kill).toHaveBeenCalledWith('SIGKILL')
   })
 })

src/main/codex/codex-app-server-connection-types.ts

@@ -22,5 +22,6 @@ export type CodexAppServerConnection = {
   notify: (method: string, params?: Record<string, unknown>) => void
   respond: (id: number | string, result: unknown) => void
   respondWithError: (id: number | string, code: number, message: string) => void
-  close: () => Promise<void>
+  /** Resolves true only after the child emitted `exit`; false is unproven. */
+  close: () => Promise<void | boolean>
 }

src/main/codex/codex-app-server-connection.test.ts

@@ -307,6 +307,22 @@ describe('openCodexAppServerConnection', () => {
     expect(child.kill).toHaveBeenCalledWith('SIGKILL')
   })
 
+  it('reports unproven close when forced termination did not produce an exit event', async () => {
+    vi.useFakeTimers()
+    const { child, spawnImpl } = stubChild({ exitOnStdinEnd: false })
+    answerInitialize(child)
+    const connection = await openCodexAppServerConnection(
+      { command: 'codex', args: ['app-server'] },
+      {},
+      spawnImpl
+    )
+
+    const closing = connection.close()
+    await vi.advanceTimersByTimeAsync(2_600)
+
+    await expect(closing).resolves.toBe(false)
+  })
+
   it('ends the connection rather than buffering an oversized line', async () => {
     const { child, spawnImpl } = stubChild({ exitOnStdinEnd: false })
     answerInitialize(child)

src/main/codex/codex-app-server-connection.ts

@@ -86,6 +86,7 @@ export async function openCodexAppServerConnection(
   let stderrTail = ''
   let nextRequestId = 1
   let exited = false
+  let exitObserved = false
   let closing = false
   /** First terminal cause, or null while the transport is still usable. Set once:
    *  a child that dies reaches us through several listeners, and the specific
@@ -95,6 +96,7 @@ export async function openCodexAppServerConnection(
   const exitPromise = new Promise<void>((resolve) => {
     child.on('exit', () => {
       exited = true
+      exitObserved = true
       resolve()
     })
   })
@@ -127,7 +129,6 @@ export async function openCodexAppServerConnection(
   }
 
   child.on('error', (error) => {
-    exited = true
     handleUnexpectedEnd(error)
   })
   // Why: 'close' rather than 'exit' guarantees the stderr tail is complete, so
@@ -291,10 +292,9 @@ export async function openCodexAppServerConnection(
     }
   }
 
-  async function close(): Promise<void> {
+  async function close(): Promise<boolean> {
     if (closing) {
-      await exitPromise
-      return
+      return exitObserved
     }
     closing = true
     try {
@@ -310,6 +310,7 @@ export async function openCodexAppServerConnection(
       }
     }
     failPending(new Error('codex app-server connection closed'))
+    return exitObserved
   }
 
   const connection: CodexAppServerConnection = {

src/main/codex/codex-app-server-session.ts

@@ -88,6 +88,21 @@ export function killCodexAppServerProcessTree(
       // Fall through to the direct-child best effort when taskkill cannot start.
     }
   }
+  if (child.pid) {
+    try {
+      // npm/package-manager launchers insert a shim child on POSIX. Reap its
+      // direct descendants before signalling the wrapper itself.
+      const descendants = spawnImpl('pkill', ['-KILL', '-P', String(child.pid)], {
+        stdio: 'ignore'
+      })
+      // A missing pkill surfaces as an async 'error' event, and an unhandled one
+      // takes down the main process.
+      descendants.on('error', () => undefined)
+      descendants.unref()
+    } catch {
+      // The direct kill below remains the fallback when pkill is unavailable.
+    }
+  }
   child.kill('SIGKILL')
 }
 

src/main/codex/codex-structured-session-adapter.ts

@@ -291,14 +291,14 @@ export class CodexStructuredSessionAdapter implements StructuredAgentSessionAdap
 
   /** Reaps one session's child. The proven handle chain is already durable, so
    *  a graceful close loses nothing. */
-  async closeSession(sessionId: string): Promise<void> {
+  async closeSession(sessionId: string): Promise<void | boolean> {
     const attempt = this.acquisitions.get(sessionId)
     if (attempt) {
       attempt.cancelled = true
       await attempt.window.connection?.close()
       await attempt.finished
     }
-    await closeCodexPublishedSession(this.sessions, sessionId, this.deps.onEvent)
+    return closeCodexPublishedSession(this.sessions, sessionId, this.deps.onEvent)
   }
 
   async closeAll(): Promise<void> {
@@ -309,8 +309,9 @@ export class CodexStructuredSessionAdapter implements StructuredAgentSessionAdap
     }
   }
 
-  releaseAcquisition = (input: { sessionId: string }): Promise<void> =>
-    this.closeSession(input.sessionId)
+  releaseAcquisition = async (input: { sessionId: string }): Promise<void> => {
+    await this.closeSession(input.sessionId)
+  }
 
   private session(sessionId: string): CodexSession {
     const session = this.sessions.get(sessionId)

src/main/codex/codex-structured-session-close.ts

@@ -4,13 +4,19 @@ export async function closeCodexPublishedSession(
   sessions: Map<string, CodexSession>,
   sessionId: string,
   onEvent?: (event: CodexStructuredSessionEvent) => void
-): Promise<void> {
+): Promise<boolean> {
   const session = sessions.get(sessionId)
   if (!session) {
-    return
+    return true
+  }
+  session.prompts.clear()
+  // Keep the session indexed until the child exit is observed. A timeout or
+  // failed kill must leave the live connection available for a safe retry.
+  const exited = await session.connection.close()
+  if (exited === false) {
+    return false
   }
   sessions.delete(sessionId)
-  session.prompts.clear()
   const event: CodexStructuredSessionEvent = {
     type: 'ended',
     sessionId,
@@ -20,5 +26,5 @@ export async function closeCodexPublishedSession(
   onEvent?.(event)
   session.translator?.flush()
   session.translator?.dispose()
-  await session.connection.close()
+  return true
 }

src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts

@@ -102,5 +102,5 @@ export type StructuredAgentSessionAdapter = {
    *  resolver discover it from the provider session id. */
   historyFilePath?(input: { identity: AgentSessionJournalIdentity }): Promise<string | null>
   /** Gracefully stops the structured owner after its event stream is drained. */
-  closeSession?(sessionId: string): Promise<void>
+  closeSession?(sessionId: string): Promise<void | boolean>
 }

src/main/native-chat/agent-session-wire/structured-agent-session-handoff-forward.ts

@@ -33,7 +33,12 @@ export async function handoffStructuredSessionToTui(
     throw new Error('agent_session_operation_conflict')
   }
   record = context.requireRecord(sessionId)
-  await deps.suspendNative(sessionId)
+  // A kill is a request. Everything below advances the fence and hands the
+  // provider session to a TUI, so an unproven exit must stop here rather than
+  // create a second live writer on the same thread.
+  if ((await deps.suspendNative(sessionId)) === false) {
+    throw new Error('agent_session_owner_exit_unproven')
+  }
   record = await stopStoredAgentSessionOwnerForHandoff(deps.store, {
     sessionId,
     expectedFence: record.lease.runtimeFence,

src/main/native-chat/agent-session-wire/structured-agent-session-handoff-options.test.ts

@@ -39,6 +39,8 @@ let activeModel: string
 let activeEffort: string | null
 let transcriptPath: string
 let optionFailure: Error | null
+/** What the adapter's closeSession reports about the child's exit. */
+let closeSessionExit: boolean | undefined
 const dispatchedModels: string[] = []
 const launchedOptions: (Readonly<Record<string, string>> | undefined)[] = []
 const closedTuiOwners: StructuredTuiOwner[] = []
@@ -158,6 +160,7 @@ function adapter(): StructuredAgentSessionAdapter {
     })),
     closeSession: vi.fn(async () => {
       activeModel = DEFAULT_MODEL
+      return closeSessionExit
     })
   }
 }
@@ -168,6 +171,7 @@ beforeEach(async () => {
   activeModel = DEFAULT_MODEL
   activeEffort = null
   optionFailure = null
+  closeSessionExit = undefined
   dispatchedModels.length = 0
   launchedOptions.length = 0
   closedTuiOwners.length = 0
@@ -282,4 +286,17 @@ describe('structured session handoff options', () => {
     ).toMatchObject({ ok: true })
     expect(dispatchedModels).toEqual([PICKED_MODEL])
   })
+
+  // A kill is a request. If the app-server never proved it exited, handing the
+  // thread to a TUI would put two live writers on one provider session.
+  it('refuses the handoff when closing the native owner cannot prove it exited', async () => {
+    closeSessionExit = false
+
+    expect(await host.requestHandoff(CALLER, handoff('to-tui'))).toMatchObject({ ok: true })
+    await vi.waitFor(async () =>
+      expect(await host.handoffStatus(SESSION)).toMatchObject({ phase: 'failed' })
+    )
+    expect(launchedOptions).toEqual([])
+    expect(store.getRecord(SESSION)?.lease.runtimeKind).toBe('native')
+  })
 })

src/main/native-chat/agent-session-wire/structured-agent-session-handoff-types.ts

@@ -55,7 +55,7 @@ export type StructuredAgentSessionHandoffDeps = {
   claimKeyId: string
   transport?: StructuredAgentSessionHandoffTransport
   session: (sessionId: string) => { journal: AgentSessionJournal; fence: number }
-  suspendNative: (sessionId: string) => Promise<void>
+  suspendNative: (sessionId: string) => Promise<void | boolean>
   acquireNative: (input: {
     sessionId: string
     fence: number

src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts

@@ -68,9 +68,14 @@ export function createStructuredAgentSessionHostHandoff(
     ...(deps.handoffTransport ? { transport: deps.handoffTransport } : {}),
     session: host.session,
     suspendNative: async (sessionId) => {
-      await deps.adapter.closeSession?.(sessionId)
+      const exited = await deps.adapter.closeSession?.(sessionId)
+      if (exited === false) {
+        // Report the unproven exit; the forward handoff refuses on it.
+        return false
+      }
       await host.flush(sessionId)
       host.eventSink(sessionId).unbind()
+      return true
     },
     acquireNative: (input) => acquireNativeHandoffOwner(deps, host, input),
     acquireNativeStop: async (sessionId, turnId, fence) =>

src/main/runtime/orca-runtime.ts

@@ -10800,7 +10800,25 @@ export class OrcaRuntimeService {
       }
       await new Promise((resolve) => setTimeout(resolve, 100))
     }
-    throw new Error('The recovered owner process did not exit.')
+    // SIGTERM is only a request. Escalate once, then require an independent
+    // absence probe before allowing the lease transition to proceed.
+    try {
+      process.kill(identity.pid, 'SIGKILL')
+    } catch (error) {
+      if ((error as NodeJS.ErrnoException).code !== 'ESRCH') {
+        throw error
+      }
+      return
+    }
+    const forcedDeadline = Date.now() + 5_000
+    while (Date.now() < forcedDeadline) {
+      const current = await probeAgentSessionProcessIdentity({ identity })
+      if (current.outcome === 'pid-absent' || current.outcome === 'identity-mismatch') {
+        return
+      }
+      await new Promise((resolve) => setTimeout(resolve, 100))
+    }
+    throw new Error('The recovered owner process did not exit after forced termination.')
   }
 
   private structuredTuiStatus(owner: StructuredTuiOwner): 'idle' | 'busy' {