Review feedback — round 1

Resolved 3/3 inline threads.

Review feedback — round 2 (wenshao)

Resolved 3/3 threads.

✅ Local verification — daemon connection stress test + perf-harness refactor (Linux)

Verified on the true post-merge state (the PR is rebased on current daemon_mode_b_main — git rev-list --left-right --count origin/daemon_mode_b_main...HEAD = 0 3, so the head already is the merge result). The PR touches only integration-tests/ (10 files); packages/{cli,core,sdk-typescript} source is unchanged, so the daemon under test is the existing one. The author's matrix lists macOS only — this run adds Linux (x64) coverage.

Built the full daemon chain for the harness (core → cli → sdk-typescript → web-templates → esbuild bundle): 0 TypeScript errors across the whole graph; the bundled daemon (dist/cli.js, which globalSetup.ts wires in via TEST_CLI_PATH) boots and listens.

1. New loadtest suite — 5/5 scenarios PASS ✅

QWEN_LOADTEST_ENABLED=1 vitest run --config integration-tests/vitest.loadtest.config.ts (real daemon + mock-ACP child, no model):

Test Files 1 passed · Tests 5 passed, 67.7s total. The daemon's HTTP/SSE surface survives the lifecycle churn, backpressure flood, reconnect, and a crash-on-prompt child without wedging.

2. Typecheck (tsc --noEmit over all integration-tests) — clean for the PR ✅

