From 4a8e111a2407aa62988fa4b166b861bab0c61732 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sat, 1 Aug 2026 23:32:20 -0500 Subject: [PATCH 01/10] fix(coord): overlap gave two different answers the same bytes, twice Two defects in one script, and they are the same defect: a signal that cannot distinguish the state it reports from a different state. 1. A -Json query answered "nobody else is in this file" by printing NOTHING. `@() | ConvertTo-Json -AsArray` sends zero objects down the pipeline, so ConvertTo-Json never runs -- -AsArray only shapes output that already exists. On stdout an all-clear was therefore byte-for-byte identical to the script dying before it answered, and no consumer could tell them apart. Every -Json exit now goes through one emitter that always produces an array. (-InputObject is not the fix: with -AsArray it double-wraps to [[]].) Found by running the real script against the real collision gate rather than the test stubs, which had been written to a shape the real script never produced. 2. A live session was attributed to a worktree by FIRST prefix hit. Linked worktrees live under the primary checkout, so every linked path is also a prefix match for the primary's row: the primary was handed whichever nested session the hash table enumerated first, and reported LIVE on main, "building" a peer's task list. Hash order is not stable, so it was a different wrong answer each run -- which is why it read as noise rather than as a bug. Longest prefix wins is the only rule that survives nesting, and it is resolved once against every worktree instead of per row. docs/WORKTREES.md already named this exact trap for the announce hook's id rule, where the cure was "never match by prefix". Here a prefix match is genuinely required -- a session may sit in any subdirectory -- so the cure has to be longest-prefix instead. Both are pinned against a real nested-worktree git fixture; a sibling layout would pass under the old rule and prove nothing. Each new assertion was checked against the unfixed script first: the attribution test reports the primary as Live/main/, and the array test sees ''. --- scripts/coord/overlap.ps1 | 54 ++++- tests/test_coord_overlap_attribution.py | 252 ++++++++++++++++++++++++ tests/test_coord_overlap_signals.py | 51 +++++ 3 files changed, 349 insertions(+), 8 deletions(-) create mode 100644 tests/test_coord_overlap_attribution.py diff --git a/scripts/coord/overlap.ps1 b/scripts/coord/overlap.ps1 index feed3ba0..9dc58300 100644 --- a/scripts/coord/overlap.ps1 +++ b/scripts/coord/overlap.ps1 @@ -68,6 +68,13 @@ function ConvertTo-Norm([string]$p) { return ($p -replace '\\', '/').TrimEnd('/').ToLowerInvariant() } +# One emitter for every -Json exit, so no path can print nothing. See the note at the -File query below. +function Write-JsonArray($Rows) { + $r = @($Rows) + if ($r.Count -eq 0) { Write-Output "[]"; return } + Write-Output ($r | ConvertTo-Json -Depth 6 -AsArray) +} + $gitArgs = @(); if ($Repo) { $gitArgs = @("-C", $Repo) } $common = (& git @gitArgs rev-parse --path-format=absolute --git-common-dir 2>$null) if ($LASTEXITCODE -ne 0 -or -not $common) { @@ -127,8 +134,35 @@ function Build-Map { $sessionsByCwd[$k] = $e.Record } + $worktrees = @(Get-WorktreeList) + + # ATTRIBUTE A SESSION TO THE MOST SPECIFIC WORKTREE CONTAINING IT, not to the first one that matches. + # + # Linked worktrees live UNDER the primary checkout (.../MessageFoundry/.claude/worktrees/), so + # every linked worktree's path is ALSO a prefix match for the primary's row. The old rule walked the + # session table and broke on the first hit, so the primary row was handed whichever linked-worktree + # session the hashtable happened to enumerate first -- reporting the primary checkout as LIVE, on a + # branch nobody was on, "building" a peer's task list. Hashtable order is not stable, so the wrong + # answer was also a different wrong answer each run, which is why it read as noise rather than a bug. + # + # Longest prefix wins is the only rule that survives nesting: a session in .../worktrees/foo matches + # both the primary and foo, and foo is the one it is actually sitting in. + $ownerByWorktree = @{} + foreach ($k in @($sessionsByCwd.Keys | Sort-Object)) { + $best = $null + foreach ($w in $worktrees) { + $wn = ConvertTo-Norm $w.Path + if ($k -eq $wn -or $k.StartsWith("$wn/")) { + if ($null -eq $best -or $wn.Length -gt $best.Length) { $best = $wn } + } + } + # Two sessions inside one worktree: first by sorted cwd, so the answer is at least the SAME + # answer every run. Reporting one of them is correct -- the row says the worktree is live. + if ($best -and -not $ownerByWorktree.ContainsKey($best)) { $ownerByWorktree[$best] = $sessionsByCwd[$k] } + } + $rows = @() - foreach ($w in (Get-WorktreeList)) { + foreach ($w in $worktrees) { $norm = ConvertTo-Norm $w.Path if ($norm -eq $myRootNorm) { continue } # not a collision with yourself if (-not (Test-Path -LiteralPath $w.Path)) { continue } @@ -169,11 +203,9 @@ function Build-Map { $files += $dirty $files = @($files | Where-Object { $_ } | Sort-Object -Unique) - # A session sitting anywhere INSIDE the worktree owns it, not just one whose cwd is the root. - $sess = $null - foreach ($k in $sessionsByCwd.Keys) { - if ($k -eq $norm -or $k.StartsWith("$norm/")) { $sess = $sessionsByCwd[$k]; break } - } + # A session sitting anywhere INSIDE the worktree owns it, not just one whose cwd is the root -- + # resolved above, against every worktree at once, because "inside" is ambiguous when they nest. + $sess = $ownerByWorktree[$norm] if ($files.Count -eq 0 -and -not $sess) { continue } $rows += [pscustomobject]@{ @@ -241,7 +273,13 @@ if ($File) { $hits += $r } } - if ($Json) { ($hits | ConvertTo-Json -Depth 6 -AsArray) | Write-Output; exit 0 } + # ALWAYS EMIT AN ARRAY. `@() | ConvertTo-Json -AsArray` sends ZERO objects down the pipeline, so + # ConvertTo-Json never runs and the script prints NOTHING -- despite -AsArray, which only shapes + # output that exists. "Nobody else is in this file" therefore looked identical on stdout to "this + # script died before it could answer", and a consumer had no way to tell an all-clear from a + # failure. (-InputObject is not the fix: with -AsArray it double-wraps to [[]].) Measured + # 2026-08-02 against the real script. + if ($Json) { (Write-JsonArray $hits); exit 0 } foreach ($h in $hits) { $state = if ($h.Live) { "LIVE $($h.Surface) session $($h.Short)" } else { "dormant worktree" } Write-Host " $File is also changed by $state in $($h.Worktree) [$($h.Branch)]" @@ -249,7 +287,7 @@ if ($File) { exit 0 } -if ($Json) { ($map | ConvertTo-Json -Depth 6 -AsArray) | Write-Output; exit 0 } +if ($Json) { (Write-JsonArray $map); exit 0 } if (@($map).Count -eq 0) { Write-Host "No other worktree has changes."; exit 0 } Write-Host "" diff --git a/tests/test_coord_overlap_attribution.py b/tests/test_coord_overlap_attribution.py new file mode 100644 index 00000000..397927f2 --- /dev/null +++ b/tests/test_coord_overlap_attribution.py @@ -0,0 +1,252 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Which worktree does ``scripts/coord/overlap.ps1`` say a session is sitting in? + +A session is attributed to a worktree by its cwd. The rule has to survive the layout this project +actually uses: linked worktrees live **inside** the primary checkout, at +``/.claude/worktrees/``. So every linked worktree's path is *also* a prefix match for +the primary's own row, and "the worktree containing this cwd" has two correct-looking answers. + +The old rule walked the session table and took the first prefix hit, which handed the primary row +whichever linked-worktree session the hashtable happened to enumerate first. The primary checkout was +then reported LIVE, on a branch nobody was on, "building" a peer's task list -- and because hashtable +order is not stable, it was a *different* wrong answer run to run, which is why it read as noise +rather than as a bug. + +Driven against a REAL nested worktree, because nesting is the entire mechanism. A fixture with two +sibling directories would pass under both the old rule and the new one and prove nothing. +""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +import sys +import time +from pathlib import Path +from typing import Any + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +OVERLAP = ROOT / "scripts" / "coord" / "overlap.ps1" +TIMEOUT = 60 +NOW_MS = int(time.time() * 1000) + +pytestmark = pytest.mark.skipif( + shutil.which("pwsh") is None or os.name != "nt", + reason="overlap.ps1 needs pwsh on Windows", +) + + +def git(repo: Path, *args: str) -> str: + proc = subprocess.run( + ["git", "-C", str(repo), *args], capture_output=True, text=True, timeout=TIMEOUT, check=True + ) + return proc.stdout + + +@pytest.fixture +def nested_layout(tmp_path: Path) -> tuple[Path, Path, Path]: + """The real shape: a primary, a linked worktree NESTED under it, and an outside vantage point. + + ``.claude/`` is gitignored and the ignore is pushed, so the primary sits exactly on origin/main + with a clean tree -- otherwise the nested worktree's own files would show up as the primary's + untracked changes and give it a row on their own merits, masking the attribution question. + """ + origin = tmp_path / "origin.git" + subprocess.run( + ["git", "init", "-q", "--bare", "-b", "main", str(origin)], check=True, capture_output=True + ) + primary = tmp_path / "primary" + primary.mkdir() + subprocess.run( + ["git", "init", "-q", "-b", "main", str(primary)], check=True, capture_output=True + ) + git(primary, "config", "user.email", "t@example.invalid") + git(primary, "config", "user.name", "t") + (primary / ".gitignore").write_text(".claude/\n", encoding="utf-8") + (primary / "alpha.txt").write_text("base\n", encoding="utf-8") + git(primary, "add", "-A") + git(primary, "commit", "-qm", "base") + git(primary, "remote", "add", "origin", str(origin)) + git(primary, "push", "-q", "origin", "main") + + nested = primary / ".claude" / "worktrees" / "peer" + git(primary, "worktree", "add", "-q", "-b", "peer-branch", str(nested)) + vantage = tmp_path / "vantage" + git(primary, "worktree", "add", "-q", "-b", "vantage-branch", str(vantage)) + return primary, nested, vantage + + +@pytest.fixture +def config_root(tmp_path: Path) -> Path: + root = tmp_path / ".claude-config" + (root / "sessions").mkdir(parents=True) + return root + + +def write_session( + config_root: Path, *, pid: int, cwd: Path, session_id: str, started_at: int = NOW_MS +) -> None: + """A live session record. A record stamped alongside its process's start fences as LIVE. + + ``started_at`` defaults to import time, which is the right stamp for *this* process; a spawned + child needs its own, because the fence rejects a record that precedes its process by too much. + """ + (config_root / "sessions" / f"{pid}.json").write_text( + json.dumps( + { + "pid": pid, + "sessionId": session_id, + "cwd": str(cwd), + "startedAt": started_at, + "version": "2.1.219", + "peerProtocol": 1, + "kind": "interactive", + "entrypoint": "claude-vscode", + "name": session_id[:8], + "nameSource": "derived", + } + ), + encoding="utf-8", + ) + + +def survey(vantage: Path, config_root: Path, tmp_path: Path) -> list[dict[str, Any]]: + """The whole map, from a worktree that is neither the primary nor the one under test.""" + proc = subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(OVERLAP), + "-Repo", + str(vantage), + "-Json", + "-Refresh", + "-ConfigRoot", + str(config_root), + "-TasksDir", + str(tmp_path / "no-such-tasks"), + ], + capture_output=True, + text=True, + timeout=TIMEOUT, + check=False, + ) + assert proc.returncode == 0, f"overlap exited {proc.returncode}: {proc.stderr}" + out = proc.stdout.strip() + parsed: list[dict[str, Any]] = json.loads(out) if out else [] + return parsed + + +def row_for(rows: list[dict[str, Any]], path: Path) -> dict[str, Any] | None: + want = str(path).replace("\\", "/").rstrip("/").lower() + for r in rows: + if str(r.get("Path", "")).replace("\\", "/").rstrip("/").lower() == want: + return r + return None + + +def test_a_nested_worktrees_session_is_attributed_to_it_not_the_primary( + nested_layout: tuple[Path, Path, Path], config_root: Path, tmp_path: Path +) -> None: + """THE BUG. One session, sitting in a worktree nested under the primary checkout. + + Its cwd is a prefix match for both. Before the fix the primary could win that race and be reported + as a live session on ``main`` -- a collision signal naming the wrong worktree, wrong branch and + wrong work. + """ + primary, nested, vantage = nested_layout + (nested / "alpha.txt").write_text("base\npeer edit\n", encoding="utf-8") + write_session(config_root, pid=os.getpid(), cwd=nested, session_id="aaaaaaaa-1111") + + rows = survey(vantage, config_root, tmp_path) + + peer = row_for(rows, nested) + assert peer is not None, "the nested worktree must be reported" + assert peer["Live"] is True, "the session is sitting in it" + assert peer["Short"] == "aaaaaaaa" + assert peer["Branch"] == "peer-branch" + + got = row_for(rows, primary) + # The primary is clean and holds no session, so the correct answer is no row at all. Before the + # fix it appeared, Live, wearing this session's id. + assert got is None or got["Live"] is False, f"the primary is not where that session is: {got}" + + +def test_the_primary_still_owns_a_session_whose_cwd_is_actually_in_it( + nested_layout: tuple[Path, Path, Path], config_root: Path, tmp_path: Path +) -> None: + """Longest-prefix must not become never-the-primary. + + A session genuinely sitting in the primary checkout -- including one in a subdirectory of it that + is not a worktree -- still belongs to the primary. This is the over-correction that would swap one + silent mis-attribution for another. + """ + primary, _nested, vantage = nested_layout + subdir = primary / "messagefoundry" + subdir.mkdir() + (primary / "alpha.txt").write_text("base\nprimary edit\n", encoding="utf-8") + write_session(config_root, pid=os.getpid(), cwd=subdir, session_id="bbbbbbbb-2222") + + rows = survey(vantage, config_root, tmp_path) + + got = row_for(rows, primary) + assert got is not None, "a session in the primary's own subdirectory must attribute to it" + assert got["Live"] is True + assert got["Short"] == "bbbbbbbb" + + +def test_two_nested_sessions_each_land_in_their_own_worktree( + nested_layout: tuple[Path, Path, Path], config_root: Path, tmp_path: Path +) -> None: + """The ordering hazard, with enough sessions present for order to matter. + + First-hit-wins is only *visibly* arbitrary once more than one session is a candidate. Two sessions + in two nested worktrees: each must land in its own, the primary must claim neither, and the answer + must be the same every run -- the old rule's depended on hashtable enumeration order, so an + unstable wrong answer read as noise and nobody filed it. + """ + primary, nested, vantage = nested_layout + other = primary / ".claude" / "worktrees" / "peer2" + git(primary, "worktree", "add", "-q", "-b", "peer2-branch", str(other)) + (nested / "alpha.txt").write_text("base\npeer edit\n", encoding="utf-8") + (other / "alpha.txt").write_text("base\npeer2 edit\n", encoding="utf-8") + + write_session(config_root, pid=os.getpid(), cwd=nested, session_id="cccccccc-3333") + child = subprocess.Popen( # a second REAL live pid; the fence needs a process, not a number + [sys.executable, "-c", "import time; time.sleep(120)"], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + try: + write_session( + config_root, + pid=child.pid, + cwd=other, + session_id="dddddddd-4444", + started_at=int(time.time() * 1000), + ) + answers = [ + json.dumps( + sorted((str(r["Path"]).lower(), bool(r["Live"]), str(r["Short"])) for r in rows) + ) + for rows in (survey(vantage, config_root, tmp_path) for _ in range(3)) + ] + finally: + child.kill() + child.wait(timeout=10) + + assert len(set(answers)) == 1, f"overlap gave more than one answer for one state: {answers}" + + rows = json.loads(answers[0]) + by_path = {p: (live, short) for p, live, short in rows} + assert by_path.get(str(nested).replace("\\", "/").lower()) == (True, "cccccccc") + assert by_path.get(str(other).replace("\\", "/").lower()) == (True, "dddddddd") + got = by_path.get(str(primary).replace("\\", "/").lower()) + assert got is None or got[0] is False, f"the primary owns neither session: {got}" diff --git a/tests/test_coord_overlap_signals.py b/tests/test_coord_overlap_signals.py index 98cdf298..490e0cfe 100644 --- a/tests/test_coord_overlap_signals.py +++ b/tests/test_coord_overlap_signals.py @@ -162,6 +162,57 @@ def test_an_untouched_file_is_not_reported( assert query(primary, tmp_path, "beta.txt") == [] +def raw_query(primary: Path, tmp_path: Path, path: str) -> str: + """The same query, but returning stdout VERBATIM -- ``query`` above maps '' to [] and would hide + the very difference under test.""" + proc = subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(OVERLAP), + "-Repo", + str(primary), + "-File", + path, + "-Json", + "-Refresh", + "-ConfigRoot", + str(tmp_path / "no-such-config"), + "-TasksDir", + str(tmp_path / "no-such-tasks"), + ], + capture_output=True, + text=True, + timeout=TIMEOUT, + check=False, + ) + assert proc.returncode == 0, f"overlap exited {proc.returncode}: {proc.stderr}" + return proc.stdout.strip() + + +def test_a_json_query_always_emits_an_array( + peer_worktree: tuple[Path, Path], tmp_path: Path +) -> None: + """AN ANSWER OF "NOBODY" MUST NOT LOOK LIKE NO ANSWER. + + ``@() | ConvertTo-Json -AsArray`` sends zero objects down the pipeline, so ConvertTo-Json never runs + and the script printed NOTHING -- ``-AsArray`` only shapes output that exists. On stdout that is + byte-for-byte what a script dying before it answers looks like, so no consumer could separate an + all-clear from a failure. The collision gate consumes this on every Edit and Write; measured + 2026-08-02, the gate reported "could not check" on an ordinary edit to an untouched file because of + exactly this. + """ + primary, peer = peer_worktree + (peer / "alpha.txt").write_text("base\nunsaved\n", encoding="utf-8") + + assert raw_query(primary, tmp_path, "beta.txt") == "[]", "a no-hit query must still answer" + hit = raw_query(primary, tmp_path, "alpha.txt") + assert hit.startswith("["), f"a hit must be an array too, not a bare object: {hit[:80]}" + assert json.loads(hit), "and it must parse with at least one row" + + def test_overlap_does_not_rewrite_a_peers_git_index( peer_worktree: tuple[Path, Path], tmp_path: Path ) -> None: From c6b8f42e774dcc81008c4e2ce6d9e539ee784cfc Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sat, 1 Aug 2026 23:32:43 -0500 Subject: [PATCH 02/10] fix(coord): the collision gate reported an all-clear when it had checked nothing Every fail-open path in this hook -- overlap script missing, throwing, or printing garbage -- exited 0 with EMPTY STDOUT. On a PreToolUse hook whose stdout is parsed as a decision, empty stdout means "allow", which is byte-for-byte what "checked, nobody else is in this file" looks like. So a gate that had consulted nothing was indistinguishable from a gate reporting all-clear, and its own failure reached the session as reassurance. That is the silent-control class this repo has now hit five times, and it is the same shape as the wired-but-inert announce shim: the surface that was supposed to report sat downstream of the failure it existed to detect. The posture does not change -- every one of these paths still ALLOWS. Only the silence does. It now emits a hookSpecificOutput.additionalContext notice naming which reason (overlap-missing / overlap-failed / overlap-empty / overlap-unparseable / payload-unreadable). It must be that JSON shape and never a bare line: this hook's stdout is a decision, so a stray line risks a misparse on every Edit and Write -- a diagnostic that would be a worse fault than the one it reports. There is deliberately no permissionDecision key: a notice that blocked would invert the fail-open posture that is the whole point of this gate. Rate-limited per reason (30 min, -NoticeCooldownMinutes) so a persistently broken overlap cannot narrate itself into every edit -- this gate's own docstring records where a gate that cries wolf ends up. The stamp lives under -StateDir, defaulting to the repo's coordination dir and resolved ONLY when about to report, so nothing new runs on the hot path. If the stamp cannot be read or written the notice is emitted anyway: the failure mode of a noise-suppressor must be noise, never quiet, or an unwritable directory silently restores exactly the behaviour this removes. Distinguishing overlap-empty from a resolved "nobody" required fixing the producer first (previous commit) -- you cannot detect a difference the producer never encoded. Verified against the real overlap script, not only the stubs: an ordinary edit to an untouched file is silent. Tests: -StateDir isolates the throttle per test, or the first notice would silence the next test's and the suite would pass on run order. --- scripts/hooks/collision_gate.ps1 | 101 +++++++++++++++++++++-- tests/test_collision_gate.py | 132 ++++++++++++++++++++++++++++--- 2 files changed, 215 insertions(+), 18 deletions(-) diff --git a/scripts/hooks/collision_gate.ps1 b/scripts/hooks/collision_gate.ps1 index b07b9c35..7fd64a91 100644 --- a/scripts/hooks/collision_gate.ps1 +++ b/scripts/hooks/collision_gate.ps1 @@ -22,6 +22,16 @@ session cannot work. That is the opposite of the worktree gate's posture (which protects the shared tree and should fail closed), and the difference is intentional. + FAILS OPEN, BUT NOT SILENTLY. Every one of those error paths used to `exit 0` with no output, which + on stdout is byte-for-byte what "checked, nobody else is touching this file" looks like. So a gate + that had checked NOTHING was indistinguishable from a gate reporting all-clear, and a session could + read the absence of a warning as evidence -- while the guard was inert. An unresolved run now says + so via additionalContext. It still allows: the posture is unchanged, only the silence is. + + Rate-limited per reason (`-NoticeCooldownMinutes`), because a persistently broken overlap script + would otherwise inject a notice into EVERY Edit and Write. If the throttle state cannot be read or + written the notice is emitted anyway -- silence is the defect being fixed, so the fallback is noise. + Wired on Edit|Write|MultiEdit|NotebookEdit. The overlap map is cached, so the common case is a cache read, not a git walk across every worktree. #> @@ -31,7 +41,13 @@ param( # than re-implementing its rule -- a test that asserts a copy of the rule proves nothing. [string]$OverlapScript = (Join-Path $PSScriptRoot "..\coord\overlap.ps1"), # Emit the decision and skip reading stdin (tests). - [string]$PathOverride + [string]$PathOverride, + # Where the per-reason "already told them" stamps live. Defaults to the repo's coordination dir. + # Parameterised so tests are isolated from the real repo AND from each other -- a throttle sharing + # one directory with the suite would make the first test's notice suppress the second's. + [string]$StateDir, + # How long one unresolved reason stays quiet after being reported. + [int]$NoticeCooldownMinutes = 30 ) # No $ErrorActionPreference = Stop: this gate fails OPEN, and a throw would be a deny-by-crash. @@ -51,10 +67,60 @@ function Deny([string]$Reason) { exit 0 } +function Write-Unresolved([string]$Slug, [string]$Detail) { + # THE GATE DID NOT CHECK. Say so, and allow anyway. + # + # It must be a JSON hookSpecificOutput.additionalContext payload and never a bare line: this is a + # PreToolUse hook whose stdout is PARSED AS A DECISION, so a stray line risks a misparse on every + # single Edit and Write -- turning a diagnostic into a worse fault than the one it reports. + # + # No permissionDecision key: adding one here would convert a broken guard into a blocked session, + # which is the fail-open posture inverted. + $emit = $true + try { + $dir = $StateDir + if (-not $dir) { + $common = (& git rev-parse --path-format=absolute --git-common-dir 2>$null) + if ($LASTEXITCODE -eq 0 -and $common) { $dir = Join-Path ([string]$common).Trim() "mefor-coord" } + } + if ($dir) { + $stamp = Join-Path $dir "gate-unresolved/$Slug.stamp" + $prev = Get-Item -LiteralPath $stamp -ErrorAction SilentlyContinue + if ($prev) { + $mins = ((Get-Date) - $prev.LastWriteTime).TotalMinutes + # Bound BOTH ways. A stamp dated in the future (clock skew, a copied tree) would read as + # eternally fresh and suppress this notice forever -- the same silence, now self-inflicted. + if ($mins -ge 0 -and $mins -lt $NoticeCooldownMinutes) { $emit = $false } + } + if ($emit) { + New-Item -ItemType Directory -Force -Path (Split-Path $stamp) | Out-Null + Set-Content -LiteralPath $stamp -Value ((Get-Date).ToString("o")) -Encoding UTF8 + } + } + } catch { + # Cannot throttle -> report. Silence is the defect being fixed, so the failure mode of the + # noise-suppressor must be noise, never quiet. + $emit = $true + } + if ($emit) { + $payload = @{ + hookSpecificOutput = @{ + hookEventName = "PreToolUse" + additionalContext = "[collision] The collision gate could NOT check this edit ($Slug): $Detail. It allowed the edit without consulting any peer worktree, so an absent collision warning means UNKNOWN here, not clear. Check by hand before assuming nobody else is in this file: pwsh -NoProfile -File scripts\coord\overlap.ps1" + } + } + [Console]::Out.Write(($payload | ConvertTo-Json -Compress -Depth 6)) + } + exit 0 +} + $target = $PathOverride if (-not $target) { if (-not [Console]::IsInputRedirected) { exit 0 } - try { $hook = [Console]::In.ReadToEnd() | ConvertFrom-Json } catch { exit 0 } + try { $hook = [Console]::In.ReadToEnd() | ConvertFrom-Json -ErrorAction Stop } catch { + Write-Unresolved "payload-unreadable" ` + "the PreToolUse payload on stdin was not readable JSON, so the gate never learned which file this edit targets" + } if (-not $hook) { exit 0 } $target = [string]$hook.tool_input.file_path # NotebookEdit and some variants name the path differently; absence just means nothing to check. @@ -62,14 +128,35 @@ if (-not $target) { } if (-not $target) { exit 0 } -if (-not (Test-Path -LiteralPath $OverlapScript)) { exit 0 } +if (-not (Test-Path -LiteralPath $OverlapScript)) { + Write-Unresolved "overlap-missing" "no overlap script at $OverlapScript" +} -$rows = @() +$raw = $null +$code = 0 try { $raw = & pwsh -NoProfile -NonInteractive -File $OverlapScript -File $target -Json 2>$null - if ($raw) { $rows = @($raw | ConvertFrom-Json) } -} catch { exit 0 } -if (-not $rows -or $rows.Count -eq 0) { exit 0 } + $code = $LASTEXITCODE +} catch { + Write-Unresolved "overlap-threw" "invoking the overlap script raised: $($_.Exception.Message)" +} +if ($code -ne 0) { + Write-Unresolved "overlap-failed" "the overlap script exited $code" +} + +# EMPTY OUTPUT IS NOT AN ANSWER. Under -Json a resolved "nobody else is in this file" is the two bytes +# `[]`; nothing at all is a script that never produced a verdict. Folding those together is precisely +# how this gate came to report all-clear while checking nothing. +$text = (@($raw) -join "`n").Trim() +if (-not $text) { + Write-Unresolved "overlap-empty" "the overlap script produced no output at all (a resolved 'nobody else' is '[]', not nothing)" +} + +$rows = @() +try { $rows = @($text | ConvertFrom-Json -ErrorAction Stop) } catch { + Write-Unresolved "overlap-unparseable" "the overlap script's output was not JSON" +} +if (-not $rows -or $rows.Count -eq 0) { exit 0 } # RESOLVED, and nobody else is touching it $live = @($rows | Where-Object { $_.Live }) if ($live.Count -eq 0) { exit 0 } # dormant only: worth knowing, not worth blocking diff --git a/tests/test_collision_gate.py b/tests/test_collision_gate.py index 240ab6bb..88371b14 100644 --- a/tests/test_collision_gate.py +++ b/tests/test_collision_gate.py @@ -53,16 +53,33 @@ def make_overlap_stub(tmp_path: Path, rows: list[dict[str, Any]]) -> Path: return stub -def run_gate(overlap: Path | None, file_path: str | None = "a.py") -> dict[str, Any] | None: - """Invoke the gate exactly as Claude Code does. Returns the deny object, or None for allow.""" +def run_gate( + overlap: Path | None, + file_path: str | None = "a.py", + state_dir: Path | None = None, + raw_input: str | None = None, +) -> dict[str, Any] | None: + """Invoke the gate exactly as Claude Code does. Returns the emitted object, or None for silence. + + ``state_dir`` isolates the unresolved-notice throttle. Without it the gate would stamp the REAL + repository's coordination directory, so one test's notice would silence the next one's -- and the + suite would pass or fail on the order it happened to run in. + """ payload: dict[str, Any] = {"tool_name": "Edit", "tool_input": {}} if file_path is not None: payload["tool_input"]["file_path"] = file_path args = ["pwsh", "-NoProfile", "-NonInteractive", "-File", str(GATE)] if overlap is not None: args += ["-OverlapScript", str(overlap)] + if state_dir is not None: + args += ["-StateDir", str(state_dir)] proc = subprocess.run( - args, input=json.dumps(payload), capture_output=True, text=True, timeout=180, check=False + args, + input=json.dumps(payload) if raw_input is None else raw_input, + capture_output=True, + text=True, + timeout=180, + check=False, ) # A hook must never crash the tool call: a non-zero exit is ignored by the harness, which would # leave the gate looking installed while permitting everything. @@ -158,26 +175,119 @@ def test_an_editing_peer_still_denies_when_another_peer_merely_committed(tmp_pat assert got["hookSpecificOutput"]["permissionDecision"] == "deny" -def test_fails_open_when_the_overlap_script_is_missing(tmp_path: Path) -> None: - assert run_gate(tmp_path / "does-not-exist.ps1") is None +def test_allows_a_payload_with_no_file_path(tmp_path: Path) -> None: + assert run_gate(make_overlap_stub(tmp_path, [LIVE_ROW]), file_path=None) is None + + +# ------------------------------------------------------------------- failing open, but not silently +# +# Every one of these paths used to `exit 0` with EMPTY STDOUT -- which is byte-for-byte what "checked, +# nobody else is in this file" looks like. A gate that had consulted nothing was indistinguishable from +# a gate reporting all-clear, so its own failure was reported to the session as reassurance. +# +# The posture does not change: all of them still ALLOW. Only the silence does. -def test_fails_open_when_the_overlap_script_throws(tmp_path: Path) -> None: +def unresolved(got: dict[str, Any] | None) -> str: + """Assert the shape of an unresolved notice and return its text.""" + assert got is not None, "an unresolved gate must say so, not exit silently" + out = got["hookSpecificOutput"] + # THE FAIL-OPEN POSTURE IS THE POINT. A notice that carried a permissionDecision would turn a + # broken guard into a blocked session -- strictly worse than the silence it replaces. + assert "permissionDecision" not in out, f"a diagnostic must never block: {out}" + assert out["hookEventName"] == "PreToolUse" + ctx: str = out["additionalContext"] + # It must be a JSON payload, not a bare line: this hook's stdout is parsed as a DECISION, so a + # stray line risks a misparse on every Edit and Write. json.loads in run_gate already proved that. + assert "could NOT check" in ctx + return ctx + + +def test_says_so_when_the_overlap_script_is_missing(tmp_path: Path) -> None: + got = run_gate(tmp_path / "does-not-exist.ps1", state_dir=tmp_path / "state") + assert "overlap-missing" in unresolved(got) + + +def test_says_so_when_the_overlap_script_throws(tmp_path: Path) -> None: broken = tmp_path / "broken.ps1" broken.write_text("param([string]$File,[switch]$Json)\nthrow 'boom'\n", encoding="utf-8") - assert run_gate(broken) is None + ctx = unresolved(run_gate(broken, state_dir=tmp_path / "state")) + assert "overlap-failed" in ctx or "overlap-threw" in ctx -def test_fails_open_when_the_overlap_script_emits_junk(tmp_path: Path) -> None: +def test_says_so_when_the_overlap_script_emits_junk(tmp_path: Path) -> None: junk = tmp_path / "junk.ps1" junk.write_text( "param([string]$File,[switch]$Json)\nWrite-Output 'not json'\n", encoding="utf-8" ) - assert run_gate(junk) is None + assert "overlap-unparseable" in unresolved(run_gate(junk, state_dir=tmp_path / "state")) -def test_allows_a_payload_with_no_file_path(tmp_path: Path) -> None: - assert run_gate(make_overlap_stub(tmp_path, [LIVE_ROW]), file_path=None) is None +def test_says_so_when_the_overlap_script_answers_with_nothing(tmp_path: Path) -> None: + """THE CONFLATION THIS FIX EXISTS FOR, and the only one with no other symptom. + + Under ``-Json`` a *resolved* "nobody else is in this file" is the two bytes ``[]``. A script that + exits 0 having printed nothing has produced no verdict at all. The gate treated both as "no rows, + allow", so a silently-broken overlap script was reported to the session as an all-clear forever. + """ + mute = tmp_path / "mute.ps1" + mute.write_text("param([string]$File,[switch]$Json)\nexit 0\n", encoding="utf-8") + assert "overlap-empty" in unresolved(run_gate(mute, state_dir=tmp_path / "state")) + + +def test_a_resolved_empty_answer_stays_silent(tmp_path: Path) -> None: + """The other half of that pair, and the reason it cannot simply always warn. + + ``[]`` IS an answer. This is the hot path on every single Edit and Write, so a gate that spoke up + here would put a line into the context of every edit in the repo forever. + """ + assert run_gate(make_overlap_stub(tmp_path, []), state_dir=tmp_path / "state") is None + + +def test_says_so_when_the_hook_payload_is_unreadable(tmp_path: Path) -> None: + got = run_gate( + make_overlap_stub(tmp_path, [LIVE_ROW]), state_dir=tmp_path / "state", raw_input="{not json" + ) + assert "payload-unreadable" in unresolved(got) + + +def test_the_notice_is_rate_limited_per_reason(tmp_path: Path) -> None: + """A persistently broken overlap script must not narrate itself into every edit. + + Same state dir twice: the first call reports, the second is suppressed. This is the difference + between a diagnostic and a nag, and this gate's own docstring names where nags end up. + """ + state = tmp_path / "state" + missing = tmp_path / "does-not-exist.ps1" + assert run_gate(missing, state_dir=state) is not None, "first occurrence must be reported" + assert run_gate(missing, state_dir=state) is None, "second occurrence must be suppressed" + + +def test_a_different_reason_is_not_suppressed_by_the_first(tmp_path: Path) -> None: + """Per-reason, not global -- otherwise one benign fault masks every later one.""" + state = tmp_path / "state" + assert run_gate(tmp_path / "does-not-exist.ps1", state_dir=state) is not None + junk = tmp_path / "junk.ps1" + junk.write_text( + "param([string]$File,[switch]$Json)\nWrite-Output 'not json'\n", encoding="utf-8" + ) + assert "overlap-unparseable" in unresolved(run_gate(junk, state_dir=state)) + + +def test_an_unwritable_throttle_reports_anyway(tmp_path: Path) -> None: + """The noise-suppressor must fail toward NOISE. + + If it failed toward quiet, a coordination directory that could not be written would restore the + exact silent-allow this whole change removes -- and it would do it invisibly. + """ + blocker = tmp_path / "a-file" + blocker.write_text("not a directory", encoding="utf-8") + unwritable = blocker / "state" # cannot be created: its parent is a file + missing = tmp_path / "does-not-exist.ps1" + assert run_gate(missing, state_dir=unwritable) is not None + assert run_gate(missing, state_dir=unwritable) is not None, ( + "must not go quiet when it cannot stamp" + ) # --------------------------------------------------------------------------------- installer From 07deb2fe4b4bab8f559ac8b3eeb6661dbff8159d Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sat, 1 Aug 2026 23:33:06 -0500 Subject: [PATCH 03/10] fix(coord): claim.ps1 accepted a new note, reported success, and discarded it -Take documented itself as idempotent -- "re-taking your own claim just refreshes the note" -- and did not refresh anything. A new -Note was taken, acknowledged and dropped. That is worse than an outright failure, because of what the note is for. It is the only field written deliberately to say what a session is doing, and announce-session.ps1 broadcasts it to every session joining the repo while telling them to prefer it over the worktree name. So the one field elevated to authoritative was the one field that could not be corrected. Measured 2026-08-02: a claim note was still announcing "NO PR OPENED -- honouring the #119 merge freeze" to every joining session hours after both that PR and the one it gated had merged. The workaround people reached for -- -Release then -Take -- drops the claim in between, re-opening the race the claim exists to close. Re-taking a key you hold now rewrites the file in place: note, branch (a worktree can have switched branches, and a claim naming a branch nobody is on is another confidently-wrong coordination fact) and a new `refreshed` stamp, leaving `claimed` untouched -- which is what proves the claim was never let go. Write-then-rename, not a truncating write: claim_check.py swallows a JSON parse error into "not claimed", so a torn file is a silently disabled gate, and a crash mid-refresh must leave the old note. Mutual exclusion is unchanged and pinned: a peer's key is still refused. One trap found by the test rather than by reading. ConvertFrom-Json silently coerces an ISO-8601 string to [datetime], so [string]$c.claimed returns the local short form -- sub-second precision and UTC offset gone. Writing that back would have downgraded the stamp on every refresh, and it would still have parsed, so nothing would ever have complained. Stamps now round-trip through "o", and the test asserts byte equality rather than "still parses". The same coercion is handled where announce reads it, with an invariant-culture parse for the string case. announce-session.ps1 now prints each claim note's AGE (from `refreshed` else `claimed`, "age unknown" when it cannot be determined -- an unknown age must not render as a fresh one). Elevating a note to authoritative makes a stale one strictly more dangerous than none, and age is the cheap signal that lets a reader discount it. Not taken here: claim -List's staleness-vs-liveness rendering, which is already open as its own change. --- scripts/coord/claim.ps1 | 52 ++++++++- scripts/hooks/announce-session.ps1 | 36 ++++++- tests/test_coord_claim_refresh.py | 164 +++++++++++++++++++++++++++++ 3 files changed, 247 insertions(+), 5 deletions(-) create mode 100644 tests/test_coord_claim_refresh.py diff --git a/scripts/coord/claim.ps1 b/scripts/coord/claim.ps1 index e9cabda9..61111ab4 100644 --- a/scripts/coord/claim.ps1 +++ b/scripts/coord/claim.ps1 @@ -34,7 +34,8 @@ #> [CmdletBinding()] param( - # Claim this key for THIS worktree. Idempotent: re-taking your own claim just refreshes the note. + # Claim this key for THIS worktree. Idempotent: re-taking a key you already hold refreshes its note + # and branch in place (never dropping the claim), rather than failing. [string]$Take, # Release a claim this worktree holds. [string]$Release, @@ -62,6 +63,16 @@ function ConvertTo-KeyFile([string]$Key) { $safe } +# ConvertFrom-Json SILENTLY COERCES an ISO-8601 string into a [datetime], so `[string]$c.claimed` does +# not give you back what was written -- it gives the local short form ("08/01/2026 23:17:46"), losing +# the sub-second precision and the UTC offset. Writing that back would quietly downgrade the stamp on +# every refresh, and it would still parse, so nothing would ever complain. Round-trip it instead. +function ConvertTo-Stamp($Value) { + if ($Value -is [datetime]) { return $Value.ToString("o") } + if ($Value -is [datetimeoffset]) { return $Value.ToString("o") } + return [string]$Value +} + function Get-Mine([string]$Path) { $c = Get-Content $Path -Raw | ConvertFrom-Json $held = ($c.worktree -replace '\\', '/').TrimEnd('/') @@ -126,8 +137,45 @@ try { } catch [System.IO.IOException] { $info = Get-Mine $file if ($info.IsMine) { - # Re-taking your own claim is a no-op, not an error: a session should be able to re-assert freely. + # Re-taking your own claim is not an error: a session should be able to re-assert freely. It was + # also, until now, not a REFRESH -- despite the -Take parameter documenting one. A new -Note was + # accepted, reported as success, and silently discarded. + # + # That is worse than an outright failure, because the note is the one field written deliberately + # to say what a session is doing, and announce-session.ps1 broadcasts it to every session that + # joins the repo, telling them to prefer it over the worktree name. Measured 2026-08-02: the + # 'announce-hook' claim still read "NO PR OPENED -- honouring the #119 merge freeze" to every + # joining session hours after both #133 and #119 had merged. A stale note broadcast as current + # intent is a coordination fault, not a cosmetic one -- and the documented workaround (-Release + # then -Take) drops the claim in between, re-opening the race the claim exists to close. + if ($Note) { + $updated = [ordered]@{ + key = [string]$info.Claim.key + note = $Note + # Refresh the branch too: a worktree can have switched branches since the claim was made, + # and a claim naming a branch nobody is on is another confidently-wrong coordination fact. + branch = $branch + worktree = [string]$info.Claim.worktree + # `claimed` is the identity of the claim and never moves; `refreshed` is what tells a + # reader how old the NOTE is, which is the question a stale note makes urgent. + claimed = ConvertTo-Stamp $info.Claim.claimed + refreshed = (Get-Date).ToString("o") + } | ConvertTo-Json -Compress + # Write-then-rename, not a truncating in-place write. A torn claim file does not fail loudly: + # scripts/hooks/claim_check.py swallows a parse error into "not claimed", which would disable + # the enforced gate for that key -- so a crash mid-write must leave the OLD file intact. + $tmp = "$file.$PID.tmp" + # UTF8 WITHOUT a BOM, for that same reader: a BOM makes json.loads raise. + [System.IO.File]::WriteAllBytes($tmp, [System.Text.Encoding]::UTF8.GetBytes($updated)) + Move-Item -LiteralPath $tmp -Destination $file -Force + Write-Host "" + Write-Host "REFRESHED '$Take' (held since $($info.Claim.claimed))." -ForegroundColor Green + Write-Host " note : $Note" + exit 0 + } Write-Host "You already hold '$Take' (claimed $($info.Claim.claimed))." -ForegroundColor Green + Write-Host " note : $($info.Claim.note)" + Write-Host " (pass -Note to update it -- it is what other sessions are shown.)" exit 0 } Write-Host "" diff --git a/scripts/hooks/announce-session.ps1 b/scripts/hooks/announce-session.ps1 index 88ebbd79..60014ecc 100644 --- a/scripts/hooks/announce-session.ps1 +++ b/scripts/hooks/announce-session.ps1 @@ -96,6 +96,13 @@ function Get-ClaimNotes { # The claim note is the only field written DELIBERATELY to say what a session is doing, so lead # with it where one exists. Fail-open: no claims, unreadable claims, or no claim for a peer all # just mean the name is all we have. + # + # CARRY THE NOTE'S AGE, NOT JUST THE NOTE. Elevating the claim note to authoritative -- which the + # roster below explicitly does, telling readers to prefer it over the worktree name -- makes a STALE + # note strictly more dangerous than no note: it is read as current intent. Measured 2026-08-02: the + # 'announce-hook' note still told every joining session "NO PR OPENED -- honouring the #119 merge + # freeze" hours after both #133 and #119 had merged. Age is the cheap signal that lets a reader + # discount it. `refreshed` where the holder updated the note, else `claimed`. param([string]$ClaimsDir) $map = @{} try { @@ -103,7 +110,25 @@ function Get-ClaimNotes { foreach ($f in @(Get-ChildItem -LiteralPath $ClaimsDir -Filter '*.json' -ErrorAction SilentlyContinue)) { try { $c = Get-Content -LiteralPath $f.FullName -Raw | ConvertFrom-Json - if ($c.worktree -and $c.note) { $map[(Get-Norm ([string]$c.worktree))] = [string]$c.note } + if (-not ($c.worktree -and $c.note)) { continue } + $hrs = $null + # An unparseable or absent stamp leaves $hrs null, and the caller then prints the note + # with no age claim at all -- an unknown age must not be rendered as a fresh one. + try { + # ConvertFrom-Json hands these back as [datetime] already. Round-tripping through + # [string] would lose the offset and then re-parse under the host's culture, which + # is how a date silently becomes a different date. + $at = if ($c.refreshed) { $c.refreshed } else { $c.claimed } + $when = if ($at -is [datetime]) { $at } + elseif ($at -is [datetimeoffset]) { $at.LocalDateTime } + elseif ($at) { [datetime]::Parse([string]$at, [cultureinfo]::InvariantCulture) } + else { $null } + if ($when) { $hrs = [int]((Get-Date) - $when).TotalHours } + } catch { $hrs = $null } + $map[(Get-Norm ([string]$c.worktree))] = [pscustomobject]@{ + Note = [string]$c.note + Hours = $hrs + } } catch { } } } catch { } @@ -565,6 +590,8 @@ try { $lines += ' Read "claim:" where present and IGNORE the worktree name: the name is a' $lines += ' creation-time label, nothing keeps it current, and one of them is known to' $lines += ' describe work that session never did. The claim is written deliberately.' + $lines += ' Its bracketed age is how old the NOTE is, not how long the work has run --' + $lines += ' an old note may describe a PR that has since merged. Verify before relying.' $i = 0 foreach ($e in $listed) { $i++ @@ -575,8 +602,11 @@ try { $tail = if ($e.Reason) { " ($($e.Reason))" } else { '' } $lines += " [$i] $verb $(Get-Clean ([string]$p.Worktree) 40) [$(Get-Clean ([string]$p.Branch) 60)] $(Get-Clean ([string]$p.Surface) 16)/$(Get-Clean ([string]$p.Login) 24)$flag$tail" $lines += " cwd: $(Get-Clean ([string]$p.Cwd) 200)" - $note = $claims[(Get-Norm ([string]$p.Cwd))] - if ($note) { $lines += " claim: $(Get-Clean ([string]$note) 160)" } + $claim = $claims[(Get-Norm ([string]$p.Cwd))] + if ($claim) { + $stamp = if ($null -ne $claim.Hours) { " [written $($claim.Hours)h ago]" } else { ' [age unknown]' } + $lines += " claim: $(Get-Clean ([string]$claim.Note) 160)$stamp" + } } if ($more -gt 0) { $lines += " ...and $more more (run: pwsh -NoProfile -File scripts\coord\presence.ps1)" diff --git a/tests/test_coord_claim_refresh.py b/tests/test_coord_claim_refresh.py new file mode 100644 index 00000000..fe0a9383 --- /dev/null +++ b/tests/test_coord_claim_refresh.py @@ -0,0 +1,164 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Re-taking a claim you already hold must actually refresh its note. + +``scripts/coord/claim.ps1 -Take`` documented itself as idempotent -- "re-taking your own claim just +refreshes the note" -- and did not do it. A new ``-Note`` was accepted, reported as success, and +silently discarded. + +That is worse than a plain failure. The note is the one field written deliberately to say what a +session is doing, and ``announce-session.ps1`` broadcasts it to every session joining the repo while +telling them to prefer it over the worktree name. Measured 2026-08-02: the ``announce-hook`` claim was +still announcing "NO PR OPENED -- honouring the #119 merge freeze" to every joining session hours +after both PRs had merged. A stale note presented as current intent is a coordination fault. + +The workaround people reached for -- ``-Release`` then ``-Take`` -- drops the claim in between, which +re-opens the very race the claim exists to close. So the property under test is *refresh in place*: +the note changes and ``claimed`` does not, which is what proves the claim was never let go. +""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +from pathlib import Path +from typing import Any + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +CLAIM = ROOT / "scripts" / "coord" / "claim.ps1" +TIMEOUT = 60 + +pytestmark = pytest.mark.skipif( + shutil.which("pwsh") is None or os.name != "nt", + reason="claim.ps1 needs pwsh on Windows", +) + + +def git(repo: Path, *args: str) -> str: + proc = subprocess.run( + ["git", "-C", str(repo), *args], capture_output=True, text=True, timeout=TIMEOUT, check=True + ) + return proc.stdout + + +@pytest.fixture +def repo(tmp_path: Path) -> Path: + r = tmp_path / "repo" + r.mkdir() + 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") + (r / "f.txt").write_text("x", encoding="utf-8") + git(r, "add", "-A") + git(r, "commit", "-qm", "base") + return r + + +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.""" + return subprocess.run( + ["pwsh", "-NoProfile", "-NonInteractive", "-File", str(CLAIM), *args], + cwd=str(cwd), + capture_output=True, + text=True, + timeout=TIMEOUT, + check=False, + ) + + +def claim_file(repo: Path, key: str) -> Path: + common = git(repo, "rev-parse", "--path-format=absolute", "--git-common-dir").strip() + return Path(common) / "mefor-coord" / "claims" / f"{key}.json" + + +def read_claim(repo: Path, key: str) -> dict[str, Any]: + raw = claim_file(repo, key).read_bytes() + # NO BOM. scripts/hooks/claim_check.py reads this with encoding="utf-8" and a BOM makes json.loads + # raise -- which that gate swallows into "not claimed", silently disabling itself for this key. + assert not raw.startswith(b"\xef\xbb\xbf"), "claim file must be UTF-8 without a BOM" + parsed: dict[str, Any] = json.loads(raw.decode("utf-8")) + return parsed + + +def test_retaking_your_own_claim_replaces_the_note(repo: Path) -> None: + assert claim(repo, "-Take", "k", "-Note", "first").returncode == 0 + before = read_claim(repo, "k") + + proc = claim(repo, "-Take", "k", "-Note", "second -- the PR has merged") + assert proc.returncode == 0, proc.stderr + + after = read_claim(repo, "k") + assert after["note"] == "second -- the PR has merged" + assert "REFRESHED" in proc.stdout, "it must not report success without saying what it did" + # THE CLAIM WAS NEVER RELEASED. `claimed` is the claim's identity; if the refresh had gone through + # release-then-retake, this would have moved -- and there would have been a window in between with + # the key free for another session to take. + # Byte-identical, not merely "still parses". ConvertFrom-Json coerces an ISO-8601 string into a + # [datetime], so a naive rewrite emits the local short form -- lower precision, no UTC offset, and + # culture-dependent on the way back in. It would still parse, so nothing would ever complain. + assert after["claimed"] == before["claimed"] + assert "T" in str(before["claimed"]), "the stamp under test must be the ISO-8601 form" + assert after["refreshed"], "a refreshed note needs its own stamp, or readers cannot date it" + + +def test_a_fresh_claim_has_no_refreshed_stamp(repo: Path) -> None: + """Absent, not zero. Readers fall back to ``claimed``; a fabricated stamp would date it wrong.""" + assert claim(repo, "-Take", "k", "-Note", "first").returncode == 0 + assert "refreshed" not in read_claim(repo, "k") + + +def test_retaking_without_a_note_leaves_the_note_alone(repo: Path) -> None: + """Re-asserting a claim is a normal thing to do and must not blank out what it says.""" + assert claim(repo, "-Take", "k", "-Note", "the real work").returncode == 0 + proc = claim(repo, "-Take", "k") + assert proc.returncode == 0 + assert read_claim(repo, "k")["note"] == "the real work" + assert "the real work" in proc.stdout, "show the note, so a session can see it is stale" + + +def test_the_refresh_updates_the_branch_too(repo: Path) -> None: + """A claim naming a branch nobody is on is another confidently-wrong coordination fact.""" + assert claim(repo, "-Take", "k", "-Note", "n").returncode == 0 + git(repo, "checkout", "-q", "-b", "moved-on") + assert claim(repo, "-Take", "k", "-Note", "n2").returncode == 0 + assert read_claim(repo, "k")["branch"] == "moved-on" + + +def test_a_peers_claim_is_still_refused(repo: Path, tmp_path: Path) -> None: + """THE REGRESSION GUARD. The refresh path must not become a way to rewrite someone else's note. + + Mutual exclusion is the whole point of this script; a fix for a stale note that let any session + overwrite any other session's note would trade a stale fact for a forged one. + """ + peer = tmp_path / "peer-wt" + git(repo, "worktree", "add", "-q", "-b", "peer-branch", str(peer)) + assert claim(peer, "-Take", "k", "-Note", "the peer's work").returncode == 0 + + proc = claim(repo, "-Take", "k", "-Note", "mine now") + assert proc.returncode == 1, "taking a peer's key must fail" + assert "BLOCKED" in proc.stdout + assert read_claim(repo, "k")["note"] == "the peer's work", ( + "a refused take must not have written" + ) + + +def test_a_refresh_that_cannot_be_written_leaves_the_old_claim_intact(repo: Path) -> None: + """Write-then-rename, not truncate-in-place. + + A claim file caught mid-write does not fail loudly: ``claim_check.py`` swallows a parse error into + "not claimed" and stops enforcing that key. So a crash during a refresh must leave the OLD note -- + a stale claim is a bad note, a torn one is a disabled gate. + """ + assert claim(repo, "-Take", "k", "-Note", "first").returncode == 0 + path = claim_file(repo, "k") + # Hold the file open for exclusive write: the rename cannot land, and the old bytes must survive. + with open(path, "r+b") as held: + held.read() + proc = claim(repo, "-Take", "k", "-Note", "second") + assert proc.returncode != 0 or read_claim(repo, "k")["note"] in {"first", "second"} + # Whatever happened, the file is still parseable -- that is the property the gate depends on. + assert read_claim(repo, "k")["key"] == "k" From 3bacb6a2fa9d209ed2017bc3a5ef2c96f8b68b87 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sat, 1 Aug 2026 23:33:22 -0500 Subject: [PATCH 04/10] docs(coord): record the three fixes, and correct a claim that has expired SESSION-DRIFT-CONTROLS.md: a fifth instance of the silent-control class, in the collision gate itself, added to the callout that names the class. It carries the part worth reusing -- the fix was not "check harder", it was giving two states different bytes, and the first attempt failed because the PRODUCER had never encoded the difference. Status-table rows for the three controls, and the claim-refresh behaviour beside claim.ps1's entry. WORKTREES.md: the announce id rule already warned that a prefix match resolves a peer in the primary to an arbitrary worktree session, because every worktree cwd extends the primary's. overlap.ps1 had that same trap live at the same time. Noted there, with the distinction that matters: overlap genuinely needs a prefix match, so the cure is longest-prefix rather than exact-match. And a correction. The broadcast-constraints list said of last week's merge freeze that "#119 never merged (it died on an unrelated CI timeout)". It merged the following day, 2026-08-02 01:45Z. Verified against the API rather than restated. The lesson is unchanged and in fact sharper: the recipients could not evaluate the predicate, so the freeze outlived its own condition in both directions -- five sessions held while it had not arrived, and a claim note was still announcing it hours after it had. --- docs/SESSION-DRIFT-CONTROLS.md | 29 ++++++++++++++++++++++++++++- docs/WORKTREES.md | 15 +++++++++++---- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/docs/SESSION-DRIFT-CONTROLS.md b/docs/SESSION-DRIFT-CONTROLS.md index c1253941..01d0a20b 100644 --- a/docs/SESSION-DRIFT-CONTROLS.md +++ b/docs/SESSION-DRIFT-CONTROLS.md @@ -85,7 +85,12 @@ and the open work claims. Frequently forgotten in discussions of "the gate", but it is the same problem class: - **[`scripts/coord/claim.ps1`](../scripts/coord/claim.ps1)** — atomic exclusive-create of - `/mefor-coord/claims/.json`. Claims work, not numbers. + `/mefor-coord/claims/.json`. Claims work, not numbers. `-Take` on a key you + already hold **refreshes the note and branch in place**, stamping `refreshed`; until 2026-08-02 it + accepted a new `-Note`, reported success and discarded it, so the only way to correct a note was + `-Release` then `-Take` — which drops the claim in between and re-opens the race it exists to close. + The note is what `announce-session.ps1` broadcasts to every joining session *in preference to the + worktree name*, so a note that cannot be corrected is announced as current intent indefinitely. - **[`scripts/hooks/claim_check.py`](../scripts/hooks/claim_check.py)** — `commit-msg` gate: a commit whose *subject* declares `BACKLOG #N` with a code-touching diff must hold a claim on N **for this worktree**. Motivated by a recorded incident: three sessions independently fixed one npm advisory; two PRs were @@ -114,6 +119,25 @@ writes when measured. > strictly downstream of the failure it existed to detect. Looking was not neglected; it was > impossible. When adding a control, ask which surface still reports when the control itself fails to > load. (Formulation owed to the session that hit four instances of the same class in one day.) +> +> **A fifth instance, found 2026-08-02 in the collision gate itself — fixed.** Every fail-open path +> (`overlap.ps1` missing, throwing, or printing garbage) exited 0 with **empty stdout**, which on this +> hook's stdout is byte-for-byte identical to *"checked, nobody else is in this file"*. A gate that had +> consulted nothing reported an all-clear, so its own failure reached the session as reassurance. The +> sharpest case needed no breakage at all, and the first attempt at the fix walked straight into it: +> **`overlap.ps1` had no representation for "nobody else"**. `@() | ConvertTo-Json -AsArray` sends zero +> objects down the pipeline, so ConvertTo-Json never runs and the script printed *nothing* — `-AsArray` +> only shapes output that already exists. A gate rule of "empty output means the script never answered" +> therefore fired on an ordinary edit to an untouched file, which is most edits. Caught by running the +> real pair rather than the test stubs, which used a shape the real script never produced. +> +> So the fix is in two places, and the second is the load-bearing one: `overlap.ps1` now always emits a +> JSON array (`[]` for no hits), and only then can the gate treat silence as a fault. It emits a +> `hookSpecificOutput.additionalContext` notice naming the reason, rate-limited per reason so a +> persistent fault cannot narrate itself into every edit, and **fails toward noise** when it cannot +> write that rate-limit stamp. The posture is unchanged: it still allows. Note the shape of the fix — +> it is not "check harder", it is *give the two states different bytes*, and you cannot detect a +> difference the producer never encoded. ### Recovery and lifecycle @@ -145,6 +169,9 @@ reading the emitted decision — not by reading source alone. | Announce-on-join (`announce-session.ps1`) | user | **NEW** — the only **push** control; asks, cannot send, and every decision leaves a receipt | | Announce wiring reaches a real script | test | **NEW** — `tests/test_announce_wiring.py`; nothing asserted this for *any* hook before, which is how a wired-but-inert shim survived weeks | | Announce missing-script notice | user | **NEW** — the one surface that still reports when the script itself fails to resolve | +| Collision gate — unresolved notice | user | **NEW** — every fail-open path used to be indistinguishable from an all-clear; it now says which reason, once per 30 min, and still allows | +| Claim note refresh (`-Take -Note` on a held key) | manual | **NEW** — was a silent discard; refresh is now in place, so correcting a note never drops the claim | +| Overlap session attribution | manual + gate | **FIXED** — longest-prefix, not first-hit. Linked worktrees are nested under the primary, so the primary's row used to absorb an arbitrary peer's session and report itself LIVE on `main` | | Claim / alloc / ledger gates | git hooks | LIVE | | `new.ps1` / `remove.ps1` / `prune-merged.ps1` | manual | LIVE, **sibling-layout only** | | `tests/test_worktree_gate*.py`, `test_install_gate_wiring.py` | CI + local | Was **85 green, and blind** — every one bound the repo copy; nothing read the installed copy or any live `settings.json`. Now 91 across six files, plus the local-only parity check below | diff --git a/docs/WORKTREES.md b/docs/WORKTREES.md index 1d85314f..939bd2f4 100644 --- a/docs/WORKTREES.md +++ b/docs/WORKTREES.md @@ -341,7 +341,11 @@ the id rule; the model does the sending. banners is the **registry** id. `ccd_session_mgmt` uses a *different* id for the same session. **The cwd is the only join key, and it must be matched exactly, never by prefix** — every worktree cwd is an extension of the primary's, so a prefix match resolves a peer in the primary to an arbitrary worktree -session. Branch is not a join key either: measured 2026-08-01, the two rosters reported different +session. (The same trap, same nesting, was live in `overlap.ps1` until 2026-08-02: it *needs* a prefix +match, since a session may sit in any subdirectory, and it took the **first** hit — so the primary's row +absorbed whichever nested-worktree session the hash table enumerated first and reported the primary LIVE +on `main`. Where a prefix match is genuinely required, the rule has to be **longest prefix wins**.) +Branch is not a join key either: measured 2026-08-01, the two rosters reported different branches for the same checkout in 2 of 6 cases. A usable id starts with `local_`. **A registry id passed to `send_message` fails silently**, which reads as the peer ignoring you. @@ -395,9 +399,12 @@ it by hand. Three constraints came out of that, recorded here so the next attemp them: - **A broadcast needs an expiry or a predicate the *recipient* can evaluate — never a promise from the - sender.** A merge freeze went out with "lift when #119 merges". #119 never merged (it died on an - unrelated CI timeout), so five sessions held on a condition that could not arrive, and it took a - second round to retract. + sender.** A merge freeze went out with "lift when #119 merges". #119 did not merge that day (an + unrelated CI timeout), so five sessions held on a condition that had not arrived and it took a second + round to retract. It merged the *following* day, 2026-08-02 01:45Z — which sharpens the point rather + than softening it: the recipients had no way to evaluate the predicate, so the freeze outlived its + own condition in both directions. As of 2026-08-02 a claim note still announcing that freeze was + being read by every joining session hours after both it and #133 had merged. - **"Don't do X" is the wrong primitive when automation already has X armed.** The freeze asked sessions not to merge, while six PRs had auto-merge *armed* and would have landed with nobody clicking anything. The correct ask was an action — "disarm auto-merge" — not restraint. From c8f0030ba7f383daa7f9e6b4f7b084e9b755c23d Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sat, 1 Aug 2026 23:40:34 -0500 Subject: [PATCH 05/10] docs(coord): announce-on-join merged and was never installed Found while checking a peer session's report, not by looking for it. That session announced itself by hand on 2026-08-02 and gave the reason as "the hook is on an unmerged branch". It had merged (#133, 3389aa2b) hours earlier, so the observation was right and the diagnosis was not, and nothing would have corrected it. Measured across all five config roots: - no `mefor-announce` UserPromptSubmit entry anywhere - the one UserPromptSubmit entry installed is `# mefor-web-announce`, which resolves scripts/hooks/announce.ps1 -- a different script in a different repo, and one the installer's own comment already warns is easy to confuse with this marker - /mefor-coord/announce/ does not exist, so there is not a single receipt: it has never executed install-coordination.ps1 was last run before the announce row existed, and merging a hook does not install one. Its two other entries -- the SessionStart banner and the collision gate -- were wired then and are present, which is precisely why nothing looked wrong. The part worth carrying: the missing-script notice was built so this class could not hide, and it CANNOT FIRE when the hook is not wired at all, because it lives inside the shim. Same shape as the defect this document already records one level down -- the detector sat downstream of the failure it existed to detect. So the status table now distinguishes rule 4's inert-BY-DESIGN from this one's inert-BY-ACCIDENT, and the confirmation step is a receipt on disk rather than a reading of the settings file. Not installed here: that writes ~/.claude/settings.json, which is shared with every session on this machine. Owner's call, from a plain terminal. --- docs/SESSION-DRIFT-CONTROLS.md | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/docs/SESSION-DRIFT-CONTROLS.md b/docs/SESSION-DRIFT-CONTROLS.md index 01d0a20b..0dc56f92 100644 --- a/docs/SESSION-DRIFT-CONTROLS.md +++ b/docs/SESSION-DRIFT-CONTROLS.md @@ -166,7 +166,7 @@ reading the emitted decision — not by reading source alone. | Selfheal — primary auto-repair | user (4 of 5 dirs) | LIVE | | Selfheal — hijack warning | user (4 of 5 dirs) | **LIVE and currently mis-firing** (§3, G4) | | `session-context.ps1` banner | project | LIVE where the branch carries the file | -| Announce-on-join (`announce-session.ps1`) | user | **NEW** — the only **push** control; asks, cannot send, and every decision leaves a receipt | +| Announce-on-join (`announce-session.ps1`) | user | **MERGED, NOT INSTALLED — inert by accident, 2026-08-02.** The only **push** control; asks, cannot send, and every decision leaves a receipt. It has never run. See below | | Announce wiring reaches a real script | test | **NEW** — `tests/test_announce_wiring.py`; nothing asserted this for *any* hook before, which is how a wired-but-inert shim survived weeks | | Announce missing-script notice | user | **NEW** — the one surface that still reports when the script itself fails to resolve | | Collision gate — unresolved notice | user | **NEW** — every fail-open path used to be indistinguishable from an all-clear; it now says which reason, once per 30 min, and still allows | @@ -180,6 +180,33 @@ Rule 4 being inert is **deliberate and announced** — the commit that landed it nothing changes until `install-gate.ps1` is re-run." It is listed as INERT here because a control that has never been installed is a source artefact, not an enforcement. +**Announce-on-join is inert too, and that one is an accident.** Measured 2026-08-02, hours after +[#133](https://github.com/MEFORORG/MessageFoundry/pull/133) merged it to `main`: + +| Check | Result | +|---|---| +| `mefor-announce` UserPromptSubmit entry in any of the 5 config roots | **absent** | +| The one UserPromptSubmit entry that *is* installed | `# mefor-web-announce`, resolving `scripts/hooks/announce.ps1` — **a different script in a different repo** | +| `/mefor-coord/announce/` | **does not exist**, so there is not one receipt: it has never executed | + +`install-coordination.ps1` was last run before the announce row existed, and merging a hook does not +install one. The two `mefor-coord` entries it wired then — the SessionStart banner and the collision +gate — are present, which is exactly why nothing looked wrong. + +Two things this costs, both observed rather than predicted. A peer session announced itself **by hand** +on 2026-08-02 and reported the hook as unavailable because it was "on an unmerged branch"; it had +merged, so the correct diagnosis was never reached. And the missing-script notice listed below — the +surface built precisely so this class cannot hide — **cannot fire when the hook is not wired at all**, +because it lives inside the shim. That is this section's own lesson recurring one level up: the +detector was still downstream of the failure. Re-arm from a plain terminal, then confirm by receipt +rather than by reading the settings file: + +```powershell +pwsh -NoProfile -File scripts\coord\install-coordination.ps1 +# then, after one prompt in any session in this repo: +ls (Join-Path (git rev-parse --path-format=absolute --git-common-dir) 'mefor-coord/announce/receipts') +``` + --- ## 2. What it demonstrably buys From 81e11d84e5c30888e5e663278ef386876fbd0977 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sun, 2 Aug 2026 08:16:31 -0500 Subject: [PATCH 06/10] fix(coord): five defects this PR's own first pass introduced or left Found by an adversarial review of the preceding commits, then each one reproduced by execution before being touched. Two were regressions I had introduced; three were gaps. 1. THE CLAIM FILE'S EXISTENCE IS THE LOCK, and the refresh unlinked it. `Move-Item -Force` is delete-then-rename. The take path is an exclusive CreateNew, so any instant the name does not exist is an instant another worktree can claim a key we hold -- i.e. the note refresh could hand a claim away. Measured on this box: 400 moves left the destination absent on 2,559 of 154,506 polls. [IO.File]::Move with overwrite is MoveFileEx(MOVEFILE_REPLACE_EXISTING), and the same harness never once saw the name missing across 134,581 polls. It fails transiently instead (13.5% under back-to-back churn, nothing like one refresh per run), so it retries five times and then reports; failing is the safe direction -- the old note survives and the claim stays ours. The catch around it is deliberately UNTYPED: PowerShell wraps a .NET method's exception in a MethodInvocationException, so the typed catch I wrote first never matched, the failure escaped to ErrorActionPreference = Stop, and the temp file was orphaned in the claim registry. The orphaned-temp assertion is what caught it. 2. `overlap.ps1 -Json` emitted `[null]` for an empty map. Build-Map returns AutomationNull, which PARAMETER BINDING converts to a real $null at the call -- and `@($null).Count` is 1, so the zero-rows guard was dead in exactly the case it was added for and the whole-map query printed a phantom row. Strictly worse than the nothing it replaced. The -File path I had verified by hand was fine; the two call sites do not fail alike. 3. The unresolved-notice throttle was repo-wide. The stamp lives in the SHARED git-common-dir and production invokes the gate with no arguments, so the first session to hit a broken gate silenced it for every other session -- and those sessions read that silence as "checked, nobody is here", which is the precise defect the notice exists to remove. One session's diagnostic must never become another's false all-clear. Keyed per worktree now. 4. An empty payload or a literal `null` on stdin does not throw, so that was the one unreadable-input path still exiting silently. 5. A ghost session could outrank a live one. UNVERIFIED is the shape a crashed session's record takes once its pid is recycled; last-write-wins had no opinion about which record it kept for a directory, so a ghost could supply the id and branch reported for a worktree somebody is really sitting in. Fenced records now win, then sorted cwd. Each fix is pinned, and the two regressions were checked against the unfixed code: the phantom-row test sees `[null]`, and the claim test asserts the file name never disappears while a refresh is failing. --- scripts/coord/claim.ps1 | 45 ++++++++++++++++++++++++++-- scripts/coord/overlap.ps1 | 27 +++++++++++++---- scripts/hooks/collision_gate.ps1 | 21 +++++++++++-- tests/test_collision_gate.py | 34 +++++++++++++++++++++ tests/test_coord_claim_refresh.py | 38 ++++++++++++++++++++---- tests/test_coord_overlap_signals.py | 46 +++++++++++++++++++++++++++++ 6 files changed, 194 insertions(+), 17 deletions(-) diff --git a/scripts/coord/claim.ps1 b/scripts/coord/claim.ps1 index aa875d87..958aa235 100644 --- a/scripts/coord/claim.ps1 +++ b/scripts/coord/claim.ps1 @@ -178,7 +178,12 @@ try { # then -Take) drops the claim in between, re-opening the race the claim exists to close. if ($Note) { $updated = [ordered]@{ - key = [string]$info.Claim.key + # $Take, not the stored key: ConvertFrom-Json date-coerces any ISO-8601-SHAPED string, + # and a key is free text, so a key like "2026-08-01T00:00:00" would come back through + # [string] as "08/01/2026 00:00:00" -- a record naming a key nobody typed and -Release + # cannot be spelled to match. $Take is the caller's current spelling of the same key, + # and it folds to the same filename, which is the real identity. + key = $Take note = $Note # Refresh the branch too: a worktree can have switched branches since the claim was made, # and a claim naming a branch nobody is on is another confidently-wrong coordination fact. @@ -189,13 +194,47 @@ try { claimed = ConvertTo-Stamp $info.Claim.claimed refreshed = (Get-Date).ToString("o") } | ConvertTo-Json -Compress - # Write-then-rename, not a truncating in-place write. A torn claim file does not fail loudly: + # Write-then-replace, not a truncating in-place write. A torn claim file does not fail loudly: # scripts/hooks/claim_check.py swallows a parse error into "not claimed", which would disable # the enforced gate for that key -- so a crash mid-write must leave the OLD file intact. $tmp = "$file.$PID.tmp" # UTF8 WITHOUT a BOM, for that same reader: a BOM makes json.loads raise. [System.IO.File]::WriteAllBytes($tmp, [System.Text.Encoding]::UTF8.GetBytes($updated)) - Move-Item -LiteralPath $tmp -Destination $file -Force + + # [IO.File]::Move(.., overwrite) AND NOT `Move-Item -Force`. THE CLAIM FILE'S EXISTENCE *IS* + # THE LOCK -- the take path above is an exclusive CreateNew, so any instant in which the name + # does not exist is an instant another worktree can claim a key we hold. `Move-Item -Force` + # is delete-then-rename and opens exactly that window: measured on this box, 400 moves left + # the destination absent on 2,559 of 154,506 polls. The same harness over [IO.File]::Move + # with overwrite -- which is MoveFileEx(MOVEFILE_REPLACE_EXISTING), atomic on NTFS -- polled + # 134,581 times and never once saw the name missing. + # + # It can fail transiently instead (a scanner or an editor holding the destination without + # FILE_SHARE_DELETE); the same harness saw 13.5% under back-to-back churn, which is nothing + # like one refresh per invocation but is cheap to absorb. Failing is the SAFE direction: the + # old note survives and the claim stays ours. Losing the lock is not. + # + # The catch is deliberately UNTYPED. PowerShell wraps an exception thrown by a .NET METHOD in + # a MethodInvocationException, so `catch [System.IO.IOException]` around this call never + # matches -- the failure escapes to $ErrorActionPreference = "Stop", the cleanup below never + # runs, and the temp file is orphaned in the claim registry. (Written typed first; the + # orphaned-temp assertion is what caught it.) Every failure here has the same right answer + # anyway: leave the old note, keep the claim, say so. + $moved = $false + foreach ($attempt in 1..5) { + try { [System.IO.File]::Move($tmp, $file, $true); $moved = $true; break } + catch { Start-Sleep -Milliseconds (20 * $attempt) } + } + if (-not $moved) { + # Never orphan the temp: this directory is the claim registry, and a k.json..tmp + # nothing ever removes accumulates in it for the life of the repo. + Remove-Item -LiteralPath $tmp -Force -ErrorAction SilentlyContinue + Write-Host "" + Write-Host "Could NOT refresh the note on '$Take' -- the file was locked by another process." -ForegroundColor Yellow + Write-Host " Your claim is UNCHANGED and still yours; only the note was not updated." + Write-Host " Retry in a moment. Do NOT -Release: that would drop a claim you still hold." + exit 1 + } Write-Host "" Write-Host "REFRESHED '$Take' (held since $($info.Claim.claimed))." -ForegroundColor Green Write-Host " note : $Note" diff --git a/scripts/coord/overlap.ps1 b/scripts/coord/overlap.ps1 index 9dc58300..74252b06 100644 --- a/scripts/coord/overlap.ps1 +++ b/scripts/coord/overlap.ps1 @@ -69,8 +69,14 @@ function ConvertTo-Norm([string]$p) { } # One emitter for every -Json exit, so no path can print nothing. See the note at the -File query below. +# +# The null filter is load-bearing, not defensive tidying. `Build-Map` returning no rows yields +# AutomationNull, which PARAMETER BINDING converts to a real $null on the way in -- and `@($null).Count` +# is 1, so the zero-rows guard was dead in exactly the case it was written for and the whole-map query +# emitted `[null]`: a phantom row, strictly worse than the nothing it replaced. Caught by adversarial +# review after the -File path had been verified by hand; the two call sites do not fail alike. function Write-JsonArray($Rows) { - $r = @($Rows) + $r = @($Rows | Where-Object { $null -ne $_ }) if ($r.Count -eq 0) { Write-Output "[]"; return } Write-Output ($r | ConvertTo-Json -Depth 6 -AsArray) } @@ -131,7 +137,13 @@ function Build-Map { # we report a worktree as dormant when its owner is actually around, and dormant still shows. if ($l.State -ne "LIVE" -and $l.State -ne "UNVERIFIED") { continue } $k = ConvertTo-Norm $e.Record.cwd - $sessionsByCwd[$k] = $e.Record + # A FENCED session outranks an unfenceable one for the same directory. UNVERIFIED is the shape a + # crashed session's record takes once its pid is recycled -- alive, but nothing proves it is the + # same run -- so letting one displace a genuinely LIVE record would report a ghost's id and branch + # for a worktree somebody is really sitting in. Last-write-wins had no opinion about which it kept. + $rank = if ($l.State -eq "LIVE") { 0 } else { 1 } + if ($sessionsByCwd.ContainsKey($k) -and $sessionsByCwd[$k].Rank -le $rank) { continue } + $sessionsByCwd[$k] = [pscustomobject]@{ Record = $e.Record; Rank = $rank } } $worktrees = @(Get-WorktreeList) @@ -156,9 +168,12 @@ function Build-Map { if ($null -eq $best -or $wn.Length -gt $best.Length) { $best = $wn } } } - # Two sessions inside one worktree: first by sorted cwd, so the answer is at least the SAME - # answer every run. Reporting one of them is correct -- the row says the worktree is live. - if ($best -and -not $ownerByWorktree.ContainsKey($best)) { $ownerByWorktree[$best] = $sessionsByCwd[$k] } + # Two sessions inside one worktree: the fenced one wins, then first by sorted cwd -- so the + # answer is the same every run, and a ghost never displaces a session that is really there. + if ($best) { + $cur = $ownerByWorktree[$best] + if ($null -eq $cur -or $sessionsByCwd[$k].Rank -lt $cur.Rank) { $ownerByWorktree[$best] = $sessionsByCwd[$k] } + } } $rows = @() @@ -205,7 +220,7 @@ function Build-Map { # A session sitting anywhere INSIDE the worktree owns it, not just one whose cwd is the root -- # resolved above, against every worktree at once, because "inside" is ambiguous when they nest. - $sess = $ownerByWorktree[$norm] + $sess = if ($ownerByWorktree.ContainsKey($norm)) { $ownerByWorktree[$norm].Record } else { $null } if ($files.Count -eq 0 -and -not $sess) { continue } $rows += [pscustomobject]@{ diff --git a/scripts/hooks/collision_gate.ps1 b/scripts/hooks/collision_gate.ps1 index 7fd64a91..1582ddf1 100644 --- a/scripts/hooks/collision_gate.ps1 +++ b/scripts/hooks/collision_gate.ps1 @@ -84,7 +84,18 @@ function Write-Unresolved([string]$Slug, [string]$Detail) { if ($LASTEXITCODE -eq 0 -and $common) { $dir = Join-Path ([string]$common).Trim() "mefor-coord" } } if ($dir) { - $stamp = Join-Path $dir "gate-unresolved/$Slug.stamp" + # PER WORKTREE, not per repo. The stamp lives in the SHARED git-common-dir, so a single + # repo-wide stamp would mean the first session to hit a broken gate silences it for every + # other session for the whole cooldown -- and those sessions would read that silence as + # "checked, nobody is here", which is precisely the defect this notice exists to remove. + # One session's diagnostic must never become another session's false all-clear. + $who = "shared" + $top = (& git rev-parse --path-format=absolute --show-toplevel 2>$null) + if ($LASTEXITCODE -eq 0 -and $top) { + $who = (Split-Path ([string]$top).Trim() -Leaf) -replace '[^A-Za-z0-9._-]+', '-' + if (-not $who) { $who = "shared" } + } + $stamp = Join-Path $dir "gate-unresolved/$who.$Slug.stamp" $prev = Get-Item -LiteralPath $stamp -ErrorAction SilentlyContinue if ($prev) { $mins = ((Get-Date) - $prev.LastWriteTime).TotalMinutes @@ -121,7 +132,13 @@ if (-not $target) { Write-Unresolved "payload-unreadable" ` "the PreToolUse payload on stdin was not readable JSON, so the gate never learned which file this edit targets" } - if (-not $hook) { exit 0 } + # Parsed, but to nothing. An empty payload or a literal `null` on stdin does not throw, so this + # used to be the one unreadable-input path that still exited silently -- the gate had learned no + # more than in the case above, and said no more than an all-clear. + if (-not $hook) { + Write-Unresolved "payload-empty" ` + "stdin carried no PreToolUse payload, so the gate never learned which file this edit targets" + } $target = [string]$hook.tool_input.file_path # NotebookEdit and some variants name the path differently; absence just means nothing to check. if (-not $target) { $target = [string]$hook.tool_input.notebook_path } diff --git a/tests/test_collision_gate.py b/tests/test_collision_gate.py index 88371b14..cc3c6b55 100644 --- a/tests/test_collision_gate.py +++ b/tests/test_collision_gate.py @@ -58,6 +58,7 @@ def run_gate( file_path: str | None = "a.py", state_dir: Path | None = None, raw_input: str | None = None, + cwd: Path | None = None, ) -> dict[str, Any] | None: """Invoke the gate exactly as Claude Code does. Returns the emitted object, or None for silence. @@ -80,6 +81,7 @@ def run_gate( text=True, timeout=180, check=False, + cwd=None if cwd is None else str(cwd), ) # A hook must never crash the tool call: a non-zero exit is ignored by the harness, which would # leave the gate looking installed while permitting everything. @@ -274,6 +276,38 @@ def test_a_different_reason_is_not_suppressed_by_the_first(tmp_path: Path) -> No assert "overlap-unparseable" in unresolved(run_gate(junk, state_dir=state)) +def test_the_throttle_does_not_silence_a_different_worktree(tmp_path: Path) -> None: + """ONE SESSION'S DIAGNOSTIC MUST NOT BECOME ANOTHER SESSION'S FALSE ALL-CLEAR. + + The stamp lives in the SHARED git-common-dir. Keyed per repo, the first session to hit a broken + gate would silence it for every other session for the whole cooldown -- and those sessions read + silence as "checked, nobody is here", which is the exact defect the notice exists to remove. + """ + state = tmp_path / "state" + missing = tmp_path / "does-not-exist.ps1" + a, b = tmp_path / "wt-a", tmp_path / "wt-b" + for wt in (a, b): + wt.mkdir() + subprocess.run(["git", "init", "-q", str(wt)], check=True, capture_output=True) + + assert run_gate(missing, state_dir=state, cwd=a) is not None, "first worktree must be told" + assert run_gate(missing, state_dir=state, cwd=b) is not None, "so must the second" + assert run_gate(missing, state_dir=state, cwd=a) is None, "but not the same one twice" + + +def test_says_so_when_the_payload_is_empty(tmp_path: Path) -> None: + """Empty stdin and a literal `null` do not raise, so this was the one unreadable-input path that + still exited silently -- having learned no more than the case above, and said no less than an + all-clear.""" + for payload in ("", "null"): + got = run_gate( + make_overlap_stub(tmp_path, [LIVE_ROW]), + state_dir=tmp_path / f"state-{len(payload)}", + raw_input=payload, + ) + assert "payload-empty" in unresolved(got), f"silent on {payload!r}" + + def test_an_unwritable_throttle_reports_anyway(tmp_path: Path) -> None: """The noise-suppressor must fail toward NOISE. diff --git a/tests/test_coord_claim_refresh.py b/tests/test_coord_claim_refresh.py index fe0a9383..49ef8a1f 100644 --- a/tests/test_coord_claim_refresh.py +++ b/tests/test_coord_claim_refresh.py @@ -146,19 +146,45 @@ def test_a_peers_claim_is_still_refused(repo: Path, tmp_path: Path) -> None: ) -def test_a_refresh_that_cannot_be_written_leaves_the_old_claim_intact(repo: Path) -> None: - """Write-then-rename, not truncate-in-place. +def test_a_key_that_looks_like_a_date_survives_a_refresh(repo: Path) -> None: + """A key is free text, and ConvertFrom-Json date-coerces anything ISO-8601 shaped. + + Round-tripping the stored key through [string] would rewrite "2026-08-01T00:00:00" as + "08/01/2026 00:00:00" -- a record naming a key nobody typed, which -List reports and -Release + cannot be spelled to match. + """ + key = "2026-08-01T00:00:00" + assert claim(repo, "-Take", key, "-Note", "first").returncode == 0 + assert claim(repo, "-Take", key, "-Note", "second").returncode == 0 + assert read_claim(repo, "2026-08-01t00-00-00")["key"] == key - A claim file caught mid-write does not fail loudly: ``claim_check.py`` swallows a parse error into - "not claimed" and stops enforcing that key. So a crash during a refresh must leave the OLD note -- - a stale claim is a bad note, a torn one is a disabled gate. + +def test_a_refresh_that_cannot_be_written_leaves_the_old_claim_intact(repo: Path) -> None: + """Write-then-REPLACE, not truncate-in-place and not delete-then-rename. + + Two properties, and the second is the one that bites hardest: + + * A claim file caught mid-write does not fail loudly -- ``claim_check.py`` swallows a parse error + into "not claimed" and stops enforcing that key. A crash during a refresh must leave the OLD note. + * **The claim file's existence IS the lock.** The take path is an exclusive ``CreateNew``, so any + instant the name does not exist is an instant another worktree can claim a key we hold. + ``Move-Item -Force`` is delete-then-rename: measured on this box, 400 moves left the destination + absent on 2,559 of 154,506 polls. ``[IO.File]::Move(.., overwrite)`` is MoveFileEx and never + unlinked the name across 134,581 polls -- it fails transiently instead, which is the safe + direction: the old note survives and the claim stays ours. """ assert claim(repo, "-Take", "k", "-Note", "first").returncode == 0 path = claim_file(repo, "k") - # Hold the file open for exclusive write: the rename cannot land, and the old bytes must survive. + # Hold the file open without sharing Delete: the replace cannot land, and the old bytes must survive. with open(path, "r+b") as held: held.read() proc = claim(repo, "-Take", "k", "-Note", "second") + # The NAME must never have gone away, even while the refresh was failing. + assert path.exists(), ( + "the claim file was unlinked -- a peer could take the key in that window" + ) assert proc.returncode != 0 or read_claim(repo, "k")["note"] in {"first", "second"} # Whatever happened, the file is still parseable -- that is the property the gate depends on. assert read_claim(repo, "k")["key"] == "k" + # And nothing was orphaned: this directory is the claim registry, not a scratch space. + assert not list(path.parent.glob("*.tmp")), "a failed refresh left a temp file behind" diff --git a/tests/test_coord_overlap_signals.py b/tests/test_coord_overlap_signals.py index 490e0cfe..158b7409 100644 --- a/tests/test_coord_overlap_signals.py +++ b/tests/test_coord_overlap_signals.py @@ -192,6 +192,52 @@ def raw_query(primary: Path, tmp_path: Path, path: str) -> str: return proc.stdout.strip() +def raw_survey(primary: Path, tmp_path: Path) -> str: + """The WHOLE-MAP -Json query, verbatim. A different call site from ``raw_query``, and it fails + differently: this one is fed ``Build-Map``'s return value, which is AutomationNull when empty.""" + proc = subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(OVERLAP), + "-Repo", + str(primary), + "-Json", + "-Refresh", + "-ConfigRoot", + str(tmp_path / "no-such-config"), + "-TasksDir", + str(tmp_path / "no-such-tasks"), + ], + capture_output=True, + text=True, + timeout=TIMEOUT, + check=False, + ) + assert proc.returncode == 0, f"overlap exited {proc.returncode}: {proc.stderr}" + return proc.stdout.strip() + + +def test_the_whole_map_json_query_emits_no_phantom_row( + peer_worktree: tuple[Path, Path], tmp_path: Path +) -> None: + """A ZERO-ROW ANSWER MUST BE ZERO ROWS, not one null one. + + ``Build-Map`` returns AutomationNull when it has no rows, and parameter binding converts that to a + real ``$null`` at the call -- where ``@($null).Count`` is 1, not 0. So a naive "if empty print []" + guard is dead in precisely the case it exists for, and the query emits ``[null]``: a phantom row, + strictly worse for any consumer that counts or indexes than the nothing it replaced. + """ + primary, _peer = ( + peer_worktree # peer worktree exists but is clean, and no session is registered + ) + out = raw_survey(primary, tmp_path) + assert out == "[]", f"a no-rows map must be an empty array, not {out!r}" + assert json.loads(out) == [] + + def test_a_json_query_always_emits_an_array( peer_worktree: tuple[Path, Path], tmp_path: Path ) -> None: From 38505f3abf66a15cc96c6368d4d9abd80cb595f3 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sun, 2 Aug 2026 08:17:35 -0500 Subject: [PATCH 07/10] docs(worktrees): "is it live yet" has two answers, and they are different I broadcast a merged claim.ps1 improvement to seven sessions as something they could use immediately. A peer tried it, got the old behaviour, and measured why: claim.ps1 is invoked BY HAND from the session's own worktree, so it runs that worktree's copy, and their branch predated the change. The in-force check I had given them was for the hook-run path and returned 0 for them. Both halves of what I said were individually true. The combination was wrong, because there are two rules and I collapsed them into one: hook-run (collision_gate.ps1, and overlap.ps1 as its callee) -- the installed shim resolves the PRIMARY first, so it is live when the primary advances, whatever any branch contains hand-run (claim.ps1, overlap.ps1, presence.ps1) -- resolved from the session's OWN tree, so it is live when that branch has it, and the primary is irrelevant Tabulated, with the check spelled out per path. The point generalises past this PR: test the property where the script will actually run from, because a token that resolves in the primary says nothing about a hand-run script. Also surfaces `collision_gate.ps1 -PathOverride ` as the read-only "who holds this file right now" query. It is documented in-script only as a test affordance, and the peer above found it by reading the source after it answered a question nothing else would. Both points are theirs, not mine. --- docs/WORKTREES.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/docs/WORKTREES.md b/docs/WORKTREES.md index 939bd2f4..cf9be9c0 100644 --- a/docs/WORKTREES.md +++ b/docs/WORKTREES.md @@ -321,6 +321,35 @@ hooks are wired at **user** level by [../scripts/coord/install-coordination.ps1](../scripts/coord/install-coordination.ps1): git cannot deliver a project-level hook to a worktree. +**A change to these scripts reaches a session by one of TWO rules, and they are not the same rule.** +Getting this wrong produces a confident, wrong answer to "can I use it yet", so state which one applies: + +| How the script is reached | When your change reaches a session | +|---|---| +| **Run by a hook** (`collision_gate.ps1`, and `overlap.ps1` as its callee) | The installed shim resolves the **primary checkout** first, falling back to the calling worktree. So: **when the primary advances** — regardless of what any session's own branch contains | +| **Run by hand from a worktree** (`claim.ps1`, `overlap.ps1`, `presence.ps1`) | From that session's **own tree**. So: **when that session's branch contains it** — the primary is irrelevant | + +Reported 2026-08-02 by a peer session that was told a merged `claim.ps1` improvement was available to +it, tried it, and got the old behaviour: its branch predated the change. Both halves of the answer were +individually true and the combination was wrong. Test the property, not the provenance, and test it +where the script will actually run from: + +```powershell +grep -c /scripts/coord/overlap.ps1 # hook-run scripts +grep -c ./scripts/coord/claim.ps1 # hand-run scripts +``` + +**Asking who holds a file, right now, without changing anything.** The collision gate answers this +directly, and it is the only command that does — `-PathOverride` makes it skip stdin and print its +decision: + +```powershell +pwsh -NoProfile -File scripts\hooks\collision_gate.ps1 -PathOverride docs\BACKLOG.md +``` + +Empty output means no live session holds it. Documented in-script as a test affordance; surfaced here +because a session that needed the answer found it by reading the source. + ## Announcing yourself (UserPromptSubmit hook) **What it fixes.** Everything above is **pull**-based: a new session discovers its peers and the peers From bc0f6845251c78e465e15d15ff030f7209cc8536 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sun, 2 Aug 2026 08:28:38 -0500 Subject: [PATCH 08/10] docs(worktrees): the freeze bullet had the right lesson and the wrong reason MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Routed here by the ADR 0154 session because I was the one live in this file. I had already corrected the false half -- "#119 never merged" -- but only to "it merged the following day", and their framing is better, so this takes theirs. The failure was never that the condition could not arrive. #119 merged (2026-08-02 01:45:00Z, 002be182). It is that THE WORLD MOVED WHILE EVERYONE WAITED: main advanced four times first -- #74 20:27:03Z, #120 23:59:43Z, #131 00:35:29Z, #130 01:01:35Z. So the freeze did not hold main still even while nominally in force. It held only the sessions honouring it, which is the worst of both, and it is a sharper argument for the same bullet without resting on a false fact. Every timestamp re-verified against the API here rather than restated; the measurements are theirs. The same framing was independently corrected in ci.yml (07b6e55a) and in BACKLOG #340, making this the third document to carry it and the last one live. Also names what the bullet had become: a compensating control resting on a false premise, which is the failure CLAUDE.md §11 lists -- occurring inside the document that argues for the rule. That is worth one sentence, because the next stale premise will look just as settled as this one did. --- docs/WORKTREES.md | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/docs/WORKTREES.md b/docs/WORKTREES.md index cf9be9c0..8e1e15ea 100644 --- a/docs/WORKTREES.md +++ b/docs/WORKTREES.md @@ -428,12 +428,28 @@ it by hand. Three constraints came out of that, recorded here so the next attemp them: - **A broadcast needs an expiry or a predicate the *recipient* can evaluate — never a promise from the - sender.** A merge freeze went out with "lift when #119 merges". #119 did not merge that day (an - unrelated CI timeout), so five sessions held on a condition that had not arrived and it took a second - round to retract. It merged the *following* day, 2026-08-02 01:45Z — which sharpens the point rather - than softening it: the recipients had no way to evaluate the predicate, so the freeze outlived its - own condition in both directions. As of 2026-08-02 a claim note still announcing that freeze was - being read by every joining session hours after both it and #133 had merged. + sender.** A merge freeze went out with "lift when #119 merges", and five sessions held. **#119 + merged** — 2026-08-02 01:45:00Z, merge commit `002be182`. The failure was never that the condition + could not arrive; it was that **the world moved while everyone waited**. `main` advanced four times + before it did: + + | | | + |---|---| + | #74 | 2026-08-01 20:27:03Z | + | #120 | 2026-08-01 23:59:43Z | + | #131 | 2026-08-02 00:35:29Z | + | #130 | 2026-08-02 01:01:35Z | + + So the freeze did not hold `main` still even while it was nominally in force — it held only the + sessions honouring it, which is the worst of both. And it outlived its own condition in the other + direction too: on 2026-08-02 a claim note still announcing the freeze was read by every joining + session hours after #119 had merged. A predicate the recipient cannot evaluate does not expire. + + This bullet said "#119 never merged" for a day, which made it a compensating control resting on a + false premise — the failure [`CLAUDE.md`](../CLAUDE.md) §11 names, inside the document arguing for + it. Corrected against the API, and the same framing was independently corrected in `ci.yml` + (`07b6e55a`) and in BACKLOG #340 by two other sessions; timestamps above are theirs, re-verified + here rather than restated. - **"Don't do X" is the wrong primitive when automation already has X armed.** The freeze asked sessions not to merge, while six PRs had auto-merge *armed* and would have landed with nobody clicking anything. The correct ask was an action — "disarm auto-merge" — not restraint. From de868f59bd2975f89b839f6d43eba28a04ffa15d Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sun, 2 Aug 2026 08:33:12 -0500 Subject: [PATCH 09/10] docs(worktrees): put the two numbers behind the freeze bullet, with their sources I omitted both for want of a source; the ADR 0154 session found both and I re-ran each before taking it. 12h15m #119's auto-merge armed 2026-08-01 13:29:37Z, merged 01:45:00Z. The timeline event is `auto_squash_enabled` -- a filter on `auto_merge_enabled` returns nothing, which is why the wait looked unmeasurable. Recorded in the doc, since the next person to look will reach for the wrong event name too. 8m26s the claim declaring the freeze is stamped 2026-08-01 23:51:17Z; #120 merged 23:59:43Z. The second is hedged in the doc, and their caveat was the right one: `claimed` records when the KEY was taken, not when the NOTE was written. What tightens it is that `refreshed` is ABSENT on that claim -- and on the code of the day there was no way to edit a note in place at all, so the two coincide unless someone hand-edited the JSON. Stated as "the claim was taken at", which is what the argument needs and no more. That claim is still on the board, still announcing the freeze, which is why it is cited in the present tense. --- docs/WORKTREES.md | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/docs/WORKTREES.md b/docs/WORKTREES.md index 8e1e15ea..e0f8d872 100644 --- a/docs/WORKTREES.md +++ b/docs/WORKTREES.md @@ -429,9 +429,11 @@ them: - **A broadcast needs an expiry or a predicate the *recipient* can evaluate — never a promise from the sender.** A merge freeze went out with "lift when #119 merges", and five sessions held. **#119 - merged** — 2026-08-02 01:45:00Z, merge commit `002be182`. The failure was never that the condition - could not arrive; it was that **the world moved while everyone waited**. `main` advanced four times - before it did: + merged** — 2026-08-02 01:45:00Z, merge commit `002be182`, **12h15m** after its auto-merge was armed + (`auto_squash_enabled` 2026-08-01 13:29:37Z, from the issue timeline — note the event is + `auto_squash_enabled`, so a filter on `auto_merge_enabled` finds nothing and the wait looks + unmeasurable). The failure was never that the condition could not arrive; it was that **the world + moved while everyone waited**. `main` advanced four times before it did: | | | |---|---| @@ -440,9 +442,16 @@ them: | #131 | 2026-08-02 00:35:29Z | | #130 | 2026-08-02 01:01:35Z | + The first of those landed **8m26s** after the claim declaring the freeze was taken (that claim is + stamped 2026-08-01 23:51:17Z and is *still on the board*; #120 merged 23:59:43Z). Stated as the claim + timestamp rather than "when the freeze was written" deliberately — `claimed` records when the key was + taken, and only a `refreshed` stamp would evidence when the note itself was last edited. That field + is absent here, which on the code of the day is consistent: there was no way to edit a note in place, + so the two coincide unless someone hand-edited the JSON. + So the freeze did not hold `main` still even while it was nominally in force — it held only the sessions honouring it, which is the worst of both. And it outlived its own condition in the other - direction too: on 2026-08-02 a claim note still announcing the freeze was read by every joining + direction too: on 2026-08-02 that same claim note was still announcing the freeze to every joining session hours after #119 had merged. A predicate the recipient cannot evaluate does not expire. This bullet said "#119 never merged" for a day, which made it a compensating control resting on a From 8555fb47f66f7fdc1850f7be4129212c5ab8aae0 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sun, 2 Aug 2026 08:36:31 -0500 Subject: [PATCH 10/10] docs(ledger): the CI backstop does not re-check ownership, and said it did MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found while unblocking another session that could not commit a rescued ADR: its number is allocated to a worktree that is not theirs. LEDGER-GATE.md §3 said "CI re-runs the same rules with --ci", and Limits said the --ci leg "is the backstop, and it cannot be bypassed from a branch". Both are true of every rule except the one a reader is most likely to be relying on. ledger_check.py:196 and :241 are each guarded by `not self.ci`, so "was this number allocated to you" runs LOCALLY AND NEVER IN CI. It has to be that way, and the reason is worth keeping: owns() reads the allocation store from /mefor-coord/alloc, and a CI runner clones fresh with no store, so the check would return False for every ADR and no ADR could ever merge. This is not a bug to fix. It is a limit that was documented as its own opposite. The consequence is now stated rather than left as an inference: a green CI on an ADR or BACKLOG PR is NOT evidence the number was allocated to anyone. And the residual is bounded in both directions -- after --no-verify a number belonging to another session's unmerged branch can be committed with nothing objecting, but the collision rule still blocks whichever of the two merges second. Late, loud and recoverable, rather than silent, which is the property the gate was actually built for. Same defect class as the freeze bullet corrected two commits ago, and as the collision gate this PR started with: a compensating control resting on a false premise -- CLAUDE.md §11 -- this time inside the document describing the control. --- docs/LEDGER-GATE.md | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/docs/LEDGER-GATE.md b/docs/LEDGER-GATE.md index ab1385ab..680054de 100644 --- a/docs/LEDGER-GATE.md +++ b/docs/LEDGER-GATE.md @@ -100,7 +100,22 @@ worse than no gate. `git commit --no-verify` bypasses the hook, and a branch cut from a **stale main** cannot see a collision at all — each branch is internally consistent, and the duplicate only exists once *both* have merged. So -CI re-runs the same rules with `--ci` against a freshly fetched `origin/main`. +CI re-runs the rules with `--ci` against a freshly fetched `origin/main`. + +**It re-runs all of them but one, and the exception is the ownership rule** — `ledger_check.py:196` and +`:241` both read `not self.ci and not self.owns(...)`, so *"was this number allocated to you"* is +**enforced locally and never in CI**. It has to be: `owns()` reads the allocation store from +`/mefor-coord/alloc`, a CI runner clones fresh and has none, so the check would return +False for every ADR and no ADR could ever merge. The consequence is worth stating plainly rather than +leaving as an inference — **a green CI on an ADR or BACKLOG PR is not evidence that the number was +allocated to anybody.** What CI *does* still catch, and what actually prevents the ledger corruption +this gate exists for, is collision-with-base, the missing index row, and duplicate rows. + +So the residual after `--no-verify` is narrower than "unprotected" and wider than "backstopped": a +number belonging to another session's *unmerged* branch can be taken and committed with nothing +objecting. The corruption surfaces when the second of the two merges — the collision rule blocks it — +which is late, loud, and recoverable, rather than silent. That is the property the gate was built for; +allocation discipline itself is on the honour system once the local hook is skipped. That step is **deliberately ungated** in `.github/workflows/ci.yml`. Every other step in the `test` job is conditioned on `code == 'true'`, which is **false for a docs-only PR** — and an ADR-only PR *is* docs-only. @@ -119,7 +134,8 @@ a pair. ## Limits - **`--no-verify` bypasses the pre-commit hook.** It is a guardrail, not a security boundary. The `--ci` - leg is the backstop, and it cannot be bypassed from a branch. + leg is the backstop for *collision*, and it cannot be bypassed from a branch — but it does **not** + re-check ownership (§3), so that one rule has no backstop at all. - **It does not stop two sessions building the same thing** under two different numbers. Duplicated work has no file conflict and no number conflict; nothing here sees it. - **Numbers leak.** An abandoned branch's number is never reclaimed. Accepted, deliberately.