diff --git a/scripts/coord/claim.ps1 b/scripts/coord/claim.ps1 index df1294292..5fca3b631 100644 --- a/scripts/coord/claim.ps1 +++ b/scripts/coord/claim.ps1 @@ -75,6 +75,25 @@ if (-not $repo) { throw "scripts/coord/ is not inside a git repository: $PSScrip # because functions sourced inside a function do not survive it. try { . "$PSScriptRoot/occupancy.ps1" } catch { } $repo = $repo.Trim() + +# ONE divergence test, three call sites (BACKLOG #1358). The note used to be written inline at the very +# end of the script, which put it after the `-Take` success block and therefore made it UNREACHABLE from +# `-Release` -- the script stated the release rule at claim time and went silent at the moment the +# operator applied it. `$Subject` is the only part that varies, because the true sentence differs: a take +# is recorded to $repo, whereas a release is BOTH recorded to it and adjudicated against it. +# +# Deliberately reads $repo at CALL time from script scope rather than taking it as a parameter: a second +# copy of "which tree is this" is exactly the drift this note exists to report. +function Write-DivergenceNote([Parameter(Mandatory)][string]$Subject) { + $cwdTop = (& git rev-parse --path-format=absolute --show-toplevel 2>$null) + if (-not $cwdTop) { return } + $a = ($cwdTop.Trim() -replace '\\', '/').TrimEnd('/') + $b = ($repo -replace '\\', '/').TrimEnd('/') + if ($a -ieq $b) { return } + Write-Host " NOTE: your shell is in $a, but this script lives in $b," -ForegroundColor Yellow + Write-Host " so the $Subject" -ForegroundColor Yellow +} + $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 @@ -349,6 +368,12 @@ if ($Release) { Write-Host " Confirm with that session before using -Force." } } + # BACKLOG #1358, and this is the placement that earns the most. The refusal above says the claim + # is "held by another worktree" -- but under divergence the ownership test ran against the SCRIPT's + # tree, so "another worktree" can be the operator's OWN, with the foreign thing being the copy of + # this script they invoked. Without the note that reads as a genuine cross-session collision and + # invites a -Force, which is the one action the whole block exists to talk them out of. + Write-DivergenceNote "ownership was judged against it, NOT against your shell's tree -- re-run this from $repo before concluding anyone else holds it." exit 1 } # RECORD FIRST, then act. Both orders can lie once and only one lie is recoverable: removing first @@ -403,6 +428,9 @@ if ($Release) { Write-Host " TOOK OVER a claim held by $($info.Claim.worktree) [$($info.Claim.branch)]." -ForegroundColor Yellow } Write-Host " recorded in $history" + # BACKLOG #1358. The release SUCCEEDS either way, so without this the history records a tree the + # operator was never standing in and nothing anywhere says so. + Write-DivergenceNote "release was recorded there, and ownership was judged against it rather than against your shell's tree." exit 0 } @@ -571,13 +599,5 @@ Write-Host " release when done: pwsh -NoProfile -File scripts\coord\claim.ps1 # 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 - } -} +Write-DivergenceNote 'claim is recorded there. -Release must be run against that same worktree.' exit 0 diff --git a/tests/test_script_root_anchoring.py b/tests/test_script_root_anchoring.py index 2714680aa..3875c6c38 100644 --- a/tests/test_script_root_anchoring.py +++ b/tests/test_script_root_anchoring.py @@ -217,6 +217,83 @@ def test_claim_records_the_checkout_it_lives_in_not_the_cwd(tmp_path: Path) -> N ) +def _run_claim(script_tree: Path, cwd: Path, *args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(script_tree / "scripts" / "coord" / "claim.ps1"), + *args, + ], + cwd=str(cwd), + capture_output=True, + text=True, + timeout=180, + ) + + +#: The divergence note's own load-bearing sentence, asserted as a SUBSTRING of the rendered line. +#: +#: Not the whole line: it interpolates two absolute paths and PowerShell hard-wraps host output at the +#: console width, so an equality assertion would fail on formatting rather than on behaviour. This +#: fragment carries the instruction a reader has to act on and nothing that varies. +_DIVERGENCE = "but this script lives in" + + +def test_the_divergence_note_FIRES_on_take_from_a_foreign_cwd(tmp_path: Path) -> None: + """POSITIVE CONTROL for the test below, and it must be read as one. + + Asserting only that ``-Release`` prints nothing cannot distinguish "this path is silent" from "the + note never fires anywhere" -- an absent feature and a broken assertion print the same empty string. + So this pins that the mechanism WORKS on the path that has it, in the same fixture, with the same + divergence, before its sibling asserts the path that does not. + """ + named = _coord_checkout(tmp_path / "Named", drafted=4242, boundary=1200) + caller = _coord_checkout(tmp_path / "Caller", drafted=7777, boundary=1900) + + proc = _run_claim(named, caller, "-Take", "note-probe", "-Note", "divergence fixture") + + assert proc.returncode == 0, proc.stderr or proc.stdout + assert _DIVERGENCE in proc.stdout, ( + "the -Take divergence note did not fire, so this control cannot support its sibling:\n" + f"{proc.stdout}" + ) + + +def test_the_divergence_note_ALSO_fires_on_release_from_a_foreign_cwd(tmp_path: Path) -> None: + """BACKLOG #1358 -- the note names the release rule and was printed only at CLAIM time. + + ``-Release`` resolves ownership and records ``released_by`` from ``$repo``, which is anchored on + ``$PSScriptRoot`` and NOT on the caller's cwd. That is correct and deliberate; the defect is that the + warning about it was unreachable from this path. The note lives at the end of the script, after the + ``-Take`` success block, and ``-Release`` exits some 170 lines earlier -- so the one message that + states *"-Release must be run against that same worktree"* was shown to the operator at claim time + and withheld at the moment they actually did it. + + Its absence is silent in the worst way: the release SUCCEEDS, and the history records a tree the + operator was never standing in. + """ + named = _coord_checkout(tmp_path / "Named", drafted=4242, boundary=1200) + caller = _coord_checkout(tmp_path / "Caller", drafted=7777, boundary=1900) + + # Claim from INSIDE the named tree, so the take is the ordinary same-tree case and this test turns + # only on the release. A divergent take would print the note here and confuse which call emitted it. + taken = _run_claim(named, named, "-Take", "release-note-probe", "-Note", "fixture") + assert taken.returncode == 0, taken.stderr or taken.stdout + assert _DIVERGENCE not in taken.stdout, "the same-tree take must stay silent" + + released = _run_claim(named, caller, "-Release", "release-note-probe") + + assert released.returncode == 0, released.stderr or released.stdout + assert "Released claim" in released.stdout, released.stdout + assert _DIVERGENCE in released.stdout, ( + "-Release recorded the claim against the SCRIPT's tree and said nothing about it:\n" + f"{released.stdout}" + ) + + 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.