fix: respect explicit openai chat completion toggle

[mzbgf]

Summary

This fixes the case where OPENAI_USE_CHAT_COMPLETIONS=false / openai.useChatCompletions=false was effectively ignored for custom OPENAI_BASE_URL values.

The root cause was that the current flow collapsed false and unset together, and custom OpenAI-compatible base URLs were then forced onto chat completions.

What changed

• preserve true | false | unset for the OpenAI chat-completions toggle
• only auto-force chat completions when the toggle is actually unset
• keep existing auto behavior for OpenRouter and other explicit force-chat paths

Tests

• node node_modules/vitest/vitest.mjs run tests/llm.openai-provider.test.ts tests/run.config.test.ts tests/daemon.chat.test.ts tests/run.context.test.ts
• node node_modules/vitest/vitest.mjs run tests/daemon.agent.test.ts

Closes #235.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed June 4, 2026, 5:05 AM ET / 09:05 UTC.

Summary
The PR changes OpenAI chat-completions routing to preserve explicit true, explicit false, and unset states across config, daemon, summary engine, provider config, and tests.

Reproducibility: yes. Current main source shows a custom OpenAI base URL forces chat completions even when the toggle is false, and the patch regression is also source-reproducible through the unchanged daemon OpenRouter call site.

Review metrics: none identified.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• [P1] Fix the OpenRouter forced-chat regression and add focused coverage for that route.
• Post redacted terminal output or logs from a real custom OPENAI_BASE_URL setup showing OPENAI_USE_CHAT_COMPLETIONS=false using the Responses path; remove IPs, keys, phone numbers, private endpoints, and other private details.

Proof guidance:

• [P1] Needs real behavior proof before merge: The PR body lists test commands but does not include after-fix real behavior proof from a custom gateway setup; redacted terminal output or logs would satisfy this gate. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

• [P1] Merging as-is can regress existing fixed OpenRouter sidepanel chat model overrides because forceChatCompletions: false is no longer ignored when isOpenRouter is true.
• [P1] The PR body lists test commands, but it does not include redacted live output, logs, or terminal proof from a real custom OpenAI-compatible gateway using the Responses path.

Maintainer options:

1. Preserve OpenRouter before merge (recommended)
   Adjust the provider or daemon OpenRouter call site so OpenRouter still forces chat completions, and cover the sidepanel fixed OpenRouter path with a regression test.
2. Pause until proof is added
   Hold the PR until the contributor posts redacted terminal output or logs showing the fixed custom-base Responses behavior in a real setup.

Next step before merge

• [P1] Contributor action is needed: the branch has a blocking provider-routing regression and missing real behavior proof, which automation cannot supply for the contributor’s gateway setup.

Security
Cleared: The diff changes provider-routing logic and tests only; I found no concrete dependency, workflow, secret-handling, or supply-chain regression.

Review findings

• [P1] Preserve OpenRouter chat-completions forcing — src/llm/providers/openai.ts:98-101

Review details

Best possible solution:

Keep the three-state toggle approach, but make OpenRouter and other required chat-completions routes continue to override false where they are not the user’s OpenAI custom-base toggle, then add focused regression coverage and real gateway proof.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main source shows a custom OpenAI base URL forces chat completions even when the toggle is false, and the patch regression is also source-reproducible through the unchanged daemon OpenRouter call site.

Is this the best way to solve the issue?

No, not yet. The three-state toggle is the right direction, but the implementation must preserve OpenRouter’s forced chat-completions behavior before merge.

Full review comments:

• [P1] Preserve OpenRouter chat-completions forcing — src/llm/providers/openai.ts:98-101
  This ternary makes forceChatCompletions: false override isOpenRouter, but the fixed OpenRouter sidepanel path in src/daemon/chat.ts still passes false. That path currently relies on the provider’s isOpenRouter check to force chat completions, so after this patch it would route OpenRouter through the Responses API and break that chat model path. Make OpenRouter override explicit false here, or stop passing false from the OpenRouter caller, and add a regression test for that route.
  Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 821e76613ded.

Label changes

Label changes:

• add P2: The PR addresses a normal-priority provider routing bug, with a blocking but focused regression to fix before merge.
• add merge-risk: 🚨 compatibility: Existing OpenRouter fixed-model users could see a runtime routing break after upgrade if chat completions are no longer forced.
• add merge-risk: 🚨 auth-provider: The diff changes OpenAI/OpenRouter provider selection semantics, so model transport routing can change beyond the intended custom-base fix.
• add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists test commands but does not include after-fix real behavior proof from a custom gateway setup; redacted terminal output or logs would satisfy this gate. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

• P2: The PR addresses a normal-priority provider routing bug, with a blocking but focused regression to fix before merge.
• merge-risk: 🚨 compatibility: Existing OpenRouter fixed-model users could see a runtime routing break after upgrade if chat completions are no longer forced.
• merge-risk: 🚨 auth-provider: The diff changes OpenAI/OpenRouter provider selection semantics, so model transport routing can change beyond the intended custom-base fix.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists test commands but does not include after-fix real behavior proof from a custom gateway setup; redacted terminal output or logs would satisfy this gate. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

Acceptance criteria:

• [P1] node node_modules/vitest/vitest.mjs run tests/llm.openai-provider.test.ts tests/run.config.test.ts tests/daemon.chat.test.ts tests/run.context.test.ts.
• [P1] node node_modules/vitest/vitest.mjs run tests/daemon.agent.test.ts.

What I checked:

• Repository policy read: Read all 25 lines of AGENTS.md; its read-only checkout guidance applied to this review. (AGENTS.md:1)
• OpenRouter daemon call site remains false: Current main’s fixed OpenRouter sidepanel path passes forceChatCompletions: false; under the PR’s provider ternary, that value becomes an explicit instruction to disable chat completions even when isOpenRouter is true. (src/daemon/chat.ts:185, 821e76613ded)
• Current provider forces OpenRouter today: Current main uses Boolean(forceChatCompletions) || isOpenRouter || isCustomBaseURL, so the daemon’s false sentinel does not currently disable OpenRouter chat-completions mode. (src/llm/providers/openai.ts:98, 821e76613ded)
• Provider/config provenance: Blame for the current provider/config toggle behavior points to the v0.16.3 release commit in this checkout. (src/llm/providers/openai.ts:98, fcf8c8e5e98d)
• Recent adjacent routing work: Current main head recently touched daemon chat and summary-engine routing while adding the Antigravity CLI provider. (src/daemon/chat.ts:315, 821e76613ded)

Likely related people:

• steipete: Blame for resolveOpenAiClientConfig, openaiUseChatCompletions, and daemon chat routing points to the v0.16.3 release commit authored by Peter Steinberger. (role: introduced current provider/config behavior in checkout history; confidence: medium; commits: fcf8c8e5e98d; files: src/llm/providers/openai.ts, src/run/run-config.ts, src/daemon/chat.ts)
• Mykhailo: The current main head commit recently touched daemon chat and summary-engine provider routing while adding the Antigravity CLI provider. (role: recent adjacent contributor; confidence: medium; commits: 821e76613ded; files: src/daemon/chat.ts, src/run/summary-engine.ts)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.