From d38ba4c901824f1a276e14c8ac16c9ac38453bed Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 00:27:44 +0000 Subject: [PATCH 1/2] feat(scripts): gate vi.mock specifiers that resolve to no file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/vi-mock-specifiers.yml | 81 +++ content/docs/guide/ci-cd-pipeline.md | 48 ++ package.json | 1 + .../check-vi-mock-specifiers.test.ts | 510 ++++++++++++++++++ .../__tests__/merge-queue-reporting.test.ts | 7 + scripts/check-vi-mock-specifiers.mjs | 449 +++++++++++++++ scripts/dependabot-merge-gate.mjs | 2 + 7 files changed, 1098 insertions(+) create mode 100644 .github/workflows/vi-mock-specifiers.yml create mode 100644 scripts/__tests__/check-vi-mock-specifiers.test.ts create mode 100644 scripts/check-vi-mock-specifiers.mjs diff --git a/.github/workflows/vi-mock-specifiers.yml b/.github/workflows/vi-mock-specifiers.yml new file mode 100644 index 0000000000..593fe77c97 --- /dev/null +++ b/.github/workflows/vi-mock-specifiers.yml @@ -0,0 +1,81 @@ +name: Inert vi.mock Specifiers + +# Why this is its own workflow rather than a step in `ci.yml` or `lint.yml`: a +# module mock can be written into any package, in any shape of pull request, and +# both of those workflows decide inside the job whether the change "needs a full +# run" — with an exclusion list that skips every expensive step on a +# markdown-only or changeset-only change. This gate costs a checkout plus one +# `node` call, so there is nothing to gain by putting it behind that switch and +# a whole class of pull request to lose. +# +# Same shape and the same reasoning as `docs-links.yml`, `control-bytes.yml`, +# `skills-paths.yml`, `changeset-presence.yml` and `pre-install-import-graph.yml`, +# whose headers record the conclusion this repository has now reached five +# times: a gate that cannot see the pull request shape most likely to trip it +# "rebuilds the hole it exists to close". One gate, one home. +# +# Hence: no `paths` and no `paths-ignore` here, deliberately. +# `scripts/__tests__/check-vi-mock-specifiers.test.ts` fails if either is ever +# added, and fails too if a second workflow starts running the same script. +# Reporting on every pull request is also what makes the check requirable, and +# `scripts/dependabot-merge-gate.mjs` classifies it as a required context — an +# unclassified blocking check is one a Dependabot merge would be let past +# (objectui#6135). +# +# It needs no install and no build — a checkout plus one `node` call over ~3.7k +# tracked source files, measured at ~2.7s on this tree. Keep it that way; the +# import graph is builtins plus repo-relative modules only, which +# `pre-install-import-graph.yml` enforces (objectui#6148). + +on: + pull_request: + branches: [main, develop] + push: + branches: [main, develop] + # Merge queue (objectui#3523 — see `ci.yml`'s trigger block for the full note + # and the measurements behind it). A required check that does not report on a + # queue build stalls the queue until the ruleset's 60-minute timeout fails it, + # so an unfiltered gate that can become required subscribes here from the + # start. `types:` is named although `checks_requested` is currently the only + # activity type GitHub defines for `merge_group`. + merge_group: + types: [checks_requested] + workflow_dispatch: + +concurrency: + group: vi-mock-specifiers-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + vi-mock-specifiers: + name: Inert vi.mock Specifier Check + runs-on: ubuntu-latest + timeout-minutes: 5 + + steps: + - name: Checkout code + uses: actions/checkout@v7 + + - name: Setup Node.js + uses: actions/setup-node@v7 + with: + node-version: '22.x' + + # A `vi.mock` whose relative specifier resolves to no file 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 — + # identically to a correct one, with no warning and no smaller assertion + # count. objectui#5646: the one known instance (PR #5645) passed even with + # the code under test reverted to the shape the suite was written to + # catch, and only an ablation leg exposed it. + # + # This gate is GREEN AT REST — there are zero unresolvable specifiers in + # the tree and there should stay zero — so it prints its census rather + # than a bare "OK", and it FAILS if the population collapses to nothing. + # A scan that silently finds nothing reads as coverage, which is this + # gate's own defect one level up. + - name: Check every relative vi.mock specifier resolves + run: node scripts/check-vi-mock-specifiers.mjs diff --git a/content/docs/guide/ci-cd-pipeline.md b/content/docs/guide/ci-cd-pipeline.md index b9ca8edfff..6e70bb68ea 100644 --- a/content/docs/guide/ci-cd-pipeline.md +++ b/content/docs/guide/ci-cd-pipeline.md @@ -34,6 +34,7 @@ one has its own section below. | `doc-snippet-types.yml` | Doc Snippet Type Check | Push / PR to `main`, `develop` — **no path filter**; merge-queue builds; manual | **Yes** — when a covered documentation snippet no longer compiles against the packages' built types | | `doc-fence-languages.yml` | Doc Fence Language Check | Push / PR to `main`, `develop` — **no path filter**; merge-queue builds; manual | **Yes** — when a TypeScript block sits under a fence the snippet gate does not read | | `pre-install-import-graph.yml` | Pre-Install Import Graph Check | Push / PR to `main`, `develop` — **no path filter**; merge-queue builds; manual | **Yes** — when a gate a workflow runs *before* `pnpm install` reaches a package anywhere in its import graph | +| `vi-mock-specifiers.yml` | Inert vi.mock Specifier Check | Push / PR to `main`, `develop` — **no path filter**; merge-queue builds; manual | **Yes** — when a `vi.mock` / `vi.doMock` relative specifier resolves to no file, or the scan's population collapses | | `performance-budget.yml` | Bundle Analysis | Push / PR touching `packages/**`, `apps/console/**`, `pnpm-lock.yaml` | **Yes** — the console entry gzip budget | | `live-e2e.yml` | Live E2E (informational) | PR to `main`, `develop` (code paths); nightly cron `30 6 * * *`; manual | No — informational lane, `continue-on-error` | | `labeler.yml` | Auto Label PRs | PR `opened`, `synchronize`, `reopened` | No | @@ -783,6 +784,53 @@ the import is deliberately *not* this gate's job: either drop the package, or mo the derived population and every module walked, or `--self-test` to exercise the parser and the walk against fixtures. +## Inert vi.mock Specifiers (`vi-mock-specifiers.yml`) + +**Triggers:** Push and PR to `main`/`develop`, merge-queue builds, plus manual dispatch — with **no +path filter at all**. A module mock can be written into any package in any shape of pull request, and +the scan costs a checkout plus one `node` call, so there is nothing to gain by hiding it behind a +filter. It appears in the checks list as **Inert vi.mock Specifier Check**. + +Runs `scripts/check-vi-mock-specifiers.mjs`. It walks every tracked JS/TS-family source file, finds +each `vi.mock` / `vi.doMock` call site, and resolves the **relative** specifiers against the calling +file's own directory. Any that resolves to no file fails the run. + +**Why it needed a gate.** A mock whose specifier names no file does **not** error. Vitest registers +it 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. In +[#5646](https://github.com/objectstack-ai/objectui/issues/5646)'s one known instance (PR #5645) the +suite passed even when the code under test was reverted to the exact broken shape it had been written +to catch; only an ablation leg exposed it. Neighbouring mocks in that same file made it invisible to a +reader: one stepped up a single level and one stepped up two, and **both were correct**, because +their targets sat at different depths. This is +[#4347](https://github.com/objectstack-ai/objectui/issues/4347) one layer down — a declaration +pointing at nothing, reported as a pass. + +**It is green at rest, so its census is part of the verdict.** There are zero unresolvable specifiers +in the tree and there should stay zero, which means the run's output alone cannot distinguish a +working gate from one that matches nothing. Two things answer that. The verdict line prints the +**population** it judged, not a bare `OK`. And the scan **fails when that population collapses**: no +source files, no test files, or no relative specifiers is a broken walk, not a clean tree, and +reporting `OK` for it would be this gate's own defect one level up. The evidence that the gate works +lives in `scripts/__tests__/check-vi-mock-specifiers.test.ts`, which reconstructs the historical +specifier on a fixture tree and pins that the two correct neighbours are *not* flagged. + +**Resolution matches how this repo spells specifiers**, which is more than an existence check: the +bare path plus `.ts/.tsx/.js/.jsx/.mjs/.cjs`, the `/index.*` forms, and a trailing `.js` stripped and +retried, because `src/` is NodeNext throughout. The judgement is `isFile` rather than "exists", so a +directory with no index is correctly unresolved. Comments are masked and a call quoted inside a string +literal is counted but not judged — an ESLint `RuleTester` code sample is source text, not a mock. + +**Scope:** relative specifiers only. A bare specifier (`@object-ui/…`, `lucide-react`) can be +misspelled too, but resolving one needs the workspace map rather than the filesystem — a different +check with a different failure mode. Bare specifiers are counted in the census and never judged. + +**If it fails:** it names the file, the line and the specifier, and the first path it tried. Fix the +specifier, then confirm the mock is really installed by reverting the code under test and checking +that the suite goes red. Run it locally with `pnpm check:vi-mock-specifiers`, or +`node scripts/check-vi-mock-specifiers.mjs --list` to see every call site the walk found. It needs no +install and no build. + ## Link Checking (`check-links.yml`) **Trigger:** Weekly cron (`17 4 * * 0` — Sundays, off the top of the hour, when the scheduled-run diff --git a/package.json b/package.json index 55f2bdf24b..1697fe87ea 100644 --- a/package.json +++ b/package.json @@ -58,6 +58,7 @@ "check:eager-closure": "node scripts/check-eager-closure-budget.mjs", "check:entry-guard": "node scripts/check-entry-guard.mjs", "check:pre-install-import-graph": "node scripts/check-pre-install-import-graph.mjs", + "check:vi-mock-specifiers": "node scripts/check-vi-mock-specifiers.mjs", "cli": "node packages/cli/dist/cli.js", "objectui": "node packages/cli/dist/cli.js", "create-plugin": "node packages/create-plugin/dist/index.js", diff --git a/scripts/__tests__/check-vi-mock-specifiers.test.ts b/scripts/__tests__/check-vi-mock-specifiers.test.ts new file mode 100644 index 0000000000..2e69641d2c --- /dev/null +++ b/scripts/__tests__/check-vi-mock-specifiers.test.ts @@ -0,0 +1,510 @@ +import { describe, expect, it } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +// Plain-JS CI helper. Its types are INFERRED from the .mjs source by +// `tsconfig.scripts.json` (`allowJs`), so no `@ts-expect-error` here — +// re-adding one is now itself an error (TS2578). See objectui#3494. +import { + CALL_RE, + FLOORS, + SOURCE_EXTENSIONS, + candidatesFor, + findCallSites, + resolveSpecifier, + scan, + summarise, +} from '../check-vi-mock-specifiers.mjs'; + +/** + * objectui#5646 — the test for `scripts/check-vi-mock-specifiers.mjs`. + * + * ## Why this file carries more weight than usual + * + * The tree has ZERO unresolvable specifiers and is expected to keep having zero, + * so a green run of the gate over this repo proves only that the tree is clean. + * It cannot distinguish a working gate from one that matches nothing at all — + * which is this card's own defect, one level up. + * + * So the ABLATION below is the evidence the gate exists. It is not synthetic: + * the fixture tree reproduces the geometry of the real instance (PR #5645), and + * `THE HISTORICAL INSTANCE` reconstructs the exact specifier that was written. + * The discriminating half sits right beside it — the neighbours in that same + * real file step up one level and two levels and are BOTH correct, so a + * resolver that got depth wrong would flag them and be deleted by the first + * person it annoyed. + * + * ## Fixture discipline: never write a matchable call site into this source + * + * This file is inside the gate's own scan scope. Two things keep its fixtures + * out of the repo census, and they are belt and braces on purpose: + * + * 1. every fixture is built through `mockCall()`, which interpolates the + * quote — the source text here reads `vi.${fn}(` and the pattern needs a + * literal `mock`/`doMock` followed by a quote, so it never matches; + * 2. and any text that DID match would be inside a string literal, which the + * gate classifies as `embedded` and declines to judge anyway. + * + * The first is what keeps the census figure honest: without it this suite's + * fixtures would dominate the `embedded` count and drown the real signal. + */ + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); + +/** A quote, from its code point — see "Fixture discipline" above. */ +const Q = String.fromCharCode(39); + +/** A mock call as SOURCE TEXT, unmatchable in this file, matchable on disk. */ +const mockCall = (spec: string, fn: 'mock' | 'doMock' = 'mock') => `vi.${fn}(${Q}${spec}${Q}, () => ({}));`; + +/** The same, in the `import()` form the dispatch ruling asked to cover for free. */ +const mockCallViaImport = (spec: string) => `vi.${'mock'}(import(${Q}${spec}${Q}), () => ({}));`; + +/** Build a throwaway tree and hand back its root plus a relative file list. */ +function fixtureTree(files: Record) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'check-vi-mock-')); + for (const [rel, body] of Object.entries(files)) { + const full = path.join(root, rel); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, body); + } + return { root, files: Object.keys(files) }; +} + +/** Fixture scans pass their own file list and switch the floors off. */ +const scanFixture = (root: string, files: string[]) => scan(root, { files, floors: {} }); + +// --------------------------------------------------------------------------- +// The pattern +// --------------------------------------------------------------------------- + +describe('findCallSites — what counts as a call site at all', () => { + it('matches both mock functions and classifies a relative specifier', () => { + const sites = findCallSites([mockCall('./a'), mockCall('../b', 'doMock')].join('\n')); + expect(sites.map((s: { fn: string; specifier: string; kind: string }) => [s.fn, s.specifier, s.kind])).toEqual([ + ['mock', './a', 'relative'], + ['doMock', '../b', 'relative'], + ]); + }); + + it('classifies a bare specifier as out of scope rather than judging it', () => { + const sites = findCallSites(mockCall('@object-ui/react')); + expect(sites[0].kind).toBe('bare'); + }); + + it('covers the import() form, and marks it as such', () => { + const sites = findCallSites(mockCallViaImport('../thing')); + expect(sites).toHaveLength(1); + expect(sites[0].kind).toBe('relative'); + expect(sites[0].viaImport).toBe(true); + }); + + it('reports the line the call is on, so a finding is navigable', () => { + const sites = findCallSites(['// a comment', '', mockCall('./a')].join('\n')); + expect(sites[0].line).toBe(3); + }); + + it('leaves an interpolated specifier unjudged instead of guessing at it', () => { + // Not a static path, so there is nothing to resolve. Counted, never resolved. + const sites = findCallSites(`vi.${'mock'}(\`./\${name}\`, () => ({}));`); + expect(sites[0].kind).toBe('dynamic'); + }); + + it('does not match a mention with no specifier — prose and partial calls', () => { + expect(findCallSites('see vi.mock(..., ...) for details')).toEqual([]); + }); + + it('is a global pattern that is safe to reuse (lastIndex is reset per call)', () => { + const src = mockCall('./a'); + expect(findCallSites(src)).toHaveLength(1); + expect(findCallSites(src)).toHaveLength(1); + expect(CALL_RE.global).toBe(true); + }); +}); + +describe('findCallSites — only text the language would execute', () => { + it('ignores a commented-out mock: it is not executed, so it cannot be inert', () => { + expect(findCallSites(`// ${mockCall('./gone')}`)).toEqual([]); + expect(findCallSites(`/* ${mockCall('./gone')} */`)).toEqual([]); + }); + + it('counts a call quoted inside a literal as `embedded`, and does not judge it', () => { + // The real instance of this shape: + // `eslint-rules/no-dynamic-import-in-test-hook.test.js` lints code SAMPLES + // held in template literals, and one of them mocks a fictional './dep'. An + // ESLint fixture is source text — there is no directory it is relative to + // and no mock to be inert, so flagging it fabricates a finding. + const sites = findCallSites(`const sample = \`beforeAll(() => { ${mockCall('./dep', 'doMock')} });\`;`); + expect(sites).toHaveLength(1); + expect(sites[0].kind).toBe('embedded'); + }); + + it('separates the two by the CALL TOKEN, not by the specifier', () => { + // Both spellings below carry an identical specifier. Nothing about it + // separates them; only whether `vi` is code does. + const real = findCallSites(mockCall('./dep')); + const quoted = findCallSites(`const s = \`${mockCall('./dep')}\`;`); + expect(real[0].specifier).toBe(quoted[0].specifier); + expect([real[0].kind, quoted[0].kind]).toEqual(['relative', 'embedded']); + }); + + it('still sees a real call in a file that also holds prose about one', () => { + // The blinding direction: a mask that dropped too much would report clean + // over live code. This gate's own header quotes the defect in prose. + const sites = findCallSites([`/** docs mentioning ${mockCall('./prose')} */`, mockCall('./real')].join('\n')); + expect(sites.map((s: { specifier: string }) => s.specifier)).toEqual(['./real']); + }); +}); + +// --------------------------------------------------------------------------- +// The resolver +// --------------------------------------------------------------------------- + +describe('resolveSpecifier — the ladder this repo actually needs', () => { + const { root } = fixtureTree({ + 'src/runtime-config.ts': 'export const x = 1;\n', + 'src/hooks/surfaceAgent.ts': 'export const a = 1;\n', + 'src/hooks/index.ts': 'export * from "./surfaceAgent";\n', + 'src/layout/AiUsageIndicator.tsx': 'export const C = () => null;\n', + 'src/legacy/thing.jsx': 'export const y = 1;\n', + 'src/empty-dir/.keep': '', + }); + const from = path.join(root, 'src/layout/__tests__'); + + const resolves = (spec: string) => Boolean(resolveSpecifier(from, spec).resolved); + + it('resolves a bare path by appending an extension', () => { + expect(resolves('../../runtime-config')).toBe(true); + expect(resolves('../AiUsageIndicator')).toBe(true); + }); + + it('resolves a directory specifier through its index file', () => { + expect(resolves('../../hooks')).toBe(true); + }); + + it('strips a trailing .js — src/ is NodeNext throughout', () => { + expect(resolves('../../runtime-config.js')).toBe(true); + expect(resolves('../../hooks/surfaceAgent.js')).toBe(true); + // ...and the same for the sibling JS-ish extensions. `thing.jsx` really is + // on disk here, so this leg is about the extension, not about the depth. + expect(resolves('../../legacy/thing.jsx')).toBe(true); + }); + + it('does NOT accept a directory that has no index — isFile, not existsSync', () => { + // The directory exists. A bare existence check would call this resolved and + // let a real inert mock through. + expect(fs.existsSync(path.join(root, 'src/empty-dir'))).toBe(true); + expect(resolves('../../empty-dir')).toBe(false); + }); + + it('reports a miss, with the candidates it tried', () => { + const { resolved, tried } = resolveSpecifier(from, './nowhere'); + expect(resolved).toBeNull(); + expect(tried.length).toBeGreaterThan(1); + expect(tried[0]).toBe(path.join(from, 'nowhere')); + }); + + it('offers every declared extension, and the index form of each', () => { + const tried = candidatesFor(from, '../x'); + for (const ext of SOURCE_EXTENSIONS) { + expect(tried).toContain(path.join(root, 'src/layout', `x${ext}`)); + expect(tried).toContain(path.join(root, 'src/layout/x', `index${ext}`)); + } + }); +}); + +// --------------------------------------------------------------------------- +// THE ABLATION — the real historical instance, and its correct neighbours +// --------------------------------------------------------------------------- + +describe('ablation — PR #5645, reconstructed', () => { + /** + * The geometry of the real file, reproduced: the suite sits in + * `layout/__tests__/`, one mock target is a sibling of `layout/` and another + * is two levels up in `src/`. That difference in depth is the whole reason the + * bug was invisible to a reader scanning the mock block. + */ + const tree = { + 'src/runtime-config.ts': 'export const getRuntimeConfig = () => ({});\n', + 'src/hooks/surfaceAgent.ts': 'export const resolveSurfaceAgent = () => null;\n', + 'src/hooks/index.ts': 'export * from "./surfaceAgent";\n', + 'src/layout/AiUsageIndicator.tsx': 'export const AiUsageIndicator = () => null;\n', + }; + + const suiteAt = 'src/layout/__tests__/ChatDock.partialRuntimeConfig.test.tsx'; + + const runWith = (mockBlock: string[]) => { + const { root, files } = fixtureTree({ ...tree, [suiteAt]: `${mockBlock.join('\n')}\n` }); + return scanFixture(root, files); + }; + + const flagged = (result: { unresolvable: { specifier: string }[] }) => + result.unresolvable.map((u) => u.specifier); + + it('THE HISTORICAL INSTANCE: one .. short, and the gate goes RED', () => { + // What PR #5645 actually wrote. The suite passed — it passed even with the + // code under test reverted to the shape the suite was written to catch. + const result = runWith([mockCall('../runtime-config')]); + expect(flagged(result)).toEqual(['../runtime-config']); + }); + + it('the corrected specifier resolves — the fix is recognised as a fix', () => { + expect(flagged(runWith([mockCall('../../runtime-config')]))).toEqual([]); + }); + + it('THE DISCRIMINATING HALF: the correct neighbours are NOT flagged', () => { + // Both are real specifiers from that same file, at DIFFERENT depths, and + // both are correct. A resolver that got depth wrong flags these — and a + // gate that flags hundreds of correct call sites gets deleted, not fixed. + const result = runWith([ + mockCall('../AiUsageIndicator'), // one level up — a sibling of layout/ + mockCall('../../hooks/surfaceAgent'), // two levels up — under src/ + mockCall('../../hooks'), // two levels up — resolved via index + mockCall('@object-ui/i18n'), // bare — out of scope, never judged + ]); + expect(flagged(result)).toEqual([]); + expect(result.census.relative).toBe(3); + expect(result.census.bare).toBe(1); + }); + + it('catches the broken one while its correct neighbours sit around it', () => { + // The mock block as the file really reads: a plausible mix of depths with + // exactly one wrong. This is the case a human reviewer failed. + const result = runWith([ + mockCall('@object-ui/i18n'), + mockCall('../runtime-config'), // the defect + mockCall('../../hooks/surfaceAgent'), + mockCall('../../hooks'), + mockCall('../AiUsageIndicator'), + ]); + expect(flagged(result)).toEqual(['../runtime-config']); + }); + + it('names the file and the line, so the finding is actionable', () => { + const result = runWith([mockCall('@object-ui/i18n'), mockCall('../runtime-config')]); + expect(result.unresolvable[0].file).toBe(suiteAt); + expect(result.unresolvable[0].line).toBe(2); + }); + + it('catches the same defect written as vi.doMock and as the import() form', () => { + expect(flagged(runWith([mockCall('../runtime-config', 'doMock')]))).toEqual(['../runtime-config']); + expect(flagged(runWith([mockCallViaImport('../runtime-config')]))).toEqual(['../runtime-config']); + }); + + it('judges a setup file too — the walk is not restricted to test-NAMED files', () => { + // Measured on this tree: two files carrying real call sites match no + // test-file naming convention at all, and both are vitest setup files. + const { root, files } = fixtureTree({ ...tree, 'vitest.setup.ts': `${mockCall('../runtime-config')}\n` }); + expect(flagged(scanFixture(root, files))).toEqual(['../runtime-config']); + }); +}); + +// --------------------------------------------------------------------------- +// NON-VACUITY — a scan that finds nothing must FAIL, not pass +// --------------------------------------------------------------------------- + +describe('non-vacuity — the population refuses to collapse', () => { + /** + * objectui#6195 landed this discipline one level over. The reasoning is the + * card's own: a scan that silently finds nothing reports OK and reads as + * coverage, which is the exact failure this gate exists to catch. So an empty + * population is a FAILURE here, not a pass. + */ + it('declares a floor for every counter a collapse would zero', () => { + expect(Object.keys(FLOORS).sort()).toEqual(['relative', 'sources', 'testFiles']); + for (const [name, floor] of Object.entries(FLOORS)) { + expect(floor, `FLOORS.${name} must be a real floor, not zero`).toBeGreaterThan(0); + } + }); + + it('reports every floor as breached when the walk returns nothing at all', () => { + const result = scan(repoRoot, { files: [] }); + expect(result.unresolvable).toEqual([]); // clean by the only measure it has... + expect(result.vacuous.map((v: { counter: string }) => v.counter).sort()).toEqual([ + 'relative', + 'sources', + 'testFiles', + ]); // ...and that is exactly why it must still fail + }); + + it('breaches the test-file floor when the walk finds sources but no tests', () => { + const files = Array.from({ length: 2000 }, (_, i) => `packages/p/src/mod${i}.ts`); + const result = scan(repoRoot, { files }); + const breached = result.vacuous.map((v: { counter: string }) => v.counter); + expect(breached).toContain('testFiles'); + expect(breached).toContain('relative'); + expect(breached).not.toContain('sources'); + }); + + it('exits non-zero on a collapsed population, with the census in the message', () => { + // End to end through `main()`, because the exit code is the whole contract + // with CI — the two assertions above are about the scan's return value, and + // a `main()` that swallowed `vacuous` would pass both. + // + // The gate resolves its repo root from its OWN location, never from `cwd`, + // so pointing a child process at an empty directory would still scan THIS + // tree and exit 0. The probe therefore copies the whole import graph into a + // throwaway git repo and runs it from there. + const probe = fs.mkdtempSync(path.join(os.tmpdir(), 'check-vi-mock-empty-')); + fs.mkdirSync(path.join(probe, 'scripts')); + for (const f of ['check-vi-mock-specifiers.mjs', 'invoked-as.mjs', 'js-comment-mask.mjs']) { + fs.copyFileSync(path.join(repoRoot, 'scripts', f), path.join(probe, 'scripts', f)); + } + execFileSync('git', ['init', '-q'], { cwd: probe }); + + let status = 0; + let output = ''; + try { + // Nothing is `git add`ed, so `git ls-files` succeeds and returns nothing. + execFileSync('node', ['scripts/check-vi-mock-specifiers.mjs'], { cwd: probe, encoding: 'utf8' }); + } catch (err) { + const e = err as { status: number; stdout: string; stderr: string }; + status = e.status; + output = `${e.stdout}${e.stderr}`; + } finally { + fs.rmSync(probe, { recursive: true, force: true }); + } + expect(status, 'an empty scan must be RED — a green here is the defect itself').toBe(1); + expect(output).toMatch(/population COLLAPSED/); + expect(output, 'the message must name which counters collapsed').toMatch(/sources: found 0, floor is/); + }); +}); + +// --------------------------------------------------------------------------- +// The tree as it stands +// --------------------------------------------------------------------------- + +describe('repo state — the gate is green on this tree', () => { + const result = scan(repoRoot); + + it('has no vi.mock specifier resolving to nothing', () => { + expect( + result.unresolvable.map((u: { file: string; line: number; specifier: string }) => `${u.file}:${u.line} ${u.specifier}`), + 'Run `pnpm check:vi-mock-specifiers` for the full report and the fix guidance.', + ).toEqual([]); + }); + + it('actually walked the tree rather than silently matching nothing', () => { + // The empty-verdict trap: without these, the assertion above passes for the + // wrong reason on the day the walk breaks. Floors, not exact counts — the + // measured figures move every day (they were 3694 / 2012 / 675 when this + // landed, against 1807 / 577 in the card three days earlier). + expect(result.census.sources).toBeGreaterThan(1000); + expect(result.census.testFiles).toBeGreaterThan(1000); + expect(result.census.relative).toBeGreaterThan(100); + expect(result.vacuous).toEqual([]); + }); + + it('puts the census in the verdict, so a reader sees the population', () => { + // "OK" alone is what a gate that does nothing also prints. + const line = summarise(result); + expect(line).toMatch(/\d+ tracked source file\(s\)/); + expect(line).toMatch(/\d+ relative specifier\(s\) resolved/); + const out = execFileSync('node', ['scripts/check-vi-mock-specifiers.mjs'], { cwd: repoRoot, encoding: 'utf8' }); + expect(out).toMatch(/check-vi-mock-specifiers: OK/); + expect(out).toContain(`${result.census.relative} relative specifier(s) resolved`); + }); + + it('needs no install and no build — it is a cheap-tier gate', () => { + // The claim the tier rests on. `node_modules` is not consulted: the whole + // import graph is builtins plus repo-relative modules, which + // `check-pre-install-import-graph.mjs` enforces for every pre-install gate. + const src = fs.readFileSync(path.join(repoRoot, 'scripts/check-vi-mock-specifiers.mjs'), 'utf8'); + const imports = [...src.matchAll(/^import .*? from '([^']+)';$/gm)].map((m) => m[1]); + expect(imports.length).toBeGreaterThan(3); + for (const spec of imports) { + expect(spec.startsWith('node:') || spec.startsWith('./'), `${spec} would need an install`).toBe(true); + } + }); +}); + +describe('objectui#5646 — the real file the instance was written in', () => { + const target = 'packages/app-shell/src/layout/__tests__/ChatDock.partialRuntimeConfig.test.tsx'; + + it('still exists — or the case below tests nothing', () => { + expect(fs.existsSync(path.join(repoRoot, target))).toBe(true); + }); + + it('has every relative mock in it resolving, at all three depths', () => { + // Pins the corrected specifier AND its neighbours against the real tree, so + // a future move of `runtime-config.ts` or `hooks/` reddens here too. + const source = fs.readFileSync(path.join(repoRoot, target), 'utf8'); + const from = path.dirname(path.join(repoRoot, target)); + const relative = findCallSites(source).filter((s: { kind: string }) => s.kind === 'relative'); + expect(relative.length).toBeGreaterThan(2); + for (const site of relative) { + expect(resolveSpecifier(from, site.specifier).resolved, `${site.specifier} resolves to nothing`).not.toBeNull(); + } + }); +}); + +// --------------------------------------------------------------------------- +// Wiring +// --------------------------------------------------------------------------- + +describe('wiring — the gate is reachable and every PR shape starts it', () => { + const SCRIPT = 'scripts/check-vi-mock-specifiers.mjs'; + const WORKFLOW = 'vi-mock-specifiers.yml'; + const workflowDir = path.join(repoRoot, '.github/workflows'); + const workflowFiles = fs.readdirSync(workflowDir).filter((f) => f.endsWith('.yml')); + + /** + * A workflow's YAML with whole-line comments removed — this workflow's header + * discusses `paths` and `paths-ignore` in prose, and the sibling cheap gates + * name each other's scripts in theirs. A scan that counted comments would + * report filters and duplicate homes that no file has. + */ + const yamlOf = (file: string) => + fs + .readFileSync(path.join(workflowDir, file), 'utf8') + .split('\n') + .filter((line) => !/^\s*#/.test(line)) + .join('\n'); + + it('is exposed as a root package script', () => { + const pkg = JSON.parse(fs.readFileSync(path.join(repoRoot, 'package.json'), 'utf8')); + expect(pkg.scripts['check:vi-mock-specifiers']).toBe(`node ${SCRIPT}`); + }); + + it('has a workflow that gates pull requests, not just pushes', () => { + expect(fs.existsSync(path.join(workflowDir, WORKFLOW)), 'a check nothing runs is not a gate').toBe(true); + const yaml = yamlOf(WORKFLOW); + expect(yaml).toMatch(new RegExp(`run:\\s*node\\s+${SCRIPT.replace(/[.]/g, '\\.')}`)); + expect(yaml).toMatch(/^\s*pull_request:/m); + expect(yaml).toMatch(/^\s*push:/m); + }); + + it('subscribes merge_group — a required check that skips a queue build stalls it', () => { + // objectui#3523: `main` sits behind an enforced queue, and a required + // context that never reports on the queue build does not fail it, it hangs + // it until the ruleset's 60-minute timeout. + expect(yamlOf(WORKFLOW)).toMatch(/^\s*merge_group:/m); + }); + + it('runs it in NO path-filtered workflow', () => { + // A mock can be written into any package, so no path filter is correct — + // and reporting on every PR shape is also what makes the check requirable. + expect(workflowFiles.length, 'the workflow directory scan returned implausibly few files').toBeGreaterThan(5); + for (const file of workflowFiles) { + const yaml = yamlOf(file); + if (!yaml.includes(SCRIPT)) continue; + expect(yaml, `${file} runs ${SCRIPT} behind a paths-ignore`).not.toMatch(/paths-ignore:/); + expect(yaml, `${file} runs ${SCRIPT} behind a paths filter — see objectui#3448`).not.toMatch(/^\s+paths:/m); + } + }); + + it('has exactly one home', () => { + // A second copy in a path-filtered workflow is how a gate ends up looking + // covered while the change it exists for still slips past. + expect(workflowFiles.filter((f) => yamlOf(f).includes(SCRIPT))).toEqual([WORKFLOW]); + }); + + it('installs nothing before running the gate — the cheap tier, mechanically', () => { + const yaml = yamlOf(WORKFLOW); + expect(yaml).not.toMatch(/pnpm install/); + expect(yaml.indexOf(SCRIPT)).toBeGreaterThan(-1); + }); +}); diff --git a/scripts/__tests__/merge-queue-reporting.test.ts b/scripts/__tests__/merge-queue-reporting.test.ts index a1667b93c6..806330802e 100644 --- a/scripts/__tests__/merge-queue-reporting.test.ts +++ b/scripts/__tests__/merge-queue-reporting.test.ts @@ -100,6 +100,13 @@ const MUST_SUBSCRIBE_MERGE_GROUP = new Map([ 'pull request, and is requirable; `scripts/dependabot-merge-gate.mjs` already classifies ' + 'it as a required context', ], + [ + 'vi-mock-specifiers.yml', + 'produces Inert vi.mock Specifier Check — added by objectui#5646. A module mock can be ' + + 'written into any package in any shape of pull request, and the gate costs a checkout plus ' + + 'one node call, so it carries no path filter, reports on every pull request, and is ' + + 'requirable; `scripts/dependabot-merge-gate.mjs` already classifies it as a required context', + ], ]); /** diff --git a/scripts/check-vi-mock-specifiers.mjs b/scripts/check-vi-mock-specifiers.mjs new file mode 100644 index 0000000000..ec15e7e0e8 --- /dev/null +++ b/scripts/check-vi-mock-specifiers.mjs @@ -0,0 +1,449 @@ +#!/usr/bin/env node +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Rejects a `vi.mock` / `vi.doMock` RELATIVE specifier that resolves to no file. + * + * Run: node scripts/check-vi-mock-specifiers.mjs + * node scripts/check-vi-mock-specifiers.mjs --list # every call site found + * node scripts/check-vi-mock-specifiers.mjs --json + * Exit: 0 = OK, 1 = an inert mock, or the population collapsed (see below) + * + * ## The defect (objectui#5646) + * + * A module mock whose specifier names no file on disk does NOT error. Vitest + * registers the mock against a module id that nothing imports, the run proceeds + * with the REAL module everywhere, and the suite passes. + * + * That is the worst available failure direction. The test does not fail, does + * not warn, and does not report fewer assertions -- it reports exactly the same + * green a correct one does, so nothing in the output separates "this stand-in is + * installed" from "this stand-in is inert". + * + * ## The instance this gate was built against + * + * PR #5645 added + * `packages/app-shell/src/layout/__tests__/ChatDock.partialRuntimeConfig.test.tsx`, + * which installs a deliberately partial runtime-config snapshot. The mock was + * written one `..` short: it named the module as a sibling of the `layout/` + * directory, where nothing of that name exists, instead of two levels up in + * `src/` where it does. + * + * The suite passed. It also passed when the code under test was reverted to the + * exact pre-fix shape the suite was written to catch -- a `TypeError` that, with + * the mock actually installed, fails all four cases. Only an ablation leg + * exposed it. + * + * The neighbours in that same file are what made it invisible to a reader: one + * of them steps up ONE level and one steps up TWO, and BOTH are correct, + * because their targets sit at different depths. A mock block is therefore a + * plausible-looking mix of one-dot-dot and two, with nothing in it to say which + * depth belongs to which line. That is also the discriminating case for this + * gate: a resolver that got depth wrong would flag those two, and be deleted by + * the first person it annoyed. + * + * ## The lineage + * + * objectui#4347 was `check-type-check-coverage.mjs` reporting green when a + * `type-check` script chained a tsconfig project that does not exist -- a + * declaration pointing at nothing, reported as a pass. This is the same shape + * one layer down, at the module-mock level. + * + * ## Green at rest, and what follows from that + * + * There were ZERO unresolvable specifiers in this tree when this landed, and the + * expectation is that there stay zero. So on any ordinary day this gate is + * indistinguishable, from its output alone, from a gate that does nothing -- + * which is the very defect it exists to catch, one level up. Two consequences, + * both load-bearing: + * + * 1. **The population must refuse to collapse.** A walk that finds no source + * files, no test files, or no relative specifiers is not a clean tree; it + * is a broken walk, and reporting OK for it would be this card's defect + * wearing the gate's uniform. `FLOORS` below turn each of those into a + * failure. objectui#6195 landed the same discipline in a test as + * `expect(tracked.length).toBeGreaterThan(1000)`. + * 2. **The verdict line carries the census**, not just "OK", so a reader can + * see the population the green was computed over. + * + * `scripts/__tests__/check-vi-mock-specifiers.test.ts` carries the ablation -- + * the historical specifier reconstructed on a real fixture tree, and its two + * correct neighbours -- because on this tree the run itself proves nothing. + * + * ## Scope: RELATIVE specifiers only (decided at dispatch, objectui#5646) + * + * A bare specifier (a package name, an alias) can be misspelled too, but + * resolving one needs the workspace/package map rather than the filesystem -- a + * different check with a different failure mode. Bare specifiers are COUNTED + * here and reported in the census, never judged. Widening this gate into them is + * a separate card, not an extension of this one. + * + * The `vi.mock(import(...))` form is matched by the same pattern, so it is + * covered for free, and no handling was invented for it beyond that. The card + * and the dispatch ruling both recorded its population as ZERO; re-measured on + * this tree it is THREE, all of them in `registration`-style suites and all + * naming a PACKAGE: + * + * packages/plugin-kanban/src/registration.test.tsx:14 + * packages/plugin-calendar/src/registration.test.tsx:31 + * packages/plugin-list/src/__tests__/ListView.sharedGate.test.tsx:60 + * + * What is zero is the intersection that matters here -- the form written with a + * RELATIVE specifier, which is the set this gate judges. The census counts the + * form separately so that stays visible rather than being inferred from a green. + * + * ## Why the walk is not restricted to test-NAMED files + * + * The obvious population is the `*.test.*` / `*.spec.*` naming. It has a hole, + * measured on this tree: THREE files carrying a real call site match no such + * suffix, and TWO of those match no test-file naming convention at all, not even + * a `__tests__/` directory -- + * + * apps/console/dev/__tests__/setup/common-mocks.ts (suffix: no, dir: yes) + * packages/plugin-map/vitest.setup.ts (neither) + * vitest.setup.base.ts (neither) + * + * A setup file is exactly where a repo-wide mock gets written, and a mock helper + * shared by a directory of suites is exactly where one goes unreviewed. So the + * walk takes every tracked JS/TS-family source file and lets the PATTERN decide + * what is a call site. The test-named count is still derived and reported, as a + * census figure and as its own floor, but it does not narrow the scan. + * + * ## Resolution mirrors how this repo really spells specifiers + * + * More than an existence check on the literal path, or the gate reddens on + * hundreds of correct call sites: + * + * - the bare path itself, plus each of `SOURCE_EXTENSIONS`; + * - the `/index.` forms, for a directory specifier; + * - and a trailing `.js` / `.jsx` / `.mjs` / `.cjs` STRIPPED and the whole + * ladder retried on the stem, because `src/` is NodeNext throughout and + * `./x.js` is how a `./x.ts` is named there. + * + * The judgement is `isFile`, never a bare existence test: a specifier naming a + * DIRECTORY is not resolved by that directory existing -- it is resolved by an + * index file inside it, which the ladder covers explicitly. + * + * ## Only text the language would EXECUTE is judged + * + * A mock call has to be real source before it can be an inert mock, and there + * are two ways for matching text not to be. Both are answered by one pass of the + * shared `js-comment-mask.mjs` scanner, which is the repo's single answer to "is + * this span comment, literal, or code": + * + * - **comments are blanked.** A commented-out mock is not executed, so it + * cannot be inert, and flagging one fabricates a finding. This file's own + * header quotes the defect, so without the mask the gate reds on its own + * prose. + * - **a match whose call token is inside a literal is counted, not judged.** + * That is a code SAMPLE, not a call -- see `findCallSites` for the instance + * that made the distinction necessary and why the token, not the specifier, + * is the discriminator. + * + * String CONTENT is deliberately left intact through all of this -- a gate whose + * signal IS a quoted specifier cannot afford to erase quoted text. The + * consequence lands on this gate's own test suite, which has to build call-site + * fixtures without writing a matchable one into its own source; it says so where + * it does it. + */ + +import { execFileSync } from 'node:child_process'; +import { readFileSync, statSync } from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { isEntrypoint } from './invoked-as.mjs'; +import { blank, scanSource } from './js-comment-mask.mjs'; + +/** Extensions the resolver will append to a specifier, in preference order. */ +export const SOURCE_EXTENSIONS = Object.freeze(['.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs']); + +/** Files the walk reads at all. */ +const SOURCE_FILE_RE = /\.[cm]?[jt]sx?$/; + +/** The test-file naming convention, for the census figure and its floor. */ +const TEST_FILE_RE = /(\.(test|spec)\.[cm]?[jt]sx?$)|((^|\/)__tests__\/)/; + +/** Belt-and-braces: git ignores these already, so nothing matches today. */ +const EXCLUDED = /(^|\/)(node_modules|dist|build|\.next|\.turbo|\.wt-[^/]*)\//; + +/** + * A mock call followed by its opening quote, with an optional `import(` between + * the two -- which is the whole of the `vi.mock(import(...))` support the + * dispatch ruling asked for, and it costs nothing. + * + * A specifier may not span a line: a template literal carrying a newline is not + * a static specifier, and neither is one carrying an interpolation. Both are + * counted as `dynamic` and left unjudged rather than guessed at. + */ +export const CALL_RE = /\bvi\s*\.\s*(mock|doMock)\s*\(\s*(import\s*\(\s*)?(['"`])([^'"`\n]*)\3/g; + +/** + * Floors below which a green verdict is a claim about coverage rather than a + * statement about the tree. Set with room -- the point is to catch a walk that + * COLLAPSED (a broken `git ls-files`, a pattern that stopped matching, a filter + * inverted), not to pin today's exact numbers, which move every day. + */ +export const FLOORS = Object.freeze({ + sources: 1000, + testFiles: 1000, + relative: 100, +}); + +/** Every path the resolver would accept for `specifier`, in order. */ +export function candidatesFor(fromDir, specifier) { + const base = resolve(fromDir, specifier); + const stems = [base]; + // NodeNext: `./x.js` is how `./x.ts` is spelled. Strip and retry the ladder. + const stripped = base.replace(/\.(js|jsx|mjs|cjs)$/, ''); + if (stripped !== base) stems.push(stripped); + + const out = [base]; + for (const stem of stems) { + for (const ext of SOURCE_EXTENSIONS) out.push(stem + ext); + for (const ext of SOURCE_EXTENSIONS) out.push(join(stem, `index${ext}`)); + } + return [...new Set(out)]; +} + +function isFile(p) { + try { + return statSync(p).isFile(); + } catch { + return false; + } +} + +/** + * The resolved path for `specifier` as written from `fromDir`, or `null`. + * + * @returns {{ resolved: string | null, tried: string[] }} + */ +export function resolveSpecifier(fromDir, specifier) { + const tried = candidatesFor(fromDir, specifier); + for (const candidate of tried) { + if (isFile(candidate)) return { resolved: candidate, tried }; + } + return { resolved: null, tried }; +} + +/** 1-based line number of `offset` in `source`. */ +function lineOf(source, offset) { + let line = 1; + for (let i = 0; i < offset && i < source.length; i++) if (source[i] === '\n') line++; + return line; +} + +/** + * Every mock call site in one file's source, classified. + * + * `kind` is `relative` (judged here), `bare` (out of scope -- needs the + * workspace map), `dynamic` (an interpolated specifier, which is not a static + * path at all) or `embedded` (see below). + * + * ## `embedded`: the call token is inside a string, so it is a code SAMPLE + * + * The one hit the first run over this tree produced was + * `eslint-rules/no-dynamic-import-in-test-hook.test.js:37`, and it is correct as + * written: the mock call sits inside a TEMPLATE LITERAL, as one of the code + * samples that rule's `RuleTester` lints. Its `'./dep'` names a module that does + * not exist and must not -- an ESLint fixture is source text, never something + * vitest loads, so the specifier has no directory to be relative to and no mock + * to be inert. + * + * The discriminator is exactly the one the language uses: the `vi` TOKEN has to + * be code. `scanSource` flags a literal's content, so a call written in source + * has its token unflagged while its specifier is flagged, and a call quoted + * inside a sample has both flagged. Nothing about the specifier separates them. + * + * These are COUNTED, not silently dropped -- an exclusion nobody can see in the + * census is how a scan narrows itself into vacuity. + * + * A real call written inside an interpolation would be skipped by this too. That + * is not a hole worth closing: vitest hoists mock calls out of MODULE SOURCE, so + * a call assembled inside a template is not one it would ever register either. + */ +export function findCallSites(source) { + const { comment, literal } = scanSource(source); + const masked = blank(source, comment); + const sites = []; + CALL_RE.lastIndex = 0; + let m; + while ((m = CALL_RE.exec(masked)) !== null) { + const specifier = m[4]; + if (literal[m.index]) { + sites.push({ fn: m[1], specifier, kind: 'embedded', viaImport: Boolean(m[2]), line: lineOf(masked, m.index) }); + continue; + } + const kind = specifier.includes('${') + ? 'dynamic' + : specifier === '.' || specifier === '..' || specifier.startsWith('./') || specifier.startsWith('../') + ? 'relative' + : 'bare'; + sites.push({ + fn: m[1], + specifier, + kind, + viaImport: Boolean(m[2]), + line: lineOf(masked, m.index), + }); + } + return sites; +} + +/** The NUL that `git ls-files -z` delimits with, built from its code point. */ +const NUL = String.fromCharCode(0); + +function trackedFiles(root) { + return execFileSync('git', ['ls-files', '-z'], { + cwd: root, + encoding: 'buffer', + maxBuffer: 64 * 1024 * 1024, + }) + .toString('utf8') + .split(NUL) + .filter(Boolean); +} + +/** + * The one scan. `main()`, `--list`, `--json` and the test suite all go through + * here, so the tests exercise the real code path rather than an imitation. + */ +export function scan(root, { files = null, floors = FLOORS } = {}) { + const tracked = files ?? trackedFiles(root); + const sources = tracked.filter((f) => SOURCE_FILE_RE.test(f) && !EXCLUDED.test(f)); + const testFiles = sources.filter((f) => TEST_FILE_RE.test(f)); + + const unresolvable = []; + const sites = []; + const counters = { relative: 0, bare: 0, dynamic: 0, embedded: 0, viaImport: 0, filesWithMocks: 0 }; + + for (const file of sources) { + let source; + try { + source = readFileSync(join(root, file), 'utf8'); + } catch { + continue; // symlink, gitlink, unreadable -- nothing to judge + } + // Cheap pre-filter only. The pattern below is what actually decides. + if (!source.includes('vi')) continue; + const found = findCallSites(source); + if (found.length === 0) continue; + counters.filesWithMocks++; + + const fromDir = dirname(join(root, file)); + for (const site of found) { + counters[site.kind]++; + if (site.viaImport) counters.viaImport++; + const record = { file, ...site }; + if (site.kind === 'relative') { + const { resolved, tried } = resolveSpecifier(fromDir, site.specifier); + record.resolved = resolved; + if (!resolved) unresolvable.push({ ...record, tried }); + } + sites.push(record); + } + } + + const census = { + tracked: tracked.length, + sources: sources.length, + testFiles: testFiles.length, + ...counters, + }; + + // The population, checked for collapse. See "Green at rest" in the header. + const vacuous = []; + for (const [counter, floor] of Object.entries(floors)) { + if (census[counter] < floor) vacuous.push({ counter, value: census[counter], floor }); + } + + return { census, sites, unresolvable, vacuous }; +} + +function repoRoot() { + return resolve(dirname(fileURLToPath(import.meta.url)), '..'); +} + +/** The census, as one line, for the verdict. */ +export function summarise({ census }) { + return ( + `${census.sources} tracked source file(s), ${census.testFiles} test-named; ` + + `${census.filesWithMocks} carry a mock; ` + + `${census.relative} relative specifier(s) resolved, ` + + `${census.bare} bare (out of scope), ${census.dynamic} non-static, ` + + `${census.embedded} embedded in a string literal, ` + + `${census.viaImport} via the import() form` + ); +} + +function main() { + const result = scan(repoRoot()); + const { unresolvable, vacuous } = result; + + if (unresolvable.length === 0 && vacuous.length === 0) { + console.log(`✅ check-vi-mock-specifiers: OK (${summarise(result)}).`); + process.exit(0); + } + + if (unresolvable.length > 0) { + const plural = unresolvable.length === 1 ? 'mock resolves' : 'mocks resolve'; + console.error(`❌ check-vi-mock-specifiers: ${unresolvable.length} ${plural} to no file on disk\n`); + console.error(' Vitest does not error on these. It registers the mock against a module id'); + console.error(' nothing imports, runs the REAL module everywhere, and the suite PASSES --'); + console.error(' identically to a correct one. Each of these is an inert stand-in:\n'); + for (const u of unresolvable) { + console.error(` - ${u.file}:${u.line} -- vi.${u.fn}(${JSON.stringify(u.specifier)})`); + console.error(` tried ${u.tried.length} path(s), first: ${u.tried[0]}`); + } + console.error(` +Fix the specifier, then confirm the mock is actually installed: revert the code +under test to the shape the suite was written to catch and check that the suite +goes RED. A mock whose depth was wrong passes that ablation only once it is +correctly resolved. + +Count from the test file's OWN directory: a suite in \`src/x/__tests__/\` reaches +a sibling of \`x/\` with one step up and a sibling of \`src/\` with two. +Neighbouring mocks in one block legitimately use different depths, so copying the +prefix off the line above is how this gets written wrong. + +Bare specifiers (a package name, an alias) are OUT OF SCOPE here -- resolving +those needs the workspace map, not the filesystem (objectui#5646).`); + } + + if (vacuous.length > 0) { + console.error('\n❌ check-vi-mock-specifiers: the population COLLAPSED -- this run proves nothing\n'); + for (const v of vacuous) { + console.error(` - ${v.counter}: found ${v.value}, floor is ${v.floor}`); + } + console.error(` +A scan that finds nothing reports OK, and reads as coverage. That is the exact +defect this gate exists to catch, one level up, so it is a FAILURE here instead. + +Something upstream of the judgement broke: \`git ls-files\` returned little or +nothing, a filter inverted, or the pattern stopped matching. Fix the walk. If a +floor is genuinely too high because the tree changed shape, move it in \`FLOORS\` +deliberately and say why -- never to make a red run green. + +Census: ${summarise(result)}`); + } + + process.exit(1); +} + +// Run only when invoked directly -- the test suite imports `scan` and friends +// and must not trigger a repo scan (or a `process.exit`) on import. +if (isEntrypoint(import.meta.url)) { + if (process.argv.includes('--json')) { + const result = scan(repoRoot()); + console.log(JSON.stringify({ census: result.census, unresolvable: result.unresolvable, vacuous: result.vacuous }, null, 2)); + } else if (process.argv.includes('--list')) { + const result = scan(repoRoot()); + for (const s of result.sites) { + const mark = s.kind === 'relative' ? (s.resolved ? 'ok ' : 'UNRESOLVED') : s.kind.padEnd(10); + console.log(`${mark} ${s.file}:${s.line} vi.${s.fn}(${JSON.stringify(s.specifier)})`); + } + console.log(`\n${summarise(result)}`); + } else { + main(); + } +} diff --git a/scripts/dependabot-merge-gate.mjs b/scripts/dependabot-merge-gate.mjs index 02bd41835a..5f5de3a605 100644 --- a/scripts/dependabot-merge-gate.mjs +++ b/scripts/dependabot-merge-gate.mjs @@ -127,6 +127,7 @@ import { isEntrypoint } from './invoked-as.mjs'; * doc-snippet-types.yml Doc Snippet Type Check * doc-fence-languages.yml Doc Fence Language Check * pre-install-import-graph.yml Pre-Install Import Graph Check + * vi-mock-specifiers.yml Inert vi.mock Specifier Check * * The four shards are spelled out individually on purpose. A single `Test` * entry, or any pattern match, would be satisfied by whichever shard happened @@ -150,6 +151,7 @@ export const REQUIRED_CONTEXTS = Object.freeze([ 'Doc Snippet Type Check', 'Doc Fence Language Check', 'Pre-Install Import Graph Check', + 'Inert vi.mock Specifier Check', ]); /** From 519ce076bf2e4155c43cb44f55800146ff8cf692 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 00:30:48 +0000 Subject: [PATCH 2/2] fix(scripts): widen scan() JSDoc so callers can override files and floors 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`. --- scripts/check-vi-mock-specifiers.mjs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scripts/check-vi-mock-specifiers.mjs b/scripts/check-vi-mock-specifiers.mjs index ec15e7e0e8..cd2ba64454 100644 --- a/scripts/check-vi-mock-specifiers.mjs +++ b/scripts/check-vi-mock-specifiers.mjs @@ -307,6 +307,12 @@ function trackedFiles(root) { /** * The one scan. `main()`, `--list`, `--json` and the test suite all go through * here, so the tests exercise the real code path rather than an imitation. + * + * @param {string} root Repository root to scan. + * @param {{ files?: string[] | null, floors?: Record }} [options] + * `files` overrides the `git ls-files` walk (fixtures pass their own list); + * `floors` overrides `FLOORS` — pass `{}` to switch the collapse check off for + * a fixture tree, which is legitimately far below every repo floor. */ export function scan(root, { files = null, floors = FLOORS } = {}) { const tracked = files ?? trackedFiles(root);