Merge pull request #2375 from JSap0914/fix/2265-tool-output-size-cap
fix(ui): cap oversized tool output before rendering to prevent renderer OOM (#2265)
This commit is contained in:
@@ -32,6 +32,7 @@ import {
|
||||
renderTodoOutput,
|
||||
tryParseJsonOutput,
|
||||
coerceToText,
|
||||
capToolOutputText,
|
||||
} from '../toolRenderers';
|
||||
import { JsonTreeViewer } from '@/components/ui/JsonTreeViewer';
|
||||
import { JsonSummaryView } from './JsonSummaryView';
|
||||
@@ -606,11 +607,15 @@ const getToolOutputText = (
|
||||
part: ToolPartType,
|
||||
metadata: Record<string, unknown> | undefined,
|
||||
): string => {
|
||||
// Cap oversized payloads before JSON.parse / syntax highlighting / DOM work
|
||||
// so a single huge tool output can't trigger a V8 Zone-allocation OOM that
|
||||
// hard-crashes the renderer (issue #2265).
|
||||
const capped = capToolOutputText(output);
|
||||
if (part.tool === 'bash') {
|
||||
return output;
|
||||
return capped;
|
||||
}
|
||||
|
||||
return formatEditOutput(output, part.tool, metadata);
|
||||
return formatEditOutput(capped, part.tool, metadata);
|
||||
};
|
||||
|
||||
const StreamingPlainTextOutput: React.FC<{ output: string }> = ({ output }) => {
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
|
||||
import { capToolOutputText, TOOL_OUTPUT_MAX_CHARS } from './toolRenderers';
|
||||
|
||||
// Regression coverage for issue #2265: the desktop renderer hard-crashes with a
|
||||
// V8 "Zone Allocation failed" OOM when a tool returns oversized external content
|
||||
// (e.g. a fetched Google Slides page with full-resolution base64 images inlined),
|
||||
// because the whole payload previously flowed through JSON.parse / syntax
|
||||
// highlighting / DOM rendering as a single unbounded JS string. capToolOutputText
|
||||
// is the bounded size guard that runs before any of that work.
|
||||
describe('capToolOutputText (issue #2265 renderer OOM guard)', () => {
|
||||
test('exposes a sane positive default cap', () => {
|
||||
expect(typeof TOOL_OUTPUT_MAX_CHARS).toBe('number');
|
||||
expect(TOOL_OUTPUT_MAX_CHARS).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
test('returns short output unchanged', () => {
|
||||
const output = 'hello world';
|
||||
expect(capToolOutputText(output)).toBe(output);
|
||||
});
|
||||
|
||||
test('returns output at exactly the cap unchanged', () => {
|
||||
const output = 'a'.repeat(TOOL_OUTPUT_MAX_CHARS);
|
||||
expect(capToolOutputText(output)).toBe(output);
|
||||
expect(capToolOutputText(output).length).toBe(TOOL_OUTPUT_MAX_CHARS);
|
||||
});
|
||||
|
||||
test('caps oversized output and never emits the full string', () => {
|
||||
const oversized = 'x'.repeat(TOOL_OUTPUT_MAX_CHARS + 10_000);
|
||||
const capped = capToolOutputText(oversized);
|
||||
|
||||
// The pathological full-size string must not survive to the renderer.
|
||||
expect(capped.length).toBeLessThan(oversized.length);
|
||||
// Head of the payload is preserved for the user.
|
||||
expect(capped.startsWith('x'.repeat(1000))).toBe(true);
|
||||
// A truncation notice is appended so the truncation is visible.
|
||||
expect(capped).toContain('output truncated');
|
||||
expect(capped).toContain('10000 more characters');
|
||||
});
|
||||
|
||||
test('honors a custom cap', () => {
|
||||
const output = 'abcdefghij'; // 10 chars
|
||||
const capped = capToolOutputText(output, 4);
|
||||
expect(capped.startsWith('abcd')).toBe(true);
|
||||
expect(capped).toContain('output truncated');
|
||||
// Only the first 4 chars of the original body are retained.
|
||||
expect(capped).not.toContain('efghij');
|
||||
});
|
||||
|
||||
test('simulated large webfetch payload is bounded well below original size', () => {
|
||||
// ~6MB single string, matching the 5MB-20MB Zone-allocation trigger range
|
||||
// described in the issue (a Slides page with embedded base64 images).
|
||||
const base64Blob = 'QUJD'.repeat(1_500_000); // 6,000,000 chars
|
||||
const capped = capToolOutputText(base64Blob);
|
||||
|
||||
expect(base64Blob.length).toBeGreaterThan(5_000_000);
|
||||
expect(capped.length).toBeLessThan(TOOL_OUTPUT_MAX_CHARS + 256);
|
||||
expect(capped).toContain('renderer from running out of memory');
|
||||
});
|
||||
|
||||
test('non-string input is returned unchanged (defensive)', () => {
|
||||
// @ts-expect-error verifying runtime robustness against non-string inputs
|
||||
expect(capToolOutputText(undefined)).toBeUndefined();
|
||||
// @ts-expect-error verifying runtime robustness against non-string inputs
|
||||
expect(capToolOutputText(null)).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -22,6 +22,28 @@ export const coerceToText = (value: unknown, fallback = ''): string => {
|
||||
}
|
||||
};
|
||||
|
||||
// Guards the renderer process against V8 "Zone Allocation failed" OOM crashes
|
||||
// (issue #2265). When a tool returns oversized external content — e.g. a fetched
|
||||
// web page with full-resolution base64 images inlined — the entire payload flows
|
||||
// through this module as a single JS string that is JSON.parsed, syntax
|
||||
// highlighted, and attached to the DOM. A large enough single string exceeds
|
||||
// V8's Zone allocator and hard-crashes the renderer before any virtualization or
|
||||
// CSS clip can help. Capping the string length before that work happens keeps a
|
||||
// useful head of the output while preventing the pathological allocation.
|
||||
export const TOOL_OUTPUT_MAX_CHARS = 512 * 1024;
|
||||
|
||||
export const capToolOutputText = (
|
||||
output: string,
|
||||
maxChars: number = TOOL_OUTPUT_MAX_CHARS,
|
||||
): string => {
|
||||
if (typeof output !== 'string' || output.length <= maxChars) {
|
||||
return output;
|
||||
}
|
||||
const omitted = output.length - maxChars;
|
||||
const notice = `\n\n… [output truncated: ${omitted} more characters not shown to prevent the renderer from running out of memory]`;
|
||||
return output.slice(0, maxChars) + notice;
|
||||
};
|
||||
|
||||
const hasLspDiagnostics = (output: string): boolean => {
|
||||
if (!output) return false;
|
||||
return output.includes('<diagnostics')
|
||||
|
||||
Reference in New Issue
Block a user