diff --git a/CHANGELOG.md b/CHANGELOG.md index 50c86469..e77743ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,18 @@ # @copilotkit/aimock +## [1.37.4] - 2026-07-20 + +### Fixed + +- `hasToolResult` record and replay are symmetric again: both the recorder (`buildFixtureMatch`) and the matcher (`matchFixtureDiagnostic`) now derive the value from one shared `currentTurnHasToolResult` helper. The recorder previously STAMPED the value with a whole-conversation `messages.some((m) => m.role === "tool")` while the matcher (as of 1.37.3) CHECKS it scoped to the current turn, so a genuinely-recorded multi-turn fixture (a fresh user turn whose history still carried an earlier turn's tool result) was stamped `true` but matched `false` and could never replay its own recorded request. Sharing one predicate makes the two sides structurally unable to drift (#319) +- Anthropic `tool_result` blocks bundled WITH accompanying text in a single user message now normalize with the text message BEFORE the tool message (`[..., user(text), tool]`), so a leg-2-with-text turn is correctly classified as carrying a tool result. Previously the tool message was emitted first (`[..., tool, user(text)]`), which is byte-identical to a fresh leg-1 turn and was misclassified `false`. The common `tool_result`-only follow-up (no text) is unaffected (#319) + +## [1.37.3] - 2026-07-20 + +### Changed + +- **Behavior change:** the `hasToolResult` match predicate is now scoped to the CURRENT turn (messages after the last `role: "user"` message) instead of the whole conversation. This lets a leg-1 (`false`) / leg-2 (`true`) fixture pair keep discriminating across multi-turn sessions, where the request on the 2nd+ user turn still carries earlier turns' tool results and a whole-conversation check would pin `hasToolResult` to `true` forever. Migration: any fixture that relied on `hasToolResult: true` meaning "a tool result appears anywhere in the conversation" (e.g. a multi-turn fixture that previously stuck on `true` once the first tool ran) will now only match on the turn that actually carries the tool result; a request with no `role: "user"` message still falls back to the whole-conversation scan (#316) + ## [1.37.2] - 2026-07-17 ### Fixed diff --git a/skills/write-fixtures/SKILL.md b/skills/write-fixtures/SKILL.md index c9c7e05a..262ef457 100644 --- a/skills/write-fixtures/SKILL.md +++ b/skills/write-fixtures/SKILL.md @@ -19,26 +19,26 @@ aimock is a zero-dependency mock infrastructure for AI apps. Fixture-driven. Mul ## Match Field Reference -| Field | Type | Matches Against | -| -------------------- | ----------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `userMessage` | `string` | Substring of last `role: "user"` message text | -| `userMessage` | `RegExp` | Pattern test on last `role: "user"` message text | -| `systemMessage` | `string` | Substring of the concatenated text of every `role: "system"` message in the request. Use to gate a fixture on host-supplied context (persona, agent-context entries) so changes to that context cause the fixture to fall through instead of returning a stale baked response | -| `systemMessage` | `string[]` | Array of substrings — ALL must be present in the joined system text (AND semantics). Use when the gate must combine multiple non-adjacent tokens whose serialisation order isn't stable | -| `systemMessage` | `RegExp` | Pattern test on the concatenated system-message text | -| `inputText` | `string` | Substring of embedding input text (concatenated if multiple inputs) | -| `inputText` | `RegExp` | Pattern test on embedding input text | -| `toolName` | `string` | Exact match on any tool in request's `tools[]` array (by `function.name`) | -| `toolCallId` | `string` | Exact match on `tool_call_id` of last `role: "tool"` message | -| `toolResultContains` | `string` | Substring of the last tool message's text content, gated on that message being the request's LAST message (same rule as `toolCallId`). Discriminates resume paths that share a `tool_call_id` and differ only inside the tool-result payload (e.g. approve `{"chosen_time": …}` vs cancel `{"cancelled": true}`) | -| `model` | `string` | Exact match on `req.model` | -| `model` | `RegExp` | Pattern test on `req.model` | -| `responseFormat` | `string` | Exact match on `req.response_format.type` (`"json_object"`, `"json_schema"`) | -| `sequenceIndex` | `number` | Matches only when this fixture's match count equals the given index (0-based) | -| `turnIndex` | `number` | Stateless conversation-depth matching. Counts `role: "assistant"` messages in the request; matches when that count equals the value. `turnIndex: 0` = first turn (no prior assistant messages). Use instead of `sequenceIndex` for shared/deployed instances where stateful counters break under concurrency | -| `hasToolResult` | `boolean` | Stateless tool-message presence matching. `true` matches when any `role: "tool"` message exists in the request; `false` matches when none exist. Provider-consistent across all aimock handlers (OpenAI, Claude, Gemini, Bedrock, Ollama, Cohere) | -| `endpoint` | `string` | Restrict to endpoint type: `"chat"`, `"image"`, `"speech"`, `"transcription"`, `"video"`, `"embedding"` | -| `predicate` | `(req: ChatCompletionRequest) => boolean` | Custom function — full access to request | +| Field | Type | Matches Against | +| -------------------- | ----------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `userMessage` | `string` | Substring of last `role: "user"` message text | +| `userMessage` | `RegExp` | Pattern test on last `role: "user"` message text | +| `systemMessage` | `string` | Substring of the concatenated text of every `role: "system"` message in the request. Use to gate a fixture on host-supplied context (persona, agent-context entries) so changes to that context cause the fixture to fall through instead of returning a stale baked response | +| `systemMessage` | `string[]` | Array of substrings — ALL must be present in the joined system text (AND semantics). Use when the gate must combine multiple non-adjacent tokens whose serialisation order isn't stable | +| `systemMessage` | `RegExp` | Pattern test on the concatenated system-message text | +| `inputText` | `string` | Substring of embedding input text (concatenated if multiple inputs) | +| `inputText` | `RegExp` | Pattern test on embedding input text | +| `toolName` | `string` | Exact match on any tool in request's `tools[]` array (by `function.name`) | +| `toolCallId` | `string` | Exact match on `tool_call_id` of last `role: "tool"` message | +| `toolResultContains` | `string` | Substring of the last tool message's text content, gated on that message being the request's LAST message (same rule as `toolCallId`). Discriminates resume paths that share a `tool_call_id` and differ only inside the tool-result payload (e.g. approve `{"chosen_time": …}` vs cancel `{"cancelled": true}`) | +| `model` | `string` | Exact match on `req.model` | +| `model` | `RegExp` | Pattern test on `req.model` | +| `responseFormat` | `string` | Exact match on `req.response_format.type` (`"json_object"`, `"json_schema"`) | +| `sequenceIndex` | `number` | Matches only when this fixture's match count equals the given index (0-based) | +| `turnIndex` | `number` | Stateless conversation-depth matching. Counts `role: "assistant"` messages in the request; matches when that count equals the value. `turnIndex: 0` = first turn (no prior assistant messages). Use instead of `sequenceIndex` for shared/deployed instances where stateful counters break under concurrency | +| `hasToolResult` | `boolean` | Stateless tool-message presence matching, scoped to the CURRENT turn (messages after the last `role: "user"` message). `true` matches when a `role: "tool"` message appears after the last user message; `false` matches when none does. (If the request has no user message, the whole conversation is scanned.) Provider-consistent across all aimock handlers (OpenAI, Claude, Gemini, Bedrock, Ollama, Cohere) | +| `endpoint` | `string` | Restrict to endpoint type: `"chat"`, `"image"`, `"speech"`, `"transcription"`, `"video"`, `"embedding"` | +| `predicate` | `(req: ChatCompletionRequest) => boolean` | Custom function — full access to request | **AND logic**: all specified fields must match. Empty match `{}` = catch-all. @@ -46,13 +46,13 @@ Multi-part content (e.g., `[{type: "text", text: "hello"}]`) is automatically ex ### When to Use Each Multi-turn Matching Approach -| Approach | Stateless? | Best For | -| -------------------- | ---------- | --------------------------------------------------------------------------------------------------------- | -| `turnIndex` | Yes | Shared/deployed instances; matches on conversation depth (count of assistant messages in request) | -| `hasToolResult` | Yes | Simplest option for 2-step tool flows — boolean: are there tool results in the request? | -| `sequenceIndex` | No | Single-client unit tests with repeated identical requests (server-side counter, breaks under concurrency) | -| `toolCallId` | Yes | Matching specific tool result IDs in the conversation history | -| `toolResultContains` | Yes | Same tool call id, different outcomes — match on the tool-result payload (approve vs cancel legs) | +| Approach | Stateless? | Best For | +| -------------------- | ---------- | ------------------------------------------------------------------------------------------------------------------------- | +| `turnIndex` | Yes | Shared/deployed instances; matches on conversation depth (count of assistant messages in request) | +| `hasToolResult` | Yes | Simplest option for 2-step tool flows — boolean: does the current turn (after the last user message) carry a tool result? | +| `sequenceIndex` | No | Single-client unit tests with repeated identical requests (server-side counter, breaks under concurrency) | +| `toolCallId` | Yes | Matching specific tool result IDs in the conversation history | +| `toolResultContains` | Yes | Same tool call id, different outcomes — match on the tool-result payload (approve vs cancel legs) | **Prefer stateless approaches** (`turnIndex`, `hasToolResult`, `toolResultContains`) for shared aimock instances (deployed via Docker, used by multiple test runners). Use `sequenceIndex` only in isolated single-client unit tests where the counter won't be corrupted by concurrent requests. @@ -495,7 +495,7 @@ These fields map correctly across all provider formats — for example, `finishR 4. **`streamingProfile` takes precedence over `latency`** — when both are set on a fixture, `streamingProfile` controls timing. Use one or the other. -5. **Tool result messages don't change the user message** — after a tool call, the client sends the same conversation + tool result. Matching on `userMessage` will hit the SAME fixture again → infinite loop. Always use `predicate` checking `role === "tool"` for tool results. +5. **Tool result messages don't change the user message** — after a tool call, the client sends the same conversation + tool result. Matching on `userMessage` will hit the SAME fixture again → infinite loop. Always use `predicate` checking `role === "tool"` for tool results. Note: a whole-conversation `role === "tool"` check (e.g. `req.messages.some((m) => m.role === "tool")`) diverges from `hasToolResult`'s current-turn scoping in multi-turn flows — the built-in `hasToolResult` matcher only looks after the last user message, so a later turn whose history carries an earlier tool result still reads `false`. 6. **`clearFixtures()` preserves the array reference** — uses `.length = 0`, not reassignment. The running server reads the same array object. diff --git a/src/__tests__/hastoolresult-record-replay-symmetry.test.ts b/src/__tests__/hastoolresult-record-replay-symmetry.test.ts new file mode 100644 index 00000000..46f9422b --- /dev/null +++ b/src/__tests__/hastoolresult-record-replay-symmetry.test.ts @@ -0,0 +1,197 @@ +import { describe, it, expect, afterEach } from "vitest"; +import * as http from "node:http"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { createServer, type ServerInstance } from "../server.js"; +import { loadFixturesFromDir } from "../fixture-loader.js"; + +// --------------------------------------------------------------------------- +// HTTP helper +// --------------------------------------------------------------------------- + +function post( + url: string, + body: unknown, + headers?: Record, +): Promise<{ status: number; headers: http.IncomingHttpHeaders; body: string }> { + return new Promise((resolve, reject) => { + const data = JSON.stringify(body); + const parsed = new URL(url); + const req = http.request( + { + hostname: parsed.hostname, + port: parsed.port, + path: parsed.pathname, + method: "POST", + headers: { + "Content-Type": "application/json", + "Content-Length": Buffer.byteLength(data), + ...headers, + }, + }, + (res) => { + const chunks: Buffer[] = []; + res.on("data", (c: Buffer) => chunks.push(c)); + res.on("end", () => + resolve({ + status: res.statusCode ?? 0, + headers: res.headers, + body: Buffer.concat(chunks).toString(), + }), + ); + }, + ); + req.on("error", reject); + req.write(data); + req.end(); + }); +} + +// --------------------------------------------------------------------------- +// Server lifecycle +// --------------------------------------------------------------------------- + +let upstream: ServerInstance | undefined; +let recorder: ServerInstance | undefined; +let replay: ServerInstance | undefined; +let tmpDir: string | undefined; + +afterEach(async () => { + for (const s of [replay, recorder, upstream]) { + if (s) await new Promise((resolve) => s.server.close(() => resolve())); + } + replay = recorder = upstream = undefined; + if (tmpDir) { + fs.rmSync(tmpDir, { recursive: true, force: true }); + tmpDir = undefined; + } +}); + +// --------------------------------------------------------------------------- +// Finding 1 — recorder/matcher record→replay symmetry (OpenAI shape) +// --------------------------------------------------------------------------- + +describe("hasToolResult record→replay symmetry", () => { + it("replays a genuinely-recorded turn-2 leg-1 request against its own recorded fixture", async () => { + // Turn-2 leg-1: a FRESH user question whose history still carries turn-1's + // completed tool result. Shape: [user, assistant(tool_call), tool, user]. + // Current-turn hasToolResult is FALSE (nothing after the last user message), + // but the whole conversation DOES contain a tool message. Pre-fix the + // recorder stamped `true` (whole-conversation) while the matcher checked + // `false` (turn-scoped) → the fixture could never match its own request. + const turn2Leg1 = { + model: "gpt-4", + messages: [ + { role: "user", content: "first question" }, + { + role: "assistant", + content: null, + tool_calls: [ + { + id: "call_1", + type: "function", + function: { name: "lookup", arguments: "{}" }, + }, + ], + }, + { role: "tool", content: "tool output", tool_call_id: "call_1" }, + { role: "user", content: "second question" }, + ], + }; + + // 1. Upstream mock returns an answer for the second question. + upstream = await createServer( + [ + { + match: { userMessage: "second question" }, + response: { content: "Answer to the second question." }, + }, + ], + { port: 0, logLevel: "silent" }, + ); + + // 2. Recorder proxies the turn-2 leg-1 request to upstream and records it. + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "aimock-sym-")); + recorder = await createServer([], { + port: 0, + logLevel: "silent", + record: { providers: { openai: upstream.url }, fixturePath: tmpDir }, + }); + const recResp = await post(`${recorder.url}/v1/chat/completions`, turn2Leg1, { + "x-test-id": "symmetry", + }); + expect(recResp.status).toBe(200); + + // 3. Replay the SAME request against the just-recorded fixtures. + const fixtures = loadFixturesFromDir(tmpDir); + expect(fixtures.length).toBeGreaterThan(0); + replay = await createServer(fixtures, { port: 0, logLevel: "silent", strict: true }); + const replayResp = await post(`${replay.url}/v1/chat/completions`, turn2Leg1); + + // Pre-fix: 503 "No fixture matched" (recorded true vs matched false). + // Post-fix: matches and serves the recorded answer. + expect(replayResp.status).toBe(200); + expect(replayResp.body).toContain("Answer to the second question."); + }); + + it("replays an Anthropic leg-2 turn that bundles tool_result WITH accompanying text", async () => { + // Anthropic packs a leg-2 tool_result and accompanying user text into ONE + // user message. Normalization must emit `[..., user(text), tool]` so the tool + // trails the last user message and the turn is classified as carrying a tool + // result (`true`). Pre-fix it emitted `[..., tool, user(text)]`, byte-identical + // to a fresh leg-1 turn, so the recorded fixture (whole-conversation `true`) + // could never match the turn-scoped `false` the matcher computed on replay. + const leg2WithText = { + model: "claude-3-5-sonnet-20241022", + max_tokens: 1024, + messages: [ + { role: "user", content: "initial question" }, + { + role: "assistant", + content: [{ type: "tool_use", id: "toolu_1", name: "lookup", input: {} }], + }, + { + role: "user", + content: [ + { type: "tool_result", tool_use_id: "toolu_1", content: "tool data" }, + { type: "text", text: "please summarize the tool result" }, + ], + }, + ], + }; + + // Upstream matches on the accompanying text (the last user message). + upstream = await createServer( + [ + { + match: { userMessage: "please summarize the tool result" }, + response: { content: "Recorded narration answer." }, + }, + ], + { port: 0, logLevel: "silent" }, + ); + + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "aimock-anthropic-")); + recorder = await createServer([], { + port: 0, + logLevel: "silent", + record: { providers: { anthropic: upstream.url }, fixturePath: tmpDir }, + }); + const recResp = await post(`${recorder.url}/v1/messages`, leg2WithText, { + "x-test-id": "anthropic-leg2-text", + }); + expect(recResp.status).toBe(200); + + const fixtures = loadFixturesFromDir(tmpDir); + expect(fixtures.length).toBeGreaterThan(0); + // The recorded fixture must carry the turn-scoped classification (true). + expect(fixtures.some((f) => f.match.hasToolResult === true)).toBe(true); + + replay = await createServer(fixtures, { port: 0, logLevel: "silent", strict: true }); + const replayResp = await post(`${replay.url}/v1/messages`, leg2WithText); + + expect(replayResp.status).toBe(200); + expect(replayResp.body).toContain("Recorded narration answer."); + }); +}); diff --git a/src/__tests__/messages.test.ts b/src/__tests__/messages.test.ts index d38bc192..63620f26 100644 --- a/src/__tests__/messages.test.ts +++ b/src/__tests__/messages.test.ts @@ -854,12 +854,16 @@ describe("claudeToCompletionRequest (fallback branches)", () => { }, ], }); - // Should produce tool message + user message + // Should produce user message (accompanying text) THEN tool message. The + // text is emitted FIRST so the tool message trails the last user message and + // the current-turn hasToolResult predicate classifies this leg-2 turn as + // carrying a tool result (`true`). Emitting the tool first would make this + // byte-identical to a genuine leg-1 turn and misclassify it `false`. expect(result.messages).toHaveLength(2); - expect(result.messages[0].role).toBe("tool"); - expect(result.messages[0].content).toBe("result data"); - expect(result.messages[1].role).toBe("user"); - expect(result.messages[1].content).toBe("follow up question"); + expect(result.messages[0].role).toBe("user"); + expect(result.messages[0].content).toBe("follow up question"); + expect(result.messages[1].role).toBe("tool"); + expect(result.messages[1].content).toBe("result data"); }); it("handles text content blocks with missing text (text ?? '')", () => { @@ -1086,8 +1090,12 @@ describe("claudeToCompletionRequest (fallback branches)", () => { }, ], }); - expect(result.messages[1].role).toBe("user"); - expect(result.messages[1].content).toBe(""); + // Accompanying text is emitted BEFORE the tool message (see the ordering + // rationale in claudeToCompletionRequest); a missing text field → "". + expect(result.messages[0].role).toBe("user"); + expect(result.messages[0].content).toBe(""); + expect(result.messages[1].role).toBe("tool"); + expect(result.messages[1].content).toBe("result"); }); it("handles system content blocks with text ?? '' in filter/map", () => { diff --git a/src/__tests__/router.test.ts b/src/__tests__/router.test.ts index e76c66e3..a1a7f749 100644 --- a/src/__tests__/router.test.ts +++ b/src/__tests__/router.test.ts @@ -5,6 +5,7 @@ import { getSystemText, getTextContent, getLastUserText, + currentTurnHasToolResult, } from "../router.js"; import type { ChatCompletionRequest, ChatMessage, ContentPart, Fixture } from "../types.js"; @@ -1292,6 +1293,88 @@ describe("matchFixture — hasToolResult", () => { expect(matchFixture([fixture], req)).toBe(fixture); }); + it("matches hasToolResult: false on a system-message-led new turn (scoping survives a leading system prompt)", () => { + // A leading system message must not shift the turn boundary: the CURRENT + // turn is still "after the last user message". Here that turn has no tool, + // so leg-1 (false) matches even though an earlier turn carried a tool result. + const fixture = makeFixture({ userMessage: "hello", hasToolResult: false }); + const req = makeReq({ + messages: [ + { role: "system", content: "you are helpful" }, + { role: "user", content: "hello" }, + { role: "assistant", content: "calling tool" }, + { role: "tool", content: "prior-turn tool output" }, + { role: "user", content: "hello" }, + ], + }); + expect(matchFixture([fixture], req)).toBe(fixture); + }); + + it("falls back to whole-conversation scan when there is no user message (system-led leg-2)", () => { + // No user message at all → lastUserIdx stays -1 and the scan covers the whole + // conversation (the safe, record/replay-symmetric fallback). A tool present + // anywhere therefore reads as hasToolResult: true. + const fixture = makeFixture({ hasToolResult: true }, { content: "ok" }); + const req = makeReq({ + messages: [ + { role: "system", content: "you are helpful" }, + { role: "assistant", content: "calling tool" }, + { role: "tool", content: "tool output" }, + ], + }); + expect(matchFixture([fixture], req)).toBe(fixture); + }); + + it("composes current-turn hasToolResult scoping with an explicit turnIndex disambiguator", () => { + // Both fixtures agree on the turn-scoped shape (leg-1, false) for a turn-2 + // request; turnIndex then disambiguates AMONG the shape matches. Verifies the + // shape predicate is evaluated correctly ALONGSIDE the position gate. + const turn0 = makeFixture( + { userMessage: "hello", hasToolResult: false, turnIndex: 0 }, + { content: "turn-0" }, + ); + const turn1 = makeFixture( + { userMessage: "hello", hasToolResult: false, turnIndex: 1 }, + { content: "turn-1" }, + ); + // Turn-2 leg-1: one assistant bubble → turnIndex 1; current turn has no tool. + const req = makeReq({ + messages: [ + { role: "user", content: "hello" }, + { role: "assistant", content: "calling tool" }, + { role: "tool", content: "prior-turn tool output" }, + { role: "user", content: "hello" }, + ], + }); + expect(matchFixture([turn0, turn1], req)).toBe(turn1); + }); + + it("composes current-turn hasToolResult scoping with a sequenceIndex gate", () => { + // A leg-2 request (current turn has a tool result). The sequenceIndex gate + // consumes the first sibling, so the second (sequenceIndex 1) is served — the + // hasToolResult shape predicate must pass for BOTH before the gate decides. + const seq0 = makeFixture( + { userMessage: "hello", hasToolResult: true, sequenceIndex: 0 }, + { content: "first" }, + ); + const seq1 = makeFixture( + { userMessage: "hello", hasToolResult: true, sequenceIndex: 1 }, + { content: "second" }, + ); + const counts = new Map([ + [seq0, 1], + [seq1, 1], + ]); + const req = makeReq({ + messages: [ + { role: "user", content: "hello" }, + { role: "assistant", content: "calling tool" }, + { role: "tool", content: "result" }, + ], + }); + expect(matchFixture([seq0, seq1], req, counts)).toBe(seq1); + }); + it("discriminates 2-step HITL flow with hasToolResult across turns", () => { const beforeTool = makeFixture( { userMessage: "hello", hasToolResult: false }, @@ -1333,6 +1416,61 @@ describe("matchFixture — hasToolResult", () => { }); }); +// --------------------------------------------------------------------------- +// currentTurnHasToolResult — the shared record/replay predicate +// --------------------------------------------------------------------------- + +describe("currentTurnHasToolResult", () => { + // This is the SINGLE predicate the recorder stamps and the matcher checks, so + // its behavior is what keeps record→replay symmetric. Each case pins a branch + // a plausible wrong implementation (whole-conversation scan, or a length-based + // default for the no-user case) would get wrong. + + it("is true when a tool result trails the last user message (leg-2)", () => { + expect( + currentTurnHasToolResult([ + { role: "user", content: "hello" }, + { role: "assistant", content: "calling tool" }, + { role: "tool", content: "output" }, + ]), + ).toBe(true); + }); + + it("is false on a new turn whose history carries an earlier turn's tool result (turn-scoped, not whole-conversation)", () => { + expect( + currentTurnHasToolResult([ + { role: "user", content: "hello" }, + { role: "assistant", content: "calling tool" }, + { role: "tool", content: "prior-turn output" }, + { role: "user", content: "next question" }, + ]), + ).toBe(false); + }); + + it("ignores a leading system message when locating the current turn", () => { + expect( + currentTurnHasToolResult([ + { role: "system", content: "you are helpful" }, + { role: "user", content: "hello" }, + { role: "assistant", content: "calling tool" }, + { role: "tool", content: "output" }, + ]), + ).toBe(true); + }); + + it("falls back to a whole-conversation scan when there is no user message", () => { + // lastUserIdx === -1 → slice(0) covers everything. A length-based default + // would slice past the end and wrongly return false. + expect( + currentTurnHasToolResult([ + { role: "system", content: "you are helpful" }, + { role: "assistant", content: "calling tool" }, + { role: "tool", content: "output" }, + ]), + ).toBe(true); + }); +}); + // --------------------------------------------------------------------------- // matchFixture — context matching // --------------------------------------------------------------------------- diff --git a/src/messages.ts b/src/messages.ts index b3fcf149..2adf1da5 100644 --- a/src/messages.ts +++ b/src/messages.ts @@ -168,6 +168,26 @@ export function claudeToCompletionRequest(req: ClaudeRequest): ChatCompletionReq const textBlocks = blocks.filter((b) => b.type === "text"); if (toolResults.length > 0) { + // Anthropic bundles a leg-2 tool_result and any accompanying user text + // into ONE user-role message. We emit the accompanying text FIRST, then + // the tool message(s), so the normalized shape is + // `[..., user(text), tool]` — the tool message trails the last user + // message and the current-turn `hasToolResult` predicate + // ({@link currentTurnHasToolResult}) correctly classifies it `true` + // (this IS a leg-2 turn that carries a tool result). Emitting the tool + // FIRST would produce `[..., tool, user(text)]`, which is byte-identical + // to a genuine turn-N leg-1 (a fresh user question after a completed + // tool turn) and would be misclassified `false`. The source-message + // grouping — that this text and this tool_result came from the SAME + // Anthropic message — is only available here, so this ordering is the + // only place the two cases can be told apart. The common + // tool_result-only follow-up (no text blocks) is unaffected. + if (textBlocks.length > 0) { + messages.push({ + role: "user", + content: textBlocks.map((b) => b.text ?? "").join(""), + }); + } // Each tool_result → tool message for (const tr of toolResults) { const resultContent = @@ -185,13 +205,6 @@ export function claudeToCompletionRequest(req: ClaudeRequest): ChatCompletionReq tool_call_id: tr.tool_use_id, }); } - // Any accompanying text blocks → user message - if (textBlocks.length > 0) { - messages.push({ - role: "user", - content: textBlocks.map((b) => b.text ?? "").join(""), - }); - } continue; } } diff --git a/src/recorder.ts b/src/recorder.ts index a4faaa1c..f98b0881 100644 --- a/src/recorder.ts +++ b/src/recorder.ts @@ -14,7 +14,7 @@ import type { RecordProviderKey, ToolCall, } from "./types.js"; -import { getLastMessageByRole, getTextContent } from "./router.js"; +import { getLastMessageByRole, getTextContent, currentTurnHasToolResult } from "./router.js"; import { normalizeModelName } from "./model-utils.js"; import type { Logger } from "./logger.js"; import { collapseStreamingResponse, capturedRedactedData } from "./stream-collapse.js"; @@ -2134,13 +2134,22 @@ export function buildFixtureMatch( // in-memory cache shadow follow-up turns that share it (initial tool call // vs. text reply after the tool result). turnIndex + hasToolResult give // each call a distinct, matcher-aware key. Skip for non-chat (no messages). + // + // hasToolResult MUST be stamped with the SAME current-turn predicate the + // matcher uses ({@link currentTurnHasToolResult}) — NOT a whole-conversation + // `messages.some(role === "tool")`. A whole-conversation stamp on a genuine + // turn-2 leg-1 request (a fresh user question whose history still carries an + // earlier turn's tool result) writes `true`, but the matcher computes the + // turn-scoped `false` for that same request, so the fixture could never match + // its own recorded request. Sharing the one helper is what keeps record and + // replay symmetric. const messages = request.messages ?? []; if ( messages.length > 0 && (request._endpointType === "chat" || request._endpointType === undefined) ) { match.turnIndex = messages.filter((m) => m.role === "assistant").length; - match.hasToolResult = messages.some((m) => m.role === "tool"); + match.hasToolResult = currentTurnHasToolResult(messages); } if (request._context) { diff --git a/src/router.ts b/src/router.ts index 984a99ac..b4ac8656 100644 --- a/src/router.ts +++ b/src/router.ts @@ -21,6 +21,48 @@ export function getLastMessageByRole(messages: ChatMessage[], role: string): Cha return null; } +/** + * `hasToolResult` request-SHAPE predicate — does the CURRENT turn contain a + * tool result message? "Current turn" = the messages AFTER the last `user` + * message, so this is scoped to the turn being matched rather than the whole + * conversation. This is what keeps leg-1 (tool call, `hasToolResult` false) vs + * leg-2 (narration, `hasToolResult` true) fixtures working across MULTI-TURN + * sessions: on the 2nd+ user turn the request still carries earlier turns' tool + * results, and a whole-conversation check would force `hasToolResult=true` + * forever, so the turn's leg-1 fixture (false) could never match again ("No + * fixture matched" on every pill after the first). + * + * This is the SINGLE source of truth shared by the RECORD side + * ({@link buildFixtureMatch} in recorder.ts, which STAMPS the value onto a new + * fixture) and the MATCH side ({@link matchFixtureDiagnostic} below, which + * CHECKS a fixture's stored value against the incoming request). The two MUST + * agree byte-for-byte: if the recorder stamps a value the matcher would never + * compute for the same request, a genuinely-recorded fixture can never match + * its own request. Keeping the predicate in one function is what guarantees + * they cannot drift. + * + * INVARIANT: callers MUST pass the POST-`requestTransform` (normalized) + * messages — the same array `buildFixtureMatch` and `matchFixtureDiagnostic` + * both feed here. Passing raw/untransformed messages on one side would let the + * set-side and check-side diverge again and silently reopen the multi-turn bug. + * + * When there is no `user` message at all, `lastUserIdx` stays -1 and the scan + * covers the whole conversation — the safe/consistent fallback for a user-less + * request (both sides degrade identically). For a single-turn request whose + * only tool message trails a lone user message this is identical to the old + * whole-conversation check. + */ +export function currentTurnHasToolResult(messages: ChatMessage[]): boolean { + let lastUserIdx = -1; + for (let i = messages.length - 1; i >= 0; i--) { + if (messages[i].role === "user") { + lastUserIdx = i; + break; + } + } + return messages.slice(lastUserIdx + 1).some((m) => m.role === "tool"); +} + /** * Concatenate the text content of every `system` role message in order. * Hosts that build a system context from multiple sources (persona, agent @@ -461,29 +503,15 @@ export function matchFixtureDiagnostic( } } - // hasToolResult — request-SHAPE predicate: does the CURRENT turn contain a - // tool result message? "Current turn" = messages after the last user - // message, so this is scoped to the turn being matched rather than the - // whole conversation. This is what makes leg-1 (tool call, hasToolResult - // false) vs leg-2 (narration, hasToolResult true) fixtures keep working - // across MULTI-TURN sessions: on the 2nd+ user turn the request still - // carries earlier turns' tool results, and a whole-conversation check would - // force hasToolResult=true forever, so the turn's leg-1 fixture (false) - // could never match again ("No fixture matched" on every pill after the - // first). For a single-turn request this is identical to the old - // whole-conversation check. Must be evaluated with the other shape - // predicates ABOVE the sequence/turn state gates so that a fixture whose - // shape never matched is not miscounted as "skipped by sequence/turn state". + // hasToolResult — request-SHAPE predicate scoped to the CURRENT turn. Shares + // the exact predicate with the recorder via {@link currentTurnHasToolResult} + // so the value the matcher CHECKS can never drift from the value the recorder + // STAMPED for the same request (see that helper's doc). Must be evaluated + // with the other shape predicates ABOVE the sequence/turn state gates so that + // a fixture whose shape never matched is not miscounted as "skipped by + // sequence/turn state". if (match.hasToolResult !== undefined) { - let lastUserIdx = -1; - for (let i = effective.messages.length - 1; i >= 0; i--) { - if (effective.messages[i].role === "user") { - lastUserIdx = i; - break; - } - } - const hasTool = effective.messages.slice(lastUserIdx + 1).some((m) => m.role === "tool"); - if (hasTool !== match.hasToolResult) continue; + if (currentTurnHasToolResult(effective.messages) !== match.hasToolResult) continue; } // At this point every SHAPE / CONTENT predicate above has passed, so this diff --git a/src/types.ts b/src/types.ts index 51301447..219ca0bf 100644 --- a/src/types.ts +++ b/src/types.ts @@ -107,6 +107,14 @@ export interface FixtureMatch { /** Which occurrence of this match to respond to (0-indexed). Undefined means match any. */ sequenceIndex?: number; turnIndex?: number; + /** + * Turn-scoped tool-result presence gate: matches on whether the CURRENT turn + * (the messages after the last `role: "user"` message) contains a `role: "tool"` + * message. Scoping to the current turn — rather than the whole conversation — + * lets a leg-1 (`false`) / leg-2 (`true`) fixture pair keep discriminating on + * later turns whose history still carries earlier turns' tool results. If the + * request has no user message, it falls back to scanning the whole conversation. + */ hasToolResult?: boolean; endpoint?: | "chat" @@ -559,6 +567,13 @@ export interface FixtureFileEntry { responseFormat?: string; sequenceIndex?: number; turnIndex?: number; + /** + * Turn-scoped tool-result presence gate: matches on whether the CURRENT turn + * (the messages after the last `role: "user"` message) contains a `role: "tool"` + * message, not the whole conversation. Falls back to scanning the whole + * conversation when the request has no user message. Mirrors the runtime + * `FixtureMatch.hasToolResult`. + */ hasToolResult?: boolean; endpoint?: | "chat"