feat(serve): add daemon file logger (#4548)#4559
Conversation
📋 Review SummaryThis PR adds a per-process daemon file logger for 🔍 General Feedback
🎯 Specific Feedback🟢 Medium
🔵 Low
✅ Highlights
|
|
Thanks for the review.
Re Low #5 (double-logging): |
There was a problem hiding this comment.
Pull request overview
This PR adds a daemon-scoped file logger for qwen serve so serve/bridge diagnostics are persisted under the runtime debug directory (with a daemon/latest symlink), while keeping existing stderr behavior for operators.
Changes:
- Introduces
initDaemonLogger()(CLI) to write daemon diagnostics to${runtimeDir}/debug/daemon/serve-<pid>-<workspaceHash>.logand tee selected messages to stderr. - Wires the daemon logger through
runQwenServeandserver.ts(routingsendBridgeError5xx diagnostics into the daemon log). - Extends
@qwen-code/acp-bridgeto accept anonDiagnosticLinecallback and a configurablecreateSpawnChannelFactory({ onDiagnosticLine })to tee ACP child stderr and serve debug breadcrumbs into the daemon log.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/src/serve/daemonLogger.ts | New daemon file logger (formatting, append queue, symlink update). |
| packages/cli/src/serve/daemonLogger.test.ts | Unit coverage for formatter + init + raw + symlink behavior. |
| packages/cli/src/serve/runQwenServe.ts | Initializes daemon logger, wires diagnostic sink into bridge/spawn, uses daemon logger for lifecycle diagnostics. |
| packages/cli/src/serve/runQwenServe.test.ts | Integration test verifying boot creates daemon log file under runtime dir. |
| packages/cli/src/serve/server.ts | Injects optional daemon logger into error helpers; routes 5xx bridge errors through it. |
| packages/cli/src/serve/server.test.ts | Verifies sendBridgeError writes structured daemon log output when injected. |
| packages/acp-bridge/src/bridgeOptions.ts | Adds DiagnosticLineSink and optional onDiagnosticLine hook. |
| packages/acp-bridge/src/bridge.ts | Tees writeServeDebugLine output into onDiagnosticLine when serve debug is enabled. |
| packages/acp-bridge/src/bridge.test.ts | Tests the onDiagnosticLine tee behavior under QWEN_SERVE_DEBUG. |
| packages/acp-bridge/src/spawnChannel.ts | Refactors spawn factory to createSpawnChannelFactory + extracts/testable stderr forwarder; tees stderr lines via callback. |
| packages/acp-bridge/src/spawnChannel.test.ts | Tests stderr forwarder buffering/truncation + callback behavior. |
| docs/users/qwen-serve.md | Documents daemon log file path, latest symlink, opt-out, and relationship to session logs. |
| docs/superpowers/specs/2026-05-26-daemon-logger-design.md | Design doc for the daemon logger feature. |
| docs/superpowers/plans/2026-05-26-daemon-logger.md | Implementation plan doc for the feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| server.removeAllListeners('error'); | ||
| server.on('error', (err) => { | ||
| writeStderrLine( | ||
| `qwen serve: server error: ${err instanceof Error ? err.message : String(err)}`, | ||
| ); | ||
| daemonLog.error('server error', err instanceof Error ? err : null); | ||
| }); |
There was a problem hiding this comment.
Thanks — won't take this one. Node server/bridge errors are always Error instances in practice; wrapping unknown values adds complexity for a codepath that doesn't fire.
| export function initDaemonLogger(opts: InitDaemonLoggerOptions): DaemonLogger { | ||
| if (isOptedOut()) return NOOP_LOGGER; | ||
|
|
There was a problem hiding this comment.
Thanks — agreed, will fix. The opt-out / init-failure path should still tee to stderr (only file writes disabled). Spec §10 guarantees 'stderr unchanged' when file logging is off.
| for (const key of FIXED_CTX_ORDER) { | ||
| const v = ctx[key]; | ||
| if (v !== undefined && v !== null) { | ||
| parts.push(`${key}=${String(v)}`); | ||
| } |
There was a problem hiding this comment.
Thanks — won't take this one. The asymmetry is intentional: fixed keys (route, sessionId) are well-known identifiers safe to render unquoted; extra keys are arbitrary and need quoting for safe log parsing. Test fixtures encode this contract.
| } catch (err) { | ||
| writeStderrLine( | ||
| `qwen serve: force-kill error: ${err instanceof Error ? err.message : String(err)}`, | ||
| daemonLog.error( | ||
| 'force-kill error', | ||
| err instanceof Error ? err : null, | ||
| ); |
There was a problem hiding this comment.
Thanks — won't take this one. Node server/bridge errors are always Error instances in practice; wrapping unknown values adds complexity for a codepath that doesn't fire.
| process.exit(0); | ||
| } catch (err) { | ||
| writeStderrLine(`qwen serve: shutdown error: ${String(err)}`); | ||
| daemonLog.error('shutdown error', err instanceof Error ? err : null); |
There was a problem hiding this comment.
Thanks — won't take this one. Node server/bridge errors are always Error instances in practice; wrapping unknown values adds complexity for a codepath that doesn't fire.
| daemonLog.error( | ||
| 'bridge shutdown error', | ||
| err instanceof Error ? err : null, | ||
| ); |
There was a problem hiding this comment.
Thanks — won't take this one. Node server/bridge errors are always Error instances in practice; wrapping unknown values adds complexity for a codepath that doesn't fire.
| err instanceof Error ? err.message : String(err) | ||
| }`, | ||
| ); | ||
| return NOOP_LOGGER; |
There was a problem hiding this comment.
Thanks — agreed, will fix. The opt-out / init-failure path should still tee to stderr (only file writes disabled). Spec §10 guarantees 'stderr unchanged' when file logging is off.
ba45d2e to
fc83f5c
Compare
wenshao
left a comment
There was a problem hiding this comment.
[Suggestion] Test gap in runQwenServe.test.ts — the test "creates a daemon log file at boot and flushes on shutdown" asserts expect(finalContent).toContain('daemon started') after handle.close(), which is the same assertion made before close. It doesn't verify that flush() actually wrote any new content during shutdown. Consider asserting finalContent.length > logContent.length to confirm the flush produced output.
Additional test gaps: no coverage for sendPermissionVoteErrorImpl's daemonLog fallthrough path in server.test.ts, and no test for createSpawnChannelFactory in spawnChannel.test.ts (only createStderrForwarder is tested). — qwen3.7-max via Qwen Code /review
| for (const key of FIXED_CTX_ORDER) { | ||
| const v = ctx[key]; | ||
| if (v !== undefined && v !== null) { | ||
| parts.push(`${key}=${String(v)}`); |
There was a problem hiding this comment.
[Suggestion] renderCtx applies String(v) for fixed-order keys (line 45) but renderCtxValue(v) for extra keys (line 54). renderCtxValue JSON-quotes values containing whitespace or =, while String(v) performs no sanitization.
sessionId comes from req.params['id'] in server routes (Express URL-decodes path parameters). A crafted URL like /session/foo%0Afake-entry/prompt produces a sessionId with an embedded newline. When sendBridgeError passes it through daemonLog.error(msg, err, { sessionId }), the newline is written raw into the log file — enabling log line forgery.
The codebase already treats this class of injection as a real concern elsewhere (e.g., the workspace_mismatch handler uses JSON.stringify for the same reason).
| parts.push(`${key}=${String(v)}`); | |
| parts.push(`${key}=${renderCtxValue(v)}`); |
— qwen3.7-max via Qwen Code /review
| // inside createHttpAcpBridge we use this wrapper exclusively. | ||
| const teeServeDebugLine = (message: string): void => { | ||
| writeServeDebugLine(message); | ||
| if (opts.onDiagnosticLine && isServeDebugLoggingEnabled()) { |
There was a problem hiding this comment.
[Suggestion] teeServeDebugLine gates the onDiagnosticLine callback on isServeDebugLoggingEnabled(). This means bridge-internal diagnostics (permission vote rejections, broadcast failures, workspace event publish errors) are not written to the daemon log unless QWEN_SERVE_DEBUG=1 was set at boot.
The daemon log — the whole point of this PR — silently misses bridge-internal diagnostics by default. An operator reading ~/.qwen/debug/daemon/latest at 3 AM will see a gap exactly where they need data most.
The stderr gate (writeServeDebugLine on line 704) should remain. But the file path (onDiagnosticLine on line 705) should fire unconditionally:
| if (opts.onDiagnosticLine && isServeDebugLoggingEnabled()) { | |
| const teeServeDebugLine = (message: string): void => { | |
| writeServeDebugLine(message); | |
| if (opts.onDiagnosticLine) { | |
| opts.onDiagnosticLine(`qwen serve debug: ${message}`, 'info'); | |
| } | |
| }; |
— qwen3.7-max via Qwen Code /review
| const flush = (line: string) => { | ||
| if (line.length > 0) { | ||
| process.stderr.write(prefix + line + '\n'); | ||
| if (onDiagnosticLine) onDiagnosticLine(prefix + line, 'warn'); |
There was a problem hiding this comment.
[Suggestion] onDiagnosticLine(prefix + line, 'warn') is called without try/catch inside the child.stderr.on('data') event handler. If the callback throws, the exception propagates through Node's EventEmitter 'data' handler — potentially detaching it and silently dropping all remaining child stderr output for the lifetime of that child process.
The current callback (daemonLog.raw) is unlikely to throw, but defense-in-depth suggests wrapping:
| if (onDiagnosticLine) onDiagnosticLine(prefix + line, 'warn'); | |
| if (onDiagnosticLine) { | |
| try { onDiagnosticLine(prefix + line, 'warn'); } catch { /* swallow */ } | |
| } |
— qwen3.7-max via Qwen Code /review
| writeStderrLine(`qwen serve: received ${signal}, draining...`); | ||
| daemonLog.warn(`received ${signal}, draining`); | ||
| try { | ||
| await handle.close(); |
There was a problem hiding this comment.
[Suggestion] The signal handler correctly flushes here after handle.close(). However, handle.close() itself (line 989+) orchestrates bridge.shutdown(), device-flow registry dispose, and server.close() — without calling daemonLog.flush().
Programmatic callers (tests, embedded deployments) calling handle.close() directly will not get async log entries flushed. Shutdown-phase diagnostics (bridge shutdown errors, force-close timeouts) are lost.
Add await daemonLog.flush().catch(() => {}) at the end of the close() closure (before finish() resolve), or expose flush on the RunHandle interface.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
[Suggestion] Test gap in runQwenServe.test.ts — the test "creates a daemon log file at boot and flushes on shutdown" asserts expect(finalContent).toContain('daemon started') after handle.close(), which is the same assertion made before close. It doesn't verify that flush() actually wrote any new content during shutdown. Consider asserting finalContent.length > logContent.length to confirm the flush produced output.
Additional test gaps: no coverage for sendPermissionVoteErrorImpl's daemonLog fallthrough path in server.test.ts, and no test for createSpawnChannelFactory in spawnChannel.test.ts (only createStderrForwarder is tested). — qwen3.7-max via Qwen Code /review
| for (const key of FIXED_CTX_ORDER) { | ||
| const v = ctx[key]; | ||
| if (v !== undefined && v !== null) { | ||
| parts.push(`${key}=${String(v)}`); |
There was a problem hiding this comment.
[Suggestion] renderCtx applies String(v) for fixed-order keys (line 45) but renderCtxValue(v) for extra keys (line 54). renderCtxValue JSON-quotes values containing whitespace or =, while String(v) performs no sanitization.
sessionId comes from req.params['id'] in server routes (Express URL-decodes path parameters). A crafted URL like /session/foo%0Afake-entry/prompt produces a sessionId with an embedded newline. When sendBridgeError passes it through daemonLog.error(msg, err, { sessionId }), the newline is written raw into the log file — enabling log line forgery.
The codebase already treats this class of injection as a real concern elsewhere (e.g., the workspace_mismatch handler uses JSON.stringify for the same reason).
| parts.push(`${key}=${String(v)}`); | |
| parts.push(`${key}=${renderCtxValue(v)}`); |
— qwen3.7-max via Qwen Code /review
| // inside createHttpAcpBridge we use this wrapper exclusively. | ||
| const teeServeDebugLine = (message: string): void => { | ||
| writeServeDebugLine(message); | ||
| if (opts.onDiagnosticLine && isServeDebugLoggingEnabled()) { |
There was a problem hiding this comment.
[Suggestion] teeServeDebugLine gates the onDiagnosticLine callback on isServeDebugLoggingEnabled(). This means bridge-internal diagnostics (permission vote rejections, broadcast failures, workspace event publish errors) are not written to the daemon log unless QWEN_SERVE_DEBUG=1 was set at boot.
The daemon log — the whole point of this PR — silently misses bridge-internal diagnostics by default. An operator reading ~/.qwen/debug/daemon/latest at 3 AM will see a gap exactly where they need data most.
The stderr gate (writeServeDebugLine on line 704) should remain. But the file path (onDiagnosticLine on line 705) should fire unconditionally:
| if (opts.onDiagnosticLine && isServeDebugLoggingEnabled()) { | |
| const teeServeDebugLine = (message: string): void => { | |
| writeServeDebugLine(message); | |
| if (opts.onDiagnosticLine) { | |
| opts.onDiagnosticLine(`qwen serve debug: ${message}`, 'info'); | |
| } | |
| }; |
— qwen3.7-max via Qwen Code /review
| const flush = (line: string) => { | ||
| if (line.length > 0) { | ||
| process.stderr.write(prefix + line + '\n'); | ||
| if (onDiagnosticLine) onDiagnosticLine(prefix + line, 'warn'); |
There was a problem hiding this comment.
[Suggestion] onDiagnosticLine(prefix + line, 'warn') is called without try/catch inside the child.stderr.on('data') event handler. If the callback throws, the exception propagates through Node's EventEmitter 'data' handler — potentially detaching it and silently dropping all remaining child stderr output for the lifetime of that child process.
The current callback (daemonLog.raw) is unlikely to throw, but defense-in-depth suggests wrapping:
| if (onDiagnosticLine) onDiagnosticLine(prefix + line, 'warn'); | |
| if (onDiagnosticLine) { | |
| try { onDiagnosticLine(prefix + line, 'warn'); } catch { /* swallow */ } | |
| } |
— qwen3.7-max via Qwen Code /review
| writeStderrLine(`qwen serve: received ${signal}, draining...`); | ||
| daemonLog.warn(`received ${signal}, draining`); | ||
| try { | ||
| await handle.close(); |
There was a problem hiding this comment.
[Suggestion] The signal handler correctly flushes here after handle.close(). However, handle.close() itself (line 989+) orchestrates bridge.shutdown(), device-flow registry dispose, and server.close() — without calling daemonLog.flush().
Programmatic callers (tests, embedded deployments) calling handle.close() directly will not get async log entries flushed. Shutdown-phase diagnostics (bridge shutdown errors, force-close timeouts) are lost.
Add await daemonLog.flush().catch(() => {}) at the end of the close() closure (before finish() resolve), or expose flush on the RunHandle interface.
— qwen3.7-max via Qwen Code /review
1284383 to
1ca0572
Compare
Document the architecture, daemon-id scheme, API surface, tee semantics, boot/shutdown flow, and test plan for adding a daemon- specific file sink to qwen serve diagnostics. Companion to issue #4548.
Bite-sized task list covering: pure formatter, file init, info/warn/ error + flush, raw file-only tee, latest symlink, acp-bridge sink injection, spawn factory refactor, runQwenServe wiring, docs, and final verification + PR creation. Companion to the design spec.
- updateSymlink: re-export from core barrel first, then import - bridge.test.ts harness: use makeBridge/makeChannel from testUtils (MockStream was hallucinated) - writeServeDebugLine: enumerate all 6 call sites, not 2 - createServeApp: correct 3-arg signature (opts, getPort, deps); daemonLog goes in deps, not as a 1st-arg key
fc83f5c to
f152614
Compare
| } | ||
| }), | ||
| ); | ||
| }; |
There was a problem hiding this comment.
[Suggestion] After degraded is set to true, enqueueAppend continues chaining .then(() => appendFile(...).catch(...)) for every subsequent call. Each doomed write triggers a microtask + I/O syscall that produces no output. Under high-volume degraded logging (e.g., a crashing child flooding stderr while the log dir is gone), this accumulates thousands of wasted microtasks per second on the event loop.
| }; | |
| const enqueueAppend = (line: string): void => { | |
| if (degraded) return; | |
| pending = pending.then(() => | |
| nodeFs.promises.appendFile(logPath, line).catch((err) => { | |
| if (!degraded) { | |
| degraded = true; | |
| stderr( | |
| `qwen serve: daemon log write failed — entering degraded mode: ${ | |
| err instanceof Error ? err.message : String(err) | |
| }`, | |
| ); | |
| } | |
| }), | |
| ); | |
| }; |
— qwen3.7-max via Qwen Code /review
| ); | ||
| } catch { | ||
| // Defensive: any sync throw is ignored. | ||
| } |
There was a problem hiding this comment.
[Suggestion] flush() returns the tail of the pending promise chain with no timeout. In the force-exit path (runQwenServe.ts: await daemonLog.flush().catch(() => {}); process.exit(1)), a wedged disk (NFS hang, FUSE deadlock) causes appendFile to never resolve or reject — the .catch only guards against rejections, not hangs. The operator's last-resort double-Ctrl+C escape hatch becomes a deadlock.
Consider wrapping flush() with a timeout at the call sites, or inside flush() itself:
flush: () => Promise.race([
pending,
new Promise<void>((resolve) => setTimeout(resolve, 2_000)),
]),— qwen3.7-max via Qwen Code /review
| for (let i = 0; i < 50; i++) { | ||
| expect(msgLines[i]).toContain(`msg-${i}`); | ||
| } | ||
| }); |
There was a problem hiding this comment.
[Suggestion] Test gap: the "warns once on append failure and keeps trying" test passes stderr: () => {} (discards all output) and only asserts "no throw". The degraded-mode stderr warning — the logger's primary resilience mechanism — is never actually verified. If the if (!degraded) guard were accidentally removed or inverted, this test would still pass.
Suggested fix: capture stderr lines and assert:
- Exactly one degraded-mode warning after the first failed write
- No second warning after the second write (confirming the
degradedflag suppresses duplicates)
Similarly, the signal handler rewrite in runQwenServe.ts (5 daemonLog.warn/error/flush calls) has zero test coverage — shutdown logging, the PR's core motivation, is untested.
— qwen3.7-max via Qwen Code /review
| // Init daemon logger early so all subsequent lifecycle events | ||
| // (bridge spawn diagnostics, shutdown errors) are captured to file. | ||
| const daemonLog: DaemonLogger = initDaemonLogger({ boundWorkspace }); | ||
| writeStderrLine( |
There was a problem hiding this comment.
[Suggestion] This writeStderrLine and the subsequent boot banners (workspace binding ~line 922, bearer auth status ~lines 926/936) still go to stderr only, not to the daemon log file. Since daemonLog is already initialized above, consider routing these through daemonLog.info(...) — this preserves stderr output (tee behavior) while also capturing boot-critical context (workspace, auth config) in the file for post-mortem analysis:
daemonLog.info(`daemon log → ${daemonLog.getLogPath() || '(disabled)'}`);
// ...later in app.listen callback:
daemonLog.info(`bound to workspace ${JSON.stringify(boundWorkspace)}`);— qwen3.7-max via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
No new review findings in this round. All prior-round suggestions remain open (boot banner bypass, degraded mode, flush timeout, renderCtx quoting, teeServeDebugLine gating, onDiagnosticLine try/catch, handle.close() flush). CI all green (5/5), 593 tests pass. LGTM! ✅ — qwen3.7-max via Qwen Code /review
本地验证报告（maintainer review）PR 目标 结论✅ 建议 MERGE。
§1. 验证矩阵
§2. 架构核对（PR 自描述 ↔ 代码）
§3. 直驱
|
| # | 场景 | 断言数 | 结果 |
|---|---|---|---|
| 1 | Happy path — 文件路径形状 / boot banner 内容 / 无 stderr 噪音 | 5 | ✅ |
| 2 | computeDaemonId 稳定性 — 同 (workspace,pid) → 同 id；不同 workspace → 不同 id |
2 | ✅ |
| 3 | info/warn/error 同时进文件 + stderr，含 ctx kv + Error stack |
4 | ✅ |
| 4 | raw(line, level) 只进文件不进 stderr，3 个 level 都覆盖 |
2 | ✅ |
| 5 | daemon/latest symlink 创建 + resolve 到真 log 文件 |
2 | ✅ |
| 6 | QWEN_DAEMON_LOG_FILE 关闭值 6 种（0 / false / off / no / OFF / " No "） |
12 | ✅ |
| 7 | QWEN_DAEMON_LOG_FILE 真值 5 种（1 / true / on / YES / ""） 仍照常初始化 |
5 | ✅ |
| 8 | Boot probe 失败（baseDir 父目录是普通文件，mkdirSync 失败）→ NOOP + 单条 "daemon log disabled" 到 stderr |
2 | ✅ |
| 9 | Post-boot 写失败（init 后把 log 文件换成目录，appendFile 触发 EISDIR）→ "entering degraded mode" 恰好 1 次（不是每次失败都喊） |
1 | ✅ |
| 10 | 50 条 info() 并发提交 → 写入文件后顺序保留（单 promise chain） |
1 | ✅ |
| 11 | buildDaemonLogLine formatter — ISO 时间 / [LEVEL] / [DAEMON] / ctx fixed-order / Error stack 2-space 缩进 |
2 | ✅ |
| 12 | tee 顺序 — info() 调 stderr 同步 完成（在任何 await/flush 之前） |
1 | ✅ |
#10 是我特别加的：单
pendingpromise chain 的实现假设依赖 promise micro-task 顺序保序，并发 50 次还能严格顺序写盘说明这一点不破。
#9 是 PR review 来回了 R3/R4 之后最微妙的语义之一 —— "失败要喊一声但只能喊一次"，行为正确。
§4. 真机 qwen serve × tmux 行为验证
| # | 操作 | 期望 | 实测 |
|---|---|---|---|
| 1 | QWEN_RUNTIME_DIR=/tmp/pr4559-runtime node dist/cli.js serve --port 0 --workspace ... |
stderr 第一行打印 qwen serve: daemon log → <path> + 文件创建 |
✅ 路径 = /private/tmp/pr4559-runtime/debug/daemon/serve-69760-5048c40d.log（pid + sha256(workspace).slice(0,8) 形态正确） |
| 2 | ls /private/tmp/pr4559-runtime/debug/daemon/ |
含 serve-<pid>-<hash>.log + latest -> serve-<pid>-<hash>.log symlink |
✅ |
| 3 | cat <log-file> |
含 boot banner [INFO] [DAEMON] daemon started pid=... workspace=... |
✅ 一字不差 |
| 4 | kill -TERM <daemon-pid> |
log 文件追加 [WARN] [DAEMON] received SIGTERM, draining；同 stderr 也看得到（tee） |
✅ 文件 + stderr 都有；额外还出现一条 "received SIGTERM during drain — forcing exit"（多 signal handler 触发了 fallback 路径，这是 PR 设计内行为，参考 runQwenServe.ts:960） |
| 5 | tail -f /private/tmp/pr4559-runtime/debug/daemon/latest 在另一个 shell 起，然后再发一次 kill -TERM |
tail 实时收到 WARN 行 | ✅ tail -f 流式收到 2026-05-27T04:45:37.119Z [WARN] [DAEMON] received SIGTERM, draining 后 daemon 退出 |
| 6 | QWEN_DAEMON_LOG_FILE=0 node dist/cli.js serve ... |
stderr 第一行打印 qwen serve: daemon log → (disabled) + 不创建任何 daemon dir |
✅ runtime dir /private/tmp/pr4559-runtime-optout/ 完全不存在 |
| 7 | opt-out 模式下后续 lifecycle 仍写 stderr | listening / bound / bearer 三行都在 | ✅ stderr 仍工作，仅文件路径分支被禁用 |
| 8 | 触发 POST /session 工作空间不匹配（400 路径 workspace_mismatch） |
走 stderr-only（不进 daemonLog，因为 sendBridgeError 只对 5xx 走 daemonLog.error，见 server.ts:3409-3425 注释） |
✅ stderr 有 workspace_mismatch (POST /session): ...，daemon log 文件无变化 —— 设计意图与实现一致 |
#4 顺带发现 SIGTERM 路径会触发两条 WARN（drain + "during drain — forcing exit"），不是 bug，是
signalHandler注册时同时绑定两个不同的 fallback path 导致；不影响功能也不漏行。
§5. 代码评审要点
DaemonLogger接口设计克制：3 个 tee（info/warn/error） + 1 个 file-only（raw） + 3 个 getter +flush。raw这个口子开得很有必要 —— acp-bridge 把已经在写 stderr 的内容镜像到文件，避免重复 stderr。- boot 同步 probe + 运行时异步 queue：选择得对。boot 同步保证"启动时知道日志是不是真的能写"，启动后异步保证 hot path 不阻塞。同步 probe 失败直接降级 NOOP 是好默认 —— 不会让一个非关键的 logging 问题阻塞
qwen serve启动。 - 一次性 degraded warning：
degradedflag 全局 latch，保证只 emit 一次 "entering degraded mode"。生产环境如果~/.qwen/debug满了，不会被几十万行同样的错误刷屏。 updateSymlink({ fallbackCopy: false })是对的：symlink 失败时不要拷贝（拷贝静态文件意味着第一次拷出去之后就再也看不到新行了，反而比没有 latest 更糟）。失败时就静默忽略，tail -f <full-path>仍可用。- acp-bridge 解耦做到位：
BridgeOptions.onDiagnosticLine（types 文件只 17 行）+createSpawnChannelFactory({ onDiagnosticLine })工厂。acp-bridge 内部 0 个 cli import。这条线后续可以独立演进（比如换成 structured emitter）。 sendBridgeError5xx-only 路由：4xx（如 workspace_mismatch）属于"客户端犯错"，不该刷 operator 的故障日志；只有 5xx 才真正需要 operator 排查。这个划线对的，§4 report error when try to auth #8 实测验证。daemonLog.flush()在 SIGTERM drain 后：保证关键 shutdown 日志真的落盘（不丢最后几行）。在 catch 里也flush().catch(() => {})，错误降级到 best-effort 而不是抛出。
§6. 风险与遗留
- ✅ opt-out 路径完整 & 安全默认：
QWEN_DAEMON_LOG_FILE=0完全短路，连 dir 都不 mkdir。对在共享主机或 CI runner 上跑 daemon 的用户，单一 env 即可解决"我不要这文件"诉求。 - ✅ boot 失败 graceful degrade：log dir 写不了不阻塞
qwen serve启动。生产环境从只读 FS、磁盘满、权限不对等情况都不会让 daemon 死。 - ✅ 文档完备：
docs/users/qwen-serve.md+24 行讲清楚路径、latest用法、opt-out 环境变量。docs/superpowers/{plans,specs}/2026-05-26-daemon-logger-*.md1777 行设计文档供后续 review 参考（不影响 runtime，纯文档）。 ⚠️ 设计文档体积大（1777 行）：内部 superpowers/ 目录的两份文档比代码本身还多。如果项目对设计文档落仓有偏好就放着，如果不想要可以建议作者拆到.qwen/design/或单独 PR。不阻塞。⚠️ server.test.ts 2 个测试隔离 flake：PR 描述已交代，本地隔离重跑确认是 pre-existing，与本 PR 无关。不阻塞。⚠️ SIGTERM 路径会双 emit："received SIGTERM, draining" + "received SIGTERM during drain — forcing exit"。看起来是同一信号触发了两条 handler；功能上无害但日志略重复。如果作者关心可以 follow-up 优化 signal handler 去重。不阻塞。- ✅ 仅 macOS arm64 本机覆盖；GitHub Actions CI 因 base 是
daemon_mode_b_main没跑完整 matrix（只 2 条 review-pr 自动 SKIPPED），但 base 分支本身 CI 是绿的。merge 到main时若有 windows / linux 差异，主要影响nodeFs.symlink（windows symlink 需 admin /--unprivileged），已通过updateSymlinkcore helper +fallbackCopy: false的"失败静默"语义降到非阻塞。
验证环境：macOS Darwin 25.4.0 arm64，Node v22.17.0；tmux 会话 pr4559（160×50）；merged-base worktree /private/tmp/pr4559-merged（branch pr-4559-test = PR head f15261440，与 base 1ca057251 已 up-to-date 无需 merge）；runtime dirs /private/tmp/pr4559-runtime{,-optout,-tail}；driver realio-daemon-logger.mjs 39 assertion + 8 live qwen-serve 场景全绿。— wenshao
Summary
~/.qwen/debug/daemon/serve-<pid>-<workspaceHash>.log(configurable viaQWEN_RUNTIME_DIR, opt-out viaQWEN_DAEMON_LOG_FILE=0).runQwenServelifecycle messages,sendBridgeErrorroute errors,writeServeDebugLinedebug breadcrumbs, and ACP child stderr into the daemon log without removing existing stderr output.BridgeOptions.onDiagnosticLineandcreateSpawnChannelFactory({ onDiagnosticLine })to keepacp-bridgeignorant of cli.daemon/latestsymlink fortail -f.Closes #4548.
Architecture
packages/cli/src/serve/daemonLogger.ts—initDaemonLogger(opts) → DaemonLoggerinfo/warn/error→ file + stderr (tee)raw→ file only (for lines whose caller already writes to stderr)acp-bridgechanges:DiagnosticLineSinktype + callback injection (no cli dependency)runQwenServewiring: init at boot → pass to bridge/server → flush on shutdownTest plan
packages/cli/src/serve/daemonLogger.test.ts— 21 specs (formatter, opt-out, file init, info/warn/error, raw, latest symlink, degraded fallback)packages/acp-bridge/src/bridge.test.ts— 2 new specs (onDiagnosticLine tee)packages/acp-bridge/src/spawnChannel.test.ts— 6 new specs (createStderrForwarder)packages/cli/src/serve/server.test.ts— 2 new specs (sendBridgeError → daemonLog routing)packages/cli/src/serve/runQwenServe.test.ts— 1 new integration test (boot banner + daemon file)tsc --noEmit) for both packagespackages/acp-bridgesuite: 308/308 passpackages/cli/src/serve/suite: 706/708 pass (2 pre-existing test-isolation flakes unrelated to this PR)🤖 Generated with Qwen Code