Repository navigation
fix(docs): bound every harness command so a stuck wheels can't hang the run - #4447
Conversation
…he run runExec() had no default timeout and resolved only when the command's output pipes closed, which a leftover process can hold open forever: one stuck wheels call waited out a test's 300 s limit and then hung the verify job. - A default timeout (240 s, WHEELS_EXEC_TIMEOUT_MS) kills the command's whole process group and fails with 'timed out after Ns running <command>'. - It returns 2 s after the command exits even if a leftover process still holds the pipes (not killed: a block may leave a server running). - Commands still running when the harness exits are stopped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com>
|
rev1-r3 review @ 5b6d5cb: CHANGES (one gap in the leftover-process cleanup) The timeout path is sound:
Needs a fix: what happens to the leftover
Minor, not blocking: Gate: PR CI (18 pending). I'll re-review the fix promptly. |
A command that exited while something it started still held its pipes was dropped from the harness-exit cleanup, so a server a block started outlived the harness (and its isolated home). Those groups are now kept and stopped when the harness exits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com>
|
rev1-r3 review @ 941f018: code APPROVE (final pending PR CI)
One limitation, not blocking: this reaches processes that stay in the command's process group. A server that calls Gate: PR CI. I'll post the final when it's green. |
|
rev1-r3 review @ 941f018: APPROVE (final) PR CI is now 20 pass / 0 fail / 0 pending, with 3 skipping. In Ready to merge. Once both this and #4444 are on |
Summary
The docs-harness test "verify-docs reports pass/fail counts" normally takes about 5–11 s. On one CI run (37385398443) it hung:
javaprocess.Cause (from the code; the hung run's log is silent about which command stuck).
lib/exec.mjsrunExec(), which runs everywheelscall the harness makes:closeevent, which waits for every holder of its stdout/stderr pipes.So a
wheelscall that stalls, or that leaves a process holding its pipes, keeps the harness waiting forever. The firstwheelscall in that test isresetFixture()'swheels new.Fix, in
runExec():WHEELS_EXEC_TIMEOUT_MS(defaultExecTimeout()). It's below the tests' 300 s limit, so a stuck command fails with its own name rather than as an anonymous test timeout. On timeout it kills the command's whole process group (the child is spawned detached, so it leads one; a JVM it started goes too) and returns{ code: -1, timedOut: true }withtimed out after Ns running <command>in stderr.{test:cli}block may legitimately leave a server running (wheels start). Its group is kept in aleftoversset and stopped when the harness exits, since its isolated home is deleted then and a detached group doesn't get the terminal's Ctrl-C.wheelsprocesses behind. It pairs with the SIGINT/SIGTERM handlers inisolated-home.mjs.The
timeoutoption callers could already pass now goes through this path instead ofspawn()'s owntimeout, which signalled only the child.Tests
New in
test/exec.test.mjs, using smallnodeprograms in place ofwheels:leftoversfix);WHEELS_EXEC_TIMEOUT_MSsets the default, and an invalid value falls back to 240 s.RED on develop: all 4 fail. The run itself hung with the old
runExec()and had to be stopped, which is the bug.GREEN: all of
exec.test.mjs, 7 / 7. The fullpnpm test:docs-harnessgives 71 pass, 1 fail. The failure is "tutorial driver walks mini-tutorial end to end" (Reload requires this project's own server), which is #4426, fixed by #4444 (not yet on develop).🤖 Generated with Claude Code