diff --git a/agent-bot/src/history.ts b/agent-bot/src/history.ts new file mode 100644 index 00000000..5a3e4920 --- /dev/null +++ b/agent-bot/src/history.ts @@ -0,0 +1,88 @@ +/** + * The conversation AG-UI carries, as the shape the model provider expects. + * + * Its own module so it can be tested without starting a server: `index.ts` calls `serve()` at module + * scope, so importing it to reach one pure function binds a port. `agent-langgraph/src/history.ts` + * and `agent-computer/src/control.ts` were split out for the same reason. + */ +import type { RunAgentInput } from "@ag-ui/core"; +import type OpenAI from "openai"; +import { COMPUTER_GUIDANCE, NO_ANSWER_CAME } from "../../shared/bot-prompt"; + +export { NO_ANSWER_CAME }; + +/** Translate the conversation AG-UI carries into the shape the model provider expects. */ +export function toProviderMessages( + input: RunAgentInput, +): OpenAI.Chat.ChatCompletionMessageParam[] { + const messages: OpenAI.Chat.ChatCompletionMessageParam[] = [ + { role: "system", content: COMPUTER_GUIDANCE }, + ]; + + /* + * Which calls in this history were ever answered. + * + * Collected up front because an answer arrives as a later message than the call it answers. See + * `NO_ANSWER_CAME` for what happens to a conversation carrying a call nothing ever answered. + */ + const answered = new Set( + input.messages + .filter((message) => message.role === "tool") + .map((message) => (message as { toolCallId?: string }).toolCallId) + .filter((id): id is string => Boolean(id)), + ); + + for (const message of input.messages) { + if (message.role === "user") { + messages.push({ role: "user", content: String(message.content ?? "") }); + continue; + } + if (message.role === "system" || message.role === "developer") { + messages.push({ role: "system", content: String(message.content ?? "") }); + continue; + } + if (message.role === "tool") { + // Tool results are appended so the model can continue from the completed call. + messages.push({ + role: "tool", + tool_call_id: message.toolCallId, + content: String(message.content ?? ""), + }); + continue; + } + if (message.role === "assistant") { + const toolCalls = message.toolCalls?.map((call) => ({ + id: call.id, + type: "function" as const, + function: { + name: call.function.name, + arguments: call.function.arguments, + }, + })); + messages.push({ + role: "assistant", + content: message.content ?? null, + ...(toolCalls?.length ? { tool_calls: toolCalls } : {}), + }); + + /* + * Close any of its calls that nothing ever answered, immediately after it. + * + * Position is not cosmetic: a tool result has to follow the assistant message that made the + * call, so these go here rather than being appended at the end. A call answered later in the + * history is left alone and its real answer arrives in its own turn. + */ + for (const call of message.toolCalls ?? []) { + if (call.id && !answered.has(call.id)) { + messages.push({ + role: "tool", + tool_call_id: call.id, + content: NO_ANSWER_CAME, + }); + } + } + } + } + + return messages; +} diff --git a/agent-bot/src/index.ts b/agent-bot/src/index.ts index 962d8381..da4d18a0 100644 --- a/agent-bot/src/index.ts +++ b/agent-bot/src/index.ts @@ -3,7 +3,7 @@ import { EventEncoder } from "@ag-ui/encoder"; import { serve } from "bun"; import OpenAI from "openai"; import { hasManagedAgentToken } from "../../shared/agent-authorisation"; -import { COMPUTER_GUIDANCE } from "../../shared/bot-prompt"; +import { toProviderMessages } from "./history"; /** * The built-in Bot is an AG-UI HTTP service registered the same way as any customer-provided Bot. @@ -67,50 +67,6 @@ const openai = new OpenAI({ baseURL: BASE_URL, }); -/** Translate the conversation AG-UI carries into the shape the model provider expects. */ -function toProviderMessages(input: RunAgentInput) { - const messages: OpenAI.Chat.ChatCompletionMessageParam[] = [ - { role: "system", content: COMPUTER_GUIDANCE }, - ]; - - for (const message of input.messages) { - if (message.role === "user") { - messages.push({ role: "user", content: String(message.content ?? "") }); - continue; - } - if (message.role === "system" || message.role === "developer") { - messages.push({ role: "system", content: String(message.content ?? "") }); - continue; - } - if (message.role === "tool") { - // Tool results are appended so the model can continue from the completed call. - messages.push({ - role: "tool", - tool_call_id: message.toolCallId, - content: String(message.content ?? ""), - }); - continue; - } - if (message.role === "assistant") { - const toolCalls = message.toolCalls?.map((call) => ({ - id: call.id, - type: "function" as const, - function: { - name: call.function.name, - arguments: call.function.arguments, - }, - })); - messages.push({ - role: "assistant", - content: message.content ?? null, - ...(toolCalls?.length ? { tool_calls: toolCalls } : {}), - }); - } - } - - return messages; -} - /** Every tool comes from the caller. This service publishes none of its own, on purpose. */ function toProviderTools(input: RunAgentInput) { if (!input.tools?.length) return undefined; diff --git a/agent-bot/tests/history.test.ts b/agent-bot/tests/history.test.ts new file mode 100644 index 00000000..1318c638 --- /dev/null +++ b/agent-bot/tests/history.test.ts @@ -0,0 +1,148 @@ +import { describe, expect, test } from "bun:test"; +import type { RunAgentInput } from "@ag-ui/core"; +import { NO_ANSWER_CAME, toProviderMessages } from "../src/history"; + +/** + * The Bot that ships in the box, and the conversation a declined handover used to end. + * + * `agent-langgraph` was fixed for this and `agent-bot` was not, so the Bot behind the Browser Bot + * went on failing in exactly the same way. Found by driving it: take the wheel at a sign-in wall, + * decline to finish, and the next turn answers + * + * 400 An assistant message with 'tool_calls' must be followed by tool messages responding to each + * 'tool_call_id' + * + * on screen, in red, for every message after it. These are the same four cases the other Bot has, + * against this one's provider shape. + */ + +type Message = RunAgentInput["messages"][number]; + +function input(messages: Message[]): RunAgentInput { + return { messages } as RunAgentInput; +} + +function call(id: string, name = "computer_request_help") { + return { id, type: "function" as const, function: { name, arguments: "{}" } }; +} + +/** The system prompt is always first and is not what any of this is about. */ +function withoutGuidance(messages: ReturnType) { + return messages.slice(1); +} + +describe("a tool call nothing ever answered", () => { + test("is answered, so the next turn is not refused outright", () => { + const messages = withoutGuidance( + toProviderMessages( + input([ + { + id: "1", + role: "user", + content: "Read my display name.", + } as Message, + { + id: "2", + role: "assistant", + content: "", + toolCalls: [call("c1")], + } as unknown as Message, + { + id: "3", + role: "user", + content: "Never mind. What is 17 times 3?", + } as Message, + ]), + ), + ); + + const answer = messages.find( + (m) => + m.role === "tool" && + (m as { tool_call_id?: string }).tool_call_id === "c1", + ); + expect(answer).toBeDefined(); + expect((answer as { content?: string }).content).toBe(NO_ANSWER_CAME); + }); + + test("says no result rather than inventing a successful one", () => { + // A fake success would have the Bot report reading a page it never reached. + expect(NO_ANSWER_CAME.toLowerCase()).toContain("no result"); + expect(NO_ANSWER_CAME.toLowerCase()).toContain( + "do not assume it succeeded", + ); + }); + + test("lands directly after the assistant message that made it", () => { + /* + * Position is the requirement, not presence. A provider matches a tool result to the assistant + * message it follows, so an answer appended at the end of the history fixes nothing. + */ + const messages = withoutGuidance( + toProviderMessages( + input([ + { id: "1", role: "user", content: "Go." } as Message, + { + id: "2", + role: "assistant", + content: "", + toolCalls: [call("c1")], + } as unknown as Message, + { id: "3", role: "user", content: "Stop." } as Message, + ]), + ), + ); + + const assistantAt = messages.findIndex((m) => m.role === "assistant"); + expect(messages[assistantAt + 1]?.role).toBe("tool"); + expect( + (messages[assistantAt + 1] as { tool_call_id?: string }).tool_call_id, + ).toBe("c1"); + }); + + test("a call that was answered keeps its real answer and gains nothing", () => { + const messages = withoutGuidance( + toProviderMessages( + input([ + { + id: "1", + role: "assistant", + content: "", + toolCalls: [call("c1", "computer_navigate")], + } as unknown as Message, + { + id: "2", + role: "tool", + toolCallId: "c1", + content: "Example Domain", + } as unknown as Message, + ]), + ), + ); + + const answers = messages.filter((m) => m.role === "tool"); + expect(answers).toHaveLength(1); + expect((answers[0] as { content?: string }).content).toBe("Example Domain"); + }); + + test("several unanswered calls in one message each get their own answer", () => { + // A provider names every unanswered id, not just the first, so closing one is not enough. + const messages = withoutGuidance( + toProviderMessages( + input([ + { + id: "1", + role: "assistant", + content: "", + toolCalls: [call("c1"), call("c2", "computer_snapshot")], + } as unknown as Message, + ]), + ), + ); + + const ids = messages + .filter((m) => m.role === "tool") + .map((m) => (m as { tool_call_id?: string }).tool_call_id); + expect(ids).toEqual(["c1", "c2"]); + }); +}); diff --git a/agent-langgraph/src/history.ts b/agent-langgraph/src/history.ts index f81e34ed..e95f376a 100644 --- a/agent-langgraph/src/history.ts +++ b/agent-langgraph/src/history.ts @@ -13,19 +13,13 @@ import { SystemMessage, ToolMessage, } from "@langchain/core/messages"; -import { COMPUTER_GUIDANCE } from "../../shared/bot-prompt"; +import { COMPUTER_GUIDANCE, NO_ANSWER_CAME } from "../../shared/bot-prompt"; -/** - * What a tool call is given when its answer never came. - * - * Written for the model rather than for a log, because the model is the only reader: it has to - * understand that the call is over and not worth waiting for, and be able to say something useful - * about it. "Nothing happened" would leave it repeating the request. +/* + * Re-exported so this module's own tests and callers keep reading it from here, while the wording + * itself lives in `shared` where the other Bot can reach it. Both have to say the same thing. */ -export const NO_ANSWER_CAME = - "No result. The person did not answer this, and the run it belonged to has ended. " + - "Do not wait for it and do not assume it succeeded. Carry on without it, and say plainly what " + - "you could not do if it mattered."; +export { NO_ANSWER_CAME }; /** Translate the conversation AG-UI carries into LangChain's message classes. */ export function toLangChainMessages(input: RunAgentInput): BaseMessage[] { diff --git a/shared/bot-prompt.ts b/shared/bot-prompt.ts index 49a5c306..7b487d40 100644 --- a/shared/bot-prompt.ts +++ b/shared/bot-prompt.ts @@ -137,3 +137,29 @@ export const PROVENANCE_GUIDANCE = PROVENANCE_GUIDANCE_LINES.reduce( }, [""], ).join("\n\n"); + +/** + * What a tool call is given when its answer never came. + * + * A tool call the surface owns ends the run without a result on purpose: the surface draws it, or + * puts it to a person, and starts the next run carrying the answer. When nobody answers — a Bot asks + * for the wheel to get past a sign-in and the person decides they do not need it after all — no + * answer is ever carried, and the call stays in the history with nothing following it. + * + * Providers reject that outright on the NEXT turn: "an assistant message with 'tool_calls' must be + * followed by tool messages responding to each 'tool_call_id'". So the conversation is not merely + * stuck on that one request, it is finished, and the only escape is starting a new one. + * + * Written for the model rather than for a log, because the model is the only reader: it has to + * understand the call is over and not worth waiting for, and be able to say something useful about + * it. "Nothing happened" would leave it repeating the request, and a fake success would have it + * report work it never did. + * + * Shared because both Bots in this repo have to say the same thing. The first fix for this landed in + * `agent-langgraph` alone, and `agent-bot` — the Bot that ships in the box, and the one behind the + * Browser Bot — went on failing in exactly the same way until somebody drove it. + */ +export const NO_ANSWER_CAME = + "No result. The person did not answer this, and the run it belonged to has ended. " + + "Do not wait for it and do not assume it succeeded. Carry on without it, and say plainly what " + + "you could not do if it mattered.";