feat(lint): enforce ARCH-025 loop shapes with no-loop-test-cases

.archgate/adrs/ARCH-025-idiomatic-bun-test-parametrization-and-matchers.md

@@ -19,7 +19,7 @@ Both patterns compile, pass `oxlint`, and pass `bun test` — they are not caugh
 **Alternatives considered:**
 
 - **Add these as more Do's/Don'ts to ARCH-005** — Rejected: ARCH-005's own Compliance section already documents that its Do's and Don'ts section exceeds the `archgate review-context` briefing budget; appending more would push more of it out of every future briefing, including the parts already there.
-- **Enforce via a new oxlint plugin immediately** — Deferred, not rejected: this repo already has a precedent (`lint/expect-expect.ts`, `lint/no-bare-env-restore.ts`) for exactly this kind of AST-detectable test-shape rule, and it is the right enforcement layer for both patterns (syntax-detectable, not behavioral). Building it is out of scope for the session that produced this ADR, which only fixed existing instances; the rule is recorded as future work in Compliance and Enforcement below.
+- **Enforce via an oxlint plugin** — Adopted for the loop pattern: this repo's precedent (`lint/expect-expect.ts`, `lint/no-bare-env-restore.ts`) is exactly this kind of AST-detectable test-shape rule, and `lint/no-loop-test-cases.ts` follows it. The boolean-collapse pattern is syntax-detectable too and is a candidate for the same layer (Compliance and Enforcement below).
 - **Leave undocumented, rely on code review alone** — Rejected: the instances this ADR responds to were already written by reviewed, merged PRs; undocumented convention did not prevent the drift.
 
 ## Decision
@@ -41,20 +41,20 @@ A loop that only builds shared setup data or fixtures for a single overall asser
 
 ### Do
 
-- **DO** use `test.each(cases)("<title with %s/%p/%d>", (...args) => {...})` for the same check against independent inputs — each case becomes its own named, independently-reportable test.
-- **DO** use `describe.each(cases)(...)` when a case needs its own group of related tests, not just one assertion.
+- **DO** use `test.each(cases)("<title with %s/%p/%d>", (...args) => {...})` for the same check against independent inputs — each case becomes its own named test.
+- **DO** use `describe.each(cases)(...)` when a case needs a group of related tests, not one assertion.
 - **DO** pass array rows (`[a, b, expected]`) for positional destructuring, object rows (`{a, b, expected}`) for named fields (title with `$field`).
 - **DO** assert derived comparisons directly on the values: `expect(actual).toBe(expected)` / `.toEqual(expected)`.
-- **DO** match the matcher to the check: `.toContain()`/`.toMatch()` for substrings, `.toBeInstanceOf()` for type checks (including `Array.isArray` replacements), `.toHaveLength()` for counts, `.find(...)` + `.toBeDefined()`/`.toBeUndefined()` for "does any item satisfy X."
-- **DO** keep loop→`test.each()` conversions 1:1 — every assertion that ran per-iteration in the original loop must still run per-case; never drop or merge assertions.
+- **DO** match the matcher to the check: `.toContain()`/`.toMatch()` for substrings, `.toBeInstanceOf()` for type checks, `.toHaveLength()` for counts, `.find(...)` + `.toBeDefined()`/`.toBeUndefined()` for "does any item satisfy X."
+- **DO** keep loop→`test.each()` conversions 1:1 — every per-iteration assertion still runs per case; never drop or merge assertions.
 
 ### Don't
 
-- **DON'T** register a `test()`/`it()` or call `expect()` once per iteration inside a `for`/`.forEach` loop — use `test.each()`/`describe.each()` instead.
+- **DON'T** register `test()`/`it()`/`describe()` inside a loop, loop `expect()` over rows that exist at registration time (an array literal or `const` array), or let a loop carry a test's only `expect()` — use `test.each()`, or assert the collection outside the loop.
 - **DON'T** write `expect(a === b).toBe(true)` or `.toBe(false)` — assert `expect(a).toBe(b)` / `.not.toBe(b)` directly.
-- **DON'T** collapse `.some(predicate)`/`.every(predicate)` into `.toBe(true)` — it reports `false !== true` with no indication of which element (or none) satisfied the predicate. Use `.find(predicate)` + `.toBeDefined()`, a per-item `expect()` loop, or a matcher over the mapped values (e.g. `expect(arr.map(x => x.message)).toContain(...)`).
+- **DON'T** collapse `.some(predicate)`/`.every(predicate)` into `.toBe(true)` — it reports `false !== true` without saying which element satisfied the predicate. Use `.find(predicate)` + `.toBeDefined()`, a per-item `expect()` loop guarded by an assertion on the whole collection, or a matcher over the mapped values (e.g. `expect(arr.map(x => x.message)).toContain(...)`).
 - **DON'T** write `expect(Array.isArray(x)).toBe(true)` — use `expect(x).toBeInstanceOf(Array)`.
