From 05d53c553528b681b88a2108f656f6636e5c8c32 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Fri, 28 Aug 2026 23:03:02 +0200 Subject: [PATCH] fix(client): surface cancelled actions and protect the released composer Track cancellation separately from failure in grouped tool metadata and name it in the group header. A stopped step is settled and not an error, so the group auto-collapsed under a success-sounding header with the only cancellation notice hidden inside the panel it had just closed. Preserve an ordinary unsent message across an ask handoff. With Save drafts off, collapsing a question releases the composer; typing there and then moving the question back overwrote that text with the answer and lost it. The released text is now stashed per pending action and handed back on the next collapse. Skip the named view transition for batched questions. Only the single-question surfaces declare view-transition-name: ask-question, so wrapping a batch handoff gave the browser nothing to pair and it cross-faded the whole root instead of morphing the question. --- .../Chat/Messages/Content/ToolCallGroup.tsx | 33 ++++++- .../Content/__tests__/ToolCallGroup.test.tsx | 35 ++++++++ client/src/hooks/Input/useAskAnswerMode.ts | 87 +++++++++++++++++-- client/src/locales/en/translation.json | 2 + 4 files changed, 148 insertions(+), 9 deletions(-) diff --git a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx index d7462f03c6..38f89956a9 100644 --- a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx +++ b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx @@ -40,6 +40,7 @@ interface ToolMeta { iconName: string; hasOutput: boolean; failed: boolean; + cancelled: boolean; } function hasFailedOutput(output: unknown): boolean { @@ -60,9 +61,9 @@ function resolveOutcome( runStepStatus: PartMetadata['runStepStatus'], completed: boolean, hasError: boolean, -): Pick { +): Pick { if (runStepStatus == null) { - return { hasOutput: completed, failed: hasError }; + return { hasOutput: completed, failed: hasError, cancelled: false }; } const phase = resolveToolCallPhase({ runStepStatus, @@ -71,7 +72,15 @@ function resolveOutcome( isSubmitting: false, hasError, }); - return { hasOutput: phase !== 'running', failed: phase === 'failed' }; + return { + hasOutput: phase !== 'running', + failed: phase === 'failed', + /** Tracked separately from `failed`: a stopped step is settled and not an + * error, but the group still must not auto-collapse under a + * success-sounding header with the only cancellation notice hidden in the + * panel. */ + cancelled: phase === 'cancelled', + }; } function hasPendingAuth(part: TMessageContentParts): boolean { @@ -286,6 +295,7 @@ export default function ToolCallGroup({ let subagentCount = 0; let askQuestionCount = 0; let failedCount = 0; + let cancelledCount = 0; for (const tool of toolMetadata) { if (tool.name === Tools.web_search) { @@ -300,6 +310,9 @@ export default function ToolCallGroup({ if (tool.failed) { failedCount++; } + if (tool.cancelled) { + cancelledCount++; + } if (!tool.name) { continue; } @@ -321,6 +334,7 @@ export default function ToolCallGroup({ subagentCount, askQuestionCount, failedCount, + cancelledCount, toolNameSummary, }; }, [toolMetadata, localize, mcpServerNames]); @@ -529,6 +543,19 @@ export default function ToolCallGroup({ ), ); } + /** A stopped action is settled but not successful, and its only other notice + * lives inside the panel the group is about to collapse, so the header has + * to say so. */ + if (activitySummary.cancelledCount > 0) { + groupDetailParts.push( + localize( + activitySummary.cancelledCount === 1 + ? 'com_ui_one_action_cancelled' + : 'com_ui_n_actions_cancelled', + { 0: String(activitySummary.cancelledCount) }, + ), + ); + } const groupDetail = groupDetailParts.filter(Boolean).join(' · '); const groupAriaLabel = [groupLabel, groupDetail, hasReasoning ? localize('com_ui_thoughts') : ''] .filter(Boolean) diff --git a/client/src/components/Chat/Messages/Content/__tests__/ToolCallGroup.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/ToolCallGroup.test.tsx index f207616816..c9895bc5db 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ToolCallGroup.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/ToolCallGroup.test.tsx @@ -26,6 +26,12 @@ jest.mock('~/hooks', () => ({ if (key === 'com_ui_one_action_failed') { return '1 failed'; } + if (key === 'com_ui_n_actions_cancelled') { + return `${values?.[0]} cancelled`; + } + if (key === 'com_ui_one_action_cancelled') { + return '1 cancelled'; + } if (key === 'com_ui_web_searched') { return 'Searched the web'; } @@ -851,6 +857,35 @@ describe('ToolCallGroup image hoisting', () => { ).toBeInTheDocument(); }); + it('names a cancelled action in the header before collapsing', () => { + const cancelled = { + type: ContentTypes.TOOL_CALL, + [ContentTypes.TOOL_CALL]: { + id: 'c2', + name: 'create_file', + args: '{}', + output: '', + runStepStatus: 'cancelled', + }, + } as unknown as TMessageContentParts; + + renderGroup({ + ...baseProps, + parts: [ + { part: makePart('c1', 'created', 'create_file'), idx: 0 }, + { part: cancelled, idx: 1 }, + ], + lastContentIdx: 1, + }); + + /** A stopped action is settled but not successful, and the group collapses + * over the only other notice, so the header must carry it and must not + * count it as a failure. */ + expect( + screen.getByRole('button', { name: 'Ran 2 actions, Create File ×2 · 1 cancelled' }), + ).toBeInTheDocument(); + }); + it('labels a homogeneous ask_user_question group as its own category', () => { renderGroup({ ...baseProps, diff --git a/client/src/hooks/Input/useAskAnswerMode.ts b/client/src/hooks/Input/useAskAnswerMode.ts index fe7cb6c01c..d632fed146 100644 --- a/client/src/hooks/Input/useAskAnswerMode.ts +++ b/client/src/hooks/Input/useAskAnswerMode.ts @@ -52,6 +52,18 @@ const askAnswerTextAtom = atom>({ default: {}, }); +/** + * Ordinary composer text typed AFTER a question moved to the chat card, keyed + * by pending action id. Expanding hands the composer back to the answer, which + * would otherwise overwrite an unsent normal message with no route back to it. + * Only needed while Save drafts is off; with saving on the conversation draft + * already holds that text. + */ +const releasedComposerTextAtom = atom>({ + key: 'askAnswerModeReleasedComposerText', + default: {}, +}); + /** * First-class "answer mode" for a live `ask_user_question` pause. Clicking an * option submits it immediately (multi-select clicks toggle instead, confirmed @@ -86,6 +98,7 @@ export default function useAskAnswerMode(conversationId?: string | null) { const [selected, setSelected] = useRecoilState(askAnswerSelectionAtom); const [checked, setChecked] = useRecoilState(askAnswerCheckedAtom); const [answerDrafts, setAnswerDrafts] = useRecoilState(askAnswerTextAtom); + const [releasedComposerText, setReleasedComposerText] = useRecoilState(releasedComposerTextAtom); const saveDrafts = useRecoilValue(store.saveDrafts); const { submitAskAnswer } = useResumeSubmit(); /** Recoil-backed so the lock/status works from the composer, which renders @@ -187,15 +200,47 @@ export default function useAskAnswerMode(conversationId?: string | null) { /** Popover ⇄ chat-card handoffs run inside a view transition: both * surfaces carry the same `view-transition-name`, so the browser morphs * one into the other instead of swapping. Both are user-event driven, - * which morphTransition's synchronous flush requires. */ + * which morphTransition's synchronous flush requires. + * + * Batches opt out: only the single-question surfaces declare + * `view-transition-name: ask-question`, so wrapping a batch handoff would + * give the browser nothing to pair and it would cross-fade the whole root + * (chat plus composer) instead. */ + const runHandoff = useCallback( + (update: () => void) => { + if (batchMode) { + update(); + return; + } + morphTransition(update); + }, + [batchMode], + ); + const collapse = useCallback(() => { if (liveAsk) { const composerAnswer = !batchMode ? (formContext?.getValues('text') ?? answerText) : ''; - morphTransition(() => { + runHandoff(() => { if (!batchMode) { setAnswerDrafts((current) => ({ ...current, [liveAsk.actionId]: composerAnswer })); if (!saveDrafts) { - formContext?.reset(); + /** Hand back whatever ordinary message was typed while this + * question owned the card, rather than clearing the composer and + * losing it. */ + const released = releasedComposerText[liveAsk.actionId]; + if (released) { + formContext?.setValue('text', released); + } else { + formContext?.reset(); + } + setReleasedComposerText((current) => { + if (current[liveAsk.actionId] == null) { + return current; + } + const next = { ...current }; + delete next[liveAsk.actionId]; + return next; + }); } } setCollapsedIds((prev) => @@ -203,14 +248,35 @@ export default function useAskAnswerMode(conversationId?: string | null) { ); }); } - }, [liveAsk, batchMode, formContext, answerText, saveDrafts, setAnswerDrafts, setCollapsedIds]); + }, [ + liveAsk, + batchMode, + formContext, + answerText, + saveDrafts, + releasedComposerText, + setReleasedComposerText, + setAnswerDrafts, + setCollapsedIds, + runHandoff, + ]); const expand = useCallback(() => { if (liveAsk) { - morphTransition(() => { + /** Read before the transition: the composer is about to be handed back + * to the answer, and an ordinary unsent message typed while the card + * owned the question must survive the round trip. */ + const released = !batchMode && !saveDrafts ? (formContext?.getValues('text') ?? '') : ''; + runHandoff(() => { /** Autosave restores the ask-specific draft after the key switch. If * drafts are disabled, perform that handoff directly. */ if (!batchMode && !saveDrafts) { + if (released) { + setReleasedComposerText((current) => ({ + ...current, + [liveAsk.actionId]: released, + })); + } formContext?.setValue('text', answerText); } else if (!batchMode && answerText) { /** Drafts were re-enabled after this answer was captured with saving @@ -224,7 +290,16 @@ export default function useAskAnswerMode(conversationId?: string | null) { setCollapsedIds((prev) => prev.filter((id) => id !== liveAsk.actionId)); }); } - }, [liveAsk, batchMode, saveDrafts, formContext, answerText, setCollapsedIds]); + }, [ + liveAsk, + batchMode, + saveDrafts, + formContext, + answerText, + setReleasedComposerText, + setCollapsedIds, + runHandoff, + ]); /** Pure check toggle: the keyboard highlight is steered only by the * composer's digit/arrow shortcuts, so a mouse toggle never leaves a diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 51fc7dcba3..4b6b673d3f 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -1629,6 +1629,7 @@ "com_ui_more_options": "More options", "com_ui_my_prompts": "My Prompts", "com_ui_my_skills": "My Skills", + "com_ui_n_actions_cancelled": "{{0}} cancelled", "com_ui_n_actions_failed": "{{0}} failed", "com_ui_n_files": "{{0}} files", "com_ui_n_files_changed": "{{0}} files changed", @@ -1699,6 +1700,7 @@ "com_ui_omitted": "Omitted", "com_ui_on": "On", "com_ui_one_file_changed": "1 file changed", + "com_ui_one_action_cancelled": "1 cancelled", "com_ui_one_action_failed": "1 failed", "com_ui_open_archived_chat_new_tab_title": "{{title}} (opens in new tab)", "com_ui_open_artifact": "Open artifact",