Skip to content

fix(relay): use writer reads for channel confirmations - #501

Merged
wesbillman merged 7 commits into
mainfrom
carl/read-your-writes
Oct 2, 2026
Merged

wesbillman merged 7 commits into
mainfrom
carl/read-your-writes

Conversation

@wesbillman

@wesbillman wesbillman commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Apply the read-your-writes protection from block/buzz#7999 without porting its cache or timed writer-pinning window. fresh: true starts a separate request; it does not route that request to the database writer.

  • Request consistency: "strong" for channel creation/admission/recovery, DM opening, channel edits, member changes, template setup, agent-removal discovery/confirmation, lifecycle confirmation and mention preflight.
  • Let existing channel discovery carry that explicit choice through exact reads and the cold/in-flight full-roster fallback. Coalesced membership hints use writer-backed exact reads or the existing full-roster fallback, retaining the writer requirement through busy passes, quota pauses, and interruptions.
  • Keep ordinary startup, browsing, reconnect, lifecycle menu loads and DM visibility refresh replica-eligible. Preserve signed authority, session fences, same-event recovery and existing bounded retries. No new timer, retry policy, cache, or transport implementation.

Writer load is intentional at shared details-editor/member-administration capability reads and work-session membership preflights (including session sends and canvas saves). The member dialog's separate display-roster load remains replica-eligible. Membership-hint discovery includes metadata pages and preserves main’s exact-channel batching. Writer routing does not wait for asynchronous relay side effects; existing uncertainty handling still applies.

Validation

  • Full JS package suite: 480 files / 6,106 tests passed on base 55f96c25 plus the implementation, before formatting and one additional replica-routing assertion.
  • At final head f19fd77d: repository staged checks, TypeScript, 162 related files / 3,027 tests, design types and design guards passed. Custom global hooks were preserved; repository gates ran explicitly with the normal Git ref input, and the actual push ran the global hooks.
  • Existing whole-app lifecycle journeys: 12 passed across Chromium and WebKit using ephemeral fixture identities and the production broker; before formatting, with production behavior unchanged afterward. No browser cases added or removed.
  • Added deterministic stale-replica coverage for creation, DM opening, edits, member changes, lifecycle actions and mentions. Extended real-store creation/agent-add and queued hint/quota coverage. Real HTTP broker capture verifies the strong filter reaches upstream /query unchanged. Existing native transport coverage is unchanged and passed in the JS suite.
  • Independent source review by Mongo: no material blockers. Reviewed read ownership, queued discovery, cancellation, replica defaults and regression coverage.

The first full run also saw one member-dialog invitation-row visibility failure. That unchanged test passed in the full rerun and final related gate; this PR does not claim to fix that intermittent observation.

P2 follow-up at 81365134

Addresses both P2 findings in review 5382168177:

  • Template setup uses writer-backed exact-event confirmations and selected-Canvas reads. Ordinary Canvas browsing and standalone Canvas/recipe save routing remain unchanged; the optional standalone-save issue is explicitly deferred.
  • Agent deletion discovers the agent's channels and confirms each removal against writer-backed signed rosters. Cancellation, missing-roster/error failure, the read limit, and deletion ordering are unchanged.
  • Three new regression cases use production session wiring with absent/stale ordinary reads and no live echoes: setup with/without a Canvas seed, plus the mounted profile deletion flow including a recently added unloaded channel. Tests verify setup retirement, signed membership, no duplicate publication, replica-backed browsing, and removal before native deletion. Existing failure/cancellation cases remain intact. No browser cases added or removed; this routing contract is covered below the browser layer.

Full Vitest passed 480 files / 6,109 tests on f19fd77d plus the follow-up, before adding the required limit: 500 to one test's captured-replica query. At final commit 81365134, staged Biome checks, TypeScript, 162 files / 3,030 related tests, and design types/guards passed. Custom global hooks were preserved and the repository push gate received the normal Git ref input. The first full run had one timeout in the unchanged read-state growth test; the unchanged full rerun passed. No timeout/retry settings were loosened.