-- **DON'T** precompute a boolean just to assert it (`const equal = JSON.stringify(a) === JSON.stringify(b); expect(equal).toBe(true);`) — assert on the values directly with `.toEqual()`/`.toBe()` so a failure shows the actual diff.
+- **DON'T** precompute a boolean just to assert it (`const equal = a === b; expect(equal).toBe(true);`) — assert on the values directly with `.toEqual()`/`.toBe()` so a failure shows the actual diff.
 
 ## Implementation Pattern
 
@@ -111,22 +111,23 @@ expect(violations.some((v) => v.message.includes('"fetch" global'))).toBe(true);
 
 ### Risks
 
-- **No automated enforcement yet**: neither pattern is currently caught by `oxlint` or `archgate check`, so new instances can still be introduced and merged undetected, the same way the instances motivating this ADR were.
-  - **Mitigation:** tracked as future work in Compliance and Enforcement below (a candidate `oxlint` plugin following the existing `bun-test/expect-expect` precedent); until it exists, code review is the only enforcement layer, so reviewers MUST check new/changed test files against the Do's and Don'ts above.
+- **Boolean collapse has no automated enforcement**: `expect(<comparison>).toBe(true)` is caught by neither `oxlint` nor `archgate check`, so new instances can be introduced and merged undetected.
+  - **Mitigation:** a `test-cases/no-boolean-collapse` rule beside `no-loop-test-cases` is the intended layer (Compliance and Enforcement below); until it exists, reviewers MUST check new/changed test files against Manual Enforcement item 2.
 - **Existing violations elsewhere in the codebase may resurface**: this ADR's Context reflects a repository-wide audit at a point in time; files not covered by that audit, or added afterward, may still contain either anti-pattern.
   - **Mitigation:** treat any test file touched for an unrelated change as an opportunity to fix nearby instances of either pattern, consistent with ARCH-005's existing test-hygiene expectations.
 
 ## Compliance and Enforcement
 
 ### Automated Enforcement
 
-- **Not yet implemented.** No `oxlint` rule or `archgate` rule currently detects either anti-pattern. A future `oxlint` plugin (e.g. `bun-test/no-loop-test-cases` and `bun-test/no-boolean-collapse`, registered via `jsPlugins` in `.oxlintrc.json` alongside the existing `bun-test/expect-expect` and `test-isolation/no-bare-env-restore` plugins) is the intended enforcement layer once written, matching this repo's existing pattern of using oxlint for syntax-detectable test-shape rules and reserving `archgate` rules for structural/governance checks. This ADR's `rules: false` reflects that no companion `.rules.ts` exists.
+- **`test-cases/no-loop-test-cases`** (`lint/no-loop-test-cases.ts`, registered in `.oxlintrc.json` `jsPlugins` and enabled as an error for `tests/**/*.test.ts`) reports three shapes: a `test()`/`it()`/`describe()` registered inside a `for`/`for...in`/`for...of`/`while`/`do...while` body or an iteration callback (`.forEach`, `.map`, `.flatMap`, `.filter`, `.some`, `.every`); an `expect()` inside a `for...of` or iteration callback over rows that exist at registration time (a non-empty array literal, or a `const NAME = [...]` binding in module, `describe`, or test scope, including `as const`/`satisfies` forms); and a loop that carries a test's only `expect()` calls. A runtime-derived loop of per-item `expect()`s with an assertion on the collection outside the loop is not reported. Plugin names are unique within `jsPlugins`, so this plugin is `test-cases`.
+- **Boolean collapse is unenforced.** No rule detects `expect(<comparison>).toBe(true)`; a `test-cases/no-boolean-collapse` rule in the same plugin is the intended layer. `rules: false` reflects that no companion `.rules.ts` exists: both patterns are syntax-detectable, which this repo enforces with oxlint plugins, reserving `archgate` rules for structural checks.
 
 ### Manual Enforcement
 
 Code reviewers MUST verify, for any new or changed file under `tests/**/*.test.ts`:
 
-1. No `for`/`.forEach` loop registers a `test()`/`it()`/`describe()` call, and no `for`/`.forEach` loop inside a single test calls `expect()` once per logically independent case — either shape MUST be `test.each()`/`describe.each()` instead.
+1. Beyond what `test-cases/no-loop-test-cases` reports, no loop inside a test calls `expect()` once per logically independent case over rows that exist at registration time but are not an array literal or `const` array (for example rows returned by a helper call) — that shape MUST be `test.each()`/`describe.each()` too.
 2. No `expect(<comparison>).toBe(true)` or `.toBe(false)` where `<comparison>` is itself a boolean expression (`===`, `.some()`, `.every()`, `Array.isArray()`, `.includes()`) — the assertion MUST target the underlying values with a specific matcher.
 3. A `test.each()`/`describe.each()` conversion preserves every assertion that ran in the original loop — none dropped, none merged into a single case.
 

.claude/agent-memory/archgate-developer/MEMORY.md

@@ -22,9 +22,11 @@ Exceptions: minor follow-up tweaks after validation already passed, and non-code
 - **This repo sets `strict: true`** in `.archgate/config.json`, so the `[suppression]`, `[briefing]`, and `[adr]` advisories block `check`, `review-context`, and `adr sync` alike, with no flag passed (ARCH-026). The practical bite: an over-budget ADR section fails the build rather than warning it — see [[adr-briefing-budget]].
 - **Commit with `--signoff`** — the DCO check rejects commits without `Signed-off-by`.
 - **This repo is PUBLIC** — no private sibling-repo internals, no Claude session links in PRs or commits.
