From b3131dbf959bef84d29ab0c154b398a7b56d836f Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Fri, 31 Jul 2026 19:46:33 -0400 Subject: [PATCH] =?UTF-8?q?docs(framing):=20record=20the=20deliberate=20de?= =?UTF-8?q?parture=20from=20the=20=C2=A77=20branch=20plan?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §7 said the classification comes first and alone, "its answer belongs in review before any mechanical edit rides on it." The classification came back at 342 sites across 66 of 97 files, and the whole-corpus migration rode this PR anyway. That was a decision, not an oversight, and revision 5 records it as one so a later reader does not have to reconstruct it from the diff. The reasoning, in short: splitting would either leave 65 suites still writing the developer's real data root while the seam sat unused, or ship acceptance 12's ratchet with a ~65-file allowlist — and a ratchet exempting most of the corpus records rather than ratchets. §7's ORDERING is honoured (the census is the first commit); its implied SCOPING is not. Revision 5 also records what review round 1 established about the shape acceptance 12 needs: "a narrow, named allowlist" is not sufficient by itself, because narrowness constrains which files are exempt and says nothing about how far each exemption stretches. Exemptions carry counts. `docs/active-work.md` picks up the same two facts and the PR number. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T --- docs/active-work.md | 12 +++- docs/test-ambient-config-isolation-framing.md | 55 +++++++++++++++++-- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index 625509f..75eed40 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -417,8 +417,16 @@ compatible. ## Test ambient-root isolation — IMPLEMENTATION OPEN - **Framing MERGED (#201)**; - `docs/test-ambient-config-isolation-framing.md` revision 4, three - review rounds closed (eight blocking, six major, all accepted). + `docs/test-ambient-config-isolation-framing.md` **revision 5** + (revision 4 approved after three review rounds — eight blocking, six + major, all accepted; revision 5 rides the implementation PR and records + findings, not a new design round). +- **PR #206 OPEN**, one review round closed. **§7's branch plan was + consciously exceeded**: the whole-corpus migration rides this PR rather + than a follow-up lane, because splitting would either leave 65 suites + writing the real data root or ship acceptance 12's ratchet with a + ~65-file allowlist that does not ratchet. Recorded in framing revision + 5; accepted in review. - **Implementation branch `test-ambient-isolation-impl`**, worktree `../pmacs-test-isolation-impl`, based on `githubsucks/main` @ `54a092e`. The framing-only worktree `../pmacs-test-isolation` diff --git a/docs/test-ambient-config-isolation-framing.md b/docs/test-ambient-config-isolation-framing.md index d9971ba..b8aa64e 100644 --- a/docs/test-ambient-config-isolation-framing.md +++ b/docs/test-ambient-config-isolation-framing.md @@ -1,9 +1,11 @@ # 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). +**Revision 5.** Status: implemented on branch +`test-ambient-isolation-impl` (worktree `../pmacs-test-isolation-impl`, +based on `githubsucks/main` @ `54a092e`), PR #206. Revision 4 was +approved after three review rounds and merged as #201; revision 5 records +implementation findings and one **deliberate departure from §7's branch +plan**, not a new design round. **The suite is green in CI and red on a developer machine that has a real `~/.config/pmacs/init.lua`.** Not flaky — deterministic, and @@ -16,6 +18,51 @@ directory**. The lane is therefore about *ambient roots*, not about ## Revision history +**Revision 4 → 5**, at implementation. No design changed; two things are +recorded that a later reader would otherwise have to reconstruct. + +### The §7 branch plan was consciously exceeded + +**§7 said: classification first and alone, "its answer belongs in review +before any mechanical edit rides on it."** The classification came back +at **342 in-process construction sites across 66 of 97 files** — large +enough that §7's shape would naturally suggest splitting the mechanical +migration into its own lane. **It was not split, and that is deliberate.** + +The alternatives were both worse, and both worse in the way this lane +exists to prevent: + +- **Split, migrating nothing now.** The seam would land while 65 files + kept reading the developer's real `init.lua` and writing their real + data root. The lane's own defect would survive its own PR. +- **Split, with a broad temporary allowlist.** Acceptance 12's ratchet + would ship exempting ~65 files. A ratchet whose allowlist is most of + the corpus does not ratchet; it records. And the exemption would have + to be removed later by the same reviewer who granted it, with nothing + failing in the meantime to remind anyone. + +So the whole-corpus migration is the atomic remediation and rides this +PR. §7's *ordering* is honoured — the classification is the first commit, +before any mechanical edit — while its implied *scoping* is not. The +qualification this leaves on acceptance 12 (the allowlist is narrow +because everything else moved, not because the change was small) was +raised in the PR and accepted in review round 1. + +### The exemption shape acceptance 12 needs + +Review round 1 found an ambient `EditorState::new()` in an ordinary +parent test of the isolation suite itself — the exposure, committed by +the suite written to remove it. **A file-level allowlist did not catch +it**, because the file was already exempted for its re-exec'd positive +control, and a bare exemption licenses a named file to grow new ambient +sites indefinitely. + +The fix is that **every exemption carries its exact site count**, so an +added site fails even inside an exempted file. That is the shape +acceptance 12 needs; "a narrow, named allowlist" is not sufficient on its +own, because narrowness constrains which files are exempt and says +nothing about how far each exemption stretches. + **Revision 3 → 4**, after review round 3 (one blocking, two major). All three accepted.