fix(ai): clear malformed tool-call queues at turn boundaries
Review feedback on #3459: the per-id FIFO queue still carried a stale malformed occurrence when its rejection toolResult never arrived before conversation history moved on. If the same id was reused later by a valid call, that valid call's real result shifted the stale `true` and was dropped, forcing `transformMessages` to synthesize `No result provided` despite real output existing. Scope malformed-result pairing to a single assistant-to-tool-result window: clear queues at assistant/user/developer/other non-result boundaries, push current assistant occurrences, and pop only contiguous matching tool results. Missing results remain handled by the existing transform passes instead of poisoning later duplicate-id calls. Adds a regression test for malformed-without-result followed by valid reuse of the same id.
This commit is contained in:
@@ -5,7 +5,7 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed prior-turn reasoning being lost on cross-API provider switches: when a session moved from an Anthropic-compatible 3p endpoint to an OpenAI-compatible one (Z.AI Anthropic → Z.AI OpenAI, Kimi Anthropic → Kimi OpenAI, DeepSeek, OpenCode-hosted reasoning models, or any custom `models.yaml` switch that crosses API types), the cross-API path of `transformMessages` text-demoted every prior `thinking` block, so the next request shipped the reasoning chain as plain conversation `content` instead of structured `reasoning_content` — losing it as reasoning context and re-billing it. `convertMessages` now threads the request-time resolved compat into `transformMessages`, which preserves the prior reasoning as a native, signature-stripped `thinking` block whenever that resolved target accepts `reasoning_content` as a continuation hint (`requiresReasoningContentForToolCalls` — including the `whenThinking` policy OpenCode reactivates for thinking-on requests, #1071/#1484 — or `thinkingFormat: "zai"`); the `openai-completions` encoder surfaces those blocks via `reasoningContentField`, with a new branch for Z.AI-format hosts (Z.AI, Zhipu, Moonshot Kimi, Xiaomi MiMo) that accept but don't require the field. Targets that can't replay unsigned reasoning (encrypted reasoning blobs, signed thought parts, non-reasoning models, thinking-disabled OpenCode) still text-demote so the reasoning survives as conversation context. ([#3437](https://github.com/can1357/oh-my-pi/pull/3437), [#3439](https://github.com/can1357/oh-my-pi/pull/3439) by [@roboomp](https://github.com/roboomp); [#3433](https://github.com/can1357/oh-my-pi/issues/3433), [#3434](https://github.com/can1357/oh-my-pi/issues/3434))
|
||||
- Fixed malformed tool calls (empty `name`) wedging entire sessions in HTTP 400 loops: when a model occasionally emits `{ "name": "", "arguments": "{}" }` (observed: GLM-5.2 + thinking on long turns), the agent rejected the call at execution time with `Tool not found`, but the malformed block plus its error `toolResult` stayed in conversation history and every subsequent request 400'd on `tool_use.name`/`tool_calls[i].function.name` validation until the user ran `/clear`. `transformMessages` — the canonical sanitize boundary every provider passes through — now drops `toolCall` blocks with empty/whitespace `name`, pairs them positionally with their `toolResult` messages (per-id FIFO queue, so when a duplicate id replays across turns we drop only the result tied to the malformed occurrence and the surviving valid duplicate's real result still reaches the wire), and drops the assistant turn when it has no replayable content left. Defensive (provider-agnostic, fires regardless of model), idempotent (no-op on a clean history), and self-healing (one round-trip after the fix lands sanitizes an already-poisoned session). ([#3458](https://github.com/can1357/oh-my-pi/issues/3458))
|
||||
- Fixed malformed tool calls (empty `name`) wedging entire sessions in HTTP 400 loops: when a model occasionally emits `{ "name": "", "arguments": "{}" }` (observed: GLM-5.2 + thinking on long turns), the agent rejected the call at execution time with `Tool not found`, but the malformed block plus its error `toolResult` stayed in conversation history and every subsequent request 400'd on `tool_use.name`/`tool_calls[i].function.name` validation until the user ran `/clear`. `transformMessages` — the canonical sanitize boundary every provider passes through — now drops `toolCall` blocks with empty/whitespace `name`, pairs them with their `toolResult` messages only inside the same assistant→tool-result window (per-id FIFO queue cleared at non-result boundaries, so stale malformed calls without a result cannot consume later valid duplicate-id outputs), and drops the assistant turn when it has no replayable content left. Defensive (provider-agnostic, fires regardless of model), idempotent (no-op on a clean history), and self-healing (one round-trip after the fix lands sanitizes an already-poisoned session). ([#3458](https://github.com/can1357/oh-my-pi/issues/3458))
|
||||
|
||||
## [16.1.18] - 2026-06-25
|
||||
|
||||
|
||||
@@ -160,18 +160,21 @@ function sanitizeMalformedToolCalls(messages: Message[]): Message[] {
|
||||
}
|
||||
if (!hasMalformed) return messages;
|
||||
|
||||
// Positional FIFO pairing: a tool-call id can repeat across history when an
|
||||
// OpenAI-Responses composite id (`callId|itemId`) collapses on the wire to
|
||||
// the same `callId` (see `deduplicateToolCallIds` + `transform-messages-dedup`).
|
||||
// A set-based "drop every result for this id" loses the real output for the
|
||||
// surviving valid occurrence whenever one duplicate is malformed. Track each
|
||||
// `toolCall` occurrence's malformed-ness on a per-id queue and pop on the
|
||||
// first unconsumed matching `toolResult` so we drop only the one tied to the
|
||||
// malformed call.
|
||||
// Positional FIFO pairing within one assistant→tool-result window: a tool-call
|
||||
// id can repeat across history when an OpenAI-Responses composite id
|
||||
// (`callId|itemId`) collapses on the wire to the same `callId` (see
|
||||
// `deduplicateToolCallIds` + `transform-messages-dedup`). A set-based "drop
|
||||
// every result for this id" loses the real output for the surviving valid
|
||||
// occurrence whenever one duplicate is malformed. Track each `toolCall`
|
||||
// occurrence's malformed-ness on a per-id queue and pop on matching
|
||||
// `toolResult`, but clear the queues at every non-result boundary so a
|
||||
// malformed call whose rejection result never arrived cannot consume a later
|
||||
// valid call's real result when the id is reused.
|
||||
const dropQueues = new Map<string, boolean[]>();
|
||||
const result: Message[] = [];
|
||||
for (const msg of messages) {
|
||||
if (msg.role === "assistant") {
|
||||
dropQueues.clear();
|
||||
const filtered: AssistantMessage["content"] = [];
|
||||
for (const block of msg.content) {
|
||||
if (block.type === "toolCall") {
|
||||
@@ -189,10 +192,15 @@ function sanitizeMalformedToolCalls(messages: Message[]): Message[] {
|
||||
}
|
||||
if (msg.role === "toolResult") {
|
||||
const queue = dropQueues.get(msg.toolCallId);
|
||||
if (queue && queue.length > 0 && queue.shift() === true) continue;
|
||||
if (queue && queue.length > 0) {
|
||||
const drop = queue.shift() === true;
|
||||
if (queue.length === 0) dropQueues.delete(msg.toolCallId);
|
||||
if (drop) continue;
|
||||
}
|
||||
result.push(msg);
|
||||
continue;
|
||||
}
|
||||
dropQueues.clear();
|
||||
result.push(msg);
|
||||
}
|
||||
return result;
|
||||
|
||||
@@ -206,4 +206,29 @@ describe("transformMessages drops malformed (empty-name) tool calls", () => {
|
||||
expect(toolResults[0]?.content).toEqual([{ type: "text", text: "real file contents" }]);
|
||||
expect(toolResults[0]?.toolName).toBe("read");
|
||||
});
|
||||
|
||||
it("does not let a missing malformed-result consume a later valid call with the same id", () => {
|
||||
const sharedId = "toolu_reused";
|
||||
const messages: Message[] = [
|
||||
{ role: "user", content: "first read", timestamp: 1 },
|
||||
assistant([{ type: "toolCall", id: sharedId, name: "", arguments: {} }], 2),
|
||||
// No `Tool not found` result arrived for the malformed call before the
|
||||
// conversation moved on. That stale malformed occurrence must not drop
|
||||
// the next valid call's real output when the id is reused.
|
||||
{ role: "user", content: "second read", timestamp: 3 },
|
||||
assistant([{ type: "toolCall", id: sharedId, name: "read", arguments: { path: "foo" } }], 4),
|
||||
toolResult(sharedId, "real file contents", 5),
|
||||
];
|
||||
|
||||
const transformed = transformMessages(messages, model);
|
||||
|
||||
const survivingCalls = getToolCalls(transformed);
|
||||
expect(survivingCalls).toHaveLength(1);
|
||||
expect(survivingCalls[0]).toMatchObject({ name: "read" });
|
||||
|
||||
const toolResults = transformed.filter((m): m is ToolResultMessage => m.role === "toolResult");
|
||||
expect(toolResults).toHaveLength(1);
|
||||
expect(toolResults[0]?.content).toEqual([{ type: "text", text: "real file contents" }]);
|
||||
expect(toolResults[0]?.toolName).toBe("read");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user