fix(cli): keep the selected model when refreshing another provider's template - #8868
Merged
Conversation
…template A provider template refresh reuses the first-time-install plan, which always carries a default "select the first model" instruction. The refresh acted on it whenever the current model was missing from the refreshed list — and a model owned by a different provider is always missing from it, so accepting an update silently rewrote model.name and cleared model.baseUrl. Leave the selection alone when the current model is installed under the same protocol but owned by another provider; a model that is not installed at all is still migrated. Also stop reporting "Model switched to <old model>" when the active model did not actually change. Fixes #8863
github-actions
Bot
added
the
review/self-reported
Collaborator
|
|
Collaborator
|
Thanks for the PR! Template ✓ — all sections present and filled in substantively. Two cosmetic misses, not worth a round-trip: the linked issue is declared inline ( Problem: real and well-evidenced. Linked issue #8863 is an observed bug with a full reproduction on 0.21.8 — a three-case comparison table, the disk-vs-UI contradiction captured verbatim, root cause traced against Direction: aligned. A template refresh silently rewriting Size: not a core-module path ( Approach: right scope. The guard goes at the only refresh call site: skip the plan's default selection when the current model is installed under the same protocol but owned by another provider, while a model installed nowhere is still migrated. The confirmation-message fix belongs to the same bug — the old code read the untouched runtime model and reported it as the "switched to" value precisely when the on-disk selection had just been overwritten. One boundary worth revisiting in code review: the guard is same-protocol-only by design, so I'll check what happens to a current model that lives under a different protocol. Risk: no elevated risk signals — no high-risk-path match. Moving on to code review. 🔍 中文说明感谢贡献！ 模板 ✓ —— 各部分齐全且内容充实。两处形式上的小缺失，不值得单独打回：关联 issue 以行内方式声明（"Why it's needed" 里的 问题：真实存在，证据充分。 关联 issue #8863 是已观测到的 bug，附有 0.21.8 上的完整复现——三种情形对比表、界面与磁盘矛盾的原文记录、针对 方向：对齐。 模板刷新静默改写 规模： 非核心模块路径（ 方案：范围合理。 守卫放在唯一的刷新调用点：当前模型安装在同一协议下但归属其他 provider 时，跳过 plan 的默认选择；完全未安装的模型仍然迁移。确认文案的修复属于同一个 bug——旧代码读的是未被改动的运行时模型，恰恰在磁盘上的选择刚被覆写时，把它报成了"切换到"的值。代码审查时值得再看一眼的边界：守卫按设计只覆盖同协议情形，我会确认当前模型位于其他协议下时会发生什么。 风险： 无升级风险信号——未命中高风险路径。 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewI traced the fix end-to-end before agreeing with it. Deleting Checked against the issue's fixture: Also confirmed the guard sits at the only call site that needs it: One observation, non-blocking: all eleven built-in providers are No critical blockers, no convention violations. Tests pin the change where it lives — asserting Test evidenceThis is an unattended CI run — no PR code was built or executed here; the evidence below is the PR's own CI, read via the API at the reviewed commit. The unit suite Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification would settle what unit mocks can't: 中文说明代码审查：先把修复链路完整走了一遍才认可。删掉 用 issue 的 fixture 核对过： 还确认了守卫放在唯一需要它的调用点： 一个非阻塞的观察：十一个内置 provider 都是 无关键阻塞项，无规范违规。测试钉住了改动发生的位置——断言 测试证据：本次为无人值守 CI 运行——未构建或执行任何 PR 代码，以上证据来自 API 读取的被审 commit 自身 CI。单元测试 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — a correct, minimal fix for a P1 data-loss bug, verified mechanically against core's install path; the only reservation is a pre-existing residual gap, named below and non-blocking. Stepping back: the motivation is beyond doubt — this is the unfinished half of #5819, where a user was silently moved onto an expensive model and only noticed when their prepaid balance ran out, and #8863 shows the same overwrite still reachable through every one of the eleven built-in providers. The implementation is the minimum-blast-radius version of the fix: drop the plan's default selection at the one call site that carries no selection intent, keep the pre-existing migration for genuinely dangling models, and correct a confirmation message that was actively lying about the other half of the same bug. The alternatives I'd have reached for — a refresh flag in The tests assert the right thing at the right level — The unit suite is still running, so approval is deferred until CI lands green on 中文说明整体来看：动机毋庸置疑——这是 #5819 未完成的一半，当时有用户被静默换到高价模型、直到预存余额烧完才发现；#8863 证明同样的覆写在十一个内置 provider 上依然可达。实现是爆炸半径最小的修法：在唯一没有选择意图的调用点丢弃 plan 的默认选择，保留对真正悬空模型的既有迁移，并修正了同一个 bug 另一半里"睁眼说瞎话"的确认文案。我原本会想到的替代方案——给 测试在正确的层面断言了正确的事—— 单元测试仍在运行，因此批准推迟到 CI 在被审 commit 上全绿之后。 — Qwen Code · qwen3.8-max Reviewed at |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
Author
|
@qwen-code /verify |
Collaborator
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 43 passed · 0 failed · 43 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：43 通过 · 0 失败 · 43 总计 Verification reportPR #8868 — fix(cli): keep the selected model when refreshing another provider's templateVerdict: 中文摘要
Central claim & A/BCentral claim: accepting a built-in provider template refresh must not rewrite the user's selected model / base URL when the current model belongs to a different provider, and must never change the selected auth method — while genuine migration (refreshed provider is the active one and the model left its list) and owned-model carryover keep working. Both arms are real bundled CLIs (
The C1 panes are captured as-rendered: Secondary claim (migration preserved): for built-in providers an owned model is always carried into or regenerated by the install plan, so the migration branch is only reachable via a runtime-only (uninstalled) current model — not constructible as a stable E2E fixture (it needs the runtime's baseUrl→envKey derivation). It is instead proven at the unit level: Reviewer Test Plan, step by step
Mutation matrix (vacuity)Raw run:
No survivors. Attribution nuance worth keeping: M3 kills only the migration test — the Findings
Not covered
MethodologyEnvironment: Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
doudouOUC
approved these changes
Aug 10, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the current head 1bdc311 end to end.
- Traced the refresh path through
buildInstallPlanandapplyProviderInstallPlan: inactive-provider refreshes now dropmodelSelection, preserve the selected auth method, and still persist the refreshed provider models and metadata; an active provider whose selected model is genuinely unavailable still follows the migration path. - Checked same-protocol, cross-protocol, runtime-only, multi-provider sequencing, adapter rollback delegation, and success-message behavior.
- Verified locally: 20/20 targeted hook tests, related ESLint, and Core typecheck. The current Ubuntu suite, Desktop Shell checks, and web-shell E2E are also green.
No blocking findings.
yiliang114
enabled auto-merge
August 10, 2026 12:09
wenshao
reviewed
Aug 10, 2026
| (cfg) => cfg.id === previousModel, | ||
| ); | ||
| // Only the active provider may migrate model selection. | ||
| if (!updatesActiveProvider || previousModelStillAvailable) { |
Collaborator
There was a problem hiding this comment.
Gating only modelSelection doesn't stop the refresh from re-pointing the active user's model id.
applyProviderInstallPlan still writes modelProviders.<protocol> with the refreshed provider's models prepended (prepend-and-remove-owned), and ModelRegistry.getModel(authType, id) resolves a bare id to the first matching array entry.
Failure scenario: Coding Plan and Token Plan are both AuthType.USE_OPENAI and share model ids (qwen3.7-plus, qwen3.6-plus, kimi-k2.5, glm-5, MiniMax-M2.5). A user onboarded via /auth on Coding Plan has model.name = qwen3.7-plus and model.baseUrl = '' (the empty tombstone applyProviderInstallPlan writes when the plan's modelSelection carries no baseUrl). They accept the Token Plan template refresh.
With this patch, model.name is indeed never written and the message is "Token Plan configuration updated successfully." — but the array handed to config.reloadModelProvidersConfig now has
{ id: 'qwen3.7-plus',
baseUrl: 'https://token-plan.cn-beijing.maas.aliyuncs.com/compatible-mode/v1',
envKey: 'BAILIAN_TOKEN_PLAN_API_KEY' }
as the first entry for that id. Because model.baseUrl is empty, the id-only lookup (modelRegistry.ts getModel scan, and the ?? this.modelRegistry.getModel(authType, modelId) fallback at modelsConfig.ts:1039) now returns the Token Plan entry. On the next launch (or any in-session re-resolve) requests for qwen3.7-plus go to the Token Plan endpoint with BAILIAN_TOKEN_PLAN_API_KEY, which the user never set — missing-API-key / 401.
That is exactly the outcome the PR title promises to prevent. The new it.each('does not move the user off a model $name') case only asserts model.name / model.baseUrl are not written, so it passes while the re-pointing still happens. Consider also suppressing the modelProviders prepend (or appending instead of prepending) when !updatesActiveProvider, and asserting on the array passed to reloadModelProviders.
wenshao
reviewed
Aug 10, 2026
| delete installPlan.modelSelection; | ||
| } | ||
| const activeConfig = config.getContentGeneratorConfig(); | ||
| const updatesActiveProvider = |
Collaborator
There was a problem hiding this comment.
updatesActiveProvider is false whenever getContentGeneratorConfig() is undefined, so the new gate also suppresses migration for the provider the user is actually on.
That state is reachable and this suite documents it — see the existing test "does not refresh auth before auth initialization completes".
Failure scenario: startup auth fails (missing/expired key), so performInitialAuth leaves contentGeneratorConfig undefined and the auth dialog opens alongside the provider-update prompt. The user accepts the Coding Plan template refresh to repair things. activeConfig?.authType === providerCfg.protocol evaluates undefined === 'openai' → false, so !updatesActiveProvider deletes installPlan.modelSelection and neither syncAuthState nor refreshAuth runs.
Before this change the plan still ran its own planOffersCurrentModel check and, when the current model was not offered by the refreshed template, wrote model.name to the template's first model. Now model.name keeps pointing at a model the refreshed template no longer offers, so after the user fixes their API key the session still fails to resolve a model and they have to run /model by hand — while the hook reported "Coding Plan configuration updated successfully." (the message condition at line 295 is now satisfied by activeModel === previousModel).
Also worth checking: the providerMatchesCredentials(providerCfg, activeConfig.baseUrl, ...) comparison is unnormalized, so a trailing-slash / case difference in baseUrl produces the same false negative even when auth is initialized.
wenshao
reviewed
Aug 10, 2026
| ...settingsAdapter, | ||
| setValue: (key, value) => { | ||
| // Template updates never change the selected auth method. | ||
| if (key !== 'security.auth.selectedType') { |
Collaborator
There was a problem hiding this comment.
The security.auth.selectedType write is suppressed unconditionally — including for the active provider, where re-persisting it is the one thing that repairs an unpersisted auth type.
selectedAuthType can be resolved from --auth-type or getAuthTypeFromEnv() without ever being written to settings (packages/cli/src/config/config.ts:1908-1912). A user in that state who is genuinely on this provider accepts a template refresh: the hook still calls refreshAuth(plan.authType) and can still write a migrated model.name, but security.auth.selectedType is now never persisted. The settings file ends up holding a model id for provider X with no selected auth type, so the next launch without the env var resolves a different (or no) auth type and fails to load the model.
The suppression only needs to cover the inactive-provider case that motivated it:
Suggested change
| if (key !== 'security.auth.selectedType') { | |
| if (updatesActiveProvider || key !== 'security.auth.selectedType') { |
Collaborator
Local verification on a real stack — LGTM, recommend mergeI rebuilt both sides and drove the real bundled CLI through the actual update prompt (no mocks, no unit-test harness): two worktrees,
Provenance check on the two bundles, so the A/B can't be a stale artifact: // base/dist/chunks/chunk-4HGX7VSQ.js
if (previousModelStillAvailable) { delete installPlan.modelSelection; }
...
if (previousModelStillAvailable && activeModel === previousModel) {
// head/dist/chunks/chunk-6IQIEFUZ.js
if (!updatesActiveProvider || previousModelStillAvailable) { delete installPlan.modelSelection; }
...
setValue: (key, value) => { if (key !== "security.auth.selectedType") { settingsAdapter.setValue(key, value); } }
...
if (activeModel === previousModel) {The prompt under test
Case A — the #8863 repro (self-hosted gateway model, Token Plan template stale)The UI line on
What lands on disk, and what the next launch runs:
Case B — the auth method is rewritten too (second commit, undocumented in the description)This one is arguably worse than the titled bug and is worth surfacing: a Gemini user whose Token Plan template happens to be stale accepts the update, and
On-disk state, both cases
All cases
The refresh itself keeps working in every case: built-in models are installed (17/17, 16/16, 15/15 entries in TestsMutation A/B — each guard rolled back on its own, in the PR's own tree:
No guard passes for free. Also green: Notes before merge (non-blocking)
Environment: macOS 15 (Darwin 25.6.0, arm64), Node v24.18.1, 中文版本本地真实环境验证 —— 结论：可以合并我把两侧分别重新打包，用真实的 bundle 后 CLI 走了一遍真正的更新提示（没有 mock，也不是只跑单测）：两个独立 worktree 各自
为了排除产物陈旧，先核对了两个 bundle 里的实际代码（见上方英文段落的代码块）：base 是旧的 用例 A（#8863 复现）： 用例 B（第二个 commit，描述里没写）：这个问题比标题里的更严重。一个用 Gemini 的用户，只要 Token Plan 模板恰好过期，接受更新后磁盘上的
刷新本身在所有用例里都没被破坏：内置模型照常安装（17/17、16/16、15/15 条），用户自定义条目保留， 测试： 合并前的建议（都不阻塞）：
环境：macOS 15（Darwin 25.6.0, arm64），Node v24.18.1， |
chiga0
reviewed
Aug 10, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Code Review Overview (AI Generated)
PR: #8868 fix(cli): keep the selected model when refreshing another provider's template
Type: Bug fix
Change size: +151/-36 across 2 files
Findings Summary
- Critical/Major: 3 items (all cross-validated against existing reviewer comments)
- Minor: 0
- Nit: 0
Key Observations
The fix correctly stops model.name/model.baseUrl from being overwritten when refreshing an inactive provider, and it improves the success message. However, the same prepend-and-remove-owned merge still re-orders the modelProviders.<protocol> array so that a bare model id can resolve to the refreshed provider's endpoint on the next lookup. In addition, two gating decisions are over-broad: model migration is suppressed when getContentGeneratorConfig() is undefined, and security.auth.selectedType is suppressed even for the active provider.
Cross-Validation
| Finding | Other Reviewer | My Assessment |
|---|---|---|
C1: Gating only modelSelection does not prevent inactive-provider refreshes from re-pointing a bare model id, because modelProviders.<protocol> is still prepended and ModelRegistry.getModel resolves id-only lookups to the first matching entry. |
wenshao at useProviderUpdates.ts:267 |
Confirmed — Major. Verified applyProviderInstallPlan uses prepend-and-remove-owned and ModelRegistry.getModel scans by insertion order when baseUrl is omitted/empty. The new tests do not assert the order of the array passed to reloadModelProvidersConfig. |
C2: When config.getContentGeneratorConfig() is undefined (e.g., auth not initialized), updatesActiveProvider becomes false and the current model is no longer migrated even if it is genuinely removed from the refreshed template. |
wenshao at useProviderUpdates.ts:255 |
Confirmed — Major/Minor. The new gate intentionally avoids migration without a known active provider, but this is a behavior change from the previous planOffersCurrentModel path and can leave a stale model id behind after the user fixes auth. |
C3: The setValue wrapper suppresses security.auth.selectedType for every provider, including the active one. For users whose selected auth type was inferred from --auth-type or env and never persisted, an active-provider refresh will not repair it. |
wenshao at useProviderUpdates.ts:277 |
Confirmed — Major. The suppression should be conditional: `if (updatesActiveProvider |
Additional Audit Coverage
Areas I independently checked that go beyond the existing findings:
- Caller/consumer impact: Searched usages of
useProviderUpdates; onlyAppContainer.tsxconsumes the hook and its interface is unchanged. No public function signatures changed. - Project convention compliance: The PR modifies an existing camelCase file (
useProviderUpdates.ts) that is allow-listed ineslint.legacy-filenames.mjs; no new cross-package imports or style violations. - Core install guard: I verified that
applyProviderInstallPlaninpackages/core/src/providers/install.tsstill carries theplanOffersCurrentModelguard, so the caller-side fix is the intended control point; however, the unchanged core guard does not protect against the array-reordering issue identified above. - Test coverage: New tests cover same-protocol/different-protocol inactive providers, the success-message wording, and auth non-initialization, but none assert
modelProvidersordering or the active-providerselectedTypepersistence case.
Final Verdict
Needs changes before merge. The silent model re-pointing via registry ordering and the unconditional auth-type suppression can re-create the data-loss/failure modes the PR aims to fix. Once C1 and C3 are addressed (and C2 is explicitly accepted or handled), this should be re-reviewed.
This review was generated by QoderWork AI
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.8-max via Qwen Code /review (v0.21.8)
Comment on lines
+295
to
296
| if (activeModel === previousModel) { | ||
| addItem( |
Collaborator
There was a problem hiding this comment.
[Suggestion] The kept-model success message on the active-provider refresh path is pinned by no test — Concrete cost: mutating this condition to if (!updatesActiveProvider && activeModel === previousModel) leaves all 20 tests green (verified by running the suite), so a regression reintroducing the false Model switched to "…" confirmation on this exact path would ship green. 'executes update when user confirms with "update"' is the only active-provider test where the model is kept, and it asserts side effects only, never the toast text.
Suggested addition to that test (in useProviderUpdates.test.ts):
expect(mockAddItem).toHaveBeenCalledWith(
{ type: 'info', text: 'Coding Plan configuration updated successfully.' },
expect.any(Number),
);Verified this assertion catches the mutant (1 failed | 19 passed).
— qwen3.8-max via Qwen Code /review (v0.21.8)
wenshao
approved these changes
Aug 10, 2026
qwen-code-ci-bot
added a commit
that referenced
this pull request
Aug 10, 2026
This was referenced Aug 10, 2026
Collaborator
|
Released in v0.21.9. |
Collaborator
Author
|
Recording the final shape of this change, since the description above was written against an earlier revision of it. The condition that decides whether a template refresh may migrate the model selection ended up as This also fixes a second write the description does not mention. The confirmation message reports a switch only when the active model actually changed ( Tests cover both provider-owned and active-runtime-only foreign models via |
rbalachandar
pushed a commit
to rbalachandar/qwen-code
that referenced
this pull request
Aug 12, 2026
…ates (QwenLM#8879) * test(cli): pin the model selection across multi-provider template updates The worst case reported in QwenLM#8863 was the batch path: with several providers updating in one confirmation, each executeUpdate in the loop rewrote model.name in turn and the last provider in registry order won. The QwenLM#8868 gate protects it, but no test failed if that regressed — its regression tests cover a single updating provider. Pin the batch scenario: two stale providers, a current model owned by neither, one 'Update all' confirmation, and no model.name/model.baseUrl writes from either pass. Reverting the gate to the pre-QwenLM#8868 form now fails this test alongside the two single-provider ones. * test(cli): verify both batch entries execute and pin the mixed-batch gate Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): pin no-migration in the mixed batch after QwenLM#8889 QwenLM#8889 made the template update path drop the plan's model selection unconditionally, so the mixed-batch test that expected the active provider to migrate now pins removed behavior. Invert it: a batch mixing the active provider (current model gone from its plan) with an inactive one must leave model.name and model.baseUrl untouched while both updates still run to completion — the batch-loop side of the invariant whose single-provider side QwenLM#8889 pins itself. --------- Co-authored-by: ComplexSimply <rudy.arrowsong@gmail.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
A built-in provider template refresh no longer changes which model the user has selected when that model belongs to a different provider. The install plan that a refresh reuses always carries a first-time-install default selection — pick the provider's first model — and until now a refresh acted on it whenever the current model was absent from the refreshed list. Because a model owned by any other provider is by definition absent from that list, accepting a refresh silently rewrote the user's selected model and wiped the base URL that disambiguated it.
The refresh path now leaves the selection alone when the current model is installed under the same protocol but owned by a different provider. A model that is not installed at all is still migrated, so a user is never left pointing at nothing.
The accompanying confirmation message was also wrong: it reported
Model switched to "<the user's old model>"in exactly the case where the selection had just been overwritten, because it read the runtime model, which the refresh does not touch. It now reports a switch only when the active model actually changed.Why it's needed
Anyone who mixes a built-in provider with a self-hosted gateway, an internal proxy, or another vendor's provider loses their model selection and their custom base URL the first time they accept a provider update. Nothing signals it: the current session keeps running the old model, the footer keeps showing it, and the confirmation message claims nothing changed. The new value only takes effect on the next launch — by which point the update prompt that caused it is long gone from the user's mind.
All eleven built-in providers share this path, and each one substitutes its own first model, most of which are the flagship, highest-priced tier:
qwen3.5-plus,qwen3.7-plus,qwen3.6-plus,deepseek-v4-pro,grok-4.5,MiniMax-M3,GLM-5.2, and so on. When several providers are stale at once the loop applies each in turn and the last one wins, so the resulting model bears no relation to anything the user chose.This is the unfinished half of #5819, where the reporter was moved from a cheap model to the expensive one and only discovered it when their prepaid balance ran out. The guard added for that issue asks the same question as the one it was backing up — "is the current model in this plan?" — so both miss together on a model this provider never owned.
Fixes #8863
Reviewer Test Plan
How to verify
Point
model.nameat a model that a built-in provider does not own, mark that provider's stored version stale, then accept the update prompt. The startup check only compares the version string for equality, so any stale value works and no real API key is needed.Choose
1. Update all, then inspect the file:node -e 'const s=require(process.env.REPRO_HOME+"/.qwen/settings.json"); console.log(s.model)'A reviewer should confirm that
model.namestaysmy-own-modelandmodel.baseUrlkeeps the gateway URL, that the built-in models were still installed and the custom model preserved (the refresh itself must keep working), and that the confirmation message readsconfiguration updated successfully.with noModel switched toclause.Three neighbouring cases are worth checking as well, since they are what bounds the fix: a model the provider owns is untouched, a model absent from every installed list is still migrated (covered by the pre-existing
switches model when previous model is no longer availabletest), and the update prompt itself is unchanged — the prompt reappearing on every launch is a separate defect tracked in #8504 and deliberately out of scope here.Unit tests:
Both new tests were checked against a reverted fix: each one fails on its own when the corresponding guard is rolled back, so neither passes for the wrong reason.
Evidence (Before & After)
Same fixture, same keystroke, reading the file on disk right after the prompt was accepted.
Before:
After:
With two providers stale at once, the before case ends on
grok-4.5rather thanqwen3.7-pluswhile both confirmation lines claim the old model was kept — the same fixture is clean after the fix.Tested on
Environment (optional)
npm run build && npm run bundle, then the bundleddist/cli.jsdriven against an isolatedHOME.Risk & Scope
/modelremains the way to switch.