fix(responses): done-event repair must not clobber tool-call identity (#244)

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 All notable changes to the **OpenCode Go BYOK Provider** extension are documented here.
 
+## [0.7.8] — 2026-09-25
+
+### Fixed
+
+- **`[Responses]` The #244 done-event repair no longer corrupts tool-call identity — the 0.7.7 regression causing `400 No tool output found for function call call_*` is fixed (#244 follow-up).** The real luna `response.function_call_arguments.done` event carries only `item_id` (`fc_1`), never `call_id`. The repair delta built `id: firstString(call_id, item_id)` and the accumulator adopted it, REPLACING the real `call_*` id captured from `output_item.added` — and item ids are reused across turns. Two turns both carrying `fc_1` produced two `function_call` items against one `function_call_output` at the gateway, which rejects the whole request with `No tool output found`. Fix: the done event now repairs **arguments only** (id forwarded only when a real `call_id` is present); `ToolCallAccumulator` captures an id once and never lets a later fragment overwrite it; and `pairResponsesFunctionCallItems` collapses duplicate `function_call` items sharing a call_id, self-healing histories already poisoned by 0.7.7. The identity rule is enforced on every Responses entry point — `output_item.added` and the full-response mapping no longer fall back to `item.id` either. Proven empirically against the captured luna event shapes (part id stays `call_*` end to end) plus a 17-check E2E simulation of the full tool-call loop. Documented in `docs/issues/108-20260925-issue244-followup-done-event-id-clobber.md`.
+
 ## [0.7.7] — 2026-09-24
 
 ### Fixed

docs/devlog.md

@@ -1,6 +1,20 @@
 # 🧠 OPENCODE COPILOT CHAT DEVLOG
 
-**Branch:** `main` (fix uncommitted, branch TBD) | **Updated:** 2026-09-24 Asia/Jakarta | **Current Phase:** issue #244 fix — tool-call arguments whitespace on Responses API; implemented + tested (489/489), docs synced, CHANGELOG `[0.7.7]`, VSIX 0.7.7 built + installed locally, pending commit.
+**Branch:** `fix/issue244-followup-id-clobber` (work on `main`) | **Updated:** 2026-09-25 Asia/Jakarta | **Current Phase:** #244 follow-up fix complete + deep-verified (495/495 tests, lint 7/7, retry E2E 9/9, pre-release E2E 17/17); 0.7.8 pending push/PR/build.
+
+---
+
+## ✅ #244 Follow-up — done-event id clobber → 400 No tool output found — 2026-09-25
+
+**Scope:** regression OF the #244 fix (0.7.7), reported by the same user 18h later: every OpenAI-model request 400s with `No tool output found for function call call_*` (#216 error class). The done-event repair delta built `id: firstString(call_id, item_id)` — but the real luna done event carries only `item_id: "fc_1"`, which the accumulator adopted, REPLACING the real `call_*` id. Item ids are reused across turns → two turns with `fc_1` = two function_calls against one output at the gateway → 400. Proven empirically by replaying the captured luna event shapes (part id came out `fc_1` instead of `call_Vf1vzJwf...`).
+
+**Fix (3 layers):** (1) done event forwards `id` only when a real `call_id` is present — repair is arguments-only; (2) `ToolCallAccumulator` captures id once, never overwritten; (3) `pairResponsesFunctionCallItems` collapses duplicate function_call items sharing a call_id — poisoned histories self-heal.
+
+**Tests:** 4 new in `src/test/issue244-followup-regression.test.ts` (identity preserved, round-trip pairing, #244 arguments repair preserved, poisoned-history dedupe). **493/493 pass**, compile clean.
+
+**Lesson:** a streaming repair event must never mutate call identity; `firstString(call_id, item_id)`-style fallbacks across semantically different identifiers are the trap.
+
+Docs: `docs/issues/108`, doc 107 annotated, CHANGELOG `[0.7.8]`.
 
 ---
 

docs/issues/107-20260924-issue244-tool-call-arguments-whitespace.md

@@ -54,6 +54,8 @@ The reporter's OpenCode Diagnostics dumps (Go + Zen) corroborate the root cause:
 
 Design note: the done-event path makes the fix **permanent** — even if some gateway trims or mangles delta fragments, the final `response.function_call_arguments.done` value overwrites the accumulated string, matching official SDK semantics (`output.arguments = event.arguments`).
 
+> **⚠️ Follow-up regression (fixed in 0.7.8):** the original repair delta also forwarded `id: firstString(call_id, item_id)` — and the real luna done event carries only `item_id` (`fc_1`), which the accumulator adopted, clobbering the real `call_*` identity and causing `400 No tool output found` (item ids are reused across turns). See [doc 108](108-20260925-issue244-followup-done-event-id-clobber.md). The arguments-only repair remains; identity is never touched.
+
 ## Tests
 
 - `src/test/routing.test.ts` — "function_call_arguments whitespace preservation (#244)": fragments split inside a JSON string value must preserve the space; done-event mapping; done-event without arguments emits no choices.

docs/issues/108-20260925-issue244-followup-done-event-id-clobber.md

@@ -0,0 +1,81 @@
+# Issue #244 Follow-up — 0.7.7 Done-Event Repair Corrupted Tool-Call Identity (`400 No tool output found`)
+
+**Status:** ✅ Solved (0.7.8)
+**Topic:** streaming / responses-api / tool-calls / pairing
+**Updated:** 2026-09-25
+**Tags:** #responses-api #bug #tool-calls #pairing #regression
+**GitHub Issue:** [#244](https://github.com/ltmoerdani/opencode-copilot-chat/issues/244) (comment 18h after the 0.7.7 fix)
+**Related:** doc [107 — #244 arguments whitespace](107-20260924-issue244-tool-call-arguments-whitespace.md) (the fix this regressed), docs [96 — #216 pairing](../issues/96-20260808-issue216-no-tool-output-for-function-call.md), [90 — #206 fc_ ids](90-20260903-issue206-luna-responses-fc-id-mismatch.md)
+
+---
+
+## Problem
+
+Immediately after updating to 0.7.7, the reporter's every OpenAI-model request failed:
+
+```text
+OpenCode Zen API request failed (400) model=gpt-5.6-luna payloadBytes=19484:
+No tool output found for function call call_Vf1vzJwf5xa44CfwtrzYe4y7.
+```
+
+Same on OpenCode Go (`gpt-6-luna`, `call_d0KxiJXE7jjKAM4MQrXdbCZl`). All requests 400 at the gateway, before streaming.
+
+## Root Cause — the done-event repair changed tool-call IDENTITY
+
+The #244 fix (0.7.7) added a `response.function_call_arguments.done` handler emitting a repair delta tagged `argumentsDone: true`, with:
+
+```ts
+id: firstString(data.call_id, data.item_id) ?? "",
+```
+
+The captured REAL luna event shapes (#216/#217 suite) show the done event carries **only `item_id: "fc_1"` — never `call_id`**. So the repair delta adopted `fc_1`, and `ToolCallAccumulator.collect()` REPLACEd the real `call_*` id captured from `output_item.added`.
+
+Fatal because **item ids are per-response and reused across turns**. Two turns both carrying `fc_1` as the part id produce, at the gateway: `function_call(fc_1) ×2` + `function_call_output(fc_1) ×1` — our pairing's `consumedOutputs` drops the second output, so the second function_call reaches the gateway unpaired → the exact 400.
+
+Proven empirically: replaying the captured luna event sequence through `normalizeResponsesStreamEvent` + `OpenAiResponseExtractor` emitted `callId: "fc_1"` instead of `call_Vf1vzJwf5xa44CfwtrzYe4y7`.
+
+## Fix
+
+| Layer      | File                         | Change                                                                                                                                                                       |
+| ---------- | ---------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| Root cause | `src/core/routing.ts`        | Done event forwards `id` **only when a real `call_id` is present**; otherwise no id — the repair is arguments-only                                                           |
+| Defense    | `src/toolCallAccumulator.ts` | An id is captured **once** (first fragment that carries one) and never overwritten by later fragments                                                                        |
+| Self-heal  | `src/responsesRequest.ts`    | `pairResponsesFunctionCallItems` collapses duplicate `function_call` items sharing one call_id (keeps the first) — histories already poisoned by 0.7.7 recover automatically |
+
+### Identity Rule extended to every Responses entry point
+
+The post-fix audit found the same latent fallback in two more places and removed it — **no Responses path may ever derive call identity from `item.id`**:
+
+- `output_item.added` handler: `firstString(item.call_id, item.id)` → `call_id` only (empty → the flush fabricates a unique id, which cannot collide).
+- `normalizeResponsesFullResponse` function_call mapping: same fallback removed.
+
+Gateways that always send `call_id` (all known real shapes) are unaffected; only the hypothetical call_id-less shape changes behavior, and strictly for the better.
+
+The `argumentsDone` REPLACE repair for arguments (the actual #244 fix) is preserved and pinned by test.
+
+## Tests
+
+`src/test/issue244-followup-regression.test.ts` (renamed from the repro) — 6 tests:
+
+1. Part id stays `call_*` when done carries only `item_id` (the exact reported shape).
+2. Full round-trip: wire request pairs function_call with its output.
+3. `argumentsDone` still repairs mis-joined arguments (#244 fix preserved).
+4. Pairing collapses duplicate function_call items (poisoned-history self-heal).
+5. `output_item.added` without `call_id` never adopts the `fc_` item id.
+6. `normalizeResponsesFullResponse` maps `call_id` only (no `item.id` fallback).
+
+### Pre-release E2E (`tmp/e2e-issue244-prerelease.mjs` — 17/17 checks)
+
+Full Copilot Chat loop simulated without VS Code, against the captured luna shapes:
+
+- Turn 1: stream → part id = gateway `call_id`, arguments `"what is 2 plus 2?"` keep their spaces (the original #244 symptom).
+- Turn 2: history replay → wire request → pairing call/output = 1:1, no 400 possible.
+- Turn 3: second turn reusing item id `fc_1` (the 0.7.7 poison) stays clean.
+- Poisoned-history self-heal: a history already written by 0.7.7 collapses to 1 call + 1 output — users recover without clearing the chat.
+- Non-stream full-response path sanity.
+
+Verification: **495/495** unit tests, `npm run lint` 7/7, retry E2E mock server 9/9, `npm run compile` clean.
+
+## Lesson Recorded
+
+A streaming **repair** event must never mutate call **identity** — identity comes from `output_item.added`, arguments from `*.delta`/`*.done`. When forwarding fields from a done event, forward only the fields the repair needs; `firstString(call_id, item_id)`-style fallbacks across semantically different identifiers are how `fc_` item ids leaked into call identity.

package.json

@@ -2,7 +2,7 @@
   "name": "opencode-copilot-chat",
   "displayName": "OpenCode for Copilot Chat: BYOK 30+ AI Models",
   "description": "Use 30+ frontier AI models (DeepSeek V4, Kimi K2.6, GLM-5.1, Qwen3.7, MiMo V2.5, MiniMax M2.7, free Claude Opus, GPT-5.5, Gemini 3.5, Grok) in GitHub Copilot Chat. Bring Your Own Key, no Copilot Pro needed.",
-  "version": "0.7.7",
+  "version": "0.7.8",
   "publisher": "ltmoerdani",
   "license": "MIT",
   "icon": "media/opencodego.png",

src/core/routing.ts

@@ -78,6 +78,10 @@ export function normalizeResponsesStreamEvent(data: unknown): unknown {
   if (eventType === "response.output_item.added") {
     const item = data.item;
     if (isRecord(item) && item.type === "function_call" && typeof item.name === "string") {
+      // IDENTITY RULE (#244 follow-up): call identity comes ONLY from
+      // `call_id`. Never fall back to `item.id` (`fc_*`) — item ids are
+      // reused across turns and would collide in replayed history.
+      const callId = typeof item.call_id === "string" && item.call_id.trim() ? item.call_id : "";
       return {
         choices: [
           {
@@ -86,7 +90,7 @@ export function normalizeResponsesStreamEvent(data: unknown): unknown {
               tool_calls: [
                 {
                   index: typeof data.output_index === "number" ? data.output_index : 0,
-                  id: firstString(item.call_id, item.id) ?? "",
+                  id: callId,
                   type: "function",
                   function: { name: item.name, arguments: "" },
                 },
@@ -152,11 +156,16 @@ export function normalizeResponsesStreamEvent(data: unknown): unknown {
   // Use it as an authoritative repair: the accumulator REPLACES (not appends)
   // the pending arguments so any corruption from mis-joined delta fragments is
   // healed at stream end (issue #244).
+  // IDENTITY RULE (0.7.7 regression): the done event may carry only `item_id`
+  // (e.g. `fc_1` on gpt-5.6-luna) — item ids are REUSED across turns, so they
+  // must never become the tool-call part id. Only a real `call_id` is
+  // forwarded; the accumulator keeps the id captured from output_item.added.
   if (eventType === "response.function_call_arguments.done") {
     const args = firstStringRaw(data.arguments);
     if (args === undefined) {
       return { choices: [] };
     }
+    const callId = typeof data.call_id === "string" && data.call_id.trim() ? data.call_id : undefined;
     return {
       choices: [
         {
@@ -165,7 +174,7 @@ export function normalizeResponsesStreamEvent(data: unknown): unknown {
             tool_calls: [
               {
                 index: typeof data.output_index === "number" ? data.output_index : 0,
-                id: firstString(data.call_id, data.item_id) ?? "",
+                ...(callId ? { id: callId } : {}),
                 type: "function",
                 function: { arguments: args },
                 argumentsDone: true,
@@ -237,7 +246,9 @@ export function normalizeResponsesFullResponse(data: unknown): unknown {
 
     if (item.type === "function_call" && typeof item.name === "string") {
       toolCalls.push({
-        id: firstString(item.call_id, item.id) ?? "",
+        // IDENTITY RULE (#244 follow-up): call_id only — no item.id fallback
+        // (fc_* item ids are reused across turns; see added-handler above).
+        id: firstString(item.call_id) ?? "",
         type: "function",
         function: {
           name: item.name,

src/responsesRequest.ts

@@ -137,6 +137,10 @@ export function responsesInputItemsFromMessage(message: ResponsesApiMessage): Re
  * - A `function_call` with no output is dropped too (the gateway 400s on it;
  *   keeping it would fail the entire turn).
  * - The first output wins if a call_id is duplicated.
+ * - Duplicate `function_call` items sharing one call_id are collapsed to the
+ *   first (self-heals histories poisoned by 0.7.7's id-clobbering bug, where
+ *   reused `fc_*` item ids made two turns emit the same call id; #244
+ *   follow-up).
  */
 export function pairResponsesFunctionCallItems(items: Record<string, unknown>[]): Record<string, unknown>[] {
   const callIdsWithOutput = new Set<string>();
@@ -156,6 +160,7 @@ export function pairResponsesFunctionCallItems(items: Record<string, unknown>[])
     }
   }
   const consumedOutputs = new Set<string>();
+  const seenCalls = new Set<string>();
   return items.filter((item) => {
     const callId = item.call_id;
     if (item.type === "function_call_output") {
@@ -164,7 +169,16 @@ export function pairResponsesFunctionCallItems(items: Record<string, unknown>[])
       return matched;
     }
     if (item.type === "function_call") {
-      return typeof callId === "string" && callIdsWithOutput.has(callId);
+      if (typeof callId !== "string" || !callIdsWithOutput.has(callId)) {
+        return false;
+      }
+      // Collapse duplicate function_call items: one output can only serve one
+      // call, so extras with the same call_id would 400 at the gateway.
+      if (seenCalls.has(callId)) {
+        return false;
+      }
+      seenCalls.add(callId);
+      return true;
     }
     return true;
   });