fix(relay): reject presence updates when Redis storage fails

[loganj]

Summary

• Reject kind:20001 presence events with OK false / error: presence storage unavailable when Redis SET or DEL fails, before publishing, local fan-out, or local-event marking.
• Preserve the producer contract needed by snapshot-confirming consumers: delivered live presence must follow successful mutation of the Redis state read by snapshots.
• Classify those backend rejections with the existing IngestError taxonomy so a presence storage outage counts as buzz_events_rejected_total{transport="ws",reason="error"}, not client reason="invalid"; genuine client-input refusals (verification failure, membership gates) stay invalid, and every wire message is an unchanged fixed sanitized string (review follow-up, no protocol wording change).
• Add actual handle_event integration coverage for rejected online/offline transitions, healthy online→offline accepted/stored/fanned-out behavior, and the rejection-counter routing on storage failure with an invalid-signature control.

This is standalone on main; it does not depend on the mobile implementation. Deploy this relay prerequisite before relying on #7526's snapshot-confirmation policy. Existing pubsub-failure-after-successful-storage behavior and disconnect TTL cleanup are deliberately unchanged. A storage error may be an ambiguous write outcome, not a rollback guarantee; the rejected event is not published by this handler. Clients may retry the generic error: rejection. Desktop's 60s heartbeat retries non-offline presence, not every explicit offline transition.

Related issue

Addresses the relay prerequisite identified in #7526 review 5157607827. Searched open presence/storage PRs; no duplicate relay storage-error rejection fix found. #7382/#7383/#7526 heads and bases are unchanged.

Testing

Exact head: 389174df29cc02d0f885c03209eff661d8bb2ec0 (+380/-13; 393 total), one commit 389174df2 on top of the reviewed c031d6eb1 (DCO-signed; base bfc384855889432df4a333a0edf3080f332ee169 unchanged).

• PASS: cargo fmt --all -- --check, cargo clippy -p buzz-relay --all-targets -- -D warnings, git diff --check, just file-size-check, PostgreSQL discovery validation — all run at the exact final head with a clean tree before and after.
• PASS: documented native scripts/postgres-test-run.sh -p buzz-relay --lib --tests: 89/89 actual integration tests, including the four presence cases (online/offline storage rejection, healthy online→offline, and the new rejection-classification case). Owned PostgreSQL 17/Redis on isolated loopback ports, schema plus reconciliation applied; no shared development database.
• PASS: explicit cargo test -p buzz-relay presence_storage -- --ignored --nocapture: 4/4, not skipped.
• Full isolated relay crate suite at the final head (cargo nextest run -p buzz-relay --lib --tests): 1062 run: 1062 passed, 94 skipped. The previously failing api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo passed in this run (1.5s); it is a known timing-sensitive main baseline failure tracked open in fix(mesh): retain echo receive across housekeeping ticks #7140 and untouched by this PR, so this single passing run is reported as-is and does not claim environmental clearance or close fix(mesh): retain echo receive across housekeeping ticks #7140. No full-suite-green claim is made beyond this run.
• Mobile is untouched; fix(mobile): order presence snapshots against delayed live events #7526's existing 2090-test/format/analyze evidence remains scoped to its unchanged head. Its separate Desktop Smoke E2E (2) failure remains red; no CI retries requested.

Production-seam regression coverage: the metric case drives real handle_event traffic against a genuinely dead Redis endpoint with a seeded active PostgreSQL community and a registered presence watcher, asserts the storage rejection counts reason="error" while a tampered-signature control through the same dispatcher arm stays reason="invalid", and re-asserts the rejected ACK, no fan-out, and no local-event marker. Counter assertions use a thread-local recorder guard held across .await points (the buzz-db counter-test convention) inside the per-process nextest postgres-ci lane, so no parallel test can race the counter snapshot.

No UI change or screenshot. Local logs and reproducible service/gate scripts are retained under WORK_LOGS/MOBILE_FEEDBACK_PRESENCE_20260909/relay_prerequisite/metric_correction/ in the engineering workspace. This PR is a review candidate, not merge clearance.

Causal checks: restoring only the pre-fix production mutation block makes both original rejection tests fail (OK true instead of false); healthy success still passes. Reverting only the typed classification (mapping the ephemeral Internal arm back to invalid) makes the new metric regression fail with the outage counted as [("ws","invalid",2)] instead of [("ws","error",1),("ws","invalid",1)]. The unchanged mesh echo case also failed 504/200 with the main-production block restored in the prior run, supporting its separation from this change without claiming environmental clearance. Candidate source restored byte-for-byte after each mutation. Repository-wide just ci was not rerun; the scoped relay gates above are the new evidence.

[github-actions]

🔐 Codex Security Review

Status: review required for the current range.

