diff --git a/packages/ui/src/components/chat/message/parts/ToolPart.tsx b/packages/ui/src/components/chat/message/parts/ToolPart.tsx index 5af7e702..0753d58b 100644 --- a/packages/ui/src/components/chat/message/parts/ToolPart.tsx +++ b/packages/ui/src/components/chat/message/parts/ToolPart.tsx @@ -32,6 +32,7 @@ import { formatInputForDisplay, renderTodoOutput, tryParseJsonOutput, + coerceToText, } from '../toolRenderers'; import { JsonTreeViewer } from '@/components/ui/JsonTreeViewer'; import { JsonSummaryView } from './JsonSummaryView'; @@ -1868,7 +1869,7 @@ const ToolExpandedContent: React.FC = React.memo(({ color: 'var(--status-error)', borderColor: 'var(--status-error-border)', }}> - {state.error} + {coerceToText(state.error)} ); @@ -1884,14 +1885,14 @@ const ToolExpandedContent: React.FC = React.memo(({ {questionInput.questions.map((q, index) => (
{q.header ? ( -
{q.header}
+
{coerceToText(q.header)}
) : null} -
{q.question}
+
{coerceToText(q.question)}
{Array.isArray(q.options) && q.options.length > 0 ? (
{q.options.map((opt) => ( - - {opt.label} + + {coerceToText(opt.label)} ))}
@@ -1909,7 +1910,7 @@ const ToolExpandedContent: React.FC = React.memo(({ if (part.tool === 'task' && hasStringOutput) { return renderScrollableBlock(
- +
); } @@ -1978,7 +1979,7 @@ const ToolExpandedContent: React.FC = React.memo(({ if (hasStringOutput && outputString.trim()) { return renderScrollableBlock( = React.memo(({ color: 'var(--status-error)', borderColor: 'var(--status-error-border)', }}> - {state.error} + {coerceToText(state.error)}
)} diff --git a/packages/ui/src/components/chat/message/parts/__tests__/issue-2011-react-error-31.test.ts b/packages/ui/src/components/chat/message/parts/__tests__/issue-2011-react-error-31.test.ts new file mode 100644 index 00000000..101e3cc2 --- /dev/null +++ b/packages/ui/src/components/chat/message/parts/__tests__/issue-2011-react-error-31.test.ts @@ -0,0 +1,100 @@ +import { describe, test, expect } from 'bun:test'; +import { coerceToText, renderTodoOutput } from '../../toolRenderers'; + +describe('coerceToText (issue #2011)', () => { + test('returns strings unchanged', () => { + expect(coerceToText('hello')).toBe('hello'); + }); + + test('coerces plain objects to JSON strings', () => { + // The exact shape that produced React error #31: object with {TODO} key + const result = coerceToText({ TODO: 'Review the diff' }); + expect(typeof result).toBe('string'); + expect(result).toContain('TODO'); + expect(result).toContain('Review the diff'); + }); + + test('coerces nested objects to JSON strings', () => { + const result = coerceToText({ todos: [{ TODO: 'a' }, { content: 'b' }] }); + expect(typeof result).toBe('string'); + const parsed = JSON.parse(result); + expect(parsed).toBeTruthy(); + }); + + test('coerces numbers and booleans', () => { + expect(coerceToText(42)).toBe('42'); + expect(coerceToText(true)).toBe('true'); + expect(coerceToText(false)).toBe('false'); + }); + + test('returns fallback for null/undefined', () => { + expect(coerceToText(null)).toBe(''); + expect(coerceToText(undefined)).toBe(''); + expect(coerceToText(null, 'oops')).toBe('oops'); + }); + + test('handles circular structures without throwing', () => { + const obj: Record = {}; + obj.self = obj; + // Must not throw, must not recurse forever + const result = coerceToText(obj); + expect(typeof result).toBe('string'); + }); +}); + +describe('renderTodoOutput (issue #2011)', () => { + const labels = { + total: 'Total', + inProgress: 'In progress', + pending: 'Pending', + completed: 'Completed', + cancelled: 'Cancelled', + }; + + test('returns null for invalid JSON', () => { + expect(renderTodoOutput('not json', labels)).toBeNull(); + }); + + test('returns null when parsed value is not an array', () => { + expect(renderTodoOutput(JSON.stringify({ foo: 'bar' }), labels)).toBeNull(); + }); + + test('renders valid todo arrays', () => { + const output = JSON.stringify([ + { id: '1', content: 'Do the thing', status: 'pending', priority: 'high' }, + ]); + const result = renderTodoOutput(output, labels); + expect(result).not.toBeNull(); + }); + + test('filters out todos with non-string content (the {TODO} object case)', () => { + // The exact pathological shape from the issue: a todo where content + // is an object instead of a string. Previously this triggered + // React error #31 when rendered as {todo.content}. + const output = JSON.stringify([ + { id: '1', content: { TODO: 'Review the diff' }, status: 'pending' }, + { id: '2', content: 'Real string content', status: 'completed' }, + ]); + // Must not throw. Either returns valid React element (with bad row + // filtered out) or null. + const result = renderTodoOutput(output, labels); + expect(result).not.toBeNull(); + }); + + test('returns null when all todos have non-string content', () => { + const output = JSON.stringify([ + { id: '1', content: { TODO: 'x' }, status: 'pending' }, + { id: '2', content: { foo: 'bar' }, status: 'completed' }, + ]); + expect(renderTodoOutput(output, labels)).toBeNull(); + }); + + test('filters out todos with non-string status', () => { + const output = JSON.stringify([ + { id: '1', content: 'Valid', status: { broken: true } }, + { id: '2', content: 'Valid', status: 'pending' }, + ]); + const result = renderTodoOutput(output, labels); + expect(result).not.toBeNull(); + }); +}); diff --git a/packages/ui/src/components/chat/message/toolRenderers.tsx b/packages/ui/src/components/chat/message/toolRenderers.tsx index a781f35b..3c4dd773 100644 --- a/packages/ui/src/components/chat/message/toolRenderers.tsx +++ b/packages/ui/src/components/chat/message/toolRenderers.tsx @@ -11,6 +11,17 @@ const cleanOutput = (output: string) => { return cleaned.trim(); }; +export const coerceToText = (value: unknown, fallback = ''): string => { + if (typeof value === 'string') return value; + if (value === null || value === undefined) return fallback; + if (typeof value === 'number' || typeof value === 'boolean' || typeof value === 'bigint') return String(value); + try { + return JSON.stringify(value); + } catch { + return fallback; + } +}; + const hasLspDiagnostics = (output: string): boolean => { if (!output) return false; return output.includes(' { try { - const todos = JSON.parse(output) as Todo[]; - if (!Array.isArray(todos)) { + const raw: unknown = JSON.parse(output); + if (!Array.isArray(raw)) { + return null; + } + const todos: Todo[] = raw.filter( + (t): t is Todo => + !!t && + typeof t === 'object' && + typeof (t as { content?: unknown }).content === 'string' && + typeof (t as { status?: unknown }).status === 'string', + ); + if (todos.length === 0) { return null; } @@ -446,7 +467,7 @@ export const renderTodoOutput = ( {todosByStatus.in_progress.map((todo, idx) => (
{getPriorityDot(todo.priority)} - {todo.content} + {coerceToText(todo.content)}
))} @@ -463,7 +484,7 @@ export const renderTodoOutput = ( {todosByStatus.pending.map((todo, idx) => (
{getPriorityDot(todo.priority)} - {todo.content} + {coerceToText(todo.content)}
))} @@ -480,7 +501,7 @@ export const renderTodoOutput = ( {todosByStatus.completed.map((todo, idx) => (
- {todo.content} + {coerceToText(todo.content)}
))} @@ -497,7 +518,7 @@ export const renderTodoOutput = ( {todosByStatus.cancelled.map((todo, idx) => (
× - {todo.content} + {coerceToText(todo.content)}
))}