+- **An ADR restates its enforcement status in up to five places** — the Context alternatives, a Risk with its mitigation, Compliance → Automated Enforcement, Compliance → Manual Enforcement, and the Don'ts. After changing one, grep the whole file for the old claim ("not yet", "future work", the old rule name) before calling the edit done; `check` validates only the section budget, not consistency.
+- **A hook-created worktree is cut from whatever branch the primary checkout has out, not from `origin/main`.** Run `git log --oneline origin/main..HEAD` before the first commit; when foreign commits show, confirm they touch none of your files while HEAD is still the hook-created branch (`git diff --name-only origin/main HEAD -- <your files>` must print nothing), then commit the work and `git rebase --onto origin/main HEAD~1` so the PR carries only its own commit.
 - **Commit before fire-testing a rule or guard.** The loop (mutate → confirm the check fails → restore) restores with `git checkout <file>`, which discards all uncommitted work in that file, including the fix under test.
 - **Fire-test with `ARCHGATE_TELEMETRY=0` whenever the guard under test throws.** A local `bun run cli` run reports `environment: production` to Sentry, so a guard reaching the exit-2 boundary files a real issue against the product from a deliberately broken tree. A guard reporting an ADR violation is safe; one that throws is not.
-- **Fire-test a guard in BOTH directions** — that it blocks the bad case AND still permits the legitimate one. A green suite proves only that the gate closes, not that it isn't over-rejecting. Fire-testing also exposes branches that can never fire: when an earlier pipeline stage aborts on the same condition, the later branch is dead governance (`check` regenerates `rules.d.ts` before any rule runs, so a throw from `generateRulesDts()` preempts any rule testing the same thing). Delete the unreachable branch and name the real enforcement point.
+- **Fire-test a guard in BOTH directions** — that it blocks the bad case AND still permits the legitimate one. A regression test for a timing or scheduling bug gets the same treatment: run it against the pre-fix module (`git show origin/main:<path>` copied into place, restored from a saved copy) and make the victim do what the bug needs — a rule that never yields is never charged by a shared clock, so a no-op victim passes on the buggy code. A green suite proves only that the gate closes, not that it isn't over-rejecting. Fire-testing also exposes branches that can never fire: when an earlier pipeline stage aborts on the same condition, the later branch is dead governance (`check` regenerates `rules.d.ts` before any rule runs, so a throw from `generateRulesDts()` preempts any rule testing the same thing). Delete the unreachable branch and name the real enforcement point.
 - **Confirm a probe or fire-test fixture holds the bytes you meant before believing the result.** A `bun -e` body inside shell single quotes is still parsed as a JS string literal, so an escape written for the file collapses one level on the way in and the injected "violation" lands as ordinary well-formed text — a false negative that looks like a defect in the rule. A quoted `<<'EOF'` heredoc through the Bash tool is not reliably literal either: a backslash-heavy regex probe run that way reported no match for a pattern that matches fine. Write any escape-sensitive probe to a real file with the Write tool instead of inlining it in a shell command, build such fixtures from `String.fromCodePoint` (oxlint's `unicorn/prefer-code-point` rejects `fromCharCode`), and print the file back (`cat -A`) first. The same applies to WHERE a mutation lands: a first-occurrence substitution of `});` in a test file hits the `beforeEach`/`afterEach` closings before any `test(...)`, so a rule that ignores hooks reports nothing and the fire-test reads as a defect in the rule. Target the line numbers of the construct under test and show `git diff -U0` before running the check.
 - **A Sentry issue whose `install_path` is a local worktree is an artifact of that worktree, not a user report.** Confirm provenance from `install_path`, `is_ci`, and the timestamp — then still ask whether the error was classified correctly, because reaching Sentry at all claims archgate has a bug rather than the caller.
 - **`--update-snapshots` is never on its own the fix for a failing snapshot.** `tests/helpers/__snapshots__/rules-shim.test.ts.snap` is the entire `rules.d.ts` a governed project receives, and its diff is the review artifact — read every hunk and confirm it follows from an intended `src/formats/rules.ts` edit before regenerating. Deleting the file routes around nothing: Bun fails a missing snapshot whenever `CI` is set, and passes it locally.

.oxlintrc.json

@@ -7,6 +7,7 @@
     "./lint/no-bare-fetch.ts",
     "./lint/no-shared-module-mock.ts",
     "./lint/no-lowered-test-timeout.ts",
+    "./lint/no-loop-test-cases.ts",
     "./.archgate/lint/oxlint.ts"
   ],
   "options": { "typeAware": true },
@@ -141,7 +142,8 @@
         "bun-test/expect-expect": "error",
         "test-mocking/no-shared-module-mock": "error",
         "test-isolation/no-bare-env-restore": "error",
-        "test-timeout/no-lowered-test-timeout": "error"
+        "test-timeout/no-lowered-test-timeout": "error",
+        "test-cases/no-loop-test-cases": "error"
       }
     },
     {