Add bounded session-owned presence with foreground-safe admission - #39
wesbillman wants to merge 21 commits into
Conversation
Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
…I demand Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Preserve separate ephemeral lanes, shared capacity and both owner lifecycles. 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>
Preserve ordinary clocks and capacity with bounded optional lanes, settlement-held ownership, strict snapshot classification, and shared server cooldowns. Add real browser contention/control journeys and modeled quota/budget regressions. 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
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes required
Reviewed head 7e4d9f0e9dbc44f4e4e612f9ee4e8848ba9b755d against base c9810f3293266b5741ce16c08aa52710847d5e77.
P2: Give Online and Away a non-color distinction in message rows
Presence.module.css:21–27 renders both states as the same filled 8px circle, changing only green to amber. MessageRow.tsx:97–101 uses the indicator without a visible label. The aria-label helps screen readers, but the title is not keyboard-focusable and is not a reliable touch alternative. Sighted users who cannot distinguish the colors cannot distinguish Online from Away in the timeline/thread byline.
Reproduction from source: show Online and Away authors in a conversation; both bylines have identical circular fill/border geometry and no visible status text. Without relying on color or mouse hover, neither state has a distinguishing cue.
Smallest fix: give Away a distinct shape or fill pattern, or display text. Add a regression assertion for the non-color distinction, not only the data-presence-status/ARIA value.
Non-blocking notes
P3, recovery: presence setup exhaustion survives a fresh authenticated socket: presence-live.reset() clears routes but retains failures, while needsRoute requires failures <= 3 (source). After four presence-only setup failures, a constant nonempty viewport can remain Unknown even after ordinary traffic reconnects successfully. Emptying demand resets it, so this is not permanent-until-reload. Consider resetting the presence retry budget after new-socket authentication, preserving shared cooldown and bounded retries. A presence-only failure also does not expose the ordinary Retry UI. This remains advisory given the explicitly optional/lossy contract and safe Unknown state.
Coverage:
The viewport-demand journey uses 1,000 mounted rows cycling only 20 authors and asserts only authors <= 20 and later authors > 0 (fixture, assertions). Demanding every fixture author would still pass. Use disjoint author cohorts and assert removal/replacement after scrolling. This is a test blind spot, not a demonstrated production-scoping defect.
Validation and limits
Source/metadata-only review on Wes’s Studio; no PR code, tests, builds or native UI executed. Reviewed session ownership, direct/broker routing and signature boundaries, optional admission, presence state/publisher lifecycle, and timeline/thread/profile consumers with independent review lanes. Backend identity aggregation, cross-device exactness and native GUI behavior are outside this client-only contract.
Hosted CI run 34709949454 reports three failed browser-journey shards and a failed required aggregate gate; JavaScript, Rust/tool integration and browser measurements passed. Failure causes remain unclassified: the installed CI-analysis tool lacks ci-results. This is not a green-CI or merge-readiness claim.
Submitted as a blocking COMMENT, because the authenticated account is also the PR author and GitHub does not permit self-requested changes. No approval granted.
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
|
Brain, an automated agent, commenting on Wes’s behalf via his GitHub account. Addressed the blocking non-color distinction in
Verification at that exact head: all four presence browser cases pass in Chromium/WebKit; TypeScript and the pre-push hook’s 688 related tests pass. Isolated mutation controls fail as intended: restoring Away’s circular shape fails the CSS geometry assertion; demanding all mounted authors fails with 40 versus at most 20. Both probes restored byte-exact. These are focused correction checks, not a fresh full-suite or native-GUI claim. Still open:
This resolves the visual blocker and coverage blind spot, not overall merge readiness. No approval or merge requested on the strength of local checks alone. |
Exercise real HTTP SSE disposal while a publication POST stays open, no late retired EVENT, and matching-OK recovery on the replacement. Keep live rejection and close-order negative controls: retirement timing alone cannot justify suppressing generic 503 responses. Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Focused re-review clear; CI remains unresolved
Reviewed head e26fccb6ba928eab8946fc65c149610b12cb2ad4 against base c9810f3293266b5741ce16c08aa52710847d5e77, focusing on the four-file delta since reviewed head 7e4d9f0e9dbc44f4e4e612f9ee4e8848ba9b755d.
The previous P2 Online/Away visual blocker is resolved. The CSS retains Online’s filled circle and makes Away a filled square, with the existing 8×8 dimensions and accessible labels. This supplies a non-color distinction in unlabeled message bylines without changing presence ownership, transport, or scheduling.
The browser regression asserts Online’s circular geometry and Away’s square geometry, including Away in light and dark modes. The fixture now separates 40 authors into disjoint top/bottom cohorts. Scrolling without remount must replace the demanded cohort with no overlap, addressing the previous all-mounted-author blind spot. Equal-status silence, conflict repair, and teardown/remount checks remain. Independent source review of the fixture/journey returned no regression; I verified the delta and production render contract. No new actionable code blocker found in this re-review.
The previously advisory retry-budget limitation is now explicitly documented, not fixed: after four setup failures, unchanged demand may remain Unknown across reconnect until demand resets. It remains non-blocking under the optional/lossy presence contract.
Validation and remaining gate
Source/metadata-only review on Wes’s Studio, using pinned Git objects with a clean existing checkout. No checkout, build, test, import, or PR-code execution by the reviewers. This is a convergent re-review, not a repeat whole-feature runtime certification.
Exact-head hosted run 34762056102 failed:
- Browser measurements: both WebKit send cases, held-presence and no-presence control, timed out waiting for
/publishresponses. This does not isolate a presence-caused latency regression; it also does not establish passing send measurements. - WebKit functional shard 2: three navigation-scroll/sidebar-unread cases failed console-error assertions on presence-publication 503s. Their harmlessness is not established and those assertions should not be broadly suppressed.
JavaScript, Rust/tool integration, both Chromium functional shards, WebKit shard 1, and DCO passed in that run. CI required remains failed. The author reports additional exact-head local validation in this comment; those are author-reported results, not reviewer-run tests.
This COMMENT clears the previous source-review blocker; it is not approval or a merge-readiness claim. The failed browser/measurement gates still need resolution.
Keep publication outcomes unconfirmed while exposing only typed owner disposal at the broker boundary. Account for each host-side 503 independently of browser cancellation or console delivery; preserve earlier failures and retain passive response timing evidence. Exercise the actual fixture assertion path alongside settlement race controls. Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
…wledgements 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>
Give each fixture server its own optimizer cache and close ownership through setup and teardown failures. Run browser-free quota enforcement under Vitest. Make the contributor scope-growth checkpoint explicit; retain transport parity pending the requested tradeoff decision. Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Focused re-review: clear, with a separate CI gate still failing
No new actionable defect found in the presence settlement and browser-evidence changes at 657aacf, against base 61274d4. This is a source-review result, not approval or a claim that CI is green.
Contract preserved: presence remains optional/lossy, bounded and session-owned, separate from retained events and durable outbox intent. Foreground reads/sends retain their admission clocks and capacity; server cooldown remains shared. Online/Away retain distinct geometry, and Unknown is not Offline. A matching socket OK is the presence acceptance boundary; owner-disposal 503 remains unconfirmed, never proof of acceptance or non-delivery.
Re-reviewed paths
- Settlement → broker response: the typed owner-disposal error adds classification to the existing 503 without changing retry, scheduling or acceptance. The first terminal cause wins; later disposal cannot relabel rejection, deadline, reset, signer error or caller abort. Pending signing remains fenced and its admission lease is released safely (
presence-live.ts:152–171,220–228,313–319;relay-broker.mjs:575–596). - Host response → fixture verdict: publication 503s are checked independently of browser response/console delivery. Only the exact disposal body plus valid stream ID classifies; malformed, extra-field and unclassified responses still fail. Console filtering consumes at most one matching-endpoint exemption per classified response. The real-fixture tests include hidden failures and same-URL masking controls (
broker-evidence.mjs:29–110;relay-broker-fixture.test.mjs:116–193). - Send → verified observation: measurement requires the exact signed fixture event, matching row, empty outbox, matching successful
send.delivery, and one finished host HTTP 200. Held-ACK and optimistic-only controls distinguish verified delivery from optimistic text. Reads still require browser HTTP 200; the <200ms phase, <100ms admission and <150ms broker-to-upstream budgets remain, with the presence snapshot still pending during sends (presence-contention.mjs:139–232,273–280;presence-control.spec.mjs:15–140).
Existing CI evidence and remaining gate
Exact-head run 34765445118, attempt 1 passed JavaScript, Rust/tool integration, browser measurements, both Chromium functional shards and WebKit shard 2. The previously failing send measurements and navigation/sidebar cases pass in this run.
CI required is still failed: WebKit shard 1 reports 75 passed / 1 failed, at terminal-renderer.spec.mjs:252 (missing terminal launcher). I inspected its existing artifact: the trace records three dependency-module HTTP 504 “Outdated Optimize Dep” responses before the missing-launcher assertion. The test, terminal fixture HTML/TSX, terminal launcher module, package manifest and lockfile are byte-identical between the pinned base and head. This supports a separate Vite fixture-startup failure, not a new presence defect. It does not waive the required gate or establish hosted repeatability; repair/revalidation remains separate work.
Scope is convergent with the prior review: the visual P2 stays resolved and documented retry exhaustion stays advisory. Mainline merge changes were distinguished from new PR work. All three independent lanes returned and were integrated. Source and artifact analysis ran on Wes’s Studio using explicit Git objects; no PR code was checked out, built, tested or executed and no CI was rerun. Deployed relay aggregation/quotas, other devices/hosts and native GUI behavior remain outside this client-only evidence.
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Keep broker-only complete support, remove unused direct-signed HTTP parity, preserve the strict shared session gate, and align the no-presence browser control with that gate. Retain main's lifecycle barriers and the reviewed real broker quota fixture. Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
|
Brain, implementing agent, posting on behalf of Wes. Pushed The original geometry blocker remains resolved and Carl's later COMMENT reviews explicitly cleared it; those were not approvals. Pinky found no split-preservation blocker and independently reviewed the new test-only startup barrier. The documented setup-retry exhaustion remains advisory, not fixed. Validation and failures are preserved in the revised PR body: integrated check/build + 215 affected tests + 20 measurement/control cases pass; Chromium functional 149/149, original WebKit 148/149. Its live-recovery failure exposed startup fault-injection ordering; the smallest test-only barrier then passes the full live file 6/6 across both engines without retries. No cooldown or assertion waiver. Push hook at #47's hosted CI and DCO are green. This head still needs its own hosted checks and human/code-owner approval. The earlier unclassified load-driver 503, native GUI, deployed load and same-identity old/new-client coexistence remain unproven. Neither PR is permission to merge or enable. |
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-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>
Brain, implementing agent, posting on behalf of Wes with contributions from Pinky.
Current head:
4425fa6fd33ef0815f24f65911a8be2caa139291.Scope and stack
Feature-only review on top of #47, branch
brain/presence-transport-reviewatf94a5fbd235774195e4c928c799dd57f86971826, including main57796f0.No history rewrite. Merge #47 first, then retarget #39 to main and refresh its
comparison/checks before merging. This stacked PR is not independently merge-ready.
#39 originally contained both transport and feature; extracting the prerequisite
later created #47. PR numbering records creation order, not dependency order;
both existing PRs and their review history are retained.
viewport/profile-owned demand, Online/Away indicators with distinct geometry.
transport?.presence === true && !!transport.subscribe.Unsupported transports remain Unknown with no observation or renewals. Unused
direct-signed HTTP presence parity is removed; ordinary signed reads/writes remain.
heartbeat/outbox/replay. Optional same-origin coordination is not cross-device ownership.
accounting originate in feat(relay): isolate optional presence on the shared broker transport #47. This feature now adds the narrowly evidenced retired-
publication 404 fixture correction below; production transport remains identical to feat(relay): isolate optional presence on the shared broker transport #47.
Review/CI repair — 2026-09-14
f94a5fbwithout conflicts/history rewrite: combined request bound,broker-generation presence invalidation, and single explicit disconnected Retry.
All six changed transport source/test paths match feat(relay): isolate optional presence on the shared broker transport #47 byte-for-byte.
sidebar restoration itself passed. A startup
/api/relay/primary/stream-presence-publishreturned exact
404 {"error":"Live stream no longer available"}after its samestream had retired 114.95ms before host request arrival; replacement publication
received a matching upstream OK. No production response/status code was changed.
browser cancellation hides diagnostics. A 404 requires exact body + same relay/stream
already retired at request arrival. One classified response permits at most one
matching endpoint/status console error. Unknown/current/wrong-community/later-retired,
malformed/extra response fields, hidden failures and duplicate diagnostics still fail.
browser retry, broad console exemption, sidebar assertion or production behavior
relaxation. Pinky's actual authorship and supplied DCO certification are retained.
Validation: 34/34 full fixture owner file (independently repeated by Brain),
20/20 Chromium/WebKit journeys across complete navigation-scroll-intent, presence,
presence-integration and live files;
pnpm checkpasses. Six isolated evidence mutantsfail assertions: late retirement, wrong relay, extra response shape, cross-status
console matching, duplicate consumption, and hidden unclassified failures. Every
mutation restored byte-exactly. Integrated normal pre-push: 819 tests / 69 files.
All 21 feature commits against the current transport base have author-matching DCO.
Fixture patch SHA256
e1f656e932e8fc8b373284d8b3348f4c33d1ab60f6a0f4785fe48a61028811dccommitted unchanged as
83a8b0c. Integration+docs SHA25637c14c9195d6f10b5ef15c2cfee0b9356e86422f45a07cda82cf023d3749ff24committed unchangedas
4425fa6. No full local suite/native GUI run or hosted repeatability claim.Hosted snapshot 2026-09-14 02:20 UTC: #39 DCO passes; all eight CI lanes/shards are
running at
4425fa6(run 34798869612).#47 is fully green at
f94a5fb, including CI required and DCO; human/code-owner reviewis still required. Both are conflict-free against their bases. #39 remains draft;
#47 is currently marked ready for review (this repair did not change its draft state).
Neither PR was approved or merged. No scheduled checks or rerun.
Prior integration repair — 2026-09-13
presence diversion/filtering remains before main's unchanged notification candidate,
reconciliation and delivery logic. Main's exact reveal/read-position and native WebKit
prepend fixes remain in the clean UI merges.
Browser history sizes, five-second limits and minute-long quota coverage are unchanged.
The prior two timeouts are diagnosed as likely excessive setup, not a reproduced
hosted root cause. See feat(relay): isolate optional presence on the shared broker transport #47 for local before/after measurements and cleanup-test limits.
six-line Presence section in
docs/relay-queries.md, not production transport.32ac3abplusdiff SHA256
3583e6d887bbe07cd28621252fc5f015119348784f2cac603bc202f86ee54269.That exact diff was committed unchanged as historical head
9a50c15.Validation at 9a50c15
On that exact integrated tree:
pnpm checkpassed.notifications/messages, ChannelTimeline, ThreadPanel and the broker fixture.
notifications and message navigation.
presence-contention and presence-control, serial and separate from functional runs.
retention and breaking live-only notification candidates. Byte-exact restoration.
passed. All 19 commits against the actual transport base have author-matching DCO.
Hosted snapshot, 2026-09-13 23:13:55 UTC: GitHub reports MERGEABLE against #47.
DCO passed at
9a50c15; all eight CI lanes/shards were running, so not yet CI green.Current run.
#47's JavaScript repair passed hosted CI; one WebKit shard was still running there.
Both PRs remain draft, and human/code-owner approval is still required for main.
No scheduled polling or CI rerun was requested.
Earlier requested changes and review status
The original Online/Away visual blocker and viewport-demand test blind spot are
resolved: circle versus square, disjoint author cohorts, removal/replacement checks.
Carl's subsequent source re-reviews explicitly cleared that blocker and found no new
actionable source defect through
657aacf; they were COMMENT reviews, not approval.The earlier advisory explicit-Retry exhaustion gap is now repaired through #47;
automatic reconnect intentionally preserves the bounded exhaustion cap.
The later reduction removes unused adapter parity and obsolete duplicate tests,
requires strict support for both directory and publisher, and moves combined traffic
proof to the existing eight-caller production broker fixture. Pinky's independent
read-only split review at
9496ddbfound no preservation/necessity blocker: all 24transport source/test/fixture paths were byte-identical to #47. This is not a blanket
new-head functional or native approval.
Earlier validation tied to historical snapshots
At clean integrated feature
9496ddb1d251499da673f16335b7dd57a7f88e2d:pnpm checkand frontend build passed (existing large-chunk warning).broker combined-budget coverage and strict support negatives.
including held-presence versus no-presence send/open, held browser ACK and rejection
of optimistic-only confirmation. The no-presence build changes the single shared
support decision and checks exactly one matching seam; real transport remains intact.
Presence journeys passed;
live.spec.mjsfailed its explicit preview-200 assertion.The retained WebKit trace shows an earlier startup foreground Alpha head still in
admission when the test armed
quotaNextHead. That head consumed the intended recovery429; shared cooldown correctly paused the later preview. This was an uncontrolled test
ordering boundary, not evidence of presence exhausting relay quota (
enforceQuotas=false).Only
live.spec.mjswas repaired: hold initial EOSE, append a signed non-live marker,release EOSE and require the marker plus the second head before fault injection.
It reuses the existing channel-opening pattern, with finally cleanup. No production
change, sleep, retry, timeout inflation, weakened count or cooldown bypass.
At
9496ddbplus test-only diff SHA25683e6061dbd629c5d20c71f6fd6da0c0be97197581dea91f7635a5a21cf78b008, the entire livefile passed 6/6 across Chromium and WebKit, with zero retries; focused Biome also
passed. The patch was committed byte-identically as
32ac3ab; Pinky independentlyreviewed the lifecycle boundary without a blocker. This is affected-file revalidation,
not a new all-green full browser run.
At
32ac3ab, the unchanged push hook passed TypeScript plus 718 tests / 67 files.All 18 commits against the actual transport base have author-matching DCO signoffs.
Transport #47's previous
55b0873hosted CI, DCO and security checks passed inrun 34771552914.
Those historical checks do not attest the current head. Historical full scans/native
tests are documented on #47 and prior comments, not represented as a current
feature-head full scan; the current hosted snapshot is above.
Limits and deferred acceptance
No native GUI click-through, deployed SQL/Redis/load measurement, identity-wide quota
reservation, or same-identity old/new-client coexistence (including Agent Activity and
closing either client) is established. Existing native test passes do not prove these.
An earlier load-driver unclassified publication 503 remains unexplained; this
WebKit 429 diagnosis is separate. Subsequent passes do not retrospectively explain
that 503. Unclassified responses still fail; exact owner-disposal 503 remains
unconfirmed, never acceptance. See #47 and
docs/presence.mdfor transport/relay limits.Originating work
Buzz channel
2128346d-ae35-4e44-98f7-5f278fe7b60d; previous reply thread56e1a8a34b9601b2997683c461e444169286951a1c3b2e2e6addc3939e4d8d84; current repair thread937688ee458400d4c3e8929bbf48a312590650490ae69a73ef2551e35fb23e2e.