test(cli): cover the tested/failed/refused split on a mixed verify/test run

[thecodedrift]

Stack (root → tip):

• test(cli): cover PlatformBinaryResolution.source and asMatchMode's refusal #304
  • test(cli): fingerprint the reference shape, cover the wizard's reload banner #305
    • test(cli): cover restore.ts HTTP branches and the repair-path purge notice #306
      • ➡️ test(cli): cover the tested/failed/refused split on a mixed verify/test run #307 (you are here)

Summary

packages/cli/src/commands/verify.ts:127-176 computes tested = results.length - refused.length, and derives both the human summary and the --json ok field from failed, which excludes anything isRefused. Every existing test in the suite runs exactly one rule per invocation, so neither arithmetic operation was ever exercised: with one rule, results.length and results.length - refused.length agree, and a refused rule "leaking" into failed is invisible unless something else in the run is a genuine, non-refused failure to compare it against.

Adds two tests to packages/cli/test/verify.test.ts that spawn the built CLI's test command (not verify — refused is only ever set by testOneRule on a runtime rule denied --dangerously-run-scripts; verify never produces a refusal, so it cannot exercise this split at all).

• "keeps tested, failed, and refused as three independent counts" — one passing sg rule, one failing sg rule, one refused runtime rule. results.length is 3, tested must read 2, failed must read 1. Asserts the per-rule ok/refused fields via --json, plus the human summary text "1 of 2 rule(s) failed. 1 rule(s) did not run.".
• "does not let a refused rule count as a failure" — one passing rule plus one refused rule, no genuine failure. Asserts --json ok: true and exit code 0, plus the human summary "1 rule(s) tested. 1 rule(s) did not run." with no "failed" anywhere in the output.

Mutation-check table

Each test was verified to fail against the corresponding regression, then the source was reverted (confirmed via git diff showing no change) and both tests passed again.

Test	Mutation	Caught?
"keeps tested, failed, and refused as three independent counts"	tested = results.length (drop the - refused.length)	Yes — human summary read "1 of 3 rule(s) failed." instead of the expected "1 of 2 rule(s) failed."
"keeps tested, failed, and refused as three independent counts"	failed = results.filter((r) => !r.ok) (drop the !isRefused(result) exclusion)	Yes — human summary read "2 of 2 rule(s) failed." instead of "1 of 2 rule(s) failed."
"does not let a refused rule count as a failure"	tested = results.length	Yes — human summary read "2 rule(s) tested." instead of "1 rule(s) tested."
"does not let a refused rule count as a failure"	failed = results.filter((r) => !r.ok)	Yes — --json ok read false instead of the expected true

No production code was changed; git diff on packages/cli/src/commands/verify.ts is empty on this branch.

Test plan

• NODE_OPTIONS= pnpm test — 82 files, 1327 tests, all green (fresh run, --no-file-parallelism)
• NODE_OPTIONS= pnpm typecheck — clean
• NODE_OPTIONS= pnpm lint — clean (eslint + taskless check)
• Mutation-checked both new tests against both described regressions (see table above)

No changeset: this PR ships no user-visible behaviour, only test coverage.

Refs #284

[thecodedrift]

@claude /review

[claude]

Claude finished @thecodedrift's task in 4m 59s —— View job

---

Review

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contained no reviewThreads, no reviews, and no top-level comments besides the trigger (@claude /review) itself. This is the first review of this PR, so the whole diff was assessed fresh (nothing to classify as already-addressed or still-open).

Todo list

• Read .prior-review.json (empty — first review)
• Read the diff (gh pr diff — single file, packages/cli/test/verify.test.ts, +185/-1)
• Read the production code the tests exercise: packages/cli/src/commands/verify.ts, src/rules/inspect.ts, src/rules/engines.ts, src/rules/layout.ts, src/rules/runtime/discover.ts, src/rules/runtime/fixtures.ts, src/rules/assemble.ts, src/schemas/verify-test.ts
• Cross-check the new fixtures (sg rule + runtime capture shape) against what verifyOneRule/testOneRule/assessCaptureRule/assembleSgConfig actually require
• Check for duplicate/overlapping test coverage elsewhere in the suite
• Post findings

Assessment

This PR adds no production code (git diff on verify.ts is empty, confirmed) — only two new integration tests in packages/cli/test/verify.test.ts that spawn the built CLI to cover the tested/failed/refused arithmetic in runOverPath (packages/cli/src/commands/verify.ts:127-176).

I independently verified the new fixtures against the current production code paths, not just the PR's own mutation-check table:

• The sg rule/test-file layout (.taskless/rules/sg/<id>/<id>.yml, .taskless/rules/sg/<id>/.tests/<id>-test.yml) matches ruleFilePath/ruleTestsDirectory (src/rules/engines.ts) and the ${ruleId}-...-test.yml discovery filter in src/rules/verify.ts.
• The runtime rule (captures/logs.yml + check.ts, no .tests/) is sufficient: verifyOneRule's runtime branch only checks check.ts presence, stray modules, and capture-directory validity (assessCaptureRule in src/rules/runtime/discover.ts) — it never touches .tests/. testOneRule then refuses before ever calling discoverRuntimeRules/readRuntimeFixtures, since --dangerously-run-scripts is never passed. So the missing fixture directories are correctly irrelevant to this test.
• assembleSgConfig (src/rules/assemble.ts) points ruleDirs at .taskless/rules/sg only, so the runtime capture's logs-abc12345 id (which is itself a valid-looking ast-grep rule shape) is never picked up by sg test — no cross-engine interference.
• The sgRule helper's inverted "valid fixture that actually fires" trick for the failing rule is real: the valid: bucket case is deliberately code that matches the pattern, which is what makes ast-grep test (and thus taskless test) report a genuine failure rather than a refusal — matches the human-summary math (failed=1, tested=2, refused=1) traced through runOverPath.
• JSON assertions match outputSchema (src/schemas/verify-test.ts): ok, rules[].ok, rules[].refused all line up.
• language: "TypeScript" (canonical casing) and language: "typescript" (fine for the capture, which isn't canonicalized) are both used correctly per src/rules/capabilities.ts.

I did not find any correctness bugs in the new tests, and no production code is touched. One non-blocking organizational note posted as an inline comment: the new describe block brings a second CLI-spawn harness into a file that was previously unit-level only (verifyRule/getSchemaPayload via direct import), when packages/cli/test/verify-test-commands.test.ts already exists for spawning verify/test through the built CLI and has a near-identical runCli shape. This is consistent with the rest of the suite's existing per-file binPath/runCli convention (per support/spawn-cli.ts's own docstring), so it's a "worth a look," not a blocker.

No changeset is correctly omitted here — this ships no user-visible behavior.

[thecodedrift]

No blocker, and the summary reads clean. The one inline note about a second CLI-spawn harness is with the author as a low-priority call.

Incidentally this review is a useful data point for #303: our own bucketing filed this entirely positive summary as high, on content patterns alone. That is the crude half of the LOGAF classifier doing its job badly, and it is why the review-in-progress work reports a separate count rather than leaning on needs_attention.

— AI Coding Agent