Repository navigation
feat(mobile): configurable hold-to-talk for Bestie - #8665
klopez4212 wants to merge 11 commits into
Conversation
Codex Review Summary: ✅ CompletedThis 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. |
GitHub Actions
Error
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
buzz/mobile/lib/features/channels/channels_page.dart
Lines 264 to 268 in ebe2283
Preserve Solo channel IDs while their record reloads
During cold startup and every reconnect-triggered meGroupsProvider invalidation, the provider is temporarily AsyncLoading (potentially retaining previous data), but .asData discards that previous snapshot and substitutes an empty ID list. Cached personal DMs are consequently rendered under Team until Solo finishes loading, or indefinitely when its refresh errors; unwrap the previous value or maintain an account/community-scoped cache before classifying channels.
ℹ️ 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 defaultAgent = ref.watch(defaultAgentProvider).value; | ||
| Future<void> chooseDefault(String key) async { | ||
| if (savingDefault.value) return; | ||
| savingDefault.value = true; | ||
| final sync = ref.read(meSyncProvider); | ||
| try { | ||
| await saveDefaultAgent(sync, key); |
There was a problem hiding this comment.
When the starred agent is deleted, the successful path in delete_agent_action.dart:62-74 only refreshes the directory and never clears this preference. The launcher therefore continues presenting the deleted key as the default and can send voice notes into a conversation with a permanently stopped agent, while the filtered Agents list offers no star to repair the selection; clear or replace the preference as part of successful deletion.
AGENTS.md reference: AGENTS.md:L228-L235
Useful? React with 👍 / 👎.
GitHub Actions
Inactive
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 319bf2ba39
ℹ️ 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".
github-actions
Bot
added
the
codex-security-review-current
🔐 Codex Security Review
|
319bf2b to
3af2b2c
Compare
github-actions
Bot
removed
the
codex-security-review-current
GitHub Actions
Error
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3af2b2c608
ℹ️ 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 descriptor = await uploadService.uploadVoiceNote( | ||
| recording.file, | ||
| duration: recording.duration, | ||
| ); |
There was a problem hiding this comment.
When there is no cached DM and openDm fails—or the destination becomes invalid after both futures finish—while this upload succeeds, the record wait throws and discards the returned descriptor even though the blob was committed remotely. Each Retry then packages and uploads the recording again, consuming repeated bandwidth and potentially leaving multiple unreferenced blobs; persist the successful descriptor with the draft or defer uploading until destination resolution succeeds.
AGENTS.md reference: AGENTS.md:L219-L226
Useful? React with 👍 / 👎.
GitHub Actions
Error
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38f7092899
ℹ️ 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".
GitHub Actions
Inactive
github-actions
Bot
added
the
codex-security-review-current
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
🤖 :bot: Jude's code review agent
Reviewed exact PR head b77ad12a50839219011f7d331c69582153fea90d against current origin/main 76cd4294f65bde33eff031315509e81deccf7c9d. Requesting changes for one mergeability gate failure and one accessibility defect.
Required changes
-
Blocking — this PR is currently unmergeable against current
main.Evidence: after a fresh fetch,
gh pr view 8665 --json headRefOid,mergeStateStatus,mergeablereported headb77ad12a50839219011f7d331c69582153fea90d,mergeStateStatus: DIRTY, andmergeable: CONFLICTING.git merge-tree --write-tree origin/main HEADexits1with a content conflict inmobile/lib/features/channels/channel_detail_page.dart. Currentmainhas the visible-typing path usingvisibleTypingProvider(... selfPubkey: currentPubkey, include: ...), while this PR changes the same block to include default-agent threaded typing viachannelTypingProvider(channel.id)and manual self-filtering.Author action: rebase or merge current
main, resolvechannel_detail_page.dart, preserve both currentmain's visible-typing/self-filtering behavior and this PR's default-agent threaded typing behavior, then rerun the affected Mobile and Mobile Swift gates on the new head. -
Medium — saved/retry voice-note states expose a false screen-reader instruction.
Evidence:
DefaultAgentVoiceControltreats recording, finalizing, sending, pending saved voice notes, and restore-failed states asactive(mobile/lib/features/channels/default_agent_voice_control.dart:271-276), but the outerSemanticsalways advertisesHold to record a voice note. Release to send. Slide up to cancel.(mobile/lib/features/channels/default_agent_voice_control.dart:302-309). In pending/restore-failed/sending states,start()explicitly refuses to record (mobile/lib/features/channels/default_agent_voice_control.dart:172-180), while the visible controls are Retry/Discard/Sending (mobile/lib/features/channels/default_agent_voice_control.dart:394-436). After a delivery failure or failed saved-draft restore, VoiceOver users can be told to hold/release/slide even though the required recovery action is Retry or Discard.Author action: make the semantics state-specific. The hold/slide hint and
Record voice notecustom action should only be exposed when recording can actually start; recording should expose recording/release/cancel semantics; pending/restore-failed/sending states should expose only the actual Retry/Discard/Sending semantics. Add widget semantics regression coverage for delivery-failed and restore-failed states proving the hold/slide hint is absent and Retry/Discard remain actionable.
Verification noted
- GitHub status rollup at
b77ad12a50839219011f7d331c69582153fea90dshowed relevant Mobile, Mobile Android, Mobile Swift native/release/results, DCO, Semgrep, and zizmor checks green;gh pr checksreported no non-success/non-skipped checks. - Local reviewer validation at exact head included
just mobile-install,just mobile-check, and focused affected Flutter tests coveringdefault_agent_voice_control,default_agent_voice_draft,default_agent_launcher,send_message_provider, andhome_page. - Physical device/simulator live long-press voice-note smoke was not part of this review; after the rebase/conflict resolution, use risk judgment to rerun live gesture validation on the final head.
A new head is required before this can be approved; this exact-head review does not carry forward automatically.
| if (!context.mounted || !destinationIsCurrent()) { | ||
| throw StateError('The community changed.'); | ||
| } | ||
| await session.publish(previousEvent); |
There was a problem hiding this comment.
🤖 P2 · Accepted voice-note retries disappear from cached timelines
If the relay accepts the first send but its EVENT echo and acknowledgement are lost, SendMessage rolls back the optimistic row while the signed event remains in the draft. Retrying here publishes that event directly. The relay's duplicate path returns success without broadcasting another EVENT, and this branch bypasses the local add/complete bookkeeping. Reopening a previously loaded conversation within its 30-second subscription-retention window does not reload history, so the draft is cleared and the chat opens without the successfully sent voice note.
User/system effect: successful recovery appears to have sent nothing, which can lead the user to record and send the same note again.
Test: accept the initial publication while dropping its echo/OK, retry before the cached timeline detaches, and assert that the accepted event appears once. This finding is based on source tracing; dependency DNS failure prevented running the prepared Flutter probe.
…8731) Messages with a reply footer now keep even vertical text padding unless they have actual overlapping reactions. Agent typing indicators are now visible in both one-to-one and group DMs when the response is thread-scoped, regardless of Bestie selection. The typing fix originated in #8665 and now applies to every agent DM using the existing agent classification. It retains main's `visibleTypingProvider` filtering. Ordinary channels and human thread-typing signals retain their existing scope; voice recording is not included. Validation: required pre-push hooks passed, including mobile formatting, analysis, tests, native crypto package tests, and file-size checks. Flutter analysis and the full mobile suite pass (3,995 passed, 28 skipped). Widget coverage compares bubble heights with and without a reply footer and checks typing across Solo, ordinary channels, agent DMs, human DMs, and group DMs with agents. Marked ready for review at the owner’s request. Device verification remains pending. To try: open a DM with any agent, including one not selected as Bestie, while it handles a thread-scoped response and check for its avatar and three dots; repeat in a group DM with an agent; compare equal text in messages with and without a reply footer, with no reactions, and confirm equal bubble heights. The broad `just ci` check was stopped during the unrelated desktop Rust dependency build after the mobile pre-push checks passed; no full-repository CI pass is claimed. ### Before / after Flutter widget-harness snapshots with identical synthetic data; not native device captures. Before: the reply-footer bubble has excess bottom padding and the thread-scoped Bestie typing signal is hidden. After: both text bubbles have equal height, and Bestie’s avatar and three dots are visible. | Before | After | | --- | --- | |  |  | --------- Signed-off-by: kenny lopez <[email protected]> Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz>
…rt (#8741) ## Summary Two RunnerTests fixes from the 23–30 Oct flake audit, both in `mobile/ios/RunnerTests/RunnerTests.swift`. Both are test-only. This is one of six PRs split from the reviewed `flakey-tests` branch (siblings below). ### `testLargeTitleTextFadesWithScrollAndCompactTitleCanReverse` (3 runs) - **Runs:** 40748163526 (job 123028155856), 40826294064 (#8679, job 123290415194, passed on attempt 2) and 40921441156 (#8694, job 123617149320), all in Mobile (iOS), attempt 1. None of the branches touches the navigation bar. Signature: `XCTAssertLessThan(visibleOpacity(of: centeredTitle), 0.99)` read 0.9999499, 1.0 and 1.0. - **Cause:** collapsing adds a 0.18 s `CABasicAnimation` on the compact title mask's opacity (`IosNavigationBar.swift` `applyScroll`). The test slept 60 ms and expected the presentation opacity to be mid-fade. The presentation layer follows wall-clock media time. When the Flutter app in the test host keeps the main thread busy, `Task.sleep` resumes after the fade has finished. - **Change:** drop the 60 ms sleep and the mid-fade samples. Right after `scroll(to: boundary)`, which runs the handler synchronously, check that the mask's model opacity is 1. Also check that it carries a `"titleFade"` opacity `CABasicAnimation` from 0 to 1, lasting 0.1–0.5 s. Nothing turns the run loop before the check, so the animation is still attached however long the main thread stalls. After the scroll back, a new `XCTAssertNil` checks that reversing removes the fade. The later 240 ms waits and their end-state checks are unchanged. - **Mutations that should fail it:** - Delete `compactTitleMask.add(fade, forKey: "titleFade")`: the `XCTUnwrap` throws. - Delete `removeAllAnimations()`: the new `XCTAssertNil` fails. The old test missed this. - Set `fade.duration = 0.001`: the band's lower bound fails. ### `testNativeRemovalConfirmationUsesDestructiveAlertAndCancelsOnce` (1 run) - **Run:** 40950548609 (#8433, Dart only, job 123717768224), attempt 1. Signature: `fulfillment(of: [completed], timeout: 3)` expired. - **Cause:** the busy test-host main thread that #8678 handled for other classes. The cancel reply comes from UIKit's dismissal completion, after the dismiss animation runs on that thread. - **Change:** raise the wait to 10 s, matching #8678's `callbackTimeout`. The assertion is unchanged. Dropping `resolve` from the dismissal completion still fails it, and so does resolving before `alert.dismiss`. ### Related issue None found. **Conflicts with #8568** ("Adapt iPhone Duo layouts, rail navigation, and native sheets"). #8568 rewrites the fade test into a loop but keeps the 60 ms sleep and the mid-fade samples. It already conflicts with `main`, and this PR adds `RunnerTests.swift` to its conflicts. #8665 only adds a voice-note test. ### Testing Not compiled against the app or run locally, because Xcode and RunnerTests bog this machine down. `swiftc -parse` accepts the file. The new assertions, copied into a stub `XCTestCase`, typecheck with `swiftc -typecheck` against the iOS simulator SDK and XCTest (Swift 5 mode). The full file needs Flutter and the Buzz module, so it wasn't typechecked as a whole. **The Mobile (iOS) lane is the check.** Pushed with `git push --no-verify`, because the pre-push suites fail when the laptop is under load. ### Readiness An agent review of the combined `flakey-tests` branch (b432453af) found no blockers. `buzz-review-completed` is not added, because the human test step (AGENTS.md "Before opening a PR", step 3) hasn't happened yet. ### Siblings - #8739 ci: Hermit fetch deadline and sccache fallback - #8740 test(relay): thread_window aux-page hold and `api::media` Postgres tests - #8742 fix(desktop): deep-linked message hold (product change) - #8743 test(desktop): four specs racing scroll settles and focus - #8744 test(desktop): five settle classes and remote-owned-mentions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Matt Toohey <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]>
|
Hi @klopez4212, I noticed this PR had merge conflicts with the latest I've resolved all conflicts on a dedicated branch here: Summary of Conflict Reconciliations:
You can rebase or cherry-pick git fetch origin fix/mobile-default-agent-voice-8665
git merge origin/fix/mobile-default-agent-voice-8665 |
b77ad12 to
9f53118
Compare
GitHub Actions
Error
github-actions
Bot
removed
the
codex-security-review-current
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f53118351
ℹ️ 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".
| if (!await folder.exists()) return []; | ||
| final recipients = <String>{}; | ||
| await for (final entry in folder.list()) { | ||
| if (entry is! File || !entry.path.endsWith('.json')) continue; |
There was a problem hiding this comment.
If the process is killed or the manifest write/rename fails after save() has written the .identity sidecar and copied the .m4a, this scan ignores both surviving files because it only considers .json entries. After restart the recording has no Retry or Discard affordance and remains permanently stranded; enumerate orphan sidecars/audio files or make every persisted prefix discoverable.
AGENTS.md reference: AGENTS.md:L256-L261
Useful? React with 👍 / 👎.
| final descriptor = await uploadService.uploadVoiceNote( | ||
| recording.file, | ||
| duration: recording.duration, | ||
| ); |
There was a problem hiding this comment.
🤖 P2 · A stalled upload hides the saved-note recovery actions
This new caller supplies neither a cancellation token nor a deadline to uploadVoiceNote. Its HTTP send and response-body wait are unbounded, so a server that keeps the connection open without completing the upload response leaves send() in sending indefinitely. That state hides Retry and Discard, and the navigation overlay also hides Search/New. The recording is saved, but recovery requires remounting the launcher or restarting the app instead of using its recovery controls.
Test: hold the upload response open after receiving the request and verify that bounded failure or cancellation makes the saved recording's Retry/Discard actions reachable. Source review only; I did not run this stalled-server scenario.
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <[email protected]>
9f53118 to
9f0d07a
Compare
GitHub Actions
Error
klopez4212
changed the title
GitHub Actions
Error
| if (!context.mounted || !destinationIsCurrent()) { | ||
| throw StateError('The community changed.'); | ||
| } | ||
| await session.publish(previousEvent); |
There was a problem hiding this comment.
🤖 P1 · Saved voice notes become permanently unretryable after 15 minutes
After a failed publication or lost acknowledgement, this branch republishes the original signed event. The relay rejects timestamps outside its 900-second window before ordinary message deduplication (guard). Retrying after a longer outage or next-day restart therefore always fails, including when the original event was accepted.
User effect: the retained voice note cannot complete recovery. A regression can retain a failed signed note, advance past 15 minutes, and retry with both absent and previously accepted originals.
| channel: channel, | ||
| content: descriptor.toMarkdownImage(), | ||
| mentionPubkeys: [pubkey], | ||
| mediaTags: [descriptor.toImetaTag()], |
There was a problem hiding this comment.
🤖 P2 · Membership changes can send private audio to an expanded DM
Destination selection checks the cached two-person roster before upload, but SendMessage fetches current DM recipients. An existing participant can add another member through kind9000 during upload: that path permits member additions to DMs. The sender then includes the new participant without checking the roster against the captured account-and-Bestie pair.
User effect: audio recorded for Bestie is sent into the expanded conversation before its audience is shown. A regression can pause upload, add a participant to the cached DM, then finish upload and inspect the signed recipients.
| ], | ||
| child: NavigationButtonSurface( | ||
| child: DefaultAgentVoiceControl( | ||
| key: ValueKey((scope, pubkey)), |
There was a problem hiding this comment.
🤖 P2 · Reconnecting can discard the recording in progress
Reconnect invalidates the Bestie preference. During its loading state this launcher substitutes a null pubkey, changing this key even when the account, community and selected Bestie stay the same. The old control unmounts and its cleanup cancels the recorder.
User effect: a transient reconnect while holding Bestie silently loses the ongoing recording. A regression can reconnect during recording while delaying the preference refresh, then release the control.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
🤖 :bot: Jude’s code review agent
Verdict: REQUEST CHANGES. Reviewed PR base bd1b17dee8348b21b33abe35ee4eafa8730f8a83 through clean head 9f0d07ad42b6289f53ed309057a15bb50136823a; also checked mergeability against fetched current main 44b3c4823f0cf1f7715f85dbf5566cb64e848162. This is a high-risk mobile change: durable voice drafts, async send/retry, Android/iOS media packaging, and VoiceOver recovery. The feature direction fits VISION_MOBILE.md's no-data-loss and accessibility contracts; these four findings prevent that contract being met.
Author-actionable findings
- Current-main merge blocker.
git merge-tree --write-tree origin/main HEADexits 1 with content conflicts inmobile/ios/Runner/AppDelegate.swift,mobile/lib/features/home/home_page.dart, andmobile/lib/features/home/messages_filters.dart; GitHub reportsDIRTY. The PR cannot land as-is. Author action: rebase/merge current main, resolve all three while preserving both sides' behavior, and rerun affected Flutter/iOS gates. Verification owner: author for resolution and gates; reviewer for fresh-head merge check. - Android codec fallback fails on capable devices.
mobile/android/app/src/main/kotlin/xyz/block/buzz/mobile/AndroidVoiceNotePackager.kt:88–105selects the first AVC encoder supporting the target dimensions, then checks its color formats. If that encoder is surface-only and a later encoder supports YUV420, packaging throws rather than using the valid candidate. This remains open in the existing inline discussion. Author action: select/iterate encoders by both dimensions and supported YUV420 format; test the selection seam with surface-only first and usable second. Verification owner: author for regression and representative Android runtime; reviewer for patch. - Late send can delete a newer saved draft.
mobile/lib/features/channels/default_agent_voice_control.dart:82–97awaitsonRecorded(retained)and calls unconditionaldraft.clear()afterward. If the control unmounts and a new one saves a different note under the same identity before the old send completes, the old completion deletes the replacement. The relay publish ACK is bounded to 8 seconds, not unbounded; the race fits inside that window, and the new caller's media-upload phase can wait longer. See prior discussion. Author action: clear only if the persisted draft still matches this send's generation/event, with a held-send/remount/replacement regression at the production storage seam. Verification owner: author for test; reviewer for source and race check. - Recovery gives VoiceOver an impossible instruction.
mobile/lib/features/channels/default_agent_voice_control.dart:302–320advertises “Hold to record… Release to send. Slide up to cancel” whenever enabled, even while a saved note or restore failure exposes Retry/Discard andstart()refuses to record. Author action: make the hint/value state-specific and add a widget semantics regression proving recovery advertises usable actions without the idle hold hint. Verification owner: author for test; reviewer for accessibility recheck.
Validation and scope: At clean 9f0d07a, git diff --check 133c883b…HEAD passed, and git merge-tree --write-tree origin/main HEAD failed with the three conflicts above. Focused Flutter voice-control and launcher widget tests passed (10 tests) at that head. A full local flutter test attempt reached 1,942 tests and timed out at 120 seconds after five failures in notification/settings tests; it was not completed, and those failures have not been established as PR-caused. GitHub's exact-head Mobile, Android, Swift Native/Release and Mobile Checks lanes report pass. “Run Codex Security Review” shows failure in the rollup, but its job conclusion is cancelled after 45 minutes; this is not an established code failure.
Manual/native evidence and residual risk: No exact-head physical iOS VoiceOver or Android encoder/playback journey was observed in this review. The iOS packaging test checks track presence/duration, not decoded playback. These are verification gaps, not additional author defects; reviewer/device and release validation remain outstanding. Re-review the exact new head after fixes, including the merge resolution and all four regressions.
Fence persisted voice drafts by generation across remounted sends, expose state-specific recovery semantics, and select a compatible Android encoder. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <[email protected]>
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <[email protected]>
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <[email protected]>
GitHub Actions
Error
|
🤖 Addressed the four required changes in
Validation: clean analysis; 4,694 Flutter tests (71 skipped), 4 unconfigured-push tests, 27 crypto-package tests, 34 Android unit tests, and 107 iOS native tests pass. iOS and physical-device tests ran at production commit — Terp |
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <[email protected]>
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04457e7332
ℹ️ 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".
| child: DefaultAgentVoiceControl( | ||
| key: ValueKey((scope, pubkey)), | ||
| draftIdentity: draftIdentity, |
There was a problem hiding this comment.
If the relay reconnects during a hold, defaultAgentProvider invalidates itself (shared/me/default_agent.dart:43–44). While that refresh is loading, pubkey becomes null, changing this key even though the account, community, and selected agent have not changed. Flutter then disposes the recording control, whose cleanup calls cancel(), silently discarding the captured audio without sending it or creating a recoverable draft. Keep the current recording's recipient and widget identity stable during a same-scope preference refresh; loading can disable new recordings without destroying an active one.
Useful? React with 👍 / 👎.
github-actions
Bot
added
the
codex-security-review-current
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <[email protected]>
github-actions
Bot
removed
the
codex-security-review-current
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd9ad2ff09
ℹ️ 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".
| if (!context.mounted || !destinationIsCurrent()) { | ||
| return; | ||
| } | ||
| unawaited( | ||
| Navigator.of(context).push<void>( |
There was a problem hiding this comment.
If the user releases a recording and opens another conversation while upload or relay acknowledgement is pending, Home remains mounted, so this check passes and the completed send pushes Bestie's conversation over the screen the user just selected. The ordinary tap path in bestie_conversation.dart explicitly checks ModalRoute.of(context)?.isCurrent, but the new voice path bypasses that protection. Check that the originating route is still current before navigating, while still allowing successful delivery and draft cleanup to complete.
Useful? React with 👍 / 👎.
github-actions
Bot
added
the
codex-security-review-current
… layouts Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <[email protected]>
github-actions
Bot
removed
the
codex-security-review-current
GitHub Actions
Error
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d7c6e4776
ℹ️ 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".
| useEffect(() { | ||
| return () { | ||
| WidgetsBinding.instance.addPostFrameCallback((_) { | ||
| recordingCallback?.call(false); | ||
| }); |
There was a problem hiding this comment.
During a real hold on Home, reporting true rebuilds HomePage, which creates a new inline onRecordingChanged callback (home_page.dart:338). Because this effect depends on that callback, each rebuild schedules its cleanup to report false, even though recording is still active. The preceding effect also schedules true, so successive frames repeatedly toggle the parent state and rebuild Home, while the stale cleanup removes the recording blur and semantics exclusion. Stabilize the callback or make this reset run only on actual disposal, rather than on callback replacement.
AGENTS.md reference: AGENTS.md:L231-L234
Useful? React with 👍 / 👎.
brow
left a comment
brow
left a comment
There was a problem hiding this comment.
🤖 Tom's pre-review of this PR recommends rework: it regresses Home in three ways. (1) An undelivered voice note replaces Home's Search and New buttons: a stalled upload shows only a spinner with no timeout or cancel, and after a failure they return only when Retry succeeds or the recording is deleted, even with hold-to-talk switched off. (2) A reconnect while holding cancels the recording silently: the Bestie preference reloads on every reconnect, its loading state maps to a null pubkey, and the voice control keyed on it is rebuilt, whose dispose calls cancel(). (3) If the scan for saved voice notes fails (this PR is the app's first use of Application Support, and path_provider_foundation creates the directory inside the lookup, so a full device is enough), the Bestie button becomes a "Retry saved voice notes" button for everyone, switch on or off. Please keep Home's navigation and the existing Bestie tap independent of voice-note state, and keep an in-progress recording alive across preference refreshes. Full pre-review: buzz://message?channel=e71f9a2a-ac46-473d-a24e-5e16fd8cb7b1&id=f594e28f385b6a069655505b038fa1bc94523b8252715cae116aa8324ca1bd82
GitHub Actions
Error
Hold Bestie to record a voice note, release to send, or slide up to cancel. Holding does nothing when Bestie is unset or the new Settings → Voice notes → Hold to talk to Bestie switch is off. The setting defaults on and is saved on the device; normal taps keep their existing behavior.
Review fixes at
74b4c86e0: merged current main, select Android encoders by size and YUV420 support, fence draft cleanup/event writes by persisted generation, and expose state-specific VoiceOver recovery instructions.Validation: analysis, all 4,694 Flutter tests (71 skipped), supporting package tests, 34 Android unit tests, and 107 iOS native tests pass. iOS and physical-device tests ran at production commit
24c1145d1; subsequent commits only change Flutter semantics and test cleanup; native code is unchanged. The new regressions fail when their fixes are removed. Voice packaging passes on a physical Pixel 10; the device suite passes 4/5 tests, with an unrelated PNG chunk-order assertion failure in unchanged image-processing code. Physical VoiceOver and decoded playback/listening were not exercised. The updated preview has not been installed on the iPhone.These earlier Flutter widget previews use synthetic audio and omit native iOS glass and full-screen blur. No image files are committed.