Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
56 changes: 56 additions & 0 deletions .qwen/design/serve-server-split.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# serve server.ts staged split

## Goal

Split `packages/cli/src/serve/server.ts` in stages without changing daemon behavior. The first stage extracts shared helpers and route groups whose boundaries are already clear, while keeping `createServeApp()` responsible for wiring middleware, stateful dependencies, transport mounts, and final error handling.

## Middleware And Route Order

The app assembly order is part of the public behavior and must stay stable:

1. same-origin `Origin` stripping
2. CORS and host allowlist
3. pre-auth `/health` and `/demo` on allowed loopback setups
4. access logging
5. Web Shell static assets
6. bearer auth
7. rate limit
8. JSON body parser and JSON parser error mapper
9. post-auth `/health` and `/demo` when required
10. daemon telemetry
11. REST route groups
12. ACP HTTP and WebSocket routes
13. Web Shell fallback
14. final error handler

## Extracted Boundaries

`server/request-helpers.ts` owns request-body sanitization, client-id parsing, loopback detection, path/query validators, and permission vote body parsing. Route modules depend on this file instead of importing from `server.ts`.

`server/error-response.ts` owns bridge error taxonomy and HTTP response mapping. The exported wrappers accept an optional daemon logger so route modules can keep the existing stderr and daemon-log behavior.

`server/session-list.ts` owns the persisted-plus-live session list merge used by both REST and ACP HTTP callers.

`server/fs-factory.ts` owns default workspace filesystem factory construction and fs audit warning emission.

`server/telemetry.ts` owns route classification and daemon HTTP telemetry middleware.

`server/prompt-deadline.ts` owns prompt deadline resolution and its abort sentinel class.

Route modules follow the existing `registerXRoutes(app, deps)` style. They receive only the dependencies they need, not a single god context.

## Non-goals

This stage does not change response bodies, status codes, headers, SSE frame format, authentication order, or error taxonomy. It does not delete compatibility re-export shims such as `status.ts`, `event-bus.ts`, or `in-memory-channel.ts`. It does not rename historical docs or cleanup unrelated camelCase paths.

`server.ts` may remain over 200 lines after this stage. The acceptance criterion is stable boundaries that make later session and SSE extraction mechanical.

## Audit Notes

Round 1 checked architecture boundaries and rejected a new Router abstraction because existing route modules already use direct `registerXRoutes(app, deps)` functions.

Round 2 checked failure paths and kept error taxonomy in one helper so route extraction cannot silently drift HTTP status codes.

Round 3 checked compatibility and keeps the public exports consumed by `run-qwen-serve.ts`, ACP HTTP dispatch, and tests.

Round 4 checked testing strategy and relies on focused `server.test.ts`, ACP HTTP, and route tests because this is structural refactoring with no user-visible behavior change.
2 changes: 1 addition & 1 deletion packages/cli/src/serve/rate-limit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ function resolveTier(method: string, path: string): RateLimitTier | null {
// Key Extraction
// ---------------------------------------------------------------------------

// Keep in sync with server.ts CLIENT_ID_RE / MAX_CLIENT_ID_LENGTH.
// Keep in sync with server/request-helpers.ts CLIENT_ID_RE / MAX_CLIENT_ID_LENGTH.
const MAX_CLIENT_ID_LENGTH = 128;
const CLIENT_ID_RE = /^[A-Za-z0-9._:-]+$/;

Expand Down
88 changes: 88 additions & 0 deletions packages/cli/src/serve/routes/daemon-status.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/**
* @license
* Copyright 2025 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/

import type { Application } from 'express';
import { writeStderrLine } from '../../utils/stdioHelpers.js';
import type { AcpHttpHandle } from '../acp-http/index.js';
import type { AcpSessionBridge } from '../acp-session-bridge.js';
import type {
DeviceFlowProviderId,
DeviceFlowRegistry,
} from '../auth/device-flow.js';
import type { DaemonLogger } from '../daemon-logger.js';
import {
buildDaemonStatusResponse,
type DaemonStartupSnapshot,
parseDaemonStatusDetail,
} from '../daemon-status.js';
import type { RateLimiterInstance } from '../rate-limit.js';
import type { ServeOptions } from '../types.js';
import type { DaemonWorkspaceService } from '../workspace-service/index.js';
import { getServeProtocolVersions } from '../capabilities.js';

interface RegisterDaemonStatusRoutesDeps {
opts: ServeOptions;
boundWorkspace: string;
bridge: AcpSessionBridge;
workspace: DaemonWorkspaceService;
daemonLog?: DaemonLogger;
startup?: DaemonStartupSnapshot;
qwenCodeVersion?: string;
getAcpHandle: () => AcpHttpHandle | undefined;
getRateLimiter: () => RateLimiterInstance | undefined;
getRestSseActive: () => number;
currentServeFeatures: () => ReturnType<
typeof import('../capabilities.js').getAdvertisedServeFeatures
>;
getSupportedDeviceFlowProviders: () => DeviceFlowProviderId[];
deviceFlowRegistry: DeviceFlowRegistry;
sessionShellCommandEnabled: boolean;
}

export function registerDaemonStatusRoutes(
app: Application,
deps: RegisterDaemonStatusRoutesDeps,
): void {
app.get('/daemon/status', async (req, res) => {
const detail = parseDaemonStatusDetail(req.query['detail']);
if (!detail.ok || !detail.detail) {
res.status(400).json({
error: 'detail must be one of: summary, full',
code: 'invalid_detail',
});
return;
}
try {
res.status(200).json(
await buildDaemonStatusResponse(detail.detail, {
opts: deps.opts,
boundWorkspace: deps.boundWorkspace,
bridge: deps.bridge,
workspace: deps.workspace,
daemonLog: deps.daemonLog,
startup: deps.startup,
qwenCodeVersion: deps.qwenCodeVersion,
acpHandle: deps.getAcpHandle(),
rateLimiter: deps.getRateLimiter(),
getRestSseActive: deps.getRestSseActive,
features: deps.currentServeFeatures(),
protocolVersions: getServeProtocolVersions(),
supportedDeviceFlowProviders: deps.getSupportedDeviceFlowProviders(),
deviceFlowRegistry: deps.deviceFlowRegistry,
sessionShellCommandEnabled: deps.sessionShellCommandEnabled,
}),
);
} catch (err) {
writeStderrLine(
`qwen serve: /daemon/status failed: ${err instanceof Error ? err.message : String(err)}`,
);
res.status(500).json({
error: 'Failed to build daemon status',
code: 'daemon_status_failed',
});
}
});
}
131 changes: 131 additions & 0 deletions packages/cli/src/serve/routes/health-demo.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/**
* @license
* Copyright 2025 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/

import type { Application, Request, Response } from 'express';
import { writeStderrLine } from '../../utils/stdioHelpers.js';
import type { AcpSessionBridge } from '../acp-session-bridge.js';
import { getDemoHtml } from '../demo.js';
import { isLoopbackBind } from '../loopback-binds.js';
import type { RateLimiterInstance } from '../rate-limit.js';
import type { ServeOptions } from '../types.js';

interface CreateHealthDemoRoutesDeps {
opts: Pick<ServeOptions, 'hostname' | 'requireAuth'>;
getPort: () => number;
bridge: AcpSessionBridge;
getActiveSseCount: () => number;
getRateLimiter: () => RateLimiterInstance | undefined;
}

interface HealthDemoRoutes {
exposeHealthPreAuth: boolean;
register(app: Application): void;
}

export function createHealthDemoRoutes(
deps: CreateHealthDemoRoutesDeps,
): HealthDemoRoutes {
const { opts, getPort, bridge, getActiveSseCount, getRateLimiter } = deps;

// --- Demo page: mirrors the `/health` loopback-gating pattern.
// On loopback binds, registered BEFORE bearerAuth so browsers can
// reach the page via address-bar navigation (which cannot attach
// Authorization headers). On non-loopback binds, registered AFTER
// bearerAuth — an unauthenticated `/demo` on a public interface
// would leak the full API surface (route enumeration + interactive
// console), far more than `/health`'s `{"status":"ok"}`.
// X-Frame-Options: DENY + CSP frame-ancestors 'none' prevent
// clickjacking — a malicious site embedding the demo in an iframe
// could trick a user into performing daemon actions via transparent
// overlay (the iframe's same-origin fetches bypass CORS).
const demoHandler = (_req: Request, res: Response) => {
try {
res
.type('html')
.set('X-Frame-Options', 'DENY')
.set(
'Content-Security-Policy',
"default-src 'none'; script-src 'unsafe-inline'; style-src 'unsafe-inline'; connect-src 'self'; frame-ancestors 'none'",
)
.send(getDemoHtml(getPort()));
} catch (err) {
writeStderrLine(
`qwen serve: /demo render failed: ${err instanceof Error ? err.message : String(err)}`,
);
res.status(500).json({ error: 'Failed to render demo page' });
}
};

// `/health` is exempted from `bearerAuth` ONLY on loopback binds —
// the canonical liveness-probe case (k8s/Compose probes don't
// carry the daemon's bearer; round-tripping a 401 just to know
// the listener is up is waste). On non-loopback binds the
// exemption becomes a low-severity info leak (attacker can probe
// arbitrary IP:port to confirm a `qwen serve` is listening), so
// we register `/health` AFTER `bearerAuth` and let it 401 like
// every other route. Operators using the loopback default get the
// probe-friendly behavior; operators exposing the daemon publicly
// gate `/health` behind their token alongside everything else.
// CORS deny + Host allowlist still apply to `/health` in both
// cases.
// Shared handler so loopback (pre-auth) and non-loopback (post-auth)
// routes return the same shape. `?deep=1` exposes bridge counters
// (`sessions`, `pendingPermissions`) for observability — it is
// INFORMATIONAL only, not a true liveness probe. Counter getters
// are size accessors that don't perform per-session/channel pings,
// so a wedged child (stuck on a request, leaked FD, etc.) won't
// change the response. We retain the try/catch + 503 as a
// defense-in-depth net for custom bridge impls whose getters MAY
// throw — but the real bridge's getters never do, so under normal
// operation the 503 path is unreachable. The docs
// (`docs/users/qwen-serve.md` + `qwen-serve-protocol.md`) clarify
// that deep is for counters, not health verification. Default (no
// query) stays cheap so high-frequency liveness probes don't load
// the bridge.
const healthHandler = (req: Request, res: Response): void => {
const deepQuery = req.query['deep'];
const deep = deepQuery === '1' || deepQuery === 'true' || deepQuery === '';
if (!deep) {
res.status(200).json({ status: 'ok' });
return;
}
try {
const lastActivity = bridge.lastActivityAt;
const now = Date.now();
const rateLimiter = getRateLimiter();
res.status(200).json({
status: 'ok',
sessions: bridge.sessionCount,
pendingPermissions: bridge.pendingPermissionCount,
activePrompts: bridge.activePromptCount,
connectedClients: getActiveSseCount(),
channelAlive: bridge.isChannelLive(),
lastActivityAt:
lastActivity !== null ? new Date(lastActivity).toISOString() : null,
idleSinceMs: lastActivity !== null ? now - lastActivity : null,
...(rateLimiter ? { rateLimitHits: rateLimiter.getHitCounts() } : {}),
});
} catch (err) {
writeStderrLine(
`qwen serve: /health deep probe failed: ${err instanceof Error ? err.message : String(err)}`,
);
res.status(503).json({ status: 'degraded' });
}
};

const loopback = isLoopbackBind(opts.hostname);
// `--require-auth` extends the non-loopback "gate /health behind
// bearer too" rule to loopback.
const exposeHealthPreAuth = loopback && !opts.requireAuth;

return {
exposeHealthPreAuth,
register(app: Application): void {
app.get('/health', healthHandler);
app.get('/demo', demoHandler);
},
};
}
104 changes: 104 additions & 0 deletions packages/cli/src/serve/routes/permission.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/**
* @license
* Copyright 2025 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/

import type { Application, RequestHandler, Response } from 'express';
import type { AcpSessionBridge } from '../acp-session-bridge.js';
import {
detectFromLoopback,
parseClientIdHeader,
parsePermissionVoteBody,
} from '../server/request-helpers.js';

type SendPermissionVoteError = (
res: Response,
err: unknown,
ctx: { route: string; sessionId?: string },
) => void;

interface RegisterPermissionRoutesDeps {
bridge: AcpSessionBridge;
mutate: (opts?: { strict?: boolean }) => RequestHandler;
sendPermissionVoteError: SendPermissionVoteError;
}

export function registerPermissionRoutes(
app: Application,
deps: RegisterPermissionRoutesDeps,
): void {
const { bridge, mutate, sendPermissionVoteError } = deps;

app.post('/session/:id/permission/:requestId', mutate(), (req, res) => {
const sessionId = req.params['id'];
const requestId = req.params['requestId'];
const response = parsePermissionVoteBody(req, res);
if (response === undefined) return;
const clientId = parseClientIdHeader(req, res);
if (clientId === null) return;
// Thread the kernel-stamped peer-IP loopback bit through the bridge
// context so the `local-only` policy can gate votes by transport.
const fromLoopback = detectFromLoopback(req);
const context = {
...(clientId !== undefined ? { clientId } : {}),
fromLoopback,
};
let accepted: boolean;
try {
accepted = bridge.respondToSessionPermission(
sessionId,
requestId,
response,
context,
);
} catch (err) {
sendPermissionVoteError(res, err, {
route: 'POST /session/:id/permission/:requestId',
sessionId,
});
return;
}
if (!accepted) {
res.status(404).json({
error: 'No pending permission request for session',
sessionId,
requestId,
});
return;
}
res.status(200).json({});
});

app.post('/permission/:requestId', mutate(), (req, res) => {
const requestId = req.params['requestId'];
const response = parsePermissionVoteBody(req, res);
if (response === undefined) return;
const clientId = parseClientIdHeader(req, res);
if (clientId === null) return;
// Same loopback bit threading as the session-scoped route above.
const fromLoopback = detectFromLoopback(req);
const context = {
...(clientId !== undefined ? { clientId } : {}),
fromLoopback,
};
let accepted: boolean;
try {
accepted = bridge.respondToPermission(requestId, response, context);
} catch (err) {
sendPermissionVoteError(res, err, {
route: 'POST /permission/:requestId',
});
return;
}
if (!accepted) {
// Either the requestId never existed or another client already won
// the race. Stage 1 doesn't distinguish — both surface as 404.
res
.status(404)
.json({ error: 'No pending permission request', requestId });
return;
}
res.status(200).json({});
});
}
Loading
Loading