fix: let the Codex session exit on stdin EOF before killing its tree

docs/adapters/codex.md

@@ -35,6 +35,9 @@ For existing integrations, follow the [SDK 4.x to 5.x migration](../acp.md#upgra
   close errors and close timeouts retain the claim for retry. A timed-out or
   cancelled caller does not abandon subprocess cleanup; retry room cleanup to
   finish releasing ownership. Shutdown attempts every room and reports failures.
+  On POSIX, close sends stdin EOF and gives the session a bounded chance to
+  exit before reaping remaining descendants. Windows shutdown does not promise
+  session-visible stdin EOF.
   On Windows, the client owns the helper and its process-tree job from startup
   through cleanup. It starts the interpreter image directly so a virtual-environment
   launcher cannot hide another helper process. Cleanup confirms the helper has

src/band/integrations/codex/stdio_client.py

@@ -30,6 +30,9 @@
 # Long enough for the child watcher to record an exit, short enough that a
 # held stdio pipe cannot stall room cleanup.
 _EXIT_OBSERVE_S = 2.0
+# How long a closed stdin gets to end the whole Codex session before it is killed.
+_SESSION_EXIT_S = 1.0
+_SESSION_POLL_S = 0.02
 _STDOUT_LIMIT = 16 * 1024 * 1024
 HELPER_PATH = Path(__file__).resolve().with_name("winmain.py")
 
@@ -112,6 +115,30 @@ async def kill_process_tree(process: asyncio.subprocess.Process) -> None:
         return
 
 
+async def _await_session_exit(
+    process: asyncio.subprocess.Process, timeout: float
+) -> bool:
+    """True once the child and every other process in its session are gone.
+
+    The session id stays reserved while any member lives, so probing the
+    group is exact; the launcher alone exiting is not enough, since its
+    real CLI child would keep the room's working directory.
+    """
+    loop = asyncio.get_running_loop()
+    deadline = loop.time() + timeout
+    while True:
+        try:
+            os.killpg(process.pid, 0)
+        except ProcessLookupError:
+            return True
+        except PermissionError:
+            pass
+        remaining = deadline - loop.time()
+        if remaining <= 0:
+            return False
+        await asyncio.sleep(min(_SESSION_POLL_S, remaining))
+
+
 class CodexStdioClient(BaseJsonRpcClient):
     """Bidirectional JSON-RPC client with a single stdout read loop."""
 
@@ -258,8 +285,11 @@ async def _close_process(self) -> None:
                 await self._windows_process.close(_EXIT_OBSERVE_S)
                 self._windows_process = None
             elif (process := self._proc) is not None:
-                await kill_process_tree(process)
+                # Codex and its launcher exit on their own once stdin closes; the
+                # tree kill is the fallback for a session that does not.
                 await close_stdin(process)
+                if not await _await_session_exit(process, _SESSION_EXIT_S):
+                    await kill_process_tree(process)
                 if not await await_returncode(process, _EXIT_OBSERVE_S):
                     logger.warning(
                         "Codex process %s still alive after tree kill", process.pid

tests/integrations/codex/test_cleanup.py

@@ -223,8 +223,7 @@ async def test_forced_close_stops_what_a_launcher_shim_spawned(
     async def test_close_reaps_a_worker_when_its_launcher_exits_on_stdin_eof(
         self, tmp_path: Path
     ) -> None:
-        """A launcher that exits on stdin EOF would otherwise leave its child
-        holding the workdir."""
+        """POSIX launchers observe stdin EOF; every platform reaps their worker."""
         workdir = tmp_path / "room"
         workdir.mkdir()
         client = CodexStdioClient(
@@ -237,7 +236,8 @@ async def test_close_reaps_a_worker_when_its_launcher_exits_on_stdin_eof(
             await asyncio.wait_for(wait_for_heartbeat(heartbeat), timeout=CLOSE_BOUND_S)
             await asyncio.wait_for(client.close(), timeout=CLOSE_BOUND_S)
             await assert_heartbeat_stopped(heartbeat)
-            assert not (workdir / STDIN_EOF_MARKER).exists()
+            if sys.platform != "win32":
+                assert (workdir / STDIN_EOF_MARKER).exists()
             shutil.rmtree(workdir)
         finally:
             await asyncio.wait_for(client.close(), timeout=CLOSE_BOUND_S)

tests/integrations/codex/test_stdio_client.py

@@ -408,3 +408,17 @@ async def test_stdio_client_eof_fails_pending_and_enqueues_disconnect(
     event = await client.recv_event(timeout_s=1.0)
     assert event.method == "transport/closed"
     await client.close()
+
+
+@pytest.mark.asyncio
+@pytest.mark.skipif(os.name == "nt", reason="Windows closes through the job object")
+async def test_close_lets_codex_exit_on_stdin_eof_before_killing_its_tree(
+    fake_codex_server_script: Path,
+) -> None:
+    """A harness that exits once its stdin closes ends on its own terms; the
+    tree kill is the fallback for one that does not."""
+    client = await _build_client(fake_codex_server_script, scenario="basic")
+    process = client._proc
+    assert process is not None
+    await client.close()
+    assert process.returncode == 0