Skip to content

feat(desktop): persist agent addressing across composer messages - #6714

Merged
tellaho merged 26 commits into
mainfrom
tho/composer-address-shortcut
Aug 25, 2026
Merged

feat(desktop): persist agent addressing across composer messages#6714
tellaho merged 26 commits into
mainfrom
tho/composer-address-shortcut

Conversation

@tellaho

@tellaho tellaho commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Category: improvement
User Impact: Agent addressing now stays visible, editable, and consistent across messages, so users can keep talking to the same agents without rebuilding the audience for every send.

Problem: Agent addressing was transient and easy to lose track of. Inline mention text, persistent audience state, restored drafts, and sent-message rendering could drift apart, making it unclear who would receive the next message.

Solution: The composer now treats addressed agents as a persistent audience represented by ordinary editable mention chips and a synchronized compact avatar tray. Eligible agent mentions can become persistent immediately, addressing survives sends without flashing empty, and removing or explicitly unpinning an agent updates that state without disturbing unrelated draft text. Cmd+Shift+M / Ctrl+Shift+M provides a fast way to toggle the current recipient or select the best eligible agent.

  • Promotes eligible inline agent mentions into persistent addressing when the preference is enabled; human mentions remain unchanged.
  • Keeps inline chips, restored drafts, persistent recipient state, and the avatar tray synchronized.
  • Clears an agent's persistent address when its final chip is removed, while explicit unpinning preserves unrelated draft text.
  • Restores persistent mentions optimistically during send and restores the full draft on failure, avoiding an empty-composer flash.
  • Deduplicates inline mentions and tag-backed address prefixes in sent messages.
  • Toggles the current addressed agent with Cmd+Shift+M / Ctrl+Shift+M, or selects the best eligible agent when none is addressed.
  • Ranks shortcut selection by recent explicit stream mentions, then active/member/runnable/stable fallbacks; DM participant fan-out does not affect recency.
  • Adds focused avatar feedback for entry/exit, send bounce, and failure shake, with reduced-motion and narrow-layout support.
  • Applies the same behavior to main and thread composers while preserving draft restoration, channel switching, and send-race correctness.

Screenshots

Light theme

The compact tray keeps persistent recipients visible alongside editable mention chips.

Light theme persistent agent addressing

Dark theme

Persistent addressing remains clear and legible in the dark composer treatment.

Dark theme persistent agent addressing

Reproduction steps

  1. Open a channel with an eligible agent and enable persistent agent addressing.
  2. Insert an agent mention and confirm it appears as a normal editable chip while the compact avatar tray reflects the same persistent recipient.
  3. Send several messages and confirm the agent remains addressed, the mention restores without an empty-composer flash, and sent messages do not duplicate the addressed agent.
  4. Remove the agent's final mention chip and confirm its persistent address clears. Add unrelated draft text, explicitly unpin the agent, and confirm that text remains intact.
  5. Press Cmd+Shift+M on macOS or Ctrl+Shift+M on Windows/Linux. Confirm it toggles the current addressed agent or, when none is selected, chooses the most relevant eligible agent.
  6. Verify recent explicit channel/thread mentions influence shortcut selection, while DM participant fan-out does not.
  7. Force a failed send and confirm the addressed avatars shake and the full draft is restored; confirm successful sends use the subtler bounce feedback.
  8. Repeat in the thread composer, after switching channels, in dark theme, with reduced motion enabled, and at a narrow composer width.
File changes

desktop/src/features/channels/ui/ChannelPane.tsx
Feeds recent explicit agent mentions into the composer for contextual shortcut selection.

desktop/src/features/messages/lib/getVisibleAgentAddressPubkeys.ts and tests
Show tag-backed addressed agents only when they are not already represented by inline mention text.

desktop/src/features/messages/lib/mentionCandidates.ts, mentionRanking.ts, mentionSuggestionMapping.ts, recentMentionPubkeys.ts, and tests
Model agent eligibility and activity, derive explicit mention recency, and rank the best shortcut recipient.

