amadan
Whole-branch review by Fable is in, and a fix round has landed at e412077. Verdict was approve with changes; the changes are done, the gate is green, and the branch is back to waiting on one ruling.
What the review found
It confirmed the implementation honours the spec — D1–D5 and §6 are all in the code as written, and both design-review defects are genuinely fixed rather than merely claimed (it checked, rather than trusting the spec's word for it). The hub cannot block a request goroutine, cannot leak, and cannot grow unbounded; the stream handler returns on every path it must; the committed bundle is byte-identical to a fresh build.
Then it found the two most embarrassing bugs on the branch, both mine, both in the tests written to guard the defects this whole slice exists to prevent:
1. The Docs-Seq regression test could not fail against the implementation it rejects. Every case asked with since == the current MAX — and max(since, nothing) and MAX(seq) agree on every one of those. The comment claiming "a MAX(seq) implementation returns 2 here and loses row 2" was simply wrong; the derived implementation returns 2 as well. The discriminating case is since > MAX, which is also the shape of the real TOCTOU: a client holding a seq the slice query cannot see. A guard-rail with nothing behind it is worse than no test, because it reads as coverage.
2. The client-side monotonic-cursor test was inert. Its "straggler" was delivered by settling a promise that had already resolved — a no-op. The assertions passed whether the module used max() or plain assignment, so the one client-side rule the spec says these tests must pin was untested.
Both are fixed, and both fixes were verified by breaking the code: I put the MAX(seq) implementation back and watched the new case fail with the intended message, then restored it; same for reverting max() to assignment on the client. A regression test nobody has watched fail is a guess.
3. A real defect, low severity. The save queue's post closure read the module-level currentLive rather than its own lifecycle's subscriber. attemptResync stops the old subscriber but cannot stop the old queue (save-queue.mjs has no stop, by design), so a POST issued before a resync and resolving after it fed noteOwnSeq to the fresh subscriber — whose cursor came from a GET predating that row. No interleaving loses visible content, but it breaks D3's stated invariant across lifecycles, silently. It now closes over a lifecycle-local reference.
Also fixed: a tab that loads already-hidden never fires visibilitychange, so it held a stream for the full idle window it never needed. And hub.go's comment justified not closing the channel by a race this code cannot have — broadcast holds the mutex from lookup through send. The decision stands; the reasoning was wrong, and AGENTS.md has a name for that.
One behaviour worth a decision, not a bug
A hands-free watcher goes stale at the first server TTL. Receiving events defers the idle close but cannot reopen after the server's 5-minute TTL cut, because the client treats every EventSource error — an expected TTL close included — as "drop it and wait for activity". A wall-display tab nobody touches shows five minutes of updates and then freezes until someone moves the pointer.
§6 arguably endorses this ("someone is there" implies input), which is why the review called it a product call rather than a defect, and why I have not changed it unasked at one in the morning. Worth knowing there is a clean fix if it matters: at error time EventSource.readyState distinguishes a close the browser intends to retry from a fatal one, so a TTL cut can be told apart from a dead server. That would mean relaxing §6.1's blanket "never rely on native reconnection", which was my rule and is deliberately stronger than it needs to be — es.close() does not trigger reconnection, so only server-side closes were ever the issue. Filed as task 18 rather than decided here.
Still blocking merge
The §9 ruling: amend the 2026-08-30 spec's §7.1 to describe the doc-seq protocol, or rule otherwise. The wider spec is untouched on this branch, so that is still open. Task 15.
One note for whoever reads the diff: main has not yet received the click-to-place-caret merge, so git diff main...worktree-live-sync includes that work too (show.html's flex/outline rules and placeCaretFromPage). The review read those parts as well and found them sound; they carry their own review round.