fix(cli): make a vitest filter that selects no test file say so - #17965
Conversation
`vitest run` drops a positional filter that matches nothing without any diagnostic, as long as some other filter matched: five paths in, four counted, the discarded name printed nowhere. The output is byte-identical to the run that named only the four, so it is indistinguishable from "ran and passed". Wire a reporter that reads vitest's own resolved filters and collected specifications and names every path that selected nothing, with the tier it lives in and a command that runs it. It writes zero bytes when nothing was lost. Rewrite the tier comment block, which printed its "nothing is skipped" reassurance three lines above the two commands that can lose things. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
Naming `test.reporters` replaces vitest's own reporter defaulting instead of extending it: it pins `default` where an agent terminal gets `agent` (measured: a healthy control run gained two lines, so direction ② failed) and it would drop the `github-actions` reporter on every CI run, removing the failure annotations in the one environment a local control cannot observe. Read the command line through vitest's own exported `parseCLI` instead, at config load, over the same two derived tier arrays the projects take as their `include`. Every case the preflight cannot model — an argv the parser refuses, a `--project` that is not a tier name, a `--changed` run — declines silently rather than guessing. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
vitest's `CliOptions` is an interface, so it is not assignable to `Record<string, unknown>`; the cast that would hide that is also what would stop a vitest upgrade from surfacing a renamed option as a type error. Declare the three fields this module reads instead, and hand the real `parseCLI` to the pin unaltered so the compatibility is pinned. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
…r load vitest loads this config once per project and each load is its own module instance, so the module-level flag guarded nothing: measured, three loads printed the notice three times above the banner and three more at exit. Move the guard onto a scope object defaulting to globalThis, and pin it by calling twice on one scope. A repeat call still returns the notice to its caller; returning the empty string there would read as "nothing lost". Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
📓 Docs Drift CheckThis PR changes 1 package(s): 4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 24 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin acebaa4fd578fb99687941c41a2578bbbe62f23b && git checkout acebaa4fd578fb99687941c41a2578bbbe62f23b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 146c291943400664b8d0ef6dc39aba9c98746550 cc49814d84f0bfa21851f1731d3207cbb5614983 && git checkout -B drift-repro 146c291943400664b8d0ef6dc39aba9c98746550 && git merge --no-ff cc49814d84f0bfa21851f1731d3207cbb5614983
node scripts/docs-audit/affected-docs.mjs --json 146c291943400664b8d0ef6dc39aba9c98746550
|
Contract reviewPR #17965 · card #17853 · head
① ⭐ The card's headline premise is FALSIFIED, and the dev handled it exactly as instructedAcceptance item 1 said: run triage's pair, and if both legs give the same result, STOP and correct the card. They gave different results, so the stop condition did not trip and the work proceeded — but the card's framing is wrong: ⇒ Naming ONE integration-tier file under ⭐ The real defect is one step over, and it is exactly what cost #16872 a round — the PARTIAL filter. Once at least one named path matches, the paths that matched nothing are dropped in total silence: three paths under ② The fix, and the comment block that made the harm possibleA config-load preflight ( ⇒ That block was the whole trap: it reassured the reader that ③ ⛔ This seat's dispatch order was WRONG about the changeset, and the dev was rightThe order said “Changeset required: ⇒ The build compiles ⭐ Ruled: ④ ⭐ The delivery falsified its own first design — twiceThis is the part that makes the rest trustworthy:
Direction 2 now measures byte-identical: raw bytes 235 before / 235 after, sha256 of the timing-normalised capture identical ( ⑤ Fences and declared gaps⛔ ⭐ The docs blind spot this seat's rider flagged over ⭐ #17978 filed for the same silent partial-filter drop in the seven other packages that declare vitest Verdict and what remainsPASS.
⛔ This seat's own rider ( ⛔ This seat cannot re-run it:
Generated by Claude Code |
|
Fixes #17853
Clause-②: no
packages/cli's tier comment block told the reader that nothing is lost, three lines above two commands that lose things silently. This makes the loss visible and rewrites the block that invited the misreading.1. Acceptance item 1 first — and a correction the card is owed
Neither the filing seat nor triage could run the repro (no
node_modulesin a shared checkout). This is the first reading taken in a real worktree, on a built tree, at base84e6b05b6:vitest run --project unit test/i18n-extract-companion-orphan.test.tsNo test files found, exiting with code 1, plusfilter:andprojects:vitest run test/i18n-extract-companion-orphan.test.tsTest Files 1 passed (1),Tests 4 passed (4)Two different counts, so the pair does not trip the STOP condition. But⚠️ the card's headline sentence is FALSIFIED as literally written. Naming ONE integration-tier file under
--project unitis not "reported as a passing file with zero cases executed" — it is red, with a diagnostic. vitest'sprintNoTestFound()already covers the all-filters-lost case.⭐ The defect is one step over, and it is the shape that actually cost #16872 a round: the PARTIAL filter. Once at least one named path matches, the ones that matched nothing are dropped in complete silence. Measured here, two unit-tier paths plus one integration-tier path under
--project unit:The run naming only the two prints the same summary, and
diffover the two captures is empty but for timestamps and durations. The discarded name appears nowhere in vitest's own output — the single occurrence in a captured terminal is pnpm's echo of the argv inERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL, which pnpm prints only when the run was already red, and which is therefore absent from exactly the green run that needed it. That is why the card's own count (five named, four counted) was right about the harm while its one-file repro was not.2. The change
packages/cli/vitest-filter-preflight.ts(new) — readsprocess.argvthroughparseCLI, vitest's OWN exported parser, and the SAME two derived tier arrays the projects take as theirinclude. Any named path that will select no test file is reported by name, with the tier it really lives in and a command that runs it.packages/cli/vitest.config.ts— the whole:498-509block rewritten (see below), and the preflight invoked at config load.packages/cli/test/vitest-project-filter-preflight.test.ts(new, unit tier) — 28 cases.packages/cli/tsconfig.test.json— the new root harness module named ininclude, one by one like its three siblings, so it is in the tsc program by declaration and not by a test's import.⛔ Why this is not a reporter — a measured reversal
It was built as a
test.reportersentry first, which is the obvious seam. Direction ② caught it. Namingtest.reportersreplaces vitest's own defaulting instead of extending it:Two costs, one measured here and one that would only ever have shown up in CI:
defaultwhere an agent terminal getsagent— a healthy control run gained two lines, so direction ② failed outright; andgithub-actionsreporter on every CI run, silently removing failure annotations, in the one environment a local control run cannot observe.The reporter list belongs to vitest. The preflight runs at config load and touches no vitest seam.
⛔ Every uncertainty declines rather than guesses
An argv
parseCLIrefuses, a--projectthat is not a tier name (a negation or a glob), a--changed/--relatedrun: each prints nothing. Silence is the status quo, so declining can never make a run worse than it is today; a guess could.The rewritten block
The reassurance ("nothing is skipped…
pnpm testruns everything") is kept — it is true — but it is now stated as a fact about the POPULATION and immediately denied as a guarantee about one narrowed invocation. The safe command is printed FIRST, above the two that can lose things.3. Two directions, both measured
Both legs of a guarded ablation in one worktree, against base
84e6b05b6. The mutation was proved on disk by blob hash (git hash-object== the base blob) before each BEFORE run, and the restore proved the same way (== the HEAD blob,git diff HEADempty), under atrap ... EXIT INT TERM.① A filtered-out invocation is now distinguishable. Same command, before → after:
Occurrences of that banner — before: 0. After: 2 (once above the banner, once below the summary, which is where a reader looks for
Test Files N passed).② A healthy narrowed run is byte-identical.
--project unitnaming two unit-tier files, before vs after:A once-guard, also measured. vitest loads this config once per project and each load is its own module instance, so the first module-level flag guarded nothing: three loads printed the notice three times above the banner and three more at exit — six copies. The guard now lives on a scope object defaulting to
globalThis, and the pin calls twice on one scope.4. Verification
pnpm --filter @objectstack/cli typecheckvitest run --project unit test/vitest-project-filter-preflight.test.tsTest Files 1 passed (1),Tests 28 passed (28)pnpm lintscripts/pm/dispatch-gates.mjs→ 48 commandsExit codes were captured before any pipe (
cmd > file 2>&1; EXIT=$?).check:dual-build-cjs-loadsfirst returned exit 3PREREQUISITE NOT METandcheck:query-options-erasureexit 124 from a 150s cap of my own; both were re-run to a real verdict (after a fullpnpm build, and without the cap) and both are green — neither was reported as a measurement it was not.NOT MEASURED, declared rather than smoothed: the
integrationtier was not run locally. The diff touches no integration-tier file, no spawn entry point and no driver/kernel boot path, so that layer is declared to CI.packages/cli'sunittier was likewise not run whole — the derived families and the new pin were.5.⚠️ Changeset — a measured divergence from the dispatch order
The order says "Changeset required:
@objectstack/cli". Measured, nothing published moves, so this carriesskip-changesetinstead — flagged here rather than decided silently.files[]for@objectstack/cliisdist README.md CHANGELOG.md. After a full build, each new symbol greps to 0 hits across all three; positive controls on the same paths hit (runServe→ 1 file,objectstack→ 150 files,dist/holds 16 entries, so the probe is not blind). The four changed paths —vitest.config.ts,vitest-filter-preflight.ts,tsconfig.test.json,test/**— are test harness, andtsconfig.build.jsoncompilessrconly.Per AGENTS.md, a
patchwould publish a CHANGELOG sentence to consumers about a change they cannot observe, and picking a level to satisfy a gate is exactly what that rule forbids. ⇒ If the seat wants the changeset anyway, say so and it lands in a follow-up commit.6. Fences observed
vitest-tiers.ts's SPAWN / KERNEL predicates and the two-tier partition: untouched. The new pin classifies asunit(isIntegration = false,fired: none), so it adds one file to the unit tier and moves no file between tiers..claude/: untouched. See the acceptance notes.content/docs/releases/: untouched.naming/namespace-prefixreports a legitimate cross-package name reuse as an intra-package duplicate, contradicting the ADR-0048 §3.4 sentence in its own message #17821's concurrent test files underpackages/cli/test/are not reconciled here, and the derived population is exactly why they need not be.Acceptance notes
.claude/half stays unfiled by this seat, as ordered. The card's option ③ and its closing "second axis" — the agent contract judges integration-tier exposure by which FILES a diff touches, while the real exposure is what the derived command EXECUTES — are governed surface and route todomain:skills. This work makes that card no less necessary: the preflight tells a reader when a narrowed run lost something, but nothing here makes a narrowed run a valid pre-delivery target.projects(types,objectql,rest,runtime,spec,qa/dogfood,core). The silent partial-filter drop is vitest's behaviour, not this config's, so it is live in all of them; the preflight landed here is package-local by design. Raised in the report for the seat to route.:496-509, one line below the:498-509the dispatch printed, and the "fast, local default" line is:506— the card's own citation — not:505. The:583/:613citations hold exactly.slash()normalisation vitest applies to filters is deliberately not mirrored, and a--projectnegation or glob is deliberately not modelled. Both are stated in the module header; each can only ever mis-aim a diagnostic, never a run.Generated by Claude Code