From ff6979713564a1eff020681fa1989d7aa52ed4b0 Mon Sep 17 00:00:00 2001 From: Elliot Slusky <44592435+ElliotSlusky@users.noreply.github.com> Date: Thu, 13 Aug 2026 17:03:10 -0700 Subject: [PATCH] fix(web): normalize tool call arguments (#738) * fix(web): normalize tool call arguments * fix(web): preserve chats when repair writeback fails --- frontend/src/components/Chat/InputArea.tsx | 5 +- frontend/src/components/Chat/ToolCallCard.tsx | 10 +- frontend/src/lib/api.ts | 3 +- .../src/lib/store.tool-call-repair.test.ts | 122 ++++++++++++++++++ frontend/src/lib/store.ts | 26 +++- frontend/src/lib/tool-call.test.ts | 22 ++++ frontend/src/lib/tool-call.ts | 11 ++ src/openjarvis/server/stream_bridge.py | 6 + tests/server/test_stream_bridge.py | 30 +++++ 9 files changed, 228 insertions(+), 7 deletions(-) create mode 100644 frontend/src/lib/store.tool-call-repair.test.ts create mode 100644 frontend/src/lib/tool-call.test.ts create mode 100644 frontend/src/lib/tool-call.ts create mode 100644 tests/server/test_stream_bridge.py diff --git a/frontend/src/components/Chat/InputArea.tsx b/frontend/src/components/Chat/InputArea.tsx index 7c7f5978..20cf7301 100644 --- a/frontend/src/components/Chat/InputArea.tsx +++ b/frontend/src/components/Chat/InputArea.tsx @@ -5,6 +5,7 @@ import { useAppStore, generateId } from '../../lib/store'; import { streamChat, streamResearch } from '../../lib/sse'; import { fetchSavings, getBase } from '../../lib/api'; import { listConnectors, getSyncStatus } from '../../lib/connectors-api'; +import { serializeToolCallArguments } from '../../lib/tool-call'; import { MicButton } from './MicButton'; import { useSpeech } from '../../hooks/useSpeech'; import type { @@ -389,7 +390,7 @@ export function InputArea() { const tc: ToolCallInfo = { id: generateId(), tool: data.tool, - arguments: data.arguments || '', + arguments: serializeToolCallArguments(data.arguments), status: 'running', }; toolCalls.push(tc); @@ -400,7 +401,7 @@ export function InputArea() { updateLastAssistant(convId, accumulatedContent, [...toolCalls]); useAppStore.getState().addLogEntry({ timestamp: Date.now(), level: 'info', category: 'tool', - message: `Calling ${data.tool}(${data.arguments || ''})`, + message: `Calling ${data.tool}(${serializeToolCallArguments(data.arguments)})`, }); } catch {} } else if (eventName === 'tool_call_end') { diff --git a/frontend/src/components/Chat/ToolCallCard.tsx b/frontend/src/components/Chat/ToolCallCard.tsx index eb3d6aa3..03419672 100644 --- a/frontend/src/components/Chat/ToolCallCard.tsx +++ b/frontend/src/components/Chat/ToolCallCard.tsx @@ -1,6 +1,7 @@ import { useState } from 'react'; import { ChevronDown, ChevronRight, Loader2, CheckCircle2, XCircle } from 'lucide-react'; import type { ToolCallInfo } from '../../types'; +import { serializeToolCallArguments } from '../../lib/tool-call'; interface Props { toolCall: ToolCallInfo; @@ -35,7 +36,10 @@ export function ToolCallCard({ toolCall }: Props) { const [expanded, setExpanded] = useState(false); const config = statusConfig[toolCall.status]; const StatusIcon = config.icon; - const preview = previewArgs(toolCall.arguments); + // Persisted conversations may contain the pre-fix object payload despite + // the TypeScript contract, so normalize again at the final render boundary. + const argumentsText = serializeToolCallArguments(toolCall.arguments); + const preview = previewArgs(argumentsText); return (
- {toolCall.arguments && ( + {argumentsText && (
- {formatJson(toolCall.arguments)} + {formatJson(argumentsText)}
)} diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index e635f771..56ff88a8 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -1,5 +1,6 @@ import type { ModelInfo, SavingsData, ServerInfo } from '../types'; import { SUPABASE_ANON_KEY, SUPABASE_URL } from './supabase'; +import { serializeToolCallArguments } from './tool-call'; // --------------------------------------------------------------------------- // Supabase config @@ -741,7 +742,7 @@ export async function sendAgentMessage( const parsed = JSON.parse(data); callbacks?.onToolCallStart?.({ tool: parsed.tool, - arguments: parsed.arguments ?? '', + arguments: serializeToolCallArguments(parsed.arguments), }); } catch { /* skip */ diff --git a/frontend/src/lib/store.tool-call-repair.test.ts b/frontend/src/lib/store.tool-call-repair.test.ts new file mode 100644 index 00000000..a88fedf0 --- /dev/null +++ b/frontend/src/lib/store.tool-call-repair.test.ts @@ -0,0 +1,122 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const CONVERSATIONS_KEY = 'openjarvis-conversations'; + +class MemoryStorage { + private store = new Map(); + + getItem(key: string): string | null { + return this.store.get(key) ?? null; + } + + setItem(key: string, value: string): void { + this.store.set(key, String(value)); + } + + removeItem(key: string): void { + this.store.delete(key); + } +} + +beforeEach(() => { + vi.resetModules(); + (globalThis as unknown as { localStorage: MemoryStorage }).localStorage = + new MemoryStorage(); +}); + +afterEach(() => { + (globalThis as unknown as { localStorage?: MemoryStorage }).localStorage = + undefined; +}); + +describe('persisted tool calls', () => { + it('repairs parsed argument objects while loading conversations', async () => { + localStorage.setItem( + CONVERSATIONS_KEY, + JSON.stringify({ + version: 1, + activeId: 'conversation-1', + conversations: { + 'conversation-1': { + id: 'conversation-1', + title: 'Broken chat', + createdAt: 1, + updatedAt: 1, + model: 'test-model', + messages: [ + { + id: 'assistant-1', + role: 'assistant', + content: '', + timestamp: 1, + toolCalls: [ + { + id: 'call-1', + tool: 'web_search', + arguments: { query: 'python' }, + status: 'success', + }, + ], + }, + ], + }, + }, + }), + ); + + const { useAppStore } = await import('./store'); + + expect(useAppStore.getState().messages[0].toolCalls?.[0].arguments).toBe( + '{"query":"python"}', + ); + const repaired = JSON.parse(localStorage.getItem(CONVERSATIONS_KEY) ?? '{}'); + expect( + repaired.conversations['conversation-1'].messages[0].toolCalls[0].arguments, + ).toBe('{"query":"python"}'); + }); + + it('keeps repaired conversations in memory when writeback fails', async () => { + localStorage.setItem( + CONVERSATIONS_KEY, + JSON.stringify({ + version: 1, + activeId: 'conversation-1', + conversations: { + 'conversation-1': { + id: 'conversation-1', + title: 'Readable chat', + createdAt: 1, + updatedAt: 1, + model: 'test-model', + messages: [ + { + id: 'assistant-1', + role: 'assistant', + content: '', + timestamp: 1, + toolCalls: [ + { + id: 'call-1', + tool: 'web_search', + arguments: { query: 'python' }, + status: 'success', + }, + ], + }, + ], + }, + }, + }), + ); + vi.spyOn(localStorage, 'setItem').mockImplementation(() => { + throw new DOMException('Storage quota exceeded', 'QuotaExceededError'); + }); + + const { useAppStore } = await import('./store'); + + expect(useAppStore.getState().messages).toHaveLength(1); + expect(useAppStore.getState().messages[0].toolCalls?.[0].arguments).toBe( + '{"query":"python"}', + ); + }); +}); diff --git a/frontend/src/lib/store.ts b/frontend/src/lib/store.ts index dc5909c4..f2c91f79 100644 --- a/frontend/src/lib/store.ts +++ b/frontend/src/lib/store.ts @@ -16,6 +16,7 @@ import type { } from '../types'; import type { ManagedAgent } from './api'; import { isEmbedOnlyModel } from './model-capabilities'; +import { serializeToolCallArguments } from './tool-call'; export interface CachedConnector { connector_id: string; @@ -55,7 +56,30 @@ function loadConversations(): ConversationStore { const raw = localStorage.getItem(CONVERSATIONS_KEY); if (!raw) return { version: 1, conversations: {}, activeId: null }; const parsed = JSON.parse(raw); - if (parsed.version === 1) return parsed; + if (parsed.version === 1) { + let repaired = false; + for (const conversation of Object.values(parsed.conversations ?? {}) as Conversation[]) { + for (const message of conversation.messages ?? []) { + for (const toolCall of message.toolCalls ?? []) { + const argumentsText = serializeToolCallArguments(toolCall.arguments); + if (argumentsText !== toolCall.arguments) { + toolCall.arguments = argumentsText; + repaired = true; + } + } + } + } + if (repaired) { + try { + localStorage.setItem(CONVERSATIONS_KEY, JSON.stringify(parsed)); + } catch { + // Keep the repaired conversations usable in memory when storage is + // read-only or full. A failed best-effort writeback must not make + // otherwise readable conversation history disappear from the UI. + } + } + return parsed; + } return { version: 1, conversations: {}, activeId: null }; } catch { return { version: 1, conversations: {}, activeId: null }; diff --git a/frontend/src/lib/tool-call.test.ts b/frontend/src/lib/tool-call.test.ts new file mode 100644 index 00000000..3540780a --- /dev/null +++ b/frontend/src/lib/tool-call.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from 'vitest'; + +import { serializeToolCallArguments } from './tool-call'; + +describe('serializeToolCallArguments', () => { + it('preserves JSON strings', () => { + expect(serializeToolCallArguments('{"query":"python"}')).toBe( + '{"query":"python"}', + ); + }); + + it('serializes parsed argument objects', () => { + expect(serializeToolCallArguments({ query: 'python' })).toBe( + '{"query":"python"}', + ); + }); + + it('uses an empty string for missing arguments', () => { + expect(serializeToolCallArguments(null)).toBe(''); + expect(serializeToolCallArguments(undefined)).toBe(''); + }); +}); diff --git a/frontend/src/lib/tool-call.ts b/frontend/src/lib/tool-call.ts new file mode 100644 index 00000000..22321cd8 --- /dev/null +++ b/frontend/src/lib/tool-call.ts @@ -0,0 +1,11 @@ +/** Convert tool-call arguments from API or persisted data into display-safe text. */ +export function serializeToolCallArguments(value: unknown): string { + if (typeof value === 'string') return value; + if (value == null) return ''; + + try { + return JSON.stringify(value) ?? String(value); + } catch { + return String(value); + } +} diff --git a/src/openjarvis/server/stream_bridge.py b/src/openjarvis/server/stream_bridge.py index 6d331021..b1c2e8f3 100644 --- a/src/openjarvis/server/stream_bridge.py +++ b/src/openjarvis/server/stream_bridge.py @@ -110,6 +110,12 @@ class AgentStreamBridge: def _format_named_event(self, name: str, data: dict) -> str: """Format an SSE event with an explicit ``event:`` field.""" + if name == "tool_call_start" and not isinstance(data.get("arguments"), str): + # The in-process event bus uses parsed arguments for trace/eval + # consumers, while the web SSE contract expects their JSON text. + # Copy before normalizing so other subscribers keep the object. + data = dict(data) + data["arguments"] = json.dumps(data.get("arguments")) return f"event: {name}\ndata: {json.dumps(data)}\n\n" def _run_agent(self) -> object: diff --git a/tests/server/test_stream_bridge.py b/tests/server/test_stream_bridge.py new file mode 100644 index 00000000..727dda38 --- /dev/null +++ b/tests/server/test_stream_bridge.py @@ -0,0 +1,30 @@ +import json + +from openjarvis.server.stream_bridge import AgentStreamBridge + + +def test_tool_call_start_serializes_arguments_for_sse_without_mutating_event(): + bridge = object.__new__(AgentStreamBridge) + event_data = { + "tool": "web_search", + "arguments": {"query": "python"}, + "agent": "agent-1", + } + + event = bridge._format_named_event("tool_call_start", event_data) + payload = json.loads(event.split("data: ", 1)[1]) + + assert payload["arguments"] == '{"query": "python"}' + assert event_data["arguments"] == {"query": "python"} + + +def test_tool_call_start_preserves_already_serialized_arguments(): + bridge = object.__new__(AgentStreamBridge) + + event = bridge._format_named_event( + "tool_call_start", + {"tool": "web_search", "arguments": '{"query":"python"}'}, + ) + payload = json.loads(event.split("data: ", 1)[1]) + + assert payload["arguments"] == '{"query":"python"}'