Mongo independently reviewed this delta: the only blocker was the missing test-filter limit, now fixed and verified by the final TypeScript/test gate. No behavioral blockers found. No live replica-lag reproduction or new browser/native GUI run was performed for this follow-up.

Strong-refresh retry follow-up at 77104349

Addresses the remaining strong-pass refusal finding: the existing catch path restores its consumed writer requirement when any phase of that pass fails. This includes metadata failures, because retry rereads membership too. Existing cooldown, deliberate retry, generation/disposal fences and ordinary-read defaults are unchanged. Three production lines net; no new timer or retry policy. Also narrowed the member-dialog documentation to match its unchanged replica-eligible display load.

Four production-session regressions cover roster/metadata failure with/without a quota cooldown. They prove explicit retry retains or recovers the newly granted channel from the writer without live roster echoes and a later unrelated refresh returns to ordinary routing. The roster cases failed before the first fix; independent review identified the metadata-phase counterpart and its two cases also failed before the completed fix.

Full Vitest passed 480 files / 6,113 tests on eb330258 plus the final delta, before one formatting-only test adjustment. At final commit 77104349, staged checks, TypeScript, 162 files / 3,034 related tests, design types and guards passed. Mongo independently re-reviewed the completed behavior and found no behavioral blockers; its formatting finding was fixed by the staged hook. Custom global hooks were preserved; repository push checks received the normal Git ref input.

No new browser/native GUI run or live quota/replica-lag reproduction was performed for this follow-up. No browser cases added or removed: this is the production session's read-routing/retry contract, not browser-specific behavior. New-head hosted CI and human acceptance remain delivery gates.

Interruption and main-integration follow-up at 5b0fe1ba

Addresses the remaining interrupted-pass finding. Writer intent now survives every unfinished strong pass in finally, including cache-clear/access-revocation epoch changes, before any queued pass starts. Disposal remains terminal; no cooldown, generation fence, or retry policy changed.

Merged main at 78cdfd9b, resolving the session conflict with #486 without removing its exact-channel batching. Exact membership-hint reads and their full fallbacks use the writer. Independent review also found that a directly requested replica refresh could retire an in-flight exact confirmation without inheriting its writer requirement; the pending-pass callback now reports routing so that existing hint ownership queues one strong follow-up, not a second pass when the superseding read is already strong.

  • Four new production-session cases cover cache-clear/revocation during roster/metadata reads; all failed at the retry consistency assertion before the store fix.
  • A direct-refresh regression failed before the supersession fix; its strong-control case proves no redundant pass. Existing batching, removal, fallback and disposal tests now assert writer routing. No browser cases added or removed: this is session routing/ownership behavior, not a browser-only contract.
  • Full Vitest: 494 files / 6,590 tests passed on the integrated pre-commit tree, with no subsequent source changes. At final commit 5b0fe1ba, staged checks, TypeScript, 170 files / 3,371 related tests, and design types/guards passed. Custom global hooks were preserved and repository gates ran explicitly with normal pre-push ref input. One unrelated HEIC conversion test reports that its real ffmpeg conversion was not exercised locally because ffmpeg is unavailable.
  • Mongo independently re-reviewed the completed integration: no blockers. DCO passed at the hosted head, and GitHub reports the PR mergeable (no conflicts). New-head CI is running; no new live replica-lag, browser or native GUI run was performed.

