fix(gate): review round 4 --- four discriminators that constrained nothing

Every item here is the same failure in a different place: a property
fixed or claimed, and a suite that would stay green without it.

**CANONICAL ANCESTRY WAS UNWITNESSED.** Round 3 fixed word-splitting
and lexical traversal and verified both BY HAND, which is exactly the
evidence that does not survive the next edit --- reverting to the
`for _anc in $(... dirname ...)` loop left all 26 rows green. There is
now a row driving a **space-containing root** (an unquoted expansion
tears it into fragments and the real ancestor is never tested, so the
guard passes on the path it must reject) and a **symlinked root** (a
marker visible only after `pwd -P`, which is what `detect_project`
sees). `M-G-8` restores the old loop and fails it. The traversal is
encoded in framing §2a rather than left as an implementation detail.

**THE SOCKET GUARD MISSED ITS OWN BOUNDARY.** The rows generated ~51-
and ~71-byte paths against a 55-byte cutoff, so they constrained the
guard's EXISTENCE and not its VALUE: raising the budget from 103 to 118
would have kept both green. They now hit **exactly 55 accepted and 56
rejected**, assert the measured byte lengths, and check that the
refusal reports precisely one byte over.

**REJECTION-CLEANUP CHECKED ONE AREA OF TWO.** Only `<root>/tmp` was
inspected, so leaking AMBIENT alone would have passed --- and AMBIENT
is created before the guard can measure anything, which is the whole
reason the trap moved earlier. Both areas are inspected now, the
ambient one under the derived per-worktree target whose hashed name the
test does not compute.

**THE MULTIBYTE ROW DEPENDED ON THE INHERITED LOCALE.** Under
`LC_ALL=C`, `${#var}` already counts bytes, so the character-counting
mutant passed and the row's verdict was a property of the environment
rather than of the code. It sets `LC_ALL=C.UTF-8` explicitly;
`M-G-6` now fails even when the harness itself runs under `LC_ALL=C`.

Stale test prose corrected, including one claim that was the very
inference the ancestor check exists to refute: **placement under a
managed root does not make a path marker-free**. Also, the tmp parent
is SHARED between worktrees and untouched by `--prune` (unlike the
ambient root, which is per-worktree), and the module header named
framing revision 4.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
This commit is contained in:
Levi Neuwirth 2026-08-13 19:33:51 +02:00
parent 0a04d55a35
commit 8e1f03a4a0
No known key found for this signature in database
2 changed files with 218 additions and 64 deletions

View File

@ -477,6 +477,17 @@ Four decisions inside that, each of which had a cheaper wrong answer:
the gate owns is *necessary, not sufficient*, so the precondition the gate owns is *necessary, not sufficient*, so the precondition
is verified rather than assumed. is verified rather than assumed.
**THE TRAVERSAL IS CANONICAL AND MUST NOT WORD-SPLIT**, and both
halves are contract rather than style. An unquoted `$(...)`
expansion splits on `IFS`, so a gate root containing a **space**
is torn into fragments and its real ancestor is never tested —
the guard then passes on exactly the path it exists to reject.
And `dirname` walks **lexical** ancestry while `detect_project`
canonicalizes, so a **symlinked** root hides a marker the editor
plainly sees; the gate and the editor must not disagree about the
same tree. The walk resolves with `pwd -P` first and iterates a
quoted loop, and both shapes are witnessed.
**MIRRORING THE NAMES IS NOT ENOUGH — the TYPES are part of the **MIRRORING THE NAMES IS NOT ENOUGH — the TYPES are part of the
contract.** `match_marker` (`src/project.rs`) requires `.git` to be contract.** `match_marker` (`src/project.rs`) requires `.git` to be
a **directory** and the seven language markers to be **files**, so a **directory** and the seven language markers to be **files**, so

View File

