test(cli): pin the model selection across multi-provider template updates - #8879
Conversation
…ates 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.
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
|
Thanks for the PR — re-running triage on the new head, which adapted the tests to #8889. Template looks good ✓ Problem: real and verified. The #8863 worst case was the multi-provider batch path — Direction: aligned — a regression pin for an already-merged fix chain (#8868 → #8889), test-only. Size: not applicable — 0 production lines. The whole diff is 137 added lines in one Approach: scope is right. Two tests covering the two batch axes — an all-inactive batch, and a mixed batch containing the active provider — each pinning the batch shape ( Risk: no elevated risk signals — the only changed file is a test, and high-risk-path detection excludes test files. Moving on to code review. 🔍 中文说明感谢贡献——新 head（已适配 #8889）重新执行 triage。 模板完整 ✓ 问题：真实存在且已核实。#8863 的最坏情形是多 provider 批量路径—— 方向：对齐——为已合并的修复链（#8868 → #8889）补回归钉桩，纯测试改动。 规模：不适用——0 行生产代码。整个 diff 是一个 方案：范围合理。两个测试覆盖批量的两个轴——全非活跃批量、以及包含活跃 provider 的混合批量——各自显式钉住批量形态（确认前断言 风险：无升级风险信号——唯一改动文件是测试，高风险路径检测排除测试文件。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent proposal for "pin the batch path after #8889", before reading the diff: two tests in the existing suite — one batch where no provider is active (assert nothing writes The PR does exactly this. What I verified statically (worktree on
No critical blockers, no convention violations. Testing evidenceUnattended run — PR code is never executed here; the evidence below is the PR's own CI read via the API plus the static trace above. The PR's CI (
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Mutation check: not executed here — PR code is never run in this environment. The kill argument above is a deterministic static trace of 中文说明代码审查读 diff 之前，我对"#8889 之后钉住批量路径"的独立方案是：在现有套件中加两个测试——一个所有 provider 均非活跃的批量（断言没有任何写入触碰 PR 正是这样做的。静态核实（worktree 位于
无关键阻塞项，无规范违规。 测试证据无人值守运行——本环境从不执行 PR 代码；以下证据来自 API 读取的 PR 自身 CI 结果及上述静态追溯。PR 自身 CI（ 变异检查：未在此执行——本环境从不运行 PR 代码。上面的杀死论证是对 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — clean across every stage on the new head: a verified coverage gap, minimal idiomatic tests, green CI, and the pin substantiated in both directions. Stepping back: this is the follow-through the #8863 → #8868 chain promised, now correctly adapted to #8889's unconditional drop. The two tests sit exactly at the seam where coverage was missing — the batch loop — and each kills a mutant the other doesn't: removing the protection entirely (every pass rewrites One standing nit, non-blocking and already flagged by wenshao: the PR body still says "one regression test" / "21/21" and its mutation instructions describe the pre-#8889 gate — worth refreshing before merge, but hygiene doesn't gate approval. CI is fully green on the reviewed commit, so approving now, pinned to that commit. 中文说明回顾全局：这是 #8863 → #8868 链条承诺的后续，现已正确适配 #8889 的无条件丢弃。两个测试恰好落在覆盖缺失的接缝——批量循环——上，且各自杀死对方杀不掉的变异体：完全移除保护（每轮都改写 一个长期存在的小问题，不阻塞且 wenshao 已指出：PR 正文仍写"一个回归测试"/"21/21"，其变异操作说明描述的也是 #8889 之前的门控——合并前值得刷新，但规范问题不影响批准。 被审查提交上 CI 全绿，现在批准，钉在该提交。 — Qwen Code · qwen3.8-max Reviewed at |
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. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Test Plan (not a blocker): src/ui/hooks/useProviderUpdates.test.ts — no such file or directory.
中文说明
已审查——无阻断问题。 建议见行内评论。 Test Plan（非阻断）：src/ui/hooks/useProviderUpdates.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| await waitFor(() => { | ||
| expect(mockSettings.setValue).toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] The batch-confirm regression test asserts only negatives after onConfirm('update') — it never verifies that both providers' updates actually executed. This waitFor is satisfied by the first provider's metadata writes alone, and every assertion after it is negative, so a batch loop that drops the second provider still passes. — Failure scenario: a future change that breaks the onConfirm('update') loop after the first provider (early break/return, an off-by-one, or a second executeUpdate that throws — executeUpdate catches every error and the loop ignores its return value) leaves the token-plan template stale; findAllPendingUpdates then re-surfaces the update prompt on every startup while all 21 tests, including this dedicated batch regression test, stay green. Mutation-verified: truncating the loop to pendingList.slice(0, 1) leaves 21/21 green; the assertions below kill that mutant and stay green without it.
| await waitFor(() => { | |
| expect(mockSettings.setValue).toHaveBeenCalled(); | |
| }); | |
| await waitFor(() => { | |
| expect(mockSettings.setValue).toHaveBeenCalledWith( | |
| expect.anything(), | |
| `${PROVIDER_METADATA_NS}.${METADATA_KEY}.version`, | |
| chinaVersion, | |
| ); | |
| expect(mockSettings.setValue).toHaveBeenCalledWith( | |
| expect.anything(), | |
| `${PROVIDER_METADATA_NS}.${TOKEN_METADATA_KEY}.version`, | |
| tokenVersion, | |
| ); | |
| }); |
中文说明
[建议] 批量确认回归测试在 onConfirm('update') 之后只断言了否定条件——从未验证两个 provider 的更新是否都真正执行。这个 waitFor 仅凭第一个 provider 的元数据写入即可通过，而其后的断言又全是否定形式，因此即便批量循环丢掉了第二个 provider，测试依然会通过。— 失败场景：未来某个改动在第一个 provider 之后中断 onConfirm('update') 循环（提前 break/return、差一错误，或第二个 executeUpdate 抛异常——executeUpdate 会捕获所有错误，且循环忽略其返回值），token-plan 模板将保持过期，findAllPendingUpdates 会在每次启动时重新弹出更新提示，而包括这个专门的批量回归测试在内的全部 21 个测试仍然全绿。已通过变异验证：把循环截断为 pendingList.slice(0, 1) 后 21/21 仍全绿；下方建议的断言能杀死该变异，且自身保持绿色。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| expect(result.current.providerUpdateRequest!.entries.length).toBe(2); | ||
|
|
||
| await result.current.providerUpdateRequest!.onConfirm('update'); |
There was a problem hiding this comment.
[Suggestion] The mixed-batch scenario has zero coverage: no test confirms 'update' on a batch mixing the ACTIVE provider (whose update removes the current model) with an inactive one. This new test covers two entries that are both inactive, and the gate's keep-modelSelection branch (updatesActiveProvider && !previousModelStillAvailable) is exercised only by the single-provider 'switches model when previous model is no longer available' test. — Failure scenario: a refactor that derives per-provider state once per batch — hoisting updatesActiveProvider/previousModel out of executeUpdate into the onConfirm loop, or applying the first entry's gate decision to all entries — lets an inactive co-batched provider keep its modelSelection and rewrite model.name after the active provider's intended migration, reproducing the exact #8863 issue symptom while all 21 tests stay green. Mutation-verified: with the gate hoisted, the suite stays 21/21 green and a mixed-batch probe test fails (the inactive entry writes model.name a second time). Suggested fix: add one mixed-batch test — both metadata keys stale; modelProviders = chinaTemplate + tokenTemplate; active credentials = Coding Plan (the default mock config); getModel → a model id absent from both templates; confirm 'update'; then assert model.name was written exactly once (the fallback), syncAfterAuthRefresh was called exactly once, and no second model.name/model.baseUrl write followed from the inactive entry.
中文说明
[建议] 混合批量场景完全没有覆盖：没有任何测试对一个「包含活跃 provider（其更新会移除当前模型）与非活跃 provider」的批量确认 'update' 进行验证。本新测试覆盖的两个条目均不活跃，而门控的保留 modelSelection 分支（updatesActiveProvider && !previousModelStillAvailable）此前仅由单 provider 的 'switches model when previous model is no longer available' 测试覆盖。— 失败场景：未来某个重构把逐 provider 的状态改为按批量推导一次——例如把 updatesActiveProvider/previousModel 从 executeUpdate 提升到 onConfirm 循环中，或把第一个条目的门控决定套用到所有条目——就会让同批次中的非活跃 provider 保留其 modelSelection，在活跃 provider 完成预期迁移之后再次改写 model.name，从而重现 #8863 的确切症状，而全部 21 个测试仍然全绿。已通过变异验证：门控被提升后套件仍为 21/21 全绿，而混合批量探针测试会失败（非活跃条目第二次写入了 model.name）。建议修复：新增一个混合批量测试——两个元数据键均过期；modelProviders = chinaTemplate + tokenTemplate；活跃凭据 = Coding Plan（默认 mock 配置）；getModel → 一个在两个模板中都不存在的模型 id；确认 'update'；然后断言 model.name 恰好被写入一次（回退模型）、syncAfterAuthRefresh 恰好被调用一次，且非活跃条目没有引发第二次 model.name/model.baseUrl 写入。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 1/100) — it will retry on the next scan.
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/模型 |
…pdate-selection-8863 # Conflicts: # packages/cli/src/ui/hooks/useProviderUpdates.test.ts
…gate Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix review round summary — PR #8879Feedback points and decisions
Conflict resolution (
|
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 reviewed: reverse audit — reached the 5-round cap without two consecutive dry rounds (rounds 3, 4 and 5 each reported new findings).
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget..
Test Plan (not a blocker): src/ui/hooks/useProviderUpdates.test.ts — no such file or directory.
中文说明
已审查。 建议见行内评论。
未审查：reverse audit — reached the 5-round cap without two consecutive dry rounds (rounds 3, 4 and 5 each reported new findings)。
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.。
Test Plan（非阻断）：src/ui/hooks/useProviderUpdates.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| expect(mockModelsConfig.syncAfterAuthRefresh).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('migrates the model only for the active provider in a mixed batch', async () => { |
There was a problem hiding this comment.
[Suggestion] The mixed-batch test only exercises the one registry order where the active provider is first: ALL_PROVIDERS iterates coding-plan before token-plan and the default mock credentials point at coding-plan, so the migration winner is also the batch's first entry — the test cannot distinguish gating by credentials from gating by batch position. — Failure scenario: a refactor that grants migration rights by batch position (treating the first pending entry as the migrator whenever any entry matches credentials) passes both new tests unchanged yet reintroduces the exact #8863 symptom for a user whose active provider is later in registry order. Probe-verified: such a mutant passes this whole file, and the variant below fails it (observed model.name write of qwen3.5-plus instead of the token template's model). Suggested fix: add one variant of this test where getContentGeneratorConfig returns the Token Plan credentials (active provider second in ALL_PROVIDERS), asserting the single model.name write is tokenTemplate[0]!.id.
中文说明
[建议] 混合批量测试只演练了活跃 provider 恰好排在注册表第一位的这一种顺序：ALL_PROVIDERS 中 coding-plan 在 token-plan 之前，而默认 mock 凭据指向 coding-plan，因此迁移的胜出者同时也是批量的第一个条目——测试无法区分「按凭据门控」与「按批量位置门控」。— 失败场景：未来某个重构按批量位置授予迁移权（只要任一条目匹配凭据就把第一个待更新条目当作迁移者），两个新测试仍会全绿，但对活跃 provider 在注册表中靠后的用户会重现 #8863 的确切症状。已通过探针验证：该变异通过整个测试文件，而下面建议的变体会使其失败（观察到写入的 model.name 是 qwen3.5-plus 而非 token 模板的模型）。建议修复：为本测试增加一个变体，getContentGeneratorConfig 返回 Token Plan 凭据（活跃 provider 在 ALL_PROVIDERS 中排第二），断言唯一的 model.name 写入是 tokenTemplate[0]!.id。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| mockSettings.merged['modelProviders'] = { | ||
| [AuthType.USE_OPENAI]: [...chinaTemplate, ...tokenTemplate], | ||
| }; |
There was a problem hiding this comment.
[Suggestion] Neither batch test can detect a regression where a later entry's modelProviders patch is computed from the pre-batch store and silently reverts the earlier entry's merged model list: mockSettings.setValue is a bare vi.fn() that never writes back into mockSettings.merged — real LoadedSettings recomputes merged on every write — so entry 2's patch is always composed against the pre-batch list here, and no assertion inspects any list write or reload payload. — Failure scenario: probe-verified — a snapshot-before-loop regression passes all 28 tests: entry 2's final modelProviders.USE_OPENAI write reverts entry 1's added model while both version writes bump, after which findAllPendingUpdates never re-prompts and the user is silently stuck on the stale list. Suggested fix: make the mock apply modelProviders.* writes into mockSettings.merged (mirroring production write-through), then assert the final modelProviders.USE_OPENAI write contains ids from both chinaTemplate and tokenTemplate.
中文说明
[建议] 两个批量测试都无法发现这样的回归：后一个条目的 modelProviders 补丁基于批量开始前的存储计算，从而悄悄回滚前一个条目合并后的模型列表。mockSettings.setValue 是一个裸 vi.fn()，从不写回 mockSettings.merged——而真实的 LoadedSettings 每次写入都会重算 merged——因此这里条目 2 的补丁始终基于批量开始前的列表合成，且没有任何断言检查列表写入或 reload 载荷。— 失败场景：已通过探针验证——「循环前快照一次」的回归通过全部 28 个测试：条目 2 最终的 modelProviders.USE_OPENAI 写入回滚了条目 1 新增的模型，而两个 version 写入照常推进，之后 findAllPendingUpdates 永远不会再次提示，用户被静默卡在过期列表上。建议修复：让 mock 把 modelProviders.* 写入应用到 mockSettings.merged（模拟生产环境的写穿透），然后断言最终的 modelProviders.USE_OPENAI 写入同时包含 chinaTemplate 与 tokenTemplate 的 id。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // Default mock credentials point at Coding Plan, so only the first entry | ||
| // is the active provider. The current model exists in neither template, | ||
| // so the active entry must migrate the selection to its fallback model. | ||
| mockConfig.getModel.mockReturnValue('removed-model'); |
There was a problem hiding this comment.
[Suggestion] Because every test's current model exists in neither template, a batch-hoisted availability mutant survives both new tests — the per-provider availability computation is not actually pinned. — Failure scenario: probe-verified — feeding the gate a pendingList.some(...) union check keeps both tests green. The discriminating input is a mixed batch whose current model is offered only by the inactive entry (a token-plan-only model id with Coding Plan active — reachable, since the model picker enumerates across authTypes): correct per-provider code migrates once to chinaTemplate[0]!.id; the union mutant computes "some batch entry offers it", deletes modelSelection and writes nothing — leaving the selection on a model the active provider doesn't offer, the #8863 symptom class. Suggested fix: add one mixed-batch case with such a model id, asserting the same single migration.
中文说明
[建议] 由于每个测试的当前模型都不存在于任何模板中，「把可用性检查提升到批量层级」的变异能在两个新测试下存活——逐 provider 的可用性计算实际上并未被钉住。— 失败场景：已通过探针验证——给门控喂一个 pendingList.some(...) 并集检查，两个测试仍然全绿。区分性输入是：混合批量中当前模型只由非活跃条目提供（Coding Plan 活跃时选了一个仅 token-plan 提供的模型 id——这是可达状态，因为模型选择器会跨 authType 枚举）。正确的逐 provider 代码会迁移一次到 chinaTemplate[0]!.id；并集变异算出「某个批量条目提供了它」，删除 modelSelection 且什么都不写——选择被留在一个活跃 provider 并不提供的模型上，属于 #8863 的症状类别。建议修复：新增一个使用该模型 id 的混合批量用例，断言同样的单次迁移。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // Both entries ran to completion, regardless of provider order. | ||
| await waitFor(() => { | ||
| expect(mockConfig.reloadModelProvidersConfig).toHaveBeenCalledTimes(2); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] Three distinct properties this completion gate should pin are unpinned — reported as three separate comments on this block; this one is about version persistence: this test measures entry completion only by reload count and never asserts that either entry persisted its new providerMetadata.<key>.version on the migration path. No test anywhere pins version persistence when a migration happened (the three existing .version assertions are all on non-migration paths). — Failure scenario: probe-verified end to end — a one-line regression skipping the providerState step when modelSelection was applied passes all 28 tests here plus core install.test.ts and useAuth.test.ts; the user then gets the identical update prompt on every launch because metadata.version stays at the old hash. Suggested fix: after this wait, assert both version writes as the first batch test does (chinaVersion / tokenVersion) — adding them flips the probe.
中文说明
[建议] 该完成门控应当钉住的三个不同属性都未被钉住——已作为三条评论分别报告在这个代码块上；本条关于版本持久化：本测试只用 reload 次数衡量条目完成，从未断言任一条目在迁移路径上持久化了新的 providerMetadata.<key>.version。整个仓库没有任何测试钉住「发生迁移时」的版本持久化（现有三处 .version 断言都在非迁移路径上）。— 失败场景：已端到端探针验证——当 modelSelection 被应用时跳过 providerState 步骤的一行回归，能通过这里全部 28 个测试以及 core 的 install.test.ts 和 useAuth.test.ts；由于 metadata.version 停在旧哈希上，用户每次启动都会看到同样的更新提示。建议修复：在此等待之后，像第一个批量测试那样断言两个 version 写入（chinaVersion / tokenVersion）——加上后探针翻转。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // Both entries ran to completion, regardless of provider order. | ||
| await waitFor(() => { | ||
| expect(mockConfig.reloadModelProvidersConfig).toHaveBeenCalledTimes(2); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] (Second of the three unpinned properties on this gate — sequential execution.) Neither new batch test pins the loop's sequential await executeUpdate semantics — the completion gates count events that also accumulate under concurrent execution. — Failure scenario: probe-verified — replacing the loop with Promise.all (or dropping the await) keeps all 28 tests green. In production both providers share USE_OPENAI, and each applyProviderInstallPlan snapshots getModelProviders() and calls backup()/restore() on failure, so concurrent entries can roll each other's persisted writes back and reload stale maps — one provider's update silently lost after its success message was already posted. Suggested fix: record event order through the mocks (reload / refreshAuth-resolved) and assert the interleaved order ['reload', 'refreshAuth-resolved', 'reload'] — it fails against the Promise.all mutant and passes on current code.
中文说明
[建议]（该门控上三个未钉住属性中的第二个——顺序执行。）两个新批量测试都没有钉住循环的逐个 await executeUpdate 语义——完成门控统计的事件在并发执行下同样会累积。— 失败场景：已通过探针验证——把循环换成 Promise.all（或丢掉 await）后 28 个测试仍全绿。生产中两个 provider 共享 USE_OPENAI，每个 applyProviderInstallPlan 都会快照 getModelProviders() 并在失败时调用 backup()/restore()，因此并发条目可能互相回滚已持久化的写入、并用过期快照 reload——某个 provider 的更新在成功消息已经发出后被静默丢失。建议修复：通过 mock 记录事件顺序（reload / refreshAuth-resolved），断言交错的顺序 ['reload', 'refreshAuth-resolved', 'reload']——它对 Promise.all 变异失败、对当前代码通过。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // Both entries ran to completion, regardless of provider order. | ||
| await waitFor(() => { | ||
| expect(mockConfig.reloadModelProvidersConfig).toHaveBeenCalledTimes(2); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] (Third of the three unpinned properties on this gate — failure continuation.) No test in this suite ever makes an entry fail (zero mockRejectedValue in the file), so the batch loop's continue-on-entry-failure semantics — executeUpdate's per-entry try/catch feeding the loop — are unpinned. — Failure scenario: probe-verified — a mutant making the catch rethrow keeps all 28 tests green. In production, if the first (active) entry's refreshAuth rejects (endpoint briefly unreachable), without the catch the rejection propagates out of the loop: entry 2 never runs, its version is never persisted, no error item is emitted, and the user is re-prompted on every launch with no explanation. Suggested fix: add a batch variant with mockConfig.refreshAuth.mockRejectedValueOnce(new Error('endpoint down')), asserting the error addItem for entry 1 and that the token-plan version write still happens — it fails under the mutant and passes on current code.
中文说明
[建议]（该门控上三个未钉住属性中的第三个——失败后继续。）本套件没有任何测试让某个条目失败（整个文件没有一处 mockRejectedValue），因此批量循环「单条目失败后继续」的语义——executeUpdate 的逐条目 try/catch 喂养循环——未被钉住。— 失败场景：已通过探针验证——让 catch 重新抛出的变异能使 28 个测试全绿。生产中，如果第一个（活跃）条目的 refreshAuth 被拒绝（端点短暂不可达），没有该 catch 时拒绝会传出循环：条目 2 永远不会执行，其 version 不会被持久化，不会发出任何错误条目，用户每次启动都被重新提示且没有任何解释。建议修复：新增一个批量变体，mockConfig.refreshAuth.mockRejectedValueOnce(new Error('endpoint down'))，断言条目 1 的错误 addItem 且 token-plan 的 version 写入仍然发生——该测试在变异下失败、对当前代码通过。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| const modelBaseUrlWrites = mockSettings.setValue.mock.calls.filter( | ||
| (call: unknown[]) => call[1] === 'model.baseUrl', | ||
| ); | ||
| expect(modelBaseUrlWrites).toHaveLength(1); |
There was a problem hiding this comment.
[Suggestion] This test pins the value of the model.name write but only the count of the model.baseUrl write. — Failure scenario: a regression that migrates with the correct model id but the wrong baseUrl (the inactive entry's base URL leaking into the active entry's modelSelection, or a stale non-empty disambiguator surviving) keeps toHaveLength(1) green and ships the wrong model.baseUrl to user settings. The expected value is knowable: syncAfterAuthRefresh is asserted to receive undefined, and applyProviderInstallPlan then writes the '' tombstone — probe-verified that the assertion below passes on current code.
| const modelBaseUrlWrites = mockSettings.setValue.mock.calls.filter( | |
| (call: unknown[]) => call[1] === 'model.baseUrl', | |
| ); | |
| expect(modelBaseUrlWrites).toHaveLength(1); | |
| const modelBaseUrlWrites = mockSettings.setValue.mock.calls.filter( | |
| (call: unknown[]) => call[1] === 'model.baseUrl', | |
| ); | |
| expect(modelBaseUrlWrites).toEqual([ | |
| [expect.anything(), 'model.baseUrl', ''], | |
| ]); |
中文说明
[建议] 本测试钉住了 model.name 写入的值，却只钉住了 model.baseUrl 写入的次数。— 失败场景：迁移时模型 id 正确但 baseUrl 错误的回归（非活跃条目的 baseUrl 泄漏进活跃条目的 modelSelection，或非空的过期消歧值残留）仍能让 toHaveLength(1) 通过，把错误的 model.baseUrl 写进用户设置。期望值可以确定：syncAfterAuthRefresh 已被断言收到 undefined，而 applyProviderInstallPlan 随后写入 '' 墓碑值——已探针验证下方断言在当前代码上通过。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| expect(mockModelsConfig.syncAfterAuthRefresh).toHaveBeenCalledTimes(1); | ||
| expect(mockModelsConfig.syncAfterAuthRefresh).toHaveBeenCalledWith( | ||
| AuthType.USE_OPENAI, | ||
| chinaTemplate[0]!.id, | ||
| undefined, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] This test measures the inactive entry's completion only by reload count and never asserts it persisted its modelProviders.<authType> list write. — Failure scenario: probe-verified — a mutant deleting installPlan.modelProviders for inactive entries passes all 28 tests (including the pre-existing inactive single-entry tests). In production findAllPendingUpdates then skips that provider forever (metadata.version === currentVersion), so the user silently never receives that provider's added/removed models and the update prompt never returns for it. Suggested fix: filter setValue calls on modelProviders.${AuthType.USE_OPENAI} and expect 2 writes (or assert the final reload payload contains tokenTemplate models) — the length-2 assertion fails against the mutant.
中文说明
[建议] 本测试只用 reload 次数衡量非活跃条目的完成，从未断言它持久化了自己的 modelProviders.<authType> 列表写入。— 失败场景：已通过探针验证——对非活跃条目删除 installPlan.modelProviders 的变异能通过全部 28 个测试（包括既有的非活跃单条目测试）。生产中 findAllPendingUpdates 从此永远跳过该 provider（metadata.version === currentVersion），用户静默地永远收不到该 provider 新增/移除的模型，更新提示也不会再为它出现。建议修复：过滤 modelProviders.${AuthType.USE_OPENAI} 上的 setValue 调用并期望 2 次写入（或断言最终 reload 载荷包含 tokenTemplate 的模型）——长度为 2 的断言对该变异失败。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 3/100) — 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/31416010267 🧠 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/31419817978 🧠 Handled by Qwen Code · model/模型 |
|
⏸️ Takeover paused: this PR reached its round cap (100/100). Comment 中文说明⏸️ 托管已暂停：本 PR 达到轮次上限（100/100）。评论 |
|
@qwen-code /takeover (Re-arming after the round cap — the substantive work is already done: |
…pdate-selection-8863
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.
|
Synced with #8889 made the update path drop the plan's (Worth noting: #8889's unconditional drop converges with the approach originally proposed in #8873 — nice to see the semantics land either way.) |
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): This PR adds two regression tests to packages/cli/src/ui/...: full log of the first 6-failure run was not captured; its attribution to environment ENOSPC rests on the captured ENOSPC run plus ~30 consecutive green runs aft….
Test Plan (not a blocker): src/ui/hooks/useProviderUpdates.test.ts — no such file or directory.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：This PR adds two regression tests to packages/cli/src/ui/...：full log of the first 6-failure run was not captured; its attribution to environment ENOSPC rests on the captured ENOSPC run plus ~30 consecutive green runs aft…。
Test Plan（非阻断）：src/ui/hooks/useProviderUpdates.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const foreignModel = { | ||
| id: 'my-own-model', | ||
| baseUrl: 'https://my-own-gateway.example.com/v1', | ||
| envKey: 'MY_OWN_KEY', | ||
| name: '[Mine] my-own-model', | ||
| }; |
There was a problem hiding this comment.
[Suggestion] R3-1: foreignModel fixture duplicated from the pre-existing it.each test — this block is byte-identical to the fixture in 'does not move the user off a model $name' (~lines 615-620): same four fields and the same getModel/getContentGeneratorConfig wiring. The file already keeps shared template fixtures (chinaTemplate, tokenTemplate) at module scope, so the reuse point exists. — Failure scenario: both copies encode "a user model owned by neither plan provider"; a future change to that fixture (a newly required ModelInfo field, an id/envKey rename) must be applied in two places 65 lines apart, and updating only one makes the two tests silently pin different "foreign model" scenarios while both copies still look canonical.
Suggested fix — hoist the fixture to module scope alongside chinaTemplate/tokenTemplate and reference it from both tests (mockConfig.getModel.mockReturnValue(foreignModel.id)):
const foreignModel = {
id: 'my-own-model',
baseUrl: 'https://my-own-gateway.example.com/v1',
envKey: 'MY_OWN_KEY',
name: '[Mine] my-own-model',
};中文说明
[建议] R3-1：foreignModel 夹具与既有 it.each 测试 'does not move the user off a model $name'（约 615-620 行）中的夹具逐字相同：四个字段与 getModel/getContentGeneratorConfig 接线完全一致。本文件已在模块作用域维护共享模板夹具（chinaTemplate、tokenTemplate），复用点已存在。— 失败场景：两处副本编码同一概念——「不属于任何计划 provider 的用户自有模型」；未来对该夹具的任何修改（新增必需的 ModelInfo 字段、id/envKey 改名）都必须在相隔 65 行的两处同步应用，只改一处会让两个测试静默钉住不同的「外部模型」场景，而两处副本看起来都像是权威定义。
建议修复——将夹具提升到模块作用域，与 chinaTemplate/tokenTemplate 并列，两个测试改为引用它（如 mockConfig.getModel.mockReturnValue(foreignModel.id)）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| mockSettings.merged['modelProviders'] = { | ||
| [AuthType.USE_OPENAI]: [foreignModel, ...chinaTemplate, ...tokenTemplate], | ||
| }; |
There was a problem hiding this comment.
[Suggestion] R3-4: This test seeds a user-owned provider entry (foreignModel) into the same protocol list as the two batch-updated providers, but neither batch test ever asserts the merged list's content — the entry's survival across two sequential applyModelProvidersPatch merges rests solely on the preservedModels filter and is pinned by nothing. (The pre-existing 'persists the template version and preserves custom models' covers only one entry and only provider-owned ids carried via customIds — a different mechanism.) — Failure scenario: probe-verified — a merge regression, or a widened ownsModel predicate, silently drops the user's custom endpoint from modelProviders during a two-provider batch update (their self-hosted model vanishes from the picker) while both tests stay green.
Suggested fix — assert both reload payloads keep the foreign entry; match baseUrl as well, since an id-only matcher would miss the widened-ownsModel variant, which re-adds the id rebuilt under the provider's baseUrl:
const [firstReload, secondReload] =
mockConfig.reloadModelProvidersConfig.mock.calls;
for (const reloaded of [firstReload?.[0], secondReload?.[0]]) {
expect(reloaded?.[AuthType.USE_OPENAI]).toEqual(
expect.arrayContaining([
expect.objectContaining({
id: 'my-own-model',
baseUrl: 'https://my-own-gateway.example.com/v1',
}),
]),
);
}中文说明
[建议] R3-4：本测试把用户自有的 provider 条目（foreignModel）与两个批量更新的 provider 放进同一协议列表，但两个批量测试都从未断言合并后列表的内容——该条目在连续两次 applyModelProvidersPatch 合并后仍然存活，完全依赖 preservedModels 过滤器，却没有任何断言钉住它。（既有测试 'persists the template version and preserves custom models' 只覆盖单条目、且只覆盖经由 customIds 携带的 provider 自有 id——机制不同。）— 失败场景：已通过探针验证——合并逻辑回归、或 ownsModel 判定被放宽，都会在双 provider 批量更新期间静默丢弃用户的自定义端点（其自托管模型从选择器中消失），而两个测试仍然全绿。
建议修复——断言两次 reload 载荷都保留了该外部条目；同时匹配 baseUrl，因为仅匹配 id 会漏掉「放宽 ownsModel」的变体——该变体会把同一 id 以 provider 的 baseUrl 重建后重新加回列表。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| await waitFor(() => { | ||
| expect(mockSettings.setValue).toHaveBeenCalledWith( | ||
| expect.anything(), | ||
| `${PROVIDER_METADATA_NS}.${METADATA_KEY}.version`, | ||
| chinaVersion, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R3-3: This completion gate is recorded before every failure-capable step and survives rollback — a per-entry failure swallowed by executeUpdate's catch leaves the test green. The version writes asserted here happen at the providerState step — before persist, reloadModelProviders and refreshAuth — and the catch-path rollback cannot un-record mock calls; neither batch test asserts the success addItem the single-provider sibling 'leaves the model selection alone when the previous model is gone' uses to pin completion. — Failure scenario: probe-verified — making any post-providerState step throw for one entry rolls that entry back and shows the user a "Failed to update provider configuration" item, yet this gate is already satisfied and all negative assertions hold vacuously — the batch test passes with one provider not updated at all. The second test's reloadModelProvidersConfig x2 gate has the same hole (a rollback reload also satisfies the count), and asserting version writes does not close it — version writes are equally rollback-invisible in this harness.
Suggested fix — pin completion via the per-provider success items, the one observable that distinguishes success from swallowed failure (in both batch tests; optionally plus no type: 'error' item):
expect(mockAddItem).toHaveBeenCalledWith(
expect.objectContaining({
text: 'Coding Plan configuration updated successfully.',
}),
expect.any(Number),
);
expect(mockAddItem).toHaveBeenCalledWith(
expect.objectContaining({
text: 'Token Plan configuration updated successfully.',
}),
expect.any(Number),
);中文说明
[建议] R3-3：该完成门控记录在所有可能失败的步骤之前、且能在回滚后幸存——executeUpdate 的 catch 吞掉的单条目失败仍会让测试全绿。这里断言的 version 写入发生在 providerState 步骤——早于 persist、reloadModelProviders 与 refreshAuth——而 catch 路径的回滚无法取消已记录的 mock 调用；两个批量测试都没有断言单 provider 同类测试 'leaves the model selection alone when the previous model is gone' 用来钉住「完成」的成功 addItem。— 失败场景：已通过探针验证——让任一 providerState 之后的步骤对某个条目抛错，会使该条目回滚并向用户展示 "Failed to update provider configuration" 条目，但此门控此时已经满足、所有负向断言也空转通过——批量测试在一个 provider 完全未更新的情况下仍然通过。第二个测试的 reloadModelProvidersConfig x2 门控有同样的漏洞（回滚 reload 也会计入次数），且断言 version 写入并不能堵住它——在本测试环境中 version 写入同样对回滚不可见。
建议修复——用逐 provider 的成功条目钉住「完成」，这是本环境中唯一能区分成功与被吞失败的信号（两个批量测试都加；可选再加「无 type: 'error' 条目」断言）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| mockConfig.getModel.mockReturnValue('removed-model'); | ||
| mockConfig.refreshAuth.mockResolvedValue(undefined); |
There was a problem hiding this comment.
[Suggestion] R3-2: This test's stated purpose is to pin the ACTIVE-provider side of a mixed batch, but no assertion verifies the active-provider path was actually exercised — the premise rests entirely on the shared beforeEach credentials (verified to hold at this commit). refreshAuth is stubbed here but never asserted, and since #8889 the only observable active-vs-inactive difference in a template update is the refreshAuth call. The sibling 'executes update when user confirms with "update"' pins the same side condition with toHaveBeenCalledWith(AuthType.USE_OPENAI). — Failure scenario: probe-verified — a future edit to the shared beforeEach mock credentials, the coding-plan preset's baseUrl list, or providerMatchesCredentials makes the first entry silently stop being the active provider; every assertion still passes (no template update touches model.name regardless of active/inactive status), so the file stays green while this test no longer exercises the path it is titled for.
Suggested fix — after onConfirm('update') and the existing completion gate:
expect(mockConfig.refreshAuth).toHaveBeenCalledTimes(1);(optionally expect(mockConfig.refreshAuth).toHaveBeenCalledWith(AuthType.USE_OPENAI);).
中文说明
[建议] R3-2：本测试声称要钉住混合批量中活跃 provider 的一侧，但没有任何断言验证活跃 provider 路径真的被执行——该前提完全依赖共享的 beforeEach 凭据（已验证在当前提交下成立）。这里对 refreshAuth 只做了 stub、从未断言其被调用；而 #8889 之后，模板更新中活跃与非活跃 provider 唯一可观测的差异就是 refreshAuth 调用。同类测试 'executes update when user confirms with "update"' 用 toHaveBeenCalledWith(AuthType.USE_OPENAI) 钉住了同样的前提条件。— 失败场景：已通过探针验证——未来对共享 beforeEach mock 凭据、coding-plan 预设 baseUrl 列表或 providerMatchesCredentials 的修改，可能让第一个条目静默不再是活跃 provider；而所有断言仍会通过（模板更新无论活跃与否都不会碰 model.name），文件全绿，这个以「混合批量中的活跃 provider」为名的测试却不再演练该路径。
建议修复——在 onConfirm('update') 与既有完成门控之后加：expect(mockConfig.refreshAuth).toHaveBeenCalledTimes(1);（可选再加 toHaveBeenCalledWith(AuthType.USE_OPENAI)）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
@yiliang114 Gentle ping — this batch-path regression pin for your #8868 (and its post-#8889 adaptation) is green and only waiting on the required approvals. No rush. / 顺手 ping：#8868 批量路径的回归测试（已适配 #8889）已全绿，只差必需的 approve，方便时请过目，不急。 |
Local verification report (maintainer)I built a real environment for this PR and ran it end to end on macOS, plus a mutation matrix to measure what the two added tests actually buy us. Verdict: verified — safe to merge. One mutant that today's Environment
1. Merge shape
2. Suite on macOS — 31/31CI ran the ubuntu leg only here (
So macOS can be promoted from 3. Mutation matrix — does the PR earn its 137 lines?I applied five production mutants to
What this says:
4. Nits on the PR body (not blocking, but please refresh)
Reproducegit worktree add head <PR-head-sha> && cd head && npm ci
cd packages/cli && npx vitest run src/ui/hooks/useProviderUpdates.test.ts --reporter=verbose
# M4 mutant: in executeUpdate, replace the unconditional
# delete installPlan.modelSelection;
# with a guard that only fires on the first provider of a batch
# (reset a counter before the `for (const p of pendingList)` loop in onConfirm),
# then re-run against main's test file vs this PR's.中文版本本地验证报告（维护者）我在本地搭了真实环境跑完这个 PR，并额外做了变异测试矩阵，用来量化新增的两个用例到底值不值这 137 行。 结论：验证通过，可以合并。 有一类变异当前 环境
1. 合并形态PR head 上的 2. macOS 上跑套件 —— 31/31CI 这里只实跑了 ubuntu 腿（
macOS 可以从 3. 变异矩阵 —— 这 137 行值不值我对
解读：
4. PR 正文的两处小问题（不阻塞，但建议更新）
|
|
@qwen-code /triage |
|
Sandboxed verification: 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: 28 passed · 0 failed · 28 total 中文 — 判定：
|
| mutant | old arm (base tests) | new arm (PR tests) | status |
|---|---|---|---|
| M0 control (none) | 0/29 red | 0/31 red | green both arms |
M1 remove delete installPlan.modelSelection |
4/29 red | 6/31 red | killed/killed — both new tests newly red |
M2 break; after first executeUpdate in the batch loop |
1/29 red | 3/31 red | killed/killed — both new tests newly red |
M3 unconditional refreshAuth pass-through |
3/29 red | 3/31 red | killed/killed (same pre-existing killers) |
M4 disable core planOffersCurrentModel gate |
0/29 red | 0/31 red | survived/survived → Finding 1 |
Witnesses: evidence/01-suite-head-31-pass.png (head suite 31/31 verbose), evidence/02-mutation-matrix.png (teed live matrix output), evidence/03-m1-new-arm-red.png (M1 red run; the failure detail shows the pre-#8889 wire behavior — 7th spy call writes model.name = qwen3.7-plus, the plan default, then a model.baseUrl tombstone — failing at the mixed-batch test's model.name assertion, [6/6]).
Attribution (scripted, from vitest JSON): under M1 both new tests fail on expected "spy" to not be called with arguments: [Anything, 'model.name', Anything] — exactly the assertion their names promise. Under M2, test 1 fails on the missing second provider's version write and test 2 on expected "spy" to be called 2 times, but got 1 times — a seam-level proof that the unmutated batch loop runs both iterations and the new tests pin both (reachability/vacuity check: the negative model.name assertions cannot pass vacuously; each test also asserts entries.length === 2 and a positive completion before the negatives).
Fixture-name check: fixture-check.mjs (11/11, folded into the assertion count) verified the tests' naming claims against the real registry in the built core package: removed-model/my-own-model are in neither provider's built-in list; coding-plan precedes token-plan in ALL_PROVIDERS; the default mock credentials make coding-plan the active provider and token-plan inactive (so "mixed batch" is literally true); the metadata keys match resolveMetadataKey.
No mutant regressed killed→survived (M1/M2/M3 killed on both arms; M4 survived on both).
Corrections
These correct the PR description, not the code — no change requested:
- The Reviewer Test Plan's mutation recipe is stale. It says reverting the fix(cli): keep the selected model when refreshing another provider's template #8868 gate to
if (previousModelStillAvailable)without!updatesActiveProvider ||fails the new test. Neither identifier exists anywhere in the tree at this head: fix(core,vscode): keep provider update versions in sync #8889 replaced that gate with the unconditionaldelete installPlan.modelSelectionin the hook (the coreplanOffersCurrentModelgate is the 奇怪的bug，升级以后默认会使用更高单价的model自动修改setting.json中的参数，并自行调用浪费tokens的策略 #5819 install-path gate, a different layer). The substantive claim holds — the intent-equivalent revert (M1) fails both new tests alongside the single-provider ones (6 red) — but a reviewer following the recipe verbatim will find no code to revert. - "21/21" is stale. The suite is 31 tests at head and 29 at base (the
it.eachcases expand); the +2 delta the PR adds is correct.
Findings (non-blocking)
F1 — Suggestion (pre-existing completeness gap, not introduced by this PR): the core keep-gate is pinned by no test anywhere
M4 (disabling if (planOffersCurrentModel) in packages/core/src/providers/install.ts) survives every suite that touches the install path: the useProviderUpdates suite on both arms (0/29, 0/31), the full core providers directory (146/146 green), and the cli auth+updates suites (56/56 green) — all measured with the mutant applied. The gate's keep-branch ("if the plan still offers the current model, keep it", #5819) is live code (reachable via /auth re-runs and ACP reconnects) but has no failing test when disabled; every existing model.name assertion in the repo is on the write side.
This is not a defect in this PR: the PR's subject — the template-update path — is protected by the hook's delete (M1, pinned by this PR's tests and the pre-existing single-provider ones), and M4 cannot leak into template updates because the hook strips modelSelection before applyProviderInstallPlan. It is a completeness report on the neighboring layer, labeled as such, not a merge condition.
Reproduce (survivor):
# in packages/core/src/providers/install.ts: s/if (planOffersCurrentModel) {/if (false) {/
cd packages/core && npx vitest run src/providers --coverage.enabled=false # 146/146 greenThe fixture that would pin it (measured: green unmutated, red under M4 — adjudicate.mjs section 3b): an adapter whose getValue('model.name') returns 'model-a', a plan offering model-a with modelSelection: { modelId: 'model-b' }, asserting setValue is never called with 'model.name'. If a follow-up PR adds that test, it should ship with it.
F2 — Note: the new batch tests do not pin the refreshAuth gate in batch mode
Under M3 (unconditional refreshAuth pass-through), both arms fail identically on the three pre-existing single-provider tests (does not change auth when updating an inactive provider ×2, does not refresh auth before auth initialization completes); the two new batch tests carry no refreshAuth assertion, so they add nothing on that axis. The behavior remains pinned by the pre-existing tests — completeness note only.
Not covered
- Per-commit attribution: the checkout is depth-2/shallow;
git rev-list HEAD^1..HEAD^2yields onlyf36e9e044fwhile the metadata lists 5 commits, so per-commit verification was out of reach. The aggregateHEAD^1..HEADdiff (one test file, +137) was verified and matches the stated intent. - Full cli workspace suite and repo-wide lint: delegated to the PR's own CI. Targeted gates run here with liveness proofs:
tsc --noEmitin packages/cli (exit 0, empty), eslint on the changed file (exit 0; planted unused-var probe caught, exit 1), prettier--checkon the changed file (exit 0; misformatted probe caught, exit 1). - TUI/E2E behavior: the change is unit-test-only; no user-observable surface.
- The description's mutation recipe as literally written: the code shape no longer exists (see Corrections); M1 is the intent-equivalent.
- vscode-ide-companion's
applyProviderInstallPlanToFile: checked for a duplicated predicate — it has a different concern (runtime-snapshot-ID filter), no second keep-gate; not mutation-tested.
Methodology
Environment: CI verify container (node:22-bookworm), merge-ref checkout at depth 2 (HEAD merge 9d87c27999, base 187637449b, head f36e9e044f), npm ci + build pre-run; no GitHub access. Harnesses (all in this artifact dir, rerunnable): mutation-matrix.mjs (10 vitest cells = 5 mutants × 2 test-file arms, JSON reporter, mutants applied/restored with post-run git status verified clean), fixture-check.mjs (registry oracle over the built core dist), adjudicate.mjs (28 scripted assertions incl. M4 lethality pairs, the keep-branch probe, and liveness-proven gates; writes assertions.json). Raw per-cell logs and JSON in logs/. Core code under test ran from source via the cli vitest alias, so mutants in both packages took effect without rebuilds.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
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. ✅
|
Released in v0.21.11. |
4 participants
What this PR does
Adds one regression test pinning the multi-provider batch path of the #8863 fix: two providers with stale templates, a current model owned by neither, a single
Update allconfirmation — and nomodel.name/model.baseUrlwrites from either pass of the update loop.Why it's needed
The worst case reported in #8863 was the batch path:
onConfirmloopsexecuteUpdateover every pending provider, and before #8868 each iteration rewrotemodel.namein turn — the last provider inALL_PROVIDERSorder won, regardless of the user's intent (observed astoken-plan+grokboth stale ending ongrok-4.5). The #8868 gate protects this, but its regression tests cover a single updating provider; if the batch behavior regressed, nothing failed. Follow-up promised in #8873 (a parallel fix for #8863 superseded by #8868).Reviewer Test Plan
How to verify
vitest run src/ui/hooks/useProviderUpdates.test.tsinpackages/cli— 21/21.if (previousModelStillAvailable)without!updatesActiveProvider ||) fails this test alongside the two single-providerdoes not move the user off a model …tests — the batch scenario is the added dimension.Evidence (Before & After)
N/A (test-only).
Tested on
Environment (optional)
N/A (unit tests only).
Risk & Scope
useProviderUpdatessuite.Linked Issues
Refs #8863, #8868, #8873.
中文说明
本 PR 做了什么
为 #8863 修复的多 provider 批量路径补一个回归测试：两个 provider 模板同时过期、当前模型不属于其中任何一个、一次
Update all确认——更新循环的两轮都不得写model.name/model.baseUrl。为什么需要
#8863 报告的最坏情形正是批量路径：
onConfirm对每个待更新 provider 循环执行executeUpdate，#8868 之前每轮都会改写一次model.name，ALL_PROVIDERS注册表顺序最后的胜出（实测token-plan+grok同时过期最终落在grok-4.5）。#8868 的门控保护了它，但其回归测试只覆盖单 provider 更新；批量行为若回归不会有测试失败。这是在 #8873（与 #8868 平行的 #8863 修复，已被取代关闭）中承诺的后续。验证方式
packages/cli下vitest run src/ui/hooks/useProviderUpdates.test.ts—— 21/21。!updatesActiveProvider ||）会让本测试与两个单 provider 的does not move the user off a model …测试一同失败——批量场景是新增的独立维度。风险与范围
useProviderUpdates套件之外无。关联 Issue
参考 #8863、#8868、#8873。