From 98731bccce385238a68133da21fbc39015fd68f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 23:37:52 +0000 Subject: [PATCH] fix(ci): teach the completeness guard turbo's failing-task header, so a red shard's own package stops being the one it cannot grade Under grouped log order turbo folds each task into a collapsible `::group::` EXCEPT the one it surfaces as the run's failure, which gets a bare `:test` header and no `::endgroup::`. The guard read the `:test:` stream prefix and the group header but not that third shape, so the FAILING package -- the one it exists to grade -- was the one package whose summary it could not attribute. Its "refuse to guess" backstop then withheld judgement on every candidate and exited 0. Measured on run 32391924279 (Test Core (3/3), job 96499888763, complete 31839-line log): the guard printed `OK (24 of 29 ...)` directly under a note naming @objectstack/example-showcase as ungraded. That is the #10032 shape -- a green that reads like a clearance -- reproduced by the change that was supposed to close it. - parseSummaries understands the bare header, and turbo's end-of-run roster closes it (that block has no ::endgroup:: of its own). - classifyShard never lets the stray-summary backstop excuse a package turbo named in `Failed:`. Rule A cannot cost a green shard: it needs turbo to have named a failed task, which already failed the run. - An ungraded package downgrades the verdict word from OK to PARTIAL and raises a ::warning:: annotation, instead of being a note nobody reads. The original zero-output event remains unreproduced and undiagnosed; the 2026-08-20 recurrence was not one -- that suite streamed ~940 lines and a complete summary. Part of #10032 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt --- .github/workflows/ci.yml | 29 +++- scripts/check-test-completeness.mjs | 229 +++++++++++++++++++++++++--- 2 files changed, 235 insertions(+), 23 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7214d4bc23..51fdb10003 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -514,14 +514,39 @@ jobs: # (2/3) failed naming @objectstack/example-showcase#test while this guard # printed OK, because the failing package was the one that printed # nothing — its complete 5083-line job log mentions that package twice, - # both times in the error summary, with no vitest output anywhere. The - # dichotomy is true again only because green now covers both questions. + # both times in the error summary, with no vitest output anywhere. + # + # ⛔ AND THE REPLACEMENT WAS STILL BLIND, FOR A SECOND REASON — measured + # on run 32391924279 (`Test Core (3/3)`, job 96499888763, complete 31839- + # line log). Under grouped log order turbo folds each task into a + # collapsible `::group::` EXCEPT the one it surfaces as the run's failure, + # which gets a bare `:test` header instead. The guard read groups and + # prefixes but not that third shape, so the FAILING package — the one it + # exists to grade — was the one package whose summary it could not + # attribute, and its "refuse to guess" backstop then withheld judgement on + # every candidate and exited 0. It printed `OK (24 of 29 …)` under a note + # naming @objectstack/example-showcase as ungraded: the same green-that- + # reads-like-a-clearance, one turn of the screw further out. + # ⇒ The guard now knows all three header shapes, a package named in + # turbo's `Failed:` roster is judged even when the log holds strays it + # cannot place, and an ungraded package downgrades the verdict word from + # OK to PARTIAL and raises a ::warning:: annotation. Only with all three + # does green cover both questions — which is what makes the dichotomy + # above true. ⚠ A red HERE can never turn a green shard red: it needs + # turbo to have named a failed task, and that already failed the run. # # ⚠ The guard does NOT charge a silent package to this shard when turbo # simply never reached it — turbo stops scheduling on the first failure, # so an ordinary red suite leaves later packages unrun. Those are printed # as notes. See the script header for the two exemptions and the two # rules that decide red. + # + # ⚠ NOT diagnosed, by either pass: the original zero-output event itself. + # It has never been reproduced. The 2026-08-20 recurrence was NOT one — + # that failing suite streamed ~940 lines and a complete summary + # (`Test Files 21 passed (21)` / `Tests 342 passed (342)`), and failed on + # a vitest teardown error, not on silence. Read a "zero output" claim as + # a hypothesis until someone has grepped the WHOLE log for it. - name: Test completeness guard if: always() run: | diff --git a/scripts/check-test-completeness.mjs b/scripts/check-test-completeness.mjs index 77ffa18d8b..082f4523b1 100644 --- a/scripts/check-test-completeness.mjs +++ b/scripts/check-test-completeness.mjs @@ -145,8 +145,46 @@ export function stripAnsi(raw) { // `@objectstack/spec` reported 415 files and 11045 tests, and the guard said it // had reported nothing). The group header is turbo's own statement of whose // output follows, so it is read as the primary attribution, not a heuristic. +// +// ⛔ THIRD SHAPE, and it is the one that matters most here: THE FAILING TASK +// IS NOT WRAPPED IN A GROUP. Under grouped order turbo folds each task into a +// collapsible `::group::`, EXCEPT the task it surfaces as the run's failure -- +// that one gets a bare, red-coloured header and no `::endgroup::`, so the +// failure is readable in the GitHub UI without expanding anything: +// `\x1b[;31m@objectstack/example-showcase:test\x1b[;0m` +// ` Test Files 21 passed (21)` +// ` Tests 342 passed (342)` +// +// So the ONE package whose output the guard most needs to grade -- the one that +// failed -- was the one package it could not attribute. Measured twice, byte for +// byte: in a controlled 4-package turbo 2.10.10 sandbox, and in the real +// incident (run 32391924279, `Test Core (3/3)`, job 96499888763, complete +// 31839-line log, line 23907). In that log the pattern `^:test$` +// matches 24 lines: 23 are `##[group]` headers, and the 24th is this bare one. +// +// ⭐ WHAT IT COST, which is the whole reason this file exists. Anonymous +// summaries put every candidate into the "refuse to guess" backstop below, so +// Rule A -- the #10032 rule -- was disabled for the failing package on EVERY red +// shard. That run printed, and it is what a clearance looks like: +// +// check-test-completeness: note: 3 scheduled package(s) could not be matched +// to a summary ... @objectstack/runtime, @objectstack/cli, +// @objectstack/example-showcase. +// check-test-completeness: OK (24 of 29 scheduled package(s) reported ...) +// +// The note even names its own cause -- "the log carries a shape neither the +// `:test:` prefix nor turbo's `::group:::test` header covers, that is +// the bug to fix" -- and then exits 0. This is that fix. +// +// ⚠ One more measured turbo property, load-bearing for Rule A: `Failed:` names +// only the task turbo surfaced, NOT every task that failed. With two failing +// tasks in one run the roster listed one; the other failed inside an ordinary +// `::group::`. Bare header <=> named in `Failed:` held in every run observed. const GROUP_OPEN = /^(?:::group::|##\[group\])(.+?)\s*$/; const GROUP_CLOSE = /^(?:::endgroup::|##\[endgroup\])\s*$/; +// A line that is nothing but `:test`. Reached only after the two markers +// above have had their turn, so a `::group::` header can never land here. +const BARE_TASK_HEADER = /^(\S+):test$/; export function parseSummaries(text) { const rows = []; @@ -166,6 +204,20 @@ export function parseSummaries(text) { group = sep > 0 && label.slice(sep + 1) === 'test' ? label.slice(0, sep) : null; continue; } + // The failing task's bare header. `##[group]` / `::group::` spellings have + // already been consumed above, and the guard belts-and-braces that here so a + // future marker spelling cannot be read as a package name. + const bare = line.match(BARE_TASK_HEADER); + if (bare && !bare[1].includes('::') && !bare[1].startsWith('#')) { + group = bare[1]; + continue; + } + // turbo's end-of-run roster closes the failing task's block, which has no + // `::endgroup::` of its own. Nothing after it belongs to any package. + if (TASKS.test(line) || FAILED.test(line)) { + group = null; + continue; + } const m = line.match(SUMMARY); if (!m) continue; const [, pkg, kind, tallies, declared] = m; @@ -252,18 +304,39 @@ export function classifyShard({ scheduled, reported, anonymousReports = 0, faile // the one error worse than not attributing at all. One candidate is not a // guess (there is nothing else it could belong to); more than one is. const candidates = expected.filter((name) => !reported.has(name)); + + // ⛔ THE BACKSTOP MUST NEVER SWALLOW RULE A. A package turbo named in + // `Failed:` is judged even when stray summaries are present, because that + // pairing IS the #10032 signature and excusing it is the exact green that + // this guard exists to stop -- measured on run 32391924279, where the + // backstop turned the failing package into a note and exited 0. + // + // This cannot cost a green shard: Rule A only fires when turbo already + // reported a failed task, so the test step has already failed and the shard + // is already red. The worst case is an extra, wrongly-worded paragraph on a + // run that was going to be red anyway; the alternative is a clearance on the + // one run where the guard had something to say. The wording below stays + // honest about which of the two it is. + const failedCandidates = candidates.filter((name) => failed.has(name)); + const otherCandidates = candidates.filter((name) => !failed.has(name)); let unattributable = []; let judged = candidates; - if (anonymousReports > 0 && candidates.length === 1) { - judged = []; - } else if (anonymousReports > 0 && candidates.length > 1) { - unattributable = candidates; - judged = []; + if (anonymousReports > 0) { + judged = failedCandidates; + if (otherCandidates.length > 1) unattributable = otherCandidates; } for (const name of judged) { if (failed.has(name)) { - silent.push({ name, why: 'turbo reported this task FAILED, and the log holds no vitest summary for it' }); + silent.push({ + name, + why: + anonymousReports > 0 + ? `turbo reported this task FAILED, and no summary in this log could be attributed to it ` + + `(${anonymousReports} unattributed summary line(s) present -- this log carries a task-header ` + + `shape this guard does not understand, so it cannot tell a silent suite from a misread one)` + : 'turbo reported this task FAILED, and the log holds no vitest summary for it', + }); } else if (runCompleted === true) { silent.push({ name, why: 'the run completed with every task successful, and this package reported nothing' }); } else { @@ -499,6 +572,50 @@ function selfTest({ quiet = false } = {}) { 'grouped: an explicit prefix lost to the enclosing group', ); + // -- THE FAILING TASK'S BARE HEADER: turbo's third shape. -- + // The exact bytes from run 32391924279 / job 96499888763, line 23907 (and + // byte-identical in a 4-package turbo 2.10.10 sandbox), followed by the two + // summary lines that actually sat under it. + const bareFail = stripAnsi( + ['\x1B[;31m@objectstack/example-showcase:test\x1B[;0m', ' Test Files 21 passed (21)', ' Tests 342 passed (342)'].join('\n'), + ); + eq( + parseSummaries(bareFail).map((r) => [r.pkg, r.kind, r.counted, r.declared]), + [['@objectstack/example-showcase', 'Test Files', 21, 21], ['@objectstack/example-showcase', 'Tests', 342, 342]], + 'bare header: the failing task\'s summary was not attributed to it', + ); + // The next task's group must take over -- the bare block has no ::endgroup::. + eq( + parseSummaries(['@objectstack/example-showcase:test', ' Tests 342 passed (342)', '::group::@objectstack/runtime:test', ' Tests 10 passed (10)'].join('\n')).map((r) => r.pkg), + ['@objectstack/example-showcase', '@objectstack/runtime'], + 'bare header: attribution leaked into the next task\'s group', + ); + // ...and turbo's own end-of-run roster closes it, so nothing after the + // failing block can be charged to the failing package. + eq( + parseSummaries(['@objectstack/example-showcase:test', 'Failed: @objectstack/example-showcase#test', ' Tests 5 passed (5)'].join('\n')).map((r) => r.pkg), + ['(vitest)'], + 'bare header: attribution survived turbo\'s Failed: roster', + ); + eq( + parseSummaries([' Tasks: 1 successful, 4 total', ' Tests 5 passed (5)'].join('\n')).map((r) => r.pkg), + ['(vitest)'], + 'bare header: a summary after the Tasks roster was attributed', + ); + // A `:build` bare header attributes nothing, exactly like a build group. + eq( + parseSummaries('@objectstack/spec:build\n Tests 5 passed (5)').map((r) => r.pkg), + ['(vitest)'], + 'bare header: a bare build header was read as a test header', + ); + // Marker spellings must never be mistaken for a package name, whichever + // branch above happens to consume them. + eq(parseSummaries('::group::@objectstack/spec:test\n Tests 5 passed (5)').map((r) => r.pkg), ['@objectstack/spec'], 'bare header: ::group:: fell through to the bare branch'); + eq(parseSummaries('##[group]@objectstack/spec:test\n Tests 5 passed (5)').map((r) => r.pkg), ['@objectstack/spec'], 'bare header: ##[group] fell through to the bare branch'); + // Ordinary prose that merely ends in `:test` needs a space to be safe; a + // no-space line is turbo's header and nothing else. Both directions pinned. + eq(parseSummaries('see foo:test\n Tests 5 passed (5)').map((r) => r.pkg), ['(vitest)'], 'bare header: a phrase ending in :test was read as a header'); + // -- Anonymous summaries: attribute only when there is nothing to guess. -- const anon = (over) => classifyShard({ @@ -540,6 +657,56 @@ function selfTest({ quiet = false } = {}) { 'anonymous: the #10032 case stopped being red', ); + // ⭐ THE REGRESSION THIS FIX EXISTS FOR. Run 32391924279 in miniature: three + // candidates, one of them named in `Failed:`, and stray summaries present + // because the failing task's header was not understood. The backstop used to + // excuse ALL THREE and exit 0. Rule A must survive it, naming only the + // failed one, and the other two stay ungraded notes rather than reds. + const backstop = classifyShard({ + scheduled: ['showcase', 'runtime', 'cli'], + reported: new Set(), + anonymousReports: 2, + failed: new Set(['showcase']), + runCompleted: false, + describe: describe({ + showcase: { hasTestScript: true, testFileCount: 21 }, + runtime: { hasTestScript: true, testFileCount: 9 }, + cli: { hasTestScript: true, testFileCount: 9 }, + }), + }); + eq(backstop.silent.map((s) => s.name), ['showcase'], 'backstop: a FAILED package was excused by the stray-summary backstop'); + eq(backstop.unattributable, ['runtime', 'cli'], 'backstop: the ungraded non-failed packages were lost'); + eq(backstop.notReached, [], 'backstop: an ungraded package was also counted as never reached'); + if (!backstop.silent[0].why.includes('unattributed summary line(s) present')) { + throw new Error('backstop: the red overclaims silence when stray summaries are present'); + } + // ...and with no stray summaries the wording goes back to the plain claim. + eq( + classifyShard({ + scheduled: ['showcase'], + reported: new Set(), + anonymousReports: 0, + failed: new Set(['showcase']), + runCompleted: false, + describe: describe({ showcase: { hasTestScript: true, testFileCount: 21 } }), + }).silent.map((s) => s.why), + ['turbo reported this task FAILED, and the log holds no vitest summary for it'], + 'backstop: the plain Rule A wording changed', + ); + // A failed package that DID report is still an ordinary failure, strays or not. + eq( + classifyShard({ + scheduled: ['showcase', 'runtime'], + reported: new Set(['showcase']), + anonymousReports: 3, + failed: new Set(['showcase']), + runCompleted: false, + describe: describe({ showcase: { hasTestScript: true, testFileCount: 21 }, runtime: { hasTestScript: true, testFileCount: 9 } }), + }).silent, + [], + 'backstop: a reported failure was charged as silent', + ); + if (!quiet) console.log('check-test-completeness: self-test OK'); } @@ -627,22 +794,36 @@ function main() { console.log(`check-test-completeness: note: ${n} was scheduled but never reached -- the run stopped before it.`); } if (shard.unattributable.length > 0) { - console.log( - `check-test-completeness: note: ${shard.unattributable.length} scheduled package(s) could not be ` + - `matched to a summary, and the log holds unattributed summaries, so completeness is NOT graded for ` + - `them: ${shard.unattributable.join(', ')}. Attributing a stray summary by guesswork would be worse ` + - 'than this gap. If you are seeing this, the log carries a shape neither the `:test:` prefix nor ' + - 'turbo\'s `::group:::test` header covers -- that is the bug to fix.', - ); + const note = + `${shard.unattributable.length} scheduled package(s) could not be ` + + `matched to a summary, and the log holds unattributed summaries, so completeness is NOT graded for ` + + `them: ${shard.unattributable.join(', ')}. Attributing a stray summary by guesswork would be worse ` + + 'than this gap. None of them was named in turbo\'s `Failed:` roster -- a failed package is judged ' + + 'even here, never excused. If you are seeing this, the log carries a shape none of the three this ' + + 'guard knows covers (`:test:` prefix, `::group:::test` header, bare `:test` header) ' + + '-- that is the bug to fix.'; + console.log(`check-test-completeness: note: ${note}`); + // A note in a 30k-line log is not seen; that is how the last one sat + // unread under a green. An annotation is. + if (process.env.GITHUB_ACTIONS === 'true') { + console.log(`::warning::check-test-completeness: ${note}`); + } } } if (holes.length === 0 && silent.length === 0) { if (shard) { - const expected = shard.scheduledCount - shard.exempt.length - shard.notReached.length; + const ungraded = shard.unattributable.length; + const expected = shard.scheduledCount - shard.exempt.length - shard.notReached.length - ungraded; + // ⛔ NOT "OK" when something went ungraded. The word is the payload: the + // whole #10032 family is a green sentence read as a clearance, and + // "OK (24 of 29 ... all accounted for)" printed directly under a note + // saying three packages were not graded is exactly that sentence. console.log( - `check-test-completeness: OK (${expected} of ${shard.scheduledCount} scheduled package(s) ` + - `reported, ${shard.exempt.length} had nothing to run, ${shard.notReached.length} never reached; ` + + `check-test-completeness: ${ungraded > 0 ? 'PARTIAL' : 'OK'} (${expected} of ${shard.scheduledCount} ` + + `scheduled package(s) reported, ${shard.exempt.length} had nothing to run, ` + + `${shard.notReached.length} never reached` + + `${ungraded > 0 ? `, ${ungraded} NOT GRADED` : ''}; ` + `${rows.filter((r) => r.kind === 'Tests').reduce((s, r) => s + r.declared, 0)} test(s) declared and all accounted for).`, ); process.exit(0); @@ -666,9 +847,9 @@ function main() { } if (silent.length > 0) { - const were = silent.length === 1 ? 'package was' : 'packages were'; + const has = silent.length === 1 ? 'package has' : 'packages have'; console.error( - `check-test-completeness: ${silent.length} ${were} scheduled on this shard and reported no vitest summary\n`, + `check-test-completeness: ${silent.length} scheduled ${has} no vitest summary this log can account for\n`, ); for (const s of silent) { console.error(` • ${s.name} -- ${s.why}`); @@ -687,9 +868,15 @@ level before writing a summary, or its captured output was lost. Precedent: #10032, where \`Test Core (2/3)\` failed naming @objectstack/example-showcase#test and the complete 5083-line job log contained no other mention of that package -- no summary, no FAIL, no test name -- while -this guard printed OK. The mechanism was never reproduced and is still unknown; -what this red buys is that the next occurrence names itself instead of sending -triage to a wrong hypothesis.`); +this guard printed OK. That original zero-output event was never reproduced and +is still undiagnosed; what this red buys is that the next occurrence names +itself instead of sending triage to a wrong hypothesis. + +⚠ If the reason above cites UNATTRIBUTED summary lines, read this red as "the +guard could not grade it", not as "the suite printed nothing" -- the summary may +be sitting in the log under a task header this guard does not parse. Check the +lines around the package's own header before concluding anything about the +suite, and fix the header shape here.`); } if (holes.length > 0) {