Skip to content

feat(scripts): gate vi.mock specifiers that resolve to no file - #6198

Merged
yinlianghui-tw merged 2 commits into
mainfrom
claude/issue-5646-inert-vi-mock-gate
Aug 25, 2026
Merged

feat(scripts): gate vi.mock specifiers that resolve to no file#6198
yinlianghui-tw merged 2 commits into
mainfrom
claude/issue-5646-inert-vi-mock-gate

Conversation

@yinlianghui-tw

Copy link
Copy Markdown
Collaborator

Fixes#5646

A vi.mock / vi.doMock whose relative specifier names no file on disk does not error. Vitest registers the mock against a module id nothing imports, the run proceeds with the real module everywhere, and the suite passes — no warning, no smaller assertion count, identical green to a correct one. Same family as #4347 (a declaration pointing at nothing, reported as a pass), one layer down.

Adds scripts/check-vi-mock-specifiers.mjs, its test, and the workflow wiring.

The evidence, because this gate is GREEN AT REST

There are zero unresolvable specifiers in the tree, so a green run proves nothing on its own. The ablation is the evidence the gate exists. It uses the real historical instance, not a synthetic one.

PR #5645 wrote the runtime-config mock in packages/app-shell/src/layout/__tests__/ChatDock.partialRuntimeConfig.test.tsx one .. short. Reconstructed on the real file at head 519ce076b:

