Skip to content

fix(mistral): expand cookie import to Safari, Chrome, and Firefox#1985

Open
djsavvy wants to merge 2 commits into
steipete:mainfrom
djsavvy:fix/mistral-firefox-cookie-import
Open

fix(mistral): expand cookie import to Safari, Chrome, and Firefox#1985
djsavvy wants to merge 2 commits into
steipete:mainfrom
djsavvy:fix/mistral-firefox-cookie-import

Conversation

@djsavvy

@djsavvy djsavvy commented Jul 8, 2026

Copy link
Copy Markdown

Mistral's importSession hardcoded preferredBrowsers to [.chrome], bypassing the descriptor's browserCookieOrder. Users signed in via Firefox or Firefox Developer Edition saw "No Mistral session cookies found in browsers."

Add a mistralCookieImportOrder preset ([.safari, .chrome, .firefox]) matching the MiMo #1304 precedent, wire it through the descriptor, and change importSession/hasSession to use the descriptor order when no explicit preferredBrowsers are passed (matching the Manus/Perplexity pattern).

Heads up that Ollama seems to have the same bug, but I didn't address that here.

The PR was generated with a Hermes agent, but I gave it a review as well.

Mistral's importSession hardcoded preferredBrowsers to [.chrome], bypassing
the descriptor's browserCookieOrder. Users signed in via Firefox or Firefox
Developer Edition saw "No Mistral session cookies found in browsers."

Add a mistralCookieImportOrder preset ([.safari, .chrome, .firefox]) matching
the MiMo steipete#1304 precedent, wire it through the descriptor, and change
importSession/hasSession to use the descriptor order when no explicit
preferredBrowsers are passed (matching the Manus/Perplexity pattern).

Also update docs/providers.md and add a focused test.
@clawsweeper

clawsweeper Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 8, 2026, 1:36 AM ET / 05:36 UTC.

Summary
The branch changes Mistral cookie auto-import from Chrome-only to a descriptor-backed Chrome, Firefox, Safari order, and updates provider docs plus a focused browser-order test.

Reproducibility: yes. from source inspection: current main defaults Mistral importSession to [.chrome], and resolveCookieHeader calls it without an explicit browser order. I did not run live browser-cookie import because AGENTS.md warns those checks can display macOS Keychain prompts.

Review metrics: 2 noteworthy metrics.

  • Changed files: 5 files affected. The PR changes one provider auth path plus its provider metadata, docs, and focused browser-order coverage.
  • Automatic browser order: Chrome, Firefox, Safari. Mistral Auto would move from the current no-argument Chrome-only import to three possible browser cookie sources.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof is added.

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

Rank-up moves:

  • [P2] Fix the empty preferredBrowsers fallback so it never probes Browser.defaultImportOrder for Mistral Auto.
  • [P1] Add redacted real behavior proof showing Mistral cookie import succeeds from the newly supported browser source.

Proof guidance:

  • [P1] Needs real behavior proof before merge: No after-fix live output, logs, terminal screenshot, or recording proves Mistral import from Firefox/Safari; the contributor should post redacted proof, and updating the PR body should trigger a fresh ClawSweeper review or a maintainer can ask @clawsweeper re-review. 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] Contributor-owned real behavior proof is missing for the Firefox/Safari Mistral cookie-import path, so reviewers cannot confirm the changed auth source works in a real setup without unexpected prompts or stale-cookie behavior.
  • [P1] The explicit empty preferredBrowsers path still scans Browser.defaultImportOrder, contrary to the PR's bounded Chrome/Firefox/Safari intent and docs that other Chromium forks remain Manual.
  • [P1] Expanding automatic cookie import changes auth-provider behavior by allowing additional browsers to supply Mistral credentials after Chrome misses.

Maintainer options:

  1. Fix fallback and require proof (recommended)
    Keep the proposed Chrome to Firefox to Safari order, but change empty preferences to the bounded Mistral descriptor order and require redacted live output or logs proving Firefox/Safari import before merge.
  2. Accept the expanded auth order
    Maintainers may intentionally accept the broader current behavior, but that owns the risk that empty preferences can probe the full default browser list.
  3. Pause for a narrower design
    If Mistral Auto should stay Chrome-only or needs multi-session retry first, pause or close this branch and request a smaller follow-up.

Next step before merge

  • [P1] Human follow-up is needed because contributor-owned real behavior proof is missing and maintainers should accept the Mistral Auto browser-source tradeoff before merge.

Maintainer decision needed

  • Question: Should Mistral Auto expand from Chrome-only to a bounded Chrome, Firefox, Safari cookie-import order after the fallback fix and proof are added?
  • Rationale: AGENTS.md asks cookie imports to stay Chrome-only when possible, and this PR intentionally broadens an auth-cookie source list, so maintainer intent should confirm the provider-specific prompt/privacy tradeoff.
  • Likely owner: steipete — steipete is the recent Mistral-area merger/contributor and best visible owner for accepting the provider-specific cookie-import policy tradeoff.
  • Options:
    • Approve bounded expansion (recommended): Allow Chrome, Firefox, and Safari for Mistral Auto once the empty-order fallback is bounded and contributor proof shows the real import path working.
    • Keep Chrome-only Auto: Reject the browser expansion and leave Firefox/Safari users on Manual cookie-header import until a broader browser-picker design exists.
    • Require multi-session retry first: Defer this PR until Mistral can collect multiple sessions and retry after invalid credentials instead of selecting only the first cookie source.

Security
Cleared: The diff changes a bounded provider cookie-import order and docs/tests; I found no new dependency, CI, script, secret-storage, or supply-chain concern beyond the intended auth-source behavior.

