SIGN IN SIGN UP

fix(byoa): kill the one-shot engine child when its runner is stopped (#31)

* fix(byoa): kill the one-shot engine child when its runner is stopped

EngineRunArgs.signal is documented as "Aborts the run (daemon shutdown /
future mid-run steering)", and spawnEngine wires it to child.kill('SIGTERM').
But nothing ever called abort() on it: runTurn built an AbortController it never
aborted, maybeAgendaTurn passed `new AbortController().signal` that no one even
held, and AgentRunner.stop() tore down only the persistent engineSession.

So on the one-shot path — codex on Windows, CUMORA_CODEX_NO_APP_SERVER=1, a
CUMORA_CODEX_ARGS/CUMORA_CLAUDE_ARGS override, or codex where
ensureGitRepoForCodex throws — a stopped runner left its `codex exec` /
`claude -p` child running. The orphan keeps a valid runtime token and the
`cumora` shim on PATH, so it goes on posting AS the agent while the replacement
runner independently answers the same unacked messages: duplicate, conflicting
replies from one teammate.

sync() is where this really bites, not shutdown: it tears a runner down on any
engine/model/persona change or unassign while the daemon keeps running, so the
orphan is still acting with the OLD persona the operator just replaced — the
opposite of what the config-change path promises. `--stop` reports "agents are
fully offline" while killRunningDaemons only matches the daemon's own command
line, never engine children.

Give AgentRunner one lifetime AbortController, hand its signal to both one-shot
run() call sites, and abort it in stop() beside the existing engineSession.stop()
— making teardown symmetric.

One supporting line in engine.ts: spawnEngine registers its abort listener after
spawning, and a listener added after the abort event never fires, so a turn still
queued behind the concurrency gate when stop() landed would spawn an unowned
child even with the daemon fix. `if (signal.aborted) onAbort()` closes that.

* fix(byoa): settle an aborted turn on `exit`, not `close`

CI caught the first version of this on Linux: both abort tests hung for the full
race budget and failed. The cause is real and not test-only.

`spawnEngine` resolves on the child's `close` event, which waits for every
inherited stdio pipe to reach EOF. The engine's OWN children hold those pipes —
`claude` spawns one per Bash tool command — so a grandchild that outlives the
kill keeps them open and `close` may never fire at all. Confirmed directly:

  exit  fired at: 3 ms
  close fired at: NEVER (grandchild still holds the pipe)

So killing the engine stopped the orphan from posting, but the turn's promise
never resolved: the daemon's shutdown drain would wait forever on a turn it had
already killed, and `busy` plus the big-brain slot would never be released.

Once the run is aborted its remaining output is moot, so settle on `exit` and
leave the normal path on `close` (which still waits for the last of stdout).
Both routes go through one guarded `settle`, so whichever fires first wins.

The tests were also platform-dependent: the fake engine was a `#!/bin/sh` script
running `sleep`, and macOS's /bin/sh is bash, which EXECs a lone last command
(one process), while Linux's dash forks it (grandchild holds the pipes). They now
use a Node fake that explicitly spawns a stdio-inheriting child that outlives it
— the shape a real engine actually has — so the process tree is identical
everywhere. The race budget also goes to 15s so a loaded CI runner can never be
mistaken for an orphan.
X
Xialie Zhuang committed
5a8a08f4f1eea504e329b4858dce0643cabb8fd0
Parent: 2b3b0bf
Committed by GitHub <noreply@github.com> on 8/20/2026, 12:13:55 AM