dispose() ended stdin and sent SIGTERM in the same tick, so the child's
EOF-driven quiesce had no window to run. The real acp-agent has no SIGTERM
handler in a normal session — it flushes persistence and stops child-owned
work via the server bridge's connection-close path (conn.closed → per-agent
dispose → final session/flush), driven by stdin EOF, NOT by a signal. A prompt
response can resolve from a turn/end before that post-turn flush lands, so the
child still owes durable work when dispose runs; a same-tick default SIGTERM
terminated it mid-flush, orphaning child-owned bash and dropping the flush.
dispose now waits for the child's natural exit after stdin EOF first, then
escalates SIGTERM (grace), then SIGKILL — a three-tier ladder. Add an
`exitsWithin` helper for the bounded waits.
Regression coverage: a new mock mode (MOCK_FLUSH_ON_EOF) flushes a marker
asynchronously on EOF then self-exits; the tier-1 test asserts the marker
lands (proven RED on the same-tick-SIGTERM ordering — child killed mid-flush).
MOCK_IGNORE_EOF covers the middle tier (ignores EOF, dies on default SIGTERM);
the existing MOCK_TRAP_SIGTERM test covers the SIGKILL tier.
Two lifecycle findings from the review:
- A (blocker): dispose() could hang forever. It only sent SIGTERM and awaited
exit, with no escalation — a child that traps SIGTERM (or our acp-agent if it
doesn't quiesce on stdin EOF) would wedge dispose, stranding tool-subagent's
finally cleanup and orphaning child-owned work (e.g. bash subprocesses). dispose
now: ends stdin (graceful ACP close so the child can flush + exit), SIGTERM,
then escalates to SIGKILL if it doesn't exit within a grace period
(DEFAULT_DISPOSE_GRACE_MS, injectable via spec.disposeGraceMs), awaiting the
certain exit. Mirrors the bash executor's bounded teardown. Regression test
drives a SIGTERM-trapping mock subprocess and asserts dispose returns promptly
— proven to hang (red) without the escalation.
- B: an already-aborted request still spawned the configured binary. startAcpRun
now returns an inert already-aborted run BEFORE spawning, so a pre-cancelled
request launches nothing. Test points the command at `touch <sentinel>` and
asserts the sentinel never appears.
The dispose regression test exposed (via systematic-debugging) that the child
must signal trap-armed readiness before the test cancels — a bare timeout raced
the trap install and the default SIGTERM handler killed the child, making the
guard a no-op. The mock now touches its ready file once the trap is in place and
the test waits on that condition. The `cancelled` flag moved onto a holder object
so TS control-flow doesn't narrow the catch-time read to always-false.
The first OUT-OF-PROCESS subagent backend, proving the seam generalizes past the
in-process backends. @deepseek-ai/dsh-subagent-acp runs each child agent in a
spawned subprocess, driven over the Agent Client Protocol as the CLIENT — the
direction-inverted twin of the dsh-acp server bridge. Point the configured
command at the acp-agent example and the harness talks to its own process.
- Fresh process per run: start spawns, runs one ACP session (initialize →
newSession → prompt), dispose kills the subprocess and awaits its exit.
- Minimal client stub: advertises no fs/terminal; accumulates agent_message_chunk
text as the result output; auto-answers session/request_permission by a
configured policy (reject default / allow). No start-time capabilities (an
out-of-process child can't enforce the parent's depth/tool-filter); ignores
request.parent; injects only `subagents`.
- StopReason mapping (end_turn→completed, cancelled→aborted, …); result resolves
error/aborted on a child failure, never rejects (seam contract).
- Security: credential-shaped ambient env vars are scrubbed; the child's own key
is forwarded only via explicit config.env. A spawn-level error (ENOENT) is
captured and raced against the ACP drive so a bad command settles error rather
than crashing the parent.
Testing designed at every tier: keyless integration drives a scripted mock ACP
server subprocess (cancellation incl. the pre-newSession race and a
torn-pipe-after-cancel, permission auto-answer, non-message updates, spawn
failure, HMR, export shape) at 100% coverage; a with-key e2e drives the REAL
acp-agent example process (PONG + real file write, verified on disk) — the
harness driving itself. Snapshot coverage of an ACP child is deferred as
TODO(acp-subagent-replay) (each child is its own process with its own replay).
Stayed on @agentclientprotocol/sdk 0.25.1: the proposed 0.28.x bump only
deprecates the stable ClientSideConnection/AgentSideConnection API this layer
uses (33 sites incl. the server bridge), turning no-deprecated red across code
this PR shouldn't rewrite — that fluent-API migration is its own follow-up. The
backend needs nothing 0.28.x adds.
This completes the subagent seam stack (PR1 interface → PR2 in-process → PR2.5
snapshot infra → PR3 ACP); the seam RFC moves to implemented/, amended.
