Skip to content

Commit 922afee

Browse files
authored
fix: keep hasToolResult record/replay symmetric + Anthropic tool_result+text scoping (#319)
## Problem PR #316 (released as v1.37.3) scoped the replay-side `hasToolResult` check to the **current turn** (messages after the last `user` message) but left two paths inconsistent, so v1.37.3 can silently produce fixtures that never match their own recorded request. ## Root cause **1. Record/replay split-brain.** The matcher (`src/router.ts`) was changed to a turn-scoped check, but the recorder (`src/recorder.ts`, `buildFixtureMatch`) still STAMPED `hasToolResult` with a whole-conversation predicate `messages.some(m => m.role === "tool")`. Both operate on the same normalized `ChatCompletionRequest.messages`, so for a genuinely-recorded **turn-2 leg-1** request `[user, assistant, tool, user]` (a fresh question whose history still carries turn-1's tool result): - Recorder writes `hasToolResult: true` (whole conversation has a tool). - Matcher on replay computes `false` (nothing after the last user) → `true !== false` → the fixture can never match its own recorded request. The comment at `recorder.ts` even claimed the value was "matcher-aware" — it no longer was. **2. Anthropic `tool_result` + text ordering.** Anthropic bundles a leg-2 `tool_result` and accompanying user text into ONE user message. Normalization emitted `[..., tool, user(text)]`, which is **byte-identical** to a genuine leg-1 turn, so the turn-scoped check saw no tool after the last user and misclassified a real leg-2 turn as `false`. Because the two shapes are indistinguishable *after* normalization, this can only be fixed where the source-message grouping is still known — in normalization, not in the shared helper. ## Fix - Extracted one shared `currentTurnHasToolResult(messages)` helper in `src/router.ts` (where the other message-shape utilities live and which `recorder.ts` already imports) and used it on **both** the record side and the match side. Sharing a single predicate makes the two sides structurally incapable of drifting. Replaced the false "matcher-aware" comment. - Fixed `claudeToCompletionRequest` to emit the accompanying text user message **before** the tool message(s), so a real Anthropic leg-2-with-text turn normalizes to `[..., user(text), tool]` and is classified `true`, while a genuine leg-1 turn (separate messages) stays `false`. The common `tool_result`-only follow-up is unchanged. ## Red-green proof Both defects are proven on the **real HTTP record→replay surface** (spin up a mock upstream, record through the real recorder proxy to disk, replay the recorded fixtures through the real server/matcher). Source fixes were `git stash`-ed to observe RED, then restored for GREEN — tests unchanged between runs. ### RED (source fixes reverted) ``` FAIL hasToolResult record→replay symmetry > replays a genuinely-recorded turn-2 leg-1 request against its own recorded fixture AssertionError: expected 503 to be 200 // Object.is equality - Expected 200 + Received 503 FAIL hasToolResult record→replay symmetry > replays an Anthropic leg-2 turn that bundles tool_result WITH accompanying text AssertionError: expected 503 to be 200 // Object.is equality - Expected 200 + Received 503 FAIL claudeToCompletionRequest > handles tool_result with text blocks alongside in same user message AssertionError: expected 'tool' to be 'user' // Object.is equality Expected: "user" Received: "tool" ``` (503 = strict-mode "No fixture matched": the recorded fixture could not match its own request.) ### GREEN (source fixes restored, tests unchanged) ``` ✓ src/__tests__/hastoolresult-record-replay-symmetry.test.ts (2 tests) ✓ src/__tests__/router.test.ts (144 tests) ✓ src/__tests__/messages.test.ts (73 tests) ✓ src/__tests__/thinking-invariants.test.ts (47 tests) Test Files 4 passed (4) Tests 266 passed (266) ``` Both the OpenAI split-brain (Finding 1) and the Anthropic tool_result+text case (Finding 2) are covered by the real-path HTTP record→replay test, so both got a genuine RED→GREEN on the actual failure surface (not tests-against-fakes). ## Tests added - `src/__tests__/hastoolresult-record-replay-symmetry.test.ts` — full HTTP record→replay for (a) an OpenAI turn-2 leg-1 request and (b) an Anthropic `tool_result`+text leg-2 turn; both would 503 pre-fix. - `router.test.ts` — direct `currentTurnHasToolResult` coverage (leg-2 true; new-turn turn-scoped false; leading-system-message; `lastUserIdx === -1` no-user whole-conversation fallback), plus matcher composition of the scoping with explicit `turnIndex` and `sequenceIndex` gates, and a system-message-led new-turn case. - `messages.test.ts` — the two existing tool_result+text tests that encoded the old (buggy) tool-before-text order were corrected to the fixed order. ## Full suite `pnpm format:check`, `pnpm lint`, `pnpm build` all clean. `pnpm test`: 4683 passed, 1 pre-existing wall-clock flake (`timing-replay.test.ts > replaySpeed 2.0 halves the replay duration`, `expected 209 to be less than 200`) that passes in isolation and is unrelated to this change. ## Notes No version bump and no changeset (repo has no `.changeset/`; releases are cut by a separate release PR). This completes the incomplete fix from #316. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
2 parents 50bc447 + e192e5b commit 922afee

9 files changed

Lines changed: 487 additions & 66 deletions

File tree

CHANGELOG.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,18 @@
11
# @copilotkit/aimock
22

3+
## [1.37.4] - 2026-07-20
4+
5+
### Fixed
6+
7+
- `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)
8+
- 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)
9+
10+
## [1.37.3] - 2026-07-20
11+
12+
### Changed
13+
14+
- **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)
15+
316
## [1.37.2] - 2026-07-17
417

518
### Fixed

skills/write-fixtures/SKILL.md

Lines changed: 28 additions & 28 deletions
Large diffs are not rendered by default.
Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
import { describe, it, expect, afterEach } from "vitest";
2+
import * as http from "node:http";
3+
import * as fs from "node:fs";
4+
import * as os from "node:os";
5+
import * as path from "node:path";
6+
import { createServer, type ServerInstance } from "../server.js";
7+
import { loadFixturesFromDir } from "../fixture-loader.js";
8+
9+
// ---------------------------------------------------------------------------
10+
// HTTP helper
11+
// ---------------------------------------------------------------------------
12+
13+
function post(
14+
url: string,
15+
body: unknown,
16+
headers?: Record<string, string>,
17+
): Promise<{ status: number; headers: http.IncomingHttpHeaders; body: string }> {
18+
return new Promise((resolve, reject) => {
19+
const data = JSON.stringify(body);
20+
const parsed = new URL(url);
21+
const req = http.request(
22+
{
23+
hostname: parsed.hostname,
24+
port: parsed.port,
25+
path: parsed.pathname,
26+
method: "POST",
27+
headers: {
28+
"Content-Type": "application/json",
29+
"Content-Length": Buffer.byteLength(data),
30+
...headers,
31+
},
32+
},
33+
(res) => {
34+
const chunks: Buffer[] = [];
35+
res.on("data", (c: Buffer) => chunks.push(c));
36+
res.on("end", () =>
37+
resolve({
38+
status: res.statusCode ?? 0,
39+
headers: res.headers,
40+
body: Buffer.concat(chunks).toString(),
41+
}),
42+
);
43+
},
44+
);
45+
req.on("error", reject);
46+
req.write(data);
47+
req.end();
48+
});
49+
}
50+
51+
// ---------------------------------------------------------------------------
52+
// Server lifecycle
53+
// ---------------------------------------------------------------------------
54+
55+
let upstream: ServerInstance | undefined;
56+
let recorder: ServerInstance | undefined;
57+
let replay: ServerInstance | undefined;
58+
let tmpDir: string | undefined;
59+
60+
afterEach(async () => {
61+
for (const s of [replay, recorder, upstream]) {
62+
if (s) await new Promise<void>((resolve) => s.server.close(() => resolve()));
63+
}
64+
replay = recorder = upstream = undefined;
65+
if (tmpDir) {
66+
fs.rmSync(tmpDir, { recursive: true, force: true });
67+
tmpDir = undefined;
68+
}
69+
});
70+
71+
// ---------------------------------------------------------------------------
72+
// Finding 1 — recorder/matcher record→replay symmetry (OpenAI shape)
73+
// ---------------------------------------------------------------------------
74+
75+
describe("hasToolResult record→replay symmetry", () => {
76+
it("replays a genuinely-recorded turn-2 leg-1 request against its own recorded fixture", async () => {
77+
// Turn-2 leg-1: a FRESH user question whose history still carries turn-1's
78+
// completed tool result. Shape: [user, assistant(tool_call), tool, user].
79+
// Current-turn hasToolResult is FALSE (nothing after the last user message),
80+
// but the whole conversation DOES contain a tool message. Pre-fix the
81+
// recorder stamped `true` (whole-conversation) while the matcher checked
82+
// `false` (turn-scoped) → the fixture could never match its own request.
83+
const turn2Leg1 = {
84+
model: "gpt-4",
85+
messages: [
86+
{ role: "user", content: "first question" },
87+
{
88+
role: "assistant",
89+
content: null,
90+
tool_calls: [
91+
{
92+
id: "call_1",
93+
type: "function",
94+
function: { name: "lookup", arguments: "{}" },
95+
},
96+
],
97+
},
98+
{ role: "tool", content: "tool output", tool_call_id: "call_1" },
99+
{ role: "user", content: "second question" },
100+
],
101+
};
102+
103+
// 1. Upstream mock returns an answer for the second question.
104+
upstream = await createServer(
105+
[
106+
{
107+
match: { userMessage: "second question" },
108+
response: { content: "Answer to the second question." },
109+
},
110+
],
111+
{ port: 0, logLevel: "silent" },
112+
);
113+
114+
// 2. Recorder proxies the turn-2 leg-1 request to upstream and records it.
115+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "aimock-sym-"));
116+
recorder = await createServer([], {
117+
port: 0,
118+
logLevel: "silent",
119+
record: { providers: { openai: upstream.url }, fixturePath: tmpDir },
120+
});
121+
const recResp = await post(`${recorder.url}/v1/chat/completions`, turn2Leg1, {
122+
"x-test-id": "symmetry",
123+
});
124+
expect(recResp.status).toBe(200);
125+
126+
// 3. Replay the SAME request against the just-recorded fixtures.
127+
const fixtures = loadFixturesFromDir(tmpDir);
128+
expect(fixtures.length).toBeGreaterThan(0);
129+
replay = await createServer(fixtures, { port: 0, logLevel: "silent", strict: true });
130+
const replayResp = await post(`${replay.url}/v1/chat/completions`, turn2Leg1);
131+
132+
// Pre-fix: 503 "No fixture matched" (recorded true vs matched false).
133+
// Post-fix: matches and serves the recorded answer.
134+
expect(replayResp.status).toBe(200);
135+
expect(replayResp.body).toContain("Answer to the second question.");
136+
});
137+
138+
it("replays an Anthropic leg-2 turn that bundles tool_result WITH accompanying text", async () => {
139+
// Anthropic packs a leg-2 tool_result and accompanying user text into ONE
140+
// user message. Normalization must emit `[..., user(text), tool]` so the tool
141+
// trails the last user message and the turn is classified as carrying a tool
142+
// result (`true`). Pre-fix it emitted `[..., tool, user(text)]`, byte-identical
143+
// to a fresh leg-1 turn, so the recorded fixture (whole-conversation `true`)
144+
// could never match the turn-scoped `false` the matcher computed on replay.
145+
const leg2WithText = {
146+
model: "claude-3-5-sonnet-20241022",
147+
max_tokens: 1024,
148+
messages: [
149+
{ role: "user", content: "initial question" },
150+
{
151+
role: "assistant",
152+
content: [{ type: "tool_use", id: "toolu_1", name: "lookup", input: {} }],
153+
},
154+
{
155+
role: "user",
156+
content: [
157+
{ type: "tool_result", tool_use_id: "toolu_1", content: "tool data" },
158+
{ type: "text", text: "please summarize the tool result" },
159+
],
160+
},
161+
],
162+
};
163+
164+
// Upstream matches on the accompanying text (the last user message).
165+
upstream = await createServer(
166+
[
167+
{
168+
match: { userMessage: "please summarize the tool result" },
169+
response: { content: "Recorded narration answer." },
170+
},
171+
],
172+
{ port: 0, logLevel: "silent" },
173+
);
174+
175+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "aimock-anthropic-"));
176+
recorder = await createServer([], {
177+
port: 0,
178+
logLevel: "silent",
179+
record: { providers: { anthropic: upstream.url }, fixturePath: tmpDir },
180+
});
181+
const recResp = await post(`${recorder.url}/v1/messages`, leg2WithText, {
182+
"x-test-id": "anthropic-leg2-text",
183+
});
184+
expect(recResp.status).toBe(200);
185+
186+
const fixtures = loadFixturesFromDir(tmpDir);
187+
expect(fixtures.length).toBeGreaterThan(0);
188+
// The recorded fixture must carry the turn-scoped classification (true).
189+
expect(fixtures.some((f) => f.match.hasToolResult === true)).toBe(true);
190+
191+
replay = await createServer(fixtures, { port: 0, logLevel: "silent", strict: true });
192+
const replayResp = await post(`${replay.url}/v1/messages`, leg2WithText);
193+
194+
expect(replayResp.status).toBe(200);
195+
expect(replayResp.body).toContain("Recorded narration answer.");
196+
});
197+
});

src/__tests__/messages.test.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -854,12 +854,16 @@ describe("claudeToCompletionRequest (fallback branches)", () => {
854854
},
855855
],
856856
});
857-
// Should produce tool message + user message
857+
// Should produce user message (accompanying text) THEN tool message. The
858+
// text is emitted FIRST so the tool message trails the last user message and
859+
// the current-turn hasToolResult predicate classifies this leg-2 turn as
860+
// carrying a tool result (`true`). Emitting the tool first would make this
861+
// byte-identical to a genuine leg-1 turn and misclassify it `false`.
858862
expect(result.messages).toHaveLength(2);
859-
expect(result.messages[0].role).toBe("tool");
860-
expect(result.messages[0].content).toBe("result data");
861-
expect(result.messages[1].role).toBe("user");
862-
expect(result.messages[1].content).toBe("follow up question");
863+
expect(result.messages[0].role).toBe("user");
864+
expect(result.messages[0].content).toBe("follow up question");
865+
expect(result.messages[1].role).toBe("tool");
866+
expect(result.messages[1].content).toBe("result data");
863867
});
864868

865869
it("handles text content blocks with missing text (text ?? '')", () => {
@@ -1086,8 +1090,12 @@ describe("claudeToCompletionRequest (fallback branches)", () => {
10861090
},
10871091
],
10881092
});
1089-
expect(result.messages[1].role).toBe("user");
1090-
expect(result.messages[1].content).toBe("");
1093+
// Accompanying text is emitted BEFORE the tool message (see the ordering
1094+
// rationale in claudeToCompletionRequest); a missing text field → "".
1095+
expect(result.messages[0].role).toBe("user");
1096+
expect(result.messages[0].content).toBe("");
1097+
expect(result.messages[1].role).toBe("tool");
1098+
expect(result.messages[1].content).toBe("result");
10911099
});
10921100

10931101
it("handles system content blocks with text ?? '' in filter/map", () => {

0 commit comments

Comments
 (0)