refactor(acp-bridge): lift BridgeOptions + introduce DaemonStatusProvider seam (#4175 PR 22b/2)

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
 

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": {

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<ServeWorkspaceEnvStatus>;
+
+  /**
+   * 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<ServePreflightCell[]>;
+}
+
+/**
+ * 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 <n>`. 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<string, string | undefined>`. 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<Record<string, string | undefined>>;
+  /**
+   * #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<void>;
+  /**
+   * #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<void>;
+  /**
+   * #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;
+}

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';

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',