fix(acp): hide discontinued OAuth model for other auth types - #7522
Conversation
|
E2E verification report (Linux) I reproduced the issue against the built CLI with the reporter's provider shape and a deterministic local API key. On current main, raw With this change, the same JSON-RPC ACP handshake reports:
An independent Qwen OAuth scenario retains Checks completed:
The external SDK's separate incorrect-current-model label was not reproducible on current main, so this PR intentionally claims only the model-list portion that was reproduced end to end. |
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with a clear reproduction in #7433 — a user running local OpenAI-compatible models via ACP sees Direction: aligned. The interactive model picker already filters the discontinued OAuth entry; this brings the ACP surface in line with that behavior. CHANGELOG has several analogous fixes for model picker filtering on other surfaces (Bedrock, Size: not applicable — no core paths touched. 17 production lines, 127 test lines. Approach: the scope feels right. A single Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测到的 bug,#7433 中有清晰复现——用户通过 ACP 使用本地 OpenAI 兼容模型时, 方向:对齐。交互式模型选择器已经过滤了已停用的 OAuth 条目;本改动让 ACP 接口与该行为保持一致。CHANGELOG 中有多个类似的模型选择器过滤修复(Bedrock、 规模:不适用——未触及核心路径。17 行生产代码,127 行测试代码。 方案:范围合理。一个 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewThe implementation is clean and minimal. A single The filter logic is correct: The integration test is updated to directly request No critical blockers. No AGENTS.md violations. 301/301 unit tests pass, build and typecheck clean, ESLint clean on all changed files. Real-Scenario TestingSpawned the ACP server with a deterministic OpenAI-compatible test configuration (two models, Before (main branch)After (this PR)The discontinued 中文说明代码审查实现简洁且最小化。一个 过滤逻辑正确: 集成测试更新为直接通过 无关键阻塞项。无 AGENTS.md 违规。301/301 单元测试通过,构建和类型检查通过,ESLint 对所有改动文件通过。 真实场景测试使用确定性 OpenAI 兼容测试配置(两个模型, 修改前: 修改后: — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean fix for a real, reproduced bug; minimal diff, thorough tests, before/after confirms the fix. This is exactly the kind of PR you want to see: a real user hit a real bug (#7433), the fix is 15 lines of production code that mirrors what the interactive model picker already does, and the tests cover both the happy path and the edge case (OAuth sessions keep their model, direct requests for the hidden route still get a structured error). The before/after reproduction confirms the fix works end-to-end. Nothing to cut, nothing missing, nothing unrelated in the diff. 中文说明置信度:5/5 — 对已复现的真实 bug 的干净修复;最小化 diff,充分的测试,before/after 确认修复有效。 这正是理想的 PR:真实用户遇到了真实 bug(#7433),修复仅 15 行生产代码,与交互式模型选择器已有的行为保持一致,测试覆盖了正常路径和边界情况(OAuth 会话保留其模型,直接请求隐藏路由仍返回结构化错误)。before/after 复现确认修复端到端有效。无需删减,无遗漏,diff 中无无关改动。 — Qwen Code · qwen3.8-max-preview Reviewed at |
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.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
gwinthis
left a comment
There was a problem hiding this comment.
Review: APPROVE (C=0)
Summary
Clean fix (+132/-12) hiding the discontinued qwen-oauth model from non-OAuth sessions while keeping it visible for existing qwen-oauth sessions. The auth error path stays structured even when requesting a discontinued route directly.
Design
- Non-OAuth sessions (e.g.,
openaiauth): discontinued model hidden from selector, not advertised - Existing qwen-oauth sessions: model still visible and usable
- Auth error path: requesting discontinued route directly still returns structured error (tested in integration test)
Test Coverage
Parameterized tests cover both auth types:
openaiauth: onlyqwen3.6-27b(openai)visible, discontinued model hiddenqwen-oauthauth: bothcoder-model(qwen-oauth)andqwen3.6-27b(openai)visible
Integration test verifies the auth-required error path stays structured when requesting the discontinued route.
Pattern
Graceful deprecation by auth context: When deprecating a model tied to a specific auth type, hide it from sessions using other auth types (prevents confusion) but keep it for sessions already using that auth type (prevents breakage). The error path for direct requests stays structured regardless.
中文说明
评审:APPROVE (C=0)
概要
干净修复(+132/-12):对非 OAuth 会话隐藏已废弃的 qwen-oauth 模型,对现有 qwen-oauth 会话保持可见。
模式
按认证上下文优雅废弃: 废弃与特定认证类型绑定的模型时,对其他认证类型的会话隐藏(防混淆),对该类型会话保留(防中断)。直接请求的错误路径保持结构化。
— qwen3.7-max via Qwen Code /review
What this PR does
ACP sessions now omit the discontinued built-in Qwen OAuth model from model selectors when another authentication type is active. Existing Qwen OAuth sessions keep their current model and selector entry. Session creation, model configuration, and daemon session context use the same filtered options.
Why it's needed
The global model registry includes a built-in OAuth model even for users who configured only a local or custom OpenAI-compatible provider. Raw ACP responses consequently advertised that unrelated model first, allowing clients to present it as the preferred choice. The interactive model picker already filters the same discontinued entry; this brings the ACP surface in line with that behavior.
Reviewer Test Plan
How to verify
Create an ACP session with an authenticated OpenAI-compatible provider and a non-OAuth model selected. Confirm that the current model is unchanged, the discontinued OAuth model is absent from both model selectors, and the daemon session context matches. Then create a Qwen OAuth session and confirm its active model remains available. The regression tests exercise both cases and preserve the structured authentication error for a direct request to the hidden route.
Evidence (Before & After)
Before, the reproduced raw ACP response for an OpenAI-compatible session reported
qwen3.6-27b(openai)as current but placedcoder-model(qwen-oauth)first in both available model lists. After, the same built CLI reports only the configured OpenAI-compatible models; the current model remainsqwen3.6-27b(openai). The separate incorrect-current-model label reported by an external SDK was not reproduced on current main.Tested on
Environment (optional)
Built CLI with a local deterministic OpenAI-compatible test configuration; no external model request was required.
Risk & Scope
Linked Issues
Addresses the reproducible ACP model-list portion of #7433.
中文说明
本 PR 的改动
当会话当前使用其他认证类型时,ACP 模型选择器不再展示已停用的内置 Qwen OAuth 模型。已有的 Qwen OAuth 会话仍保留当前模型及其选择项。会话创建、模型配置和守护进程会话上下文现在使用同一组过滤后的选项。
为什么需要此改动
即使用户只配置了本地或自定义的 OpenAI 兼容提供商,全局模型注册表仍会包含一个内置 OAuth 模型。因此,原始 ACP 响应会把这个无关模型放在第一位,客户端可能将其显示为首选项。交互式模型选择器已经过滤了同一个已停用条目;本改动让 ACP 接口与该行为保持一致。
审阅者测试计划
验证方法
使用已认证的 OpenAI 兼容提供商并选择非 OAuth 模型来创建 ACP 会话。确认当前模型不变、两个模型选择器都不包含已停用的 OAuth 模型,并且守护进程会话上下文与其一致。然后创建 Qwen OAuth 会话,确认其当前模型仍可选择。回归测试覆盖这两种情况,同时保留直接请求隐藏路由时的结构化认证错误。
证据(修改前后)
修改前,在 OpenAI 兼容会话中复现的原始 ACP 响应将
qwen3.6-27b(openai)正确报告为当前模型,但在两个可用模型列表中都把coder-model(qwen-oauth)放在第一位。修改后,同一个构建后的 CLI 只报告已配置的 OpenAI 兼容模型,当前模型仍为qwen3.6-27b(openai)。外部 SDK 报告的另一项“当前模型标签错误”在当前 main 分支上未能复现。测试平台
环境(可选)
使用本地确定性的 OpenAI 兼容测试配置运行构建后的 CLI;无需向外部模型发起请求。
风险与范围
关联 Issue
本 PR 处理 #7433 中可复现的 ACP 模型列表问题。