diff --git a/packages/acp-bridge/README.md b/packages/acp-bridge/README.md index 0f8295a491d..2baba0078e7 100644 --- a/packages/acp-bridge/README.md +++ b/packages/acp-bridge/README.md @@ -30,6 +30,35 @@ extraction is split: `cli/src/serve/httpAcpBridge.ts BridgeClient.requestPermission`. PR 24 will move that and add the other three policies behind this interface. +- `status` (PR 22b/1) — wire-contract status types for + `/workspace/{mcp,skills,providers,env,preflight}` and + `/session/:id/{context,supported-commands}` routes, the + `STATUS_SCHEMA_VERSION` / `SERVE_*_EXT_METHODS` constants, + `BridgeTimeoutError` / `MissingCliEntryError` / + `BridgeChannelClosedError` typed exceptions, and the + `mapDomainErrorToErrorKind` classifier (regex → `instanceof` after + #4299 / #4300). The 27-symbol contract `acp-integration/acpAgent.ts` + consumes lives here. +- `workspacePaths` (PR 22b/1) — `canonicalizeWorkspace` (the + cross-module BX9_q contract used by `config.ts` / `settings.ts` / + `sandbox.ts` / bridge to collapse boot-time + per-request workspace + paths to one canonical key) plus `MAX_WORKSPACE_PATH_LENGTH`. +- `bridgeErrors` (PR 22b/1) — 11 typed `Error` subclasses the bridge + throws (`SessionNotFoundError`, `WorkspaceMismatchError`, + `RestoreInProgressError`, etc.); HTTP route layer + `instanceof`-branches on these to map to specific status codes. +- `bridgeTypes` (PR 22b/1) — public bridge contract types: + `BridgeSpawnRequest`, `BridgeSession`, `BridgeRestoreSessionRequest`, + `BridgeSessionState`, `BridgeRestoredSession`, `BridgeSessionSummary`, + `SessionMetadataUpdate`, `BridgeClientRequestContext`, + `BridgeHeartbeatResult`, `BridgeHeartbeatState`, plus the + `HttpAcpBridge` interface itself (~30-method facade). +- `bridgeOptions` (PR 22b/2) — `BridgeOptions` interface (factory + construction contract: `boundWorkspace`, `channelFactory`, + `maxSessions`, `eventRingSize`, `permissionResponseTimeoutMs`, + persistence callbacks, etc.) and the new `DaemonStatusProvider` + injection seam for daemon-host env / preflight cells (production + impl in `cli/src/serve/daemonStatusProvider.ts`). ## What's not here yet diff --git a/packages/acp-bridge/package.json b/packages/acp-bridge/package.json index 9ae206506d8..c2cb6fadccc 100644 --- a/packages/acp-bridge/package.json +++ b/packages/acp-bridge/package.json @@ -47,6 +47,10 @@ "types": "./dist/bridgeTypes.d.ts", "import": "./dist/bridgeTypes.js" }, + "./bridgeOptions": { + "types": "./dist/bridgeOptions.d.ts", + "import": "./dist/bridgeOptions.js" + }, "./package.json": "./package.json" }, "scripts": { diff --git a/packages/acp-bridge/src/bridgeOptions.ts b/packages/acp-bridge/src/bridgeOptions.ts new file mode 100644 index 00000000000..6a44540312f --- /dev/null +++ b/packages/acp-bridge/src/bridgeOptions.ts @@ -0,0 +1,242 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * `BridgeOptions` and the daemon-host injection seam (`DaemonStatusProvider`) + * for the ACP bridge factory. Lifted to `@qwen-code/acp-bridge` in #4175 PR + * 22b/2 so the bridge package owns the construction contract independently + * of `cli/src/serve/`. The factory implementation itself moves in PR 22b/3. + */ + +import type { ApprovalMode } from '@qwen-code/qwen-code-core'; +import type { ChannelFactory } from './channel.js'; +import type { ServePreflightCell, ServeWorkspaceEnvStatus } from './status.js'; + +/** + * Optional injection seam for daemon-host-specific status cells — + * `process.env` snapshots and the daemon-side preflight checks + * (Node version, CLI entry path, ripgrep, git, npm, workspace dir). + * + * The bridge is intentionally agnostic about how its host computes + * these cells; production `qwen serve` provides + * `cli/src/serve/daemonStatusProvider.ts` which wraps + * `buildEnvStatusFromProcess` + `buildDaemonPreflightCells`. Future + * Mode A / in-process consumers may omit the provider entirely; the + * bridge falls back to idle placeholders so `getWorkspaceEnvStatus` + * and the daemon half of `getWorkspacePreflightStatus` stay + * queryable without coupling the bridge to `process.*` state. + * + * Scope is intentionally narrow — strictly the two daemon-host + * cells the bridge currently delegates. NOT a generic logger / + * metrics seam; new injection needs should go through their own + * typed interfaces. + */ +export interface DaemonStatusProvider { + /** + * Snapshot of the daemon-host process environment for the bound + * workspace. Reads `process.versions`, runtime / sandbox / proxy + * state, and presence-only env-var checks. Returns a full + * `ServeWorkspaceEnvStatus` envelope so the bridge can pass it + * through to the route handler verbatim — the wire shape is + * unchanged from pre-injection behavior. + * + * @param boundWorkspace canonicalized workspace path the daemon + * is bound to (the same value as `BridgeOptions.boundWorkspace`). + * @param acpChannelLive whether an ACP child is currently up. + * Drives the `acpChannelLive` field on the returned envelope so + * SDK consumers can render a clear "daemon up but child not + * spawned yet" state. The bridge owns this state and passes it + * in; the provider does not need to introspect bridge internals. + */ + getEnvStatus( + boundWorkspace: string, + acpChannelLive: boolean, + ): Promise; + + /** + * Daemon-host preflight cells: Node version, CLI entry path, + * workspace directory existence, ripgrep / git / npm + * availability. The implementation typically runs each cell via + * `Promise.allSettled` so a single failing check doesn't poison + * the whole result. + * + * Returns ONLY the daemon-host cells; the ACP-level cells (auth, + * mcp_discovery, skills, providers, tool_registry, egress) are + * fetched separately by the bridge through the ACP child's + * extMethod RPC. The bridge stitches the two halves together for + * `getWorkspacePreflightStatus`. + * + * @param boundWorkspace canonicalized workspace path; cells like + * `workspace_dir` stat this path to check existence. + */ + getDaemonPreflightCells( + boundWorkspace: string, + ): Promise; +} + +/** + * Construction options for `createHttpAcpBridge`. Most fields are + * tuning knobs with sensible defaults; `boundWorkspace` is the only + * strictly-required field. See per-field JSDoc for caller contract. + */ +export interface BridgeOptions { + /** + * §03 decision §1. `single` shares one session per workspace across HTTP + * clients (live-collaboration default); `thread` gives each `spawnOrAttach` + * call its own session for strict isolation. + * + * Daemon-wide default. Per-request callers can override via + * `BridgeSpawnRequest.sessionScope` — the override wins and the + * daemon-wide value acts only as the fallback when the request + * omits the field. See the `session_scope_override` capability on + * `/capabilities.features` for negotiation. + * Reference: + * https://github.com/QwenLM/qwen-code/pull/3889#issuecomment-4427875644 + */ + sessionScope?: 'single' | 'thread'; + /** Channel factory; defaults to spawning `qwen --acp` as a child process. */ + channelFactory?: ChannelFactory; + /** How long to wait for the child's `initialize` reply before giving up. */ + initializeTimeoutMs?: number; + /** + * Cap on concurrent live sessions. `spawnOrAttach` calls that would + * cross this throw `SessionLimitExceededError`; attaches to an + * existing session (same workspace under `single` scope) are not + * counted. `0` / `Infinity` disable the cap. Defaults to 20 — see + * `ServeOptions.maxSessions` for the rationale. + */ + maxSessions?: number; + /** + * Per-session SSE replay ring depth. Sets `ringSize` on every + * `new EventBus(...)` the bridge constructs (both fresh sessions + * and restored sessions). Defaults to `DEFAULT_RING_SIZE` (8000, + * #3803 §02 target). Must be a positive finite integer; `0` / + * `NaN` / negative throw at boot (fail-CLOSED — same posture as + * `maxSessions`, where silently disabling a backpressure knob on a + * config typo is worse than failing to start). + * + * Operators tune via `qwen serve --event-ring-size `. Cost + * scales linearly with `ringSize`; each retained `BridgeEvent` is + * an object reference plus its serialized payload (text chunks / + * tool-call args / etc.), so the per-session memory ceiling is + * `ringSize × average-event-size` held until the session ends. + */ + eventRingSize?: number; + /** + * Per-`requestPermission` wall clock. After this many ms with + * no client vote, the agent's permission promise resolves as + * cancelled — the per-session FIFO can drain instead of poisoning + * forever on a missing SSE subscriber. Defaults to 5 minutes. + * `0` / `Infinity` / non-finite disable the timeout (matches + * legacy behavior, NOT recommended). + */ + permissionResponseTimeoutMs?: number; + /** + * Per-session cap on pending permissions in flight. New + * `requestPermission` calls past this cap resolve as cancelled with + * a stderr warning. Defaults to 64. `0` / `Infinity` disable the + * cap. + */ + maxPendingPermissionsPerSession?: number; + /** + * Absolute, **already-canonical** path this daemon is bound to (per + * #3803 §02: 1 daemon = 1 workspace). `spawnOrAttach` calls whose + * `workspaceCwd` doesn't canonicalize to this same value throw + * `WorkspaceMismatchError` (route → 400 with code `workspace_mismatch`). + * + * **Caller contract**: pass the result of + * `canonicalizeWorkspace(path)`. `runQwenServe` does this at boot + * and threads the same canonical value into the bridge AND + * `createServeApp` (via `deps.boundWorkspace`) so all three — + * `/capabilities.workspaceCwd`, the `POST /session` cwd fallback, + * and this bridge's mismatch check — share one canonical form. The + * constructor only checks `path.isAbsolute`; it does NOT + * re-canonicalize (a redundant `realpathSync.native` could + * theoretically diverge from the runQwenServe canonicalize on + * NFS-transient / mid-rename filesystems, landing the bridge with + * one canonical form while `/capabilities` advertises another). + * Direct embeds / tests calling `createHttpAcpBridge` themselves + * MUST canonicalize before passing. + */ + boundWorkspace: string; + /** + * Per-handle env overrides forwarded to `defaultSpawnChannelFactory` + * at spawn time. Concurrent embedded daemons in the same process + * use this to avoid cross-contaminating each other's MCP budget / + * mode env (the `defaultSpawnChannelFactory` snapshots + * `process.env` AT SPAWN TIME, not at `runQwenServe()` call + * time — so the last `runQwenServe()` to set the global env + * would win for all subsequent spawns across all daemon + * handles, breaking the documented per-daemon policy). + * + * Shape: `Record`. A `string` value + * sets the env var for the child; `undefined` explicitly + * REMOVES the var from the child env (useful for "this daemon + * has no MCP budget" embedded callers that need to scrub a + * stale global). Keys NOT present in this record are inherited + * from `process.env` as before. + * + * Custom `channelFactory` callers receive this through the + * factory's second arg and decide what to do with it (tests + * typically ignore it; the production factory merges it). + */ + childEnvOverrides?: Readonly>; + /** + * #4175 Wave 4 PR 17 — optional callback for persisting `tools. + * approvalMode` to the workspace settings file. Invoked by + * `setSessionApprovalMode` ONLY when the route caller passes + * `{persist: true}`. The default `runQwenServe` wires this to + * `loadSettings(boundWorkspace).setValue(SettingScope.Workspace, + * 'tools.approvalMode', mode)`. Bridge tests and embedded callers + * may omit it; when omitted, `setSessionApprovalMode` still applies + * the in-process change and returns `persisted: false` regardless + * of the request flag. + */ + persistApprovalMode?: ( + boundWorkspace: string, + mode: ApprovalMode, + ) => Promise; + /** + * #4175 Wave 4 PR 17 — optional callback for mutating + * `tools.disabled` in workspace settings. Invoked by + * `setWorkspaceToolEnabled` to add (`enabled: false`) or remove + * (`enabled: true`) `toolName` from the persisted disabled set. + * The default `runQwenServe` wires this to a fresh + * `loadSettings(boundWorkspace)` per call so concurrent edits from + * other writers (CLI, another daemon, an editor) are picked up. + * Bridge tests / embedded callers may omit it; without the hook + * `setWorkspaceToolEnabled` throws a clear error rather than + * silently dropping the write. + */ + persistDisabledTools?: ( + boundWorkspace: string, + toolName: string, + enabled: boolean, + ) => Promise; + /** + * #4175 Wave 5 PR 22b/2 — optional injection seam for daemon-host + * status cells (env snapshot, daemon preflight). Production + * `qwen serve` provides + * `createDaemonStatusProvider()` from + * `cli/src/serve/daemonStatusProvider.ts`. + * + * **When omitted**: the bridge returns idle placeholders for + * `getWorkspaceEnvStatus` (full envelope with empty `cells: []` + * and `acpChannelLive` from bridge state) and an empty array for + * the daemon half of `getWorkspacePreflightStatus` (the ACP-level + * cells are still fetched normally when a child is live). This + * matches the "idle status is queryable" pattern PR 12 / 13 + * established for diagnostic routes — direct embeds and tests + * that don't need daemon-host cells can omit the provider + * without crashing those routes. + * + * Mode A in-process consumers (`qwen --serve`, future) typically + * omit this provider — they don't run a separate daemon process + * so daemon-host environment cells are not meaningful. They can + * still query the routes; they'll see empty/idle cells. + */ + statusProvider?: DaemonStatusProvider; +} diff --git a/packages/acp-bridge/src/index.ts b/packages/acp-bridge/src/index.ts index 3de8304efac..642d8c10263 100644 --- a/packages/acp-bridge/src/index.ts +++ b/packages/acp-bridge/src/index.ts @@ -12,3 +12,4 @@ export * from './workspacePaths.js'; export * from './status.js'; export * from './bridgeErrors.js'; export * from './bridgeTypes.js'; +export * from './bridgeOptions.js'; diff --git a/packages/acp-bridge/src/status.test.ts b/packages/acp-bridge/src/status.test.ts index c97b317a384..e2040400df7 100644 --- a/packages/acp-bridge/src/status.test.ts +++ b/packages/acp-bridge/src/status.test.ts @@ -63,9 +63,9 @@ describe('BridgeChannelClosedError', () => { expect( new BridgeChannelClosedError('mid-request (workspace status)').message, ).toBe('agent channel closed mid-request (workspace status)'); - expect( - new BridgeChannelClosedError('during session/load').message, - ).toBe('agent channel closed during session/load'); + expect(new BridgeChannelClosedError('during session/load').message).toBe( + 'agent channel closed during session/load', + ); }); }); @@ -131,6 +131,35 @@ describe('mapDomainErrorToErrorKind', () => { expect(mapDomainErrorToErrorKind(synthetic)).toBe('auth_env_error'); }); + it('classifies SkillError via .name fallback when instanceof breaks across package boundaries (#4298 follow-up)', () => { + // Wenshao review fold-in (#4298 thread r3262781757): the same + // cross-package bundling concern that drives the TrustGateError + // `.name` matcher applies to `SkillError`. Synthesize a foreign + // copy of the class (carrying the right `.name` + `.code` but + // failing `instanceof SkillError`) and assert classification still + // works. + const parseSynthetic = Object.assign(new Error('foreign-bundled'), { + name: 'SkillError', + code: 'PARSE_ERROR', + }); + expect(mapDomainErrorToErrorKind(parseSynthetic)).toBe('parse_error'); + + const fileSynthetic = Object.assign(new Error('foreign-bundled'), { + name: 'SkillError', + code: 'FILE_ERROR', + }); + expect(mapDomainErrorToErrorKind(fileSynthetic)).toBe('missing_file'); + + // Unknown skill code on a cross-bundle SkillError still degrades + // to undefined rather than a misleading category — same behavior + // as the genuine `instanceof` path. + const unknownSynthetic = Object.assign(new Error('foreign-bundled'), { + name: 'SkillError', + code: 'NOT_A_REAL_CODE', + }); + expect(mapDomainErrorToErrorKind(unknownSynthetic)).toBeUndefined(); + }); + it('classifies ModelConfigError subclasses (recognized via .name) as auth_env_error', () => { for (const name of [ 'StrictMissingCredentialsError', diff --git a/packages/acp-bridge/src/status.ts b/packages/acp-bridge/src/status.ts index f9dc0e17b1f..7d78ffd48f9 100644 --- a/packages/acp-bridge/src/status.ts +++ b/packages/acp-bridge/src/status.ts @@ -457,6 +457,33 @@ export function createIdleWorkspaceProvidersStatus( }; } +/** + * #4175 PR 22b/2: idle envelope for `/workspace/env` when the bridge + * has no `DaemonStatusProvider` injected (Mode A in-process consumers, + * tests, embedded callers that don't need daemon-host cells). Single + * construction site so future optional-field additions to + * `ServeWorkspaceEnvStatus` only need updating in one place — the + * production builder in `cli/src/serve/envSnapshot.ts buildEnvStatusFromProcess` + * and this helper would otherwise diverge silently (TS won't flag a + * missing optional field). + * + * Note: `initialized: true` matches `buildEnvStatusFromProcess` — + * the daemon answers env from `process.*` state without consulting + * ACP, so even an "empty" envelope is initialized. + */ +export function createIdleEnvStatus( + workspaceCwd: string, + acpChannelLive: boolean, +): ServeWorkspaceEnvStatus { + return { + v: STATUS_SCHEMA_VERSION, + workspaceCwd, + initialized: true, + acpChannelLive, + cells: [], + }; +} + /** * Discriminant for diagnostic cells emitted by `/workspace/env`. * `env_var` cells are presence-only (the daemon never echoes secret values @@ -609,9 +636,24 @@ export function mapDomainErrorToErrorKind( if (err instanceof BridgeTimeoutError) return 'init_timeout'; if (err instanceof BridgeChannelClosedError) return 'protocol_error'; if (err instanceof MissingCliEntryError) return 'missing_binary'; - if (err instanceof SkillError) { - if (SKILL_PARSE_CODES.has(err.code)) return 'parse_error'; - if (SKILL_FILE_CODES.has(err.code)) return 'missing_file'; + // `SkillError` is defined in `@qwen-code/qwen-code-core/skills`; same + // cross-package bundling concern as `TrustGateError` below — when this + // function is consumed from outside the monorepo (or under a bundler + // that doesn't dedupe `file:` workspace deps), the `SkillError` class + // identity at the throw site (cli's `SkillManager`) can diverge from + // the one resolved here through acp-bridge's `@qwen-code/qwen-code-core` + // dependency, silently making `instanceof` return `false` and + // dropping the skill `errorKind` classification on diagnostic cells. + // The `OR .name === 'SkillError'` branch keeps classification working + // regardless of which copy of the class the value carries. + // Wenshao review fold-in (#4298 thread r3262781757). + if ( + err instanceof SkillError || + (err as Error | undefined)?.name === 'SkillError' + ) { + const code = (err as { code?: string }).code; + if (code && SKILL_PARSE_CODES.has(code)) return 'parse_error'; + if (code && SKILL_FILE_CODES.has(code)) return 'missing_file'; return undefined; } if (err instanceof SyntaxError) return 'parse_error'; diff --git a/packages/cli/src/serve/daemonStatusProvider.ts b/packages/cli/src/serve/daemonStatusProvider.ts new file mode 100644 index 00000000000..dc5fee8c467 --- /dev/null +++ b/packages/cli/src/serve/daemonStatusProvider.ts @@ -0,0 +1,287 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Daemon-host implementation of the `DaemonStatusProvider` interface + * (declared in `@qwen-code/acp-bridge/bridgeOptions`). Production + * `qwen serve` wires this into `BridgeOptions.statusProvider` so the + * bridge factory can pull env / preflight cells without importing + * daemon-host-specific modules directly. + * + * Lift origin (#4175 PR 22b/2): the inline `buildDaemonPreflightCells` + * function moved here from `httpAcpBridge.ts`; `buildEnvStatusFromProcess` + * stays in `envSnapshot.ts` and is wrapped here. Mode A consumers can + * omit this provider entirely — the bridge falls back to idle placeholders. + */ + +import { promises as fs } from 'node:fs'; +import { canUseRipgrep } from '@qwen-code/qwen-code-core'; +import { + type DaemonStatusProvider, + mapDomainErrorToErrorKind, + type ServePreflightCell, + type ServePreflightKind, + type ServeWorkspaceEnvStatus, +} from '@qwen-code/acp-bridge'; +import { getGitVersion, getNpmVersion } from '../utils/systemInfo.js'; +import { buildEnvStatusFromProcess } from './envSnapshot.js'; + +const REQUIRED_NODE_MAJOR = 22; + +/** + * Construct the production `DaemonStatusProvider` for `qwen serve`. + * Returns a fresh provider per call; provider is stateless so callers + * can cache if hot-path overhead matters (currently both methods are + * called only from the route handlers, so per-request allocation is + * fine). + */ +export function createDaemonStatusProvider(): DaemonStatusProvider { + return { + async getEnvStatus( + boundWorkspace: string, + acpChannelLive: boolean, + ): Promise { + // `buildEnvStatusFromProcess` is synchronous (no I/O) — wrap + // in a resolved Promise to match the async `DaemonStatusProvider` + // contract. Future async-needing implementations (e.g. reading + // a config file) get the seam without changing the bridge. + return buildEnvStatusFromProcess(boundWorkspace, acpChannelLive); + }, + + async getDaemonPreflightCells( + boundWorkspace: string, + ): Promise { + return buildDaemonPreflightCells(boundWorkspace); + }, + }; +} + +/** + * Daemon-side preflight cells for `GET /workspace/preflight`. Synchronous + * cells (`node_version`, `cli_entry`) and async cells + * (`workspace_dir` stat, `ripgrep` / `git` / `npm` PATH lookups) run in + * parallel via `Promise.allSettled`; a single failing cell becomes an + * `error` cell rather than poisoning the whole response. The + * corresponding ACP-side cells (auth, MCP, skills, providers, + * tool_registry, egress) are stitched in by the bridge's + * `requestWorkspaceStatus` helper when a child is live, or fall back + * to `not_started` placeholders when idle. + * + * Lifted verbatim from `httpAcpBridge.ts:4104-4280` in #4175 PR 22b/2 + * so the bridge factory no longer hard-imports daemon-host helpers. + */ +async function buildDaemonPreflightCells( + boundWorkspace: string, +): Promise { + // Each builder returns (or eventually returns) one cell. We run them via + // `Promise.allSettled` after wrapping every call in `Promise.resolve().then` + // so that synchronous throws from any builder become rejected promises + // instead of escaping out of `Promise.all`'s array construction. A throw + // there would propagate up to the route handler and turn the whole + // `/workspace/preflight` envelope into a 500 — directly contradicting the + // design promise that "daemon cells always render even when ACP is sick" + // (see the route handler's catch ladder). + // + // For any rejected slot we synthesize an `error` cell with the slot's + // expected `kind` so the response shape (length, ordering, locality) is + // bit-for-bit the same regardless of failure modes. + const nodeVersionCell = (): ServePreflightCell => { + try { + const nodeVersion = process.versions.node; + const major = Number.parseInt(nodeVersion.split('.')[0] ?? '0', 10); + if (Number.isFinite(major) && major >= REQUIRED_NODE_MAJOR) { + return { + kind: 'node_version', + status: 'ok', + locality: 'daemon', + detail: { + version: nodeVersion, + required: `>=${REQUIRED_NODE_MAJOR}`, + }, + }; + } + return { + kind: 'node_version', + status: 'error', + errorKind: 'missing_binary', + error: `Node ${nodeVersion} is below the required >=${REQUIRED_NODE_MAJOR}.`, + hint: `Upgrade Node to v${REQUIRED_NODE_MAJOR} or newer.`, + locality: 'daemon', + detail: { version: nodeVersion, required: `>=${REQUIRED_NODE_MAJOR}` }, + }; + } catch (err) { + return { + kind: 'node_version', + status: 'error', + error: err instanceof Error ? err.message : String(err), + locality: 'daemon', + }; + } + }; + + // Mirrors `defaultSpawnChannelFactory`'s lookup so the preflight cell + // reflects the path the child would actually be spawned from. + const cliEntryCell = (): ServePreflightCell => { + const cliEntry = process.env['QWEN_CLI_ENTRY'] || process.argv[1] || ''; + if (cliEntry) { + return { + kind: 'cli_entry', + status: 'ok', + locality: 'daemon', + detail: { + path: cliEntry, + source: process.env['QWEN_CLI_ENTRY'] + ? 'QWEN_CLI_ENTRY' + : 'process.argv[1]', + }, + }; + } + return { + kind: 'cli_entry', + status: 'error', + errorKind: 'missing_binary', + error: 'Cannot determine CLI entry path for spawning the ACP child.', + hint: 'Set QWEN_CLI_ENTRY to the absolute path of the qwen entry script.', + locality: 'daemon', + }; + }; + + const workspaceDirCell = async (): Promise => { + try { + const stat = await fs.stat(boundWorkspace); + if (stat.isDirectory()) { + return { + kind: 'workspace_dir', + status: 'ok', + locality: 'daemon', + detail: { path: boundWorkspace }, + }; + } + return { + kind: 'workspace_dir', + status: 'error', + errorKind: 'missing_file', + error: `Bound workspace path is not a directory: ${boundWorkspace}`, + locality: 'daemon', + detail: { path: boundWorkspace }, + }; + } catch (err) { + const errorKind = mapDomainErrorToErrorKind(err); + return { + kind: 'workspace_dir', + status: 'error', + error: err instanceof Error ? err.message : String(err), + ...(errorKind ? { errorKind } : {}), + locality: 'daemon', + detail: { path: boundWorkspace }, + }; + } + }; + + type Slot = { + kind: ServePreflightKind; + run: () => ServePreflightCell | Promise; + }; + const slots: Slot[] = [ + { kind: 'node_version', run: nodeVersionCell }, + { kind: 'cli_entry', run: cliEntryCell }, + { kind: 'workspace_dir', run: workspaceDirCell }, + { + kind: 'ripgrep', + run: () => + safeCheck('ripgrep', async () => { + // Mirror runtime behavior: `Config.useBuiltinRipgrep` defaults to + // `true`, so `canUseRipgrep(true)` reports the *bundled* binary + // when no system `rg` is installed. Passing `false` here would + // tell users "ripgrep missing" while the runtime can still use + // the bundled one — a misleading warning. + const ok = await canUseRipgrep(true); + return ok + ? { status: 'ok' as const } + : { + status: 'warning' as const, + hint: 'Install ripgrep for faster grep tool execution.', + }; + }), + }, + { + kind: 'git', + run: () => + safeCheck('git', async () => { + const v = await getGitVersion(); + return v && v !== 'unknown' + ? { status: 'ok' as const, detail: { version: v } } + : { status: 'warning' as const, hint: 'git not found on PATH.' }; + }), + }, + { + kind: 'npm', + run: () => + safeCheck('npm', async () => { + const v = await getNpmVersion(); + return v && v !== 'unknown' + ? { status: 'ok' as const, detail: { version: v } } + : { status: 'warning' as const, hint: 'npm not found on PATH.' }; + }), + }, + ]; + + // `Promise.resolve().then(run)` coerces sync throws into rejected + // promises so `Promise.allSettled` can absorb them as `error` cells + // rather than letting them escape the route. + const settled = await Promise.allSettled( + slots.map((s) => Promise.resolve().then(s.run)), + ); + return settled.map((result, i) => { + if (result.status === 'fulfilled') return result.value; + const err = result.reason; + const errorKind = mapDomainErrorToErrorKind(err); + return { + kind: slots[i]!.kind, + status: 'error' as const, + locality: 'daemon' as const, + error: err instanceof Error ? err.message : String(err), + ...(errorKind ? { errorKind } : {}), + }; + }); +} + +async function safeCheck( + kind: 'ripgrep' | 'git' | 'npm', + body: () => Promise<{ + status: 'ok' | 'warning'; + detail?: Record; + hint?: string; + }>, +): Promise { + try { + const r = await body(); + return { + kind, + status: r.status, + locality: 'daemon', + ...(r.detail ? { detail: r.detail } : {}), + ...(r.hint ? { hint: r.hint } : {}), + }; + } catch (err) { + // Classify so SDK consumers can render structured remediation + // (`missing_binary` for ENOENT, `missing_file` for EACCES, etc.). + // Without this tag, the rg/git/npm catch path differs from the + // sync-builder catch paths above, which all classify their own + // errors. The outer `Promise.allSettled` catch in + // `buildDaemonPreflightCells` is unreachable for slots whose `run` + // is `() => safeCheck(...)`, because `safeCheck` always resolves + // (its own try/catch swallows). So this is the only place to tag. + const errorKind = mapDomainErrorToErrorKind(err); + return { + kind, + status: 'error', + error: err instanceof Error ? err.message : String(err), + locality: 'daemon', + ...(errorKind ? { errorKind } : {}), + }; + } +} diff --git a/packages/cli/src/serve/httpAcpBridge.test.ts b/packages/cli/src/serve/httpAcpBridge.test.ts index 7083738ab2e..08d7323456a 100644 --- a/packages/cli/src/serve/httpAcpBridge.test.ts +++ b/packages/cli/src/serve/httpAcpBridge.test.ts @@ -35,6 +35,7 @@ import type { SetSessionModeRequest, SetSessionModeResponse, } from '@agentclientprotocol/sdk'; +import { createDaemonStatusProvider } from './daemonStatusProvider.js'; import { createHttpAcpBridge, InvalidClientIdError, @@ -71,9 +72,19 @@ const SESS_A = `sess:${WS_A}`; * `WS_A` would otherwise repeat `boundWorkspace: WS_A` everywhere; this * helper defaults it. Tests that need a different bind path (e.g. the * mismatch test) pass `boundWorkspace` explicitly. + * + * #4175 PR 22b/2: also defaults `statusProvider` to the production daemon + * impl so existing env / preflight tests (which exercise the bridge's + * delegation path) keep seeing populated cells. Tests that want to + * exercise the no-provider idle fallback can override with + * `{ statusProvider: undefined }`. */ function makeBridge(opts: Partial = {}): HttpAcpBridge { - return createHttpAcpBridge({ boundWorkspace: WS_A, ...opts }); + return createHttpAcpBridge({ + boundWorkspace: WS_A, + statusProvider: createDaemonStatusProvider(), + ...opts, + }); } interface FakeAgentOpts { @@ -543,6 +554,110 @@ describe('createHttpAcpBridge', () => { await bridge.shutdown(); }); + it('returns idle env envelope when statusProvider is omitted (Mode A fallback)', async () => { + // PR 22b/2 fold-in: covers the no-provider branch in + // `getWorkspaceEnvStatus`. Production `runQwenServe` and + // `createServeApp` both wire `createDaemonStatusProvider()`, but + // direct embeds (Mode A in-process consumers, future) may omit it. + // The bridge must still answer the route — falling back to the + // shared `createIdleEnvStatus` helper rather than throwing. + const bridge = makeBridge({ statusProvider: undefined }); + + const idle = await bridge.getWorkspaceEnvStatus(); + expect(idle).toMatchObject({ + v: 1, + workspaceCwd: WS_A, + initialized: true, + acpChannelLive: false, + cells: [], + }); + + await bridge.shutdown(); + }); + + it('returns empty daemon preflight cells when statusProvider is omitted (Mode A fallback)', async () => { + // PR 22b/2 fold-in: covers the no-provider branch in + // `getWorkspacePreflightStatus`. ACP-side cells still render + // (idle `not_started` placeholders here since no channel is up); + // only the daemon-host half is empty. + const bridge = makeBridge({ statusProvider: undefined }); + + const status = await bridge.getWorkspacePreflightStatus(); + expect(status).toMatchObject({ + v: 1, + workspaceCwd: WS_A, + initialized: true, + acpChannelLive: false, + }); + + // No daemon cells; only ACP-side `not_started` placeholders. + const daemonCells = status.cells.filter((c) => c.locality === 'daemon'); + const acpCells = status.cells.filter((c) => c.locality === 'acp'); + expect(daemonCells).toHaveLength(0); + expect(acpCells.length).toBeGreaterThan(0); + expect(acpCells.every((c) => c.status === 'not_started')).toBe(true); + + await bridge.shutdown(); + }); + + it('falls back to idle env envelope when statusProvider.getEnvStatus throws', async () => { + // PR 22b/2 wenshao [Critical] fold-in: a custom provider that + // throws would otherwise propagate past the bridge into the route + // handler as a 500. The catch-and-log preserves the + // pre-injection invariant that `/workspace/env` always answers, + // even when the daemon-host helper is sick. + const throwingProvider = { + async getEnvStatus(): Promise { + throw new Error('boom — env collector crashed'); + }, + async getDaemonPreflightCells(): Promise { + return []; + }, + }; + const bridge = makeBridge({ statusProvider: throwingProvider }); + + const env = await bridge.getWorkspaceEnvStatus(); + expect(env).toMatchObject({ + v: 1, + workspaceCwd: WS_A, + initialized: true, + acpChannelLive: false, + cells: [], + }); + + await bridge.shutdown(); + }); + + it('falls back to empty daemon cells when statusProvider.getDaemonPreflightCells throws', async () => { + // PR 22b/2 wenshao [Critical] fold-in: parallel to env — a + // throwing preflight provider must NOT take down the route, so + // the ACP-side cells still render even when the daemon-side + // collector is sick. + const throwingProvider = { + async getEnvStatus(): Promise { + throw new Error('unused'); + }, + async getDaemonPreflightCells(): Promise { + throw new Error('boom — preflight collector crashed'); + }, + }; + const bridge = makeBridge({ statusProvider: throwingProvider }); + + const status = await bridge.getWorkspacePreflightStatus(); + expect(status).toMatchObject({ + v: 1, + workspaceCwd: WS_A, + initialized: true, + acpChannelLive: false, + }); + const daemonCells = status.cells.filter((c) => c.locality === 'daemon'); + const acpCells = status.cells.filter((c) => c.locality === 'acp'); + expect(daemonCells).toHaveLength(0); + expect(acpCells.length).toBeGreaterThan(0); + + await bridge.shutdown(); + }); + it('returns daemon preflight cells with not_started ACP cells when idle', async () => { const handles: ChannelHandle[] = []; const bridge = makeBridge({ diff --git a/packages/cli/src/serve/httpAcpBridge.ts b/packages/cli/src/serve/httpAcpBridge.ts index d0752ab8690..c440b458ffb 100644 --- a/packages/cli/src/serve/httpAcpBridge.ts +++ b/packages/cli/src/serve/httpAcpBridge.ts @@ -25,22 +25,19 @@ import { SERVE_STATUS_EXT_METHODS, STATUS_SCHEMA_VERSION, createIdleAcpPreflightCells, + createIdleEnvStatus, createIdleWorkspaceMcpStatus, createIdleWorkspaceProvidersStatus, createIdleWorkspaceSkillsStatus, mapDomainErrorToErrorKind, type ServePreflightCell, - type ServePreflightKind, type ServeStatusCell, } from './status.js'; -import { buildEnvStatusFromProcess } from './envSnapshot.js'; import type { ApprovalMode } from '@qwen-code/qwen-code-core'; import { TrustGateError, - canUseRipgrep, getCurrentGeminiMdFilename, } from '@qwen-code/qwen-code-core'; -import { getGitVersion, getNpmVersion } from '../utils/systemInfo.js'; import type { CancelNotification, Client, @@ -188,143 +185,20 @@ export type { AcpChannel, AcpChannelExitInfo, ChannelFactory }; // semantics today instead of a wire-level break at Stage 2. Tracked // under #3803. Reference: // https://github.com/QwenLM/qwen-code/pull/3889#issuecomment-4427773706 -export interface BridgeOptions { - /** - * §03 decision §1. `single` shares one session per workspace across HTTP - * clients (live-collaboration default); `thread` gives each `spawnOrAttach` - * call its own session for strict isolation. - * - * Daemon-wide default. Per-request callers can override via - * `BridgeSpawnRequest.sessionScope` — the override wins and the - * daemon-wide value acts only as the fallback when the request - * omits the field. See the `session_scope_override` capability on - * `/capabilities.features` for negotiation. - * Reference: - * https://github.com/QwenLM/qwen-code/pull/3889#issuecomment-4427875644 - */ - sessionScope?: 'single' | 'thread'; - /** Channel factory; defaults to spawning `qwen --acp` as a child process. */ - channelFactory?: ChannelFactory; - /** How long to wait for the child's `initialize` reply before giving up. */ - initializeTimeoutMs?: number; - /** - * Cap on concurrent live sessions. `spawnOrAttach` calls that would - * cross this throw `SessionLimitExceededError`; attaches to an - * existing session (same workspace under `single` scope) are not - * counted. `0` / `Infinity` disable the cap. Defaults to 20 — see - * `ServeOptions.maxSessions` for the rationale. - */ - maxSessions?: number; - /** - * Per-session SSE replay ring depth. Sets `ringSize` on every - * `new EventBus(...)` the bridge constructs (both fresh sessions - * and restored sessions). Defaults to `DEFAULT_RING_SIZE` (8000, - * #3803 §02 target). Must be a positive finite integer; `0` / - * `NaN` / negative throw at boot (fail-CLOSED — same posture as - * `maxSessions`, where silently disabling a backpressure knob on a - * config typo is worse than failing to start). - * - * Operators tune via `qwen serve --event-ring-size `. Cost - * scales linearly with `ringSize`; each retained `BridgeEvent` is - * an object reference plus its serialized payload (text chunks / - * tool-call args / etc.), so the per-session memory ceiling is - * `ringSize × average-event-size` held until the session ends. - */ - eventRingSize?: number; - /** - * Bd1yh: per-`requestPermission` wall clock. After this many ms with - * no client vote, the agent's permission promise resolves as - * cancelled — the per-session FIFO can drain instead of poisoning - * forever on a missing SSE subscriber. Defaults to 5 minutes. - * `0` / `Infinity` / non-finite disable the timeout (matches - * legacy behavior, NOT recommended). - */ - permissionResponseTimeoutMs?: number; - /** - * Bd1z5: per-session cap on pending permissions in flight. New - * `requestPermission` calls past this cap resolve as cancelled with - * a stderr warning. Defaults to 64. `0` / `Infinity` disable the - * cap. - */ - maxPendingPermissionsPerSession?: number; - /** - * Absolute, **already-canonical** path this daemon is bound to (per - * #3803 §02: 1 daemon = 1 workspace). `spawnOrAttach` calls whose - * `workspaceCwd` doesn't canonicalize to this same value throw - * `WorkspaceMismatchError` (route → 400 with code `workspace_mismatch`). - * - * **Caller contract**: pass the result of - * `canonicalizeWorkspace(path)`. `runQwenServe` does this at boot - * and threads the same canonical value into the bridge AND - * `createServeApp` (via `deps.boundWorkspace`) so all three — - * `/capabilities.workspaceCwd`, the `POST /session` cwd fallback, - * and this bridge's mismatch check — share one canonical form. The - * constructor only checks `path.isAbsolute`; it does NOT - * re-canonicalize (a redundant `realpathSync.native` could - * theoretically diverge from the runQwenServe canonicalize on - * NFS-transient / mid-rename filesystems, landing the bridge with - * one canonical form while `/capabilities` advertises another). - * Direct embeds / tests calling `createHttpAcpBridge` themselves - * MUST canonicalize before passing. - */ - boundWorkspace: string; - /** - * PR 14 fix (review #4247 wenshao R5 runQwenServe.ts:216): per- - * handle env overrides forwarded to `defaultSpawnChannelFactory` - * at spawn time. Replaces the prior `process.env` mutation in - * `runQwenServe` so concurrent embedded daemons in the same - * process don't cross-contaminate each other's MCP budget / - * mode env (the `defaultSpawnChannelFactory` snapshots - * `process.env` AT SPAWN TIME, not at `runQwenServe()` call - * time — so the last `runQwenServe()` to set the global env - * would win for all subsequent spawns across all daemon - * handles, breaking the documented per-daemon policy). - * - * Shape: `Record`. A `string` value - * sets the env var for the child; `undefined` explicitly - * REMOVES the var from the child env (useful for "this daemon - * has no MCP budget" embedded callers that need to scrub a - * stale global). Keys NOT present in this record are inherited - * from `process.env` as before. - * - * Custom `channelFactory` callers receive this through the - * factory's second arg and decide what to do with it (tests - * typically ignore it; the production factory merges it). - */ - childEnvOverrides?: Readonly>; - /** - * #4175 Wave 4 PR 17 — optional callback for persisting `tools. - * approvalMode` to the workspace settings file. Invoked by - * `setSessionApprovalMode` ONLY when the route caller passes - * `{persist: true}`. The default `runQwenServe` wires this to - * `loadSettings(boundWorkspace).setValue(SettingScope.Workspace, - * 'tools.approvalMode', mode)`. Bridge tests and embedded callers - * may omit it; when omitted, `setSessionApprovalMode` still applies - * the in-process change and returns `persisted: false` regardless - * of the request flag. - */ - persistApprovalMode?: ( - boundWorkspace: string, - mode: ApprovalMode, - ) => Promise; - /** - * #4175 Wave 4 PR 17 — optional callback for mutating - * `tools.disabled` in workspace settings. Invoked by - * `setWorkspaceToolEnabled` to add (`enabled: false`) or remove - * (`enabled: true`) `toolName` from the persisted disabled set. - * The default `runQwenServe` wires this to a fresh - * `loadSettings(boundWorkspace)` per call so concurrent edits from - * other writers (CLI, another daemon, an editor) are picked up. - * Bridge tests / embedded callers may omit it; without the hook - * `setWorkspaceToolEnabled` throws a clear error rather than - * silently dropping the write. - */ - persistDisabledTools?: ( - boundWorkspace: string, - toolName: string, - enabled: boolean, - ) => Promise; -} + +// `BridgeOptions` + `DaemonStatusProvider` lifted to +// `@qwen-code/acp-bridge/bridgeOptions` in #4175 PR 22b/2 — the +// daemon-host injection seam (`statusProvider`) is now part of the +// bridge package's public construction contract. `runQwenServe` wires +// `createDaemonStatusProvider()` (production impl in +// `cli/src/serve/daemonStatusProvider.ts`) when the bridge is built; +// embedded callers that don't need daemon-host cells may omit it, +// in which case the factory returns idle placeholders. +import type { + BridgeOptions, + DaemonStatusProvider, +} from '@qwen-code/acp-bridge/bridgeOptions'; +export type { BridgeOptions, DaemonStatusProvider }; /** * The single `qwen --acp` child + the ACP connection on top of it, @@ -3263,11 +3137,74 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { }, async getWorkspaceEnvStatus() { - return buildEnvStatusFromProcess(boundWorkspace, !!liveChannelInfo()); + const acpChannelLive = !!liveChannelInfo(); + // PR 22b/2: daemon-host env snapshot delegated to + // `BridgeOptions.statusProvider`. When omitted (Mode A in-process + // consumers, tests) the bridge returns an idle envelope — + // matches the "queryable but empty" pattern PR 12 / 13 + // established for diagnostic routes. + // + // Wenshao review fold-in (#4304): a custom provider that throws + // would otherwise propagate past the bridge into `/workspace/env` + // as a 500. Catch + log + fall back to the idle envelope so the + // route still responds — the `daemon cells always answerable` + // invariant the pre-injection `buildEnvStatusFromProcess` carried + // (it never threw because it was synchronous and self-contained) + // is preserved structurally. + if (!opts.statusProvider) { + return createIdleEnvStatus(boundWorkspace, acpChannelLive); + } + try { + return await opts.statusProvider.getEnvStatus( + boundWorkspace, + acpChannelLive, + ); + } catch (err) { + writeStderrLine( + `qwen serve: statusProvider.getEnvStatus failed; ` + + `falling back to idle envelope: ` + + (err instanceof Error ? err.message : String(err)), + ); + return createIdleEnvStatus(boundWorkspace, acpChannelLive); + } }, async getWorkspacePreflightStatus() { - const daemonCells = await buildDaemonPreflightCells(boundWorkspace); + // PR 22b/2: daemon-host preflight cells delegated to + // `BridgeOptions.statusProvider`. Without a provider the daemon + // half is empty `[]`; ACP-side cells are still fetched normally + // when a child is live. + // + // Wenshao review fold-in (#4304): a throwing provider would + // otherwise propagate past the bridge and turn the entire + // preflight envelope into a 500 — losing both daemon cells AND + // the ACP-side cells fetched below. Catch + log + fall back to + // empty so ACP cells still render. Pre-injection + // `buildDaemonPreflightCells` used `Promise.allSettled` and was + // effectively unthrowable; this preserves that route-level + // invariant for custom provider impls that may throw. + let daemonCells: ServePreflightCell[]; + if (!opts.statusProvider) { + // Asymmetric vs `getWorkspaceEnvStatus` (which falls back to a + // full `createIdleEnvStatus` envelope): preflight is the union + // of daemon-locality + ACP-locality cells stitched below, so an + // empty daemon slice IS the right fallback — the ACP slice + // fills in independently from the live channel (or its + // `not_started` placeholders). + daemonCells = []; + } else { + try { + daemonCells = + await opts.statusProvider.getDaemonPreflightCells(boundWorkspace); + } catch (err) { + writeStderrLine( + `qwen serve: statusProvider.getDaemonPreflightCells failed; ` + + `falling back to empty daemon cells: ` + + (err instanceof Error ? err.message : String(err)), + ); + daemonCells = []; + } + } const acpChannelLive = !!liveChannelInfo(); let acpResponse: @@ -4097,227 +4034,6 @@ async function withTimeout( } } -/** - * Daemon-side preflight cells. Always-answerable from the bridge process - * without consulting ACP; the corresponding ACP-side cells (auth, MCP, skills, - * providers, tool_registry, egress) are stitched in by `requestWorkspaceStatus` - * when a child is live, or fall back to `not_started` placeholders when idle. - */ -async function buildDaemonPreflightCells( - boundWorkspace: string, -): Promise { - const REQUIRED_NODE_MAJOR = 22; - - // Each builder returns (or eventually returns) one cell. We run them via - // `Promise.allSettled` after wrapping every call in `Promise.resolve().then` - // so that synchronous throws from any builder become rejected promises - // instead of escaping out of `Promise.all`'s array construction. A throw - // there would propagate up to the route handler and turn the whole - // `/workspace/preflight` envelope into a 500 — directly contradicting the - // design promise that "daemon cells always render even when ACP is sick" - // (see the route handler's catch ladder). - // - // For any rejected slot we synthesize an `error` cell with the slot's - // expected `kind` so the response shape (length, ordering, locality) is - // bit-for-bit the same regardless of failure modes. - const nodeVersionCell = (): ServePreflightCell => { - try { - const nodeVersion = process.versions.node; - const major = Number.parseInt(nodeVersion.split('.')[0] ?? '0', 10); - if (Number.isFinite(major) && major >= REQUIRED_NODE_MAJOR) { - return { - kind: 'node_version', - status: 'ok', - locality: 'daemon', - detail: { - version: nodeVersion, - required: `>=${REQUIRED_NODE_MAJOR}`, - }, - }; - } - return { - kind: 'node_version', - status: 'error', - errorKind: 'missing_binary', - error: `Node ${nodeVersion} is below the required >=${REQUIRED_NODE_MAJOR}.`, - hint: `Upgrade Node to v${REQUIRED_NODE_MAJOR} or newer.`, - locality: 'daemon', - detail: { version: nodeVersion, required: `>=${REQUIRED_NODE_MAJOR}` }, - }; - } catch (err) { - return { - kind: 'node_version', - status: 'error', - error: err instanceof Error ? err.message : String(err), - locality: 'daemon', - }; - } - }; - - // Mirrors `defaultSpawnChannelFactory`'s lookup so the preflight cell - // reflects the path the child would actually be spawned from. - const cliEntryCell = (): ServePreflightCell => { - const cliEntry = process.env['QWEN_CLI_ENTRY'] || process.argv[1] || ''; - if (cliEntry) { - return { - kind: 'cli_entry', - status: 'ok', - locality: 'daemon', - detail: { - path: cliEntry, - source: process.env['QWEN_CLI_ENTRY'] - ? 'QWEN_CLI_ENTRY' - : 'process.argv[1]', - }, - }; - } - return { - kind: 'cli_entry', - status: 'error', - errorKind: 'missing_binary', - error: 'Cannot determine CLI entry path for spawning the ACP child.', - hint: 'Set QWEN_CLI_ENTRY to the absolute path of the qwen entry script.', - locality: 'daemon', - }; - }; - - const workspaceDirCell = async (): Promise => { - try { - const stat = await fs.stat(boundWorkspace); - if (stat.isDirectory()) { - return { - kind: 'workspace_dir', - status: 'ok', - locality: 'daemon', - detail: { path: boundWorkspace }, - }; - } - return { - kind: 'workspace_dir', - status: 'error', - errorKind: 'missing_file', - error: `Bound workspace path is not a directory: ${boundWorkspace}`, - locality: 'daemon', - detail: { path: boundWorkspace }, - }; - } catch (err) { - const errorKind = mapDomainErrorToErrorKind(err); - return { - kind: 'workspace_dir', - status: 'error', - error: err instanceof Error ? err.message : String(err), - ...(errorKind ? { errorKind } : {}), - locality: 'daemon', - detail: { path: boundWorkspace }, - }; - } - }; - - type Slot = { - kind: ServePreflightKind; - run: () => ServePreflightCell | Promise; - }; - const slots: Slot[] = [ - { kind: 'node_version', run: nodeVersionCell }, - { kind: 'cli_entry', run: cliEntryCell }, - { kind: 'workspace_dir', run: workspaceDirCell }, - { - kind: 'ripgrep', - run: () => - safeCheck('ripgrep', async () => { - // Mirror runtime behavior: `Config.useBuiltinRipgrep` defaults to - // `true`, so `canUseRipgrep(true)` reports the *bundled* binary - // when no system `rg` is installed. Passing `false` here would - // tell users "ripgrep missing" while the runtime can still use - // the bundled one — a misleading warning. - const ok = await canUseRipgrep(true); - return ok - ? { status: 'ok' as const } - : { - status: 'warning' as const, - hint: 'Install ripgrep for faster grep tool execution.', - }; - }), - }, - { - kind: 'git', - run: () => - safeCheck('git', async () => { - const v = await getGitVersion(); - return v && v !== 'unknown' - ? { status: 'ok' as const, detail: { version: v } } - : { status: 'warning' as const, hint: 'git not found on PATH.' }; - }), - }, - { - kind: 'npm', - run: () => - safeCheck('npm', async () => { - const v = await getNpmVersion(); - return v && v !== 'unknown' - ? { status: 'ok' as const, detail: { version: v } } - : { status: 'warning' as const, hint: 'npm not found on PATH.' }; - }), - }, - ]; - - // `Promise.resolve().then(run)` coerces sync throws into rejected - // promises so `Promise.allSettled` can absorb them as `error` cells - // rather than letting them escape the route. - const settled = await Promise.allSettled( - slots.map((s) => Promise.resolve().then(s.run)), - ); - return settled.map((result, i) => { - if (result.status === 'fulfilled') return result.value; - const err = result.reason; - const errorKind = mapDomainErrorToErrorKind(err); - return { - kind: slots[i]!.kind, - status: 'error' as const, - locality: 'daemon' as const, - error: err instanceof Error ? err.message : String(err), - ...(errorKind ? { errorKind } : {}), - }; - }); -} - -async function safeCheck( - kind: 'ripgrep' | 'git' | 'npm', - body: () => Promise<{ - status: 'ok' | 'warning'; - detail?: Record; - hint?: string; - }>, -): Promise { - try { - const r = await body(); - return { - kind, - status: r.status, - locality: 'daemon', - ...(r.detail ? { detail: r.detail } : {}), - ...(r.hint ? { hint: r.hint } : {}), - }; - } catch (err) { - // Classify so SDK consumers can render structured remediation - // (`missing_binary` for ENOENT, `missing_file` for EACCES, etc.). - // Without this tag, the rg/git/npm catch path differs from the - // sync-builder catch paths above, which all classify their own - // errors. The outer `Promise.allSettled` catch in - // `buildDaemonPreflightCells` is unreachable for slots whose `run` - // is `() => safeCheck(...)`, because `safeCheck` always resolves - // (its own try/catch swallows). So this is the only place to tag. - const errorKind = mapDomainErrorToErrorKind(err); - return { - kind, - status: 'error', - error: err instanceof Error ? err.message : String(err), - locality: 'daemon', - ...(errorKind ? { errorKind } : {}), - }; - } -} - /** * Default channel factory: spawn the current Node executable running this * CLI's entry script in `--acp` mode. `process.argv[1]` resolves to the qwen diff --git a/packages/cli/src/serve/runQwenServe.ts b/packages/cli/src/serve/runQwenServe.ts index f269a6b65d0..5d463917360 100644 --- a/packages/cli/src/serve/runQwenServe.ts +++ b/packages/cli/src/serve/runQwenServe.ts @@ -16,6 +16,7 @@ import { createHttpAcpBridge, type HttpAcpBridge, } from './httpAcpBridge.js'; +import { createDaemonStatusProvider } from './daemonStatusProvider.js'; import { isLoopbackBind } from './loopbackBinds.js'; import { createDefaultFsAuditEmit, createServeApp } from './server.js'; import type { ServeOptions } from './types.js'; @@ -313,6 +314,12 @@ export async function runQwenServe( : {}), boundWorkspace, childEnvOverrides, + // #4175 PR 22b/2: inject the daemon-host status provider so the + // bridge can pull env / preflight cells through a typed seam + // instead of importing daemon-host helpers directly. Production + // implementation wraps `buildEnvStatusFromProcess` and the + // (lifted) `buildDaemonPreflightCells` body. + statusProvider: createDaemonStatusProvider(), // #4175 Wave 4 PR 17: `POST /session/:id/approval-mode` accepts // an opt-in `persist: true` flag. We re-load settings on each // persist call rather than caching a `LoadedSettings` handle — diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index dbcbacc3dab..773a32baba5 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -28,6 +28,7 @@ import { type DeviceFlowPublicView, } from './auth/deviceFlow.js'; import { QwenOAuthDeviceFlowProvider } from './auth/qwenDeviceFlowProvider.js'; +import { createDaemonStatusProvider } from './daemonStatusProvider.js'; import { isLoopbackBind } from './loopbackBinds.js'; import { canonicalizeWorkspace, @@ -247,6 +248,14 @@ export function createServeApp( ? { eventRingSize: opts.eventRingSize } : {}), boundWorkspace, + // PR 22b/2 (wenshao/gpt-5.5 review fold-in #4304): symmetric + // with `runQwenServe.ts` — direct embeds / tests that don't + // inject `deps.bridge` would otherwise silently lose the + // daemon env + preflight cells the default server app + // reported pre-injection. Wiring the production status provider + // here preserves byte-for-byte route output on the default + // bridge construction path. + statusProvider: createDaemonStatusProvider(), }); // Allow same-origin requests from the demo page. Browsers send an