The current range is 82656ffea080cc28cec9163ebbf7d1c6be327e43...389174df29cc02d0f885c03209eff661d8bb2ec0.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review 389174df29cc02d0f885c03209eff661d8bb2ec0 to authorize a new review.
Any previous review applies only to its recorded range.

[jedwards27]

Verdict: REQUEST CHANGES

Reviewed: bfc384855889432df4a333a0edf3080f332ee169..c031d6eb1f0aa38b08259036eba4f7ab9991e7bf (exact head c031d6eb1f0aa38b08259036eba4f7ab9991e7bf)

Risk: high — this changes the relay's accepted/fanned-out presence contract and the operational signal emitted when storage is unavailable.

Blocking finding

[P2] Redis storage outage is counted as invalid client input.

Presence SET/DEL failure now correctly returns error: presence storage unavailable before success ACK, local-event marking, Redis publish, or local fan-out (crates/buzz-relay/src/handlers/event.rs:839-862). But the shared ephemeral error arm unconditionally records reject("invalid") (event.rs:743-749). The dispatcher already uses error for backend/lifecycle failure (event.rs:722-729), and reject_with_transport supports that bounded reason (crates/buzz-relay/src/handlers/ingest.rs:296-304).

During a Redis outage, rejected heartbeats therefore inflate buzz_events_rejected_total{reason="invalid"} rather than the backend-error signal, corrupting diagnosis precisely when presence storage is unavailable. The new production-seam tests cover ACK, storage, and fan-out but do not assert the rejection classification.

Author action: preserve a typed backend-failure classification from handle_ephemeral_event (or otherwise classify presence storage failures as error) and add a production-seam metric regression that fails if this path returns to invalid.

Verification owner: author for the classification and regression; :bot: Jude’s code review agent for exact-new-head review.

Confirmed behavior

Both lanes agree the primary ordering repair is sound: successful mutation precedes publish/local fan-out/success ACK, while failed mutation returns before all of them. Snapshot reads use the same tenant-scoped keyspace and surface lookup outage rather than synthesizing Offline. This establishes the producer invariant needed by #7526 once the dependency is structurally landed and deployed.

Validation

At matching clean exact head across the assigned lanes:

• PASS — cargo fmt --all -- --check
• PASS — cargo clippy -p buzz-relay --all-targets -- -D warnings
• PASS — git diff --check
• PASS — just file-size-check and PostgreSQL discovery validation
• Author evidence: 3/3 focused Redis/PostgreSQL presence cases and 88/88 PostgreSQL integration tests; reverting the mutation guard makes both rejection cases fail.
• Full relay execution observed one unchanged mesh-demo echo timeout (504 vs expected 200). No causal attribution to this PR was established; it remains a CI/reviewer confidence gap rather than additional author action.
• Docker-backed independent integration execution was unavailable because managed Docker required organization sign-in. That is reviewer/tooling-owned, not author rework.

Manual/native evidence: not applicable; no UI code changed.

Residual risk: Redis command errors can have ambiguous write outcomes, so a rejected idempotent SET/DEL may already be reflected by a later snapshot. Successful mutation followed by pubsub failure retains pre-existing best-effort delivery semantics. Concurrent same-subject write ordering also predates this diff. Exact-head CI still had unrelated Desktop setup failures/pending jobs during review; gate ownership remains CI infrastructure unless causality is shown.

— :bot: Jude’s code review agent

[jedwards27]

Verdict: REQUEST CHANGES
Reviewed: bfc384855889432df4a333a0edf3080f332ee169..c031d6eb1f0aa38b08259036eba4f7ab9991e7bf (exact head c031d6eb1f0aa38b08259036eba4f7ab9991e7bf)
Risk: high — relay presence mutation, ACK, cross-node publish, local fan-out, and snapshot ordering.

The core correctness fix is sound: successful tenant-scoped SET/DEL now precedes Redis publish, local-event marking, local fan-out, and the success ACK; mutation failure returns before all four and produces one sanitized negative ACK. Snapshot reads use the same community-scoped keyspace and surface Redis lookup failure rather than manufacturing Offline. The production-seam tests cover failed online/offline mutations and healthy online→offline storage/fan-out. No UI surface changed.

Blocking finding

[P2] Redis storage outages are recorded as invalid client events — crates/buzz-relay/src/handlers/event.rs:743-749

Presence SET/DEL failure correctly returns error: presence storage unavailable at event.rs:839-862, but the shared ephemeral error arm unconditionally calls reject("invalid"). That increments buzz_events_rejected_total{transport="ws",reason="invalid"} for every rejected presence heartbeat during a Redis outage, hiding a backend incident inside the client-input bucket. This dispatcher already uses reason="error" for lifecycle-store failure at event.rs:722-729, and handlers/ingest.rs:296-304 supports error as a bounded reason. The new tests verify ACK/fan-out/local marking but not this operational classification.

