Repository navigation
feat(mobile): follow the portable mention rules, including DMs - #8102
Conversation
🔐 Codex Security Review
|
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: REQUEST CHANGES
Reviewed: 46e60206a47ed1f696d625df57a1e5dd2d019a42..57058f9ce5053799f5cc0b6f8fe11b83c1f43bdd (exact live head reconfirmed).
P1 — Mention-directory results and errors can cross communities
mentionUserSearchProvider is keyed only by query and reads the stable relay-session notifier without watching the active relay/community configuration (mobile/lib/features/channels/mentions/mention_candidates_provider.dart:28-40). _settledSearchesProvider resets on relay session status, which is not a community-generation fence (:96-99). The analogous directory providers explicitly watch relayConfigProvider because the notifier is stable and communities can share a signing key (mobile/lib/features/channels/channel_management_provider.dart:349-360,421-449).
A request for @Mary J in community A can therefore settle—or reuse cached state—after switching to community B, displaying A-only identities or A’s error in B. Later send validation reduces notification risk, but does not undo the cross-community privacy leak or misleading chooser state. This violates the community-isolation contract (VISION.md:50-56).
Author action: bind both the request and settled result/error to the active relay/community generation. Add deterministic A-request → switch-to-B-with-same-query regressions for late success and late failure; neither A rows nor A error may surface in B. Mutation-prove that removing the fence fails both behaviors.
P1 — DM mention send can fail silently when membership lookup fails
The DM path now awaits channelMembersProvider(...).future while scanning non-member mentions (mobile/lib/features/channels/compose_bar/helpers.dart:514-536). That await happens before isSending and outside the send try/catch (compose_bar_widget.dart:558-596), while the button discards the future via unawaited(send()) (:1047). A transient membership-provider error therefore escapes as an unhandled async failure: the draft is not sent, and the user receives no durable error or retry affordance. This is newly reachable on the headline DM path and violates the visible/retryable failure contract (VISION_MOBILE.md:9-13,47-51).
Author action: catch scan failures at the send boundary, preserve the draft, and show a visible retryable error. Add a production-seam widget regression with a failing membership future and successful second attempt; mutation-prove that bypassing the error handling fails behaviorally.
Verification owner: author for causal regressions and mutation receipts; :bot: Jude’s code review agent for refreshed exact-head review; mobile release QA for native/device observation.
What held
The signed-tag path itself is sound: DM outsiders are demoted to two-field mention reference tags rather than recipient p tags; fixed DM roster recipients remain addressed; channels retain invite gating; sessions and unknown types remain member-only; final signing revalidates recipients as members; identity ambiguity fails closed. Matching/order, stable rows, labels, Space selection, retry UI, and attachment/text tag propagation were also coherent in the reviewed source/tests.
Validation / confidence gaps
Both lanes rechecked clean local and live head equality. just mobile-install mobile-check, formatting, analysis, exact-head hosted Mobile/Mobile Swift/security/DCO checks passed; final poll had 16 successes, 33 skips, and no pending or failed checks. Reviewer-local full Flutter execution was blocked before tests by this host’s unaccepted Xcode license; real-device behavior and independent byte-for-byte comparison to the external buzz-app fixture were not observed. Those are confidence gaps, not additional author defects.
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent reviewed exact head 57058f9ce5053799f5cc0b6f8fe11b83c1f43bdd against base 46e60206a47ed1f696d625df57a1e5dd2d019a42.
Verdict: BLOCK
1. Major — directory results are not fenced to the active community
mentionUserSearchProvider is keyed only by the query and reads the stable relaySessionProvider.notifier without watching the relay/community configuration (mobile/lib/features/channels/mentions/mention_candidates_provider.dart:28-40). _settledSearchesProvider resets only when session status changes (:96-99), which is not a community-identity fence. The analogous directory providers explicitly watch relayConfigProvider because the notifier is stable and the same signing key can persist across communities (mobile/lib/features/channels/channel_management_provider.dart:347-360,421-449).
A search started in community A can therefore complete after a switch and surface A-only rows—or its error—in community B for the same query. This violates the community isolation contract in VISION.md:50-56; later send validation does not undo the profile leak or misleading chooser state.
Author action: bind both the search request and settled result/error to the active relay/community generation, and add deterministic A request → switch to B → A late-success and late-failure regressions. Neither A rows nor A errors may enter B. The regression should fail when the generation fence is removed.
Verification owner: author supplies the causal tests; reviewer mutation-checks the fence and reruns the full mobile gate.
2. Major — a DM send can fail silently when membership lookup fails
This change removes the DM early return from _scanNonMemberMentions, so a DM containing mentions now awaits channelMembersProvider(channelId).future (mobile/lib/features/channels/compose_bar/helpers.dart:514-536). That await occurs before isSending is set and outside the send try/catch (mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:558-597), while the button intentionally discards send() with unawaited (:1047). A transient membership-provider error therefore escapes as an unhandled async error: the draft remains, but the user sees no failure or retry state and the message is not sent. This newly affects the headline DM path and conflicts with visible send failures in VISION_MOBILE.md:9-13,47-51.
Author action: handle scan failures at the send boundary, preserve the draft, show a visible retryable failure, and add a production-seam widget regression for a failing member future plus successful recovery on the next attempt.
Verification owner: author adds the regression; reviewer mutation-checks it and reruns the full mobile suite.
What held up
The signed-tag path is otherwise coherent: DM outsiders are demoted to two-field mention reference tags rather than recipient p tags; fixed DM participants remain recipients; channel outsiders still go through the invite/send-without-inviting decision; text and attachment sends carry references into signing; malformed references and non-member recipients are filtered at the writer boundary; and unknown/session types remain member-only. The product/UI pass found no additional defect in ranking, Space selection, labels, stable rows, retry UI, accessibility, or invite/session behavior.
Validation and confidence gaps
- PASS:
git diff --check; merge-base equals46e60206a47ed1f696d625df57a1e5dd2d019a42; both reviewer trees were clean at exact head. - PASS locally at exact head:
just mobile-install mobile-check(format, analyze, and mobile gateway recipe checks). - PASS on GitHub at exact head: Clients/Mobile, Mobile, Mobile Swift, Semgrep, zizmor, DCO, and Codex security review.
- Full local
mobile-testand a real device journey were not run on the review host because its Xcode license is unaccepted and theobjective_cnative-asset hook exits throughxcrun. This is a reviewer-tooling confidence gap, not a third author defect. The PR reports 2,991 mobile tests passing and labels its screenshots as Flutter test-renderer captures rather than device captures. - Fixture-driven parity is covered in mobile, but the external buzz-app fixture was not independently byte-compared in this checkout.
Minimalism: 9/10. Elegance: 9/10. Correctness: 7/10 until the two failure paths are fixed.
The mobile mention chooser now uses the shared buzz-app mention spec and its fixtures: the same query syntax, match tiers, sort order, Space selection, stable rows, and p and mention tags. A multi-word query still runs the directory search, so @Mary J finds Mary Jane outside the channel. The portable fixtures run as Dart conformance tests. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
When the community directory search fails, the mention chooser now shows the error and a Retry button instead of closing, as the portable mention rules require. Retry runs the search again. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
A DM chooser now searches the community directory, as streams and forums do. Outside people show "not in DM". On send, the app asks first. Nobody can be added to a DM, so the prompt offers only Send anyway. Outside people are sent as mention references and are not notified. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Nobody can be added to a DM, so the send prompt offered no real choice. A DM now sends outside people straight away as references, which do not notify them. Streams and forums still ask. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
The mention directory search was keyed only by query. Two communities can share a signing key and session notifier, so a search started in community A could finish after a switch to B and show A's people, or A's search error, in B's chooser. The last settled page also survived the switch. Watch the relay config in the search and in the settled-page store so a community switch restarts both. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Sending with a mention first checks who is in the channel. If loading the members failed, the error escaped the fire-and-forget send: nothing was sent and nothing was shown. Now the draft stays and a snackbar offers Retry. Two related fixes at the same send boundary: - Read the channel actions before the first await, so an Invite answered after a community switch is refused instead of adding people in the new community. - In a DM, use current membership as the authority, as the DM send does, so stale participant metadata does not count as inside. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
57058f9 to
de1ca08
Compare
|
🤖 Review fixes are on head P1: mention search crossed communities (both review-agent reviews). Fixed in
P1: silent send failure when the member check fails. Fixed in
Codex security review, MEDIUM: Invite after a community switch. Fixed in
Codex security review, MEDIUM: stale DM participant metadata. Fixed in
Gate: |
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: APPROVE
Reviewed: 5173fad6602ae82c06f338d19ef0f18eff57159b..de1ca0804f498195cc176809e9cc862c86dbf72a (exact live head de1ca0804f498195cc176809e9cc862c86dbf72a)
Risk: high — mobile mention search, membership authority, community switching, signed reference tags, and visible send failure/retry cross identity, tenant-isolation, async lifecycle, and relay-publication boundaries.
Behavior/contracts traced: community-scoped directory request and settled-state generations; composer scan failure through retained draft, snackbar, Retry, and eventual send; stale channel-actions capture through the real kind 9000 submit guard; DM current-membership authority, metadata fallback, recipient resolution, and non-notifying two-field mention references.
Findings: no unresolved blocking or non-blocking defect. The remediation answers the prior two blockers and both additional security findings:
mentionUserSearchProviderand_settledSearchesProvidernow watchrelayConfigProvider, fencing both late request completion and retained rows/errors (mobile/lib/features/channels/mentions/mention_candidates_provider.dart:29-35,101-105). Regressions cover A→B late success, late failure, and already-settled A data (mobile/test/features/channels/mentions/mention_search_community_test.dart:140-186).- The production send boundary catches membership-scan failure before clearing the draft, displays a specific retryable failure, and retries the same send closure (
mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:563-585). The widget regression proves zero first send, exact draft retention, Retry, and one recovered send (mobile/test/features/channels/compose_bar_test.dart:4627-4669). channelActionsProvideris captured before the first await; its original community identity is revalidated immediately beforeSignedEventRelay.submit(kind: 9000)(compose_bar_widget.dart:559-565;channel_management_actions.dart:80-116,383-400). The regression switches communities with the prompt open and proves zero actual membership publications (compose_bar_test.dart:4671-4727).- Shared
dmParticipantPubkeystreats non-empty current membership as authoritative and falls back to metadata only when membership is absent/empty (channel_management_provider.dart:46-54). Both mention classification and final DM recipients use it (compose_bar/helpers.dart:532-552;send_message_provider.dart:137-161), with stale-participant and fallback regressions (compose_bar_test.dart:4561-4625).
Author action: none.
Verification owner: the still-running hosted Codex Security Review remains owned by that CI gate; mobile release QA owns optional native-device observation. Neither is author rework absent a concrete failure.
Validation at matching exact head:
- PASS — exact live head/base refresh; clean reviewer trees;
git diff --check. - PASS —
just mobile-check(format and analyze) at the clean exact head. - PASS — source-level causal mutation review for all four regressions: removing either community watch, the scan catch, moving the actions read after the prompt, or restoring metadata/current-member union defeats the corresponding behavioral assertion.
- PASS — exact-head hosted Clients/Mobile, Mobile Swift Domain, DCO, Semgrep, and zizmor gates as of submission.
- PENDING external gate — Codex Security Review; no failure reported at submission.
Manual/native evidence: no independent device run. The production widget/provider/relay boundaries and their deterministic tests were reviewed; screenshots remain Flutter test-renderer captures rather than device captures.
Residual risk: focused/full Flutter execution could not start on the reviewer host because the unchanged objective_c native-asset hook calls xcrun and the host has not accepted the Xcode license. This is reviewer-tooling debt, not a code defect. Native observation of failure/retry and community switching remains outstanding. Any new head invalidates this approval.
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent re-reviewed exact head de1ca0804f498195cc176809e9cc862c86dbf72a against base 5173fad6602ae82c06f338d19ef0f18eff57159b and the prior blockers at 57058f9c….
Verdict: APPROVE
No author-actionable defect remains. The remediation closes all four affected seams:
- Community generation:
mentionUserSearchProviderand_settledSearchesProviderboth watchrelayConfigProvider, fencing late successes, late failures, and retained rows from the previous community (mobile/lib/features/channels/mentions/mention_candidates_provider.dart:29-35,101-105,196-208). Production-provider regressions exercise all three A→B cases with a positive B control (mobile/test/features/channels/mentions/mention_search_community_test.dart:140-186); removing the corresponding watch breaks the causal assertion. - Visible send retry: membership-scan failure is caught before draft mutation and shown with Retry; the same send closure is retried (
mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:563-585). The widget regression proves no first send, exact retained draft, visible failure/action, and one successful second send (mobile/test/features/channels/compose_bar_test.dart:4627-4669). - Invite publication fence: channel actions are captured before the await/prompt, retain the original relay URL and key, and revalidate immediately before real kind
9000submission (mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:559-565;mobile/lib/features/channels/channel_management_actions.dart:80-116,383-400). The regression switches communities while the prompt is open and proves zero kind9000publications and zero message sends (mobile/test/features/channels/compose_bar_test.dart:4671-4727); moving the provider read after the prompt makes the test publish and fail. - DM membership authority:
dmParticipantPubkeysprefers non-empty current membership and falls back to metadata only when current membership is unavailable/empty (mobile/lib/features/channels/channel_management_provider.dart:46-54). Both scan classification and final recipient resolution use it (mobile/lib/features/channels/compose_bar/helpers.dart:532-552;mobile/lib/features/channels/send_message_provider.dart:137-161). Tests prove stale metadata becomes a two-field non-notifyingmentionreference while unavailable membership preserves the fallback send (mobile/test/features/channels/compose_bar_test.dart:4561-4625). Restoring the old union breaks the stale-participant assertion.
The wider signed-tag contract still holds: current DM members remain recipients, outsiders remain reference-only, channel invitation stays gated, and final signing filters recipient mentions while preserving validated reference tags.
Validation
- PASS: live PR head and clean reviewer trees pinned to
de1ca0804…; merge-base/base5173fad6602ae82c06f338d19ef0f18eff57159b;git diff --checkclean. - PASS locally at exact head:
just mobile-check(format and analyze). - PASS hosted at exact head: required Mobile, DCO, and Desktop Release Candidate checks; Clients/Mobile, Mobile Swift, Semgrep, and zizmor are also green.
- Reviewer-local focused/full Flutter execution remains blocked before test execution by this host's Xcode/native-asset toolchain failure. Native-device observation was not independently performed. These are confidence gaps owned by mobile release QA, not author rework; the production-seam tests are green in hosted Mobile CI.
- The non-required Codex security-review job is still in progress after its first attempt was cancelled and automatically restarted. This is not a PR-caused required-gate failure and does not change the approval; any concrete later finding must be evaluated normally.
Author action: none. Minimalism, elegance, and correctness: 9/10.
…#8102) 🤖 ## Summary The mobile app's @mention chooser now follows the same rules as the desktop app (buzz-app). The same typed text finds the same people in the same order on both apps, and sending works the same way. - **Same matching and order.** Mobile uses the shared mention rules and runs their shared test cases. A multi-word search such as `@Mary J` still searches the community directory, so it can find "Mary Jane" even when she is not in the channel. - **DMs can mention people outside the DM.** The chooser marks them "not in DM". Nobody can be added to a DM, so the message sends without a prompt. Outside people go out as references, so they are not notified. This matches buzz-app block#510. - **Channels still ask.** In a stream or forum, mentioning someone outside the channel asks "Invite" or "Do nothing" before the message sends. A channel with no type tag counts as a stream. - **Sessions do not search the directory.** Only session members appear. - **A failed directory search stays visible.** The chooser shows the error and a Retry button. Before, it closed. - **Search results stay in their community.** If you switch communities while a search runs, its people and its error do not show in the new community. The search starts again there. - **A failed send check keeps the draft.** Before a message with a mention sends, the app checks who is in the channel. If that check fails, the message stays in the compose bar and a notice offers Retry. Before, nothing was sent and nothing was shown. - **Invite stays in its community.** If you switch communities while the "Invite" question is open, Invite adds nobody. ### Screenshots The real compose bar widget, rendered by Flutter's test renderer with the app's fonts and theme. The data is sample data. These are not device captures. | DM: outside person is "not in DM" | Stream: outside person is "not in channel" | |---|---| | <img width="320" alt="DM chooser" src="https://github.com/user-attachments/assets/0e884518-2386-4cd1-9f19-16661aaa194e" /> | <img width="320" alt="Stream chooser" src="https://github.com/user-attachments/assets/5395e4e1-032e-459b-98c6-0126c060c4d9" /> | | Stream: send asks first | Session: members only | Search failed: Retry | |---|---|---| | <img width="260" alt="Stream ask on send" src="https://github.com/user-attachments/assets/2b988257-3c6a-445d-acfa-fca47b9752fe" /> | <img width="260" alt="Session chooser" src="https://github.com/user-attachments/assets/e193551f-f5bf-41d9-aa29-bc8665c643f9" /> | <img width="260" alt="Search failed with Retry" src="https://github.com/user-attachments/assets/e8c4b9bb-067e-4fe9-9fdd-4e5691bc66be" /> | ### Details - `mobile/lib/shared/mentions/mention_rules.dart` holds the rules: query syntax, match tiers, sort order, Space selection of an exact unique name, stable rows while results arrive, and `p` and `mention` tags. A `p` tag notifies the person. A `mention` tag is a reference and does not notify. - `mobile/test/shared/mentions/mention-rules.fixtures.json` is an exact copy of the buzz-app fixture file. `mention_rules_test.dart` runs every case. - Known difference: desktop gives the directory to a channel with an unknown explicit type (not stream, forum, DM or session). Mobile does not. No such type exists today. ### Related issue None found. Desktop counterpart: block/buzz-app#510. block#7513 also edits `send_message_provider.dart` and may conflict. ### Testing `just mobile-install mobile-check mobile-test` passes: analyze clean, 2998 tests pass on `49c44653` (before rebase). The pre-push hook ran `just mobile-check && just mobile-test` again on the rebased head `de1ca080` and passed. Each review fix was mutation-checked: removing the fix makes its new test fail. --------- Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
* commit '6d07a291376477942b70ba44489a47c9f6bfe006': (63 commits) perf(relay): bound fleet usage metrics collection (#7176) fix(ios): let channel titles use available navigation space (#8137) buzz-db: purge retention-free deletions in the app and index mentions in the event transaction (#8128) feat(relay): time post-metrics-bind startup steps (#8132) release: push gateway chart 0.3.5 (#8141) feat(push): support Deployment metadata annotations (#8136) fix(buzz-pair): install rustls ring CryptoProvider before WSS (#7998) fix(desktop): address workflow deletions at the workflow's actual owner (#2669) fix(buzz-acp): bound sibling-gate lookup failures instead of caching them (#7216) feat(relay): private read-state accessory API (#7906) fix(acp): keep agent-to-agent thread replies in the thread (#8124) release: push gateway chart 0.3.4 (#8126) feat(push): support platform-managed gateway runtime pods (#8113) chore(release): release Buzz Desktop version 0.5.27 (#8125) fix(desktop): show OpenAI-compatible endpoint settings in the baked-env UI (#8117) feat(buzz-agent): send User-Agent on LLM requests (#8116) docs(changelog): remove hand-written Unreleased section (#8123) feat(desktop): offer Claude effort levels in agent edit and create (#8059) BUZZ-175: Route serving event writes through the tenant lock (#7828) feat(mobile): follow the portable mention rules, including DMs (#8102) ... Signed-off-by: Luke Tornquist <tornquist@squareup.com>
🤖
Summary
The mobile app's @mention chooser now follows the same rules as the desktop app (buzz-app). The same typed text finds the same people in the same order on both apps, and sending works the same way.
@Mary Jstill searches the community directory, so it can find "Mary Jane" even when she is not in the channel.Screenshots
The real compose bar widget, rendered by Flutter's test renderer with the app's fonts and theme. The data is sample data. These are not device captures.
Details
mobile/lib/shared/mentions/mention_rules.dartholds the rules: query syntax, match tiers, sort order, Space selection of an exact unique name, stable rows while results arrive, andpandmentiontags. Aptag notifies the person. Amentiontag is a reference and does not notify.mobile/test/shared/mentions/mention-rules.fixtures.jsonis an exact copy of the buzz-app fixture file.mention_rules_test.dartruns every case.Related issue
None found. Desktop counterpart: block/buzz-app#510. #7513 also edits
send_message_provider.dartand may conflict.Testing
just mobile-install mobile-check mobile-testpasses: analyze clean, 2998 tests pass on49c44653(before rebase). The pre-push hook ranjust mobile-check && just mobile-testagain on the rebased headde1ca080and passed.Each review fix was mutation-checked: removing the fix makes its new test fail.