Skip to content

Ways: encode 'a green check is a claim' — new Guards way + amendments to testing, mocking, groundtruth #359

Description

@aaronsb

Summary

A single session on aaronsb/google-workspace-mcp (a Jest→Vitest migration, branch next) produced eight distinct defects that all share one signature:

A check reported success while measuring the wrong thing.

Not one announced itself as a failure. Three rounds of adversarial code review were needed to drain them, and each round of fixes introduced a fresh instance of the same pattern — including one committed by me in the very commit where I was fixing the previous ones.

The existing corpus gets close but doesn't cover it. freshness/groundtruth already says "treat design docs / ADRs / specs as claims to verify" — but it is scoped to prose vs code drift. Every failure below came from a different axis: the instrument lied. A green test run, a passing CI, a clean npm audit, a successful npm pack are themselves claims about the system, and they are exactly the claims nobody verifies, because verifying them is what they were supposed to do.

This issue proposes one new way and three amendments, each defended below with a claim and reproducible evidence.


Claim 1 — A passing test run is a claim, not evidence. Compare counts to a baseline.

Evidence. Bumping sanitize-html caused 15 test suites to fail to parse. Jest reported:

Test Suites: 15 failed, 23 passed, 38 total
Tests:       456 passed, 456 total     <-- zero failures

222 tests silently stopped running and not one test was marked failed, because a suite that fails to load contributes no test results. The Tests: line — the line a human reads — was 100% green.

This was caught only by diffing against a known baseline (main = 678 tests). Nothing in the tooling surfaced it, and CI would have merged it.

Why the current code/testing way doesn't catch this. It covers what to assert and how to structure tests. It has nothing about trusting the report. The failure isn't in any test; it's in the summary.

Proposed amendment to softwaredev/code/testing/testing.md — new section:

## The Report Is a Claim

A suite that fails to *load* contributes zero failing tests. `456 passed, 0 failed`
is compatible with a fifth of your suite never executing.

- Know your test count. A drop is a regression even when everything is green.
- Diff against a baseline (the merge base) before trusting a green run on a
  branch that touched the toolchain, config, or dependencies.
- Make suite-load failures fail the build — assert on `numFailedTestSuites`, not
  just failing tests.
- Coverage tools measure files that were *loaded*. A file that never loaded is
  not 0% covered; it is absent.

Claim 2 — A guard you have never seen fail is not a guard.

This is the proposed new way, and it is the load-bearing one. Two independent instances:

Evidence 2a — the guard that never ran. I removed ts-jest (which type-checked every test file on every run), wrote a tsconfig.test.json to replace it, and stated in the commit message that "tsconfig.test.json type-checks them so the guard ts-jest used to provide is kept."

I never checked whether anything invoked it. Injecting one deliberate type error into a test file:

Gate Caught it?
make typecheck passed
make check (self-described "CI gate") passed
npm test passed
npm run build passed
npm run lint passed
npm run type-check caught it — and nothing ran it

The guard existed. It was correct. It was wired to nothing. A comment and a commit message asserted otherwise.

Evidence 2b — the guard that was load-bearing by luck. A shared test helper registered vi.mock() for the executor. Vitest hoists vi.mock per-file, so the mock only applied because every consuming test happened to import the helper before the module under test. Proven by swapping two import lines:

# before fix — reorder two imports:
FAIL src/__tests__/server/handlers/drive.test.ts  (6 failed)
# after fix (vi.mock moved into the test file) — same reorder:
Tests  10 passed (10)

An organize-imports autofix would have silently disarmed it. Worse, the helper exported execute as MockedFunction<typeof execute> — a cast the type system cannot keep: a test that forgot its vi.mock would get the real executor behind a lying type, and shell out to the real gws binary against live Google APIs.

The move that works. Every check must be falsification-tested: deliberately break the thing it guards, watch it fail, restore. The three guards added in response now each ship with a probe:

  • type error in a test → make check fails ✔
  • test file outside every gate → make check fails ✔ (before: npm test reported "36 passed" and ignored it entirely)
  • test imports the mock helper without vi.mock → throws at import, naming the fix ✔

Proposed new way: softwaredev/code/testing/guards/guards.md

---
description: proving that a check actually fires — falsification-testing guards, gates, CI jobs, lint rules and assertions before trusting them
vocabulary: guard gate check assertion ci job lint rule enforce invariant does it actually catch prove the check fires green build passing pipeline canary probe
pattern: (does|will) (it|this|ci|the (check|gate|test|lint)) (actually |really )?(catch|fail|fire)|prove the (guard|check|gate)|(guard|gate|check) (that )?(never|doesn.t) (fire|fail)|passing (for the )?wrong reason|green (but|and) (wrong|broken)
scope: agent, subagent
refire: 0.2
---
<!-- epistemic: heuristic -->
# Guards Way

A guard you have never seen fail is not a guard. It is a claim that a guard exists.

## Prove it fires

When you add or rely on a check — a CI job, a lint rule, a type-check, an
assertion, a schema validation — **break the thing it guards and watch it fail.**
Then restore. If you cannot make it fail, you have not verified it; you have
assumed it.

    inject the defect  ->  run the gate  ->  it MUST fail  ->  revert  ->  it MUST pass

Both halves matter. A check that always fails is as useless as one that never does.

## Wired ≠ written

A check that exists but is invoked by nothing is worse than no check, because it
buys false confidence. Ask, separately:

1. Does the check detect the defect?  (run it directly)
2. **Does anything run the check?**   (grep the CI workflow, the Makefile, the
   pre-commit hook — do not assume the script name implies a caller)

The second question is the one that gets skipped. A `type-check` script nothing
invokes, a test directory no glob collects, a lint config with no CI step: each
looks like coverage in the repo and provides none.

## An invariant in a comment is not an invariant

If you write "every test runs somewhere", "this list stays in sync", "callers must
hold the lock" — either **enforce it in a check** or admit it is a convention.
Prose cannot fail a build. When the invariant is worth stating, it is usually
cheap to assert:

- "every test file is run by some gate" -> a script that diffs the file list
  against the gate globs and exits non-zero
- "these two lists stay in sync" -> a test that compares them
- "this cast is safe" -> a runtime assertion that throws when it isn't

## Allowlists and denylists fail in opposite directions

- **Denylist** ("everything except X") -> new things are *auto-enrolled*. A new
  network-touching test silently joins the fast, offline gate.
- **Allowlist** ("only A, B, C") -> new things are *orphaned*. A new test outside
  the list runs in no gate at all, reports nothing, and CI stays green.

Neither is safe alone. Pick the one whose failure mode you can *detect*, then add
the check that detects it.

## See Also

- code/testing(softwaredev) — the test report is itself a claim
- freshness/groundtruth(softwaredev) — docs are claims to verify; so is tool output

(Placement note: I've put this under code/testing/ because that's where it will match most prompts, but it generalizes past tests — npm audit, engines, npm pack and CI config all produced instances below. If you'd rather it sit as a peer of groundtruth, or under delivery/, say so and I'll re-cut it.)


Claim 3 — Module-mock registration cannot live in a shared helper.

