-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(cli): Add channel worker settings reload for serve --channel #6598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -162,6 +162,15 @@ sequenceDiagram | |||||
| - `shutdown()` closes every active session and the underlying transport (the channel's WebSocket / long-poll). | ||||||
| - DingTalk's WebSocket stream supports server-push; WeChat's long-poll requires a backoff strategy on idle responses; Telegram's long-poll has a built-in `timeout` parameter. | ||||||
|
|
||||||
| ### 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`): | ||||||
|
|
||||||
| - 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`. | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The implementation and route test currently return only the bridge error body on relaunch failure (
Suggested change
— GPT-5 via Qwen Code /review |
||||||
| - The `channel_reload` capability and the route are advertised only when the daemon was started with `--channel`. Adding a brand-new channel name to a `--channel <names>` selection still requires a daemon restart; `--channel all` picks up newly-configured channels on reload. | ||||||
|
|
||||||
| ## Dependencies | ||||||
|
|
||||||
| - `packages/channels/base/` — `ChannelBase`, `DaemonChannelBridge`, `types.ts` (`ChannelConfig`, `Envelope`, `SessionScope`, `ChannelPlugin`). | ||||||
|
|
@@ -196,6 +205,8 @@ Channel-specific keys layer on top (DingTalk: `streamCredentials`; WeChat: `ilin | |||||
| - `packages/channels/base/src/DaemonChannelBridge.ts` | ||||||
| - `packages/channels/base/src/ChannelBase.ts` | ||||||
| - `packages/channels/base/src/types.ts` | ||||||
| - `packages/cli/src/serve/channel-worker-supervisor.ts` (worker supervision + `restart()`) | ||||||
| - `packages/cli/src/serve/routes/workspace-channel-control.ts` (`POST /workspace/channel/reload`) | ||||||
| - `packages/channels/dingtalk/src/DingtalkAdapter.ts` | ||||||
| - `packages/channels/weixin/src/WeixinAdapter.ts` | ||||||
| - `packages/channels/telegram/src/TelegramAdapter.ts` | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| const mockReloadChannelWorker = vi.hoisted(() => vi.fn()); | ||
| const mockDaemonClient = vi.hoisted(() => | ||
| vi.fn(() => ({ reloadChannelWorker: mockReloadChannelWorker })), | ||
| ); | ||
| const mockWriteStdoutLine = vi.hoisted(() => vi.fn()); | ||
| const mockWriteStderrLine = vi.hoisted(() => vi.fn()); | ||
|
|
||
| vi.mock('@qwen-code/sdk/daemon', () => ({ | ||
| DaemonClient: mockDaemonClient, | ||
| })); | ||
| vi.mock('../../utils/stdioHelpers.js', () => ({ | ||
| writeStdoutLine: mockWriteStdoutLine, | ||
| writeStderrLine: mockWriteStderrLine, | ||
| })); | ||
|
|
||
| import { reloadCommand } from './reload.js'; | ||
|
|
||
| type ReloadHandler = NonNullable<typeof reloadCommand.handler>; | ||
|
|
||
| async function runHandler(argv: Record<string, unknown>): Promise<void> { | ||
| await (reloadCommand.handler as ReloadHandler)({ | ||
| _: [], | ||
| $0: 'qwen', | ||
| ...argv, | ||
| } as never); | ||
| } | ||
|
|
||
| describe('channel reload command', () => { | ||
| beforeEach(() => { | ||
| vi.spyOn(process, 'exit').mockImplementation((() => undefined) as never); | ||
| vi.unstubAllEnvs(); | ||
| vi.stubEnv('QWEN_DAEMON_URL', undefined); | ||
| vi.stubEnv('QWEN_SERVER_TOKEN', undefined); | ||
| vi.stubEnv('QWEN_DAEMON_TOKEN', undefined); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.restoreAllMocks(); | ||
| vi.unstubAllEnvs(); | ||
| mockReloadChannelWorker.mockReset(); | ||
| mockDaemonClient.mockClear(); | ||
| mockWriteStdoutLine.mockClear(); | ||
| mockWriteStderrLine.mockClear(); | ||
| }); | ||
|
|
||
| it('reloads via the resolved daemon URL/token and prints the snapshot', async () => { | ||
| mockReloadChannelWorker.mockResolvedValue({ | ||
| reloaded: true, | ||
| worker: { | ||
| enabled: true, | ||
| state: 'running', | ||
| channels: ['telegram'], | ||
| pid: 4321, | ||
| restartCount: 2, | ||
| }, | ||
| }); | ||
|
|
||
| await runHandler({ 'daemon-url': 'http://daemon:9', token: 'secret' }); | ||
|
|
||
| expect(mockDaemonClient).toHaveBeenCalledWith({ | ||
| baseUrl: 'http://daemon:9', | ||
| token: 'secret', | ||
| }); | ||
| expect(mockReloadChannelWorker).toHaveBeenCalledTimes(1); | ||
| const line = mockWriteStdoutLine.mock.calls[0]?.[0] as string; | ||
| expect(line).toContain('state=running'); | ||
| expect(line).toContain('channels=telegram'); | ||
| expect(line).toContain('pid=4321'); | ||
| expect(line).toContain('restarts=2'); | ||
| expect(process.exit).toHaveBeenCalledWith(0); | ||
| expect(mockWriteStderrLine).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('defaults the daemon URL and omits the token when neither flag nor env is set', async () => { | ||
| mockReloadChannelWorker.mockResolvedValue({ | ||
| reloaded: true, | ||
| worker: { enabled: true, state: 'running', channels: [] }, | ||
| }); | ||
|
|
||
| await runHandler({}); | ||
|
|
||
| expect(mockDaemonClient).toHaveBeenCalledWith({ | ||
| baseUrl: 'http://127.0.0.1:4170', | ||
| }); | ||
| }); | ||
|
|
||
| it('falls back to QWEN_DAEMON_URL and QWEN_SERVER_TOKEN from the environment', async () => { | ||
| vi.stubEnv('QWEN_DAEMON_URL', 'http://env-daemon:5'); | ||
| vi.stubEnv('QWEN_SERVER_TOKEN', 'env-token'); | ||
| mockReloadChannelWorker.mockResolvedValue({ | ||
| reloaded: true, | ||
| worker: { enabled: true, state: 'running', channels: [] }, | ||
| }); | ||
|
|
||
| await runHandler({}); | ||
|
|
||
| expect(mockDaemonClient).toHaveBeenCalledWith({ | ||
| baseUrl: 'http://env-daemon:5', | ||
| token: 'env-token', | ||
| }); | ||
| }); | ||
|
|
||
| it('reports failures on stderr and exits non-zero', async () => { | ||
| mockReloadChannelWorker.mockRejectedValue(new Error('no channel worker')); | ||
|
|
||
| await runHandler({ 'daemon-url': 'http://daemon:9' }); | ||
|
|
||
| const line = mockWriteStderrLine.mock.calls[0]?.[0] as string; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Two test coverage gaps for this command:
— qwen3.7-max via Qwen Code /review |
||
| expect(line).toContain('Reload failed'); | ||
| expect(line).toContain('no channel worker'); | ||
| expect(process.exit).toHaveBeenCalledWith(1); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,123 @@ | ||||||||||||||||||||
| import type { CommandModule } from 'yargs'; | ||||||||||||||||||||
| import { writeStderrLine, writeStdoutLine } from '../../utils/stdioHelpers.js'; | ||||||||||||||||||||
| import { | ||||||||||||||||||||
| QWEN_DAEMON_TOKEN_ENV, | ||||||||||||||||||||
| QWEN_DAEMON_URL_ENV, | ||||||||||||||||||||
| QWEN_SERVER_TOKEN_ENV, | ||||||||||||||||||||
| } from '../../serve/channel-worker-env.js'; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const DEFAULT_DAEMON_URL = 'http://127.0.0.1:4170'; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Structural subset of the SDK's DaemonChannelReloadResult. Kept local (like | ||||||||||||||||||||
| // daemon-worker.ts's DaemonSdkLike) so this command doesn't take a static | ||||||||||||||||||||
| // type dependency on the SDK subpath. | ||||||||||||||||||||
| interface ChannelReloadResultLike { | ||||||||||||||||||||
| reloaded: boolean; | ||||||||||||||||||||
| worker: { | ||||||||||||||||||||
| state: string; | ||||||||||||||||||||
| channels: string[]; | ||||||||||||||||||||
| pid?: number; | ||||||||||||||||||||
| restartCount?: number; | ||||||||||||||||||||
| error?: string; | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| interface DaemonClientLike { | ||||||||||||||||||||
| reloadChannelWorker(opts?: { | ||||||||||||||||||||
| clientId?: string; | ||||||||||||||||||||
| timeoutMs?: number; | ||||||||||||||||||||
| }): Promise<ChannelReloadResultLike>; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| interface DaemonSdkLike { | ||||||||||||||||||||
| DaemonClient: new (opts: { | ||||||||||||||||||||
| baseUrl: string; | ||||||||||||||||||||
| token?: string; | ||||||||||||||||||||
| }) => DaemonClientLike; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| interface ReloadArgs { | ||||||||||||||||||||
| 'daemon-url'?: string; | ||||||||||||||||||||
| token?: string; | ||||||||||||||||||||
| timeout?: number; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function resolveDaemonUrl(flag: string | undefined): string { | ||||||||||||||||||||
| // `||` (not `??`) so an empty flag or empty env var falls through to the | ||||||||||||||||||||
| // default rather than producing an unusable empty base URL. | ||||||||||||||||||||
| return flag || process.env[QWEN_DAEMON_URL_ENV] || DEFAULT_DAEMON_URL; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function resolveToken(flag: string | undefined): string | undefined { | ||||||||||||||||||||
| return ( | ||||||||||||||||||||
| flag ?? | ||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||||||||||
| process.env[QWEN_SERVER_TOKEN_ENV] ?? | ||||||||||||||||||||
| process.env[QWEN_DAEMON_TOKEN_ENV] | ||||||||||||||||||||
| ); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export const reloadCommand: CommandModule<unknown, ReloadArgs> = { | ||||||||||||||||||||
| command: 'reload', | ||||||||||||||||||||
| describe: | ||||||||||||||||||||
| 'Reload the daemon-managed channel worker so it re-reads settings.json', | ||||||||||||||||||||
| builder: (yargs) => | ||||||||||||||||||||
| yargs | ||||||||||||||||||||
| .option('daemon-url', { | ||||||||||||||||||||
| type: 'string', | ||||||||||||||||||||
| description: `Daemon base URL (default: $${QWEN_DAEMON_URL_ENV} or ${DEFAULT_DAEMON_URL})`, | ||||||||||||||||||||
| }) | ||||||||||||||||||||
| .option('token', { | ||||||||||||||||||||
| type: 'string', | ||||||||||||||||||||
| description: `Bearer token (default: $${QWEN_SERVER_TOKEN_ENV})`, | ||||||||||||||||||||
| }) | ||||||||||||||||||||
| .option('timeout', { | ||||||||||||||||||||
| type: 'number', | ||||||||||||||||||||
| description: 'Request timeout in milliseconds', | ||||||||||||||||||||
| }), | ||||||||||||||||||||
| handler: async (argv) => { | ||||||||||||||||||||
| const baseUrl = resolveDaemonUrl(argv['daemon-url']); | ||||||||||||||||||||
| const token = resolveToken(argv.token); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| let sdk: DaemonSdkLike; | ||||||||||||||||||||
| try { | ||||||||||||||||||||
| sdk = (await import('@qwen-code/sdk/daemon')) as unknown as DaemonSdkLike; | ||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||
| writeStderrLine( | ||||||||||||||||||||
| `[Channel] Failed to load daemon SDK: ${ | ||||||||||||||||||||
| err instanceof Error ? err.message : String(err) | ||||||||||||||||||||
| }`, | ||||||||||||||||||||
| ); | ||||||||||||||||||||
| process.exit(1); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const client = new sdk.DaemonClient({ | ||||||||||||||||||||
| baseUrl, | ||||||||||||||||||||
| ...(token ? { token } : {}), | ||||||||||||||||||||
| }); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| try { | ||||||||||||||||||||
| const result = await client.reloadChannelWorker( | ||||||||||||||||||||
| argv.timeout !== undefined ? { timeoutMs: argv.timeout } : undefined, | ||||||||||||||||||||
| ); | ||||||||||||||||||||
| const worker = result.worker; | ||||||||||||||||||||
| const parts = [ | ||||||||||||||||||||
| `state=${worker.state}`, | ||||||||||||||||||||
| `channels=${worker.channels.join(', ') || 'none'}`, | ||||||||||||||||||||
| ...(worker.pid !== undefined ? [`pid=${worker.pid}`] : []), | ||||||||||||||||||||
| ...(worker.restartCount !== undefined | ||||||||||||||||||||
| ? [`restarts=${worker.restartCount}`] | ||||||||||||||||||||
| : []), | ||||||||||||||||||||
| ...(worker.error ? [`error=${worker.error}`] : []), | ||||||||||||||||||||
| ]; | ||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The CLI always exits 0 after a successful HTTP round-trip, even when
Suggested change
— qwen3.7-max via Qwen Code /review |
||||||||||||||||||||
| writeStdoutLine(`[Channel] Reloaded (${parts.join(', ')}).`); | ||||||||||||||||||||
| process.exit(0); | ||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||
| writeStderrLine( | ||||||||||||||||||||
| `[Channel] Reload failed (${baseUrl}): ${ | ||||||||||||||||||||
| err instanceof Error ? err.message : String(err) | ||||||||||||||||||||
| }`, | ||||||||||||||||||||
| ); | ||||||||||||||||||||
| process.exit(1); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| }; | ||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 includeworker/snapshot data. Please either include the snapshot in the 5xx response or narrow this doc to say callers should useGET /daemon/statusfor the latest snapshot after a failed reload.