The bug, in one paragraph
Our shell tool resolves a process close event as
code ?? 0 — if the child reports no exit code, assume
zero, which means success. Separately, a timeout path sets a
timedOut flag and then awaits the process kill
before rejecting. Both the close handler and the timeout handler are
racing for the same promise. If the close event lands during that
await — which is exactly when it lands, since you are in the middle
of killing the process — the close handler wins the race and returns
an ordinary success.
A signal-only termination had the same shape, independently mislabelled as success. The agent asked the shell to do something, the shell was killed, and the tool said: done, exit code 0.
A crash is loud and self-limiting. A false success is quiet and contagious. The agent stops retrying something that never ran. The run is recorded as passing. Every evaluation that includes this transcript now contains a wrong answer that looks like a right one. You cannot find it by reading the score; you can only find it by reading the run.
Fixing the tool is not fixing the bug
The useful part of the investigation was noticing that a second code
path already did the right thing. The platform's own execute path
kills the whole POSIX process group and returns a proper timeout
result. But the agent does not call that path — it calls the managed
shell spawner, which has its own logic. Fixing or testing
execute alone would have produced a green test suite and
left the defect in production.
This is the general lesson about harness bugs: the entry point the product uses is often not the entry point the tests exercise. Before you fix anything, find the call. Otherwise you are repairing a road nobody drives on.
One outcome per invocation
The real fix was structural rather than a patch. Every invocation of the tool must settle into exactly one terminal outcome, and the set is closed:
- exited, with a code
- timed out
- cancelled by the user
- terminated by a signal
- failed to start
- failed during cleanup
Three rules follow from that list, and they are the part worth stealing:
- Settle the outcome before teardown. The timeout or cancellation decision must be recorded before any asynchronous cleanup can fire. A late close event is evidence about cleanup, not about outcome. Do not let it compete in a race it can win.
- Unknown is not zero. If we cannot determine the exit status, the answer is unknown, and unknown is non-success. Coercing a missing value into the happy default is how you manufacture false confidence at the exact moment you have the least information.
- Preserve the evidence. Exit code, signal, termination reason, partial stdout and stderr, and the cleanup result all travel back through the existing adapters to the recorder. A partial transcript of a command that failed is often more useful than a clean transcript of one that succeeded.
What we could reproduce
We wrote tests that characterise the defect before fixing it, then converted those characterisations into regression tests that assert the behaviour we want. That ordering matters and it is easy to get backwards: a test that reproduces today's bug is not certification of the fix, it is a photograph of the problem. Keep both, and label them differently.
The recheck confirms the false success is gone for a mocked child in the shell tool. A headless smoke test now exercises a real shell timeout and recovery on macOS and on packaged Linux. Windows and full descendant-process cleanup are explicitly not certified — we do not claim what we have not run.
Interruption is a separate problem
There is a tempting shortcut here: since we now detect interrupted work, why not have the agent repair what the interruption broke? If a package install was cut off mid-write, package state can be inconsistent even when process cleanup worked perfectly.
We deliberately did not take that shortcut. Automatically running privileged repair commands on a machine whose state you were just surprised by is how you turn a reporting bug into a data-loss incident. Detecting interruption and recovering from it are two different projects with two different risk profiles, and merging them makes both worse.
The generalisable rule
Any place your harness writes a value into a transcript is a place you have made a claim about the world. A fabricated default is a lie with a longer half-life than a crash.
Audit for it directly. Search the codebase for every
?? 0, every || true, every
catch {}, every optional field that gets a cheerful
default before it is persisted. Most of them are fine. The ones that
are not are all in the path between "something happened" and "we
recorded what happened," and that path is the product.