From f1e4f1497e87bdc2ed90722903bc911d74e1ac98 Mon Sep 17 00:00:00 2001 From: ltmoerdani Date: Wed, 23 Sep 2026 20:47:22 +0700 Subject: [PATCH 1/5] fix(retry): self-heal 400 when reasoning echo is missing from history (fixes #239) DeepSeek V4 thinking mode requires reasoning_content to be passed back on multi-turn requests. When Copilot Chat compaction, history trimming, or a pre-thinking-capture turn removes it from the replayed history, every follow-up turn 400s and retries never recover. Add a recoverable-400 pattern that strips the reasoning_content echo from assistant messages AND turns reasoning_effort off, making the request self-contained. Dropping reasoning_effort stops the cycle: with thinking still on, the next response emits new reasoning that compaction strips again, re-400ing the turn after. --- src/retry.ts | 29 +++++++++++++++++++++++++++++ src/test/retry.test.ts | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/src/retry.ts b/src/retry.ts index 83f2d6835..5c6f964c5 100644 --- a/src/retry.ts +++ b/src/retry.ts @@ -206,6 +206,35 @@ const RECOVERABLE_ERROR_PATTERNS: { }, describe: () => "removed thinking_budget (not accepted by this model)", }, + + // --- Reasoning echo missing (DeepSeek V4 thinking mode) --- + // "The `reasoning_content` in the thinking mode must be passed back to the + // API." (issue #239). Happens when prior-turn reasoning is gone from the + // replayed history (Copilot Chat conversation summarization/compaction, + // history trimming, or a conversation started before thinking capture was + // active). Retrying without a patch fails forever — every turn re-400s — + // so we self-heal by stripping the reasoning_content echo from assistant + // messages AND turning reasoning_effort off, making the request + // self-contained. Dropping reasoning_effort stops the cycle: with thinking + // still on, the next response would emit new reasoning that compaction + // strips again, re-400ing the turn after. + { + pattern: /reasoning_content`? in the thinking mode must be passed back/i, + patch: (body) => { + const messages: unknown[] = Array.isArray(body.messages) ? body.messages : []; + return { + ...body, + messages: messages.map((message) => { + if (typeof message !== "object" || message === null) return message; + const msg = message as Record; + if (msg.role !== "assistant" || msg.reasoning_content === undefined) return msg; + return { ...msg, reasoning_content: undefined }; + }), + reasoning_effort: undefined, + }; + }, + describe: () => "stripped reasoning_content echo + reasoning_effort (history lost prior reasoning)", + }, // budget_tokens — used by Mimo thinking payload to cap reasoning tokens { pattern: /extra inputs are not permitted.*budget_tokens/i, diff --git a/src/test/retry.test.ts b/src/test/retry.test.ts index ce2e25483..0d99b4e79 100644 --- a/src/test/retry.test.ts +++ b/src/test/retry.test.ts @@ -70,6 +70,40 @@ describe("analyzeHttp400ForRetry — reasoning_effort errors", () => { }); }); +describe("analyzeHttp400ForRetry — reasoning echo missing (issue #239)", () => { + const echoError = + "Upstream request failed: [invalid_request_error] The `reasoning_content` in the thinking mode must be passed back to the API."; + + it("strips reasoning_content from assistant messages and reasoning_effort", () => { + const body = { + model: "deepseek-v4.1-flash", + reasoning_effort: "low", + messages: [ + { role: "user", content: "hello" }, + { role: "assistant", content: "working", reasoning_content: "chain of thought", tool_calls: [] }, + { role: "tool", tool_call_id: "call_1", content: "result" }, + ], + }; + const result = analyzeHttp400ForRetry(echoError, body); + assert.ok(result, "should be recoverable"); + assert.deepEqual(result.body, { + model: "deepseek-v4.1-flash", + reasoning_effort: undefined, + messages: [ + { role: "user", content: "hello" }, + { role: "assistant", content: "working", reasoning_content: undefined, tool_calls: [] }, + { role: "tool", tool_call_id: "call_1", content: "result" }, + ], + }); + }); + + it("reports no change when there is nothing to strip (no infinite retry)", () => { + const body = { model: "deepseek-v4.1-flash", messages: [{ role: "user", content: "hello" }] }; + const result = analyzeHttp400ForRetry(echoError, body); + assert.equal(result, undefined, "patch must be a no-op when nothing changes"); + }); +}); + describe("analyzeHttp400ForRetry — non-recoverable errors", () => { it("returns undefined for auth errors", () => { const body = { model: "test" }; From 6dae6bcfc57927bbad85981a42e9e644d9a59090 Mon Sep 17 00:00:00 2001 From: ltmoerdani Date: Wed, 23 Sep 2026 20:47:45 +0700 Subject: [PATCH 2/5] fix(provider): relocate glm-5.3* tool-result images to a user message (fixes #233) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit glm-5.3-flash upstream (Console Go) rejects image_url parts inside role:"tool" content with 422 "[invalid_request_error] Input should be a valid string", while the SAME images in a user message return 200 (reproduced directly against zen/go/v1 chat-completions). The tool result stays in history, so every follow-up turn re-422s. Introduce requiresStringToolContent() mapping per upstream: - "drop" — mimo-*: flatten + placeholder (existing #38 behavior, unchanged) - "defer" — glm-5.3*: flatten the tool message to a string and move the images into a follow-up user message, preserving vision - null — forward multimodal tool content unchanged (kimi, glm-5.2, minimax, qwen) --- src/models/modelCapabilities.ts | 25 +++++++++ src/provider/messages.ts | 85 ++++++++++++++++++++---------- src/test/modelCapabilities.test.ts | 21 ++++++++ 3 files changed, 102 insertions(+), 29 deletions(-) create mode 100644 src/test/modelCapabilities.test.ts diff --git a/src/models/modelCapabilities.ts b/src/models/modelCapabilities.ts index 03ce8054b..c2ebd557e 100644 --- a/src/models/modelCapabilities.ts +++ b/src/models/modelCapabilities.ts @@ -14,3 +14,28 @@ export function buildStableModelCapabilities(supportsVision: boolean): StableMod supportsToolCalling: true, }; } + +/** + * How tool-result images must be serialized for this model's chat-completions + * upstream. + * + * CONTRACT (evidence-based, per-provider): + * - "drop" — MiMo (`mimo-*`): upstream strictly requires plain-string tool + * message content AND cannot see tool images at all. Flatten + drop with a + * placeholder note (issue #38, upstream anomalyco/opencode#32613). + * - "defer" — glm-5.3* (`glm-5.3*`): tool-message image_url parts are + * rejected with HTTP 422 "[invalid_request_error] Input should be a valid + * string", but identical images in a user message return 200 (reproduced + * directly against zen/go/v1 chat-completions, issue #233). Move images + * into a follow-up user message instead of dropping them. + * - null — the upstream accepts multimodal tool content; forward it + * unchanged (Kimi, GLM-5.1/5.2, MiniMax, Qwen verified in production). + * + * Pure so the mapping stays unit-testable without a VS Code host. + */ +export function requiresStringToolContent(rawModelId: string | undefined): "drop" | "defer" | null { + if (rawModelId === undefined) return null; + if (/^mimo-/i.test(rawModelId)) return "drop"; + if (/^glm-5\.3/i.test(rawModelId)) return "defer"; + return null; +} diff --git a/src/provider/messages.ts b/src/provider/messages.ts index 10b8413da..c27eb5762 100644 --- a/src/provider/messages.ts +++ b/src/provider/messages.ts @@ -1,5 +1,6 @@ import * as vscode from "vscode"; import { isInternalDataPart, isReasoningMarkerPart, readReasoningMarker } from "../chatParts"; +import { requiresStringToolContent } from "../models/modelCapabilities"; import { MAX_HISTORY_IMAGES_KEPT, MAX_TOOL_RESULT_IMAGE_BYTES } from "../config"; import { getImageDataUrlBase64Bytes, MAX_IMAGE_BASE64_BYTES, normalizeImageDataUrl } from "../imageNormalizer"; import { shouldEchoThinkingHistory, thinkingTextFromValue } from "../reasoningHistory"; @@ -19,6 +20,9 @@ export async function convertMessage( const imageParts: OpenAiContentPart[] = []; const toolCalls: OpenAiToolCall[] = []; const toolResults: ApiMessage[] = []; + // Tool-result images deferred to a follow-up user message (see + // requiresStringToolContent — "defer" mode). Emitted by finish(). + const deferredToolImageParts: OpenAiContentPart[] = []; let normalizedImageCount = 0; const normalizeImagePart = async (part: vscode.LanguageModelDataPart): Promise => { @@ -30,10 +34,26 @@ export async function convertMessage( return normalizedUrl; }; - const finish = (messages: ApiMessage[]): ConvertedMessageResult => ({ - messages, - normalizedImageCount, - }); + const finish = (messages: ApiMessage[]): ConvertedMessageResult => { + const result = [...messages]; + if (deferredToolImageParts.length > 0) { + // Tool images rejected in role:"tool" content but accepted in user + // content (verified against zen/go/v1 chat-completions for + // glm-5.3-flash: tool image_url → 422, user image_url → 200). Append + // them as a follow-up user message so vision capability is preserved. + result.push({ + role: "user", + content: [ + { + type: "text", + text: "Images returned by a tool result (the upstream provider does not accept images inside tool messages):", + }, + ...deferredToolImageParts, + ], + }); + } + return { messages: result, normalizedImageCount }; + }; for (const part of message.content) { if (part instanceof vscode.LanguageModelToolCallPart) { @@ -92,38 +112,45 @@ export async function convertMessage( } let toolContent: string | OpenAiContentPart[]; - if (toolImageParts.length > 0) { - // PROVIDER QUIRK: Xiaomi MiMo (and GLM-5.2) reject list-type tool - // message content with HTTP 400 "text is not set" (upstream issue - // anomalyco/opencode#32613). MiMo accepts multimodal content in - // user/assistant messages but strictly requires `role: "tool"` - // messages to have a plain string content. The OpenCode Go gateway - // passes list-type content through unchanged, so we must flatten it - // client-side for MiMo. - // - // For MiMo: emit a plain string — join text parts, and replace each - // image with a short placeholder note (the model cannot see tool - // images on MiMo upstream anyway, so we lose nothing and gain a - // working request). For other providers: keep the multimodal array - // (Kimi, GLM-5.1, MiniMax, Qwen all accept list-type tool content). - const isMimoModel = rawModelId !== undefined && /^mimo-/i.test(rawModelId); - if (isMimoModel) { - const flattened: string[] = [...toolTextParts]; + // PROVIDER QUIRK: several chat-completions upstreams reject multimodal + // (list-type) content on role:"tool" messages while accepting the same + // parts in user/assistant messages. requiresStringToolContent() maps a + // model to its handling mode: + // "drop" — MiMo: upstream cannot see tool images at all (issue #38, + // upstream anomalyco/opencode#32613); flatten to a string + // and replace each image with a placeholder note. + // "defer" — glm-5.3* (issue #233): tool-message image_url parts are + // rejected with HTTP 422 "[invalid_request_error] Input + // should be a valid string", but the SAME images in a user + // message return 200. Flatten the tool message to a string + // and move the images into a follow-up user message (see + // finish()), preserving vision instead of dropping it. + // null — default: keep the multimodal array (Kimi, GLM-5.1/5.2, + // MiniMax, Qwen all accept list-type tool content). + const stringToolContentMode = requiresStringToolContent(rawModelId); + if (toolImageParts.length > 0 && stringToolContentMode !== null) { + const flattened: string[] = [...toolTextParts]; + if (stringToolContentMode === "drop") { for (let i = 0; i < toolImageParts.length; i++) { flattened.push( `[Tool returned an image attachment, but the MiMo upstream provider does not accept images in tool messages. Image ${String(i + 1)} of ${String(toolImageParts.length)} was dropped to keep the request valid.]`, ); } - toolContent = flattened.join("\n"); } else { - const multimodal: OpenAiContentPart[] = []; - const joinedText = toolTextParts.join("\n"); - if (joinedText) { - multimodal.push({ type: "text", text: joinedText }); - } - multimodal.push(...toolImageParts); - toolContent = multimodal; + flattened.push( + `[Tool returned ${toolImageParts.length === 1 ? "an image attachment" : `${String(toolImageParts.length)} image attachments`}; ${toolImageParts.length === 1 ? "it is" : "they are"} included in the following message.]`, + ); + deferredToolImageParts.push(...toolImageParts); + } + toolContent = flattened.join("\n"); + } else if (toolImageParts.length > 0) { + const multimodal: OpenAiContentPart[] = []; + const joinedText = toolTextParts.join("\n"); + if (joinedText) { + multimodal.push({ type: "text", text: joinedText }); } + multimodal.push(...toolImageParts); + toolContent = multimodal; } else { toolContent = toolTextParts.join("\n"); } diff --git a/src/test/modelCapabilities.test.ts b/src/test/modelCapabilities.test.ts new file mode 100644 index 000000000..377d92126 --- /dev/null +++ b/src/test/modelCapabilities.test.ts @@ -0,0 +1,21 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { requiresStringToolContent } from "../models/modelCapabilities.js"; + +describe("requiresStringToolContent", () => { + it("defers tool images to a user message for glm-5.3* (issue #233)", () => { + assert.equal(requiresStringToolContent("glm-5.3-flash"), "defer"); + assert.equal(requiresStringToolContent("glm-5.3"), "defer"); + }); + + it("drops tool images for MiMo (issue #38 behavior preserved)", () => { + assert.equal(requiresStringToolContent("mimo-v2.5"), "drop"); + }); + + it("keeps multimodal tool content for other families", () => { + assert.equal(requiresStringToolContent("glm-5.2"), null); + assert.equal(requiresStringToolContent("kimi-k3"), null); + assert.equal(requiresStringToolContent("deepseek-v4.1-flash"), null); + assert.equal(requiresStringToolContent(undefined), null); + }); +}); From 18a0d3e73352b7ea90e1264977420ae35d8a3b5e Mon Sep 17 00:00:00 2001 From: ltmoerdani Date: Wed, 23 Sep 2026 20:48:09 +0700 Subject: [PATCH 3/5] fix(provider): make provider toggle idempotent and fix wrong-key write (fixes #228) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects in the Remove/Re-add Provider flow: 1. Wrong-key write on agent variants: toggleProviderEnabled() wrote .enabled (e.g. opencodezen-agent.enabled) for agent-host definitions, but the provider when-clause and settings schema only read opencodezen.enabled. The provider stayed removed while settings looked enabled — the exact workaround in the issue report (manually deleting the stale key). Resolve agent variants to their base vendor before touching configuration. 2. Blind toggle: the command flipped the setting without regard to the desired outcome, so running it twice could never re-add the provider without a reload in between. Replace with a state-aware quick-pick (Remove when enabled / Re-add when disabled, Esc cancels) and retitle the command to 'Toggle Provider Registration in Language Models'. --- package.json | 4 +-- src/commands/providers.ts | 62 +++++++++++++++++++++++++-------- src/provider/providerDialogs.ts | 12 +++---- 3 files changed, 54 insertions(+), 24 deletions(-) diff --git a/package.json b/package.json index dca081d88..23aab29d2 100644 --- a/package.json +++ b/package.json @@ -68,7 +68,7 @@ }, { "command": "opencodego.toggleProvider", - "title": "OpenCode Go: Remove/Re-add Provider in Language Models" + "title": "OpenCode Go: Toggle Provider Registration in Language Models" }, { "command": "opencodego.refreshModels", @@ -92,7 +92,7 @@ }, { "command": "opencodezen.toggleProvider", - "title": "OpenCode Zen: Remove/Re-add Provider in Language Models" + "title": "OpenCode Zen: Toggle Provider Registration in Language Models" }, { "command": "opencodezen.refreshModels", diff --git a/src/commands/providers.ts b/src/commands/providers.ts index a6a2cc195..f75d42566 100644 --- a/src/commands/providers.ts +++ b/src/commands/providers.ts @@ -1,5 +1,6 @@ import * as vscode from "vscode"; import { SETTING_ENABLED } from "../config"; +import { resolveBaseVendor, type AllProviderVendor } from "../providerTypes"; /** Open the Settings UI filtered to the utility-model config keys. */ export async function configureUtilityModels(): Promise { @@ -10,26 +11,59 @@ export async function configureUtilityModels(): Promise { } /** - * Toggle whether a provider (`opencodego` / `opencodezen`) is registered at - * all. Disabling removes the provider from the Language Models list and every - * model picker — the provider's vendor contribution is gated by the same - * `when` clause (`config..enabled`) and its runtime registration is - * skipped. Previously configured BYOK groups and API keys are kept, so - * re-enabling restores the provider exactly as it was. + * Remove / re-add a provider (`opencodego` / `opencodezen`) from the Language + * Models list. Disabling removes the provider from the list and every model + * picker — the provider's vendor contribution is gated by the same `when` + * clause (`config..enabled`) and its runtime registration is skipped. + * Previously configured BYOK groups and API keys are kept, so re-enabling + * restores the provider exactly as it was. + * + * NOT a blind toggle: the desired action is derived from the current setting + * and offered as a quick-pick, so invoking the command repeatedly can never + * leave the provider in an unexpected state (issue #228). * * Provider registration happens at startup, so a window reload is required * for the change to take effect. */ -export async function toggleProviderEnabled(vendor: string, displayName: string): Promise { - const cfg = vscode.workspace.getConfiguration(vendor); - const current = cfg.get(SETTING_ENABLED, true); - const next = !current; - await cfg.update("enabled", next, vscode.ConfigurationTarget.Global); +export async function toggleProviderEnabled(vendor: AllProviderVendor, displayName: string): Promise { + // Resolve agent variants (opencodego-agent / opencodezen-agent) to their + // base vendor BEFORE touching configuration — the provider `when` clause + // and the settings schema only know `opencodego.enabled` / + // `opencodezen.enabled`. Writing the agent variant's own section (e.g. + // `opencodezen-agent.enabled`) produces a key nothing reads, leaving the + // provider permanently removed while settings look "enabled" (issue #228). + const baseVendor = resolveBaseVendor(vendor); + const cfg = vscode.workspace.getConfiguration(baseVendor); + const currentlyEnabled = cfg.get(SETTING_ENABLED, true); + + const action = await vscode.window.showQuickPick( + currentlyEnabled + ? [ + { + label: `$(remove) Remove ${displayName} from Language Models`, + detail: "Provider settings and API key are kept. Requires a window reload.", + disable: true, + }, + ] + : [ + { + label: `$(add) Re-add ${displayName} to Language Models`, + detail: "Restores the provider with its existing settings and API key. Requires a window reload.", + disable: false, + }, + ], + { title: `${displayName}: Manage Provider Registration`, placeHolder: "Choose an action (Esc to cancel)" }, + ); + if (!action) { + return; + } + + await cfg.update("enabled", action.disable, vscode.ConfigurationTarget.Global); const reload = await vscode.window.showInformationMessage( - next - ? `${displayName} re-enabled. Reload the window for the provider to appear in Language Models again.` - : `${displayName} removed from Language Models. Reload the window for it to disappear from the model picker and the manage list. Your API key and group settings are kept.`, + action.disable + ? `${displayName} removed from Language Models. Reload the window for it to disappear from the model picker and the manage list. Your API key and group settings are kept.` + : `${displayName} re-enabled. Reload the window for the provider to appear in Language Models again.`, "Reload Now", ); if (reload === "Reload Now") { diff --git a/src/provider/providerDialogs.ts b/src/provider/providerDialogs.ts index 4cc8d212f..815b230ff 100644 --- a/src/provider/providerDialogs.ts +++ b/src/provider/providerDialogs.ts @@ -5,7 +5,6 @@ import { auxiliarySessionId } from "../request/headers"; import type { ProviderVendor } from "../providerTypes"; import type { ProviderDefinition } from "./definitions"; import { configureUtilityModels, toggleProviderEnabled } from "../commands/providers"; -import { providerEnabledSetting } from "../providerEnablement"; /** * Provider management UI flows (gear-icon menu + connection test). Pure with @@ -22,18 +21,15 @@ export interface DialogDeps { /** Gear-icon quick-pick: test / refresh / utility models / diagnostics / enable. */ export async function manageProvider(deps: DialogDeps): Promise { - // Read via the base-vendor full key so agent variants (opencodego-agent, - // opencodezen-agent) follow the same switch as the vendor they mirror. - const providerEnabled = vscode.workspace.getConfiguration().get(providerEnabledSetting(deps.definition.vendor), true); const choice = await vscode.window.showQuickPick( [ { label: "Test Connection", action: "test" as const }, { label: "Refresh Models", action: "refresh" as const }, { label: "Configure Utility Models", action: "utility" as const }, { label: "Open Diagnostics", action: "diagnostics" as const }, - ...(providerEnabled - ? [{ label: "Remove from Language Models", action: "remove" as const }] - : [{ label: "Re-add to Language Models", action: "remove" as const }]), + // toggleProviderEnabled derives Remove vs Re-add from the current + // setting and confirms via its own quick-pick (issue #228). + { label: "Toggle Registration in Language Models…", action: "toggle" as const }, ], { title: `Manage ${deps.definition.displayName}`, @@ -45,7 +41,7 @@ export async function manageProvider(deps: DialogDeps): Promise { return; } - if (choice.action === "remove") { + if (choice.action === "toggle") { await toggleProviderEnabled(deps.definition.vendor, deps.definition.displayName); return; } From 1504375265a4d0fa7dd4ea07560001e4dbb30ed7 Mon Sep 17 00:00:00 2001 From: ltmoerdani Date: Wed, 23 Sep 2026 21:24:10 +0700 Subject: [PATCH 4/5] test(e2e): cover the #239 retry loop and #233 serialization decision chain - Extract the deferred tool-image emission into the pure withDeferredToolImageMessages() helper (request/shared.ts) so the glm-5.3* relocation is testable without a VS Code host; convertMessage now delegates to it (behavior unchanged). - Mock-server retry E2E: new deepseek-v4.1-flash validator scenario + full-loop and healthy-request cases (9/9). - Unit tests for the deferred-message shape and the requiresStringToolContent mapping (480 total). --- scripts/test-retry-e2e.ts | 65 +++++++++++++++++++++++++++++ src/provider/messages.ts | 25 ++++------- src/request/shared.ts | 32 +++++++++++++- src/test/deferredToolImages.test.ts | 41 ++++++++++++++++++ 4 files changed, 144 insertions(+), 19 deletions(-) create mode 100644 src/test/deferredToolImages.test.ts diff --git a/scripts/test-retry-e2e.ts b/scripts/test-retry-e2e.ts index 762f36d71..2f7cf9825 100644 --- a/scripts/test-retry-e2e.ts +++ b/scripts/test-retry-e2e.ts @@ -22,6 +22,7 @@ interface MockRequestBody { thinking?: { type?: unknown }; temperature?: unknown; reasoning_effort?: unknown; + messages?: unknown; } /** JSON.parse returns `any`; shape it into a minimal, typed view of the body. */ @@ -65,6 +66,36 @@ function createMockServer() { return; } + // DeepSeek V4.1 Flash thinking mode (issue #239): 400 when reasoning + // is still enabled OR any assistant history message carries a + // reasoning_content echo — mirrors the upstream validator that + // demands prior-turn reasoning be passed back. + if (model === "deepseek-v4.1-flash") { + const messages = Array.isArray(parsed.messages) ? parsed.messages : []; + const hasEcho = messages.some( + (m: unknown) => + typeof m === "object" && + m !== null && + (m as { role?: unknown; reasoning_content?: unknown }).role === "assistant" && + (m as { reasoning_content?: unknown }).reasoning_content !== undefined, + ); + if (parsed.reasoning_effort !== undefined || hasEcho) { + res.writeHead(400, { "Content-Type": "application/json" }); + res.end( + JSON.stringify({ + error: { + param: null, + type: "invalid_request_error", + code: "invalid_request_error", + message: + "Upstream request failed: [invalid_request_error] The `reasoning_content` in the thinking mode must be passed back to the API.", + }, + }), + ); + return; + } + } + // MiniMax M2.7: reject thinking.type "disabled" if (model === "minimax-m2.7" && parsed.thinking?.type === "disabled") { res.writeHead(400, { "Content-Type": "application/json" }); @@ -163,6 +194,40 @@ const TEST_CASES: TestCase[] = [ badBody: { model: "kimi-k2.5", messages: [{ role: "user", content: "Hi" }], max_tokens: 10, thinking: { type: "enabled" } }, expectedPatch: (b) => b, }, + { + // Issue #239: reasoning lost from history while thinking is still on. + // The upstream 400s demanding the echo back; the patch strips the echo + // AND reasoning_effort so the request is self-contained again. + name: "DeepSeek V4.1 Flash: reasoning echo missing → strip echo + effort", + badBody: { + model: "deepseek-v4.1-flash", + max_tokens: 10, + reasoning_effort: "low", + messages: [ + { role: "user", content: "do a task" }, + { role: "assistant", content: "working", reasoning_content: "lost chain of thought", tool_calls: [] }, + { role: "tool", tool_call_id: "call_1", content: "tool output" }, + ], + }, + expectedPatch: (b) => { + const messages = (b.messages as Array>).map((m) => + m.role === "assistant" ? { ...m, reasoning_content: undefined } : m, + ); + return { ...b, messages, reasoning_effort: undefined }; + }, + }, + { + name: "DeepSeek V4.1 Flash: no echo + no effort → 200 (healthy request)", + badBody: { + model: "deepseek-v4.1-flash", + max_tokens: 10, + messages: [ + { role: "user", content: "do a task" }, + { role: "assistant", content: "working" }, + ], + }, + expectedPatch: (b) => b, + }, ]; // --------------------------------------------------------------------------- diff --git a/src/provider/messages.ts b/src/provider/messages.ts index c27eb5762..72378dc06 100644 --- a/src/provider/messages.ts +++ b/src/provider/messages.ts @@ -4,6 +4,7 @@ import { requiresStringToolContent } from "../models/modelCapabilities"; import { MAX_HISTORY_IMAGES_KEPT, MAX_TOOL_RESULT_IMAGE_BYTES } from "../config"; import { getImageDataUrlBase64Bytes, MAX_IMAGE_BASE64_BYTES, normalizeImageDataUrl } from "../imageNormalizer"; import { shouldEchoThinkingHistory, thinkingTextFromValue } from "../reasoningHistory"; +import { withDeferredToolImageMessages } from "../request/shared"; import type { ApiMessage, OpenAiContentPart, OpenAiToolCall } from "../request/types"; import { partToText } from "./tokens"; import type { ConvertedMessageResult } from "./definitions"; @@ -35,24 +36,12 @@ export async function convertMessage( }; const finish = (messages: ApiMessage[]): ConvertedMessageResult => { - const result = [...messages]; - if (deferredToolImageParts.length > 0) { - // Tool images rejected in role:"tool" content but accepted in user - // content (verified against zen/go/v1 chat-completions for - // glm-5.3-flash: tool image_url → 422, user image_url → 200). Append - // them as a follow-up user message so vision capability is preserved. - result.push({ - role: "user", - content: [ - { - type: "text", - text: "Images returned by a tool result (the upstream provider does not accept images inside tool messages):", - }, - ...deferredToolImageParts, - ], - }); - } - return { messages: result, normalizedImageCount }; + // Tool images rejected in role:"tool" content but accepted in user + // content (verified against zen/go/v1 chat-completions for + // glm-5.3-flash: tool image_url → 422, user image_url → 200) are + // appended as a follow-up user message so vision capability is + // preserved. See withDeferredToolImageMessages for the CONTRACT. + return { messages: withDeferredToolImageMessages(messages, deferredToolImageParts), normalizedImageCount }; }; for (const part of message.content) { diff --git a/src/request/shared.ts b/src/request/shared.ts index b663fb6e0..9e72f24d5 100644 --- a/src/request/shared.ts +++ b/src/request/shared.ts @@ -3,9 +3,39 @@ * * CONTRACT: pure functions only — no `vscode` import, no side effects. */ -import type { ApiMessage } from "./types"; +import type { ApiMessage, OpenAiContentPart } from "./types"; /** Whether any message in the conversation carries an image part. */ export function messagesHaveImages(messages: readonly ApiMessage[]): boolean { return messages.some((message) => Array.isArray(message.content) && message.content.some((part) => part.type === "image_url")); } + +/** Leading note on deferred tool-image messages (see below). */ +export const DEFERRED_TOOL_IMAGES_NOTE = + "Images returned by a tool result (the upstream provider does not accept images inside tool messages):"; + +/** + * Append tool-result images that were deferred out of role:"tool" messages as + * a follow-up user message (issue #233). Some chat-completions upstreams + * (glm-5.3*) reject image_url parts in tool messages with 422 while accepting + * identical images in user messages — moving them preserves vision instead of + * dropping them. + * + * Pure: returns a new array; the input is not mutated. When there is nothing + * deferred, the input array is returned unchanged. + */ +export function withDeferredToolImageMessages( + messages: readonly ApiMessage[], + deferredImageParts: readonly OpenAiContentPart[], +): ApiMessage[] { + if (deferredImageParts.length === 0) { + return [...messages]; + } + return [ + ...messages, + { + role: "user", + content: [{ type: "text", text: DEFERRED_TOOL_IMAGES_NOTE }, ...deferredImageParts], + }, + ]; +} diff --git a/src/test/deferredToolImages.test.ts b/src/test/deferredToolImages.test.ts new file mode 100644 index 000000000..5763ef1c2 --- /dev/null +++ b/src/test/deferredToolImages.test.ts @@ -0,0 +1,41 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { DEFERRED_TOOL_IMAGES_NOTE, withDeferredToolImageMessages } from "../request/shared.js"; +import type { ApiMessage, OpenAiContentPart } from "../request/types.js"; + +const imagePart: OpenAiContentPart = { type: "image_url", image_url: { url: "data:image/png;base64,AAAA" } }; + +describe("withDeferredToolImageMessages (issue #233)", () => { + it("returns messages unchanged when nothing is deferred", () => { + const messages: ApiMessage[] = [ + { role: "user", content: "hi" }, + { role: "assistant", content: null, tool_calls: [] }, + ]; + const result = withDeferredToolImageMessages(messages, []); + assert.deepEqual(result, messages); + assert.equal(result.length, 2); + }); + + it("appends one user message carrying the deferred tool images", () => { + const messages: ApiMessage[] = [ + { role: "user", content: "hi" }, + { role: "tool", tool_call_id: "call_1", content: "[Tool returned an image attachment; it is included in the following message.]" }, + ]; + const result = withDeferredToolImageMessages(messages, [imagePart]); + assert.equal(result.length, 3); + const deferred = result[2]; + assert.equal(deferred.role, "user"); + assert.ok(Array.isArray(deferred.content)); + const textPart = deferred.content[0] as { type: string; text: string }; + assert.equal(textPart.type, "text"); + assert.equal(textPart.text, DEFERRED_TOOL_IMAGES_NOTE); + assert.deepEqual(deferred.content[1], imagePart); + }); + + it("does not mutate the input array", () => { + const messages: ApiMessage[] = [{ role: "tool", tool_call_id: "call_1", content: "text only" }]; + const snapshot = structuredClone(messages); + withDeferredToolImageMessages(messages, [imagePart]); + assert.deepEqual(messages, snapshot); + }); +}); From cef2b4b7998ff179626bb85fb6beab02ff36b832 Mon Sep 17 00:00:00 2001 From: ltmoerdani Date: Wed, 23 Sep 2026 21:24:30 +0700 Subject: [PATCH 5/5] docs: issue docs + changelog + architecture map for #239, #233, #228 --- ARCHITECTURE-MAP.md | 29 ++++---- CHANGELOG.md | 6 ++ ...60923-issue239-reasoning-echo-self-heal.md | 62 ++++++++++++++++ ...-20260923-issue233-glm-tool-image-defer.md | 72 +++++++++++++++++++ ...0923-issue228-provider-toggle-wrong-key.md | 52 ++++++++++++++ 5 files changed, 208 insertions(+), 13 deletions(-) create mode 100644 docs/issues/102-20260923-issue239-reasoning-echo-self-heal.md create mode 100644 docs/issues/103-20260923-issue233-glm-tool-image-defer.md create mode 100644 docs/issues/104-20260923-issue228-provider-toggle-wrong-key.md diff --git a/ARCHITECTURE-MAP.md b/ARCHITECTURE-MAP.md index 447a56bdd..abe0aa7d8 100644 --- a/ARCHITECTURE-MAP.md +++ b/ARCHITECTURE-MAP.md @@ -35,17 +35,17 @@ Total `src/` ≈ **16,310 lines** across ~109 files (excl. tests). Grouped by do | Domain | Folder / Files | LoC | Owns | Key contracts | | --------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| **Provider** | `src/provider/` — `OpenCodeProvider.ts` (532), `chatPrep.ts` (332), `messages.ts` (430), `modelInfo.ts` (239), `historyTrim.ts` (200), `settings.ts` (260), `modelList.ts` (185), `visionProxy.ts` (345), `providerDialogs.ts` (118), `transportLog.ts` (109), `definitions.ts` (238), `tokens.ts` (106), `providerUtils.ts` (35) | ~3,009 | The `OpenCodeProvider` class implementing `LanguageModelChatProvider` (thin — delegates to siblings via deps objects), request preparation (`chatPrep`: message conversion, vision proxy, history trimming, budgets, thinking payload), model-info assembly (`modelInfo`), model-list fetch/cache (`modelList`), Manage/Test-Connection flows (`providerDialogs`), rolling transport diagnostics (`transportLog`) | `prepareChatRequest()` / `provideModelChatInformation()` / `convertMessage()` / `normalizeMessages()` / `trimOldMessagesToFitContext()` / `historyByteCapForBudget()` / `ModelListFetcher` | +| **Provider** | `src/provider/` — `OpenCodeProvider.ts` (532), `chatPrep.ts` (332), `messages.ts` (446), `modelInfo.ts` (239), `historyTrim.ts` (200), `settings.ts` (260), `modelList.ts` (185), `visionProxy.ts` (345), `providerDialogs.ts` (117), `transportLog.ts` (109), `definitions.ts` (238), `tokens.ts` (106), `providerUtils.ts` (35) | ~3,017 | The `OpenCodeProvider` class implementing `LanguageModelChatProvider` (thin — delegates to siblings via deps objects), request preparation (`chatPrep`: message conversion, vision proxy, history trimming, budgets, thinking payload), model-info assembly (`modelInfo`), model-list fetch/cache (`modelList`), Manage/Test-Connection flows (`providerDialogs`), rolling transport diagnostics (`transportLog`) | `prepareChatRequest()` / `provideModelChatInformation()` / `convertMessage()` / `normalizeMessages()` / `trimOldMessagesToFitContext()` / `historyByteCapForBudget()` / `ModelListFetcher` | | **Transports** | `src/transports/` — `engine.ts` (409), `extractors.ts` (556), `extract.ts` (225), `thinkTags.ts` (139), `streamParts.ts` (88), `sse.ts` (31), `chatCompletions.ts` (64), `responses.ts` (30), `anthropic.ts` (28), `google.ts` (31) | ~1,601 | One adapter per wire format (OpenAI chat-completions, OpenAI Responses, Anthropic Messages, Google generateContent) + the shared streaming engine, SSE parser, response extractors | `streamOpenCodeResponse()` (engine), `OpenAiResponseExtractor` / `AnthropicResponseExtractor` | | **Core (registry/routing)** | `src/core/` — `routing.ts` (441), `registry.ts` (142), `transport.ts` (70) | ~653 | Data-driven model registry (`MODEL_REGISTRY`), transport resolution (`resolveModelRouting`), Responses/Google SSE normalization, shared `StreamRequestOptions` contract | **Pure** — no `vscode` import, no side effects | -| **Models (metadata)** | `src/models/` — `metadata.ts` (523), `modelTables.ts` (141), `metadataFetcher.ts` (102), `modelLimits.ts` (52), `modelCapabilities.ts` (16), `modelNames.ts` (29), `pricing.ts` (88) | ~951 | models.dev live metadata + bundled fallback snapshot (static data tables in `modelTables.ts`), limit/capability resolution, pricing | Live fetch may fail → bundled snapshot MUST exist | +| **Models (metadata)** | `src/models/` — `metadata.ts` (523), `modelTables.ts` (141), `metadataFetcher.ts` (102), `modelLimits.ts` (52), `modelCapabilities.ts` (41), `modelNames.ts` (29), `pricing.ts` (88) | ~976 | models.dev live metadata + bundled fallback snapshot (static data tables in `modelTables.ts`), limit/capability resolution, pricing | Live fetch may fail → bundled snapshot MUST exist | | **Usage** | `src/usage/` — `tracker.ts` (585, thin class shell), `trackerTypes.ts` (96), `trackerWindows.ts` (140), `trackerSummary.ts` (238), `dashboard.ts` (19 barrel) + `dashboard/` (`webview.ts`, `webviewData.ts`, `webviewHtml.ts`, `state.ts`, `statusBar.ts`, `targetEditor.ts`, `tooltip.ts` ≈ 1,405), `history.ts` (378), `usage.ts` (146), `goUsageSync.ts` (128), `formatting.ts` (129), `usageProfile.ts` (74), `pricing.ts` (62) | ~3,571 | Go usage tracker (types/windows/summary split out of the old god file), per-profile tracking, CLI SQLite history reader, server-usage sync, status bar + usage webview + quick-pick (webview split into state/status/webview modules) | Server meters authoritative for Session/Weekly/Monthly; device-local for Today/Yesterday | | **Thinking** | `src/thinking/` — `provider.ts` (78), `base.ts` (75), `resolve.ts` (94), `deepseek.ts` (53), `glm.ts` (53), `kimi.ts` (81), `minimax.ts` (54), `mimo.ts` (72), `openai.ts` (57), `qwen.ts` (103), `fallback.ts` (39), `schema.ts` (106), `payload.ts` (26), `types.ts` (51) | ~942 | Per-family thinking strategy classes + config resolution (per-model config wins over workspace — but schema-default echoes are stripped first, `stripSchemaDefaultEcho` in `resolve.ts`, issue #226) | **Pure** — no `vscode` import; family from registry | -| **Request builders** | `src/request/` — `anthropic.ts` (233), `google.ts` (176), `types.ts` (117), `schema.ts` (94), `openai.ts` (95), `headers.ts` (110), `builders.ts` (17), `shared.ts` (11) | ~853 | Per-endpoint request-body builders + shared header builders (`x-opencode-session` / `x-opencode-request`) | `builders.ts` is the public barrel | -| **Commands** | `src/commands/` — `agentsWindow.ts` (130), `diagnostics.ts` (41), `providers.ts` (38), `thinkingPicker.ts` (31) | ~240 | Command handlers: diagnostics, agents-window BYOK bridge, provider enable/disable, thinking picker | Thin — delegates to provider/usage modules | +| **Request builders** | `src/request/` — `anthropic.ts` (233), `google.ts` (176), `types.ts` (117), `schema.ts` (94), `openai.ts` (95), `headers.ts` (110), `builders.ts` (17), `shared.ts` (41) | ~883 | Per-endpoint request-body builders + shared header builders (`x-opencode-session` / `x-opencode-request`) | `builders.ts` is the public barrel | +| **Commands** | `src/commands/` — `agentsWindow.ts` (130), `diagnostics.ts` (41), `providers.ts` (72), `thinkingPicker.ts` (31) | ~274 | Command handlers: diagnostics, agents-window BYOK bridge, provider enable/disable (state-aware toggle, base-vendor resolved — issue #228), thinking picker | Thin — delegates to provider/usage modules | | **Autocomplete** | `src/autocomplete/` — `index.ts` (157), `engine.ts` (143), `provider.ts` (127), `context.ts` (89), `usage.ts` (88), `throttle.ts` (79), `prompt.ts` (60), `types.ts` (32) | ~775 | Inline code suggestions (opt-in) — FIM emulation over chat-completions, debounce/throttle, usage counters | Separate subsystem; not wired into Go cost tracker yet | | **Extension entry** | `src/extension.ts` | 415 | Thin `activate()`/`deactivate()` — wiring only (command registration, provider registration, status bar init) | Target <300 lines; keep wiring-only | -| **Root utilities** | `src/config.ts` (278), `contextWindowHook.ts` (485), `contextWindowHookBridge.ts` (122), `errors.ts` (294), `retry.ts` (255), `utils.ts` (186), `responsesRequest.ts` (180), `toolCallAccumulator.ts` (138), `imageNormalizer.ts` (118), `visionProxyCache.ts` (79), `runtimeDiagnostics.ts` (57), `chatParts.ts` (55), `reasoningHistory.ts` (43), `tokenEstimate.ts` (39), `providerTypes.ts` (23), `openCodeAuth.ts` (20), `providerEnablement.ts` (18), `apiKeyResolution.ts` (8), `thinking.ts` (30, legacy barrel) | ~2,428 | Cross-cutting utilities (plus the two proposed-API `.d.ts` module augmentations ≈ 231 LoC — `chatProvider` v6 + `languageModelThinkingPart` v1) | `config.ts` must stay **dependency-free** | +| **Root utilities** | `src/config.ts` (278), `contextWindowHook.ts` (485), `contextWindowHookBridge.ts` (122), `errors.ts` (294), `retry.ts` (463), `utils.ts` (186), `responsesRequest.ts` (180), `toolCallAccumulator.ts` (138), `imageNormalizer.ts` (118), `visionProxyCache.ts` (79), `runtimeDiagnostics.ts` (57), `chatParts.ts` (55), `reasoningHistory.ts` (43), `tokenEstimate.ts` (39), `providerTypes.ts` (23), `openCodeAuth.ts` (20), `providerEnablement.ts` (18), `apiKeyResolution.ts` (8), `thinking.ts` (30, legacy barrel) | ~2,428 | Cross-cutting utilities (plus the two proposed-API `.d.ts` module augmentations ≈ 231 LoC — `chatProvider` v6 + `languageModelThinkingPart` v1) | `config.ts` must stay **dependency-free** | --- @@ -280,7 +280,7 @@ flowchart LR ```mermaid flowchart TD - A[resolve apiKey: BYOK config → per-model cache → SecretStorage cold-start] --> B[convertMessage per message
tool calls / tool results / images / thinking echo] + A[resolve apiKey: BYOK config → per-model cache → SecretStorage cold-start] --> B[convertMessage per message
tool calls / tool results / images / thinking echo
tool-result images deferred per model: drop (mimo) · user-message defer (glm-5.3*, #233)] B --> C[flatten messages + source index] C --> D[resolve thinking config
per-model config wins, schema-default echo stripped
issue #226] D --> E[vision proxy: text-only model + images → relay to vision model] @@ -366,6 +366,7 @@ Reuse these before writing new logic (all under `src/` root unless noted): | `runtimeDiagnostics.ts` | `runtimeDiagnosticsLines()` — version/host/platform/integrity lines for the Diagnostics command | provider | | `request/headers.ts` | `buildOpenCodeRequestHeaders` (sticky session/request id) | provider | | `request/builders.ts` | public barrel for body builders per endpoint | provider | +| `request/shared.ts` | `messagesHaveImages`, `withDeferredToolImageMessages` (tool-image defer, #233) — pure, no `vscode` import | provider | --- @@ -411,17 +412,17 @@ Reuse these before writing new logic (all under `src/` root unless noted): ## 8. Tests, Tooling & CI -### Unit tests (`src/test/` — 34 files, 466 cases) +### Unit tests (`src/test/` — 37 files, 480 cases) -Pure/domain modules get co-located tests. **Harness:** Node's built-in `node --test` runner via `scripts/run-unit-tests.ts`, which collects the compiled `out/test/*.test.js` files; tests import from compiled `out/` modules with explicit `.js` extensions (Node16/ESM-style) and use `node:test` + `node:assert/strict`. Tests never need a live model — they cover the deterministic parts (message conversion, chunk parsing, token estimation, routing). Per-file case counts (466 total, audited 2026-09-23): +Pure/domain modules get co-located tests. **Harness:** Node's built-in `node --test` runner via `scripts/run-unit-tests.ts`, which collects the compiled `out/test/*.test.js` files; tests import from compiled `out/` modules with explicit `.js` extensions (Node16/ESM-style) and use `node:test` + `node:assert/strict`. Tests never need a live model — they cover the deterministic parts (message conversion, chunk parsing, token estimation, routing). Per-file case counts (480 total, audited 2026-09-23): | Test file | Cases | | Test file | Cases | | --------------------------------- | ----- | --- | ------------------------------- | ----- | | `thinking.test.ts` | 71 | | `goUsageTrackerWindows.test.ts` | 17 | -| `retry.test.ts` | 35 | | `responsesRequest.test.ts` | 16 | +| `retry.test.ts` | 37 | | `responsesRequest.test.ts` | 16 | | `goUsageTracker.test.ts` | 35 | | `registry.test.ts` | 16 | | `metadata.test.ts` | 28 | | `routing.test.ts` | 15 | -| `utils.test.ts` | 22 | | `messages.test.ts` | 15 | +| `utils.test.ts` | 22 | | `messages.test.ts` | 19 | | `autocomplete.test.ts` | 21 | | `extractors.test.ts` | 15 | | `toolCallAccumulator.test.ts` | 19 | | `config.test.ts` | 15 | | `visionProxy.test.ts` | 17 | | `openai-request.test.ts` | 12 | @@ -431,9 +432,11 @@ Pure/domain modules get co-located tests. **Harness:** Node's built-in `node --t | `schema.test.ts` | 7 | | `modelLimits.test.ts` | 5 | | `imageNormalizer.test.ts` | 5 | | `session-header.test.ts` | 4 | | `issue216-217-regression.test.ts` | 4 | | `providerEnablement.test.ts` | 3 | -| `modelNames.test.ts` | 3 | | `engine.test.ts` | 3 | -| `chatParts.test.ts` | 3 | | `apiKeyResolution.test.ts` | 3 | -| `tokenEstimate.test.ts` | 2 | | `agentProvider.test.ts` | 1 | +| `deferredToolImages.test.ts` | 3 | | `modelCapabilities.test.ts` | 3 | +| `modelNames.test.ts` | 3 | | `apiKeyResolution.test.ts` | 3 | +| `chatParts.test.ts` | 3 | | `engine.test.ts` | 3 | +| `tokenEstimate.test.ts` | 2 | | `headers.test.ts` | 2 | +| | | | `agentProvider.test.ts` | 1 | ### Scripts (`scripts/`) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b2be9cd3..54da4ce1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ All notable changes to the **OpenCode Go BYOK Provider** extension are documente ### Fixed +- **`[Provider]` Tool-result images on `glm-5.3*` are deferred to a user message instead of 422-ing every turn (#233).** The Console Go upstream rejects `image_url` parts inside `role: "tool"` content with `422 Input should be a valid string` while accepting identical images in user messages (verified with a direct gateway repro matrix in the issue). New `requiresStringToolContent()` mapping: `mimo-*` keeps the existing drop-with-placeholder behavior (#38, unchanged), `glm-5.3*` flattens the tool message to a string and moves the images into a follow-up user message via the new pure `withDeferredToolImageMessages()` helper (vision preserved), everyone else keeps multimodal tool content unchanged. Documented in `docs/issues/103-20260923-issue233-glm-tool-image-defer.md`. + +- **`[Retry]` DeepSeek thinking-mode 400s self-heal when history loses the `reasoning_content` echo (#239).** When Copilot Chat compaction, history trimming, or a pre-thinking-capture turn strips prior-turn reasoning from the replayed history, DeepSeek V4.1 Flash rejected every follow-up turn with `The reasoning_content in the thinking mode must be passed back to the API` — and retries could never recover. A new recoverable-400 pattern strips the echo from assistant messages **and** turns `reasoning_effort` off (stopping the lose-echo→400 cycle), executed by the existing 400-patch retry loop; no-ops when there is nothing to strip. Documented in `docs/issues/102-20260923-issue239-reasoning-echo-self-heal.md`. + +- **`[Providers]` Toggle Provider Registration is now idempotent and no longer writes a dead settings key (#228).** Two defects: agent-variant definitions (e.g. the gear-icon manage flow on `opencodezen-agent`) wrote `.enabled` — a key the provider's `when` clause never reads — leaving the provider permanently removed while settings looked enabled; and the command was a blind toggle behind a "Remove/Re-add" title, so running it twice could never re-add without a reload in between. `toggleProviderEnabled` now resolves agent variants to their base vendor before touching configuration and derives Remove vs Re-add from the current setting via a confirmable quick-pick; command titles reworded to "Toggle Provider Registration in Language Models". Documented in `docs/issues/104-20260923-issue228-provider-toggle-wrong-key.md`. + - **`[Provider]` History-trim cuts are cache-stable — a session at the context ceiling keeps its prefix-cache hits (~99% instead of ~11%).** When the trimmed history landed just under the input budget, the minimal-fit trim moved the cut point on nearly every following turn — and the provider's prefix cache only reuses the bytes before the first changed message, so each moved cut re-billed the whole conversation at full input price (measured on a 614K-token session: hit rate collapsed from ~99% to ~11.4%, with only system + tools — 69,888 tokens — still cached; 207 trims fired across a ~12-hour span). Two changes now keep the cut still, with the same unit granularity and tool-group safety rules: a **low-water mark** (`budget − headroom`, new `HISTORY_TRIM_HEADROOM_*` constants: 3% of the budget, clamped to 8,192–32,768 tokens, never more than 10% of a small budget) and **cut-step alignment** to the next `HISTORY_TRIM_CUT_STEP_TOKENS` (32,768, capped at 10% of the budget) boundary of dropped payload. The step is what makes it robust: the crossing alone still hugs the mark within one unit, so sessions with ~2.7K-token units against ~1K of growth per request kept moving the cut every 1-3 requests (the live evening run: 12.4% misses for hours, hit rate down to 37-68%), while a smaller-unit morning session only looked stable by luck (large tool-result units). Simulation with production parameters: cut moves fall from 84/300 to 12/300 (evening regime) and 244/300 to 14/300 (smaller-unit regime). Four tests pin the low-water landing, the no-re-trim behavior, the re-supplied-history shape (constant cut → nested payload prefixes), and the cut-step stability; the first live ceiling crossing confirmed 15 trims with every landing ≤ 595,534 tokens (budget 613,952) and high-context misses down from 100/243 pre-fix to 3/55. Documented in `docs/issues/101-20260920-history-trim-cache-hysteresis.md`. - **`[Thinking]` Global `opencodego.thinking.*` settings now take effect for models without a per-model pick (#226).** VS Code merges our picker schema defaults into the per-model `modelConfiguration` on every request, so any reasoning-capable model the user never configured arrived with `reasoningEffort: "off"` attached — and the resolver treated any delivered `modelConfiguration` as the single authority, letting the echoed `"off"` beat the global setting every time (the #214 symptom; diagnosis by @nickchomey). `resolveThinkingConfig` now strips override keys equal to the family's picker schema default before applying them — lossless, because VS Code itself strips default-equal values when persisting user picks, so such a value can never be a genuine user choice. Non-default per-model picks still win; the Agents-window default path is untouched (removing the schema default instead would have made host-side fallbacks pick `medium`/`high`). Documented in `docs/issues/100-20260923-issue226-thinking-default-echo.md`. diff --git a/docs/issues/102-20260923-issue239-reasoning-echo-self-heal.md b/docs/issues/102-20260923-issue239-reasoning-echo-self-heal.md new file mode 100644 index 000000000..20dafe3f6 --- /dev/null +++ b/docs/issues/102-20260923-issue239-reasoning-echo-self-heal.md @@ -0,0 +1,62 @@ +# Issue #239 — DeepSeek 400 "reasoning_content must be passed back": Self-Healing Retry When History Loses the Echo + +**Status:** ✅ Solved — implemented + verified end-to-end +**Topic:** retry / thinking / deepseek +**Updated:** 2026-09-23 +**Tags:** #retry #thinking #deepseek #resilience +**GitHub Issue:** [ltmoerdani/opencode-copilot-chat#239](https://github.com/ltmoerdani/opencode-copilot-chat/issues/239) +**Related:** issue doc [55 — PR123 deepseek reasoning_content echo](55-20260811-pr123-deepseek-reasoning-content-echo.md), issue doc [34 — MCP tool result image](34-20260720-mcp-tool-result-image-dropped.md), issue doc [102 — #233 glm tool-image defer](102-20260923-issue233-glm-tool-image-defer.md) + +--- + +## Problem + +`deepseek-v4.1-flash` with thinking effort `low` failed on multi-turn agent conversations: + +```text +[http-error-body] {"error":{"param":null,"type":"invalid_request_error","code": +"invalid_request_error","message":"Upstream request failed: [invalid_request_error] +The `reasoning_content` in the thinking mode must be passed back to the API."}} +[http] 400 Bad Request +``` + +Every retry failed identically; only starting a new conversation recovered. Reported by @itsmorty (0.7.5). + +## Root Cause + +The echo itself is implemented correctly (`src/reasoningHistory.ts` + `src/provider/messages.ts` — see doc 55). The failure appears when the reasoning **disappears from the replayed history** while thinking mode is still on: + +1. **Copilot Chat conversation summarization/compaction** — summarized history carries no reasoning parts (the same mechanism issue #232 users tune via `summarizeAgentConversationHistoryThreshold`). +2. **History trimming** (`trimOldMessagesToFitContext`) can cut an assistant turn that carried the reasoning. +3. **Conversations started before thinking capture was active.** + +Once one turn ships without the echo, DeepSeek's validator rejects it, and since the history shape never changes between retries, every subsequent turn 400s forever — the exact "every retry same session → 400" pattern from the MiMo saga (doc 34 family). + +## Fix — recoverable-400 self-heal pattern (extension-side, post-serialization) + +One new entry in `RECOVERABLE_ERROR_PATTERNS` (`src/retry.ts`), executed by the existing `MAX_400_PATCH_ATTEMPTS` loop in `transports/engine.ts` (same mechanism as #190/#171): + +- **Pattern:** `/reasoning_content`? in the thinking mode must be passed back/i` — matches the verbatim upstream body (backtick-safe). +- **Patch:** strip `reasoning_content` from every assistant message **and** set `reasoning_effort: undefined`. +- **Why also drop `reasoning_effort`:** with thinking still enabled, the next response emits new reasoning that compaction strips again — the turn after would 400 again. Turning thinking off makes the request self-contained and **stops the cycle** for the rest of the conversation, not just one turn. +- **No-op guard:** the patch only fires when something actually changes (`analyzeHttp400ForRetry`'s existing JSON-diff check), so a genuinely healthy request is never patched and real failures still surface unchanged. + +The patch runs **after** message conversion on the wire body — zero contact with the conversion, vision, or tool-call paths. + +## Files Changed + +| File | Change | +| --------------------------- | ------------------------------------------------------------------------------------------------------------ | +| `src/retry.ts` | New recoverable-400 pattern (echo + effort strip), with issue/rationale comment | +| `src/test/retry.test.ts` | 2 tests: patch correctness (echo + effort stripped, other content untouched) and no-op when nothing to strip | +| `scripts/test-retry-e2e.ts` | Mock-server scenario replicating the DeepSeek validator (400 until echo+effort absent) + 2 e2e cases | + +## Verification + +- `npm run lint` (full 7-check gate) pass; 480/480 unit tests pass. +- Mock-server retry E2E (`npx tsx scripts/test-retry-e2e.ts`) — 9/9 pass, including the new full-loop scenario (request → 400 → analyze → patch → retry → 200) and the healthy-request no-retry case. +- Real-model manual test (Copilot Chat, deepseek-v4.1-flash + effort low) — PASS (2026-09-23). + +--- + +Detected 2026-09-22 | Reported by @itsmorty | Fixed 2026-09-23 diff --git a/docs/issues/103-20260923-issue233-glm-tool-image-defer.md b/docs/issues/103-20260923-issue233-glm-tool-image-defer.md new file mode 100644 index 000000000..4d3209824 --- /dev/null +++ b/docs/issues/103-20260923-issue233-glm-tool-image-defer.md @@ -0,0 +1,72 @@ +# Issue #233 — glm-5.3-flash 422 on Images: Tool-Result Images Deferred to a User Message + +**Status:** ✅ Solved — implemented + verified end-to-end +**Topic:** provider / serialization / vision / chat-completions +**Updated:** 2026-09-23 +**Tags:** #tool-calling #vision #serialization #chat-completions +**GitHub Issue:** [ltmoerdani/opencode-copilot-chat#233](https://github.com/ltmoerdani/opencode-copilot-chat/issues/233) +**Related:** issue doc [34 — MCP tool result image](34-20260720-mcp-tool-result-image-dropped.md), issue doc [75 — #173 vision byte cap](75-20260821-issue173-vision-byte-cap.md), issue doc [102 — #239 reasoning echo self-heal](102-20260923-issue239-reasoning-echo-self-heal.md) + +--- + +## Problem + +On `glm-5.3-flash` (OpenCode Go), any conversation whose history contains a **tool result carrying an image** (e.g. a browser/screenshot MCP tool) failed on every subsequent turn: + +```text +OpenCode Go API request failed (422) model=glm-5.3-flash payloadBytes=949611: +Error from provider (Console Go): Upstream request failed: [invalid_request_error] +Input should be a valid string +``` + +The tool result stays in the history, so like the MiMo #38 family, the failure repeats on every follow-up turn until a new conversation. Reported by @felocru (0.7.5). + +## Root Cause (verified with a direct gateway reproducer) + +Excellent community diagnosis in the issue (minimal repro matrix against `zen/go/v1/chat/completions`, model `glm-5.3-flash`): + +| # | Payload shape | Result | +| --- | ---------------------------------------------- | -------------------------------------------------------------------- | +| 1 | user message, string content | 200 | +| 2 | user message, array with `image_url` | **200 — vision works** | +| 3 | user message, two text parts | 200 | +| 4 | assistant `tool_calls` with string arguments | 200 | +| 5 | tool message, array of one **text** part | 200 | +| 6 | **tool message, array containing `image_url`** | **422 — this issue** | +| 7 | assistant `tool_calls` with object arguments | 422 (not reachable from our serializer — we always `JSON.stringify`) | + +So the upstream **does** accept images in user messages — only **list-type content on `role: "tool"` messages** is rejected (the gateway returns the offending field, e.g. `messages.2.tool.content.str`). Our converter emits a multimodal array on tool messages whenever a tool result carries an image and the model isn't MiMo (doc 34) — glm-5.3-flash's Console Go upstream rejects exactly that. Notably it is a **422**, so the engine's 400-patch retry loop never engages — serialization is the only correct fix layer. + +## Fix — per-upstream handling mode, images preserved + +New pure mapping `requiresStringToolContent(rawModelId)` (`src/models/modelCapabilities.ts`): + +| Mode | Models | Handling | +| --------- | ------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `"drop"` | `mimo-*` | Unchanged #38 behavior: flatten to string, images replaced with placeholder notes (upstream cannot see tool images at all) | +| `"defer"` | `glm-5.3*` | Tool message flattened to a string with a pointer note; images **moved into a follow-up user message** — vision preserved (upstream accepts images there, per repro #2) | +| `null` | everyone else | Multimodal tool content forwarded unchanged (kimi, glm-5.1/5.2, minimax, qwen) | + +The deferred emission is a pure helper `withDeferredToolImageMessages()` (`src/request/shared.ts`, CONTRACT: no `vscode` import) called from `convertMessage()`'s `finish()`, so the appended user message lands after the tool results in the same conversion output and flows through the normal normalization/trim paths. + +## Files Changed + +| File | Change | +| ------------------------------------------ | ---------------------------------------------------------------------------------------------------- | +| `src/models/modelCapabilities.ts` | `requiresStringToolContent()` — evidence-based per-upstream mapping (drop/defer/null) | +| `src/request/shared.ts` | `withDeferredToolImageMessages()` + `DEFERRED_TOOL_IMAGES_NOTE` (pure, unit-tested) | +| `src/provider/messages.ts` | Tool branch reworked to the 3-mode gate; MiMo path byte-identical; `finish()` emits deferred message | +| `src/test/modelCapabilities.test.ts` | 3 tests: defer for glm-5.3*, drop for mimo, null for other families | +| `src/test/deferredToolImages.test.ts` | 3 tests: append shape, no-op when empty, input not mutated | +| `tmp/e2e-issues-233-239-serialization.mjs` | Serialization e2e simulation (decision chain + wire shapes, 13 checks) | + +## Verification + +- `npm run lint` (full 7-check gate) pass; 480/480 unit tests pass. +- E2E simulation (`tmp/e2e-issues-233-239-serialization.mjs`) — 13/13 pass, including regression guards: MiMo behavior unchanged (no deferred message), kimi-k3 multimodal forwarded unchanged, text-only tool results never change shape. +- Real-model manual test (Copilot Chat, glm-5.3-flash + screenshot tool, multi-turn) — PASS (2026-09-23). +- Note: `tool_calls[].function.arguments` (reproducer #7) is already always a string from our serializer — no change needed. + +--- + +Detected 2026-09-16 | Reported by @felocru | Fixed 2026-09-23 diff --git a/docs/issues/104-20260923-issue228-provider-toggle-wrong-key.md b/docs/issues/104-20260923-issue228-provider-toggle-wrong-key.md new file mode 100644 index 000000000..2196b50b9 --- /dev/null +++ b/docs/issues/104-20260923-issue228-provider-toggle-wrong-key.md @@ -0,0 +1,52 @@ +# Issue #228 — `toggleProvider` Removes the Provider but "Re-add" Never Brings It Back + +**Status:** ✅ Solved — implemented + verified end-to-end +**Topic:** provider / commands / configuration +**Updated:** 2026-09-23 +**Tags:** #provider #byok #commands #configuration +**GitHub Issue:** [ltmoerdani/opencode-copilot-chat#228](https://github.com/ltmoerdani/opencode-copilot-chat/issues/228) +**Related:** issue doc [93 — #214 thinking settings scope](93-20260903-issue214-thinking-settings-scope.md) (same section-scoped-config failure class) + +--- + +## Problem + +Running **OpenCode Zen: Remove/Re-add Provider in Language Models** removed the provider; running it again did not re-add it. The only workaround was manually deleting `"opencodezen.enabled": false` from `settings.json`. + +## Root Cause — two defects, both confirmed from code + +### 1. Wrong-key write on agent variants + +`manageProvider` (gear-icon flow) reads the current state correctly via `providerEnabledSetting(deps.definition.vendor)`, which resolves agent variants to their base vendor (`opencodezen-agent` → `opencodezen`). But it then called `toggleProviderEnabled(deps.definition.vendor, …)`, which writes to the **section-scoped configuration of that vendor** — for an agent variant that is `opencodezen-agent.enabled`, a key **nothing reads**: the vendor contribution's `when` clause and the settings schema only know `config.opencodezen.enabled`. + +Result: settings look "enabled" while the provider stays unregistered — permanent divergence between config and runtime. (Class sibling of the #214 scope bug: section-scoped reads/writes vs. root keys.) + +### 2. Blind toggle + reload dependency + +The command flipped `enabled` unconditionally (`next = !current`) while its title promised "Remove/Re-add". Provider (de)registration only happens at startup (`onLanguageModelChatProvider` activation), so re-adding requires a window reload — invoking the command twice without reloading left the picker empty even though the setting had flipped back. + +## Fix + +`src/commands/providers.ts` — `toggleProviderEnabled` rebuilt: + +1. **Base-vendor resolution first:** `resolveBaseVendor(vendor)` before any configuration access, mirroring the `providerEnabledSetting` contract (agent variants follow the same switch as the vendor they mirror). +2. **State-aware, not a blind toggle:** the command derives the available action from the current setting and shows a quick-pick — _"Remove from Language Models"_ when enabled, _"Re-add to Language Models"_ when disabled — so repeated invocations can never leave the provider in an unexpected state. Esc cancels without touching anything. +3. Reload prompt in both directions (unchanged behavior), action key in `providerDialogs.ts` collapsed to a single `"toggle"` entry (the old code reused the `"remove"` action for two opposite labels), and the command titles now read "Toggle Provider Registration in Language Models". + +## Files Changed + +| File | Change | +| --------------------------------- | ------------------------------------------------------------------------------------- | +| `src/commands/providers.ts` | Base-vendor resolution + state-aware quick-pick replacing the blind toggle | +| `src/provider/providerDialogs.ts` | Manage menu: one "Toggle Registration…" entry; dropped the now-unused enablement read | +| `package.json` | Retitled `opencodego.toggleProvider` / `opencodezen.toggleProvider` commands | + +## Verification + +- `npm run lint` (full 7-check gate) pass; 480/480 unit tests pass. +- Static regression check: grep confirms the only `enabled` write site is now `getConfiguration(baseVendor)` — no section-scoped writer remains. +- Real manual test (Extension Development Host): palette command twice in a row, gear-icon manage flow, `settings.json` key inspection after each step — PASS (2026-09-23). + +--- + +Detected 2026-09-19 | Fixed 2026-09-23