diff --git a/test/harness/replayingCapiProxy.test.ts b/test/harness/replayingCapiProxy.test.ts index c5747a306..cbe058093 100644 --- a/test/harness/replayingCapiProxy.test.ts +++ b/test/harness/replayingCapiProxy.test.ts @@ -11,7 +11,7 @@ import type { } from "openai/resources/chat/completions"; import os from "os"; import path from "path"; -import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import yaml from "yaml"; import { NormalizedData, @@ -693,7 +693,7 @@ Always include PINEAPPLE_COCONUT_42. async function makeRequest( proxyUrl: string, requestPath: string, - options?: { method?: string; body?: object }, + options?: { method?: string; body?: object; signal?: AbortSignal }, ): Promise<{ status: number; body: string }> { return new Promise((resolve, reject) => { const url = new URL(proxyUrl); @@ -704,6 +704,7 @@ Always include PINEAPPLE_COCONUT_42. path: requestPath, method: options?.method ?? "POST", headers: { "content-type": "application/json" }, + signal: options?.signal, }, (res) => { const chunks: Buffer[] = []; @@ -724,6 +725,99 @@ Always include PINEAPPLE_COCONUT_42. }); } + function viewMessages( + toolCallId: string, + content: string, + toolArguments: string, + toolName = "view", + userContent = "Read the file", + ) { + return [ + { role: "system", content: "${system}" }, + { role: "user", content: userContent }, + { + role: "assistant", + tool_calls: [ + { + id: toolCallId, + type: "function", + function: { name: toolName, arguments: toolArguments }, + }, + ], + }, + { role: "tool", tool_call_id: toolCallId, content }, + ]; + } + + async function replayViewResult( + savedContent: string, + requestContent: string, + toolArguments: string, + options?: { + errorStatus?: number; + savedToolName?: string; + requestToolName?: string; + requestUserContent?: string; + strict?: boolean; + }, + ) { + const cachePath = path.join(tempDir, "cache.yaml"); + const savedMessages = viewMessages( + "toolcall_0", + savedContent, + toolArguments, + options?.savedToolName, + ); + const cacheContent = yaml.stringify({ + models: ["test-model"], + errors: options?.errorStatus + ? [ + { + model: "test-model", + status: options.errorStatus, + message: "Expected error", + messages: savedMessages, + }, + ] + : undefined, + conversations: options?.errorStatus + ? [] + : [{ messages: [...savedMessages, { role: "assistant", content: "Done" }] }], + } satisfies NormalizedData); + await writeFile(cachePath, cacheContent); + + const proxy = new ReplayingCapiProxy( + "http://localhost:9999", + cachePath, + workDir, + ); + const proxyUrl = await proxy.start(); + if (options?.strict) { + vi.stubEnv("GITHUB_ACTIONS", "true"); + } + try { + return await makeRequest(proxyUrl, "/chat/completions", { + body: { + model: "test-model", + messages: viewMessages( + "runtime-call-id", + requestContent, + toolArguments, + options?.requestToolName, + options?.requestUserContent, + ).map((message) => + message.role === "system" + ? { ...message, content: "System prompt" } + : message, + ), + }, + }); + } finally { + vi.unstubAllEnvs(); + await proxy.stop(); + } + } + test("returns cached response when request matches prefix", async () => { const cachePath = path.join(tempDir, "cache.yaml"); const cacheContent = yaml.stringify({ @@ -820,6 +914,136 @@ Always include PINEAPPLE_COCONUT_42. } }); + test.each([ + { + savedContent: "1. Hello\n2. World\n3.", + requestContent: "Hello\nWorld", + arguments: '{"path":"file"}', + }, + { + savedContent: "2. second\n3. third\n4. fourth", + requestContent: "second\nthird\nfourth", + arguments: '{"path":"file","view_range":[2,4]}', + }, + { + savedContent: "1.", + requestContent: "", + arguments: '{"path":"empty"}', + }, + { + savedContent: '1. {\n2. "b": 2,\n3. "a": 1\n4. }', + requestContent: '{"a":1,"b":2}', + arguments: '{"path":"data.json"}', + }, + ])( + "matches view results across the line-number output transition", + async ({ savedContent, requestContent, arguments: toolArguments }) => { + const response = await replayViewResult( + savedContent, + requestContent, + toolArguments, + ); + expect(response.status).toBe(200); + expect( + (JSON.parse(response.body) as ChatCompletion).choices[0].message + .content, + ).toBe("Done"); + }, + ); + + test("matches cached errors with legacy numbered view results", async () => { + const response = await replayViewResult( + "1. Hello", + "Hello", + '{"path":"file"}', + { errorStatus: 429 }, + ); + expect(response.status).toBe(429); + }); + + test.each([ + { + name: "non-view tool", + savedContent: "1. Hello", + requestContent: "Hello", + arguments: '{"path":"file"}', + options: { savedToolName: "grep", requestToolName: "grep" }, + }, + { + name: "wrong starting line", + savedContent: "2. Hello", + requestContent: "Hello", + arguments: '{"path":"file"}', + options: {}, + }, + { + name: "changed user message", + savedContent: "1. Hello", + requestContent: "Hello", + arguments: '{"path":"file"}', + options: { requestUserContent: "Read a different file" }, + }, + ])( + "rejects legacy view compatibility for $name", + async ({ savedContent, requestContent, arguments: toolArguments, options }) => { + const response = await replayViewResult( + savedContent, + requestContent, + toolArguments, + { ...options, strict: true }, + ); + expect(response.status).toBe(500); + }, + ); + + test("matches request-only snapshots with legacy numbered view results", async () => { + const cachePath = path.join(tempDir, "cache.yaml"); + const toolArguments = '{"path":"file"}'; + const cacheContent = yaml.stringify({ + models: ["test-model"], + conversations: [ + { + messages: viewMessages( + "toolcall_0", + "1. Hello", + toolArguments, + ), + }, + ], + } satisfies NormalizedData); + await writeFile(cachePath, cacheContent); + + const proxy = new ReplayingCapiProxy( + "http://localhost:9999", + cachePath, + workDir, + ); + const proxyUrl = await proxy.start(); + try { + const result = await makeRequest(proxyUrl, "/chat/completions", { + signal: AbortSignal.timeout(50), + body: { + model: "test-model", + messages: viewMessages( + "runtime-call-id", + "Hello", + toolArguments, + ).map((message) => + message.role === "system" + ? { ...message, content: "System prompt" } + : message, + ), + }, + }).then( + (response) => `response:${response.status}`, + (error: Error) => error.name, + ); + expect(result).toBe("AbortError"); + } finally { + await proxy.stop(); + } + }); + test("matches shell tool results with shell ID completion markers", async () => { const originalShellConfig = process.platform === "win32" ? ShellConfig.powerShell : ShellConfig.bash; diff --git a/test/harness/replayingCapiProxy.ts b/test/harness/replayingCapiProxy.ts index 47ebda9f7..69e0b2b54 100644 --- a/test/harness/replayingCapiProxy.ts +++ b/test/harness/replayingCapiProxy.ts @@ -687,7 +687,15 @@ function diagnoseMatchFailure( // Find the first message that doesn't match let mismatchIndex = -1; for (let i = 0; i < requestMessages.length; i++) { - if (JSON.stringify(requestMessages[i]) !== JSON.stringify(saved[i])) { + if ( + !normalizedMessagesEqual( + requestMessages[i], + saved[i], + requestMessages, + saved, + i, + ) + ) { mismatchIndex = i; break; } @@ -830,7 +838,14 @@ async function findSavedChatCompletionError( if ( requestMessages.length === error.messages.length && requestMessages.every( - (msg, i) => JSON.stringify(msg) === JSON.stringify(error.messages[i]), + (msg, i) => + normalizedMessagesEqual( + msg, + error.messages[i], + requestMessages, + error.messages, + i, + ), ) ) { return error; @@ -860,7 +875,13 @@ async function isRequestOnlySnapshot( requestMessages.length === conversation.messages.length && requestMessages.every( (msg, i) => - JSON.stringify(msg) === JSON.stringify(conversation.messages[i]), + normalizedMessagesEqual( + msg, + conversation.messages[i], + requestMessages, + conversation.messages, + i, + ), ) ) { return true; @@ -1201,27 +1222,7 @@ function transformOpenAIRequestMessage( } content = parts.join("\n") || undefined; } else if (m.role === "tool" && typeof m.content === "string") { - // If it's a JSON tool call result, normalize the whitespace and property ordering. - // For successful tool results wrapped in {resultType, textResultForLlm}, unwrap to - // just the inner value so snapshots stay stable across envelope format changes. - try { - const parsed = JSON.parse(m.content); - if ( - parsed && - typeof parsed === "object" && - parsed.resultType === "success" && - "textResultForLlm" in parsed - ) { - content = - typeof parsed.textResultForLlm === "string" - ? parsed.textResultForLlm - : JSON.stringify(sortJsonKeys(parsed.textResultForLlm)); - } else { - content = JSON.stringify(sortJsonKeys(parsed)); - } - } catch { - content = m.content.trim(); - } + content = normalizeToolMessageContent(m.content); } else if (typeof m.content === "string") { content = m.content; } @@ -1552,7 +1553,15 @@ function findAssistantIndexAfterPrefix( for (let i = 0; i < requestMessages.length; i++) { const reqMsg = JSON.stringify(requestMessages[i]); const savedMsg = JSON.stringify(savedMessages[i]); - if (reqMsg !== savedMsg) { + if ( + !normalizedMessagesEqual( + requestMessages[i], + savedMessages[i], + requestMessages, + savedMessages, + i, + ) + ) { log(`mismatch at index ${i}:`); log(` REQ: ${reqMsg.substring(0, 1000)}`); log(` SAVED: ${savedMsg.substring(0, 1000)}`); @@ -1574,6 +1583,124 @@ function findAssistantIndexAfterPrefix( return undefined; } +function normalizedMessagesEqual( + requestMessage: NormalizedMessage, + savedMessage: NormalizedMessage, + requestMessages: NormalizedMessage[], + savedMessages: NormalizedMessage[], + index: number, +): boolean { + if (JSON.stringify(requestMessage) === JSON.stringify(savedMessage)) { + return true; + } + if ( + requestMessage.role !== "tool" || + savedMessage.role !== "tool" || + !requestMessage.tool_call_id || + requestMessage.tool_call_id !== savedMessage.tool_call_id || + typeof savedMessage.content !== "string" + ) { + return false; + } + + const requestToolCall = findToolCall( + requestMessages, + index, + requestMessage.tool_call_id, + ); + const savedToolCall = findToolCall( + savedMessages, + index, + requestMessage.tool_call_id, + ); + if ( + requestToolCall?.function?.name !== "view" || + savedToolCall?.function?.name !== "view" || + normalizeToolMessageContent( + stripLegacyViewLineNumbers( + savedMessage.content, + viewRangeStart(savedToolCall), + ) ?? "", + ) !== requestMessage.content + ) { + return false; + } + + return ( + JSON.stringify({ ...requestMessage, content: savedMessage.content }) === + JSON.stringify(savedMessage) + ); +} + +function findToolCall( + messages: NormalizedMessage[], + beforeIndex: number, + toolCallId: string, +): NormalizedToolCall | undefined { + for (let i = beforeIndex - 1; i >= 0; i--) { + const toolCall = messages[i].tool_calls?.find( + (candidate) => candidate.id === toolCallId, + ); + if (toolCall) { + return toolCall; + } + } + return undefined; +} + +function viewRangeStart(toolCall: NormalizedToolCall): number { + try { + const input = JSON.parse(toolCall.function?.arguments ?? "{}") as { + view_range?: unknown; + }; + const start = Array.isArray(input.view_range) ? input.view_range[0] : 1; + return Number.isInteger(start) && start > 0 ? (start as number) : 1; + } catch { + return 1; + } +} + +function stripLegacyViewLineNumbers( + content: string, + firstLineNumber: number, +): string | undefined { + const stripped: string[] = []; + let expectedLineNumber = firstLineNumber; + + for (const line of content.split("\n")) { + const match = /^(\d+)\.(?: (.*))?$/.exec(line); + if (!match || Number(match[1]) !== expectedLineNumber) { + return undefined; + } + expectedLineNumber++; + stripped.push(match[2] ?? ""); + } + + return stripped.join("\n").trim(); +} + +function normalizeToolMessageContent(content: string): string | undefined { + // If it's a JSON tool call result, normalize the whitespace and property ordering. + // For successful tool results wrapped in {resultType, textResultForLlm}, unwrap to + // just the inner value so snapshots stay stable across envelope format changes. + try { + const parsed = JSON.parse(content); + if ( + parsed && + typeof parsed === "object" && + parsed.resultType === "success" && + "textResultForLlm" in parsed + ) { + return typeof parsed.textResultForLlm === "string" + ? parsed.textResultForLlm + : JSON.stringify(sortJsonKeys(parsed.textResultForLlm)); + } + return JSON.stringify(sortJsonKeys(parsed)); + } catch { + return content.trim() || undefined; + } +} + function expandWorkDir( content: string | undefined, workDir: string,