From d84df23aa7219075c28208322428bcac7aee5c09 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 13 Aug 2026 15:55:19 +0200 Subject: [PATCH] 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.