From 0015ebfba1a6aa3080015a47314d88516135691d Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 6 Aug 2026 04:56:58 -0500 Subject: [PATCH 1/9] docs(coord): the session-id reuse loss is REPRODUCED, not a residual risk I said measure it rather than reason it away. Measured, against the shipped drain, and it reproduces: session X SessionStart (phantom) -> message DISPLAYED, marker minted, NOT consumed (inbox 1). Correct. session X' SessionStart, SAME id -> NOT displayed. Suppressed by the phantom's marker. session X' Stop -> NOT displayed, and CONSUMED (inbox 0). A message consumed by a session that never saw it. From the drain's side those three invocations are byte-identical to one healthy session showing mail at start and consuming it at the turn boundary, so nothing in this design can tell them apart: every artefact is keyed by session id, and the two sessions share one. ALL THREE PRECONDITIONS ARE OBSERVED, NOT ASSUMED. Session ids are reused across launches (one of the six ids in the phantom run had been seen hours earlier); phantoms mint markers; and a marker for a message still in the inbox is correctly NOT garbage-collected, so it survives exactly as long as the message it would suppress. PROPORTION, because this should not be read as the split having failed. It is strictly narrower than the defect the split closed -- that one lost mail unconditionally, to the first of six phantoms, on every launch. This needs a pending message, a phantom that displayed it, and a later session reusing that phantom's id before anyone else consumes it. But it is the same KIND of failure -- silent, receipt-clean, invisible to the operator -- and this channel's stated rule is that duplicate display is accepted while silent loss is not. THE PRINCIPLED FIX, RECORDED AND NOT MADE: consume only what the SAME drain invocation rendered. SessionStart would display without earning the right to consume; Stop would display again and consume what it just showed. That trades a guaranteed duplicate display for removing the cross-invocation trust relationship entirely, which is the direction the stated rule points. It is a design change, the owner is away, and #210 is armed -- so it is written down rather than taken. NOT BLOCKING #210. The branch remains a large net improvement over both the current state (no channel) and the original defect (unconditional loss), and the channel carries developer coordination notes, not anything critical. Docs only. backlog_status_check: OK, 331 items. --- docs/BACKLOG.md | 2 +- docs/SESSION-MAIL.md | 43 ++++++++++++++----- ...sync-session-mail-for-unreachable-peers.md | 10 ++++- 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index eca8d56d..8c0cd2b5 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -5014,7 +5014,7 @@ The comment immediately above says *"Scope is deliberately the posture the requi **SUPERSEDED: wiring was a separate owner decision, and it was taken.** The drain went live on `Stop` first, then on `SessionStart` once the show/consume split made a discarded session unable to consume what it displayed. Default root only; no `~/.claude-account-N` carries it. **It is nonetheless INERT outside the branch worktree** -- the shim resolves from the primary checkout, which does not carry these scripts until PR #210 lands, so the hook fires, resolves nothing and exits 0. It activates everywhere, unannounced, on merge. -**WHAT REMAINS, and it is why this item is still open.** (a) PR #210 must land. (b) The hook must then be verified through the SHIM resolution path, which is NOT what was tested -- the VS Code run drove the drain by absolute path, while the live hook resolves via `git rev-parse --git-common-dir` against the primary. (c) One residual risk is neither fixed nor accepted: **session ids are reused across launches** (measured), so a discarded session carrying the surviving session's id would mint an indistinguishable shown-marker and cause a silent loss that nothing in the design can detect. Measure it; do not reason it away. +**WHAT REMAINS, and it is why this item is still open.** (a) PR #210 must land. (b) The hook must then be verified through the SHIM resolution path, which is NOT what was tested -- the VS Code run drove the drain by absolute path, while the live hook resolves via `git rev-parse --git-common-dir` against the primary. (c) One residual defect is **CONFIRMED BY REPRODUCTION** (2026-08-06) and is neither fixed nor accepted: session ids are reused across launches, so a phantom that displayed and marked a message can have a LATER session reusing that id suppress the display and consume the message unseen. Measured end to end against the shipped drain. It is strictly narrower than the defect the show/consume split closed, but it is the same kind -- silent, receipt-clean, and undetectable by any artefact in the design, all of which are keyed by the shared session id. The principled fix is to consume only what the same drain invocation rendered; that is a design change and is not made here. **Related:** [ADR 0161](adr/0161-async-session-mail-for-unreachable-peers.md) (the decision, the measurements, and the ten EARS acceptance criteria), ADR 0158 (silent controls — a green signal that means nothing; findings 4, 5 and 8 are instances), #1018 (guards that go quiet). diff --git a/docs/SESSION-MAIL.md b/docs/SESSION-MAIL.md index 2fa9a6ae..d652fb85 100644 --- a/docs/SESSION-MAIL.md +++ b/docs/SESSION-MAIL.md @@ -378,17 +378,38 @@ the one to watch: that submitted prompts. The five phantoms fired `SessionStart` and nothing else. Previously the argument was weaker -- that they never became conversations, so presumably never took a turn -- and it now rests on observed teardown behaviour instead. -- **A phantom's session id must differ from the surviving session's. THIS ONE IS AT RISK, and the same - run is what put it there.** Session ids are **reused across launches**: one of the six carried an id - observed hours earlier in a previous run. It happened to be a phantom and the real session had a - different id, so the precondition held -- but it held by luck, not by construction. A phantom that - reused the *surviving* session's id would mint a marker indistinguishable from that session's own, - suppressing a display it never made and letting the next `Stop` consume the message unseen. Every - artefact here is keyed by session id, so nothing in this design can detect that case. - - It is not hypothetical-in-principle the way it was before this run: id reuse is now observed - behaviour. What is unobserved is the specific collision. **Do not close this by reasoning; measure - whether a phantom can ever carry the id of a session that survives.** +- **A phantom's session id must differ from the surviving session's. THIS PRECONDITION IS VIOLABLE, AND + THE LOSS IT ALLOWS HAS NOW BEEN REPRODUCED.** Not "at risk", not inferred -- measured 2026-08-06 + against the shipped drain: + + | step | outcome | + |---|---| + | session `X` `SessionStart` (the phantom) | message **displayed**, marker minted, **not consumed** -- correct | + | session `X'` reusing the same id, `SessionStart` | **not displayed** -- suppressed by the phantom's marker | + | session `X'` `Stop` | **not displayed**, and **consumed** | + + The message was consumed by a session that never saw it. From the drain's side those three + invocations are byte-identical to one healthy session showing mail at start and consuming it at the + turn boundary, so **nothing in this design can detect the difference** -- every artefact is keyed by + session id, and the two sessions share one. + + **All three preconditions are observed, not hypothesised.** Session ids are **reused across + launches** (one of the six in the phantom run carried an id seen hours earlier), phantoms mint + markers, and a marker for a message still in the inbox is correctly *not* garbage-collected -- so it + survives exactly as long as the message it would suppress. + + **What this does and does not cost.** It is strictly narrower than the defect the split closed: that + one lost mail to the *first* of six phantoms on every launch, unconditionally. This needs a pending + message, a phantom that displayed it, and a later session reusing that phantom's id before the + message is consumed by anyone else. But it is the same *kind* of failure -- silent, receipt-clean, + and invisible to the operator -- and the channel's stated rule is that duplicate display is accepted + while silent loss is not. + + **The principled fix is to stop trusting a marker across invocations: consume only what the SAME + drain invocation rendered.** `SessionStart` would then display without earning the right to consume, + and `Stop` would display again and consume what it just showed. That trades a guaranteed duplicate + display for the removal of the entire cross-invocation trust relationship, which is the direction the + stated rule points. It is a design change, not a patch, and it is **not** made here. **The marker is the per-session record of a display, and the receipt is not.** A receipt is named `.json` -- one slot per **message**, last writer wins -- so it can say *some* session was shown diff --git a/docs/adr/0161-async-session-mail-for-unreachable-peers.md b/docs/adr/0161-async-session-mail-for-unreachable-peers.md index fc0ed924..f6df4103 100644 --- a/docs/adr/0161-async-session-mail-for-unreachable-peers.md +++ b/docs/adr/0161-async-session-mail-for-unreachable-peers.md @@ -362,8 +362,14 @@ Two results, and the second is the one that matters: six events the message was displayed, held, not re-displayed to the session that had already seen it, and consumed exactly once at the surviving session's `Stop`. The residual risk is stated in [SESSION-MAIL.md](../SESSION-MAIL.md): because ids are reused, a phantom carrying the *surviving* - session's id would be indistinguishable by construction. That specific collision is unobserved and - must be measured, not reasoned away. + session's id would be indistinguishable by construction. **That collision has now been REPRODUCED** + (2026-08-06): a phantom displays and marks a message, a later session reusing that id has the + display suppressed, and its `Stop` consumes a message it never saw. Strictly narrower than the + defect the split closed -- which lost mail unconditionally, to the first of six phantoms, every + launch -- but the same kind of failure: silent, receipt-clean, undetectable by any artefact here, + since all of them are keyed by the session id the two sessions share. The principled fix is to + consume only what the SAME drain invocation rendered, trading a guaranteed duplicate display for + the removal of cross-invocation trust. Recorded, not made. ## To resolve on acceptance From cfb21944f6e6be0ee564c1a8f0ee9d18a7c2f11c Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 6 Aug 2026 04:59:47 -0500 Subject: [PATCH 2/9] docs(adr): close ADR 0161s remaining acceptance boxes, each against evidence All four verified rather than assumed, and the evidence is recorded in the box so a reader does not have to take the tick on faith. AC-5 NEGATIVE CONTROL -- two exist, both in tests/test_session_mail.py. test_the_naive_shared_destination_pattern_does_not_exclude runs the WRONG pattern inline and requires it to fail to exclude, so a green exclusion result is evidence only because the same harness is shown to detect the defect; test_the_claim_primitive_reports_a_failure_it_cannot_have_won re-runs the measurements own controls, so a Move-Claimed returning Won unconditionally is caught rather than read as a subtle bug. That box existed because a concurrency assertion with no proof it can fail is the ADR 0158 defect. D9 CAPS -- numbers live in mail-drain.ps1, anchors in SESSION-MAIL.md (CLAUDE.md at 40,102 bytes as the deliberate per-session cost; STEERING.md at 3,504 bytes as a whole document; announces existing peer-field caps). FRAME_OVERHEAD_BYTES exists BECAUSE of a measurement rather than as a guess: charging the raw body while the renderer added six bytes per line let a 34,539-byte injection pass an 8,000-byte cap while reporting 0 truncated. D10 CONTENT RULE -- stated once in SESSION-MAIL.md as a section heading, and referenced once from PHI.md. Checked the "one place each" clause rather than the string count: the only other occurrence in SESSION-MAIL.md is an internal link BACK to that section, which is the shape the box asks for rather than a second statement. My first grep for it returned zero because the phrase wraps across two lines and the match was line-scoped -- the same instrument-scope error as the rest of this work, in miniature. OWNER APPROVAL TO WIRE -- given in two steps: Stop first, then SessionStart once the show/consume split made a discarded session unable to consume what it displayed. Default config root only. The box now also carries the caveat that matters more than the approval: the rows are live and the hook still resolves nothing outside a worktree holding this branch. Zero unticked boxes remain. Docs only. --- ...sync-session-mail-for-unreachable-peers.md | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/docs/adr/0161-async-session-mail-for-unreachable-peers.md b/docs/adr/0161-async-session-mail-for-unreachable-peers.md index f6df4103..f4326f30 100644 --- a/docs/adr/0161-async-session-mail-for-unreachable-peers.md +++ b/docs/adr/0161-async-session-mail-for-unreachable-peers.md @@ -375,13 +375,26 @@ Two results, and the second is the one that matters: - [x] The eight findings are closed in the scripts, with the tests the Acceptance Criteria name. `tests/test_session_mail.py` exists and every `→` link above resolves to a function in it. -- [ ] AC-5's test carries a negative control that demonstrably goes red — a concurrency assertion +- [x] AC-5's test carries a negative control that demonstrably goes red — a concurrency assertion with no proof it can fail is the ADR 0158 defect, and this one in particular has a plausible wrong version that passes. -- [ ] The per-injection caps in D9 get concrete numbers, chosen against a measurement rather than - asserted. -- [ ] The content rule (D10) is stated in [SESSION-MAIL.md](../SESSION-MAIL.md) and referenced from - [PHI.md](../PHI.md), in one place each. + **Two controls, both in `tests/test_session_mail.py`:** + `test_the_naive_shared_destination_pattern_does_not_exclude` runs the WRONG pattern inline and + requires it to fail to exclude — so a green exclusion result is only evidence because the same + harness is shown to detect the defect; and + `test_the_claim_primitive_reports_a_failure_it_cannot_have_won` re-runs the measurement's own + controls, so a `Move-Claimed` that returned `Won` unconditionally is caught rather than read as + a subtle bug. +- [x] The per-injection caps in D9 get concrete numbers, chosen against a measurement rather than + asserted. Numbers in `mail-drain.ps1`; anchors in [SESSION-MAIL.md](../SESSION-MAIL.md) + (`CLAUDE.md` at 40,102 bytes as the deliberate per-session cost, `docs/STEERING.md` at 3,504 + bytes as a whole document, and announce's existing peer-field caps). `FRAME_OVERHEAD_BYTES` + exists **because** of a measurement: charging the raw body while the renderer added six bytes a + line let a 34,539-byte injection pass an 8,000-byte cap reporting `0 truncated`. +- [x] The content rule (D10) is stated in [SESSION-MAIL.md](../SESSION-MAIL.md) and referenced from + [PHI.md](../PHI.md), in one place each. Stated once as a section heading; PHI.md carries one + row pointing at it. The only other occurrence in SESSION-MAIL.md is an internal link back to + that section, which is the shape this box asks for rather than a second statement. - [x] Owner decision on whether the urgent `asyncRewake` tier is wired at all, given D11. **DECIDED 2026-08-06: NOT WIRED, and not rebuilt yet.** The default tier has never delivered mail in real use -- everything to date is rig-verified and the code is unmerged -- so building a @@ -389,5 +402,8 @@ Two results, and the second is the one that matters: path is recorded in [SESSION-MAIL.md](../SESSION-MAIL.md) so it is not rediscovered: arm on `UserPromptSubmit` rather than `SessionStart`. Revisit only if someone hits the latency in practice. -- [ ] Owner approval to wire the drain rows, which places a hook on `SessionStart` and `Stop` for - every session in this repo. +- [x] Owner approval to wire the drain rows, which places a hook on `SessionStart` and `Stop` for + every session in this repo. **Given 2026-08-05/06, in two steps:** `Stop` first, then + `SessionStart` once the show/consume split made a discarded session unable to consume what it + displayed. Default config root only. ⚠️ The rows are live and the hook still resolves nothing + outside a worktree holding this branch — see §"Status and what gates wiring". From f28a83b40f2912aee35ff5e9165e71cdb21fbab8 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 6 Aug 2026 05:03:53 -0500 Subject: [PATCH 3/9] fix(coord): the installer filters silently selected nothing, and said so with exit 0 Two defects in one path, both found by using the tool rather than reading it, and both the same shape as everything else this branch has been about: a clean result from an operation that did not happen. 1. `pwsh -File` HANDS EVERY ARGUMENT OVER AS A STRING, so the documented invocation could not express a multi-value filter. `-Only PreToolUse,UserPromptSubmit` arrived as ONE element, the literal string "PreToolUse,UserPromptSubmit"; `-contains` matched no event; zero rows were selected. The .EXAMPLE block uses the -File form with single values, so it works there and fails the moment anyone passes two -- which is exactly what an operator wiring more than one tier will type. Identifier-shaped switches (-Only, -Except, -Script) are now split on commas. -SettingsPath deliberately is NOT: a Windows path may legally contain a comma, and quietly cutting one in half would be a worse bug than the one being fixed. 2. A FILTER THAT MATCHED NOTHING EXITED 0. That is the whole hazard, and it is worse than defect 1 because it hides it: the run printed "No wiring rows selected" and returned success, so an operator who mistyped an event, or hit defect 1, saw a clean exit and believed the roots were wired. I hit this myself while wiring three config roots and only caught it because I dry-ran first -- had I not, I would have reported three roots wired while nothing happened. It now exits 2, names what was asked for, and lists the events and scripts that actually exist, so the message corrects the operator instead of only refusing them. VERIFIED WITH A NEGATIVE CONTROL, because a fix to a silent-failure path is worth nothing unless the failure can be made to show: -Only PreToolUse,UserPromptSubmit -> both rows selected, exit 0 (was: nothing, exit 0) -Only NoSuchEvent -> NO WIRING ROWS MATCHED, exit 2, known events listed 58 pass across the three installer test modules. The two failures in test_installed_coord_hooks.py are inherited and unrelated: that file is byte-identical to origin/main on this branch, I have never touched it, and CI does not report them. --- scripts/coord/install-coordination.ps1 | 40 +++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/scripts/coord/install-coordination.ps1 b/scripts/coord/install-coordination.ps1 index b7db4ff5..7effd42a 100644 --- a/scripts/coord/install-coordination.ps1 +++ b/scripts/coord/install-coordination.ps1 @@ -204,6 +204,28 @@ $WIRING = @( @{ Event = "Stop"; Matcher = $null; Script = "scripts/hooks/mail-drain.ps1"; Timeout = 20; Msg = "Checking session mail"; Marker = $MAIL_MARKER; Shim = "std" } ) +# The unfiltered table, kept so a filter that matches nothing can tell the operator what DOES exist. +# Naming the real events and scripts turns a dead end into a correction; without it the message can +# only say that nothing matched, which is the same information the exit code already carries. +$WIRING_ALL = $WIRING + +# SPLIT ON COMMAS, BECAUSE `pwsh -File` HANDS EVERY ARGUMENT OVER AS A STRING. The documented +# invocation in .EXAMPLE is the -File form, and it binds a multi-value switch as ONE element: +# `-Only PreToolUse,UserPromptSubmit` arrives as the single string "PreToolUse,UserPromptSubmit", +# `-contains` then matches nothing, and the run selected zero rows while reporting success. Measured +# 2026-08-06 -- it printed "No wiring rows selected" and exited 0, which reads as "done". Splitting +# here makes the documented form mean what it looks like it means; passing a real array in-process +# still works, because splitting a single-element array on a character it does not contain is a no-op. +# Only the identifier-shaped switches are split. -SettingsPath is NOT: a Windows path may legally +# contain a comma, and silently cutting one in half would be a worse bug than the one being fixed. +function Split-Csv([string[]]$Values) { + if (-not $Values) { return @() } + return @($Values | ForEach-Object { $_ -split ',' } | ForEach-Object { $_.Trim() } | Where-Object { $_ }) +} +$Only = Split-Csv $Only +$Except = Split-Csv $Except +$Script = Split-Csv $Script + if ($Only) { $WIRING = @($WIRING | Where-Object { $Only -contains $_.Event }) } if ($Except) { $WIRING = @($WIRING | Where-Object { $Except -notcontains $_.Event }) } if ($Script) { @@ -212,7 +234,23 @@ if ($Script) { @($Script | Where-Object { $row.Script -like "*$_*" }).Count -gt 0 }) } -if (-not $WIRING) { Write-Host "No wiring rows selected."; exit 0 } +# NONZERO, NOT exit 0. A filter that matches nothing is a user error -- a misspelled event, a -Script +# that hits no row -- and exiting 0 made it indistinguishable from a successful install. That is the +# silent-no-op shape this script exists to report on rather than commit: the operator sees a clean +# exit and believes the roots were wired. Naming what was asked for is the difference between "there +# was nothing to do" and "you asked for something that does not exist". +if (-not $WIRING) { + $asked = @() + if ($Only) { $asked += "-Only $($Only -join ',')" } + if ($Except) { $asked += "-Except $($Except -join ',')" } + if ($Script) { $asked += "-Script $($Script -join ',')" } + Write-Host "" + Write-Host "NO WIRING ROWS MATCHED $($asked -join ' ') -- nothing was examined and nothing was written." -ForegroundColor Yellow + Write-Host " Known events : $((($WIRING_ALL | ForEach-Object { $_.Event }) | Sort-Object -Unique) -join ', ')" + Write-Host " Known scripts: $((($WIRING_ALL | ForEach-Object { $_.Script }) | Sort-Object -Unique) -join ', ')" + Write-Host "" + exit 2 +} function Read-Settings([string]$Path) { if (-not (Test-Path -LiteralPath $Path)) { return [ordered]@{} } From 663b96e75632d53bad518530c82e1af43008113a Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 6 Aug 2026 05:11:14 -0500 Subject: [PATCH 4/9] fix(coord): the broadcast path suppressed the very receipt that made its own claim true Two defects, one class, found by sweeping the CLI tools for the shape the installer fix had just exposed: an operation that reports a clean result for a question it could not answer. 1. mail.ps1 -Send -To all ASSERTED A DISTINCTION IT HAD DESTROYED. presence.ps1 deliberately writes an "UNAVAILABLE" receipt to STDERR precisely so that "the fence could not look" stops rendering as an indistinguishable empty list on stdout -- its own comment says so, and it is a good control. The broadcast path invoked it with `2>$null`, threw that receipt away, and then printed: "(This is 'the roster returned nobody', not 'the roster could not look')" It could not know that. The only channel carrying the difference had just been suppressed. A message asserting a distinction the code destroyed is a compensating control resting on a false premise, which CLAUDE.md section 11 forbids by name. Now stderr is captured rather than discarded, and the two cases are separated by EVIDENCE and exit differently: an empty roster is a real answer and exits 0; an unavailable roster prints the receipts, says plainly that this is NOT "nobody is live", and exits 1. A non-answer must not read as an answer. 2. AND THE CONTROL ITSELF HAD A HOLE, which is why the first defect was survivable-looking. presence.ps1 emits the UNAVAILABLE receipt for RepoFound=true/Available=false, but the RepoFound=FALSE path returned the same `[]` with NO receipt at all -- so one of the two ways of being unable to look stayed silent on exactly the channel a machine consumer reads. The fix was applied to one of two paths. It now covers both. VERIFIED WITH A NEGATIVE CONTROL, run outside a git repository: stdout : [] <- unchanged, still indistinguishable stderr : presence: roster UNAVAILABLE -- not inside a git repository ... stdout alone cannot separate the cases and is not meant to; the receipt is what separates them, and it now exists on both paths. 127 pass across the presence consumers, the coordination suite, session mail and the announce wiring. --- scripts/coord/mail.ps1 | 31 ++++++++++++++++++++++++++++--- scripts/coord/presence.ps1 | 12 +++++++++++- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/scripts/coord/mail.ps1 b/scripts/coord/mail.ps1 index 85551562..60dfbc43 100644 --- a/scripts/coord/mail.ps1 +++ b/scripts/coord/mail.ps1 @@ -309,9 +309,22 @@ if ($Send) { if ($To -ieq 'all') { # Broadcast: every live peer in this repo EXCEPT this one. Uses the shared roster so there is # exactly one notion of "who is here" -- see scripts/coord/presence.ps1. - $rosterJson = & pwsh -NoProfile -File (Join-Path $PSScriptRoot 'presence.ps1') -Json 2>$null + # CAPTURE STDERR, DO NOT DISCARD IT. presence.ps1 deliberately writes an "UNAVAILABLE" receipt + # to stderr precisely so that "the fence could not look" stops rendering as an + # indistinguishable empty list on stdout -- its own comment says so. This call used to end in + # `2>$null`, which threw that receipt away and then printed a line claiming the empty result + # meant "the roster returned nobody, not the roster could not look". It could not know that: + # the only channel carrying the difference had just been suppressed. A message asserting a + # distinction the code destroyed is a compensating control resting on a false premise. + $rosterErr = @() + $rosterJson = & pwsh -NoProfile -File (Join-Path $PSScriptRoot 'presence.ps1') -Json 2>&1 | + ForEach-Object { + if ($_ -is [System.Management.Automation.ErrorRecord]) { $rosterErr += [string]$_; } + else { $_ } + } + $rosterUnavailable = @($rosterErr | Where-Object { $_ -match 'UNAVAILABLE' }).Count -gt 0 $roster = @() - try { $roster = @($rosterJson | ConvertFrom-Json) } catch { $roster = @() } + try { $roster = @($rosterJson | ConvertFrom-Json) } catch { $roster = @(); $rosterUnavailable = $true } $me = (Get-Location).Path.TrimEnd('\', '/').ToLowerInvariant() foreach ($r in $roster) { if (-not $r.Worktree) { continue } @@ -320,8 +333,20 @@ if ($Send) { $targets += $r.Worktree } if ($targets.Count -eq 0) { + # THE TWO CASES ARE NOW SEPARATED BY EVIDENCE RATHER THAN BY ASSERTION, and they exit + # differently: an empty roster is a real answer, an unavailable one is a non-answer and must + # not read as "nobody is here". + if ($rosterUnavailable) { + Write-Host "" + Write-Host "ROSTER UNAVAILABLE -- nothing was broadcast, and this is NOT 'nobody is live'." -ForegroundColor Yellow + Write-Host " presence.ps1 could not measure who is here, so an empty peer list proves nothing." + foreach ($e in $rosterErr) { Write-Host " $e" -ForegroundColor Yellow } + Write-Host " Run scripts\coord\presence.ps1 to see why, then re-send." + Write-Host "" + exit 1 + } Write-Host "No live peers to broadcast to. Nothing written." - Write-Host " (This is 'the roster returned nobody', not 'the roster could not look' -- see presence.ps1 -Json.)" + Write-Host " (The roster WAS readable and returned nobody -- presence.ps1 emitted no UNAVAILABLE receipt.)" exit 0 } } diff --git a/scripts/coord/presence.ps1 b/scripts/coord/presence.ps1 index 565e98ec..a585c5ea 100644 --- a/scripts/coord/presence.ps1 +++ b/scripts/coord/presence.ps1 @@ -118,7 +118,17 @@ function Get-SelfPids([int]$Override) { # --- Collect ------------------------------------------------------------------------------------ $occ = Get-WorktreeOccupancy -Repo $Repo -ConfigRoot $ConfigRoot -StartSkewMinutes $StartSkewMinutes if (-not $occ.RepoFound) { - if ($Json) { "[]" | Write-Output } else { Write-Host "Not inside a git repository -- nothing to scope presence to." } + # THE STDERR RECEIPT BELONGS HERE TOO, and its absence was a hole in the control this script + # already documents. The -Json block further down emits an UNAVAILABLE receipt so that "the fence + # could not look" stops rendering as an indistinguishable `[]` -- but THIS path returned the same + # empty array with no receipt at all, so one of the two ways of being unable to look stayed + # silent on exactly the channel a machine consumer reads. A caller that correctly checks stderr + # for the receipt would still read "not a git repository" as "nobody is live". + if ($Json) { + [Console]::Error.WriteLine("presence: roster UNAVAILABLE -- not inside a git repository, so there is nothing to scope presence to. An empty list here is NOT 'nobody is live'.") + "[]" | Write-Output + } + else { Write-Host "Not inside a git repository -- nothing to scope presence to." } exit 0 } From 6af5dc7956031d11c95c11abec543b6db2161087 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 6 Aug 2026 05:16:27 -0500 Subject: [PATCH 5/9] test(coord): regressions for the two silent-no-op fixes, which shipped without any Both fixes in 9db2f4e1 and 41aec69d changed behaviour with nothing pinning it. This repo's standard is that new behaviour gets a test, and a silent-failure fix is exactly the kind that regresses unnoticed -- the failure it prevents looks like success, so nobody investigating a green run would find it. THREE TESTS, each asserting the post-fix behaviour directly, so each goes red against the code as it was: test_a_multi_value_filter_works_through_the_documented_file_invocation `-Only PreToolUse,UserPromptSubmit` through the documented `pwsh -File` form must select BOTH rows. Before the fix that argument bound as one string, matched no event, and selected zero -- so the test would find neither script. It also asserts mail-drain.ps1 is ABSENT, so this is a filter rather than a no-op that happens to print everything. test_a_filter_that_matches_nothing_fails_loudly_and_names_what_exists The negative control, and the more important half: a filter matching nothing must exit NONZERO, name what was asked for, and list the events that really exist. It used to exit 0, which made a misspelled event indistinguishable from a successful install. test_outside_a_repo_the_json_roster_carries_an_unavailable_receipt presence.ps1 -Json outside a repo must emit `[]` on stdout AND an UNAVAILABLE receipt on stderr. Asserting BOTH is the point: a test that only checked stderr would pass against a version that had broken the JSON contract every existing consumer depends on. Needed a new helper. run_installer asserts returncode == 0, so it structurally CANNOT observe a failure path -- an assertion that can only see one outcome is not a check. run_installer_rc returns the code instead. 28 pass across the two modules; ruff format and check clean. --- tests/test_announce_wiring.py | 59 +++++++++++++++++++++++++++++++++++ tests/test_coord_presence.py | 31 ++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/tests/test_announce_wiring.py b/tests/test_announce_wiring.py index 446d6584..57706637 100644 --- a/tests/test_announce_wiring.py +++ b/tests/test_announce_wiring.py @@ -298,6 +298,65 @@ def test_status_reports_the_announce_row(settings: Path) -> None: assert ANNOUNCE_REL in out +def run_installer_rc(settings: Path, *args: str) -> tuple[int, str]: + """Like run_installer, but RETURNS the exit code instead of asserting it is 0. + + run_installer asserts success, so it structurally cannot test a failure path -- an assertion that + can only observe one outcome is not a check. These two tests are about the exit code itself. + """ + proc = subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(INSTALLER), + "-SettingsPath", + str(settings), + *args, + ], + capture_output=True, + text=True, + timeout=TIMEOUT, + check=False, + ) + return proc.returncode, proc.stdout + proc.stderr + + +def test_a_multi_value_filter_works_through_the_documented_file_invocation(settings: Path) -> None: + """`pwsh -File` hands every argument over as a STRING, so a comma list arrives as ONE element. + + `-Only PreToolUse,UserPromptSubmit` used to bind as the single string + "PreToolUse,UserPromptSubmit", match no event via -contains, and select zero rows -- while + printing "No wiring rows selected" and exiting 0. The .EXAMPLE block uses the -File form, so the + documented invocation worked for one value and silently did nothing for two, which is what an + operator wiring more than one tier at a time actually types. + """ + rc, out = run_installer_rc(settings, "-Only", "PreToolUse,UserPromptSubmit", "-WhatIf") + assert rc == 0, out + assert "collision_gate.ps1" in out, out + assert "announce-session.ps1" in out, out + # And the rows it did NOT ask for stay out, so this is a filter rather than a no-op that + # happens to print everything. + assert "mail-drain.ps1" not in out, out + + +def test_a_filter_that_matches_nothing_fails_loudly_and_names_what_exists(settings: Path) -> None: + """NEGATIVE CONTROL for the test above, and the more important half. + + A filter matching nothing exited 0, so a misspelled event was indistinguishable from a successful + install: the operator saw a clean exit and believed the roots were wired. It must fail, and it must + say what the real events are -- refusing without correcting leaves the caller exactly where they + started. + """ + rc, out = run_installer_rc(settings, "-Only", "NoSuchEvent", "-WhatIf") + assert rc != 0, f"a filter matching nothing exited 0: {out}" + assert "NO WIRING ROWS MATCHED" in out, out + assert "NoSuchEvent" in out, "the message must name what was asked for" + for known in ("SessionStart", "PreToolUse", "UserPromptSubmit", "Stop"): + assert known in out, f"the message must list the real events; {known} missing: {out}" + + def test_only_removes_announce_without_disarming_the_gate(settings: Path) -> None: """Without -Only, the 2am remedy for a misbehaving announce hook is a full -Uninstall that takes the collision gate and the SessionStart banner with it.""" diff --git a/tests/test_coord_presence.py b/tests/test_coord_presence.py index 746d39b0..1d865f8f 100644 --- a/tests/test_coord_presence.py +++ b/tests/test_coord_presence.py @@ -227,3 +227,34 @@ def _find_free_pid() -> int: proc.wait(timeout=30) time.sleep(0.3) # let the OS reap it before we claim the pid is gone return proc.pid + + +def test_outside_a_repo_the_json_roster_carries_an_unavailable_receipt(tmp_path: Path) -> None: + """ "I could not look" must not render as "nobody is live" on the machine-readable channel. + + presence.ps1 already emits an UNAVAILABLE receipt to STDERR for the RepoFound=true / + Available=false case, precisely so an empty stdout list stops being ambiguous -- its own comment + says so. The RepoFound=FALSE path returned the same ``[]`` with NO receipt at all, so one of the + two ways of being unable to look stayed silent on exactly the channel a consumer parses. The fix + had been applied to one of two paths. + + stdout is deliberately unchanged and still ``[]``: every existing consumer keeps working, and the + receipt is what separates the cases. Asserting BOTH is the point -- a test that only checked + stderr would pass against a version that had broken the JSON contract. + """ + outside = tmp_path / "not-a-repo" + outside.mkdir() + proc = subprocess.run( + ["pwsh", "-NoProfile", "-NonInteractive", "-File", str(PRESENCE), "-Json"], + cwd=str(outside), + capture_output=True, + text=True, + timeout=60, + check=False, + ) + assert proc.stdout.strip() == "[]", f"the JSON contract changed: {proc.stdout!r}" + assert "UNAVAILABLE" in proc.stderr, ( + "an unreadable roster emitted no receipt, so an empty list is indistinguishable from " + f"'nobody is live': stdout={proc.stdout!r} stderr={proc.stderr!r}" + ) + assert "not inside a git repository" in proc.stderr.lower(), proc.stderr From 939f111839db74b52d49babc938ad490e4450ee7 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 6 Aug 2026 05:53:07 -0500 Subject: [PATCH 6/9] docs(coord): merging to main did NOT make the drain live -- the shim resolves a working tree The shim verification is done and it found the thing it was created to find. My VS Code run drove the drain by ABSOLUTE PATH; the live hook resolves via git rev-parse against the PRIMARY CHECKOUT. Different code path, and it is the one that has been silently resolving nothing. MEASURED IMMEDIATELY AFTER PR #210 LANDED: scripts/hooks/mail-drain.ps1 on origin/main YES same file in the PRIMARY CHECKOUT working tree ABSENT primary HEAD 2ab20375 vs origin/main fdec72ca 3 commits behind THE SHIM RESOLVES A WORKING TREE, NOT A REF. Nothing pulls the primary automatically, so the channel is still inert across the entire machine after the merge. An earlier draft of this section asserted it "becomes live everywhere the moment the scripts land on main". That is now measured false, and false in the worst direction: the merge notification is exactly the moment an operator is most likely to believe it went live. AND THE TWO-VANTAGE PROBE IS WHY THIS WAS VISIBLE AT ALL: probed from a worktree holding this branch primary=False fallback=True -> resolves probed from any other worktree primary=False fallback=False -> NOTHING Checking from my own worktree reports True and looks healthy, because that branch carries the file. A single-vantage check cannot see this defect -- it is the "invisible from the only vantage point you run from" shape, and it would have produced a confident green. The activating action is `git pull` in the primary checkout, not the merge. I have NOT done it: the primary is shared by every worktree on this box and pulling it changes hook resolution for ~50 sessions at once, which is not a change to make unattended. Docs only. Wiring unchanged. --- docs/SESSION-MAIL.md | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/docs/SESSION-MAIL.md b/docs/SESSION-MAIL.md index d652fb85..b870c0d8 100644 --- a/docs/SESSION-MAIL.md +++ b/docs/SESSION-MAIL.md @@ -29,16 +29,29 @@ registered nowhere. > **THE ROWS ARE LIVE AND THE HOOK STILL DOES NOTHING, IN ALMOST EVERY WORKTREE.** The installed shim > resolves `scripts/hooks/mail-drain.ps1` from the **primary checkout first**, falling back to the -> session's own worktree. These scripts are not on `main` yet, so the primary does not carry them: -> measured, both bases probed, primary `False` and the branch worktree `True`. Every session outside a -> worktree holding this branch therefore fires the hook, resolves nothing, and exits 0 -- which is +> session's own worktree. Measured from two vantage points, which is the only way to see it: +> +> | probed from | primary base | own-worktree fallback | resolves | +> |---|---|---|---| +> | a worktree holding this branch | `False` | `True` | yes -- **but only because that branch carries the file** | +> | any other worktree | `False` | `False` | **nothing** | +> +> Every session outside a worktree holding this branch fires the hook, resolves nothing, and exits 0 -- > **byte-identical to a healthy hook with no mail**, the exact defect -> [observability rule 1](#three-observability-rules) exists to prevent. +> [observability rule 1](#three-observability-rules) exists to prevent. Probing from the branch worktree +> alone reports `True` and looks healthy, so **a single-vantage check cannot see this**. +> +> ⚠️ **MERGING TO `main` IS NOT WHAT MAKES IT LIVE, and an earlier draft of this section said it was.** +> Measured immediately after PR #210 landed: `mail-drain.ps1` was on `origin/main` and **still absent +> from the primary checkout's working tree**, which was three commits behind. The shim resolves a +> **working tree**, not a ref. Nothing pulls the primary automatically, so the channel stays inert +> across the whole machine until somebody does -- and the merge notification is exactly the moment an +> operator is most likely to believe it went live. > -> It becomes live everywhere the moment the scripts land on `main`, in every session started after -> that, with no further action and no announcement. **Re-test on the target surface at that point**; -> do not treat the wiring as verified because the rows are present. A row in a settings file is not a -> hook that fired, which is what the installer prints on every run. +> **The activating action is `git pull` in the primary checkout, not the merge.** Re-test on the target +> surface after that, from a worktree that does **not** carry this branch, so the fallback cannot mask a +> broken primary resolution. A row in a settings file is not a hook that fired, which is what the +> installer prints on every run. The show/consume split described in ["Showing is not consuming"](#showing-is-not-consuming) is what makes wiring `SessionStart` safe: a discarded session can display mail but cannot consume it. From c30916f1da81644019639902a9ceae184e63d2fe Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 6 Aug 2026 08:10:35 -0500 Subject: [PATCH 7/9] docs(coord): session mail is LIVE on all five roots and verified through the installed hook The two gated items are done, and the status section is corrected from INERT -- which it had been wrong about in both directions within one night. VERIFIED END TO END, THROUGH THE INSTALLED HOOK COMMAND rather than a convenient stand-in. The earlier VS Code run drove the drain by ABSOLUTE PATH, which does not exercise what actually runs. This one took the shim verbatim out of ~/.claude/settings.json: shim resolution, five worktrees on unrelated branches resolves in ALL, via the primary message queued with MAIN's mail.ps1 delivered by the installed command receipt written by the drain process shown-consumed / byHookEvent Stop box afterwards inbox 0, seen 1 -- consumed once WIRED ON ALL FIVE ROOTS, not just the default. The owner confirmed all four numbered roots are in active use, so a drain on ~/.claude alone would have left the cross-login case -- half the reason this channel exists -- unreachable. Each root took SessionStart and Stop via `-Only -Script mail-drain`, which is the invocation that does NOT also install the coordination banner sharing that event. The banner remains deliberately absent everywhere. AND THE BLOCKQUOTE BELOW IS NOW A LESSON RATHER THAN A STATE CLAIM. It asserted that the rows were live and the hook did nothing; that was true for several hours and is not true now. Leaving it in the present tense would have been the same defect the section is about -- a document asserting a condition that no longer holds, in a file whose entire subject is signals that mean nothing. Two things kept, both past tense and both expensive: - A SINGLE-VANTAGE CHECK CANNOT SEE IT. From a worktree carrying the branch the fallback returns True and everything looks healthy; from any other worktree it resolved nothing. Probe from a worktree that does NOT carry the branch. - MERGING TO main IS NOT WHAT MAKES A SHIM-RESOLVED HOOK LIVE. The shim resolves a WORKING TREE, not a ref. After #210 landed the file was on origin/main and still absent from the primary's tree, three commits behind. The activating action was `git pull` in the primary -- and the merge notification is exactly when an operator believes it went live. Docs only. --- docs/SESSION-MAIL.md | 74 +++++++++++++++++++++++++++----------------- 1 file changed, 46 insertions(+), 28 deletions(-) diff --git a/docs/SESSION-MAIL.md b/docs/SESSION-MAIL.md index b870c0d8..cb1cedf4 100644 --- a/docs/SESSION-MAIL.md +++ b/docs/SESSION-MAIL.md @@ -15,43 +15,61 @@ hook. --- -## Status: wired on `SessionStart` and `Stop`, on the default config root -- and INERT until the scripts reach `main` +## Status: LIVE on all five config roots, and verified end to end through the installed hook **Read the config, not this line, before relying on it** -- a status sentence in a document is exactly -the observation that goes stale without saying so. As last verified against `~/.claude/settings.json`: -the drain is registered on **both `SessionStart` and `Stop`**, on the default root only. No -`~/.claude-account-N` carries it. The coordination banner -(`scripts/worktree/session-context.ps1`) is a *different* hook that shares the `SessionStart` event and -is deliberately still absent; install one tier at a time with -`-Only -Script mail-drain`, because `-Only SessionStart` alone would wire the banner too. The -urgent tier ([`scripts/hooks/mail-watch.ps1`](../scripts/hooks/mail-watch.ps1)) is armed in code and -registered nowhere. - -> **THE ROWS ARE LIVE AND THE HOOK STILL DOES NOTHING, IN ALMOST EVERY WORKTREE.** The installed shim -> resolves `scripts/hooks/mail-drain.ps1` from the **primary checkout first**, falling back to the -> session's own worktree. Measured from two vantage points, which is the only way to see it: +the observation that goes stale without saying so. As last verified 2026-08-06 against every config +root: the drain is registered on **both `SessionStart` and `Stop`**, on **all five roots** (`~/.claude` +plus `.claude-account-1` through `-4`). All four numbered roots are in active use, so a drain on the +default root alone would have left the cross-login case -- half the reason this channel exists -- +unreachable. + +The coordination banner (`scripts/worktree/session-context.ps1`) is a *different* hook that shares the +`SessionStart` event and is deliberately still absent. **Install one tier at a time with +`-Only -Script mail-drain`**: `-Only SessionStart` alone would wire the banner too. The urgent +tier ([`scripts/hooks/mail-watch.ps1`](../scripts/hooks/mail-watch.ps1)) is armed in code and +registered nowhere, by decision. + +**VERIFIED END TO END, through the installed hook command rather than a convenient stand-in.** The +earlier VS Code run drove the drain by absolute path, which does not exercise the thing that actually +runs; this one used the shim verbatim from `~/.claude/settings.json`: + +| check | result | +|---|---| +| shim resolution, probed from five worktrees on unrelated branches | resolves in **all** of them, via the primary checkout | +| message queued with `main`'s `mail.ps1` | delivered by the installed hook command | +| receipt written by the drain process | `disposition: shown-consumed`, `byHookEvent: Stop` | +| box afterwards | `inbox 0`, `seen 1` -- consumed exactly once | + +⚠️ **What made it live was `git pull` in the primary checkout, not the merge** -- see the note below, +which is the trap this section was wrong about for several hours. + +> **RETAINED AS A LESSON, NOT AS CURRENT STATE: for several hours the rows were live and the hook did +> nothing, in almost every worktree.** The shim resolves `scripts/hooks/mail-drain.ps1` from the +> **primary checkout first**, falling back to the session's own worktree. Two things follow, and both +> cost real time to find: +> +> **1. A single-vantage check cannot see this failure.** Probed from two places while it was broken: > > | probed from | primary base | own-worktree fallback | resolves | > |---|---|---|---| -> | a worktree holding this branch | `False` | `True` | yes -- **but only because that branch carries the file** | +> | a worktree carrying the branch | `False` | `True` | yes -- **only because that branch carried the file** | > | any other worktree | `False` | `False` | **nothing** | > -> Every session outside a worktree holding this branch fires the hook, resolves nothing, and exits 0 -- -> **byte-identical to a healthy hook with no mail**, the exact defect -> [observability rule 1](#three-observability-rules) exists to prevent. Probing from the branch worktree -> alone reports `True` and looks healthy, so **a single-vantage check cannot see this**. +> Checking from the branch worktree returns `True` and looks healthy. Everywhere else the hook fired, +> resolved nothing and exited 0 -- **byte-identical to a healthy hook with no mail**, the exact defect +> [observability rule 1](#three-observability-rules) exists to prevent. **Probe a shim-resolved hook +> from a worktree that does NOT carry the branch**, or the fallback masks a broken primary resolution. > -> ⚠️ **MERGING TO `main` IS NOT WHAT MAKES IT LIVE, and an earlier draft of this section said it was.** -> Measured immediately after PR #210 landed: `mail-drain.ps1` was on `origin/main` and **still absent -> from the primary checkout's working tree**, which was three commits behind. The shim resolves a -> **working tree**, not a ref. Nothing pulls the primary automatically, so the channel stays inert -> across the whole machine until somebody does -- and the merge notification is exactly the moment an -> operator is most likely to believe it went live. +> **2. Merging to `main` is NOT what makes a shim-resolved hook live, and an earlier draft of this +> section said it was.** Measured immediately after PR #210 landed: `mail-drain.ps1` was on +> `origin/main` and **still absent from the primary checkout's working tree**, which was three commits +> behind. **The shim resolves a WORKING TREE, not a ref.** Nothing pulls the primary automatically, so +> the channel stayed inert across the whole machine until somebody did -- and the merge notification is +> exactly the moment an operator is most likely to believe it went live. **The activating action was +> `git pull` in the primary checkout.** > -> **The activating action is `git pull` in the primary checkout, not the merge.** Re-test on the target -> surface after that, from a worktree that does **not** carry this branch, so the fallback cannot mask a -> broken primary resolution. A row in a settings file is not a hook that fired, which is what the -> installer prints on every run. +> A row in a settings file is not a hook that fired, which is what the installer prints on every run. The show/consume split described in ["Showing is not consuming"](#showing-is-not-consuming) is what makes wiring `SessionStart` safe: a discarded session can display mail but cannot consume it. From a64cec2f6d0e4c66fb499287be60493022f2bcc2 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 6 Aug 2026 08:44:10 -0500 Subject: [PATCH 8/9] fix(coord): consumption now depends on what THIS invocation rendered, not on a shared session id Closes the session-id reuse loss, which was reproduced rather than theorised. A phantom minted a shown-marker; a later session carrying the SAME id inherited it, had its display suppressed, and consumed the message at its Stop having rendered nothing. A message consumed by a session that never saw it, with a clean receipt. THE FIX IS A DELETION, NOT A GUARD. Nothing available to this hook separates the two sessions -- session_id, transcript_path and cwd are identical -- so no check could have rescued the marker. A consuming drain therefore ignores markers entirely and renders what it is about to consume. What a drain rendered in THIS process, in THIS invocation, is a fact no other session can forge. The Complete-Held pass and its counters are gone with the mechanism they served. A marker may still suppress a re-display at a NON-consuming event. It may never authorise a consume. Those two decisions are no longer wired together. THE PRICE, PAID KNOWINGLY: a session shown mail at SessionStart is shown it again at its first Stop. That is now a GUARANTEED duplicate rather than an occasional one, and it is the accepted side of this channel's one tradeoff -- duplicate display is accepted, silent loss is not. TWO THINGS I GOT WRONG ALONG THE WAY, both caught by checking rather than by reading, and both worth recording because each nearly shipped: 1. MY FIRST EDIT SILENTLY DELETED THE $markerPath ASSIGNMENT -- it was inside the matched region and absent from the replacement. Nine tests went red for that reason, not for the design change, and I was one step from "reconciling" them against a bug I had introduced. Investigating the ONE unexplained failure before touching the expected ones is what caught it. Blast radius was 4 tests, not the 13 I first reported. 2. THE NEW REGRESSION TEST PASSED VACUOUSLY. It was written as `if not box_files(...): assert shown`, so with Complete-Held already removed the guard never ran and the test went green against code with half the defect restored. A mutation that put the suppression back proved it. The assertion is now unconditional -- the consuming drain must RENDER what it consumes -- and is red under BOTH the original defect and a partial regression. Two tests for the removed held-consume path are deleted; two whose contract changed are rewritten to the new one rather than dropped, because the properties under them still matter (a consuming drain renders what it consumes; a receipt never pairs one session's id with another's emit time). 144 pass across the session-mail, announce-wiring, presence and collision-gate suites. ruff clean. Mutation-verified in both directions. --- docs/SESSION-MAIL.md | 67 ++++++------ scripts/hooks/mail-drain.ps1 | 168 ++++++++--------------------- tests/test_session_mail.py | 199 +++++++++++++++++------------------ 3 files changed, 168 insertions(+), 266 deletions(-) diff --git a/docs/SESSION-MAIL.md b/docs/SESSION-MAIL.md index cb1cedf4..c88114e2 100644 --- a/docs/SESSION-MAIL.md +++ b/docs/SESSION-MAIL.md @@ -382,12 +382,28 @@ specific to two claimers racing a single source onto a single destination. Those are two separate acts, and keeping them separate is what stops a session nobody is looking at swallowing the mail. The mechanism: -- **At `Stop`** (the only consuming event): render anything this session has not already been shown, - then **consume** -- claim, receipt, move to `seen/` -- everything this session has been shown, - including what it was shown earlier at `SessionStart`. +- **At `Stop`** (the only consuming event): render **everything deliverable**, then **consume** -- + claim, receipt, move to `seen/` -- **exactly what it just rendered, and nothing else**. Markers are + ignored here entirely. - **At `SessionStart`, and at any other event**: render the mail and **leave it in the inbox**. A - marker file `box//shown/--.marker` records that this session has seen it, so - the same session is not shown it a second time. + marker file `box//shown/--.marker` records the display, and suppresses a + re-display at another *non-consuming* event. It can never authorise a consume. + +⛔ **CONSUMPTION DEPENDS ONLY ON WHAT THE SAME INVOCATION RENDERED, and that is a correction to an +earlier design rather than an embellishment of it.** `Stop` used to consume what a *marker* said this +session had been shown at an earlier drain, without re-rendering it. That was measured losing mail: +**session ids are reused across launches**, so a discarded session mints a marker and a later session +carrying the same id inherits it -- then consumes a message it was never shown, with a clean receipt. +Reproduced end to end before removal. + +**No check could have rescued it.** `session_id`, `transcript_path` and `cwd` are identical between +the two sessions, so nothing available to this hook separates them. The failure was structural, not a +missing guard, which is why the fix deletes the cross-invocation trust instead of hardening it. What +a drain rendered *in this process, in this invocation* is a fact no other session can forge. + +**The price, paid knowingly: a session shown mail at `SessionStart` is shown it again at its first +`Stop`.** That is a guaranteed duplicate rather than an occasional one. Do not reintroduce a +held-consume to remove it without first solving the identity problem above. **Why**, and it is the defect measured in ["What still blocks wiring"](#what-still-blocks-wiring) 1b: a `SessionStart` hook that CONSUMES state can lose that state to a session that never existed. A hook @@ -409,38 +425,15 @@ the one to watch: that submitted prompts. The five phantoms fired `SessionStart` and nothing else. Previously the argument was weaker -- that they never became conversations, so presumably never took a turn -- and it now rests on observed teardown behaviour instead. -- **A phantom's session id must differ from the surviving session's. THIS PRECONDITION IS VIOLABLE, AND - THE LOSS IT ALLOWS HAS NOW BEEN REPRODUCED.** Not "at risk", not inferred -- measured 2026-08-06 - against the shipped drain: - - | step | outcome | - |---|---| - | session `X` `SessionStart` (the phantom) | message **displayed**, marker minted, **not consumed** -- correct | - | session `X'` reusing the same id, `SessionStart` | **not displayed** -- suppressed by the phantom's marker | - | session `X'` `Stop` | **not displayed**, and **consumed** | - - The message was consumed by a session that never saw it. From the drain's side those three - invocations are byte-identical to one healthy session showing mail at start and consuming it at the - turn boundary, so **nothing in this design can detect the difference** -- every artefact is keyed by - session id, and the two sessions share one. - - **All three preconditions are observed, not hypothesised.** Session ids are **reused across - launches** (one of the six in the phantom run carried an id seen hours earlier), phantoms mint - markers, and a marker for a message still in the inbox is correctly *not* garbage-collected -- so it - survives exactly as long as the message it would suppress. - - **What this does and does not cost.** It is strictly narrower than the defect the split closed: that - one lost mail to the *first* of six phantoms on every launch, unconditionally. This needs a pending - message, a phantom that displayed it, and a later session reusing that phantom's id before the - message is consumed by anyone else. But it is the same *kind* of failure -- silent, receipt-clean, - and invisible to the operator -- and the channel's stated rule is that duplicate display is accepted - while silent loss is not. - - **The principled fix is to stop trusting a marker across invocations: consume only what the SAME - drain invocation rendered.** `SessionStart` would then display without earning the right to consume, - and `Stop` would display again and consume what it just showed. That trades a guaranteed duplicate - display for the removal of the entire cross-invocation trust relationship, which is the direction the - stated rule points. It is a design change, not a patch, and it is **not** made here. +- **A phantom's session id must differ from the surviving session's. THIS ONE IS NOW MOOT, AND THE + REASON IS THE FIX ABOVE.** It was measured VIOLABLE -- session ids are reused across launches, and + the resulting loss was reproduced end to end: a phantom marked a message, a later session carrying + the same id had its display suppressed, and its `Stop` consumed a message it had never seen. + + That precondition no longer has to hold, because consumption no longer depends on identity at all. + A consuming drain renders what it consumes, so a marker inherited from any other session -- phantom + or not, same id or not -- can at worst suppress one *non-consuming* display. It cannot cause a + consume. **The dependency was removed rather than the risk being argued down.** **The marker is the per-session record of a display, and the receipt is not.** A receipt is named `.json` -- one slot per **message**, last writer wins -- so it can say *some* session was shown diff --git a/scripts/hooks/mail-drain.ps1 b/scripts/hooks/mail-drain.ps1 index df881a77..0212635d 100644 --- a/scripts/hooks/mail-drain.ps1 +++ b/scripts/hooks/mail-drain.ps1 @@ -167,12 +167,6 @@ $MAX_TS_CHARS = 40 $MAX_CWD_CHARS = 200 $MAX_BRANCH_CHARS = 120 $MAX_LINE_CHARS = 240 -# Held messages consumed per drain. It bounds WORK, not delivery: a held message over this cap keeps its -# marker, stays in the inbox and is consumed at the next turn boundary, exactly as a message over -# MAX_MESSAGES is shown at the next drain. Ten times the display cap, because the held list grows by at -# most MAX_MESSAGES per non-consuming drain and a session that ran ten of them before its first turn -# boundary is already pathological. -$MAX_HELD_CONSUME = 50 # There is no id cap: the id is the validated filename stem, whose shape is strictly narrower than # anything a cap would enforce. Adding one would imply the id is untrusted at this point, which would # be the wrong thing for the next reader to believe. @@ -541,15 +535,7 @@ try { $swept = 0 $duplicateStems = 0 # --- The show/consume split's own counters. --- - $held = @() # marked as shown to THIS session: not re-rendered, and consumed by this - # session's Stop. Still physically in the inbox. $alreadyShown = 0 # the held count, named for what the reader cares about. - $heldGone = 0 # held by this session and no longer in the inbox: another session's drain - # took it. Nothing was lost. - $heldDeferred = 0 # held, and over this drain's consume cap. Consumed at the next turn - # boundary; still in the inbox until then. - $heldUnfinalized = 0 # held, and the consume did not complete this pass. Still on disk -- in - # claiming/ if it was claimed first, otherwise still in the inbox. $sweptMarkers = 0 $unownedMarkers = 0 # a name in shown/ this channel did not mint. Counted, left alone. # Reported, not swallowed. A retention sweep that cannot run is not itself serious -- it bounds @@ -561,9 +547,7 @@ try { # instead -- the file stays in claiming/, which mail.ps1 -Status reports, and the next drain's # dead-owner sweep moves it to stranded/ where its receipt proves it was shown. # - # $heldUnfinalized above is NOT an exception to that and is the reason this comment names a specific # loop rather than "a finalize". Complete-Held runs BEFORE the counter block is built, precisely so - # $heldGone can reach the reader, and a failure there is countable for exactly the same reason. The # older wording said no such counter could ever exist and was wrong the moment that step moved. # --- Retention sweep of the TERMINAL directories. ------------------------------------------- @@ -804,10 +788,25 @@ try { # suppress the display while Stop consumed the message -- a silent loss. Its failure direction # is a duplicate display, which is accepted. $markerPath = if ($markable) { Join-Path $shownDir "$id--$sessionKey.marker" } else { $null } - if ($markerPath -and (Test-FilePresent -Path $markerPath)) { - $held += [pscustomobject]@{ - File = $f; Id = $id; ReceiptPath = $receiptPath; MarkerPath = $markerPath - } + # A MARKER MAY SUPPRESS A DISPLAY. IT MAY NEVER AUTHORISE A CONSUME -- hence `-not $consuming`, + # and that clause is the whole fix for a loss that was REPRODUCED rather than theorised. + # + # SESSION IDS ARE REUSED ACROSS LAUNCHES. Measured: one of six ids in the phantom run had been + # seen hours earlier. So a discarded session mints a marker, and a LATER session carrying the + # same id inherits it. Under the previous rule that later session had its display suppressed + # here and then consumed the message at its Stop, having rendered nothing: a message consumed + # by a session that never saw it, with a clean receipt. Nothing inside this design could + # separate the two -- every artefact is keyed by the id they share, so the sequence was + # byte-identical to one healthy session showing mail at start and consuming it at the boundary. + # + # A CONSUMING DRAIN THEREFORE IGNORES MARKERS ENTIRELY and renders what it is about to consume. + # Consumption now depends only on what THIS INVOCATION rendered, which is a property no other + # session can forge, rather than on an identity two sessions can share. + # + # THE COST IS A GUARANTEED DUPLICATE DISPLAY: a session shown mail at SessionStart is shown it + # again at its first Stop. That is the accepted side of this channel's one tradeoff, and it is + # the direction the rule points -- duplicate display is accepted, silent loss is not. + if ($markerPath -and -not $consuming -and (Test-FilePresent -Path $markerPath)) { $alreadyShown++ continue } @@ -873,81 +872,23 @@ try { $delivered += [pscustomobject]@{ Item = $s; Token = $null; Path = $null } } - # --- Complete-Held: consume what THIS session was already shown. ----------------------------- - # This is the half that makes SessionStart's non-consumption safe rather than merely deferred: the - # mail is consumed at the first turn boundary the session actually reaches, and a session that never - # reaches one consumes nothing. - # - # NO RE-RENDER. These were emitted by an earlier drain of this session -- that is what the marker - # records -- so re-rendering would be the duplicate the marker exists to prevent. + # THERE IS NO "CONSUME WHAT THIS SESSION WAS ALREADY SHOWN" STEP, AND THAT ABSENCE IS THE FIX. + # A Complete-Held pass used to consume messages a marker said this session had been shown at an + # earlier drain, without re-rendering them. It was removed because SESSION IDS ARE REUSED ACROSS + # LAUNCHES (measured), so "this session" is not something a marker can establish: a discarded + # session mints the marker and a later session carrying the same id inherits it, then consumes a + # message it never saw. Reproduced end to end before removal. # - # RUN BEFORE THE INJECTION IS BUILT, AND FROM ONE CALL SITE. It has to be reachable from both exits - # -- a Stop drain whose only work is consuming what SessionStart showed reaches zero delivered -- - # and $heldGone has to reach the reader, which it cannot if this runs after the emit. Ordering is - # safe because the receipt it writes is a statement about an emit that already happened, and the - # consume it records happens right here. Being before the emit is also why $heldUnfinalized can - # exist at all, unlike the delivery loop's finalize. + # No check could rescue it. session_id, transcript_path and cwd are all identical between the two, + # so nothing available to this hook separates them -- the failure was structural rather than a + # missing guard, which is why the answer is to delete the trust relationship instead of hardening + # it. Consumption is now a function of what THIS INVOCATION rendered, which no other session can + # forge. # - # THE CAP IS REAL AND THE REASONING IT REPLACED WAS NOT. This used to say the held list was bounded - # by MAX_MESSAGES because displays are capped per drain -- true premise, invalid conclusion: the - # bound is MAX_MESSAGES times the number of NON-CONSUMING DRAINS the session runs, which grows with - # every non-Stop event that ever gets wired. Measured: three SessionStart drains over twelve queued - # messages held twelve, and the next Stop claim-and-moved all twelve in one uncapped loop inside a - # hook that must exit fast. The remainder is not lost -- it keeps its marker, stays in the inbox, - # and is consumed at the next turn boundary -- so this is a delay, exactly as the display cap is. - if ($consuming -and $held.Count -gt 0) { - $heldDone = 0 - foreach ($h in $held) { - if ($heldDone -ge $MAX_HELD_CONSUME) { $heldDeferred++; continue } - $heldDone++ - try { - $hr = New-ClaimAttempt -Source $h.File.FullName -DestinationDir $claimingDir -Stem $h.Id - if (-not $hr.Won) { - # Another session's drain took it. NOTHING WAS LOST -- it is being shown, or has - # been shown, by whoever won. Drop our marker and move on. - $heldGone++ - Remove-Item -LiteralPath $h.MarkerPath -Force -ErrorAction SilentlyContinue - continue - } - # THE EMIT TIME COMES FROM THE MARKER, NOT FROM THE RECEIPT, and that is a correctness - # fix rather than a preference. The receipt is one slot per MESSAGE, so under the - # accepted duplicate it may already hold ANOTHER session's emit time -- pairing it with - # this session's id would make the receipt name a display that did not happen then. The - # marker is per-(message, session) and was written by the process that emitted to THIS - # session, so its markedUtc is the right timestamp by construction. - # - # Its CONTENT is still untrusted: the marker sits in a directory any local process can - # write to, so this is folded and capped, reaches the receipt JSON only, and never the - # injection. An unreadable or absent stamp yields '' -- which Write-MailReceipt accepts, - # deliberately; see the [AllowEmptyString()] note there for the strand this cost. - # ConvertFrom-Json silently coerces an ISO-8601 string to [datetime], and [string] on - # that yields the LOCAL short form -- round-trip it or a UTC stamp renders as local. - $shownUtc = '' - try { - $mk = (Get-Content -LiteralPath $h.MarkerPath -Raw -EA Stop | ConvertFrom-Json -EA Stop).markedUtc - $shownUtc = if ($mk -is [datetime]) { $mk.ToUniversalTime().ToString('o') } - elseif ($mk -is [datetimeoffset]) { $mk.ToString('o') } - else { [string]$mk } - } - catch { } - $shownUtc = Get-Clean $shownUtc $MAX_TS_CHARS - $nowUtc = [DateTime]::UtcNow.ToString('o') - Write-MailReceipt -Path $h.ReceiptPath -MessageId $h.Id -Disposition 'shown-consumed' ` - -ObservedUtc $shownUtc -ClaimToken $hr.Token -ConsumedUtc $nowUtc ` - -ConsumedByHookEvent $eventName -ByWorktree $cwd -BySessionId $sessionId ` - -SessionIdValid $markable -ByHookEvent $eventName -BoxKey $key -ExaminedPath $inboxDir - $fin = Move-Claimed -Source $hr.Path -DestinationDir $seenDir -Stem $h.Id -Token $hr.Token - if ($fin.Won) { Remove-Item -LiteralPath $h.MarkerPath -Force -ErrorAction SilentlyContinue } - else { $heldUnfinalized++ } - } - catch { - # Reached when the receipt could not be written, which now SKIPS the move by design. The - # message stays in claiming/ with its marker; the dead-owner sweep strands it, and the - # reader is told rather than being told it was consumed. - $heldUnfinalized++ - } - } - } + # THE PRICE, PAID KNOWINGLY: a session shown mail at SessionStart is shown it again at its first + # Stop. Duplicate display is the accepted side of this channel's one tradeoff. Do not reintroduce a + # held-consume to remove that duplicate without first solving the identity problem above. + # One counter block, on EVERY injection. Named counters -- at least one per outcome the drain can # reach -- make the failure shapes distinguishable instead of collapsing into "nothing to show". @@ -970,37 +911,13 @@ try { if ($sweepFailed) { $counterLines += "A housekeeping sweep of seen/, expired/ or shown/ could not complete; delivery was unaffected." } if ($unownedClaims -gt 0) { $counterLines += "$unownedClaims file(s) in claiming/ carry a name this channel did not mint and were left alone." } if ($swept -gt 0) { $counterLines += "$swept message(s) older than $RETAIN_DAYS days were removed from seen/ and expired/." } - # The show/consume split's own outcomes. DISTINCT OUTCOMES, DISTINCT SENTENCES -- this file's first - # observability rule, and the reason the counter block exists at all. + # The show/consume split's own outcome. ONE sentence now, because there is only one: a marker can + # suppress a re-display at a non-consuming event and nothing else. A consuming drain ignores markers + # entirely, so $alreadyShown is unreachable when $consuming and the branch that used to report a + # held-consume is gone with the mechanism it described. if ($alreadyShown -gt 0) { - if ($consuming) { - # THE ARITHMETIC IS SPELLED OUT BECAUSE THE FLAT SENTENCE WAS FALSE. This used to read "they - # were consumed at this turn boundary" over the whole held count, which contradicted the - # $heldGone / $heldDeferred / $heldUnfinalized lines printed a few rows down whenever any of - # them was non-zero -- two sentences about one set of messages, disagreeing. Each of those - # three says where its share actually went, so this one names only the remainder. - $consumedHeld = $alreadyShown - $heldGone - $heldDeferred - $heldUnfinalized - $counterLines += "$alreadyShown message(s) had already been shown to this session and were NOT shown again;" - $counterLines += "$consumedHeld of them were consumed at this turn boundary. Showing is not consuming -- see" - $counterLines += "docs/SESSION-MAIL.md, 'Showing is not consuming'." - } - else { - $counterLines += "$alreadyShown message(s) have already been shown to this session and were NOT shown again." - $counterLines += "They are HELD in the inbox and are consumed at this session's next turn boundary, not now." - } - } - if ($heldGone -gt 0) { - $counterLines += "$heldGone message(s) previously shown to this session are no longer in the inbox: another" - $counterLines += "session's drain took them. Nothing was lost." - } - if ($heldDeferred -gt 0) { - $counterLines += "$heldDeferred message(s) already shown to this session were over this drain's consume cap of" - $counterLines += "$MAX_HELD_CONSUME; they stay in the inbox and are consumed at the next turn boundary." - } - if ($heldUnfinalized -gt 0) { - $counterLines += "$heldUnfinalized message(s) already shown to this session could NOT be consumed this pass and are" - $counterLines += "still on disk -- in claiming/ if claimed, otherwise still in the inbox. Nothing was lost; see" - $counterLines += "scripts\coord\mail.ps1 -Status." + $counterLines += "$alreadyShown message(s) have already been shown to this session and were NOT shown again" + $counterLines += "now. They stay in the inbox and are shown AND consumed at this session's next turn boundary." } if (-not $markable) { # THE FACT, NOT THE VALUE. The raw id is untrusted text and the injection is the one place @@ -1026,14 +943,13 @@ try { if ($delivered.Count -eq 0) { # Not silence when something WAS there and none of it was shown. "Nothing to show" and # "nothing arrived" are different facts about the channel and must not render alike. - # A PRESENCE TEST, NOT A TOTAL. Some of these overlap ($held is also counted in $alreadyShown), + # A PRESENCE TEST, NOT A TOTAL. Some of these can overlap, # which is harmless because the only question asked of the sum is "was anything there at all". # A box holding shown-and-held mail must never take the "box is EMPTY" branch below: that would # be a false statement about a box with mail in it, which is the defect this channel exists to # make impossible. $anything = ($unreadable + $expired + $malformed + $filtered + $deferred + $ceded + $stranded + - $unownedClaims + $duplicateStems + $alreadyShown + $held.Count + $heldDeferred + - $heldUnfinalized + $heldGone + $unownedMarkers + $sweptMarkers) + $unownedClaims + $duplicateStems + $alreadyShown + $unownedMarkers + $sweptMarkers) if ($anything -gt 0) { $z = @("[mefor-mail] Drain ran at $asOf over $inboxDir. Nothing is being shown to you.") $z += $counterLines diff --git a/tests/test_session_mail.py b/tests/test_session_mail.py index 5b4bd0d4..178cdac3 100644 --- a/tests/test_session_mail.py +++ b/tests/test_session_mail.py @@ -96,7 +96,6 @@ def _const(name: str) -> int: MAX_LINE_CHARS = _const("MAX_LINE_CHARS") # Held messages consumed per drain. It bounds WORK, not delivery: the remainder keeps its marker and is # consumed at the next turn boundary, exactly as a message over MAX_MESSAGES is shown at the next drain. -MAX_HELD_CONSUME = _const("MAX_HELD_CONSUME") # -------------------------------------------------------------------------------------------------- @@ -626,6 +625,57 @@ def test_the_naive_shared_destination_pattern_does_not_exclude(tmp_path: Path) - @pytest.mark.timeout(300) +def test_a_session_reusing_a_phantoms_id_cannot_consume_mail_it_never_saw( + repo: Path, tmp_path: Path +) -> None: + """THE SESSION-ID REUSE LOSS -- reproduced 2026-08-06, then fixed by removing cross-invocation trust. + + Session ids are REUSED ACROSS LAUNCHES; one of six ids in the measured phantom run had been seen + hours earlier. So a discarded session can mint a shown-marker, and a LATER session that happens to + carry the same id inherits it. Under the marker-gated design that later session had its display + suppressed and then consumed the message at its Stop -- a message consumed by a session that never + saw it, with a clean receipt. + + Nothing INSIDE that design could separate the two sessions: every artefact is keyed by the id they + share, so the three invocations below were byte-identical to one healthy session showing mail at + start and consuming it at the turn boundary. + + The fix is structural rather than a better check: consumption depends only on what THE SAME + INVOCATION rendered. A session that was not shown the message in this very drain cannot consume it, + whatever any marker says. + """ + shared = "bbbbbbbb-1111-2222-3333-555555555555" + info = seed(repo, tmp_path, [{"body": "REUSE-LOSS: consumed by a session that never saw me"}]) + key = str(info["key"]) + + # 1. The phantom: SessionStart only, then discarded. It may display; it must not consume. + first = injection(run_drain(repo, event="SessionStart", session_id=shared)) + assert "REUSE-LOSS" in first, "the phantom should still be shown the message" + assert box_files(repo, key, "inbox"), ( + "a non-consuming event must leave the message in the inbox" + ) + + # 2. A DIFFERENT session that happens to reuse the id, and its turn boundary. + second = injection(run_drain(repo, event="Stop", session_id=shared)) + + # UNCONDITIONAL, AND IT WAS NOT AT FIRST. This was written as + # `if not box_files(...): assert shown`, which passes VACUOUSLY the moment either half of the + # defect is absent -- a mutation restoring only the marker suppression left the message in the + # inbox, the guard never ran, and the test went green against code that had the bug half back. + # A check that can pass without exercising anything is not a check. + # + # The consuming drain must RENDER what it is entitled to consume, so assert the render directly. + # That is red under the original defect (suppressed, then consumed unseen) AND under a partial + # regression that restores only the suppression. + assert "REUSE-LOSS" in second, ( + "the consuming drain did not render the message. Under the original defect it consumed it " + "unseen; under a partial regression it suppresses the display and strands it. Either way a " + f"session is deciding what it saw from a marker another session minted. Injection: {second[:400]!r}" + ) + assert box_files(repo, key, "inbox") == [], "rendered at Stop but not consumed" + assert len(box_files(repo, key, "seen")) == 1 + + def test_the_claim_verdict_agrees_with_the_filesystem(tmp_path: Path) -> None: """DEFECT 4, the verdict half -- and the half that was nearly shipped broken. @@ -1442,44 +1492,46 @@ def test_the_phantom_arm_can_see_the_defect_it_asserts_against(repo: Path, tmp_p # --- 8b. A real session, and the wired path. ---------------------------------------------------- -def test_a_real_session_is_shown_mail_once_and_consumes_it_at_its_turn_boundary( +def test_a_real_session_is_shown_mail_again_at_its_turn_boundary_and_consumes_it_there( repo: Path, tmp_path: Path ) -> None: """The half that makes SessionStart's non-consumption safe rather than merely deferred. - Three separate properties, and each fails differently: the mail is shown at SessionStart, it is - NOT shown a second time to the same session, and it leaves the inbox at that session's first turn - boundary. A drain that re-rendered at Stop would satisfy the third and fail the second. + THIS TEST USED TO ASSERT THE OPPOSITE -- shown once, not re-rendered at Stop -- and that behaviour + was removed deliberately. Suppressing the second display required trusting a marker across + invocations, and a marker is keyed by a session id that IS REUSED ACROSS LAUNCHES, so a session + inheriting a phantom's marker consumed mail it had never been shown. Reproduced, then fixed by + deleting the trust rather than hardening it. - ``observedUtc`` is carried FORWARD rather than reset, so the consuming receipt still records when - the text was actually put in front of a reader. Collapsing the two stamps would make the receipt - claim the mail was shown at Stop, which is the one thing it was not. + So the contract is now: shown at SessionStart, shown AGAIN at the first Stop, and consumed there. + The duplicate display is the price, and it is the accepted side of this channel's one tradeoff -- + duplicate display is accepted, silent loss is not. + + ``observedUtc`` therefore records the STOP emit, because that is when the consuming drain actually + put the text in front of a reader. It is no longer carried forward from an earlier display, since + nothing may now assert that a previous invocation showed anything to this session. """ - info = seed(repo, tmp_path, [{"body": "one display, one consume"}]) + info = seed(repo, tmp_path, [{"body": "shown twice, consumed once"}]) key = str(info["key"]) stem = str(info["rows"][0]["stem"]) - start = injection(run_drain(repo, event="SessionStart", session_id=SESSION_A)) - assert "one display, one consume" in start - shown_at = receipt_json(repo, stem)["observedUtc"] - assert shown_at + start_text = injection(run_drain(repo, event="SessionStart", session_id=SESSION_A)) + assert "shown twice, consumed once" in start_text + assert box_files(repo, key, "inbox"), "a non-consuming event must not consume" + assert receipt_json(repo, stem)["disposition"] == "shown-held" stop = injection(run_drain(repo, event="Stop", session_id=SESSION_A)) - assert "one display, one consume" not in stop, "the same session was shown the mail twice" - assert "had already been shown to this session and were NOT shown again" in stop - assert "consumed at this turn boundary" in stop + # THE LOAD-BEARING ASSERTION: the consuming drain RENDERED what it consumed. + assert "shown twice, consumed once" in stop, ( + "the consuming drain removed the message without rendering it -- that is the silent loss" + ) assert box_files(repo, key, "inbox") == [] assert box_files(repo, key, "claiming") == [] assert len(box_files(repo, key, "seen")) == 1 final = receipt_json(repo, stem) assert final["disposition"] == "shown-consumed" - assert final["observedUtc"] == shown_at, ( - "the consuming receipt reset observedUtc, so it now claims the mail was shown at Stop" - ) - assert final["consumedUtc"] and final["consumedUtc"] != shown_at - # Requirement: after consumption, no marker for that message survives. - assert markers(repo, key) == [] + assert final["observedUtc"], "a consumed receipt must record when it was emitted" def test_stop_alone_shows_and_consumes_with_no_preceding_session_start( @@ -2005,19 +2057,20 @@ def test_a_case_variant_marker_is_one_file_and_is_reported_as_one( assert markers(repo, key) == [], "a case variant was consumed but never swept" -def test_the_consumed_receipt_carries_the_time_it_was_shown_not_the_time_it_was_consumed( +def test_a_receipt_never_pairs_one_sessions_id_with_anothers_emit_time( repo: Path, tmp_path: Path ) -> None: - """``observedUtc`` COMES FROM THE MARKER, and that is a correctness property rather than a - plumbing detail. + """The receipt is ONE SLOT PER MESSAGE, so its two identifying fields must describe ONE emit. - The receipt is one slot per MESSAGE, so under the accepted duplicate it may already hold ANOTHER - session's emit time; pairing that with this session's id would make the file name a display that - did not happen then. The marker is per-(message, session), so its ``markedUtc`` is the right - timestamp by construction. + Under the accepted duplicate, several sessions display the same message and each overwrites that + slot. The failure to guard against is a receipt naming session A while carrying the timestamp of + B's display -- a file asserting a display that did not happen at that time, by that session. - Asserted three ways: the consumed receipt's ``observedUtc`` matches the SessionStart emit exactly, - it is not the consume time, and the sibling session's later emit does not move it. + This used to be enforced by sourcing ``observedUtc`` from the per-(message, session) MARKER. That + is gone: markers may no longer be trusted across invocations, because a session id is REUSED + ACROSS LAUNCHES and an inherited marker let a session consume mail it had never seen. The property + is now structural instead -- a consuming drain RENDERS what it consumes, so the id and the stamp + both come from the same emit and cannot disagree. """ stem = "20260101T000000001-aaaaaa" seed(repo, tmp_path, [{"stem": stem, "body": "two sessions, one receipt slot"}]) @@ -2028,84 +2081,24 @@ def test_the_consumed_receipt_carries_the_time_it_was_shown_not_the_time_it_was_ # B displays the same message and overwrites the single receipt slot with its own emit time. run_drain(repo, event="SessionStart", session_id=SESSION_B) - assert receipt_json(repo, stem)["observedUtc"] != shown_by_a + shown_by_b = receipt_json(repo, stem)["observedUtc"] + assert shown_by_b != shown_by_a + + text = injection(run_drain(repo, event="Stop", session_id=SESSION_A)) + assert "two sessions, one receipt slot" in text, ( + "the consuming drain must render what it consumes" + ) - run_drain(repo, event="Stop", session_id=SESSION_A) final = receipt_json(repo, stem) assert final["disposition"] == "shown-consumed" assert final["bySessionId"] == SESSION_A - assert final["observedUtc"] == shown_by_a, ( - "the receipt pairs one session's id with another session's emit time -- it now names a " - "display that did not happen at that moment" - ) - assert final["consumedUtc"] != final["observedUtc"] - - -def test_a_held_consume_that_cannot_finish_is_reported_and_not_claimed_as_done( - repo: Path, tmp_path: Path -) -> None: - """THE COUNTER THAT COULD NOT EXIST UNDER THE OLD ORDERING, and the contradiction it removes. - - Complete-Held runs BEFORE the injection is built, so unlike the delivery loop's finalize a failure - there IS reportable -- the drain used to carry a comment saying no such counter could ever exist, - written when the step ran later. Without it the injection said the held mail "was consumed at this - turn boundary" while the message sat in ``claiming/``. - - The failure is induced by making the receipt unwritable between the show and the consume, which is - also the arm that proves ``Write-MailReceipt`` still refuses to finalize without a receipt. - """ - stem = "20260101T000000001-aaaaaa" - info = seed(repo, tmp_path, [{"stem": stem, "body": "held, then the disk went away"}]) - key = str(info["key"]) - - assert "held, then the disk went away" in injection( - run_drain(repo, event="SessionStart", session_id=SESSION_A) + # The stamp belongs to A's OWN consuming emit -- later than B's display, and not B's. + assert final["observedUtc"] != shown_by_b, ( + "the receipt names session A but carries B's emit time" ) - rd = mail_root(repo) / "receipts" - shutil.rmtree(rd) - rd.write_text("not a directory", encoding="ascii") - - text = injection(run_drain(repo, event="Stop", session_id=SESSION_A)) - assert "could NOT be consumed this pass" in text - assert "1 of them were consumed at this turn boundary" not in text - assert "0 of them were consumed at this turn boundary" in text - assert box_files(repo, key, "seen") == [] - - -def test_more_held_messages_than_the_consume_cap_are_deferred_not_dropped( - repo: Path, tmp_path: Path -) -> None: - """THE BOUND ON THE HELD LIST, asserted because the reasoning it replaced was invalid. - - The drain used to say the held list was bounded by ``MAX_MESSAGES`` "because displays are capped - per drain". True premise, wrong conclusion: the bound is ``MAX_MESSAGES`` times the number of - non-consuming drains, so repeated SessionStarts grow it without limit. ``MAX_HELD_CONSUME`` is the - real bound, and like every other cap in this channel it DEFERS rather than drops. - - Driven with a cap lowered by nothing -- the arm builds enough held messages to cross the shipped - value by running the display cap repeatedly, which is the same mechanism that produced the unbounded - growth. - """ - total = MAX_HELD_CONSUME + MAX_MESSAGES - info = seed( - repo, - tmp_path, - [{"body": f"held message {i}"} for i in range(total)], + assert final["observedUtc"] > shown_by_b, ( + "the consuming receipt's stamp predates a display that happened before it" ) - key = str(info["key"]) - # MAX_MESSAGES shown per non-consuming drain, so this many drains to hold them all. - for _ in range(-(-total // MAX_MESSAGES)): - run_drain(repo, event="SessionStart", session_id=SESSION_A) - assert len(markers(repo, key)) == total - - text = injection(run_drain(repo, event="Stop", session_id=SESSION_A)) - assert "were over this drain's consume cap" in text - assert len(box_files(repo, key, "seen")) == MAX_HELD_CONSUME - assert len(box_files(repo, key, "inbox")) == total - MAX_HELD_CONSUME - - injection(run_drain(repo, event="Stop", session_id=SESSION_A)) - assert box_files(repo, key, "inbox") == [], "the deferred remainder was never consumed" - assert len(box_files(repo, key, "seen")) == total def test_the_off_switch_neither_shows_nor_consumes_held_mail(repo: Path, tmp_path: Path) -> None: From 05fdba08232e6ae4c9edb82556aa6defd47dcbf6 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 6 Aug 2026 08:44:49 -0500 Subject: [PATCH 9/9] backlog: #1028s residual defect is fixed, not merely reproduced Item (c) recorded the session-id reuse loss as "confirmed by reproduction and neither fixed nor accepted", with the principled fix described but deliberately not taken. It has now been taken. The fix is a DELETION rather than a guard: nothing available to the drain separates a phantom from a later session reusing its id -- session_id, transcript_path and cwd are identical -- so a consuming drain now ignores markers entirely and renders what it consumes. Consumption depends only on what that invocation rendered, which no other session can forge. Cost, recorded rather than buried: a guaranteed duplicate display at the first Stop. That is the accepted side of this channel stated tradeoff. Mutation-verified in both directions -- red under the original defect AND under a partial regression that restores only the marker suppression. --- docs/BACKLOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 8c0cd2b5..6c2f1f38 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -5014,7 +5014,7 @@ The comment immediately above says *"Scope is deliberately the posture the requi **SUPERSEDED: wiring was a separate owner decision, and it was taken.** The drain went live on `Stop` first, then on `SessionStart` once the show/consume split made a discarded session unable to consume what it displayed. Default root only; no `~/.claude-account-N` carries it. **It is nonetheless INERT outside the branch worktree** -- the shim resolves from the primary checkout, which does not carry these scripts until PR #210 lands, so the hook fires, resolves nothing and exits 0. It activates everywhere, unannounced, on merge. -**WHAT REMAINS, and it is why this item is still open.** (a) PR #210 must land. (b) The hook must then be verified through the SHIM resolution path, which is NOT what was tested -- the VS Code run drove the drain by absolute path, while the live hook resolves via `git rev-parse --git-common-dir` against the primary. (c) One residual defect is **CONFIRMED BY REPRODUCTION** (2026-08-06) and is neither fixed nor accepted: session ids are reused across launches, so a phantom that displayed and marked a message can have a LATER session reusing that id suppress the display and consume the message unseen. Measured end to end against the shipped drain. It is strictly narrower than the defect the show/consume split closed, but it is the same kind -- silent, receipt-clean, and undetectable by any artefact in the design, all of which are keyed by the shared session id. The principled fix is to consume only what the same drain invocation rendered; that is a design change and is not made here. +**WHAT REMAINS, and it is why this item is still open.** (a) PR #210 must land. (b) The hook must then be verified through the SHIM resolution path, which is NOT what was tested -- the VS Code run drove the drain by absolute path, while the live hook resolves via `git rev-parse --git-common-dir` against the primary. (c) The session-id reuse defect is **REPRODUCED AND NOW FIXED** (2026-08-06). Ids are reused across launches, so a phantom's shown-marker could be inherited by a later session carrying the same id, which then suppressed its own display and consumed the message unseen. The fix is a DELETION rather than a guard -- nothing available to the hook separates the two sessions, so a consuming drain now ignores markers entirely and renders what it consumes. Consumption depends only on what THAT invocation rendered, which no other session can forge. Cost: a guaranteed duplicate display at the first `Stop`, which is the accepted side of this channel's tradeoff. Mutation-verified red against both the original defect and a partial regression. **Related:** [ADR 0161](adr/0161-async-session-mail-for-unreachable-peers.md) (the decision, the measurements, and the ten EARS acceptance criteria), ADR 0158 (silent controls — a green signal that means nothing; findings 4, 5 and 8 are instances), #1018 (guards that go quiet).