fix(chat): harden tool output rendering against non-string fields (#2011) (#2071)

React error #31 (Objects are not valid as a React child) was thrown intermittently when a task/subagent tool returned structured data (e.g. { TODO: '...' }) in a field that the OpenCode SDK types as a plain string. Pathological payloads would propagate into JSX children without runtime validation, white-screening the chat until refresh. This change adds a single `coerceToText` helper in toolRenderers.tsx and applies it at every vulnerable JSX expression: - ToolPart.tsx:1807,1975 {state.error} (typed string, can be object) - ToolPart.tsx:1825 {q.question} (QuestionCard input cast) - ToolPart.tsx:1830 {opt.label} (QuestionCard input cast) - ToolPart.tsx:1848 task tool markdown output - ToolPart.tsx:1898 ToolScrollableTextOutput entry - toolRenderers.tsx {todo.content} x4 in renderTodoOutput renderTodoOutput now also validates the parsed array at the boundary (JSON.parse result is filtered to objects whose content and status are runtime strings), so a single bad row no longer poisons the whole tool output. Tests: 12 new unit tests in packages/ui/src/components/chat/message/parts/__tests__/issue-2011-react-error-31.test.ts covering the {TODO}-key object path, circular references, and non-string content/status on parsed todos. Co-authored-by: bashrusakh <bashrusakh@users.noreply.github.com>
2026-07-11 14:42:40 +03:00
parent d8a904954b
commit f6326e1c35
3 changed files with 136 additions and 14 deletions
@@ -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<ToolExpandedContentProps> = React.memo(({
                            color: 'var(--status-error)',
                            borderColor: 'var(--status-error-border)',
                        }}>
-                            {state.error}
+                            {coerceToText(state.error)}
                        </div>
                    </div>
                );
@@ -1884,14 +1885,14 @@ const ToolExpandedContent: React.FC<ToolExpandedContentProps> = React.memo(({
                        {questionInput.questions.map((q, index) => (
                            <div key={index} className="space-y-0.5">
                                {q.header ? (
-                                    <div className="typography-micro text-muted-foreground">{q.header}</div>
+                                    <div className="typography-micro text-muted-foreground">{coerceToText(q.header)}</div>
                                ) : null}
-                                <div className="typography-meta text-foreground">{q.question}</div>
+                                <div className="typography-meta text-foreground">{coerceToText(q.question)}</div>
                                {Array.isArray(q.options) && q.options.length > 0 ? (
                                    <div className="flex flex-wrap gap-1 mt-0.5">
                                        {q.options.map((opt) => (
-                                            <span key={opt.label} className="typography-micro px-1.5 py-0.5 rounded bg-muted/30 border border-border/30 text-muted-foreground">
-                                                {opt.label}
+                                            <span key={coerceToText(opt.label)} className="typography-micro px-1.5 py-0.5 rounded bg-muted/30 border border-border/30 text-muted-foreground">
+                                                {coerceToText(opt.label)}
                                            </span>
                                        ))}
                                    </div>
@@ -1909,7 +1910,7 @@ const ToolExpandedContent: React.FC<ToolExpandedContentProps> = React.memo(({
        if (part.tool === 'task' && hasStringOutput) {
            return renderScrollableBlock(
                <div className="w-full min-w-0">
-                    <SimpleMarkdownRenderer content={outputString} variant="tool" onShowPopup={onShowPopup} />
+                    <SimpleMarkdownRenderer content={coerceToText(outputString)} variant="tool" onShowPopup={onShowPopup} />
                </div>
            );
        }
@@ -1978,7 +1979,7 @@ const ToolExpandedContent: React.FC<ToolExpandedContentProps> = React.memo(({
        if (hasStringOutput && outputString.trim()) {
            return renderScrollableBlock(
                <ToolScrollableTextOutput
-                    output={outputString}
+                    output={coerceToText(outputString)}
                    part={part}
                    metadata={metadata}
                    input={input}
@@ -2097,7 +2098,7 @@ const ToolExpandedContent: React.FC<ToolExpandedContentProps> = React.memo(({
                                color: 'var(--status-error)',
                                borderColor: 'var(--status-error-border)',
                            }}>
-                                {state.error}
+                                {coerceToText(state.error)}
                            </div>
                        </div>
                    )}
@@ -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<string, unknown> = {};
+        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();
+    });
+});
@@ -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('<diagnostics')
@@ -387,8 +398,18 @@ export const renderTodoOutput = (
    options?: { unstyled?: boolean },
 ) => {
    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) => (
                                <div key={todo.id || idx} className="flex items-start gap-2">
                                    {getPriorityDot(todo.priority)}
-                                    <span className="typography-code text-foreground flex-1 leading-relaxed">{todo.content}</span>
+                                    <span className="typography-code text-foreground flex-1 leading-relaxed">{coerceToText(todo.content)}</span>
                                </div>
                            ))}
                        </div>
@@ -463,7 +484,7 @@ export const renderTodoOutput = (
                            {todosByStatus.pending.map((todo, idx) => (
                                <div key={todo.id || idx} className="flex items-start gap-2">
                                    {getPriorityDot(todo.priority)}
-                                    <span className="typography-code text-foreground flex-1 leading-relaxed">{todo.content}</span>
+                                    <span className="typography-code text-foreground flex-1 leading-relaxed">{coerceToText(todo.content)}</span>
                                </div>
                            ))}
                        </div>
@@ -480,7 +501,7 @@ export const renderTodoOutput = (
                            {todosByStatus.completed.map((todo, idx) => (
                                <div key={todo.id || idx} className="flex items-start gap-2">
                                    <Icon name="check" className="w-3 h-3 mt-0.5 flex-shrink-0"  style={{ color: 'var(--status-success)', opacity: 0.7 }}/>
-                                    <span className="typography-code text-foreground flex-1 leading-relaxed">{todo.content}</span>
+                                    <span className="typography-code text-foreground flex-1 leading-relaxed">{coerceToText(todo.content)}</span>
                                </div>
                            ))}
                        </div>
@@ -497,7 +518,7 @@ export const renderTodoOutput = (
                            {todosByStatus.cancelled.map((todo, idx) => (
                                <div key={todo.id || idx} className="flex items-start gap-2">
                                    <span className="w-3 h-3 text-muted-foreground/50 mt-0.5 flex-shrink-0">×</span>
-                                    <span className="typography-code text-muted-foreground/50 line-through flex-1 leading-relaxed">{todo.content}</span>
+                                    <span className="typography-code text-muted-foreground/50 line-through flex-1 leading-relaxed">{coerceToText(todo.content)}</span>
                                </div>
                            ))}
                        </div>