Repository navigation
Speed up mobile Solo chat creation and agent discovery - #8547
Conversation
Use owner-scoped agent inventory and known-archive filtering, load native compose choices asynchronously, and overlap independent creation reads while retaining durable recovery and relay confirmation. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <klopez4212@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Signed-off-by: kenny lopez <klopez4212@gmail.com>
🔐 Codex Security Review
Review SummaryOverall Risk: NONE
FindingsNo concrete security, correctness, or reliability findings were identified. Notes
Generated by Codex Security Review | |
Keep project selection from main and refresh the agent row at its new section. Cover the combined native loading flow in the existing simulator regression. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e66bfab750
ℹ️ 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".
| final id = event.getTagValue('d'); | ||
| if (id == null || !RegExp(r'^[0-9a-f]{64}$').hasMatch(id)) continue; | ||
| // Verify before allowing a newer tombstone or record to replace a head. | ||
| nostr.Event.fromJson(jsonEncode(event.toJson())); |
There was a problem hiding this comment.
If an otherwise well-formed relay response contains one owner-scoped kind:30177 event with an invalid ID or signature, this constructor throws out of the isolate and causes the entire agent directory to fail, hiding every unrelated valid agent. This can occur with a buggy or untrusted relay and differs from _verifiedProfiles, which explicitly skips invalid events; catch verification failures per inventory event and continue processing the remaining records.
Useful? React with 👍 / 👎.
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Reviewed exact head e66bfab750c3579a9fdaef38fe1654a6ae72b7d1 against base dc25198d530acc6237a0320b697365f29daf0ea6. I recommend holding merge for one author-actionable P1.
P1 — native iOS locked retry can strand a durable Solo chat retry when the saved agent is no longer in the refreshed picker inventory
The retry path restores the saved pending intent and presents the native sheet with the saved selectedAssistant and fieldsLocked: retry (mobile/lib/features/channels/me/native_new_me_chat.dart:41-48, :78, :85). The PR then refreshes the assistant list asynchronously and replaces the native assistants inventory (mobile/lib/features/channels/me/native_new_me_chat.dart:91-121; mobile/ios/Runner/NativeProfileTextEditor.swift:442-449).
That refreshed list is a discovery surface: it intentionally filters archived/currently hidden agents for new chat creation (mobile/lib/shared/me/me_assistants.dart:31-34). Admission for a single saved retry is separate: _createMeChat revalidates the chosen assistant through isOwnedMeAssistant, which explicitly supports existing chats whose agent may no longer be offered in the new-chat inventory (mobile/lib/shared/me/me_assistants.dart:63-68; mobile/lib/features/channels/me/native_new_me_chat.dart:175-184).
Native currently gates both the Retry button and submit on picker membership instead:
- button enabled only if
assistants.contains { $0["id"] == selectedAssistant }(mobile/ios/Runner/NativeProfileTextEditor.swift:452-459) - submit repeats
assistants.contains(where: { $0["id"] == selectedAssistant })before returning the saved assistant id to Flutter (mobile/ios/Runner/NativeProfileTextEditor.swift:498-503)
Failure scenario: the user has an unfinished Solo chat with saved agent A. Before native iOS retry, A is removed from the refreshed picker inventory because it is archived/hidden/stale. The form is locked, so the user cannot choose another agent, and native disables/refuses Retry before _createMeChat can either complete the durable retry or surface the intended authoritative “This agent is no longer available.” error. The durable journal survives, but the only recovery affordance is blocked. That violates the PR’s stated “preserve durable retries” contract and the mobile vision’s pending-action recovery principle.
Author action: for fieldsLocked retry mode, allow submit when the saved selectedAssistant != nil and the chat name is valid, even if that assistant is absent from the refreshed assistants list. Keep the stricter picker-membership gate for new/non-locked creation. Return the saved id to Flutter so the existing _createMeChat ownership revalidation remains authoritative. Add a native regression covering fieldsLocked=true, saved assistant absent from assistants: Retry is enabled and onSet returns the saved id; also cover that non-retry creation still cannot submit an unlisted assistant.
Validation notes: local review pinned the live PR head to e66bfab750c3579a9fdaef38fe1654a6ae72b7d1; reviewer identity jedwards27 is distinct from PR author klopez4212. Mobile-focused checks reported for this head include just mobile-install, just mobile-check, targeted Flutter tests for Solo creation/preflight/tags/assistants/archived agents/chat creation, and the native control script on a booted simulator. I did not treat the lack of a physical-device recording or a full local just mobile-test run as the blocker here; the blocker is the source-level native admission defect above.
Allow locked retries to return their saved agent to Flutter for authoritative ownership revalidation. Keep inventory membership required for new chats and cover both native button and submit gates across inventory refreshes. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2282c11ea
ℹ️ 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".
| await for (final chunk in response.stream.timeout( | ||
| const Duration(seconds: 3), | ||
| )) { |
There was a problem hiding this comment.
When the relay sends a chunk at least once every three seconds, Stream.timeout resets after each event, so a 256 KiB NIP-11 body can keep meArchivedAgentsProvider—and therefore agent discovery—pending for days despite the apparent three-second bound. Wrap the entire body-consumption operation in a single wall-clock deadline while retaining the inactivity and size limits.
AGENTS.md reference: AGENTS.md:L246-L254
Useful? React with 👍 / 👎.
Skip invalid inventory events without losing valid agents, and cancel metadata body reads after a total deadline. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <klopez4212@gmail.com>
Preserve targeted fresh channel lookup and cover interrupted recovery alongside asynchronous compose loading. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <klopez4212@gmail.com>
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes's GitHub account.
Reviewed exact head 21879534 against base dc25198d, with separate lanes for relay contracts, the saved-chat retry path and native iOS.
Verdict: hold merge for two P2s (inline): a retry can run without a saved recovery record, and a locked retry can send to an agent the sheet doesn't show.
The earlier findings are addressed: Jude's locked-retry P1 (b2282c1), and Codex's per-event inventory verification and NIP-11 total deadline (2187953). No material findings in the relay/security lane: inventory pagination, deletes, archive authority and ownership admission all hold up on a source read.
Validation gaps: source review plus one targeted probe for the persistence P2; no device run by us. The PR notes that final compose timing on a device is still pending.
Merge criteria: fix both P2s; the P3s are optional.
| previous.project != project)) { | ||
| throw StateError('Resume the pending chat before starting another.'); | ||
| } | ||
| if (previous != null) return; |
There was a problem hiding this comment.
P2: Retry can run creation without a saved recovery record.
SharedPreferences.setString (legacy API, 2.5.5) writes to its in-memory cache before the platform write, and keeps the cached value even when that write returns false. So if the first save fails ("Couldn't save this chat on your phone"), pending(sync) still returns the intent from RAM. On the next Retry, previous != null takes this early return, complete goes on to create the channel, and nothing was ever saved to disk. If the app is killed partway through, the resume prompt is gone after restart, and you're left with a half-created channel.
We reproduced this through the production method: the first save throws, Retry reaches createChannel, and reloading preferences shows no saved intent. The probe fails at this head and passes with only this early return removed.
Fix: drop the early return and keep the previous idempotent rewrite, or only skip the write once persistence has been confirmed, for example by reloading after a failed write.
| // Flutter revalidates ownership before any creation writes. | ||
| private var canSubmitAssistant: Bool { | ||
| guard let selectedAssistant else { return false } | ||
| return fieldsLocked || assistants.contains { $0["id"] == selectedAssistant } |
There was a problem hiding this comment.
P2: A locked retry submits an agent the sheet doesn't show.
Gating submit on fieldsLocked correctly unblocks Retry. But the agent row (:635-640, outside this diff) still builds its label and avatar only from assistants. So in both cases this gate now allows, the row reads "Choose an agent" with no avatar, and it's disabled because fieldsLocked is set:
- the sheet opens before discovery finishes. That's the normal cold-start resume path now, since
assistantsLoadingis true; - discovery leaves out the saved agent, which is Jude's P1 case.
Retry is enabled anyway, and it sends selectedAssistant to Flutter. The user can't see or change which agent will be added to the chat.
Fix: in presentChat, pass the saved recipient's display name and avatar (Flutter already has pending.agent and the user cache). When the selected agent isn't in assistants, render the row from those values.
|
|
||
| override func tableView(_ tableView: UITableView, titleForFooterInSection section: Int) -> String? { | ||
| guard isChatEditor && section == 2 else { return nil } | ||
| if chatOptions?["assistantsLoading"] as? Bool == true { return "Loading agents…" } |
There was a problem hiding this comment.
P3 (optional): In locked mode, this footer order hides the retry reason.
assistantsLoading and assistantError are checked before chatOptions["error"]. On a cold-start resume, "Resume your unfinished chat." or the previous failure is replaced by "Loading agents…". If discovery then fails, it's replaced by "Couldn't load agents. Close this sheet and try again." That's wrong advice in locked mode, because Retry is enabled and doesn't need discovery. Suggest checking error first when fieldsLocked is set, or skipping the discovery footers in that mode.
| final directory = await ref.read(meAssistantsProvider.future); | ||
| final directory = ref.read(meAssistantsProvider).asData?.value; | ||
| final directoryRequest = ref.read(meAssistantsProvider).hasError | ||
| ? ref.refresh(meAssistantsProvider.future) |
There was a problem hiding this comment.
P3 (optional): The new guards against late or stale results have no regression coverage.
None of these new guards fails a test if removed:
- refreshing the agent list after an error (this line);
- the
activecheck that drops late results; - the
assistantErrorupdate; - Swift ignoring an update whose
presentationIddoesn't match.
For example, if this refresh is dropped, every reopen after one discovery failure shows the cached error again until the relay status changes, and no test notices. Per TESTING.md, suggest extending existing tests rather than adding new ones: one failure → reopen → success step in the solo_creation_tags_test journey, and a mismatched-presentationId row in MeAgentAccessibilityTests.
jedwards27
left a comment
There was a problem hiding this comment.
🤖
:bot: Jude’s code review agent
Reviewed exact head 5e8c5493394bbee9a13fc16fb1d8832647d7378b against base 209fcd21725b8728fed8b14dca1636018d175458. Reviewer identity jedwards27 is distinct from PR author klopez4212.
Verdict: hold merge for two author-actionable P2s. The previous exact-head checks at 218795345885a9789be42b9079825db653afc418 are historical only because the PR head moved; I re-pinned and reviewed the current head. The range-diff keeps the PR commits intact and the current merge changes only the test-side Solo journey relative to 2187953, but the current source still has the two retry/recovery defects below.
P2 — retry can perform relay writes after the first durable chat-save failed
MeChatCreation.save() now returns early whenever pending(sync) sees a matching record (mobile/lib/shared/me/me_chat_creation.dart:45-53). That read is from the legacy SharedPreferences in-memory cache. In shared_preferences 2.5.5, setString updates that cache before the platform write result is known, and the package explicitly says the cache is not guaranteed to stay in sync when a setter fails. So when the first setString returns false, this PR throws “Couldn’t save this chat…”, but the same-process cache still makes pending(sync) non-null.
On the next Retry, complete() calls save() (mobile/lib/shared/me/me_chat_creation.dart:105), the early return skips the disk rewrite, and then complete() proceeds to findChannel / createChannel / membership / placement writes (mobile/lib/shared/me/me_chat_creation.dart:106-124) without any durable recovery record on disk. If the app is killed after createChannel or addAgent, restart has no saved me-chat-creation intent, so the user loses the resume prompt and is left with a partial Solo channel. That violates the PR’s durable retry contract and the mobile vision’s “don’t lose user-generated data” rule.
Author action: remove the early return, or only skip the rewrite after proving the matching record is actually durable (for example by reloading/confirming disk state after write failures). Add a regression using a SharedPreferencesStorePlatform whose first chat creation setValue returns false: the same-process retry must not reach createChannel until a durable journal exists, and after prefs.reload() the recovery record must still be present.
P2 — locked iOS retry can submit an agent the sheet does not identify
The fix for the previous locked-retry blocker correctly lets retry submit the saved selectedAssistant even when discovery does not list it (mobile/ios/Runner/NativeProfileTextEditor.swift:452-457, :506-510). But the agent row still renders only from the current assistants list (mobile/ios/Runner/NativeProfileTextEditor.swift:635-639) and is disabled in locked mode (mobile/ios/Runner/NativeProfileTextEditor.swift:643-644).
That means both normal cold-start resume while discovery is still loading and the “saved agent no longer appears in discovery” case show the locked row as “Choose an agent” with no avatar, while the Retry button is enabled and sends the hidden saved pubkey to Flutter. The user cannot see or change which agent will be added to the chat. This preserves the transport path but breaks the visible confirmation boundary for a destructive-ish membership write.
Author action: pass the saved retry recipient’s display name/avatar (Flutter has the saved pubkey and user/profile cache) or otherwise include a non-selectable saved-recipient display model. In locked retry, render that identity even when it is absent from assistants, while keeping new/non-locked creation gated on current picker membership. Add a native regression for fieldsLocked=true, selectedAssistant absent from assistants: the row names the saved agent and onSet still returns the saved id; keep the non-retry absent-agent submit blocked.
Validation / residual risk
- Pinned current GitHub state with
gh pr view: head5e8c5493394bbee9a13fc16fb1d8832647d7378b, base209fcd21725b8728fed8b14dca1636018d175458. - Worktree HEAD matched the pinned head and was clean after review.
git range-diff dc25198d530acc6237a0320b697365f29daf0ea6..218795345885a9789be42b9079825db653afc418 209fcd21725b8728fed8b14dca1636018d175458..5e8c5493394bbee9a13fc16fb1d8832647d7378bshowed the same PR commits carried forward;2187953..5e8c549changes onlymobile/test/features/channels/me/solo_creation_tags_test.dartwithin the PR-touched mobile files.- Current GitHub checks had mobile checks/tests green when inspected, with Codex security review still in progress. That CI state is not the blocker; the two source-level defects above are.
- I did not require a physical-device recording for these findings. Simulator/native harness evidence is useful, but these are deterministic recovery/UI-state bugs at the source seam.
Rewrite the recovery journal after failed cached preference writes. Render the saved agent identity in locked native retries even when discovery has not returned it. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad99b13f46
ℹ️ 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".
| final response = await client | ||
| .send( | ||
| http.Request('GET', uri) | ||
| ..headers['Accept'] = 'application/nostr+json', | ||
| ) | ||
| .timeout(const Duration(seconds: 3)); |
There was a problem hiding this comment.
When a relay accepts the connection but never sends response headers, Future.timeout completes this provider with an empty set but does not cancel the underlying Client.send; because the shared client remains alive, each subsequent agent-directory refresh can leave another request and socket pending indefinitely. Use an abortable request (or otherwise close the request on deadline) so the three-second timeout also releases the transport resource.
AGENTS.md reference: AGENTS.md:L246-L254
Useful? React with 👍 / 👎.
| ..headers['Accept'] = 'application/nostr+json', | ||
| ) | ||
| .timeout(const Duration(seconds: 3)); | ||
| if (response.statusCode != 200) return {}; |
There was a problem hiding this comment.
When the relay returns a non-200 response with a slow or unbounded body, this early return never listens to or cancels response.stream, leaving the HTTP connection occupied until the provider's shared client is eventually disposed. Repeated discovery retries against such a relay can accumulate abandoned connections; cancel or boundedly drain the stream before returning.
AGENTS.md reference: AGENTS.md:L246-L254
Useful? React with 👍 / 👎.
jedwards27
left a comment
There was a problem hiding this comment.
🤖 :bot: Jude’s code review agent
Review verdict: changes recommended for one remaining P2 at exact head ad99b13f46cebfcf6ee6603fd8a6810a43db7c22.
Base/head pinned for this review: 209fcd21725b8728fed8b14dca1636018d175458...ad99b13f46cebfcf6ee6603fd8a6810a43db7c22. I verified the authenticated reviewer is jedwards27 and the PR author is klopez4212 before posting.
Finding
P2 — Flutter fallback / Android locked retry can submit a hidden saved agent while the row says “Choose an agent”
The iOS native retry sheet now has a saved-recipient fallback, but the Flutter fallback path still has the same misleading locked-action state:
- A pending Solo retry seeds
selectedandfrozenAgentfrom the durable intent:mobile/lib/features/channels/me/new_me_chat.dart:8-15. - The visible assistant row resolves
chosenonly from the current pickerchoices:mobile/lib/features/channels/me/new_me_chat.dart:30-34. - If refreshed discovery omits the saved id, the locked retry row renders the plus icon / “Choose an agent” and disables tapping because
frozenNameis set:mobile/lib/features/channels/me/new_me_chat.dart:85-104. - Pressing Retry still proceeds because
selected.valueis non-null, freezes/submitsfrozenAgent.value, and calls_createMeChat:mobile/lib/features/channels/me/new_me_chat.dart:144-188. _createMeChatthen intentionally revalidates the single saved recipient separately from picker discovery:mobile/lib/features/channels/me/native_new_me_chat.dart:184-197;mobile/lib/shared/me/me_assistants.dart:63-68.
This state is reachable after the PR’s discovery changes because the picker is current owner inventory minus archived identities (mobile/lib/shared/me/me_assistants.dart:31-37), while retry/admission can still allow an owned saved recipient absent from that picker.
Failure scenario: an Android user, or an iOS user on the Flutter fallback sheet, resumes a durable Solo setup whose saved agent is no longer present in refreshed picker discovery. The sheet is locked and visibly/accessibly says “Choose an agent”, but Retry can still add the hidden saved pubkey after ownership revalidation.
Impact: the retry may be mechanically correct, but the visible confirmation boundary lies by omission: the user cannot verify which agent will be added before a membership/write recovery action.
Author action: teach _NewMeChat the same saved-recipient display model as the native retry path. In locked retry mode, render the saved agent identity from the journal/cache/directory fallback, or at minimum an explicit unavailable saved-agent label. Do not render “Choose an agent” when selected.value / frozenAgent is non-null and will be submitted. Add a Flutter widget regression for the default/Android fallback where a pending saved agent is absent from meAssistantsProvider: the locked row visibly/accessibly identifies the saved/unavailable recipient and Retry submits that same id. Preserve non-retry empty discovery behavior as “Choose an agent” plus blocked submit.
Verified at this head
- The prior durable-retry blocker is fixed:
MeChatCreation.save()re-runs the persisted write before relay work and the new regression models the failed-first-write / polluted-cache shape (mobile/lib/shared/me/me_chat_creation.dart:43-69;mobile/test/shared/me/me_chat_creation_test.dart:9-58). - The prior native iOS locked-retry display blocker is fixed: Flutter passes
savedAssistant, native falls back to the saved recipient instead of “Choose an agent,” and the native control test covers the absent-from-picker retry row (mobile/lib/features/channels/me/native_new_me_chat.dart:78-91;mobile/ios/Runner/NativeProfileTextEditor.swift:635-643;mobile/ios/NativeControlTests/MeAgentAccessibilityTests.swift:19-70). - Local evidence reviewed at this exact head:
git diff --check,just mobile-install,just mobile-check, focused Flutter tests for Solo creation/retry/agent discovery, the native Swift control script on an iPhone 17 Pro simulator, and fulljust mobile-testall passed in the review lanes. - GitHub CI sample at posting time: mobile checks/tests/Android, Mobile Swift Native/Release/Results, DCO, Semgrep, and zizmor were green.
Run Codex Security Reviewwas still in progress; that is a verification gap owned by CI/reviewer unless it fails, not an author-actionable defect by itself.
Render the saved recipient from the profile cache or public key when picker discovery omits it. Extend the Solo journey to cover Android retry labels and semantics, the submitted recipient, and empty new-chat discovery. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <klopez4212@gmail.com>
jedwards27
left a comment
There was a problem hiding this comment.
🤖 :bot: Jude’s code review agent — APPROVE at exact head f3118beee8f9f4f28ebd23300d64f89b6447fc3d.
I re-reviewed this changed head against the stated intent: speed up mobile Solo chat creation/agent discovery while preserving retry durability, saved-agent visibility, ownership checks, relay confirmation, archived-agent filtering, and picker correctness.
Verdict
No unresolved author-actionable defects found.
Author action: none.
Integrated review evidence
- The prior Flutter/Android fallback retry issue is fixed: locked retry now falls back from live picker choices to the frozen saved agent/profile label (
mobile/lib/features/channels/me/new_me_chat.dart:30-39), keeps the row locked/non-tappable, and submits the frozen saved id on Retry. - Native iOS retry presentation preserves the same behavior: Flutter passes a
savedAssistantfor locked retry (mobile/lib/features/channels/me/native_new_me_chat.dart:78-91), the native sheet can submit locked retry independent of current picker membership while Flutter still revalidates before relay writes, and the saved identity renders instead of “Choose an agent” (mobile/ios/Runner/NativeProfileTextEditor.swift:452-510,:635-643). - Retry intent remains durable before relay side effects:
MeChatCreation.save()rewrites encrypted storage and fails closed if the disk write fails (mobile/lib/shared/me/me_chat_creation.dart:43-69), andcomplete()keeps the journal through channel create/member/place before clearing the reviewed record (:95-130). - The channel-create speedup still waits for relay confirmation and then fails closed unless targeted metadata + roster + self-membership are visible (
mobile/lib/features/channels/channel_management_actions.dart:56-64,:69-108). - Discovery and creation stay separated correctly: picker inventory filters current owner inventory/archived evidence, while creation admission revalidates the chosen recipient directly before writes.
Validation
Exact state checked locally:
git rev-parse HEAD→f3118beee8f9f4f28ebd23300d64f89b6447fc3dgit merge-base HEAD 209fcd21725b8728fed8b14dca1636018d175458→209fcd21725b8728fed8b14dca1636018d175458git status --short→ cleangit diff --check 209fcd21725b8728fed8b14dca1636018d175458...HEAD→ PASSgh api user/gh pr viewconfirmed reviewerjedwards27, PR authorklopez4212, base209fcd21725b8728fed8b14dca1636018d175458, and headf3118beee8f9f4f28ebd23300d64f89b6447fc3d.
Review lanes also reported passing local evidence at this exact head, including:
just mobile-check— PASS- Focused Flutter Solo/retry/discovery/archive/preflight tests — PASS
- Full
just mobile-test— PASS in the systems lane (3789passing /19skipped, plus push-unconfigured suite4passing) - Native Swift control script on iOS simulator — PASS
- Mutation control for the Flutter saved-recipient fallback — mutation killed
GitHub status check sample at final review time showed no failed or pending required checks for this head. Mobile checks/tests/Android, Mobile Swift native/release/results, DCO, Semgrep, zizmor, and Codex security were green or completed successfully where applicable.
Residual risk
I did not require a physical-device recording or a fresh final timing recording. Simulator/native harness coverage, widget regressions, mutation control, full mobile test coverage, and green CI exercise the changed retry/native/fallback seams sufficiently for approval. Any additional device/timing recording would be verification depth, not author rework.
jedwards27
left a comment
There was a problem hiding this comment.
🤖 :bot: Jude’s code review agent — changes recommended at exact head f3118beee8f9f4f28ebd23300d64f89b6447fc3d.
This supersedes my earlier approval on this same head. I rechecked the NIP-11 archive-discovery resource boundary and found one PR-caused P2. The prior saved-recipient/durable-retry blockers remain fixed, but this archive path still needs author action before I would recommend merge.
Reviewed: 209fcd21725b8728fed8b14dca1636018d175458..f3118beee8f9f4f28ebd23300d64f89b6447fc3d
Risk: medium — mobile Solo discovery now refreshes relay NIP-11 archive evidence during assistant discovery. Relays are untrusted, so error/timeout paths must not leave unbounded HTTP work attached to a long-lived client.
Finding
P2 — NIP-11 archive-filter error responses can leave an unused response stream open
meArchivedAgentsProvider sends a streaming HTTP request for relay information, but it only wraps/cancels the response body on the 200 path:
mobile/lib/shared/me/me_archived_agents.dart:25-31sendsclient.send(...)and returns{}immediately whenresponse.statusCode != 200.mobile/lib/shared/me/me_archived_agents.dart:32-50createsStreamIterator(...)and callschunks.cancel()only after the status-code check.mobile/lib/shared/me/me_assistants.dart:23-29refreshes this archive evidence during Solo assistant discovery, including explicit retries.
Failure scenario: a relay returns 503 (or another non-200) with a slow/unbounded body. Treating archive evidence as unavailable is fine, but the code returns without draining or cancelling the body. With a real HTTP client that can keep the underlying response/socket alive until broader client disposal; in a deterministic MockClient.streaming regression, onCancel is never called and the test process remains open until timeout. Repeated discovery opens/retries against a bad relay can accumulate dangling HTTP work, which violates the mobile resource-boundary rule for untrusted relays.
The header deadline has the same ownership gap: Future.timeout around a plain http.Request stops awaiting the future but does not abort the underlying send. This package already has http.AbortableRequest; the request deadline should own the in-flight request, not just the awaiter.
Author action: ensure every unused NIP-11 response body is cancelled or drained before returning, including non-200 responses, and add a regression with a non-200 slow/unbounded body. Also make the request/header timeout abort the in-flight request (http.AbortableRequest/abort trigger or equivalent) and test that timeout path.
Verification owner: author for code + regressions; reviewer/CI can rerun the archive cancellation regression and focused mobile tests on the next exact head.
Previously reported blockers rechecked
- Flutter/Android locked retry saved-recipient display is fixed: the fallback sheet resolves the visible selected row from current picker choices or the frozen saved retry recipient/profile label (
mobile/lib/features/channels/me/new_me_chat.dart:30-39) and no longer shows “Choose an agent” while retrying a hidden saved id. - Native iOS locked retry display remains fixed: Flutter passes
savedAssistantfor retry (mobile/lib/features/channels/me/native_new_me_chat.dart:78-91), and UIKit can render/submit the saved recipient independent of current picker membership while Flutter still revalidates before relay writes (mobile/ios/Runner/NativeProfileTextEditor.swift:452-510,:635-643). - Durable retry before relay side effects remains fixed:
MeChatCreation.save()rewrites encrypted storage and fails closed if the disk write fails before channel/member placement work (mobile/lib/shared/me/me_chat_creation.dart:43-69,:95-130).
Validation
Exact local state for the reproducer:
git rev-parse HEAD→f3118beee8f9f4f28ebd23300d64f89b6447fc3dgit status --short→ clean after the temporary regression file was removedgh api user/gh pr viewconfirmed reviewerjedwards27, PR authorklopez4212, base209fcd21725b8728fed8b14dca1636018d175458, and headf3118beee8f9f4f28ebd23300d64f89b6447fc3d;git ls-remote origin pull/8547/headmatched the same head.- Temporary regression added locally:
MockClient.streamingreturnshttp.StreamedResponse(body.stream, 503); the provider returns an empty archive set, butbody.onCancelstays false and the test times out. That failed at this exact head, then the temporary file was removed and the worktree was clean.
Additional exact-head evidence from the review room still stands for the already-fixed seams: just mobile-install, just mobile-check, focused Flutter Solo/retry/discovery/archive/preflight tests, full just mobile-test, native Swift control script on an iOS simulator, and mutation control for the Flutter saved-recipient fallback all passed.
Residual risk
I did not require a physical-device recording for this finding. The blocker is a deterministic source/testable resource-boundary defect in the new archive-discovery path, not a confidence gap about native UI observation.
Adopt #8547's owner inventory discovery; keep this branch's owner-name labels. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@block.xyz>
* origin/main: Make mobile onboarding QR-first and synchronize scanner motion (#8599) Speed up mobile Solo chat creation and agent discovery (#8547) fix(desktop): keep FileCard inside narrow message rows (#8612) fix(mobile): isolate push setup failures from app rendering (#8610) chore(toolchain): bump Rust to 1.98.1 for macOS 27 proc-macro loading (#8598) fix(desktop): recognize agents without NIP-OA in Huddles (#8544) Generate user community inventory for the dummy deletion workflow (#8605) perf(mobile): batch initial channel read-state seeds (#8607) Signed-off-by: Tom Brow <tomb@block.xyz>
* origin/main: fix(push): accept canonical channel UUIDs across versions (#8609) fix(mobile): align Solo agent policy and labels with the owner's inventory (#8616) fix(mobile): open message links to public channels the user has not joined (#8518) Make mobile onboarding QR-first and synchronize scanner motion (#8599) Speed up mobile Solo chat creation and agent discovery (#8547) fix(desktop): keep FileCard inside narrow message rows (#8612) Signed-off-by: Tom Brow <tomb@block.xyz>
Summary
Solo compose now opens before agent discovery finishes, and chat creation avoids full channel-list reloads while overlapping independent reads. Durable retries, ownership checks, and relay confirmation remain intact.
The agent picker uses the owner's saved inventory, excludes known archived identities, verifies profiles off the UI thread, and reuses avatar artwork.
Related issue
No matching issue found. Related: #8546 adds project selection to the same compose surface.
Testing