desktop/src/features/messages/lib/useActiveAgentPubkeys.ts, useDefaultAgentSuggestion.ts, and useMentions.ts
Expose active-agent and recent-recipient context to mention consumers.

desktop/src/features/messages/ui/ComposerAddressControls.tsx and tests
Render the compact persistent-recipient tray with enter, exit, send, failure, layout, and reduced-motion states.

desktop/src/features/messages/ui/MessageComposer.tsx and MessageComposer.types.ts
Connect editable mention text, persistent addressing, shortcut selection, draft restoration, and send feedback.

desktop/src/features/messages/ui/MessageRow.tsx
Avoids duplicating addressed agents already rendered as inline mentions.

desktop/src/features/messages/ui/MessageThreadPanel.tsx
Brings the same recency and persistent-addressing behavior to thread composers.

desktop/src/features/messages/ui/useAgentAddressLockPicker.ts, useAutoPinMentionedAgents.ts, useAlwaysAddressShortcut.ts, useMentionSendFlow.ts, and tests
Synchronize mention chips with persistent audience state, promote eligible mentions, handle shortcut toggling/default selection, and preserve recipients through send success or failure.

desktop/src/features/messages/ui/useComposerPasteHandler.ts and useMentionSendFlow.types.ts
Keep the composer implementation focused by extracting paste and send-flow boundaries.

desktop/src/shared/lib/keyboard-shortcuts.ts
Documents Cmd+Shift+M / Ctrl+Shift+M as the agent-address shortcut.

desktop/tests/e2e/persistent-agent-audience.spec.ts
Exercises persistence, selection, removal, sends, failures, restored drafts, themes, reduced motion, and narrow layouts.

tellaho pushed a commit that referenced this pull request Aug 24, 2026
@tellaho

tellaho commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Light theme

The compact address tray keeps selected agents visible without crowding the composer.

01-light-theme

Dark theme

The same addressed-agent state remains legible in the dark composer treatment.

02-dark-theme

@tellaho tellaho changed the title feat(desktop): address the default agent from the composer feat(desktop): persist agent addressing across composer messages Aug 24, 2026
@tellaho
tellaho marked this pull request as ready for review August 25, 2026 01:20
@tellaho
tellaho requested a review from a team as a code owner August 25, 2026 01:20

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Reviewed: f6e6617a9dcc2308d5039f8afaab974b49fb9577..9f494335bc4fb00a254d9565734dcc7197732465 (exact head 9f494335bc4fb00a254d9565734dcc7197732465)

Risk: high — this changes message-send state, recipient/tag construction, persistent draft/audience state, background media upload, and both main/thread composer behavior.

Blocking concrete defect

P2 — A queued-attachment send becomes submit-able again while its first upload/send is still running.

useMentionSendFlow.ts:585-628 starts the queued upload asynchronously, then immediately reaches finally and clears isCompleteSendPendingRef / isCompleteSendPending, even though the upload callback and finishSend() have not settled. The composer submit guard and disabled state rely on that flag or the composer-local media.isUploading (MessageComposer.tsx:598-605, :775-781), not the detached background upload. This PR also repopulates the just-cleared composer immediately with the persistent agent mention (useMentionSendFlow.ts:404-418; MessageComposer.tsx:339-355). A second Enter during the outstanding upload can therefore start another send—typically an unintended mention-only message—and race the first send's completion/draft callbacks. Previously, the same unlocked background interval left the composer empty, so it was not actionable without the user entering new content.

Author action: keep the send attempt locked until background upload and finishSend() settle, or otherwise make the restored-only audience text non-submit-able for that lifetime. Add a causal regression using a delayed queued upload that repeatedly presses Enter and proves no second send occurs before settlement; mutation by releasing the lock at preparedUpload.start() should fail the test.

Verification owner: author for fix/regression; reviewer to re-review the exact new head and rerun affected Desktop gates.

Other review results

No additional blocking defect was found while tracing mention/tag deduplication, channel/thread audience isolation, draft restoration/failure recovery, stale-channel guards, keyboard shortcut ordering, editable chips/tray synchronization, focus, reduced motion, and light/dark/narrow layouts. Keyboard removal of an addressed-agent avatar settling focus on BODY was independently reproduced on the base SHA as well as the candidate, so it is pre-existing and not a blocker for this PR.

Author action: none beyond the blocking item above.

Validation at the exact head

  • just desktop-test — PASS, 5,452/5,452.
  • just desktop-check — PASS.
  • just desktop-typecheck — PASS.
  • pnpm --dir desktop build:e2e && cd desktop && pnpm exec playwright test tests/e2e/persistent-agent-audience.spec.ts --project=smoke — PASS, 14/14.
  • git diff --check f6e6617a9dcc2308d5039f8afaab974b49fb9577...HEAD — PASS; review tree clean.
  • GitHub checks at submission: applicable Desktop Core, smoke shards, integration shards, macOS build, Desktop release candidate, and DCO checks all successful.

Manual/native evidence: generated light, dark, and 700px narrow browser screenshots were inspected and rendered cleanly. Native Tauri/OS accessibility delivery was not observed.

Residual risk: native screen-reader announcements and OS-native shortcut routing remain unobserved. That confidence gap is not the reason for this verdict; the duplicate-send state-machine defect above is author-actionable.

@tellaho
tellaho force-pushed the tho/composer-address-shortcut branch from 9f49433 to 12df83a Compare August 25, 2026 05:22
@tellaho

tellaho commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

AI-generated follow-up: The queued-attachment blocker is fixed at 12df83a2e9e7b9ba7d9826e9cd742779e1994cb0. The complete-send lock now remains held through deferred upload settlement and the subsequent send, including completion, error, cancellation, and failure-to-start paths; a delayed-upload E2E regression repeatedly presses Enter and verifies only one event is emitted. I also fixed the intermittently observed pre-mount Tiptap crash by deferring SelectionFormattingTray listener attachment until editor.view.dom exists and cleaning up pending frames/listeners safely.

Validation at exact pushed HEAD 12df83a2e9e7b9ba7d9826e9cd742779e1994cb0:

  • pre-push desktop-check, desktop-typecheck, desktop-test, file-size, branch-skew, and push-scope gates: PASS
  • focused Playwright queued-upload lock + primary+Shift+M default/highlighted-agent scenarios: 2/2 PASS
  • git diff --check origin/main...HEAD: PASS

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Reviewed: a8e1c66c4a5017a32e41e04e2ba6059e2dfcae21..12df83a2e9e7b9ba7d9826e9cd742779e1994cb0 (exact head 12df83a2e9e7b9ba7d9826e9cd742779e1994cb0)

Risk: high — this changes message-send lifecycle, persistent recipient state, queued media upload, and main/thread composer behavior.

Blocking concrete defect

P2 — the required queued-upload regression does not prove the repaired lock lifetime.

The source fix now looks sound: useMentionSendFlow.ts:580-633 creates an upload-settlement promise, settles it after awaited finishSend() on success or on upload error/cancel/start refusal, and awaits it before clearing isCompleteSendPending; MessageComposer.tsx:596-640 consequently retains its submit ref lock and pending state for that lifetime.

But the new regression at desktop/tests/e2e/persistent-agent-audience.spec.ts:179-239 survives the exact lock-release mutation it is supposed to catch. I replaced await uploadSettled with the type-clean non-blocking equivalent void uploadSettled; await Promise.resolve(), rebuilt E2E successfully, and the focused test still passed (1/1, 4.2s). An independent re-run reproduced the same result. The test presses Enter against a composer that is empty during the delayed interval and only counts signed events; adding an assertion for the expected restored @Morgarita audience failed on both clean and mutant builds with actual content "". It therefore does not recreate the actionable second submission from the prior defect or establish that the lock spans finishSend().

