fix(web): default provider selection for users without Codex#4117
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
f720ae6 to
d6da114
Compare
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces significant behavioral changes to provider selection logic with new fallback mechanisms and UI states. Two unresolved medium-severity review comments identify potential bugs in the new resolution logic (partial hydration issues and settings overlay not being applied). You can customize Macroscope's approvability policy. Learn more. |
|
Pushed a follow-up commit addressing the review findings:
Added unit tests for the fallback tiering, requested-instance retention, and the model helper. |
New projects created via the command palette hard-coded codex as their defaultModelSelection, and the composer dispatched a persisted instance id even when that instance was known-disabled. Users without Codex got threads permanently pinned to a provider that can never start (issue pingdotgg#4116). - CommandPalette: resolve the project default to the first ready provider instance (falling back to enabled, then the codex literal) and stamp that instance's default model instead of the codex one; add missing providers dependency so the callback doesn't capture the at-mount provider list. - ChatComposer: when the persisted instance is known-but-disabled, fall through to an enabled instance instead of returning the raw id. - resolveSelectableProviderInstance: prefer ready instances so an enabled codex without the CLI installed no longer wins by list order.
… recovery Review follow-ups on the pingdotgg#4116 fix: - resolveSelectableProviderInstance: fall back ready -> non-error -> enabled so unprobed (warning) instances beat errored ones; previously an enabled codex with a failed probe could still win by list order during startup. - ChatComposer: abandon a known-disabled persisted instance only when an enabled instance satisfies the thread's driver/continuation lock, keeping the accurate server error otherwise; derive the composer's driver kind from the resolved instance so options and send context follow the instance that actually runs. - CommandPalette: stamp the resolved instance's own default model via instance-scoped lookup instead of the driver default, which could pair a custom instance with another instance's model slug. - Tests for the fallback tiering, requested-instance retention, and the instance-scoped model helper.
Preserve valid stored selections, recover stale selections deterministically, and make the no-provider composer state explicit and non-dispatchable. Co-authored-by: codex <codex@users.noreply.github.com>
caf8c05 to
44b14f4
Compare
Dismissing prior approval to re-evaluate 44b14f4
| * only when it was explicitly requested; it is never invented as a new-user | ||
| * default. | ||
| */ | ||
| export function resolveSelectableProviderInstanceEntry( |
There was a problem hiding this comment.
🟡 Medium src/providerInstances.ts:299
resolveSelectableProviderInstanceEntry silently replaces a persisted instanceId whose snapshot hasn't arrived yet with the first ready/non-error instance, because it treats a not-yet-hydrated custom instance the same as a deleted one. During partial provider hydration, this causes callers to dispatch turns through the wrong provider instead of retaining the unknown ID until its snapshot arrives. Consider distinguishing absent-but-known IDs from stale ones, or preserving the original instanceId when no matching entry is found instead of eagerly substituting a fallback.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/providerInstances.ts around line 299:
`resolveSelectableProviderInstanceEntry` silently replaces a persisted `instanceId` whose snapshot hasn't arrived yet with the first ready/non-error instance, because it treats a not-yet-hydrated custom instance the same as a deleted one. During partial provider hydration, this causes callers to dispatch turns through the wrong provider instead of retaining the unknown ID until its snapshot arrives. Consider distinguishing absent-but-known IDs from stale ones, or preserving the original `instanceId` when no matching entry is found instead of eagerly substituting a fallback.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 44b14f4. Configure here.
| defaultModelSelection: resolveDefaultProviderModelSelection( | ||
| targetEnvironmentProviders, | ||
| null, | ||
| ), |
There was a problem hiding this comment.
Defaults ignore settings overlay
Medium Severity
New project defaults call resolveDefaultProviderModelSelection on raw serverConfig.providers (or the primary providers atom) without applying applyProviderInstanceSettings. A provider disabled in settings can still look enabled in the probe snapshot, so palette-created projects can be persisted back onto that instance—the same failure mode this PR aims to fix.
Reviewed by Cursor Bugbot for commit 44b14f4. Configure here.
…gg#4117) Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: codex <codex@users.noreply.github.com>
…gg#4117) Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: codex <codex@users.noreply.github.com>
…dotgg#2284) (#184) * Sidebar v2 beta: flat thread list with a server-backed settled lifecycle (pingdotgg#4026) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: maria-rcks <maria@kuuro.net> (cherry picked from commit 32c6012) * fix(settings): validate the add-provider wizard step before advancing (pingdotgg#2813) (pingdotgg#3100) Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 282ecb3) * fix(claude): isolate capability probe from user MCP servers (pingdotgg#4015) Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit aa5ec80) * Preserve connecting status while a turn starts (pingdotgg#4101) Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 783692a) * fix(server): stop restoring stale OpenCode models (pingdotgg#4095) Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 4e09cdd) * [codex] keep scoped package references as text (pingdotgg#4167) Co-authored-by: codex <codex@users.noreply.github.com> Co-authored-by: Julius Marminge <julius0216@outlook.com> (cherry picked from commit c7b21ff) * fix(web): default provider selection for users without Codex (pingdotgg#4117) Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit b6e1b39) * Unify temporary worktree branch naming (pingdotgg#4278) (cherry picked from commit 571a8b4) * fix(web): use message-square icon for settled icon-less project threads in sidebar v2 (pingdotgg#4279) (cherry picked from commit 020179c) * Stabilize sidebar settling animations (pingdotgg#4280) (cherry picked from commit 18b4688) * Restore Copy Link in chat link context menu (pingdotgg#4161) Co-authored-by: Julius Marminge <julius0216@outlook.com> (cherry picked from commit e5fba26) * fix(desktop): handle EPIPE errors on stdout/stderr to prevent crash dialog (pingdotgg#4213) (cherry picked from commit f74eb62) * Preserve draft thread highlighting during promotion (pingdotgg#4283) Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 18fa89c) * Move mobile working timer into the thread timeline (pingdotgg#4285) (cherry picked from commit 7e2bb47) * Stabilize PR status lookups and provider session lifecycle (pingdotgg#4281) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 376c149) * fix: open command palette instead of custom dialog for new thread picker in SidebarV2 (pingdotgg#4269) (cherry picked from commit 9fe4832) * fix(server): don't drop sticky PR fallback when remote URL can't be resolved (pingdotgg#4289) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 9a0a071) * feat(web): copy branch name via right-click in the branch selector (pingdotgg#4275) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 78a0ea5) * Add remote server updates and standalone service management (pingdotgg#4286) Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit ab4a883) * Refine light-mode sidebar surfaces (pingdotgg#4268) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 593289c) * fix(mobile): don't mark Android VPN/Tailscale as offline when connected (pingdotgg#3949) (cherry picked from commit bc9428a) * improve and prevent silent thread branch drift and PR fetching (pingdotgg#2284) Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 2d31cb0) * fix(sync): restore fork migration IDs under Sidebar v2 settled Upstream pingdotgg#4026 replaced Migrations.ts with linear upstream numbering and dropped fork-only migrations (NormalizeLegacyProviderKinds, RepairProposedPlanColumns, BackfillForkProviderInstanceIds). Restore the fork registry and append ProjectionThreadsSettled as ID 36. Co-authored-by: aaditagrawal <aaditagrawal@users.noreply.github.com> --------- Co-authored-by: Theo Browne <me@t3.gg> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: maria-rcks <maria@kuuro.net> Co-authored-by: Leonel Rivas <encriptandost@gmail.com> Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: codex <codex@users.noreply.github.com> Co-authored-by: Jaret Bottoms <jaretbottoms@gmail.com> Co-authored-by: Ishan <ishansachu1@gmail.com> Co-authored-by: Yukun Shan <92423096+nateEc@users.noreply.github.com> Co-authored-by: Maxwell Young <maxtheyoung@gmail.com> Co-authored-by: Miklós Fazekas <mfazekas@szemafor.com> Co-authored-by: Utkarsh Patil <73941998+UtkarshUsername@users.noreply.github.com> Co-authored-by: Henry Zhang <113233555+caezium@users.noreply.github.com> Co-authored-by: Rushikesh Gaikwad <81632222+Wraient@users.noreply.github.com> Co-authored-by: legs <145564979+justsomelegs@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: aaditagrawal <aaditagrawal@users.noreply.github.com>


What Changed
codexas the new project'sdefaultModelSelection. It resolves the first usable provider instance (ready → non-error → enabled, then the codex literal), stamps that instance's own default model, and adds the missingprovidersentry to the callback dependencies so the handler does not act on the at-mount provider snapshot.resolveSelectableProviderInstancetiers its fallback by probe status andgetDefaultProviderInstanceModelprovides an instance-scoped default model. Unit tests cover the tiering, requested-instance retention, and the model helper.Why
Fixes #4116. New users without Codex get their first palette-created project defaulted to codex, so the first thread fails (
Failed to spawn Codex App Server processon fresh settings, orProvider instance 'codex' is disabled in T3 Code settings.when codex is disabled) and the thread stays permanently pinned to codex. The composer change also lets existing broken threads recover: on the next send they fall through to a working provider.UI Changes
Before (thread pinned to codex, turn fails, composer shows the OpenAI icon with a claude model):
After (same flow defaults to the ready Claude instance and the send succeeds):
Checklist
Note
Fix default provider selection for users without Codex access
codexas the default provider/model across project creation, the chat composer, and the command palette; new projects now start withnullfordefaultModelSelection.NO_PROVIDER_MODEL_SELECTIONas a sentinel value in providerInstances.ts and new helpers (resolveDefaultProviderModelSelection,resolveSelectableProviderInstanceEntry,getDefaultProviderInstanceModel) that prefer ready/non-error instances and returnnullwhen no viable provider exists.resolveSelectableProviderInstancemay now returnundefinedinstead of always falling back to the first enabled instance, and storeddefaultModelSelectionon new projects isnullinstead of a codex default.📊 Macroscope summarized 44b14f4. 3 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.
Note
Medium Risk
Changes default model selection and composer send gating across project creation and chat, but behavior is covered by expanded unit tests and narrows failure modes for misconfigured providers.
Overview
Stops assuming Codex as the default provider when none is configured or available. New projects from the command palette and the low-level
project.createpath now getdefaultModelSelectionfromresolveDefaultProviderModelSelection(first ready instance, then non-error, with that instance’s default model) ornullwhen nothing is sendable. Draft threads without a project default use a localNO_PROVIDER_MODEL_SELECTIONplaceholder instead of inventing Codex.providerInstancesgains instance-scopedgetDefaultProviderInstanceModel, tieredresolveSelectableProviderInstanceEntry, and shared default-resolution logic; errored instances are not auto-picked for new users but can stay selected when explicitly requested.The composer resolves the active instance via the new helpers (respecting locks), drives UI/send off whether a real provider exists (
providerAvailable), and blocks send with “Enable a provider in Settings” / “No provider available” when none is available.ChatViewsend paths honorproviderAvailableand drop the Codex fallback in provider resolution.Reviewed by Cursor Bugbot for commit 44b14f4. Bugbot is set up for automated code reviews on this repo. Configure here.