fix(release): block the transport-timeout pass-through on any exception header

.github/workflows/release.yml

@@ -537,6 +537,14 @@ jobs:
       - *unpack_release_build
 
       - name: 'Run Workspace Tests'
+        # GitHub's default for `run:` is `bash -e {0}` — no `pipefail` — so
+        # `npm … | tee` yields tee's status, the `||` handler never fires and
+        # every branch below is dead code: a shard with a genuinely failing
+        # test exits 0 and the release proceeds. Measured on release run
+        # 33806806226, job 100824085040, which reports
+        # `shell: /usr/bin/bash -e {0}`. Naming the shell is what makes `$?`
+        # npm's.
+        shell: 'bash'
         env:
           # Match the tunable per-process bound used by the main CI gate.
           VITEST_MAX_THREADS: "${{ startsWith(runner.name, 'ecs-qwen-') && (vars.QWEN_CI_VITEST_MAX_WORKERS || '4') || '' }}"
@@ -558,6 +566,14 @@ jobs:
           # release lane alone, and an empty variable just falls back to the
           # default.
           VITEST_RETRY: "${{ vars.QWEN_RELEASE_VITEST_RETRY || '2' }}"
+          # The guard below reads this log line by line. Vitest colours its
+          # output from the mere presence of CI, and a coloured summary sits
+          # escapes between a label and its value
+          # (`ESC[2m      Tests ESC[22m ESC[1mESC[32m394 passed`), which no
+          # anchored pattern can match — on real release bytes three of the
+          # four conditions return 0 and the guard never reaches the fourth.
+          # ci.yml already sets this on three steps for the same reason.
+          NO_COLOR: 'true'
         run: |-
           retry_arg=()
           if [ -n "${VITEST_RETRY}" ] && [ "${VITEST_RETRY}" != 'off' ]; then
@@ -567,29 +583,45 @@ jobs:
           # killed by Vitest's own worker RPC timing out reads identically to
           # a real break, and this release lost two attempts to that
           # (run 33713579913). The status is re-raised untouched either way;
