From 3a25d71004a853e82f4e5821a61675ac0aeb6786 Mon Sep 17 00:00:00 2001 From: gnanam1990 Date: Mon, 6 Apr 2026 18:38:33 +0530 Subject: [PATCH] fix: comprehensive tool argument normalization hardening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove all { raw: ... } returns that caused InputValidationError with z.strictObject schemas — return {} instead for clean Zod errors - Extend normalizeAtStop buffering to all mapped tools (Read, Write, Edit, Glob, Grep) so streaming paths also get normalized - Make repairPossiblyTruncatedObjectJson generic — repair any valid JSON object, not just ones with a command field - Export hasToolFieldMapping for streaming normalizeAtStop decision - Skip normalization on finish_reason: length to preserve raw truncated buffer - Update all test expectations to match new behavior Co-Authored-By: Claude Opus 4.6 (1M context) --- src/services/api/openaiShim.test.ts | 10 +++---- src/services/api/openaiShim.ts | 12 +++----- .../api/toolArgumentNormalization.test.ts | 30 ++++++++----------- src/services/api/toolArgumentNormalization.ts | 27 ++++++++--------- 4 files changed, 35 insertions(+), 44 deletions(-) diff --git a/src/services/api/openaiShim.test.ts b/src/services/api/openaiShim.test.ts index b47d84f6..029bf300 100644 --- a/src/services/api/openaiShim.test.ts +++ b/src/services/api/openaiShim.test.ts @@ -744,7 +744,7 @@ test('keeps terminal empty Bash tool arguments invalid in non-streaming response type: 'tool_use', id: 'function-call-1', name: 'Bash', - input: { raw: '' }, + input: {}, }, ]) }) @@ -1094,7 +1094,7 @@ test('keeps terminal whitespace-only Bash arguments invalid in streaming respons .map(event => (event.delta as Record).partial_json) .join('') - expect(normalizedInput).toBe('{"raw":" "}') + expect(normalizedInput).toBe('{}') }) test('normalizes streaming Bash arguments that begin with bracket syntax', async () => { @@ -1423,7 +1423,7 @@ test('does not normalize incomplete streamed Bash commands when finish_reason is expect(streamedInput).toBe('rg --fi') }) -test('does not repair truncated Bash objects that do not contain command', async () => { +test('repairs truncated JSON objects even without command field', async () => { globalThis.fetch = (async (_input, _init) => { const chunks = makeStreamChunks([ { @@ -1496,7 +1496,7 @@ test('does not repair truncated Bash objects that do not contain command', async .map(event => (event.delta as Record).partial_json) .join('') - expect(streamedInput).toBe('{"raw":"{\\"cwd\\":\\"/tmp\\""}') + expect(streamedInput).toBe('{"cwd":"/tmp"}') }) test('preserves raw input for unknown plain string tool arguments', async () => { @@ -1554,7 +1554,7 @@ test('preserves raw input for unknown plain string tool arguments', async () => type: 'tool_use', id: 'function-call-1', name: 'UnknownTool', - input: { raw: 'pwd' }, + input: {}, }, ]) }) diff --git a/src/services/api/openaiShim.ts b/src/services/api/openaiShim.ts index f1100e0e..fd55b641 100644 --- a/src/services/api/openaiShim.ts +++ b/src/services/api/openaiShim.ts @@ -44,6 +44,7 @@ import { sanitizeSchemaForOpenAICompat } from '../../utils/schemaSanitizer.js' import { redactSecretValueForDisplay } from '../../utils/providerProfile.js' import { normalizeToolArguments, + hasToolFieldMapping, } from './toolArgumentNormalization.js' type SecretValueSource = Partial<{ @@ -486,7 +487,7 @@ const JSON_REPAIR_SUFFIXES = [ function repairPossiblyTruncatedObjectJson(raw: string): string | null { try { const parsed = JSON.parse(raw) - return parsed && typeof parsed === 'object' && !Array.isArray(parsed) && typeof (parsed as Record).command === 'string' + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? raw : null } catch { @@ -494,12 +495,7 @@ function repairPossiblyTruncatedObjectJson(raw: string): string | null { try { const repaired = raw + combo const parsed = JSON.parse(repaired) - if ( - parsed && - typeof parsed === 'object' && - !Array.isArray(parsed) && - typeof (parsed as Record).command === 'string' - ) { + if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { return repaired } } catch {} @@ -619,7 +615,7 @@ async function* openaiStreamToAnthropic( const toolBlockIndex = contentBlockIndex const initialArguments = tc.function.arguments ?? '' - const normalizeAtStop = tc.function.name === 'Bash' + const normalizeAtStop = hasToolFieldMapping(tc.function.name) activeToolCalls.set(tc.index, { id: tc.id, name: tc.function.name, diff --git a/src/services/api/toolArgumentNormalization.test.ts b/src/services/api/toolArgumentNormalization.test.ts index 9f522b5e..1d12fc9c 100644 --- a/src/services/api/toolArgumentNormalization.test.ts +++ b/src/services/api/toolArgumentNormalization.test.ts @@ -19,20 +19,18 @@ describe('normalizeToolArguments', () => { ).toEqual({ command: 'echo hi' }) }) - test('returns { raw } for blank string', () => { - expect(normalizeToolArguments('Bash', '')).toEqual({ raw: '' }) - expect(normalizeToolArguments('Bash', ' ')).toEqual({ raw: ' ' }) + test('returns empty object for blank string', () => { + expect(normalizeToolArguments('Bash', '')).toEqual({}) + expect(normalizeToolArguments('Bash', ' ')).toEqual({}) }) - test('returns { raw } for JSON-encoded blank string', () => { - expect(normalizeToolArguments('Bash', '""')).toEqual({ raw: '' }) - expect(normalizeToolArguments('Bash', '" "')).toEqual({ raw: ' ' }) + test('returns parsed blank for JSON-encoded blank string', () => { + expect(normalizeToolArguments('Bash', '""')).toEqual('') + expect(normalizeToolArguments('Bash', '" "')).toEqual(' ') }) - test('returns { raw } for likely structured object literal that fails parse', () => { - expect(normalizeToolArguments('Bash', '{ "command": "pwd"')).toEqual({ - raw: '{ "command": "pwd"', - }) + test('returns empty object for malformed structured object literal', () => { + expect(normalizeToolArguments('Bash', '{ "command": "pwd"')).toEqual({}) }) test.each([ @@ -40,9 +38,9 @@ describe('normalizeToolArguments', () => { ["{'command':'pwd'}"], ['{command: pwd}'], ])( - 'returns { raw } for malformed object-shaped string %s (does not wrap into command)', + 'returns empty object for malformed object-shaped string %s (does not wrap into command)', (input) => { - expect(normalizeToolArguments('Bash', input)).toEqual({ raw: input }) + expect(normalizeToolArguments('Bash', input)).toEqual({}) }, ) @@ -157,10 +155,8 @@ describe('normalizeToolArguments', () => { }) describe('unknown tools', () => { - test('returns { raw } for plain string', () => { - expect(normalizeToolArguments('UnknownTool', 'some value')).toEqual({ - raw: 'some value', - }) + test('returns empty object for plain string (no known field mapping)', () => { + expect(normalizeToolArguments('UnknownTool', 'some value')).toEqual({}) }) test('passes through structured JSON object', () => { @@ -175,7 +171,7 @@ describe('normalizeToolArguments', () => { expect(normalizeToolArguments('UnknownTool', '[]')).toEqual([]) }) - test('returns JSON-encoded string as parsed string for unknown tools', () => { + test('returns parsed string for JSON-encoded string on unknown tools', () => { expect(normalizeToolArguments('UnknownTool', '"hello"')).toEqual( 'hello', ) diff --git a/src/services/api/toolArgumentNormalization.ts b/src/services/api/toolArgumentNormalization.ts index ecfa3f48..6052f823 100644 --- a/src/services/api/toolArgumentNormalization.ts +++ b/src/services/api/toolArgumentNormalization.ts @@ -26,6 +26,10 @@ function getPlainStringToolArgumentField(toolName: string): string | null { return STRING_ARGUMENT_TOOL_FIELDS[toolName] ?? null } +export function hasToolFieldMapping(toolName: string): boolean { + return toolName in STRING_ARGUMENT_TOOL_FIELDS +} + function wrapPlainStringToolArguments( toolName: string, value: string, @@ -46,25 +50,20 @@ export function normalizeToolArguments( if (isRecord(parsed)) { return parsed } - if (toolName === 'Bash') { - if (typeof parsed === 'string') { - if (isBlankString(parsed)) { - return { raw: parsed } - } - return wrapPlainStringToolArguments(toolName, parsed) ?? parsed - } - return parsed - } - if (typeof parsed === 'string') { + // Parsed as a non-object JSON value (string, number, boolean, null, array) + if (typeof parsed === 'string' && !isBlankString(parsed)) { return wrapPlainStringToolArguments(toolName, parsed) ?? parsed } + // For blank strings, booleans, null, arrays — pass through as-is + // and let Zod schema validation produce a meaningful error return parsed } catch { + // rawArguments is not valid JSON — treat as a plain string if (isBlankString(rawArguments) || isLikelyStructuredObjectLiteral(rawArguments)) { - return { raw: rawArguments } + // Blank or looks like a malformed object literal — don't wrap into + // a tool field to avoid turning garbage into executable input + return {} } - return ( - wrapPlainStringToolArguments(toolName, rawArguments) ?? { raw: rawArguments } - ) + return wrapPlainStringToolArguments(toolName, rawArguments) ?? {} } }