feat(cli): Add ability to switch models non-interactively from the cli

[alex-musick]

Summary

• What changed: Added new syntax to the /model command. /model, /model --fast, and /model --fast [model-name] still function as previously. This PR adds a new syntax /model [model-name] which allows for immediately switching the active model without using the interactive model selector.
• Why it changed: The primary use case for this is to allow switching between upstream models when those models are available at the configured base url, but not explicitly configured in the config. This behavior is already present when launching with qwen --model [model-name]; the intent of this feature is to replicate this functionality at runtime. This functionality also brings greater parity with the existing functionality of other coding agents.

This is a small PR with no platform-specific or core changes.
This PR contains no AI-generated code.

Validation

• Commands run:

- npm run build
- npm run test:scripts
- npm run start

All test scripts passed.

Functionality before changes:

Functionality after changes:

Scope / Risk

• Main risk or tradeoff: This feature allows the user to try switching to an unavailable model at runtime. This replicates the functionality of the launch argument --model and is intended.
• Breaking changes / migration notes: This PR changes the behavior of invoking /model with invalid syntax. Extraneous arguments were previously ignored. Now, the first extraneous argument is interpreted as a model name. Existing correct syntax is unchanged, and the changes explicitly do not take effect in non-interactive mode. This should not break any workflows or orchestration flows, but the change should be noted.

Linked Issues / Bugs

• Closes Allow for specifying upstream model name after /model #3410

[wenshao]

⚠️ Downgraded from Approve to Comment: CI failing: Test (windows-latest, 20.x).

Second-opinion review with pai/glm-5 (previous review used glm-5.1). The 4 findings from the prior review (missing await, missing persistence, dead code, typo) are all confirmed and still apply. Here are additional findings not previously reported:

[Critical] config.setModel() rollback leaves misleading state — When switchModel throws (model exists in global registry but not for current auth type), it calls rollbackToStateSnapshot internally. Without await, the user sees "Model: X" while the runtime state has actually rolled back. This is a false-positive confirmation — strictly worse than a silent no-op.

[Suggestion] No model name validation — args.trim().split(' ')[0] can produce an empty string (e.g., /model passes args !== '' but trim+split yields ""). config.setModel() accepts any string without validation (raw model override path). Consider validating non-empty after extracting first token, and optionally warning if the model is not in the registry.

[Suggestion] Inconsistent model name parsing — Interactive path uses args.trim().split(' ')[0] (first token) while non-interactive path uses args.trim() (full string). /model some name with spaces produces "some" in interactive vs "some name with spaces" in non-interactive. Consider aligning the parsing strategy.

[Suggestion] Completion handler doesn't suggest model names — The completion handler only suggests --fast. With the new feature, /model qwen3 should tab-complete matching model IDs. config.getAvailableModels() is available but unused.

[Suggestion] Command description doesn't document new feature — The description only mentions --fast. Users have no indication that /model <model-name> is valid. Update the i18n string to mention both usage patterns.

[Nice to have] Missing space after // — //Handle and //Use should be // Handle and // Use per codebase convention.

[alex-musick]

I have merged and implemented the proposed changes. A few notes:

1. I believe the CI failure is inherited from the base repo. Relevant CI passes on my mac, and I haven't changed anything that would cause platform-specific failures.

2. The existing structure does not provide an efficiently filterable array of model IDs. To overcome this, I generate one on the spot with availableModels.map((model) => model.id) (line 26). This is a potentially O(n) operation on every keystroke after /model has been typed. This could be made more time efficient by refactoring modelsConfig to maintain this array internally, but it would also require duplicating the data in memory. Probably not necessary but worth considering.

3. To address inconsistent model name parsing, I have reworked the non-interactive path to also use the args.trim().split(' ')[0] approach. Model IDs can not contain spaces anyway, so this should not be a breaking change. However, if preferred, I can revert this change and instead use the old logic in the new feature.

[alex-musick]

Updated with newest requested changes.

[wenshao]

Test gap: The new /model <name> feature has zero test coverage. No tests exist for: interactive mode model switching with valid/invalid model names, non-interactive mode model switching, the getAvailableModelIds helper, or the updated completion handler. The only test change is the description string update.

[Suggestion] _context misleading prefix: The completion handler parameter is named _context (underscore convention for unused), but it's passed to getAvailableModelIds(_context). Rename to context.

[wenshao]

[Critical] Two issues at this setModel call:

