Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions openspec/changes/runtime-fixture-runner/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-09-02
222 changes: 222 additions & 0 deletions openspec/changes/runtime-fixture-runner/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
## Context

**Runtime rules do execute today.** `check` runs them against the repository
through `dispatch.ts:205` into `executeRuntimeRule`, gated by the signature
planning that happens before dispatch. The executor, the process spawn, the
timeout and the result shape all exist and are exercised on every authenticated
`check`. Nothing here is about making runtime rules runnable.

What is missing is running them against **fixtures**. Three engines, three
fixture stories, and one of them is absent. `sg` runs `ast-grep test` over
`.tests/`; `vale` runs its rule over `pass/` and `fail/` buckets and checks both
directions; `runtime` returns `ran: false, ok: true` and prints a tick. Two
commands, one engine, one of them wired up.

The Vale runner is the model, and it is worth reading before writing this one:
`rules/vale/verify.ts` already carries the lessons this tier will otherwise
relearn. Buckets are read independently, so a swallowed `EACCES` on `pass/`
cannot present as "no pass fixtures were written". A nested directory is
rejected rather than ignored, because the reader is flat while the engine
recurses, and silently skipping an entry fails in the dangerous direction.
Coverage is a four-way classification rather than a boolean, because
`fail-only` is more misleading than `none`.

## Goals / Non-Goals

**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.

## 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`.

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.

### D2 — A run that did not happen is a third state, not a pass and not a failure

This is the decision the whole change turns on.

Today the runtime branch returns `ok: true`, which is wrong. The obvious
correction is `ok: false`, and it is also wrong: a rule that cannot run because
nothing blessed it is not a defective rule, and failing it would turn `test` red
for every project holding a runtime rule, with no action available that makes it
green. That trades a silent wrong answer for a loud useless one.

So `test` reports three outcomes for a runtime rule: it ran and the fixtures
behaved, it ran and they did not, or it did not run and here is why. The third
prints its own marker rather than a tick, names the reason, and does not count
toward "N rule(s) tested". `ran` stops being advisory metadata in the `--json`
envelope and becomes the field a caller branches on.

The precedent cuts the other way for the other engines and that is consistent
rather than contradictory. `verify.ts` says "skips are errors, never a pass",
and for `sg` a skip means something went wrong — the runner exists, so declining
to use it is a fault. For `runtime` the run is refused by a security policy
working as designed. Same word, two situations, opposite correct handling. That
distinction is the one this codebase got wrong once already with `unsafe` versus
`missing`.

### D3 — A fixture case is a directory, because the harness already takes a root

This is less a choice than an observation. `executeRuntimeRule(root, rule,
options)` in `runtime/harness.ts` already takes a root, and `check` reaches it
through `dispatch.ts:205` with the repository root. A fixture case directory is
simply a different root handed to the same function.

So the runner is a caller of the existing executor rather than a sibling of it:
a loop over case directories plus the pass/fail assertion. Process spawn,
timeout, narrowing, capture discovery and the result shape are all shared with
`check` and already proven there. Any other case layout would need new plumbing
instead of reusing what exists, which is the argument for directories rather
than a preference for them.

Vale's buckets hold documents; runtime's hold **directories**, one per case.
`types/runtime-rule.ts` defines a check as a function over `(root, matches)`
reading files under `root`, and `create-runtime-rule.txt` already documents
`.tests/pass/case-1/` with that meaning.

This is not a stylistic difference. 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 the tier is for. It also settles the recursion question Vale
had to answer: a bucket is read one level deep, and each entry must be a
directory. A loose file directly in `pass/` is an error naming the path, not a
case with an implicit root.

### D4 — Coverage is classified, and only `both` can pass

Copied from `ValeFixtureCoverage` deliberately, including the reasoning. A rule
with only `fail/` cases has shown it fires and not that it stays quiet; a rule
with only `pass/` cases has shown the opposite; and a rule with neither has
shown nothing while exiting zero. `none`, `pass-only`, `fail-only` and `both`
are kept apart because a caller says different things about them.

### D5 — Reading the buckets cannot be lenient

The Vale reader rethrows anything that is not a missing directory, because
swallowing an `EACCES` on one bucket makes a two-sided rule look one-sided and a
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

