3 Commits

Author	SHA1	Message	Date
Will Pfleger	52c4c08c9e	fix(sidebar): converge stale-at-open state across devices (sections/sort/stars/mutes) (#7805) ... Adds bounded-backoff recovery reads to all four sidebar sync hooks (sections, sort, stars and mutes). A stale relay read at app open now converges without a user edit or a reconnect. ## What this does **Shared recovery hook (`useStaleReaderRecovery`).** All four hooks use one retry effect. It reads immediately on mount, then at 5, 10, 30 and 60 s, staying at 60 s after that. A change of `visibilityState` to `visible` triggers an immediate read. Reads are single-flight. The effect restarts whenever `pubkey` or `relayUrl` changes, so a hook that stays mounted while switching relays drops the old relay's in-flight reads. **Guards that stop a recovery read from rolling back a local edit:** 1. **Skip the read while an edit is pending.** If an edit is pending when a tick starts, the tick skips its read and just schedules the next one. Without this, the read could return the pre-edit blob after the edit published and pending cleared. At that point neither apply-time guard could reject it. 2. **Pending check inside the state updater.** `hasPending()` and the effect lifetime are re-read inside `setStore(prev => ...)`. That way the check runs when React executes the updater, not when the response arrives. 3. **Local-mutation revision.** Each hook counts its local edits. The count is snapshotted when a read starts and checked again at apply time. If an edit landed while the read was in flight, the response is discarded. 4. **Head advanced only after a successful cache write.** A failed `write*Store()` leaves the same head retryable on the next tick. **Ownership-aware pending clear.** Each sync manager's `doPublish(store)` now clears `pendingStore` only while it still holds that same `store`, on both the identical-payload early return and the success path. Before, an older publish that finished after a newer edit had replaced `pendingStore` wiped the newer edit's pending marker, so a recovery read before the newer debounce could apply the older relay head over it. `destroy()` still clears unconditionally. ## Scope Reader-side recovery plus the ownership-aware pending clear. No outboxes, arbitration or replay changes. It applies to `useChannelSections`, `useChannelSortPreference`, `useChannelStars` and `useChannelMutes`. Related: [#7806](https://github.com/block/buzz/pull/7806) --------- Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> Co-authored-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>	2026-09-28 12:48:16 -06:00
Wes	f3408fc620	fix(desktop): isolate quota backoff and reuse channel discovery rosters (#6998) ... Pinky, an AI agent, is updating this PR on Wes's behalf. ## Targeted repair, not a startup rewrite This replaces the previous readiness/replay candidate. Current head: **`83ac81ff6ae439b0e68f4a3dafd4b2af6dbadcdb`**, two independent commits on main base `3c7f288c60d67df78577b237e27c3dfc8831aaa1`: 1. **`0b6681de2` — keep quota backoff on its transport.** HTTP 429s retain native HTTP backoff without arming the renderer WebSocket gate. WebSocket quota/concurrency CLOSED retains its retry deadline without damping HTTP's separate quota. The explicit `rate-limited: shared admission unavailable` signal still damps HTTP too. Reuse existing all-channel live streams for Home mention invalidation instead of a second per-channel mention subscription; retain kind, recipient, self, membership, replay-deduplication, notification and unread boundaries. 2. **`83ac81ff6` — reuse complete membership rosters within each channel fetch.** Discovery already returns full paginated rosters. Retain those results and request only uncovered directory/pending-owner channel IDs. Skip empty fallback filters and preserve covered counts/pubkeys if that tolerant fallback fails. The next fetch reads current membership again; there is no persistent cache or new invalidation state. No send-route, AUTH-readiness, replay-ordering, scheduler, pacing, relay policy, dependency, or authorization changes. Refresh triggers and activity batching stay unchanged. Independent tracing did not establish a safe-to-delete duplicate normal-startup refresh; recovery invalidations at different replay stages are not interchangeable. **Size:** 18 files, +1,066/-350 overall; nine production paths +76/-205 (**129 fewer production lines**). The roster follow-up itself touches only `fetch.rs` (+31/-14) and its new 378-line regression file. Test bulk is not new startup machinery. ## Comparative roster measurement Actual native HTTP fetch code against a synthetic Axum loopback fixture: 265 channels × 100 members, deterministic signed events, debug build. Seven iterations per condition; medians below exclude the first warm-up. Baseline is `0b6681de2` production fetch, candidate is `83ac81ff6` production fetch, with the same temporary benchmark harness. | Measure | Baseline | Candidate | | --- | ---: | ---: | | HTTP reads per fetch | 7 | 6 | | JSON response-body bytes per fetch | 4,803,136 | 2,453,380 | | Median, no artificial request delay | 262.9 ms | 149.3 ms | | Median, 25 ms artificial delay per request | 376.7 ms | 282.7 ms | Channel/member assertions and output hash (`fa7cc551bc855e4d`) match throughout. This removes **one batched roster request**, not 265 requests. The measured ~43%/~25% reductions describe this large-roster synthetic fixture, **not production startup latency**. Activity requests run concurrently with roster enrichment, so real savings vary. Counts now use the discovery snapshot rather than a later reread within that same non-atomic fetch. ## Verification At pushed head **`83ac81ff6`**: - All enabled pre-push gates passed without bypass: `push-head-scope`, `branch-skew`, `desktop-check`, `file-size-check`, `desktop-typecheck`, `desktop-test`, `desktop-tauri-checks` (native fmt/Clippy/tests). - Full Desktop frontend suite: **6,457 passed**, zero failures; separate tooling suite: **10 passed**. - Full native Desktop workspace: **3,276 passed**, zero failures, 20 ignored, across unit/integration/doc bins. The main native lib accounts for 3,175 passed / 19 ignored; this is not 3,276 lib tests. - `pnpm build` passed the protected-feature artifact matrix; `pnpm build:e2e` also passed. Existing chunk-size/dynamic-import warnings remain. No full repository `just ci` run locally. - **130 selected headless mock-bridge smoke cases passed across two runs**: messaging, reconnect, reconnect affordances, inbox live updates, sidebar snapshots, send-channel binding, boot splash. The first invocation hit the shell's 300-second limit after 117 passes and no failures; all remaining 13 passed in a separate invocation. This is not a claim the interrupted invocation completed. - Six new tests invoke production `fetch_channels` through the real native HTTP client and cover 265/501-channel pagination (including same-timestamp continuation), exact requests, complete/deduplicated rosters, directory/pending fallback, hidden DMs, repeated membership/identity/relay changes, empty lists and errors. - Falsification in an isolated source copy: restoring the redundant baseline fetch fails four roster regressions; removing missing-roster fallback fails two. Restored candidate passes all six. Earlier transport mutations fail HTTP→WS coupling, WS→HTTP coupling, omitted shared-outage damping and duplicate-mention regressions. No mutation was applied to the committed worktree. - Brain's independent static reviews found no blockers in the transport repair or immutable roster patch. Source tracing confirms relay discovery serializes complete rosters, not projected matching p-tags. This was source review, not independent test execution or approval. ### Hosted CI [CI run 34185670504](https://github.com/block/buzz/actions/runs/34185670504) **completed successfully** for this head. GitHub checked out merge ref `25bcf09627e129887481d4441a1af3c6cb0af1ff`; its Git tree is identical to `83ac81ff6` (`49a01f3c2ebf4e547bbe3cd31406f984ac6cefe8`), verified through the GitHub Git API. The final PR rollup has 33 successful checks and 28 skipped, with none failing or pending. - Desktop Core passed frontend/native suites and compiled-feature verification. Windows Rust tests, Windows Desktop build, macOS `.app`/`.dmg` build, all four smoke shards and both relay-backed E2E job shards passed, as did Rust lint, DCO, Semgrep and zizmor. - Browser job totals: **1,620 passed, 13 flaky cases passing automatic retry #1, five skipped, zero terminal failures**. Smoke accounts for 1,389 passed / 12 flaky / three skipped; integration jobs report 231 passed / one flaky / two skipped. No manual CI reruns or source changes to obtain green. - The retries cover channel-browser, empty-edit, huddle menu, runtime-install, overscroll, onboarding avatar, video attachment and workflow UI assertions. They are disclosed, not collectively declared unrelated. The video case reaches the changed real TS all-channel hook despite mocking native I/O: independent tracing found no regression mechanism or blocker, but the missing second-video player remains **unexplained**, and an indirect timing effect is not excluded. The self-authored, unmentioned event cannot use either the new mention branch or the removed old mention handler as a delivery path. A passing retry does not diagnose its cause. - The separate “Desktop Release Candidate” check skipped its validation step; it is not additional release validation. Codex security review was skipped, not passed. ## Remaining acceptance limits Wes's earlier manual trial of the transport-only candidate reported improvement; relay readback found both a top-level message and a thread reply. That was not a fingerprinted or timed trial of the new roster commit. The remaining few-second top-level send wait is **unattributed**, and this PR does not claim to resolve every startup delay. No visible app launch, relaunch, laptop input automation, or production test messages during this follow-up. Local native tests use fixtures; browser suites do not run the Tauri backend. CI building an app does not validate bundled macOS alerts or inactive-channel notification delivery. Those bundled real-workflow checks remain outstanding. No visual UI changes or screenshots; no approval or merge performed. Originating conversation: buzz://message?channel=5b130804-d759-40ad-a564-d64cc907fa8e&id=cb432417e290e06a1842639cfe0268f9d43d186defbcb59260930d129a26af83 --------- Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz> Co-authored-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>	2026-09-11 09:11:39 -06:00
Wes	2f3dd850db	fix(relay): reject a frame on its own acknowledgement channel (#6961) ... Pinky, an AI agent, updated this description on Wes's behalf after taking over the startup investigation. **Category:** fix **User Impact:** An EVENT refused by WebSocket admission or handler saturation receives a correlated `OK(event_id, false, reason)` instead of an uncorrelated NOTICE, so the client can settle that refusal without waiting for its publish timeout. Rate-limited refusals also arm client backoff. This fixes a protocol failure mechanism; it does not establish that every startup send will succeed or that the reported Desktop startup incident is fully resolved. **Problem:** Startup opens several live subscriptions and publishes at once, and the relay's WebSocket admission gate is a fixed 5-second window (`ws_admission_budget` = `human_ws_events_per_sec * 5`). If that shared per-principal quota is exhausted, `enforce_ws_admission` previously rejected an EVENT with a bare `["NOTICE", reason]`. Quota pressure is a possible trigger, not proof of the original incident's complete cause. A NOTICE carries no event id. Both clients settle a pending publish *only* from an `OK` keyed by event id (desktop `pendingEvents`, mobile `_pendingEvents`), so nothing settled — and `handle_text_message` returns early, so no `OK` ever followed either. The send **could not fail**; it could only time out at `PUBLISH_TIMEOUT_MS` = 25s. That explains how this rejection mechanism can produce a roughly 25-second timeout; attributing the original report to it still requires the actual startup/send workflow. The handler-semaphore saturation path had the identical defect, and that one needs no quota burst to fire. **Solution:** NIP-01 gives each request type its own acknowledgement channel, and a rejection is only actionable on the same one. Reject a REQ with `CLOSED`, an EVENT with `OK(id, false, reason)`, and fall back to `NOTICE` only where no per-request correlation exists. COUNT refusals now also use `CLOSED(query_id, reason)` per NIP-45, covering both quota admission and handler saturation (added in `cd12c93804b87a24b61075dfd171dc471a0a527f`). Reason strings are unchanged, so the `rate-limited:` prefix and `retry in {N}s` hint that existing client gates parse keep working (desktop `parseRateLimitHint`, mobile `RelayRateLimitGate`, buzz-acp `set_rate_limit_gate`). Only the frame *type* changes, so `docs/multi-tenant-relay.md` L7 stays satisfied. Two notes on how this landed, both worth a reviewer's attention: 1. **A survived mutation became a design change.** `send_admission_result` originally took a `RejectionTarget` parameter, and reverting the *second* call site (the per-minute message quota) survived the whole suite — with Redis unreachable the first quota check short-circuits, so that line is unreachable in test. Rather than test around it, the parameter is gone: the target is derived from the frame, so no call site can name the wrong channel. 2. **The relay fix would have caused a client regression on its own.** Gate arming lived only in the NOTICE branch. Once rejections arrive as `OK:false`, `handleOk` failed the send without ever backing off — the client would retry straight into the same quota. Desktop and Mobile now arm on a `rate-limited:` OK rejection. ACP was subsequently fixed in `3b06dd32493596ec650f20abf8805791c50fdc24`: it arms the gate and re-parks only the refused observer frame, preserving other in-flight frames. Desktop gets `activateRateLimitIfSignalled` as the single owner of that prefix test, called from both `handleOk` and the NOTICE branch. <details> <summary>File changes</summary> **crates/buzz-relay/src/rejection.rs** (new) Owns the admission-rejection concern: `RejectionTarget`, `rejection_target_for`, `request_rejection_message`, `send_admission_result`, and `enforce_ws_admission`, moved out of `connection.rs`. Six tests, two of which drive the real `enforce_ws_admission` against a real `AppState`. **crates/buzz-relay/src/connection.rs** Fix the EVENT handler-semaphore rejection to correlate to the event id; delegate admission to the new module. Add two tests that drive the real `handle_text_message` with every handler permit held. Down from 1319 to 1116 lines. **crates/buzz-relay/src/state.rs** Widen the existing `test_state` helper to `pub(crate)` so the rejection tests reuse it rather than adding a ninth copy of `AppState` construction. **desktop/src/shared/api/relayRateLimitGate.ts** Add `activateRateLimitIfSignalled` — one owner for the `rate-limited:` prefix test, since three inbound frame types now carry it. **desktop/src/shared/api/relayClientSession.ts** Arm the gate on a rate-limited OK rejection; route the NOTICE branch through the same helper. Net zero lines, which keeps this already-oversized file within the differential ratchet. **desktop/src/shared/api/relayClientPublishRejection.test.mjs** (new) Four tests against the real `RelayClient`: a rate-limited OK settles the pending publish and arms the gate; an ordinary rejection does not arm it; an accepted OK still resolves. **mobile/lib/shared/relay/relay_session.dart** Arm the gate in `_handleOk` for a rate-limited rejection. **mobile/test/shared/relay/relay_session_test.dart** Two tests driving the real `publish` + `debugHandleMessage` path. </details> <details> <summary>Validation</summary> **Mutation-tested — 5 mutations, all now killed.** Each production call site was reverted to the defective behaviour to confirm a test fails. This caught two false-negative tests: | # | Mutation | Result | |---|----------|--------| | 1 | `rejection_target_for`: EVENT → `Connection` | 4 tests fail | | 2 | EVENT handler-semaphore call site → bare NOTICE | **survived at first** | | 3 | per-minute quota call site → `Connection` | **survived**; fixed by removing the parameter | | 4 | desktop `handleOk` gate arming removed | 1 test fails | | 5 | mobile `_handleOk` gate arming removed | 1 test fails | Mutation 2 is the lesson: my first saturation test called `request_rejection_message` directly, so reverting the real call site inside the `match` arm left it green. It now drives `handle_text_message` itself and dies on that mutation. - `cargo test -p buzz-relay` — 928 passed, 1 failed: `api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo`, **pre-existing**, reproduced with all changes stashed at `4dd4d73de`. - `cd desktop && npm test` — 5721 passed, 0 failed (full suite). - `cd mobile && flutter test` — 1876 passed, 0 failed (full suite). - `just fmt-check`, `just clippy`, `just desktop-check`, `just mobile-check`, `just file-size-check` — clean. Desktop's 5 biome warnings are pre-existing (reproduced with changes stashed). - All 9 pre-push lanes green, including `rust-tests` and `desktop-tauri-checks`. **Not verified:** not reproduced end-to-end against a live relay under a forced quota burst. The causal chain is source-proven and mutation-proven at the frame level; the ~25s attribution follows from `PUBLISH_TIMEOUT_MS` but is not directly measured. A packaged-build click-through would close that gap. </details> Related work: #6957 bounds Desktop HTTP event submission, but safe retained-operation recovery after exhausted/ambiguous outcomes remains unfinished. #6998 is the separately reviewable Desktop readiness/duplicate-subscription slice. Neither is claimed to complete native before/after startup-send validation. Diagnosis note: `RESEARCH/DESKTOP_STARTUP_SEND_STALL_2026_08_27.md` (Brain's workspace). ## Current review disposition (2026-08-28) The [review on `cd12c938`](https://github.com/block/buzz/pull/6961#pullrequestreview-5052902510) identified ACP's missing rate-limited-OK handling. Commit `3b06dd32493596ec650f20abf8805791c50fdc24` fixes gate arming, re-parking the specifically refused observer frame, and the stale NOTICE comment. Two regressions drive the real frame dispatcher. See [the implementation and validation response](https://github.com/block/buzz/pull/6961#issuecomment-5455032054). The Mobile generation-check inline thread is resolved: its `async publish` returns a failed Future when superseded; it does not throw synchronously at invocation. No further production change was indicated by that comment. The validation counts above describe the original slice, not a new rerun. At `3b06dd324`, the current GitHub check rollup has successful completed test/build checks (non-applicable jobs skipped). The security-review comment still requires review for the current base/head range; do not read a green authorization job as a completed security review. Approval and merge remain human decisions. --------- Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz> Co-authored-by: Carl <9d00794d3df50972eb8b615511783cab12a77a8fd5dd5edd58073ec73b54bd8b@buzz.block.builderlab.xyz>	2026-08-31 14:43:31 -06:00