Skip to content

v1.10.5 ·

expected false to be true: a flake that stopped a release, and a prompt no one could answer

The problem

firePluginHooks and emit run hook/action commands with execSync(cmd, { input, stdio: ['pipe','pipe','pipe'], timeout: 30_000 }). spawnSync forks first and writes the stdin payload afterwards, so a command that never reads stdin can exit before that write lands — the write then hits a closed pipe and spawnSync throws EPIPE for a command that ran and exited 0. The race is deterministic above the 64 KiB pipe buffer and load-dependent below it; a 2-core CI runner executing three package suites concurrently starves the parent long enough to lose it with a ~100-byte payload. All three levels of the code then discard the error, so the failure surfaces as expected false to be true and nothing else. Full evidence, and the falsification of the "30-second timeout" hypothesis, in reproduction.md.

How it could be solved

Both fixes in this release had an obvious cheap option, and both rejected it for the same reason: it would have hidden the failure rather than removed it.

For the flake, the cheap option was already in the tree. A comment in packages/cli/vitest.config.ts had been retrying spawning tests twice in CI since 2026-08-16, on the theory that they lose a race to runner load. The next step along that path was a serial lane — vitest's fileParallelism: false for the suites that shell out. That was rejected once the evidence came in. The failing test file ran in 81 milliseconds and all three of its attempts failed inside that window, which rules out the 30-second timeout the retry was hedging against and rules out contention as something a retry can outlast. Worse, a serial lane fixes only the test: the same race lives in aidlc transition on a user's machine, where nothing retries anything.

Raising the timeout was rejected on the same arithmetic. The command was echo — a shell builtin, no external binary — and nothing about a loaded runner makes it take half a minute.

What the probe actually found was a race between the parent's write and the child's exit. spawnSync forks first and writes the stdin payload afterwards, so a hook that never reads stdin can exit and close the pipe before the write lands, and the write then fails with EPIPE for a command that ran and exited zero. It reproduces deterministically at exactly one byte past the 64 KiB pipe buffer, on both Node 22 and Node 26, and it fails at millisecond speed — which is why three retries in a row made no difference.

So the fix stops treating the parent's write as a verdict on the child. Blanket-swallowing EPIPE was rejected, because it would mask a hook that genuinely failed; the discriminator is the exit status, which spawnSync still reports alongside the error. Dropping the stdin payload entirely was rejected too — hooks are documented to receive the event on stdin, and taking that away to dodge a race would break the contract to fix the plumbing. Retrying every spawn error was rejected as too broad: only transient resource errnos get a second attempt, and the vitest retry stays as a backstop for failures no code can prevent, no longer as cover for a known defect.

The second half of that fix is legibility, and it is the half that mattered most. The error text existed all along — the dispatch report carried it, emit never printed it, and the assertion was a bare boolean — so a release-blocking failure printed expected false to be true and nothing else. Failures now carry exit status, signal, errno and the child's stderr; non-blocking action failures are written to stderr, because aidlc transition discards the report; and every assertion on a spawned command prints what it captured. That change proved itself the same afternoon, on an unrelated website test that had been failing with expected 1 to be +0 and started naming its own cause instead.

The prompt-hang fix faced a different version of the same choice: guard the twelve call sites that were reported, or make the guard inheritable. Twelve local fixes would have left the thirteenth call site free to reintroduce the defect, so the predicate moved to one module in core — deliberately not in menu, because menu imports the command registrars and the other direction would be an import cycle — and an enumeration test now walks the registered commands and fails if any of them can exit 13 or hang without a terminal, with exactly two documented exclusions.

Its consent gates went further than better messages. A prompt nobody can see is not a question, so with no terminal add skill writes nothing, install-merge-driver writes nothing, and add action declines to save even an action it could have inferred. The alternative — treating an unanswerable prompt as agreement — is how a tool authorises a write no one approved.

How AIDLC solves it

subprocess-test-flake

The lifecycle event dispatcher reported commands as failed when they had succeeded. execSync with an input payload throws EPIPE if the child exits before the parent's stdin write lands — and a hook like echo never reads stdin, so it usually does. The race is deterministic above the 64 KiB pipe buffer and load-dependent below it, which is how it stayed invisible for two weeks and then aborted the v1.10.2 release at pnpm test, having failed all three CI retries. Retrying could not have helped: the attempts run inside the same starved window.

The fix stops treating the parent's stdin write as a verdict on the child. One shared runner (packages/cli/src/events/run-command.ts) now classifies the raw spawnSync result for both lifecycle actions and plugin hooks: a clean exit is a success even when the payload could not be delivered, a non-zero exit still fails, a timeout kill is reported as a timeout rather than a bad exit code, and a transient spawn errno (EAGAIN, EMFILE, ENFILE, ENOMEM) buys one retry.

The second half of the fix is legibility, because the first failure was diagnosable only by reproducing it from scratch. Failures now carry exit status, signal, errno and the child's stderr; emit writes non-blocking action failures to stderr, as plugin hooks already did — aidlc transition discards the report, so nothing said them out loud; and every assertion on a spawned command prints the captured error. That last change proved itself the same day: a website test that failed with expected 1 to be +0 now names the generator's own complaint instead.

17 new tests force the race deterministically with an oversized payload rather than waiting for load. Four of them fail against the pre-fix code, which is the only evidence that they cover the defect rather than the patch.

tty-prompt-guard

FileChange
core/tty.tsNew. isInteractive() (both stdin.isTTY and stdout.isTTY) + shared NO_TTY_REASON clause
core/index.tsRe-exports tty.js
menu/prompt.tsisInteractive re-exported from core/tty.js instead of defined here; the both-streams rationale moved to the definition
commands/start.tsTemplate picker joins the --non-interactive branch, reason clause per condition; concurrency gate off !process.stdout.isTTY
commands/init.tsNew else if (!isInteractive()) arm ahead of the wizard; cost-hooks consent gains && isInteractive()
commands/add-action.tsEvent picker joins the --non-interactive branch; save confirm gets its own exit-1 arm
commands/add-skill.tsGeneration consent fails closed, after the --dry-run return
commands/knowledge.tsinstall-merge-driver consent fails closed
doctor/context.tsinteractive default off process.stdout.isTTY
test/tty-prompt-guard.test.tsNew, 26 tests
test/cli-start.test.tsOne assertion reworded for the new message