diff --git a/docs/active-work.md b/docs/active-work.md index fede39e..6def70f 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -270,6 +270,99 @@ 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 — 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`.** +- **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 + 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. +- **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 + 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 **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. `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. **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 + 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..b852072 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -280,10 +280,44 @@ 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 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_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.** + + **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 + 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 +361,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/docs/ci-red-signatures.md b/docs/ci-red-signatures.md index 6cf2aea..cdc4818 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,48 @@ 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. + +**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. + +**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. + +**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 three fragments verified** against the durable gate log diff --git a/docs/gate-script-framing.md b/docs/gate-script-framing.md index b28ea0b..180ddb1 100644 --- a/docs/gate-script-framing.md +++ b/docs/gate-script-framing.md @@ -1,6 +1,30 @@ # `scripts/gate` — per-worktree build isolation, and one gate suite -**Status: revision 5. Approved at revision 4 and IMPLEMENTED; revision +**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 four review rounds plus a locale follow-up, all of +which turned on evidence rather than design.** Round 1 corrected a +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.** **Neither was a design gap — both were the implementation failing to @@ -417,6 +441,147 @@ under real parallel load, direnv is the escalation. --- +## 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 +`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_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 + 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 `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 + 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. + + **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 + 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 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 +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 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 +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 b946f69..2ddd9d7 100755 --- a/scripts/gate +++ b/scripts/gate @@ -287,7 +287,20 @@ 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. + # + # 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' } @@ -516,13 +529,169 @@ 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 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 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 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 +# 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_TMP_PARENT="$(gate_root)/tmp" +mkdir -p "$GATE_TMP_PARENT" + +# 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 ------------------------------------------------ +# +# 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 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 +# ~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=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 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 + +# --- 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. +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 +# 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 + 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 + gate_marker_refusal "$_anc/.git" + fi + [ "$_anc" = "/" ] && break + _anc=$(dirname "$_anc") +done +fi + 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 +700,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..15b36b5 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 //! @@ -39,11 +40,39 @@ 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) .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"); ( @@ -66,7 +95,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"); @@ -92,7 +124,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!( @@ -107,7 +142,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"]); @@ -155,7 +193,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"; @@ -222,7 +263,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"; @@ -286,7 +330,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![], @@ -338,7 +385,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!( @@ -375,7 +425,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!( @@ -428,12 +481,589 @@ 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::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .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 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 + // 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::Builder::new() + .prefix("g-") + .tempdir_in(short_root_base()) + .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 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() + .expect("gate-tmp parent") + .exists(), + "only the per-run directory is reaped, not its parent" + ); +} + +/// 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("gm-") + .tempdir_in(short_root_base()) + .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}" + ); +} + +/// 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. + // 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(); + 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 socket-path guard, **at the boundary**: 55 bytes accepted, 56 +/// rejected, with the measured lengths asserted. +/// +/// **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 the 48-byte +/// fixture reserve. +#[test] +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() && err.contains("too long for a unix socket path"), + "one byte past the cutoff must be REJECTED; 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, 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 BOTH created areas; found {leaked:?}" + ); +} + +/// The guard counts BYTES, not characters — **under a UTF-8 locale**. +/// +/// `${#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. **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() { + // 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(22)) + .tempdir_in(short_root_base()) + .expect("multibyte root"); + let path = root.path().to_string_lossy().into_owned(); + let chars = path.chars().count(); + let bytes = path.len(); + assert!( + chars + 11 <= 55 && bytes + 11 > 55, + "fixture must straddle: {chars} chars (must pass) vs {bytes} \ + bytes (must fail), path {path}" + ); + + // **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()) + .env("PMACS_GATE_ALLOW_ANCESTOR_MARKER", "1") + .env("LC_ALL", &locale) + .env("LANG", &locale) + .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 root over the BYTE budget must be refused even though it is \ + under the CHARACTER budget ({chars} chars, {bytes} bytes); \ + stderr:\n{err}" + ); +} + +/// A `(shell, locale)` pair under which **`${#x}` counts CHARACTERS** — +/// established by asking that very shell, never by trusting a name. +/// +/// **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. +/// +/// `/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 mut locales: 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(); + locales.dedup(); + + 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 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:?}" + ); +} + +/// A root whose full path is exactly `total` bytes. +fn root_of_len(total: usize) -> tempfile::TempDir { + let base = short_root_base(); + 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"); + 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 +/// 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") +} + +/// 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(); + 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. #[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(), &[ @@ -460,7 +1090,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"); @@ -472,7 +1105,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()); @@ -512,8 +1148,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 @@ -550,7 +1192,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"]); @@ -567,7 +1212,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"]); @@ -615,8 +1263,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") @@ -673,9 +1327,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"]); @@ -705,7 +1365,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"); @@ -746,7 +1409,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}"); @@ -762,7 +1428,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( @@ -790,7 +1459,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"]);