From acde6dcb852ab9b5a54b1c3fc646660475b64ce3 Mon Sep 17 00:00:00 2001 From: Seydi Charyyev Date: Sat, 8 Aug 2026 08:16:35 +0500 Subject: [PATCH] fix(desktop): keep publishing a mention when a channel agent fails to start ensureManagedAgentMentionsReady collects every agent-preparation failure into one list, and any entry in it returns before the send, so the mention is gone. That is right when the agent is not in the channel yet, because nothing else would put it there. It is wrong when the agent is already a member: an agent that runs outside this desktop has no private key on this machine and never will, so its launch fails every time while it sits in the channel and would answer. A preparation failure now blocks only when the agent is not already a member of the target channel. For a member the local start is an optimization, so the message is published and the failure is reported instead. That notice is emitted before Huddle sync, media upload and the send itself, any of which can still abort, so it is worded pre-send ("sending the mention anyway") and raised with toast.warning. Past-tense copy there would be a false delivery confirmation on an error path. Blocking behaviour is unchanged for a failed attach and for an agent only prepared for a channel this send creates or expands, which is what channels.spec.ts pins for expanded DMs. Closes #5099. Signed-off-by: Seydi Charyyev --- .../ui/useMentionSendFlow.helpers.test.mjs | 90 +++++++++++++++++ .../messages/ui/useMentionSendFlow.helpers.ts | 55 +++++++++++ .../messages/ui/useMentionSendFlow.ts | 28 +++--- desktop/tests/e2e/mentions.spec.ts | 98 +++++++++++++++++++ 4 files changed, 257 insertions(+), 14 deletions(-) create mode 100644 desktop/src/features/messages/ui/useMentionSendFlow.helpers.test.mjs diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.test.mjs b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.test.mjs new file mode 100644 index 0000000000..a783240e83 --- /dev/null +++ b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.test.mjs @@ -0,0 +1,90 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { describeAgentReadinessFailures } from "./useMentionSendFlow.helpers.ts"; + +test("describeAgentReadinessFailures: no failures blocks nothing and warns about nothing", () => { + assert.deepEqual(describeAgentReadinessFailures([]), { + blocking: null, + warning: null, + }); +}); + +test("describeAgentReadinessFailures: a launch failure for a channel member only warns", () => { + // #5099: the agent runs on the user's own server, so this desktop holds no + // key for it and the launch can never succeed. It is in the channel and + // answers there, so the mention must still go out. + const result = describeAgentReadinessFailures([ + { blocking: false, message: "Backend · Claude: agent has no private key" }, + ]); + + assert.equal(result.blocking, null); + assert.equal( + result.warning, + "Could not start the mentioned agent; sending the mention anyway: Backend · Claude: agent has no private key", + ); +}); + +test("describeAgentReadinessFailures: the warning never claims the message was sent", () => { + // The notice is emitted before Huddle sync, media upload and the send, each + // of which can still abort. Past-tense copy here would be a false delivery + // confirmation on an error path. + for (const count of [1, 2]) { + const { warning } = describeAgentReadinessFailures( + Array.from({ length: count }, (_, index) => ({ + blocking: false, + message: `Agent ${index}: no private key`, + })), + ); + + assert.match(warning, /sending the mention anyway/); + assert.doesNotMatch(warning, /\bsent\b/i); + assert.doesNotMatch(warning, /\bdelivered\b/i); + } +}); + +test("describeAgentReadinessFailures: a blocking failure keeps stopping the send", () => { + const result = describeAgentReadinessFailures([ + { blocking: true, message: "Fizz: Mock agent startup failed." }, + ]); + + assert.equal( + result.blocking, + "Could not start agent mention: Fizz: Mock agent startup failed.", + ); + assert.equal(result.warning, null); +}); + +test("describeAgentReadinessFailures: a blocking failure wins over a warning", () => { + const result = describeAgentReadinessFailures([ + { blocking: false, message: "Remote: no private key" }, + { blocking: true, message: "Fizz: startup failed" }, + ]); + + assert.equal( + result.blocking, + "Could not start agent mention: Fizz: startup failed", + ); + assert.equal( + result.warning, + "Could not start the mentioned agent; sending the mention anyway: Remote: no private key", + ); +}); + +test("describeAgentReadinessFailures: several failures of one kind are joined and pluralised", () => { + const result = describeAgentReadinessFailures([ + { blocking: true, message: "A: one" }, + { blocking: true, message: "B: two" }, + { blocking: false, message: "C: three" }, + { blocking: false, message: "D: four" }, + ]); + + assert.equal( + result.blocking, + "Could not start agent mentions: A: one; B: two", + ); + assert.equal( + result.warning, + "Could not start the mentioned agents; sending the mention anyway: C: three; D: four", + ); +}); diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts index b2eb3893b7..7f50694edb 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts @@ -75,3 +75,58 @@ export function isManagedAgentRunning(agent: ManagedAgent) { export function isProviderBackedAgent(agent: ManagedAgent) { return agent.backend.type === "provider"; } + +/** + * A mentioned agent that could not be prepared for the send. + * + * `blocking` is false only when the agent is already a member of the channel + * the message is going to. Launching it on this desktop is then an + * optimization: an agent that runs elsewhere — a container on the user's own + * server — has no private key on this machine and never will, so its launch + * fails every time, yet it is in the channel and still answers. Refusing to + * publish loses a valid mention and fixes nothing. + * + * Everything else blocks, because nothing else would put the agent in the + * channel: a failed attach leaves it outside, and a failed launch for an agent + * that is only being prepared for a channel this send creates or expands + * leaves that channel without the participant it was expanded for. + */ +export type AgentReadinessFailure = { + blocking: boolean; + message: string; +}; + +/** + * Build the send-blocking message and the send-anyway notice, or null. + * + * The non-blocking copy is deliberately pre-send tense. It is shown before + * Huddle sync, media upload and the send itself, any of which can still fail + * and abort, so it must not claim the message was delivered. + */ +export function describeAgentReadinessFailures( + failures: readonly AgentReadinessFailure[], +) { + const describe = ( + singular: string, + plural: string, + selected: readonly AgentReadinessFailure[], + ) => + selected.length === 0 + ? null + : `${selected.length === 1 ? singular : plural}: ${selected + .map((failure) => failure.message) + .join("; ")}`; + + return { + blocking: describe( + "Could not start agent mention", + "Could not start agent mentions", + failures.filter((failure) => failure.blocking), + ), + warning: describe( + "Could not start the mentioned agent; sending the mention anyway", + "Could not start the mentioned agents; sending the mention anyway", + failures.filter((failure) => !failure.blocking), + ), + }; +} diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.ts b/desktop/src/features/messages/ui/useMentionSendFlow.ts index cc51c73345..b583b86d5f 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.ts @@ -38,6 +38,8 @@ import type { AcpRuntime, ChannelType, ManagedAgent } from "@/shared/api/types"; import { normalizePubkey, truncatePubkey } from "@/shared/lib/pubkey"; import { buildCustomEmojiTags } from "@/shared/lib/customEmojiTags"; import { + type AgentReadinessFailure, + describeAgentReadinessFailures, getErrorMessage, isManagedAgentRunning, isProviderBackedAgent, @@ -183,7 +185,7 @@ export function useMentionSendFlow({ ) => { if (!capturedChannelId || mentionPubkeys.length === 0) { return { - errors: [] as string[], + errors: [] as AgentReadinessFailure[], pubkeys: [] as string[], }; } @@ -195,13 +197,14 @@ export function useMentionSendFlow({ ...mentions.memberPubkeys, ...preparedParticipantPubkeys.map(normalizePubkey), ]); - const errors: string[] = []; + const errors: AgentReadinessFailure[] = []; const pubkeys: string[] = []; for (const pubkey of uniqueNormalizedPubkeys(mentionPubkeys)) { const agent = managedAgentsByPubkey.get(pubkey); if (!agent) { continue; } + const isExistingMember = mentions.memberPubkeys.has(pubkey); try { if (participantPubkeys.has(pubkey)) { if (isProviderBackedAgent(agent)) { @@ -220,12 +223,13 @@ export function useMentionSendFlow({ } pubkeys.push(pubkey); } catch (error) { - errors.push( - `${agent.name}: ${getErrorMessage( + errors.push({ + blocking: !isExistingMember, + message: `${agent.name}: ${getErrorMessage( error, "Could not prepare agent.", )}`, - ); + }); } } return { @@ -452,17 +456,13 @@ export function useMentionSendFlow({ persistPreflightDraft(); return; } - if (agentReadiness.errors.length > 0) { - const message = - agentReadiness.errors.length === 1 - ? `Could not start agent mention: ${agentReadiness.errors[0]}` - : `Could not start agent mentions: ${agentReadiness.errors.join( - "; ", - )}`; - setNonMemberPromptError(message); - toast.error(message); + const readiness = describeAgentReadinessFailures(agentReadiness.errors); + if (readiness.blocking) { + setNonMemberPromptError(readiness.blocking); + toast.error(readiness.blocking); return; } + if (readiness.warning) toast.warning(readiness.warning); if (preparedAgentPubkeys.length > 0 && sendChannelId) { try { diff --git a/desktop/tests/e2e/mentions.spec.ts b/desktop/tests/e2e/mentions.spec.ts index ed00e8c355..a4d201f63d 100644 --- a/desktop/tests/e2e/mentions.spec.ts +++ b/desktop/tests/e2e/mentions.spec.ts @@ -1185,6 +1185,104 @@ test("mentioning an in-channel provider managed agent deploys it before sending" await expect(mentionChip).toBeVisible(); }); +test("mentioning an in-channel agent still sends when its start fails", async ({ + page, +}) => { + // #5099: the agent runs on the user's own server, so this desktop holds no + // key for it and the launch can never succeed. It is a member of the channel + // and answers there, so the mention has to be published anyway. + const startError = "agent has no private key available"; + await installMockBridge(page, { + managedAgents: [ + { + pubkey: OUT_OF_CHANNEL_PROVIDER_AGENT_PUBKEY, + name: "portal", + status: "not_deployed", + channelNames: ["general"], + backend: { + type: "provider", + id: "portal", + config: { region: "test" }, + }, + }, + ], + startManagedAgentErrors: [startError], + }); + 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("Hey @portal"); + + const dropdown = autocomplete(page); + await expect(dropdown.getByText("portal")).toBeVisible(); + await input.press("Enter"); + await page.keyboard.type(" can you help?"); + await page.getByTestId("send-message").click(); + + await expect( + page.getByText(startError, { exact: false }).first(), + ).toBeVisible(); + const mentionChip = page + .getByTestId("message-row") + .last() + .locator("[data-mention].agent-mention-highlight", { hasText: "portal" }); + await expect(mentionChip).toBeVisible(); +}); + +test("a start failure never reports delivery when the send itself fails", async ({ + page, +}) => { + // The start notice is emitted before Huddle sync, media upload and the send, + // so it has to stay pre-send tense: a later abort must not leave the user + // with a message that claims the mention went out. + const startError = "agent has no private key available"; + const sendError = "Mock send failed."; + await installMockBridge(page, { + managedAgents: [ + { + pubkey: OUT_OF_CHANNEL_PROVIDER_AGENT_PUBKEY, + name: "portal", + status: "not_deployed", + channelNames: ["general"], + backend: { + type: "provider", + id: "portal", + config: { region: "test" }, + }, + }, + ], + startManagedAgentErrors: [startError], + sendMessageErrors: [sendError], + }); + 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("Hey @portal"); + + const dropdown = autocomplete(page); + await expect(dropdown.getByText("portal")).toBeVisible(); + await input.press("Enter"); + await page.keyboard.type(" can you help?"); + await page.getByTestId("send-message").click(); + + await expect( + page.getByText("sending the mention anyway", { exact: false }).first(), + ).toBeVisible(); + // The send aborted: the composer keeps the draft and nothing reached the + // timeline, so the notice above must not have claimed delivery. + await expect(input).toContainText("portal"); + await expect( + page + .getByTestId("message-row") + .locator("[data-mention].agent-mention-highlight", { hasText: "portal" }), + ).toHaveCount(0); + await expect(page.getByText(/message sent/i)).toHaveCount(0); +}); + test("mentioning a non-member managed agent adds and starts it before sending", async ({ page, }) => {