@ -1,6 +1,7 @@
//! `scripts/gate` — the behaviour a shell script can be held to. //! `scripts/gate` — the behaviour a shell script can be held to.
//! //!
//! Framing: `docs/gate-script-framing.md` §4 (revision 4, approved). //! Framing: `docs/gate-script-framing.md` §4, and §2a for the
//! `TMPDIR` isolation these rows cover (revision 6).
//! //!
//! # Why these tests exist, and why they are shaped like this //! # Why these tests exist, and why they are shaped like this
//! //!
@ -513,9 +514,16 @@ fn the_isolated_tmpdir_reaches_a_spawned_child_under_the_managed_root() {
}) })
.unwrap_or_else(|| panic!("the gate must announce its TMPDIR; stdout:\n{out}")); .unwrap_or_else(|| panic!("the gate must announce its TMPDIR; stdout:\n{out}"));
// The contract is **under the managed target root**, which is what // The contract is **under the managed gate root**, which is what
// makes it both marker-free and disk-backed in production: the // makes it disk-backed in production: the gate root sits beside the
// target root sits beside the build artifacts, not in `/tmp`. // build artifacts, not in `/tmp`.
//
// **Placement does NOT make it marker-free** — that is the false
// inference the ancestor check exists to correct, and an earlier
// version of this comment made it. A `.git` in `$HOME` or above
// `$HOME/build` re-roots fixtures just as `/tmp/.git` did.
// Marker-freeness is a separate, verified precondition; see
// `the_ancestor_check_honours_marker_types`.
// //
// Deliberately NOT asserting `!starts_with("/tmp/")` here. This // Deliberately NOT asserting `!starts_with("/tmp/")` here. This
// test's own root is a `tempfile::tempdir()`, so on a normal // test's own root is a `tempfile::tempdir()`, so on a normal
@ -587,7 +595,10 @@ fn the_isolated_tmpdir_is_reaped_when_the_run_ends() {
"the exit trap must remove the TMPDIR even when a gate failed; \ "the exit trap must remove the TMPDIR even when a gate failed; \
{announced} survived" {announced} survived"
); );
// The parent stays: it is the per-worktree home the next run uses. // The parent stays. It is SHARED between worktrees — it hangs off
// the gate root, not the derived per-worktree target, for the
// socket budget — and `--prune` never touches it, because prune
// only considers directories carrying an ownership marker.
assert!( assert!(
Path::new(&announced) Path::new(&announced)
.parent() .parent()
@ -722,93 +733,140 @@ fn the_ancestor_check_honours_marker_types() {
); );
} }
/// The socket-path guard: rejection, acceptance, and cleanup on /// The socket-path guard, **at the boundary**: 55 bytes accepted, 56
/// rejection. /// rejected, with the measured lengths asserted.
/// ///
/// **The ordinary gate only ever exercises the passing side.** Every /// **Straddling the cutoff is not testing it.** An earlier version of
/// other row here runs with a short root, so the guard is silent and a /// this row generated roughly 51- and 71-byte paths against a 55-byte
/// broken guard would look identical. These construct the boundary /// cutoff; raising `SUN_LEN_BUDGET` from 103 to 118 would have left
/// deliberately. /// both green, so the row constrained the guard's existence and not its
/// value. These aim at 55 and 56 exactly and assert the byte lengths
/// they achieved, so a drifting budget fails here rather than in a
/// socket bind.
/// ///
/// The budget is **103 usable bytes** — Darwin's `sun_path[104]` minus /// The budget is **103 usable bytes** — Darwin's `sun_path[104]` minus
/// its terminating NUL, the supported-platform floor — less a 48-byte /// its terminating NUL, the supported-platform floor — less the 48-byte
/// fixture reserve, so a root whose derived TMPDIR exceeds 55 bytes is /// fixture reserve.
/// refused.
#[test] #[test]
fn the_socket_path_guard_rejects_accepts_and_cleans_up() { fn the_socket_path_guard_holds_at_its_exact_boundary() {
// The gate appends `/tmp/XXXXXX` (11 bytes) to the root, so a root // The gate derives `<root>/tmp/XXXXXX`, i.e. root + 11 bytes.
// of N bytes yields a TMPDIR of N + 11. const DERIVED: usize = 11;
let over_root = long_root(60); const CUTOFF: usize = 103 - 48;
let ok_root = root_of_len(CUTOFF - DERIVED);
let out = run_guarded(ok_root.path());
let stdout = String::from_utf8_lossy(&out.stdout).into_owned();
let announced = stdout
.lines()
.find_map(|l| {
l.split_once("gate: tmpdir")
.map(|(_, p)| p.trim().to_owned())
})
.unwrap_or_else(|| {
panic!(
"a root at the cutoff must be ACCEPTED and announce its \
TMPDIR; stdout:\n{stdout}stderr:\n{}",
String::from_utf8_lossy(&out.stderr)
)
});
assert_eq!(
announced.len(),
CUTOFF,
"the accepted fixture must sit exactly ON the cutoff, not below \
it — otherwise a widened budget still passes. Path: {announced}"
);
let over_root = root_of_len(CUTOFF - DERIVED + 1);
let out = run_guarded(over_root.path()); let out = run_guarded(over_root.path());
let err = String::from_utf8_lossy(&out.stderr).into_owned(); let err = String::from_utf8_lossy(&out.stderr).into_owned();
assert!( assert!(
!out.status.success(), !out.status.success() && err.contains("too long for a unix socket path"),
"a root past the budget must be refused; stderr:\n{err}" "one byte past the cutoff must be REJECTED; stderr:\n{err}"
); );
assert!( // Keyed on the line that reports the measurement, not on the first
err.contains("too long for a unix socket path"), // `gate: ` line — that one is the path.
"and must say why; stderr:\n{err}" let measured = err
.lines()
.find(|l| l.contains("bytes, but a fixture needs"))
.and_then(|l| l.split_whitespace().nth(1))
.and_then(|s| s.parse::<usize>().ok());
assert_eq!(
measured,
Some(CUTOFF + 1),
"and the refusal must report exactly one byte over; stderr:\n{err}"
); );
// REJECTION MUST NOT LEAK. The guard creates both temporary areas // REJECTION MUST NOT LEAK, and **both** areas are checked. The
// before it can measure anything, so a rejection that exits before // guard creates the ambient root and the TMPDIR before it can
// the trap is armed leaves them behind — on every rejection, which // measure anything, so a rejection that exits before the trap is
// is worse than the failure it prevents. // armed leaves both behind. Checking only `<root>/tmp` would pass
let leaked: Vec<_> = walkdir_shallow(over_root.path()); // while `gate-ambient` leaked.
let leaked = leaked_temp_areas(over_root.path());
assert!( assert!(
leaked.is_empty(), leaked.is_empty(),
"a rejected run must reap what it created; found {leaked:?}" "a rejected run must reap BOTH created areas; found {leaked:?}"
);
// Just inside the budget: accepted.
let ok_root = long_root(40);
let out = run_guarded(ok_root.path());
let err = String::from_utf8_lossy(&out.stderr).into_owned();
assert!(
!err.contains("too long for a unix socket path"),
"a root inside the budget must not be refused; stderr:\n{err}"
); );
} }
/// The guard counts BYTES, not characters. /// The guard counts BYTES, not characters — **under a UTF-8 locale**.
/// ///
/// `${#var}` counts characters under a UTF-8 locale while `sun_path` is /// `${#var}` counts characters in a UTF-8 locale while `sun_path` is
/// byte-limited, so a multibyte path measures short and passes a check /// byte-limited, so a multibyte path measures short and passes a check
/// it should fail. Each `é` here is one character and **two bytes**. /// it should fail. **The locale is set explicitly**: under an inherited
/// `LC_ALL=C`, `${#var}` already counts bytes and the character-counting
/// mutant would pass, making this row's verdict depend on the
/// environment rather than on the code.
#[test] #[test]
fn the_socket_path_guard_counts_bytes_not_characters() { fn the_socket_path_guard_counts_bytes_not_characters() {
// Character-length ~34 but byte-length ~68: rejected only if the // Each `é` is one character and two bytes, so this root is under
// guard measures bytes. // the cutoff by character count and over it by byte count — the
// only shape that separates the two implementations.
let root = tempfile::Builder::new() let root = tempfile::Builder::new()
.prefix(&"é".repeat(24)) .prefix(&"é".repeat(22))
.tempdir_in(short_root_base()) .tempdir_in(short_root_base())
.expect("multibyte root"); .expect("multibyte root");
let chars = root.path().to_string_lossy().chars().count(); let path = root.path().to_string_lossy().into_owned();
let bytes = root.path().to_string_lossy().len(); let chars = path.chars().count();
let bytes = path.len();
assert!( assert!(
bytes > chars, chars + 11 <= 55 && bytes + 11 > 55,
"fixture must actually be multibyte: {chars} chars, {bytes} bytes" "fixture must straddle: {chars} chars (must pass) vs {bytes} \
bytes (must fail), path {path}"
); );
let out = run_guarded(root.path()); let out = std::process::Command::new(gate())
.arg("--self-test")
.current_dir(repo_root())
.env("PMACS_GATE_TARGET_ROOT", root.path())
.env("PMACS_GATE_ALLOW_ANCESTOR_MARKER", "1")
.env("LC_ALL", "C.UTF-8")
.env("LANG", "C.UTF-8")
.env_remove("TMPDIR")
.output()
.expect("run gate");
let err = String::from_utf8_lossy(&out.stderr).into_owned(); let err = String::from_utf8_lossy(&out.stderr).into_owned();
assert!( assert!(
!out.status.success() && err.contains("too long for a unix socket path"), !out.status.success() && err.contains("too long for a unix socket path"),
"a multibyte root over the BYTE budget must be refused — it is \ "a root over the BYTE budget must be refused even though it is \
{chars} characters but {bytes} bytes; stderr:\n{err}" under the CHARACTER budget ({chars} chars, {bytes} bytes); \
stderr:\n{err}"
); );
} }
/// A root of exactly `total` bytes, so the boundary can be aimed at. /// A root whose full path is exactly `total` bytes.
fn long_root(total: usize) -> tempfile::TempDir { fn root_of_len(total: usize) -> tempfile::TempDir {
let base = short_root_base(); let base = short_root_base();
// `<base>/<prefix>XXXXXX` let fixed = base.display().to_string().len() + 1 + 6; // `<base>/` + XXXXXX
let fixed = base.display().to_string().len() + 1 + 6; let dir = tempfile::Builder::new()
let pad = total.saturating_sub(fixed); .prefix(&"a".repeat(total.saturating_sub(fixed)))
tempfile::Builder::new()
.prefix(&"a".repeat(pad))
.tempdir_in(&base) .tempdir_in(&base)
.expect("sized root") .expect("sized root");
assert_eq!(
dir.path().to_string_lossy().len(),
total,
"fixture must be exactly {total} bytes"
);
dir
} }
/// Run the gate with the ancestor escape set (so only the LENGTH guard /// Run the gate with the ancestor escape set (so only the LENGTH guard
@ -824,17 +882,102 @@ fn run_guarded(root: &Path) -> std::process::Output {
.expect("run gate") .expect("run gate")
} }
/// Entries left under `root/tmp` and any `gate-ambient` leaf. /// Leftovers in **both** areas a run creates: `<root>/tmp/*` and the
fn walkdir_shallow(root: &Path) -> Vec<PathBuf> { /// derived target's `gate-ambient/*`.
fn leaked_temp_areas(root: &Path) -> Vec<PathBuf> {
let mut out = Vec::new(); let mut out = Vec::new();
for sub in ["tmp"] { if let Ok(rd) = std::fs::read_dir(root.join("tmp")) {
if let Ok(rd) = std::fs::read_dir(root.join(sub)) { out.extend(rd.filter_map(Result::ok).map(|e| e.path()));
out.extend(rd.filter_map(Result::ok).map(|e| e.path())); }
// The ambient root DOES live under the derived per-worktree target
// (unlike the tmp parent, which is shared), and its name is a hash
// this test does not compute — so every `gate-ambient` beneath the
// root is inspected.
if let Ok(rd) = std::fs::read_dir(root) {
for entry in rd.filter_map(Result::ok) {
if let Ok(inner) = std::fs::read_dir(entry.path().join("gate-ambient")) {
out.extend(inner.filter_map(Result::ok).map(|e| e.path()));
}
} }
} }
out out
} }
/// The ancestor walk is **canonical** and **does not word-split**.
///
/// Both properties were fixed without a witness, and reverting to the
/// obvious `for _anc in $(... dirname ...)` loop left every other row
/// green — so the suite constrained the check's existence and neither
/// of its two hard-won properties.
///
/// * **A space in the root** is torn into fragments by an unquoted
/// `$(...)` expansion, and the real ancestor is then never tested —
/// the guard passes on exactly the path it must reject.
/// * **A symlinked root** hides a marker under lexical `dirname` that
/// `detect_project` sees after canonicalization, so the gate and the
/// editor would disagree about the same tree.
#[test]
fn the_ancestor_walk_is_canonical_and_does_not_word_split() {
// A space in the path, with a marker above it.
let spaced = tempfile::Builder::new()
.prefix("has space ")
.tempdir_in(short_root_base())
.expect("spaced base");
assert!(
spaced.path().to_string_lossy().contains(' '),
"fixture must actually contain a space"
);
let marker = spaced.path().join(".git");
std::fs::create_dir(&marker).expect("marker");
let root = spaced.path().join("inner");
std::fs::create_dir_all(&root).expect("root");
let out = run_unescaped(&root);
let err = String::from_utf8_lossy(&out.stderr).into_owned();
assert!(
err.contains(&format!("{}", marker.display())),
"a marker above a root containing a SPACE must be found and \
named; stderr:\n{err}"
);
// A symlinked root whose marker is only visible after resolving.
let base = tempfile::Builder::new()
.prefix("sym-")
.tempdir_in(short_root_base())
.expect("sym base");
let real = base.path().join("real");
std::fs::create_dir_all(real.join("inner")).expect("real tree");
let hidden = real.join(".git");
std::fs::create_dir(&hidden).expect("hidden marker");
let link = base.path().join("link");
std::os::unix::fs::symlink(&real, &link).expect("symlink");
let out = run_unescaped(&link.join("inner"));
let err = String::from_utf8_lossy(&out.stderr).into_owned();
assert!(
!out.status.success(),
"a marker reachable only after canonicalization must still \
refuse; stderr:\n{err}"
);
assert!(
err.contains(&format!("{}", hidden.display())),
"and must name it at its RESOLVED path, which is what \
`detect_project` would see; stderr:\n{err}"
);
}
/// Run the gate with the ancestor check ACTIVE (no escape).
fn run_unescaped(root: &Path) -> std::process::Output {
std::process::Command::new(gate())
.arg("--self-test")
.current_dir(repo_root())
.env("PMACS_GATE_TARGET_ROOT", root)
.env_remove("PMACS_GATE_ALLOW_ANCESTOR_MARKER")
.env_remove("TMPDIR")
.output()
.expect("run gate")
}
/// The seam handoff §3 keeps authority over: a script cannot infer /// The seam handoff §3 keeps authority over: a script cannot infer
/// which acceptance suites a change touched, so it runs what it is /// which acceptance suites a change touched, so it runs what it is
/// handed — each one, in order. /// handed — each one, in order.