fix(giellie): harden agent loop approval resolution and checkpoints

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

src/lib/server/giellie-agent.test.ts

@@ -53,7 +53,12 @@ function setup(turns: Block[][], over: Partial<AgentDeps> = {}) {
   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<AgentDeps> = {}) {
     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<AgentDeps> = {}) {
   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);

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<string, unknown> }
     >;
     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<void> {
+  const known = new Set(deps.tools.map((t) => t.name));
+  const inOrder = new Set(pending.order);
   const byId = new Map<string, Block>();
   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);
 }