feat(core): add model grade selection for subagent spawn (#7685) - #7702
Conversation
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. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@yiliang114 This PR's body is a placeholder ("Not ready for review") and is missing most required sections from the PR template:
## What this PR does(has## What— close, but the template heading is different)## Why it's needed## Reviewer Test Plan(including### How to verify,### Evidence (Before & After),### Tested on)## Risk & Scope## Linked Issues<details><summary>中文说明</summary>
The implementation in the diff looks substantial (settings schema, grade resolution, dynamic tool schema, fork/teammate validation, and tests), so when you're ready for review, filling in the template — especially the Why and Reviewer Test Plan sections — will help reviewers get up to speed quickly.
中文说明
此 PR 的描述是占位符（"Not ready for review"），缺少 PR 模板 中的大部分必填章节：
## What this PR does（当前为## What——接近但标题不完全匹配）## Why it's needed## Reviewer Test Plan（包括### How to verify、### Evidence (Before & After)、### Tested on）## Risk & Scope## Linked Issues<details><summary>中文说明</summary>
diff 中的实现内容很充实（settings schema、grade 解析、动态工具 schema、fork/teammate 校验以及测试），准备好 review 时，填写模板——尤其是 Why 和 Reviewer Test Plan 部分——能帮助 reviewer 快速了解上下文。
— Qwen Code · qwen3.8-max-preview
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
…ranch - subagent-manager: add tests for non-string grade values, blank values, array-shaped modelGrades, and missing modelGrades (all return undefined) - background-agent-resume: assert configured subagent model is preserved (not forced to 'inherit') when launch flags (model + authType) are absent Addresses test-coverage review findings.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
中文说明
已审查——无阻断问题。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
— qwen3.7-max via Qwen Code /review
|
I did not deduplicate the grade validation in this PR. That would be a shared-logic refactor beyond this closeout scope, so I am leaving it as follow-up. |
Review — model grade selection for subagent spawnVerified locally at Overview. Adds an optional What I verified
1. The
|
|
PR body updated with full template — should pass the Stage 1a gate on re-triage. Re: grade validation duplication — noted. This PR is currently a placeholder (design doc only, no implementation yet). When the actual code lands, |
…el validate - Extract normalizeModelGradeSettings helper shared by resolveModelGrade and the Agent tool schema build, so the advertised grades and runtime resolution cannot drift (addresses duplicated shape invariant). - Merge the three model-parameter validate branches under a single `params.model !== undefined` guard. - Update agent.test.ts mock to preserve the real helper while still mocking SubagentManager.
…into codex/pr-7702-review-fixes # Conflicts: # packages/core/src/subagents/subagent-manager.ts # packages/core/src/tools/agent/agent.ts
|
Re-review closeout for the findings in @wenshao's review:
Focused verification: core 375/375 and CLI 335/335 passed before the closeout changes; the runtime-validation pass added 204/204 core tests, full typecheck passed, and the subsequent schema/doc-only changes passed their focused check. |
Independent local verification report (head
|
| Scenario | Observed |
|---|---|
| Schema exposure | model present with enum exactly ["small", "high"] — hidden (not allowlisted) absent, concrete selectors never visible to the model |
Grade small (bogus selector) |
Subagent failed with the provider's model-not-found error — proof the mapped selector, not the grade name or a default, reached the provider |
Grade high |
Provider error naming the exact mapped selector qwen3.7-max-2026-06-08 (this key lacks direct authorization for it) — grade→selector mapping proven end-to-end at the wire |
Unknown grade medium |
Rejected at validation: Unknown model grade "medium". |
subagent_type: "fork" + model |
Rejected at validation: Parameter "model" cannot be used with subagent_type "fork". |
3. Tests and code review
- Focused suites on this branch: core
agent.test.ts+subagent-manager.test.ts+background-agent-resume.test.ts378/378; CLIsettingsSchema.test.ts+config.test.ts335/335. - No-drift claim holds structurally: the schema enum and runtime resolution both read
getAvailableModelGrades(), a single normalized map (trimmed keys/values, allowlist applied, malformed shapes → empty map). - Teammate rejection is enforced twice —
validateToolParamsand again in the execute-time team-routing branch — with matching guard conditions (name && !isTeammate() && isTopLevelSession() && teamManager), so a validation bypass cannot reachexecuteTeammatewith a grade. - Custom-agent precedence sits in
resolveModelGrade: explicit non-inheritmodel on a non-builtin agent returnsundefinedbefore any map lookup — the grade cannot override it.
4. Note (non-blocking)
The Risk section says unknown/disallowed non-empty grades "silently use the existing default model". Observed behavior is stronger: validateToolParams rejects them with Unknown model grade before launch (verified in §2). The silent-fallback path in resolveModelGrade is only reachable if settings change between validation and execution. Suggest updating the risk note so reviewers don't test for a fallback that normally cannot happen.
Not exercised here: teammate rejection under an active team, and background pause/resume model retention — both covered by unit tests but not by this live run.
Conclusion
Thesis: grades are a policy boundary, and this PR keeps both sides of that boundary honest — the model only ever sees validated grade names, and the runtime provably launches the mapped selector. Evidence: baseline absence (§1), wire-level proof via bogus and unauthorized selectors (§2), single-map no-drift structure and double-guarded teammate rejection (§3). This also covers the PR's "Linux CI pending" gap. LGTM with the §4 doc-level note.
中文摘要
Linux 真机（tmux + 活体 API）独立验证：基线 0.20.1 无 model 参数；补丁版 schema 仅暴露 allowlist 内等级（hidden 不可见、不泄露具体 selector）；把 small 映射到不存在的模型、high 映射到未授权模型，provider 报错分别点名映射后的具体 selector——证明等级→模型映射端到端真实生效；未知等级与 fork+model 均在校验层被拒。聚焦单测 core 378/378、CLI 335/335。一条非阻塞备注：PR 风险段写"未知等级静默回退默认模型"，实测是校验期直接拒绝（更强），建议更新描述。
— independent review loop, live-API verification on Linux
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.7-max via Qwen Code /review
| grade.trim() !== '' && | ||
| typeof model === 'string' && | ||
| model.trim() !== '' && | ||
| (allowedGrades === undefined || allowedGrades.includes(grade)) | ||
| ? [[grade, model.trim()] as const] | ||
| : [], |
There was a problem hiding this comment.
[Suggestion] Grade keys are checked for emptiness via grade.trim() but stored in the map (and advertised in the tool schema enum) untrimmed, while values are trimmed. This asymmetry creates invisible misconfiguration. — Failure scenario: modelGrades: { "small ": "fast" } with allowedGrades: ["small"] — the allowlist check (["small"].includes("small ")) silently excludes the grade, the model parameter disappears from the schema entirely. Without an allowlist, the schema advertises "small " (trailing space), the LLM sends "small", and validation rejects it with a nearly invisible trailing space in the error message.
| grade.trim() !== '' && | |
| typeof model === 'string' && | |
| model.trim() !== '' && | |
| (allowedGrades === undefined || allowedGrades.includes(grade)) | |
| ? [[grade, model.trim()] as const] | |
| : [], | |
| grade.trim() !== '' && | |
| typeof model === 'string' && | |
| model.trim() !== '' && | |
| (allowedGrades === undefined || allowedGrades.includes(grade.trim())) | |
| ? [[grade.trim(), model.trim()] as const] | |
| : [], |
中文说明
等级键通过 grade.trim() 检查非空，但存入 Map（和工具 schema 枚举）时未 trim，而值被 trim。这种不对称会导致不可见的配置错误。—— 失败场景：modelGrades: { "small ": "fast" } 配合 allowedGrades: ["small"]——白名单检查 (["small"].includes("small ")) 会静默排除该等级，model 参数从 schema 中完全消失。没有白名单时，schema 展示 "small "（尾部空格），LLM 发送 "small"，验证拒绝，错误信息中的尾部空格几乎不可见。
建议修复：存储前和对白名单检查前都 trim 等级键。
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Fixed in ad35d02b — getAvailableModelGrades now trims the grade key once (normalizedGrade) and uses it for the emptiness check, the allowlist comparison, and the stored map key, so a padded key like " small " publishes "small" and matches an "small"-only allowlist. Added "trims grade keys before publishing and resolving" to lock the behavior in.
Review R2 — model grade selection for subagent spawnRe-reviewed at R1 findings are fixed and the fixes are mutation-verified. The unknown-grade rejection is real, and the schema/runtime "no drift" claim is now structurally pinned rather than asserted. No blockers. Everything below is Low or nit. What I verified
Mutation matrix — 7/7 caught (no vacuous tests):
M7 is the meaningful one: the single-map contract is genuinely enforced, not just documented. 1. The resume hunk changes behavior for every background agent, and the PR body doesn't say so (Low–Medium)
authType: runtimeAuthOverrides?.authType ?? contentGeneratorConfig.authType,— so The PR's own test proves it isn't grade-specific: it uses a plain The actual semantic change: M6 confirms the exposure is one assertion bolted onto an unrelated pre-existing test; nothing else in 378 tests depends on it. Ask: either declare it in Risk & Scope with the "resume pins the launch-time model" wording, or split it into its own PR. Note also that grades work without it — 2. Fork protection is validation-only, and the execute path still resolves a grade onto
|
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
|
Thanks for the PR! Template looks good ✓ Problem: This is a feature request (not a bug fix) — linked issue #7685 clearly describes the user need: delegating tasks at different cost/capability tiers without exposing provider-specific model IDs. The issue is labeled Direction: Aligned. Claude Code's CHANGELOG shows active investment in subagent model selection (explicit model overrides on resume, Explore model inheritance). This PR adds a user-facing grade abstraction on top of that existing infrastructure — additive, optional, and within the subagent domain. No direction concerns. Size: 138 production logic lines (6 source files), 252 test lines (6 test files), 14 schema lines, 75 docs lines. Well within bounds for a feat touching core paths. Approach: The scope feels right — settings definition, schema publication, validation, resolution, and background resume are all necessary for this feature to work end-to-end. No unrelated changes or drive-by refactors. The design doc is a nice addition for a feature of this complexity. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是一个功能请求（非 bug 修复）——关联 issue #7685 清楚描述了用户需求：以不同成本/能力等级委派任务，同时不暴露 provider 专属模型 ID。Issue 已标记 方向：对齐。Claude Code CHANGELOG 显示 subagent 模型选择是活跃投入方向（恢复时显式模型覆盖、Explore 模型继承）。本 PR 在现有基础设施上增加用户可见的等级抽象——附加、可选、在 subagent 领域内。无方向顾虑。 规模：138 行生产逻辑（6 个源文件），252 行测试（6 个测试文件），14 行 schema，75 行文档。对于触及核心路径的 feat 完全在范围内。 方案：范围合理——设置定义、schema 发布、校验、解析、后台恢复都是此功能端到端工作所必需的。无无关改动或顺手重构。设计文档对于此复杂度的功能是不错的补充。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: I would add Comparison: The PR matches this approach exactly. The implementation is straightforward and follows existing patterns in the codebase. No critical blockers found. Specific observations:
One minor note (non-blocking): the grade validation logic in Test EvidenceCI on
macOS/Windows/Integration tests are skipped (CI optimization for same-repo branches). Ubuntu unit tests pass. Coverage: CLI 83.06%, Core 87.12%. Not verified: real-scenario TUI testing (this is a CI run; a maintainer can trigger the isolated 中文说明代码审查独立方案： 我会添加 对比： PR 完全匹配此方案。实现直接，遵循代码库现有模式。 未发现关键阻塞问题。具体观察：
测试证据CI 在 未验证：真实场景 TUI 测试（本次为 CI 运行；如需可触发 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — Clean across every stage; well-scoped feature with solid test coverage and two maintainer approvals already on the current commit. This is a straightforward, well-executed feature addition. The grade abstraction sits cleanly on top of the existing model resolution infrastructure without modifying it. Every change in the diff serves the stated goal — settings, schema, validation, resolution, resume — nothing extra. The test coverage is thorough (252 lines covering resolution, allowlist filtering, malformed input, fork/teammate rejection, schema publication, and background slot selection). Two maintainers (@wenshao, @doudouOUC) have independently verified and approved the current commit. The previous CHANGES_REQUESTED from an earlier triage run (on 中文说明置信度：5/5 —— 每个阶段都干净；范围合理的功能，测试覆盖扎实，当前 commit 已有两位维护者批准。 这是一个直接、执行良好的功能添加。等级抽象干净地建立在现有模型解析基础设施之上，未修改它。Diff 中每个改动都服务于既定目标——设置、schema、校验、解析、恢复——没有多余内容。测试覆盖全面（252 行覆盖解析、allowlist 过滤、格式错误输入、fork/teammate 拒绝、schema 发布、后台槽位选择）。两位维护者（@wenshao、@doudouOUC）已独立验证并批准当前 commit。 之前 triage 运行的 CHANGES_REQUESTED（在 — Qwen Code · qwen3.8-max-preview Reviewed at |
Stale — findings addressed in subsequent commits, confirmed by maintainer R2 review.
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. ✅
Grade keys were checked for emptiness via grade.trim() but stored in the map and advertised in the tool schema enum untrimmed, while values were trimmed. A padded key like ' small ' published a padded enum name the model had to reproduce verbatim, and the allowlist check silently excluded it. Normalize the key before storing, allowlist matching, and schema publication. Also adds a test for the delete schema.properties.model branch that fires when grades transition from available to empty, so a regression that breaks the delete leaves no stale model enum in the tool schema.
ad35d02
|
Closeout round for @wenshao's R2 and the two open inline threads. Addressed in
Deferred (Low/nit, not blockers; recorded so they aren't dropped):
Focused verification: core |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.7-max via Qwen Code /review
|
Closeout for this pass: fixed the |
Review R3 — model grade selection for subagent spawnRe-reviewed at R2 #3 is fixed and mutation-confirmed. The two new commits ( What I verified
Mutation matrix — 5/5 caught:
M3 is the one the new test was written for, and it does its job — a regression that leaves a stale 1. R2 #3 — confirmed fixedProbe against the real Both halves are covered — key normalization and the allowlist 2. Carried findings — status at this head
#1 is unchanged and is still my one ask. Re-read at this head: #2 — I'm downgrading this. R2 said the fork guard was single-layered; that's accurate, but I've now checked reachability and it's not exploitable today. requestedType?.toLowerCase() === FORK_SUBAGENT_TYPEso there's no state that passes validation and then takes the fork branch. The residue is genuinely just robustness — the execute path still does resolve a grade onto and the comment at agent.ts:2555 ("forks always inherit, since FORK_AGENT has no model selector") now states an invariant that's enforced two layers away rather than by the type itself. For the record, I also chased two things that turned out to be non-issues, so nobody else needs to: the foreground path never sets 3. New nits from the trim fix3a. Two strings that render identically in most UIs, with no hint that whitespace is the problem. This is exactly the failure mode 3b. Keys that collide after trimming silently last-win.
3c. Naming. The parameter is VerdictGood to merge from my side once the PR body is corrected. The implementation has held up across three rounds: the single-map contract between schema and runtime is mutation-pinned (M3/M5), the unknown-grade rejection is real (M4), and the trim fix is complete on both the key and allowlist sides (M1/M2). Remaining ask, in priority order:
中文说明在隔离 worktree 中于 R2 的第 3 项已修复并经变异测试确认。 新增的两个提交（ 验证：core 433/433、CLI 335/335、 1（已修复） 实测 遗留项状态： #1 未处理（仍是我唯一的请求）、#2 降级、#3 已修复、#4/#5 未处理、#6 未处理。
新增细节（均为 nit）：
结论： 我这边在 PR 描述修正后即可合并。三轮下来实现是站得住的——schema 与运行时的单一 Map 契约由 M3/M5 钉住，未知等级拒绝由 M4 钉住，trim 修复在 key 与白名单两侧都由 M1/M2 钉住。 剩余请求按优先级：(1) 修正 Risk & Scope 中"未知/禁用等级会静默回退默认模型"的说法——现在是直接拒绝（ — reviewed in an isolated worktree at |
qwen-code-ci-bot
left a comment
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
|
Released in v0.21.1. |
5 participants
What this PR does
Adds an optional model grade to regular Agent tool launches. Users map semantic grade names such as
smallorhighto private model selectors and may restrict which grades are exposed. The tool schema publishes only validated grade names; runtime resolution uses the same normalized map, so advertised and executable grades cannot drift.Grades apply to built-in agents and custom agents that inherit their model. A custom agent with an explicit model keeps that model. Fork agents and named teammates reject the parameter. Background launches reserve capacity and resume with the concrete launch-time model and provider settings.
Why it is needed
Different delegated tasks need different cost and capability levels, but callers should not need access to provider-specific model IDs. User-defined grades provide a stable policy boundary while preserving existing behavior when the parameter is omitted, unknown, or disallowed.
Reviewer Test Plan
How to verify
Evidence (Before & After)
Before: invocation-time selection could not express a user-defined model tier. After: regular subagents accept validated semantic grades without exposing concrete model configuration.
Tested on
Environment (optional)
Local Node 22 workspace. Five focused test files passed: 375 core tests and 335 CLI tests. Full build and typecheck pass.
Risk & Scope
Linked Issues
Closes #7685
中文说明
本 PR 做了什么
为普通 Agent 工具启动增加可选的模型等级。用户把
small、high等语义等级映射到私有模型 selector，并可限制允许暴露的等级。工具 schema 只发布校验后的等级名；运行时复用同一份规范化映射，因此 schema 与实际可执行等级不会漂移。等级适用于内置 agent，以及继承模型的自定义 agent。显式配置模型的自定义 agent 仍优先使用自身模型。Fork agent 与命名 teammate 会拒绝该参数。后台启动会按具体模型预留并发槽，并在恢复时保留启动时的模型和 provider 配置。
为什么需要
不同委派任务需要不同成本与能力等级，但调用方不应接触 provider 专属模型 ID。用户自定义等级提供稳定的策略边界；参数省略、未知或被禁用时，现有行为保持不变。
Reviewer Test Plan
如何验证
前后证据
Before：调用时无法表达用户自定义模型档位。After：普通 subagent 可使用经过校验的语义等级，且不会暴露具体模型配置。
测试环境
macOS 已验证；Windows 未验证；Linux CI 运行中。5 个聚焦测试文件通过：core 375 项、CLI 335 项；全量 build 与 typecheck 通过。
风险与范围
Closes #7685