From 118f831dc3c5d2b0dd5c30bd0cf2b25ca0d210f7 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 25 Aug 2026 12:51:25 -0500 Subject: [PATCH 1/2] fix(coord): claim.ps1 read a directory as an occupant, so an abandoned worktree looked like a live lane (BACKLOG #1348) `Get-HolderLiveness` returned `present` when the holder's path EXISTED. It never asked whether a session was in it, so a worktree that outlived its session rendered identically to a lane actively building. That is a third state the tool could not represent, and it is the one that produces work which is done, correct, and held by nobody: the sanctioned verbs refuse, CLAUDE.md forbids releasing another worktree's claim, and nothing can regularise it. MEASURED ON THE LIVE BOARD, before and after, same command: 35 HOLDER GONE unchanged 8 LIVE SESSION in it was indistinguishable from the next row 23 DIRECTORY ONLY previously rendered as "holder last committed Nh ago" So 23 of the 31 non-gone holders were abandoned directories wearing the same label as the 8 real lanes. The dispatcher's independent split put that middle band at 16; mine says 23. DIFFERENT INSTRUMENTS, DIFFERENT NUMBERS, and I am not reconciling them by picking one -- theirs was taken at a different time and may key liveness differently. The shape is what both agree on. *** THE STATE REPORTS. IT DOES NOT PERMIT. *** `unoccupied` REFUSES exactly as `present` does, and that is not timidity. occupancy.ps1 states the rule this inherits: "there is no heartbeat on this host, so nothing here can prove a session is GONE. Occupancy may therefore only ever VETO an action; a DEAD/STALE/absent verdict must never by itself authorise one." The item asked that the two be DISTINGUISHABLE, not that the second become releasable. A cwd-keyed probe cannot see a session working in that path BY ABSOLUTE PATH from elsewhere, and the deny text now says so. POLARITY, the same rule the blanket-stage guard carries (#1341, and #1229 before it): recognition may only ever SUPPRESS. Downgrading to `unoccupied` needs a POSITIVE determination -- the probe loaded, reported itself Available, and returned zero vetoing sessions for that exact path. Every other outcome, including occupancy.ps1 failing to load at all, stays `present` and refuses. A missing answer costs a refusal, never a licence. COST, measured rather than assumed, because -List runs constantly: Get-WorktreeOccupancy is 304 ms once, Get-WorktreeOccupants 9 ms per lookup, and it is built LAZILY and cached for the invocation. Full -List over 66 claims: 1.8 s wall. TESTS -- extended tests/test_coord_claim_liveness.py rather than adding a module. IT ALREADY EXISTED; I nearly wrote a duplicate off a filename in a dispatch note before reading the tests directory. 9 -> 14 cases. the three states are distinguishable, and a vanished holder still outranks the new one the new state still REFUSES a -Take and does not recommend -Force on a -Release *** the fallback: with occupancy.ps1 absent, the probe cannot load and the holder must read as present and still refuse -- never as the new state. The base fixture copies only claim.ps1, so it produces that condition naturally, which is why the pre-existing tests were already exercising the fail-safe and passing. MUTATION-PROVED, with a hash gate on both ends: flipped the fallback to return `unoccupied`, confirmed by hash the mutant applied (609b0017 -> 10940148), scored 4 failures including the polarity test and three pre-existing ones. *** AND THE RESTORE WAS ITSELF A BAD MUTATION, CAUGHT ONLY BY THE HASH. *** Restoring searched for the one-line `unoccupied` return and hit the FIRST occurrence -- the real third-state branch -- not the mutated fallback, silently swapping the two. The file was left functionally wrong: the third state could never fire. The tests would not reliably have caught it, because the swap leaves both branches returning plausible values. A RESTORE IS A MUTATION AND NEEDS THE SAME VERIFICATION AS THE MUTANT. Repaired by line range, binary-safe so the file's 549 CRLF endings survived, and the hash is byte-identical to pre-mutation. VERIFIED, scope named: pytest 14 passed tests/test_coord_claim_liveness.py pytest 79 passed every other module that drives claim.ps1 (test_coord_claim_refresh, test_coord_claim_release_history, test_coord_claim_adjudicate, test_coord_claim_reconcile, test_claim_check, test_coord) ruff format --check and ruff check -- run separately, each with its own exit code, both clean the live board rendered three ways before and after the repair, same counts NOT a full-suite run COORDINATION: the Cleaner is running a claim survey off this tool's output and I told them the predicate before it landed, as they asked. Their census of "3 of 62 held by a live session" is the figure this change makes legible; the label on 23 rows moves with no change to any claim. Co-Authored-By: Claude Opus 5 --- scripts/coord/claim.ps1 | 92 +++++++++++++++++++++-- tests/test_coord_claim_liveness.py | 113 ++++++++++++++++++++++++++++- 2 files changed, 195 insertions(+), 10 deletions(-) diff --git a/scripts/coord/claim.ps1 b/scripts/coord/claim.ps1 index 4e6c28b41..97b038f51 100644 --- a/scripts/coord/claim.ps1 +++ b/scripts/coord/claim.ps1 @@ -68,6 +68,12 @@ $ErrorActionPreference = "Stop" # 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" } + +# Occupancy is what lets a holder that is a DIRECTORY be told apart from one that is a SESSION +# (BACKLOG #1348). Loaded best-effort and NEVER fatal: if this fails, Get-HolderLiveness keeps its +# older, stricter meaning and refuses, which is the safe direction. Dot-sourced at script scope +# because functions sourced inside a function do not survive it. +try { . "$PSScriptRoot/occupancy.ps1" } catch { } $repo = $repo.Trim() $common = (& git -C $repo rev-parse --path-format=absolute --git-common-dir).Trim() $claims = Join-Path $common "mefor-coord/claims" @@ -154,23 +160,69 @@ function Add-HistoryLine([string]$Line) { # Reports only what it can PROVE. A vanished directory is a fact and the one state safe to act on # unasked. Everything else is 'unknown' or a quiet-hours count -- never "probably fine": a session can # be alive and simply not committing, so silence is not evidence of death. +# A DIRECTORY IS NOT A PERSON, AND THIS FUNCTION USED TO CONFLATE THEM (BACKLOG #1348). +# +# `present` meant "the path exists". It never asked whether a SESSION was in it, so a worktree that +# outlived its session -- the directory still on disk, nobody in it -- rendered identically to a +# lane that is actively building. That is a THIRD state the tool could not represent, and it is the +# one that produces work that is done, correct, and held by nobody: the sanctioned verbs refuse, +# `-Force` is forbidden by CLAUDE.md, and the claim cannot be regularised by anything. +# +# ***THE NEW STATE REPORTS. IT DOES NOT PERMIT.*** `unoccupied` still REFUSES, exactly as `present` +# does, and this is not timidity -- `occupancy.ps1` states the rule it inherits: "there is no +# heartbeat on this host, so nothing here can prove a session is GONE. Occupancy may therefore only +# ever VETO an action; a DEAD/STALE/absent verdict must never by itself authorise one." What the +# item asked for is that the two be DISTINGUISHABLE, not that the second become releasable. +# +# POLARITY, and it is the same rule the blanket-stage guard carries (#1341, #1229): recognition may +# only ever SUPPRESS. Downgrading to `unoccupied` requires a POSITIVE determination -- the occupancy +# probe loaded, reported itself Available, and returned zero vetoing sessions for that exact path. +# Every other outcome, including the probe failing to load at all, stays `present`. A missing +# answer must cost a refusal, never a licence. +$script:OccupancyProbe = $null # $null = not yet tried; $false = unavailable; else the object + +function Get-OccupancyOnce { + if ($null -ne $script:OccupancyProbe) { return $script:OccupancyProbe } + try { + if (-not (Get-Command Get-WorktreeOccupancy -EA SilentlyContinue)) { + $script:OccupancyProbe = $false + return $false + } + $occ = Get-WorktreeOccupancy -Repo $repo + # `Available` false means the fence could not be built -- an unplaceable record, no repo. + # Treat that exactly like a failed probe. + $script:OccupancyProbe = if ($occ -and $occ.Available) { $occ } else { $false } + } + catch { $script:OccupancyProbe = $false } + return $script:OccupancyProbe +} + function Get-HolderLiveness([string]$HeldPath) { try { if (-not (Test-Path -LiteralPath $HeldPath)) { - return [pscustomobject]@{ State = 'gone'; QuietHours = $null } + return [pscustomobject]@{ State = 'gone'; QuietHours = $null; Occupants = $null } } $ct = & git -C $HeldPath log -1 --format=%ct 2>$null if ($ct) { $quiet = [int]((Get-Date) - [System.DateTimeOffset]::FromUnixTimeSeconds([long]$ct).LocalDateTime).TotalHours - return [pscustomobject]@{ State = 'present'; QuietHours = $quiet } + $occ = Get-OccupancyOnce + if ($occ) { + $who = @(Get-WorktreeOccupants -Occupancy $occ -Path $HeldPath) + if ($who.Count -eq 0) { + return [pscustomobject]@{ State = 'unoccupied'; QuietHours = $quiet; Occupants = 0 } + } + return [pscustomobject]@{ State = 'present'; QuietHours = $quiet; Occupants = $who.Count } + } + # Probe unavailable: fall back to the old meaning, which refuses. Never to 'unoccupied'. + return [pscustomobject]@{ State = 'present'; QuietHours = $quiet; Occupants = $null } } # Present on disk but no commit to date it by -- a brand-new worktree looks exactly like this. - return [pscustomobject]@{ State = 'unknown'; QuietHours = $null } + return [pscustomobject]@{ State = 'unknown'; QuietHours = $null; Occupants = $null } } catch { # Say so rather than returning 'gone'. A failed probe that reported death would turn an # unreadable path into a licence to release someone's live claim. - return [pscustomobject]@{ State = 'failed'; QuietHours = $null } + return [pscustomobject]@{ State = 'failed'; QuietHours = $null; Occupants = $null } } } @@ -200,8 +252,16 @@ function Show-List { switch ($live.State) { 'gone' { $age = " [HOLDER GONE -- worktree no longer exists; release with -Force]" } 'present' { - $age = " [held ${hrs}h; holder last committed $($live.QuietHours)h ago]" - if ($live.QuietHours -ge 12) { $age += " -- QUIET, confirm with the holder before releasing" } + $age = " [held ${hrs}h; LIVE SESSION in the holder, last committed $($live.QuietHours)h ago]" + if ($live.QuietHours -ge 12) { $age += " -- QUIET but OCCUPIED, ask before releasing" } + } + # THE THIRD STATE, and the listing is the surface that matters (BACKLOG #1348). + # -List is what a Cleaner or Dispatcher reads to decide where to spend attention, so + # a directory that outlived its session must not render identically to a lane that + # is building. It still says ROUTE, not release: the refusal is unchanged. + 'unoccupied' { + $age = " [held ${hrs}h; DIRECTORY ONLY -- no live session in it, last commit $($live.QuietHours)h ago]" + $age += " -- ROUTE to the Cleaner/Dispatcher; not releasable on this signal alone" } default { $age = " [held ${hrs}h; holder liveness UNKNOWN -- confirm before releasing]" } } @@ -245,11 +305,19 @@ if ($Release) { Write-Host " Safe to take over: claim.ps1 -Release $Release -Force" } 'present' { - Write-Host " HOLDER IS STILL THERE -- that worktree exists and last committed $($live.QuietHours)h ago." -ForegroundColor Red + Write-Host " HOLDER IS STILL THERE -- that worktree exists, a live session is IN it, and it last committed $($live.QuietHours)h ago." -ForegroundColor Red Write-Host " Do NOT -Force it on the strength of a quiet period: a session can be alive and" Write-Host " simply not committing. Ask that session first -- releasing a live claim is how two" Write-Host " sessions end up building the same thing." } + 'unoccupied' { + Write-Host " HOLDER IS A DIRECTORY, NOT A SESSION -- that worktree exists and last committed $($live.QuietHours)h ago," -ForegroundColor Yellow + Write-Host " but NO live session is placed in it. This is the third state (BACKLOG #1348)." + Write-Host " STILL NOT YOURS TO -Force. Nothing on this host can prove a session is gone: occupancy" + Write-Host " sees a session by the cwd it launched in, so one working here BY ABSOLUTE PATH from" + Write-Host " elsewhere is invisible to it. This is reported so you can ROUTE it, not act on it." + Write-Host " Route to the Cleaner or the Dispatcher -- releasing another worktree's claim is theirs." + } default { Write-Host " HOLDER LIVENESS UNKNOWN -- the worktree exists but could not be dated." -ForegroundColor Yellow Write-Host " Confirm with that session before using -Force." @@ -424,11 +492,19 @@ try { Write-Host " pwsh -NoProfile -File scripts\coord\claim.ps1 -Take $Take -Note """"" } 'present' { - Write-Host " HOLDER IS STILL THERE -- that worktree exists and last committed $($live.QuietHours)h ago." -ForegroundColor Red + Write-Host " HOLDER IS STILL THERE -- that worktree exists, a live session is IN it, and it last committed $($live.QuietHours)h ago." -ForegroundColor Red Write-Host " Do NOT build it in parallel -- that is the duplicate-work this gate exists to stop," Write-Host " and do NOT -Force it: quiet is not dead. Coordinate with that session or pick" Write-Host " different work. Its note above says what it is doing." } + 'unoccupied' { + Write-Host " HOLDER IS A DIRECTORY, NOT A SESSION -- that worktree exists and last committed $($live.QuietHours)h ago," -ForegroundColor Yellow + Write-Host " but NO live session is placed in it. This is the third state (BACKLOG #1348)." + Write-Host " STILL NOT YOURS TO -Force, and the refusal is deliberate: occupancy can VETO but never" + Write-Host " authorise, because nothing here can prove a session is gone. A session working in this" + Write-Host " path BY ABSOLUTE PATH from another cwd does not appear as an occupant." + Write-Host " Hand it to the Cleaner or the Dispatcher with this line; do not build it in parallel." + } default { Write-Host " HOLDER LIVENESS UNKNOWN -- the worktree exists but could not be dated." -ForegroundColor Yellow Write-Host " Treat it as live: coordinate with that session before -Force." diff --git a/tests/test_coord_claim_liveness.py b/tests/test_coord_claim_liveness.py index 9bb784c39..4621faea4 100644 --- a/tests/test_coord_claim_liveness.py +++ b/tests/test_coord_claim_liveness.py @@ -219,8 +219,117 @@ def test_list_still_flags_a_vanished_holder(repo: Path, tmp_path: Path) -> None: def test_list_still_reports_a_living_holders_quiet_hours(repo: Path, tmp_path: Path) -> None: - """Anti-regression for the shared helper: 'present' must still carry its commit age.""" + """Anti-regression for the shared helper: 'present' must still carry its commit age. + + The wording moved with BACKLOG #1348 -- "holder last committed Nh ago" became "LIVE SESSION in + the holder, last committed Nh ago" -- because the old phrase was true of a directory nobody was + in. The commit age is still there; only the claim about WHO is there is new. + """ peer_holding(repo, tmp_path, "k") out = claim(repo, "-List").stdout - assert "holder last committed" in out + assert "last committed" in out assert "HOLDER GONE" not in out + + +# -------------------------------------------------------------------------------------------------- +# THE THIRD STATE: a holder that is a DIRECTORY rather than a SESSION (BACKLOG #1348) +# -------------------------------------------------------------------------------------------------- + +# `present` used to mean "the path exists". It never asked whether a session was IN it, so a +# worktree that outlived its session rendered identically to a lane actively building. Measured on +# the live board when this landed: 35 holders gone, 8 with a live session, and 23 that were a +# directory with nobody in it -- and all 31 of the last two groups had been rendering the same way. +# +# THE STATE REPORTS. IT DOES NOT PERMIT. Every test below pins BOTH halves, because a fix that +# turned the new state into a licence to release would be worse than the defect: occupancy.ps1's own +# rule is that it "may only ever VETO an action; a DEAD/STALE/absent verdict must never by itself +# authorise one", since a session working in a path by ABSOLUTE PATH from another cwd is invisible +# to a cwd-keyed probe. + + +@pytest.fixture +def repo_with_occupancy(repo: Path) -> Path: + """The sandbox, plus the occupancy probe claim.ps1 consults. + + The base `repo` fixture deliberately copies ONLY claim.ps1, which is what makes the probe + unavailable there -- and that is a real configuration, not an artifact: a checkout carrying a + partial scripts/coord/ behaves exactly that way. Both are worth testing, so they get separate + fixtures rather than one that hides the difference. + """ + src = CLAIM.parent + for name in ("occupancy.ps1", "session-registry.ps1"): + shutil.copy2(src / name, repo / "scripts" / "coord" / name) + return repo + + +def test_a_holder_with_no_live_session_is_distinguishable_from_one_with_a_session( + repo_with_occupancy: Path, tmp_path: Path +) -> None: + """The whole point of #1348: the two must not render identically. + + Nothing is running in the sandbox's peer worktree, so the probe places zero sessions in it. + """ + peer_holding(repo_with_occupancy, tmp_path, "k") + out = claim(repo_with_occupancy, "-List").stdout + assert "DIRECTORY ONLY" in out, out + assert "no live session in it" in out + # and it must not claim someone is there + assert "LIVE SESSION in the holder" not in out + + +def test_the_third_state_still_refuses_a_take(repo_with_occupancy: Path, tmp_path: Path) -> None: + """Distinguishable is not releasable. The refusal is unchanged.""" + peer_holding(repo_with_occupancy, tmp_path, "k") + proc = claim(repo_with_occupancy, "-Take", "k", "-Note", "mine now") + assert proc.returncode != 0, "an unoccupied holder must still block a take" + assert "A DIRECTORY, NOT A SESSION" in proc.stdout + assert "still not yours to -force" in proc.stdout.lower() + + +def test_the_third_state_does_not_recommend_force_on_a_release( + repo_with_occupancy: Path, tmp_path: Path +) -> None: + """`-Force` is recommended for exactly one state, and this is not it. + + The negative control is the vanished-holder case above, which DOES recommend it -- so this + assertion is about the state, not about the word being absent everywhere. + """ + peer_holding(repo_with_occupancy, tmp_path, "k") + proc = claim(repo_with_occupancy, "-Release", "k") + assert proc.returncode != 0 + assert "A DIRECTORY, NOT A SESSION" in proc.stdout + assert "Safe to take over" not in proc.stdout + + +def test_a_vanished_holder_still_outranks_the_third_state( + repo_with_occupancy: Path, tmp_path: Path +) -> None: + """Positive control for the probe being live at all. + + With occupancy available, a DELETED worktree must still read GONE and still recommend -Force. + If this ever reported the third state instead, the new branch would be swallowing the one + verdict that is safe to act on unasked. + """ + peer = peer_holding(repo_with_occupancy, tmp_path, "k") + orphan(repo_with_occupancy, peer) + out = claim(repo_with_occupancy, "-List").stdout + assert "HOLDER GONE" in out + assert "DIRECTORY ONLY" not in out + + +def test_without_the_probe_it_falls_back_to_REFUSING_not_to_the_new_state( + repo: Path, tmp_path: Path +) -> None: + """THE POLARITY RULE, and it is the one that must never regress. + + The base fixture has no occupancy.ps1, so the probe cannot load. A missing answer must cost a + REFUSAL, never a licence: recognition may only ever suppress. If an unloadable probe ever + produced "DIRECTORY ONLY", every checkout with a partial scripts/coord/ would start reporting + live lanes as abandoned -- the same false record, arrived at from the other side. + """ + peer_holding(repo, tmp_path, "k") + out = claim(repo, "-List").stdout + assert "DIRECTORY ONLY" not in out, "an unavailable probe must not produce the new state" + assert "last committed" in out + proc = claim(repo, "-Take", "k", "-Note", "mine now") + assert proc.returncode != 0, "the take must still be refused when the probe cannot load" From d37de583c19026cacc4ffba3d4e627561850c341 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 25 Aug 2026 14:17:50 -0500 Subject: [PATCH 2/2] fix(coord): the unknown-occupancy fallback asserted a live session it never observed (BACKLOG #1348) CI FIX for PR 585. The required windows-2025 leg failed three of the third-state tests, deterministically rather than as a flake, and it is a real bug in the feature -- not an environment quirk. *** THE BUG: ONE STATE MEANT TWO THINGS. *** `Get-HolderLiveness` returned `present` BOTH when the probe looked and found an occupant AND when the probe could not look at all -- and then `present` was labelled "LIVE SESSION in the holder". So wherever occupancy is unavailable the tool ASSERTED A LIVE SESSION IT HAD NEVER OBSERVED. That is the exact conflation this item exists to remove -- telling a DIRECTORY apart from a PERSON -- reintroduced one level down, in the deny text, by me. REPRODUCED LOCALLY BEFORE FIXING, rather than reasoned about from the CI log: no Claude config root at all Available=False "no Claude config root ... was found" a config root, empty sessions dir Available=False "not one readable session record in them" a config root + one session record Available=True Sessions=0 A CI runner has no session registry, so the probe reports unavailable and the fallback fires. THE FIX IS A STATE SPLIT, and `present` goes back to meaning exactly what it meant before #1348 -- the path is there and that is all this function knows: gone path absent occupied path + probe AVAILABLE + a vetoing occupant for this exact path unoccupied path + probe AVAILABLE + zero occupants <- the third state, unchanged present path + probe UNAVAILABLE, occupancy UNKNOWN <- no longer claims anything unknown / failed unchanged `occupied` is now the ONLY state that claims a session, and it is reachable only through a probe that returned Available. A REGRESSION MY OWN EDIT INTRODUCED AND THE EXISTING TESTS CAUGHT. My first split gave `present` a shorter message that dropped the do-not-Force guidance the occupied branch carries. `test_take_blocked_by_a_LIVING_holder_does_not_offer_force` and its release sibling failed on exactly that. The two branches now differ ONLY in the liveness claim; the advice is identical, and a test asserts that. *** THE TEST FIXTURE WAS THE OTHER HALF OF THE DEFECT, AND IT IS THE WORSE SHAPE. *** The third-state tests asserted DIRECTORY-ONLY, which is reachable only through an AVAILABLE probe, while depending on the developer machine happening to have a Claude session registry. They passed locally and failed on windows-2025: green exactly where nobody is watching. `repo_with_occupancy` now PLANTS its own config root with one session record carrying a DEAD pid -- which makes the probe available while placing no occupant anywhere, since occupancy is decided by whether the pid is running. The measured table above is in the fixture docstring so the next reader does not have to re-derive why a config root alone is not enough. NEW TEST: `test_an_unavailable_probe_never_claims_a_live_session`, on the base fixture where the probe genuinely cannot load. Asserts the output does NOT say "LIVE SESSION", DOES say "OCCUPANCY UNKNOWN", and still refuses -- naming the unknown is not a licence. MUTATION: reintroduce the bug (fallback returns `occupied`) -> that test alone fails. Restored from a byte copy, hash-verified identical. VERIFIED, scope named: pytest 32 passed test_coord_claim_liveness (14 -> 15) + test_coord_claim_refresh + test_coord_claim_release_history pytest 28 passed test_coord_claim_adjudicate + test_coord ruff format --check, ruff check -- run separately, both clean the live board still separates three ways on this box NOT a full-suite run, and NOT verified on windows-2025 -- that leg is what found this and only CI can run it. The Lander should re-check there rather than take this commit as proof. WHAT I CANNOT CLAIM: this is not corroborated by a second Windows leg. The module is skipif(os.name != "nt") and the job has no windows-2022 leg, so windows-2025 remains the only data point either way. Co-Authored-By: Claude Opus 5 --- scripts/coord/claim.ps1 | 40 +++++++++++++-- tests/test_coord_claim_liveness.py | 80 +++++++++++++++++++++++++++++- 2 files changed, 115 insertions(+), 5 deletions(-) diff --git a/scripts/coord/claim.ps1 b/scripts/coord/claim.ps1 index 97b038f51..df1294292 100644 --- a/scripts/coord/claim.ps1 +++ b/scripts/coord/claim.ps1 @@ -211,9 +211,22 @@ function Get-HolderLiveness([string]$HeldPath) { if ($who.Count -eq 0) { return [pscustomobject]@{ State = 'unoccupied'; QuietHours = $quiet; Occupants = 0 } } - return [pscustomobject]@{ State = 'present'; QuietHours = $quiet; Occupants = $who.Count } + return [pscustomobject]@{ State = 'occupied'; QuietHours = $quiet; Occupants = $who.Count } } - # Probe unavailable: fall back to the old meaning, which refuses. Never to 'unoccupied'. + # PROBE UNAVAILABLE -> `present`, which keeps its PRE-#1348 meaning exactly: the path is + # there and that is all this function knows. It still refuses. + # + # THIS SPLIT IS A BUG FIX, NOT A TIDY-UP. The first version of #1348 returned `present` + # for BOTH "the probe looked and found an occupant" and "the probe could not look", and + # then labelled `present` "LIVE SESSION in the holder". On a machine with no Claude + # config root -- every CI runner -- the probe reports Available=false, so the fallback + # fired and the tool ASSERTED A LIVE SESSION IT HAD NEVER OBSERVED. Caught by the + # required windows-2025 leg on PR 585, three tests, deterministic rather than flaky. + # + # One state cannot mean both "I measured this" and "I could not measure this", and the + # deny text is where that conflation becomes a false statement to an operator. `occupied` + # is now the only state that claims a session, and it is reachable only through a probe + # that returned Available with a vetoing occupant for this exact path. return [pscustomobject]@{ State = 'present'; QuietHours = $quiet; Occupants = $null } } # Present on disk but no commit to date it by -- a brand-new worktree looks exactly like this. @@ -251,10 +264,15 @@ function Show-List { $live = Get-HolderLiveness $held switch ($live.State) { 'gone' { $age = " [HOLDER GONE -- worktree no longer exists; release with -Force]" } - 'present' { + 'occupied' { $age = " [held ${hrs}h; LIVE SESSION in the holder, last committed $($live.QuietHours)h ago]" if ($live.QuietHours -ge 12) { $age += " -- QUIET but OCCUPIED, ask before releasing" } } + # The probe could not run -- no Claude config root, or it failed. Says what it knows + # and no more: the path is there. It must NOT claim a session it never looked for. + 'present' { + $age = " [held ${hrs}h; holder present, last committed $($live.QuietHours)h ago; OCCUPANCY UNKNOWN -- the session probe could not run]" + } # THE THIRD STATE, and the listing is the surface that matters (BACKLOG #1348). # -List is what a Cleaner or Dispatcher reads to decide where to spend attention, so # a directory that outlived its session must not render identically to a lane that @@ -305,6 +323,14 @@ if ($Release) { Write-Host " Safe to take over: claim.ps1 -Release $Release -Force" } 'present' { + Write-Host " HOLDER IS STILL THERE -- that worktree exists and last committed $($live.QuietHours)h ago." -ForegroundColor Red + Write-Host " OCCUPANCY UNKNOWN: the session probe could not run, so this does NOT say whether" + Write-Host " anyone is in it. Treat that as MORE reason to coordinate, not less." + Write-Host " Do NOT -Force it on the strength of a quiet period: a session can be alive and" + Write-Host " simply not committing. Ask that session first -- releasing a live claim is how two" + Write-Host " sessions end up building the same thing." + } + 'occupied' { Write-Host " HOLDER IS STILL THERE -- that worktree exists, a live session is IN it, and it last committed $($live.QuietHours)h ago." -ForegroundColor Red Write-Host " Do NOT -Force it on the strength of a quiet period: a session can be alive and" Write-Host " simply not committing. Ask that session first -- releasing a live claim is how two" @@ -492,6 +518,14 @@ try { Write-Host " pwsh -NoProfile -File scripts\coord\claim.ps1 -Take $Take -Note """"" } 'present' { + Write-Host " HOLDER IS STILL THERE -- that worktree exists and last committed $($live.QuietHours)h ago." -ForegroundColor Red + Write-Host " OCCUPANCY UNKNOWN: the session probe could not run, so this does NOT say whether" + Write-Host " anyone is in it. Treat that as MORE reason to coordinate, not less." + Write-Host " Do NOT build it in parallel -- that is the duplicate-work this gate exists to stop," + Write-Host " and do NOT -Force it: quiet is not dead. Coordinate with that session or pick" + Write-Host " different work. Its note above says what it is doing." + } + 'occupied' { Write-Host " HOLDER IS STILL THERE -- that worktree exists, a live session is IN it, and it last committed $($live.QuietHours)h ago." -ForegroundColor Red Write-Host " Do NOT build it in parallel -- that is the duplicate-work this gate exists to stop," Write-Host " and do NOT -Force it: quiet is not dead. Coordinate with that session or pick" diff --git a/tests/test_coord_claim_liveness.py b/tests/test_coord_claim_liveness.py index 4621faea4..8eeffabb7 100644 --- a/tests/test_coord_claim_liveness.py +++ b/tests/test_coord_claim_liveness.py @@ -23,6 +23,7 @@ from __future__ import annotations +import json import os import shutil import subprocess @@ -248,17 +249,58 @@ def test_list_still_reports_a_living_holders_quiet_hours(repo: Path, tmp_path: P @pytest.fixture -def repo_with_occupancy(repo: Path) -> Path: - """The sandbox, plus the occupancy probe claim.ps1 consults. +def repo_with_occupancy(repo: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + """The sandbox, plus the occupancy probe claim.ps1 consults, plus a config root it can read. The base `repo` fixture deliberately copies ONLY claim.ps1, which is what makes the probe unavailable there -- and that is a real configuration, not an artifact: a checkout carrying a partial scripts/coord/ behaves exactly that way. Both are worth testing, so they get separate fixtures rather than one that hides the difference. + + *** THE PLANTED CONFIG ROOT IS WHY THIS PASSES ON CI, and its absence is why the first version + of these tests did not. *** `Get-WorktreeOccupancy` reports Available=FALSE unless it finds at + least one READABLE SESSION RECORD -- not merely a config root, an actual record. Measured: + + no config root at all Available=False "no Claude config root ... was found" + a config root, empty sessions Available=False "not one readable session record in them" + a config root + one record Available=True Sessions=0 + + A CI runner has no Claude session registry, so the probe was unavailable there and the code fell + back to its unknown-occupancy state -- while these tests asserted the DIRECTORY-ONLY state, which + is reachable only through an available probe. They passed on a developer box and failed on + windows-2025, which is the worst shape a test can have: green exactly where nobody is watching. + + The record carries a DEAD pid, so it makes the probe available while placing NO occupant + anywhere. Occupancy is decided by whether the pid is running (session-registry.ps1), and 999999 + is not a live process on any runner. The cwd points outside the sandbox for the same reason. """ src = CLAIM.parent for name in ("occupancy.ps1", "session-registry.ps1"): shutil.copy2(src / name, repo / "scripts" / "coord" / name) + + home = tmp_path / "fakehome" + sessions = home / ".claude" / "sessions" + sessions.mkdir(parents=True) + (sessions / "999999.json").write_text( + json.dumps( + { + "cwd": str(tmp_path / "somewhere-else"), + "pid": 999999, + "sessionId": "planted-dead-session", + "startedAt": "2026-01-01T00:00:00Z", + "procStart": "2026-01-01T00:00:00Z", + "kind": "desktop", + "entrypoint": "test", + "name": "planted", + "nameSource": "test", + "peerProtocol": "none", + } + ), + encoding="utf-8", + ) + # Both, because the roots are resolved from the user profile and the two differ across shells. + monkeypatch.setenv("USERPROFILE", str(home)) + monkeypatch.setenv("HOME", str(home)) return repo @@ -333,3 +375,37 @@ def test_without_the_probe_it_falls_back_to_REFUSING_not_to_the_new_state( assert "last committed" in out proc = claim(repo, "-Take", "k", "-Note", "mine now") assert proc.returncode != 0, "the take must still be refused when the probe cannot load" + + +def test_an_unavailable_probe_never_claims_a_live_session(repo: Path, tmp_path: Path) -> None: + """THE REGRESSION TEST FOR THE BUG THAT REACHED CI. One state cannot mean two things. + + The first version of BACKLOG #1348 returned `present` BOTH when the probe looked and found an + occupant AND when the probe could not look at all -- then labelled `present` "LIVE SESSION in + the holder". On any machine with no Claude session registry, which is every CI runner, the + fallback fired and the tool ASSERTED A LIVE SESSION IT HAD NEVER OBSERVED. + + That is not a cosmetic wording defect. The whole point of the item is telling a DIRECTORY apart + from a PERSON, and a state that reports "person" when it means "I could not look" is the same + conflation the item exists to remove, reintroduced in the deny text. + + The base `repo` fixture copies only claim.ps1, so occupancy.ps1 cannot be dot-sourced and the + probe is genuinely unavailable -- the same condition as a runner, reached a different way. + """ + peer_holding(repo, tmp_path, "k") + out = claim(repo, "-List").stdout + assert "LIVE SESSION in the holder" not in out, ( + "the tool claimed a live session while the occupancy probe could not run. It cannot know " + f"that, and saying it is what shipped to CI.\n{out}" + ) + assert "OCCUPANCY UNKNOWN" in out, ( + "an unavailable probe must SAY it could not look, not fall silent into a state that reads " + f"as a measurement.\n{out}" + ) + # And it must still refuse -- naming the unknown is not a licence. + proc = claim(repo, "-Take", "k", "-Note", "mine now") + assert proc.returncode != 0 + assert "quiet is not dead" in proc.stdout, ( + "the unknown-occupancy branch dropped the safety guidance the occupied branch carries. " + "Only the liveness CLAIM should differ between them; the advice must not." + )