0 errors in all 10 changed files (incl. mock-acp-typecheck.test.ts, whose whole point is the compile-time Agent-interface assertion). The only diagnostic emitted is tsconfig.json(8,13) TS5063 for the "//" paths-comment — and that file is byte-identical to base (not in this PR's changeset), so it's pre-existing.

3. Refactor safety net — baseline test: 6 passed, 1 skipped, 1 pre-existing env failure ✅

vitest run --root ./integration-tests cli/qwen-serve-baseline.test.ts (the test slimmed by the helper extraction):

• ✅ 6 passed — RSS scaling, attach latency, MCP grandchildren count as sessions grow, 2× SSE backpressure unit, etc. These exercise the extracted _daemon-perf-report.ts + _daemon-harness.ts helpers, confirming the extraction is behavior-preserving.
• ↓ 1 skipped — prompt-latency (no model-credential env), expected.
• × 1 failed — MCP child amplification > clientCount matches external pgrep observation (expected 4 MCP grandchildren, the sandbox's external pgrep saw 2). This is not a PR regression: the failing test is at line 480, which lies outside every diff hunk (hunks touch only lines 33–228 and 704), i.e. its code is byte-unchanged by this PR. It is an environment-sensitive process-tree observation test (pgrep visibility of spawned MCP children under a root/container sandbox); its sibling subtest in the same block passed.

Methodology note

act/real CI is not reproducible locally; this exercises the real built daemon + mock-ACP child end-to-end. The _daemon-benchmark-helpers.ts extraction (the /usr/bin/time wrappers used only by the benchmark suite) typechecks clean and follows the same extraction pattern as the two runtime-validated modules; the full benchmark run is cold-start-bound and did not complete within the time budget in this sandbox (a property of the benchmark, not the PR).

Verdict

✅ Verified. The new mock-ACP loadtest harness works end-to-end (5/5 on Linux), the perf-harness refactor is behavior-preserving (6/6 runnable baseline assertions pass; the lone failure is a pre-existing, env-sensitive pgrep test byte-unchanged by this PR), and all 10 changed files typecheck clean. No blockers from a runtime-verification standpoint.

🔍 Local verification report — real build + daemon integration/load tests

中文 TL;DR: 在本地用 tmux，基于 PR 的真实合并结果（PR 已是 base 分支 daemon_mode_b_main 最新态，0 落后）做了「构建 + 重构回归 + 真实跑 loadtest」验证。结论：重构是忠实的（benchmark 装上 /usr/bin/time 后 10/10 全过、typecheck 过、baseline 除一个既有环境问题外全过），新的 loadtest 框架 5 个场景过了 4 个。唯一一个真问题：SSE 慢消费者驱逐场景在本 Linux 沙箱里确定性失败（连跑两次都 evicted=false）。 由于整套 loadtest 默认被 CI 排除、且本 PR 不碰任何生产代码，可以合并；但建议把那个 SSE 场景的“灌流”改成并发再依赖它。其余两个失败（benchmark 的 /usr/bin/time ENOENT、baseline 的 pgrep MCP 计数）都已证明是环境问题、与本 PR 无关。

Verdict: ✅ Mergeable as test-infra. The refactoring is faithful and the harness mostly works; zero production code is touched and the whole load-test suite is excluded from default CI. One new scenario (SSE eviction) needs hardening to be reliable cross-platform — details + a suggested fix below.

Environment

• Base is daemon_mode_b_main. PR e1b0df4e0 is 0 commits behind base 82879b300 → PR head is the merge result.
• Linux (kernel 6.12), Node v22.22.2 (author validated on macOS / Node v24). Built CI-style (npm install + npm run build, exit 0); daemon binary dist/cli.js produced. All via tmux.

Results

Loadtest scenario breakdown:

✓ rapid lifecycle: create-prompt-close cycles        3171ms
× SSE slow consumer triggers eviction through HTTP   61002ms   ← deterministic fail here
✓ Last-Event-ID reconnect under concurrent load      1389ms
✓ ACP child crash → session error recovery           1081ms
✓ burst: concurrent sessions with mock prompts       1080ms

Two failures that are environmental, not this PR (both proven)

1. Benchmark — spawn /usr/bin/time ENOENT. GNU time isn't installed in this sandbox; the benchmark (and its extracted _daemon-benchmark-helpers.ts) wraps /usr/bin/time -v. After apt-get install time, the benchmark passes 10/10 → the helper extraction is correct.
2. Baseline — clientCount matches external pgrep observation. Timed out waiting for 4 MCP grandchildren … mcpGrandchildren=[2 pids]. The PR does not touch this test, and it fails identically on the clean base branch (no PR) — the sandbox only spawns 2 MCP grandchildren, not 4. Pre-existing/environmental.

The one real finding — SSE eviction scenario is environment-sensitive

SSE slow consumer triggers eviction through HTTP fails at expect(evicted).toBe(true) — the slow consumer is never evicted within 60s in this environment. I re-ran it twice in isolation; it failed both times → deterministic here, not flaky.

Likely cause: the flood fires prompts sequentially awaited:

for (let i = 0; i < promptCount; i++) {
  await d.client.prompt(session.sessionId, { prompt: [{ type: 'text', text: `flood-${i}` }] });
}

Awaiting each prompt throttles event production to roughly the slow consumer's drain rate (consumerDelayMs: 200), so the 16-deep queue doesn't reliably overflow → the daemon's queue-overflow eviction doesn't trip on this Linux/Node-22 host (it presumably did on the author's macOS/Node-24). The daemon eviction logic itself is base code and isn't exercised to failure here — the scenario just doesn't generate enough backpressure.

Suggested fix: make the flood genuinely concurrent so it outpaces the consumer regardless of host timing, e.g. fire without awaiting and then settle:

await Promise.allSettled(
  Array.from({ length: promptCount }, (_, i) =>
    d.client.prompt(session.sessionId, { prompt: [{ type: 'text', text: `flood-${i}` }] })),
);

This isn't a CI blocker (the suite is gated behind QWEN_LOADTEST_ENABLED=1 and excluded from default config), but worth hardening before this scenario is relied on in cron/CI.

Refactoring soundness

The ~500-line extraction into _daemon-benchmark-helpers.ts / _daemon-perf-report.ts / harness exports is faithful: benchmark passes 10/10 (with /usr/bin/time), mock-acp-typecheck passes, and the baseline test's only failure reproduces identically on base. The new vitest.loadtest.config.ts is correctly isolated (10-min timeout, fileParallelism:false, SDK alias) and the default config correctly excludes the loadtest.

Scope: build + refactor regression + live load-test on Linux/Node-22 against the real merge result. No production code is modified by this PR. Not covered: macOS/Node-24 (author's platform) where the SSE-eviction scenario reportedly passes.

Review feedback — round 3 (wenshao R2)

Fixed: 2 | Pushed back: 5 | Commit: 44dcc98