review(drip-171): batch 2 — litellm + gemini-cli + qwen-code + goose (5 PRs)

- BerriAI/litellm#26764 merge-after-nits: multipart user_config/tags JSON decode
- BerriAI/litellm#26769 request-changes: FuturMix add bundles unrelated deletions
- google-gemini/gemini-cli#26184 merge-after-nits: deleteSession fail-loud on missing file
- QwenLM/qwen-code#3737 merge-after-nits: drop strip-thoughts helpers, preserve reasoning_content
- aaif-goose/goose#8781 needs-discussion: ACP per-connection inline→spawn ordering question

Author: Bojun-Vvibe

reviews/2026-W18/drip-171/BerriAI-litellm-pr-26764.md

@@ -0,0 +1,96 @@
+# BerriAI/litellm#26764 — `fix(proxy): decode multipart user_config JSON`
+
+- URL: https://github.com/BerriAI/litellm/pull/26764
+- Head SHA: `b63721c45f28`
+- Author: Kcstring
+- Size: +69 / -2 across 2 files
+- Fixes: #26707
+
+## Summary
+
+Multipart proxy requests previously only auto-decoded the `metadata`
+form field as JSON. `user_config` (per-request override of the
+router config) and `tags` arrays were left as raw strings, so a
+client sending the canonical multipart form for image-edit calls
+hit `TypeError: string indices must be integers` deep in routing.
+This PR generalizes the field-by-field JSON decode and adds two
+focused unit tests.
+
+## Specific references
+
+`litellm/proxy/common_utils/http_parsing_utils.py`:
+
+- New helper `_parse_form_json_field(parsed_body, field_name, *, require_json=False)`
+  at lines 13–22:
+
+  ```py
+  if not isinstance(value, str):
+      return
+  if require_json or value.lstrip().startswith(("{", "[")):
+      parsed_body[field_name] = json.loads(value)
+  ```
+
+- Replacement at lines 49–54 (form branch of `_read_request_body`):
+
+  ```py
+  parsed_body = dict(await request.form())
+  _parse_form_json_field(parsed_body, "metadata", require_json=True)
+  _parse_form_json_field(parsed_body, "user_config")
+  _parse_form_json_field(parsed_body, "tags")
+  ```
+
+`tests/test_litellm/proxy/common_utils/test_multipart_json_form_fields.py` (new):
+
+- `test_form_data_with_json_user_config_and_tags` — verifies a
+  full `user_config` model_list payload plus a JSON-array `tags`
+  string both round-trip into native objects.
+- `test_form_data_with_plain_string_tags_is_left_unchanged` —
+  pins the back-compat carve-out for clients that send `tags`
+  as a single bare string ("production").
+
+## Design analysis
+
+The `require_json=True` carve-out for `metadata` preserves the
+old "always parse, raise on bad JSON" behavior, while the new
+heuristic for `user_config`/`tags` only parses when the value
+*looks* like JSON (`{` or `[` prefix after stripping). That's the
+right shape: a client that sends `tags=production` keeps working,
+a client that sends `tags=["a","b"]` gets the array.
+
+Two minor concerns about the heuristic:
+
+1. A legitimate `user_config` value that is a JSON *string*
+   (e.g. literally `"foo"` with quotes) would not start with `{`
+   or `[`, so it stays a string. Probably fine because
+   `user_config` is documented to be an object, but worth a
+   one-line comment in the helper.
+2. `require_json=False` swallows the parse only by virtue of the
+   prefix check. If the prefix matches but the body is malformed
+   JSON (e.g. `tags=[broken`), `json.loads` will still raise.
+   That's actually the correct fail-loud behavior, but the test
+   suite doesn't pin it. A `test_invalid_json_tags_raises` would
+   document that contract.
+
+## Risks
+
+- Low. The change is additive and the existing `metadata`
+  behavior is preserved verbatim via `require_json=True`.
+- Watch for any callers downstream of `_read_request_body` that
+  previously called `json.loads(parsed_body["user_config"])`
+  themselves; they now receive a dict and would crash. The diff
+  doesn't show any such call sites, but a quick
+  `rg 'parsed_body\["user_config"\]'` and `rg 'body\["user_config"\]'`
+  in proxy code is the right confidence check.
+
+## Verdict
+
+`merge-after-nits`
+
+## Suggested nits
+
+- Add a comment on `_parse_form_json_field` explaining the prefix
+  heuristic and the back-compat goal.
+- Add a third test asserting malformed JSON raises (pins the
+  fail-loud behavior).
+- Run `rg 'parsed_body\["user_config"\]'` to confirm no
+  downstream caller still does its own decode.

reviews/2026-W18/drip-171/BerriAI-litellm-pr-26769.md

@@ -0,0 +1,90 @@
+# BerriAI/litellm#26769 — `feat: add FuturMix as named OpenAI-compatible provider`
+
+- URL: https://github.com/BerriAI/litellm/pull/26769
+- Head SHA: `6bd3935d3a60`
+- Author: Gzhao-redpoint
+- Size: +25 / -19 across 2 files
+
+## Summary
+
+Registers `futurmix` as a named OpenAI-compatible provider in
+`litellm/llms/openai_like/providers.json`, with a corresponding
+endpoint-support entry in `provider_endpoints_support.json`. The
+provider declares chat/completions, messages, responses,
+embeddings, and image generations as supported, with the standard
+audio/moderations/batches/rerank disabled.
+
+## Specific references
+
+`litellm/llms/openai_like/providers.json` lines ~80–90 (after the
+`gmi` entry):
+
+```json
+"futurmix": {
+  "base_url": "https://futurmix.ai/v1",
+  "api_key_env": "FUTURMIX_API_KEY",
+  "api_base_env": "FUTURMIX_API_BASE",
+  "param_mappings": {
+    "max_completion_tokens": "max_tokens"
+  }
+}
+```
+
+`provider_endpoints_support.json`:
+
+- New `futurmix` entry around line 1063 with the standard
+  endpoint matrix.
+- **Unrelated deletions** in the same file:
+  - Removed `"a2a": false` and `"interactions": false` keys from
+    an existing provider entry near lines 471–477 (visible at
+    diff lines 31–34).
+  - Removed the entire `charity_engine` provider entry at lines
+    2553–2570 (visible at diff lines 70–84).
+
+## Concerns
+
+The two deletions noted above are unrelated to "add FuturMix" and
+should not be in this PR. They look like merge-conflict resolution
+or a stale local checkout. Without a justification in the PR body
+they are silent regressions:
+
+- Removing `charity_engine` entirely deletes a previously
+  supported provider's discoverability metadata. If charity_engine
+  was deprecated, that should be its own PR with a changelog
+  entry.
+- Removing the `a2a` and `interactions` flags from another
+  provider changes the schema-shape for that provider only,
+  causing schema heterogeneity across the file.
+
+The FuturMix-only changes are otherwise straightforward and match
+the structure of neighboring entries (e.g. `gmi`, `sarvam`).
+
+## Risks
+
+- The `param_mappings: { "max_completion_tokens": "max_tokens" }`
+  is correct for an OpenAI-compatible upstream that uses the
+  legacy `max_tokens` field. Sanity-check: confirm FuturMix's
+  public docs actually require `max_tokens` not
+  `max_completion_tokens`. If they accept both, the mapping is
+  fine but redundant; if they only accept `max_completion_tokens`,
+  this mapping would break the call.
+- No model list / pricing entry is added in `model_prices_and_context_window.json`,
+  so cost tracking will fall back to defaults. That's fine for a
+  provider-only PR but worth flagging in the description.
+- No tests, but provider-registration PRs in this repo
+  conventionally don't add unit tests.
+
+## Verdict
+
+`request-changes`
+
+Reason: the unrelated deletions in `provider_endpoints_support.json`
+(charity_engine entry + `a2a`/`interactions` keys) need to be
+reverted from this PR. Submit them separately if intentional.
+
+## Suggested nits
+
+- Confirm in the PR body that FuturMix's API expects `max_tokens`
+  (not `max_completion_tokens`).
+- Optional: add a docs page under `docs/my-website/docs/providers/futurmix.md`
+  matching the URL declared in `provider_endpoints_support.json`.

reviews/2026-W18/drip-171/QwenLM-qwen-code-pr-3737.md

@@ -0,0 +1,132 @@
+# QwenLM/qwen-code#3737 — `fix(core): preserve reasoning_content in rewind, compression, and merge paths (#3579)`
+
+- URL: https://github.com/QwenLM/qwen-code/pull/3737
+- Head SHA: `480f9e71b4c4`
+- Author: fyc09
+- Size: +48 / -363 across 8 files
+- Closes the #3579 series (after #3590, #3682)
+
+## Summary
+
+Three remaining paths still silently dropped DeepSeek-class
+`reasoning_content` from history:
+
+1. **Rewind / double-ESC** — `AppContainer.tsx` called
+   `geminiClient.stripThoughtsFromHistory()` after truncating
+   to the rewind point.
+2. **Chat compression** — `chatCompressionService.ts` built the
+   acknowledgment model turn with no thought part, so converters
+   that key off the presence of *any* thought in a model turn
+   broke the reasoning chain across the compression boundary.
+3. **Idle/merge** — the `stripThoughtsFromHistory` and
+   `stripThoughtsFromHistoryKeepRecent` helpers themselves were
+   load-bearing for a use case the project no longer wants to
+   support; this PR retires them outright.
+
+## Specific references
+
+`packages/cli/src/ui/AppContainer.tsx:1739-1745` (post-patch):
+
+```tsx
+- // 3. Truncate API history and strip stale thinking blocks
++ // 3. Truncate API history to the target point.
++ // Do NOT strip thought parts — reasoning models (e.g. DeepSeek)
++ // require reasoning_content continuity across all turns ...
+  geminiClient.truncateHistory(apiTruncateIndex);
+- geminiClient.stripThoughtsFromHistory();
+```
+
+`packages/core/src/core/client.ts:204-210` removes the
+`stripThoughtsFromHistory()` pass-through on `GeminiClient`.
+
+`packages/core/src/core/geminiChat.ts:792-919` (the deletion block)
+removes both `stripThoughtsFromHistory()` and
+`stripThoughtsFromHistoryKeepRecent(keepTurns)` from `GeminiChat`,
+and `geminiChat.test.ts:2039-2227` removes their dedicated tests.
+This is a real public-API surface reduction.
+
+`packages/core/src/services/chatCompressionService.ts:265-292` —
+the new logic detects whether any preserved model turn carries
+`thought: true` and, if so, appends a sentinel thought part to
+the compression acknowledgment model turn:
+
+```ts
+const hasThoughtContentInKeep = historyToKeep.some(
+  (content) =>
+    content.role === 'model' &&
+    content.parts?.some((part) => 'thought' in part && part.thought),
+);
+
+const ackParts: Part[] = [
+  { text: 'Got it. Thanks for the additional context!' },
+];
+if (hasThoughtContentInKeep) {
+  ackParts.push({ text: ' ', thought: true });
+}
+```
+
+## Design analysis
+
+The PR implements a clean policy reversal: instead of stripping
+thoughts on three control paths (rewind, compression-ack, idle),
+preserve them and remove the strippers entirely. That matches
+the framing in #3579 — DeepSeek-class providers require an
+unbroken `reasoning_content` chain across the *entire* assistant
+sequence, so any helper that selectively drops thoughts is a
+loaded gun.
+
+Two design points worth noting:
+
+1. **Sentinel thought part with `text: ' '`.** The acknowledgment
+   only carries a thought part *when* prior turns had one. The
+   chosen sentinel is a single-space text with `thought: true`.
+   That works for downstream converters that key off "is there a
+   thought part on this turn" — but it's brittle if any consumer
+   ever asserts non-empty thought content. A short comment
+   right above the `ackParts.push` line explaining "sentinel,
+   intentionally minimal" would help future maintainers.
+2. **API surface removal vs deprecation.** Deleting public
+   methods on `GeminiClient` and `GeminiChat` is fine for an
+   internal/CLI-only surface, but if anything in
+   `packages/cli`-adjacent extension code calls them, this is
+   a hard break. A `rg "stripThoughtsFromHistory"` across the
+   whole repo (already implied by the test cleanup) plus a note
+   in the PR body would close the loop.
+
+The test cleanup is mechanical: every `stripThoughtsFromHistory: vi.fn()`
+mock entry is removed across `client.test.ts`, `config.test.ts`,
+and `geminiChat.test.ts`. No assertion logic for the new behavior
+was added in `geminiChat.test.ts` itself; the new behavior is
+exercised through `chatCompressionService` (presumed to have its
+own tests, not visible in the diff).
+
+## Risks
+
+- Compression test coverage: the diff doesn't show a new test in
+  `chatCompressionService.test.ts` pinning the
+  `hasThoughtContentInKeep ? thought-part-present : not` branch.
+  This is the only new code path; it should have a direct test.
+- The `text: ' '` sentinel is invisible in user-facing logs but
+  may surface in trace dumps. Worth confirming no downstream
+  formatter trims whitespace-only text and drops the thought part
+  with it.
+- Prior fixes #3590 and #3682 closed the resume/idle and
+  model-switch paths. After this PR, any *new* control path that
+  wants to mutate history needs to learn this lesson without the
+  guardrail of helpers — the helpers are gone. A short
+  `CHANGELOG`/comment block in `geminiChat.ts` explaining
+  *why* these helpers no longer exist would prevent re-introduction.
+
+## Verdict
+
+`merge-after-nits`
+
+## Suggested nits
+
+- Add at least one test in `chatCompressionService.test.ts`
+  pinning both branches of the `hasThoughtContentInKeep` check.
+- Add a short tombstone comment in `geminiChat.ts` (where the
+  methods used to live) saying "intentionally removed in #3737
+  — reasoning_content must remain contiguous; do not reintroduce".
+- Confirm in PR body that `rg stripThoughtsFromHistory` returns
+  zero hits across the whole repo after this PR.