Repository navigation
Fix ACP model selection to handle all configured authentication types - #1555
Merged
Merged
Conversation
Contributor
📋 Review SummaryThis PR addresses model selection to properly handle authentication types when configuring and displaying available models. The changes introduce new utility functions to format and parse model IDs with their associated authentication types, and modify the model selection process to work with these combined identifiers. Overall, the changes improve the consistency of how models are handled across different authentication types. 🔍 General Feedback
🎯 Specific Feedback🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Mingholy
marked this pull request as ready for review
January 21, 2026 03:02
Mingholy
requested review from
DennisYu07,
LaZzyMan,
gwinthis,
pomelo-nwu and
tanzhenxin
as code owners
January 21, 2026 03:02
Mingholy
force-pushed
the
mingholy/fix/acp-model-list
branch
from
February 2, 2026 10:47
14826dd to
0137b31
Compare
…istry models Co-authored-by: Qwen-Coder <[email protected]>
xaelistic
pushed a commit
to xaelistic/qwen-code
that referenced
this pull request
Jun 7, 2026
Co-authored-by: Scott Densmore <[email protected]>
xaelistic
pushed a commit
to xaelistic/qwen-code
that referenced
this pull request
Jun 7, 2026
Fix ACP model selection to handle all configured authentication types
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
This pull request updates the model selection system to properly handle authentication types (authType) when configuring and displaying available models. It introduces new utility functions to format and parse model IDs with their associated authentication types, and modifies the model selection process to work with these combined identifiers.
Dive Deeper
The changes address how models are stored, retrieved, and displayed in the application, particularly in relation to their authentication types. Key modifications include:
Added new utility functions (
formatAcpModelId,parseAcpBaseModelId,parseAcpModelOption) inpackages/cli/src/utils/acpModelUtils.tsto properly format model IDs with their authentication types in the format${modelId}(${authType}).Updated the
acpAgent.tsto use these new utilities when building available models, ensuring that model IDs include their authentication type information.Modified the
Sessionclass to use the newswitchModelmethod instead ofsetModel, properly parsing the model ID and authentication type from the request.Updated the
ModelDialogcomponent to usegetAllConfiguredModels()instead of the previous approach, which groups models by authentication type and orders them consistently.Enhanced the core
ModelsConfigclass with a newgetAllConfiguredModels()method that retrieves all configured models across authentication types, with qwen-oauth models prioritized first.Updated tests across multiple files to reflect the new model identification and selection approach, ensuring proper handling of authentication types.
Reviewer Test Plan
npm installto ensure all dependencies are up to date.npm testto verify all tests pass:vitest --runvitest --run --src cli/src/acp-integration/session/Session.test.tsvitest --run --src cli/src/ui/components/ModelDialog.test.tsxvitest --run --src cli/src/utils/acpModelUtils.test.tsvitest --run --src core/src/models/modelsConfig.test.tsTesting Matrix
Tested with Zed.
Linked issues / bugs