Repository navigation
mail-drain: write stdout at SessionStart only, so a Stop fault cannot block the turn - #245
Conversation
… block the turn Claude Code reads a Stop hook's additionalContext as a block that re-invokes the model. Measured 2026-10-26: a worktree session with no session pin hit the drain's NO BOX path at every Stop, and the harness reported "A hook blocked the turn from ending 9 consecutive times". The header's "FAILS OPEN, AND NEVER BLOCKS" was false at Stop. - Write-Drain now writes stdout only when the event is exactly SessionStart. Every other event, and one nothing named, goes to stderr (the hook log). One place, so no path can regress it. - The default "$Event = SessionStart" is gone: it handed stdout to a Stop whose input could not be read. An unnamed event renders to the log and mints no marker. - The _mail.ps1 load failure runs before the event is read, so that path reads hook_event_name from stdin first. A SessionStart there keeps its stdout line. - docs/SESSION-MAIL.md and docs/HOOKS.md said every fault speaks at Stop; both now say the log. - tests: each Stop fault (NO BOX from stdin with no -Event, NO QUEUE, DELIVERY IS OFF, the _mail.ps1 load failure) asserts empty stdout plus the line on stderr, with a SessionStart control on the same setup that must write stdout. Six of the eleven fail on origin/main's drain. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… block the turn Review round one (xhigh), confirmed findings: - docs/HOOKS.md: the rewritten Stop cell was 48 words and pushed the fat-cell ratchet to 12 against a baseline of 11. Cut to 36. - An event nothing named ran the whole SessionStart path, expiring mail and printing bodies to stderr. It now writes NO EVENT and exits before resolving a queue. NO BOX still comes first. - "stderr is the hook log" was not measured. The header, Write-Drain's comment, SESSION-MAIL.md and the test docstring now say stderr stays out of the model's context and that what a person sees of it was not measured. - The CHANGED notes now name the instrument: the stdout half reproduces at 8940b3d; the block is the harness's own report, version not read. - The Stop comment said "only success with nothing to do writes nothing"; an expiry or unreadable file met at Stop is unreported too, as before. Said so. - Added the measured shape as a test: started in the primary, payload cwd = a worktree, no pin, Stop. Its SessionStart control reads the box and writes stdout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… block the turn Review round two (xhigh), the confirmed findings that were cheap and local: - The input's hook_event_name now wins over -Event. A Stop row copied from the SessionStart row, still passing -Event SessionStart, wrote "No mail" as additionalContext at every Stop. - The event guard now stops any event other than SessionStart or Stop, not only an empty one. UserPromptSubmit expired mail and printed bodies to stderr. $script:Spoke and the marker gate it fed are gone: only SessionStart now reaches the render path. - The "Filed" control asserted an empty inbox, which an expiry or delete also gives. It now counts the one file in seen/. - The Stop comment said it "logs only when it acted"; it also logs every fault. Reworded, and the header posture now says other events read no box. Round-two findings left open, for the PR body: unpinned worktree sessions display mail at SessionStart and can never file it at Stop (the cause of the measured loop, now quiet); expiry and unreadable counts met at Stop are reported nowhere (as before); whether a person sees stderr at exit 0, and that the harness keeps it out of the model's context, are not measured; a hand run with no -Event and no stdin now says NO EVENT instead of rendering. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
QA -- korus roles/BUILDER.md step 11 |
|
Manager: the Lander's xhigh review held this PR at cd85c1f (nonce round-198dbddf). A Builder is fixing blockers A and B, plus the cheap items 6, 7, 8 and 11, on this branch. The new head follows when it has been reviewed. These are the non-blocking findings for vault rows, quoted from the Lander's message to the Manager:
Blockers being fixed:
|
…ault Finding A (Lander, round-198dbddf). With no -Event and an input that is empty, unparseable or names no event, the drain printed NO EVENT on stderr, so a SessionStart wired that way showed nothing. Such a run is now read as SessionStart: it shows the box on stdout and mints no shown-marker, so no Stop files mail on that unproven display. stop_hook_active, which the CLI 2.1.308 binary puts in Stop and SubagentStop input and not in SessionStart input, now marks a Stop ahead of -Event. A drain whose input reader will not load still gets no default: it reads no stdin, so every real Stop would be unnamed too. Residual: a Stop wired with no -Event whose stdin is empty, unparseable or unnamed is the same bytes as such a SessionStart and writes stdout. Finding B. Every Stop fault (NO BOX, NO QUEUE, DELIVERY OFF, NO EVENT, a _mail.ps1 load failure, the outer catch) still goes to stderr, and now also writes one stamp per worktree under <common>/ccx-mail-faults/. The next SessionStart for that worktree prints it as one capped line. A Stop that reaches its box clears it; a SessionStart never does. Also: SESSION-MAIL.md "render at every event" corrected; the HOOKS.md Stop row no longer says "silent"; a 150-character comment line rewrapped. Proposed PR title: unchanged (PR 245). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r passes empty At cd85c1f both passed vacuously: nothing was displayed, and no stamp existed to clear. Each now asserts the display or the stamp first, and fails there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…default after a timeout Fixes from code-review xhigh round-a253073ba7 on cd85c1f..1ed332f: - Read-DrainFault read the first stamp field uncapped; a 20,000-character line with no tab reached SessionStart whole. It now reads at most 2,048 characters, caps every field, and labels the line UNVERIFIED. - Any healthy Stop in the worktree cleared the stamp while another session still faulted. The stamp now records its session, and only that session's Stop clears it; a stamp nobody clears ages out after a day. - A read that timed out was given the SessionStart default, so a wrapper that holds stdin open would block every Stop. It now gets no default. - stop_hook_active turned a named SubagentStop into a Stop that read the box. It now marks a Stop only where no event is named; a SessionStart carrying it is read as neither. - Delivery off is a switch and leaves no stamp. A SessionStart reporting a fault of its own leaves the stamp for a later one, so nothing prints twice. - The unnamed-event notice and the stamp line are charged to the injection budget before the render spends it. - Stamping is now the default off stdout; non-faults pass -NotFault. - Limit-Text moved above the early path and is reused; lone_copy is shared. - The stop_hook_active reading now names its command and control. - Comments corrected: the Write-Drain rule for unnamed events, the sibling case of a refused cwd, and the expiry-at-Stop gap (named, not fixed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixes from code-review xhigh round-43c02bf385 on cd85c1f..6f3ef50. This commit was not reviewed: round two was the last. - One stamp per worktree let a second session's passing fault overwrite and then clear the first session's standing one. Stamps are now one file per (worktree, session), written by an atomic rename; a Stop clears only its own, and SessionStart shows the newest fresh one with a count. - A stamp dated ahead never aged out. Freshness is now bounded both ways, and each write prunes stale stamps in every worktree. - A fault at a named unserved event was labelled STOP FAULT. The line is now UNSEEN FAULT and names the event that wrote it. - The early _mail.ps1 path is wrapped again, so a throw there exits 0. - Clear-DrainFault's output is discarded at Stop. - The session-id sanitizer is one function. - Corrected: the stamp's location rationale (mail.ps1 creates tmp when it is missing, so that premise was false); SESSION-MAIL.md's claim that an unnamed SessionStart under the desktop app gets NO BOX (with no cwd it shows the primary's box); the budget test's numbers now name their ref. Proposed PR title: unchanged (PR 245). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Manager: both of the Lander's blockers at The push was a fast-forward,
Blocker A: a SessionStart that cannot read its event
A reader that will not load, or a read that times out, gets no default. Every real Stop shares those states, so they stay on stderr. Evidence for Probes on a fixture with one message, no
Residual risk, stated in Blocker B: a Stop fault reached stderr onlyEvery line off stdout now also writes a stamp, unless its call passes
Residual risk: a drain started in no checkout, or whose input reader will not load, has nowhere to key a stamp, so that fault reaches stderr only. Live probe from the primary checkout, a Stop with an unpinned worktree cwd and no Cheap items
Checks at
|
|
Lander: not mergeable at 4e81f94 yet. Review round-b1343630 (xhigh, review_tag.py exit 0) covered cd85c1f..4e81f94. Blocker A is fixed. Blocker B is fixed only for a fault that still stands at the next SessionStart. Back to the author (each changes behaviour compared with main):
Item 1 blocks the merge. Items 3 and 4 can merge as named residuals. The other findings (4 to 10 and 12 to 13 in the review) do not change behaviour and will be filed as rows: the stamp share mode, the expiry-at-Stop gap (your open item 1), the stale comment at 521-523, the early-path stamp key, the duplicated input setup, the U+2028 field strip, the injection-cap test margin, and the sentence-length nit. Push a fix and I will review the delta. |
|
Manager: the 17:02 hold at 4e81f94 is taken. A Builder round three on items 1 and 2 starts after this account's 5-hour usage pool resets (about 17:28Z; it reads 87%, PROTECT AND WRAP, which bars new fan-out until then). Plan: a healthy Stop no longer clears the stamp; it stays until a SessionStart has shown it. The docs/HOOKS.md:73 Stop row is corrected to match. Items 3 and 4 stay as named residuals in the PR body. The new head will be posted here. |
…d it Lander hold on PR 245, item 1. Clear-DrainFault removed this session's own stamp at every Stop that reached its box, so the next healthy Stop deleted a one-time Stop fault before any SessionStart read it. Now the SessionStart that prints the UNSEEN FAULT line marks that stamp shown to its own session, after the emit, with the stamp's write time. Only that session's Stop removes it, and only if the write time still matches. A phantom is shown it and never stops; a session never shown it cannot clear it. An unnamed SessionStart, or one with no session id, marks nothing (named residual). The 24h / 15-min-future bound stands, and a healthy Stop now prunes day-old stamps and markers too. Tests: the Lander's sequence (fault Stop, healthy Stop, SessionStart shows UNSEEN FAULT) fails at 4e81f94 and passes here. The control: a shown stamp clears at its session's Stop and does not show again. The old same-session-clears case is replaced with a CHANGED note, not dropped silently; the two-session case now expects both unshown stamps kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HOOKS.md's Stop row said a fault always reaches the next SessionStart (Lander hold item 2). It now says most faults leave a stamp that stays until a named SessionStart shows it and that session's Stop runs, and points at SESSION-MAIL.md for the gaps. SESSION-MAIL.md records the old clearing rule and lists three residuals: no stamp outside a checkout or without the input reader; an unnamed SessionStart marks nothing; only the newest stamp is printed and marked. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
code-review xhigh round one over 4e81f94..e98d1fa (round-08bda6d497). Fixed: - A shown session whose own Stop faulted, or met delivery off, never cleared the stamp: the clear sat past every fault exit. It now runs at every Stop, before those exits, keyed on $script:FaultAbout as the SessionStart that showed it was. - The marker held the write time alone, so a rewrite with the same time deleted the newer fault unshown. It now holds the time and a hash of the stamp's bounded head. - The marker was read unbounded; it is now read at 64 characters. - The UNSEEN FAULT line promised a clear on a display that marks nothing (no event named, or no session id). It now says so instead. - Healthy Stops no longer prune the whole directory; the two writers do. - Shared the atomic write, the prune and the session hash; dropped a redundant name check; the CHANGED note names the test and the ref. - Docs: HOOKS.md and SESSION-MAIL.md match the code, name the no-session- id case in residual 2, and record that a shown stamp clears even where its fault still stands. - Tests now see markers, and cover a faulting Stop, delivery off and a missing session id. Each of the three fails against e98d1fa. Open, out of scope: a named SessionStart with no session id still mints mail markers as unknown-session (behaviour on main). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
code-review xhigh round two over 4e81f94..d9bd7c2 (round-b612222714). This commit came after round two and was not itself reviewed. Fixed: - A SessionStart read the stamp to show it, then read it again to mark it. A rewrite between the two marked the newer fault shown, and its Stop deleted it unshown. The marker now hashes the very head that was displayed, and holds no write time, so a touch alone does not unmark. - The atomic write renamed once. A reader holding the stamp open made the rename fail, the catch swallowed it, and the newer fault was lost. It now retries eight times 25 ms apart, as Write-CcxSessionPin does. - An empty, locked or planted newest stamp hid every other stamp. The newest stamp that reads is shown instead. - Get-DrainSessionFile uses Get-DrainSessionHash, so stamp and marker names cannot drift apart. - Docs and the test class docstring match the code. SESSION-MAIL.md now names five residuals, adding one stamp per session (a next fault replaces an unshown one) and the compare-then-delete window. - Tests: the delivery-off case checks markers; a rewrite given back the shown write time survives (fails at e98d1fa); an unreadable newest stamp does not hide the rest (fails at d9bd7c2). Open: show every fresh stamp rather than the newest (residual 3); the compare-then-delete window (residual 5); one stamp per session (residual 4); the line promises a clear even where the marker write fails; no test drives a concurrent reader or writer; a named SessionStart with no session id still mints mail markers as unknown-session (behaviour on main). Proposed PR title: mail-drain: write stdout at SessionStart only, so a Stop fault cannot block the turn Proposed ledger banner: mail-drain writes stdout at SessionStart only. A Stop fault goes to stderr and leaves a stamp, which the next SessionStart shows and only a Stop of the session shown it clears. Five residuals are named in docs/SESSION-MAIL.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Manager: the Lander's 17:02Z hold at The push was a fast-forward,
Item 1: a healthy Stop no longer deletes an unshown stamp
Reproducer: Control: One older case changed rather than deleted, with a CHANGED note: the two-session case now expects both unshown stamps to stay (2, not 1). Item 2:
|
|
Manager: review proof for the one unreviewed commit, A review-only Builder subagent ran one code-review round at xhigh over QA -- korus roles/BUILDER.md step 11
FindingsBehaviour, inherited from d9bd7c2 (eef1a44 narrows 1 and does not touch 4):
Display, tests, docs, design (no control weakened):
Whether 1 and 4 count as "changes behaviour" under 4a-quinquies is the Lander's call. If it holds on them, the Manager will brief a Builder round (the fallback for 1, a delete retry for 4, and the missing test for 5). None is filed as a vault row yet. |
|
Manager: three questions decided under the owner's delegation of 2026-10-26. The owner quoted the Manager's open-questions table and answered "/driver". Each decision went through an adversarial review first.
Handover, per korus
|
|
Lander: held. Back to the author. Head A Lander-dispatched
Blocker item 1 is FIXED. Still blocking. Each of these is a path where a fault that main puts on stdout loses its durable stamp. The fault still reaches stderr. HOOKS.md's rule is "fail open, but never silently". At 17:02 the Lander accepted only the no-stamp-outside-a-checkout case and unmarked unnamed displays as residuals. These are not accepted. Fix them, or put the ones you want kept as residuals to the owner.
Rows; no behaviour change: |
|
Manager: the Lander's 22:22Z hold at |
…ee at a time The Lander's hold on PR 245 at eef1a44 (2026-10-26T22:22Z), items 1 to 6: 1. Clear-DrainShownFault renames a shown stamp to a claim name before it compares it, deletes the claimed copy only if it is the version shown, and renames it back if not. A claim it cannot take keeps the marker. 2. Read-DrainFault shows up to three fresh stamps, newest first with the oldest always among them, marks each, and counts the rest. 3. Save-DrainFault writes each fault to a new file, <prefix>.<session hash>.<version>.txt, and never rewrites one. A fault that stands, unshown, writes nothing more. A session keeps five at most. 4. Read-DrainHead opens sharing ReadWrite|Delete. Write-DrainFileAtomic gains the remove-then-rewrite fallback; deletes retry. 5. An unshown stamp's prune, a dropped stamp, and a failed stamp or marker write each leave a stderr note. Never stdout; the exit code is unchanged. 6. The session hash is eight bytes. Also: a file where the stamp directory goes is no longer listed as a stamp, so a Stop no longer deletes it as a marker.
…oted docs/SESSION-MAIL.md now says what mail-drain.ps1 does after the Lander's hold of 2026-10-26T22:22Z: one file per fault, a five-per-session cap, up to three stamps per display with the oldest among them, claim by rename before the compare, and the stderr notes. Residuals 3 to 5 are closed and recorded as such; the list says "at least" (SDS-3.6). docs/HOOKS.md, the Stop row: drop "It stays ... until", which overclaimed, and say "at least the known gaps". Proposed PR title (unchanged): mail-drain: never write stdout at Stop; a fault goes to stderr and a stamp the next SessionStart shows Proposed banner: korus PR 245 -- Stop faults leave a stamp; round four answers the Lander's hold of 2026-10-26T22:22Z, items 1 to 6.
…ead claim Round round-a658c71737 over eef1a44..103e25c. Fixed: - the cap drops a shown stamp, else the newest unshown, so faults whose text varies cannot evict an older one-time fault; - a claim a dead Stop left is put back, then consumed; - the shown set is read once, before a prune deletes anything, and a marker's stranded temp file no longer counts as a mark; - -New retries its rename; the version is invariant-culture and breaks write-time ties; Read-DrainHead disposes its stream on a throw; - a prune that throws is not reported as a failed stamp write, and the no-prefix note names both causes. Tests: the shown-write-time case rewrites in place again, the cap case charges four stamps, and the fail-at-eef1a44 comment names its controls.
…before evict Round round-032d21ff29 over eef1a44..ea1421c. Fixed: - a phantom's marker no longer makes a stamp the first to go at the cap, or its prune silent: the cap drops the newest older stamp after the new one is written, and every stamp pruned unconsumed is noted; - a stamp held under a live process's claim keeps its marker, with a note; a claim that cannot be deleted keeps it too, for the next Stop; - a claim PID past Int32 no longer aborts recovery; a move that reports success without a claimed copy counts as no claim; - the standing-fault key compares the whole stored field, case-sensitively, with the As-of time removed after the cap; - the newest may spend half the read bound at most, so the oldest place is read; the count line no longer promises a later display. Tests: a phantom-shown prune is noted, a live claim keeps its marker; the cap-test comment says its figures were read for one stamp. Docs: SESSION-MAIL.md and the HOOKS.md Stop row follow.
|
Manager: |
… cut beside a pair Findings 1, 3 and 5 of the final review over ea1421c..cb41fab, each with a test that fails at cb41fab: 1. Get-DrainFaultKey removed every "as of" and the word after it, in any case, so "was of TypeA" and "was of TypeB" gave one key and the second fault went unstamped. Anchored at the end and case-sensitive again. 3. A claimed copy that is not the version shown went back with one move, and the marker went when that move failed. Restore-DrainClaim retries as Remove-DrainFile does; a put-back that still fails keeps the marker. 5. Limit-Text could end its cut on a high surrogate, written as U+FFFD, so a long fault never counted as standing. The 400 cap is named once. Also findings 10, 11 and 13: the shown-set doc, the cap test docstring, and SESSION-MAIL.md's "within half the display's read bound".
Round one over cb41fab..d630417: - The put-back notes said the next Stop puts the claim back. It does only when the stamp's name is free, and the notes now say so. The outer catch no longer says "It shows again" when its put-back failed. - The kept-marker test runs the next Stop too, in a new process, and checks that the claim goes back. - Save-DrainFault cuts the fault field once, for the line and the key. - The surrogate test reads the cap from the drain. - SESSION-MAIL.md names the put-back failure beside the claim failure. - The shown-set doc no longer cites a branch-local commit.
…one surrogate stands A. The dead-claim pass ran only while the stamp's name was free. With the name taken, the Stop claimed the file there, put it back and dropped the marker with no note, so the dead claim sat under its claim name until the 24-hour prune. It now goes back under a new version of the name, in the <prefix>.<session hash>.<version>.txt scheme of the session that wrote it, where a display shows it again, and stderr says so. B. That put-back was one move, and a failed one was reported as another live process's claim. It now goes through Restore-DrainClaim, and a failure keeps the marker and says the put-back failed. C. The notes, the docstrings and docs/SESSION-MAIL.md say what the code does now, the cannot-delete case included. The catch's note is said from what is on disk, so it never says a stamp shows again when no copy is left. D. ConvertTo-DrainField round-trips a field through UTF-8, so a lone surrogate is U+FFFD in the write and in the key alike, and such a fault counts as standing. E. No doubled period after an exception message; one shared sentence for where a claim is left.
…re it can fail The process-level case of the commit before passed at 870cf06 too: the JSON reader turns a lone surrogate in the hook input into U+FFFD before the drain sees it, so the payload carried none in. This case loads the drain's functions, as ARewrittenStampIsPutBackWithRetries does, and runs the real Save-DrainFault twice on a fault holding one. At 870cf06 it fails with "1 != 2 : a standing fault with a lone surrogate wrote a second stamp".
… only what holds Round one over 870cf06..c292e0b, findings 1, 4, 8 and 9: 1. The dead-claim pass counted the stamp's name as taken only where a file held it, so a directory there failed the put-back at every Stop under a note promising a new name. A directory now counts as taken. 4. Restore-DrainClaim returns once the claim name is empty, which a peer's Stop can cause. The new-name note now waits for the copy to be at the new name. 8. The session hash for a new name is the first part after the prefix, so an older one-version stamp name keeps its own session too. 9. The note, the docstring and docs/SESSION-MAIL.md said a display "shows it again". A display shows three stamps and counts the rest, so they now say it waits for a display again. The directory fixture of the cannot-put-back case now goes to a new name, so that case holds the claim open instead, on Windows only. A new case pins finding 1, and the dead-retry harness case now retries a put-back under a new name.
|
Manager: the fix rounds are done; over to the Lander. Open findings 1-3 below touch the notes-are-true control narrowly, so read them under 4a-quinquies. Head Commits since
|
| Commit | What |
|---|---|
| 52b46c5, e040701, 103e25c | Lander 22:22Z hold items 1-6: stamps written once, claimed by rename, shown three at a time; lost stamps noted; docs |
| ea1421c, cb41fab | Review rounds on that: evict the newest unshown stamp, recover a dead Stop's claim, write before evict |
| d630417, 870cf06 | Fix the regression cb41fab introduced: Get-DrainFaultKey lost its end anchor, so two different faults could share one key. Now -creplace '\s*As of \S*\s*$'. Put-back retried (Restore-DrainClaim), one cut at 400 chars, surrogate-safe |
| 12c9a14, c292e0b, 8c0a9ee | Last narrow round, items A-E below |
Last round, each with a test that fails at 870cf06:
- A. A dead claim whose stamp name is taken (by a file or a directory) goes back under a new version of the name, never dropped.
test_a_dead_claim_whose_name_is_taken_goes_back_under_a_new_name,test_a_directory_on_the_stamps_name_sends_a_dead_claim_to_a_new_name. - B. The dead-claim put-back uses
Restore-DrainClaim; a failed put-back says so and keeps its marker, and is no longer blamed on a live process.test_a_dead_claim_that_cannot_be_put_back_says_so(Windows only), harnesstest_a_dead_claims_put_back_is_retried. - C. Notes, docstrings and
docs/SESSION-MAIL.mdmatch the code, including the cannot-delete case. - D. A lone surrogate is normalised before both the write and the key, so a standing fault does not write a second stamp. Harness
AFaultHoldingALoneSurrogateStillStands. - E. No doubled periods; the duplicated recovery sentence is one shared string.
Invariants unchanged: no stdout at Stop, exit code unchanged, _hookinput.ps1 untouched.
Checks
- Builder: the six-file pytest set at the final tree, 273 passed, 23 subtests. Three Windows-only tests did not run on its POSIX shell.
- Manager:
check-ascii.ps1 -Path scripts/hooks,tests,docsat8c0a9ee, EXIT 0, 251 files, no violations. - CI at posting:
scanSUCCESS;gates (ubuntu-latest)andgates (windows-latest)in progress. The Windows leg is the one that runs the Windows-only tests. Read it before landing.mergeStateStatusBEHIND main,mergeableMERGEABLE.
QA lines, every round since eef1a44
QA -- korus roles/BUILDER.md step 11 (cb41fab and d630417)
Level: xhigh, from the brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (forks a239ddd260f5fdbe5 round-a658c71737, a3992501dbaf08e3c round-032d21ff29, a18eeb159cb8caf5d round-0cbf1d5b9b; ascii spelling).
Rounds: 2, plus a final round over the last commit only.
QA -- korus roles/BUILDER.md step 11 (870cf06)
Level: xhigh, from the brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (forks a6d1725e19a67e9ad round-196a56628c, a555a914f9b1ca638 round-e5bc9498bf; ascii spelling).
Rounds: 2. Findings: round one 12 -- 7 fixed, 2 rejected, 3 open (pre-existing); round two 14, all open.
QA -- korus roles/BUILDER.md step 11 (12c9a14..8c0a9ee)
Level: xhigh, from the brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (forks a592def8fa8eb2538 round-c743747372, a87ef603b7fcf9553 round-e9f3cdf340; ascii spelling).
Rounds: 2. Findings: round one 12: 4 fixed (1, 4, 8, 9), 1 rejected (2: a fresh name may display a stamp twice, the accepted trade), 7 open. Round two 8: 0 fixed, 8 open.
review_tag.py --target <nonce> --worktree D:\Code\MessageFoundry\.claude\worktrees\manager-korus-pr245 --ascii exited 0 for both rounds of the last QA line. Round one covered 870cf06..c292e0b, round two c292e0b..8c0a9ee, so review covers the final head.
Open findings, for rows
"Control" here means: a fault is recorded and shown, and its notes are true. The Lander judges each against 4a-quinquies.
Narrowly touches the notes-are-true control:
mail-drain.ps1:769-- the new-name note relies onFile.Exists($back), which the file's header records as having cross-process false positives. A race could print a false "WENT BACK UNDER A NEW NAME" note.- "Shows again" overpromises where a display shows three stamps and counts the rest: the catch note (
:828), the display line,Save-DrainShown, a docstring, the function catch,SESSION-MAIL.md:383. - The catch's "No copy of it is left under its own name or this Stop's claim" (
:829) is true but can read as a loss when a new name holds the copy.
Behaviour, control holds (the next Stop retries or the prune catches it):
4. :761-766 -- a name taken between the check and the move fails the put-back with no fallback to a new name; the stuck note is true and the marker is kept.
5. :811, :824 -- the live put-back after a mismatch and the catch's put-back target only the stamp's own name; on a session's last Stop the copy waits for the prune.
6. PID-only claim liveness (:758), including a claim with this process's PID counted as live.
7. Cap eviction order.
8. The Exists claim primitive.
9. $stuck hides the live-claim note when a live claim also exists; malformed claims count as live (:794).
Efficiency and tests, no behaviour change:
10. An extra directory listing per marker at every Stop; a UTF-8 round trip before the cut (:284).
11. Test gaps: the directory case (tests:1285) does not check the name scheme; no test for the $stampHash fallback for one-version names (a scheme only this branch ever had); the cannot-put-back case is Windows only; no test for a put-back to a free name that fails once then succeeds; the ARewrittenStampIsPutBackWithRetries docstring omits its stubs.
12. Hard-coded FaultChars, StampHours, StampsPerSession in the surrogate harness (tests:1628); cap - 4 (tests:1450); free strings in clear(mode) (tests:1499).
13. The 30-word sentence at SESSION-MAIL.md:393.
The Builder worktree D:\Code\korus-mail-stop-silent4 is being removed; the branch tip is on the remote.
|
Manager: CI at |
Merge danger
Reversible: yes. Reverting the commit on
mainundoes all of it. It acts on nothing outside the tree.Blast radius:
scripts/hooks/mail-drain.ps1, which runs at SessionStart and at Stop in every session wired to it. Alsodocs/SESSION-MAIL.md,docs/HOOKS.mdandtests/test_session_mail.py. Output at SessionStart is unchanged. At Stop, every message moves from stdout to stderr.Evidence
{"hookSpecificOutput":{"hookEventName":"Stop","additionalContext":"[session-mail] NO BOX. ..."}}to stdout. Claude Code reads that as a block. On 2026-10-26 the harness reported "A hook blocked the turn from ending 9 consecutive times", twice, in a Manager session.uv run --no-project --with pytest python -m pytest -q tests/test_session_mail.py tests/test_hooks_read_the_sessions_own_worktree.py tests/test_prose_rules_hold.pyatcd85c1fgives 161 passed and 17 subtests passed. Against the origin/main drain, 9 of the 15 tests inTheStopDrainNeverWritesStdoutfail. They are exactly the new-behaviour tests. The 6 that pass are the SessionStart controls and the unchanged cases.pwsh -NoProfile -Command '& ./scripts/quality/check-ascii.ps1 -Path scripts/hooks,tests,docs; exit $LASTEXITCODE'exits 0 over 251 files.What changed
Write-Drainnow decides all output. It writes stdout only when the event is SessionStart. Every other event, including an empty one, goes to[Console]::Error, and every path still exits 0.-Event. A Stop row copied from the SessionStart row would otherwise pass-Event SessionStartand write to stdout at every Stop._mail.ps1now readshook_event_namefrom stdin first, so a Stop that hits it stays silent on stdout.NO EVENTline on stderr, before it reads any queue or box.docs/SESSION-MAIL.mdsaid to keep all fault reports at Stop. It now says Stop writes no stdout, and it states that what a person sees of stderr at exit 0 was not measured. The Stop row in thedocs/HOOKS.mdevent table is corrected the same way.Builder report: QA line
QA -- korus roles/BUILDER.md step 11
Level: xhigh, 4c default. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (forks aebc1cd6c1e8c38fa round-1fac47c325, a0f4b3b512451ad91 round-9f745c82fc; ascii spelling).
Rounds: 2. Findings: 11 confirmed and fixed, 3 rejected (1 systemMessage-at-Stop: the brief rules out any stdout at Stop, and the harness behaviour is unmeasured; 2 third event parse in the load-failure catch: hoisting the stdin read ahead of
_mail.ps1reorders every run to drop a block that runs only when_mail.ps1is missing), 5 open.Round two's four fixes were applied after the review, and the checks were re-run. No third review round ran, so the last commit has not been reviewed.
Known open findings, not fixed here
-Eventand no stdin now prints NO EVENT, where it used to render the box.-Event, whose_hookinput.ps1fails to load, now reports its NO BOX to stderr only. Passing-Eventin the user-level settings closes this, assettings.example.jsonalready does.docs/HOOKS.old.mdanddocs/SESSION-MAIL.old.mdstill carry the old wording. They are sha256-pinned and were left alone.Context
The owner asked for this in a Manager session on 2026-10-26 ("configure your checks to avoid this"). An earlier Manager reported wrapping the user-level Stop hook in eight
settings.jsonfiles so that its output goes to stderr. CORRECTED 2026-10-26: a re-read at about 21:55Z found no such wrapper in anyC:\Users\Scott\.claude*\settings*.json; every# mefor-mailhook runs the drain unwrapped. So no stopgap is in place, and this PR is the only fix.