feat(serve): add budget-based ACP child admission - #11911
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
E2E verification reportLocal verification on macOS / Node 22.22.3, using isolated temporary workspaces and no real model requests:
The final API/ACP run used CLI bundle SHA256 Build, typecheck, formatting/lint and focused tests passed. The complete server suite passed 1,278/1,278 with a runtime-only Supertest address correction, proven active for 1,774 requests. On this macOS host, default IPv6 listening can reuse a port held by an unrelated IPv4 listener while Supertest constructs an IPv4 request URL; ordinary runs therefore sometimes returned another service's response. An owned-server probe reproduced this. The original failing runs are not relabeled green, and no dependency file, production code or global network setting was changed for the workaround. Limits: the browser verifies English first-message creation, not the Provider cold-load branch; that branch is covered by the 377-test Provider suite, including a readable-error regression. Goal/shell draft preservation and Chinese notices are covered by App component tests. Natural ten-minute keep-alive expiry, real low-memory/model load, Windows/Linux runtime behavior and MCP mutation E2E were not exercised. MCP GET was metadata-only and is not presented as cold-start evidence. 中文说明E2E 验证报告本地 macOS / Node 22.22.3，使用隔离临时 workspace，不调用真实模型：
最终 API/ACP 使用 CLI bundle SHA256 构建、类型检查、格式/lint 和定向测试通过。完整服务端测试通过 1,278/1,278，使用仅在运行时生效的 Supertest 地址修正，已证明实际作用于 1,774 个请求。该 macOS 宿主上，默认 IPv6 监听可能使用已被无关 IPv4 服务占用的端口，而 Supertest 构造 IPv4 请求 URL，因此普通运行有时会收到其他服务的响应。自有服务探测已复现该机制。原始失败不重新标记为通过；该测试绕过没有修改依赖文件、生产代码或全局网络设置。 边界：浏览器验证英文首条消息创建，不覆盖 Provider 冷加载分支；后者由 377 项 Provider 测试覆盖，包括可读错误回归用例。Goal/shell 草稿保留及中文提示由 App 组件测试覆盖。未验证自然十分钟 keep-alive 到期、真实低内存/模型负载、Windows/Linux 运行行为，以及 MCP 变更 E2E。MCP GET 只读元数据，不作为冷启动证据。 |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 4 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
activeWorkStaleMs |
6 |
5 |
— Qwen Code · serve A/B
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Assert the admissionEnforced field and its false value in default observe mode. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Re-ran the gate on Thanks for the PR, and for filing the tracker (#11907) first. Gating a stage-1 slice against written acceptance criteria is much easier than guessing intent. Template looks good ✓ — all nine headings, bilingual body, and the design doc in both languages. Problem: observed, not theoretical. #8182 records that every ACP child is authorised 50% of host memory and never divided by child count, and the tracker records that Direction: aligned. This is stage 1 of a maintainer-filed tracker and it stays inside the stated criteria — no reclamation, no workspace chooser, no heap-argument change, no new env var, default still Size: cross-package ( Approach: scope matches the tracker, and leaving PR2/PR3 out is the right call. Two things I'd still push on, neither a blocker:
Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths. The real operational risk is the one you documented yourself: at full occupancy a channel replacement can be refused while the old child is still tracked, so a workspace can be unable to restart until teardown completes. PR2's idle reclamation is the relief, and it is worth restating for anyone who opts in before PR2 lands. Moving on to code review. 🔍 中文说明在 感谢贡献，也感谢先写了 tracker（#11907）。按书面验收标准审查第一阶段切片，比猜测意图容易得多。 模板完整 ✓ —— 九个标题齐全，正文双语，设计文档两种语言都有。 问题： 是已观测到的问题，不是理论性加固。#8182 记录了每个 ACP 子进程都被授权主机内存的 50%、且从不按子进程数平分；tracker 记录了 方向： 对齐。这是维护者自己提的 tracker 的第一阶段，且严格停留在既定验收标准内——不做回收、不做 workspace 选择器、不改子进程堆参数、不新增环境变量、默认仍是 规模： 跨包改动（ 方案： 范围与 tracker 一致，PR2/PR3 不塞进来是正确的。仍有两点想提出来，都不是阻塞项：
风险： 无升级风险信号——变更文件都没有命中与 revert 相关的高风险路径。真正的运维风险是你自己已经写明的：满名额时，旧子进程仍被跟踪期间，channel 替换可能被拒绝，因此 workspace 在 teardown 完成前可能无法重启。PR2 的空闲回收是解法；在 PR2 落地前开启该模式的人值得再被提醒一次。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewMy own baseline for "enforce a concurrent ACP child limit" was a counter plus a mutex in the bridge, checked before spawn, with a new error type. What shipped is better than that: the reservation already existed in What I checked hardest, and what I found — each of these I read at
Two non-blocking findings. Neither is a correctness problem and neither should hold the merge:
One thing to keep visible, not a defect: you documented it yourself, but it is the sharpest edge of turning sequenceDiagram
participant P1 as Client REST or ACP
participant P2 as standalone session service
participant P3 as spawnChannel factory
participant P4 as ProcessRegistry
participant P5 as ChildHeapPolicy
participant P6 as error mapper
participant P7 as Web Shell
P1->>P2: create or load a session in a cold workspace
P2->>P3: spawn an ACP child
P3->>P4: reserve a slot before deciding
P3->>P5: decide using the committed count
P5-->>P3: refuse, count is past the modeled limit
P3->>P4: cancel the reservation
P3-->>P2: AcpChildCapacityExceededError
P2-->>P6: rolled back, carrying the capacity cause
P6-->>P1: 503 with code acp_child_capacity_exhausted, no Retry-After
P1->>P7: surface the error
P7-->>P7: stop automatic retries, keep the draft
Files changed (23 production files of 43; 14 test files and 6 docs omitted)
TestingWhich evidence this carries: this is an unattended CI run, so the review is static — I built nothing and executed no PR-derived code. Everything below is the PR's own CI, read through the API for the reviewed commit. The author's macOS/Chromium results in the description are the author's claim, not evidence I re-ran. All 24 checks on
Two honest caveats on that green:
Sandboxed verification would settle exactly that gap, and it is already running: this re-run dispatched a Not verified: Windows and Linux runtime behavior (no lane covers them). Not verified: that a capacity error delivered over the ACP SSE stream always reaches the recognizer as a 中文说明代码审查我自己对"限制并发 ACP 子进程"的基线方案是：在 bridge 里加计数器加互斥锁，spawn 前检查，再配一个新错误类型。实际交付的比这个更好： 我核得最硬的几点，都是在
两条非阻塞发现。 都不是正确性问题，都不应挡住合并：
一件需要保持可见的事，不是缺陷： 你自己已经写明，但这是今天开启 （流程图与文件清单见英文部分，此处不重复。） 测试本节承载的是哪种证据： 这是无人值守的 CI 运行，因此审查是静态的——没有构建任何东西，也没有执行任何 PR 派生代码。下面全部是通过 API 读到的、该 PR 自身在被审 commit 上的 CI 结果。描述里作者的 macOS/Chromium 结果是作者的自述，不是我复跑的证据。
对这份绿色有两点如实说明：
沙箱验证恰好能补上这个缺口，而且它已经在跑：本次 re-run 在 triage 之外还派发了 未验证：Windows 与 Linux 运行行为（无通道覆盖）。未验证：经 ACP SSE 流送达的容量错误是否总以 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 4/5 — solid, and I could not break the part that matters. What keeps it off 5 is two cosmetic gaps and the fact that no CI lane has driven a real daemon in Stepping back. My independent proposal for this was a counter and a mutex in the bridge, checked before spawn, with a new error type — a small parallel subsystem. What you shipped reuses the reservation The accounting is where admission code usually goes wrong, so I went at it expecting an off-by-one and did not find one. Counting the caller's own reservation and refusing on The fail-closed discipline is what I would have asked for and did not have to: zero-slot budget, admitting policy without a registry, On the "does default behavior regress" question I have better evidence than the unit tests. The Serve A/B lane built base and head, drove a fixed endpoint set against each, and the only differing field in the entire response diff was What is left, and none of it blocks:
Verdict is approve — but the approval is not recorded yet, and I want that to be unmissable rather than silently wrong. Two things stand in the way, and neither is about this diff:
I did not delete or submit that review. It is another lane's container, clearing it is irreversible, and a stuck sibling workflow is not a reason to destroy its state — that call belongs to a maintainer, not to this gate. So the PR currently reads as 1 of 2 required approvals, with the triage verdict in favour. To land the vote: cancel or re-run 中文说明Confidence: 4/5 —— 扎实，而且最要命的那部分我没能攻破。没到 5 分的原因是两处外观层面的缺口，以及没有任何 CI 通道以 退一步看整体。我自己对这个需求的方案是：在 bridge 里加计数器加互斥锁，spawn 前检查，再配一个新错误类型——一个小型的平行子系统。你交付的版本复用了 计数是准入代码里最容易出错的部分，所以我带着"一定有个差一错误"的预期去看，结果没找到。把调用者自己的预留计入、并以 fail-closed 的纪律是我本来会要求、结果不用开口就有的：零名额预算、拿到准入 policy 却没有 registry、 关于"默认行为是否回归"，我拿到了比单元测试更有力的证据。Serve A/B 通道分别构建 base 与 head、对同一组固定端点驱动并比对，整个响应 diff 中唯一不同的字段是 剩下的问题，都不阻塞：
结论是批准——但这一票目前尚未记录成功，我希望这一点显眼，而不是悄悄地错着。 有两个障碍，都与本 diff 无关：
我没有删除、也没有提交那个 review。它是另一个通道的容器，清除是不可逆的，而一个卡住的兄弟 workflow 不构成销毁其状态的理由——这个判断属于维护者，不属于本门禁。因此该 PR 目前显示为 2 个必需批准中的 1 个，而 triage 的结论是支持的。 要让这一票落地：取消或重跑 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 73 passed · 0 failed · 73 total Flakiness gate: 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：73 通过 · 0 失败 · 73 总计 抖动门： Verification reportPR #11911 —
|
| Cell | Oracle | HEAD | BASE |
|---|---|---|---|
| 1-slot budget models exactly 1 child | policy.snapshot().maxConcurrentChildren |
PASS (1) | PASS (1) |
| 1st spawn succeeds (real process) | factory resolves, live pid | PASS | PASS |
| 2nd concurrent spawn refused | throws / resolves | PASS (refused) | FAIL (spawned) |
refusal is AcpChildCapacityExceededError, code acp_child_capacity_exhausted |
error identity + .code |
PASS | FAIL |
maxConcurrentChildren=1, committedAcpChildren=1 |
error fields | PASS | FAIL |
| refused reservation released (no leak) | committedProcessCount back to 1 |
PASS | FAIL (2) |
refusals counted exactly once |
policy.snapshot().refusals |
PASS | PASS |
| spawn during teardown still refused (terminating child counts) | throws | PASS | FAIL |
| retry after release succeeds | factory resolves | PASS | PASS |
observe / off: both spawns succeed |
2 live processes | PASS | PASS |
| 3-slot budget: 3 spawn, 4th refused | error fields max=3 committed=3 | PASS | FAIL |
| registry really holds N active children | activeProcessCount |
PASS | FAIL (4) |
| zero-slot admit config rejected at construction | TypeError |
PASS | FAIL (constructed) |
| admit policy without explicit registry rejected | TypeError |
PASS | FAIL (constructed) |
| child heap args identical admit vs observe | child's own process.execArgv |
PASS | PASS |
Totals: HEAD 26/26, BASE 13/26 — 13 cells flip, all in the enforcement direction. The base arm's 13 reds are the control's expected pre-PR behaviour; they are encoded as 7 passing assertions in control-base.mjs (7/7), including "base really has 2 live child processes in the registry (over-admission is real, not a stub)". cell11 passes on both arms by design — it is the no-regression cell for "admit keeps legacy heap flags".
Secondary claim 1 — reservations/terminating count, registration/reuse do not. Proven for the reservation and teardown halves (cells 2 and 10). Registration/reuse consume no slot by construction: no spawn ⇒ no reserve(); the census below shows spawnChannel.ts:501 is the only --acp spawn site.
Secondary claim 2 — recognizable capacity error on REST and ACP, no Retry-After, status distinguishes enforcement. Proven by wire-contract.mjs (26/26) and consumer-contract.ts (9/9): real Express + real sendBridgeError/toRpcError over a real loopback socket. Witness: evidence/02-wire-contract-rest-rpc-status-contract.png.
- REST direct & wrapped:
503,code/errorKind=acp_child_capacity_exhausted, noRetry-After, counts on the body. - Standalone rollback + capacity:
503(not the code-derived 500),capacityobject on the wire,retryable: truebutRetry-Aftersuppressed, originalcodepreserved. - RPC/SSE:
data.errorKind/data.httpStatus/data.capacitycarry the same metadata; the Web Shell predicateisAcpChildCapacityErrorrecognises all six real producer shapes and rejects both non-capacity controls. - Status:
memory.enforcedstaysfalseunder admit (heap ceilings still not applied) whilechildHeap.admissionEnforcedistrueunder admit,falseunder observe, and defaults tofalsewhen omitted. - Regression controls: retryable non-capacity standalone still
500+Retry-After: 5; generic runtime-init still503 runtime_initialization_failed+Retry-After: 5.
Secondary claim 3 — Web Shell stops automatic capacity retries and preserves drafts. Covered by the PR's own component/provider tests (1370/1370 in the changed files, including "stops automatic load retries on capacity rejection" for both body shapes and the /goal draft path). Not independently re-driven in a browser — see Not covered.
Bypass census (the "accepting end" of the gate). spawnChannel.ts:501 is the only --acp spawn site. Four production createSpawnChannelFactory call sites exist; the three daemon bridges (run-qwen-serve.ts:5274/6286/6946) all receive the shared processRegistry + admit policy, and createServeApp's guard throws before a bridge could fall back to the ungated defaultSpawnChannelFactory under admit. No production path bypasses admission.
Findings
F1 — Suggestion: the zh-CN capacity message is unreachable from the connection-error surface
packages/web-shell/client/daemon/session/DaemonSessionProvider.tsx:4012 hardcodes getTranslator('en')('daemon.capacity.exhausted'), and the resulting message is stored into user-visible connection.error. This PR adds a zh-CN string for that key (i18n.tsx) and uses the locale-aware t() at the other two surfaces (App.tsx toast + notice handler, McpManagerPage.tsx). A zh-CN user therefore sees an English connection-error banner for the same capacity rejection whose toast is Chinese.
Reproduce:
grep -n "getTranslator('en')" packages/web-shell/client/daemon/session/DaemonSessionProvider.tsx
# 4012: ? getTranslator('en')('daemon.capacity.exhausted')The hardcoded 'en' is a deliberate guard — the PR's own test is named "shows a readable capacity reason on automatic load without an i18n provider", and the i18n context default is { language: 'en', t: (key) => key }, so useI18n().t outside a provider would leak the raw key. But the language is safe to read: the same context default supplies language: 'en', and in production DaemonSessionProvider is always rendered inside App's I18nProvider (App.tsx:17749). So getTranslator(useI18n().language) (captured in a ref for the async SSE loop) keeps the provider-less fallback and honours the active locale. Not measured in a browser; classified from source + the provider nesting census.
F2 — Suggestion: the admit + injected-bridge startup guard is pinned by no test
The description states "an injected/unwired admitting bridge are rejected at startup". The unwired half is tested (server.test.ts: "rejects unwired admission before creating the app"). The injected-bridge half (run-qwen-serve.ts:3829) has no test: disabling the guard left 503/503 tests green across run-qwen-serve.test.ts + serve.test.ts (mutation M10). House rule classifies a missing test for changed behaviour as a Suggestion. The guard's runtime behaviour was therefore not verified by this round either — it is listed under Not covered.
F3 — Nit: refusals documentation is stale under admit
packages/acp-bridge/src/child-heap-policy.ts still documents refusals as "Spawns that would have been refused" and calls the full-occupancy channel-swap case a "metric artifact". Under admit those are real, user-facing rejections (proven by cell10: a spawn during teardown of the only child is refused). The PR description acknowledges the tradeoff; the field's own doc does not.
Completeness (not merge conditions) — mutation survivors, all classified
Witness: evidence/03-mutation-matrix-killed-vs-survived.png. 23 mutation runs; every load-bearing guard is pinned (M1 central hunk, M2 zero-slot guard, M3 shared-registry guard, M4 Retry-After suppression, M5 REST branch, M6 RPC branch, M8 standalone propagation, M9-2 coordinator clause 2, and the combination row M9-all proving the coordinator clause set is load-bearing). Survivors:
- M9-1 / M9-3 / M9-13 (coordinator clauses 1 & 3, alone and together): redundant defence. Both REST (
error-response.ts:438) and RPC (dispatch.ts:661) unwrapWorkspaceRuntimeInitializationError.causeone level, so a capacity error wrapped the way those clauses would wrap it still maps to503+ capacity code + noRetry-After. Proven by theobservability-probe.mjscell "wrapped capacity still maps to 503 + capacity code", not by reading the code. - M7b (
admissionEnforceddefault flipped totrue): unreachable default.createServeAppalways passes an explicit boolean (deps.managedChildProcesses?.policy.snapshot().mode === 'admit'), so the parameter default never applies in production. The payload is pinned both ways (M7c killed byrun-qwen-serve.test.ts.toBe(false); M7d killed bydaemon-status.test.tsadmissionEnforced: true). - M7 / M7-ctl (rounds 1–2): superseded / invalid target — the same-file-control rule showed the pinning tests live in a different file than the one first chosen; rounds 3–4 re-ran against both files.
Same-file positive control: M7d turned daemon-status.test.ts red (expected { enforced: false, … } to match object …), and M1/M9-2 failure text was captured (AssertionError: expected WorkspaceRuntimeInitializationError: … to be AcpChildCapacityExceededError: …), so the harness demonstrably makes these suites fail.
Not covered
- Full-daemon end-to-end REST 503. Not booted; the wire contract was proven at the seam instead (real Express + real
sendBridgeError/toRpcError+ real error classes over a real loopback socket). This reproduces the wire shapes the daemon emits, not the route-to-spawn traversal inside a live daemon. - Daemon-log observability of capacity rejections.
observability-probe.mjscould not measure it: the case that unconditionally callsrecordExpectedBridgeError(direct capacity) emitted nothing to stderr in a bare Express harness, soemitDaemonLoghas no sink there. A dead instrument supports no conclusion either way; the apparent asymmetry (if (status === 500) recordExpectedBridgeError(...)) is therefore reported as unmeasured, not as a finding. - Browser/Chromium verification of the Web Shell behaviour (capacity toast, draft retention, single create request). Only the component/provider tests were run.
- The
admit+ injected-bridge guard's runtime behaviour (F2) — only its absence of test coverage was proven. - Per-commit attribution. The checkout is shallow:
git rev-list HEAD^1..HEAD^2returns1while$QWEN_VERIFY_CONTEXTlists 3 commits. All evidence is against the aggregateHEAD^1..HEADdiff. - CLI-parser base arm. Base's
fast-path.tswas not executed (its TS import closure fails to resolve in a bare worktree); base rejection is evidenced statically (base sourceif (read.value !== 'off' && read.value !== 'observe') return { kind: 'fallback' }, baseserve.tschoices: ['off','observe'], head compiledchoices: ['off','observe','admit']). Head acceptance was executed (2/2). - Base
tsc --buildfailed on pre-existingpackages/coretype errors unrelated to this PR; the base acp-bridgedistemitted before the failure was verified pre-change and complete for the import closure used, and is cited as such. - Windows/macOS behaviour, real low-memory hosts, natural keep-alive expiry (all also out of scope per the PR).
Methodology
Environment: CI verify container (node:22-bookworm), merge-ref checkout (HEAD = merge, HEAD^1 = base tip, HEAD^2 = PR head), npm ci + npm run build pre-run at HEAD. Harnesses import the compiled dist/ of the tree named on the command line; the base control is a git worktree at HEAD^1 under tmp/, built with npm run build -w packages/acp-bridge (its tsc --build failed on unrelated core errors but emitted the four needed modules, verified pre-change by exact-hunk grep and by ChildHeapMode in the emitted .d.ts), imported by absolute path with a realpath assertion that no @qwen-code/* symlink leaks head code. ab-spawn-admission.mjs spawns real Node children through the real factory; wire-contract.mjs / observability-probe.mjs mount the real error mappers on real Express over 127.0.0.1; consumer-contract.ts (tsx) feeds the dumped real wire bytes into the real Web Shell predicate and the real DaemonHttpError. Gates: acp-bridge targeted files (child-heap-policy, spawnChannel, process-registry) 97/97; CLI serve changed files 2162/2162; web-shell changed files 1370/1370; npm run typecheck exit 0. Mutation matrix: 23 runs, each reverting one guard in real source with byte-exact restore (git status --porcelain clean afterwards). Assertion counts in assertions.json cover only the six harnesses' scripted checks (26+7+26+9+2+3); the base arm's 13 reds are the control's expected behaviour and are counted as the 7 passing control-base.mjs assertions, and the mutation matrix is reported separately as completeness. Raw logs: logs-02-wire-contract.txt, logs-03-mutation.txt, logs-04-base-control.txt, mutation-matrix*.json, wire-bodies.json in this directory.
Flakiness gate log
rounds=5 files=14 skipped=0
file packages/acp-bridge/src/child-heap-policy.test.ts: (cd packages/acp-bridge) npx --no-install vitest run ./src/child-heap-policy.test.ts
file packages/acp-bridge/src/spawnChannel.test.ts: (cd packages/acp-bridge) npx --no-install vitest run ./src/spawnChannel.test.ts
file packages/cli/src/commands/serve.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/serve.test.ts
file packages/cli/src/serve/acp-http/dispatch-error.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/acp-http/dispatch-error.test.ts
file packages/cli/src/serve/conversations/standalone-session-service.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/conversations/standalone-session-service.test.ts
file packages/cli/src/serve/daemon-status.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/daemon-status.test.ts
file packages/cli/src/serve/fast-path.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/fast-path.test.ts
file packages/cli/src/serve/run-qwen-serve.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/run-qwen-serve.test.ts
file packages/cli/src/serve/server.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server.test.ts
file packages/cli/src/serve/server/error-response.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server/error-response.test.ts
file packages/cli/src/serve/workspace-runtime-coordinator.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/workspace-runtime-coordinator.test.ts
file packages/web-shell/client/App.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/App.test.tsx
file packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/daemon/session/DaemonSessionProvider.test.tsx
file packages/web-shell/client/daemon/session/httpErrors.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/daemon/session/httpErrors.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/acp-bridge/src/child-heap-policy.test.ts: PPPP
packages/acp-bridge/src/spawnChannel.test.ts: PPPP
packages/cli/src/commands/serve.test.ts: PPPP
packages/cli/src/serve/acp-http/dispatch-error.test.ts: PPPP
packages/cli/src/serve/conversations/standalone-session-service.test.ts: PPP
packages/cli/src/serve/daemon-status.test.ts: PPP
packages/cli/src/serve/fast-path.test.ts: PPP
packages/cli/src/serve/run-qwen-serve.test.ts: PPP
packages/cli/src/serve/server.test.ts: PPP
packages/cli/src/serve/server/error-response.test.ts: PPP
packages/cli/src/serve/workspace-runtime-coordinator.test.ts: PPP
packages/web-shell/client/App.test.tsx: PPP
packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: PPP
packages/web-shell/client/daemon/session/httpErrors.test.ts: PPP
verdict: timeout
summary: only 3 of 5 rounds fit the 15-minute budget; the completed rounds agreed
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/acp-bridge/src/child-heap-policy.test.ts: P (exit 0)
round 1 · packages/acp-bridge/src/spawnChannel.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/serve.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/acp-http/dispatch-error.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/conversations/standalone-session-service.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/daemon-status.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/fast-path.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server/error-response.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/workspace-runtime-coordinator.test.ts: P (exit 0)
round 1 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/daemon/session/httpErrors.test.ts: P (exit 0)
round 2 · packages/acp-bridge/src/child-heap-policy.test.ts: P (exit 0)
round 2 · packages/acp-bridge/src/spawnChannel.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/serve.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/acp-http/dispatch-error.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/conversations/standalone-session-service.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/daemon-status.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/fast-path.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/server/error-response.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/workspace-runtime-coordinator.test.ts: P (exit 0)
round 2 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/daemon/session/httpErrors.test.ts: P (exit 0)
round 3 · packages/acp-bridge/src/child-heap-policy.test.ts: P (exit 0)
round 3 · packages/acp-bridge/src/spawnChannel.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/serve.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/acp-http/dispatch-error.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/conversations/standalone-session-service.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/daemon-status.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/fast-path.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/server/error-response.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/workspace-runtime-coordinator.test.ts: P (exit 0)
round 3 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/daemon/session/httpErrors.test.ts: P (exit 0)
round 4 · packages/acp-bridge/src/child-heap-policy.test.ts: P (exit 0)
round 4 · packages/acp-bridge/src/spawnChannel.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/serve.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/acp-http/dispatch-error.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Review findings
Scope: Production source files only. Docs, test files reviewed structurally but not line-by-line.
Environment: Static analysis only — no local build available; rung-2 execution not performed.
R1-1 · Minor · DaemonSessionProvider.tsx:4012
getTranslator('en') hardcodes English regardless of the user's language setting. The Chinese translation for daemon.capacity.exhausted was added in i18n.tsx but is never reached from this path.
The leak is wider than the call site: connection.error is set to the (English) string, ChatPane.tsx renders it as {t('splitView.paneConnectionError')}: {connection.error}, and App.tsx re-reports it via onError(new Error(connection.error)) — a plain Error, not a DaemonHttpError, so isAcpChildCapacityError returns false there and the localized path added in this PR is bypassed on the automatic-load path it targets.
Content is accurate; this is cosmetic only. If the provider genuinely cannot access the I18nProvider context, a short comment explaining why would prevent future readers from treating it as an oversight. Every other component that needs this key derives the language (App from selectedLanguage, McpManagerPage from useI18n()).
Coverage
Checked:
- Admission slot arithmetic:
reserve()increments count beforedecide(), refusal condition is> maxConcurrentChildren— first spawn sees count=1 with limit=1 and is admitted; second sees count=2 and is refused. No off-by-one. ✓ - Reservation leak on refusal: throw inside the
trywhosecatchisreservation.cancel(); throw— cancelled before anyspawn(), no orphaned child.committedAcpChildrenreported as count-1 (pre-cancel value). ✓ - Daemon-wide count: four production
createSpawnChannelFactorycall sites all pass the sameprocessRegistryandchildHeapPolicyinstance;admissionPolicy ?? createChildHeapPolicy(...)short-circuits so no second policy is ever built. ✓ - Fail-closed: four separate construction-time guards (zero-slot budget, admitting policy without registry,
admit+ injected bridge,createServeAppwithout admitting wiring). None silently degrades toobserve. ✓ - Error survival: capacity branch in
sendBridgeErrorsits before the genericWorkspaceRuntimeInitializationErrorbranch and unwraps.cause;toRpcErrormirrors the same pattern. Raw rethrow from coordinator bypasses no handler — the sole production reader already unwraps.cause. ✓ Retry-Aftersuppressed on capacity (err.retryable && !err.capacity); auto-reconnect blocked bycapacityRejectedcondition. ✓observemode unaffected:decide()still called once per spawn; admission branch only fires when snapshot saysadmit; argv unchanged. Consistent with A/B lane showing only timing jitter. ✓isAcpChildCapacityError: five-shape recognizer covers all live REST and RPC body shapes; tests enumerate each case and the non-capacity 503s that must not match. ✓- SDK types:
admissionEnforcedandcommittedAcpChildrenadded as optional (additive for older daemons). ✓
Not covered:
- Execution rung 2 not performed (no local build). The in-process unit suite (462 lines across 14 files, including the cross-factory slot-release case) provides strong static evidence; no CI lane drives a real daemon with
--child-heap-mode admit. This is a noted gap but not an approval blocker given the author's detailed macOS verification and the A/B no-regression signal for the default path. - Windows and Linux runtime behavior — not tested on either side.
No blocking findings. Approval blockers: none.
Reviewed with AI assistance.
| error instanceof Error ? error.message : String(error); | ||
| const capacityRejected = isAcpChildCapacityError(error); | ||
| const message = capacityRejected | ||
| ? getTranslator('en')('daemon.capacity.exhausted') |
There was a problem hiding this comment.
Minor: getTranslator('en') hardcodes English regardless of user language. The Chinese translation for daemon.capacity.exhausted was added in i18n.tsx but is never reached via this path.
The leak is wider than this line: connection.error is set to the English string, re-rendered verbatim in ChatPane.tsx, and re-wrapped as a plain Error in App.tsx — so isAcpChildCapacityError returns false on that re-report path and the localized message this PR added is bypassed on exactly the automatic-load path it targets.
Content is accurate; this is cosmetic. If the provider genuinely cannot read the I18nProvider context, a one-line comment would save the next reader from treating it as an oversight.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM — no blocking issues at 41a09ac; first human review pass. The mechanism is sound where it matters: reserve() → decide(committedProcessCount) is synchronous with no await between, so racing admissions serialize through the shared registry; the budget source is the existing cgroup-aware resolver with consistent MB units; rejection is a typed 503 acp_child_capacity_exhausted with deliberate Retry-After suppression; reservations are cancelled on refusal and the ledger release on child death reuses the pre-existing registry teardown.
Two P2s for follow-ups: (1) a terminating child still counts until registry release, so at the modeled limit a workspace whose child just crashed gets 503 on its recovery spawn during the teardown window — worth a grace slot; (2) the budget and slot count are resolved once at daemon startup and never re-read, so a cgroup limit change on a long-lived daemon silently widens protection until restart — the docs added here should say limits changes require a restart.
4 participants
What this PR does
Adds opt-in
qwen serve --child-heap-mode admit, using the existing initialized budget to limit concurrent managed ACP children. Reservations and terminating children count until release; workspace registration and reuse of an existing child do not consume another slot. REST and ACP return a recognizable capacity error, including through verified standalone rollback. Web Shell stops automatic capacity retries and preserves drafts, including the first/goalcommand. Status distinguishes enforced process admission from advisory heap sizing.Why it's needed
The current
observepolicy reports a modeled limit but permits starts beyond it. This implements stage 1 of #11907 so operators can enforce predictable process concurrency and users receive actionable feedback when a cold workspace cannot start.Reviewer Test Plan
How to verify
Start an isolated daemon with two workspaces,
--memory-budget-mb 1024and--child-heap-mode admiton a host that models one slot. Keep the primary ACP resident, then create or restore a session in the cold secondary. Expect REST 503 withacp_child_capacity_exhausted, or equivalent ACP error metadata, withoutRetry-After. Existing-child sessions and workspace registration should still work. Release the tracked child and deliberately retry; the secondary should start in its own workspace.In Web Shell, submit a first message in that cold workspace. Expect a capacity notice, retained text, an enabled send button and no automatic repeated creation. Also verify
/goaland shell drafts are retained on creation rejection. Repeat the daemon comparison withobserve: the extra child should still be allowed, and child heap arguments should match both modes.Evidence (Before & After)
Before: global Qwen Code 0.22.3 with one modeled slot started two ACP children under
observe;admitwas rejected by the parser. After: isolated bundle tests passed 33 admission checks; cold create, runtime ensure and persisted-session load returned capacity 503, ACP SSE carried equivalent metadata, refused reservations were released, and manual retry succeeded after a test-owned child exited. Both modes retained--max-old-space-size=16384 --expose-gcon this host.Real Chromium verification passed 10 checks: the capacity toast was visible, the exact draft remained after 40 seconds, and submission caused one create request with no subsequent automatic create/load/ensure requests. Component tests cover Chinese notices, goal/shell draft preservation and readable automatic-load errors.
Tested on
Environment (optional)
macOS, Node 22.22.3, locally built CLI/Web Shell and cached Playwright Chromium. Daemons used isolated temporary directories, fake credentials and no real model requests. Build, typecheck, changed-code formatting/lint and focused tests passed. The complete server suite passed 1,278 tests with a test-only IPv6 request-address correction: ordinary runs could hit unrelated local IPv4 listeners because Supertest listens on
::but requests127.0.0.1. No production or global network workaround is included. Web component tests disabled Node's experimental Web Storage for jsdom.Risk & Scope
observe; opt in with--child-heap-mode admit. No new environment variables. A zero-slot admission configuration and an injected/unwired admitting bridge are rejected at startup. PR2 and PR3 remain separate follow-ups.Synchronized design: English and 简体中文.
Linked Issues
Implements stage 1 of #11907. Related to #8182; background: #11386. Does not close the three-stage tracker or the fixed-heap enforcement issue.
中文说明
What this PR does
新增显式开启的
qwen serve --child-heap-mode admit，复用已有初始化预算限制托管 ACP 子进程并发数量。预留启动和终止中的进程在登记表释放前继续计数；注册 workspace 和复用已有子进程不新增名额。REST/ACP 返回可识别的容量错误，并在已验证的 standalone 回滚中保留容量原因。Web Shell 停止自动容量重试并保留草稿，包括首次/goal命令。状态接口区分实际执行的数量准入和仅供参考的堆模型。Why it's needed
当前
observe策略只报告模型上限，仍允许超额启动。本 PR 实现 #11907 的第一阶段，让运维可以限制进程并发，并在冷 workspace 无法启动时向用户提供可操作的提示。Reviewer Test Plan
How to verify
在可计算出一个名额的宿主上，用两个 workspace、
--memory-budget-mb 1024和--child-heap-mode admit启动隔离 daemon。保持 primary ACP 常驻，再在冷 secondary 中创建或恢复会话。应收到 REST 503 和acp_child_capacity_exhausted，或等价 ACP 错误元数据，不包含Retry-After。已有子进程中的会话和 workspace 注册仍应可用。释放被跟踪子进程后主动重试，secondary 应在自己的 workspace 中启动。在 Web Shell 向该冷 workspace 提交首条消息，应看到容量提示、保留的输入、可用的发送按钮，且不会自动重复创建。另验证
/goal和 shell 草稿在创建失败后保留。使用observe重复 daemon 对照：仍应允许额外子进程，且两种模式的子进程堆参数一致。Evidence (Before & After)
变更前：全局 Qwen Code 0.22.3 在模型只有一个名额时，
observe仍启动了两个 ACP；解析器拒绝admit。变更后：隔离 bundle 的 33 项准入检查通过；冷创建、runtime ensure 和持久化会话加载均返回容量 503，ACP SSE 携带等价元数据，拒绝预留得到释放，测试自有子进程退出后主动重试成功。两种模式在该宿主上都保留--max-old-space-size=16384 --expose-gc。真实 Chromium 验证 10 项通过：容量 toast 可见，40 秒后输入完全保留，提交只产生一次创建请求，随后没有自动 create/load/ensure 请求。组件测试覆盖中文提示、goal/shell 草稿保留和自动加载的可读错误。
Tested on
Environment (optional)
macOS、Node 22.22.3、本地构建的 CLI/Web Shell 和已缓存的 Playwright Chromium。Daemon 使用隔离临时目录、虚假凭证，不调用真实模型。构建、类型检查、变更代码格式/lint 和定向测试通过。完整服务端测试在仅用于测试的 IPv6 请求地址修正下通过 1,278 项：普通运行中，Supertest 监听
::却请求127.0.0.1，可能误入本机无关 IPv4 服务。本 PR 不包含生产或全局网络绕过。Web 组件测试为 jsdom 关闭了 Node 的实验性 Web Storage。Risk & Scope
observe，通过--child-heap-mode admit显式开启。不新增环境变量。零名额的准入配置，以及注入或未接入共享准入的 bridge，会在启动时被拒绝。PR2 和 PR3 留作独立后续工作。完整同步的设计文档：English 与 简体中文。
Linked Issues
实现 #11907 第一阶段。关联 #8182，背景为 #11386。不关闭三阶段 tracker 或固定堆限额 issue。