From dcb852e7400a27c683a1d59946abc12bea661598 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sun, 9 Aug 2026 08:54:06 +0000 Subject: [PATCH] =?UTF-8?q?fix(tests):=20bound=20the=20LSP=20fixture's=20p?= =?UTF-8?q?roject=20detection=20=E2=80=94=20retires=20R8=20(#226)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs: frame the R8 fixture-boundary fix (revision 2) R8 fails m4_acceptance deterministically on one machine, and the diagnosis is that the fixture never bounds its own project detection. `display_path` (builtin/runtime/lsp.lua:2397) shortens a location against the DETECTED PROJECT ROOT. `pmacs.project.detect` walks upward for a marker; from /tmp/.tmpXXXX/r.rs it reaches /tmp, where this machine has a stray EMPTY `.git` directory. The `.git` marker is directory-only, so an empty directory matches, the root resolves to /tmp, and the prefix is stripped. THE PRODUCT BEHAVIOUR IS CORRECT AND IS NOT CHANGING. Shortening a location against its project root is the feature. The defect is that the fixture's assertion depends on whether the developer's /tmp happens to contain a `.git`. THE MECHANISM ALREADY EXISTS AND THIS SUITE ALREADY USES IT. `src/project.rs:208` documents `detect_project_within(.., stop_root)` as existing "so a stray marker in a temp-dir's ancestor (e.g. a developer's /tmp/.git) can't leak into a fixture that lives below it." It is exposed to Lua as `pmacs.project.set_search_boundary`; eight test files make fourteen real calls to it, five of them in m4_acceptance itself --- one carrying that same hazard as a comment. `open_against_fake` (tests/m4_acceptance.rs:7985) is one helper that missed the pattern. THE WITNESS PLANTS ITS OWN HAZARD, so the proof is not a property of this machine: an empty `.git` in a temporary ancestor, the file one level below, boundary at the file's parent. With the boundary the row renders absolute; reverting it strips the prefix deterministically on every machine, including CI where /tmp/.git does not exist. The /tmp/.git observation stays as corroboration, not as the bite. `scripts/gate` is deliberately NOT a criterion: this lane branches from main, where that script does not exist (it is unmerged on #225). Naming it would make this lane depend on an artifact absent from its own base. R8 lands first on its own merits; #225 then rebases and takes "gate runs green" as ITS criterion. Q#R8-1 records a limitation rather than discovering it later: parent-as-boundary is correct only while fixtures put the file as a direct child of the fixture root. A future nested fixture cannot fix itself by passing a deeper path --- the boundary is DERIVED from the parent, so a deeper path clamps sooner, never later. Provenance of /tmp/.git is left permanently unresolved, and the document says why no timestamp is treated as authoritative. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai * fix(tests): bound the LSP fixture's project detection — retires R8 `open_against_fake` never set a search boundary, so the panel tests' rendered paths were shortened against whatever project root detection found ABOVE their tempdir. On a machine with a stray `/tmp/.git` that meant `/tmp` --- and the assertion that spells a path out failed deterministically. Registry row R8. THE PRODUCT BEHAVIOUR WAS NEVER WRONG AND IS NOT CHANGED. Shortening a location against its project root is the feature; a file that really is inside a project really should render relative to it. What was wrong is that a fixture's assertion depended on the contents of the developer's /tmp. THE MECHANISM WAS ALREADY THERE. `src/project.rs:208` documents `detect_project_within(.., stop_root)` as existing "so a stray marker in a temp-dir's ancestor (e.g. a developer's /tmp/.git) can't leak into a fixture that lives below it" --- naming this exact hazard. It is exposed to Lua as `pmacs.project.set_search_boundary`, eight test files make fourteen real calls to it, and five of those are in this same file, one carrying that hazard as a comment. This was one helper that missed a pattern its own file already used. THE WITNESS PLANTS ITS OWN HAZARD, so the proof is not a property of one machine. `a_planted_ancestor_marker_does_not_reach_the_rendered_row` creates an empty `.git` in a temporary ancestor with the file one level below, and asserts the row stays absolute. Reverting the boundary fails it with `proj/r.rs:12:3` --- relative to the PLANTED marker, not to /tmp, because the nearer ancestor wins. That is what makes it bite in CI, where no /tmp/.git exists; confirmed by also running it with TMPDIR outside /tmp. Resting the bite on /tmp/.git would have been the same mistake as a test that passes only where the developer happens to be standing. /tmp/.git IS DELIBERATELY LEFT IN PLACE. Deleting it would hide the hermeticity defect rather than fix it, its provenance is unresolved, and it is the only thing on this machine that reproduces the row --- which makes it useful, not merely untouchable. The R8 fix is verified WITH it present. VERIFICATION. The R8 test passes on the machine that reproduces it. Full m4_acceptance 151/0. `--lib` 1920, `--lib --features crdt` 2105, `-p pmacs-gpu` 241, fmt, clippy, `git diff --check`. The full workspace sweep exits 0 across 113 targets --- the first fully green local sweep of this session, R8 having been the only obstacle. `scripts/gate` is deliberately not a criterion: this branches from main, where it does not exist. #225 rebases onto this and takes a green gate run as ITS criterion. R8 is RETIRED CAUSALLY --- mechanism removed plus a discriminating, portable witness --- and moved to the retired section with its disposition. What the retirement does NOT claim is stated there: 113 `new_with_roots` constructions in this suite alone, an unknown number equally unbounded, harmless only while their assertions do not render a path. That census is now a named §6 follow-on, because the next one will otherwise look like a fresh mystery rather than a known class. Framing: docs/r8-fixture-boundary-framing.md (revision 2, approved). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai * docs(tests): reunite the listview doc comment with its test; state the PR Three review findings, one of which is mine to own plainly. I REPORTED A SHA I NEVER VERIFIED. The previous message named the PR head as `21f0ed1`. That object does not exist in this repository. The true head is `78d8e1c` --- local tip, `githubsucks/r8-fixture-boundary`, and the PR all agree, and it is what was reviewed. No command in that turn ever printed `21f0ed1`; I asserted an identifier instead of reading one, which is precisely the failure a head-SHA check exists to catch. Verified this time before writing it down. THE DOC COMMENT DOCUMENTED THE WRONG TEST. Inserting the new witness anchored on `#[test]\nfn flat_listview_...`, which sits BELOW that test's 17-line doc comment --- so the comment about outline and flat listview consumers ended up introducing the planted-marker test, which touches neither, while the test it was written for was left bare. Moved back. No behaviour change; both tests still pass. That is a general hazard of anchored insertion worth naming: anchoring on the `fn` line silently steals whatever documentation precedes it. STALE STATE IN TWO DOCS. The framing still said "Pre-implementation. Awaiting approval" after being approved and implemented, and the ledger lane said "PR PENDING" after #226 opened. Both now record approval, implementation, the PR link, and that it is held for review with no merge authorization. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --------- Co-authored-by: Claude Opus 5 (1M context) --- docs/active-work.md | 45 ++++++ docs/agent-handoff.md | 12 ++ docs/ci-red-signatures.md | 94 ++++++++---- docs/r8-fixture-boundary-framing.md | 213 ++++++++++++++++++++++++++++ tests/m4_acceptance.rs | 105 +++++++++++++- 5 files changed, 440 insertions(+), 29 deletions(-) create mode 100644 docs/r8-fixture-boundary-framing.md diff --git a/docs/active-work.md b/docs/active-work.md index 439f9fe..3c585cf 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -245,6 +245,51 @@ 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 8e296e0..96e25ce 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -2922,6 +2922,18 @@ round-trip cannot detect a discriminant shift. ## 6. Named deferrals (the standing backlog, consolidated) +**Fixture project-detection census — NOT STARTED, follow-on from R8.** +`tests/m4_acceptance.rs` alone constructs state through +`EditorState::new_with_roots` **113 times**, and an unknown number of +those never call `pmacs.project.set_search_boundary`. Unbounded +detection is harmless *only while the assertion does not render a +path* — which is why exactly one of them (R8) ever failed, and why the +next one will look like a fresh mystery rather than a known class. The +work is a census across the suites, not a rewrite: find the unbounded +constructions whose assertions are path-sensitive, and bound those. +`src/project.rs:208` documents the hazard and `detect_project_within` +is the mechanism; eight test files already use it. + **Capability-aware keymap resolution — CROSS-CUTTING, NOT STARTED, needs its own framing.** Named here because #220 hit its absence and worked around it, not because any of it is designed. Today diff --git a/docs/ci-red-signatures.md b/docs/ci-red-signatures.md index 2b7f268..ad050e9 100644 --- a/docs/ci-red-signatures.md +++ b/docs/ci-red-signatures.md @@ -104,10 +104,11 @@ evidence is macOS / lua54 and that one is macOS / luajit. ## Live rows -**Four of the six evidenced rows are live.** R2 and R4 were retired on -2026-08-05 and are below, under "Retired rows", with their dispositions. -R5 and R6 were added on 2026-08-06 from an occurrence scan and a live -red; neither is diagnosed. +**Four of the seven evidenced rows are live.** R2 and R4 were retired +on 2026-08-05 and **R8 on 2026-08-08**; all three are below, under +"Retired rows", with their dispositions. R5 and R6 were added on +2026-08-06 from an occurrence scan and a live red; neither is +diagnosed. **Rate, as of 2026-08-06.** Of the last 25 `main` runs, **23 green and 2 red** — the two reds being R2's second occurrence (30710662474) and @@ -226,30 +227,6 @@ readiness helpers exist, whether they can be one, and what each promises. Patching this call site alone would leave the same question open under a fourth selector. -### R8 — LSP listview row renders a path relative to a root the test does not expect - -The first row here that is **not intermittent**. It reproduces on every -run, and the merge-base control has already been done — so the one -question this registry exists to answer is settled for it, and settled -against the branch that found it. - -| field | value | -|---|---| -| **selector** | `--test m4_acceptance flat_listview_consumers_render_byte_identically_after_the_tree_extension` | -| **job / flavor** | local (Linux), any invocation — isolated single-test runs included. Not load-sensitive | -| **required fragments** | `the flat references row renders verbatim` **and** a `left` value that is the `right` value with a **leading directory removed** | -| **status** | **deterministic locally, NOT attributed to the long-lines arc** | -| **what IS established** | it fails identically on `main` and on `gpu-horizontal-scroll` with the working tree stashed — a merge-base control, not an inference from "my diff looks unrelated". The rendered row is the expected path with `/tmp/` stripped: `.tmpPZsycN/r.rs:12:3` vs `/tmp/.tmpPZsycN/r.rs:12:3` | -| **what is NOT** | the mechanism. **This is a prefix strip, not a width truncation** — the fixture declares `CellSize::new(40, 100)`, so 25 characters fit with room to spare, and the missing text is at the front. The likeliest reading is that the row is rendered relative to a workspace root that resolves to `/tmp` on this machine, which would make it depend on `TMPDIR`. **Not verified**, and the alternative — that the renderer relativizes against something else entirely — is not excluded | -| **why CI is green** | unestablished. If the `TMPDIR` reading is right, a runner whose temp dir is not directly under the relativization root would never see it. That is a hypothesis, not a finding | -| **retirement** | a diagnosis of what the row is rendered relative to, then either fixing the renderer or making the assertion state the relativization it expects. A green run on a machine with a different `TMPDIR` retires nothing | - -**Not this lane's to fix, and deliberately not fixed here.** Stage 5 -touches `pmacs-gpu`, `pmacs-protocol::scroll`, and `horizontal_follow` -in `src/editor.rs`; it has no path to the LSP listview renderer. Fixing -it inside this branch would put an unrelated, undiagnosed change in a -viewport PR. - ## Retired rows **These stay here on purpose.** A retirement is a claim that a mechanism @@ -257,6 +234,67 @@ is gone; keeping the signature is what makes a recurrence recognisable as a falsification of that claim rather than as a fresh mystery. Both were retired **causally** — the mechanism removed, plus a discriminating witness that fails without the fix — never by a count of green runs. +**R8 joined them on 2026-08-08**, and unlike the other two it was never +intermittent — it was deterministic, and the "flake" reading was never +available to it. + +### R8 — LSP listview row rendered relative to a stray ancestor marker — RETIRED 2026-08-08 + +| field | value | +|---|---| +| **selector** | `--test m4_acceptance flat_listview_consumers_render_byte_identically_after_the_tree_extension` | +| **job / flavor** | local (Linux), any invocation — isolated single-test runs included. Not load-sensitive | +| **required fragments** | `the flat references row renders verbatim` **and** a `left` value that is the `right` value with a **leading directory removed** | +| **causal status** | **DIAGNOSED and FIXED — test hermeticity** | +| **evidence** | reproduced deterministically on one Linux workstation; merge-base control confirmed it on `main` | +| **disposition** | `tests/m4_acceptance.rs::open_against_fake` now sets `pmacs.project.set_search_boundary` to the fixture directory. `docs/r8-fixture-boundary-framing.md` | + +**Mechanism, established rather than guessed:** + +1. `builtin/runtime/lsp.lua:2397` `display_path` shortens a location + against the **detected project root** before rendering it. +2. `pmacs.project.detect` walks **upward** for a marker. From + `/tmp/.tmpXXXXXX/r.rs` it reached `/tmp`. +3. That machine had a stray **`/tmp/.git`** — an *empty directory*, not + a repository. The `.git` marker is directory-only, so an empty + directory still matched. +4. Root resolved to `/tmp`, the prefix was stripped, and the row + rendered as observed. + +Control at diagnosis time: the same test with `TMPDIR` outside `/tmp` +passed. + +**The product behaviour was never wrong and was not changed.** +Shortening a location against its project root is the feature. The +defect was that the fixture did not bound its own project detection, so +its assertion depended on what the developer's `/tmp` contained. +`src/project.rs:208` had documented this exact hazard — *"a developer's +`/tmp/.git`"*, in those words — and provided +`detect_project_within`; `open_against_fake` was one helper that missed +the pattern the same file already used five times. + +**Retired causally, and the witness is portable.** A new test, +`a_planted_ancestor_marker_does_not_reach_the_rendered_row`, **plants +an empty `.git` in a temporary ancestor** and asserts the rendered row +stays absolute. Removing the boundary fails it *deterministically on +every machine* — including CI, where no `/tmp/.git` exists — because +the planted marker is nearer than any real one. **The original +machine's stray directory was corroboration, never the proof**, and it +was deliberately left in place: deleting it would have hidden the +hermeticity defect. + +**Provenance of that `/tmp/.git` remains unresolved and is not needed.** +Observations of its timestamps disagreed, and `/tmp` is a tmpfs whose +entries are touched by inspection, so no timestamp is authoritative +here. It may have been created by the session that found the row. The +fix does not depend on the answer. + +**What this retirement does NOT claim:** that the rest of the suite is +hermetic. `EditorState::new_with_roots` is called **113 times** in +`m4_acceptance` alone, an unknown number of them equally unbounded — +harmless only while their assertions do not render a path. That census +is a named follow-on in `docs/agent-handoff.md` §6, not a completed +audit. ### R2 — USR1 delivered before the trap is installed — RETIRED 2026-08-05 diff --git a/docs/r8-fixture-boundary-framing.md b/docs/r8-fixture-boundary-framing.md new file mode 100644 index 0000000..4c50ed7 --- /dev/null +++ b/docs/r8-fixture-boundary-framing.md @@ -0,0 +1,213 @@ +# R8 — the fixture boundary the LSP panel tests never set + +**Status: revision 2 — APPROVED 2026-08-08 and IMPLEMENTED. PR #226 +open (`https://github.com/levineuwirth/pmacs/pull/226`), held for +review; not merged.** The verification in §5 is no longer a plan: every +item was run, and the results are recorded in the lane in +`docs/active-work.md` and in the retired R8 row of +`docs/ci-red-signatures.md`. + +**Revision 2 corrects two counts I got wrong by miscounting, adds the +portable witness revision 1 lacked, and bounds Q#R8-1's answer.** The +counts came from raw `grep -c`, which counted a function definition, a +comment, a test *name* and two assertion strings as if they were calls +— the same class of error as trusting a test-name match instead of a +signature, which this project has a whole registry about. + +Test hermeticity. **Not coherence-affecting** — no journey step, no +interaction island, no config-registry setting, no background work +(`COHERENCE.md` §20). Named so the absence is a statement. + +--- + +## 1. What is wrong, and what is not + +`docs/ci-red-signatures.md` **R8** fails deterministically on this +machine: + +``` +assertion `left == right` failed: the flat references row renders verbatim + left: ".tmpPZsycN/r.rs:12:3" + right: "/tmp/.tmpPZsycN/r.rs:12:3" +``` + +Diagnosed 2026-08-08: + +1. `builtin/runtime/lsp.lua:2397` `display_path` shortens a location + against the **detected project root** before rendering it. +2. `pmacs.project.detect` walks **upward** for a marker. From + `/tmp/.tmpXXXXXX/r.rs` it reaches `/tmp`. +3. This machine has a stray **`/tmp/.git`** — an empty directory, not a + repository. The `.git` marker is directory-only, so it matches. +4. Root resolves to `/tmp`; the prefix is stripped; the row renders as + observed. + +Control: the same test with `TMPDIR` outside `/tmp` **passes**. + +**The product behaviour is correct and is not being changed.** Shortening +a location against its project root is the feature. A file that really +does sit inside a project really should render relative to it. + +**The defect is that the fixture does not bound its own project +detection**, so its assertion depends on whether the developer's `/tmp` +happens to contain a `.git`. That is a hermeticity bug in the test, and +it is what this lane fixes. + +## 2. The mechanism already exists, and this suite already uses it + +`src/project.rs:208` documents `detect_project_within(start, markers, +stop_root)` as existing + +> *"so a stray marker in a temp-dir's ancestor (e.g. a developer's +> `/tmp/.git`) can't leak into a fixture that lives below it."* + +It is reachable from Lua as `pmacs.project.set_search_boundary(path)` +(`src/lua_bindings/mod.rs:12308`), and **eight test files already call +it** — fourteen real calls between them, of which +`tests/m4_acceptance.rs` holds **five**. One of those five carries the +comment *"(a developer's `/tmp/.git`, say) can't masquerade as the +root."* + +(Revision 1 said nine, from a raw `grep -c` that also counted a +comment, the name of a test *about* the binding, and two assertion +message strings.) + +So this is not a missing capability, a design question, or a new +pattern. **It is one helper that missed an established one**, and the +hazard it missed is documented by name in the same file. + +`open_against_fake` (`tests/m4_acceptance.rs:7985`) builds an +`EditorState`, declares frame geometry, points the `rust` server at the +fake, and opens the file — and never sets a boundary. + +## 3. The change + +Set the search boundary to the fixture's own temporary directory inside +`open_against_fake`, before the file is opened, matching the existing +call sites. + +**Three tests** call `open_against_fake` (`tests/m4_acceptance.rs:8048, +8137, 8261` — revision 1 said four, counting the definition at 7985). +All three must still pass, and the fix is expected to change the +rendering of exactly one — the one whose assertion spells the path out. + +## 4. Open questions + +### Q#R8-1 — the boundary's value: the file's parent, or the tempdir root? + +`open_against_fake` receives a *path*, not the `TempDir`. The parent +directory of that path is the fixture root in every current caller. + +*My vote: **the file's parent directory***, derived inside the helper. +It needs no signature change, and it is what the other call sites in +this file effectively use. + +**Its limitation, stated now rather than discovered later.** This is +correct only while fixtures put the file as a **direct child** of the +fixture root, which all three callers do. A future nested fixture — +say `/crate/src/r.rs` — that *wants* detection to reach its outer +root would not be served by this, and **passing a deeper path cannot +fix it**: the boundary is derived from the path's parent, so a deeper +path clamps the walk *sooner*, never later. That case needs an explicit +boundary argument or a second helper, and revision 1's suggestion that +a caller "can pass a deeper path" had the direction backwards. + +### Q#R8-2 — is this one helper, or a census? + +The suite constructs state through `EditorState::new_with_roots` **113 +times**. An unknown number of those are equally unbounded, and the same +stray marker would affect any of them **whose assertion renders a +path**. Most do not, which is why only this one fails. + +*My vote: **fix this helper in this lane; census separately.*** A +113-site audit is not a bug fix, and bundling it would make the +regression-fixing change unreviewable. But the census should be a named +follow-on rather than a good intention — this row cost real time twice, +and the next one will look like a new mystery. + +**Recommend: file the census as a named backlog item** in +`docs/agent-handoff.md` §6 when this lands. + +### Q#R8-3 — does anything depend on the unbounded walk? + +If some test's expectation quietly relies on detection escaping its +fixture, bounding it would break that test — and that would be a +finding worth having rather than an obstacle. + +*Expected: no.* To be established by running the suite, not asserted +here. + +## 5. Verification + +### 5.1 The primary witness is PORTABLE and plants its own marker + +Revision 1 rested the bite on `/tmp/.git`, which makes the proof a +property of **this machine** — the same mistake as a test that passes +only where the developer happens to be standing. A hermeticity fix +whose only evidence is one machine's stray directory is not +demonstrated; it is anecdotal. + +So the primary witness builds the hazard itself: + +``` +/ <- an empty `.git` is PLANTED here + proj/ <- the file's parent; becomes the boundary + r.rs +``` + +- **With the helper's boundary set to `proj`**, detection examines + `proj`, finds no marker, and stops — the planted `.git` one level up + is out of reach. `display_path` finds no root and falls back, so the + rendered row is the **absolute** path. +- **Reverting the boundary** lets the walk reach ``, match the + planted marker, and strip the prefix — **deterministically, on every + machine**, with no dependence on `/tmp` or `TMPDIR`. + +That pair is the bite. It runs in CI, where `/tmp/.git` does not exist, +and it fails for the right reason if the helper regresses. + +*This is a new test rather than a rewrite of the existing one*: the +existing assertion's value is that it renders a real path verbatim, and +changing its layout to carry a planted marker would blur two purposes +into one fixture. + +### 5.2 The machine observation stays, as confirmation only + +- **The existing test passes with `/tmp/.git` still present** — on the + machine that reproduces R8, not merely on a clean one. The stray + directory is deliberately **not** removed: deleting it would hide the + hermeticity defect, and it is an external directory with unresolved + provenance. +- This is corroboration for the portable witness above, **not the bite + itself**. + +### 5.3 The rest + +- **The other two `open_against_fake` tests still pass**, and the full + `m4_acceptance` suite passes (Q#R8-3). +- **The standard gate suite**, run by hand from `docs/agent-handoff.md` + §3. + +**`scripts/gate` is deliberately NOT a criterion here.** This lane +branches from `main`, where that script **does not exist** — it is +unmerged on `gate-script` (#225). Naming it would make this lane's +verification depend on an artifact absent from its own base, and would +quietly couple two lanes that are meant to land independently. + +The sequencing instead: **R8 lands first, on its own merits. Then #225 +rebases onto it, and "`scripts/gate` runs green" becomes #225's +re-gate criterion** — which is where it belongs, since a green gate run +is the thing #225 ships. + +**What this does not prove:** that the other 113 construction sites are +hermetic. Q#R8-2's census is why. + +## 6. Not in scope + +Removing or altering `/tmp/.git` — and equally, settling its provenance. +Observations of its timestamps have disagreed, `/tmp` is a tmpfs whose +entries are touched by inspection, and nothing in this lane depends on +the answer: §5.1's witness plants its own marker precisely so the fix +does not rest on that directory at all. Changing `display_path` or project +detection semantics — the product behaviour is correct. The 113-site +census (Q#R8-2). Rebasing or merging #225, which follows this landing. diff --git a/tests/m4_acceptance.rs b/tests/m4_acceptance.rs index b271d3e..b106aee 100644 --- a/tests/m4_acceptance.rs +++ b/tests/m4_acceptance.rs @@ -7997,11 +7997,36 @@ fn open_against_fake(path: &std::path::Path) -> pmacs::editor::EditorState { pmacs::protocol::FrontendId::LOCAL, pmacs::protocol::CellSize::new(40, 100), ); + // Clamp the marker walk to the fixture directory so a stray + // ancestor marker (a developer's /tmp/.git, say) can't masquerade + // as the root — the same guard the `go`/`rooturi` fixture above + // already applies, and the hazard `src/project.rs` names in + // `detect_project_within`'s own doc comment. + // + // Without this, `display_path` in lsp.lua shortens rendered + // locations against whatever root detection finds ABOVE the + // tempdir, so an assertion that spells a path out passes or fails + // according to what is in the developer's /tmp. That is registry + // row R8, and it is why this line exists. + // + // The file's PARENT is the boundary, which is correct while + // fixtures put the file as a direct child of the fixture root — all + // three callers do. A future nested fixture that wants detection to + // reach an outer root needs an explicit argument, not a deeper + // path: the boundary is derived from the parent, so a deeper path + // clamps the walk sooner, never later. + let boundary = path + .parent() + .expect("the fixture file has a parent directory"); let fake = fake_lsp_path(); state .lua_host .lua() - .load(format!("pmacs.lsp.config.rust = {{ command = '{fake}' }}")) + .load(format!( + "pmacs.project.set_search_boundary('{}') + pmacs.lsp.config.rust = {{ command = '{fake}' }}", + boundary.display() + )) .exec() .expect("override rust config"); state @@ -8023,6 +8048,84 @@ fn open_against_fake(path: &std::path::Path) -> pmacs::editor::EditorState { state } +/// **R8's portable witness: a planted ancestor marker must not reach a +/// rendered path.** +/// +/// The row above renders a location verbatim, so it is only meaningful +/// if project detection is bounded by the fixture. It was not, and the +/// consequence was registry row **R8** — the assertion passed or failed +/// according to whether the developer's `/tmp` happened to contain a +/// `.git`, which is a property of the machine rather than of pmacs. +/// +/// This test refuses to depend on that. **It plants the hazard itself:** +/// +/// ```text +/// / <- an empty `.git` is created HERE +/// proj/ <- the file's parent; `open_against_fake` bounds to it +/// r.rs +/// ``` +/// +/// With the boundary, detection examines `proj`, finds no marker, and +/// stops — the planted marker one level up is out of reach, so +/// `display_path` finds no root and falls back to the absolute path. +/// **Remove the boundary from `open_against_fake` and this fails +/// deterministically on every machine**, including CI, where no +/// `/tmp/.git` exists. That is the bite; the machine's own stray +/// directory is corroboration, not proof. +/// +/// The product behaviour is deliberately NOT under test here — a file +/// that really is inside a project really should render relative to it. +/// What is under test is that a fixture's rendering cannot be steered +/// by whatever sits above its tempdir. +#[test] +fn a_planted_ancestor_marker_does_not_reach_the_rendered_row() { + let dir = tempfile::tempdir().expect("tempdir"); + // The hazard, planted: an EMPTY `.git` directory, which is exactly + // what the machine that first exhibited R8 had in /tmp. The `.git` + // marker is directory-only, so emptiness does not save us. + std::fs::create_dir_all(dir.path().join(".git")).expect("plant ancestor marker"); + + let proj = dir.path().join("proj"); + std::fs::create_dir_all(&proj).expect("create fixture root"); + let a_path = proj.join("r.rs"); + std::fs::write(&a_path, b"fn main() {}\n").expect("write r"); + + let mut state = open_against_fake(&a_path); + let body = |state: &pmacs::editor::EditorState| -> String { + state + .lua_host + .lua() + .load("local b = pmacs.window.buffer() return b:slice(0, b:len())") + .eval() + .expect("panel text") + }; + + state + .lua_host + .lua() + .load("pmacs.lsp.find_references()") + .exec() + .expect("invoke find_references"); + assert!( + pump_lua_flag( + &mut state, + "pmacs.describe.buffer(pmacs.window.buffer()).name == '*references*'", + 5, + ), + "the references panel opened" + ); + + let refs = body(&state); + let (_header, rows) = refs.split_once('\n').expect("header then rows"); + assert_eq!( + rows, + format!("{}:12:3", a_path.display()), + "the planted `.git` one level above the fixture root must not \ + shorten the rendered path; an unbounded walk would render this \ + as `proj/r.rs:12:3`" + ); +} + /// The *outline* panel end-to-end against the fake server's /// hierarchical documentSymbol response ("Outer" class > "inner" /// method): open, depth-indented rows, RET jump-ring visit to the