Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 25 additions & 5 deletions docs/users/qwen-serve.md
Original file line number Diff line number Diff line change
Expand Up @@ -270,11 +270,31 @@ The token comparison is constant-time (SHA-256 + `crypto.timingSafeEqual`); 401
> "alive" until Node's keepalive probes time out — typically ~2 hours
> on Linux defaults. On `--hostname 0.0.0.0` deployments behind such
> NATs, phantom SSE connections can accumulate and eventually hit the
> 256 `server.maxConnections` ceiling. Stage 2 will add an
> application-level idle deadline (last-byte-written tracking +
> per-connection timeout). Until then, operators on networks that
> swallow RSTs may want to lower `server.keepAliveTimeout` via a
> reverse proxy or accept periodic daemon restarts.
> 256 `server.maxConnections` ceiling.
>
> Set [`--writer-idle-timeout-ms <n>`](#deadlines-and-writer-idle-timeout)
> (issue [#4514](https://github.com/QwenLM/qwen-code/issues/4514) T2.9)
> to close the gap with an explicit application-level idle deadline:
> when no write has successfully flushed for `n` ms the daemon emits
> a terminal `client_evicted` frame with
> `reason: 'writer_idle_timeout'` and closes the stream. The flag is
> off by default to preserve the legacy contract — operators on
> networks that swallow RSTs should pick a value well above the 15s
> heartbeat interval (e.g. `60000`–`300000`) so legitimate idle
> connections aren't evicted while genuinely stuck writers are
> reaped promptly. Pre-flight `caps.features.includes('writer_idle_timeout')`
> from your SDK to confirm the daemon supports it.

### Deadlines and writer idle timeout

Issue [#4514](https://github.com/QwenLM/qwen-code/issues/4514) T2.9 ships two opt-in flags that close the long-running / remote-deployment gaps the 15s heartbeat + AbortSignal don't cover. Both are off by default — single-user loopback workflows stay bit-for-bit unchanged.

| Flag | Env var | Default | What it does |
| ------------------------------ | ----------------------------------- | ------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `--prompt-deadline-ms <n>` | `QWEN_SERVE_PROMPT_DEADLINE_MS` | unset | Server-side wallclock cap on a single `POST /session/:id/prompt`. On expiry the daemon aborts the prompt's AbortController and returns HTTP `504` with `{code:"prompt_deadline_exceeded", errorKind:"prompt_deadline_exceeded", deadlineMs:n}`. A per-prompt request body field `deadlineMs` can SHORTEN the effective deadline below the flag but never extend it. Capability tag (conditional): `prompt_absolute_deadline`. |
| `--writer-idle-timeout-ms <n>` | `QWEN_SERVE_WRITER_IDLE_TIMEOUT_MS` | unset | Per-SSE-connection idle deadline. When no write has SUCCESSFULLY flushed for `n` ms — neither a real event nor the 15s heartbeat — the daemon emits a terminal `client_evicted` frame with `data.reason = 'writer_idle_timeout'` (mirrored on `data.errorKind`) and closes the stream. **Pick a value comfortably above the 15s heartbeat** (e.g. `30000`–`300000`) so legitimate idle streams aren't evicted; values `< 15000` WILL evict otherwise-healthy idle connections before the first heartbeat fires (intentional only for tests / short-lived dev sessions). Capability tag (conditional): `writer_idle_timeout`. |

Both flags accept a positive integer in milliseconds; `0`, `NaN`, non-integer, or negative values are rejected at boot with a clear error message. CLI flag wins over env var; explicit `ServeOptions` field (embedded callers) wins over env. SDK consumers should pre-flight the matching capability tag before relying on either behavior — daemons predating this PR omit both tags and the request `deadlineMs` field is silently dropped.

## Multi-session & multi-workspace deployment

Expand Down
11 changes: 8 additions & 3 deletions packages/acp-bridge/src/status.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@ describe('SERVE_ERROR_KINDS', () => {
// kinds; PR 14 added `'budget_exhausted'` for MCP guardrail
// refusals (see #4175 PR 14); PR 16 added `'stat_failed'` for
// non-ENOENT stat failures on workspace memory discovery (see
// #4175 PR 16). Future additions append to this list — the
// order is part of the contract so SDK consumers can pattern-
// match without per-kind lookups.
// #4175 PR 16). Issue #4514 T2.9 appended
// `'prompt_deadline_exceeded'` (POST /session/:id/prompt 504) and
// `'writer_idle_timeout'` (terminal SSE client_evicted frame).
// Future additions append to this list — the order is part of the
// contract so SDK consumers can pattern-match without per-kind
// lookups.
expect(SERVE_ERROR_KINDS).toEqual([
'missing_binary',
'blocked_egress',
Expand All @@ -33,6 +36,8 @@ describe('SERVE_ERROR_KINDS', () => {
'parse_error',
'stat_failed',
'budget_exhausted',
'prompt_deadline_exceeded',
'writer_idle_timeout',
]);
});
});
Expand Down
13 changes: 13 additions & 0 deletions packages/acp-bridge/src/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,19 @@ export const SERVE_ERROR_KINDS = [
// Surfaced on per-server `mcp_server` cells (refused at discovery)
// and on the workspace-level `mcp_budget` cell (any refusal this pass).
'budget_exhausted',
// Issue #4514 T2.9: a prompt exceeded the server-configured wallclock
// cap (`--prompt-deadline-ms`) or the request's own `deadlineMs`
// (capped at the server flag). Surfaced on the
// `POST /session/:id/prompt` 504 response so callers can branch on a
// typed kind instead of regex-matching the message.
'prompt_deadline_exceeded',
Comment thread
doudouOUC marked this conversation as resolved.
// Issue #4514 T2.9: an SSE writer's last successful flush was older
// than `--writer-idle-timeout-ms`. The daemon emits a terminal
// `client_evicted` frame with `reason: 'writer_idle_timeout'` before
// tearing the connection down; the kind appears on the frame payload
// (not an HTTP response — by the time we detect the stall the stream
// is already in flight).
'writer_idle_timeout',
] as const;

export type ServeErrorKind = (typeof SERVE_ERROR_KINDS)[number];
Expand Down
35 changes: 21 additions & 14 deletions packages/cli/src/commands/serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ interface ServeArgs {
'mcp-client-budget'?: number;
'mcp-budget-mode'?: 'enforce' | 'warn' | 'off';
'allow-origin'?: string[];
'prompt-deadline-ms'?: number;
'writer-idle-timeout-ms'?: number;
}

export const serveCommand: CommandModule<unknown, ServeArgs> = {
Expand Down Expand Up @@ -150,17 +152,19 @@ export const serveCommand: CommandModule<unknown, ServeArgs> = {
type: 'string',
array: true,
description:
'T2.4 (#4514). Cross-origin allowlist for browser webui clients. ' +
'Repeatable; each value must be a canonical URL origin ' +
'(`<scheme>://<host>[:<port>]`, no trailing slash) or `*` for any ' +
'origin (loud warning; boot refuses if no bearer token is ' +
'configured. Recommended: pair with --require-auth on loopback so ' +
'/health and /demo are also bearer-gated). When unset, ' +
'the daemon rejects every request carrying an `Origin` header with ' +
"403 (today's behavior). Matched origins receive proper CORS " +
'response headers; unmatched still 403. Example: `--allow-origin ' +
'http://localhost:3000 --allow-origin http://localhost:5173`. ' +
'Pre-flight via `caps.features.allow_origin`.',
'T2.4 (#4514). Cross-origin allowlist for browser webui clients.',
})
.option('prompt-deadline-ms', {
type: 'number',
description:
'T2.9 (#4514). Server-side wallclock cap on POST /session/:id/prompt (ms). ' +
'Falls back to QWEN_SERVE_PROMPT_DEADLINE_MS. Positive integer.',
})
.option('writer-idle-timeout-ms', {
type: 'number',
description:
'T2.9 (#4514). Per-SSE-connection idle deadline (ms). ' +
'Falls back to QWEN_SERVE_WRITER_IDLE_TIMEOUT_MS. Positive integer.',
}) as unknown as Argv<ServeArgs>,
handler: async (argv) => {
if (!argv['http-bridge']) {
Expand Down Expand Up @@ -236,12 +240,15 @@ export const serveCommand: CommandModule<unknown, ServeArgs> = {
requireAuth: argv['require-auth'],
mcpClientBudget,
mcpBudgetMode: resolvedMcpMode,
// T2.4 (#4514). Pass through verbatim; runQwenServe re-runs
// `parseAllowOriginPatterns` at boot so a malformed entry is
// rejected before the listener binds.
...(argv['allow-origin'] && argv['allow-origin'].length > 0
? { allowOrigins: argv['allow-origin'] }
: {}),
...(argv['prompt-deadline-ms'] !== undefined
? { promptDeadlineMs: argv['prompt-deadline-ms'] }
: {}),
...(argv['writer-idle-timeout-ms'] !== undefined
? { writerIdleTimeoutMs: argv['writer-idle-timeout-ms'] }
: {}),
});
} catch (err) {
writeStderrLine(
Expand Down
46 changes: 17 additions & 29 deletions packages/cli/src/serve/capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,42 +221,27 @@ export const SERVE_CAPABILITY_REGISTRY = {
// status route (extension data on `/capabilities` would inflate the
// descriptor shape; we keep the registry uniform).
auth_device_flow: { since: 'v1' },
// #4175 F3 (Commit 6). Daemon advertises which permission mediation
// policies it can run. Clients introspect `modes` to discover the
// closed set of strategies before relying on `permission_partial_vote`
// / `permission_forbidden` SSE events. The active policy for THIS
// daemon is exposed in the `/capabilities` envelope's
// `policy.permission` field — the mode list here is the
// build-supported set, distinct from runtime configuration.
permission_mediation: {
since: 'v1',
modes: ['first-responder', 'designated', 'consensus', 'local-only'],
},
prompt_absolute_deadline: { since: 'v1' },
writer_idle_timeout: { since: 'v1' },
} as const satisfies Record<string, ServeCapabilityDescriptor>;

export type ServeFeature = keyof typeof SERVE_CAPABILITY_REGISTRY;

/**
* Per-deployment feature toggles surfaced through `/capabilities`.
*
* `requireAuth` controls whether the conditional `require_auth` tag is
* advertised. `mcpPoolActive` (F2 #4175 commit 5) advertises
* `mcp_workspace_pool` + `mcp_pool_restart` together when the daemon
* runs with the pool enabled (default; off only with
* `QWEN_SERVE_NO_MCP_POOL=1`). Other Wave 4 follow-ups can extend
* this object as more deployment-shape capability tags appear (e.g.
* `redact_errors`).
* advertised.
*/
export interface AdvertiseFeatureToggles {
requireAuth?: boolean;
mcpPoolActive?: boolean;
/**
* T2.4 (issue #4514). `true` iff the daemon booted with at least one
* `--allow-origin <pattern>` entry. Drives advertisement of the
* `allow_origin` capability tag so SDK / webui clients can pre-flight
* the cross-origin path.
*/
allowOriginActive?: boolean;
promptDeadlineMs?: number;
writerIdleTimeoutMs?: number;
}

/**
Expand Down Expand Up @@ -296,18 +281,21 @@ export const CONDITIONAL_SERVE_FEATURES: ReadonlyMap<
(toggles: AdvertiseFeatureToggles) => boolean
> = new Map<ServeFeature, (toggles: AdvertiseFeatureToggles) => boolean>([
['require_auth', (toggles) => toggles.requireAuth === true],
// F2 (#4175 commit 5): pool tags advertise as a unit. Both keys
// share the same predicate so SDK clients can rely on
// `mcp_workspace_pool ⇒ entryCount/entrySummary fields present`
// and `mcp_pool_restart ⇒ ?entryIndex= + entries[] response shape
// valid` without per-tag pre-flighting.
['mcp_workspace_pool', (toggles) => toggles.mcpPoolActive === true],
['mcp_pool_restart', (toggles) => toggles.mcpPoolActive === true],
// T2.4 (issue #4514): the `allow_origin` tag tracks whether the
// daemon was booted with at least one `--allow-origin` pattern. SDK
// clients pre-flight on it before issuing a cross-origin request
// they would otherwise expect to be 403'd by `denyBrowserOriginCors`.
['allow_origin', (toggles) => toggles.allowOriginActive === true],
[
'prompt_absolute_deadline',
(toggles) =>
typeof toggles.promptDeadlineMs === 'number' &&
toggles.promptDeadlineMs > 0,
],
[
'writer_idle_timeout',
(toggles) =>
typeof toggles.writerIdleTimeoutMs === 'number' &&
toggles.writerIdleTimeoutMs > 0,
],
]);

export const SERVE_FEATURES = Object.freeze(
Expand Down
101 changes: 90 additions & 11 deletions packages/cli/src/serve/runQwenServe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,28 +20,66 @@ import { createBridgeFileSystemAdapter } from './bridgeFileSystemAdapter.js';
import { createDaemonStatusProvider } from './daemonStatusProvider.js';
import { isLoopbackBind } from './loopbackBinds.js';
import { parseAllowOriginPatterns } from './auth.js';
import {
createPermissionAuditPublisher,
PermissionAuditRing,
} from './permissionAudit.js';
import { createServeApp, resolveBridgeFsFactory } from './server.js';
// Wenshao review #4335 / 3272581563 — single runtime source of
// truth for the closed permission-policy set. `validatePolicyConfig`
// derives its valid-set from `permission_mediation.modes` so a
// future 5th policy lands in one place.
import { SERVE_CAPABILITY_REGISTRY } from './capabilities.js';
import type { ServeOptions } from './types.js';
import type { WorkspaceFileSystemFactory } from './fs/index.js';
// Wenshao review #4335 / 3272493805 — use the canonical
// `PermissionPolicy` union from acp-bridge instead of inlining
// the four string literals at the let-declaration, the `as`
// cast, and the validation Set. Same drift-protection rationale
// as types.ts/3271978342.
import type { PermissionPolicy } from '@qwen-code/acp-bridge';

const QWEN_SERVER_TOKEN_ENV = 'QWEN_SERVER_TOKEN';
const QWEN_SERVE_PROMPT_DEADLINE_MS_ENV = 'QWEN_SERVE_PROMPT_DEADLINE_MS';
const QWEN_SERVE_WRITER_IDLE_TIMEOUT_MS_ENV =
'QWEN_SERVE_WRITER_IDLE_TIMEOUT_MS';
const SHUTDOWN_FORCE_CLOSE_MS = 5_000;

function isPositiveIntegerMs(value: number): boolean {
return Number.isFinite(value) && Number.isInteger(value) && value > 0;
}

const MAX_TIMEOUT_MS = 2_147_483_647;

function assertTimerDelayInRange(name: string, value: number): void {
if (value > MAX_TIMEOUT_MS) {
throw new TypeError(
`Invalid ${name}: ${value}. Exceeds maximum JS timer delay of ` +
`${MAX_TIMEOUT_MS} ms (~24.8 days); Node would silently ` +
`compress longer delays to 1ms.`,
);
}
}

/**
* Issue #4514 T2.9. Resolve a positive-integer millisecond value from
* an env var. Returns `undefined` when the var is absent (caller falls
* back to the CLI option / `ServeOptions` field), throws when the var
* is present but malformed so a typo fails the boot loudly instead of
* silently disabling the deadline. Whitespace-only values are also
* treated as malformed (rather than silently "unset") — the JSDoc
* "fail loud on typo" promise applies symmetrically.
*/
function parseDeadlineEnv(
envName: string,
raw: string | undefined,
): number | undefined {
if (raw === undefined) return undefined;
// Don't early-return on empty/whitespace: `Number('')` and
// `Number(' ')` both yield `0`, which the positive-integer check
// below rejects with the standard error message. Silently treating
// `QWEN_SERVE_PROMPT_DEADLINE_MS=" "` as "not set" would let a
// shell-substitution typo slip past.
const trimmed = raw.trim();
const parsed = Number(trimmed);
if (!isPositiveIntegerMs(parsed)) {
throw new Error(
`Invalid ${envName}="${raw}": must be a positive integer (milliseconds).`,
);
}
return parsed;
}

/**
* Wenshao review #4335 / 3271978374 — boot-time policy validation
* errors. Replaces the previous substring-matching of "invalid
Expand Down Expand Up @@ -321,7 +359,28 @@ export async function runQwenServe(
typeof rawToken === 'string' && rawToken.trim().length > 0
? rawToken.trim()
: undefined;
const opts: ServeOptions = { ...optsIn, token };
// T2.9: env-var fallback for the deadline options. Explicit option
// beats the env beats unset (= unlimited). `parseDeadlineEnv` throws
// on malformed values so an `export QWEN_SERVE_PROMPT_DEADLINE_MS=abc`
// typo fails boot loudly instead of silently disabling the cap.
const promptDeadlineMs =
optsIn.promptDeadlineMs ??
parseDeadlineEnv(
QWEN_SERVE_PROMPT_DEADLINE_MS_ENV,
process.env[QWEN_SERVE_PROMPT_DEADLINE_MS_ENV],
);
const writerIdleTimeoutMs =
Comment thread
doudouOUC marked this conversation as resolved.
optsIn.writerIdleTimeoutMs ??
parseDeadlineEnv(
QWEN_SERVE_WRITER_IDLE_TIMEOUT_MS_ENV,
process.env[QWEN_SERVE_WRITER_IDLE_TIMEOUT_MS_ENV],
);
const opts: ServeOptions = {
...optsIn,
token,
...(promptDeadlineMs !== undefined ? { promptDeadlineMs } : {}),
...(writerIdleTimeoutMs !== undefined ? { writerIdleTimeoutMs } : {}),
};

// BU-sh: catch the `--hostname localhost:4170` / `127.0.0.1:4170`
// typo BEFORE the loopback / token check so the operator sees a
Expand Down Expand Up @@ -498,6 +557,26 @@ export async function runQwenServe(
'Pass mcpClientBudget=N, or set mcpBudgetMode to "warn" or "off".',
);
}
// T2.9: validate the deadline options on the explicit option path.
// The env path is already validated inside `parseDeadlineEnv`. Boot-
// loud so an embedded caller passing `{ promptDeadlineMs: -5 }`
// doesn't end up with a daemon that silently fails to enforce the
// cap, leaving the operator believing the timeout is active.
if (opts.promptDeadlineMs !== undefined) {
if (!isPositiveIntegerMs(opts.promptDeadlineMs)) {
throw new TypeError(
`Invalid promptDeadlineMs: ${opts.promptDeadlineMs}. Must be a positive integer (milliseconds).`,
);
}
assertTimerDelayInRange('promptDeadlineMs', opts.promptDeadlineMs);
}
if (opts.writerIdleTimeoutMs !== undefined) {
if (!isPositiveIntegerMs(opts.writerIdleTimeoutMs)) {
throw new TypeError(
`Invalid writerIdleTimeoutMs: ${opts.writerIdleTimeoutMs}. Must be a positive integer (milliseconds).`,
);
}
}
// Per-handle env overrides: `undefined` value means "scrub this
// var from the child env" — important when a different daemon
// in the same process set the var globally previously. Always
Expand Down
Loading