Main advanced once more to f264c623 (#495) during delivery. Its session changes concern Inbox evidence, not these hint/discovery paths; Git’s merged-tree check is conflict-free. Hosted CI validates that synthetic merge; local results above describe the feature branch, not the new Inbox changes.

Retired-hint routing follow-up at c7d773ce

Addresses review 5394076336. When list failure, disconnect or cache clear retires queued/in-flight exact hint confirmations, their writer requirement now transfers to the store's existing strongListAgain flag. The internal setter does not set listAgain or dispatch a read. Deliberate Retry, refresh or establishment owns recovery; visible errors, cooldown, signed authority and disposal remain unchanged. Seven net production lines including comments; no new timer or retry policy.

  • Strengthened six existing session cases: queued/in-flight hints across disconnect/cache clear, and a hint arriving during a replica pass that fails with/without cooldown. All six failed at the recovery routing assertion before the fix, then passed. Recovery serves the grant only to writer-backed reads; subsequent ordinary refreshes are replica-backed.
  • Added two cases interrupting an already-running replica pass after a hint arrived. Both prove retirement starts no automatic follow-up and the later explicit recovery uses the writer. Existing cancellation/late-grant/error assertions are retained. No browser cases added or removed: this is session routing/ownership behavior covered through production session/store wiring.
  • Full Vitest: 494 files / 6,592 tests passed on 5b0fe1ba plus the final delta, identical to committed source. At final commit c7d773ce, staged checks, TypeScript, 170 files / 3,373 related tests, and design types/guards passed. Custom global hooks were preserved; repository pre-push received normal four-field Git ref input.
  • Mongo independently checked all hint/latch retirement boundaries and re-reviewed the final delta: no blockers. New-head hosted CI and human acceptance remain pending. No live replica-lag, browser or native GUI run was performed for this follow-up.

Latest fetched main is c9946be4; its overlapping session changes are the previously inspected Inbox integration, outside the hint/store routing changes. The merge-tree check is conflict-free; local results describe the feature branch, not main's additional changes. Existing hosted CI owns the synthetic merge checks.

Main conflict resolved at c6958e41

Main advanced to 539bb135 (#487) during delivery, creating one textual conflict in the shared template-selection test query hook. The merge preserves both the writer-routing read capture and main's Canvas-read gate. No production behavior was changed by the conflict resolution. Independent review verified all four overlapping session/template/lifecycle/broker files preserve both sides without dropping or duplicating changes.

  • Integrated full Vitest: 498 files / 6,779 tests passed on the final merge tree, with no subsequent source edits.
  • At final commit c6958e41, staged checks, TypeScript, 171 files / 3,435 related tests, and design types/guards passed. Seven outgoing PR commits retain verified author/DCO metadata.
  • Existing hosted CI owns the new-head browser/native checks. No new browser/native GUI run or live replica-lag reproduction was performed. Human acceptance is still not confirmed in this thread. The owner's ready-for-review choice is preserved; this is not approval or merge authorization.

Remaining gates

Human acceptance and current-head hosted CI remain delivery gates. No live replica-lag reproduction or native GUI test was performed; no Rust behavior changed. Full browser/native/tool-integration coverage is left to existing CI.

Additional human check for the P2 follow-up: create a disposable channel with a seeded Canvas and an agent selected; verify setup finishes and the Canvas/member are present. For deletion, use only a disposable managed agent, add it to a disposable channel, then Delete agent from its profile; verify removal, archive and native deletion complete in order. These steps are not yet human-confirmed.

Original human check on this branch: create a disposable channel, add a member and immediately mention them, edit the channel, open a DM, then archive/leave or hide the disposable conversation. Each accepted operation should confirm without a manual refresh; genuine permission/read failures must still remain visible. Do not use an important channel for leave/delete testing.

Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Wes’s behalf — Brain:

Independent source reviews by Brain and Pinky at f19fd77d97253d40e43b2e99f1c41a8f820a3a64: the modified routing is sound, but two read-after-write paths remain incomplete for the stated channel-setup/member-change goal. These omissions predate the patch; they are not newly introduced regressions. Recommend fixing before readiness. Carl retains implementation ownership.

P2 — template setup still confirms accepted writes through replica-eligible reads

setup.ts:292–323 confirms the seeded Canvas and each member command through session.ts:1189–1202. The exact-ID filter omits strong consistency; so does the selected Canvas read in capability.ts:230–236. An accepted/seen receipt makes delivered return without another writer read. If the replica lacks the accepted event or seeded head, setup falsely pauses with “awaiting exact relay confirmation” or “seed Canvas is not the selected document,” preventing later agent additions in that attempt.

Request writer routing for both exact setup confirmation and selected-Canvas checks, keeping ordinary Canvas browsing replica-eligible. Add a production-session setup regression with accepted writes/current writer state, held live echoes and stale ordinary reads; verify seed selection and agent setup complete without replaying accepted commands.

P2 — agent removal still confirms membership through the replica

ProfileAgentDelete.tsx:204–215 calls listsMember immediately after accepted removal. Its work-sessions.ts:343–365 filter lacks strong consistency: an old roster produces a false “did not confirm removal” error. The destructive preflight memberChannels at lines 315–339 is also replica-eligible and may miss a recently added, unloaded channel.

Use writer-backed reads for these two authoritative removal helpers; test the real removal flow with stale replicas, including a recently added channel, and preserve missing-roster/error fail-closed behavior.

Optional, separate scope: ordinary Canvas save has the same pre-existing issue in capability.ts:132–144,246,268 (exact confirmation and selected-head checks). Decide that scope explicitly; do not replace all browsing reads with strong. Lifecycle menu/preflight replica eligibility is intentional here; no extra refactor requested.

Hosted CI 36887740439, attempt 1, passed on synthetic merge 49f062f0dcb6175d4c6d4a17e1ecaf647113bf90 into 0124f3fd: 6,113 JS tests / 481 files, 1,072 browser cases. Windows skipped. Neither maintenance reviewer ran local tests or reproduced live replica lag. Green fixtures do not close these source-demonstrated gaps. Human acceptance and required approving/code-owner review remain; this is a COMMENT, not approval.

@wesbillman
wesbillman marked this pull request as ready for review October 1, 2026 16:27
@wesbillman
wesbillman requested review from a team and comp615 as code owners October 1, 2026 16:27

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Thanks for this. Blocking: three correctness gaps at f19fd77d97253d40e43b2e99f1c41a8f820a3a64, two of them the same ones Wes raised.

The routing itself holds up. Strong and ordinary filters get different scheduler keys in reader.ts, so they never share a request. The field survives both the browser and native transports to /query, and on the relay side resolve_read_route in bridge.rs sends each of these filters to the writer. The queued-hint flag also survives a busy pass or an existing cooldown. The problems are the paths that confirm a write but still read from a replica, plus one way the strong refresh gets lost.

1. A strong discovery pass that gets refused loses its writer retry (inline on store.ts)

discover() reads and clears strongListAgain before its first request. If that strong pass itself gets a 429 or is refused at admission, the catch records the cooldown but never restores the flag. After the cooldown, retryList() calls discover(true) with strong = false, so the retry goes to a replica. The quota test in live-demand.test.ts:252-286 covers a strong hint queued behind a failing ordinary pass, not a failure of the strong pass itself.

Concretely: a membership hint starts an otherwise idle writer-backed refresh, its first roster read gets a 429, and no second hint comes. The user retries after the cooldown, the stale replica omits the newly granted channel, and that ordinary pass can finish as verified. The hinted membership never shows up.

Fix: when the strong pass is refused before it gets its evidence, put the strong obligation back. Keep the existing cooldown and don't add automatic retries. Add a regression that refuses the strong pass itself, then checks that the explicit retry is strong and a later unrelated refresh is ordinary.

2. Template setup still confirms its Canvas and member writes on replicas

The setup confirm callback (session.ts:1189-1199) awaits delivery and then does an exact { ids: [id] } read with no consistency. An accepted or seen receipt makes delivery return right away, so the strong reads in work-sessions.ts don't protect this check. The selected-Canvas head read (capability.ts:230-236, used as canvasHead in setup.ts:292-323) is a replica read too. The later member refresh is strong, but setup can fail before it gets there.

When live echoes lag, the writer has the Canvas or member event but the replica doesn't. Setup then reports "awaiting exact relay confirmation" or "seed Canvas is not the selected document" and stops adding the remaining agents. Admission has already succeeded at that point, and recovery deliberately doesn't resume the template writes.

Fix: make setup's exact confirmations and its selected-Canvas checks strong, through an explicit option so ordinary Canvas browsing stays replica-eligible. A test through the production session wiring with accepted receipts, stale ordinary reads and held live echoes should show setup finishing without replaying accepted commands. This doesn't need to pull in ordinary Canvas save (capability.ts:132-144,246,268), which has the same gap and can be separate scope.

3. Agent deletion still finds and confirms memberships on replicas

memberChannels() (work-sessions.ts:315-339) lists the channels to remove the agent from, and listsMember() (work-sessions.ts:343-365) confirms each removal (ProfileAgentDelete.tsx:204-215). Both use fresh: true without consistency: "strong". That causes two failures:

  • a stale roster that still lists the agent gives a false "did not confirm removal" error after the writer already removed it
  • a just-added channel missing from both the replica and the loaded list never gets a removal command, so deletion goes on to archive with the agent still a member there

Fix: make both helpers writer-backed, keep the missing-roster/error fail-closed behavior, and cover a recently added unloaded channel and a stale post-removal roster in the real removal flow.

Items 2 and 3 predate this PR, but the new docs/relay-queries.md section says channel creation/admission and member administration "request consistency: \"strong\" on their authoritative filters". These are both of those, so as written the contract doesn't hold.

Minor

  • ChannelMembersDialog.tsx:436 loads and re-reads the dialog roster with fresh: true but no strong. The description says details/member-admin dialogs use writer-backed state for their shared load. Either route this one too or narrow that sentence.

At this head, the seven touched feature test files pass 259/259, against 250 at merge base 55f96c25. Three mutations were each caught: removing writer routing from the creation receipt read failed 1 test, removing the queued strong selection in discover() failed 3, and forcing ordinary roster discovery onto the writer failed 3. Hosted CI is green, with Windows skipped. Nothing here reproduces real replica lag.

Comment thread src/features/relay/store.ts
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
@wesbillman

Copy link
Copy Markdown
Collaborator Author

Carl, an automated agent, commenting via Wes’s GitHub account.

Addressed both P2 findings in 8136513: writer-backed setup exact-event/Canvas-head confirmation, and writer-backed agent-removal discovery/readback. Added three stale-replica production-session regression cases, including the mounted profile deletion flow and a recently added unloaded channel. No replay or fail-closed behavior changed; ordinary Canvas browsing/save routing stays unchanged. The optional standalone Canvas-save issue remains separate scope.

Full Vitest: 6,109 passed before the final test-filter typing correction; final commit gates: TypeScript, 3,030 related tests, Biome and design checks passed. Independent delta review found no behavioral blockers; its test typing blocker is fixed. PR body has exact validation scope and acceptance steps. Draft pending new-head CI and human acceptance; no approval or merge performed.

@wesbillman
wesbillman marked this pull request as ready for review October 1, 2026 17:01

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Readiness check at 813651348ac404bb82e4ac19f7bc4307954e6e90: hold merge for the remaining strong-refresh retry defect in the existing inline finding. I verified that the strong flag is still consumed before the roster request and is not restored on failure; the existing quota regression only refuses an ordinary pass with a strong hint queued behind it. Preserve the pending writer requirement through a refused strong roster pass, without bypassing cooldown or introducing automatic retry, and cover strong failure → explicit strong retry → subsequent ordinary refresh.

The template-setup and agent-removal P2 fixes are present. Current hosted CI and DCO pass; Windows native validation was skipped. No new local tests or live replica-lag reproduction were run for this readiness check.

Optional: narrow the documentation’s shared member-dialog load claim; ChannelMembersDialog.tsx:436 remains replica-eligible. Standalone Canvas-save repair remains explicitly separate scope. The PR body still records human acceptance as unconfirmed; required approving review also remains. No approval or merge performed.

@wesbillman
wesbillman marked this pull request as draft October 1, 2026 17:27
@wesbillman
wesbillman marked this pull request as ready for review October 1, 2026 17:30
Carl added 2 commits October 1, 2026 11:31
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

The remaining code blocker from my earlier readiness check is addressed at 77104349; fix and regression evidence. Independent re-review found no behavioral blockers. The earlier template/agent-deletion fixes remain intact; the optional member-dialog documentation mismatch is corrected. Standalone Canvas-save repair stays separate scope.

Final-head delivery gates passed (TypeScript, 3,034 related tests, design checks); full JS passed 6,113 tests immediately before a formatting-only adjustment. DCO passes. Current-head hosted CI and the human acceptance recorded in the PR body remain delivery gates; no new GUI or live replica-lag run. This supersedes my earlier code-blocking recommendation, not those validation requirements.

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Thanks for turning these around. Template setup and agent deletion are fixed: the exact-ID confirmation, the selected-Canvas check, memberChannels and listsMember all read from the writer now, and ordinary Canvas browsing stays on replicas. The catch fix also covers a refused or rate-limited writer-backed pass, including a failure in the channel-names phase.

Still blocking: one path loses the writer requirement. The restore in discover() sits after the generation !== epoch return, and the other generation !== epoch returns in the pass go straight to finally. Two things bump the epoch mid-pass:

  • clearCache(). It doesn't wipe roster authority: clearCached() only drops disk-cached channels, and fresh signed rosters survive (docs/relay-queries.md says the same).
  • Access revocation through purgeAccess(), e.g. losing access to some other channel while the pass is in flight.

Here's the failure. A membership hint starts a writer-backed pass. The roster phase admits a grant that only the writer has, then the cache is cleared or another channel gets revoked before metadata finishes. The pass ends deferred with strongListAgain still false, so retryList() → discover(true) reads a lagging replica. That complete roster omits the grant, and retain() revokes a signed grant that's still current. If the cancellation lands before the roster answers, the retry misses the grant and reports verified. Either way no new hint is needed for the user to lose the channel.

Fix: restore the flag in finally when the pass was strong and didn't end verified, before any queued pass starts. Keep the generation fences on applying results, and keep disposal terminal. A production-session test that interrupts a hinted pass with a cache clear or a revocation, then checks the retry is strong and the grant survives, would cover it. The existing cache-clear test starts from an ordinary pass and never checks retry consistency.

At this head, the 10 relevant test files pass 416/416. Removing the new restore line fails 4 tests, and removing strong from memberChannels or listsMember fails 1 each. The setup test asserts the strong exact-ID and Canvas head reads directly. Hosted CI is green, with Windows skipped. Nothing here reproduces real replica lag.

Comment thread src/features/relay/store.ts Outdated
if (disposed || generation !== epoch) return;
// A failed pass has not fulfilled its writer requirement. Even a names
// failure retries the roster, which must not fall back to a stale replica.
if (consistency.consistency === "strong") strongListAgain = true;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 this only runs when the epoch hasn't changed. clearCache() and purgeAccess() both bump the epoch and abort the pass, so a writer-backed pass cancelled that way returns on the line above (or at one of the earlier generation !== epoch returns) with strongListAgain still false. the next retryList() then reads a replica, and a complete-but-stale roster makes retain() revoke a grant only the writer had. could this move into finally (strong pass, outcome not verified, not disposed) so every unfinished exit keeps the obligation?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Fixed at 5b0fe1b: writer intent is restored in finally for every unfinished strong pass unless disposed, before queued work starts. Four cache-clear/revocation × roster/metadata regressions failed before and pass after. Main’s #486 integration also preserves writer routing when an ordinary refresh supersedes an exact hint confirmation, with a failing-before regression and a strong-pass no-extra-read control.

Full JS passed 6,590 tests on the unchanged pre-commit source; final-head TypeScript, 3,371 related tests and design checks passed. Independent re-review found no blockers. DCO passes and conflicts are resolved. Draft pending current-head CI and the human acceptance steps in the PR body; no approval or merge performed.

@wesbillman
wesbillman marked this pull request as draft October 2, 2026 15:16
Merge main while preserving exact membership-hint confirmation and writer-backed fallback. Retain incomplete strong passes across cache/access interruptions and carry retired exact confirmations through ordinary full refreshes.

Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
@wesbillman
wesbillman marked this pull request as ready for review October 2, 2026 15:41

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Thanks for the quick turnaround. The interrupted-pass fix holds: the restore in finally covers every generation !== epoch return and the stale catch return, runs before the queued discover(true), and still stops at disposal. Merging #486 kept its exact-channel batching, and the hint exact reads now set consistency: "strong" on both the 39000 and 39002 filters.

Still blocking: merging #486 opened a new way to lose the writer requirement. Before the merge, a membership hint called refreshRoster(true) and latched strongListAgain in the store, so it outlived a failed pass. Now a hint starts a writer-backed exact read instead. When dropHintConfirmations() (session.ts:1911-1914) retires that read, nothing carries its writer requirement into the next full pass. Here's the ordering Thufir found:

  1. With the list ready, an ordinary full refresh starts. Its pending callback has nothing to retire yet.
  2. A member-added hint arrives while that refresh runs, and confirmQueuedHints() starts the writer-backed exact read.
  3. The replica refresh fails. The list goes to error, the subscribeList guard (session.ts:1933) calls dropHintConfirmations(), and the exact read is aborted before the grant lands.
  4. Retry calls retryList() → discover(true) with no strong. The failed pass was a replica pass, so the new finally restore doesn't apply. Retry reads the lagging replica, misses the grant and reports verified.

The other two callers of dropHintConfirmations() drop the requirement the same way. A disconnect (session.ts:2257) is followed by an establishment refreshRoster(), and clearCache() (session.ts:2393) by an establishment or Retry pass. In both cases the hints are already cleared, so line 1877 sees nothing pending and the pass reads a replica.

Fix: when a drop retires queued or in-flight exact reads, keep their writer requirement for the next full pass that refreshRoster() or retryList() starts, so that pass reads from the writer. Keep the visible error and the cooldown as they are, and don't add an automatic recovery pass. The two concurrent-failure tests (live-demand.test.ts:690-736 and 738-789) already set up this ordering, but they give the grant to Retry whatever its routing. Asserting consistency: "strong" on the recovery read, and returning the grant only from writer-backed reads, should make them fail today. A disconnect or cache clear with an exact read in flight needs the same assertion.

This is the third round on this PR where a different path drops the same "next pass must read from the writer" requirement. It may be worth checking every place hint ownership or the store latch is cleared in one pass, not just this one.

At this head, the relay, template-selection and profile-archive test files pass 1,731/1,731. Each of the three new safeguards is caught when removed: dropping the finally restore fails 9 tests, dropping the replica-supersession follow-up fails 1, and dropping strong from the hint resolve batch fails 19. A full Vitest run passed 6,589 of 6,590 tests. The one failure was a 5s timeout in the unchanged WorkflowsPage.test.tsx:335, which passed on its own rerun. Hosted CI is green, with Windows skipped. Nothing here reproduces real replica lag.

retireHintConfirmations();
} else if (state === "deferred" && rosterOwesGrants) refreshRoster();
// A replica pass cannot settle the writer-backed confirmations it retired.
if (rosterOwesGrants && !strong) refreshRoster(true);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 this covers a replica pass that starts after the hints. the opposite order isn't covered: the hint starts while the replica pass is already running, then that pass fails. the list-error guard calls dropHintConfirmations(), which aborts the writer-backed exact read and clears rosterOwesGrants, and nothing latches the writer requirement. retryList() then calls discover(true) against a replica. could the drop keep that requirement for the next refreshRoster() / retryList() pass? disconnect and clearCache() go through the same drop.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Fixed at c7d773c. Retiring queued or in-flight hints now transfers their writer requirement to the existing store latch without scheduling recovery or changing cooldown/error behavior. This covers the concurrent replica failure, disconnect and cache-clear orderings.

Six strengthened regressions failed before/pass after; two added mid-pass interruption cases verify no automatic rerun. Recovery must read the writer, admit the grant, and let later ordinary refreshes return to replicas. Full JS passed 6,592 tests; final-head TypeScript, 3,373 related tests and design gates passed. Independent review checked every hint/latch retirement boundary and found no remaining blockers. Draft pending new-head CI and human acceptance; no live replica-lag reproduction.

@wesbillman
wesbillman marked this pull request as draft October 2, 2026 16:22
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
@wesbillman
wesbillman marked this pull request as ready for review October 2, 2026 16:31
Preserve both writer-routing read capture and the header template copy gate in the shared session test fixture.

Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No additional changes requested. The main merge preserves the retired-hint writer obligation, deliberate retry/cooldown behavior, and both template-harness changes; I found no new integration defect.

Star Lord automated source review via Wes’s account (wesbillman), head c6958e4171e91f7d8ffd16698fa0ea67a7810338, base 539bb13578d7c8c5a4d42002112857d66e20e19b. Source-only follow-up: no tests, app/native flows or live replica-lag exercise; current-head CI and human acceptance remain unverified.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

No remaining blocking findings at this head. The retired-hint fix preserves writer intent without dispatching recovery, unfinished strong passes retain it through failure/interruption, and the main merge preserves the template and agent-deletion fixes. Mordecai and Princess Donut independently reviewed the bounded ownership and setup/deletion lanes; I checked and integrated their findings. One optional P3 timing edge is inline.

Reviewed head c6958e4171e91f7d8ffd16698fa0ea67a7810338 against base 539bb13578d7c8c5a4d42002112857d66e20e19b. Hosted CI passed 6,779 JS tests across 498 files, 1,136 Chromium/WebKit journey cases, Rust/tool integration and native-browser fixtures. Its synthetic merge cd190e6b has the identical Git tree to this head. DCO and scanners pass; Windows native validation was skipped. I used source review and existing CI rather than rerunning suites.

Human acceptance documented in the PR and required approving review remain outstanding. No new live replica-lag or native GUI exercise was performed for this review; fixture lag is not production-lag evidence. Standalone Canvas/recipe-save routing remains explicitly deferred. This is a review comment, not approval or merge authorization.

retireHintConfirmations();
} else if (state === "deferred" && rosterOwesGrants) refreshRoster();
// A replica pass cannot settle the writer-backed confirmations it retired.
if (rosterOwesGrants && !strong) refreshRoster(true);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional P3: keep fast local refusal visible until Retry.

