feat(cli): add --project and --global flags to /model for per-project model persistence#6060
feat(cli): add --project and --global flags to /model for per-project model persistence#6060Alex-ai-future wants to merge 11 commits into
Conversation
| let scopeOverride: SettingScope | undefined; | ||
| let remaining = args; | ||
|
|
||
| if (remaining.startsWith('--project')) { |
There was a problem hiding this comment.
Bug: parseScopeFlags only recognizes --project/--global at the start of the argument string.
startsWith('--project') means /model --fast --project my-model silently drops the scope flag. The fast-model branch then strips --fast and treats --project my-model as the model name, which fails resolveModelId() and produces a confusing error:
Fast model '--project my-model' is not configured for any auth type.
Same issue for --vision --global, --voice --global, etc. The argumentHint lists these flags flat ([--fast|--voice|--vision|--project|--global]), so users will expect order-independent composition.
Suggested fix — extract scope flags from anywhere in the string before dispatching:
function parseScopeFlags(args: string) {
let scopeOverride: SettingScope | undefined;
let remaining = args;
if (/\b--project\b/.test(remaining)) {
scopeOverride = SettingScope.Workspace;
remaining = remaining.replace(/\s*--project\s*/, ' ').trim();
} else if (/\b--global\b/.test(remaining)) {
scopeOverride = SettingScope.User;
remaining = remaining.replace(/\s*--global\s*/, ' ').trim();
}
return { scopeOverride, remaining };
}The same fix should be applied to the completion branch (~line 299–310) where --project / --global are only stripped when they appear at the start of trimmed.
| dialog: 'model', | ||
| ...(scopeOverride === SettingScope.Workspace | ||
| ? { persistScope: 'workspace' as const } | ||
| : scopeOverride === SettingScope.User |
There was a problem hiding this comment.
[Critical] persistScope is correctly propagated for the main 'model' dialog here, but silently dropped for all three subcommand dialog paths: 'voice-model', 'fast-model', and 'vision-model'. Their return objects (lines ~355, ~424, ~514) omit persistScope, and the corresponding cases in slashCommandProcessor.ts (lines ~830, ~833, ~836) call openModelDialog({ fastModelMode: true }) etc. without forwarding result.persistScope.
The result: /model --project --fast opens the model picker, the user selects a model, and it silently writes to the default scope (potentially global) instead of the requested project scope. No error, no warning.
Both layers need fixing:
- Add
persistScopeto each subcommand dialog return (same spread pattern as above):
return {
type: 'dialog',
dialog: 'fast-model',
...(scopeOverride === SettingScope.Workspace
? { persistScope: 'workspace' as const }
: scopeOverride === SettingScope.User
? { persistScope: 'user' as const }
: {}),
};- Forward it in
slashCommandProcessor.ts:
case 'fast-model':
actions.openModelDialog({ fastModelMode: true, persistScope: result.persistScope });
case 'voice-model':
actions.openModelDialog({ voiceModelMode: true, persistScope: result.persistScope });
case 'vision-model':
actions.openModelDialog({ visionModelMode: true, persistScope: result.persistScope });— qwen3.7-max via Qwen Code /review
| @@ -769,13 +793,17 @@ export function ModelDialog({ | |||
| width="100%" | |||
There was a problem hiding this comment.
[Suggestion] When both persistScope and a subcommand mode are set, the user sees "Select Model (this project)" instead of a combined title like "Select Fast Model (this project)". The persistScope check at the top of the ternary overrides all subcommand-specific titles.
Currently masked by the Critical finding above (scope never reaches subcommand dialogs), but will surface immediately once that's fixed. Consider combining both dimensions:
{isVoiceModelMode
? t('Select Voice Model') + (persistScope === 'workspace' ? ' (this project)' : persistScope === 'user' ? ' (global)' : '')
: isVisionModelMode
? t('Select Vision Model') + scopeLabel
: isFastModelMode
? t('Select Fast Model') + scopeLabel
: t('Select Model') + scopeLabel}— qwen3.7-max via Qwen Code /review
| @@ -345,7 +398,7 @@ export const modelCommand: SlashCommand = { | |||
| }; | |||
There was a problem hiding this comment.
[Suggestion] The main model path correctly shows scope-aware confirmation (Model: {{model}} (project) / Model: {{model}} (global)), but the voice, fast, and vision subcommand paths show no scope indication:
- Line ~405:
t('Voice Model') + ': ' + modelName - Line ~489:
t('Fast Model') + ': ' + modelName - Line ~589:
t('Vision Model') + ': ' + modelName + visionWarning
Users can't tell from the confirmation whether the write went to project or global scope. Apply the same contentKey pattern used for the main model:
const scopeSuffix =
scopeOverride === SettingScope.Workspace ? ' (project)' :
scopeOverride === SettingScope.User ? ' (global)' : '';
content: t('Voice Model') + ': ' + modelName + scopeSuffix,— qwen3.7-max via Qwen Code /review
| expect(modelCommand.name).toBe('model'); | ||
| expect(modelCommand.description).toBe( | ||
| 'Switch the model for this session (--fast for suggestion model, --voice for voice transcription model, --vision for the vision bridge 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).', |
There was a problem hiding this comment.
[Suggestion] The only test change in this PR is the description string update. There are no tests for:
parseScopeFlags()functionresolveScope()with scope overrides- Scope-aware
persistSetting()calls - Scope appearing in confirmation messages
persistScopein dialog return objects- Scope flag +
--voice/--fast/--visionflag interactions
The Critical finding (persistScope silently dropped for subcommand dialogs) would have been caught by a single test asserting that /model --project --fast returns a dialog action with persistScope: 'workspace'.
— qwen3.7-max via Qwen Code /review
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
Incremental Review — 9e18b4a (fix: TS index signature)
Scope: 1 file, +12/-5 in modelCommand.test.ts
Change: Replaces Record<string, unknown> casts with Partial<Config> & { [key: string]: unknown } in three places to fix a TypeScript index signature compatibility error.
Verdict: LGTM. Clean, minimal type fix. Config is already imported, and the mocked methods (getAvailableModelsForAuthType, getAllConfiguredModels) are real Config class methods. No behavioral changes.
No high-confidence issues found.
| @@ -544,7 +566,7 @@ export function ModelDialog({ | |||
| return; | |||
There was a problem hiding this comment.
[Suggestion] The resolvePersistScope change is correctly applied here (and at lines 585, 639), but the corresponding historyManager.addItem calls at lines 571, 592, 652 don't indicate which scope was written. When a user runs /model --project → picks from the dialog, they see "Voice Model: <name>" without any scope hint — unlike the CLI direct-set path which correctly shows "Voice Model: <name> (project)".
Consider adding a scope suffix to the dialog history items for consistency:
const scopeSuffix = persistScope === 'workspace' ? t(' (this project)')
: persistScope === 'user' ? t(' (global)') : '';
// ...
text: `${t('Voice Model')}: ${voiceModel}${scopeSuffix}`,Same pattern applies to the fast model (line 592), vision model (line 652), and main model (line 208) history items.
— qwen3.7-max via Qwen Code /review
… model persistence Add scope control to the /model command so users can persist model selections to either project-level or user-level settings independently. - /model --project: persist to workspace .qwen/settings.json - /model --global: persist to user ~/.qwen/settings.json - /model (no flag): unchanged behavior (backward compatible) - Model dialog title shows scope: 'Select Model (this project)' / 'Select Model (global)' - Completion and argumentHint updated with new flags - Full i18n support for zh/en Closes QwenLM#6052 Signed-off-by: Alex <alex.tech.lab@outlook.com>
Signed-off-by: Alex <alex.tech.lab@outlook.com>
…itles, tests - parseScopeFlags: use (?:^|\s) instead of \b for --flag matching (\b fails because - is not a word character) - Completion: strip all flags to isolate model prefix, supports any order - Subcommand dialogs (fast/voice/vision) now propagate persistScope - slashCommandProcessor forwards persistScope for all subcommand cases - ModelDialog title combines subcommand mode + scope label e.g. 'Select Fast Model (this project)' - Subcommand confirmations show scope suffix (project/global) - Extract persistScopeSpread() helper to reduce duplication - Add 9 tests covering scope flags, dialog returns, confirmations - Add i18n keys for scope suffix labels in zh/en/zh-TW Signed-off-by: Alex <alex.tech.lab@outlook.com>
…ignature TS error
Replace Record<string,unknown> with Partial<Config> & {[key:string]:unknown}
to satisfy TS4111 index signature access rule in the CI build.
Signed-off-by: Alex <alex.tech.lab@outlook.com>
Address review comment: historyManager.addItem for voice/fast/vision/main model selections now shows scope indicator like ' (this project)' or ' (global)', consistent with CLI direct-set confirmations. Affected: handleModelSwitchSuccess (main), handleSelect (voice/fast/vision) Signed-off-by: Alex <alex.tech.lab@outlook.com>
Alex-ai-future
force-pushed
the
dive/muti-model
branch
from
58deb19 to
f1589c4
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
Incremental Review — f1589c4 (scope suffix for ModelDialog history items)
Scope reviewed: 7 source files + 3 i18n files (+394/-47 vs main). Incremental since 9e18b4a: 5 commits including 459730456 (address PR review feedback), f1589c4be (scope suffix for history items), and upstream merges.
Changes since last review (9e18b4a)
ModelDialog.tsx—scopeSuffix(project/global) now appended tohistoryManager.addItemtext in all 4 model-switch success paths: main, voice, fast, and vision. This addresses @doudouOUC's earlier suggestion about history items not indicating scope.- Upstream merges —
matchesAnyServerPatternadoption across MCP management UI,collapsePreviewCountfeature. These are from merged upstream PRs, not PR-specific.
Findings
No new high-confidence issues. All previously reported issues have been addressed:
persistScopesilently dropped for subcommand dialog paths459730456. All 4 dialog return objects now spread...persistScopeSpread(scopeOverride).parseScopeFlagsposition-dependent459730456. Now uses(?:^|\s)--flag(?:\s|$)regex.Subcommand confirmation messages lack scope indication— Fixed. All paths includescopeSuffix.Dialog title doesn't combine scope with subcommand mode— Fixed. Title appends scope suffix after mode ternary.
Remaining open suggestions (not blocking)
- The
scopeSuffixternary is duplicated 5 times acrossModelDialog.tsxandmodelCommand.ts. A small shared helper (e.g.,formatScopeSuffix) would reduce repetition. - No unit tests for
parseScopeFlags(),resolveScope(), or scope-aware persist paths (previously noted by @doudouOUC).
Verdict: The implementation is correct and the previously identified issues have been properly resolved.
…bal with inline one-shot Merge our --project/--global scope flags with upstream's inline one-shot override feature (QwenLM#5967). Both features coexist in description, argumentHint, and i18n translation keys across en/zh/zh-TW locales. Signed-off-by: Alex <alex.tech.lab@outlook.com>
| // Parse --project / --global scope flags first, then process the rest | ||
| const rawArgs = context.invocation?.args?.trim() || actionArgs.trim(); | ||
| const { scopeOverride, remaining: args } = parseScopeFlags(rawArgs); | ||
| const scopeSuffix = |
There was a problem hiding this comment.
Bug (i18n + inconsistency): scopeSuffix uses hardcoded English strings ' (project)' and ' (global)' without t() wrapping. This is consumed by voice/fast/vision confirmation messages (lines ~422, ~506, ~609), so non-English users see English text for those confirmations.
Additionally, the wording is inconsistent with ModelDialog.tsx, which uses t(' (this project)') (note "this project" vs just "project"). The main model path here uses t('Model: {{model}} (project)') — a third variant.
Suggested fix: Wrap in t() and align the wording with ModelDialog.tsx:
const scopeSuffix =
scopeOverride === SettingScope.Workspace
? t(' (this project)')
: scopeOverride === SettingScope.User
? t(' (global)')
: '';This reuses the existing zh translation ' (this project)': '（当前项目）' and ' (global)': '（全局）'.
- scopeSuffix in modelCommand.ts now uses t(' (this project)') / t(' (global)')
instead of hardcoded English strings, matching ModelDialog.tsx wording
- Main model confirmation uses shared scopeSuffix instead of separate
i18n keys, eliminating 'Model: {{model}} (project)' duplication
- Remove unused i18n keys from en/zh/zh-TW locales
- Update tests to expect '(this project)' wording
Signed-off-by: Alex <alex.tech.lab@outlook.com>
|
@qwen-code /resolve |
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge Conflict Resolution SummaryConflicts FoundTwo files had merge conflicts when merging
Resolution DetailsConflict 1: modelCommand.ts — Function definitionsHEAD (PR branch) added:
origin/main added:
Resolution: Kept both sets of functions. They are independent and serve different purposes — scope management for the PR feature and display formatting from main. Conflict 2: modelCommand.ts — Vision model persistenceHEAD (PR branch): persistSetting(settings, 'visionModel', modelName, scopeOverride);Persisted the raw model name with scope override support. origin/main: const qualifiedModelName = `${selector.authType ?? matched.authType}:${selector.modelId}`;
const visionModel = matched.baseUrl
? `${qualifiedModelName}\0${matched.baseUrl}`
: qualifiedModelName;
persistSetting(settings, 'visionModel', visionModel);Constructed a qualified model name with auth type and base URL for disambiguation, but without scope override. Resolution: Combined both approaches — use main's qualified vision model construction (which properly disambiguates models across providers and encodes base URL) AND pass the const qualifiedModelName = `${selector.authType ?? matched.authType}:${selector.modelId}`;
const visionModel = matched.baseUrl
? `${qualifiedModelName}\0${matched.baseUrl}`
: qualifiedModelName;
persistSetting(settings, 'visionModel', visionModel, scopeOv |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Code Review — qwen3.7-max
Build: ✅ PASSED | TypeScript: ✅ PASSED | Tests: @xterm/headless CJS/ESM interop, unrelated to PR)
Critical
1. Inline prompt + scope flag silently discards scope
packages/cli/src/ui/commands/modelCommand.ts (action function, inline prompt return path)
scopeOverride is correctly extracted by parseScopeFlags, but the inline prompt path returns { type: 'submit_prompt', content: inlinePrompt, modelOverride: parsed.modelId } without referencing scopeOverride or scopeSuffix. For /model --project qwen-max explain code, the --project flag is silently consumed — the model is NOT persisted to project settings, and the user gets no warning.
Fix: Either persist the model before returning submit_prompt, or reject the combination:
if (inlinePrompt && scopeOverride) {
return {
type: 'message',
messageType: 'error',
content: t('Cannot combine --project/--global with an inline prompt. Run \'/model --project <id>\' first, then send your prompt.'),
};
}2. Test gap — settings.setValue scope argument never verified
packages/cli/src/ui/commands/modelCommand.test.ts (scope suffix tests)
Both suffix tests (should show scope suffix in fast/main model confirmation) assert on result.content containing "(this project)" but never verify setValue was called with SettingScope.Workspace. The core feature — writing to the correct settings file — is untested.
Fix: Add assertions:
expect(setValue).toHaveBeenCalledWith(
expect.stringContaining('workspace'),
'fastModel',
'qwen3-coder-flash',
);Also add a --global variant asserting 'user' scope.
Suggestions
3. scopeSuffix ternary duplicated 5+ times across modelCommand.ts and ModelDialog.tsx. Extract a shared formatScopeSuffix() helper.
4. Dead i18n keys — 'Select Model (this project)' and 'Select Model (global)' in en.js:1691-1692 (and zh/zh-TW) are never referenced in source code. The dialog title concatenates t('Select Model') + suffix separately. Remove the unused composite keys.
5. Conflicting --project --global produces confusing error — parseScopeFlags uses if/else if; when both flags present, one remains in remaining and gets parsed as a model name → "Model '--global' not found". Add mutual exclusivity check.
6. ModelDialogPersistScope exported but never imported (useModelCommand.ts:9). All consumers use the raw 'workspace' | 'user' literal. Either import it or remove export.
7. Non-interactive help text not t()-wrapped (modelCommand.ts, no-model fallback). Raw template literal, unlike all other messages. Also missing --project, --global, --vision.
8. Scope suffix placed on API key line — In ModelDialog.tsx handleModelSwitchSuccess, scopeSuffix is appended to API key: sk...abc (this project) instead of the model line. All other paths place it after the model name.
9. Untrusted workspace: --project writes into void — resolveScope honors --project unconditionally, but mergeSettings replaces workspace with {} when isTrusted is false. User sees "(this project)" but setting is dormant.
10. Missing fr.js/ja.js translations — 4 actively-used i18n keys ( (this project), (global), 2 completion descriptions) are translated in en/zh/zh-TW but absent from fr/ja locales.
11. argumentHint groups all flags as mutually exclusive — [--fast|--voice|--vision|--project|--global] should be [--fast|--voice|--vision] [--project|--global] since scope flags combine with mode flags.
12. Unrelated Prettier formatting in 6 files — fr.js, ja.js, background-agent-resume.test.ts, workflow-orchestrator.test.ts, agent.test.ts, agent.ts contain formatting-only changes. Revert to keep diff focused.
Needs Human Review
13. Cross-scope shadowing (low confidence) — When workspace settings already contain model.name, /model --global writes to user scope but workspace wins on merge. Confirmation says "(global)" but setting may not take effect.
— qwen3.7-max via Qwen Code /review
- Reject inline prompt + scope flag combination with clear error (#1) - Add mutual exclusivity check for --project and --global (QwenLM#5) - Verify setValue scope parameter in tests + add --global test (#2) - Extract scopeSuffix to shared variable, remove duplication (#3) - Remove dead i18n keys 'Select Model (this project)' / '(global)' (#4) - Fix scopeSuffix placement on model line not API key line (QwenLM#8) - Add fr.js / ja.js translations for scope keys (QwenLM#10) - Remove unused export ModelDialogPersistScope (QwenLM#6) - Wrap non-interactive help text in t() with new flags (QwenLM#7) - Fix argumentHint grouping to show mode vs scope flags (QwenLM#11) Signed-off-by: Alex <alex.tech.lab@outlook.com>
Alex-ai-future
force-pushed
the
dive/muti-model
branch
from
aa0ca27 to
8d1cd20
Compare
…delDialog.tsx - modelCommand.ts: keep our scope flag parsing + upstream's formatVisionModelSettingForDisplay - modelCommand.ts vision path: merge upstream's baseUrl/qualifiedModelName with our scopeOverride - ModelDialog.tsx: merge upstream's visionModelDisplay with our scopeSuffix Signed-off-by: Alex <alex.tech.lab@outlook.com>
| settings: LoadedSettings, | ||
| scopeOverride: SettingScope | undefined, | ||
| ): SettingScope { | ||
| return scopeOverride ?? getPersistScopeForModelSelection(settings); |
There was a problem hiding this comment.
[Critical] --project currently forces SettingScope.Workspace even when the workspace is untrusted. LoadedSettings ignores workspace settings in that state, and nearby workspace-scoped flows guard this before writing, so this can report a project save that will not actually take effect until the folder is trusted.
Reject --project when settings.isTrusted is false before direct persistence or opening ModelDialog, and mirror that guard in the dialog scope resolver. For example:
if (scopeOverride === SettingScope.Workspace && !settings.isTrusted) {
return {
type: 'message',
messageType: 'error',
content: t('Workspace is untrusted; run /trust first or use --global.'),
};
}— GPT-5 Codex via Qwen Code /review
Reject --project scope flag before direct persistence or opening ModelDialog when settings.isTrusted is false. Workspace settings are ignored on merge in that state, so the save would silently not take effect. Also mirrors the guard in ModelDialog.tsx resolvePersistScope() to fall back to user scope when the dialog is opened with --project on an untrusted folder. Default mock settings now includes isTrusted: true. Signed-off-by: Alex <alex.tech.lab@outlook.com>
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
Incremental Review — 2ccb615055
Scope reviewed: 15 source files (+627/-120 vs main). Full review of correctness, security, and code quality.
Summary
This PR adds --project and --global scope flags to /model for per-project model persistence. The implementation threads a persistScope value through the command → dialog pipeline, with proper i18n, untrusted-workspace guards, and inline-prompt rejection.
Verdict: No new high-confidence issues found
All significant issues raised by previous reviewers (@DragonnZhang, @doudouOUC, @wenshao) have been addressed in the latest commits:
| Issue | Reporter | Status |
|---|---|---|
parseScopeFlags position-dependent matching |
@DragonnZhang | Fixed — regex `(?:^ |
persistScope dropped for subcommand dialogs |
@doudouOUC | Fixed — persistScopeSpread on all dialog returns |
| Dialog title not combining mode + scope | @doudouOUC | Fixed — ternary + suffix |
| Confirmation messages missing scope for subcommands | @doudouOUC | Fixed — scopeSuffix in all paths |
Hardcoded English strings without t() |
@DragonnZhang | Fixed — all wrapped in t() |
--project when workspace is untrusted |
@wenshao | Fixed — command-level guard + dialog fallback |
| Inline prompt + scope flag silent discard | @wenshao | Fixed — explicit error message |
| Test coverage gaps | @doudouOUC | Open (suggestion) |
The scope propagation chain is correct end-to-end: modelCommand.action → OpenDialogActionReturn.persistScope → slashCommandProcessor → openModelDialog({persistScope}) → useModelCommand state → UIState.modelDialogPersistScope → DialogManager → ModelDialog.persistScope → resolvePersistScope() → settings.setValue(scope, ...).
The untrusted-workspace handling is properly layered: command-level rejection in modelCommand.ts prevents the dialog from opening, and resolvePersistScope in ModelDialog.tsx provides defense-in-depth by falling back to SettingScope.User.
Note: Test coverage for parseScopeFlags, resolveScope, scope-aware persistSetting, and scope in confirmation messages remains unaddressed (previously noted by @doudouOUC).
| type: 'message', | ||
| messageType: 'error', | ||
| content: t( | ||
| "Cannot combine {{flag}} with an inline prompt. Run '/model {{flag}} {{model}}' first, then send your prompt.", |
There was a problem hiding this comment.
[Critical] The t() call includes a {{model}} placeholder in the template string but the interpolation params only provide { flag: scopeFlag } — the model key is missing. The i18n interpolation renders unsubstituted placeholders as literal text, so users see:
Cannot combine --project with an inline prompt. Run '/model --project {{model}}' first, then send your prompt.
The {{model}} leaks raw into the UI. Note how the adjacent inline-prompt error at line ~771 correctly passes { model: modelName }.
| "Cannot combine {{flag}} with an inline prompt. Run '/model {{flag}} {{model}}' first, then send your prompt.", | |
| "Cannot combine {{flag}} with an inline prompt. Run '/model {{flag}} {{model}}' first, then send your prompt.", | |
| { flag: scopeFlag, model: modelName }, |
— qwen3.7-max via Qwen Code /review
| type: 'message', | ||
| messageType: 'error', | ||
| content: t( | ||
| 'Cannot use both --project and --global. Choose one scope flag.', |
There was a problem hiding this comment.
[Suggestion] Several new t()-wrapped strings introduced by this PR are not registered in any locale file (en, fr, ja, zh, zh-TW). The t() function falls back to the key string for English, so English works, but non-English locales see untranslated English text. Missing keys:
"Cannot use both --project and --global. Choose one scope flag."(line 414)"Workspace is untrusted; run /trust first or use --global."(line 428)"Cannot combine {{flag}} with an inline prompt. Run '/model {{flag}} {{model}}' first, then send your prompt."(line 789)"Current model: {{model}}\nUse \"/model <model-id>\" to switch models, \"/model --fast <model-id>\" to set the fast model, \"/model --project <model-id>\" to persist to project settings, or \"/model --global <model-id>\" to persist to user settings."(line 855)
The PR correctly added i18n entries for (this project), (global), the completion descriptions, and the updated command description — these four strings were missed. Add them to all five locale files for consistency.
— qwen3.7-max via Qwen Code /review
E2E Tmux Test Report -- PR #6060Test Environment
Unit Test Results
All 11 new scope-flag tests in the
Tmux Interactive E2E TestsTest 1: CLI Startup
Test 2:
|
| 测试套件 | 测试数 | 通过 | 失败 |
|---|---|---|---|
modelCommand.test.ts |
67 | 67 | 0 |
config.test.ts |
313 | 313 | 0 |
所有 11 个新增的作用域标志测试均通过。
Tmux 交互式 E2E 测试
| 测试 | 命令 | 结果 |
|---|---|---|
| CLI 启动 | npm run dev |
PASS |
/model --project 对话框 |
标题显示 "Select Model (this project)" | PASS |
/model --global 对话框 |
标题显示 "Select Model (global)" | PASS |
/model --project <model> |
确认信息含 "(this project)"，写入项目级设置 | PASS |
/model --global <model> |
确认信息含 "(global)"，写入全局设置 | PASS |
--project --global 互斥 |
报错 "Cannot use both" | PASS |
无效模型 + --project |
正确显示可用模型列表 | PASS |
无头模式测试
| 测试 | 结果 |
|---|---|
/model 非交互模式 |
PASS -- 正确显示当前模型和帮助文本 |
/model --project 无 model-id |
PASS -- 显示当前模型 |
/model --fast --project 无 model-id |
PASS -- 显示当前快速模型 |
结论
全部 10 个 E2E 测试通过，全部 380 个单元测试通过 (67 modelCommand + 313 config)。
实现质量良好：作用域标志解析正确、互斥检查到位、持久化到正确的设置文件、UI 正确反映作用域信息。不受信任工作空间的保护和向后兼容性均处理妥当。
|
|
||
| persistSetting(settings, 'voiceModel', modelName); | ||
| persistSetting(settings, 'voiceModel', modelName, scopeOverride); | ||
| return { |
There was a problem hiding this comment.
[Suggestion] The scopeSuffix ternary (persistScope === 'workspace' ? t(' (this project)') : persistScope === 'user' ? t(' (global)') : '') appears 6 times across modelCommand.ts and ModelDialog.tsx. A future scope addition (e.g., "team") would require updating all 6 sites — easy to miss one.
| return { | |
| const scopeSuffix = | |
| scopeOverride === SettingScope.Workspace | |
| ? t(' (this project)') | |
| : scopeOverride === SettingScope.User | |
| ? t(' (global)') | |
| : ''; |
Consider extracting a shared helper like formatScopeSuffix(scope: 'workspace' | 'user' | undefined): string used by both files.
— qwen3.7-max via Qwen Code /review
| }); | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
[Suggestion] The new "scope flags" describe block covers dialog returns, fast-model direct-set, main-model direct-set, untrusted workspace, and no-flag default — good coverage of the happy path. However, several specific code paths in the diff remain untested:
- Voice model direct-set with scope (
/model --project --voice <model-id>) — thepersistSetting(settings, 'voiceModel', modelName, scopeOverride)call atmodelCommand.ts:~503is never exercised withSettingScope.Workspace. - Vision model direct-set with scope (
/model --project --vision <model-id>) — same gap atmodelCommand.ts:~700. - Inline prompt + scope flag rejection (
/model --project qwen-max explain this) — the error path atmodelCommand.ts:~780is untested. - Mutual exclusivity (
/model --project --global) — the rejection atmodelCommand.ts:~410is untested. - The
switchMainModelscope test assertssetValue(scope, 'model.name', ...)but not the companionmodel.baseUrltombstone orsecurity.auth.selectedTypewrites at the same scope.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
added
category/cli
|
Thanks for the PR, @Alex-ai-future! Template looks good ✓ Direction: Clearly aligned. Per-project model persistence is a real user need — different projects having different cost/complexity requirements is a common scenario. Claude Code's CHANGELOG has direct precedent: "selections now persist across restarts even when the project pins a different model, and the startup header shows when the active model comes from a project or managed-settings pin." This is exactly the kind of quality-of-life improvement users notice and appreciate. Approach: The core feature is well-structured — threading One concern before diving into code review: the diff carries significant formatting-only changes in unrelated files (~8 files including Moving on to code review. 🔍 中文说明感谢 PR！ 模板完整 ✓ 方向：明确对齐。按项目持久化模型选择是真实的用户需求——不同项目有不同的成本/复杂度需求。Claude Code 的 CHANGELOG 有直接先例："selections now persist across restarts even when the project pins a different model"。这类体验改善用户很在意。 方案：核心功能结构良好——将 一个顾虑：diff 中包含大量无关文件的纯格式化改动（约 8 个文件，包括 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal for per-project model persistence: I would have added The PR's approach matches this almost exactly. No critical blockers found. The untrusted-workspace guard, mutual exclusion check, and backward compatibility are all properly handled. One minor note: the Formatting churn (flagged in Stage 1): ~8 files with unrelated prettier reformatting still present in the diff. Not a blocker but adds noise to review. Tmux Test ResultsUnit TestsAll 67 tests in Headless E2E TestsTest 1: ✅ PASS — mutual exclusion enforced correctly. Test 2: ✅ PASS — default behavior unchanged, help text surfaces new flags. Test 3: ✅ PASS — shows current model, doesn't try to open dialog in headless mode. Test 4: Summary
中文说明代码审查独立方案与 PR 实现基本一致：添加 未发现关键阻塞问题。 不受信任工作空间保护、互斥检查、向后兼容均处理正确。 小建议： 格式化改动（Stage 1 已提出）：约 8 个无关文件的 prettier 重排仍在 diff 中，增加审查噪音。 Tmux 测试结果
— Qwen Code · qwen3.7-max |
ReflectionGoing back to my independent proposal: I would have done essentially the same thing — add two flags, parse them early, map to This solves a problem users actually hit: switching models in one project affecting all others. The fix is minimal and non-breaking — no flag means current behavior, which is exactly right for an additive feature. The code is straightforward. The tmux results confirm what the unit tests already showed: flag parsing works, mutual exclusion fires, backward compatibility is preserved. The collaborator's interactive E2E report confirms the full dialog-to-persistence flow works end-to-end with correct scope suffixes and file writes. The formatting churn in ~8 unrelated files is the one thing I'd ask to clean up — it makes future Ships the feature cleanly. Approving. ✅ 中文说明总结回到独立方案：我基本会做同样的事——加两个标志、解析、映射到 解决了用户实际遇到的问题：一个项目切换模型影响所有项目。改动最小且非破坏性——不带标志则行为不变。 代码直白： Tmux 测试确认标志解析正确、互斥生效、向后兼容。协作者的交互式 E2E 报告确认完整流程正确。 唯一建议：清理约 8 个无关文件的格式化改动——拆成单独 PR 或丢弃（prettier 会在下次 format 时处理）。 功能实现干净，批准合并 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅ The formatting churn in unrelated files is minor — consider splitting or dropping those on a future format pass.
✅ Local build & real-binary verification — PR #6060 (
|
| Check | Command | Result |
|---|---|---|
| Unit (command) | vitest run modelCommand.test.ts |
67/67 pass |
| Unit (config) | vitest run config.test.ts |
313/313 pass |
| i18n | npm run check-i18n |
✅ All checks passed |
| Typecheck | npm run typecheck (all workspaces) |
0 errors |
| Lint | eslint on all changed files |
clean |
| Format | node scripts/lint.js --prettier (exact CI cmd) |
pass |
| Build | npm run build |
exit 0, binary runs (--version → 0.19.3) |
3. Real-binary TUI verification (isolated HOME, folder-trust configured, tmux)
| # | Scenario | Observed | ✓ |
|---|---|---|---|
| A | untrusted ws + /model --project qwen-max |
✕ Workspace is untrusted; run /trust first or use --global. — and no .qwen/ dir or settings file was created in the workspace |
✅ |
| B | /model --project --global |
✕ Cannot use both --project and --global. Choose one scope flag. |
✅ |
| C | untrusted ws + /model --global |
Dialog opens titled “Select Model (global)”; selecting → Using runtime model: qwen3.5-plus (global) and model.name written to user settings — proving the block is scoped to --project only |
✅ |
| D | trusted ws + /model --project |
Dialog opens titled “Select Model (this project)”; selecting → (this project) confirmation and model.name written to the workspace .qwen/settings.json (feature’s core purpose) |
✅ |
Scenario A is the exact behavior in the PR title, confirmed on the real binary; the negative-control (no file written on rejection) also holds.
4. Non-blocking nits (for your judgment, none block merge)
- Unrelated formatting churn.
packages/core/src/config/config.ts,config.test.ts, andtools/mcp-transport-pool.test.tsare prettier-3.6.1 reformatting only (union-type wrapping), no semantic change, swept in by a local pre-commit run. Zero CI risk (the CI “prettier” step isprettier --write ., which never fails on drift), but it adds diff noise + merge-conflict surface. Consider dropping these files from the PR. - Two validation branches lack unit tests: the
--project --globalmutual-exclusion and the “scope flag + inline prompt” rejection. I confirmed the former via E2E (Scenario B) and the latter by code inspection (correctly placed after model-availability validation), but adding two small unit tests would lock them in. - Latent suffix mismatch in
ModelDialog.resolvePersistScope. WhenpersistScope==='workspace'but untrusted, the dialog silently falls back to User scope yet still shows “(this project)”. This path is currently unreachable (the command hard-rejects that combo before opening any dialog), so it’s harmless defensive code — but if a future caller ever opens the dialog withpersistScope:'workspace'while untrusted, the suffix would mislead. Either drop the fallback (command already guards) or make the suffix reflect the resolved scope. - Branch is 5 commits behind
origin/main. Worth updating before merge so CI runs against current trunk. fr.js/ja.jsdon’t translate the updated/modeldescription string (they add the new suffix/persist keys but not the reworded description). Runtime falls back to English for those two locales;check-i18npasses. Cosmetic.
Verdict
Correct, well-tested, and the security fix works on the real binary. Safe to merge once you decide on nit #1 (formatting churn) and, ideally, a branch update (nit #4).
🇨🇳 中文版（点击展开）
✅ 本地构建 + 真实二进制验证 —— PR #6060（/model --project / --global 作用域开关）
我从源码构建了本 PR，并用真实编译出的 qwen 二进制（不只是单测）做了端到端验证。结论：可以合并。 所有 CI 门禁检查通过，标题所述的安全行为（工作区不受信任时拒绝 --project）端到端可用。下方列了几条不阻塞合并的小建议。
验证基于 PR head
2ccb615（该分支落后origin/main8d5e47a共 5 个提交）。真实功能改动 = 21 个文件；two-dot diff 看起来很大只是因为分支早于 main 最近的重构，与本 PR 无关。
1. 功能
给 /model（及其 --fast/--voice/--vision 子模式和模型对话框）新增 --project（→ 工作区/项目设置）和 --global（→ 用户设置）作用域开关，从而把模型选择固定到「按项目」或「全局」。核心修复：工作区不受信任时拒绝 /model --project——因为不受信任的工作区设置在合并时会被丢弃，写入虽落盘却永远不生效，还会给出误导性的「（当前项目）」确认。（无开关的默认行为不变；getModelProvidersOwnerScope 在不受信任时本就不会返回 Workspace，所以只有显式 --project 才会踩这个坑。）
2. CI 门禁检查 —— 全绿
| 检查 | 命令 | 结果 |
|---|---|---|
| 单测（命令） | vitest run modelCommand.test.ts |
67/67 通过 |
| 单测（config） | vitest run config.test.ts |
313/313 通过 |
| i18n | npm run check-i18n |
✅ 全部通过 |
| 类型检查 | npm run typecheck（全 workspace） |
0 错误 |
| Lint | 对所有改动文件跑 eslint |
干净 |
| 格式 | node scripts/lint.js --prettier（CI 原命令） |
通过 |
| 构建 | npm run build |
exit 0，二进制可运行（--version → 0.19.3） |
3. 真实二进制 TUI 验证（隔离 HOME、配置文件夹信任、tmux 驱动）
| # | 场景 | 观察结果 | ✓ |
|---|---|---|---|
| A | 不受信任工作区 + /model --project qwen-max |
✕ Workspace is untrusted; run /trust first or use --global.——且工作区内未创建 .qwen/ 目录或设置文件 |
✅ |
| B | /model --project --global |
✕ Cannot use both --project and --global. Choose one scope flag. |
✅ |
| C | 不受信任工作区 + /model --global |
对话框标题为 “Select Model (global)”；选中后 → Using runtime model: qwen3.5-plus (global)，model.name 写入用户设置——证明拦截只针对 --project |
✅ |
| D | 受信任工作区 + /model --project |
对话框标题为 “Select Model (this project)”；选中后 → (this project) 确认，model.name 写入工作区 .qwen/settings.json（功能核心目的） |
✅ |
场景 A 就是 PR 标题所述行为，在真实二进制上确认；拒绝时不落盘的反向对照也成立。
4. 不阻塞合并的小建议（供你判断）
- 无关的格式化改动。
packages/core/src/config/config.ts、config.test.ts、tools/mcp-transport-pool.test.ts只是 prettier 3.6.1 的重新排版（联合类型换行），无语义变化，是本地 pre-commit 顺带带进来的。对 CI 无风险（CI 的 prettier 步骤是prettier --write .，永不因格式漂移失败），但增加了 diff 噪声和合并冲突面。建议把这几个文件从 PR 中移除。 - 两个校验分支缺单测：
--project --global互斥、以及「作用域开关 + 内联 prompt」的拒绝。前者我用 E2E 确认（场景 B），后者用代码审查确认（正确地放在模型可用性校验之后），但补两个小单测能把它们固化下来。 ModelDialog.resolvePersistScope存在潜在的后缀不一致。 当persistScope==='workspace'但不受信任时，对话框会静默回退到 User 作用域，却仍显示「（当前项目）」。该路径当前不可达（命令在打开任何对话框前就硬拒绝了该组合），属无害的防御性代码——但若将来有调用方在不受信任时用persistScope:'workspace'打开对话框，后缀就会误导。建议要么去掉这个回退（命令已把关），要么让后缀反映真正解析出的作用域。- 分支落后
origin/main5 个提交。 合并前最好更新分支，让 CI 跑在最新 trunk 上。 fr.js/ja.js没有翻译更新后的/model描述串（只加了新的后缀/persist 键，没改重写后的描述）。这两种语言运行时回退到英文；check-i18n通过。仅外观问题。
结论
正确、测试充分，安全修复在真实二进制上生效。可以安全合并，只需你就第 1 条（格式化噪声）拿主意，并最好更新一下分支（第 4 条）。
🤖 Verified locally with a source build + real-binary tmux E2E. Full CI-gate suite + 4 end-to-end scenarios above.
6 participants
What this PR does
Adds
--projectand--globalscope flags to the/modelcommand, allowing users to choose which settings file the model selection is persisted to./model --projectopens the model picker and writes the selection to the project-level.qwen/settings.json(workspace scope)/model --globalopens the model picker and writes to the user-level~/.qwen/settings.json(global scope)/model --project <model-id>sets the model directly and persists to project scope, showingModel: qwen-max (project)as confirmation/model --global <model-id>sets the model directly and persists to global scope, showingModel: qwen-max (global)/modelwithout flags retains the current behavior (backward compatible)--fast,--voice, and--visionsubcommandsThe implementation threads a
persistScopevalue through the existing dialog pipeline:modelCommand->OpenDialogActionReturn->slashCommandProcessor->useModelCommand->UIState->DialogManager->ModelDialog. TheModelDialogusesresolvePersistScope()to map the string scope to aSettingScopeenum, overriding the defaultgetPersistScopeForModelSelectionbehavior when a scope is explicitly provided.Why it's needed
Different projects have different complexity and cost requirements. A simple scripting project may only need a lightweight model, while a complex architecture project needs a stronger one. Today,
/modelalways writes to the scope that ownsmodelProviders(usually~/.qwen/settings.json), so switching models in one project affects all projects globally. The only workaround is to duplicate the entiremodelProvidersconfig at project level, which is impractical.This PR implements the "explicit flags" approach from the #6052 triage discussion — keeping the current default (global write) while adding opt-in
--project/--globalflags. This is non-breaking and gives users precise control without surprising behavior changes.Reviewer Test Plan
How to verify
/model --project— the dialog title should show "Select Model (this project)". Select a model. Verify it is written to<project>/.qwen/settings.json(not~/.qwen/settings.json)./model --global— the dialog title should show "Select Model (global)". Select a model. Verify it is written to~/.qwen/settings.json./model --project qwen-max— should show "Model: qwen-max (project)" and write to project settings./model(no flag) — behavior unchanged, writes to the scope that ownsmodelProviders(current behavior)./modelin the input — autocomplete should show--projectand--globalalongside existing flags./model --project --fast qwen3-coder-flash— should write fast model to project scope.Evidence (Before & After)
Before:
/modelalways writes to global settings. No way to set per-project model without duplicatingmodelProviders.After:
/model --project qwen-maxwrites to project settings./model --global qwen-maxwrites to global settings. Dialog title shows scope.Tested on
Environment (optional)
npm run devRisk & Scope
/modelbehavior is unchanged when no flag is provided.--projectflag (the model must be available under the current auth type). The--projectflag for daemon/ACP sessions is not explicitly handled but should work since the settings merge logic is the same.Linked Issues
Closes #6052
中文说明
这个 PR 做了什么
为
/model命令添加--project和--globalscope 标志，让用户可以选择模型选择持久化到哪个设置文件。/model --project打开模型选择器，写入项目级.qwen/settings.json/model --global打开模型选择器，写入用户级~/.qwen/settings.json/model --project <model-id>直接设置模型并持久化到项目级，显示Model: qwen-max (project)/model不带标志保持当前行为（向后兼容）--fast、--voice、--vision为什么需要
不同项目有不同的复杂度和成本需求。今天
/model总是写入modelProviders所在的 scope（通常是全局），导致在一个项目里切换模型会影响所有项目。唯一的变通方法是在项目级复制整个modelProviders配置，这不现实。本 PR 实现了 #6052 triage 讨论中的"显式标志"方案——保持当前默认行为（全局写入），同时添加 opt-in 的
--project/--global标志。非破坏性，给用户精确控制。审查者测试计划
如何验证
/model --project— 标题应显示"选择模型（当前项目）"，选择后验证写入项目级设置/model --global— 标题应显示"选择模型（全局）"，选择后验证写入全局设置/model --project qwen-max— 应显示"模型：qwen-max（项目）"/model（无标志）— 行为不变/model— 自动补全应显示--project和--global证据（前后对比）
之前：
/model总是写入全局设置，无法设置项目级模型。之后：
/model --project qwen-max写入项目设置，/model --global qwen-max写入全局设置。测试环境
风险与范围
--project标志关联 Issue
Closes #6052