diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index eca8d56d..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 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) 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). diff --git a/docs/SESSION-MAIL.md b/docs/SESSION-MAIL.md index 2fa9a6ae..c88114e2 100644 --- a/docs/SESSION-MAIL.md +++ b/docs/SESSION-MAIL.md @@ -15,30 +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. 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 -> **byte-identical to a healthy hook with no mail**, the exact defect -> [observability rule 1](#three-observability-rules) exists to prevent. +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: > -> 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. +> **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 carrying the branch | `False` | `True` | yes -- **only because that branch carried the file** | +> | any other worktree | `False` | `False` | **nothing** | +> +> 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. +> +> **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.** +> +> 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. @@ -351,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 @@ -378,17 +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 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 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/docs/adr/0161-async-session-mail-for-unreachable-peers.md b/docs/adr/0161-async-session-mail-for-unreachable-peers.md index fc0ed924..f4326f30 100644 --- a/docs/adr/0161-async-session-mail-for-unreachable-peers.md +++ b/docs/adr/0161-async-session-mail-for-unreachable-peers.md @@ -362,20 +362,39 @@ 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 - [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 @@ -383,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". 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]@{} } 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 } 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_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 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: