Fix #239 reasoning-echo 400 self-heal, #233 glm tool-image 422, #228 provider toggle

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 `<variant>.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`.

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

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