diff --git a/CHANGELOG.md b/CHANGELOG.md index d446e043..536b88bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,21 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A conversation is no longer stuck after a tool call went unanswered + +A tool that runs in the browser can be torn down while its call is still open, most often because +the tab was closed or reloaded mid-run. The call stayed in the thread with no result, every retry +sent it back up, and the model API refused the whole conversation with `Tool result is missing for +tool call ...`. The next three things the person typed failed identically, and the only way out was +to notice that and start another channel. + +A chat turn now drops a tool call nothing is going to answer before the conversation reaches the +model, which is what routines already did for the history they seed. A result counts as an answer +only if it arrives before the next thing the person or the system said, matching the rule the model +API enforces, so a handler that resolves after the person has typed again no longer looks like an +answer. Ids are never rewritten and the stored thread is untouched, so the transcript still shows +what happened and a call waiting on a resume still gets its result. + ### Coworkers are made in a wizard and managed in a dialog Creating a coworker is now a three-step wizard — who it is, who may see it, then where it runs, diff --git a/server/src/agents/history-sanitize.ts b/server/src/agents/history-sanitize.ts new file mode 100644 index 00000000..c99f3483 --- /dev/null +++ b/server/src/agents/history-sanitize.ts @@ -0,0 +1,153 @@ +/** + * The one filter that keeps a broken conversation from being replayed at a model provider for ever. + * + * IT LIVES HERE BECAUSE BOTH TURN PATHS NEED IT. A routine's headless turn seeds history itself + * (`routines/run-turn.ts`) and a chat turn is handed history by the browser (`copilot.ts`). Both + * hand that history to a `BuiltInAgent`, which converts it and lets the model provider validate the + * call/result pairing. `run-turn.ts` imports `../copilot`, so this cannot live in either of them + * without one importing the other back. + */ +import type { Message, ToolCall } from "@ag-ui/client"; + +/** Whether a message said nothing at all — no text, no parts, nothing to show a person. */ +function isSilent(message: Message): boolean { + const content = (message as { content?: unknown }).content; + if (content === undefined || content === null) return true; + if (typeof content === "string") return content.length === 0; + if (Array.isArray(content)) return content.length === 0; + return false; +} + +/** + * Refuse to re-present a conversation the model API will reject. + * + * FOUND IN PRODUCTION, TWICE, ON BOTH PATHS. First on routines: two firings of one routine, fifteen + * minutes apart, both failed with `Tool result is missing for tool call + * call_TTbiXzJVNifQt8ioU1JJmj4S.` — the SAME call id both times, so it did not come from the live + * turn: the channel's Intelligence thread held an assistant message carrying a tool call whose + * result message never landed, because an earlier CHAT turn was interrupted mid-call. Then on chat + * itself: `AI_MissingToolResultsError: Tool result is missing for tool call + * chatcmpl-tool-8dd56dc7497c5ea9`, thrown three times in a row on one person's next three attempts + * to say anything. There the damaged message was not even durable: a frontend tool handler was torn + * down mid-run, so the live agent's messages in the browser held the call, the store did not, and + * every retry sent the same unanswerable call back up as `input.messages`. + * + * A model provider validates call/result pairing, so one historical dangle poisons EVERY later turn + * that replays it: on routines until the fatigue rule disables the routine, and on chat until the + * person works out for themselves that the conversation is dead and starts another one. A permanent + * failure grown out of transient damage, and nothing the person did wrong. + * + * WHY DROPPING IS THE RIGHT ANSWER, and not repair. History here is CONTEXT for a turn, not a + * transaction to resume. A dangling call is already permanently unanswerable — the tool run that + * would have answered it ended when that turn did, and there is no result to invent. The only two + * options are to seed a conversation the API refuses, or to seed the same conversation minus a call + * that never completed. The second one loses a fragment of an interrupted exchange; the first one + * takes the conversation away. + * + * WHAT THIS DOES NOT DO. It does not DELETE anything from the platform. The thread still holds every + * row and the person still sees the interrupted exchange in their channel. This is a read-side + * filter on one turn's input and nothing more. + * + * IDS ARE NEVER CHANGED, which is what keeps `persistedInputMessages`' id-subtraction in + * `run-turn.ts` correct: a message this pass stripped a tool call from keeps its id and is still + * subtracted out as historic, and a message it dropped was never a candidate to persist. So + * sanitizing cannot turn a firing into one that re-persists the transcript. + * + * The rules, in order: + * 1. A tool call is ANSWERED if some later message carries it as `toolCallId`, or if the caller + * says it is answered elsewhere. Later, not merely present: a result ahead of its call is not a + * pairing any provider accepts either. + * 2. An assistant message keeps only its answered calls. If that leaves it with no calls and + * nothing said, the message is dropped — an empty assistant husk is itself invalid for some + * providers, so stripping the call is not enough. + * 3. A tool result whose `toolCallId` matches no surviving call is dropped: the mirror-image dangle, + * which is what an interruption between the two rows leaves behind in the other order. + * + * Order is preserved, the input array is not mutated, and a message the pass does not change is + * returned as the same object — a healthy thread, which is nearly all of them, goes through + * untouched rather than through a re-normalization that could quietly differ. + * + * @param answeredElsewhere Call ids that are about to be answered by something this history cannot + * see, and so must survive. That is the interrupt resume: `BuiltInAgent.run` appends a tool result + * per `input.resume` entry, keyed by `interruptId`, AFTER converting the messages + * (`@copilotkit/runtime/dist/agent/index.mjs`, the `resumeEntries` block in `run`). Dropping the + * call that the resume answers would turn a resumable interrupt into an orphaned result, which is + * the same error seen from the other side. + */ +export function sanitizeSeededHistory( + history: Message[], + answeredElsewhere: ReadonlySet = new Set(), +): Message[] { + /** Every position that answers a call id, in order. */ + const answersFor = new Map(); + for (const [index, message] of history.entries()) { + const { toolCallId } = message as { toolCallId?: string }; + if (toolCallId === undefined) continue; + answersFor.set(toolCallId, [...(answersFor.get(toolCallId) ?? []), index]); + } + /* + * Where a call's answer may still land: before the next thing a person or the system said. + * + * The model API walks the conversation in order and refuses it the moment a user or system + * message arrives while a call is still unanswered. So a result that turns up after a later + * user message does not answer anything, however real it was. This happened live: a browser + * tool handler resolved late, its result was appended after the person had already typed the + * next message, and the call read as answered here while the API still threw on every retry. + */ + const boundaryAfter: number[] = new Array(history.length).fill( + history.length, + ); + for ( + let index = history.length - 1, next = history.length; + index >= 0; + index -= 1 + ) { + boundaryAfter[index] = next; + const { role } = history[index] as { role?: string }; + if (role === "user" || role === "system" || role === "developer") + next = index; + } + const answeredWithin = (id: string, index: number): boolean => + (answersFor.get(id) ?? []).some( + (at) => at > index && at < (boundaryAfter[index] ?? history.length), + ); + + const surviving = new Set(); + const kept: (Message | undefined)[] = history.map((message, index) => { + const { toolCalls } = message as { toolCalls?: ToolCall[] }; + if (toolCalls === undefined) return message; + + const answered = toolCalls.filter((call) => { + if (answeredElsewhere.has(call.id)) return true; + return answeredWithin(call.id, index); + }); + for (const call of answered) surviving.add(call.id); + + // The husk check goes FIRST so it also catches a row that arrived with no calls and nothing + // said — the same invalid shape, reached without a dangle. + if (answered.length === 0 && isSilent(message)) return undefined; + // The healthy path, and the only one that returns the very same object. + if (answered.length === toolCalls.length) return message; + + /* + * Cast for the same reason `toAgentMessage` casts: `Message` is a union discriminated on `role`, + * and a spread over the union widens past every branch of it. Neither rewrite here can change + * the role or the shape — one narrows the `toolCalls` array, the other removes the key — so + * there is nothing to narrow against and nothing that could stop being a `Message`. + */ + if (answered.length > 0) { + return { ...message, toolCalls: answered } as Message; + } + // Text it did say, minus a call it cannot complete. + const { toolCalls: _dropped, ...rest } = message as Message & { + toolCalls?: ToolCall[]; + }; + return rest as Message; + }); + + return kept.filter((message): message is Message => { + if (message === undefined) return false; + const { toolCallId } = message as { toolCallId?: string }; + return toolCallId === undefined || surviving.has(toolCallId); + }); +} diff --git a/server/src/copilot.ts b/server/src/copilot.ts index 547e329a..f9c567bc 100644 --- a/server/src/copilot.ts +++ b/server/src/copilot.ts @@ -14,6 +14,7 @@ import { COMPUTER_GUIDANCE, PROVENANCE_GUIDANCE, } from "../../shared/bot-prompt"; +import { sanitizeSeededHistory } from "./agents/history-sanitize"; import type { AgentActor } from "./agents/profile-types"; import type { AgentFetch, StallGuard } from "./channels/stall-guard"; import type { DeploymentConfig } from "./config"; @@ -419,7 +420,7 @@ async function buildAgent( * what keeps a narrowed run from being told it holds something it was not offered. */ const withTools = (tools: GrantedTool[]) => - new BuiltInAgent( + new BuiltInAgentWithSaneHistory( builtInAgentConfiguration( agent, model, @@ -666,6 +667,68 @@ function remoteAgentWithStandingRole( return remote; } +/** + * A built-in Bot that will not hand the model provider a conversation it is going to refuse. + * + * FOUND LIVE, ON CHAT. One person's next three messages each failed with + * `AI_MissingToolResultsError: Tool result is missing for tool call chatcmpl-tool-8dd56dc7497c5ea9`, + * thrown out of the AI SDK's `convertToLanguageModelPrompt`. A frontend tool handler had been torn + * down while its call was open, so the agent's live messages in the browser carried an assistant + * message whose tool call never got a result. The durable store did not have it, nothing was going + * to answer it, and every retry sent it straight back up as `input.messages`. The conversation was + * finished until the person worked out for themselves to start another one. + * + * The guard has to be on this side of `run`. `BuiltInAgent.run` converts `input.messages` itself, + * with no seam in between, so wrapping the agent is the only place left to stand. The reasoning for + * why a dangling call is DROPPED rather than repaired, and why ids are never changed, is in + * `agents/history-sanitize.ts`, where the routines path found the same failure first. + * + * A RESUMED CALL IS NOT A DANGLE. `run` appends a tool result for each `input.resume` entry by + * `interruptId` AFTER converting the messages, so a call that a resume is about to answer must + * survive this pass or the appended result lands on nothing. + */ +class BuiltInAgentWithSaneHistory extends BuiltInAgent { + /** + * The configuration, held a second time because the base class keeps its own copy private and + * {@link clone} has to build another one of THIS class rather than of the base. + */ + private readonly configuration: BuiltInAgentConfiguration; + + constructor(configuration: BuiltInAgentConfiguration) { + super(configuration); + this.configuration = configuration; + } + + run(input: RunAgentInput): Observable { + const answeredByResume = new Set( + (input.resume ?? []).map((entry) => entry.interruptId), + ); + return super.run({ + ...input, + messages: sanitizeSeededHistory(input.messages, answeredByResume), + }); + } + + /** + * Carried by hand, for the same reason {@link RunBuiltAgent.clone} is. + * + * The runtime clones an agent before every run, and the base class's clone hard-codes + * `new BuiltInAgent(this.config)`: inherited unchanged, the very first message anybody sends + * would go through an agent that does none of the above. The middleware list is copied because + * the base clone copies it, and it is reached through a cast because `AbstractAgent` declares it + * private. Nothing registers middleware on a built-in Bot today, and this is here so that the day + * something does, it is not lost in a clone. + */ + clone(): BuiltInAgentWithSaneHistory { + const cloned = new BuiltInAgentWithSaneHistory(this.configuration); + type WithMiddlewares = { middlewares: unknown[] }; + (cloned as unknown as WithMiddlewares).middlewares = [ + ...(this as unknown as WithMiddlewares).middlewares, + ]; + return cloned; + } +} + /** * An agent whose tools are decided when the run starts, because that is the first moment anybody * knows what the run is about, and who is asking on whose behalf. diff --git a/server/src/routines/run-turn.ts b/server/src/routines/run-turn.ts index 3a62953d..e3d5929f 100644 --- a/server/src/routines/run-turn.ts +++ b/server/src/routines/run-turn.ts @@ -53,9 +53,9 @@ import type { BaseEvent, Message, RunAgentInput, - ToolCall, } from "@ag-ui/client"; import { EventType } from "@ag-ui/client"; +import { sanitizeSeededHistory } from "../agents/history-sanitize"; import { historyOrEmpty } from "../copilot"; import type { TurnRunner } from "./runner"; @@ -196,104 +196,13 @@ function toAgentMessage(message: ThreadHistoryMessage): Message { } as Message; } -/** Whether a message said nothing at all — no text, no parts, nothing to show a person. */ -function isSilent(message: Message): boolean { - const content = (message as { content?: unknown }).content; - if (content === undefined || content === null) return true; - if (typeof content === "string") return content.length === 0; - if (Array.isArray(content)) return content.length === 0; - return false; -} - /** - * Refuse to re-present a conversation the model API will reject. - * - * FOUND IN PRODUCTION. Two firings of one routine, fifteen minutes apart, both failed with - * `Tool result is missing for tool call call_TTbiXzJVNifQt8ioU1JJmj4S.` — the SAME call id both - * times, so it did not come from the live turn: the channel's Intelligence thread held an assistant - * message carrying a tool call whose result message never landed, because an earlier CHAT turn was - * interrupted mid-call. The seeding below hands the whole converted history to the runner, the model - * provider validates call/result pairing, and it rejects the conversation. One historical dangle - * therefore poisons EVERY future firing in that channel until the fatigue rule disables the routine: - * a permanent failure grown out of transient damage, and nothing the person did wrong. - * - * WHY DROPPING IS THE RIGHT ANSWER, and not repair. History here is CONTEXT for a turn, not a - * transaction to resume. A dangling call is already permanently unanswerable — the tool run that - * would have answered it ended when that chat turn did, and there is no result to invent. The only - * two options are to seed a conversation the API refuses, or to seed the same conversation minus a - * call that never completed. The second one loses a fragment of an interrupted exchange; the first - * one disables a routine forever. - * - * WHAT THIS DOES NOT DO. It does not DELETE anything from the platform. The thread still holds every - * row, the person still sees the interrupted exchange in their channel, and a browser turn is - * unaffected. This is a read-side filter on one turn's input and nothing more. - * - * IDS ARE NEVER CHANGED, which is what keeps `persistedInputMessages`' id-subtraction below correct: - * a message this pass stripped a tool call from keeps its id and is still subtracted out as historic, - * and a message it dropped was never a candidate to persist. So sanitizing cannot turn a firing into - * one that re-persists the transcript. - * - * The rules, in order: - * 1. A tool call is ANSWERED if some later message carries it as `toolCallId`. Later, not merely - * present: a result ahead of its call is not a pairing any provider accepts either. - * 2. An assistant message keeps only its answered calls. If that leaves it with no calls and - * nothing said, the message is dropped — an empty assistant husk is itself invalid for some - * providers, so stripping the call is not enough. - * 3. A tool result whose `toolCallId` matches no surviving call is dropped: the mirror-image dangle, - * which is what an interruption between the two rows leaves behind in the other order. - * - * Order is preserved, the input array is not mutated, and a message the pass does not change is - * returned as the same object — a healthy thread, which is nearly all of them, goes through - * untouched rather than through a re-normalization that could quietly differ. + * Re-exported from `agents/history-sanitize.ts`, where it now lives, because a chat turn needs + * it too and this module cannot be imported from `copilot.ts`, since the import already runs the + * other way. Kept as a name on this module because this is where the reasoning was found and where the + * tests that cover the seeding path still reach for it. */ -export function sanitizeSeededHistory(history: Message[]): Message[] { - /** For each answered call id, the earliest position that answers it. */ - const answeredAt = new Map(); - for (const [index, message] of history.entries()) { - const { toolCallId } = message as { toolCallId?: string }; - if (toolCallId === undefined) continue; - if (!answeredAt.has(toolCallId)) answeredAt.set(toolCallId, index); - } - - const surviving = new Set(); - const kept: (Message | undefined)[] = history.map((message, index) => { - const { toolCalls } = message as { toolCalls?: ToolCall[] }; - if (toolCalls === undefined) return message; - - const answered = toolCalls.filter((call) => { - const at = answeredAt.get(call.id); - return at !== undefined && at > index; - }); - for (const call of answered) surviving.add(call.id); - - // The husk check goes FIRST so it also catches a row that arrived with no calls and nothing - // said — the same invalid shape, reached without a dangle. - if (answered.length === 0 && isSilent(message)) return undefined; - // The healthy path, and the only one that returns the very same object. - if (answered.length === toolCalls.length) return message; - - /* - * Cast for the same reason `toAgentMessage` casts: `Message` is a union discriminated on `role`, - * and a spread over the union widens past every branch of it. Neither rewrite here can change - * the role or the shape — one narrows the `toolCalls` array, the other removes the key — so - * there is nothing to narrow against and nothing that could stop being a `Message`. - */ - if (answered.length > 0) { - return { ...message, toolCalls: answered } as Message; - } - // Text it did say, minus a call it cannot complete. - const { toolCalls: _dropped, ...rest } = message as Message & { - toolCalls?: ToolCall[]; - }; - return rest as Message; - }); - - return kept.filter((message): message is Message => { - if (message === undefined) return false; - const { toolCallId } = message as { toolCallId?: string }; - return toolCallId === undefined || surviving.has(toolCallId); - }); -} +export { sanitizeSeededHistory }; /** What a message said out loud, or nothing if it did not say anything. */ function assistantText(message: Message): string | undefined { diff --git a/server/tests/copilot.test.ts b/server/tests/copilot.test.ts index 6d439464..59da7f0c 100644 --- a/server/tests/copilot.test.ts +++ b/server/tests/copilot.test.ts @@ -1,6 +1,8 @@ import { describe, expect, spyOn, test } from "bun:test"; +import type { RunAgentInput } from "@ag-ui/client"; import { HttpAgent } from "@ag-ui/client"; import { BuiltInAgent } from "@copilotkit/runtime/v2"; +import { EMPTY } from "rxjs"; import { PROVENANCE_GUIDANCE } from "../../shared/bot-prompt"; import { buildAgents, @@ -803,3 +805,182 @@ describe("where a Bot says its answer came from", () => { } }); }); + +/** + * The dangling tool call, refused before it reaches the model provider. + * + * FOUND LIVE. Three consecutive attempts to say anything in one conversation failed with + * `AI_MissingToolResultsError: Tool result is missing for tool call chatcmpl-tool-8dd56dc7497c5ea9`. + * A frontend tool handler had been torn down while its call was open, so the browser's live agent + * messages carried an assistant message whose tool call would never be answered, and each retry sent + * it back up as `input.messages`. `BuiltInAgent.run` converts those messages itself, so the only + * place a guard can stand is in front of it, and these are the properties that say it is standing + * there: on the agent a request is handed, on the clone the runtime makes before every run, and on + * the narrowed path, which builds its agent again per run. + */ +describe("a chat turn is not sent a conversation the model API refuses", () => { + const assistant = { + id: "general-assistant", + name: "General Assistant", + type: "built_in" as const, + systemPrompt: "Be helpful.", + }; + const model = { provider: "openai" as const, defaultModel: "gpt-5.6-terra" }; + + /** The messages a run reaches `BuiltInAgent.run` with, without a model call behind them. */ + function captureRuns() { + const seen: RunAgentInput[] = []; + const spy = spyOn(BuiltInAgent.prototype, "run").mockImplementation( + (input: RunAgentInput) => { + seen.push(input); + return EMPTY; + }, + ); + return { seen, restore: () => spy.mockRestore() }; + } + + function input( + messages: unknown[], + resume?: { interruptId: string; status: "resolved" }[], + ): RunAgentInput { + return { + threadId: "thread_1", + runId: "run_1", + messages: messages as RunAgentInput["messages"], + tools: [], + context: [], + forwardedProps: {}, + state: {}, + ...(resume === undefined ? {} : { resume }), + }; + } + + const danglingCall = [ + { id: "m1", role: "user", content: "Save that." }, + { + id: "m2", + role: "assistant", + content: "Saving it.", + toolCalls: [ + { + id: "chatcmpl-tool-8dd56dc7497c5ea9", + type: "function", + function: { name: "saveDocument", arguments: "{}" }, + }, + ], + }, + { id: "m3", role: "user", content: "Did that work?" }, + ]; + + async function builtIn() { + const agents = await buildAgents([assistant], model, "openai-secret"); + return agents["general-assistant"]; + } + + test("the unanswerable call is gone from what the run converts", async () => { + const agent = await builtIn(); + const { seen, restore } = captureRuns(); + + try { + agent?.run(input(danglingCall)); + } finally { + restore(); + } + + const messages = seen[0]?.messages ?? []; + // Everything the person and the Bot said survives. Only the call nothing will ever answer is + // gone, and with it the message that carried nothing else. + expect(messages.map((message) => message.id)).toEqual(["m1", "m2", "m3"]); + expect(messages[1]).not.toHaveProperty("toolCalls"); + // And the caller's own array is untouched, because the browser goes on using it. + expect(danglingCall[1]).toHaveProperty("toolCalls"); + }); + + test("the clone the runtime runs guards it too", async () => { + // `agents[agentId].clone()` happens before every single run, and the base class's clone builds a + // plain `BuiltInAgent`. Inherited unchanged, the guard would never once be reached in production. + const agent = (await builtIn())?.clone(); + const { seen, restore } = captureRuns(); + + try { + agent?.run(input(danglingCall)); + } finally { + restore(); + } + + expect(seen[0]?.messages).toHaveLength(3); + expect(seen[0]?.messages?.[1]).not.toHaveProperty("toolCalls"); + }); + + test("a call the run is about to resume is kept", async () => { + /* + * `run` appends a tool result per `input.resume` entry, keyed by `interruptId`, AFTER converting + * the messages. So an interrupted call is the one dangle that is not a dangle: dropping it would + * leave that appended result pointing at a call no longer in the conversation, which is the same + * error arriving from the other side. + */ + const agent = await builtIn(); + const { seen, restore } = captureRuns(); + + try { + agent?.run( + input(danglingCall, [ + { interruptId: "chatcmpl-tool-8dd56dc7497c5ea9", status: "resolved" }, + ]), + ); + } finally { + restore(); + } + + expect(seen[0]?.messages?.[1]).toMatchObject({ + toolCalls: [{ id: "chatcmpl-tool-8dd56dc7497c5ea9" }], + }); + }); + + test("the narrowed path is guarded, because it builds its agent the same way", async () => { + // Tool selection defers the build to the run, so this is a different agent object than the one + // the request was handed. It is built through the same `withTools`, and that is the property. + const granted = Array.from({ length: 3 }, (_, index) => ({ + ref: `drive/tool_${index}`, + name: `mcp__drive__tool_${index}`, + description: `drive tool ${index}`, + })) as never[]; + const agents = await buildAgents( + [assistant], + model, + "openai-secret", + undefined, + async () => granted, + undefined, + undefined, + undefined, + { + loadSkills: async () => [ + { + slug: "drive-audit", + title: "Drive audit", + summary: "Read documents out of Google Drive.", + tools: ["drive/tool_0"], + }, + ], + choose: async () => JSON.stringify({ skills: ["drive-audit"] }), + floor: 0, + }, + ); + const { seen, restore } = captureRuns(); + + try { + // Subscribed, because the narrowing wrapper builds the inner agent lazily on subscription. + await new Promise((resolve) => { + agents["general-assistant"] + ?.run(input(danglingCall)) + .subscribe({ complete: resolve, error: () => resolve() }); + }); + } finally { + restore(); + } + + expect(seen[0]?.messages).toHaveLength(3); + expect(seen[0]?.messages?.[1]).not.toHaveProperty("toolCalls"); + }); +}); diff --git a/server/tests/history-sanitize.test.ts b/server/tests/history-sanitize.test.ts new file mode 100644 index 00000000..7be5362d --- /dev/null +++ b/server/tests/history-sanitize.test.ts @@ -0,0 +1,238 @@ +import { describe, expect, test } from "bun:test"; +import type { Message } from "@ag-ui/client"; +import { sanitizeSeededHistory } from "../src/agents/history-sanitize"; + +/** + * The filter itself, asserted as a function rather than through either turn path. + * + * Both callers reach it through machinery of their own: a routine seeds history off the platform + * (`routine-run-turn.test.ts`) and a chat turn is handed it by the browser (`copilot.test.ts`). Those + * files assert that the guard IS applied where it has to be. The rules it applies are asserted once, + * here, because they are the same rules on both paths and neither path is a good place to enumerate + * them. + */ + +/** History rows are written in the platform's shape and cast once, as the callers do. */ +function history(rows: unknown[]): Message[] { + return rows as Message[]; +} + +function call(id: string, name = "search", args = "{}") { + return { id, type: "function", function: { name, arguments: args } }; +} + +describe("sanitizeSeededHistory", () => { + test("drops an unanswered call and keeps the answered one, with all text intact", () => { + const sanitized = sanitizeSeededHistory( + history([ + { id: "m1", role: "user", content: "Look two things up." }, + { + id: "m2", + role: "assistant", + content: "Looking them up.", + toolCalls: [call("call_answered"), call("call_dangling")], + }, + { + id: "m3", + role: "tool", + content: "found x", + toolCallId: "call_answered", + }, + { id: "m4", role: "assistant", content: "Here is x." }, + ]), + ); + + expect(sanitized.map((message) => message.id)).toEqual([ + "m1", + "m2", + "m3", + "m4", + ]); + expect(sanitized[1]).toMatchObject({ + content: "Looking them up.", + toolCalls: [call("call_answered")], + }); + }); + + test("drops an assistant message whose only content was a dangling call", () => { + // An assistant row with neither text nor tool calls is itself invalid for some providers, so + // stripping the call is not enough: the husk has to go too. + const sanitized = sanitizeSeededHistory( + history([ + { id: "m1", role: "user", content: "Look it up." }, + { id: "m2", role: "assistant", toolCalls: [call("call_dangling")] }, + { id: "m3", role: "user", content: "Anything?" }, + ]), + ); + + expect(sanitized.map((message) => message.id)).toEqual(["m1", "m3"]); + }); + + test("keeps text a message did say, minus the call it cannot complete", () => { + const sanitized = sanitizeSeededHistory( + history([ + { + id: "m1", + role: "assistant", + content: "Let me check.", + toolCalls: [call("call_dangling")], + }, + ]), + ); + + expect(sanitized).toHaveLength(1); + expect(sanitized[0]).toEqual({ + id: "m1", + role: "assistant", + content: "Let me check.", + } as unknown as Message); + }); + + test("drops an orphaned tool result", () => { + // The mirror-image dangle: a result whose call is not in the history at all. + const sanitized = sanitizeSeededHistory( + history([ + { id: "m1", role: "user", content: "Hello." }, + { id: "m2", role: "tool", content: "left over", toolCallId: "gone" }, + { id: "m3", role: "assistant", content: "Hello back." }, + ]), + ); + + expect(sanitized.map((message) => message.id)).toEqual(["m1", "m3"]); + }); + + test("a result that lands after a later user message answers nothing", () => { + const history = [ + { id: "u1", role: "user", content: "do it" }, + { + id: "a1", + role: "assistant", + content: "", + toolCalls: [ + { + id: "late", + type: "function", + function: { name: "x", arguments: "{}" }, + }, + ], + }, + { id: "u2", role: "user", content: "also this" }, + { + id: "t1", + role: "tool", + toolCallId: "late", + content: "arrived too late", + }, + ] as unknown as Message[]; + const out = sanitizeSeededHistory(history); + expect(out.map((m) => m.id)).toEqual(["u1", "u2"]); + }); + + test("a result ahead of its own call answers nothing", () => { + // Position, not mere presence: no provider accepts a result that arrives before the call it + // belongs to, so a history in that order is still a history to be repaired. + const sanitized = sanitizeSeededHistory( + history([ + { id: "m1", role: "tool", content: "early", toolCallId: "call_1" }, + { id: "m2", role: "assistant", toolCalls: [call("call_1")] }, + ]), + ); + + expect(sanitized).toEqual([]); + }); + + test("a clean history passes through unchanged, object for object", () => { + const clean = history([ + { id: "m1", role: "user", content: "Look it up." }, + { + id: "m2", + role: "assistant", + content: "Looking it up.", + toolCalls: [call("call_1")], + }, + { id: "m3", role: "tool", content: "found", toolCallId: "call_1" }, + { id: "m4", role: "assistant", content: "Here it is." }, + ]); + const snapshot = structuredClone(clean); + + const sanitized = sanitizeSeededHistory(clean); + + // Nothing reordered, nothing rewritten, and not even reallocated, so there is no room for a + // silent normalization to creep in on the overwhelmingly common healthy-thread path. + expect(sanitized).toEqual(snapshot); + for (const [index, message] of sanitized.entries()) { + expect(message).toBe(clean[index] as Message); + } + // And the caller's array was not mutated underneath it. + expect(clean).toEqual(snapshot); + }); +}); + +/* + * The second parameter is the interrupt resume, and it exists because the runtime answers those + * calls AFTER this pass has run. Dropping one would leave the appended tool result pointing at a + * call that is no longer in the conversation: the same error, reached from the other side. + */ +describe("a call answered elsewhere survives", () => { + test("an id named by the caller is kept, and the message is the very same object", () => { + const rows = history([ + { id: "m1", role: "user", content: "Book it." }, + { + id: "m2", + role: "assistant", + content: "Asking first.", + toolCalls: [call("interrupt_1", "confirm")], + }, + ]); + + const sanitized = sanitizeSeededHistory(rows, new Set(["interrupt_1"])); + + expect(sanitized).toHaveLength(2); + expect(sanitized[1]).toBe(rows[1] as Message); + }); + + test("it also saves a message that would otherwise have been dropped as a husk", () => { + const rows = history([ + { id: "m1", role: "assistant", toolCalls: [call("interrupt_1")] }, + ]); + + expect(sanitizeSeededHistory(rows, new Set(["interrupt_1"]))).toHaveLength( + 1, + ); + // And without the resume it is the husk case again, which is what makes the parameter load-bearing. + expect(sanitizeSeededHistory(rows)).toEqual([]); + }); + + test("only the named ids are spared, alongside the ones the history answers", () => { + const sanitized = sanitizeSeededHistory( + history([ + { + id: "m1", + role: "assistant", + content: "Two things.", + toolCalls: [ + call("interrupt_1"), + call("call_answered"), + call("call_dangling"), + ], + }, + { id: "m2", role: "tool", content: "ok", toolCallId: "call_answered" }, + ]), + new Set(["interrupt_1"]), + ); + + expect(sanitized[0]).toMatchObject({ + toolCalls: [call("interrupt_1"), call("call_answered")], + }); + }); + + test("an empty set is the default, and changes nothing", () => { + const rows = history([ + { id: "m1", role: "assistant", toolCalls: [call("call_dangling")] }, + ]); + + expect(sanitizeSeededHistory(rows, new Set())).toEqual( + sanitizeSeededHistory(rows), + ); + }); +});