fix(desktop): bind channel and thread context at compose time to prevent wrong-channel send (#1472)

Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
Will Pfleger
2026-07-02 20:15:44 +00:00
committed by GitHub
co-authored by npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7
parent 02ff06cac2
commit d369ca9df1
14 changed files with 781 additions and 49 deletions
+1
View File
@@ -64,6 +64,7 @@ export default defineConfig({
"**/timeline-no-shift.spec.ts",
"**/human-edit-agent-content.spec.ts",
"**/reaction-order.spec.ts",
"**/send-channel-binding.spec.ts",
],
use: {
...devices["Desktop Chrome"],
+24 -10
View File
@@ -432,15 +432,24 @@ export function useAttachManagedAgentToChannelMutation(
const queryClient = useQueryClient();
return useMutation({
mutationFn: async (input: AttachManagedAgentToChannelInput) => {
if (!channelId) {
mutationFn: async (
input: AttachManagedAgentToChannelInput & { channelId?: string },
) => {
const { channelId: capturedChannelId, ...rest } = input;
const effectiveChannelId = capturedChannelId ?? channelId;
if (!effectiveChannelId) {
throw new Error("No channel selected.");
}
return attachManagedAgentToChannel(channelId, input);
return attachManagedAgentToChannel(effectiveChannelId, rest);
},
onSettled: () => {
invalidateAgentQueriesInBackground(queryClient, channelId);
onSettled: (_data, _err, variables) => {
// Invalidate the effective channel (the one the server actually mutated)
// so its membership/agent state stays fresh. Invalidating the live
// hook-closure channelId when the user has already switched away would
// leave the compose-time channel stale.
const effectiveChannelId = variables?.channelId ?? channelId;
invalidateAgentQueriesInBackground(queryClient, effectiveChannelId);
},
});
}
@@ -469,13 +478,17 @@ export function useCreateChannelManagedAgentMutation(channelId: string | null) {
return useMutation({
mutationFn: async (
input: CreateChannelManagedAgentInput,
input: CreateChannelManagedAgentInput & { channelId?: string },
): Promise<CreateChannelManagedAgentResult> => {
if (!channelId) {
const { channelId: capturedChannelId, ...rest } = input;
const effectiveChannelId = capturedChannelId ?? channelId;
if (!effectiveChannelId) {
throw new Error("No channel selected.");
}
const result = await createChannelManagedAgents(channelId, [input]);
const result = await createChannelManagedAgents(effectiveChannelId, [
rest,
]);
const success = result.successes[0];
if (success) {
return success;
@@ -484,8 +497,9 @@ export function useCreateChannelManagedAgentMutation(channelId: string | null) {
const failure = result.failures[0];
throw new Error(failure?.error ?? "Could not create agent.");
},
onSettled: () => {
invalidateAgentQueriesInBackground(queryClient, channelId);
onSettled: (_data, _err, variables) => {
const effectiveChannelId = variables?.channelId ?? channelId;
invalidateAgentQueriesInBackground(queryClient, effectiveChannelId);
},
});
}
+14 -5
View File
@@ -396,15 +396,24 @@ export function useAddChannelMembersMutation(channelId: string | null) {
const queryClient = useQueryClient();
return useMutation({
mutationFn: (input: Omit<AddChannelMembersInput, "channelId">) => {
if (!channelId) {
mutationFn: (
input: Omit<AddChannelMembersInput, "channelId"> & {
channelId?: string;
},
) => {
const { channelId: capturedChannelId, ...rest } = input;
const effectiveChannelId = capturedChannelId ?? channelId;
if (!effectiveChannelId) {
throw new Error("No channel selected.");
}
return addChannelMembers({ ...input, channelId });
return addChannelMembers({ ...rest, channelId: effectiveChannelId });
},
onSettled: async () => {
await invalidateChannelState(queryClient, channelId);
onSettled: async (_data, _err, variables) => {
// Invalidate the effective channel (the one actually mutated) not the
// live hook-closure channel, which may have changed mid-send.
const effectiveChannelId = variables?.channelId ?? channelId;
await invalidateChannelState(queryClient, effectiveChannelId);
},
});
}
@@ -297,6 +297,7 @@ export const ChannelPane = React.memo(function ChannelPane({
content: string,
mentionPubkeys: string[],
mediaTags?: string[][],
channelId?: string | null,
) => {
const shouldCompleteWelcomeBanner =
isActiveWelcomeChannel &&
@@ -304,7 +305,7 @@ export const ChannelPane = React.memo(function ChannelPane({
mentionsKnownAgent(mentionPubkeys, knownAgentPubkeys));
messageTimelineRef.current?.scrollToBottomOnNextUpdate();
await onSendMessage(content, mentionPubkeys, mediaTags);
await onSendMessage(content, mentionPubkeys, mediaTags, channelId);
if (shouldCompleteWelcomeBanner) {
completeWelcomeComposerBanner();
@@ -67,6 +67,7 @@ export type ChannelPaneProps = {
content: string,
mentionPubkeys: string[],
mediaTags?: string[][],
channelId?: string | null,
) => Promise<void>;
onSendVideoReviewComment?: (
message: TimelineMessage,
@@ -79,6 +80,11 @@ export type ChannelPaneProps = {
content: string,
mentionPubkeys: string[],
mediaTags?: string[][],
channelId?: string | null,
threadContext?: {
parentEventId: string | null;
threadHeadId: string | null;
} | null,
) => Promise<void>;
onTargetReached?: (messageId: string) => void;
onToggleReaction?: (
@@ -6,6 +6,7 @@ import type {
useSendMessageMutation,
useToggleReactionMutation,
} from "@/features/messages/hooks";
import { resolveThreadReplyTarget } from "@/features/messages/hooks";
/**
* Stable callback references for ChannelPane so that keystroke-driven
@@ -230,11 +231,13 @@ export function useChannelPaneHandlers({
content: string,
mentionPubkeys: string[],
mediaTags?: string[][],
channelId?: string | null,
) => {
await sendMutateRef.current({
content,
mentionPubkeys,
mediaTags,
channelId: channelId ?? undefined,
});
},
[],
@@ -245,13 +248,24 @@ export function useChannelPaneHandlers({
content: string,
mentionPubkeys: string[],
mediaTags?: string[][],
channelId?: string | null,
threadContext?: {
parentEventId: string | null;
threadHeadId: string | null;
} | null,
) => {
const activeThreadHeadId = openThreadHeadIdRef.current;
const parentEventId =
threadReplyTargetIdRef.current ?? activeThreadHeadId;
if (!parentEventId) {
// Resolve target using captured submit-time context (race-free) or live
// refs (legacy path). When threadContext is supplied, no live-ref reads
// occur after the mention-flow awaits; the resolution is purely data.
const target = resolveThreadReplyTarget(
threadContext,
threadReplyTargetIdRef.current,
openThreadHeadIdRef.current,
);
if (!target) {
return;
}
const { parentEventId, threadHeadId: activeThreadHeadId } = target;
if (
activeThreadHeadId &&
@@ -270,10 +284,17 @@ export function useChannelPaneHandlers({
mentionPubkeys,
parentEventId,
mediaTags,
channelId: channelId ?? undefined,
});
setThreadReplyTargetId(activeThreadHeadId);
if (activeThreadHeadId && parentEventId !== activeThreadHeadId) {
setThreadScrollTargetId(sentMessage.id);
// Only update thread UI state if the user is still viewing the same
// thread. If they navigated away during the async send, don't disrupt
// the thread they are currently viewing.
if (openThreadHeadIdRef.current === activeThreadHeadId) {
setThreadReplyTargetId(activeThreadHeadId);
if (activeThreadHeadId && parentEventId !== activeThreadHeadId) {
setThreadScrollTargetId(sentMessage.id);
}
}
},
[
+113 -10
View File
@@ -18,6 +18,7 @@ import {
import { splitOutgoingTags } from "@/features/messages/lib/imetaMediaMarkdown";
import { relayClient } from "@/shared/api/relayClient";
import { customEmojiQueryKey } from "@/features/custom-emoji/hooks";
import { channelsQueryKey } from "@/features/channels/hooks";
import { reactionEmojiUrl } from "@/shared/api/customEmoji";
import type { CustomEmoji } from "@/shared/lib/remarkCustomEmoji";
import {
@@ -174,6 +175,71 @@ export function createOptimisticMessage(
};
}
/**
* Resolves the effective target channel for a send operation.
*
* When `capturedChannelId` is supplied (non-null), the target is looked up from
* `channelsCache` — this pins the send to the compose-time channel regardless
* of any subsequent navigation. If the id is supplied but resolves to nothing,
* returns `null` (caller should throw — don't silently fall back to the live
* channel). When `capturedChannelId` is null, the caller didn't capture one and
* the closed-over `fallbackChannel` is the intended target.
*
* Exported for unit testing.
*/
export function resolveEffectiveChannel(
capturedChannelId: string | null | undefined,
channelsCache: Channel[] | undefined,
fallbackChannel: Channel | null,
): Channel | null {
if (capturedChannelId == null) {
return fallbackChannel;
}
return channelsCache?.find((c) => c.id === capturedChannelId) ?? null;
}
/**
* Resolves the thread reply target from a submit-time captured context or,
* for callers that predate the capture pattern, from live refs.
*
* When `threadContext` is supplied (non-null), its values are used exclusively
* — no live-ref reads occur. This is the race-free path: the context was
* captured synchronously at submit time before any async awaits.
*
* When `threadContext` is null/undefined (legacy callers), falls back to
* `liveReplyTargetId ?? liveThreadHeadId`.
*
* Returns null when no parentEventId can be resolved (caller should bail).
*/
export function resolveThreadReplyTarget(
threadContext:
| { parentEventId: string | null; threadHeadId: string | null }
| null
| undefined,
liveReplyTargetId: string | null | undefined,
liveThreadHeadId: string | null | undefined,
): { parentEventId: string; threadHeadId: string | null } | null {
if (threadContext != null) {
// Captured context: use exclusively — no ?? fallback to live refs.
if (!threadContext.parentEventId) {
return null;
}
return {
parentEventId: threadContext.parentEventId,
threadHeadId: threadContext.threadHeadId,
};
}
// Legacy path: read from live refs.
const parentEventId = liveReplyTargetId ?? liveThreadHeadId ?? null;
if (!parentEventId) {
return null;
}
return {
parentEventId,
threadHeadId: liveThreadHeadId ?? null,
};
}
export function useChannelMessagesQuery(channel: Channel | null) {
const queryClient = useQueryClient();
const queryKey = channelMessagesKey(channel?.id ?? "none");
@@ -385,6 +451,7 @@ export function useSendMessageMutation(
RelayEvent,
Error,
{
channelId?: string;
content: string;
mentionPubkeys?: string[];
parentEventId?: string | null;
@@ -393,12 +460,28 @@ export function useSendMessageMutation(
MessageQueryContext | undefined
>({
mutationFn: async ({
channelId: capturedChannelId,
content,
mentionPubkeys,
parentEventId,
mediaTags,
}) => {
if (!channel || channel.channelType === "forum") {
// Resolve the target channel from the compose-time id when provided, so
// a channel switch mid-send does not redirect the message. Fall back to
// the closed-over `channel` for callers that don't supply a capturedId.
// A supplied-but-unresolvable id throws rather than silently falling back
// to the live channel (silent misdelivery is the failure mode we're fixing).
const effectiveChannel = resolveEffectiveChannel(
capturedChannelId,
queryClient.getQueryData<Channel[]>(channelsQueryKey),
channel,
);
if (capturedChannelId != null && effectiveChannel == null) {
throw new Error("Channel is no longer available.");
}
if (!effectiveChannel || effectiveChannel.channelType === "forum") {
throw new Error("This channel does not support message sending yet.");
}
@@ -422,10 +505,10 @@ export function useSendMessageMutation(
if (parentEventId || imetaTags.length > 0 || emojiTags.length > 0) {
const cachedMessages =
queryClient.getQueryData<RelayEvent[]>(
channelMessagesKey(channel.id),
channelMessagesKey(effectiveChannel.id),
) ?? [];
const result = await sendChannelMessage(
channel.id,
effectiveChannel.id,
content,
parentEventId ?? null,
imetaTags,
@@ -440,7 +523,7 @@ export function useSendMessageMutation(
// For non-replies (media-only), we add them ourselves.
const replyTags = parentEventId
? buildReplyTags(
channel.id,
effectiveChannel.id,
identity.pubkey,
parentEventId,
resolveReplyRootId(parentEventId, cachedMessages),
@@ -450,7 +533,7 @@ export function useSendMessageMutation(
const baseTags = parentEventId
? replyTags // buildReplyTags includes h + author p + mention ps
: [
["h", channel.id],
["h", effectiveChannel.id],
["p", identity.pubkey],
]; // non-reply: add ourselves
@@ -478,24 +561,44 @@ export function useSendMessageMutation(
}
return relayClient.sendMessage(
channel.id,
effectiveChannel.id,
content,
mentionPubkeys ?? [],
mentionTags,
);
},
onMutate: async ({ content, mentionPubkeys, parentEventId, mediaTags }) => {
if (!channel || !identity || channel.channelType === "forum") {
onMutate: async ({
channelId: capturedChannelId,
content,
mentionPubkeys,
parentEventId,
mediaTags,
}) => {
// Mirror the mutationFn channel resolution so the optimistic message
// lands in the same cache key the real send will eventually populate.
// A supplied-but-unresolvable id returns undefined (skips optimistic write)
// rather than silently writing to the live channel.
const effectiveChannel = resolveEffectiveChannel(
capturedChannelId,
queryClient.getQueryData<Channel[]>(channelsQueryKey),
channel,
);
if (
!effectiveChannel ||
!identity ||
effectiveChannel.channelType === "forum"
) {
return undefined;
}
const queryKey = channelMessagesKey(channel.id);
const queryKey = channelMessagesKey(effectiveChannel.id);
await queryClient.cancelQueries({ queryKey });
const previousMessages =
queryClient.getQueryData<RelayEvent[]>(queryKey) ?? [];
const optimisticMessage = createOptimisticMessage(
channel.id,
effectiveChannel.id,
content.trim(),
identity,
previousMessages,
@@ -0,0 +1,290 @@
/**
* Regression tests for wrong-channel send bug.
*
* The bug: when a channel switch happens mid-send (during the async agent-prep
* await in useMentionSendFlow), the "latest-value" onSendRef and sendMutateRef
* would already point at the new channel. The fix threads capturedChannelId as
* data through the entire pipeline so the mutation always targets the
* compose-time channel regardless of navigation.
*
* Test coverage:
* 1. createOptimisticMessage uses the supplied channelId for the h-tag.
* 2. resolveEffectiveChannel pins the send to the captured channel even when
* the closed-over channel is different (the core invariant).
* 3. resolveEffectiveChannel returns null for a supplied-but-unresolvable id
* so the caller can throw rather than silently misdeliver.
* 4. resolveEffectiveChannel falls back to the closed-over channel when no
* capturedChannelId was supplied (legacy-caller path).
*/
import assert from "node:assert/strict";
import test from "node:test";
import {
createOptimisticMessage,
resolveEffectiveChannel,
resolveThreadReplyTarget,
} from "../hooks.ts";
// ---------------------------------------------------------------------------
// Minimal stubs
// ---------------------------------------------------------------------------
const IDENTITY = {
pubkey: "aaaa1111bbbb2222cccc3333dddd4444eeee5555ffff6666aaaa1111bbbb2222",
};
function makeChannel(id) {
return {
id,
name: id,
channelType: "channel",
// Only id and channelType are required by the resolution logic.
};
}
// ---------------------------------------------------------------------------
// createOptimisticMessage — h-tag carries the compose-time channelId
// ---------------------------------------------------------------------------
test("createOptimisticMessage_composedChannelId_hTagMatchesComposedChannel", () => {
const composeChannelId = "channel-A";
const msg = createOptimisticMessage(
composeChannelId,
"hello",
IDENTITY,
[], // currentMessages
[], // mentionPubkeys
null, // parentEventId
[], // mediaTags
);
const hTag = msg.tags.find(([name]) => name === "h");
assert.ok(hTag, "message must have an h-tag");
assert.equal(
hTag[1],
composeChannelId,
"h-tag must match the compose-time channelId, not any other channel",
);
assert.equal(msg.content, "hello");
assert.equal(msg.pending, true);
});
test("createOptimisticMessage_differentChannelIds_hTagsAreIndependent", () => {
// Simulate two messages composed in two different channels.
// If a channel switch had corrupted channelId, both would carry the same tag.
const msgA = createOptimisticMessage(
"channel-A",
"msg A",
IDENTITY,
[],
[],
null,
[],
);
const msgB = createOptimisticMessage(
"channel-B",
"msg B",
IDENTITY,
[],
[],
null,
[],
);
const hTagA = msgA.tags.find(([n]) => n === "h");
const hTagB = msgB.tags.find(([n]) => n === "h");
assert.equal(hTagA[1], "channel-A", "message A must target channel-A");
assert.equal(hTagB[1], "channel-B", "message B must target channel-B");
assert.notEqual(
hTagA[1],
hTagB[1],
"compose-time channel isolation: the two h-tags must differ",
);
});
test("createOptimisticMessage_withReply_hTagStillCarriesSuppliedChannelId", () => {
// Thread replies also carry the h-tag via buildReplyTags.
// Verify the channel id flows through when a parentEventId is set.
const composeChannelId = "channel-A";
const parentEvent = createOptimisticMessage(
"channel-A",
"parent",
IDENTITY,
[],
[],
null,
[],
);
const replyMsg = createOptimisticMessage(
composeChannelId,
"reply",
IDENTITY,
[parentEvent],
[],
parentEvent.id,
[],
);
const hTag = replyMsg.tags.find(([name]) => name === "h");
assert.ok(hTag, "reply must have an h-tag");
assert.equal(
hTag[1],
composeChannelId,
"reply h-tag must match the compose-time channelId",
);
});
// ---------------------------------------------------------------------------
// resolveEffectiveChannel — the channel-binding invariant
// ---------------------------------------------------------------------------
test("resolveEffectiveChannel_capturedIdPresentInCache_returnsComposeTimeChannel", () => {
// Core invariant: closure channel is B, variables carry channel A.
// The mutation must target A regardless of what the closure says.
const channelA = makeChannel("channel-A");
const channelB = makeChannel("channel-B");
const cache = [channelA, channelB];
const result = resolveEffectiveChannel("channel-A", cache, channelB);
assert.strictEqual(
result?.id,
"channel-A",
"must return the compose-time channel even when the closed-over channel is B",
);
});
test("resolveEffectiveChannel_capturedIdNotInCache_returnsNull", () => {
// F3 invariant: a supplied-but-unresolvable id must not fall back to the
// live channel — the caller is expected to throw "channel no longer available".
const channelB = makeChannel("channel-B");
const cache = [channelB]; // channel-A is absent (e.g. new channel, cache miss)
const result = resolveEffectiveChannel("channel-A", cache, channelB);
assert.strictEqual(
result,
null,
"a supplied-but-unresolvable capturedChannelId must return null, not the live channel",
);
});
test("resolveEffectiveChannel_capturedIdNull_returnsFallbackChannel", () => {
// Legacy callers (thread reply, InboxDetailPane) don't supply a capturedId.
// They rely on the closed-over channel being correct for other reasons.
const channelB = makeChannel("channel-B");
const cache = [channelB];
const result = resolveEffectiveChannel(null, cache, channelB);
assert.strictEqual(
result?.id,
"channel-B",
"null capturedChannelId must fall through to the closed-over channel",
);
});
test("resolveEffectiveChannel_capturedIdUndefined_returnsFallbackChannel", () => {
// Same as null — undefined means the caller didn't capture an id.
const channelB = makeChannel("channel-B");
const result = resolveEffectiveChannel(undefined, [channelB], channelB);
assert.strictEqual(result?.id, "channel-B");
});
test("resolveEffectiveChannel_emptyCache_capturedIdPresent_returnsNull", () => {
// Cache was wiped (e.g. sign-out race). Must not fall back to live channel.
const channelB = makeChannel("channel-B");
const result = resolveEffectiveChannel("channel-A", [], channelB);
assert.strictEqual(result, null);
});
// ---------------------------------------------------------------------------
// resolveThreadReplyTarget — flush-time resolution for handleSendThreadReply
//
// These tests exercise the production resolveThreadReplyTarget function.
// Key invariant (the race): when a captured context is provided, the live
// ref values (liveReplyTargetId, liveThreadHeadId) must be IGNORED even if
// they point at a different thread — they represent post-navigation state.
// ---------------------------------------------------------------------------
test("resolveThreadReplyTarget_capturedContext_ignoresLiveRefs", () => {
// The race scenario: compose-time context captured A, live refs now point
// at B (user switched threads mid-send).
const result = resolveThreadReplyTarget(
{ parentEventId: "parent-A", threadHeadId: "head-A" },
/* liveReplyTargetId = */ "parent-B",
/* liveThreadHeadId = */ "head-B",
);
assert.deepStrictEqual(result, {
parentEventId: "parent-A",
threadHeadId: "head-A",
});
});
test("resolveThreadReplyTarget_capturedContextNullParent_returnsNull", () => {
// Captured context has no parentEventId — bail before any await fires.
const result = resolveThreadReplyTarget(
{ parentEventId: null, threadHeadId: "head-A" },
"live-parent",
"live-head",
);
assert.strictEqual(result, null);
});
test("resolveThreadReplyTarget_capturedContextNullThreadHead_usesItNotLiveRef", () => {
// F7 degenerate case: threadContext is non-null but threadHeadId is null.
// Must not fall through to the live ref — use null from the context itself.
const result = resolveThreadReplyTarget(
{ parentEventId: "parent-A", threadHeadId: null },
"live-parent",
"live-head",
);
assert.deepStrictEqual(result, {
parentEventId: "parent-A",
threadHeadId: null,
});
});
test("resolveThreadReplyTarget_nullContext_fallsBackToLiveRefs", () => {
// Legacy path: no captured context — fall back to live refs.
const result = resolveThreadReplyTarget(
null,
/* liveReplyTargetId = */ "live-parent",
/* liveThreadHeadId = */ "live-head",
);
assert.deepStrictEqual(result, {
parentEventId: "live-parent",
threadHeadId: "live-head",
});
});
test("resolveThreadReplyTarget_nullContext_noLiveReplyTarget_fallsBackToThreadHead", () => {
// When there is no specific reply target (just the thread head), parentEventId
// equals the thread head.
const result = resolveThreadReplyTarget(
null,
/* liveReplyTargetId = */ null,
/* liveThreadHeadId = */ "head-only",
);
assert.deepStrictEqual(result, {
parentEventId: "head-only",
threadHeadId: "head-only",
});
});
test("resolveThreadReplyTarget_nullContext_noLiveRefs_returnsNull", () => {
// No context and no live refs — bail.
const result = resolveThreadReplyTarget(null, null, null);
assert.strictEqual(result, null);
});
@@ -86,10 +86,25 @@ type MessageComposerProps = {
*/
onEditLastOwnMessage?: () => boolean;
onEditSave?: (content: string, mediaTags?: string[][]) => Promise<void>;
/**
* Called synchronously at the start of `submitMessage`, before any awaits,
* to capture context that must be stable throughout the async send pipeline.
* Used by the thread-reply composer to capture the current reply target before
* the mention-flow awaits can change navigation state.
*/
onCaptureSendContext?: () => {
parentEventId: string | null;
threadHeadId: string | null;
} | null;
onSend: (
content: string,
mentionPubkeys: string[],
mediaTags?: string[][],
channelId?: string | null,
threadContext?: {
parentEventId: string | null;
threadHeadId: string | null;
} | null,
) => Promise<void>;
placeholder?: string;
profiles?: UserProfileLookup;
@@ -115,6 +130,7 @@ function MessageComposerImpl({
isSending = false,
onCancelEdit,
onCancelReply,
onCaptureSendContext,
onEditLastOwnMessage,
onEditSave,
onSend,
@@ -570,13 +586,26 @@ function MessageComposerImpl({
return;
}
const capturedThreadContext = onCaptureSendContext?.() ?? null;
// If a thread-reply composer reported no reply target at submit time,
// bail here rather than discovering the null later after async awaits.
if (
capturedThreadContext !== null &&
!capturedThreadContext.parentEventId
) {
return;
}
await mentionSendFlow.sendMessageWithMentionFlow({
capturedChannelId: channelId,
capturedThreadContext,
pendingImeta: currentPendingImeta,
sentDraftKey: effectiveDraftKeyRef.current,
spoileredAttachmentUrls,
trimmed,
});
}, [
channelId,
channelLinks.clearChannels,
customEmoji,
emojiAutocomplete.clearEmojis,
@@ -590,6 +619,7 @@ function MessageComposerImpl({
setComposerContent,
spoileredAttachmentUrls,
syncComposerContentFromEditor,
onCaptureSendContext,
]);
submitMessageRef.current = submitMessage;
@@ -69,6 +69,11 @@ type MessageThreadPanelProps = {
content: string,
mentionPubkeys: string[],
mediaTags?: string[][],
channelId?: string | null,
threadContext?: {
parentEventId: string | null;
threadHeadId: string | null;
} | null,
) => Promise<void>;
onToggleReaction?: (
message: TimelineMessage,
@@ -340,6 +345,19 @@ export function MessageThreadPanel({
isSinglePanelView,
);
// Live ref so onCaptureSendContext can read reply state at submit time
// (before any async mention-flow awaits change navigation state).
const replyTargetMessageRef = React.useRef(replyTargetMessage);
replyTargetMessageRef.current = replyTargetMessage;
const onCaptureSendContext = React.useCallback(
() => ({
parentEventId: replyTargetMessageRef.current?.id ?? threadHeadId,
threadHeadId,
}),
[threadHeadId],
);
const collapseThreadHeadReplies = React.useCallback(() => {
if (!threadHeadId) {
return;
@@ -849,6 +867,7 @@ export function MessageThreadPanel({
isSending={isSending}
onCancelEdit={onCancelEdit}
onCancelReply={composerReplyTarget ? onCancelReply : undefined}
onCaptureSendContext={onCaptureSendContext}
onEditLastOwnMessage={onEditLastOwnMessage}
onEditSave={onEditSave}
onSend={onSend}
@@ -27,6 +27,12 @@ import { MENTION_REFERENCE_TAG } from "@/shared/lib/resolveMentionNames";
import { buildCustomEmojiTags } from "@/shared/lib/customEmojiTags";
type PendingNonMemberMentionSend = {
capturedChannelId: string | null;
/** Thread context captured at submit time — null for main-timeline sends. */
capturedThreadContext: {
parentEventId: string | null;
threadHeadId: string | null;
} | null;
finalContent: string;
mentionPubkeys: string[];
nonMemberPubkeys: string[];
@@ -39,6 +45,12 @@ type PendingNonMemberMentionSend = {
};
type SendMessageWithMentionFlowInput = {
capturedChannelId: string | null;
/** Thread context captured at submit time — null for main-timeline sends. */
capturedThreadContext?: {
parentEventId: string | null;
threadHeadId: string | null;
} | null;
pendingImeta: ImetaMedia[];
sentDraftKey: string | null | undefined;
spoileredAttachmentUrls?: ReadonlySet<string>;
@@ -59,6 +71,11 @@ type UseMentionSendFlowOptions = {
content: string,
mentionPubkeys: string[],
mediaTags?: string[][],
channelId?: string | null,
threadContext?: {
parentEventId: string | null;
threadHeadId: string | null;
} | null,
) => Promise<void>
>;
richText: Pick<UseRichTextEditorResult, "clearContent" | "setContent">;
@@ -128,6 +145,10 @@ export function useMentionSendFlow({
const isMentionSendPendingRef = React.useRef(false);
const isCompleteSendPendingRef = React.useRef(false);
const previousChannelIdRef = React.useRef(channelId);
// Tracks the live channel so completeSend can ask "is the user still here?"
// without being frozen to the compose-time closure.
const channelIdRef = React.useRef(channelId);
channelIdRef.current = channelId;
const addMembersMutation = useAddChannelMembersMutation(channelId);
const attachAgentMutation = useAttachManagedAgentToChannelMutation(channelId);
@@ -170,8 +191,8 @@ export function useMentionSendFlow({
]);
const ensureManagedAgentMentionsReady = React.useCallback(
async (mentionPubkeys: string[]) => {
if (!channelId || mentionPubkeys.length === 0) {
async (mentionPubkeys: string[], capturedChannelId: string) => {
if (!capturedChannelId || mentionPubkeys.length === 0) {
return [];
}
@@ -195,6 +216,7 @@ export function useMentionSendFlow({
}
} else {
await attachAgentMutation.mutateAsync({
channelId: capturedChannelId,
agent,
role: "bot",
});
@@ -213,7 +235,6 @@ export function useMentionSendFlow({
},
[
attachAgentMutation,
channelId,
getManagedAgentsByPubkey,
mentions.memberPubkeys,
startAgentMutation,
@@ -221,9 +242,9 @@ export function useMentionSendFlow({
);
const createMentionedPersonaAgents = React.useCallback(
async (trimmed: string) => {
async (trimmed: string, capturedChannelId: string) => {
const personaMentions = mentions.extractMentionPersonas(trimmed);
if (!channelId || personaMentions.length === 0) {
if (!capturedChannelId || personaMentions.length === 0) {
return {
errors: [] as string[],
pubkeys: [] as string[],
@@ -254,6 +275,7 @@ export function useMentionSendFlow({
try {
const result = await createPersonaAgentMutation.mutateAsync({
channelId: capturedChannelId,
runtime,
name: persona.displayName,
personaId: persona.id,
@@ -284,7 +306,6 @@ export function useMentionSendFlow({
};
},
[
channelId,
createPersonaAgentMutation,
getAvailableRuntimes,
mentions.extractMentionPersonas,
@@ -346,6 +367,7 @@ export function useMentionSendFlow({
mentionPubkeys.filter(
(pubkey) => !readyAgentPubkeys.has(normalizePubkey(pubkey)),
),
draft.capturedChannelId ?? "",
);
if (agentReadinessErrors.length > 0) {
const message =
@@ -359,25 +381,36 @@ export function useMentionSendFlow({
return;
}
clearComposer();
// Only clear the composer if the user has not switched channels since
// submit. If they have, the composer they see belongs to the new channel
// and we must not wipe it.
if (draft.capturedChannelId === channelIdRef.current) {
clearComposer();
}
try {
await onSendRef.current(
draft.finalContent,
mentionPubkeys,
outgoingTags,
draft.capturedChannelId,
draft.capturedThreadContext,
);
if (draft.sentDraftKey) {
drafts.clearDraft(draft.sentDraftKey);
}
} catch {
setContent(draft.savedContent);
contentRef.current = draft.savedContent;
richText.setContent(draft.savedContent);
setPendingImeta(draft.savedImeta);
setSpoileredAttachmentUrls?.(
new Set(draft.savedSpoileredAttachmentUrls),
);
// Only restore the composer content if the user is still on the
// channel that originated the send.
if (draft.capturedChannelId === channelIdRef.current) {
setContent(draft.savedContent);
contentRef.current = draft.savedContent;
richText.setContent(draft.savedContent);
setPendingImeta(draft.savedImeta);
setSpoileredAttachmentUrls?.(
new Set(draft.savedSpoileredAttachmentUrls),
);
}
}
} finally {
isCompleteSendPendingRef.current = false;
@@ -416,6 +449,8 @@ export function useMentionSendFlow({
const sendMessageWithMentionFlow = React.useCallback(
async ({
capturedChannelId,
capturedThreadContext = null,
pendingImeta,
sentDraftKey,
spoileredAttachmentUrls = new Set(),
@@ -428,8 +463,10 @@ export function useMentionSendFlow({
isMentionSendPendingRef.current = true;
setIsMentionSendPending(true);
try {
const personaMentionResult =
await createMentionedPersonaAgents(trimmed);
const personaMentionResult = await createMentionedPersonaAgents(
trimmed,
capturedChannelId ?? "",
);
if (personaMentionResult.errors.length > 0) {
const message =
personaMentionResult.errors.length === 1
@@ -479,6 +516,8 @@ export function useMentionSendFlow({
}
const pendingDraft: PendingNonMemberMentionSend = {
capturedChannelId,
capturedThreadContext,
finalContent,
mentionPubkeys: pubkeys,
nonMemberPubkeys: promptNonMemberPubkeys,
@@ -578,6 +617,7 @@ export function useMentionSendFlow({
const errors: string[] = [];
if (peoplePubkeys.length > 0) {
const result = await addMembersMutation.mutateAsync({
channelId: pendingNonMemberSend.capturedChannelId ?? undefined,
pubkeys: peoplePubkeys,
role: "member",
});
@@ -586,6 +626,7 @@ export function useMentionSendFlow({
if (relayAgentPubkeys.length > 0) {
const result = await addMembersMutation.mutateAsync({
channelId: pendingNonMemberSend.capturedChannelId ?? undefined,
pubkeys: relayAgentPubkeys,
role: "bot",
});
+7
View File
@@ -91,6 +91,7 @@ type E2eConfig = {
relayAgents?: MockRelayAgentSeed[];
agentListDelayMs?: number;
agentMemory?: RawAgentMemoryListing | Record<string, RawAgentMemoryListing>;
addChannelMembersDelayMs?: number;
createManagedAgentDelayMs?: number;
channelsReadError?: string;
feedReadError?: string;
@@ -4962,6 +4963,12 @@ async function handleAddChannelMembers(
},
config: E2eConfig | undefined,
): Promise<RawAddChannelMembersResponse> {
const addChannelMembersDelayMs = config?.mock?.addChannelMembersDelayMs ?? 0;
if (addChannelMembersDelayMs > 0) {
await new Promise((resolve) =>
window.setTimeout(resolve, addChannelMembersDelayMs),
);
}
const identity = getIdentity(config);
if (!identity) {
const channel = getMockChannel(args.channelId);
@@ -0,0 +1,189 @@
/**
* E2E regression for the wrong-channel send bug.
*
* Repro: compose a message in channel A that tags a non-member managed agent
* (which forces the slow `add_channel_members` path), submit, then immediately
* switch to channel B before the agent-attach await resolves. Without the fix,
* the message lands in B's timeline; with the fix it must land in A's.
*
* The `addChannelMembersDelayMs` bridge knob holds the `add_channel_members`
* handler open long enough for the channel click to race the in-flight send.
*/
import { expect, test } from "@playwright/test";
import { installMockBridge } from "../helpers/bridge";
// A managed agent that is NOT a member of any channel in the seed data.
const OUT_OF_CHANNEL_BOT_PUBKEY =
"ee00000000000000000000000000000000000000000000000000000000000001";
/** Locator scoped to the mention autocomplete dropdown inside the composer. */
function autocomplete(page: import("@playwright/test").Page) {
return page
.getByTestId("message-composer")
.getByTestId("mention-autocomplete");
}
async function readCommandLog(page: import("@playwright/test").Page) {
return page.evaluate(() => {
return (
(window as Window & { __BUZZ_E2E_COMMANDS__?: string[] })
.__BUZZ_E2E_COMMANDS__ ?? []
);
});
}
function commandCount(commands: string[], command: string) {
return commands.filter((c) => c === command).length;
}
// The channel timeline renders off a `useDeferredValue` snapshot; poll for the
// pending marker to clear before asserting on freshly-sent content.
async function waitForTimelineSettled(page: import("@playwright/test").Page) {
await expect(page.locator("[data-render-pending]")).toHaveCount(0);
}
// ---------------------------------------------------------------------------
// Main regression: message always lands in the compose-time channel
// ---------------------------------------------------------------------------
test("message with agent mention lands in compose-time channel despite mid-send navigation", async ({
page,
}) => {
const MESSAGE_TEXT = `send-binding-repro-${Date.now()}`;
// Install bridge with:
// - a managed agent that is NOT in general (forces add_channel_members path)
// - a 500ms delay on add_channel_members to open the race window
await installMockBridge(page, {
addChannelMembersDelayMs: 500,
managedAgents: [
{
pubkey: OUT_OF_CHANNEL_BOT_PUBKEY,
name: "BotA",
status: "running",
// No channelNames → agent is not a member of any channel
},
],
});
await page.goto("/");
await page.getByTestId("channel-general").click();
await expect(page.getByTestId("chat-title")).toHaveText("general");
// Type a message that mentions the out-of-channel agent
const input = page.getByTestId("message-input");
await input.fill("@BotA");
const dropdown = autocomplete(page);
const botRow = dropdown.locator("button", { hasText: "BotA" });
await expect(botRow).toBeVisible();
await expect(botRow.getByText("not in channel")).toBeVisible();
// Select BotA from the autocomplete
await input.press("Enter");
await page.keyboard.type(` ${MESSAGE_TEXT}`);
// Verify the mention chip is present before submitting
const composerChip = input.locator(".agent-mention-highlight", {
hasText: "BotA",
});
await expect(composerChip).toBeVisible();
// Snapshot the baseline command count before sending
const baselineCommands = await readCommandLog(page);
const baselineAddCount = commandCount(
baselineCommands,
"add_channel_members",
);
// Submit the message — this triggers the async add_channel_members path
await page.getByTestId("send-message").click();
// Immediately switch to channel-agents BEFORE the 500ms delay resolves.
// This is the race the fix closes.
await page.getByTestId("channel-agents").click();
await expect(page.getByTestId("chat-title")).toHaveText("agents");
// Wait for add_channel_members to fire (confirms the race window opened and
// the fix's captured channel id was used for the agent-attach call).
await expect
.poll(async () =>
commandCount(await readCommandLog(page), "add_channel_members"),
)
.toBeGreaterThan(baselineAddCount);
// Let the in-flight send finish (500ms delay + buffer).
await page.waitForTimeout(800);
// --- Assert message landed in general (compose-time channel) ---
await page.getByTestId("channel-general").click();
await expect(page.getByTestId("chat-title")).toHaveText("general");
await waitForTimelineSettled(page);
// The message must appear in general's timeline (not the switched-to channel).
await expect(page.getByTestId("message-timeline")).toContainText(
MESSAGE_TEXT,
);
// --- Assert message did NOT land in agents (switched-to channel) ---
await page.getByTestId("channel-agents").click();
await expect(page.getByTestId("chat-title")).toHaveText("agents");
await waitForTimelineSettled(page);
await expect(page.getByTestId("message-timeline")).not.toContainText(
MESSAGE_TEXT,
);
});
// ---------------------------------------------------------------------------
// Invariant: without mid-send navigation, normal agent-mention send still works
// ---------------------------------------------------------------------------
test("message with agent mention delivers correctly when no channel switch occurs", async ({
page,
}) => {
const MESSAGE_TEXT = `no-switch-verify-${Date.now()}`;
await installMockBridge(page, {
addChannelMembersDelayMs: 0,
managedAgents: [
{
pubkey: OUT_OF_CHANNEL_BOT_PUBKEY,
name: "BotA",
status: "running",
},
],
});
await page.goto("/");
await page.getByTestId("channel-general").click();
await expect(page.getByTestId("chat-title")).toHaveText("general");
const input = page.getByTestId("message-input");
await input.fill("@BotA");
const dropdown = autocomplete(page);
await expect(dropdown.locator("button", { hasText: "BotA" })).toBeVisible();
await input.press("Enter");
await page.keyboard.type(` ${MESSAGE_TEXT}`);
const baselineCommands = await readCommandLog(page);
const baselineAddCount = commandCount(
baselineCommands,
"add_channel_members",
);
await page.getByTestId("send-message").click();
// Wait for the agent-attach step to complete before asserting the timeline.
await expect
.poll(async () =>
commandCount(await readCommandLog(page), "add_channel_members"),
)
.toBeGreaterThan(baselineAddCount);
await waitForTimelineSettled(page);
await expect(page.getByTestId("message-timeline")).toContainText(
MESSAGE_TEXT,
);
});
+1
View File
@@ -110,6 +110,7 @@ type MockBridgeOptions = {
relayAgents?: MockRelayAgentSeed[];
agentListDelayMs?: number;
createManagedAgentDelayMs?: number;
addChannelMembersDelayMs?: number;
channelsReadError?: string;
feedReadError?: string;
canvasReadError?: string;