diff --git a/app/src/components/flows/ToolActivityChip.test.tsx b/app/src/components/flows/ToolActivityChip.test.tsx new file mode 100644 index 000000000..d77a1be9d --- /dev/null +++ b/app/src/components/flows/ToolActivityChip.test.tsx @@ -0,0 +1,67 @@ +import { render, screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; + +import ToolActivityChip from './ToolActivityChip'; + +vi.mock('../../lib/i18n/I18nContext', () => ({ useT: () => ({ t: (key: string) => key }) })); + +describe('ToolActivityChip', () => { + it('renders the proposing label for propose_workflow', () => { + render(); + expect(screen.getByTestId('tool-activity-chip')).toHaveTextContent( + 'flows.copilot.tool.proposing' + ); + }); + + it('renders the proposing label for revise_workflow too', () => { + render(); + expect(screen.getByTestId('tool-activity-chip')).toHaveTextContent( + 'flows.copilot.tool.proposing' + ); + }); + + it('renders the dry-running label for dry_run_workflow', () => { + render(); + expect(screen.getByTestId('tool-activity-chip')).toHaveTextContent( + 'flows.copilot.tool.dryRunning' + ); + }); + + it('renders the saving label for save_workflow', () => { + render(); + expect(screen.getByTestId('tool-activity-chip')).toHaveTextContent('flows.copilot.tool.saving'); + }); + + it('renders a generic "using tools" label for an unrecognized tool name', () => { + render(); + expect(screen.getByTestId('tool-activity-chip')).toHaveTextContent( + 'flows.copilot.tool.usingTools' + ); + }); + + it('renders nothing for an empty toolNames array', () => { + const { container } = render(); + expect(container).toBeEmptyDOMElement(); + }); + + it('renders the shared label when every tool name maps to the same label', () => { + render(); + expect(screen.getByTestId('tool-activity-chip')).toHaveTextContent( + 'flows.copilot.tool.proposing' + ); + }); + + it('renders the generic label when tool names map to different labels', () => { + render(); + expect(screen.getByTestId('tool-activity-chip')).toHaveTextContent( + 'flows.copilot.tool.usingTools' + ); + }); + + it('renders the generic label when one tool is unrecognized, even if another is recognized', () => { + render(); + expect(screen.getByTestId('tool-activity-chip')).toHaveTextContent( + 'flows.copilot.tool.usingTools' + ); + }); +}); diff --git a/app/src/components/flows/ToolActivityChip.tsx b/app/src/components/flows/ToolActivityChip.tsx new file mode 100644 index 000000000..0d39c1ea6 --- /dev/null +++ b/app/src/components/flows/ToolActivityChip.tsx @@ -0,0 +1,41 @@ +/** + * ToolActivityChip — replaces raw tool-call JSON in the copilot chat with a + * compact, human-readable status pill (B25). Users should know the agent + * used a tool (it explains why the turn took longer), but they should never + * see the raw JSON arguments (e.g. the whole workflow graph payload). + */ +import { useT } from '../../lib/i18n/I18nContext'; + +interface Props { + /** Tool names extracted from the turn's tool-call envelope, in call order. */ + toolNames: string[]; +} + +/** Tools that map to a specific, more informative status label. */ +const KNOWN_TOOL_LABEL_KEYS: Record = { + propose_workflow: 'flows.copilot.tool.proposing', + revise_workflow: 'flows.copilot.tool.proposing', + dry_run_workflow: 'flows.copilot.tool.dryRunning', + save_workflow: 'flows.copilot.tool.saving', +}; + +export default function ToolActivityChip({ toolNames }: Props) { + const { t } = useT(); + if (toolNames.length === 0) return null; + + // Every tool must map to the SAME recognized label before we show a + // specific status (e.g. all of `propose_workflow`/`revise_workflow` map to + // "proposing..."); any unrecognized tool, or a mix of tools with different + // labels, falls back to a generic "Using tools..." pill rather than + // picking one tool's label arbitrarily or dumping tool names verbatim. + const labelKeys = toolNames.map(name => KNOWN_TOOL_LABEL_KEYS[name]); + const labelKey = labelKeys.every(key => key && key === labelKeys[0]) ? labelKeys[0] : undefined; + + return ( + + {t(labelKey ?? 'flows.copilot.tool.usingTools')} + + ); +} diff --git a/app/src/components/flows/WorkflowCopilotPanel.test.tsx b/app/src/components/flows/WorkflowCopilotPanel.test.tsx index 9a744770b..9842052da 100644 --- a/app/src/components/flows/WorkflowCopilotPanel.test.tsx +++ b/app/src/components/flows/WorkflowCopilotPanel.test.tsx @@ -167,6 +167,41 @@ describe('WorkflowCopilotPanel', () => { expect(screen.queryByTestId('workflow-copilot-empty')).not.toBeInTheDocument(); }); + it('B25: unwraps a raw tool-call envelope message into clean text + a tool activity chip, never raw JSON', () => { + // Repro for B25: a turn that both talks and calls a tool can land in the + // thread transcript as the provider wire-format `{ content, tool_calls }` + // envelope. The panel must render only the human text — never the raw + // JSON — plus a compact status chip for the tool activity. + hookState.displayMessages = [ + { id: 'm1', content: 'build me a Slack digest', sender: 'user' }, + { + id: 'm2', + content: JSON.stringify({ + content: "Here's the workflow I propose.", + tool_calls: [{ id: 'call_1', name: 'propose_workflow', arguments: '{"nodes":[]}' }], + }), + sender: 'agent', + }, + ]; + render( + + ); + const bubble = screen.getByTestId('workflow-copilot-agent'); + expect(bubble).toHaveTextContent("Here's the workflow I propose."); + // The raw envelope must never reach the DOM as text. + expect(bubble).not.toHaveTextContent('tool_calls'); + expect(bubble).not.toHaveTextContent('"nodes":[]'); + expect(screen.getByTestId('tool-activity-chip')).toHaveTextContent( + 'flows.copilot.tool.proposing' + ); + }); + it('does not render an isInterim agent message as a bubble, only the terminal one', () => { // The panel only ever renders `displayMessages` — the same filtered set // `useWorkflowBuilderChat` computes from the raw transcript (isInterim diff --git a/app/src/components/flows/WorkflowCopilotPanel.tsx b/app/src/components/flows/WorkflowCopilotPanel.tsx index 9f14a3de2..7dfe1da6c 100644 --- a/app/src/components/flows/WorkflowCopilotPanel.tsx +++ b/app/src/components/flows/WorkflowCopilotPanel.tsx @@ -25,12 +25,14 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import { BubbleMarkdown } from '../../features/conversations/components/AgentMessageBubble'; import { ToolTimelineBlock } from '../../features/conversations/components/ToolTimelineBlock'; import { useWorkflowBuilderChat } from '../../hooks/useWorkflowBuilderChat'; +import { unwrapToolCallEnvelope } from '../../lib/flows/copilotMessageSanitizer'; import { diffGraphs } from '../../lib/flows/graphDiff'; import type { WorkflowGraph } from '../../lib/flows/types'; import { useT } from '../../lib/i18n/I18nContext'; import type { WorkflowProposal } from '../../store/chatRuntimeSlice'; import ChatComposer from '../chat/ChatComposer'; import Button from '../ui/Button'; +import ToolActivityChip from './ToolActivityChip'; const log = createDebug('app:flows:copilot-panel'); @@ -298,7 +300,11 @@ export default function WorkflowCopilotPanel({ const diff = proposal ? diffGraphs(graph, proposal.graph as WorkflowGraph) : null; const hasTimeline = toolTimeline.length > 0; - const hasLiveText = liveResponse.trim().length > 0; + // B25: the in-flight streaming text can also carry the raw tool-call + // envelope mid-turn — unwrap once and reuse the clean text everywhere below + // (the pre-tool streaming bubble and the shared `ToolTimelineBlock`). + const liveResponseText = unwrapToolCallEnvelope(liveResponse).text; + const hasLiveText = liveResponseText.trim().length > 0; const isEmpty = displayMessages.length === 0 && !proposal && !sending && !error && !hasTimeline && !hasLiveText; @@ -335,22 +341,34 @@ export default function WorkflowCopilotPanel({ Renders `displayMessages` (interim narration bubbles filtered out — that narration already streams via the tool timeline / live text below it double-renders it as a bubble too, see B4). */} - {displayMessages.map(message => - message.sender === 'user' ? ( -
-
- {message.content} + {displayMessages.map(message => { + if (message.sender === 'user') { + return ( +
+
+ {message.content} +
-
- ) : ( + ); + } + // B25: a turn that both talks and calls a tool can carry the + // provider wire-format `{ content, tool_calls }` envelope as this + // message's raw `content` — unwrap it so the bubble renders only + // the human text (+ a compact tool-activity chip), never raw JSON. + const { text, toolNames } = unwrapToolCallEnvelope(message.content); + return (
- + +
- ) - )} + ); + })} {/* Live builder activity — the SHARED tool timeline (tool cards + the streaming reply) the main chat uses, fed from the runtime's streamed @@ -360,7 +378,7 @@ export default function WorkflowCopilotPanel({
)} @@ -372,7 +390,7 @@ export default function WorkflowCopilotPanel({
- +
)} diff --git a/app/src/hooks/useWorkflowBuilderChat.test.ts b/app/src/hooks/useWorkflowBuilderChat.test.ts index 4b8e0c5c0..b865b1d98 100644 --- a/app/src/hooks/useWorkflowBuilderChat.test.ts +++ b/app/src/hooks/useWorkflowBuilderChat.test.ts @@ -52,15 +52,6 @@ vi.mock('../store/chatRuntimeSlice', () => ({ }), })); -// The hook reads the live store directly (not the stale closed-over selector -// value) to dedup against a message the streamed `chat_done` path may have -// already appended for this exact turn — see the `assistantText` fallback -// branch. Controllable per test via `rawStoreState.thread.messagesByThreadId`. -const rawStoreState = vi.hoisted(() => ({ - thread: { messagesByThreadId: {} as Record }, -})); -vi.mock('../store', () => ({ store: { getState: () => rawStoreState } })); - const okResult = (over: Partial = {}): BuilderTurnResult => ({ proposal: null, assistantText: 'done', @@ -76,7 +67,6 @@ describe('useWorkflowBuilderChat', () => { selectorState.messagesByThreadId = {}; selectorState.toolTimelineByThread = {}; selectorState.streamingAssistantByThread = {}; - rawStoreState.thread.messagesByThreadId = {}; dispatch.mockReset().mockImplementation((action: { type: string }) => { if (action.type === 'createNewThread') { return { unwrap: () => Promise.resolve({ id: 'builder-1' }) }; @@ -139,13 +129,7 @@ describe('useWorkflowBuilderChat', () => { ); }); - it('appends the user turn locally — the runtime normally owns the agent reply', async () => { - // Simulate the streamed path already having delivered this exact text via - // `chat_done` (the normal case when streaming is wired) so the fallback - // branch below can prove it does NOT double the bubble. - rawStoreState.thread.messagesByThreadId = { - 'builder-1': [{ sender: 'agent', content: 'Here is your workflow.' }], - }; + it('appends the user turn locally but never the agent reply — onDone is the single authoritative path (B26)', async () => { buildWorkflow.mockResolvedValue(okResult({ assistantText: 'Here is your workflow.' })); const { result } = renderHook(() => useWorkflowBuilderChat()); await act(async () => { @@ -160,12 +144,13 @@ describe('useWorkflowBuilderChat', () => { // The web channel never persists user messages, so the hook appends the // user turn itself... expect(appended.some(a => a.p?.message?.sender === 'user')).toBe(true); - // ...but NOT the agent reply when it was already streamed — appending - // here too would double it. + // ...but NEVER the agent reply — `ChatRuntimeProvider.onDone` is the sole + // path that persists it (B26: the local fallback append that used to race + // it, doubling the bubble on tool-calling turns, is gone entirely). expect(appended.some(a => a.p?.message?.sender === 'agent')).toBe(false); }); - it('surfaces a clarifying question as an assistant message when the builder returns plain text with no proposal (fallback)', async () => { + it('never locally appends an assistant message, even for a clarifying-question-shaped reply with no proposal (B26)', async () => { buildWorkflow.mockResolvedValue( okResult({ proposal: null, @@ -184,12 +169,10 @@ describe('useWorkflowBuilderChat', () => { const appendedAgentMessages = dispatch.mock.calls .map(([a]) => a as { type: string; p?: { threadId?: string; message?: ThreadMessage } }) .filter(a => a.type === 'addMessageLocal' && a.p?.message?.sender === 'agent'); - expect(appendedAgentMessages).toHaveLength(1); - expect(appendedAgentMessages[0]?.p?.message?.content).toBe( - 'Which Slack channel — #eng or #sales?' - ); - expect(appendedAgentMessages[0]?.p?.threadId).toBe('builder-1'); - // No proposal was surfaced for this turn. + // No local fallback append — the assistant's reply (if any) arrives only + // via the streamed `chat_done` -> `ChatRuntimeProvider.onDone` path. + expect(appendedAgentMessages).toHaveLength(0); + // No proposal was surfaced for this turn either. expect(dispatch.mock.calls.some(([a]) => (a as { type: string }).type === 'setProposal')).toBe( false ); @@ -354,10 +337,9 @@ describe('useWorkflowBuilderChat', () => { content: 'Now let me build the workflow.', extraMetadata: { isInterim: true, requestId: 'r1' }, }, - // The #4630-style clarifying question is appended via - // `addMessageLocal` (the `send` fallback branch), never through - // `onInterim` — so it carries no `isInterim` tag and must still - // render as a bubble. + // The #4630-style clarifying question is persisted by + // `ChatRuntimeProvider.onDone` on the turn's `chat_done` event — + // it carries no `isInterim` tag and must still render as a bubble. { id: 'm4', sender: 'agent', @@ -370,6 +352,44 @@ describe('useWorkflowBuilderChat', () => { expect(result.current.messages).toHaveLength(4); expect(result.current.displayMessages.map(m => m.id)).toEqual(['m1', 'm4']); }); + + it('dedupes consecutive agent messages with identical content (B26 defense-in-depth)', () => { + // Simulates a doubled persistence (e.g. a socket reconnect replaying + // `chat_done`): two consecutive agent messages with the exact same + // content must collapse to a single rendered bubble. + selectorState.messagesByThreadId = { + 'builder-1': [ + { id: 'm1', sender: 'user', content: 'build me a digest', extraMetadata: {} }, + { + id: 'm2', + sender: 'agent', + content: "I've built this — review below.", + extraMetadata: {}, + }, + { + id: 'm3', + sender: 'agent', + content: "I've built this — review below.", + extraMetadata: {}, + }, + ] as ThreadMessage[], + }; + const { result } = renderHook(() => useWorkflowBuilderChat('builder-1')); + expect(result.current.messages).toHaveLength(3); + expect(result.current.displayMessages.map(m => m.id)).toEqual(['m1', 'm2']); + }); + + it('keeps consecutive agent messages with DIFFERENT content (no over-collapsing)', () => { + selectorState.messagesByThreadId = { + 'builder-1': [ + { id: 'm1', sender: 'user', content: 'build me a digest', extraMetadata: {} }, + { id: 'm2', sender: 'agent', content: 'First reply.', extraMetadata: {} }, + { id: 'm3', sender: 'agent', content: 'Second, different reply.', extraMetadata: {} }, + ] as ThreadMessage[], + }; + const { result } = renderHook(() => useWorkflowBuilderChat('builder-1')); + expect(result.current.displayMessages.map(m => m.id)).toEqual(['m1', 'm2', 'm3']); + }); }); describe('rehydration on mount with seedThreadId', () => { diff --git a/app/src/hooks/useWorkflowBuilderChat.ts b/app/src/hooks/useWorkflowBuilderChat.ts index 4e0c4e86b..8a7a4af3d 100644 --- a/app/src/hooks/useWorkflowBuilderChat.ts +++ b/app/src/hooks/useWorkflowBuilderChat.ts @@ -16,8 +16,10 @@ * `pendingWorkflowProposalsByThread` for this thread as the turn runs. This hook * only appends the local USER turn (the web channel never persists user * messages) and reads the streamed state back out; the blocking - * `{proposal, error}` return is a fallback for when streaming isn't wired - * (CLI / tests / a missed socket event). + * `{proposal, error}` return is used for the proposal/error signal only — + * `ChatRuntimeProvider.onDone` is the SINGLE authoritative path for + * persisting the assistant's reply (B26: a local fallback append here used + * to race it and double the bubble on tool-calling turns). * * Invariant: `create`/`revise`/`repair` never persist; only a `build` turn (with * a real flow id) may save onto an existing flow. Nothing here enables a flow. @@ -26,7 +28,6 @@ import createDebug from 'debug'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { type BuilderTurnRequest, buildWorkflow } from '../services/api/flowsApi'; -import { store } from '../store'; import { clearWorkflowProposalForThread, fetchAndHydrateTurnHistory, @@ -110,8 +111,8 @@ export interface UseWorkflowBuilderChat { * that narration already renders live via `toolTimeline`/`liveResponse` * below — showing it again as a bubble double-renders it. User messages and * any non-interim agent message (the turn's terminal answer, including a - * clarifying question appended via the `assistantText` fallback in `send`) - * are always kept. + * clarifying question) are kept, with consecutive identical-content agent + * messages collapsed to one (B26 dedup guard). */ displayMessages: ThreadMessage[]; /** @@ -204,10 +205,20 @@ export function useWorkflowBuilderChat(seedThreadId?: string | null): UseWorkflo // non-interim agent turn (the terminal answer for a round, including a // clarifying question with no `isInterim` tag). `messages` itself stays the // full set — rehydration (below) and any future persistence need it intact. - const displayMessages = useMemo( - () => messages.filter(m => m.sender === 'user' || !m.extraMetadata?.isInterim), - [messages] - ); + // + // Also dedupes consecutive agent messages with identical content (B26 + // defense-in-depth): the fallback append in `send()` that used to race + // `ChatRuntimeProvider.onDone` is gone, but this guards against any future + // regression (e.g. a socket reconnect replaying `chat_done`) producing a + // doubled bubble. + const displayMessages = useMemo(() => { + const filtered = messages.filter(m => m.sender === 'user' || !m.extraMetadata?.isInterim); + return filtered.filter((m, i) => { + if (m.sender !== 'agent' || i === 0) return true; + const prev = filtered[i - 1]; + return !(prev.sender === 'agent' && prev.content === m.content); + }); + }, [messages]); const toolTimeline = useMemo( () => (threadId ? (toolTimelineByThread[threadId] ?? EMPTY_TIMELINE) : EMPTY_TIMELINE), @@ -314,10 +325,9 @@ export function useWorkflowBuilderChat(seedThreadId?: string | null): UseWorkflo // text/thinking/tool events + a terminal `chat_done` keyed by it. The // GLOBAL `ChatRuntimeProvider` owns that transcript — it appends the // final assistant message on `chat_done` and fills the streaming/tool - // slices as the turn runs, so in the normal (streaming-wired) case this - // hook must NOT also append the agent reply (doing so would double - // it) — see the dedup check below. We still await the blocking result - // for its `proposal`/`error`/`assistantText` fallback. + // slices as the turn runs, so this hook must NOT also append the agent + // reply (doing so would double it — B26). We still await the blocking + // result for its `proposal`/`error` signal. log('send: running flows_build thread=%s mode=%s', targetThreadId, request.mode); const result = await buildWorkflow(request, targetThreadId); @@ -333,39 +343,18 @@ export function useWorkflowBuilderChat(seedThreadId?: string | null): UseWorkflo ); } else if (result.error) { setError(result.error); - } else if (result.assistantText?.trim()) { - // Neither a proposal nor an error: the agent replied with plain - // text instead of proposing this turn — most commonly a clarifying - // question (the "ask" branch of the clarify/verify posture). When - // streaming is wired (the normal case) `ChatRuntimeProvider` already - // appended this exact text on the turn's `chat_done` — the Rust - // side (`finalize_flow_stream`) delivers it unconditionally, - // independent of whether a proposal was made — so re-appending here - // would double the bubble. Read the live store (not the stale - // closed-over `messages`) to check whether that already landed; - // only append when it hasn't, which is the actual fallback case - // (streaming not wired: CLI / tests / a missed socket event). - const latest = store.getState().thread.messagesByThreadId[targetThreadId] ?? []; - const lastMessage = latest[latest.length - 1]; - const alreadyStreamed = - lastMessage?.sender === 'agent' && lastMessage.content === result.assistantText; - log( - 'send: assistantText fallback thread=%s alreadyStreamed=%s', - targetThreadId, - alreadyStreamed - ); - if (!alreadyStreamed) { - const assistantMessage: ThreadMessage = { - id: `msg_${globalThis.crypto.randomUUID()}`, - content: result.assistantText, - type: 'text', - extraMetadata: {}, - sender: 'agent', - createdAt: new Date().toISOString(), - }; - dispatch(addMessageLocal({ threadId: targetThreadId, message: assistantMessage })); - } } + // Note: no local fallback append for `result.assistantText` here (B26). + // `ChatRuntimeProvider.onDone` is the SINGLE authoritative path that + // persists the assistant's reply on the turn's `chat_done` event — the + // Rust side (`finalize_flow_stream`) delivers it unconditionally, + // independent of whether a proposal was made. A local fallback here + // raced that streamed append (the socket event isn't guaranteed to have + // landed by the time this blocking call resolves) and produced a + // doubled bubble on tool-calling turns, which take longer and widen the + // race window. If streaming is ever not wired (CLI / tests), the + // assistant's reply simply won't appear in the thread transcript — the + // `proposal` still surfaces via the Redux slice above. return { outcome: 'dispatched', proposed }; } catch (err) { const msg = err instanceof Error ? err.message : String(err); diff --git a/app/src/lib/flows/copilotMessageSanitizer.test.ts b/app/src/lib/flows/copilotMessageSanitizer.test.ts new file mode 100644 index 000000000..44db34b8f --- /dev/null +++ b/app/src/lib/flows/copilotMessageSanitizer.test.ts @@ -0,0 +1,131 @@ +import { describe, expect, it } from 'vitest'; + +import { unwrapToolCallEnvelope } from './copilotMessageSanitizer'; + +describe('unwrapToolCallEnvelope', () => { + it('returns plain text unchanged with no tool names', () => { + const result = unwrapToolCallEnvelope('Hello, how can I help?'); + expect(result).toEqual({ text: 'Hello, how can I help?', toolNames: [] }); + }); + + it('extracts the content text and tool name from a valid native envelope', () => { + const raw = JSON.stringify({ + content: 'Let me build that for you.', + tool_calls: [{ id: 'call_1', name: 'propose_workflow', arguments: '{}' }], + }); + const result = unwrapToolCallEnvelope(raw); + expect(result).toEqual({ text: 'Let me build that for you.', toolNames: ['propose_workflow'] }); + }); + + it('passes through JSON with extra/unexpected top-level keys (not an envelope)', () => { + const raw = JSON.stringify({ content: 'hi', tool_calls: [], unexpected: true }); + const result = unwrapToolCallEnvelope(raw); + expect(result).toEqual({ text: raw, toolNames: [] }); + }); + + it('passes through malformed JSON unchanged', () => { + const raw = '{"content": "oops", "tool_calls": ['; + const result = unwrapToolCallEnvelope(raw); + expect(result).toEqual({ text: raw, toolNames: [] }); + }); + + it('passes through a JSON array unchanged (not a plain object)', () => { + const raw = '[1, 2, 3]'; + const result = unwrapToolCallEnvelope(raw); + expect(result).toEqual({ text: raw, toolNames: [] }); + }); + + it('extracts an empty content string, still surfacing tool names', () => { + const raw = JSON.stringify({ + content: '', + tool_calls: [{ id: 'c1', name: 'save_workflow', arguments: '{}' }], + }); + const result = unwrapToolCallEnvelope(raw); + expect(result).toEqual({ text: '', toolNames: ['save_workflow'] }); + }); + + it('returns an empty string for null/undefined-ish input without throwing', () => { + expect(unwrapToolCallEnvelope('')).toEqual({ text: '', toolNames: [] }); + // Defensive runtime guard against a non-string value that could arrive + // from a loosely-typed message payload. + // @ts-expect-error — intentionally passing a non-string to exercise the guard. + const result = unwrapToolCallEnvelope(null); + expect(result).toEqual({ text: '', toolNames: [] }); + }); + + it('preserves nested JSON inside `content` as opaque text', () => { + const raw = JSON.stringify({ + content: '{"nested": "value"}', + tool_calls: [{ id: 'c1', name: 'dry_run_workflow', arguments: '{}' }], + }); + const result = unwrapToolCallEnvelope(raw); + expect(result.text).toBe('{"nested": "value"}'); + expect(result.toolNames).toEqual(['dry_run_workflow']); + }); + + it('extracts every tool name when multiple tool_calls are present', () => { + const raw = JSON.stringify({ + content: 'Working on it.', + tool_calls: [ + { id: 'c1', name: 'dry_run_workflow', arguments: '{}' }, + { id: 'c2', name: 'save_workflow', arguments: '{}' }, + ], + }); + const result = unwrapToolCallEnvelope(raw); + expect(result.toolNames).toEqual(['dry_run_workflow', 'save_workflow']); + }); + + it('unwraps an envelope that also carries a reasoning_content key', () => { + const raw = JSON.stringify({ + content: 'Here is the plan.', + tool_calls: [{ id: 'c1', name: 'propose_workflow', arguments: '{}' }], + reasoning_content: 'internal chain of thought', + }); + const result = unwrapToolCallEnvelope(raw); + expect(result.text).toBe('Here is the plan.'); + expect(result.toolNames).toEqual(['propose_workflow']); + }); + + it('unwraps a tool-only envelope with `content` missing entirely, surfacing tool names with empty text', () => { + const raw = JSON.stringify({ tool_calls: [{ id: 'c1', name: 'x', arguments: '{}' }] }); + const result = unwrapToolCallEnvelope(raw); + expect(result).toEqual({ text: '', toolNames: ['x'] }); + }); + + it('unwraps a tool-only envelope with `content: null`, surfacing tool names with empty text', () => { + const raw = JSON.stringify({ + content: null, + tool_calls: [{ id: 'c1', name: 'save_workflow', arguments: '{}' }], + }); + const result = unwrapToolCallEnvelope(raw); + expect(result).toEqual({ text: '', toolNames: ['save_workflow'] }); + }); + + it('passes through when `content` is present but non-string (and not null)', () => { + const raw = JSON.stringify({ content: 42, tool_calls: [] }); + const result = unwrapToolCallEnvelope(raw); + expect(result).toEqual({ text: raw, toolNames: [] }); + }); + + it('passes through content-only JSON that lacks a `tool_calls` array (not an envelope)', () => { + const raw = JSON.stringify({ content: 'hi' }); + const result = unwrapToolCallEnvelope(raw); + expect(result).toEqual({ text: raw, toolNames: [] }); + }); + + it('passes through when `tool_calls` is present but not an array', () => { + const raw = JSON.stringify({ content: 'hi', tool_calls: 'nope' }); + const result = unwrapToolCallEnvelope(raw); + expect(result).toEqual({ text: raw, toolNames: [] }); + }); + + it('handles a `tool_calls` array with non-string/missing names gracefully', () => { + const raw = JSON.stringify({ + content: 'hi', + tool_calls: [{ id: 'c1' }, { id: 'c2', name: 42 }, { id: 'c3', name: 'ok_tool' }], + }); + const result = unwrapToolCallEnvelope(raw); + expect(result.text).toBe('hi'); + expect(result.toolNames).toEqual(['ok_tool']); + }); +}); diff --git a/app/src/lib/flows/copilotMessageSanitizer.ts b/app/src/lib/flows/copilotMessageSanitizer.ts new file mode 100644 index 000000000..856e9a078 --- /dev/null +++ b/app/src/lib/flows/copilotMessageSanitizer.ts @@ -0,0 +1,119 @@ +import createDebug from 'debug'; + +/** + * copilotMessageSanitizer — defends the Flows copilot chat against rendering + * the raw provider wire-format envelope instead of clean assistant text + * (B25). + * + * The Rust core's `NativeToolDispatcher::to_provider_messages` + * (`src/openhuman/agent/dispatcher.rs`) and the tinyagents bridge's + * `message_to_native_chat_message` (`src/openhuman/tinyagents/convert.rs`) + * serialize an assistant turn that both talks AND calls a tool into a + * `{ "content": "...", "tool_calls": [...] }` JSON envelope — the provider + * wire format used to round-trip tool-calling history on the NEXT request. + * That envelope is meant to stay internal to the agent session; this + * sanitizer is a shape-based (not string-match) belt-and-suspenders guard so + * that if it ever leaks into a chat message's `content` (from any Rust-side + * vector, present or future), the copilot still renders only the human + * text — never the raw JSON — mirroring the `unwrapPayloadEnvelope` + * philosophy from PR #4822 (`app/src/lib/flows/runItems.ts`). + */ +const log = createDebug('app:flows:copilot-sanitizer'); + +/** Top-level keys the native tool-call envelope may carry. */ +const ENVELOPE_KEYS = new Set(['content', 'tool_calls', 'reasoning_content']); + +/** A single entry of the envelope's `tool_calls` array. */ +interface EnvelopeToolCall { + name?: unknown; +} + +/** Result of unwrapping a chat message's raw `content` string. */ +export interface UnwrappedToolCallMessage { + /** The human-readable text to render (never the raw JSON). */ + text: string; + /** Tool names found on the envelope's `tool_calls` array, in order. */ + toolNames: string[]; +} + +function isPlainObject(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** + * If `raw` is a serialized `{ "content": "...", "tool_calls": [...] }` JSON + * envelope (the provider wire format for a native tool-calling turn), extract + * and return only the human-readable `content` text plus the tool names found + * (for chip rendering). Otherwise returns `raw` unchanged. + * + * Shape-based, not string-match: only collapses when the parsed value is a + * plain object whose keys are ALL drawn from the known envelope key set AND a + * string `content` key is present — so ordinary JSON-looking assistant prose + * (e.g. a code block containing `{"content": "hi"}`) is never misread as an + * envelope unless it structurally matches. Never silently drops data: any + * non-envelope shape (including malformed JSON, or JSON with unexpected + * top-level keys) passes `raw` straight through unchanged. + */ +export function unwrapToolCallEnvelope(raw: string): UnwrappedToolCallMessage { + if (typeof raw !== 'string' || raw.trim().length === 0) { + return { text: raw ?? '', toolNames: [] }; + } + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + log('unwrapToolCallEnvelope: not JSON — pass through (chars=%d)', raw.length); + return { text: raw, toolNames: [] }; + } + + if (!isPlainObject(parsed)) { + log('unwrapToolCallEnvelope: JSON but not an object — pass through'); + return { text: raw, toolNames: [] }; + } + + const keys = Object.keys(parsed); + if (keys.length === 0 || !keys.every(key => ENVELOPE_KEYS.has(key))) { + log('unwrapToolCallEnvelope: non-envelope object (keys=%o) — pass through', keys); + return { text: raw, toolNames: [] }; + } + + // Require `tool_calls` to be an array before treating this as a tool-call + // envelope at all — a content-only object like `{ "content": "hi" }` is + // ordinary JSON-looking prose, not a native envelope, and must pass + // through unchanged rather than being unwrapped down to `hi`. + if (!Array.isArray(parsed.tool_calls)) { + log( + 'unwrapToolCallEnvelope: envelope-shaped keys=%o but `tool_calls` is not an array — pass through', + keys + ); + return { text: raw, toolNames: [] }; + } + const toolCalls = parsed.tool_calls as EnvelopeToolCall[]; + + // The native dispatcher can serialize a tool-only turn with `content: null` + // (or omit `content` entirely) while still including `tool_calls`. Treat + // null/missing `content` as `''` so the bubble still exposes the tool + // activity chip instead of falling back to the raw JSON. + const { content } = parsed; + if (content !== null && content !== undefined && typeof content !== 'string') { + log( + 'unwrapToolCallEnvelope: envelope-shaped keys=%o but `content` is non-string (and not null/missing) — pass through', + keys + ); + return { text: raw, toolNames: [] }; + } + const text = typeof content === 'string' ? content : ''; + + const toolNames = toolCalls + .map(call => call?.name) + .filter((name): name is string => typeof name === 'string' && name.length > 0); + + log( + 'unwrapToolCallEnvelope: envelope keys=%o — extracted text (chars=%d) + toolNames=%o', + keys, + text.length, + toolNames + ); + return { text, toolNames }; +} diff --git a/app/src/lib/i18n/ar.ts b/app/src/lib/i18n/ar.ts index 84739360c..a6eec0ddb 100644 --- a/app/src/lib/i18n/ar.ts +++ b/app/src/lib/i18n/ar.ts @@ -3828,6 +3828,10 @@ const messages: TranslationMap = { 'flows.copilot.reject': 'تجاهل', 'flows.copilot.previewHint': 'جارٍ مراجعة مسودة مقترحة — لم يُحفظ شيء بعد.', 'flows.copilot.repairDisplay': 'فشل تشغيل؛ راجعه واقترح إصلاحًا.', + 'flows.copilot.tool.proposing': 'اقتراح سير العمل…', + 'flows.copilot.tool.dryRunning': 'تجربة تشغيل سير العمل…', + 'flows.copilot.tool.saving': 'حفظ سير العمل…', + 'flows.copilot.tool.usingTools': 'استخدام الأدوات…', 'flows.list.view': 'عرض سير العمل', 'flows.list.export': 'تصدير', 'flows.list.exported': 'تم تصدير سير العمل', diff --git a/app/src/lib/i18n/bn.ts b/app/src/lib/i18n/bn.ts index 7bc4b2653..32e3b67d2 100644 --- a/app/src/lib/i18n/bn.ts +++ b/app/src/lib/i18n/bn.ts @@ -3917,6 +3917,10 @@ const messages: TranslationMap = { 'flows.copilot.previewHint': 'একটি প্রস্তাবিত খসড়া পর্যালোচনা হচ্ছে — এখনও কিছু সংরক্ষণ করা হয়নি।', 'flows.copilot.repairDisplay': 'একটি রান ব্যর্থ হয়েছে; এটি দেখুন এবং একটি সমাধান প্রস্তাব করুন।', + 'flows.copilot.tool.proposing': 'ওয়ার্কফ্লো প্রস্তাব করা হচ্ছে…', + 'flows.copilot.tool.dryRunning': 'ওয়ার্কফ্লো ড্রাই-রান করা হচ্ছে…', + 'flows.copilot.tool.saving': 'ওয়ার্কফ্লো সংরক্ষণ করা হচ্ছে…', + 'flows.copilot.tool.usingTools': 'টুল ব্যবহার করা হচ্ছে…', 'flows.list.view': 'ওয়ার্কফ্লো দেখুন', 'flows.list.export': 'রপ্তানি', 'flows.list.exported': 'ওয়ার্কফ্লো রপ্তানি হয়েছে', diff --git a/app/src/lib/i18n/de.ts b/app/src/lib/i18n/de.ts index 1d01da56c..3f02e545f 100644 --- a/app/src/lib/i18n/de.ts +++ b/app/src/lib/i18n/de.ts @@ -4017,6 +4017,10 @@ const messages: TranslationMap = { 'Ein vorgeschlagener Entwurf wird geprüft — es wurde noch nichts gespeichert.', 'flows.copilot.repairDisplay': 'Eine Ausführung ist fehlgeschlagen; sieh sie dir an und schlage eine Lösung vor.', + 'flows.copilot.tool.proposing': 'Workflow wird vorgeschlagen…', + 'flows.copilot.tool.dryRunning': 'Workflow wird testweise ausgeführt…', + 'flows.copilot.tool.saving': 'Workflow wird gespeichert…', + 'flows.copilot.tool.usingTools': 'Werkzeuge werden verwendet…', 'flows.list.view': 'Workflow anzeigen', 'flows.list.export': 'Exportieren', 'flows.list.exported': 'Workflow exportiert', diff --git a/app/src/lib/i18n/en.ts b/app/src/lib/i18n/en.ts index 0706813c4..408e0f832 100644 --- a/app/src/lib/i18n/en.ts +++ b/app/src/lib/i18n/en.ts @@ -4603,6 +4603,10 @@ const en: TranslationMap = { 'flows.copilot.reject': 'Dismiss', 'flows.copilot.previewHint': 'Reviewing a proposed draft — nothing is saved yet.', 'flows.copilot.repairDisplay': 'A run failed — please look at it and propose a fix.', + 'flows.copilot.tool.proposing': 'Proposing workflow…', + 'flows.copilot.tool.dryRunning': 'Dry-running workflow…', + 'flows.copilot.tool.saving': 'Saving workflow…', + 'flows.copilot.tool.usingTools': 'Using tools…', // ── Workflow Canvas (issue B5b.1) — the read-only graph view of a saved // flow at /flows/:id. `flows.nodeKind.*` labels the 12 tinyflows node diff --git a/app/src/lib/i18n/es.ts b/app/src/lib/i18n/es.ts index d6f251695..d8e85ffbb 100644 --- a/app/src/lib/i18n/es.ts +++ b/app/src/lib/i18n/es.ts @@ -3982,6 +3982,10 @@ const messages: TranslationMap = { 'flows.copilot.reject': 'Descartar', 'flows.copilot.previewHint': 'Revisando un borrador propuesto: aún no se ha guardado nada.', 'flows.copilot.repairDisplay': 'Falló una ejecución; revísala y propón una solución.', + 'flows.copilot.tool.proposing': 'Proponiendo flujo de trabajo…', + 'flows.copilot.tool.dryRunning': 'Ejecutando prueba del flujo de trabajo…', + 'flows.copilot.tool.saving': 'Guardando flujo de trabajo…', + 'flows.copilot.tool.usingTools': 'Usando herramientas…', 'flows.list.view': 'Ver flujo de trabajo', 'flows.list.export': 'Exportar', 'flows.list.exported': 'Flujo de trabajo exportado', diff --git a/app/src/lib/i18n/fr.ts b/app/src/lib/i18n/fr.ts index bedf360ce..0a9dff0da 100644 --- a/app/src/lib/i18n/fr.ts +++ b/app/src/lib/i18n/fr.ts @@ -3999,6 +3999,10 @@ const messages: TranslationMap = { 'flows.copilot.reject': 'Ignorer', 'flows.copilot.previewHint': 'Examen d’un brouillon proposé — rien n’est encore enregistré.', 'flows.copilot.repairDisplay': 'Une exécution a échoué ; examinez-la et proposez une correction.', + 'flows.copilot.tool.proposing': 'Proposition du workflow…', + 'flows.copilot.tool.dryRunning': 'Exécution d’essai du workflow…', + 'flows.copilot.tool.saving': 'Enregistrement du workflow…', + 'flows.copilot.tool.usingTools': 'Utilisation d’outils…', 'flows.list.view': 'Voir le workflow', 'flows.list.export': 'Exporter', 'flows.list.exported': 'Workflow exporté', diff --git a/app/src/lib/i18n/hi.ts b/app/src/lib/i18n/hi.ts index 8fe728543..d1fb34f46 100644 --- a/app/src/lib/i18n/hi.ts +++ b/app/src/lib/i18n/hi.ts @@ -3915,6 +3915,10 @@ const messages: TranslationMap = { 'flows.copilot.previewHint': 'एक प्रस्तावित ड्राफ़्ट की समीक्षा हो रही है — अभी कुछ सहेजा नहीं गया।', 'flows.copilot.repairDisplay': 'एक रन विफल हुआ; उसे देखें और सुधार सुझाएँ।', + 'flows.copilot.tool.proposing': 'वर्कफ़्लो प्रस्तावित किया जा रहा है…', + 'flows.copilot.tool.dryRunning': 'वर्कफ़्लो का ड्राई-रन किया जा रहा है…', + 'flows.copilot.tool.saving': 'वर्कफ़्लो सहेजा जा रहा है…', + 'flows.copilot.tool.usingTools': 'टूल का उपयोग किया जा रहा है…', 'flows.list.view': 'वर्कफ़्लो देखें', 'flows.list.export': 'निर्यात', 'flows.list.exported': 'वर्कफ़्लो निर्यात किया गया', diff --git a/app/src/lib/i18n/id.ts b/app/src/lib/i18n/id.ts index 28b94c4ff..bb6875e43 100644 --- a/app/src/lib/i18n/id.ts +++ b/app/src/lib/i18n/id.ts @@ -3927,6 +3927,10 @@ const messages: TranslationMap = { 'flows.copilot.reject': 'Buang', 'flows.copilot.previewHint': 'Meninjau draf yang diusulkan — belum ada yang disimpan.', 'flows.copilot.repairDisplay': 'Sebuah eksekusi gagal; periksa dan usulkan perbaikan.', + 'flows.copilot.tool.proposing': 'Mengusulkan alur kerja…', + 'flows.copilot.tool.dryRunning': 'Menjalankan uji coba alur kerja…', + 'flows.copilot.tool.saving': 'Menyimpan alur kerja…', + 'flows.copilot.tool.usingTools': 'Menggunakan alat…', 'flows.list.view': 'Lihat alur kerja', 'flows.list.export': 'Ekspor', 'flows.list.exported': 'Alur kerja diekspor', diff --git a/app/src/lib/i18n/it.ts b/app/src/lib/i18n/it.ts index 3f48adf4c..07acaaab2 100644 --- a/app/src/lib/i18n/it.ts +++ b/app/src/lib/i18n/it.ts @@ -3976,6 +3976,10 @@ const messages: TranslationMap = { 'flows.copilot.reject': 'Ignora', 'flows.copilot.previewHint': 'Revisione di una bozza proposta: non è stato ancora salvato nulla.', 'flows.copilot.repairDisplay': 'Un’esecuzione è fallita; esaminala e proponi una correzione.', + 'flows.copilot.tool.proposing': 'Proposta del flusso di lavoro…', + 'flows.copilot.tool.dryRunning': 'Esecuzione di prova del flusso di lavoro…', + 'flows.copilot.tool.saving': 'Salvataggio del flusso di lavoro…', + 'flows.copilot.tool.usingTools': 'Utilizzo degli strumenti…', 'flows.list.view': 'Visualizza flusso di lavoro', 'flows.list.export': 'Esporta', 'flows.list.exported': 'Flusso di lavoro esportato', diff --git a/app/src/lib/i18n/ko.ts b/app/src/lib/i18n/ko.ts index bf31b95c2..809ddbf7a 100644 --- a/app/src/lib/i18n/ko.ts +++ b/app/src/lib/i18n/ko.ts @@ -3875,6 +3875,10 @@ const messages: TranslationMap = { 'flows.copilot.reject': '버리기', 'flows.copilot.previewHint': '제안된 초안을 검토 중입니다 — 아직 저장되지 않았습니다.', 'flows.copilot.repairDisplay': '실행이 실패했습니다. 확인하고 수정을 제안하세요.', + 'flows.copilot.tool.proposing': '워크플로 제안 중…', + 'flows.copilot.tool.dryRunning': '워크플로 시험 실행 중…', + 'flows.copilot.tool.saving': '워크플로 저장 중…', + 'flows.copilot.tool.usingTools': '도구 사용 중…', 'flows.list.view': '워크플로 보기', 'flows.list.export': '내보내기', 'flows.list.exported': '워크플로를 내보냈습니다', diff --git a/app/src/lib/i18n/pl.ts b/app/src/lib/i18n/pl.ts index e3d9ddfa8..d86755829 100644 --- a/app/src/lib/i18n/pl.ts +++ b/app/src/lib/i18n/pl.ts @@ -3965,6 +3965,10 @@ const messages: TranslationMap = { 'Przeglądasz proponowaną wersję roboczą — nic nie zostało jeszcze zapisane.', 'flows.copilot.repairDisplay': 'Uruchomienie nie powiodło się; przejrzyj je i zaproponuj poprawkę.', + 'flows.copilot.tool.proposing': 'Proponowanie przepływu pracy…', + 'flows.copilot.tool.dryRunning': 'Testowe uruchamianie przepływu pracy…', + 'flows.copilot.tool.saving': 'Zapisywanie przepływu pracy…', + 'flows.copilot.tool.usingTools': 'Używanie narzędzi…', 'flows.list.view': 'Wyświetl przepływ pracy', 'flows.list.export': 'Eksportuj', 'flows.list.exported': 'Wyeksportowano przepływ pracy', diff --git a/app/src/lib/i18n/pt.ts b/app/src/lib/i18n/pt.ts index 74f57fc2e..4267c0961 100644 --- a/app/src/lib/i18n/pt.ts +++ b/app/src/lib/i18n/pt.ts @@ -3977,6 +3977,10 @@ const messages: TranslationMap = { 'flows.copilot.reject': 'Descartar', 'flows.copilot.previewHint': 'Revisando um rascunho proposto — nada foi salvo ainda.', 'flows.copilot.repairDisplay': 'Uma execução falhou; analise-a e proponha uma correção.', + 'flows.copilot.tool.proposing': 'Propondo fluxo de trabalho…', + 'flows.copilot.tool.dryRunning': 'Executando teste do fluxo de trabalho…', + 'flows.copilot.tool.saving': 'Salvando fluxo de trabalho…', + 'flows.copilot.tool.usingTools': 'Usando ferramentas…', 'flows.list.view': 'Ver fluxo de trabalho', 'flows.list.export': 'Exportar', 'flows.list.exported': 'Fluxo de trabalho exportado', diff --git a/app/src/lib/i18n/ru.ts b/app/src/lib/i18n/ru.ts index 04741b0f9..5d9e08445 100644 --- a/app/src/lib/i18n/ru.ts +++ b/app/src/lib/i18n/ru.ts @@ -3953,6 +3953,10 @@ const messages: TranslationMap = { 'flows.copilot.reject': 'Отклонить', 'flows.copilot.previewHint': 'Просмотр предложенного черновика — пока ничего не сохранено.', 'flows.copilot.repairDisplay': 'Запуск завершился ошибкой; изучите его и предложите исправление.', + 'flows.copilot.tool.proposing': 'Предлагается рабочий процесс…', + 'flows.copilot.tool.dryRunning': 'Выполняется пробный запуск рабочего процесса…', + 'flows.copilot.tool.saving': 'Сохранение рабочего процесса…', + 'flows.copilot.tool.usingTools': 'Использование инструментов…', 'flows.list.view': 'Просмотреть рабочий процесс', 'flows.list.export': 'Экспорт', 'flows.list.exported': 'Рабочий процесс экспортирован', diff --git a/app/src/lib/i18n/zh-CN.ts b/app/src/lib/i18n/zh-CN.ts index 15300cac4..606a73166 100644 --- a/app/src/lib/i18n/zh-CN.ts +++ b/app/src/lib/i18n/zh-CN.ts @@ -3712,6 +3712,10 @@ const messages: TranslationMap = { 'flows.copilot.reject': '放弃', 'flows.copilot.previewHint': '正在查看建议的草稿——尚未保存任何内容。', 'flows.copilot.repairDisplay': '一次运行失败了,请查看并提出修复方案。', + 'flows.copilot.tool.proposing': '正在提出工作流…', + 'flows.copilot.tool.dryRunning': '正在试运行工作流…', + 'flows.copilot.tool.saving': '正在保存工作流…', + 'flows.copilot.tool.usingTools': '正在使用工具…', 'flows.list.view': '查看工作流', 'flows.list.export': '导出', 'flows.list.exported': '工作流已导出',