fix(cli): keep model switches session-scoped - #6579
Conversation
Local verification reportI verified this PR with a real local CLI run using a temporary Real CLI behavior checkTemporary settings started with: {
"model": { "name": "gpt-4o-mini" },
"security": { "auth": { "selectedType": "openai" } }
}Session-scoped switch: $ QWEN_HOME="$tmp" OPENAI_API_KEY=dummy npm run dev -- /model gpt-4o --auth-type openai --output-format text
Model: gpt-4oResult: Explicit default switch: $ QWEN_HOME="$tmp" OPENAI_API_KEY=dummy npm run dev -- -p "/model --default gpt-4o" --auth-type openai --output-format text
Model: gpt-4o (default)Result: {
"model": { "name": "gpt-4o", "baseUrl": "" },
"security": { "auth": { "selectedType": "openai" } }
}Note: for the second command I used Automated checks$ cd packages/cli && npx vitest run src/ui/commands/modelCommand.test.ts src/ui/components/ModelDialog.test.tsx src/ui/hooks/slashCommandProcessor.test.ts
Test Files 3 passed (3)
Tests 186 passed (186)$ npm run build
# passed$ npm run typecheck
# passedScreenshot / visual evidenceThis change is command/settings behavior rather than a visual UI rendering change, so I did not include a screenshot. The CLI transcript above is the relevant before/after evidence. 中文验证报告本地验证报告我使用临时 真实 CLI 行为验证临时 settings 初始内容为: {
"model": { "name": "gpt-4o-mini" },
"security": { "auth": { "selectedType": "openai" } }
}会话级切换: $ QWEN_HOME="$tmp" OPENAI_API_KEY=dummy npm run dev -- /model gpt-4o --auth-type openai --output-format text
Model: gpt-4o结果: 显式默认值切换: $ QWEN_HOME="$tmp" OPENAI_API_KEY=dummy npm run dev -- -p "/model --default gpt-4o" --auth-type openai --output-format text
Model: gpt-4o (default)结果: {
"model": { "name": "gpt-4o", "baseUrl": "" },
"security": { "auth": { "selectedType": "openai" } }
}说明:第二条命令使用了 自动化检查$ cd packages/cli && npx vitest run src/ui/commands/modelCommand.test.ts src/ui/components/ModelDialog.test.tsx src/ui/hooks/slashCommandProcessor.test.ts
Test Files 3 passed (3)
Tests 186 passed (186)$ npm run build
# passed$ npm run typecheck
# passed截图 / 可视化证据这个改动是命令和 settings 行为变化,不是 UI 渲染变化,所以没有附截图。上面的 CLI transcript 就是相关的 before/after 证据。 |
|
Thanks for the PR! Template looks good ✓ Problem: Observed bug with clear evidence. Issue #4331 documents that Direction: Aligned. The command's own description says "switch the model for this session" — persisting the switch silently is a UX bug. Adding Size: Not applicable — all changes are in Approach: Scope feels right. The Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测到的 bug,有明确证据。Issue #4331 记录了 方向:对齐。命令自己的描述说"为当前会话切换模型"——默默持久化是一个 UX bug。添加 规模:不适用——所有改动都在 方案:范围合理。 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal before reading the diff: gate The PR's approach matches this exactly — No critical blockers found. No AGENTS.md violations. The code follows existing conventions — regex flag parsing mirrors Reuse check: no missed reuse opportunities. Flag parsing, state threading, and dialog props all follow established patterns in the codebase. Test ResultsFocused unit tests (PR code, worktree)CLI verification (PR code via
|
|
This is a clean, well-scoped fix for a genuine UX bug. The The implementation matches what I'd have done independently: add The focused test suite (186 tests across 3 files) covers the new flag parsing, the conditional persistence, the mutual exclusion with auxiliary model flags, and the dialog threading. The author's own CLI verification shows the behavior working correctly end-to-end. The breaking change is intentional and well-documented: users who relied on plain Approving. ✅ 中文说明这是一个干净、范围明确的修复,解决了一个真实的 UX bug。 实现方案与我独立构思的完全一致:添加 聚焦测试套件(3 个文件中的 186 个测试)覆盖了新的标志解析、条件持久化、与辅助模型标志的互斥,以及对话框传递。作者自己的 CLI 验证展示了端到端行为的正确性。 破坏性变更是有意的且有充分记录的:之前依赖普通 批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Two additional Critical findings could not be anchored to diff lines:
[Critical] --default silently dropped with inline prompt (modelCommand.ts:830-871): /model --default qwen-max some prompt silently ignores --default. The inline prompt path checks scopeOverride (rejecting it) but never checks persistDefault. Add a guard analogous to the scopeOverride check before the submit_prompt return.
— qwen3.7-max via Qwen Code /review
| } | ||
|
|
||
| if (!settings) { | ||
| if (persistDefault && !settings) { |
There was a problem hiding this comment.
[Critical] Voice/fast/vision model !settings guard incorrectly relaxed.
persistDefault is always false here because --default --voice is rejected by the earlier mutual-exclusion guard. This makes persistDefault && !settings dead code. But persistSetting(settings, 'voiceModel', ...) is called unconditionally downstream — when settings is undefined, this crashes with a TypeError.
The original unconditional if (!settings) was correct here since voice/fast/vision models always persist regardless of --default.
| if (persistDefault && !settings) { | |
| if (!settings) { |
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Thanks. I re-checked this against every production CommandContext constructor. services.settings is a required LoadedSettings, and the interactive, non-interactive, and ACP command entry points all provide it. The conditional is redundant in the voice path, but there is no production-reachable settings === undefined context that can reach the downstream call, so the reported crash scenario is a false positive rather than a Critical defect. After 15 reviewed commits I am not widening this round to harden an impossible mocked context; an independent exact-diff review reached the same conclusion.
中文说明
谢谢。我重新核对了所有生产环境中的 CommandContext 构造入口。services.settings 是必填的 LoadedSettings,交互式、非交互式和 ACP 命令入口都会传入它。语音模型路径中的条件确实是冗余的,但生产环境中不存在能够到达后续调用的 settings === undefined 上下文,因此所描述的崩溃场景属于误报,而不是 Critical 缺陷。该 PR 已经跨 15 个提交接受评审,本轮不再为了一个不可能的 mock 上下文扩大改动;独立精确 diff 审查也得出了相同结论。
| settings, | ||
| authType, | ||
| modelName, | ||
| persistDefault, |
There was a problem hiding this comment.
[Critical] The caller's !settings guard at line 877 is unconditional, but switchMainModel only accesses settings inside if (persistDefault) blocks. A session-only /model <id> (no --default) in an environment where settings is unavailable will error with "Settings service not available" even though it no longer needs settings.
| persistDefault, | |
| if (persistDefault && !settings) { |
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Thanks. This scenario is not production-reachable: CommandContext.services.settings is non-optional, and every production command entry point constructs the context with a LoadedSettings instance. The unconditional guard is therefore redundant but cannot block a real session-only switch because of a missing settings service. I am classifying this Critical as a false positive and leaving the unrelated defensive shape unchanged after 15 reviewed commits; the independent review also found no reachable consumer without settings.
中文说明
谢谢。这个场景在生产环境中不可达:CommandContext.services.settings 是非可选字段,每个生产命令入口都会用一个 LoadedSettings 实例构造上下文。因此这个无条件 guard 虽然冗余,却不会因为 settings 服务缺失而阻断真实的仅会话模型切换。我将这个 Critical 归类为误报;该 PR 已经跨 15 个提交接受评审,本轮保持这处无关的防御结构不变。独立审查同样没有发现任何缺少 settings 的可达消费者。
There was a problem hiding this comment.
Classified as false positive — no code change. CommandContext.services.settings is typed LoadedSettings (non-optional, types.ts:52) and every production constructor (interactive, non-interactive, ACP) provides it. The unconditional guard is redundant but cannot block a real session-only switch. The voice-path guard was separately reverted to unconditional for consistency with fast/vision paths.
中文说明
归类为误报——未修改代码。CommandContext.services.settings 类型为 LoadedSettings(非可选,types.ts:52),所有生产构造入口(交互式、非交互式、ACP)都会提供它。无条件守卫虽然冗余,但不会阻断真实的仅会话模型切换。语音路径守卫已单独恢复为无条件形式,以与 fast/vision 路径保持一致。
|
|
||
| const result = await modelCommand.action!(mockContext, 'qwen-max'); | ||
|
|
||
| expect(switchModel).toHaveBeenCalledWith( |
There was a problem hiding this comment.
[Critical] Missing test coverage for several --default code paths:
--default --fast/--default --voice/--default --visionrejection (mutual-exclusion guard atmodelCommand.ts:468)- Bare
/model --default(no model, no scope) opening the dialog with{ persistDefault: true } --defaultwith auth-qualified model (e.g.,openai:gpt-4) which exercises thesecurity.auth.selectedTypepersistence branch
Suggested tests:
it('should reject --default combined with --fast', async () => {
const ctx = setupContext();
const result = await modelCommand.action!(ctx, '--default --fast');
expect(result).toMatchObject({
type: 'message',
messageType: 'error',
content: expect.stringContaining('--default only applies to the main model'),
});
});
it('should open the model dialog with persistDefault for /model --default', async () => {
const ctx = setupContext();
const result = await modelCommand.action!(ctx, '--default');
expect(result).toEqual({
type: 'dialog',
dialog: 'model',
persistDefault: true,
});
});— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
The requested cases are reasonable coverage suggestions, but the repository review rules classify missing tests as Suggestion-level unless the untested path is itself the defect. This round adds regression coverage for the two reproduced user-visible blockers: the silently dropped --default inline intent and scoped image routing. The remaining coverage requests are deferred because the PR has already been reviewed across 15 distinct commits, so this Critical severity is over-classified rather than an unresolved correctness blocker.
中文说明
这些用例作为测试覆盖建议是合理的,但仓库评审规则明确规定:除非未测试路径本身就是缺陷,否则缺少测试属于 Suggestion。本轮已经为两个已复现的用户可见阻断问题补充了回归覆盖:静默丢弃 --default 的内联提示路径,以及带作用域的图像模型路由。其余覆盖建议留待后续,因为该 PR 已经跨 15 个不同提交接受评审;因此这里的 Critical 严重度属于过度分类,而不是仍未解决的正确性阻断。
There was a problem hiding this comment.
Partially addressed this round: added tests for --default --fast rejection, --default --image rejection, and bare /model --default opening the dialog with persistDefault: true. Remaining coverage requests (--default --voice, --default --vision, auth-qualified model paths) are deferred to a follow-up per the repository's review-round policy.
中文说明
本轮已部分处理:新增了 --default --fast 拒绝、--default --image 拒绝、以及裸 /model --default 打开带 persistDefault: true 对话框的测试。剩余覆盖请求(--default --voice、--default --vision、带认证限定的模型路径)按仓库审查轮次规则延期至后续处理。
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
packages/cli/src/ui/commands/modelCommand.ts:465 vs ModelDialog.tsx:244-250 |
Confirmation message inconsistency across command and dialog paths. Command path shows (this project) for --default --project (missing "default"), and no suffix at all for session-only /model <id>. Dialog path shows (this project default) and (current session) respectively. Same user operations, different feedback. |
Use a shared formatPersistSuffix(scope, isDefault) helper, or align both paths to show (this project default) / (global default) / (current session) consistently. |
packages/cli/src/ui/contexts/UIActionsContext.tsx:56-60 |
openModelDialog type missing persistScope and persistDefault params that useModelCommand and slashCommandProcessor both declare. |
Add persistScope?: 'workspace' | 'user' and persistDefault?: boolean to the context type. |
packages/cli/src/ui/commands/modelCommand.ts:904 |
Non-interactive help text drops --project/--global documentation — old text listed scope flags, new text only mentions --default and --fast. |
Add "/model --default --project <id>" and "/model --default --global <id>" to the help string. |
packages/cli/src/i18n/locales/*.js |
New i18n keys untranslated: ' (this project default)', ' (global default)', ' (current session)', 'Set Default Model', --default error messages, and completion description absent from most locale files (fr, ru, de, ca, pt, ja). |
Add translations for all non-English locale files. |
packages/cli/src/ui/commands/modelCommand.ts:900 |
/model --default in non-interactive mode with no model id is a silent no-op — flag consumed with zero effect and no error. |
Return error: "--default requires a model id in non-interactive mode." |
packages/cli/src/ui/commands/modelCommand.test.ts |
Missing test: bare /model --default (no scope flag, no model id) should open dialog with persistDefault: true. |
Add test asserting { type: 'dialog', dialog: 'model', persistDefault: true }. |
packages/cli/src/ui/components/ModelDialog.test.tsx |
Missing test: persistDefault: true + persistScope: 'workspace' combination (the (this project default) suffix path). |
Add test rendering with both props and asserting suffix text. |
packages/cli/src/ui/hooks/useModelCommand.ts:33-36 |
Missing test: modelDialogPersistDefault state tracking through openModelDialog({ persistDefault: true }) and closeModelDialog(). Existing test file has zero references to persistDefault. |
Add test asserting state defaults to false, is set to true on open, and resets on close. |
— qwen3.7-max via Qwen Code /review
doudouOUC
left a comment
There was a problem hiding this comment.
[Critical] --default silently dropped with inline prompt
/model --default qwen-max hello world parses persistDefault=true and extracts inlinePrompt="hello world", but the inline prompt path (around line 831) only checks scopeOverride to reject --project/--global — it never checks persistDefault. The function returns { type: 'submit_prompt', modelOverride: 'qwen-max', content: 'hello world' }, producing a one-shot override with no persistence and no error.
A user who runs /model --default gpt-4 explain this codebase believes they have both persisted the model as default AND sent the prompt. Neither happens — only the one-shot override runs. The user discovers the persistence didn't take effect only when they start a new session.
This is the same class of silent-ignore bug that the existing scopeOverride guard was explicitly added to prevent. The fix is to add an analogous persistDefault guard inside the if (inlinePrompt) block:
if (persistDefault) {
return {
type: 'message',
messageType: 'error',
content: t(
"Cannot combine --default with an inline prompt. Run '/model --default {{model}}' first, then send your prompt.",
{ model: modelName },
),
};
}— qwen3.7-max via Qwen Code /review
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
packages/cli/src/ui/commands/modelCommand.ts:465 |
Confirmation message suffix inconsistency: defaultSuffix = scopeSuffix || t(' (default)') drops the word "default" when a scope flag is present. /model --default --project qwen-max shows "Model: qwen-max (this project)" — indistinguishable from a non-default scoped switch. Meanwhile ModelDialog.tsx:243-249 correctly produces " (this project default)" / " (global default)". |
Replace the fallback chain with explicit branches: const defaultSuffix = persistDefault ? (scopeOverride === SettingScope.Workspace ? t(' (this project default)') : scopeOverride === SettingScope.User ? t(' (global default)') : t(' (default)')) : ''; |
packages/cli/src/ui/commands/modelCommand.ts:915 |
Bare /model --default (no model id, no scope flag) dialog path untested. Tests cover --default --project and --default --global dialog returns, but not the plain --default invocation that should return { type: 'dialog', dialog: 'model', persistDefault: true }. This is the most common interactive "set default" flow. |
Add test: it('should open dialog with persistDefault for bare /model --default', ...) asserting the dialog return includes persistDefault: true. |
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no new blockers beyond the three open Critical threads. Suggestion-level recommendations are in the Suggestion summary comment below.
Follow-up validation updateI pushed a small follow-up commit ( Validated locally with Node cd packages/cli
PATH="$HOME/.nvm/versions/node/v22.23.1/bin:$PATH" npx vitest run src/ui/commands/modelCommand.test.ts src/i18n/mustTranslateKeys.test.tsResult: Additional repository checks: PATH="$HOME/.nvm/versions/node/v22.23.1/bin:$PATH" npm run build
PATH="$HOME/.nvm/versions/node/v22.23.1/bin:$PATH" npm run typecheckResult: both passed. Notes from broader local testing: No screenshot is attached for this follow-up because the change is CLI command semantics plus locale coverage; the reproducible command transcript above is the relevant evidence. 中文验证报告已推送一个小的跟进提交( 本地使用 Node cd packages/cli
PATH="$HOME/.nvm/versions/node/v22.23.1/bin:$PATH" npx vitest run src/ui/commands/modelCommand.test.ts src/i18n/mustTranslateKeys.test.ts结果: 额外仓库检查: PATH="$HOME/.nvm/versions/node/v22.23.1/bin:$PATH" npm run build
PATH="$HOME/.nvm/versions/node/v22.23.1/bin:$PATH" npm run typecheck结果:均通过。 全量测试说明:也尝试运行了 本次跟进没有截图,因为改动是 CLI 命令语义和 locale 覆盖;上面的可复现命令输出是更直接的验证证据。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no new blockers beyond the three open Critical threads. Suggestion-level recommendations are in the Suggestion summary comment below.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no new blockers beyond the three open Critical threads. Suggestion-level recommendations are in the Suggestion summary comment below.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no new blockers beyond the three open Critical threads. Suggestion-level recommendations are in the Suggestion summary comment below.
✅ Maintainer local verification — merge referenceI built and exercised this PR locally against a real CLI bundle (not just unit tests) to confirm the behavior change. Everything checks out — recommend merge.
Checks
The fix, before → afterTo make sure this is a real behavioral change and not just a test-only diff, I reproduced issue #4331 on
I also confirmed the no-leak end-to-end: after a session-scoped Full scenario matrixNotable points I specifically checked:
How to reproducegit fetch origin pull/6579/head && git checkout FETCH_HEAD
npm ci && npm run bundle
export QWEN_HOME=$(mktemp -d) OPENAI_API_KEY=sk-dummy
cat > "$QWEN_HOME/settings.json" <<'JSON'
{ "security": { "auth": { "selectedType": "openai" } },
"model": { "name": "gpt-4o-mini" },
"modelProviders": { "openai": [
{ "id": "gpt-4o", "envKey": "OPENAI_API_KEY", "baseUrl": "https://api.openai.com/v1" },
{ "id": "gpt-4o-mini", "envKey": "OPENAI_API_KEY", "baseUrl": "https://api.openai.com/v1" } ] } }
JSON
node dist/cli.js -p "/model gpt-4o" # session only -> settings.json unchanged
node dist/cli.js -p "/model --default gpt-4o" # opt-in persist -> model.name = gpt-4oVerdictThe change does exactly what the description says, is well guarded, keeps auxiliary-model behavior intact, and is covered by the updated unit tests. LGTM ✅ 中文版本(点击展开)✅ 维护者本地验证 —— 合并参考我在本地对着真实的 CLI 产物(不仅仅是单元测试)构建并实际运行了这个 PR,确认行为变更符合预期。全部通过 —— 建议合并。
检查项
修复效果:修改前 → 修改后为确认这是真实的行为变更、而不只是测试层面的改动,我先在 (截图见上方英文部分)
我还端到端确认了“无泄漏”:在一次会话级 完整场景矩阵(截图见上方英文部分) 我特别检查的要点:
复现方式git fetch origin pull/6579/head && git checkout FETCH_HEAD
npm ci && npm run bundle
export QWEN_HOME=$(mktemp -d) OPENAI_API_KEY=sk-dummy
# settings.json: model.name = gpt-4o-mini + openai modelProviders (gpt-4o / gpt-4o-mini)
node dist/cli.js -p "/model gpt-4o" # 仅会话 -> settings.json 不变
node dist/cli.js -p "/model --default gpt-4o" # 显式持久化 -> model.name = gpt-4o结论改动完全符合描述,保护逻辑完善,辅助模型行为保持不变,并有更新后的单元测试覆盖。LGTM ✅ |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
| // undefined write is dropped from JSON and would not override on merge). | ||
| persistSetting(settings, 'model.baseUrl', '', scopeOverride); | ||
| if (persistDefault) { | ||
| if (!settings) { |
There was a problem hiding this comment.
[Suggestion] Dead code: the if (!settings) { throw new Error(...) } guards inside the if (persistDefault) blocks (lines 137-139 and 155-157) are currently unreachable. The sole caller at line 877 unconditionally checks if (!settings) before calling switchMainModel, so the function is never reached with undefined settings.
If the caller's guard is fixed to if (persistDefault && !settings) (as suggested for the open Critical at line 877/889), these internal guards become necessary — they align with the function's declared contract (settings: LoadedSettings | undefined). Consider keeping them as defensive code with a comment, or removing them and narrowing the parameter type to LoadedSettings if the caller's unconditional guard is the intended final design.
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Deferred. The LoadedSettings | undefined widening and internal throws are unreachable given the caller's unconditional guard, but cleaning this up would widen a PR that has been through 18+ reviewed commits. The voice-path guard was reverted to unconditional this round for consistency; the switchMainModel signature cleanup is left for a follow-up.
中文说明
延期处理。鉴于调用方的无条件守卫,LoadedSettings | undefined 放宽和内部 throw 不可达,但清理这些会扩大一个已经历 18+ 个审查提交的 PR。本轮已将语音路径守卫恢复为无条件形式以保持一致;switchMainModel 签名清理留待后续。
| @@ -31,13 +33,16 @@ export const useModelCommand = (): UseModelCommandReturn => { | |||
| const [modelDialogPersistScope, setModelDialogPersistScope] = useState< | |||
There was a problem hiding this comment.
[Suggestion] The hook gained modelDialogPersistDefault state and a persistDefault option on openModelDialog(), plus reset logic in closeModelDialog(). The existing test file has no coverage for: (a) openModelDialog({ persistDefault: true }) setting modelDialogPersistDefault to true, (b) closeModelDialog() resetting it to false, or (c) it defaulting to false. If this state wiring breaks, the dialog would silently lose its persist-default behavior.
Similarly, ModelDialog.tsx has no test for the title changing to "Set Default Model" when persistDefault is true (line 880), or for the (current session) confirmation suffix for non-persist selections (lines 241-249).
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Deferred per the repository's review-round policy. Valid coverage suggestion for useModelCommand state wiring and ModelDialog title/suffix behavior; deferred to a follow-up to avoid widening this PR.
中文说明
按仓库审查轮次规则延期。对 useModelCommand 状态连接和 ModelDialog 标题/后缀行为的有效覆盖建议;延期至后续处理以避免扩大本 PR。
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
| @@ -852,7 +904,7 @@ export const modelCommand: SlashCommand = { | |||
| type: 'message', | |||
There was a problem hiding this comment.
[Suggestion] Non-interactive /model --default without a model ID silently falls through to the "Current model" info message. The --default flag is ignored with no error or guidance. In CI/CD or scripted use, a user expecting --default to take effect gets no indication it was dropped.
Consider adding a guard before the non-interactive fallback:
if (persistDefault) {
return {
type: 'message',
messageType: 'error',
content: t('--default requires a model ID in non-interactive mode. Use: /model --default <model-id>'),
};
}— qwen3.7-max via Qwen Code /review
| type: 'message', | ||
| messageType: 'info', | ||
| content: t('Model') + ': ' + effectiveModelName + scopeSuffix, | ||
| content: t('Model') + ': ' + effectiveModelName + defaultSuffix, |
There was a problem hiding this comment.
[Suggestion] The CLI confirmation for /model <id> shows Model: qwen-max with no suffix indicating the change is session-only. The model dialog correctly shows "(current session)" for the same scenario. Users migrating from the old persistent behavior get no visual feedback that their switch is ephemeral.
Consider adding a session-only suffix:
const defaultSuffix = persistDefault
? scopeSuffix || t(' (default)')
: t(' (this session)');— qwen3.7-max via Qwen Code /review
| @@ -434,6 +462,19 @@ export const modelCommand: SlashCommand = { | |||
| : scopeOverride === SettingScope.User | |||
| ? t(' (global)') | |||
There was a problem hiding this comment.
[Suggestion] Inconsistent confirmation labels between command and dialog paths. The command path shows (this project) / (global) while the dialog shows (this project default) / (global default) for the same operation with the same flags. Users see different messages depending on which UI path they take.
Consider aligning the command path labels with the dialog:
const defaultSuffix = persistDefault
? scopeOverride === SettingScope.Workspace
? t(' (this project default)')
: scopeOverride === SettingScope.User
? t(' (global default)')
: t(' (default)')
: '';— qwen3.7-max via Qwen Code /review
| : ''; | ||
| const defaultSuffix = persistDefault ? scopeSuffix || t(' (default)') : ''; | ||
| if ( | ||
| persistDefault && |
There was a problem hiding this comment.
[Suggestion] The mutual-exclusion guard rejecting --default combined with --fast, --voice, or --vision has no test coverage. If the regex or condition logic were wrong (e.g., accidentally matching --default-model or missing --vision), no test would catch it.
Consider adding tests for each rejected combination:
it('should reject --default --fast', async () => { ... });
it('should reject --default --voice', async () => { ... });
it('should reject --default --vision', async () => { ... });— qwen3.7-max via Qwen Code /review
| @@ -835,12 +886,13 @@ export const modelCommand: SlashCommand = { | |||
| settings, | |||
There was a problem hiding this comment.
[Critical] --default is silently dropped when combined with an inline prompt. /model --default qwen-max "hello" parses persistDefault=true, but the inline-prompt branch (line ~830) only rejects scopeOverride — there is no equivalent check for persistDefault. The function returns submit_prompt before reaching switchMainModel, so the persistence intent is silently discarded.
This is the same category of silent flag drop that the scopeOverride guard (line 830) was designed to prevent. Add a persistDefault check inside the if (inlinePrompt) block:
| settings, | |
| if (persistDefault) { | |
| return { | |
| type: 'message', | |
| messageType: 'error', | |
| content: t( | |
| "Cannot combine --default with an inline prompt. Run '/model --default {{model}}' first, then send your prompt.", | |
| { model: modelName }, | |
| ), | |
| }; | |
| } | |
| if (scopeOverride) { |
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Confirmed and fixed in 1adfab3f9269fe0bcc17bcecfd855f92e9a5c2cc. Persistence flags are now rejected inside the inline-prompt branch before submit_prompt can be returned, and both flag and model are interpolated into the corrective command. The regression test asserts that neither switchModel nor settings setValue is called. A bundled-CLI E2E run also returned the expected error with duration_api_ms=0, num_turns=0, and no default-model change.
中文说明
已确认,并在 1adfab3f9269fe0bcc17bcecfd855f92e9a5c2cc 中修复。现在持久化 flag 会在内联提示分支内部、返回 submit_prompt 之前被拒绝,修正命令中的 flag 和 model 也都会正确插值。回归测试断言不会调用 switchModel 或 settings 的 setValue。打包后 CLI 的 E2E 也返回了预期错误,duration_api_ms=0、num_turns=0,默认模型没有发生变化。
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review
| const flag = persistDefault | ||
| ? '--default' | ||
| : hasProject | ||
| ? '--project' | ||
| : hasGlobal | ||
| ? '--global' | ||
| : ''; |
There was a problem hiding this comment.
[Suggestion] The corrective hint in this rejection can steer the user into a dead end. When a scope flag is combined with an inline prompt but --default is absent (e.g. /model --project qwen-max explain this), flag resolves to --project, so the error reads "Run '/model --project qwen-max' first". But the new guard at modelCommand.ts:1071 (scopeOverride && !persistDefault && !inlinePrompt) rejects exactly /model --project qwen-max — following the hint then hits "Use --default with --project or --global when persisting the main model." The command that actually works is /model --default --project qwen-max. — Failure scenario: the user follows the suggested command and gets a second, contradictory error instead of persisting the model.
| const flag = persistDefault | |
| ? '--default' | |
| : hasProject | |
| ? '--project' | |
| : hasGlobal | |
| ? '--global' | |
| : ''; | |
| const flag = persistDefault | |
| ? '--default' | |
| : hasProject | |
| ? '--default --project' | |
| : hasGlobal | |
| ? '--default --global' | |
| : ''; |
中文说明
这个拒绝分支里的纠正提示可能把用户引向死路。当作用域标志与内联 prompt 组合、但没有 --default 时(例如 /model --project qwen-max explain this),flag 会解析为 --project,于是错误提示显示 “Run '/model --project qwen-max' first”。但 modelCommand.ts:1071 处新增的守卫(scopeOverride && !persistDefault && !inlinePrompt)恰好会拒绝 /model --project qwen-max —— 按提示执行会得到 “Use --default with --project or --global when persisting the main model.”。真正可用的命令是 /model --default --project qwen-max。—— 失败场景:用户照做后得到第二个相互矛盾的报错,而不是持久化模型。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 9/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 9/100 轮)。改动内容与我反驳保留之处如下: Review feedback addressedOne inline suggestion was open on this PR. It is resolved in code. [rc:3697562104] Inline-prompt rejection hint pointed at a dead-end command — FIXEDFinding (Suggestion). When a scope flag is combined with an inline prompt but Verification of the finding. Confirmed by tracing the parser: for Decision: implement (reviewer's suggestion adopted verbatim). The hint now resolves to the persistence flags that produce a working command:
So the rejection now reads "Run '/model --default --project qwen-max' first", which passes the scope guard and persists as intended. The existing unit test asserting the old Changes:
Conflict notes: none ( Verification
中文说明已处理的审查反馈本 PR 上有一条行内建议处于待处理状态,已在代码中解决。 [rc:3697562104] 内联 prompt 拒绝提示指向了一个无法成功的命令 — 已修复发现(建议)。 当作用域标志与内联 prompt 组合、但缺少 对发现的核实。 通过跟踪解析器确认:对于 决定:实现(逐字采纳审查者的建议)。 提示现在会解析为能产生可用命令的持久化标志:
因此拒绝提示现在显示 “Run '/model --default --project qwen-max' first”,该命令能通过作用域守卫并按预期持久化。已将断言旧 改动:
冲突说明: 无( 验证
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.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review
| expect(setValue).toHaveBeenCalledWith( | ||
| expect.anything(), | ||
| 'model.name', | ||
| 'qwen-max', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The trailing-flag --default test pins the scope with expect.anything(), while its two sibling tests pin SettingScope.Workspace (--default --project) and SettingScope.User (--default --global) exactly. The resolved scope here is deterministic — for this mock (untrusted workspace, empty user settings) getPersistScopeForModelSelection always falls back to SettingScope.User. — Failure scenario: if a future change moves the bare---default fallback scope (e.g. to Workspace), /model qwen-max --default would persist to the wrong settings file, yet this test still passes because expect.anything() matches any scope — the regression ships uncaught while the explicit --project/--global cases stay pinned. (The leading-flag --default qwen-max tests use a similarly loose expect.any(String) matcher.)
| expect(setValue).toHaveBeenCalledWith( | |
| expect.anything(), | |
| 'model.name', | |
| 'qwen-max', | |
| ); | |
| expect(setValue).toHaveBeenCalledWith( | |
| SettingScope.User, | |
| 'model.name', | |
| 'qwen-max', | |
| ); |
中文说明
尾随标志 --default 测试用 expect.anything() 匹配 scope,而其两个兄弟测试分别精确固定了 SettingScope.Workspace(--default --project)和 SettingScope.User(--default --global)。此处的解析 scope 是确定的——对该 mock(不受信任的工作区、空的 user 设置),getPersistScopeForModelSelection 总是回退到 SettingScope.User。— 失败场景:若未来改动把裸 --default 的回退 scope 改到(如 Workspace),/model qwen-max --default 会持久化到错误的设置文件,但本测试仍会通过,因为 expect.anything() 匹配任意 scope——回归会未被捕获就上线,而显式 --project/--global 用例仍是固定的。(前导标志 --default qwen-max 测试也用了类似松散的 expect.any(String) 匹配器。)
— qwen3.8-max-preview via Qwen Code /review
| }; | ||
| } | ||
|
|
||
| if (persistDefault && context.executionMode !== 'interactive') { |
There was a problem hiding this comment.
[Suggestion] No test exercises the non-interactive SUCCESS path /model --default <model-id> — only the bare---default rejection is covered. This guard sits AFTER the modelName handling block, which is what lets headless /model --default qwen-max persist and succeed. — Failure scenario: a "fail-fast" refactor that hoists this guard above the modelName block stays green under the entire suite, after which every valid headless /model --default <id> invocation is wrongly rejected with "--default requires a model ID in non-interactive mode" even though an ID was supplied, and the default is never persisted.
it('should persist the main model for /model --default <id> in non-interactive mode', async () => {
const ctx = setupContext();
ctx.executionMode = 'non_interactive';
const result = await modelCommand.action!(ctx, '--default qwen-max');
expect(ctx.services.config.switchModel).toHaveBeenCalled();
expect(setValue).toHaveBeenCalledWith(SettingScope.User, 'model.name', 'qwen-max');
expect(result).toMatchObject({
type: 'message',
content: expect.stringContaining('(default)'),
});
});中文说明
没有测试覆盖非交互模式下的成功路径 /model --default <model-id>——只覆盖了裸 --default 的拒绝。这个守卫位于 modelName 处理块之后,正是它让无头 /model --default qwen-max 能够持久化并成功。— 失败场景:把该守卫提到 modelName 块之上的"快速失败"重构会让整个测试套件保持绿色,之后所有合法的无头 /model --default <id> 调用都会被错误拒绝(报 "--default requires a model ID in non-interactive mode"),即便已经提供了 model id,默认值也永远不会被持久化。
— qwen3.8-max-preview via Qwen Code /review
| actions.openModelDialog({ | ||
| persistScope: result.persistScope, | ||
| persistDefault: result.persistDefault, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] The persistDefault hand-off to openModelDialog is not pinned by any test. The 'dialog: model' test (slashCommandProcessor.test.ts) asserts only toHaveBeenCalled(), unlike the voice/vision/image/compaction cases which use toHaveBeenCalledWith. — Failure scenario: the chain is tested at both ends (the command returns persistDefault: true; ModelDialog persists when the prop is true) but not across this middle. Deleting the persistDefault: result.persistDefault line stays green end-to-end: bare /model --default would then open the picker with the prop defaulted to false, so a user choosing a model from the "default" picker flow gets a session-only switch with nothing persisted — the exact regression this flag exists to prevent, shipping silently.
// in slashCommandProcessor.test.ts, the 'dialog: model' case:
mockCommand.action.mockResolvedValue({
type: 'dialog',
dialog: 'model',
persistDefault: true,
persistScope: 'user',
});
// ...after processing the command:
expect(mockOpenModelDialog).toHaveBeenCalledWith({
persistScope: 'user',
persistDefault: true,
});中文说明
传给 openModelDialog 的 persistDefault 没有任何测试固定。'dialog: model' 测试(slashCommandProcessor.test.ts)只断言了 toHaveBeenCalled(),而 voice/vision/image/compaction 用例都用了 toHaveBeenCalledWith。— 失败场景:链路两端都有测试(命令返回 persistDefault: true;ModelDialog 在 prop 为 true 时持久化),但中间这一环没有。删掉 persistDefault: result.persistDefault 这一行,端到端仍会是绿色:裸 /model --default 会打开 prop 默认为 false 的选择器,于是从"默认"选择器流程选模型的用户只会得到一次会话级切换、什么都不持久化——正是这个标志要防止的回归,却会静默上线。
— qwen3.8-max-preview via Qwen Code /review
| }, | ||
| argumentHint: | ||
| '[--fast|--voice|--vision|--compaction|--image] [--project|--global] [<model-id>] | <model-id> <prompt>', | ||
| '[--default] [--fast|--voice|--vision|--compaction|--image] [--project|--global] [<model-id>] | <model-id> <prompt>', |
There was a problem hiding this comment.
[Suggestion] This argumentHint (and the description getter above it) still present --project/--global as standalone options, but the new guard at modelCommand.ts:1072 (scopeOverride && !persistDefault && !inlinePrompt) rejects /model --project <id> / /model --global <id> for the MAIN model unless --default is also present. — Failure scenario: a user reads the /model help (via /help, tab-completion, or this hint) and runs /model --project qwen-max to persist to project scope exactly as the text says; they instead get "Use --default with --project or --global when persisting the main model." The clauses stay accurate for sub-model commands (e.g. /model --voice --project <id>), which makes the mismatch subtle — the text is only wrong for the main model, the most common case. Notably this diff already updated the bare-/model non-interactive message to drop these forms, but left the description getter, this hint, and the --project/--global tab-completion descriptions stale.
| '[--default] [--fast|--voice|--vision|--compaction|--image] [--project|--global] [<model-id>] | <model-id> <prompt>', | |
| '[--default [--project|--global]] [--fast|--voice|--vision|--compaction|--image [--project|--global]] [<model-id>] | <model-id> <prompt>', |
中文说明
这个 argumentHint(以及上方的 description getter)仍把 --project/--global 呈现为独立选项,但 modelCommand.ts:1072 的新守卫(scopeOverride && !persistDefault && !inlinePrompt)会在没有 --default 时拒绝主模型的 /model --project <id> / /model --global <id>。— 失败场景:用户阅读 /model 帮助(通过 /help、tab 补全或这个提示),按文案字面意思运行 /model --project qwen-max 想持久化到项目作用域,却得到 "Use --default with --project or --global when persisting the main model."。这些子句对子模型命令仍然准确(如 /model --voice --project <id>),使得不一致很隐蔽——文案只在主模型(最常见的情况)上是错的。值得注意的是,本 diff 已经更新了裸 /model 非交互消息去掉了这些形式,却漏掉了 description getter、这个提示以及 --project/--global 的 tab 补全描述。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 10/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 10/100 轮)。改动内容与我反驳保留之处如下: Review feedback addressed on PR #6579All four inline suggestions from the automated reviewer were verified against the code and implemented. Findings1. Loose scope matcher in
|
|
📊 Takeover milestone — round 10/100, in the current window. Census: 10 pushed fix(es), 1 no-change review(s), 0 timeout(s), 0 rejected attempt(s), 0 other round(s) (crash / model error / gate error / infra), 1 base update(s). This many rounds deserves a human look. Options: keep going (fine — nothing changes), split or reduce the PR if rounds keep accumulating, or release takeover (remove the 中文说明📊 接管里程碑 —— 第 10/100 轮(当前窗口)。统计:推送修复 10 次、审阅无需改动 1 次、超时 0 次、验证拒绝 0 次、其他轮次(崩溃/模型错误/门错误/infra)0 次、base 更新 1 次。 轮次到这个量值得人工看一眼。可选:继续(无需操作);若轮次持续累积,考虑拆分或缩减 PR;或释放接管(移除 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| 'Switch the model for this session (--fast for suggestion model, --voice for voice transcription model, [model-id] to switch immediately).', | ||
| 'Switch the model for this session (--fast for suggestion model, --voice for voice transcription model, --vision for the vision bridge model, --project to persist to project settings, --global to persist to user settings, [model-id] to switch immediately, or [model-id] [prompt] to run a one-off prompt on another model; the inline prompt is sent verbatim without @file expansion).': | ||
| 'Switch the model for this session (--fast for suggestion model, --voice for voice transcription model, --vision for the vision bridge model, --project to persist to project settings, --global to persist to user settings, [model-id] to switch immediately, or [model-id] [prompt] to run a one-off prompt on another model; the inline prompt is sent verbatim without @file expansion).', | ||
| 'Switch the model for this session (--fast for suggestion model, --voice for voice transcription model, --vision for the vision bridge model, --image for the image generation model, --project to persist to project settings, --global to persist to user settings, [model-id] to switch immediately, or [model-id] [prompt] to run a one-off prompt on another model; the inline prompt is sent verbatim without @file expansion).': |
There was a problem hiding this comment.
[Suggestion] Two of the /model description-key variants added in this diff are never referenced by any code or test. The command description is now a single static t() call (modelCommand.ts:385) using only the full --default + --compaction + --image variant, so the --image-without---default variant (this line) and the --default-without---compaction variant (en.js:1497) are dead keys, as are their newly added zh.js / zh-TW.js translations. — Concrete cost: six added key/value entries (en + zh + zh-TW × 2 variants) that can never be displayed, imply a conditional description assembly that no longer exists, must be kept in sync on every future wording change, and mislead translators into maintaining unreachable strings.
Fix: drop the two unreferenced variants (en.js:1495-1498 and their zh.js:1435-1438 / zh-TW.js:1321-1324 translations); keep only the full --default + --compaction + --image key that modelCommand.ts:385 actually passes to t().
中文说明
本 diff 新增的 /model 描述键中有两个变体没有被任何代码或测试引用。命令描述现在是一处静态的 t() 调用(modelCommand.ts:385),只使用包含 --default + --compaction + --image 的完整变体,因此不含 --default 的 --image 变体(本行)和不含 --compaction 的 --default 变体(en.js:1497)都是死键,它们在 zh.js / zh-TW.js 中新增的翻译同样是死键。——具体代价:新增的 6 条 key/value(en + zh + zh-TW × 2 个变体)永远不会被显示,暗示了一种已不存在的条件式描述拼装,今后每次改文案都要同步维护,还会误导翻译者去维护不可达的字符串。
建议:删除这两个未被引用的变体(en.js:1495-1498 及其 zh.js:1435-1438 / zh-TW.js:1321-1324 翻译),只保留 modelCommand.ts:385 实际传入 t() 的完整变体。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
There was a problem hiding this comment.
Addressed in 237b2ed. I removed both unreferenced description variants from en, zh, and zh-TW, leaving only the full --default + --compaction + --image key used by modelCommand. npm run check-i18n passes.
| if (persistDefault) { | ||
| persistModelSelection( | ||
| settings, | ||
| effectiveModelId, | ||
| effectiveBaseUrl, | ||
| persistScope, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] Three user-facing docs pages still describe the pre-PR behavior — that ordinary /model picker selections persist — which this diff changes to session-only. After this change, docs/users/configuration/auth.md:300 says of the /model picker "Your selection is persisted across sessions"; docs/users/configuration/model-providers.md:677 says "/model and /auth persist model.name ... and security.auth.selectedType"; docs/users/configuration/settings.md:164 says model.baseUrl is "Persisted automatically by the model picker". All three are now true only for the --default path (this gate). — Failure scenario: a user follows the docs to switch models via the ordinary picker, expecting it to persist; the selection is now session-only, so they silently revert to the old model on next launch with no doc-provided hint that --default exists.
Fix: update the three pages to state that plain /model <id> and ordinary picker selections are session-scoped, and that /model --default <id> (or the picker opened via /model --default) persists model.name / model.baseUrl / security.auth.selectedType.
中文说明
有三个面向用户的文档页面仍在描述本 PR 之前的行为——即普通的 /model 选择器会持久化——而本 diff 已将其改为仅当前会话生效。改动后,docs/users/configuration/auth.md:300 仍说 /model 选择器“您的选择会跨会话持久化”;model-providers.md:677 仍说“/model 和 /auth 会持久化 model.name……和 security.auth.selectedType”;settings.md:164 仍说 model.baseUrl “由模型选择器自动持久化”。这三处现在都只对 --default 路径成立(即此处的守卫)。——失败场景:用户按文档用普通选择器切换模型并期望持久化,但现在只对当前会话生效,下次启动时会静默恢复到旧模型,而文档没有任何提示说明已存在 --default。
建议:更新这三个页面,说明普通 /model <id> 和普通选择器选择仅当前会话生效,而 /model --default <id>(或通过 /model --default 打开的选择器)才会持久化 model.name / model.baseUrl / security.auth.selectedType。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
There was a problem hiding this comment.
Addressed in 237b2ed. The authentication, model-provider, and settings docs now distinguish session-scoped plain /model selections from persistent /model --default selections, including model.name, model.baseUrl, and the selected auth type. Formatting and typecheck pass.
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix: no action this roundNo actionable feedback was found for this round.
The only feedback present is in the Deferred non-Critical feedback section. Critical-only mode is active after 10 change-producing rounds, so those non-Critical items are an audit record for human follow-up — not work for this round. Per the workflow rules, no code was modified, no threads were resolved, and no comment replies were written for them. No changes were made to the working tree. 中文说明Autofix:本轮无需处理本轮未发现可执行的反馈。
唯一存在的反馈位于 Deferred non-Critical feedback(已延后的非 Critical 反馈) 区域。在完成 10 个产生改动的轮次后已进入仅处理 Critical 的模式,因此这些非 Critical 条目属于供人工跟进的审计记录,而非本轮的工作内容。按照工作流规则,未对它们修改任何代码、未解决任何线程,也未撰写任何评论回复。 工作树未做任何改动。 Deferred non-Critical feedbackCritical-only mode is active after 10 change-producing rounds. 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. (
中文说明完成 10 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: No action required this round (PR #6579)The only feedback newer than the last evaluation consists of two inline comments that are this workflow's own previous-round "Addressed in 237b2ed" status replies, echoed back by the feedback filter. They contain no new requests or defects. Both underlying findings were re-verified against the current head (
Dispositions: both findings Resolved in code (both rc handles are listed in No checks are failing, the branch has no conflict with 中文说明本轮无需任何改动(PR #6579)上次评估之后新增的反馈仅有两条行内评论,它们是本工作流上一轮自己发布的「Addressed in 237b2ed(已在 237b2ed 中处理)」状态回复,被反馈过滤器回传。这两条评论不包含任何新的要求或缺陷。它们对应的两个原始发现已在当前 head(
处置结论:两个发现均已在代码中解决(两个 rc 句柄已列入 当前没有失败的检查,分支与 Deferred non-Critical feedbackCritical-only mode is active after 10 change-producing rounds. 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. ( 中文说明完成 10 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
[Critical] parseDefaultFlag misses a trailing --default after a whitespace run — silent persistence loss (packages/cli/src/ui/commands/modelCommand.ts:76-88). The new detection code splits on single whitespace (args.trim().split(/\s/)) and checks token index 0/1; a whitespace RUN (two spaces or space+tab) between the model id and the flag yields ['qwen-max', '', '--default'], so leadingTokens[1] === '' and persistDefault stays false. The undetected --default text then survives as the inline prompt, passes every guard, and the action returns submit_prompt with content '--default' — the literal flag text is sent to the model as a one-shot prompt, nothing is persisted, and even the session switch does not happen. Probe-verified at this commit: /model qwen-max --default (double space) and /model qwen-max \t--default (space+tab) both reproduce; the single-space forms work. Failure scenario: a user types or pastes /model qwen-max --default with two spaces (common when pasting from wrapped docs) — instead of persisting the default, the CLI sends the literal text '--default' to the model and reports nothing wrong; the next session silently starts on the old model. This is the exact silent-intent-loss class this PR's own inline-prompt guard was added to prevent. Fix (probe-verified): const leadingTokens = args.trim().split(/\s+/); — preserves the 'qwen-max what does --default do' prompt-body case; add a trailing-flag test with a double space. Not posted inline because an existing comment already occupies the same anchor line (a test-coverage Suggestion that presumes this branch works — a different claim, so this Critical is reported here instead of being dropped). (Note: the related but distinct trailing --default after an AUXILIARY flag, e.g. /model --fast <id> --default, is a second gap at the same function — it bypasses the mutual-exclusion rejection and is folded into the aux model name, producing a misleading 'model not configured' error; it shares the detection root cause.)
中文说明
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
[Critical] parseDefaultFlag misses a trailing --default after a whitespace run — silent persistence loss (packages/cli/src/ui/commands/modelCommand.ts:76-88). The new detection code splits on single whitespace (args.trim().split(/\s/)) and checks token index 0/1; a whitespace RUN (two spaces or space+tab) between the model id and the flag yields ['qwen-max', '', '--default'], so leadingTokens[1] === '' and persistDefault stays false. The undetected --default text then survives as the inline prompt, passes every guard, and the action returns submit_prompt with content '--default' — the literal flag text is sent to the model as a one-shot prompt, nothing is persisted, and even the session switch does not happen. Probe-verified at this commit: /model qwen-max --default (double space) and /model qwen-max \t--default (space+tab) both reproduce; the single-space forms work. Failure scenario: a user types or pastes /model qwen-max --default with two spaces (common when pasting from wrapped docs) — instead of persisting the default, the CLI sends the literal text '--default' to the model and reports nothing wrong; the next session silently starts on the old model. This is the exact silent-intent-loss class this PR's own inline-prompt guard was added to prevent. Fix (probe-verified): const leadingTokens = args.trim().split(/\s+/); — preserves the 'qwen-max what does --default do' prompt-body case; add a trailing-flag test with a double space. Not posted inline because an existing comment already occupies the same anchor line (a test-coverage Suggestion that presumes this branch works — a different claim, so this Critical is reported here instead of being dropped). (Note: the related but distinct trailing --default after an AUXILIARY flag, e.g. /model --fast <id> --default, is a second gap at the same function — it bypasses the mutual-exclusion rejection and is folded into the aux model name, producing a misleading 'model not configured' error; it shares the detection root cause.)
— qwen3.8-max via Qwen Code /review (v0.21.3)
| ``` | ||
|
|
||
| The picker will show all models from your `modelProviders` configuration, grouped by their protocol (e.g. `openai`, `anthropic`, `gemini`). Your selection is persisted across sessions. | ||
| The picker will show all models from your `modelProviders` configuration, grouped by their protocol (e.g. `openai`, `anthropic`, `gemini`). A selection from the ordinary `/model` picker applies only to the current session. To persist a selection for future sessions, use `/model --default <model-id>` or open the picker with `/model --default` and select a model. |
There was a problem hiding this comment.
[Suggestion] docs/users/features/commands.md was not updated for the new persistence semantics — Failure scenario: a user upgrading from the pre-PR behavior (plain /model <id> persisted the model) opens the command reference to find how to persist a default model and sees the /model, /model --fast, /model --voice, /model --vision rows with no --default anywhere on the page — concluding the persist capability was removed rather than moved behind a flag.
That page contains zero --default mentions at this commit (grep-verified), and no note that plain /model switches are now session-scoped, while this diff updated three other docs pages (auth.md — this hunk, model-providers.md, settings.md) for exactly this change. The maintainer review (item 6) asked for a docs line because this is an intentional breaking change to behavior people rely on; the file is not touched by any commit in this PR.
Suggested fix: add a /model --default row next to the existing /model row (e.g. | → --default | Persist the main model for future sessions | /model --default <model-id>, /model --default --project <model-id> |) and note in the /model row that plain switches are session-scoped.
中文说明
[Suggestion] docs/users/features/commands.md 没有针对新的持久化语义更新 —— 失败场景:从旧行为(普通 /model <id> 会持久化模型)升级的用户打开命令参考,想找到持久化默认模型的方法,却只看到 /model、/model --fast、/model --voice、/model --vision 行,整页没有任何 --default —— 会以为持久化能力被移除了,而不是挪到了标志后面。
该页面在当前提交中没有出现任何 --default(已用 grep 验证),也没有说明普通 /model 切换现在是会话级的;而本 diff 为同一语义变更更新了另外三个文档页面(auth.md —— 即本行、model-providers.md、settings.md)。维护者评审(第 6 项)要求补一行文档,因为这是用户依赖的行为上有意的破坏性变更;该文件不在本 PR 的任何提交中。
建议修复:在现有 /model 行旁增加 /model --default 行(例如 | → --default | 为后续会话持久化主模型 | /model --default <model-id>、/model --default --project <model-id> |),并在 /model 行注明普通切换仅作用于当前会话。
— qwen3.8-max via Qwen Code /review (v0.21.3)
| firstSpace === -1 ? trimmedArgs : trimmedArgs.slice(0, firstSpace); | ||
| const inlinePrompt = | ||
| firstSpace === -1 ? '' : trimmedArgs.slice(firstSpace + 1).trim(); | ||
| if (scopeOverride && !persistDefault && !inlinePrompt) { |
There was a problem hiding this comment.
[Suggestion] No test covers this guard's model-bearing form — /model --project qwen-max / /model --global qwen-max (scope flag without --default, WITH a model id) in a trusted workspace — Concrete cost: the gate is tested only with '--project'/'--global' alone (no model) and via the inline-prompt path; the only '--project qwen-max' test runs untrusted and short-circuits at the earlier trust check, never reaching this gate (the rejection message string occurs nowhere in the test file). Mutation probe at this commit: narrowing the gate to the no-model path (&& !modelName) ships green (98/98 pass) — trusted /model --project qwen-max would then silently session-switch instead of rejecting, dropping the requested persistence without any error: precisely the silent-intent-loss this PR exists to prevent.
Suggested fix (in modelCommand.test.ts, trusted context):
it('should reject --project with a model id when --default is absent', async () => {
const ctx = setupContext();
const result = await modelCommand.action!(ctx, '--project qwen-max');
expect(result).toMatchObject({
type: 'message',
messageType: 'error',
content: expect.stringContaining(
'Use --default with --project or --global when persisting the main model.',
),
});
expect(setValue).not.toHaveBeenCalled();
});(and a mirrored --global qwen-max case)
中文说明
[Suggestion] 该守卫的“带模型 ID”形式没有测试覆盖 —— 即受信工作区下的 /model --project qwen-max / /model --global qwen-max(有作用域标志、有模型 ID、没有 --default)—— 具体代价:目前只有 '--project'/'--global'(不带模型)以及内联 prompt 路径测试了这个守卫;唯一的 '--project qwen-max' 测试在非受信环境下运行,会在更早的信任检查处短路,根本到不了这个守卫(拒绝消息字符串在整个测试文件中零出现)。在本提交上做了变异探针:把守卫收窄到无模型路径(&& !modelName)后测试套件仍然全绿(98/98 通过)—— 届时受信的 /model --project qwen-max 会静默变成仅会话级切换而不再报错,用户请求的持久化被无声丢弃:这正是本 PR 要消灭的“静默丢失意图”。
建议修复:在 modelCommand.test.ts 受信上下文中补充上述测试(以及对称的 --global qwen-max 用例),断言返回拒绝消息且 setValue 未被调用。
— qwen3.8-max via Qwen Code /review (v0.21.3)
|
🤖 Addressed the latest review feedback (round 11/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 11/100 轮)。改动内容与我反驳保留之处如下: Review feedback round — PR #6579All three findings this round were addressed and resolved in code (commit 1. [Critical]
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Critical] R4-6: The PR branch conflicts with main — GitHub reports mergeStateStatus: DIRTY, mergeable: CONFLICTING at review time (checked 2026-08-04 via gh pr view, head 44be9d799). This is blocker (3) of the maintainer verification review (issue comment 5073042997), which required a merge/rebase before landing; it was reported cleared on 2026-08-03 but main has since advanced and the conflict is live again at the reviewed head. The PR cannot be merged as-is — merge current main into the branch (or rebase) and resolve the conflicts. Failure scenario: merging now fails on conflicts; certifying this head without naming the conflict would approve a branch that cannot land.
— qwen3.8-max via Qwen Code /review (v0.21.3)
| > Define `modelProviders` in the user-scope `~/.qwen/settings.json` whenever possible and avoid persisting credential overrides in any scope. Keeping the provider catalog in user settings prevents merge/override conflicts between project and user scopes and ensures `/auth` and `/model` updates always write back to a consistent scope. | ||
|
|
||
| - `/model` and `/auth` persist `model.name` (where applicable) and `security.auth.selectedType` to the closest writable scope that already defines `modelProviders`; otherwise they fall back to the user scope. This keeps workspace/user files in sync with the active provider catalog. | ||
| - Plain `/model <model-id>` commands and selections from the ordinary `/model` picker apply only to the current session. Use `/model --default <model-id>` or select a model from the picker opened by `/model --default` to persist `model.name`, `model.baseUrl` when needed for disambiguation, and the selected provider's `security.auth.selectedType`. `/auth` continues to persist its selected authentication type. Persistent selections use the closest writable scope that already defines `modelProviders`; otherwise they fall back to the user scope. |
There was a problem hiding this comment.
[Suggestion] This new sentence bundles the persistence effects of both --default forms, but they differ: the argument form /model --default <model-id> persists model.name, clears model.baseUrl (the "selects by id only" tombstone in switchMainModel), and persists security.auth.selectedType only when the argument carries an explicit auth-type suffix. Only the picker path (persistModelSelection in ModelDialog.tsx) persists the real disambiguating model.baseUrl. — Failure scenario: a user with two modelProviders entries sharing the same model id follows this doc, runs /model --default qwen-max expecting the disambiguator to be persisted; instead the previously persisted model.baseUrl is wiped, so a later session can resolve the id to a different provider than intended.
Suggested wording — split the claim by form, e.g.: "Use /model --default <model-id> to persist model.name (it selects by id only and clears any persisted model.baseUrl disambiguator; an explicit auth-type suffix also persists security.auth.selectedType), or select a model from the picker opened by /model --default to additionally persist the disambiguating model.baseUrl and the selected provider's security.auth.selectedType."
中文说明
[建议] 这句新文案把两种 --default 形式的持久化效果写成了一样,但两者并不相同:参数形式 /model --default <model-id> 只持久化 model.name、会清空 model.baseUrl(switchMainModel 中"仅按 id 选择"的墓碑写入),且只有当参数带显式认证类型后缀时才持久化 security.auth.selectedType;只有选择器路径(ModelDialog.tsx 的 persistModelSelection)才会持久化真正用于消歧的 model.baseUrl。—— 失败场景:用户配置了两个同 model id 的 modelProviders 条目,按文档运行 /model --default qwen-max,以为消歧符会被持久化;实际上之前持久化的 model.baseUrl 被清掉,之后的会话可能把该 id 解析到另一个 provider。
建议措辞——按形式拆分表述,例如:"/model --default <model-id> 持久化 model.name(仅按 id 选择,会清除已持久化的 model.baseUrl 消歧符;带显式认证类型后缀时还会持久化 security.auth.selectedType);或通过 /model --default 打开的选择器选择模型,额外持久化用于消歧的 model.baseUrl 及所选 provider 的 security.auth.selectedType。"
— qwen3.8-max via Qwen Code /review (v0.21.3)
| | `model.name` | string | The Qwen model to use for conversations. | `undefined` | | ||
| | `model.reasoningEffort` | enum | How hard reasoning-capable models think, applied across all providers. Set with the [`/effort`](../features/commands) command (`low`, `medium`, `high`, `xhigh`, `max`). Each provider maps and clamps this to what the active model supports (e.g. Gemini caps at `high`; Anthropic clamps tiers a model lacks). Leave unset to use the model/provider default. | `undefined` | | ||
| | `model.baseUrl` | string | Persisted automatically by the model picker to disambiguate when multiple `modelProviders` entries share the same model id. Not intended to be set by hand — use the `/model` picker or a `modelProviders` entry instead; a stale hand-edited value can silently route requests to a different same-id provider. | `undefined` | | ||
| | `model.baseUrl` | string | Persisted with a default model selection (`/model --default <model-id>` or the picker opened with `/model --default`) to disambiguate when multiple `modelProviders` entries share the same model id. Plain model switches are session-scoped. Not intended to be set by hand — use the default-model flow or a `modelProviders` entry instead; a stale hand-edited value can silently route requests to a different same-id provider. | `undefined` | |
There was a problem hiding this comment.
[Suggestion] The new model.baseUrl description claims the /model --default <model-id> argument form persists the disambiguator, but that form deliberately clears it (switchMainModel writes model.baseUrl as '' with the comment "/model --default <id> selects by id only, so clear any baseUrl disambiguator"). Only a selection made through the picker opened with /model --default persists a real model.baseUrl. — Failure scenario: a user with two same-id modelProviders entries who previously persisted provider B via the --default picker runs /model --default qwen-max to pin the default; the disambiguator is silently cleared, and on next launch the resolver picks the first same-id entry — possibly provider A — while this row (whose stated purpose is warning about exactly that hazard) said disambiguation was persisted.
Suggested fix: attribute the baseUrl persistence to the picker flow only, e.g. "Persisted with a default model selection made through the picker opened with /model --default. The /model --default <model-id> argument form selects by id only and clears any persisted disambiguator."
中文说明
[建议] 新增的 model.baseUrl 描述声称 /model --default <model-id> 参数形式会持久化消歧符,但该形式实际上是主动清空它(switchMainModel 把 model.baseUrl 写为 '',注释为"/model --default <id> 仅按 id 选择,因此清除之前模型选择器留下的 baseUrl 消歧符")。只有通过 /model --default 打开的选择器做出的选择才会持久化真实的 model.baseUrl。—— 失败场景:用户有两个同 id 的 modelProviders 条目,之前通过 --default 选择器持久化了 provider B;运行 /model --default qwen-max 固定默认模型后,消歧符被静默清除,下次启动时解析器会选择第一个同 id 条目(可能是 provider A),而这一行文档(其声明的目的正是警示该风险)却说消歧已被持久化。
建议修复:把 baseUrl 的持久化只归于选择器流程,例如"通过 /model --default 打开的选择器做出的默认模型选择会持久化它;/model --default <model-id> 参数形式仅按 id 选择,并会清除已持久化的消歧符。"
— qwen3.8-max via Qwen Code /review (v0.21.3)
| .replace(/(?:^|\s)--vision(?:\s|$)/, ' ') | ||
| .replace(/(?:^|\s)--default(?:\s|$)/, ' ') |
There was a problem hiding this comment.
[Suggestion] Tab completion completes model ids for --default + auxiliary-flag combinations that the action path now always rejects. This diff extended the completion-prefix strip with --default (this line), so /model --default --fast qwen3<TAB> offers qwen3-coder-flash (probe-verified: completion("--default --fast qwen3") => ['qwen3-coder-flash'], both flag orders), but accepting yields /model --default --fast qwen3-coder-flash, which the new mutual-exclusion guard ALWAYS rejects with "--default only applies to the main model…". Before this diff every completable flag combination was runnable. — Concrete cost: every accepted completion for these combinations is a guaranteed error; completion now advertises invocations this diff's own guard forbids.
Suggested fix: in the completion model-prefix branch, return no model completions when the partial arg contains both --default and an aux flag (AUX_MODEL_FLAGS).
中文说明
[建议] Tab 补全仍在为"--default + 辅助标志"组合补全模型 id,但 action 路径现在总是拒绝这些组合。本 diff 在补全前缀剥离链中新增了 --default(即本行),因此 /model --default --fast qwen3<TAB> 会给出 qwen3-coder-flash(已用探针验证:两种标志顺序均返回 ['qwen3-coder-flash']);接受补全后得到的 /model --default --fast qwen3-coder-flash 必然被新的互斥守卫以 "--default only applies to the main model…" 拒绝。本 diff 之前,所有可补全的标志组合都是可运行的。—— 具体代价:接受这些组合的补全必然报错;补全在宣传本 diff 自己的守卫所禁止的调用。
建议修复:在补全的模型前缀分支中,当输入同时包含 --default 与任一辅助标志(AUX_MODEL_FLAGS)时不返回模型补全。
— qwen3.8-max via Qwen Code /review (v0.21.3)
| // Persistence flags are consumed before the inline prompt path, which | ||
| // does not persist the model. Reject them instead of silently dropping | ||
| // the requested behavior. | ||
| if (persistDefault || scopeOverride) { |
There was a problem hiding this comment.
[Suggestion] In ACP mode the unchanged executionMode === 'acp' inline-rejection branch (just above) returns BEFORE this new flag-aware rejection, so under ACP the persistence-intent warning is unreachable. Probe-verified: ACP action("--default qwen-max fix the bug") returns the generic advice "run '/model qwen-max' first, then send your prompt"; following it performs a session-only switch (setValue never called) — the persistence intent is silently dropped from the guidance, the exact silent-drop this block was added to prevent. The two-step form IS valid under ACP (probe: /model --default qwen-max persists model.name), so this block's flag-aware advice would be actionable there. — Failure scenario: an ACP user runs /model --default qwen-max fix the bug, follows the advice, believes the default was persisted; the next session silently starts on the old model.
Suggested fix: move this if (persistDefault || scopeOverride) block above the if (context.executionMode === 'acp') block so the flag-aware message wins in every mode.
中文说明
[建议] 在 ACP 模式下,上方未改动的 executionMode === 'acp' 内联拒绝分支会先于本新增的"感知标志"拒绝返回,因此 ACP 下永远到不了这段持久化意图提示。已用探针验证:ACP 中 action("--default qwen-max fix the bug") 返回通用提示 "run '/model qwen-max' first, then send your prompt";照做只会执行会话级切换(setValue 从未被调用)——持久化意图被提示文案静默丢弃,而这正是本代码块要防止的问题。两步形式在 ACP 下是有效的(探针:/model --default qwen-max 会持久化 model.name),所以本块的标志感知提示在 ACP 下本可执行。—— 失败场景:ACP 用户运行 /model --default qwen-max fix the bug,按提示操作后以为默认模型已持久化;下一个会话却静默地仍以旧模型启动。
建议修复:把本 if (persistDefault || scopeOverride) 块移到 if (context.executionMode === 'acp') 块之前,使标志感知提示在所有模式下优先生效。
— qwen3.8-max via Qwen Code /review (v0.21.3)
| const flag = persistDefault | ||
| ? '--default' |
There was a problem hiding this comment.
[Suggestion] When only a scope flag (no --default) is combined with an inline prompt, this ternary yields '--default --project'/'--default --global', so the error reads "Cannot combine --default --project with an inline prompt" even though the user never typed --default (probe-verified: /model --project qwen-max fix the bug produces that message). The same mistake without a prompt hits the earlier guard and gets the accurate "Use --default with --project or --global when persisting the main model." — one mistake, two inconsistent explanations, one of which misquotes the invocation. The pre-diff ternary quoted the user's actual flags; the misquote is introduced by this diff's rewrite. The suggested command in the message is runnable; only the attribution is wrong. (Distinct from the deferred scope-flag-drop thread: that one is the persistDefault=true branch dropping the scope flag; this is --default fabricated when persistDefault is false.)
Suggested fix: when !persistDefault, phrase the message without attributing --default — e.g. reuse the sibling guard's "Use --default with --project or --global when persisting the main model." wording.
中文说明
[建议] 当只有作用域标志(没有 --default)与内联 prompt 组合时,该三元表达式会产出 '--default --project'/'--default --global',于是错误信息显示 "Cannot combine --default --project with an inline prompt"——尽管用户从未输入 --default(已用探针验证:/model --project qwen-max fix the bug 产生该消息)。同样的输入若不带 prompt,会命中更早的守卫并得到准确的 "Use --default with --project or --global when persisting the main model."—— 同一个错误,两种不一致的解释,其中一种还错误引用了用户的调用。diff 之前的三元表达式引用的是用户实际输入的标志;这个错误引用是本 diff 重写引入的。消息中建议的命令本身可运行,只是归因错误。(与已延期的"作用域标志被丢弃"线程不同:那是 persistDefault=true 分支丢掉了作用域标志;这里是 persistDefault 为 false 时凭空多出了 --default。)
建议修复:当 !persistDefault 时,消息措辞不要归因 --default——例如复用同级守卫的 "Use --default with --project or --global when persisting the main model." 文案。
— qwen3.8-max via Qwen Code /review (v0.21.3)
|
🤖 Addressed the latest review feedback (round 12/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 12/100 轮)。改动内容与我反驳保留之处如下: Autofix review round summary — PR #6579Feedback triageThis round ran in Critical-only mode (after 10 change-producing rounds). The workflow-provided feedback contained no actionable items:
Conflict resolution (
|
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
|
🤖 Addressed the latest review feedback (round 13/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 13/100 轮)。改动内容与我反驳保留之处如下: Autofix review round summary — PR #6579All findings this round were addressed. Two commits were added: a merge of current Feedback points and decisions1. R4-6 (Critical, review body) — merge conflict with
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| value: '--default', | ||
| description: t( |
There was a problem hiding this comment.
[Suggestion] No test asserts that --default is offered as a flag-completion candidate — all new completion tests use partials longer than any flag value, so flagCompletions filtering always yields [] and nothing reads the list's contents. — Failure scenario: mutation-probe verified at this commit — removing the --default entry from flagCompletions leaves all 104 tests green, while a probe asserting { value: '--default' } in the completion output flips between the mutated and restored trees; a future cleanup can silently drop tab-completion for /model --def<Tab> (the discoverability surface of this PR's headline flag) with no red test.
Suggested fix:
it('offers --default in flag completions', async () => {
const completions = await modelCommand.completion!(mockContext, '--def');
expect(completions).toEqual(
expect.arrayContaining([expect.objectContaining({ value: '--default' })]),
);
});中文说明
[建议] 没有任何测试断言 --default 会作为标志补全候选项出现——新增的补全测试所用的前缀都比任何标志值更长,因此 flagCompletions 过滤总是得到 [],没有测试读取该列表的内容。—— 失败场景:已在本提交上用变异探针验证——从 flagCompletions 中移除 --default 条目后,全部 104 个测试仍然通过;而断言补全结果包含 { value: '--default' } 的探针在变异前后会翻转。未来的清理可能在没有任何红色测试的情况下悄悄移除 /model --def<Tab> 的补全(本 PR 核心标志的可发现入口)。
建议修复:新增一个补全测试,断言 completion('--def') 的结果包含 { value: '--default', ... }。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| // The action path always rejects --default combined with an | ||
| // auxiliary model flag, so never offer model completions for it. |
There was a problem hiding this comment.
[Suggestion] Completion returns model-id candidates for scope-flag combinations the action always rejects — probe-verified at this commit: completion('--project q'), ('--global q'), ('--project --global q') and ('--default --project --global q') all return ['qwen-max'], while the action rejects /model --project qwen-max ("Use --default with --project or --global when persisting the main model.") and any --project --global input ("Cannot use both --project and --global."). The analogous --default+aux class guarded here has a suppression guard and a test; these shapes have neither. — Failure scenario: /model --project qw<Tab> completes to /model --project qwen-max, which always errors on execution — completion steers the user into an input the action provably rejects.
Suggested fix: extend this guard to also return null when both --project and --global are present (unconditionally), or when a single scope flag appears without --default and without an aux flag (keep /model --project --fast <id> completing — it is a valid aux dialog path); add the four partials to the suppression test expecting null.
中文说明
[建议] 补全会为 action 必然拒绝的作用域标志组合返回模型 ID 候选——已在本提交上用探针验证:completion('--project q')、('--global q')、('--project --global q') 和 ('--default --project --global q') 都返回 ['qwen-max'],而 action 会拒绝 /model --project qwen-max("Use --default with --project or --global when persisting the main model.")以及任何 --project --global 输入("Cannot use both --project and --global.")。此处类似的 --default+辅助标志组合已有抑制守卫和测试,而这两种形态两者皆无。—— 失败场景:/model --project qw<Tab> 补全为 /model --project qwen-max,执行时必然报错——补全把用户引向 action 必然拒绝的输入。
建议修复:扩展该守卫——当同时出现 --project 和 --global 时无条件返回 null;或当单个作用域标志在没有 --default 且没有辅助标志时出现时也返回 null(保留 /model --project --fast <id> 的补全——它是合法的辅助对话框路径);并在抑制测试中加入这四个前缀,断言返回 null。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| const flag = hasProject | ||
| ? '--default --project' | ||
| : hasGlobal | ||
| ? '--default --global' | ||
| : '--default'; |
There was a problem hiding this comment.
[Suggestion] The scoped flag substitutions here are untested (mutation-quiet) — no test exercises /model --default --project <id> <prompt> or --default --global <id> <prompt>; all three existing "Cannot combine --default" tests reach only the --default-alone branch. — Failure scenario: mutation-probe verified — replacing this ternary with const flag = '--default'; leaves all 104 tests green; a flattened ternary would tell a user who typed /model --default --project qwen-max explain this to run /model --default qwen-max — which persists the default to user scope instead of the workspace scope they asked for, with no red test.
Suggested fix: add two action tests with args '--default --project qwen-max explain this code' and '--default --global qwen-max explain this code' asserting the full message including the scoped corrective command.
中文说明
[建议] 此处的作用域 flag 替换没有测试覆盖(变异静默)——没有测试执行 /model --default --project <id> <prompt> 或 --default --global <id> <prompt>;现有三个 "Cannot combine --default" 测试都只走到 --default 单独出现的分支。—— 失败场景:已经变异探针验证——把该三元表达式替换为 const flag = '--default'; 后全部 104 个测试仍通过;被压平的三元表达式会告诉输入了 /model --default --project qwen-max explain this 的用户去运行 /model --default qwen-max——这会把默认模型持久化到用户作用域而非其要求的工作区作用域,且没有红色测试。
建议修复:新增两个 action 测试,参数分别为 '--default --project qwen-max explain this code' 和 '--default --global qwen-max explain this code',断言完整消息(含带作用域的纠正命令)。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| ); | ||
| }); | ||
|
|
||
| it('persists the main model for the trailing-flag form qwen-max --default', async () => { |
There was a problem hiding this comment.
[Suggestion] No test combines trailing --default with a scope flag — /model --project qwen-max --default works today only because parseScopeFlags strips --project before parseDefaultFlag checks token positions, and that ordering dependency is untested. — Failure scenario: mutation-probe verified — swapping the parse order (running parseDefaultFlag first) makes /model --project qwen-max --default reject with "Use --default with --project or --global when persisting the main model." — telling the user to add the flag they already typed — while all 104 tests stay green.
Suggested fix: add a persistence test next to this one for '--project qwen-max --default' (and optionally '--global qwen-max --default') asserting the (this project default) suffix and setValue at SettingScope.Workspace / SettingScope.User.
中文说明
[建议] 没有测试把尾部 --default 与作用域标志组合——/model --project qwen-max --default 目前能工作,仅因为 parseScopeFlags 先剥掉 --project,parseDefaultFlag 才检查 token 位置,而这一顺序依赖没有测试覆盖。—— 失败场景:已经变异探针验证——交换解析顺序(先运行 parseDefaultFlag)会让 /model --project qwen-max --default 报 "Use --default with --project or --global when persisting the main model."——叫用户补上他们已经输入的标志——而全部 104 个测试仍然通过。
建议修复:在此测试旁新增 '--project qwen-max --default'(可选再加 '--global qwen-max --default')的持久化测试,断言 (this project default) 后缀以及 setValue 的 SettingScope.Workspace / SettingScope.User 作用域。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| expect(mockOpenModelDialog).toHaveBeenCalledWith({ | ||
| persistScope: 'user', | ||
| persistDefault: true, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] This new forwarding is pinned only in the positive direction — the pre-existing bare-dialog test ('should handle "dialog: model" action') asserts only toHaveBeenCalled() with no argument shape, so nothing catches persistDefault becoming unconditionally true here. — Failure scenario: mutation-probe verified — hardcoding persistDefault: true in the processor's case 'model' leaves all 239 tests across the four owning suites green (AppContainer.test.tsx mocks useModelCommand wholesale). Shipped, every plain /model would open the default-picker and any selection would silently write model.name/model.baseUrl/security.auth.selectedType — the exact persist-always regression this PR removes.
Suggested fix: in the pre-existing bare-dialog test assert the call shape — expect(mockOpenModelDialog).toHaveBeenCalledWith({ persistScope: undefined, persistDefault: undefined }) — or add a dedicated negative-case test next to this one.
中文说明
[建议] 这条新转发只被正向断言钉住——既有的裸对话框测试('should handle "dialog: model" action')只断言 toHaveBeenCalled(),不校验参数形状,因此此处 persistDefault 变成无条件为真也不会有测试发现。—— 失败场景:已经变异探针验证——把处理器 case 'model' 中的转发硬编码为 persistDefault: true 后,四个相关测试套件的全部 239 个测试仍通过(AppContainer.test.tsx 整体 mock 了 useModelCommand)。若带着该回归发布,每次普通 /model 都会打开默认模型选择器,任何选择都会静默写入 model.name/model.baseUrl/security.auth.selectedType——正是本 PR 要移除的"总是持久化"回归。
建议修复:在既有的裸对话框测试中把断言改为 expect(mockOpenModelDialog).toHaveBeenCalledWith({ persistScope: undefined, persistDefault: undefined }),或在本测试旁新增一个专门的反向用例。
— qwen3.8-max via Qwen Code /review (v0.21.5)
doudouOUC
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM!✅
— qwen3.7-max via Qwen Code /review (v0.21.4)
doudouOUC
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM!✅
— qwen3.7-max via Qwen Code /review (v0.21.4)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: No action this round (PR #6579)All actionable sections of this round's feedback are empty:
Critical-only mode is active after 10 change-producing rounds. The deferred non-Critical feedback listed in the feedback summary (one review and five inline findings from the automated reviewer) is an audit record only; per the workflow rules, no code changes were made, no threads were resolved, and no comment replies were written for those items. They remain open for human follow-up. No base-conflict resolution was needed ( Result: no code changes and no commits this round. 中文说明本轮无需处理(PR #6579)本轮反馈的所有可执行区域均为空:
在完成 10 个产生改动的轮次后,已进入仅处理 Critical 的模式。反馈摘要中列出的延后非 Critical 反馈(来自自动审查者的一条 review 和五条行内发现)仅作为审计记录;按照工作流规则,未对这些条目进行任何代码修改、未解决任何话题、也未撰写任何评论回复。它们保持开放,留待人工跟进。 无需解决 base 冲突( 结果: 本轮无代码改动,无新提交。 Deferred non-Critical feedbackCritical-only mode is active after 10 change-producing rounds. 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. (
中文说明完成 10 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |


What this PR does
This changes the main model switch flow so ordinary
/model <model-id>and ordinary model-picker selections only update the active session. Persisting the main model as the default now requires the explicit/model --default <model-id>path, and/model --defaultopens a default-setting picker. Scope flags for the main model are also explicit:/model --default --project ...writes the project default, and/model --default --global ...writes the user default.Auxiliary model settings keep their existing behavior:
/model --fast,/model --voice, and/model --visionstill persist their configured values.Why it's needed
The command description says
/modelswitches the model for this session, but the previous implementation also wrotemodel.name,model.baseUrl, and sometimessecurity.auth.selectedTypeto settings. That made a temporary model switch leak into later sessions and made it easy to accidentally change the global or project default.Reviewer Test Plan
How to verify
Run
/model <model-id>in a directory with configured models and confirm the active session switches whilesettings.jsonremains unchanged. Then run/model --default <model-id>and confirmsettings.jsonupdatesmodel.namefor future sessions. Also verify/model --default --project <model-id>and/model --default --global <model-id>write to the expected scope, while/model --fast,/model --voice, and/model --visioncontinue to persist their auxiliary settings.Evidence (Before & After)
Before: source-inspection repro from #4331 showed
/model <model-id>wrote persistent settings even though the command is described as session-scoped.After: local CLI verification with a temporary
QWEN_HOMEshowed/model gpt-4oprintedModel: gpt-4owhilesettings.jsonkeptmodel.name: gpt-4o-mini; running-p "/model --default gpt-4o"printedModel: gpt-4o (default)and updatedsettings.jsontomodel.name: gpt-4o.Focused tests passed:
cd packages/cli && npx vitest run src/ui/commands/modelCommand.test.ts src/ui/components/ModelDialog.test.tsx src/ui/hooks/slashCommandProcessor.test.ts.Full checks passed:
npm run buildandnpm run typecheck.Tested on
Environment (optional)
macOS, Node.js v22.23.1, local repo dev build via
npm run dev.Risk & Scope
/model <id>to update their default must now use/model --default <id>.Linked Issues
Closes #4331
中文说明
本 PR 做了什么
这个改动让主模型切换流程中普通的
/model <model-id>和普通模型选择器只更新当前会话。要把主模型持久化为默认值,现在必须显式使用/model --default <model-id>;/model --default会打开设置默认模型的选择器。主模型的作用域标志也变为显式语义:/model --default --project ...写项目默认值,/model --default --global ...写用户默认值。辅助模型设置保持原有行为:
/model --fast、/model --voice、/model --vision仍然会持久化对应配置值。为什么需要
命令描述说
/model是切换当前会话的模型,但旧实现还会把model.name、model.baseUrl,有时还有security.auth.selectedType写入 settings。这会让一次临时模型切换泄漏到后续会话,也很容易意外改掉全局或项目默认模型。Reviewer Test Plan
如何验证
在配置了模型的目录中运行
/model <model-id>,确认当前会话模型切换,但settings.json不变。然后运行/model --default <model-id>,确认settings.json的model.name被更新并影响后续会话。也可以验证/model --default --project <model-id>和/model --default --global <model-id>写入预期作用域,而/model --fast、/model --voice、/model --vision仍然持久化辅助设置。证据(Before & After)
Before:#4331 中的源码级复现显示
/model <model-id>会写持久设置,尽管命令文案说它是会话级切换。After:我用临时
QWEN_HOME做了本地 CLI 验证,/model gpt-4o输出Model: gpt-4o,但settings.json仍保持model.name: gpt-4o-mini;运行-p "/model --default gpt-4o"后输出Model: gpt-4o (default),并把settings.json更新为model.name: gpt-4o。聚焦测试已通过:
cd packages/cli && npx vitest run src/ui/commands/modelCommand.test.ts src/ui/components/ModelDialog.test.tsx src/ui/hooks/slashCommandProcessor.test.ts。完整检查已通过:
npm run build和npm run typecheck。测试平台
环境(可选)
macOS,Node.js v22.23.1,本地仓库 dev build,通过
npm run dev验证。风险与范围
/model <id>更新默认模型,现在需要改用/model --default <id>。关联 Issue
Closes #4331