feat(mobile): browse and join open channels - #6243
Conversation
Signed-off-by: Tom Brow <tomb@block.xyz> Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true
Signed-off-by: Tom Brow <tomb@block.xyz> Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true
Signed-off-by: Tom Brow <tomb@block.xyz>
Signed-off-by: Tom Brow <tomb@block.xyz> Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true
Signed-off-by: Tom Brow <tomb@block.xyz>
Signed-off-by: Tom Brow <tomb@block.xyz>
Signed-off-by: Tom Brow <tomb@block.xyz>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14b8a9e7c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Tom Brow <tomb@block.xyz>
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Requesting changes at exact head fafe4cb74452dd4ad1553c399426b9b6741a5cca.
Blocker: discard directory work that settles after the relay or identity changes
mobile/lib/features/channels/channels_provider.dart:173-205 captures the directory scope for the load-status label, but after awaiting _fetchChannelDirectoryMetas it unconditionally assigns _directoryMetas and merges those events into the channel list. retryDirectory() then unconditionally installs that completed fetch into current provider state at mobile/lib/features/channels/channels_provider.dart:930-957. The HTTP request snapshots its relay URL and signing key before its await (mobile/lib/shared/relay/relay_session.dart:171-190), so switching communities while Browse is loading lets community A's delayed response populate community B's current state and browse sheet. That crosses the tenant boundary in VISION.md:52-56.
A deterministic paused-response probe reproduced this twice independently: start A's directory query, switch relayConfigProvider to B and rebuild, then release A. Expected B's list to remain empty; actual current state contained A's channel. The shipped tests do not exercise this ordering.
Fence completion with the captured relay + identity scope and a request generation. Re-check after every await and before writing directory metadata, load status, subscriptions, or provider state. Please add deterministic stale-success and stale-failure regressions for both community and identity switches.
Validation
just mobile-check: pass at exact clean head.- Full
just mobile-test: pass, 1,560 tests, at exact clean head. - GitHub Mobile and all completed required checks are green at this exact head.
- Pagination's composite
until+before_idcursor, repeated-page termination, access filtering, truthful unknown counts, and kind9021join followed by membership refresh were traced; no second material defect found.
The PR screenshots show the intended UI but are not a SHA-bearing exact-head native receipt. After the fix, please attach fresh exact-head simulator/native workflow evidence for browse, join, and membership refresh.
A community or identity switch changes which tenant the channel directory belongs to, but an in-flight directory query kept writing its response into whatever scope was active when it landed. The user switched community, and the previous community's open channels appeared in the new community's list. The load status had the same hole on the failure path: a request that failed after the switch marked the new scope as errored. VISION.md describes tenant isolation as a boundary, not a filter, so a retired response is discarded rather than merged. The fix captures the relay-and-identity scope plus a monotonic generation at request time, then re-checks both after the await and before every write, on the success path and the failure path alike. A retired request throws _StaleDirectoryRequest, which retryDirectory swallows so it writes neither the channel list nor the load status. An in-scope failure still returns null, which keeps the existing "retain the cached discovery" behavior. The loader lives in the sibling part file channel_directory.dart because channels_provider.dart already sits at the 1000-line ceiling that `just file-size-check` enforces, so it may not grow at all. Its inline fetch block moved into the loader and a `forRef` factory keeps the construction to one line, which leaves the provider slightly smaller than before. The split is mechanical: the fence moved out, nothing else changed shape. Tests: four new deterministic arms in channels_provider_test.dart, covering stale success and stale failure for both a community switch and an identity switch. The fake relay session gained a pausable directory query so the switch can be interleaved between request and response, and a request-time snapshot so a paused response reflects the community that issued it. All four failed against the unfixed provider, and the first reproduces the reported symptom exactly. Positive control: mutating the fence's isCurrent() to `=> true` turns all four red again, so the fence and not the harness is what makes them pass. Suite moved from 1560 to 1564 tests, all passing. `just mobile-check` and `just file-size-check` are clean. Co-authored-by: Tom Brow <tomb@block.xyz> Signed-off-by: Tom Brow <tomb@block.xyz>
|
🤖 @jedwards27 Blocker addressed in a5fc7c4. The defect, confirmedReproduced before fixing. A directory request captured its relay and identity only for the load-status label. After awaiting The fence
The fence lives in the Regressions, four arms as requested
The fake relay session gained a pausable directory query, so the community or identity switch is interleaved between the request and its response, and a request-time snapshot so a paused response reflects the community that issued it. All four fail against the unfixed provider. The first reproduces your symptom exactly: community A's channel present in community B's list. Positive control: mutating the fence's ValidationAt This branch is behind Still outstanding from your reviewThe exact-head native receipt for browse, join and membership refresh is not attached yet. I am attempting it now and will post it as a follow-up comment. I am not going to claim evidence I have not produced, so if the rig does not work at this head I will say so here plainly rather than leave the ask looking satisfied. |
|
🤖 Review comments addressed at a5fc7c4. Three items, one code change. 1. Stale directory across a relay or identity switch (blocker, fixed)A directory request captured its scope only for the load-status label, then wrote directory metadata and the merged channel list after the await without re-checking it. Switching communities while Browse was loading let the previous community's delayed response populate the new one. The failure path had the same hole: a request failing after the switch marked the new scope as errored.
Four new deterministic regressions cover stale success and stale failure for both a community switch and an identity switch. All four fail against the unfixed provider. Forcing the fence's 2. Load the directory independently of the channel list (P2, not reproduced)Already fixed in 3. False "0 members" on discovered channels (P2, not reproduced)Also already fixed in ValidationAt This branch is behind Re-review requested from jedwards27 at this head. The exact-head native receipt for browse, join and membership refresh is still pending, and I will post it as a follow-up comment or state the gap plainly if the rig does not work at this head. |
Mobile previously exposed no way to browse or join channels.
Users can now browse and join eligible open channels from the Home quick-actions menu. The public directory loads on demand when Browse channels opens, while the existing kind 9021 join path refreshes membership after success.
How is it tested?
Manually tested (see screenshots) and added tests:
channels_provider_test.dartcovers access filtering, independently paginated membership and directory queries, relay-capped pages, repeated-page termination, hard page caps, on-demand directory loading, load failures, retry, and cached-channel retention.channels_page_test.dartcovers browse eligibility, loading and retry states, quick-action layout, and scrolling and joining from a 500-channel directory.search_page_test.dartcovers discoverable open-channel results without presenting unknown membership counts as zero.Local validation:
just mobile-checkjust mobile-test(1,560 tests)