Skip to content

fix(relay): exempt ephemeral events from Messages quota; add limit_type observability - #4902

Closed
wpfleger96 wants to merge 1 commit into
mainfrom
duncan/ephemeral-admission-fix
Closed

wpfleger96 wants to merge 1 commit into
mainfrom
duncan/ephemeral-admission-fix

Conversation

@wpfleger96

Copy link
Copy Markdown
Member

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 Messages budget 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.rs

Ephemeral events (kinds 20000–29999) now skip LimitType::Messages in WS admission. Uses the existing is_ephemeral() range predicate from buzz-core — the same one buzz-db uses to refuse persistence, so admission and storage agree by construction. Ephemeral events still count against WsEvents (per-second burst protection unchanged).

Observability — crates/buzz-relay/src/connection.rs, src/api/bridge.rs

limit_type is now included in:

  • WS rejection NOTICE/CLOSED text: rate-limited: quota exceeded (ws_events); retry in 5s
  • HTTP 429 response body: rate-limited: quota exceeded (api_calls); retry in 3s
  • buzz_admission_rejections_total metric label

The retry in {N}s phrase is preserved intact — ACP and CLI both parse it.

Agent WS budget — crates/buzz-auth/src/rate_limit.rs, crates/buzz-relay/src/config.rs

Added agent_ws_events_per_sec to RateLimitConfig with env override BUZZ_RATE_LIMIT_AGENT_WS_EVENTS_PER_SEC. Default matches the human default (10/s) so this is behavior-neutral at merge. Tune on builderlab once limit_type instrumentation data establishes the right operating value.

Dead config cleanup — crates/buzz-auth/src/rate_limit.rs, crates/buzz-relay/src/config.rs

Deleted 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() has agent_ws_events_per_sec equal to human default
  • send_admission_result: NOTICE text names the limit type and preserves retry in Ns phrase for both Messages and WsEvents; Ok(()) sends nothing; sub-scoped rejection emits CLOSED
  • BUZZ_RATE_LIMIT_AGENT_WS_EVENTS_PER_SEC env override works and rejects zero

Full suite: 844 passing / 1 pre-existing failure (mesh_demo — reproduces on origin/main before 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 >5s retry hint on the WS path indicates an unenumerated durable WS publisher.

…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>
@wpfleger96
wpfleger96 requested a review from a team as a code owner August 5, 2026 15:29
@wpfleger96

Copy link
Copy Markdown
Member Author

🤖 Superseded by #4912, which carries the same ephemeral-exemption + limit_type change as part of the consolidated single-PR approach. Branch kept — the agent_ws_events_per_sec config and dead tier-field removal from this PR aren't in #4912 and will follow up separately after it lands.

@wpfleger96 wpfleger96 closed this Aug 5, 2026
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>
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