diff --git a/.claude/skills/ci-gates/SKILL.md b/.claude/skills/ci-gates/SKILL.md index eac00bb74..a65e01464 100644 --- a/.claude/skills/ci-gates/SKILL.md +++ b/.claude/skills/ci-gates/SKILL.md @@ -11067,3 +11067,94 @@ rather than the case: `each_float_rule_decides_a_case_the_other_misses`. deletion answers. Mutate each clause of a compound guard **separately** and require a red for each; a clause whose removal leaves the suite green is either dead or untested, and those two are indistinguishable until you go looking. + +## 437. Five local previews of a gate, and not one asked its question + +`docs/now/` is read by five instruments before a push: `.githooks/pre-commit` +(via `scripts/tri check-now`), `scripts/pre-commit`, `scripts/verify.sh`, +`tri hooks now-gate` and `tri hooks pre-commit`. Every one of them checks +**freshness** -- an entry exists, dated inside the window. The *required* +`check` context checks **shape** -- `# NOW -- (YYYY-MM-DD)` as the +first line, the heading date matching the filename, a `## ` section, at least +one bullet that is not a placeholder. + +Same directory, same label, a different question. Measured by planting one +malformed entry dated today: + +| reader | verdict | what it actually read | +|---|---|---| +| `tools/check_now_entry_shape.py` (**required**) | **FAIL**, 3 complaints | the entry | +| `scripts/pre-commit` | PASS | that *some* entry is dated today | +| `tri hooks now-gate` | PASS | the same | +| `tri hooks pre-commit` | fail, **for L1** | the commit message | +| `scripts/verify.sh` | WARN, **for staleness** | the committed diff, not the file | + +The line that matters is the second. `scripts/pre-commit` went green +**because of** the malformed file: its freshness loop found the entry the gate +rejects, stopped looking, and reported health. A preview that the offending +artefact *satisfies* is worse than no preview -- it converts the defect into +evidence of correctness. + +This costs a full CI round every time, and it has now been paid at least three +times on this repository (#2991, #2994, and the pass that wrote this section). + +**The repair is not a sixth reader.** A local check that re-implements a gate +answers the question once and then drifts away from it -- and drift here is +invisible, because both sides stay green until the day they disagree. `tri now +check` shells out to the gate's own file, `tools/check_now_entry_shape.py +--check-files`, so the local answer *is* the gate's answer and there is nothing +to keep in sync. The script grew one flag; nobody grew a second opinion. + +Two properties the wrapper needs, and both are refusals: + +* **Unreachable is not green.** If the script is missing or `python3` is not on + PATH, it exits non-zero saying nothing was checked. Ask the OS whether the + interpreter exists -- `python3 -c ""` -- rather than matching an error + message, which is the tool's to reword. +* **Empty is not green either, and it is not red.** A change that adds no entry + has no shape to judge; the command says exactly that and points at + `tri hooks now-gate`, which asks the other question. Printing `OK` over an + empty set is the shape the gate itself was written to replace. + +**Generalisation.** When a local tool "previews" a gate, the thing to check is +not whether it is *strict enough* -- it is whether it reads the same **subject**. +Two populations under one label is a defect this page has recorded from four +other directions; a preview and its gate is the cheapest place to meet it, +because the preview is the one everybody trusts. + +## 438. A test that moves the process makes its siblings pass vacuously + +The suite for the above went green at 3 of 3, and one of the three was a lie. + +`every_shape_the_gate_names_reaches_the_same_verdict_from_here` plants six +entry shapes and asserts each gets the gate's own verdict. It opened with a +guard: `let root = match repo_root() { Ok(r) => r, Err(_) => return };`. A +sibling test in the same binary changed the process working directory to a +scratch tree to prove that a missing gate script is refused -- and +`std::env::set_current_dir` is **process-global**, so while it held, the first +test's `git rev-parse --show-toplevel` failed and the test took its silent +early return. + +That hid a second, independent defect: the planted filenames were +`zz-check-test-<date>-…`, which the gate's own filename rule rejects, so the +`well formed` case could never have passed. **Two defects, one green line**, and +neither is visible in the output -- a skipped test and a passing test print the +same nothing. + +Both repairs are structural rather than careful: + +* **No test moves the process.** The refusal was reachable without it: lift the + guard into `fn gate_script(root: &Path) -> Result<PathBuf>` and hand it a + scratch directory. A test needing `set_current_dir` is a function that should + have taken a path. +* **A guard that returns is a skip, and a silent skip is a pass.** `Err(_) => + return` reads as caution and behaves as a green. Tests here run inside the + repository; `expect("tests run inside the repository")` states that, and + fails loudly on the day it stops being true. + +Related, from the same hour: the probe that *found* the disagreement ran +`git add -A` and `git reset --hard HEAD~1` in a loop, which swept the +implementation under test into a probe commit and then deleted it. Recovered +from the reflog, whole. **A probe that mutates shared state is not an +observation** -- probe on a throwaway branch, stage explicit paths, and never +`-A` while the thing being measured is uncommitted. diff --git a/cli/tri/src/hooks.rs b/cli/tri/src/hooks.rs index 9854a0bc3..959faa61d 100644 --- a/cli/tri/src/hooks.rs +++ b/cli/tri/src/hooks.rs @@ -45,6 +45,12 @@ pub fn run(cmd: &HooksCmd) -> Result<()> { fn pre_commit() -> Result<()> { now_gate(None, None)?; + // Freshness and shape are two questions, and until this line only the + // first was asked here. Measured on one malformed entry dated today: the + // required `check` context reported three complaints while this hook, and + // three of the other four local readers, went green -- one of them green + // BECAUSE of that file, since its freshness loop found it and stopped. + crate::nownote::check_staged()?; l1_check()?; println!("tri hooks pre-commit: PASSED"); Ok(()) diff --git a/cli/tri/src/nownote.rs b/cli/tri/src/nownote.rs index 20038eb81..613dc05a6 100644 --- a/cli/tri/src/nownote.rs +++ b/cli/tri/src/nownote.rs @@ -15,7 +15,7 @@ use anyhow::{Context, Result}; use clap::Subcommand; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; #[derive(Subcommand)] pub enum NowCmd { @@ -48,6 +48,17 @@ pub enum NowCmd { #[arg(long, conflicts_with = "closes")] refs: Option<u64>, }, + /// Ask the REQUIRED `check` gate its own question, before pushing. + Check { + /// Judge exactly these paths. Overrides `--staged` and `--base`. + paths: Vec<PathBuf>, + /// Judge what the index adds -- what a pre-commit hook can see. + #[arg(long)] + staged: bool, + /// Judge what this branch adds against a revision. + #[arg(long, default_value = "origin/master")] + base: String, + }, } pub fn run(cmd: &NowCmd) -> Result<()> { @@ -58,6 +69,11 @@ pub fn run(cmd: &NowCmd) -> Result<()> { closes, refs, } => add(title, bullets, *closes, *refs), + NowCmd::Check { + paths, + staged, + base, + } => check(paths, *staged, base), } } @@ -239,3 +255,268 @@ mod tests { } } } + +// --------------------------------------------------------------------------- +// `tri now check` -- the blocking gate's question, asked locally. +// --------------------------------------------------------------------------- + +/// Judge the entries a change adds, using the gate's own implementation. +/// +/// **Why this delegates instead of deciding.** Five local instruments already +/// read `docs/now/`: `.githooks/pre-commit` (via `scripts/tri check-now`), +/// `scripts/pre-commit`, `scripts/verify.sh`, `tri hooks now-gate` and +/// `tri hooks pre-commit`. Every one of them checks FRESHNESS -- an entry +/// exists, dated inside the window -- and the required `check` context checks +/// SHAPE. Same directory, same label, a different question, and measured on +/// one malformed entry dated today: the gate reported three complaints while +/// three of the five local instruments went green. `scripts/pre-commit` went +/// green **because of** that file: its freshness loop found the entry the gate +/// rejects and stopped looking. +/// +/// A sixth reader written in Rust would answer the question and then drift +/// away from it. This one shells out to `tools/check_now_entry_shape.py +/// --check-files`, so the local answer IS the gate's answer and disagreement +/// is not something to test for. +fn check(paths: &[PathBuf], staged: bool, base: &str) -> Result<()> { + let root = repo_root()?; + let script = gate_script(&root)?; + // Ask the OS whether the interpreter exists, not an error message: a + // message is the tool's to reword and PATH is not. + if std::process::Command::new("python3") + .arg("-c") + .arg("") + .output() + .is_err() + { + anyhow::bail!( + "python3 is not on PATH, so the gate could not be run and nothing was \ + checked. Reporting that rather than a pass this run did not earn." + ); + } + + let files: Vec<String> = if !paths.is_empty() { + paths.iter().map(|p| p.display().to_string()).collect() + } else if staged { + git_paths( + &root, + &["diff", "--cached", "--name-only", "--diff-filter=A"], + None, + )? + } else { + git_paths( + &root, + &["diff", "--name-only", "--diff-filter=A"], + Some(&format!("{base}...HEAD")), + )? + }; + + if files.is_empty() { + println!( + "tri now check: this change adds no docs/now/ entry, so no SHAPE was \ + checked.\n Whether one is REQUIRED is a different question and \ + `tri hooks now-gate` asks it." + ); + return Ok(()); + } + + let status = std::process::Command::new("python3") + .arg(&script) + .arg("--check-files") + .args(&files) + .current_dir(&root) + .status() + .context("failed to run tools/check_now_entry_shape.py")?; + if !status.success() { + anyhow::bail!( + "the required `check` context would refuse this change. \ + The complaints above are the gate's own words." + ); + } + Ok(()) +} + +/// The gate's own file, or a refusal naming what could not be checked. +/// +/// Separate from `check` so the refusal can be exercised without changing the +/// process working directory: a test that does that races every other test in +/// the binary, which is a collision this repository has already paid for once. +fn gate_script(root: &Path) -> Result<PathBuf> { + let script = root.join("tools/check_now_entry_shape.py"); + if !script.is_file() { + anyhow::bail!( + "{} is missing, so nothing was checked. This command is the gate's own \ + implementation reached from here; without the file it has no answer to \ + give, and printing a pass would be the failure it exists to prevent.", + script.display() + ); + } + Ok(script) +} + +/// The shape of what the index adds -- the pre-commit hook's entry point. +pub fn check_staged() -> Result<()> { + check(&[], true, "origin/master") +} + +/// Added `docs/now/*.md` paths from one git invocation, README excluded. +fn git_paths(root: &Path, args: &[&str], range: Option<&str>) -> Result<Vec<String>> { + let mut cmd = std::process::Command::new("git"); + cmd.current_dir(root).args(args); + if let Some(r) = range { + cmd.arg(r); + } + cmd.args(["--", "docs/now/"]); + let out = cmd.output().context("failed to invoke git diff")?; + if !out.status.success() { + anyhow::bail!( + "git {:?} failed: {}. The range is wrong, not the tree -- and a wrong \ + range prints an empty list, which reads as \"nothing to check\".", + args, + String::from_utf8_lossy(&out.stderr).trim() + ); + } + Ok(entry_paths(&String::from_utf8_lossy(&out.stdout))) +} + +/// The entry paths in one `git diff --name-only` listing. +/// +/// Split from the git call so the filter can be mutated and seen to fail: +/// while it lived inside `git_paths` no test reached it, and a clause no test +/// reaches is indistinguishable from a dead one. The rule mirrors the gate's +/// own `added_now_entries` -- `README.md` is documentation about the +/// directory, not an entry in it, and a non-`.md` file is not one either. +fn entry_paths(stdout: &str) -> Vec<String> { + stdout + .lines() + .map(str::trim) + .filter(|l| l.ends_with(".md") && !l.ends_with("README.md")) + .map(str::to_string) + .collect() +} + +#[cfg(test)] +mod check_tests { + use super::*; + + /// The six shapes `tools/check_now_entry_shape.py --self-check` names, + /// with the verdict that file itself states for each. + /// + /// They are transcribed here for a reason the pair makes sharp: this + /// command DELEGATES, so any test comparing its answer to the gate's + /// compares the gate to itself and cannot fail. What can fail is the + /// wiring -- a wrong flag, a wrong working directory, a range that prints + /// an empty list -- and that is what these exercise, end to end, through + /// the real script. + fn shapes() -> Vec<(&'static str, &'static str, &'static str, bool)> { + let good = "# NOW -- A real entry about a real thing (2026-08-28)\n\n\ + ## A real entry about a real thing (Refs #1)\n\n\ + - something specific happened and here is what it was\n"; + vec![ + // (label, the date the FILENAME carries, body, gate accepts it) + ("well formed", "2026-08-28", good, true), + ("heading date disagrees", "2026-08-27", good, false), + ( + "no bullets", + "2026-08-28", + "# NOW -- Title (2026-08-28)\n\n## Title\n", + false, + ), + ( + "placeholder bullets", + "2026-08-28", + "# NOW -- Title (2026-08-28)\n\n## Title\n\n- TBD\n- ...\n", + false, + ), + ( + "wrong first line", + "2026-08-28", + "# Some other heading\n\n## Title\n\n- a bullet that is long enough\n", + false, + ), + ("empty file", "2026-08-28", "", false), + ] + } + + static NEXT: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + + fn scratch(tag: &str) -> PathBuf { + let d = std::env::temp_dir().join(format!( + "now_check_{}_{}_{}", + tag, + std::process::id(), + NEXT.fetch_add(1, std::sync::atomic::Ordering::SeqCst) + )); + std::fs::create_dir_all(&d).unwrap(); + d + } + + /// The planted filename must be one the gate ACCEPTS on its own, or every + /// case fails for the same unrelated reason and the table says nothing. + /// + /// The first version named the files `zz-check-test-<date>-...`, which the + /// filename rule rejects, so `well formed` could never pass -- and the run + /// was GREEN, because a sibling test changed the process working directory + /// out of the repository and this one took its silent early return. Two + /// defects, and the cwd one was mine: the guard against it is that no test + /// here moves the process. + #[test] + fn every_shape_the_gate_names_reaches_the_same_verdict_from_here() { + let root = repo_root().expect("tests run inside the repository"); + assert!(gate_script(&root).is_ok()); + let dir = root.join("docs/now"); + for (i, (label, date, text, want_ok)) in shapes().into_iter().enumerate() { + let name = format!("{date}-zzcheck-{}-{i}.md", std::process::id()); + std::fs::write(dir.join(&name), text).unwrap(); + let got = check( + &[PathBuf::from(format!("docs/now/{name}"))], + false, + "origin/master", + ) + .is_ok(); + let _ = std::fs::remove_file(dir.join(&name)); + assert_eq!( + got, want_ok, + "{label}: the gate says {want_ok} and this command said {got}" + ); + } + } + + #[test] + fn the_directorys_readme_is_not_an_entry_in_it() { + let listing = "docs/now/README.md\n\ + docs/now/2026-01-01-a-real-entry.md\n\ + docs/now/notes.txt\n"; + assert_eq!( + entry_paths(listing), + vec!["docs/now/2026-01-01-a-real-entry.md".to_string()] + ); + } + + #[test] + fn an_empty_set_says_nothing_was_checked_rather_than_passing_quietly() { + // Reached through the git path with a range that adds nothing. + let root = repo_root().expect("tests run inside the repository"); + assert!(gate_script(&root).is_ok()); + assert!(check(&[], false, "HEAD").is_ok()); + } + + #[test] + fn a_missing_gate_script_refuses_instead_of_passing() { + // This command's whole value is that it IS the gate, so being unable + // to reach the gate must not print a pass. + let dir = scratch("noscript"); + let r = gate_script(&dir); + let _ = std::fs::remove_dir_all(&dir); + assert!( + r.is_err(), + "a tree with no gate script must refuse, not pass" + ); + } + + #[test] + fn the_script_is_found_where_it_lives() { + // The other half: a refusal that fires everywhere is not a check. + let root = repo_root().expect("tests run inside the repository"); + assert!(gate_script(&root).is_ok()); + } +} diff --git a/docs/now/2026-09-03-the-local-gates-read-freshness-the-blocking-one-reads-shape.md b/docs/now/2026-09-03-the-local-gates-read-freshness-the-blocking-one-reads-shape.md new file mode 100644 index 000000000..938041cfb --- /dev/null +++ b/docs/now/2026-09-03-the-local-gates-read-freshness-the-blocking-one-reads-shape.md @@ -0,0 +1,7 @@ +# NOW -- The local gates read freshness; the blocking one reads shape (2026-09-03) + +## tri now check (Refs #2994) + +- Five local instruments read `docs/now/` -- `.githooks/pre-commit` via `scripts/tri check-now`, `scripts/pre-commit`, `scripts/verify.sh`, `tri hooks now-gate` and `tri hooks pre-commit` -- and every one checks FRESHNESS while the required `check` context checks SHAPE. +- Measured on one malformed entry dated today: the gate reported **three** complaints and three of the five local readers went green. `scripts/pre-commit` went green **because of** that file -- its freshness loop found the entry the gate rejects and stopped looking. +- `tri now check` asks the gate its own question locally by **delegating** to `tools/check_now_entry_shape.py --check-files`, so the local answer is the gate's answer and drift is impossible rather than tested for. Wired into `tri hooks pre-commit` and into `scripts/verify.sh`'s gate preview. diff --git a/scripts/verify.sh b/scripts/verify.sh index 71525540f..e70eb472d 100755 --- a/scripts/verify.sh +++ b/scripts/verify.sh @@ -180,6 +180,26 @@ else NOW_DATE="now-date:STALE (${LAST:-none})" GATE_ISSUES="${GATE_ISSUES} now-entry-date-stale" fi + # (b2) SHAPE of the added entries, asked of the gate itself. + # (a) and (b) are FRESHNESS -- an entry exists, dated in the + # window. The required `check` context reads SHAPE, and this file + # claimed to preview "the same three conditions" while never + # opening an entry. Measured on one malformed entry dated today: + # the gate had three complaints and this preview said nothing. + # Delegated rather than reimplemented, so the preview cannot drift + # away from the gate it previews. + NOW_SHAPE="now-shape:not-run" + if [ -n "$ADDED_NOW" ] && command -v python3 >/dev/null 2>&1 \ + && [ -f tools/check_now_entry_shape.py ]; then + if python3 tools/check_now_entry_shape.py --check-files $ADDED_NOW >/dev/null 2>&1; then + NOW_SHAPE="now-shape:ok" + else + NOW_SHAPE="now-shape:MALFORMED" + GATE_ISSUES="${GATE_ISSUES} now-entry-malformed" + fi + elif [ -z "$ADDED_NOW" ]; then + NOW_SHAPE="now-shape:nothing-added" + fi # (c) Added lines in the diff vs base are ASCII-only (L3 PURITY preview). NONASCII="$(git diff "$BASE_REF"...HEAD 2>/dev/null | grep -n '^+' | grep -P '[^\x00-\x7F]' | head -5 || true)" if [ -z "$NONASCII" ]; then @@ -189,12 +209,12 @@ else GATE_ISSUES="${GATE_ISSUES} non-ascii-added-lines" fi if [ -z "$GATE_ISSUES" ]; then - GATES_VERDICT="gates:OK ($NOW_IN_DIFF, $NOW_DATE, $ASCII)" - log " [4/5] gate-preview-> OK ($NOW_IN_DIFF | $NOW_DATE | $ASCII) [base $BASE_REF]" + GATES_VERDICT="gates:OK ($NOW_IN_DIFF, $NOW_DATE, $NOW_SHAPE, $ASCII)" + log " [4/5] gate-preview-> OK ($NOW_IN_DIFF | $NOW_DATE | $NOW_SHAPE | $ASCII) [base $BASE_REF]" else - GATES_VERDICT="gates:WARN ($NOW_IN_DIFF, $NOW_DATE, $ASCII) -- advisory" + GATES_VERDICT="gates:WARN ($NOW_IN_DIFF, $NOW_DATE, $NOW_SHAPE, $ASCII) -- advisory" log " [4/5] gate-preview-> WARN [base $BASE_REF]:" - log " $NOW_IN_DIFF | $NOW_DATE | $ASCII" + log " $NOW_IN_DIFF | $NOW_DATE | $NOW_SHAPE | $ASCII" log " likely CI-gate issue(s):$GATE_ISSUES (advisory; fix before push)" if [ -n "$NONASCII" ]; then log " first non-ASCII added line(s):" diff --git a/tools/check_now_entry_shape.py b/tools/check_now_entry_shape.py index 33713f2ab..022a6e6d3 100644 --- a/tools/check_now_entry_shape.py +++ b/tools/check_now_entry_shape.py @@ -164,6 +164,17 @@ def main(): if "--self-check" in sys.argv: return self_check() + # `--check-files <path>...`: judge exactly these paths, no git range. + # An empty list is refused rather than passed -- a caller that computed no + # files and got a green back would read it as "the entries are fine". + if "--check-files" in sys.argv: + paths = sys.argv[sys.argv.index("--check-files") + 1:] + if not paths: + print("FAIL: --check-files was given no paths. Nothing was checked,") + print(" and a pass over an empty set is the shape this file replaces.") + return 1 + return report(paths) + base = os.environ.get("PR_BASE_SHA", "") head = os.environ.get("PR_HEAD_SHA", "HEAD") @@ -199,6 +210,19 @@ def main(): print(" refuses rather than passing over an empty set.") return 1 + return report(entries) + + +def report(entries): + """Read and judge each entry. The ONE body both callers share. + + `--check-files` exists so a contributor can ask this gate its own question + before pushing. It must not be a second implementation of the answer: five + local instruments already check `docs/now/` and every one of them reads + FRESHNESS -- that an entry exists, dated inside the window -- while the + required `check` context reads SHAPE. Same directory, same label, different + question, and the local ones went green on an entry this gate rejects. + """ bad = 0 for path in entries: full = ROOT / path