Skip to content

Flaky CI at the cause: task exit verdict, race-free private-root removal, stalled-file teardown - #422

Merged
raiseCatError merged 8 commits into
devfrom
fix/v021-flaky-tests
Oct 18, 2026
Merged

raiseCatError merged 8 commits into
devfrom
fix/v021-flaky-tests

Conversation

@raiseCatError

Copy link
Copy Markdown
Owner

Stacked on #421. Fixes the three CI flakes seen during the v0.21 stack (plus one seen in a local release run) at their cause: no longer sleeps, reruns or looser assertions. Two of them were product defects.

Flake Root cause Fix Evidence
backgroundSession.test.ts:92 (macOS, #417) Product. When a task's leader exited, the supervisor set leftovers = group.alive, but that is the guard's own liveness. The guard is a group member and confirms the group is empty with ps before it exits. Every cleanly exiting task showed "exited, processes left" (red) for one probe, and Delete was refused. The guard marks left when members remain. A cleanly exited leader finishes on the guard's verdict: guard exit means nothing is left, the marker means something is. A 3 s bound applies if the guard can't tell. New supervisor test fails on the old code every run. The original test failed 3/60 under 10× parallel load, 0/60 with the fix.
configuredCompletion.test.ts:89 (Ubuntu, #417) Product. At a startup timeout, NMSh's reset and the helper's own rm -rf remove the private root while a helper shell may still write its pid file. rmSync threw ENOTEMPTY (macOS, 39/300) or, on Linux/Node 26, returned with the directory still present (24/300). The throw also escaped a timer. removeDirectoryNow: rename into a fresh sibling first (atomic), so no path is left to write into, then remove the copy in bounded passes. It never throws. New race test (120 rounds of writer + remover) fails with plain rmSync on macOS and Linux and passes with the fix on both.
Ubuntu shard hung 6 min until the job limit (#415) Test infrastructure. The reporter flushes a file when it completes. Output stopped after shellAdapters, so shellContextEnvironment never completed, and a fish was still alive at cancellation. A test that passes its timeout is cancelled, but nothing ends what it started: there, start() and its prompt waits sit outside the try/finally that kills the shell. The live PTY keeps the file's process alive and its failure is never printed. 121 files start live sessions this way. The runner preloads scripts/test-file-teardown.mjs into each test file. Once the file's tests finish, an unref'd timer gives the file's own cleanup 15 s; if the process is still alive, every process it left running is ended (its own groups, never the runner's) and named on stderr. Runner test: a stalled file with a piped child now fails in ~1 s with nothing left; without the preload it waits for the child.
nativeCaptureLifecycle "reaps a PTY before …" (local verify:release) Test. Its 3 s bound also covered the owner's cold node --import=tsx compile under full-suite load. The owner marks when it has loaded; the 3 s bound starts there. Not reproduced in isolation (40/40 before and after); the full release run is the check.

Linux evidence came from a Debian container (Node 26.12, zsh 5.9, Fish 3.6) whose disk is on the T7. The underlying Fish stall behind the #415 hang (a prompt not arriving in time once on CI) was not reproduced: 20 Linux runs of shellAdapters and the full context-environment file passed. If it recurs, it now shows as a named test failure with the process listed.

Refs #410

… never "processes left" by guess

backgroundSession.test.ts:92 (macOS CI on #417) refused to remove a task that had
just exited: "a running task, or one whose processes still run, is never removed".

Root cause: when the command's leader exited, the supervisor finished the task with
`leftovers = group.alive`, but `alive` is the guard's own liveness (the guard is a
member of the group and confirms it is empty with `ps` before it exits). Every task
that exited cleanly was therefore reported, for one guard probe (tens of ms, longer
under load), as "exited, processes left", with Delete refused; users saw the red
"processes of this task still run" row flicker. The test removed the task inside
that window on a loaded runner (reproduced 3 of 60 under 10x parallel load).

The guard now says once when members remain (a `left` marker beside its `exit`
file), and the supervisor finishes a cleanly exited leader only on the guard's
verdict: the guard exits (nothing left) or marks what is left. If the guard cannot
tell within 3 s (its ps keeps failing), something is reported as left, as before.
A stop requested meanwhile keeps the stopping semantics.

New test: a task that leaves nothing behind is never reported with leftovers, even
for a moment, and is removable at once (fails on the old supervisor every run).

Refs #410
…imeout returns

configuredCompletion.test.ts:89 (Ubuntu CI on #417) found the helper's private root
still present right after a startup timeout. At that moment two parties remove it
(NMSh's reset and the helper's own EXIT trap `rm -rf`) while a helper shell may
still write its pid file into it. rmSync then threw ENOTEMPTY (macOS, 39 of 300
rounds in a reproduction) or, on Linux with Node 26, returned normally with the
directory still there (24 of 300), and a throw from reset escaped a timer.

removeDirectoryNow renames the directory into a fresh sibling first (atomic, same
filesystem), so no path to it remains for a late write or a concurrent remover, then
removes the renamed copy in a few bounded passes (GNU rm works through directory
descriptors and can follow the rename) and never throws. The new test races a
writer-and-remover against it 120 times; plain rmSync fails it on both platforms.

Refs #410
…instead of hanging the run

#415's Ubuntu shard sat silent for six minutes until the job limit, with a node
test worker and a `fish` still running. The reporter flushes a file's results when
the file completes; output stopped after shellAdapters, so the next file,
shellContextEnvironment, never completed. A test that passes its timeout is
cancelled, but nothing ends what it started: a live shell PTY (there, `start()` and
its prompt waits sit outside the try/finally that kills the shell) keeps the file's
process alive, its failure is never printed, and the run waits for the CI limit.
121 test files start live sessions this way, so the fix is in the runner.

Each test file now preloads test-file-teardown.mjs: once the file's tests are done,
an unref'd timer gives the file's own cleanup 15 s; if the process is still alive
then, every process it left running (its own process groups, never the runner's) is
ended and named on stderr, so the stall surfaces as the test failure it is. The new
runner test stalls a file with a piped child: it now ends in about a second with a
failure and no child left; without the preload it waits for the child.

Refs #410
…d start

"native cancellation reaps a PTY before it can process the initialization command"
failed once in a local verify:release (passes alone, and 40 of 40 under 10x
parallel isolated runs): its 3 s wait for the inner shell also covered the owner
process's cold `node --import=tsx` start compiling CompletionService's imports,
which a full parallel suite can stretch past 3 s. The owner now marks when it is
loaded; that gets a generous bound, and the 3 s bound starts there. Not reproduced
in isolation; the full release run is the check.

Refs #410
…her remover still works in it

CI on #422 (Ubuntu) left a renamed copy behind after the five rmSync passes: GNU
`rm -rf` works through directory descriptors, so the helper's own cleanup keeps
deleting inside the renamed directory, and Node's rmSync stops early (no error)
each time an entry vanishes under it. The path itself was gone every round.

The copy is now removed pass by pass: list it, remove each entry tolerating ones
that vanished, then remove the directory, retrying while it is not empty. Nothing
can add entries to it (its path is gone), so every pass shrinks it; a deferred
removal remains only as a last resort. Passes 3/3 on macOS and 5/5 on Linux.

Refs #410
…he moment the turn settles

Failed once on dev (dispatched CI, Ubuntu): `client.length` 5, expected 6. The fake
server records a client message when it reads it. When its request 101 reached the
client after the person had answered 100, the client's refusal of 101 was written
after that answer; the server completed the turn first and the test read the log at
`settled`, one message short. Not a product defect: both responses are sent.

The fixture now sends 101 a moment later, so this order happens every run (the
old test then fails 3 of 3), and the helper waits (bounded) until the messages the
test asserts are on record. 5 of 5 runs pass.

Refs #410
@raiseCatError

Copy link
Copy Markdown
Owner Author

Added c350057: a fourth flake, seen once on dev itself in a dispatched CI run. codexAdapter.test.ts:84 read the fake Codex server's log the moment the turn settled. When request 101 reached the client after the person's answer to 100, the refusal of 101 arrived at the server after the turn completed, so the log was one message short. This is a test race, not a product defect. The fixture now forces that order every run (the old test then fails 3/3), and the helper waits, bounded, for the messages the test asserts (5/5 pass). Also 8f57e6a: the renamed copy in removeDirectoryNow is now removed entry by entry, because GNU rm -rf follows a renamed directory through its descriptors (Ubuntu CI on this PR). Merged forward into #423.

… capture windows begin

Two more tests in this file failed in a local verify:release run with the load
average at 35-67 (the machine shared with other workloads): their 3 s windows began
at spawn, and the owner process had not finished its cold `node --import=tsx`
start ("loadedAfterMs": null). Like 25759ce, they now wait (bounded, 30 s) for the
owner to load, and their tight windows start there. 9/9 pass, three runs.

Refs #410
From the last local verify:release (3,138 of 3,142 passing, 2 skipped): two tests
in code v0.21 does not change failed under load and pass alone (5/5).

- directoryNavigation: the fake zoxide was a Node script whose cold start can exceed
  the zoxide query's 1.5 s limit, so the service (rightly) fell back to Native. It is
  now a shell script, quick to start like the real native binary.
- configuredCompletion 'bounded to 4096 records': it tests the record cap, not
  startup; its helper now gets the full startup cap (5 s) so a loaded machine's
  compinit does not turn into an empty result.

Refs #410
@raiseCatError
raiseCatError changed the base branch from fix/v021-stabilize to dev October 18, 2026 14:15
@raiseCatError
raiseCatError merged commit ac0f780 into dev Oct 18, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant