From 3130430ccb0f4a08f9f2dcd07e8f19dd4ecb3522 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Thu, 5 Nov 2026 16:31:12 -0800 Subject: [PATCH] fix(cli): stop verify exiting 0 when a rule did not run 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. --- .changeset/verify-exits-2-when-not-run.md | 7 + .../proposal.md | 29 ++++ .../specs/cli-rule-validation/spec.md | 124 ++++++++++++++++++ .../2026-11-05-verify-exit-not-run/tasks.md | 14 ++ openspec/specs/cli-rule-validation/spec.md | 19 ++- packages/cli/src/agent/hooks.md | 14 +- packages/cli/src/agent/validate-rule.md | 19 ++- packages/cli/src/commands/verify.ts | 30 ++++- packages/cli/src/schemas/rule-check.ts | 8 +- packages/cli/test/migrate-round-trip.test.ts | 17 ++- .../cli/test/runtime-fixture-runner.test.ts | 13 +- packages/cli/test/verify.test.ts | 74 +++++++++-- 12 files changed, 327 insertions(+), 41 deletions(-) create mode 100644 .changeset/verify-exits-2-when-not-run.md create mode 100644 openspec/changes/archive/2026-11-05-verify-exit-not-run/proposal.md create mode 100644 openspec/changes/archive/2026-11-05-verify-exit-not-run/specs/cli-rule-validation/spec.md create mode 100644 openspec/changes/archive/2026-11-05-verify-exit-not-run/tasks.md diff --git a/.changeset/verify-exits-2-when-not-run.md b/.changeset/verify-exits-2-when-not-run.md new file mode 100644 index 00000000..c4109c46 --- /dev/null +++ b/.changeset/verify-exits-2-when-not-run.md @@ -0,0 +1,7 @@ +--- +"@taskless/cli": patch +--- + +`verify` no longer reports success when a rule did not run. It used to exit 0 and print `ok: true` when a runtime rule's fixtures were skipped for want of `--dangerously-run-scripts`, so CI and agents read a rule nothing exercised as a pass. 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. + +If a script or hook runs a bare `verify` in a project with runtime rules, it now needs to change: pass `--dangerously-run-scripts` if it should run them, or name `.taskless/rules/sg` and `.taskless/rules/vale`. Pre-commit hooks should do the latter, since `--dangerously-run-scripts` never belongs in a hook. diff --git a/openspec/changes/archive/2026-11-05-verify-exit-not-run/proposal.md b/openspec/changes/archive/2026-11-05-verify-exit-not-run/proposal.md new file mode 100644 index 00000000..9b23d046 --- /dev/null +++ b/openspec/changes/archive/2026-11-05-verify-exit-not-run/proposal.md @@ -0,0 +1,29 @@ +## Why + +`verify` on a runtime rule without `--dangerously-run-scripts` skips every fixture and exits 0, with a top-level `ok: true` over a rule reporting `ran: false`. Automation usually reads only the exit code, so a rule nothing exercised looks the same as one that passed. `verify` is how an agent proves a rule before relying on it, so this is a false pass (#612). + +The spec requires this behavior today, on the grounds that no action could turn the failure green. That is not true of `verify`: the flag runs the fixtures. + +## What Changes + +- `verify` exits 2 when no rule failed and at least one requested rule did not run, including when other rules in the run passed. +- A failed rule still exits 1, and outranks a rule that did not run. +- Top-level `ok` under `--json` is `false` whenever the exit code is non-zero. +- The pre-commit recipe names `.taskless/rules/sg` and `.taskless/rules/vale` instead of running a bare `verify`, because a hook never passes the flag and any non-zero exit blocks the commit. +- A script or hook running bare `verify` in a project with runtime rules now sees exit 2 unless it passes the flag or scopes to the static engines. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `cli-rule-validation`: `verify` no longer treats a rule that did not run as success. + +## Impact + +- `packages/cli/src/commands/verify.ts`, `src/schemas/rule-check.ts`, and the `validate-rule` and `hooks` recipes. +- `validate` is unaffected: it never runs fixtures, so it never refuses. +- Delivery shape: **single PR**. The exit code, the recipes that depend on it and the spec change only make sense together, and the diff fits one review. diff --git a/openspec/changes/archive/2026-11-05-verify-exit-not-run/specs/cli-rule-validation/spec.md b/openspec/changes/archive/2026-11-05-verify-exit-not-run/specs/cli-rule-validation/spec.md new file mode 100644 index 00000000..fb2c2681 --- /dev/null +++ b/openspec/changes/archive/2026-11-05-verify-exit-not-run/specs/cli-rule-validation/spec.md @@ -0,0 +1,124 @@ +## MODIFIED Requirements + +### Requirement: Verify runs a rule's fixtures and runs validate first + +`verify` SHALL execute a rule against its fixture cases, laid out and judged as `cli-rule-fixtures` describes for every engine, and SHALL run `validate` first, stopping on a validate failure without running the fixtures. + +Ordering is the point. When a rule is both malformed and under-fixtured, the fixture complaint is the less useful of the two errors and is the one that surfaces first if the checks run in the other order — so the author is told their fixtures are incomplete while the reason the rule could never have run goes unmentioned. + +A rule that populates only one bucket has proved only half of what a rule claims, whatever its engine. An engine SHALL NOT be trusted to report this itself: `ast-grep test` reported an empty `invalid:` bucket as `1 passed; 0 failed` and exited zero, so a rule that had never matched anything was indistinguishable from one that passed. Coverage is therefore counted by the CLI over case directories, never read from an engine's summary. + +A runtime rule's fixtures execute code, so they SHALL run only when `--dangerously-run-scripts` is passed, and under no other mechanism. `verify` SHALL NOT consult the rule service to decide this, and SHALL make no network request in the course of running fixtures. + +That is deliberately stricter than the policy `check` applies, not softer. `check` executes rules as a side effect of scanning a repository, so a blessed signature admits code the user never asked to run and a reconcile stands between the request and the execution. `verify` runs fixtures because the user asked for them, so the verb is the consent and the flag is the confirmation; a rule that `check` would run unflagged on a blessed signature still requires the flag here. Nothing executes under `verify` that would not have executed under the shared policy. + +A run refused for want of the flag SHALL be reported as not run, and SHALL be reported as neither a pass nor a failure: the rule is not defective, but nothing has shown that it works. The refusal SHALL name the flag, and SHALL NOT direct the reader to authenticate — authenticating cannot bless a rule that never left the working tree, so naming it would offer a fix that is not one. + +`verify` SHALL exit 0 only when every requested rule was verified. It SHALL exit 1 when any rule failed, and SHALL exit 2 when no rule failed and at least one rule did not run, whether or not other rules in the run were verified. Under `--json`, the top-level `ok` SHALL be `false` whenever the exit code is non-zero. An exit code is often all a CI step or an agent reads, so a rule nothing exercised SHALL NOT share a code with a rule that passed, and a distinct code keeps it apart from a rule that is broken. + +`verify` SHALL report, per rule, the findings its fixtures produced, as a `findings` array carrying the same finding shape `check --json` prints — `source`, `ruleId`, `severity`, `message`, `file`, `range`, `matchedText` and the optional `note` and `fix` — plus the `bucket` the fixture that produced it belongs to. + +The RENDERED message is the point, and it is what a verdict cannot carry. A rule whose message interpolates its captures can have its slots in the wrong order and still fire on every `fail/` fixture and stay quiet on every `pass/` one, so a boolean verdict reports it as a rule that passed. The only evidence that the message says what its author meant is the message as the engine rendered it, against material the author wrote, which `verify` already has in hand and discards. + +The array SHALL be present on every rule result and SHALL be empty rather than absent when there is nothing to report — including for an engine that does not yet surface its fixture findings, a rule whose validation failed before fixtures ran, and a run the execution policy refused. A consumer SHALL NOT have to distinguish "this rule produced no findings" from "this command does not report findings", because a key that is sometimes absent is one a reader learns to treat as optional, and the reading that follows is that its absence means zero. + +Fail-bucket findings SHALL be reported under `--json` on a passing run. They are the evidence, and a payload that carries the evidence only once the rule is already failing carries it at the one moment it is no longer needed. + +The human rendering SHALL stay a single line per passing rule. Under a FAILING rule it SHALL print the findings that bear on the failure, labelled by bucket, using the same renderer `check` prints findings with, so one finding does not read two ways depending on which command surfaced it. A pass-bucket finding SHALL be printed there: it is a fixture that wrongly fired, and naming the file says only that it happened, while the finding says what matched. + +#### Scenario: A malformed rule reports the malformation, not the fixtures + +- **WHEN** `verify` runs against a rule that is both invalid and missing a fixture bucket +- **THEN** it SHALL report the validation error +- **AND** it SHALL NOT report the fixture coverage as the failure + +#### Scenario: Another rule's config error does not fail this rule's fixtures + +- **WHEN** `verify` runs against one `sg` rule while a different `sg` rule in the project is one ast-grep refuses +- **THEN** the rule under verification SHALL be judged on its own fixtures alone +- **AND** its result SHALL NOT carry the other rule's error + +#### Scenario: Vale fixtures are tested per bucket + +- **WHEN** `verify` runs against a Vale rule +- **THEN** every case directory under `tests/fail/` SHALL produce at least one finding for that rule +- **AND** every case directory under `tests/pass/` SHALL produce none +- **AND** a rule populating only one bucket SHALL be reported as unverified rather than passing + +#### Scenario: ast-grep fixtures are counted per bucket + +- **WHEN** `verify` runs against an ast-grep rule +- **THEN** every case directory under `tests/fail/` SHALL produce at least one finding for that rule +- **AND** every case directory under `tests/pass/` SHALL produce none +- **AND** a rule populating only one bucket SHALL be reported as unverified rather than passing +- **AND** a rule whose buckets are all empty or absent SHALL be reported as unverified rather than passing + +#### Scenario: Runtime fixtures are run per case + +- **WHEN** `verify` runs against a runtime rule with `--dangerously-run-scripts` +- **THEN** each directory under `tests/fail/` SHALL be passed to the check as its `root` and SHALL produce at least one finding +- **AND** each directory under `tests/pass/` SHALL be passed as its `root` and SHALL produce none +- **AND** a rule populating only one bucket SHALL be reported as unverified rather than passing +- **AND** a rule holding no fixture cases at all SHALL be reported as unverified rather than passing + +#### Scenario: A runtime rule without the flag is reported as not run + +- **WHEN** `verify` runs against a runtime rule without `--dangerously-run-scripts` +- **THEN** the rule SHALL NOT be reported as passing +- **AND** the output SHALL say the fixtures did not run and why +- **AND** the output SHALL name `--dangerously-run-scripts` as what would run them +- **AND** the output SHALL NOT direct the reader to authenticate +- **AND** the rule SHALL NOT be counted among the rules verified +- **AND** the command SHALL exit with code 2 +- **AND** under `--json` the top-level `ok` SHALL be `false` + +#### Scenario: A run mixing verified and unrun rules is reported as incomplete + +- **WHEN** `verify` runs over rules where every rule that ran passed and at least one runtime rule did not run +- **THEN** the command SHALL exit with code 2 +- **AND** under `--json` the top-level `ok` SHALL be `false` +- **AND** the output SHALL report the verified rules and the rules that did not run separately + +#### Scenario: A failure outranks a rule that did not run + +- **WHEN** `verify` runs over rules where at least one rule failed and at least one runtime rule did not run +- **THEN** the command SHALL exit with code 1 + +#### Scenario: Verifying a runtime rule reaches no network + +- **WHEN** `verify` runs against a runtime rule, with or without `--dangerously-run-scripts` +- **THEN** the CLI SHALL NOT request a token, resolve an organization, or reconcile +- **AND** the outcome SHALL NOT depend on authentication state, a git remote, or the availability of the rule service + +#### Scenario: A case that never reaches the check is reported as a fixture defect + +- **WHEN** a fixture case produces no narrow matches, so the check is never invoked +- **THEN** the CLI SHALL report that case as a fixture defect naming the case +- **AND** SHALL say the check did not run +- **AND** SHALL do so whether the case is in `pass/` or `fail/`, since a case that never invokes the check is evidence about the fixture rather than about the rule + +#### Scenario: A check that throws is distinguished from one that finds nothing + +- **WHEN** a runtime rule's check raises while running a fixture case +- **THEN** that SHALL be reported as the check failing, not as the case producing no findings + +#### Scenario: Verify reports the findings its fixtures produced + +- **WHEN** `verify --json` runs against a rule whose fixtures produced findings +- **THEN** each rule result SHALL carry a `findings` array +- **AND** each entry SHALL carry the rendered `message`, `file`, `range`, `matchedText` and `severity` that `check --json` reports for the same finding +- **AND** each entry SHALL name the bucket of the fixture that produced it +- **AND** the fail-bucket findings SHALL be reported even when the rule passed + +#### Scenario: The findings array is present and empty rather than absent + +- **WHEN** `verify --json` reports a rule that produced no findings, whose validation failed before its fixtures ran, whose run was refused, or whose engine does not surface fixture findings +- **THEN** the rule result SHALL still carry a `findings` array +- **AND** that array SHALL be empty + +#### Scenario: A failing rule prints the findings that bear on the failure + +- **WHEN** `verify` runs without `--json` and a rule fails because a `pass/` fixture fired +- **THEN** the offending findings SHALL be printed under that rule, labelled by bucket +- **AND** they SHALL be rendered the way `check` renders a finding +- **AND** a rule that passed SHALL still print one line diff --git a/openspec/changes/archive/2026-11-05-verify-exit-not-run/tasks.md b/openspec/changes/archive/2026-11-05-verify-exit-not-run/tasks.md new file mode 100644 index 00000000..c67680e3 --- /dev/null +++ b/openspec/changes/archive/2026-11-05-verify-exit-not-run/tasks.md @@ -0,0 +1,14 @@ +## 1. Exit code + +- [x] 1.1 Exit 2 from `verify` when no rule failed and a rule did not run, and keep exit 1 ahead of it +- [x] 1.2 Report top-level `ok: false` whenever the exit code is non-zero +- [x] 1.3 Update the `rule-check` schema descriptions + +## 2. Recipes + +- [x] 2.1 Document the exit codes in `validate-rule` +- [x] 2.2 Scope the pre-commit recipe in `hooks` to the static engine directories + +## 3. Tests + +- [x] 3.1 Cover a mixed run, a run with nothing verified, a failure beside a refusal, and the static engine paths diff --git a/openspec/specs/cli-rule-validation/spec.md b/openspec/specs/cli-rule-validation/spec.md index 3bff754b..4604d663 100644 --- a/openspec/specs/cli-rule-validation/spec.md +++ b/openspec/specs/cli-rule-validation/spec.md @@ -215,7 +215,9 @@ A runtime rule's fixtures execute code, so they SHALL run only when `--dangerous That is deliberately stricter than the policy `check` applies, not softer. `check` executes rules as a side effect of scanning a repository, so a blessed signature admits code the user never asked to run and a reconcile stands between the request and the execution. `verify` runs fixtures because the user asked for them, so the verb is the consent and the flag is the confirmation; a rule that `check` would run unflagged on a blessed signature still requires the flag here. Nothing executes under `verify` that would not have executed under the shared policy. -A run refused for want of the flag SHALL be reported as not run, and SHALL be reported as neither a pass nor a failure: the rule is not defective, and no action available to its holder would make a failure green. The refusal SHALL name the flag, and SHALL NOT direct the reader to authenticate — authenticating cannot bless a rule that never left the working tree, so naming it would offer a fix that is not one. +A run refused for want of the flag SHALL be reported as not run, and SHALL be reported as neither a pass nor a failure: the rule is not defective, but nothing has shown that it works. The refusal SHALL name the flag, and SHALL NOT direct the reader to authenticate — authenticating cannot bless a rule that never left the working tree, so naming it would offer a fix that is not one. + +`verify` SHALL exit 0 only when every requested rule was verified. It SHALL exit 1 when any rule failed, and SHALL exit 2 when no rule failed and at least one rule did not run, whether or not other rules in the run were verified. Under `--json`, the top-level `ok` SHALL be `false` whenever the exit code is non-zero. An exit code is often all a CI step or an agent reads, so a rule nothing exercised SHALL NOT share a code with a rule that passed, and a distinct code keeps it apart from a rule that is broken. `verify` SHALL report, per rule, the findings its fixtures produced, as a `findings` array carrying the same finding shape `check --json` prints — `source`, `ruleId`, `severity`, `message`, `file`, `range`, `matchedText` and the optional `note` and `fix` — plus the `bucket` the fixture that produced it belongs to. @@ -270,7 +272,20 @@ The human rendering SHALL stay a single line per passing rule. Under a FAILING r - **AND** the output SHALL name `--dangerously-run-scripts` as what would run them - **AND** the output SHALL NOT direct the reader to authenticate - **AND** the rule SHALL NOT be counted among the rules verified -- **AND** the refusal alone SHALL NOT fail the command +- **AND** the command SHALL exit with code 2 +- **AND** under `--json` the top-level `ok` SHALL be `false` + +#### Scenario: A run mixing verified and unrun rules is reported as incomplete + +- **WHEN** `verify` runs over rules where every rule that ran passed and at least one runtime rule did not run +- **THEN** the command SHALL exit with code 2 +- **AND** under `--json` the top-level `ok` SHALL be `false` +- **AND** the output SHALL report the verified rules and the rules that did not run separately + +#### Scenario: A failure outranks a rule that did not run + +- **WHEN** `verify` runs over rules where at least one rule failed and at least one runtime rule did not run +- **THEN** the command SHALL exit with code 1 #### Scenario: Verifying a runtime rule reaches no network diff --git a/packages/cli/src/agent/hooks.md b/packages/cli/src/agent/hooks.md index e94c30b6..6b9ee24e 100644 --- a/packages/cli/src/agent/hooks.md +++ b/packages/cli/src/agent/hooks.md @@ -1,4 +1,4 @@ -# Topic: hooks (CLI v%(CLI_VERSION)s / topic v5) +# Topic: hooks (CLI v%(CLI_VERSION)s / topic v6) %(ONE_COMMAND_PER_CALL)s @@ -151,10 +151,17 @@ and it can affect files that were not staged. In that case run the rule tests and a full, unscoped `check`: ``` -node .taskless/bin/cli.mjs verify +node .taskless/bin/cli.mjs verify .taskless/rules/sg +node .taskless/bin/cli.mjs verify .taskless/rules/vale node .taskless/bin/cli.mjs check ``` +Name the two engine directories rather than running a bare `verify`. +A runtime rule's fixtures run only with `--dangerously-run-scripts`, +which never belongs in a hook, and a bare `verify` exits 2 on any +runtime rule that did not run, which blocks the commit. An engine +directory with no rules passes, so keep both lines either way. + lint-staged can express that in a JavaScript config, where a function task receives the matched files and its returned commands are run as written, with no files appended: @@ -163,7 +170,8 @@ written, with no files appended: export default { "*": "node .taskless/bin/cli.mjs check", ".taskless/rules/**": () => [ - "node .taskless/bin/cli.mjs verify", + "node .taskless/bin/cli.mjs verify .taskless/rules/sg", + "node .taskless/bin/cli.mjs verify .taskless/rules/vale", "node .taskless/bin/cli.mjs check", ], }; diff --git a/packages/cli/src/agent/validate-rule.md b/packages/cli/src/agent/validate-rule.md index 07c17ae9..58d62956 100644 --- a/packages/cli/src/agent/validate-rule.md +++ b/packages/cli/src/agent/validate-rule.md @@ -1,4 +1,4 @@ -# Topic: validate-rule (CLI v%(CLI_VERSION)s / topic v8) +# Topic: validate-rule (CLI v%(CLI_VERSION)s / topic v9) %(ONE_COMMAND_PER_CALL)s @@ -55,11 +55,20 @@ makes no network request and does not reconcile, so a signature the service blessed does not count here. Without the flag, that rule prints `○` rather than a tick, names the reason and the flag, and is reported as `"ran": false` with a `refused` string saying why. It is counted under "did not run" rather than among the -rules verified, and on its own it does not fail the command. +rules verified. -Read `ran` before `ok`. A runtime rule that did not run is not a rule -that passed, and treating the two alike is the defect this reporting -exists to prevent. +`verify` exits 0 only when every rule you asked for was verified: + +| exit | meaning | top-level `ok` | +|------|-------------------------------------------------------|----------------| +| `0` | every rule ran and passed | `true` | +| `1` | at least one rule failed | `false` | +| `2` | nothing failed, but at least one rule did not run | `false` | + +Exit `2` is not a broken rule. Read `ran` on each rule to see which ones +did not run, and rerun them with the flag if you mean to execute them. +A runtime rule that did not run is not a rule that passed, and treating +the two alike is the defect this reporting exists to prevent. ## What a path means diff --git a/packages/cli/src/commands/verify.ts b/packages/cli/src/commands/verify.ts index 76350732..4bfe846e 100644 --- a/packages/cli/src/commands/verify.ts +++ b/packages/cli/src/commands/verify.ts @@ -24,6 +24,14 @@ import { CLIError } from "../util/cli-error"; import { formatText } from "../util/format"; import { markNotice } from "../util/notices"; +/** + * `verify`'s exit code when nothing failed but a requested rule did not run. + * + * Distinct from 1 so a caller can tell "this rule is broken" from "this rule was + * never exercised", which call for different responses. + */ +const NOT_RUN_EXIT_CODE = 2; + /** * The findings a result carries, or none. * @@ -176,11 +184,11 @@ async function runOverPath(options: { } // Three outcomes, not two. A run the execution policy refused is neither a - // pass nor a failure: the rule is not defective, and no action available to - // its holder would make a failure green, so failing it would turn `verify` red - // for every project holding a runtime rule. It is excluded from the failures - // that set the exit code and from the count of rules verified, and gets its - // own marker below. + // pass nor a failure: the rule is not defective, but nothing showed it works. + // It is excluded from the failures and from the count of rules verified, gets + // its own marker below, and has its own exit code, because an exit code is all + // a CI step or an agent reads, and a rule nothing exercised must not read the + // same as a rule that passed. const isRefused = (result: RuleValidation | RuleVerification): boolean => "refused" in result && result.refused !== undefined; const refused = results.filter((result) => isRefused(result)); @@ -191,7 +199,9 @@ async function runOverPath(options: { console.log( JSON.stringify( ruleCheckOutputSchema.parse({ - ok: failed.length === 0, + // Agrees with the exit code. "1 rule did not run" under `ok: true` is + // the same false pass in a second place. + ok: failed.length === 0 && refused.length === 0, rules: results, }) ) @@ -241,7 +251,13 @@ async function runOverPath(options: { ); } - if (failed.length > 0) process.exitCode = 1; + // A failure outranks a rule that did not run: it is the one a caller has to + // act on, and the per-rule results still say what was refused. + if (failed.length > 0) { + process.exitCode = 1; + } else if (refused.length > 0) { + process.exitCode = NOT_RUN_EXIT_CODE; + } } const ruleTargetArguments = { diff --git a/packages/cli/src/schemas/rule-check.ts b/packages/cli/src/schemas/rule-check.ts index ae4733cf..e63dc34a 100644 --- a/packages/cli/src/schemas/rule-check.ts +++ b/packages/cli/src/schemas/rule-check.ts @@ -124,7 +124,7 @@ const ruleResultSchema = z.object({ .string() .optional() .describe( - "`verify` only: the execution policy declined to run the rule's fixtures, and why. Neither a pass nor a failure, excluded from the rules verified, and never on its own a reason for a non-zero exit" + "`verify` only: the execution policy declined to run the rule's fixtures, and why. Neither a pass nor a failure, and excluded from the rules verified. Sets top-level `ok` to false and, when no rule failed, makes the command exit 2" ), notices: z .array(z.string()) @@ -147,6 +147,10 @@ const ruleResultSchema = z.object({ /** Output schema for `taskless validate --json` and `taskless verify --json`. */ export const outputSchema = z.object({ - ok: z.boolean(), + ok: z + .boolean() + .describe( + "Whether every requested rule passed. False when a rule failed, and for `verify` when a rule did not run: `ok` is false whenever the exit code is non-zero" + ), rules: z.array(ruleResultSchema).describe("Per-rule results"), }); diff --git a/packages/cli/test/migrate-round-trip.test.ts b/packages/cli/test/migrate-round-trip.test.ts index 213c8a7a..92079b46 100644 --- a/packages/cli/test/migrate-round-trip.test.ts +++ b/packages/cli/test/migrate-round-trip.test.ts @@ -94,7 +94,13 @@ interface CheckOutput { interface RuleReport { ok: boolean; - rules: { engine: string; ruleId: string; ok: boolean; errors: string[] }[]; + rules: { + engine: string; + ruleId: string; + ok: boolean; + ran?: boolean; + errors: string[]; + }[]; } beforeEach(async () => { @@ -191,9 +197,14 @@ withVale("a version-4 project upgraded through 0005", () => { it("still runs the tests that moved with their rules", async () => { await seedVersion4Project(); + // The seed holds a runtime rule, which does not run without the flag, so + // the run as a whole is incomplete (exit 2) rather than failed (exit 1). const result = await runCli(["verify", "-d", cwd, "--json"]); - expect(parseJson(result.stdout).ok).toBe(true); - expect(result.exitCode).toBe(0); + expect(result.exitCode).toBe(2); + const { rules } = parseJson(result.stdout); + const statics = rules.filter((rule) => rule.engine !== "runtime"); + expect(statics.length).toBeGreaterThan(0); + expect(statics.every((rule) => rule.ran === true && rule.ok)).toBe(true); }); // Runtime capture bytes determine the server-side reconciliation hash, so a diff --git a/packages/cli/test/runtime-fixture-runner.test.ts b/packages/cli/test/runtime-fixture-runner.test.ts index 7a72fd4a..e318305b 100644 --- a/packages/cli/test/runtime-fixture-runner.test.ts +++ b/packages/cli/test/runtime-fixture-runner.test.ts @@ -194,8 +194,7 @@ afterEach(async () => { * 5.1 — the defect itself. * * With no blessed signature and no escape flag the fixtures cannot run. That is - * not a failure of the rule, but it is emphatically not a pass, and the two - * assertions below are the ones that were false before the runner existed. + * not a failure of the rule, but it is emphatically not a pass. */ describe("a runtime rule whose fixtures did not run", () => { it("is not reported as passing, and prints no tick", async () => { @@ -206,8 +205,9 @@ describe("a runtime rule whose fixtures did not run", () => { const { stdout, exitCode } = await testRule(); expect(stdout).not.toContain(`✓ runtime/${RULE}`); - // A refusal by policy is not the rule's fault, so it must not fail either. - expect(exitCode).toBe(0); + // A refusal by policy is not the rule's fault, but nothing was exercised: + // exit 2 keeps it apart from both a pass and a failure. + expect(exitCode).toBe(2); expect(stdout).not.toContain("1 rule(s) verified"); }); @@ -250,8 +250,9 @@ describe("a runtime rule whose fixtures did not run", () => { expect(rule?.engine).toBe("runtime"); expect(rule?.ran).toBe(false); expect(rule?.ok).not.toBe(true); - // The envelope still succeeds: nothing here is a defect in the rule. - expect(report.ok).toBe(true); + // The envelope agrees with the exit code. #612: this read `ok: true` over + // a run that exercised nothing. + expect(report.ok).toBe(false); }); }); diff --git a/packages/cli/test/verify.test.ts b/packages/cli/test/verify.test.ts index e871d090..8768fbfa 100644 --- a/packages/cli/test/verify.test.ts +++ b/packages/cli/test/verify.test.ts @@ -650,8 +650,8 @@ describe("getSchemaPayload", () => { * taskless/cli#284. * * `tested = results.length - refused.length`, and both the human summary and - * the `--json` `ok` field are derived from `failed`, which itself excludes - * anything `isRefused`. Every other test in this repo runs a single rule per + * the exit code are derived from `failed`, which itself excludes anything + * `isRefused`. Every other test in this repo runs a single rule per * invocation, which cannot exercise either arithmetic operation: with one * rule, `results.length - refused.length` and `results.length` agree, and a * refused rule "leaking" into `failed` is invisible unless something else in @@ -771,10 +771,8 @@ describe("verify: the verified/failed/refused split on a mixed run (#284)", () = refused?: string; }[]; }; - // The `ok` field is derived from `failed`, and must stay false because of - // the one genuine failure — a mutation that let the refusal leak into - // `failed` would not be visible here (it is already false), which is why - // the second test below isolates that case with no genuine failure. + // `ok` is false for the failure and the refusal alike here, which is why + // the second test below isolates the refusal with no genuine failure. expect(report.ok).toBe(false); expect(jsonResult.exitCode).not.toBe(0); expect(report.rules.find((rule) => rule.ruleId === "pass-rule")?.ok).toBe( @@ -794,10 +792,11 @@ describe("verify: the verified/failed/refused split on a mixed run (#284)", () = expect(humanResult.stdout).toContain("1 rule(s) did not run."); }); - it("does not let a refused rule count as a failure", async () => { - // No genuine failure at all: one passing rule and one refused rule. If a - // refused rule were counted in `failed`, `ok` would read false and the - // human summary would read "of 1 rule(s) failed" instead of "tested". + it("does not let a refused rule count as a pass or a failure", async () => { + // No genuine failure at all: one passing rule and one refused rule. Exit 2 + // and `ok: false` say the run was incomplete. Exit 1 or an "of 1 rule(s) + // failed" summary would mean the refusal leaked into `failed`; exit 0 is + // #612, a rule nothing exercised reading as a pass. await sgRule("pass-rule", { passes: true }); await refusedRuntimeRule("rt-rule"); @@ -808,15 +807,64 @@ describe("verify: the verified/failed/refused split on a mixed run (#284)", () = "--json", ]); const report = JSON.parse(jsonResult.stdout) as { ok: boolean }; - expect(report.ok).toBe(true); - expect(jsonResult.exitCode).toBe(0); + expect(report.ok).toBe(false); + expect(jsonResult.exitCode).toBe(2); const humanResult = await runVerifyMixedRunCli(["verify", "-d", cwd]); - expect(humanResult.exitCode).toBe(0); + expect(humanResult.exitCode).toBe(2); expect(humanResult.stdout).toContain("1 rule(s) verified."); expect(humanResult.stdout).toContain("1 rule(s) did not run."); expect(humanResult.stdout).not.toContain("failed"); }); + + it("exits 2 when nothing was verified at all", async () => { + await refusedRuntimeRule("rt-rule"); + + const jsonResult = await runVerifyMixedRunCli([ + "verify", + join(".taskless", "rules", "runtime", "rt-rule"), + "-d", + cwd, + "--json", + ]); + expect(jsonResult.exitCode).toBe(2); + const report = JSON.parse(jsonResult.stdout) as { + ok: boolean; + rules: { ok: boolean; ran?: boolean }[]; + }; + expect(report.ok).toBe(false); + expect(report.rules[0]?.ran).toBe(false); + + const humanResult = await runVerifyMixedRunCli(["verify", "-d", cwd]); + expect(humanResult.exitCode).toBe(2); + expect(humanResult.stdout).toContain("0 rule(s) verified."); + expect(humanResult.stdout).toContain("1 rule(s) did not run."); + }); + + it("lets a failure outrank a rule that did not run", async () => { + await sgRule("fail-rule", { passes: false }); + await refusedRuntimeRule("rt-rule"); + + const { exitCode } = await runVerifyMixedRunCli(["verify", "-d", cwd]); + expect(exitCode).toBe(1); + }); + + // The pre-commit recipe depends on this: it names the static engine + // directories so that holding a runtime rule cannot block a commit. + it("passes on the static engine paths in a project holding a runtime rule", async () => { + await sgRule("pass-rule", { passes: true }); + await refusedRuntimeRule("rt-rule"); + + for (const engine of ["sg", "vale"]) { + const { exitCode } = await runVerifyMixedRunCli([ + "verify", + join(".taskless", "rules", engine), + "-d", + cwd, + ]); + expect(exitCode, engine).toBe(0); + } + }); }); /**