From dd8425d0b3d1b44612b630549b829e0b199ecec6 Mon Sep 17 00:00:00 2001 From: Jack Champagne Date: Tue, 28 Jul 2026 21:50:56 -0400 Subject: [PATCH] test(http-recorder): match cassettes prompt-agnostically for transport tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three `session.llm native recorded` scenarios are the only failures on `local/amicode`, so the fork's `test` job is red and every PR into it inherits a red check — including #93, whose own suite is 3366 pass / 3 fail with zero failures of its own. Cause is ours. AMICODE-PATCHES.md ADDENDUM 2 consolidated the prompt so `system.ts provider()` returns `[providerBase(model), PROMPT_COMMUNICATING]`, appending `communicating.txt` for every model family. The cassettes still record the pre-patch prompt, so all three break on a diff of prose. Those tests assert TRANSPORT mechanics — that a tool loop is driven to a final text answer. The prompt is incidental, so matching on it couples them to text they do not test. Patching the recorded prompt instead would work until the next prompt edit, and that patch already has three addenda. `RecorderOptions.match` is an existing seam (types.ts:85, threaded through `http()` to `recordingLayer`) that nothing used. Adds `promptAgnosticMatcher` alongside `defaultMatcher` and passes it from this one test. The prompt arrives in three shapes, all covered: instructions OpenAI Responses system[] Anthropic Messages input[].role=="system" the OpenCode proxy System-message content is replaced with a placeholder rather than deleted, so "a system message is present, with content" stays part of the match. Verified narrow, in test/matching.test.ts: it ignores prompt prose in all three shapes, and still rejects a different user message, model, tool set, URL, or a missing-vs-present system message. 49 pass in http-recorder; the three recorded scenarios go 3 fail -> 3 pass; typecheck green across 23 packages. --- packages/http-recorder/src/internal.ts | 1 + packages/http-recorder/src/matching.ts | 59 +++++++++++++++++++ .../http-recorder/test-matcher-tmp.test.ts | 59 +++++++++++++++++++ packages/http-recorder/test/matching.test.ts | 59 +++++++++++++++++++ .../test/session/llm-native-recorded.test.ts | 16 ++++- 5 files changed, 193 insertions(+), 1 deletion(-) create mode 100644 packages/http-recorder/test-matcher-tmp.test.ts create mode 100644 packages/http-recorder/test/matching.test.ts diff --git a/packages/http-recorder/src/internal.ts b/packages/http-recorder/src/internal.ts index 7faecf0db0..ccdb507ed6 100644 --- a/packages/http-recorder/src/internal.ts +++ b/packages/http-recorder/src/internal.ts @@ -1,5 +1,6 @@ export { CassetteNotFoundError, hasCassetteSync, UnsafeCassetteError } from "./cassette.js" export { cassetteLayer, recordingLayer, type RecordReplayMode, type RecordReplayOptions } from "./internal-effect.js" +export { defaultMatcher, promptAgnosticMatcher } from "./matching.js" export { redactHeaders, redactUrl, secretFindings, type SecretFinding } from "./redaction.js" export { socketLayer } from "./socket.js" export { diff --git a/packages/http-recorder/src/matching.ts b/packages/http-recorder/src/matching.ts index 731aa8b57a..b805669f7c 100644 --- a/packages/http-recorder/src/matching.ts +++ b/packages/http-recorder/src/matching.ts @@ -36,6 +36,65 @@ export const canonicalSnapshot = (snapshot: RequestSnapshot): string => export const defaultMatcher: RequestMatcher = (incoming, recorded) => canonicalSnapshot(incoming) === canonicalSnapshot(recorded) +/** Placeholder standing in for elided system-prompt prose. Substituting rather than + * deleting keeps "a system message is present, with content" part of the match. */ +const SYSTEM_PROMPT_ELIDED = "" + +/** Top-level keys carrying the system prompt: OpenAI Responses uses `instructions`, + * Anthropic Messages uses `system`. */ +const SYSTEM_PROMPT_KEYS = ["instructions", "system"] as const + +/** Keys carrying a message list, where the prompt may instead arrive as a + * `role: "system"` entry (the shape the OpenCode proxy sends). */ +const MESSAGE_LIST_KEYS = ["input", "messages"] as const + +/** Neutralizes system-prompt prose wherever a provider puts it, leaving every other + * part of the request — model, tools, the conversation itself — matched exactly. */ +const withoutSystemPrompt = (snapshot: RequestSnapshot): RequestSnapshot => { + const body = jsonBody(snapshot.body) + if (!isRecord(body)) return snapshot + + const next: Record = { ...body } + let changed = false + + for (const key of SYSTEM_PROMPT_KEYS) { + if (!(key in next)) continue + delete next[key] + changed = true + } + + for (const key of MESSAGE_LIST_KEYS) { + const list = next[key] + if (!Array.isArray(list)) continue + let listChanged = false + const elided = list.map((entry) => { + if (!isRecord(entry) || entry["role"] !== "system" || !("content" in entry)) return entry + listChanged = true + return { ...entry, content: SYSTEM_PROMPT_ELIDED } + }) + if (!listChanged) continue + next[key] = elided + changed = true + } + + return changed ? { ...snapshot, body: JSON.stringify(next) } : snapshot +} + +/** + * Like {@link defaultMatcher}, but ignores the system prompt. + * + * A cassette records the exact request that produced its response, system prompt + * included — so any edit to a prompt invalidates every cassette that carries it, + * even when the behaviour under test has nothing to do with prompt text. Tests that + * assert on transport mechanics (tool loops, streaming, retries) should pin the + * mechanics and stay indifferent to wording. + * + * Use this when the prompt is incidental to what the test asserts. Do NOT use it for + * a test whose subject IS the prompt — there the exact text is the assertion. + */ +export const promptAgnosticMatcher: RequestMatcher = (incoming, recorded) => + canonicalSnapshot(withoutSystemPrompt(incoming)) === canonicalSnapshot(withoutSystemPrompt(recorded)) + export const safeText = (value: unknown) => { if (value === undefined) return "undefined" if (secretFindings(value).length > 0) return JSON.stringify(REDACTED) diff --git a/packages/http-recorder/test-matcher-tmp.test.ts b/packages/http-recorder/test-matcher-tmp.test.ts new file mode 100644 index 0000000000..7b46faa2ad --- /dev/null +++ b/packages/http-recorder/test-matcher-tmp.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, test } from "bun:test" +import { promptAgnosticMatcher, defaultMatcher } from "@opencode-ai/http-recorder/internal" + +const snap = (body: unknown) => ({ + method: "POST", + url: "https://api.example.com/v1/x", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), +}) + +describe("promptAgnosticMatcher", () => { + test("ignores instructions (OpenAI shape)", () => { + const a = snap({ model: "m", instructions: "OLD PROMPT", input: [] }) + const b = snap({ model: "m", instructions: "TOTALLY NEW PROMPT", input: [] }) + expect(defaultMatcher(a, b)).toBe(false) + expect(promptAgnosticMatcher(a, b)).toBe(true) + }) + + test("ignores system[] (Anthropic shape)", () => { + const a = snap({ model: "m", system: [{ type: "text", text: "OLD" }] }) + const b = snap({ model: "m", system: [{ type: "text", text: "NEW" }] }) + expect(promptAgnosticMatcher(a, b)).toBe(true) + }) + + test("ignores a role:system message (proxy shape)", () => { + const a = snap({ model: "m", input: [{ role: "system", content: "OLD" }, { role: "user", content: "hi" }] }) + const b = snap({ model: "m", input: [{ role: "system", content: "NEW" }, { role: "user", content: "hi" }] }) + expect(promptAgnosticMatcher(a, b)).toBe(true) + }) + + // The guard rails: everything that is NOT prompt prose must still differentiate. + test("still rejects a different user message", () => { + const a = snap({ model: "m", input: [{ role: "system", content: "P" }, { role: "user", content: "Paris" }] }) + const b = snap({ model: "m", input: [{ role: "system", content: "P" }, { role: "user", content: "Berlin" }] }) + expect(promptAgnosticMatcher(a, b)).toBe(false) + }) + + test("still rejects a different model", () => { + expect(promptAgnosticMatcher(snap({ model: "a", instructions: "P" }), snap({ model: "b", instructions: "P" }))).toBe(false) + }) + + test("still rejects different tools", () => { + const a = snap({ model: "m", instructions: "P", tools: [{ name: "get_weather" }] }) + const b = snap({ model: "m", instructions: "P", tools: [{ name: "rm_rf" }] }) + expect(promptAgnosticMatcher(a, b)).toBe(false) + }) + + test("still rejects a missing vs present system message", () => { + const a = snap({ model: "m", input: [{ role: "system", content: "P" }, { role: "user", content: "hi" }] }) + const b = snap({ model: "m", input: [{ role: "user", content: "hi" }] }) + expect(promptAgnosticMatcher(a, b)).toBe(false) + }) + + test("still rejects a different url", () => { + const a = { ...snap({ model: "m", instructions: "P" }), url: "https://api.example.com/v1/x" } + const b = { ...snap({ model: "m", instructions: "P" }), url: "https://evil.example.com/v1/x" } + expect(promptAgnosticMatcher(a, b)).toBe(false) + }) +}) diff --git a/packages/http-recorder/test/matching.test.ts b/packages/http-recorder/test/matching.test.ts new file mode 100644 index 0000000000..7b46faa2ad --- /dev/null +++ b/packages/http-recorder/test/matching.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, test } from "bun:test" +import { promptAgnosticMatcher, defaultMatcher } from "@opencode-ai/http-recorder/internal" + +const snap = (body: unknown) => ({ + method: "POST", + url: "https://api.example.com/v1/x", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), +}) + +describe("promptAgnosticMatcher", () => { + test("ignores instructions (OpenAI shape)", () => { + const a = snap({ model: "m", instructions: "OLD PROMPT", input: [] }) + const b = snap({ model: "m", instructions: "TOTALLY NEW PROMPT", input: [] }) + expect(defaultMatcher(a, b)).toBe(false) + expect(promptAgnosticMatcher(a, b)).toBe(true) + }) + + test("ignores system[] (Anthropic shape)", () => { + const a = snap({ model: "m", system: [{ type: "text", text: "OLD" }] }) + const b = snap({ model: "m", system: [{ type: "text", text: "NEW" }] }) + expect(promptAgnosticMatcher(a, b)).toBe(true) + }) + + test("ignores a role:system message (proxy shape)", () => { + const a = snap({ model: "m", input: [{ role: "system", content: "OLD" }, { role: "user", content: "hi" }] }) + const b = snap({ model: "m", input: [{ role: "system", content: "NEW" }, { role: "user", content: "hi" }] }) + expect(promptAgnosticMatcher(a, b)).toBe(true) + }) + + // The guard rails: everything that is NOT prompt prose must still differentiate. + test("still rejects a different user message", () => { + const a = snap({ model: "m", input: [{ role: "system", content: "P" }, { role: "user", content: "Paris" }] }) + const b = snap({ model: "m", input: [{ role: "system", content: "P" }, { role: "user", content: "Berlin" }] }) + expect(promptAgnosticMatcher(a, b)).toBe(false) + }) + + test("still rejects a different model", () => { + expect(promptAgnosticMatcher(snap({ model: "a", instructions: "P" }), snap({ model: "b", instructions: "P" }))).toBe(false) + }) + + test("still rejects different tools", () => { + const a = snap({ model: "m", instructions: "P", tools: [{ name: "get_weather" }] }) + const b = snap({ model: "m", instructions: "P", tools: [{ name: "rm_rf" }] }) + expect(promptAgnosticMatcher(a, b)).toBe(false) + }) + + test("still rejects a missing vs present system message", () => { + const a = snap({ model: "m", input: [{ role: "system", content: "P" }, { role: "user", content: "hi" }] }) + const b = snap({ model: "m", input: [{ role: "user", content: "hi" }] }) + expect(promptAgnosticMatcher(a, b)).toBe(false) + }) + + test("still rejects a different url", () => { + const a = { ...snap({ model: "m", instructions: "P" }), url: "https://api.example.com/v1/x" } + const b = { ...snap({ model: "m", instructions: "P" }), url: "https://evil.example.com/v1/x" } + expect(promptAgnosticMatcher(a, b)).toBe(false) + }) +}) diff --git a/packages/opencode/test/session/llm-native-recorded.test.ts b/packages/opencode/test/session/llm-native-recorded.test.ts index d17d7f8e5a..745c1fb137 100644 --- a/packages/opencode/test/session/llm-native-recorded.test.ts +++ b/packages/opencode/test/session/llm-native-recorded.test.ts @@ -289,7 +289,21 @@ function recordedNativeLLMLayer(scenario: RecordedScenario) { metadata, redactor: HttpRecorderInternal.Redactor.make(redact), }) - : HttpRecorder.http(scenario.cassette, { directory: FIXTURES_DIR, metadata, redact }) + : // These scenarios assert TRANSPORT mechanics — that a tool loop is driven to a final + // text answer — not prompt wording. The default matcher compares the whole request, + // so appending to the system prompt invalidates every cassette here even though the + // behaviour under test is unchanged. That is exactly what happened when + // `provider()` began appending `communicating.txt` for every model family: all three + // cassettes broke at once, on a diff of prose. + // + // Matching prompt-agnostically pins what these tests are actually about and leaves + // prompt evolution to the tests that assert on prompts. + HttpRecorder.http(scenario.cassette, { + directory: FIXTURES_DIR, + metadata, + redact, + match: HttpRecorderInternal.promptAgnosticMatcher, + }) const recordedClient = LLMClient.layer.pipe( Layer.provide(Layer.mergeAll(RequestExecutor.layer.pipe(Layer.provide(recordedHttp)), WebSocketExecutor.layer)), )