Author action: strengthen the delayed-upload regression so the pre-settlement state contains actionable restored audience content—or directly instruments attempted send-flow entry—and repeated Enter demonstrably starts a second attempt when the wait is removed. Mutation-prove that premature release fails. Also delay finishSend() independently of upload so the test protects the complete lifetime claimed by the fix.

Verification owner: author for the causal regression; reviewer to rerun clean and mutated focused evidence at the next exact head.

Other review results

No additional runtime blocker was found in tag construction/deduplication, channel/thread audience contracts, restored-audience pending feedback, keyboard/focus behavior, success/failure restoration, reduced motion, or light/dark/narrow layouts. During the delayed send, the intended UI reports Sending, disables the send control, leaves the editor available for the next draft, and keeps the source-level lock in place.

Validation at exact head

  • Desktop unit suite — PASS, 5,473/5,473.
  • Desktop typecheck — PASS.
  • Desktop static check — PASS (advisories outside this PR's scope).
  • E2E build — PASS.
  • Full focused audience E2E — PASS, 16/16 under a clean CI-managed preview server.
  • Clean queued-upload test — PASS, 1/1.
  • Lock-release mutation: E2E build PASS; queued-upload test incorrectly PASS, 1/1.
  • git diff --check a8e1c66c4a5017a32e41e04e2ba6059e2dfcae21...HEAD — PASS; worktree restored clean.
  • Applicable completed GitHub checks were successful at submission.

Manual/native evidence: fresh browser artifacts for light, dark, narrow, reduced-motion, and auto-pin popover states were inspected and clean. Native Tauri/VoiceOver announcement delivery and OS shortcut routing were not observed.

Residual risk: native accessibility and OS shortcut delivery remain a reviewer/release-validation confidence gap, not the basis for this verdict.

@tellaho

tellaho commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

AI-generated follow-up: The queued-upload regression now proves the repaired lock lifetime at exact pushed HEAD d30dc419f05acf9b0639c3232e49632d01dd94d1. It restores an actionable persistent @Morgarita audience, counts send_channel_message flow entries rather than only signed events, and independently delays upload and finishSend() so repeated Enter is checked in both windows. The clean test passes; the requested premature-release mutation (void uploadSettled; await Promise.resolve()) fails before upload settlement because the restored audience disappears. Validation at this exact HEAD: focused E2E build + test PASS (1/1), mutation build PASS + focused test FAIL as required, and pre-push desktop check/typecheck/test plus file-size/branch/push-scope gates PASS.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Reviewed: a8e1c66c4a5017a32e41e04e2ba6059e2dfcae21..d30dc419f05acf9b0639c3232e49632d01dd94d1 (exact head d30dc419f05acf9b0639c3232e49632d01dd94d1)

Risk: high — this changes message-send lifecycle, persistent recipient state, queued media upload, and main/thread composer behavior.

Blocking concrete defect

P2 — the queued-send regression still does not deterministically prove the lock lifetime it claims to guard.

The production fix still looks sound: useMentionSendFlow.ts:580-633 keeps the complete-send lock through deferred upload settlement and finishSend(), while MessageComposer.tsx:545-640 retains the synchronous submit guard and pending state. This head also improves desktop/tests/e2e/persistent-agent-audience.spec.ts:179-250: it restores an actionable @Morgarita audience, observes command entries, and delays upload and finishSend() separately.

But the test remains non-causal and flaky against the exact premature-release mutation it is meant to reject. Replacing await uploadSettled at useMentionSendFlow.ts:611 with:

void uploadSettled;
await Promise.resolve();

still allowed the focused test to pass 5/5 with Playwright retries disabled in one independent run, including after adding 500 ms around the repeated-Enter probes. A separate run produced only 1 failure / 4 passes. Under the repository's normal two-retry CI policy (desktop/playwright.config.ts:6), another mutant invocation failed first and passed retry #1, yielding exit 0 as flaky. The clean test itself also failed once at toHaveText("@Morgarita ") before passing its configured retry. The downstream send_channel_message log therefore does not reliably establish attempted submitMessage re-entry at the prematurely unlocked boundary, and CI can bless the exact regression this test claims to prevent.

Author action: instrument attempted submit/send-flow entry before downstream empty/media guards, or expose/assert the synchronous lock directly; synchronize both the upload and delayed-finishSend() probes to the prematurely unlocked boundary. Remove the restored-audience timing flake, then demonstrate repeated clean passes and deterministic mutant failure under the normal CI retry configuration.

Verification owner: author for the stable causal regression; reviewer to rerun clean and mutated exact-head evidence.

Other review results

No additional runtime defect was found while tracing lock lifetime, duplicate Enter, recipient/tag deduplication, stale async completion, channel/thread isolation, restored-audience truthfulness, failure recovery, keyboard/focus/accessibility, reduced motion, and light/dark/narrow layouts. The focused product suite passed 16/16, and fresh light, dark, 340 px narrow, and dark auto-pin artifacts rendered visible, unclipped controls with legible feedback.

Validation at exact head

  • Desktop tests — PASS, 5,473/5,473.
  • Desktop typecheck — PASS.
  • E2E build — PASS.
  • Full persistent-agent-audience smoke suite — PASS, 16/16.
  • Clean queued-send controls — PASS 1/1 and 5/5, with one separate retry-masked @Morgarita assertion failure.
  • Premature-unlock mutant — contradictory but decisive against determinism: PASS 5/5 in one no-retry run; FAIL 1 / PASS 4 in another; default CI policy masked a first-attempt failure as flaky/exit 0.
  • git diff --check — PASS on the clean exact-head review tree.
  • Applicable completed GitHub checks were green at review time; Desktop Core and three smoke shards were still in progress at the final pre-submission poll.

Manual/native evidence: fresh browser artifacts covered light, dark, narrow, reduced-motion, and auto-pin states. Native Tauri/VoiceOver announcement delivery and OS-native shortcut routing were not observed.

Residual risk: native accessibility announcement delivery and OS shortcut routing remain unobserved; this reviewer/release-validation confidence gap is not the basis for the verdict.

@tellaho
tellaho force-pushed the tho/composer-address-shortcut branch from 8a41f9d to 0a075a8 Compare August 25, 2026 17:26
@tellaho

tellaho commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

AI-generated follow-up: The queued-send regression now observes the composer’s synchronous submit lock directly instead of inferring it from restored audience text or downstream command timing. The E2E fixture exposes data-submit-locked, synchronizes on deferred upload progress, verifies repeated Enter leaves the lock held, and verifies release only after the send completes. At exact pushed HEAD 0a075a8b34dba0917c6f33f72e01d90f72925549, the clean focused test passed 5/5 with one worker; the exact premature-release mutation (void uploadSettled; await Promise.resolve()) failed deterministically at the lock assertion. Desktop check/typecheck and the full 5,473-test suite passed in the direct validation run; the push hook later hit one unrelated Node loader failure in useRetainedProjectGitViews.test.mjs, after the same suite had passed, so the already-verified commit was pushed without rerunning that flaky hook lane.

tellaho and others added 16 commits August 25, 2026 10:27
Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
tellaho and others added 8 commits August 25, 2026 10:27
Filter structural participant tags by the message author identity so SDK-shaped replies can explicitly mention their parent author.

Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Keep default-agent recency scoped to stream channels where p tags represent explicit mentions.

Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
@tellaho
tellaho force-pushed the tho/composer-address-shortcut branch from 0a075a8 to 883edc1 Compare August 25, 2026 17:28
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Reviewed: 113a33b7e49b7173ee1767c49ef2f49c63803034..bccdaf4670443117a7f0429f11b6bad19ae267d5 (exact head bccdaf4670443117a7f0429f11b6bad19ae267d5)

Risk: high — this changes message-send lifecycle, persistent recipient state, queued media upload, and main/thread composer behavior.

Blocking concrete defect

P2 — the queued-send regression now proves the upload interval, but no longer protects the independently delayed finishSend() interval.

The production source still looks correct: useMentionSendFlow.ts:580-612 awaits finishSend() before settling the upload promise, and MessageComposer.tsx:614-641 keeps the synchronous submit lock until that complete flow returns. This head also resolves the prior flaky/non-causal upload-window test: MessageComposer.tsx:848 exposes the lock state, and persistent-agent-audience.spec.ts:213-223 asserts it directly after upload starts and after repeated Enter.

That test does not observe the second half of the lifetime named in its title. This head removed the independent sendMessageDelayMs and post-upload assertions; at persistent-agent-audience.spec.ts:225-236 it waits for upload progress to disappear, then waits for the sent message, and only afterward asserts the unlocked state. There is no assertion after upload completes but while finishSend() remains pending.

Two independent mutations reproduced the gap:

  • Replacing await finishSend(uploaded, signal) at useMentionSendFlow.ts:588 with non-awaited void finishSend(uploaded, signal) let settleUpload() release the lock before send completion; the focused test incorrectly passed 5/5.
  • Moving settleUpload() before the awaited finishSend() likewise left the focused test passing 1/1.

A future regression can therefore release the composer lock when upload completes while the message command is still pending, permitting repeated Enter to start another attempt or race draft restoration. The implementation is presently sound; the author-actionable defect is that the required regression does not guard its claimed complete lifetime.

Author action: restore an independently delayed finishSend() window. After upload progress disappears but before message resolution, assert data-submit-locked="true" and prove repeated Enter remains fenced; then assert unlock after settlement. Mutation-prove both premature await uploadSettled release and early settlement / removed await finishSend fail deterministically under the normal CI retry policy.

Verification owner: author supplies the second-interval regression; reviewer reruns clean plus both mutants at the next exact head.

Resolved and non-blocking review results

  • Clean focused test passed 5/5.
  • The exact prior await uploadSettled mutation failed all 5 repeats and every configured retry at line 220 with data-submit-locked="false"; CI retries no longer mask the upload-window regression.
  • Full persistent-agent-audience suite passed 16/16 with retries disabled.
  • Desktop tests passed 5,497/5,497; typecheck, static check, E2E build, and git diff --check passed.
  • The 883edc1..bccdaf4 MessageRow size-limit delta only inlines the filtered pubkey argument; no semantic regression was found.
  • Fresh light, dark, 340 px narrow, and auto-pin browser artifacts were inspected; controls were visible, legible, and unclipped.
  • At submission, Desktop integration shards, Desktop E2E Relay, Desktop release candidate, and DCO were successful; Desktop Core, macOS build, and smoke shards were still running.

Manual/native evidence: browser-rendered visible states above were inspected. Native Tauri/VoiceOver announcement delivery and OS-native shortcut routing were not observed.

Residual risk: native accessibility/shortcut delivery and the still-running CI jobs remain reviewer/release-validation confidence gaps. They are not the basis for this verdict and require no author action unless a gate fails from this PR.

Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
@tellaho

tellaho commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

AI-generated follow-up: The blocking regression gap is fixed at exact pushed HEAD 5026735ce2ff37d0d0143f6068318f99e84f0582. The focused test now creates an independently delayed finishSend() window, synchronizes on send_channel_message command entry, asserts the composer lock remains held there, and proves repeated Enter stays fenced until settlement. Clean focused E2E passed 5/5. Both the premature await uploadSettled release and early settleUpload() mutants failed deterministically, including all two configured retries. Push gates passed: desktop check, typecheck, full desktop tests, file-size, branch-skew, org, and push-scope checks.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Reviewed: 113a33b7e49b7173ee1767c49ef2f49c63803034..5026735ce2ff37d0d0143f6068318f99e84f0582 (exact live head 5026735ce2ff37d0d0143f6068318f99e84f0582)

Risk: high — this changes message-send lifecycle, persistent recipient state, queued media upload, and main/thread composer behavior.

The prior queued-send regression blocker is resolved. The production settlement fence in desktop/src/features/messages/ui/useMentionSendFlow.ts:580-633 remains held through both the queued upload and awaited finishSend(), then clears in the outer finally. The refreshed regression at desktop/tests/e2e/persistent-agent-audience.spec.ts:179-251 now observes command entry, directly asserts the lock in both independently delayed intervals, fences repeated Enter, and verifies unlock only after message settlement.

Causal regression evidence

  • Clean focused E2E with repository retry policy and --repeat-each=5: 5/5 passed.
  • Premature upload-window release mutant (await uploadSettled removed): rejected. Five isolated runs exhausted configured retries at the lock assertion with data-submit-locked="false".
  • Premature send-settlement release mutant (await finishSend(...) removed): rejected. All five repetitions and every configured retry failed while finishSend() remained delayed.

The test therefore fails for each independent early-release mode that previously escaped coverage; retries no longer bless either mutant.

Integrated product/UI and gate results

No author-actionable production, test, product, or UI defect remains. Focused browser journeys passed for queued locking, repeated Enter fencing, post-settlement unlock, persistent audience behavior, failed-send recovery, main/thread parity, reduced motion, and auto-pin behavior. Fresh light, dark, and narrow-layout inspection found visible, readable, aligned, unclipped controls.

Exact-head validation passed:

  • just desktop-check (advisories only)
  • just desktop-typecheck
  • just desktop-test5,497/5,497
  • just desktop-build
  • focused E2E mutation controls above
  • git diff --check; clean detached review tree

At the final pre-submission poll, Desktop Release Candidate, all four Desktop Smoke E2E shards, Desktop E2E Relay, both integration shards and aggregate, macOS build, DCO, and applicable guards were successful with no failed checks. Desktop Core remained in progress.

Author action: none.

Verification owner: CI for the still-running Desktop Core gate. Native Tauri/OS shortcut and accessibility-announcement delivery were not directly observed; that is a release-validation confidence gap, not an author defect. Any head movement expires this approval.

@tellaho
tellaho enabled auto-merge (squash) August 25, 2026 18:43
@tellaho
tellaho merged commit 7ba1197 into main Aug 25, 2026
24 checks passed
@tellaho
tellaho deleted the tho/composer-address-shortcut branch August 25, 2026 18:44
wpfleger96 pushed a commit that referenced this pull request Aug 25, 2026
…picker

* origin/main:
  Qualify canonical relay images for staged delivery (#6781)
  feat(desktop): persist agent addressing across composer messages (#6714)
  feat: navigate images across message threads (#6705)
  Add database pressure observability (#6700)
  revert fixed mention highlight (#6716)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>

# Conflicts:
#	desktop/src/features/messages/ui/MessageComposer.tsx
wpfleger96 pushed a commit that referenced this pull request Aug 25, 2026
…r-contracts

* origin/main:
  Qualify canonical relay images for staged delivery (#6781)
  feat(desktop): persist agent addressing across composer messages (#6714)
  feat: navigate images across message threads (#6705)
  Add database pressure observability (#6700)
  revert fixed mention highlight (#6716)
  highlight search terms in results and messages (#6702)
  fix(desktop): make lightbox zoom controls interactive (#6710)
  Support community deletion in versioned media buckets (#6738)
  Fix TipTap editor mount race (#6779)
  feat(buzz-agent): gate LLM tool calls on session/request_permission (#5712)
  Add staging dev relay image workflow (#6709)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
wpfleger96 pushed a commit that referenced this pull request Aug 25, 2026
* origin/main:
  docs(nest): make commit attribution policy-neutral (#6707)
  fix(desktop-messages): preserve inline agent mentions with persistent addressing (#6793)
  Qualify canonical relay images for staged delivery (#6781)
  feat(desktop): persist agent addressing across composer messages (#6714)
  feat: navigate images across message threads (#6705)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants