Decouple and fix mistral

[lunamonke]

Summary

Mistral was previously giving error due to sending max_completion_tokens. This pr decouples mistral as a seperate provider similar to gemini pattern and doesn't send max_completion_tokens for mistral.

Before:

After:

Now mistral can be configured via:

# mistral
export CLAUDE_CODE_USE_MISTRAL=1
export MISTRAL_API_KEY=""
# export MISTRAL_MODEL="devstral-latest"
# export MISTRAL_BASE_URL="https://api.mistral.ai/v1"

Testing

• bun run build
• bun run smoke

[kevincodex1]

please fix failing tests

[gnanam1990]

Thanks for the PR. Separating Mistral from the generic OpenAI-compatible path looks like a sensible direction, and I like the goal of avoiding max_completion_tokens for Mistral requests.

I had a few things I wanted to double-check before approving:

1. In openaiShim.ts, it looks like the Mistral path removes max_completion_tokens, but I wasn't sure whether the intended replacement is max_tokens. My concern is that we may be dropping the explicit output cap entirely for Mistral requests.

2. In auth.ts, Mistral seems to be added to one 3P-auth check, but I may be missing a matching update for isUsing3PServices(). If that is still unchanged, I wonder whether some CI/test auth paths could still treat Mistral like first-party.

3. In spawnUtils.ts, I noticed MISTRAL_API_KEY and MISTRAL_MODEL are forwarded to teammates, but I didn't see MISTRAL_BASE_URL. That made me wonder whether subagents would fall back to the default endpoint when a custom Mistral base URL is configured.

4. In python/smart_router.py, could you also take another quick look at the new Mistral provider block? I may be misreading it, but the added entry looked like it might need a small syntax fix as well.

If I'm missing something on any of these, happy to be corrected. I'd be glad to re-check after that.

[kevincodex1]

please fix conflicts

[Vasanthdev2004]

Rechecked the latest head 9490af7efbb960dca8736f4e0480a89338d4cb4d against current origin/main 72e6a945fe1311a0b9eea1033703a74dc6cdbc18.

I still can't approve this because the new provider-flag clearing logic introduces a real branch-specific regression.

Current blocker:

1. src/utils/providerFlag.ts:82-88 clears provider flags with assignments like process.env.CLAUDE_CODE_USE_GEMINI = undefined instead of deleting them. Under Node/Bun that leaves the literal string "undefined" in the environment, so the flags are no longer truly absent.

   Direct repro on this head:

   • applyProviderFlag('anthropic', [])
   • resulting env contains values like:
     • CLAUDE_CODE_USE_OPENAI === "undefined"
     • CLAUDE_CODE_USE_GEMINI === "undefined"
     • CLAUDE_CODE_USE_MISTRAL === "undefined"
   • downstream effects are real, not just cosmetic:
     • hasExplicitProviderSelection() returns true
     • buildInheritedEnvVars() forwards CLAUDE_CODE_USE_GEMINI=undefined, CLAUDE_CODE_USE_MISTRAL=undefined, etc. to teammates

   This also regresses the existing unit contract on this branch:

   • bun test src/utils/providerFlag.test.ts -> fails on this head
   • same test on current origin/main -> passes

   So even though truthy provider detection still happens to work for the simple case, the env is observably wrong and leaks through code paths that check !== undefined rather than truthiness.

What I verified on this head:

• direct provider-flag repro above
• direct downstream repro through hasExplicitProviderSelection() and buildInheritedEnvVars()
• bun test src/utils/providerFlag.test.ts -> fails on this head, passes on current origin/main
• Mistral request-body concern is fixed: direct shim repro now sends max_tokens and omits max_completion_tokens
• bun run build -> success
• bun run smoke -> success

I didn't find a second high-confidence blocker beyond the provider-flag regression, but I wouldn't merge this until those env vars are actually deleted rather than assigned undefined.

[Vasanthdev2004]

Rechecked the latest head 653637acdd6e001a5066d770d3f5504c686294dd against current origin/main aff2bd89e22161f57ef330737b7f149208b96097.

The previous provider-flag blocker is fixed on this head, but I still can't approve because custom Mistral endpoints are not preserved consistently across the new user-facing flows.

Current blocker:

1. Custom MISTRAL_BASE_URL values are still lost or misreported in the new Mistral UX paths.

   There are two concrete places this shows up:

   • src/commands/provider/provider.tsx:150-171 / 1205-1223 / 1245-1249

     • getProviderWizardDefaults() captures mistralModel but not mistralBaseUrl
     • the mistral-base step uses an initialValue expression that is always ''
     • on submit, the wizard passes processEnv: {} into buildMistralProfileEnv()

     Direct repros on this head:

     • getProviderWizardDefaults({ MISTRAL_BASE_URL: 'https://custom.mistral.test/v1', MISTRAL_MODEL: 'devstral-latest' })
       returns no Mistral base-url default at all
     • buildMistralProfileEnv({ model: 'devstral-latest', baseUrl: null, apiKey: 'mistral-key', processEnv: {} })
       returns:
       {"MISTRAL_API_KEY":"mistral-key","MISTRAL_MODEL":"devstral-latest"}
       with no MISTRAL_BASE_URL

     So if a user with a working custom Mistral endpoint reopens the provider wizard and just confirms the defaults, the saved profile silently drops the custom endpoint and falls back to the default Mistral API.

   • src/components/StartupScreen.ts:96-100

     • the startup screen hardcodes https://api.mistral.ai/v1 instead of using process.env.MISTRAL_BASE_URL

     Direct repro on this head:

     • with CLAUDE_CODE_USE_MISTRAL=1 and MISTRAL_BASE_URL='https://custom.mistral.test/v1'
     • captured startup-screen output shows the default https://api.mistral.ai/v1
     • it does not show the configured custom endpoint

   This is exactly the kind of regression that can trip up an already working custom Mistral project: the runtime request path may be correct, but the setup and status UI around it still nudges the user back toward the default endpoint.

What I verified on this head:

• previous provider-flag blocker is fixed:
  • applyProviderFlag('anthropic', []) now leaves provider env vars absent
  • hasExplicitProviderSelection() -> false
  • buildInheritedEnvVars() stays clean
• direct Mistral request-path repro is correct:
  • custom MISTRAL_BASE_URL is used
  • Authorization: Bearer mistral-key
  • max_tokens present
  • max_completion_tokens absent
• bun test src/utils/providerFlag.test.ts -> 20 pass
• bun test src/commands/model/model.test.tsx -> 1 pass
• bun test src/commands/provider/provider.test.tsx -> 10 pass
• bun test src/utils/providerProfile.test.ts src/utils/providerProfiles.test.ts src/utils/model/providers.test.ts src/services/api/providerConfig.local.test.ts -> 68 pass
• py -3 -m py_compile python/smart_router.py -> success
• bun run build -> success
• bun run smoke -> success

[Vasanthdev2004]

Rechecked the latest head 1c84b27127b66052ae1eb7579ad7391fcf0e146c against current origin/main.

The earlier Mistral UX/base-url blocker is fixed on this head, and I reverified the runtime request path:

• custom MISTRAL_BASE_URL is used
• Authorization: Bearer mistral-key
• max_tokens present
• max_completion_tokens absent

I still can't approve because this branch now introduces a real regression for already-working Ollama/custom-endpoint setups.

Current blocker:

1. src/utils/providerFlag.ts:124-130 no longer preserves explicit Ollama/OpenAI-compatible env overrides.

   The function header still says explicit env vars should win, but the new ollama branch now does this:

   • forces OPENAI_BASE_URL back to http://localhost:11434/v1 unless it matches one hardcoded string
   • forces OPENAI_API_KEY = 'ollama'

   Direct repro on this head:

   • set OPENAI_BASE_URL='http://remote-ollama.internal:11434/v1'
   • set OPENAI_API_KEY='secret-token'
   • call applyProviderFlag('ollama', [])
   • resulting env becomes:
     • OPENAI_BASE_URL === 'http://localhost:11434/v1'
     • OPENAI_API_KEY === 'ollama'

   So a working remote/self-hosted Ollama-compatible setup gets silently rewritten to localhost with a fake key just by using --provider ollama.

   This is exactly the kind of regression we should block: it breaks an already-working configuration outside the new Mistral path.

What I verified on this head:

• bun test src/utils/providerFlag.test.ts -> 20 pass
• bun test src/commands/model/model.test.tsx -> 1 pass
• bun test src/commands/provider/provider.test.tsx -> 10 pass
• bun test src/utils/providerProfile.test.ts src/utils/providerProfiles.test.ts src/utils/model/providers.test.ts src/services/api/providerConfig.local.test.ts -> 68 pass
• bun run build -> success
• bun run smoke -> success
• py -3 -m py_compile python/smart_router.py -> success
• direct Mistral wizard/env repro -> fixed
• direct Ollama override repro above -> still broken

Non-blocking note:

• the existing Ollama test only preserves one exact URL (http://my-ollama:11434/v1), which is why this broader regression slipped through.

[gnanam1990]

Thanks for continuing to iterate on this. I rechecked the latest head and I still see two blocking regressions:

1. src/utils/auth.ts
isUsing3PServices() now checks MISTRAL_API_KEY instead of the actual active-provider flag. That means simply having a Mistral key in the environment can make the app behave as if it is in 3P mode even when Mistral is not the active provider.

2. src/utils/model/model.ts
   In GitHub mode, the env-configured model path is no longer handled consistently. That can cause a saved settings.model to override the intended OPENAI_MODEL value for GitHub setups, which is a regression for env-based configurations.

I’d want those provider-selection regressions fixed before approving.

[Vasanthdev2004]

Thanks for continuing to work on this. I rechecked the current head and I do not think it is reviewable as a merge-ready Mistral PR in its current shape.

Scope

This is a targeted re-review of the current head, latest commits, and current green check, not a full fresh re-review of every line from scratch.

Verdict

Needs changes

Current blocker

1. The PR is no longer scoped to one coherent change.

   The current head now pulls in the large web-search provider adapter surface as well (the same area covered by #512), alongside the Mistral/provider-decoupling work. That makes this branch much harder to reason about as a focused Mistral fix, and it creates review ambiguity about what is actually intended to merge from this PR versus what was inherited through repeated merges from main.

   Concretely, the current diff for #411 now includes unrelated files such as:

   • src/tools/WebSearchTool/**
   • src/tools/WebSearchTool/README_SEARCH_PROVIDERS.md

   even though the PR title/theme is still Decouple and fix mistral.

   At this point I would want this narrowed back to the intended Mistral/provider-decoupling surface before approving. Otherwise we are effectively blessing a mixed branch whose review story no longer matches its actual changed surface.

Non-blocking notes

• The earlier provider-flag / base-url / env-selection regressions may already be in better shape now, but I do not want to treat those as the only question while the branch itself is carrying unrelated large-surface changes.
• smoke-and-tests being green is good, but green CI does not solve the scope problem here.

If you narrow this back to the actual Mistral/provider work, I’m happy to re-review that focused branch.

[lunamonke]

@Vasanthdev2004 I just merged to main to resolve conflicts so it is normal that it contains functionality from a merged branch?? Can you please re review?

[lunamonke]

@gnanam1990

[lunamonke]

Ok, I will revert those commits and do a rebase instead later

[Vasanthdev2004]

@lunamonke Yeah, merging main to resolve conflicts is normal by itself, and I’m not treating the merge commits alone as the problem.

The blocker I was calling out is the current merge diff against main, not just the raw commit history. On the current head, the PR is still carrying a much broader provider/setup surface than a narrow "Decouple and fix mistral" change, so I’m not comfortable approving it yet as a focused Mistral PR.

If you revert the unrelated carried-over changes and rebase/narrow it back to the intended Mistral/provider-decoupling surface, I’m happy to re-review that version.

[lunamonke]

@lunamonke Yeah, merging main to resolve conflicts is normal by itself, and I’m not treating the merge commits alone as the problem.

The blocker I was calling out is the current merge diff against main, not just the raw commit history. On the current head, the PR is still carrying a much broader provider/setup surface than a narrow "Decouple and fix mistral" change, so I’m not comfortable approving it yet as a focused Mistral PR.

If you revert the unrelated carried-over changes and rebase/narrow it back to the intended Mistral/provider-decoupling surface, I’m happy to re-review that version.

Well, I have not introduced any other changes so not sure what you are talking about. Can you point out the commit where these changes are introduced other than the merge commits?

[lunamonke]

@Vasanthdev2004

[Vasanthdev2004]

@lunamonke Fair question. To be precise, I’m not saying you manually authored unrelated feature commits on top of this PR. I’m saying the current merge diff vs main is broader than the PR’s stated Mistral scope, and that broader surface is what I’m not comfortable approving under this PR title/theme.

So no, I’m not pointing to a non-merge commit from you as “the bad commit.” The issue is that after the repeated merges from main, the review surface of the branch is no longer a tight Mistral-only diff.

My concern is about the branch’s current reviewability / merge surface, not blame for a specific authored commit. If you rebase or otherwise narrow the branch back to the intended Mistral/provider-decoupling diff, I’m happy to re-review that focused version.

[lunamonke]

@lunamonke Yeah, merging main to resolve conflicts is normal by itself, and I’m not treating the merge commits alone as the problem.

So you are treating the merge commit alone as the problem. That's fine but if you are doing so don't say you are not. This paragraph is saying something completely different than what you are saying right now, that the non merge changes also have a problem. Please have more clear non conflicting reviews. I will rebase instead later.

[Vasanthdev2004]

@lunamonke Fair pushback. You’re right that my wording there was not clear enough.

What I should have said more plainly is: on my side, the practical review problem was the branch state / compare state after the merges, not that you had manually added unrelated feature work outside the Mistral scope.

So yes, if you’re planning to rebase and narrow the branch state, that addresses the reviewability concern I was trying to describe. Thanks for sticking with the back-and-forth here.

[Vasanthdev2004]

im sorry for the confusion @lunamonke

[lunamonke]

at this point, it is infeasible for me to rebase due to many merges already done. please use https://github.com/Gitlawb/openclaude/pull/411/changes to give review, or close this diff.

[lunamonke]

Or, if you prefer we can close this branch and I can open a new branch, make my changes and rebase. If that works for you better, it is fine for me as well. Please let me know as I use mistral a lot

[Vasanthdev2004]

Thanks for the follow-up, and thanks for explicitly pointing back to the GitHub changed-files view. I rechecked the current head 7162dfaf840d47c9b47b5cb1fe9e982a81b1a721 from the actual PR surface first.

Scope

This is a full review of the current GitHub PR surface and current green check, not the earlier local-branch confusion.

Verdict

Needs changes

Blocking issue

1. GitHub model precedence is still inconsistent on the current head.

   The earlier settings.model vs OPENAI_MODEL concern does still appear to be real.

   On this head:

   • src/utils/model/model.ts:getUserSpecifiedModelSetting() now prefers provider-matching env vars before saved settings, including GitHub mode:

     • for GitHub it can resolve from process.env.OPENAI_MODEL before settings.model

   • but src/utils/model/model.ts:getDefaultMainLoopModelSetting() still does the opposite for GitHub:

   if (getAPIProvider() === 'github') {
     const settings = getSettings_DEPRECATED() || {}
     return settings.model || process.env.OPENAI_MODEL || 'github:copilot'
   }

   So the file now has two different precedence rules for GitHub model resolution depending on which path is used.

   That is exactly the kind of provider-selection inconsistency that can lead to “saved settings unexpectedly override my env-configured GitHub model” in one code path but not another. For a provider-decoupling PR, I would want this normalized before merge.

What I rechecked on the current head

• the actual GitHub PR surface is now the Mistral/provider diff, not the unrelated web-search surface I previously called out by mistake
• earlier blockers around provider-flag deletion, MISTRAL_BASE_URL preservation, startup display, and swarm forwarding look fixed on this head
• src/utils/auth.ts:isUsing3PServices() is now keyed off CLAUDE_CODE_USE_MISTRAL, not just presence of MISTRAL_API_KEY
• smoke-and-tests is green on the current head

Non-blocking note

• If you want the cleanest route from here, fixing the GitHub precedence inconsistency on this branch is enough for me to recheck again without reopening the earlier branch-state argument.

[lunamonke]

@Vasanthdev2004 This is unrelated to my changes, and an issue in the main branch as well. at this rate I could be fixing issues introduced by others forever on this branch. Please review again.

[lunamonke]

I am closing this branch and I will create a new branch and rebase