legresult
baseline, unmutatedexit=0
mutation landed on disk (anchored counts)vi.mock('../../runtime-config' 1 → 0; vi.mock('../runtime-config' 0 → 1
gate on mutated treeexit=1, 1 finding, naming …ChatDock.partialRuntimeConfig.test.tsx:48
restoregit checkout HEAD -- <path>, git diff HEADempty
gate on restored treeexit=0 ✅, identical census

The mutation is proven by anchored grep -c counts, never an editor's exit code. That guard earned its keep on the first attempt: a perl -pi whose s/// delimiter collided with the path failed, the counts stayed 2/0, and the run was reported as a no-op instead of a false green.

The discriminating half

The neighbours in that same file are what made the bug invisible: they sit at different depths and are both correct. A resolver that got depth wrong would flag them and be deleted by the first person it annoyed. With the defect present, none of them appears in the report:

  • '../AiUsageIndicator' — one level up → not flagged
  • '../../hooks/surfaceAgent' — two levels up → not flagged
  • '../../hooks' — two levels up, resolved via index.tsnot flagged

Repo-wide, the same half is measured by the census: 675 relative specifiers resolve, 0 unresolvable.

Non-vacuity

The population refuses to collapse. Zero source files, zero test files or zero relative specifiers is a broken walk, not a clean tree — reporting OK for it is this gate's own defect one level up. FLOORS makes each a failure, pinned end-to-end through main()'s exit code (a throwaway git repo carrying the import graph and nothing tracked → exit=1, population COLLAPSED), following the toBeGreaterThan(1000) shape from PR #6195. The verdict line prints the census rather than a bare OK.

Census — re-derived, not quoted

The card's figures were three days old. Measured at 519ce076b:

card (2026-08-21)this PR
test files18072013 test-named, of 3696 tracked source files walked
relative specifiers577675
unresolvable00

Plus, newly separated: 645 bare (out of scope), 0 non-static, 1 embedded in a string literal, 3 via the import() form.

Two corrections to the card

  1. vi.mock(import(…)) is not a zero population — the card and the dispatch order both recorded zero; there are three live call sites (plugin-kanban, plugin-calendar, plugin-list registration suites). What is zero is the intersection this gate judges: that form written with a relative specifier. The pattern covers it for free either way, and the census counts it separately so this stays visible.
  2. A test-file-named population has a hole. Three files carrying real call sites match no *.test.* / *.spec.* suffix, and two match no test-file convention at all — vitest.setup.base.ts and packages/plugin-map/vitest.setup.ts. A setup file is exactly where a repo-wide mock gets written. So the walk takes every tracked JS/TS source file and lets the pattern decide; the test-named count is still derived, reported, and floored.

Resolution, and the two things it must not judge

Matches the repo's real specifier style: bare path plus.ts/.tsx/.js/.jsx/.mjs/.cjs, the /index.* forms, and a trailing .jsstripped and the ladder retried (src/ is NodeNext throughout). The judgement is isFile, never a bare existence test — a directory with no index is correctly unresolved, which a bare existence check would wave through.

Two classes are counted but never judged, both because they are not text the language executes:

  • comments, masked via the shared scripts/js-comment-mask.mjs;
  • a call whose token sits inside a literal. The first run over the tree produced exactly one hit — eslint-rules/no-dynamic-import-in-test-hook.test.js:37 — and it is correct as written: a RuleTester code sample in a template literal, mocking a fictional './dep'. An ESLint fixture is source text; there is no directory it is relative to and no mock to be inert. The discriminator is the vitoken, not the specifier — nothing about the specifier separates the two spellings.

⚠️ Reported as a finding rather than fixed, per the dispatch order: no test file is touched by this PR.

Scope, per the dispatch ruling

  • Bare specifiers — out of scope. Counted in the census, never judged; resolving them needs the workspace map, not the filesystem.
  • Error, not warning.
  • vi.mock(import(…)) — covered because the same pattern already matches it; no handling invented beyond that.

Tier and wiring — confirmed, not assumed

Cheap-gate tier: no install, no build, no vitest run. Measured ~2.7s over 3696 files (the card said ~1s; the honest number is 2.7s, still a checkout plus one node call). Its own unfiltered workflow, vi-mock-specifiers.yml, alongside control-bytes.yml / skills-paths.yml / pre-install-import-graph.yml.

Mechanically confirmed rather than asserted: check-pre-install-import-graph derives its population from the workflows and now reports 15 pre-install steps in 14 jobs (was 14/13), having picked up this step and walked its import graph — builtins plus repo-relative modules only.

Registries updated because a new unfiltered blocking check must be classified, not defaulted into silence: REQUIRED_CONTEXTS in scripts/dependabot-merge-gate.mjs, the merge_group reason map in merge-queue-reporting.test.ts, and a section plus inventory row in content/docs/guide/ci-cd-pipeline.md.

Verification — all at 519ce076b

Gate list derived from package.json and .github/workflows/, not from the dispatch order. Every exit code captured before any pipe; every verdict below is the line the gate itself printed.

check:control-bytes [exit=0] OK (scanned 5116 tracked text file(s); skipped 85 binary)
check:vi-mock-specifiers [exit=0] OK (3696 source, 2013 test-named, 675 relative resolved)
check:pre-install(self-test) [exit=0] 16 cases pass
check:pre-install-import-graph [exit=0] OK - 15 pre-install step(s) in 14 job(s), 17 module(s) walked
check:entry-guard(self-test) [exit=0] 63 cases pass
check:entry-guard [exit=0] 44 scripts/ file(s), no guard outside the baseline
check:lint-coverage [exit=0] 46/46 packages linted, 0 errors
type-check:coverage [exit=0] 45/46 via type-check, 0 known-broken
docs:check-links [exit=0] Links are valid across 15 scan roots
check:doc-fences [exit=0] OK across 223 document(s)
changeset-presence [exit=0] No source of a released package changed - no changeset owed
changeset:check(fixed / no-major) [exit=0] OK
tsc -p tsconfig.scripts.json [exit=0]
pnpm run lint:root [exit=0] 28 problems (0 errors, 28 warnings - all pre-existing, none in this diff)
vitest run scripts/__tests__/ [exit=0] 72 files, 1976 tests passed

Root vitest only (#3378). No changeset: check-changeset-presence computed the diff against the merge-base and reports 0 files of published package source — nothing in this PR ships in a tarball.


Generated by Claude Code

A `vi.mock` / `vi.doMock` whose relative specifier names no file on disk does
not error. Vitest registers the mock against a module id nothing imports, the
run proceeds with the real module everywhere, and the suite passes — with no
warning and no smaller assertion count, identically to a correct one.
Adds `scripts/check-vi-mock-specifiers.mjs`: walks every tracked JS/TS-family
source file, finds each mock call site, and resolves the relative specifiers
against the calling file's own directory. Resolution matches the repo's real
specifier style (extension ladder, /index.* forms, NodeNext trailing-.js strip)
and judges with isFile rather than a bare existence test.
The gate is green at rest, so two things keep it from being indistinguishable
from a gate that does nothing: the verdict line carries the census, and the
scan FAILS when its population collapses to nothing.
Wired as a cheap-tier gate in its own unfiltered workflow — checkout plus one
node call, no install and no build.
…oors
The inferred types from the parameter defaults were `null | undefined` and the
frozen literal shape of FLOORS, so a fixture caller passing its own file list
or `{}` for the floors failed `tsc -p tsconfig.scripts.json`.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nothing catches a vi.mock() whose relative specifier resolves to no file — vitest no-ops it and the suite stays green

2 participants

@yinlianghui-tw@claude