1. Validation too late: hasModel() check runs AFTER config.setModel() + settings.setValue(). An invalid model name (e.g., /model gpt4 instead of gpt-4) gets silently set and persisted; the (not in model registry) suffix is only info level, not a guard. Move hasModel() before setModel and return an error for unknown models.

2. No error handling: No try/catch around setModel + setValue. If setModel throws, the error propagates unhandled and may crash the session. If setModel succeeds but setValue fails, the model is switched in memory but not persisted — user sees success but the change is lost on restart with no warning.

— deepseek-v4-pro via Qwen Code /review

[alex-musick]

Returning an error for unknown models is explicitly not the intended behavior. Per the feature request, the new syntax should allow the user to use model IDs that are not already in the local registry.

This should only be relevant in the raw path, which does not have any internal validation. I can add try/catch blocks to address the other cases, but explicitly guarding against unknown model IDs would defeat the purpose of the feature. A note is already provided when the model ID is unknown, and clear 404 errors are already safely passed to the user if they switch to a model that's not available upstream and then attempt to use it.

[alex-musick]

I also question if this needed since the other cases already fail gracefully on their own.

[wenshao]

[Critical] The non-interactive path has no hasModel() check at all (unlike the interactive path which at least appends (not in model registry) after the fact). Users switching models in non-interactive/ACP/CI mode get zero indication whether the model name is valid — it's silently persisted regardless.

Extract the shared model-switching logic (both paths do setModel + setValue + return info) into a single helper that includes the registry check, and use it in both paths. This eliminates both the code duplication and the behavioral divergence.

— deepseek-v4-pro via Qwen Code /review

[alex-musick]

A lot of the proposed changes for this PR have focused on differences between the interactive and non-interactive paths. At this point, I would propose that the non-interactive path be removed entirely, since its syntax is almost identical to the new feature.

To ensure the previous behavior is fully maintained, the check for non-interactive calls would be moved to the case with no arguments, where it would decide whether to open the interactive model switcher or simply print the model name.

Since this marks a sizeable refactor of modelCommand, should I go ahead with this change here or save it for a separate PR for organization?

[wenshao]

[Suggestion] Interactive and non-interactive model-switch blocks are nearly identical — consider extracting a shared helper

Both blocks extract modelName the same way (args.trim().split(' ')[0]), guard with modelName.trim(), check settings availability, call config.setModel() then settings.setValue(). The only difference is the hasModel() check in the interactive path. This duplication caused the setModel/setValue ordering inconsistency in the previous version and makes future maintenance error-prone.

Suggested change

	};
	async function switchModel(
	config: Config,
	settings: LoadedSettings,
	authType: AuthType,
	modelName: string,
	): Promise<MessageActionReturn> {
	if (!settings) {
	return { type: 'message', messageType: 'error', content: t('Settings service not available.') };
	}
	await config.setModel(modelName);
	settings.setValue(
	getPersistScopeForModelSelection(settings),
	'model.name',
	modelName,
	);
	const suffix = config.getModelsConfig().hasModel(authType, modelName)
	? ''
	: ' (not in model registry)';
	return { type: 'message', messageType: 'info', content: t('Model') + ': ' + modelName + suffix };
	}

— pai/glm-5.1 via Qwen Code /review

[alex-musick]

Addressed in a different comment where I propose merging the two paths.

[wenshao]

[Suggestion] Non-interactive path silently persists unregistered models with no warning

The interactive path appends " (not in model registry)" when hasModel() returns false, but the non-interactive/ACP path returns a plain "Model: X" with no indication the model is unrecognized. Automation pipelines and ACP integrations get zero feedback when switching to an invalid model.

Suggested change

	content: t('Settings service not available.'),
	await config.setModel(modelName);
	settings.setValue(
	getPersistScopeForModelSelection(settings),
	'model.name',
	modelName,
	);
	if (config.getModelsConfig().hasModel(authType, modelName)) {
	return {
	type: 'message',
	messageType: 'info',
	content: t('Model') + ': ' + modelName,
	};
	} else {
	return {
	type: 'message',
	messageType: 'info',
	content: t('Model') + ': ' + modelName + ' (not in model registry)',
	};
	}

— pai/glm-5.1 via Qwen Code /review

[alex-musick]

This is intended behavior, as mentioned in a separate comment.

[wenshao]

[Suggestion] No test coverage for the new interactive immediate-switch feature

The core of this PR — /model <model-id> in interactive mode — has zero test cases. All existing interactive tests pass args = '' (dialog path). Consider adding tests for: (1) valid model switch with setModel/setValue called, (2) unregistered model showing "(not in model registry)", (3) multi-argument input using only first token, (4) settings unavailable guard.

