fix(tabs): never crash on duplicate toolCallIds in tool batches

4 files changed, 214 insertions, 1 deletions

diff --git a/src/core/chunks/groups.test.ts b/src/core/chunks/groups.test.ts
index c8b0fa2..db632a3 100644
--- a/src/core/chunks/groups.test.ts
+++ b/src/core/chunks/groups.test.ts
@@ -122,4 +122,77 @@ describe("groupRenderedChunks", () => {
     expect(singles[0]?.kind === "single" && singles[0].chunk.chunk.type).toBe("tool-call");
     expect(singles[1]?.kind === "single" && singles[1].chunk.chunk.type).toBe("tool-result");
   });
+
+  // --- Duplicate toolCallId hardening (fix-toolcall-duplicate-keys) ---
+  // A provider double-emission (e.g. openrouter/stealth/ox-alpha) can persist the
+  // SAME toolCallId twice within one step. The renderer must NEVER crash on that
+  // (Svelte `each_key_duplicate` aborts the reactive flush — the tab appears dead).
+
+  it("dedupes duplicate toolCallIds within a batch", () => {
+    // The PII_REDACTED_CONVERSATION_SHORT_01 pattern: every id emitted twice, in order (seq 1,2 then 3,4).
+    const groups = groupRenderedChunks([
+      call(1, "a", "s1"),
+      call(2, "b", "s1"),
+      call(3, "a", "s1"),
+      call(4, "b", "s1"),
+    ]);
+    expect(groups).toHaveLength(1);
+    const g = groups[0];
+    if (g?.kind !== "tool-batch") throw new Error("expected a tool-batch group");
+    expect(g.entries).toHaveLength(2);
+    expect(g.entries.map((e) => e.call.toolCallId)).toEqual(["a", "b"]);
+  });
+
+  it("dedupes duplicate toolCallIds and pairs each unique call with its result", () => {
+    const groups = groupRenderedChunks([
+      call(1, "a", "s1"),
+      call(2, "b", "s1"),
+      call(3, "a", "s1"),
+      call(4, "b", "s1"),
+      result(5, "a", "s1"),
+      result(6, "b", "s1"),
+      result(7, "a", "s1"),
+      result(8, "b", "s1"),
+    ]);
+    expect(groups).toHaveLength(1);
+    const g = groups[0];
+    if (g?.kind !== "tool-batch") throw new Error("expected a tool-batch group");
+    expect(g.entries).toHaveLength(2);
+    expect(g.entries[0]?.call.toolCallId).toBe("a");
+    expect(g.entries[0]?.result?.content).toBe("result-a");
+    expect(g.entries[1]?.call.toolCallId).toBe("b");
+    expect(g.entries[1]?.result?.content).toBe("result-b");
+  });
+
+  it("keeps the first occurrence of a duplicate toolCallId and skips the second", () => {
+    const firstA = call(1, "a", "s1"); // first occurrence of a
+    const firstB = call(2, "b", "s1"); // first occurrence of b
+    const groups = groupRenderedChunks([
+      firstA,
+      firstB,
+      call(3, "a", "s1"), // duplicate of a — skipped
+      call(4, "b", "s1"), // duplicate of b — skipped
+    ]);
+    const g = groups[0];
+    if (g?.kind !== "tool-batch") throw new Error("expected a tool-batch group");
+    expect(g.entries).toHaveLength(2);
+    // The FIRST occurrence (by stream order) is the one kept — referential
+    // equality against the seq-1 / seq-2 chunks (the entry stores the inner
+    // chunk; the outer `seq` lives on the RenderedChunk wrapper).
+    expect(g.entries[0]?.call).toBe(firstA.chunk);
+    expect(g.entries[1]?.call).toBe(firstB.chunk);
+  });
+
+  it("keeps the first (provisional) occurrence of a duplicate and marks the batch provisional", () => {
+    // One provisional + one committed chunk with the SAME id: the first
+    // (provisional) occurrence wins, and the batch stays provisional.
+    const prov = call(1, "a", "s1", true);
+    const groups = groupRenderedChunks([prov, call(2, "a", "s1")]);
+    const g = groups[0];
+    if (g?.kind !== "tool-batch") throw new Error("expected a tool-batch group");
+    expect(g.provisional).toBe(true);
+    expect(g.entries).toHaveLength(1);
+    expect(g.entries[0]?.call).toBe(prov.chunk);
+    expect(g.entries[0]?.result).toBeNull();
+  });
 });
diff --git a/src/core/chunks/groups.ts b/src/core/chunks/groups.ts
index 53a2873..c1c80c9 100644
--- a/src/core/chunks/groups.ts
+++ b/src/core/chunks/groups.ts
@@ -72,9 +72,17 @@ export function groupRenderedChunks(rendered: readonly RenderedChunk[]): readonl
       emittedSteps.add(stepId);
 
       const entries: ToolBatchEntry[] = [];
