Skip to content

Commit a49846a

Browse files
committed
fix(desktop): settle pasted recipients within the source draft visit
Capture recipients after clipboard verification without borrowing a newer authored draft. Preserve ordinary post-capture destination-bound sends. Align inherited revocation tests with fail-closed publication, and pin actual recipients in the detached duplicate-wake test. Signed-off-by: Logan Johnson <loganj@squareup.com>
1 parent 331cb66 commit a49846a

4 files changed

Lines changed: 147 additions & 37 deletions

File tree

desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs

Lines changed: 88 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -473,19 +473,23 @@ test("ordinary send may clean its untouched source even if navigation preceded o
473473
s.calls.push(["mark-sent", ...args]);
474474
const gate = deferred();
475475
s.control.publish = gate;
476+
// Hold destination preparation AFTER clipboard settlement has captured the
477+
// source selections; navigation during settlement now cancels before capture.
478+
const preparation = deferred();
479+
s.options.onPrepareSendChannel = () => preparation.promise;
476480
s.rerender();
477481
let send;
478-
s.act(() => {
482+
await s.act(async () => {
479483
send = s.result.current.sendMessageWithMentionFlow({
480-
capturedChannelId: "general",
484+
capturedChannelId: null,
481485
pendingImeta: [],
482486
trimmed: TEXT,
483487
recoveryDraftKey: "thread:a",
484488
sentDraftKey: "thread:a",
485489
});
486-
// Preparation yields before completeSend can clear the composer.
487-
s.navigate("thread:b");
488490
});
491+
s.navigate("thread:b");
492+
preparation.resolve("general");
489493
await s.flush();
490494
assert.equal(s.events("publish").length, 1);
491495
assert.equal(s.store.get("thread:a").content, TEXT);
@@ -520,3 +524,83 @@ for (const upload of [false, true]) {
520524
assert.equal(s.result.current.isPreparingMentionSend, false);
521525
});
522526
}
527+
528+
// Clipboard verification is the only pre-capture await. Later preparation
529+
// consumes a snapshot; this await must instead fence the maps before reading.
530+
test("settled paste supplies exact recipient and recovery refs before preparation", async () => {
531+
const s = await setup({ lifecycle: true });
532+
s.dismiss();
533+
const gate = deferred();
534+
const pastedRefs = [
535+
{ displayName: "RemoteScout", pubkey: "c".repeat(64), isAgent: true },
536+
];
537+
s.options.mentions.settlePendingMentionBindings = async () => {
538+
await gate.promise;
539+
s.control.currentRefs = pastedRefs;
540+
};
541+
s.options.mentions.extractMentionPubkeys = () =>
542+
s.control.currentRefs.map((ref) => ref.pubkey);
543+
s.rerender();
544+
let sending;
545+
await s.act(async () => {
546+
sending = s.result.current.sendMessageWithMentionFlow({
547+
capturedChannelId: "general",
548+
trimmed: TEXT,
549+
pendingImeta: [],
550+
});
551+
});
552+
await s.finish(gate);
553+
await sending;
554+
await s.invite();
555+
assert.deepEqual(Array.from(s.events("SEND")[0][2]), [pastedRefs[0].pubkey]);
556+
});
557+
558+
for (const action of ["edit", "delete", "navigation", "return", "unmount"]) {
559+
test(`paste settlement after ${action} cannot read another draft or publish`, async () => {
560+
const s = await setup({ lifecycle: true });
561+
s.dismiss();
562+
const gate = deferred();
563+
s.options.mentions.settlePendingMentionBindings = () => gate.promise;
564+
s.rerender();
565+
let sending;
566+
await s.act(async () => {
567+
sending = s.result.current.sendMessageWithMentionFlow({
568+
capturedChannelId: "general",
569+
trimmed: TEXT,
570+
pendingImeta: [],
571+
});
572+
});
573+
const otherRefs = [
574+
{ displayName: "RemoteScout", pubkey: "c".repeat(64), isAgent: true },
575+
];
576+
if (action === "edit") s.edit(TEXT, otherRefs);
577+
if (action === "delete") s.edit("");
578+
if (action === "navigation" || action === "return") {
579+
s.store.set("thread:b", {
580+
content: TEXT,
581+
channelId: "general",
582+
pendingImeta: [],
583+
spoileredAttachmentUrls: [],
584+
mentionRefs: otherRefs,
585+
});
586+
s.navigate("thread:b");
587+
if (action === "return") s.navigate("thread:a");
588+
}
589+
if (action === "unmount") s.unmount();
590+
let reads = 0;
591+
s.options.mentions.getDraftMentionRefs = () => {
592+
reads++;
593+
return otherRefs;
594+
};
595+
await s.finish(gate);
596+
await sending;
597+
assert.equal(reads, 0);
598+
assert.equal(s.events("add").length, 0);
599+
assert.equal(s.events("persona").length, 0);
600+
assert.equal(s.events("SEND").length, 0);
601+
assert.equal(s.result.current.nonMemberPromptProps.open, false);
602+
if (action === "delete") assert.equal(s.options.contentRef.current, "");
603+
if (action === "edit" || action === "navigation")
604+
assert.deepEqual(s.control.currentRefs, otherRefs);
605+
});
606+
}

desktop/src/features/messages/ui/useMentionSendFlow.ts

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -741,13 +741,9 @@ export function useMentionSendFlow({
741741
}
742742
isMentionSendPendingRef.current = true;
743743
setIsMentionSendPending(true);
744-
// Capture exact selections before any async preparation can navigate the
745-
// reused editor to another draft (possibly with identical display text).
744+
// Bind settlement to this authored visit before reading its recipients.
746745
claimDraftSend(effectiveDraftKey);
747746
const composerRevision = getComposerRevision();
748-
const savedMentionRefs = mentions.getDraftMentionRefs(trimmed).slice();
749-
const selectedMentionPubkeys = mentions.extractMentionPubkeys(trimmed);
750-
const selectedPersonas = mentions.extractMentionPersonas(trimmed);
751747
const isSendCancelled = () =>
752748
preparedLinkPreviews?.signal.aborted === true;
753749
let sendPromoted = false;
@@ -762,7 +758,18 @@ export function useMentionSendFlow({
762758
// publish a readable `@Label` with no `p` tag. Bounded inside, so a
763759
// lookup that never answers delays the send rather than blocking it.
764760
await mentions.settlePendingMentionBindings();
765-
if (isSendCancelled()) return;
761+
// Settlement may outlive an edit or A → B → A navigation. In that
762+
// case the live mention maps no longer belong to this send.
763+
if (
764+
isSendCancelled() ||
765+
!isMountedRef.current ||
766+
sourceOwnerRef.current !== sourceOwner ||
767+
getComposerRevision() !== composerRevision
768+
)
769+
return;
770+
const savedMentionRefs = mentions.getDraftMentionRefs(trimmed).slice();
771+
const selectedMentionPubkeys = mentions.extractMentionPubkeys(trimmed);
772+
const selectedPersonas = mentions.extractMentionPersonas(trimmed);
766773
const dmThreadAgentMentionErrorMessage = dmThreadAgentMentionError({
767774
trimmed,
768775
isThreadReply: capturedThreadContext != null,

desktop/tests/e2e/mentions.spec.ts

Lines changed: 41 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2204,7 +2204,7 @@ test("deferred-upload sends revalidate agent authorization at the publish bounda
22042204
page,
22052205
}) => {
22062206
// A background media upload can hold the publish open for arbitrarily long —
2207-
// authorization revoked during that window must still strip the p tag. This
2207+
// authorization revoked during that window must block publication. This
22082208
// pins the publish-boundary revalidation on the deferred path.
22092209
await installMockBridge(page, {
22102210
deferredComposerUploads: true,
@@ -2289,24 +2289,29 @@ test("deferred-upload sends revalidate agent authorization at the publish bounda
22892289
});
22902290

22912291
const outgoingContent = `@quinn hello\n![video](https://mock.relay/media/${"c".repeat(64)}.mp4)`;
2292-
await expect
2293-
.poll(() => readOutgoingMentionPubkeys(page, outgoingContent))
2294-
.not.toBeNull();
2295-
await expect
2296-
.poll(() => readOutgoingMentionPubkeys(page, outgoingContent))
2297-
.not.toContain(ALLOWLIST_RELAY_AGENT_PUBKEY);
2292+
await expect(
2293+
page.getByText("Could not authorize a mentioned agent.", { exact: false }),
2294+
).toBeVisible();
2295+
expect(await readOutgoingMentionPubkeys(page, outgoingContent)).toBeNull();
2296+
await expect(input).toHaveText("@quinn hello");
2297+
await expect(
2298+
page.getByTestId("composer-queued-media-attachment"),
2299+
).toBeVisible();
22982300
const commands = await readCommandLog(page);
22992301
expect(commandCount(commands, "revalidate_relay_agents")).toBe(
23002302
commandCount(baselineCommands, "revalidate_relay_agents") + 2,
23012303
);
2304+
expect(commandCount(commands, "start_managed_agent")).toBe(
2305+
commandCount(baselineCommands, "start_managed_agent"),
2306+
);
23022307
});
23032308

23042309
test("sends that attach a mentioned agent revalidate at the publish boundary", async ({
23052310
page,
23062311
}) => {
23072312
// The awaited membership write for a non-member managed agent is a relay
23082313
// round-trip between the pre-side-effect authorization pass and the publish
2309-
// — authorization revoked during that window must still strip the p tag.
2314+
// — authorization revoked during that window must block publication.
23102315
await installMockBridge(page, {
23112316
managedAgents: [
23122317
{
@@ -2385,15 +2390,13 @@ test("sends that attach a mentioned agent revalidate at the publish boundary", a
23852390
window.__BUZZ_E2E__.mock.relayAgentRevalidationRevokedPubkeys = [pubkey];
23862391
}, ALLOWLIST_RELAY_AGENT_PUBKEY);
23872392

2388-
await expect
2389-
.poll(() => readOutgoingMentionPubkeys(page, "@quinn @fizz hello"))
2390-
.not.toBeNull();
2391-
const outgoingPubkeys = await readOutgoingMentionPubkeys(
2392-
page,
2393-
"@quinn @fizz hello",
2394-
);
2395-
expect(outgoingPubkeys).toContain(OUT_OF_CHANNEL_MANAGED_AGENT_PUBKEY);
2396-
expect(outgoingPubkeys).not.toContain(ALLOWLIST_RELAY_AGENT_PUBKEY);
2393+
await expect(
2394+
page.getByText("Could not authorize a mentioned agent.", { exact: false }),
2395+
).toBeVisible();
2396+
expect(
2397+
await readOutgoingMentionPubkeys(page, "@quinn @fizz hello"),
2398+
).toBeNull();
2399+
await expect(input).toHaveText("@quinn @fizz hello");
23972400
const commands = await readCommandLog(page);
23982401
expect(commandCount(commands, "revalidate_relay_agents")).toBe(
23992402
commandCount(baselineCommands, "revalidate_relay_agents") + 2,
@@ -2403,6 +2406,9 @@ test("sends that attach a mentioned agent revalidate at the publish boundary", a
24032406
expect(commandCount(commands, "update_managed_agent")).toBe(
24042407
commandCount(baselineCommands, "update_managed_agent"),
24052408
);
2409+
expect(commandCount(commands, "start_managed_agent")).toBe(
2410+
commandCount(baselineCommands, "start_managed_agent"),
2411+
);
24062412
});
24072413

24082414
test("sends that enroll agents into an active huddle revalidate at the publish boundary", async ({
@@ -2477,7 +2483,7 @@ test("a send held open by a no-write step still revalidates at the publish bound
24772483
// here the only thing separating the authorization pass from the publish is
24782484
// the huddle sync — which with no active huddle writes nothing to the relay
24792485
// — and the revocation is released with zero further hold. A revocation
2480-
// landing in any admission-to-publish gap must strip the p tag; this is the
2486+
// landing in any admission-to-publish gap must block publication; this is the
24812487
// reviewer's sub-threshold probe of the since-removed elapsed-time bound,
24822488
// which deliberately accepted this very staleness.
24832489
await installMockBridge(page, {
@@ -2551,12 +2557,11 @@ test("a send held open by a no-write step still revalidates at the publish bound
25512557
)
25522558
.toBeGreaterThan(0);
25532559

2554-
await expect
2555-
.poll(() => readOutgoingMentionPubkeys(page, "@quinn hello"))
2556-
.not.toBeNull();
2557-
await expect
2558-
.poll(() => readOutgoingMentionPubkeys(page, "@quinn hello"))
2559-
.not.toContain(ALLOWLIST_RELAY_AGENT_PUBKEY);
2560+
await expect(
2561+
page.getByText("Could not authorize a mentioned agent.", { exact: false }),
2562+
).toBeVisible();
2563+
expect(await readOutgoingMentionPubkeys(page, "@quinn hello")).toBeNull();
2564+
await expect(input).toHaveText("@quinn hello");
25602565

25612566
const commands = await readCommandLog(page);
25622567
expect(commandCount(commands, "revalidate_relay_agents")).toBe(
@@ -3048,7 +3053,9 @@ test("a second mention while the first wake is in flight does not start the agen
30483053
await input.fill("Hey @fizz");
30493054
await expect(dropdown.getByText("fizz")).toBeVisible();
30503055
await input.press("Enter");
3051-
await page.keyboard.type(" do X");
3056+
await expect(input.locator(".mention-chip")).toHaveText("fizz");
3057+
await page.keyboard.type("do X");
3058+
await expect(input).toHaveText("Hey @fizz do X");
30523059
await page.getByTestId("send-message").click();
30533060
await expect(
30543061
page.getByTestId("message-row").filter({ hasText: "do X" }),
@@ -3062,14 +3069,22 @@ test("a second mention while the first wake is in flight does not start the agen
30623069
await input.fill("Hey @fizz");
30633070
await expect(dropdown.getByText("fizz")).toBeVisible();
30643071
await input.press("Enter");
3065-
await page.keyboard.type(" also Y");
3072+
await expect(input.locator(".mention-chip")).toHaveText("fizz");
3073+
await page.keyboard.type("also Y");
3074+
await expect(input).toHaveText("Hey @fizz also Y");
30663075
await page.getByTestId("send-message").click();
30673076

30683077
// The second message publishes on its own — suppression is of the wake, not
30693078
// of the send; the composer is never gated on a pending start again.
30703079
await expect(
30713080
page.getByTestId("message-row").filter({ hasText: "also Y" }),
30723081
).toBeVisible();
3082+
expect(await readOutgoingMentionPubkeys(page, "Hey @fizz do X")).toContain(
3083+
IN_CHANNEL_MANAGED_AGENT_PUBKEY,
3084+
);
3085+
expect(await readOutgoingMentionPubkeys(page, "Hey @fizz also Y")).toContain(
3086+
IN_CHANNEL_MANAGED_AGENT_PUBKEY,
3087+
);
30733088
// One wake serves both messages: its replay floor predates the first
30743089
// message, and the floor is a lower bound, so one harness boot covers both.
30753090
expect(commandCount(await readCommandLog(page), "start_managed_agent")).toBe(

docs/remote-mention-routing.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,11 @@ completion cannot revive that recovery or release a newer attempt's latch.
6666
The editor's authored revision distinguishes an intentional edit → clear from an
6767
optimistic empty composer. Programmatic send clear/recovery runs inside the draft
6868
lifecycle's restoration boundary, so it does not mark the source as authoritatively
69-
deleted. Exact selected mention refs are captured before asynchronous preparation.
69+
deleted. Pending clipboard identity verification settles before exact selected
70+
mention refs are captured. The source visit and authored revision are captured
71+
before that wait: an edit, navigation (including A → B → A), or unmount cancels
72+
rather than reading the new draft's maps. Subsequent asynchronous preparation
73+
consumes the captured selections.
7074
Persona preparation similarly consumes captured selections and returns resolved
7175
refs, writing them into the editor only while the original visit/revision remains
7276
current. Normal persona creation/reuse and ordinary destination-bound background

0 commit comments

Comments
 (0)