diff --git a/docs/active-work.md b/docs/active-work.md index 3c585cf..fc91e78 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -210,6 +210,61 @@ 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` — PR #225 OPEN (build tooling) + +**PR #225** — https://github.com/levineuwirth/pmacs/pull/225. Written +**after** the PR existed, again, and again because review asked. Two +lanes in a row have now been added late; the correction from #171 and +#215 is not sticking, and recording that is more useful than a +back-dated block that pretends it did. + +- **Branch `gate-script`**, base `githubsucks/main` @ `b833b13` (the + #224 merge). **`githubsucks/gate-script` is the authoritative tip** — + the ref, not a SHA. Recover with + `git fetch githubsucks && git checkout gate-script`. +- **Framing `docs/gate-script-framing.md`**, approved at revision 4 + after four review rounds; revision 5 records two safety defects found + against the implementation, not the design. +- **Scope:** `scripts/gate` (per-worktree `CARGO_TARGET_DIR`, five + ambient roots, durable per-gate logs, the fixed gate suite), + `tests/gate_script_acceptance.rs`, and a handoff §3 rewrite pointing + at the script while §3 keeps policy and acceptance-suite selection. + No `src/`, no crate, no manifest, no protocol. +- **Verification:** 15 acceptance tests over the no-gates paths, each + isolated by `PMACS_GATE_TARGET_ROOT`. Mutation-tested; the two prune + guards are redundant by design and only fail the test when **both** + are removed, which is recorded in the test itself. Observed real runs + confirm failed-gate naming, log paths, ambient creation and reaping, + and distinct log directories per run. + +**GATE STATUS — R8 RESOLVED.** This lane was blocked because +`scripts/gate` exited 1 on a clean tree: **R8** failed `m4_acceptance` +and therefore the sweep. That was never a footnote — #225 is the lane +that makes the gate suite authoritative, and a tool shipping with its +own gate red teaches the opposite of what it exists to teach. + +**R8 was fixed and retired in #226** (`dcb852e`), which bounded the LSP +fixture's project detection. This branch is rebased onto it. + +**RE-GATED 2026-08-09: `scripts/gate` exits 0.** All nine gates green in +one command — fmt, clippy, `--lib`, `--lib --features crdt`, both named +acceptance suites, `m4_acceptance`, `-p pmacs-gpu`, and the full +workspace sweep. That is #225's own acceptance criterion, and it is the +first time the tool has passed the suite it exists to run. + +**Rebase resolution, per the standing rule that #226's R8 documentation +wins.** Three conflicts, all in R8 text this branch had written while +the row was still an open investigation: two in +`docs/ci-red-signatures.md` (both resolved to #226's retired row, this +branch's pre-fix copy dropped), and the framing-doc pair +(`e71e1bd` added it, `7cfba73` removed it — both **skipped**, since they +are net-zero here and `main` owns the file authoritatively; replaying +the second would have deleted `main`'s copy). Two now-stale lanes were +also removed: this branch's "R8 NEEDS A LANE" investigation block, and +#226's own lane, which Rule 4 retires now that it has merged — its +durable facts are in the retired registry row and the handoff §6 +census. + ## QoL arc retirement — PR #224 OPEN (docs only) **PR #224** — https://github.com/levineuwirth/pmacs/pull/224. Written @@ -245,51 +300,6 @@ the PR that retires other lanes. - **Retire this block in the next absorption after #224 merges.** It describes a docs PR; once merged there is nothing volatile left. -## R8 fixture boundary — PR #226 OPEN, HELD FOR REVIEW (test hermeticity) - -**PR #226** — https://github.com/levineuwirth/pmacs/pull/226. **Open, -awaiting review; no merge authorization.** - -**Branch `r8-fixture-boundary`**, base `githubsucks/main` @ `b833b13` -(the #224 merge). **`githubsucks/r8-fixture-boundary` is the -authoritative tip** — the ref, not a SHA. Recover with -`git fetch githubsucks && git checkout r8-fixture-boundary`. - -**Written with the lane's first commit, before the PR existed** — the -standing correction from #171 and #215, which the previous two lanes -both missed and review both caught. - -- **Framing `docs/r8-fixture-boundary-framing.md`**, revision 2, - approved 2026-08-08. -- **Scope:** `tests/m4_acceptance.rs` only — `open_against_fake` sets - `pmacs.project.set_search_boundary` to the fixture directory, plus a - portable planted-marker witness. **No `src/`, no runtime, no - product-behaviour change.** `display_path` and project detection are - deliberately untouched: shortening a location against its project - root is the feature. -- **Deliberately NOT branched from `gate-script`.** `scripts/gate` does - not exist on `main`, so naming it as a criterion would have made this - lane depend on an artifact absent from its own base. -- **Verification (all run):** the R8 test passes **with `/tmp/.git` - still present**, i.e. on the machine that reproduces it; the planted - marker witness passes, and also passes with `TMPDIR` outside `/tmp` - (the CI shape); reverting the boundary fails **both**, the witness - with `proj/r.rs:12:3` — relative to the *planted* marker, which is - what makes the bite portable; full `m4_acceptance` 151/0; `--lib` - 1920 and `--lib --features crdt` 2105; `-p pmacs-gpu` 241; fmt, - clippy, `git diff --check`; and the **full workspace sweep exit 0 - across 113 targets** — the first fully green local sweep of this - session. -- **`/tmp/.git` was NOT removed**, deliberately: deleting it would hide - the hermeticity defect, its provenance is unresolved, and it is the - only thing on this machine that reproduces the row. - -**Sequencing:** this lands first, on its own merits. **Then #225 rebases -onto it** and takes "`scripts/gate` runs green" as *its* re-gate -criterion. On that rebase, **the R8 documentation here is authoritative** -— #225 carries an earlier, pre-fix copy of the R8 registry row and lane -text from when it was still an open investigation, and those must lose. - ## Docs absorption after #217 — MERGED as #218 (2026-08-06 09:59Z) **PR #218** — https://github.com/levineuwirth/pmacs/pull/218. **This diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index 96e25ce..19155ca 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -2230,18 +2230,67 @@ its own step, never `&&`-chained. ## 3. Gate suite (all green before any PR) +**Run it with `scripts/gate`. Do not retype it.** + +``` +scripts/gate [--acceptance SUITE]... [--protocol] +``` + +The script is **authoritative for the fixed executable gates** — the +list below is what it runs. **This section stays authoritative for +policy** (everything under "Every part is load-bearing", the +protocol-bump rule, the machine-specific caveats) **and for CHOOSING +the touched acceptance suites**, which reach the script only through +`--acceptance`. No script can infer those from a working tree, and one +that guessed would report coverage it does not have. + +Three things it does that a retyped command line kept failing to do: + +- **A per-worktree `CARGO_TARGET_DIR`.** This machine exports one + globally, and cargo locks it exclusively — two worktrees building at + once serialize and thrash each other's artifacts, making parallel + lanes *slower* than serial. Note the trap: the environment variable + **overrides** `build.target-dir` in any `config.toml`, so a + per-worktree config file silently does nothing. +- **All five ambient roots** (§ the caveat below), fresh per run. +- **Every gate's full output to a durable log**, with the sweep paths + printed. This is the U2/U3 remedy: both registry notes exist because + a sweep was piped through `grep` before anyone read it, and the + fragments a `docs/ci-red-signatures.md` row needs were gone. **Read + the log; do not re-run and filter.** + +`scripts/gate --print-plan` prints the commands without running them; +`tests/gate_script_acceptance.rs` asserts that plan still matches this +section, so the two cannot drift silently. `--init` prepares a +worktree's build directory; `--prune` reclaims directories whose +worktree is gone (dry-run unless given `--force`). Framing: +`docs/gate-script-framing.md`. + +It is a **convention, not an enforcement** — a bare `cargo test` still +takes the shared lock. Nothing in-repo can change that while the +variable is exported globally. + +What it runs: + ``` cargo fmt --check cargo clippy --workspace --all-targets -- -D warnings # own step cargo test --lib # ~1500 cargo test --lib --features crdt # ~1672 -cargo test --test +cargo test --test # --acceptance cargo test --test m4_acceptance -- --skip basedpyright PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu # 58 -cargo test --workspace -- --skip basedpyright # full sweep +cargo test --workspace --no-fail-fast -- --skip basedpyright # sweep git diff --check ``` +**`--no-fail-fast` is now on the normal sweep too**, not only the +protocol-bump form below. The reason given there — *"cargo stops at the +first failing target: without it a bump that breaks eight assertions +reports one"* — was never specific to bumps; a lane that breaks three +unrelated targets has the same problem. The cost is paid **only on +red**: a green sweep is byte-for-byte the same work. + **The full sweep is not optional, and `CLAUDE.md`'s shorter list is not a substitute.** That list stops at "the touched acceptance suites"; this one continues. Long-lines Stage 3 followed the short form and put @@ -2279,15 +2328,24 @@ some are tripwires doing their job, and one pin Machine-specific caveats — re-verify on a machine you haven't used before trusting them: -- **Ambient storage roots: control all FIVE, not four.** Until the - ambient-root isolation lane lands - (`docs/test-ambient-config-isolation-framing.md`), the ~96 integration - suites read the developer's real `~/.config/pmacs/init.lua` and - **write** bundled packages into the real data root: - `#[cfg(not(test))]` guards the crate's own unit tests only, and - `EditorState::new` materializes packages outside every `cfg` guard. - A local full-suite run therefore needs, all pointed at a fresh - directory: +- **Ambient storage roots: control all FIVE, not four.** + **`scripts/gate` does this for you** — it is only stated here because + the reasoning has to live somewhere, and because a run done by hand + still needs it. + + **The ambient-root isolation implementation MERGED as #206**, so the + in-crate paths resolve roots explicitly rather than from the caller's + environment; the earlier text here still said "until the lane lands", + which stopped being true at that merge. What the five variables cover + now is everything *outside* that guarantee — integration suites that + spawn the real binary, PTY and daemon fixtures, anything reaching a + production resolution path — where the process under test reads the + environment it was handed. Belt and braces: a gate run that scribbles + in the developer's real config or data root is a bad failure mode + whether or not the crate promises not to. + + A local full-suite run done by hand therefore needs, all pointed at a + fresh directory: ``` XDG_CONFIG_HOME XDG_DATA_HOME XDG_STATE_HOME XDG_CACHE_HOME diff --git a/docs/gate-script-framing.md b/docs/gate-script-framing.md new file mode 100644 index 0000000..b28ea0b --- /dev/null +++ b/docs/gate-script-framing.md @@ -0,0 +1,548 @@ +# `scripts/gate` — per-worktree build isolation, and one gate suite + +**Status: 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 +honour this document, which is worth stating because it is the case a +framing doc cannot prevent by itself:** + +- **`--prune` could delete every managed directory.** §2.6 requires + liveness to be *established*; the code masked + `git worktree list` failure with `|| true`, so running from outside + any repository produced an **empty** live set — under which every + marked directory is an orphan and `--prune --force` deletes all of + them, live lanes included. Now: two refusals (not in a worktree; the + enumeration failed), and an outside-repo survivor test. +- **`--acceptance` was shell-injectable.** §2.3 hands the name into a + command the runner evaluates; nothing validated it, so + `--acceptance 'x; rm -rf ~'` would have executed. Now: an allowlist + of what a cargo target can be named, refused at parse time, with a + metacharacter rejection test and a canary. + +Also fixed: log directories now carry the PID as well as a +whole-second timestamp (two runs in one second shared a directory and +could overwrite each other's evidence — reintroducing U2/U3 through a +naming choice); the ownership marker is enforced as exactly one line +rather than read head-first; and the `prunable` test fails loudly +instead of returning green when `git worktree add` fails, with cleanup +through a `Drop` guard that survives a panicking assertion. + +Developer tooling. **Not coherence-affecting** — it touches no journey +step, adds no interaction island, adopts no config-registry setting, and +creates no background work (`COHERENCE.md` §20). It is named here so the +absence is a statement rather than an omission. + +**Revision 2** answered five review findings: the managed root and +ownership marker Q#GS1 needed, the narrowing of Q#GS2 (the script +cannot infer touched acceptance suites), Q#GS4's dry-run and +eligibility rules, the corrected — conditional — CRDT workspace sweep, +and the test-root override. + +**Revision 4** makes the log runner `set -e`-safe (the earlier +`rc=$?` form never executes under `set -eu`, which `scripts/bite` +already uses), corrects a stale justification — ambient-root isolation +**merged as #206**, so the five variables are belt-and-braces for +external and integration paths rather than cover for a missing lane — +and extends the canonical-path rule to **directory derivation**, which +§2.5 had omitted while §4's symlink test depended on it. + +**Revision 3 answered three findings, two of which were regressions I +introduced.** Rewriting §2 for revision 2 **silently dropped two of the +original core responsibilities**: ambient-root isolation and durable +sweep logs. The second of those *is* the U2/U3 remedy, so losing it +would have left this document proposing a fix for a problem it had +stopped solving. Both are restored as §2.2, with the exit-status +hazard in log capture specified rather than left to the implementation. +Prune liveness is made precise in §2.6 — `prunable` entries and one +canonical path representation — and §4's marker claim is reconciled +with the interface by `--init`. + +--- + +## 1. Why this exists + +### 1.1 The measured problem + +Parallel worktrees are about to become the working method. They do not +work on this machine today, and the reason is one line of shell config: + +``` +set -gx CARGO_TARGET_DIR $HOME/build/cargo-target # fish config.fish:55 +``` + +Every worktree therefore builds into **one** directory, and **cargo +takes an exclusive lock on it**. Two lanes building concurrently do not +run in parallel — the second blocks — and they invalidate each other's +artifacts, so alternating between lanes recompiles from scratch each +time. Parallel worktree development under this arrangement is *slower +than serial*. + +### 1.2 The measurement that corrected the first plan + +The shared directory was **285G** (`debug/deps` 222G across 12,084 +files; `debug/incremental` 52G). That number drove an initial proposal +to add `sccache` so per-worktree directories would not lose artifact +sharing. + +**The number was misleading and the proposal was wrong.** 285G is years +of accumulation across *two* projects — pmacs and levcs share that +directory. Measured directly, on a cleaned tree: + +| | wall | size | +|---|---|---| +| `cargo build --workspace`, cold | 55s | 3.4G | +| **`cargo test --workspace --no-run`, cold** | **80s** | **19G** | + +And sccache, measured across two target directories: + +| | hit rate | +|---|---| +| C/C++ | 50.00% | +| **Rust** | **0.00%** | + +Rust rlibs embed their target-dir path in metadata, so dependency +artifacts are not bit-identical between directories; `--extern` content +hashes differ and misses cascade. sccache does not deliver +cross-worktree Rust reuse without `--remap-path-prefix`, which would +degrade backtrace paths for every project on the machine. + +**So the sharing that per-worktree directories cost is worth 80 seconds +and 19G per lane.** Four lanes is 76G against 604G free. There is +nothing to buy back. sccache stays configured — it is cheap and earns +its keep on the C/C++ dependencies — but it is **not** what makes +parallel lanes work, and this document exists partly so that claim is +not repeated. + +### 1.3 The second problem, which shares a solution + +The gate suite in `docs/agent-handoff.md` §3 is retyped by hand every +time. It has been gotten wrong repeatedly, **including twice in the +session that motivated this script**: + +- A remediation sweep used `--tests` instead of `--workspace`, silently + dropping `pmacs_protocol` and `pmacs_gpu` — including protocol tests + that same lane had just written. §3 now carries a warning about + exactly this. +- A full sweep was piped through `grep`, discarding the failure output. + An intermittent red was then unmatchable against + `docs/ci-red-signatures.md`, because a row needs its fragments. This + produced registry note **U2**, and then **U3** when it happened again + a second time in the same session. + +Both are the same failure: **a procedure that lives only in prose gets +executed differently each time.** A script that sets the target +directory must already know how to run the suite, so it should own the +part of it that is fixed. + +--- + +## 2. Design + +`scripts/gate` — POSIX `sh`, matching `scripts/bite`'s shape: heavy +header comment explaining the reasoning, distinct exit codes, failure +output that says what to do next. + +### 2.1 The managed target root (Q#GS1, Q#GS4) + +Everything the script creates lives under **one root it owns**: + +``` +$HOME/build/pmacs-gate-targets/-<8 hex of CANONICAL worktree path>/ +``` + +Both parts derive from the **canonical physical path** of §2.5 — not +from `$PWD`, which preserves whatever symlinked spelling the caller +happened to use. Two spellings of one worktree must produce **one** +directory, or the same lane silently builds into two and the isolation +buys nothing while costing double. + +Deliberately **not** `$HOME/build/` directly: that holds +`cargo-target`, `go`, and `cargo`, none of which this script may reason +about. A dedicated root means `--prune` never has to decide whether an +unfamiliar sibling is fair game. + +Each managed directory carries an **ownership marker** at its top level: + +``` +.pmacs-gate-target # one line: the absolute worktree path it serves +``` + +The marker is what makes deletion safe. It is written at creation, and +**a directory without a well-formed marker is never touched**, whatever +its name. Discovery is: *direct children of the managed root, that are +directories, that contain a readable `.pmacs-gate-target` whose single +line is an absolute path.* Nothing recursive, nothing outside the root, +no name-pattern matching. + +`PMACS_GATE_TARGET_ROOT` overrides the root. It exists for the test +harness (§4) and is documented as test-only; the behavior tests must +never be able to reach the real root. + +### 2.2 Ambient roots and durable logs + +Two responsibilities that revision 2's rewrite dropped. They are core, +not incidental. + +**All five ambient roots, fresh per invocation.** Every gate command +runs with + +``` +XDG_CONFIG_HOME XDG_DATA_HOME XDG_STATE_HOME XDG_CACHE_HOME PMACS_STATE_HOME +``` + +all pointed at one directory created fresh for that run. **The fifth is +not redundant**: `PMACS_STATE_HOME` outranks `XDG_STATE_HOME` +(`src/state.rs`), so redirecting only the four XDG variables leaves the +real state root live on a machine that exports it. + +**This is belt-and-braces, not a workaround for a missing lane.** The +ambient-root isolation implementation merged as **#206**; the in-crate +paths resolve roots explicitly and no longer depend on the caller's +environment. What the five variables cover is everything *outside* that +guarantee — integration suites that spawn the real binary, PTY and +daemon fixtures, and anything reaching a production resolution path — +where the process under test reads the environment it was handed. A +gate runner that scribbles in the developer's real config or data root +is a bad failure mode whether or not the crate promises not to, and +setting five variables is cheap insurance against it. + +`HOME` is deliberately left alone, matching the existing guidance. + +The directory is created under the managed target dir and **removed on +exit**, including on failure. Diagnosis comes from the logs below, not +from a retained config tree, and un-reaped ambient directories would +accumulate silently. + +**Every gate's full output goes to a durable log.** Per invocation: + +``` +/gate-logs//-.log +``` + +retained after the run, and the **workspace sweep log paths are printed +prominently** whether the run passes or fails. This is the direct U2/U3 +remedy: both notes exist because a sweep's output was filtered through +`grep` before anyone read it, so an intermittent red could not be +matched against `docs/ci-red-signatures.md` — a row needs its exact +fragments, and they were gone. A log on disk cannot be filtered away by +the person reading it. + +**The capture must preserve the gate's own exit status, and the obvious +way does not.** In POSIX `sh`, `cmd | tee log` reports **tee's** status, +not `cmd`'s — so a failing gate whose output was teed exits 0 and the +suite reports green. `set -o pipefail` is not POSIX (`dash` lacks it) +and this script targets `sh`. + +The specified form is therefore **redirection, not a pipeline** — and +it must also survive `set -e`, which `scripts/bite` and +`scripts/feature-census` both use (`set -eu`) and this script will too. +**The naive form is doubly wrong**: `cmd > "$log" 2>&1; rc=$?` never +reaches the `rc=` assignment under `set -e`, because the shell exits on +the failing command — so the runner dies without printing which gate +failed or where its log is, which is the entire point of capturing it. + +A failing command is only exempt from `set -e` when it is the condition +of an `if`, so the runner is written as one: + +```sh +if cargo test --workspace --no-fail-fast -- --skip basedpyright > "$log" 2>&1 +then rc=0 +else rc=$? +fi +``` + +On a non-zero `rc` the runner prints **the failed gate's name and its +log path** and then exits non-zero itself. No pipeline exists, so no +status is lost to `tee`; no bare failing command exists, so no status is +lost to `set -e`. + +The cost is that output is not live; that is the right trade for a gate +suite whose failures are read afterwards, and it is exactly how the +sweeps were run successfully in the session that motivated this +script. + +### 2.3 Interface + +``` +scripts/gate [--acceptance SUITE]... [--protocol] [--print-plan] +scripts/gate --print-target-dir +scripts/gate --init +scripts/gate --prune [--force] +``` + +- **`--acceptance SUITE`** (repeatable) — the touched acceptance + suites. **This is the Q#GS2 seam**: a script cannot infer from a + working tree which suites a change touches, and guessing would be + worse than asking, because a wrong guess reads as coverage. §3 stays + authoritative for *choosing* them; the script only runs what it is + handed, and **prints the list it ran** so a PR can quote it. +- **`--protocol`** — the change touches `PROTOCOL_VERSION`. Adds the + CRDT *workspace* sweep (§2.4). +- **`--print-plan`** — print the exact commands and exit without + running them. This is what makes drift testable (§4). +- **`--print-target-dir`** — print the derived directory and exit. + **Pure**: it creates nothing, so it can be called freely. +- **`--init`** — create the managed directory and write its ownership + marker, print the path, exit. **Runs no gates.** The gate path calls + the same internal routine, so this is not a second implementation. + + It exists because §4's verification needs a *mutating* path it can + drive safely: "the marker is written" cannot be tested through a + pure printer, and testing it through a real gate run would execute + the whole suite inside the suite. It is also genuinely useful — a + worktree can be prepared before any work starts. +- **`--prune [--force]`** — see §2.6. + +### 2.4 The gate policy the script encodes + +Revision 1 said "both feature configurations", which **misstated §3**. +The corrected encoding, fixed portion first: + +``` +cargo fmt --check +cargo clippy --workspace --all-targets -- -D warnings # its own step +cargo test --lib +cargo test --lib --features crdt +cargo test --test +cargo test --test m4_acceptance -- --skip basedpyright +PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu +cargo test --workspace --no-fail-fast -- --skip basedpyright +git diff --check +``` + +and **only with `--protocol`**, per §3's "touching `PROTOCOL_VERSION` +STRENGTHENS the sweep line; it does not replace it": + +``` +cargo test --workspace --features crdt --no-fail-fast -- --skip basedpyright +``` + +**One strengthening is proposed rather than assumed.** §3 writes the +normal sweep without `--no-fail-fast` and adds it only in the +protocol-bump form. I propose `--no-fail-fast` **always**, and the +justification is that §3's own stated reason is not specific to +bumps — *"cargo stops at the first failing target: without it a bump +that breaks eight assertions reports one."* A lane that breaks three +unrelated targets has the same problem. + +**The cost is bounded and paid only on red.** A green sweep is +byte-for-byte the same work; a red one continues instead of stopping, +so it costs the remaining targets' runtime exactly when the complete +picture is most wanted. That is the same diagnosability argument that +produced U2 and U3, and it is cheap here. If the added red-run time is +judged not worth it, dropping this reverts one flag and nothing else. + +The CRDT **library** tests (`--lib --features crdt`) stay +unconditional — they always were. + +### 2.5 One canonical path representation + +Marker creation and prune comparison **must** use the same spelling of +a path, or a live worktree can be pruned. `/home/jeans/Repos/...` and a +symlinked route to the same directory are the same worktree and +different strings; git reports the path as it was registered, which +need not match how the marker was written. + +The canonical form is the **physical absolute path** — symlinks +resolved, `pwd -P` semantics: + +```sh +canon() { ( cd "$1" 2>/dev/null && pwd -P ) } +``` + +Applied at **all three** points, which is the correction revision 3 +needed: **directory derivation** (§2.1) hashes `canon` of the worktree, +the marker stores `canon` of the worktree at creation, and every path +from `git worktree list` is passed through `canon` before comparison. + +Naming only the last two, as an earlier draft did, leaves the hash +computed from an uncanonicalized `$PWD` — and then a symlinked +invocation derives a *different* directory whose marker records the +*canonical* path. The result is a second target directory for a live +worktree, indistinguishable from an orphan. The symlink test in §4 +exists for exactly this and would fail against that draft. A path that cannot be `cd`-ed into yields +empty, which never matches a marker — correct, because a directory that +is gone is not a live worktree. + +This matters concretely here: `~/.config/fish/config.fish` and the +cargo config are already symlinks into a dotfiles repo on this machine, +so symlinked spellings are the norm, not a hypothetical. + +### 2.6 Pruning (Q#GS4) + +**`--prune` is dry-run by default and prints what it would delete.** +Deleting requires `--force` as a second, explicit step. There is no +automatic pruning, and pruning never happens on the gate path. + +A managed directory is **eligible** when all of: + +1. it is a direct child of the managed root; +2. it contains a readable `.pmacs-gate-target` whose content is a + single absolute path; +3. that path, canonicalized per §2.5, is **not a live worktree**. + +**"Live" is narrower than "listed", and the difference is the whole +correctness of this rule.** `git worktree list --porcelain` keeps +reporting a worktree that was **administratively registered but whose +directory was manually deleted** — it simply adds a `prunable ` +line to that entry. Treating every listed path as live would therefore +make exactly the directories most worth reclaiming permanently +ineligible. + +So: an entry counts as live **only when its record carries no +`prunable` line**. Records are the blank-line-separated blocks of +`--porcelain` output, read from the primary checkout. + +Anything failing any condition is **listed as skipped, with the +reason** rather than passed over silently — a prune that quietly +ignores things is how one learns too late that the marker was never +written. + +### 2.7 What it cannot do, stated plainly + +**This is a convention, not an enforcement.** An agent that runs bare +`cargo test` still gets the shared directory and still takes the lock. +Nothing in-repo can prevent that while `CARGO_TARGET_DIR` is exported +globally, because **the environment variable overrides +`build.target-dir` in `.cargo/config.toml`** — a per-worktree config +file is silently ineffective, which is worse than absent, because it +looks like it worked. + +Real enforcement would need one of: dropping the global export (affects +levcs and whatever else uses it), or `direnv` per worktree. Both are +machine-config changes outside this repo. **The proposal is the script +plus a handoff §3 rewrite that points at it**; if the convention proves leaky +under real parallel load, direnv is the escalation. + +--- + +## 3. Resolved questions + +### Q#GS1 — directory naming — **RESOLVED** + +`-<8 hex of absolute path>` under the managed root of §2.1. +Branch-name derivation was rejected: the name would change on every +branch switch inside one worktree, discarding artifacts precisely when +they are most reusable. Basename alone collides — the recovery +convention in `docs/active-work.md` actively produces sibling +worktrees with predictable names. + +### Q#GS2 — gate authority — **RESOLVED, NARROWED** + +The script is authoritative for the **fixed executable gates**. §3 +remains authoritative for **policy** (why `--workspace` not `--tests`, +why `--skip basedpyright`, when `--protocol` applies) and for +**selecting the touched acceptance suites**, which reach the script +only through `--acceptance`. + +Revision 1 claimed the script should own the suite outright. That was +wrong for a concrete reason: **no script can infer which acceptance +suites a change touches**, and one that guessed would report coverage +it did not have. + +### Q#GS3 — does the primary checkout get an isolated directory — **RESOLVED** + +Yes. Uniform, no special case. The primary abandons the warm shared +directory and pays 80s once. A rule with an exception gets applied +wrongly. + +### Q#GS4 — pruning — **RESOLVED** + +Explicit only, dry-run by default, `--force` to delete, eligibility and +skip-reporting per §2.6, liveness narrowed to non-`prunable` records +and paths canonicalized per §2.5. + +### Q#GS5 — CI — **RESOLVED, UNCHANGED** + +CI runs in fresh containers with their own target directory and no +shared lock, so the isolation is a no-op there. CI's matrix splits the +suite across jobs deliberately (`Test (crdt)`, `M4 Perf Gates`, …); +collapsing that into one script would serialize the matrix and lose +per-job signal. Worth revisiting; not free, and not this change. + +--- + +## 4. Verification + +Revision 1 proposed behavior tests with no mechanism to run them. +`PMACS_GATE_TARGET_ROOT` (§2.1) is that mechanism: **every behavior +test points it at a `tempfile::tempdir()`, and the real managed root is +unreachable from the suite.** + +The tests live in `tests/gate_script_acceptance.rs`, so they ride the +existing workspace sweep — a new test, not a CI change (Q#GS5). + +**The recursion constraint shapes what is testable.** A test that ran +`scripts/gate` for real would run the full suite *inside* the suite. So +the tests exercise only paths that **run no gates** — which is a +stronger constraint than "non-mutating", and revision 2 conflated the +two. Asserting that the marker is written requires something that +*writes* it; a pure printer cannot, and a real gate run must not. That +is what `--init` is for (§2.3): it mutates, it runs no gates, and the +gate path calls the same routine, so the test is not exercising a +second implementation. + +The paths under test: + +- **`--print-plan` matches §3.** Asserts the plan contains + `--workspace` and **never** `--tests`; carries `--skip basedpyright` + on both suite-wide lines; includes both `--lib` configurations; and + that `--protocol` adds the CRDT workspace sweep while the default + does **not**. This is the direct test of Q#GS2's named drift risk, + and it is why `--print-plan` exists. +- **`--acceptance` is reflected in the plan**, once per suite, in + order — the seam §3 keeps authority over. +- **Derivation is stable across a branch switch** and **differs + between two worktrees** — the two properties Q#GS1 turns on. + `--print-target-dir`, which mutates nothing. +- **`--init` writes the marker**, and its content is the worktree's + path in the §2.5 canonical form. Also that `--init` is idempotent: + running it twice leaves one directory and one marker. +- **A symlinked spelling of the same worktree derives the same + directory and does not prune.** The fixture reaches one worktree + through a symlink; without §2.5's canonicalization at both ends this + is precisely the case that deletes a live lane's artifacts. +- **`--prune` is dry-run by default**: against a fixture root holding + one eligible directory, one unmarked look-alike, and one marker + pointing at a live worktree, it *names* the eligible one and + **deletes nothing**. Re-run with `--force`, it deletes exactly that + one; the look-alike and the live one survive. This is the test that + matters — a prune bug is unrecoverable. +- **A `prunable` worktree record counts as dead.** The fixture + registers a worktree and deletes its directory without + `git worktree remove`, so `git worktree list --porcelain` still + reports it, carrying `prunable`. Its target directory must be + eligible. Treating "listed" as "live" would make exactly the + directories most worth reclaiming permanently un-prunable, and + nothing else in the suite would notice. +- **Skip reasons are reported**, not silent. + +**Verified by observation, not automated** — and named so the gap is +explicit: that a failing gate exits non-zero and says which gate. A +deliberately broken tree is a real run of the full suite, so it is a +one-off recorded in the lane's PR rather than a test. + +**Also verified by observation:** that the ambient roots are actually +redirected and the sweep logs actually appear. Both are properties of a +real gate run, so they are confirmed in the same one-off as the exit +status above, and named here so the confirmation is not skipped. **If +the log file is absent, the U2/U3 fix is not real** — that is the one +outcome of the observed run that would block the lane. + +**What none of this proves:** that agents will use the script. That is +§2.7's admission, and no test in this repo can close it. + +Gates for the lane itself: the standard suite (via the script, once it +exists — the lane gates itself by construction), plus `shellcheck` if +available and `git diff --check`. + +--- + +## 5. Not in scope + +Dropping the global `CARGO_TARGET_DIR` export. direnv. `sccache` +tuning or `--remap-path-prefix`. CI restructuring (Q#GS5). Pruning the +233G shared directory beyond the 51G of `debug/incremental` already +reclaimed — it has a co-tenant, and `cargo clean` there would destroy +levcs's artifacts. diff --git a/scripts/gate b/scripts/gate new file mode 100755 index 0000000..c305f8e --- /dev/null +++ b/scripts/gate @@ -0,0 +1,451 @@ +#!/bin/sh +# scripts/gate --- run the fixed gate suite in a per-worktree build +# directory, with isolated ambient roots and durable logs. +# +# scripts/gate [--acceptance SUITE]... [--protocol] [--print-plan] +# scripts/gate --print-target-dir +# scripts/gate --init +# scripts/gate --prune [--force] +# +# Framing: docs/gate-script-framing.md (revision 4, approved). +# +# WHY A PER-WORKTREE TARGET DIRECTORY. This machine exports one +# CARGO_TARGET_DIR for every checkout, and cargo takes an EXCLUSIVE LOCK +# on it. Two worktrees building at once do not run in parallel --- the +# second blocks --- and they invalidate each other's artifacts, so +# alternating between lanes recompiles from scratch each time. Parallel +# worktree development under that arrangement is slower than serial. +# Measured cost of the fix: a cold `cargo test --workspace --no-run` is +# 80s and 19G, so per-lane directories are cheap and there is no shared +# cache worth preserving. +# +# NOTE ON PRECEDENCE, because the obvious alternative silently fails. +# CARGO_TARGET_DIR (the environment) OVERRIDES `build.target-dir` in any +# config.toml. A per-worktree `.cargo/config.toml` therefore does +# nothing at all while that variable is exported --- which is worse than +# doing nothing visibly, because it looks configured. Only a +# per-invocation value beats the environment, which is why this exists +# as a script rather than a config file. +# +# THIS IS A CONVENTION, NOT AN ENFORCEMENT. A bare `cargo test` still +# uses the shared directory and still takes the lock. Nothing in-repo +# can prevent that while the variable is exported globally; real +# enforcement would mean dropping the export or adopting direnv, both +# machine-config changes outside this repository. +set -eu + +usage() { + cat >&2 <<'EOF' +usage: scripts/gate [--acceptance SUITE]... [--protocol] [--print-plan] + scripts/gate --print-target-dir + scripts/gate --init + scripts/gate --prune [--force] + + --acceptance SUITE a touched acceptance suite to run (repeatable). + docs/agent-handoff.md section 3 stays authoritative + for CHOOSING these; this script only runs what it + is handed. A script cannot infer them from a + working tree, and one that guessed would report + coverage it does not have. + --protocol the change touches PROTOCOL_VERSION; adds the CRDT + workspace sweep on top of the default one. + --print-plan print the exact gate commands and exit. + --print-target-dir print this worktree's build directory and exit. + Creates nothing. + --init create the build directory and ownership marker, + print it, exit. Runs no gates. + --prune list managed directories whose worktree is gone. + Deletes NOTHING without --force. + --force with --prune, actually delete. +EOF + exit 2 +} + +# --------------------------------------------------------------------- +# One canonical path representation. +# +# `pwd -P` semantics: physical, symlinks resolved. Used at ALL THREE +# points that compare or derive from a worktree path --- directory +# derivation, marker creation, and prune comparison. Using it at only +# some of them is a real bug and not a tidiness point: a symlinked +# invocation would then derive a DIFFERENT directory whose marker +# records the CANONICAL path, producing a second build directory for a +# live worktree that is indistinguishable from an orphan. +# +# A path that cannot be entered yields the empty string, which never +# matches a marker --- correct, since a directory that is gone is not a +# live worktree. +# +# HONEST NOTE ON HOW MUCH THIS CURRENTLY DOES. Measured on the git in +# use here, BOTH `git rev-parse --show-toplevel` and +# `git worktree list --porcelain` already report resolved physical +# paths, including when a worktree was registered through a symlinked +# parent. So on this git, canon() is defence in depth rather than +# load-bearing, and the acceptance test for symlinked spellings passes +# with or without it --- recorded so nobody reads a mutation-test +# "vacuous" result as a hole in the suite. +# +# It stays because the property it guarantees is one this script's +# correctness rests on, that guarantee is not contractual in git, and +# a marker can also be written by hand. The cost is one subshell. +# --------------------------------------------------------------------- +canon() { + ( cd "$1" 2>/dev/null && pwd -P ) || true +} + +# Short digest of a string. sha256sum on Linux, shasum on macOS, and +# cksum as the POSIX floor so this degrades rather than failing. +digest8() { + if command -v sha256sum >/dev/null 2>&1; then + printf '%s' "$1" | sha256sum | cut -c1-8 + elif command -v shasum >/dev/null 2>&1; then + printf '%s' "$1" | shasum -a 256 | cut -c1-8 + else + printf '%s' "$1" | cksum | awk '{printf "%08x", $1}' + fi +} + +MARKER_NAME='.pmacs-gate-target' + +# --------------------------------------------------------------------- +# Acceptance suite names are INTERPOLATED INTO A COMMAND that the runner +# evaluates, so they are an injection surface and are validated as one. +# `--acceptance 'x; rm -rf ~'` must be refused, not executed. +# +# The allowlist is what a cargo test target can actually be named: +# letters, digits, underscore, hyphen. That excludes every shell +# metacharacter, whitespace, and path separators, so nothing reaching +# the plan can be anything but a bare target name. A leading hyphen is +# refused separately --- it is not dangerous, it would just become a +# stray flag to cargo and fail confusingly. +# --------------------------------------------------------------------- +validate_suite() { + case $1 in + '') + echo "gate: --acceptance needs a suite name" >&2 + exit 2 ;; + -*) + echo "gate: acceptance suite may not start with '-': $1" >&2 + exit 2 ;; + *[!A-Za-z0-9_-]*) + echo "gate: refusing acceptance suite name: $1" >&2 + echo "gate: names are cargo test targets --- letters, digits," >&2 + echo "gate: underscore and hyphen only. The name is interpolated" >&2 + echo "gate: into a command, so anything else is rejected rather" >&2 + echo "gate: than escaped." >&2 + exit 2 ;; + esac +} + +# The managed root. PMACS_GATE_TARGET_ROOT exists for the behaviour +# tests and is documented test-only: it is what keeps them from being +# able to reach the real root. +gate_root() { + if [ -n "${PMACS_GATE_TARGET_ROOT:-}" ]; then + printf '%s' "$PMACS_GATE_TARGET_ROOT" + else + printf '%s' "${HOME}/build/pmacs-gate-targets" + fi +} + +worktree_root() { + git rev-parse --show-toplevel 2>/dev/null || { + echo "gate: not inside a git worktree" >&2 + exit 2 + } +} + +# The derived build directory for this worktree. Pure: creates nothing. +target_dir_for() { + _wt=$1 + _base=$(basename "$_wt") + printf '%s/%s-%s' "$(gate_root)" "$_base" "$(digest8 "$_wt")" +} + +# Create the directory and its ownership marker. Idempotent. The gate +# path calls this too, so --init is not a second implementation. +ensure_target_dir() { + _wt=$1 + _dir=$(target_dir_for "$_wt") + mkdir -p "$_dir" + printf '%s\n' "$_wt" > "$_dir/$MARKER_NAME" + printf '%s' "$_dir" +} + +# --------------------------------------------------------------------- +# The plan. +# +# Emits one `namecommand` line per gate. docs/agent-handoff.md +# section 3 owns the REASONING for each of these --- why --workspace and +# never --tests, why --no-fail-fast, why --skip basedpyright, when +# --protocol applies. What lives here is the executable form, so it +# cannot be retyped differently each time. +# +# --print-plan renders this without running anything, which is what +# makes drift from section 3 testable. +# --------------------------------------------------------------------- +emit_plan() { + printf 'fmt\tcargo fmt --check\n' + printf 'clippy\tcargo clippy --workspace --all-targets -- -D warnings\n' + printf 'lib\tcargo test --lib\n' + printf 'lib-crdt\tcargo test --lib --features crdt\n' + for _s in $ACCEPTANCE; do + printf 'acceptance-%s\tcargo test --test %s\n' "$_s" "$_s" + done + printf 'm4\tcargo test --test m4_acceptance -- --skip basedpyright\n' + printf 'gpu\tPMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu\n' + printf 'sweep\tcargo test --workspace --no-fail-fast -- --skip basedpyright\n' + if [ "$PROTOCOL" = 1 ]; then + # Section 3: touching PROTOCOL_VERSION STRENGTHENS the sweep + # line, it does not replace it. Both sweeps run. + printf 'sweep-crdt\tcargo test --workspace --features crdt --no-fail-fast -- --skip basedpyright\n' + fi + printf 'diff-check\tgit diff --check\n' +} + +# --------------------------------------------------------------------- +# Pruning. +# +# Dry run by default; --force to delete. Never automatic, never on the +# gate path. +# +# "LIVE" IS NARROWER THAN "LISTED", and that difference is the whole +# correctness of this. `git worktree list --porcelain` keeps reporting a +# worktree that was administratively registered but whose directory was +# manually deleted --- it adds a `prunable ` line to that +# record. Treating every listed path as live would make exactly the +# directories most worth reclaiming permanently ineligible. +# --------------------------------------------------------------------- +# +# FAILS LOUDLY RATHER THAN RETURNING NOTHING. An empty live set means +# "every managed directory is an orphan", so a silent failure here is a +# command to delete all of them. The first version piped git straight +# into awk and the caller masked the result with `|| true`; run from +# outside any repository that produced an empty set and +# `--prune --force` would have deleted every marked directory. The +# capture-then-check shape below is what makes that unrepresentable. +live_worktrees() { + _porc=$(git worktree list --porcelain 2>/dev/null) || return 1 + # A repository always has at least its own worktree, so empty output + # is a failure, not an answer. + [ -n "$_porc" ] || return 1 + # Records are blank-line separated. Emit the canonical path of every + # record that carries NO `prunable` line. + printf '%s\n' "$_porc" | awk ' + /^worktree / { path = substr($0, 10); prunable = 0; next } + /^prunable/ { prunable = 1; next } + /^$/ { if (path != "" && !prunable) print path; path = ""; next } + END { if (path != "" && !prunable) print path } + ' | while IFS= read -r p; do + c=$(canon "$p") + [ -n "$c" ] && printf '%s\n' "$c" + done +} + +do_prune() { + # SAFETY GATE, and it is the most important thing in this file. + # Pruning decides what to DELETE by subtracting the live worktree + # set from the managed root. If that set cannot be established, the + # correct answer is not "nothing is live" --- it is "refuse". + if ! git rev-parse --show-toplevel >/dev/null 2>&1; then + echo "gate: refusing to prune --- not inside a git worktree." >&2 + echo "gate: the live-worktree set cannot be established from here," >&2 + echo "gate: and an empty one would mark every managed directory an" >&2 + echo "gate: orphan. Run --prune from inside a checkout." >&2 + exit 2 + fi + if ! _live=$(live_worktrees); then + echo "gate: refusing to prune --- could not enumerate git worktrees." >&2 + echo "gate: nothing was examined and nothing was deleted." >&2 + exit 2 + fi + + _root=$(gate_root) + if [ ! -d "$_root" ]; then + echo "gate: no managed root at $_root; nothing to prune" + return 0 + fi + _found=0 + for _d in "$_root"/*; do + [ -d "$_d" ] || continue + _found=1 + if [ ! -r "$_d/$MARKER_NAME" ]; then + # The protection that matters: a directory that merely + # RESEMBLES a managed one is never touched. + echo "gate: skip $_d --- no readable $MARKER_NAME" + continue + fi + # The marker is DOCUMENTED as one line, so enforce that rather + # than reading the first line of whatever is there. Reading only + # line 1 would accept a multi-line file and act on its head --- + # which is exactly the shape a corrupted or hand-edited marker + # takes, and acting on it means deleting a directory on the + # strength of a file we did not understand. + _lines=$(wc -l < "$_d/$MARKER_NAME" 2>/dev/null || echo 0) + if [ "$_lines" -ne 1 ]; then + echo "gate: skip $_d --- marker is not exactly one line ($_lines)" + continue + fi + _owner=$(cat "$_d/$MARKER_NAME" 2>/dev/null || true) + case $_owner in + /*) ;; + *) echo "gate: skip $_d --- marker is not an absolute path" + continue ;; + esac + if printf '%s\n' "$_live" | grep -qxF "$_owner"; then + echo "gate: skip $_d --- worktree is live ($_owner)" + continue + fi + if [ "$FORCE" = 1 ]; then + rm -rf "$_d" + echo "gate: deleted $_d (worktree gone: $_owner)" + else + echo "gate: WOULD delete $_d (worktree gone: $_owner)" + fi + done + [ "$_found" = 1 ] || echo "gate: managed root is empty" + if [ "$FORCE" != 1 ]; then + echo "gate: dry run --- nothing was deleted. Re-run with --force." + fi +} + +# --------------------------------------------------------------------- +# Argument parsing +# --------------------------------------------------------------------- +ACCEPTANCE='' +PROTOCOL=0 +FORCE=0 +MODE=run + +while [ $# -gt 0 ]; do + case $1 in + --acceptance) + [ $# -ge 2 ] || usage + validate_suite "$2" + ACCEPTANCE="$ACCEPTANCE $2" + shift 2 ;; + --protocol) PROTOCOL=1; shift ;; + --print-plan) MODE=plan; shift ;; + --print-target-dir) MODE=printdir; shift ;; + --init) MODE=init; shift ;; + --prune) MODE=prune; shift ;; + --force) FORCE=1; shift ;; + -h|--help) usage ;; + *) echo "gate: unknown argument: $1" >&2; usage ;; + esac +done + +case $MODE in + plan) + emit_plan | cut -f2- + exit 0 ;; + printdir) + target_dir_for "$(canon "$(worktree_root)")" + echo + exit 0 ;; + init) + ensure_target_dir "$(canon "$(worktree_root)")" + echo + exit 0 ;; + prune) + do_prune + exit 0 ;; +esac + +# --------------------------------------------------------------------- +# Run +# --------------------------------------------------------------------- +WT=$(canon "$(worktree_root)") +cd "$WT" + +TARGET=$(ensure_target_dir "$WT") +STAMP=$(date -u +%Y%m%dT%H%M%SZ) +# $$ as well as the timestamp: two invocations in the same worktree +# within one second would otherwise share a log directory and overwrite +# each other's evidence --- which is precisely the U2/U3 failure this +# logging exists to prevent, reintroduced by a naming choice. +LOGDIR="$TARGET/gate-logs/$STAMP-$$" +mkdir -p "$LOGDIR" + +# Ambient roots: all five, one fresh directory, reaped on exit. +# +# THE FIFTH IS NOT REDUNDANT. PMACS_STATE_HOME outranks XDG_STATE_HOME +# (src/state.rs), so redirecting only the four XDG variables leaves the +# real state root live on a machine that exports it. +# +# Belt and braces rather than a workaround: ambient-root isolation +# merged as #206 and the in-crate paths resolve roots explicitly. What +# this covers is everything OUTSIDE that guarantee --- integration +# suites that spawn the real binary, PTY and daemon fixtures, anything +# reaching a production resolution path --- where the process under test +# reads the environment it was handed. HOME is deliberately left alone. +AMBIENT="$TARGET/gate-ambient/$STAMP-$$" +mkdir -p "$AMBIENT" +cleanup() { rm -rf "$AMBIENT"; } +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" + +echo "gate: worktree $WT" +echo "gate: target dir $TARGET" +echo "gate: logs $LOGDIR" +# Printed so the isolation is OBSERVABLE rather than merely asserted: +# 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" +[ -n "$ACCEPTANCE" ] && echo "gate: acceptance $ACCEPTANCE" +[ "$PROTOCOL" = 1 ] && echo "gate: protocol yes (CRDT workspace sweep added)" +echo + +PLAN_FILE="$LOGDIR/plan.txt" +emit_plan > "$PLAN_FILE" + +N=0 +FAILED='' +while IFS="$(printf '\t')" read -r name cmd; do + N=$((N + 1)) + log=$(printf '%s/%02d-%s.log' "$LOGDIR" "$N" "$name") + printf 'gate: [%02d] %-14s ' "$N" "$name" + # THE RUNNER MUST SURVIVE `set -e`, and the obvious forms do not. + # + # cmd | tee log --- reports TEE's status, so a failing gate + # exits 0 and the suite reads green. + # `set -o pipefail` is not POSIX; dash + # lacks it and this script targets sh. + # cmd > log; rc=$? --- under `set -e` the shell exits AT the + # failing command, so `rc=$?` never runs + # and nothing prints which gate failed or + # where its log is --- destroying the + # entire point of capturing it. + # + # A failing command is exempt from `set -e` only as an `if` + # condition. Hence this shape: no pipeline, so no status is lost to + # tee; no bare failing command, so no status is lost to set -e. + if eval "$cmd" > "$log" 2>&1; then + echo "ok" + else + rc=$? + echo "FAILED (exit $rc)" + echo "gate: log: $log" >&2 + FAILED="$FAILED $name" + fi +done < "$PLAN_FILE" + +echo +echo "gate: sweep logs (the U2/U3 remedy --- read these, do not re-run and grep):" +for f in "$LOGDIR"/*-sweep.log "$LOGDIR"/*-sweep-crdt.log; do + [ -f "$f" ] && echo "gate: $f" +done + +if [ -n "$FAILED" ]; then + echo >&2 + echo "gate: FAILED:$FAILED" >&2 + echo "gate: logs in $LOGDIR" >&2 + exit 1 +fi + +echo +echo "gate: all gates passed" diff --git a/tests/gate_script_acceptance.rs b/tests/gate_script_acceptance.rs new file mode 100644 index 0000000..ac68b3e --- /dev/null +++ b/tests/gate_script_acceptance.rs @@ -0,0 +1,505 @@ +//! `scripts/gate` — the behaviour a shell script can be held to. +//! +//! Framing: `docs/gate-script-framing.md` §4 (revision 4, approved). +//! +//! # Why these tests exist, and why they are shaped like this +//! +//! The script exists to make two things unforgettable: a per-worktree +//! `CARGO_TARGET_DIR` (because cargo locks it exclusively, so shared +//! target directories make parallel worktrees *slower* than serial), +//! and the fixed gate suite itself, which had been retyped by hand and +//! gotten wrong twice in one session. +//! +//! # The recursion constraint shapes what is testable +//! +//! A test that ran `scripts/gate` for real would run the whole gate +//! suite **inside** the gate suite. So every test here drives a path +//! that **runs no gates** — which is stricter than "non-mutating", and +//! is why `--init` exists: asserting the ownership marker is written +//! needs something that *writes* it, a pure printer cannot, and a real +//! gate run must not. `--init` shares the gate path's routine, so this +//! is not a second implementation being tested. +//! +//! # The real managed root is unreachable from here +//! +//! Every test sets `PMACS_GATE_TARGET_ROOT` to a `tempdir`. That +//! override exists for this file. Nothing here can touch +//! `~/build/pmacs-gate-targets`, which matters most for the prune +//! tests — a prune bug is unrecoverable. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +fn repo_root() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) +} + +fn gate() -> PathBuf { + repo_root().join("scripts/gate") +} + +/// Run `scripts/gate` with an isolated managed root, from `cwd`. +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) + .output() + .expect("run scripts/gate"); + ( + String::from_utf8_lossy(&out.stdout).into_owned(), + String::from_utf8_lossy(&out.stderr).into_owned(), + out.status.success(), + ) +} + +fn run(root: &Path, args: &[&str]) -> (String, String, bool) { + run_in(&repo_root(), root, args) +} + +// --- The plan matches handoff §3 ---------------------------------------- +// +// This is the direct test of the framing's named drift risk (Q#GS2): +// the script is authoritative for the FIXED gates, so if it drifts from +// §3, nothing else in the repository would notice. `--print-plan` +// exists to make that checkable without executing anything. + +#[test] +fn the_plan_sweeps_the_workspace_and_never_only_the_tests() { + let root = tempfile::tempdir().expect("tempdir"); + let (plan, _, ok) = run(root.path(), &["--print-plan"]); + assert!(ok, "--print-plan must succeed"); + + assert!( + plan.contains("cargo test --workspace --no-fail-fast -- --skip basedpyright"), + "the sweep must be --workspace; plan was:\n{plan}" + ); + // The specific mistake §3 warns about: `--tests` selects 108 targets + // where `--workspace` selects 110, dropping `pmacs_protocol` and + // `pmacs_gpu`. A lane that had just written protocol tests swept + // without running them. + assert!( + !plan.contains("--tests"), + "`--tests` silently drops the protocol and GPU crates; plan was:\n{plan}" + ); + assert!( + plan.contains("cargo fmt --check") + && plan.contains("cargo clippy --workspace --all-targets -- -D warnings") + && plan.contains("git diff --check"), + "plan was:\n{plan}" + ); +} + +#[test] +fn the_plan_runs_the_library_tests_in_both_feature_configurations() { + let root = tempfile::tempdir().expect("tempdir"); + let (plan, _, _) = run(root.path(), &["--print-plan"]); + assert!(plan.contains("cargo test --lib\n"), "plan was:\n{plan}"); + assert!( + plan.contains("cargo test --lib --features crdt"), + "the CRDT LIBRARY tests are unconditional — only the crdt \ + WORKSPACE sweep is gated on --protocol; plan was:\n{plan}" + ); +} + +/// §3: "Touching `PROTOCOL_VERSION` STRENGTHENS the sweep line. It does +/// not replace it." So `--protocol` must *add* a sweep, leaving the +/// 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 crdt_sweep = "cargo test --workspace --features crdt --no-fail-fast"; + + let (default_plan, _, _) = run(root.path(), &["--print-plan"]); + assert!( + !default_plan.contains(crdt_sweep), + "a normal lane must not pay for the CRDT workspace sweep; plan was:\n{default_plan}" + ); + + let (proto_plan, _, _) = run(root.path(), &["--protocol", "--print-plan"]); + assert!( + proto_plan.contains(crdt_sweep), + "--protocol must add the CRDT workspace sweep; plan was:\n{proto_plan}" + ); + assert!( + proto_plan.contains("cargo test --workspace --no-fail-fast -- --skip basedpyright"), + "STRENGTHENS, not replaces — the default sweep must survive; plan was:\n{proto_plan}" + ); +} + +/// 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 (plan, _, _) = run( + root.path(), + &[ + "--acceptance", + "alpha_acceptance", + "--acceptance", + "beta_acceptance", + "--print-plan", + ], + ); + let a = plan + .find("cargo test --test alpha_acceptance") + .unwrap_or_else(|| panic!("alpha missing from plan:\n{plan}")); + let b = plan + .find("cargo test --test beta_acceptance") + .unwrap_or_else(|| panic!("beta missing from plan:\n{plan}")); + assert!( + a < b, + "suites must keep their given order; plan was:\n{plan}" + ); +} + +// --- Derivation, marker, canonical paths -------------------------------- + +#[test] +fn printing_the_target_dir_creates_nothing() { + let root = tempfile::tempdir().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"); + assert!( + !Path::new(dir.trim()).exists(), + "--print-target-dir must be pure — it printed {dir} and created it" + ); +} + +#[test] +fn init_writes_the_ownership_marker_and_is_idempotent() { + let root = tempfile::tempdir().expect("tempdir"); + let (dir, _, ok) = run(root.path(), &["--init"]); + assert!(ok, "--init must succeed"); + let dir = PathBuf::from(dir.trim()); + + let marker = dir.join(".pmacs-gate-target"); + assert!(marker.is_file(), "the ownership marker must exist"); + let owner = std::fs::read_to_string(&marker).expect("read marker"); + // Canonical form (§2.5): what prune compares against. + let expected = repo_root().canonicalize().expect("canonicalize repo root"); + assert_eq!( + owner.trim(), + expected.to_string_lossy(), + "the marker must record the CANONICAL worktree path" + ); + + run(root.path(), &["--init"]); + let n = std::fs::read_dir(root.path()) + .expect("read root") + .filter(|e| e.as_ref().is_ok_and(|e| e.path().is_dir())) + .count(); + assert_eq!(n, 1, "--init must be idempotent"); +} + +/// **The case that deletes a live lane's artifacts if derivation is not +/// canonical.** Reaching one worktree through a symlink must derive the +/// same directory. If the hash came from an uncanonicalized `$PWD`, the +/// symlinked spelling would derive a *different* directory whose marker +/// records the *canonical* path — a second build directory for a live +/// worktree, indistinguishable from an orphan. +/// +/// **This currently passes for a reason the script does not control**, +/// and saying so is more useful than implying otherwise: measured here, +/// `git rev-parse --show-toplevel` already returns a resolved physical +/// path, so the derivation is canonical before `canon()` touches it. +/// Removing `canon()` does not make this test fail today. It pins the +/// **property**, which is what must hold — not the mechanism, which is +/// 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 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 + } + + let (direct, _, _) = run(root.path(), &["--print-target-dir"]); + let (through_link, _, _) = run_in(&link, root.path(), &["--print-target-dir"]); + assert_eq!( + direct.trim(), + through_link.trim(), + "two spellings of one worktree must share one build directory" + ); +} + +// --- Pruning ------------------------------------------------------------ + +/// Build a managed root holding three entries: one orphan (eligible), +/// one unmarked look-alike, and one owned by a live worktree. +fn prune_fixture(root: &Path) -> (PathBuf, PathBuf, PathBuf) { + let orphan = root.join("gone-00000000"); + std::fs::create_dir_all(&orphan).expect("mkdir orphan"); + std::fs::write( + orphan.join(".pmacs-gate-target"), + format!("{}\n", root.join("no-such-worktree").display()), + ) + .expect("write orphan marker"); + + let lookalike = root.join("pmacs-deadbeef"); + std::fs::create_dir_all(&lookalike).expect("mkdir lookalike"); + + let (live, _, _) = run(root, &["--init"]); + (orphan, lookalike, PathBuf::from(live.trim())) +} + +#[test] +fn prune_is_a_dry_run_by_default_and_deletes_nothing() { + let root = tempfile::tempdir().expect("tempdir"); + let (orphan, lookalike, live) = prune_fixture(root.path()); + + let (out, _, ok) = run(root.path(), &["--prune"]); + assert!(ok, "--prune must succeed"); + assert!( + out.contains("WOULD delete") && out.contains(&orphan.to_string_lossy().to_string()), + "the orphan must be named; output was:\n{out}" + ); + assert!( + orphan.exists() && lookalike.exists() && live.exists(), + "a dry run must delete nothing" + ); +} + +#[test] +fn force_deletes_only_the_orphan() { + let root = tempfile::tempdir().expect("tempdir"); + let (orphan, lookalike, live) = prune_fixture(root.path()); + + let (out, _, ok) = run(root.path(), &["--prune", "--force"]); + assert!(ok, "output was:\n{out}"); + assert!(!orphan.exists(), "the orphan must be gone"); + assert!( + lookalike.exists(), + "a directory that merely RESEMBLES a managed one must never be touched" + ); + assert!(live.exists(), "a live worktree's directory must survive"); +} + +/// **A `prunable` worktree record counts as DEAD**, and nothing else in +/// this suite would catch getting it wrong. +/// +/// `git worktree list --porcelain` keeps reporting a worktree that was +/// registered but whose directory was deleted without +/// `git worktree remove` — it adds a `prunable ` line to that +/// record. Treating every *listed* path as live would make exactly the +/// directories most worth reclaiming permanently ineligible, silently. +/// +/// The other prune tests use a marker pointing at a path git never knew +/// about, so they cannot distinguish "absent from the list" from "listed +/// but prunable". This one registers a real worktree first. +/// +/// **Guarded twice, deliberately.** `live_worktrees` also drops any path +/// it cannot enter, so a deleted directory is excluded even if the +/// `prunable` line were ignored — which is why mutating that line away +/// does not fail this test. The check stays because `prunable` is +/// reported for causes *other* than a missing directory (a gitdir file +/// pointing elsewhere, for one), and those the path filter would miss. +/// Deregisters probe worktrees on the way out **even if an assertion +/// panics**. Cleanup written after the asserts would be skipped by the +/// unwind, leaving the real repository carrying a stale record. +struct WorktreePruneGuard; + +impl Drop for WorktreePruneGuard { + fn drop(&mut self) { + let _ = Command::new("git") + .args(["worktree", "prune"]) + .current_dir(repo_root()) + .output(); + } +} + +#[test] +fn a_registered_worktree_whose_directory_was_deleted_is_prunable() { + let root = tempfile::tempdir().expect("tempdir"); + let home = tempfile::tempdir().expect("tempdir"); + let wt = home.path().join("gate-prunable-probe"); + + let added = Command::new("git") + .args(["worktree", "add", "-q", "--detach"]) + .arg(&wt) + .arg("HEAD") + .current_dir(repo_root()) + .output() + .expect("git worktree add"); + // A HARD failure, not a silent return. Skipping here would make the + // one test that covers `prunable` handling report green on a machine + // where it never ran — the failure mode this whole suite exists to + // avoid. + assert!( + added.status.success(), + "could not register a probe worktree, so this test proved nothing:\n{}", + String::from_utf8_lossy(&added.stderr) + ); + let _guard = WorktreePruneGuard; + + let (dir, _, ok) = run_in(&wt, root.path(), &["--init"]); + let dir = PathBuf::from(dir.trim()); + assert!(ok && dir.is_dir(), "--init in the probe worktree"); + + // Deleted WITHOUT `git worktree remove`: still registered, and now + // reported with a `prunable` line. + std::fs::remove_dir_all(&wt).expect("remove the worktree directory"); + + let (out, _, ok) = run(root.path(), &["--prune", "--force"]); + assert!(ok, "prune must succeed; output was:\n{out}"); + assert!( + !dir.exists(), + "a `prunable` record is not a live worktree — its build directory \ + must be reclaimable, or orphans accumulate forever. Output was:\n{out}" + ); +} + +// --- Refusals: the two ways prune and the plan could do harm ----------- + +/// **The data-loss case.** Pruning decides what to delete by subtracting +/// the live worktree set from the managed root. Run from outside any +/// repository, that set cannot be established — and the first version of +/// this script masked the failure with `|| true`, making the set *empty*, +/// which marks **every** managed directory an orphan. `--prune --force` +/// would then have deleted all of them, including live lanes' artifacts. +/// +/// The correct answer to "I cannot tell what is live" is to refuse. +/// +/// **Two guards, deliberately redundant.** The script refuses both when +/// `git rev-parse --show-toplevel` fails and when `live_worktrees` +/// cannot enumerate — either alone satisfies this test, so mutating +/// away one at a time reads as "vacuous". Removing **both** fails it. +/// Recorded so a later reader does not delete one of them on the +/// grounds that no test noticed. +#[test] +fn prune_outside_a_repository_refuses_and_every_directory_survives() { + let root = tempfile::tempdir().expect("tempdir"); + let (orphan, lookalike, live) = prune_fixture(root.path()); + let outside = tempfile::tempdir().expect("tempdir"); + + // Sanity: the fixture's orphan really is eligible from inside a repo. + let (inside, _, _) = run(root.path(), &["--prune"]); + assert!( + inside.contains("WOULD delete"), + "fixture is not discriminating — nothing was eligible:\n{inside}" + ); + + let (out, err, ok) = run_in(outside.path(), root.path(), &["--prune", "--force"]); + assert!( + !ok, + "pruning from outside a repository must FAIL, not proceed:\n{out}{err}" + ); + assert!( + err.contains("refusing to prune"), + "the refusal must say why; stderr was:\n{err}" + ); + assert!( + orphan.exists() && lookalike.exists() && live.exists(), + "nothing may be deleted when the live set is unknown" + ); +} + +/// `--acceptance` is interpolated into a command the runner evaluates, +/// so a name carrying shell metacharacters is an injection. It must be +/// refused rather than escaped, and refused at parse time — before any +/// gate runs. +#[test] +fn acceptance_names_with_shell_metacharacters_are_refused() { + let root = tempfile::tempdir().expect("tempdir"); + let canary = root.path().join("canary"); + std::fs::write(&canary, "intact").expect("write canary"); + + let hostile = [ + format!("x; rm -f {}", canary.display()), + format!("x$(rm -f {})", canary.display()), + "x`id`".to_string(), + "x && id".to_string(), + "../escape".to_string(), + "x y".to_string(), + "-flag".to_string(), + ]; + + for name in &hostile { + let (out, err, ok) = run(root.path(), &["--acceptance", name, "--print-plan"]); + assert!( + !ok, + "must refuse acceptance name {name:?}; stdout was:\n{out}" + ); + assert!( + err.contains("refusing acceptance suite name") || err.contains("may not start with"), + "refusal for {name:?} must say why; stderr was:\n{err}" + ); + assert!( + !out.contains("rm -f") && !out.contains("id"), + "a hostile name must never reach the plan; stdout was:\n{out}" + ); + } + + assert_eq!( + std::fs::read_to_string(&canary).expect("read canary"), + "intact", + "no injected command may have executed" + ); +} + +/// A well-formed name still works — otherwise the validator could pass +/// the test above by rejecting everything. +#[test] +fn ordinary_acceptance_names_are_accepted() { + let root = tempfile::tempdir().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}"); + assert!( + plan.contains(&format!("cargo test --test {name}")), + "plan was:\n{plan}" + ); + } +} + +/// The marker is documented as one line. Reading only its first line +/// would accept a corrupted or hand-edited file and then delete a +/// 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 bad = root.path().join("bad-00000000"); + std::fs::create_dir_all(&bad).expect("mkdir"); + std::fs::write( + bad.join(".pmacs-gate-target"), + format!( + "{}\nstray second line\n", + root.path().join("gone").display() + ), + ) + .expect("write marker"); + + let (out, _, ok) = run(root.path(), &["--prune", "--force"]); + assert!(ok, "output was:\n{out}"); + assert!( + bad.exists(), + "a malformed marker must not authorise deletion" + ); + assert!( + out.contains("not exactly one line"), + "the skip reason must name the problem; output was:\n{out}" + ); +} + +/// Skips are reported with reasons. A prune that quietly ignores things +/// 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 (_, lookalike, _) = prune_fixture(root.path()); + + let (out, _, _) = run(root.path(), &["--prune"]); + assert!( + out.contains(&lookalike.to_string_lossy().to_string()) + && out.contains("no readable .pmacs-gate-target"), + "the unmarked directory must be named with its reason; output was:\n{out}" + ); + assert!( + out.contains("worktree is live"), + "the live one must be named with its reason too; output was:\n{out}" + ); +}