diff --git a/src/lib/server/giellie-agent.test.ts b/src/lib/server/giellie-agent.test.ts index 10e51d6..bad539e 100644 --- a/src/lib/server/giellie-agent.test.ts +++ b/src/lib/server/giellie-agent.test.ts @@ -53,7 +53,12 @@ function setup(turns: Block[][], over: Partial = {}) { const client = fakeClient(turns); const events: GielieEvent[] = []; const callTool = vi.fn(async () => ({ text: '{"ok":true}', isError: false })); - const checkpoint = vi.fn(async () => {}); + const order: string[] = []; + const snaps: Array<{ messages: StoredMessage[]; pending: PendingAction | null }> = []; + const checkpoint = vi.fn(async (m: StoredMessage[], p: PendingAction | null) => { + order.push("checkpoint"); + snaps.push({ messages: structuredClone(m), pending: p ? structuredClone(p) : null }); + }); const deps: AgentDeps = { client, model: "m", @@ -61,7 +66,10 @@ function setup(turns: Block[][], over: Partial = {}) { system: "sys", tools: TOOLS, callTool, - emit: (e) => events.push(e), + emit: (e) => { + order.push(`emit:${e.type}`); + events.push(e); + }, checkpoint, trust: false, ...over, @@ -69,7 +77,7 @@ function setup(turns: Block[][], over: Partial = {}) { const messages: StoredMessage[] = [ { role: "user", content: [text("hi")], ts: "t" }, ]; - return { client, events, callTool, checkpoint, deps, messages }; + return { client, events, callTool, checkpoint, deps, messages, snaps, order }; } describe("runAgent", () => { @@ -105,7 +113,16 @@ describe("runAgent", () => { { id: "t1", name: "orders.update", input: { id: "a" }, confirmAlways: false }, ]); expect(ev.pending.order).toEqual(["t1"]); - expect(s.checkpoint).toHaveBeenLastCalledWith(s.messages, ev.pending); + const last = s.snaps.at(-1)!; + expect(last.messages).toEqual(s.messages); + expect(last.pending).toEqual(ev.pending); + }); + + it("persists the pending action before showing the approval card", async () => { + const s = setup([[use("t1", "orders__update")]]); + await runAgent(s.messages, s.deps); + expect(s.order.indexOf("checkpoint")).toBeGreaterThanOrEqual(0); + expect(s.order.indexOf("checkpoint")).toBeLessThan(s.order.indexOf("emit:approval_required")); }); it("keeps read results in pending.completed when a write pauses the turn", async () => { @@ -158,19 +175,70 @@ describe("runAgent", () => { expect(String((res?.content[0] as unknown as { content: string }).content)).toContain("boom"); }); - it("stops at maxTurns", async () => { + it("stops at maxTurns with an assistant note so roles keep alternating", async () => { const s = setup([[use("t1", "orders__list")], [use("t2", "orders__list")]], { maxTurns: 2 }); expect(await runAgent(s.messages, s.deps)).toBe("max_turns"); + const last = s.messages.at(-1)!; + const note = 'Stopped after 2 steps without finishing. Say "continue" and I\'ll pick up where I left off.'; + expect(last.role).toBe("assistant"); + expect(last.content).toEqual([{ type: "text", text: note }]); + expect(s.events).toContainEqual({ type: "text", delta: note }); + expect(s.snaps.at(-1)!.messages.at(-1)!.role).toBe("assistant"); + expect(s.snaps.at(-1)!.pending).toBeNull(); }); - it("does not checkpoint a dangling tool_use when the model call fails", async () => { + it("checkpoints only consistent states when the model call fails", async () => { const s = setup([[use("t1", "orders__list")]]); // Second model call is unscripted -> throws. await expect(runAgent(s.messages, s.deps)).rejects.toThrow(); - for (const call of s.checkpoint.mock.calls as unknown as Array<[StoredMessage[], unknown]>) { - const last = call[0].at(-1)!; - expect(last.content.some((b) => b.type === "tool_use") && last.role === "assistant").toBe(false); + expect(s.snaps.length).toBeGreaterThanOrEqual(1); + for (const snap of s.snaps) { + const last = snap.messages.at(-1)!; + const danglingToolUse = last.role === "assistant" && last.content.some((b) => b.type === "tool_use"); + // Only an approval pause may persist a tool_use without its result. + if (danglingToolUse) expect(snap.pending).not.toBeNull(); } + expect(s.snaps.some((snap) => snap.messages.at(-1)!.role === "user" && snap.messages.length === 3)).toBe(true); + // Nothing was ever checkpointed with an unanswered tool_use and no pending. + expect( + s.snaps.some((snap) => { + const last = snap.messages.at(-1)!; + return snap.pending === null && last.role === "assistant" && last.content.some((b) => b.type === "tool_use"); + }), + ).toBe(false); + }); + + it("executes nothing when the response was cut off by max_tokens", async () => { + const s = setup([[use("t1", "orders__list"), use("w1", "orders__update")], [text("ok")]], { + maxTurns: 5, + }); + // Script stop_reason on the first response. + const orig = s.deps.client.messages.stream; + let n = 0; + s.deps.client.messages.stream = (params) => { + const st = orig(params); + if (n++ === 0) { + return { on: (ev, cb) => (st.on(ev, cb), st), finalMessage: async () => ({ ...(await st.finalMessage()), stop_reason: "max_tokens" }) }; + } + return st; + }; + const out = await runAgent(s.messages, s.deps); + expect(out).toBe("done"); + expect(s.callTool).not.toHaveBeenCalled(); + expect(s.events.some((e) => e.type === "approval_required")).toBe(false); + const res = s.messages.find((m) => m.role === "user" && m.content[0].type === "tool_result")!; + expect(res.content.map((b) => b.tool_use_id)).toEqual(["t1", "w1"]); + for (const b of res.content) { + expect(b).toMatchObject({ is_error: true }); + expect(String(b.content)).toContain("cut off"); + } + }); + + it("replaces an empty model response with a placeholder", async () => { + const s = setup([[]]); + const out = await runAgent(s.messages, s.deps); + expect(out).toBe("done"); + expect(s.messages.at(-1)!.content).toEqual([{ type: "text", text: "(no response)" }]); }); }); @@ -190,6 +258,73 @@ describe("resolvePending", () => { expect(last.content.map((b) => b.tool_use_id)).toEqual(["r1", "w1"]); }); + it("checkpoints the resolved state once with pending cleared", async () => { + const s = setup([]); + await resolvePending(s.messages, pending, true, s.deps); + expect(s.checkpoint).toHaveBeenCalledTimes(1); + const snap = s.snaps[0]; + expect(snap.pending).toBeNull(); + const last = snap.messages.at(-1)!; + expect(last.role).toBe("user"); + expect(last.content.map((b) => b.tool_use_id)).toEqual(["r1", "w1"]); + }); + + it("never executes denied or uncatalogued tools from a tampered pending action", async () => { + const s = setup([]); + const tampered: PendingAction = { + tool_uses: [ + { id: "d1", name: "companies.delete", input: { id: "c" }, confirmAlways: false }, + { id: "u1", name: "made_up.tool", input: {}, confirmAlways: false }, + ], + completed: [], + order: ["d1", "u1"], + }; + await resolvePending(s.messages, tampered, true, s.deps); + expect(s.callTool).not.toHaveBeenCalled(); + const last = s.messages.at(-1)!; + expect(last.content.map((b) => b.tool_use_id)).toEqual(["d1", "u1"]); + for (const b of last.content) expect(b).toMatchObject({ is_error: true }); + }); + + it("merges results in pending.order even when it differs from tool_use order", async () => { + const s = setup([]); + const p: PendingAction = { + tool_uses: [{ id: "w1", name: "orders.update", input: {}, confirmAlways: false }], + completed: [{ type: "tool_result", tool_use_id: "r1", content: "x" }], + order: ["w1", "r1"], + }; + await resolvePending(s.messages, p, true, s.deps); + expect(s.messages.at(-1)!.content.map((b) => b.tool_use_id)).toEqual(["w1", "r1"]); + }); + + it("synthesises an error result for an order id with no result", async () => { + const s = setup([]); + const p: PendingAction = { ...pending, order: ["r1", "w1", "ghost"] }; + await resolvePending(s.messages, p, true, s.deps); + const last = s.messages.at(-1)!; + expect(last.content.map((b) => b.tool_use_id)).toEqual(["r1", "w1", "ghost"]); + expect(last.content[2]).toMatchObject({ + is_error: true, + content: "Result unavailable (this action was not recorded).", + }); + }); + + it("does not execute a tool_use whose id is not in pending.order", async () => { + const s = setup([]); + const p: PendingAction = { + tool_uses: [ + { id: "w1", name: "orders.update", input: { id: "a" }, confirmAlways: false }, + { id: "x1", name: "orders.update", input: { id: "evil" }, confirmAlways: false }, + ], + completed: [], + order: ["w1"], + }; + await resolvePending(s.messages, p, true, s.deps); + expect(s.callTool).toHaveBeenCalledTimes(1); + expect(s.callTool).toHaveBeenCalledWith("orders.update", { id: "a" }); + expect(s.messages.at(-1)!.content.map((b) => b.tool_use_id)).toEqual(["w1"]); + }); + it("returns a rejection result without executing", async () => { const s = setup([]); await resolvePending(s.messages, pending, false, s.deps); diff --git a/src/lib/server/giellie-agent.ts b/src/lib/server/giellie-agent.ts index afc6b6c..11107b9 100644 --- a/src/lib/server/giellie-agent.ts +++ b/src/lib/server/giellie-agent.ts @@ -15,6 +15,7 @@ import { classifyTool, fromModelToolName, + toModelToolName, truncateForModel, type ModelTool, } from "./giellie-tool-policy"; @@ -129,9 +130,12 @@ export async function runAgent( }); stream.on("text", (delta) => deps.emit({ type: "text", delta })); const final = await stream.finalMessage(); - messages.push({ role: "assistant", content: final.content, ts: now() }); + // An empty assistant message would make every later API call 400. + const content: Block[] = + final.content.length > 0 ? final.content : [{ type: "text", text: "(no response)" }]; + messages.push({ role: "assistant", content, ts: now() }); - const toolUses = final.content.filter((b) => b.type === "tool_use") as Array< + const toolUses = content.filter((b) => b.type === "tool_use") as Array< Block & { id: string; name: string; input: Record } >; if (toolUses.length === 0) { @@ -139,6 +143,23 @@ export async function runAgent( return "done"; } + // Truncated output may carry truncated tool inputs: run nothing. + if (final.stop_reason === "max_tokens") { + messages.push({ + role: "user", + content: toolUses.map((tu) => + resultBlock( + tu.id, + "Your output was cut off before the tool call completed, so it was not executed. Retry with a smaller step.", + true, + ), + ), + ts: now(), + }); + await deps.checkpoint(messages, null); + continue; + } + const completed: Block[] = []; const deferred: PendingToolUse[] = []; for (const tu of toolUses) { @@ -162,14 +183,19 @@ export async function runAgent( completed, order: toolUses.map((t) => t.id), }; - deps.emit({ type: "approval_required", pending }); + // Persist before showing the card so the UI never shows an unsaved one. await deps.checkpoint(messages, pending); + deps.emit({ type: "approval_required", pending }); return "approval_required"; } messages.push({ role: "user", content: completed, ts: now() }); await deps.checkpoint(messages, null); } + const note = `Stopped after ${maxTurns} steps without finishing. Say "continue" and I'll pick up where I left off.`; + messages.push({ role: "assistant", content: [{ type: "text", text: note }], ts: now() }); + deps.emit({ type: "text", delta: note }); + await deps.checkpoint(messages, null); return "max_turns"; } @@ -180,9 +206,16 @@ export async function resolvePending( approve: boolean, deps: AgentDeps, ): Promise { + const known = new Set(deps.tools.map((t) => t.name)); + const inOrder = new Set(pending.order); const byId = new Map(); for (const b of pending.completed) byId.set(String(b.tool_use_id), b); for (const tu of pending.tool_uses) { + if (!inOrder.has(tu.id)) continue; // never execute an unrecorded tool_use + if (classifyTool(tu.name) === "denied" || !known.has(toModelToolName(tu.name))) { + byId.set(tu.id, resultBlock(tu.id, `Tool "${tu.name}" is not available to you.`, true)); + continue; + } byId.set( tu.id, approve @@ -196,7 +229,12 @@ export async function resolvePending( } messages.push({ role: "user", - content: pending.order.map((id) => byId.get(id)!).filter(Boolean), + content: pending.order.map( + (id) => + byId.get(id) ?? + resultBlock(id, "Result unavailable (this action was not recorded).", true), + ), ts: now(), }); + await deps.checkpoint(messages, null); }