vish engineering notebook

Postmortem

When your agent lies about success

A shell command timed out. The tool reported exit code zero. The agent believed it, the transcript agreed, and every metric downstream was built on a fabricated number.

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.

Why this class of bug is worse than a crash

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:

  1. 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.
  2. 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.
  3. 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.