fix(core): compute stored provider version from built-in template so 'update all' clears the prompt - #8830
fix(core): compute stored provider version from built-in template so 'update all' clears the prompt#8830qwen-code-dev-bot wants to merge 7 commits into
Conversation
…'update all' clears the prompt For editable providers (e.g. Token Plan, modelsEditable=true), the stored providerMetadata version was computed from the INSTALLED model list (default + user custom models), while the launch-time check recomputes currentVersion from the built-in template (default models only). With any custom model present the two hashes never match, so choosing "update all" never cleared the Built-in Provider Update prompt — it reappeared on every launch. Compute the stored version from buildProviderTemplate (default models only, same baseUrl) so it uses the identical input as the launch-time computeModelListVersion(buildProviderTemplate(...)) check. Custom models no longer affect the version hash, and a successful update now makes metadata.version === currentVersion on next launch.
|
Re-run note: the head moved while this pass was running — Template: substance fully covered ✓ — what/why with a confirmed root cause, reviewer test plan, Tested-on table, risk & scope, Chinese summary. Same two non-blocking deviations as before (no literal Problem: observed, not theoretical. Tracked in #8504 with a v0.21.4 reproduction, and now demonstrated end-to-end by wenshao's four-bundle real-build A/B (base reproduces the re-prompt loop on the actual Direction: aligned — a provider-update bugfix squarely in scope. No escalation triggers. Size: core + cli + vscode-ide-companion paths. 219 production lines (+200/−19) vs 516 test lines (+514/−2), 0 generated/schema — far below every threshold ( Approach: the shape at this head is the right one — a single classifier ( Risk: one Stage 1e match — Moving on to code review. 🔍 中文说明重跑说明：本次审查期间 head 发生变化 —— 开始审查时为 模板：实质内容齐全 ✓。与上轮相同的两处小偏差（不阻塞）：没有字面的 问题：已观测到的 bug。#8504 有 v0.21.4 复现；wenshao 用四个真实 bundle 的 A/B 已在真实改名时间线上完整演示（base 复现循环弹窗，head 清除）。代码中写入/读取 hash 不对称的机制也已复核。 方向：对齐 —— provider 更新流程的 bugfix，核心范围内，无升级触发条件。 规模：触及 core + cli + vscode-ide-companion。生产代码 219 行（+200/−19），测试 516 行（+514/−2），生成/schema 0 行 —— 远低于所有阈值（ 方案：当前形态正确 —— core 中单一分类函数（ 风险：命中一条 Stage 1e —— 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewIndependent proposal before reading the diff: with two writers (CLI install/update, VS Code sync) and four readers (launch check, update confirm, Verified against the code at this head:
No new Critical findings from this pass. What remains open is from the maintainer's 12:14 pass — unaddressed at this head, no reply yet, and it is product judgment rather than a code defect I can settle from the diff:
Files changed (15)
Testing evidenceThe PR's own CI at the reviewed commit, fetched once via the API (per the gate rules I do not run PR code). At review time Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Two prior evidence sources cover the previous head, not this one — attributing them precisely: the sandboxed Sandboxed verification would settle this: a re-run of 中文说明代码审查：读 diff 前的独立方案是——同一份 metadata 有两个写入方（CLI 安装/更新、VS Code 同步）和四个读取方（启动检查、更新确认、 已核实：写入/读取对称 —— 未解决项来自维护者 12:14 一轮 —— 本 head 未处理、暂无回复，属产品判断而非可从 diff 裁定的代码缺陷：1）"Skip this version" 之后重跑 测试证据：上表取自该 commit 的 PR 自身 CI（审查时 Qwen Code CI 的 Linux 单测仍在运行，finalize 会就地更新表格；macOS/Windows/集成为 skipped）。需要精确区分两份既有证据：沙箱 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — the fix itself is proven and the review-driven hardening landed cleanly, but three open maintainer threads on re-entry behavior and a dismissed approval need a human call before anyone approves. Stepping back: this PR has genuinely matured across its rounds. The core asymmetry (stored version hashed over installed models vs launch check hashed over the template) is fixed with a single owned function; the stale-built-in cleanup that grew out of round 1 now has one classifier shared by all four entry points; and the round-2 commit fixed exactly the regression four reviewers converged on — the VS Code sync carrying a stale Why I'm not approving:
⏸️ Deferring to @wenshao — needs a human call on two questions: (a) do your skip-version/ACP threads block this PR, or are they follow-up material given the core fix is verified? (b) your approval was dismissed by the 12:45:48 push — if it still reflects your intent after 中文说明总体看：这个 PR 在多轮评审中确实成熟了。核心不对称（存储 version 按已安装模型哈希、启动检查按模板哈希）已由单一归属函数修复；第一轮长出来的过期内置清理现在由四个入口共享的单一分类函数支撑；第二轮提交恰好修复了四位评审者共同点名的回归 —— VS Code 同步跨区域切换时携带过期 baseUrl/builtinIds —— 并配了跨区域测试。当前 head 的测试具备判别性（第二轮存活的回退突变体现已全部被杀）。 不批准的原因：1）未解决的线索是行为层面的，且出自维护者本人 —— wenshao 12:14 提出的 ⏸️ 转交 @wenshao —— 需要人工裁定两个问题：(a) skip-version/ACP 线索是否阻塞本 PR，还是在核心修复已验证的前提下留作后续？(b) 你的批准已被 12:45:48 的推送 dismiss —— 如果对 — Qwen Code · qwen3.8-max Reviewed at |
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. |
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 — CI landed green after the review. ✅
Record the built-in model ids at install time (metadata.builtinIds). On a later update, a model that is installed+owned but absent from the current default list is only preserved as "custom" if it was NOT in the previous built-in list. A model that WAS a built-in at last install but is now renamed/removed upstream (e.g. deepseek-v4-flash -> deepseek-v4-flash-0731, commit 7612725) is treated as stale and dropped, so it gets cleaned up instead of persisting forever as a phantom "custom" model. True user-added custom models (never in any built-in list) are still carried through, preserving the original intent of the customIds logic.
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x), web-shell E2E Smoke (ubuntu-latest, Node 22.x), Post Coverage Comment (ubuntu-latest, 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x), web-shell E2E Smoke (ubuntu-latest, Node 22.x), Post Coverage Comment (ubuntu-latest, 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
[Critical] 本 PR 导致 tsc --build 失败，Test (ubuntu-latest, Node 22.x) / web-shell E2E Smoke / Post Coverage Comment 三个 check 全红，阻塞合并。
位置：packages/core/src/providers/provider-config.ts:250 写入 builtinIds: getDefaultModelIds(config)（string[]），但该函数的返回类型 ProviderInstallState 在 packages/core/src/providers/types.ts:161 定义为 Record<string, Record<string, string>>，值只允许 string。该类型定义不在本 PR 改动范围内，且在 main 上早已存在（最近一次改动是 #7607），因此不是 base skew。
触发条件：npm run build --workspace=packages/core（CI 的 Install dependencies 步骤会跑 scripts/prepare.js → tsc --build）。
影响：当前 head a1faea17（已合并 main 之后）的 run 31358772460 在依赖安装阶段就失败：
src/providers/provider-config.ts(243,5): error TS2322: Type ... is not assignable to type 'ProviderInstallState'.
'string' index signatures are incompatible.
Property 'builtinIds' is incompatible with index signature.
Type 'string[]' is not assignable to type 'string'.
编译产物出不来，后续三个 job 连带全红。注意 #8830 的“Base updated: red check(s) pass on current main”判断在这里不成立 —— 合并 main 后的 head 上这三个 check 依旧红，原因是本 PR 自己的 38569c96。
修复方向：放宽 ProviderInstallState 的值类型（如 Record<string, string | string[]>，或为 provider metadata 定义专门的 interface），并同步确认 packages/core/src/providers/install.ts:275-278 的 settings.setValue(${key}.${field}, value) 在 value 为数组时的类型与落盘行为。消费侧 packages/cli/src/ui/hooks/useProviderUpdates.ts:62 已按 builtinIds?: string[] 声明，运行时语义是自洽的，只需要把写入侧的类型对齐。
|
🤖 Addressed the latest review feedback (round 1/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/10 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #8830Feedback points and decisions[Critical] @qqqys (CHANGES_REQUESTED): Reproduced locally first: Changes (all minimal, root-cause):
No conflict resolution was needed ( VerificationCommands actually run on the final tree (commit
Integration tests after The three red CI checks ( 中文说明Autofix 评审轮次 — PR #8830反馈点与处理决定[Critical] @qqqys（CHANGES_REQUESTED）： 先在本地复现：在 改动内容（均为最小化、根因级修复）：
无需解决冲突（ 验证以下命令均在最终代码树（提交
未运行 三个红色 CI check（ Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审；第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
— qwen3.8-max via Qwen Code /review (v0.21.8)
… built-in cleanup (#8830) - Show stale built-in removals in the update prompt diff so the preview matches what confirm actually does (removed lines, current-model warning, fallback model). - Apply the same stale built-in classification to the /auth wizard prefill and the ACP providers/list serialization so neither path reinstalls ids the update cleans up. - Emit an info item listing dropped outdated built-in ids after update. - Preserve CLI-recorded providerMetadata fields (builtinIds, baseUrl, ignoredVersion) when the VS Code companion re-syncs coding plan settings instead of replacing the whole object. - Pin the template-based stored version with an assertion in the test that carries an owned custom model through the update.
|
🤖 Addressed the latest review feedback (round 2/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/10 轮）。改动内容与我反驳保留之处如下： Review feedback addressed (round: PR #8830)One commit: Findings and dispositions
Note on the CHANGES_REQUESTED review stateThe review was marked CHANGES_REQUESTED because "Integration Tests (CLI, No Sandbox)" were skipped in CI and not run locally. No integration spec exercises the changed paths — Pre-existing failures observed on this runner (not caused by this change)
VerificationCommands actually run on the final tree (commit
中文说明评审意见处理（轮次：PR #8830）一次提交： 意见与处置
关于 CHANGES_REQUESTED 评审状态的说明评审被标记为 CHANGES_REQUESTED 的原因是"Integration Tests (CLI, No Sandbox)"在 CI 被跳过且本地未运行。没有任何集成测试覆盖本次改动的路径—— 本 runner 上观察到的既有失败（非本次改动引入）
验证在最终代码树（提交
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审；第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /verify |
|
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: 809 passed · 0 failed · 809 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：809 通过 · 0 失败 · 809 总计 Verification reportPR #8830 deep verification —
|
| Cell | Fixture (settings as a previous install left it) | BASE (HEAD^1) |
HEAD (01f27b1d) |
|---|---|---|---|
| L1 control | installed = template, stale version | prompt clears; stored = template version | same (bug needs customs) |
| L2 custom | template + my-custom-model |
re-prompts; stored version = hash(template+custom) ≠ template version | clears; custom kept |
| L3 recorded stale | template + stale + custom, builtinIds incl. stale |
re-prompts; stale kept; preview hides removal | clears; stale dropped; custom kept; info item; preview shows removal |
| L3p current=stale | as L3, active model is the stale id | currentModelAffected=false |
removed ∋ stale, fallback = first default |
| L4 migration | stale + custom, no builtinIds |
re-prompts; stale kept | clears; stale kept (documented fallback); recording lands |
| L5 / L5b malformed | builtinIds = string / mixed array |
no crash, stale kept | validated ([] / strings kept), clears |
Both arms 7/7 (base reds are the predicted bug behavior, asserted green in the harness). Witness captures: 01-ab-lifecycle-head-update-clears-prompt.png, 02-ab-lifecycle-base-bug-reproduces.png.
Core-level probe on compiled output (harness/core-probe.mjs, HEAD dist): 17/17 — with a custom model, providerState version == computeModelListVersion(buildProviderTemplate(...)) for both coding-plan and token-plan; builtinIds recorded; readRecordedBuiltinIds boundary ladder (missing key, string, mixed array, scalar/null/array record, number) all []/filtered as specified.
PR's own tests as cross-arm cells: useProviderUpdates.test.ts on base = 17 pass + exactly the 2 new stale-builtin tests red on the intended assertions (expected [] to include 'renamed-away-model'; expected […] to not include 'renamed-away-model') — capture 03-pr-tests-on-base-red-by-design.png; on head 19/19. Companion settingsWriter.test.ts: head 18/18; base 17/18 with the merge test red (expected undefined to be 'https://coding.dashscope…' — base wiped baseUrl). ACP providers/list new test: head green (396/396 file); base red with modelIds still containing deepseek-renamed-away.
Mutation matrix (scratch edits on HEAD, reverted; capture 04-mutation-m2-killed-by-intended-assertion.png):
| Mutant | Suite | Result |
|---|---|---|
| M1 version from installed models (base semantics) | useProviderUpdates.test.ts | killed by the setValue(…version, chinaVersion) assertion (1 red / 18 green) |
| M1 | lifecycle harness | killed by 4 prompt-clearing cells; L1 control survives (bug needs customs) |
M2 drop !previousBuiltinIds.includes(id) clause |
useProviderUpdates.test.ts | killed by not.toContain(staleBuiltinId) (1 red / 18 green) |
| M3 companion metadata replace-instead-of-merge | settingsWriter.test.ts | killed by the merge test (1 red / 17 green) |
| M4 invert version-match early return (positive control) | useProviderUpdates.test.ts | killed by the existing "versions match" test |
Findings (non-blocking)
- Migration boundary — pre-recording installs never get the stale model cleaned up. Cell L4 proves: with metadata lacking
builtinIds(every install predating this PR), the update clears the prompt but keeps the phantom model as "custom" forever, becausebuiltinIdsrecorded at that update contains only current defaults — the stale id was already gone. This matches the PR's own Risk & Scope disclosure, so it is an accepted tradeoff, not a defect; but the body's manual step "stale id is removed" is only reachable with a recorded fixture, and the reported real-world cohort (installs before thedeepseek-v4-flashrename) keeps the phantom. Suggested doc-level fix: state in the PR body that cleanup applies from the first recorded install onward. - AuthDialog prefill regression coverage is CI-skipped. The new test uses the pre-existing
itWhenTuiInputReliableguard (CI=true→ skip), so in CI mode the prefill classification is unasserted. Verified the test passes when run outside CI (25 pass / 1 environmental fail, see Not covered). Suggestion: extract the pure prefill filter into a unit-testable helper so the classification is pinned in CI. - Dead fallback:
metadata.builtinIds ?? []infindAllPendingUpdatescan never seeundefined—getProviderMetadataalways returns a validated array. Cosmetic.
Not covered
- Per-commit attribution: shallow depth-2 checkout; only
HEAD^2of the 5 commits in the snapshot is locally reachable (git rev-list HEAD^1..HEAD^2= 1). Verified the aggregateHEAD^1..HEADdiff only. - Base arm of the core dist probe:
tsc --buildin a worktree fails identically atHEAD^1andHEAD^2(A/A control,logs/head2-core-build.log— same 16-error signature:@lydell/node-ptyTS7016,mime/lite,fdir,ignoretypings) — environmental (worktrees lack the root tree's resolution state), not PR-attributable. Vitest source-level cells (real TS via alias) stand in for the base arm. - AuthDialog TUI navigation test (
drives API key provider steps…) fails in this container withCIunset, identically on base (A/A) — pre-existing environmental flake, which is exactly what theCI=trueskip guard encodes; excluded from counts. CI-mode gate run (the repo's own contract) is green: 26 pass / 19 skipped. - Real-terminal TUI flow of the prompt (hook-level
renderHookharness used instead); the real upstream rename timeline (rename synthesized in fixtures — reproduces the wire/settings shape, not the model-side event); repo-wide suites beyond the three touched workspaces.
Methodology
Environment: node:22-bookworm container, merge-ref checkout at depth 2 (HEAD=merge, HEAD^1=base, HEAD^2=verified head). Harnesses drove compiled/source code mock-free with respect to the units under test: the lifecycle harness renders the real hook and executes the real applyProviderInstallPlan (only settingsUtils backup/restore mocked, mirroring the PR's own tests), recording every settings write and remounting to observe the re-prompt; the core probe imports the built dist of the head tree. Base control = worktree at HEAD^1 with vitest aliases pinning @qwen-code/qwen-code-core to base sources (verified by behavior flip + grep), third-party-only package node_modules symlinked (no internal @qwen-code links exist nested — asserted). Counts are test-level scripted assertions; predicted base/mutant reds count as passes per the A/B contract; the CI-unset AuthDialog probe is excluded as environmental (A/A-proven). Raw logs in logs/; harnesses rerunnable from harness/ (mutation-m2.sh, gates-summary.sh, core-probe.mjs, lifecycle test). Gate summary capture: 05-targeted-gates-head.png (core providers 149/149; cli hook+AuthDialog 26 pass/19 skip; acpAgent 396/396; companion 18/18; dist probe 17/0; plus tsc --noEmit green on core+cli and check-types green on the companion).
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
The cross-entry-point cleanup mostly lines up. I found one metadata-coherence issue on VS Code region changes, inline.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): PR #8830 (round-2 re-review): computes the stored provide...: did not verify whether any desktop code path persists existingConfig.modelIds across sessions (e.g. workspace storage) rather than re-fetching the catalog — i….
中文说明
未探索到全部深度（达到工具调用预算）：PR #8830 (round-2 re-review): computes the stored provide...：did not verify whether any desktop code path persists existingConfig.modelIds across sessions (e.g. workspace storage) rather than re-fetching the catalog — i…。
— qwen3.8-max via Qwen Code /review (v0.21.8)
Maintainer verification — real local build, real TUI, real ACP wireI built this PR and its merge base into four real CLI bundles and drove the actual TUI (tmux, isolated Verdict: works as advertised. Both fixes are real, the blast radius is what the PR says, and every changed production line is pinned by a test. Recommend merge. One caveat worth putting in the release notes (below). Setup — four bundles, four scenarios
The Each run: 1. The bug reproduces before any upstream renameOn
Same install with 2. Upgrade across the real rename — 2×2 matrixInstall with
A vs D isolates the code from the data: D has the exact same settings as B (
The cleanup is also surfaced to the user, not silent:
Stored version hashes (
B and C hold different model lists yet converge on the same hash — the stored version now depends only on the built-in template, exactly as the launch check recomputes it. 3. Re-entry paths (the parts beyond the two headline fixes)
ACP 4. Tests, mutation coverage, gatesTargeted suites on head — all green:
Mutation matrix — I reverted each production hunk one at a time on head and re-ran the relevant suite. Every one is killed, so no line here is untested:
5. Notes for the merge decision
All 18 captured framesHosted at 中文版维护者本地验证 —— 真实构建、真实 TUI、真实 ACP 线协议我把本 PR 和它的 merge base 各构建成真实的 CLI bundle（共 4 个），用 tmux 驱动真实 TUI（独立 结论：两个修复都成立，影响面与 PR 描述一致，改动的每一行生产代码都有测试兜住。建议合入。 有一条建议写进 release note（见下）。 环境
1. 不需要任何上游改名，bug 就已经复现在 2. 真实改名升级的 2×2 矩阵
D 和 B 用完全相同的数据（都带 版本哈希（ 清理动作对用户也是可见的，不是静默的： 3. 两条"再入口"
4. 测试与变异覆盖针对性套件在 head 全绿：core 96 通过、
变异矩阵：逐条 revert 生产代码改动再跑对应套件，7 条变异全部被杀（M1 版本改回按已安装列表算、M2 不再记录 5. 给合入决策的几点
|
|
@qwen-code /triage |
|
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: 1041 passed · 0 failed · 1041 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：1041 通过 · 0 失败 · 1041 总计 Verification report<!-- qwen-triage:verify --> Sandboxed verification: ✅ passed — merge-ready (agent verdict) - follow-up round at an unchanged head 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: 1041 passed · 0 failed · 1041 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：1041 通过 · 0 失败 · 1041 总计 说明：本轮是 follow-up round，但 HEAD（merge Verification reportPR #8830 deep verification — follow-up round,
|
| # | Finding (previous round) | Severity | Status at 01f27b1d |
Re-measurement this round |
|---|---|---|---|---|
| F1 | Migration boundary — installs predating builtinIds recording keep the phantom model as "custom" forever |
non-blocking, accepted tradeoff | stands | Cells L4/L5 re-run: head clears the prompt but staleKept=true; the recording written at that update contains only current defaults, so later updates still classify the stale id as custom. Matches the PR's Risk & Scope disclosure. |
| F2 | AuthDialog prefill regression coverage is CI-skipped (itWhenTuiInputReliable) |
non-blocking | stands | Head gate in CI mode: 19 skipped incl. the new prefill test. With CI unset: head 25 passed incl. prefill green; base red on the intended assertion (not.toContain('renamed-away-model')); the one failing test (drives API key provider steps…) fails identically on both arms (A/A) — environmental, exactly the regime the guard encodes. |
| F3 | Dead fallback metadata.builtinIds ?? [] in findAllPendingUpdates (and the twin in executeUpdate) |
cosmetic | stands | Code read at head: getProviderMetadata always returns a validated array, so both ?? [] operands are unreachable. |
Agree with all three previous classifications; none blocks the merge.
Central claim + A/B
Central claim: after "update all" on an editable built-in provider, the stored providerMetadata.<key>.version equals the launch-time currentVersion (computed from the built-in template), so the Built-in Provider Update prompt clears and stays cleared — even with user-added custom models; recorded stale built-ins (renamed/removed upstream) are dropped while true customs survive; and the preview/re-entry paths (update prompt diff, /auth prefill, ACP providers/list) agree with what confirm actually does.
A/B harness: harness/pr8830-lifecycle.harness.test.ts (identical file on both arms; arm expectations encoded via VERIFY_ARM), driving the real useProviderUpdates hook + real core install logic (buildInstallPlan/applyProviderInstallPlan) with a write-recording settings adapter; after confirm, every recorded setValue write is replayed into a fresh merged view and the hook is remounted to observe whether the prompt reappears. Base control: git worktree at HEAD^1; the cli vitest alias resolves @qwen-code/qwen-code-core to the base tree's own source (grep -c readRecordedBuiltinIds = 0 in base core); package-level node_modules symlinked in after asserting they contain no @qwen-code scope entries (third-party only).
| Cell | Fixture (settings as a previous install left them) | BASE (77bd04bd) |
HEAD (01f27b1d) |
|---|---|---|---|
| L1 control | installed = template, stale version | clears (bug needs customs) | clears; builtinIds recorded |
| L2 custom | template + my-custom-model |
re-prompts; stored version = hash(template+custom) ≠ template version | clears; custom kept |
| L3 recorded stale | template + stale + custom, builtinIds incl. stale |
re-prompts; stale kept; preview hides removal | clears; stale dropped; custom kept; info item; preview shows removal |
| L3p current=stale | as L3, active model is the stale id | currentModelAffected=false (silent) |
removed ∋ stale, currentModelAffected=true, fallback = first default, model.name written |
| L4 migration | stale + custom, no builtinIds |
re-prompts; stale kept | clears; stale kept (documented fallback); recording lands |
| L5 malformed string | builtinIds = 'not-an-array' |
re-prompts; stale kept | validated to [] → behaves like L4, clears |
| L5b malformed mixed | builtinIds = [stale, 42, null] |
re-prompts; stale kept | string entries survive validation → stale dropped, clears |
Both arms 7/7 cells = 77/77 named checks (base reds are the predicted bug behavior, asserted green in the harness). Witness captures: 01-ab-lifecycle-head-update-clears-prompt.png, 02-ab-lifecycle-base-bug-reproduces.png.
Core-level probe on compiled output (harness/core-probe.mjs, HEAD dist): 32/32 — with a custom model present, buildInstallPlan.providerState version == computeModelListVersion(buildProviderTemplate(...)) and != hash(installed list) for coding-plan-china, token-plan-china and token-plan-global; builtinIds recorded; readRecordedBuiltinIds boundary ladder (missing key/ns, string, mixed array, scalar/null/array record, number, nested non-strings) all []/filtered as specified.
PR's own tests as cross-arm cells (capture 03-pr-tests-on-base-red-by-design.png): on base, useProviderUpdates.test.ts = 17 pass + exactly the 2 new stale-builtin tests red on the intended assertions (expected [] to include 'renamed-away-model'; expected […] to not include 'renamed-away-model'); settingsWriter.test.ts = 17 pass + merge test red (expected undefined to be 'https://coding.dashscope…'); the new ACP providers/list test red with modelIds still containing deepseek-renamed-away (clients echoing it back would reinstall what the update cleans up); install.test.ts green on both arms by construction (it pins the unchanged array-value write loop, not the recording). On head: hook 19/19, companion 18/18, acpAgent 396/396, core provider suites 96/96 (capture 05-targeted-gates-head.png).
Mutation matrix at HEAD (scratch edits, applied then restored byte-exact; capture 04-mutation-matrix-all-mutants-killed-or-classified.png):
| Mutant | Suite | Result |
|---|---|---|
| M1 stored version from installed models (base semantics) | useProviderUpdates.test.ts | killed by the setValue(…version, chinaVersion) assertion in the new stale-drop test (1 red / 18 green) |
| M1 | core provider suites (4 files) | survives (96/96) — classified coverage gap, not dead code: no core fixture carries a custom model through buildInstallPlan; the axis is pinned by the hook suite, the lifecycle cells, and the dist probe's stored != installed-hash check |
M2 drop !previousBuiltinIds.includes(id) from the customIds filter |
useProviderUpdates.test.ts | killed by not.toContain(staleBuiltinId) (1 red / 18 green) |
| M3 companion replace-instead-of-merge | settingsWriter.test.ts | killed by the merge test (1 red / 17 green) |
| M4 invert version-match early return (positive control) | useProviderUpdates.test.ts | killed wholesale (17 red / 2 green) |
M5 readRecordedBuiltinIds always [] |
provider-config.test.ts | killed by 2 of the 3 new unit tests (2 red / 66 green) |
Findings (non-blocking)
- F1 stands — migration boundary. Users whose install predates
builtinIdsrecording get the prompt cleared but keep the phantom model forever (cell L4). Accepted tradeoff, disclosed in the PR's Risk & Scope; the body's manual step "stale id is removed" is only reachable with a recorded fixture. Suggested doc-level fix unchanged: state that cleanup applies from the first recorded install onward. - F2 stands — AuthDialog prefill classification is unasserted in CI. The new TUI test is skipped under
CI=true(this container's default and the repo's CI contract). Verified it passes outside CI and is red-by-design on base; the co-failingdrives API key provider steps…test is environmental (A/A-identical). Suggestion unchanged: extract the pure prefill filter into a unit-testable helper. - F3 stands — dead
?? []at bothbuiltinIdsread sites. Cosmetic. - New (cosmetic): the PR body's "104/104 pass" for Reviewer Test Plan step 1 is stale. At this head the three named files run 68 + 22 + 19 = 109 tests, all passing. The count predates the last commit's added assertions; the command itself is green.
Observation (not a PR defect): the hook's executeUpdate/checkForUpdates dependency arrays include addItem; my first harness version passed an inline vi.fn() per render and hit an infinite re-prompt loop on base only (a pending update keeps firing setUpdateRequest, and head's fix leaves no pending update, so the loop is latent there). The dependency arrays are unchanged by this PR (identical at base), and the real call site (AppContainer.tsx) passes historyManager.addItem, a useCallback-memoized stable reference — so production cannot reach the loop. Recorded so future harness authors hoist their mocks.
Not covered
- Per-commit attribution: shallow depth-2 checkout;
git rev-list --count HEAD^1..HEAD^2= 1 while the snapshot lists 5 commits; verified the aggregateHEAD^1..HEADdiff only. - AuthDialog runs with
CIunset are excluded from the assertion counts (one environmental flake, A/A-proven); they are reported above as evidence, not counted. - Real-terminal TUI flow of the prompt (hook-level
renderHookharness used instead); the real upstream rename timeline (rename synthesized in fixtures — reproduces the settings shape, not the model-side event); repo-wide suites beyond the touched workspaces; base-arm compiled-dist build (vitest source-level alias stands in, as in the previous round's A/A-proven environmental failure).
Methodology
Environment: node:22-bookworm container, merge-ref checkout at depth 2 (HEAD=merge ffea5be0e4, HEAD^1=base 77bd04bd61, HEAD^2=verified head 01f27b1d); npm ci + npm run build pre-run at HEAD. Harnesses drove code mock-free with respect to the units under test: the lifecycle harness renders the real hook and executes the real applyProviderInstallPlan (only settingsUtils backup/restore mocked, mirroring the PR's own tests), records every settings write, replays them into a fresh merged view and remounts to observe the re-prompt; the core probe imports the built dist of the head tree. Base control = worktree at HEAD^1 with vitest aliases pinning @qwen-code/qwen-code-core to base sources (verified by grep + behavior flip), third-party-only package node_modules symlinked (no internal @qwen-code links — asserted). Counts are named-check/test-level scripted assertions; predicted base/mutant reds count as passes per the A/B contract; skipped and environmentally-excluded tests are not counted. Raw logs in logs/; harnesses rerunnable from harness/ (pr8830-lifecycle.harness.test.ts, core-probe.mjs, mutations.sh). Gate summary capture: 05-targeted-gates-head.png (core providers 96/96; hook 19/19; AuthDialog 26 pass/19 skip; acpAgent 396/396; companion 18/18; tsc --noEmit exit 0).
Evidence images
— Qwen Code · sandboxed verification
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 3/10) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31387091506 🧠 Handled by Qwen Code · model/模型 |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
中文说明
— DeepSeek/deepseek-v4-pro via Qwen Code /review (v0.21.8)
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31394772225 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 5/10) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31395261378 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix stopped: this counting window now contains 3 time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is 3 full agent runs that pushed nothing. 3 of those were silent-sandbox (idle) timeouts that no budget increase can cure — investigate the sandbox image and runner docker daemon for those. A human should investigate the sandbox image and runner docker daemon, then comment What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31399630023 🧠 Handled by Qwen Code · model/模型 |
|
⏸️ AutoFix paused: this PR reached its automatic round cap (10/10) and the loop will not manage it further — new feedback and base conflicts stay unhandled. Comment 中文说明⏸️ AutoFix 已暂停：本 PR 达到自动轮次上限（10/10），循环不再管理——新反馈与 base 冲突将无人处理。评论 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget (~24 of ~57 calls).; PR #8830 computes the stored provider metadata version fr...: did not execute the new test suites — the review worktree has no node_modules (root or per-package) and a full monorepo npm install + build was outside this…; PR #8830 computes the stored provider metadata version fr...: run the affected vitest suites ( useProviderUpdates.test.ts , install.test.ts , provider-config.test.ts , acpAgent.test.ts , AuthDialog.test.tsx , settings….
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget (~24 of ~57 calls).；PR #8830 computes the stored provider metadata version fr...：did not execute the new test suites — the review worktree has no node_modules (root or per-package) and a full monorepo npm install + build was outside this…；PR #8830 computes the stored provider metadata version fr...：run the affected vitest suites ( useProviderUpdates.test.ts , install.test.ts , provider-config.test.ts , acpAgent.test.ts , AuthDialog.test.tsx , settings…。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // Settings are hand-editable — validate builtinIds before any removal | ||
| // decision relies on it. | ||
| return { ...record, builtinIds: readRecordedBuiltinIds(metadataKey, ns) }; |
There was a problem hiding this comment.
[Suggestion] The newly added ProviderMetadata.builtinIds field is populated and validated here, but no code ever reads it. The sole caller (findAllPendingUpdates) consumes only metadata.version / .baseUrl / .ignoredVersion; both removal decisions — the preview diff (~L226) and the confirm path in executeUpdate (~L277) — call getStaleBuiltinIds(provider, providerMetadataNs) on the raw namespace, which re-reads and re-validates the record independently. The comment claims removal decisions rely on this validation; they do not (AGENTS.md: "for every added field … grep its read sites").
Concrete cost: a maintainer fixing a stale-removal bug follows this comment, hardens the validated copy, and observes zero behavior change because the decision path reads a different source; the two validation sites can also drift silently when one is changed without the other, and the record is parsed twice per provider per launch scan for nothing.
Suggested fix — drop the field, or make it load-bearing:
// Option A: return record unchanged, drop `builtinIds?: string[]` from
// ProviderMetadata and the now-unused readRecordedBuiltinIds import.
return record;
// Option B: derive the stale set in findAllPendingUpdates / executeUpdate
// from metadata.builtinIds instead of re-reading providerMetadataNs.中文说明
新增的 ProviderMetadata.builtinIds 字段在这里被填充并校验，但没有任何代码读取它。唯一调用方（findAllPendingUpdates）只消费 metadata.version / .baseUrl / .ignoredVersion；两处移除决策——预览 diff（约 L226）和 executeUpdate 的确认路径（约 L277）——都对原始命名空间调用 getStaleBuiltinIds(provider, providerMetadataNs)，其内部会独立地重新读取并校验该记录。注释声称移除决策依赖这里的校验，实际并非如此（AGENTS.md："对每个新增字段……grep 其读取位置"）。
具体代价：维护者修复 stale 移除问题时按此注释加固这份校验副本，不会观察到任何行为变化，因为决策路径读的是另一个来源；两处校验点还可能在一处被修改而另一处未动时悄悄漂移，而且每次启动扫描都会对每个 provider 白白多解析一次该记录。
建议修复：删掉该字段（return record;，同时从 ProviderMetadata 移除 builtinIds?: string[] 及不再使用的 readRecordedBuiltinIds import），或让 findAllPendingUpdates / executeUpdate 改为消费 metadata.builtinIds，使其真正生效。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const staleBuiltinIds = new Set( | ||
| getStaleBuiltinIds( | ||
| providerConfig, | ||
| (settings.merged as Record<string, unknown>)[PROVIDER_METADATA_NS] as |
There was a problem hiding this comment.
[Suggestion] The "extract the providerMetadata namespace from settings.merged → classify via getStaleBuiltinIds" block is hand-assembled four times in this PR: here, in acpAgent.ts readExistingProviderConfig (~L2093), and twice in useProviderUpdates.ts (findAllPendingUpdates ~L206 and executeUpdate ~L277). The classification itself is centralized in core's getStaleBuiltinIds, but the namespace extraction feeding it is not — despite that function's docstring ("Every entry point … must derive from this function so the classification cannot drift between copies").
Concrete cost: any future change to how the namespace is reached (merged-settings typing, a namespace move, or a malformation guard like the !Array.isArray check the companion's settingsWriter.ts already applies) must be made in four places; missing one makes that entry point (ACP providers/list, /auth prefill, update preview, update execute) classify stale built-ins differently from the others — precisely the cross-entry-point drift the shared helper exists to prevent.
Suggested fix — one small CLI-side helper used by all four sites:
function getStaleBuiltinIdsForSettings(
provider: ProviderConfig,
settings: LoadedSettings,
): Set<string> {
return new Set(
getStaleBuiltinIds(
provider,
(settings.merged as Record<string, unknown>)[PROVIDER_METADATA_NS] as
| Record<string, unknown>
| undefined,
),
);
}中文说明
"从 settings.merged 提取 providerMetadata 命名空间 → 调用 getStaleBuiltinIds 分类"这段代码在本 PR 中手工拼装了四处：此处、acpAgent.ts 的 readExistingProviderConfig（约 L2093）、以及 useProviderUpdates.ts 的两处（findAllPendingUpdates 约 L206 与 executeUpdate 约 L277）。分类本身已集中在 core 的 getStaleBuiltinIds，但喂给它的命名空间提取没有集中——尽管该函数的 docstring 写着"每个入口……都必须经由本函数派生，使分类不会在副本之间漂移"。
具体代价：未来任何对命名空间获取方式的改动（merged-settings 类型、命名空间迁移、或像 companion settingsWriter.ts 已应用的 !Array.isArray 这类畸形防护）都必须在四处同时修改；漏掉一处就会让该入口（ACP providers/list、/auth 预填、更新预览、更新执行）对过期内置模型的分类与其他入口不一致——正是共享 helper 要防止的跨入口漂移。
建议修复：抽出一个 CLI 侧小 helper（如 getStaleBuiltinIdsForSettings(provider, settings)，包含类型转换 + getStaleBuiltinIds + Set 包装），四处统一调用。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| version: computeProviderTemplateVersion(config, baseUrl), | ||
| baseUrl, |
There was a problem hiding this comment.
[Suggestion] The template-version stamp is applied globally in resolveProviderState (every install path), which deviates from the fix direction stated in target issue #8504's thread — "the version correction should only happen on the update path, not globally in resolveProviderState — hashing the full built-in template unconditionally at install time would silently suppress the prompt for users who deliberately installed a subset of built-in models" (@doudouOUC, 2026-08-04). The deviation is not acknowledged in the PR's Risk & Scope.
Failure scenario: 11 presets set modelsEditable: true, and the /auth wizard's Model IDs step is a free-text input prefilled with the defaults, so a user who deletes one built-in id flows through buildInstallPlan → resolveProviderState and stores the FULL template hash. The launch check then sees metadata.version === currentVersion and never prompts for the deliberately-missing built-ins — pre-PR that user was offered them on next launch. The first prompt now arrives only at the next template change, where "Update all" reinstalls the excluded id (visible in the diff, skippable — consent is informed at that point). All three of #8504's acceptance criteria are met and the behavior is defensible (it also stops nagging about a deliberate choice) — this is a judgment call, hence Suggestion.
Suggested fix: scope the stamp to the update path (pass the detection-time currentVersion into executeUpdate and override the plan's providerState version), or explicitly document in the PR description that deliberate subset installs intentionally no longer prompt until the template changes.
中文说明
模板版本戳被全局应用于 resolveProviderState（所有安装路径），偏离了目标 issue #8504 讨论中给出的修复方向——"版本修正只应发生在更新路径，而不是全局地在 resolveProviderState 中——在安装时无条件地对完整内置模板哈希，会静默抑制那些故意安装了内置模型子集的用户的提示"（@doudouOUC，2026-08-04）。该偏离未在 PR 的 Risk & Scope 中说明。
失败场景：11 个 preset 设置了 modelsEditable: true，/auth 向导的 Model IDs 步骤是预填默认值的自由文本输入，因此删掉某个内置 id 的用户会经 buildInstallPlan → resolveProviderState 存下完整模板的哈希；启动检查随后看到 metadata.version === currentVersion，永远不会为故意缺失的内置模型弹出提示——PR 之前该用户下次启动会被提供这些模型。首个提示要等到下一次模板变更才出现，届时"Update all"会重装被排除的 id（在 diff 中可见、可跳过——那时的授权是知情的）。#8504 的三条验收标准均已满足、该行为也说得通（同时也避免了对用户故意选择的反复打扰）——属于判断分歧，故为 Suggestion。
建议修复：把版本戳限定在更新路径（把检测时的 currentVersion 传入 executeUpdate 并覆盖计划的 providerState version），或在 PR 描述中明确说明故意安装子集的用户在模板变更前不再收到提示。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const existingMetadata = | ||
| providerMetadata['coding-plan'] && | ||
| typeof providerMetadata['coding-plan'] === 'object' && | ||
| !Array.isArray(providerMetadata['coding-plan']) | ||
| ? (providerMetadata['coding-plan'] as Record<string, unknown>) | ||
| : {}; |
There was a problem hiding this comment.
[Suggestion] The newly added guard against a hand-edited, non-object providerMetadata['coding-plan'] entry has no test exercising either rejection branch (scalar or array seed) — the new refresh test seeds only a well-formed object.
Concrete cost: if a future refactor simplifies this to providerMetadata['coding-plan'] ?? {} (dropping the typeof / Array.isArray checks — the guard is the only thing distinguishing them), nothing in the suite fails; then with a hand-edited "coding-plan": "corrupted" (or an array) the next sync spreads character/index keys ("0", "1", …) into the persisted metadata object, writing junk keys that every later sync carries forward. Impact is cosmetic (the CLI readers pull named fields only), but the branch exists precisely for this input and is unpinned.
Suggested fix: parametrize the refresh test (or add one case) seeding providerMetadata['coding-plan'] as a string and as an array, asserting the written entry contains exactly the template-derived fields (region, baseUrl, version, builtinIds) with no stray index keys.
中文说明
新增的针对手改非对象 providerMetadata['coding-plan'] 条目的防护，没有任何测试覆盖其两个拒绝分支（标量或数组种子）——新增的 refresh 测试只注入了合法对象。
具体代价：如果未来重构把它简化为 providerMetadata['coding-plan'] ?? {}（丢掉 typeof / Array.isArray 检查——该 guard 是唯一区分它们的代码），测试套件不会有任何失败；此时手改的 "coding-plan": "corrupted"（或数组）会在下次同步时把字符/索引键（"0"、"1"……）spread 进持久化的 metadata 对象，写入之后每次同步都会携带的垃圾键。影响是表面的（CLI 读取方只取命名字段），但该分支正是为这种输入而存在，却没有被测试钉住。
建议修复：给 refresh 测试参数化字符串与数组两种种子，断言写入条目恰好包含模板派生字段（region、baseUrl、version、builtinIds），无多余索引键。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| type: 'info', | ||
| text: t('Removed outdated built-in models: {{ids}}', { | ||
| ids: droppedStaleBuiltinIds.join(', '), | ||
| }), |
There was a problem hiding this comment.
[Suggestion] The new message key 'Removed outdated built-in models: {{ids}}' is registered in NO locale file under packages/cli/src/i18n/locales/. Probe-verified under the zh locale: the key renders in English via the translations[key] ?? key fallback while a registered control key renders Chinese; registering the key flips the observation. check-i18n exits 0 and cannot see source-used-but-unregistered keys, so the omission is sticky — no automated pass will ever surface it. Note: the sibling {{plan}} success messages are equally unregistered (pre-existing orphaned translations from the {{region}} → {{plan}} rename), so this key joins an already-English cluster rather than breaking a translated one — smaller harm, but still permanently English for non-English users.
Concrete cost: a user on zh / zh-TW / ja / … whose update removes stale built-ins — the exact feature this PR adds — sees a permanently English line, and no translation pass can ever pick the key up.
Suggested fix: register the key in en.js (self-mapped, as checkKeyValueConsistency enforces) and in the strict-parity locales (zh.js, zh-TW.js) — or check-i18n will fail — plus the remaining locale files:
// en.js
'Removed outdated built-in models: {{ids}}':
'Removed outdated built-in models: {{ids}}',
// zh.js
'Removed outdated built-in models: {{ids}}': '已移除过时的内置模型：{{ids}}',中文说明
新消息键 'Removed outdated built-in models: {{ids}}' 未在 packages/cli/src/i18n/locales/ 的任何语言文件中注册。已在 zh 语言下用探针验证：该键经 translations[key] ?? key 回退渲染为英文，而已注册的对照键渲染为中文；注册该键后观察结果翻转。check-i18n 退出码为 0，且看不到"源码使用但未注册"的键，因此该遗漏是粘性的——没有任何自动化流程会发现它。注意：同组的 {{plan}} 成功消息同样未注册（{{region}} → {{plan}} 改名遗留的既有孤儿翻译），所以本键加入的是一个本就已全英文的消息簇，而不是打破已翻译的簇——危害更小，但对非英文用户仍是永久英文。
具体代价：zh / zh-TW / ja 等用户在更新移除过期内置模型时（正是本 PR 新增的功能）看到的永远是英文行，且任何翻译流程都无法拾取该键。
建议修复：在 en.js（自映射，checkKeyValueConsistency 强制键值一致）及严格配对语言（zh.js、zh-TW.js，否则 check-i18n 会失败）中注册该键，其余语言文件同步补充（见上方代码块）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
🐑 Merge conflict with main detected — dispatched the autofix loop to resolve it. / 检测到与 main 的合并冲突，已触发 autofix 处理。 |
|
⏸️ Dispatch refused: this PR has exhausted its automatic round cap (10/10), so the loop will not touch it — whatever triggered this dispatch (a merge conflict, new feedback) stays unhandled. Comment 中文说明⏸️ 已拒绝本次调度：本 PR 的自动轮次上限已用完（10/10），循环不会介入——触发本次调度的事项（合并冲突、新反馈）仍未处理。评论 |
|
The version half of this PR is now split out as #8889, so the two overlap and will conflict. #8889 carries only the version fix: The three places we both touch:
We also both modify What I'd suggest: rebase this PR once #8889 lands and keep only the stale-built-in cleanup — Two reasons for the split rather than landing this as-is. First, the prompt is misbehaving in a shipped release: 0.21.9 has #8829's cooldown and #8868's model-selection guard but not this fix, so Second, the cleanup half has an open question that the version fix does not. Also flagging that |
|
Closing as superseded by #8889 (merged as 7425e42), which carries the core fix from this PR: the stored provider version now matches what the launch check recomputes, scoped so ordinary installs keep recording the models they installed while a confirmed update records the template version. The remaining part of this PR — the builtinIds tracking mechanism — is not pursued: it spans three packages for a problem the version fix already solves, and it would not take effect for existing users whose settings carry no builtinIds marker. Thanks for the detailed investigation; the update-only stamping approach here shaped the final fix. |
…pute (QwenLM#9204) Opening a same-repo PR that touches CODEOWNERS-covered paths auto-requests every owner individually, so one PR open emits one review_requested run per owner (five within the same second on QwenLM#8830/QwenLM#9142). Only the bot-requested run can reach review-pr; the human-requested siblings used to spend a review-config runner plus an authorize job (CI_BOT_PAT permission API) each before no-op exiting. Mirror the requested_reviewer predicate precheck-pr already applies to fork PRs into authorize.if and review-config.if so the siblings complete as instant all-skipped runs. Co-authored-by: yiliang114 <yiliang114@users.noreply.github.com>
6 participants
What this PR does
Two related fixes for the Built-in Provider Update prompt on editable providers (e.g. Token Plan):
Version-hash consistency — the stored
providerMetadata.<plan>.versionis now computed from the built-in template (default models only), the same input the launch-time check uses, so a successful "update all" makesmetadata.version === currentVersionand clears the prompt.Stale built-in cleanup — record
builtinIds(built-in model ids) at install time. On a later update, an installed+owned model absent from the current default list is preserved as "custom" only if it was NOT in the previous built-in list. A model that was a built-in at last install but is now renamed/removed upstream is treated as stale and dropped.Why it's needed
Confirmed root cause (commit
761272542drenameddeepseek-v4-flash→deepseek-v4-flash-0731):customIdspreservation logic couldn't distinguish "old built-in renamed/removed" from "user manually added", so the stale renamed model (deepseek-v4-flash) was kept forever as a phantom "custom" model and never cleaned up.How it works
resolveProviderStatestoresversionfrombuildProviderTemplateandbuiltinIdsfromgetDefaultModelIds.executeUpdatefilterscustomIdsto exclude ids present inmetadata.builtinIds(stale built-ins), keeping only true user-custom models.Files changed
packages/core/src/providers/provider-config.ts— version from template + storebuiltinIdspackages/cli/src/ui/hooks/useProviderUpdates.ts—customIdsexcludes previous built-in idsReviewer Test Plan
How to verify
npx vitest run packages/core/src/providers/__tests__/provider-config.test.ts packages/core/src/providers/__tests__/install.test.ts packages/cli/src/ui/hooks/useProviderUpdates.test.ts— 104/104 pass.Tested on
Risk & Scope
builtinIds; prior stored metadata without it falls back to[](all non-default owned ids treated as custom, matching old behavior).Related
中文说明
两个修复：1) 存的 version 改用内置模板算，和启动检查同口径，update all 能清除提示；2) 记录 builtinIds，更新时把"旧内置改名/移除的"（如 deepseek-v4-flash）当过期内置清理掉，不再当自定义模型永久保留；真正手动加的自定义模型仍保留。