diff --git a/.github/workflows/perf.yml b/.github/workflows/perf.yml index e44add87b..87a3f1bce 100644 --- a/.github/workflows/perf.yml +++ b/.github/workflows/perf.yml @@ -14,13 +14,18 @@ name: perf # minutes are metered; `ci-local-parity` exists to keep the landing path to # things a local `mise run verify` already proved. # -# WHAT DEFENDS THE NUMBER PER-COMMIT, then, is `tests/perf-assert.bats`: the -# gate is a pure function of records on stdin, so its decision runs in the hk -# gate on every commit in milliseconds, with no build and no hyperfine. This -# workflow supplies the one thing that suite cannot — a real measurement of the -# real binary — and applies the same gate to it. The split is the repo's -# standing agents-fetch/gates-decide pattern, and the `coverage.yml` / -# `lock-currency.yml` precedent for "a property of the world runs on a clock". +# WHAT DEFENDS THE NUMBER PER-COMMIT, then, is `policy/perf-assert.rego`'s README +# half (CLOUD-1321): the published budget column and the enforced table are held +# in agreement on every `batten check`, with no build, no hyperfine and no record +# — so the number this workflow measures cannot be quietly re-published as +# something else between runs. `crates/batten/tests/it/perf_assert.rs` is the tier +# over that, and it carries the case that reads the committed README. +# +# This workflow supplies the one thing that half cannot — a real measurement of +# the real binary — and files it where the module's other half reads it. The +# split is the repo's standing agents-fetch/gates-decide pattern, and the +# `coverage.yml` / `lock-currency.yml` precedent for "a property of the world runs +# on a clock". # # Not a push-to-main trigger either, for the reason stated in every other # scheduled workflow here: `main` only ever advances by fast-forward to a commit @@ -107,8 +112,17 @@ jobs: # locally, and no less wrong in a workflow. - name: Measure run: mise run perf >"$RUNNER_TEMP/perf.txt" + # Two steps where there was one, because the measurement and the verdict + # are now on opposite sides of the engine boundary (CLOUD-1321). The + # producer reduces the records to a p95 per path and files them under a key + # naming hyperfine, its pin and the digest of the binary measured; the gate + # is a `batten check` over the row that reads them back. A record taken over + # bytes that have since changed lives under a different key and does not + # answer, which is the staleness the redirect below could never state. + - name: Record the measurement + run: mise run record-perf <"$RUNNER_TEMP/perf.txt" - name: Assert the budget - run: mise run perf-assert <"$RUNNER_TEMP/perf.txt" + run: mise run perf-assert # THE SEED FOR EVERY PULL REQUEST'S BASE ARM, WRITTEN HERE BECAUSE ONLY # `main` CAN WRITE IT (CLOUD-1342). A cache entry written from a pull # request is scoped to `refs/pull/N/merge` and no other pull request can diff --git a/README.md b/README.md index 92e9750e9..20a4d3fb3 100644 --- a/README.md +++ b/README.md @@ -206,7 +206,7 @@ the launcher's own share is attributable. | path | what it does | p50 | p95 | budget | | ------------- | ------------------------------------------------------ | ------ | ------ | -------- | | `noop` | process start, command tree, render | 2.1 ms | 2.4 ms | ≤ 100 ms | -| `check` | + config load, trust resolution, one-rule tree | 2.3 ms | 2.7 ms | — | +| `check` | + config load, trust resolution, one-rule tree | 2.3 ms | 2.7 ms | ≤ 100 ms | | `hook` | + envelope decode, adjudication, decision write | 2.8 ms | 3.0 ms | ≤ 100 ms | | `passthrough` | a call no rule selects — decode, allow, no config load | — | — | ≤ 100 ms | | `posttool` | a PostToolUse call — decode, capture the response | — | — | ≤ 100 ms | diff --git a/batten.toml b/batten.toml index bbbbbf6f2..028dbead8 100644 --- a/batten.toml +++ b/batten.toml @@ -449,6 +449,16 @@ scope = "mediated_call" module = "policy/leased-push.rego" severity = "deny" +# CLOUD-1340. The mediated half of a variable whose declared half `ci-suite-lane` +# already gates over the workflow files; the two read different surfaces and do +# not overlap. +[[rule]] +id = "hook-skip-local" +kind = "policy" +scope = "mediated_call" +module = "policy/hook-skip-local.rego" +severity = "deny" + [[rule]] id = "gh-run-watch" kind = "shape" @@ -1890,6 +1900,23 @@ regex = '^(mise run )?$' id = "shell-script-directory-marker" regex = '\$\{?BASH_SOURCE|\$0' +# An environment assignment switching `hk` steps off, as one token (CLOUD-1340). +# +# ANCHORED AT THE LEFT EDGE, which is what makes it a name rather than a +# substring: `regex.match` is a SEARCH, so an unanchored row would match any +# token merely CONTAINING the variable — a quoted sentence in an `echo`, or +# another variable ending in the same letters. The right edge is deliberately +# open, because the VALUE is the author's and this row decides only that an +# assignment was written; which values are exempt is the module's clause, where a +# reader can see the reasoning beside the exemption. +# +# ONE CONCEPT, ONE SPELLING: `ci-suite-lane` reads the same variable out of a +# workflow document by key rather than by text, so it needs no pattern and there +# is no second regex for this name anywhere. +[[pattern]] +id = "hook-step-skip-assignment" +regex = '^HK_SKIP_STEPS=' + # The name of a shell variable, for reading the left-hand side of an assignment # without a submatch. A capture would need a format string, and a format string # inside a `regex.*` argument is an inline regex under another spelling — @@ -1898,6 +1925,17 @@ regex = '\$\{?BASH_SOURCE|\$0' id = "shell-identifier" regex = '^[A-Za-z_][A-Za-z0-9_]*$' +# A published budget cell's VALUE, so the number can be taken as the cell's one +# numeric token rather than by stripping every non-digit out of it. README writes +# a gated path's budget as `≤ ms` and an ungated one's as `—`, and the +# difference has to survive: stripping non-digits turns `—` into the empty +# string, which reads as a row publishing nothing rather than as a row publishing +# "not gated". A shape, not a threshold — the numbers themselves are `budgets` in +# `policy/perf-assert.rego`, which is where a value belongs. +[[pattern]] +id = "published-budget-value" +regex = '^[0-9]+$' + # A retirement arm's INVOCATION field (CLOUD-1219). A ledger row declares its # successors as PATHS, which is the wrong shape for a caller of a program that # retired onto a verb: substituting one yields @@ -5434,6 +5472,44 @@ input = "Cargo.lock" # # NULL ON A SHALLOW CLONE, which is this container's own state: the family # refuses to half-answer a history it cannot see, and the module guards for it. +# The latency budget (CLOUD-207), ported off `mise-tasks/perf-assert.sh` under +# CLOUD-1321. The program and `tests/perf-assert.bats` fall in the same commit. +# +# TWO HALVES WITH TWO REACHES, and that is the port working rather than a +# compromise. The README-agreement half reads only tracked lines, so it decides on +# EVERY run — which is what `tests/perf-assert.bats` used to buy per-commit, now +# bought by the gate itself rather than by a suite over it. The measurement half +# reads a record keyed to (tool, version, input digest), so it decides only where +# such a record resolves: inside `.github/workflows/perf.yml`, which is the only +# place `perf-assert` ever ran. +# +# `input = "target/release/batten"` IS THE SUBJECT HYPERFINE MEASURED, and the +# keying is the safety property: rebuild the binary and the record lives under a +# different name, so it is ABSENT rather than stale and this row abstains instead +# of answering from a measurement of other bytes. `Cargo.lock` would be cheaper to +# digest and would be wrong — a source change keeping the lockfile would let a +# stale record answer as current, which is the one failure the key exists to +# prevent. The cost is a read and a hash of the release binary wherever one is +# present; where it is absent — CI's debug-only jobs, a fresh clone — the row +# could not look and costs nothing. +# +# The version tracks `mise.toml`'s `aqua:sharkdp/hyperfine` pin. A differently +# pinned producer writes under a different key and does not answer here, which is +# the same property stated from the tool's side. +[[rule]] +id = "perf-assert" +kind = "policy" +scope = "tree" +lines = ["README.md"] +module = "policy/perf-assert.rego" +severity = "deny" + +[[rule.tools]] +id = "perf-p95" +tool = "hyperfine" +version = "1.20.0" +input = "target/release/batten" + [[rule]] id = "release-tag-shape" kind = "policy" @@ -6308,7 +6384,8 @@ measured = "2026-09-02" # the gate is what the number is compared against — a basis refreshed from a # second reading of the tree would red again on the next lap while looking correct # in review. Which file the two readers disagree about is unresolved and is not -# this bundle's; it is a pointer for whoever takes CLOUD-1158's floor re-derivation.# +# this bundle's; it is a pointer for whoever takes CLOUD-1158's floor re-derivation. +# # THE DISAGREEING READER IS THE GLOB IMPLEMENTATION, NOT A MISSING FILE, and the # entry above leaves it open. Measured 2026-09-02 over one tree: # `git ls-files 'crates/batten/tests/**/*.rs'` and the gate's own walk differ by @@ -6330,15 +6407,58 @@ measured = "2026-09-02" # that tripped it. `target prune`'s stale-basis exit is not its below-floor exit, # and the caller collapses them — so an operator who reads the caller rather than # the callee deletes files and gets nowhere. - +# +# CLOUD-1321 LANDS ON TOP OF THIS BASIS AND DELIBERATELY BUYS NO MOVE, which is +# worth a line because the branch drafted one twice and both times it was a +# duplicate. Its two compiled-binary tiers — `perf_assert.rs` (the +# `mise-tasks/perf-assert.sh` retirement) and `shell_retirement_cost.rs` (the +# deletion-linear term's gate) — put the gate's own reading two past the 175 +# basis, well inside a tolerance of 10, so the gate is silent and there is +# nothing to refresh. +# +# THE FIRST DRAFT OF THIS ENTRY MOVED THE COUNT TO 175 AGAINST A BASIS OF 164, and +# `main` reached the same number first while the branch was open; rebasing turned +# two records of one move into a conflict, twice. Keeping `main`'s and reducing +# this branch's to the observation above is the honest resolution — a second entry +# re-stating a move somebody else made is a ledger row with no reader. The draft +# also read the tree with `git ls-files`, which the entry above now explains is +# not a worse measurement of the same set but a measurement of a DIFFERENT one; +# the sentence is stated in the gate's reading instead. + +# THE 2026-09-03 MOVE, FOURTH OF THE SAME SHAPE, and the repetition is now the +# reading rather than an aside: the basis drifts once per bundle, and every entry +# above says so. `main`'s test files plus CLOUD-1321's three tiers took the live +# count to 186 against a 175 basis — eleven past, one outside the tolerance. +# +# `count` moves and THE FLOORS AND `measured` DELIBERATELY DO NOT, which is the +# half this entry has to be explicit about because the refusal itself asks for +# more. It says "Re-measure the floor and move `count` and `measured` together: a +# count refreshed without a new measurement is the same staleness wearing a newer +# number." That is right, and it is exactly why `measured` is untouched: an honest +# floor reading needs a build from an empty `target` for cold and a minimal +# post-prune tree for warm, which is CLOUD-1158's row and was not taken here. +# Bumping the date to satisfy the sentence would be the staleness it warns about, +# performed on the field that records it. Refreshing the count alone, and saying +# which half is missing, is the honest remedy available to this bundle. +# +# Free space was again not close to the problem: the lap reported 18989MB against +# a 17167MB warm floor. +# +# AND THE CALLER STILL MISREPORTS IT, which is the fourth time this has cost time +# on this branch and the reason it is restated rather than left to the entry +# above. `verify` renders the refusal as "not enough disk to run the gate, and +# pruning did not recover it — the refusal above names free space and the floor." +# The refusal names neither: it names a STEM COUNT, and free space was 1.8GB +# clear. An operator who reads the caller rather than the callee deletes files and +# gets nowhere. [prune.warm.basis] glob = "crates/batten/tests/**/*.rs" -count = 175 +count = 186 tolerance = 10 [prune.cold.basis] glob = "crates/batten/tests/**/*.rs" -count = 175 +count = 186 tolerance = 10 # THE REGROWABLE ROOTS THE ESCALATION MAY DROP (CLOUD-1157), in the order it drops @@ -8105,6 +8225,59 @@ id = "source read first" kind = "document" target = "mise-tasks/sbom-actions.tsv" +[[verdict]] +id = "path measure late" +gloss = "a measured invocation path is over the latency budget this repository publishes" +class = """ +An absolute ceiling rather than a ratchet: 100ms is the Command Line Interface Guidelines' floor for a response that reads as instant, and the ceiling sitting ~20-30x over the measured value IS the tolerance band a shared runner's p95 needs. So this is not noise -- a path here is genuinely slower than the contract batten publishes for itself. Fix what got slower, or move the budget in `policy/perf-assert.rego` AND in README's Performance table together, which the `perf budget unpublished` class refuses to let you do by halves. Whether this commit is slower than the trunk is a different question with a different answer shape and is `perf-gate`'s. +""" + +[[verdict.route]] +id = "module read first" +kind = "document" +target = "policy/perf-assert.rego" + +[[verdict]] +id = "path measure partial" +gloss = "the perf record carries no measurement for a path this repository budgets" +class = """ +A run that measured five of six paths and reported green over the five is the partial-coverage false green this repository keeps re-meeting, so a budgeted path missing from a PRESENT record is a finding rather than a smaller pass. Absence of the whole record is a different state and is silent: a record whose key does not resolve -- a differently-pinned hyperfine, or a binary rebuilt since the measurement -- is not found at all. So this fires only when a producer ran, recorded, and judged less than the budget table claims. +""" + +[[verdict.route]] +id = "module read first" +kind = "document" +target = "policy/perf-assert.rego" + +[[verdict]] +id = "prose state wrong" +gloss = "the budget this gate enforces and the one README publishes disagree" +class = """ +Publishing a number in one file and enforcing it in another is two authorities for one number, and the one that drifts is always the published one because it has no mechanism. Non-negotiable rule 2 is the general form: a rule ships with its mechanism. The remedy is to move both together -- `budgets` in the module and the Performance table's budget column -- never to silence one side. A path the module budgets whose published cell carries no number is this class too, since an ungated cell beside a gated predicate is the same disagreement written the other way round. +""" + +[[verdict.route]] +id = "module read first" +kind = "document" +target = "policy/perf-assert.rego" + +[[verdict.route]] +id = "source read first" +kind = "document" +target = "README.md" + +[[verdict]] +id = "source read missing" +gloss = "README could not be read, so the published budget cannot be held to the enforced one" +class = """ +Could-not-look, never a verdict about the tree. The predecessor exited 2 here for the `lock-complete` and `timeout-check` doctrine -- the gate could not read what it was asked to judge -- and that is distinct from a violation on purpose: a gate reporting green over input it failed to parse is the failure that gets a gate switched off. Restore the file or fix the row's `line_sources`; nothing about the measurement is being claimed. +""" + +[[verdict.route]] +id = "module read first" +kind = "document" +target = "policy/perf-assert.rego" + [[verdict]] id = "tool judge dirty" gloss = "a third-party validator judged this file and reported something" @@ -8639,6 +8812,44 @@ id = "path admit first" kind = "override" precondition = "the history being replaced is this clone's own and no other clone has fetched it, so there is no expected value to name that a reader could check" +# CLOUD-1340, and it is a class for `branch write unsafe`'s reason exactly: a +# consumer `[[rule]]` row carries no token an admission could bind, so a `shape` +# row would leave `bypass_env` as the only way through, which is the password +# shape CLOUD-1051 retired. +# +# THE ROW IS FILED FROM THE SESSION THAT NEEDED IT. `hooks-wiring-check` refused, +# the refusal was read as environmental and unfixable, `HK_SKIP_STEPS` went onto +# three commits, and the false justification went into two commit messages. One +# `batten wiring reclaim -y` cleared the condition. The variable is `hk`'s and +# batten never saw it, so the gate that was switched off had no way to say so. +[[verdict]] +id = "hook skip unseen" +gloss = "a gate is switched off for this call by an environment assignment nothing recorded" +class = """ +`HK_SKIP_STEPS` names steps for `hk` to skip, and batten does not read it — so \ +until this class existed a switched-off gate and a satisfied one were \ +byte-identical from here. The asymmetry is what makes it a defect rather than a \ +missing feature: `ci-suite-lane` already governs the one declared use, over the \ +workflow files, so the deliberate carve was gated and the ad-hoc one was free. \ +NO OVERRIDE ROUTE, on `shell edit refused`'s precedent rather than by oversight. \ +The one case that cannot be satisfied at the commit — a step reading a generated \ +file a LATER commit in the same rebase sequence writes — is already served by \ +`--no-verify` plus the articulation block `commit check` requires, which is gated \ +and leaves a record a reviewer reads. Two mechanisms for one object is what this \ +declines: `--no-verify` is that route, and this class has none. Fix what the step \ +names instead. +""" + +[[verdict.route]] +id = "task run first" +kind = "command" +target = "read the step's own refusal and fix what it names — `batten wiring reclaim -y` is the one for `hooks-wiring-check`" + +[[verdict.route]] +id = "module read first" +kind = "document" +target = "policy/hook-skip-local.rego" + # CLOUD-1311. The punt sweep had a question and no exit code. # # `stop_nudges` rule 5 has asked the right thing at the end of every turn since diff --git a/bench/suites/RESULTS.md b/bench/suites/RESULTS.md index 2d61759f0..1f65a9837 100644 --- a/bench/suites/RESULTS.md +++ b/bench/suites/RESULTS.md @@ -6,121 +6,120 @@ runner measured it; the suite runs `--no-parallelize-within-files`, so a file's number is its own serial cost and is what an author adding a case to it pays. -- suites: 113 -- serial total: 443.0s +- suites: 112 +- serial total: 498.4s | seconds | share | suite | | ---: | ---: | --- | -| 132.8 | 30.0% | `tests/land-lock.bats` | -| 76.5 | 17.3% | `tests/land.bats` | -| 34.6 | 7.8% | `tests/main-watch.bats` | -| 19.2 | 4.3% | `tests/graph-check.bats` | -| 13.1 | 3.0% | `tests/board-diff-overlap.bats` | -| 7.9 | 1.8% | `tests/token-bench.bats` | -| 7.4 | 1.7% | `tests/ready-lint.bats` | -| 7.4 | 1.7% | `tests/target-race.bats` | -| 6.7 | 1.5% | `tests/released.bats` | -| 6.0 | 1.4% | `tests/ready-guard.bats` | -| 6.0 | 1.3% | `tests/board-sweep.bats` | -| 5.2 | 1.2% | `tests/release-tracking-check.bats` | -| 4.9 | 1.1% | `tests/singleton.bats` | -| 4.8 | 1.1% | `tests/release-assets-check.bats` | -| 4.8 | 1.1% | `tests/in-progress-drain.bats` | -| 4.7 | 1.1% | `tests/sbom.bats` | -| 4.1 | 0.9% | `tests/step-receipt.bats` | -| 3.9 | 0.9% | `tests/mcp-allow-check.bats` | -| 3.8 | 0.9% | `tests/task-registry.bats` | -| 3.6 | 0.8% | `tests/hk-selection.bats` | -| 3.5 | 0.8% | `tests/land-divergence.bats` | -| 3.5 | 0.8% | `tests/doctor-race.bats` | -| 3.3 | 0.7% | `tests/ready-cites-check.bats` | -| 3.2 | 0.7% | `tests/ntia-check.bats` | -| 3.1 | 0.7% | `tests/target-ensure.bats` | -| 2.7 | 0.6% | `tests/with-lock.bats` | -| 2.2 | 0.5% | `tests/finding-sink-check.bats` | -| 2.2 | 0.5% | `tests/landed-check.bats` | -| 2.1 | 0.5% | `tests/closing-key-check.bats` | -| 2.1 | 0.5% | `tests/install.bats` | -| 1.9 | 0.4% | `tests/claim-race-check.bats` | -| 1.8 | 0.4% | `tests/suite-select.bats` | -| 1.6 | 0.4% | `tests/ci-slow-needed.bats` | -| 1.6 | 0.4% | `tests/reclaim-census.bats` | -| 1.6 | 0.4% | `tests/spec-ref-check.bats` | -| 1.5 | 0.3% | `tests/signing-posture.bats` | -| 1.5 | 0.3% | `tests/claimed-keys.bats` | -| 1.4 | 0.3% | `tests/tree-clean.bats` | -| 1.4 | 0.3% | `tests/evaluator-closure-check.bats` | +| 131.4 | 26.4% | `tests/land-lock.bats` | +| 91.5 | 18.4% | `tests/land.bats` | +| 34.5 | 6.9% | `tests/main-watch.bats` | +| 25.2 | 5.1% | `tests/graph-check.bats` | +| 15.0 | 3.0% | `tests/board-diff-overlap.bats` | +| 10.1 | 2.0% | `tests/token-bench.bats` | +| 9.6 | 1.9% | `tests/ready-lint.bats` | +| 7.9 | 1.6% | `tests/released.bats` | +| 7.6 | 1.5% | `tests/target-race.bats` | +| 7.6 | 1.5% | `tests/board-sweep.bats` | +| 7.0 | 1.4% | `tests/ready-guard.bats` | +| 6.4 | 1.3% | `tests/in-progress-drain.bats` | +| 6.3 | 1.3% | `tests/release-assets-check.bats` | +| 6.2 | 1.2% | `tests/release-tracking-check.bats` | +| 5.8 | 1.2% | `tests/sbom.bats` | +| 5.1 | 1.0% | `tests/singleton.bats` | +| 5.0 | 1.0% | `tests/mcp-allow-check.bats` | +| 5.0 | 1.0% | `tests/step-receipt.bats` | +| 4.9 | 1.0% | `tests/hk-selection.bats` | +| 4.6 | 0.9% | `tests/land-divergence.bats` | +| 4.2 | 0.8% | `tests/task-registry.bats` | +| 3.9 | 0.8% | `tests/ntia-check.bats` | +| 3.8 | 0.8% | `tests/doctor-race.bats` | +| 3.5 | 0.7% | `tests/ready-cites-check.bats` | +| 3.2 | 0.6% | `tests/with-lock.bats` | +| 3.2 | 0.6% | `tests/landed-check.bats` | +| 2.8 | 0.6% | `tests/closing-key-check.bats` | +| 2.7 | 0.5% | `tests/finding-sink-check.bats` | +| 2.7 | 0.5% | `tests/target-ensure.bats` | +| 2.4 | 0.5% | `tests/install.bats` | +| 2.2 | 0.4% | `tests/suite-select.bats` | +| 2.2 | 0.4% | `tests/claim-race-check.bats` | +| 1.9 | 0.4% | `tests/claimed-keys.bats` | +| 1.9 | 0.4% | `tests/spec-ref-check.bats` | +| 1.8 | 0.4% | `tests/signing-posture.bats` | +| 1.8 | 0.4% | `tests/reclaim-census.bats` | +| 1.7 | 0.3% | `tests/ci-tools-check.bats` | +| 1.7 | 0.3% | `tests/alive.bats` | +| 1.6 | 0.3% | `tests/install-check.bats` | +| 1.6 | 0.3% | `tests/ready-lint-deferral.bats` | +| 1.6 | 0.3% | `tests/ci-slow-needed.bats` | +| 1.5 | 0.3% | `tests/tree-clean.bats` | +| 1.5 | 0.3% | `tests/verify.bats` | +| 1.5 | 0.3% | `tests/deferral-check.bats` | +| 1.4 | 0.3% | `tests/done-check.bats` | +| 1.4 | 0.3% | `tests/land-divergence-assert.bats` | | 1.4 | 0.3% | `tests/ci-lease-precondition.bats` | -| 1.4 | 0.3% | `tests/ci-tools-check.bats` | -| 1.3 | 0.3% | `tests/alive.bats` | -| 1.2 | 0.3% | `tests/verify.bats` | -| 1.2 | 0.3% | `tests/ready-lint-deferral.bats` | -| 1.2 | 0.3% | `tests/install-check.bats` | -| 1.1 | 0.3% | `tests/perf-record.bats` | -| 1.1 | 0.2% | `tests/deferral-check.bats` | -| 1.1 | 0.2% | `tests/land-divergence-assert.bats` | -| 1.0 | 0.2% | `tests/linear-check.bats` | -| 1.0 | 0.2% | `tests/done-check.bats` | -| 1.0 | 0.2% | `tests/render-cli.bats` | -| 0.9 | 0.2% | `tests/nonverdict-scan.bats` | -| 0.9 | 0.2% | `tests/spawn-census.bats` | -| 0.9 | 0.2% | `tests/release-backfill.bats` | -| 0.9 | 0.2% | `tests/hook-matcher-check.bats` | +| 1.3 | 0.3% | `tests/perf-record.bats` | +| 1.2 | 0.2% | `tests/spawn-census.bats` | +| 1.2 | 0.2% | `tests/lint-rego.bats` | +| 1.2 | 0.2% | `tests/evaluator-closure-check.bats` | +| 1.2 | 0.2% | `tests/linear-check.bats` | +| 1.2 | 0.2% | `tests/commit-attribution.bats` | +| 1.1 | 0.2% | `tests/release-backfill.bats` | +| 1.1 | 0.2% | `tests/nonverdict-scan.bats` | +| 1.1 | 0.2% | `tests/render-cli.bats` | +| 1.1 | 0.2% | `tests/done-pr-check.bats` | +| 1.1 | 0.2% | `tests/doctor.bats` | +| 1.0 | 0.2% | `tests/hook-matcher-check.bats` | +| 1.0 | 0.2% | `tests/awk-regex-check.bats` | +| 1.0 | 0.2% | `tests/duplicate-close-check.bats` | +| 1.0 | 0.2% | `tests/lint-deno.bats` | +| 0.9 | 0.2% | `tests/attestation-check.bats` | +| 0.9 | 0.2% | `tests/timeout-drift.bats` | | 0.9 | 0.2% | `tests/module-map-check.bats` | -| 0.9 | 0.2% | `tests/macos-link-check.bats` | -| 0.8 | 0.2% | `tests/awk-regex-check.bats` | -| 0.8 | 0.2% | `tests/perf-assert.bats` | -| 0.8 | 0.2% | `tests/lint-rego.bats` | -| 0.8 | 0.2% | `tests/done-pr-check.bats` | -| 0.8 | 0.2% | `tests/attestation-check.bats` | -| 0.8 | 0.2% | `tests/pr-unsubscribed.bats` | -| 0.8 | 0.2% | `tests/doctor.bats` | -| 0.7 | 0.2% | `tests/commit-attribution.bats` | -| 0.7 | 0.2% | `tests/lint-deno.bats` | -| 0.7 | 0.2% | `tests/sbom-binary.bats` | -| 0.7 | 0.2% | `tests/timeout-drift.bats` | -| 0.7 | 0.1% | `tests/duplicate-close-check.bats` | -| 0.6 | 0.1% | `tests/mcp-timeout-budget.bats` | -| 0.6 | 0.1% | `tests/verified.bats` | -| 0.6 | 0.1% | `tests/evaluator-io-check.bats` | -| 0.6 | 0.1% | `tests/merged-pr-keys.bats` | -| 0.6 | 0.1% | `tests/suite-bench-check.bats` | -| 0.6 | 0.1% | `tests/mcp-attach-check.bats` | -| 0.5 | 0.1% | `tests/stop-posture-check.bats` | -| 0.5 | 0.1% | `tests/checksums.bats` | +| 0.9 | 0.2% | `tests/sbom-binary.bats` | +| 0.9 | 0.2% | `tests/pr-unsubscribed.bats` | +| 0.8 | 0.2% | `tests/mcp-timeout-budget.bats` | +| 0.7 | 0.1% | `tests/commit-convention.bats` | +| 0.7 | 0.1% | `tests/mcp-attach-check.bats` | +| 0.7 | 0.1% | `tests/macos-link-check.bats` | +| 0.7 | 0.1% | `tests/suite-bench-check.bats` | +| 0.7 | 0.1% | `tests/merged-pr-keys.bats` | +| 0.7 | 0.1% | `tests/stop-posture-check.bats` | +| 0.7 | 0.1% | `tests/verified.bats` | +| 0.6 | 0.1% | `tests/checksums.bats` | +| 0.6 | 0.1% | `tests/connector-allow-guard.bats` | +| 0.6 | 0.1% | `tests/hook-pin-check.bats` | +| 0.6 | 0.1% | `tests/land-lock-check.bats` | +| 0.6 | 0.1% | `tests/digest-major-agreement.bats` | +| 0.5 | 0.1% | `tests/pipefail-grep-check.bats` | | 0.5 | 0.1% | `tests/publish-credential-check.bats` | -| 0.4 | 0.1% | `tests/hook-pin-check.bats` | -| 0.4 | 0.1% | `tests/connector-allow-guard.bats` | -| 0.4 | 0.1% | `tests/digest-major-agreement.bats` | -| 0.4 | 0.1% | `tests/land-lock-check.bats` | -| 0.4 | 0.1% | `tests/pipefail-grep-check.bats` | -| 0.4 | 0.1% | `tests/msrv-pin-agreement.bats` | -| 0.4 | 0.1% | `tests/board-payloads.bats` | -| 0.4 | 0.1% | `tests/abandon-matrix.bats` | -| 0.4 | 0.1% | `tests/branch-age-check.bats` | -| 0.4 | 0.1% | `tests/sonar-gate.bats` | -| 0.4 | 0.1% | `tests/serena-mcp.bats` | +| 0.5 | 0.1% | `tests/board-payloads.bats` | +| 0.5 | 0.1% | `tests/branch-age-check.bats` | +| 0.5 | 0.1% | `tests/msrv-pin-agreement.bats` | +| 0.5 | 0.1% | `tests/abandon-matrix.bats` | +| 0.5 | 0.1% | `tests/timeout-check.bats` | +| 0.5 | 0.1% | `tests/sonar-gate.bats` | +| 0.5 | 0.1% | `tests/serena-mcp.bats` | | 0.4 | 0.1% | `tests/transcript-corpus-check.bats` | -| 0.3 | 0.1% | `tests/commit-convention.bats` | -| 0.3 | 0.1% | `tests/timeout-check.bats` | -| 0.3 | 0.1% | `tests/report-only-check.bats` | -| 0.3 | 0.1% | `tests/no-doctests.bats` | -| 0.3 | 0.1% | `tests/connector-allow-resolve.bats` | -| 0.3 | 0.1% | `tests/license-table-check.bats` | -| 0.3 | 0.1% | `tests/nonverdict-assert.bats` | -| 0.3 | 0.1% | `tests/release-due.bats` | -| 0.3 | 0.1% | `tests/cap-drift.bats` | +| 0.4 | 0.1% | `tests/no-doctests.bats` | +| 0.4 | 0.1% | `tests/license-table-check.bats` | +| 0.4 | 0.1% | `tests/release-due.bats` | +| 0.4 | 0.1% | `tests/report-only-check.bats` | +| 0.4 | 0.1% | `tests/connector-allow-resolve.bats` | +| 0.4 | 0.1% | `tests/nonverdict-assert.bats` | +| 0.4 | 0.1% | `tests/cap-drift.bats` | +| 0.4 | 0.1% | `tests/git-hook.bats` | | 0.3 | 0.1% | `tests/container-preflight.bats` | | 0.3 | 0.1% | `tests/batten-glob-check.bats` | | 0.3 | 0.1% | `tests/coderabbit-config-check.bats` | -| 0.2 | 0.1% | `tests/rust-paths-check.bats` | -| 0.2 | 0.1% | `tests/git-hook.bats` | +| 0.3 | 0.1% | `tests/rust-paths-check.bats` | +| 0.3 | 0.1% | `tests/evaluator-io-check.bats` | | 0.2 | 0.0% | `tests/mise-action-floor.bats` | | 0.2 | 0.0% | `tests/remedy-payload-source.bats` | +| 0.2 | 0.0% | `tests/dist.bats` | | 0.2 | 0.0% | `tests/token-bench-check.bats` | -| 0.1 | 0.0% | `tests/dist.bats` | -| 0.1 | 0.0% | `tests/task-fail-closed.bats` | +| 0.2 | 0.0% | `tests/task-fail-closed.bats` | | 0.1 | 0.0% | `tests/egress-check.bats` | -| 0.1 | 0.0% | `tests/darwin-link.bats` | | 0.1 | 0.0% | `tests/cross-check.bats` | +| 0.1 | 0.0% | `tests/darwin-link.bats` | | 0.0 | 0.0% | `tests/zizmor-split.bats` | diff --git a/crates/batten/tests/it/cli.rs b/crates/batten/tests/it/cli.rs index 6d09562f4..f6a6ba947 100644 --- a/crates/batten/tests/it/cli.rs +++ b/crates/batten/tests/it/cli.rs @@ -262,6 +262,27 @@ fn committed_budget_surfaces(dir: &Path) { fs::create_dir_all(dir.join(".serena")).expect("create fixture serena dir"); fs::write(dir.join(".serena/project.yml"), "initial_prompt: ''\n") .expect("write fixture project config"); + // The committed `perf-assert` row declares `README.md` as a LITERAL `lines` + // entry (CLOUD-1321), so it is acquired whether or not this fixture has one + // and an absent file reaches the module through `input.tree.missing`. That is + // deliberate there — a glob would match nothing in a treeless fixture and the + // could-not-look clause would be unreachable — and it means a fixture running + // the committed config owes this surface exactly as it owes `AGENTS.md`. + // + // The table AGREES with the module's `budgets`, so a case about some other + // rule is not also a case about the published budget. + fs::write( + dir.join("README.md"), + "| path | what it does | p50 | p95 | budget |\n\ + | ---- | ------------ | --- | --- | ------ |\n\ + | `noop` | process start | 2.1 ms | 2.4 ms | \u{2264} 100 ms |\n\ + | `check` | one-rule tree | 2.3 ms | 2.7 ms | \u{2264} 100 ms |\n\ + | `hook` | adjudication | 2.8 ms | 3.0 ms | \u{2264} 100 ms |\n\ + | `passthrough` | a call no rule selects | \u{2014} | \u{2014} | \u{2264} 100 ms |\n\ + | `posttool` | a PostToolUse call | \u{2014} | \u{2014} | \u{2264} 100 ms |\n\ + | `wired` | as settings.json invokes it | 8.0 ms | 8.4 ms | \u{2264} 100 ms |\n", + ) + .expect("write fixture README"); committed_policy_modules(dir); } diff --git a/crates/batten/tests/it/hook_skip_local.rs b/crates/batten/tests/it/hook_skip_local.rs new file mode 100644 index 000000000..c097f9e51 --- /dev/null +++ b/crates/batten/tests/it/hook_skip_local.rs @@ -0,0 +1,132 @@ +//! Switching an `hk` step off locally, over the compiled binary and the +//! committed table (CLOUD-1340). +//! +//! # The gap this covers +//! +//! Measured 2026-09-02 on this branch. `hooks-wiring-check` refused; the session +//! read the refusal as environmental and unfixable, set `HK_SKIP_STEPS` on three +//! commits, wrote that justification into two commit messages, and put a +//! four-option menu to a human. `batten wiring reclaim -y` cleared the condition +//! in one command. **Nothing in Batten fired at any point** — the variable is read +//! by `hk`, which batten never sees, so a switched-off gate and a satisfied one +//! were byte-identical from here. +//! +//! # Why the exemption case is the load-bearing one +//! +//! `ci-suite-lane` already governs this variable where CI sets it, so the +//! DECLARED use is gated and the ad-hoc one was free — a hole shaped exactly like +//! the repository's own legitimate use. That shape is what makes the exemption +//! assertion matter more than the deny: a row that refused the `ci` job's own line +//! would be a guard people switch off rather than satisfy, which is the failure +//! this whole family exists to avoid. So `HK_SKIP_STEPS=test:bats` must be +//! allowed, and `HK_SKIP_STEPS=test:bats,batten-check` must not — the second is +//! what an author reaches for after finding the first in a workflow file. +//! +//! # This is the tier that proves the key exists +//! +//! The module's own `test_` rules fabricate `input.call.segments` with +//! `with input as`, so they pass over a shape the engine may never build — +//! `.claude/rules/policy-modules.md`'s opening defect, and the reason both live +//! instances of it were found by adding a tier like this one. The specific risk +//! here is real rather than notional: `hook::is_env_assignment` is what the +//! boundary uses to look THROUGH an assignment when resolving the effective +//! program, so `input.call.programs` reports `git` and never the variable. These +//! cases are what prove the token survives in `words`. +//! +//! Judged against the committed `batten.toml` rather than a fixture, for +//! `forced_push.rs`'s reason: a fixture would assert the engine CAN express this, +//! which was never in doubt. What is in doubt is whether the table this +//! repository ships refuses the command. + +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use crate::common; + +/// A Claude Code `PreToolUse` envelope carrying a shell command. +fn bash_payload(command: &str) -> String { + let escaped = serde_json::to_string(command).expect("a command is encodable"); + format!( + "{{\"hook_event_name\":\"PreToolUse\",\"tool_name\":\"Bash\",\ + \"tool_input\":{{\"command\":{escaped}}}}}" + ) +} + +fn decision(command: &str) -> String { + let root = common::at_root("."); + common::stdout(&common::run_with_stdin( + &root, + &["hook", "--harness", "claude-code"], + &bash_payload(command), + )) +} + +/// Refused, and by THIS row — an assertion that would go green on some other +/// row's coverage proves nothing about this one. +fn denied_by_this_row(command: &str) { + let out = decision(command); + assert!( + out.contains("\"deny\""), + "the committed policy must refuse: {command}\n{out}" + ); + assert!( + out.contains("hook-skip-local"), + "the refusal for `{command}` must come from this row\n{out}" + ); +} + +fn allowed(command: &str) { + let out = decision(command); + assert!( + !out.contains("\"deny\""), + "the committed policy must allow: {command}\n{out}" + ); +} + +#[test] +fn a_local_step_skip_is_refused() { + // The measured command, as it was actually run on this branch. + denied_by_this_row("HK_SKIP_STEPS=hooks-wiring-check git commit -m 'x'"); + denied_by_this_row("HK_SKIP_STEPS=batten-check,test:cargo git commit --amend --no-edit"); +} + +#[test] +fn a_step_skip_behind_a_compound_command_is_still_reached() { + // `input.call.segments`, not the first word of the line (CLOUD-857). A real + // agent command is compound most of the time, and `git add -A && git + // commit` is the exact shape this session ran. + denied_by_this_row("git add -A && HK_SKIP_STEPS=hooks-wiring-check git commit -m 'x'"); + denied_by_this_row("cd /home/user/batten && HK_SKIP_STEPS=batten-check git commit -m 'x'"); +} + +#[test] +fn the_declared_ci_carve_is_not_judged_here() { + // THE CASE THAT KEEPS THIS FROM BEING SWITCHED OFF. `.github/workflows/ci.yml` + // hands hk exactly this, and `ci-suite-lane` is the row that governs it. A + // guard refusing the repository's own declared invocation gets disabled, and + // then it enforces nothing at all. + allowed("HK_SKIP_STEPS=test:bats mise run ci"); +} + +#[test] +fn the_carve_with_a_step_appended_is_refused() { + // The arm a prefix test would lose, and the one an author actually reaches + // for: find the line in a workflow, add "just one more" step to it. + denied_by_this_row("HK_SKIP_STEPS=test:bats,batten-check mise run ci"); + denied_by_this_row("HK_SKIP_STEPS=test:bats,hooks-wiring-check mise run verify"); +} + +#[test] +fn an_ordinary_command_is_allowed() { + // ANTI-VACUITY. Without these the denies above are satisfied by a build that + // refuses every command, which would name this row every time. + allowed("git commit -m 'an ordinary commit'"); + allowed("mise run ci"); + allowed("RUST_LOG=debug git commit -m 'another variable is not this one'"); +} + +#[test] +fn a_quoted_mention_is_not_an_invocation() { + // The tokenizer's own quoting, reached through the engine rather than + // re-derived: prose naming the variable is not a command setting it. + allowed("echo 'set HK_SKIP_STEPS=x to skip a step'"); +} diff --git a/crates/batten/tests/it/main.rs b/crates/batten/tests/it/main.rs index 393ed8592..81bf9901f 100644 --- a/crates/batten/tests/it/main.rs +++ b/crates/batten/tests/it/main.rs @@ -119,6 +119,7 @@ mod history_facts; mod hk_fix_selection; mod hook_cost; mod hook_profile; +mod hook_skip_local; mod hook_worktree_root; mod identity_churn; mod identity_precedence; @@ -139,6 +140,7 @@ mod mutate; mod mutation_declared_case; mod narrow_adoption; mod obligations_bound; +mod perf_assert; mod perf_compare; mod perf_pair; mod pinned_programs; @@ -179,6 +181,7 @@ mod retirement_doctrine; mod review_answered; mod review_dispatched; mod rule_cost_census; +mod rule_cost_rung; mod rules_builtin_claims; mod rules_drift; mod run_shape; @@ -191,6 +194,7 @@ mod semver_gate; mod session_drain; mod session_provisioning; mod shell_retirement; +mod shell_retirement_cost; mod shell_write_advisory; mod sinks; mod skill_contract; diff --git a/crates/batten/tests/it/mediated_verbs.rs b/crates/batten/tests/it/mediated_verbs.rs index 27a7d9cc3..3d31c9b71 100644 --- a/crates/batten/tests/it/mediated_verbs.rs +++ b/crates/batten/tests/it/mediated_verbs.rs @@ -478,13 +478,39 @@ fn write_payload(tool: &str, path: &str) -> String { } fn write_verdict(tool: &str, path: &str) -> Option { - run_with_stdin_at_real_root( + write_decision(tool, path).0 +} + +/// The verdict AND what the engine said reaching it. +/// +/// **An exit code alone cannot be debugged from a CI log** (CLOUD-1388). The +/// `windows` job reported `left: Some(0) right: Some(2)` for an absolute +/// `batten.toml` and nothing else — no pointer, no path, no clue whether the +/// target was relativised, matched, or never read as a write at all. Three +/// separate hypotheses were argued from that one integer and all three were +/// wrong, because the number is the same whichever of them holds. +/// +/// `-vv` is the rung the engine renders its own reasoning at, so the failure +/// message carries it. This costs nothing on a green run and is the difference +/// between one round and three on a red one. +fn write_decision(tool: &str, path: &str) -> (Option, String) { + let output = run_with_stdin_at_real_root( &root(), - &["hook", "--harness", "exit-code"], + &["hook", "--harness", "exit-code", "-vv"], &write_payload(tool, path), - ) - .status - .code() + ); + let told = format!( + "sent {tool} at {path}\n stdout: {}\n stderr: {}", + common::stdout(&output).trim(), + common::stderr(&output).trim() + ); + (output.status.code(), told) +} + +/// Assert a write is refused, and say what the engine did when it is not. +fn assert_write_refused(tool: &str, path: &str, why: &str) { + let (code, told) = write_decision(tool, path); + assert_eq!(code, Some(2), "{why}\n{told}"); } /// THE DISCRIMINATING PAIR (CLOUD-1133). One protected target, two spellings. @@ -503,15 +529,11 @@ fn a_protected_write_is_refused_in_both_spellings_the_host_can_send() { .canonicalize() .expect("the repository root resolves") .join(GUARDED); - assert_eq!( - write_verdict("Write", GUARDED), - Some(2), - "the relative spelling is refused" - ); - assert_eq!( - write_verdict("Write", &absolute.display().to_string()), - Some(2), - "and so is the absolute one, which is what the host actually sends" + assert_write_refused("Write", GUARDED, "the relative spelling is refused"); + assert_write_refused( + "Write", + &absolute.display().to_string(), + "and so is the absolute one, which is what the host actually sends", ); } @@ -525,10 +547,10 @@ fn the_absolute_spelling_is_refused_for_every_write_tool() { .display() .to_string(); for tool in ["Write", "Edit", "MultiEdit"] { - assert_eq!( - write_verdict(tool, &absolute), - Some(2), - "{tool} at an absolute protected path is refused" + assert_write_refused( + tool, + &absolute, + &format!("{tool} at an absolute protected path is refused"), ); } } diff --git a/crates/batten/tests/it/perf_assert.rs b/crates/batten/tests/it/perf_assert.rs new file mode 100644 index 000000000..7e7fe07ce --- /dev/null +++ b/crates/batten/tests/it/perf_assert.rs @@ -0,0 +1,405 @@ +//! `policy/perf-assert.rego` over the compiled binary (CLOUD-207, retired under +//! CLOUD-1321). +//! +//! **This is the tier the module's own `test_` rules cannot be.** A `with input +//! as` block writes the shape it then reads, so it is green over a key the engine +//! never fills — CLOUD-845's defect, and the one +//! `.claude/rules/policy-modules.md` records both live instances of. Two claims +//! here are exactly of that kind and are asserted nowhere else: that the engine +//! fills `input.tree.lines["README.md"]` from a row declaring it, and that +//! `input.tree.missing` reaches the module when it cannot. +//! +//! **The row declares `lines`, not `line_sources`, and the difference is a dead +//! gate.** `line_sources` is the GLOB field: its entries are matched against the +//! walked file list, so an absent `README.md` matches nothing, is never declared, +//! is never acquired, and never reaches `input.tree.missing` — the could-not-look +//! clause simply cannot fire. `lines` is the literal field and is unioned into the +//! declared set unconditionally, so an absent path is acquired, fails, and is +//! named with its cause. The first spelling here was `line_sources` and +//! `an_unreadable_readme_is_reported` is what caught it; every other case in this +//! file passed over it, which is the shape the second tier exists for. +//! +//! **The producer is RUN, never planted.** `batten record tool` writes the record +//! and `batten check` reads it back, so these cases prove the writer and the +//! reader compose the SAME key. A hand-written record agrees with the reader by +//! construction, which is how `validator-verdict-clean` shipped resolving `null` +//! on every real checkout — the only writer in the tree was a test helper. +//! +//! The module read here is the COMMITTED one, copied into each scratch tree, and +//! the pattern row is derived from the committed table rather than restated: an +//! inline copy of either would drift and pass while the real gate was broken. +//! +//! # RETIREMENT LEDGER, PER PATH — what `shell-retirement` reads +//! +//! `perf-assert.sh` was a pure function of stdin that adjudicated two questions +//! over data something else measured. Both move: the measurement verdict onto the +//! `perf-p95` `[[rule.tools]]` row, and the README-agreement clause onto the +//! module. The MEASUREMENT itself never lived in the program and does not move — +//! `mise run perf` still spawns hyperfine, and `.github/workflows/perf.yml` still +//! runs it. What changes is that the records now reach a gate through a key +//! naming the tool, its pin and the digest of the binary measured, so a record +//! taken over bytes that have since changed is absent rather than stale. The +//! predecessor's stdin pipe could not state that at all. + +// carried: mise-tasks/perf-assert.sh policy/perf-assert.rego crates/batten/tests/it/perf_assert.rs +// carried: tests/perf-assert.bats policy/perf-assert.rego crates/batten/tests/it/perf_assert.rs + +//! # RETIREMENT LEDGER — `tests/perf-assert.bats`, 17 cases +//! +//! CARRIED — the budget verdict, the presence gate and the README clause, which +//! are the whole of what the gate decided. + +// carried: "records inside budget pass, and say so" crates/batten/tests/it/perf_assert.rs +// carried: "the real README publishes the budgets this gate enforces" crates/batten/tests/it/perf_assert.rs +// carried: "a budgeted path over its budget is a violation, and is named" crates/batten/tests/it/perf_assert.rs +// carried: "a budgeted path missing from the records is exit 2, not a pass" crates/batten/tests/it/perf_assert.rs +// carried: "a README publishing a different budget fails" crates/batten/tests/it/perf_assert.rs +// carried: "a README with no row for a budgeted path fails" crates/batten/tests/it/perf_assert.rs +// carried: "a missing README is could-not-look, not a pass" crates/batten/tests/it/perf_assert.rs +// carried: "a wired path over budget is named, with its measurement and its ceiling" crates/batten/tests/it/perf_assert.rs +// carried: "a missing wired record is exit 2 — could not look is not a pass" crates/batten/tests/it/perf_assert.rs +// carried: "a missing posttool record is exit 2 — the capture cost cannot go unmeasured" crates/batten/tests/it/perf_assert.rs +// carried: "a posttool path over budget is named, with its measurement and its ceiling" crates/batten/tests/it/perf_assert.rs +// carried: "a README with no wired row fails, so the budget cannot be enforced unpublished" crates/batten/tests/it/perf_assert.rs + +//! CHANGED — the ungated-path case, and it is the one this row deliberately +//! inverts. `check` was measured and NOT budgeted, on the predecessor's argument +//! that a tree walk over a large consumer repo is legitimately slower and no +//! ceiling could tell that apart from a regression. CLOUD-1321 re-decides it with +//! a number: the `perf` arm is a one-rule FIXTURE repo, so it is bounded by what +//! batten costs rather than by a consumer's tree, which is the reasoning that +//! budgets `noop`. So the successor asserts the opposite of what this case +//! asserted, and says so here rather than letting a `carried` arm hide it. + +// changed: "the ungated path is never a violation, however slow" crates/batten/tests/it/perf_assert.rs `check` is budgeted now (CLOUD-1321); the successor is `a_measurement_inside_every_budget_is_silent`, which includes `check` among the paths a clean record must satisfy, and an unbudgeted path staying ungated is `an_unbudgeted_path_in_the_record_is_ignored` in the module tier + +//! CHANGED — the four stdin-parsing cases. Their subject was the program's own +//! record parser, and there is no parser left to test: the record reaches the +//! module as a projected map the engine composed, so "a line that is not a +//! record" is refused by `batten record tool` at the WRITE (it demands +//! ` `) rather than adjudicated at the read. The could-not-look +//! meaning is conserved and its exit code moves from the program's `2` to the +//! engine's contract. + +// changed: "empty stdin is exit 2, and names the redirect" crates/batten/tests/it/perf_assert.rs the record store is read by key rather than piped, so there is no stdin to be empty and an absent record makes the module abstain — `no_record_at_all_is_silent` +// changed: "whitespace-only stdin is empty, not malformed" crates/batten/tests/it/perf_assert.rs the record store is read by key rather than piped, so there is no stdin to be whitespace and an absent record makes the module abstain — `no_record_at_all_is_silent` +// changed: "a line that is not a record is exit 2 and points at the line" crates/batten/tests/it/perf_assert.rs `batten record tool` refuses a line carrying no token at the WRITE, so the reader never sees a malformed one and the pointer moves to the producer +// changed: "a record whose p95 is not a number is malformed, not zero" crates/batten/tests/it/perf_assert.rs the comparison is numeric in Rego and a non-numeric token cannot satisfy it, so such a path is not judged rather than read as zero + +// Panicking on setup failure is the idiomatic way for a test to fail loudly. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use crate::common; + +use std::fmt::Write as _; +use std::path::{Path, PathBuf}; + +use common::{batten, git_in, run_with_stdin, scratch, stderr, stdout, write}; + +/// The pin the fixture row declares. Any value works — what matters is that the +/// producer and the reader compose the key from the SAME one. +const DECLARED_VERSION: &str = "1.20.0"; + +/// The bytes standing in for the measured binary. Small, because the digest is +/// the point and the size is not. +const SUBJECT: &str = "the release binary hyperfine measured\n"; + +/// The committed pattern row this module resolves, rendered back as TOML. +/// +/// DERIVED, never restated: an inline regex here would drift from `batten.toml` +/// and let this tier pass over a module whose reference the real config could not +/// satisfy. +fn pattern_rows() -> String { + let committed = common::committed_patterns(); + let row = committed + .iter() + .find(|pattern| pattern.id == "published-budget-value") + .expect("the committed table declares the row the module resolves"); + format!( + "[[pattern]]\nid = \"{}\"\nregex = '{}'\n", + row.id, row.regex + ) +} + +/// Exactly the four classes `policy/perf-assert.rego` raises. +/// +/// **NAMED, NEVER PREFIX-MATCHED, and that is a measured correction** (CLOUD-1321). +/// This selected on `starts_with("path measure")`, `("prose state")` and +/// `("source read")`, which reads as "the module's families" and is really "every +/// class anybody ever names that way". A rebase brought in `prose state other`, +/// raised by an unrelated `pr-partition-restated` row; the prefix swept it into a +/// bundle that enables ONE module, nothing there raises it, and the registry's own +/// both-directions check failed the config LOAD. The fixture cannot grow a class +/// its module does not raise, so the list is the four ids and the coupling to +/// somebody else's naming is gone. +const RAISED: [&str; 4] = [ + "path measure late", + "path measure partial", + "prose state wrong", + "source read missing", +]; + +/// The committed verdict rows this module raises, rendered back as TOML. +/// +/// Still DERIVED from the committed table rather than restated, for +/// `pattern_rows`' reason and because a module raising a token no row declares +/// fails to LOAD — so a restated table that fell behind would redden every case +/// here over a module that is fine. What changed is only which rows are selected. +fn verdict_rows() -> String { + let declared = common::verdicts_in(&common::at_root(".")); + let mut rows = String::new(); + for id in RAISED { + // A MISS IS LOUD. Selecting by name means a rename in the committed table + // yields a SHORT list rather than a wrong one, and a short list is a + // fixture whose module raises a token nothing declares — the same load + // failure arriving from the other side, with nothing to point at. + assert!( + declared.iter().any(|verdict| verdict.id == id), + "the committed table no longer declares `{id}`, which \ + `policy/perf-assert.rego` raises — rename it in both places" + ); + let _ = write!( + rows, + "[[verdict]]\nid = \"{id}\"\ngloss = \"a fixture gloss\"\nclass = \"A fixture class.\"\n\n\ + [[verdict.route]]\nid = \"module read first\"\nkind = \"document\"\ntarget = \"policy/perf-assert.rego\"\n\n", + ); + } + rows +} + +fn config() -> String { + format!( + r#"version = 1 + +[[rule]] +id = "perf-assert" +kind = "policy" +scope = "tree" +lines = ["README.md"] +module = "policy/perf-assert.rego" +severity = "deny" + +[[rule.tools]] +id = "perf-p95" +tool = "hyperfine" +version = "{DECLARED_VERSION}" +input = "subject.bin" + +{} +{}"#, + pattern_rows(), + verdict_rows() + ) +} + +/// A README table publishing `budget` for every path the module gates. +fn agreeing_readme() -> String { + "| path | what it does | p50 | p95 | budget |\n\ + | ---- | ------------ | --- | --- | ------ |\n\ + | `noop` | process start | 2.1 ms | 2.4 ms | ≤ 100 ms |\n\ + | `check` | one-rule tree | 2.3 ms | 2.7 ms | ≤ 100 ms |\n\ + | `hook` | adjudication | 2.8 ms | 3.0 ms | ≤ 100 ms |\n\ + | `passthrough` | a call no rule selects | — | — | ≤ 100 ms |\n\ + | `posttool` | a PostToolUse call | — | — | ≤ 100 ms |\n\ + | `wired` | as settings.json invokes it | 8.0 ms | 8.4 ms | ≤ 100 ms |\n" + .to_owned() +} + +/// Every budgeted path measured comfortably inside its ceiling, as the producer +/// takes them: ` ` per line. +fn clean_record() -> String { + "noop 2.4\ncheck 2.7\nhook 3.0\npassthrough 2.2\nposttool 2.3\nwired 8.4\n".to_owned() +} + +/// A scratch repository carrying the fixture config, the COMMITTED module, and +/// `readme` as its README. +fn repo(name: &str, readme: Option<&str>) -> PathBuf { + let dir = scratch(&format!("perf-assert-{name}")); + write(&dir, "batten.toml", &config()); + write(&dir, "subject.bin", SUBJECT); + if let Some(body) = readme { + write(&dir, "README.md", body); + } + let module = common::at_root("policy/perf-assert.rego"); + std::fs::create_dir_all(dir.join("policy")).expect("scratch policy dir"); + std::fs::copy(module, dir.join("policy/perf-assert.rego")).expect("install committed module"); + git_in(&dir, &["init", "-q", "-b", "main", "."]); + dir +} + +/// Run the producer, handing it the records on stdin — the real writer, so the +/// key under test is the one the engine will look for. +fn record(dir: &Path, records: &str) { + let outcome = run_with_stdin(dir, &["record", "tool", "perf-p95"], records); + assert!( + outcome.status.success(), + "the producer must accept a well-formed record: {}", + common::stderr(&outcome) + ); +} + +fn findings(dir: &Path) -> String { + let mut command = batten(); + command.current_dir(dir).arg("check"); + let outcome = command.output().expect("run batten check"); + + // A NON-VERDICT EXIT IS NOT AN EMPTY ANSWER, and reading it as one is how this + // file went green over a dead gate (CLOUD-1321). `check` exits 0 clean and 2 + // on a policy verdict; every other code is a statement about the INVOCATION — + // a config that will not load exits 1 and says why on stderr, which this + // helper used to discard while returning an empty stdout. Every case asserting + // a finding then failed with a blank message, and the one asserting SILENCE + // passed, so the suite reported the dead module as a partially working one. + // + // Fails by: dropping this assertion and adding a `[[verdict]]` row the fixture + // bundle raises nowhere, which is precisely what a rebase brought in. + let code = outcome.status.code(); + assert!( + matches!(code, Some(0 | 2)), + "`batten check` exited {code:?} rather than deciding: the fixture config \ + did not load, so an empty answer here is a broken gate rather than a \ + clean tree.\nstderr: {}", + stderr(&outcome) + ); + stdout(&outcome) +} + +#[test] +fn a_measurement_inside_every_budget_is_silent() { + let dir = repo("clean", Some(&agreeing_readme())); + record(&dir, &clean_record()); + let answer = findings(&dir); + assert!( + answer.trim().is_empty(), + "a clean measurement against an agreeing README decides nothing:\n{answer}" + ); +} + +#[test] +fn an_over_budget_path_is_refused() { + let dir = repo("over", Some(&agreeing_readme())); + record( + &dir, + "noop 150\ncheck 2.7\nhook 3.0\npassthrough 2.2\nposttool 2.3\nwired 8.4\n", + ); + let answer = findings(&dir); + assert!( + answer.contains("perf-over-budget"), + "a p95 over its ceiling is a finding:\n{answer}" + ); +} + +#[test] +fn a_budgeted_path_absent_from_a_present_record_is_refused() { + // PARTIAL COVERAGE IS NOT A SMALLER PASS. A run that measured five of six and + // reported green over the five is the false green this repository keeps + // re-meeting. + let dir = repo("partial", Some(&agreeing_readme())); + record( + &dir, + "noop 2.4\ncheck 2.7\nhook 3.0\npassthrough 2.2\nposttool 2.3\n", + ); + let answer = findings(&dir); + assert!( + answer.contains("perf-record-incomplete"), + "a budgeted path missing from a present record is a finding:\n{answer}" + ); +} + +#[test] +fn no_record_at_all_is_silent() { + // ABSENT IS NOT INCOMPLETE, and this is the case that keeps every checkout + // that has never run `perf` from reddening. + let dir = repo("absent", Some(&agreeing_readme())); + let answer = findings(&dir); + assert!( + answer.trim().is_empty(), + "a tree with no record decides nothing about the measurement:\n{answer}" + ); +} + +#[test] +fn a_record_does_not_survive_its_subject() { + // THE KEYING, AND IT IS WHY THIS FAMILY WAS CHOSEN. The record is written over + // one binary and the subject is then rebuilt; the key no longer resolves, so + // the module abstains rather than answering from a measurement of other bytes. + // A file of records on stdin could not state this. + let dir = repo("rebuilt", Some(&agreeing_readme())); + record( + &dir, + "noop 150\ncheck 2.7\nhook 3.0\npassthrough 2.2\nposttool 2.3\nwired 8.4\n", + ); + write(&dir, "subject.bin", "a rebuilt binary\n"); + let answer = findings(&dir); + assert!( + !answer.contains("perf-over-budget"), + "a record taken over bytes that have since changed must not answer:\n{answer}" + ); +} + +#[test] +fn a_readme_publishing_a_different_budget_is_refused() { + let dir = repo( + "readme-disagrees", + Some(&agreeing_readme().replace( + "| `noop` | process start | 2.1 ms | 2.4 ms | ≤ 100 ms |", + "| `noop` | process start | 2.1 ms | 2.4 ms | ≤ 50 ms |", + )), + ); + record(&dir, &clean_record()); + let answer = findings(&dir); + assert!( + answer.contains("perf-budget-unpublished"), + "the published budget and the enforced one must agree:\n{answer}" + ); +} + +#[test] +fn a_readme_with_no_row_for_a_budgeted_path_is_refused() { + let dir = repo( + "readme-short", + Some(&agreeing_readme().replace( + "| `wired` | as settings.json invokes it | 8.0 ms | 8.4 ms | ≤ 100 ms |\n", + "", + )), + ); + record(&dir, &clean_record()); + let answer = findings(&dir); + assert!( + answer.contains("perf-budget-unpublished"), + "a budgeted path README does not publish is a disagreement:\n{answer}" + ); +} + +#[test] +fn an_unreadable_readme_is_reported() { + // THE `missing` CLAUSE, over the compiled binary and never with `with input + // as` — the whole question is whether the ENGINE routes an unacquirable + // declared path into `input.tree.missing`, and a fabricated input would answer + // it by construction. + let dir = repo("no-readme", None); + record(&dir, &clean_record()); + let answer = findings(&dir); + assert!( + answer.contains("perf-budget-unreadable"), + "a declared source that could not be read is reported, never assumed clean:\n{answer}" + ); +} + +#[test] +fn the_committed_readme_publishes_the_budgets_this_module_enforces() { + // THE REAL PER-COMMIT GATE, and the case `tests/perf-assert.bats` ran on every + // commit. It is the one assertion here whose subject is this repository rather + // than a fixture: the committed README, the committed module, and no record at + // all — so the measurement half abstains and only the published column is + // judged. + let committed = + std::fs::read_to_string(common::at_root("README.md")).expect("README.md is committed"); + let dir = repo("committed-readme", Some(&committed)); + let answer = findings(&dir); + assert!( + !answer.contains("perf-budget-unpublished"), + "README's Performance table must publish the budget `policy/perf-assert.rego` \ + enforces — move both together:\n{answer}" + ); +} diff --git a/crates/batten/tests/it/perf_pair.rs b/crates/batten/tests/it/perf_pair.rs index 4642f4612..a0d64bbcc 100644 --- a/crates/batten/tests/it/perf_pair.rs +++ b/crates/batten/tests/it/perf_pair.rs @@ -300,26 +300,36 @@ fn the_keyed_base_directory_survives_the_per_run_wipe() { /// on the other would have balanced out. #[test] fn every_path_perf_assert_budgets_is_paired() { - let budgets = std::fs::read_to_string(common::at_root("mise-tasks/perf-assert.sh")) + // THE TABLE MOVED WITH THE GATE (CLOUD-1321). `mise-tasks/perf-assert.sh` is + // retired onto `policy/perf-assert.rego`, so the budgets are a Rego object + // rather than a single-quoted shell block. The OBLIGATION is unchanged and is + // why this case survives the retirement rather than dying with the program: + // every path the gate budgets must be a path the pair actually measures, or + // the budget is enforced over a number nothing produces. + let budgets = std::fs::read_to_string(common::at_root("policy/perf-assert.rego")) .expect("the budget table is where the gate says it is"); - // READ THE BLOCK, NOT THE LINES, and both edges are the retired case's own - // measured lesson arriving intact. The first entry shares its line with the - // assignment (`BUDGETS='noop 100`) and the last carries the closing quote - // (`wired 100'`), so a line-oriented read silently loses one at each end — - // which is exactly what happened here on the first run of this port. + // READ THE BLOCK, NOT THE LINES, which is the retired case's own measured + // lesson and still applies for a different reason: `budgets := {` shares its + // line with the opening brace and the last entry is followed by `}` on its + // own line, so anchoring on the braces is what keeps an entry from being lost + // at either edge. let block = budgets - .split_once("BUDGETS='") - .and_then(|(_, rest)| rest.split_once('\'')) + .split_once("budgets := {") + .and_then(|(_, rest)| rest.split_once('}')) .map(|(block, _)| block) - .expect("the budget table is a single-quoted block"); + .expect("the budget table is a braced object"); let budgeted: Vec = block .lines() .filter_map(|line| { - let mut fields = line.split_whitespace(); - let name = fields.next()?; - let budget = fields.next()?; - (fields.next().is_none() + // `"noop": 100,` — the name is the quoted key and the value is the + // ceiling. A line carrying anything else (a comment, a blank) yields + // nothing rather than a bogus path. + let (name, budget) = line.trim().split_once(':')?; + let name = name.trim().strip_prefix('"')?.strip_suffix('"')?; + let budget = budget.trim().trim_end_matches(','); + (!name.is_empty() && name.chars().all(|c| c.is_ascii_lowercase()) + && !budget.is_empty() && budget.chars().all(|c| c.is_ascii_digit())) .then(|| name.to_owned()) }) diff --git a/crates/batten/tests/it/rule_cost_rung.rs b/crates/batten/tests/it/rule_cost_rung.rs new file mode 100644 index 000000000..42a9ae3f3 --- /dev/null +++ b/crates/batten/tests/it/rule_cost_rung.rs @@ -0,0 +1,144 @@ +//! The per-rule cost reading is on the `-vv` rung and the answer channel does not +//! carry it (CLOUD-1321). +//! +//! # Why this is a separate case from `rule_cost_census.rs` +//! +//! That file asserts the census is CORRECT — one row per rule, counts tracking +//! what was opened, cleared per run — through `run_static`, in process. It never +//! runs the binary, so nothing in it can tell which verbosity rung the reading is +//! rendered at, or whether rendering it disturbed stdout. Those are the two +//! properties a CONSUMER depends on, and they were unheld: CLOUD-1217 landed the +//! instrument and argued the rung at `lib.rs:9872-9896`, and the argument was +//! prose with no mechanism under it, which non-negotiable rule 2 refuses. +//! +//! # The rung is the load-bearing assertion, not the presence +//! +//! `-vv` carrying `rule cost:` is the easy half and a build that emitted it at +//! every rung would satisfy it. The case that decides something is **`-v` staying +//! silent**: it pins the boundary, so a later promotion of the reading to `-v` is +//! a red case here rather than ~84 lines of non-byte-stable stderr appearing +//! under every `-v` consumer without anyone choosing it. CLOUD-1321 proposed +//! exactly that promotion, on the premise that the instrument did not yet exist; +//! it did, and this is what makes the rung a decision somebody has to reverse +//! deliberately. +//! +//! # And stdout is compared byte for byte +//! +//! A duration is not byte-stable, so house-style §6 puts it on stderr or nowhere. +//! Asserting that `check` and `check -vv` agree on stdout EXACTLY is what proves +//! the reading never leaked into the answer, and it is the assertion that would +//! catch the obvious wrong fix — rendering the census through the same writer the +//! findings use. + +// Panicking on setup failure is the idiomatic way for a test to fail loudly. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use crate::common; + +use common::Fixture; + +/// The marker `report_rule_costs` renders each row with. +const MARKER: &str = "rule cost:"; + +/// A one-rule fixture that finds nothing. +/// +/// **A CLEAN tree on purpose.** The subject here is the census, which is emitted +/// for every rule whether or not it reported — so a fixture with a finding would +/// put a refusal on stdout and make the byte-for-byte comparison below a +/// comparison of two refusals instead of two clean answers. +fn fixture(name: &str) -> std::path::PathBuf { + Fixture::new(name) + .config( + "version = 1\n\n\ + [[rule]]\n\ + id = \"reads-the-md\"\n\ + kind = \"forbid\"\n\ + scope = \"tree\"\n\ + glob = \"*.md\"\n\ + pattern = \"a-literal-no-fixture-carries\"\n\ + severity = \"deny\"\n", + ) + .file("README.md", "# a fixture\n\nnothing this rule looks for.\n") + .build() +} + +#[test] +fn the_cost_reading_is_on_the_debug_rung_and_never_below_it() { + let root = fixture("cost-rung"); + + let quiet = common::run(&root, &["check"]); + let verbose = common::run(&root, &["check", "-v"]); + let debug = common::run(&root, &["check", "-vv"]); + + // Every arm is the same clean answer, or the comparison below is between two + // different questions. + for (label, output) in [ + ("check", &quiet), + ("check -v", &verbose), + ("check -vv", &debug), + ] { + assert_eq!( + output.status.code(), + Some(0), + "{label} must be clean over this fixture, or the arms are not comparable: {}", + common::stderr(output) + ); + } + + // THE RUNG. Silent at the default and at `-v`; present at `-vv`. + // + // Fails by: moving the `report_rule_costs` call in `lib.rs` from the + // `Verbosity::Debug` arm to the `Verbose` one, which is precisely the change + // CLOUD-1321 asked for and this case exists to make deliberate. + assert!( + !common::stderr(&quiet).contains(MARKER), + "the default rung carries no cost reading: {}", + common::stderr(&quiet) + ); + assert!( + !common::stderr(&verbose).contains(MARKER), + "`-v` carries no cost reading — promoting it here widens every `-v` \ + consumer's stderr by a row per rule, none of it byte-stable: {}", + common::stderr(&verbose) + ); + assert!( + common::stderr(&debug).contains(MARKER), + "`-vv` is the rung the reading is rendered at, and it is missing: {}", + common::stderr(&debug) + ); +} + +#[test] +fn reading_the_cost_leaves_the_answer_channel_byte_identical() { + let root = fixture("cost-answer-channel"); + + let quiet = common::run(&root, &["check"]); + let debug = common::run(&root, &["check", "-vv"]); + + // THE §6 PROPERTY. A duration is not byte-stable, so it belongs on stderr or + // nowhere — and the way that goes wrong is rendering the census through the + // writer the findings use, which this compares exactly rather than by + // substring. + // + // Fails by: rendering `report_rule_costs` to stdout. + assert_eq!( + common::stdout(&quiet), + common::stdout(&debug), + "raising verbosity to the cost rung must not move a byte of the answer" + ); + assert_eq!( + quiet.status.code(), + debug.status.code(), + "nor the exit code" + ); + + // ANTI-VACUITY. Both stdouts being empty would satisfy the equality above + // however the census behaved, so assert the debug arm actually took the + // branch this case is about. + assert!( + common::stderr(&debug).contains(MARKER), + "the `-vv` arm must have rendered the census, or the equality above \ + compares two runs that never reached it: {}", + common::stderr(&debug) + ); +} diff --git a/crates/batten/tests/it/shell_retirement.rs b/crates/batten/tests/it/shell_retirement.rs index d76167894..64be63042 100644 --- a/crates/batten/tests/it/shell_retirement.rs +++ b/crates/batten/tests/it/shell_retirement.rs @@ -33,7 +33,7 @@ use batten::rules::{self, Rule}; /// struct-literalled: `Rule` carries `deny_unknown_fields`, so this goes through /// the same column census a consumer's config does and a row the loader would /// refuse cannot be smuggled in by hand. -fn row() -> Rule { +pub(crate) fn row() -> Rule { serde_json::from_value(serde_json::json!({ "id": "shell-retirement", "kind": "policy", @@ -64,7 +64,7 @@ fn row() -> Rule { /// `origin/main` is a real remote-tracking ref rather than a local branch, /// because that is the name the committed row declares and a fixture that /// resolved a different one would be testing a different question. -fn repo(name: &str, base: &[(&str, &str)], head: &Head<'_>) -> PathBuf { +pub(crate) fn repo(name: &str, base: &[(&str, &str)], head: &Head<'_>) -> PathBuf { let root = common::scratch(&format!("shell-retirement-{name}")); common::git_in(&root, &["init", "--initial-branch=main"]); write_all(&root, base); @@ -88,9 +88,9 @@ fn repo(name: &str, base: &[(&str, &str)], head: &Head<'_>) -> PathBuf { } /// What the working tree does to the base: files written, files removed. -struct Head<'a> { - written: &'a [(&'a str, &'a str)], - removed: &'a [&'a str], +pub(crate) struct Head<'a> { + pub(crate) written: &'a [(&'a str, &'a str)], + pub(crate) removed: &'a [&'a str], } fn write_all(root: &Path, files: &[(&str, &str)]) { @@ -103,7 +103,7 @@ fn write_all(root: &Path, files: &[(&str, &str)]) { } } -fn install_module(root: &Path) { +pub(crate) fn install_module(root: &Path) { let source = common::at_root("policy/shell-retirement.rego") .canonicalize() .expect("the committed module is where the row says it is"); @@ -114,7 +114,7 @@ fn install_module(root: &Path) { /// The vocabulary the installed module needs, read off the module itself /// (CLOUD-1050). Derived rather than listed: this fixture copies the COMMITTED /// module in so it cannot drift, and a hand-written table beside it would. -fn scan(root: &Path) -> rules::Scan { +pub(crate) fn scan(root: &Path) -> rules::Scan { let verdicts = common::verdicts_in(root); // THE COMMITTED PATTERN TABLE, and it stopped being optional the moment the // module started resolving `data.batten.patterns[…]` (CLOUD-1219). An empty diff --git a/crates/batten/tests/it/shell_retirement_cost.rs b/crates/batten/tests/it/shell_retirement_cost.rs new file mode 100644 index 000000000..42ba17277 --- /dev/null +++ b/crates/batten/tests/it/shell_retirement_cost.rs @@ -0,0 +1,242 @@ +//! `policy/shell-retirement.rego`'s cost is flat in the deleted-path count +//! (CLOUD-1321). +//! +//! **This is a wall-clock assertion, deliberately, and `.claude/rules/rust.md`'s +//! standing rule is why that needs saying.** That rule forbids a clock *where a +//! counter would answer* — "assert it with a counter and a repeat-run comparison, +//! never with wall clock: a timing assertion discriminates nothing here". Read +//! the clause, not the slogan. No counter answers this question: `RuleCost`'s +//! `files_read` and `bytes_read` are identical across all three arms below, +//! because the corpus is opened once per rule however many paths the delta +//! deletes, and regorus exposes no evaluation-step counter — its engine offers a +//! coverage report (which lines ran) and nothing that counts how often one ran. +//! The term being measured is precisely *work repeated per deleted path*, and the +//! clock is the only instrument that sees it. That is CLOUD-1321's own premise. +//! +//! **What keeps it from being a coin flip is the margin, not a tolerance band.** +//! Measured on this fixture, unflattened: 0.39s / 27.5s / 81.3s for zero, two and +//! six deleted paths — 210x the floor, reproducing the ~15s-per-path term +//! CLOUD-1321 measured on the #793 branch (0.43s / 29.6s / 96.2s). Flattened: +//! 0.36s / 0.85s / 0.91s here, and 0.68s / 1.99s / 2.10s on the Windows CI +//! runner. Both assertions below sit an order of magnitude clear of the +//! unflattened reading, so noise would have to dwarf the signal to flip either. A +//! percentage-band timing assertion would be the thing rust.md refuses; these are +//! step-change detectors. +//! +//! **The Windows reading is why `RATIO` is 8 rather than 3, and it is recorded +//! rather than tuned away.** The two platforms agree on the term this case names +//! — four more deletions cost 0.06s here and 0.12s there, against first-two steps +//! of 0.49s and 1.31s — and disagree on the ratio of the index build to the +//! floor, which is a machine property and not the module's. A bound that a green +//! tree fails on a slower box is measuring the box. +//! +//! Three further guards: the floor case fails LOUDLY if the fixture corpus ever +//! stops being large enough for the term to exist (an anti-vacuity term — a +//! shrunken corpus would otherwise make the ratio pass over nothing), each arm is +//! the MINIMUM of three runs (a latency floor is a minimum; noise only adds), and +//! every arm asserts a clean verdict first, so the case can only ever be +//! measuring evaluation and never a finding. +//! +//! `rules::rule_costs()` is process-global and cleared per `run`, so the arms +//! live in one `#[test] fn`, read back between runs. Under nextest — which is how +//! `mise run test` invokes this — that function owns its process. Under a bare +//! `cargo test` a sibling could interleave, which is the hazard `mise.toml` +//! documents for exactly this reason. + +// Panicking on setup failure is the idiomatic way for a test to fail loudly. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use std::fmt::Write as _; +use std::time::Duration; + +use batten::rules; + +use crate::shell_retirement::{Head, install_module, repo, scan}; + +/// How many times each arm is run. The reading is the MINIMUM across them. +const RUNS: usize = 3; + +/// The absolute bound the six-deletion arm must stay inside, against the +/// zero-deletion floor. +/// +/// **This ratio is over two different constants, which is why it is loose and +/// why 3 was wrong.** The floor is one scan of the corpus with no index built at +/// all — `arm_pairs`' first conjunct is `count(delta.deleted) > 0` — and every +/// deleting arm is that scan PLUS the one-off index build. Those two are +/// different work, so their ratio is a property of the machine rather than of the +/// module: measured 2.5x on this container (0.36s / 0.91s) and **3.1x on the +/// Windows CI runner** (0.68s / 2.10s), where the same flattened module is +/// correct. A `RATIO` of 3 therefore failed a green tree on a slower box, which +/// is the percentage-band assertion `.claude/rules/rust.md` refuses wearing a +/// step-change detector's clothes. +/// +/// 8 is the step-change line: ~2.6x above the worst passing reading either +/// platform produced, and ~26x below the 210x the unflattened module reads. The +/// linearity term below is what actually names the defect and it is unmoved; +/// this is the coarse bound beside it, and it only has to refuse a shape nothing +/// between those two numbers can produce. +const RATIO: u32 = 8; + +/// Below this, the fixture corpus is too small for the term to be measurable at +/// all and the ratio assertions would pass over nothing. +const MEASURABLE: Duration = Duration::from_millis(20); + +/// A governed shell program, which is what `governed_when_deleted` classifies a +/// `mise-tasks/*.sh` path as. +const GATE: &str = "#!/usr/bin/env bash\n#MISE description=\"a gate\"\necho hi\n"; + +/// The corpus `arms_for` walks: files under `crates/batten/tests/`, which is the +/// prefix the module filters `input.tree.lines` to. +/// +/// **Sized so one scan is measurable and a green run is still cheap.** The +/// production corpus is ~152k lines (`batten.toml`'s `line_sources` spans +/// `mise-tasks/*.sh`, `crates/batten/tests/**/*.rs` and `tests/**/*.bats`); this +/// is roughly a quarter of it. The unflattened module walked all of it once per +/// deleted path, so the six-deletion arm walked it six times. +const CORPUS_FILES: usize = 40; +const CORPUS_LINES: usize = 1_000; + +/// One fully-mapped ledger arm per retired path, so every arm below is a clean +/// verdict rather than a refusal being timed. +fn ledger(index: usize) -> String { + format!( + "// carried: mise-tasks/gone-{index}.sh policy/gone-{index}.rego \ + crates/batten/tests/gone_{index}.rs\n" + ) +} + +/// The base tree: the corpus, plus `count` governed programs and the ledger rows +/// that map them. +fn base(count: usize) -> Vec<(String, String)> { + let mut files: Vec<(String, String)> = Vec::new(); + for file in 0..CORPUS_FILES { + let mut body = String::with_capacity(CORPUS_LINES * 24); + for line in 0..CORPUS_LINES { + // Ordinary source lines. None carries an arm marker, so every one of + // them is a line the scan must look at and reject — which is the work + // being measured. + let _ = writeln!(body, "// corpus file {file} line {line}"); + } + files.push((format!("crates/batten/tests/it/corpus_{file}.rs"), body)); + } + // The ledger the retirements are mapped by, in one file, as the real one is. + let mut rows = String::new(); + for index in 0..count { + rows.push_str(&ledger(index)); + } + files.push(("crates/batten/tests/it/ledger.rs".to_owned(), rows)); + for index in 0..count { + files.push((format!("mise-tasks/gone-{index}.sh"), GATE.to_owned())); + // The successors the ledger row names have to exist, or the arm is + // refused for a reason that has nothing to do with cost. + files.push((format!("policy/gone-{index}.rego"), String::new())); + files.push(( + format!("crates/batten/tests/gone_{index}.rs"), + String::new(), + )); + } + files +} + +/// What one arm costs: `count` governed paths deleted at head, everything else +/// unchanged. No EDITED governed file in any arm — CLOUD-1321's §2 protocol, and +/// the reason the `base_set` hoist that shipped alongside is not claimed here. +fn arm(name: &str, count: usize) -> Duration { + let owned = base(count); + let base_files: Vec<(&str, &str)> = owned + .iter() + .map(|(path, body)| (path.as_str(), body.as_str())) + .collect(); + let removed: Vec = (0..count) + .map(|index| format!("mise-tasks/gone-{index}.sh")) + .collect(); + let removed_refs: Vec<&str> = removed.iter().map(String::as_str).collect(); + + let root = repo( + name, + &base_files, + &Head { + written: &[], + removed: &removed_refs, + }, + ); + install_module(&root); + + let mut best = Duration::MAX; + for _ in 0..RUNS { + let scanned = scan(&root); + assert!( + scanned.findings.is_empty(), + "{name}: every arm must be a CLEAN verdict, or the reading is timing a \ + refusal rather than the scan: {:?}", + scanned + .findings + .iter() + .map(|finding| finding.rule.as_str()) + .collect::>() + ); + let costs = rules::rule_costs(); + let cost = costs + .iter() + .find(|cost| cost.rule == "shell-retirement") + .expect("the census carries the row that just ran"); + best = best.min(cost.elapsed); + } + best +} + +/// The §2 table, as a case: four more deletions cost less than the first two, +/// and the six-deletion arm reads within `RATIO` of the zero-deletion floor. +/// +/// Shown able to fail per CLOUD-418 by reverting `arm_pairs`/`arm_rows` in +/// `policy/shell-retirement.rego` to the `arms_for(path) := rows if { … }` +/// function this replaced, and watching both terms go red at ~30x and ~210x. +#[test] +fn deleting_six_governed_paths_costs_a_flat_multiple_of_the_floor() { + let floor = arm("cost-zero", 0); + let two = arm("cost-two", 2); + let six = arm("cost-six", 6); + + // THE ANTI-VACUITY TERM. If the corpus ever stops being big enough for one + // scan to be measurable, the ratios below would pass over nothing at all — + // so this shouts rather than going quietly green. + assert!( + floor >= MEASURABLE, + "the fixture corpus no longer makes this term measurable ({floor:?} < \ + {MEASURABLE:?}), so the ratio assertions below would pass over nothing: \ + restore CORPUS_FILES x CORPUS_LINES" + ); + + let table = format!("0 deletions {floor:?}, 2 deletions {two:?}, 6 deletions {six:?}"); + + // THE LINEARITY TERM, and it is the one that names the defect. Going from two + // deletions to six adds four more paths; going from zero to two pays the + // one-off index build plus two. If the ledger is scanned per path then the + // four-path step is roughly twice the two-path one and this fails; if the + // index is built once then the four-path step is nearly free. + // + // Measured on this fixture: unflattened, 0.39s / 27.5s / 81.3s — the step is + // 53.8s against 27.1s, so it fails by 2x. Flattened, 0.36s / 0.85s / 0.91s — + // the step is 0.06s against 0.49s, so it passes by 8x. An order of magnitude + // either side of the line. + let first_step = two.saturating_sub(floor); + let second_step = six.saturating_sub(two); + assert!( + second_step <= first_step, + "the ledger scan is linear in the deleted-path count again — {table}; four \ + more deletions cost {second_step:?} where the first two cost {first_step:?}, \ + so `arms_for` is scanning `input.tree.lines` per path instead of looking \ + its answer up in `arm_rows`" + ); + + // AND AN ABSOLUTE BOUND, because a linearity test alone would pass over a term + // that grew quadratically and then flattened, or over one whose constant had + // exploded. `RATIO` is deliberately loose, and its doc comment says why: the + // floor builds no index and every deleting arm does, so the ratio between them + // is a machine property — 2.5x here, 3.1x on the Windows runner — against 210x + // unflattened. Nothing between 8x and 210x is a shape this module can produce. + assert!( + six <= floor * RATIO, + "six deletions cost more than {RATIO}x the zero-deletion floor — {table}" + ); +} diff --git a/mise-tasks/perf-assert.sh b/mise-tasks/perf-assert.sh deleted file mode 100755 index 29591bf86..000000000 --- a/mise-tasks/perf-assert.sh +++ /dev/null @@ -1,226 +0,0 @@ -#!/usr/bin/env bash -#MISE description="Gate: every measured invocation path is inside its latency budget, and README publishes the budget this gate enforces (reads `perf` records on stdin)" -# -# CLOUD-207. The other half of `mise run perf`, and the half that makes the -# published number mean anything: a latency figure with no assertion behind it -# decays in silence, because nothing about a slower binary announces itself. -# Nobody notices 8ms becoming 40ms; they notice, months later, that the hook got -# uninstalled. -# -# A PURE FUNCTION OF STDIN, the `graph-check`/`claim-check` interface: agents -# fetch, gates decide. The measurement needs a release build, a quiet machine and -# a couple of minutes; this needs none of them, which is the whole point of the -# split. `tests/perf-assert.bats` therefore runs in the hk gate on every commit -# — the DECISION is defended per-commit at zero cost, while the MEASUREMENT runs -# on a schedule (.github/workflows/perf.yml) where a shared runner's noise can -# be absorbed rather than paid for on the landing path. -# -# THE BUDGET IS AN ABSOLUTE CEILING, NOT A RATCHET, and that is deliberate. -# 100ms is the Command Line Interface Guidelines' floor for a response that -# reads as instant, and it is the number CLOUD-207 names. A tight band around -# the measured value — "p95 must stay within 20% of yesterday's" — is the -# obvious alternative and is wrong here twice over: a shared runner's p95 varies -# by more than that between two runs of identical bytes, so the gate would fire -# on noise, and the issue's own flakiness posture says a shared-runner p95 needs -# a tolerance band rather than a point equality. The ceiling being ~20-30x the -# measured value IS that band. "Did this commit make it slower than the trunk" -# is a different question with a different answer shape — a series and a -# comparison against the merge base — and it is CLOUD-172's, not this gate's. -# -# WHY `check` IS NOT BUDGETED. CLOUD-207 scopes the assertion to the no-op and -# hook paths, and the reason survives inspection: those two are bounded by what -# batten itself costs, while `check` is bounded by the repository it is pointed -# at — a tree walk over a large consumer repo is legitimately slower and no -# ceiling here could tell that apart from a regression. It is measured and -# published because the number is worth knowing; it is not gated because this -# gate could not decide it honestly. A gate that cannot decide its object does -# not get to guess (non-negotiable rule 3). -# -# THE README CLAUSE. Publishing a budget in one file and enforcing it in another -# is two authorities for one number, and the one that drifts is always the -# published one — it has no mechanism. So this also reads README's budget column -# and fails when it disagrees with the data below, the same shape `hk-version` -# uses to hold mise.toml and hk.pkl in agreement. It judges the BUDGET, never the -# measured figures: those are a snapshot of one machine at one commit and are -# expected to be stale between perf runs, so gating them would fail the repo for -# the passage of time. -# -# Exit 0 pass / 1 a path over budget or a README disagreement / 2 could not look. -# `2` is the `lock-complete` and `timeout-check` doctrine — "the gate could not -# read what it was asked to judge" — and it is distinct from a violation on -# purpose: a gate that reports green over input it failed to parse is the failure -# that gets a gate switched off. -# A gate listed in $MUTANT_GATES with no row here fails `mise run mutant`. -#MUTANT over-budget-passes|s/^\[\[ "\$fail" = 0 \]\]/true/|over its budget is a violation, and is named - -set -euo pipefail - -# The budgets, in milliseconds, written once here as data — the same placement -# `run-shape-guard`'s verdict-bearing list and `batten.toml`'s own `protected` -# set use. A path named here must appear in the records; a path in the records -# that is not named here is measured and not gated, which is `check`. -# -# One field per line: . -# -# `wired` is the one an agent actually waits on: the launcher plus the binary, -# derived by `perf` from `.claude/settings.json` rather than hardcoded. It is -# budgeted at the same clig floor, because the floor is about what a human -# perceives and the wiring is what they experience. `hook` stays budgeted -# alongside it so the launcher's own share stays attributable — a regression in -# one and not the other says where to look. -# -# `posttool` is budgeted at the same floor (CLOUD-919). Naming it here is what -# ARMS the presence gate below: from now on a `perf` run that does not emit -# `path=posttool` is exit 2 rather than a green run with one fewer measurement, -# which is what makes the arm's existence non-optional rather than a courtesy. -BUDGETS='noop 100 -passthrough 100 -posttool 100 -hook 100 -wired 100' - -# Where the published budget is asserted to match. An argument so the bats suite -# can point the clause at a fixture; the real file is the default. -README="${1:-README.md}" - -fail=0 -# Pointer-only per non-negotiable rule 4: the path id, the measured p95 and the -# budget it broke. Never the raw hyperfine output, never a command line. -report() { - echo " $1" >&2 - fail=1 -} - -# `2`, not `1`: an unreadable stdin is "could not look", and the caller that -# redirected nothing into this gate needs to hear that rather than "green". -records=$(cat) -if [[ -z "${records//[[:space:]]/}" ]]; then - echo "::error:: perf-assert: stdin is empty — pipe \`mise run perf\` records in (redirect to a file, then read it back; a pipeline would hand this gate's exit status to its last stage)." >&2 - exit 2 -fi - -# One record per line: `path= p50= p95= mean= runs=`. Parsed with -# a literal-pattern awk rather than a `-v` regex — a pattern reaching awk through -# `-v` goes through assignment escape processing first, which `mise run -# awk-regex-check` refuses for being implementation-defined. -# -# A line that does not parse is a malformed record, not an absent one, and the -# difference matters: absent means "this run measured less than it claims", -# malformed means "something other than perf wrote here". Both are exit 2. -parsed=$(awk ' - /^path=/ { - id = ""; p95 = "" - for (i = 1; i <= NF; i++) { - split($i, kv, "=") - if (kv[1] == "path") id = kv[2] - if (kv[1] == "p95") p95 = kv[2] - } - if (id == "" || p95 == "" || p95 + 0 != p95 || p95 == "0") { - print "MALFORMED\t" NR - next - } - print "OK\t" id "\t" p95 - next - } - # Anything that is not a record and not blank is noise in a stream that is - # supposed to carry records alone. - /[^[:space:]]/ { print "MALFORMED\t" NR } -' <<<"$records") - -malformed=$(awk -F'\t' '$1=="MALFORMED"{print $2}' <<<"$parsed") -if [[ -n "$malformed" ]]; then - echo "::error:: perf-assert: stdin carries lines that are not \`perf\` records, so the measurement cannot be judged:" >&2 - while IFS= read -r line; do - [[ -n "$line" ]] || continue - echo " stdin:$line: not a \`path= p50=… p95=… mean=… runs=…\` record" >&2 - done <<<"$malformed" - exit 2 -fi - -# Every budgeted path must be present. A run that measured two of three paths and -# reported green over the two is exactly the partial-coverage false green this -# repo keeps re-meeting, so absence is `could not look`, never a pass. -missing="" -while read -r id _; do - [[ -n "$id" ]] || continue - awk -F'\t' -v want="$id" '$1=="OK" && $2==want {found=1} END{exit !found}' <<<"$parsed" || - missing="$missing $id" -done <<<"$BUDGETS" -if [[ -n "$missing" ]]; then - echo "::error:: perf-assert: the records carry no measurement for a budgeted path, so this run judged less than it claims:" >&2 - for id in $missing; do - echo " $id: budgeted here, absent from stdin — did \`mise run perf\` complete?" >&2 - done - exit 2 -fi - -# The verdict. `awk` for the comparison because the values are fractional -# milliseconds and `[ ]` compares integers only — a p95 of 8.4 against a budget -# of 100 is not a comparison bash can make. -reported=0 -while read -r id budget; do - [[ -n "$id" ]] || continue - p95=$(awk -F'\t' -v want="$id" '$1=="OK" && $2==want {print $3; exit}' <<<"$parsed") - if awk -v measured="$p95" -v limit="$budget" 'BEGIN { exit !(measured > limit) }'; then - if [[ "$reported" = 0 ]]; then - echo "::error:: perf-assert: a measured invocation path is over its latency budget (see README, Performance):" >&2 - reported=1 - fi - report "$id: p95=${p95}ms exceeds the ${budget}ms budget" - fi -done <<<"$BUDGETS" - -# The README clause. The published budget is read out of the Performance table's -# budget column, whose cell is written as `≤ ms` for a gated path and `—` for -# an ungated one, so the two states are distinguishable in the table itself. -if [[ ! -f "$README" ]]; then - echo "::error:: perf-assert: $README not found, so the published budget cannot be held to the enforced one." >&2 - exit 2 -fi - -# The budget column is found by its HEADER, never by a fixed index. A column -# index is a coupling between this gate and the table's shape, so adding a -# "what it does" column to the published table would silently start comparing -# the wrong cell — and comparing the wrong cell is how a gate reports a -# disagreement that is really its own arithmetic. -published=$(awk ' - function trim(s) { gsub(/^[[:space:]]+|[[:space:]]+$/, "", s); return s } - /^\|/ { - line = $0 - gsub(/`/, "", line) - n = split(line, cell, "|") - if (col == 0) { - for (i = 2; i < n; i++) { - if (trim(cell[i]) == "budget") col = i - } - next - } - if (col < n) print trim(cell[2]) "\t" trim(cell[col]) - } -' "$README") - -while read -r id budget; do - [[ -n "$id" ]] || continue - row=$(awk -F'\t' -v want="$id" '$1==want {print $2; exit}' <<<"$published") - if [[ -z "$row" ]]; then - if [[ "$reported" = 0 ]]; then - echo "::error:: perf-assert: the enforced budget and the published one disagree (non-negotiable rule 2 — the rule ships with its mechanism):" >&2 - reported=1 - fi - report "$id: budgeted at ${budget}ms here, but $README's Performance table has no row for it" - continue - fi - # The cell as published, reduced to its number, so the surrounding `≤` and - # unit are presentation and only the value is compared. - value=$(printf '%s' "$row" | tr -cd '0-9.') - if [[ "$value" != "$budget" ]]; then - if [[ "$reported" = 0 ]]; then - echo "::error:: perf-assert: the enforced budget and the published one disagree (non-negotiable rule 2 — the rule ships with its mechanism):" >&2 - reported=1 - fi - report "$id: enforced ${budget}ms, $README publishes '$row'" - fi -done <<<"$BUDGETS" - -[[ "$fail" = 0 ]] || exit 1 -echo "perf-assert: every budgeted path is inside its budget, and $README publishes the budget this gate enforces" diff --git a/mise.toml b/mise.toml index 7a3dea8b0..12bead911 100644 --- a/mise.toml +++ b/mise.toml @@ -475,7 +475,7 @@ CI_FANIN_WORKFLOW = ".github/workflows/ci.yml" # which is a property of the world and belongs on a clock (`lock-complete`). REGORUS_OPA_COMPLIANCE = "1.2.0" REGORUS_OPA_COMPLIANCE_FOR = "0.11" -MUTANT_GATES = "alive,attestation-check,awk-regex-check,bats-invocation,batten-glob-check,board-diff-overlap,board-payloads,board-sweep,branch-age-check,cap-drift,ci-hygiene,ci-lease-precondition,ci-parity,ci-slow-needed,ci-suite-lane,ci-tools-check,claim-before-code,claim-order-is-stated,claim-race-check,claimed-keys,closing-key-check,coderabbit-config-check,commit-hygiene,connector-allow-guard,connector-allow-resolve,container-preflight,darwin-link,deferral-check,denials-outlive-the-turn,digest-major-agreement,doctor,done-check,done-pr-check,duplicate-close-check,evaluator-closure-check,evaluator-io-check,filed-here,finding-sink-check,forge-verdict-required,graph-check,harness-grant,harness-wiring,hk-fix-selection,hook-matcher-check,hook-pin-check,in-progress-drain,install-check,land,land-divergence-assert,land-lock,land-lock-check,landed-check,landing-loop,leased-push,license-table-check,linear-check,lock-complete,macos-link-check,mcp-allow-check,mcp-attach-check,mcp-timeout-budget,merged-pr-keys,mise-action-floor,mise-pin-agreement,module-map-check,msrv-pin-agreement,mutation-declared-case,no-doctests,nonverdict-assert,ntia-check,obligations-bound,perf-assert,pinned-toolchain,pipefail-grep-check,plan-complete,pr-partition-restated,pr-unsubscribed,privileged-lane,prose-only,publish-credential-check,ready-cites-check,ready-guard,ready-lint,reclaim-census,release-assets-check,release-due,release-tag-shape,release-tracking-check,released,remedy-authorship,repetition-without-progress,report-only-check,review-answered,review-dispatched,run-shape,rust-paths-check,sbom,sbom-inventory,serena-mcp,shell-hygiene,shell-retirement,shell-write-advisory,signing-posture,sonar-gate,spec-ref-check,stop-posture,stop-posture-check,suite-bench-check,suite-subject-retirable,task-substitution,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,trunk-based,validator-verdict-clean,verdict-routes-resolve,verified,weakens-declared" +MUTANT_GATES = "alive,attestation-check,awk-regex-check,bats-invocation,batten-glob-check,board-diff-overlap,board-payloads,board-sweep,branch-age-check,cap-drift,ci-hygiene,ci-lease-precondition,ci-parity,ci-slow-needed,ci-suite-lane,ci-tools-check,claim-before-code,claim-order-is-stated,claim-race-check,claimed-keys,closing-key-check,coderabbit-config-check,commit-hygiene,connector-allow-guard,connector-allow-resolve,container-preflight,darwin-link,deferral-check,denials-outlive-the-turn,digest-major-agreement,doctor,done-check,done-pr-check,duplicate-close-check,evaluator-closure-check,evaluator-io-check,filed-here,finding-sink-check,forge-verdict-required,graph-check,harness-grant,harness-wiring,hk-fix-selection,hook-matcher-check,hook-pin-check,hook-skip-local,in-progress-drain,install-check,land,land-divergence-assert,land-lock,land-lock-check,landed-check,landing-loop,leased-push,license-table-check,linear-check,lock-complete,macos-link-check,mcp-allow-check,mcp-attach-check,mcp-timeout-budget,merged-pr-keys,mise-action-floor,mise-pin-agreement,module-map-check,msrv-pin-agreement,mutation-declared-case,no-doctests,nonverdict-assert,ntia-check,obligations-bound,perf-assert,pinned-toolchain,pipefail-grep-check,plan-complete,pr-partition-restated,pr-unsubscribed,privileged-lane,prose-only,publish-credential-check,ready-cites-check,ready-guard,ready-lint,reclaim-census,release-assets-check,release-due,release-tag-shape,release-tracking-check,released,remedy-authorship,repetition-without-progress,report-only-check,review-answered,review-dispatched,run-shape,rust-paths-check,sbom,sbom-inventory,serena-mcp,shell-hygiene,shell-retirement,shell-write-advisory,signing-posture,sonar-gate,spec-ref-check,stop-posture,stop-posture-check,suite-bench-check,suite-subject-retirable,task-substitution,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,trunk-based,validator-verdict-clean,verdict-routes-resolve,verified,weakens-declared" # --- GitHub reachability behind an egress proxy (Claude Code web sandbox etc.) --- # mise resolves every tool's release through GitHub's *API* host, api.github.com. @@ -1751,6 +1751,59 @@ jq -r --slurpfile full "$full" ' ' <"$fast" | record hk-plan ''' +# The perf producer and the perf gate (CLOUD-207, ported off +# `mise-tasks/perf-assert.sh` under CLOUD-1321). +# +# THE SPLIT IS THE SAME ONE `record-verdicts` HAS, and for the same forced reason: +# house style §5 makes `check` `read` and structurally incapable of spawning, so +# hyperfine stays a command on PATH and only the ADJUDICATION moved inside the +# engine. This task is the seam that split makes necessary. +# +# AN INLINE TASK RATHER THAN A `mise-tasks/` PROGRAM, and it is forced: +# `governed_at_head` selects any `mise-tasks/` path carrying a shebang or a +# `#MISE description=`, so a new program there is `shell add refused` — refused at +# `deny`, in the same change that is retiring one of them. +# +# A REDUCTION, NEVER THE REPORT (non-negotiable rule 4). One number per path +# reaches the record; hyperfine's own output stays on the terminal. +[tasks.record-perf] +description = "Effect: reduce `perf` records to a p95 per path and record them where `batten check` reads them (reads `perf` records on stdin)" +run = ''' +records=$(mktemp) +trap 'rm -f "$records"' EXIT +if ! awk '/^path=/ { + id = ""; p95 = "" + for (i = 1; i <= NF; i++) { + split($i, kv, "=") + if (kv[1] == "path") id = kv[2] + if (kv[1] == "p95") p95 = kv[2] + } + if (id != "" && p95 != "") print id " " p95 +}' >"$records"; then + echo "::error:: record-perf: the records on stdin could not be reduced, so nothing is recorded." >&2 + exit 1 +fi +# AN EMPTY REDUCTION FAILS HERE RATHER THAN RECORDING SILENCE. An empty record is +# a PRESENT record carrying no path, which the module reads as "this run judged +# less than it claims" — a true finding, but one whose cause is this task and +# whose pointer would send a reader to the module instead. +if [ ! -s "$records" ]; then + echo "::error:: record-perf: stdin carried no path record — did \`mise run perf\` complete?" >&2 + exit 2 +fi +cargo run --quiet -p batten -- record tool perf-p95 <"$records" +''' + +# The gate the producer above feeds, and the same wrapper shape `lock-complete` +# takes: the task name and the rule id are one object, so the workflow step, this +# task and `batten.toml`'s row cannot drift apart. +# +# `--rule` SELECTS ONE ROW. A bare `batten check` under a name that promises the +# latency gate would report every other tree rule through it. +[tasks.perf-assert] +description = "Gate: every measured invocation path is inside its latency budget, and README publishes the budget this gate enforces" +run = "cargo run --quiet -p batten -- check --rule perf-assert" + # `[tasks.msrv]` IS RETIRED (CLOUD-593), replaced by # `mise-tasks/msrv-pin-agreement.sh`. # diff --git a/policy/harness-wiring.rego b/policy/harness-wiring.rego index 4bb48cd55..48e5adc03 100644 --- a/policy/harness-wiring.rego +++ b/policy/harness-wiring.rego @@ -354,6 +354,35 @@ enforced(pattern) if { enforced(pattern) if { not contains(pattern, "/") merged_read > 0 + + # READ IS NOT THE SAME AS CARRIED, and `merged_read` alone cannot tell them + # apart (CLOUD-1340). It counts surfaces that RESOLVED; a host that writes a + # wiring file with no `hooks` key at all resolves one and carries nothing, so + # every merged row became enforced against a set with nothing in it to match + # and the whole table went stale at once. + # + # Measured 2026-09-02 in this container after a restart: + # `~/.claude/launcher-settings.json` present and hookless, the other three + # merged ids absent, `merged_read` = 1, `merged_commands` = {} -- and + # `harness-wiring` reported 2, one per row of `policy/harness-declared.json`, + # while `stop-hook-git-check.sh` (which those rows license) was demonstrably + # still running, from a surface outside the declared four. + # + # That is could-not-look rendered as a spent licence, which is the exact + # collapse the comment above this predicate says the `merged_read` guard exists + # to prevent -- so this carries that argument one step further rather than + # introducing a new one. A surface set carrying NO commands cannot distinguish + # "this row's subject was retired" from "nothing was looked at", and only the + # first is a finding. The remedy the bare refusal appears to name is worse than + # the defect: dropping the two rows turns a correct licence into a future + # `hook wire duplicate` the moment a host wires those scripts again. + # + # IT DOES NOT WEAKEN THE STALE DIRECTION, which is the test of the change. As + # soon as any merged surface carries a single command every merged row is + # enforced again, and a row matching nothing still fires -- + # `test_a_merged_row_matching_nothing_is_stale` is unmoved. What stops firing is + # only the case where there was nothing to match against at all. + count(merged_commands) > 0 } matches_something(pattern) if { @@ -642,16 +671,45 @@ test_a_merged_row_is_unenforced_where_no_merged_surface_was_read if { not "hook declare stale" in vs } -# NO MERGED ROW IS DECLARED ANY MORE, so a read merged surface spends nothing and -# the committed row is judged on its own surface. The `enforced` merged arm stays -# in the module for the next merged row rather than being deleted with these two: -# removing it would be coverage loss dressed as cleanup, and it is unreachable -# rather than wrong. +# A read merged surface spends nothing on its own: the fixture's table declares no +# merged row, so the committed row is judged on its own surface and nothing else is. +# +# THIS COMMENT SAID "NO MERGED ROW IS DECLARED ANY MORE" AND THAT WAS FALSE +# (CLOUD-1340). `policy/harness-declared.json` declares two, and both are +# basenames, which is what makes them merged rows -- so the `enforced` merged arm +# was described here as unreachable while being the arm every live row went +# through. That is why the too-coarse `merged_read` guard sat unexamined: a reader +# checking whether the arm mattered was told it did not. test_a_read_merged_surface_alone_spends_no_declaration if { vs := verdicts with input as launcher({mediates}) not "hook declare stale" in vs } +# THE GUARD CLOUD-1340 ADDED. A merged surface that RESOLVED but carries no +# command at all is could-not-look, not a spent licence. +# +# Measured in this container: `~/.claude/launcher-settings.json` present and +# hookless, the other three merged ids absent, so `merged_read` was 1 while +# `merged_commands` was empty -- and both live rows fired at once, while the +# `stop-hook-git-check.sh` they license was still running from a surface outside +# the declared four. +test_a_merged_surface_carrying_no_commands_spends_nothing if { + vs := verdicts with input as launcher_with({}, {"stop-hook-git-check.sh": "CLOUD-1"}) + not "hook declare stale" in vs +} + +# AND THE DIRECTION IT MUST NOT WEAKEN, which is the actual test of the change: +# once any merged surface carries a command, every merged row is enforced again and +# one matching nothing still fires. Without this case the guard above is satisfied +# by a module that never reports a stale merged row at all. +test_a_merged_row_matching_nothing_is_stale if { + some v in violation with input as launcher_with( + {mediates}, + {"stop-hook-git-check.sh": "CLOUD-1"}, + ) + v.verdict == "hook declare stale" +} + # ANTI-VACUITY over BOTH classes at once, which the committed-only case cannot # reach: every declared row matches on the surface that owns it, so nothing fires. test_both_surfaces_wired_correctly_is_clean if { diff --git a/policy/hook-skip-local.rego b/policy/hook-skip-local.rego new file mode 100644 index 000000000..51e37a52f --- /dev/null +++ b/policy/hook-skip-local.rego @@ -0,0 +1,177 @@ +#MUTANT-SUITE crates/batten/tests/it/hook_skip_local.rs +#MUTANT skip-assignment-unread|s@^\tsome word in segment.words$@\tsome word in []@|a_local_step_skip_is_refused +#MUTANT ci-carve-unread|s@^\tnot ci_lane$@\tfalse@|the_declared_ci_carve_is_not_judged_here +# Switching a gate off for a local commit is a decision, not a flag (CLOUD-1340). +# +# MEASURED ON THE BRANCH THAT FILED IT, and the incident is the whole reason this +# exists rather than an argument for it. `hooks-wiring-check` refused; the session +# read the refusal as environmental and unfixable, set `HK_SKIP_STEPS` on three +# commits, wrote that false justification into two commit messages, and put a +# four-option menu to a human. `batten wiring reclaim -y` cleared the condition in +# one command. Nothing in this engine fired at any point: the variable is read by +# `hk`, which batten never sees, so the gate that was switched off had no way to +# say so. +# +# THE ASYMMETRY IS THE FINDING. `ci-suite-lane` already governs `HK_SKIP_STEPS` +# where CI sets it -- `input.tree.documents` over the workflow files, refusing the +# `test:bats` carve coming apart -- so the DECLARED use is gated and the ad-hoc +# one was not. A variable a repository deliberately depends on in one place and +# refuses nowhere else is a hole shaped exactly like its own legitimate use, which +# is why this reads the mediated call rather than widening that row. +# +# WHY THIS IS A MODULE RATHER THAN A `shape` ROW. `refusal.rs` is explicit that a +# refusal composed from a consumer `[[rule]]` row carries no declared class and no +# token an admission could bind, so such a row could only ever be reached through +# `bypass_env` -- the password shape CLOUD-1051 retired on the ground that *the +# point of the admission mechanism is that the bare variable stops working*. A +# module raises a declared class, which is what makes the refusal one a reader can +# look up. +# +# AND THE CLASS DECLARES NO OVERRIDE, which is a decision rather than an omission +# and was reversed once while landing. The route drafted here read "the step +# cannot be satisfied at this commit -- it reads a generated file a LATER commit in +# the same sequence writes"; that case is REAL and was met three times on this +# branch, over `bench/suites/RESULTS.md` during a rebase. It is already served, +# though, by `--no-verify` plus the articulation block `commit check` requires +# (CLOUD-1278) -- gated, and leaving a record in the commit where a reviewer reads +# it. `shell edit refused` is the precedent for a class that refuses outright. +# +# So `--no-verify` is the route and this class has none, which also drops a +# `verdict-override-added` weakening the branch would otherwise have had to +# declare. That is the better trade in both directions: one object gets one +# authority, and the more visible mechanism is the one that survives. +# +# WHAT THIS DOES NOT CLOSE, for the same reason stated forwards: `--no-verify` is +# NOT judged here. `commit check` already refuses a commit that wrote a protected +# path with no block claiming it, and it caught this same session's `--no-verify` +# amend by noticing the block had been dropped. This closes the variable, which +# nothing read. +# METADATA +# description: | +# Bound to the mediated-call surface: this module is `scope = "mediated_call"`, +# so it reads `{call, facts}` and NOT the tree document. `ci-suite-lane` is the +# tree-surface half over the same variable and they do not overlap -- one reads +# a workflow file, this reads a command. +# THE BRACKETS ARE NOT STYLE: the schema file carries a hyphen, so the dotted +# form is a parse error reported as `invalid schema reference`. +# schemas: +# - input: schema["policy-call.schema"] +package batten.hook_skip_local + +import rego.v1 + +rules contains "hook-skip-local" + +# The declared carve, which is CI's and is judged by `ci-suite-lane` instead. +# +# NARROW ON PURPOSE AND ANCHORED AT BOTH ENDS. This exempts the one spelling the +# `ci` job actually hands hk, and nothing that merely contains it -- so +# `HK_SKIP_STEPS=test:bats,batten-check` is still refused, which is the shape an +# author reaches for when adding "just one more" to a line they found in a +# workflow. A prefix test here would hand back the whole hole. +ci_lane if { + some segment in input.call.segments + some word in segment.words + word == "HK_SKIP_STEPS=test:bats" +} + +violation contains { + "rule": "hook-skip-local", + "verdict": "hook skip unseen", + "subjects": [{"count": 1}], +} if { + # PER SEGMENT, NOT PER LINE (CLOUD-857). A real agent command is compound most + # of the time, and the preset this sits beside carries the measured instance: + # anchored on the whole command line, `git push --force` denied while + # `cd /tmp && git push --force` was allowed, with a green suite over it. + some segment in input.call.segments + + # THE ASSIGNMENT IS A WORD, WHICH IS WHY THIS READS `words` AND NOT + # `programs`. `hook::is_env_assignment` is what the boundary uses to look + # THROUGH an assignment when resolving the effective program, so `programs` + # reports `git` for `HK_SKIP_STEPS=x git commit` and never the variable. The + # tokens survive in `words` exactly as written, which is the surface that can + # see this at all. + some word in segment.words + regex.match(data.batten.patterns["hook-step-skip-assignment"], word) + + not ci_lane +} + +# The predicate's own tests. The exemption case is the one that matters: a module +# that only proved the deny fires would be satisfied by a build that refuses the +# `ci` job's own line, which is the shape that gets a guard switched off rather +# than satisfied. +# +# EVERY CASE PASSES SEGMENTS AND AT LEAST ONE IS COMPOUND (CLOUD-857): +# `batten policy test` refuses a mediated-call module whose cases all pass a bare +# command. +test_a_local_step_skip_is_refused if { + some _ in violation with input as {"call": {"segments": [{ + "words": ["HK_SKIP_STEPS=hooks-wiring-check", "git", "commit", "-m", "x"], + "raw": "HK_SKIP_STEPS=hooks-wiring-check git commit -m x", + "terminator": null, + }]}} +} + +test_a_step_skip_in_a_compound_command_is_refused if { + some _ in violation with input as {"call": {"segments": [ + {"words": ["git", "add", "-A"], "raw": "git add -A", "terminator": "&&"}, + { + "words": ["HK_SKIP_STEPS=batten-check", "git", "commit", "-m", "x"], + "raw": "HK_SKIP_STEPS=batten-check git commit -m x", + "terminator": null, + }, + ]}} +} + +# THE EXEMPTION, AND IT IS EXACT. `ci.yml` hands hk this precise value; anything +# else is an author's own decision and is judged. +test_the_declared_ci_carve_is_not_judged_here if { + count(violation) == 0 with input as {"call": {"segments": [{ + "words": ["HK_SKIP_STEPS=test:bats", "mise", "run", "ci"], + "raw": "HK_SKIP_STEPS=test:bats mise run ci", + "terminator": null, + }]}} +} + +# A VALUE THAT MERELY CONTAINS THE CARVE IS STILL A DECISION. This is the arm a +# prefix test would lose, and it is the one an author actually reaches for. +test_the_carve_with_a_step_appended_is_refused if { + some _ in violation with input as {"call": {"segments": [{ + "words": ["HK_SKIP_STEPS=test:bats,batten-check", "mise", "run", "ci"], + "raw": "HK_SKIP_STEPS=test:bats,batten-check mise run ci", + "terminator": null, + }]}} +} + +test_an_ordinary_commit_is_allowed if { + count(violation) == 0 with input as {"call": {"segments": [{ + "words": ["git", "commit", "-m", "x"], + "raw": "git commit -m x", + "terminator": null, + }]}} +} + +# ANOTHER VARIABLE IS NOT THIS ONE. The pattern is anchored at its left edge, so a +# name that merely ends in the same letters does not reach it. +test_another_assignment_is_not_judged if { + count(violation) == 0 with input as {"call": {"segments": [{ + "words": ["RUST_LOG=debug", "git", "commit", "-m", "x"], + "raw": "RUST_LOG=debug git commit -m x", + "terminator": null, + }]}} +} + +test_a_quoted_mention_is_not_an_invocation if { + count(violation) == 0 with input as {"call": {"segments": [{ + "words": ["echo", "set HK_SKIP_STEPS=x to skip a step"], + "raw": "echo \"set HK_SKIP_STEPS=x to skip a step\"", + "terminator": null, + }]}} +} + +deny contains message if { + some v in violation + message := v.verdict +} diff --git a/policy/perf-assert.rego b/policy/perf-assert.rego new file mode 100644 index 000000000..65b2a5bce --- /dev/null +++ b/policy/perf-assert.rego @@ -0,0 +1,385 @@ +# Every measured invocation path is inside its latency budget, and README +# publishes the budget this gate enforces (CLOUD-207, retired off +# `mise-tasks/perf-assert.sh` under CLOUD-1321). +# +# THE MEASUREMENT CANNOT HAPPEN INSIDE THE ENGINE, which is why this reads a +# record rather than taking a reading. `check` is declared `read` and structurally +# cannot spawn, so hyperfine stays a command on PATH and something outside has to +# invoke it — the same split `validator-verdict-clean` already has for pkl, and +# the reason `[[rule.tools]]` exists at all. The producer runs in +# `.github/workflows/perf.yml`, where the measurement's release build and quiet +# machine live; this decides. +# +# THE KEY IS THE SAFETY PROPERTY. `input.tree["tool-verdict"]` is keyed by +# (tool, pinned version, input digest), so a record taken over a binary that has +# since been rebuilt lives under a different name and DOES NOT ANSWER — it is +# absent rather than stale. That is what makes reading a measurement taken +# elsewhere honest, and it is a property the predecessor's stdin pipe could not +# have: a file of records carries no statement about what produced it. +# +# THE BUDGET IS AN ABSOLUTE CEILING, NOT A RATCHET, and that is deliberate. 100ms +# is the Command Line Interface Guidelines' floor for a response that reads as +# instant, and it is the number CLOUD-207 names. A tight band around the measured +# value — "p95 must stay within 20% of yesterday's" — is the obvious alternative +# and is wrong twice over: a shared runner's p95 varies by more than that between +# two runs of identical bytes, so the gate would fire on noise. The ceiling being +# ~20-30x the measured value IS the tolerance band. "Did this commit make it +# slower than the trunk" is a different question with a different answer shape, +# and it is CLOUD-172's — `perf-gate`'s — not this one's. +# +# WHY THERE IS A `check` ROW NOW, when the predecessor deliberately had none. +# `perf-assert.sh` argued that `check` is bounded by the repository it is pointed +# at, so no ceiling could tell a large consumer tree apart from a regression, and +# `.claude/rules/rust.md` records the absence as a stated decision. The `perf` +# arm this row gates is not that: it is a ONE-RULE FIXTURE repo, so it measures +# process start plus config load plus trust resolution plus one rule, all of which +# are bounded by what batten costs rather than by a consumer's tree. That is +# budgetable on exactly the reasoning that budgets `noop`. It is NOT the +# deletion-linear term CLOUD-1321 flattened — that is a property of one module +# over a 152k-line corpus, measured in seconds, and its gate is +# `crates/batten/tests/it/shell_retirement_cost.rs`, which discriminates growth +# SHAPE rather than machine speed. Two gates, two subjects, and neither is claimed +# to be the other. +# +# THE README CLAUSE. Publishing a budget in one file and enforcing it in another +# is two authorities for one number, and the one that drifts is always the +# published one — it has no mechanism. So this reads README's budget column and +# refuses when it disagrees with `budgets` below. It judges the BUDGET, never the +# measured figures: those are a snapshot of one machine at one commit and are +# expected to be stale between `perf` runs, so gating them would fail the repo for +# the passage of time. +#MUTANT-SUITE crates/batten/tests/it/perf_assert.rs +#MUTANT over-budget-passes|s@to_number(measured) > budgets[id]@false@|an_over_budget_path_is_refused +#MUTANT readme-budget-disagreement-passes|s@published[id] != budgets[id]@false@|a_readme_publishing_a_different_budget_is_refused +#MUTANT absent-budgeted-path-passes|s@not id in object.keys(judged)@false@|a_budgeted_path_absent_from_a_present_record_is_refused + +# METADATA +# description: | +# Bound to the TREE surface: this row is `scope = "tree"`, so it reads +# `input.tree` and never the mediated call. +# THIS BLOCK IS YAML AND MUST STAY THE LAST COMMENT BLOCK BEFORE `package`. +# schemas: +# - input: schema["policy-input.schema"] +package batten.perf_assert + +import rego.v1 + +rules contains "perf-over-budget" + +rules contains "perf-record-incomplete" + +rules contains "perf-budget-unpublished" + +rules contains "perf-budget-unreadable" + +# The budgets, in milliseconds, written once here as data — the placement the +# predecessor's `BUDGETS` table had, one level over. +# +# A LITERAL IN A CONSUMER MODULE, NOT A `[[pattern]]` ROW. Non-negotiable rule 1 +# scopes to `crates/batten`, and a `policy/*.rego` module IS consumer config, so a +# number is at home here exactly as it was at home in the shell table. +# `.claude/rules/policy-modules.md` refuses a threshold spelled as a pattern for +# the opposite reason — arithmetic is not a concept with one spelling — and this +# is not that. +# +# A path named here must appear in the record; a path in the record that is not +# named here is measured and not gated. +budgets := { + "noop": 100, + "passthrough": 100, + "posttool": 100, + "hook": 100, + "wired": 100, + "check": 100, +} + +# The record ids this module owns. +# +# `input.tree["tool-verdict"]` is built from every `[[rule.tools]]` row in the +# config — the projection flattens across all rules — so a sibling row's record +# reaches this module too. `validator-verdict-clean` records the measured defect: +# `hk-plan`'s seven ` included` lines were read as seven findings by a +# module that did not name its ids. Named here for that reason. +owned := "perf-p95" + +# What the producer recorded, or nothing. +# +# GUARDED on `is_object`: the key is `null` when nobody declared a tool, and +# indexing into `null` is a hard evaluation FAULT in Rego rather than a silent +# miss. +# +# ABSENT IS NOT EMPTY, and the whole family turns on it. A record whose key does +# not resolve — no such tool version, or a binary rebuilt since the measurement — +# is absent from the map entirely, so every clause below abstains rather than +# reporting clean. A checkout that has never run `perf` is silent here, which is +# the honest answer and not a pass. +judged := measurements if { + is_object(input.tree["tool-verdict"]) + measurements := input.tree["tool-verdict"][owned] +} + +# --- the measurement half ---------------------------------------------------- + +# A budgeted path whose measured p95 is over its ceiling. +violation contains { + "rule": "perf-over-budget", + "verdict": "path measure late", + "subjects": [{"count": count(over_budget)}], +} if { + count(over_budget) > 0 +} + +over_budget contains id if { + some id, measured in judged + id in object.keys(budgets) + to_number(measured) > budgets[id] +} + +# A budgeted path the record does not carry. +# +# A run that measured five of six paths and reported green over the five is +# exactly the partial-coverage false green this repository keeps re-meeting, so +# absence within a PRESENT record is a finding. The guard is that `judged` itself +# must resolve: with no record at all there is nothing to be incomplete about. +violation contains { + "rule": "perf-record-incomplete", + "verdict": "path measure partial", + "subjects": [{"count": count(unmeasured)}], +} if { + count(unmeasured) > 0 +} + +unmeasured contains id if { + is_object(judged) + some id, _ in budgets + not id in object.keys(judged) +} + +# --- the README half --------------------------------------------------------- + +# Every row of every Markdown table in README, as its cells. +# +# Backticks are stripped so a cell written as `` `noop` `` matches the id the +# record carries, which is what the predecessor's `gsub(/`/, "", line)` did. +table_rows := [cells | + some line in input.tree.lines["README.md"] + startswith(trim_space(line), "|") + cells := split(replace(line, "`", ""), "|") +] + +# The budget column's index, FOUND BY ITS HEADER and never by a fixed position. +# +# A column index hardcoded here is a coupling between this gate and the table's +# shape, so adding a column to the published table would silently start comparing +# the wrong cell — and comparing the wrong cell is how a gate reports a +# disagreement that is really its own arithmetic. +budget_column := index if { + some cells in table_rows + some index, cell in cells + trim_space(cell) == "budget" +} + +# The published budget per path id, for the rows that publish a number. +# +# A cell is written `≤ ms` for a gated path and `—` for an ungated one, so the +# two states are distinguishable in the table itself. The number is taken as the +# cell's one numeric token rather than by stripping non-digits: `—` yields no such +# token and so is absent here, which is what lets an ungated row be told apart +# from a row publishing zero. +published[id] := budget if { + some cells in table_rows + id := trim_space(cells[1]) + id in object.keys(budgets) + cell := trim_space(cells[budget_column]) + some token in split(cell, " ") + regex.match(data.batten.patterns["published-budget-value"], token) + budget := to_number(token) +} + +# A budgeted path README publishes a different number for. +violation contains { + "rule": "perf-budget-unpublished", + "verdict": "prose state wrong", + "subjects": [{"count": count(disagreeing)}], +} if { + count(disagreeing) > 0 +} + +disagreeing contains id if { + some id, _ in budgets + id in object.keys(published) + published[id] != budgets[id] +} + +# A budgeted path README carries no numeric budget cell for at all — either no +# row, or a row publishing `—` while this module gates it. +disagreeing contains id if { + some id, _ in budgets + count(table_rows) > 0 + not id in object.keys(published) +} + +# --- could not look ---------------------------------------------------------- + +# THE `missing` CLAUSE, and it is not optional. A module that iterates only what +# acquired reports green over a file it never read, and a dead gate and a clean +# tree are byte-identical on the decision surface. `NotAcquired` keeps the two +# causes apart deliberately, so this reports that it could not look rather than +# deciding. +violation contains { + "rule": "perf-budget-unreadable", + "verdict": "source read missing", + "subjects": [{"path": "README.md"}], +} if { + input.tree.missing["README.md"] +} + +# --- the load-time tier ------------------------------------------------------ +# +# These pin the PREDICATE. They cannot pin that the ENGINE composes the record's +# key from a pinned tool and an input digest, or that it fills +# `input.tree.lines["README.md"]` at all — a `with input as` block writes the +# shape it then reads. `crates/batten/tests/it/perf_assert.rs` is the tier that +# drives the compiled binary, and it is where the `missing` clause is asserted for +# that reason. + +# A README table publishing exactly what `budgets` enforces, so a case about the +# measurement half is not also a case about the README half. +agreeing_readme := [ + "| path | what it does | p50 | p95 | budget |", + "| ---- | ------------ | --- | --- | ------ |", + "| `noop` | process start | 2.1 ms | 2.4 ms | ≤ 100 ms |", + "| `check` | one-rule tree | 2.3 ms | 2.7 ms | ≤ 100 ms |", + "| `hook` | adjudication | 2.8 ms | 3.0 ms | ≤ 100 ms |", + "| `passthrough` | a call no rule selects | — | — | ≤ 100 ms |", + "| `posttool` | a PostToolUse call | — | — | ≤ 100 ms |", + "| `wired` | as settings.json invokes it | 8.0 ms | 8.4 ms | ≤ 100 ms |", +] + +# Every budgeted path measured, comfortably inside. +clean_record := { + "noop": "2.4", + "check": "2.7", + "hook": "3.0", + "passthrough": "2.2", + "posttool": "2.3", + "wired": "8.4", +} + +test_a_measurement_inside_every_budget_is_silent if { + count(violation) == 0 with input as {"tree": { + "lines": {"README.md": agreeing_readme}, + "tool-verdict": {"perf-p95": clean_record}, + }} +} + +test_an_over_budget_path_is_refused if { + count(violation) == 1 with input as {"tree": { + "lines": {"README.md": agreeing_readme}, + "tool-verdict": {"perf-p95": object.union(clean_record, {"noop": "150"})}, + }} +} + +# The p95 is fractional milliseconds, so the comparison has to be numeric rather +# than lexical — "99.5" sorts after "100" as a string. +test_a_fractional_p95_inside_its_budget_is_silent if { + count(violation) == 0 with input as {"tree": { + "lines": {"README.md": agreeing_readme}, + "tool-verdict": {"perf-p95": object.union(clean_record, {"noop": "99.5"})}, + }} +} + +test_a_budgeted_path_absent_from_a_present_record_is_refused if { + count(violation) == 1 with input as {"tree": { + "lines": {"README.md": agreeing_readme}, + "tool-verdict": {"perf-p95": object.remove(clean_record, {"wired"})}, + }} +} + +# ABSENT IS NOT INCOMPLETE. A key that does not resolve — a differently pinned +# tool, or a binary rebuilt since the measurement — is not a partial record, and +# reading it as one would fail every checkout that has never run `perf`. +test_no_record_at_all_is_silent if { + count(violation) == 0 with input as {"tree": {"lines": {"README.md": agreeing_readme}}} +} + +# A record carrying a path this module does not budget is measured and not gated, +# which is what the predecessor's table said about `check` before it had a row. +test_an_unbudgeted_path_in_the_record_is_ignored if { + count(violation) == 0 with input as {"tree": { + "lines": {"README.md": agreeing_readme}, + "tool-verdict": {"perf-p95": object.union(clean_record, {"unbudgeted": "9000"})}, + }} +} + +# A SIBLING ROW'S RECORD IS NOT THIS MODULE'S. `input.tree["tool-verdict"]` is +# flattened across every `[[rule.tools]]` row, so without `owned` this module +# would read another producer's lines as its own findings. +test_a_sibling_rows_record_is_not_read if { + count(violation) == 0 with input as {"tree": { + "lines": {"README.md": agreeing_readme}, + "tool-verdict": { + "perf-p95": clean_record, + "hk-plan": {"some-step": "included"}, + }, + }} +} + +test_a_readme_publishing_a_different_budget_is_refused if { + count(violation) == 1 with input as {"tree": { + "lines": {"README.md": array.concat( + array.slice(agreeing_readme, 0, 2), + array.concat( + ["| `noop` | process start | 2.1 ms | 2.4 ms | ≤ 50 ms |"], + array.slice(agreeing_readme, 3, 8), + ), + )}, + "tool-verdict": {"perf-p95": clean_record}, + }} +} + +test_a_readme_with_no_row_for_a_budgeted_path_is_refused if { + count(violation) == 1 with input as {"tree": { + "lines": {"README.md": array.slice(agreeing_readme, 0, 7)}, + "tool-verdict": {"perf-p95": clean_record}, + }} +} + +# THE UNGATED CELL IS THE DISAGREEMENT WRITTEN THE OTHER WAY ROUND. `—` publishes +# "not gated" while this module gates the path, and letting that pass is how the +# published column and the enforced table drift apart in the direction nobody +# notices. +test_a_readme_publishing_no_budget_for_a_gated_path_is_refused if { + count(violation) == 1 with input as {"tree": { + "lines": {"README.md": array.concat( + array.slice(agreeing_readme, 0, 2), + array.concat( + ["| `noop` | process start | 2.1 ms | 2.4 ms | — |"], + array.slice(agreeing_readme, 3, 8), + ), + )}, + "tool-verdict": {"perf-p95": clean_record}, + }} +} + +# THE COLUMN IS FOUND BY ITS HEADER, so a table that grows a column still compares +# the right cell. A fixed index would silently start reading `p95` here. +test_a_reordered_budget_column_is_still_found if { + count(violation) == 0 with input as {"tree": { + "lines": {"README.md": [ + "| path | budget | p50 | p95 |", + "| ---- | ------ | --- | --- |", + "| `noop` | ≤ 100 ms | 2.1 ms | 2.4 ms |", + "| `check` | ≤ 100 ms | 2.3 ms | 2.7 ms |", + "| `hook` | ≤ 100 ms | 2.8 ms | 3.0 ms |", + "| `passthrough` | ≤ 100 ms | — | — |", + "| `posttool` | ≤ 100 ms | — | — |", + "| `wired` | ≤ 100 ms | 8.0 ms | 8.4 ms |", + ]}, + "tool-verdict": {"perf-p95": clean_record}, + }} +} + +test_an_unreadable_readme_is_reported if { + count(violation) == 1 with input as {"tree": {"missing": {"README.md": "absent"}}} +} diff --git a/policy/shell-retirement.rego b/policy/shell-retirement.rego index 670797b44..9c89eca42 100644 --- a/policy/shell-retirement.rego +++ b/policy/shell-retirement.rego @@ -298,7 +298,15 @@ only_drops_a_retired_reference(path) if { # the dropped remainder naming a path this delta deleted, is exactly that edit # and nothing else: it can only ever shorten, never introduce a byte the base # did not already carry at that position. - added := {line | some line in input.tree.lines[path]; not line in {l | some l in base}} + # + # `base_set` IS BOUND ONCE, not rebuilt per head line (CLOUD-1321). The set + # comprehension used to sit inside the `added` comprehension's own body, where + # Rego rebuilds it for every candidate — O(head × base) for one edited path. + # It is a different term from the deletion-linear one this row's harness + # measures (that harness carries no edited governed file in any arm, by the + # row's own protocol), so it is fixed here rather than claimed as the fix. + base_set := {l | some l in base} + added := {line | some line in input.tree.lines[path]; not line in base_set} count({line | some line in added admitted_addition(path, line, removed) @@ -1225,22 +1233,83 @@ withdrawal_reason(path) := words if { } } -# Every ledger row naming `path`, as ` ...`. A row is -# matched on the path being the FIRST field after the marker, so a successor path -# that happens to equal another retired file's name does not claim its mapping. -arms_for(path) := rows if { - rows := {row | - some file, lines in input.tree.lines - startswith(file, "crates/batten/tests/") - some line in lines - some marker in arm_markers - trimmed := trim_space(line) - startswith(trimmed, marker) - fields := split(trim_space(substring(trimmed, count(marker), -1)), " ") - fields[0] == path - row := trimmed - } -} +# Every ledger row, INDEXED BY the path it names — one pass over the corpus for +# the whole evaluation, rather than one pass per deleted path (CLOUD-1321). +# +# THE COST THIS REMOVES, MEASURED. `arms_for` was a FUNCTION whose body was this +# comprehension, and regorus memoizes RULES but not function calls +# (`interpreter.rs:3583`), so the scan below ran once per call. Eleven `violation` +# bodies reach it under `some path in delta.deleted`, and four more reach it +# transitively — `repoints_at_the_declared_successor` and +# `repoints_at_the_declared_invocation` from inside an `added × removed × +# deleted` triple loop. Against the corpus `batten.toml`'s `line_sources` +# declares (~152k lines × 5 markers, with a `trim_space`/`substring`/`split` per +# candidate) that is **~15 s per deleted governed path**: 0.43 s at zero +# deletions, 29.6 s at two, 56.7 s at four, 96.2 s at six. +# +# A RULE RATHER THAN A FUNCTION IS THE WHOLE FIX. `arm_rows` is memoized, so the +# corpus is walked once per `data.batten` query — and `policy::deny` issues +# exactly one of those per bundle, by design. The lookup below is then O(1) and +# the term is flat in the deletion count. +# +# A row is still matched on the path being the FIRST field after the marker, so a +# successor path that happens to equal another retired file's name does not claim +# its mapping — the binding is `path := fields[0]` where the predecessor tested +# `fields[0] == path`, which is the same statement read forwards. +# A COMPREHENSION IN A COMPLETE RULE, never a partial rule, and the difference is +# not style. `arm_rows[path] contains row if { … }` was the first spelling and it +# is UNDEFINED when no body succeeds — an empty corpus does not give it `{}`, it +# gives it nothing — so `arms_for` went undefined for every path and +# `count(arms_for(path)) == 0` stopped holding, silently deleting `shell retire +# missing`. `test_deleted_without_a_mapping_is_refused` caught it, which is the +# case the comment on `arms_for` names for exactly this reason. A comprehension +# always succeeds, so these two rules are total by construction. +arm_pairs := {[path, row] | + # THE GUARD IS FIRST, AND IT IS WHAT KEEPS THE COMMON CASE FREE. `policy::deny` + # queries `data.batten` once for the whole package, so every rule in it is + # evaluated whether or not a `violation` body reads it — where the FUNCTION + # this replaced was only ever called from inside `some path in delta.deleted`. + # Without this conjunct the index is therefore built on every run, and a change + # deleting no governed path pays a corpus scan the predecessor never paid. + # Measured on the fixture corpus: the zero-deletion floor went 388ms -> 864ms, + # a 2.2x regression on by far the most common shape. + # + # A failing conjunct yields no bindings for the generators after it, so when + # the delta deletes nothing this comprehension costs one `count` and stops. It + # stays a comprehension, so it still always succeeds and `arms_for` stays + # total. + count(delta.deleted) > 0 + some file, lines in input.tree.lines + startswith(file, "crates/batten/tests/") + some line in lines + some marker in arm_markers + trimmed := trim_space(line) + startswith(trimmed, marker) + fields := split(trim_space(substring(trimmed, count(marker), -1)), " ") + path := fields[0] + row := trimmed +} + +# The pairs regrouped as `path -> rows`. Quadratic in the LEDGER (~113 rows in +# this tree, so ~13k steps), which is why it is affordable where a re-scan of the +# corpus is not: the corpus is three orders of magnitude larger and was walked +# once per deleted path. +arm_rows := {path: rows | + some [path, _] in arm_pairs + rows := {r | some [p, r] in arm_pairs; p == path} +} + +# Every ledger row naming `path`, as ` ...`. +# +# TOTAL, AND THAT IS LOAD-BEARING RATHER THAN TIDY. The predecessor's body was a +# lone comprehension, which always succeeds, so an unmapped path evaluated to the +# empty set. `arm_rows` has no key for such a path at all, and a bare +# `arm_rows[path]` would be UNDEFINED there — which Rego reads as *does not hold*, +# so `count(arms_for(path)) == 0` below would stop holding and `shell retire +# missing`, this module's central refusal, would silently delete itself. The +# default is what conserves it; `test_deleted_without_a_mapping_is_refused` pins +# it. +arms_for(path) := object.get(arm_rows, path, set()) # The successors named on `path`'s one arm, as the fields after the retired path. successors_for(path) := names if { diff --git a/tests/perf-assert.bats b/tests/perf-assert.bats deleted file mode 100644 index 436c13d27..000000000 --- a/tests/perf-assert.bats +++ /dev/null @@ -1,241 +0,0 @@ -#!/usr/bin/env bats -# subject: mise-tasks/perf-assert.sh -# The decision half of CLOUD-207's latency mechanism, exercised without ever -# measuring anything. -# -# That separation is the point of the suite, not an economy: `mise run perf` -# needs a release build, hyperfine and a quiet machine, none of which belong on -# the landing path — but the thing that can silently rot is the DECISION, and it -# is a pure function of stdin. So this runs in the hk gate on every commit, over -# heredocs, in milliseconds. A published latency number whose gate went dead -# would look exactly like a published latency number. - -setup() { - GATE="$BATS_TEST_DIRNAME/../mise-tasks/perf-assert.sh" - README="$BATS_TEST_TMPDIR/README.md" - # The published table this gate holds its own budgets against. Written per - # test so a case can make it disagree; the real file is asserted separately. - cat >"$README" <<-'EOF' - ## Performance - - | path | p50 | p95 | budget | - | ------- | ------ | ------ | --------- | - | `noop` | 2.6 ms | 3.3 ms | ≤ 100 ms | - | `passthrough` | 2.8 ms | 3.4 ms | ≤ 100 ms | - | `check` | 2.5 ms | 3.2 ms | — | - | `hook` | 2.7 ms | 3.5 ms | ≤ 100 ms | - | `posttool` | 3.0 ms | 3.8 ms | ≤ 100 ms | - | `wired` | 3.4 ms | 4.1 ms | ≤ 100 ms | - EOF -} - -# A full set of records, all comfortably inside budget. -green_records() { - cat <<-'EOF' - path=noop p50=2.59 p95=3.27 mean=2.67 runs=100 - path=passthrough p50=2.81 p95=3.40 mean=2.90 runs=100 - path=check p50=2.54 p95=3.17 mean=2.64 runs=100 - path=hook p50=2.72 p95=3.48 mean=2.96 runs=100 - path=posttool p50=3.02 p95=3.77 mean=3.15 runs=100 - path=wired p50=3.41 p95=4.09 mean=3.55 runs=100 - EOF -} - -@test "records inside budget pass, and say so" { - run bash -c "'$GATE' '$README' <<'IN' -$(green_records) -IN" - [ "$status" -eq 0 ] - [[ "$output" == *"inside its budget"* ]] -} - -@test "the real README publishes the budgets this gate enforces" { - run bash -c "'$GATE' '$BATS_TEST_DIRNAME/../README.md' <<'IN' -$(green_records) -IN" - [ "$status" -eq 0 ] -} - -@test "a budgeted path over its budget is a violation, and is named" { - run bash -c "'$GATE' '$README' <<'IN' -path=noop p50=2.59 p95=3.27 mean=2.67 runs=100 -path=passthrough p50=2.81 p95=3.40 mean=2.90 runs=100 -path=check p50=2.54 p95=3.17 mean=2.64 runs=100 -path=hook p50=90.1 p95=140.5 mean=95.2 runs=100 -path=posttool p50=3.02 p95=3.77 mean=3.15 runs=100 -path=wired p50=3.41 p95=4.09 mean=3.55 runs=100 -IN" - [ "$status" -eq 1 ] - [[ "$output" == *"hook: p95=140.5ms exceeds the 100ms budget"* ]] - # The path that is inside budget is not reported alongside it. - [[ "$output" != *"noop: p95"* ]] -} - -# `check` is measured and deliberately not gated: its cost is bounded by the -# repository it is pointed at, not by batten, so no ceiling here could tell a -# large tree apart from a regression. -@test "the ungated path is never a violation, however slow" { - run bash -c "'$GATE' '$README' <<'IN' -path=noop p50=2.59 p95=3.27 mean=2.67 runs=100 -path=passthrough p50=2.81 p95=3.40 mean=2.90 runs=100 -path=check p50=800.0 p95=1200.0 mean=850.0 runs=100 -path=hook p50=2.72 p95=3.48 mean=2.96 runs=100 -path=posttool p50=3.02 p95=3.77 mean=3.15 runs=100 -path=wired p50=3.41 p95=4.09 mean=3.55 runs=100 -IN" - [ "$status" -eq 0 ] -} - -# Absence is `could not look`, never a pass: a run that measured two of three -# paths and reported green over the two is the partial-coverage false green. -@test "a budgeted path missing from the records is exit 2, not a pass" { - run bash -c "'$GATE' '$README' <<'IN' -path=noop p50=2.59 p95=3.27 mean=2.67 runs=100 -path=passthrough p50=2.81 p95=3.40 mean=2.90 runs=100 -path=check p50=2.54 p95=3.17 mean=2.64 runs=100 -IN" - [ "$status" -eq 2 ] - [[ "$output" == *"hook: budgeted here, absent from stdin"* ]] -} - -@test "empty stdin is exit 2, and names the redirect" { - run bash -c "'$GATE' '$README' "$published" - run bash -c "'$GATE' '$published' <<'IN' -$(green_records) -IN" - [ "$status" -eq 1 ] - [[ "$output" == *"wired"* ]] -}