fix(cli): restore saved custom model IDs when re-entering the auth wizard - #5654
Conversation
E2E / Test Report (updated after rebase + design improvements)Automated tests (vitest, all green):
Design improvement (post-rebase): The rebase surfaced that
Real-CLI verification (tmux, isolated HOME): Settings pre-seeded with Coding Plan: 1 built-in model ( Scene 1 — startup diff prompt (custom model NOT in diff):
Scene 2 —
Full tmux log: 中文说明E2E / 测试报告（rebase + 设计改进后更新）自动化测试（vitest，全部通过）：
设计改进（rebase 后）： rebase 发现
真实 CLI 验证（tmux，隔离 HOME）： settings 预置 Coding Plan：1 个内置 model（ 场景一 — 启动 diff 提醒（自定义 model 不在 diff 中）：
场景二 —
完整 tmux 日志： — Qwen Code |
|
Thanks for the PR! Template looks good ✓ On direction: this is a clear-cut data-loss bug — re-entering the auth wizard silently overwrites saved custom model IDs with built-in defaults. Issue #5636 is tagged P1, and the desktop app already has the correct behavior. Bringing the CLI to parity is well within scope. Aligned. On approach: the scope feels right. The PR does exactly what's needed — moves the existing Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 方向：这是一个明确的静默数据丢失 bug —— 重新进入 auth 向导会用内置默认值覆盖用户保存的自定义 model ID。Issue #5636 标记为 P1，桌面端已有正确行为。CLI 对齐完全合理。 方案：范围恰当。将已有的 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal (before reading the diff): I'd add a lookup for existing model IDs in settings when The PR does exactly this — and better. The The callers are correct:
No correctness bugs, no security issues, no AGENTS.md violations. Clean, minimal change. TestingBefore (installed build, v0.18.5 — bug reproduces)Settings pre-configured with custom model ID Bug confirmed: Input shows empty After (dev build — TUI interaction issue)The dev build's TUI (via tsx) didn't accept tmux send-keys input in this CI environment — the Ink stdin handler wasn't responding to tmux keystrokes. Unable to complete the interactive After test. Mitigated by: the new AuthDialog component test directly validates the fix: This test creates a settings object with Unit Test Results
All tests pass, including the new 中文说明代码审查独立方案（读 diff 之前）：在 PR 完全做到了这一点。 无正确性 bug、无安全问题、无 AGENTS.md 违规。 测试修复前（已安装构建 v0.18.5 — bug 重现）输入框为空 修复后（dev 构建 — TUI 交互问题）dev 构建的 TUI 在此 CI 环境中不响应 tmux 按键输入。通过 AuthDialog 组件测试验证： 测试通过。 单元测试结果
— Qwen Code · qwen3.7-max |
|
This is a clean, well-scoped bug fix that solves a real problem — silent data loss when re-entering the auth wizard. Stepping back: the PR takes an existing lookup function that was buried in the ACP agent path, moves it to core where it belongs, and wires it into the auth wizard that was previously ignoring saved state. The approach matches my independent proposal exactly. The diff is 8 files, all directly related to the fix — no scope creep, no drive-by changes. The Before test clearly shows the bug: empty One note: I couldn't complete the interactive After test because the dev build's TUI didn't accept tmux keystrokes in this environment. The AuthDialog component test covers the same scenario programmatically and passes, so I'm confident the fix works. Approving. ✅ 中文说明这是一个干净、范围恰当的 bug 修复，解决了真实问题 —— 重新进入 auth 向导时的静默数据丢失。 PR 将已有的查找函数从 ACP agent 路径移到 core，然后接入之前忽略已保存状态的 auth 向导。方案与我的独立构想完全一致。diff 涉及 8 个文件，全部与修复直接相关。 修复前测试清楚展示了 bug：空的 注意：由于 dev 构建的 TUI 在此环境中不响应 tmux 按键，未能完成交互式 After 测试。AuthDialog 组件测试以编程方式覆盖了相同场景并通过。 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
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. |
33fc4f0 to
461b6df
Compare
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Downgraded from Approve to Comment: CI failing (Post Coverage Comment, Test on ubuntu-latest Node 22.x) — cannot approve on red CI. Code-level review found no Critical issues; two Suggestions below. The rest of the extraction is well-executed and the added AuthDialog test correctly validates the pre-fill behavior.
— qwen3.7-max via Qwen Code /review
d776c09 to
596883e
Compare
…zard When re-entering the provider auth wizard for a ModelStudio provider that already had saved model IDs, the Model IDs step reset to the provider's built-in defaults instead of the user's saved models. Submitting then overwrote the previously saved custom model IDs with the defaults, losing the user's configuration. Share the existing model-lookup logic from core between the desktop/ACP path and the CLI wizard, and have the wizard pre-fill the Model IDs step with the saved models when present, falling back to the built-in defaults otherwise. Closes #5636 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
596883e to
aaf2542
Compare
executeUpdate rebuilt the install plan with only built-in model IDs, so the prepend-and-remove-owned merge silently deleted user-added custom models. Carry custom IDs into the install plan so an update only refreshes built-in models and leaves user-configured ones untouched.
|
@qwen-code /triage |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No new issues found. The earlier review threads — carrying user-added custom IDs through executeUpdate, the diff-filter test coverage, and the ModelIdsStep split comment — are addressed at this head, and the built-in/custom split via getDefaultModelIds is applied consistently across the three call sites (getExistingModelIds, getInstalledOwnedModelIds, executeUpdate).
One thing I checked and cleared: the provider.protocol (in readInstalledOwnedIds) vs protocolOptions (in findExistingProviderModels) divergence cannot cause data loss today — the only multi-protocol provider is customProvider, which has models: undefined, so resolveMetadataKey returns undefined and it never enters findAllPendingUpdates/executeUpdate. It's a latent inconsistency worth a comment in the code if findExistingProviderModels is meant to be the single source of truth, but not a bug.
Local verification on 213e07a (isolated worktree, fresh npm ci): npm run typecheck ✅, ESLint on changed files ✅, core provider-config ✅ 63/63, cli useProviderUpdates + AuthDialog + acpAgent ✅ 176/176.
cli/qwen-serve-streaming.test.ts (multi-client permission fan-out / Last-Event-ID resume) — an SSE integration test unrelated to this PR's changed files, failing on a ~90s timeout with retry x2 (flaky), green on macOS/Windows. Not a regression from this change.
— claude-opus-4-8 via Qwen Code /qreview
✅ Local runtime verification report (maintainer) — restore saved custom model IDsVerified this PR at head Scene 1 — startup "Built-in Provider Update" prompt (the spurious-diff bug)Booting with the stale version hash fires the update prompt. The discriminator is whether the user's custom model is listed as removed. BEFORE ( AFTER ( Scene 2 — re-entering
|
|
Thanks for the PR, @pomelo-nwu! Template looks good ✓ — all required sections present, bilingual description included, test plan with concrete reproduction steps. On direction: This fixes two real, closely related user-facing bugs — silent data loss of custom model IDs in the auth wizard, and spurious "removed" entries in the provider update diff on every restart. Both are the kind of quiet corruption that erodes user trust. Clearly aligned with the project's auth/model-configuration mission. Claude Code's CHANGELOG has analogous fixes for model ID persistence (e.g. Bedrock inference profile IDs, On approach: The scope feels right — one root cause ( One observation for the author to consider: Moving on to code review. 🔍 中文说明感谢 PR，@pomelo-nwu！ 模板完整 ✓ — 所有章节齐全，双语描述，测试计划包含具体复现步骤。 方向： 修复了两个真实的、紧密相关的用户可见 bug — auth 向导中自定义 model ID 静默丢失，以及每次重启都出现虚假的 provider 更新 diff。两者都是侵蚀用户信任的隐性数据损坏。与项目的 auth/model 配置使命明显对齐。Claude Code CHANGELOG 中有类似的 model ID 持久化修复（如 Bedrock inference profile IDs、 方案： 范围合理 — 一个根因（ 一个供作者参考的观察： 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Critical provider custom-model data-loss issues look resolved at the current head. I rechecked auth wizard prefill, provider-update diffing, and update execution preserving user-added custom IDs; no new critical blocker found.
Code ReviewIndependent proposal (before reading diff): Split model IDs into built-in vs custom using Comparison with the PR: Matches and slightly exceeds my proposal. The extraction into core is cleaner than I expected — Previous review's Critical findings — both addressed:
Reuse check: No critical blockers found. Test ResultsUnit Tests (all pass ✓)Real-Scenario Testing (tmux)Interactive mode launched successfully with the test settings ( — Qwen Code · qwen3.7-max |
ReflectionStepping back: this PR fixes a root cause that most users would struggle to diagnose — their custom model IDs silently disappearing from settings, or being trained to ignore "provider update" prompts because every restart shows spurious removals. Both bugs share one conceptual gap ( Going back to my independent proposal from Stage 2 — the PR matches it and improves on it by extracting The previous review's two Critical findings have both been addressed: the 221 unit tests pass across the four affected test files. The tmux interactive test couldn't trigger the provider update prompt (fake API key limitation), but the unit tests directly exercise the diff detection, pre-fill, and update execution paths — the exact behavior the PR claims to fix. CI status: Windows tests still pending, Ubuntu had a failure earlier but the author appears to have pushed fixes. Code-level review is clean regardless. Verdict: Approve. The code is correct, focused, well-tested, and solves real user pain. 中文说明退一步看：这个 PR 修复了一个大多数用户难以诊断的根因 — 自定义 model ID 从设置中静默消失，或者因为每次重启都显示虚假的"移除"而逐渐忽略 provider 更新提示。两个 bug 共享一个概念缺口（ 回到 Stage 2 的独立方案 — PR 匹配并改进了它，将 之前审查的两个 Critical 发现都已解决： 四个受影响测试文件共 221 个单元测试全部通过。tmux 交互测试未能触发 provider 更新提示（假 API key 限制），但单元测试直接覆盖了 diff 检测、预填和更新执行路径 — 正是 PR 声称修复的行为。 CI 状态：Windows 测试仍在进行，Ubuntu 之前有失败但作者似乎推送了修复。代码层面的审查无论 CI 如何都是干净的。 结论：通过。 代码正确、聚焦、测试充分、解决真实用户痛点。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
| // must be carried through so they are not deleted by the | ||
| // prepend-and-remove-owned merge. | ||
| const defaultIds = getDefaultModelIds(providerCfg); | ||
| const customIds = readInstalledOwnedIds(settings, providerCfg).filter( |
There was a problem hiding this comment.
[Suggestion] When a user's custom model ID later becomes a built-in model in a provider template update, this filter silently reclassifies it from customIds to defaultIds. The custom entry's generationConfig (e.g., enable_thinking), baseUrl, and envKey are then discarded — buildModelConfigs rebuilds the entry from the built-in ModelSpec with no trace of the user's original configuration. There is no log or notification.
Consider logging when a previously-custom ID appears in defaultIds, or preserving the original entry's fields when it transitions from custom to built-in.
— qwen3.7-max via Qwen Code /review
| baseUrl: resolved, | ||
| apiKey: '', | ||
| modelIds: getDefaultModelIds(providerCfg), | ||
| modelIds: [...defaultIds, ...customIds], |
There was a problem hiding this comment.
[Suggestion] buildInstallPlan receives only bare string IDs ([...defaultIds, ...customIds]) with no advancedConfig. Inside buildModelConfigs, custom IDs (those not in config.models specMap) are rebuilt from scratch via buildAdvancedGenerationConfig(inputs.advancedConfig), which receives undefined. Any generationConfig the user's saved custom model entry had — extra_body.enable_thinking, contextWindowSize, modalities, samplingParams.max_tokens — is silently dropped.
The existing test (preserves user-added custom models when executing an update) uses a custom model with no generationConfig and asserts only objectContaining({ id: 'my-custom-model' }), so this data loss is not caught.
Fix approach: instead of passing custom IDs as bare strings, read the original ProviderModelConfig entries from settings.merged.modelProviders[protocol] and splice them directly into the install plan's model array after buildInstallPlan returns.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Downgraded from Approve to Comment: CI failing (Post Coverage Comment, Test).
No new findings beyond the existing review threads. The core fix is well-designed — using getDefaultModelIds(config) as the built-in/custom discriminator is the right approach. Typecheck, eslint, and all 221 tests pass clean.
— qwen3.7-max via Qwen Code /review
CI failure analysis — this looks like runner/infra flakiness, not your codeI dug into the failing
(An earlier run, 27966630471, also died at Checkout with Why this is environmental, not this PR:
Suggestion: re-run the 中文CI 失败分析 —— 这是 runner/基础设施抖动，不是你这个 PR 的问题我排查了最新一次运行（run 28061007960，commit
（更早的一次 27966630471 也是挂在 Checkout： 为什么判定为环境问题、与本 PR 无关：
建议： 重跑 |
Update — re-run tally (attempt 5), still CI instability, not this PRFollowing up on the analysis above: the
So blindly re-running isn't clearing it: every attempt that reached the no-AK smoke step (1, 3, 5) failed there on the Separately, heads-up: 中文跟进 —— 重试统计（到第 5 次），依然是 CI 不稳定，不是本 PR接上面的分析：
所以一味重跑解决不了:每一次跑到 no-AK smoke 步骤的（1、3、5）都挂在 另外提醒一下: |
5 participants
What this PR does
A user adds a custom model ID to a Coding Plan / Token Plan provider (e.g.
my-custom-modelalongside the built-inqwen3-coder-plus). Two things break:Re-opening
/authsilently loses the custom model. The Model IDs step always reset to built-in defaults, ignoring what was saved. Completing the wizard overwrote the user's custom entries — silent data loss.A spurious "provider update" diff appears on every restart. The update detector compared all saved model IDs (including user-added ones) against the current built-in list. Since custom models are never in the built-in list, they showed up as "removed" on every launch — noise that trains users to ignore real updates.
Both bugs share one root cause:
ownsModeldetermines which provider a model belongs to, but cannot distinguish built-in defaults from user-added custom models — both pass ownership checks becausebuildModelConfigsstamps custom models with the sameenvKeyandnameprefix as built-in ones.This PR fixes both by splitting built-in vs custom at each call site, using
getDefaultModelIds(config)as the discriminator:AuthDialog.getExistingModelIds: filters out built-in IDs from saved models, passes only custom IDs to the wizard.useProviderSetupFlow: pre-fills[...getDefaultModelIds(config), ...customIds]. TheModelIdsStepcomponent auto-splits — built-in models go to the recommended list (checked), custom models go to the input box.useProviderUpdates.getInstalledOwnedModelIds: filters to only built-in IDs before diffing, so custom models never appear as "removed".findExistingProviderModels(shared core helper, extracted from cli): unchanged — still returns all saved owned models. Callers decide what to exclude.Reviewer Test Plan
How to verify
Pre-seed an isolated settings file with a Coding Plan provider that has one built-in model and one custom model, plus a stale version hash:
Scene 1 — startup diff prompt: custom model is NOT listed
my-custom-test-modeldoes not appear. Before the fix it was listed as "removed" on every restart.Scene 2 —
/authwizard, Model IDs step: custom in input, built-in in recommendedWalk
/auth→ Alibaba ModelStudio → Coding Plan → China → submit API key → reach Model IDs:my-custom-test-model(restored from saved settings, user can edit or remove)qwen3-coder-nextwhich wasn't in the saved settings)Tested on
Risk & Scope
old-deprecated-model), the diff detector can no longer flag it as "removed" because it is indistinguishable from a user-added custom model — both are absent fromgetDefaultModelIds. This is accepted as a rare case; the alternative (flagging every custom model as "removed") is worse.ownsModelresolvable) is unaffected.Linked Issues
Closes #5636
中文说明
这个 PR 做了什么
用户给 Coding Plan / Token Plan provider 添加了自定义 model ID（比如在内置的
qwen3-coder-plus之外加了my-custom-model）。然后出现两个问题：重新打开
/auth时自定义 model 静默丢失。 Model IDs 步骤总是重置为内置默认列表，忽略已保存的内容。走完向导会用默认值覆盖用户的自定义条目——静默数据丢失。每次重启都弹出虚假的"provider 更新"提醒。 更新检测器把所有 saved model IDs（包括用户自定义的）和当前内置列表做 diff。自定义 model 从来不在内置列表里，所以每次启动都显示为"removed"——噪音让用户逐渐忽略真正的更新。
两个 bug 的根因相同：
ownsModel判断的是 model 属于哪个 provider，但无法区分内置默认和用户自定义——两者都通过归属检查，因为buildModelConfigs给自定义 model 打了和内置一样的envKey和nameprefix。本 PR 在每个调用点用
getDefaultModelIds(config)作为判据做拆分：AuthDialog.getExistingModelIds：从 saved models 中过滤掉内置 ID，只传自定义 ID 给 wizarduseProviderSetupFlow：预填[...getDefaultModelIds(config), ...customIds]。ModelIdsStep组件自动拆分——内置去 recommended 列表（选中），自定义去 input 框useProviderUpdates.getInstalledOwnedModelIds：过滤为只含内置 ID 再做 diff，自定义 model 不会出现为 "removed"findExistingProviderModels（共享 core helper，从 cli 提取）：不变——仍返回全部 saved owned models，由调用方决定排除什么评审测试计划
如何验证
预置隔离 settings 文件，包含一个内置 model + 一个自定义 model + 旧 version hash：
场景一 — 启动 diff 提醒：自定义 model 不在列表中
my-custom-test-model未出现。修复前每次启动都会被列为 "removed"。场景二 —
/auth向导 Model IDs 步骤：自定义在 input 框，内置在 recommended 列表my-custom-test-model（从 saved 恢复，可编辑或删除）qwen3-coder-next也出现了）风险与范围
getDefaultModelIds里。这个罕见场景被接受；替代方案（每次把自定义 model 标为 "removed"）更差。关联 Issue
Closes #5636