feat(providers): load model recommendations before editing - #9980
Conversation
E2E / behavioral verification reportExact verified head: Baseline on Final-head automated verification:
Manual limitation: this checkout has no global |
|
Thanks for the PR — this is a much tighter take than #9389, and it shows.
Moving on to code review. 🔍 中文说明感谢贡献——相比 #9389,这一版收敛了很多,能看出来下了功夫。
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewBefore reading the diff, my independent take on this problem was: one small core module wrapping the existing bounded-fetch helper with a hard timeout and byte cap, an opt-in flag on the two presets, and a wizard step that refuses to mount the editor until the request settles — because the one unforgivable failure mode is a late response rewriting text under the cursor. The PR lands essentially on that same shape, which is a good sign it is the minimal design. What I verified against the code:
No blockers found. Two non-blocking notes:
The new flow has one moving part worth a picture: sequenceDiagram
participant P1 as User
participant P2 as Auth wizard
participant P3 as Discovering step
participant P4 as discoverProviderModels
participant P5 as Provider models endpoint
P1->>P2: enter Model IDs step
P2->>P3: mount (preset opted in)
P3->>P3: show loading, editor not mounted
P3->>P4: baseUrl, key, static models, abort signal
P4->>P5: GET baseUrl/models (5s budget, 1 MiB cap)
alt catalog ok
P5-->>P4: data with model ids
P4-->>P3: merged list (known specs kept, new ids appended)
P3->>P2: mount editor with provider snapshot
else any failure
P5-->>P4: timeout, non-2xx, malformed or empty
P4-->>P3: null
P3->>P2: mount editor with built-ins plus notice
end
P1->>P2: Esc during load aborts the request
Files changed (10 of 10 shown)
Testing evidenceThis is an unattended CI run — I do not execute PR code, so the evidence below is the PR's own CI on the reviewed commit, read through the API. The unit suite was still running at review time; the table region below is updated in place by the finalize job once CI settles. Security-related checks (secret scan, dependency CVE audit) and both Desktop Shell jobs are already green. The author's message cites focused-test counts (84 core cases, 10 render cases) — that is the author's claim; the CI result below is the evidence. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Sandboxed verification would settle the remaining gap: 中文说明代码审查:实现与独立评估的最小设计一致。发现模块复用现有的受限 fetch 工具(5 秒全传输预算、1 MiB 流式上限、仅同 host 重定向、调用方可中断),跨 host 重定向按回退处理,bearer key 不会离开配置的 host。"迟到响应覆盖输入"的竞态确实被关闭:编辑器在快照落定前不挂载,编辑仅在 Enter 时提交,卸载时中止请求;已核对向导外壳——Esc 在外壳层处理,加载中也能返回,且不存在能在编辑器挂载前推进步骤的 Enter 路径。未选择加入的 provider 完全保持原行为。所有失败路径(缺 key、超时、非 2xx、格式错误、空目录、全为非聊天模型)都回退到完整内置列表并行内提示,不落日志、不做缓存。无阻塞问题。两个非阻塞提醒:三个新界面字符串未加入九个语言文件( 测试证据:本次为无人值守 CI 运行,不执行 PR 代码,以上证据来自 API 读取的 PR 自身 CI。审查时单元测试套件仍在运行,下方表格区域会在 CI 落定后由 finalize 任务原地更新;安全扫描与两个 Desktop Shell 任务已通过。作者描述的测试数量是作者自述,以下方 CI 结果为准。 沙箱验证建议:渲染测试与核心测试分别 mock 了发现函数与 fetch 层,没有任何一层跑过真实目录响应,作者也说明未用真实认证的 Token Plan 目录探测过(且仅在 macOS 上测试)。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — clean static review and a genuinely well-reduced design, but this sits on the auth wizard and model-selection surface, so it needs a maintainer's sign-off rather than an auto-approval. Stepping back: this is the second attempt by the same author, and the iteration went the right way — 6,640 additions became 503, the persistent cache the issue sketched was dropped for a stateless snapshot, and what remains is exactly the minimal design I would have proposed: one bounded request per wizard visit, reuse of the existing fetch policy helper, and an editor that physically cannot mount before the snapshot settles. The race everyone worries about with this kind of feature is closed at the structure level, not just by a flag. Every line in the diff serves the stated goal; no drive-bys. What I cannot honestly sign off on, and why I'm not approving:
So: no approval, no request-changes — the code itself has nothing I would block on. ⏸️ Deferring to @wenshao and the 中文说明信心度:3/5 —— 静态审查干净,设计也收敛得相当好,但改动落在认证向导与模型选择这一敏感面上,需要维护者拍板,而不是自动批准。 整体看:这是同一作者的第二版,迭代方向正确——6,640 行新增收敛到 503 行,issue 草案里的持久缓存被换成无状态快照,剩下的正是我会提出的最小设计:每次进入向导只发一次受限请求、复用现有 fetch 策略工具、编辑器在快照落定前物理上不可能挂载。这类功能最担心的竞态在结构层面就被关死了,而不是靠某个标志位。diff 中每一行都服务于既定目标,没有夹带改动。 我无法签字认可、因此不予批准的原因:
因此:不批准、也不要求修改——代码本身没有可阻塞的问题。 ⏸️ 转交 @wenshao 及 — Qwen Code · qwen3.8-max Reviewed at |
|
🤝 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. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
| if (provider.supportsModelDiscovery) { | ||
| return <DiscoveringModelIdsStep config={provider} flow={flow} />; | ||
| } |
There was a problem hiding this comment.
[Critical] This gate defers mounting the model editor behind a real network fetch, which breaks two existing Token Plan tests in AuthDialog.test.tsx (a file this PR does not touch) and turns them into real-network tests against the production endpoint.
AuthDialog renders the Step 3/3 · Model IDs header from flow state independently of the step body, so the tests' pressEnterAndWaitFor passes while DiscoveringModelIdsStep is still on the loading screen. AuthDialog.test.tsx has no mock of discoverProviderModels (only ProviderSetupSteps.test.tsx mocks the core module), so a real fetchWithPolicy request goes to the production Token Plan /models endpoint with the fake bearer key. While loading, no keypress handler consumes Enter (ModelIdsStep is not mounted yet; the ProviderSetupSteps handler is only active for advancedConfig/review), so the submit test's \r is dropped.
Witness — A/B run at this commit, same environment both arms, CI= unset:
BASE: ✓ should submit Token Plan through the shared subscription handler 425ms
BASE: ✓ should pre-fill the Model IDs step with previously saved custom model IDs 440ms
PR: × should submit Token Plan… → expected "spy" to be called at least once (vi.waitFor timeout, 5427ms)
PR: × should pre-fill… → frame stuck on "Loading models from provider… │ Esc to go back" (deterministic, 448ms)
Both tests are skipped when process.env.CI === 'true' (itWhenTuiInputReliable), which is why CI stays green while local/non-CI runs of the auth suite fail deterministically — and no CI surface exercises the discovery path at all.
Suggested fix: mock discoverProviderModels in AuthDialog.test.tsx the same way ProviderSetupSteps.test.tsx does — vi.mock('@qwen-code/qwen-code-core', ...) with importOriginal, the stub resolving null — and have the two tests wait for the editor to mount (e.g. the Enter model IDs directly text) before asserting or sending Enter.
中文说明
这个 gate 把模型编辑器的挂载推迟到一次真实网络请求之后,破坏了 AuthDialog.test.tsx(本 PR 未改动的文件)中两个既有的 Token Plan 测试,并使它们变成对生产端点发起真实网络请求的测试。
AuthDialog 从 flow state 渲染 Step 3/3 · Model IDs 标题时独立于步骤内容,因此测试的 pressEnterAndWaitFor 在 DiscoveringModelIdsStep 仍停留在加载界面时就已经通过。AuthDialog.test.tsx 没有 mock discoverProviderModels(只有 ProviderSetupSteps.test.tsx mock 了 core 模块),于是会用伪造的 bearer key 向生产环境的 Token Plan /models 端点发起真实的 fetchWithPolicy 请求。加载期间没有任何按键处理器消费 Enter(ModelIdsStep 尚未挂载;ProviderSetupSteps 的处理器只在 advancedConfig/review 阶段激活),因此提交测试的 \r 被丢弃。
见证——在本提交上进行的 A/B 运行,两臂使用相同环境且未设置 CI=:两个测试在 merge base 上通过,在本 PR 上确定性失败(一个因 vi.waitFor 超时,一个在断言时帧仍停留在 "Loading models from provider… │ Esc to go back")。
这两个测试在 process.env.CI === 'true' 时被跳过(itWhenTuiInputReliable),所以 CI 保持绿色,而本地/非 CI 环境运行 auth 测试套件会确定性失败——并且没有任何 CI 环节执行到发现(discovery)路径。
修复建议:在 AuthDialog.test.tsx 中用与 ProviderSetupSteps.test.tsx 相同的方式 mock discoverProviderModels——带 importOriginal 的 vi.mock('@qwen-code/qwen-code-core', ...),stub 解析为 null——并让这两个测试在断言或发送 Enter 之前先等待编辑器挂载(例如等待 Enter model IDs directly 文本出现)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| /(^|[-_.])(asr|audio|embed|embedding|image|moderation|rerank|speech|tts|video)([-_.]|$)/i; | ||
| const NON_CHAT_MODEL_FAMILY = | ||
| /(cosyvoice|sambert|paraformer|whisper|stable-diffusion|(^|[-_.])flux([-_.]|$))/i; | ||
| const MEDIA_MODEL_PREFIX = /^wan(?:\d|x[-_.]|[-_.])/i; |
There was a problem hiding this comment.
[Suggestion] The non-chat filter misses current-generation media-model naming. MEDIA_MODEL_PREFIX matches wan+digit and wanx+separator, but not wanx immediately followed by a digit, and the part/family blocklists have no t2i/t2v tokens or bge/sensevoice families. Probe-confirmed by driving the real discoverProviderModels with a mocked transport: wanx2.1-t2i-turbo, wanx2.1-t2v-turbo, bge-m3, sensevoice-v1 all pass the filter and land in "Recommended models · from the provider", while legacy wanx-v1 and wan2.7-image are caught — same family, one generation caught, the next slips through. A user who checks one persists a non-chat model as a chat model, and later chat-completion calls against it fail.
Witness:
PR: surviving ids = ["qwen-chat-control","wanx2.1-t2i-turbo","wanx2.1-t2v-turbo","bge-m3","sensevoice-v1"]
FIX: /^wan(?:\d|x[-_.\d]|[-_.])/i plus (bge|sensevoice) family tokens → surviving ids = ["qwen-chat-control"]
This stays a Suggestion because discovered IDs are never auto-checked (initial selections are the intersection with built-ins only), so impact requires manually selecting a misclassified entry. The blocklist-over-unbounded-space design never converges, though, so also consider failing closed (only IDs intersecting built-in specs count as recommendations) or dropping the filter entirely — noise is cheaper than misclassification when nothing is auto-selected.
| const MEDIA_MODEL_PREFIX = /^wan(?:\d|x[-_.]|[-_.])/i; | |
| const MEDIA_MODEL_PREFIX = /^wan(?:\d|x[-_.\d]|[-_.])/i; |
中文说明
非聊天模型过滤器漏掉了新一代媒体模型命名。MEDIA_MODEL_PREFIX 能匹配 wan+数字和 wanx+分隔符,但匹配不到紧跟数字的 wanx;part/family 黑名单也缺少 t2i/t2v 词元以及 bge/sensevoice 系列。通过用 mock 传输层驱动真实的 discoverProviderModels 进行探针验证:wanx2.1-t2i-turbo、wanx2.1-t2v-turbo、bge-m3、sensevoice-v1 全部通过过滤器并进入 "Recommended models · from the provider",而旧版 wanx-v1 和 wan2.7-image 被过滤——同一系列,抓住了上一代,漏掉了下一代。用户若勾选其中一项,会把非聊天模型作为聊天模型持久化,之后对它的聊天补全调用会失败。
由于发现到的 ID 从不被自动勾选(初始勾选仅取与内置列表的交集),影响需要用户手动误选,因此严重级别维持 Suggestion。但针对无界输入空间的手工黑名单永远不会收敛,也可以考虑收紧策略(只把与内置规格相交的 ID 作为推荐)或干脆去掉过滤——在没有任何自动勾选的前提下,列表里多一些噪音比误分类代价更低。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const knownModels = staticModels.filter((model) => | ||
| discoveredIds.has(model.id), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] A previously selected built-in model that the account's /models listing does not serve becomes invisible everywhere in the models step and is silently removed from settings on submit. mergeModelSpecs filters it out of the snapshot, getCustomModelIdsText excludes it from the free-form input precisely because it is a built-in id, and no option row exists for it — the selection has no representation anywhere. Realistic triggers: entitlement-scoped listings (the static lists themselves describe "Pro subscribers only" models), the China-vs-Intl region catalogs this wizard switches between, and a transiently partial or paginated listing (readModelIds reads only data and ignores pagination markers).
Your own test demonstrates the mechanism: with modelIds: 'custom-model, MiniMax-M3, MiniMax-M2.7' and a snapshot omitting MiniMax-M2.7, submit produces ['xcustom-model', 'MiniMax-M3'] — the prior selection is gone. "Built-in IDs absent from the catalog are no longer recommended" covers de-recommendation, but not silently dropping a previously selected id from settings with no trace. Consider preserving previously selected built-ins the snapshot does not serve (seed them into the custom-ids input or keep them as visible rows), or at least make the drop visible before submit.
中文说明
账号 /models 列表中未提供的、此前已被选中的内置模型,会在模型步骤中完全不可见,并在提交时被从设置中静默移除。mergeModelSpecs 把它从快照中过滤掉;getCustomModelIdsText 恰恰因为它是内置 ID 而将其排除在自由输入框之外;同时也不存在对应的选项行——该选择在任何地方都没有表示。现实触发场景:按权益范围返回的列表(静态列表本身就描述了 "Pro 订阅专属" 模型)、本向导可切换的中国区/国际区目录、以及瞬时不完整或分页的列表(readModelIds 只读取 data,忽略分页标记)。
你们自己的测试演示了这个机制:当 modelIds: 'custom-model, MiniMax-M3, MiniMax-M2.7' 且快照缺少 MiniMax-M2.7 时,提交结果为 ['xcustom-model', 'MiniMax-M3']——之前的选择丢失了。"目录中缺失的内置 ID 不再推荐"覆盖的是取消推荐,但不覆盖在毫无提示的情况下把一个已被选中的 ID 从设置中移除。建议保留快照未提供但此前已选中的内置模型(把它们放进自定义输入框,或保持为可见选项行),至少应在提交前让这种移除对用户可见。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const trimmedId = id.trim(); | ||
| if ( | ||
| trimmedId && |
There was a problem hiding this comment.
[Suggestion] readModelIds accepts any non-empty trimmed string — no validation for characters the downstream model-id pipeline treats as structural (comma) or for control sequences. This diff is the first path where model IDs travel from a network response into settings.json and the TUI. A served id "a, b" survives the first submit as one entry (the override array is used as-is), but on the next wizard run start() rebuilds state via [...defaultIds, ...customIds].join(', ') and normalizeModelIds splits on commas — one served id silently becomes two bogus models persisted to settings. Control/ANSI bytes in a served id render raw through formatModelOptionLabel → <Text> and persist into every UI that lists configured models. The trigger is attenuated (both presets pin baseUrl to fixed https option lists, so it takes a buggy or compromised listing), but this ingestion point is the cheap place to guard.
| const trimmedId = id.trim(); | |
| if ( | |
| trimmedId && | |
| const trimmedId = id.trim(); | |
| if ( | |
| trimmedId && | |
| !/[\u0000-\u001f\u007f,]/.test(trimmedId) && |
中文说明
readModelIds 接受任何非空的去空白字符串——没有校验下游模型 ID 管线视为结构化字符的逗号,也没有校验控制字符。这个 diff 是模型 ID 第一次从网络响应进入 settings.json 和 TUI 的路径。服务端返回的 ID "a, b" 在第一次提交时会作为一个条目存活(override 数组被原样使用),但下一次运行向导时 start() 通过 [...defaultIds, ...customIds].join(', ') 重建状态,而 normalizeModelIds 按逗号拆分——一个服务端 ID 会静默变成两个被持久化到设置的假模型。服务端 ID 中的控制/ANSI 字节会原样通过 formatModelOptionLabel → <Text> 渲染,并持久化到每个列出已配置模型的界面。触发条件有所削弱(两个 preset 都把 baseUrl 固定为 https 选项,需要列表本身有缺陷或被攻陷),但这个入口正是成本最低的设防点。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| expect.objectContaining({ | ||
| timeoutMs: 5000, | ||
| maxBytes: 1024 * 1024, |
There was a problem hiding this comment.
[Suggestion] The bounded-request contract is pinned for 2 of its 3 parameters — this assertion checks timeoutMs: 5000 and maxBytes: 1024 * 1024, but maxRedirects: 2 from the same options literal is never asserted, so any change to it ships green. fetchWithPolicy iterates for (let hop = 0; hop <= options.maxRedirects; hop++), so a future maxRedirects: 2 → 0 edit makes a provider whose /models endpoint issues even one benign same-host redirect fail with EMAXREDIRECTS, silently degrading every such account to the static built-in list while the suite stays green.
Witness — mutation-probed in a scratch tree:
PR baseline: 12 passed (12)
mutation maxRedirects: 2 → 0: 12 passed (12) ← mutation survives
mutation + fix below: 1 failed | 11 passed ← fix discriminates
| expect.objectContaining({ | |
| timeoutMs: 5000, | |
| maxBytes: 1024 * 1024, | |
| expect.objectContaining({ | |
| timeoutMs: 5000, | |
| maxBytes: 1024 * 1024, | |
| maxRedirects: 2, |
中文说明
有界请求契约的三个参数中只有两个被固定——此断言检查了 timeoutMs: 5000 和 maxBytes: 1024 * 1024,但同一个选项字面量中的 maxRedirects: 2 从未被断言,因此对它的任何修改都会在测试全绿的情况下上线。fetchWithPolicy 以 for (let hop = 0; hop <= options.maxRedirects; hop++) 迭代,因此未来把 maxRedirects 改为 0 会让任何 /models 端点即便只发一次良性同域重定向的 provider 以 EMAXREDIRECTS 失败,并把这类账号静默降级到静态内置列表,而测试套件依然全绿。
见证——在临时树中进行的变异测试:基线 12 个测试全部通过;变异 maxRedirects: 2 → 0 后仍然 12 个全部通过(变异存活);加上下方的修复后 1 个失败、11 个通过(修复可以区分变异)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Autofix round 1 — all five findings addressed[rc:3849825673] Critical — model-discovery gate breaks two Token Plan tests in
|
| Probe | Result |
|---|---|
| Revert filter regex/tokens + remove unsafe-char guard | 3 tests fail (falls back when all returned ids are non-chat models, both unsafe-id tests); restored → 14/14 |
Product maxRedirects: 2 → 0 |
merge test fails on the new assertion; restored → 14/14 |
Revert getCustomModelIdsText to built-in exclusion |
updated snapshot test fails; restored → 10/10 |
| Critical: the two Token Plan tests on the pre-round head | both fail (witness above); with the fix both pass |
Note on one unrelated local failure
Running the full AuthDialog.test.tsx with CI= unset shows one additional failure: drives API key provider steps from endpoint options metadata. I verified it is not caused by this round or this PR: it fails identically with the pre-round head (without my test change) and after swapping packages/cli + packages/core to origin/main sources and rebuilding (same assertion: expected frame to match /›\s*(?:\d+\.\s*)?MiniMax API Key/). It is an environment-specific TUI-input timing failure of the kind the itWhenTuiInputReliable guard exists for (the test is skipped when CI=true), and it exercises only third-party provider navigation, which this PR does not touch. In CI=true mode the suite is fully green.
Conflict notes
None — --conflict false; no merge performed.
Verification
npm run build— passed (exit 0)npm run typecheck— passed (exit 0)npm run lint— passed (exit 0, no findings)prettier --checkon the five touched files — passedvitest run src/providersinpackages/core— 16 files, 168 passedvitest run src/ui/auth/ProviderSetupSteps.test.tsxinpackages/cli(CI=unset) — 10 passedvitest run src/ui/auth/AuthDialog.test.tsxinpackages/cliwithCI=true— 7 passed | 18 skipped (green)vitest run src/ui/auth/AuthDialog.test.tsxinpackages/cliwithCI=unset — 24 passed | 1 failed (the pre-existing MiniMax endpoint navigation failure reproduced identically onorigin/main, see note above); the two Token Plan tests from the Critical finding both pass- No settings source changed →
npm run generate:settings-schemanot required
中文说明
Autofix 第 1 轮 — 五项发现全部处理
[rc:3849825673] Critical — 模型发现 gate 破坏 AuthDialog.test.tsx 中两个 Token Plan 测试 → 已修复
在轮次前的 HEAD 上、未设置 CI= 的环境下复现,与评审见证完全一致:
should submit Token Plan through the shared subscription handler→expected "spy" to be called at least once(vi.waitFor 超时)should pre-fill the Model IDs step with previously saved custom model IDs→ 帧停留在 "Loading models from provider…"
修复采用评审建议的形态:AuthDialog.test.tsx 现在通过带 importOriginal 的 vi.mock('@qwen-code/qwen-code-core', …) mock discoverProviderModels(与 ProviderSetupSteps.test.tsx 相同的模式),stub 解析为 null(回退到内置列表),两个测试在发送 Enter / 断言之前先等待编辑器挂载(Enter model IDs directly)。该测试套件不再向生产 /models 端点发起真实网络请求。两个测试在未设置 CI= 的环境下均已通过。
[rc:3849825677] Suggestion — 非聊天过滤器漏掉新一代媒体模型 → 已修复
采用评审建议的正则与词元:MEDIA_MODEL_PREFIX 改为 /^wan(?:\d|x[-_.\d]|[-_.])/i(可捕获 wanx2.1-…),NON_CHAT_MODEL_PART 增加 t2i/t2v,NON_CHAT_MODEL_FAMILY 增加 sensevoice 以及带锚定的 (^|[-_.])bge([-_.]|$)(与 flux 一样加锚定,避免子串误伤)。falls back when all returned ids are non-chat models 测试现在会喂入 wanx2.1-t2i-turbo、wanx2.1-t2v-turbo、bge-m3、sensevoice-v1。保留黑名单设计、未采用收紧策略/去掉过滤器的替代方案:改变推荐语义属于产品决策,而补齐已点名的代际漏洞不属于。
[rc:3849825691] Suggestion — 快照未提供的已选内置模型被静默移除 → 已修复
实现了评审的第一选项(放进自定义输入框)。getCustomModelIdsText 现在只排除已有预勾选推荐行表示的 ID,而不是排除所有内置 ID,因此账号列表未提供的、此前已选的内置模型会出现在自由输入框中并在提交时保留。快照已提供的模型、以及非发现/回退路径的行为与之前完全一致(由相邻未改动的测试验证)。快照测试已更新以固定新契约:MiniMax-M2.7 没有选项行,但在输入框中可见且包含在提交结果中(['xcustom-model', 'MiniMax-M2.7', 'MiniMax-M3'])。这是发现所要求的有意的行为变更,而不是弱化断言——更新后的测试比旧测试固定了更多内容。
[rc:3849825694] Suggestion — readModelIds 接受逗号/控制字节 → 已修复
新增 UNSAFE_MODEL_ID_CHARS = /[,\u0000-\u001f\u007f]/,跳过任何匹配的服务端 ID,并按代码库惯例添加 eslint-disable-next-line no-control-regex 注释(控制字节正是该正则要拒绝的对象)。新测试固定:"a, b"、含 ESC 序列的 ID、含 DEL 的 ID 被跳过而合法 ID 保留;全部 ID 均不安全时回退到内置列表。
[rc:3849825698] Suggestion — maxRedirects: 2 从未被断言 → 已修复
完全按建议在 fetchWithPolicy 的 objectContaining 断言中增加 maxRedirects: 2。
变异探针(每个守卫都有自己的见证)
| 探针 | 结果 |
|---|---|
| 回退过滤器正则/词元 + 移除不安全字符守卫 | 3 个测试失败(falls back when all returned ids are non-chat models、两个不安全 ID 测试);恢复后 14/14 |
产品代码 maxRedirects: 2 → 0 |
merge 测试在新断言上失败;恢复后 14/14 |
将 getCustomModelIdsText 回退为按内置排除 |
更新后的快照测试失败;恢复后 10/10 |
| Critical:两个 Token Plan 测试在轮次前 HEAD 上 | 均失败(见上方见证);修复后均通过 |
关于一个无关的本地失败
在未设置 CI= 的环境下运行完整的 AuthDialog.test.tsx 会出现一个额外失败:drives API key provider steps from endpoint options metadata。我已确认它不是本轮或本 PR 引入的:在轮次前 HEAD(不含我的测试改动)上它以完全相同的方式失败;把 packages/cli + packages/core 换成 origin/main 的源码并重新构建后同样失败(相同断言:expected frame to match /›\s*(?:\d+\.\s*)?MiniMax API Key/)。这是一个环境相关的 TUI 输入时序失败,正是 itWhenTuiInputReliable 守卫存在的意义(CI=true 时该测试被跳过),且它只涉及本 PR 未改动的第三方 provider 导航。在 CI=true 模式下该套件全绿。
冲突说明
无 — --conflict false;未执行任何合并。
验证
npm run build— 通过(exit 0)npm run typecheck— 通过(exit 0)npm run lint— 通过(exit 0,无发现)- 对五个改动文件运行
prettier --check— 通过 - 在
packages/core中运行vitest run src/providers— 16 个文件,168 个通过 - 在
packages/cli中运行vitest run src/ui/auth/ProviderSetupSteps.test.tsx(未设置CI=)— 10 个通过 - 在
packages/cli中以CI=true运行vitest run src/ui/auth/AuthDialog.test.tsx— 7 个通过 | 18 个跳过(全绿) - 在
packages/cli中以未设置CI=运行vitest run src/ui/auth/AuthDialog.test.tsx— 24 个通过 | 1 个失败(即上文说明的、在origin/main上同样复现的既有 MiniMax 端点导航失败);Critical 发现中的两个 Token Plan 测试均已通过 - 未改动任何 settings 源 → 无需运行
npm run generate:settings-schema
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/auth/ProviderSetupSteps.tsx:334 — [probe] stale modelIdsError banner never cleared on discovery-path editspackages/cli/src/ui/auth/ProviderSetupSteps.tsx:460 — [probe] no test pins the absence of the discovery source suffix on the non-discovery pathpackages/core/src/providers/model-discovery.ts:113 — [review] 401 (wrong key) indistinguishable from outage in the generic fallback message
中文说明
已审查——无阻断问题。 建议见行内评论。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const NON_CHAT_MODEL_PART = | ||
| /(^|[-_.])(asr|audio|embed|embedding|image|moderation|rerank|speech|t2i|t2v|tts|video)([-_.]|$)/i; |
There was a problem hiding this comment.
[Suggestion] These three filter regexes curate recommendations with a hand-maintained enumeration of non-chat model families, but the entrance space — ids served by the account's /models — is unbounded, so the list has no last corner and both error directions fail silently. This supersedes the round-1 finding on this filter: that round named a specific media-model naming gap, the autofix patched it by extending this very list, and the next corners are already demonstrable. Executing the regexes at this commit: qwen-vl-ocr-latest (a single-task OCR-extraction family) passes all three filters and is returned as a recommended chat model, while qwen2-audio-instruct (an audio-input chat model) is dropped by the audio token and becomes invisible in the recommendations. The documented heuristic rationale answers the drop direction (filtered ids remain free-form enterable), but not the wrong-endorsement direction, where a non-chat model receives positive endorsement and, if selected, becomes the chat model with no uncertainty signal. Any preset that later sets supportsModelDiscovery silently inherits this taxonomy.
Witness — probe at this commit against the real discoverProviderModels:
input {data:[{id:'qwen-vl-ocr-latest'}]} → returned [{ id: 'qwen-vl-ocr-latest' }] (endorsed)
input {data:[{id:'qwen2-audio-instruct'}]} → returned null (dropped)
Suggested direction (spans the merge/render sites, so no one-click block): endorse only the built-in specs as "recommended", render served-but-unclassified ids without endorsement (e.g. a separate heading, or unfiltered) instead of dropping them, and delete the family regexes — then no future family can produce a wrong endorsement or hide a model.
中文说明
这三个过滤正则用人工维护的非聊天模型家族枚举来筛选推荐,但入口空间——账号 /models 返回的 id——是无界的,因此枚举永远补不完,且两个错误方向都会静默失败。本条取代第一轮针对同一过滤器的发现:那一轮指出了具体的媒体模型命名缺口,autofix 通过扩展这个列表来修补,而新的缺口已经可以演示。在本提交上执行这些正则:qwen-vl-ocr-latest(单任务 OCR 抽取家族)通过全部三个过滤,被作为推荐聊天模型返回;而 qwen2-audio-instruct(音频输入聊天模型)被 audio 词条丢弃,在推荐中不可见。文档中“启发式过滤、被过滤 ID 仍可自由输入”的说明只回应了丢弃方向,未回应错误背书方向——非聊天模型获得正面背书,一旦被勾选就会成为聊天模型且没有任何不确定性提示。未来任何设置 supportsModelDiscovery 的 preset 都会静默继承这套分类法。
见证——在本提交上对真实 discoverProviderModels 运行探针:输入 {data:[{id:'qwen-vl-ocr-latest'}]} 返回 [{ id: 'qwen-vl-ocr-latest' }](被背书);输入 {data:[{id:'qwen2-audio-instruct'}]} 返回 null(被丢弃)。
建议方向(跨合并/渲染多处,故不提供一键应用块):只把内置规格背书为“推荐”,对服务端返回但未分类的 id 不做背书地展示(例如单独标题或不外过滤)而不是丢弃,并删除家族正则——这样未来任何新家族都不会产生错误背书或隐藏模型。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const trimmedId = id.trim(); | ||
| if ( | ||
| trimmedId && | ||
| !UNSAFE_MODEL_ID_CHARS.test(trimmedId) && |
There was a problem hiding this comment.
[Suggestion] The validation guard checks characters but not length: the 1 MiB response budget can become a single ~1 MiB model id (no commas/control bytes) that passes every check here, merges into the snapshot, and is rendered raw — formatModelOptionLabel applies padEnd with no truncation, wrapped to terminal width for up to 8 visible options and re-diffed on every keystroke. If selected, the string is also written to settings.json and sent as the model field on subsequent chat requests; normalizeModelIds and submitModelIds apply no length limit either. The trigger is a compromised or misconfigured endpoint behind one of the two fixed preset URLs — inside this feature's own threat model, which the character filter was added for.
Witness — probe at this commit against the real readModelIds/discoverProviderModels: serving 'a'.repeat(1_000_000) as a single id returned it unchanged (1 id, length 1,000,000).
| !UNSAFE_MODEL_ID_CHARS.test(trimmedId) && | |
| trimmedId.length <= 256 && | |
| !UNSAFE_MODEL_ID_CHARS.test(trimmedId) && |
An id longer than that is never a real model name; over-length ids can alternatively fail the whole listing to the built-in fallback.
中文说明
该校验守卫检查了字符但没有检查长度:1 MiB 的响应预算可以变成一个约 1 MiB 的单个模型 id(不含逗号/控制字节),它能通过这里的所有检查、并入快照并被原样渲染——formatModelOptionLabel 只做 padEnd 不做截断,按终端宽度换行后最多 8 个可见选项、每次按键都重新 diff 重绘,实际上会冻结配置向导。若被选中,该字符串还会写入 settings.json 并作为后续聊天请求的 model 字段发送;normalizeModelIds 和 submitModelIds 也没有长度限制。触发条件是固定 preset URL 背后被入侵或配置错误的端点——这属于本功能自身的威胁模型,字符过滤器正是为此而加的。
见证——在本提交上对真实 readModelIds/discoverProviderModels 运行探针:提供 'a'.repeat(1_000_000) 作为单个 id,原样返回(1 个 id,长度 1,000,000)。
建议应用上面的代码块,在守卫中加入长度上限(例如 <= 256);超过该长度的 id 不可能是真实模型名,也可以选择让整个列表失败并回退到内置列表。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // The wizard joins ids with commas and renders them raw, so a served id with | ||
| // a comma or control bytes would split into bogus models or poison the TUI. | ||
| // eslint-disable-next-line no-control-regex -- control bytes are exactly what this rejects | ||
| const UNSAFE_MODEL_ID_CHARS = /[,\u0000-\u001f\u007f]/; |
There was a problem hiding this comment.
[Suggestion] The TUI-poisoning filter added in the previous round rejects commas and control bytes but misses invisible Unicode format/zero-width characters (U+200B, U+200C/D, U+202A–E, U+2060–2069, U+00AD, interior U+FEFF — trim() only strips edge ones), so a served id can render visually identical to a legitimate one. The endpoint can serve both qwen3.7-plus and qwen3.7\u200b-plus; both pass all four regexes and the trim, so the wizard renders two recommendation rows whose ID columns are pixel-identical, and if the lookalike is selected, the invisible-character id lands in settings.json (modelProviders, model.name) where it never matches the real id while remaining visually indistinguishable. The filter's own comment states its purpose is stopping served ids from poisoning the TUI; invisible characters are exactly that channel.
Witness — probe at this commit against the real function:
served {data:[{id:'qwen3.7-plus'},{id:'qwen3.7\u200b-plus'}]} → returned BOTH ids (no dedupe)
served {id:'\u200bqwen-lookalike'} → returned unchanged (trim() keeps U+200B)
| const UNSAFE_MODEL_ID_CHARS = /[,\u0000-\u001f\u007f]/; | |
| const UNSAFE_MODEL_ID_CHARS = /[,\u0000-\u001f\u007f\u00ad\u200b-\u200f\u202a-\u202e\u2060-\u2069\ufeff]/; |
中文说明
上一轮加入的 TUI 污染过滤器拒绝了逗号和控制字节,但漏掉了不可见的 Unicode 格式/零宽字符(U+200B、U+200C/D、U+202A–E、U+2060–2069、U+00AD、内部的 U+FEFF——trim() 只去除首尾的),因此服务端返回的 id 可以渲染得与合法 id 完全相同。端点可以同时提供 qwen3.7-plus 和 qwen3.7\u200b-plus;两者都通过全部四个正则和 trim,向导会渲染出 ID 列逐像素相同的两行推荐,若选中了那个“孪生” id,带不可见字符的 id 会落入 settings.json(modelProviders、model.name),永远无法匹配真实 id,却又无法从视觉上区分。过滤器自身的注释写明其目的是阻止服务端 id 污染 TUI;不可见字符正是这条通道。
见证——在本提交上对真实函数运行探针:提供 {data:[{id:'qwen3.7-plus'},{id:'qwen3.7\u200b-plus'}]} 返回两个 id(未去重);提供 {id:'\u200bqwen-lookalike'} 原样返回(trim() 保留 U+200B)。
建议应用上面的代码块,把拒绝正则扩展到零宽/格式字符。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Review round 2 — address summary (PR #9980)All three inline Feedback points[rc:3851459630] Hand-maintained family taxonomy — RESOLVEDReproduced at the pre-round commit: Following the suggested direction:
Witnesses: the rewritten merge test now returns [rc:3851459636] No length bound on model ids — RESOLVEDReproduced: serving Added [rc:3851459653] Invisible Unicode format characters bypass the TUI-poisoning filter — RESOLVEDReproduced: Extended Deleted test — content evidence
Reviewer-deferred probes — no action this roundThe review body lists three probes "Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round" (stale Pre-existing local test failure — not introduced by this round
Verification
中文说明审查第 2 轮——处理摘要(PR #9980)三条行内 反馈点[rc:3851459630] 手工维护的模型家族分类法——已解决在本轮前的提交上复现: 按建议方向处理:
见证:重写后的合并测试现在原样返回 [rc:3851459636] 模型 id 没有长度上限——已解决复现:下发单个 新增 [rc:3851459653] 不可见 Unicode 格式字符绕过了 TUI 污染过滤器——已解决复现: 扩展 被删除的测试——内容证据
审查者延后的探针——本轮不处理审查正文列出了三条"在收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改"的探针(过期的 本地既有测试失败——非本轮引入
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Convergence: round 3 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 3 (3 new). Findings keep coming back to the same files: packages/core/src/providers/model-discovery.ts (findings in round 2; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛情况:第 3 轮发布了 2 条行内评论,其中 2 条是首次提出;上一轮发布了 3 条(其中 3 条首次提出)。发现反复回到同一批文件:packages/core/src/providers/model-discovery.ts(第 2 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const UNSAFE_MODEL_ID_CHARS = | ||
| // eslint-disable-next-line no-control-regex -- control bytes are exactly what this rejects | ||
| /[,\u0000-\u001f\u007f\u00ad\u200b-\u200f\u202a-\u202e\u2060-\u2069\ufeff]/; |
There was a problem hiding this comment.
[Critical] This character class hand-enumerates Unicode danger ranges over an unbounded untrusted surface (the account's /models response), so whole categories of the characters its own comment targets pass validation, are rendered raw in the Model IDs step, and are persisted into provider settings. A probe driving the real discoverProviderModels (fetch mocked as in the suite), serving one id per claimed class, returned every one of them to the UI at this commit: U+2028/U+2029 line/paragraph separators, the C1 controls U+009B (single-byte CSI), U+0085 (NEL), U+0090 (DCS), U+009C (ST), plus U+061C and U+180E — with U+034F, variation selectors and homoglyph letters passing as well. The repo's own sanitizers treat exactly these as dangerous: terminalSafe.ts collapses [\x00-\x1f\x7f-\x9f] and suggestionGenerator.ts rejects the same range for terminal-rendered text. This is the same surface R2-3 patched one range-set at a time, so a compromised or MITM'd catalog endpoint — the threat the neighbouring test's control-byte fixture guards against — can still poison the TUI (on a terminal honouring 8-bit C1 controls a served U+009B id executes as a CSI sequence) and persist the poisoned id to settings. Close the class instead of enumerating it: with /[,\p{Cc}\p{Cf}\p{Zl}\p{Zp}]/u patched in, the same probe reports all eight primary entrances rejected while good-model still passes (probe flipped). Note U+034F/U+FE0F (Mn) and homoglyphs still pass property escapes — treat this as the floor, not the full remedy.
| const UNSAFE_MODEL_ID_CHARS = | |
| // eslint-disable-next-line no-control-regex -- control bytes are exactly what this rejects | |
| /[,\u0000-\u001f\u007f\u00ad\u200b-\u200f\u202a-\u202e\u2060-\u2069\ufeff]/; | |
| const UNSAFE_MODEL_ID_CHARS = /[,\p{Cc}\p{Cf}\p{Zl}\p{Zp}]/u; |
中文说明
这个字符类在无界且不可信的表面(账号 /models 响应)上手工枚举 Unicode 危险区间,因此其注释所要拦截的整类字符都能通过校验,被原样渲染到 Model IDs 步骤,并持久化进 provider 设置。在该提交上驱动真实 discoverProviderModels(fetch 按测试套件方式 mock)、逐类投喂单个 id 的探针显示,它们全部被返回给 UI:U+2028/U+2029 行/段分隔符、C1 控制字符 U+009B(单字节 CSI)、U+0085(NEL)、U+0090(DCS)、U+009C(ST),以及 U+061C、U+180E——此外 U+034F、变体选择符与同形字母同样通过。仓库自己的净化器恰恰把这些视为危险:terminalSafe.ts 收敛 [\x00-\x1f\x7f-\x9f],suggestionGenerator.ts 对终端渲染文本拒绝相同区间。这与 R2-3 逐区间修补的是同一个表面,因此被攻破或中间人的目录端点(邻近测试中控制字节夹具所防御的威胁)依然能毒害 TUI(在支持 8 位 C1 控制符的终端上,服务端投喂的 U+009B id 会作为 CSI 序列执行),并把被污染的 id 持久化到设置。应关闭整个类别而不是继续枚举:打上 /[,\p{Cc}\p{Cf}\p{Zl}\p{Zp}]/u 后,同一探针报告八个主要入口全部被拒绝,而 good-model 仍通过(探针翻转)。注意 U+034F/U+FE0F(Mn)与同形字母仍能通过属性转义——请把它当作底线而非完整补救。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| it.each([ | ||
| response({}, 401), |
There was a problem hiding this comment.
[Suggestion] These two fixtures cannot detect deletion of the guard they exist to pin: with if (result.kind !== 'response' || result.status < 200 || result.status >= 300) return null; removed, the 401 body {} still fails readModelIds' shape check and the cross-host-redirect fixture has no body field (so result.body.toString throws into the catch) — both paths still resolve to null. Probe at this commit: the guard-deletion mutant leaves the whole file green (15/15 tests pass), while changing the 401 fixture to carry a valid listing body kills it (expected [ { id: 'model-a' } ] to be null), and the improved fixture stays green against the real code (15/15). As it stands, a future refactor trusting readModelIds validation alone ships green, and an error response whose body happens to be listing-shaped (a gateway/proxy 4xx or 5xx JSON containing data: [{id: ...}]) would be rendered in the wizard as the provider's live catalog instead of falling back to built-ins. Give the failure fixture a valid listing body so the guard becomes the discriminating factor.
| it.each([ | |
| response({}, 401), | |
| it.each([ | |
| response({ data: [{ id: 'model-a' }] }, 401), |
中文说明
这两个夹具无法检测到它们所要钉住的守卫被删除:移除 if (result.kind !== 'response' || result.status < 200 || result.status >= 300) return null; 后,401 夹具的空响应体 {} 仍会被 readModelIds 的形状检查拒绝,而 cross-host-redirect 夹具没有 body 字段(result.body.toString 会抛进 catch)——两条路径仍解析为 null。在本提交上的探针:删除守卫的突变体让整个文件保持绿色(15/15 通过);把 401 夹具改为携带合法列表响应体后突变体被杀死(expected [ { id: 'model-a' } ] to be null),改进后的夹具对真实代码仍保持绿色(15/15)。照现状发展,未来某个只信任 readModelIds 校验的重构会一路绿灯地合入,而一个响应体恰好是列表形状的错误响应(网关/代理返回的、包含 data: [{id: ...}] 的 4xx/5xx JSON)将在向导中被渲染为 provider 的实时目录,而不是回退到内置列表。请给失败夹具一个合法的列表响应体,让守卫成为判别因素。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: Autofix review round summary — PR #9980Commit: Feedback points and dispositions[rc:3854486497] Critical — hand-enumerated Unicode danger ranges in
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/core/src/providers/__tests__/model-discovery.test.ts:84 — [probe] fail-closed reject-whole decision unpinned — no fixture mixes valid and malformed entriespackages/core/src/providers/model-discovery.ts:44 — [probe] per-id trim() has no surviving fixture — deletion leaves the suite greenpackages/core/src/providers/__tests__/model-discovery.test.ts:83 — [probe] it.each spreads the single-element array row — raw-array rejection path uncovered
Convergence: round 4 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: packages/core/src/providers/model-discovery.ts (findings in round 3; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查。 建议见行内评论。
收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 4 轮发布了 1 条行内评论,其中 1 条是首次提出;上一轮发布了 2 条(其中 2 条首次提出)。发现反复回到同一批文件:packages/core/src/providers/model-discovery.ts(第 3 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。本轮没有未决的 Critical,因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // The wizard joins ids with commas and renders them raw, so a served id with | ||
| // a comma, control byte, or invisible format character would split into bogus | ||
| // models or poison the TUI. | ||
| const UNSAFE_MODEL_ID_CHARS = /[,\p{Cc}\p{Cf}\p{Zl}\p{Zp}]/u; |
There was a problem hiding this comment.
[Suggestion] The property-class rewrite drops exactly one code point the old hand-enumerated ranges rejected: U+2065, which the old \u2060-\u2069 range swept up incidentally. U+2065 is unassigned (category Cn) and matches none of Cc/Cf/Zl/Zp, so a served model id containing it now passes validation, is merged into the wizard's comma-joined Model IDs list, rendered raw, and persisted to settings. A probe sweeping U+0000–U+10FFFF at this commit confirms it is the only dropped point — the change is otherwise a strict net hardening, rejecting 183 more code points (all C1 controls, the remaining Cf incl. U+061C/U+180E, Zl U+2028, Zp U+2029) — so this is the one monotonicity delta in a commit whose purpose was to widen the filter:
oldRe.test('\u2065')=true, newRe.test('\u2065')=false
OLD-rejects-and-NEW-accepts = exactly 1 code point (U+2065)
NEW additionally rejects 183 code points OLD accepted
Impact is bounded (the character has no control, bidi, or format semantics and renders visibly in standard terminals), so this is a note, not a blocker: either state in the comment that unassigned code points are deliberately out of scope, or close the last corner by rejecting all of category C:
| const UNSAFE_MODEL_ID_CHARS = /[,\p{Cc}\p{Cf}\p{Zl}\p{Zp}]/u; | |
| const UNSAFE_MODEL_ID_CHARS = /[,\p{C}\p{Zl}\p{Zp}]/u; |
中文说明
属性类重写恰好漏掉了一个旧手工枚举区间会拒绝的码位:U+2065——它是旧 \u2060-\u2069 区间顺带扫进来的。U+2065 是未分配码位(类别 Cn),不匹配 Cc/Cf/Zl/Zp 中的任何一类,因此包含它的模型 id 现在能通过校验,被合并进向导的逗号分隔 Model IDs 列表、原样渲染并持久化到设置。在本提交上对 U+0000–U+10FFFF 全量扫描的探针确认它是唯一被漏掉的码位——此改动在其他方面是严格的净强化,多拒绝了 183 个码位(全部 C1 控制字符、其余 Cf 含 U+061C/U+180E、Zl U+2028、Zp U+2029)——所以这是「以扩大过滤器为目的的提交」中唯一的单调性回退。影响有限(该字符没有控制、双向或格式化语义,在标准终端上可见地渲染),因此这是一条提示而非阻断:要么在注释里说明有意不覆盖未分配码位,要么通过拒绝整个 C 类关掉最后这个角。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 4/100 轮)。改动内容与我反驳保留之处如下: Autofix review round summary — PR #9980 (round 4)Commit: No base conflict was reported ( Feedback points and dispositions1. [rc:3856380340] [Suggestion] Property-class rewrite drops U+2065 — RESOLVED (implemented)The finding is real and was reproduced before changing anything:
2. Deferred probe: fail-closed reject-whole decision unpinned — RESOLVED (implemented)Added the missing mixed fixture 3. Deferred probe: per-id
|
| # | Temporary mutation | Expected witness | Result |
|---|---|---|---|
| 1 | Regex reverted to \p{Cc}\p{Cf} |
new unassigned/private-use test fails | FAILED as expected (U+2065 + U+E000 leaked through); restored → green |
| 2 | .trim() removed from readModelIds |
ordering/dedup test fails on ' padded-model ' |
FAILED as expected; restored → green |
| 3 | Reject-whole return null → continue (skip-and-keep) |
mixed row {data:[{id:'model-a'},null]} fails |
only that row FAILED, all other rows stayed green; restored → green |
| 4 | Raw arrays accepted (Array.isArray(value) → readModelIds({data: value})) |
original raw-array row fails | FAILED as expected — confirms pre-existing coverage (see item 4); restored → green |
Verification
npm run build— passed (exit 0)npm run typecheck— passed (exit 0)npm run lint— passed (exit 0);npx prettier --checkon both changed files — passednpx vitest run src/providers/(packages/core, touched) — 16 files, 172 passednpx vitest run src/ui/auth/(packages/cli, touched,CI=trueas in CI) — 3 files, 44 passed | 18 skipped (the skips areisUnreliableTuiInputEnvironment-gated TUI-input tests, skipped on every CI platform and win32; the gate comes from Feat/openrouter auth #3576, not this PR)- Local note: without
CI=true, one pre-existing gated test (drives API key provider steps from endpoint options metadata) fails locally on input-timing flakiness; it is skipped in CI by that gate and is not touched by this round. - Integration tests after
npm run bundle: not run — the touched behavior (model-id validation + wizard rendering) is fully covered by the unit suites above; nothing here is only exercisable through the bundled CLI.
中文说明
Autofix 评审轮次总结 — PR #9980(第 4 轮)
提交:34d352a6aa — fix(providers): close the unassigned-code-point gap in model id validation (#9980)(2 个文件,+22/−3)。
未报告基线冲突(--conflict false);未执行合并。
反馈点与处置
1. [rc:3856380340] [Suggestion] 属性类重写漏掉 U+2065 — 已解决(已实现)
该发现属实,且在改动前已复现:/[,\p{Cc}\p{Cf}\p{Zl}\p{Zp}]/u 会接受 U+2065(类别 Cn),而旧的手工区间 \u2060-\u2069 会拒绝它。对 U+0000–U+10FFFF 的全量扫描确认,建议的 \p{C} 形式是严格强化:没有新接受任何码位,新拒绝约 95.2 万个码位(全部为 Cn 未分配、Co 私有区、Cs 孤立代理项——旧类别同样从不拒绝孤立代理项)。实现了发现中的选项 (b),这也是更小的改动:
UNSAFE_MODEL_ID_CHARS→/[,\p{C}\p{Zl}\p{Zp}]/u(类别数比之前更少),注释同步更新。- 新增回归测试
skips ids with unassigned or private-use code points,含 U+2065 与 U+E000 夹具;若回退到Cc/Cf,测试套件会失败(已用变异探针验证,见下表)。
2. 延后探针:fail-closed 整体拒绝判定未被固定 — 已解决(已实现)
在 rejects a non-standard or empty listing 表中补上缺失的混合夹具 { data: [{ id: 'model-a' }, null] }:合法条目后跟一个畸形条目时,必须整体拒绝(fail closed),而不是保留合法前缀。变异探针确认它是有效见证(见下表)。
3. 延后探针:逐条 trim() 没有存活的夹具 — 已解决(已实现)
在排序/去重测试中加入 { id: ' padded-model ' },期望合并输出中出现 padded-model。现在删除 .trim() 会使该测试失败(已变异验证)。
4. 延后探针:「it.each 展开了单元素数组行——原始数组拒绝路径未被覆盖」— 已拒绝(有证据驳回)
该说法对本仓库所用的 vitest(3.2.7)不成立。@vitest/runner 中的 taskFn.each 仅在所有行都是数组时才把行展开为参数列表(arrayOnlyCases = cases.every(Array.isArray));否则每行原样作为单个参数传入。本表混合了数组行和对象行,因此原有的 [{ id: 'model-a' }] 行本来就传入原始数组 [{ id: 'model-a' }] 作为响应体——原始数组拒绝路径已被覆盖。证据:
- 源码:
node_modules/@vitest/runner/dist/chunk-hooks.js中taskFn.each(arrayOnlyCases分支)。 - 用相同表结构写的捕获测试显示:混合表中第 0 行按
[[…]]原样接收(未展开)。 - 加入「接受原始数组」的临时变异(
Array.isArray(value) → readModelIds({ data: value }))后,恰好只有该原始数组行失败,其余行全绿——即现有夹具确实固定了探针声称未覆盖的路径。 - 探针误读的可能原因:
%j标题把该行渲染为{"id":"model-a"},因为formatTitle格式化的是行的各个元素,脱掉了一层方括号,使原始数组行看起来像裸对象。
此项未做任何代码改动。
5. 失败检查:Test (macos-latest) / Test (windows-latest) — 已诊断,本 PR 足迹内没有证据支持的修复可做
- 这些平台通道自 2026-07-02 起一直处于关闭状态,直到今天:fix(ci): give the macOS and Windows lanes a trigger again #9370 于 11:53 UTC 重新启用触发器,fix: repair the Windows and macOS test lane failures #9728 于 11:49 UTC 修复了 72 个既有的平台失败(「reviving them in fix(ci): give the macOS and Windows lanes a trigger again #9370 exposed these pre-existing failures」)。失败的 merge-queue 运行开始于 17:45 UTC。
- 在修复与该运行之间,main 又进了 12 个提交,其中包括一个构建失败窗口(test(cli): teach session-swap telemetry fakes the /branch title prerequisites #9998 引入的 TS1117,由 fix(cli): drop duplicate fake properties breaking the main build #10022 于 14:08 修复)——平台通道正在追赶约 7 周积累的变化。
- 本 PR 的 diff 经审阅是平台无关的:纯校验逻辑加 Ink 字符串帧断言;数字格式化固定为
en-US;无平台/路径/时区/环境依赖;新测试所用的vi.mock(importOriginal)模式在仓库中已有 500+ 处成熟用法。同一次 CI 运行中,相同的合并树在 Ubuntu 通道为绿色。 - 本运行器无法访问 CI 日志(无 GitHub 凭据;
checks.json只有结论),因此无法点名具体失败的测试。本 PR 触及的所有测试在本地(Linux)全部通过。留给 CI 失败巡检/下一次 merge-queue 尝试处理;在此说明以便维护者对照通道结果阅读。
变异探针(本轮提交的见证验证)
| # | 临时变异 | 预期见证 | 结果 |
|---|---|---|---|
| 1 | 正则回退为 \p{Cc}\p{Cf} |
新的 unassigned/private-use 测试失败 | 按预期失败(U+2065 与 U+E000 漏过);恢复后全绿 |
| 2 | 从 readModelIds 删除 .trim() |
排序/去重测试在 ' padded-model ' 上失败 |
按预期失败;恢复后全绿 |
| 3 | 整体拒绝 return null → continue(跳过保留) |
混合行 {data:[{id:'model-a'},null]} 失败 |
仅该行失败,其余行保持绿色;恢复后全绿 |
| 4 | 接受原始数组(Array.isArray(value) → readModelIds({data: value})) |
原始数组行失败 | 按预期失败——确认既有覆盖(见第 4 项);恢复后全绿 |
验证
npm run build— 通过(exit 0)npm run typecheck— 通过(exit 0)npm run lint— 通过(exit 0);对两个改动文件执行npx prettier --check— 通过npx vitest run src/providers/(packages/core,触及包)— 16 个文件,172 通过npx vitest run src/ui/auth/(packages/cli,触及包,按 CI 环境设CI=true)— 3 个文件,44 通过 | 18 跳过(跳过项为isUnreliableTuiInputEnvironment门控的 TUI 输入测试,在所有 CI 平台与 win32 上均跳过;该门控来自 Feat/openrouter auth #3576,非本 PR 引入)- 本地说明:不设
CI=true时,一个既有门控测试(drives API key provider steps from endpoint options metadata)因输入时序抖动在本地失败;它在 CI 中由该门控跳过,且本轮未触及。 npm run bundle后的集成测试:未运行——触及的行为(model-id 校验 + 向导渲染)已被上述单元测试完全覆盖;此处没有任何只能通过打包 CLI 才能验证的内容。
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/core/src/providers/model-discovery.ts:107 — [review] all discovery failure modes collapse to a bare null with zero diagnostics
Convergence: round 5 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 1 (1 new). The rate of new findings is not falling. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查,审查缺口已披露。 建议见行内评论。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后(第 5 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 5 轮发布了 1 条行内评论,其中 1 条是首次提出;上一轮发布了 1 条(其中 1 条首次提出)。新发现的产出速度没有下降。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。本轮没有未决的 Critical,因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
| it('skips ids with unassigned or private-use code points', async () => { | ||
| fetchWithPolicyMock.mockResolvedValue( | ||
| response({ |
There was a problem hiding this comment.
[Suggestion] R5-1: The regex was widened to the full \p{C} supercategory this round, but the surrogate subcategory (\p{Cs}) has no fixture pinning it — no test id contains a lone surrogate. A mutation that drops surrogates (for example spelling the class out as \p{Cc}\p{Cf}\p{Co}\p{Cn}) survives the whole suite green, while a served catalog can deliver such an id: {"data":[{"id":"\ud800x"}]} is valid JSON, and JSON.parse yields the lone surrogate. If that rejection guarantee ever regresses, the id merges into the snapshot and is comma-joined and rendered raw in the wizard — the exact surface this regex exists to close. Measured at this commit: under that mutant all 18 shipped tests still pass and only a probe fixture fails (expected [ { id: '…x' }, { id: 'good-model' } ] to deeply equal [ { id: 'good-model' } ]), while the same probe passes against the real code. Add a lone-surrogate id to this test's data array (keeping the same toEqual([{ id: 'good-model' }]) expectation) so the \p{Cs} branch stays pinned:
{ id: 'surrogate\ud800point' },中文说明
本轮把正则放宽到了完整的 \p{C} 超类,但代理子类别(\p{Cs})没有任何夹具钉住——没有测试 id 包含孤立代理项。一个丢掉代理项的突变(例如把类别写成 \p{Cc}\p{Cf}\p{Co}\p{Cn})能让整个测试套件保持绿色,而服务端目录可以投喂这样的 id:{"data":[{"id":"\ud800x"}]} 是合法 JSON,JSON.parse 会解析出孤立代理项。如果这条拒绝保证将来回退,该 id 会并入快照,被逗号拼接并原样渲染到向导中——正是这个正则存在所要封堵的表面。在本提交上实测:应用该突变体后 18 个既有测试全部通过,只有探针夹具失败(expected [ { id: '…x' }, { id: 'good-model' } ] to deeply equal [ { id: 'good-model' } ]),而同一探针对真实代码保持绿色。请在本测试的 data 数组中加入一个孤立代理项 id(保持同样的 toEqual([{ id: 'good-model' }]) 期望),以钉住 \p{Cs} 分支:
{ id: 'surrogate\ud800point' },— qwen3.8-max via Qwen Code /review (v0.22.0)
The revived macOS and Windows test lanes check out the PR head, which predates b331509 (the lane repairs) — both lanes failed on those pre-existing platform test failures. Bring main in so the lanes test a tree that contains the fixes.
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 5/100 轮)。改动内容与我反驳保留之处如下: 🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 5/100 轮)。改动内容与我反驳保留之处如下: Autofix review round summary — PR #9980 (round 5)Commits this round (all additive, no history rewrite):
Feedback points and dispositions[rc:3857629635] Suggestion — the
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/core/src/providers/__tests__/model-discovery.test.ts:45 — [review] PR description still asserts a non-chat filter the code no longer has (undisclosed deviation from issue 9368 item 4)packages/cli/src/ui/auth/ProviderSetupSteps.tsx:507 — [probe] showOtherModelsHeading visibleIndex===0 branch untested — guard-deletion mutant survivespackages/cli/src/ui/auth/ProviderSetupSteps.tsx:341 — [probe] syncChangesToFlow=true default asserted only in the negative — default-flip mutant survivespackages/core/src/providers/model-discovery.ts:107 — [review] all discovery failure modes collapse to a bare null with zero diagnostics
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后(第 6 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| recommendationSource={snapshot.source} | ||
| syncChangesToFlow={false} |
There was a problem hiding this comment.
[Critical] On the discovery path ModelIdsStep is rendered with syncChangesToFlow={false}, so after an empty submit sets modelIdsError ("Model IDs cannot be empty.") the error is never cleared while the user types or toggles corrections: handleCustomModelIdsChange/toggleRecommendationAtIndex call the no-op syncModelIds, and only flow.changeModelIds clears the error (useProviderSetupFlow.ts:312-315). The stale banner persists until the next successful Enter, while on every non-discovery provider the first keystroke clears it. (The related "Esc discards edits" behavior was verified as documented design — the PR description states edits are committed only on Enter — so this comment targets only the error clearing.)
Witness (probe driving the real hook + component at this commit):
DISCOVERY ARM {"errorAfterTyping":"Model IDs cannot be empty.","bannerVisibleAfterTyping":true}
NON-DISCOVERY ARM {"errorAfterTyping":null,"bannerVisibleAfterTyping":false}
flip syncChangesToFlow={true} -> discovery arm {"errorAfterTyping":null,"bannerVisibleAfterTyping":false}
Fix (spans two files, so no one-click suggestion): add a clearModelIdsError action to useProviderSetupFlow and call it from handleCustomModelIdsChange/toggleRecommendationAtIndex on the discovery path — or have syncModelIds call flow.changeModelIds(flow.state.modelIds) (value unchanged, error cleared) when syncChangesToFlow is false — so the error clears on edit without re-enabling the suppressed sync.
中文说明
发现(discovery)路径以 syncChangesToFlow={false} 渲染 ModelIdsStep,因此空提交触发 modelIdsError("Model IDs cannot be empty.")之后,用户输入或切换纠正时该错误永远不会被清除:handleCustomModelIdsChange/toggleRecommendationAtIndex 调用的是空操作 syncModelIds,而只有 flow.changeModelIds 会清除该错误(useProviderSetupFlow.ts:312-315)。过期的错误横幅会一直保留到下一次成功回车,而在所有非发现路径的 provider 上,第一次按键就会清除它。见证:在本提交上驱动真实 hook + 组件的探针——发现路径输入后错误仍在、横幅可见;非发现路径输入后错误清空、横幅消失;把 syncChangesToFlow 翻转为 true 后发现路径同样翻转。(相关的"Esc 丢弃编辑"行为已核实为文档声明的设计——PR 描述写明编辑仅在回车时提交——因此本评论只针对错误清除。)
修复(跨两个文件,故不提供一键 suggestion):在 useProviderSetupFlow 中新增 clearModelIdsError 动作,并在发现路径的 handleCustomModelIdsChange/toggleRecommendationAtIndex 中调用;或让 syncModelIds 在 syncChangesToFlow 为 false 时调用 flow.changeModelIds(flow.state.modelIds)(值不变、错误被清除),从而在不重新启用被抑制同步的情况下实现编辑即清除错误。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const [selectedRecommendationKeys, setSelectedRecommendationKeys] = useState( | ||
| () => getRecommendedSelections(selectedModelIds, modelOptions), | ||
| () => | ||
| getRecommendedSelections(selectedModelIds, modelOptions, builtInModelIds), | ||
| ); | ||
| const [customModelIdsText, setCustomModelIdsText] = useState(() => | ||
| getCustomModelIdsText(selectedModelIds, selectedRecommendationKeys), | ||
| ); |
There was a problem hiding this comment.
[Critical] With a partial catalog, unserved built-in defaults are demoted into the free-form input (getCustomModelIdsText), and mergeModelIds puts input ids first — so a no-edit Enter submits a list starting with an unserved model, which buildInstallPlan (models[0] → modelSelection) and applyProviderInstallPlan (providers/install.ts) write as model.name on first-time setup. planOffersCurrentModel only guards re-runs, and re-running the wizard does not self-heal because the unserved id stays in modelProviders — a manual /model switch is required.
Failure scenario: first-time Coding Plan setup on a non-Pro account whose /models does not serve the Pro-only default qwen3.6-plus (the preset's own description tags it "Currently available to Pro subscribers only", and linked issue 9368 documents live endpoints serving partial catalogs): pressing Enter without editing makes model.name = qwen3.6-plus — the CLI's active model is one the account's own catalog does not list, and the first chat request targets it. This contradicts the PR's stated purpose ("selecting a stale recommendation persists a broken entry") — here the stale entry becomes the active model with zero user selection.
Witness (probes at this commit):
PARTIAL-CATALOG submit order ["qwen3.6-plus","qwen3.5-plus",…] -> model.name writes [["model.name","qwen3.6-plus"]]
FULL-CATALOG model.name writes [["model.name","qwen3.5-plus"]]
UI probe (catalog serves only MiniMax-M3, no edits): SUBMITTED {"modelIds":["MiniMax-M2.7","MiniMax-M3"]} <- unserved first
recommendations-first mutant flips the order to ["MiniMax-M3","MiniMax-M2.7"]
Fix (spans merge ordering + tests, so no one-click suggestion): order checked served recommendations ahead of demoted/free-form ids on submit (e.g. recommendations-first in mergeModelIds, or reorder the no-edit submit so previously-default-but-unserved ids follow the served ones), so models[0] stays a model the account's catalog serves; add a test asserting modelSelection/model.name for a partial-catalog no-edit submit.
中文说明
当目录不完整时,未被提供的内置默认模型会被降级进自由输入框(getCustomModelIdsText),而 mergeModelIds 把输入框的 id 排在最前——于是不做任何编辑直接回车提交的列表会以一个未被提供的模型开头,buildInstallPlan(models[0] → modelSelection)和 applyProviderInstallPlan(providers/install.ts)在首次配置时会把它写成 model.name。planOffersCurrentModel 只保护重复运行的场景,且重新运行向导无法自愈——未被提供的 id 仍留在 modelProviders 中,必须手动 /model 切换。
失败场景:非 Pro 账号首次配置 Coding Plan,其 /models 不提供 Pro 专属的默认模型 qwen3.6-plus(preset 自己的描述标注 "Currently available to Pro subscribers only",关联 issue 9368 也记录了线上端点只提供部分目录):不编辑直接回车会让 model.name = qwen3.6-plus——CLI 的激活模型不在账号自己的目录里,第一次聊天请求就会打到它。这与 PR 自述的目的("选中陈旧推荐会持久化一个坏条目")相矛盾——此处陈旧条目在用户零选择的情况下直接成为激活模型。
见证(本提交上的探针):部分目录下提交顺序 ["qwen3.6-plus","qwen3.5-plus",…] → model.name 写入 qwen3.6-plus;完整目录时写入 qwen3.5-plus。UI 探针(目录只提供 MiniMax-M3、不做编辑):提交结果为 ["MiniMax-M2.7","MiniMax-M3"](未提供的排在最前);把合并顺序改为推荐优先的突变体将顺序翻转为 ["MiniMax-M3","MiniMax-M2.7"]。
修复(跨合并顺序与测试,故不提供一键 suggestion):提交时让已勾选的被提供服务推荐排在被降级/自由输入的 id 之前(例如在 mergeModelIds 中推荐优先,或重排无编辑提交使"曾是默认但未被提供"的 id 排在被提供模型之后),保证 models[0] 始终是账号目录中存在的模型;并新增测试断言部分目录下无编辑提交的 modelSelection/model.name。
— qwen3.8-max via Qwen Code /review (v0.22.0)
…dels (QwenLM#9980) Two Critical review findings on the discovery wizard. On the discovery path edits never cleared the empty-submit error banner because they only call the no-op sync; add a clearModelIdsError flow action for that branch. And mergeModelIds put free-form ids first, so a partial-catalog no-edit submit led with an unserved demoted default, which buildInstallPlan writes as model.name on first-time setup; checked recommendations lead now.
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 6/100 轮)。改动内容与我反驳保留之处如下: Autofix round summary — PR #9980 (Critical-only mode)Both Critical inline findings from the automated review (rv:5025619269) are fixed. Each was reproduced on the pre-fix tree with a focused failing test first, then fixed minimally; the committed tests fail before the fix and pass after it. No conflict resolution was needed ( Findings and dispositions[rc:3858676058] Critical — partial-catalog no-edit submit leads with an unserved model → FixedReproduced before fixing: with the built-in defaults [rc:3858676053] Critical — stale empty-submit error banner on the discovery path → FixedReproduced before fixing by driving the real Failed check: Test (windows-latest, Node 22.x) → diagnosed; no evidence-backed fix exists inside this PR's footprint
Verification
中文说明Autofix 轮次总结 — PR #9980(仅处理 Critical 模式)自动审查(rv:5025619269)提出的两条 Critical 行内发现均已修复。每一条都先在修复前的代码树上用聚焦的失败测试复现,再做最小修复;本轮提交的测试在修复前失败、修复后通过。无需冲突处理( 发现与处置[rc:3858676058] Critical — 部分目录下无编辑提交以未提供服务模型开头 → 已修复修复前已复现:内置默认 [rc:3858676053] Critical — 发现路径上空提交错误横幅长期不消失 → 已修复修复前已复现:用真实的 失败检查:Test (windows-latest, Node 22.x) → 已诊断;本 PR 足迹内没有证据支持的修复可做
验证
Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明已进入仅处理 Critical 的模式:已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:
packages/core/src/providers/types.ts:70 — [review] supportsModelDiscovery does not state the OpenAI-compatible catalog contract (Bearer GET {baseUrl}/models)
中文说明
无阻断问题。LGTM!✅
收敛姿态下延后(第 7 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
Released in v0.22.2. |
What this PR does
This replaces #9389 with a bounded, snapshot-before-editing design. Token Plan and Coding Plan opt in to one authenticated OpenAI-compatible model-list request when the setup wizard reaches Model IDs. The wizard shows a cancellable loading state and does not mount the editor until the request has either produced a usable catalog or fallen back, so a late network result can never overwrite text under the cursor.
Successful standard
{ data: [{ id }] }responses are filtered for chat-capable model IDs and merged with the built-in recommendations. Known IDs keep all curated capability metadata and stable built-in order; newly served IDs are appended without invented metadata and start unchecked; built-in IDs absent from the catalog are no longer recommended; user-entered custom IDs remain in the free-form input. Edits stay local to that visit and are committed only on Enter, so Esc discards unsubmitted changes and re-entry creates a fresh snapshot.Every missing-key, timeout, cancellation, redirect, non-2xx, malformed, empty, or all-non-chat result falls back to the complete built-in list with an inline notice. The request has a five-second whole-transfer budget, a one-megabyte response cap, same-host redirect enforcement, and no logging or persistent cache. All other providers retain the existing behavior.
Why it's needed
The ModelStudio plan catalogs change independently of Qwen Code releases. Static recommendations can therefore continue offering retired IDs or omit models an account can already use, and selecting a stale recommendation persists a broken entry whose failure appears only on the first request. Loading a bounded account snapshot at the one point where recommendations are edited closes that drift without adding a background refresh or a second source of persistent state.
Reviewer Test Plan
How to verify
Open
/auth, choose Alibaba ModelStudio and then Token Plan or Coding Plan, enter a valid key, and continue to Model IDs. Expect a loading-only state first, followed byRecommended models · from the provider; the editor must not appear before the catalog settles. With an account catalog that omits a built-in ID and adds a new ID, expect the omitted ID to disappear, the new ID to be shown unchecked, existing custom IDs to remain in the input, and Enter to persist only the custom IDs plus checked known recommendations. Press Esc during loading or editing and re-enter to confirm a fresh snapshot with no unsubmitted edit retained. With a syntactically valid bad key or an offline endpoint, expectRecommended models · provider list unavailable, showing built-insand the complete static list.Automated verification passed: the focused core tests cover 84 cases, including 12 discovery cases; the rendered setup-step tests cover 10 cases; full build, full workspace typecheck, focused ESLint, pre-commit Prettier, and pre-commit ESLint all pass.
Evidence (Before & After)
The global
qwenbinary is not available in this checkout environment, so the TUI evidence below comes from the real rendered component frames asserted by the focused tests rather than a live authenticated account.Before:
After a successful snapshot:
After fallback:
Tested on
Environment (optional)
macOS, Node.js 22, mocked provider responses plus rendered Ink component tests. No authenticated live provider key was stored or used.
Risk & Scope
/modelintegration from feat: Add dynamic multi-model support for OpenAI-compatible APIs #1206, and static catalog curation remain separate work. Name-based non-chat filtering is necessarily heuristic, and any filtered ID remains available through free-form input.Linked Issues
Closes #9368
中文说明
这个 PR 做了什么
这个 PR 用“编辑前先确定快照”的受限方案替代 #9389。Token Plan 和 Coding Plan 选择加入:配置向导进入 Model IDs 时,只发起一次带认证的 OpenAI 兼容模型列表请求。向导先显示可取消的加载状态,在请求得到可用目录或完成回退前不挂载编辑器,因此再晚到达的网络结果也不可能覆盖光标下的文本。
成功的标准
{ data: [{ id }] }响应会先过滤出可聊天模型 ID,再与内置推荐合并。已知 ID 保留全部人工维护的能力元数据和稳定的内置顺序;服务端新增 ID 追加展示,不虚构元数据且默认不勾选;目录中缺失的内置 ID 不再推荐;用户手输的自定义 ID继续保留在自由输入框中。编辑只在本次访问的局部状态中发生,按 Enter 才提交,所以 Esc 会丢弃未提交修改,重新进入会创建新的快照。缺少 key、超时、取消、重定向、非 2xx、响应格式错误、空目录或过滤后全是非聊天模型时,全部回退到完整内置列表并显示行内提示。请求采用 5 秒全传输预算、1 MiB 响应上限和同 host 重定向限制,不记录日志,也不做持久缓存。其他 provider 维持现有行为。
为什么需要
ModelStudio 套餐目录的变化不依赖 Qwen Code 发版。因此静态推荐可能继续提供已退役 ID,或漏掉账号已经可用的模型;用户选中陈旧推荐后,会把坏条目持久化,直到第一次请求才看到错误。在唯一需要编辑推荐的步骤加载一次有界账号快照,可以消除这类漂移,同时不引入后台刷新或第二套持久状态。
Reviewer 测试计划
如何验证
打开
/auth,选择 Alibaba ModelStudio,再选择 Token Plan 或 Coding Plan,输入有效 key 并进入 Model IDs。应先只看到加载状态,随后看到Recommended models · from the provider;目录确定前编辑器不能出现。使用一个会缺少某个内置 ID、同时包含新 ID 的账号目录时,缺失的 ID 应消失,新 ID 应显示为未勾选,现有自定义 ID 应继续留在输入框中,按 Enter 后只持久化自定义 ID 与已勾选的已知推荐。加载或编辑期间按 Esc,再重新进入,应得到新快照且不保留未提交编辑。使用格式合法但错误的 key 或离线端点时,应看到Recommended models · provider list unavailable, showing built-ins和完整静态列表。自动验证已通过:core 聚焦测试覆盖 84 个用例,其中 12 个是发现逻辑用例;渲染后的配置步骤测试覆盖 10 个用例;全量 build、全 workspace typecheck、聚焦 ESLint、pre-commit Prettier 和 pre-commit ESLint 均通过。
证据(Before & After)
当前检出环境中没有全局
qwen可执行文件,因此下面的 TUI 证据来自聚焦测试所断言的真实组件渲染帧,而不是带真实账号认证的在线截图。之前:
成功获得快照后:
回退后:
测试平台
环境(可选)
macOS、Node.js 22、模拟 provider 响应与 Ink 组件渲染测试。未存储或使用带认证的在线 provider key。
风险与范围
/model集成以及静态目录维护仍是独立工作。基于名称的非聊天过滤必然是启发式,被过滤的 ID 仍可通过自由输入框使用。关联 Issue
Closes #9368