fix(relay): exempt ephemeral events from Messages quota; add limit_type observability - #4902
Closed
wpfleger96 wants to merge 1 commit into
Closed
wpfleger96 wants to merge 1 commit into
wpfleger96 wants to merge 1 commit into
Conversation
…pe observability
Ephemeral events (kinds 20000–29999) are never persisted by storage, yet
WS admission billed them against the per-minute durable Messages budget.
With buzz-acp publishing up to 90 observer frames/min + 20 typing
indicators/min/channel + 1 presence/min, agents consumed ~111 of their
120/min Messages budget on pure telemetry, causing repeated 40s quota
stalls that blocked real message delivery.
Changes:
- WS admission now skips LimitType::Messages for ephemeral kinds, using
the existing is_ephemeral() range predicate (same one storage uses to
refuse persistence — admission and storage now agree by construction).
Ephemeral events still count against WsEvents so per-second burst
protection remains intact.
- Add agent_ws_events_per_sec to RateLimitConfig (env:
BUZZ_RATE_LIMIT_AGENT_WS_EVENTS_PER_SEC). Agents previously inherited
human_ws_events_per_sec silently. Default matches human default (10/s)
so this is behavior-neutral at merge; tune on builderlab once
limit_type instrumentation data is available.
- Delete three dead tier fields that were defined and env-loadable but
enforced nowhere: agent_elevated_messages_per_min,
agent_platform_messages_per_min, agent_standard_api_calls_per_min.
- Add limit_type to NOTICE/CLOSED rejection text (format: 'quota exceeded
({limit_type}); retry in {N}s' — the 'retry in Ns' phrase is preserved
for client parsers) and to buzz_admission_rejections_total metric as a
new label on both WS and HTTP paths.
Post-deploy validation: ACP Messages rejections should drop to ~0;
any residual >5s retry hint on the WS path indicates an unenumerated
durable WS publisher.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Member
Author
wesbillman
added a commit
that referenced
this pull request
Sep 19, 2026
## Summary Stop charging ephemeral WebSocket activity against the per-minute `Messages` quota. Presence, typing, and observer traffic should not consume the allowance intended for persistable event attempts. - Use the existing `buzz_core::kind::is_ephemeral` predicate (kinds `20000..=29999`) at the second admission gate. All authenticated EVENT/REQ/COUNT traffic still consumes the shared `WsEvents` budget first. Non-ephemeral EVENT attempts remain charged, including attempts later rejected by authorization or validation. - Add bounded `bucket` labels (`messages`, `ws_operations`, `api_calls`) to `buzz_admission_rejections_total`. HTTP changes are metric labeling only. Wire rejection text remains byte-for-byte unchanged, preserving existing client parsers. - Add real-Redis admission regressions and select them in the existing relay integration CI job. No quota increases, identity/keying changes, client changes, new limiter, or deployment. Downstream authorization and fail-closed Redis behavior are unchanged. ### 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 `779af8886caae1317b4de962082429867ab61503` plus the exact patch now committed as `70ea0528594e25e6dcb40cd74620bd5ef0177592` (binary diff SHA-256 `5064e1cbb87ecb9eff10336bae14b912cac442e1edc247aa4f093de30e4f9717`). Normal commit/push hooks subsequently passed on the committed head, without bypass. Passed: - Independent review and rerun: 11 rejection tests (including real Redis) plus 4 admission tests. Covers ephemeral range boundaries, human/agent budgets, persistable events, ephemeral admission after message exhaustion, mixed EVENT/REQ/COUNT flood exhaustion, Redis outage, exact wire payloads, and bucket metrics. Three mutations were caught: reintroducing ephemeral billing, exempting stored events, and bypassing WS rejection. - `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. - Relay all-target Clippy, formatting, workflow actionlint, pre-commit/signoff hooks, and all applicable pre-push lanes (`push-head-scope`, `branch-skew`, `file-size-check`, `rust-tests`, `desktop-tauri-checks`). - Release relay/CLI/admin build and isolated release-binary workflow: channel creation, signed root/reply sends, exact-ID readback, and thread retrieval. Debug and release WS smoke admitted three presence events with a Messages limit of two, admitted two persistable events, rejected the third, still admitted presence afterward, and rejected author mismatch. Debug Prometheus confirmed `bucket="messages"`. **Outstanding validation gap:** the additional `cargo test -p buzz-relay --no-fail-fast` run was not green: library 1,055 passed / 2 failed / 100 ignored; binary 13 passed / 1 ignored; boot integration 9 passed. Failures were `api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo` (504 instead of 200) and `telemetry::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 ci` was 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 `55fe5f16618a51d03ad48d6f2f97b2f8b463607a` holds 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 `70ea052859` plus this test patch, before the pre-commit-only assertion wrapping: full `cargo test -p buzz-agent --no-fail-fast` passed (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. --------- Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Co-authored-by: kiranmagic7 <262980978+kiranmagic7@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Relay WS admission bills every EVENT kind against the per-pubkey durable-message quota (), regardless of whether the event is persisted. With buzz-acp publishing up to 90 observer frames/min + 20 typing indicators/min/channel + 1 presence/min, agents consumed ~111 of their 120/min
Messagesbudget on pure telemetry — leaving only 9 msg/min for real messages and causing repeated 40s quota stalls.Additionally, agents silently inherited the human WS burst budget with no dedicated config field, and three tier-config fields (
agent_elevated_messages_per_min,agent_platform_messages_per_min,agent_standard_api_calls_per_min) were defined, env-loadable, and enforced nowhere.Changes
Core fix —
crates/buzz-relay/src/connection.rsEphemeral events (kinds 20000–29999) now skip
LimitType::Messagesin WS admission. Uses the existingis_ephemeral()range predicate frombuzz-core— the same onebuzz-dbuses to refuse persistence, so admission and storage agree by construction. Ephemeral events still count againstWsEvents(per-second burst protection unchanged).Observability —
crates/buzz-relay/src/connection.rs,src/api/bridge.rslimit_typeis now included in:rate-limited: quota exceeded (ws_events); retry in 5srate-limited: quota exceeded (api_calls); retry in 3sbuzz_admission_rejections_totalmetric labelThe
retry in {N}sphrase is preserved intact — ACP and CLI both parse it.Agent WS budget —
crates/buzz-auth/src/rate_limit.rs,crates/buzz-relay/src/config.rsAdded
agent_ws_events_per_sectoRateLimitConfigwith env overrideBUZZ_RATE_LIMIT_AGENT_WS_EVENTS_PER_SEC. Default matches the human default (10/s) so this is behavior-neutral at merge. Tune on builderlab oncelimit_typeinstrumentation data establishes the right operating value.Dead config cleanup —
crates/buzz-auth/src/rate_limit.rs,crates/buzz-relay/src/config.rsDeleted three fields enforced nowhere:
agent_elevated_messages_per_min,agent_platform_messages_per_min,agent_standard_api_calls_per_min. Removal is grep-clean — no dangling readers in the owned crates.Tests
New unit tests cover:
LimitType::as_str()values are stable (breaking change if they change — they appear in metric labels)RateLimitConfig::default()hasagent_ws_events_per_secequal to human defaultsend_admission_result: NOTICE text names the limit type and preservesretry in Nsphrase for bothMessagesandWsEvents;Ok(())sends nothing; sub-scoped rejection emits CLOSEDBUZZ_RATE_LIMIT_AGENT_WS_EVENTS_PER_SECenv override works and rejects zeroFull suite: 844 passing / 1 pre-existing failure (
mesh_demo— reproduces onorigin/mainbefore this branch).Post-deploy validation
After deploy,
buzz_admission_rejections_total{reason="quota",limit_type="messages"}for agent pubkeys should drop to ~0. Any residual>5sretry hint on the WS path indicates an unenumerated durable WS publisher.