fix(mobile): distinguish unknown presence on identity surfaces - #7383
fix(mobile): distinguish unknown presence on identity surfaces#7383loganj wants to merge 1 commit into
Conversation
Presence states — Flutter widget rendersCaptured at 390×844 from the actual widgets with Inter loaded. These are not simulator/device or relay-backed E2E screenshots. DM header: unknown versus confirmed
Profile: unknown versus confirmed
|
Independent presence UI readiness evidence — 2026-09-07Verified current stacked range:
UI review confirmed Unknown labels and absent unconfirmed dots, distinct confirmed Online/Away/Offline presentation, preserved avatar geometry, and one owner for the profile chip's accessibility label. The existing screenshot comment correctly labels its 390×844 Flutter widget renders; fresh exact-head widget renders were also checked for distinct hashes. These are not simulator/device or relay-backed E2E evidence. No Xcode/simulator is available on this host; a device-equipped validator is still needed for a real foreground/background/reconnect walkthrough. The trusted advisory intentionally cannot run against this stacked base (only open PRs targeting main are eligible; its skipped jobs are not required merge gates). No base/authorization bypass was attempted. The parent's authorized advisory reached a final cancelled result after exceeding its 40-minute job timeout, with no output/artifacts available; it is not a clean security result. The trusted-workflow maintainer must resolve that runner/action failure and rerun the current authorized parent range before advisory findings can be inspected. No substantive mobile defect identified. Presence remains conversational availability rather than substrate-health telemetry. Both PRs remain drafts; no merge/readiness-state change was made. Durable evidence: |
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — APPROVE at exact head dd955a0c2ac80ada39b416a2f8b7481afb8773bf against base a8e8028bfb9bb67de2db03ed448839e2f3ab2b0d.
No concrete defects found.
The renderer correction preserves the intended epistemic boundary across all three production identity surfaces: absent cache data remains unknown, while only confirmed relay state renders offline. Unknown suppresses the DM header/list presence dot and header avatar notch; the profile renders an explicit Presence: Unknown semantic with a single accessibility owner. Confirmed online, away, and offline remain visually distinct.
The surrounding state contract also holds: presence is scoped by community/account, generation- and revision-fenced across scope/lifecycle/reconnect changes, rejects invalid or stale live events, and treats snapshot failure as unknown. A successful host-bound snapshot may authoritatively classify omissions as offline. This PR changes no persistence or downgrade format.
Validation on a clean exact-head checkout:
just mobile-check— PASS; formatter clean and analyzer reported no issues.just mobile-test— PASS; full mobile package, 2,086 tests.- Production-seam mutation matrix — PASS; restoring the old unknown→offline fallback independently in header, list, and profile caused each corresponding regression test to fail, then shipped bytes were restored.
- Opt-in production-widget rendering — PASS; eight 390×844 state renders were distinct and manual inspection confirmed truthful Unknown vs Offline presentation, stable avatar bounds, no unknown dot/notch, and readable profile chips.
- Fresh GitHub preflight — head/base unchanged, mergeable/CLEAN, zero unresolved threads, and Mobile, aggregate, DCO, Semgrep, and zizmor checks successful.
Confidence gaps, not author defects: no authenticated native iOS cold-open → snapshot → reconnect journey, dark-theme/wide/iPad/large-text pass, or live VoiceOver walkthrough was performed. Flutter production-widget evidence reduces but does not erase native rendering and lifecycle risk. Optional native/device verification belongs to reviewer/tooling ownership; author action: none.
## 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](#7526 (review)).
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 #7140 and untouched by this PR, so this single passing
run is reported as-is and does not claim environmental clearance or
close #7140. No full-suite-green claim is made beyond this run.
- Mobile is untouched; #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](https://github.com/block/buzz/blob/389174df29cc02d0f885c03209eff661d8bb2ec0/crates/buzz-relay/src/handlers/event.rs#L1491-L1803):
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.
---------
Signed-off-by: Logan Johnson <loganj@squareup.com>
a8e8028 to
2e3eae6
Compare
dd955a0 to
82fd3ae
Compare
|
Rebased onto corrected #7382 at The old reviewed child-only boundary was Integration retained the parent’s authoritative-empty-snapshot geometry expectations (confirmed Offline has a dot/notch), rather than replaying obsolete Unknown expectations into those fixtures. The added state-transition/accessibility tests are unchanged from the reviewed child. No outstanding author-action finding was present in the literal prior review. Fresh isolated offline, lock-enforced Flutter validation of the exact published tree: 2116 tests passed, analyzer clean, formatter 554 files unchanged; package graph has 222 private roots including Initial CI run 34556289645 is in progress; DCO passed. This is a re-review request, not a new approval or merge-readiness claim. #7526 still needs its own subsequent stack update. |
Signed-off-by: Logan Johnson <loganj@squareup.com>
2e3eae6 to
3ff2f29
Compare
82fd3ae to
9a0fc26
Compare
|
@jedwards27 — requesting exact-new-head re-review after independent review and parent-first publication. Head: The true child patch is unchanged from its previous actual parent→child diff; this restack retains the geometry/Unknown/accessibility widget coverage on the corrected governing root #7382. Production source is identical to the parent, including its new equal-second ambiguity correction. The prior approval was for an older head, so fresh review of this head is requested. Freshness/producer evidence: the root increments the per-key revision before queuing ambiguous evidence. A pre-event in-flight response cannot settle that key; the pending loop issues a distinct subsequent query. Idle coalescing groups keys, not response evidence. Local evidence (one composed leaf execution, not separate 2126-test runs per ancestor): immutable leaf Causal rollback: minimal Outstanding: initial exact-head CI is pending/in progress, not green; old-head CI is not evidence for these commits. Re-review and required new-head CI remain merge gates. No new Rust/Desktop runtime, simulator/native or live deployment acceptance is claimed. Structural #7532 ancestry is not a deployed relay receipt: release ownership must still establish the deployed producer guard and relay-before-client rollout/native acceptance. No merge or release clearance is requested by this evidence post. |
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: APPROVE
Reviewed: 3ff2f290bac41811fe2db0b141961148740ebce5..9a0fc26d3deb4ef826dea33267873423a35d38ff (exact live head; prior approval at dd955a0c… expired)
Risk: high — presence truthfulness crosses relay snapshot ordering, scoped lifecycle state, and three visible identity surfaces.
No author-actionable defect remains in this child. The changed range is tests-only; production comes from its exact base. Snapshot reads are fresh authenticated host/tenant-bound queries; missing/failed evidence remains unknown, while successful omission alone means confirmed offline. Scope, generation, epoch, disconnect, lifecycle, and late-query fences prevent state crossing identity/community sessions (mobile/lib/features/profile/presence_cache_provider.dart:32-72,92-142,166-218).
The repository-wide production-consumer trace found exactly three Mobile consumers, all preserving the distinction:
- DM list: unknown renders no presence dot; confirmed states use their intended indicators (
mobile/lib/features/channels/channels_page/channel_tile.dart:177-188,220-247). - DM header: unknown renders
Unknownwith no dot or avatar notch; confirmed offline remains explicitlyOffline(mobile/lib/features/channels/channel_detail_page/app_bar.dart:218-228,247-298,326-331;mobile/lib/shared/widgets/masked_avatar_badge.dart:309-330). - Profile sheet: unknown and confirmed states map to truthful labels with one owned
Presence: <state>semantics stop (mobile/lib/features/profile/user_profile_sheet.dart:52-78,338-379).
Author action: none.
Verification owner: reviewer/tooling for optional native iOS cold-open/background/reconnect, dark-theme, Dynamic Type, and VoiceOver observation; CI owns remaining unrelated full-suite instability. These are confidence gaps, not author rework.
Exact-head validation
- Focused provider/cache tests: 21/21 passed.
- Full changed consumer suites: 268 passed.
- Three production-seam UI mutations (
null→offline in header/profile; unconditional DM-list dot) each failed on the intended truthfulness assertion, then were restored cleanly. - Equal-second
<=→<mutation failed the intended online/offline ordering assertions, then was restored cleanly. - Full Mobile runs from the two lanes reached 2,118 and 2,114 passes respectively but had unrelated untouched-test timeout/failure noise; the changed suites pass in full and exact-head GitHub Mobile is green.
git diff --checkand changed-file formatting passed; final lane trees were clean and live head rechecked exact.
No native artifact was produced; source, production-widget tests, and causal mutations establish the changed contract. Any new head invalidates this approval.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — APPROVE
Reviewed: 3ff2f290bac41811fe2db0b141961148740ebce5..9a0fc26d3deb4ef826dea33267873423a35d38ff (exact live head re-fetched before submission)
Risk: medium — user-visible mobile identity truthfulness plus an adjacent relay/cache freshness contract. This child is tests-only (+212/-1 across four Mobile test files); production bytes are inherited unchanged from the exact base.
Behavior/contracts traced: authenticated host-bound presence snapshots; Redis failure versus authoritative successful omission; live/snapshot ordering; (baseUrl,nsec) scope; disconnect/background/reconnect/dispose fencing; bounded polling; and every Mobile production consumer found under mobile/lib: DM header, DM list, and viewed profile.
Findings: no blocking or non-blocking code defect.
- Missing or failed evidence remains absent/Unknown; only a successful snapshot omission becomes confirmed Offline. Fresh query responses are generation/epoch/revision fenced, including equal-second live/snapshot ambiguity.
- DM header renders
Unknownwithout a presence badge or avatar notch; confirmed Online/Away/Offline retain distinct labels and dots. - DM list hides the dot for unknown and preserves fixed avatar bounds while confirmed states use success/warning/outline colors.
- Profile renders truthful Unknown/Online/Away/Offline text with one
Presence: <state>accessibility owner and no duplicate child announcement. - Production-seam mutation checks independently restored the old unknown→offline behavior in header/profile and forced an unknown DM-list dot; each corresponding regression failed, then shipped bytes were restored. The cache ordering
<=→<mutation also failed on equal-second truth inversions.
Author action: none.
Verification owner: reviewer/tooling for optional native iOS cold-open/background/reconnect, dark-theme, Dynamic Type, and VoiceOver observation. The existing failing Desktop Smoke E2E aggregate is outside this four-file Mobile child boundary; CI/repository gate ownership must still resolve it before merge. Parent PR #7382 retains its own independent review/merge gate.
Validation at matching clean HEAD:
just mobile-check— PASS; formatter 554 files / 0 changed, analyzer no issues.just mobile-test— PASS; full Mobile package, 2,119 tests.- Independent changed-consumer suites — PASS; 268 tests.
- Exact-head GitHub
Clients / Mobile, DCO, Semgrep, and zizmor — PASS. - Diff hygiene and boundary checks — PASS:
git diff --check; no production Mobile/dependency diff from base; final tree clean.
Manual/native evidence: no native-device or simulator run claimed. Existing Flutter production-widget renders and structural/widget assertions reduce, but do not erase, native rendering and lifecycle risk.
Residual risk: native pixels, dark theme, Dynamic Type, VoiceOver, and a real authenticated cold-open→snapshot→reconnect journey were not independently witnessed. These are verification gaps, not author-actionable defects in this tests-only child.
2 participants
🤖
Summary
When mobile didn't know someone's presence — right after opening the app, or when the relay simply had no data — DM headers, DM-list avatars and profiles showed that person as Offline, with the same gray dot as someone genuinely offline. This PR separates "we don't know" from "we know they're offline": unknown presence now reads Unknown with no dot or notch, while confirmed Online/Away/Offline look exactly as before.
Related issue
Testing
just mobile-checkand the full mobile test suite pass independently, and every broader CI gate has successful evidence after isolated recovery runs (no monolithic localjust cirun) — receipts in the exact-head evidence comment.To see it: open a DM whose presence isn't confirmed yet (e.g. right after a cold open) — the header and profile now read Unknown instead of Offline.
Screenshots
Flutter production-widget test renders — not native-device screenshots or acceptance captures.
Confirmed Online/Away/Offline presentations are unchanged by this PR.
Capture provenance
Rendered by the Flutter widget engine in a
flutter testrun (production widgets, production theme; no device or simulator). Before: this PR's declared basea8e8028bfb9bb67de2db03ed448839e2f3ab2b0d. After: its headdd955a0c2ac80ada39b416a2f8b7481afb8773bf.