Evidence. As in 2b: vi.mock / jest.mock hoist per-file. A mock registered in an imported helper applies only if that helper is imported before the module under test — i.e. by accident of line order. This pattern was pre-existing on main under Jest (jest.mock sat in the same helper; Babel's hoist is also per-file), so it is not a Vitest quirk — it is a general property of hoisted module mocks that reads as safe and isn't.

Proposed amendment to softwaredev/code/testing/mocking/mocking.md — new section under Mock Hygiene:

## Register module mocks in the test file

`vi.mock` / `jest.mock` are hoisted **within the file that contains them**. A mock
registered in a shared helper therefore takes effect only when the test happens to
import that helper *before* the module under test — an import reorder, or an
organize-imports autofix, silently disarms it.

- Register the mock in **each test file**. Share the *fixtures* from a helper, not
  the registration.
- A helper that exports `dep as MockedFunction<typeof dep>` is making a promise the
  type system cannot keep. If the importing test forgot its `vi.mock`, that cast
  hands back the **real** dependency wearing a mock's type — and the test may quietly
  hit the network. Assert it at import:

      if (!vi.isMockFunction(dep)) throw new Error('...not mocked; add vi.mock in the test file')

Claim 4 — groundtruth should extend "claims to verify" from prose to tool output.

Evidence. The most expensive defect of the session was a production crash that every check passed.

sanitize-html@2.17.6 pulls a pure-ESM htmlparser2@12. I reasoned: "production is unaffected — this package is "type": "module"." That reasoning is about the wrong module. sanitize-html is itself CommonJS and does the require():

$ head -1 node_modules/sanitize-html/index.js
const htmlparser = require('htmlparser2');

$ node --no-experimental-require-module -e "require('sanitize-html')"
Error [ERR_REQUIRE_ESM]: require() of ES Module .../htmlparser2/dist/index.js
  from .../sanitize-html/index.js not supported.

Every Node 18 user would have crashed at startup. It passed a full prototype, a green 691-test suite, a green CI, and npm audit = 0 vulnerabilities — because the dev box was Node 26 and CI pinned node-version: '20' (resolving to a current 20.x). Nothing in the repo tested the floor the README advertised. The instruments were all green and all measuring a Node version no user was on.

Two more from the same axis:

  • npm pack --dry-run showed the published tarball shipping 120 compiled test files, each starting import ... from 'vitest' — a devDependency consumers never install. npm test, npm run build and npm run lint were all green.
  • An ADR Evidence table, explicitly headed "Measured, not asserted", contained a row I had asserted and not measured (devDeps 9 → 6; actually 8 → 6).

Proposed amendment to softwaredev/freshness/groundtruth/groundtruth.md — extend the "claims to verify" frame:

## Tool output is a claim too

The way's discipline — *derive ground truth from executable artifacts; treat prose as
claims* — applies to your **instruments**, not just to docs. A green suite, a passing
CI, a clean `audit`, a successful `pack` are assertions *about* the system, produced by
a tool with its own scope and blind spots. Ask what each one actually measured:

- A green CI measures the environment CI ran in. Pin drift, an unpinned matrix, or a
  dev box newer than the advertised floor means "green" describes a machine no user has.
- `audit` sees declared dependencies. It does not see whether a CJS package can `require()`
  an ESM one on your minimum supported runtime.
- A passing test run measures the tests that *loaded*.
- Your own verification command is an instrument. When it reports something surprising,
  suspect it before you suspect the system.

Evidence for that last line — and the recursive part. While checking whether any test file was orphaned, my probe reported 36 files run by no gate. Alarming, specific, and entirely false: the shell is zsh, which does not word-split unquoted $UNIT, so six paths were passed as one argument and vitest matched nothing.

$ npx vitest run $UNIT          # zsh: ONE arg, not six
No test files found, exiting with code 1
$ npm test                       # sh: splits correctly
Test Files  36 passed (36)

Had I trusted my own instrument, I would have "fixed" a working allowlist. The audit tool produced a confident, success-shaped, wrong answer — which is the thesis of this issue, arriving unbidden, in the middle of proving the thesis.


Counter-arguments considered

  • "This is just 'test your tests'." Partly — but Claim 2's sharp edge is "does anything invoke it", which no amount of test-quality discipline surfaces. The type-check script was correct, complete, and dead.
  • "groundtruth already covers it." It covers prose vs code. Every failure here was tool output vs reality, and the tools were the things you'd normally use to check prose. Hence an amendment there plus a distinct way, rather than folding it in.
  • "Too situational — it's really about Jest/Vitest." The Jest specifics are the examples, not the content. The suite-load blind spot exists in pytest and go test; allowlist-vs-denylist orphaning is CI-agnostic; "nothing invokes the script" is universal. The one Vitest-specific item (mock hoisting) is proposed as a mocking amendment, where it belongs — and it existed identically under Jest.
  • "Adding a way per incident bloats the corpus." Fair, and it's why this is 1 new way + 3 amendments rather than 4 new ways. If the corpus can only take one thing, take the Guards way — the other three are refinements of ways that already exist.

Provenance

All evidence from aaronsb/google-workspace-mcp, branch next (ADR-101, Jest→Vitest migration).
Commits: 90b6043 (migration), ff8e77d (review 1 fixes), 2a9d206 (review 2 fixes), 2978d8d (review 3 fixes — the guards).
Related: google-workspace-mcp#135, #136, #139.
Found by three rounds of adversarial multi-agent code review; each round's fixes introduced a fresh instance of the same pattern, which is itself the argument for encoding it as a way rather than as a lesson learned.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:waysWays CLI, matching, steering layerenhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions