diff --git a/scripts/gate b/scripts/gate index 9eb3d3a..a351371 100755 --- a/scripts/gate +++ b/scripts/gate @@ -3,14 +3,15 @@ # directory, with isolated ambient roots and durable logs. # # scripts/gate [--acceptance SUITE]... [--protocol] [--print-plan] +# scripts/gate [--acceptance SUITE]... [--protocol] --print-plan-named # scripts/gate --print-target-dir # scripts/gate --init # scripts/gate --prune [--force] # scripts/gate --self-test # # Framing: docs/gate-script-framing.md (revision 4, approved), and -# docs/gate-protocol-build-framing.md (revision 3, approved) for the -# crdt build step and --self-test. +# docs/gate-protocol-build-framing.md (revision 4, approved) for the +# crdt build step, --self-test and --print-plan-named. # # WHY A PER-WORKTREE TARGET DIRECTORY. This machine exports one # CARGO_TARGET_DIR for every checkout, and cargo takes an EXCLUSIVE LOCK @@ -40,6 +41,7 @@ set -eu usage() { cat >&2 <<'EOF' usage: scripts/gate [--acceptance SUITE]... [--protocol] [--print-plan] + scripts/gate [--acceptance SUITE]... [--protocol] --print-plan-named scripts/gate --print-target-dir scripts/gate --init scripts/gate --prune [--force] @@ -54,7 +56,13 @@ usage: scripts/gate [--acceptance SUITE]... [--protocol] [--print-plan] --protocol the change touches PROTOCOL_VERSION; adds the CRDT workspace sweep on top of the default one, plus the build that sweep needs (see build-crdt below). - --print-plan print the exact gate commands and exit. + --print-plan print the exact gate commands and exit. Names are + stripped, so every line is runnable as printed. + --print-plan-named print the plan as `namecommand` lines and exit + --- the same text the runner reads. Exists because + a step's NAME is half its contract (a build failure + must be attributed to build-crdt, not sweep-crdt) + and --print-plan cannot show it. --print-target-dir print this worktree's build directory and exit. Creates nothing. --init create the build directory and ownership marker, @@ -407,6 +415,7 @@ while [ $# -gt 0 ]; do shift 2 ;; --protocol) PROTOCOL=1; shift ;; --print-plan) MODE=plan; shift ;; + --print-plan-named) MODE=plannamed; shift ;; --print-target-dir) MODE=printdir; shift ;; --init) MODE=init; shift ;; --prune) MODE=prune; shift ;; @@ -417,10 +426,54 @@ while [ $# -gt 0 ]; do esac done +# --------------------------------------------------------------------- +# Mode dispatch. +# +# TWO RENDERINGS OF ONE PLAN, and the second exists because the first +# hid a defect. `--print-plan` pipes through `cut -f2-` so every printed +# line is a command a reader can copy and run --- and that same cut is +# why the emitted NAMES never reached a test. Not cosmetic: the entire +# reason build-crdt is a separate step is that a build failure must be +# attributed to `build-crdt` rather than to `sweep-crdt`, and with the +# names stripped, RENAMING THE REAL BUILD STEP TO `sweep-crdt` left the +# ordering assertion green. The witness could not reach the step it +# named. --self-test could not either --- it hardcodes the string +# `build-crdt` in its own synthetic plan, which proves things about the +# RUNNER and nothing about this emitter. +# +# --print-plan-named prints emit_plan VERBATIM: the same +# `namecommand` text the runner reads back from PLAN_FILE, so a +# test can assert both halves of a real step together and a rename +# cannot pass. +# +# WHY THIS RATHER THAN THE TWO ALTERNATIVES. +# +# Injecting PLAN_FILE would let a test hand the runner a plan and read +# the names back, and it would turn the runner's `eval` into a general +# command executor --- the same class of defect this script's own +# review caught in --acceptance and fixed with a parse-time refusal. +# Declined there; declined here for the same reason. +# +# Re-deriving the plan test-side (sourcing this file, or parsing +# emit_plan out of it) would be a SECOND implementation of the thing +# under test, which is the exact failure being repaired one level up. +# +# A DISTINCT MODE, not a modifier on --print-plan: there is then no +# `--with-names` without `--print-plan` whose behaviour has to be +# defined, and --print-plan's contract --- runnable lines --- is left +# exactly as it was. Both modes call emit_plan, and so does the runner, +# so neither rendering can drift from what actually executes; +# tests/gate_script_acceptance.rs pins that the stripped rendering is +# the named one minus its names, so this stays true by test and not +# only by reading. +# --------------------------------------------------------------------- case $MODE in plan) emit_plan | cut -f2- exit 0 ;; + plannamed) + emit_plan + exit 0 ;; printdir) target_dir_for "$(canon "$(worktree_root)")" echo diff --git a/tests/gate_script_acceptance.rs b/tests/gate_script_acceptance.rs index a3da6ee..37de5f2 100644 --- a/tests/gate_script_acceptance.rs +++ b/tests/gate_script_acceptance.rs @@ -146,6 +146,13 @@ fn the_crdt_workspace_sweep_is_added_by_protocol_and_absent_without_it() { /// the right place under the right name and leave the gate exactly as /// unsound: the crdt sweep needs *those* features, and the wrong ones /// produce a binary the sweep cannot use. +/// +/// **What this test cannot see: the names.** `--print-plan` strips them +/// (`emit_plan | cut -f2-`), so everything below is an assertion about +/// *commands in an order* — renaming the real build step to `sweep-crdt` +/// leaves it green. The step's **name** is asserted by +/// `the_crdt_build_step_carries_its_own_name_and_its_exact_command` +/// below, which reads the plan in the form the runner reads it. #[test] fn the_crdt_sweep_is_immediately_preceded_by_the_build_that_produces_its_binary() { let root = tempfile::tempdir().expect("tempdir"); @@ -183,6 +190,139 @@ fn the_crdt_sweep_is_immediately_preceded_by_the_build_that_produces_its_binary( ); } +/// **The witness that reaches the step it names**, and the reason this +/// lane needed a second round. +/// +/// This lane exists to guarantee two things: that the crdt sweep is +/// preceded by the build producing its binary, and that a build failure +/// is attributed to **`build-crdt`** rather than to `sweep-crdt`. The +/// first round shipped with neither guaranteed, because **neither +/// witness could see a name**: +/// +/// - `--print-plan` renders `emit_plan | cut -f2-`, so the ordering test +/// above compares commands and never sees the names beside them. +/// - `--self-test` hardcodes the string `build-crdt` inside its **own +/// synthetic** plan, so it proves things about the *runner* and +/// nothing about the real emitter. +/// +/// Review demonstrated the consequence directly: **renaming the real +/// build step to `sweep-crdt` left both tests passing** — a plan that +/// reports a build failure under the sweep's name, which is exactly the +/// misattribution the separate step exists to prevent, sitting green. +/// +/// So the pair is asserted **together, as one emitted line**, against +/// `--print-plan-named` — the plan in the form the runner reads it back +/// from `PLAN_FILE`. Name and command in the same `assert`, from the +/// real emitter, is what makes a rename unable to pass; either half +/// alone lets the other drift. +/// +/// The mode is a *rendering*, not a seam: `PLAN_FILE` stays +/// uninjectable, because a test that supplied the runner's plan would +/// turn its `eval` into a general command executor — the defect the +/// `--acceptance` refusal below exists to prevent. +#[test] +fn the_crdt_build_step_carries_its_own_name_and_its_exact_command() { + let root = tempfile::tempdir().expect("tempdir"); + let build = "build-crdt\tcargo build --workspace --no-default-features --features luajit,crdt"; + let sweep = + "sweep-crdt\tcargo test --workspace --features crdt --no-fail-fast -- --skip basedpyright"; + + let (plan, err, ok) = run(root.path(), &["--protocol", "--print-plan-named"]); + assert!( + ok, + "--protocol --print-plan-named must succeed; stderr:\n{err}" + ); + let lines: Vec<&str> = plan.lines().collect(); + + // Whole-line equality, not `contains`: the name, the tab, and the + // command with nothing appended. A step is its (name, command) pair + // and the plan is where both are decided. + let b = lines.iter().position(|l| *l == build).unwrap_or_else(|| { + panic!( + "no plan line is exactly:\n {build}\nA build step under a \ + different NAME misattributes its own failure; a build step \ + with different FEATURES hands the sweep a binary it cannot \ + use. Plan was:\n{plan}" + ) + }); + // The sweep's own pair, for the same reason in the other direction: + // asserting only the build's name lets a rename of the SWEEP slip + // through the identical hole. + let s = lines + .iter() + .position(|l| *l == sweep) + .unwrap_or_else(|| panic!("no plan line is exactly:\n {sweep}\nPlan was:\n{plan}")); + + assert_eq!( + s, + b + 1, + "the build must be the step IMMEDIATELY before the crdt sweep — a \ + build merely somewhere earlier could be separated from it by a \ + step that rewrites the same target directory. Plan was:\n{plan}" + ); + + // Conditionality, on this rendering too: an ordinary lane must not + // carry the step at all, not merely not carry its command. + let (default_plan, _, ok) = run(root.path(), &["--print-plan-named"]); + assert!(ok, "--print-plan-named must succeed"); + assert!( + !default_plan.contains("build-crdt"), + "the default sweep never builds pmacs-gpu and never needs it, so no \ + ordinary lane may pay for a workspace build; plan was:\n{default_plan}" + ); +} + +/// **The new rendering must be the same plan, or the assertion above +/// pins a string only the test ever reads.** +/// +/// `--print-plan-named` and `--print-plan` both call one emitter, and +/// the runner writes that same emitter to `PLAN_FILE` — so today they +/// cannot disagree. This pins that from outside, where a later edit +/// giving either mode its own plan text would be caught rather than +/// producing a witness that asserts a name the runner never uses. +/// +/// It also pins the **shape** the runner depends on: the loop reads each +/// line with `IFS= read -r name cmd`, so a plan line without its +/// tab would silently run under an empty command. +#[test] +fn the_named_plan_is_the_printed_plan_with_its_names_removed() { + let root = tempfile::tempdir().expect("tempdir"); + + for flags in [ + vec![], + vec!["--protocol"], + vec!["--acceptance", "m4_acceptance"], + ] { + let mut named_args = flags.clone(); + named_args.push("--print-plan-named"); + let mut plain_args = flags.clone(); + plain_args.push("--print-plan"); + + let (named, err, ok_named) = run(root.path(), &named_args); + assert!(ok_named, "{named_args:?} must succeed; stderr:\n{err}"); + let (plain, err, ok_plain) = run(root.path(), &plain_args); + assert!(ok_plain, "{plain_args:?} must succeed; stderr:\n{err}"); + + let mut stripped = String::new(); + for l in named.lines() { + let (_name, cmd) = l.split_once('\t').unwrap_or_else(|| { + panic!( + "every plan line must be `namecommand` — the runner \ + splits on that tab, so a line without one runs an empty \ + command under the whole line's name. Line was:\n {l:?}" + ) + }); + stripped.push_str(cmd); + stripped.push('\n'); + } + + assert_eq!( + stripped, plain, + "the two renderings must be one plan; with {flags:?} they diverged" + ); + } +} + /// **Conditionality, settled by measurement rather than by reading** — /// which is the whole methodological point of this lane, since the /// defect it repairs was a precondition nobody checked.