fix(relay): exclude ephemeral activity from message quota - #7736
Conversation
Keep the shared WebSocket flood budget and existing authorization paths. Identify exhausted buckets in rejection metrics without changing wire errors. Exercise real Redis accounting and exact rejection payloads in CI. Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
🔐 Codex Security Review
|
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Reviewed exact head 70ea0528594e25e6dcb40cd74620bd5ef0177592 against base 779af8886caae1317b4de962082429867ab61503. No author-actionable defect found.
The two-stage contract is coherent:
EVENT,REQ, andCOUNTstill spend the sharedWsEventsbudget before dispatch; only persistableEVENTs then spendMessages(crates/buzz-relay/src/rejection.rs:66-103). A WebSocket-budget rejection returns before the message counter can advance.- The exemption uses the canonical NIP-01 ephemeral range
20000..=29999(crates/buzz-core/src/kind.rs:772-774), matching the relay's downstream non-storage boundary (crates/buzz-relay/src/handlers/event.rs:693-746). This covers presence, typing, and observer activity without creating a second allowlist. - Quota identity remains the authenticated connection pubkey, and Redis keys remain community + authenticated pubkey + bucket. The existing Lua limiter retains atomic increment/expiry behavior and admission still fails closed when Redis is unavailable.
- EVENT rejection remains correlated as
OK(event_id, false, reason); REQ/COUNT remainCLOSED(sub_id, reason). The wire reason grammar is unchanged. The new metrics use only static bucket labels (messages,ws_operations,api_calls), so they do not introduce request-derived cardinality.
Risk-shaped evidence at the exact head:
- Independent real-Redis runs passed the quota regressions, including human/agent message exhaustion, ephemeral activity after exhaustion, mixed EVENT/REQ/COUNT WebSocket exhaustion, HTTP accounting, and Redis-unavailable behavior.
- Two independent mutations of the production ephemeral predicate caused the exhaustion regression to fail, establishing that the test is causal rather than decorative.
cargo fmt --all -- --check, relay all-target Clippy,git diff --check, and the focused rejection/admission tests passed in reviewer checkouts.- Hosted
Relay Artifact Producer / Desktop E2E Relaypassed, exercising the newly selected ignored Redis quota tests in CI.
Residual verification
- Exact-head
Rust / Unit Testsis red on untouchedbuzz-agent::fake_llm::cancelled_turn_with_usage_emits_notification_before_response(stopReasonnull vscancelled). The PR changes only four relay/workflow files, and base/main SHA779af888…previously completed CI successfully. Author action: none based on current evidence. Verification owner: CI/merge policy. - Full live two-client pub/sub observation after Messages exhaustion was not independently repeated. Deterministic production-entrypoint Redis coverage is sufficient for this change; additional operational confirmation belongs to release/dogfood, not author rework.
Any new head invalidates this approval.
Adapt the ACK barrier and ordered frame replay from PR #5003, original commit cc8dc61. Preserve usage and prompt frames received before the acknowledgement, validate its success, and explicitly require the cancelled prompt response. No production changes. Co-authored-by: kiranmagic7 <262980978+kiranmagic7@users.noreply.github.com> Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Reviewed 779af8886caae1317b4de962082429867ab61503..55fe5f16618a51d03ad48d6f2f97b2f8b463607a at exact live head 55fe5f16618a51d03ad48d6f2f97b2f8b463607a. Risk is high because this PR changes relay quota semantics and repairs an agent-runtime cancellation test harness. No unresolved author-actionable defect remains.
The relay quota correction approved at the parent remains intact. The new one-file test delta replaces the invalid assumption that writing session/cancel means cancellation was processed with a real protocol fence: production sets the session watch before enqueueing the successful cancellation response (crates/buzz-agent/src/lib.rs:320-323, 609-614), while the run loop concurrently and preferentially observes that watch (crates/buzz-agent/src/agent.rs:406-410). Waiting for the matching successful ACK before releasing the fake provider is therefore a valid happens-before barrier without deadlock.
Frames consumed before that ACK are retained and replayed exactly once in FIFO order, and the budget grows by the retained count (crates/buzz-agent/tests/fake_llm.rs:1421-1437). The test still requires observed usage before the prompt response, a completed prompt with stopReason: cancelled, and a successful null/error-free cancellation ACK (1424-1465). The harness receive path has a bounded 10-second timeout and loud EOF/JSON failures, so missing ACK, usage, or completion cannot silently pass. The delta is test-only versus parent 70ea0528594e25e6dcb40cd74620bd5ef0177592; it does not alter production cancellation behavior or conceal prompt-before-usage ordering.
Provenance was checked against #5003 commit cc8dc6124957acfd67fe2336396b58e0ff43ec60. This adaptation preserves its ACK/replay mechanism and strengthens it with ACK-shape validation and explicit prompt completion. The unrelated repair is bounded to the pre-existing harness failure that blocked this PR's hosted Rust gate, and authorship is preserved.
Findings
- Concrete defects: none.
- Author action: none.
- Verification owner: reviewer/tooling completed source, provenance, full-package, and repetition checks; CI/merge policy owns remaining hosted gates.
Validation at matching head
- Two independent clean dedicated worktrees ran
cargo test -p buzz-agent --no-fail-fast: PASS in both; 715 tests passed across targets, one ignored. - Targeted cancellation test repetition: PASS 40/40 with four concurrent workers in one lane and PASS 50/50 in another.
git diff --check 779af8886caae1317b4de962082429867ab61503..55fe5f16618a51d03ad48d6f2f97b2f8b463607a: PASS.- Source trace covered cancellation ACK ordering, watch delivery, provider select behavior, frame replay cardinality/order, timeout behavior, retained assertions, and the production usage-before-prompt writer contract.
- Hosted exact-head
Rust / Unit Tests,Rust / Rust Lint, DCO, security, and cross-compile gates are green at review time.
Residual risk
The pre-fix scheduler race did not reproduce locally in control repetitions; causality rests on source ordering plus the documented hosted failures in #4945. Several hosted matrix lanes, including Relay Artifact Producer / Desktop E2E Relay, were still running with no failures at the freshness check. Neither gap requires author rework; CI/merge policy owns completion. No manual/native run was warranted for this test-only follow-up. Any new head invalidates this approval.
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Reviewed exact head 55fe5f16618a51d03ad48d6f2f97b2f8b463607a against base 779af8886caae1317b4de962082429867ab61503. No author-actionable defect remains.
The new test-only cancellation synchronization is sound:
session/cancelawaitscancel_session, sets the active turn's cancellation watch, and only then sends the successful JSON-RPC response (crates/buzz-agent/src/lib.rs:320-323,609-614). Waiting for that exact ACK before releasing the gated provider therefore establishes the intended happens-before relation instead of assuming a stdin write has already been processed.- Frames consumed while waiting for the ACK are retained and replayed once in FIFO order (
crates/buzz-agent/tests/fake_llm.rs:1421-1437). The scan budget grows by the retained count, preserving the prior post-ACK allowance without dropping or double-counting frames. - The test still requires usage before the prompt response and
stopReason: cancelled, and now also requires a successful null ACK and prompt completion (fake_llm.rs:1424-1465). Timeout, EOF, and JSON failures remain loud. - The delta from the previously approved
70ea0528594e25e6dcb40cd74620bd5ef0177592is confined tocrates/buzz-agent/tests/fake_llm.rs; production cancellation and relay quota behavior are unchanged. The mechanism matches the #5003 repair while adding stronger ACK-shape and completion assertions.
Risk-shaped exact-head evidence:
- Three independent clean-checkout package runs of
cargo test -p buzz-agent --no-fail-fastpassed. The two A Team lanes additionally ran the target 40 and 50 times respectively without failure. cargo clippy -p buzz-agent --all-targets -- -D warningspassed in the integration checkout.- Hosted exact-head
Rust / Unit Tests,Rust / Rust Lint, DCO, security, cross-compile, macOS build, mobile Swift, and container builds passed at verdict time.
Residual verification
- The exact-head relay artifact and several Desktop lanes were still running at the final poll. This head does not change their production code, and the relay artifact passed on the previously reviewed parent; Author action: none. Verification owner: CI / merge policy. Any PR-caused required-gate failure must still be resolved before merge.
- Local control mutations did not reproduce the scheduler-dependent pre-fix race on reviewer hosts. Source ordering and the independently documented historical reproduction establish the defect mechanism; longer scheduler stress belongs to hosted CI/tooling, not author rework.
Any new head invalidates this approval.
* origin/main: (75 commits) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) fix(desktop): keep managed agent avatars usable across communities (#7732) fix(mobile): fail open when age checks are unavailable (#7714) fix(ci): don't run desktop tests for purely mobile client changes (#7709) fix(mobile): temporarily disable age gating (#7708) feat(db): expose connection setup metrics (#7286) Isolate S3 storage metrics from the relay (#7543) fix(web): route mobile invite downloads to app stores (#7702) feat(mobile): show build number with version in settings (#7697) release(mobile-infra): buzz-push-gateway 0.3.0 (#7685) Add authenticated WebSocket recovery telemetry (#7546) Instrument database pool roles (#7356) ... # Conflicts: # crates/buzz-db/src/runtime/migration.rs
* origin/main: (81 commits) fix(mobile): avoid opening empty threads on message tap (#7756) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) fix(desktop): keep managed agent avatars usable across communities (#7732) fix(mobile): fail open when age checks are unavailable (#7714) fix(ci): don't run desktop tests for purely mobile client changes (#7709) fix(mobile): temporarily disable age gating (#7708) feat(db): expose connection setup metrics (#7286) Isolate S3 storage metrics from the relay (#7543) fix(web): route mobile invite downloads to app stores (#7702) feat(mobile): show build number with version in settings (#7697) release(mobile-infra): buzz-push-gateway 0.3.0 (#7685) Add authenticated WebSocket recovery telemetry (#7546) ... Signed-off-by: Hayt <9e1c23a3fd83f61da34420e4e88ff1b16e45cafcc0cd9019eb07d4ecfa8ca9b0@buzz.block.builderlab.xyz>
* origin/main: (75 commits) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) fix(desktop): keep managed agent avatars usable across communities (#7732) fix(mobile): fail open when age checks are unavailable (#7714) fix(ci): don't run desktop tests for purely mobile client changes (#7709) fix(mobile): temporarily disable age gating (#7708) feat(db): expose connection setup metrics (#7286) Isolate S3 storage metrics from the relay (#7543) fix(web): route mobile invite downloads to app stores (#7702) feat(mobile): show build number with version in settings (#7697) release(mobile-infra): buzz-push-gateway 0.3.0 (#7685) Add authenticated WebSocket recovery telemetry (#7546) Instrument database pool roles (#7356) ...
* origin/main: (75 commits) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) fix(desktop): keep managed agent avatars usable across communities (#7732) fix(mobile): fail open when age checks are unavailable (#7714) fix(ci): don't run desktop tests for purely mobile client changes (#7709) fix(mobile): temporarily disable age gating (#7708) feat(db): expose connection setup metrics (#7286) Isolate S3 storage metrics from the relay (#7543) fix(web): route mobile invite downloads to app stores (#7702) feat(mobile): show build number with version in settings (#7697) release(mobile-infra): buzz-push-gateway 0.3.0 (#7685) Add authenticated WebSocket recovery telemetry (#7546) Instrument database pool roles (#7356) ... Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…p-20260915 * origin/main: (28 commits) fix(mobile): avoid opening empty threads on message tap (block#7756) fix(workflows): make deletion persistent and retryable (block#7735) fix(mobile): preserve thread replies through refresh failures (block#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (block#7758) fix(relay): exclude ephemeral activity from message quota (block#7736) release: push gateway chart 0.3.1 (block#7749) fix(push): label plaintext push gateway service as HTTP (block#7717) Replace personal and internal data in desktop test fixtures (block#7748) Add mobile VISION (block#7710) fix(mobile): keep relay sessions stable during push lease updates (block#7745) fix(desktop): keep managed agent avatars usable across communities (block#7732) fix(mobile): fail open when age checks are unavailable (block#7714) fix(ci): don't run desktop tests for purely mobile client changes (block#7709) fix(mobile): temporarily disable age gating (block#7708) feat(db): expose connection setup metrics (block#7286) Isolate S3 storage metrics from the relay (block#7543) fix(web): route mobile invite downloads to app stores (block#7702) feat(mobile): show build number with version in settings (block#7697) release(mobile-infra): buzz-push-gateway 0.3.0 (block#7685) Add authenticated WebSocket recovery telemetry (block#7546) ... Signed-off-by: Brainy Bumble <0ed7657b57c0e8a9f5288390dd6c8d5d0a3a06abe9b01b9006814f52077d6cdf@buzz.block.builderlab.xyz>
…ecurity * origin/main: (22 commits) feat(relay): add admin HTTP routes for member restriction management (#7302) fix(relay): fire kick live side effects at convergence; persist target; fence re-add race with held lock (#7298) feat(relay): add atomic complete read-state snapshots (#7572) fix(desktop): register macOS badges for new and existing installs (#7783) fix(mobile): avoid opening empty threads on message tap (#7756) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) fix(desktop): keep managed agent avatars usable across communities (#7732) fix(mobile): fail open when age checks are unavailable (#7714) fix(ci): don't run desktop tests for purely mobile client changes (#7709) fix(mobile): temporarily disable age gating (#7708) feat(db): expose connection setup metrics (#7286) Isolate S3 storage metrics from the relay (#7543) ... Signed-off-by: Alex Rosenzweig <arosenzweig@squareup.com>
…ness-overload * origin/main: (74 commits) feat(relay): add admin HTTP routes for member restriction management (#7302) fix(relay): fire kick live side effects at convergence; persist target; fence re-add race with held lock (#7298) feat(relay): add atomic complete read-state snapshots (#7572) fix(desktop): register macOS badges for new and existing installs (#7783) fix(mobile): avoid opening empty threads on message tap (#7756) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) fix(desktop): keep managed agent avatars usable across communities (#7732) fix(mobile): fail open when age checks are unavailable (#7714) fix(ci): don't run desktop tests for purely mobile client changes (#7709) fix(mobile): temporarily disable age gating (#7708) feat(db): expose connection setup metrics (#7286) Isolate S3 storage metrics from the relay (#7543) ... Signed-off-by: tornquist <tornquist@squareup.com>
…rcement * origin/main: (87 commits) feat(relay): add admin HTTP routes for member restriction management (#7302) fix(relay): fire kick live side effects at convergence; persist target; fence re-add race with held lock (#7298) feat(relay): add atomic complete read-state snapshots (#7572) fix(desktop): register macOS badges for new and existing installs (#7783) fix(mobile): avoid opening empty threads on message tap (#7756) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) fix(desktop): keep managed agent avatars usable across communities (#7732) fix(mobile): fail open when age checks are unavailable (#7714) fix(ci): don't run desktop tests for purely mobile client changes (#7709) fix(mobile): temporarily disable age gating (#7708) feat(db): expose connection setup metrics (#7286) Isolate S3 storage metrics from the relay (#7543) ... Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com> # Conflicts: # crates/buzz-auth/src/nip98.rs # crates/buzz-relay/Cargo.toml
…-dev * origin/main: fix(desktop): bound startup request bursts and recover quota refusals (#7790) fix(audit): frame hash inputs with TLV (#7492) fix(admin): allow cold storage worker DB startup (#7770) feat(relay): add admin HTTP routes for member restriction management (#7302) fix(relay): fire kick live side effects at convergence; persist target; fence re-add race with held lock (#7298) feat(relay): add atomic complete read-state snapshots (#7572) fix(desktop): register macOS badges for new and existing installs (#7783) fix(mobile): avoid opening empty threads on message tap (#7756) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) Signed-off-by: Tom Brow <tomb@block.xyz>
Summary
Stop charging ephemeral WebSocket activity against the per-minute
Messagesquota. Presence, typing, and observer traffic should not consume the allowance intended for persistable event attempts.buzz_core::kind::is_ephemeralpredicate (kinds20000..=29999) at the second admission gate. All authenticated EVENT/REQ/COUNT traffic still consumes the sharedWsEventsbudget first. Non-ephemeral EVENT attempts remain charged, including attempts later rejected by authorization or validation.bucketlabels (messages,ws_operations,api_calls) tobuzz_admission_rejections_total. HTTP changes are metric labeling only. Wire rejection text remains byte-for-byte unchanged, preserving existing client parsers.Related issue
Duplicate search found overlapping work: open #4912 and closed #4902. They propose the same ephemeral-accounting correction alongside broader wire/client/config changes. The relay fix is a deliberately narrower four-file implementation on the current admission module, with metrics-only diagnostics and real-Redis regression coverage. A separate test-only CI repair is documented below. It does not close or claim to supersede those PRs.
Related symptom: #7411. This PR does not resolve subscription bursts exhausting the shared WS budget; that budget is intentionally preserved. It also does not establish or fix the cause of HTTP quota exhaustion.
Testing
Validation ran against base
779af8886caae1317b4de962082429867ab61503plus the exact patch now committed as70ea0528594e25e6dcb40cd74620bd5ef0177592(binary diff SHA-2565064e1cbb87ecb9eff10336bae14b912cac442e1edc247aa4f093de30e4f9717). Normal commit/push hooks subsequently passed on the committed head, without bypass.Passed:
just test-unit: all 17 runner groups;just test-integration: all 3 runner groups. Auth integration reported no tests; ignored database integration tests are not claimed as executed.push-head-scope,branch-skew,file-size-check,rust-tests,desktop-tauri-checks).bucket="messages".Outstanding validation gap: the additional
cargo test -p buzz-relay --no-fail-fastrun was not green: library 1,055 passed / 2 failed / 100 ignored; binary 13 passed / 1 ignored; boot integration 9 passed. Failures wereapi::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo(504 instead of 200) andtelemetry::tests::trace_context_lookup_does_not_enable_callsites(tracing assertion). Neither failure was diagnosed or repaired in this scope, and this PR does not claim a fully green package suite.just ciwas not run locally; hosted CI remains to validate the PR.All runtime tests used isolated local services. Owned services have been stopped; nothing was deployed and no production data was touched.
CI cancellation-test repair
At the original relay head, hosted Rust unit tests failed on the pre-existing cancellation harness race tracked in #4945. With owner authorization, this PR now adapts the one-file repair from #5003 (original commit
cc8dc6124957acfd67fe2336396b58e0ff43ec60), preserving author credit. #5003's old hosted workflows failed without starting jobs; their status is not used as validation evidence.Commit
55fe5f16618a51d03ad48d6f2f97b2f8b463607aholds the fake provider response until a successful cancellation acknowledgement, then replays all pre-acknowledgement usage/prompt frames in order. It retains the cancellation and usage-before-response assertions and explicitly requires prompt completion. No production code changes in this follow-up.Validation on parent
70ea052859plus this test patch, before the pre-commit-only assertion wrapping: fullcargo test -p buzz-agent --no-fail-fastpassed (715 tests, one ignored); 100 targeted repetitions passed with four concurrent workers; all-target buzz-agent Clippy passed. The unmodified parent also passed 50 local repetitions, so this is not claimed as a local reproduction of the intermittent CI failure. The synchronization defect was confirmed from the source, original CI log, and #4945. Hosted CI must validate the updated head.Independent source review cleared exact commit
55fe5f16618a51d03ad48d6f2f97b2f8b463607a. Normal pre-commit/signoff and all applicable pre-push hooks passed without bypass. The original relay-package mesh-demo and telemetry failures remain disclosed above and were not modified by this test-only repair.