From 465acae547b5822b7532e8b994c9a9c3d16de8f5 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 13 Aug 2026 16:52:26 +0200 Subject: [PATCH] fix(gate): review round 2 --- the platform floor, the nested reserve, and marker types **108 IS LINUX'S NUMBER, NOT THE FLOOR.** Darwin's `sun_path` is 104 (xnu `bsd/sys/un.h`) and pmacs supports macOS --- CI runs a `macos-latest` leg --- so a Linux-derived limit passes on the machine that writes it and bind-fails on the other. **The usable PATH length is one less than the array**, because the stored value is NUL-terminated: 103 on Darwin, 107 on Linux. The script takes **103**, and the diagnostic says which platform's floor it is quoting. **THE NESTED CASE IS NOW RULED, NOT ACCOMMODATED BY LOOSENING THE GUARD.** The reserve exists for fixtures that bind sockets under TMPDIR; this script's own behaviour suite runs nested gates whose plans are synthetic and bind nothing, so charging them the fixture reserve rejects a configuration that cannot suffer the failure it guards against. Exempting nested runs was rejected --- it makes the guard untestable in the very configuration the tests exercise, and "this run is nested" is not reliably knowable. **The suite roots its gates at a short base instead**, so a nested TMPDIR is ~24 bytes rather than ~71 and clears the real reserve. Recorded in revision 6 with the rejected alternative, and with the obligation that a future row which DOES bind a socket must move off that base and take the reserve with it. **MIRRORING THE MARKER NAMES WAS NOT ENOUGH; THE TYPES ARE PART OF THE CONTRACT.** `match_marker` requires `.git` to be a DIRECTORY and the seven language markers to be FILES, so `[ -e ]` rejected ancestors project detection walks straight past. The case is not exotic: **a git WORKTREE has a `.git` FILE**, so every worktree in this repository would have tripped the guard. It tests `[ -d ]` for `.git` and `[ -f ]` for the rest, with a witness covering all three shapes --- `.git` file accepted, `.git` directory refused, `Cargo.toml` directory accepted. That witness keys on WHICH marker the gate named rather than on whether a refusal happened, because the ancestors of any base a test can create are outside its control; "no refusal" is not a claim it can make anywhere, while "the refusal did not name MY file" is. `M-G-4` reverts the guard to existence-only and the row fails. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/gate-script-framing.md | 46 ++++++++++++++++++++++++ scripts/gate | 32 ++++++++++++++--- tests/gate_script_acceptance.rs | 62 +++++++++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 4 deletions(-) diff --git a/docs/gate-script-framing.md b/docs/gate-script-framing.md index 61b4070..ef86fce 100644 --- a/docs/gate-script-framing.md +++ b/docs/gate-script-framing.md @@ -475,6 +475,52 @@ Four decisions inside that, each of which had a cheaper wrong answer: the gate owns is *necessary, not sufficient*, so the precondition is verified rather than assumed. + **MIRRORING THE NAMES IS NOT ENOUGH — the TYPES are part of the + contract.** `match_marker` (`src/project.rs`) requires `.git` to be + a **directory** and the seven language markers to be **files**, so + an existence-only test rejects ancestors detection itself ignores. + The case that matters is not exotic: **a git WORKTREE has a `.git` + FILE**, so every worktree in this repository would have tripped an + `[ -e ]` check while project detection walked straight past it. + The guard tests `[ -d ]` for `.git` and `[ -f ]` for the rest. + +**The budget is the SUPPORTED-PLATFORM FLOOR, not Linux's.** `sun_path` +is 108 bytes on Linux but **104 on Darwin** (xnu `bsd/sys/un.h`), and +pmacs supports macOS — CI runs a `macos-latest` leg. A Linux-derived +limit would pass on the machine that wrote it and bind-fail on the +other, which is the worst place to find out. **The usable PATH length is +one less than the array**, because the stored value is NUL-terminated: +103 on Darwin, 107 on Linux. The script takes **103**. + +**RULING — a synthetic nested gate does not pay a reserve it never +uses.** The reserve exists for fixtures that bind sockets under +`TMPDIR`. This script's own behaviour suite runs *nested* gates whose +plans are synthetic (`true`, `false`, one `echo`) and which bind no +socket at all, so applying the fixture reserve to them would reject a +configuration that cannot suffer the failure it guards against — and +the suite would fail on a setup it created rather than on the behaviour +under test. That is not hypothetical: at a 45-byte reserve the nested +path measured ~71 bytes and was rejected. + +Two ways to resolve it were available, and **the layout was changed +rather than the guard weakened**: + +- *Rejected — exempt nested runs from the guard.* It would make the + guard untestable in the configuration the tests exercise, and "this + run is nested" is not something the script can know reliably. +- **Adopted — the behaviour suite roots its gates at a SHORT base + (`/tmp`) instead of inheriting the ambient `TMPDIR`.** A nested gate + then sits at ~24 bytes rather than ~71 and clears the real reserve + with room to spare. The suite is explicit that it does this for the + socket budget, and it is free to use `/tmp` precisely because its + plans create no markerless fixture — the same reason it may set the + ancestor escape. + +**The guard therefore keeps the true maximum for real runs**, and the +tests stop paying for a hazard they cannot encounter. If a future +behaviour row *does* bind a socket, it must move off the short base and +take the reserve with it. + **Escape hatch, documented test-only.** `PMACS_GATE_ALLOW_ANCESTOR_MARKER` exists for this script's own behaviour tests, which run the gate under a `tempfile::tempdir()` whose diff --git a/scripts/gate b/scripts/gate index 1b2dbd2..d1e2c4b 100755 --- a/scripts/gate +++ b/scripts/gate @@ -588,6 +588,14 @@ trap cleanup EXIT INT TERM # unrelated-looking socket failures deep in a suite, naming a limit # rather than a cause. # +# THE BUDGET IS THE SUPPORTED-PLATFORM FLOOR, NOT LINUX'S. `sun_path` +# is 108 bytes on Linux but **104 on Darwin** (xnu `bsd/sys/un.h`), and +# pmacs supports macOS --- CI runs a `macos-latest` leg. A +# Linux-derived limit would pass here and bind-fail there, which is the +# worst place to discover it. **The usable PATH length is one less than +# the array**, because the value stored in `sun_path` is +# NUL-terminated: 103 on Darwin, 107 on Linux. This script takes 103. +# # THE RESERVE IS THE MEASURED MAXIMUM PLUS HEADROOM, not a round # number. The longest observed is `/.tmpXXXXXX/directory-target.sock` # --- 33 bytes (`tests/gpu_invocation_acceptance.rs`) --- so 48 leaves @@ -596,14 +604,15 @@ trap cleanup EXIT INT TERM # COUNTED IN BYTES, NOT CHARACTERS. `${#var}` counts characters under a # UTF-8 locale while `sun_path` is byte-limited, so a multibyte path # would measure short and pass a check it should fail. -SUN_LEN_BUDGET=108 +SUN_LEN_BUDGET=103 TMPDIR_SUFFIX_RESERVE=48 GATE_TMPDIR_BYTES=$(printf '%s' "$GATE_TMPDIR" | LC_ALL=C wc -c) if [ "$GATE_TMPDIR_BYTES" -gt "$((SUN_LEN_BUDGET - TMPDIR_SUFFIX_RESERVE))" ]; then echo "gate: TMPDIR is too long for a unix socket path:" >&2 echo "gate: $GATE_TMPDIR" >&2 echo "gate: $GATE_TMPDIR_BYTES bytes, but a fixture needs" \ - "$TMPDIR_SUFFIX_RESERVE of the $SUN_LEN_BUDGET-byte SUN_LEN budget" >&2 + "$TMPDIR_SUFFIX_RESERVE of the $SUN_LEN_BUDGET usable bytes" \ + "(Darwin sun_path[104] minus its NUL --- the supported floor)" >&2 echo "gate: shorten PMACS_GATE_TARGET_ROOT (or \$HOME) and retry." >&2 exit 2 fi @@ -639,9 +648,15 @@ for _anc in $( done printf '/\n' ); do + # TYPE MATTERS, and an existence-only test is wrong in both + # directions. `match_marker` in `src/project.rs` requires `.git` to + # be a DIRECTORY and the seven language markers to be FILES, so + # `[ -e ]` would reject ancestors detection itself ignores --- most + # importantly a `.git` FILE, which is exactly what a git WORKTREE + # has. Every worktree in this repo would have tripped it. for _m in Cargo.toml .luarc.json pyproject.toml go.mod deno.json \ - deno.jsonc package.json .git; do - if [ -e "$_anc/$_m" ]; then + deno.jsonc package.json; do + if [ -f "$_anc/$_m" ]; then echo "gate: a project marker sits above the gate TMPDIR:" >&2 echo "gate: $_anc/$_m" >&2 echo "gate: TMPDIR is $GATE_TMPDIR" >&2 @@ -651,6 +666,15 @@ for _anc in $( exit 2 fi done + if [ -d "$_anc/.git" ]; then + echo "gate: a project marker sits above the gate TMPDIR:" >&2 + echo "gate: $_anc/.git" >&2 + echo "gate: TMPDIR is $GATE_TMPDIR" >&2 + echo "gate: every markerless test fixture beneath it would be" >&2 + echo "gate: re-rooted at that directory. Move the gate root" >&2 + echo "gate: (PMACS_GATE_TARGET_ROOT) somewhere without one." >&2 + exit 2 + fi done fi diff --git a/tests/gate_script_acceptance.rs b/tests/gate_script_acceptance.rs index cb9a4f5..a07eb42 100644 --- a/tests/gate_script_acceptance.rs +++ b/tests/gate_script_acceptance.rs @@ -646,6 +646,68 @@ fn a_project_marker_above_the_gate_tmpdir_is_refused() { ); } +/// A `.git` **FILE** above the root is accepted; a `.git` **DIRECTORY** +/// is refused. +/// +/// **The types are the contract, not the names.** `match_marker` +/// (`src/project.rs`) requires `.git` to be a directory and the seven +/// language markers to be files, so an existence-only check would +/// reject ancestors project detection walks straight past. The case is +/// not exotic: **a git worktree has a `.git` FILE**, so every worktree +/// in this repository would have tripped an `[ -e ]` guard. +#[test] +fn the_ancestor_check_honours_marker_types() { + // **Asserted on WHICH marker is named, not on whether a refusal + // happened.** The ancestors of any base this test can create are + // outside its control — `/tmp` may hold a real `.git` directory, + // and the repo root holds a `Cargo.toml` — so "no refusal" is not + // a claim it can make anywhere. "The refusal did not name MY file" + // is, and it is the claim that actually distinguishes the two + // types. + let base = tempfile::Builder::new() + .prefix("gt-") + .tempdir_in(short_root_base()) + .expect("base"); + let root = base.path().join("inner"); + std::fs::create_dir_all(&root).expect("root"); + + let run_it = || { + 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") + }; + + // A `.git` FILE — a worktree — is not a project root to detection, + // so it must not be one here either. + let mine = base.path().join(".git"); + std::fs::write(&mine, "gitdir: /elsewhere\n").expect("git file"); + let err = String::from_utf8_lossy(&run_it().stderr).into_owned(); + assert!( + !err.contains(&format!("{}", mine.display())), + "a `.git` FILE must not be treated as a marker, but the gate \ + named it; stderr:\n{err}" + ); + + // The same name as a DIRECTORY is a real marker. + std::fs::remove_file(&mine).expect("rm git file"); + std::fs::create_dir(&mine).expect("git dir"); + let out = run_it(); + let err = String::from_utf8_lossy(&out.stderr).into_owned(); + assert!( + !out.status.success(), + "a `.git` DIRECTORY must refuse the run" + ); + assert!( + err.contains(&format!("{}", mine.display())), + "and must name it, not some other ancestor; stderr:\n{err}" + ); +} + /// The seam handoff §3 keeps authority over: a script cannot infer /// which acceptance suites a change touched, so it runs what it is /// handed — each one, in order.