+      // Dedupe by toolCallId within the batch: a provider double-emission (or any
+      // corrupted transcript) can carry the SAME call id on multiple tool-call
+      // chunks in one step. The first occurrence (stream order) wins; later ones
+      // are skipped so the keyed `{#each}` in the UI never sees duplicate keys
+      // (Svelte `each_key_duplicate` aborts the reactive flush — tab appears dead).
+      const seenCallIds = new Set<string>();
       let provisional = false;
       for (const inner of rendered) {
         if (inner.chunk.type === "tool-call" && inner.chunk.stepId === stepId) {
+          if (seenCallIds.has(inner.chunk.toolCallId)) continue;
+          seenCallIds.add(inner.chunk.toolCallId);
           const result = resultByCallId.get(inner.chunk.toolCallId) ?? null;
           entries.push({ call: inner.chunk, result });
           if (inner.provisional) provisional = true;
diff --git a/src/features/chat/ui/ChatView.svelte b/src/features/chat/ui/ChatView.svelte
index e67ca5b..6805b20 100644
--- a/src/features/chat/ui/ChatView.svelte
+++ b/src/features/chat/ui/ChatView.svelte
@@ -301,7 +301,7 @@
       <div class="chat chat-start [&>.chat-bubble]:max-w-5xl [&>.chat-bubble]:p-0">
         <div class="chat-bubble w-full bg-transparent">
           <div class="flex flex-col gap-1">
-            {#each row.group.entries as entry (entry.call.toolCallId)}
+            {#each row.group.entries as entry, entryIndex (entryIndex)}
               <div class="collapse w-full rounded-box bg-base-200 text-sm">
                 <input type="checkbox" aria-label="Toggle tool call" />
                 <div class="collapse-title flex min-h-0 items-center gap-2 py-2 font-medium">
diff --git a/src/features/chat/ui/ChatView.test.ts b/src/features/chat/ui/ChatView.test.ts
new file mode 100644
index 0000000..eb807c1
--- /dev/null
+++ b/src/features/chat/ui/ChatView.test.ts
@@ -0,0 +1,132 @@
+import type { StepId } from "@dispatch/wire";
+import { render, screen } from "@testing-library/svelte";
+import { describe, expect, it } from "vitest";
+import type { RenderedChunk } from "../../../core/chunks";
+import ChatView from "./ChatView.svelte";
+
+// Defensive rendering: a conversation transcript can contain duplicate
+// `toolCallId`s (provider double-emission, persisted — e.g. PII_REDACTED_CONVERSATION_SHORT_01). The
+// grouped render path MUST dedupe them so the keyed {#each} never throws
+// Svelte `each_key_duplicate` (which aborts the reactive flush and makes the
+// tab appear dead). See fix-toolcall-duplicate-keys plan Part 1.
+describe("ChatView — duplicate toolCallIds", () => {
+  it("renders a tool batch with duplicated toolCallIds without throwing, showing one card per unique call", () => {
+    const chunks: RenderedChunk[] = [
+      {
+        seq: 1,
+        role: "assistant",
+        chunk: {
+          type: "tool-call",
+          toolCallId: "a",
+          toolName: "read_file",
+          input: { path: "/a" },
+          stepId: "t1#0" as StepId,
+        },
+        provisional: false,
+      },
+      {
+        seq: 2,
+        role: "assistant",
+        chunk: {
+          type: "tool-call",
+          toolCallId: "b",
+          toolName: "list_dir",
+          input: { path: "/b" },
+          stepId: "t1#0" as StepId,
+        },
+        provisional: false,
+      },
+      // The PII_REDACTED_CONVERSATION_SHORT_01 pattern: the same ids re-emitted immediately after.
+      {
+        seq: 3,
+        role: "assistant",
+        chunk: {
+          type: "tool-call",
+          toolCallId: "a",
+          toolName: "read_file",
+          input: { path: "/a" },
+          stepId: "t1#0" as StepId,
+        },
+        provisional: false,
+      },
+      {
+        seq: 4,
+        role: "assistant",
+        chunk: {
+          type: "tool-call",
+          toolCallId: "b",
+          toolName: "list_dir",
+          input: { path: "/b" },
+          stepId: "t1#0" as StepId,
+        },
+        provisional: false,
+      },
+      {
+        seq: 5,
+        role: "tool",
+        chunk: {
+          type: "tool-result",
+          toolCallId: "a",
+          toolName: "read_file",
+          content: "contents-of-a",
+          isError: false,
+          stepId: "t1#0" as StepId,
+        },
+        provisional: false,
+      },
+      {
+        seq: 6,
+        role: "tool",
+        chunk: {
+          type: "tool-result",
+          toolCallId: "b",
+          toolName: "list_dir",
+          content: "contents-of-b",
+          isError: false,
+          stepId: "t1#0" as StepId,
+        },
+        provisional: false,
+      },
+      // Duplicate results too — absorbed into the single deduped batch.
+      {
+        seq: 7,
+        role: "tool",
+        chunk: {
+          type: "tool-result",
+          toolCallId: "a",
+          toolName: "read_file",
+          content: "contents-of-a",
+          isError: false,
+          stepId: "t1#0" as StepId,
+        },
+        provisional: false,
+      },
+      {
+        seq: 8,
+        role: "tool",
+        chunk: {
+          type: "tool-result",
+          toolCallId: "b",
+          toolName: "list_dir",
+          content: "contents-of-b",
+          isError: false,
+          stepId: "t1#0" as StepId,
+        },
+        provisional: false,
+      },
+    ];
+
+    const { container } = render(ChatView, { props: { chunks } });
+
+    // No each_key_duplicate crash. Batched calls render as collapsible cards —
+    // exactly ONE per unique toolCallId (deduped), not 4.
+    const collapses = container.querySelectorAll(".collapse");
+    expect(collapses).toHaveLength(2);
+
+    // Both unique tool names shown; each result rendered once (deduped).
+    expect(screen.getByText("read_file")).toBeInTheDocument();
+    expect(screen.getByText("list_dir")).toBeInTheDocument();
+    expect(screen.getByText("contents-of-a")).toBeInTheDocument();
+    expect(screen.getByText("contents-of-b")).toBeInTheDocument();
+  });
+});