fix(desktop): bind channel and thread context at compose time to prevent wrong-channel send#1472
Merged
Merged
Conversation
…l send
When the user tagged an agent and immediately switched channels, the
outgoing message landed in the newly-selected channel instead of the
channel it was composed in.
Root cause: the send pipeline read the channel from "latest-value" refs
that are refreshed every render. useMentionSendFlow's async agent-prep
awaits (createMentionedPersonaAgents, ensureManagedAgentMentionsReady)
opened a window during which a channel switch rotated both onSendRef and
sendMutateRef to the new channel's handlers. When completeSend finally
called onSendRef.current(), it targeted the wrong channel.
Fix: capture channelId at submitMessage entry and thread it as data
through the entire pipeline:
submitMessage → sendMessageWithMentionFlow (capturedChannelId)
→ PendingNonMemberMentionSend.capturedChannelId
→ completeSend → onSendRef.current(..., channelId)
→ handleSendMessage / handleSendThreadReply → sendMutateRef.current
→ useSendMessageMutation variables.channelId
In useSendMessageMutation, mutationFn and onMutate resolve the target
channel from the query cache using variables.channelId when provided,
falling back to the closed-over channel for callers that don't supply
one. This ensures both the actual send and the optimistic cache write
target the compose-time channel.
Also guard clearComposer() in completeSend: when capturedChannelId no
longer matches the live channelId the user has switched away, skip the
clear so the newly-active channel's composer text is not wiped.
The non-member-prompt paths (handleSendWithoutInviting,
handleInviteNonMembers) already carry capturedChannelId via the shared
PendingNonMemberMentionSend draft, so they are fixed by the same change.
InboxDetailPane is immune: its composer receives channelId from the item
data rather than from live navigation state.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
F1: channelIdRef for live channel in completeSend guard The clearComposer/restore guard compared draft.capturedChannelId to the completeSend closure's channelId, which is always the compose-time channel on the direct-send path (the outer useCallback captures the same render). A === A always, so the guard never fired. Add channelIdRef updated every render (outside useCallback) and compare against channelIdRef.current so the guard reads the live navigation state, not the frozen submit-time value. F2: thread capturedChannelId into all channel-scoped mutation call sites attachAgentMutation, createPersonaAgentMutation, and addMembersMutation were instantiated with the hook's live channelId. RQ observer.setOptions refreshes mutationFn every render, so any call after a mid-flight switch executed the switched-to channel's closure. Extend each mutation's variable type with an optional channelId field (same pattern as useSendMessageMutation) and pass draft.capturedChannelId / capturedChannelId through ensureManagedAgentMentionsReady, createMentionedPersonaAgents, and handleInviteNonMembers so every Tauri call targets the compose-time channel. F3: throw on non-null cache miss instead of silent live-channel fallback effectiveChannel = cacheLookup(capturedId) ?? channel was the original code; when the captured id was non-null but absent from the channels cache (most likely for a brand-new channel, exactly the repro's step 1), the send silently fell through to the live channel. Split the resolution: capturedId == null → use fallbackChannel (legacy callers); non-null → return null on cache miss so the caller throws 'Channel is no longer available' rather than misdelivering. Extract resolveEffectiveChannel as a named pure function used by both mutationFn and onMutate. F4: add resolveEffectiveChannel unit tests that pin the invariant Five deterministic pure-function tests: - capturedId present → returns compose-time channel (core invariant) - capturedId not in cache → returns null (F3 throw case) - capturedId null → falls back to closed-over channel (legacy path) - capturedId undefined → same as null - empty cache + non-null capturedId → returns null Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wesbillman
approved these changes
Jul 2, 2026
Thread-reply send context (CRITICAL):
- Add onCaptureSendContext optional prop to MessageComposer, called
synchronously at submitMessage before any awaits.
- MessageThreadPanel provides onCaptureSendContext that reads live refs
at submit time (replyTargetMessageRef, threadHeadId), producing an
immutable {parentEventId, threadHeadId} object.
- Bail at submit time if parentEventId is null — no post-await discovery.
- handleSendThreadReply uses threadContext?.parentEventId ?? ... falling
back to live refs only for direct callers that bypass onCaptureSendContext.
- Post-send UI updates guarded: skip setThreadReplyTargetId /
setThreadScrollTargetId if user navigated away (live ref check).
- capturedThreadContext threaded through useMentionSendFlow →
PendingNonMemberMentionSend → completeSend → onSendRef.current.
onSettled invalidation (IMPORTANT-1):
- useAttachManagedAgentToChannelMutation, useCreateChannelManagedAgentMutation,
useAddChannelMembersMutation each now derive the effective channel from
mutation variables in onSettled: variables?.channelId ?? channelId.
Server-mutated channel stays fresh; no stale-A after A→B switch.
E2E + bridge delay knob (IMPORTANT-2):
- Add addChannelMembersDelayMs to MockBridgeOptions (bridge.ts + e2eBridge.ts)
and thread into handleAddChannelMembers (same pattern as sendMessageDelayMs).
- send-channel-binding.spec.ts: exact repro (managed agent not in channel A,
500ms delay, submit + immediate switch to B, assert message in A not B) plus
baseline no-switch test.
- Add spec to playwright.config.ts smoke project.
- Unit tests in sendChannelBinding.test.mjs cover thread context invariants.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wpfleger96
changed the title
wesbillman
approved these changes
Jul 2, 2026
F5 — extract resolveThreadReplyTarget pure fn and replace tautological tests: - Add resolveThreadReplyTarget(threadContext, liveReplyTargetId, liveThreadHeadId) to messages/hooks.ts alongside resolveEffectiveChannel. When threadContext is non-null, uses it exclusively (no ?? fallback to live refs). When null, falls back to liveReplyTargetId ?? liveThreadHeadId. Returns null when no parentEventId can be resolved. - handleSendThreadReply now calls resolveThreadReplyTarget and destructures the result — the live-ref resolution logic is no longer inline in the callback. - Replace 4 tautological makeCapturedThreadContext tests with 6 tests of the production function: captured-wins-over-live-refs (the race), null parent returns null, null threadHeadId uses context not live ref (F7), legacy null context falls back to live refs, null context + no reply target falls back to thread head, all-null returns null. F6 — remove dead threadHeadRef in MessageThreadPanel: - threadHeadRef was created and live-updated but never read after onCaptureSendContext was changed to use the closure threadHeadId directly. Remove both lines. F7 — closed by resolveThreadReplyTarget: - The ?? openThreadHeadIdRef.current fallback on a non-null threadContext is now structurally impossible: resolveThreadReplyTarget returns threadContext.threadHeadId (which may be null) without any ?? fallback to live refs. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a user:
add_channel_memberscall resolvesThe message was delivered to channel B. The send pipeline was reading the currently selected channel from latest-value refs at flush time, after slow async work (agent attach/start) opened a race window. The same late-bind class applied to thread replies:
parentEventIdandthreadHeadIdwere read from live refs after the mention-flow awaits.Fix
Channel binding at compose time
capturedChannelIdcaptured synchronously atsubmitMessageentry and threaded as data through the full async pipeline.resolveEffectiveChannel(capturedChannelId, channelsCache, fallback): null captured id → fallback (legacy callers); supplied-but-unresolvable → null (throw at call site, never misdeliver).mutationFnandonMutateinuseSendMessageMutationroute throughresolveEffectiveChannel.useAttachManagedAgentToChannelMutation,useCreateChannelManagedAgentMutation,useAddChannelMembersMutation) acceptchannelIdin variables;onSettledderives the invalidation target fromvariables?.channelId ?? channelIdso the server-mutated channel stays fresh after a switch.channelIdRefso navigation-away doesn't wipe the new channel's draft.Thread-reply context capture
onCaptureSendContextoptional prop onMessageComposer, called synchronously atsubmitMessagebefore any awaits.MessageThreadPanelprovides it: readsreplyTargetMessage?.id ?? threadHeadIdinto an immutable{ parentEventId, threadHeadId }object at submit time.parentEventIdis null — no post-await discovery.resolveThreadReplyTarget(threadContext, liveReplyTargetId, liveThreadHeadId): whenthreadContextis non-null, uses its values exclusively (no live-ref fallback — the F7 degenerate case is structurally impossible). When null, falls back to live refs (legacy callers).handleSendThreadReplycallsresolveThreadReplyTarget— zero inline live-ref reads after the mention-flow awaits.capturedThreadContextthreaded throughuseMentionSendFlow→PendingNonMemberMentionSend→completeSend→onSendRef.current.E2E regression
addChannelMembersDelayMsknob added toMockBridgeOptionsandhandleAddChannelMembers(same pattern assendMessageDelayMs).send-channel-binding.spec.ts: exact repro — managed agent not in channel A, 500ms bridge delay, submit + immediate switch to channel B, assert message appears in A's timeline and not B's. Plus a baseline no-switch test.Tests
Unit tests in
sendChannelBinding.test.mjsexercise both pure functions:resolveEffectiveChannel: captured-id-wins, null-on-miss, legacy-fallback, empty-cacheresolveThreadReplyTarget: captured-context-wins-over-live-refs (the race), null parent → null, null threadHeadId uses context not live ref, legacy null context falls back to live refs, null context + no reply target → thread head, all-null → nullFiles changed
desktop/src/features/messages/hooks.tsresolveEffectiveChannel+resolveThreadReplyTargetpure fns;capturedChannelIdinuseSendMessageMutationdesktop/src/features/messages/lib/sendChannelBinding.test.mjsdesktop/src/features/messages/ui/useMentionSendFlow.tscapturedChannelId+capturedThreadContextthreaded;channelIdReffor composer guardsdesktop/src/features/messages/ui/MessageComposer.tsxonCaptureSendContextprop; bail-at-submit for null parentEventIddesktop/src/features/messages/ui/MessageThreadPanel.tsxonCaptureSendContextimpl; removed deadthreadHeadRefdesktop/src/features/channels/useChannelPaneHandlers.tshandleSendThreadReplyusesresolveThreadReplyTarget; UI guard on navigationdesktop/src/features/channels/ui/ChannelPane.types.tsthreadContextinonSendThreadReplysignaturedesktop/src/features/agents/hooks.tsonSettledusesvariables?.channelId ?? channelIddesktop/src/features/channels/hooks.tsonSettledusesvariables?.channelId ?? channelIddesktop/src/testing/e2eBridge.tsaddChannelMembersDelayMsinhandleAddChannelMembersdesktop/tests/helpers/bridge.tsaddChannelMembersDelayMsinMockBridgeOptionsdesktop/tests/e2e/send-channel-binding.spec.tsdesktop/playwright.config.ts