diff --git a/docs/active-work.md b/docs/active-work.md index 28be8ca..09c5c9d 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -118,69 +118,6 @@ declares canonical will pass on a tree the rest of this file does not describe. If it does not, stop and repair the remote/fetch configuration. -## Process-signal diagnostic completeness — IMPLEMENTED, PR OPEN - -- **Portable branch:** `githubsucks/process-signal-diagnostic-completeness`; - worktree `../pmacs-signal-identity`. Governing document: - `docs/process-signal-diagnostic-completeness-framing.md`, **revision 6**; - the contract was approved at revision 4 and has been revised twice - since to match what shipped. -- **State:** **PR #200 open**, four review rounds closed, held for - review. Canonical `githubsucks/main` @ `b8e18f6` (the ledger - absorption #199) is integrated. Four commits: Bet 1 alone, Bets 2–4, - Bet 1's fallback after CI falsified it, then round 4's corrections. -- **Boundary, unchanged:** evidence collection only. No signal - retargeting, tolerance, disposition change, or reap-ledger repair. - **Group identity remains unprovable** (framing §1.5): the measured - group is read in the same read-then-act window, and no portable - mechanism closes that for a *group* — `pidfd` covers a process, and - macOS has neither. The ledger's silent cancellation is parked as its - own lane. -- **What shipped.** The PTY foreground-group lookup now reports four - distinct outcomes instead of one string, through a safe - `filedescriptor::OwnedHandle::dup` bridge that keeps the errno - `portable-pty` discards — with no `unsafe` in pmacs. The report names - the signal. A `measured_group` field from `getpgid` is the only field - in the report able to disagree with its input. The reap-ledger comment - claiming "EPERM cannot happen for our own children" is corrected - **without** claiming the child itself received EPERM. -- **Verification at the merged tree** (not inherited from the pre-merge - head): 11 gates, **4471 tests, zero failures** — fmt, diff-check, - clippy, `--lib`, `--lib --features crdt`, compile-mode, copy-mode in - both feature configurations, bottom-panel Stage 1, M4 with the - basedpyright skip, and required GPU. The job-control divergence - fixture was run 20/20. All local runs used an isolated - `XDG_CONFIG_HOME`. -- **Four bites, each by an actual revert, all observed to fail:** - collapsing the PTY fallback back into a bare `leader-pid`; dropping - `signal=` from the report; making the measured group restate the pid - it was handed; and — as a positive control on the fixture itself — - replacing the `bash -m` job-control child with a plain `sleep`. - Separately, the **pre-Stage-B test was restored verbatim under the - substitution mutation and PASSED**, which is the finding that - justified rewriting it rather than adding to it. -- **Bet 1 was falsified by CI and the framing's fallback shipped.** - `bash -m` diverges on Linux and **never on macOS**, where both legs - observed the terminal stay with the leader for a full 10s wait. The - divergent case is now pinned by **injecting** the foreground group - (runs everywhere, weaker); a Linux-only corroboration drives a real - shell and is the **only** test exercising `pty_foreground_group` - end-to-end. `/bin/bash` is a declared optional test dependency armed - by `PMACS_REQUIRE_BASH` on **Linux only** — macOS ships bash but - cannot produce the precondition, so arming it there would make a - missing binary fatal for a test that can never run. -- **Recovery from a clean checkout:** - - ```sh - git fetch githubsucks --prune - git worktree add ../pmacs-signal-identity \ - -b process-signal-diagnostic-completeness \ - githubsucks/process-signal-diagnostic-completeness - cd ../pmacs-signal-identity - git merge-base --is-ancestor b8e18f6 HEAD - git status --short --branch - ``` - ## The CRDT half of the test corpus is dark in CI — NEEDS A LANE - **No branch, no framing yet.** Found while gating #166, then measured @@ -350,6 +287,52 @@ compatible. - **DAP waits for Stage 2, not Stage 1** — that dependency is now satisfied. +## Test ambient-root isolation — FRAMING OPEN, revision 4 + +- **Branch `test-ambient-config-isolation`**, worktree + `../pmacs-test-isolation`, based on `githubsucks/main` @ `4cd4a7b`. + **Framing only; no code, no PR yet.** + `docs/test-ambient-config-isolation-framing.md` revision 4, three review + rounds closed (eight blocking, six major, all accepted). +- **What it is.** Integration tests use the developer's real ambient + roots. `#[cfg(not(test))]` guards config loading against the crate's + own unit tests only, so the **65** files in `tests/` that construct an + editor load the real `init.lua` — and, separately, `EditorState::new` materializes bundled + packages unconditionally into the real `XDG_DATA_HOME`/`$HOME`. + **It is not read-only**: `~/.local/share/pmacs/builtin-packages/` + exists on the development machine. +- **Why it matters now.** `cargo test` is red on any machine with a real + `~/.config/pmacs/init.lua` (11 of 67 in `compile_mode_acceptance`) and + green in CI, so the failure is attributed to whatever branch is + checked out. Every local gate run in this repo currently needs all + five bootstrap-storage variables controlled as a workaround: + `XDG_CONFIG_HOME`, `XDG_DATA_HOME`, `XDG_STATE_HOME`, + `XDG_CACHE_HOME`, and `PMACS_STATE_HOME`. +- **Round 1's four blocking findings, all confirmed in code:** the + read-only assumption was already false; the population count was 18 + when 65 of 96 files construct an editor, from a grep that did not + match `EditorState::new`; 5 files are both in-process and spawned, so + a file-level partition cannot work; and `EditorState::open` calls + `Self::new()` while `journey_acceptance` requires that exact entry + point. +- **Two constraints any fix must respect.** `std::env::set_var` is + `unsafe` and the crate forbids it, so in-process tests cannot isolate + themselves (precedent: `Installer::root_override`). And config loading + shares one block with `set_init_complete()`, which + `m8_2_acceptance.rs:75` explicitly depends on — skipping the block + would leave integration tests permanently in the init phase. +- Recovery from a clean checkout — **the two-argument form does not + work**, verified by running it (`git worktree add + ` fails with `fatal: invalid reference`, because + after a bare fetch no local branch exists): + + ```sh + git fetch githubsucks + git worktree add ../pmacs-test-isolation \ + -b test-ambient-config-isolation \ + githubsucks/test-ambient-config-isolation + ``` + ## Folding lane (Arc 6) — Stages 1 and 2 MERGED; Stage 3 (GPU) is next Both shipped stages are on `main`; nothing in this arc is in flight. Stage 3 @@ -394,6 +377,27 @@ git worktree add --track \ ## Closed since the last snapshot +- **Process-signal diagnostic completeness — MERGED as #200** + (`main` @ `a2a92bb`), atop Stage A #176. `docs/process-signal-diagnostic-completeness-framing.md` + revision 6; framing approved at revision 4 after three rounds, then + five review rounds on the implementation. Durable facts are in + `docs/agent-handoff.md` §1. **Evidence collection only** — no + tolerance rule, no retargeting, no disposition change. + - **Bet 1 was falsified by CI and the framing's own fallback shipped.** + `bash -m` diverges the terminal's foreground group on Linux and + never on macOS. The divergent case is pinned by injection + everywhere; a Linux-only corroboration drives a real shell and is + the **only** test exercising `pty_foreground_group` end-to-end, so + **on macOS that lookup has no end-to-end coverage**. + - **Group identity remains unprovable**, and the pre-kill sample does + not change that: moving `getpgid` before the `kill` removed a + post-hoc reading, it did not make the reading contemporaneous. + - **Still parked, each needing its own lane:** the reap ledger's + silent cancellation (an EPERM probe drops the entry; a failed + `SIGKILL` is marked killed) — **being scoped next**; retargeting to + the measured pgid; any EPERM/ESRCH tolerance rule; Q#PS6; and + `signal_target`'s read-then-kill of `tcgetpgrp` on the PTY path. + - **Six lanes removed by the 2026-07-30 absorption pass**, all merged, all with their durable facts in `docs/agent-handoff.md` §1: **#190** resource-op delete-guard implementation; **#196** dired @@ -547,11 +551,13 @@ git worktree add --track \ acting frontend made a total function partial, and six runtime modules silently dropped operations (`kill_ring_acceptance` 30/30 → 25/5). Fixed in `9110f9f` before merge. - - Gating fact found on the way: **the workspace sweep must run with an - isolated `XDG_CONFIG_HOME`**, because the real user `init.lua` - installs a local package and the losing race leaks a status message - into painted-frame comparisons. There is also a latent pre-existing - `main` bug in the buffer CRDT undo path, unrelated to this arc. + - Gating fact found on the way: an isolated `XDG_CONFIG_HOME` prevents + the real user `init.lua` from installing a local package and leaking + a status message into painted-frame comparisons. **That isolates the + observed config symptom only, not the gate:** the ambient-root lane + above establishes that data/state/cache must be controlled too. + There is also a latent pre-existing `main` bug in the buffer CRDT + undo path, unrelated to this arc. - `compile_mode_acceptance` is load-sensitive under default parallelism (~1 run in 3, a different test each time); verified pre-existing by swapping in `main`'s `compile.lua`. It is 67/67 at diff --git a/docs/test-ambient-config-isolation-framing.md b/docs/test-ambient-config-isolation-framing.md new file mode 100644 index 0000000..d9971ba --- /dev/null +++ b/docs/test-ambient-config-isolation-framing.md @@ -0,0 +1,521 @@ +# Framing — integration tests use the developer's real ambient roots + +**Revision 4.** Status: awaiting review round 4. Proposed lane: +`test-ambient-config-isolation`, worktree `../pmacs-test-isolation`, +based on `githubsucks/main` @ `4cd4a7b` (a reading; re-measure at branch +time). + +**The suite is green in CI and red on a developer machine that has a +real `~/.config/pmacs/init.lua`.** Not flaky — deterministic, and +attributed to whatever branch happens to be checked out. + +**And it is not only reads.** Review round 1 falsified revision 1's +central assumption: integration tests **write into the real user data +directory**. The lane is therefore about *ambient roots*, not about +`init.lua`. See §1.6. + +## Revision history + +**Revision 3 → 4**, after review round 3 (one blocking, two major). All +three accepted. + +- **The isolated gate still inherited `PMACS_STATE_HOME`** (§1.6a, + §6). That override wins over `XDG_STATE_HOME`, so setting the four XDG + storage variables did not isolate state on a machine that exports it. + The contract now names the exact five variables rather than counting + roots. +- **Q#TI5 still called self-spawn the durable regression guard** after + revision 3 had accepted that it proves behaviour, not continued + adoption. Q#TI5 now names both guards and their different jobs. +- **The active-work lane still said revision 2 and still prescribed + isolated `XDG_CONFIG_HOME` alone.** Both are synchronized with this + revision. + +**Revision 2 → 3**, after review round 2 (three blocking, two major). +All five accepted; all five verified before acceptance, two of them by +running the thing rather than reading it. + +- **Journey isolation had no executable mechanism** (§1.10). `cargo test + --test journey_acceptance` launches with the caller's environment and + the binary cannot isolate itself before its own tests run. Rev 2 said + journey "is isolated by its launched environment" and nothing arranged + that — which quietly assumed the external wrapper this lane exists to + delete. Now named and pinned: per-test parent/child re-exec. +- **One self-spawning test is not a ratchet** (§4.7). It proves the seam + works; it cannot notice a raw `EditorState::new()` added to a + different binary next month. A checked source inventory is now an + acceptance criterion. +- **The root list and the gates disagreed with the audit** (§1.6a, §6). + `BootstrapRoots` covered four roots while §1.6a named six, and the + revised gate still isolated only `XDG_CONFIG_HOME` — so the + "isolated" gate could still write through the real data root. **Every + local gate run in this repo today had exactly that hole.** +- **The count was one high and the ledger overstated it further** + (§1.5). 65 files, not 66; and "all 96 test files load the real config" + was false. +- **The recovery command did not work.** Verified by running it: + `git worktree add ` fails with `fatal: + invalid reference`. + +**Revision 1 → 2**, after review round 1 (four blocking, two major). All +six accepted; all six verified in the code before acceptance. + +- **Rev 1's "the exposure is read-only" was already false** (§1.6). + `EditorState::new` materializes bundled packages *before* config + loading and unconditionally, and `materialize_all` creates + directories. Confirmed on the development machine: + `~/.local/share/pmacs/builtin-packages/v1.0.0` exists. A + non-config-loading constructor does not fix this. +- **Rev 1's population count was wrong, and its stated method did not + match the command that produced it** (§1.5). It reported 18 candidate + files from a grep for `Editor::new`, while its prose described a + broader pattern — and the real constructor is `EditorState::new`, + which `Editor::new` does not match. **66 of 96** test files construct + an editor directly. +- **File-level classification cannot work** (§1.5). At least **5** files + are both in-process and spawned. +- **The seam must cover `EditorState::open`, not only `new`** (§1.7). + `open` calls `Self::new()` directly, and `journey_acceptance` requires + that exact public entry point on purpose. +- **Isolated construction must still finish initialization** (§1.8). + Config loading and `set_init_complete()` share one conditional block, + and `m8_2_acceptance` *documents* its dependence on integration-test + construction being init-complete. +- **Rev 1 contradicted itself on the regression guard** (§2 Q#TI3 vs §5) + and **never added its lane to `docs/active-work.md`**, which the + repository's volatile-work protocol requires. + +## 0. Coherence impact (COHERENCE §20) + +- **No journey step, no user-facing behaviour.** This is test + infrastructure. +- **Serves §9 (worker model) indirectly**: a gate that fails for reasons + unrelated to the change under test destroys the signal the gate exists + to give. +- **Interaction islands: none. Config registry: not adopted. + Background-work attribution: unchanged.** +- **No audited claim in COHERENCE.md changes**; under §25 no COHERENCE + edit rides this PR. + + +## 1. Ground truth (verified at `4cd4a7b`) + +### 1.1 The observation + +On 2026-07-30, `cargo test --all-targets --no-default-features --features +lua54` on a developer machine failed **11 of 67** in +`compile_mode_acceptance`, every one with: + +``` +[@/home/jeans/.config/pmacs/init.lua] command "find-file" is already +defined (refusing to overwrite) +stack traceback: + [C]: in field 'define' + /home/jeans/.config/pmacs/init.lua:3: in main chunk +``` + +Failing tests: `acc14`, `acc24`, `acc25a`, `acc27`, `acc29`, `r1f2`, +`r1f5`, `r2f1`, `r3f1`, `r4f1` (×2). With `XDG_CONFIG_HOME` pointed at an +empty directory the same suite is **67/67**. + +### 1.2 The mechanism, and why the existing guard misses + +`src/editor.rs:770` guards user-config loading: + +```rust +#[cfg(not(test))] +{ + crate::config::load_user_config(&mut lua_host); + lua_host.set_init_complete(); +``` + +with a comment stating the intent exactly: *"Skipped under `cfg(test)` so +the lib's own test suite doesn't pick up the developer's real +`~/.config/pmacs/init.lua` and turn into a flaky environment-dependent +run."* + +**`cfg(test)` is set only when compiling the crate's own unit tests.** +An integration test in `tests/` links `pmacs` as an ordinary dependency, +compiled *without* `cfg(test)` — so the guard is inactive for every one +of them. `cargo test --lib` is protected; `cargo test --test ` is +not. + +The hazard was identified, a mitigation was written, and its scope does +not match the threat. That is the finding — not that nobody thought +about it. + +### 1.3 There are two populations, and they need different fixes + +- **Spawned.** The test launches `pmacs --daemon` as a child process. + Isolation works by setting the child's environment. +- **In-process.** The test constructs `pmacs::editor::EditorState` + directly in the test binary. `compile_mode_acceptance` is this kind + (`tests/compile_mode_acceptance.rs:14`). + +**The spawned case is already solved, once.** `tests/m5_7_acceptance.rs:132`: + +```rust +fn spawn_pmacs_daemon(socket_path: &Path) -> Child { + // Isolate user config: HOME and XDG_CONFIG_HOME both point at the + // (currently empty) socket parent directory, so the daemon won't + // try to read the developer's real `init.lua`. + ... + .env("HOME", isolated_home) + .env("XDG_CONFIG_HOME", isolated_home) +``` + +Both variables, not just one — `config_dir` falls back from +`XDG_CONFIG_HOME` to `$HOME/.config`, so setting one alone leaves the +other path live. + +### 1.4 In-process tests cannot isolate themselves + +The obvious fix — set the variable in test setup — **is unavailable**. +`std::env::set_var` is `unsafe` in the 2024 edition and the crate is +`#![forbid(unsafe_code)]`. + +This is already established in the codebase rather than inferred: +`src/packages/installer.rs:368` carries a `root_override` field +explicitly *because* of it — *"the project forbids `unsafe_code`, so +mutating `XDG_DATA_HOME` directly is not an option"*. + +So an in-process test can only be isolated by (a) the environment it is +launched with, or (b) an injection point in the code under test. There +is precedent for (b) in the same repository. + +### 1.5 Scale, measured + +96 files in `tests/`. **65** contain an actual `EditorState::new()` or +`EditorState::open(` **call**. **5** are *both* in-process and spawned — +`vterm_stage3_acceptance` constructs an editor at `:159` and spawns a +daemon at `:665`. + +**66 files match the bare name; 65 call it.** The 66th is +`tests/m5_6_acceptance.rs`, which mentions `EditorState::new` only to +say it deliberately does *not* use it (`:94`): *"Tests can't go through +`EditorState::new` here because the integration-test build doesn't have +`cfg(test)` set on the lib."* That makes it the **third** place in the +suite documenting the §1.2 gap — after `m8_2_acceptance` and the +`src/editor.rs` comment itself. A grep for a name counts mentions; only +reading counts calls. + +**Revision 1 said 18, and its stated method did not match the command +that produced it.** The prose named a broad pattern; the command grepped +`Editor::new`, which does not match `EditorState::new` — the actual +constructor. So the number was ~4x low *and* described a different +search than the one run. Rev 1 hedged the number as "a candidate count, +not a census" while leaving both the figure and the description wrong; +a disclaimer on a bad measurement does not make it a good one. + +**Consequence for the design:** classification must be per *construction +site* or per *test case*, never per file. A mutually exclusive file +partition cannot represent the 5 mixed files, and acceptance 1 of +revision 1 required exactly that partition. + +### 1.6 The exposure is NOT read-only — this is established + +`EditorState::new` materializes bundled packages **before** config +loading and **unconditionally** — outside any `cfg` guard +(`src/editor.rs:730`): + +```rust +let bundled_root = crate::builtin_packages::bundled_runtime_dir(); +let bundled_packages = crate::builtin_packages::materialize_all(&bundled_root) + .expect("materialize bundled packages"); +``` + +`bundled_runtime_dir()` resolves `XDG_DATA_HOME`, else `$HOME/.local/share` +(`src/builtin_packages.rs:142,148`), and `materialize_all` **creates +directories and writes package files** (`:174`). + +**Confirmed on the development machine:** +`~/.local/share/pmacs/builtin-packages/` exists containing `v0.1.0` and +`v1.0.0`. Whatever produced those, the write path is live and reachable +from every in-process test, none of which override `HOME` or +`XDG_DATA_HOME` at all. + +So revision 1's Bet 2 was not a question to investigate — it was already +answered, in the direction that matters. This upgrades the lane from +"local gates lie" to **"tests write into real user data"**. + +### 1.6a Ambient variables, and the harness's incomplete storage coverage + +`src/` reads: `HOME` (9 sites), `XDG_DATA_HOME`, `XDG_CONFIG_HOME`, +`XDG_STATE_HOME`, `XDG_CACHE_HOME`, `XDG_RUNTIME_DIR`, and +`PMACS_STATE_HOME`. + +The four bootstrap storage roots resolve through **five variables**: +`XDG_CONFIG_HOME`, `XDG_DATA_HOME`, `XDG_STATE_HOME`, +`XDG_CACHE_HOME`, and `PMACS_STATE_HOME`. The fifth is not redundant: +`PMACS_STATE_HOME` wins over `XDG_STATE_HOME` +(`src/state.rs:47-68`), so redirecting the latter while inheriting the +former leaves the real state root live. + +The shared harness `spawn_daemon_process_with_env` +(`tests/common/daemon.rs:154`) sets **`HOME` and `XDG_CONFIG_HOME` +only**. A spawned daemon therefore inherits the developer's real +`XDG_DATA_HOME`, `XDG_STATE_HOME`, `XDG_CACHE_HOME` and +`PMACS_STATE_HOME`. + +**Setting `HOME` isolates a root only when the corresponding `XDG_*` +variable is unset**, because `HOME` is the *fallback*. On this machine +`XDG_DATA_HOME` happens to be unset, so `HOME` does cover it — which +means the harness's apparent adequacy is a property of one developer's +environment, not of the harness. On a machine that exports +`XDG_DATA_HOME`, spawned daemons write to the real one. + +### 1.6b Scope: bootstrap STORAGE roots only + +`BootstrapRoots` covers **config, data, state and cache** — the roots +that decide *where pmacs stores things at startup*. It does **not** +cover: + +- **`HOME`'s non-storage semantics.** `expand_tilde` + (`src/editor_core.rs:5457`) resolves a leading `~` for ordinary path + entry, and `find_file_acceptance.rs:344` consumes `HOME` on purpose to + pin that expansion (skipping when unset). Blanket-overriding `HOME` + would silently retarget a user-facing path feature and its test. +- **`XDG_RUNTIME_DIR`**, which addresses sockets rather than stored + data. + +**Rev 2 listed six roots and proposed covering four without saying so.** +The two exclusions are deliberate and named here so the gap is a +decision rather than an oversight. A later lane may take `HOME` +semantics; this one must not, because the fix for a storage root +(redirect it) is the wrong fix for a path-expansion root (leave it and +isolate the *file* instead). + +### 1.10 Journey needs a mechanism, not an intention + +Rev 2 said `journey_acceptance` keeps the ambient `EditorState::open` +and "is isolated by the environment its binary is launched with". That +is not a mechanism. **Cargo launches each integration-test binary with +the caller's environment**, and a binary cannot re-point its own roots +before its ordinary tests run — §1.4's `set_var` prohibition applies to +itself. So under a plain `cargo test --test journey_acceptance` the +suite is ambient, and the only thing that made rev 2's sentence true was +an external environment wrapper — the very workaround this lane exists +to delete. + +**The mechanism, named:** each journey test becomes a thin parent that +re-execs `std::env::current_exe()` with `--exact `, a marker +variable, and controlled roots; the child, seeing the marker, runs the +real body and calls the **ambient** `EditorState::open`. + +That is the only shape found that satisfies both constraints at once: +the child drives the true production entry point, so +`journey_acceptance`'s ratchet discipline is untouched, while the +child's roots are controlled, so nothing reaches the developer's. The +same shape serves Bet 3's hostile-environment check, so it is one +helper, not two. + +### 1.7 The seam must cover `open`, not only `new` + +`EditorState::open` calls `Self::new()` directly (`src/editor.rs:944`), +so a non-loading *constructor* leaves every open-path test unisolated. + +It cannot simply be bypassed. `tests/journey_acceptance.rs:16` requires +that exact public entry point, and says why: *"A directory arm with no +production caller passes every direct-call test, so step 3 goes through +`EditorState::open` — the same function `pmacs FILE` calls."* The +golden-journey ratchet and the isolation seam pull in opposite +directions, and the framing must resolve it rather than pick one. + +### 1.8 Isolated construction must still finish initialization + +Config loading and `set_init_complete()` share **one** conditional block +(`src/editor.rs:769-773`). Factoring by skipping the block would leave +every integration test permanently in the init phase, changing package +APIs and startup-only config behaviour. + +**This is not hypothetical — it is already depended upon.** +`tests/m8_2_acceptance.rs:75` documents it: + +> `EditorState::new()` sets the init-complete flag during startup (the +> integration-test build doesn't get the `cfg(test)` guard that lib +> tests do), so we reopen the init phase before `install_local`. + +So the `cfg(test)` gap §1.2 calls a defect is, in this one respect, +load-bearing behaviour another suite was written against. Any fix must +skip **ambient reads** while still returning `is_init_complete() == true`. + +### 1.9 What is still NOT established + +- **Whether CI is genuinely unaffected**, as opposed to merely having no + config and no prior data dir today. A CI image that grows either would + break the same way, silently. +- **Whether the 11 failures are the whole blast radius.** The run + aborted at the first failing binary, so every suite ordered after + `compile_mode_acceptance` never executed. +- **What wrote `v0.1.0` and `v1.0.0`** on the development machine — a + test run, or ordinary use of pmacs. The write *path* is proven; the + provenance of those two directories is not, and this lane does not + claim it. + + +## 2. Questions + +- **Q#TI1** — Widen the `cfg(test)` guard, or make isolation the + harness's job? *Proposed: neither alone. The guard must not widen + (production deciding it is under test is how a test passes against + behaviour production never runs), and §1.4 shows the harness cannot + set env in-process. The answer is an explicit **bootstrap-roots + parameter** threaded through construction.* +- **Q#TI2** — What is the seam, exactly? *Proposed: a + `BootstrapRoots` value naming the config root and the data/state/cache + roots, with a `BootstrapRoots::ambient()` used by production and a + test constructor taking an explicit one. It must cover **both** + `EditorState::new` and `EditorState::open` (§1.7), following + `Installer::root_override`'s precedent (§1.4).* +- **Q#TI3** — How does `open`-path isolation coexist with the + golden-journey ratchet? *Proposed: `open` gains a roots-taking sibling + and `journey_acceptance` keeps calling the ambient `open`, because its + purpose is to prove the production entry point is wired. Journey is + then isolated by the **environment its binary is launched with**, not + by a different call — which is the only option that preserves what the + ratchet exists to prove.* +- **Q#TI4** — Must isolated construction remain init-complete? **Yes** + (§1.8), and it is a criterion rather than an assumption. +- **Q#TI5** — What are the durable regression guards? *Proposed: two + guards with different jobs. A **test-binary self-spawn under a + controlled environment** is the behavioural proof: the isolated seam + ignores hostile ambient roots and leaves them unmodified. A **checked + source inventory** is the adoption proof: a raw ambient constructor + added to another test binary fails the suite. Revision 1 proposed a + hostile-config CI leg in Q#TI3 and parked that same mechanism in §5 — + a contradiction; self-spawn replaces that workflow leg, while the + inventory prevents migration drift.* + + +## 3. Bets + +- **Bet 1 — the population is classifiable by construction site.** + Every `EditorState::new`/`open` call site in `tests/`, and every + daemon spawn, is classified. Files are not the unit: 5 are both + (§1.5). + - *Falsified if* sites are reached through helpers that obscure which + kind they are. Then the helper is the unit and that is stated. + +- **Bet 2 — every bootstrap storage root can be redirected without + `unsafe`.** + A `BootstrapRoots` parameter covers config, data, state and cache; the + spawned harness controls all five storage variables of §1.6a. + - *Falsified if* any root is resolved somewhere that cannot accept the + parameter — e.g. behind a `OnceLock` initialised before construction. + **That is a real risk and is checked first**, because it decides + whether this design is possible at all. + +- **Bet 3 — isolation is provable by a hostile-environment test.** A + test spawns the test binary itself with `HOME`/`XDG_*` pointing at a + directory containing an `init.lua` that would break a known assertion, + and a pre-seeded data dir. The suite stays green, and the hostile + directory is **unmodified afterwards**. + - *Falsified if* the child cannot be given a controlled environment + without the in-process `set_var` §1.4 rules out. (It can: `Command` + takes `.env`. This bet is cheap and its failure would be + informative.) + - **The unmodified-afterwards half is the write half of the + check** — a green suite that still wrote into the hostile root has + not demonstrated isolation. + +- **Bet 4 — the fix does not change production behaviour.** Ambient + resolution stays the default; isolated construction still returns + `is_init_complete() == true` (§1.8). + - *Falsified if* any production call site changes, or if + `m8_2_acceptance`'s `reopen_init_phase_for_testing` dance stops + working. + + +## 4. Acceptance + +1. A classification of every editor-construction site and daemon spawn + in `tests/`, by **site**, with mixed files represented explicitly and + counts stated. Method named (read, not grepped) — §1.5 is what a + grep-shaped answer costs. +2. A `BootstrapRoots`-style parameter covering config, data, state and + cache roots, reachable from **both** `EditorState::new` and + `EditorState::open` (§1.7). +3. Isolated construction returns `is_init_complete() == true`, pinned by + a test that fails if the init flip is skipped along with the ambient + reads (§1.8). +4. `journey_acceptance` still drives the ambient `EditorState::open`; + its isolation comes from the launched environment. The ratchet's + production-entry-point discipline is unweakened, and this is asserted + rather than asserted-to-be-obvious. +5. The shared harness `spawn_daemon_process_with_env` controls all + **five storage variables** of §1.6a: + `XDG_CONFIG_HOME`, `XDG_DATA_HOME`, `XDG_STATE_HOME`, + `XDG_CACHE_HOME`, and `PMACS_STATE_HOME`. +6. `compile_mode_acceptance` passes with a real + `~/.config/pmacs/init.lua` defining `find-file` present — the exact + condition of §1.1. +7. A hostile-environment self-spawn test (Bet 3) that asserts both + green **and** an unmodified hostile root. +8. `cfg(test)`-only guards are not widened (Q#TI1). +9. The `src/editor.rs:770` comment is corrected: it claims a protection + it does not provide for integration tests, and says nothing about the + unconditional package materialization above it. +10. This lane is recorded in `docs/active-work.md` with its branch and + worktree, per the volatile-work protocol — revision 1 omitted it. +11. README or handoff records that a local full-suite run needs all + **five storage variables** controlled until this lands: + `XDG_CONFIG_HOME`, `XDG_DATA_HOME`, `XDG_STATE_HOME`, + `XDG_CACHE_HOME`, and `PMACS_STATE_HOME`. Naming only the four XDG + variables leaves a higher-precedence state override live. +12. **A durable adoption ratchet**, not a one-time census: a checked + source inventory that fails when a new ambient + `EditorState::new()`/`open(` appears in `tests/` outside a narrow, + named allowlist. Acceptance 1 proves today's state; this keeps it. + Falsified by adding an ambient constructor to any suite and + observing the inventory test fail. +13. The journey mechanism of §1.10 is implemented and pinned: a plain + `cargo test --test journey_acceptance`, with a hostile `init.lua` + present and no external wrapper, is green and leaves the hostile + root unmodified. + + +## 5. Parked + +- **Any change to how production resolves ambient roots.** The default + stays ambient; only construction gains a parameter. +- **A hostile-config CI leg.** Superseded by Q#TI5's self-spawn, which + is strictly stronger — it travels with the test rather than with the + workflow file. Recorded because revision 1 both proposed and parked + it. +- **Cleaning up whatever already wrote into + `~/.local/share/pmacs/builtin-packages/`.** Out of scope; this lane + stops the writes, it does not audit the past. +- **The `crdt` half of the corpus being dark in CI** — separate lane. + + +## 6. Gates + +Standard suite, each its own step with a real exit status and nothing +after the command that could mask it. + +**Isolate every storage root, not just the config one.** Rev 2's gate +set only `XDG_CONFIG_HOME`, which stops the `init.lua` reads and leaves +the data root live — so an "isolated" run could still write to +`~/.local/share/pmacs`. **Every local gate run in this repository today +had that hole.** The correct invocation sets, to a fresh directory: + +``` +XDG_CONFIG_HOME XDG_DATA_HOME XDG_STATE_HOME XDG_CACHE_HOME +PMACS_STATE_HOME +``` + +`PMACS_STATE_HOME` must be redirected or explicitly removed; it takes +precedence over `XDG_STATE_HOME`. `HOME` is deliberately left alone +(§1.6b). + +**Run the suite twice: once isolated, once with a hostile environment** +— an `init.lua` that would break a known assertion, plus a pre-seeded +data root — and assert the hostile root is **byte-identical +afterwards**. A lane about ambient state verified only in a clean +environment has not been verified. + +## 7. Branch plan + +One branch, one PR. Bet 1's classification first and alone — it decides +how large the change is, and its answer belongs in review before any +mechanical edit rides on it.