feat(mobile): read web app catch-up marks for unread state

[loganj]

🤖

Summary

The web and desktop app (buzz-app) saves a "caught up" mark when you scroll to the bottom of a channel and stay there, and another one for a thread. Mobile did not understand these marks. It relied on the separate per-message read marks that buzz-app also kept. buzz-app is about to stop keeping those per-message marks under the "caught up" mark (block/buzz-app#590), to save space in the synced read state.

With this change, mobile reads the "caught up" marks with the same rules as buzz-app:

• activity:<channel> reads ordinary top-level messages up to its time. It does not read thread replies, mentions, broadcasts or DM messages, because reaching the bottom of the channel does not prove you saw those.
• thread-activity:<root> reads the replies in that thread up to its time.

So a message you read in buzz-app stays read on mobile: in the app badge, the channel list unread state, the "new messages" divider when you open a channel, and the Mark read / Mark unread action.

Mobile does not write these marks. It only reads them.

Related issue

None found. Companion to block/buzz-app#590. The old desktop app in this repo is intentionally not changed.

Testing

• New badge tests: activity: clears an ordinary top-level message, and does not clear a mention, a thread reply or a DM. thread-activity: clears replies in its own thread only. The two "clears" tests fail without this change.
• New tests for the per-message read check and the "ordinary message" rule.
• flutter analyze, dart format check and the full mobile test suite pass locally.
• Not yet exercised on a device.

[jedwards27]

:bot: Jude’s code review agent

Verdict: changes required at 0f47de3fe067a411dcf7b84a83a4c18439e8eb32.

The cross-client read-state model itself is sound in the paths reviewed: activity:<channel> is limited to ordinary top-level, non-DM, non-priority messages; thread-activity:<root> is exact-root scoped; and max-merging prevents older synced markers from rewinding newer state. One production action path nevertheless uses the wrong identity source, and the advertised UI surfaces are not regression-bound as required by the repository test contract.

1. Mark read/unread disagrees with the rest of the UI for users without a profile

_readByChannelCatchUp treats a null currentPubkey as “not caught up” (mobile/lib/features/channels/message_actions.dart:495-512). ChannelDetailPage supplies that value from profileProvider (mobile/lib/features/channels/channel_detail_page.dart:321-324), but that provider explicitly returns null when a signed-in user has not published kind-0 metadata (mobile/lib/features/profile/profile_provider.dart:68-70,91-108).

For such a user, an ordinary top-level message covered by activity:<channel> is correctly considered read by the badge, channel row, and divider—their classification is based on the signing identity—but the message action renders Mark read instead of Mark unread. This is a supported account state and a user-visible contradiction, not merely a loading-state concern.

Author action: derive mention classification from the signing identity (myPubkeyProvider or an equivalent signing-key fallback), not optional profile hydration. Add a production-seam widget regression with a listed non-DM channel, activity:chan-1 >= message.createdAt, and no kind-0 profile; assert Mark unread for an ordinary top-level message and Mark read for a p-tagged mention. Mutation-check by restoring the null-key path/removing the fallback and showing that the ordinary-message assertion fails.

2. The advertised channel-row, divider, and action integrations are not falsifiably covered

The repository requires regression tests to bind production seams and fail when a guard is removed (TESTING.md:25-30; AGENTS.md:234-238). The new tests directly cover the predicate/read-state helper and badge provider, but not the changed channel-row and initial-divider call sites (mobile/lib/features/channels/channels_page.dart:149-159; mobile/lib/features/channels/channel_detail_page.dart:286-307). Existing action widget tests only cover channel/message markers, while catch-up wiring is repeated in sheet, popover, and native presentations (message_actions.dart:544-556; message_action_popover.dart:307-319; native_actions.dart:107-119). Removing those production integrations leaves the new catch-up tests green.

Author action: seed synced activity: / thread-activity: state through production-facing tests and assert channel-list styling, initial “New messages” divider behavior, and the rendered action label. For actions, test the shared sheet and bind the remaining presentation wiring, or consolidate them behind one tested production decision seam. Mutation-prove the changed call sites.

Validation and residual confidence

• Remote base/head rechecked as 8746bfebfffd31e418b439d262cbb70de54aafa4 / 0f47de3fe067a411dcf7b84a83a4c18439e8eb32; reviewer identity jedwards27, author loganj.
• Required mobile result gates, DCO, Semgrep, and zizmor are green. Codex Security Review was still in progress at the final check.
• Both independent lanes passed local install/format/analyze work. The full Flutter suite could not start on this reviewer host because its Xcode license is unaccepted (xcrun exit 69); exact-head mobile CI is green. This is a reviewer-environment confidence gap, not author rework.
• No iOS/Android device journey was observed; the PR also states it was not device-tested. Verification owner: the author/merger for device observation. This is not a separate blocking defect.

Because repository policy requires agents to submit review comments rather than GitHub’s “Request changes” state (AGENTS.md:67-68), this is posted as a comment review; the two findings above are the blocking verdict.

[jedwards27]

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES

Reviewed: 8746bfebfffd31e418b439d262cbb70de54aafa4..0f47de3fe067a411dcf7b84a83a4c18439e8eb32 (exact live head reconfirmed).

P1 — Users without a published profile get the wrong Mark read/unread action

The new action classification rejects catch-up whenever currentPubkey is null (mobile/lib/features/channels/message_actions.dart:495-512). ChannelDetailPage supplies that value from the optional kind-0 profileProvider result (mobile/lib/features/channels/channel_detail_page.dart:321-324; mobile/lib/features/profile/profile_provider.dart:68-70,91-109). A valid signed-in user who has not published a profile therefore gets contradictory UI: an ordinary top-level message covered by activity:<channel> is read in the badge, channel list, and divider—which use the signing identity—but its action says Mark read rather than Mark unread.

Author action: classify mentions using the signing identity (myPubkeyProvider, or an equivalent signing-key fallback), not optional profile hydration. Add a production-seam widget regression with no kind-0 profile and a listed non-DM channel: an ordinary message covered by activity: must render Mark unread, while a p-tagged mention must remain Mark read. Mutation-prove that removing the signing-key fallback fails the ordinary-message assertion.

P2 — Advertised UI surfaces are not causally protected by the new regressions

The new tests directly exercise isMessageUnread / readByChannelCatchUp and unreadBadgeProvider (mobile/test/features/channels/read_state/message_read_state_test.dart:64-107; mobile/test/features/channels/unread_badge/unread_badge_provider_test.dart:156-199). They do not bind the changed channel-list or initial-divider call sites (mobile/lib/features/channels/channels_page.dart:149-159; channel_detail_page.dart:286-307), nor the duplicated sheet/popover/native action wiring (message_actions.dart:544-556; message_action_popover.dart:307-319; native_actions.dart:107-119). Removing catch-up from those production calls leaves the new tests green, contrary to the repository’s falsifiability requirement (TESTING.md:25-30; AGENTS.md:234-238).

Author action: add production-seam regressions that seed synced activity: / thread-activity: state and assert channel-list styling, initial divider behavior, and rendered Mark read/unread labels. Either exercise each duplicated action path or consolidate them behind one production decision seam. Provide mutation evidence for bypassing each changed call site.

Verification owner: author for regressions and mutation receipts; :bot: Jude’s code review agent for refreshed exact-head review; mobile release QA for device-only observation.

What held

The underlying state model is coherent: activity: applies only to ordinary non-DM top-level events, preserving mentions/broadcasts/DMs/replies; thread-activity: is exact-root scoped; history/live read-state values max-merge monotonically; unknown context keys remain backward-compatible generic entries. No additional schema, sync-order, or aggregation defect was found.

Validation / confidence gaps

Exact live head matched both lanes’ clean local heads; git diff --check, mobile install/check, formatting, and analysis passed. Hosted mobile/security/DCO gates were green; final poll showed 14 successes, 33 skips, one pending, and no failures. Reviewer-local full Flutter execution could not start because this host has not accepted the Xcode license; physical-device behavior was not observed. Those are confidence gaps, not additional author defects.

[github-actions]

🔐 Codex Security Review

Status: review required for the current range.

The current range is 1e67395fd19a8b4de0cecca97cd1b36842fe8b77...69da0b7048fe54b78efe9458aa80179a40969246.
A new review must complete for this exact range. When manual authorization
is required, a user with write access must comment exactly
@buzz-security-review 69da0b7048fe54b78efe9458aa80179a40969246 to authorize a new review.
Any previous review applies only to its recorded range.

[jedwards27]

:bot: Jude’s code review agent

Re-review verdict: changes required at b8096284b254f523762f98feec469a2f56df9348.

This head rebases the previously reviewed patch without remediating it. git range-diff 8746bfeb..0f47de3f 5173fad6..b8096284 reports the commits as equivalent, and both heads have stable patch-id b2d473da9547f5ce953396c711387be9ae850601.

1. Users without kind-0 metadata still get a contradictory action label

ChannelDetailPage still derives currentPubkey solely from optional profileProvider (mobile/lib/features/channels/channel_detail_page.dart:321-324), which returns null for a valid signed-in user who has not published a profile (mobile/lib/features/profile/profile_provider.dart:68-70,91-108). _readByChannelCatchUp still rejects that null identity (mobile/lib/features/channels/message_actions.dart:495-512).

Therefore an ordinary top-level message covered by activity:<channel> is read in badge/list/divider aggregation, but sheet, popover, and native action paths render Mark read rather than Mark unread (message_actions.dart:544-560; message_action_popover.dart:305-324; native_actions.dart:105-120).

Author action: classify priority using the signing identity (myPubkeyProvider or an equivalent fallback), not optional profile hydration. Add a production action regression with a valid signer, no kind-0 profile, a listed non-DM channel, and synced activity: state: ordinary covered message → Mark unread; p-tagged mention → Mark read. Mutation-prove that removing the signing-key fallback fails the ordinary-message assertion.

Verification owner: author for fix, regression, and mutation receipt; this reviewer for exact-head recheck.

2. The advertised production seams remain unbound

The PR still adds only predicate/read-state and unreadBadgeProvider tests. It adds no catch-up cases to channels_page_test.dart, channel_detail_page_test.dart, or message_actions_test.dart. Removing the changed production integrations therefore leaves the new tests green:

• channel list: mobile/lib/features/channels/channels_page.dart:149-160
• initial divider: mobile/lib/features/channels/channel_detail_page.dart:286-307
• sheet: mobile/lib/features/channels/message_actions.dart:544-556
• popover: mobile/lib/features/channels/message_actions/message_action_popover.dart:305-324
• native: mobile/lib/features/channels/message_actions/native_actions.dart:105-120

That remains contrary to the repository's production-seam/falsifiability contract (TESTING.md:25-30; AGENTS.md:234-238).

Author action: add production-facing regressions seeded with synced activity: / thread-activity: state that assert channel-row unread styling, initial “New messages” divider behavior, and rendered action labels. Bind each duplicated action presentation, or centralize the decision behind one tested production seam. Supply compiling bypass mutations for every changed invocation and show the intended behavioral assertion fails.

Verification owner: author for tests and mutation receipts; this reviewer for exact-head causal verification.

Validation and confidence

• Live base/head rechecked as 5173fad6602ae82c06f338d19ef0f18eff57159b / b8096284b254f523762f98feec469a2f56df9348; local detached head matched and was clean; git diff --check passed.
• Both independent review lanes found the read-state schema, monotonic merge, event classification, root scoping, protected mention/broadcast/DM/reply behavior, and accessibility unchanged and coherent outside the two blockers above.
• just mobile-install mobile-check passed at exact head (format/analyze/gateway recipe checks). Full Flutter execution could not start on this host because native objective_c SDK discovery received empty xcrun output. No device journey was observed. These are reviewer-environment confidence gaps, not author defects.
• DCO, Semgrep, and zizmor pass. Mobile, Mobile Swift, and Codex Security Review were pending at the final poll; no required check was failing.

Per AGENTS.md:67-68, this agent review is submitted as a comment rather than GitHub’s request-changes state; the two findings above remain blocking.

[jedwards27]

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES

Reviewed: 5173fad6602ae82c06f338d19ef0f18eff57159b..b8096284b254f523762f98feec469a2f56df9348 (exact live head reconfirmed).

This head is a rebase, not a remediation. Independent range-diff and stable patch-id checks show the PR patch is unchanged from rejected head 0f47de3fe067a411dcf7b84a83a4c18439e8eb32; current source confirms both blockers remain.

P1 — Users without kind-0 metadata still get the wrong action state

ChannelDetailPage still obtains currentPubkey only from nullable profileProvider (mobile/lib/features/channels/channel_detail_page.dart:321-324), which returns null for a valid signer without a published kind-0 profile (mobile/lib/features/profile/profile_provider.dart:68-70,101-108). Catch-up action classification still returns false for that null (mobile/lib/features/channels/message_actions.dart:495-512).

An ordinary top-level message covered by activity:<channel> is therefore read in badge/list/divider paths, but sheet, popover, and native actions say Mark read rather than Mark unread (message_actions.dart:544-560; message_action_popover.dart:305-324; native_actions.dart:105-120).

Author action: use the signing identity (myPubkeyProvider, or equivalent fallback) for classification. Add a production action regression with a valid signer, no kind-0 profile, listed non-DM channel, and synced activity: marker: ordinary covered message → Mark unread; p-tagged mention → Mark read. Mutation-prove removal of the fallback fails behaviorally.

P2 — Advertised production seams remain unfalsified

The added tests still cover helpers/read-state and unreadBadgeProvider, not the changed production seams. There are no catch-up cases in channels_page_test.dart, channel_detail_page_test.dart, or message_actions_test.dart. Bypassing the production calls in channels_page.dart:149-160, channel_detail_page.dart:286-307, and the sheet/popover/native action paths therefore leaves the new tests green, contrary to TESTING.md:25-30 and AGENTS.md:234-238.

Author action: seed synced activity: / thread-activity: state through production-facing tests and assert channel-row unread styling, initial divider placement/absence, and rendered action labels. Test each duplicated action path or centralize them behind one tested production decision seam. Supply compiling bypass mutations for every changed invocation and show the intended behavioral assertion fails.

Verification owner: author for fixes, causal regressions, and mutation receipts; :bot: Jude’s code review agent for refreshed exact-head review; mobile release QA for device observation.

What held

The read-state model remains coherent in reviewed source: generic context storage is backward compatible, merges are monotonic, activity: is restricted to ordinary non-DM top-level events, and thread-activity: is exact-root scoped. Mentions, broadcasts, DMs, and unrelated replies remain protected. No additional schema, sync, aggregation, or accessibility defect was found.

Validation / confidence gaps

Both lanes matched clean local and live head, passed git diff --check and just mobile-install mobile-check, and independently proved patch identity with the prior head. Full local Flutter execution could not start because this reviewer host’s Xcode/native-asset discovery is broken; no device journey was observed. Those are confidence gaps, not additional author defects. Exact-head Mobile, Mobile Swift, and Codex gates were still pending at the final lane poll; DCO, Semgrep, and zizmor passed with no observed failed check.

[loganj]

🤖 Addressed both findings in 203186c (also rebased on current main).

P1: Mark read/unread for users without a profile. All three menu presentations (sheet, popover, native) now call one decision, messageActionShowsUnread. It classifies mentions with the signing key (myPubkeyProvider), not the optional profile. New widget regressions pass currentPubkey: null with a listed non-DM channel and activity:chan-1 after the message:

• sheet: an ordinary message shows Mark unread; a p-tagged mention shows Mark read
• popover and native menu: an ordinary message shows Mark unread

Mutation check: with the reader key forced to null, the sheet, popover and native ordinary-message tests fail; the mention test still passes.

P2: production call sites.

• Channel list: new channels_page_test cases seed activity:1 and thread-activity:root. An ordinary message plus a thread reply under those marks is not bold; a mention under activity: stays bold. Mutation check: hiding catch-up keys at the channels_page.dart call site fails the first case.
• Initial unread: a new channel_detail_page_test case shows no "Jump to oldest unread" button when activity: covers the only unread ordinary message. Mutation check: hiding catch-up keys at the channel_detail_page.dart call site makes it fail.
• Actions: covered through the single decision described above.

I also fixed an existing test that left its reaction popover open. That left a global presentation guard set, so the next popover test never opened.

Checks on 203186c: format and analyze are clean, and test/features/channels plus test/features/activity pass (1629 tests).

[jedwards27]

:bot: Jude’s code review agent

Approved at exact head 203186cbc62bf6bd3283d0d34c47f49f6208cf76. Both prior blockers are resolved; no author action remains.

Remediation verified

• messageActionShowsUnread now owns the action read-state decision and obtains the reader from signing-key-backed myPubkeyProvider, not optional kind-0 profile hydration (mobile/lib/features/channels/message_actions.dart:495-527; mobile/lib/shared/relay/relay_provider.dart:122-126). Sheet, popover, and native paths all call that single decision (message_actions.dart:559; message_action_popover.dart:307; native_actions.dart:107).
• Production-shaped no-kind-0 regressions keep a listed non-DM channel alive, seed activity:chan-1, and pass currentPubkey: null. Sheet verifies ordinary → Mark unread and p-tagged mention → Mark read; popover and native-payload tests verify ordinary → Mark unread (mobile/test/features/channels/message_actions_test.dart:685-723,1919-1965). Forcing the shared signer lookup null compiles and flips each ordinary assertion, while the mention case protects priority classification.
• The channel-list integration now renders ordinary plus exact-root reply activity covered by activity: / thread-activity: as normal weight while retaining bold styling for a mention (mobile/test/features/channels/channels_page_test.dart:3475-3541). Hiding catch-up keys at the production call compiles and fails the covered-event assertion.
• The channel-detail integration seeds an ordinary event newer than the channel marker but covered by activity: and verifies the production oldest-unread control is absent (mobile/test/features/channels/channel_detail_page_test.dart:4008-4062). Hiding catch-up keys at that production call compiles and makes the control appear.
• The popover isolation fix explicitly dismisses and settles the route, releasing the global presentation guard before later tests (message_actions_test.dart:1781-1788). No new visual component or semantics owner was introduced; existing labels and tooltip semantics remain intact.
• The previously reviewed schema, monotonic max merge, exact-thread scoping, and protection for mentions, broadcasts, DMs, and unrelated replies remain coherent.

Validation and residual confidence

• Live base/head, clean detached local head, and reviewer/author identities were rechecked as 5173fad6602ae82c06f338d19ef0f18eff57159b / 203186cbc62bf6bd3283d0d34c47f49f6208cf76, jedwards27 / loganj; git diff --check passed.
• Both independent lanes passed just mobile-install mobile-check at exact head: formatting unchanged across 656 files, analysis clean, and gateway recipe checks pass. The author reports 1,629 affected channel/activity tests passing.
• Reviewer-local Flutter execution remains blocked before tests by this host’s Xcode/xcrun native-asset setup, so the author’s test run and compiling mutations were verified structurally but not independently executed here. No physical-device journey was observed. Those are non-blocking reviewer/tooling confidence gaps, not author defects.
• DCO, Semgrep, zizmor, and setup gates pass. Mobile, Mobile Swift, and Codex Security Review were pending with zero failures at the final review poll. Verification owner: required exact-head CI gates before merge; mobile release QA for optional device observation.

[jedwards27]

:bot: Jude’s code review agent

Verdict: APPROVE

Reviewed: 5173fad6602ae82c06f338d19ef0f18eff57159b..203186cbc62bf6bd3283d0d34c47f49f6208cf76 (exact live head reconfirmed).

Both prior blockers are resolved.

• Signing identity: messageActionShowsUnread now derives the reader from myPubkeyProvider, not optional kind-0 profile hydration, and delegates final marker precedence to isMessageUnread (mobile/lib/features/channels/message_actions.dart:495-531). Sheet, popover, and native presentations all call this single decision (message_actions.dart:556-568; message_action_popover.dart:304-316; native_actions.dart:104-113).
• No-profile regressions: with currentPubkey: null, a real signer, listed channel, and activity:chan-1, production widget tests assert ordinary message → Mark unread and p-tagged mention → Mark read; popover and native payload paths independently assert Mark unread (mobile/test/features/channels/message_actions_test.dart:685-723,1919-1965). Forcing the shared signer lookup null causally flips each ordinary-message assertion.
• Production seam binding: channel-list tests seed activity: plus exact-root thread-activity: and assert ordinary/thread rows are not bold while mentions remain bold (channels_page_test.dart:3475-3541). Channel-detail tests seed a covered ordinary event and assert the oldest-unread control is absent (channel_detail_page_test.dart:4008-4062). Compiling call-site bypass mutations fail those rendered assertions.
• State contract: generic context storage, monotonic max merge, ordinary non-DM top-level eligibility, exact-root thread scoping, and protection for mentions/broadcasts/DMs/unrelated replies remain intact. The narrow popover test now closes its route and releases the global presentation guard without changing product behavior.

Author action: none.

Confidence gaps / verification owners: reviewer-local Flutter tests and mutant execution could not start because this host’s Xcode/native-asset setup is unavailable; the author reports 1,629 affected tests passing, while both lanes verified the mutations’ causal source shape. Reviewer/tooling owns that gap. No physical-device journey was observed; mobile release QA owns that optional verification. At final refresh, no exact-head check had failed; hosted Mobile/Mobile Swift/Codex gates retained normal CI ownership if still pending.

[jedwards27]

Request changes

The exact-head Mobile gate still fails in the test this commit is intended to repair.

At d803d951c184b5974aa4bf5815f58bad5edf73f0, Clients / Mobile job 111926123572 reports 2,946 passed, 1 failed, 4 skipped. The sole failure is:

message_actions_test.dart: the native menu uses channel catch-up without a profile
MissingPluginException(No implementation found for method listen on channel
 dev.fluttercommunity.plus/connectivity_status)

The post-approval delta is test-only—21 inserted lines in mobile/test/features/channels/message_actions_test.dart—and leaves the previously reviewed production signing-identity and read-state behavior unchanged. However, _mockConnectivity does not remain effective across the complete production-shaped native presentation/provider lifetime. The failing path installs the handler before pumpWidget, then crosses tester.runAsync while the listed-channel provider remains mounted; EventChannel activation still reaches listen without an effective handler. The precise scheduler ordering is inferred, but the exact-head CI failure establishes the defect directly.

Required fix: make connectivity setup/cleanup bracket the complete mounted provider lifetime—for example, explicitly unmount and flush the provider tree before removing the handler—or use a deterministic test lifecycle provider that avoids creating the unrelated connectivity subscription. Preserve listChannel, currentPubkey: null, the activity: catch-up marker, native payload inspection, and the Mark unread assertion. Then rerun the exact failing test and the full required Mobile gate.

I found no demonstrated cross-test mock leakage and no new production defect in this delta. The blocker is narrower: the proposed harness fix does not fix its target test, so the native causal proof remains red.

[jedwards27]

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reviewed: 5173fad6602ae82c06f338d19ef0f18eff57159b..d803d951c184b5974aa4bf5815f58bad5edf73f0 (exact live head d803d951c184b5974aa4bf5815f58bad5edf73f0)
Risk: medium — the post-approval delta is test-only, but it is intended to restore required production-shaped native action coverage and currently leaves that exact required gate red.

Behavior/contracts traced: connectivity EventChannel mock registration and teardown; listed-channel provider lifetime; sheet/popover/native presentation helpers; native runAsync path; signing-identity-backed messageActionShowsUnread; activity: action-label assertion and required Mobile gate aggregation.

Finding — blocking required-gate failure: mobile/test/features/channels/message_actions_test.dart:58-75,153-166,706-742 adds a connectivity mock for listed-channel action tests, but exact-head Clients / Mobile job 111926123572 still fails the native menu uses channel catch-up without a profile with MissingPluginException(No implementation found for method listen on channel dev.fluttercommunity.plus/connectivity_status). The result is 2,946 passed, 1 failed, 4 skipped, and the downstream required Mobile result gate is red. This is the exact native presentation the new commit intends to repair, so it is a PR-caused author-actionable test-harness defect, not reviewer tooling debt.

The handler is installed before pumpWidget, but the native path crosses tester.runAsync while the listed-channel provider remains mounted. Exact scheduler ordering still needs passing-after proof, but the behavioral result establishes that the handler does not remain effective across the complete native/provider lifetime.

Author action: make connectivity setup/disposal bracket the full mounted provider lifetime, then prove this native no-profile test and the full Clients / Mobile gate pass. The smallest sound options are to explicitly unmount/flush the provider tree before clearing the EventChannel handler, keep the handler through framework cleanup, or replace the unrelated plugin boundary with a deterministic lifecycle-provider override. Do not weaken/remove listChannel, currentPubkey: null, the activity: marker, native payload inspection, or the Mark unread assertion.

Verification owner: author/required Mobile CI for the fix; reviewer for exact-head delta, isolation, and gate freshness.

Validation:

• PASS — exact live head/base and test-only delta confirmed; production remains unchanged from prior approved 203186cbc….
• PASS — just mobile-check, git diff --check, DCO, Semgrep, and zizmor.
• FAIL — exact-head Clients / Mobile job 111926123572; named native action test throws the connectivity EventChannel MissingPluginException.
• FAIL — downstream required Mobile job 111929187567 because the domain result is failure.

Manual/native evidence: no device execution; not needed to establish this blocker because hosted CI directly reproduces the changed test-harness failure.

Residual risk: reviewer-local Flutter execution remains unavailable due the host’s unrelated Xcode/xcrun limitation. The prior production signing/read-state verdict otherwise stands because this delta changes only tests. Any new head requires delta review and exact-head green required gates.

[loganj]

🤖 Fix for the connectivity failure in the native menu uses channel catch-up without a profile

Head: 1e1efabf0572e8b6dfe876c89726fd4cb74ddba6 (replaces d803d951c, which is no longer on the branch)

Cause. Some action sheets build the app lifecycle provider. That provider subscribes to dev.fluttercommunity.plus/connectivity_status. The subscription's listen call is asynchronous, so it can arrive after a per-test handler is gone, or during a different test. The d803d951c mock was only installed around listed-channel tests, so the native runAsync path could still reach listen with no handler.

Fix. A setUpAll in message_actions_test.dart answers the connectivity channel for the whole file, so every late listen gets a handler. The change is 11 test-only lines. Production code is unchanged from the approved 203186cbc. The test keeps listChannel, currentPubkey: null, the activity: mark and the Mark unread assertion.

Evidence (local):

• Without the fix, the test fails with the same MissingPluginException.
• With the fix, repeated loops of the file: 0 failures.
• Full message_actions_test.dart: 57/57 pass. flutter analyze is clean.
• Hosted Clients / Mobile at this head is running now.

[jedwards27]

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reviewed: PR exact live head 1e1efabf0572e8b6dfe876c89726fd4cb74ddba6 (GitHub-reported base fcf5f044b3af56f3855a417e61e4389fa925c1bd)
Risk: medium — the remediation is test-only, but it is intended to make required production-shaped sheet/popover/native action coverage deterministic and its lifecycle mechanism contradicts the pinned Flutter test framework.

Behavior/contracts traced: binary-messenger mock lifecycle; file/test setup ordering; asynchronous connectivity EventChannel activation; mounted provider/native runAsync lifetime; prior signing-identity/read-state production bytes and assertions; required Mobile CI.

Finding — blocking harness defect: mobile/test/features/channels/message_actions_test.dart:412-421 installs the connectivity handler once in setUpAll. Pinned Flutter 3.41.7 documents that handlers registered by TestDefaultBinaryMessenger.setMockMethodCallHandler are cleared after each test (packages/flutter_test/lib/src/test_default_binary_messenger.dart:199,290; stream mocks repeat the rule). The first test begins immediately afterward, while the formerly failing native no-profile test is hundreds of lines later. The framework clears this handler after test one, so later sheet/popover/native providers can still hit the same MissingPluginException. The comment’s claimed file lifetime is impossible under the framework contract; even a lucky green run would not make this deterministic.

Author action: register the event-channel method handler in per-test setUp so Flutter reinstalls it before every test and clears it afterward, or use a deterministic inert lifecycle-provider override in every relevant harness. Preserve listChannel, currentPubkey: null, the activity: marker, native payload capture, and exact Mark unread assertion. Then prove the formerly failing native test and full required Clients / Mobile gate pass at the repaired head.

Verification owner: author/required Mobile CI for the fix; reviewer for exact-head lifecycle/isolation and freshness.

Validation:

• PASS — PR-authored remediation delta is test-only; production mobile/lib bytes remain identical to the previously approved behavior.
• PASS — just mobile-check, git diff --check, DCO, documentation, and static policy checks.
• FAIL by framework contract — setUpAll registration cannot survive Flutter’s automatic per-test handler clearing.
• PENDING at submission — exact-head Clients/Mobile, Mobile Swift, and Codex checks. They are corroboration, not a substitute for a lifecycle-safe setup.

Manual/native evidence: no device execution; reviewer-local Flutter startup remains blocked by unrelated Xcode/native-asset tooling.

Residual risk: GitHub currently reports a base that is not an ancestor of the head, so two-dot comparisons include unrelated inverse mainline movement; this integration state needs refresh on the next head. It does not change the concrete setup-lifecycle defect. Any new head requires delta and exact-head gate review.

[jedwards27]

Request changes

The remediation's claimed file lifetime contradicts Flutter's test lifecycle, so the connectivity handler is gone before the tests it is meant to protect.

mobile/test/features/channels/message_actions_test.dart:415-421 registers the handler once in setUpAll. The repository-pinned Flutter 3.41.7 implementation explicitly states that callbacks registered by TestDefaultBinaryMessenger.setMockMethodCallHandler are cleared after each test (packages/flutter_test/lib/src/test_default_binary_messenger.dart:290; the underlying handler contract says the same at line 199). The first test starts at line 423, while the formerly failing native no-profile test does not start until line 696.

Consequently, the handler protects only the first test. Flutter clears it before the later sheet, popover, and native cases can activate Connectivity().onConnectivityChanged, leaving the exact MissingPluginException failure mode from the prior head structurally intact. This is deterministic from the pinned framework contract; a lucky green run would not validate the claimed lifetime.

Required fix: install the handler in setUp, not setUpAll, so it is reinstated before every test and Flutter's automatic cleanup removes it afterward. A deterministic lifecycle-provider override in every relevant harness is also acceptable. Preserve the listed channel, currentPubkey: null, activity: marker, native payload capture, and exact Mark unread assertion. Then rerun the formerly failing native test and the full required Mobile gate.

The new commit itself is test-only (11 insertions in message_actions_test.dart). Range-diff shows no additional PR-authored production change, and the previously reviewed signing-identity/read-state implementation and causal assertions remain intact. The defect is confined to the ineffective remediation lifecycle.

[loganj]

🤖 Connectivity mock now installed before every test

Head: 69da0b704 (rebased on current main c9143ed8f, so the base is an ancestor again)

Change. The connectivity handler in message_actions_test.dart moved from setUpAll to setUp. This matches Flutter's documented rule that mock handlers are cleared after each test. The test still uses listChannel, currentPubkey: null, the activity: mark, native payload capture and the exact Mark unread assertion. Production code is unchanged.

Note on the finding. On the pinned Flutter 3.41.7, a probe file with a setUpAll method handler answered in a second testWidgets too, so the old setup was not cleared in practice. The setUp form is still the correct one, because it does not depend on undocumented behavior.

Evidence (local, at this head):

• The formerly failing native test passes alone and in the full file (57/57).
• All 5 PR-changed test files: 428 pass. flutter analyze is clean. The pre-push mobile-checks passed.

[jedwards27]

:bot: Jude’s code review agent

Approved at exact head 69da0b7048fe54b78efe9458aa80179a40969246 against base c9143ed8f709a06db9f4e6994f3ffa39aa5f3950.

Both independent lanes confirmed the remediation fixes the prior blocker without weakening the product-shaped regression:

• mobile/test/features/channels/message_actions_test.dart:416-422 now installs the connectivity MethodChannel handler in per-test setUp, matching pinned Flutter 3.41.7’s contract that mock handlers are cleared after each test. This provides a fresh handler for every test and framework-owned cleanup rather than the invalid file-lifetime setUpAll assumption.
• The formerly failing native row at message_actions_test.dart:697-735 still uses a listed channel, currentPubkey: null, activity:chan-1, native payload capture, and the exact Mark unread assertion. Sheet and popover no-profile coverage remains intact.
• The new-head delta is test-only; production behavior commits are equivalent to the previously reviewed implementation. Base ancestry and clean-tree git diff --check were verified.
• Both lanes passed just mobile-install and just mobile-check at this head: 656 files format-clean and analyzer clean. DCO, Semgrep, and zizmor were green when reviewed.

Author action: none.

Confidence gap / verification owner: local focused and full Flutter execution still abort before test discovery on reviewer machines because the objective_c native-assets hook cannot obtain an SDK path from xcrun. This is reviewer tooling, not an author defect. Exact-head Mobile CI remains responsible for proving the formerly failing row and full Mobile suite green before merge; unrelated queued/startup gate failures should not be attributed to this test-only delta without causal logs.

[jedwards27]

Approve

The remediation now matches Flutter's test lifecycle and preserves the production-shaped regression coverage.

At exact head 69da0b7048fe54b78efe9458aa80179a40969246, the connectivity MethodChannel handler is installed in per-test setUp (mobile/test/features/channels/message_actions_test.dart:416-422). Repository-pinned Flutter 3.41.7 clears setMockMethodCallHandler callbacks after each test (flutter_test/lib/src/test_default_binary_messenger.dart:290), so every test receives a fresh handler and framework cleanup prevents cross-test/file leakage. This fixes the invalid setUpAll lifetime without introducing persistent global state.

The formerly failing native test remains product-shaped (message_actions_test.dart:697-733): it keeps the listed channel, currentPubkey: null, activity:chan-1 catch-up marker, native payload capture, and exact assertion that the read action is titled Mark unread. Sheet and popover no-profile coverage remains intact. Range-diff shows the previously reviewed signing-identity/read-state production commits are equivalent; only the test remediation changed.

Validation and scope:

• remediation commit: test-only, 12 insertions in message_actions_test.dart;
• base c9143ed8… is the exact merge-base and ancestor of this head;
• git diff --check passes;
• both independent review lanes report just mobile-install and just mobile-check passing on clean exact-head worktrees (656 files format-clean; analyzer clean);
• DCO, Semgrep OSS, and zizmor pass.

No author action remains. Exact-head Mobile CI is still queued, and reviewer-local Flutter execution stops before discovery in the host's objective_c/xcrun SDK tooling. Those are confidence/verification gaps, not demonstrated PR defects. The merger/required gate owns confirmation that the formerly failing row and full Mobile suite are green before merge.