diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c1ce54b5..a5f0ec41 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -107,6 +107,10 @@ jobs: # the real entrypoint, so its dependencies have to be installed as well. - run: bun install --frozen-lockfile working-directory: agent-bot + # Same for the LangGraph Bot: its history tests import @langchain/core, which lives in that + # Bot's own tree and not in the root one. + - run: bun install --frozen-lockfile + working-directory: agent-langgraph # Not the db:migrate script: that one loads ../.env, which does not exist in CI. DATABASE_URL # comes from the job env instead, which drizzle.config.ts already reads. - run: bunx drizzle-kit migrate --config=drizzle.config.ts diff --git a/agent-langgraph/src/history.ts b/agent-langgraph/src/history.ts new file mode 100644 index 00000000..f81e34ed --- /dev/null +++ b/agent-langgraph/src/history.ts @@ -0,0 +1,124 @@ +/** + * The conversation AG-UI carries, as LangChain's message classes. + * + * 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-computer/src/control.ts` + * was split out for the same reason, to keep state-machine tests away from a browser. + */ +import type { RunAgentInput } from "@ag-ui/core"; +import { + AIMessage, + type BaseMessage, + HumanMessage, + SystemMessage, + ToolMessage, +} from "@langchain/core/messages"; +import { COMPUTER_GUIDANCE } 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. + */ +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."; + +/** Translate the conversation AG-UI carries into LangChain's message classes. */ +export function toLangChainMessages(input: RunAgentInput): BaseMessage[] { + const messages: BaseMessage[] = [new SystemMessage(COMPUTER_GUIDANCE)]; + + /* + * Which calls in this history were ever answered. + * + * 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. + * + * OpenAI rejects 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 was not merely + * stuck on that request, it was finished. Every later message failed the same way, and the only + * escape was starting a new one, which loses it. + * + * Collected up front because an answer arrives as a later message than the call it answers. + */ + 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(new HumanMessage(String(message.content ?? ""))); + continue; + } + if (message.role === "system" || message.role === "developer") { + messages.push(new SystemMessage(String(message.content ?? ""))); + continue; + } + if (message.role === "tool") { + // Tool results are appended so the model can continue from the completed call. + messages.push( + new ToolMessage({ + tool_call_id: message.toolCallId, + content: String(message.content ?? ""), + }), + ); + continue; + } + if (message.role === "assistant") { + messages.push( + new AIMessage({ + content: message.content ?? "", + tool_calls: + message.toolCalls?.map((call) => ({ + id: call.id, + name: call.function.name, + // LangChain wants parsed arguments where AG-UI carries the raw string. A call whose + // arguments did not parse is passed as empty rather than dropped: the model needs to + // see that it made the call, or it makes it again. + args: parseArguments(call.function.arguments), + })) ?? [], + }), + ); + + /* + * 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( + new ToolMessage({ + tool_call_id: call.id, + content: NO_ANSWER_CAME, + name: call.function.name, + }), + ); + } + } + } + } + + return messages; +} + +function parseArguments(raw: string): Record { + try { + const parsed = JSON.parse(raw || "{}"); + return typeof parsed === "object" && parsed !== null + ? (parsed as Record) + : {}; + } catch { + return {}; + } +} diff --git a/agent-langgraph/src/index.ts b/agent-langgraph/src/index.ts index 61361e09..3d183e1a 100644 --- a/agent-langgraph/src/index.ts +++ b/agent-langgraph/src/index.ts @@ -1,13 +1,7 @@ import type { BaseEvent, RunAgentInput } from "@ag-ui/core"; import { EventEncoder } from "@ag-ui/encoder"; import { ChatAnthropic } from "@langchain/anthropic"; -import { - AIMessage, - type BaseMessage, - HumanMessage, - SystemMessage, - ToolMessage, -} from "@langchain/core/messages"; +import { type AIMessage, ToolMessage } from "@langchain/core/messages"; import { ChatGoogleGenerativeAI } from "@langchain/google-genai"; import { END, @@ -18,7 +12,7 @@ import { import { ChatOpenAI } from "@langchain/openai"; import { serve } from "bun"; import { hasManagedAgentToken } from "../../shared/agent-authorisation"; -import { COMPUTER_GUIDANCE } from "../../shared/bot-prompt"; +import { toLangChainMessages } from "./history"; /** * The same Bot, on a framework. @@ -127,61 +121,6 @@ if (!API_KEY) { process.exit(1); } -/** Translate the conversation AG-UI carries into LangChain's message classes. */ -function toLangChainMessages(input: RunAgentInput): BaseMessage[] { - const messages: BaseMessage[] = [new SystemMessage(COMPUTER_GUIDANCE)]; - - for (const message of input.messages) { - if (message.role === "user") { - messages.push(new HumanMessage(String(message.content ?? ""))); - continue; - } - if (message.role === "system" || message.role === "developer") { - messages.push(new SystemMessage(String(message.content ?? ""))); - continue; - } - if (message.role === "tool") { - // Tool results are appended so the model can continue from the completed call. - messages.push( - new ToolMessage({ - tool_call_id: message.toolCallId, - content: String(message.content ?? ""), - }), - ); - continue; - } - if (message.role === "assistant") { - messages.push( - new AIMessage({ - content: message.content ?? "", - tool_calls: - message.toolCalls?.map((call) => ({ - id: call.id, - name: call.function.name, - // LangChain wants parsed arguments where AG-UI carries the raw string. A call whose - // arguments did not parse is passed as empty rather than dropped: the model needs to - // see that it made the call, or it makes it again. - args: parseArguments(call.function.arguments), - })) ?? [], - }), - ); - } - } - - return messages; -} - -function parseArguments(raw: string): Record { - try { - const parsed = JSON.parse(raw || "{}"); - return typeof parsed === "object" && parsed !== null - ? (parsed as Record) - : {}; - } catch { - return {}; - } -} - /** * Every tool comes from the caller. This service publishes none of its own, on purpose. * diff --git a/agent-langgraph/tests/history.test.ts b/agent-langgraph/tests/history.test.ts new file mode 100644 index 00000000..e910c3f4 --- /dev/null +++ b/agent-langgraph/tests/history.test.ts @@ -0,0 +1,106 @@ +import { describe, expect, test } from "bun:test"; +import { AIMessage, ToolMessage } from "@langchain/core/messages"; +import type { RunAgentInput } from "@ag-ui/core"; +import { NO_ANSWER_CAME, toLangChainMessages } from "../src/history"; + +/** + * A tool call nobody answered does not end the conversation. + * + * A 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 sits in the history with nothing following it. + * + * OpenAI rejects that 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 was not stuck on that one + * request, it was finished: every later message failed identically, and the only way out was a new + * conversation, which loses this one. + */ +const input = (messages: unknown[]): RunAgentInput => + ({ messages }) as unknown as RunAgentInput; + +const assistantAsking = { + role: "assistant", + content: "", + toolCalls: [ + { + id: "call_1", + function: { name: "computer_request_help", arguments: "{}" }, + }, + ], +}; + +describe("history with a tool call nobody answered", () => { + test("closes it, so the next turn is not rejected", () => { + const messages = toLangChainMessages( + input([{ role: "user", content: "open a page" }, assistantAsking]), + ); + + const closing = messages.find( + (message): message is ToolMessage => + message instanceof ToolMessage && + (message as ToolMessage).tool_call_id === "call_1", + ); + expect(closing).toBeDefined(); + expect(String(closing?.content)).toBe(NO_ANSWER_CAME); + }); + + test("puts the result immediately after the call that made it", () => { + /* + * Position is the requirement, not merely presence. A tool result has to follow the assistant + * message carrying the call; appended at the end of a longer history it would be rejected for + * the same reason the missing one was. + */ + const messages = toLangChainMessages( + input([ + { role: "user", content: "open a page" }, + assistantAsking, + { role: "user", content: "never mind, what is 17 times 3?" }, + ]), + ); + + const asked = messages.findIndex((m) => m instanceof AIMessage); + expect(messages[asked + 1]).toBeInstanceOf(ToolMessage); + }); + + test("leaves a call that was answered alone", () => { + // The ordinary path. Inventing a second result for a call that already has one would tell the + // model its tool ran twice. + const messages = toLangChainMessages( + input([ + assistantAsking, + { role: "tool", toolCallId: "call_1", content: "the real answer" }, + ]), + ); + + const results = messages.filter( + (m): m is ToolMessage => m instanceof ToolMessage, + ); + expect(results).toHaveLength(1); + expect(String(results[0]?.content)).toBe("the real answer"); + }); + + test("closes only the calls that are missing one", () => { + const messages = toLangChainMessages( + input([ + { + role: "assistant", + content: "", + toolCalls: [ + { id: "answered", function: { name: "a", arguments: "{}" } }, + { id: "orphan", function: { name: "b", arguments: "{}" } }, + ], + }, + { role: "tool", toolCallId: "answered", content: "real" }, + ]), + ); + + const byId = new Map( + messages + .filter((m): m is ToolMessage => m instanceof ToolMessage) + .map((m) => [m.tool_call_id, String(m.content)]), + ); + expect(byId.get("answered")).toBe("real"); + expect(byId.get("orphan")).toBe(NO_ANSWER_CAME); + }); +});