A locally authored rule has no signature and never will, because blessing is
recording and nothing recorded it. So its author must pass
`--dangerously-run-scripts` to test their own rule, which `sg` and `vale`
authors do not have to do.

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
one are indistinguishable on disk. The warning the flag prints is accurate in
both cases.

Worth stating in the recipe rather than discovered: an author who runs `test`
and sees "did not run" should be told the flag in that message.

### D7 — Nothing to test means the delivery was incomplete, and that belongs in delivery

`test` failing a fixtureless rule is the right answer for a rule someone wrote.
It is the wrong place to catch a rule the service sent, because by then the rule
is on disk and the failure reads as the holder's fault for a file they never
authored.

Delivery already answers "is this a complete rule": `describeIncompleteSet`
requires the rule file, the per-engine config where one exists, and at least one
capture, and the whole set is assessed before anything is written so a refused
delivery leaves no directory behind. A rule with nothing to test is incomplete
by the same standard as a rule with no captures. Both are rules that cannot
demonstrate anything, and both are cheaper refused than written.

This is the styleguide's own rule about build output applied to a payload: an
invariant enforced where the artifact is produced cannot be violated, while one
enforced afterwards can only be detected.

**This is not a negotiation.** A runtime rule ships with a `.tests/` directory;
a delivery without one is a defect on the service side, not a payload shape we
are asking them to adopt. They already hold the material — their verification
gate executes the generated check against failing and passing examples on its
way to accepting the rule — and they have already established that fixtures are
files in the collection rather than a field beside it.

**What is sequenced is enforcement, not the expectation.** Being right about
whose bug it is does not stop `describeIncompleteSet` from turning that bug into
a refused write for every user, with nothing written and no action available to
them. So the order is: confirm deliveries carry `.tests/`, and if they do not,
file it as the defect it is rather than proposing it as a change. Enforce once
the fix has shipped.

That leaves the two halves landing in the right order for a reason that has
nothing to do with agreement. `test` failing an authored rule is ours alone and
ships now. Delivery completeness follows once we know we are enforcing a rule
the service already satisfies, rather than discovering it does not through an
outage.

### D8 — A case that never reaches the check is its own outcome, not zero findings

`executeRuntimeRule` runs the narrow first and gates on it:

```ts
if (matches.length === 0) return []; // gate: no matches, no check
```

So a fixture case whose narrow matches nothing returns an empty array having
never invoked `check.ts`, which is indistinguishable downstream from a check
that ran and found nothing. Under the scenarios as first drafted, a `fail/` case
in that state fails, and the message blames the check for a fixture that never
reached it.

That is this change's own failure mode, reintroduced inside the fix. The runner
therefore has to know whether the check was **invoked**, not only what it
returned, and report three outcomes per case rather than two.

It matters in both buckets, and the `pass/` side is the quieter half. A `pass/`
case with no narrow matches looks like a clean pass and proves nothing about the
check: it demonstrates that the narrow did not match, which is a fact about the
fixture. A case that never invokes the check cannot show the check stays quiet,
so counting it as evidence is the same empty-scan-reports-success shape one
level down.

So a case producing no narrow matches is reported as a **fixture defect** in
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.

## Risks / Trade-offs

**A rule with no fixtures fails, and that is the decision.** The other two
engines fail it, on the grounds that nothing shows the rule fires or stays
quiet, and making runtime the exception would leave the tier that executes
arbitrary code as the one whose rules need not prove anything.

Depth is not policed. A trivial case that exercises little and passes is
acceptable; the requirement is that a rule has fixtures, not that they are
thorough. But that latitude only reaches one bucket. A `fail/` case producing no
findings is the silent regression this runner exists to catch, so a case that
tests nothing can only be a `pass/` case, and the `fail/` case is the one that
has to do real work. That is also the one worth having, since it is what proves
the rule fires at all.

The cost is real and worth naming: runtime rules have not been carrying
fixtures, because nothing ran them, so this fails rules that were correct under
the old behaviour. That is the same trade this CLI already made when `check`,
`verify` and `test` stopped migrating silently — a wall met once, in exchange
for a report that means something.

**The runner is a second execution path.** It reuses `invoke.ts` rather than
reimplementing invocation, so a divergence between how a fixture runs and how
`check` runs is a shared-code question rather than a drift question. If the two
ever need different behaviour, that is the signal to stop and ask why.
88 changes: 88 additions & 0 deletions openspec/changes/runtime-fixture-runner/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
## Why

`taskless test` reports a runtime rule as passing when its fixtures never ran.
Measured against a scaffolded project holding one runtime rule:

```
$ taskless test $ taskless test --json
✓ runtime/demo-probe {"ok":true,"rules":[{"engine":"runtime",
"ruleId":"demo-probe","ok":true,
1 rule(s) tested. "errors":[],"ran":false}]}
exit 0
```

`testOneRule` returns `ok: true, errors: [], ran: false` for the runtime engine,
and the human renderer reads only `ok` — `ran` exists in the `--json` schema and
nothing prints it. So the command whose job is to prove a rule fires says
"1 rule(s) tested" about a rule it did not test.

**This is an unimplemented requirement, not a missing feature.**
`cli-rule-validation` already says `test` executes a rule against "ast-grep test
cases, Vale `pass`/`fail` fixture buckets, or the runtime harness". Two of those
three are implemented and have scenarios. The third has neither, and its absence
is reported as success.

Three things make it worth doing now rather than later.

**Nothing else consumes a runtime rule's fixtures.** Delivery accepts `.tests/`
paths, `PRESERVED_SUBTREES` keeps them through a purge, and `strayModules`
exempts them at any depth so a TypeScript fixture does not trip the
one-executable-file rule. Every piece of support exists except the thing that
runs them.

**The demonstration depends on it.** The generator team withdrew the findings
the demo was going to serve, on the grounds that they are implied by delivery.
What a reader gets instead is a rule they can run — so a `test` that reports
green without running is, in their words, the difference between a
demonstration and a claim. They have recorded it as a limit on what their side
can assert.

**It is this repository's own named failure mode, in the command built to catch
it.** The same requirement says an engine "SHALL NOT be trusted to report this
itself", because `ast-grep test` calls an empty `invalid:` bucket a pass. The
runtime tier does the same thing one level up.

## What Changes

**A runtime fixture runner.** `test` executes a rule's `check.ts` against each
case under `.tests/pass/` and `.tests/fail/`, with the case directory as the
`root` the harness hands the check. A `fail/` case SHALL produce at least one
finding; a `pass/` case SHALL produce none.

**Gated by the gate that already exists.** Running a fixture executes delivered
code, so it runs under the same policy `check` uses: an authenticated reconcile
that returns the rule's signature in `run`, or `--dangerously-run-scripts`.
Neither is a new mechanism and no demo rule id is special-cased.

**A run that did not happen is reported as such.** Not as a pass, and not as a
failure. A rule that cannot run because nothing blessed it and no flag was
passed is not a defect in the rule, and failing it would turn `test` red for
every project holding a runtime rule with no way to make it green.

**Fixture coverage is classified, not counted.** Only a rule populating both
buckets can pass, which is what the requirement already demands of the other
two engines.

## Capabilities

### Modified Capabilities

- `cli-rule-validation`: the `test` requirement gains the runtime scenarios it
names an engine for and never specified, plus the reporting rule for a run
that was gated out.
- `cli-runtime-rule-execution`: gains the fixture runner and its gating.

## Impact

- `packages/cli/src/rules/runtime/` — a fixture runner that CALLS the existing
executor rather than sitting beside it. `executeRuntimeRule` already takes a
root, and `check` already passes the repository root through `dispatch.ts`;
a case directory is a different root handed to the same function. What is new
is the loop over cases and the pass/fail assertion, not the execution.
- `packages/cli/src/rules/inspect.ts` — `testOneRule`'s runtime branch stops
returning `ok: true` for a run that did not happen.
- `packages/cli/src/commands/verify.ts` — the renderer distinguishes a skipped
rule from a passing one, and the summary line stops counting it as tested.
- `packages/cli/src/schemas/verify-test.ts` — `ran` becomes load-bearing rather
than advisory.
- No change to `check`, to reconcile, or to the execution gate itself.
Loading
Loading