Add mobile voice notes - #7121
Conversation
Signed-off-by: kenny lopez <klopez4212@gmail.com>
🔐 Codex Security Review
|
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. |
![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
Here are some automated review suggestions for this pull request.
Reviewed commit: 3862546116
ℹ️ 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: kenny lopez <klopez4212@gmail.com>
…ce-notes 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: 1573c5a008
ℹ️ 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".
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — A Team verdict at exact head 1573c5a0085de69fbcbac7cdf5be39286b9deea3: REQUEST CHANGES.
Blocking defects
-
Recorder startup can outlive dismissal and activate after teardown.
DeviceVoiceNoteRecorder.start()awaits permission, temp-directory lookup, and native start without checking a cancellation/disposal generation after those suspension points (mobile/lib/features/channels/voice_note_recording.dart:64-99,121-134). The recorder surface starts cancellation/disposal unawaited during teardown (mobile/lib/features/channels/voice_note_composer_recorder.dart:48-85). If the user discards or navigates while permission/start is pending, the stale continuation can call into a disposed recorder or briefly activate the microphone after the UI disappeared.Author action: add an immediate cancellation/disposal fence checked after every startup await and before amplitude monitoring, serialize startup versus teardown, and add a deterministic delayed-permission/start regression proving unmount/discard cannot leave native recording active or leak an async error.
Verification owner: author for implementation/tests; A Team re-review for the repaired exact head.
-
The waveform exposes slider semantics but cannot be adjusted with VoiceOver. The semantics node sets
slider: trueand a percentage value, but provides noonIncrease/onDecrease; seeking is available only through tap/drag gestures (mobile/lib/features/channels/voice_note_waveform.dart:74-104). A VoiceOver user can focus the control but cannot operate it with standard adjustable actions.Author action: implement bounded semantic increment/decrement actions through the same seek path, including updated announced values, and test the semantics actions at 0%, an interior position, and 100%.
Verification owner: author for implementation/tests; A Team re-review for the repaired exact head.
-
Multiple mobile voice notes can play simultaneously. Each mounted attachment owns an independent
DeviceVoiceNotePlayerController/just_audioplayer (mobile/lib/features/channels/voice_note_attachment.dart:44-65;mobile/lib/features/channels/voice_note_recording.dart:186-295), andtoggle()starts it without app-scoped arbitration. Starting B therefore does not pause A, including duplicate-URL and composer-preview/timeline combinations.Author action: add mobile-wide playback arbitration with safe ownership release on completion/disposal, plus causal regressions for two distinct instances, duplicate URLs, and composer preview versus timeline playback.
Verification owner: author for implementation/tests; A Team systems lane for focused tests,
flutter analyze, and the full mobile suite on the repaired exact head.
Exact-head evidence
At 1573c5a0085de69fbcbac7cdf5be39286b9deea3, with the working tree rechecked clean:
just mobile-check: PASS — Dart format checked 544 files with 0 changes; Flutter analyze reported no issues.- Full
cd mobile && flutter test: PASS — 2,026 tests. - The changed-head feature delta hides Voice note on Android and retains it on iOS; the PR remains the 32-file iOS/mobile voice-note diff against the live base.
- Source tracing found owned temporary output cleanup, cancellation-generation checks outside the startup race, retry restoration, and a shared canonical
video/mp4voice-note contract consumed by Mobile and Desktop. - Completed exact-head CI observed by the team was green; some Mobile/Desktop jobs were still pending at the last lane poll. CI owns their final freshness, but green completion cannot cure the defects above.
Confidence gaps (not additional defects)
Physical-device microphone denial, AVAudioSession interruption/route changes, real relay decode, haptics, VoiceOver end-to-end behavior, Liquid Glass/fallback, large-text/narrow layouts, and sustained many-message resources were not independently observed. Author action: none solely for this review verdict beyond fixing the concrete defects above. Verification owner: release/manual QA and reviewer tooling; CI owns pending jobs.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — REQUEST CHANGES at 1573c5a0085de69fbcbac7cdf5be39286b9deea3 (base bc006f67087b049e2f9c4d8a2f26faceff628225).
The exact-head mobile gates pass (format, analyze, and all 2,026 Flutter tests), and the iOS MP4/upload contract is compatible with existing Desktop voice notes. Three user-visible defects remain:
-
[P2] Recorder startup is not fenced against teardown (
mobile/lib/features/channels/voice_note_recording.dart:64-99,121-134; teardown invoice_note_composer_recorder.dart:48-85).start()awaits permission, temp-directory lookup, and native start without checking cancellation/disposal after those awaits, while unmount launches cancel/dispose asynchronously. If permission resolves after discard/navigation, stale startup can activate the microphone or call a disposed recorder after the UI is gone. Fence each startup continuation against cancellation/disposal, serialize teardown, and add a deterministic delayed-permission/start regression proving unmount prevents recording and emits no unhandled error. -
[P2] The VoiceOver slider cannot be adjusted (
mobile/lib/features/channels/voice_note_waveform.dart:74-104). The waveform exposesslider: trueand a percentage, but only pointer tap/drag changes position; there are no semantic increase/decrease actions. Implement boundedonIncrease/onDecreaseactions through the same seek path and test semantics at the beginning, middle, and end. -
[P2] Multiple voice-note cards can play simultaneously (
mobile/lib/features/channels/voice_note_attachment.dart:44-65;mobile/lib/features/channels/voice_note_recording.dart:186-295). Every card owns an independent player, and starting one never pauses the previously active instance. This also affects duplicate URLs and composer-preview versus timeline instances, diverging from Desktop’s cross-instance arbitration. Add app-scoped playback arbitration with safe completion/disposal ownership release, plus causal regressions for two distinct instances, duplicate URLs, and preview-versus-timeline playback.
Verification: rerun the causal regressions, flutter analyze, and the full mobile suite on the repaired immutable head. Physical-device permission/interruption, real haptics, VoiceOver, relay decode, layout, and sustained many-message behavior remain release confidence gaps rather than additional blockers.
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: 3ab606ec9f
ℹ️ 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".
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — REQUEST CHANGES at exact head 3ab606ec9fcee6bbc8f3f60fb8b8294b495788bb (base bc006f67087b049e2f9c4d8a2f26faceff628225).
This head adds Android H.264/AAC MP4 packaging but leaves the previously reviewed recorder, waveform, and playback implementations byte-identical. Four P2 defects remain:
-
Recorder startup can still outlive teardown (
mobile/lib/features/channels/voice_note_recording.dart:65-99,121-134;mobile/lib/features/channels/voice_note_composer_recorder.dart:64-85).start()crosses permission, temporary-directory, and native-start awaits without a cancellation/disposal fence, while unmount startscancel()→dispose()asynchronously. A stale continuation can activate the microphone or call the disposed recorder after the UI vanishes.Author action: serialize startup with teardown; check an immediate cancellation/disposal generation after every startup await and before amplitude monitoring. Add a deterministic delayed-permission/native-start regression proving discard/unmount leaves no active recorder and emits no unhandled async error.
Verification owner: author for implementation/tests; reviewer for repaired-head tracing and exact-head gates.
-
Stop is enabled before startup completes (
mobile/lib/features/channels/voice_note_composer_recorder.dart:33-45,64-76,166-173). The stop button can callrecorder.stop()whilerecorder.start()is still pending, producing a false failure while stale startup may continue afterward.Author action: track explicit started state, disable stop until startup settles, and test immediate-stop/delayed-start ordering.
Verification owner: author for implementation/tests; reviewer for repaired-head lifecycle verification.
-
The waveform is exposed as a slider but remains inoperable with VoiceOver/TalkBack adjustable actions (
mobile/lib/features/channels/voice_note_waveform.dart:74-104). It suppliesslider: trueand a percentage but noonIncrease/onDecrease; seeking remains pointer-only.Author action: add bounded semantic increment/decrement actions through the existing seek path, announce the resulting value, and test semantics at 0%, an interior value, and 100%.
Verification owner: author for implementation/tests; reviewer/tooling for semantic action verification.
-
Multiple voice-note instances can still play simultaneously (
mobile/lib/features/channels/voice_note_attachment.dart:44-65,137-144;mobile/lib/features/channels/voice_note_recording.dart:191-233,281-289). Every attachment owns a private player andtoggle()starts it without app-scoped arbitration. Distinct cards, duplicate URLs, and composer preview versus timeline playback can overlap.Author action: add mobile-wide playback arbitration; starting B must pause A, with safe ownership release on completion/disposal. Add causal regressions for distinct instances, duplicate URLs, and preview-versus-timeline playback.
Verification owner: author for implementation/tests; reviewer for repaired-head arbitration tracing and full mobile gate.
Exact-head validation
At clean 3ab606ec9fcee6bbc8f3f60fb8b8294b495788bb:
just mobile-check: PASS — 544 files formatted with zero changes; Flutter analyze found no issues.just mobile-test: PASS — all 2,026 tests.- GitHub exact-head
Mobile(including Android debug APK),Mobile Swift, DCO, Semgrep, and zizmor checks: PASS. 1573c5a… → 3ab606e…: one commit/five files; the recorder, waveform, attachment/player implementations are unchanged.
The Android packaging path is internally coherent in the searched source: it validates AAC, emits H.264/AAC MP4, fast-start rewrites it, uploads canonical video/mp4 metadata, releases extractor/codec/muxer resources, and cleans intermediate/returned package files (AndroidVoiceNotePackager.kt:24-52,59-80,84-194,204-243; media_upload/helpers.dart:76-103; media_upload.dart:465-503). No additional source-proven leak was found in those paths.
Residual confidence gaps
Physical-device permission denial/interruption and microphone teardown, Android codec/mux/fast-start playback through a real relay, haptics, VoiceOver/TalkBack operation, layout/keyboard boundaries, and sustained many-card resources were not independently observed. The PR body reports Pixel menu visibility and an installed iPhone build, but provides no inspectable native journey/semantic receipt. Author action: none solely for these evidence gaps. Verification owner: native reviewer tooling and release/manual QA. These gaps do not supersede the concrete defects above.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Request changes at 3ab606ec9fcee6bbc8f3f60fb8b8294b495788bb
Reviewed against bc006f67087b049e2f9c4d8a2f26faceff628225. Seven author-actionable defects remain. Existing review findings below were independently checked, not treated as proof.
1. [P1] Preserve the uploaded MP4's authoritative duration
mobile/lib/shared/relay/media_upload.dart:175–189, 487–494 replaces the server descriptor's duration with the recorder's wall-clock duration. That clock is sampled only after native stop/finalization (voice_note_recording.dart:102–116), so it is not the packaged media duration. crates/buzz-relay/src/handlers/imeta.rs:270–275 rejects a difference greater than 0.1 seconds, and normal event ingestion invokes that validation (handlers/ingest.rs:2973–2984).
A remote Dart probe of the unchanged descriptor functions produced: server duration 3.0, outgoing duration 3.25, outgoing tag duration 3.25. That necessarily fails the relay comparison. Thus a recording whose shutdown/package duration differs sufficiently uploads successfully but cannot be sent; retry retains the same incorrect duration. Preserve the server duration, and add a regression whose upload response deliberately differs from the recording timer. This independently confirms the existing Codex report; the probe is not a live-relay/native reproduction.
2. [P2] Keep voice notes out of the trailing image carousel
The new audio classification in mobile/lib/features/channels/message_media.dart:105–120 changes an existing consumer's behavior: _extractTrailingImageGallery in message_content/media_carousel.dart:69–95 excludes only MessageMediaKind.video. Two trailing voice notes, or a photo plus voice note, therefore become image-gallery entries. MessageContent removes them from the markdown body before _buildMedia can select the voice player (message_content.dart:185–189, 304–320), and the carousel uses MediaImageProvider/the image viewer.
Remote execution of the unchanged classifier/extractor returned gallery_count=2 with kinds [audio, audio] and [image, audio]; a single voice note correctly returned no gallery. These are ordinary composer attachment combinations, and the received note has no playback controls. Exclude audio from gallery extraction and add widget regressions for both combinations without regressing photo-only galleries.
3. [P2] Fence recorder startup against cancellation and disposal
mobile/lib/features/channels/voice_note_recording.dart:65–99 crosses permission, temporary-directory, and native-start awaits without checking whether cancellation/disposal was requested. Unmount launches cancel then dispose asynchronously (voice_note_composer_recorder.dart:64–85). Delay permission/directory resolution, discard or pop, then resolve it: the stale continuation can still invoke native start and install amplitude monitoring after teardown intent. The _finished flag is never consulted by start().
Serialize startup/teardown and fence each startup continuation; test delayed permission, directory lookup, and native start against discard/unmount. This confirms the prior review's recorder-ownership finding. Native microphone behavior was not exercised on Linux.
4. [P2] Disable Stop until startup succeeds
mobile/lib/features/channels/voice_note_composer_recorder.dart:166–172 enables Stop immediately, while recorder.start() is still pending at lines 64–76. An immediate tap enters stop() (voice_note_recording.dart:102–113), sets _finished, and can fail before a recording path exists; later startup can still complete, leaving an unrecoverable attempt. Track successful startup before enabling Stop and test immediate Stop with delayed startup. This is the separate user action from finding 3, although the same lifecycle repair can address both.
5. [P2] Retain visible ownership when navigation covers a recorder
Recorder cleanup only runs on unmount (voice_note_composer_recorder.dart:48–85). Start recording in a channel, then tap a message: channel_detail_page/message_bubble.dart:120–133 pushes a MaterialPageRoute, retaining the underlying channel/composer rather than disposing it. Its recorder and timer remain active behind the new thread, which has another independent composer (thread_detail_page.dart:938–960). The original microphone controls are no longer visible.
Stop/finalize/cancel when the recording route becomes covered, or provide persistent visible ownership and single-recorder coordination. Add a navigation regression asserting that the hidden composer no longer owns an active recording. Actual simultaneous native capture is not claimed.
6. [P2] Make the waveform's slider semantics adjustable
mobile/lib/features/channels/voice_note_waveform.dart:83–104 advertises a slider and percentage but supplies no semantic increase/decrease actions. All seeking is pointer-only. VoiceOver/TalkBack users cannot perform the standard adjustable interaction. Wire bounded semantic actions through the seek path with updated values, and test beginning/middle/end positions. This confirms the existing accessibility finding and the repository's WCAG requirement.
7. [P2] Coordinate playback across voice-note instances
Each card creates its own player (mobile/lib/features/channels/voice_note_attachment.dart:44–65); DeviceVoiceNotePlayerController.toggle() starts it without pausing another instance (voice_note_recording.dart:282–289). Starting a second card leaves the first playing, including duplicate-URL cards and composer preview versus timeline playback. Add instance-based playback ownership and regressions for those combinations, including safe release on completion/disposal. This confirms the prior playback-arbitration finding.
Verification, dispositions, and exit criteria
- All repository inspection and the integrated Dart probes ran on the authorized Linux Blox worktree. HEAD and clean status were checked before/after the probe;
git diff --checkpassed. No repository files were modified and no CI-equivalent suites were rerun. Existing exact-head GitHub Mobile, Mobile Swift, Desktop Core, desktop smoke/integration, and desktop build checks were observed successful. Earlier reviewers report the full 2,026-test mobile pass; that is their evidence, not a run performed here. - The old missing-Android-packaging report is fixed:
MainActivity.kt:110dispatches the implemented packager. An independent proposed native buffer defect was rejected on type inspection:blackYuv420Frame()returnsByteArray, so repeatedput(byte[])copies the full frame. No native packaging blocker was established. Non-finiteimeta.durationcan throw in the client, but supported relay ingress rejects it; absent a reachable bypass, that is hardening, not a merge blocker. The AppDelegate size-policy concern is separate from this code/product verdict. - Authenticated iOS playback remains a credible, unverified risk: voice notes disable the header proxy (
voice_note_recording.dart:191–194, 251–258), whereas the existing video viewer deliberately downloads an authenticated local copy to avoid AVPlayer dropping later range-request headers (media_viewer_page/video_viewer.dart:51–55). This Linux review cannot establish the reported 403-on-seek. Native permission/interruption, codec/mux/decode, haptics, accessibility end-to-end, and layout also remain device-confidence limits, not additional asserted defects. Desktop at this head provides generic MP4 playback; dedicated desktop voice notes are in the still-open counterpart #6978.
The author owns the seven repairs and causal regressions. Re-review should target those repaired contracts and the exact resulting head, backed by normal package-wide CI. Native QA owns device confidence. No unrelated refactor or speculative hardening is required by this verdict.
Signed-off-by: Princess Donut <b238ea756dee4d98afa5883fc7f1de61eeabe65bf700e3a5a5a80db5e42e2c2b@buzz.block.builderlab.xyz>
|
Addressed Wes’s and Jude’s requested changes in
Validation at exact head
I replied to and resolved the implemented inline threads. I left the older |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 845c528d8d
ℹ️ 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".
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Review verdict: Request changes
Reviewed exact head 845c528d8d96121d863b62ac2f31a3b89a0fb4f3 against base bc006f67087b049e2f9c4d8a2f26faceff628225.
Findings
-
[P2] Stop can race route teardown into concurrent native terminal operations.
finish()awaitsrecorder.stop()while route/unmount cleanup independently runscancel()thendispose()(mobile/lib/features/channels/voice_note_composer_recorder.dart:55-67,102-109).DeviceVoiceNoteRecorder.stop()marks_finished = truebefore awaiting backendstop()(mobile/lib/features/channels/voice_note_recording.dart:179-187). Teardown'scancel()consequently returns immediately (:203-205), after whichdispose()can invoke backendcancel()anddispose()while backendstop()remains unresolved (:220-238). A deterministic temporarystart → blocked stop → disposeregression failed at this head because backend cancellation occurred before stop settled. This can lose/fail the preview or misuse the native recorder when a user taps Stop and immediately navigates.Author action: serialize terminal ownership with a shared terminal future/state. Teardown must await an in-flight stop, and backend disposal must occur only after exactly one native terminal operation settles. Add deterministic stop-pending + navigation/disposal regressions asserting no overlapping backend calls, no asynchronous error, and a defined preview/discard outcome.
-
[P2] Active recording has no app-lifecycle fence.
The composer handles route coverage but does not observe app lifecycle transitions (
mobile/lib/features/channels/voice_note_composer_recorder.dart:70-110). Locking/backgrounding can therefore leave microphone capture active while the Dart duration timer is suspended. In the searched platform configuration, Android declares recording permission but no foreground recording service, and iOS declares no audio background mode. This risks prolonged capture, OS-forced recording loss, and misleading elapsed/stop behavior after resume.Author action: deterministically stop or cancel on non-foreground lifecycle states, with causal coverage for paused/inactive/hidden transitions and truthful recovery UI.
Disposition of prior concerns
The changed head materially repairs the prior source-level blockers: bounded adjustable waveform actions, Stop disabled until startup completes, route-coverage cancellation, image-only carousel extraction, and provider-scoped playback arbitration across local preview and remote cards. Upload/auth/metadata tracing also remains coherent: packaged MP4 bytes use the normal SHA-bound Nostr upload path, server-measured duration wins, and relay processing validates MP4/auth before emitting measured duration.
AppDelegate.swift remains large (1,246 lines, +280 from base), but the configured differential file-size policy applies to mobile/lib/*.dart; the exact-base file-size gate passes. I found no repository rule making the Swift 1,000-line split a required gate, so this is maintainability debt rather than an additional blocker.
Exact-head evidence
- PASS —
just mobile-check(545 Dart files; analyze clean) - PASS — full
just mobile-test(2,034 tests; clean tree afterward) - PASS — differential mobile file-size check against
bc006f67087b049e2f9c4d8a2f26faceff628225 - PASS —
git diff --check bc006f67087b049e2f9c4d8a2f26faceff628225..HEAD - PASS — live/local head agreement and exact-head GitHub checks, including Mobile, Mobile Swift, DCO, Semgrep, and zizmor
- FAIL — temporary blocking-stop teardown regression, exposing finding 1; temporary test removed and tree restored
- NOT RUN — physical/simulator microphone interruption and rapid-navigation behavior, VoiceOver/TalkBack, packaged relay playback, and sustained many-card resource observation. These are release-verification gaps, not extra author defects.
Verification owner: author for fixes and causal tests; code review on the repaired exact head; native/release QA for device lifecycle, accessibility, and packaged playback behavior.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested at 845c528d8d96121d863b62ac2f31a3b89a0fb4f3
Corrective source review against exact base bc006f67087b049e2f9c4d8a2f26faceff628225, including the delta from previously reviewed head 3ab606ec9fcee6bbc8f3f60fb8b8294b495788bb.
[P1] Configure the loopback transport before enabling the audio-header proxy
mobile/lib/features/channels/voice_note_recording.dart:320–324 changes AudioPlayer(useProxyForRequestHeaders: false) to AudioPlayer(). The pinned just_audio 0.10.6 defaults this option to true: remote sources with headers are rewritten to http://127.0.0.1:<port>/.... Every remote voice card passes a headers map (voice_note_attachment.dart:49–59, voice_note_recording.dart:383–389), including an empty map when no auth is needed; the dependency tests headers != null, not whether it is empty.
The Android app manifest (mobile/android/app/src/main/AndroidManifest.xml:18–21) provides neither a cleartext allowance nor a network-security config permitting loopback HTTP. The checked-in debug/profile manifests add only INTERNET permission. Consequently the normal Android network policy blocks the new HTTP proxy path even when the original media URL is HTTPS. Loading a received voice note reaches _load’s error state and disables Play as “Voice note unavailable”; local composer preview bypasses the proxy, so preview-only validation misses it.
Reproduction path: install a release Android build with the shipped network policy, open a channel containing a voice note, and load/play the received card. The native player is handed cleartext loopback instead of the HTTPS relay URL. This is a source-established transport/configuration mismatch, not a device run performed here. The pinned dependency’s platform configuration explicitly requires non-HTTPS support for its header proxy (archive README lines 113, 239–265; lib/just_audio.dart:2748–2753,2488–2492).
Smallest safe fix: explicitly retain the direct-header transport on Android, or configure a narrowly scoped loopback exception if the proxy is intentional. Do not enable arbitrary cleartext network traffic globally just to support localhost. Also reconcile the iOS proxy configuration with the dependency’s platform requirements; native iOS range/auth behavior remains a device-validation limit, not a second asserted defect. Validate received-note playback, not just local preview, under release-equivalent network policy.
Prior findings and scope
The seven prior repairs are supported by source: server duration survives into imeta; audio is excluded from image galleries; startup continuations are fenced; Stop waits for startup; route cover relinquishes recording; waveform semantics expose bounded adjustments; and preview/timeline players share instance-based arbitration. The root ProviderScope and both real player consumers were checked, not inferred from coordinator-only tests.
The compatibility contract remains canonical H.264/AAC MP4 with relay-authoritative metadata and accessible mobile controls. Native packaging is unchanged by the correction; the previously dispositioned packaging concerns are not reopened. Desktop-specific voice-note UI remains counterpart scope. The new blocker above is introduced by this correction, not unrelated hardening.
Validation: exact source/guidance, producer/consumer paths, corrective regression-test sources, and the lockfile-matched just_audio archive were inspected. No checkout, build, test execution, CI query, or device run was performed in this automation. Added playback tests use a fake controller and do not exercise the native transport. The Stop/route-cover overlap was assessed but not promoted to a blocker: cover intentionally discards the note, and no additional native failure was established. Normal package-wide validation and native QA remain separate requirements.
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz> Signed-off-by: Kenny Lopez <klopez4212@gmail.com>
klopez4212
added
bug
|
Addressed the remaining Wes/Jude findings in
Validation at exact head
The workspace build reached native linking after |
12f975b to
845c528
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12f975b21d
ℹ️ 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".
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested
Reviewed head 12f975b21d0b9328da541f1b4b2bfb6f92191eea against exact base 59328d5ae38a51a618dd2fddd7faf1343d42096f, focusing on the seven-file corrective delta from 845c528d8d96121d863b62ac2f31a3b89a0fb4f3. The Android proxy defect is fixed, but three actionable issues remain in the correction.
1. [P1] Restore the canonicalizer binding after extracting the Swift packager
VoiceNotePackager.swift:248 still invokes Self.neutralizeSampleDependencyBoxes. Self now denotes VoiceNotePackager, which has no such member. The implementation remains a private static member of AppDelegate (:899–967); ordinary video transcoding still calls it there at :788. The new packager is included in Runner's Sources phase (project.pbxproj:589), so this prevents iOS Runner compilation, rather than merely failing an export at runtime.
Fix: bind both callers to an accessible canonicalizer, preserving the MP4 canonicalization step. Qualifying the call as AppDelegate alone does not fix private access. Validate with an iOS Runner build. This is a source-established missing-member error, not a compiler run performed here.
2. [P2] Do not discard pending recording startup when its permission dialog takes focus
The new effect treats every non-resumed state as cancellation (voice_note_composer_recorder.dart:51–56). Startup is still awaiting microphone permission at voice_note_recording.dart:140–142. Android system dialogs can produce Flutter's inactive state while the app remains visible. If the microphone prompt does so, this effect calls onCancel, the parent removes the recorder (compose_bar/voice_note.dart:79–83), and teardown invalidates startup (voice_note_composer_recorder.dart:111–118). Granting permission then returns to _ensureStartupActive, which rejects the cancelled attempt instead of starting recording.
Reproduction: with microphone permission not yet granted, select Voice note and allow the system prompt. On a prompt-induced inactive transition the first attempt is silently discarded; the user must reopen Voice note.
Fix: distinguish transient permission/startup focus loss from actual backgrounding. Preserve the pending permission flow while still cancelling on genuine capture loss. Add a delayed-permission regression that delivers inactive/resumed before permission resolves and confirms the approved attempt can proceed. The new tests change lifecycle only after their fake recorder has already started.
3. [P2] Cancel on background transitions without requiring another rendered frame
The same effect is driven by ref.watch(appLifecycleProvider) and then schedules addPostFrameCallback. The actual lifecycle listener only updates provider state (app_lifecycle_provider.dart:35–48); it does not cancel recording. Flutter stops begin/draw frame delivery while paused.
A rapid inactive → hidden → paused sequence before the next build can therefore leave the widget unable to observe the non-resumed state or run its cancellation callback. When frames resume, provider state may already be resumed, so the recorder was never cancelled or disposed during the background interval. Native capture can continue or be interrupted independently while the UI/timer retain stale recording state. The added pumpAndSettle tests force frames and do not exercise this sequence.
Fix: invoke the recorder's terminal ownership directly from a lifecycle listener/subscription that does not depend on widget rendering, then reconcile UI state safely. Add a regression delivering background transitions without pumping a frame and verify cancellation starts before resume, including pending startup/stop cases.
The platform contracts behind findings 2–3 are documented in Flutter's AppLifecycleState reference: system dialogs can cause Android inactive, and paused suppresses frame callbacks. These are source/lifecycle traces, not device reproductions.
Repairs, scope, and validation
- Android playback:
AudioPlayer(useProxyForRequestHeaders: false)removes the previously introduced localhost HTTP path. Authenticated iOS playback now downloads through the shared HTTP client and hands the native player a local file; Android and header-free URLs retain direct loading. Native received-note playback and download/disposal behavior still need platform validation; fake-player tests do not establish that integration. - Recorder terminal ownership: stop now installs a shared terminal future; cancel/dispose await it before backend release. The delayed-stop test binds the production recorder and checks non-overlap. This repairs the reviewed stop/dispose race at the source level.
- Compatibility: canonical H.264/AAC MP4, relay-authoritative duration, image-only galleries, accessible waveform controls, and shared preview/timeline arbitration remain the agreed contract. Unchanged prior repairs, Android packaging, ordinary UI, and desktop counterpart scope were not reopened. The Swift move otherwise preserves the packaging operations; the missing helper binding is the extraction-specific defect.
Validation: immutable source/tree-blob verification, exact-base AGENTS/TESTING/product/architecture guidance, and completed independent recorder-lifecycle and Swift-extraction reviews, integrated and checked by Carl. No checkout, build, tests, PR-code execution, CI query, or device run was performed. The three findings above are the consolidated exit criteria; no unrelated hardening or process-metadata blocker is added.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — REQUEST CHANGES at exact head 12f975b21d0b9328da541f1b4b2bfb6f92191eea (base 59328d5ae38a51a618dd2fddd7faf1343d42096f).
Blocking defects
-
[P1] The extracted iOS voice-note packager does not compile.
VoiceNotePackagercallsSelf.neutralizeSampleDependencyBoxes(at:)after export (mobile/ios/Runner/VoiceNotePackager.swift:243-259, call at line 248), but that type ends at line 272 without defining the member. The only implementation and its endian/error helpers in the searchedmobile/iosSwift sources remainprivate staticmembers ofAppDelegate(mobile/ios/Runner/AppDelegate.swift:899-967), so the new type can neither resolve nor access them. The extraction moved the caller and left its dependency behind.Author action: move the canonicalization helper set into
VoiceNotePackageror an intentionally shared accessible utility, then compile the Runner target. Add or repair an exact-head CI gate that compiles changed Runner Swift; the greenMobile Swiftjob only builtBuzzPushKit, not this target.Verification owner: author for the repair; iOS build/release CI for Runner compilation; code review for the repaired delta.
-
[P2] First-use microphone permission can cancel the recording attempt as if the app backgrounded. Recorder startup first awaits
record.hasPermission()(mobile/lib/features/channels/voice_note_recording.dart:140-145), which requests permission by default. The new lifecycle effect invokesonCancel()for every state other thanresumed, includinginactive(mobile/lib/features/channels/voice_note_composer_recorder.dart:42,51-56), and its regression explicitly requires cancellation forinactive(mobile/test/features/channels/compose_bar_test.dart:4843-4876). That conflicts with Buzz’s shared lifecycle policy, which treatsinactiveas a brief transition requiring no pause action (mobile/lib/shared/relay/app_lifecycle_provider.dart:35-48). Pinned Flutter 3.41.7 documentsinactiveas mobile input-focus loss while a system dialog is visible. The permission sheet can therefore unmount/cancel the recorder while permission is pending; the startup-generation fence then rejects the granted continuation (voice_note_recording.dart:119-147,217-241). The user must reopen Voice note after granting access.Author action: do not treat transient
inactiveas definitive backgrounding. Preserve first-use permission startup while still cancelling on true background/lock states according to the app lifecycle contract. Add a deterministicstart pending → inactive → permission granted/resumedregression proving recording remains mounted and starts, while retaining causal true-background cancellation coverage.Verification owner: author for policy and regression; code review for lifecycle tracing; native QA for first-install permission-sheet behavior on iOS and Android.
Repairs verified in source
The previous stop/dispose race is repaired: disposal awaits the shared terminal operation before backend release (voice_note_recording.dart:183-270) and the delayed-stop regression passes. Startup generation fences remain across each await. Source/tests also support image-carousel exclusion, bounded adjustable waveform semantics, route-cover cancellation, cross-instance playback arbitration, relay-authoritative duration, Android direct playback, and owned iOS authenticated-download cleanup. No additional author-actionable defect was established in those reviewed paths.
Exact-head evidence
At clean detached head 12f975b21d0b9328da541f1b4b2bfb6f92191eea:
- PASS —
just mobile-check: 545 Dart files format-clean; Flutter analyze clean. - PASS — full
just mobile-test: all 2,038 tests (independent product/UI lane). - PASS — focused recorder/composer/media/upload suites: 230 tests; independent systems lane passed 147 tests; integrator reran recorder + full compose-bar files, 111 tests.
- PASS —
git diff --check 59328d5a...HEAD. - PASS — mutation excluding
inactivemade the current inactive-cancellation test fail at its protected behavior, proving the test enshrines the harmful policy; source restored and tree rechecked clean. - Exact-head GitHub
MobileandMobile Swiftwere green when inspected, butMobile Swiftlogs show onlymobile/ios/BuzzPushKitbuilds/tests. A local Runner build did not reach Swift compilation because CocoaPods xcfilelist paths were unavailable, which is a tooling confidence gap rather than the basis for finding 1.
Confidence gaps
No inspectable exact-head native journey/receipt independently exercised first-install permission UI, lock/background transitions, AVAudioSession interruption/route changes, authenticated relay seek, Android codec/mux/fast-start round trip, packaged cross-client playback, haptics, VoiceOver/TalkBack, extreme layout, crash/restart temp scavenging, or sustained many-card resources. Author action: none solely for these gaps beyond the two concrete repairs above. Verification owner: native/release QA and the applicable exact-head CI/release gates.
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: 59328d5ae38a51a618dd2fddd7faf1343d42096f..12f975b21d0b9328da541f1b4b2bfb6f92191eea (exact head 12f975b21d0b9328da541f1b4b2bfb6f92191eea)
Risk: high — native microphone lifecycle, platform media packaging, authenticated playback, temp-file ownership, asynchronous teardown, and accessibility.
Behavior/contracts traced: recorder start/stop/cancel/dispose; route/app lifecycle and first-use permission; preview/send recovery; playback arbitration/authenticated iOS download/temp cleanup; iOS Runner packaging extraction; relay MP4/auth/duration compatibility; waveform/error/loading UI.
Blocking findings
[P1] Extracted iOS packager does not compile
mobile/ios/Runner/VoiceNotePackager.swift:248 calls Self.neutralizeSampleDependencyBoxes(at:), but that type has no such member. In searched mobile/ios/Runner/*.swift, the only implementation and helper functions remain private static members of AppDelegate at mobile/ios/Runner/AppDelegate.swift:899-967; they are neither VoiceNotePackager members nor accessible across the type boundary. The extraction moved the caller and stranded its helper.
The green Mobile Swift job does not compile this target: .github/workflows/ci.yml:1036-1049 only builds/tests mobile/ios/BuzzPushKit, not Runner or VoiceNotePackager.swift.
Author action: move the MP4-box canonicalization helpers into VoiceNotePackager or an accessible shared utility; compile the iOS Runner target; add/route a required gate that compiles new Runner Swift files.
Verification owner: author; iOS Runner build/release CI; reviewer exact-head retrace.
[P1] First-use microphone permission can cancel the recording attempt
mobile/lib/features/channels/voice_note_composer_recorder.dart:51-56 cancels for every state except resumed, including inactive; the new regression requires cancellation for inactive (mobile/test/features/channels/compose_bar_test.dart:4843-4876). Buzz's shared lifecycle contract explicitly classifies inactive and hidden as brief transitions requiring no pause action (mobile/lib/shared/relay/app_lifecycle_provider.dart:35-48). Native permission/system overlays can produce inactive while record.hasPermission() is pending (voice_note_recording.dart:140-145), so cleanup unmounts the recorder and fences startup as cancelled even after permission is granted.
The product lane mutation-proved the checked-in test protects this harmful behavior: excluding inactive made its cancellation assertion fail.
Author action: do not treat transient inactive as backgrounding during permission/startup. Fence actual background states (paused/detached, with an explicit hidden policy). Add start pending → inactive → permission granted → resumed coverage proving startup survives, while retaining true-background/lock cancellation tests.
Verification owner: author; reviewer exact-head tests; native QA on iOS/Android first-install permission UI.
Repaired and cleared
The prior stop/navigation race is repaired: stop, cancel, and dispose share _terminalOperation; disposal awaits in-flight stop; the deterministic blocked-stop test proves no backend overlap. Both lanes found no further author-actionable defect in route-cover cancellation, preview/remove/send restoration, playback arbitration, waveform semantics/scrubbing/speed, reduced-motion/loading/error UI, upload/auth/duration flow, or carousel exclusion.
Validation:
just mobile-check: PASS — 545 files, analyze clean.- Full
just mobile-test: PASS — 2,038/2,038 in Carl's clean exact-head checkout; product lane independently matched. - Product focused voice-note/composer/media/upload: 230 passed; systems focused run: 147 passed.
git diff --check 59328d5a...HEAD: PASS.- Local/live exact head and clean state were verified after tests; live head and actor (
jedwards27) versus author (klopez4212) were refreshed immediately before submission. - Hosted Mobile, Mobile Swift (BuzzPushKit only), Unit, relay/backend/integration E2E, builds, lint, Security, DCO, Semgrep, and zizmor were green; remaining running Desktop/Windows jobs are CI-owned confidence gaps.
Manual/native evidence: no independent exact-head device/simulator permission, recording, packaged playback, accessibility, haptics, interruption, or route-change receipt.
Residual risk: native lock/background transitions, permission UI, AVAudioSession interruption, authenticated playback, VoiceOver/TalkBack, extreme layout, and resource behavior remain native/release-QA ownership. These confidence gaps are not additional author defects.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88e3cdf2c2
ℹ️ 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".
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Reviewed exact head 88e3cdf2c2a66a708f4f9188f496216bed50187e against base 1c8321cd08feb597f8bcff5195c21148fb3e98ed.
Blocking findings
-
[P1] A second toggle during coordinator activation still allows the authenticated GET to start (
mobile/lib/features/channels/voice_note_recording.dart:669-676,699-715).toggle()records_toggleCancellationRequested, but_toggle()never observes it. If activation is awaiting the previous owner'spause(), the second toggle runs before_downloadAbortexists; activation then resumes and starts the remote request. A temporary production-controller regression reproduced this deterministically: hold prior-owner pause → toggle → toggle again → release pause yielded anAbortableStreamedRequest GETwhen zero requests were expected.Author action: fence the operation with a generation/token checked after coordinator activation and before request/source work. Commit a deterministic regression asserting no GET/load/play/temp residue and that a third toggle retries successfully with fresh auth.
Verification owner: author for fix and regression; reviewer for exact-head rerun.
-
[P2] A second toggle while
setFilePathis pending still allows playback to start (mobile/lib/features/channels/voice_note_recording.dart:615-619,642-665,714-734).The download promotes the temporary file and clears
_pendingRemotebefore returning the pendingsetFilePathfuture. The second toggle callspause(), but neither the post-load path nor the final_play()gate checks cancellation. A temporary production-controller regression reproducedplayCount == 1after GET completed,setFilePathwas held, and a second toggle requested cancellation.Author action: fence every post-await transition, especially after source loading and before
_play(), with the same operation token. Commit the exact interleaving and assert a subsequent toggle recovers.Verification owner: author for fix and regression; reviewer for exact-head rerun.
-
[P2 accessibility] The enabled loading control announces the wrong action and has two semantics owners (
mobile/lib/features/channels/voice_note_attachment.dart:134-161).While loading, activating the control cancels the operation, but the
IconButtontooltip remains “Play voice note” and the child spinner separately announces “Loading voice note.” Assistive-technology users are not told the available cancellation action and receive competing semantics.Author action: expose one semantics owner named “Cancel voice note loading,” exclude decorative spinner semantics, and add a semantics-action test that invokes that named control and proves cancellation.
Verification owner: author for fix/test; reviewer for semantics rerun; native release QA for VoiceOver/TalkBack.
Validation
just mobile-check: passed; 545 files formatted, 0 changed; analyzer clean.just mobile-test: passed; 2,061 tests.- Two temporary production-controller regressions failed causally as described above; test-only edits were reverted.
git diff --check 1c8321cd...88e3cdf2: passed.- Local and live PR heads were rechecked as
88e3cdf2c2a66a708f4f9188f496216bed50187e; review tree was clean.
Confidence gaps — not additional author action
Real-device authenticated cancellation, native source-load timing, OS-interruption temp cleanup, audio-route lifecycle, and VoiceOver/TalkBack were not exercised. Native/release QA owns these after the code races are repaired. Some exact-head CI jobs remained in progress at the final snapshot; CI owns completion.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested at head 88e3cdf2c2a66a708f4f9188f496216bed50187e, base 1c8321cd08feb597f8bcff5195c21148fb3e98ed.
The previous duplicate-operation defect is repaired: overlapping toggles share an operation. HTTP-failure retry now works with request-time authorization, including deferred Android loading. The remaining cancellation and recovery defects are below; no MP4-format or product redesign is requested.
P2: Cancellation must invalidate the whole pending playback operation
toggle() sets _toggleCancellationRequested and calls pause(), but the operation never reads that cancellation flag. pause() aborts only an already-created request and does not relinquish coordinator ownership or change the source generation.
Two source-reproducible windows share this defect:
- Hold the previous player's
pause(), invoke the new remote player'stoggle()twice, then release the previous pause. The second toggle sees no_downloadAbort; activation resumes and starts the authenticated GET anyway. Cancellation can therefore download and play the note the user just canceled. - Let the GET complete, hold
setFilePath, invoke the second toggle, then complete source loading. The file has already been promoted and_pendingRemotecleared; the post-load and final play gates still allow playback. Android's awaitedsetUrlhas the same unchecked post-load transition.just_audio0.10.6pause()does nothing when not playing; it does not cancel a pending preload.
Smallest safe repair: fence each playback attempt before the first await, invalidate it on cancellation/pause, and check it after coordinator activation and source loading, before committing source ownership or playing. Preserve single-flight settlement and retry. Add concrete-controller regressions for both windows, asserting no post-cancel GET/load/play as applicable and successful subsequent retry. The committed second-toggle test waits until the GET exists; the rapid-toggle test adds a separate later pause(), masking the first window. This consolidates the two cancellation windows reported in review 5093358843; severity is P2 for canceled work/unwanted playback, not an established auth-boundary violation.
P2: Keep a recoverable source until native loading succeeds
On a fresh iOS card, _pendingRemote is cleared before setFilePath succeeds. If source loading throws, _load() sets hasError without setting _hasPlayableSource. The widget offers Retry, but line 705 immediately returns: no remote descriptor and no playable source remain. Even if the native failure clears, the mounted card cannot recover.
Reproduction/exit: successful GET → fail the first backend setFilePath → let the next load succeed → invoke the same mounted card's Retry. Retain a retryable descriptor or explicitly reload the retained file, rather than losing both recovery routes before load success. Bind a controller regression to this sequence and verify the UI reaches it. The existing 503 regression proves only failure before the descriptor is cleared. This is the remaining native-load branch of the earlier recovery finding, not a request for unlimited automatic retry.
P2: The loading action must announce cancellation, not playback
The enabled loading button invokes toggle() to cancel, but its tooltip is still “Play voice note”; the child spinner separately supplies “Loading voice note.” A screen-reader user is not told that activating this control cancels the transfer. Expose one actionable semantics owner named “Cancel voice note loading,” with decorative spinner semantics excluded. Add a semantics-action regression that invokes that named control and verifies cancellation. The current loading test asserts the spinner label, and the repeated-tap test only counts fake-controller toggles.
P2: Delete the generated Android package when fast-start rewriting fails
After native packaging succeeds, _packagePickedVoiceNoteForUpload() owns a generated source MP4. If rewriteMp4ForFastStart fails, its catch deletes only the destination and rethrows. uploadVoiceNote has not assigned packagedPath yet, so its finally cannot remove the generated source either. The original recorded .m4a is a different file. Retrying packaging creates another UUID-named MP4, leaving each failed attempt's generated file in cache.
Reproduction/exit: make the native channel return a generated MP4, then force the real rewrite to reject malformed bytes or fail its destination write. Assert that both generated package and partial rewrite are removed, while the original recording remains available for retry. Perform cleanup at the helper's ownership boundary and preserve the original error. The current upload test injects an already-packaged path, bypassing this failure seam. This confirms new inline evidence 3917098361; no inevitable permanent OS-level leak is claimed, but application-owned cleanup is missing and low-storage retries can worsen it.
Scope and validation
Corrective review of the 13 changed feature files since 9257c1bb20ef2dedc913f371a5fbc3e8b1bf7ef7, with current production controller, coordinator, UI, auth service and upload ownership traced. Contract remains canonical H.264/AAC MP4, relay-authoritative upload duration, image-only galleries, shared preview/timeline playback, fresh scoped authorization and draft isolation. Unchanged wire-format/parser/gallery/desktop surfaces were not broadly re-audited.
Independent UI/draft and native-packaging lanes completed and were integrated before publication. Draft identity now resets pending/active recording, new-recording revision isolation remains intact, and finalized recordings dropped after unmount are deleted. Android encoder and iOS envelope-writer deadlines have coherent source cleanup; the new iOS test checks only the timeout constant, not behavior. The unchanged later iOS export lacks a separate deadline, but no concrete stalled export is established here; inline 3917098354 is deferred hardening, not another blocker.
Exact-base AGENTS, TESTING, VISION, VISION_SOVEREIGN and relevant architecture read; 58 cached exact-head/base blobs hash-verified against immutable Git trees. The just_audio 0.10.6 source was verified against the exact-head lockfile archive hash. Source/metadata only: no checkout, build, tests, CI query or PR-code execution. Review 5093358843 reports executable cancellation reproductions; I independently traced the source and dependency behavior, not those test runs. Native-device, real-relay recovery and VoiceOver/TalkBack behavior remain unobserved in this automation.
Co-authored-by: Carl <3c4caeafb646d23867f1c4832e68211d77e2561946171625f75c3ce1a3f2670f@buzz.block.builderlab.xyz> Signed-off-by: Kenny Lopez <klopez4212@gmail.com>
|
@jedwards27 @wesbillman I addressed the latest cancellation, recoverability, accessibility, Android cleanup, and iOS export-deadline findings in |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a377d34e91
ℹ️ 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".
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Reviewed exact head a377d34e915a90ccc27eee71f7a47511ae5c164c against base 1c8321cd08feb597f8bcff5195c21148fb3e98ed.
Blocking finding
[P2] A timed-out iOS export can leave a late output file behind (mobile/ios/Runner/VoiceNotePackager.swift:266-319).
The timeout path calls asynchronous cancelExport(), immediately removes outputURL, and marks the operation completed. When AVFoundation's completion callback later reaches complete, the guard !completed return skips the entire callback body—including the only post-cancellation output cleanup. If the exporter creates or writes the destination after timeout-side deletion while cancellation settles, that file is left in the temporary directory. Flutter completion correctly remains one-shot, but cleanup ownership incorrectly shares that one-shot guard. The checked-in Swift coverage only asserts the two timeout constants (mobile/ios/RunnerTests/RunnerTests.swift:11-14); it does not exercise late completion or cleanup.
Author action: separate idempotent late output cleanup from one-shot Flutter result delivery (or await cancellation completion under a separately bounded cleanup path), and add a deterministic native seam/regression for timeout wins → cancellation completion arrives later → result exactly once and output/video temp files absent.
Verification owner: author for the seam/fix/regression; reviewer for exact-head Swift/native rerun.
Prior blockers verified repaired
- Playback startup now uses an operation generation fenced after coordinator activation, after download/source load, and before playback; pause invalidates the generation and releases ownership (
mobile/lib/features/channels/voice_note_recording.dart:553-658,670-710,714-808). - Checked-in causal tests cover both prior cancellation interleavings, absence of request/play/temp residue, and successful third-toggle recovery with fresh auth (
mobile/test/features/channels/voice_note_recording_test.dart:435-634). Mutation probes recreated both old failures. - Loading exposes one actionable “Cancel voice note loading” semantics owner with excluded child semantics and a semantics-action regression (
mobile/lib/features/channels/voice_note_attachment.dart:114-187;mobile/test/features/channels/message_content_test.dart:1012-1068). - Android fast-start failure removes source and destination artifacts with regression coverage (
mobile/lib/features/media_upload/helpers.dart:76-103;mobile/test/features/media_upload_test.dart:1223-1290).
Validation
just mobile-check: passed; 545 files formatted, 0 changed; analyzer clean.- Full
just mobile-test: passed; 2,065 tests. - Focused controller/message-content tests: 98 passed; focused controller/media-upload suites: 55 passed.
- Three product-path mutation probes failed causally as intended; production files were restored.
git diff --check 1c8321cd...a377d34e: passed.- Local and live PR heads were independently checked as
a377d34e915a90ccc27eee71f7a47511ae5c164c; lane trees were clean.
Confidence gaps — not additional author action
Real-device authenticated network cancellation, actual AVFoundation timeout timing, native audio interruption/routes, Android codec failure cleanup, and VoiceOver/TalkBack observation were not exercised. Native/release QA owns those after the deterministic iOS cleanup race is repaired. Exact-head CI was still partly in progress at the final snapshot; completed visible gates had no failures, and CI/integrator owns completion.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested at head a377d34e915a90ccc27eee71f7a47511ae5c164c, base 1c8321cd08feb597f8bcff5195c21148fb3e98ed.
The prior pending-playback cancellation, first-native-load retry, and Android generated-package cleanup defects are repaired in source. Two bounded issues remain; no format or composer redesign is requested.
P2: Separate late export cleanup from one-shot result delivery
In VoiceNotePackager.swift:268–291, the timeout requests cancelExport(), sets completed, removes the output, and returns a Flutter error. The eventual export callback passes through the same guard !completed, so it skips the entire body, including post-export output cleanup.
Cancellation is not a join with the exporter: AVFoundation's AVAssetExportSession.h documents asynchronous terminal callbacks after cancellation and does not promise that output creation has ceased before cancelExport() returns. In the ordering deadline wins → immediate deletion → exporter creates/recreates its destination while cancellation settles → terminal callback, that late file has no remaining application cleanup owner. Repeated failed attempts can accumulate temporary MP4s. This independently confirms the ownership defect in review 5094996130; it is not a claim that every timeout leaks or an executed native reproduction.
Exit: retain one-shot Flutter delivery, but perform idempotent cleanup of abandoned output at terminal export completion even when timeout already won. Preserve the successful output handed to Dart. Add a deterministic timeout-wins/late-completion regression asserting one result and no abandoned output/envelope. The current XCTest only checks constants, not this ordering.
P2: Advertise Cancel only for a genuinely cancellable loading phase
The changed shared control gives every isLoading state an enabled “Cancel voice note loading” action. That is correct for an active remote toggle, but not local preview initialization:
- Mount a local preview and hold backend
setFilePath. The mount callsloadLocal, which directly sets loading through_load, without creating_toggleOperation. - Activate the advertised Cancel action.
toggle()has no active operation to cancel and enters_toggle, which immediately returns because loading is true. No pause, abort or load invalidation occurs.
The enabled control therefore promises cancellation but does nothing. isLoading also includes backend buffering, so it is not itself a cancellability predicate. Exit: expose the actual cancellable phase and offer Cancel only there; show an honest non-action loading state otherwise, or implement the cancellation being advertised. No requirement to make every native load cancellable. Add a mounted production-controller regression with held local initialization, while retaining remote cancellation coverage. The new semantics-action test only counts a fake controller's toggles and cannot catch this mismatch.
Repairs, contract and validation
Playback-operation generations now fence coordinator activation, source loading and play; pause invalidates the attempt and releases ownership. Remote descriptors survive first native-load failure for freshly authenticated retry. Android rewrite failure removes both generated files while retaining the recording. The remote loading semantics owner is corrected, but its applicability remains too broad as above.
Corrective review of eight changed feature files since 88e3cdf2c2a66a708f4f9188f496216bed50187e at the same base, with controller/coordinator/UI/auth/upload ownership traced. Accepted contract remains canonical H.264/AAC MP4, relay-authoritative duration, image-only galleries, shared preview/timeline playback, request-time scoped auth, one playback owner and isolated drafts. Unchanged composer/parser/gallery/desktop surfaces retain the prior review disposition, not a new broad audit.
Both independent lanes returned and were integrated. Exact-base product/guidance and relevant architecture were checked; 86 cached source/guidance blobs hash-verified against pinned trees. Source/metadata only: no checkout, build, tests, CI query or PR-code execution. Earlier reviewers' executed checks are not my test results. Device timing, real-relay behavior and VoiceOver/TalkBack remain unobserved here; missing execution is not a separate blocker.
Co-authored-by: Carl <3c4caeafb646d23867f1c4832e68211d77e2561946171625f75c3ce1a3f2670f@buzz.block.builderlab.xyz> Signed-off-by: Kenny Lopez <klopez4212@gmail.com>
|
@wesbillman @jedwards27 — please re-review head This head addresses the two blockers from the reviews on
It also fixes the two cross-client Codex findings: Mobile emits packaged voice notes as canonical ordinary links, and Mobile’s anchor renderer recognizes Desktop-created packaged voice-note links through imeta. Both Codex threads have been answered and resolved. Validation on the pushed head: pre-push file-size checks passed and the full Mobile suite passed (2,069 tests). Fresh CI is currently running with no failures reported so far. — Carl, commenting on Kenny’s behalf |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a07df69c4
ℹ️ 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".
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Reviewed: 1c8321cd08feb597f8bcff5195c21148fb3e98ed..9a07df69c48df56345f830690300268ee5fe3733 (exact live head rechecked before submission)
Risk: high — this adds native iOS/Android recording and packaging, asynchronous cancellation/recovery, temporary-file ownership, remote playback, and user-visible accessibility behavior.
Behavior/contracts traced: Flutter recorder/player state and operation generations; Android just_audio preload behavior; iOS AVFoundation timeout/late completion; native/Dart packaging and cleanup ownership; MP4/imeta rendering contract; loading/error/retry/accessibility journeys.
Blocking findings
P1 — Android's enabled Cancel action does not interrupt a pending native URL load
Remote loading exposes Cancel and routes the second tap through toggle() → pause() (mobile/lib/features/channels/voice_note_attachment.dart:117-163; mobile/lib/features/channels/voice_note_recording.dart:725-743,787-820). But Android is awaiting _player.setUrl(...) before playback begins, while the production backend's pause() delegates to just_audio (voice_note_recording.dart:415-418), whose pinned 0.10.6 implementation returns immediately when playing == false. Generation invalidation prevents later stale playback, but it neither interrupts nor settles the native preload.
A temporary production-controller regression modeled this exact pre-play pause no-op: the first toggle held setUrl, the second invoked Cancel, and the URL load remained pending instead of settling. The probe failed as expected and was removed.
Consequence: on a slow or wedged Android load, the advertised Cancel control does nothing observable; loading and native/network resource ownership can remain stuck until the plugin settles.
Author action: introduce a production-proven backend primitive that actually interrupts/settles pending setUrl, call it from cancellation, and add a deterministic controller regression proving pending preload + Cancel settles, clears loading, never plays late, and permits retry with fresh auth. If Android preload cannot safely be interrupted, do not advertise canCancelLoading there.
Verification owner: author for code/regression; reviewer for causal mutation and exact-head gates; Android/native QA for slow-network/TalkBack observation.
P1 — unrelated state updates erase the active cancellation capability
VoiceNotePlaybackState.copyWith defaults canCancelLoading to false and always stores it (mobile/lib/features/channels/voice_note_recording.dart:293-306). During cancellable remote loading _load sets it true (:676-688), but position/duration listeners perform unrelated partial copies (:447-455) and silently reset it. The attachment keys its only Cancel action directly on this bit (mobile/lib/features/channels/voice_note_attachment.dart:117-128).
A temporary exact-head state test constructed {isLoading: true, canCancelLoading: true}, applied copyWith(position: 1s), and failed expected true / actual false; it was then removed and the tree restored clean.
Consequence: backend events can turn a live cancellable operation into a non-actionable “Loading voice note” state.
Author action: preserve canCancelLoading when omitted and clear it explicitly only at terminal/non-cancellable transitions. Add a production-controller regression with a pending remote load plus position and duration emissions, proving Cancel remains exposed and aborts the operation.
Verification owner: author for patch/test; reviewer for mutation and exact-head mobile gates.
P2 — new public Dart API is undocumented
Repository policy requires doc comments on new public API (AGENTS.md:147-150). The exact-head additions include undocumented public voice-note widgets, recorder/player interfaces and implementations, coordinator/state, waveform and formatter/normalizer functions, MessageMediaKind, and PackageVoiceNoteForUpload.
Author action: document intended public API or make implementation-only declarations library-private.
Verification owner: author + reviewer policy audit.
Verified repaired/intact
- iOS timeout handling now separates one-shot Flutter delivery from terminal cleanup; timeout removes current files and late completion removes recreated abandoned output while success preserves output (
VoiceNotePackager.swift:267-315,326-364;RunnerTests.swift:16-81). - Operation generations fence coordinator activation, download/native load, source replacement, pause, and pre-play; stale downloads are deleted in
finally(voice_note_recording.dart:545-664,676-820). - Android and Dart package-failure paths remove owned output/envelope while preserving the original recording; upload cleanup owns the final packaged path (
AndroidVoiceNotePackager.kt:35-51;media_upload/helpers.dart:76-104;media_upload.dart:464-503). - The canonical
video/mp4/voice-note-*.mp4imeta-duration contract remains intact (media_upload.dart:219-230;message_content.dart:273-376). - Scrub/speed semantics, retry state, recording lifecycle cancellation, narrow/text-scale coverage, and reduced motion were present in source/tests.
Validation
At exact clean head 9a07df69c48df56345f830690300268ee5fe3733:
just mobile-check— PASS; analyzer clean, 545 files, no formatting changes.just mobile-test— PASS; 2,069 tests.just file-size-check— PASS.git diff --check 1c8321cd...HEAD— PASS.- Temporary Android cancellation probe — FAIL causally: pending URL load did not settle; removed afterward.
- Temporary
copyWithinvariant probe — FAIL causally: expected true, actual false; removed afterward. - CI snapshot: Mobile Swift and DCO passed; no completed red check. Clients/Mobile and unrelated lanes were still pending.
Manual/native evidence and residual risk
No physical iOS/Android recording, codec failure, audio-route interruption, authenticated slow-network cancellation, or packaged-media roundtrip was observed. The checked-in RunnerTests target reported device-only SUPPORTED_PLATFORMS=iphoneos, so local simulator XCTest execution was unavailable. These are confidence gaps owned by native/release QA after the concrete defects above are repaired, not additional author defects.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested at head 9a07df69c48df56345f830690300268ee5fe3733, base 1c8321cd08feb597f8bcff5195c21148fb3e98ed.
Both prior requested repairs are present: iOS late export cleanup is separated from one-shot Flutter delivery, and held local-preview initialization has an honest non-action loading state with a production-controller widget regression. One new, bounded P2 remains.
P2: Preserve cancellation capability across unrelated state updates
VoiceNotePlaybackState.copyWith defaults canCancelLoading to false and always stores it. Consequently, the existing position/duration listeners remove the capability despite leaving the pending operation and isLoading unchanged. The new UI gate then disables both pointer and accessibility cancellation.
A concrete production-path sequence, without relying on arbitrary native timing:
- Mount an iOS remote card with nonzero duration and let initial idle events settle. Tap Play and hold its authenticated HTTP GET. The controller is loading/cancellable, but the audio backend remains idle until download completion.
- Tap/drag the still-enabled waveform to a nonzero position. The pinned
just_audio0.10.6seek()publishes the changed position while idle; its position stream emits it, whileplayerStateStream.distinct()suppresses the unchanged(false, idle)pair (lib/just_audio.dart:353–356,752–759,1308–1332,4165–4171). - Buzz's position-only copy resets the flag. The card now says “Loading voice note” with no Cancel action, even though
_toggleOperationand the abortable GET remain active. There need not be another backend-state event to restore it before the download finishes or times out.
Exit: preserve the capability when omitted from copyWith, and clear it deliberately when leaving a cancellable phase. Add a mounted production-controller regression holding a remote download, delivering unrelated position/duration updates (including the waveform route), and asserting Cancel remains actionable and aborts the request without later playback. Retain the held-local-preview non-action regression. This repairs capability retention; it does not require redesigning native cancellation.
Scope and dispositions
- This independently confirms the capability-loss finding in review 5098751561, at P2 for loss of an in-progress user control. The current local-load test uses empty backend streams and cannot cover this transition.
- Android
pause()not physically interrupting pendingsetUrlwas already known and unchanged when the prior review accepted generation-fenced cancellation. I am not reopening that accepted contract as a new blocker. Public-API documentation is a non-blocking policy follow-up, not an additional code/product defect here. - Corrective review covers the nine changed files since
a377d34e915a90ccc27eee71f7a47511ae5c164cat the same base. Canonical H.264/AAC MP4, relay-authoritative duration, filename/imeta playback recognition, request-time scoped auth, single-owner playback and isolated drafts remain the contract. Unchanged recorder, Android packaging, gallery and draft paths retain their previous disposition rather than receiving a new broad audit.
Exact-base guidance/product documents and relevant architecture were checked; independent native and UI/state lanes were integrated. The delegated transport lane was explicitly cancelled after Carl completed its bounded trace; no lane remains pending. Source/metadata only: no checkout, build, tests, CI query or PR-code execution. The reproduction above is a source trace, not an executed device test; other reviewers' reported executions are not my results.
Co-authored-by: Carl <3c4caeafb646d23867f1c4832e68211d77e2561946171625f75c3ce1a3f2670f@buzz.block.builderlab.xyz> Signed-off-by: Kenny Lopez <klopez4212@gmail.com>
|
@wesbillman @jedwards27 — please re-review head |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4db279c5c8
ℹ️ 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".
| _state.hasError) { | ||
| return; | ||
| } | ||
| if (identical(_pendingRemote, remote)) _pendingRemote = null; |
There was a problem hiding this comment.
On Android, after setUrl succeeds this clears _pendingRemote, so a later playback failure can only retry _player.play() with the original header map. If a relay-hosted note is paused until its 600-second authorization expires and then seeks outside ExoPlayer's retained buffer, the new range request fails, and the displayed Retry action repeatedly reuses the expired authorization instead of invoking the header factory. Fresh evidence after the claimed fix is that the current successful-load path still discards the only data needed to rebuild the source with fresh authentication.
AGENTS.md reference: AGENTS.md:L211-L216
Useful? React with 👍 / 👎.
| return Semantics( | ||
| label: 'Voice note waveform', | ||
| slider: onSeek != null, | ||
| value: onSeek == null ? null : '${(progress * 100).round()} percent', |
There was a problem hiding this comment.
When VoiceNoteWaveform is used by the active recorder, onSeek is null, but this still publishes a labeled semantics node. Screen-reader users therefore encounter a separate “Voice note waveform” stop that has no value or available action; exclude the waveform from semantics when it is decorative, while retaining the slider semantics for playable attachments.
AGENTS.md reference: AGENTS.md:L218-L226
Useful? React with 👍 / 👎.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Reviewed: 1c8321cd08feb597f8bcff5195c21148fb3e98ed..4db279c5c83b6ff6aa33cdc1828554bfaaa352ad (exact live head rechecked before submission)
Risk: high — native iOS/Android recording and packaging, asynchronous cancellation/recovery, temporary-file ownership, remote playback, upload/render interoperability, and accessible mobile controls.
Behavior/contracts traced: Flutter recorder/player state and operation generations; Android just_audio pre-play cancellation; iOS timeout/late completion; native/Dart packaging and cleanup; MP4/imeta upload and Desktop/mobile recognition; loading/error/retry, playback, layout, and accessibility semantics.
Findings
No unresolved author-actionable defect at this head. The three prior blockers are repaired.
- Android pre-play Cancel: cancellable loads now call backend
cancelPendingLoad()→ productionAudioPlayer.stop()rather than ineffectivepause()(mobile/lib/features/channels/voice_note_recording.dart:715-788,807-877). Pinnedjust_audio0.10.6 invalidates the activation and interrupts the pending load. The regression holdssetUrl, cancels, verifies interruption/zero play/settled state, then retries with fresh auth (mobile/test/features/channels/voice_note_recording_test.dart:932-981). Mutationstop → pausecaused that test to time out and fail. - Stable cancellation capability:
copyWithpreserves omittedcanCancelLoading(voice_note_recording.dart:310-324), so position/duration events at:475-483do not disable Cancel; terminal paths clear it explicitly. The held-load regression emits both events before Cancel and retains capability. Restoring the old default-false behavior made the assertion fail. - Public API policy: new public voice-note/media state, controller, backend, coordinator, providers, widgets, helpers, and
PackageVoiceNoteForUploadnow have declaration-level Dartdocs (voice_note_recording.dart:15-78,291-457,910-916;message_media.dart:3-115;voice_note_attachment.dart:17;voice_note_composer_recorder.dart:23;voice_note_play_pause_icon.dart:6;voice_note_waveform.dart:22;media_upload.dart:77-78,177-193).
Contract verification
- Async generations fence activation, download/native load, replacement, cancellation, and late completion; stale downloads are removed in
finally(voice_note_recording.dart:577-697,708-877). - Android package failure, Dart fast-start failure, and upload cleanup retain bounded ownership of temporary source/output/envelope files while preserving the original recording (
AndroidVoiceNotePackager.kt:22-51;media_upload/helpers.dart:76-104;media_upload.dart:467-506). - Upload/render interoperability remains
video/mp4+voice-note-*.mp4+ duration metadata, matching mobile and Desktop recognition (media_upload.dart:210-233,467-506,793-808;message_media.dart:111-126;desktop/src/features/messages/lib/audioAttachment.ts:21-33). - Source/widget review found loading/error/retry and playback controls truthful, with no new accessibility or cross-platform source defect.
Author action: none required.
Verification owner: native/release QA owns the residual hardware journeys below; optional production-adapter mutation strengthening may be owned by author/reviewer but is not merge-blocking.
Validation at exact clean head
just mobile-check— PASS; analyzer clean, 545 files unchanged.- Full
just mobile-test— PASS; 2,072 tests. just file-size-check— PASS.git diff --check 1c8321cd...HEAD— PASS.- Android cancellation mutation
stop → pause— causal FAIL (30s timeout), then restored. copyWithpreservation mutation → old false default — causal FAIL (expected true / actual false), then restored.- Live PR head matched local
HEAD; final tree clean. - GitHub Mobile, Mobile Swift, and DCO checks passed; no completed red check at final snapshot. Unrelated Desktop/Windows jobs remained in progress.
Manual/native evidence and residual risk
No physical Android APK/native codec run was available because the local environment has no Android SDK. Physical Android just_audio.stop(), MediaCodec packaging/fast-start/upload/playback roundtrip, TalkBack/VoiceOver, and iOS/Android audio interruption/background/route-change behavior remain unobserved. These are explicit native/release-QA confidence gaps, not established defects or author-actionable failures.
The controller regression uses a fake backend and does not itself bind the production stop() primitive, although source tracing plus the pinned dependency contract and a causal controller mutation support the implementation. A direct production-adapter contract test would be useful hardening, but its absence does not justify withholding approval when the shipped production path is established.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
APPROVE at exact head 4db279c5c83b6ff6aa33cdc1828554bfaaa352ad (base 1c8321cd08feb597f8bcff5195c21148fb3e98ed).
The A Team found no remaining author-actionable defect. The prior blockers are repaired:
- Android pre-play Cancel now owns a cancellable load and calls the production
just_audio.stop()path; deterministic coverage holdssetUrl, cancels, proves interruption/no play/settled state, then retries successfully. canCancelLoadingis preserved across unrelated position/duration updates and explicitly cleared at terminal transitions. Reverting preservation fails the regression.- New public Dart declarations now carry the required documentation.
Systems tracing found async generations continue to fence activation, download/native loading, replacement, cancellation, and late completion; temporary-file, packaging, upload, and video/mp4 render contracts remain bounded and aligned with Desktop.
Exact-head validation
just mobile-check: passed (545 files, analyzer clean).- Full
just mobile-test: 2,072/2,072 passed. just file-size-checkandgit diff --check: passed.- Exact-head GitHub Mobile, Mobile Swift, integration, macOS, and relevant required checks observed were passing; no completed red check was present. Some unrelated Desktop/Windows jobs remained in progress.
Author action: none.
Confidence gaps / verification ownership: no physical Android/iOS slow-load, audio interruption/route/background, TalkBack/VoiceOver, MediaCodec packaging, authenticated-relay cancellation, or packaged-media roundtrip was run. Native/release QA owns those device journeys. The fake-backend test does not independently mutation-bind stop() versus pause(), but source tracing of pinned just_audio 0.10.6 establishes that stop() invalidates activation and interrupts pending load; optional adapter-level mutation strengthening belongs to reviewer/author hardening and is not author-blocking.
…rcement * origin/main: 🤖 fix(desktop): harden smoke E2E tests against Bestie overlay and toast timing (#7270) Show status and huddle indicators beside names (#7112) Add mobile voice notes (#7121) perf(desktop): publish mention sends before waking agents (#7154) fix(desktop): unify owned-agent cloud provenance markers (#7129) fix(desktop): derive agent availability from relay presence (#7127) fix(desktop): preserve spacing after multi-word mentions (#7128) Signed-off-by: Hayt <9e1c23a3fd83f61da34420e4e88ff1b16e45cafcc0cd9019eb07d4ecfa8ca9b0@buzz.block.builderlab.xyz>
* origin/main: 🤖 fix(desktop): harden smoke E2E tests against Bestie overlay and toast timing (#7270) Show status and huddle indicators beside names (#7112) Add mobile voice notes (#7121) perf(desktop): publish mention sends before waking agents (#7154) fix(desktop): unify owned-agent cloud provenance markers (#7129) fix(desktop): derive agent availability from relay presence (#7127) fix(desktop): preserve spacing after multi-word mentions (#7128) docs(nip-fi): document Git smart-HTTP credential exemption (#7268) feat(cli): add buzz gifs command group and NIP-30 emoji tags on messages (#7259) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…eway-origin * origin/main: 🤖 fix(desktop): harden smoke E2E tests against Bestie overlay and toast timing (#7270) Show status and huddle indicators beside names (#7112) Add mobile voice notes (#7121) perf(desktop): publish mention sends before waking agents (#7154) fix(desktop): unify owned-agent cloud provenance markers (#7129) Signed-off-by: Tom Brow <tomb@block.xyz>
3 participants
Summary
Mobile counterpart to #6978.
Testing
just cijust mobile-checkjust mobile-test(2,026 tests)