diff --git a/src/query/proxyToolCommit.test.ts b/src/query/proxyToolCommit.test.ts index e6921471..a9551d0c 100644 --- a/src/query/proxyToolCommit.test.ts +++ b/src/query/proxyToolCommit.test.ts @@ -7,7 +7,7 @@ import { createSandboxedTestEnvironment } from '../../scripts/pr/test-environmen import type { Tool, ToolUseContext } from '../Tool.js' import type { QueryParams } from '../query.js' -const scenarios = ['chat-eof', 'chat-length', 'chat-error', 'chat-completed', 'responses-incomplete', 'responses-failed', 'responses-done-only', 'anthropic-duplicate', 'anthropic-eof', 'anthropic-truncated'] as const +const scenarios = ['chat-eof', 'chat-length', 'chat-error', 'chat-completed', 'responses-incomplete', 'responses-failed', 'responses-done-only', 'anthropic-duplicate', 'anthropic-eof', 'anthropic-truncated', 'chat-malformed-corrected', 'chat-malformed-repeated', 'chat-mixed-corrected'] as const type Scenario = typeof scenarios[number] const resultPrefix = 'PROXY_TOOL_COMMIT_RESULT:' const childScenario = process.env.CC_HAHA_PROXY_TOOL_COMMIT_SCENARIO @@ -37,9 +37,16 @@ async function runScenario(root: string, scenario: Scenario) { let executions = 0 const committedToolIds: string[] = [] let requests = 0 + let maxTurnsReached = false + const executedPaths: string[] = [] + const feedback: Array<{ request: number; id: string; error: boolean; content: string }> = [] + const correctionScenario = scenario === 'chat-malformed-corrected' || scenario === 'chat-mixed-corrected' + const repeatedMalformed = scenario === 'chat-malformed-repeated' + const malformedScenario = correctionScenario || repeatedMalformed const target = join(root, `${scenario}.txt`) const input = { file_path: target, content: 'written exactly once' } const args = JSON.stringify(input) + const malformedArgs = JSON.stringify({ file_path: target }).slice(0, -1) + ',"content": invalid}' const isChat = scenario.startsWith('chat-') let wire = isChat ? chatTool(args, scenario === 'chat-completed' ? 'tool_calls' : scenario === 'chat-length' ? 'length' : undefined) @@ -59,8 +66,25 @@ async function runScenario(root: string, scenario: Scenario) { } if (scenario === 'chat-error') wire += `data: ${JSON.stringify({ error: { type: 'server_error', message: 'fixture upstream failure' } })}\n\n` const server = Bun.serve({ hostname: '127.0.0.1', port: 0, async fetch(request) { - await request.json() + const body = await request.json() as { messages?: Array<{ content?: unknown }> } requests++ + for (const message of body.messages ?? []) { + if (!Array.isArray(message.content)) continue + for (const block of message.content) { + if (block?.type === 'tool_result') feedback.push({ request: requests, id: block.tool_use_id, error: block.is_error === true, content: JSON.stringify(block.content) }) + } + } + if (malformedScenario) { + let calls: Array<{ id: string; arguments: string }> = [] + if (requests === 1 || repeatedMalformed) { + calls = [{ id: repeatedMalformed ? `call_invalid_${requests}` : 'call_fixture', arguments: malformedArgs }] + if (scenario === 'chat-mixed-corrected') calls.unshift({ id: 'call_valid', arguments: JSON.stringify({ ...input, file_path: join(root, 'already-written.txt') }) }) + } else if (requests === 2) calls = [{ id: 'call_corrected', arguments: args }] + const correctedWire = calls.length + ? chatTools(calls) + : `data: ${JSON.stringify({ choices: [{ delta: { content: 'complete' }, finish_reason: 'stop' }] })}\n\n` + return new Response(openaiChatStreamToAnthropic(upstream(correctedWire), 'fixture-model'), { headers: { 'content-type': 'text/event-stream' } }) + } const stream = requests === 1 ? (scenario.startsWith('anthropic-') ? upstream(wire) : isChat ? openaiChatStreamToAnthropic(upstream(wire), 'fixture-model') : openaiResponsesStreamToAnthropic(upstream(wire), 'fixture-model')) : openaiChatStreamToAnthropic(upstream(`data: ${JSON.stringify({ choices: [{ delta: { content: 'complete' }, finish_reason: 'stop' }] })}\n\n`), 'fixture-model') @@ -73,6 +97,7 @@ async function runScenario(root: string, scenario: Scenario) { isEnabled: () => true, userFacingName: () => 'fixture write', description: async () => 'Write a fixture file', call: async (value: typeof input) => { executions++ + executedPaths.push(value.file_path) await writeFile(value.file_path, value.content) return { data: 'written' } }, @@ -92,18 +117,19 @@ async function runScenario(root: string, scenario: Scenario) { messages: [createUserMessage({ content: 'Run the fixture write once' })], systemPrompt: asSystemPrompt([]), userContext: {}, systemContext: {}, canUseTool: async (_tool, value) => ({ behavior: 'allow', updatedInput: value }), - toolUseContext, querySource: 'sdk', maxTurns: 2, + toolUseContext, querySource: 'sdk', maxTurns: malformedScenario ? 3 : 2, deps: { callModel, microcompact: async messages => ({ messages }), autocompact: async () => ({}), uuid: randomUUID }, } try { for await (const message of query(params)) { + if (message.type === 'attachment' && message.attachment.type === 'max_turns_reached') maxTurnsReached = true if (message.type === 'assistant') { for (const block of message.message.content) { if (block.type === 'tool_use') committedToolIds.push(block.id) } } } - return { executions, requests, committedToolIds } + return { executions, requests, committedToolIds, executedPaths, feedback, maxTurnsReached } } finally { toolUseContext.abortController.abort() server.stop(true) @@ -123,6 +149,12 @@ function event(type: string, fields: Record): string { return `event: ${type}\ndata: ${JSON.stringify({ type, ...fields })}\n\n` } +function chatTools(calls: Array<{ id: string; arguments: string }>): string { + return `data: ${JSON.stringify({ choices: [{ index: 0, delta: { tool_calls: calls.map((call, index) => ({ + index, id: call.id, type: 'function', function: { name: 'FixtureWrite', arguments: call.arguments }, + })) }, finish_reason: 'tool_calls' }] })}\n\n` +} + function chatTool(argumentsJson: string, finish?: string): string { return `data: ${JSON.stringify({ choices: [{ index: 0, delta: { tool_calls: [{ index: 0, id: 'call_fixture', type: 'function', @@ -180,6 +212,33 @@ for (const scenario of scenarios) { const resultLine = stdout.split('\n').find(line => line.startsWith(resultPrefix)) expect(resultLine, stdout + stderr).toBeDefined() const result = JSON.parse(resultLine!.slice(resultPrefix.length)) + if (scenario === 'chat-malformed-corrected' || scenario === 'chat-mixed-corrected' || scenario === 'chat-malformed-repeated') { + const mixed = scenario === 'chat-mixed-corrected' + const repeated = scenario === 'chat-malformed-repeated' + expect(result.requests).toBe(3) + expect(result.maxTurnsReached).toBe(repeated) + expect(result.executions).toBe(repeated ? 0 : mixed ? 2 : 1) + expect(result.committedToolIds).toEqual(repeated + ? ['call_invalid_1', 'call_invalid_2', 'call_invalid_3'] + : mixed ? ['call_valid', 'call_fixture', 'call_corrected'] : ['call_fixture', 'call_corrected']) + const invalidId = repeated ? 'call_invalid_1' : 'call_fixture' + expect(result.feedback).toContainEqual({ request: 2, id: invalidId, error: true, content: expect.stringContaining('InputValidationError') }) + const target = join(root, `${scenario}.txt`) + if (repeated) { + expect(result.executedPaths).toEqual([]) + expect(await Bun.file(target).exists()).toBe(false) + expect(result.feedback).toContainEqual({ request: 3, id: 'call_invalid_2', error: true, content: expect.stringContaining('InputValidationError') }) + } else { + expect(await readFile(target, 'utf8')).toBe('written exactly once') + expect(result.executedPaths).toEqual(mixed ? [join(root, 'already-written.txt'), target] : [target]) + expect(result.feedback).toContainEqual({ request: 3, id: 'call_corrected', error: false, content: JSON.stringify('written') }) + if (mixed) { + expect(await readFile(join(root, 'already-written.txt'), 'utf8')).toBe('written exactly once') + expect(result.feedback).toContainEqual({ request: 2, id: 'call_valid', error: false, content: JSON.stringify('written') }) + } + } + return + } const success = scenario === 'chat-completed' || scenario === 'responses-done-only' || scenario === 'anthropic-duplicate' expect(result.executions).toBe(success ? 1 : 0) expect(result.committedToolIds).toEqual(success ? ['call_fixture'] : []) diff --git a/src/server/proxy/streaming/openaiChatStreamToAnthropic.test.ts b/src/server/proxy/streaming/openaiChatStreamToAnthropic.test.ts index 9e5330b3..e5b7828d 100644 --- a/src/server/proxy/streaming/openaiChatStreamToAnthropic.test.ts +++ b/src/server/proxy/streaming/openaiChatStreamToAnthropic.test.ts @@ -52,7 +52,7 @@ describe('Chat stream protocol boundaries', () => { expect(events.some(e => e.type === 'message_stop')).toBe(false) }) - for (const broken of [tool(0, { id: '' }), tool(0, { function: { name: '', arguments: '{}' } }), tool(0, { function: { name: 'Read', arguments: '{"path":' } }), tool(0, { function: { name: 'Read', arguments: '[]' } })]) { + for (const broken of [tool(0, { id: '' }), tool(0, { function: { name: '', arguments: '{}' } }), tool(0, { function: { name: 'Read', arguments: '[]' } })]) { test(`rejects completed invalid tool ${JSON.stringify(broken)}`, async () => { const events = await collect(chunk({ tool_calls: [broken] }) + chunk({}, 'tool_calls')) expect(events.at(-1).type).toBe('error') @@ -60,6 +60,16 @@ describe('Chat stream protocol boundaries', () => { }) } + test('completed malformed arguments reach CLI validation unchanged', async () => { + const raw = '{"path":' + const events = await collect(chunk({ tool_calls: [tool(0, { function: { name: 'Read', arguments: raw } })] }) + chunk({}, 'tool_calls')) + expect(events.at(-1).type).toBe('message_stop') + expect(events.find(e => e.type === 'message_delta').delta.stop_reason).toBe('tool_use') + expect(events.find(e => e.content_block?.type === 'tool_use').content_block.id).toBe('call_0') + expect(events.filter(e => e.delta?.partial_json).map(e => e.delta.partial_json).join('')).toBe(raw) + expect(events.some(e => e.type === 'error')).toBe(false) + }) + test('length retains truncation cause without synthesizing tool success', async () => { const events = await collect(chunk({ tool_calls: [tool(0, { function: { name: 'Read', arguments: '{"path":' } })] }) + chunk({}, 'length')) expect(events.find(e => e.type === 'message_delta').delta.stop_reason).toBe('max_tokens') diff --git a/src/server/proxy/streaming/openaiChatStreamToAnthropic.ts b/src/server/proxy/streaming/openaiChatStreamToAnthropic.ts index ec0dcfc5..50403751 100644 --- a/src/server/proxy/streaming/openaiChatStreamToAnthropic.ts +++ b/src/server/proxy/streaming/openaiChatStreamToAnthropic.ts @@ -517,6 +517,8 @@ function handleFinishReason( if (finishReason !== 'length' && finishReason !== 'content_filter') { for (const block of state.toolBlocks.values()) { if (!block.id.trim() || !block.name.trim()) throw new Error('OpenAI Chat tool call is missing its id or function name') + // Syntax errors are tool input errors, not transport failures. Keep the + // original deltas so the CLI can return a matching error tool_result. parseCompleteChatToolArguments(block.argsBuffer) } if (stopReason === 'tool_use' && state.toolBlocks.size === 0) { diff --git a/src/server/proxy/streaming/openaiResponsesContract.test.ts b/src/server/proxy/streaming/openaiResponsesContract.test.ts index 28003128..c3dec8f4 100644 --- a/src/server/proxy/streaming/openaiResponsesContract.test.ts +++ b/src/server/proxy/streaming/openaiResponsesContract.test.ts @@ -82,8 +82,8 @@ describe('Responses tool finalization and snapshots', () => { const events = await collect(startTool + event('response.function_call_arguments.delta', { item_id: 'fc_1', delta: '{"path":' }) + completed([tool])) expect(events.filter(item => item.delta?.type === 'input_json_delta').map(item => item.delta.partial_json).join('')).toBe(tool.arguments) }) - test('completed malformed, missing and conflicting arguments are rejected', async () => { - for (const argumentsValue of ['{"path":', '[]', 'null']) { + test('completed non-object, missing and conflicting arguments are rejected', async () => { + for (const argumentsValue of ['[]', 'null']) { await expect(collect(startTool + event('response.function_call_arguments.delta', { item_id: 'fc_1', delta: argumentsValue }) + completed())).rejects.toThrow('tool arguments') } await expect(collect(startTool + completed())).rejects.toThrow('tool arguments') @@ -105,8 +105,8 @@ describe('Responses non-streaming terminal validation', () => { test('content_filter is not labeled as a token limit', () => { expect(() => openaiResponsesToAnthropic(response({ status: 'incomplete', incomplete_details: { reason: 'content_filter' } }), 'fixture')).toThrow('content_filter') }) - test('malformed completed tool input is not converted into raw or empty objects', () => { - for (const argumentsValue of ['{"path":', '', null, '[]']) { + test('missing and non-object completed tool input is rejected', () => { + for (const argumentsValue of ['', null, '[]']) { expect(() => openaiResponsesToAnthropic(response({ output: [{ ...tool, arguments: argumentsValue }] }), 'fixture')).toThrow('tool arguments') } }) @@ -191,3 +191,24 @@ test('corrupt SSE frames cannot be hidden by a later completed tool snapshot', a await expect(openaiResponsesStreamToAnthropicResponse(stream(input), 'fixture', { openAICodexOAuth: oauth })).rejects.toThrow('Invalid OpenAI Responses SSE JSON') } }) + +for (const oauth of [false, true]) { + test(`completed malformed arguments survive for CLI correction (oauth=${oauth})`, async () => { + const raw = '{"path":"fixture","text":"broken\\escape"}' + const input = startTool + event('response.function_call_arguments.delta', { item_id: 'fc_1', delta: raw }) + completed([{ ...tool, arguments: raw }]) + const events = await collect(input, oauth) + expect(events.filter(item => item.delta?.type === 'input_json_delta').map(item => item.delta.partial_json).join('')).toBe(raw) + expect(events.find(item => item.type === 'message_delta')?.delta.stop_reason).toBe('tool_use') + const result = await openaiResponsesStreamToAnthropicResponse(stream(input), 'fixture', { openAICodexOAuth: oauth }) + expect(result.content.find(item => item.type === 'tool_use')).toMatchObject({ id: 'call_1', input: { __unparsedToolInput: { raw, len: raw.length } } }) + const buffered = openaiResponsesToAnthropic(response({ output: [{ ...tool, arguments: raw }] }), 'fixture') + expect(buffered.content.find(item => item.type === 'tool_use')).toMatchObject({ id: 'call_1', input: { __unparsedToolInput: { raw, len: raw.length } } }) + }) +} + +test('different malformed arguments with identical truncated markers remain a snapshot conflict', async () => { + const prefix = '{"text":"' + 'a'.repeat(2200) + const delta = prefix + 'X' + const final = prefix + 'Y' + await expect(collect(startTool + event('response.function_call_arguments.delta', { item_id: 'fc_1', delta }) + completed([{ ...tool, arguments: final }]))).rejects.toThrow('snapshot conflicts') +}) diff --git a/src/server/proxy/streaming/openaiResponsesStreamToAnthropic.ts b/src/server/proxy/streaming/openaiResponsesStreamToAnthropic.ts index 5402072d..9fe2ff01 100644 --- a/src/server/proxy/streaming/openaiResponsesStreamToAnthropic.ts +++ b/src/server/proxy/streaming/openaiResponsesStreamToAnthropic.ts @@ -586,7 +586,9 @@ function reconcileToolArguments(index: number, value: unknown, state: StreamStat // Compatible providers can serialize the same final object differently. // Keep the emitted representation when the parsed inputs are identical. try { - if (isDeepStrictEqual(parseResponsesToolArguments(text), parseResponsesToolArguments(tool.arguments))) return + const finalInput = asRecord(JSON.parse(text)) + const streamedInput = asRecord(JSON.parse(tool.arguments)) + if (finalInput && streamedInput && isDeepStrictEqual(finalInput, streamedInput)) return } catch { // Partial JSON cannot establish semantic equality. } diff --git a/src/server/proxy/transform/openaiChatToAnthropic.test.ts b/src/server/proxy/transform/openaiChatToAnthropic.test.ts index 710fa19f..3b1f9b9a 100644 --- a/src/server/proxy/transform/openaiChatToAnthropic.test.ts +++ b/src/server/proxy/transform/openaiChatToAnthropic.test.ts @@ -7,11 +7,20 @@ function response(tool: Record, reason = 'tool_calls'): OpenAIC } describe('Chat non-streaming response integrity', () => { - for (const args of ['{"path":', '[]', 'null', '42']) { + for (const args of ['[]', 'null', '42']) { test(`completed malformed/non-object arguments reject: ${args}`, () => { expect(() => openaiChatToAnthropic(response({ id: 'call', function: { name: 'Read', arguments: args } }), 'fixture')).toThrow() }) } + test('completed malformed arguments retain identity and a bounded non-executable marker', () => { + const raw = '{"path":"' + 'x'.repeat(3000) + const result = openaiChatToAnthropic(response({ id: 'call', function: { name: 'Read', arguments: raw } }), 'fixture') + expect(result.stop_reason).toBe('tool_use') + expect(result.content[0]).toEqual({ + type: 'tool_use', id: 'call', name: 'Read', + input: { __unparsedToolInput: { raw: raw.slice(0, 2048), len: raw.length } }, + }) + }) test('error envelope rejects rather than returning empty success', () => { expect(() => openaiChatToAnthropic({ error: { message: 'Fixture failure' } } as unknown as OpenAIChatResponse, 'fixture')).toThrow('Fixture failure') }) diff --git a/src/server/proxy/transform/openaiChatToAnthropic.ts b/src/server/proxy/transform/openaiChatToAnthropic.ts index e69f51c3..5403fa20 100644 --- a/src/server/proxy/transform/openaiChatToAnthropic.ts +++ b/src/server/proxy/transform/openaiChatToAnthropic.ts @@ -10,6 +10,7 @@ import type { AnthropicContentBlock, } from './types.js' import { openaiUsageToAnthropic } from './usage.js' +import { createUnparsedToolInput } from '../../../utils/unparsedToolInput.js' /** * Convert OpenAI Chat Completions response to Anthropic Messages response. @@ -99,9 +100,17 @@ function mapFinishReason(reason: string | null): string { } } -/** Completed tool input must be an object, never a repaired fragment. */ +/** Preserve malformed completed calls for CLI validation, never repair or execute them. */ export function parseCompleteChatToolArguments(value: unknown): Record { - const parsed: unknown = value == null || value === '' ? {} : typeof value === 'string' ? JSON.parse(value) : value + let parsed: unknown = value == null || value === '' ? {} : value + if (typeof value === 'string' && value !== '') { + try { + parsed = JSON.parse(value) + } catch (error) { + if (!(error instanceof SyntaxError)) throw error + return createUnparsedToolInput(value) + } + } if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { throw new Error('OpenAI Chat completed tool arguments must be a JSON object') } diff --git a/src/server/proxy/transform/openaiResponsesTerminal.ts b/src/server/proxy/transform/openaiResponsesTerminal.ts index d850f6c8..c35dbb08 100644 --- a/src/server/proxy/transform/openaiResponsesTerminal.ts +++ b/src/server/proxy/transform/openaiResponsesTerminal.ts @@ -1,3 +1,4 @@ +import { createUnparsedToolInput } from '../../../utils/unparsedToolInput.js' import { getOpenAIPolicyError } from '../../../services/openaiAuth/policyError.js' export function responsesRecord(value: unknown): Record | null { @@ -39,14 +40,14 @@ export function responsesTerminalStop( throw Object.assign(new Error(`OpenAI response has no valid terminal status: ${status ?? 'missing'}`), { type: 'api_error' }) } -/** Tool input is executable data: malformed JSON must never become {} or raw. */ +/** Preserve complete malformed calls as non-executable input for model correction. */ export function parseResponsesToolArguments(value: unknown): Record { let parsed = value if (typeof value === 'string' && value.trim()) { try { parsed = JSON.parse(value) } catch { - throw new Error('Invalid OpenAI Responses tool arguments: incomplete or malformed JSON') + return createUnparsedToolInput(value) } } const record = responsesRecord(parsed) diff --git a/src/services/tools/toolExecution.test.ts b/src/services/tools/toolExecution.test.ts new file mode 100644 index 00000000..f390e40e --- /dev/null +++ b/src/services/tools/toolExecution.test.ts @@ -0,0 +1,94 @@ +import { expect, mock, test } from 'bun:test' +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { createSandboxedTestEnvironment } from '../../../scripts/pr/test-environment.js' +import type { Tool, ToolUseContext } from '../../Tool.js' + +const childScenario = process.env.CC_HAHA_TOOL_INPUT_GUARD_SCENARIO + +// Importing the executor loads the full tools/permissions/MCP graph. Isolate +// those caches from shared-process suites that install their own module mocks. +async function runScenario(root: string, source: string) { + ;(globalThis as typeof globalThis & { MACRO?: { BUILD_TIME: string } }).MACRO = { BUILD_TIME: '' } + const { z } = await import('zod/v4') + const bootstrap = await import('../../bootstrap/state.js') + bootstrap.setCwdState(root) + bootstrap.setOriginalCwd(root) + bootstrap.setProjectRoot(root) + process.chdir(root) + const { createAssistantMessage, normalizeContentFromAPI } = await import('../../utils/messages.js') + const { createUnparsedToolInput } = await import('../../utils/unparsedToolInput.js') + const { runToolUse } = await import('./toolExecution.js') + const raw = '{"path":' + 'x'.repeat(3000) + const marker = createUnparsedToolInput(raw) + const normalized = normalizeContentFromAPI([{ type: 'tool_use', id: 'bad-call', name: 'OptionalTool', input: raw }], [])[0]! + if (normalized.type !== 'tool_use') throw new Error('Expected tool use') + const input = source === 'provider' ? normalized.input : JSON.parse(JSON.stringify(marker)) + const inputSchema = z.object({ optional: z.string().optional() }) + expect(inputSchema.safeParse({}).success).toBe(true) + const safeParse = mock(inputSchema.safeParse.bind(inputSchema)) + inputSchema.safeParse = safeParse + const call = mock(async () => ({ data: 'must not run' })) + const validateInput = mock(async () => ({ result: true })) + const tool = { name: 'OptionalTool', inputSchema, call, validateInput } as unknown as Tool + const canUseTool = mock(async () => ({ behavior: 'allow' as const, updatedInput: {} })) + const toolUse = { type: 'tool_use' as const, id: 'bad-call', name: tool.name, input } + const assistant = createAssistantMessage({ content: [toolUse] }) + const context = { options: { tools: [tool], mcpClients: [] }, abortController: new AbortController(), messages: [] } as unknown as ToolUseContext + const updates = [] + for await (const update of runToolUse(toolUse, assistant, canUseTool, context)) updates.push(update) + expect(updates).toHaveLength(1) + const message = updates[0]!.message + expect(message.type).toBe('user') + if (message.type !== 'user') throw new Error('Expected tool error result') + expect(message.message.content).toEqual([{ + type: 'tool_result', tool_use_id: 'bad-call', is_error: true, + content: expect.stringContaining('InputValidationError: OptionalTool was called with input that could not be parsed as JSON.'), + }]) + const content = JSON.stringify(message.message.content) + expect(content).toContain('Retry with valid JSON') + expect(content).toContain(`first 200 of ${raw.length} bytes`) + expect(content).not.toContain('x'.repeat(201)) + expect(message.toolUseResult).toBe(`InputValidationError: JSON parse failed (${raw.length} bytes)`) + expect(safeParse).not.toHaveBeenCalled() + expect(validateInput).not.toHaveBeenCalled() + expect(canUseTool).not.toHaveBeenCalled() + expect(call).not.toHaveBeenCalled() + +} + +for (const scenario of ['provider', 'history']) { + if (childScenario && childScenario !== scenario) continue + test(`rejects ${scenario} markers before permissive schema, permission checks or tool side effects`, async () => { + if (childScenario) { + await runScenario(process.env.HOME!, scenario) + return + } + const root = await mkdtemp(join(tmpdir(), 'tool-input-guard-')) + const child = Bun.spawn([process.execPath, '--no-env-file', 'test', fileURLToPath(import.meta.url)], { + cwd: root, + env: createSandboxedTestEnvironment(root, { + CC_HAHA_TOOL_INPUT_GUARD_SCENARIO: scenario, + NODE_ENV: 'production', + CLAUDE_CODE_SIMPLE: '1', + CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC: '1', + ANTHROPIC_API_KEY: 'offline-fixture-key', + }), + stdin: 'ignore', stdout: 'pipe', stderr: 'pipe', + }) + const timeout = setTimeout(() => child.kill('SIGKILL'), 15_000) + try { + const [exitCode, stdout, stderr] = await Promise.all([ + child.exited, new Response(child.stdout).text(), new Response(child.stderr).text(), + ]) + expect(exitCode, stdout + stderr).toBe(0) + } finally { + clearTimeout(timeout) + child.kill() + await child.exited + await rm(root, { recursive: true, force: true }) + } + }, 20_000) +} diff --git a/src/services/tools/toolExecution.ts b/src/services/tools/toolExecution.ts index 3c95f98e..02396e79 100644 --- a/src/services/tools/toolExecution.ts +++ b/src/services/tools/toolExecution.ts @@ -86,6 +86,7 @@ import { stopSessionActivity, } from '../../utils/sessionActivity.js' import { jsonStringify } from '../../utils/slowOperations.js' +import { isUnparsedToolInput } from '../../utils/unparsedToolInput.js' import { Stream } from '../../utils/stream.js' import { logOTelEvent } from '../../utils/telemetry/events.js' import { @@ -341,6 +342,26 @@ export async function* runToolUse( toolUseContext: ToolUseContext, ): AsyncGenerator { const toolName = toolUse.name + if (isUnparsedToolInput(toolUse.input)) { + const { raw, len } = toolUse.input.__unparsedToolInput + const preview = raw.slice(0, 200) + const errorContent = `${toolName} was called with input that could not be parsed as JSON.\n` + + `You sent (first ${preview.length} of ${len} bytes): ${preview}\n` + + 'Common causes: unescaped backslashes in file paths (use / or \\\\), unescaped control characters, or truncated output. Retry with valid JSON.' + yield { + message: createUserMessage({ + content: [{ + type: 'tool_result', + tool_use_id: toolUse.id, + is_error: true, + content: `InputValidationError: ${errorContent}`, + }], + toolUseResult: `InputValidationError: JSON parse failed (${len} bytes)`, + sourceToolAssistantUUID: assistantMessage.uuid, + }), + } + return + } // First try to find in the available tools (what the model sees) let tool = findToolByName(toolUseContext.options.tools, toolName) diff --git a/src/utils/messages.test.ts b/src/utils/messages.test.ts index 4cd3cde0..818a4735 100644 --- a/src/utils/messages.test.ts +++ b/src/utils/messages.test.ts @@ -1,10 +1,12 @@ import { describe, expect, test } from 'bun:test' import type { ContentBlockParam } from '@anthropic-ai/sdk/resources/index.mjs' +import type { Tool } from '../Tool.js' import type { AssistantMessage } from '../types/message.js' import { createAssistantMessage, createUserMessage, normalizeMessagesForAPI, + normalizeContentFromAPI, stripSignatureBlocksAfterModelChange, } from './messages.js' @@ -229,3 +231,46 @@ describe('stripSignatureBlocksAfterModelChange', () => { ).toBe(messages) }) }) + +describe('malformed provider tool arguments', () => { + test('preserves invalid JSON as a bounded marker rather than executable empty input', () => { + const raw = '{"path":' + 'x'.repeat(3000) + const [block] = normalizeContentFromAPI([{ type: 'tool_use', id: 'bad-call', name: 'TaskList', input: raw }], []) + expect(block).toEqual({ type: 'tool_use', id: 'bad-call', name: 'TaskList', input: { __unparsedToolInput: { raw: raw.slice(0, 2048), len: raw.length } } }) + }) + + test('retains existing history markers and still accepts genuine empty input', () => { + const marker = { __unparsedToolInput: { raw: '{broken', len: 7 } } + const normalized = normalizeContentFromAPI([ + { type: 'tool_use', id: 'historical', name: 'Read', input: marker }, + { type: 'tool_use', id: 'empty', name: 'TaskList', input: '' }, + { type: 'tool_use', id: 'valid', name: 'TaskList', input: '{}' }, + { type: 'tool_use', id: 'null', name: 'TaskList', input: ' null ' }, + ], []) + expect(normalized.map(block => block.type === 'tool_use' ? block.input : null)).toEqual([marker, {}, {}, {}]) + }) +}) + + +test('malformed and restored markers bypass TaskOutput default injection and survive API history normalization', () => { + const tool = { name: 'TaskOutput' } as Tool + const raw = '{truncated' + const marker = { __unparsedToolInput: { raw, len: raw.length } } + for (const input of [raw, marker]) { + const blocks = normalizeContentFromAPI([{ type: 'tool_use', id: 'task-output', name: tool.name, input }], [tool]) + expect(blocks[0]).toMatchObject({ input: marker }) + const restored = JSON.parse(JSON.stringify(createAssistantMessage({ content: blocks }))) + const history = normalizeMessagesForAPI([restored, toolResult('task-output')], [tool]) + expect(history[0]!.message.content).toEqual([{ type: 'tool_use', id: 'task-output', name: tool.name, input: marker }]) + } +}) + +test('legacy ordinary tool-use JSON still round-trips without a migration', () => { + const tool = { name: 'Read' } as Tool + const historical = JSON.parse(JSON.stringify(assistant('legacy-response', [toolUse('legacy-read')]))) + const blocks = normalizeContentFromAPI(historical.message.content, [tool]) + expect(blocks).toEqual(historical.message.content) + historical.message.content = blocks + const replay = normalizeMessagesForAPI([historical, toolResult('legacy-read')], [tool]) + expect(replay[0]!.message.content).toEqual(historical.message.content) +}) diff --git a/src/utils/messages.ts b/src/utils/messages.ts index 506177b3..812e9449 100644 --- a/src/utils/messages.ts +++ b/src/utils/messages.ts @@ -162,6 +162,7 @@ import { TASK_OUTPUT_TOOL_NAME } from '../tools/TaskOutputTool/constants.js' import { TASK_UPDATE_TOOL_NAME } from '../tools/TaskUpdateTool/constants.js' import type { PermissionMode } from '../types/permissions.js' import { normalizeToolInput, normalizeToolInputForAPI } from './api.js' +import { createUnparsedToolInput, isUnparsedToolInput } from './unparsedToolInput.js' import { getCurrentProjectConfig } from './config.js' import { logAntError, logForDebugging } from './debug.js' import { stripIdeContextTags } from './displayTags.js' @@ -2273,7 +2274,7 @@ export function normalizeMessagesForAPI( content: message.message.content.map(block => { if (block.type === 'tool_use') { const tool = tools.find(t => toolMatchesName(t, block.name)) - const normalizedInput = tool + const normalizedInput = tool && !isUnparsedToolInput(block.input) ? normalizeToolInputForAPI( tool, block.input as Record, @@ -2852,11 +2853,8 @@ export function normalizeContentFromAPI( let normalizedInput: unknown if (typeof contentBlock.input === 'string') { const parsed = safeParseJSON(contentBlock.input) - if (parsed === null && contentBlock.input.length > 0) { - // TET/FC-v3 diagnostic: the streamed tool input JSON failed to - // parse. We fall back to {} which means downstream validation - // sees empty input. The raw prefix goes to debug log only — no - // PII-tagged proto column exists for it yet. + if (parsed === null && contentBlock.input.trim() !== 'null' && contentBlock.input.length > 0) { + normalizedInput = createUnparsedToolInput(contentBlock.input) logEvent('tengu_tool_input_json_parse_fail', { toolName: sanitizeToolNameForAnalytics(contentBlock.name), inputLen: contentBlock.input.length, @@ -2867,14 +2865,15 @@ export function normalizeContentFromAPI( { level: 'warn' }, ) } + } else { + normalizedInput = parsed ?? {} } - normalizedInput = parsed ?? {} } else { normalizedInput = contentBlock.input } // Then apply tool-specific corrections - if (typeof normalizedInput === 'object' && normalizedInput !== null) { + if (typeof normalizedInput === 'object' && normalizedInput !== null && !isUnparsedToolInput(normalizedInput)) { const tool = findToolByName(tools, contentBlock.name) if (tool) { try { diff --git a/src/utils/unparsedToolInput.test.ts b/src/utils/unparsedToolInput.test.ts new file mode 100644 index 00000000..95e7d239 --- /dev/null +++ b/src/utils/unparsedToolInput.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, test } from 'bun:test' +import { createUnparsedToolInput, isUnparsedToolInput } from './unparsedToolInput.js' + +describe('unparsed tool input marker', () => { + test('bounds the raw argument prefix while retaining original length', () => { + const raw = '{' + 'x'.repeat(3000) + expect(createUnparsedToolInput(raw)).toEqual({ __unparsedToolInput: { raw: raw.slice(0, 2048), len: raw.length } }) + expect(isUnparsedToolInput(createUnparsedToolInput(raw))).toBe(true) + }) + + test.each([null, [], {}, { __unparsedToolInput: null }, { __unparsedToolInput: 'bad' }, + { __unparsedToolInput: { raw: 'bad', len: '3' } }, + { __unparsedToolInput: { raw: 'bad', len: 3 }, legitimate: true }, + ].map(input => [input]))('does not treat ordinary or malformed objects as the reserved marker: %j', input => { + expect(isUnparsedToolInput(input)).toBe(false) + }) +}) diff --git a/src/utils/unparsedToolInput.ts b/src/utils/unparsedToolInput.ts new file mode 100644 index 00000000..d27151d2 --- /dev/null +++ b/src/utils/unparsedToolInput.ts @@ -0,0 +1,16 @@ +/** Preserve invalid provider arguments without turning them into executable {}. */ +export type UnparsedToolInput = { + __unparsedToolInput: { raw: string; len: number } +} + +export function createUnparsedToolInput(raw: string): UnparsedToolInput { + return { __unparsedToolInput: { raw: raw.slice(0, 2048), len: raw.length } } +} + +export function isUnparsedToolInput(input: unknown): input is UnparsedToolInput { + if (typeof input !== 'object' || input === null || Array.isArray(input)) return false + if (Object.keys(input).length !== 1 || !Object.hasOwn(input, '__unparsedToolInput')) return false + const value = (input as UnparsedToolInput).__unparsedToolInput + return typeof value === 'object' && value !== null + && typeof value.raw === 'string' && typeof value.len === 'number' +}