Skip to content

Fix Codex startup live import duplication#2590

Open
DhruvShankpal wants to merge 2 commits intofarion1231:mainfrom
DhruvShankpal:main
Open

Fix Codex startup live import duplication#2590
DhruvShankpal wants to merge 2 commits intofarion1231:mainfrom
DhruvShankpal:main

Conversation

@DhruvShankpal
Copy link
Copy Markdown

Fix Issue #2535

Summary

Fixes the startup live-import path so CC Switch does not recreate 'id="default"' for Codex when the app already has provider entries such as 'codex-official'.

Also keeps local current-provider settings in sync when a live import does happen.

What changed

  • Added a stricter startup-only provider existence check:
    • skip live import if the app already has any provider row
    • this includes official seeded providers like 'codex-official'
  • Updated live import to also write the local current-provider setting after setting DB 'is_current'

Why

Before this change, startup import only skipped when a non-official provider existed. That meant Codex could still auto-import live config even when 'codex-official' was already present, creating a 'default' provider and causing DB/settings current-provider drift.

Result

  • Existing Codex installs with 'codex-official' no longer create 'default' on every startup
  • Fresh installs can still import live config once when no providers exist
  • DB 'is_current' and local 'currentProvider' settings stay aligned after import

Notes

I could not run automated tests in the current shell because 'cargo' and 'pnpm' were not available in the environment.
I am relatively new to Rust and used the assistance of codex to help solve the problem so advise supervision before merge.

@JiangHe12
Copy link
Copy Markdown

Thanks for the PR! The approach looks correct — the root cause is that has_non_official_seed_provider treats codex-official as "no providers", so adding a broader check at startup makes sense.

A few things:

  1. Could you verify manually that a fresh install (empty DB) still correctly imports live config on first run?
  2. Could you check that restarting after the fix doesn't create new default entries?
  3. The third change (syncing set_current_provider to settings) is a nice touch but could be a separate commit for clarity — up to the maintainer.

Overall the fix is clean and addresses the issue well. 👍

@DhruvShankpal
Copy link
Copy Markdown
Author

DhruvShankpal commented May 6, 2026

Fix: Prevent duplicate Codex default provider on restart & add startup import tests

Hey @JiangHe12 I fixed the issue where restarting the app could create duplicate Codex default provider entries. The startup logic now checks for any existing provider before importing the live config, ensuring the import only happens on a true fresh install.

What’s included:

Refactored startup guard to check for any provider, not just non-official ones.

Added tests to verify:
Fresh install imports Codex live config as default provider.
Restart with existing provider skips import and does not create duplicates.

Test results:
Both new tests pass locally:

codex_startup_import_fresh_install_imports_once_and_syncs_current_setting
codex_startup_import_skips_when_only_official_seed_exists

Notes:
I locally installed the dependencies and the tests passed ok.
I synced set_current_provider to settings after import , let me know if you’d like the settings sync change split into a separate commit. You previously said it was optional so I didn't do that here, but i can explicitly change if you want.

@DhruvShankpal
Copy link
Copy Markdown
Author

Also @JiangHe12 I'm curious what the appropriate etiquette is when improving on my initial pull request. Is it better to just commit changes to same branch(like i did above) or create a new branch?

@JiangHe12
Copy link
Copy Markdown

The refactored approach with should_import_default_config_on_startup is cleaner, and the two tests cover exactly what I was concerned about — looks good to me.

For your etiquette question: committing to the same branch is the standard practice. It automatically updates this PR, keeps all discussion in one place, and the maintainer can review the full diff. Creating a new branch would mean a new PR and split context. So what you did is correct.

@farion1231
Copy link
Copy Markdown
Owner

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants