Skip to content

feat(relay): isolate optional presence on the shared broker transport - #47

Closed
wesbillman wants to merge 6 commits into
mainfrom
brain/presence-transport-review
Closed

wesbillman wants to merge 6 commits into
mainfrom
brain/presence-transport-review

Conversation

@wesbillman

@wesbillman wesbillman commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator

Implemented by Brain with Pinky, submitted on behalf of Wes.

Scope

Transport prerequisite extracted from #39 at the broker/shared-socket ownership boundary. #39 originally contained transport and feature together; the prerequisite was split out later, which is why the higher-numbered #47 must merge first. Both existing PRs and their review history are preserved.

Merge order: #47#39. #47 targets main; #39 targets this transport branch. After #47 merges, retarget #39 to main and refresh its comparison/checks before merging. Neither PR has been merged or approved by this repair.

  • Add bounded optional snapshot admission and same-socket presence observation/publication, preserving ordinary read/write capacity and start clocks, shared relay cooldown, cancellation/signing ownership, correlated WebSocket OK receipts and the 1,024-wire ceiling.
  • Advertise complete presence only from the production broker (presence === true && live === true). The unused direct-signed HTTP presence extension/parity tests from Add bounded session-owned presence with foreground-safe admission #39 are not included; ordinary signed reads/writes remain supported and runtime priority cannot spoof optional admission.
  • Reuse the existing real-HTTP broker/policy-relay fixture for combined quota proof across eight callers; retain deterministic exact-clock owner tests and strict host-response accounting.

Non-goals: no session/community presence activation, UI indicators, activity tracking, renewals, extra adapter framework, server/storage change or claim of live old/new-client coexistence. The existing shared live owner reserves two presence slots; ordinary capacity is 1,020 channels (1,019 with the agent observer). Live Retry may send an empty author control, but no presence REQ/EVENT is created without demand.

Review repair — 2026-09-14

Current head: f94a5fbd235774195e4c928c799dd57f86971826.
Both P2 review findings and the P3 disconnected-Retry follow-up are addressed:

  • Bounded shared-stream body is 160,000 bytes for a maximum compact producer body
    of 159,747 bytes: 1,024 × 128-character channels, 64 priorities, 256 authors and
    a maximum safe-integer observer. Real HTTP regression verifies 200 plus ordinary
    signed traffic/limited routes/observer toggles; malformed and over-budget requests
    still reject before socket allocation.
  • Browser stream replacement, loss and terminal failure invalidate separate presence
    readiness. Held headers, stale-generation frames, terminal HTTP statuses, EOF/error
    reconnect exhaustion, and explicit retry require fresh matching presence-state
    evidence; generic connected frames cannot restore readiness.
  • One explicit Retry clears exhausted presence even before reauthentication.
    Automatic reconnect still preserves its exhaustion cap.

Exact reviewed diff committed unchanged: SHA256
b40a41f6a37b4aee01e71fe9c64e6680388e2d3e4d0d7608514939235aaed4db.
Pinky independently found no new blocker (source review, not approval).
pnpm check, 65/65 tests across the three full affected owner files, and
6/6 Chromium/WebKit live journeys pass. Seven isolated production mutants
fail the relevant assertions, with byte-exact restoration: old/absent body cap,
omitted connecting/retrying/error invalidation, authenticated-only explicit Retry,
and automatic reconnect bypassing exhaustion. The old cap produces an actual HTTP
413 versus expected 200. Normal pre-push passed 729 tests / 61 related files.
All PR commits have author-matching DCO trailers; hosted checks at the new head
remain a separate gate. No full local suite, native GUI, approval, or merge.

#39 consumes this transport head at 4425fa6, with a separate retired-publication
404 fixture-evidence repair; production transport is unchanged there.

Hosted snapshot 2026-09-14 02:20 UTC: all 12 listed checks pass at f94a5fb,
including CI required and DCO (run 34798024255).
Conflict-free against main; human/code-owner review still required. GitHub currently
marks #47 ready for review; this repair did not change its draft state. No approval
or merge performed. Existing evidence limits below are unchanged.

Prior repair and validation — 2026-09-13

Current head: 33b699e0ceea6b0ff0b39830ff76e56bef31d60b, including main
57796f034f290fbb5d7ee4fc41807f5c784591ff via a merge commit, without rewriting history.

  • Resolved main conflicts while preserving notification replay/live provenance through
    WS → broker traffic SSE → session. Ordinary SSE excludes presence/observer telemetry;
    exact-message fixture lookup remains community-scoped.
  • Repaired the shared HTTP-only fixture implicated in Add bounded session-owned presence with foreground-safe admission #39's two five-second timeouts:
    the Node wrapper requests four distinct genuinely signed retained records rather
    than signing 1,440 unrelated history records per case. Browser defaults remain
    640/80 per community; fresh keys, real HTTP/broker/policy wiring, assertions,
    five-second limits and the entire minute-long quota driver remain intact.
  • Publication failures are observed immediately; finally aborts owned stream/request
    and drains the original promise without replacing the failure. Two real-HTTP failure
    controls assert preserved errors and stream closure before teardown. They do not
    independently prove request cancellation, since stream disposal can settle the request.
  • Local profiling of the complete fixture file measured history signing at 1.728–1.956s
    before and 5.85–8.29ms after. This supports excessive setup as the leading diagnosis,
    not a reproduced proof of the hosted incident's cause or a demonstrated broker deadlock.
    Original failure: JavaScript job.
  • pnpm check passed; 48/48 tests across the complete fixture, live and broker-live
    files
    passed on the repaired tree. The normal push hook at committed 33b699e
    passed TypeScript and 719 tests / 61 related files.
  • Seven isolated mutations were caught: both WS/SSE special-event exclusions and
    provenance paths, compact-history use, immediate rejection observation, and stream
    failure cleanup. Probe restoration was byte-exact. Pinky's independent source-only
    review found no blocker at the exact diff committed here.

Hosted snapshot, 2026-09-13 23:13:55 UTC: GitHub reports MERGEABLE against main.
At this head, JavaScript, Rust/tool, Windows native notifications, measurements,
both Chromium shards, WebKit shard 2, DCO, Semgrep and zizmor passed. WebKit shard 1
was still running, so CI required is not yet established.
Current run.
The PR remains draft; main requires CI required, DCO, and human/code-owner approval.
No scheduled polling or CI rerun was requested.

Earlier validation, tied to its historical state

