From d84df23aa7219075c28208322428bcac7aee5c09 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 13 Aug 2026 15:55:19 +0200 Subject: [PATCH 01/10] fix(gate): isolate TMPDIR per invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discharges the standing fix recorded in `docs/agent-handoff.md` §1 and assigned to this lane. Every gate invocation now gets a fresh, disk-backed `TMPDIR` at `/tmp/`, exported once so every stage and every process they spawn inherits it, reaped by the same exit trap as the ambient root. **A gate run no longer needs a `TMPDIR=` override.** **A CHILD OF `/tmp` WOULD NOT HAVE WORKED**, which is why the obvious cheaper fix was not taken. The hazard is an ANCESTOR marker: project detection walks upward, so a fresh subdirectory of `/tmp` inherits `/tmp`'s ancestors and the same stray `.git`. The directory had to move somewhere the gate already owns. **`SUN_LEN` shaped the layout, and the fix's own gate run is what found it.** A Unix socket path cannot exceed 108 bytes, and the suites bind sockets INSIDE `TMPDIR`. The first placement --- `$TARGET/gate-tmp/$STAMP-$$` --- produced a 114-byte socket path and failed SIX daemon and attach tests with "path must be shorter than SUN_LEN". It hangs off the gate root (36 bytes) rather than the per-worktree target (60) now, with a short name: 47 bytes, leaving 61 for fixtures. Running the real gate rather than only the witnesses is what caught this. **A startup guard turns that failure class into a named one.** Six socket failures deep in a suite name a LIMIT, not a CAUSE; the guard fails immediately with the path, its length, and what to shorten. **Its reserve is measured, not round, and the first value was wrong in the more embarrassing direction.** The longest suffix a fixture appends is `/.tmpXXXXXXX/test.sock`, 21 bytes, so 30 leaves ~40% headroom. An earlier "generous" 45 FIRED ON THE GATE'S OWN BEHAVIOUR TESTS: they run the gate inside the gate, so their root sits under the outer run's TMPDIR and the nested path reaches 71 bytes. A guard that rejects a legitimate configuration is worse than the failure it prevents, because it fires on every run instead of a rare one. Verified both directions: still catches an 87-byte root, silent on the real one and on the nested tests. **Two witnesses, each mutation-checked.** `M-G-1` removes the export -> the propagation row alone; `M-G-2` stops the reaping -> the cleanup row alone. Propagation is observed in a SPAWNED CHILD --- the self-test's first step reports its own `$TMPDIR` into its log --- because asserting the variable inside the script would only prove the script can set a variable. The cleanup row runs against the self-test, which FAILS on purpose, so it also pins that the trap fires on the failure path, which is the path a leak would actually happen on. One witness of mine needed correcting twice, both times because it asserted something adjacent to the contract: first `!starts_with("/tmp/")`, which tested where the FIXTURE put its root and failed on correct code; then `contains("/gate-tmp/")`, stale after the directory was shortened. It now asserts the exact parent, `/tmp`. **Proved against the live hazard:** `/tmp/.git` is still present on this machine, and `m4_24_bare_string_glob_stays_relative` --- one of the two tests it reddened --- passes with no override. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/active-work.md | 50 ++++++++++++++ docs/agent-handoff.md | 30 +++++++-- scripts/gate | 83 +++++++++++++++++++++++- tests/gate_script_acceptance.rs | 111 ++++++++++++++++++++++++++++++++ 4 files changed, 265 insertions(+), 9 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index fede39e..606b144 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -270,6 +270,56 @@ hazard in a shape that looks committed. **A documented error message that never appears is worse than no documentation**, because the reader waits for a signal that is not coming. +## `scripts/gate` TMPDIR isolation — branch OPEN, no PR yet + +**Written with the branch's first commit**, per the standing correction +from #171 and #215. + +- **Branch `gate-tmpdir-isolation`**, base `githubsucks/main` @ + `ca92796` exactly (the #239 merge). **Recover with `git fetch + githubsucks && git checkout gate-tmpdir-isolation`.** +- **No framing.** This discharges a standing fix already recorded in + `docs/agent-handoff.md` §1 and assigned to this lane; the design + question was settled when the hazard was diagnosed. +- **What it does:** every gate invocation gets a fresh, disk-backed + `TMPDIR` at `/tmp/`, exported once so every stage and + every process they spawn inherits it, and reaped by the same exit trap + as the ambient root. **A gate run no longer needs a `TMPDIR=` + override.** +- **A CHILD OF `/tmp` WOULD NOT HAVE WORKED.** The hazard is an + ANCESTOR marker — project detection walks upward — so a fresh + subdirectory of `/tmp` inherits `/tmp`'s ancestors and the same stray + `.git`. The directory had to move somewhere the gate already owns. +- **`SUN_LEN` is the constraint that shaped the layout, and the fix's + own gate run found it.** A Unix socket path cannot exceed 108 bytes + and the suites bind sockets inside `TMPDIR`. The first placement, + `$TARGET/gate-tmp/$STAMP-$$`, produced a 114-byte socket path and + failed **six** daemon and attach tests with *"path must be shorter + than SUN_LEN"*. It now hangs off the **gate root** (36 bytes) rather + than the per-worktree target (60), with a short name: **47 bytes**, + leaving 61 for fixtures. +- **A startup guard converts that failure class into a named one.** Six + socket failures deep in a suite name a limit, not a cause. The guard + fails immediately with the path, its length and what to shorten. + **Its reserve is measured, not round**: the longest suffix a fixture + appends is 21 bytes, so 30 leaves ~40% headroom. An earlier + "generous" 45 **fired on the gate's own behaviour tests**, which run + the gate inside the gate and so reach 71 bytes — a guard that rejects + a legitimate configuration fails on every run rather than a rare one. +- **Witnesses (2), each mutation-checked:** `M-G-1` removes the export + → the propagation row alone; `M-G-2` stops the reaping → the cleanup + row alone. Propagation is observed in a **spawned child** (the + self-test's first step reports its own `$TMPDIR` into its log), + because the gate exporting a variable would only prove the gate can + export a variable. +- **Proved against the live hazard:** `/tmp/.git` is still present on + this machine, and the tests it reddened now pass with **no override**. +- **Gates:** `./scripts/gate --acceptance gate_script_acceptance`, run + with `env -u TMPDIR` — the point is that it needs no override. No + `--protocol`: no wire change. +- **Out of scope, deliberately:** the overdue absorption of #239. This + lane is the gate fix and nothing else. + ## GUI arc Stage 1a — `TextInput` at v24 — PR #239 OPEN **Written with the branch's first commit**, per the standing correction diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index 84e71e6..ca67e29 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -280,10 +280,25 @@ commands, read `docs/active-work.md` immediately after this file. markerless-fixture red that looks like a watcher bug may be an ancestor marker, on any machine. - **SEEN AGAIN 2026-08-11, and `scripts/gate` DOES NOT PROTECT YOU - FROM IT.** The gate isolates the target directory and five ambient - roots but **not `TMPDIR`**, so `tempfile::tempdir()` still lands - under a `/tmp` that may carry a marker. It surfaced inside a gate + **FIXED 2026-08-13: `scripts/gate` now isolates `TMPDIR`.** Each + invocation gets a fresh directory under the managed target root + (`/gate-tmp/-`), exported once so every stage + and every process they spawn inherits it, and reaped by the same + exit trap as the ambient root. **A gate run no longer needs a + `TMPDIR=` override**, and the two witnesses in + `tests/gate_script_acceptance.rs` pin propagation-to-a-child and + cleanup-on-exit separately. + + **Why a subdirectory of `/tmp` would NOT have worked**, since that + is the obvious cheaper fix: the hazard is an ANCESTOR marker, and a + child of `/tmp` has exactly the same ancestors. The directory had to + move somewhere the gate already owns. + + *(Historical, and still the shape to recognize outside the gate — + `cargo test` run by hand is unprotected.)* The gate isolated the + target directory and five ambient roots but **not `TMPDIR`**, so + `tempfile::tempdir()` landed under a `/tmp` that may carry a + marker. It surfaced inside a gate run on an unrelated lane (GUI 1-pre, whose whole **executable** diff is inside the `pmacs-gpu` crate) as **`m4_24_bare_string_glob_stays_relative` and `m4_24_d3_fallback_base_is_the_smallest_attachment_dir`**, in @@ -327,9 +342,10 @@ commands, read `docs/active-work.md` immediately after this file. its ancestor. Verify with `git -C rev-parse --show-toplevel` failing, not by eye. **Do not delete a foreign marker** — isolating is sufficient and deletion is someone else's call. **Isolating - `TMPDIR` inside `scripts/gate` is the standing fix and belongs to - the gate lane**, not to whichever feature PR happens to trip over - it. + `TMPDIR` inside `scripts/gate` was the standing fix; it landed + 2026-08-13** (see the FIXED note above), so a gate run is protected + and the override is no longer needed. **A bare `cargo test` still + is not** — that is when to check the ancestors. - **A libtest filter that matches nothing reports `test result: ok`.** `0 passed; 0 failed; N filtered out` and a zero exit code are what a *typo'd or mis-quoted filter* looks like, and it is indistinguishable diff --git a/scripts/gate b/scripts/gate index b946f69..994be13 100755 --- a/scripts/gate +++ b/scripts/gate @@ -287,7 +287,17 @@ emit_plan() { # command failed. # --------------------------------------------------------------------- emit_self_test_plan() { - printf 'self-pass\ttrue\n' + # `self-pass` reports the TMPDIR its own CHILD PROCESS sees, rather + # than `true`. The step is still a trivially-passing command with + # its own log, so every existing assertion about it holds --- but + # the log now carries evidence that the isolated TMPDIR reached a + # spawned process, which is the only thing that matters. Asserting + # the variable inside this script would prove that this script can + # set a variable. + # + # `$TMPDIR` is literal here: the single-quoted printf format leaves + # it for the runner's `eval` to expand, in the child. + printf 'self-pass\tsh -c "echo gate-child-tmpdir=$TMPDIR"\n' printf 'build-crdt\tfalse\n' printf 'self-sentinel\ttrue\n' } @@ -516,13 +526,79 @@ mkdir -p "$LOGDIR" # reads the environment it was handed. HOME is deliberately left alone. AMBIENT="$TARGET/gate-ambient/$STAMP-$$" mkdir -p "$AMBIENT" -cleanup() { rm -rf "$AMBIENT"; } + +# TMPDIR: a fresh, DISK-BACKED directory per invocation, reaped with the +# rest. +# +# THE HAZARD IS AN ANCESTOR MARKER, NOT A DIRTY TEMP DIRECTORY. Project +# detection walks upward, so a stray `/tmp/.git` re-roots every +# markerless `tempfile::tempdir()` fixture beneath it at `/tmp` --- and +# the tests then faithfully exercise a tree of several thousand +# unrelated entries. Seen for real: an empty `/tmp/.git` reddened +# `m4_24_bare_string_glob_stays_relative` and +# `m4_24_d3_fallback_base_is_the_smallest_attachment_dir` inside a gate +# run on a lane whose whole executable diff was in `pmacs-gpu`, a crate +# the failing test binary does not even link. +# +# So the fix is NOT "clean the temp directory" --- a fresh subdirectory +# OF `/tmp` inherits the same ancestors and the same marker. It has to +# live somewhere the gate already owns, which is the target root: no +# marker above it, and it is per-worktree already. +# +# Disk-backed matters independently. `/tmp` is commonly a tmpfs, so a +# sweep's fixtures compete with the machine for RAM; a build here has +# hit tmpfs quota mid-compile. The gate root is on the same filesystem +# as the build artifacts, which is where the space is. +# +# IT HANGS OFF THE GATE ROOT, NOT THE PER-WORKTREE TARGET, AND THE NAME +# IS SHORT ON PURPOSE. A Unix socket path cannot exceed `SUN_LEN` (108 +# bytes on Linux), and the suites bind sockets INSIDE `TMPDIR`: the +# first version of this put it at `$TARGET/gate-tmp/$STAMP-$$`, and +# `.../gate-tmp/-/.tmpXXXXXXX/test.sock` came to 114 bytes, +# failing six daemon and attach tests with `path must be shorter than +# SUN_LEN`. The per-worktree target alone is 60 bytes; the gate root is +# 36. Every byte spent here is a byte a fixture cannot use. +# +# `HOME` is still deliberately left alone, as above. +GATE_TMPDIR="$(gate_root)/tmp/$$" +mkdir -p "$GATE_TMPDIR" + +# Fail LOUDLY and immediately if the budget is gone. Without this the +# symptom is six unrelated-looking socket failures deep in a suite, +# naming a limit rather than a cause; a long `$HOME` or a deeply-nested +# checkout is all it takes. +# +# THE RESERVE IS MEASURED, NOT ROUND. The longest suffix a fixture +# actually appends is `/.tmpXXXXXXX/test.sock`, 21 bytes; 30 leaves +# ~40% headroom over that. An earlier draft reserved a "generous" 45, +# which fired on the gate's OWN behaviour tests: they run the gate +# inside the gate, so their `PMACS_GATE_TARGET_ROOT` is itself under +# the outer run's TMPDIR and the nested path reaches 71 bytes. A guard +# that rejects a legitimate configuration is a worse failure than the +# one it prevents, because it fires on every run rather than on a rare +# one. +SUN_LEN_BUDGET=108 +TMPDIR_SUFFIX_RESERVE=30 +if [ "${#GATE_TMPDIR}" -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, but a fixture needs" \ + "$TMPDIR_SUFFIX_RESERVE of the $SUN_LEN_BUDGET-byte SUN_LEN budget" >&2 + echo "gate: shorten PMACS_GATE_TARGET_ROOT (or \$HOME) and retry." >&2 + exit 2 +fi + +cleanup() { rm -rf "$AMBIENT" "$GATE_TMPDIR"; } trap cleanup EXIT INT TERM export CARGO_TARGET_DIR="$TARGET" export XDG_CONFIG_HOME="$AMBIENT" XDG_DATA_HOME="$AMBIENT" \ XDG_STATE_HOME="$AMBIENT" XDG_CACHE_HOME="$AMBIENT" \ PMACS_STATE_HOME="$AMBIENT" +# Exported once, here, so EVERY stage and every process they spawn +# inherits it --- the stages run as children of this shell, so there is +# no per-stage plumbing to forget. +export TMPDIR="$GATE_TMPDIR" echo "gate: worktree $WT" echo "gate: target dir $TARGET" @@ -531,6 +607,9 @@ echo "gate: logs $LOGDIR" # this is the directory all five ambient roots point at, and the exit # trap removes it, so it should be gone once the run finishes. echo "gate: ambient $AMBIENT" +# Printed for the same reason as the ambient root: the isolation is +# observable, and the exit trap removes this too. +echo "gate: tmpdir $GATE_TMPDIR" [ -n "$ACCEPTANCE" ] && echo "gate: acceptance $ACCEPTANCE" [ "$PROTOCOL" = 1 ] && echo "gate: protocol yes (CRDT build + workspace sweep added)" if [ "$MODE" = selftest ]; then diff --git a/tests/gate_script_acceptance.rs b/tests/gate_script_acceptance.rs index 37de5f2..deaa2a1 100644 --- a/tests/gate_script_acceptance.rs +++ b/tests/gate_script_acceptance.rs @@ -428,6 +428,117 @@ fn self_test_names_the_failing_gate_and_the_suite_continues_past_it() { ); } +/// The isolated `TMPDIR` reaches a spawned CHILD, and is disk-backed +/// under the managed target root. +/// +/// **The hazard is an ancestor marker, not a dirty temp directory.** +/// Project detection walks upward, so a stray `/tmp/.git` re-roots every +/// markerless `tempfile::tempdir()` fixture beneath it — which is how +/// two LSP file-watcher tests reddened a gate run on a lane whose whole +/// executable diff lived in a crate the failing binary does not link. A +/// fresh subdirectory *of* `/tmp` would inherit the same ancestors and +/// the same marker, so the directory has to live where the gate already +/// owns the path. +/// +/// **Observed in a child process, deliberately.** The gate exporting a +/// variable proves only that the gate can export a variable; what the +/// suites need is that a process the runner spawns inherits it. The +/// self-test's first step reports its own `$TMPDIR`, so the assertion +/// reads a real child's environment out of a real gate log. +#[test] +fn the_isolated_tmpdir_reaches_a_spawned_child_under_the_managed_root() { + let root = tempfile::tempdir().expect("tempdir"); + let (out, err, _ok) = run(root.path(), &["--self-test"]); + + let announced = out + .lines() + .find_map(|l| { + l.split_once("gate: tmpdir") + .map(|(_, p)| p.trim().to_owned()) + }) + .unwrap_or_else(|| panic!("the gate must announce its TMPDIR; stdout:\n{out}")); + + // The contract is **under the managed target root**, which is what + // makes it both marker-free and disk-backed in production: the + // target root sits beside the build artifacts, not in `/tmp`. + // + // Deliberately NOT asserting `!starts_with("/tmp/")` here. This + // test's own root is a `tempfile::tempdir()`, so on a normal + // machine it IS under `/tmp` and the gate's directory inherits + // that — such an assertion would be testing where the fixture put + // its root, not what the gate does, and would fail on correct code. + let expected_prefix = root.path().join("").display().to_string(); + assert!( + announced.starts_with(&expected_prefix), + "the gate TMPDIR must live under the managed target root, which \ + the gate owns and prunes; expected a child of {expected_prefix}, \ + was {announced}" + ); + // Its own named area under the root, so `prune` and a human can + // both tell it from the ambient root and the logs. Asserted as the + // exact parent rather than a substring: the directory is called + // `tmp`, and a `contains("/tmp/")` check would also pass for a path + // that merely happened to sit under a `/tmp` somewhere. + assert_eq!( + Path::new(&announced).parent().expect("tmpdir parent"), + root.path().join("tmp"), + "the per-run TMPDIR must sit directly under /tmp; was {announced}" + ); + + // The child's own view, read out of the log the runner wrote. + let log = Path::new( + err.lines() + .find_map(|l| l.split_once("log: ").map(|(_, p)| p.trim())) + .unwrap_or_else(|| panic!("expected a log path; stderr:\n{err}")), + ) + .parent() + .expect("log directory") + .join("01-self-pass.log"); + let seen = + std::fs::read_to_string(&log).unwrap_or_else(|e| panic!("read {}: {e}", log.display())); + assert_eq!( + seen.trim(), + format!("gate-child-tmpdir={announced}"), + "a spawned child must inherit exactly the announced TMPDIR" + ); +} + +/// The `TMPDIR` is reaped when the run ends, like the ambient root. +/// +/// Without this the gate would leak a directory per invocation into the +/// target root — the same accumulation `prune` exists to clean up, but +/// created by the tool itself and on every single run. +#[test] +fn the_isolated_tmpdir_is_reaped_when_the_run_ends() { + let root = tempfile::tempdir().expect("tempdir"); + let (out, _err, _ok) = run(root.path(), &["--self-test"]); + + let announced = out + .lines() + .find_map(|l| { + l.split_once("gate: tmpdir") + .map(|(_, p)| p.trim().to_owned()) + }) + .unwrap_or_else(|| panic!("the gate must announce its TMPDIR; stdout:\n{out}")); + + // The run above FAILED on purpose (the self-test's middle step), so + // this also pins that the trap fires on the failure path — the path + // a leak would actually happen on. + assert!( + !Path::new(&announced).exists(), + "the exit trap must remove the TMPDIR even when a gate failed; \ + {announced} survived" + ); + // The parent stays: it is the per-worktree home the next run uses. + assert!( + Path::new(&announced) + .parent() + .expect("gate-tmp parent") + .exists(), + "only the per-run directory is reaped, not its parent" + ); +} + /// 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. From 72647829df19530447dd340e08d579bfcb0ae070 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 13 Aug 2026 16:00:17 +0200 Subject: [PATCH 02/10] docs: record PR #240 in the gate lane The number goes in the moment the PR opens, per this file's own rule. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/active-work.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/active-work.md b/docs/active-work.md index 606b144..39a028f 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -270,11 +270,12 @@ hazard in a shape that looks committed. **A documented error message that never appears is worse than no documentation**, because the reader waits for a signal that is not coming. -## `scripts/gate` TMPDIR isolation — branch OPEN, no PR yet +## `scripts/gate` TMPDIR isolation — PR #240 OPEN **Written with the branch's first commit**, per the standing correction from #171 and #215. +- **PR #240** — https://github.com/levineuwirth/pmacs/pull/240. - **Branch `gate-tmpdir-isolation`**, base `githubsucks/main` @ `ca92796` exactly (the #239 merge). **Recover with `git fetch githubsucks && git checkout gate-tmpdir-isolation`.** From 1bd52b7f0d95ae7488884d8fcd258124e2864385 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 13 Aug 2026 16:34:21 +0200 Subject: [PATCH 03/10] fix(gate): review round 1 --- the propagation row proved nothing, and two guards were wrong MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **THE PROPAGATION WITNESS DID NOT OBSERVE INHERITANCE.** The runner's `eval` expanded `$TMPDIR` in the PARENT before `sh -c` ever started, so the child received an already-substituted literal --- and an unexported `TMPDIR=` would have passed the row unchanged. Single-quoted inside `sh -c` now, so the CHILD expands it. **M-G-1b keeps the assignment and removes only `export`: the row fails.** That is the mutation the previous version could not catch, and the reason to prefer it over M-G-1's blunter deletion. **THE RESERVE WAS NOT THE MAXIMUM.** `/.tmpXXXXXX/directory-target.sock` is 33 bytes (`tests/gpu_invocation_acceptance.rs`), so paths of 76-78 passed the 30-byte guard and still blew the 108-byte limit during the CRDT sweep. Reserve is 48 now --- the measured maximum plus ~45% headroom. And the length is counted in BYTES: `${#var}` counts CHARACTERS under a UTF-8 locale while `sun_path` is byte-limited, so a multibyte path measured short and passed a check it should fail. **A MANAGED ROOT IS NOT INHERENTLY MARKER-FREE**, and assuming it was rebuilt the original defect one directory up: a `.git` in `$HOME`, a marker above `$HOME/build`, or a contaminated `PMACS_GATE_TARGET_ROOT`. Placement under a directory the gate owns is NECESSARY, NOT SUFFICIENT, and the old test proved only placement. The gate now walks the ancestors and refuses, naming the marker it found. `PMACS_GATE_ALLOW_ANCESTOR_MARKER` is the documented test-only escape, beside `PMACS_GATE_TARGET_ROOT` in kind and risk: the behaviour tests run under a tempdir whose ancestors they do not control, on a machine whose `/tmp` carries this very marker, and their plans are synthetic so no markerless fixture exists to re-root. **The check is witnessed by a row that deliberately does not set it**, and M-G-3 (check removed) fails that row. **The guard leaked what it exists to manage.** It created both temporary areas and exited before the trap was armed, so every rejection left an AMBIENT and a TMPDIR behind. The trap is installed first now; verified by rejecting a run and finding neither. **`tmp/$$` with `mkdir -p` was not fresh.** PIDs are reused, so after a SIGKILL it silently ADOPTS a leftover directory and the run inherits another run's fixtures. `mktemp -d` fails rather than reuses. **Prose corrected to match.** The handoff described `/gate-tmp/-`; the implementation uses `/tmp/`. Comments called the shared parent per-worktree and pruned --- it is neither: `--prune` only considers directories carrying an ownership marker, so the parent is skipped and each run removes its own leaf. **AND THE LANE CLAIMED A FRAMING EXCEPTION THAT DOES NOT EXIST.** `AGENTS.md` says framing -> approval -> branch -> implement, unconditionally; "the fix was already recorded as standing" is not an exemption it grants. `docs/gate-script-framing.md` is amended as **revision 6, AWAITING APPROVAL** --- a widening of §2's existing isolation responsibility rather than a new feature, which is why it amends that document instead of opening another. **This PR must not merge before that revision is approved.** Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/active-work.md | 11 ++- docs/agent-handoff.md | 29 +++++-- docs/gate-script-framing.md | 81 ++++++++++++++++++- scripts/gate | 137 +++++++++++++++++++++++--------- tests/gate_script_acceptance.rs | 63 +++++++++++++++ 5 files changed, 273 insertions(+), 48 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index 39a028f..4051a83 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -279,9 +279,14 @@ from #171 and #215. - **Branch `gate-tmpdir-isolation`**, base `githubsucks/main` @ `ca92796` exactly (the #239 merge). **Recover with `git fetch githubsucks && git checkout gate-tmpdir-isolation`.** -- **No framing.** This discharges a standing fix already recorded in - `docs/agent-handoff.md` §1 and assigned to this lane; the design - question was settled when the hazard was diagnosed. +- **Framing `docs/gate-script-framing.md`, revision 6 — AWAITING + APPROVAL, and the PR must not merge before it has it.** An earlier + version of this bullet claimed "no framing" on the grounds that the + fix was already recorded as standing. **`AGENTS.md` grants no such + exception**: its workflow is framing → approval → branch → implement, + unconditionally. Revision 6 widens §2's existing isolation + responsibility to `TMPDIR` rather than adding a feature, which is why + it amends this document instead of opening a new one. - **What it does:** every gate invocation gets a fresh, disk-backed `TMPDIR` at `/tmp/`, exported once so every stage and every process they spawn inherits it, and reaped by the same exit trap diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index ca67e29..47c6a21 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -281,13 +281,28 @@ commands, read `docs/active-work.md` immediately after this file. ancestor marker, on any machine. **FIXED 2026-08-13: `scripts/gate` now isolates `TMPDIR`.** Each - invocation gets a fresh directory under the managed target root - (`/gate-tmp/-`), exported once so every stage - and every process they spawn inherits it, and reaped by the same - exit trap as the ambient root. **A gate run no longer needs a - `TMPDIR=` override**, and the two witnesses in - `tests/gate_script_acceptance.rs` pin propagation-to-a-child and - cleanup-on-exit separately. + invocation gets a directory created fresh by `mktemp -d` under + **`/tmp/`** — the shared gate root, not the per-worktree + target, because a Unix socket path cannot exceed **`SUN_LEN`** (108 + bytes) and the suites bind sockets inside `TMPDIR`. It is exported + once so every stage and every process they spawn inherits it, and + reaped by the same exit trap as the ambient root. **A gate run no + longer needs a `TMPDIR=` override.** + + **That parent is SHARED between worktrees and `--prune` does not + touch it** — prune only considers directories carrying an ownership + marker. Each run removes its own leaf; the parent stays as a stable + empty directory. + + **Two guards, both of which cost a round to get right.** A + byte-counted length check (`${#var}` counts CHARACTERS under UTF-8 + while `sun_path` is byte-limited) reserving the measured maximum + suffix — `/.tmpXXXXXX/directory-target.sock`, 33 bytes — plus + headroom. And an **ancestor-marker check**, because *a managed root + is not inherently marker-free*: a `.git` in `$HOME` or above + `$HOME/build` rebuilds the original defect one directory up. + `PMACS_GATE_ALLOW_ANCESTOR_MARKER` is the documented test-only + escape, and a row that does not set it witnesses the refusal. **Why a subdirectory of `/tmp` would NOT have worked**, since that is the obvious cheaper fix: the hazard is an ANCESTOR marker, and a diff --git a/docs/gate-script-framing.md b/docs/gate-script-framing.md index b28ea0b..61b4070 100644 --- a/docs/gate-script-framing.md +++ b/docs/gate-script-framing.md @@ -1,6 +1,14 @@ # `scripts/gate` — per-worktree build isolation, and one gate suite -**Status: revision 5. Approved at revision 4 and IMPLEMENTED; revision +**Status: revision 6 — AWAITING APPROVAL.** Revision 6 extends the +isolation contract to `TMPDIR` (§2a below) and is the only unapproved +part of this document; everything else is as approved. It is a +*widening of an existing responsibility*, not a new feature: §2 already +owns "what the gate isolates", and `TMPDIR` was simply missing from +that list — which is how a stray `/tmp/.git` came to redden a gate run +on an unrelated lane. + +**Previously, revision 5. Approved at revision 4 and IMPLEMENTED; revision 5 records two safety defects review found in the implementation.** **Neither was a design gap — both were the implementation failing to @@ -417,6 +425,77 @@ under real parallel load, direnv is the escalation. --- +## 2a. `TMPDIR` isolation (revision 6, AWAITING APPROVAL) + +**The gap.** §2 lists what a gate run isolates: the target directory and +five ambient roots. `TMPDIR` was not on that list, so +`tempfile::tempdir()` fixtures landed wherever the operator's `/tmp` +pointed. That is not a hygiene preference — **project detection walks +UPWARD**, so a marker anywhere above the temp directory re-roots every +markerless fixture beneath it. + +**Observed, not hypothesised.** An empty `/tmp/.git` reddened +`m4_24_bare_string_glob_stays_relative` and +`m4_24_d3_fallback_base_is_the_smallest_attachment_dir` *inside a gate +run*, on a lane whose entire executable diff lived in `pmacs-gpu` — a +crate the failing test binary does not link. Diagnosing it cost a review +round, and the workaround was a manual `TMPDIR=` on every invocation. + +**The contract.** Each invocation gets a directory created fresh by +`mktemp -d` under `/tmp/`, exported once so every stage and +every process they spawn inherits it, and reaped by the exit trap that +already removes the ambient root. + +Four decisions inside that, each of which had a cheaper wrong answer: + +1. **Not a subdirectory of `/tmp`.** It inherits `/tmp`'s ancestors and + therefore the marker. The directory has to sit somewhere with no + marker above it. +2. **Off the GATE ROOT, not the per-worktree target.** A Unix socket + path cannot exceed `SUN_LEN` (108 bytes) and the suites bind sockets + *inside* `TMPDIR`. The per-worktree target is 60 bytes and the gate + root 36; the first implementation used the former and produced + 114-byte socket paths, failing six daemon and attach tests. **The + parent is consequently SHARED between worktrees and is not covered + by `--prune`**, which only considers directories carrying an + ownership marker; each run removes its own leaf. +3. **Created by `mktemp -d`, not `mkdir -p` on a pid.** PIDs are reused, + so after a SIGKILL a `mkdir -p` silently *adopts* a leftover + directory and the run inherits another run's fixtures. +4. **Two guards, and both fail loudly at startup** rather than letting + the symptom appear deep in a suite as a limit with no cause: + - a **byte-counted** length check reserving the measured maximum + suffix (`/.tmpXXXXXX/directory-target.sock`, 33 bytes) plus + headroom — byte-counted because `${#var}` counts *characters* + under UTF-8 while `sun_path` is byte-limited; + - an **ancestor-marker check**, because **a managed root is not + inherently marker-free**: a `.git` in `$HOME`, a marker above + `$HOME/build`, or a contaminated `PMACS_GATE_TARGET_ROOT` rebuilds + the original defect one directory up. Placement under a directory + the gate owns is *necessary, not sufficient*, so the precondition + is verified rather than assumed. + +**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 +ancestors they do not control — on a machine whose `/tmp` carries the +very marker in question — and whose plans are synthetic, so no +markerless fixture exists for a marker to re-root. It sits beside +`PMACS_GATE_TARGET_ROOT` in kind and in risk. **The check is witnessed +by a row that deliberately does not set it.** + +**Verification.** Two witnesses beyond the refusal row: propagation +observed in a *spawned child* (the self-test's first step reports its +own `$TMPDIR` into its log — asserting the variable inside the script +would only prove the script can set a variable), and cleanup after a +run that **failed on purpose**, which is the path a leak would actually +take. + +**Residual, stated rather than covered.** A custom project marker +registered at runtime is invisible to a shell script and is not +checked. The built-in list mirrors `default_markers()` in +`src/project.rs` and will drift if that list grows. + ## 3. Resolved questions ### Q#GS1 — directory naming — **RESOLVED** diff --git a/scripts/gate b/scripts/gate index 994be13..1b2dbd2 100755 --- a/scripts/gate +++ b/scripts/gate @@ -295,9 +295,12 @@ emit_self_test_plan() { # the variable inside this script would prove that this script can # set a variable. # - # `$TMPDIR` is literal here: the single-quoted printf format leaves - # it for the runner's `eval` to expand, in the child. - printf 'self-pass\tsh -c "echo gate-child-tmpdir=$TMPDIR"\n' + # SINGLE-QUOTED INSIDE `sh -c`, so the CHILD expands `$TMPDIR`. + # With double quotes the runner's own `eval` expands it in the + # PARENT before `sh` ever starts, and the row then passes even when + # the variable is assigned but never EXPORTED --- which is precisely + # the regression it exists to catch. + printf 'self-pass\tsh -c %s\n' "'echo gate-child-tmpdir=\$TMPDIR'" printf 'build-crdt\tfalse\n' printf 'self-sentinel\ttrue\n' } @@ -534,16 +537,15 @@ mkdir -p "$AMBIENT" # detection walks upward, so a stray `/tmp/.git` re-roots every # markerless `tempfile::tempdir()` fixture beneath it at `/tmp` --- and # the tests then faithfully exercise a tree of several thousand -# unrelated entries. Seen for real: an empty `/tmp/.git` reddened -# `m4_24_bare_string_glob_stays_relative` and -# `m4_24_d3_fallback_base_is_the_smallest_attachment_dir` inside a gate -# run on a lane whose whole executable diff was in `pmacs-gpu`, a crate -# the failing test binary does not even link. +# unrelated entries. Seen for real: an empty `/tmp/.git` reddened two +# LSP file-watcher tests inside a gate run on a lane whose whole +# executable diff was in `pmacs-gpu`, a crate the failing test binary +# does not even link. # # So the fix is NOT "clean the temp directory" --- a fresh subdirectory # OF `/tmp` inherits the same ancestors and the same marker. It has to -# live somewhere the gate already owns, which is the target root: no -# marker above it, and it is per-worktree already. +# live somewhere with no marker above it, which the check below +# VERIFIES rather than assumes. # # Disk-backed matters independently. `/tmp` is commonly a tmpfs, so a # sweep's fixtures compete with the machine for RAM; a build here has @@ -551,45 +553,106 @@ mkdir -p "$AMBIENT" # as the build artifacts, which is where the space is. # # IT HANGS OFF THE GATE ROOT, NOT THE PER-WORKTREE TARGET, AND THE NAME -# IS SHORT ON PURPOSE. A Unix socket path cannot exceed `SUN_LEN` (108 -# bytes on Linux), and the suites bind sockets INSIDE `TMPDIR`: the -# first version of this put it at `$TARGET/gate-tmp/$STAMP-$$`, and -# `.../gate-tmp/-/.tmpXXXXXXX/test.sock` came to 114 bytes, -# failing six daemon and attach tests with `path must be shorter than -# SUN_LEN`. The per-worktree target alone is 60 bytes; the gate root is -# 36. Every byte spent here is a byte a fixture cannot use. +# IS SHORT ON PURPOSE --- see the SUN_LEN budget below. Note this +# parent is SHARED between worktrees and is NOT covered by `--prune`, +# which only considers directories carrying an ownership marker. Each +# run removes its own leaf on exit; the parent is a stable empty +# directory. # # `HOME` is still deliberately left alone, as above. -GATE_TMPDIR="$(gate_root)/tmp/$$" -mkdir -p "$GATE_TMPDIR" +GATE_TMP_PARENT="$(gate_root)/tmp" +mkdir -p "$GATE_TMP_PARENT" -# Fail LOUDLY and immediately if the budget is gone. Without this the -# symptom is six unrelated-looking socket failures deep in a suite, -# naming a limit rather than a cause; a long `$HOME` or a deeply-nested -# checkout is all it takes. +# FRESH BY CREATION, not by hope. `tmp/$$` with `mkdir -p` silently +# ADOPTS a leftover directory after a SIGKILL or a power loss, because +# PIDs are reused; the run would then inherit another run's fixtures. +# `mktemp -d` fails rather than reuses, and the template is kept short +# because every byte here is a byte a socket path cannot use. +GATE_TMPDIR=$(mktemp -d "$GATE_TMP_PARENT/XXXXXX") || { + echo "gate: could not create a fresh TMPDIR under $GATE_TMP_PARENT" >&2 + exit 2 +} + +# THE TRAP IS INSTALLED BEFORE THE CHECKS BELOW, deliberately. An +# earlier draft ran the length guard first and exited on rejection with +# both temporary areas already created and no trap armed --- so the +# guard leaked exactly what it exists to manage, on every rejection. +cleanup() { rm -rf "$AMBIENT" "$GATE_TMPDIR"; } +trap cleanup EXIT INT TERM + +# --- Socket-path budget ------------------------------------------------ # -# THE RESERVE IS MEASURED, NOT ROUND. The longest suffix a fixture -# actually appends is `/.tmpXXXXXXX/test.sock`, 21 bytes; 30 leaves -# ~40% headroom over that. An earlier draft reserved a "generous" 45, -# which fired on the gate's OWN behaviour tests: they run the gate -# inside the gate, so their `PMACS_GATE_TARGET_ROOT` is itself under -# the outer run's TMPDIR and the nested path reaches 71 bytes. A guard -# that rejects a legitimate configuration is a worse failure than the -# one it prevents, because it fires on every run rather than on a rare -# one. +# A Unix socket path cannot exceed SUN_LEN (108 BYTES on Linux) and the +# suites bind sockets INSIDE TMPDIR, so TMPDIR must leave room for the +# longest path a fixture appends. Without this the symptom is several +# unrelated-looking socket failures deep in a suite, naming a limit +# rather than a cause. +# +# 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 +# ~45% headroom for a longer fixture name later. +# +# 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 -TMPDIR_SUFFIX_RESERVE=30 -if [ "${#GATE_TMPDIR}" -gt "$((SUN_LEN_BUDGET - TMPDIR_SUFFIX_RESERVE))" ]; then +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, but a fixture needs" \ + echo "gate: $GATE_TMPDIR_BYTES bytes, but a fixture needs" \ "$TMPDIR_SUFFIX_RESERVE of the $SUN_LEN_BUDGET-byte SUN_LEN budget" >&2 echo "gate: shorten PMACS_GATE_TARGET_ROOT (or \$HOME) and retry." >&2 exit 2 fi -cleanup() { rm -rf "$AMBIENT" "$GATE_TMPDIR"; } -trap cleanup EXIT INT TERM +# --- Ancestor markers -------------------------------------------------- +# +# A MANAGED ROOT IS NOT INHERENTLY MARKER-FREE, and assuming it was +# would rebuild the original defect one directory up: a `.git` in +# `$HOME`, any recognized marker above `$HOME/build`, or a contaminated +# `PMACS_GATE_TARGET_ROOT` re-roots every markerless fixture again. +# Placement under a directory the gate owns is a NECESSARY condition, +# not a sufficient one, so the precondition is checked rather than +# asserted. +# +# The list mirrors `default_markers()` in `src/project.rs`. A custom +# marker registered at runtime is out of reach from here, which is +# stated rather than papered over. +# +# PMACS_GATE_ALLOW_ANCESTOR_MARKER exists for the behaviour tests and +# is documented test-only, exactly like PMACS_GATE_TARGET_ROOT. They +# run the gate under a tempdir whose ancestors they do not control --- +# on a machine whose `/tmp` carries the very marker this checks for --- +# and their plans are synthetic (`true` / `false` / one `echo`), so no +# markerless fixture exists for a marker to re-root. The check itself +# is witnessed by a test that deliberately does NOT set this and +# asserts the refusal. +if [ -z "${PMACS_GATE_ALLOW_ANCESTOR_MARKER:-}" ]; then +for _anc in $( + _p="$GATE_TMPDIR" + while [ "$_p" != "/" ] && [ -n "$_p" ]; do + printf '%s\n' "$_p" + _p=$(dirname "$_p") + done + printf '/\n' +); do + for _m in Cargo.toml .luarc.json pyproject.toml go.mod deno.json \ + deno.jsonc package.json .git; do + if [ -e "$_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 + 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 +done +fi export CARGO_TARGET_DIR="$TARGET" export XDG_CONFIG_HOME="$AMBIENT" XDG_DATA_HOME="$AMBIENT" \ diff --git a/tests/gate_script_acceptance.rs b/tests/gate_script_acceptance.rs index deaa2a1..a2073c0 100644 --- a/tests/gate_script_acceptance.rs +++ b/tests/gate_script_acceptance.rs @@ -44,6 +44,14 @@ fn run_in(cwd: &Path, root: &Path, args: &[&str]) -> (String, String, bool) { .args(args) .current_dir(cwd) .env("PMACS_GATE_TARGET_ROOT", root) + // Test-only, like PMACS_GATE_TARGET_ROOT itself. These roots are + // `tempfile::tempdir()`s whose ancestors the suite does not + // control — on a machine whose `/tmp` carries a marker, every + // row here would otherwise be refused. The plans are synthetic, + // so no markerless fixture exists for a marker to re-root. The + // check is witnessed separately, by a row that does NOT set + // this. + .env("PMACS_GATE_ALLOW_ANCESTOR_MARKER", "1") .output() .expect("run scripts/gate"); ( @@ -539,6 +547,61 @@ fn the_isolated_tmpdir_is_reaped_when_the_run_ends() { ); } +/// A project marker above the gate TMPDIR is REFUSED. +/// +/// **Placement under a managed root is necessary, not sufficient.** A +/// `.git` in `$HOME`, any recognized marker above `$HOME/build`, or a +/// contaminated `PMACS_GATE_TARGET_ROOT` re-roots every markerless +/// fixture beneath it — which is the original defect, rebuilt one +/// directory up. Asserting only "the path sits under the configured +/// root" would prove placement and nothing about the hazard. +/// +/// This row deliberately does **not** set +/// `PMACS_GATE_ALLOW_ANCESTOR_MARKER`, which is what every other row +/// here sets; it is the one place the check itself runs. +#[test] +fn a_project_marker_above_the_gate_tmpdir_is_refused() { + // Built OUTSIDE the system temp dir on purpose: the point is to + // control what sits above the root, and `/tmp` may already carry a + // marker — which would make the row pass for the wrong reason. + let base = tempfile::Builder::new() + .prefix("gate-marker-") + .tempdir_in( + repo_root() + .join("target") + .exists() + .then(|| repo_root().join("target")) + .unwrap_or_else(std::env::temp_dir), + ) + .expect("base"); + let root = base.path().join("inner"); + std::fs::create_dir_all(&root).expect("root"); + std::fs::write(base.path().join("Cargo.toml"), "[package]\n").expect("marker"); + + let out = 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"); + let err = String::from_utf8_lossy(&out.stderr); + + assert!( + !out.status.success(), + "a marker above the TMPDIR must refuse the run; stderr:\n{err}" + ); + assert!( + err.contains("a project marker sits above the gate TMPDIR"), + "and must say so, naming the marker; stderr:\n{err}" + ); + assert!( + err.contains("Cargo.toml"), + "the message must name the marker it found; 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. From 30e976ae1e6976431fc8cfd47176757e967494d1 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 13 Aug 2026 16:43:05 +0200 Subject: [PATCH 04/10] docs,test: record the fourth managed_retry occurrence, and unblock the nested suite **The gate's own run reproduced a REGISTERED signature**, and it is recorded as a fourth occurrence rather than waved through: same selector, same `gpu`-step flavor, all three required fragments verified against the durable log. Three isolated re-runs were green, which this file's rule says establishes intermittence only. **This lane is code-neutral for `pmacs-gpu` but NOT environment-neutral**, and that distinction is the entry's point. Occurrence 3 excluded "the added GPU test is the mechanism"; this occurrence adds nothing to that binary at all, which corroborates the exclusion independently. But the lane moves `TMPDIR` off `/tmp`, taking every `tempfile::tempdir()` in the run from **tmpfs to btrfs** --- and the failing test runs a handshake against a **one-second deadline**. A slower filesystem under a timing-bounded test is a plausible mechanism that did not exist in occurrences 1-3. Booking this as "the usual flake" when the observing lane changed the conditions the flake is sensitive to is exactly the reasoning this registry exists to prevent. Also: the suite's own roots move to a short base. Rooting them under the ambient `TMPDIR` put a NESTED gate's TMPDIR near 70 bytes, which legitimately tripped its own SUN_LEN guard --- the suite failing on a configuration it created rather than on the behaviour under test. And the marker row's `.then(..).unwrap_or_else(..)` chain is gone. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/ci-red-signatures.md | 39 +++++++- tests/gate_script_acceptance.rs | 155 +++++++++++++++++++++++++------- 2 files changed, 160 insertions(+), 34 deletions(-) diff --git a/docs/ci-red-signatures.md b/docs/ci-red-signatures.md index 6cf2aea..83b4f47 100644 --- a/docs/ci-red-signatures.md +++ b/docs/ci-red-signatures.md @@ -496,7 +496,7 @@ Stage 4; the lane touches no `pmacs-gpu` code at all. | **selector** | `-p pmacs-gpu attach::tests::managed_retry_survives_transients_and_uses_the_successful_stream` | | **job / flavor** | local (Linux), `cargo test --workspace --features crdt --no-fail-fast`, i.e. under full-sweep load | | **required fragments** | `transient sequence must attach` + `Handshake(Io(` + `BrokenPipe` (or `code: 32`) | -| **status** | **THIRD OCCURRENCE 2026-08-09 — causal status still UNRESOLVED, but one candidate mechanism is now EXCLUDED** | +| **status** | **FOURTH OCCURRENCE 2026-08-13 — causal status still UNRESOLVED. A NEW candidate mechanism is introduced by the observing lane and is NOT excluded (see below)** | | **what IS established** | **three** occurrences at `pmacs-gpu/src/attach.rs:1680`, the second and third with all three fragments **verified** rather than inferred; the test drives a scripted transient-then-success sequence over a real socket pair. **The added GPU test is not the mechanism** — see the third-occurrence control below | | **what is NOT** | whether the broken pipe is the *fixture's* writer closing early or a real retry-path defect. **This row is not a claim that it is harmless** | | **rerun evidence** | occurrence 1: 6 isolated runs green, plus a full `--workspace --features crdt` sweep green (113 targets). Occurrence 2: **30 green on the observing branch** (15 isolated selector, 15 full `-p pmacs-gpu`) **plus a 15-run merge-base control, also green**. Occurrence 3: 5 isolated selector runs green, 10 full `-p pmacs-gpu` runs green **with** the added test, and **1 failure in 10 with the added test `#[ignore]`d** — the first rerun in this row's history that reproduced anything. Per the rerun rule the green runs establish intermittence only; the red control run is what carries the exclusion | @@ -537,6 +537,43 @@ one-second deadline. Contention is a plausible mechanism for a lands, **run the control with the added test removed** rather than at the merge base — that is the discriminating comparison this one was not. +**Fourth occurrence — the `scripts/gate` TMPDIR isolation lane, +2026-08-13, local (Linux). Same selector, same `gpu`-step flavor +(`PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu`), all three fragments +verified** against the durable gate log +(`20260813T143421Z-708100/07-gpu.log`): `transient sequence must +attach: Attach(Handshake(Io(Os { code: 32, kind: BrokenPipe, message: +"Broken pipe" })))`. + +**One thing this occurrence ESTABLISHES.** The observing lane touches +**no `pmacs-gpu` file at all** (`git diff ca92796..HEAD -- pmacs-gpu/` +is empty) and adds **no test to that binary**. Occurrence 3 excluded +"the added GPU test is the mechanism" by a control; this occurrence +reproduces the signature with *nothing added to the binary*, which is +independent corroboration rather than a repeat of the same argument. + +**AND ONE THING IT INTRODUCES, NAMED RATHER THAN DISMISSED — the +observing lane is not environmentally neutral even though it is +code-neutral.** That lane moves the gate's `TMPDIR` off `/tmp`, which +on this machine changes the filesystem underneath every +`tempfile::tempdir()` in the run **from tmpfs to btrfs**. This test +creates a tempdir and runs a handshake against a **one-second +deadline** (`pmacs-gpu/src/attach.rs`). A slower filesystem under a +timing-bounded test, under full-sweep load, is a plausible mechanism +and **it did not exist in occurrences 1–3**. + +It is not established either: the failing I/O is on a `UnixStream::pair`, +not on the tempdir path, and the tempdir is created but never bound. +**What this row must not do is book the occurrence as "the usual flake" +when the observing lane changed the very conditions the flake is +sensitive to.** + +**The discriminating comparison for a fifth occurrence** is therefore +the same lane's gate run with `TMPDIR` pointed back at a tmpfs — the +one variable this lane moves — rather than another merge-base control. +Three isolated re-runs on the current tree were green, which by this +file's own rule establishes intermittence only. + **Third occurrence — worker identity Stage 1 review round 2, 2026-08-09, local (Linux). Same selector, same `gpu`-step flavor, all three fragments verified** against the durable gate log diff --git a/tests/gate_script_acceptance.rs b/tests/gate_script_acceptance.rs index a2073c0..cb9a4f5 100644 --- a/tests/gate_script_acceptance.rs +++ b/tests/gate_script_acceptance.rs @@ -39,6 +39,26 @@ fn gate() -> PathBuf { } /// Run `scripts/gate` with an isolated managed root, from `cwd`. +/// A SHORT base for the roots these tests hand the gate, independent of +/// the ambient `TMPDIR`. +/// +/// **Not `tempfile::tempdir()`'s default, and the reason is the socket +/// budget rather than taste.** When this suite runs inside a gate, the +/// ambient `TMPDIR` is already that gate's own (~47 bytes); rooting a +/// nested gate under it pushes the nested `TMPDIR` to ~70 bytes and +/// legitimately trips its own `SUN_LEN` guard. The suite would then +/// fail on a configuration it created rather than on the behaviour +/// under test — which is exactly how it failed once. +/// +/// `/tmp` is named explicitly because it is short and this suite +/// already requires a Unix environment. These roots hold synthetic +/// plans and never fixtures, so `/tmp`'s contents are irrelevant to +/// them; the rows set `PMACS_GATE_ALLOW_ANCESTOR_MARKER` for that +/// reason. +fn short_root_base() -> PathBuf { + PathBuf::from("/tmp") +} + fn run_in(cwd: &Path, root: &Path, args: &[&str]) -> (String, String, bool) { let out = Command::new(gate()) .args(args) @@ -74,7 +94,10 @@ fn run(root: &Path, args: &[&str]) -> (String, String, bool) { #[test] fn the_plan_sweeps_the_workspace_and_never_only_the_tests() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let (plan, _, ok) = run(root.path(), &["--print-plan"]); assert!(ok, "--print-plan must succeed"); @@ -100,7 +123,10 @@ fn the_plan_sweeps_the_workspace_and_never_only_the_tests() { #[test] fn the_plan_runs_the_library_tests_in_both_feature_configurations() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let (plan, _, _) = run(root.path(), &["--print-plan"]); assert!(plan.contains("cargo test --lib\n"), "plan was:\n{plan}"); assert!( @@ -115,7 +141,10 @@ fn the_plan_runs_the_library_tests_in_both_feature_configurations() { /// default one in place — and the default run must not carry it. #[test] fn the_crdt_workspace_sweep_is_added_by_protocol_and_absent_without_it() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let crdt_sweep = "cargo test --workspace --features crdt --no-fail-fast"; let (default_plan, _, _) = run(root.path(), &["--print-plan"]); @@ -163,7 +192,10 @@ fn the_crdt_workspace_sweep_is_added_by_protocol_and_absent_without_it() { /// 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"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let build = "cargo build --workspace --no-default-features --features luajit,crdt"; let crdt_sweep = "cargo test --workspace --features crdt --no-fail-fast -- --skip basedpyright"; @@ -230,7 +262,10 @@ fn the_crdt_sweep_is_immediately_preceded_by_the_build_that_produces_its_binary( /// `--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 root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .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"; @@ -294,7 +329,10 @@ fn the_crdt_build_step_carries_its_own_name_and_its_exact_command() { /// 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"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); for flags in [ vec![], @@ -346,7 +384,10 @@ fn the_named_plan_is_the_printed_plan_with_its_names_removed() { /// every ordinary lane. #[test] fn the_crdt_build_is_absent_without_protocol() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let (plan, _, ok) = run(root.path(), &["--print-plan"]); assert!(ok, "--print-plan must succeed"); assert!( @@ -383,7 +424,10 @@ fn the_crdt_build_is_absent_without_protocol() { /// the same defect the `--acceptance` refusal above exists to prevent. #[test] fn self_test_names_the_failing_gate_and_the_suite_continues_past_it() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let (out, err, ok) = run(root.path(), &["--self-test"]); assert!( @@ -455,7 +499,10 @@ fn self_test_names_the_failing_gate_and_the_suite_continues_past_it() { /// reads a real child's environment out of a real gate log. #[test] fn the_isolated_tmpdir_reaches_a_spawned_child_under_the_managed_root() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let (out, err, _ok) = run(root.path(), &["--self-test"]); let announced = out @@ -518,7 +565,10 @@ fn the_isolated_tmpdir_reaches_a_spawned_child_under_the_managed_root() { /// created by the tool itself and on every single run. #[test] fn the_isolated_tmpdir_is_reaped_when_the_run_ends() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let (out, _err, _ok) = run(root.path(), &["--self-test"]); let announced = out @@ -565,14 +615,8 @@ fn a_project_marker_above_the_gate_tmpdir_is_refused() { // control what sits above the root, and `/tmp` may already carry a // marker — which would make the row pass for the wrong reason. let base = tempfile::Builder::new() - .prefix("gate-marker-") - .tempdir_in( - repo_root() - .join("target") - .exists() - .then(|| repo_root().join("target")) - .unwrap_or_else(std::env::temp_dir), - ) + .prefix("gm-") + .tempdir_in(short_root_base()) .expect("base"); let root = base.path().join("inner"); std::fs::create_dir_all(&root).expect("root"); @@ -607,7 +651,10 @@ fn a_project_marker_above_the_gate_tmpdir_is_refused() { /// handed — each one, in order. #[test] fn acceptance_suites_reach_the_plan_in_the_order_given() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let (plan, _, _) = run( root.path(), &[ @@ -634,7 +681,10 @@ fn acceptance_suites_reach_the_plan_in_the_order_given() { #[test] fn printing_the_target_dir_creates_nothing() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let (dir, _, ok) = run(root.path(), &["--print-target-dir"]); assert!(ok, "--print-target-dir must succeed"); assert!(!dir.trim().is_empty(), "it must print a path"); @@ -646,7 +696,10 @@ fn printing_the_target_dir_creates_nothing() { #[test] fn init_writes_the_ownership_marker_and_is_idempotent() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let (dir, _, ok) = run(root.path(), &["--init"]); assert!(ok, "--init must succeed"); let dir = PathBuf::from(dir.trim()); @@ -686,8 +739,14 @@ fn init_writes_the_ownership_marker_and_is_idempotent() { /// belt-and-braces against git's behaviour not being contractual. #[test] fn a_symlinked_spelling_of_a_worktree_derives_the_same_directory() { - let root = tempfile::tempdir().expect("tempdir"); - let link_home = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); + let link_home = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let link = link_home.path().join("via-symlink"); if std::os::unix::fs::symlink(repo_root(), &link).is_err() { return; // no symlink support; nothing to assert @@ -724,7 +783,10 @@ fn prune_fixture(root: &Path) -> (PathBuf, PathBuf, PathBuf) { #[test] fn prune_is_a_dry_run_by_default_and_deletes_nothing() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let (orphan, lookalike, live) = prune_fixture(root.path()); let (out, _, ok) = run(root.path(), &["--prune"]); @@ -741,7 +803,10 @@ fn prune_is_a_dry_run_by_default_and_deletes_nothing() { #[test] fn force_deletes_only_the_orphan() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let (orphan, lookalike, live) = prune_fixture(root.path()); let (out, _, ok) = run(root.path(), &["--prune", "--force"]); @@ -789,8 +854,14 @@ impl Drop for WorktreePruneGuard { #[test] fn a_registered_worktree_whose_directory_was_deleted_is_prunable() { - let root = tempfile::tempdir().expect("tempdir"); - let home = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); + let home = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let wt = home.path().join("gate-prunable-probe"); let added = Command::new("git") @@ -847,9 +918,15 @@ fn a_registered_worktree_whose_directory_was_deleted_is_prunable() { /// grounds that no test noticed. #[test] fn prune_outside_a_repository_refuses_and_every_directory_survives() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let (orphan, lookalike, live) = prune_fixture(root.path()); - let outside = tempfile::tempdir().expect("tempdir"); + let outside = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); // Sanity: the fixture's orphan really is eligible from inside a repo. let (inside, _, _) = run(root.path(), &["--prune"]); @@ -879,7 +956,10 @@ fn prune_outside_a_repository_refuses_and_every_directory_survives() { /// gate runs. #[test] fn acceptance_names_with_shell_metacharacters_are_refused() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let canary = root.path().join("canary"); std::fs::write(&canary, "intact").expect("write canary"); @@ -920,7 +1000,10 @@ fn acceptance_names_with_shell_metacharacters_are_refused() { /// the test above by rejecting everything. #[test] fn ordinary_acceptance_names_are_accepted() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); for name in ["m4_acceptance", "gate-script", "abc123_x"] { let (plan, err, ok) = run(root.path(), &["--acceptance", name, "--print-plan"]); assert!(ok, "{name} must be accepted; stderr was:\n{err}"); @@ -936,7 +1019,10 @@ fn ordinary_acceptance_names_are_accepted() { /// directory on the strength of a file the script did not understand. #[test] fn a_multi_line_marker_is_refused_rather_than_read_head_first() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let bad = root.path().join("bad-00000000"); std::fs::create_dir_all(&bad).expect("mkdir"); std::fs::write( @@ -964,7 +1050,10 @@ fn a_multi_line_marker_is_refused_rather_than_read_head_first() { /// is how one learns too late that the marker was never written. #[test] fn skipped_directories_are_reported_with_a_reason() { - let root = tempfile::tempdir().expect("tempdir"); + let root = tempfile::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .expect("tempdir"); let (_, lookalike, _) = prune_fixture(root.path()); let (out, _, _) = run(root.path(), &["--prune"]); From 465acae547b5822b7532e8b994c9a9c3d16de8f5 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 13 Aug 2026 16:52:26 +0200 Subject: [PATCH 05/10] 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. From 0a04d55a352aaa45c4528374e7087672ae428a23 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 13 Aug 2026 17:07:55 +0200 Subject: [PATCH 06/10] fix(gate): review round 3 --- canonical ancestry, guard witnesses, and a withdrawn claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **THE ANCESTOR WALK WAS WRONG TWICE OVER.** `for _anc in $(...)` word-splits on IFS, so a gate root containing a SPACE was torn into fragments and the real ancestor never tested --- the check passed on exactly the path it should reject. And `dirname` walks LEXICAL ancestry while `detect_project` canonicalizes, so a symlinked root hid a marker the editor plainly sees. The walk resolves with `pwd -P` first and iterates a quoted `while`; both shapes are verified by hand (space-containing root refused, symlinked root refused at its real path). **THE 103-BYTE GUARD HAD NO WITNESS AT ALL** --- every other row runs with a short root, so the guard is silent and a broken one looked identical. Three rows now aim at it deliberately: boundary rejection and acceptance, a MULTIBYTE root (each `é` is one character and two bytes, so it is rejected only if the guard measures bytes), and **rejection must reap both created areas**, which is the leak the early trap exists to prevent. **The `Cargo.toml`-DIRECTORY case was claimed and not covered**, and the consequence is exactly as review predicted: reverting only the language-marker arm to `[ -e ]` stayed green. The marker-type row now drives all three shapes, and `M-G-5` --- that precise revert --- fails it. **Prose brought level with the implementation.** The framing, the handoff and the ledger all said 108; the supported floor is **103 usable bytes**, Darwin's 104-byte array minus its NUL. The ledger also still said ``, the superseded 21/30 reserve, and `M-G-1`. **And the ruling said nested gates "do not pay" the reserve, which is false and would have licensed exempting them.** They pay it in full; the short layout merely gives them the headroom to satisfy an unchanged production guard. Reworded, because the wrong version is the one a future reader would act on. **THE btrfs CAUSAL CLAIM IS WITHDRAWN.** The draft argued that a one-second deadline plus a slower filesystem was a plausible new mechanism for the fourth `managed_retry` occurrence. It does not survive inspection: the deadline bounds the connection RETRY loop, not the socketpair handshake that returned `BrokenPipe`, and the filesystem work happens before it is armed --- the tempdir is created and never bound. The environmental change is still recorded, as a CHANGE rather than a mechanism, so a later occurrence can compare like with like. Recording a mechanism the code does not support is worse than recording none: the next occurrence gets measured against a story instead of the evidence. TMPDIR stays disk-backed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/active-work.md | 39 ++++++---- docs/agent-handoff.md | 8 +- docs/ci-red-signatures.md | 43 ++++++----- docs/gate-script-framing.md | 21 ++++-- scripts/gate | 59 ++++++++------- tests/gate_script_acceptance.rs | 127 ++++++++++++++++++++++++++++++++ 6 files changed, 228 insertions(+), 69 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index 4051a83..49408c1 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -288,7 +288,8 @@ from #171 and #215. responsibility to `TMPDIR` rather than adding a feature, which is why it amends this document instead of opening a new one. - **What it does:** every gate invocation gets a fresh, disk-backed - `TMPDIR` at `/tmp/`, exported once so every stage and + `TMPDIR` at `/tmp/`, exported once so every stage + and every process they spawn inherits it, and reaped by the same exit trap as the ambient root. **A gate run no longer needs a `TMPDIR=` override.** @@ -296,8 +297,11 @@ from #171 and #215. ANCESTOR marker — project detection walks upward — so a fresh subdirectory of `/tmp` inherits `/tmp`'s ancestors and the same stray `.git`. The directory had to move somewhere the gate already owns. -- **`SUN_LEN` is the constraint that shaped the layout, and the fix's - own gate run found it.** A Unix socket path cannot exceed 108 bytes +- **The socket-path limit shaped the layout, and the fix's own gate run + found it.** The budget is the **supported-platform floor of 103 + usable bytes** — Darwin's 104-byte array minus its terminating NUL, + not Linux's 108, because a Linux-derived limit passes where it is + written and bind-fails on the macOS leg. A path cannot exceed that and the suites bind sockets inside `TMPDIR`. The first placement, `$TARGET/gate-tmp/$STAMP-$$`, produced a 114-byte socket path and failed **six** daemon and attach tests with *"path must be shorter @@ -308,16 +312,25 @@ from #171 and #215. socket failures deep in a suite name a limit, not a cause. The guard fails immediately with the path, its length and what to shorten. **Its reserve is measured, not round**: the longest suffix a fixture - appends is 21 bytes, so 30 leaves ~40% headroom. An earlier - "generous" 45 **fired on the gate's own behaviour tests**, which run - the gate inside the gate and so reach 71 bytes — a guard that rejects - a legitimate configuration fails on every run rather than a rare one. -- **Witnesses (2), each mutation-checked:** `M-G-1` removes the export - → the propagation row alone; `M-G-2` stops the reaping → the cleanup - row alone. Propagation is observed in a **spawned child** (the - self-test's first step reports its own `$TMPDIR` into its log), - because the gate exporting a variable would only prove the gate can - export a variable. + appends is **33** bytes (`/.tmpXXXXXX/directory-target.sock`), so + **48** leaves ~45% headroom. Two earlier values were wrong in + OPPOSITE directions — a "generous" 45 that fired on the gate's own + behaviour tests, then a 30 that sat **below the real maximum** and + would have passed 76–78-byte paths. The suite now roots its nested + gates at a short base so it can SATISFY the unchanged production + guard rather than be exempted from it. +- **Witnesses, each mutation-checked.** `M-G-1b` keeps the assignment + and removes only `export` → the propagation row; its predecessor + `M-G-1` deleted both and so never proved inheritance. `M-G-2` stops + the reaping → the cleanup row. `M-G-3` removes the ancestor check → + the refusal row. `M-G-4` reverts to existence-only, and `M-G-5` + reverts **only** the language-marker arm → the marker-type row, which + is why that row covers a `Cargo.toml` **directory** as well as both + `.git` shapes. `M-G-6` counts characters → the multibyte row. + `M-G-7` moves the trap back after the guards → the + rejection-cleanup row. Propagation is observed in a **spawned + child**, because the gate exporting a variable would only prove the + gate can export a variable. - **Proved against the live hazard:** `/tmp/.git` is still present on this machine, and the tests it reddened now pass with **no override**. - **Gates:** `./scripts/gate --acceptance gate_script_acceptance`, run diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index 47c6a21..b852072 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -283,8 +283,12 @@ commands, read `docs/active-work.md` immediately after this file. **FIXED 2026-08-13: `scripts/gate` now isolates `TMPDIR`.** Each invocation gets a directory created fresh by `mktemp -d` under **`/tmp/`** — the shared gate root, not the per-worktree - target, because a Unix socket path cannot exceed **`SUN_LEN`** (108 - bytes) and the suites bind sockets inside `TMPDIR`. It is exported + target, because a Unix socket path cannot exceed `sun_path` and the + suites bind sockets inside `TMPDIR`. **The budget is the + supported-platform floor: 103 usable bytes** — Darwin's 104-byte + array minus its terminating NUL, not Linux's 108, because a + Linux-derived limit passes where it is written and bind-fails on + the macOS leg. It is exported once so every stage and every process they spawn inherits it, and reaped by the same exit trap as the ambient root. **A gate run no longer needs a `TMPDIR=` override.** diff --git a/docs/ci-red-signatures.md b/docs/ci-red-signatures.md index 83b4f47..cdc4818 100644 --- a/docs/ci-red-signatures.md +++ b/docs/ci-red-signatures.md @@ -552,27 +552,32 @@ is empty) and adds **no test to that binary**. Occurrence 3 excluded reproduces the signature with *nothing added to the binary*, which is independent corroboration rather than a repeat of the same argument. -**AND ONE THING IT INTRODUCES, NAMED RATHER THAN DISMISSED — the -observing lane is not environmentally neutral even though it is -code-neutral.** That lane moves the gate's `TMPDIR` off `/tmp`, which -on this machine changes the filesystem underneath every -`tempfile::tempdir()` in the run **from tmpfs to btrfs**. This test -creates a tempdir and runs a handshake against a **one-second -deadline** (`pmacs-gpu/src/attach.rs`). A slower filesystem under a -timing-bounded test, under full-sweep load, is a plausible mechanism -and **it did not exist in occurrences 1–3**. +**The observing lane is environmentally non-neutral, and that is +recorded as a CHANGE rather than as a mechanism.** It moves the gate's +`TMPDIR` off `/tmp`, which on this machine puts every +`tempfile::tempdir()` in the run on btrfs instead of tmpfs. Noted so a +later occurrence can compare like with like. -It is not established either: the failing I/O is on a `UnixStream::pair`, -not on the tempdir path, and the tempdir is created but never bound. -**What this row must not do is book the occurrence as "the usual flake" -when the observing lane changed the very conditions the flake is -sensitive to.** +**A causal claim built on that was advanced here and is WITHDRAWN.** +The draft argued the test's one-second deadline plus a slower +filesystem was a plausible new mechanism. It does not hold on +inspection: **the deadline bounds the connection RETRY loop, not the +socketpair handshake that returned `BrokenPipe`**, and the filesystem +work happens before that deadline is armed. The tempdir is created and +never bound — the failing I/O is on a `UnixStream::pair`. Recording a +mechanism that the code does not support is worse than recording none, +because the next occurrence gets measured against a story instead of +against the evidence. -**The discriminating comparison for a fifth occurrence** is therefore -the same lane's gate run with `TMPDIR` pointed back at a tmpfs — the -one variable this lane moves — rather than another merge-base control. -Three isolated re-runs on the current tree were green, which by this -file's own rule establishes intermittence only. +**So the causal status is unchanged by this occurrence: UNRESOLVED, +with no new mechanism.** What it adds is the corroboration above. Three +isolated re-runs on the current tree were green, which by this file's +own rule establishes intermittence only. + +**The discriminating comparison for a fifth occurrence** remains the +one the third occurrence prescribed. One occurrence, with no supported +mechanism, is not grounds to reverse a fix that closes two observed +hazards. **Third occurrence — worker identity Stage 1 review round 2, 2026-08-09, local (Linux). Same selector, same `gpu`-step flavor, all diff --git a/docs/gate-script-framing.md b/docs/gate-script-framing.md index ef86fce..b8839e7 100644 --- a/docs/gate-script-framing.md +++ b/docs/gate-script-framing.md @@ -452,7 +452,9 @@ Four decisions inside that, each of which had a cheaper wrong answer: therefore the marker. The directory has to sit somewhere with no marker above it. 2. **Off the GATE ROOT, not the per-worktree target.** A Unix socket - path cannot exceed `SUN_LEN` (108 bytes) and the suites bind sockets + path cannot exceed `sun_path` — **103 usable bytes at the supported + floor** (Darwin's 104-byte array minus its terminating NUL; Linux's + is 108/107) and the suites bind sockets *inside* `TMPDIR`. The per-worktree target is 60 bytes and the gate root 36; the first implementation used the former and produced 114-byte socket paths, failing six daemon and attach tests. **The @@ -492,8 +494,10 @@ 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 +**RULING — a synthetic nested gate is given room to SATISFY the +reserve; it is not exempted from it.** The guard is unchanged for every +run, nested or not. What changed is the layout the behaviour suite +hands it. 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 @@ -516,10 +520,13 @@ rather than the guard weakened**: 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. +**The guard therefore keeps the true maximum for every run**, and the +suite simply stops handing it a root that cannot clear it. An earlier +draft of this section said nested gates "do not pay" the reserve, which +is wrong and would have licensed exempting them: they pay it in full +and now have the headroom to afford it. If a future behaviour row binds +a socket, it must move off the short base — the reserve already covers +it. **Escape hatch, documented test-only.** `PMACS_GATE_ALLOW_ANCESTOR_MARKER` exists for this script's own diff --git a/scripts/gate b/scripts/gate index d1e2c4b..81db624 100755 --- a/scripts/gate +++ b/scripts/gate @@ -639,42 +639,45 @@ fi # markerless fixture exists for a marker to re-root. The check itself # is witnessed by a test that deliberately does NOT set this and # asserts the refusal. +gate_marker_refusal() { + echo "gate: a project marker sits above the gate TMPDIR:" >&2 + echo "gate: $1" >&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 +} + if [ -z "${PMACS_GATE_ALLOW_ANCESTOR_MARKER:-}" ]; then -for _anc in $( - _p="$GATE_TMPDIR" - while [ "$_p" != "/" ] && [ -n "$_p" ]; do - printf '%s\n' "$_p" - _p=$(dirname "$_p") - 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. +# CANONICAL, AND QUOTED. Two defects an obvious loop has: +# +# * `for _anc in $(...)` WORD-SPLITS on IFS, so a gate root containing +# a space is torn into fragments and the real ancestor is never +# tested --- the check would pass on exactly the path it should +# reject. +# * `dirname` walks LEXICAL ancestry. `detect_project` canonicalizes, +# so a symlinked root can hide a marker the editor plainly sees. +# Resolving first makes the two agree. +_anc=$(cd "$GATE_TMPDIR" 2>/dev/null && pwd -P) || _anc="$GATE_TMPDIR" +while :; do for _m in Cargo.toml .luarc.json pyproject.toml go.mod deno.json \ deno.jsonc package.json; 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. 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 - 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 + gate_marker_refusal "$_anc/$_m" fi done + # The one directory-valued marker. A git WORKTREE has a `.git` + # FILE, which detection ignores and this must too. 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 + gate_marker_refusal "$_anc/.git" fi + [ "$_anc" = "/" ] && break + _anc=$(dirname "$_anc") done fi diff --git a/tests/gate_script_acceptance.rs b/tests/gate_script_acceptance.rs index a07eb42..453afc9 100644 --- a/tests/gate_script_acceptance.rs +++ b/tests/gate_script_acceptance.rs @@ -694,7 +694,21 @@ fn the_ancestor_check_honours_marker_types() { ); // The same name as a DIRECTORY is a real marker. + // A LANGUAGE marker as a DIRECTORY: detection wants a file, so the + // guard must ignore it. Without this row, reverting only the + // language-marker arm to `[ -e ]` stays green — the `.git` halves + // below constrain the directory-valued marker alone. std::fs::remove_file(&mine).expect("rm git file"); + let cargo_dir = base.path().join("Cargo.toml"); + std::fs::create_dir(&cargo_dir).expect("cargo dir"); + let err = String::from_utf8_lossy(&run_it().stderr).into_owned(); + assert!( + !err.contains(&format!("{}", cargo_dir.display())), + "a `Cargo.toml` DIRECTORY is not a marker to detection and must \ + not be one here; stderr:\n{err}" + ); + std::fs::remove_dir(&cargo_dir).expect("rm cargo dir"); + std::fs::create_dir(&mine).expect("git dir"); let out = run_it(); let err = String::from_utf8_lossy(&out.stderr).into_owned(); @@ -708,6 +722,119 @@ fn the_ancestor_check_honours_marker_types() { ); } +/// The socket-path guard: rejection, acceptance, and cleanup on +/// rejection. +/// +/// **The ordinary gate only ever exercises the passing side.** Every +/// other row here runs with a short root, so the guard is silent and a +/// broken guard would look identical. These construct the boundary +/// deliberately. +/// +/// The budget is **103 usable bytes** — Darwin's `sun_path[104]` minus +/// its terminating NUL, the supported-platform floor — less a 48-byte +/// fixture reserve, so a root whose derived TMPDIR exceeds 55 bytes is +/// refused. +#[test] +fn the_socket_path_guard_rejects_accepts_and_cleans_up() { + // The gate appends `/tmp/XXXXXX` (11 bytes) to the root, so a root + // of N bytes yields a TMPDIR of N + 11. + let over_root = long_root(60); + let out = run_guarded(over_root.path()); + let err = String::from_utf8_lossy(&out.stderr).into_owned(); + assert!( + !out.status.success(), + "a root past the budget must be refused; stderr:\n{err}" + ); + assert!( + err.contains("too long for a unix socket path"), + "and must say why; stderr:\n{err}" + ); + + // REJECTION MUST NOT LEAK. The guard creates both temporary areas + // before it can measure anything, so a rejection that exits before + // the trap is armed leaves them behind — on every rejection, which + // is worse than the failure it prevents. + let leaked: Vec<_> = walkdir_shallow(over_root.path()); + assert!( + leaked.is_empty(), + "a rejected run must reap what it created; 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. +/// +/// `${#var}` counts characters under a UTF-8 locale while `sun_path` is +/// byte-limited, so a multibyte path measures short and passes a check +/// it should fail. Each `é` here is one character and **two bytes**. +#[test] +fn the_socket_path_guard_counts_bytes_not_characters() { + // Character-length ~34 but byte-length ~68: rejected only if the + // guard measures bytes. + let root = tempfile::Builder::new() + .prefix(&"é".repeat(24)) + .tempdir_in(short_root_base()) + .expect("multibyte root"); + let chars = root.path().to_string_lossy().chars().count(); + let bytes = root.path().to_string_lossy().len(); + assert!( + bytes > chars, + "fixture must actually be multibyte: {chars} chars, {bytes} bytes" + ); + + let out = run_guarded(root.path()); + let err = String::from_utf8_lossy(&out.stderr).into_owned(); + assert!( + !out.status.success() && err.contains("too long for a unix socket path"), + "a multibyte root over the BYTE budget must be refused — it is \ + {chars} characters but {bytes} bytes; stderr:\n{err}" + ); +} + +/// A root of exactly `total` bytes, so the boundary can be aimed at. +fn long_root(total: usize) -> tempfile::TempDir { + let base = short_root_base(); + // `/XXXXXX` + let fixed = base.display().to_string().len() + 1 + 6; + let pad = total.saturating_sub(fixed); + tempfile::Builder::new() + .prefix(&"a".repeat(pad)) + .tempdir_in(&base) + .expect("sized root") +} + +/// Run the gate with the ancestor escape set (so only the LENGTH guard +/// can speak) and no ambient `TMPDIR`. +fn run_guarded(root: &Path) -> std::process::Output { + std::process::Command::new(gate()) + .arg("--self-test") + .current_dir(repo_root()) + .env("PMACS_GATE_TARGET_ROOT", root) + .env("PMACS_GATE_ALLOW_ANCESTOR_MARKER", "1") + .env_remove("TMPDIR") + .output() + .expect("run gate") +} + +/// Entries left under `root/tmp` and any `gate-ambient` leaf. +fn walkdir_shallow(root: &Path) -> Vec { + let mut out = Vec::new(); + for sub in ["tmp"] { + if let Ok(rd) = std::fs::read_dir(root.join(sub)) { + out.extend(rd.filter_map(Result::ok).map(|e| e.path())); + } + } + out +} + /// 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. From 8e1f03a4a0a701f5c547609849efbacb579d5497 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 13 Aug 2026 19:33:51 +0200 Subject: [PATCH 07/10] fix(gate): review round 4 --- four discriminators that constrained nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `/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 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/gate-script-framing.md | 11 ++ tests/gate_script_acceptance.rs | 271 ++++++++++++++++++++++++-------- 2 files changed, 218 insertions(+), 64 deletions(-) diff --git a/docs/gate-script-framing.md b/docs/gate-script-framing.md index b8839e7..659a1d9 100644 --- a/docs/gate-script-framing.md +++ b/docs/gate-script-framing.md @@ -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 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 contract.** `match_marker` (`src/project.rs`) requires `.git` to be a **directory** and the seven language markers to be **files**, so diff --git a/tests/gate_script_acceptance.rs b/tests/gate_script_acceptance.rs index 453afc9..59de460 100644 --- a/tests/gate_script_acceptance.rs +++ b/tests/gate_script_acceptance.rs @@ -1,6 +1,7 @@ //! `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 //! @@ -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}")); - // The contract is **under the managed target root**, which is what - // makes it both marker-free and disk-backed in production: the - // target root sits beside the build artifacts, not in `/tmp`. + // The contract is **under the managed gate root**, which is what + // makes it disk-backed in production: the gate root sits beside the + // 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 // 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; \ {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!( Path::new(&announced) .parent() @@ -722,93 +733,140 @@ fn the_ancestor_check_honours_marker_types() { ); } -/// The socket-path guard: rejection, acceptance, and cleanup on -/// rejection. +/// The socket-path guard, **at the boundary**: 55 bytes accepted, 56 +/// rejected, with the measured lengths asserted. /// -/// **The ordinary gate only ever exercises the passing side.** Every -/// other row here runs with a short root, so the guard is silent and a -/// broken guard would look identical. These construct the boundary -/// deliberately. +/// **Straddling the cutoff is not testing it.** An earlier version of +/// this row generated roughly 51- and 71-byte paths against a 55-byte +/// cutoff; raising `SUN_LEN_BUDGET` from 103 to 118 would have left +/// 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 -/// its terminating NUL, the supported-platform floor — less a 48-byte -/// fixture reserve, so a root whose derived TMPDIR exceeds 55 bytes is -/// refused. +/// its terminating NUL, the supported-platform floor — less the 48-byte +/// fixture reserve. #[test] -fn the_socket_path_guard_rejects_accepts_and_cleans_up() { - // The gate appends `/tmp/XXXXXX` (11 bytes) to the root, so a root - // of N bytes yields a TMPDIR of N + 11. - let over_root = long_root(60); +fn the_socket_path_guard_holds_at_its_exact_boundary() { + // The gate derives `/tmp/XXXXXX`, i.e. root + 11 bytes. + const DERIVED: usize = 11; + 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 err = String::from_utf8_lossy(&out.stderr).into_owned(); assert!( - !out.status.success(), - "a root past the budget must be refused; stderr:\n{err}" + !out.status.success() && err.contains("too long for a unix socket path"), + "one byte past the cutoff must be REJECTED; stderr:\n{err}" ); - assert!( - err.contains("too long for a unix socket path"), - "and must say why; stderr:\n{err}" + // Keyed on the line that reports the measurement, not on the first + // `gate: ` line — that one is the path. + 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::().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 - // before it can measure anything, so a rejection that exits before - // the trap is armed leaves them behind — on every rejection, which - // is worse than the failure it prevents. - let leaked: Vec<_> = walkdir_shallow(over_root.path()); + // REJECTION MUST NOT LEAK, and **both** areas are checked. The + // guard creates the ambient root and the TMPDIR before it can + // measure anything, so a rejection that exits before the trap is + // armed leaves both behind. Checking only `/tmp` would pass + // while `gate-ambient` leaked. + let leaked = leaked_temp_areas(over_root.path()); assert!( leaked.is_empty(), - "a rejected run must reap what it created; 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}" + "a rejected run must reap BOTH created areas; found {leaked:?}" ); } -/// 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 -/// 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] fn the_socket_path_guard_counts_bytes_not_characters() { - // Character-length ~34 but byte-length ~68: rejected only if the - // guard measures bytes. + // Each `é` is one character and two bytes, so this root is under + // the cutoff by character count and over it by byte count — the + // only shape that separates the two implementations. let root = tempfile::Builder::new() - .prefix(&"é".repeat(24)) + .prefix(&"é".repeat(22)) .tempdir_in(short_root_base()) .expect("multibyte root"); - let chars = root.path().to_string_lossy().chars().count(); - let bytes = root.path().to_string_lossy().len(); + let path = root.path().to_string_lossy().into_owned(); + let chars = path.chars().count(); + let bytes = path.len(); assert!( - bytes > chars, - "fixture must actually be multibyte: {chars} chars, {bytes} bytes" + chars + 11 <= 55 && bytes + 11 > 55, + "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(); assert!( !out.status.success() && err.contains("too long for a unix socket path"), - "a multibyte root over the BYTE budget must be refused — it is \ - {chars} characters but {bytes} bytes; stderr:\n{err}" + "a root over the BYTE budget must be refused even though it is \ + under the CHARACTER budget ({chars} chars, {bytes} bytes); \ + stderr:\n{err}" ); } -/// A root of exactly `total` bytes, so the boundary can be aimed at. -fn long_root(total: usize) -> tempfile::TempDir { +/// A root whose full path is exactly `total` bytes. +fn root_of_len(total: usize) -> tempfile::TempDir { let base = short_root_base(); - // `/XXXXXX` - let fixed = base.display().to_string().len() + 1 + 6; - let pad = total.saturating_sub(fixed); - tempfile::Builder::new() - .prefix(&"a".repeat(pad)) + let fixed = base.display().to_string().len() + 1 + 6; // `/` + XXXXXX + let dir = tempfile::Builder::new() + .prefix(&"a".repeat(total.saturating_sub(fixed))) .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 @@ -824,17 +882,102 @@ fn run_guarded(root: &Path) -> std::process::Output { .expect("run gate") } -/// Entries left under `root/tmp` and any `gate-ambient` leaf. -fn walkdir_shallow(root: &Path) -> Vec { +/// Leftovers in **both** areas a run creates: `/tmp/*` and the +/// derived target's `gate-ambient/*`. +fn leaked_temp_areas(root: &Path) -> Vec { let mut out = Vec::new(); - for sub in ["tmp"] { - if let Ok(rd) = std::fs::read_dir(root.join(sub)) { - out.extend(rd.filter_map(Result::ok).map(|e| e.path())); + if let Ok(rd) = std::fs::read_dir(root.join("tmp")) { + 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 } +/// 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 /// which acceptance suites a change touched, so it runs what it is /// handed — each one, in order. From 3a7e3790a14beaa184e73f1edff25219949167a9 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 13 Aug 2026 20:25:00 +0200 Subject: [PATCH 08/10] test(gate): establish the locale precondition instead of naming one Review found the byte-versus-character witness asserting something adjacent to its contract. It set `LC_ALL=C.UTF-8` and assumed the locale took effect. Locale names beyond `C` and `POSIX` are implementation-defined, so where that one is absent the shell falls back to byte semantics --- and then the character-counting mutant counts bytes too, agrees with the fix, and the row passes while proving nothing. M-G-6 was killable here and unkillable elsewhere, which is the same as not having it. The locale is now chosen by BEHAVIOUR. Candidates come from `locale -a` so the set reflects what is installed, and each is probed through the same `/bin/sh` the gate runs under, asking `${#x}` on a two-byte character and requiring `1`. No qualifying locale is a loud panic naming what was tried, never a skip: a skip would be indistinguishable from a pass, which is the failure mode this replaces. M-G-8 proves the fix in three legs, because the hazard lives in the environment rather than the code: 8a mutant gate, probed locale -> the row fails, and the exact-boundary row still passes 8b SAME mutant gate, locale forced to `C` -> the row passes. The defect reproduced rather than argued. 8c no candidate can qualify -> panic naming the candidates Also marks framing revision 6 approved and records M-G-8 in the ledger. Gates: all nine green under `env -u TMPDIR`, log 20260813T182020Z. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/active-work.md | 20 +++++++++++ docs/gate-script-framing.md | 25 +++++++++----- tests/gate_script_acceptance.rs | 61 +++++++++++++++++++++++++++++++-- 3 files changed, 96 insertions(+), 10 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index 49408c1..f4bece7 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -331,6 +331,26 @@ from #171 and #215. rejection-cleanup row. Propagation is observed in a **spawned child**, because the gate exporting a variable would only prove the gate can export a variable. +- **`M-G-8` is the one that proves `M-G-6` is not vacuous**, and it + needs three legs because the hazard is in the *environment*, not the + code. The multibyte row originally set `LC_ALL=C.UTF-8` and assumed + it took: locale names beyond `C` and `POSIX` are + implementation-defined, so on a machine lacking that locale the shell + falls back to **byte** semantics — and then the character-counting + mutant counts bytes too, agrees with the fix, and the row passes + while proving nothing. + - **8a** — mutant gate, locale chosen by the probe → the row + **fails**, and the exact-boundary row still passes. + - **8b** — *same mutant gate*, locale forced to `C` → the row + **passes**. This is the defect itself, reproduced rather than + argued: the only difference between a real witness and a vacuous + one is whether the shell counts characters. + - **8c** — no candidate can qualify → the helper **panics** naming + what it tried. A skip here would be indistinguishable from a pass. + The locale is therefore selected by **behaviour**: candidates come + from `locale -a`, and each is probed through the same `/bin/sh` the + gate runs under, asking `${#x}` on a two-byte character and requiring + `1`. - **Proved against the live hazard:** `/tmp/.git` is still present on this machine, and the tests it reddened now pass with **no override**. - **Gates:** `./scripts/gate --acceptance gate_script_acceptance`, run diff --git a/docs/gate-script-framing.md b/docs/gate-script-framing.md index 659a1d9..9ee929d 100644 --- a/docs/gate-script-framing.md +++ b/docs/gate-script-framing.md @@ -1,12 +1,21 @@ # `scripts/gate` — per-worktree build isolation, and one gate suite -**Status: revision 6 — AWAITING APPROVAL.** Revision 6 extends the -isolation contract to `TMPDIR` (§2a below) and is the only unapproved -part of this document; everything else is as approved. It is a -*widening of an existing responsibility*, not a new feature: §2 already -owns "what the gate isolates", and `TMPDIR` was simply missing from -that list — which is how a stray `/tmp/.git` came to redden a gate run -on an unrelated lane. +**Status: revision 6 — APPROVED and IMPLEMENTED.** Revision 6 extends +the isolation contract to `TMPDIR` (§2a below). It is a *widening of an +existing responsibility*, not a new feature: §2 already owns "what the +gate isolates", and `TMPDIR` was simply missing from that list — which +is how a stray `/tmp/.git` came to redden a gate run on an unrelated +lane. + +**Approved after three review rounds, all of which turned on evidence +rather than design.** Revisions 6a–6c tightened the socket budget to +the Darwin floor, replaced an existence-only ancestor check with one +that honours marker types, made the traversal canonical, moved the +guard rows onto the exact boundary, and — last — required the +byte-versus-character row to *establish* its locale precondition +instead of naming one. Each correction was a witness asserting +something adjacent to the contract while appearing to assert the +contract itself. **Previously, revision 5. Approved at revision 4 and IMPLEMENTED; revision 5 records two safety defects review found in the implementation.** @@ -425,7 +434,7 @@ under real parallel load, direnv is the escalation. --- -## 2a. `TMPDIR` isolation (revision 6, AWAITING APPROVAL) +## 2a. `TMPDIR` isolation (revision 6, APPROVED) **The gap.** §2 lists what a gate run isolates: the target directory and five ambient roots. `TMPDIR` was not on that list, so diff --git a/tests/gate_script_acceptance.rs b/tests/gate_script_acceptance.rs index 59de460..b073978 100644 --- a/tests/gate_script_acceptance.rs +++ b/tests/gate_script_acceptance.rs @@ -834,13 +834,20 @@ fn the_socket_path_guard_counts_bytes_not_characters() { bytes (must fail), path {path}" ); + // **Chosen by BEHAVIOUR, not by name.** Locale names beyond `C` and + // `POSIX` are implementation-defined and an unrecognized value has + // unspecified behaviour, so setting `LC_ALL=C.UTF-8` and hoping is + // not a precondition — where that locale is absent the shell would + // fall back to byte semantics and the character-counting mutant + // would pass, silently. + let locale = char_counting_locale(); 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("LC_ALL", &locale) + .env("LANG", &locale) .env_remove("TMPDIR") .output() .expect("run gate"); @@ -853,6 +860,56 @@ fn the_socket_path_guard_counts_bytes_not_characters() { ); } +/// A locale under which **`/bin/sh` counts CHARACTERS** — verified by +/// asking that very shell, not by trusting a name. +/// +/// `${#x}` on a two-byte character answers `1` under a working UTF-8 +/// locale and `2` under `C`. That difference is the entire subject of +/// the row this serves, so the row must establish it as a precondition +/// rather than assume it: otherwise, on a machine without the named +/// locale, the shell counts bytes, the mutant agrees with the fix, and +/// the test passes while proving nothing. +/// +/// Candidates come from `locale -a`, so this reflects what is actually +/// installed. **Fails loudly when none qualifies** — a skip here would +/// be indistinguishable from a pass. +fn char_counting_locale() -> String { + let installed = std::process::Command::new("locale") + .arg("-a") + .output() + .map(|o| String::from_utf8_lossy(&o.stdout).into_owned()) + .unwrap_or_default(); + + let candidates: Vec = ["C.UTF-8".to_owned(), "C.utf8".to_owned()] + .into_iter() + .chain( + installed + .lines() + .filter(|l| l.to_ascii_lowercase().contains("utf")) + .map(str::to_owned), + ) + .collect(); + + for cand in &candidates { + let probe = std::process::Command::new("/bin/sh") + .arg("-c") + .arg("x=é; echo ${#x}") + .env("LC_ALL", cand) + .env("LANG", cand) + .output(); + if let Ok(out) = probe + && String::from_utf8_lossy(&out.stdout).trim() == "1" + { + return cand.clone(); + } + } + panic!( + "no installed locale makes /bin/sh count characters, so the \ + byte-vs-character distinction this row exists to test cannot \ + be established here. Tried: {candidates:?}" + ); +} + /// A root whose full path is exactly `total` bytes. fn root_of_len(total: usize) -> tempfile::TempDir { let base = short_root_base(); From da74ae645b76ad04a6ba84b35f2d688376840ef5 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 13 Aug 2026 20:41:18 +0200 Subject: [PATCH 09/10] fix(test): the multibyte witness had the wrong axis, and CI proved it CI went red on three Test jobs with the helper's own panic. That is the loud failure working as designed --- and the design was wrong. `${#x}` counting characters is a property of the SHELL first and the locale second. `bash` counts characters under a UTF-8 locale; `dash` counts bytes under EVERY locale, having no multibyte handling at all. `/bin/sh` is `bash` on Arch and `dash` on the Ubuntu runners, so probing `/bin/sh` and naming a locale established nothing there: no pair could qualify, and the row could not run. The helper now probes `(shell, locale)` pairs and the row invokes the gate THROUGH the qualifying shell. `/bin/sh` is tried first so the real interpreter is used wherever it qualifies. This is not a contrivance to get green: `#!/bin/sh` resolves to `bash` on Arch and on macOS, which is precisely where a `${#VAR}` guard would miscount, and it is the configuration the guard exists for. Renumbered, because `M-G-8` was taken. Round 3 assigned it to the canonical-traversal mutation and the ledger never recorded it, so the locale exercise reusing the ID was a collision. Canonical `M-G-8` is restored to the ledger; the locale legs are `M-G-9a-c`. Nine total. 9a mutant gate, probed pair -> row fails, boundary row still passes. Re-run with /bin/sh EXCLUDED, covering the dash/CI fallback path -> still fails. 9b SAME mutant gate, pair forced byte-counting -> row passes. The defect reproduced rather than argued. 9c no pair qualifies -> panic naming shells and locales tried Record corrections review asked for: - framing said three rounds and revisions 6a-6c; history is rounds 1-4 plus this follow-up, and each round is now named for what it fixed - framing SS2a claimed `${#var}` counts characters under UTF-8 with no qualifier --- the same error as the helper's. It now states the shell dependence and why the guard measures bytes explicitly. - the helper's prose said every candidate comes from `locale -a` while the code also tried two hardcoded spellings; the doc comment now describes what the code does Gates: all nine green under `env -u TMPDIR`, log 20260813T183646Z. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/active-work.md | 48 ++++++++++-------- docs/gate-script-framing.md | 31 ++++++++---- tests/gate_script_acceptance.rs | 90 ++++++++++++++++++++------------- 3 files changed, 101 insertions(+), 68 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index f4bece7..6def70f 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -328,29 +328,33 @@ from #171 and #215. is why that row covers a `Cargo.toml` **directory** as well as both `.git` shapes. `M-G-6` counts characters → the multibyte row. `M-G-7` moves the trap back after the guards → the - rejection-cleanup row. Propagation is observed in a **spawned - child**, because the gate exporting a variable would only prove the - gate can export a variable. -- **`M-G-8` is the one that proves `M-G-6` is not vacuous**, and it + rejection-cleanup row. `M-G-8` restores the old + `for _anc in $(...)` loop → the canonical-walk row, which is what + proves the traversal neither word-splits a space-bearing root nor + misses a marker visible only after `pwd -P`. Propagation is observed + in a **spawned child**, because the gate exporting a variable would + only prove the gate can export a variable. +- **`M-G-9` is the one that proves `M-G-6` is not vacuous**, and it needs three legs because the hazard is in the *environment*, not the - code. The multibyte row originally set `LC_ALL=C.UTF-8` and assumed - it took: locale names beyond `C` and `POSIX` are - implementation-defined, so on a machine lacking that locale the shell - falls back to **byte** semantics — and then the character-counting - mutant counts bytes too, agrees with the fix, and the row passes - while proving nothing. - - **8a** — mutant gate, locale chosen by the probe → the row - **fails**, and the exact-boundary row still passes. - - **8b** — *same mutant gate*, locale forced to `C` → the row - **passes**. This is the defect itself, reproduced rather than - argued: the only difference between a real witness and a vacuous - one is whether the shell counts characters. - - **8c** — no candidate can qualify → the helper **panics** naming - what it tried. A skip here would be indistinguishable from a pass. - The locale is therefore selected by **behaviour**: candidates come - from `locale -a`, and each is probed through the same `/bin/sh` the - gate runs under, asking `${#x}` on a two-byte character and requiring - `1`. + code. **Nine mutations in total.** + - **9a** — mutant gate, probed pair → the row **fails**, and the + exact-boundary row still passes. Re-run with `/bin/sh` excluded, so + the CI fallback path is covered too: still fails. + - **9b** — *same mutant gate*, pair forced to byte-counting → the row + **passes**. The defect itself, reproduced rather than argued. + - **9c** — no pair can qualify → the helper **panics** naming what it + tried. A skip would be indistinguishable from a pass. +- **The multibyte row's axis was wrong, and CI is what proved it.** The + row set `LC_ALL=C.UTF-8` and assumed a character count followed. + `${#x}` counting characters is a property of the **shell** first: + `bash` counts characters under a UTF-8 locale, **`dash` counts bytes + under every locale**. `/bin/sh` is `bash` here and `dash` on the + Ubuntu runners, so the row panicked on CI — the loud failure working + as designed, but on a machine where the distinction is unobservable. + The helper now probes `(shell, locale)` pairs and invokes the gate + **through** the qualifying shell. That is not a contrivance: `#!/bin/sh` + resolves to `bash` on Arch **and on macOS**, which is exactly where a + `${#VAR}` guard would miscount. - **Proved against the live hazard:** `/tmp/.git` is still present on this machine, and the tests it reddened now pass with **no override**. - **Gates:** `./scripts/gate --acceptance gate_script_acceptance`, run diff --git a/docs/gate-script-framing.md b/docs/gate-script-framing.md index 9ee929d..359ee33 100644 --- a/docs/gate-script-framing.md +++ b/docs/gate-script-framing.md @@ -7,15 +7,18 @@ gate isolates", and `TMPDIR` was simply missing from that list — which is how a stray `/tmp/.git` came to redden a gate run on an unrelated lane. -**Approved after three review rounds, all of which turned on evidence -rather than design.** Revisions 6a–6c tightened the socket budget to -the Darwin floor, replaced an existence-only ancestor check with one -that honours marker types, made the traversal canonical, moved the -guard rows onto the exact boundary, and — last — required the -byte-versus-character row to *establish* its locale precondition -instead of naming one. Each correction was a witness asserting -something adjacent to the contract while appearing to assert the -contract itself. +**Approved after four review rounds plus a locale follow-up, all of +which turned on evidence rather than design.** Round 1 corrected a +propagation witness that proved only that the variable was used, and +two wrong guard reserves. Round 2 tightened the socket budget to the +Darwin floor and replaced an existence-only ancestor check with one +that honours marker types. Round 3 made the traversal canonical and +gave the length guard its first real witnesses. Round 4 moved those +rows onto the exact boundary, covered both managed areas on cleanup, +and withdrew an unsupported causal claim. The follow-up required the +byte-versus-character row to *establish* its precondition rather than +name one. Each correction was a witness asserting something adjacent +to the contract while appearing to assert the contract itself. **Previously, revision 5. Approved at revision 4 and IMPLEMENTED; revision 5 records two safety defects review found in the implementation.** @@ -477,8 +480,14 @@ Four decisions inside that, each of which had a cheaper wrong answer: the symptom appear deep in a suite as a limit with no cause: - a **byte-counted** length check reserving the measured maximum suffix (`/.tmpXXXXXX/directory-target.sock`, 33 bytes) plus - headroom — byte-counted because `${#var}` counts *characters* - under UTF-8 while `sun_path` is byte-limited; + headroom — byte-counted because `sun_path` is byte-limited while + `${#var}` counts *characters* in a shell that handles multibyte. + **Which shell runs the script decides this**: `bash` counts + characters under a UTF-8 locale, `dash` counts bytes under every + locale, and `#!/bin/sh` is `bash` on Arch and macOS but `dash` on + Debian and Ubuntu. The count must not depend on that, so the guard + measures bytes explicitly (`printf | LC_ALL=C wc -c`) rather than + relying on the interpreter it happens to get; - an **ancestor-marker check**, because **a managed root is not inherently marker-free**: a `.git` in `$HOME`, a marker above `$HOME/build`, or a contaminated `PMACS_GATE_TARGET_ROOT` rebuilds diff --git a/tests/gate_script_acceptance.rs b/tests/gate_script_acceptance.rs index b073978..15b36b5 100644 --- a/tests/gate_script_acceptance.rs +++ b/tests/gate_script_acceptance.rs @@ -834,14 +834,21 @@ fn the_socket_path_guard_counts_bytes_not_characters() { bytes (must fail), path {path}" ); - // **Chosen by BEHAVIOUR, not by name.** Locale names beyond `C` and - // `POSIX` are implementation-defined and an unrecognized value has - // unspecified behaviour, so setting `LC_ALL=C.UTF-8` and hoping is - // not a precondition — where that locale is absent the shell would - // fall back to byte semantics and the character-counting mutant - // would pass, silently. - let locale = char_counting_locale(); - let out = std::process::Command::new(gate()) + // **Chosen by BEHAVIOUR, not by name**, and the interpreter is part + // of the choice: `${#x}` counting characters is a property of the + // SHELL first and the locale second. `bash` counts characters under + // a UTF-8 locale; `dash` counts bytes under every locale. Naming a + // locale and running `/bin/sh` therefore proves nothing on its own — + // where `/bin/sh` is `dash`, the character-counting mutant measures + // bytes too, agrees with the fix, and this row passes vacuously. + // + // The gate is invoked THROUGH that shell rather than by its + // shebang, because the configuration being pinned is a real one: + // `#!/bin/sh` resolves to `bash` on Arch and on macOS, which is + // exactly where a `${#VAR}` guard would miscount. + let (shell, locale) = char_counting_shell(); + let out = std::process::Command::new(&shell) + .arg(gate()) .arg("--self-test") .current_dir(repo_root()) .env("PMACS_GATE_TARGET_ROOT", root.path()) @@ -860,27 +867,35 @@ fn the_socket_path_guard_counts_bytes_not_characters() { ); } -/// A locale under which **`/bin/sh` counts CHARACTERS** — verified by -/// asking that very shell, not by trusting a name. +/// A `(shell, locale)` pair under which **`${#x}` counts CHARACTERS** — +/// established by asking that very shell, never by trusting a name. /// -/// `${#x}` on a two-byte character answers `1` under a working UTF-8 -/// locale and `2` under `C`. That difference is the entire subject of -/// the row this serves, so the row must establish it as a precondition -/// rather than assume it: otherwise, on a machine without the named -/// locale, the shell counts bytes, the mutant agrees with the fix, and -/// the test passes while proving nothing. +/// **Both axes matter, and the shell matters more.** `${#x}` on a +/// two-byte character answers `1` under `bash` with a UTF-8 locale and +/// `2` under `bash` with `C` — but `dash` answers `2` under *every* +/// locale, because it has no multibyte handling at all. So naming a +/// locale and invoking `/bin/sh` establishes nothing: where `/bin/sh` +/// is `dash` (Debian and Ubuntu, including CI) the character-counting +/// mutant measures bytes too, agrees with the fix, and the row this +/// serves passes while proving nothing. /// -/// Candidates come from `locale -a`, so this reflects what is actually -/// installed. **Fails loudly when none qualifies** — a skip here would -/// be indistinguishable from a pass. -fn char_counting_locale() -> String { +/// `/bin/sh` is tried first, so the real interpreter is used wherever +/// it qualifies — as it does on Arch and macOS, which is precisely +/// where a `${#VAR}` guard would miscount. +/// +/// Locale candidates are the two conventional `C.UTF-8` spellings, +/// which are commonly usable without appearing in `locale -a`, +/// followed by every UTF-8 entry `locale -a` does report. **Fails +/// loudly when no pair qualifies** — a skip here would be +/// indistinguishable from a pass. +fn char_counting_shell() -> (String, String) { let installed = std::process::Command::new("locale") .arg("-a") .output() .map(|o| String::from_utf8_lossy(&o.stdout).into_owned()) .unwrap_or_default(); - let candidates: Vec = ["C.UTF-8".to_owned(), "C.utf8".to_owned()] + let mut locales: Vec = ["C.UTF-8".to_owned(), "C.utf8".to_owned()] .into_iter() .chain( installed @@ -889,24 +904,29 @@ fn char_counting_locale() -> String { .map(str::to_owned), ) .collect(); + locales.dedup(); - for cand in &candidates { - let probe = std::process::Command::new("/bin/sh") - .arg("-c") - .arg("x=é; echo ${#x}") - .env("LC_ALL", cand) - .env("LANG", cand) - .output(); - if let Ok(out) = probe - && String::from_utf8_lossy(&out.stdout).trim() == "1" - { - return cand.clone(); + let shells = ["/bin/sh", "/bin/bash", "bash"]; + for shell in shells { + for locale in &locales { + let probe = std::process::Command::new(shell) + .arg("-c") + .arg("x=é; echo ${#x}") + .env("LC_ALL", locale) + .env("LANG", locale) + .output(); + if let Ok(out) = probe + && String::from_utf8_lossy(&out.stdout).trim() == "1" + { + return (shell.to_owned(), locale.clone()); + } } } panic!( - "no installed locale makes /bin/sh count characters, so the \ - byte-vs-character distinction this row exists to test cannot \ - be established here. Tried: {candidates:?}" + "no available shell counts characters under any installed \ + locale, so the byte-versus-character distinction this row \ + exists to test cannot be established here. Tried shells \ + {shells:?} against locales {locales:?}" ); } From cf09f5ad3ae911c9f25784deb4c0913fba081aaf Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 13 Aug 2026 21:09:35 +0200 Subject: [PATCH 10/10] docs: correct a misattributed review round and an unmeasured claim Records only. No behaviour change; the focused suite stays 27/27 and the full gate stays green. **THE WITHDRAWN CAUSAL CLAIM WAS ASSIGNED TO THE WRONG ROUND.** The framing credited it to round 4. It was round 3 --- whose subject line says so plainly, "canonical ancestry, guard witnesses, and a withdrawn claim". Round 4 was the four discriminators: it added the canonical-traversal WITNESS (round 3 had fixed the traversal and verified it by hand, which is the evidence that does not survive the next edit), moved the guard rows onto the exact boundary, and covered both managed areas on cleanup. Rounds 1 and 2 were re-checked against their commits in the same pass, and round 1 was also wrong: it carried ONE reserve finding, not two. The nested-reserve ruling is round 2's. Each round is now described from what its commit actually did. **AND THE GATE CLAIMED A TMPFS QUOTA THAT WAS NEVER ESTABLISHED.** `scripts/gate` said a build here "hit tmpfs quota mid-compile". Nothing measured that, and it contradicts what was measured later: `/tmp` was at 24G of a 30G tmpfs with 6G free --- NOT out of space. What actually happened is worth keeping precisely because it is subtler than a quota: 24G of tmpfs is 24G of RAM, free memory fell to ~4G of 61G, process spawning became unreliable, and eleven rows failed with EMPTY output. That reads like a code defect. The comment records the measurement and the symptom now, and drops the quota story. The independent reason for a disk-backed TMPDIR is unchanged and is still the measured one: tmpfs fixtures compete for memory. Gates: all nine green under `env -u TMPDIR`, log 20260813T190456Z. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/gate-script-framing.md | 24 ++++++++++++++---------- scripts/gate | 9 ++++++--- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/docs/gate-script-framing.md b/docs/gate-script-framing.md index 359ee33..180ddb1 100644 --- a/docs/gate-script-framing.md +++ b/docs/gate-script-framing.md @@ -9,16 +9,20 @@ lane. **Approved after four review rounds plus a locale follow-up, all of which turned on evidence rather than design.** Round 1 corrected a -propagation witness that proved only that the variable was used, and -two wrong guard reserves. Round 2 tightened the socket budget to the -Darwin floor and replaced an existence-only ancestor check with one -that honours marker types. Round 3 made the traversal canonical and -gave the length guard its first real witnesses. Round 4 moved those -rows onto the exact boundary, covered both managed areas on cleanup, -and withdrew an unsupported causal claim. The follow-up required the -byte-versus-character row to *establish* its precondition rather than -name one. Each correction was a witness asserting something adjacent -to the contract while appearing to assert the contract itself. +propagation witness that observed no inheritance, a reserve that was +not the maximum, and a guard that leaked what it exists to manage. +Round 2 tightened the socket budget to the Darwin floor, *ruled* the +nested case rather than accommodating it by loosening the reserve, and +replaced an existence-only ancestor check with one that honours marker +types. Round 3 made the traversal canonical, gave the length guard its +first witnesses, and **withdrew an unsupported causal claim**. Round 4 +found four properties that were fixed or claimed but would have stayed +green if reverted: it added the **canonical-traversal witness**, moved +the guard rows onto the exact boundary, and covered both managed areas +on cleanup. The follow-up required the byte-versus-character row to +*establish* its precondition rather than name one. Each correction was +a witness asserting something adjacent to the contract while appearing +to assert the contract itself. **Previously, revision 5. Approved at revision 4 and IMPLEMENTED; revision 5 records two safety defects review found in the implementation.** diff --git a/scripts/gate b/scripts/gate index 81db624..2ddd9d7 100755 --- a/scripts/gate +++ b/scripts/gate @@ -548,9 +548,12 @@ mkdir -p "$AMBIENT" # VERIFIES rather than assumes. # # Disk-backed matters independently. `/tmp` is commonly a tmpfs, so a -# sweep's fixtures compete with the machine for RAM; a build here has -# hit tmpfs quota mid-compile. The gate root is on the same filesystem -# as the build artifacts, which is where the space is. +# sweep's fixtures compete with the machine for RAM rather than for +# disk. Measured here: `/tmp` is a 30G tmpfs that reached 24G occupied, +# leaving ~4G of 61G free, at which point process spawning became +# unreliable and rows failed with EMPTY output --- a symptom that reads +# like a code defect. It was not out of space. The gate root is on the +# same filesystem as the build artifacts, which is where the space is. # # IT HANGS OFF THE GATE ROOT, NOT THE PER-WORKTREE TARGET, AND THE NAME # IS SHORT ON PURPOSE --- see the SUN_LEN budget below. Note this