Skip to content

fix(cli): stop verify exiting 0 when a rule did not run - #615

Merged
theCodeDrift merged 1 commit into
mainfrom
fix/verify-exit-not-run
Nov 6, 2026
Merged

theCodeDrift merged 1 commit into
mainfrom
fix/verify-exit-not-run

Conversation

@theCodeDrift

Copy link
Copy Markdown
Member

taskless verify exited 0 with a top-level ok: true when a runtime rule's fixtures were skipped for want of --dangerously-run-scripts. Anything reading only the exit code saw a pass for a rule nothing exercised.

Exit Meaning Top-level ok
0 Every requested rule ran and passed true
1 At least one rule failed, even if others did not run false
2 Nothing failed, but at least one rule did not run false

A run where some rules pass and a runtime rule is skipped exits 2, because exit 0 would keep the false pass for any repo holding both kinds of rule.

Pre-commit recipe. The hooks recipe ran a bare verify and forbids the flag, and any non-zero exit blocks the commit. It now names .taskless/rules/sg and .taskless/rules/vale, which never reach a runtime rule. Hooks already installed from the old recipe are not rewritten, so the changeset says what to change.

Spec. cli-rule-validation required the old behavior. The delta restates the requirement in full, and all existing scenarios survived the archive, with two added. The change is archived on this PR.

Fixes #612

verify exited 0 with a top-level ok: true when a runtime rule's fixtures
were skipped for want of --dangerously-run-scripts, so anything reading
the exit code saw a pass for a rule nothing exercised.

It now exits 2 and reports ok: false when nothing failed but a rule did
not run, and still exits 1 when a rule failed. The pre-commit recipe
names the static engine directories, because a hook never passes the
flag and any non-zero exit blocks the commit.
@theCodeDrift

Copy link
Copy Markdown
Member Author

@claude /review

@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @theCodeDrift's task in 1m 5s —— View job


Review: fix(cli): stop verify exiting 0 when a rule did not run

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contained no reviews and no review threads for this PR (only the @claude /review trigger comment itself), so this is the first review of this PR — every finding below is classified [New].

  • Read prior review data
  • Fetch PR diff
  • Review verify.ts logic
  • Review schema changes
  • Review spec/changeset docs
  • Review test coverage
  • Post final review

Summary

This is a tight, well-scoped fix. I traced the full chain and didn't find a correctness issue:

  • packages/cli/src/commands/verify.ts: the new NOT_RUN_EXIT_CODE = 2 branch is only reached via else if after the failed.length > 0 check, so a failure correctly outranks a refusal (exit 1 wins over exit 2), matching the "A failure outranks a rule that did not run" spec scenario. The --json ok computation (failed.length === 0 && refused.length === 0) agrees with the exit-code precedence.
  • The "no rules found under <target>" early-return path (verify.ts:162-179) still reports ok: true / exit 0 unconditionally — this is correct and matches the hooks.md claim that "an engine directory with no rules passes," which I confirmed via resolveRulePath/listRuleIds in resolve-path.ts: an empty engine directory resolves to zero rules, not a refusal, so the pre-commit recipe's two verify .taskless/rules/<engine> lines are safe to run unconditionally even when one engine has no rules yet.
  • Schema (rule-check.ts): refused and top-level ok descriptions were updated to state the exit-code/ok relationship; consistent with the implementation.
  • Docs (hooks.md, validate-rule.md): topic versions were bumped (v5→v6, v8→v9) alongside the content change, and the new exit-code table in validate-rule.md matches the one in the PR description and the spec. The hooks recipe change (naming .taskless/rules/sg and .taskless/rules/vale instead of a bare verify) is exercised by a dedicated new test (verify.test.ts: "passes on the static engine paths in a project holding a runtime rule").
  • Spec: the archived full-text spec (openspec/changes/archive/.../specs/cli-rule-validation/spec.md) and the live delta applied to openspec/specs/cli-rule-validation/spec.md are consistent word-for-word in the modified requirement section, and all new scenarios (mixed run, failure-outranks-refusal, all-refused) have matching test coverage in verify.test.ts and runtime-fixture-runner.test.ts.
  • Tests: migrate-round-trip.test.ts was updated correctly — the seeded v4 project holds a runtime rule, so the expectation correctly moved from exit 0/ok: true to exit 2, while asserting the static-engine rules still ran and passed individually.

No correctness, security, or test-adequacy issues found. One minor, non-blocking observation:

  • packages/cli/src/commands/verify.ts:258-259: refused.length > 0 is checked only when failed.length === 0, which is correct per the precedence rule, but relies on isRefused/!result.ok partition being mutually exclusive and exhaustive across all three states (verified/failed/refused). That invariant held under every case I traced (and is well covered by the new mixed-run tests), so this is just a note for future changes touching that partition rather than a defect today.

@theCodeDrift

Copy link
Copy Markdown
Member Author

Re: @github-actions — "Claude finished @theCodeDrift's task in 1m 5s…"
#615 (comment)

Read. No code change: the note about the verified, failed and refused partition is an observation for future edits, and the review found no defect today.

— AI Coding Agent

@theCodeDrift
theCodeDrift merged commit 226e590 into main Nov 6, 2026
8 checks passed
@theCodeDrift
theCodeDrift deleted the fix/verify-exit-not-run branch November 6, 2026 00:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

verify exits 0 when no runtime fixtures ran

1 participant