diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index ab7b9bcb..ec2e8218 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -5539,10 +5539,14 @@ 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. + +**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. @@ -5615,7 +5619,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. @@ -5629,7 +5633,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/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/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_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 new file mode 100644 index 00000000..5d51f4c6 --- /dev/null +++ b/tests/test_script_root_anchoring.py @@ -0,0 +1,233 @@ +"""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 json +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" +_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" +) + + +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 _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. + + 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}" + )