Previous head: 55b08733a48e487443457488e4198e4e9fb3845e, included main d8fa1a2 (#45) and merged fixture prerequisite #46.

  • At that previous head: pnpm check and 134 tests in 15 complete affected files pass, including the 13-test real broker fixture and main's changed signed-admission/signed-priority/agent-observer tests. The unchanged pre-push hook passes TypeScript and 647 related tests / 59 files.
  • At parent f0a215f plus exact diff SHA256 ea4cc4d97dd00d844ee988783113f192f2c98cac7515b34d290fb7c24a94c55b (committed byte-identically as 69aeee1): 300 browser journeys, 14 design-browser journeys, Rust formatting, Clippy and 9 Tauri unit tests pass. The full PR diff at that earlier main merge had that exact hash; it is not the current repair hash. Main's later browser-test changes were not part of that browser run; the subsequent exact-head hosted browser lanes passed (see run below).
  • The earlier batch passed 1,181 Vitest tests / 118 files and plugin-manager 8 unit + 8 integration tests on the preceding test-driver revision. The final revised fixture was rerun separately; this is not an exact-head full-scan claim.
  • Disposable-copy mutations detect priority spoofing, omitted host/broker advertisement and per-socket budget ownership; the latter produces 58 charged WS attempts versus cap 27. Removing the snapshot driver fails its separate 0 < 11 assertion.
  • Independent source review by Pinky found no outstanding blocker on the reviewed diff; reviewer did not run these tests. At 55b0873, all four PR-only commits had author-matching DCO signoffs. That head’s hosted CI required, all seven constituent lanes, DCO Check, Semgrep OSS and zizmor passed in run 34771552914. Required human/code-owner approval remains outstanding; passing checks do not authorize merge.

Evidence limits

The steady-state fixture retains all eight HTTP/REQ callers and chooses publication owners from actual EOSE-ready state. Separate snapshot throughput (at least 11), ordinary throughput and combined charged-arrival upper bounds prevent a vacuous pass. Real-time arrival assertions do not replace deterministic start-clock tests.

An earlier load-driver run produced an unclassified publication 503 whose cause remains unproven. A separate cold-owner probe reached only six of eight EOSE-ready owners after 15 seconds under saturation. Improved driver settlement/capture and subsequent passing runs do not retrospectively explain that 503 or certify cold start. Unclassified 503s still fail; no retry/tolerance/allowlist waiver was added.

No native GUI click-through, deployed SQL/Redis/load measurement, identity-wide quota reservation, or same-identity old/new Buzz coexistence (including closing either client) has been established. This draft is a reviewable transport extraction, not permission to enable or merge the feature.

Originating work

Buzz channel 2128346d-ae35-4e44-98f7-5f278fe7b60d, original thread f86020aa851a6a4989ad174b1726fa61da77f573d8d29fe8507b71aa362ff2ff; current repair thread 937688ee458400d4c3e8929bbf48a312590650490ae69a73ef2551e35fb23e2e.

Brain and others added 4 commits September 13, 2026 09:48
Give concurrent fixture servers separate temporary caches and preserve close ownership through setup/cleanup failure. Keep assertions, timing budgets and zero retries unchanged. Record the scope-growth checkpoint before extending shared systems.

Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Keep bounded snapshot and same-socket observation/publication ownership separate from foreground reads, sends and route setup. Share actual relay cooldowns and preserve cancellation/signing settlement. Exercise combined budgets through the existing broker fixture; do not add unused signed-HTTP adapter parity.

Co-authored-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
@wesbillman
wesbillman marked this pull request as ready for review September 14, 2026 00:44

@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.

Changes requested: one P2 transport contract defect. Reviewed head 33b699e0ceea6b0ff0b39830ff76e56bef31d60b against base 57796f034f290fbb5d7ee4fc41807f5c784591ff.

GitHub rejected a formal REQUEST_CHANGES review because the authenticated account authored this PR (422: Can not request changes on your own pull request). Publishing the integrated findings as a comment review instead; this is not approval.

P2: optional presence can make valid shared-stream interests fail the HTTP body limit

The newly serialized author list can push an otherwise valid stream request over the existing 150,000-byte ceiling (broker input boundary, browser producer). With 1,024 distinct 128-character channel IDs, 64 valid priorities, and observer:null, the ordinary body is 142,571 bytes. Adding 256 full author keys makes it 159,735 bytes; even an empty priority list is 151,352 bytes. These values pass the individual validators, but the broker rejects the combined request before parsing. The browser treats that 413 as terminal, so ordinary live chat is lost with the optional capability instead of exposing only over-capacity channel routes as limited.

This is a supported-boundary defect for long, high-cardinality channel IDs, not a claim that ordinary UUID-sized lists currently fail. I independently verified compact serialization sizes on Studio and traced the exact-head producer, validators, guard and terminal-response handler. I did not execute a live HTTP reproduction.

Exit criteria: size the bounded stream body for the combined allowed fields and serialization overhead; add a real broker HTTP regression at the combined maxima while retaining malformed and over-budget rejection. This needs a bound adjustment and regression, not a new adapter or broader feature work.

P3 follow-up, non-blocking: disconnected Retry does not reset exhausted presence

After presence exhausts its setup attempts, an upstream socket disconnect preserves failures > 3. If the existing broker owner receives Retry before reauthentication, live.retry() takes connect() without calling presence.retry(). reset() retains exhaustion, so ordinary routes reconnect but presence cannot dispatch; a second Retry after authentication is required. This is a source-traced recovery gap, not a runtime reproduction or current activated-UI failure. Before activation, clear presence exhaustion on explicit Retry in both connection states and cover exhausted presence → upstream disconnect → single Retry → reauthentication. Preserve exhaustion across automatic reconnects. This follow-up is not an additional merge blocker.

Validation and scope

Studio-only pinned source review with independent socket, broker and fixture lanes; complete base-to-head diff check passed. One exact-head GitHub snapshot showed all 12 checks successful, including all browser shards and CI required. No broad CI-equivalent rerun, native GUI launch, approval or merge.

This review covers the transport prerequisite only. It does not authorize presence activation or claim deployed relay load, same-identity old/new-client coexistence, or a proven cause for the historical fixture timeouts. PR47 remains the prerequisite to PR39.

@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.

Changes requested: one additional P2 presence-lifecycle defect. Reviewed head 33b699e0ceea6b0ff0b39830ff76e56bef31d60b against base 57796f034f290fbb5d7ee4fc41807f5c784591ff.

The concurrent human-requested review already reports the combined stream-body limit blocker on this same head. That finding is not duplicated here. This automated review adds the independently verified lifecycle defect below. GitHub rejected formal changes-requested with 422: Can not request changes on your own pull request; this COMMENTED fallback is explicitly blocking, not approval.

P2: invalidate presence readiness when the browser's broker stream is replaced or lost

After a nonempty author set receives a ready presence-state frame, an ordinary subscription.update(channels) calls start() and retires the SSE generation. start() aborts the old owner and reports generic live connecting, but never invalidates the separate presence-state callback. The same omission occurs when the stream ends, fails, or exhausts reconnects (lines 119–122 and 199–219), and when replacement receives a terminal 400/401/403/413 before any SSE state arrives (lines 94–106).

Consequently the presence consumer's last state remains ready for a route that no longer exists. It can remain so indefinitely after terminal reconnect/rejection. The old server cannot repair this via SSE: the browser has aborted and fenced that generation. Only changing authors or receiving a new generation's presence-state frame corrects it. This contradicts the transport's separate EOSE-established readiness contract; it is not a request to activate presence UI or a claim of a current user-facing indicator failure.

Source reproduction: receive ready for authors A; change ordinary channel interests while holding the replacement stream response; observe that no non-ready presence transition is emitted. Reject that replacement with 403, or exhaust reconnects, and the last presence callback is still ready.

Smallest exit: have the existing browser stream owner synchronously invalidate presence readiness whenever its generation is retired/lost: pending for nonempty desired authors while reconnecting, idle for no demand, and an appropriate non-ready terminal state on unrecoverable failure. Keep generation fencing and require a new valid presence-state frame to regain ready. Add controlled regressions for ready → ordinary channel replacement with the response held, and ready → stream loss → terminal rejection/reconnect exhaustion. No new connection, adapter framework, session activation, or heartbeat is needed.

Other findings and validation boundaries

The disconnected-Retry exhaustion gap is independently confirmed (live.ts:583-597, presence-live.ts:96-103,298-311). Consistent with the existing review, retain it as a P3 activation follow-up, not another merge blocker: one explicit disconnected Retry should clear presence exhaustion, while automatic reconnect should not bypass its retry cap.

All three independent lanes returned before this publication. Reader/HTTP capacity isolation, signer/body ownership retention, shared cooldown with separate start clocks, matched WS OK receipts, two-slot handover capacity, and ordinary WS→SSE notification provenance were source-reviewed without another material finding. The fixture audit found no additional blocker; its quota evidence is a local policy model, not deployed coexistence or latency proof.

This review was pinned-object source-only over strict-verified SSH on Wes’s Studio. No PR checkout, imports, builds, tests, or runtime reproduction were executed. CI tooling preflight was unavailable in this automation session, so it does not independently certify the author/other review's test results. No approval, merge, or feature-activation authorization.

…e readiness

Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@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.

Review clear: the prior two P2 findings and P3 Retry follow-up are addressed in source. No new actionable blocker found in this convergent repair review. This is a comment review, not approval or merge authorization.

Head: f94a5fbd235774195e4c928c799dd57f86971826
Base/merge-base: 57796f034f290fbb5d7ee4fc41807f5c784591ff
Repair comparison: six files changed since previously reviewed 33b699e0ceea6b0ff0b39830ff76e56bef31d60b (earlier body-limit review, earlier lifecycle review).

  • Combined stream body: the bounded envelope is now 160,000 bytes, accommodating the production compact maximum of 159,747 bytes. The real-HTTP regression uses the actual browser producer with all four fields at their combined maxima, requires HTTP 200, checks ordinary signed traffic and limited routes through observer toggles, and retains invalid/oversized rejection before socket allocation. Boundary, regression.
  • Presence readiness: local stream replacement/loss and terminal failure now emit non-ready presence state. Existing generation and author-set fences remain intact; generic connected frames do not restore readiness. Regressions hold replacement headers, inject retired frames, cover terminal 400/401/403/413, EOF/error reconnect exhaustion, explicit recovery, and idle without demand. Owner, regressions.
  • Disconnected Retry: the explicit action resets presence exhaustion before branching on authentication; automatic reconnect still retains the cap. The paired regression distinguishes those paths through reauthentication and fresh EOSE. Owner, regression.

Validation and scope: Mongo’s independent body-limit lane returned with no blocker before publication; Carl verified the producer, validation boundary and regression path. This is source/arithmetic evidence, not a reproduced HTTP run. Pinned-object source review over strict-verified SSH on Wes’s Studio; base-to-head diff check passed. No PR checkout, imports, builds, tests, or runtime reproductions were executed. CI-tool preflight did not expose ci-results, so current hosted CI and the author's reported local passes are not independently certified here. This clears the reviewed source findings, not the separate validation/required-review gates.

The agreed boundary remains the transport prerequisite: optional presence must preserve ordinary traffic and shared ownership. No session/UI activation, extra adapter framework, deployed quota claim, or same-identity old/new-client coexistence claim. PR47 remains the prerequisite to PR39.

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.

1 participant