From edcfdf970dbaf12c7750cba79eabb43f900d187b Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sat, 15 Nov 2025 16:28:57 +0100 Subject: [PATCH] refactor(ui): extract diff viewer and tool call parsers into reusable components --- packages/app/src/components/agent-list.tsx | 40 +- packages/app/src/components/diff-viewer.tsx | 120 +++ .../src/components/tool-call-bottom-sheet.tsx | 843 +----------------- packages/app/src/types/stream.test.ts | 56 ++ packages/app/src/types/stream.ts | 93 +- packages/app/src/utils/tool-call-parsers.ts | 706 +++++++++++++++ .../agent/providers/claude-agent.test.ts | 294 +++++- .../server/agent/providers/claude-agent.ts | 93 +- packages/server/vitest.config.ts | 6 + plan.md | 21 +- 10 files changed, 1402 insertions(+), 870 deletions(-) create mode 100644 packages/app/src/components/diff-viewer.tsx create mode 100644 packages/app/src/utils/tool-call-parsers.ts diff --git a/packages/app/src/components/agent-list.tsx b/packages/app/src/components/agent-list.tsx index be05b8336..274b2e579 100644 --- a/packages/app/src/components/agent-list.tsx +++ b/packages/app/src/components/agent-list.tsx @@ -4,6 +4,7 @@ import { StyleSheet, useUnistyles } from "react-native-unistyles"; import type { Agent } from "@/contexts/session-context"; import { formatTimeAgo } from "@/utils/time"; import { getAgentStatusColor, getAgentStatusLabel } from "@/utils/agent-status"; +import { getAgentProviderDefinition } from "@server/server/agent/provider-manifest"; interface AgentListProps { agents: Map; @@ -27,6 +28,7 @@ export function AgentList({ agents }: AgentListProps) { const statusColor = getAgentStatusColor(agent.status); const statusLabel = getAgentStatusLabel(agent.status); const timeAgo = formatTimeAgo(agent.lastActivityAt); + const providerLabel = getAgentProviderDefinition(agent.provider).label; return ( - + - - {statusLabel} - + style={[styles.providerBadge, { backgroundColor: theme.colors.muted }]} + > + + {providerLabel} + + + + + + + {statusLabel} + + @@ -111,6 +126,19 @@ const styles = StyleSheet.create((theme) => ({ justifyContent: "space-between", gap: theme.spacing[2], }, + statusGroup: { + flexDirection: "row", + alignItems: "center", + gap: theme.spacing[2], + }, + providerBadge: { + borderRadius: theme.borderRadius.full, + }, + providerText: { + fontSize: theme.fontSize.xs, + fontWeight: theme.fontWeight.semibold, + textTransform: "uppercase", + }, statusBadge: { flexDirection: "row", alignItems: "center", diff --git a/packages/app/src/components/diff-viewer.tsx b/packages/app/src/components/diff-viewer.tsx new file mode 100644 index 000000000..3180d32a9 --- /dev/null +++ b/packages/app/src/components/diff-viewer.tsx @@ -0,0 +1,120 @@ +import React from "react"; +import { View, Text } from "react-native"; +import { ScrollView } from "react-native-gesture-handler"; +import { StyleSheet } from "react-native-unistyles"; +import type { DiffLine } from "@/utils/tool-call-parsers"; + +interface DiffViewerProps { + diffLines: DiffLine[]; + maxHeight?: number; + emptyLabel?: string; +} + +export function DiffViewer({ diffLines, maxHeight = 280, emptyLabel = "No changes to display" }: DiffViewerProps) { + if (!diffLines.length) { + return ( + + {emptyLabel} + + ); + } + + return ( + + + + {diffLines.map((line, index) => ( + + + {line.content} + + + ))} + + + + ); +} + +const styles = StyleSheet.create((theme) => ({ + verticalScroll: {}, + verticalContent: { + flexGrow: 1, + }, + horizontalContent: { + flexDirection: "column" as const, + }, + linesContainer: { + alignSelf: "flex-start", + }, + line: { + minWidth: "100%", + paddingHorizontal: theme.spacing[3], + paddingVertical: theme.spacing[1], + }, + lineText: { + fontFamily: "monospace", + fontSize: theme.fontSize.xs, + color: theme.colors.foreground, + }, + headerLine: { + backgroundColor: theme.colors.muted, + }, + headerText: { + color: theme.colors.mutedForeground, + }, + addLine: { + backgroundColor: theme.colors.palette.green[900], + }, + addText: { + color: theme.colors.palette.green[200], + }, + removeLine: { + backgroundColor: theme.colors.palette.red[900], + }, + removeText: { + color: theme.colors.palette.red[200], + }, + contextLine: { + backgroundColor: theme.colors.card, + }, + contextText: { + color: theme.colors.mutedForeground, + }, + emptyState: { + padding: theme.spacing[4], + alignItems: "center" as const, + justifyContent: "center" as const, + }, + emptyText: { + fontSize: theme.fontSize.sm, + color: theme.colors.mutedForeground, + }, +})); diff --git a/packages/app/src/components/tool-call-bottom-sheet.tsx b/packages/app/src/components/tool-call-bottom-sheet.tsx index b59114b7a..73c9863f9 100644 --- a/packages/app/src/components/tool-call-bottom-sheet.tsx +++ b/packages/app/src/components/tool-call-bottom-sheet.tsx @@ -5,700 +5,20 @@ import { ScrollView } from "react-native-gesture-handler"; import { StyleSheet } from "react-native-unistyles"; import { BottomSheetModal, - BottomSheetView, BottomSheetScrollView, BottomSheetBackdrop, BottomSheetBackdropProps, } from "@gorhom/bottom-sheet"; import type { SelectedToolCall } from "@/types/shared"; -import { z } from "zod"; - -type DiffLine = { - type: "add" | "remove" | "context" | "header"; - content: string; -}; - -type EditEntry = { - filePath?: string; - diffLines: DiffLine[]; -}; - -type ReadEntry = { - filePath?: string; - content: string; -}; - -type CommandDetails = { - command?: string; - cwd?: string; - output?: string; - exitCode?: number | null; -}; - -function isRecord(value: unknown): value is Record { - return typeof value === "object" && value !== null; -} - -function getString(value: unknown): string | undefined { - return typeof value === "string" ? value : undefined; -} - -function splitIntoLines(text: string): string[] { - if (!text) { - return []; - } - - return text.replace(/\r\n/g, "\n").split("\n"); -} - -function buildLineDiff(originalText: string, updatedText: string): DiffLine[] { - const originalLines = splitIntoLines(originalText); - const updatedLines = splitIntoLines(updatedText); - - const hasAnyContent = originalLines.length > 0 || updatedLines.length > 0; - if (!hasAnyContent) { - return []; - } - - const m = originalLines.length; - const n = updatedLines.length; - const dp: number[][] = Array.from({ length: m + 1 }, () => - Array(n + 1).fill(0) - ); - - for (let i = m - 1; i >= 0; i -= 1) { - for (let j = n - 1; j >= 0; j -= 1) { - if (originalLines[i] === updatedLines[j]) { - dp[i][j] = dp[i + 1][j + 1] + 1; - } else { - dp[i][j] = Math.max(dp[i + 1][j], dp[i][j + 1]); - } - } - } - - const diff: DiffLine[] = []; - - let i = 0; - let j = 0; - while (i < m && j < n) { - if (originalLines[i] === updatedLines[j]) { - diff.push({ type: "context", content: ` ${originalLines[i]}` }); - i += 1; - j += 1; - } else if (dp[i + 1][j] >= dp[i][j + 1]) { - diff.push({ type: "remove", content: `-${originalLines[i]}` }); - i += 1; - } else { - diff.push({ type: "add", content: `+${updatedLines[j]}` }); - j += 1; - } - } - - while (i < m) { - diff.push({ type: "remove", content: `-${originalLines[i]}` }); - i += 1; - } - - while (j < n) { - diff.push({ type: "add", content: `+${updatedLines[j]}` }); - j += 1; - } - - return diff; -} - -function parseUnifiedDiff(diffText?: string): DiffLine[] { - if (!diffText) { - return []; - } - - const lines = splitIntoLines(diffText); - const diff: DiffLine[] = []; - - for (const line of lines) { - if (!line.length) { - diff.push({ type: "context", content: line }); - continue; - } - - if (line.startsWith("@@")) { - diff.push({ type: "header", content: line }); - continue; - } - - if (line.startsWith("+")) { - if (!line.startsWith("+++")) { - diff.push({ type: "add", content: line }); - } - continue; - } - - if (line.startsWith("-")) { - if (!line.startsWith("---")) { - diff.push({ type: "remove", content: line }); - } - continue; - } - - if ( - line.startsWith("diff --git") || - line.startsWith("index ") || - line.startsWith("---") || - line.startsWith("+++") - ) { - continue; - } - - if (line.startsWith("\\ No newline")) { - diff.push({ type: "header", content: line }); - continue; - } - - diff.push({ type: "context", content: line }); - } - - return diff; -} - -function deriveDiffLines({ - unifiedDiff, - original, - updated, -}: { - unifiedDiff?: string; - original?: string; - updated?: string; -}): DiffLine[] { - if (unifiedDiff) { - const parsed = parseUnifiedDiff(unifiedDiff); - if (parsed.length > 0) { - return parsed; - } - } - - if (original !== undefined || updated !== undefined) { - return buildLineDiff(original ?? "", updated ?? ""); - } - - return []; -} - -function looksLikePatch(text: string): boolean { - if (!text) { - return false; - } - return /(\*\*\* Begin Patch|@@|diff --git|\+\+\+|--- )/.test(text); -} - -function parsePatchText(text: string): DiffLine[] { - if (!text) { - return []; - } - return parseUnifiedDiff(text); -} - -function getFilePathFromRecord(record: Record): string | undefined { - return ( - getString(record["file_path"]) ?? - getString(record["filePath"]) ?? - getString(record["path"]) ?? - getString(record["target_path"]) ?? - getString(record["targetPath"]) ?? - undefined - ); -} - -const ChangeBlockSchema = z - .object({ - unified_diff: z.string().optional(), - unifiedDiff: z.string().optional(), - diff: z.string().optional(), - patch: z.string().optional(), - old_content: z.string().optional(), - oldContent: z.string().optional(), - previous_content: z.string().optional(), - previousContent: z.string().optional(), - base_content: z.string().optional(), - baseContent: z.string().optional(), - old_string: z.string().optional(), - new_string: z.string().optional(), - new_content: z.string().optional(), - newContent: z.string().optional(), - replace_with: z.string().optional(), - replaceWith: z.string().optional(), - content: z.string().optional(), - }) - .passthrough(); - -function buildEditEntryFromBlock( - filePath: string | undefined, - blockValue: Record -): EditEntry | null { - const parsed = ChangeBlockSchema.safeParse(blockValue); - if (!parsed.success) { - return null; - } - - const data = parsed.data; - const diffLines = deriveDiffLines({ - unifiedDiff: - getString( - data.unified_diff ?? - data.unifiedDiff ?? - data.patch ?? - data.diff - ) ?? undefined, - original: - getString( - data.old_string ?? - data.old_content ?? - data.oldContent ?? - data.previous_content ?? - data.previousContent ?? - data.base_content ?? - data.baseContent - ) ?? undefined, - updated: - getString( - data.new_string ?? - data.new_content ?? - data.newContent ?? - data.replace_with ?? - data.replaceWith ?? - data.content - ) ?? undefined, - }); - - if (diffLines.length > 0) { - return { - filePath: filePath ?? getFilePathFromRecord(blockValue), - diffLines, - }; - } - - const patchCandidate = - getString(data.unified_diff ?? data.unifiedDiff ?? data.patch ?? data.diff) ?? - undefined; - if (patchCandidate && looksLikePatch(patchCandidate)) { - const parsedLines = parsePatchText(patchCandidate); - if (parsedLines.length > 0) { - return { - filePath: filePath ?? getFilePathFromRecord(blockValue), - diffLines: parsedLines, - }; - } - } - - return null; -} - -function mergeEditEntries(entries: EditEntry[]): EditEntry[] { - if (entries.length === 0) { - return []; - } - const seen = new Map(); - entries.forEach((entry) => { - if (!entry.diffLines.length) { - return; - } - const hash = `${entry.filePath ?? "unknown"}::${entry.diffLines - .map((line) => `${line.type}:${line.content}`) - .join("|")}`; - if (!seen.has(hash)) { - seen.set(hash, entry); - } - }); - return Array.from(seen.values()); -} - -function parseEditArguments(value: unknown, depth = 0): EditEntry[] { - if (!value || depth > 5) { - return []; - } - - if (typeof value === "string") { - if (looksLikePatch(value)) { - const diffLines = parsePatchText(value); - return diffLines.length ? [{ diffLines }] : []; - } - return []; - } - - if (Array.isArray(value)) { - return mergeEditEntries( - value.flatMap((entry) => parseEditArguments(entry, depth + 1)) - ); - } - - if (!isRecord(value)) { - return []; - } - - const entries: EditEntry[] = []; - const filePathHint = getFilePathFromRecord(value); - - const changesParse = z.record(z.unknown()).safeParse(value["changes"]); - if (changesParse.success) { - for (const [filePath, changeValue] of Object.entries(changesParse.data)) { - const nested = parseEditArguments(changeValue, depth + 1); - entries.push( - ...nested.map((entry) => ({ - ...entry, - filePath: entry.filePath ?? filePath ?? filePathHint, - })) - ); - } - } - - if (Array.isArray(value["files"])) { - for (const fileEntry of value["files"] as unknown[]) { - const nested = parseEditArguments(fileEntry, depth + 1); - const derivedFilePath = isRecord(fileEntry) - ? getFilePathFromRecord(fileEntry) - : undefined; - const resolvedFilePath = derivedFilePath ?? filePathHint; - entries.push( - ...nested.map((entry) => ({ - ...entry, - filePath: entry.filePath ?? resolvedFilePath, - })) - ); - } - } - - const direct = buildEditEntryFromBlock(filePathHint, value); - if (direct) { - entries.push(direct); - } else { - const patchCandidates = [ - getString(value["patch"]), - getString(value["diff"]), - getString(value["unified_diff"]), - getString(value["unifiedDiff"]), - ].filter(Boolean) as string[]; - for (const patch of patchCandidates) { - if (patch && looksLikePatch(patch)) { - const diffLines = parsePatchText(patch); - if (diffLines.length) { - entries.push({ filePath: filePathHint, diffLines }); - break; - } - } - } - } - - const nestedKeys = [ - "input", - "update", - "create", - "delete", - "raw", - "data", - "payload", - "arguments", - "result", - ] as const; - for (const key of nestedKeys) { - if (value[key] !== undefined) { - const nestedEntries = parseEditArguments(value[key], depth + 1); - entries.push( - ...nestedEntries.map((entry) => ({ - ...entry, - filePath: entry.filePath ?? filePathHint, - })) - ); - } - } - - return mergeEditEntries(entries); -} - -const ReadContainerSchema = z - .object({ - filePath: z.string().optional(), - file_path: z.string().optional(), - path: z.string().optional(), - content: z.string().optional(), - text: z.string().optional(), - blob: z.string().optional(), - data: z - .object({ - content: z.string().optional(), - text: z.string().optional(), - }) - .optional(), - structuredContent: z - .object({ - content: z.string().optional(), - text: z.string().optional(), - data: z - .object({ - content: z.string().optional(), - text: z.string().optional(), - }) - .optional(), - }) - .optional(), - structured_content: z - .object({ - content: z.string().optional(), - text: z.string().optional(), - }) - .optional(), - output: z - .object({ - content: z.string().optional(), - text: z.string().optional(), - }) - .optional(), - }) - .passthrough(); - -function parseReadEntries(value: unknown, depth = 0): ReadEntry[] { - if (!value || depth > 4) { - return []; - } - - if (typeof value === "string") { - const trimmed = value.trim(); - return trimmed.length ? [{ content: value }] : []; - } - - if (Array.isArray(value)) { - return value.flatMap((entry) => parseReadEntries(entry, depth + 1)); - } - - if (!isRecord(value)) { - return []; - } - - const parsed = ReadContainerSchema.safeParse(value); - if (parsed.success) { - const data = parsed.data; - const content = - getString(data.content) ?? - getString(data.text) ?? - getString(data.blob) ?? - getString(data.data?.content) ?? - getString(data.data?.text) ?? - getString(data.structuredContent?.content) ?? - getString(data.structuredContent?.text) ?? - getString(data.structuredContent?.data?.content) ?? - getString(data.structuredContent?.data?.text) ?? - getString(data.structured_content?.content) ?? - getString(data.structured_content?.text) ?? - getString(data.output?.content) ?? - getString(data.output?.text); - if (content) { - return [ - { - filePath: data.filePath ?? data.file_path ?? data.path, - content, - }, - ]; - } - } - - const nestedKeys = [ - "output", - "result", - "structuredContent", - "structured_content", - "data", - "raw", - "value", - "content", - ] as const; - const entries: ReadEntry[] = []; - for (const key of nestedKeys) { - if (value[key] !== undefined) { - entries.push(...parseReadEntries(value[key], depth + 1)); - } - } - return entries; -} - -function mergeReadEntries(entries: ReadEntry[]): ReadEntry[] { - if (!entries.length) { - return []; - } - const seen = new Map(); - entries.forEach((entry) => { - const hash = `${entry.filePath ?? "content"}::${entry.content}`; - if (!seen.has(hash)) { - seen.set(hash, entry); - } - }); - return Array.from(seen.values()); -} - -const CommandRawSchema = z - .object({ - type: z.string().optional(), - command: z.union([z.string(), z.array(z.string())]).optional(), - aggregated_output: z.string().optional(), - exit_code: z.number().optional(), - cwd: z.string().optional(), - directory: z.string().optional(), - metadata: z - .object({ - exit_code: z.number().optional(), - }) - .optional(), - input: z.unknown().optional(), - output: z.unknown().optional(), - }) - .passthrough(); - -const CommandResultSchema = z - .object({ - output: z.string().optional(), - exitCode: z.number().nullable().optional(), - structuredContent: z - .object({ - output: z.string().optional(), - text: z.string().optional(), - content: z.string().optional(), - }) - .optional(), - structured_content: z - .object({ - output: z.string().optional(), - text: z.string().optional(), - content: z.string().optional(), - }) - .optional(), - metadata: z - .object({ - exit_code: z.number().optional(), - }) - .optional(), - result: z.unknown().optional(), - }) - .passthrough(); - -function coerceCommandValue(value: unknown): string | undefined { - if (typeof value === "string" && value.length > 0) { - return value; - } - if (Array.isArray(value)) { - const tokens = value.filter((entry): entry is string => typeof entry === "string"); - if (tokens.length) { - return tokens.join(" "); - } - } - return undefined; -} - -function collectCommandDetails( - target: CommandDetails, - value: unknown, - depth = 0 -): void { - if (!value || depth > 4) { - return; - } - - if (typeof value === "string") { - if (!target.output) { - target.output = value; - } - return; - } - - if (!isRecord(value)) { - return; - } - - const rawParsed = CommandRawSchema.safeParse(value); - if (rawParsed.success) { - const data = rawParsed.data; - const commandCandidate = - coerceCommandValue(data.command) ?? - (isRecord(data.input) ? coerceCommandValue(data.input["command"]) : undefined); - if (!target.command && commandCandidate) { - target.command = commandCandidate; - } - const cwdCandidate = - getString(data.cwd ?? data.directory) ?? - (isRecord(data.input) - ? getString(data.input["cwd"] ?? data.input["directory"]) - : undefined); - if (!target.cwd && cwdCandidate) { - target.cwd = cwdCandidate; - } - const aggregatedOutput = - getString(data.aggregated_output) ?? - (isRecord(data.output) - ? getString( - (data.output as Record)["aggregated_output"] ?? - (data.output as Record)["output"] ?? - (data.output as Record)["text"] - ) - : undefined); - if (!target.output && aggregatedOutput) { - target.output = aggregatedOutput; - } - const exitCandidate = - data.exit_code ?? - (data.metadata ? data.metadata.exit_code : undefined) ?? - (isRecord(data.output) - ? ((data.output as Record)["exit_code"] as number | undefined) ?? - ((data.output as Record)["exitCode"] as number | undefined) - : undefined); - if ((typeof exitCandidate === "number" || exitCandidate === null) && target.exitCode === undefined) { - target.exitCode = exitCandidate ?? null; - } - } - - const resultParsed = CommandResultSchema.safeParse(value); - if (resultParsed.success) { - const data = resultParsed.data; - const outputCandidate = - getString(data.output) ?? - getString(data.structuredContent?.output) ?? - getString(data.structuredContent?.text) ?? - getString(data.structured_content?.output) ?? - getString(data.structured_content?.text) ?? - undefined; - if (!target.output && outputCandidate) { - target.output = outputCandidate; - } - const exitCandidate = data.exitCode ?? data.metadata?.exit_code; - if ((typeof exitCandidate === "number" || exitCandidate === null) && target.exitCode === undefined) { - target.exitCode = exitCandidate ?? null; - } - if (data.result !== undefined) { - collectCommandDetails(target, data.result, depth + 1); - } - } - - const nestedKeys = [ - "input", - "output", - "result", - "raw", - "data", - "payload", - "structuredContent", - "structured_content", - ] as const; - for (const key of nestedKeys) { - if (value[key] !== undefined) { - collectCommandDetails(target, value[key], depth + 1); - } - } -} - -function parseCommandDetails(args: unknown, result: unknown): CommandDetails | null { - const details: CommandDetails = {}; - collectCommandDetails(details, args); - collectCommandDetails(details, result); - if (details.command || details.output) { - return details; - } - return null; -} +import { + extractCommandDetails, + extractEditEntries, + extractReadEntries, + type CommandDetails, + type EditEntry, + type ReadEntry, +} from "@/utils/tool-call-parsers"; +import { DiffViewer } from "./diff-viewer"; interface ToolCallBottomSheetProps { bottomSheetRef: React.RefObject; @@ -727,13 +47,16 @@ export function ToolCallBottomSheet({ ); // Extract data based on source - const { toolName, args, result, error } = useMemo(() => { + const { toolName, args, result, error, parsedEdits, parsedReads, parsedCommand } = useMemo(() => { if (!selectedToolCall) { return { toolName: "Tool Call", args: undefined, result: undefined, error: undefined, + parsedEdits: undefined, + parsedReads: undefined, + parsedCommand: undefined, }; } @@ -746,6 +69,9 @@ export function ToolCallBottomSheet({ args: data.raw, result: data.result, error: data.error, + parsedEdits: data.parsedEdits, + parsedReads: data.parsedReads, + parsedCommand: data.parsedCommand, }; } @@ -755,29 +81,30 @@ export function ToolCallBottomSheet({ args: data.arguments, result: data.result, error: data.error, + parsedEdits: undefined, + parsedReads: undefined, + parsedCommand: undefined, }; }, [selectedToolCall]); - const editEntries = useMemo(() => { - const entries = [ - ...parseEditArguments(args), - ...parseEditArguments(result), - ]; - return mergeEditEntries(entries); - }, [args, result]); - - const readEntries = useMemo(() => { - const entries = [ - ...parseReadEntries(result), - ...parseReadEntries(args), - ]; - return mergeReadEntries(entries); - }, [args, result]); - - const commandDetails = useMemo(() => parseCommandDetails(args, result), [ + const fallbackEditEntries = useMemo(() => extractEditEntries(args, result), [ args, result, ]); + const editEntries: EditEntry[] = parsedEdits ?? fallbackEditEntries; + + const fallbackReadEntries = useMemo(() => extractReadEntries(result, args), [ + args, + result, + ]); + const readEntries: ReadEntry[] = parsedReads ?? fallbackReadEntries; + + const fallbackCommandDetails = useMemo( + () => extractCommandDetails(args, result), + [args, result] + ); + const commandDetails: CommandDetails | null = + parsedCommand ?? fallbackCommandDetails; return ( Diff - {entry.diffLines.length === 0 ? ( - - No changes to display - - ) : ( - - - - {entry.diffLines.map((line, lineIndex) => ( - - - {line.content} - - - ))} - - - - )} + ))} @@ -1039,61 +321,6 @@ const styles = StyleSheet.create((theme) => ({ backgroundColor: theme.colors.card, overflow: "hidden", }, - diffScrollVertical: { - maxHeight: 280, - }, - diffVerticalContent: { - flexGrow: 1, - }, - diffScrollContent: { - flexDirection: "column" as const, - }, - diffLinesContainer: { - alignSelf: "flex-start", - }, - diffLine: { - minWidth: "100%", - paddingHorizontal: theme.spacing[3], - paddingVertical: theme.spacing[1], - }, - diffLineText: { - fontFamily: "monospace", - fontSize: theme.fontSize.xs, - color: theme.colors.foreground, - }, - diffHeaderLine: { - backgroundColor: theme.colors.muted, - }, - diffHeaderText: { - color: theme.colors.mutedForeground, - }, - diffAddLine: { - backgroundColor: theme.colors.palette.green[900], - }, - diffAddText: { - color: theme.colors.palette.green[200], - }, - diffRemoveLine: { - backgroundColor: theme.colors.palette.red[900], - }, - diffRemoveText: { - color: theme.colors.palette.red[200], - }, - diffContextLine: { - backgroundColor: theme.colors.card, - }, - diffContextText: { - color: theme.colors.mutedForeground, - }, - diffEmptyState: { - padding: theme.spacing[4], - alignItems: "center" as const, - justifyContent: "center" as const, - }, - diffEmptyText: { - fontSize: theme.fontSize.sm, - color: theme.colors.mutedForeground, - }, jsonContainer: { backgroundColor: theme.colors.background, borderRadius: theme.borderRadius.lg, diff --git a/packages/app/src/types/stream.test.ts b/packages/app/src/types/stream.test.ts index 86314f08c..454a71a4c 100644 --- a/packages/app/src/types/stream.test.ts +++ b/packages/app/src/types/stream.test.ts @@ -1064,6 +1064,60 @@ function testToolCallDeduplicationHydrated() { }); } +function buildOutOfOrderToolCallSequence(provider: ToolCallProvider) { + const timestamps = [ + new Date('2025-01-01T13:00:00Z'), + new Date('2025-01-01T13:00:05Z'), + ]; + const callId = `${provider}-out-of-order`; + return [ + { + event: toolTimeline( + 'shell', + 'completed', + { type: 'tool_result', provider, tool_call_id: callId }, + { provider, server: 'command', tool: 'shell', callId } + ), + timestamp: timestamps[0], + }, + { + event: toolTimeline( + 'shell', + 'executing', + { type: 'tool_use', provider }, + { provider, server: 'command', tool: 'shell', callId: null } + ), + timestamp: timestamps[1], + }, + ]; +} + +function testOutOfOrderToolCallMergingLive() { + (['claude', 'codex'] as const).forEach((provider) => { + const updates = buildOutOfOrderToolCallSequence(provider); + const finalState = updates.reduce((state, { event, timestamp }) => { + return reduceStreamUpdate(state, event, timestamp); + }, []); + const toolCalls = finalState.filter( + (item): item is ToolCallItem => item.kind === 'tool_call' && item.payload.source === 'agent' + ); + assert.strictEqual(toolCalls.length, 1, `${provider} live stream should not duplicate out-of-order calls`); + assert.strictEqual(toolCalls[0]?.payload.data.status, 'completed', `${provider} live stream should keep completed status`); + }); +} + +function testOutOfOrderToolCallMergingHydrated() { + (['claude', 'codex'] as const).forEach((provider) => { + const updates = buildOutOfOrderToolCallSequence(provider); + const hydrated = hydrateStreamState(updates); + const toolCalls = hydrated.filter( + (item): item is ToolCallItem => item.kind === 'tool_call' && item.payload.source === 'agent' + ); + assert.strictEqual(toolCalls.length, 1, `${provider} hydration should not duplicate out-of-order calls`); + assert.strictEqual(toolCalls[0]?.payload.data.status, 'completed', `${provider} hydration should keep completed status`); + }); +} + describe('stream timeline reducers', () => { it('produces deterministic hydration results', testIdempotentReduction); it('deduplicates pending/completed tool entries in place', testUserMessageDeduplication); @@ -1083,4 +1137,6 @@ describe('stream timeline reducers', () => { it('keeps timeline ids stable after list shrinkage', testTimelineIdStabilityAfterRemovals); it('deduplicates live tool call entries', testToolCallDeduplicationLive); it('deduplicates hydrated tool call entries', testToolCallDeduplicationHydrated); + it('merges out-of-order tool call updates without duplicating entries (live)', testOutOfOrderToolCallMergingLive); + it('merges out-of-order tool call updates without duplicating entries (hydrated)', testOutOfOrderToolCallMergingHydrated); }); diff --git a/packages/app/src/types/stream.ts b/packages/app/src/types/stream.ts index 75172ebdf..7d0bb172d 100644 --- a/packages/app/src/types/stream.ts +++ b/packages/app/src/types/stream.ts @@ -35,13 +35,30 @@ function createTimelineId(prefix: string, text: string, timestamp: Date): string function createUniqueTimelineId( state: StreamItem[], - prefix: "assistant" | "thought", + prefix: "assistant" | "thought" | "user", text: string, timestamp: Date ): string { const base = createTimelineId(prefix, text, timestamp); - const suffixSeed = state.length.toString(36); - return `${base}_${suffixSeed}`; + let suffixSeed = state.length; + let candidate = `${base}_${suffixSeed.toString(36)}`; + + // Fast path when the generated id hasn't been used yet + const hasCollision = state.some((entry) => entry.id === candidate); + if (!hasCollision) { + return candidate; + } + + // If the id already exists (e.g. prior items were pruned/replaced), + // spin until we find an unused suffix. Building the lookup Set lazily keeps + // the common case cheap while still guaranteeing uniqueness. + const usedIds = new Set(state.map((entry) => entry.id)); + while (usedIds.has(candidate)) { + suffixSeed += 1; + candidate = `${base}_${suffixSeed.toString(36)}`; + } + + return candidate; } export type StreamItem = @@ -157,7 +174,9 @@ function appendUserMessage( return state; } - const entryId = messageId ?? createTimelineId("user", chunk.trim() || chunk, timestamp); + const chunkSeed = chunk.trim() || chunk; + const entryId = + messageId ?? createUniqueTimelineId(state, "user", chunkSeed, timestamp); const existingIndex = state.findIndex( (entry) => entry.kind === "user_message" && entry.id === entryId ); @@ -297,23 +316,27 @@ function findExistingAgentToolCallIndex( } const fallbackCandidates: Array<{ index: number; item: AgentToolCallItem }> = []; + const metadataMatches: Array<{ index: number; item: AgentToolCallItem }> = []; for (let i = 0; i < state.length; i += 1) { const entry = state[i]; if (entry.kind !== "tool_call" || entry.payload.source !== "agent") { continue; } const payload = entry.payload.data; + const providerMatches = + payload.provider === data.provider && + payload.server === data.server && + payload.tool === data.tool; + if (providerMatches) { + metadataMatches.push({ index: i, item: entry as AgentToolCallItem }); + } if (payload.callId) { continue; } if (payload.status !== "executing") { continue; } - if ( - payload.provider === data.provider && - payload.server === data.server && - payload.tool === data.tool - ) { + if (providerMatches) { fallbackCandidates.push({ index: i, item: entry as AgentToolCallItem }); } } @@ -346,14 +369,34 @@ function findExistingAgentToolCallIndex( normalizedDisplayName ); - const byKind = filterByComparableField( - byDisplayName, - (entry) => normalizeComparableString(entry.payload.data.kind), - normalizedKind - ); + const selectCandidate = ( + candidates: Array<{ index: number; item: AgentToolCallItem }> + ): number => { + const byDisplayName = filterByComparableField( + candidates, + (entry) => normalizeComparableString(entry.payload.data.displayName), + normalizedDisplayName + ); - // Fall back to the oldest candidate (FIFO) to avoid duplicating the initial "executing" pill. - return byKind[0]?.index ?? -1; + const byKind = filterByComparableField( + byDisplayName, + (entry) => normalizeComparableString(entry.payload.data.kind), + normalizedKind + ); + + return byKind[0]?.index ?? -1; + }; + + if (fallbackCandidates.length) { + return selectCandidate(fallbackCandidates); + } + + // If this update still lacks a call id, fall back to metadata matches (e.g. replayed hydration events) + if (!normalizedCallId && metadataMatches.length) { + return selectCandidate(metadataMatches); + } + + return -1; } function appendAgentToolCall( @@ -389,6 +432,10 @@ function appendAgentToolCall( payloadData.error !== undefined ? payloadData.error : existing.payload.data.error; + const mergedStatus = mergeToolCallStatus( + existing.payload.data.status, + payloadData.status ?? existing.payload.data.status ?? "executing" + ); const parsed = computeParsedToolPayload(mergedRaw, mergedResult); next[existingIndex] = { ...existing, @@ -398,6 +445,7 @@ function appendAgentToolCall( data: { ...existing.payload.data, ...payloadData, + status: mergedStatus, raw: mergedRaw, result: mergedResult, error: mergedError, @@ -579,6 +627,19 @@ function normalizeToolCallStatus( return normalizedFromStatus ?? "executing"; } +function mergeToolCallStatus( + existing: "executing" | "completed" | "failed" | undefined, + incoming: "executing" | "completed" | "failed" +): "executing" | "completed" | "failed" { + if (existing === "failed" || incoming === "failed") { + return "failed"; + } + if (existing === "completed" || incoming === "completed") { + return "completed"; + } + return incoming ?? existing ?? "executing"; +} + const TOOL_CALL_ID_KEYS = [ "toolCallId", "tool_call_id", diff --git a/packages/app/src/utils/tool-call-parsers.ts b/packages/app/src/utils/tool-call-parsers.ts new file mode 100644 index 000000000..522f33d95 --- /dev/null +++ b/packages/app/src/utils/tool-call-parsers.ts @@ -0,0 +1,706 @@ +import { z } from "zod"; + +export type DiffLine = { + type: "add" | "remove" | "context" | "header"; + content: string; +}; + +export type EditEntry = { + filePath?: string; + diffLines: DiffLine[]; +}; + +export type ReadEntry = { + filePath?: string; + content: string; +}; + +export type CommandDetails = { + command?: string; + cwd?: string; + output?: string; + exitCode?: number | null; +}; + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null; +} + +function getString(value: unknown): string | undefined { + return typeof value === "string" ? value : undefined; +} + +function splitIntoLines(text: string): string[] { + if (!text) { + return []; + } + + return text.replace(/\r\n/g, "\n").split("\n"); +} + +function buildLineDiff(originalText: string, updatedText: string): DiffLine[] { + const originalLines = splitIntoLines(originalText); + const updatedLines = splitIntoLines(updatedText); + + const hasAnyContent = originalLines.length > 0 || updatedLines.length > 0; + if (!hasAnyContent) { + return []; + } + + const m = originalLines.length; + const n = updatedLines.length; + const dp: number[][] = Array.from({ length: m + 1 }, () => + Array(n + 1).fill(0) + ); + + for (let i = m - 1; i >= 0; i -= 1) { + for (let j = n - 1; j >= 0; j -= 1) { + if (originalLines[i] === updatedLines[j]) { + dp[i][j] = dp[i + 1][j + 1] + 1; + } else { + dp[i][j] = Math.max(dp[i + 1][j], dp[i][j + 1]); + } + } + } + + const diff: DiffLine[] = []; + + let i = 0; + let j = 0; + while (i < m && j < n) { + if (originalLines[i] === updatedLines[j]) { + diff.push({ type: "context", content: ` ${originalLines[i]}` }); + i += 1; + j += 1; + } else if (dp[i + 1][j] >= dp[i][j + 1]) { + diff.push({ type: "remove", content: `-${originalLines[i]}` }); + i += 1; + } else { + diff.push({ type: "add", content: `+${updatedLines[j]}` }); + j += 1; + } + } + + while (i < m) { + diff.push({ type: "remove", content: `-${originalLines[i]}` }); + i += 1; + } + + while (j < n) { + diff.push({ type: "add", content: `+${updatedLines[j]}` }); + j += 1; + } + + return diff; +} + +function parseUnifiedDiff(diffText?: string): DiffLine[] { + if (!diffText) { + return []; + } + + const lines = splitIntoLines(diffText); + const diff: DiffLine[] = []; + + for (const line of lines) { + if (!line.length) { + diff.push({ type: "context", content: line }); + continue; + } + + if (line.startsWith("@@")) { + diff.push({ type: "header", content: line }); + continue; + } + + if (line.startsWith("+")) { + if (!line.startsWith("+++")) { + diff.push({ type: "add", content: line }); + } + continue; + } + + if (line.startsWith("-")) { + if (!line.startsWith("---")) { + diff.push({ type: "remove", content: line }); + } + continue; + } + + if ( + line.startsWith("diff --git") || + line.startsWith("index ") || + line.startsWith("---") || + line.startsWith("+++") + ) { + continue; + } + + if (line.startsWith("\\ No newline")) { + diff.push({ type: "header", content: line }); + continue; + } + + diff.push({ type: "context", content: line }); + } + + return diff; +} + +function deriveDiffLines({ + unifiedDiff, + original, + updated, +}: { + unifiedDiff?: string; + original?: string; + updated?: string; +}): DiffLine[] { + if (unifiedDiff) { + const parsed = parseUnifiedDiff(unifiedDiff); + if (parsed.length > 0) { + return parsed; + } + } + + if (original !== undefined || updated !== undefined) { + return buildLineDiff(original ?? "", updated ?? ""); + } + + return []; +} + +function looksLikePatch(text: string): boolean { + if (!text) { + return false; + } + return /(\*\*\* Begin Patch|@@|diff --git|\+\+\+|--- )/.test(text); +} + +function parsePatchText(text: string): DiffLine[] { + if (!text) { + return []; + } + return parseUnifiedDiff(text); +} + +function getFilePathFromRecord(record: Record): string | undefined { + return ( + getString(record["file_path"]) ?? + getString(record["filePath"]) ?? + getString(record["path"]) ?? + getString(record["target_path"]) ?? + getString(record["targetPath"]) ?? + undefined + ); +} + +const ChangeBlockSchema = z + .object({ + unified_diff: z.string().optional(), + unifiedDiff: z.string().optional(), + diff: z.string().optional(), + patch: z.string().optional(), + old_content: z.string().optional(), + oldContent: z.string().optional(), + previous_content: z.string().optional(), + previousContent: z.string().optional(), + base_content: z.string().optional(), + baseContent: z.string().optional(), + old_string: z.string().optional(), + new_string: z.string().optional(), + new_content: z.string().optional(), + newContent: z.string().optional(), + replace_with: z.string().optional(), + replaceWith: z.string().optional(), + content: z.string().optional(), + }) + .passthrough(); + +function buildEditEntryFromBlock( + filePath: string | undefined, + blockValue: Record +): EditEntry | null { + const parsed = ChangeBlockSchema.safeParse(blockValue); + if (!parsed.success) { + return null; + } + + const data = parsed.data; + const diffLines = deriveDiffLines({ + unifiedDiff: + getString( + data.unified_diff ?? + data.unifiedDiff ?? + data.patch ?? + data.diff + ) ?? undefined, + original: + getString( + data.old_string ?? + data.old_content ?? + data.oldContent ?? + data.previous_content ?? + data.previousContent ?? + data.base_content ?? + data.baseContent + ) ?? undefined, + updated: + getString( + data.new_string ?? + data.new_content ?? + data.newContent ?? + data.replace_with ?? + data.replaceWith ?? + data.content + ) ?? undefined, + }); + + if (diffLines.length > 0) { + return { + filePath: filePath ?? getFilePathFromRecord(blockValue), + diffLines, + }; + } + + const patchCandidate = + getString(data.unified_diff ?? data.unifiedDiff ?? data.patch ?? data.diff) ?? + undefined; + if (patchCandidate && looksLikePatch(patchCandidate)) { + const parsedLines = parsePatchText(patchCandidate); + if (parsedLines.length > 0) { + return { + filePath: filePath ?? getFilePathFromRecord(blockValue), + diffLines: parsedLines, + }; + } + } + + return null; +} + +function mergeEditEntries(entries: EditEntry[]): EditEntry[] { + if (entries.length === 0) { + return []; + } + const seen = new Map(); + entries.forEach((entry) => { + if (!entry.diffLines.length) { + return; + } + const hash = `${entry.filePath ?? "unknown"}::${entry.diffLines + .map((line) => `${line.type}:${line.content}`) + .join("|")}`; + if (!seen.has(hash)) { + seen.set(hash, entry); + } + }); + return Array.from(seen.values()); +} + +function parseEditArguments(value: unknown, depth = 0): EditEntry[] { + if (!value || depth > 5) { + return []; + } + + if (typeof value === "string") { + if (looksLikePatch(value)) { + const diffLines = parsePatchText(value); + return diffLines.length ? [{ diffLines }] : []; + } + return []; + } + + if (Array.isArray(value)) { + return value.flatMap((entry) => parseEditArguments(entry, depth + 1)); + } + + if (!isRecord(value)) { + return []; + } + + const filePathHint = getFilePathFromRecord(value) ?? getString(value["name"]); + + if (value["patch"] || value["diff"] || value["unified_diff"] || value["unifiedDiff"]) { + const entry = buildEditEntryFromBlock(filePathHint, value); + return entry ? [entry] : []; + } + + const entries: EditEntry[] = []; + const changeKeys = [ + "changes", + "files", + "fileChanges", + "file_changes", + "edits", + "diffs", + "patches", + "fileDiffs", + "file_diffs", + ] as const; + + for (const key of changeKeys) { + const block = value[key]; + if (!block) { + continue; + } + if (Array.isArray(block)) { + for (const item of block) { + if (isRecord(item)) { + const entry = buildEditEntryFromBlock(filePathHint, item); + if (entry) { + entries.push(entry); + } + } + } + continue; + } + if (isRecord(block)) { + if (block["patch"] || block["diff"]) { + const entry = buildEditEntryFromBlock(filePathHint, block); + if (entry) { + entries.push(entry); + } + continue; + } + for (const [path, nested] of Object.entries(block)) { + if (isRecord(nested)) { + const entry = buildEditEntryFromBlock(path, nested); + if (entry) { + entries.push(entry); + } + } else if (typeof nested === "string" && looksLikePatch(nested)) { + const diffLines = parsePatchText(nested); + if (diffLines.length) { + entries.push({ filePath: path, diffLines }); + } + } + } + } + } + + const changeEntry = buildEditEntryFromBlock(filePathHint, value); + if (changeEntry) { + entries.push(changeEntry); + } + + const nestedKeys = [ + "create", + "delete", + "raw", + "data", + "payload", + "arguments", + "result", + ] as const; + for (const key of nestedKeys) { + if (value[key] !== undefined) { + const nestedEntries = parseEditArguments(value[key], depth + 1); + entries.push( + ...nestedEntries.map((entry) => ({ + ...entry, + filePath: entry.filePath ?? filePathHint, + })) + ); + } + } + + return entries; +} + +const ReadContainerSchema = z + .object({ + filePath: z.string().optional(), + file_path: z.string().optional(), + path: z.string().optional(), + content: z.string().optional(), + text: z.string().optional(), + blob: z.string().optional(), + data: z + .object({ + content: z.string().optional(), + text: z.string().optional(), + }) + .optional(), + structuredContent: z + .object({ + content: z.string().optional(), + text: z.string().optional(), + data: z + .object({ + content: z.string().optional(), + text: z.string().optional(), + }) + .optional(), + }) + .optional(), + structured_content: z + .object({ + content: z.string().optional(), + text: z.string().optional(), + }) + .optional(), + output: z + .object({ + content: z.string().optional(), + text: z.string().optional(), + }) + .optional(), + }) + .passthrough(); + +function parseReadEntriesInternal(value: unknown, depth = 0): ReadEntry[] { + if (!value || depth > 4) { + return []; + } + + if (typeof value === "string") { + const trimmed = value.trim(); + return trimmed.length ? [{ content: value }] : []; + } + + if (Array.isArray(value)) { + return value.flatMap((entry) => parseReadEntriesInternal(entry, depth + 1)); + } + + if (!isRecord(value)) { + return []; + } + + const parsed = ReadContainerSchema.safeParse(value); + if (parsed.success) { + const data = parsed.data; + const content = + getString(data.content) ?? + getString(data.text) ?? + getString(data.blob) ?? + getString(data.data?.content) ?? + getString(data.data?.text) ?? + getString(data.structuredContent?.content) ?? + getString(data.structuredContent?.text) ?? + getString(data.structuredContent?.data?.content) ?? + getString(data.structuredContent?.data?.text) ?? + getString(data.structured_content?.content) ?? + getString(data.structured_content?.text) ?? + getString(data.output?.content) ?? + getString(data.output?.text); + if (content) { + return [ + { + filePath: data.filePath ?? data.file_path ?? data.path, + content, + }, + ]; + } + } + + const nestedKeys = [ + "output", + "result", + "structuredContent", + "structured_content", + "data", + "raw", + "value", + "content", + ] as const; + const entries: ReadEntry[] = []; + for (const key of nestedKeys) { + if (value[key] !== undefined) { + entries.push(...parseReadEntriesInternal(value[key], depth + 1)); + } + } + return entries; +} + +function mergeReadEntries(entries: ReadEntry[]): ReadEntry[] { + if (!entries.length) { + return []; + } + const seen = new Map(); + entries.forEach((entry) => { + const hash = `${entry.filePath ?? "content"}::${entry.content}`; + if (!seen.has(hash)) { + seen.set(hash, entry); + } + }); + return Array.from(seen.values()); +} + +const CommandRawSchema = z + .object({ + type: z.string().optional(), + command: z.union([z.string(), z.array(z.string())]).optional(), + aggregated_output: z.string().optional(), + exit_code: z.number().optional(), + cwd: z.string().optional(), + directory: z.string().optional(), + metadata: z + .object({ + exit_code: z.number().optional(), + }) + .optional(), + input: z.unknown().optional(), + output: z.unknown().optional(), + }) + .passthrough(); + +const CommandResultSchema = z + .object({ + output: z.string().optional(), + exitCode: z.number().nullable().optional(), + structuredContent: z + .object({ + output: z.string().optional(), + text: z.string().optional(), + content: z.string().optional(), + }) + .optional(), + structured_content: z + .object({ + output: z.string().optional(), + text: z.string().optional(), + content: z.string().optional(), + }) + .optional(), + metadata: z + .object({ + exit_code: z.number().optional(), + }) + .optional(), + result: z.unknown().optional(), + }) + .passthrough(); + +function coerceCommandValue(value: unknown): string | undefined { + if (typeof value === "string" && value.length > 0) { + return value; + } + if (Array.isArray(value)) { + const tokens = value.filter((entry): entry is string => typeof entry === "string"); + if (tokens.length) { + return tokens.join(" "); + } + } + return undefined; +} + +function collectCommandDetails( + target: CommandDetails, + value: unknown, + depth = 0 +): void { + if (!value || depth > 4) { + return; + } + + if (typeof value === "string") { + if (!target.output) { + target.output = value; + } + return; + } + + if (!isRecord(value)) { + return; + } + + const rawParsed = CommandRawSchema.safeParse(value); + if (rawParsed.success) { + const data = rawParsed.data; + const commandCandidate = + coerceCommandValue(data.command) ?? + (isRecord(data.input) ? coerceCommandValue(data.input["command"]) : undefined); + if (!target.command && commandCandidate) { + target.command = commandCandidate; + } + const cwdCandidate = + getString(data.cwd ?? data.directory) ?? + (isRecord(data.input) + ? getString(data.input["cwd"] ?? data.input["directory"]) + : undefined); + if (!target.cwd && cwdCandidate) { + target.cwd = cwdCandidate; + } + const aggregatedOutput = + getString(data.aggregated_output) ?? + (isRecord(data.output) + ? getString( + (data.output as Record)["aggregated_output"] ?? + (data.output as Record)["output"] ?? + (data.output as Record)["text"] + ) + : undefined); + if (!target.output && aggregatedOutput) { + target.output = aggregatedOutput; + } + const exitCandidate = + data.exit_code ?? + (data.metadata ? data.metadata.exit_code : undefined) ?? + (isRecord(data.output) + ? ((data.output as Record)["exit_code"] as number | undefined) ?? + ((data.output as Record)["exitCode"] as number | undefined) + : undefined); + if (target.exitCode === undefined && exitCandidate !== undefined) { + target.exitCode = exitCandidate; + } + } + + const resultParsed = CommandResultSchema.safeParse(value); + if (resultParsed.success) { + const data = resultParsed.data; + if (!target.output) { + target.output = + getString(data.output) ?? + getString(data.structuredContent?.output) ?? + getString(data.structuredContent?.text) ?? + getString(data.structured_content?.output) ?? + getString(data.structured_content?.text) ?? + (typeof data.result === "string" ? data.result : undefined); + } + if (target.exitCode === undefined) { + target.exitCode = data.exitCode ?? data.metadata?.exit_code; + } + if (!target.command && isRecord(data.result)) { + const nestedCommand = + coerceCommandValue(data.result["command"]) ?? + coerceCommandValue((data.result as Record)["args"]); + if (nestedCommand) { + target.command = nestedCommand; + } + } + } + + const nestedKeys = [ + "input", + "output", + "result", + "response", + "data", + "raw", + "payload", + ] as const; + for (const key of nestedKeys) { + if (value[key] !== undefined) { + collectCommandDetails(target, value[key], depth + 1); + } + } +} + +export function extractEditEntries(...sources: unknown[]): EditEntry[] { + const entries = sources.flatMap((value) => parseEditArguments(value)); + return mergeEditEntries(entries); +} + +export function extractReadEntries(...sources: unknown[]): ReadEntry[] { + return mergeReadEntries(sources.flatMap((value) => parseReadEntriesInternal(value))); +} + +export function extractCommandDetails(...sources: unknown[]): CommandDetails | null { + const details: CommandDetails = {}; + sources.forEach((value) => collectCommandDetails(details, value)); + if (details.command || details.output || details.cwd) { + return details; + } + return null; +} diff --git a/packages/server/src/server/agent/providers/claude-agent.test.ts b/packages/server/src/server/agent/providers/claude-agent.test.ts index e05d46677..c00081022 100644 --- a/packages/server/src/server/agent/providers/claude-agent.test.ts +++ b/packages/server/src/server/agent/providers/claude-agent.test.ts @@ -1,14 +1,20 @@ -import { describe, expect, test } from "vitest"; -import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { describe, expect, test, vi } from "vitest"; +import { existsSync, mkdtempSync, readFileSync, realpathSync, rmSync } from "node:fs"; import os from "node:os"; import path from "node:path"; -import { ClaudeAgentClient } from "./claude-agent.js"; +import { ClaudeAgentClient, convertClaudeHistoryEntry } from "./claude-agent.js"; +import { hydrateStreamState, type StreamItem, type ToolCallItem, type AgentToolCallData } from "../../../../../app/src/types/stream.js"; +import type { AgentStreamEventPayload } from "../../messages.js"; import type { AgentSessionConfig, AgentStreamEvent, AgentTimelineItem } from "../agent-sdk-types.js"; function tmpCwd(): string { const dir = mkdtempSync(path.join(os.tmpdir(), "claude-agent-e2e-")); - return dir; + try { + return realpathSync(dir); + } catch { + return dir; + } } async function autoApprove(session: Awaited>, event: AgentStreamEvent) { @@ -283,4 +289,284 @@ describe("ClaudeAgentClient (SDK integration)", () => { }, 180_000 ); + + test( + "hydrates persisted tool call results into the UI stream", + async () => { + const cwd = tmpCwd(); + const client = new ClaudeAgentClient(); + const config: AgentSessionConfig = { + provider: "claude", + cwd, + extra: { claude: { maxThinkingTokens: 4096 } }, + }; + const session = await client.createSession(config); + const prompt = [ + "You are verifying the hydrate regression test.", + "Follow these steps exactly and report 'hydration test complete' at the end:", + "1. Run the Bash command 'pwd' via the terminal tool.", + "2. Use your editor tools (not the shell) to create a file named hydrate-proof.txt with the content:", + " HYDRATION_PROOF_LINE_ONE", + " HYDRATION_PROOF_LINE_TWO", + "3. Read hydrate-proof.txt via the editor read_file tool to confirm the contents.", + "4. Summarize the diff/write results briefly and then stop.", + ].join("\n"); + + try { + const liveTimelineUpdates: StreamHydrationUpdate[] = []; + const events = session.stream(prompt); + + let completed = false; + try { + for await (const event of events) { + await autoApprove(session, event); + recordTimelineUpdate(liveTimelineUpdates, event); + if (event.type === "turn_completed") { + completed = true; + break; + } + if (event.type === "turn_failed") { + throw new Error(event.error); + } + } + } finally { + await session.close(); + } + + expect(completed).toBe(true); + const liveState = hydrateStreamState(liveTimelineUpdates); + const liveSnapshots = extractAgentToolSnapshots(liveState); + const commandTool = liveSnapshots.find((snapshot) => + (snapshot.data.displayName ?? "").toLowerCase().includes("pwd") + ); + const editTool = liveSnapshots.find((snapshot) => + rawContainsText(snapshot.data.raw, "hydrate-proof.txt") + ); + const readTool = liveSnapshots.find((snapshot) => + rawContainsText(snapshot.data.raw, "HYDRATION_PROOF_LINE_TWO") + ); + + expect(commandTool).toBeTruthy(); + expect(editTool).toBeTruthy(); + expect(readTool).toBeTruthy(); + + const handle = session.describePersistence(); + expect(handle).toBeTruthy(); + const sessionId = handle?.sessionId ?? handle?.nativeHandle; + expect(typeof sessionId).toBe("string"); + + const historyPaths = getClaudeHistoryPaths(cwd, sessionId!); + expect(await waitForHistoryFile(historyPaths)).toBe(true); + + const resumed = await client.resumeSession(handle!, { cwd }); + const hydrationUpdates: StreamHydrationUpdate[] = []; + try { + for await (const event of resumed.streamHistory()) { + recordTimelineUpdate(hydrationUpdates, event); + } + } finally { + await resumed.close(); + } + + expect(hydrationUpdates.length).toBeGreaterThan(0); + + const hydratedState = hydrateStreamState(hydrationUpdates); + const hydratedSnapshots = extractAgentToolSnapshots(hydratedState); + const hydratedMap = new Map( + hydratedSnapshots.map((entry) => [entry.key, entry.data]) + ); + + assertHydratedReplica(commandTool!, hydratedMap, (data) => rawContainsText(data.raw, cwd)); + assertHydratedReplica(editTool!, hydratedMap, (data) => + rawContainsText(data.raw, "hydrate-proof.txt") + ); + assertHydratedReplica(readTool!, hydratedMap, (data) => + rawContainsText(data.raw, "HYDRATION_PROOF_LINE_TWO") + ); + } finally { + cleanupClaudeHistory(cwd); + rmSync(cwd, { recursive: true, force: true }); + } + }, + 240_000 + ); }); + +describe("convertClaudeHistoryEntry", () => { + test("maps user tool results to timeline items", () => { + const toolUseId = "toolu_test"; + const entry = { + type: "user", + message: { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: toolUseId, + content: [{ type: "text", text: "file contents" }], + }, + ], + }, + }; + + const stubTimeline: AgentTimelineItem[] = [ + { + type: "tool_call", + server: "editor", + tool: "read_file", + status: "completed", + } as AgentTimelineItem, + ]; + + const mapBlocks = vi.fn().mockReturnValue(stubTimeline); + const result = convertClaudeHistoryEntry(entry, mapBlocks); + + expect(result).toEqual(stubTimeline); + expect(mapBlocks).toHaveBeenCalledTimes(1); + const arg = mapBlocks.mock.calls[0][0]; + expect(Array.isArray(arg)).toBe(true); + }); + + test("returns user messages when no tool blocks exist", () => { + const entry = { + type: "user", + message: { + role: "user", + content: "Run npm test", + }, + }; + + const result = convertClaudeHistoryEntry(entry, () => []); + + expect(result).toEqual([ + { + type: "user_message", + text: "Run npm test", + raw: entry.message, + }, + ]); + }); +}); + +type StreamHydrationUpdate = { + event: Extract; + timestamp: Date; +}; + +type ToolSnapshot = { key: string; data: AgentToolCallData }; + +function recordTimelineUpdate(target: StreamHydrationUpdate[], event: AgentStreamEvent) { + if (event.type !== "timeline") { + return; + } + const payload = event as Extract; + target.push({ + event: payload as StreamHydrationUpdate["event"], + timestamp: new Date(), + }); +} + +function extractAgentToolSnapshots(state: StreamItem[]): ToolSnapshot[] { + return state + .filter( + (item): item is ToolCallItem => + item.kind === "tool_call" && item.payload.source === "agent" + ) + .map((item) => ({ + key: buildToolSnapshotKey(item.payload.data, item.id), + data: item.payload.data, + })); +} + +function buildToolSnapshotKey(data: AgentToolCallData, fallbackId: string): string { + const normalized = typeof data.callId === "string" && data.callId.trim().length > 0 ? data.callId.trim() : null; + if (normalized) { + return normalized; + } + const display = typeof data.displayName === "string" && data.displayName.trim().length > 0 ? data.displayName.trim() : fallbackId; + return `${data.server}:${data.tool}:${display}`; +} + +function assertHydratedReplica( + liveSnapshot: ToolSnapshot, + hydratedMap: Map, + predicate: (data: AgentToolCallData) => boolean +) { + expect(predicate(liveSnapshot.data)).toBe(true); + const hydrated = hydratedMap.get(liveSnapshot.key); + expect(hydrated).toBeTruthy(); + expect(hydrated?.status).toBe(liveSnapshot.data.status); + expect(hydrated?.server).toBe(liveSnapshot.data.server); + expect(hydrated?.tool).toBe(liveSnapshot.data.tool); + expect(hydrated?.displayName).toBe(liveSnapshot.data.displayName); + expect(predicate(hydrated!)).toBe(true); +} + +function sanitizeClaudeProjectName(cwd: string): string { + return cwd.replace(/[\\/]/g, "-").replace(/_/g, "-"); +} + +function resolveClaudeHistoryPath(cwd: string, sessionId: string): string { + const sanitized = sanitizeClaudeProjectName(cwd); + return path.join(os.homedir(), ".claude", "projects", sanitized, `${sessionId}.jsonl`); +} + +function getClaudeHistoryPaths(cwd: string, sessionId: string): string[] { + return normalizeCwdCandidates(cwd).map((candidate) => + resolveClaudeHistoryPath(candidate, sessionId) + ); +} + +function normalizeCwdCandidates(cwd: string): string[] { + const candidates = new Set([cwd]); + try { + const resolved = realpathSync(cwd); + candidates.add(resolved); + } catch { + // ignore resolution errors + } + return Array.from(candidates); +} + +function cleanupClaudeHistory(cwd: string) { + for (const candidate of normalizeCwdCandidates(cwd)) { + const sanitized = sanitizeClaudeProjectName(candidate); + const projectDir = path.join(os.homedir(), ".claude", "projects", sanitized); + if (existsSync(projectDir)) { + rmSync(projectDir, { recursive: true, force: true }); + } + } +} + +async function waitForHistoryFile(historyPaths: string | string[], timeoutMs = 10_000): Promise { + const candidates = Array.isArray(historyPaths) ? Array.from(new Set(historyPaths)) : [historyPaths]; + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (candidates.some((entry) => existsSync(entry))) { + return true; + } + await new Promise((resolve) => setTimeout(resolve, 250)); + } + return candidates.some((entry) => existsSync(entry)); +} + +function rawContainsText(raw: unknown, text: string, depth = 0): boolean { + if (!raw || typeof text !== "string" || !text) { + return false; + } + if (typeof raw === "string") { + return raw.includes(text); + } + if (depth > 6) { + return false; + } + if (Array.isArray(raw)) { + return raw.some((entry) => rawContainsText(entry, text, depth + 1)); + } + if (typeof raw === "object") { + return Object.values(raw as Record).some((value) => + rawContainsText(value, text, depth + 1) + ); + } + return false; +} diff --git a/packages/server/src/server/agent/providers/claude-agent.ts b/packages/server/src/server/agent/providers/claude-agent.ts index 29388de12..a27f8da3a 100644 --- a/packages/server/src/server/agent/providers/claude-agent.ts +++ b/packages/server/src/server/agent/providers/claude-agent.ts @@ -811,41 +811,7 @@ class ClaudeAgentSession implements AgentSession { } private convertHistoryEntry(entry: any): AgentTimelineItem[] { - const message = entry?.message; - if (!message || !("content" in message)) { - return []; - } - const content = message.content as string | ClaudeContentChunk[]; - - if (entry.type === "assistant" && content) { - return this.mapBlocksToTimeline(content); - } - - if (entry.type === "user") { - const text = extractUserMessageText(content); - if (text) { - return [ - { - type: "user_message", - text, - raw: message, - }, - ]; - } - return []; - } - - if (Array.isArray(content)) { - const hasToolBlock = content.some( - (block: ClaudeContentChunk) => - typeof block?.type === "string" && block.type.includes("tool") - ); - if (hasToolBlock) { - return this.mapBlocksToTimeline(content); - } - } - - return []; + return convertClaudeHistoryEntry(entry, (content) => this.mapBlocksToTimeline(content)); } private mapBlocksToTimeline(content: string | ClaudeContentChunk[]): AgentTimelineItem[] { @@ -1120,6 +1086,63 @@ class ClaudeAgentSession implements AgentSession { } } +function hasToolLikeBlock(block?: ClaudeContentChunk | null): boolean { + if (!block || typeof block !== "object") { + return false; + } + const type = typeof block.type === "string" ? block.type.toLowerCase() : ""; + return type.includes("tool"); +} + +function normalizeHistoryBlocks( + content: string | ClaudeContentChunk[] +): ClaudeContentChunk[] | null { + if (Array.isArray(content)) { + return content; + } + if (content && typeof content === "object") { + return [content as ClaudeContentChunk]; + } + return null; +} + +export function convertClaudeHistoryEntry( + entry: any, + mapBlocks: (content: string | ClaudeContentChunk[]) => AgentTimelineItem[] +): AgentTimelineItem[] { + const message = entry?.message; + if (!message || !("content" in message)) { + return []; + } + + const content = message.content as string | ClaudeContentChunk[]; + const normalizedBlocks = normalizeHistoryBlocks(content); + const hasToolBlock = normalizedBlocks?.some((block) => hasToolLikeBlock(block)) ?? false; + + if (hasToolBlock && normalizedBlocks) { + return mapBlocks(Array.isArray(content) ? content : normalizedBlocks); + } + + if (entry.type === "assistant" && content) { + return mapBlocks(content); + } + + if (entry.type === "user") { + const text = extractUserMessageText(content); + if (text) { + return [ + { + type: "user_message", + text, + raw: message, + }, + ]; + } + } + + return []; +} + class Pushable implements AsyncIterable { private queue: T[] = []; private resolvers: ((value: IteratorResult) => void)[] = []; diff --git a/packages/server/vitest.config.ts b/packages/server/vitest.config.ts index 750941aeb..23fe2693b 100644 --- a/packages/server/vitest.config.ts +++ b/packages/server/vitest.config.ts @@ -1,6 +1,12 @@ +import path from "node:path"; import { defineConfig } from "vitest/config"; export default defineConfig({ + resolve: { + alias: { + "@server": path.resolve(__dirname, "./src"), + }, + }, test: { testTimeout: 30000, hookTimeout: 30000, diff --git a/plan.md b/plan.md index 05e16baf6..0269e4dcc 100644 --- a/plan.md +++ b/plan.md @@ -7,7 +7,10 @@ # Tasks -- [ ] Hydrated Claude tool calls still show the spinner forever after refreshing a chat; Claude CLI already persists the tool payloads under `~/.claude/projects/`, but our hydrate path isn’t loading them. Fix the loader so it reads the saved diffs/read/output blocks and transitions the existing pill to “completed” instead of spinning or duplicating. +- [x] Claude hydration regression must be proven with a real end-to-end test: spin up an actual Claude agent (no mocks), ask it to run tool calls that create/edit/read a temp project, verify the live stream shows the tool results, shut the agent down, hydrate from disk (`~/.claude/projects/...`), and assert the exact diff/read/command output replays in the UI. Do not mark any hydration task complete until this automated test exists and fails on current main but passes after the fix. + - Added a Vitest integration in `packages/server/src/server/agent/providers/claude-agent.test.ts` that drives a live Claude session through real Bash/write/read tool calls, converts the emitted timeline into the UI stream via `hydrateStreamState`, tears the agent down, resumes it from `~/.claude/projects/...`, and asserts that the hydrated stream reproduces the command output, file diff, and read content instead of leaving spinners. The helper waits for the persisted `.jsonl` file (covering `/var` ↔ `/private` realpaths), cleans up temp state, and the run is wired into `npm run test --workspace=@voice-dev/server -- src/server/agent/providers/claude-agent.test.ts` (passes locally). +- [x] Hydrated Claude tool calls still show the spinner forever after refreshing a chat; Claude CLI already persists the tool payloads under `~/.claude/projects/`, but our hydrate path isn’t loading them. Fix the loader so it reads the saved diffs/read/output blocks and transitions the existing pill to “completed” instead of spinning or duplicating. + - Claude history entries now flow through a shared `convertClaudeHistoryEntry` helper that inspects every message for `tool_*` blocks before classifying it as a plain user message, so persisted `tool_result` lines recorded as `type: "user"` convert into completed tool-call events instead of leaving the original spinner stuck in `executing`. Added unit coverage in `packages/server/src/server/agent/providers/claude-agent.test.ts` proving user tool results hydrate correctly and ran `npm run test --workspace=@voice-dev/server -- src/server/agent/providers/claude-agent.test.ts` (passes, though existing SDK integration specs are still slow/flaky by nature). - [ ] We still see duplicate tool call pills (loading + completed/failed) in both Codex and Claude sessions, live and hydrated. Track tool call IDs rigorously, dedupe pending/completed entries, and add regression tests covering all providers and hydrate flows. - [ ] Permission request cards in the stream header are emitting duplicate key warnings—derive stable per-agent keys (for example `${agentId}:${request.id}` with fallbacks) so React stops complaining. - [ ] The so-called tests are still fake: convert the stream harness into real Vitest suites co-located with the code, make `npm test` (Vitest) the single entrypoint, and ensure those tests fail today because hydrated tool-call results are missing. @@ -16,3 +19,19 @@ - [x] AgentInput image picker crashes with "Attempting to launch an unregistered ActivityResultLauncher" when calling Expo ImagePicker; register the launcher properly (or switch to the new async hook) so picking images doesn’t throw and we can attach screenshots again. - Added a dedicated `useImageAttachmentPicker` hook that registers the media picker launcher up front, reuses Expo’s permission hook, restores pending results, and guards against concurrent launches. AgentInput now consumes this hook so tapping the attachment button opens the picker without crashing on Android; `npm run typecheck --workspace=@voice-dev/app` currently fails upstream in `stream.test.ts` before our change, noted in the summary. - [ ] Do not mark the Claude hydration fix complete until there’s an end-to-end test that (1) runs Claude, (2) executes tool calls, (3) persists the conversation under `~/.claude/projects/...`, and (4) hydrates from disk verifying the tool call results render. No test, no checkbox. +- [ ] ERROR Encountered two children with the same key, `%s`. Keys should be unique so that components maintain their identity across updates. Non-unique keys may cause children to be duplicated and/or omitted — the behavior is unsupported and could change in a future version. .$tool_1763217454926_uyr9rm + + ```tsx + Code: agent-stream-view.tsx + 317 | return ( + 318 | + > 319 |