— pai/glm-5.1 via Qwen Code /review

[alex-musick]

--fast is already not tested with model IDs, and most of these changes are beyond the scope of this PR. Testing with any specific valid or invalid model ID is difficult since the registry is user-configurable and therefore not stable. If there is a specific "default" registry to test against, this can be implemented; but again, this is asking for a sizeable update to the unit tests which is probably best handled separately.

[alex-musick]

I've reviewed the new proposed changes and made comments. Here is a summary of my input.

• Allowing unknown models to be used is explicitly an intended feature of the PR. The user should be free to try calling upstream models without adding them to the local registry. The raw path for switching models already contains no validation. Explicitly guarding against unknown model IDs would remove intended functionality, and would not enhance stability since upstream 404 errors are already handled gracefully. I could add try/catch blocks to handle the other cases, but those already fail gracefully on their own with instructive errors, so all this would do is obscure the underlying issues.

• To completely fix misalignment between the interactive and non-interactive paths, I am proposing the non-interactive path be removed, and the new code serve both functions. A check for non-interactive mode would be added to the case with no arguments to preserve the behavior of printing the active model ID when /model is called non-interactively without arguments. This becomes a sizeable refactor and I don't know whether it would be out of scope for this PR.

• Test cases need to be added. However, available arguments are based on registered model names, which are user-configurable and therefore shouldn't be considered stable unless we already have a default registry to test against (do we?). Additionally, the --fast path is already not checked with a specified model ID. I believe it would be more appropriate to make refactoring the modelCommand tests a seperate issue.

I have fixed the _context -> context convention requirement and updated the autocomplete suggestions to only trigger when an argument is actually being typed.

Awaiting comment on the other requests.

[alex-musick]

Still awaiting input on my proposal to merge the interactive and non-interactive paths.
仍在等待关于我合并交互式与非交互式路径提案的反馈。

[wenshao]

Thanks for the iteration — CI is now fully green and the previous critical findings are resolved. Two remaining items before approval:

[Blocker] i18n regression introduced by this PR

1. The new description key at modelCommand.ts:33-35 ('Switch the model for this session (--fast for suggestion model, [model-id] to switch immediately).') does not exist in any of the 9 locale files (en.js, zh.js, zh-TW.js, ja.js, ca.js, de.js, fr.js, pt.js, ru.js). Per packages/cli/src/i18n/index.ts:236, t() falls back to the raw key when the translation is missing — so all non-English users will see the English description in place of their previously translated string, and the old key becomes dead weight in every locale. Please replace the old key with the new key in each locale file (English fallback is fine for non-English ones; native translators can backfill later).

2. ' (not in model registry)' at line 163 is hardcoded English (string concatenation, not wrapped in t()). Same impact: non-English users always see this in English. Either wrap it via t() and add the key to the locales, or extract a translatable suffix.

On the proposal to merge interactive and non-interactive paths

Let's keep that as a follow-up PR — the divergence (hasModel check is intentionally interactive-only) is by design and already explained in your replies; widening the diff now would re-open review surface that has converged. Happy to land the refactor separately once this is in.

[alex-musick]

Thank you for the prompt and clear feedback. i18n has been updated.

[wenshao]

Second-opinion review with deepseek-v4-pro (previous review used deepseek-v4-pro). This PR has been through extensive review already; most findings from this pass overlap with previously discussed issues.

One new finding that hasn't been discussed:

[Suggestion] modelName.trim() is redundant — At lines 135 and 173, args.trim().split(' ')[0] already yields a trimmed string (no leading/trailing whitespace after split), so the if (modelName.trim()) check's .trim() is a no-op. Change to if (modelName).

— deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] The updated translation key now documents /model [model-id], but the localized value still translates only the old --fast behavior. This fixes the missing-key fallback, but command help in this locale still omits the feature added by this PR.

Please update the localized text to mention direct model-id switching as well. If native translations are unavailable, temporarily using the English value would preserve feature parity across locales. The same stale value pattern appears in the other non-English locale files changed by this PR.

— gpt-5.5 via Qwen Code /review

[wenshao]

All previously raised blockers resolved (i18n keys added across all 9 locales; (not in model registry) wrapped in t(); CI fully green). Verified the new /model <model-id> flow locally — main switch, --fast, no-args display, multi-token handling, whitespace-only fallthrough, and settings persistence all behave as advertised. LGTM.