Merge branch 'codex/simp-hide-concrete-agent-loop' into codex/simp-hide-subagent-internals

packages/subagent/subagent-acp/src/run.ts

@@ -311,7 +311,9 @@ export async function startAcpRun(request: SubagentStartRequest, spec: AcpRunSpe
           clientCapabilities: {},
         })
         const session = await conn.newSession({ cwd: spec.cwd, mcpServers: [] })
-        sessionId = session.sessionId
+        const returnedSessionId: unknown = Reflect.get(session, 'sessionId')
+        if (typeof returnedSessionId !== 'string') throw new Error('ACP child published without a session id')
+        sessionId = returnedSessionId
         if (flags.cancelled) throw new Error('subagent cancelled before the ACP session started')
       })(),
       spawnFailed.then((err): never => { throw err }),
@@ -323,10 +325,9 @@ export async function startAcpRun(request: SubagentStartRequest, spec: AcpRunSpe
     if (flags.cancelled) throw new Error('subagent request was aborted before the ACP child started')
     throw toError(error)
   }
-  // The startup race can fulfill only after newSession assigned the id; this
-  // guard keeps that cross-closure invariant explicit for TypeScript.
-  /* v8 ignore next */
-  if (sessionId === undefined) throw new Error('ACP child published without a session id')
+  // The startup transaction validates the returned id before it can fulfill.
+  // This assertion carries that cross-closure invariant into TypeScript.
+  if (sessionId === undefined) throw new Error('unreachable: ACP startup fulfilled without a session id')
   const remoteSessionId = sessionId
 
   const result: Promise<SubagentResult> = (async (): Promise<SubagentResult> => {

packages/subagent/subagent-acp/tests/mock-acp-server.ts

@@ -19,6 +19,8 @@
  *                        handler is in flight (it has streamed its chunk). A test
  *                        polls for this file to cancel on a CONDITION rather than
  *                        an arbitrary timeout (subprocess cold-start is variable).
+ * - `MOCK_MISSING_SESSION_ID` — if `1`, return a malformed empty `session/new`
+ *                        response to exercise startup rollback.
  * - `MOCK_FLUSH_ON_EOF` — if set, on stdin EOF the agent takes an async beat
  *                         (MOCK_FLUSH_DELAY_MS, default 150) simulating the real
  *                         acp-agent's EOF-driven quiesce+flush, then touches this
@@ -99,6 +101,7 @@ function makeAgent(conn: AgentSideConnection): Agent {
         writeFileSync(NEWSESSION_GATE.ready, 'at-newSession')
         while (!existsSync(NEWSESSION_GATE.go)) await new Promise(r => setTimeout(r, 10))
       }
+      if (process.env.MOCK_MISSING_SESSION_ID === '1') return {} as NewSessionResponse
       return { sessionId: process.env.MOCK_SESSION_ID ?? randomUUID() }
     },
     authenticate(_params: AuthenticateRequest): Promise<void> {

packages/subagent/subagent-acp/tests/subagent-acp.spec.ts

@@ -194,6 +194,32 @@ describe('dsh-subagent-acp', () => {
     }
   })
 
+  it('reaps a child whose session/new response omits the session id', async () => {
+    const tmp = mkdtempSync(join(tmpdir(), 'acp-malformed-session-'))
+    const flushed = join(tmp, 'flushed')
+    try {
+      await expect(startAcpRun(request(), {
+        command: process.execPath,
+        args: ['--import', tsxLoader, mockServer],
+        cwd: process.cwd(),
+        permission: 'reject',
+        env: {
+          MOCK_MISSING_SESSION_ID: '1',
+          MOCK_FLUSH_ON_EOF: flushed,
+          MOCK_FLUSH_DELAY_MS: '20',
+          TSX_TSCONFIG_PATH: repoTsconfig,
+        },
+        disposeEofGraceMs: 1000,
+        disposeGraceMs: 100,
+      })).rejects.toThrow('ACP child published without a session id')
+      // Startup rejects only after its private child reaches quiescence. The
+      // marker proves rollback closed stdin and allowed the child's EOF flush.
+      expect(existsSync(flushed)).toBe(true)
+    } finally {
+      rmSync(tmp, { recursive: true, force: true })
+    }
+  })
+
   it('dispose escalates SIGTERM → SIGKILL for a child that traps SIGTERM (bounded quiescence)', async () => {
     // The child traps SIGTERM and keeps its event loop alive, so a graceful
     // term alone would hang dispose forever. With a short grace, dispose must