-          # `-o pipefail` is the step default, so `$?` is npm's.
+          # `shell: bash` above supplies `-o pipefail`, so `$?` is npm's.
           log="${RUNNER_TEMP:-/tmp}/workspace-tests-${{ matrix.shard }}.log"
           npm run test:release:workspaces -- --shard=${{ matrix.shard }}/3 --passWithNoTests "${retry_arg[@]}" 2>&1 | tee "${log}" || {
             status=$?
             if grep -qE '^[[:space:]]*FAIL ' "${log}"; then
               : # A failing test names itself; an annotation adds nothing.
-            elif grep -q 'Timeout calling' "${log}"; then
-              # Passed only with proof the run reached its end and nothing
-              # else broke: a normal exit (a signal death is 128+N), a
-              # passing tally, no failing tally, and no unhandled error that
-              # is not the transport. Vitest's worker RPC giving up says
-              # nothing about the product, and --retry cannot cover it —
-              # retries re-run failing TESTS while an unhandled error fails
-              # the run outright. It has now cost this release three
-              # attempts (run 33713579913).
+            elif grep -q '\[vitest-worker\]: Timeout calling' "${log}"; then
+              # Vitest's worker RPC giving up says nothing about the product,
+              # and --retry cannot cover it — retries re-run failing TESTS
+              # while an unhandled error fails the run outright. It has now
+              # cost this release three attempts (run 33713579913).
+              #
+              # Passed only with proof the run reached its end and that every
+              # unhandled error WAS the transport. That proof is Vitest's own
+              # count rather than a reading of the crash: it prints
+              # `Errors  N errors` whenever unhandled errors occurred, so the
+              # guard compares that count with how many carried the
+              # transport's own `[vitest-worker]: Timeout calling` message.
+              # Recognising a crash by its header cannot be made to work — the
+              # header is producer-chosen (26 of this repo's 293 Error
+              # subclasses have no Error/Exception suffix, and four assign
+              # that bare name to err.name), so any pattern over headers is
+              # incomplete by construction, while the count needs no pattern.
+              #
+              # `--workspaces` prints one summary per workspace into one log,
+              # so both figures are whole-file sums and a passing tally cannot
+              # cover a later workspace's crash. Reaching this branch means at
+              # least one transport line, so a log with no `Errors` summary at
+              # all counts 0 against it and the pass is refused, not granted.
+              errors=$(awk '/^[[:space:]]*Errors[[:space:]]+[0-9]+ errors?$/ { total += $2 } END { print total + 0 }' "${log}")
+              timeouts=$(grep -cE '\[vitest-worker\]: Timeout calling' "${log}" || true)
               if [ "${status}" -lt 128 ] \
                 && grep -qE '^[[:space:]]*Tests[[:space:]]+[0-9]+ passed' "${log}" \
                 && ! grep -qE '^[[:space:]]*(Tests|Test Files)[[:space:]]+[0-9]+ failed' "${log}" \
-                && ! grep -E '^[[:space:]]*Error:' "${log}" | grep -qv 'Timeout calling'; then
-                echo "::warning title=Workspace tests passed through a Vitest transport timeout::Every test passed and no other error was reported; Vitest's own worker RPC timed out. Treated as a pass."
+                && [ "${errors}" -eq "${timeouts}" ]; then
+                echo "::warning title=Workspace tests passed through a Vitest transport timeout::Every test passed and all ${errors} unhandled error(s) Vitest counted were its own worker RPC timing out. Treated as a pass."
                 exit 0
               fi
-              echo "::warning title=Workspace tests exited ${status} on a Vitest transport timeout::A transport timeout with no passing tally to back it, so the failure stands. Rerun the job."
+              echo "::warning title=Workspace tests exited ${status} on a Vitest transport timeout::A transport timeout the run cannot account for — no passing tally, a failing tally, a signal death, an unhandled error that was not the transport, or the two counts disagreeing for a reason this log does not show. The failure stands (status ${status}, ${errors} counted error(s) vs ${timeouts} transport line(s)); rerun the job."
             else
               echo "::error title=Workspace tests exited ${status} with no failing test::No FAIL line and no transport timeout in the log. Look for a Vitest Unhandled Errors section, or a worker killed before it could report."
             fi

scripts/tests/release-workflow.test.js

@@ -548,6 +548,20 @@ describe('release workflow', () => {
     expect(testStep.env.VITEST_RETRY).toBe(
       "${{ vars.QWEN_RELEASE_VITEST_RETRY || '2' }}",
     );
+    // `shell: 'bash'` is this step's only source of `-o pipefail` — the
+    // workflow has no `defaults:` block — so it is what makes the guard live
+    // rather than dead code. Drop it and GitHub falls back to `bash -e {0}`,
+    // where `npm … | tee` reports tee's status 0, the `||` handler never
+    // fires, and a shard with genuinely failing tests exits 0 into the
+    // release. The probe harness below passes `-o pipefail` itself, so no
+    // behavioural row would notice the line being cleaned up as redundant.
+    expect(testStep.shell).toBe('bash');
+    // Vitest colours its summaries from the mere presence of CI, and a
+    // coloured summary sits escape bytes between a label and its value, so
+    // every anchored pattern in the guard matches nothing: the pass-through
+    // is never granted again and each transport timeout reddens the release.
+    // Quoted in YAML, so it parses to the string rather than a boolean.
+    expect(testStep.env.NO_COLOR).toBe('true');
 
     const workspacePackages = getTestCiWorkspaces();
 
@@ -643,44 +657,193 @@ describe('release workflow', () => {
     // A shard that died on Vitest's own worker RPC timing out reads
     // identically to a real break, and this release lost two attempts before
     // anyone could tell them apart (run 33713579913). The annotation says
-    // which; the child's status is re-raised untouched either way, so no
-    // reading of the log can turn a failure green.
+    // which; the child's status is re-raised untouched, so no reading of the
+    // log can turn a failure green except the one deliberate pass-through.
+    //
+    // That pass-through is granted by Vitest's own count of unhandled errors
+    // (`Errors  N errors`) matching how many carried the transport's message
+    // — not by recognising a crash from its header. The header is
+    // producer-chosen, so a pattern over headers is incomplete by
+    // construction; the rows below carry the shapes that defeat one
+    // (suffix-less class, no class at all) and the shapes that defeat the
+    // count (an extra error the timeouts do not account for, a summary the
+    // run never reached, words a test merely printed).
     const testStep = releaseYaml.jobs.workspace_tests.steps.find(
       (step) => step.name === 'Run Workspace Tests',
     );
     const script = testStep.run.replaceAll('${{ matrix.shard }}', '1');
+    const timeout = 'Error: [vitest-worker]: Timeout calling "x"';
+    const tally = ' Tests  10614 passed (10614)';
+    const passedThrough =
+      '::warning title=Workspace tests passed through a Vitest transport timeout::';
+    const stands =
+      '::warning title=Workspace tests exited 1 on a Vitest transport timeout::';
 
     for (const [label, stub, code, annotation, expected] of [
       // A failing test names itself; an annotation would only add noise.
       ['failing test', ' FAIL  src/a.test.ts > boom', 1, null],
       // Vitest's worker RPC giving up says nothing about the product, and
-      // --retry cannot cover it. Passed only with proof the run reached its
-      // end: a normal exit, a passing tally, no failing tally, and no other
-      // unhandled error. It cost this release three attempts before that.
+      // --retry cannot cover it — retries re-run failing TESTS while an
+      // unhandled error fails the run outright. Passed only with proof the
+      // run reached its end and that the transport accounts for every
+      // unhandled error Vitest counted. It cost this release three attempts.
       [
         'transport timeout, run completed',
-        'Error: [vitest-worker]: Timeout calling "x"\n Tests  10614 passed (10614)',
+        `${timeout}\n${tally}\n     Errors  1 error`,
         1,
-        '::warning title=Workspace tests passed through a Vitest transport timeout::',
+        passedThrough,
         0,
       ],
       [
         'transport timeout, killed by a signal',
-        'Error: [vitest-worker]: Timeout calling "x"\n Tests  10614 passed (10614)',
+        `${timeout}\n${tally}\n     Errors  1 error`,
         137,
         '::warning title=Workspace tests exited 137 on a Vitest transport timeout::',
       ],
+      // One more error than the transport accounts for: the run broke on
+      // something else as well, whatever its header said.
       [
         'transport timeout beside a real one',
-        'Error: [vitest-worker]: Timeout calling "x"\nError: write after end\n Tests  10614 passed (10614)',
+        `${timeout}\nError: write after end\n${tally}\n     Errors  2 errors`,
+        1,
+        stands,
+      ],
+      // The counts can also disagree with no crash in the log at all — the
+      // same transport message on two lines inflates `timeouts` past what
+      // Vitest counted. That is a fifth way to reach this refusal, so the
+      // annotation names it and prints both figures it compared; without
+      // them the oncall is told one of four things happened when none did.
+      [
+        'transport timeout counted twice against one unhandled error',
+        `${timeout}\n${timeout}\n${tally}\n     Errors  1 error`,
+        1,
+        '::warning title=Workspace tests exited 1 on a Vitest transport timeout::A transport timeout the run cannot account for — no passing tally, a failing tally, a signal death, an unhandled error that was not the transport, or the two counts disagreeing for a reason this log does not show. The failure stands (status 1, 1 counted error(s) vs 2 transport line(s)); rerun the job.',
+      ],
+      // The two shapes no header pattern reaches. A class whose name carries
+      // no Error/Exception suffix is not hypothetical — 26 of this repo's 293
+      // Error subclasses are named that way, four of them assigning the bare
+      // name to err.name — and a bare string throw prints under Vitest's own
+      // `Unknown Error:` heading, which a header matcher misses on the space.
+      // The count sees both, because it never looks at the header.
+      [
+        'transport timeout beside a suffix-less crash header',
+        `${timeout}\nPoolTimeout: worker pool exhausted\n${tally}\n     Errors  2 errors`,
+        1,
+        stands,
+      ],
+      [
+        'transport timeout beside a bare string throw',
+        `${timeout}\nUnknown Error: a bare string, no class header\n${tally}\n     Errors  2 errors`,
+        1,
+        stands,
+      ],
+      // Ordinary `Error:` lines are test output, not evidence of a break: the
+      // log of the run this guard was written for carries three of them as
+      // fixture data. A matcher over headers refuses the pass-through on
+      // those and reddens a release the guard exists to save; the count is
+      // unmoved by them.
+      [
+        'transport timeout beside Error: lines a test printed',
+        `${timeout}\nError: boom\nError: Not implemented: navigation\n${tally}\n     Errors  1 error`,
+        1,
+        passedThrough,
+        0,
+      ],
+      // `--workspaces` prints one summary per workspace into one log, so both
+      // figures are whole-file sums: a passing tally cannot cover a later
+      // workspace's crash, and two transport deaths in two workspaces still
+      // pass.
+      [
+        'tally, then a later workspace crashing',
+        `${timeout}\n${tally}\n Tests  8 passed (8)\n     Errors  2 errors`,
+        1,
+        stands,
+      ],
+      // The shape of the log this guard was written for: several transport
+      // deaths in one run, and the plural summary Vitest prints for more than
+      // one of them (run 33713579913).
+      [
+        'four transport deaths, four unhandled errors',
+        `${timeout}\n${timeout}\n${timeout}\n${timeout}\n${tally}\n     Errors  4 errors`,
+        1,
+        passedThrough,
+        0,
+      ],
+      [
+        'two workspaces, both lost to the transport',
+        `${timeout}\n Tests  5 passed (5)\n     Errors  1 error\nError: [vitest-worker]: Timeout calling "y"\n Tests  7 passed (7)\n     Errors  1 error`,
         1,
-        '::warning title=Workspace tests exited 1 on a Vitest transport timeout::',
+        passedThrough,
+        0,
+      ],
+      // Absent evidence refuses the pass rather than granting it: no summary
+      // line at all, no passing tally, or a failing tally.
+      [
+        'transport timeout, no error summary to count',
+        `${timeout}\n${tally}`,
+        1,
+        stands,
       ],
       [
         'transport timeout, no tally to back it',
         'Error: [vitest-worker]: Timeout calling "onTaskUpdate"',
         1,
-        '::warning title=Workspace tests exited 1 on a Vitest transport timeout::',
+        stands,
+      ],
+      // Vitest's own summary is the only thing that grants the pass, so a
+      // run that never printed a tally does not get one even when its error
+      // count is all transport.
+      [
+        'error summary with no tally to back it',
+        `${timeout}\n     Errors  1 error`,
+        1,
+        stands,
+      ],
+      // `Timeout calling` in a test's own output is not the transport dying:
+      // the branch is entered on Vitest's own `[vitest-worker]:` message, so
+      // a log carrying only the words is unexplained, not passed through.
+      [
+        'Timeout calling printed by a test',
+        `Timeout calling the vendor API\n${tally}`,
+        1,
+        '::error title=Workspace tests exited 1 with no failing test::',
+      ],
+      // ...and the count is anchored on the same message, so those words
+      // beside a real transport death do not inflate it into a mismatch.
+      [
+        'transport timeout, and Timeout calling printed by a test',
+        `${timeout}\nTimeout calling the vendor API\n${tally}\n     Errors  1 error`,
+        1,
+        passedThrough,
+        0,
+      ],
+      // ...and the summary sum is anchored on the section-line shape for the
+      // same reason. Vitest echoes a test's console output at column 0, so a
+      // workspace printing summary-shaped fixture data lands there; an
+      // unanchored `Errors  N errors` match would add it to the sum, inflate
+      // `errors` past `timeouts`, and refuse a pass-through every test
+      // earned — the false-red this PR exists to remove, back again.
+      [
+        'transport timeout, and a summary-shaped line a test printed',
+        `${timeout}\nErrors 2 errors occurred in fixture data\n${tally}\n     Errors  1 error`,
+        1,
+        passedThrough,
+        0,
+      ],
+      [
+        'transport timeout, failing tally',
+        `${timeout}\n Tests  3 failed | 10611 passed (10614)\n     Errors  1 error`,
+        1,
+        stands,
+      ],
+      // One workspace can print a passing tally while a later one fails
+      // without ever emitting a FAIL line, so the failing tally is checked
+      // across the whole log rather than trusted to the branch above.
+      [
+        'passing tally in one workspace, failing tally in another',
+        `${timeout}\n Tests  10 passed (10)\n Test Files  1 failed (3)\n     Errors  1 error`,
+        1,
+        stands,
       ],
       [
         'unexplained',