From 20cc935851417469cb6a830e907378247cd9b23e Mon Sep 17 00:00:00 2001 From: Christo Wilken Date: Sat, 12 Sep 2026 09:37:22 +0200 Subject: [PATCH 1/2] fix(oauth): keep the ChatGPT chain alive when the client echoes tool defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a tool call goes through Claude Code's permission path, the echoed call comes back with its zod defaults filled in: an Edit the model emitted without `replace_all` returns as `replace_all: false`. The head snapshot holds the model's raw arguments, so the strict-prefix comparison failed on every such call and the whole conversation was re-sent uncached (measured 2026-09-11 on one GPT-5.6 Luna session through the proxy: 77.8% → 92.1% cached input). This is the permission path, not a Claude Code version. `replace_all` carries the same `.default(false)` in 2.1.267 as in 2.1.268, and captured echoes from both binaries agree: bypass mode never fills the property, `acceptEdits` and `--allowedTools` fill it in both versions. `checkPermissions` writing `updatedInput` back to the transcript is what fills it. `toolSchemaDefaults(payload)` derives `{tool → {property → canonical default}}` from ONE request's `tools` array, namespaced groups included, and `normalizeToolCallJson` drops from BOTH sides of the comparison any `arguments` property whose value equals its declared default. Compare-only: outgoing payloads are untouched. A value that differs from the default, or a property with no declared default, still diverges as before. The map is per-request and pure, for the same reason `headRequiredToolProps` snapshots `required` from the head's own turn. A process-global map keyed only by tool name is last-writer-wins across every client, partition and session one `clodex server` handles, and `entry.canonicalPrefix` caches the head side permanently under whatever the map held when it was first built — so another client's schema can flip the verdict in either direction: under-stripping loses a chain that should have continued, over-stripping continues on a history that genuinely changed. The two prefix memos and the in-flight `canonicalInput` memo are therefore keyed on a fingerprint of the map they were built under and recomputed when it changes; within one client that fingerprint is constant, so the memos still do their job. Trade-off: a request whose `tools` omit a tool appearing in its own history gets no stripping for that tool, which is the pre-fix behaviour. Subagent histories never contain the parent's calls, so the residual is narrow. Tests stage heads through `response.output_item.done`: the continuation with an echoed default, a new chain on a non-default value, a new chain when the schema declares no default, a namespaced tool group, and a three-request two-client regression with no reset between clients — one client's schema, a scan that caches the victim's prefix under it, and the victim's next turn. Each reds under the process-global behaviour or without its own fix. Session: a5ae056f-79a5-4beb-831d-c16d7b2d37dc --- .claude/docs/oauth-continuation.md | 35 ++++ src/oauth/responses-websocket.ts | 155 ++++++++++++++-- tests/responses-websocket.test.ts | 289 +++++++++++++++++++++++++++++ 3 files changed, 466 insertions(+), 13 deletions(-) diff --git a/.claude/docs/oauth-continuation.md b/.claude/docs/oauth-continuation.md index 5d65b120..4dbffc9d 100644 --- a/.claude/docs/oauth-continuation.md +++ b/.claude/docs/oauth-continuation.md @@ -552,6 +552,41 @@ transport-failure replay, although an OAuth 401 refresh can still start a new au **This policy can recover only while no model output has been exposed downstream**; replay after partial output could duplicate content or tool calls. +### Schema defaults are filler on both sides + +When a tool call goes through Claude Code's permission path, the echoed call comes back with its zod +defaults filled in: an `Edit` the model emitted without `replace_all` returns as `replace_all: false`. +The head snapshot holds the model's raw arguments, so the strict-prefix comparison failed on every +such call and re-sent the whole conversation (measured 2026-09-11 on one Luna session: 2 of 14 turns, +~88k tokens each; 77.8% → 92.1% cached input with the fix). + +This is not a version change. `replace_all` carries the same `.default(false)` in 2.1.267 as in +2.1.268, and captured echoes from both binaries agree: under `--dangerously-skip-permissions` the +property is never filled, while `acceptEdits` and `--allowedTools` fill it in both versions. What +fills it is `checkPermissions` writing `updatedInput` back to the transcript, so the trigger is the +permission path, not a release. + +`toolSchemaDefaults(payload)` derives `{tool → {property → canonical default}}` from ONE request's +`tools` array — namespaced tool groups included — and `normalizeToolCallJson` drops, from BOTH sides, +any `arguments` property whose value equals its declared default. Compare-only: the outgoing payload +is untouched. A value that differs from the default (`replace_all: true`) and a property with no +declared default still diverge, as before. + +The map is per-request and pure, for the same reason `headRequiredToolProps` snapshots `required` +from the head's own turn. A process-global map keyed only by tool name is last-writer-wins across +every client, partition and session one `clodex server` handles, and `entry.canonicalPrefix` caches +the head side permanently under whatever the map held when it was first built — so another client's +schema can flip the verdict in either direction: under-stripping costs a chain that should have +continued (the everyday parent-then-subagent sequence), over-stripping continues on a history that +genuinely changed. Both were reproduced through the real WebSocket transport. The two prefix memos +and the in-flight `canonicalInput` memo are therefore keyed on a fingerprint of the defaults map they +were built under, and recomputed when it changes; within one client that fingerprint is constant, so +the memos still do their job. + +Trade-off: a request whose `tools` omit a tool that appears in its own history gets no stripping for +that tool, which is the pre-fix behaviour. Subagent histories never contain the parent's calls, so +the residual is narrow. + ### Mismatch diagnostics On a history mismatch the head-decision log includes `expected_hash`/`actual_hash` (SHA-256 of each diff --git a/src/oauth/responses-websocket.ts b/src/oauth/responses-websocket.ts index 398a0d70..23cbec76 100644 --- a/src/oauth/responses-websocket.ts +++ b/src/oauth/responses-websocket.ts @@ -155,9 +155,13 @@ interface RequestContext { * assigned once at construction and never reassigned — a transport retry resets * `sendPayload` back to it and reuses this same context — so the memo cannot go * stale, and it keeps a wide fan-out from re-serializing every in-flight - * conversation once per arriving sibling. + * conversation once per arriving sibling. It IS invalidated when the arriving + * request's tool-schema defaults differ from the ones it was built under: the + * payload is fixed, but the normalization applied to it is not. */ canonicalInput?: string[]; + /** Fingerprint of the tool-schema defaults `canonicalInput` was built under. */ + canonicalInputToolDefaultsId?: string; sendPayload: JsonObject; promptFieldHashes: Record; instructionsSnapshot?: string; @@ -211,6 +215,13 @@ interface ConnectionEntry { /** Memoized canonical form of the stored prefix; cleared whenever it changes. */ canonicalPrefix?: string[]; canonicalEchoablePrefix?: string[]; + /** + * Fingerprint of the tool-schema defaults the two memos above were built under. + * A head is reused across requests, and a request's defaults come from its own + * `tools`, so bytes canonicalized under a different map must be discarded rather + * than compared — otherwise one client's schema decides another client's verdict. + */ + canonicalToolDefaultsId?: string; options: Required>; debug: (message: string) => void; } @@ -445,12 +456,96 @@ function inputArray(payload: JsonObject): unknown[] { return Array.isArray(payload.input) ? payload.input : []; } -function normalizeToolCallJson(value: unknown): unknown { - if (Array.isArray(value)) return value.map(normalizeToolCallJson); +/** + * Schema defaults per tool, derived from ONE request's `tools` array. + * + * When a tool call goes through Claude Code's permission path, the echoed call + * comes back with its zod defaults filled in (an `Edit` the model emitted + * without `replace_all` returns as `replace_all: false`), while the head + * snapshot holds the model's raw arguments without them. The strict-prefix + * comparison then fails on every such call and the whole conversation is + * re-sent uncached. Compare-only: a property whose value equals the declared + * default is dropped from BOTH sides before hashing; outgoing payloads are + * untouched. + * + * Deliberately per-request and pure, for the same reason `headRequiredToolProps` + * snapshots `required` from the head's own turn: a process-global map keyed only + * by tool name is last-writer-wins across every client, partition and session a + * `clodex server` handles, and reading another client's schema can flip the + * verdict in either direction with no code change — under-stripping (a cached + * head keeps a property the client strips, so an ordinary parent-then-subagent + * sequence loses its chain) or over-stripping (a genuine history change compares + * equal). Both were reproduced through the real WebSocket transport. + */ +type ToolSchemaDefaults = Map>; + +export function toolSchemaDefaults(payload: JsonObject): ToolSchemaDefaults { + const defaults: ToolSchemaDefaults = new Map(); + const add = (tool: unknown): void => { + if (!tool || typeof tool !== 'object') return; + const record = tool as JsonObject; + if (record.type === 'namespace' && Array.isArray(record.tools)) { + for (const nested of record.tools) add(nested); + return; + } + if (record.type !== 'function' || typeof record.name !== 'string') return; + const parameters = record.parameters; + const properties = parameters && typeof parameters === 'object' + ? (parameters as JsonObject).properties : undefined; + if (!properties || typeof properties !== 'object') return; + const perTool = new Map(); + for (const [prop, schema] of Object.entries(properties as JsonObject)) { + if (schema && typeof schema === 'object' && 'default' in (schema as JsonObject)) { + perTool.set(prop, canonicalJson((schema as JsonObject).default)); + } + } + if (perTool.size) defaults.set(record.name, perTool); + }; + if (Array.isArray(payload.tools)) for (const tool of payload.tools) add(tool); + return defaults; +} + +/** + * Identity of a defaults map, for cache keys. + * + * `entry.canonicalPrefix` is memoized across requests, so the head side must not + * keep bytes that were normalized under a different map than the client side is + * being normalized under right now. Within one client the map is constant and the + * memo still holds; when it changes, the fingerprint changes and the prefix is + * recomputed. + */ +function toolSchemaDefaultsFingerprint(defaults: ToolSchemaDefaults): string { + if (!defaults.size) return 'none'; + const parts: string[] = []; + for (const name of [...defaults.keys()].sort()) { + const perTool = defaults.get(name)!; + const props = [...perTool.entries()].sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)) + .map(([prop, value]) => `${prop}=${value}`).join(','); + parts.push(`${name}:${props}`); + } + return createHash('sha256').update(parts.join(';')).digest('hex').slice(0, 16); +} + +function stripSchemaDefaults(name: unknown, args: unknown, defaults: ToolSchemaDefaults | undefined): unknown { + if (!defaults) return args; + if (typeof name !== 'string' || !args || typeof args !== 'object' || Array.isArray(args)) return args; + const perTool = defaults.get(name); + if (!perTool) return args; + const out: JsonObject = {}; + for (const [key, value] of Object.entries(args as JsonObject)) { + const expected = perTool.get(key); + if (expected !== undefined && canonicalJson(value) === expected) continue; + out[key] = value; + } + return out; +} + +function normalizeToolCallJson(value: unknown, defaults?: ToolSchemaDefaults): unknown { + if (Array.isArray(value)) return value.map(item => normalizeToolCallJson(item, defaults)); if (!value || typeof value !== 'object') return value; const record = value as JsonObject; const out: JsonObject = {}; - for (const [key, child] of Object.entries(record)) out[key] = normalizeToolCallJson(child); + for (const [key, child] of Object.entries(record)) out[key] = normalizeToolCallJson(child, defaults); // Claude parses tool_use input into an object. The OpenAI SDK later serializes // it again, so insignificant whitespace and object-key order can differ from @@ -461,7 +556,10 @@ function normalizeToolCallJson(value: unknown): unknown { : record.type === 'custom_tool_call' ? 'input' : undefined; if (jsonField && typeof record[jsonField] === 'string') { try { - out[jsonField] = canonicalJson(JSON.parse(record[jsonField] as string)); + const parsed = JSON.parse(record[jsonField] as string); + out[jsonField] = canonicalJson( + jsonField === 'arguments' ? stripSchemaDefaults(record.name, parsed, defaults) : parsed, + ); } catch { // A malformed/non-JSON custom-tool input must still match byte-for-byte. } @@ -874,8 +972,8 @@ function mismatchDumpLine(items: unknown[], index: number): string { * meaning to comparing whole arrays, but it lets both sides be computed once * instead of re-serializing an entire conversation for every candidate head. */ -function canonicalItemStrings(items: unknown[]): string[] { - return items.map(item => canonicalJson(normalizeToolCallJson([item]))); +function canonicalItemStrings(items: unknown[], defaults?: ToolSchemaDefaults): string[] { + return items.map(item => canonicalJson(normalizeToolCallJson([item], defaults))); } /** @@ -899,12 +997,24 @@ function continuationMatch( entry: ConnectionEntry, payload: JsonObject, clientItems: string[], + defaults: ToolSchemaDefaults, + defaultsId: string, ): ContinuationMatch | undefined { if (!entry.responseId || !entry.requestInput || !entry.expectedAssistant) return undefined; const full = inputArray(payload); + // Both sides of every comparison have to be canonicalized under the SAME + // defaults map. The memo is keyed on that map's fingerprint, so a head cached + // under one client's schemas is recomputed rather than compared across. + if (entry.canonicalToolDefaultsId !== defaultsId) { + entry.canonicalPrefix = undefined; + entry.canonicalEchoablePrefix = undefined; + entry.canonicalToolDefaultsId = defaultsId; + } // The stored prefix only changes when a response completes, so canonicalize it // once per head rather than once per lookup. - entry.canonicalPrefix ??= canonicalItemStrings([...entry.requestInput, ...entry.expectedAssistant]); + entry.canonicalPrefix ??= canonicalItemStrings( + [...entry.requestInput, ...entry.expectedAssistant], defaults, + ); if (isStrictPrefix(entry.canonicalPrefix, clientItems)) { return { delta: full.slice(entry.canonicalPrefix.length), mode: 'exact' }; } @@ -916,7 +1026,7 @@ function continuationMatch( // remaining response items still match exactly. const echoedAssistant = entry.expectedAssistant.filter(item => conversationItemKind(item) !== 'reasoning'); if (echoedAssistant.length === entry.expectedAssistant.length) return undefined; - entry.canonicalEchoablePrefix ??= canonicalItemStrings([...entry.requestInput, ...echoedAssistant]); + entry.canonicalEchoablePrefix ??= canonicalItemStrings([...entry.requestInput, ...echoedAssistant], defaults); if (!isStrictPrefix(entry.canonicalEchoablePrefix, clientItems)) return undefined; return { delta: full.slice(entry.canonicalEchoablePrefix.length), mode: 'omitted_reasoning' }; } @@ -2196,6 +2306,10 @@ export function createResponsesWebSocketFetch( authorizationFingerprint, claudeAgentId, ); + // Per-request, never shared: see toolSchemaDefaults' comment for what a + // process-global map does to a server with more than one client. + const requestToolDefaults = toolSchemaDefaults(payload); + const requestToolDefaultsId = toolSchemaDefaultsFingerprint(requestToolDefaults); const promptFingerprint = responsesWebSocketPromptFingerprint(payload); const promptFieldHashes = responsesWebSocketPromptFieldHashes(payload); const instructionsSnapshot = instructionsFromPayload(payload); @@ -2208,7 +2322,9 @@ export function createResponsesWebSocketFetch( // scans and the in-flight lineage test below need it, and none of them needs // it when the partition holds nothing to compare against. let canonicalClientItems: string[] | undefined; - const clientItems = (): string[] => (canonicalClientItems ??= canonicalItemStrings(inputArray(payload))); + const clientItems = (): string[] => ( + canonicalClientItems ??= canonicalItemStrings(inputArray(payload), requestToolDefaults) + ); // Hoisted verbatim so the SAME scan can run a second time after a pacing // wait: same expressions, same ordering, same tie-breaks. Nothing here is @@ -2226,7 +2342,10 @@ export function createResponsesWebSocketFetch( candidates: scanned, idleCandidates: idle, matches: idle - .map(entry => ({ entry, match: continuationMatch(entry, payload, canonical) })) + .map(entry => ({ + entry, + match: continuationMatch(entry, payload, canonical, requestToolDefaults, requestToolDefaultsId), + })) .filter((candidate): candidate is { entry: ConnectionEntry; match: ContinuationMatch } => candidate.match !== undefined) // Prefer the longest matching history, which produces the smallest delta. .sort((left, right) => left.match.delta.length - right.match.delta.length @@ -2251,7 +2370,9 @@ export function createResponsesWebSocketFetch( */ const couldPrecedeThisRequest = (entry: ConnectionEntry): boolean => { if (entry.responseId && entry.requestInput && entry.expectedAssistant) { - return continuationMatch(entry, payload, clientItems()) !== undefined; + return continuationMatch( + entry, payload, clientItems(), requestToolDefaults, requestToolDefaultsId, + ) !== undefined; } const streaming = entry.current; // `inFlight` and `current` are set together, so a candidate this predicate is @@ -2261,7 +2382,15 @@ export function createResponsesWebSocketFetch( // continuation check uses: a client that re-sent the same turn is a duplicate // of the response in flight, not a branch off it, and must not be stitched // onto a turn whose output it has never seen. - streaming.canonicalInput ??= canonicalItemStrings(inputArray(streaming.originalPayload)); + // Same snapshot rule as the head caches below: bytes canonicalized under a + // different defaults map cannot be compared against this request's client side. + if (streaming.canonicalInputToolDefaultsId !== requestToolDefaultsId) { + streaming.canonicalInput = undefined; + streaming.canonicalInputToolDefaultsId = requestToolDefaultsId; + } + streaming.canonicalInput ??= canonicalItemStrings( + inputArray(streaming.originalPayload), requestToolDefaults, + ); return isPrefixOrEqual(streaming.canonicalInput, clientItems()); }; /** The in-flight head that forced isolation, for the diagnostic. */ diff --git a/tests/responses-websocket.test.ts b/tests/responses-websocket.test.ts index a091538e..b917f690 100644 --- a/tests/responses-websocket.test.ts +++ b/tests/responses-websocket.test.ts @@ -54,6 +54,11 @@ function lastSocket(): FakeWebSocket { return fakeSockets[fakeSockets.length - 1]!; } +/** How many sockets have been created — for asserting that a turn reused one. */ +function socketCount(): number { + return fakeSockets.length; +} + const sessionPayload = (input: unknown[], extra: Record = {}) => ({ model: 'gpt-5.6-sol', prompt_cache_key: 'relay-session-abc', @@ -1992,6 +1997,290 @@ describe('createResponsesWebSocketFetch', () => { await readAll(second); }); + it('continues when the client echoed a schema default the model never sent', async () => { + // A tool call that went through Claude Code's permission path comes back with + // its zod defaults filled in: an Edit the model emitted without `replace_all` + // returns as `replace_all: false` (measured on 2.1.267 and 2.1.268 alike — + // bypass mode never fills, `acceptEdits` always does). The schema in the + // request declares that default, so the property is dropped from both sides. + const tools = [{ + type: 'function', name: 'Edit', + parameters: { + type: 'object', + properties: { + file_path: { type: 'string' }, + old_string: { type: 'string' }, + new_string: { type: 'string' }, + replace_all: { type: 'boolean', default: false }, + }, + required: ['file_path', 'old_string', 'new_string'], + }, + }]; + const input = [{ role: 'user', content: [{ type: 'input_text', text: 'fix it' }] }]; + const wsFetch = createResponsesWebSocketFetch(WS_URL, undefined, { accountId: 'acct-schema-default' }); + const first = await wsFetch('https://x', { + method: 'POST', headers: {}, body: JSON.stringify(sessionPayload(input, { tools })), + }); + const socket = lastSocket(); + socket.emit('open'); + socket.emit('message', Buffer.from(JSON.stringify({ type: 'response.created', response: { id: 'resp_edit' } }))); + socket.emit('message', Buffer.from(JSON.stringify({ + type: 'response.output_item.done', output_index: 0, + item: { + type: 'function_call', call_id: 'call_e', name: 'Edit', + arguments: '{"file_path":"a.py","old_string":"x","new_string":"y"}', + }, + }))); + socket.emit('message', Buffer.from(JSON.stringify({ type: 'response.completed', response: { id: 'resp_edit' } }))); + await readAll(first); + + const echoedCall = { + type: 'function_call', call_id: 'call_e', name: 'Edit', + arguments: '{"file_path":"a.py","old_string":"x","new_string":"y","replace_all":false}', + }; + const toolOutput = { type: 'function_call_output', call_id: 'call_e', output: 'edited' }; + const second = await wsFetch('https://x', { + method: 'POST', headers: {}, body: JSON.stringify(sessionPayload([...input, echoedCall, toolOutput], { tools })), + }); + expect(lastSocket()).toBe(socket); + const sent = JSON.parse(socket.send.mock.calls[1]![0] as string); + expect(sent.previous_response_id).toBe('resp_edit'); + expect(sent.input).toEqual([toolOutput]); + emitTextResponse(socket, 'resp_edit_done', 'done'); + await readAll(second); + }); + + it('finds a schema default inside a namespaced tool group', async () => { + // The `tools` array can nest function declarations inside a namespace entry, + // and the walk recurses into those. This is the case that recursion is for. + const tools = [{ + type: 'namespace', + name: 'file_ops', + tools: [{ + type: 'function', name: 'Edit', + parameters: { + type: 'object', + properties: { + file_path: { type: 'string' }, + replace_all: { type: 'boolean', default: false }, + }, + required: ['file_path'], + }, + }], + }]; + const input = [{ role: 'user', content: [{ type: 'input_text', text: 'fix it' }] }]; + const wsFetch = createResponsesWebSocketFetch(WS_URL, undefined, { accountId: 'acct-schema-namespace' }); + const first = await wsFetch('https://x', { + method: 'POST', headers: {}, body: JSON.stringify(sessionPayload(input, { tools })), + }); + const socket = lastSocket(); + socket.emit('open'); + socket.emit('message', Buffer.from(JSON.stringify({ type: 'response.created', response: { id: 'resp_ns' } }))); + socket.emit('message', Buffer.from(JSON.stringify({ + type: 'response.output_item.done', output_index: 0, + item: { type: 'function_call', call_id: 'call_ns', name: 'Edit', arguments: '{"file_path":"a.py"}' }, + }))); + socket.emit('message', Buffer.from(JSON.stringify({ type: 'response.completed', response: { id: 'resp_ns' } }))); + await readAll(first); + + const echoedCall = { + type: 'function_call', call_id: 'call_ns', name: 'Edit', + arguments: '{"file_path":"a.py","replace_all":false}', + }; + const toolOutput = { type: 'function_call_output', call_id: 'call_ns', output: 'edited' }; + const second = await wsFetch('https://x', { + method: 'POST', headers: {}, body: JSON.stringify(sessionPayload([...input, echoedCall, toolOutput], { tools })), + }); + expect(lastSocket()).toBe(socket); + const sent = JSON.parse(socket.send.mock.calls[1]![0] as string); + expect(sent.previous_response_id).toBe('resp_ns'); + expect(sent.input).toEqual([toolOutput]); + emitTextResponse(socket, 'resp_ns_done', 'done'); + await readAll(second); + }); + + it('does not let one client\'s tool schema decide another client\'s continuation', async () => { + // The defaults map used to be process-global and keyed only by tool name, so + // whichever client declared `Edit` last decided how every other client's + // history was normalized — and `entry.canonicalPrefix` caches the head side + // permanently under whatever the map held at the instant it was first built. + // Three requests, one process, NO reset between them (the reset in beforeEach + // is what hid this): + // 1. client A establishes a head whose call carries replace_all EXPLICITLY, + // under a schema declaring default false — so both sides strip it; + // 2. client B runs a turn declaring the same tool with default TRUE; + // 3. a second request in A's own partition whose tools do NOT include Edit + // (a title generation, a subagent) scans A's idle head and caches its + // canonical prefix — under B's schema, with a shared map; + // 4. A's next real turn then strips its echo client-side while the cached + // head keeps it, so A loses a chain it should have kept. + const toolsWithDefault = (value: boolean) => [{ + type: 'function', name: 'Edit', + parameters: { + type: 'object', + properties: { + file_path: { type: 'string' }, + replace_all: { type: 'boolean', default: value }, + }, + required: ['file_path'], + }, + }]; + const input = [{ role: 'user', content: [{ type: 'input_text', text: 'fix it' }] }]; + const explicitArgs = '{"file_path":"a.py","replace_all":false}'; + + // 1. Client A's head. + const clientA = createResponsesWebSocketFetch(WS_URL, undefined, { accountId: 'acct-two-client-a' }); + const firstA = await clientA('https://x', { + method: 'POST', headers: {}, body: JSON.stringify(sessionPayload(input, { tools: toolsWithDefault(false) })), + }); + const socketA = lastSocket(); + socketA.emit('open'); + socketA.emit('message', Buffer.from(JSON.stringify({ type: 'response.created', response: { id: 'resp_a' } }))); + socketA.emit('message', Buffer.from(JSON.stringify({ + type: 'response.output_item.done', output_index: 0, + item: { type: 'function_call', call_id: 'call_a', name: 'Edit', arguments: explicitArgs }, + }))); + socketA.emit('message', Buffer.from(JSON.stringify({ type: 'response.completed', response: { id: 'resp_a' } }))); + await readAll(firstA); + + // 2. Client B, same tool name, opposite default. + const clientB = createResponsesWebSocketFetch(WS_URL, undefined, { accountId: 'acct-two-client-b' }); + const firstB = await clientB('https://x', { + method: 'POST', headers: {}, body: JSON.stringify(sessionPayload(input, { tools: toolsWithDefault(true) })), + }); + const socketB = lastSocket(); + expect(socketB).not.toBe(socketA); + socketB.emit('open'); + emitTextResponse(socketB, 'resp_b', 'b done'); + await readAll(firstB); + + // 3. A request in A's partition that declares no Edit tool at all, so it + // re-records nothing and its scan is what populates A's prefix cache. + const sideA = await clientA('https://x', { + method: 'POST', headers: {}, + body: JSON.stringify(sessionPayload( + [{ role: 'user', content: [{ type: 'input_text', text: 'name this chat' }] }], + { tools: [{ type: 'function', name: 'Read', parameters: { type: 'object' } }] }, + )), + }); + const sideSocket = lastSocket(); + if (sideSocket !== socketA) sideSocket.emit('open'); + emitTextResponse(sideSocket, 'resp_a_side', 'title'); + await readAll(sideA); + + // 4. A's next real turn must still continue on its own head. + const echoedCall = { + type: 'function_call', call_id: 'call_a', name: 'Edit', arguments: explicitArgs, + }; + const toolOutput = { type: 'function_call_output', call_id: 'call_a', output: 'edited' }; + const socketsBefore = socketCount(); + const secondA = await clientA('https://x', { + method: 'POST', headers: {}, + body: JSON.stringify(sessionPayload([...input, echoedCall, toolOutput], { tools: toolsWithDefault(false) })), + }); + // Reusing A's socket means no new one is created, so this asserts the count + // rather than which socket is last. + expect(socketCount()).toBe(socketsBefore); + const sentA = socketA.send.mock.calls.map(call => JSON.parse(call[0] as string)); + const continuation = sentA.find(sent => sent.previous_response_id === 'resp_a'); + expect(continuation, `A did not continue on its own head: ${JSON.stringify(sentA.map(s => s.previous_response_id))}`) + .toBeDefined(); + expect(continuation.input).toEqual([toolOutput]); + emitTextResponse(socketA, 'resp_a_done', 'done'); + await readAll(secondA); + }); + + it('still starts a new chain when the echoed value differs from the schema default', async () => { + // Only a value EQUAL to the declared default is filler. `replace_all: true` + // is a real argument the model never sent, so this history diverged. + const tools = [{ + type: 'function', name: 'Edit', + parameters: { + type: 'object', + properties: { + file_path: { type: 'string' }, + replace_all: { type: 'boolean', default: false }, + }, + required: ['file_path'], + }, + }]; + const input = [{ role: 'user', content: [{ type: 'input_text', text: 'fix it' }] }]; + const wsFetch = createResponsesWebSocketFetch(WS_URL, undefined, { accountId: 'acct-schema-nondefault' }); + const first = await wsFetch('https://x', { + method: 'POST', headers: {}, body: JSON.stringify(sessionPayload(input, { tools })), + }); + const socket = lastSocket(); + socket.emit('open'); + socket.emit('message', Buffer.from(JSON.stringify({ type: 'response.created', response: { id: 'resp_edit_t' } }))); + socket.emit('message', Buffer.from(JSON.stringify({ + type: 'response.output_item.done', output_index: 0, + item: { type: 'function_call', call_id: 'call_t', name: 'Edit', arguments: '{"file_path":"a.py"}' }, + }))); + socket.emit('message', Buffer.from(JSON.stringify({ type: 'response.completed', response: { id: 'resp_edit_t' } }))); + await readAll(first); + + const divergedCall = { + type: 'function_call', call_id: 'call_t', name: 'Edit', + arguments: '{"file_path":"a.py","replace_all":true}', + }; + const toolOutput = { type: 'function_call_output', call_id: 'call_t', output: 'edited' }; + const fullInput = [...input, divergedCall, toolOutput]; + const second = await wsFetch('https://x', { + method: 'POST', headers: {}, body: JSON.stringify(sessionPayload(fullInput, { tools })), + }); + const isolated = lastSocket(); + expect(isolated).not.toBe(socket); + isolated.emit('open'); + const sent = JSON.parse(isolated.send.mock.calls[0]![0] as string); + expect(sent.previous_response_id).toBeUndefined(); + expect(sent.input).toEqual(fullInput); + emitTextResponse(isolated, 'resp_edit_t_new', 'done'); + await readAll(second); + }); + + it('does not strip a property the tool schema declares no default for', async () => { + // Without a declared default there is nothing to identify filler, so an + // added property is a divergent history — today's behaviour, unchanged. + const tools = [{ + type: 'function', name: 'Edit', + parameters: { + type: 'object', + properties: { file_path: { type: 'string' }, replace_all: { type: 'boolean' } }, + required: ['file_path'], + }, + }]; + const input = [{ role: 'user', content: [{ type: 'input_text', text: 'fix it' }] }]; + const wsFetch = createResponsesWebSocketFetch(WS_URL, undefined, { accountId: 'acct-schema-nodefault' }); + const first = await wsFetch('https://x', { + method: 'POST', headers: {}, body: JSON.stringify(sessionPayload(input, { tools })), + }); + const socket = lastSocket(); + socket.emit('open'); + socket.emit('message', Buffer.from(JSON.stringify({ type: 'response.created', response: { id: 'resp_edit_n' } }))); + socket.emit('message', Buffer.from(JSON.stringify({ + type: 'response.output_item.done', output_index: 0, + item: { type: 'function_call', call_id: 'call_u', name: 'Edit', arguments: '{"file_path":"a.py"}' }, + }))); + socket.emit('message', Buffer.from(JSON.stringify({ type: 'response.completed', response: { id: 'resp_edit_n' } }))); + await readAll(first); + + const echoedCall = { + type: 'function_call', call_id: 'call_u', name: 'Edit', + arguments: '{"file_path":"a.py","replace_all":false}', + }; + const toolOutput = { type: 'function_call_output', call_id: 'call_u', output: 'edited' }; + const second = await wsFetch('https://x', { + method: 'POST', headers: {}, body: JSON.stringify(sessionPayload([...input, echoedCall, toolOutput], { tools })), + }); + const isolated = lastSocket(); + expect(isolated).not.toBe(socket); + isolated.emit('open'); + const sent = JSON.parse(isolated.send.mock.calls[0]![0] as string); + expect(sent.previous_response_id).toBeUndefined(); + emitTextResponse(isolated, 'resp_edit_n_new', 'done'); + await readAll(second); + }); + it('starts a new chain when the echoed call differs in a meaningful argument value', async () => { const input = [{ role: 'user', content: [{ type: 'input_text', text: 'read it back' }] }]; const wsFetch = createResponsesWebSocketFetch(WS_URL, undefined, { accountId: 'acct-real-diff' }); From c795cb973f51e9b004e5186f876cd9465c5d9e08 Mon Sep 17 00:00:00 2001 From: integ Date: Sat, 12 Sep 2026 18:42:31 -0500 Subject: [PATCH 2/2] fix(oauth): keep approved tool calls cached reliably when tool lists change Normalize head diagnostics with the request's defaults so accepted continuations no longer record misleading tool-argument gaps. Use collision-safe defaults fingerprints and add transport tests that mutation-pin the omitted-reasoning and in-flight memo invalidations. --- .claude/docs/oauth-continuation.md | 27 ++- src/oauth/responses-websocket.ts | 101 ++++---- tests/responses-websocket.test.ts | 376 ++++++++++++++++++++++++++++- 3 files changed, 438 insertions(+), 66 deletions(-) diff --git a/.claude/docs/oauth-continuation.md b/.claude/docs/oauth-continuation.md index 4dbffc9d..ed014bd1 100644 --- a/.claude/docs/oauth-continuation.md +++ b/.claude/docs/oauth-continuation.md @@ -574,14 +574,16 @@ declared default still diverge, as before. The map is per-request and pure, for the same reason `headRequiredToolProps` snapshots `required` from the head's own turn. A process-global map keyed only by tool name is last-writer-wins across -every client, partition and session one `clodex server` handles, and `entry.canonicalPrefix` caches -the head side permanently under whatever the map held when it was first built — so another client's -schema can flip the verdict in either direction: under-stripping costs a chain that should have -continued (the everyday parent-then-subagent sequence), over-stripping continues on a history that -genuinely changed. Both were reproduced through the real WebSocket transport. The two prefix memos -and the in-flight `canonicalInput` memo are therefore keyed on a fingerprint of the defaults map they -were built under, and recomputed when it changes; within one client that fingerprint is constant, so -the memos still do their job. +every client, partition and session one `clodex server` handles. Independently, a request in the same +partition can carry a different tool list — for example, a main-agent auxiliary request or a +mid-session tool-list change — and populate a head memo under a map the next request no longer uses. +An in-process subagent has its own partition because `responsesWebSocketPartitionKey` includes +`x-claude-code-agent-id`, so it never scans its parent's heads. Without keyed invalidation, +under-stripping costs a chain that should have continued and over-stripping can continue changed +history. The two prefix memos and the in-flight `canonicalInput` memo are therefore keyed on a +fingerprint of the defaults map they were built under and recomputed when it changes. While a +request's tool defaults are unchanged, the fingerprint is stable and the memos still do their job; +when the tool list changes, keyed invalidation recomputes them. Trade-off: a request whose `tools` omit a tool that appears in its own history gets no stripping for that tool, which is the pre-fix behaviour. Subagent histories never contain the parent's calls, so @@ -592,6 +594,9 @@ the residual is narrow. On a history mismatch the head-decision log includes `expected_hash`/`actual_hash` (SHA-256 of each side's canonical item bytes) whenever at least one side has an item at the divergent index, so same-kind mismatches are diagnosable without exposing content; `none` marks an unavailable side. +The mismatch index, hashes, tool-argument gap check, and opt-in dump all use the same per-request +tool-defaults map as head matching, so a default-stripped continuation records the full matched +prefix instead of a misleading normalization gap. `CLODEX_MISMATCH_DUMP=1` additionally writes both divergent items' canonical bytes (capped per line, `(absent)` past a history's end) into the adapter debug log. **Privacy tradeoff:** the dump contains @@ -609,9 +614,9 @@ genuine rewind or branch regenerates the call under a new one. These record `toolArgumentNormalizationGap` (`tool`, `equalAfterStrip`) on the head-decision diagnostic. - **Only `equalAfterStrip: true` warns on stderr**, deduplicated by tool and hard-capped (the - terminal is shared with Claude Code's UI). It means the two items are identical once the shared - filler-strip rule is applied to `arguments` — nothing but filler stood between the head and its - own echo. + terminal is shared with Claude Code's UI). It means the two items are identical once head + matching's schema-default normalization and the shared filler-strip rule are both applied to + `arguments` — nothing but filler stood between the head and its own echo. - **Coverage is narrower than it looks, in two directions.** It fires only when the divergent `function_call` is the *first* divergent item, with one alignment: a stored reasoning item Claude legitimately omitted (`continuationMatch`'s omitted-reasoning mode) shifts divergence onto a diff --git a/src/oauth/responses-websocket.ts b/src/oauth/responses-websocket.ts index 23cbec76..405704f7 100644 --- a/src/oauth/responses-websocket.ts +++ b/src/oauth/responses-websocket.ts @@ -472,10 +472,11 @@ function inputArray(payload: JsonObject): unknown[] { * snapshots `required` from the head's own turn: a process-global map keyed only * by tool name is last-writer-wins across every client, partition and session a * `clodex server` handles, and reading another client's schema can flip the - * verdict in either direction with no code change — under-stripping (a cached - * head keeps a property the client strips, so an ordinary parent-then-subagent - * sequence loses its chain) or over-stripping (a genuine history change compares - * equal). Both were reproduced through the real WebSocket transport. + * verdict in either direction with no code change. A request in the same + * partition can also carry a different tool list — for example, a main-agent + * auxiliary request or a mid-session tool-list change — and populate a memo + * under a map that the next request no longer uses. Without keyed invalidation, + * under-stripping loses a chain and over-stripping can accept changed history. */ type ToolSchemaDefaults = Map>; @@ -510,20 +511,19 @@ export function toolSchemaDefaults(payload: JsonObject): ToolSchemaDefaults { * * `entry.canonicalPrefix` is memoized across requests, so the head side must not * keep bytes that were normalized under a different map than the client side is - * being normalized under right now. Within one client the map is constant and the - * memo still holds; when it changes, the fingerprint changes and the prefix is - * recomputed. + * being normalized under right now. While a request's tool defaults are unchanged, + * the fingerprint is stable and the memo still holds; when the tool list changes, + * keyed invalidation recomputes the prefix. */ function toolSchemaDefaultsFingerprint(defaults: ToolSchemaDefaults): string { if (!defaults.size) return 'none'; - const parts: string[] = []; - for (const name of [...defaults.keys()].sort()) { - const perTool = defaults.get(name)!; - const props = [...perTool.entries()].sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)) - .map(([prop, value]) => `${prop}=${value}`).join(','); - parts.push(`${name}:${props}`); - } - return createHash('sha256').update(parts.join(';')).digest('hex').slice(0, 16); + const tuples = [...defaults.entries()] + .sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)) + .map(([name, perTool]) => [ + name, + [...perTool.entries()].sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)), + ] as const); + return createHash('sha256').update(JSON.stringify(tuples)).digest('hex').slice(0, 16); } function stripSchemaDefaults(name: unknown, args: unknown, defaults: ToolSchemaDefaults | undefined): unknown { @@ -540,7 +540,7 @@ function stripSchemaDefaults(name: unknown, args: unknown, defaults: ToolSchemaD return out; } -function normalizeToolCallJson(value: unknown, defaults?: ToolSchemaDefaults): unknown { +function normalizeToolCallJson(value: unknown, defaults: ToolSchemaDefaults): unknown { if (Array.isArray(value)) return value.map(item => normalizeToolCallJson(item, defaults)); if (!value || typeof value !== 'object') return value; const record = value as JsonObject; @@ -586,8 +586,9 @@ function normalizeToolCallJson(value: unknown, defaults?: ToolSchemaDefaults): u return out; } -function arraysEqual(left: unknown[], right: unknown[]): boolean { - return canonicalJson(normalizeToolCallJson(left)) === canonicalJson(normalizeToolCallJson(right)); +function arraysEqual(left: unknown[], right: unknown[], defaults: ToolSchemaDefaults): boolean { + return canonicalJson(normalizeToolCallJson(left, defaults)) + === canonicalJson(normalizeToolCallJson(right, defaults)); } type ContinuationMatchMode = 'exact' | 'omitted_reasoning'; @@ -605,8 +606,11 @@ function conversationItemKind(value: unknown): string { return 'object'; } -function conversationItemHash(value: unknown): string { - return createHash('sha256').update(canonicalJson(normalizeToolCallJson(value))).digest('hex').slice(0, 16); +function conversationItemHash(value: unknown, defaults: ToolSchemaDefaults): string { + return createHash('sha256') + .update(canonicalJson(normalizeToolCallJson(value, defaults))) + .digest('hex') + .slice(0, 16); } /** @@ -620,7 +624,11 @@ function conversationItemHash(value: unknown): string { * fresh turn) and the mismatch is correct, not a defect — reporting those would * bury the signal in noise. */ -function reasoningNormalizationGap(expected: unknown, actual: unknown): string[] | undefined { +function reasoningNormalizationGap( + expected: unknown, + actual: unknown, + defaults: ToolSchemaDefaults, +): string[] | undefined { if (conversationItemKind(expected) !== 'reasoning' || conversationItemKind(actual) !== 'reasoning') return undefined; const left = expected as JsonObject; const right = actual as JsonObject; @@ -628,8 +636,8 @@ function reasoningNormalizationGap(expected: unknown, actual: unknown): string[] if (typeof blob !== 'string' || !blob || blob !== right.encrypted_content) return undefined; // Diff the NORMALIZED items. Diffing the raw ones names fields that // normalization already reconciles, which points a reader at a red herring. - const normalizedLeft = normalizeToolCallJson(left) as JsonObject; - const normalizedRight = normalizeToolCallJson(right) as JsonObject; + const normalizedLeft = normalizeToolCallJson(left, defaults) as JsonObject; + const normalizedRight = normalizeToolCallJson(right, defaults) as JsonObject; const fields = [...new Set([...Object.keys(normalizedLeft), ...Object.keys(normalizedRight)])].sort() .filter(key => canonicalJson(normalizedLeft[key]) !== canonicalJson(normalizedRight[key])); return fields.length ? fields : undefined; @@ -724,8 +732,9 @@ export function resetReasoningGapWarningsForTests(): void { * clean. * * `equalAfterStrip` separates the two mechanisms. It re-compares the WHOLE - * items with the shared filler-strip rule applied to `arguments` — not the - * arguments alone, or a divergence in any other field would be reported as a + * items with head matching's schema-default normalization and the shared + * filler-strip rule both applied to `arguments` — not the arguments alone, or + * a divergence in any other field would be reported as a * strip-rule gap the code never examined. When that makes them equal, the only * thing standing between the head and its own echo is filler the shared rule * removes, which is the shape #84 had. When they still differ, the difference @@ -743,6 +752,7 @@ export function resetReasoningGapWarningsForTests(): void { function toolArgumentNormalizationGap( expected: unknown, actual: unknown, + defaults: ToolSchemaDefaults, requiredProps: () => Map>, ): Record | undefined { if (conversationItemKind(expected) !== 'function_call') return undefined; @@ -754,7 +764,8 @@ function toolArgumentNormalizationGap( if (typeof left.name !== 'string' || left.name !== right.name) return undefined; // Same call, same tool, different bytes. Compare NORMALIZED arguments so the // canonical-JSON reconciliation this file already applies is not re-reported. - if (canonicalJson(normalizeToolCallJson(left)) === canonicalJson(normalizeToolCallJson(right))) { + if (canonicalJson(normalizeToolCallJson(left, defaults)) + === canonicalJson(normalizeToolCallJson(right, defaults))) { return undefined; } const required = requiredProps().get(left.name); @@ -767,8 +778,8 @@ function toolArgumentNormalizationGap( // Carry the rest of the item along, so a difference somewhere other than // `arguments` cannot be reported as the filler-strip rule having forked. return canonicalJson({ - ...(normalizeToolCallJson(item) as JsonObject), - arguments: canonicalJson(sanitizeToolInput(parsed, required)), + ...(normalizeToolCallJson(item, defaults) as JsonObject), + arguments: canonicalJson(stripSchemaDefaults(item.name, sanitizeToolInput(parsed, required), defaults)), }); } catch { return undefined; } }; @@ -830,6 +841,7 @@ export function resetToolArgumentGapWarningsForTests(): void { function continuationMismatchDetails( entry: ConnectionEntry, payload: JsonObject, + defaults: ToolSchemaDefaults, log?: (message: string) => void, // Only the head clodex actually gave up on should reach stderr. Every candidate // head is described in the diagnostic, and a gap on a head that lost to a better @@ -854,14 +866,14 @@ function continuationMismatchDetails( const comparable = Math.min(full.length, prefix.length); let mismatch = comparable; for (let index = 0; index < comparable; index += 1) { - if (!arraysEqual([full[index]], [prefix[index]])) { + if (!arraysEqual([full[index]], [prefix[index]], defaults)) { mismatch = index; break; } } const expected = mismatch < prefix.length ? prefix[mismatch] : undefined; const actual = mismatch < full.length ? full[mismatch] : undefined; - const reasoningGap = reasoningNormalizationGap(expected, actual); + const reasoningGap = reasoningNormalizationGap(expected, actual, defaults); if (reasoningGap && warnOnGap) raise(() => warnReasoningNormalizationGap(reasoningGap, log)); // Claude may legitimately omit stored reasoning items (continuationMatch's // omitted_reasoning mode), which shifts the exact-prefix divergence onto a @@ -885,6 +897,7 @@ function continuationMismatchDetails( toolArgumentGap = toolArgumentNormalizationGap( gapExpected, actual, + defaults, // The head's own schema when it has one; the current turn's tools are only a // fallback for a head that predates the snapshot (see headRequiredToolProps). () => entry.headRequiredToolProps ?? requiredToolProps(payload), @@ -910,8 +923,8 @@ function continuationMismatchDetails( firstMismatch: mismatch, expectedKind: expected === undefined ? 'none' : conversationItemKind(expected), actualKind: actual === undefined ? 'none' : conversationItemKind(actual), - ...(expected !== undefined ? { expectedHash: conversationItemHash(expected) } : {}), - ...(actual !== undefined ? { actualHash: conversationItemHash(actual) } : {}), + ...(expected !== undefined ? { expectedHash: conversationItemHash(expected, defaults) } : {}), + ...(actual !== undefined ? { actualHash: conversationItemHash(actual, defaults) } : {}), ...(reasoningGap ? { reasoningNormalizationGap: reasoningGap, @@ -927,11 +940,12 @@ function continuationMismatchDetails( function continuationMismatchSummary( entry: ConnectionEntry, payload: JsonObject, + defaults: ToolSchemaDefaults, log?: (message: string) => void, mismatchDump = false, precomputedDetails?: Record, ): string { - const details = precomputedDetails ?? continuationMismatchDetails(entry, payload, log, true); + const details = precomputedDetails ?? continuationMismatchDetails(entry, payload, defaults, log, true); let summary = `full_items=${details.fullItems} expected_prefix_items=${details.expectedPrefixItems} ` + `first_mismatch=${details.firstMismatch} expected=${details.expectedKind} actual=${details.actualKind}`; // The hashes make same-kind mismatches diagnosable from the log alone. With @@ -946,8 +960,8 @@ function continuationMismatchSummary( const full = inputArray(payload); const prefix = [...(entry.requestInput ?? []), ...(entry.expectedAssistant ?? [])]; const index = details.firstMismatch as number; - log(`mismatch dump expected[${index}]: ${mismatchDumpLine(prefix, index)}`); - log(`mismatch dump actual[${index}]: ${mismatchDumpLine(full, index)}`); + log(`mismatch dump expected[${index}]: ${mismatchDumpLine(prefix, index, defaults)}`); + log(`mismatch dump actual[${index}]: ${mismatchDumpLine(full, index, defaults)}`); } } return summary; @@ -955,9 +969,9 @@ function continuationMismatchSummary( /** One side of a mismatch dump: canonical item bytes, capped, or `(absent)` * when the divergence is one history simply ending before the other. */ -function mismatchDumpLine(items: unknown[], index: number): string { +function mismatchDumpLine(items: unknown[], index: number, defaults: ToolSchemaDefaults): string { if (index >= items.length) return '(absent)'; - const line = canonicalJson(normalizeToolCallJson(items[index])); + const line = canonicalJson(normalizeToolCallJson(items[index], defaults)); const max = 2_000; const marker = ' [truncated]'; return line.length <= max ? line : line.slice(0, max - marker.length) + marker; @@ -972,7 +986,7 @@ function mismatchDumpLine(items: unknown[], index: number): string { * meaning to comparing whole arrays, but it lets both sides be computed once * instead of re-serializing an entire conversation for every candidate head. */ -function canonicalItemStrings(items: unknown[], defaults?: ToolSchemaDefaults): string[] { +function canonicalItemStrings(items: unknown[], defaults: ToolSchemaDefaults): string[] { return items.map(item => canonicalJson(normalizeToolCallJson([item], defaults))); } @@ -2478,7 +2492,7 @@ export function createResponsesWebSocketFetch( // A rewind, branch, or hidden auxiliary inference gets its own full-context // head. Existing heads remain eligible for later exact-prefix matches. const diagnosticMismatch = continuationMismatchDetails( - diagnosticEntry, payload, debug, true, deferredMismatchWarnings, + diagnosticEntry, payload, requestToolDefaults, debug, true, deferredMismatchWarnings, ); candidateMismatchDetails = new Map([[diagnosticEntry, diagnosticMismatch]]); // Every abandoned non-diagnostic head warns independently of diagnostics. @@ -2487,7 +2501,9 @@ export function createResponsesWebSocketFetch( if (candidate === diagnosticEntry) continue; candidateMismatchDetails.set( candidate, - continuationMismatchDetails(candidate, payload, debug, true, deferredMismatchWarnings), + continuationMismatchDetails( + candidate, payload, requestToolDefaults, debug, true, deferredMismatchWarnings, + ), ); } debug( @@ -2495,6 +2511,7 @@ export function createResponsesWebSocketFetch( + `(${continuationMismatchSummary( diagnosticEntry, payload, + requestToolDefaults, debug, mismatchDump, diagnosticMismatch, @@ -2690,7 +2707,7 @@ export function createResponsesWebSocketFetch( input: { count: requestInput.length, kinds: requestInput.map(conversationItemKind), - hashes: requestInput.map(conversationItemHash), + hashes: requestInput.map(item => conversationItemHash(item, requestToolDefaults)), }, candidateCount: candidates.length, idleCandidateCount: idleCandidates.length, @@ -2726,7 +2743,7 @@ export function createResponsesWebSocketFetch( idleMs: Math.max(0, now - entry.lastUsedAt), promptChanges: changedPromptFields(entry.promptFieldHashes, promptFieldHashes), mismatch: candidateMismatchDetails?.get(entry) - ?? continuationMismatchDetails(entry, payload, debug), + ?? continuationMismatchDetails(entry, payload, requestToolDefaults, debug), })), evictions, }, diagnosticCorrelation); diff --git a/tests/responses-websocket.test.ts b/tests/responses-websocket.test.ts index b917f690..5027b7a5 100644 --- a/tests/responses-websocket.test.ts +++ b/tests/responses-websocket.test.ts @@ -2100,20 +2100,17 @@ describe('createResponsesWebSocketFetch', () => { }); it('does not let one client\'s tool schema decide another client\'s continuation', async () => { - // The defaults map used to be process-global and keyed only by tool name, so - // whichever client declared `Edit` last decided how every other client's - // history was normalized — and `entry.canonicalPrefix` caches the head side - // permanently under whatever the map held at the instant it was first built. - // Three requests, one process, NO reset between them (the reset in beforeEach - // is what hid this): + // Defaults are per request so client B's schema cannot affect client A. The + // prefix memo is also keyed because A's own requests can change tool lists. + // Four requests, one process, NO reset between them (the reset in beforeEach + // is what hid the process-global map): // 1. client A establishes a head whose call carries replace_all EXPLICITLY, // under a schema declaring default false — so both sides strip it; // 2. client B runs a turn declaring the same tool with default TRUE; - // 3. a second request in A's own partition whose tools do NOT include Edit - // (a title generation, a subagent) scans A's idle head and caches its - // canonical prefix — under B's schema, with a shared map; - // 4. A's next real turn then strips its echo client-side while the cached - // head keeps it, so A loses a chain it should have kept. + // 3. a main-agent auxiliary request in A's partition omits Edit and scans + // A's idle head, caching its prefix under the empty defaults map; + // 4. A's next real turn restores Edit, so keyed invalidation must rebuild + // the head under the same map that strips its client-side echo. const toolsWithDefault = (value: boolean) => [{ type: 'function', name: 'Edit', parameters: { @@ -2190,6 +2187,241 @@ describe('createResponsesWebSocketFetch', () => { await readAll(secondA); }); + it('recomputes an omitted-reasoning prefix memo when tool defaults change', async () => { + const toolsWithDefault = [{ + type: 'function', name: 'MemoTool', + parameters: { + type: 'object', + properties: { a: { type: 'number', default: 1 } }, + }, + }]; + const toolsWithoutDefault = [{ + type: 'function', name: 'MemoTool', + parameters: { + type: 'object', + properties: { a: { type: 'number' } }, + }, + }]; + const input = [{ role: 'user', content: [{ type: 'input_text', text: 'run it' }] }]; + const echoedCall = { + type: 'function_call', call_id: 'call_memo', name: 'MemoTool', arguments: '{"a":1}', + }; + const output = { type: 'function_call_output', call_id: 'call_memo', output: 'done' }; + const diagnostics: ResponsesWebSocketDiagnosticEvent[] = []; + const wsFetch = createResponsesWebSocketFetch(WS_URL, undefined, { + accountId: 'acct-echoable-memo', + onDiagnostic: event => diagnostics.push(event), + }); + + const first = await wsFetch('https://x', { + method: 'POST', headers: {}, body: JSON.stringify(sessionPayload(input, { tools: toolsWithDefault })), + }); + const headSocket = lastSocket(); + headSocket.emit('open'); + headSocket.emit('message', Buffer.from(JSON.stringify({ + type: 'response.created', response: { id: 'resp_memo_head' }, + }))); + headSocket.emit('message', Buffer.from(JSON.stringify({ + type: 'response.output_item.done', output_index: 0, + item: { type: 'reasoning', id: 'rs_memo', encrypted_content: 'enc_memo', summary: [] }, + }))); + headSocket.emit('message', Buffer.from(JSON.stringify({ + type: 'response.output_item.done', output_index: 1, + item: { + type: 'function_call', id: 'fc_memo', call_id: 'call_memo', name: 'MemoTool', + arguments: '{"a":1}', status: 'completed', + }, + }))); + headSocket.emit('message', Buffer.from(JSON.stringify({ + type: 'response.completed', response: { id: 'resp_memo_head' }, + }))); + await readAll(first); + + // Populate both prefix memos while `a: 1` is a declared default. + const side = await wsFetch('https://x', { + method: 'POST', headers: {}, + body: JSON.stringify(sessionPayload( + [{ role: 'user', content: [{ type: 'input_text', text: 'unrelated side turn' }] }], + { tools: toolsWithDefault }, + )), + }); + const sideSocket = lastSocket(); + if (sideSocket !== headSocket) sideSocket.emit('open'); + emitTextResponse(sideSocket, 'resp_memo_side', 'done'); + await readAll(side); + + // Claude omits reasoning but echoes the call. Under the current schema `a: 1` + // is not filler, so the echoable prefix must be rebuilt with it retained. + const socketsBefore = socketCount(); + const continuation = await wsFetch('https://x', { + method: 'POST', headers: {}, + body: JSON.stringify(sessionPayload( + [...input, echoedCall, output], + { tools: toolsWithoutDefault }, + )), + }); + expect(socketCount()).toBe(socketsBefore); + expect(diagnostics.filter(event => event.event === 'ws_head_decision').at(-1)).toMatchObject({ + decision: 'continuation', + continuationMatchMode: 'omitted_reasoning', + }); + const sent = headSocket.send.mock.calls.map(call => JSON.parse(call[0] as string)) + .find(message => message.previous_response_id === 'resp_memo_head'); + expect(sent?.input).toEqual([output]); + emitTextResponse(headSocket, 'resp_memo_done', 'done'); + await readAll(continuation); + }); + + it('recomputes an in-flight input memo when an arriving request changes tool defaults', async () => { + const toolsWithDefault = [{ + type: 'function', name: 'InFlightTool', + parameters: { + type: 'object', + properties: { a: { type: 'number', default: 1 } }, + }, + }]; + const toolsWithoutDefault = [{ + type: 'function', name: 'InFlightTool', + parameters: { + type: 'object', + properties: { a: { type: 'number' } }, + }, + }]; + const user = { role: 'user', content: [{ type: 'input_text', text: 'run it' }] }; + const originalCall = { + type: 'function_call', call_id: 'call_inflight', name: 'InFlightTool', arguments: '{"a":1}', + }; + const rewrittenCall = { + type: 'function_call', call_id: 'call_inflight', name: 'InFlightTool', arguments: '{}', + }; + const output = { type: 'function_call_output', call_id: 'call_inflight', output: 'done' }; + const diagnostics: ResponsesWebSocketDiagnosticEvent[] = []; + const wsFetch = createResponsesWebSocketFetch(WS_URL, undefined, { + accountId: 'acct-inflight-memo', + onDiagnostic: event => diagnostics.push(event), + }); + + // Keep the first turn in flight. Its RequestContext owns the canonical-input memo. + const first = await wsFetch('https://x', { + method: 'POST', headers: {}, + body: JSON.stringify(sessionPayload([user, originalCall], { tools: toolsWithDefault })), + }); + const originalSocket = lastSocket(); + originalSocket.emit('open'); + + // Build the in-flight memo under default a=1; both call arguments normalize to {}. + const firstArrival = await wsFetch('https://x', { + method: 'POST', headers: {}, + body: JSON.stringify(sessionPayload( + [user, rewrittenCall, output], + { tools: toolsWithDefault }, + )), + }); + const isolatedSocket = lastSocket(); + isolatedSocket.emit('open'); + emitTextResponse(isolatedSocket, 'resp_inflight_isolated', 'done'); + await readAll(firstArrival); + expect(diagnostics.filter(event => event.event === 'ws_head_decision').at(-1)?.decision) + .toBe('parallel_isolated'); + + // With no declared default, a=1 and {} differ. The in-flight turn is unrelated, + // so this request must retain a new head instead of isolating. + const secondArrival = await wsFetch('https://x', { + method: 'POST', headers: {}, + body: JSON.stringify(sessionPayload( + [user, rewrittenCall, output], + { tools: toolsWithoutDefault }, + )), + }); + expect(diagnostics.filter(event => event.event === 'ws_head_decision').at(-1)?.decision) + .toBe('history_mismatch_new_head'); + const retainedSocket = lastSocket(); + retainedSocket.emit('open'); + emitTextResponse(retainedSocket, 'resp_inflight_retained', 'done'); + await readAll(secondArrival); + + emitTextResponse(originalSocket, 'resp_inflight_original', 'done'); + await readAll(first); + }); + + it('distinguishes default maps whose old delimiter-joined fingerprints collided', async () => { + const toolsA = [{ + type: 'function', name: 'CollisionTool', + parameters: { + type: 'object', + properties: { + a: { type: 'number', default: 1 }, + b: { type: 'number', default: 2 }, + }, + }, + }]; + // The old fingerprint joined properties as `prop=value` with commas, so this + // distinct map and toolsA both serialized as `a=1,b=2` before hashing. + const toolsB = [{ + type: 'function', name: 'CollisionTool', + parameters: { + type: 'object', + properties: { 'a=1,b': { type: 'number', default: 2 } }, + }, + }]; + const input = [{ role: 'user', content: [{ type: 'input_text', text: 'run it' }] }]; + const wsFetch = createResponsesWebSocketFetch(WS_URL, undefined, { + accountId: 'acct-default-fingerprint-collision', + }); + + const first = await wsFetch('https://x', { + method: 'POST', headers: {}, body: JSON.stringify(sessionPayload(input, { tools: toolsA })), + }); + const headSocket = lastSocket(); + headSocket.emit('open'); + headSocket.emit('message', Buffer.from(JSON.stringify({ + type: 'response.created', response: { id: 'resp_collision_head' }, + }))); + headSocket.emit('message', Buffer.from(JSON.stringify({ + type: 'response.output_item.done', output_index: 0, + item: { + type: 'function_call', call_id: 'call_collision', name: 'CollisionTool', + arguments: '{"a":1,"b":2}', + }, + }))); + headSocket.emit('message', Buffer.from(JSON.stringify({ + type: 'response.completed', response: { id: 'resp_collision_head' }, + }))); + await readAll(first); + + // Cache the head under toolsA, where both arguments strip to {}. + const side = await wsFetch('https://x', { + method: 'POST', headers: {}, + body: JSON.stringify(sessionPayload( + [{ role: 'user', content: [{ type: 'input_text', text: 'unrelated side turn' }] }], + { tools: toolsA }, + )), + }); + const sideSocket = lastSocket(); + if (sideSocket !== headSocket) sideSocket.emit('open'); + emitTextResponse(sideSocket, 'resp_collision_side', 'done'); + await readAll(side); + + // Under toolsB, a and b are not defaults. A distinct fingerprint must rebuild + // the cached head so the unchanged call still matches the unchanged echo. + const echoedCall = { + type: 'function_call', call_id: 'call_collision', name: 'CollisionTool', + arguments: '{"a":1,"b":2}', + }; + const output = { type: 'function_call_output', call_id: 'call_collision', output: 'done' }; + const socketsBefore = socketCount(); + const continuation = await wsFetch('https://x', { + method: 'POST', headers: {}, + body: JSON.stringify(sessionPayload([...input, echoedCall, output], { tools: toolsB })), + }); + expect(socketCount()).toBe(socketsBefore); + const sent = headSocket.send.mock.calls.map(call => JSON.parse(call[0] as string)) + .find(message => message.previous_response_id === 'resp_collision_head'); + expect(sent?.input).toEqual([output]); + emitTextResponse(headSocket, 'resp_collision_done', 'done'); + await readAll(continuation); + }); + it('still starts a new chain when the echoed value differs from the schema default', async () => { // Only a value EQUAL to the declared default is filler. `replace_all: true` // is a real argument the model never sent, so this history diverged. @@ -2860,8 +3092,9 @@ describe('createResponsesWebSocketFetch', () => { return (decision.heads as { mismatch: Record }[])[0]!.mismatch; } - // Drives one mismatch between a stored function_call and the call Claude echoes - // back. + // Drives one comparison between a stored function_call and the call Claude + // echoes back. Most callers exercise a mismatch; the schema-default diagnostic + // case exercises the normalized match. // // The stored call is emitted BY UPSTREAM, so it reaches the head through // `response.output_item.done` → `expectedAssistantItems` → `sanitizedCallArguments` @@ -2956,6 +3189,85 @@ describe('createResponsesWebSocketFetch', () => { }, }; + it('records the full matched prefix after a schema-default continuation', async () => { + const tools = [{ + type: 'function', name: 'Edit', + parameters: { + type: 'object', + properties: { + file_path: { type: 'string' }, + old_string: { type: 'string' }, + new_string: { type: 'string' }, + replace_all: { type: 'boolean', default: false }, + }, + required: ['file_path', 'old_string', 'new_string'], + }, + }]; + const { diagnostics } = await runToolArgumentMismatch({ + accountId: 'acct-schema-default-diagnostic', + responseId: 'resp_schema_default_diagnostic', + tools, + upstreamCall: { + type: 'function_call', call_id: 'call_edit_diagnostic', name: 'Edit', + arguments: '{"file_path":"a.py","old_string":"x","new_string":"y"}', + }, + echoedCall: { + type: 'function_call', call_id: 'call_edit_diagnostic', name: 'Edit', + arguments: '{"file_path":"a.py","old_string":"x","new_string":"y","replace_all":false}', + }, + }); + + const decision = diagnostics.filter(event => event.event === 'ws_head_decision').at(-1)!; + expect(decision).toMatchObject({ + decision: 'continuation', + continuationMatchMode: 'exact', + }); + const mismatch = firstHeadMismatch(diagnostics); + expect(mismatch).toMatchObject({ + fullItems: 3, + expectedPrefixItems: 2, + firstMismatch: 2, + expectedKind: 'none', + actualKind: 'function_call_output', + actualHash: expect.stringMatching(/^[0-9a-f]{16}$/), + }); + expect(mismatch).not.toHaveProperty('expectedHash'); + expect(mismatch).not.toHaveProperty('toolArgumentNormalizationGap'); + }); + + it('preserves an undefaulted false argument beside a declared default', async () => { + const tools = [{ + type: 'function', name: 'Edit', + parameters: { + type: 'object', + properties: { + file_path: { type: 'string' }, + replace_all: { type: 'boolean', default: false }, + dry_run: { type: 'boolean' }, + }, + required: ['file_path'], + }, + }]; + const { diagnostics } = await runToolArgumentMismatch({ + accountId: 'acct-schema-undefaulted-false', + responseId: 'resp_schema_undefaulted_false', + tools, + upstreamCall: { + type: 'function_call', call_id: 'call_edit_false', name: 'Edit', + arguments: '{"file_path":"a.py"}', + }, + echoedCall: { + type: 'function_call', call_id: 'call_edit_false', name: 'Edit', + arguments: '{"file_path":"a.py","dry_run":false}', + }, + }); + + expect(diagnostics.filter(event => event.event === 'ws_head_decision').at(-1)).toMatchObject({ + decision: 'history_mismatch_new_head', + matchingCandidateCount: 0, + }); + }); + it('warns on stderr when the filler-strip rule has forked', async () => { const { stderr, diagnostics } = await runToolArgumentMismatch({ accountId: 'acct-tool-gap-forked', @@ -2975,6 +3287,44 @@ describe('createResponsesWebSocketFetch', () => { .toMatchObject({ toolArgumentNormalizationGap: { tool: 'Grep', equalAfterStrip: true } }); }); + it('warns when a filler-strip gap accompanies a schema-default echo', async () => { + const tools = [{ + type: 'function', name: 'Edit', + parameters: { + type: 'object', + properties: { + file_path: { type: 'string' }, + old_string: { type: 'string' }, + new_string: { type: 'string' }, + glob: { type: 'string' }, + replace_all: { type: 'boolean', default: false }, + }, + required: ['file_path', 'old_string', 'new_string'], + }, + }]; + const { stderr, diagnostics } = await runToolArgumentMismatch({ + accountId: 'acct-tool-gap-default', + responseId: 'resp_tool_gap_default', + tools, + upstreamCall: { + type: 'function_call', id: 'fc_1', call_id: 'call_edit', name: 'Edit', + arguments: '{"file_path":"a.py","old_string":"x","new_string":"y"}', + status: 'completed', + }, + echoedCall: { + type: 'function_call', call_id: 'call_edit', name: 'Edit', + arguments: '{"file_path":"a.py","old_string":"x","new_string":"y","glob":null,"replace_all":false}', + }, + }); + + expect(stderr.join('')).toContain('filler-strip rule is applied'); + expect(diagnostics.filter(event => event.event === 'ws_head_decision').at(-1)) + .toMatchObject({ decision: 'history_mismatch_new_head' }); + expect(firstHeadMismatch(diagnostics)).toMatchObject({ + toolArgumentNormalizationGap: { tool: 'Edit', equalAfterStrip: true }, + }); + }); + it('routes the tool-argument canary through the channel launchClaude leaves open', async () => { // The regression detector for issue #84 is worthless if it only prints on a // path `clodex claude` never takes: with the child holding the terminal, the