Author action: preserve typed failure classification from handle_ephemeral_event (or otherwise classify presence-storage failure as error) and add a production-seam metric regression that fails if it returns to invalid.

Verification owner: author for the code/test correction; reviewer/CI for the new exact head.

Validation

At clean exact head c031d6eb1f0aa38b08259036eba4f7ab9991e7bf across the integrated review:

• PASS — cargo fmt --all -- --check
• PASS — cargo clippy -p buzz-relay --all-targets -- -D warnings
• PASS — git diff --check
• PASS — just file-size-check
• PASS — scripts/test-postgres-test-discovery.sh
• FAIL — full cargo test -p buzz-relay: 1039 passed / 1 failed / 92 ignored; the sole api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo failure (HTTP 504 vs 200) matches the disclosed unchanged failure and was not causally attributed to this PR.
• NOT RUN independently — Docker-backed PostgreSQL/Redis cases because managed Docker required sign-in. Author reports focused 3/3 and PostgreSQL lane 88/88; this is a reviewer-tooling confidence gap, not another author defect.

Manual/native evidence: not applicable; no UI or native code changed.

Residual risk: Redis command errors can be ambiguous after server-side mutation; retry remains idempotent for SET/DEL. Successful mutation followed by pubsub failure and concurrent same-subject WS handler ordering are pre-existing and outside this range. CI had four Desktop Smoke failures and pending Rust/artifact jobs during review; Desktop is untouched, so those remain CI-owned unless causality is established.

— :bot: Jude’s code review agent

[loganj]

F4 (rejection classification) addressed at 389174df29cc02d0f885c03209eff661d8bb2ec0 — single file crates/buzz-relay/src/handlers/event.rs. Correction delta over reviewed c031d6eb1 is +213/−8, of which +174/−0 is the new regression test; production code is +39/−8 (21 of those 39 added lines are comments/doc text, so net production logic ≈ +18/−8).

• handle_ephemeral_event now returns Result<(), IngestError> — the existing repo taxonomy, no new error type. The shared ephemeral Err arm maps Internal → reject("error"), Rejected → reject("invalid"), AuthFailed → reject("auth"), forwarding the message string verbatim. So a Redis presence SET/DEL outage and JoinError now count buzz_events_rejected_total{reason="error"}; verification failure and the membership gate keep their pre-existing classification, and every wire message is an unchanged fixed sanitized string.
• Proof: handlers::event::tests::presence_storage_postgres_tests::counts_presence_storage_failure_as_error_not_invalid drives real handle_event traffic against a genuinely dead Redis endpoint with a seeded PostgreSQL community, a registered watcher, and a tampered-signature control through the same dispatcher arm — exact counter map [("ws","error",1), ("ws","invalid",1)]. Reverting only the Internal mapping back to invalid fails that test with [("ws","invalid",2)].
• Gates at the exact head: cargo fmt --check, clippy -p buzz-relay --all-targets -- -D warnings, git diff --check, just file-size-check, PostgreSQL discovery script, focused presence tests 4/4, postgres lane 89/89, full relay crate suite 1062 passed / 94 skipped. The fix(mesh): retain echo receive across housekeeping ticks #7140 mesh timing case passed in this run; it stays a known inherited baseline bug, not fixed or closed here. No remote CI clearance is claimed (head CI is still pending).

[jedwards27]

Reviewed exact head 389174df29cc02d0f885c03209eff661d8bb2ec0 against base bfc384855889432df4a333a0edf3080f332ee169.

The Redis presence mutation now completes before local-event marking, pubsub/fan-out, and positive ACK. Mutation failures map through IngestError::Internal, producing the existing sanitized negative wire ACK and the error metric classification; client/input rejections remain invalid. Tenant scoping is preserved through mutation keys and snapshot reads. The production-seam regression exercises handle_event against unavailable Redis and verifies negative ACK, no fan-out/local marker, plus the exact error=1, invalid=1 split.

Exact-head evidence reviewed:

• CI is terminal green, including PostgreSQL Tests, Rust unit/lint, relay/backend/Desktop integration, Windows, Security, DCO, and release-candidate gates.
• Local whole-package relay run: 1039 passed, 1 failed, 93 ignored; the sole mesh-demo 504 reproduced independently and is the pre-existing #7140 timing/cancellation issue outside this one-file presence diff.
• Formatting, all-target package Clippy with warnings denied, diff check, file-size check, and production-seam discovery checks passed.

No author-actionable defect found. Residual confidence gaps are non-blocking: independent PostgreSQL credentials were unavailable, with the exact-head PostgreSQL CI gate green; the inherited mesh timeout remains owned by #7140.

— :bot: Jude’s code review agent