refactor(cli): Split serve server routes - #5809
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Thanks for the PR! Template looks good ✓ On direction: this directly addresses #5576 — the 5715-line On approach: the scope feels right for a first-stage split. The extracted boundaries are cohesive — each module owns a single concern, Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 方向：直接解决 #5576 — 5715 行的 方案：作为第一阶段拆分，范围合理。抽取的边界内聚——每个模块只负责一个关注点， 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
2a. Code ReviewIndependent proposal (before reading the diff): to split a 5715-line Comparison: the PR's approach matches this exactly. Reuse check: Correctness: no bugs found. Error taxonomy lives in one place ( Minor scope creep (non-blocking): 2b. Verification (re-run 2026-06-26, PR HEAD
|
| Module | Route | Response |
|---|---|---|
| health-demo | GET /health |
200 {"status":"ok"} |
| (inline) | GET /capabilities |
200 full feature list |
| daemon-status | GET /daemon/status?detail=summary |
200 {"v":1,"detail":"summary","status":"ok","issues":[]} |
| session-list | GET /workspace/:id/sessions |
200 {"sessions":[]} |
| workspace-auth | GET /workspace/auth/status |
200 {"v":1,"providers":[],"pendingDeviceFlows":[]} |
| session | POST /session (real session) |
200 {"sessionId":"cd9612b6-...","workspaceCwd":"/tmp/triage-ws-5809"} |
| error-response | GET /nonexistent |
404 HTML error page |
| permission | POST /permission/fake-id bad body |
400 {"error":"outcome must be..."} |
Daemon telemetry log confirms all routes served:
[DAEMON] route=GET /capabilities durationMs=3 status=200 request completed
[DAEMON] route=GET /daemon/status durationMs=1 status=200 request completed
[DAEMON] route=GET /workspace/%2Ftmp%2Ftriage-ws-5809/sessions durationMs=2 status=200 request completed
[DAEMON] route=POST /session durationMs=41 status=200 request completed
[DAEMON] route=GET /workspace/auth/status durationMs=0 status=200 request completed
[DAEMON] route=POST /permission/fake-id durationMs=1 status=400 request completed
Verdict: identical behavior to main — same startup sequence, same middleware stack, same response shapes. All extracted daemon routes respond correctly.
— Qwen Code · qwen3.7-max
Reflection (re-verified 2026-06-26)Re-running triage on current HEAD
My independent proposal was: extract into The two drive-by formatting changes ( Approval guardrail: this is a cross-repository (fork) PR with a @doudouOUC — thanks for the thorough work on this split. The design doc, test plan, and preserved middleware ordering make this straightforward to review. Escalating to a maintainer for the final approve/reject call. 中文说明反思（2026-06-26 重新验证）在当前 HEAD
我的独立方案是：抽取到 两处顺手格式化改动（ 审批护栏： 这是来自 fork 的 @doudouOUC — 感谢在这个拆分上的细致工作。设计文档、测试计划和保留的 middleware 顺序让审查变得直接。已转交维护者做最终 approve/reject 决定。 — Qwen Code · qwen3.7-max |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Clean mechanical extraction of the server.ts monolith into focused modules. All 770 tests pass (518 server + 251 route/integration), build succeeds, typecheck and ESLint clean. Nine review agents (correctness, security, code quality, performance, test coverage, attacker/oncall/maintainer personas) plus a reverse audit found zero behavioral regressions — middleware ordering, closure captures, re-exports, class identity, and all input validation are byte-for-byte preserved.
Downgraded from Approve to Comment: CI failing (CodeQL).
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
a6af3fb to
af2fc29
Compare
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Verified behavior-faithful mechanical extraction — no functional findings.
What I checked: byte-level comparison of every extracted function against the pre-split server.ts; middleware + route registration order (matches the design doc's required sequence, verified by reconstructing the full ordered route list for both revisions); error taxonomy / status codes / headers; validators and length caps; the SSE activeSseCount shared counter (single instance, not forked); and the lazy getter wiring (getAcpHandle / getRateLimiter / currentServeFeatures invoked per-request, so deps assigned after registration still resolve). Deterministic checks pass first-hand on HEAD ad4f097: tsc --noEmit clean, and server.test.ts + acp-http/routes suites = 769 tests green.
The latest commit also cleanly resolves the two earlier review comments — SendBridgeError is now a single exported type in error-response.ts (replacing the five duplicated local aliases + the inline one in server.ts), and the duplicated buildWorkspaceCtx closure is consolidated into makeBuildWorkspaceCtx. Both are behavior-preserving.
One minor non-blocking follow-up (out-of-diff, optional): now that CLIENT_ID_RE / MAX_CLIENT_ID_LENGTH moved out of server.ts into server/request-helpers.ts, the // Keep in sync with server.ts CLIENT_ID_RE / MAX_CLIENT_ID_LENGTH breadcrumb in packages/cli/src/serve/rate-limit.ts:95 now points at a file where those constants no longer live — worth repointing to request-helpers.ts. The duplicated values themselves are unchanged, so there's no behavioral drift today.
— claude-opus-4-8[1m] via Qwen Code /qreview
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Clean structural refactor — behavioral equivalence verified across all 60+ routes, middleware ordering, SSE lifecycle, error taxonomy, and auth gates. 769 tests pass, typecheck and lint clean. No issues found. Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
|
I checked the two CodeQL One caveat: rate limiting remains an opt-in daemon setting. If the project policy requires this auth-provider install route to be rate-limited even when global rate limiting is disabled, that would be a broader hardening change rather than a regression from this refactor. Otherwise, I think these GHAS comments can be treated as CodeQL not recognizing the global configurable middleware and should be triaged by maintainers/security. 中文说明我核对了两个 CodeQL 需要注意的是，限流本身仍是 daemon 的可选配置。如果项目策略要求该 auth-provider install 路由即使在未开启全局限流时也必须限流，那属于更大的安全加固决策，而不是本次重构引入的回归。否则我认为这两个 GHAS 评论可以按 CodeQL 未识别全局可配置中间件来处理，由维护者/security 做 triage。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
Summary: Clean structural refactor that extracts a 5700-line server.ts into 16 focused modules. Behavioral equivalence verified — middleware ordering, error contracts, SSE framing, and auth gates all preserved. Build passes, all 518 serve tests pass, typecheck and lint clean.
Needs Human Review (low confidence):
workspace-extensions.ts:safeBodyreceived via deps while sibling modules import it directly (minor DI inconsistency)workspace-agents.ts:687-696: stale comment referencingsafeLogValuelocation and visibility (not in diff, oversight during refactor)
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
✅ Local real-daemon verification — PR #5809Verdict: structural-only refactor, behavior preserved end-to-end — safe to merge. I built the real 1. Build & checks (PR's claimed commands)
The newly-extracted route groups (daemon-status, session, sse-events, workspace-auth/-status/-extensions, permission) have no dedicated unit tests — their behavior is covered by 2. Refactor invariants — statically verified (base ↔ PR)
3. Real daemon e2e (tmux) — every split module answers
4. Base-vs-PR HTTP A/B — 18/18 routes byte-identicalSame probe script run against a base-commit daemon (merge-base Every route returns an identical normalized body and identical status code on both binaries. (An earlier uncontrolled run showed 5 "diffs" that were all noise — memory RSS, 5. SSE contract preserved
Notes (non-blocking)
Method / environmentmacOS (darwin), Node v22.22.2. Two isolated worktrees built independently: PR head 🇨🇳 中文版（完整对应）✅ 本地真实 daemon 验证 —— PR #5809结论：纯结构重构，行为端到端保持不变 —— 可以合并。 我在隔离 worktree 中基于 PR head（ 1. 构建 & 检查（PR 声称的命令）
新抽取的 route 组（daemon-status、session、sse-events、workspace-auth/-status/-extensions、permission）没有独立单测——它们的行为由 2. 重构不变量 —— 静态核验（base ↔ PR）
3. 真实 daemon e2e（tmux）—— 每个拆分模块都响应loopback 上
4. Base vs PR 的 HTTP A/B —— 18/18 路由逐字节一致同一份探测脚本分别打到 base 提交的 daemon（merge-base 每个路由在两个二进制上都返回一致的归一化 body 且一致的状态码。（更早一次未受控的运行出现 5 个"diff"，全是噪音——内存 RSS、 5. SSE 契约保留对一个 live session 打 说明（不阻塞）
方法 / 环境macOS (darwin)，Node v22.22.2。两个隔离 worktree 各自独立构建：PR head |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
Clean structural refactor — behavioral equivalence verified across middleware ordering, error handling, auth gates, SSE lifecycle, and all 769 tests (518 server + 251 route/integration). Build, typecheck, and ESLint all pass. Nine review agents (correctness, security, code quality, performance, test coverage, three adversarial personas, build/test verification) found no high-confidence issues. The extraction faithfully preserves the original behavior.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Reviewed as a structural-only split — spot-checked the spots where this kind of extraction usually breaks:
- The consolidated
safeLogValue/CLIENT_ID_RE/MAX_CLIENT_ID_LENGTHinrequest-helpers.tsmatch the originals exactly, so the de-duplication removes the old "keep in sync" hazard without drift. - Every
register*Routes(app, …)is wired, andhealth-demois still mounted exactly once via the pre/post-auth branch — no dropped or double-registered routes. - No extracted route file adds a global
app.use, so the middleware stack composed inserver.tsis unchanged.
Ubuntu unit tests (server + routes) are green. LGTM.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
Clean structural refactor — behavioral equivalence verified across middleware ordering, error handling, auth gates, SSE lifecycle, and all 845 tests (529 server + 316 route). Typecheck and lint clean. 9 parallel review agents (correctness, security, code quality, performance, test coverage, attacker/oncall/maintainer audit) + deterministic analysis + reverse audit confirmed no new issues introduced by the split.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
Clean structural refactor — behavioral equivalence verified across middleware ordering, error handling, auth gates, SSE lifecycle, and all 845 tests (529 server + 316 route). Typecheck and lint clean.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
Clean structural refactor — behavioral equivalence verified across middleware ordering, error handling, auth gates, SSE lifecycle, and all 845 tests (529 server + 316 route). Typecheck and eslint clean. 9 parallel review agents + deterministic analysis + reverse audit found no issues.
— qwen3.7-max via Qwen Code /review
Round-6 review (qwen-code-ci-bot), all additive / no behavioural side effects: - [Critical] Unbounded SSE buffer in the new AcpHttpTransport session-stream parser → OOM (tab crash for browser consumers). Add a 16 MiB cap mirroring parseSseStream's MAX_BUF_CHARS, and reuse parseSseStream's CRLF-aware `consumeFrames` splitter (now exported) instead of an inline `\n\n` scan — closing the CRLF, multi-line `data:` join, and trailing-CR gaps in one go. - Deferred-flush ordering race: the pump's post-loop safety flushDeferred() now runs only on a non-aborted exit. An abort means the stream was detached/reclaimed; flushing there could drain the deferred reply onto a reclaiming stream ahead of its own replay (reintroducing the out-of-order delivery the deferral prevents). On error the frames stay buffered for the next attach — never lost. - Grace reclaim now logs (detach + grace-expiry already did) so the reconnect trail is complete for operators. - sse-last-event-id doc: corrected the "shared by REST and ACP" claim — after the QwenLM#5809 serve-route split REST keeps its own copy; unifying them would touch REST, so it's deferred (this PR keeps REST untouched). Thread on a full deferred-flush integration test: the ordering invariant is already locked at the unit layer (flushBufferedSessionFrames defer test + gap-delivery test); a full-HTTP timing test against the FakeBridge would be flake-prone, so it's intentionally not added. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…-in SDK transports export (QwenLM#5852) * fix(daemon): resume /acp session stream via Last-Event-ID (recover mid-turn content) The `/acp` Streamable-HTTP session event stream was live-only: it emitted no SSE `id:` sequence and ignored a `Last-Event-ID` reconnect header. When a control-plane proxy idle-closed the long-lived SSE mid-turn, every content frame the daemon produced during the gap (`session/update` carrying agent_thought_chunk / agent_message_chunk) was lost — the turn still settled, so the UI showed "done" with an empty/truncated body, and only a re-send recovered it (tracked as §1.8 in the integration notes). The replay engine already exists and is battle-tested on the REST surface: EventBus assigns a monotonic per-session `id`, keeps a bounded ring, and `subscribeEvents({ lastEventId })` replays `id > lastEventId` before live events flow. This wires the `/acp` transport to it — no eventBus/bridge change. - transport-stream / sse-stream / ws-stream: `send(message, id?)`. SSE emits an `id:` line when `id` is present (mirrors REST `formatSseFrame`); WS ignores it (stateful, no replay). - connection-registry: `sendSession(…, id?)` threads the cursor; the pre-attach session buffer stores `{ frame, id? }` so a buffered frame keeps its `id:`. - dispatch: `translateEvent` passes `event.id` for bus events; `pumpSessionEvents` forwards `lastEventId` to `subscribeEvents`. - index: the `GET /acp` session branch reads `Last-Event-ID` (strict decimal-only parse, same rule as REST) and passes it to the pump. Bus-originated frames (session/update, request_permission, daemon notifies) carry an `id:`; JSON-RPC responses and synthetic terminal frames do not, so they don't burn a slot in the resume sequence. Backward compatible: clients that send no `Last-Event-ID` get live-only behaviour as before, and `id:` lines are inert for clients that ignore them. Design: docs/design/daemon-acp-http/sse-resumable-stream.md * fix(daemon): make /acp resume actually engage — session-stream grace/reclaim + replay guards Addresses three review Criticals on the §1.8 plumbing: on its own the `id:`/`Last-Event-ID` wiring never fired in the real close-then-reconnect flow, and once it does fire two replay-correctness gaps become reachable. 1. Session-stream grace/reclaim (the core fix). A transport-level session-stream close used to run the FULL `closeSessionStream` teardown — removing ownership, aborting the in-flight prompt, detaching the bridge client. In the real EventSource/proxy order (old socket closes first, then reconnect) that meant the reconnect carrying `Last-Event-ID` was rejected 403 before the cursor was read, and the prompt was already aborted — so replay had nothing to resume. Now a transport close DETACHES (`detachSessionStream`): it stops only the stream + subscription and keeps the binding, ownership, prompt, and bridge-client alive for a grace window (`SESSION_GRACE_MS`, mirrors `CONN_GRACE_MS`). A reconnect within the window reclaims (clears the timer); otherwise the grace timer runs the full teardown, bounding runaway cost. Full teardown stays immediate for explicit `session/close` and connection destroy. The GET handler branches on `stream.isClosed` (transport close → grace; pump-ended-while-open → full close). 2. No double-delivery (buffer ↔ ring overlap). `attachSessionStream` records the max bus id flushed from the pre-attach buffer; the GET handler advances the replay cursor to `max(Last-Event-ID, lastFlushedEventId)` so the ring replay doesn't re-emit an already-flushed frame. 3. Idempotent `permission_request` under replay. `translateEvent` reuses the existing `conn.pending` entry for a `bridgeRequestId` (re-sends the same outbound id) instead of minting a second id+entry — no orphan pending, no duplicate prompt on a ring-replayed permission. Also: extract `parseLastEventId` to a shared `serve/sse-last-event-id.ts` used by both REST and `/acp` (no drift; logs the rejected value); log `lastEventId` in the pump error. Tests: real close-then-reconnect order (200 not 403 + prompt not aborted); overflow Last-Event-ID; replayed permission reuses pending id; registry grace/reclaim + buffer-flush-preserves-id. Full acp-http suite green (216). * feat(sdk): expose ACP transports via opt-in ./daemon/transports subpath The resumable ACP-over-HTTP transport (AcpHttpTransport, native supportsReplay + Last-Event-ID) and the negotiateTransport factory were reachable only from source paths inside the monorepo — the published `@qwen-code/sdk/daemon` barrel intentionally omits them to keep its budget-checked browser bundle lean, so external consumers (agent-web) had no import path short of forking. Add a separate opt-in subpath `@qwen-code/sdk/daemon/transports` that ships AcpHttpTransport / AcpWsTransport / AutoReconnectTransport / RestSseTransport / negotiateTransport as their own browser+node bundle. The default `./daemon` barrel and its byte budget are unchanged, so REST-only consumers stay tree-shaken and pay nothing for the transports. Also add a `fetchFn` option to NegotiateTransportOptions so callers can inject auth/proxy/test fetch instead of the hardcoded global. - build.js: emit dist/daemon/transports.{js,cjs}; reuse the node-builtin guard for the new browser bundle (no size budget — it legitimately ships the transports) while keeping the default barrel's budget check. - daemon/index.ts: update the rationale comment to point at the subpath. - daemon-transports-surface.test.ts: lock the runtime + type surface and the package.json exports entry. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(daemon): resume cursor must not skip in-flight-lost frames Round-3 review (qwen-code-ci-bot) flagged a silent-frame-loss Critical in the §1.8 resume path I added: `resumeCursor = max(Last-Event-ID, lastFlushedEventId)` advances the ring-replay cursor past the buffer, but a frame sent to the now-dead socket yet never received by the client has a bus id BELOW the buffer's ids and ABOVE the client's cursor — so the max() skips it and the ring replay never re-emits it. Exactly the proxy idle-close mid-turn frame §1.8 is meant to recover. Fix without trading loss for duplicates: a buffered bus event is ALSO in the EventBus ring (it was published there to get its id), so the ring replay started at the client's cursor is the single delivery path for every bus event after the cursor. `attachSessionStream` now takes the resume cursor and, when resuming, does NOT flush id-bearing buffered frames — the ring owns them, delivering each exactly once including the in-flight-lost frame. Id-less frames (JSON-RPC replies via `replySession`, not ring events) are still flushed — their only delivery path. The GET handler sets `resumeCursor = lastEventId` verbatim; `lastFlushedEventId` is removed. Also from the same review: - sse-last-event-id `safeLogValue`: strip ALL C0 control chars + DEL (not just CR/LF) so a crafted `Last-Event-ID` can't smuggle ANSI ESC / null bytes onto an operator's terminal via stderr. - ws-stream: regression test asserting `send(msg, id)` keeps the WS wire frame bare JSON (no SSE `id:` framing leak). - connection-registry: resume-path test (id-bearing frames skipped, id-less reply still flushed); design doc updated. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * refactor(daemon): inline resumeCursor alias to lastEventId Review nit (yiliang114): after the prior commit dropped the `max()` logic, `resumeCursor` is a pure alias for `lastEventId`. Use `lastEventId` directly in the `pumpSessionEvents` call and the error log; drop the alias. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(daemon): refresh stale resume comments + add gap-delivery test Round-4 review (qwen-code-ci-bot, against the now-corrected resume model): - connection-registry: the attachSessionStream CONTRACT comment still cited a `promptAbort?.abort()` call in the index.ts onClose handler that an earlier commit removed. Rewrite it to describe the current model — each stream's pump has its own abort controller and teardown is identity-guarded in `onPumpSettled`, so installing the new stream first makes the old stream settle into detach-with-grace rather than tearing down the in-flight prompt. - dispatch: the stream_error frame comment ("no bus id, so no SSE id: line") contradicted the code passing `event.id`. Make it truthful: pass the cursor through if present; a synthetic terminal frame has no id so none is written. - connection-registry.test: add the explicit detach → produce gap events → reattach → flush-exactly-once test (the PR's core value prop at the registry layer), incl. a second reattach asserting the buffer drained. The two Criticals in the same review referenced `resumeCursor` / `lastFlushedEventId` / `Math.max`, all removed in prior commits — obsolete against current code (answered + resolved on the threads). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(daemon): preserve stream order on resume + harden grace/permission paths Round-5 review (wenshao + qwen-code-ci-bot): - [Critical, wenshao] Out-of-order completion on resume. attachSessionStream flushed id-less buffered JSON-RPC replies (e.g. a session/prompt result that landed during the detach gap) immediately — ahead of the ring replay that redelivers the content chunks preceding them, so a client could see "prompt complete" before the body (the truncated-body failure §1.8 fixes). Now on resume those id-less frames are DEFERRED in the buffer; the event pump releases them via flushBufferedSessionFrames once the replay boundary (replay_complete / state_resync_required) passes, preserving original order. Fresh connects (no cursor, no replay) still flush the whole buffer in order. - [Critical, ci-bot] Permission auto-denied during the reconnect grace window: a permission_request arriving while binding.stream is detached cancel-denies, so a client reconnecting within grace can't vote. The structural fix (defer the vote across grace) belongs with the §1.7 permission-coordination follow-up; here, log an operator breadcrumb when it fires during grace, and document the synchronous-translateEvent INVARIANT the direct binding.stream .send relies on. - [ci-bot] Stale-stream detach is now tested (reclaim installs s2; a late s1 close is a no-op — no teardown, no grace re-arm). Grace-expiry teardown now logs a breadcrumb so a vanished session is distinguishable from explicit close. TS4111: bracket-access the index-signature exports entry in the SDK surface test. transports browser bundle now has a size budget (MAX_TRANSPORTS_BROWSER_BUNDLE_BYTES = 48KB; current ~29KB). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): route session-scoped /acp responses so prompts don't hang [Critical, wenshao] The published AcpHttpTransport could hang real session/prompt + config requests. Its subscribeEvents() opened REST GET /session/:id/events and only sendRequest's connection-scoped stream resolved responses — but the daemon's replySession() routes session-scoped JSON-RPC replies onto the session-scoped /acp stream, which the transport never read. So a session/prompt reply was never observed → the pending request never settled. Switch subscribeEvents to the session-scoped /acp stream (GET /acp + Acp-Session-Id) — the resumable §1.8 stream the daemon puts session replies on — and dispatch each raw JSON-RPC frame by shape: - response (id, no method) → resolve the shared pending map (the fix) - notification (method, no id) → DaemonEvent via denormalizeAcpNotification, stamped with the real bus id from the SSE `id:` line (the synthetic denormalizer id is not resume-compatible; supportsReplay=true now tracks the authoritative cursor) - session/request_permission → surfaced as a permission_request event so consumers still see prompts (responding to the vote is the §1.7 follow-up) The connection-scoped stream still carries replies to connection-level requests (initialize, session/new). Adds 4 subscribeEvents unit tests (stream selection + headers, notification→event+busId, response consumed-not- yielded, permission surfaced). Full SDK suite green (1062). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk,daemon): harden /acp SSE parser + grace/flush observability Round-6 review (qwen-code-ci-bot), all additive / no behavioural side effects: - [Critical] Unbounded SSE buffer in the new AcpHttpTransport session-stream parser → OOM (tab crash for browser consumers). Add a 16 MiB cap mirroring parseSseStream's MAX_BUF_CHARS, and reuse parseSseStream's CRLF-aware `consumeFrames` splitter (now exported) instead of an inline `\n\n` scan — closing the CRLF, multi-line `data:` join, and trailing-CR gaps in one go. - Deferred-flush ordering race: the pump's post-loop safety flushDeferred() now runs only on a non-aborted exit. An abort means the stream was detached/reclaimed; flushing there could drain the deferred reply onto a reclaiming stream ahead of its own replay (reintroducing the out-of-order delivery the deferral prevents). On error the frames stay buffered for the next attach — never lost. - Grace reclaim now logs (detach + grace-expiry already did) so the reconnect trail is complete for operators. - sse-last-event-id doc: corrected the "shared by REST and ACP" claim — after the QwenLM#5809 serve-route split REST keeps its own copy; unifying them would touch REST, so it's deferred (this PR keeps REST untouched). Thread on a full deferred-flush integration test: the ordering invariant is already locked at the unit layer (flushBufferedSessionFrames defer test + gap-delivery test); a full-HTTP timing test against the FakeBridge would be flake-prone, so it's intentionally not added. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(daemon,sdk): harden /acp resumable stream from review round 7 Address review-pr findings on the §1.8 resumable stream, all additive / backward-compatible (REST untouched, no behavioural side effects): - connection-registry: guard flushBufferedSessionFrames against a closed stream so deferred replies stay buffered for the next reconnect instead of being dropped onto a dead socket. Keep the synchronous in-order enqueue (SseStream serializes via one writeChain) — an await-per-frame drain would let a live event interleave between deferred frames and reorder the very replies this deferral preserves (W1). - connection-registry: log at the moment of detach so an operator can measure the real disconnect→reconnect gap against the grace window. - index: route sessionId through logSafe() in the event-pump error log, matching every other log line this PR adds (terminal-escape hardening). - AcpHttpTransport: remove the abort listener in the finally block so a long-lived signal reused across reconnects doesn't accumulate listeners. - AcpHttpTransport: parse the SSE `id:` cursor with the server's strict /^\d+$/ + MAX_SAFE_INTEGER rule instead of lenient Number() (rejects proxy-mangled hex/exponential/empty cursors). - AcpHttpTransport: document that an unparseable non-empty data frame is a corrupt frame (not a heartbeat); tracing it is a follow-up once the SDK grows a logger (the package lint config forbids console). - tests: add sse-last-event-id.test.ts (parseLastEventId accept/reject + safeLogValue control-char stripping/truncation) and a flushBufferedSessionFrames closed-stream-retains-buffer case. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(daemon,sdk): round-8 review hardening for /acp resumable stream All additive / backward-compatible (REST untouched, no behavioural side effects): - AcpHttpTransport: attach a no-op catch to abortPromise so an already-aborted signal at entry (loop never enters, Promise.race never consumes the rejection) can't surface as an unhandled rejection. - AcpHttpTransport: document that opts.maxQueued does not apply to the /acp transport (the session stream is backed by the daemon's server-controlled EventBus ring; there is no client-tunable queue to forward it to) — intentionally ignored, not silently mis-applied. - index: run err.message through logSafe() in the event-pump error log (CR/LF/ANSI in a bridge error string would otherwise reach stderr raw), and add operator breadcrumbs for the previously-silent onPumpSettled branches (pump-ended-while-open full close; superseded-stream no-op), completing the detach/reclaim/grace trail. - tests: assert subscribeEvents writes Last-Event-ID on the outbound GET when resuming and omits it on a first connect (the resume cursor must reach the wire), plus an already-aborted-signal case that would fail on an unhandled rejection without the catch above. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(daemon): flush deferred /acp replies on replay_complete only The EventBus emits `state_resync_required` BEFORE the replay frames (the `epoch_reset` and `ring_evicted` paths both fall through to the replay loop and still emit `replay_complete` at the end). The pump was releasing the deferred id-less replies on EITHER boundary, so on a resync-triggering resume the buffered `session/prompt` result was flushed ahead of the replayed content chunks — the exact truncated-body reordering §1.8 fixes (client sees "done" before the body). Flush on `replay_complete` only. The live-only case (no cursor ⇒ no replay ⇒ no `replay_complete`) is still covered by the pump's post-loop safety flush. Add an over-the-wire integration test (resume with a reply buffered during the detach gap, bridge replays resync → content → replay_complete) asserting the reply lands AFTER the replayed content; verified it fails against the previous dual-boundary flush. Design doc updated. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(daemon): close two §1.8 grace/replay-ordering holes Both additive / in-scope / no REST change: - Replay-window reply ordering (connection-registry, dispatch): the resumptive-attach deferral only covered id-less replies ALREADY buffered from the detach gap. A prompt that finished AFTER the new stream attached but BEFORE replay drained went straight out live via `sendSession`, overtaking replay frames not yet sent. Add a per-binding `replayPending` flag (armed on resumptive attach, cleared on `replay_complete` in `flushBufferedSessionFrames`) and route `replySession`'s out-of-band replies through a new `sendSessionReply` that defers while it's set. In-band pump frames keep using `sendSession`, so the `replay_complete` frame itself can't be deferred (which would deadlock the release). - Connection reaper vs session grace (index, connection-registry): the conn-stream-close reaper treated only LIVE session streams as activity, so a session detached into its own `SESSION_GRACE_MS` window (stream undefined, graceTimer armed) didn't count — the connection could be reaped at `CONN_GRACE_MS`, 404-ing the imminent session resume and aborting the in-flight prompt early. Add `hasRecoverableSession()` and treat a grace-armed session as activity in the reaper guard. Unit tests for both at the registry layer. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): thread query params into ACP transport route extractors The exported ACP HTTP/WS transports reduced request URLs to `pathname` before the route table built JSON-RPC params, so every query parameter from the REST-style DaemonClient helpers was dropped — e.g. `readWorkspaceFile('a.ts', { maxBytes: 123 })` (`/file?path=a.ts&maxBytes=123`) produced `_qwen/file/read` with `params: {}`. Same for `/file/bytes`, `/stat`, `/list`, `/glob`, and `context-usage?detail=true`. Pass `parsedUrl.searchParams` into `extractParams` and coerce each query value to the type the daemon's ACP handlers require — the daemon validates `maxBytes`/`line`/`limit`/`offset` as real numbers and `detail` as the boolean `true`, neither of which a raw query string satisfies. Helpers `strParam`/`numParam`/`boolParam` keep the per-route extractors terse. `query` is optional so the existing path-only extractors are unaffected. (`/workspace/voice/transcribe` has no ACP route at all — separate gap, binary audio doesn't belong on the JSON-RPC transport.) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): route session-stream replies via a background pump (no-subscriber prompt) The daemon answers POST /session/:id/prompt (and session/cancel, set_config_option, set_mode, set_model) with 202 and routes the JSON-RPC result onto the SESSION stream via replySession — not the connection stream the transport pumps. So a DaemonClient that calls prompt() but never iterates subscribeEvents had nothing reading that reply, and sendRequest()'s pending promise never resolved → prompt() hung forever. For these session-reply methods, sendRequest now opens a reference-counted background session-reply pump (GET /acp + Acp-Session-Id) that routes JSON-RPC responses to `pending`, released when the request settles. It's suppressed when a subscribeEvents consumer is already iterating that session (tracked via activeSessionSubscriptions) — the daemon's session stream is single-reader, so a competing GET would detach the consumer's; in that case the consumer already routes the reply (the W2 fix). The pump skips notifications and permission requests (method-bearing frames) so a permission request id can't be mis-routed onto a pending response slot. All five methods require an owned session, so the pump's GET is always authorized. Disposed pumps are aborted in dispose(). Verified the new test times out without the pump and passes with it. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): sequence /acp deferred replies by bus watermark + harden grace/reap Address review on the §1.8 resumable-stream fixes: - replayPending is now set from the current attach mode every time (resume arms, fresh connect clears) so an aborted resume that skipped its boundary flush can't strand the flag and buffer every later reply forever (MsyIq, MylZ4). - Deferred out-of-band replies carry a watermark (anchorId = bus head at produce time) and release only once the pump delivers through that id, via per-event releaseDeferredSessionReplies + endReplayDeferral at replay_complete. A result produced during a slow replay no longer jumps ahead of tail content still flowing as live events behind the boundary (MsyIt). Unanchored fallback replies still release at the boundary. - Connection reap re-evaluates after a session reclaim grace expires (connGraceExpired + onSessionGraceExpired), so a conn blocked from reaping by a then-recoverable session no longer lingers to the 30-min idle sweep (MsyIs). - Wrap the grace-timer teardown in try/catch so a throwing detach callback can't crash the daemon from a bare setTimeout (MylZ8). - sse-last-event-id reuses the shared logSafe sanitizer (covers C1 + Unicode bidi) instead of a narrower divergent regex (M1isz); refresh stale replayPending/flush JSDoc (MselO). Unit tests cover the replayPending reset, watermark ordering, grace expiry hook, and grace-timer try/catch. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): scope pending sweeps per stream + reset bus cursor on invalid id Address review on the ACP HTTP transport: - Tag each pending request with its routing scope (connection vs a sessionId). A connection-stream failure now sweeps only conn-scoped pendings, so it can't reject a session/prompt the session stream is about to resolve; the session reply pump mirrors this for its own scope (MselM). - An invalid id: line later in an SSE frame resets the bus cursor to undefined rather than carrying a stale earlier value into the event (MselW). - Strengthen the W2 response-routing test to register a pending request and assert the frame RESOLVES it, not merely that it isn't yielded (MylZ-). Add tests for the per-stream sweep partition and the id reset. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): accept `kind`-tagged _qwen/notify envelopes (don't drop resume signals) The daemon's session-stream translateEvent stamps `_qwen/notify` events under `kind` (state_resync_required, replay_complete, stream_error, model_switched, …), but denormalizeAcpNotification read only `type` and returned undefined for them — so subscribeEvents silently dropped every such event. During a ring-overflow resume the SDK would never see state_resync_required and would apply replayed events to stale state. Read `params['type'] ?? params['kind']` (preferring `type`, so other producers are unaffected) and add an SDK test feeding a `kind`-tagged notify through subscribeEvents (M2bvl). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): reject session-scoped pendings when the subscription stream closes A `session/prompt` reply routed through an active subscribeEvents consumer (no reply pump is started while a subscription is live) would hang if that session SSE stream closed before the reply arrived: the connection-stream catch only sweeps conn-scoped pendings, and subscribeEventsInner's finally cleaned up the reader but never the pendings. Sweep session-scoped pendings in that finally too, gated so it only fires when this is the session's last delivery route (no other active subscription — the ref-count still includes self here — and no reply pump), mirroring the reply-pump and connection-stream sweeps. Add a regression test (M2iHz). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): harden ACP SSE readers + reply-pump handoff + empty query param Address the latest review wave on the transport: - pumpConnStream now bounds its unread SSE buffer with the same MAX_SSE_BUF_CHARS guard the two session readers already have — the OOM vector (a server that never emits a `\n\n` boundary) was open on 1 of 3 readers — and attaches the no-op abortPromise.catch() crash guard (M3BYQ). - pumpSessionReplies mirrors subscribeEventsInner's abort handling: named listener ref removed in finally (no leak on a clean drain of a reused signal) + abortPromise.catch() so a pre-aborted signal can't surface an unhandledrejection; and it throws the HTTP status on a non-OK response so the failure is diagnosable rather than a silent void return (M3BYT, M3BYY). - subscribeEvents aborts any existing background reply pump for the session before opening the consumer stream. The single-reader session stream detaches the pump anyway; aborting it skips its teardown sweep so it can't spuriously reject the very `session/prompt` the consumer now delivers (M3BYa). - numParam treats an empty value (`?maxBytes=`) as absent, not Number('')===0 (M3BYd). Tests: empty-numeric-param omission, and the reply-pump abort-on-subscribe handoff (pump aborted, its pending not rejected). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): log a breadcrumb when the replySession anchor is unavailable The getSessionLastEventId fallback (deferring a reply unanchored when the ACP binding briefly outlives the bridge session) was silent. Emit a scoped stderr breadcrumb so an operator can tell that benign teardown race apart from an unexpected bridge regression that starts exercising the fallback (M3BYf). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): validate content-type in pumpSessionReplies before parsing as SSE pumpSessionReplies fed any 2xx body straight to the SSE frame parser. A non-SSE response (an HTML error page / a JSON proxy error injected by a CDN) would be consumed as garbage or hang the pump waiting for `data:` lines that never arrive — strictly weaker validation than its sibling subscribeEventsInner, which already guards content-type. Mirror that guard: between the res.ok check and getReader(), reject a body that isn't text/event-stream (cancelling it first). Add a test that a no-subscriber session/prompt whose reply pump GET returns text/html rejects instead of hanging (M3pAM). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): close reply-pump handoff strand race + scope-guard reply resolution Address the latest review wave: - subscribeEvents now removes the aborted reply pump's map entry SYNCHRONOUSLY, not just aborting it. Otherwise, if the subscription exited before the pump's async `.finally` deleted the entry, BOTH stranded-pending guards missed (the consumer sweep saw the entry still present and deferred; the pump's sweep skipped on abort) — a live session/prompt stayed in `pending` forever. Synchronous removal makes the consumer sweep deterministically responsible (M3w6Y). - Reply resolution (both the session-reply pump and the subscribeEvents consumer path) now skips a reply whose pending is scoped to a DIFFERENT session — defense-in-depth against a future daemon misroute silently cross-delivering across the SDK boundary (M3w6d). - denormalizeAcpNotification prefers a NON-EMPTY `type`; an empty-string `type` no longer wins over a valid `kind` and drops the event (M3w6i). Tests: reply-pump handoff happy-path (delivers/resolves) + strand case (rejects, not stranded); empty-`type`→`kind` fallback; the SSE buffer cap firing; the unanchored-reply hold/release branches; and connGraceExpired reset on reconnect (M3w6e, M3w6f, M3w6g). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): sweep session pendings in the subscribe wrapper + carry pump error Two follow-ups on the reply-pump handoff: - The session-scoped pending sweep moves from subscribeEventsInner's read-loop finally to the subscribeEvents WRAPPER finally. The read-loop finally only runs once the pump reaches the loop; a fast failure (fetch reject / non-OK / wrong content-type, all before the loop) skipped it and stranded the pending. The wrapper finally always runs, so it covers the fast-fail path too (M4DWq). - ensureSessionReplyPump captures the pump's error (HTTP 401/404, wrong content-type) and rejects swept pendings WITH it instead of a generic message, so a caller can tell auth failure from a network drop (M4DWx). Test: a 401 on the session GET (inner throws before its read loop) still rejects the in-flight session-scoped pending via the wrapper sweep. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): carry the subscription error into the wrapper sweep + guard tests Follow-ups on the reply-pump handoff: - The subscribeEvents wrapper sweep now rejects with the actual cause of the subscription's exit (captured from a try/catch around the inner generator) instead of a hard-coded generic message. On the fast-fail path (401 / wrong content-type thrown before the inner read loop) this wrapper finally is the only sweep that fires, so the caller now sees the real failure — parity with the reply-pump's pumpError reason (M4W9a). Tests: - the fast-fail sweep reason carries the 401 (not a generic message); - the M3pAM non-SSE rejection asserts the content-type cause reaches the caller (proves pumpError propagation) (M4W9g); - cross-session scope guard, both the consumer and the reply-pump resolution paths: a reply on session A's stream must not resolve a pending scoped to session B (M4W9e). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): guard onSessionGraceExpired in the grace timer against an uncaught throw The session grace-expiry setTimeout protected closeSessionStream with a try/catch but called the owner-supplied onSessionGraceExpired callback outside it. From a bare setTimeout, an uncaught throw there would crash the whole daemon — the same hazard the teardown guard exists for. Wrap it in its own try/catch (separate from teardown, so the conn-reap re-check still runs even if teardown threw). Add a regression test (M4i9z). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): make conn-stream pump CRLF-aware; align replay opt-in doc The connection-scoped SSE reply pump split frames with an LF-only `buf.indexOf('\n\n')`. A server or proxy emitting `\r\n\r\n` frame separators produces no `\n\n` substring, so the scan never found a boundary: the unread buffer grew to the OOM cap and the pump threw, leaving every connection-scoped JSON-RPC reply unresolved. Reuse the shared CRLF-aware `consumeFrames` splitter (and strip a trailing CR per data line) so the conn pump frames exactly like the session readers. Add a regression test that delivers a conn-scoped reply over `\r\n\r\n` and asserts it resolves. Also update the design doc: the in-repo SDK `AcpHttpTransport` opts in to replay in this PR (`supportsReplay = true` + resends Last-Event-ID), so the backward-compat note no longer reads as "keeps false until it opts in". Only the external agent-web transport flip stays deferred (already listed under Out of scope). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(daemon,sdk): log replay-deferral arm; document session-reply routing invariant Add a stderr breadcrumb when a resume arms `replayPending`: while armed, `sendSessionReply` defers every out-of-band reply until the pump delivers `replay_complete`. If that sentinel never arrives (a dropped frame or a pump error), the replies stay buffered indefinitely with no other trace — the log gives operators a starting point. Silent on a fresh connect (no deferral). Covered by a new test. Also strengthen the `SESSION_STREAM_REPLY_METHODS` doc comment: name the authoritative daemon call sites (dispatch.ts), spell out the hang failure mode if the set drifts, and record a build-time grep / shared-constant enforcement as a follow-up (a cross-package invariant the SDK can't type-check). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): use bracket notation for _meta index-signature access (TS4111) `extractParams` returns `Record<string, unknown>`, so dot access to `params._meta` violates `noPropertyAccessFromIndexSignature` (set in the root tsconfig). The esbuild bundle path doesn't typecheck, so CI's build stayed green, but strict `tsc --noEmit` reports 6 × TS4111 at these sites (added with the query-param routing change). Switch all six to `params['_meta']`. Purely syntactic — runtime behavior is unchanged. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): don't silently hang conn-scoped requests on a failed conn stream `pumpConnStream` swallowed two failure paths: a non-2xx / no-body `GET /acp` did a bare `return`, and read-loop errors were caught and dropped. Either way the pump promise RESOLVED, so `openConnStream`'s `.catch` never ran — connection-scoped JSON-RPC pendings stayed in the map forever, and `connStreamAbort` was never cleared, so `ensureConnStream` saw it non-null and never reopened the stream (every later 202 request hung with no pump to deliver its reply). - A non-2xx / missing-body response now throws (HTTP status in the message) so the catch sweep rejects the conn-scoped pendings. - The read-loop catch rethrows real errors and only swallows an intentional abort (dispose / reconnect, which owns its own cleanup). - `openConnStream` clears `connStreamAbort` in a `.finally` (guarded on controller identity) so the stream reopens on the next request after ANY settle — clean close, error, or abort. Regression test: a 500 `GET /acp` rejects the conn-scoped pending (leaves session-scoped ones for their own stream) and the next request reopens. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(daemon,sdk): conn-stream error propagation, listener cleanup, buffer eviction, param parsing Address review findings on the resumable /acp stream and exported SDK transports — all additive/backward-compatible, REST untouched: - openConnStream: reject connection-scoped pendings with the pump's REAL error (HTTP 401/503, network drop) instead of a generic message, mirroring ensureSessionReplyPump. - pumpConnStream: keep the abort listener in a named ref and remove it in finally so a long-lived signal reused across reconnects doesn't accumulate listeners (mirrors the session readers). - sendRequest: remove the abort listener on the happy path (the `{ once: true }` listener self-removes only when the signal fires), preventing per-call listener buildup on a shared caller signal. - pushCapped: under a content flood, evict a REPLAYABLE id-bearing frame (the ring redelivers it) before an irreplaceable id-less deferred reply — dropping the latter would hang the session/prompt caller — and log the dropped id. - acpRouteTable.boolParam: treat a present-but-empty value (`?detail=`) as absent, matching numParam, so `{ detail: false }` isn't forwarded for an unset param. - connection-registry resume flush: hoist the `splice(0)` snapshot into a named local to make the re-entrant copy-semantics invariant visible. Tests: boolParam empty-value omission; pre-attach buffer keeps the id-less reply under a 400-frame content flood. Document two exported-transport limitations (permission voting; session RPC awaited inside the subscribeEvents loop) as §1.7-adjacent follow-ups in the design doc. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(daemon): never evict an irreplaceable id-less reply from the pre-attach buffer The previous pushCapped change preferred evicting id-bearing (ring-replayable) frames, but left a degenerate hole: when the buffer fills with ONLY id-less deferred replies (no id-bearing entry exists), findIndex returned -1, dropIndex fell back to 0, and the oldest deferred JSON-RPC reply was evicted — silently hanging its session/prompt caller, the exact failure the guard exists to prevent (wenshao). Fix: when there is no replayable id-bearing frame to evict, do NOT drop — append and let the id-less replies exceed the soft cap. The cap is a memory bound against a CONTENT flood (id-bearing frames); id-less replies are bounded by the number of in-flight session RPCs the client actually issued (client-controlled, tiny), so they can't run away in practice. Log once when over the soft cap. The connection buffer (no id accessor) keeps its FIFO eviction unchanged. Test: 300 all-id-less replies buffered past the 256 cap are all delivered on reconnect, none evicted. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(daemon): hard ceiling + transition-only logging for the id-less reply buffer Follow-ups on the prior id-less-eviction fix (wenshao): - Defense-in-depth HARD cap. The soft-cap path never drops id-less replies, relying on "id-less replies are RPC-bounded" — true today but enforced only by convention. Add HARD_BUFFERED_FRAMES_CAP (4× soft = 1024): past it, drop the oldest id-less reply and log loudly, so a future non-RPC-bounded producer or a buggy client can't grow the daemon heap without limit. - Log at the soft-cap transition only (buf.length === MAX_BUFFERED_FRAMES), not on every over-cap push — the comment said "once" but it logged linearly with over-cap depth (~44 lines for 300 entries). Tests: assert the soft-cap warning fires exactly once for a 300-entry overflow; new test that 1100 id-less replies are bounded at the 1024 hard cap (oldest dropped, newest kept, loud breach log emitted). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(daemon,sdk): release all deferred replies when replay evicted frames; guard conn pump against session-scoped pending When ring replay overflows and emits state_resync_required, the watermark anchor guarantee is void (the anchored frame may have been evicted), so hold-until-watermark could freeze deferred session replies indefinitely. Track eviction through the pump loop and flush ALL buffered session frames at replay_complete in that case instead of waiting on the watermark. Also harden the SDK conn-stream pump: never resolve a session-scoped pending entry from the connection stream (scope guard), and document the fresh-attach (non-resumptive) caveat for ensureSessionReplyPump. Tests: add FakeBridge.getSessionLastEventId so integration replySession no longer throws (anchorId now reachable); cover the eviction cascade-release path, the conn-stream session-scope guard, and shared reply-pump ref-counting. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(daemon): flush deferred replies on mid-replay iterator error; cover anchored watermark e2e On an iterator error mid-replay the catch path re-throws, which drives onPumpSettled; while the session stream is still open that takes the closeSessionStream branch (full teardown, not a detach-with-grace), so any still-deferred session replies in the binding buffer were dropped rather than preserved. Flush them in the catch before signalling stream_error — same safety flush as the happy-path completion (the iterator has terminated, so no content frame can still race ahead of them). Correct the now-inaccurate happy-path comment that claimed error-path frames stay buffered. Add an end-to-end transport test for the anchored watermark path: with a real getSessionLastEventId, a deferred reply is held through pre-watermark content and released ON its anchor mid-replay, before replay_complete — distinguishing the watermark release from the unanchored release-at-boundary path. Document two deferrals in the design doc: response-replay idempotency for an already-resolved permission (a conformant client dedupes on _meta.requestId; full re-send belongs with the permission-coordination follow-up) and an automated guard for the SESSION_STREAM_REPLY_METHODS drift. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(daemon,sdk): log replay effectiveness; de-shadow pump sweep; document resume/permission edges Add an operator breadcrumb at replay completion (resumed-from cursor, delivery high-water mark, bus replayed count, eviction flag) so 'did resume recover the gap?' is answerable from server logs. Rename the reply-pump sweep loop variable so it no longer shadows the outer pump-map entry (unrelated types). Clarify why the resume path drops id-bearing buffered frames (the event pump is aborted on detach, so only id-less out-of-band replies accumulate during the gap; ring replay owns id-bearing recovery and eviction is signalled via state_resync_required). Document two opt-in-transport edges as permission-coordination follow-ups: the no-subscriber reply pump's GET stream causing an agent permission_request to be routed to the pump and dropped, and why an automated SESSION_STREAM_REPLY_METHODS drift guard needs dataflow (the prompt reply is decoupled from its case block). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
What this PR does
This PR keeps the
qwen servedaemon app as the composition point while moving cohesive request handling, response mapping, telemetry, filesystem, auth provider, session listing, prompt deadline, and route registration responsibilities into focused internal modules. It preserves the existing middleware and route ordering, compatibility exports, HTTP response contracts, SSE framing, and daemon protocol behavior.Why it's needed
Issue #5576 calls out the serve daemon implementation as too large to maintain safely. This first split reduces the central file while keeping behavior stable, so later route extraction can happen against clearer boundaries without changing daemon protocol behavior.
Reviewer Test Plan
How to verify
Run the focused serve tests with low concurrency to avoid server-heavy test interference:
cd packages/cli && npx vitest run src/serve/server.test.ts --maxWorkers=1 --maxConcurrency=1 --no-file-parallelismandcd packages/cli && npx vitest run src/serve/acp-http/*.test.ts src/serve/routes/*.test.ts --maxWorkers=1 --maxConcurrency=1 --no-file-parallelism. Also runnpm run typecheck,npm run build, andnpm run lint:ci. Expected result: all commands pass and the daemon route responses remain unchanged because this PR is structural only.Evidence (Before & After)
N/A; this is a non-UI refactor with no user-visible behavior change.
Tested on
Environment (optional)
macOS local checkout with Node v22.22.3 and npm 10.9.8.
Risk & Scope
Linked Issues
Refs #5576
中文说明
What this PR does
本 PR 保留
qwen servedaemon app 作为装配点，同时把请求处理、错误响应映射、遥测、文件系统、鉴权 provider、会话列表、prompt deadline 和 route 注册等职责移动到更聚焦的内部模块。现有 middleware 和 route 顺序、兼容导出、HTTP 响应契约、SSE 帧格式以及 daemon 协议行为都会保持不变。Why it's needed
issue #5576 指出 serve daemon 实现过大，后续维护风险高。本阶段先在行为稳定的前提下缩小中心文件，让后续 route 继续下沉时可以基于更清晰的边界推进，而不需要改变 daemon 协议行为。
Reviewer Test Plan
How to verify
用低并发方式运行 focused serve 测试以避开 server-heavy 测试互相干扰：
cd packages/cli && npx vitest run src/serve/server.test.ts --maxWorkers=1 --maxConcurrency=1 --no-file-parallelism和cd packages/cli && npx vitest run src/serve/acp-http/*.test.ts src/serve/routes/*.test.ts --maxWorkers=1 --maxConcurrency=1 --no-file-parallelism。同时运行npm run typecheck、npm run build和npm run lint:ci。预期所有命令通过，并且 daemon route 响应保持不变，因为本 PR 只做结构重构。Evidence (Before & After)
N/A；这是非 UI 重构，没有用户可见行为变化。
Tested on
Environment (optional)
macOS 本地 checkout，Node v22.22.3，npm 10.9.8。
Risk & Scope
Linked Issues
Refs #5576