feat(cli): Add channel worker settings reload for serve --channel - #6598
Conversation
The daemon-managed channel worker reads each channel's settings (tokens, proxy, per-channel model) once when it starts, so applying settings.json changes previously required restarting the whole daemon. This adds an explicit reload that stops and relaunches the worker so it re-reads settings.json, without bouncing the daemon or its live sessions. The reload is exposed as a strict-gated POST /workspace/channel/reload route, an SDK reloadChannelWorker() method, and a qwen channel reload CLI command, advertised through a channel_reload capability only when the daemon was started with --channel. The worker supervisor gains a restart() that coalesces concurrent reloads onto a single relaunch, resets the crash-restart budget so a failed worker recovers, and latches a disposed flag on hard shutdown so a racing reload cannot relaunch a worker into a tearing-down daemon. Refs QwenLM#5976
|
Thanks for the PR! Template looks good ✓ Problem: Real, observed operational limitation — the channel worker reads Direction: Clearly aligned with the daemon-managed channel worker roadmap (#5976). This PR is a natural continuation of the V1.5 hardening work (heartbeat, bounded restarts, credential redaction) that has already been merged. Adding in-place settings reload fits the established pattern of Size: Not applicable — no Approach: The scope feels right. Three access points (HTTP route, SDK method, CLI command) for the reload is standard daemon surface area — each is thin and delegates to the supervisor's Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：真实存在的运维限制——channel worker 只在启动时读取一次 方向：与 daemon 托管 channel worker 路线图 (#5976) 明确对齐。本 PR 是已合并的 V1.5 加固工作（心跳、有界重启、凭证脱敏）的自然延续。添加原地设置重载符合已建立的 规模：不适用——未触及 方案：范围合理。重载的三个入口（HTTP 路由、SDK 方法、CLI 命令）是标准的 daemon 暴露面——每个入口都很薄，委托给 supervisor 的 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal: to solve "frozen channel settings require full daemon restart", I'd add a The PR's approach matches this exactly. The supervisor's No critical blockers found. No AGENTS.md violations — the code is focused, minimal, and avoids unnecessary abstraction. The TestingTmux real-user testing is not applicable here — this is daemon HTTP / SDK / CLI surface with no TUI change. Verification is through unit tests. Unit Tests (PR branch, local)Key test scenarios covered:
Build & Typecheck— Qwen Code · qwen3.7-max |
|
This is a well-scoped, well-implemented feature that solves a real operational problem. The author has been building the daemon-managed channel worker feature methodically across four merged PRs (#5978, #6031, #6098, #6146), and this PR is a natural next step — in-place settings reload without dropping sessions. The implementation is clean and minimal. The supervisor's All 1058 unit tests pass across four test files, build succeeds, typecheck passes across all packages. The test coverage is thorough — every edge case the PR describes (coalescing, shutdown race, failure recovery, auth gating) has a corresponding test. No concerns. Approving. 中文说明这是一个范围合理、实现良好的功能，解决了真实的运维问题。作者在四个已合并的 PR（#5978、#6031、#6098、#6146）中有方法地构建了 daemon 托管 channel worker 功能，本 PR 是自然的下一步——不丢弃会话的原地设置重载。 实现干净且最小化。supervisor 的 四个测试文件共 1058 个单元测试全部通过，构建成功，所有包的 typecheck 通过。测试覆盖全面——PR 描述的每个边界情况（合并、关闭竞态、失败恢复、认证门控）都有对应的测试。 无顾虑，批准。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
| ### Settings reload (`POST /workspace/channel/reload`) | ||
|
|
||
| The daemon reads channel settings from `settings.json` once, when the channel worker starts (`packages/cli/src/commands/channel/daemon-worker.ts` → `loadSettings` → `loadChannelsConfig`). To apply changes without a full daemon restart, the daemon exposes `POST /workspace/channel/reload` (strict mutation gate; SDK `DaemonClient.reloadChannelWorker()`; CLI `qwen channel reload`): | ||
|
|
There was a problem hiding this comment.
This documents a failure response that includes the latest worker snapshot, but the route catch path currently delegates to sendBridgeError(res, err, ...), which returns the generic error body and does not include worker/snapshot data. Please either include the snapshot in the 5xx response or narrow this doc to say callers should use GET /daemon/status for the latest snapshot after a failed reload.
|
|
||
| function resolveToken(flag: string | undefined): string | undefined { | ||
| return ( | ||
| flag ?? |
There was a problem hiding this comment.
resolveToken() returns the raw environment value and then passes it as an explicit token to DaemonClient, which bypasses the SDK env fallback that trims QWEN_SERVER_TOKEN. A common export QWEN_SERVER_TOKEN="$(cat token.txt)" value with a trailing newline will be sent as Authorization: Bearer <token>\n (or rejected as an invalid header), so qwen channel reload fails even though other SDK/daemon-client uses of the same env var work. Please trim env-derived tokens here, or only pass an explicit token for the CLI flag and let DaemonClient read QWEN_SERVER_TOKEN itself.
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
| ? [`restarts=${worker.restartCount}`] | ||
| : []), | ||
| ...(worker.error ? [`error=${worker.error}`] : []), | ||
| ]; |
There was a problem hiding this comment.
[Suggestion] The CLI always exits 0 after a successful HTTP round-trip, even when worker.state is failed. Automation and CI pipelines relying on exit codes won't detect a reload that launched into a broken state.
| ]; | |
| if (worker.state === 'failed') { | |
| writeStderrLine( | |
| `[Channel] Worker is in failed state after reload${worker.error ? `: ${worker.error}` : ''}.`, | |
| ); | |
| process.exit(1); | |
| } | |
| writeStdoutLine(`[Channel] Reloaded (${parts.join(', ')}).`); | |
| process.exit(0); |
— qwen3.7-max via Qwen Code /review
| } | ||
| return (await res.json()) as DaemonChannelReloadResult; | ||
| }, | ||
| opts?.timeoutMs, |
There was a problem hiding this comment.
[Suggestion] reloadChannelWorker falls back to the generic fetchTimeoutMs (default 30s), but the daemon-side operation can take up to ~37s (7s stop + 30s startup). Other long-running operations like restartMcpServer define a dedicated timeout constant (MCP_RESTART_DEFAULT_TIMEOUT_MS = MCP_RESTART_SERVER_DEADLINE_MS + MCP_RESTART_CLIENT_HEADROOM_MS) to avoid the client timing out before the server finishes.
Consider defining a CHANNEL_RELOAD_DEFAULT_TIMEOUT_MS constant (e.g., 45s) and using opts?.timeoutMs ?? CHANNEL_RELOAD_DEFAULT_TIMEOUT_MS here.
— qwen3.7-max via Qwen Code /review
|
|
||
| await runHandler({ 'daemon-url': 'http://daemon:9' }); | ||
|
|
||
| const line = mockWriteStderrLine.mock.calls[0]?.[0] as string; |
There was a problem hiding this comment.
[Suggestion] Two test coverage gaps for this command:
-
No test exercises the
QWEN_DAEMON_TOKENenv-var fallback — the source code atreload.ts:57falls back toQWEN_DAEMON_TOKENafterQWEN_SERVER_TOKEN, but all tests either pass--tokenor stubQWEN_SERVER_TOKEN. Add a test that stubs onlyQWEN_DAEMON_TOKEN(leavingQWEN_SERVER_TOKENundefined) and asserts the client is constructed with that token. -
No test verifies that the
--timeoutflag reaches the SDK method astimeoutMs. Add a test passing{ timeout: 5000 }and assertingmockReloadChannelWorkerwas called with{ timeoutMs: 5000 }.
— qwen3.7-max via Qwen Code /review
| await supervisor.stop(); | ||
| // start() bails if a child is still attached (stop cleared it) or if | ||
| // killAllSync latched `disposed` mid-reload — avoiding an orphaned | ||
| // fork. It also resets the restart budget, so a worker previously |
There was a problem hiding this comment.
[Suggestion] restart() resets the crash-restart budget (restartAttemptTimes = [] via start()) to let a failed worker recover on explicit reload — good behavior. However, this reset lives inside start() with no comment explaining that it serves the dual purpose of fresh-start initialization AND explicit-reload recovery. A future maintainer modifying start() might extract or remove that line thinking it's only for initial startup.
Consider adding a comment on restartAttemptTimes = [] in start() noting its dual role, or extracting it into a named helper like resetRestartBudget().
— qwen3.7-max via Qwen Code /review
|
|
||
| - The route calls `ChannelWorkerSupervisor.restart()` (`packages/cli/src/serve/channel-worker-supervisor.ts`), which stops the current worker child and relaunches it. The relaunched worker re-reads `settings.json`, so channel tokens, `proxy`, and per-channel `model` all take effect. | ||
| - Concurrent reloads coalesce onto a single stop+relaunch. `restart()` also resets the crash-restart budget, so a worker parked in `failed` recovers on an explicit reload. | ||
| - If the relaunch fails (for example, settings were edited into an invalid state), the channels stay down, the route returns 5xx with the latest snapshot, and `GET /daemon/status` reports `failed`. |
There was a problem hiding this comment.
[Suggestion] The implementation and route test currently return only the bridge error body on relaunch failure (sendBridgeError maps this to { error: 'relaunch failed' }), not the latest worker snapshot. This wording will make SDK/HTTP clients expect diagnostic data that the API does not provide. Either include the snapshot in the failure response, or narrow the docs to say the latest state is available from GET /daemon/status.
| - If the relaunch fails (for example, settings were edited into an invalid state), the channels stay down, the route returns 5xx with the latest snapshot, and `GET /daemon/status` reports `failed`. | |
| - If the relaunch fails (for example, settings were edited into an invalid state), the channels stay down, the route returns 5xx, and `GET /daemon/status` reports the latest worker state as `failed`. |
— GPT-5 via Qwen Code /review
5 participants
What this PR does
Adds an explicit way to reload the daemon-managed channel worker so it re-reads
settings.jsonwithout restarting the whole daemon. A runningqwen serve --channel ...daemon can now apply channel configuration changes — bot tokens, proxy, per-channel model — by stopping and relaunching just the channel worker, while the daemon process and its live sessions keep running.The reload is available three ways: an HTTP route
POST /workspace/channel/reload, an SDK methodreloadChannelWorker(), and a CLI commandqwen channel reload. It is advertised through a newchannel_reloadcapability that only appears when the daemon was started with--channel, so clients can feature-detect it before calling. The worker supervisor gains a reload operation that coalesces concurrent requests onto a single stop-and-relaunch, resets the crash-restart budget so a worker parked in a failed state recovers, and refuses to relaunch a worker into a daemon that is already being torn down.Why it's needed
Until now the channel worker read its configuration once at startup and stayed frozen for its lifetime, so any change to channel settings required restarting the entire daemon and dropping every session it hosts. That is disruptive for a long-running daemon that also exposes IM/channel entrypoints. This change lets an operator rotate a bot token, switch a model, or adjust a proxy and have the channels pick it up in place. It continues the worker-lifecycle hardening direction already tracked in the linked issue, alongside the existing heartbeat and bounded restart policy.
Reviewer Test Plan
How to verify
qwen serve --channel telegram. ConfirmGET /capabilitiesnow listschannel_reload; a daemon started without--channelmust not list it.GET /daemon/status(runtime.channelWorker.pid), change a channel value insettings.json(for example the model), then callPOST /workspace/channel/reload(or runqwen channel reload, or call the SDKreloadChannelWorker()). Expect200 {reloaded: true, worker: {...}}, the worker pid to change (proving a real relaunch), the channels to reconnect, and the new setting to be in effect.--channel, the route returns409 channel_worker_not_enabled. On a--tokendaemon, calling it without a bearer token returns401 token_requiredand does not touch the worker. If the edited settings are invalid, the relaunch fails, the route returns a 5xx, andGET /daemon/statusreports the worker asfailed.Automated coverage (all green locally): supervisor reload (stop-and-relaunch, concurrent-reload coalescing, failure-then-recovery, and the shutdown-race guard), the route (200 / 409 / 5xx / strict-auth, and capability advertised only when both worker dependencies are wired), the SDK method, and the CLI command.
cd packages/cli && npx vitest run src/serve/channel-worker-supervisor.test.ts src/commands/channel/reload.test.tscd packages/cli && npx vitest run src/serve/server.test.ts src/serve/run-qwen-serve.test.tscd packages/sdk-typescript && npx vitest run test/unit/DaemonClient.test.tsnpm run build && npm run typecheckEvidence (Before & After)
N/A — daemon HTTP / SDK / CLI surface, no TUI change.
Tested on
Environment (optional)
Local unit tests +
npm run build+npm run typecheckon macOS. Windows/Linux via CI.Risk & Scope
--channel <names>selection still needs a daemon restart, while--channel allpicks up newly-configured channels on reload; no changes topackages/core.--channel; behavior is unchanged when the flag is absent.Linked Issues
Refs #5976
中文说明
这个 PR 做了什么
新增一种显式方式，让 daemon 托管的 channel worker 重新读取
settings.json，而无需重启整个 daemon。运行中的qwen serve --channel ...daemon 现在可以通过仅停止并重启 channel worker 来应用 channel 配置变更（bot token、proxy、单个 channel 的 model），同时 daemon 进程及其在线会话保持不变。重载提供三种入口：HTTP 路由
POST /workspace/channel/reload、SDK 方法reloadChannelWorker()、CLI 命令qwen channel reload。它通过一个新的channel_reload能力位对外暴露，且仅在 daemon 以--channel启动时才广告，便于客户端在调用前做能力探测。worker supervisor 新增了一个重载操作：合并并发请求为一次 stop+relaunch、重置崩溃重启预算以便处于 failed 状态的 worker 得以恢复，并拒绝在 daemon 正在关闭时重新拉起 worker。为什么需要
此前 channel worker 只在启动时读取一次配置并在其生命周期内冻结，因此任何 channel 设置变更都需要重启整个 daemon 并丢弃它托管的所有会话。对于同时暴露 IM/channel 入口的长期运行 daemon，这非常不便。此改动让运维可以轮换 bot token、切换 model 或调整 proxy，并让 channel 就地生效。它延续了 linked issue 中已跟踪的 worker 生命周期加固方向，与已有的心跳与有界重启策略并列。
审查者测试计划
如何验证
qwen serve --channel telegram。确认GET /capabilities现在列出channel_reload；未带--channel启动的 daemon 不应列出它。GET /daemon/status（runtime.channelWorker.pid）记录 worker pid，修改settings.json中某个 channel 值（例如 model），然后调用POST /workspace/channel/reload（或运行qwen channel reload，或调用 SDKreloadChannelWorker()）。期望返回200 {reloaded: true, worker: {...}}，worker pid 变化（证明确实发生了重启），channel 重新连接，新设置生效。--channel的 daemon 上，该路由返回409 channel_worker_not_enabled。在--tokendaemon 上，不带 bearer token 调用返回401 token_required且不触碰 worker。若修改后的设置非法，重启失败，路由返回 5xx，GET /daemon/status将 worker 报告为failed。自动化覆盖（本地全部通过）：supervisor 重载（stop+relaunch、并发合并、失败后恢复、以及关闭竞态守卫）、路由（200 / 409 / 5xx / 严格鉴权，且仅在两个 worker 依赖都接入时才广告能力）、SDK 方法、CLI 命令。
证据（前后对比）
N/A —— daemon HTTP / SDK / CLI 层，无 TUI 变化。
测试平台
macOS 已测；Windows / Linux 由 CI 覆盖。
环境（可选）
macOS 上本地单元测试 +
npm run build+npm run typecheck。Windows/Linux 走 CI。风险与范围
--channel <names>选择集新增一个全新 channel 名仍需重启 daemon，而--channel all会在重载时纳入新配置的 channel；不改动packages/core。--channel门控；未带该 flag 时行为不变。关联 Issue
Refs #5976