From 593fafdda485f8cd4d8d42bd6fe5750a2c946672 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 2 Sep 2026 12:59:29 -0700 Subject: [PATCH 1/8] feat(runtime): read a rule's fixture cases from .tests/pass and .tests/fail Group 1 of the fixture runner: the reader, with nothing executing yet. A case is a DIRECTORY, and its path is the root the harness will be handed. That follows from `executeRuntimeRule(root, rule)` already taking a root rather than from a preference: a case directory is the same argument with a smaller tree behind it. Two guards carry the weight, both taken from the Vale reader because the mistakes they prevent are the same ones. A bucket that cannot be read is an error, never an empty bucket. Swallowing an `EACCES` on `pass/` would yield nothing while `fail/` still had cases, so a two-sided rule would look one-sided and could report a pass having never checked its pass side. A loose file is refused by name rather than skipped. The check is given a root and reads beneath it, so a bare file has no root to be; ignoring it would leave an author with a fixture they wrote, that never ran, and that nothing mentioned. Both proved rather than asserted: reverting each guard fails its own test and leaves the other six green. --- .../changes/runtime-fixture-runner/tasks.md | 6 +- packages/cli/src/rules/runtime/fixtures.ts | 134 ++++++++++++++++++ packages/cli/test/runtime-fixtures.test.ts | 124 ++++++++++++++++ 3 files changed, 261 insertions(+), 3 deletions(-) create mode 100644 packages/cli/src/rules/runtime/fixtures.ts create mode 100644 packages/cli/test/runtime-fixtures.test.ts diff --git a/openspec/changes/runtime-fixture-runner/tasks.md b/openspec/changes/runtime-fixture-runner/tasks.md index 1d519425..8f950d7f 100644 --- a/openspec/changes/runtime-fixture-runner/tasks.md +++ b/openspec/changes/runtime-fixture-runner/tasks.md @@ -7,9 +7,9 @@ permanently unverified. ## 1. Read the fixtures -- [ ] 1.1 Enumerate `.tests/pass/` and `.tests/fail/` one level deep, requiring each entry to be a directory (D3). A loose file names its own path in the error rather than being skipped -- [ ] 1.2 Read the two buckets independently and rethrow anything that is not a missing directory, so an unreadable bucket cannot present as an empty one (D5) -- [ ] 1.3 Classify coverage as `both` / `pass-only` / `fail-only` / `none`, and let only `both` reach a pass (D4) +- [x] 1.1 Enumerate `.tests/pass/` and `.tests/fail/` one level deep, requiring each entry to be a directory (D3). A loose file names its own path in the error rather than being skipped +- [x] 1.2 Read the two buckets independently and rethrow anything that is not a missing directory, so an unreadable bucket cannot present as an empty one (D5) +- [x] 1.3 Classify coverage as `both` / `pass-only` / `fail-only` / `none`, and let only `both` reach a pass (D4) ## 2. Run them diff --git a/packages/cli/src/rules/runtime/fixtures.ts b/packages/cli/src/rules/runtime/fixtures.ts new file mode 100644 index 00000000..6d1a4e65 --- /dev/null +++ b/packages/cli/src/rules/runtime/fixtures.ts @@ -0,0 +1,134 @@ +import type { Dirent } from "node:fs"; +import { readdir } from "node:fs/promises"; +import { join } from "node:path"; + +import { isMissingDirectory } from "../errno"; +import { ruleTestsDirectory } from "../engines"; + +/** The two buckets a fixture case can live in. */ +export type FixtureBucket = "pass" | "fail"; + +/** + * One fixture case: a DIRECTORY, whose path is the `root` the harness hands + * the check. + * + * Vale's buckets hold documents; runtime's hold directories. That is not a + * stylistic difference. `executeRuntimeRule(root, rule)` already takes a root + * and `check` already passes the repository root, so a case directory is the + * same argument with a smaller tree behind it. A runtime rule exists because + * its evidence spans more than one file, so a layout allowing one file per + * case could not express the rules this tier is for. + */ +export interface RuntimeFixtureCase { + bucket: FixtureBucket; + /** The case directory's own name, for messages. */ + name: string; + /** Absolute path, and the `root` the check will be given. */ + root: string; +} + +/** + * Which buckets a rule actually populated. + * + * Four cases rather than a boolean, and the reasoning is `ValeFixtureCoverage`'s + * because the mistake it corrects is the same one: `"none"` is an unwritten + * rule, while `"pass-only"`/`"fail-only"` is a half-written one, which is the + * more misleading of the two. A rule with only `fail/` cases has shown it fires + * and not that it stays quiet; only `both` can reach a pass. + */ +export type RuntimeFixtureCoverage = + | "both" + | "pass-only" + | "fail-only" + | "none"; + +/** Classify a rule's buckets by how many cases each held. */ +function coverageOf( + passCount: number, + failCount: number +): RuntimeFixtureCoverage { + if (passCount > 0 && failCount > 0) return "both"; + if (passCount > 0) return "pass-only"; + if (failCount > 0) return "fail-only"; + return "none"; +} + +/** + * A missing directory is an empty bucket; anything else rethrows. + * + * The buckets are read independently and this discrimination is why. Swallowing + * an `EACCES` on `pass/` would yield `[]` while `fail/` still had cases, so the + * rule would not look one-sided and could report a pass having never checked + * the pass side at all. A permissions problem must not read as "no pass + * fixtures were written". + */ +async function directoryEntries(directory: string): Promise { + try { + return await readdir(directory, { withFileTypes: true }); + } catch (error) { + if (isMissingDirectory(error)) return []; + throw error; + } +} + +/** + * The cases in one bucket, one level deep. + * + * Every entry must be a directory, and a loose file is an error naming its own + * path rather than an entry quietly skipped. The check is handed a root and + * reads whatever it needs beneath it, so a file has no root to be: there is no + * sensible reading of `pass/example.ts` that the harness could act on, and + * ignoring it would leave an author with a fixture they wrote, that never ran, + * and that nothing mentioned. + */ +async function bucketCases( + cwd: string, + ruleId: string, + bucket: FixtureBucket +): Promise { + const directory = join(ruleTestsDirectory(cwd, "runtime", ruleId), bucket); + const entries = await directoryEntries(directory); + + const loose = entries.find((entry) => !entry.isDirectory()); + if (loose !== undefined) { + throw new Error( + `A runtime fixture case is a directory: ${join(directory, loose.name)} ` + + `is not one. The check is given the case directory as its root and ` + + `reads the files it needs beneath it, so a bare file in ${bucket}/ ` + + `has no root to be and would never run.` + ); + } + + return entries.map((entry) => ({ + bucket, + name: entry.name, + root: join(directory, entry.name), + })); +} + +/** Every fixture case a runtime rule holds, with what the buckets cover. */ +export interface RuntimeFixtures { + cases: RuntimeFixtureCase[]; + coverage: RuntimeFixtureCoverage; +} + +/** + * Read a runtime rule's fixture cases from `.tests/pass/` and `.tests/fail/`. + * + * Reads the buckets independently rather than walking `.tests/` once, so an + * unreadable bucket surfaces as itself instead of as an absence. + */ +export async function readRuntimeFixtures( + cwd: string, + ruleId: string +): Promise { + const [pass, fail] = await Promise.all([ + bucketCases(cwd, ruleId, "pass"), + bucketCases(cwd, ruleId, "fail"), + ]); + + return { + cases: [...fail, ...pass], + coverage: coverageOf(pass.length, fail.length), + }; +} diff --git a/packages/cli/test/runtime-fixtures.test.ts b/packages/cli/test/runtime-fixtures.test.ts new file mode 100644 index 00000000..9edda9a3 --- /dev/null +++ b/packages/cli/test/runtime-fixtures.test.ts @@ -0,0 +1,124 @@ +import { chmod, mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { readRuntimeFixtures } from "../src/rules/runtime/fixtures"; + +/** + * Reading a runtime rule's fixture cases. + * + * The buckets are read independently and every entry must be a directory. Both + * are load-bearing rather than tidy: a bucket that reads as empty when it is + * unreadable makes a two-sided rule look one-sided, and a case silently skipped + * is a fixture an author wrote that never ran and that nothing mentioned. + */ + +const RULE = "no-eval"; + +async function caseDirectory( + cwd: string, + bucket: "pass" | "fail", + name: string +): Promise { + const directory = join( + cwd, + ".taskless", + "rules", + "runtime", + RULE, + ".tests", + bucket, + name + ); + await mkdir(directory, { recursive: true }); + await writeFile(join(directory, "sample.ts"), "const x = 1;\n", "utf8"); + return directory; +} + +describe("reading runtime fixture cases", () => { + let cwd: string; + + beforeEach(async () => { + cwd = await mkdtemp(join(tmpdir(), "tskl-rt-fixtures-")); + }); + + afterEach(async () => { + await rm(cwd, { recursive: true, force: true }); + }); + + it("reads both buckets and reports the case directories as roots", async () => { + const failRoot = await caseDirectory(cwd, "fail", "flags-it"); + const passRoot = await caseDirectory(cwd, "pass", "leaves-it"); + + const { cases, coverage } = await readRuntimeFixtures(cwd, RULE); + + expect(coverage).toBe("both"); + // The root IS the case directory: that is what the harness is handed. + expect(cases).toEqual( + expect.arrayContaining([ + { bucket: "fail", name: "flags-it", root: failRoot }, + { bucket: "pass", name: "leaves-it", root: passRoot }, + ]) + ); + }); + + it.each([ + ["both", ["fail", "pass"]], + ["fail-only", ["fail"]], + ["pass-only", ["pass"]], + ["none", []], + ] as const)("classifies coverage as %s", async (expected, buckets) => { + for (const bucket of buckets) { + await caseDirectory(cwd, bucket, "case-1"); + } + + const { coverage } = await readRuntimeFixtures(cwd, RULE); + expect(coverage).toBe(expected); + }); + + it("treats a missing bucket as empty rather than as a failure", async () => { + // Only `fail/` exists. A rule mid-authoring is not a broken read. + await caseDirectory(cwd, "fail", "case-1"); + + const { cases, coverage } = await readRuntimeFixtures(cwd, RULE); + expect(coverage).toBe("fail-only"); + expect(cases).toHaveLength(1); + }); + + it("refuses a loose file, naming it, rather than skipping it", async () => { + await caseDirectory(cwd, "fail", "case-1"); + await writeFile( + join(cwd, ".taskless/rules/runtime", RULE, ".tests/fail/loose.ts"), + "const x = 1;\n", + "utf8" + ); + + // Skipping it would leave an author with a fixture that never ran and that + // nothing mentioned, which is the failure this tier keeps producing. + await expect(readRuntimeFixtures(cwd, RULE)).rejects.toThrow(/loose\.ts/); + }); + + it("does not read an unreadable bucket as an empty one", async () => { + // The discrimination that matters. If this swallowed the permission error, + // the rule below would report `fail-only` — a one-sided rule — when in + // truth its pass side was simply unreadable. + await caseDirectory(cwd, "fail", "case-1"); + const passBucket = join( + cwd, + ".taskless/rules/runtime", + RULE, + ".tests/pass" + ); + await mkdir(passBucket, { recursive: true }); + await chmod(passBucket, 0o000); + + try { + await expect(readRuntimeFixtures(cwd, RULE)).rejects.toThrow(); + } finally { + // Restore before cleanup, or `rm` cannot remove it either. + await chmod(passBucket, 0o755); + } + }); +}); From 4054f4e8ea258508505360cece5ec21e84c2d5e3 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 2 Sep 2026 13:21:01 -0700 Subject: [PATCH 2/8] feat(runtime): run a rule's fixture cases through the existing executor A fixture case is a case DIRECTORY handed to `executeRuntimeRule` as its `root`, which is the same argument `check` already passes with the repository root. So the runner is a caller of the executor rather than a sibling of it: the loop and the pass/fail assertion are new, the execution is not. D8 is the part that needed a change in the harness. `executeRuntimeRule` gates on the narrow and returns `[]` without invoking `check.ts`, so a case whose narrow matches nothing is indistinguishable downstream from a check that ran and found nothing. Under the pass/fail rules that would fail a `fail/` case and blame the check for a fixture that never reached it, and would pass a `pass/` case that proves only that the narrow did not match. `executeRuntimeRuleDetailed` is therefore the single execution path, adding `invoked` and `failure` beside the findings, and `executeRuntimeRule` is a thin projection of it. One narrow, one invocation, one source of truth for "did it run", and the scan's call site is untouched. A case producing no narrow matches is reported as a fixture defect in BOTH buckets, naming the case and saying the check never ran. A check that throws is reported as the check failing, since both arrive as zero findings and only one is the rule's fault. --- .../changes/runtime-fixture-runner/tasks.md | 10 +- packages/cli/src/rules/runtime/harness.ts | 86 ++++++++-- .../cli/src/rules/runtime/run-fixtures.ts | 161 ++++++++++++++++++ 3 files changed, 237 insertions(+), 20 deletions(-) create mode 100644 packages/cli/src/rules/runtime/run-fixtures.ts diff --git a/openspec/changes/runtime-fixture-runner/tasks.md b/openspec/changes/runtime-fixture-runner/tasks.md index 8f950d7f..1bc01b1b 100644 --- a/openspec/changes/runtime-fixture-runner/tasks.md +++ b/openspec/changes/runtime-fixture-runner/tasks.md @@ -13,11 +13,11 @@ permanently unverified. ## 2. Run them -- [ ] 2.1 Execute the rule's `check.ts` once per case, with the case directory as the harness `root` (D3), through the existing `invoke.ts` rather than a second invocation path -- [ ] 2.2 Require every `fail/` case to produce at least one finding, and every `pass/` case to produce none. Name the cases that broke either direction, the way the Vale runner names `missingFailures` and `unexpectedFindings` -- [ ] 2.3 Treat a check that throws as a distinct outcome from a check that returned no findings. Both are zero findings downstream, and only one of them is the rule's fault -- [ ] 2.4 Distinguish a case that never reached the check from one where the check found nothing (D8). `executeRuntimeRule` gates on the narrow and returns `[]` without invoking `check.ts`, so the runner needs the invocation signal rather than only the findings -- [ ] 2.5 Report a case producing no narrow matches as a fixture defect in BOTH buckets, naming the case and saying the check never ran. A `pass/` case that never invokes the check proves nothing about the check staying quiet +- [x] 2.1 Execute the rule's `check.ts` once per case, with the case directory as the harness `root` (D3), through the existing `invoke.ts` rather than a second invocation path +- [x] 2.2 Require every `fail/` case to produce at least one finding, and every `pass/` case to produce none. Name the cases that broke either direction, the way the Vale runner names `missingFailures` and `unexpectedFindings` +- [x] 2.3 Treat a check that throws as a distinct outcome from a check that returned no findings. Both are zero findings downstream, and only one of them is the rule's fault +- [x] 2.4 Distinguish a case that never reached the check from one where the check found nothing (D8). `executeRuntimeRule` gates on the narrow and returns `[]` without invoking `check.ts`, so the runner needs the invocation signal rather than only the findings +- [x] 2.5 Report a case producing no narrow matches as a fixture defect in BOTH buckets, naming the case and saying the check never ran. A `pass/` case that never invokes the check proves nothing about the check staying quiet ## 3. Gate it exactly as `check` does diff --git a/packages/cli/src/rules/runtime/harness.ts b/packages/cli/src/rules/runtime/harness.ts index f6ffd056..68971902 100644 --- a/packages/cli/src/rules/runtime/harness.ts +++ b/packages/cli/src/rules/runtime/harness.ts @@ -58,30 +58,62 @@ function harnessErrorResult( } /** - * Execute one runtime rule: run the ast-grep narrow, gate on matches (zero - * matches ⇒ `check.ts` is never invoked), invoke `check.ts`, and map its - * findings onto `CheckResult`. A harness failure is isolated to a single + * One run of a rule against one root, with the facts a caller cannot recover + * from the findings alone. + * + * `findings` is exactly what {@link executeRuntimeRule} returns, so the scan + * path is unchanged. The other two fields carry what an empty array hides: + * + * - `invoked` says whether `check.ts` ran at all. The narrow gates it, so a + * root with no matches produces `[]` having never reached the check, which is + * indistinguishable downstream from a check that ran and found nothing. A + * scan does not care — both mean "nothing to report about this tree" — but a + * fixture case does, in BOTH buckets: a `fail/` case in that state is a + * fixture that never reached the check rather than a rule that stopped + * firing, and a `pass/` case in that state proves the narrow did not match + * rather than that the check stays quiet. + * - `failure` says the harness itself broke — the narrow threw, or the check + * threw, timed out, or returned unusable output. Downstream that is also + * zero findings from the check's point of view, and only one of the two is + * the rule's fault. + */ +export interface RuntimeExecution { + /** The findings, mapped exactly as the scan path receives them. */ + findings: CheckResult[]; + /** Whether `check.ts` was actually invoked. */ + invoked: boolean; + /** Set when the narrow or the check failed, rather than found nothing. */ + failure?: string; +} + +/** + * Execute one runtime rule and report what happened, not only what it found. + * + * This is the single execution path. {@link executeRuntimeRule} is a thin + * projection of it, so the narrow runs once, `check.ts` is invoked once, and + * "did it run" has one source of truth rather than a scan and a fixture runner + * each deciding for themselves. A harness failure is isolated to a single * error-severity finding and never throws. */ -export async function executeRuntimeRule( +export async function executeRuntimeRuleDetailed( root: string, rule: RuntimeRule, options: RuntimeRunOptions = {} -): Promise { +): Promise { let matches; try { matches = await runNarrow(root, rule, options.paths ?? []); } catch (error) { - return [ - harnessErrorResult( - root, - rule, - `narrow failed: ${error instanceof Error ? error.message : String(error)}` - ), - ]; + const message = `narrow failed: ${error instanceof Error ? error.message : String(error)}`; + return { + findings: [harnessErrorResult(root, rule, message)], + invoked: false, + failure: message, + }; } - if (matches.length === 0) return []; // gate: no matches, no check + // gate: no matches, no check. The gate is the reason `invoked` exists. + if (matches.length === 0) return { findings: [], invoked: false }; const result = await invokeCheck( rule.checkFile, @@ -90,9 +122,33 @@ export async function executeRuntimeRule( options.timeoutMs ?? DEFAULT_CHECK_TIMEOUT_MS ); if (result.status === "error") { - return [harnessErrorResult(root, rule, result.message)]; + return { + findings: [harnessErrorResult(root, rule, result.message)], + invoked: true, + failure: result.message, + }; } - return result.findings.map((finding) => findingToCheckResult(rule, finding)); + return { + findings: result.findings.map((finding) => + findingToCheckResult(rule, finding) + ), + invoked: true, + }; +} + +/** + * Execute one runtime rule: run the ast-grep narrow, gate on matches (zero + * matches ⇒ `check.ts` is never invoked), invoke `check.ts`, and map its + * findings onto `CheckResult`. A harness failure is isolated to a single + * error-severity finding and never throws. + */ +export async function executeRuntimeRule( + root: string, + rule: RuntimeRule, + options: RuntimeRunOptions = {} +): Promise { + const execution = await executeRuntimeRuleDetailed(root, rule, options); + return execution.findings; } /** diff --git a/packages/cli/src/rules/runtime/run-fixtures.ts b/packages/cli/src/rules/runtime/run-fixtures.ts new file mode 100644 index 00000000..6caac819 --- /dev/null +++ b/packages/cli/src/rules/runtime/run-fixtures.ts @@ -0,0 +1,161 @@ +import type { RuntimeRule } from "./discover"; +import { executeRuntimeRuleDetailed } from "./harness"; +import type { RuntimeRunOptions } from "./harness"; +import type { + RuntimeFixtureCase, + RuntimeFixtureCoverage, + RuntimeFixtures, +} from "./fixtures"; + +/** A case that reached the check but broke its bucket's expectation. */ +export interface FixtureCheckFailure { + /** The case directory's name, which is what an author has to go and edit. */ + name: string; + message: string; +} + +/** + * What running a runtime rule's fixtures showed. + * + * Named after the Vale runner's fields on purpose: `missingFailures` and + * `unexpectedFindings` mean the same thing one tier up, so a reader who knows + * one runner knows this one. The two extra lists are D8's, and they exist + * because "the check produced no findings" is three different situations that + * an array of findings cannot tell apart. + */ +export interface RuntimeFixtureReport { + passed: boolean; + coverage: RuntimeFixtureCoverage; + /** `fail/` cases where the check ran and reported nothing. */ + missingFailures: string[]; + /** `pass/` cases where the check ran and reported something. */ + unexpectedFindings: string[]; + /** + * Cases whose narrow matched nothing, so `check.ts` was never invoked. + * + * A fixture defect in EITHER bucket. A `fail/` case here did not show the + * rule stopped firing, it showed the fixture never reached the check; a + * `pass/` case here looks like a clean pass and proves only that the narrow + * did not match, so it cannot show the check stays quiet. Both point at the + * fixture, which is the file the author can actually change. + */ + neverInvoked: string[]; + /** Cases where the narrow or the check itself failed, rather than found nothing. */ + checkFailures: FixtureCheckFailure[]; +} + +/** `fail/case-1` — the bucket is half the identity of a case. */ +function label(fixtureCase: RuntimeFixtureCase): string { + return `${fixtureCase.bucket}/${fixtureCase.name}`; +} + +/** + * Run every fixture case a runtime rule holds, one case directory per run. + * + * The case directory IS the `root`, which is why this is a caller of + * {@link executeRuntimeRuleDetailed} rather than a sibling of it: process + * spawn, timeout, narrowing, capture discovery and the result mapping are the + * ones `check` already proves on every authenticated scan, pointed at a smaller + * tree. Nothing here re-runs the narrow or re-invokes the check. + * + * Cases run in sequence, matching `executeRuntimeRules`, so `tsx` worker + * startup stays predictable. + */ +export async function runRuntimeFixtures( + rule: RuntimeRule, + fixtures: RuntimeFixtures, + options: RuntimeRunOptions = {} +): Promise { + const missingFailures: string[] = []; + const unexpectedFindings: string[] = []; + const neverInvoked: string[] = []; + const checkFailures: FixtureCheckFailure[] = []; + + for (const fixtureCase of fixtures.cases) { + const execution = await executeRuntimeRuleDetailed( + fixtureCase.root, + rule, + options + ); + + // Checked first, and separately from the findings. A throw and a timeout + // both arrive as one error-severity finding, so reading the findings alone + // would count a crashed check as a `fail/` case that fired correctly. + if (execution.failure !== undefined) { + checkFailures.push({ + name: label(fixtureCase), + message: execution.failure, + }); + continue; + } + + if (!execution.invoked) { + neverInvoked.push(label(fixtureCase)); + continue; + } + + if (fixtureCase.bucket === "fail") { + if (execution.findings.length === 0) + missingFailures.push(label(fixtureCase)); + } else if (execution.findings.length > 0) { + unexpectedFindings.push(label(fixtureCase)); + } + } + + return { + // Only `both` can pass, for `ValeFixtureCoverage`'s reason: a rule with one + // bucket has proved half of what a rule claims, and a rule with neither has + // proved nothing while exiting zero. + passed: + fixtures.coverage === "both" && + missingFailures.length === 0 && + unexpectedFindings.length === 0 && + neverInvoked.length === 0 && + checkFailures.length === 0, + coverage: fixtures.coverage, + missingFailures, + unexpectedFindings, + neverInvoked, + checkFailures, + }; +} + +/** + * The report as the lines `test` prints under the rule. + * + * Each line names the case, because the case is the file the author edits. The + * `neverInvoked` line says the check did not run rather than "expected a + * finding, got none": the second is true and sends the author to `check.ts`, + * which is not where the problem is. + */ +export function describeFixtureReport( + ruleId: string, + report: RuntimeFixtureReport +): string[] { + const errors: string[] = []; + + if (report.coverage !== "both") { + errors.push( + report.coverage === "none" + ? `${ruleId} has no fixtures, so nothing shows it fires or stays quiet.` + : `${ruleId} has only ${report.coverage.replace("-only", "")}/ fixtures — half a claim.` + ); + } + for (const name of report.missingFailures) { + errors.push(`fail fixture did not fire: ${name}`); + } + for (const name of report.unexpectedFindings) { + errors.push(`pass fixture wrongly fired: ${name}`); + } + for (const name of report.neverInvoked) { + errors.push( + `fixture case matched no captures, so check.ts never ran: ${name}. ` + + `The case is the defect, not the check: add code the rule's captures match.` + ); + } + for (const failure of report.checkFailures) { + errors.push(`check failed on ${failure.name}: ${failure.message}`); + } + + return errors; +} From 27989c7cdaffd16238f25780cbe4dfc924ee721d Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 2 Sep 2026 13:21:20 -0700 Subject: [PATCH 3/8] refactor(runtime): move check's execution gate beside the engine it gates `test` has to run a runtime rule's fixtures under the policy `check` already applies: an authenticated reconcile that returns the rule's signature in `run`, or `--dangerously-run-scripts`. Nothing about a fixture makes the code safer. The bytes do not know what directory they are pointed at, and "it is only running against test data" is a statement about the input rather than about what the program may do. `planRuntime` and its repair pass therefore move out of `commands/check.ts` and into `rules/runtime/plan.ts`, unchanged. The alternative was a second implementation of the gate in `test`, which is a bypass waiting to be discovered: faithful on the day it is written and drifting from then on. `createRuntimeGate` wraps it for a command that reports rule by rule. The plan is memoized, so a `test` over a tree of runtime rules asks the service once rather than once per rule, and the whole discovered set is reported to reconcile exactly as `check` reports it. It is lazy because most projects hold no runtime rules, and a `test` over `sg` rules must not reach the network. No rule id is exempt, and no path through the gate is softer than another. --- .../changes/runtime-fixture-runner/tasks.md | 8 +- packages/cli/src/commands/check.ts | 297 +------------- packages/cli/src/rules/runtime/plan.ts | 372 ++++++++++++++++++ 3 files changed, 381 insertions(+), 296 deletions(-) create mode 100644 packages/cli/src/rules/runtime/plan.ts diff --git a/openspec/changes/runtime-fixture-runner/tasks.md b/openspec/changes/runtime-fixture-runner/tasks.md index 1bc01b1b..8752bc19 100644 --- a/openspec/changes/runtime-fixture-runner/tasks.md +++ b/openspec/changes/runtime-fixture-runner/tasks.md @@ -21,10 +21,10 @@ permanently unverified. ## 3. Gate it exactly as `check` does -- [ ] 3.1 Run fixtures only when an authenticated reconcile returns the rule's signature in `run`, or when `--dangerously-run-scripts` is passed (D1) -- [ ] 3.2 Add `--dangerously-run-scripts` to `test`, printing the same warning `check` prints -- [ ] 3.3 Test that no rule id is special-cased, and that a gated-out fixture run executes nothing -- [ ] 3.4 Test that the flag is the only mechanism besides a blessed signature. A fixture path is not a softer gate because its input is test data +- [x] 3.1 Run fixtures only when an authenticated reconcile returns the rule's signature in `run`, or when `--dangerously-run-scripts` is passed (D1) +- [x] 3.2 Add `--dangerously-run-scripts` to `test`, printing the same warning `check` prints +- [x] 3.3 Test that no rule id is special-cased, and that a gated-out fixture run executes nothing +- [x] 3.4 Test that the flag is the only mechanism besides a blessed signature. A fixture path is not a softer gate because its input is test data ## 4. Report a run that did not happen diff --git a/packages/cli/src/commands/check.ts b/packages/cli/src/commands/check.ts index 97fd3fd1..d84e35fe 100644 --- a/packages/cli/src/commands/check.ts +++ b/packages/cli/src/commands/check.ts @@ -11,27 +11,12 @@ import { getTelemetry } from "../telemetry"; import { outputSchema as checkOutputSchema } from "../schemas/check"; import { makeErrorEnvelope, writeJsonError } from "../types/errors"; import { CLIError } from "../util/cli-error"; -import { getToken } from "../auth/token"; -import { resolveOrgSubject } from "../auth/org"; -import { resolveRepositoryUrl } from "../util/git-remote"; -import { getCliPrefix } from "../util/package-manager"; import { requireCurrentSchema } from "../filesystem/migrate"; -import { reconcile } from "../api/reconcile"; -import type { ReconcileResponse } from "../api/reconcile"; -import { restoreRule } from "../api/restore"; -import { repairTargets, verifyRestoredCheck } from "../rules/runtime/repair"; -import { writeRuleFile } from "../rules/files"; -import { PurgeIncompleteError } from "../rules/deliver"; -import { - discoverRuntimeRules, - type RuntimeRule, -} from "../rules/runtime/discover"; -import { - materializeRuntimeRules, - reportRuntimeChecks, - selectBlessedRuntimeRules, - signRuntimeChecks, -} from "../rules/runtime/run-set"; +import { discoverRuntimeRules } from "../rules/runtime/discover"; +// The gate lives beside the runtime engine rather than inside this command, +// because `test` runs a rule's fixtures under exactly this policy. Sharing the +// implementation is what makes that a fact rather than an intention. +import { planRuntime } from "../rules/runtime/plan"; async function pathExists(absolutePath: string): Promise { try { @@ -82,278 +67,6 @@ function extractPositionalPaths(rawArguments: string[]): string[] { return splitRawArguments(rawArguments, ["--timeout"]).positionals; } -/** A runtime rule that will not run, with why (advisory). */ -interface SkippedRuntimeRule { - rule: string; - reason: string; -} - -/** The runtime-execution plan resolved from auth state and flags. */ -interface RuntimePlan { - /** Rules to execute — materialized when gated, live under `--dangerously-run-scripts`. */ - execute: RuntimeRule[]; - /** Rules that will not run, with a reason. */ - skipped: SkippedRuntimeRule[]; - /** Human-only notices about the runtime disposition. */ - notices: string[]; -} - -/** Skip every runtime rule with a shared reason (an unverified path). */ -function skipAllRuntime(rules: RuntimeRule[], reason: string): RuntimePlan { - return { - execute: [], - skipped: rules.map((rule) => ({ rule: rule.name, reason })), - notices: [], - }; -} - -/** - * Decide which runtime rules run. A runtime rule's `check.ts` is arbitrary code - * execution, so it runs only when its signature is server-validated (an - * authenticated reconcile that returns it in `run`) or `--dangerously-run-scripts` - * is set. Every unverified path — anonymous, logged out, no remote, or a - * reconcile that cannot complete — skips runtime rules without failing. - */ -async function planRuntime( - cwd: string, - discovered: RuntimeRule[], - options: { anonymous: boolean; dangerouslyRunScripts: boolean } -): Promise { - if (discovered.length === 0) return { execute: [], skipped: [], notices: [] }; - - if (options.dangerouslyRunScripts) { - return { - execute: discovered, - skipped: [], - notices: [ - "Warning: --dangerously-run-scripts is executing runtime rule code without server verification.", - ], - }; - } - - if (options.anonymous) { - return skipAllRuntime( - discovered, - "anonymous mode — runtime rules were not verified and did not run" - ); - } - - const token = await getToken(cwd, { silent: true }); - if (!token) { - return skipAllRuntime( - discovered, - "not authenticated — runtime rules were not verified and did not run" - ); - } - - let repositoryUrl: string; - try { - repositoryUrl = await resolveRepositoryUrl(cwd); - } catch { - return skipAllRuntime( - discovered, - "no GitHub remote — runtime rules could not be verified and did not run" - ); - } - - // A rule whose check.ts is missing/unreadable is reported, not fatal: signing - // never throws, and such rules are surfaced as skipped so static checks and - // the other runtime rules are unaffected. - const { signed, unreadable } = await signRuntimeChecks(discovered); - const unreadableSkips: SkippedRuntimeRule[] = unreadable.map((rule) => ({ - rule: rule.name, - reason: "its check.ts is missing or unreadable", - })); - - const orgSubject = await resolveOrgSubject(cwd, token); - const outcome = await reconcile(token, { - orgId: orgSubject, - repositoryUrl, - files: reportRuntimeChecks(cwd, signed), - }); - - if (outcome.status === "unauthorized") { - return skipAllRuntime( - discovered, - `authentication was rejected — run \`${getCliPrefix()} auth login\` to re-authenticate` - ); - } - if (outcome.status === "unavailable") { - return skipAllRuntime( - discovered, - `the rule service was unavailable (${outcome.reason})` - ); - } - - const { blessed, withheld } = selectBlessedRuntimeRules( - signed, - outcome.result.run - ); - let execute: RuntimeRule[] = []; - try { - execute = - blessed.length > 0 ? await materializeRuntimeRules(cwd, blessed) : []; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return skipAllRuntime( - discovered, - `runtime rules could not be materialized (${message})` - ); - } - // Repair the working tree from the server's verdicts. This changes what the - // NEXT run sees and nothing about this one: an `unsafe` rule stays withheld - // below whether or not its bytes were just restored. Fetching code and - // executing it in the same pass that discovered the drift would move the - // gate, and the gate is the point. - const repair = await repairWithheldRules(cwd, token, { - repositoryUrl, - result: outcome.result, - }); - - return { - execute, - skipped: [ - ...unreadableSkips, - ...withheld.map((rule) => ({ - rule: rule.name, - reason: "not blessed by the server (unsafe / unknown / drift)", - })), - ], - notices: repair.notices, - }; -} - -/** - * Act on the verdicts `check` used to parse and discard. - * - * `unsafe` and `missing` are repairable and are fetched; `unknown` is not, and - * gets an explanation instead. Every outcome here is a NOTICE rather than a - * failure: a rule that could not be repaired is a rule that stays withheld, - * which is already the safe state. A repair failing must never be the reason a - * `check` fails. - */ -async function repairWithheldRules( - cwd: string, - token: string, - input: { repositoryUrl: string; result: ReconcileResponse } -): Promise<{ notices: string[] }> { - const notices: string[] = []; - - // A file this disk holds that the service never issued. There is nothing to - // fetch, and saying so is the whole job: it reads as an unexplained skip - // otherwise, and the causes are ordinary (hand-written, or belonging to - // another organization or installation). - for (const entry of input.result.unknown) { - notices.push( - `${entry.file} was not issued by the rule service, so it cannot be ` + - `restored and will not run. It was written by hand, or belongs to a ` + - `different organization or installation.` - ); - } - - const { targets, unidentified } = repairTargets(input.result); - - // A repairable entry that named no rule. The service's own schema requires - // one, so reaching here means it broke that contract — and the entry is - // skipped rather than guessed at, because a rule left unrepaired stays - // withheld, which is safe, while a request built from a missing id asks for - // a rule nobody named and fails in a way nobody reads. - for (const entry of unidentified) { - notices.push( - `${entry.file} needs to be restored, but the rule service did not say ` + - `which rule it belongs to, so it could not be requested and will not ` + - `run.` - ); - } - - // Fetched concurrently: each target is a different rule id under the same - // token and repository, so they do not order against each other, and a repo - // with several drifted rules would otherwise pay one round trip per rule on - // every `check` until they reconverge. The WRITES stay sequential below, - // because two rules can share a directory prefix and a half-applied set is - // the state this whole path exists to avoid. - const fetched = await Promise.all( - targets.map(async (target) => ({ - target, - outcome: await restoreRule(token, { - ruleId: target.ruleId, - repositoryUrl: input.repositoryUrl, - }), - })) - ); - - for (const { target, outcome } of fetched) { - if (outcome.status !== "ok") { - notices.push( - `${target.file} could not be restored (${ - outcome.status === "unauthorized" - ? "authentication was rejected" - : outcome.reason - }).` - ); - continue; - } - - const rule = outcome.rules.find( - (candidate) => candidate.id === target.ruleId - ); - if (rule === undefined) { - notices.push( - `${target.file} could not be restored: the service returned no rule ` + - `called ${target.ruleId}.` - ); - continue; - } - - const verdict = await verifyRestoredCheck(target, rule); - if (!verdict.ok) { - notices.push(`${target.file} was not restored: ${verdict.reason}.`); - continue; - } - - try { - await writeRuleFile(cwd, rule); - } catch (error) { - // A failed WRITE and a failed CLEANUP ask the reader for opposite - // things, and saying "could not be written" for both is worse than - // saying nothing: the blessed bytes are on disk in the second case, so - // a reader acting on it re-runs a repair that already succeeded, or - // decides the rule is unrepaired and edits it by hand. - if (error instanceof PurgeIncompleteError) { - notices.push( - `${target.file} was rewritten with the bytes the service blessed, ` + - `but ${String(error.failures.length)} stale ` + - `${error.failures.length === 1 ? "entry" : "entries"} could not ` + - `be removed and an engine still reads ` + - `${error.failures.length === 1 ? "it" : "them"}: ` + - `${error.failures.join(", ")}.` - ); - continue; - } - const message = error instanceof Error ? error.message : String(error); - notices.push(`${target.file} could not be written (${message}).`); - continue; - } - // Now says what the DIRECTORY contains, not just what was written. The - // delivered set is authoritative (see `writeDeliveredFileSet`), so a file - // the set does not name — a stray capture beside the rule, which reconcile - // never reported because only `check.ts` is signed — is gone rather than - // left in place still changing what the rule matches. The one exception is - // `.tests/`, which is named here rather than glossed: fixtures are data no - // engine reads, they are kept, and a reader should not have to infer that - // from silence. - notices.push( - `${target.file} was restored: its rule directory now holds exactly the ` + - `files the service delivered, apart from test fixtures under ` + - `\`.tests/\`, which are left alone. It does not run in this pass; the ` + - `next \`check\` reports the repaired signature and is blessed through ` + - `the ordinary path.` - ); - } - - return { notices }; -} - /** Parse `--timeout ` into milliseconds; invalid/absent → undefined (default). */ function parseTimeoutMs(raw: string | undefined): number | undefined { if (raw === undefined) return undefined; diff --git a/packages/cli/src/rules/runtime/plan.ts b/packages/cli/src/rules/runtime/plan.ts new file mode 100644 index 00000000..e1c5f878 --- /dev/null +++ b/packages/cli/src/rules/runtime/plan.ts @@ -0,0 +1,372 @@ +import { getToken } from "../../auth/token"; +import { resolveOrgSubject } from "../../auth/org"; +import { resolveRepositoryUrl } from "../../util/git-remote"; +import { getCliPrefix } from "../../util/package-manager"; +import { reconcile } from "../../api/reconcile"; +import type { ReconcileResponse } from "../../api/reconcile"; +import { restoreRule } from "../../api/restore"; +import { writeRuleFile } from "../files"; +import { PurgeIncompleteError } from "../deliver"; +import { repairTargets, verifyRestoredCheck } from "./repair"; +import { discoverRuntimeRules, type RuntimeRule } from "./discover"; +import { + materializeRuntimeRules, + reportRuntimeChecks, + selectBlessedRuntimeRules, + signRuntimeChecks, +} from "./run-set"; + +/** + * Deciding WHICH runtime rules may execute, separately from executing them. + * + * This lived inside `commands/check.ts` and moved here unchanged so `test` can + * run a rule's fixtures under the same policy rather than beside it. That is + * the whole reason for the move: a fixture run executes the same `check.ts`, + * from the same delivery, under the same signature as a scan, and "it is only + * running against test data" is a statement about the input rather than about + * what the program may do. A second implementation of this gate, however + * faithful on the day it was written, is a bypass waiting to be discovered. + */ + +/** A runtime rule that will not run, with why (advisory). */ +export interface SkippedRuntimeRule { + rule: string; + reason: string; +} + +/** The runtime-execution plan resolved from auth state and flags. */ +export interface RuntimePlan { + /** Rules to execute — materialized when gated, live under `--dangerously-run-scripts`. */ + execute: RuntimeRule[]; + /** Rules that will not run, with a reason. */ + skipped: SkippedRuntimeRule[]; + /** Human-only notices about the runtime disposition. */ + notices: string[]; +} + +/** Skip every runtime rule with a shared reason (an unverified path). */ +function skipAllRuntime(rules: RuntimeRule[], reason: string): RuntimePlan { + return { + execute: [], + skipped: rules.map((rule) => ({ rule: rule.name, reason })), + notices: [], + }; +} + +/** + * Decide which runtime rules run. A runtime rule's `check.ts` is arbitrary code + * execution, so it runs only when its signature is server-validated (an + * authenticated reconcile that returns it in `run`) or `--dangerously-run-scripts` + * is set. Every unverified path — anonymous, logged out, no remote, or a + * reconcile that cannot complete — skips runtime rules without failing. + */ +export async function planRuntime( + cwd: string, + discovered: RuntimeRule[], + options: { anonymous: boolean; dangerouslyRunScripts: boolean } +): Promise { + if (discovered.length === 0) return { execute: [], skipped: [], notices: [] }; + + if (options.dangerouslyRunScripts) { + return { + execute: discovered, + skipped: [], + notices: [ + "Warning: --dangerously-run-scripts is executing runtime rule code without server verification.", + ], + }; + } + + if (options.anonymous) { + return skipAllRuntime( + discovered, + "anonymous mode — runtime rules were not verified and did not run" + ); + } + + const token = await getToken(cwd, { silent: true }); + if (!token) { + return skipAllRuntime( + discovered, + "not authenticated — runtime rules were not verified and did not run" + ); + } + + let repositoryUrl: string; + try { + repositoryUrl = await resolveRepositoryUrl(cwd); + } catch { + return skipAllRuntime( + discovered, + "no GitHub remote — runtime rules could not be verified and did not run" + ); + } + + // A rule whose check.ts is missing/unreadable is reported, not fatal: signing + // never throws, and such rules are surfaced as skipped so static checks and + // the other runtime rules are unaffected. + const { signed, unreadable } = await signRuntimeChecks(discovered); + const unreadableSkips: SkippedRuntimeRule[] = unreadable.map((rule) => ({ + rule: rule.name, + reason: "its check.ts is missing or unreadable", + })); + + const orgSubject = await resolveOrgSubject(cwd, token); + const outcome = await reconcile(token, { + orgId: orgSubject, + repositoryUrl, + files: reportRuntimeChecks(cwd, signed), + }); + + if (outcome.status === "unauthorized") { + return skipAllRuntime( + discovered, + `authentication was rejected — run \`${getCliPrefix()} auth login\` to re-authenticate` + ); + } + if (outcome.status === "unavailable") { + return skipAllRuntime( + discovered, + `the rule service was unavailable (${outcome.reason})` + ); + } + + const { blessed, withheld } = selectBlessedRuntimeRules( + signed, + outcome.result.run + ); + let execute: RuntimeRule[] = []; + try { + execute = + blessed.length > 0 ? await materializeRuntimeRules(cwd, blessed) : []; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + return skipAllRuntime( + discovered, + `runtime rules could not be materialized (${message})` + ); + } + // Repair the working tree from the server's verdicts. This changes what the + // NEXT run sees and nothing about this one: an `unsafe` rule stays withheld + // below whether or not its bytes were just restored. Fetching code and + // executing it in the same pass that discovered the drift would move the + // gate, and the gate is the point. + const repair = await repairWithheldRules(cwd, token, { + repositoryUrl, + result: outcome.result, + }); + + return { + execute, + skipped: [ + ...unreadableSkips, + ...withheld.map((rule) => ({ + rule: rule.name, + reason: "not blessed by the server (unsafe / unknown / drift)", + })), + ], + notices: repair.notices, + }; +} + +/** + * Act on the verdicts `check` used to parse and discard. + * + * `unsafe` and `missing` are repairable and are fetched; `unknown` is not, and + * gets an explanation instead. Every outcome here is a NOTICE rather than a + * failure: a rule that could not be repaired is a rule that stays withheld, + * which is already the safe state. A repair failing must never be the reason a + * `check` fails. + */ +async function repairWithheldRules( + cwd: string, + token: string, + input: { repositoryUrl: string; result: ReconcileResponse } +): Promise<{ notices: string[] }> { + const notices: string[] = []; + + // A file this disk holds that the service never issued. There is nothing to + // fetch, and saying so is the whole job: it reads as an unexplained skip + // otherwise, and the causes are ordinary (hand-written, or belonging to + // another organization or installation). + for (const entry of input.result.unknown) { + notices.push( + `${entry.file} was not issued by the rule service, so it cannot be ` + + `restored and will not run. It was written by hand, or belongs to a ` + + `different organization or installation.` + ); + } + + const { targets, unidentified } = repairTargets(input.result); + + // A repairable entry that named no rule. The service's own schema requires + // one, so reaching here means it broke that contract — and the entry is + // skipped rather than guessed at, because a rule left unrepaired stays + // withheld, which is safe, while a request built from a missing id asks for + // a rule nobody named and fails in a way nobody reads. + for (const entry of unidentified) { + notices.push( + `${entry.file} needs to be restored, but the rule service did not say ` + + `which rule it belongs to, so it could not be requested and will not ` + + `run.` + ); + } + + // Fetched concurrently: each target is a different rule id under the same + // token and repository, so they do not order against each other, and a repo + // with several drifted rules would otherwise pay one round trip per rule on + // every `check` until they reconverge. The WRITES stay sequential below, + // because two rules can share a directory prefix and a half-applied set is + // the state this whole path exists to avoid. + const fetched = await Promise.all( + targets.map(async (target) => ({ + target, + outcome: await restoreRule(token, { + ruleId: target.ruleId, + repositoryUrl: input.repositoryUrl, + }), + })) + ); + + for (const { target, outcome } of fetched) { + if (outcome.status !== "ok") { + notices.push( + `${target.file} could not be restored (${ + outcome.status === "unauthorized" + ? "authentication was rejected" + : outcome.reason + }).` + ); + continue; + } + + const rule = outcome.rules.find( + (candidate) => candidate.id === target.ruleId + ); + if (rule === undefined) { + notices.push( + `${target.file} could not be restored: the service returned no rule ` + + `called ${target.ruleId}.` + ); + continue; + } + + const verdict = await verifyRestoredCheck(target, rule); + if (!verdict.ok) { + notices.push(`${target.file} was not restored: ${verdict.reason}.`); + continue; + } + + try { + await writeRuleFile(cwd, rule); + } catch (error) { + // A failed WRITE and a failed CLEANUP ask the reader for opposite + // things, and saying "could not be written" for both is worse than + // saying nothing: the blessed bytes are on disk in the second case, so + // a reader acting on it re-runs a repair that already succeeded, or + // decides the rule is unrepaired and edits it by hand. + if (error instanceof PurgeIncompleteError) { + notices.push( + `${target.file} was rewritten with the bytes the service blessed, ` + + `but ${String(error.failures.length)} stale ` + + `${error.failures.length === 1 ? "entry" : "entries"} could not ` + + `be removed and an engine still reads ` + + `${error.failures.length === 1 ? "it" : "them"}: ` + + `${error.failures.join(", ")}.` + ); + continue; + } + const message = error instanceof Error ? error.message : String(error); + notices.push(`${target.file} could not be written (${message}).`); + continue; + } + // Now says what the DIRECTORY contains, not just what was written. The + // delivered set is authoritative (see `writeDeliveredFileSet`), so a file + // the set does not name — a stray capture beside the rule, which reconcile + // never reported because only `check.ts` is signed — is gone rather than + // left in place still changing what the rule matches. The one exception is + // `.tests/`, which is named here rather than glossed: fixtures are data no + // engine reads, they are kept, and a reader should not have to infer that + // from silence. + notices.push( + `${target.file} was restored: its rule directory now holds exactly the ` + + `files the service delivered, apart from test fixtures under ` + + `\`.tests/\`, which are left alone. It does not run in this pass; the ` + + `next \`check\` reports the repaired signature and is blessed through ` + + `the ordinary path.` + ); + } + + return { notices }; +} + +/** + * The plan, resolved once and shared by every rule a command reports on. + * + * `test` inspects rules one at a time, and planning per rule would send one + * reconcile per runtime rule for an answer that is the same every time. The + * plan is therefore memoized on first use and the notices are emitted once, + * with the whole set of discovered runtime rules reported to reconcile exactly + * as `check` reports it. A subset would be a different question asked of the + * service. + * + * Lazy rather than eager because most projects hold no runtime rules at all, + * and a `test` run over a tree of `sg` rules must not reach the network. + */ +export interface RuntimeGate { + /** + * The rule as it may be executed, or the reason it may not be. + * + * The rule returned is the one the plan blessed, which on the gated path is + * the materialized copy: the bytes run against a fixture are the bytes the + * server blessed, not whatever the working tree happens to hold. + */ + admit( + ruleId: string + ): Promise< + { admitted: true; rule: RuntimeRule } | { admitted: false; reason: string } + >; +} + +/** Build a gate over the runtime rules `cwd` holds. */ +export function createRuntimeGate( + cwd: string, + options: { + dangerouslyRunScripts: boolean; + /** Called once, with the notices `check` prints for the same plan. */ + onNotice?: (message: string) => void; + } +): RuntimeGate { + let planned: Promise | undefined; + + const plan = async (): Promise => { + planned ??= (async () => { + const discovered = await discoverRuntimeRules(cwd); + const resolved = await planRuntime(cwd, discovered, { + anonymous: false, + dangerouslyRunScripts: options.dangerouslyRunScripts, + }); + for (const notice of resolved.notices) options.onNotice?.(notice); + return resolved; + })(); + return planned; + }; + + return { + async admit(ruleId) { + const resolved = await plan(); + const rule = resolved.execute.find( + (candidate) => candidate.name === ruleId + ); + if (rule !== undefined) return { admitted: true, rule }; + + const skipped = resolved.skipped.find((entry) => entry.rule === ruleId); + return { + admitted: false, + reason: + skipped?.reason ?? + "it was not discovered as a runnable runtime rule (no capture rules under captures/)", + }; + }, + }; +} From 5a85ce8a8146a24ae9a71d7c2b8eb54e9a56b44d Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 2 Sep 2026 13:21:37 -0700 Subject: [PATCH 4/8] fix(test): a runtime rule that did not run is neither a pass nor a failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `testOneRule` returned `{ ok: true, errors: [], ran: false }` for every runtime rule, and the renderer read only `ok`, so `test` printed `✓ runtime/` and "1 rule(s) tested" about a rule it had not tested. The one field that knew was the one nothing read. The runtime branch now takes the rule through the shared gate and, when admitted, runs its fixtures. When the gate refuses, the result carries `refused` with the reason, and three call sites read it: the renderer prints `○` rather than a tick, the summary counts the rule under "did not run" rather than among the rules tested, and the exit code ignores it. `ok: false` alone would have been the wrong correction. A rule that cannot run because nothing blessed it is not defective, and failing it turns `test` red for every project holding a runtime rule with no action available that makes it green. `ok` also cannot express the state on its own: a rule whose `verify` failed is likewise `ok: false, ran: false` and must still fail. `ran` becomes load-bearing in the `--json` envelope, `refused` carries the reason beside it, and both are documented as the fields a caller branches on. The refusal message names `--dangerously-run-scripts`, because for a locally authored rule that is the author's only route: blessing is recording, and nothing recorded a rule that has never left the working tree. The flag is added to `test` with `check`'s description and `check`'s warning, on stderr, and one gate is built per command run so a tree of runtime rules is planned once. --- .../changes/runtime-fixture-runner/tasks.md | 10 +- packages/cli/src/commands/verify.ts | 56 ++++++++-- packages/cli/src/rules/inspect.ts | 100 ++++++++++++++++-- packages/cli/src/schemas/verify-test.ts | 10 +- 4 files changed, 153 insertions(+), 23 deletions(-) diff --git a/openspec/changes/runtime-fixture-runner/tasks.md b/openspec/changes/runtime-fixture-runner/tasks.md index 8752bc19..0c4838aa 100644 --- a/openspec/changes/runtime-fixture-runner/tasks.md +++ b/openspec/changes/runtime-fixture-runner/tasks.md @@ -28,11 +28,11 @@ permanently unverified. ## 4. Report a run that did not happen -- [ ] 4.1 Stop returning `ok: true` from `testOneRule`'s runtime branch for a run that did not occur -- [ ] 4.2 Render a skipped rule with its own marker and the reason, never a tick (D2) -- [ ] 4.3 Stop counting a skipped rule in "N rule(s) tested" -- [ ] 4.4 Make `ran` load-bearing in the `--json` envelope rather than advisory, and confirm the exit code does not fail on a gated-out run alone -- [ ] 4.5 Say what to do about it: a rule that did not run because nothing blessed it should name `--dangerously-run-scripts` in the message (D6) +- [x] 4.1 Stop returning `ok: true` from `testOneRule`'s runtime branch for a run that did not occur +- [x] 4.2 Render a skipped rule with its own marker and the reason, never a tick (D2) +- [x] 4.3 Stop counting a skipped rule in "N rule(s) tested" +- [x] 4.4 Make `ran` load-bearing in the `--json` envelope rather than advisory, and confirm the exit code does not fail on a gated-out run alone +- [x] 4.5 Say what to do about it: a rule that did not run because nothing blessed it should name `--dangerously-run-scripts` in the message (D6) ## 5. Prove it bites diff --git a/packages/cli/src/commands/verify.ts b/packages/cli/src/commands/verify.ts index 121a4a27..e7731cf8 100644 --- a/packages/cli/src/commands/verify.ts +++ b/packages/cli/src/commands/verify.ts @@ -15,6 +15,7 @@ import { resolveRulePath, RuleNotFoundError, } from "../rules/resolve-path"; +import { createRuntimeGate } from "../rules/runtime/plan"; import { outputSchema as verifyTestOutputSchema } from "../schemas/verify-test"; import { makeErrorEnvelope, writeJsonError } from "../types/errors"; import { CLIError } from "../util/cli-error"; @@ -124,7 +125,19 @@ async function runOverPath(options: { results.push(await run(cwd, rule)); } - const failed = results.filter((result) => !result.ok); + // 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 `test` 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 tested, and gets its + // own marker below. + const refused = results.filter( + (result) => "refused" in result && result.refused !== undefined + ); + const isRefused = (result: RuleVerification | RuleTestResult): boolean => + "refused" in result && result.refused !== undefined; + const failed = results.filter((result) => !result.ok && !isRefused(result)); + const tested = results.length - refused.length; if (json) { console.log( @@ -137,7 +150,10 @@ async function runOverPath(options: { ); } else { for (const result of results) { - const mark = result.ok ? "✓" : "✗"; + // `○` is neither tick nor cross on purpose: the rule was not tested, and + // a reader scanning the column has to be able to see that at a glance. + // The reason prints below it, from `errors`, and names what would run it. + const mark = isRefused(result) ? "○" : result.ok ? "✓" : "✗"; console.log(`${mark} ${result.engine}/${result.ruleId}`); for (const error of result.errors) { console.log(` ${error}`); @@ -149,10 +165,17 @@ async function runOverPath(options: { console.log(` notice: ${result.notice}`); } } + // A rule that did not run is not among the rules tested. Counting it there + // is the summary half of the same defect as the tick: "1 rule(s) tested" + // about a rule nothing ran. + const notRun = + refused.length === 0 + ? "" + : ` ${String(refused.length)} rule(s) did not run.`; console.log( failed.length === 0 - ? `\n${String(results.length)} rule(s) ${label === "verify" ? "verified" : "tested"}.` - : `\n${String(failed.length)} of ${String(results.length)} rule(s) failed.` + ? `\n${String(tested)} rule(s) ${label === "verify" ? "verified" : "tested"}.${notRun}` + : `\n${String(failed.length)} of ${String(tested)} rule(s) failed.${notRun}` ); } @@ -203,15 +226,36 @@ export const testCommand = defineCommand({ name: "test", description: "Run a rule's tests, after verifying the rule itself", }, - args: ruleTargetArguments, + args: { + ...ruleTargetArguments, + // The same flag `check` carries, with the same description, because it is + // the same gate. A runtime rule's fixtures execute its `check.ts`, and + // that the input is test data is a statement about the input rather than + // about what the program may do. + "dangerously-run-scripts": { + type: "boolean", + description: + "Run runtime-rule check.ts without server verification (executes untrusted code)", + default: false, + }, + }, async run({ args }) { const cwd = resolve(args.dir ?? process.cwd()); + // One gate for the whole run, so a tree of runtime rules is planned once. + // The notices are `check`'s, on stderr, and suppressed under `--json` for + // `check`'s reason: a machine consumer cannot read prose. + const runtimeGate = createRuntimeGate(cwd, { + dangerouslyRunScripts: Boolean(args["dangerously-run-scripts"]), + onNotice: (message: string) => { + if (!args.json) console.error(message); + }, + }); await runOverPath({ cwd, target: args.path ?? ".taskless/rules", json: args.json, label: "test", - run: testOneRule, + run: async (ruleCwd, rule) => testOneRule(ruleCwd, rule, { runtimeGate }), }); }, }); diff --git a/packages/cli/src/rules/inspect.ts b/packages/cli/src/rules/inspect.ts index e7b2109e..f8443b12 100644 --- a/packages/cli/src/rules/inspect.ts +++ b/packages/cli/src/rules/inspect.ts @@ -9,10 +9,13 @@ import { ruleFilePath, } from "./engines"; import { type EngineName } from "./layout"; +import { assessCaptureDirectory, strayModules } from "./runtime/discover"; +import { readRuntimeFixtures } from "./runtime/fixtures"; +import { createRuntimeGate, type RuntimeGate } from "./runtime/plan"; import { - assessCaptureDirectory, - strayModules, -} from "./runtime/discover"; + describeFixtureReport, + runRuntimeFixtures, +} from "./runtime/run-fixtures"; import { validateValeRule } from "../schemas/vale-rule"; import { verifyRule, type VerifyResult } from "./verify"; import { verifyValeRule } from "./vale/verify"; @@ -40,8 +43,26 @@ export interface RuleTestResult { ruleId: string; ok: boolean; errors: string[]; - /** Absent when `verify` failed and the tests never ran. */ + /** + * Whether the rule's tests actually ran. + * + * Load-bearing rather than advisory. `test` used to report a runtime rule as + * `ok: true, ran: false` and print a tick, so the one field that knew the + * rule had not been tested was the one nothing read. + */ ran: boolean; + /** + * Set when the execution policy refused the run, carrying the reason. + * + * The third outcome, and the one `ok` cannot express. A rule that cannot run + * because nothing blessed it is not a pass, and it is not a failure either: + * failing it would turn `test` red for every project holding a runtime rule, + * with no action available to its holder that makes it green. So it is + * neither, and the renderer, the summary count and the exit code all read + * this field rather than inferring the state from `ok` and `ran` together + * (which cannot distinguish it from a rule whose `verify` failed). + */ + refused?: string; /** * Something the engine said about its own configuration, as opposed to about * the rule. Vale reports a misplaced `.vale.ini` assignment this way: it @@ -53,6 +74,19 @@ export interface RuleTestResult { notice?: string; } +/** What `test` needs beyond a rule, all of it about the runtime engine. */ +export interface TestOptions { + /** + * The execution gate, shared across every rule in one command run. + * + * Passed in rather than built here so a `test` over a whole tree plans once: + * the plan consults auth and reconcile, and per-rule planning would send one + * request per runtime rule for an answer that does not vary. Omitted, a + * gate with no escape flag is built, which is the safe default. + */ + runtimeGate?: RuntimeGate; +} + async function readYaml(path: string): Promise { return parse(await readFile(path, "utf8")) as unknown; } @@ -233,7 +267,8 @@ export async function verifyOneRule( */ export async function testOneRule( cwd: string, - rule: ResolvedRule + rule: ResolvedRule, + options: TestOptions = {} ): Promise { const { engine, ruleId } = rule; @@ -309,14 +344,57 @@ export async function testOneRule( }; } - // Runtime rules execute code, so their tests run through the harness under - // the same server verification `check` requires. Out of scope here: `test` - // reports that rather than quietly claiming a pass. + // Runtime rules execute code, so their fixtures run through the harness + // under the same server verification `check` requires: the gate below is + // `check`'s, imported rather than re-stated. + const gate = + options.runtimeGate ?? + createRuntimeGate(cwd, { dangerouslyRunScripts: false }); + const admission = await gate.admit(ruleId); + + if (!admission.admitted) { + // Neither a pass nor a failure (D2), and the message has to say what would + // change it. A locally authored rule has no signature and never will, + // because blessing is recording and nothing recorded this one, so the flag + // is the author's only route and naming it here is the difference between + // a dead end and an instruction. + const reason = + `fixtures did not run: ${admission.reason}. Pass ` + + `--dangerously-run-scripts to run them without server verification ` + + `(it executes the rule's check.ts).`; + return { + engine, + ruleId, + ok: false, + errors: [reason], + ran: false, + refused: reason, + }; + } + + let fixtures; + try { + fixtures = await readRuntimeFixtures(cwd, ruleId); + } catch (error) { + // A bucket that could not be read, or an entry that is not a directory. + // Both are failures of the fixtures rather than refusals of the run, so + // they fail: the alternative is a bucket reading as empty, which makes a + // two-sided rule look one-sided and a one-sided rule look complete. + return { + engine, + ruleId, + ok: false, + errors: [error instanceof Error ? error.message : String(error)], + ran: false, + }; + } + + const report = await runRuntimeFixtures(admission.rule, fixtures); return { engine, ruleId, - ok: true, - errors: [], - ran: false, + ok: report.passed, + errors: describeFixtureReport(ruleId, report), + ran: true, }; } diff --git a/packages/cli/src/schemas/verify-test.ts b/packages/cli/src/schemas/verify-test.ts index 04a92817..3a8b3cdc 100644 --- a/packages/cli/src/schemas/verify-test.ts +++ b/packages/cli/src/schemas/verify-test.ts @@ -15,7 +15,15 @@ const ruleResultSchema = z.object({ ran: z .boolean() .optional() - .describe("`test` only: whether the rule's tests actually ran"), + .describe( + "`test` only: whether the rule's tests actually ran. Branch on this, not on `ok` alone: `ok: true` with `ran: false` is not a rule that passed" + ), + refused: z + .string() + .optional() + .describe( + "`test` only: the execution policy declined to run the rule's fixtures, and why. Neither a pass nor a failure, excluded from the rules tested, and never on its own a reason for a non-zero exit" + ), notice: z .string() .optional() From ca49e4d530bc2abcd4bcd1443b6ddd34baf7c20e Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 2 Sep 2026 13:22:02 -0700 Subject: [PATCH 5/8] test: a runtime rule that did not run must not report a tick MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Written against the unfixed CLI first, where 15 of the 17 assertions failed and the first one failed with exactly the output the proposal recorded: expected '✓ runtime/no-eval\n\n1 rule(s) tested…' not to contain '✓ runtime/no-eval' The two that passed are the gate's, and they passed for the wrong reason: with no runner at all, "no fixture case was executed" was trivially true. The suite covers the defect (no tick, no count, `ran: false`, exit 0), the gate (nothing executes unblessed, no id is exempt, the flag is the only other way in), both fixture directions, both never-invoked cases, a check that throws, every coverage class, and an unreadable or malformed bucket. Two details in the fixtures are load-bearing rather than incidental. The rule flags `eval` only on a non-literal argument, because a check that flagged every match would have no `pass/` case that both matched the narrow and stayed quiet, and a `pass/` case that does not match the narrow proves nothing. And `matching` and `flagged` are separate knobs on a case, because the narrow and the check are what D8 exists to keep apart. --- .../changes/runtime-fixture-runner/tasks.md | 10 +- .../cli/test/runtime-fixture-runner.test.ts | 448 ++++++++++++++++++ 2 files changed, 453 insertions(+), 5 deletions(-) create mode 100644 packages/cli/test/runtime-fixture-runner.test.ts diff --git a/openspec/changes/runtime-fixture-runner/tasks.md b/openspec/changes/runtime-fixture-runner/tasks.md index 0c4838aa..07b2a66b 100644 --- a/openspec/changes/runtime-fixture-runner/tasks.md +++ b/openspec/changes/runtime-fixture-runner/tasks.md @@ -36,11 +36,11 @@ permanently unverified. ## 5. Prove it bites -- [ ] 5.1 Reproduce the current defect first, as a failing test: a runtime rule whose fixtures never ran must not report `ok: true` and must not print a tick -- [ ] 5.2 Test a rule whose `fail/` case produces no findings, which is the silent regression this exists to catch -- [ ] 5.3 Test a rule whose `pass/` case produces findings, which is the indiscriminate rule -- [ ] 5.4 Test each coverage class, asserting only `both` can pass -- [ ] 5.5 Test that an unreadable bucket is an error and not an empty bucket (D5) +- [x] 5.1 Reproduce the current defect first, as a failing test: a runtime rule whose fixtures never ran must not report `ok: true` and must not print a tick +- [x] 5.2 Test a rule whose `fail/` case produces no findings, which is the silent regression this exists to catch +- [x] 5.3 Test a rule whose `pass/` case produces findings, which is the indiscriminate rule +- [x] 5.4 Test each coverage class, asserting only `both` can pass +- [x] 5.5 Test that an unreadable bucket is an error and not an empty bucket (D5) - [ ] 5.6 Revert the runner and watch the suite fail before believing it ## 6. Say so where an author reads it diff --git a/packages/cli/test/runtime-fixture-runner.test.ts b/packages/cli/test/runtime-fixture-runner.test.ts new file mode 100644 index 00000000..d9cf9a99 --- /dev/null +++ b/packages/cli/test/runtime-fixture-runner.test.ts @@ -0,0 +1,448 @@ +import { execFile } from "node:child_process"; +import { chmod, mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { promisify } from "node:util"; + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +/** + * `taskless test` against a runtime rule. + * + * The defect this file exists for: `test` printed `✓ runtime/` and counted + * the rule among those tested for a rule whose fixtures never ran. Every + * assertion here is about the difference between running a rule and saying + * one ran, so none of them may be relaxed into "does not crash". + * + * These spawn the built CLI rather than calling `testOneRule`, because the tick + * and the summary line are half the defect and only the command prints them. + */ + +const execFileAsync = promisify(execFile); +const binPath = resolve(import.meta.dirname, "../dist/index.js"); + +let cwd: string; + +async function runCli(args: string[]) { + try { + const { stdout, stderr } = await execFileAsync("node", [binPath, ...args]); + return { stdout, stderr, exitCode: 0 }; + } catch (error) { + const failure = error as { stdout: string; stderr: string; code: number }; + return { + stdout: failure.stdout ?? "", + stderr: failure.stderr ?? "", + exitCode: failure.code, + }; + } +} + +interface Report { + ok: boolean; + rules: { + engine: string; + ruleId: string; + ok: boolean; + errors: string[]; + ran?: boolean; + refused?: string; + }[]; +} + +/** A capture matching `eval(...)`, which every fixture below is written around. */ +const EVAL_CAPTURE = [ + "id: no-eval-abc12345", + "language: typescript", + "rule:", + " pattern: eval($ARG)", + "metadata:", + " taskless:", + " version: 1", + " kind: runtime", + " name: no-eval", + " check: check.ts", + " match: anchor", + "", +].join("\n"); + +/** + * The realistic shape: the capture narrows to `eval(...)`, and the check + * decides. It reports only where the argument is not a literal, which is the + * judgement a runtime rule exists to make and an ast-grep pattern cannot. + * + * This is what makes a `pass/` case possible at all. A pass case has to MATCH + * the narrow, or `check.ts` is never invoked and the case proves nothing about + * the check staying quiet — so a check that flagged every match would have no + * passing fixture that ran. + */ +const FLAGS_DYNAMIC_EVAL = String.raw`import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +export default async function (root, matches) { + return matches + .filter((m) => !readFileSync(join(root, m.file), "utf8").includes('eval("')) + .map((m) => ({ + file: m.file, + line: m.line, + column: m.column, + message: "eval on a non-literal is not allowed", + severity: "warning", + })); +} +`; + +/** Reports one finding per match: the indiscriminate rule, which cannot pass. */ +const FLAGS_EVERY_MATCH = `export default async function (root, matches) { + return matches.map((m) => ({ + file: m.file, + line: m.line, + column: m.column, + message: "eval is not allowed", + severity: "warning", + })); +} +`; + +/** Reaches the check and deliberately reports nothing. */ +const FLAGS_NOTHING = `export default async function () { + return []; +} +`; + +/** Reaches the check and dies there, which is not "found nothing". */ +const THROWS = `export default async function () { + throw new Error("check exploded"); +} +`; + +const RULE = "no-eval"; + +function ruleDirectory(): string { + return join(cwd, ".taskless", "rules", "runtime", RULE); +} + +/** Write the rule itself: one capture and one `check.ts`, and no fixtures. */ +async function writeRule(check: string = FLAGS_DYNAMIC_EVAL): Promise { + const directory = ruleDirectory(); + await mkdir(join(directory, "captures"), { recursive: true }); + await writeFile(join(directory, "captures", "eval.yml"), EVAL_CAPTURE); + await writeFile(join(directory, "check.ts"), check); +} + +/** + * A fixture case: a DIRECTORY holding source, which is the `root` the check is + * given. + * + * Two axes, and keeping them apart is the point of D8. `matching` decides + * whether the NARROW finds anything, which is a property of the fixture; + * `flagged` decides whether the CHECK reports on what the narrow found, which + * is a property of the rule. A case can match and not be flagged (the useful + * `pass/` case), or fail to match at all (a fixture defect in either bucket). + */ +async function writeCase( + bucket: "pass" | "fail", + name: string, + options: { matching?: boolean; flagged?: boolean } = {} +): Promise { + const matching = options.matching ?? true; + const flagged = options.flagged ?? bucket === "fail"; + const directory = join(ruleDirectory(), ".tests", bucket, name); + await mkdir(directory, { recursive: true }); + const source = matching + ? flagged + ? "const input = globalThis.userInput;\neval(input);\n" + : 'eval("1 + 1");\n' + : "const total = 1 + 1;\n"; + await writeFile(join(directory, "sample.ts"), source); + return directory; +} + +async function testRule(...extra: string[]) { + return runCli([ + "test", + `.taskless/rules/runtime/${RULE}`, + "-d", + cwd, + ...extra, + ]); +} + +async function testRuleJson(...extra: string[]): Promise { + const { stdout } = await testRule("--json", ...extra); + return JSON.parse(stdout) as Report; +} + +beforeEach(async () => { + cwd = await mkdtemp(join(tmpdir(), "tskl-rt-runner-")); + await runCli(["init", "--no-interactive", "-d", cwd]); +}); + +afterEach(async () => { + await rm(cwd, { recursive: true, force: true }); +}); + +/** + * 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. + */ +describe("a runtime rule whose fixtures did not run", () => { + it("is not reported as passing, and prints no tick", async () => { + await writeRule(); + await writeCase("fail", "uses-eval"); + await writeCase("pass", "no-eval"); + + 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); + expect(stdout).not.toContain("1 rule(s) tested"); + }); + + it("says the fixtures did not run, and names the flag that would run them", async () => { + await writeRule(); + await writeCase("fail", "uses-eval"); + await writeCase("pass", "no-eval"); + + const { stdout } = await testRule(); + + expect(stdout).toContain("did not run"); + expect(stdout).toContain("--dangerously-run-scripts"); + }); + + it("reports ran: false in --json, and does not report ok: true", async () => { + await writeRule(); + await writeCase("fail", "uses-eval"); + await writeCase("pass", "no-eval"); + + const report = await testRuleJson(); + const rule = report.rules[0]; + + 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); + }); +}); + +/** 3.3 / 3.4 — the gate is the gate, and nothing about a fixture softens it. */ +describe("the execution gate", () => { + it("executes nothing for an unblessed rule", async () => { + // A check that writes a file is the only way to prove non-execution: the + // absence of findings is what a gated-out run and a clean run share. + const witness = join(cwd, "witness.txt"); + await writeRule( + `import { writeFileSync } from "node:fs"; +export default async function () { + writeFileSync(${JSON.stringify(witness)}, "ran"); + return []; +} +` + ); + await writeCase("fail", "uses-eval"); + await writeCase("pass", "no-eval"); + + await testRule(); + + await expect( + import("node:fs/promises").then((fs) => fs.readFile(witness, "utf8")) + ).rejects.toThrow(); + }); + + it("does not exempt a rule for its id", async () => { + // The demo rule id gets no special treatment; only the flag or a blessing + // does. Same assertion as above under the name a bypass would have used. + await writeRule(); + await writeCase("fail", "uses-eval"); + await writeCase("pass", "no-eval"); + + const report = await testRuleJson(); + + expect(report.rules[0]?.ran).toBe(false); + }); + + it("runs the fixtures under --dangerously-run-scripts, with the warning", async () => { + await writeRule(); + await writeCase("fail", "uses-eval"); + await writeCase("pass", "no-eval"); + + const { stdout, stderr, exitCode } = await testRule( + "--dangerously-run-scripts" + ); + + expect(stderr).toContain("--dangerously-run-scripts is executing runtime"); + expect(stdout).toContain(`✓ runtime/${RULE}`); + expect(exitCode).toBe(0); + }); +}); + +/** 5.2 / 5.3 — the two directions a fixture can break. */ +describe("fixture directions", () => { + it("fails a fail/ case that produced no findings", async () => { + await writeRule(FLAGS_NOTHING); + await writeCase("fail", "uses-eval"); + await writeCase("pass", "no-eval"); + + const report = await testRuleJson("--dangerously-run-scripts"); + const rule = report.rules[0]; + + expect(rule?.ran).toBe(true); + expect(rule?.ok).toBe(false); + expect(rule?.errors.join("\n")).toContain("fail/uses-eval"); + expect(report.ok).toBe(false); + }); + + it("fails a pass/ case that produced findings", async () => { + // The indiscriminate rule: it flags every match, so the `pass/` case + // reaches the check and is reported anyway. That is the rule the `pass/` + // bucket exists to catch, and nothing but running it can. + await writeRule(FLAGS_EVERY_MATCH); + await writeCase("fail", "uses-eval"); + await writeCase("pass", "literal-eval"); + + const report = await testRuleJson("--dangerously-run-scripts"); + const rule = report.rules[0]; + + expect(rule?.ran).toBe(true); + expect(rule?.ok).toBe(false); + expect(rule?.errors.join("\n")).toContain("pass/literal-eval"); + }); +}); + +/** + * 2.4 / 2.5 — a case that never reached the check. + * + * The `pass/` half is the quiet one and is the reason this is not just a + * kinder message on the `fail/` side: such a case reads as a clean pass while + * proving only that the narrow did not match. + */ +describe("a case that never reaches the check", () => { + it("is a fixture defect in fail/, not a rule that stopped firing", async () => { + await writeRule(); + await writeCase("fail", "matches-nothing", { matching: false }); + await writeCase("pass", "no-eval"); + + const report = await testRuleJson("--dangerously-run-scripts"); + const errors = report.rules[0]?.errors.join("\n") ?? ""; + + expect(report.rules[0]?.ok).toBe(false); + expect(errors).toContain("fail/matches-nothing"); + expect(errors).toContain("check.ts never ran"); + // The wrong message is the one that sends the author to the check. + expect(errors).not.toContain("fail fixture did not fire"); + }); + + it("is a fixture defect in pass/ too", async () => { + await writeRule(); + await writeCase("fail", "uses-eval"); + await writeCase("pass", "matches-nothing", { matching: false }); + + const report = await testRuleJson("--dangerously-run-scripts"); + const errors = report.rules[0]?.errors.join("\n") ?? ""; + + expect(report.rules[0]?.ok).toBe(false); + expect(errors).toContain("pass/matches-nothing"); + expect(errors).toContain("check.ts never ran"); + }); +}); + +/** 2.3 — a check that throws is not a check that found nothing. */ +describe("a check that throws", () => { + it("is reported as the check failing, not as an empty result", async () => { + await writeRule(THROWS); + await writeCase("fail", "uses-eval"); + await writeCase("pass", "no-eval"); + + const report = await testRuleJson("--dangerously-run-scripts"); + const errors = report.rules[0]?.errors.join("\n") ?? ""; + + expect(report.rules[0]?.ok).toBe(false); + expect(errors).toContain("check failed on fail/uses-eval"); + expect(errors).toContain("check exploded"); + // A throw in `fail/` must not be scored as the case firing correctly, and + // a throw in `pass/` must not be scored as the case staying quiet. + expect(errors).toContain("check failed on pass/no-eval"); + }); +}); + +/** 5.4 — coverage, where only `both` reaches a pass. */ +describe("fixture coverage", () => { + it("passes with both buckets populated", async () => { + await writeRule(); + await writeCase("fail", "uses-eval"); + await writeCase("pass", "no-eval"); + + const report = await testRuleJson("--dangerously-run-scripts"); + + expect(report.rules[0]?.ok).toBe(true); + expect(report.rules[0]?.ran).toBe(true); + }); + + it("fails with only fail/ cases", async () => { + await writeRule(); + await writeCase("fail", "uses-eval"); + + const report = await testRuleJson("--dangerously-run-scripts"); + + expect(report.rules[0]?.ok).toBe(false); + expect(report.rules[0]?.errors.join("\n")).toContain("only fail/"); + }); + + it("fails with only pass/ cases", async () => { + await writeRule(); + await writeCase("pass", "no-eval"); + + const report = await testRuleJson("--dangerously-run-scripts"); + + expect(report.rules[0]?.ok).toBe(false); + expect(report.rules[0]?.errors.join("\n")).toContain("only pass/"); + }); + + it("fails with no fixtures at all", async () => { + await writeRule(); + + const report = await testRuleJson("--dangerously-run-scripts"); + + expect(report.rules[0]?.ok).toBe(false); + expect(report.rules[0]?.errors.join("\n")).toContain("has no fixtures"); + }); +}); + +/** 5.5 — a bucket that cannot be read is not a bucket holding nothing. */ +describe("an unreadable bucket", () => { + it("is an error rather than an empty bucket", async () => { + await writeRule(); + await writeCase("fail", "uses-eval"); + const passBucket = join(ruleDirectory(), ".tests", "pass"); + await mkdir(passBucket, { recursive: true }); + await chmod(passBucket, 0o000); + + try { + const report = await testRuleJson("--dangerously-run-scripts"); + + expect(report.rules[0]?.ok).toBe(false); + // The wrong answer is "only fail/ fixtures": that reads as a rule the + // author half-wrote, when the pass cases may well be sitting right there. + expect(report.rules[0]?.errors.join("\n")).not.toContain("only fail/"); + } finally { + await chmod(passBucket, 0o755); + } + }); + + it("refuses a loose file in a bucket, naming it", async () => { + await writeRule(); + await writeCase("fail", "uses-eval"); + const passBucket = join(ruleDirectory(), ".tests", "pass"); + await mkdir(passBucket, { recursive: true }); + await writeFile(join(passBucket, "loose.ts"), "const x = 1;\n"); + + const report = await testRuleJson("--dangerously-run-scripts"); + + expect(report.rules[0]?.ok).toBe(false); + expect(report.rules[0]?.errors.join("\n")).toContain("loose.ts"); + }); +}); From d76b0ba6e8cdc4b9f9ea9216a862a1be54ba43e6 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 2 Sep 2026 13:23:26 -0700 Subject: [PATCH 6/8] docs(openspec): record the revert measurement for the fixture runner Reverting the runner and the reporting fix, and rebuilding, fails 15 of 1205 tests in one file. Restoring returns 1205 passed. The revert was verified applied before the suite was run, by grepping for the absence of `run-fixtures.ts` and `executeRuntimeRuleDetailed` and for the return of the old renderer and the old `ok: true`. A revert that does not revert makes passing tests look like proof, which has happened here before. --- openspec/changes/runtime-fixture-runner/tasks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openspec/changes/runtime-fixture-runner/tasks.md b/openspec/changes/runtime-fixture-runner/tasks.md index 07b2a66b..38cce2d9 100644 --- a/openspec/changes/runtime-fixture-runner/tasks.md +++ b/openspec/changes/runtime-fixture-runner/tasks.md @@ -41,7 +41,7 @@ permanently unverified. - [x] 5.3 Test a rule whose `pass/` case produces findings, which is the indiscriminate rule - [x] 5.4 Test each coverage class, asserting only `both` can pass - [x] 5.5 Test that an unreadable bucket is an error and not an empty bucket (D5) -- [ ] 5.6 Revert the runner and watch the suite fail before believing it +- [x] 5.6 Revert the runner and watch the suite fail before believing it. **Measured:** reverting the runner and the reporting fix (`git revert --no-commit` of both commits, confirmed applied by grep before running: no `run-fixtures.ts`, no `executeRuntimeRuleDetailed`, renderer back to `result.ok ? "✓" : "✗"`, runtime branch back to `ok: true`) and rebuilding gives **15 failed / 1190 passed of 1205**, one test file failing. Restored: **1205 passed** ## 6. Say so where an author reads it From 4231451ab5be4e061f0e477d9e8d21c646f0fb8d Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 2 Sep 2026 13:25:02 -0700 Subject: [PATCH 7/8] docs(agent): say where the fixtures run, and what stops them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `create-runtime-rule` now states that testing a rule you just wrote needs `--dangerously-run-scripts`, and why the obvious reading of the message is wrong: a locally authored rule has no signature and never will, because blessing is recording and nothing recorded a rule that has not left the working tree. The message says "not authenticated", which reads like logging in would fix it, so the recipe says plainly that it would not. It also states what a passing runtime rule needs: both buckets populated, and every case matching at least one capture. A case the captures do not match never reaches `check.ts` and is a defect in the case, in `pass/` as much as in `fail/`. `verify-rule` stopped describing runtime tests as "reported as not run" with nothing about what would run them. Its engine table now says what `test` runs for a runtime rule, and the reporting section says how a refused run appears: `○` rather than a tick, `ran: false` with `refused` beside it, counted under "did not run", and never on its own a reason for a non-zero exit. --- .../changes/runtime-fixture-runner/tasks.md | 4 +- .../cli/src/agent/create-runtime-rule.txt | 46 +++++++++++++++++-- packages/cli/src/agent/verify-rule.txt | 22 +++++++-- 3 files changed, 61 insertions(+), 11 deletions(-) diff --git a/openspec/changes/runtime-fixture-runner/tasks.md b/openspec/changes/runtime-fixture-runner/tasks.md index 38cce2d9..08ba892c 100644 --- a/openspec/changes/runtime-fixture-runner/tasks.md +++ b/openspec/changes/runtime-fixture-runner/tasks.md @@ -45,8 +45,8 @@ permanently unverified. ## 6. Say so where an author reads it -- [ ] 6.1 `create-runtime-rule` states that testing a locally authored rule needs the flag, and why (D6) -- [ ] 6.2 `verify-rule` stops reporting runtime tests as "not run" without saying what was not run +- [x] 6.1 `create-runtime-rule` states that testing a locally authored rule needs the flag, and why (D6) +- [x] 6.2 `verify-rule` stops reporting runtime tests as "not run" without saying what was not run ## 7. Close out diff --git a/packages/cli/src/agent/create-runtime-rule.txt b/packages/cli/src/agent/create-runtime-rule.txt index c0b66ccd..36142034 100644 --- a/packages/cli/src/agent/create-runtime-rule.txt +++ b/packages/cli/src/agent/create-runtime-rule.txt @@ -94,6 +94,41 @@ runtime rule it finds is listed as skipped with the reason So a runtime rule you write now is inert until the user logs in. Say that plainly rather than letting them discover it from a silent check. +## Testing a rule you wrote yourself needs the flag + +`%(TASKLESS_CLI)s test` runs a runtime rule's fixtures by executing its +`check.ts` against each case directory, so it runs them under the same +gate `check` uses. For a rule you just authored, that gate never opens +on its own: + +``` +%(TASKLESS_CLI)s test .taskless/rules/runtime/ --dangerously-run-scripts +``` + +**A locally authored rule has no signature and never will.** Blessing is +recording: a signature is blessed because a reconcile reported it and +the service recorded it, and nothing recorded a rule that has not left +the working tree. Logging in does not change this, which is the part +worth saying out loud, because "not authenticated" is the reason the +message gives and it reads like the fix. + +The friction is the correct friction. `sg` and `vale` authors do not +face it because their rules are unexecuted. A delivered rule and one you +wrote a minute ago are indistinguishable on disk, so an exemption for +"my own rule" would be an exemption for any rule, and the warning the +flag prints is accurate in both cases. + +Without the flag `test` prints `○` for the rule, says the fixtures did +not run, names the flag, and exits 0. That is neither a pass nor a +failure, and it is deliberately not a failure: nothing the holder of an +unblessed rule can do would make a failure green. + +Both buckets have to be populated for the rule to pass, and every case +has to match at least one capture. A case the captures do not match +never reaches `check.ts`, and is reported as a defect in the case rather +than in the rule, in `pass/` as well as `fail/`: such a case proves the +capture did not match, which is a fact about the fixture. + ## Steps 1. **Tell the user what their rule needs, and why it is gated.** Name @@ -121,11 +156,12 @@ that plainly rather than letting them discover it from a silent check. ## Important Notes -- `--dangerously-run-scripts` makes `check` execute runtime rules - without server verification. It exists for local iteration on a rule - you wrote yourself and just read. It is not a way to ship a rule to a - team, and suggesting it to work around a login turns a deliberate gate - into an unreviewed code-execution path on someone else's machine. +- `--dangerously-run-scripts` makes `check` and `test` execute runtime + rules without server verification. It exists for local iteration on a + rule you wrote yourself and just read, which includes running its + fixtures. It is not a way to ship a rule to a team, and suggesting it + to work around a login turns a deliberate gate into an unreviewed + code-execution path on someone else's machine. - Do not author a `check.ts` and leave it in the repository unmentioned. A skipped runtime rule reports nothing, which reads exactly like a passing one. diff --git a/packages/cli/src/agent/verify-rule.txt b/packages/cli/src/agent/verify-rule.txt index b9d511ce..c43540cc 100644 --- a/packages/cli/src/agent/verify-rule.txt +++ b/packages/cli/src/agent/verify-rule.txt @@ -35,7 +35,19 @@ What each engine is checked for: |-----------|--------------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------| | `sg` | ast-grep schema, plus `id`/`language`/`severity`/`message`/`rule`, and `regex` accompanied by `kind` | the `valid`/`invalid` cases in `.tests/` | | `vale` | style parses, `extends` and `message` present, `level` in vocabulary, and the rule's `.vale.ini` enables `.` under a matcher | the `.tests/pass/` and `.tests/fail/` buckets | -| `runtime` | `check.ts` present, at least one capture rule in `captures/` | reported as not run: the `.tests/pass/` and `.tests/fail/` cases need the server harness | +| `runtime` | `check.ts` present, at least one capture rule in `captures/` | each directory under `.tests/pass/` and `.tests/fail/`, as the check's `root` | + +A runtime rule's fixtures execute its `check.ts`, so they run only under +the policy `check` already applies: an authenticated reconcile that +returns the rule's signature, or `--dangerously-run-scripts`. When +neither holds, 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 tested, and on its own it does not fail the command. + +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. ## What a path means @@ -70,10 +82,12 @@ Both commands answer in the same shape, one entry per rule: - `ok` at the top is true only when every rule passed. - `errors` is one string per problem, written to be read by a person. - `test` adds `ran`, which is false when `verify` failed and the tests - never executed. + never executed, and false for a runtime rule the execution policy + refused. The second case also carries `refused` with the reason, and + is the one where `ok` alone would mislead you. -Without `--json` the same information prints as a `✓`/`✗` line per rule -with its errors indented beneath. +Without `--json` the same information prints as a `✓`/`✗`/`○` line per +rule with its errors indented beneath. `○` is a rule that did not run. ## Exit codes From 7c7bb8af5800fefa6c12a35935630f72f78d2c6d Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 2 Sep 2026 21:25:44 -0700 Subject: [PATCH 8/8] fix(test): the verb is the consent, so test asks no server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `createRuntimeGate` hardcoded `anonymous: false`, so `taskless test` called `getToken` and, for any project holding a runtime rule, went on to `resolveOrgSubject` and `reconcile` over the network. `test` was fully offline before this change. The fix is not `--anonymous`. It is to take reconcile out of `test` entirely. `check` reconciles because it executes rules as a SIDE EFFECT of scanning a repository: nobody asked for code to run, so a gate has to stand between the request and the execution. `test` runs fixtures because the user asked it to, and the verb is the consent — asking a server for permission to run your own fixtures is overreach. The only party a reconcile here could ever admit is someone testing an already-blessed delivered rule, which the service verified before delivering it; a locally authored rule has no signature and never will, so for the audience that actually runs `test` on a runtime rule it is a round trip whose answer is always "no". This is strictly more conservative. Nothing executes that would not have executed before, and a blessed rule that `check` runs unflagged now needs `--dangerously-run-scripts` here. It also disposes of the `--anonymous` question rather than answering it: with no network there is nothing to suppress. `check`'s gate, its reconcile, and its use of `planRuntime` are untouched. `createRuntimeGate`/`RuntimeGate` were `test`-only and are deleted. `planRuntime` and `repairWithheldRules` stay in `rules/runtime/plan.ts`, `check`-only: the move's original justification is gone, but policy still does not belong in a command file. The refusal message no longer inherits `check`'s "not authenticated", which read like a fix and was not one. Also extracts the fixture reading that had been copied a third time. Vale's `directoryEntries` claimed to be "the single place that decides which `readdir` failures are absence and which are problems, so no caller can accidentally answer that question differently", and the runtime copy made that false. The missing-vs-unreadable discrimination and the four-way coverage classification with its message now live in `rules/fixtures.ts`, and all three engines call it. Each engine's rejection stays local, because they are opposites: Vale refuses a nested DIRECTORY, runtime refuses a non-directory. The coverage message's `valid:` versus `pass/` looked like drift and is not — ast-grep's buckets are YAML keys and the other two are directories — so the suffix is a parameter rather than something normalised away. Behaviour is unchanged: 1205 tests before, 1206 after, the one addition being the new test that the refusal never says `auth login`. --- .../changes/runtime-fixture-runner/design.md | 119 +++++++++++++-- .../specs/cli-rule-validation/spec.md | 20 ++- .../specs/cli-runtime-rule-execution/spec.md | 47 +++--- .../changes/runtime-fixture-runner/tasks.md | 29 ++-- .../cli/src/agent/create-runtime-rule.txt | 31 ++-- packages/cli/src/commands/verify.ts | 40 +++--- packages/cli/src/rules/fixtures.ts | 100 +++++++++++++ packages/cli/src/rules/inspect.ts | 135 +++++++++++++----- packages/cli/src/rules/runtime/fixtures.ts | 66 +++------ packages/cli/src/rules/runtime/harness.ts | 12 ++ packages/cli/src/rules/runtime/plan.ts | 105 +++----------- .../cli/src/rules/runtime/run-fixtures.ts | 12 +- packages/cli/src/rules/vale/verify.ts | 69 ++++----- packages/cli/src/rules/verify.ts | 19 +-- .../cli/test/runtime-fixture-runner.test.ts | 17 +++ 15 files changed, 522 insertions(+), 299 deletions(-) create mode 100644 packages/cli/src/rules/fixtures.ts diff --git a/openspec/changes/runtime-fixture-runner/design.md b/openspec/changes/runtime-fixture-runner/design.md index 3e6ffa2d..9f911605 100644 --- a/openspec/changes/runtime-fixture-runner/design.md +++ b/openspec/changes/runtime-fixture-runner/design.md @@ -26,24 +26,63 @@ Coverage is a four-way classification rather than a boolean, because **Goals.** Run a runtime rule's fixtures. Report honestly when they did not run. Make `.tests/` mean something for the one tier that ships them and reads none. -**Non-Goals.** Changing the execution gate. Adding a runtime-specific bypass. -Making `check` run fixtures — `check` scans a repository, `test` runs fixtures, -and that separation is why the gate can be shared without the commands merging. +**Non-Goals.** Loosening the execution gate anywhere. Adding a runtime-specific +bypass. Touching `check`'s behaviour at all — its gate, its reconcile and its +use of `planRuntime` are exactly what they were. Making `check` run fixtures: +`check` scans a repository, `test` runs fixtures, and that separation is why +the two can hold the same flag without holding the same policy (D1). ## Decisions -### D1 — The gate is `check`'s gate, unchanged - -A fixture run executes `check.ts`. That is the same code, from the same -delivery, with the same signature, as the code `check` runs against a -repository. So it runs under the same policy: blessed by an authenticated -reconcile, or `--dangerously-run-scripts`. +### D1 — `test` takes the flag half of `check`'s gate, and no reconcile at all Nothing about a fixture makes the code safer. The bytes do not know what directory they are pointed at, and "it is only running against test data" is a -statement about the input, not about what the program may do. A separate, -softer gate for fixtures would be the client-side bypass the runtime spec -already forbids, arrived at by a different route. +statement about the input, not about what the program may do. So a fixture run +is gated, and `--dangerously-run-scripts` is that gate — the same flag `check` +carries, spelled the same way, printing the same warning. + +**What `test` does not do is reconcile.** The first draft of this change shared +`check`'s whole gate, which meant `test` called `getToken`, and on a project +holding any runtime rule went on to `resolveOrgSubject` and `reconcile` over +the network. `test` had been fully offline before this change, and that is the +half that was wrong. + +The two commands execute rules for different reasons, and the gate belongs to +the reason rather than to the code: + +- `check` executes rules as a **side effect** of scanning a repository. Nobody + asked for code to run; they asked for a report. The gate is what stands + between the request and the execution, so that code never runs silently. +- `test` runs fixtures **because the user asked it to**. The verb is the + consent. Asking a server for permission to run your own fixtures is + overreach, and it is not the kind of overreach that buys safety. + +Ask who a reconcile in `test` would ever admit, and the answer is: someone +testing an already-blessed delivered rule — a rule the service verified against +failing and passing examples on its way to accepting it. That is the one +audience for whom running the fixtures locally proves least. Meanwhile a +locally authored rule has no signature and never will, because blessing is +recording and nothing recorded it (D6), so for the audience that actually runs +`test` on a runtime rule the reconcile is pure cost paid for an answer that is +always "no". + +**This is strictly more conservative, not a softening.** Nothing executes under +`test` that would not have executed before, and one thing that would have — +a blessed delivered rule, running with no flag — now requires the flag. There +is no security argument against it, because there is no case where the new +behaviour runs code the old behaviour refused. + +It also disposes of the `--anonymous` question rather than answering it. The +review asked whether `test` should carry `check`'s `--anonymous` flag to +suppress the network call. With no network there is nothing to suppress, and a +flag whose only job is to turn off a call that should not have been there is a +worse answer than not making the call. + +The one property that had to survive is that the fixture path is not a _softer_ +gate than the scan path, since that would be the client-side bypass the runtime +spec forbids reached by a different route. It survives by being a _stricter_ +one. ### D2 — A run that did not happen is a third state, not a pass and not a failure @@ -111,13 +150,22 @@ one-sided rule look complete. The same applies here and is worth stating rather than inheriting by imitation: a fixture bucket that could not be read is an error, never an empty bucket. -### D6 — Authoring a runtime rule locally now requires the flag +### D6 — Testing a runtime rule requires the flag, authored or delivered -A locally authored rule has no signature and never will, because blessing is -recording and nothing recorded it. So its author must pass +Under D1 the flag is the whole gate for `test`, so this is now true of every +runtime rule rather than only of unblessed ones. The authored case is still the +one worth stating, because it is the one where a reader will look for a way +out and find none: a locally authored rule has no signature and never will, +because blessing is recording and nothing recorded it. Its author must pass `--dangerously-run-scripts` to test their own rule, which `sg` and `vale` authors do not have to do. +**The message must not send them to `auth login`.** The first draft inherited +`check`'s "not authenticated" reason, which reads like the fix and is not one: +authenticating would not have blessed a rule that never left the working tree. +With reconcile gone the message names the flag and nothing else, and a test +asserts the absence. + That is friction and it is the correct friction. The alternative is a rule that executes unblessed code because it happens to live in the working tree, which is exactly the property the gate exists to deny — a delivered rule and an authored @@ -195,6 +243,47 @@ either bucket, naming the case and saying the check never ran. It is actionable in a way "expected a finding, got none" is not, and it points at the file the author has to change. +### D9 — The third fixture reader is the one that had to be shared + +The Vale runner is the model this change copied (see Context), and copying it +verbatim is what made the copying a problem. `rules/vale/verify.ts` carried a +`directoryEntries` whose doc comment called itself "the single place that +decides which `readdir` failures are absence and which are problems, so no +caller can accidentally answer that question differently" — a claim a third +near-identical copy falsified the day it was written. + +The drift was already measurable rather than hypothetical: the "half a claim" +coverage message existed in three places. + +So `rules/fixtures.ts` now holds the two decisions that are genuinely the same +across `sg`, `vale` and `runtime` — the missing-versus-unreadable +discrimination, and the four-way coverage classification with its message — and +all three engines call it. + +**What stayed local is what is genuinely different, and it is the rejection.** +Vale's buckets hold documents, so a nested DIRECTORY is its error; runtime's +hold one directory per case, so a loose FILE is. Those are opposite rules for +the same-shaped read, and folding them together would have needed a flag that +made the shared function harder to read than the two callers it replaced. + +One thing looked like drift and was not. The coverage message spells the bucket +`valid:` for `sg` and `pass/` for the other two. That is not two punctuations +for one idea: ast-grep's buckets are keys in a test YAML document and the other +two are directories, so each names the bucket in the shape its author will go +looking for. It is a parameter of the shared message rather than something +normalised away. + +The classification is parameterised on the bucket names for the same reason +(`FixtureCoverage<"valid" | "invalid">` against `FixtureCoverage<"pass" | +"fail">`), so the three engines share one set of four states under their own +vocabularies rather than sharing a vocabulary they do not have. + +`sg`'s own test-file enumeration was left alone deliberately. It catches every +`readdir` failure as "no test files", which is the leniency this module exists +to prevent — but fixing it changes what `sg` reports on an unreadable `.tests/`, +and that is a behaviour change wearing a refactor's clothes. It belongs in its +own change, with its own test. + ## Risks / Trade-offs **A rule with no fixtures fails, and that is the decision.** The other two diff --git a/openspec/changes/runtime-fixture-runner/specs/cli-rule-validation/spec.md b/openspec/changes/runtime-fixture-runner/specs/cli-rule-validation/spec.md index 1e478376..33afe2fc 100644 --- a/openspec/changes/runtime-fixture-runner/specs/cli-rule-validation/spec.md +++ b/openspec/changes/runtime-fixture-runner/specs/cli-rule-validation/spec.md @@ -8,7 +8,11 @@ Ordering is the point. When a rule is both malformed and under-fixtured, the fix 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` reports an empty `invalid:` bucket as `1 passed; 0 failed` and exits zero, so a rule that has never matched anything is indistinguishable from one that passed. -A runtime rule's fixtures execute delivered code, so they SHALL run only under the policy `check` already applies: an authenticated reconcile that returns the rule's signature in `run`, or `--dangerously-run-scripts`. A run refused by that policy 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. +A runtime rule's fixtures execute code, so they SHALL run only when `--dangerously-run-scripts` is passed, and under no other mechanism. `test` 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. `test` 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 `test` 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. #### Scenario: A malformed rule reports the malformation, not the fixtures @@ -33,20 +37,28 @@ A runtime rule's fixtures execute delivered code, so they SHALL run only under t #### Scenario: Runtime fixtures are run per case -- **WHEN** `test` runs against a runtime rule and the execution policy permits it +- **WHEN** `test` 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 the policy refuses is reported as not run +#### Scenario: A runtime rule without the flag is reported as not run -- **WHEN** `test` runs against a runtime rule with no blessed signature and no `--dangerously-run-scripts` +- **WHEN** `test` 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 tested - **AND** the refusal alone SHALL NOT fail the command +#### Scenario: Testing a runtime rule reaches no network + +- **WHEN** `test` 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 diff --git a/openspec/changes/runtime-fixture-runner/specs/cli-runtime-rule-execution/spec.md b/openspec/changes/runtime-fixture-runner/specs/cli-runtime-rule-execution/spec.md index 57af3e8b..cb4f837f 100644 --- a/openspec/changes/runtime-fixture-runner/specs/cli-runtime-rule-execution/spec.md +++ b/openspec/changes/runtime-fixture-runner/specs/cli-runtime-rule-execution/spec.md @@ -37,26 +37,41 @@ one-sided rule look complete. - **THEN** the CLI SHALL report the failure - **AND** SHALL NOT treat the bucket as holding no cases -### Requirement: Fixture execution obeys the runtime execution gate - -Executing a fixture case SHALL be permitted only where executing the rule itself -would be: when an authenticated reconcile returns the rule's signature in `run`, -or when `--dangerously-run-scripts` is passed. No rule identifier SHALL be -exempt, and the fixture path SHALL NOT constitute a separate or softer gate. - -**Rationale.** A fixture run executes the same `check.ts`, from the same -delivery, under the same signature as a scan. That the input is test data is a -statement about the input, not about what the program may do. A softer gate for -fixtures would be the client-side bypass this capability already forbids, -reached by another route. - -#### Scenario: Fixtures do not run for an unblessed rule - -- **WHEN** `test` runs against a runtime rule whose signature no reconcile has blessed, without the escape flag +### Requirement: Fixture execution is gated on the escape flag alone + +Executing a fixture case SHALL require `--dangerously-run-scripts`, and SHALL be +permitted under no other mechanism. No rule identifier SHALL be exempt, and a +blessed signature SHALL NOT substitute for the flag. Deciding this SHALL NOT +involve the rule service: no token SHALL be read, no organization resolved, and +no reconcile performed. + +**Rationale.** That the input is test data is a statement about the input, not +about what the program may do, so the fixture path SHALL NOT be a softer gate +than the scan path. It is a stricter one, and for a reason that is about consent +rather than about the bytes. A scan executes rules as a **side effect** of a +request for a report, so a gate has to stand between the two or code runs +silently; that gate is what a reconcile serves. Running fixtures is the request +itself, so the verb is the consent and the flag is the confirmation. + +Reconciling here would also buy nothing. The only rule it could ever admit is +one the service already verified before delivering it, while a locally authored +rule has no signature and never will — so for the audience that runs fixtures it +is a network round trip whose answer is always "no". + +#### Scenario: Fixtures do not run without the flag + +- **WHEN** `test` runs against a runtime rule without `--dangerously-run-scripts` - **THEN** no fixture case SHALL be executed +- **AND** this SHALL hold irrespective of authentication state or any signature the rule carries #### Scenario: The documented escape runs fixtures - **WHEN** `test` runs with `--dangerously-run-scripts` - **THEN** fixture cases SHALL execute under that flag's existing warning - **AND** under no other mechanism + +#### Scenario: A scan's gate is unaffected + +- **WHEN** `check` scans a repository holding a runtime rule +- **THEN** it SHALL apply its existing policy unchanged: a signature returned in `run` by an authenticated reconcile, or `--dangerously-run-scripts` +- **AND** the fixture runner SHALL NOT alter what a scan executes diff --git a/openspec/changes/runtime-fixture-runner/tasks.md b/openspec/changes/runtime-fixture-runner/tasks.md index 08ba892c..5c7d4ff5 100644 --- a/openspec/changes/runtime-fixture-runner/tasks.md +++ b/openspec/changes/runtime-fixture-runner/tasks.md @@ -19,12 +19,14 @@ permanently unverified. - [x] 2.4 Distinguish a case that never reached the check from one where the check found nothing (D8). `executeRuntimeRule` gates on the narrow and returns `[]` without invoking `check.ts`, so the runner needs the invocation signal rather than only the findings - [x] 2.5 Report a case producing no narrow matches as a fixture defect in BOTH buckets, naming the case and saying the check never ran. A `pass/` case that never invokes the check proves nothing about the check staying quiet -## 3. Gate it exactly as `check` does +## 3. Gate it on the flag, and only the flag -- [x] 3.1 Run fixtures only when an authenticated reconcile returns the rule's signature in `run`, or when `--dangerously-run-scripts` is passed (D1) -- [x] 3.2 Add `--dangerously-run-scripts` to `test`, printing the same warning `check` prints +- [x] 3.1 Run fixtures only when `--dangerously-run-scripts` is passed. `test` does NOT reconcile: it runs fixtures because the user asked it to, so the verb is the consent (D1). `check`'s gate is untouched +- [x] 3.2 Add `--dangerously-run-scripts` to `test`, printing the same warning `check` prints, from one shared spelling so the two cannot drift - [x] 3.3 Test that no rule id is special-cased, and that a gated-out fixture run executes nothing -- [x] 3.4 Test that the flag is the only mechanism besides a blessed signature. A fixture path is not a softer gate because its input is test data +- [x] 3.4 Test that the flag is the only mechanism at all. A fixture path is not a softer gate because its input is test data — and with no reconcile it is a stricter one, since a blessed rule that `check` runs unflagged still needs the flag here +- [x] 3.5 Test that the refusal never sends the author to `auth login`. Authenticating cannot bless a rule that never left the working tree, so naming it would be a fix that is not one (D6) +- [x] 3.6 Confirm nothing was orphaned by dropping reconcile from `test`. `createRuntimeGate`/`RuntimeGate` were `test`-only and are deleted; `planRuntime` and `repairWithheldRules` stay, `check`-only, in `rules/runtime/plan.ts` ## 4. Report a run that did not happen @@ -48,10 +50,17 @@ permanently unverified. - [x] 6.1 `create-runtime-rule` states that testing a locally authored rule needs the flag, and why (D6) - [x] 6.2 `verify-rule` stops reporting runtime tests as "not run" without saying what was not run -## 7. Close out +## 7. Share the bucket reader rather than copying it a third time -- [x] 7.1 Decide the no-fixtures case. **Settled: it fails, matching the other engines.** Every rule needs a fixture. Depth is not policed, so a trivial case that exercises little and passes is acceptable, but it can only live in `pass/` — a `fail/` case that produces nothing is the silent regression this runner exists to catch -- [ ] 7.2 Tell the generator team, who have recorded this as a limit on what the demonstration can assert -- [ ] 7.3 Confirm runtime deliveries carry `.tests/`. Not a request: a runtime rule ships with fixtures, and a delivery without them is a defect on their side (D7). If any do not, file it as a bug rather than proposing it as a contract change -- [ ] 7.4 Once 7.3 confirms deliveries carry them, add fixtures to delivery completeness as its own change. Sequenced only so a service defect does not reach users as a refused write with nothing they can do about it -- [ ] 7.5 Archive the change +- [x] 7.1 Extract the missing-versus-unreadable `readdir` discrimination and the four-way coverage classification to `rules/fixtures.ts`, and point `sg`, `vale` and `runtime` at it (D9) +- [x] 7.2 Extract the "half a claim" coverage message, which existed in three places. Keep the bucket suffix a parameter: `valid:` is a YAML key and `pass/` is a directory, so the difference is meaning rather than drift +- [x] 7.3 Keep each engine's rejection local. Vale refuses a nested DIRECTORY because its buckets hold documents; runtime refuses a non-directory because its buckets hold case directories +- [x] 7.4 Change no engine's observable behaviour. **Measured:** 75 files / 1205 tests before, 75 files / 1206 after, the one addition being 3.5's new test. `pnpm cli check` reports the same 4 pre-existing warnings either side + +## 8. Close out + +- [x] 8.1 Decide the no-fixtures case. **Settled: it fails, matching the other engines.** Every rule needs a fixture. Depth is not policed, so a trivial case that exercises little and passes is acceptable, but it can only live in `pass/` — a `fail/` case that produces nothing is the silent regression this runner exists to catch +- [ ] 8.2 Tell the generator team, who have recorded this as a limit on what the demonstration can assert +- [ ] 8.3 Confirm runtime deliveries carry `.tests/`. Not a request: a runtime rule ships with fixtures, and a delivery without them is a defect on their side (D7). If any do not, file it as a bug rather than proposing it as a contract change +- [ ] 8.4 Once 8.3 confirms deliveries carry them, add fixtures to delivery completeness as its own change. Sequenced only so a service defect does not reach users as a refused write with nothing they can do about it +- [ ] 8.5 Archive the change diff --git a/packages/cli/src/agent/create-runtime-rule.txt b/packages/cli/src/agent/create-runtime-rule.txt index 36142034..3a485a82 100644 --- a/packages/cli/src/agent/create-runtime-rule.txt +++ b/packages/cli/src/agent/create-runtime-rule.txt @@ -97,20 +97,26 @@ that plainly rather than letting them discover it from a silent check. ## Testing a rule you wrote yourself needs the flag `%(TASKLESS_CLI)s test` runs a runtime rule's fixtures by executing its -`check.ts` against each case directory, so it runs them under the same -gate `check` uses. For a rule you just authored, that gate never opens -on its own: +`check.ts` against each case directory. The flag is the whole gate, for +every rule, always: ``` %(TASKLESS_CLI)s test .taskless/rules/runtime/ --dangerously-run-scripts ``` -**A locally authored rule has no signature and never will.** Blessing is -recording: a signature is blessed because a reconcile reported it and -the service recorded it, and nothing recorded a rule that has not left -the working tree. Logging in does not change this, which is the part -worth saying out loud, because "not authenticated" is the reason the -message gives and it reads like the fix. +**`test` does not talk to the rule service at all**, and there is no +blessed-signature path that runs fixtures without the flag. That makes +it stricter than `check`, which will execute a blessed rule with no flag +because it runs rules as a side effect of scanning a repository. `test` +runs them because you asked it to, so the verb is the consent and the +flag is the confirmation; nothing is asked of a server in between. + +**A locally authored rule has no signature and never will**, so this was +never going to be avoidable for a rule you wrote. Blessing is recording: +a signature is blessed because a reconcile reported it and the service +recorded it, and nothing recorded a rule that has not left the working +tree. Do not send anyone to `auth login` over this. It changes nothing +here, and the message does not mention it. The friction is the correct friction. `sg` and `vale` authors do not face it because their rules are unexecuted. A delivered rule and one you @@ -159,9 +165,10 @@ capture did not match, which is a fact about the fixture. - `--dangerously-run-scripts` makes `check` and `test` execute runtime rules without server verification. It exists for local iteration on a rule you wrote yourself and just read, which includes running its - fixtures. It is not a way to ship a rule to a team, and suggesting it - to work around a login turns a deliberate gate into an unreviewed - code-execution path on someone else's machine. + fixtures. On `test` it is the only mechanism there is. It is not a way + to ship a rule to a team, and suggesting it to work around a login + turns a deliberate gate into an unreviewed code-execution path on + someone else's machine. - Do not author a `check.ts` and leave it in the repository unmentioned. A skipped runtime rule reports nothing, which reads exactly like a passing one. diff --git a/packages/cli/src/commands/verify.ts b/packages/cli/src/commands/verify.ts index e7731cf8..b19a30c6 100644 --- a/packages/cli/src/commands/verify.ts +++ b/packages/cli/src/commands/verify.ts @@ -15,7 +15,6 @@ import { resolveRulePath, RuleNotFoundError, } from "../rules/resolve-path"; -import { createRuntimeGate } from "../rules/runtime/plan"; import { outputSchema as verifyTestOutputSchema } from "../schemas/verify-test"; import { makeErrorEnvelope, writeJsonError } from "../types/errors"; import { CLIError } from "../util/cli-error"; @@ -131,11 +130,9 @@ async function runOverPath(options: { // for every project holding a runtime rule. It is excluded from the failures // that set the exit code and from the count of rules tested, and gets its // own marker below. - const refused = results.filter( - (result) => "refused" in result && result.refused !== undefined - ); const isRefused = (result: RuleVerification | RuleTestResult): boolean => "refused" in result && result.refused !== undefined; + const refused = results.filter((result) => isRefused(result)); const failed = results.filter((result) => !result.ok && !isRefused(result)); const tested = results.length - refused.length; @@ -228,34 +225,41 @@ export const testCommand = defineCommand({ }, args: { ...ruleTargetArguments, - // The same flag `check` carries, with the same description, because it is - // the same gate. A runtime rule's fixtures execute its `check.ts`, and - // that the input is test data is a statement about the input rather than - // about what the program may do. + // The same flag `check` carries, spelled the same way. Here it is the ONLY + // thing that runs a runtime rule's fixtures: `test` does not reconcile, so + // there is no blessed-signature path that runs them without it. That makes + // this stricter than `check`, not looser — `check` will execute a blessed + // rule with no flag at all, because it runs rules as a side effect of + // scanning, whereas nothing runs under `test` unless it is asked for twice. "dangerously-run-scripts": { type: "boolean", description: - "Run runtime-rule check.ts without server verification (executes untrusted code)", + "Run runtime-rule check.ts fixtures, which executes untrusted code", default: false, }, }, async run({ args }) { const cwd = resolve(args.dir ?? process.cwd()); - // One gate for the whole run, so a tree of runtime rules is planned once. - // The notices are `check`'s, on stderr, and suppressed under `--json` for + const dangerouslyRunScripts = Boolean(args["dangerously-run-scripts"]); + // Warned once for the whole run, not once per rule: a tree of runtime rules + // is one decision by the user, and repeating the sentence per rule teaches + // people to scroll past it. On stderr, and suppressed under `--json` for // `check`'s reason: a machine consumer cannot read prose. - const runtimeGate = createRuntimeGate(cwd, { - dangerouslyRunScripts: Boolean(args["dangerously-run-scripts"]), - onNotice: (message: string) => { - if (!args.json) console.error(message); - }, - }); + let warned = false; await runOverPath({ cwd, target: args.path ?? ".taskless/rules", json: args.json, label: "test", - run: async (ruleCwd, rule) => testOneRule(ruleCwd, rule, { runtimeGate }), + run: async (ruleCwd, rule) => + testOneRule(ruleCwd, rule, { + dangerouslyRunScripts, + onRuntimeWarning: (message: string) => { + if (warned || args.json) return; + warned = true; + console.error(message); + }, + }), }); }, }); diff --git a/packages/cli/src/rules/fixtures.ts b/packages/cli/src/rules/fixtures.ts new file mode 100644 index 00000000..cd569de1 --- /dev/null +++ b/packages/cli/src/rules/fixtures.ts @@ -0,0 +1,100 @@ +import type { Dirent } from "node:fs"; +import { readdir } from "node:fs/promises"; + +import { isMissingDirectory } from "./errno"; + +/** + * The two questions every engine's fixture reader asks, answered once. + * + * Three engines verify a rule against fixtures — `sg` over `valid:`/`invalid:` + * keys in ast-grep test YAML, `vale` over `pass/`/`fail/` documents, `runtime` + * over `pass/`/`fail/` case directories — and all three had their own copy of + * the same two decisions. The copies were near-verbatim, which is worse than + * different: a comment in `rules/vale/verify.ts` claimed to be "the single + * place that decides which `readdir` failures are absence and which are + * problems, so no caller can accidentally answer that question differently", + * and a third copy made that sentence false the day it was written. + * + * What is shared here is only what is genuinely the same. What each engine + * REJECTS is not: Vale's buckets hold documents, so a nested directory is the + * error; runtime's hold one directory per case, so a loose file is. Those stay + * beside their engines, where the reason for them is legible. + */ + +/** + * Directory entries, with a directory that is not there reading as an empty one. + * + * The single place that decides which `readdir` failures are absence and which + * are problems. Buckets are read independently, and this discrimination is why + * that is safe: swallowing an `EACCES` on `pass/` would yield `[]` while + * `fail/` still had fixtures, so the rule would not look one-sided and could + * report a pass having never checked the pass side at all. A permissions + * problem must not read as "no pass fixtures were written". + */ +export async function bucketEntries(directory: string): Promise { + try { + return await readdir(directory, { withFileTypes: true }); + } catch (error) { + if (isMissingDirectory(error)) return []; + throw error; + } +} + +/** + * Which fixture buckets a rule actually populated, over that engine's bucket + * names. + * + * Four states rather than a boolean, because a caller wants to say different + * things about them: `"none"` is an unwritten rule, while a `-only` is a + * half-written one, which is the more misleading of the two. A rule with only + * failing fixtures has shown it fires and not that it stays quiet; a rule with + * only passing ones has shown the opposite. Only `"both"` can reach a pass. + * + * Parameterised on the bucket names rather than fixed to `pass`/`fail` because + * `sg`'s buckets are the `valid:`/`invalid:` keys of ast-grep's own test YAML, + * and its vocabulary is ast-grep's rather than ours. + */ +export type FixtureCoverage = + | "both" + | `${Bucket}-only` + | "none"; + +/** + * Classify a rule's buckets by how many fixtures each held. + * + * `positive` is the bucket asserting the rule stays quiet (`pass`, `valid`) and + * `negative` the one asserting it fires (`fail`, `invalid`). + */ +export function classifyCoverage< + Positive extends string, + Negative extends string, +>( + positive: { name: Positive; count: number }, + negative: { name: Negative; count: number } +): FixtureCoverage { + if (positive.count > 0 && negative.count > 0) return "both"; + if (positive.count > 0) return `${positive.name}-only`; + if (negative.count > 0) return `${negative.name}-only`; + return "none"; +} + +/** + * What `test` says about coverage that is not `"both"`, or nothing when it is. + * + * `suffix` is how that engine spells a bucket, and it genuinely differs rather + * than having drifted: Vale's and runtime's buckets are directories, so they + * read `pass/`; ast-grep's are keys in a YAML document, so they read `valid:`. + * Naming a bucket in the shape the author will go and look for it is the point + * of the message, so the difference is a parameter rather than something to + * normalise away. + */ +export function describeCoverageShortfall( + ruleId: string, + coverage: FixtureCoverage, + suffix: "/" | ":" +): string | undefined { + if (coverage === "both") return undefined; + return coverage === "none" + ? `${ruleId} has no fixtures, so nothing shows it fires or stays quiet.` + : `${ruleId} has only ${coverage.replace("-only", "")}${suffix} fixtures — half a claim.`; +} diff --git a/packages/cli/src/rules/inspect.ts b/packages/cli/src/rules/inspect.ts index f8443b12..4d0573a6 100644 --- a/packages/cli/src/rules/inspect.ts +++ b/packages/cli/src/rules/inspect.ts @@ -8,10 +8,15 @@ import { ruleDirectory, ruleFilePath, } from "./engines"; +import { describeCoverageShortfall } from "./fixtures"; import { type EngineName } from "./layout"; -import { assessCaptureDirectory, strayModules } from "./runtime/discover"; +import { + assessCaptureDirectory, + discoverRuntimeRules, + strayModules, +} from "./runtime/discover"; import { readRuntimeFixtures } from "./runtime/fixtures"; -import { createRuntimeGate, type RuntimeGate } from "./runtime/plan"; +import { RUN_SCRIPTS_WARNING } from "./runtime/harness"; import { describeFixtureReport, runRuntimeFixtures, @@ -77,14 +82,29 @@ export interface RuleTestResult { /** What `test` needs beyond a rule, all of it about the runtime engine. */ export interface TestOptions { /** - * The execution gate, shared across every rule in one command run. + * Run a runtime rule's fixtures, which executes its `check.ts`. + * + * The WHOLE gate for `test`, and deliberately not `check`'s gate. `check` + * reconciles because it executes rules as a side effect of scanning a + * repository: the user asked for a scan, code ran, and the gate is what stops + * that happening silently. `test` runs fixtures because the user asked it to + * — the verb is the consent — so asking a server for permission to run your + * own fixtures is overreach, and the flag alone decides. * - * Passed in rather than built here so a `test` over a whole tree plans once: - * the plan consults auth and reconcile, and per-rule planning would send one - * request per runtime rule for an answer that does not vary. Omitted, a - * gate with no escape flag is built, which is the safe default. + * That is STRICTLY MORE CONSERVATIVE than gating on reconcile as well: + * nothing executes here that would not have executed before, and a blessed + * rule that previously ran without the flag now requires it. Absent, which + * is the safe default, every runtime rule is refused. */ - runtimeGate?: RuntimeGate; + dangerouslyRunScripts?: boolean; + /** + * Called immediately before fixtures actually execute, never otherwise. + * + * Lazy so `test --dangerously-run-scripts` over a tree of `sg` rules does not + * warn about code it never ran. Deduplication belongs to the caller, which + * knows how many rules one command run covers. + */ + onRuntimeWarning?: (message: string) => void; } async function readYaml(path: string): Promise { @@ -285,13 +305,17 @@ export async function testOneRule( // populated only one bucket has proved only half of what a rule claims. // `ast-grep test` will not say so — an empty `invalid:` bucket is // `1 passed; 0 failed`, exit zero — so the message has to come from here. - if (result.tests.fixtures !== "both") { - errors.push( - result.tests.fixtures === "none" - ? `${ruleId} has no fixtures, so nothing shows it fires or stays quiet.` - : `${ruleId} has only ${result.tests.fixtures.replace("-only", "")}: fixtures — half a claim.` - ); - } + // + // `:` rather than `/` because ast-grep's buckets are the `valid:`/`invalid:` + // KEYS of a test YAML document, not directories. The other two engines read + // `pass/`. Naming a bucket in the shape the author will search for is the + // point of the message, so the difference is deliberate, not drift. + const shortfall = describeCoverageShortfall( + ruleId, + result.tests.fixtures, + ":" + ); + if (shortfall !== undefined) errors.push(shortfall); return { engine, ruleId, @@ -321,13 +345,10 @@ export async function testOneRule( }; } const errors: string[] = []; - if (result.fixtures !== "both") { - errors.push( - result.fixtures === "none" - ? `${ruleId} has no fixtures, so nothing shows it fires or stays quiet.` - : `${ruleId} has only ${result.fixtures.replace("-only", "")}/ fixtures — half a claim.` - ); - } + // `/` because Vale's buckets are directories; see the `sg` branch above for + // why that suffix is a parameter rather than one spelling for all three. + const shortfall = describeCoverageShortfall(ruleId, result.fixtures, "/"); + if (shortfall !== undefined) errors.push(shortfall); for (const file of result.missingFailures) { errors.push(`fail fixture did not fire: ${file}`); } @@ -344,24 +365,56 @@ export async function testOneRule( }; } - // Runtime rules execute code, so their fixtures run through the harness - // under the same server verification `check` requires: the gate below is - // `check`'s, imported rather than re-stated. - const gate = - options.runtimeGate ?? - createRuntimeGate(cwd, { dangerouslyRunScripts: false }); - const admission = await gate.admit(ruleId); - - if (!admission.admitted) { + // Runtime rules execute code, so `test` runs their fixtures only behind + // `--dangerously-run-scripts`. It does NOT reconcile, and that is the whole + // difference from `check`. + // + // `check` asks the server because it executes rules as a SIDE EFFECT of + // scanning a repository — nobody asked for code to run, so a gate has to + // stand between the scan and the execution. `test` runs fixtures because the + // user typed `test`, and the verb is the consent; asking a server for + // permission to run your own fixtures is overreach. + // + // The only party a reconcile here would ever admit is someone testing an + // already-blessed delivered rule, whose fixtures the service verified before + // it delivered anything. A locally authored rule has no signature and never + // will, because blessing is recording and nothing recorded it — so for the + // audience that actually runs `test` on a runtime rule, a reconcile is pure + // cost paid for an answer that is always "no". + // + // Dropping it is strictly more conservative: nothing runs that would not have + // run before, and the blessed case that used to run without the flag now + // needs it. It also removes the `--anonymous` question rather than answering + // it, since with no network there is nothing to suppress. + if (options.dangerouslyRunScripts !== true) { // Neither a pass nor a failure (D2), and the message has to say what would - // change it. A locally authored rule has no signature and never will, - // because blessing is recording and nothing recorded this one, so the flag - // is the author's only route and naming it here is the difference between - // a dead end and an instruction. + // change it. The flag is the only route for anyone, so naming it here is + // the difference between a dead end and an instruction. const reason = - `fixtures did not run: ${admission.reason}. Pass ` + - `--dangerously-run-scripts to run them without server verification ` + - `(it executes the rule's check.ts).`; + `fixtures did not run: running them executes the rule's check.ts. ` + + `Pass --dangerously-run-scripts to run them.`; + return { + engine, + ruleId, + ok: false, + errors: [reason], + ran: false, + refused: reason, + }; + } + + // Discovery, not a plan: the bytes are the working tree's, which is the point + // of testing a rule you are authoring. `discoverRuntimeRules` is the same + // enumeration `check` plans over, so a rule it refuses — no loadable capture, + // or an unsigned module beside `check.ts` — is refused here identically + // rather than by a second opinion. `verify` has already run and named the + // real defect; this reports that nothing was run, not a verdict on the rule. + const discovered = await discoverRuntimeRules(cwd); + const runtimeRule = discovered.find((candidate) => candidate.name === ruleId); + if (runtimeRule === undefined) { + const reason = + `fixtures did not run: it was not discovered as a runnable runtime ` + + `rule (no capture rules under captures/).`; return { engine, ruleId, @@ -389,7 +442,11 @@ export async function testOneRule( }; } - const report = await runRuntimeFixtures(admission.rule, fixtures); + // Warned at the last possible moment: everything above can still decline to + // run anything, and a warning about code that never executed is noise. + options.onRuntimeWarning?.(RUN_SCRIPTS_WARNING); + + const report = await runRuntimeFixtures(runtimeRule, fixtures); return { engine, ruleId, diff --git a/packages/cli/src/rules/runtime/fixtures.ts b/packages/cli/src/rules/runtime/fixtures.ts index 6d1a4e65..099750d8 100644 --- a/packages/cli/src/rules/runtime/fixtures.ts +++ b/packages/cli/src/rules/runtime/fixtures.ts @@ -1,8 +1,10 @@ -import type { Dirent } from "node:fs"; -import { readdir } from "node:fs/promises"; import { join } from "node:path"; -import { isMissingDirectory } from "../errno"; +import { + bucketEntries, + classifyCoverage, + type FixtureCoverage, +} from "../fixtures"; import { ruleTestsDirectory } from "../engines"; /** The two buckets a fixture case can live in. */ @@ -36,50 +38,25 @@ export interface RuntimeFixtureCase { * more misleading of the two. A rule with only `fail/` cases has shown it fires * and not that it stays quiet; only `both` can reach a pass. */ -export type RuntimeFixtureCoverage = - | "both" - | "pass-only" - | "fail-only" - | "none"; - -/** Classify a rule's buckets by how many cases each held. */ -function coverageOf( - passCount: number, - failCount: number -): RuntimeFixtureCoverage { - if (passCount > 0 && failCount > 0) return "both"; - if (passCount > 0) return "pass-only"; - if (failCount > 0) return "fail-only"; - return "none"; -} +export type RuntimeFixtureCoverage = FixtureCoverage<"pass" | "fail">; /** - * A missing directory is an empty bucket; anything else rethrows. + * The cases in one bucket, one level deep. * - * The buckets are read independently and this discrimination is why. Swallowing + * A missing bucket is an empty one and anything else rethrows, which is + * {@link bucketEntries}'s decision rather than one taken again here: swallowing * an `EACCES` on `pass/` would yield `[]` while `fail/` still had cases, so the * rule would not look one-sided and could report a pass having never checked - * the pass side at all. A permissions problem must not read as "no pass - * fixtures were written". - */ -async function directoryEntries(directory: string): Promise { - try { - return await readdir(directory, { withFileTypes: true }); - } catch (error) { - if (isMissingDirectory(error)) return []; - throw error; - } -} - -/** - * The cases in one bucket, one level deep. + * the pass side at all. * - * Every entry must be a directory, and a loose file is an error naming its own - * path rather than an entry quietly skipped. The check is handed a root and - * reads whatever it needs beneath it, so a file has no root to be: there is no - * sensible reading of `pass/example.ts` that the harness could act on, and - * ignoring it would leave an author with a fixture they wrote, that never ran, - * and that nothing mentioned. + * What is this engine's own is the rejection, and it is the OPPOSITE of Vale's. + * Vale's buckets hold documents, so a nested directory is its error; these hold + * one directory per case, so every entry must be a directory and a loose file + * is an error naming its own path rather than an entry quietly skipped. The + * check is handed a root and reads whatever it needs beneath it, so a file has + * no root to be: there is no sensible reading of `pass/example.ts` that the + * harness could act on, and ignoring it would leave an author with a fixture + * they wrote, that never ran, and that nothing mentioned. */ async function bucketCases( cwd: string, @@ -87,7 +64,7 @@ async function bucketCases( bucket: FixtureBucket ): Promise { const directory = join(ruleTestsDirectory(cwd, "runtime", ruleId), bucket); - const entries = await directoryEntries(directory); + const entries = await bucketEntries(directory); const loose = entries.find((entry) => !entry.isDirectory()); if (loose !== undefined) { @@ -129,6 +106,9 @@ export async function readRuntimeFixtures( return { cases: [...fail, ...pass], - coverage: coverageOf(pass.length, fail.length), + coverage: classifyCoverage( + { name: "pass", count: pass.length }, + { name: "fail", count: fail.length } + ), }; } diff --git a/packages/cli/src/rules/runtime/harness.ts b/packages/cli/src/rules/runtime/harness.ts index 68971902..749b9d6f 100644 --- a/packages/cli/src/rules/runtime/harness.ts +++ b/packages/cli/src/rules/runtime/harness.ts @@ -9,6 +9,18 @@ import { DEFAULT_CHECK_TIMEOUT_MS, invokeCheck } from "./invoke"; /** Scanner-agnostic `source` label for runtime-rule findings. */ export const RUNTIME_SOURCE = "taskless-runtime"; +/** + * What `--dangerously-run-scripts` says when it is what let the code run. + * + * Stated beside the executor rather than in either command, because `check` and + * `test` reach the flag by different routes — `check` through the reconcile + * plan, `test` with no plan at all — and the sentence a user reads should not + * depend on which route ran. Two spellings would drift, and the drift would be + * invisible: both are warnings nobody diffs. + */ +export const RUN_SCRIPTS_WARNING = + "Warning: --dangerously-run-scripts is executing runtime rule code without server verification."; + /** Options controlling a runtime-rule run. */ export interface RuntimeRunOptions { /** Restrict the narrow to these paths (diff scope); empty scans the repo. */ diff --git a/packages/cli/src/rules/runtime/plan.ts b/packages/cli/src/rules/runtime/plan.ts index e1c5f878..b5fddadd 100644 --- a/packages/cli/src/rules/runtime/plan.ts +++ b/packages/cli/src/rules/runtime/plan.ts @@ -8,7 +8,8 @@ import { restoreRule } from "../../api/restore"; import { writeRuleFile } from "../files"; import { PurgeIncompleteError } from "../deliver"; import { repairTargets, verifyRestoredCheck } from "./repair"; -import { discoverRuntimeRules, type RuntimeRule } from "./discover"; +import { RUN_SCRIPTS_WARNING } from "./harness"; +import { type RuntimeRule } from "./discover"; import { materializeRuntimeRules, reportRuntimeChecks, @@ -17,15 +18,26 @@ import { } from "./run-set"; /** - * Deciding WHICH runtime rules may execute, separately from executing them. + * Deciding WHICH runtime rules may execute during a `check`, separately from + * executing them. * - * This lived inside `commands/check.ts` and moved here unchanged so `test` can - * run a rule's fixtures under the same policy rather than beside it. That is - * the whole reason for the move: a fixture run executes the same `check.ts`, - * from the same delivery, under the same signature as a scan, and "it is only - * running against test data" is a statement about the input rather than about - * what the program may do. A second implementation of this gate, however - * faithful on the day it was written, is a bypass waiting to be discovered. + * `check` is the only caller and the only command that needs this. It executes + * rules as a SIDE EFFECT of scanning a repository — nobody asked for code to + * run — so a reconcile stands between the scan and the execution, and every + * unverified path skips rather than fails. + * + * `test` deliberately does not come through here. It runs a rule's fixtures + * because the user typed `test`, so the verb is the consent and + * `--dangerously-run-scripts` is its whole gate; see the runtime branch of + * `rules/inspect.ts`. That is stricter than sharing this module, not looser: + * nothing runs under `test` that would not have run before, and a blessed rule + * that used to run there without the flag now needs it. + * + * It lived inside `commands/check.ts` and moved here for a reason that has + * since evaporated (sharing the gate with `test`). It stays because a second + * reason holds on its own: this is policy, `commands/check.ts` is argument + * parsing and rendering, and `repairWithheldRules` below is a long piece of + * recovery logic that a command file has no business carrying. */ /** A runtime rule that will not run, with why (advisory). */ @@ -71,9 +83,7 @@ export async function planRuntime( return { execute: discovered, skipped: [], - notices: [ - "Warning: --dangerously-run-scripts is executing runtime rule code without server verification.", - ], + notices: [RUN_SCRIPTS_WARNING], }; } @@ -299,74 +309,3 @@ async function repairWithheldRules( return { notices }; } - -/** - * The plan, resolved once and shared by every rule a command reports on. - * - * `test` inspects rules one at a time, and planning per rule would send one - * reconcile per runtime rule for an answer that is the same every time. The - * plan is therefore memoized on first use and the notices are emitted once, - * with the whole set of discovered runtime rules reported to reconcile exactly - * as `check` reports it. A subset would be a different question asked of the - * service. - * - * Lazy rather than eager because most projects hold no runtime rules at all, - * and a `test` run over a tree of `sg` rules must not reach the network. - */ -export interface RuntimeGate { - /** - * The rule as it may be executed, or the reason it may not be. - * - * The rule returned is the one the plan blessed, which on the gated path is - * the materialized copy: the bytes run against a fixture are the bytes the - * server blessed, not whatever the working tree happens to hold. - */ - admit( - ruleId: string - ): Promise< - { admitted: true; rule: RuntimeRule } | { admitted: false; reason: string } - >; -} - -/** Build a gate over the runtime rules `cwd` holds. */ -export function createRuntimeGate( - cwd: string, - options: { - dangerouslyRunScripts: boolean; - /** Called once, with the notices `check` prints for the same plan. */ - onNotice?: (message: string) => void; - } -): RuntimeGate { - let planned: Promise | undefined; - - const plan = async (): Promise => { - planned ??= (async () => { - const discovered = await discoverRuntimeRules(cwd); - const resolved = await planRuntime(cwd, discovered, { - anonymous: false, - dangerouslyRunScripts: options.dangerouslyRunScripts, - }); - for (const notice of resolved.notices) options.onNotice?.(notice); - return resolved; - })(); - return planned; - }; - - return { - async admit(ruleId) { - const resolved = await plan(); - const rule = resolved.execute.find( - (candidate) => candidate.name === ruleId - ); - if (rule !== undefined) return { admitted: true, rule }; - - const skipped = resolved.skipped.find((entry) => entry.rule === ruleId); - return { - admitted: false, - reason: - skipped?.reason ?? - "it was not discovered as a runnable runtime rule (no capture rules under captures/)", - }; - }, - }; -} diff --git a/packages/cli/src/rules/runtime/run-fixtures.ts b/packages/cli/src/rules/runtime/run-fixtures.ts index 6caac819..88edb97f 100644 --- a/packages/cli/src/rules/runtime/run-fixtures.ts +++ b/packages/cli/src/rules/runtime/run-fixtures.ts @@ -1,3 +1,4 @@ +import { describeCoverageShortfall } from "../fixtures"; import type { RuntimeRule } from "./discover"; import { executeRuntimeRuleDetailed } from "./harness"; import type { RuntimeRunOptions } from "./harness"; @@ -134,13 +135,10 @@ export function describeFixtureReport( ): string[] { const errors: string[] = []; - if (report.coverage !== "both") { - errors.push( - report.coverage === "none" - ? `${ruleId} has no fixtures, so nothing shows it fires or stays quiet.` - : `${ruleId} has only ${report.coverage.replace("-only", "")}/ fixtures — half a claim.` - ); - } + // `/` because these buckets are directories, which is how the author will go + // looking for them. `sg`'s are YAML keys and read `valid:`. + const shortfall = describeCoverageShortfall(ruleId, report.coverage, "/"); + if (shortfall !== undefined) errors.push(shortfall); for (const name of report.missingFailures) { errors.push(`fail fixture did not fire: ${name}`); } diff --git a/packages/cli/src/rules/vale/verify.ts b/packages/cli/src/rules/vale/verify.ts index 95805bc7..dfb706a5 100644 --- a/packages/cli/src/rules/vale/verify.ts +++ b/packages/cli/src/rules/vale/verify.ts @@ -1,11 +1,14 @@ -import { type Dirent, mkdtempSync, rmSync, writeFileSync } from "node:fs"; -import { readdir } from "node:fs/promises"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, posix, relative, resolve, sep } from "node:path"; import { listRuleIds, ruleTestsDirectory } from "../engines"; import { RULES_DIRECTORY } from "../layout"; -import { isMissingDirectory } from "../errno"; +import { + bucketEntries, + classifyCoverage, + type FixtureCoverage, +} from "../fixtures"; import { runVale, type ValeRunOutcome } from "./run"; /** Where a rule's fixtures live, relative to the project root. */ @@ -56,34 +59,22 @@ export function buildIsolatingConfig(cwd: string, ruleId: string): string { ].join("\n"); } -/** - * Directory entries, with a directory that is not there reading as an empty one. - * - * The single place that decides which `readdir` failures are absence and which - * are problems, so no caller can accidentally answer that question differently. - */ -async function directoryEntries(directory: string): Promise { - try { - return await readdir(directory, { withFileTypes: true }); - } catch (error) { - if (isMissingDirectory(error)) return []; - throw error; - } -} - /** * Fixture documents directly under `///`. * - * A missing directory is an empty bucket; anything else rethrows. The buckets - * are read independently, so a swallowed `EACCES` on `pass/` would silently - * yield `[]` while `fail/` still had fixtures — the rule would not look - * one-sided, and could report `passed: true` having never checked the pass side - * at all. A permissions problem must not read as "no pass fixtures were - * written". + * A missing directory is an empty bucket and anything else rethrows, which is + * {@link bucketEntries}'s decision rather than one taken again here: the + * buckets are read independently, so a swallowed `EACCES` on `pass/` would + * silently yield `[]` while `fail/` still had fixtures — the rule would not + * look one-sided, and could report `passed: true` having never checked the pass + * side at all. * - * A bucket is one directory deep, and a nested directory is rejected rather - * than ignored. The two halves of verification disagree about recursion: this - * read is flat, but Vale is invoked over the whole `rule-tests/` tree and + * What is Vale's own is the rejection below, and it is the opposite of the + * runtime engine's: Vale's buckets hold DOCUMENTS, so a nested directory is the + * error, where runtime's hold one directory per case and a loose file is. A + * bucket is one directory deep, and a nested directory is rejected rather than + * ignored. The two halves of verification disagree about recursion: this read + * is flat, but Vale is invoked over the whole `rule-tests/` tree and * lints recursively. Silently skipping a nested entry therefore fails in the * dangerous direction — a nested `pass/` fixture that wrongly fires produces a * finding this function never collected, so `unexpectedFindings` discards it, @@ -101,7 +92,7 @@ async function fixtureFiles( bucket: "pass" | "fail" ): Promise { const directory = join(valeRuleTestsDirectory(cwd, ruleId), bucket); - const entries = await directoryEntries(directory); + const entries = await bucketEntries(directory); const nested = entries.find((entry) => entry.isDirectory()); if (nested !== undefined) { @@ -132,16 +123,13 @@ function toRelativePosix(cwd: string, absolute: string): string { * different things about them: `"none"` is an unwritten rule, while * `"fail-only"`/`"pass-only"` is a half-written one, which is the more * misleading state of the two. + * + * A name for {@link FixtureCoverage} over Vale's bucket names rather than a + * fourth hand-written union: the classification is `classifyCoverage`'s, so a + * caller reading `ValeFixtureCoverage` and one reading `SgFixtureCoverage` are + * reading the same four states under two vocabularies. */ -export type ValeFixtureCoverage = "both" | "pass-only" | "fail-only" | "none"; - -/** Classify a rule's buckets by how many documents each held. */ -function coverageOf(passCount: number, failCount: number): ValeFixtureCoverage { - if (passCount > 0 && failCount > 0) return "both"; - if (passCount > 0) return "pass-only"; - if (failCount > 0) return "fail-only"; - return "none"; -} +export type ValeFixtureCoverage = FixtureCoverage<"pass" | "fail">; export interface ValeRuleVerification { ruleId: string; @@ -195,7 +183,7 @@ export async function discoverValeRuleTests(cwd: string): Promise { const ruleIds = await listRuleIds(cwd, "vale"); const withTests: string[] = []; for (const ruleId of ruleIds) { - const entries = await directoryEntries(valeRuleTestsDirectory(cwd, ruleId)); + const entries = await bucketEntries(valeRuleTestsDirectory(cwd, ruleId)); if (entries.length > 0) withTests.push(ruleId); } return withTests; @@ -239,7 +227,10 @@ export async function verifyValeRule( // Short-circuited before Vale runs: the rule has to be edited either way, so // there is nothing a subprocess could add that `fixtures` does not say. - const fixtures = coverageOf(passFixtures.length, failFixtures.length); + const fixtures = classifyCoverage( + { name: "pass", count: passFixtures.length }, + { name: "fail", count: failFixtures.length } + ); if (fixtures !== "both") { return { ruleId, diff --git a/packages/cli/src/rules/verify.ts b/packages/cli/src/rules/verify.ts index c3d0501a..5d83ce56 100644 --- a/packages/cli/src/rules/verify.ts +++ b/packages/cli/src/rules/verify.ts @@ -12,6 +12,7 @@ import { findRegexWithoutKind, } from "../schemas/ast-grep-rule"; import { pathPrefixed, schemaLayer } from "../schemas/layer"; +import { classifyCoverage, type FixtureCoverage } from "./fixtures"; import { AST_GREP_TSX_SPLIT, AST_GREP_VERSION, @@ -65,7 +66,7 @@ export interface RequirementsResult extends LayerResult { * YAML rather than Vale's `pass/`/`fail/` directories, so the names follow * ast-grep's vocabulary. */ -export type SgFixtureCoverage = "both" | "valid-only" | "invalid-only" | "none"; +export type SgFixtureCoverage = FixtureCoverage<"valid" | "invalid">; export interface TestLayerResult extends LayerResult { passed: number; @@ -364,17 +365,6 @@ async function discoverRuleTestFiles( .map((entry) => join(directory, entry)); } -/** Classify a rule's buckets by how many sources each held. */ -function coverageOf( - validCount: number, - invalidCount: number -): SgFixtureCoverage { - if (validCount > 0 && invalidCount > 0) return "both"; - if (validCount > 0) return "valid-only"; - if (invalidCount > 0) return "invalid-only"; - return "none"; -} - /** * Count what the author actually put in each bucket, across every test file * the rule owns. @@ -422,7 +412,10 @@ async function fixtureCoverage( if (Array.isArray(buckets.invalid)) invalidCount += buckets.invalid.length; } - return coverageOf(validCount, invalidCount); + return classifyCoverage( + { name: "valid", count: validCount }, + { name: "invalid", count: invalidCount } + ); } // --- Layer 3: Test execution --- diff --git a/packages/cli/test/runtime-fixture-runner.test.ts b/packages/cli/test/runtime-fixture-runner.test.ts index d9cf9a99..a25ba523 100644 --- a/packages/cli/test/runtime-fixture-runner.test.ts +++ b/packages/cli/test/runtime-fixture-runner.test.ts @@ -213,6 +213,23 @@ describe("a runtime rule whose fixtures did not run", () => { expect(stdout).toContain("--dangerously-run-scripts"); }); + it("offers the flag as the only route, and never sends the author to auth", async () => { + // `test` does not reconcile, so authenticating changes nothing here and + // saying otherwise would send an author to a login that cannot help: a + // locally authored rule has no signature and never will. The flag is the + // whole gate, so it has to be the whole message. + await writeRule(); + await writeCase("fail", "uses-eval"); + await writeCase("pass", "no-eval"); + + const { stdout } = await testRule(); + + expect(stdout).toContain("--dangerously-run-scripts"); + expect(stdout).not.toContain("auth login"); + expect(stdout).not.toContain("not authenticated"); + expect(stdout).not.toContain("server verification"); + }); + it("reports ran: false in --json, and does not report ok: true", async () => { await writeRule(); await writeCase("fail", "uses-eval");