From 4e5df592c642d28c68613aaa719792554da57726 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 6 Aug 2026 13:59:32 -0500 Subject: [PATCH 1/3] fix(dev): anchor setup-leak-gate.ps1 to its own checkout, not the cwd (BACKLOG #1063) `$repo` came from `git rev-parse --show-toplevel`, which resolves against the CURRENT directory rather than the path the script was handed. Invoked by absolute `-File` path from another worktree -- the ordinary shape on a clone carrying dozens of them -- it armed the CALLER's checkout and printed CONFIGURED about that one, while the checkout the operator named kept no token source and went on failing closed. An absolute `-File` invocation is naming the checkout to act on; it must not then consult a different one. Now `Split-Path -Parent (Split-Path -Parent $PSScriptRoot)`, the form postgres.ps1:37 and sqlserver.ps1:56 in the same directory already use, plus an assert that the derived root actually carries scripts/security/ -- a wrong root should say so where it is derived rather than surface later as a confusing scanner failure. Tested by the DIVERGENCE, which is the only shape that can fail: two temp checkouts that both carry scripts/security/, the script invoked by absolute path while the shell stands in the other one. A test run from inside the target passes with the bug still in, because cwd and script root are then the same directory. Reverted to the old line, the same test reports "the named checkout was not armed" -- the negative control was run, not assumed. The fixture copies only the three files the script reaches for, never the whole of scripts/security/: a maintainer running this suite has the real token list sitting in that directory, and a copytree would sweep it into a temp dir. Also corrects this item's own prose. It called alloc.ps1:51 "byte-equivalent"; it is not -- alloc.ps1 carries --path-format=absolute and this script does not. The defect is identical, the bytes are not, and "byte-equivalent" is the kind of claim a later reader greps for and then trusts. --- docs/BACKLOG.md | 4 +- scripts/dev/setup-leak-gate.ps1 | 18 ++++- tests/test_script_root_anchoring.py | 113 ++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 4 deletions(-) create mode 100644 tests/test_script_root_anchoring.py diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 8f5a5835..1cf1727f 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -5613,7 +5613,7 @@ The second step's arithmetic is measured: `GetFullPath('.git', )` r ## 1063. `setup-leak-gate.ps1` picks the checkout from the current directory, so it can arm a worktree the operator did not name -> ๐Ÿ”ข **Filed 2026-08-06 โ€” not started.** Value **3/10** ยท Difficulty **1/10** ยท _quick win_. `scripts/dev/setup-leak-gate.ps1:37` is `$repo = (& git rev-parse --show-toplevel 2>$null)` โ€” no `-C`, no `-Repo` parameter, no `$PSScriptRoot` anchor. Invoked by absolute `-File` path from a different worktree, which is the ordinary shape on a clone with 40-plus of them, it installs the leak-gate token list into **the current directory's** checkout and prints `CONFIGURED` about that one, while the worktree the operator named keeps no token source. Its own directory siblings already do it correctly. +> โœ… **SHIPPED 2026-08-06 โ€” the root is anchored to the script's own location, and the divergence is under test.** Value **3/10** ยท Difficulty **1/10** ยท _quick win_. `$repo` now comes from `Split-Path -Parent (Split-Path -Parent $PSScriptRoot)`, the form `postgres.ps1` and `sqlserver.ps1` in the same directory already use, plus an assert that the derived root actually carries `scripts/security/` โ€” a wrong root should say so at the point of derivation rather than surface later as a confusing scanner failure. Tested by the **divergence**, not the happy path: two temp checkouts that both carry `scripts/security/`, the script invoked by absolute `-File` path while the shell stands in the other one, asserting the token list lands in the checkout holding the script and **not** in the caller's. The pre-fix behaviour was reproduced directly rather than inferred โ€” reverted, the same test reports *"the named checkout was not armed"*. Only the three files the script reaches for are copied into the fixture, never the whole of `scripts/security/`, because a maintainer running the suite has the real token list sitting in that directory. Original filing follows. `scripts/dev/setup-leak-gate.ps1:37` was `$repo = (& git rev-parse --show-toplevel 2>$null)` โ€” no `-C`, no `-Repo` parameter, no `$PSScriptRoot` anchor. Invoked by absolute `-File` path from a different worktree, which is the ordinary shape on a clone with 40-plus of them, it installs the leak-gate token list into **the current directory's** checkout and prints `CONFIGURED` about that one, while the worktree the operator named keeps no token source. Its own directory siblings already do it correctly. **Cluster:** Developer tooling / configuration anchoring. **Priority:** P4. **Verdict:** build (trivial). **Severity:** **low, and the low severity is load-bearing** โ€” every failure direction here is loud or fail-closed, which is why this is filed at 3 rather than alongside its siblings. Nothing is silently ungated and no wrong authorisation is granted. @@ -5627,7 +5627,7 @@ $repo = Split-Path -Parent (Split-Path -Parent $PSScriptRoot) Anchoring on `$PSScriptRoot` binds the script to the checkout it lives in, which is what an absolute `-File` invocation is asking for. `-From` names the token **source**, not the checkout, so it does not already cover this. -**Same construct as #1060.** `alloc.ps1:51` is byte-equivalent (`git rev-parse --show-toplevel` with no anchor) and produces the same class of wrong answer โ€” there, a misattributed ledger allocation; here, a token list installed into the wrong tree. Fixing them together is reasonable; filing them together was not, because their severities differ by two priority bands and folding this into #1060 would have inflated it. +**Same construct as #1060.** `alloc.ps1:51` is the same construct โ€” `git rev-parse --show-toplevel` with no anchor โ€” and produces the same class of wrong answer โ€” there, a misattributed ledger allocation; here, a token list installed into the wrong tree. Fixing them together is reasonable; filing them together was not, because their severities differ by two priority bands and folding this into #1060 would have inflated it. (The filing said *byte-equivalent*. It is not: `alloc.ps1` carries `--path-format=absolute` and this script does not. The defect is identical; the bytes are not, and "byte-equivalent" is the kind of claim a later reader greps for and then trusts.) **How it was found, and why that matters more than the defect.** A repo-wide sweep for the cwd-as-identity shape assigned five surfaces and left `scripts/dev` and `scripts/service` in **no** surface at all โ€” seven `.ps1` files in the seam. This was found only because the synthesising agent went outside its brief and swept the unassigned region. A measuring apparatus with a blind spot, hunting mechanisms with blind spots. Worth remembering when the next sweep is designed: **state the unassigned regions, or the result reads as completeness.** diff --git a/scripts/dev/setup-leak-gate.ps1 b/scripts/dev/setup-leak-gate.ps1 index 831eb833..9747f843 100644 --- a/scripts/dev/setup-leak-gate.ps1 +++ b/scripts/dev/setup-leak-gate.ps1 @@ -34,9 +34,23 @@ param( ) $ErrorActionPreference = 'Stop' -$repo = (& git rev-parse --show-toplevel 2>$null) -if (-not $repo) { throw 'Not inside a git checkout.' } + +# Repo root = parent of scripts\ = parent of this script's dir (scripts\dev). Same form postgres.ps1 and +# sqlserver.ps1 in this directory already use. +# +# NOT `git rev-parse --show-toplevel`: that resolves against the CURRENT DIRECTORY, not against the path +# this script was handed. Invoked by absolute -File path from another worktree -- the ordinary shape on a +# clone carrying dozens of them -- it armed the CALLER's checkout and printed CONFIGURED about that one, +# while the checkout the operator named kept no token source and went on failing closed (BACKLOG #1063). +# An absolute -File invocation is naming the checkout to act on; it must not then consult a different one. +# -From names the token SOURCE, not the checkout, so it never covered this. +$repo = Split-Path -Parent (Split-Path -Parent $PSScriptRoot) $secDir = Join-Path $repo 'scripts/security' +# ASSERT what the derivation only IMPLIES. Discovering a wrong root later, as a confusing scanner +# failure, is how this class stays invisible; new.ps1 takes the same posture after deriving its path. +if (-not (Test-Path -LiteralPath $secDir)) { + throw "Derived repo root has no scripts/security -- this script must live in /scripts/dev/: $repo" +} $local = Join-Path $secDir 'scan-tokens.local.txt' $example = Join-Path $secDir 'scan-tokens.local.txt.example' $scanner = Join-Path $secDir 'scan_forbidden.py' diff --git a/tests/test_script_root_anchoring.py b/tests/test_script_root_anchoring.py new file mode 100644 index 00000000..0b12e835 --- /dev/null +++ b/tests/test_script_root_anchoring.py @@ -0,0 +1,113 @@ +"""Scripts invoked by absolute ``-File`` path must act on the checkout they LIVE in, not the cwd. + +The regression home for the "cwd is not the caller" family (BACKLOG #1057, #1059, #1060, #1062, #1063). +Three mechanisms in this repo silently assumed that where a command runs is where the caller is, and all +three failed in the benign-looking direction -- a wrong owner recorded, a gate armed in the wrong tree, an +occupancy of zero for a tree in active use. None of them raised. + +**Every test here asserts the DIVERGENCE, never the happy path.** A script run from inside its own +checkout passes with the bug still in: cwd and script root are the same directory, so the two candidate +answers are indistinguishable. The case that can tell them apart is an absolute ``-File`` invocation whose +cwd is a DIFFERENT checkout that also carries the file the script writes -- which is the ordinary shape on +a clone carrying dozens of worktrees, and is measured at 29% of writes on this repo. Per BACKLOG #1000 a +control needs the case that can distinguish; a test run from inside the target proves nothing. + +The static spelling guards below are deliberately paired with a behavioural test each. On their own they +would be exactly the green-because-it-cannot-see control this project keeps filing. +""" + +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path + +import pytest + +_ROOT = Path(__file__).resolve().parents[1] +_SETUP_LEAK_GATE = _ROOT / "scripts" / "dev" / "setup-leak-gate.ps1" +_SECURITY = _ROOT / "scripts" / "security" + +pytestmark = pytest.mark.skipif( + shutil.which("pwsh") is None, reason="pwsh (PowerShell 7) not on PATH" +) + + +def _git(*args: str, cwd: Path) -> None: + subprocess.run(["git", *args], cwd=str(cwd), check=True, capture_output=True) + + +def _leak_gate_checkout(path: Path) -> Path: + """A minimal checkout carrying setup-leak-gate.ps1 and the files it reaches for. + + Only the three files the script needs are copied, NOT the whole of scripts/security. A maintainer + running this suite has the real ``scan-tokens.local.txt`` sitting in that directory, and a copytree + would sweep the private token list into a temp dir -- the one mistake the script itself refuses to + make (it deletes what it wrote rather than risk committing the list). + """ + (path / "scripts" / "dev").mkdir(parents=True) + (path / "scripts" / "security").mkdir(parents=True) + shutil.copy2(_SETUP_LEAK_GATE, path / "scripts" / "dev" / "setup-leak-gate.ps1") + for name in ("scan_forbidden.py", "scan-allowlist.txt", "scan-tokens.local.txt.example"): + shutil.copy2(_SECURITY / name, path / "scripts" / "security" / name) + # check-ignore reads the working-tree .gitignore, so no commit is needed -- but the repo must exist, + # or the script's own not-git-ignored guard deletes the file it just wrote and throws. + (path / ".gitignore").write_text("scripts/security/scan-tokens.local.txt\n", encoding="utf-8") + _git("init", "-b", "main", ".", cwd=path) + return path + + +def test_setup_leak_gate_arms_the_checkout_it_lives_in_not_the_cwd(tmp_path: Path) -> None: + """BACKLOG #1063. Reproduced by the divergence, which is the only shape that can fail. + + Pre-fix, ``$repo`` came from ``git rev-parse --show-toplevel`` -- the CURRENT directory -- so an + absolute ``-File`` invocation from another worktree installed the token list into the caller's tree + and printed CONFIGURED about it, while the tree the operator named kept no token source. Both + checkouts here carry ``scripts/security/``, so "which tree got the file" is the whole question and + neither answer is available by accident. + + The verify step's exit code is deliberately NOT asserted: it shells out to ``python``, which is not + guaranteed on PATH on every CI leg, and the copy has already happened by then. Placement is the fact + under test. + """ + named = _leak_gate_checkout(tmp_path / "Named") + caller = _leak_gate_checkout(tmp_path / "Caller") + + subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(named / "scripts" / "dev" / "setup-leak-gate.ps1"), + "-Synthetic", + ], + cwd=str(caller), # THE POINT: the shell is standing somewhere else entirely + capture_output=True, + text=True, + timeout=180, + ) + + installed = named / "scripts" / "security" / "scan-tokens.local.txt" + stray = caller / "scripts" / "security" / "scan-tokens.local.txt" + assert installed.is_file(), "the named checkout was not armed -- it read the cwd instead" + assert not stray.exists(), f"armed the CALLER's checkout instead: {stray}" + + +def test_setup_leak_gate_does_not_reintroduce_an_unanchored_toplevel(tmp_path: Path) -> None: + """A spelling guard for the regression, paired with the behavioural test above. + + Alone this would be worthless -- it cannot see the defect, only its most likely spelling. It earns + its place by naming the exact construct so a reader who reaches for ``--show-toplevel`` again gets + told why not, at the moment they do it. + """ + text = _SETUP_LEAK_GATE.read_text(encoding="utf-8") + assert "$PSScriptRoot" in text, "the repo root must be anchored to the script's own location" + for line in text.splitlines(): + stripped = line.strip() + if stripped.startswith("#") or "--show-toplevel" not in stripped: + continue + assert "-C " in stripped, ( + "an unanchored `git rev-parse --show-toplevel` resolves against the CURRENT directory, " + f"which is BACKLOG #1063: {stripped}" + ) From e4bf77f5e211fb909f8af3ab2effb16243185b19 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 6 Aug 2026 14:05:52 -0500 Subject: [PATCH 2/3] fix(coord): anchor alloc.ps1 and claim.ps1 to their own checkout (BACKLOG #1060) Both took `$repo` from an unanchored `git rev-parse --show-toplevel`, which resolves against the CURRENT directory rather than the path the script was handed. Invoked by absolute `-File` path from worktree A while intending to commit from worktree B, the claim was recorded to A; the ledger gate then refused B's commit -- correctly, it fails closed -- but far from the cause and with a message about the wrong thing, and it cost a number. `git -C $PSScriptRoot`, not `Split-Path`. The recorded `worktree` value has THREE readers: ledger_check.py:227, this script's own `-List`, and prune-merged.ps1's orphan-claim release, whose comment at :787 names the producing command -- "records `worktree = $repo` from `git rev-parse --path-format=absolute`" -- and matches on the full normalised path because a false positive there hands a live session's key to someone else. All three fold separators, so `Split-Path` would not have broken anything; it would have silently falsified that comment, in a destructive tool, for no gain. THE FILING NAMED ONE OF FOUR CWD-DERIVED READS, and the other three are measured in the negative control below: * the `branch` recorded with the claim was the CALLER's branch; * the floor's boundary was parsed from the CALLER's scripts/hooks/ledger_check.py; * the floor's WORKING-TREE term read the CALLER's docs/BACKLOG.md. The third is not friction and the item's severity paragraph is corrected in the same commit. That term exists to catch a number written but committed NOWHERE. Reading the caller's tree makes a number drafted in the target worktree invisible, so the allocator hands it out as free and two items share it -- both owned by that worktree, so owns() passes and the ledger gate never fires. The silent collision the docstring says this script exists to prevent, reached through the script. Narrow, since anything committed on any ref is still caught by the all-refs term, but a correctness hole rather than friction. claim.ps1:54 carried the same construct and was never filed -- found by inspection here, fixed in the same commit. Its enforcing hook, claim_check.py, reads the repo from cwd and is RIGHT to: a commit hook's cwd IS the committing worktree. Hook right, tool wrong, and only the tool can be invoked from somewhere else. Both scripts now print a NOTE when the shell is standing somewhere else. Anchoring is correct but surprising, and the item's other half -- showing the recorded worktree -- was already built (`claimed by:` / `by :`); what was missing is saying so when it diverges, instead of leaving it to surface as a refused commit later. Silent on the ordinary same-tree invocation, so it stays worth reading. THE FIX TURNED TWO SANDBOXED TEST FILES INTO WRITERS ON THE LIVE REGISTRY, which is worse than the red suite it also caused, and is the reason those fixtures changed here. test_coord_claim_{refresh,liveness}.py ran the REAL scripts/coord/claim.ps1 with cwd set to a temp repo -- scoped to a throwaway registry purely by ambient cwd, and one of them said so ("it scopes itself to the cwd's repo"). Once the script stopped consulting cwd, the passing half of the run wrote real claims into this clone's shared registry: two strays, `k` and a date-shaped key, were created and removed by hand. Both fixtures now stage and COMMIT a copy of the script inside the temp repo, so the sandbox is structural rather than ambient, and a linked worktree of the fixture carries its own copy -- which is how the peer-holds-the-key tests still produce a claim recorded against the peer. test_ledger_check.py already did exactly this for alloc.ps1, which is why it was the one that did not break. Tested by the DIVERGENCE, with -ShowFloor so no numbers are burned: allocation is a one-way door and a test that allocated would leave permanent holes in the shared registry for every worktree of this clone. Two temp checkouts draft different numbers and carry different PUBLIC_BACKLOG_FLOOR stubs; the caller's number is deliberately HIGHER, because the floor is a maximum and an equal or lower one would pass with the bug in. Reverted to the old lines, the same test reports floor 7777, boundary 1900 and a watermark under Caller/.git -- three independent signals, all pointing at the wrong tree. --- docs/BACKLOG.md | 4 +- scripts/coord/alloc.ps1 | 55 ++++++++++--- scripts/coord/claim.ps1 | 31 +++++-- tests/test_coord_claim_liveness.py | 16 +++- tests/test_coord_claim_refresh.py | 24 +++++- tests/test_script_root_anchoring.py | 120 ++++++++++++++++++++++++++++ 6 files changed, 229 insertions(+), 21 deletions(-) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 1cf1727f..41556cf3 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -5537,10 +5537,12 @@ The literal spellings are all caught, including the newline form. Only the indir ## 1060. `alloc.ps1` records the owning worktree from the current directory, so an absolute-path invocation misattributes it -> ๐Ÿ”ข **Filed 2026-08-05 โ€” not started.** Value **5/10** ยท Difficulty **2/10** ยท _quick win_. `scripts/coord/alloc.ps1:51` takes the owner from `git rev-parse --show-toplevel`, which resolves against the **current directory** rather than the script's location. Invoke it by absolute `-File` path from a different worktree โ€” which is how a session with several worktrees naturally calls it โ€” and the allocation is recorded to the caller's worktree while the commit comes from another. The ledger gate then refuses that commit correctly, but far away from the cause and with a message about the wrong thing. +> โœ… **SHIPPED 2026-08-06 โ€” every `git` call in both allocators is anchored to the script's own checkout, and the defect was larger than filed.** Value **5/10** ยท Difficulty **2/10** ยท _quick win_. `$repo` now comes from `git -C $PSScriptRoot rev-parse --path-format=absolute --show-toplevel`, and every subsequent call takes `-C $repo`. **`git -C`, not `Split-Path`:** the recorded `worktree` value is *compared* โ€” by `ledger_check.py:227` and by `-List` โ€” so its string form is part of a contract, and `--path-format=absolute` keeps writing the forward-slash form every claim already on disk carries. **The filing named one of four cwd-derived reads.** Also wrong, and measured on the pre-fix code: the `$branch` recorded with the claim was the *caller's* branch; the floor's boundary was parsed from the **caller's** `scripts/hooks/ledger_check.py`; and the floor's working-tree term โ€” the one whose job is to catch a number written but committed **nowhere** โ€” read the **caller's** `docs/BACKLOG.md`. That last one is not friction: a number drafted in the target worktree was invisible to the sweep and free to re-issue, which is the collision this script exists to prevent. **`scripts/coord/claim.ps1:54` carried the same construct and was never filed** โ€” found by inspection while fixing this, fixed in the same commit; its enforcing hook `claim_check.py` reads the repo from cwd and is *right* to, because a commit hook's cwd **is** the committing worktree. Hook right, tool wrong, and only the tool can be invoked from elsewhere. The item's cheap second half โ€” printing the recorded worktree at allocation time โ€” was **already built** (`claimed by:`); what was missing is a note when the shell is standing somewhere else, which is now printed. Tested by the **divergence** with `-ShowFloor`, so no numbers were burned: two temp checkouts drafting different numbers, the allocator invoked by absolute path from the other one. The negative control was run, not assumed โ€” reverted, the same test reports `floor : 7777`, `boundary : 1900` and a watermark under `Caller/.git/`, all three signals pointing at the wrong tree. Original filing follows. `scripts/coord/alloc.ps1:51` took the owner from `git rev-parse --show-toplevel`, which resolves against the **current directory** rather than the script's location. Invoke it by absolute `-File` path from a different worktree โ€” which is how a session with several worktrees naturally calls it โ€” and the allocation is recorded to the caller's worktree while the commit comes from another. The ledger gate then refuses that commit correctly, but far away from the cause and with a message about the wrong thing. **Cluster:** Session coordination / ledger integrity. **Priority:** P3. **Verdict:** build (small). **Severity:** no data loss and no security effect โ€” the ledger gate **fails closed**, which is why this is a friction defect and not a correctness one. Nothing invalid lands; a valid commit is refused. +**SEVERITY CORRECTION, 2026-08-06, made while fixing it.** The paragraph above is right about the *recorded owner* and wrong about the item as a whole, because the filing looked at one line and the defect was in four. The ledger gate does fail closed on a misattributed claim โ€” but the floor's **working-tree term** was cwd-derived too, and that term has no gate behind it. Its job is to see a number written to `docs/BACKLOG.md` and committed **nowhere**; reading the caller's tree makes a number drafted in the *target* worktree invisible, so the allocator hands it out as free and two items end up sharing it in the same tree. Both are then owned by that worktree, so `owns()` passes and the ledger gate never fires. That is the exact silent collision the script's own docstring says it exists to prevent, arrived at through the script. The window is narrow โ€” anything committed on any ref is still caught by the all-refs term โ€” but it is a correctness hole, not friction, and it was reproduced in the negative control rather than reasoned about. + **Reproduced 2026-08-05, twice, by accident.** A session ran `pwsh -NoProfile -File /scripts/coord/alloc.ps1 -Kind backlog` from worktree A while intending to commit from worktree B. `alloc/backlog/1058.json` recorded `"worktree": "<...>/trusting-wu-c2e6d5"`. The commit from `MessageFoundry-gate-deferrals` was then refused: *"BACKLOG item #1058 was not allocated to this worktree"* โ€” true, unhelpful, and pointing at the allocator rather than at the invocation. Re-running with the shell actually inside the target worktree produced `1059.json` with the right owner and the commit went through. **#1058 is an abandoned hole**, which is the sanctioned outcome (`alloc.ps1`'s own docstring: *holes are free, collisions are not*). **The fix is small and there are two defensible shapes.** Either derive the repo from `$PSScriptRoot` so the allocator is anchored to the checkout it lives in โ€” matching what `new.ps1` and `remove.ps1` already do โ€” or keep the cwd behaviour and **say so at the point of use**, printing the recorded worktree in the `ALLOCATED` output so the mismatch is visible immediately rather than at commit time. The second is weaker but nearly free, and the two compose. Prefer anchoring: an allocator invoked by absolute path is being told which checkout to act on, and it should not then consult a different one. diff --git a/scripts/coord/alloc.ps1 b/scripts/coord/alloc.ps1 index 0daca930..fd5b3fbb 100644 --- a/scripts/coord/alloc.ps1 +++ b/scripts/coord/alloc.ps1 @@ -48,9 +48,27 @@ param( $ErrorActionPreference = "Stop" -$repo = (& git rev-parse --path-format=absolute --show-toplevel).Trim() -if (-not $repo) { throw "Not inside a git repository." } -$common = (& git rev-parse --path-format=absolute --git-common-dir).Trim() +# ANCHOR ON THE SCRIPT, NOT ON THE CURRENT DIRECTORY (BACKLOG #1060). Unanchored, every `git` call below +# resolved against wherever the shell happened to be standing, so an absolute `-File` invocation from +# worktree A while intending to commit from worktree B recorded the claim to A. The ledger gate then +# refused B's commit -- correctly, fails closed, nothing invalid lands -- but far from the cause and with +# a message about the wrong thing. It also cost a number: holes are free, collisions are not. +# +# `git -C $PSScriptRoot` rather than `Split-Path`, which is what the sibling scripts in scripts/dev use. +# The recorded `worktree` value is COMPARED, by ledger_check.py:227 and by -List below, so its string form +# is part of a contract: `--path-format=absolute` returns the forward-slash absolute form every claim +# already on disk carries, and `Split-Path` would start writing backslashes into the same field. Both +# comparisons normalise separators today, so this is not a live break -- it is a field whose format +# nothing pins, and changing it for no reason is how the next reader's grep stops matching. +# +# Every `git` call in this file is anchored the same way for the same reason. The floor's working-tree +# term (below) is the one where an unanchored read is more than misattribution: it looks for numbers +# written but committed NOWHERE, so reading the caller's tree makes a number drafted in the TARGET tree +# invisible and free to re-issue -- the collision this whole script exists to prevent. +$repo = (& git -C $PSScriptRoot rev-parse --path-format=absolute --show-toplevel 2>$null) +if (-not $repo) { throw "scripts/coord/ is not inside a git repository: $PSScriptRoot" } +$repo = $repo.Trim() +$common = (& git -C $repo rev-parse --path-format=absolute --git-common-dir).Trim() $allocRoot = Join-Path $common "mefor-coord/alloc" $alloc = Join-Path $allocRoot $Kind New-Item -ItemType Directory -Force -Path $alloc | Out-Null @@ -72,8 +90,8 @@ if (-not $Title -and -not $ShowFloor) { throw "-Title is required (it is recorde # `git branch --show-current` prints NOTHING on a detached HEAD, so `& git ...` yields $null (not "") # -- calling .Trim() on it here threw *before* the detached-HEAD fallback below could run. Null-check first. -$branch = & git branch --show-current -if ([string]::IsNullOrWhiteSpace($branch)) { $branch = "detached@" + (& git rev-parse --short HEAD) } +$branch = & git -C $repo branch --show-current +if ([string]::IsNullOrWhiteSpace($branch)) { $branch = "detached@" + (& git -C $repo rev-parse --short HEAD) } $branch = $branch.Trim() # FLOOR = max over (origin/main) U (every local + remote ref) U (existing allocations). @@ -88,9 +106,9 @@ function Get-Floor { $seen.Add(0) if ($Kind -eq "adr") { - $refs = @("origin/main") + @(& git for-each-ref --format='%(refname)' refs/heads refs/remotes) + $refs = @("origin/main") + @(& git -C $repo for-each-ref --format='%(refname)' refs/heads refs/remotes) foreach ($ref in ($refs | Select-Object -Unique)) { - $names = & git ls-tree --name-only $ref docs/adr/ 2>$null + $names = & git -C $repo ls-tree --name-only $ref docs/adr/ 2>$null foreach ($n in $names) { if ($n -match 'docs/adr/(\d{4})-') { $seen.Add([int]$Matches[1]) } } @@ -117,13 +135,13 @@ function Get-Floor { # files means adding each one here; an archive file not listed here is not policed. $backlogPaths = @("docs/BACKLOG.md", "docs/archive/backlog/BACKLOG-CLOSED.md") - $refs = @("origin/main", "HEAD") + @(& git for-each-ref --format='%(refname)' refs/heads refs/remotes) + $refs = @("origin/main", "HEAD") + @(& git -C $repo for-each-ref --format='%(refname)' refs/heads refs/remotes) $specs = foreach ($r in ($refs | Select-Object -Unique)) { foreach ($p in $backlogPaths) { "${r}:${p}" } } $oids = [System.Collections.Generic.HashSet[string]]::new() - foreach ($line in ($specs -join "`n" | & git cat-file --batch-check='%(objectname) %(objecttype)' 2>$null)) { + foreach ($line in ($specs -join "`n" | & git -C $repo cat-file --batch-check='%(objectname) %(objecttype)' 2>$null)) { $p = "$line".Split(' ') if ($p.Count -ge 2 -and $p[1] -eq 'blob') { [void]$oids.Add($p[0]) } } @@ -138,7 +156,7 @@ function Get-Floor { # that had been committed somewhere -- i.e. every case except the one this term is for. $rx = [regex]::new('^#{2,3} (\d+)\.', [System.Text.RegularExpressions.RegexOptions]::Multiline) if ($oids.Count -gt 0) { - foreach ($line in (($oids -join "`n") | & git cat-file --batch 2>$null)) { + foreach ($line in (($oids -join "`n") | & git -C $repo cat-file --batch 2>$null)) { $m = $rx.Match("$line") if ($m.Success) { $seen.Add([int]$m.Groups[1].Value) } } @@ -410,6 +428,23 @@ for ($i = $start; $i -lt $start + 500; $i++) { Write-Host " file : docs/BACKLOG.md" } Write-Host " claimed by: $repo [$branch]" + + # SAY IT AT THE POINT OF USE when the shell is standing somewhere else (BACKLOG #1060). Anchoring is + # now correct, but it is also SURPRISING: a caller who runs this by absolute path from worktree A gets + # a claim recorded to worktree B, and the only other place that fact surfaces is the ledger gate + # refusing a commit later, elsewhere, with a message about the wrong thing. One line here turns a + # deferred, misdirected refusal into an immediate, accurate note. Silent on the ordinary same-tree + # invocation, so it stays worth reading. + $cwdTop = (& git rev-parse --path-format=absolute --show-toplevel 2>$null) + if ($cwdTop) { + $a = ($cwdTop.Trim() -replace '\\', '/').TrimEnd('/') + $b = ($repo -replace '\\', '/').TrimEnd('/') + if ($a -ine $b) { + Write-Host " NOTE: your shell is in $a, but this allocator lives in $b, so the claim is recorded" -ForegroundColor Yellow + Write-Host " to $b. COMMIT FROM THERE -- the ledger gate keys entitlement on the worktree" -ForegroundColor Yellow + Write-Host " named above and will refuse the commit anywhere else." -ForegroundColor Yellow + } + } exit 0 } diff --git a/scripts/coord/claim.ps1 b/scripts/coord/claim.ps1 index b7b86057..308a2172 100644 --- a/scripts/coord/claim.ps1 +++ b/scripts/coord/claim.ps1 @@ -51,9 +51,17 @@ param( $ErrorActionPreference = "Stop" -$repo = (& git rev-parse --path-format=absolute --show-toplevel).Trim() -if (-not $repo) { throw "Not inside a git repository." } -$common = (& git rev-parse --path-format=absolute --git-common-dir).Trim() +# Anchored on the SCRIPT, not the current directory -- the reasoning is written out once, at the head of +# alloc.ps1 (BACKLOG #1060). It applies here identically and was found here by inspection rather than by +# a second reproduction: this file recorded `worktree = $repo` from the same unanchored call, so an +# absolute `-File` invocation from another worktree took the claim in the caller's name. A claim is +# advisory for free-text keys and ENFORCED for numbered ones by scripts/hooks/claim_check.py, which reads +# the repo from cwd correctly because it runs as a commit hook -- cwd IS the committing worktree there. +# Hook right, tool wrong, and only the tool can be invoked from somewhere else. +$repo = (& git -C $PSScriptRoot rev-parse --path-format=absolute --show-toplevel 2>$null) +if (-not $repo) { throw "scripts/coord/ is not inside a git repository: $PSScriptRoot" } +$repo = $repo.Trim() +$common = (& git -C $repo rev-parse --path-format=absolute --git-common-dir).Trim() $claims = Join-Path $common "mefor-coord/claims" New-Item -ItemType Directory -Force -Path $claims | Out-Null @@ -202,8 +210,8 @@ if ($List) { Show-List; exit 0 } $safe = ConvertTo-KeyFile $Take $file = Join-Path $claims "$safe.json" -$branch = & git branch --show-current -if ([string]::IsNullOrWhiteSpace($branch)) { $branch = "detached@" + (& git rev-parse --short HEAD) } +$branch = & git -C $repo branch --show-current +if ([string]::IsNullOrWhiteSpace($branch)) { $branch = "detached@" + (& git -C $repo rev-parse --short HEAD) } $branch = $branch.Trim() try { @@ -345,4 +353,17 @@ Write-Host "CLAIMED '$Take'" -ForegroundColor Green Write-Host " by : $repo [$branch]" Write-Host " note : $(if ($Note) { $Note } else { '(no note)' })" Write-Host " release when done: pwsh -NoProfile -File scripts\coord\claim.ps1 -Release $Take" + +# Same note alloc.ps1 prints, for the same reason (BACKLOG #1060): anchoring is correct but surprising, +# and a claim recorded to a worktree the caller is not standing in otherwise surfaces only as a refused +# commit later. Silent on the ordinary same-tree invocation. +$cwdTop = (& git rev-parse --path-format=absolute --show-toplevel 2>$null) +if ($cwdTop) { + $a = ($cwdTop.Trim() -replace '\\', '/').TrimEnd('/') + $b = ($repo -replace '\\', '/').TrimEnd('/') + if ($a -ine $b) { + Write-Host " NOTE: your shell is in $a, but this script lives in $b, so the claim is" -ForegroundColor Yellow + Write-Host " recorded to $b. -Release must be run against that same worktree." -ForegroundColor Yellow + } +} exit 0 diff --git a/tests/test_coord_claim_liveness.py b/tests/test_coord_claim_liveness.py index 04d81ef2..9bb784c3 100644 --- a/tests/test_coord_claim_liveness.py +++ b/tests/test_coord_claim_liveness.py @@ -50,10 +50,14 @@ def git(repo: Path, *args: str) -> str: @pytest.fixture def repo(tmp_path: Path) -> Path: r = tmp_path / "repo" - r.mkdir() + (r / "scripts" / "coord").mkdir(parents=True) subprocess.run(["git", "init", "-q", "-b", "main", str(r)], check=True, capture_output=True) git(r, "config", "user.email", "t@example.invalid") git(r, "config", "user.name", "t") + # Staged and committed for the reason written out in test_coord_claim_refresh.py's fixture: + # claim.ps1 anchors on its own location now (BACKLOG #1060), so the copy IS the sandbox, and a + # linked worktree of this fixture carries its own -- which is what `peer_holding` relies on. + shutil.copy2(CLAIM, r / "scripts" / "coord" / "claim.ps1") (r / "f.txt").write_text("x", encoding="utf-8") git(r, "add", "-A") git(r, "commit", "-qm", "base") @@ -61,8 +65,16 @@ def repo(tmp_path: Path) -> Path: def claim(cwd: Path, *args: str) -> subprocess.CompletedProcess[str]: + """Run the checkout's OWN copy -- it scopes itself to where it LIVES, not to the cwd.""" return subprocess.run( - ["pwsh", "-NoProfile", "-NonInteractive", "-File", str(CLAIM), *args], + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(cwd / "scripts" / "coord" / "claim.ps1"), + *args, + ], cwd=str(cwd), capture_output=True, text=True, diff --git a/tests/test_coord_claim_refresh.py b/tests/test_coord_claim_refresh.py index 49ef8a1f..6f65cb76 100644 --- a/tests/test_coord_claim_refresh.py +++ b/tests/test_coord_claim_refresh.py @@ -48,10 +48,17 @@ def git(repo: Path, *args: str) -> str: @pytest.fixture def repo(tmp_path: Path) -> Path: r = tmp_path / "repo" - r.mkdir() + (r / "scripts" / "coord").mkdir(parents=True) subprocess.run(["git", "init", "-q", "-b", "main", str(r)], check=True, capture_output=True) git(r, "config", "user.email", "t@example.invalid") git(r, "config", "user.name", "t") + # STAGE THE SCRIPT INSIDE THE FIXTURE, and commit it, because claim.ps1 anchors on its own location + # rather than on the cwd (BACKLOG #1060). This is also what makes the sandbox STRUCTURAL: these + # tests used to be scoped to a temp registry only by ambient cwd, so the moment the script stopped + # consulting cwd they began writing REAL claims into this clone's shared registry -- measured, two + # strays landed there on the first run after the fix. Committing it means a linked worktree of this + # fixture carries its own copy, which is how `peer_holding` still gets a claim held by the peer. + shutil.copy2(CLAIM, r / "scripts" / "coord" / "claim.ps1") (r / "f.txt").write_text("x", encoding="utf-8") git(r, "add", "-A") git(r, "commit", "-qm", "base") @@ -59,9 +66,20 @@ def repo(tmp_path: Path) -> Path: def claim(cwd: Path, *args: str) -> subprocess.CompletedProcess[str]: - """Run the real script from ``cwd`` -- it scopes itself to the cwd's repo, and has no -Repo.""" + """Run the checkout's OWN copy -- the script scopes itself to where it LIVES, not to the cwd. + + ``cwd`` still names the worktree under test, but it does so by HOLDING that copy rather than by + being the ambient directory, which is what a session invoking `scripts\\coord\\claim.ps1` does. + """ return subprocess.run( - ["pwsh", "-NoProfile", "-NonInteractive", "-File", str(CLAIM), *args], + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(cwd / "scripts" / "coord" / "claim.ps1"), + *args, + ], cwd=str(cwd), capture_output=True, text=True, diff --git a/tests/test_script_root_anchoring.py b/tests/test_script_root_anchoring.py index 0b12e835..5d51f4c6 100644 --- a/tests/test_script_root_anchoring.py +++ b/tests/test_script_root_anchoring.py @@ -18,6 +18,7 @@ from __future__ import annotations +import json import shutil import subprocess from pathlib import Path @@ -27,6 +28,8 @@ _ROOT = Path(__file__).resolve().parents[1] _SETUP_LEAK_GATE = _ROOT / "scripts" / "dev" / "setup-leak-gate.ps1" _SECURITY = _ROOT / "scripts" / "security" +_ALLOC = _ROOT / "scripts" / "coord" / "alloc.ps1" +_CLAIM = _ROOT / "scripts" / "coord" / "claim.ps1" pytestmark = pytest.mark.skipif( shutil.which("pwsh") is None, reason="pwsh (PowerShell 7) not on PATH" @@ -94,6 +97,123 @@ def test_setup_leak_gate_arms_the_checkout_it_lives_in_not_the_cwd(tmp_path: Pat assert not stray.exists(), f"armed the CALLER's checkout instead: {stray}" +def _coord_checkout(path: Path, *, drafted: int, boundary: int) -> Path: + """A minimal checkout carrying scripts/coord/ and the two files the allocator reads from ``$repo``. + + ``drafted`` is a BACKLOG heading written to the working tree and committed NOWHERE -- the floor term + that exists to catch exactly that, and the one where reading the wrong tree is worse than + misattribution: a number invisible to the sweep is a number free to re-issue. + + ``boundary`` is the ``PUBLIC_BACKLOG_FLOOR`` the allocator parses out of ledger_check.py rather than + restating. A stub is faithful here because the allocator reads it with a regex over the raw text, and + it gives a second, independent signal for which checkout was consulted. + """ + (path / "scripts" / "coord").mkdir(parents=True) + (path / "scripts" / "hooks").mkdir(parents=True) + (path / "docs").mkdir(parents=True) + shutil.copy2(_ALLOC, path / "scripts" / "coord" / "alloc.ps1") + shutil.copy2(_CLAIM, path / "scripts" / "coord" / "claim.ps1") + (path / "scripts" / "hooks" / "ledger_check.py").write_text( + f"PUBLIC_BACKLOG_FLOOR = {boundary}\n", encoding="utf-8" + ) + (path / "docs" / "BACKLOG.md").write_text( + f"# Backlog\n\n## {drafted}. A number drafted here and committed nowhere\n", + encoding="utf-8", + ) + _git("init", "-b", "main", ".", cwd=path) + _git("config", "user.email", "t@e.com", cwd=path) + _git("config", "user.name", "t", cwd=path) + _git("add", "-A", cwd=path) + _git("commit", "-m", "fixture", "--no-verify", cwd=path) + return path + + +def test_alloc_reads_the_floor_from_its_own_checkout_not_the_cwd(tmp_path: Path) -> None: + """BACKLOG #1060, asserted by ``-ShowFloor`` so no numbers are burned. + + Allocation is a one-way door -- claims are never released, "holes are free, collisions are not" -- + so a test that allocated would leave permanent holes in the shared registry for every worktree of + this clone. ``-ShowFloor`` exists for exactly this and computes the same floor without advancing the + ratchet. + + The caller's drafted number is deliberately HIGHER than the target's. The floor is a maximum, so a + wrong read shows up as a floor that is too high; equal numbers, or a lower one in the caller, would + both pass with the bug still in. + """ + named = _coord_checkout(tmp_path / "Named", drafted=4242, boundary=1200) + caller = _coord_checkout(tmp_path / "Caller", drafted=7777, boundary=1900) + + proc = subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(named / "scripts" / "coord" / "alloc.ps1"), + "-ShowFloor", + "-Kind", + "backlog", + ], + cwd=str(caller), # THE POINT: the shell is standing in the other checkout + capture_output=True, + text=True, + timeout=180, + ) + assert proc.returncode == 0, proc.stderr or proc.stdout + out = proc.stdout + + assert "floor : 4242" in out, f"read the CALLER's working tree, not its own:\n{out}" + assert "7777" not in out, f"the caller's drafted number reached the floor:\n{out}" + # Independent second signal: the boundary comes from /scripts/hooks/ledger_check.py, so it + # answers "which checkout" without depending on the floor arithmetic at all. + assert "boundary : 1200" in out, f"parsed the CALLER's ledger_check.py:\n{out}" + # And the registry it would write to must live beside the named checkout's object store. + watermark = next(line for line in out.splitlines() if line.startswith("watermark:")) + assert str(named).replace("\\", "/").casefold() in watermark.replace("\\", "/").casefold(), ( + watermark + ) + + +def test_claim_records_the_checkout_it_lives_in_not_the_cwd(tmp_path: Path) -> None: + """The same construct in claim.ps1, which was NOT filed -- found by inspection while fixing #1060. + + A numbered claim is enforced by scripts/hooks/claim_check.py at commit time. That hook reads the repo + from cwd and is right to: cwd IS the committing worktree for a commit hook. Only the tool can be + invoked from somewhere else, so only the tool needed anchoring. + """ + named = _coord_checkout(tmp_path / "Named", drafted=4242, boundary=1200) + caller = _coord_checkout(tmp_path / "Caller", drafted=7777, boundary=1900) + + proc = subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(named / "scripts" / "coord" / "claim.ps1"), + "-Take", + "anchoring-probe", + "-Note", + "divergence fixture", + ], + cwd=str(caller), + capture_output=True, + text=True, + timeout=180, + ) + assert proc.returncode == 0, proc.stderr or proc.stdout + + written = list((named / ".git" / "mefor-coord" / "claims").glob("*.json")) + assert written, f"no claim landed in the named checkout's registry:\n{proc.stdout}" + recorded = json.loads(written[0].read_text(encoding="utf-8"))["worktree"] + assert recorded.replace("\\", "/").casefold() == str(named).replace("\\", "/").casefold(), ( + f"claim recorded against the CALLER: {recorded}" + ) + assert not (caller / ".git" / "mefor-coord" / "claims").exists(), ( + "the caller's registry was written to" + ) + + def test_setup_leak_gate_does_not_reintroduce_an_unanchored_toplevel(tmp_path: Path) -> None: """A spelling guard for the regression, paired with the behavioural test above. From a042c0fe4556dc4b3cdf094dfc4af85af523cacc Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 6 Aug 2026 14:16:52 -0500 Subject: [PATCH 3/3] docs(backlog): record the test-isolation trap #1060's fix walked into (BACKLOG #1060) A cwd-dependence that reads as a defect in the tool can be load-bearing ISOLATION in its tests. Both claim test files were scoped to a throwaway registry purely by ambient cwd -- one said so in a docstring -- so anchoring the script turned the passing half of the run into a writer on this clone's shared registry before the rest of it went red. Recorded in the item rather than only in the commit message, because #1057 and #1059 are the remaining instances of the same class and will hit the same trap: check what a test is isolated BY before changing what the code reads. --- docs/BACKLOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 41556cf3..8315d36b 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -5543,6 +5543,8 @@ The literal spellings are all caught, including the newline form. Only the indir **SEVERITY CORRECTION, 2026-08-06, made while fixing it.** The paragraph above is right about the *recorded owner* and wrong about the item as a whole, because the filing looked at one line and the defect was in four. The ledger gate does fail closed on a misattributed claim โ€” but the floor's **working-tree term** was cwd-derived too, and that term has no gate behind it. Its job is to see a number written to `docs/BACKLOG.md` and committed **nowhere**; reading the caller's tree makes a number drafted in the *target* worktree invisible, so the allocator hands it out as free and two items end up sharing it in the same tree. Both are then owned by that worktree, so `owns()` passes and the ledger gate never fires. That is the exact silent collision the script's own docstring says it exists to prevent, arrived at through the script. The window is narrow โ€” anything committed on any ref is still caught by the all-refs term โ€” but it is a correctness hole, not friction, and it was reproduced in the negative control rather than reasoned about. +**AND THE FIX CONVERTED TWO SANDBOXED TESTS INTO WRITERS ON THE LIVE REGISTRY, which is the part worth remembering.** `tests/test_coord_claim_{refresh,liveness}.py` ran the **real** `scripts/coord/claim.ps1` with `cwd` set to a temp repo โ€” scoped to a throwaway registry *purely by ambient cwd*, and one of them said so in a docstring: *"it scopes itself to the cwd's repo"*. The moment the script stopped consulting cwd, the passing half of the run wrote real claims into this clone's shared registry (two strays, removed by hand). So a cwd-dependence that looks like a defect in the tool can be load-bearing **isolation** in its tests, and removing it is a change to both. `tests/test_ledger_check.py` already staged a copy of `alloc.ps1` inside its fixture, which is exactly why it was the one file that did not break โ€” the pattern existed and the two claim files were the outliers. Both now stage and commit the script into the fixture, so the sandbox is structural rather than ambient. Anyone fixing the remaining instances of this class (#1057, #1059) should check what their tests are isolated *by* before changing what the code reads. + **Reproduced 2026-08-05, twice, by accident.** A session ran `pwsh -NoProfile -File /scripts/coord/alloc.ps1 -Kind backlog` from worktree A while intending to commit from worktree B. `alloc/backlog/1058.json` recorded `"worktree": "<...>/trusting-wu-c2e6d5"`. The commit from `MessageFoundry-gate-deferrals` was then refused: *"BACKLOG item #1058 was not allocated to this worktree"* โ€” true, unhelpful, and pointing at the allocator rather than at the invocation. Re-running with the shell actually inside the target worktree produced `1059.json` with the right owner and the commit went through. **#1058 is an abandoned hole**, which is the sanctioned outcome (`alloc.ps1`'s own docstring: *holes are free, collisions are not*). **The fix is small and there are two defensible shapes.** Either derive the repo from `$PSScriptRoot` so the allocator is anchored to the checkout it lives in โ€” matching what `new.ps1` and `remove.ps1` already do โ€” or keep the cwd behaviour and **say so at the point of use**, printing the recorded worktree in the `ALLOCATED` output so the mismatch is visible immediately rather than at commit time. The second is weaker but nearly free, and the two compose. Prefer anchoring: an allocator invoked by absolute path is being told which checkout to act on, and it should not then consult a different one.