Review findings

  • [P2] Keep empty Mistral preferences off the full browser order — Sources/CodexBarCore/Providers/Mistral/MistralCookieImporter.swift:45
Review details

Best possible solution:

Land a bounded Mistral auto-import path that preserves Chrome first, never falls through to the full default browser list for empty preferences, and includes redacted live proof from the changed cookie source.

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

Yes from source inspection: current main defaults Mistral importSession to [.chrome], and resolveCookieHeader calls it without an explicit browser order. I did not run live browser-cookie import because AGENTS.md warns those checks can display macOS Keychain prompts.

Is this the best way to solve the issue?

No, not yet: Chrome-first bounded Mistral Auto is a plausible direction, but the patch still falls back to the full default browser order for empty preferences and lacks real behavior proof.

Full review comments:

  • [P2] Keep empty Mistral preferences off the full browser order — Sources/CodexBarCore/Providers/Mistral/MistralCookieImporter.swift:45
    This prior blocker is still present. preferredBrowsers: [] now reaches Browser.defaultImportOrder, which can include the other Chromium sources this PR says remain Manual; empty explicit preferences should stay on the bounded Mistral descriptor order or otherwise avoid broad automatic auth-cookie probing.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against aa401f1d8b74.

Label changes

Label changes:

  • add merge-risk: 🚨 compatibility: The PR changes the public Mistral importer semantics for nil and empty browser preferences, and the empty path can broaden automatic browser probing.
  • add merge-risk: 🚨 auth-provider: The diff changes which browser cookies may be used as Mistral credentials in Auto mode.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix live output, logs, terminal screenshot, or recording proves Mistral import from Firefox/Safari; the contributor should post redacted proof, and updating the PR body should trigger a fresh ClawSweeper review or a maintainer can ask @clawsweeper re-review. 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: This is a normal provider-auth improvement with limited blast radius, but it still needs a bounded fallback fix and proof before merge.
  • merge-risk: 🚨 compatibility: The PR changes the public Mistral importer semantics for nil and empty browser preferences, and the empty path can broaden automatic browser probing.
  • merge-risk: 🚨 auth-provider: The diff changes which browser cookies may be used as Mistral credentials in Auto mode.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix live output, logs, terminal screenshot, or recording proves Mistral import from Firefox/Safari; the contributor should post redacted proof, and updating the PR body should trigger a fresh ClawSweeper review or a maintainer can ask @clawsweeper re-review. 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

What I checked:

Likely related people:

  • welcoMattic: Authored the merged Mistral provider PR that introduced the Mistral descriptor and cookie importer code path. (role: introduced behavior; confidence: high; commits: 47a4776e914d; files: Sources/CodexBarCore/Providers/Mistral/MistralCookieImporter.swift, Sources/CodexBarCore/Providers/Mistral/MistralProviderDescriptor.swift)
  • lfmundim: Authored the merged Mistral cookie/auth repair that changed imported domains and ory_session_* forwarding for the same provider. (role: recent auth-path contributor; confidence: high; commits: c6399fedc4a8; files: Sources/CodexBarCore/Providers/Mistral/MistralCookieImporter.swift, Sources/CodexBarCore/Providers/Mistral/MistralProviderDescriptor.swift, Sources/CodexBarCore/Providers/Mistral/MistralUsageFetcher.swift)
  • steipete: Recent Mistral balance work and release-history blame touch the same provider files, and GitHub metadata shows steipete merged recent Mistral auth work. (role: recent area contributor and merger; confidence: medium; commits: bc29d89ab483, a83a83fa4131, c6399fedc4a8; files: Sources/CodexBarCore/Providers/Mistral/MistralProviderDescriptor.swift, Sources/CodexBarCore/Providers/Mistral/MistralCookieImporter.swift, docs/providers.md)
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.
Review history (1 earlier review cycle)
  • reviewed 2026-07-08T05:08:27.734Z sha 3a37002 :: needs real behavior proof before merge. :: [P1] Keep Chrome ahead of new Mistral fallbacks | [P2] Do not scan the full default order for empty preferences

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a37002ec3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

/// Other Chromium forks stay on Manual import to avoid scanning the full default order.
public static var mistralCookieImportOrder: BrowserCookieImportOrder? {
#if os(macOS)
[.safari, .chrome, .firefox]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip invalid earlier browser sessions before retrying

When a user has an old or unauthorized Mistral ory_session_* cookie in Safari (or Chrome) and a valid session in a later browser, this new Safari → Chrome → Firefox order can make auto-import fail instead of falling through. MistralCookieImporter returns the first browser with any ory_session_*, and the invalidCredentials retry in MistralWebFetchStrategy clears the cache but calls resolveCookieHeader(false), which re-imports with this same order and picks the same bad source again, so Chrome/Firefox are never tried after the 401/403.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Updated to Chrome → Firefox → Safari in 525aabd so Chrome stays first — no regression from the original Chrome-only behavior. Firefox is still covered; Safari moved to last for Full Disk Access users.

The deeper issue (importSession returning the first browser with any ory_session_* cookie, without trying the next browser on 401/403) is a pre-existing design limitation shared by MiMo and other providers. Happy to address it in a follow-up if maintainers want a multi-session retry approach like ManusCookieImporter.importSessions.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. labels Jul 8, 2026
Avoids regression where a stale Safari session could block a valid
Chrome or Firefox session (Codex review feedback on steipete#1985). Chrome
first preserves the original Chrome-only behavior; Firefox still
detected; Safari follows for Full Disk Access users.
@djsavvy

djsavvy commented Jul 8, 2026

Copy link
Copy Markdown
Author

made chrome first

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. labels Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant