neuwirth
/
pmacs
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(gate): review round 1 --- the propagation row proved nothing, and two guards were wrong 

**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
`<target>/gate-tmp/<stamp>-<pid>`; the implementation uses
`<gate-root>/tmp/<mktemp>`. 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
This commit is contained in:
Levi Neuwirth 2026-08-13 16:34:21 +02:00
parent 72647829df
commit 1bd52b7f0d
No known key found for this signature in database
5 changed files with 273 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
docs/active-work.md
View File

@ -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 `<gate-root>/tmp/<pid>`, exported once so every stage and
every process they spawn inherits it, and reaped by the same exit trap

29
docs/agent-handoff.md
View File

@ -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
(`<target>/gate-tmp/<stamp>-<pid>`), 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
**`<gate-root>/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

81
docs/gate-script-framing.md
View File

@ -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 `<gate-root>/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**

137
scripts/gate
View File

@ -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/<stamp>-<pid>/.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" \

63
tests/gate_script_acceptance.rs
View File

@ -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.