Skip to content

CI worker-crash tolerance in test.yml can never fire, so every vitest worker crash is a hard red #27

Description

@pseudoseed

A vitest worker that dies in teardown turns a green suite into a hard CI failure, even though test.yml contains a guard written specifically to tolerate that case. The guard has never been reachable.

The guard

grep -q "Test Files.*passed" && ! grep -q "failed"

The second condition is a grep for the bare word failed across the WHOLE log. It matches things that have nothing to do with a failing test. In one real run it matched six times:

  • once because the runner echoes the guard script itself into the log, so the word failed appears simply because the guard mentions it
  • three times inside spec-1470 test NAMES: clear-failed, reentry-failed, reports a failed Tower send
  • twice in git fetch ... failed warnings emitted by consult tests

So ! grep -q "failed" is effectively always false, and the tolerance branch is dead code.

What it cost

Found 2026-08-21 during PIR #13. The branch was red at HEAD while a local run was green. Diagnosis, using the CI extraction tool that PR was adding:

Test Files 280 passed | 3 skipped (284)

No test failed. A vitest worker forked in teardown and died, one file went unreported, and the run was marked failed. The reviewer flagged the discrepancy, the builder spent a pass diagnosing it, and it was not its defect at all.

Fix

Scope the check so it cannot match its own script or ordinary test names:

  • match vitest summary lines specifically, e.g. Tests .* failed or Test Files .* failed, not the bare word
  • or parse the summary line and compare counts, rather than grepping
  • and make sure the guard script is not echoed into the log it is grepping (set +x around it, or read the summary from a file)

Why it matters beyond one red build

A worker crash and a genuine test failure currently look identical in CI. That teaches people to re-run rather than read, and re-running is how a real failure gets waved through as flaky. The guard exists because someone already understood that. It just never worked.

Same family as #8: a fixed limit or a crude check producing a result that misdescribes what happened.

Found and deliberately NOT fixed by the #13 builder, correctly, since it was outside that diff.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/releaseBuild, packaging, install, releasebugSomething isn't workingfork-onlyDeliberately not upstreamed to cluesmith/codev

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions