diff --git a/.config/nextest.toml b/.config/nextest.toml index 8bdb48a06..a32170fdf 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -31,6 +31,50 @@ # `files_read` and `bytes_read`, which count acquisitions at the boundary and so # cannot see an O(n*m) evaluation over a document already in hand. +# THE HANG GUARD, AND THE RATCHET THAT WILL BECOME A SLOW-TEST BAN (CLOUD-1571). +# +# `slow-timeout.period` is when nextest MARKS a case slow and prints it; +# `period x terminate-after` is when it KILLS it, reporting TIMEOUT — a FAILURE, +# because `on-timeout` defaults to "fail". `policy/nextest-slow.rego` gates the +# PRODUCT, so neither half can be raised alone to weaken it. +# +# WHAT THIS BOUNDS TODAY IS A HANG, NOT A BUDGET, and that retreat is measured +# rather than preferred. Four calibrated kills were each refused by CI: +# +# 90s -> `agentic_record::a_replay_...` TIMEOUT 90.022s, under verify load +# 240s -> the same case TIMEOUT 240.210s, windows runner +# 90s -> the three `symbols` cases TIMEOUT ~90.2s, windows runner +# 300s -> four `mutate` cases TIMEOUT 300.0s, 2-vCPU linux ci +# +# The last is the clearest: those four are ~9s each on this container. A 30x +# spread between the box a threshold is chosen on and the runners it is enforced +# on means no locally-measured per-case number is safe, and every kill DESTROYS +# the measurement that would have set the right one — each line above is the +# threshold the case was stopped at, never what it costs. Five guesses would be +# the same mistake five times. +# +# SO THE NUMBER COMES FROM THE JOB BUDGET INSTEAD, which is a real bound rather +# than a guess. `ci.yml` declares `timeout-minutes: 87` for this job on this +# repository's own convention of measured p95 x3. A 1200s per-test kill sits far +# above every case observed on any runner, well inside that budget so it names +# the failing TEST rather than letting the job die anonymously, and still catches +# the class the ban exists for: a genuine hang. Measured on this branch — +# `land.bats`'s mid-wait race sat 40 MINUTES at 0% CPU holding the landing lease +# (CLOUD-1661), which is what an unbounded case actually costs. +# +# `period = "10s"` is unchanged and is the VISIBILITY half: 11 cases here, 8 on +# windows, 4 on the ci runner are named on every run. That list, gathered across +# platforms over time, is what a future step ratchets the kill down onto — with +# data nobody had when this was first armed. +# +# NO PER-CASE OVERRIDES REMAIN. The three that existed — `agentic_record` and the +# `symbols` trio — were each 1200s, so the default now covers them and a separate +# row would assert a bound it no longer sets. CLOUD-1641 still owns +# `agentic_record`'s real cost and is NOT closed by this: the override went away +# because the floor rose, not because the case got fast. +[profile.default] +slow-timeout = { period = "10s", terminate-after = 120 } + [[profile.default.overrides]] filter = 'test(deleting_six_governed_paths_costs_a_flat_multiple_of_the_floor)' threads-required = "num-test-threads" diff --git a/batten.toml b/batten.toml index 3aada79e3..e33edbb97 100644 --- a/batten.toml +++ b/batten.toml @@ -6331,6 +6331,35 @@ tool = "hyperfine" version = "1.20.0" input = "target/release/batten" +# The slow-test ban's declaration, guarded (CLOUD-1571 follow-on). +# +# THE BAN ITSELF IS THE RUNNER'S. `.config/nextest.toml`'s `slow-timeout` marks a +# case slow at `period` and kills it at `period x terminate-after`, reporting +# TIMEOUT — a failure, since `on-timeout` defaults to "fail". Measured on this +# tree: a case under a 50ms period reported TERMINATING, then TIMEOUT, then +# `error: test run failed`, exit 100. +# +# SO THIS ROW GUARDS A DECLARATION RATHER THAN A DURATION, and that division is +# deliberate. `mise-tasks/suite-bench-check.sh` records why a duration gate is the +# wrong instrument — it "would be red on every second run and would be bypassed +# within a day" — and CLOUD-1419 wrote an aggregate ratchet and withdrew it in the +# same branch. nextest measures and decides; batten refuses the weakening. +# +# `lines` NAMES ONE FILE AND THAT IS SAFE HERE, unlike `landing-roster-guarded` +# which had to declare a whole directory. That module's absent-source arm was dead +# because a glob matching zero files leaves the rule SKIPPED rather than evaluated +# — but `.config/nextest.toml` is a literal path, not a glob, so a tree without it +# still acquires the declared source and `nextest-slow-unbounded` fires on the +# empty document. `crates/batten/tests/it/nextest_slow.rs` drives the engine over a +# tree with the file deleted, because that is the only tier that can tell. +[[rule]] +id = "nextest-slow" +kind = "policy" +scope = "tree" +lines = [".config/nextest.toml"] +module = "policy/nextest-slow.rego" +severity = "deny" + # The agentic trials protocol (CLOUD-1116). A completeness predicate over the two # records in `bench/agentic/`, and nothing else — it decides whether a row that # CLAIMS an outcome carries the arms, the fixture, the run count and the falsifier @@ -7342,10 +7371,10 @@ root = "target" keep = 2 [prune.warm] -mb = 9472 -worst_mb = 9472 +mb = 11140 +worst_mb = 11140 multiplier = 1 -measured = "2026-09-06" +measured = "2026-09-08" [prune.cold] mb = 21455 @@ -7669,14 +7698,49 @@ measured = "2026-09-06" # warm's basis refreshed to 220 and cold's left at 208, the very next lap refused # on `[prune.cold]`'s staleness arm with warm never breached. The two arms are # judged at different times and only one of them waits. +# THE 2026-09-08 MOVE, 220 -> 232, BOTH BASES, AND WARM RE-MEASURED (CLOUD-205). +# +# WHAT MOVED THE BASIS is ordinary growth plus one file from this branch: +# `crates/batten/tests/it/nextest_slow.rs`, the compiled tier for the slow-test +# ban. The tree was already 11 past the basis before it, so this branch finished a +# drift rather than causing one. +# +# WHY IT HAD TO MOVE NOW. `target-prune` refused and `verify` narrated the refusal +# as "not enough disk to run the gate" with **13809 MB free against a 10839 MB +# learned floor**. There was ample disk; the arm that refused was the staleness +# one. That misnaming is filed separately — it is the same class as a loop naming +# a cause it never read — and this entry records only that the cause here was the +# basis and not the disk. +# +# WARM IS RE-MEASURED, by the method the block above prescribes: `du -sm target` +# immediately after a successful prune on this container, **11140 MB at 232 +# stems**. That is 48.02 MB per stem against the previous basis's 45.54, so the +# `keep x stems x size` model holds and the figure is a measurement rather than a +# scaling. +# +# AND THE OLD FLOOR WAS UNDER-BUDGETED, which is what this move actually repairs. +# 9472 is 45.54 x 208 — the figure for the basis BEFORE the 2026-09-06 move to 220 +# — so warm has been budgeting for 208 stems against a tree of 232. The block above +# says what that costs: "a floor taken against a smaller stem count passes and then +# lets the build write more than it budgeted for", arriving as a rustc IO error +# inside a test run rather than as a disk fault. The engine had already noticed and +# was judging against a LEARNED floor of 10839 MB rather than the declared 9472. +# +# COLD DOES NOT MOVE, obeying the 2026-09-05 entry rather than ignoring it: its +# exact measurement needs a build from an empty `target`, which needs more free +# space than this container has. It cannot be taken here, which is different from +# being skipped, and it stays OWED. Its BASIS moves with warm's because the +# 2026-09-06 entry records what happens otherwise — refreshing one basis and not +# the other made the very next lap refuse on the other arm, with warm never +# breached. The two arms are judged at different times and only one of them waits. [prune.warm.basis] glob = "crates/batten/tests/**/*.rs" -count = 220 +count = 232 tolerance = 10 [prune.cold.basis] glob = "crates/batten/tests/**/*.rs" -count = 220 +count = 232 tolerance = 10 # THE REGROWABLE ROOTS THE ESCALATION MAY DROP (CLOUD-1157), in the order it drops @@ -12535,6 +12599,42 @@ id = "module read first" kind = "document" target = "policy/ci-cache-declared.rego" +[[verdict]] +id = "suite bind missing" +gloss = "the runner config declares no slow-timeout this gate can read, so no slow-test ban is in force" +class = """ +`period` alone only MARKS a case slow and prints it; `terminate-after` is what kills it, and the TIMEOUT that follows is what fails the run. A declaration carrying one and not the other is not a ban and must not read as one. An absent file, a commented-out declaration, and a period in a unit this module cannot convert all reach the same class deliberately: nextest accepts `2m` and `500ms`, either of which would leave the comparison unreachable and the gate silently green over a bound nobody is enforcing. Fail-closed is the only honest direction for a guard whose subject is whether a guard exists. +""" + +[[verdict.route]] +id = "module read first" +kind = "document" +target = "policy/nextest-slow.rego" + +[[verdict]] +id = "waiver file missing" +gloss = "a per-test slow-timeout override cites no tracker row, so the exception is written but not filed" +class = """ +A per-test override is how a legitimately slow case keeps the ban in force for every other case, so the mechanism is sanctioned and the ceiling deliberately does not reach it. What it owes instead is a reason a reader can follow: an override with no row behind it is the ban switched off for whichever test was inconvenient, and an unexplained exception is the thing that rots while looking like policy. +""" + +[[verdict.route]] +id = "module read first" +kind = "document" +target = "policy/nextest-slow.rego" + +[[verdict]] +id = "bound edit refused" +gloss = "the declared slow-test period is above the ceiling policy/nextest-slow.rego commits to" +class = """ +The ratchet is "never above", not "always exactly", and the asymmetry is the point: lowering the period is free, so a branch that makes the suite faster tightens the bound without negotiating with this gate. Raising it past the ceiling is refused. Lowering the CEILING is a reviewed edit to the module, which is where a reader sees it in the diff rather than inferring it from a number that moved. +""" + +[[verdict.route]] +id = "module read first" +kind = "document" +target = "policy/nextest-slow.rego" + [[startup]] id = "engine-reads-the-authority" gloss = "the engine that hook registrations invoke by bare name can read this repository's committed config" diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 8c7c58d21..4d4264a7c 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -5626,12 +5626,80 @@ fn admission_anchor( // exact match widens to those rows and no further. Every typed kind keeps // the narrow fast path it was given, which is what the ~90s measurement // above is about. + // + // AND "TO THOSE ROWS" IS A STATEMENT ABOUT WHICH *KIND*, WHICH IS NOT THE + // SAME AS WHICH ROW — the difference cost 2m22s per mint (CLOUD-1571). + // + // Widening to every `policy` row ran every policy module over the whole + // tree, whether or not any of them *could* publish the predicate being + // minted against. Measured on `main` at `6eb08e14`: `override request + // --rule protected-mutation` took **2m22.121s**, against **0.123s** for a + // full adjudication of the same tree, and one test case paying it was 24% + // of the entire suite. `protected-mutation` is an engine-side rule name + // with zero hits under `policy/`, so no bundle could ever have published + // it: 58 modules were evaluated to produce findings the filter below + // discards one line later. + // + // `Bundle::declared` is the module's own published predicate set, so + // asking it is what turns "which kind" into "which row". It is the same + // authority `attribute` resolves a violation's id against and the one + // `lint.rs` reads for `waiver-names-no-rule` (CLOUD-1553), on identical + // reasoning: reading the published set adds no second authority over what a + // bundle declares, where re-deriving it from module source would. + // + // THE LOAD IS NOT THE COST, and that is why this is affordable rather than + // merely narrower. Compiling every bundle is **2.4s** measured here against + // the 2m22s above — the expense is acquiring the tree and evaluating over + // it, which is exactly what this now skips. The load is also the same work + // `run_over` does internally, so no module is compiled that would not have + // been. + // + // CLOUD-1087/CLOUD-1125's property is untouched: a predicate a bundle DOES + // publish still selects that bundle's row and still anchors on its finding. + // This narrows the widening; it does not restore the `declared.id == rule` + // filter that made every policy admission a silent no-op. let selected: Vec<_> = if exact.is_empty() { - config + let policy_rows: Vec<_> = config .rules .iter() .filter(|declared| declared.kind == rules::RuleKind::Policy) .cloned() + .collect(); + // COULD-NOT-LOOK FALLS BACK rather than refusing, which is the posture + // the `run_over` call below already takes for its own `Err`: a bundle + // set that will not load leaves no honest way to ask who publishes the + // predicate, and `head()` is never weaker than what shipped. + let Ok(bundles) = policy::load( + root, + &policy_rows, + policy::Vocabulary { + patterns: &config.patterns, + verdicts: &config.verdicts, + recorders: &config.recorders, + }, + // The same entitlement the run below is given. A mint answers about + // one rule over one subject, so registry equality's exhausted half — + // a property of the whole authority — is not this verb's to assert. + policy::ModuleChecks::RunOverSelection, + None, + ) else { + return head(); + }; + let publishers = policy::publishers_of(&bundles, rule); + // NOBODY PUBLISHES IT, so there is nothing to scan FOR and the scan is + // skipped entirely rather than run and discarded. This is the honest + // answer for an engine-side rule name, which is a legitimate thing to + // mint against — `protected-mutation` has a real override route. + // + // Silent, deliberately: reporting the rule as unknown here would decide + // CLOUD-1551's open question about an anchor that binds `call:` when + // nothing matches, and this row must not settle that by accident. + if publishers.is_empty() { + return head(); + } + policy_rows + .into_iter() + .filter(|declared| publishers.contains(declared.id.as_str())) .collect() } else { exact diff --git a/crates/batten/src/policy.rs b/crates/batten/src/policy.rs index e90267895..195cc28d6 100644 --- a/crates/batten/src/policy.rs +++ b/crates/batten/src/policy.rs @@ -463,6 +463,36 @@ impl Bundle { } } +/// The enabling-row ids of every bundle that publishes `predicate` (CLOUD-1571). +/// +/// **The narrowing a mint needs, and the reason it is a function rather than a +/// filter written at its one call site.** [`crate::admission`]'s anchor has to +/// re-run the rule a refusal named in order to recover its fingerprint, and +/// `--rule` carries a PREDICATE id — `filed-here` publishes `filed-over-own-diff` +/// — so a row-id match selects nothing and the mint silently binds the head +/// (CLOUD-1087, CLOUD-1125). Widening from there to every `policy` row fixed that +/// and cost 2m22s per mint, because "which KIND of row" is not "which row": +/// `protected-mutation` is an engine-side name no bundle can publish, and 58 +/// modules were evaluated over the whole tree to produce findings the caller +/// discarded one line later. +/// +/// Asking each bundle's own published set answers the question exactly. An EMPTY +/// result is therefore a real answer — *nothing can raise this predicate, so +/// there is nothing to scan for* — and never a could-not-look: a caller reads it +/// as "skip the scan", which is only sound because [`Bundle::declared`] is the +/// same authority [`Bundle::attribute`] resolves a violation's id against. +/// +/// Borrowed rather than owned, so a caller filtering its own rows against the +/// result allocates nothing per row. +#[must_use] +pub fn publishers_of<'a>(bundles: &'a [Bundle], predicate: &str) -> BTreeSet<&'a str> { + bundles + .iter() + .filter(|bundle| bundle.declared().contains(predicate)) + .map(Bundle::id) + .collect() +} + impl std::fmt::Debug for Bundle { /// Names the row, its modules' paths and the ids they publish — and **never /// a source**, so a policy body cannot reach a log through a derived `Debug` diff --git a/crates/batten/tests/it/admission_narrowing.rs b/crates/batten/tests/it/admission_narrowing.rs new file mode 100644 index 000000000..fcb64d342 --- /dev/null +++ b/crates/batten/tests/it/admission_narrowing.rs @@ -0,0 +1,229 @@ +//! `policy::publishers_of` — the narrowing a mint's anchor selects with +//! (CLOUD-1571). +//! +//! # What went wrong, because it decides what these cases have to pin +//! +//! `admission_anchor` re-runs the rule a refusal named so it can recover that +//! finding's fingerprint and bind the admission to it. `--rule` carries a +//! PREDICATE id — `filed-here` publishes `filed-over-own-diff` — so filtering +//! `declared.id == rule` selected nothing, the scan produced no finding, and the +//! mint silently took the `head()` fallback: an admission answered, spent, and +//! queried by nothing (CLOUD-1087, CLOUD-1125). +//! +//! Widening an empty exact match to every `policy` row fixed that and cost +//! **2m22.121s per mint**, measured on `main` at `6eb08e14` against **0.123s** +//! for a full adjudication of the same tree. The bound "widens to those rows and +//! no further" is a statement about which KIND of row, and the regression is that +//! this is not the same as which ROW: `protected-mutation` is an engine-side rule +//! name with zero hits under `policy/`, so **no bundle could ever publish it**, +//! and 58 modules were evaluated over the whole tree to produce a finding set +//! discarded one line later. One test case paying that was 24% of the entire +//! suite. +//! +//! # Why the pure function is the subject +//! +//! The narrowing is the decision; the early return that skips the scan is one +//! line downstream of it. `admission_anchor` reads stdin and is reachable only +//! through `override request`, so the decision is extracted where it can be +//! driven directly rather than asserted through a verb that also parses answers, +//! resolves an epoch and reads a git identity. +//! +//! **The last case is the one that matters, and it is not a fixture.** It asks +//! the REAL committed bundles the real question, so a module later publishing +//! `protected-mutation` — which would silently restore the full scan — reddens +//! here. A fixture-only suite would pass over exactly that. + +// 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::fs; +use std::path::{Path, PathBuf}; + +use batten::policy::{self, Bundle}; +use batten::rules::Rule; + +/// A module publishing one predicate whose id is not its enabling row's — the +/// shape that defeated the `declared.id == rule` filter in the first place. +const PUBLISHES: &str = r#" +package batten + +import rego.v1 + +rules contains "PREDICATE" + +violation contains {"rule": "PREDICATE", "verdict": "stray key probe"} if { + input.tree.documents["config.toml"].stray +} +"#; + +fn scratch(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("batten-narrow-{name}-{}", std::process::id())); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).expect("scratch"); + dir +} + +/// One bundle folder per row, each publishing a predicate the row is not named +/// after. Distinct packages and distinct predicate ids, because `load` refuses a +/// shared source and a shared id alike. +fn write_bundle(root: &Path, row_id: &str, predicate: &str) { + let dir = root.join(format!("policy-{row_id}")); + fs::create_dir_all(&dir).expect("bundle folder"); + let module = PUBLISHES + .replace("package batten", &format!("package batten.b{row_id}")) + .replace("PREDICATE", predicate); + fs::write(dir.join("gate.rego"), module).expect("module"); +} + +fn row(id: &str) -> Rule { + serde_json::from_value(serde_json::json!({ + "id": id, + "kind": "policy", + "scope": "tree", + "bundle": format!("policy-{id}/"), + "documents": ["config.toml"], + "severity": "deny", + })) + .expect("a tree-scoped policy row the loader accepts") +} + +/// The vocabulary derived from what the fixture's modules actually raise — +/// registry equality runs both ways, so a table naming an unraised token is dead +/// vocabulary and `load` refuses it. +fn load_fixture(root: &Path, rows: &[Rule]) -> Vec { + let verdicts = common::verdicts_in(root); + policy::load( + root, + rows, + policy::Vocabulary { + // NO PATTERN ROWS, and that is a statement rather than an omission: + // these modules resolve none, so supplying any would be input no + // consumer supplies. + patterns: &[], + verdicts: &verdicts, + recorders: &[], + }, + policy::ModuleChecks::RunOverSelection, + None, + ) + .expect("the fixture bundles load") +} + +/// THE REGRESSION, in one assertion. A predicate no bundle publishes selects no +/// bundle, so the caller has nothing to scan for and skips the scan entirely. +/// +/// Fails by: restoring the widen-to-every-policy-row filter, which selects both +/// rows here and, on the real tree, all 58. +#[test] +fn a_predicate_no_bundle_publishes_selects_nothing() { + let root = scratch("unpublished"); + write_bundle(&root, "alpha", "alpha-violated"); + write_bundle(&root, "beta", "beta-violated"); + let bundles = load_fixture(&root, &[row("alpha"), row("beta")]); + + assert!( + policy::publishers_of(&bundles, "protected-mutation").is_empty(), + "an engine-side rule name is published by nothing, so nothing is selected \ + and no module is evaluated (CLOUD-1571)" + ); +} + +/// THE PROPERTY CLOUD-1087 AND CLOUD-1125 BOUGHT, which this must not spend. A +/// predicate a bundle DOES publish still selects that bundle — so the mint still +/// reaches its finding and still anchors on it rather than falling back. +/// +/// Fails by: narrowing back to `declared.id == rule`, which selects nothing here +/// because no row is named after the predicate it publishes. +#[test] +fn a_published_predicate_selects_its_own_bundle_and_only_that_one() { + let root = scratch("published"); + write_bundle(&root, "alpha", "alpha-violated"); + write_bundle(&root, "beta", "beta-violated"); + let bundles = load_fixture(&root, &[row("alpha"), row("beta")]); + + let selected = policy::publishers_of(&bundles, "beta-violated"); + assert_eq!( + selected.into_iter().collect::>(), + vec!["beta"], + "exactly the publishing bundle, so the scan is one module rather than every one" + ); +} + +/// ANTI-VACUITY ON THE FIXTURE ITSELF. Both cases above would pass over a bundle +/// set that failed to publish anything at all — an empty `declared` set makes +/// every query empty, and the first case would then be green for the wrong +/// reason. +#[test] +fn the_fixture_bundles_actually_publish_their_predicates() { + let root = scratch("anti-vacuity"); + write_bundle(&root, "alpha", "alpha-violated"); + let bundles = load_fixture(&root, &[row("alpha")]); + + assert_eq!(bundles.len(), 1, "one row, one bundle"); + assert!( + bundles[0].declared().contains("alpha-violated"), + "the module publishes the predicate the cases above query for; without \ + this, an empty declared set would make every query vacuously empty" + ); + assert!( + !policy::publishers_of(&bundles, "alpha-violated").is_empty(), + "and the query reaches it" + ); +} + +/// THE FIDELITY CASE, over the REAL committed bundles rather than a fixture. +/// +/// `protected-mutation` is the measured instance: it is the rule every protected +/// path write mints against, it is engine-side (`decision.rs`, `refusal.rs`, +/// `hook.rs`, `rules.rs`, `admission.rs`), and it has **zero hits under +/// `policy/`**. If some later module publishes it, the scan silently goes wide +/// again and the 2m22s comes back with nothing to announce it. That is what this +/// case refuses, and no fixture can. +/// +/// The committed `filed-over-own-diff` is asserted beside it in the same +/// function, because a repository whose bundles failed to load would give an +/// empty set for the first assertion and pass it for exactly the wrong reason. +#[test] +fn the_committed_bundles_publish_no_engine_side_rule_name() { + let root = common::at_root("batten.toml"); + let root = root.parent().expect("the committed config has a parent"); + let config = batten::resolve::resolve(root, &batten::resolve::Overrides::default()) + .expect("the committed config resolves"); + let policy_rows: Vec<_> = config + .rules + .iter() + .filter(|declared| declared.kind == batten::rules::RuleKind::Policy) + .cloned() + .collect(); + let bundles = policy::load( + root, + &policy_rows, + policy::Vocabulary { + patterns: &config.patterns, + verdicts: &config.verdicts, + recorders: &config.recorders, + }, + policy::ModuleChecks::RunOverSelection, + None, + ) + .expect("the committed bundles load"); + + // ANTI-VACUITY FIRST, so the refusal below cannot pass over an empty set. + assert!( + policy::publishers_of(&bundles, "filed-over-own-diff") + .into_iter() + .eq(["filed-here"]), + "the committed tree still publishes a predicate under a differently-named \ + row — the shape the whole narrowing exists to handle" + ); + + assert!( + policy::publishers_of(&bundles, "protected-mutation").is_empty(), + "`protected-mutation` is engine-side and no module may publish it; if one \ + does, every mint against it goes back to evaluating {} bundles over the \ + whole tree (CLOUD-1571)", + bundles.len() + ); +} diff --git a/crates/batten/tests/it/main.rs b/crates/batten/tests/it/main.rs index a7c3c0a10..3e0f9f2f5 100644 --- a/crates/batten/tests/it/main.rs +++ b/crates/batten/tests/it/main.rs @@ -46,6 +46,7 @@ mod acquisition_sweep; mod address_resolve; mod address_transport; mod admission; +mod admission_narrowing; mod advisory_drain; mod agent_capabilities; mod agent_facts; @@ -168,6 +169,7 @@ mod mise_pin_agreement; mod mutate; mod mutation_declared_case; mod narrow_adoption; +mod nextest_slow; mod obligations_bound; mod outcome_advice; mod perf_assert; diff --git a/crates/batten/tests/it/nextest_slow.rs b/crates/batten/tests/it/nextest_slow.rs new file mode 100644 index 000000000..fb42bf5a0 --- /dev/null +++ b/crates/batten/tests/it/nextest_slow.rs @@ -0,0 +1,227 @@ +//! `policy/nextest-slow.rego` over the COMPILED engine (CLOUD-1571 follow-on). +//! +//! # Why this file exists when the module already has `test_` rules +//! +//! Those are the load-time tier and they pin the PREDICATE. They cannot pin that +//! the engine BUILDS the input the predicate reads: `with input as` fabricates +//! the very shape the engine may be unable to produce, so a module reading a key +//! nothing fills passes its own suite green and enforces nothing. +//! +//! # The absent-file case is the one that earns this file +//! +//! `landing-roster-guarded` shipped with a could-not-look arm that could never +//! fire: its row named a GLOB, and a glob matching zero files leaves the rule +//! SKIPPED rather than evaluated over an empty document, so a branch deleting the +//! subject passed clean. `.config/nextest.toml` is a literal path rather than a +//! glob, which should mean the declared source is still acquired and the refusal +//! still fires — but "should" is exactly what that module also believed. +//! `an_absent_runner_config_is_refused` is what decides it, and only a tier +//! driving the engine over a tree with the file removed can. +//! +//! It doubles as the channel probe `rules/policy-modules.md` prescribes: a clean +//! report and a module that never ran are byte-identical on the decision surface, +//! and a case that reddens when the subject is removed tells them apart. +//! +//! # And the fidelity case drives the REAL committed file +//! +//! `the_committed_config_declares_a_terminating_slow_timeout` scans this +//! repository's own `.config/nextest.toml`. A fixture-only suite would pass over +//! a tree whose slow-test ban had been quietly disarmed, which is the regression +//! this module exists to refuse. + +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use crate::common; + +use std::fs; +use std::path::{Path, PathBuf}; + +use batten::rules::{self, Rule}; + +/// The predicate ids the module declares. +const UNBOUNDED: &str = "nextest-slow-unbounded"; +const RAISED: &str = "nextest-slow-raised"; + +/// The one path the module is anchored on. +const CONFIG: &str = ".config/nextest.toml"; + +/// `period` with no `terminate-after` only REPORTS. Not a ban. +const REPORT_ONLY: &str = "[profile.default]\nslow-timeout = \"10s\"\n"; + +/// Above the ceiling the module commits to: 30s x 60 is 1800s against 1200s. +const RAISED_BODY: &str = + "[profile.default]\nslow-timeout = { period = \"30s\", terminate-after = 60 }\n"; + +/// A fixture tree carrying a runner config with `body`, or none at all when +/// `body` is `None`. +fn repo(name: &str, body: Option<&str>) -> PathBuf { + let root = common::scratch(name); + fs::create_dir_all(root.join(".config")).expect("scratch config dir"); + if let Some(body) = body { + fs::write(root.join(CONFIG), body).expect("write the runner config"); + } + install_module(&root); + root +} + +/// The COMMITTED module, copied rather than re-typed. A fixture carrying its own +/// copy of the predicate would pass while the shipped one was broken, which is +/// the fidelity failure this tier exists to catch. +fn install_module(root: &Path) { + let source = common::at_root("policy/nextest-slow.rego") + .canonicalize() + .expect("the committed module is where the row says it is"); + fs::create_dir_all(root.join("policy")).expect("scratch policy dir"); + fs::copy(source, root.join("policy/nextest-slow.rego")).expect("install committed module"); +} + +/// The committed row's shape, so a registration the loader would reject cannot +/// pass here. +fn row() -> Rule { + serde_json::from_value(serde_json::json!({ + "id": "nextest-slow", + "kind": "policy", + "scope": "tree", + "lines": [CONFIG], + "module": "policy/nextest-slow.rego", + "severity": "deny", + })) + .expect("the loader accepts the committed row's shape") +} + +fn scan(root: &Path) -> rules::Scan { + let verdicts = common::verdicts_in(root); + // NO PATTERN ROWS, and that is a statement rather than an omission: this + // module resolves none — the period is read with string builtins precisely so + // no inline regex and no registry row is needed — so supplying any would be + // input no consumer supplies and the cases would pass for the wrong reason. + rules::run_static( + &[row()], + &[], + batten::policy::Vocabulary { + patterns: &[], + verdicts: &verdicts, + recorders: &[], + }, + root, + ) + .expect("the read surface runs a policy row") +} + +fn rules_fired(root: &Path) -> Vec { + scan(root) + .findings + .into_iter() + .map(|finding| finding.rule) + .collect() +} + +/// THE FIDELITY CASE. The COMMITTED bytes of the file that actually bans slow +/// tests here, scanned by the engine in a fixture — the repo root itself cannot be +/// scanned with a one-rule subset, because `check_registry_is_exhausted` then +/// reports every class the other rules would have raised. +/// `#MUTANT declaration-unread` reddens exactly here. +#[test] +fn the_committed_config_declares_a_terminating_slow_timeout() { + let committed = fs::read_to_string(common::at_root(CONFIG)) + .expect("the committed runner config is where the row says it is"); + let root = repo("nextest-slow-committed", Some(&committed)); + assert!( + rules_fired(&root).is_empty(), + "the committed {CONFIG} must declare a slow-timeout with terminate-after, at or \ + under the module's ceiling; if this fails the slow-test ban has been disarmed" + ); +} + +/// THE CASE THIS TIER EXISTS FOR, and the one a `with input as` case cannot +/// decide. `landing-roster-guarded` carried a could-not-look arm that never fired +/// because its row named a glob and a glob matching nothing leaves the rule +/// SKIPPED. This row names a literal path; that it therefore still acquires the +/// source and still refuses is measured here rather than assumed. +/// +/// It is also the channel probe: a clean report and a module that never ran are +/// byte-identical, and this reddening is what tells them apart. +#[test] +fn an_absent_runner_config_is_refused() { + let root = repo("nextest-slow-absent", None); + assert_eq!( + rules_fired(&root), + vec![UNBOUNDED.to_owned()], + "deleting the runner config removes the ban, and must refuse rather than read clean" + ); +} + +/// `period` ALONE ONLY REPORTS. Without `terminate-after` nextest marks a case +/// slow and lets it run, so this is not a ban and must not read as one. +#[test] +fn a_declaration_without_terminate_after_is_refused() { + let root = repo("nextest-slow-report-only", Some(REPORT_ONLY)); + assert_eq!(rules_fired(&root), vec![UNBOUNDED.to_owned()]); +} + +/// THE RATCHET, over the engine. A period above the committed ceiling is refused. +#[test] +fn a_period_above_the_ceiling_is_refused() { + let root = repo("nextest-slow-raised", Some(RAISED_BODY)); + assert_eq!(rules_fired(&root), vec![RAISED.to_owned()]); +} + +/// THE HOLE THE FIRST VERSION OF THIS MODULE SHIPPED WITH, over the engine. +/// +/// That version bounded `period` alone. `period` is only when a case is MARKED +/// slow; `terminate-after` is the multiplier that decides when it is actually +/// killed, so a small period with a large multiplier passed the gate while +/// banning nothing — 10s x 200 is a 2000s kill behind a period well inside any +/// ceiling. The bound has to be on the product, because the product is what +/// refuses a test. +#[test] +fn a_small_period_with_a_large_multiplier_is_refused() { + let root = repo( + "nextest-slow-large-multiplier", + Some("[profile.default]\nslow-timeout = { period = \"10s\", terminate-after = 200 }\n"), + ); + assert_eq!( + rules_fired(&root), + vec![RAISED.to_owned()], + "the kill threshold is period x terminate-after; a gate reading the period alone \ + passes this and bans nothing" + ); +} + +/// AND LOWERING IS FREE — the asymmetry that makes this a ratchet rather than an +/// equality check. Without this case the rule above is satisfied by a module that +/// refuses every value that is not exactly the ceiling, which would make every +/// speed-up negotiate with the gate. +#[test] +fn a_period_below_the_ceiling_is_clean() { + let root = repo( + "nextest-slow-lowered", + Some("[profile.default]\nslow-timeout = { period = \"4s\", terminate-after = 9 }\n"), + ); + assert!(rules_fired(&root).is_empty()); +} + +/// A UNIT THE MODULE CANNOT CONVERT REFUSES rather than leaving the comparison +/// unreachable. `2m` is a legal nextest value, and without this arm it would read +/// as a clean tree while no bound was being enforced at all — the silent hole the +/// module's header calls fail-closed. +#[test] +fn a_period_in_an_unconvertible_unit_is_refused() { + let root = repo( + "nextest-slow-minutes", + Some("[profile.default]\nslow-timeout = { period = \"2m\", terminate-after = 3 }\n"), + ); + assert_eq!(rules_fired(&root), vec![UNBOUNDED.to_owned()]); +} + +/// AND A COMMENTED-OUT DECLARATION IS NOT ONE. The ordinary shape of "this was +/// flaky, disabling it for now" leaves the text in the file, which is exactly how +/// `landing-roster-guarded`'s first draft was defeated by its own documentation. +#[test] +fn a_commented_declaration_does_not_arm_the_ban() { + let root = repo( + "nextest-slow-commented", + Some("[profile.default]\n# slow-timeout = { period = \"10s\", terminate-after = 9 }\n"), + ); + assert_eq!(rules_fired(&root), vec![UNBOUNDED.to_owned()]); +} diff --git a/mise.toml b/mise.toml index 211e87907..4dca15988 100644 --- a/mise.toml +++ b/mise.toml @@ -476,9 +476,24 @@ CI_FANIN_WORKFLOW = ".github/workflows/ci.yml" # exactly when upstream is the thing that moved, and never otherwise. Reading the # README from a gate instead would answer "what does upstream claim right now", # which is a property of the world and belongs on a clock (`lock-complete`). +# THE SHELL SUITE'S SLOW-TEST BAN, mirroring the nextest one (CLOUD-1571). +# +# `.config/nextest.toml` bans a slow RUST case with `slow-timeout`. bats had no +# equivalent, so a hung shell case had nothing to stop it — measured on this +# branch: `land.bats`'s "main moving mid-wait starts the next lap" sat at 0% CPU +# for 40 MINUTES inside `verify`, holding the landing lease, with no output and +# no diagnostic. The case is a stubbed race between `ci-wait` and `main-watch`, +# and under a loaded box the race did not resolve. +# +# A hang is the worst shape a test failure takes: it produces no verdict, spends +# unbounded wall clock, and on the landing path it holds a singleton the whole +# time. 300s matches the nextest kill, so both suites answer "too slow" with one +# number, and it is far above the whole suite's per-case cost — bats finishes all +# of its cases in ~228s together. +BATS_TEST_TIMEOUT = "300" REGORUS_OPA_COMPLIANCE = "1.2.0" REGORUS_OPA_COMPLIANCE_FOR = "0.11" -MUTANT_GATES = "mise,attestation-check,engine-config,engine-doctor,engine-landed,engine-pinned,engine-ready,agentic-experiment-record,awk-regex-check,bats-invocation,batten-glob-check,board-diff-overlap,board-payloads,board-sweep,branch-age-check,cap-drift,ci-cache-declared,ci-hygiene,ci-lease-precondition,ci-parity,ci-slow-needed,ci-suite-lane,ci-tools-check,claim-before-code,claim-order-is-stated,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,egress-fencing,evaluator-closure-check,evaluator-io-check,filed-here,finding-sink-check,fixture-forks,forge-verdict-required,graph-check,harness-grant,harness-wiring,hk-fix-selection,hk-plan-required,hook-pin-check,hook-skip-local,in-progress-drain,install-check,land,land-divergence-assert,land-lock,land-lock-check,landed-check,landing-loop,landing-roster-guarded,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-lint,reclaim-census,release-assets-check,release-due,release-provision-parity,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,test-targets,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,trunk-based,validator-verdict-clean,verdict-routes-resolve,verified,weakens-declared,worktree-registration" +MUTANT_GATES = "mise,attestation-check,engine-config,engine-doctor,engine-landed,engine-pinned,engine-ready,agentic-experiment-record,awk-regex-check,bats-invocation,batten-glob-check,board-diff-overlap,board-payloads,board-sweep,branch-age-check,cap-drift,ci-cache-declared,ci-hygiene,ci-lease-precondition,ci-parity,ci-slow-needed,ci-suite-lane,ci-tools-check,claim-before-code,claim-order-is-stated,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,egress-fencing,evaluator-closure-check,evaluator-io-check,filed-here,finding-sink-check,fixture-forks,forge-verdict-required,graph-check,harness-grant,harness-wiring,hk-fix-selection,hk-plan-required,hook-pin-check,hook-skip-local,in-progress-drain,install-check,land,land-divergence-assert,land-lock,land-lock-check,landed-check,landing-loop,landing-roster-guarded,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-lint,reclaim-census,release-assets-check,release-due,release-provision-parity,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,test-targets,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,trunk-based,validator-verdict-clean,verdict-routes-resolve,verified,weakens-declared,worktree-registration,nextest-slow" # --- 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. diff --git a/policy/nextest-slow.rego b/policy/nextest-slow.rego new file mode 100644 index 000000000..02236afb8 --- /dev/null +++ b/policy/nextest-slow.rego @@ -0,0 +1,357 @@ +# The slow-test ban, and the ratchet that walks it down (CLOUD-1571 follow-on). +# +# WHAT ENFORCES THE BAN IS NOT THIS MODULE. `.config/nextest.toml` declares +# `slow-timeout = { period, terminate-after }`; nextest marks a case slow at +# `period` and KILLS it at `period x terminate-after`, reporting TIMEOUT, and a +# TIMEOUT is a failure because `on-timeout` defaults to "fail". That is the ban, +# in the runner, on every local run and in CI. Measured on this tree before this +# module was written: a case run under a 50ms period reported `TERMINATING`, then +# `TIMEOUT`, then `error: test run failed`, exit 100. +# +# THIS MODULE ONLY REFUSES WEAKENING IT. The split matters. A gate that +# re-derived per-case durations would be the instrument this repository has +# already refused twice — `mise-tasks/suite-bench-check.sh` records that a +# duration gate "would be red on every second run and would be bypassed within a +# day", and CLOUD-1419 wrote an aggregate ratchet and withdrew it in the same +# branch because its first firing was on the change that improved the thing it +# guarded. The runner's timeout has neither problem, so the honest division is: +# nextest measures and decides, batten guards the declaration. +# +# IT GATES THE KILL THRESHOLD, NOT THE PERIOD, AND THE FIRST VERSION GOT THAT +# WRONG. That version bounded `period` alone — which is only when a case is +# MARKED slow. `terminate-after` is the multiplier that decides when it is +# actually killed, so `period = "10s"` with `terminate-after = 10000` passed the +# gate while banning nothing at all. The bound has to be on the product, because +# the product is what refuses a test. +# +# THE CEILING IS A LITERAL HERE, exactly as `perf-assert.rego` holds its budgets. +# A `policy/*.rego` module IS consumer config, so a number is at home in it; +# non-negotiable rule 1 scopes to `crates/batten`. `rules/policy-modules.md` +# refuses a threshold spelled as a `[[pattern]]` row for the opposite reason — +# arithmetic is not a concept with one spelling — and this is not that. +# +# THE RATCHET IS "NEVER ABOVE", NOT "ALWAYS EXACTLY". `ceiling_seconds` is the +# committed maximum kill threshold. Lowering it in `.config/nextest.toml` is free, +# which is the whole point: a branch that makes the suite faster tightens the +# bound without negotiating with this gate. Raising it above the ceiling is +# refused, and lowering the CEILING is a reviewed edit to this file that a reader +# sees in the diff. That asymmetry is what a ratchet is. +# +# AND THE DAY-ONE CEILING IS A RUNAWAY GUARD RATHER THAN A PER-CASE SLOW BAN, +# which is a measured retreat rather than a preference. Three calibration attempts +# were each refused by CI: a 90s kill, then a 240s override, then 1200s, and the +# last refusal was the three `symbols` cases at exactly 90s. CLOUD-1439 documents +# why those three: they share one cold `cargo clippy` build, so under parallelism +# one builds and two WAIT, and nextest bills all three the build. On the Windows +# runner that build is far slower than on any box this repository can measure +# from. A fourth guess at a number nobody here can observe is the same mistake a +# fourth time. +# +# So the ceiling starts above the whole known band — Windows reports 8 cases over +# 10s and 3 over 90s — and the VISIBILITY period stays at 10s, so every one of +# those cases is named on every run. The ban today is on runaway and hung tests; +# tightening it toward the per-case target is exactly what this ratchet exists to +# do, one reviewed step at a time, on cross-platform data nobody had when it was +# armed. +# +# NO INLINE REGEX, AND THE PARSE IS STRING BUILTINS ONLY. An inline pattern is +# refused at load and this is not a concept the `[[pattern]]` registry should +# carry, so both halves are read by splitting the line. +# +# AND AN UNREADABLE DECLARATION REFUSES RATHER THAN PASSING, which is the +# direction that matters. nextest accepts `2m` and `500ms` as well as `10s`; both +# would leave `to_number` undefined, the comparison unreachable, and the gate +# silently green over a bound nobody is enforcing. So the absent, the +# unterminated and the unreadable are ONE class — every one of them means no ban +# is in force that this gate can vouch for. +#MUTANT-SUITE crates/batten/tests/it/nextest_slow.rs +#MUTANT terminator-unread|s@^\tcontains(line, "terminate-after")$@\ttrue@|a_declaration_without_terminate_after_is_refused +#MUTANT ceiling-may-rise|s@^\tkill > ceiling_seconds$@\tfalse@|a_kill_threshold_above_the_ceiling_is_refused +#MUTANT declaration-unread|s@^lines := input.tree.lines\[config\]$@lines := []@|the_committed_config_declares_a_terminating_slow_timeout +#MUTANT override-reason-unread|s@^\tnot filed(i)$@\ttrue@|an_override_that_cites_a_row_is_clean +#MUTANT multiplier-ignored|s@^\tkill := period \* multiplier$@\tkill := period@|a_kill_threshold_above_the_ceiling_is_refused +# +# THE THIRD MUTATION EMPTIES THE LINE WALK rather than negating a conjunct, for +# `landing-roster-guarded`'s reason: emptying it makes the declaration +# unreachable, which reddens the PASS case over the real committed file. The +# fourth is the one the first version of this module could not have: dropping the +# multiplier restores the period-only bound, which is precisely the hole that +# shipped, and it reddens the case where a large `terminate-after` carries the +# kill past the ceiling while the period stays small. + +# 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.nextest_slow + +import rego.v1 + +rules contains "nextest-slow-unbounded" + +rules contains "nextest-slow-raised" + +rules contains "nextest-slow-override-unfiled" + +# The runner's committed configuration. A consumer path in a consumer module, +# which is where non-negotiable rule 1 puts it. +config := ".config/nextest.toml" + +# The ceiling, in seconds: the largest KILL THRESHOLD this repository accepts, +# where the threshold is `period x terminate-after` and not the period alone. +# +# Walked down as the suite gets faster, one reviewed step at a time. It sits above +# today's whole known band on the slowest runner, so the gate's first firing can +# only be on a change that loosens the bound — never on the tree it inherits, +# which is the shape `fixture-forks.rego` records as the one that gets an +# exception written for it "and the exception is what rots". +ceiling_seconds := 1200 + +# The committed runner config, by line index — the index is load-bearing, because +# which SECTION a `slow-timeout` sits under is what says whether it is the default +# bound or a named exception. +lines := input.tree.lines[config] + +# The nearest section header at or before `i`. A TOML file is ordered, so the last +# header before a line is the table that line belongs to; there is no other way to +# tell a `[profile.default]` value from a `[[profile.default.overrides]]` one when +# reading lines. +section(i) := header if { + before := [j | + some j, line in lines + j < i + startswith(trim_space(line), "[") + ] + count(before) > 0 + header := trim_space(lines[max(before)]) +} + +# A non-comment line declaring BOTH halves of the ban. `period` alone only +# reports; it is `terminate-after` that kills, so a declaration carrying one and +# not the other is not a ban and must not read as one. +terminating_at contains i if { + some i, line in lines + not startswith(trim_space(line), "#") + contains(line, "slow-timeout") + contains(line, "period") + contains(line, "terminate-after") +} + +# THE DEFAULT BOUND: what every case is held to, and the only thing the ceiling +# speaks about. +terminating contains lines[i] if { + some i in terminating_at + section(i) == "[profile.default]" +} + +# AN EXCEPTION: a per-test override. The ceiling deliberately does NOT reach these +# — bounding them by the default's number would make the exception mechanism +# unusable, and a gate that forbids the sanctioned escape is the shape that gets +# switched off. What they owe instead is a filed reason, below. +override_at contains i if { + some i in terminating_at + startswith(section(i), "[[profile.default.overrides]]") +} + +# An override whose preceding comment block cites a tracker row. The window is +# generous because the rationale for an exception is prose, and prose is the point +# — an exception nobody explained is the thing this refuses. +filed(i) if { + some j, line in lines + j < i + i - j <= 30 + contains(line, "CLOUD-") +} + +# The kill threshold in seconds: `period x terminate-after`, read with string +# builtins alone. +# +# Undefined where either half is not readable — a period that is not `s`, +# or a multiplier that is not an integer — which is deliberate and is what the +# `nextest-slow-unbounded` arm below turns into a refusal, rather than letting +# `2m` or `500ms` leave the comparison unreachable and the gate green. +kill_seconds contains kill if { + some line in terminating + quoted := split(line, "\"") + count(quoted) > 1 + value := quoted[1] + endswith(value, "s") + not endswith(value, "ms") + period := to_number(trim_suffix(value, "s")) + + after := split(line, "terminate-after") + count(after) > 1 + assigned := split(after[1], "=") + count(assigned) > 1 + multiplier := to_number(trim_space(trim_suffix(trim_space(assigned[1]), "}"))) + + kill := period * multiplier +} + +# NO TERMINATING DECLARATION THIS GATE CAN READ. +# +# Absent, comment-only, missing `terminate-after`, or carrying a period in a unit +# this module cannot convert — one class, because every one of them means no ban +# is in force that can be vouched for. Refusing on an unreadable unit is the +# fail-closed direction: the alternative is a silently unreachable comparison. +violation contains { + "rule": "nextest-slow-unbounded", + "verdict": "suite bind missing", + "subjects": [{"path": config}], +} if { + count(kill_seconds) == 0 +} + +# THE RATCHET. A kill threshold above the committed ceiling is refused; below it +# is free, so making the suite faster never has to negotiate with this gate. +violation contains { + "rule": "nextest-slow-raised", + "verdict": "bound edit refused", + "subjects": [{"path": config}], +} if { + some kill in kill_seconds + kill > ceiling_seconds +} + +# AN EXCEPTION NOBODY EXPLAINED. A per-test override is how a legitimately slow +# case keeps the ban in force everywhere else — but an override with no filed row +# behind it is just the ban switched off for whichever test was inconvenient, and +# it is the thing that rots. +violation contains { + "rule": "nextest-slow-override-unfiled", + "verdict": "waiver file missing", + "subjects": [{"path": config}], +} if { + some i in override_at + not filed(i) +} + +deny contains finding if { + some finding in violation +} + +# --- the module's own tier --------------------------------------------------- +# +# These pin the PREDICATE. They cannot pin that the ENGINE builds the input the +# predicate reads — `with input as` fabricates the very shape the engine may be +# unable to produce — so `crates/batten/tests/it/nextest_slow.rs` runs the same +# questions over the compiled binary against the real committed file. Both tiers, +# per `rules/policy-modules.md`, and the second is not optional. + +tree(lines) := {"tree": {"lines": lines}} + +armed := ["[profile.default]", "slow-timeout = { period = \"10s\", terminate-after = 120 }"] + +report_only := ["[profile.default]", "slow-timeout = \"10s\""] + +raised := ["[profile.default]", "slow-timeout = { period = \"30s\", terminate-after = 60 }"] + +minutes := ["[profile.default]", "slow-timeout = { period = \"2m\", terminate-after = 3 }"] + +commented := ["[profile.default]", "# slow-timeout = { period = \"10s\", terminate-after = 30 }"] + +# THE PASS SIDE FIRST: without it every refusal below is satisfied by a module +# that refuses everything. +test_an_armed_declaration_at_the_ceiling_is_clean if { + count(violation) == 0 with input as tree({".config/nextest.toml": armed}) +} + +# LOWERING IS FREE, which is the ratchet's whole asymmetry. +test_a_kill_threshold_below_the_ceiling_is_clean if { + count(violation) == 0 with input as tree({".config/nextest.toml": [ + "[profile.default]", + "slow-timeout = { period = \"5s\", terminate-after = 4 }", + ]}) +} + +# `period` ALONE ONLY REPORTS. A declaration that marks a case slow and never +# kills it is not a ban, and must not read as one. +test_a_period_without_terminate_after_is_refused if { + count(violation) == 1 with input as tree({".config/nextest.toml": report_only}) +} + +# THE HOLE THE FIRST VERSION SHIPPED WITH. A small period and a large multiplier +# carries the kill past the ceiling while the period alone stays well inside it — +# `10 x 100` is 1000s. A gate bounding the period would pass this and ban nothing. +test_a_small_period_with_a_large_multiplier_is_refused if { + count(violation) == 1 with input as tree({".config/nextest.toml": [ + "[profile.default]", + "slow-timeout = { period = \"10s\", terminate-after = 200 }", + ]}) +} + +test_a_kill_threshold_above_the_ceiling_is_refused if { + count(violation) == 1 with input as tree({".config/nextest.toml": raised}) +} + +# A UNIT THIS MODULE CANNOT CONVERT REFUSES rather than leaving the comparison +# unreachable. `2m` is a legal nextest value and a silent hole without this. +test_a_period_in_an_unconvertible_unit_is_refused if { + count(violation) == 1 with input as tree({".config/nextest.toml": minutes}) +} + +# A COMMENTED-OUT DECLARATION IS NOT A DECLARATION. +test_a_commented_declaration_does_not_arm_the_ban if { + count(violation) == 1 with input as tree({".config/nextest.toml": commented}) +} + +# ABSENCE IS THE SAME CLASS: a tree with no runner config has no ban in force. +test_an_absent_config_is_refused if { + count(violation) == 1 with input as tree({}) +} + +# THE POINTER IS THE FILE a reader opens (rule 4), and the class is the one the +# registry declares for it. +test_the_refusal_points_at_the_runner_config if { + some v in violation with input as tree({".config/nextest.toml": raised}) + v.subjects[0].path == ".config/nextest.toml" + v.verdict == "bound edit refused" +} + +# --- the exception mechanism ------------------------------------------------- + +unfiled_override := [ + "[profile.default]", + "slow-timeout = { period = \"10s\", terminate-after = 30 }", + "", + "[[profile.default.overrides]]", + "filter = 'test(something_slow)'", + "slow-timeout = { period = \"10s\", terminate-after = 120 }", +] + +filed_override := [ + "[profile.default]", + "slow-timeout = { period = \"10s\", terminate-after = 30 }", + "", + "# CLOUD-1641 owns making this case fast; deleting this row is its acceptance.", + "[[profile.default.overrides]]", + "filter = 'test(something_slow)'", + "slow-timeout = { period = \"10s\", terminate-after = 120 }", +] + +# AN EXCEPTION NOBODY EXPLAINED. The override is above the ceiling and that is +# FINE — the ceiling does not reach an override, deliberately. What is refused is +# that no row was cited for it. +test_an_override_citing_no_row_is_refused if { + count(violation) == 1 with input as tree({".config/nextest.toml": unfiled_override}) +} + +# AND THE SAME OVERRIDE WITH A ROW IS CLEAN, which is what shows the refusal turns +# on the missing reason rather than on the override existing at all. Without this +# case the rule above is satisfied by a module that refuses every override. +test_an_override_that_cites_a_row_is_clean if { + count(violation) == 0 with input as tree({".config/nextest.toml": filed_override}) +} + +# THE CEILING DOES NOT REACH AN OVERRIDE, stated as its own case because the first +# version conflated the two and refused the committed config: it bounded every +# `slow-timeout` line by the default's number, which makes the sanctioned +# exception mechanism unusable — the shape that gets a gate switched off. +test_an_override_above_the_ceiling_is_not_a_raise if { + every v in violation { + v.rule != "nextest-slow-raised" + } with input as tree({".config/nextest.toml": filed_override}) +}