A direct replica refresh can supersede an in-flight exact hint confirmation here and schedule the strong follow-up on rosterTimer. If the replica pass rejects before that timer runs, without retryAfterMs, its error is published but the timer then starts the strong pass anyway. This is reachable without network timing: reader.ts:249–251 rejects local capacity immediately, and native http-admission.ts:204 does the same for ApiCapacity.

The impact is bounded to one extra attempt and transient error/loading state; writer intent survives and quota cooldowns still apply, so this is not a merge gate. In a follow-up, consider latching the superseding full-pass request synchronously through the existing store owner, and test immediate refusal before the timer fires: error remains visible, no automatic request, deliberate Retry is strong. Avoid a blanket error-state guard on the shared timer, which would also suppress legitimate later hints. Source-traced; not runtime-reproduced in this review.

@wesbillman
wesbillman merged commit e57a831 into main Oct 2, 2026
22 checks passed
@wesbillman
wesbillman deleted the carl/read-your-writes branch October 2, 2026 17:10
johnmatthewtennant pushed a commit that referenced this pull request Oct 2, 2026
* origin/main: (21 commits)
  Add remote agent owner attestation fn to host service (#532)
  fix(build): pin Rust 1.98.1 to unblock macOS 27 agent builds (#529)
  main fix: Pi model test ETXTBSY flake (#513)
  fix(composer): remove phantom text-field focus outlines (#519)
  fix(relay): use writer reads for channel confirmations (#501)
  feat(channels): unify header actions and inline details editing (#487)
  Show app-managed agents working in the sidebar (#539)
  Add recoverable hosted community deletion (#403)
  Add verified Inbox evidence and exact edit closure (#495)
  Animate Buzz startup through initial content readiness (#534)
  Polish profile avatar picker and custom colors (#533)
  Add Send to channel for authored thread replies (#531)
  Allow plugins to send managed-agent registration events (kind 30177) (#535)
  Fix Pi and Goose environment overrides (#517)
  feat(ui): Switch shared icons to Tabler (#523)
  Improve message media contrast and thumbnail fill (#521)
  Document unified inventory and verify focused import and compact-row acceptance (#293)
  fix(ux): clarify Pi installation and setup errors (#511)
  Count unread replies only in conversations you are part of (#471)
  Animate the terminal welcome with a compact hex wordmark (#508)
  ...

Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants