diff --git a/docs/design/architecture-deepening-2026-08-21.md b/docs/design/architecture-deepening-2026-08-21.md new file mode 100644 index 00000000000..e6360e44bb7 --- /dev/null +++ b/docs/design/architecture-deepening-2026-08-21.md @@ -0,0 +1,80 @@ +# Architecture deepening pass + +## Goal + +Improve locality and leverage in five high-churn seams without changing user-visible behavior: + +1. make ACP transport-safety policy a deep internal module; +2. make workspace runtime and trust selection a single route policy; +3. remove the CLI-local ACP compatibility re-export shim; +4. centralize Web Shell prompt-admission and retry transitions; +5. centralize WebUI session-attachment ownership transitions. + +This is a staged refactor. Each stage must remain independently testable and must preserve the existing public package and daemon protocols. + +## Behavioral Invariants + +### ACP transport safety + +- Bounded daemon channels keep the existing frame, queue, active-handler, and byte limits. +- Invalid, cyclic, accessor-backed, or oversized values continue to fail closed before entering an unbounded SDK queue. +- Request errors exposed to SDK logging remain redacted, while unbounded public bridges retain their existing error text. +- A transport-budget failure retires the channel exactly once and triggers the existing teardown path. + +### Workspace-qualified routes + +- Workspace selectors continue to accept registered ids and portable absolute paths. +- Missing or inactive runtimes keep the existing `workspace_mismatch` and `workspace_runtime_unavailable` responses. +- Trusted-only routes keep returning `untrusted_workspace` without invoking their operation. +- Read routes retain their permissive cwd fallback; mutation routes retain fail-closed cwd handling. +- Route registration and middleware order in `createServeApp()` do not change. + +### ACP imports + +- The CLI's public exports remain available from the same CLI entrypoint. +- Internal callers import implementation and types from their owning `@qwen-code/acp-bridge` subpath. +- Startup fast paths do not acquire new ACP value imports. + +### Prompt admission and retry + +- Stale session owners cannot commit, retry, or restore prompt state. +- Definitely rejected, admitted, and unknown admission outcomes keep their existing UI behavior. +- Failed-message retry and turn-error retry keep their existing transcript-identity checks. +- The first extraction owns state transitions only; React rendering and daemon transport remain in `App.tsx`. + +### Session attachment + +- A superseded load cannot attach, detach, or report errors for the current session. +- Load watchdogs and detach cleanup remain idempotent. +- Same-session refresh, cross-session switching, missing sessions, and controlled session props retain current behavior. +- The first extraction owns pending-load and attachment epochs only; transcript replay and SSE projection stay in the provider. + +## Module Direction + +The new internal modules must not import their former composition roots. `bridge.ts`, `spawnChannel.ts`, `App.tsx`, and `DaemonSessionProvider.tsx` remain adapters that bind deep policy to their existing runtimes. + +The new interfaces should expose decisions and transitions, not React setters, Express responses, SDK connection objects, or broad dependency bags. A proposed module fails the deletion test if removing it merely inlines a few predicates into its caller. + +## Staging + +1. Move ACP value projection, budgets, redaction, and guard lifecycle behind one internal transport-safety module; retain bridge and spawn-channel integration tests. +2. Add trusted workspace resolution to the existing route policy and migrate qualified routes incrementally. +3. Replace CLI-local shim imports with owning package subpaths, preserve CLI public re-exports, then delete the shim. +4. Extract the prompt-admission attempt and retry transition model, then adapt existing App callbacks to it. +5. Extract pending session-load ownership and attachment epochs, then adapt actions and the provider effect to the shared model. + +## Verification + +- ACP bridge: `bridge.test.ts`, `spawnChannel.test.ts`, package typecheck. +- Serve routes and imports: `workspace-route-runtime.test.ts`, affected route tests, `fast-path.test.ts`, `server-default-bridge-wiring.test.ts`, CLI typecheck. +- Web Shell: focused prompt admission and retry cases in `App.test.tsx`, package typecheck. +- WebUI: `actions.test.ts`, `attachment-lifecycle.test.ts`, focused attachment and session-switch cases in `DaemonSessionProvider.test.tsx`, package typecheck. +- Repository: build, typecheck, formatting/lint checks for changed files, and two clean diff-audit passes. + +## Non-goals + +- Splitting large files to meet a line-count target. +- Changing REST, ACP, SSE, or React context contracts. +- Reordering Express middleware or route registration. +- Rewriting transcript replay, right-panel orchestration, or daemon runtime disposal in this pass. +- Introducing a generic state-machine framework or a new dependency. diff --git a/docs/developers/daemon/02-serve-runtime.md b/docs/developers/daemon/02-serve-runtime.md index 0c525099465..0bec3934143 100644 --- a/docs/developers/daemon/02-serve-runtime.md +++ b/docs/developers/daemon/02-serve-runtime.md @@ -55,7 +55,7 @@ - Event-bus primitives are imported from `@qwen-code/acp-bridge/eventBus`. - Status primitives are imported from `@qwen-code/acp-bridge/status`. -- `serve/acp-session-bridge.ts` remains as the CLI-local compatibility facade for the broader bridge surface. +- The broader bridge surface is imported directly from its owning `@qwen-code/acp-bridge` subpaths. ## Flow diff --git a/docs/developers/daemon/03-acp-bridge.md b/docs/developers/daemon/03-acp-bridge.md index 6a41e54e41c..d492539b5ae 100644 --- a/docs/developers/daemon/03-acp-bridge.md +++ b/docs/developers/daemon/03-acp-bridge.md @@ -266,7 +266,7 @@ calls; it carries `clientId`, `fromLoopback: boolean`, and `promptId`. - `MCP_RESTART_TIMEOUT_MS = 300_000` (5 min) — the bridge timeout for `/workspace/mcp/:server/restart` is intentionally large because `McpClientManager.MAX_DISCOVERY_TIMEOUT_MS` can be up to 5 min for stdio servers. A shorter deadline would produce false timeouts while the ACP child kept reconnecting in the background. - `BridgeOptions.eventRingSize > 1_000_000` throws at construction. - `connection.unstable_resumeSession` is exposed through the stable `session_resume` daemon capability; `unstable_session_resume` remains advertised as a deprecated compatibility alias for older SDKs. Clients should feature-detect `session_resume`. -- The bridge package is `@qwen-code/acp-bridge`. Current code imports event-bus and status primitives directly from package subpaths; `serve/acp-session-bridge.ts` remains as the CLI-local compatibility facade for the broader bridge surface. +- The bridge package is `@qwen-code/acp-bridge`. CLI code imports the implementation, status, errors, and types directly from their owning package subpaths. ## References diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index 43bc183029c..17854e54491 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -3296,7 +3296,7 @@ The connection then closes. | `packages/cli/src/serve/server.ts` | Express app assembly, middleware ordering, and remaining direct routes | | `packages/cli/src/serve/routes/*.ts` | Focused Express route groups, including session, SSE, workspace auth, workspace status, and file routes | | `packages/cli/src/serve/auth.ts` | bearer + Host allowlist + CORS deny | -| `packages/cli/src/serve/acp-session-bridge.ts` | CLI-local bridge compatibility facade for spawn-or-attach, per-session FIFO, and permission registry | +| `packages/acp-bridge/src/bridge.ts` | ACP bridge orchestration for spawn-or-attach, per-session FIFO, and permission registry | | `packages/acp-bridge/src/status.ts` | read-only daemon status wire types + `ServeErrorKind` + `BridgeTimeoutError` + `mapDomainErrorToErrorKind` | | `packages/cli/src/serve/env-snapshot.ts` | pure helper that builds `/workspace/env` payloads from `process.*` state, including credential redaction | | `packages/acp-bridge/src/eventBus.ts` | bounded async queue + replay ring | diff --git a/packages/acp-bridge/README.md b/packages/acp-bridge/README.md index 8dc16ce4174..2666060f42b 100644 --- a/packages/acp-bridge/README.md +++ b/packages/acp-bridge/README.md @@ -118,16 +118,15 @@ either form resolves to the same module at runtime. Pick by intent: Both variants are stable across the F1 lift. -## Backward compatibility +## CLI integration CLI code imports event-bus and in-memory channel primitives directly from `@qwen-code/acp-bridge/eventBus` and `@qwen-code/acp-bridge/inMemoryChannel`. -`packages/cli/src/serve/acp-session-bridge.ts` remains as the CLI-local -compatibility facade for the broader bridge surface, forwarding previously -exported symbols (`createHttpAcpBridge`, `defaultSpawnChannelFactory`, -`BridgeClient`, typed errors, and type aliases) from the lifted subpaths. +The CLI imports the broader bridge surface from its owning package subpaths. +Its public serve entrypoint still re-exports the supported bridge factories, +types, errors, and status helpers for downstream compatibility. ## See also diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index 25ae83e8844..32614e72508 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -6,7 +6,6 @@ import { randomBytes, randomUUID } from 'node:crypto'; import * as path from 'node:path'; -import { inspect } from 'node:util'; import { ClientSideConnection, PROTOCOL_VERSION, @@ -14,7 +13,6 @@ import { } from '@agentclientprotocol/sdk'; import type { CancelNotification, - Client, ContentBlock, PromptRequest, PromptResponse, @@ -50,7 +48,11 @@ import { type ShellOutputEvent, } from '@qwen-code/qwen-code-core'; import type { ShellCommandResult } from './bridgeTypes.js'; -import type { AcpChannel, AcpChannelTransportGuard } from './channel.js'; +import type { AcpChannel } from './channel.js'; +import { + createLogSafeAcpClient, + createOutboundGuardedConnection, +} from './transport-safety.js'; import { EventBus, DEFAULT_RING_SIZE, @@ -382,337 +384,6 @@ function isDefinitiveAcpRequestError(error: unknown): boolean { ); } -class LogSafeAcpRequestError extends RequestError { - constructor( - code: number, - message: string, - data: unknown, - private readonly reservePreparedResponse?: (value: unknown) => void, - ) { - super(code, message, data); - } - - override toResult() { - const result = super.toResult(); - if ('error' in result) { - Object.defineProperty(result.error, inspect.custom, { - configurable: true, - value: () => ({ code: result.error.code, payloadOmitted: true }), - }); - try { - this.reservePreparedResponse?.(result.error); - } catch { - // The guard already retired the transport. The ACP SDK does not await - // its message dispatcher, so throwing from toResult would be unhandled. - } - } - return result; - } -} - -const MAX_LOG_SAFE_ACP_ERROR_DETAILS_CHARS = 1_024; -const MAX_LOG_SAFE_ACP_ERROR_KIND_CHARS = 128; -const MAX_LOG_SAFE_ACP_ERROR_HINT_CHARS = 512; - -function logSafeAcpErrorDetails( - error: unknown, -): { details: string } | undefined { - const details = - error instanceof Error - ? error.message - : isRecord(error) && typeof error['message'] === 'string' - ? error['message'] - : undefined; - if (!details) return undefined; - return { - details: - details.length <= MAX_LOG_SAFE_ACP_ERROR_DETAILS_CHARS - ? details - : `${details.slice(0, MAX_LOG_SAFE_ACP_ERROR_DETAILS_CHARS)}…`, - }; -} - -function boundedAcpErrorString(value: string, maxChars: number): string { - return value.length <= maxChars ? value : `${value.slice(0, maxChars)}…`; -} - -function logSafeRequestErrorMessage(code: number): string { - switch (code) { - case -32700: - return 'Parse error'; - case -32600: - return 'Invalid request'; - case -32601: - return 'Method not found'; - case -32602: - return 'Invalid params'; - case -32603: - return 'Internal error'; - case -32000: - return 'Authentication required'; - case -32002: - return 'Resource not found'; - default: - return 'ACP client request failed'; - } -} - -function logSafeRequestErrorData(data: unknown): unknown { - if (!isRecord(data) || typeof data['errorKind'] !== 'string') { - return undefined; - } - const status = data['status']; - const hint = data['hint']; - return { - errorKind: boundedAcpErrorString( - data['errorKind'], - MAX_LOG_SAFE_ACP_ERROR_KIND_CHARS, - ), - ...(typeof status === 'number' && Number.isFinite(status) - ? { status } - : {}), - ...(typeof hint === 'string' - ? { - hint: boundedAcpErrorString(hint, MAX_LOG_SAFE_ACP_ERROR_HINT_CHARS), - } - : {}), - }; -} - -class AcpInboundHandlerLimitError extends Error { - readonly code = 'acp_handler_limit_exceeded'; - - constructor( - readonly maxActiveHandlers: number, - readonly maxActiveHandlerBytes: number, - readonly requiredBytes: number, - readonly availableBytes: number, - ) { - super('ACP inbound handler capacity exceeded'); - this.name = 'AcpInboundHandlerLimitError'; - } -} - -function estimateAcpHandlerBytes(value: unknown, limitBytes: number): number { - let bytes = 0; - const stack: unknown[] = [value]; - const seen = new WeakSet(); - while (stack.length > 0) { - const current = stack.pop(); - if (current === null) { - bytes += 4; - } else if (typeof current === 'string') { - bytes += Buffer.byteLength(current) + 2; - } else if (typeof current === 'number') { - bytes += 24; - } else if (typeof current === 'boolean') { - bytes += 5; - } else if (Array.isArray(current)) { - if (seen.has(current)) return limitBytes + 1; - seen.add(current); - bytes += 2 + Math.max(0, current.length - 1); - if (bytes + current.length > limitBytes) return limitBytes + 1; - for (let index = current.length - 1; index >= 0; index--) { - stack.push(current[index]); - } - } else if (isRecord(current)) { - if (seen.has(current)) return limitBytes + 1; - seen.add(current); - const entries = Object.entries(current); - bytes += 2 + Math.max(0, entries.length - 1); - for (const [key, entryValue] of entries) { - bytes += Buffer.byteLength(key) + 3; - stack.push(entryValue); - } - } else { - bytes += 4; - } - if (bytes > limitBytes) return limitBytes + 1; - } - return Math.max(1, bytes); -} - -class AcpInboundHandlerAdmission { - private activeHandlers = 0; - private activeBytes = 0; - - constructor(private readonly guard: AcpChannelTransportGuard) {} - - async run(params: unknown, operation: () => Promise): Promise { - const envelopeBytes = Math.min(2_048, this.guard.maxActiveHandlerBytes); - const requiredBytes = - envelopeBytes + - estimateAcpHandlerBytes( - params, - Math.max(0, this.guard.maxActiveHandlerBytes - envelopeBytes), - ); - const availableBytes = Math.max( - 0, - this.guard.maxActiveHandlerBytes - this.activeBytes, - ); - if ( - this.activeHandlers >= this.guard.maxActiveHandlers || - requiredBytes > availableBytes - ) { - const error = new AcpInboundHandlerLimitError( - this.guard.maxActiveHandlers, - this.guard.maxActiveHandlerBytes, - requiredBytes, - availableBytes, - ); - this.guard.fail(error); - throw error; - } - this.activeHandlers++; - this.activeBytes += requiredBytes; - try { - return await operation(); - } finally { - this.activeHandlers--; - this.activeBytes -= requiredBytes; - } - } -} - -async function withLogSafeAcpError( - operation: () => Promise, - reservePreparedResponse?: (value: unknown) => void, -): Promise { - try { - return await operation(); - } catch (error) { - if (error instanceof RequestError) { - const code = Number.isFinite(error.code) ? error.code : -32603; - throw new LogSafeAcpRequestError( - code, - logSafeRequestErrorMessage(code), - logSafeRequestErrorData(error.data), - reservePreparedResponse, - ); - } - throw new LogSafeAcpRequestError( - -32603, - 'Internal error', - logSafeAcpErrorDetails(error), - reservePreparedResponse, - ); - } -} - -function createLogSafeAcpClient( - client: Client, - transportGuard: AcpChannelTransportGuard, -): Client { - const admission = new AcpInboundHandlerAdmission(transportGuard); - const runNotification = (params: unknown, operation: () => Promise) => - withLogSafeAcpError(() => admission.run(params, operation)); - const runRequest = (params: unknown, operation: () => Promise) => - withLogSafeAcpError( - () => - admission.run(params, async () => { - const result = await operation(); - transportGuard.reservePreparedResponse(result ?? null); - return result; - }), - transportGuard.reservePreparedResponse, - ); - return { - requestPermission: (params) => - runRequest(params, () => client.requestPermission(params)), - sessionUpdate: (params) => - runNotification(params, () => client.sessionUpdate(params)), - writeTextFile: client.writeTextFile - ? (params) => runRequest(params, () => client.writeTextFile!(params)) - : undefined, - readTextFile: client.readTextFile - ? (params) => runRequest(params, () => client.readTextFile!(params)) - : undefined, - createTerminal: client.createTerminal - ? (params) => runRequest(params, () => client.createTerminal!(params)) - : undefined, - terminalOutput: client.terminalOutput - ? (params) => runRequest(params, () => client.terminalOutput!(params)) - : undefined, - releaseTerminal: client.releaseTerminal - ? (params) => - runRequest( - params, - async () => (await client.releaseTerminal!(params)) ?? {}, - ) - : undefined, - waitForTerminalExit: client.waitForTerminalExit - ? (params) => - runRequest(params, () => client.waitForTerminalExit!(params)) - : undefined, - killTerminal: client.killTerminal - ? (params) => - runRequest( - params, - async () => (await client.killTerminal!(params)) ?? {}, - ) - : undefined, - extMethod: client.extMethod - ? (method, params) => - runRequest(params, () => client.extMethod!(method, params)) - : undefined, - extNotification: client.extNotification - ? (method, params) => - runNotification(params, () => client.extNotification!(method, params)) - : undefined, - }; -} - -const OUTBOUND_GUARDED_CONNECTION_METHODS = new Set([ - 'initialize', - 'newSession', - 'loadSession', - 'unstable_forkSession', - 'unstable_listSessions', - 'unstable_resumeSession', - 'setSessionMode', - 'unstable_setSessionModel', - 'setSessionConfigOption', - 'authenticate', - 'prompt', - 'cancel', - 'extMethod', - 'extNotification', -]); - -function createOutboundGuardedConnection( - connection: ClientSideConnection, - transportGuard: AcpChannelTransportGuard, -): ClientSideConnection { - const wrappers = new Map< - PropertyKey, - (...args: unknown[]) => Promise - >(); - return new Proxy(connection, { - get(target, property) { - const value = Reflect.get(target, property, target); - if ( - typeof value !== 'function' || - !OUTBOUND_GUARDED_CONNECTION_METHODS.has(property) - ) { - return value; - } - let wrapper = wrappers.get(property); - if (!wrapper) { - wrapper = async (...args: unknown[]) => { - const release = transportGuard.reserveOutboundOperation(args); - try { - return await Reflect.apply(value, target, args); - } finally { - release(); - } - }; - wrappers.set(property, wrapper); - } - return wrapper; - }, - }); -} - function getCanonicalModelId(response: unknown, fallback: string): string { if (!isRecord(response) || !isRecord(response['_meta'])) return fallback; const modelSwitch = response['_meta']['qwenModelSwitch']; diff --git a/packages/acp-bridge/src/channel.ts b/packages/acp-bridge/src/channel.ts index 6d7d47eb8d0..a91ca140170 100644 --- a/packages/acp-bridge/src/channel.ts +++ b/packages/acp-bridge/src/channel.ts @@ -5,14 +5,9 @@ */ import type { Stream } from '@agentclientprotocol/sdk'; +import type { AcpChannelTransportGuard } from './transport-safety.js'; -export interface AcpChannelTransportGuard { - maxActiveHandlers: number; - maxActiveHandlerBytes: number; - reserveOutboundOperation(value: unknown): () => void; - reservePreparedResponse(value: unknown): void; - fail(error: unknown): void; -} +export type { AcpChannelTransportGuard } from './transport-safety.js'; /** * One ACP NDJSON channel to a single agent. Tests inject a fake by diff --git a/packages/acp-bridge/src/spawnChannel.ts b/packages/acp-bridge/src/spawnChannel.ts index 63c1fa3655e..0608d6563e0 100644 --- a/packages/acp-bridge/src/spawnChannel.ts +++ b/packages/acp-bridge/src/spawnChannel.ts @@ -26,18 +26,17 @@ import { import type { ChannelFactory } from './channel.js'; import { redactLogCredentials } from './logRedaction.js'; import { - NdJsonQueueLimitError, ndJsonStream, type NdJsonMessageObservation, type NdJsonStreamHooks, type NdJsonStreamLimits, validateNdJsonStreamLimits, } from './ndJsonStream.js'; +import { createBoundedAcpTransportSafety } from './transport-safety.js'; import { MissingCliEntryError } from './status.js'; import { EXTERNAL_TOOL_GUARD_TOKEN_ENV } from './externalToolGuard.js'; import { ProcessRegistry } from './process-registry.js'; import type { ChildHeapPolicy } from './child-heap-policy.js'; -import { estimateJsonStringBytes } from './json-string-bytes.js'; let cachedMemoryArgs: string[] | undefined; export const DAEMON_ACP_NDJSON_LIMITS: Readonly = @@ -68,252 +67,6 @@ function validateDaemonInboundMessage(message: AnyMessage): boolean { return !validator || validator.safeParse(message.params).success; } -class PreparedResponseBudget { - private objectCharges = new WeakMap(); - private readonly primitiveCharges = new Map(); - private retainedCount = 0; - private retainedBytes = 0; - private closed = false; - - constructor(private readonly limits: NdJsonStreamLimits) {} - - reserve(value: unknown): void { - if (this.closed) return; - const availableBytes = Math.max( - 0, - this.limits.maxQueuedBytes - this.retainedBytes, - ); - const envelopeBytes = Math.min(2_048, this.limits.maxQueuedBytes); - const charge = - envelopeBytes + - estimatePreparedResponseBytes( - value, - Math.max(0, availableBytes - envelopeBytes), - ); - if ( - this.retainedCount >= this.limits.maxQueuedMessages || - charge > availableBytes - ) { - throw new NdJsonQueueLimitError( - this.limits.maxQueuedMessages, - this.limits.maxQueuedBytes, - charge, - availableBytes, - ); - } - const charges = this.getCharges(value, true); - charges.push(charge); - this.retainedCount++; - this.retainedBytes += charge; - } - - releaseMessage(message: unknown): void { - if ( - !isPlainRecord(message) || - Object.hasOwn(message, 'method') || - !Object.hasOwn(message, 'id') - ) { - return; - } - const value = Object.hasOwn(message, 'result') - ? message['result'] - : message['error']; - const charges = this.getCharges(value, false); - if (!charges) return; - const charge = charges.shift(); - if (charge === undefined) return; - if (charges.length === 0 && !isObjectValue(value)) { - this.primitiveCharges.delete(value); - } - this.retainedCount--; - this.retainedBytes -= charge; - } - - close(): void { - this.closed = true; - this.objectCharges = new WeakMap(); - this.primitiveCharges.clear(); - this.retainedCount = 0; - this.retainedBytes = 0; - } - - private getCharges(value: unknown, create: true): number[]; - private getCharges(value: unknown, create: false): number[] | undefined; - private getCharges(value: unknown, create: boolean): number[] | undefined { - const charges = isObjectValue(value) - ? this.objectCharges.get(value) - : this.primitiveCharges.get(value); - if (charges || !create) return charges; - const created: number[] = []; - if (isObjectValue(value)) { - this.objectCharges.set(value, created); - } else { - this.primitiveCharges.set(value, created); - } - return created; - } -} - -class OutboundOperationBudget { - private retainedCount = 0; - private retainedBytes = 0; - private generation = 0; - private closed = false; - - constructor(private readonly limits: NdJsonStreamLimits) {} - - reserve(value: unknown): () => void { - if (this.closed) return () => {}; - const availableBytes = Math.max( - 0, - this.limits.maxQueuedBytes - this.retainedBytes, - ); - const envelopeBytes = Math.min(2_048, this.limits.maxQueuedBytes); - const charge = - envelopeBytes + - estimatePreparedResponseBytes( - value, - Math.max(0, availableBytes - envelopeBytes), - ); - if ( - this.retainedCount >= this.limits.maxQueuedMessages || - charge > availableBytes - ) { - throw new NdJsonQueueLimitError( - this.limits.maxQueuedMessages, - this.limits.maxQueuedBytes, - charge, - availableBytes, - ); - } - this.retainedCount++; - this.retainedBytes += charge; - const generation = this.generation; - let active = true; - return () => { - if (!active) return; - active = false; - if (this.closed || this.generation !== generation) return; - this.retainedCount--; - this.retainedBytes -= charge; - }; - } - - close(): void { - this.closed = true; - this.generation++; - this.retainedCount = 0; - this.retainedBytes = 0; - } -} - -type PreparedResponseEstimationFrame = - | { kind: 'value'; value: unknown } - | { kind: 'array'; value: unknown[]; index: number } - | { - kind: 'record'; - value: Record; - keys: Generator; - first: boolean; - }; - -function* enumerableOwnKeys( - value: Record, -): Generator { - for (const key in value) { - if (Object.hasOwn(value, key)) yield key; - } -} - -function estimatePreparedResponseBytes(value: unknown, limitBytes: number) { - let bytes = 0; - const stack: PreparedResponseEstimationFrame[] = [{ kind: 'value', value }]; - const seen = new WeakSet(); - while (stack.length > 0) { - const frame = stack.pop()!; - if (frame.kind === 'array') { - if (frame.index >= frame.value.length) continue; - if (frame.index > 0) bytes++; - if (bytes > limitBytes) return limitBytes + 1; - const descriptor = Object.getOwnPropertyDescriptor( - frame.value, - String(frame.index), - ); - if (descriptor?.get || descriptor?.set) return limitBytes + 1; - stack.push({ ...frame, index: frame.index + 1 }); - stack.push({ kind: 'value', value: descriptor?.value }); - continue; - } - if (frame.kind === 'record') { - const next = frame.keys.next(); - if (next.done) continue; - const descriptor = Object.getOwnPropertyDescriptor( - frame.value, - next.value, - ); - if (!descriptor || descriptor.get || descriptor.set) { - return limitBytes + 1; - } - bytes += - (frame.first ? 0 : 1) + - estimateJsonStringBytes(next.value, Math.max(0, limitBytes - bytes)) + - 1; - if (bytes > limitBytes) return limitBytes + 1; - stack.push({ ...frame, first: false }); - stack.push({ kind: 'value', value: descriptor.value }); - continue; - } - const current = frame.value; - if (current === null) { - bytes += 4; - } else if (current === undefined) { - bytes += 4; - } else if (typeof current === 'string') { - bytes += estimateJsonStringBytes( - current, - Math.max(0, limitBytes - bytes), - ); - } else if (typeof current === 'number') { - bytes += 24; - } else if (typeof current === 'boolean') { - bytes += 5; - } else if (Array.isArray(current)) { - if (seen.has(current) || Object.hasOwn(current, 'toJSON')) { - return limitBytes + 1; - } - seen.add(current); - bytes += 2; - stack.push({ kind: 'array', value: current, index: 0 }); - } else if (isPlainRecord(current)) { - if (seen.has(current) || Object.hasOwn(current, 'toJSON')) { - return limitBytes + 1; - } - seen.add(current); - bytes += 2; - stack.push({ - kind: 'record', - value: current, - keys: enumerableOwnKeys(current), - first: true, - }); - } else { - return limitBytes + 1; - } - if (bytes > limitBytes) return limitBytes + 1; - } - return Math.max(1, bytes); -} - -function isObjectValue(value: unknown): value is object { - return typeof value === 'object' && value !== null; -} - -function isPlainRecord(value: unknown): value is Record { - if (!isObjectValue(value) || Array.isArray(value)) return false; - const prototype = Object.getPrototypeOf(value); - return prototype === Object.prototype || prototype === null; -} - export function getAcpMemoryArgs(): string[] { if (cachedMemoryArgs) return cachedMemoryArgs; const constrainedMemory = (process as { constrainedMemory?: () => number }) @@ -519,43 +272,22 @@ export function createSpawnChannelFactory( const writable = Writable.toWeb(child.stdin) as WritableStream; const readable = Readable.toWeb(child.stdout) as ReadableStream; - const preparedResponses = options.pipeLimits - ? new PreparedResponseBudget(options.pipeLimits) - : undefined; - const outboundOperations = options.pipeLimits - ? new OutboundOperationBudget(options.pipeLimits) - : undefined; - let reportTransportFailure: ((error: unknown) => void) | undefined; - let transportFailureReported = false; - const transportFailed = options.pipeLimits - ? new Promise((resolve) => { - reportTransportFailure = resolve; - }) - : undefined; - const failTransport = options.pipeLimits - ? (error: unknown) => { - if (transportFailureReported) return; - transportFailureReported = true; - reportTransportFailure?.(error); - reportTransportFailure = undefined; - preparedResponses?.close(); - outboundOperations?.close(); + const transportSafety = options.pipeLimits + ? createBoundedAcpTransportSafety(options.pipeLimits, (error) => { child.stdout?.destroy(); child.stdin?.destroy(); void trackedChild.terminate().catch(() => {}); options.pipeHooks?.onTransportError?.(error); - } + }) : undefined; - const pipeHooks = options.pipeLimits + const pipeHooks = transportSafety ? { ...options.pipeHooks, onMessageObserved: (observation: NdJsonMessageObservation) => { - if (observation.direction === 'sent') { - preparedResponses?.releaseMessage(observation.message); - } + transportSafety.observeMessage(observation); options.pipeHooks?.onMessageObserved?.(observation); }, - onTransportError: failTransport, + onTransportError: transportSafety.guard.fail, } : options.pipeHooks; const stream = ndJsonStream( @@ -569,30 +301,10 @@ export function createSpawnChannelFactory( return { stream, - ...(transportFailed ? { transportFailed } : {}), - ...(failTransport && options.pipeLimits + ...(transportSafety ? { - transportGuard: { - maxActiveHandlers: options.pipeLimits.maxQueuedMessages, - maxActiveHandlerBytes: options.pipeLimits.maxQueuedBytes, - reserveOutboundOperation: (value) => { - try { - return outboundOperations?.reserve(value) ?? (() => {}); - } catch (error) { - failTransport(error); - throw error; - } - }, - reservePreparedResponse: (value) => { - try { - preparedResponses?.reserve(value); - } catch (error) { - failTransport(error); - throw error; - } - }, - fail: failTransport, - }, + transportFailed: transportSafety.transportFailed, + transportGuard: transportSafety.guard, } : {}), kill: () => trackedChild.terminate(), diff --git a/packages/acp-bridge/src/transport-safety.test.ts b/packages/acp-bridge/src/transport-safety.test.ts new file mode 100644 index 00000000000..bcb0212560b --- /dev/null +++ b/packages/acp-bridge/src/transport-safety.test.ts @@ -0,0 +1,425 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it, vi } from 'vitest'; +import type { Client } from '@agentclientprotocol/sdk'; +import { + createBoundedAcpTransportSafety, + createLogSafeAcpClient, + estimateTransportValueBytes, + type AcpChannelTransportGuard, +} from './transport-safety.js'; + +const GENEROUS_LIMIT = Number.MAX_SAFE_INTEGER; + +describe('estimateTransportValueBytes', () => { + it('estimates plain-data toJSON keys like any other member', () => { + // After JSON.parse a `toJSON` key is plain data — never callable — and + // JSON.stringify serializes it as an ordinary member, so it must be + // estimated, not refused. Refusing it retired whole daemon transports + // over wire-legal inbound messages. + const parsed = JSON.parse( + '{"sessionId":"s","rawInput":{"toJSON":"model-data"},"tags":[{"toJSON":"v"}]}', + ); + expect(estimateTransportValueBytes(parsed, GENEROUS_LIMIT)).toBe( + Buffer.byteLength(JSON.stringify(parsed)), + ); + }); + + it('estimates arrays and records carrying an own plain-data toJSON key', () => { + const recordWithToJSON = { toJSON: 'data', payload: 'p' }; + expect(estimateTransportValueBytes(recordWithToJSON, GENEROUS_LIMIT)).toBe( + Buffer.byteLength(JSON.stringify(recordWithToJSON)), + ); + const arrayWithToJSON = Object.assign(['a', 'b'], { toJSON: 'data' }); + // JSON.stringify serializes arrays by index only; the estimator matches. + expect(estimateTransportValueBytes(arrayWithToJSON, GENEROUS_LIMIT)).toBe( + Buffer.byteLength(JSON.stringify(arrayWithToJSON)), + ); + }); + + it('estimates boolean and JSON null values on their dedicated arms', () => { + // The boolean arm and the null half of the null/undefined arm are the + // deciding checks here: with either arm deleted the value falls through + // to the fail-closed catch-all and the estimate flips to the refusal + // sentinel. Both arms must stay bounded, or wire-legal JSON `true` / + // `false` / `null` params would retire healthy daemon transports. + expect( + estimateTransportValueBytes({ flag: true, gone: null }, 1024), + ).toBeLessThan(1025); + const parsed = JSON.parse( + '{"sessionId":"s","rawInput":null,"tags":[null]}', + ); + expect(estimateTransportValueBytes(parsed, GENEROUS_LIMIT)).toBe( + Buffer.byteLength(JSON.stringify(parsed)), + ); + }); + + it('still refuses callable toJSON values, which substitute stringify output', () => { + const record = { toJSON: () => ({ substituted: true }) }; + expect(estimateTransportValueBytes(record, 1024)).toBe(1025); + const array = Object.assign([1], { toJSON: () => [2] }); + expect(estimateTransportValueBytes(array, 1024)).toBe(1025); + }); + + it('still refuses accessor-backed toJSON values', () => { + const getterBacked: Record = {}; + Object.defineProperty(getterBacked, 'toJSON', { + enumerable: true, + configurable: true, + get: () => () => ({ substituted: true }), + }); + expect(estimateTransportValueBytes(getterBacked, 1024)).toBe(1025); + const setterBacked: Record = {}; + Object.defineProperty(setterBacked, 'toJSON', { + enumerable: true, + configurable: true, + set: () => {}, + }); + expect(estimateTransportValueBytes(setterBacked, 1024)).toBe(1025); + // For ARRAYS the hazard function is the sole defense: the array frame + // only inspects index descriptors, so an accessor-backed own `toJSON` + // must be refused by hasStructuralToJSONHazard's get/set condition + // itself. Dropping that condition leaves the record cases above green + // (the record frame refuses accessor keys independently) but admits + // these arrays. + const arrayGetter: unknown[] = [1]; + Object.defineProperty(arrayGetter, 'toJSON', { + enumerable: true, + configurable: true, + get: () => () => [2], + }); + expect(estimateTransportValueBytes(arrayGetter, 1024)).toBe(1025); + const arraySetter: unknown[] = [1]; + Object.defineProperty(arraySetter, 'toJSON', { + enumerable: true, + configurable: true, + set: () => {}, + }); + expect(estimateTransportValueBytes(arraySetter, 1024)).toBe(1025); + }); + + it('refuses accessor-backed members and indices at the frame level', () => { + // An accessor under a non-`toJSON` key never reaches + // hasStructuralToJSONHazard; the record frame's descriptor check is the + // deciding refusal. + const getterMember: Record = { keep: 'v' }; + Object.defineProperty(getterMember, 'payload', { + enumerable: true, + configurable: true, + get: () => 'x', + }); + expect(estimateTransportValueBytes(getterMember, 1024)).toBe(1025); + const setterMember: Record = { keep: 'v' }; + Object.defineProperty(setterMember, 'payload', { + enumerable: true, + configurable: true, + set: () => {}, + }); + expect(estimateTransportValueBytes(setterMember, 1024)).toBe(1025); + // Likewise for array indices: the array frame inspects each index + // descriptor and must refuse accessors. + const getterIndex: unknown[] = ['a']; + Object.defineProperty(getterIndex, '0', { + enumerable: true, + configurable: true, + get: () => 'x', + }); + expect(estimateTransportValueBytes(getterIndex, 1024)).toBe(1025); + const setterIndex: unknown[] = ['a']; + Object.defineProperty(setterIndex, '0', { + enumerable: true, + configurable: true, + set: () => {}, + }); + expect(estimateTransportValueBytes(setterIndex, 1024)).toBe(1025); + }); + + it('fails closed on nested non-plain values via the catch-all branch', () => { + // A function, a class instance, or a Map nested inside plain containers + // is skipped or substituted by JSON.stringify; the only check that can + // catch such values is the estimator's final fail-closed else, because + // every earlier check (toJSON hazard, frame-level accessors, cycles) + // sees only the plain wrappers. + expect(estimateTransportValueBytes({ nested: () => 1 }, 1024)).toBe(1025); + expect(estimateTransportValueBytes({ d: new Date() }, 1024)).toBe(1025); + expect(estimateTransportValueBytes([new Map()], 1024)).toBe(1025); + }); + + it('still refuses cyclic values', () => { + const cyclic: Record = {}; + cyclic['self'] = cyclic; + expect(estimateTransportValueBytes(cyclic, 1024)).toBe(1025); + }); + + it('refuses array cycles without re-walking the cycle', () => { + const cyclicArray: unknown[] = []; + cyclicArray.push(cyclicArray); + expect(estimateTransportValueBytes(cyclicArray, 1024)).toBe(1025); + + // The harm of a missing array-leg cycle check is WORK, not result: the + // byte limit still trips eventually, so the refusal assertion above + // passes either way. Count descriptor reads through a proxy to bound + // the walk deterministically — the pristine estimator reads index 0 + // once before `seen` refuses the re-entry, while a walker without the + // array-leg check re-reads it on every descent until the byte limit + // trips (~limitBytes / 2 reads). + let indexReads = 0; + const target: unknown[] = []; + const proxied: unknown[] = new Proxy(target, { + getOwnPropertyDescriptor(proxyTarget, prop) { + if (prop === '0') indexReads++; + return Reflect.getOwnPropertyDescriptor(proxyTarget, prop); + }, + }); + target.push(proxied); + expect(estimateTransportValueBytes(proxied, 1024)).toBe(1025); + expect(indexReads).toBeLessThan(8); + }); +}); + +function createFakeGuard(): AcpChannelTransportGuard & { + fail: ReturnType; +} { + return { + maxActiveHandlers: 8, + maxActiveHandlerBytes: 64 * 1024 * 1024, + reserveOutboundOperation: () => () => {}, + reservePreparedResponse: () => {}, + fail: vi.fn(), + }; +} + +function createFakeClient() { + const sessionUpdate = vi.fn(async () => {}); + const client = { + requestPermission: vi.fn(async () => ({})), + sessionUpdate, + } as unknown as Client; + return { client, sessionUpdate }; +} + +describe('inbound handler admission', () => { + it('admits handlers whose params carry plain-data toJSON keys', () => { + const guard = createFakeGuard(); + const { client, sessionUpdate } = createFakeClient(); + const wrapped = createLogSafeAcpClient(client, guard); + // The exact wire shape: JSON.parse'd ACP params where model/MCP content + // (rawInput is z.unknown() in the ACP SDK) contains a literal "toJSON" + // data key. The old lenient inbound estimator admitted these; the strict + // estimator must too, because such values pose no serialization hazard. + const params = JSON.parse( + '{"sessionId":"s","update":{"sessionUpdate":"tool_call_update","rawInput":{"toJSON":"model-data"}}}', + ); + return expect(wrapped.sessionUpdate(params)) + .resolves.toBeUndefined() + .then(() => { + expect(sessionUpdate).toHaveBeenCalledTimes(1); + expect(guard.fail).not.toHaveBeenCalled(); + }); + }); + + it('fails closed when params carry a callable toJSON value', async () => { + const guard = createFakeGuard(); + const { client, sessionUpdate } = createFakeClient(); + const wrapped = createLogSafeAcpClient(client, guard); + await expect( + wrapped.sessionUpdate({ + sessionId: 's', + rawInput: { toJSON: () => ({ substituted: true }) }, + } as never), + ).rejects.toThrow(); + expect(sessionUpdate).not.toHaveBeenCalled(); + expect(guard.fail).toHaveBeenCalledTimes(1); + }); + + it('keeps admission independent of the estimation refusal semantics for ordinary frames', async () => { + const guard = createFakeGuard(); + const { client, sessionUpdate } = createFakeClient(); + const wrapped = createLogSafeAcpClient(client, guard); + await wrapped.sessionUpdate({ + sessionId: 's', + update: { sessionUpdate: 'plan', entries: [] }, + } as never); + expect(sessionUpdate).toHaveBeenCalledTimes(1); + expect(guard.fail).not.toHaveBeenCalled(); + }); + + it('releases the admission budget after a handler completes', async () => { + // 3000 bytes fits exactly one small frame (2048-byte envelope + params), + // and `maxActiveHandlers: 1` fits exactly one in-flight handler; a + // second sequential call is admitted only if the first call's budget + // was released by BOTH `finally` decrements in the admission run — the + // byte decrement (`activeBytes -= requiredBytes`) and the paired count + // decrement (`activeHandlers--`). Capping only bytes would let a leaked + // handler count survive until the channel's lifetime request count + // reaches maxActiveHandlers, after which every inbound call fails + // admission and guard.fail retires the transport mid-session. + const guard = { + ...createFakeGuard(), + maxActiveHandlers: 1, + maxActiveHandlerBytes: 3000, + }; + const { client, sessionUpdate } = createFakeClient(); + const wrapped = createLogSafeAcpClient(client, guard); + const params = { + sessionId: 's', + update: { sessionUpdate: 'plan', entries: [] }, + } as never; + + await wrapped.sessionUpdate(params); + await wrapped.sessionUpdate(params); + + expect(sessionUpdate).toHaveBeenCalledTimes(2); + expect(guard.fail).not.toHaveBeenCalled(); + }); + + it('keeps the admission budget held while a handler is in flight', async () => { + const guard = { ...createFakeGuard(), maxActiveHandlerBytes: 3000 }; + let release!: () => void; + const held = new Promise((resolve) => { + release = resolve; + }); + const sessionUpdate = vi.fn(async () => { + await held; + }); + const client = { + requestPermission: vi.fn(async () => ({})), + sessionUpdate, + } as unknown as Client; + const wrapped = createLogSafeAcpClient(client, guard); + const params = { + sessionId: 's', + update: { sessionUpdate: 'plan', entries: [] }, + } as never; + + const first = wrapped.sessionUpdate(params); + // The second frame arrives while the first still holds the budget: it + // must fail closed rather than share a budget that has not been freed. + await expect(wrapped.sessionUpdate(params)).rejects.toThrow(); + expect(sessionUpdate).toHaveBeenCalledTimes(1); + expect(guard.fail).toHaveBeenCalledTimes(1); + + release(); + await first; + }); + + it('reserves the prepared response on the request success path', async () => { + const reservePreparedResponse = vi.fn(); + const guard = { ...createFakeGuard(), reservePreparedResponse }; + const requestPermission = vi.fn(async () => ({ + outcome: { outcome: 'selected', optionId: 'allow' }, + })); + const client = { + requestPermission, + sessionUpdate: vi.fn(async () => {}), + } as unknown as Client; + const wrapped = createLogSafeAcpClient(client, guard); + + const result = await wrapped.requestPermission({ + sessionId: 's', + options: [], + } as never); + + expect(requestPermission).toHaveBeenCalledTimes(1); + expect(result).toEqual({ + outcome: { outcome: 'selected', optionId: 'allow' }, + }); + // runRequest must charge the retained response against maxQueuedBytes; + // dropping the success-path reservation would let large request + // responses queue uncharged. + expect(reservePreparedResponse).toHaveBeenCalledTimes(1); + expect(reservePreparedResponse).toHaveBeenCalledWith(result); + }); + + it('reserves under null when a request handler resolves undefined', async () => { + // The ACP SDK sends `{ result: result ?? null }` on the wire, so + // runRequest must reserve under the same normalized value: a handler + // resolving `undefined` that reserved under `undefined` would charge + // under a key the release path never sees — releaseMessage looks the + // sent frame's `result: null` up, misses, and the charge leaks until + // the queue limit retires the transport. + const reservePreparedResponse = vi.fn(); + const guard = { ...createFakeGuard(), reservePreparedResponse }; + const extMethod = vi.fn( + async (): Promise> => undefined as never, + ); + const client = { + requestPermission: vi.fn(async () => ({})), + sessionUpdate: vi.fn(async () => {}), + extMethod, + } as unknown as Client; + const wrapped = createLogSafeAcpClient(client, guard); + + await wrapped.extMethod!('x/status', { sessionId: 's' }); + + expect(extMethod).toHaveBeenCalledTimes(1); + expect(reservePreparedResponse).toHaveBeenCalledTimes(1); + expect(reservePreparedResponse).toHaveBeenCalledWith(null); + }); + + it('releases the undefined-resolving charge through the null wire frame', async () => { + // Drive an undefined-resolving handler through a real reserve→release + // pairing: each call reserves under the normalized `null` result, and + // the observed `result: null` wire frame must release that same + // charge. With the `?? null` normalization dropped from runRequest, + // the release lookup misses and the third reservation trips the + // two-message queue limit (NdJsonQueueLimitError → guard.fail). + const failures: unknown[] = []; + const safety = createBoundedAcpTransportSafety( + { + maxFrameBytes: 64 * 1024, + maxQueuedMessages: 2, + maxQueuedBytes: 64 * 1024, + }, + (error) => { + failures.push(error); + }, + ); + const extMethod = vi.fn( + async (): Promise> => undefined as never, + ); + const client = { + requestPermission: vi.fn(async () => ({})), + sessionUpdate: vi.fn(async () => {}), + extMethod, + } as unknown as Client; + const wrapped = createLogSafeAcpClient(client, safety.guard); + + for (let id = 1; id <= 5; id++) { + await wrapped.extMethod!('x/status', { sessionId: 's' }); + safety.observeMessage({ + direction: 'sent', + bytes: 32, + message: { jsonrpc: '2.0', id, result: null }, + }); + } + + expect(extMethod).toHaveBeenCalledTimes(5); + expect(failures).toEqual([]); + }); + + it('fails request admission closed when params exceed the handler budget', async () => { + const guard = { ...createFakeGuard(), maxActiveHandlerBytes: 3000 }; + const requestPermission = vi.fn(async () => ({})); + const client = { + requestPermission, + sessionUpdate: vi.fn(async () => {}), + } as unknown as Client; + const wrapped = createLogSafeAcpClient(client, guard); + + await expect( + wrapped.requestPermission({ + sessionId: 's', + options: [], + note: 'x'.repeat(4096), + } as never), + ).rejects.toThrow(); + + expect(requestPermission).not.toHaveBeenCalled(); + expect(guard.fail).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/acp-bridge/src/transport-safety.ts b/packages/acp-bridge/src/transport-safety.ts new file mode 100644 index 00000000000..c2d84bda129 --- /dev/null +++ b/packages/acp-bridge/src/transport-safety.ts @@ -0,0 +1,668 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { inspect } from 'node:util'; +import { + RequestError, + type Client, + type ClientSideConnection, +} from '@agentclientprotocol/sdk'; +import { + NdJsonQueueLimitError, + type NdJsonMessageObservation, + type NdJsonStreamLimits, +} from './ndJsonStream.js'; +import { estimateJsonStringBytes } from './json-string-bytes.js'; + +export interface AcpChannelTransportGuard { + maxActiveHandlers: number; + maxActiveHandlerBytes: number; + reserveOutboundOperation(value: unknown): () => void; + reservePreparedResponse(value: unknown): void; + fail(error: unknown): void; +} + +export interface BoundedAcpTransportSafety { + readonly guard: AcpChannelTransportGuard; + readonly transportFailed: Promise; + observeMessage(observation: NdJsonMessageObservation): void; +} + +interface RetainedBudget { + close(): void; +} + +class PreparedResponseBudget implements RetainedBudget { + private objectCharges = new WeakMap(); + private readonly primitiveCharges = new Map(); + private retainedCount = 0; + private retainedBytes = 0; + private closed = false; + + constructor(private readonly limits: NdJsonStreamLimits) {} + + reserve(value: unknown): void { + if (this.closed) return; + const charge = this.charge(value); + const charges = this.getCharges(value, true); + charges.push(charge); + this.retainedCount++; + this.retainedBytes += charge; + } + + releaseMessage(message: unknown): void { + if ( + !isPlainRecord(message) || + Object.hasOwn(message, 'method') || + !Object.hasOwn(message, 'id') + ) { + return; + } + const value = Object.hasOwn(message, 'result') + ? message['result'] + : message['error']; + const charges = this.getCharges(value, false); + if (!charges) return; + const charge = charges.shift(); + if (charge === undefined) return; + if (charges.length === 0 && !isObjectValue(value)) { + this.primitiveCharges.delete(value); + } + this.retainedCount--; + this.retainedBytes -= charge; + } + + close(): void { + this.closed = true; + this.objectCharges = new WeakMap(); + this.primitiveCharges.clear(); + this.retainedCount = 0; + this.retainedBytes = 0; + } + + private charge(value: unknown): number { + const availableBytes = Math.max( + 0, + this.limits.maxQueuedBytes - this.retainedBytes, + ); + const charge = estimateRetainedValueCharge( + value, + availableBytes, + this.limits.maxQueuedBytes, + ); + if ( + this.retainedCount >= this.limits.maxQueuedMessages || + charge > availableBytes + ) { + throw new NdJsonQueueLimitError( + this.limits.maxQueuedMessages, + this.limits.maxQueuedBytes, + charge, + availableBytes, + ); + } + return charge; + } + + private getCharges(value: unknown, create: true): number[]; + private getCharges(value: unknown, create: false): number[] | undefined; + private getCharges(value: unknown, create: boolean): number[] | undefined { + const charges = isObjectValue(value) + ? this.objectCharges.get(value) + : this.primitiveCharges.get(value); + if (charges || !create) return charges; + const created: number[] = []; + if (isObjectValue(value)) { + this.objectCharges.set(value, created); + } else { + this.primitiveCharges.set(value, created); + } + return created; + } +} + +class OutboundOperationBudget implements RetainedBudget { + private retainedCount = 0; + private retainedBytes = 0; + private generation = 0; + private closed = false; + + constructor(private readonly limits: NdJsonStreamLimits) {} + + reserve(value: unknown): () => void { + if (this.closed) return () => {}; + const availableBytes = Math.max( + 0, + this.limits.maxQueuedBytes - this.retainedBytes, + ); + const charge = estimateRetainedValueCharge( + value, + availableBytes, + this.limits.maxQueuedBytes, + ); + if ( + this.retainedCount >= this.limits.maxQueuedMessages || + charge > availableBytes + ) { + throw new NdJsonQueueLimitError( + this.limits.maxQueuedMessages, + this.limits.maxQueuedBytes, + charge, + availableBytes, + ); + } + this.retainedCount++; + this.retainedBytes += charge; + const generation = this.generation; + let active = true; + return () => { + if (!active) return; + active = false; + if (this.closed || this.generation !== generation) return; + this.retainedCount--; + this.retainedBytes -= charge; + }; + } + + close(): void { + this.closed = true; + this.generation++; + this.retainedCount = 0; + this.retainedBytes = 0; + } +} + +export function createBoundedAcpTransportSafety( + limits: NdJsonStreamLimits, + onFailure: (error: unknown) => void, +): BoundedAcpTransportSafety { + const preparedResponses = new PreparedResponseBudget(limits); + const outboundOperations = new OutboundOperationBudget(limits); + const budgets: RetainedBudget[] = [preparedResponses, outboundOperations]; + let reportFailure: ((error: unknown) => void) | undefined; + let failed = false; + const transportFailed = new Promise((resolve) => { + reportFailure = resolve; + }); + const fail = (error: unknown) => { + if (failed) return; + failed = true; + for (const budget of budgets) budget.close(); + reportFailure?.(error); + reportFailure = undefined; + onFailure(error); + }; + const guard: AcpChannelTransportGuard = { + maxActiveHandlers: limits.maxQueuedMessages, + maxActiveHandlerBytes: limits.maxQueuedBytes, + reserveOutboundOperation: (value) => { + try { + return outboundOperations.reserve(value); + } catch (error) { + fail(error); + throw error; + } + }, + reservePreparedResponse: (value) => { + try { + preparedResponses.reserve(value); + } catch (error) { + fail(error); + throw error; + } + }, + fail, + }; + return { + guard, + transportFailed, + observeMessage: (observation) => { + if (observation.direction === 'sent') { + preparedResponses.releaseMessage(observation.message); + } + }, + }; +} + +function estimateRetainedValueCharge( + value: unknown, + availableBytes: number, + maxBytes: number, +): number { + const envelopeBytes = Math.min(2_048, maxBytes); + return ( + envelopeBytes + + estimateTransportValueBytes( + value, + Math.max(0, availableBytes - envelopeBytes), + ) + ); +} + +type EstimationFrame = + | { kind: 'value'; value: unknown } + | { kind: 'array'; value: unknown[]; index: number } + | { + kind: 'record'; + value: Record; + keys: Generator; + first: boolean; + }; + +function* enumerableOwnKeys( + value: Record, +): Generator { + for (const key in value) { + if (Object.hasOwn(value, key)) yield key; + } +} + +/** + * `JSON.stringify` substitutes a value with the result of calling its own + * `toJSON` property when that property is callable, so a callable own + * `toJSON` makes structural size estimation unsound and must fail closed. + * An accessor-backed `toJSON` fails closed like every other accessor: the + * estimator never reads properties whose access can have side effects. + * + * A plain-data `toJSON` key — the only shape `JSON.parse` can produce on + * the inbound path — is serialized as an ordinary member, so it is + * estimated like any other key instead of retiring the transport. + */ +function hasStructuralToJSONHazard(value: object): boolean { + const descriptor = Object.getOwnPropertyDescriptor(value, 'toJSON'); + if (!descriptor) return false; + return ( + descriptor.get !== undefined || + descriptor.set !== undefined || + typeof descriptor.value === 'function' + ); +} + +export function estimateTransportValueBytes( + value: unknown, + limitBytes: number, +): number { + let bytes = 0; + const stack: EstimationFrame[] = [{ kind: 'value', value }]; + const seen = new WeakSet(); + while (stack.length > 0) { + const frame = stack.pop()!; + if (frame.kind === 'array') { + if (frame.index >= frame.value.length) continue; + if (frame.index > 0) bytes++; + if (bytes > limitBytes) return limitBytes + 1; + const descriptor = Object.getOwnPropertyDescriptor( + frame.value, + String(frame.index), + ); + if (descriptor?.get || descriptor?.set) return limitBytes + 1; + stack.push({ ...frame, index: frame.index + 1 }); + stack.push({ kind: 'value', value: descriptor?.value }); + continue; + } + if (frame.kind === 'record') { + const next = frame.keys.next(); + if (next.done) continue; + const descriptor = Object.getOwnPropertyDescriptor( + frame.value, + next.value, + ); + if (!descriptor || descriptor.get || descriptor.set) { + return limitBytes + 1; + } + bytes += + (frame.first ? 0 : 1) + + estimateJsonStringBytes(next.value, Math.max(0, limitBytes - bytes)) + + 1; + if (bytes > limitBytes) return limitBytes + 1; + stack.push({ ...frame, first: false }); + stack.push({ kind: 'value', value: descriptor.value }); + continue; + } + const current = frame.value; + if (current === null || current === undefined) { + bytes += 4; + } else if (typeof current === 'string') { + bytes += estimateJsonStringBytes( + current, + Math.max(0, limitBytes - bytes), + ); + } else if (typeof current === 'number') { + bytes += 24; + } else if (typeof current === 'boolean') { + bytes += 5; + } else if (Array.isArray(current)) { + if (seen.has(current) || hasStructuralToJSONHazard(current)) { + return limitBytes + 1; + } + seen.add(current); + bytes += 2; + stack.push({ kind: 'array', value: current, index: 0 }); + } else if (isPlainRecord(current)) { + if (seen.has(current) || hasStructuralToJSONHazard(current)) { + return limitBytes + 1; + } + seen.add(current); + bytes += 2; + stack.push({ + kind: 'record', + value: current, + keys: enumerableOwnKeys(current), + first: true, + }); + } else { + return limitBytes + 1; + } + if (bytes > limitBytes) return limitBytes + 1; + } + return Math.max(1, bytes); +} + +function isObjectValue(value: unknown): value is object { + return typeof value === 'object' && value !== null; +} + +function isRecord(value: unknown): value is Record { + return isObjectValue(value) && !Array.isArray(value); +} + +function isPlainRecord(value: unknown): value is Record { + if (!isObjectValue(value) || Array.isArray(value)) return false; + const prototype = Object.getPrototypeOf(value); + return prototype === Object.prototype || prototype === null; +} + +class AcpInboundHandlerLimitError extends Error { + readonly code = 'acp_handler_limit_exceeded'; + + constructor( + readonly maxActiveHandlers: number, + readonly maxActiveHandlerBytes: number, + readonly requiredBytes: number, + readonly availableBytes: number, + ) { + super('ACP inbound handler capacity exceeded'); + this.name = 'AcpInboundHandlerLimitError'; + } +} + +class AcpInboundHandlerAdmission { + private activeHandlers = 0; + private activeBytes = 0; + + constructor(private readonly guard: AcpChannelTransportGuard) {} + + async run(params: unknown, operation: () => Promise): Promise { + const envelopeBytes = Math.min(2_048, this.guard.maxActiveHandlerBytes); + const requiredBytes = + envelopeBytes + + estimateTransportValueBytes( + params, + Math.max(0, this.guard.maxActiveHandlerBytes - envelopeBytes), + ); + const availableBytes = Math.max( + 0, + this.guard.maxActiveHandlerBytes - this.activeBytes, + ); + if ( + this.activeHandlers >= this.guard.maxActiveHandlers || + requiredBytes > availableBytes + ) { + const error = new AcpInboundHandlerLimitError( + this.guard.maxActiveHandlers, + this.guard.maxActiveHandlerBytes, + requiredBytes, + availableBytes, + ); + this.guard.fail(error); + throw error; + } + this.activeHandlers++; + this.activeBytes += requiredBytes; + try { + return await operation(); + } finally { + this.activeHandlers--; + this.activeBytes -= requiredBytes; + } + } +} + +class LogSafeAcpRequestError extends RequestError { + constructor( + code: number, + message: string, + data: unknown, + private readonly reservePreparedResponse?: (value: unknown) => void, + ) { + super(code, message, data); + } + + override toResult() { + const result = super.toResult(); + if ('error' in result) { + Object.defineProperty(result.error, inspect.custom, { + configurable: true, + value: () => ({ code: result.error.code, payloadOmitted: true }), + }); + try { + this.reservePreparedResponse?.(result.error); + } catch { + // The failure path has already retired the transport. Throwing here + // would escape the ACP SDK's unawaited message dispatcher. + } + } + return result; + } +} + +const MAX_LOG_SAFE_ACP_ERROR_DETAILS_CHARS = 1_024; +const MAX_LOG_SAFE_ACP_ERROR_KIND_CHARS = 128; +const MAX_LOG_SAFE_ACP_ERROR_HINT_CHARS = 512; + +function boundedAcpErrorString(value: string, maxChars: number): string { + return value.length <= maxChars ? value : `${value.slice(0, maxChars)}…`; +} + +function logSafeRequestErrorMessage(code: number): string { + switch (code) { + case -32700: + return 'Parse error'; + case -32600: + return 'Invalid request'; + case -32601: + return 'Method not found'; + case -32602: + return 'Invalid params'; + case -32603: + return 'Internal error'; + case -32000: + return 'Authentication required'; + case -32002: + return 'Resource not found'; + default: + return 'ACP client request failed'; + } +} + +function logSafeRequestErrorData(data: unknown): unknown { + if (!isRecord(data) || typeof data['errorKind'] !== 'string') { + return undefined; + } + const status = data['status']; + const hint = data['hint']; + return { + errorKind: boundedAcpErrorString( + data['errorKind'], + MAX_LOG_SAFE_ACP_ERROR_KIND_CHARS, + ), + ...(typeof status === 'number' && Number.isFinite(status) + ? { status } + : {}), + ...(typeof hint === 'string' + ? { + hint: boundedAcpErrorString(hint, MAX_LOG_SAFE_ACP_ERROR_HINT_CHARS), + } + : {}), + }; +} + +function logSafeAcpErrorDetails( + error: unknown, +): { details: string } | undefined { + const details = + error instanceof Error + ? error.message + : isRecord(error) && typeof error['message'] === 'string' + ? error['message'] + : undefined; + if (!details) return undefined; + return { + details: boundedAcpErrorString( + details, + MAX_LOG_SAFE_ACP_ERROR_DETAILS_CHARS, + ), + }; +} + +async function withLogSafeAcpError( + operation: () => Promise, + reservePreparedResponse?: (value: unknown) => void, +): Promise { + try { + return await operation(); + } catch (error) { + if (error instanceof RequestError) { + const code = Number.isFinite(error.code) ? error.code : -32603; + throw new LogSafeAcpRequestError( + code, + logSafeRequestErrorMessage(code), + logSafeRequestErrorData(error.data), + reservePreparedResponse, + ); + } + throw new LogSafeAcpRequestError( + -32603, + 'Internal error', + logSafeAcpErrorDetails(error), + reservePreparedResponse, + ); + } +} + +export function createLogSafeAcpClient( + client: Client, + transportGuard: AcpChannelTransportGuard, +): Client { + const admission = new AcpInboundHandlerAdmission(transportGuard); + const runNotification = (params: unknown, operation: () => Promise) => + withLogSafeAcpError(() => admission.run(params, operation)); + const runRequest = (params: unknown, operation: () => Promise) => + withLogSafeAcpError( + () => + admission.run(params, async () => { + const result = await operation(); + transportGuard.reservePreparedResponse(result ?? null); + return result; + }), + transportGuard.reservePreparedResponse, + ); + return { + requestPermission: (params) => + runRequest(params, () => client.requestPermission(params)), + sessionUpdate: (params) => + runNotification(params, () => client.sessionUpdate(params)), + writeTextFile: client.writeTextFile + ? (params) => runRequest(params, () => client.writeTextFile!(params)) + : undefined, + readTextFile: client.readTextFile + ? (params) => runRequest(params, () => client.readTextFile!(params)) + : undefined, + createTerminal: client.createTerminal + ? (params) => runRequest(params, () => client.createTerminal!(params)) + : undefined, + terminalOutput: client.terminalOutput + ? (params) => runRequest(params, () => client.terminalOutput!(params)) + : undefined, + releaseTerminal: client.releaseTerminal + ? (params) => + runRequest( + params, + async () => (await client.releaseTerminal!(params)) ?? {}, + ) + : undefined, + waitForTerminalExit: client.waitForTerminalExit + ? (params) => + runRequest(params, () => client.waitForTerminalExit!(params)) + : undefined, + killTerminal: client.killTerminal + ? (params) => + runRequest( + params, + async () => (await client.killTerminal!(params)) ?? {}, + ) + : undefined, + extMethod: client.extMethod + ? (method, params) => + runRequest(params, () => client.extMethod!(method, params)) + : undefined, + extNotification: client.extNotification + ? (method, params) => + runNotification(params, () => client.extNotification!(method, params)) + : undefined, + }; +} + +const OUTBOUND_GUARDED_CONNECTION_METHODS = new Set([ + 'initialize', + 'newSession', + 'loadSession', + 'unstable_forkSession', + 'unstable_listSessions', + 'unstable_resumeSession', + 'setSessionMode', + 'unstable_setSessionModel', + 'setSessionConfigOption', + 'authenticate', + 'prompt', + 'cancel', + 'extMethod', + 'extNotification', +]); + +export function createOutboundGuardedConnection( + connection: ClientSideConnection, + transportGuard: AcpChannelTransportGuard, +): ClientSideConnection { + const wrappers = new Map< + PropertyKey, + (...args: unknown[]) => Promise + >(); + return new Proxy(connection, { + get(target, property) { + const value = Reflect.get(target, property, target); + if ( + typeof value !== 'function' || + !OUTBOUND_GUARDED_CONNECTION_METHODS.has(property) + ) { + return value; + } + let wrapper = wrappers.get(property); + if (!wrapper) { + wrapper = async (...args: unknown[]) => { + const release = transportGuard.reserveOutboundOperation(args); + try { + return await Reflect.apply(value, target, args); + } finally { + release(); + } + }; + wrappers.set(property, wrapper); + } + return wrapper; + }, + }); +} diff --git a/packages/cli/src/serve/acp-http/dispatch-error.test.ts b/packages/cli/src/serve/acp-http/dispatch-error.test.ts index 3289009d7e3..5d9aaa688ac 100644 --- a/packages/cli/src/serve/acp-http/dispatch-error.test.ts +++ b/packages/cli/src/serve/acp-http/dispatch-error.test.ts @@ -12,8 +12,8 @@ import { BridgeChannelQuarantinedError, InvalidSessionMetadataError, RestoreInProgressError, - SessionRestoreTimeoutError, -} from '../acp-session-bridge.js'; +} from '@qwen-code/acp-bridge/bridgeErrors'; +import { SessionRestoreTimeoutError } from '@qwen-code/acp-bridge/status'; import { toRpcError } from './dispatch.js'; import { RPC } from './json-rpc.js'; diff --git a/packages/cli/src/serve/acp-http/dispatch.ts b/packages/cli/src/serve/acp-http/dispatch.ts index ea129f65959..e259961fdbd 100644 --- a/packages/cli/src/serve/acp-http/dispatch.ts +++ b/packages/cli/src/serve/acp-http/dispatch.ts @@ -32,17 +32,19 @@ import { // `sendPermissionVoteError` uses, so `instanceof` matches the class the bridge // actually throws (the core re-export is a distinct identity at runtime). import { + type BridgeChannelQuarantinedError, CancelSentinelCollisionError, InvalidPermissionOptionError, PermissionForbiddenError, PermissionPolicyNotImplementedError, + type RestoreInProgressError, SessionArchivingError, -} from '../acp-session-bridge.js'; -import type { - BridgeChannelQuarantinedError, - RestoreInProgressError, - SessionRestoreTimeoutError, -} from '../acp-session-bridge.js'; + SessionNotFoundError, + SessionShellClientRequiredError, + SessionShellDisabledError, + WorkspaceMismatchError, +} from '@qwen-code/acp-bridge/bridgeErrors'; +import type { SessionRestoreTimeoutError } from '@qwen-code/acp-bridge/status'; import { FsError } from '../fs/errors.js'; import { TooManyActiveDeviceFlowsError, @@ -68,12 +70,6 @@ import { canonicalizeWorkspace, } from '@qwen-code/acp-bridge/workspacePaths'; import type { BridgeEvent } from '@qwen-code/acp-bridge/eventBus'; -import { - SessionNotFoundError, - SessionShellClientRequiredError, - SessionShellDisabledError, - WorkspaceMismatchError, -} from '@qwen-code/acp-bridge/bridgeErrors'; import { SessionArtifactAuthorizationError, SessionArtifactValidationError, diff --git a/packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts b/packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts index 9953b81402d..f87de00ac4b 100644 --- a/packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts +++ b/packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts @@ -28,7 +28,7 @@ import type { WorkspaceFileSystemFactory } from '../fs/index.js'; import type { DaemonWorkspaceService } from '../workspace-service/types.js'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; import { createSessionOrganizationService } from '../session-organization-helpers.js'; -import { SessionNotFoundError } from '../acp-session-bridge.js'; +import { SessionNotFoundError } from '@qwen-code/acp-bridge/bridgeErrors'; import { SessionArchiveCoordinator } from '../server/session-archive.js'; import { createRequestedSessionIdAdmission } from '../session-id-admission.js'; diff --git a/packages/cli/src/serve/acp-session-bridge.ts b/packages/cli/src/serve/acp-session-bridge.ts deleted file mode 100644 index 5c5af44c500..00000000000 --- a/packages/cli/src/serve/acp-session-bridge.ts +++ /dev/null @@ -1,162 +0,0 @@ -/** - * @license - * Copyright 2025 Qwen Team - * SPDX-License-Identifier: Apache-2.0 - */ - -/** - * Stage 1 HTTP→ACP bridge — backward-compat re-export shim. - * - * #4175 PR F1 lifted the bridge core (`BridgeClient`, - * `defaultSpawnChannelFactory`, `createAcpSessionBridge` factory closure, - * plus the supporting types/errors/options/status) to - * `@qwen-code/acp-bridge`. This shim preserves the CLI-local bridge import - * surface so `server.ts`, `run-qwen-serve.ts`, `workspace-agents.ts`, - * `workspace-memory.ts`, `index.ts`, plus the bridge test suite, keep resolving - * through one module. - * - * The implementation now lives at: - * - `@qwen-code/acp-bridge/bridge` — `createAcpSessionBridge` factory - * - `@qwen-code/acp-bridge/bridgeClient` — `BridgeClient` class + - * permission record types - * - `@qwen-code/acp-bridge/spawnChannel` — `defaultSpawnChannelFactory` - * - `@qwen-code/acp-bridge/bridgeOptions` — `BridgeOptions` + - * `DaemonStatusProvider` interfaces - * - `@qwen-code/acp-bridge/bridgeTypes` — bridge session + heartbeat - * types + `AcpSessionBridge` interface - * - `@qwen-code/acp-bridge/bridgeErrors` — typed bridge error classes - * - `@qwen-code/acp-bridge/workspacePaths` — `canonicalizeWorkspace` - * + `MAX_WORKSPACE_PATH_LENGTH` - * - `@qwen-code/acp-bridge/status` — protocol-versioned status types - * + idle envelope helpers - * - `@qwen-code/acp-bridge/channel` — `AcpChannel` + `ChannelFactory` - * - * The bridge is bound to a single canonical workspace - * (`BridgeOptions.boundWorkspace`); multi-workspace deployments use - * multiple daemon processes. See the module docstring on `bridge.ts` - * in the lifted package for the full Stage 1/Stage 2 contract. - */ - -export { - createAcpSessionBridge, - createHttpAcpBridge, -} from '@qwen-code/acp-bridge/bridge'; -export { - DEFAULT_SESSION_RESTORE_TIMEOUT_MS, - MAX_SESSION_RESTORE_TIMEOUT_MS, - resolveSessionRestoreTimeoutMs, -} from '@qwen-code/acp-bridge/sessionRestoreTimeout'; -export { - defaultSpawnChannelFactory, - createSpawnChannelFactory, -} from '@qwen-code/acp-bridge/spawnChannel'; -// `MAX_RESOLVED_PERMISSION_RECORDS`, `PendingPermission`, -// `PermissionResolutionRecord` re-exports were removed alongside the -// source definitions — the mediator now owns pending+resolved state. -export { BridgeClient } from '@qwen-code/acp-bridge/bridgeClient'; -export type { BridgeClientSessionEntry } from '@qwen-code/acp-bridge/bridgeClient'; - -export type { - AcpChannel, - AcpChannelExitInfo, - ChannelFactory, -} from '@qwen-code/acp-bridge'; - -export type { - BridgeFreshSessionAdmission, - BridgeFreshSessionAdmissionContext, - BridgeFreshSessionReservation, - BridgeSessionLifecycle, - BridgeSessionLifecycleEvent, - BridgeOptions, - DaemonStatusProvider, -} from '@qwen-code/acp-bridge/bridgeOptions'; - -export type { BridgeFileSystem } from '@qwen-code/acp-bridge/bridgeFileSystem'; - -export type { - BridgeSpawnRequest, - BridgeSession, - BridgeRestoreSessionRequest, - BridgeSessionState, - BridgeRestoredSession, - BridgeSessionTranscriptPage, - BridgeSessionTranscriptPageRequest, - BridgeGenerationModelSource, - BridgeGenerationStreamEvent, - BridgeWorkspaceGenerationStreamEvent, - BridgePromptContentBlock, - BridgeSessionSummary, - BridgeTurnStatus, - BridgeSessionCatalogVersion, - SessionMetadataUpdate, - BridgeClientRequestContext, - BridgeHeartbeatResult, - BridgeHeartbeatState, - BridgeWorkspaceMemoryRememberContextMode, - BridgeWorkspaceMemoryRememberTargetScope, - BridgeWorkspaceMemoryRememberRequest, - BridgeWorkspaceMemoryRememberResult, - BridgeAutoMemoryTopic, - BridgeWorkspaceMemoryForgetRequest, - BridgeWorkspaceMemoryForgetMatch, - BridgeWorkspaceMemoryForgetResult, - BridgeWorkspaceMemoryDreamResult, - BridgeDaemonStatusLimits, - BridgeDaemonSessionDiagnostic, - BridgeDaemonStatusSnapshot, - BridgeShutdownOptions, - WorkspaceEventPublisher, - WorkspaceEventBridge, - AcpSessionBridge, - HttpAcpBridge, -} from '@qwen-code/acp-bridge/bridgeTypes'; - -export { - BranchWhilePromptActiveError, - CdWhilePromptActiveError, - SessionNotFoundError, - RestoreInProgressError, - SessionArchivedError, - SessionNotArchivedError, - SessionConflictError, - SessionArchivingError, - InvalidSessionScopeError, - SessionLimitExceededError, - PromptQueueFullError, - PromptDeadlineExceededError, - WorkspaceMismatchError, - InvalidClientIdError, - InvalidPermissionOptionError, - InvalidSessionMetadataError, - WorkspaceInitConflictError, - WorkspaceInitPathEscapeError, - WorkspaceInitSymlinkError, - WorkspaceInitRaceError, - McpServerNotFoundError, - McpServerRestartFailedError, - SessionBusyError, - WorkspaceDrainingError, - BridgeChannelQuarantinedError, - InvalidRewindTargetError, - TotalSessionLimitExceededError, - NOT_CURRENTLY_GENERATING_CANCEL_MESSAGE, - // Multi-client permission coordination errors. - CancelSentinelCollisionError, - PermissionForbiddenError, - PermissionPolicyNotImplementedError, - SessionShellClientRequiredError, - SessionShellDisabledError, -} from '@qwen-code/acp-bridge/bridgeErrors'; - -export { SessionRestoreTimeoutError } from '@qwen-code/acp-bridge/status'; - -export { - MAX_WORKSPACE_PATH_LENGTH, - canonicalizeWorkspace, -} from '@qwen-code/acp-bridge/workspacePaths'; - -export { - SessionArtifactAuthorizationError, - SessionArtifactValidationError, -} from '@qwen-code/acp-bridge/sessionArtifacts'; diff --git a/packages/cli/src/serve/daemon-status.test.ts b/packages/cli/src/serve/daemon-status.test.ts index ffcf08e7492..51b893f05d9 100644 --- a/packages/cli/src/serve/daemon-status.test.ts +++ b/packages/cli/src/serve/daemon-status.test.ts @@ -14,7 +14,7 @@ import type { import type { AcpSessionBridge, BridgeDaemonStatusSnapshot, -} from './acp-session-bridge.js'; +} from '@qwen-code/acp-bridge/bridgeTypes'; import { DeviceFlowRegistry } from './auth/device-flow.js'; import { buildDaemonStatusResponse, diff --git a/packages/cli/src/serve/daemon-status.ts b/packages/cli/src/serve/daemon-status.ts index 489ce2fd9f5..640a34a9b66 100644 --- a/packages/cli/src/serve/daemon-status.ts +++ b/packages/cli/src/serve/daemon-status.ts @@ -23,7 +23,7 @@ import type { import type { AcpSessionBridge, BridgeDaemonStatusSnapshot, -} from './acp-session-bridge.js'; +} from '@qwen-code/acp-bridge/bridgeTypes'; import { MAX_CHILD_HEAP_MB, MIN_CHILD_HEAP_MB, diff --git a/packages/cli/src/serve/fast-path.test.ts b/packages/cli/src/serve/fast-path.test.ts index 693432403ad..5ff3f1c1fc7 100644 --- a/packages/cli/src/serve/fast-path.test.ts +++ b/packages/cli/src/serve/fast-path.test.ts @@ -455,9 +455,6 @@ describe('CLI entry import boundary', () => { expect(runServeSource).not.toMatch(/from ['"]\.\/server\.js['"]/); expect(runServeSource).not.toMatch(/from ['"]\.\/web-shell-static\.js['"]/); - expect(runServeSource).not.toMatch( - /from ['"]\.\/acp-session-bridge\.js['"]/, - ); expect(runServeSource).not.toMatch( /from ['"]@qwen-code\/acp-bridge\/bridge['"]/, ); @@ -468,15 +465,12 @@ describe('CLI entry import boundary', () => { expect(runServeSource).toContain("import('@qwen-code/acp-bridge/bridge')"); }); - it('keeps request helpers from value-importing the ACP compatibility shim', () => { + it('keeps request helpers on narrow ACP type and workspace-path imports', () => { const requestHelpersSource = readFileSync( 'src/serve/server/request-helpers.ts', 'utf8', ); - expect(requestHelpersSource).not.toMatch( - /from ['"]\.\.\/acp-session-bridge\.js['"]/, - ); expect(requestHelpersSource).toContain( "import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes';", ); @@ -495,14 +489,6 @@ describe('CLI entry import boundary', () => { ); expect(graph.unresolvedLocalImports).toEqual([]); - const forbiddenLocalFiles = [...graph.localFiles].filter( - (filePath) => filePath === 'src/serve/acp-session-bridge.ts', - ); - expect( - forbiddenLocalFiles, - `Unexpected static source graph files:\n${forbiddenLocalFiles.join('\n')}`, - ).toEqual([]); - const forbiddenExternalImports = [ '@qwen-code/acp-bridge', '@qwen-code/acp-bridge/bridge', diff --git a/packages/cli/src/serve/index.ts b/packages/cli/src/serve/index.ts index e82940db01e..538f0f91f94 100644 --- a/packages/cli/src/serve/index.ts +++ b/packages/cli/src/serve/index.ts @@ -116,13 +116,18 @@ export { export { createAcpSessionBridge, createHttpAcpBridge, - defaultSpawnChannelFactory, - // #4297 fold-in 1 (16:32:44-round S2): export every typed error - // class that `sendBridgeError` matches via `instanceof`. External - // embeds that want to recognize these errors (parallel to how - // they already match `WorkspaceInitConflictError` / - // `SessionNotFoundError`) need them on the public barrel; without - // this they have to deep-import `./acp-session-bridge.js`. +} from '@qwen-code/acp-bridge/bridge'; +export { defaultSpawnChannelFactory } from '@qwen-code/acp-bridge/spawnChannel'; +// The bridge error classes external embeds most commonly need to recognize +// live on this public barrel. They are a subset of the classes +// `sendBridgeError` (server/error-response.ts) matches via `instanceof`; +// the rest of that match set lives across other modules — +// `@qwen-code/acp-bridge/status`, `@qwen-code/acp-bridge/sessionArtifacts`, +// `@qwen-code/qwen-code-core`, and `server/session-archive.ts` — so no +// single module carries the complete set. In-repo callers import these +// classes from `@qwen-code/acp-bridge/bridgeErrors` directly, so this block +// looks unused from inside the repo but is load-bearing public surface. +export { McpServerNotFoundError, McpServerRestartFailedError, SessionNotFoundError, @@ -132,14 +137,15 @@ export { WorkspaceInitPathEscapeError, WorkspaceInitSymlinkError, WorkspaceInitRaceError, - type AcpChannel, - type AcpSessionBridge, - type BridgeOptions, - type BridgeSession, - type BridgeSpawnRequest, - type ChannelFactory, - type HttpAcpBridge, -} from './acp-session-bridge.js'; +} from '@qwen-code/acp-bridge/bridgeErrors'; +export type { AcpChannel, ChannelFactory } from '@qwen-code/acp-bridge/channel'; +export type { + AcpSessionBridge, + BridgeSession, + BridgeSpawnRequest, + HttpAcpBridge, +} from '@qwen-code/acp-bridge/bridgeTypes'; +export type { BridgeOptions } from '@qwen-code/acp-bridge/bridgeOptions'; export { EventBus, EVENT_SCHEMA_VERSION, diff --git a/packages/cli/src/serve/multi-workspace-sessions.test.ts b/packages/cli/src/serve/multi-workspace-sessions.test.ts index c5ce468d4ce..6bd8d17274c 100644 --- a/packages/cli/src/serve/multi-workspace-sessions.test.ts +++ b/packages/cli/src/serve/multi-workspace-sessions.test.ts @@ -23,13 +23,15 @@ import { InvalidRewindTargetError, SessionBusyError, SessionNotFoundError, - type AcpSessionBridge, - type BridgeClientRequestContext, - type BridgeDaemonStatusSnapshot, - type BridgeRestoreSessionRequest, - type BridgeSessionSummary, - type BridgeSpawnRequest, -} from './acp-session-bridge.js'; +} from '@qwen-code/acp-bridge/bridgeErrors'; +import type { + AcpSessionBridge, + BridgeClientRequestContext, + BridgeDaemonStatusSnapshot, + BridgeRestoreSessionRequest, + BridgeSessionSummary, + BridgeSpawnRequest, +} from '@qwen-code/acp-bridge/bridgeTypes'; import { ClientMcpSenderRegistry } from './acp-http/client-mcp-sender-registry.js'; import type { DaemonLogger } from './daemon-logger.js'; import { createServeApp } from './server.js'; diff --git a/packages/cli/src/serve/routes/capabilities.ts b/packages/cli/src/serve/routes/capabilities.ts index 90116bf7153..7e3be991dfd 100644 --- a/packages/cli/src/serve/routes/capabilities.ts +++ b/packages/cli/src/serve/routes/capabilities.ts @@ -5,7 +5,7 @@ */ import type { Application } from 'express'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { getServeProtocolVersions } from '../capabilities.js'; import type { getAdvertisedServeFeatures } from '../capabilities.js'; import { MAX_UPLOAD_BYTES } from '../fs/index.js'; diff --git a/packages/cli/src/serve/routes/channel-notify.ts b/packages/cli/src/serve/routes/channel-notify.ts index 0013ca03604..4e863b9eca9 100644 --- a/packages/cli/src/serve/routes/channel-notify.ts +++ b/packages/cli/src/serve/routes/channel-notify.ts @@ -19,7 +19,7 @@ import { import type { WorkspaceRegistry } from '../workspace-registry.js'; import { requireTrustedWorkspaceRuntime, - resolveWorkspaceRuntimeFromParam, + resolveTrustedRuntime, } from '../workspace-route-runtime.js'; interface RegisterChannelNotifyRoutesDeps { @@ -110,12 +110,12 @@ export function registerChannelNotifyRoutes( '/workspaces/:workspace/notify', deps.mutate({ strict: true }), async (req, res) => { - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, ); - if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) return; + if (!runtime) return; if (runtime.provenance === 'live-conversation') { res.status(400).json({ error: diff --git a/packages/cli/src/serve/routes/daemon-status.ts b/packages/cli/src/serve/routes/daemon-status.ts index 252e89a498a..95a9d8f6e1a 100644 --- a/packages/cli/src/serve/routes/daemon-status.ts +++ b/packages/cli/src/serve/routes/daemon-status.ts @@ -7,7 +7,7 @@ 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 { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import type { DeviceFlowProviderId, DeviceFlowRegistry, diff --git a/packages/cli/src/serve/routes/permission.ts b/packages/cli/src/serve/routes/permission.ts index fb6d12afd1a..5986cf4bc6e 100644 --- a/packages/cli/src/serve/routes/permission.ts +++ b/packages/cli/src/serve/routes/permission.ts @@ -5,7 +5,7 @@ */ import type { Application, RequestHandler, Response } from 'express'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import type { ConversationRuntimeActivityGate } from '../conversations/conversation-runtime-activity.js'; import type { DaemonLogger } from '../daemon-logger.js'; import { diff --git a/packages/cli/src/serve/routes/scheduled-tasks.ts b/packages/cli/src/serve/routes/scheduled-tasks.ts index aff4e535c4a..b3f9a3315f5 100644 --- a/packages/cli/src/serve/routes/scheduled-tasks.ts +++ b/packages/cli/src/serve/routes/scheduled-tasks.ts @@ -1685,8 +1685,8 @@ export function registerWorkspaceQualifiedScheduledTasksRoutes( req, res, ); - if (!runtime) return null; - if (!requireTrustedWorkspaceRuntime(runtime, res)) return null; + if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) + return null; if ( runtime.provenance === 'live-conversation' && !deps.conversationRuntimeActivity diff --git a/packages/cli/src/serve/routes/session-pr-backfill.test.ts b/packages/cli/src/serve/routes/session-pr-backfill.test.ts index 47c56ca8c89..55be7c9777f 100644 --- a/packages/cli/src/serve/routes/session-pr-backfill.test.ts +++ b/packages/cli/src/serve/routes/session-pr-backfill.test.ts @@ -19,7 +19,7 @@ import { upsertSessionPr, type SessionService, } from '@qwen-code/qwen-code-core'; -import { SessionArchivingError } from '../acp-session-bridge.js'; +import { SessionArchivingError } from '@qwen-code/acp-bridge/bridgeErrors'; import { AONE_BACKFILL_PAGES_PER_STATE, AONE_MAX_MR_VIEW_CALLS_PER_RUN, @@ -41,8 +41,10 @@ import { type WorkspaceRegistry, type WorkspaceRuntime, } from '../workspace-registry.js'; -import type { SessionPrInfo } from '@qwen-code/acp-bridge/bridgeTypes'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { + AcpSessionBridge, + SessionPrInfo, +} from '@qwen-code/acp-bridge/bridgeTypes'; import { backfillWorkspaceSessionPrs, normalizeRemoteToWebUrl, diff --git a/packages/cli/src/serve/routes/session-prompt-terminals.test.ts b/packages/cli/src/serve/routes/session-prompt-terminals.test.ts index f2e30225253..1826aa04935 100644 --- a/packages/cli/src/serve/routes/session-prompt-terminals.test.ts +++ b/packages/cli/src/serve/routes/session-prompt-terminals.test.ts @@ -16,10 +16,8 @@ import { appendPromptLedgerRecord, readPromptLedgerRecords, } from '@qwen-code/acp-bridge/promptLedger'; -import { - SessionNotFoundError, - type AcpSessionBridge, -} from '../acp-session-bridge.js'; +import { SessionNotFoundError } from '@qwen-code/acp-bridge/bridgeErrors'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { createWorkspaceRegistry, type WorkspaceRuntime, diff --git a/packages/cli/src/serve/routes/session-telemetry.test.ts b/packages/cli/src/serve/routes/session-telemetry.test.ts index 37c69f46700..6fbc9a17fd7 100644 --- a/packages/cli/src/serve/routes/session-telemetry.test.ts +++ b/packages/cli/src/serve/routes/session-telemetry.test.ts @@ -8,10 +8,8 @@ import path from 'node:path'; import express, { type Response } from 'express'; import request from 'supertest'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { - SessionNotFoundError, - type AcpSessionBridge, -} from '../acp-session-bridge.js'; +import { SessionNotFoundError } from '@qwen-code/acp-bridge/bridgeErrors'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { createWorkspaceRegistry, type WorkspaceRuntime, diff --git a/packages/cli/src/serve/routes/session.ts b/packages/cli/src/serve/routes/session.ts index 3ff9d3e2e15..5dd84f467e4 100644 --- a/packages/cli/src/serve/routes/session.ts +++ b/packages/cli/src/serve/routes/session.ts @@ -42,7 +42,10 @@ import type { SessionArtifactInput } from '@qwen-code/acp-bridge/sessionArtifact import { CHANNEL_PROMPT_META_KEY, DAEMON_PROMPT_DISPLAY_TEXT_META_KEY, + type AcpSessionBridge, type BridgeBranchedSession, + type BridgePromptContentBlock, + type BridgeSessionCatalogVersion, } from '@qwen-code/acp-bridge/bridgeTypes'; import { parseSessionSource } from '@qwen-code/acp-bridge'; import { @@ -72,21 +75,18 @@ import { } from '../../config/session-id.js'; import { isChannelDeliveryError } from '../../runtime/channel-delivery-ipc.js'; import { parseChannelDelivery } from '../../runtime/channel-delivery.js'; +import { canonicalizeWorkspace } from '@qwen-code/acp-bridge/workspacePaths'; import { - canonicalizeWorkspace, InvalidClientIdError, InvalidSessionMetadataError, PromptQueueFullError, - SessionArtifactValidationError, SessionArchivedError, SessionConflictError, SessionNotFoundError, SessionShellClientRequiredError, SessionShellDisabledError, - type AcpSessionBridge, - type BridgePromptContentBlock, - type BridgeSessionCatalogVersion, -} from '../acp-session-bridge.js'; +} from '@qwen-code/acp-bridge/bridgeErrors'; +import { SessionArtifactValidationError } from '@qwen-code/acp-bridge/sessionArtifacts'; import type { DaemonLogger } from '../daemon-logger.js'; import type { SendBridgeError } from '../server/error-response.js'; import { resolvePromptDeadlineMs } from '../server/prompt-deadline.js'; diff --git a/packages/cli/src/serve/routes/sse-events.ts b/packages/cli/src/serve/routes/sse-events.ts index 888484ef921..03d4fa8001a 100644 --- a/packages/cli/src/serve/routes/sse-events.ts +++ b/packages/cli/src/serve/routes/sse-events.ts @@ -15,7 +15,7 @@ import { import { mapDomainErrorToErrorKind } from '@qwen-code/acp-bridge'; import type { Application } from 'express'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import type { ConversationRuntimeActivityGate } from '../conversations/conversation-runtime-activity.js'; import type { DaemonLogger } from '../daemon-logger.js'; import { diff --git a/packages/cli/src/serve/routes/workspace-extensions-controller.test.ts b/packages/cli/src/serve/routes/workspace-extensions-controller.test.ts index b8cc2437ef9..7e264c96754 100644 --- a/packages/cli/src/serve/routes/workspace-extensions-controller.test.ts +++ b/packages/cli/src/serve/routes/workspace-extensions-controller.test.ts @@ -10,7 +10,7 @@ import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { ExtensionManager } from '@qwen-code/qwen-code-core'; import type { Response } from 'express'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import type { DaemonWorkspaceService } from '../workspace-service/types.js'; import { resolveLanguageSetting } from '../../i18n/index.js'; import { diff --git a/packages/cli/src/serve/routes/workspace-extensions-controller.ts b/packages/cli/src/serve/routes/workspace-extensions-controller.ts index 6fd48a145c2..aa97ab829ab 100644 --- a/packages/cli/src/serve/routes/workspace-extensions-controller.ts +++ b/packages/cli/src/serve/routes/workspace-extensions-controller.ts @@ -21,7 +21,7 @@ import { } from '../../i18n/index.js'; import { resolveSupportedLanguage } from '../../i18n/languages.js'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { parseAndValidateWorkspaceClientId } from '../server/request-helpers.js'; import { STATUS_SCHEMA_VERSION, diff --git a/packages/cli/src/serve/routes/workspace-extensions.ts b/packages/cli/src/serve/routes/workspace-extensions.ts index 5854f37016c..3695b43ff37 100644 --- a/packages/cli/src/serve/routes/workspace-extensions.ts +++ b/packages/cli/src/serve/routes/workspace-extensions.ts @@ -30,7 +30,7 @@ import express, { type Response, } from 'express'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { createFifoTaskQueue } from '../extension-operation-scheduler.js'; import { isBlockedAuthProviderHost } from '../server/auth-provider-helpers.js'; import type { SendBridgeError } from '../server/error-response.js'; @@ -38,6 +38,7 @@ import type { safeBody as safeBodyType } from '../server/request-helpers.js'; import { isPortableAbsolutePath, requireTrustedWorkspaceRuntime, + resolveTrustedRuntime, resolveWorkspaceRuntimeFromParam, sendGenerationClosedError, sendUntrustedWorkspaceResponse, @@ -2324,8 +2325,12 @@ export function registerWorkspaceExtensionRoutes( '/workspaces/:workspace/extensions/:extensionId/activation', mutate({ strict: true }), (req, res) => { - const runtime = resolveWorkspaceRuntimeFromParam(registry, req, res); - if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) return; + const runtime = resolveTrustedRuntime( + registry, + req, + res, + ); + if (!runtime) return; const extensionId = parseExtensionId(req, res); if (!extensionId) return; const state = parseActivationState(req, res); @@ -2372,8 +2377,12 @@ export function registerWorkspaceExtensionRoutes( '/workspaces/:workspace/extensions/:extensionId/activation', mutate({ strict: true }), (req, res) => { - const runtime = resolveWorkspaceRuntimeFromParam(registry, req, res); - if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) return; + const runtime = resolveTrustedRuntime( + registry, + req, + res, + ); + if (!runtime) return; const extensionId = parseExtensionId(req, res); if (!extensionId) return; const manager = primaryController.createExtensionManager( @@ -2420,8 +2429,12 @@ export function registerWorkspaceExtensionRoutes( '/workspaces/:workspace/extensions/refresh', mutate({ strict: true }), (req, res) => { - const runtime = resolveWorkspaceRuntimeFromParam(registry, req, res); - if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) return; + const runtime = resolveTrustedRuntime( + registry, + req, + res, + ); + if (!runtime) return; const manager = primaryController.createExtensionManager( runtime.workspaceCwd, true, diff --git a/packages/cli/src/serve/routes/workspace-file-write.ts b/packages/cli/src/serve/routes/workspace-file-write.ts index bafae65d0ed..b9e5105ecdc 100644 --- a/packages/cli/src/serve/routes/workspace-file-write.ts +++ b/packages/cli/src/serve/routes/workspace-file-write.ts @@ -7,7 +7,7 @@ import * as path from 'node:path'; import type { Application, Request, RequestHandler, Response } from 'express'; import express from 'express'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { MAX_UPLOAD_BYTES, hasSuspiciousPathPattern, @@ -26,8 +26,7 @@ import { } from './workspace-file-read.js'; import { getWorkspaceRouteContext, - requireTrustedWorkspaceRuntime, - resolveWorkspaceRuntimeFromParam, + resolveTrustedRuntime, setWorkspaceRouteContext, } from '../workspace-route-runtime.js'; import type { WorkspaceRegistry } from '../workspace-registry.js'; @@ -334,13 +333,12 @@ export function registerWorkspaceQualifiedFileWriteRoutes( deps: RegisterDeps & { workspaceRegistry: WorkspaceRegistry }, ): void { const resolve = (req: Request, res: Response): boolean => { - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, ); if (!runtime) return false; - if (!requireTrustedWorkspaceRuntime(runtime, res)) return false; setWorkspaceRouteContext(req, { runtime, routePrefix: 'POST /workspaces/:workspace', @@ -565,9 +563,12 @@ function fileUploadAdmission( if (!registry) { throw new Error('workspace registry is not configured'); } - const runtime = resolveWorkspaceRuntimeFromParam(registry, req, res); + const runtime = resolveTrustedRuntime( + registry, + req, + res, + ); if (!runtime) return; - if (!requireTrustedWorkspaceRuntime(runtime, res)) return; setWorkspaceRouteContext(req, { runtime, routePrefix: 'POST /workspaces/:workspace', diff --git a/packages/cli/src/serve/routes/workspace-git-branches.test.ts b/packages/cli/src/serve/routes/workspace-git-branches.test.ts index e0641f80b0b..82f7a4d0c8b 100644 --- a/packages/cli/src/serve/routes/workspace-git-branches.test.ts +++ b/packages/cli/src/serve/routes/workspace-git-branches.test.ts @@ -11,7 +11,7 @@ import * as path from 'node:path'; import express from 'express'; import request from 'supertest'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { sendBridgeError } from '../server/error-response.js'; import { createWorkspaceGenerationGuard, diff --git a/packages/cli/src/serve/routes/workspace-git-diff.test.ts b/packages/cli/src/serve/routes/workspace-git-diff.test.ts index d8a04201202..eefc7fe2fd0 100644 --- a/packages/cli/src/serve/routes/workspace-git-diff.test.ts +++ b/packages/cli/src/serve/routes/workspace-git-diff.test.ts @@ -11,7 +11,7 @@ import { fetchGitDiff, fetchGitDiffHunksForFile, } from '@qwen-code/qwen-code-core'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { sendBridgeError } from '../server/error-response.js'; import { createWorkspaceGenerationGuard, diff --git a/packages/cli/src/serve/routes/workspace-git-log.test.ts b/packages/cli/src/serve/routes/workspace-git-log.test.ts index 2e99ffef8a6..f8ec767624d 100644 --- a/packages/cli/src/serve/routes/workspace-git-log.test.ts +++ b/packages/cli/src/serve/routes/workspace-git-log.test.ts @@ -8,7 +8,7 @@ import express from 'express'; import request from 'supertest'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { fetchGitLog, fetchGitCommitDetail } from '@qwen-code/qwen-code-core'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { sendBridgeError } from '../server/error-response.js'; import { createWorkspaceRegistry, diff --git a/packages/cli/src/serve/routes/workspace-git.test.ts b/packages/cli/src/serve/routes/workspace-git.test.ts index 58592025178..53a4c36fa67 100644 --- a/packages/cli/src/serve/routes/workspace-git.test.ts +++ b/packages/cli/src/serve/routes/workspace-git.test.ts @@ -7,7 +7,7 @@ import express from 'express'; import request from 'supertest'; import { describe, expect, it, vi } from 'vitest'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { sendBridgeError } from '../server/error-response.js'; import type { WorkspaceGitState } from '../workspace-git-state.js'; import { diff --git a/packages/cli/src/serve/routes/workspace-git.ts b/packages/cli/src/serve/routes/workspace-git.ts index d4f4b18bfbc..7ce5b6c22ed 100644 --- a/packages/cli/src/serve/routes/workspace-git.ts +++ b/packages/cli/src/serve/routes/workspace-git.ts @@ -6,7 +6,7 @@ import type { Application } from 'express'; import { getGitWorkingTreeStatus } from '@qwen-code/qwen-code-core'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import type { SendBridgeError } from '../server/error-response.js'; import type { WorkspaceGitState } from '../workspace-git-state.js'; import type { WorkspaceRegistry } from '../workspace-registry.js'; diff --git a/packages/cli/src/serve/routes/workspace-github-prs.test.ts b/packages/cli/src/serve/routes/workspace-github-prs.test.ts index 90ecc273f05..1ebcf9d9ca1 100644 --- a/packages/cli/src/serve/routes/workspace-github-prs.test.ts +++ b/packages/cli/src/serve/routes/workspace-github-prs.test.ts @@ -12,7 +12,7 @@ import { fetchGitHubPullRequests, getDefaultBranch, } from '@qwen-code/qwen-code-core'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { sendBridgeError } from '../server/error-response.js'; import { createWorkspaceGenerationGuard, diff --git a/packages/cli/src/serve/routes/workspace-github-prs.ts b/packages/cli/src/serve/routes/workspace-github-prs.ts index f74c593d46d..f06b9c77a88 100644 --- a/packages/cli/src/serve/routes/workspace-github-prs.ts +++ b/packages/cli/src/serve/routes/workspace-github-prs.ts @@ -16,9 +16,8 @@ import type { SendBridgeError } from '../server/error-response.js'; import { safeBody } from '../server/request-helpers.js'; import type { WorkspaceRegistry } from '../workspace-registry.js'; import { - requireTrustedWorkspaceRuntime, resolveContainedCwdOrFail, - resolveWorkspaceRuntimeFromParam, + resolveTrustedRuntime, sendGenerationClosedError, } from '../workspace-route-runtime.js'; import { applyReadHeaders } from './workspace-file-read.js'; @@ -86,13 +85,12 @@ export function registerWorkspaceQualifiedGitHubPrsRoutes( app.get('/workspaces/:workspace/github/prs', async (req, res) => { const route = 'GET /workspaces/:workspace/github/prs'; - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, ); if (!runtime) return; - if (!requireTrustedWorkspaceRuntime(runtime, res)) return; applyReadHeaders(res); try { @@ -154,13 +152,12 @@ export function registerWorkspaceQualifiedGitHubPrsRoutes( deps.mutate({ strict: true }), async (req, res) => { const route = 'POST /workspaces/:workspace/github/prs/create'; - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, ); if (!runtime) return; - if (!requireTrustedWorkspaceRuntime(runtime, res)) return; try { runtime.generationGuard?.assertOpen(); } catch (err) { @@ -242,13 +239,12 @@ export function registerWorkspaceQualifiedGitHubPrsRoutes( app.get('/workspaces/:workspace/github/default-branch', async (req, res) => { const route = 'GET /workspaces/:workspace/github/default-branch'; - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, ); if (!runtime) return; - if (!requireTrustedWorkspaceRuntime(runtime, res)) return; applyReadHeaders(res); try { diff --git a/packages/cli/src/serve/routes/workspace-lifecycle.ts b/packages/cli/src/serve/routes/workspace-lifecycle.ts index 107a9bbdd6e..16a97fa9ee3 100644 --- a/packages/cli/src/serve/routes/workspace-lifecycle.ts +++ b/packages/cli/src/serve/routes/workspace-lifecycle.ts @@ -12,8 +12,7 @@ import { } from '../server/request-helpers.js'; import type { DaemonWorkspaceService } from '../workspace-service/index.js'; import { - requireTrustedWorkspaceRuntime, - resolveWorkspaceRuntimeFromParam, + resolveTrustedRuntime, sendUntrustedWorkspaceResponse, } from '../workspace-route-runtime.js'; import type { WorkspaceRegistry } from '../workspace-registry.js'; @@ -115,12 +114,12 @@ export function registerWorkspaceQualifiedLifecycleRoutes( '/workspaces/:workspace/init', deps.mutate({ strict: true }), async (req, res) => { - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, ); - if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) return; + if (!runtime) return; const body = deps.safeBody(req); const force = body['force']; if (force !== undefined && typeof force !== 'boolean') { @@ -161,12 +160,12 @@ export function registerWorkspaceQualifiedLifecycleRoutes( .catch(() => { // The policy monitor reports reconciliation failures separately. }); - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, ); - if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) return; + if (!runtime) return; const clientId = parseAndValidateWorkspaceClientId( req, res, diff --git a/packages/cli/src/serve/routes/workspace-mcp-control.ts b/packages/cli/src/serve/routes/workspace-mcp-control.ts index 95b766ab7d0..e1275ac70ec 100644 --- a/packages/cli/src/serve/routes/workspace-mcp-control.ts +++ b/packages/cli/src/serve/routes/workspace-mcp-control.ts @@ -5,7 +5,7 @@ */ import type { Application, Request, RequestHandler, Response } from 'express'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import type { SendBridgeError } from '../server/error-response.js'; import { createBuildWorkspaceCtx, @@ -14,14 +14,8 @@ import { validateMcpRuntimeServerName, } from '../server/request-helpers.js'; import type { DaemonWorkspaceService } from '../workspace-service/index.js'; -import { - requireTrustedWorkspaceRuntime, - resolveWorkspaceRuntimeFromParam, -} from '../workspace-route-runtime.js'; -import type { - WorkspaceRegistry, - WorkspaceRuntime, -} from '../workspace-registry.js'; +import { resolveTrustedRuntime } from '../workspace-route-runtime.js'; +import type { WorkspaceRegistry } from '../workspace-registry.js'; interface RegisterWorkspaceMcpControlRoutesDeps { boundWorkspace: string; @@ -357,16 +351,6 @@ export function registerWorkspaceMcpControlRoutes( ); } -function resolveTrustedMcpRuntime( - registry: WorkspaceRegistry, - req: Request, - res: Response, -): WorkspaceRuntime | null { - const runtime = resolveWorkspaceRuntimeFromParam(registry, req, res); - if (!runtime) return null; - return requireTrustedWorkspaceRuntime(runtime, res) ? runtime : null; -} - export function registerWorkspaceQualifiedMcpControlRoutes( app: Application, deps: Pick< @@ -380,7 +364,7 @@ export function registerWorkspaceQualifiedMcpControlRoutes( '/workspaces/:workspace/mcp/initialize', deps.mutate({ strict: true }), async (req, res) => { - const runtime = resolveTrustedMcpRuntime( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, @@ -402,7 +386,7 @@ export function registerWorkspaceQualifiedMcpControlRoutes( '/workspaces/:workspace/mcp/reload', deps.mutate({ strict: true }), async (req, res) => { - const runtime = resolveTrustedMcpRuntime( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, @@ -426,7 +410,7 @@ export function registerWorkspaceQualifiedMcpControlRoutes( '/workspaces/:workspace/mcp/:server/restart', deps.mutate({ strict: true }), async (req, res) => { - const runtime = resolveTrustedMcpRuntime( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, @@ -505,7 +489,7 @@ export function registerWorkspaceQualifiedMcpControlRoutes( `/workspaces/:workspace/mcp/:server/${routeAction}`, deps.mutate({ strict: true }), async (req, res) => { - const runtime = resolveTrustedMcpRuntime( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, @@ -553,7 +537,7 @@ export function registerWorkspaceQualifiedMcpControlRoutes( '/workspaces/:workspace/mcp/servers', deps.mutate({ strict: true }), async (req, res) => { - const runtime = resolveTrustedMcpRuntime( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, @@ -609,7 +593,7 @@ export function registerWorkspaceQualifiedMcpControlRoutes( '/workspaces/:workspace/mcp/servers/:name', deps.mutate({ strict: true }), async (req, res) => { - const runtime = resolveTrustedMcpRuntime( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, diff --git a/packages/cli/src/serve/routes/workspace-permissions.ts b/packages/cli/src/serve/routes/workspace-permissions.ts index 67d703f01b7..3a8b7fd21c4 100644 --- a/packages/cli/src/serve/routes/workspace-permissions.ts +++ b/packages/cli/src/serve/routes/workspace-permissions.ts @@ -20,8 +20,7 @@ import type { DaemonWorkspaceService } from '../workspace-service/types.js'; import { WorkspacePermissionRulesSessionRequiredError } from '../workspace-service/types.js'; import { parseAndValidateWorkspaceClientId } from '../server/request-helpers.js'; import { - requireTrustedWorkspaceRuntime, - resolveWorkspaceRuntimeFromParam, + resolveTrustedRuntime, sendGenerationClosedError, } from '../workspace-route-runtime.js'; import type { WorkspaceRegistry } from '../workspace-registry.js'; @@ -197,12 +196,12 @@ export function registerWorkspaceQualifiedPermissionsRoutes( }, ): void { app.get('/workspaces/:workspace/permissions', (req, res) => { - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, ); - if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) return; + if (!runtime) return; try { res .status(200) @@ -228,12 +227,12 @@ export function registerWorkspaceQualifiedPermissionsRoutes( '/workspaces/:workspace/permissions', deps.mutate({ strict: true }), async (req, res) => { - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, ); - if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) return; + if (!runtime) return; const body = deps.safeBody(req); const scope = body['scope']; const ruleType = body['ruleType']; diff --git a/packages/cli/src/serve/routes/workspace-qualified-extensions.test.ts b/packages/cli/src/serve/routes/workspace-qualified-extensions.test.ts index 1021a9a164c..9a929a5cb56 100644 --- a/packages/cli/src/serve/routes/workspace-qualified-extensions.test.ts +++ b/packages/cli/src/serve/routes/workspace-qualified-extensions.test.ts @@ -26,7 +26,7 @@ import { createWorkspaceRegistry, type WorkspaceRuntime, } from '../workspace-registry.js'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { ConversationWorkspace } from '../conversations/conversation-workspace.js'; import type { DaemonWorkspaceService } from '../workspace-service/types.js'; @@ -1931,7 +1931,6 @@ describe('extension management v2 REST', () => { }); it('times out a legacy update check while its cache refresh stalls', async () => { - vi.useFakeTimers(); const h = await makeHarness(); mockExtensionManager(); vi.spyOn(process.stderr, 'write').mockReturnValue(true); @@ -1940,6 +1939,10 @@ describe('extension management v2 REST', () => { .spyOn(ExtensionManager.prototype, 'refreshCache') .mockImplementationOnce(async () => await stalledRefresh) .mockResolvedValue(undefined); + // Install fake timers only after the harness is up: app setup performs + // real async work that must not run under a frozen clock (the deadline + // timer this test targets is created later, inside the route handler). + vi.useFakeTimers(); try { const response = auth( request(h.app).post('/workspace/extensions/check-updates'), @@ -1960,7 +1963,10 @@ describe('extension management v2 REST', () => { vi.useRealTimers(); await fsp.rm(h.scratch, { recursive: true, force: true }); } - }); + // Generous budget: this test mixes fake timers with real socket/fs I/O, + // which the shared CI runners occasionally stretch past the 15s default + // (the assertion logic itself needs only the 2min fake-clock advance). + }, 30_000); it('reports legacy global mutations as applied after runtime reconciliation', async () => { const h = await makeHarness(); diff --git a/packages/cli/src/serve/routes/workspace-settings.ts b/packages/cli/src/serve/routes/workspace-settings.ts index 096b5e6979f..0d672db1449 100644 --- a/packages/cli/src/serve/routes/workspace-settings.ts +++ b/packages/cli/src/serve/routes/workspace-settings.ts @@ -25,8 +25,7 @@ import { import { writeStderrLine } from '../../utils/stdioHelpers.js'; import { parseAndValidateWorkspaceClientId } from '../server/request-helpers.js'; import { - requireTrustedWorkspaceRuntime, - resolveWorkspaceRuntimeFromParam, + resolveTrustedRuntime, sendGenerationClosedError, } from '../workspace-route-runtime.js'; import type { WorkspaceRegistry } from '../workspace-registry.js'; @@ -559,7 +558,7 @@ export function registerWorkspaceQualifiedSettingsRoutes( const allowedKeys = getAllowedKeys(false); app.get('/workspaces/:workspace/settings', (req: Request, res: Response) => { - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, @@ -567,7 +566,7 @@ export function registerWorkspaceQualifiedSettingsRoutes( // Legacy /workspace/settings remains primary-only and pre-trust for // compatibility; plural workspace-qualified settings intentionally follow // the Phase 3 core-route trust gate. - if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) return; + if (!runtime) return; try { const response = buildSettingsResponse(runtime.workspaceCwd, allowedKeys); res.status(200).json(response); @@ -588,12 +587,12 @@ export function registerWorkspaceQualifiedSettingsRoutes( '/workspaces/:workspace/settings', deps.mutate({ strict: true }), async (req: Request, res: Response) => { - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, ); - if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) return; + if (!runtime) return; const body = deps.safeBody(req); const scope = body['scope']; const key = body['key']; diff --git a/packages/cli/src/serve/routes/workspace-setup-github.test.ts b/packages/cli/src/serve/routes/workspace-setup-github.test.ts index 3b50d407c1b..4b9e5bb84c1 100644 --- a/packages/cli/src/serve/routes/workspace-setup-github.test.ts +++ b/packages/cli/src/serve/routes/workspace-setup-github.test.ts @@ -15,7 +15,7 @@ import { canonicalizeWorkspace, createWorkspaceFileSystemFactory, } from '../fs/index.js'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import type { BridgeEvent } from '@qwen-code/acp-bridge/eventBus'; import type { ServeOptions } from '../types.js'; import { diff --git a/packages/cli/src/serve/routes/workspace-setup-github.ts b/packages/cli/src/serve/routes/workspace-setup-github.ts index 7d1fc72657d..8933028b7fc 100644 --- a/packages/cli/src/serve/routes/workspace-setup-github.ts +++ b/packages/cli/src/serve/routes/workspace-setup-github.ts @@ -7,7 +7,7 @@ import { promises as fsp } from 'node:fs'; import * as path from 'node:path'; import type { Application, Request, RequestHandler, Response } from 'express'; -import type { WorkspaceEventBridge } from '../acp-session-bridge.js'; +import type { WorkspaceEventBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import type { BridgeEvent } from '@qwen-code/acp-bridge/eventBus'; import { isFsError, type WorkspaceFileSystemFactory } from '../fs/index.js'; import { diff --git a/packages/cli/src/serve/routes/workspace-skills.ts b/packages/cli/src/serve/routes/workspace-skills.ts index c42fbdd795f..e381653d032 100644 --- a/packages/cli/src/serve/routes/workspace-skills.ts +++ b/packages/cli/src/serve/routes/workspace-skills.ts @@ -12,7 +12,7 @@ import { } from '../server/request-helpers.js'; import { requireTrustedWorkspaceRuntime, - resolveWorkspaceRuntimeFromParam, + resolveTrustedRuntime, } from '../workspace-route-runtime.js'; import type { WorkspaceRegistry, @@ -326,12 +326,12 @@ export function registerWorkspaceQualifiedSkillsRoutes( '/workspaces/:workspace/skills/install', deps.mutate({ strict: true }), async (req, res) => { - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, ); - if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) return; + if (!runtime) return; const input = parseSkillInstallRequest(req, res, deps.safeBody); if (!input) return; const clientId = parseAndValidateWorkspaceClientId( @@ -357,12 +357,12 @@ export function registerWorkspaceQualifiedSkillsRoutes( '/workspaces/:workspace/skills/:name', deps.mutate({ strict: true }), async (req, res) => { - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, ); - if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) return; + if (!runtime) return; const rawSkillName = req.params['name']; const scope = parseDeleteScope(req, res); if (!rawSkillName || !scope) return; @@ -397,12 +397,12 @@ export function registerWorkspaceQualifiedSkillsRoutes( '/workspaces/:workspace/skills/enable', deps.mutate({ strict: true }), async (req, res) => { - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, ); - if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) return; + if (!runtime) return; const input = parseSkillBatchToggleRequest(req, res, deps.safeBody); if (!input) return; const clientId = parseAndValidateWorkspaceClientId( @@ -427,12 +427,12 @@ export function registerWorkspaceQualifiedSkillsRoutes( '/workspaces/:workspace/skills/:name/enable', deps.mutate({ strict: true }), async (req, res) => { - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, ); - if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) return; + if (!runtime) return; const input = parseSkillToggleRequest(req, res, deps.safeBody); if (!input) return; const clientId = parseAndValidateWorkspaceClientId( diff --git a/packages/cli/src/serve/routes/workspace-status.ts b/packages/cli/src/serve/routes/workspace-status.ts index 9cb18ff16a7..2edc9c0273d 100644 --- a/packages/cli/src/serve/routes/workspace-status.ts +++ b/packages/cli/src/serve/routes/workspace-status.ts @@ -5,7 +5,7 @@ */ import type { Application, RequestHandler } from 'express'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import type { SendBridgeError } from '../server/error-response.js'; import { createBuildWorkspaceCtx, diff --git a/packages/cli/src/serve/routes/workspace-tools.ts b/packages/cli/src/serve/routes/workspace-tools.ts index 340b773cdba..c2c457a0b91 100644 --- a/packages/cli/src/serve/routes/workspace-tools.ts +++ b/packages/cli/src/serve/routes/workspace-tools.ts @@ -13,8 +13,7 @@ import { } from '../server/request-helpers.js'; import type { DaemonWorkspaceService } from '../workspace-service/index.js'; import { - requireTrustedWorkspaceRuntime, - resolveWorkspaceRuntimeFromParam, + resolveTrustedRuntime, sendGenerationClosedError, } from '../workspace-route-runtime.js'; import type { WorkspaceRegistry } from '../workspace-registry.js'; @@ -133,12 +132,12 @@ export function registerWorkspaceQualifiedToolsRoutes( '/workspaces/:workspace/tools/:name/enable', deps.mutate({ strict: true }), async (req, res) => { - const runtime = resolveWorkspaceRuntimeFromParam( + const runtime = resolveTrustedRuntime( deps.workspaceRegistry, req, res, ); - if (!runtime || !requireTrustedWorkspaceRuntime(runtime, res)) return; + if (!runtime) return; const rawToolName = req.params['name']; if (!rawToolName || typeof rawToolName !== 'string') { res.status(400).json({ diff --git a/packages/cli/src/serve/server-acp-child-extra-args.test.ts b/packages/cli/src/serve/server-acp-child-extra-args.test.ts index 95714d1decc..5383353318f 100644 --- a/packages/cli/src/serve/server-acp-child-extra-args.test.ts +++ b/packages/cli/src/serve/server-acp-child-extra-args.test.ts @@ -5,22 +5,32 @@ */ import { afterEach, describe, expect, it, vi } from 'vitest'; -import type { BridgeOptions } from './acp-session-bridge.js'; +import type { BridgeOptions } from '@qwen-code/acp-bridge/bridgeOptions'; const harness = vi.hoisted(() => ({ spawnFactoryCalls: [] as Array<{ extraArgs?: string[] }>, createAcpSessionBridge: vi.fn(), })); -vi.mock('./acp-session-bridge.js', async (importOriginal) => { +vi.mock('@qwen-code/acp-bridge/spawnChannel', async (importOriginal) => { const actual = - await importOriginal(); + await importOriginal< + typeof import('@qwen-code/acp-bridge/spawnChannel') + >(); return { ...actual, createSpawnChannelFactory: (options: { extraArgs?: string[] }) => { harness.spawnFactoryCalls.push(options); return actual.createSpawnChannelFactory(options); }, + }; +}); + +vi.mock('@qwen-code/acp-bridge/bridge', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, createAcpSessionBridge: (options: BridgeOptions) => { harness.createAcpSessionBridge(options); return new Proxy( diff --git a/packages/cli/src/serve/server-default-bridge-wiring.test.ts b/packages/cli/src/serve/server-default-bridge-wiring.test.ts index 7137475f0c7..a611fc9dab4 100644 --- a/packages/cli/src/serve/server-default-bridge-wiring.test.ts +++ b/packages/cli/src/serve/server-default-bridge-wiring.test.ts @@ -6,13 +6,15 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import * as path from 'node:path'; -import { - SessionNotFoundError, - type AcpSessionBridge, - type BridgeFreshSessionAdmission, - type BridgeOptions, - type BridgeSessionSummary, -} from './acp-session-bridge.js'; +import { SessionNotFoundError } from '@qwen-code/acp-bridge/bridgeErrors'; +import type { + AcpSessionBridge, + BridgeSessionSummary, +} from '@qwen-code/acp-bridge/bridgeTypes'; +import type { + BridgeFreshSessionAdmission, + BridgeOptions, +} from '@qwen-code/acp-bridge/bridgeOptions'; import type { WorkspaceRegistry } from './workspace-registry.js'; import type { WorkspaceFileSystemFactory } from './fs/workspace-file-system.js'; import { MAX_SESSION_RESTORE_TIMEOUT_MS } from '@qwen-code/acp-bridge/sessionRestoreTimeout'; @@ -48,7 +50,7 @@ function makeBridge( describe('createServeApp default bridge wiring', () => { afterEach(() => { - vi.doUnmock('./acp-session-bridge.js'); + vi.doUnmock('@qwen-code/acp-bridge/bridge'); vi.resetModules(); vi.restoreAllMocks(); }); @@ -58,10 +60,10 @@ describe('createServeApp default bridge wiring', () => { let bridgeOptions: BridgeOptions | undefined; const liveSessionIds = new Set(); const bridge = makeBridge(0, liveSessionIds); - vi.doMock('./acp-session-bridge.js', async () => { + vi.doMock('@qwen-code/acp-bridge/bridge', async () => { const actual = await vi.importActual< - typeof import('./acp-session-bridge.js') - >('./acp-session-bridge.js'); + typeof import('@qwen-code/acp-bridge/bridge') + >('@qwen-code/acp-bridge/bridge'); return { ...actual, createAcpSessionBridge: vi.fn((opts: BridgeOptions) => { @@ -130,10 +132,10 @@ describe('createServeApp default bridge wiring', () => { it('keeps the same-host write route disabled for an injected filesystem factory', async () => { let bridgeOptions: BridgeOptions | undefined; const bridge = makeBridge(); - vi.doMock('./acp-session-bridge.js', async () => { + vi.doMock('@qwen-code/acp-bridge/bridge', async () => { const actual = await vi.importActual< - typeof import('./acp-session-bridge.js') - >('./acp-session-bridge.js'); + typeof import('@qwen-code/acp-bridge/bridge') + >('@qwen-code/acp-bridge/bridge'); return { ...actual, createAcpSessionBridge: vi.fn((opts: BridgeOptions) => { @@ -186,10 +188,10 @@ describe('createServeApp default bridge wiring', () => { it('wires total admission into the internally-created bridge', async () => { let freshSessionAdmission: BridgeFreshSessionAdmission | undefined; - vi.doMock('./acp-session-bridge.js', async () => { + vi.doMock('@qwen-code/acp-bridge/bridge', async () => { const actual = await vi.importActual< - typeof import('./acp-session-bridge.js') - >('./acp-session-bridge.js'); + typeof import('@qwen-code/acp-bridge/bridge') + >('@qwen-code/acp-bridge/bridge'); return { ...actual, createAcpSessionBridge: vi.fn((opts: BridgeOptions) => { @@ -231,10 +233,10 @@ describe('createServeApp default bridge wiring', () => { it('wires the effective restore timeout into the direct bridge', async () => { const bridgeOptions: BridgeOptions[] = []; - vi.doMock('./acp-session-bridge.js', async () => { + vi.doMock('@qwen-code/acp-bridge/bridge', async () => { const actual = await vi.importActual< - typeof import('./acp-session-bridge.js') - >('./acp-session-bridge.js'); + typeof import('@qwen-code/acp-bridge/bridge') + >('@qwen-code/acp-bridge/bridge'); return { ...actual, createAcpSessionBridge: vi.fn((opts: BridgeOptions) => { @@ -262,10 +264,10 @@ describe('createServeApp default bridge wiring', () => { it('does not let a short initialize timeout lower the restore budget', async () => { const bridgeOptions: BridgeOptions[] = []; - vi.doMock('./acp-session-bridge.js', async () => { + vi.doMock('@qwen-code/acp-bridge/bridge', async () => { const actual = await vi.importActual< - typeof import('./acp-session-bridge.js') - >('./acp-session-bridge.js'); + typeof import('@qwen-code/acp-bridge/bridge') + >('@qwen-code/acp-bridge/bridge'); return { ...actual, createAcpSessionBridge: vi.fn((opts: BridgeOptions) => { @@ -330,10 +332,10 @@ describe('createServeApp default bridge wiring', () => { ), }; }); - vi.doMock('./acp-session-bridge.js', async () => { + vi.doMock('@qwen-code/acp-bridge/bridge', async () => { const actual = await vi.importActual< - typeof import('./acp-session-bridge.js') - >('./acp-session-bridge.js'); + typeof import('@qwen-code/acp-bridge/bridge') + >('@qwen-code/acp-bridge/bridge'); return { ...actual, createAcpSessionBridge: vi.fn(() => makeBridge()), diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 38e42dcc2b9..c50d79cb89c 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -106,7 +106,6 @@ import { InvalidClientIdError, InvalidPermissionOptionError, InvalidSessionMetadataError, - MAX_WORKSPACE_PATH_LENGTH, McpServerNotFoundError, McpServerRestartFailedError, PermissionForbiddenError, @@ -114,9 +113,6 @@ import { PromptQueueFullError, RestoreInProgressError, BridgeChannelQuarantinedError, - SessionRestoreTimeoutError, - SessionArtifactAuthorizationError, - SessionArtifactValidationError, SessionShellClientRequiredError, SessionShellDisabledError, SessionBusyError, @@ -125,19 +121,27 @@ import { TotalSessionLimitExceededError, WorkspaceDrainingError, WorkspaceMismatchError, - type BridgeHeartbeatResult, - type BridgeHeartbeatState, - type BridgeDaemonStatusSnapshot, - type BridgeRestoredSession, - type BridgeClientRequestContext, - type BridgeTurnStatus, - type BridgeRestoreSessionRequest, - type BridgeSession, - type BridgeSessionSummary, - type BridgeSpawnRequest, - type AcpSessionBridge, - type SessionMetadataUpdate, -} from './acp-session-bridge.js'; +} from '@qwen-code/acp-bridge/bridgeErrors'; +import { MAX_WORKSPACE_PATH_LENGTH } from '@qwen-code/acp-bridge/workspacePaths'; +import { SessionRestoreTimeoutError } from '@qwen-code/acp-bridge/status'; +import { + SessionArtifactAuthorizationError, + SessionArtifactValidationError, +} from '@qwen-code/acp-bridge/sessionArtifacts'; +import type { + BridgeHeartbeatResult, + BridgeHeartbeatState, + BridgeDaemonStatusSnapshot, + BridgeRestoredSession, + BridgeClientRequestContext, + BridgeTurnStatus, + BridgeRestoreSessionRequest, + BridgeSession, + BridgeSessionSummary, + BridgeSpawnRequest, + AcpSessionBridge, + SessionMetadataUpdate, +} from '@qwen-code/acp-bridge/bridgeTypes'; import type { BridgeEvent, SubscribeOptions, diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index 2bcca072db8..11b83e54801 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -66,15 +66,15 @@ import { createClientMcpServerProvider, } from './acp-http/client-mcp-sender-registry.js'; import { CdpTunnelRegistry } from './cdp-tunnel/cdp-tunnel-registry.js'; +import { canonicalizeWorkspace } from '@qwen-code/acp-bridge/workspacePaths'; +import { createAcpSessionBridge } from '@qwen-code/acp-bridge/bridge'; +import { createSpawnChannelFactory } from '@qwen-code/acp-bridge/spawnChannel'; import { - canonicalizeWorkspace, - createAcpSessionBridge, - createSpawnChannelFactory, MAX_SESSION_RESTORE_TIMEOUT_MS, resolveSessionRestoreTimeoutMs, - SessionNotFoundError, - type AcpSessionBridge, -} from './acp-session-bridge.js'; +} from '@qwen-code/acp-bridge/sessionRestoreTimeout'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; +import { SessionNotFoundError } from '@qwen-code/acp-bridge/bridgeErrors'; import { type ServeAuthProviderInstallRequest, type ServeAuthProviderInstallResult, @@ -333,7 +333,7 @@ export { resolveBoundWorkspacesFromIdeEnv, resolveBridgeFsFactory, } from './server/fs-factory.js'; -export { PromptDeadlineExceededError } from './acp-session-bridge.js'; +export { PromptDeadlineExceededError } from '@qwen-code/acp-bridge/bridgeErrors'; export { resolvePromptDeadlineMs } from './server/prompt-deadline.js'; export { detectFromLoopback } from './server/request-helpers.js'; export { diff --git a/packages/cli/src/serve/server/device-flow-registry.test.ts b/packages/cli/src/serve/server/device-flow-registry.test.ts index 4b8e876def0..50d26a180c8 100644 --- a/packages/cli/src/serve/server/device-flow-registry.test.ts +++ b/packages/cli/src/serve/server/device-flow-registry.test.ts @@ -11,7 +11,7 @@ import { type DeviceFlowProvider, type DeviceFlowProviderId, } from '../auth/device-flow.js'; -import type { WorkspaceEventPublisher } from '../acp-session-bridge.js'; +import type { WorkspaceEventPublisher } from '@qwen-code/acp-bridge/bridgeTypes'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; vi.mock('../../utils/stdioHelpers.js', () => ({ writeStderrLine: vi.fn() })); diff --git a/packages/cli/src/serve/server/device-flow-registry.ts b/packages/cli/src/serve/server/device-flow-registry.ts index dad97c33fb4..8d200feff73 100644 --- a/packages/cli/src/serve/server/device-flow-registry.ts +++ b/packages/cli/src/serve/server/device-flow-registry.ts @@ -6,7 +6,7 @@ import type { Application } from 'express'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; -import type { WorkspaceEventPublisher } from '../acp-session-bridge.js'; +import type { WorkspaceEventPublisher } from '@qwen-code/acp-bridge/bridgeTypes'; import { DeviceFlowRegistry, setDeviceFlowRegistry, diff --git a/packages/cli/src/serve/server/error-response.ts b/packages/cli/src/serve/server/error-response.ts index 47cd9f45576..fbfdd34fcf2 100644 --- a/packages/cli/src/serve/server/error-response.ts +++ b/packages/cli/src/serve/server/error-response.ts @@ -35,8 +35,6 @@ import { PermissionPolicyNotImplementedError, PromptQueueFullError, RestoreInProgressError, - SessionRestoreTimeoutError, - SessionArtifactAuthorizationError, SessionArchivedError, SessionArchivingError, SessionBusyError, @@ -53,7 +51,9 @@ import { WorkspaceMismatchError, WorkspaceDrainingError, TotalSessionLimitExceededError, -} from '../acp-session-bridge.js'; +} from '@qwen-code/acp-bridge/bridgeErrors'; +import { SessionRestoreTimeoutError } from '@qwen-code/acp-bridge/status'; +import { SessionArtifactAuthorizationError } from '@qwen-code/acp-bridge/sessionArtifacts'; import type { DaemonLogger } from '../daemon-logger.js'; import { mapWorkspaceSkillToggleError } from '../workspace-service/types.js'; import { sendGenerationClosedError } from '../workspace-route-runtime.js'; diff --git a/packages/cli/src/serve/server/persisted-session-list-cache.ts b/packages/cli/src/serve/server/persisted-session-list-cache.ts index 097d77e74fd..f7116724c18 100644 --- a/packages/cli/src/serve/server/persisted-session-list-cache.ts +++ b/packages/cli/src/serve/server/persisted-session-list-cache.ts @@ -6,7 +6,7 @@ import path from 'node:path'; import type { SessionArchiveState } from '@qwen-code/qwen-code-core'; -import type { BridgeSessionSummary } from '../acp-session-bridge.js'; +import type { BridgeSessionSummary } from '@qwen-code/acp-bridge/bridgeTypes'; export interface PersistedSessionListScope { runtimeBaseDir: string; diff --git a/packages/cli/src/serve/server/session-archive.test.ts b/packages/cli/src/serve/server/session-archive.test.ts index 343e8d26cef..d28be7025c0 100644 --- a/packages/cli/src/serve/server/session-archive.test.ts +++ b/packages/cli/src/serve/server/session-archive.test.ts @@ -27,7 +27,7 @@ import { SessionConflictError, SessionNotArchivedError, SessionNotFoundError, -} from '../acp-session-bridge.js'; +} from '@qwen-code/acp-bridge/bridgeErrors'; import { archiveDaemonSessions, assertSessionArchived, diff --git a/packages/cli/src/serve/server/session-archive.ts b/packages/cli/src/serve/server/session-archive.ts index 1040d1d443e..9aae2dceaac 100644 --- a/packages/cli/src/serve/server/session-archive.ts +++ b/packages/cli/src/serve/server/session-archive.ts @@ -11,14 +11,14 @@ import { SessionWriterUnavailableError, type SessionLocation, } from '@qwen-code/qwen-code-core'; -import type { AcpSessionBridge } from '../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { SessionArchivedError, SessionArchivingError, SessionConflictError, SessionNotArchivedError, SessionNotFoundError, -} from '../acp-session-bridge.js'; +} from '@qwen-code/acp-bridge/bridgeErrors'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; import { safeLogValue } from './request-helpers.js'; import { normalizeSessionIdForLookup } from '../../config/session-id.js'; diff --git a/packages/cli/src/serve/server/session-export.ts b/packages/cli/src/serve/server/session-export.ts index 90159760692..35095d268b9 100644 --- a/packages/cli/src/serve/server/session-export.ts +++ b/packages/cli/src/serve/server/session-export.ts @@ -9,7 +9,7 @@ import { SessionService, type SessionArchiveState, } from '@qwen-code/qwen-code-core'; -import { SessionNotFoundError } from '../acp-session-bridge.js'; +import { SessionNotFoundError } from '@qwen-code/acp-bridge/bridgeErrors'; import { collectSessionData, generateExportFilename, diff --git a/packages/cli/src/serve/server/session-list.ts b/packages/cli/src/serve/server/session-list.ts index 71a85a5187c..1ce2b287d6c 100644 --- a/packages/cli/src/serve/server/session-list.ts +++ b/packages/cli/src/serve/server/session-list.ts @@ -15,11 +15,11 @@ import { type SessionGroupPresetColor, type SessionPr, } from '@qwen-code/qwen-code-core'; -import type { SessionPrInfo } from '@qwen-code/acp-bridge/bridgeTypes'; import type { AcpSessionBridge, BridgeSessionSummary, -} from '../acp-session-bridge.js'; + SessionPrInfo, +} from '@qwen-code/acp-bridge/bridgeTypes'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; import { createSessionOrganizationService } from '../session-organization-helpers.js'; import { diff --git a/packages/cli/src/serve/session-id-admission.test.ts b/packages/cli/src/serve/session-id-admission.test.ts index 70e1dce60df..5fb13b98570 100644 --- a/packages/cli/src/serve/session-id-admission.test.ts +++ b/packages/cli/src/serve/session-id-admission.test.ts @@ -9,8 +9,8 @@ import * as os from 'node:os'; import * as path from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { SessionIdCaseConflictError } from '@qwen-code/qwen-code-core'; -import type { AcpSessionBridge } from './acp-session-bridge.js'; -import { SessionNotFoundError } from './acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; +import { SessionNotFoundError } from '@qwen-code/acp-bridge/bridgeErrors'; import { SessionArchiveCoordinator } from './server/session-archive.js'; import { createRequestedSessionIdAdmission, diff --git a/packages/cli/src/serve/session-id-admission.ts b/packages/cli/src/serve/session-id-admission.ts index e2991bd61ce..5409dd77cc5 100644 --- a/packages/cli/src/serve/session-id-admission.ts +++ b/packages/cli/src/serve/session-id-admission.ts @@ -9,10 +9,8 @@ import { SessionService, } from '@qwen-code/qwen-code-core'; import { access } from 'node:fs/promises'; -import { - SessionNotFoundError, - type AcpSessionBridge, -} from './acp-session-bridge.js'; +import { SessionNotFoundError } from '@qwen-code/acp-bridge/bridgeErrors'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { normalizeSessionIdForLookup } from '../config/session-id.js'; import type { SessionArchiveCoordinator } from './server/session-archive.js'; diff --git a/packages/cli/src/serve/total-session-admission.test.ts b/packages/cli/src/serve/total-session-admission.test.ts index ef914f22bdf..605b51e412e 100644 --- a/packages/cli/src/serve/total-session-admission.test.ts +++ b/packages/cli/src/serve/total-session-admission.test.ts @@ -8,7 +8,7 @@ import { describe, expect, it } from 'vitest'; import { TotalSessionLimitExceededError, WorkspaceDrainingError, -} from './acp-session-bridge.js'; +} from '@qwen-code/acp-bridge/bridgeErrors'; import { createTotalSessionAdmissionController } from './total-session-admission.js'; describe('createTotalSessionAdmissionController', () => { diff --git a/packages/cli/src/serve/total-session-admission.ts b/packages/cli/src/serve/total-session-admission.ts index 6f478c976e0..159d8e40c68 100644 --- a/packages/cli/src/serve/total-session-admission.ts +++ b/packages/cli/src/serve/total-session-admission.ts @@ -7,10 +7,12 @@ import { TotalSessionLimitExceededError, WorkspaceDrainingError, - type BridgeFreshSessionAdmission, - type BridgeFreshSessionAdmissionContext, - type BridgeFreshSessionReservation, -} from './acp-session-bridge.js'; +} from '@qwen-code/acp-bridge/bridgeErrors'; +import type { + BridgeFreshSessionAdmission, + BridgeFreshSessionAdmissionContext, + BridgeFreshSessionReservation, +} from '@qwen-code/acp-bridge/bridgeOptions'; interface SessionCountSource { readonly sessionCount: number; diff --git a/packages/cli/src/serve/workspace-agents.test.ts b/packages/cli/src/serve/workspace-agents.test.ts index 3d3b1b03ba0..b31ec64b77c 100644 --- a/packages/cli/src/serve/workspace-agents.test.ts +++ b/packages/cli/src/serve/workspace-agents.test.ts @@ -20,7 +20,7 @@ import { } from 'vitest'; import { QWEN_DIR, Storage } from '@qwen-code/qwen-code-core'; import { createMutationGate } from './auth.js'; -import type { AcpSessionBridge } from './acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import type { BridgeEvent } from '@qwen-code/acp-bridge/eventBus'; import { mountWorkspaceAgentsRoutes } from './workspace-agents.js'; diff --git a/packages/cli/src/serve/workspace-agents.ts b/packages/cli/src/serve/workspace-agents.ts index 91170bee815..3af309c9faf 100644 --- a/packages/cli/src/serve/workspace-agents.ts +++ b/packages/cli/src/serve/workspace-agents.ts @@ -24,10 +24,7 @@ import { import { loadSettings } from '../config/settings.js'; import { writeStderrLine } from '../utils/stdioHelpers.js'; import { isServeDebugMode } from './debug-mode.js'; -import { - InvalidClientIdError, - type AcpSessionBridge, -} from './acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { safeLogValue } from './server/request-helpers.js'; import { requireTrustedWorkspaceRuntime, @@ -2267,8 +2264,3 @@ export function createDaemonSubagentManager( }) as unknown as Config; return new SubagentManager(guarded); } - -// Re-export the bridge error type used by route helpers so test files -// can import it from a single module without reaching into -// acp-session-bridge directly. -export { InvalidClientIdError }; diff --git a/packages/cli/src/serve/workspace-generation.test.ts b/packages/cli/src/serve/workspace-generation.test.ts index 5f1e7734094..355010649fb 100644 --- a/packages/cli/src/serve/workspace-generation.test.ts +++ b/packages/cli/src/serve/workspace-generation.test.ts @@ -7,7 +7,7 @@ import express from 'express'; import request from 'supertest'; import { describe, expect, it, vi } from 'vitest'; -import type { AcpSessionBridge } from './acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { createMutationGate } from './auth.js'; import { mountWorkspaceGenerationRoutes } from './workspace-generation.js'; diff --git a/packages/cli/src/serve/workspace-generation.ts b/packages/cli/src/serve/workspace-generation.ts index c0ff835f54b..9b2aa6ae9e2 100644 --- a/packages/cli/src/serve/workspace-generation.ts +++ b/packages/cli/src/serve/workspace-generation.ts @@ -7,7 +7,7 @@ import type { Application, Request, RequestHandler, Response } from 'express'; import { GENERATION_MAX_PROMPT_BYTES } from '../acp-integration/generation.js'; import { writeStderrLine } from '../utils/stdioHelpers.js'; -import type { AcpSessionBridge } from './acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { formatGenerationSse, GENERATION_HEARTBEAT_MS, diff --git a/packages/cli/src/serve/workspace-git-state.test.ts b/packages/cli/src/serve/workspace-git-state.test.ts index 620af2c58d7..77af2ad9148 100644 --- a/packages/cli/src/serve/workspace-git-state.test.ts +++ b/packages/cli/src/serve/workspace-git-state.test.ts @@ -11,7 +11,7 @@ import { watchRepoBranch, type GitWorkingTreeStatus, } from '@qwen-code/qwen-code-core'; -import type { WorkspaceEventPublisher } from './acp-session-bridge.js'; +import type { WorkspaceEventPublisher } from '@qwen-code/acp-bridge/bridgeTypes'; import { writeStderrLineSafe } from '../utils/stdioHelpers.js'; import { WorkspaceGitState } from './workspace-git-state.js'; diff --git a/packages/cli/src/serve/workspace-git-state.ts b/packages/cli/src/serve/workspace-git-state.ts index d0f8eca8c54..741f03570a2 100644 --- a/packages/cli/src/serve/workspace-git-state.ts +++ b/packages/cli/src/serve/workspace-git-state.ts @@ -11,7 +11,7 @@ import { type GitOperation, type GitWorkingTreeStatus, } from '@qwen-code/qwen-code-core'; -import type { WorkspaceEventPublisher } from './acp-session-bridge.js'; +import type { WorkspaceEventPublisher } from '@qwen-code/acp-bridge/bridgeTypes'; import { writeStderrLineSafe } from '../utils/stdioHelpers.js'; export interface WorkspaceGitStatus { diff --git a/packages/cli/src/serve/workspace-memory.test.ts b/packages/cli/src/serve/workspace-memory.test.ts index b74e97578e5..40b35c94168 100644 --- a/packages/cli/src/serve/workspace-memory.test.ts +++ b/packages/cli/src/serve/workspace-memory.test.ts @@ -25,10 +25,8 @@ import { setMemoryFilename, } from '@qwen-code/qwen-code-core'; import { createMutationGate } from './auth.js'; -import { - InvalidClientIdError, - type WorkspaceEventBridge, -} from './acp-session-bridge.js'; +import { InvalidClientIdError } from '@qwen-code/acp-bridge/bridgeErrors'; +import type { WorkspaceEventBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import type { BridgeEvent } from '@qwen-code/acp-bridge/eventBus'; import { mountWorkspaceMemoryRoutes } from './workspace-memory.js'; diff --git a/packages/cli/src/serve/workspace-memory.ts b/packages/cli/src/serve/workspace-memory.ts index f1030780f90..66814c9baed 100644 --- a/packages/cli/src/serve/workspace-memory.ts +++ b/packages/cli/src/serve/workspace-memory.ts @@ -16,7 +16,7 @@ import { } from '@qwen-code/qwen-code-core'; import { writeStderrLine } from '../utils/stdioHelpers.js'; import { isServeDebugMode } from './debug-mode.js'; -import type { WorkspaceEventBridge } from './acp-session-bridge.js'; +import type { WorkspaceEventBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { createIdleWorkspaceMemoryStatus, STATUS_SCHEMA_VERSION, diff --git a/packages/cli/src/serve/workspace-qualified-rest.test.ts b/packages/cli/src/serve/workspace-qualified-rest.test.ts index a86d9ec1c8b..4950486285e 100644 --- a/packages/cli/src/serve/workspace-qualified-rest.test.ts +++ b/packages/cli/src/serve/workspace-qualified-rest.test.ts @@ -23,7 +23,7 @@ import { createWorkspaceRegistry, type WorkspaceRuntime, } from './workspace-registry.js'; -import type { AcpSessionBridge } from './acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { WorkspaceSkillNotFoundError, type DaemonWorkspaceService, diff --git a/packages/cli/src/serve/workspace-registry.test.ts b/packages/cli/src/serve/workspace-registry.test.ts index 3f6d5be8d5a..78129ece519 100644 --- a/packages/cli/src/serve/workspace-registry.test.ts +++ b/packages/cli/src/serve/workspace-registry.test.ts @@ -5,7 +5,7 @@ */ import { describe, expect, it } from 'vitest'; -import { SessionNotFoundError } from './acp-session-bridge.js'; +import { SessionNotFoundError } from '@qwen-code/acp-bridge/bridgeErrors'; import { createWorkspaceSessionOwnerIndex, createWorkspaceGenerationGuard, diff --git a/packages/cli/src/serve/workspace-registry.ts b/packages/cli/src/serve/workspace-registry.ts index 92de6e20ef9..48a6a894ffb 100644 --- a/packages/cli/src/serve/workspace-registry.ts +++ b/packages/cli/src/serve/workspace-registry.ts @@ -4,10 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { - SessionNotFoundError, - type AcpSessionBridge, -} from './acp-session-bridge.js'; +import { SessionNotFoundError } from '@qwen-code/acp-bridge/bridgeErrors'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import type { ClientMcpSenderRegistry } from './acp-http/client-mcp-sender-registry.js'; import type { WorkspaceFileSystemFactory } from './fs/index.js'; import type { WorkspaceRuntimeProvenance } from './managed-scratch-workspace.js'; diff --git a/packages/cli/src/serve/workspace-remember.test.ts b/packages/cli/src/serve/workspace-remember.test.ts index c42b0704def..406b27278b0 100644 --- a/packages/cli/src/serve/workspace-remember.test.ts +++ b/packages/cli/src/serve/workspace-remember.test.ts @@ -15,8 +15,8 @@ import type { BridgeWorkspaceMemoryForgetResult, BridgeWorkspaceMemoryRememberRequest, BridgeWorkspaceMemoryRememberResult, -} from './acp-session-bridge.js'; -import { WorkspaceDrainingError } from './acp-session-bridge.js'; +} from '@qwen-code/acp-bridge/bridgeTypes'; +import { WorkspaceDrainingError } from '@qwen-code/acp-bridge/bridgeErrors'; import type { BridgeEvent } from '@qwen-code/acp-bridge/eventBus'; import { mountWorkspaceMemoryRememberRoutes, diff --git a/packages/cli/src/serve/workspace-remember.ts b/packages/cli/src/serve/workspace-remember.ts index 8caa03c3a8c..64393652931 100644 --- a/packages/cli/src/serve/workspace-remember.ts +++ b/packages/cli/src/serve/workspace-remember.ts @@ -15,8 +15,8 @@ import type { BridgeWorkspaceMemoryRememberContextMode, BridgeWorkspaceMemoryRememberTargetScope, BridgeWorkspaceMemoryRememberResult, -} from './acp-session-bridge.js'; -import { WorkspaceDrainingError } from './acp-session-bridge.js'; +} from '@qwen-code/acp-bridge/bridgeTypes'; +import { WorkspaceDrainingError } from '@qwen-code/acp-bridge/bridgeErrors'; import { createWorkspaceMemoryExtractionErrorLogger, shouldSuppressRememberErrorDetails, diff --git a/packages/cli/src/serve/workspace-route-runtime.test.ts b/packages/cli/src/serve/workspace-route-runtime.test.ts index 14337c630b6..d5eb748e7e6 100644 --- a/packages/cli/src/serve/workspace-route-runtime.test.ts +++ b/packages/cli/src/serve/workspace-route-runtime.test.ts @@ -249,6 +249,7 @@ describe('resolveTrustedRuntime', () => { it('returns an active trusted runtime', () => { const runtime = makeRuntime(); const registry = createSingleWorkspaceRegistry(runtime); + const response = makeResponse(); expect( resolveTrustedRuntime( @@ -256,9 +257,10 @@ describe('resolveTrustedRuntime', () => { { params: { workspace: runtime.workspaceId }, } as unknown as Request, - makeResponse(), + response, ), ).toBe(runtime); + expect(response.status).not.toHaveBeenCalled(); }); it('rejects an active untrusted runtime', () => { diff --git a/packages/cli/src/serve/workspace-route-runtime.ts b/packages/cli/src/serve/workspace-route-runtime.ts index eee319039a3..a4f1fba88d7 100644 --- a/packages/cli/src/serve/workspace-route-runtime.ts +++ b/packages/cli/src/serve/workspace-route-runtime.ts @@ -7,7 +7,7 @@ import * as fs from 'node:fs'; import path from 'node:path'; import type { Request, Response } from 'express'; -import { canonicalizeWorkspace } from './acp-session-bridge.js'; +import { canonicalizeWorkspace } from '@qwen-code/acp-bridge/workspacePaths'; import type { WorkspaceEntry, WorkspaceRegistry, diff --git a/packages/cli/src/serve/workspace-service/__tests__/integration.test.ts b/packages/cli/src/serve/workspace-service/__tests__/integration.test.ts index 708439a28a7..4c9f3e40f82 100644 --- a/packages/cli/src/serve/workspace-service/__tests__/integration.test.ts +++ b/packages/cli/src/serve/workspace-service/__tests__/integration.test.ts @@ -20,7 +20,7 @@ import type { DaemonWorkspaceService, WorkspaceRequestContext, } from '../types.js'; -import type { AcpSessionBridge } from '../../acp-session-bridge.js'; +import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; // --------------------------------------------------------------------------- // Test fixtures diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index 016393ebd5c..d7041537e0e 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -98,6 +98,21 @@ import type { } from './hooks/useComposerCore'; import type { PromptFile, PromptImage } from './adapters/promptTypes'; import type { AttachmentPreviewRequest } from './adapters/messageTypes'; +import { + PromptAdmissionAttempt, + findUserMessageByIdentity, + getLatestUserBlock, + getLatestUserBlockId, + getRetryableTurnError, + matchesTurnErrorIdentity, + matchesUserMessageIdentity, + retryOwnerMatchesCurrent, + retryTranscriptIdentityMatches, + type CancelledRetryOwner, + type FailedPromptRetry, + type TranscriptTurnErrorIdentity, + type TranscriptUserMessageIdentity, +} from './prompt-admission'; import { StatusBar, type StatusBarHandle } from './components/StatusBar'; import { GoalStatusStrip } from './components/GoalStatusStrip'; import composerStatusStyles from './components/ComposerStatusStack.module.css'; @@ -577,26 +592,6 @@ interface FailedPrompt { owner: CancelledRetryOwner; } -interface TranscriptUserMessageIdentity { - block: DaemonTranscriptBlock; -} - -interface TranscriptTurnErrorIdentity { - block: DaemonTranscriptBlock; -} - -interface FailedPromptRetry { - sessionId: string; - messageId: string; - startedAt: number; - admitted: boolean; - settled: boolean; - owner: CancelledRetryOwner; - transcriptIdentity: - | { kind: 'failed-prompt'; identity: TranscriptUserMessageIdentity } - | { kind: 'turn-error'; identity: TranscriptTurnErrorIdentity }; -} - type CancelledRetryState = | { kind: 'failed-prompt'; @@ -618,30 +613,6 @@ type CancelledRetryState = type CancelledRetryRestoreResult = 'restored' | 'pending' | 'invalid'; -interface CancelledRetryOwner { - sessionId?: string; - workspaceCwd?: string; - sessionKey?: string; - sourceVersion: number; - snapshot: DaemonSessionOwnerSnapshot; -} - -function retryOwnerMatchesCurrent( - owner: CancelledRetryOwner, - sessionId: string | undefined, - workspaceCwd: string | undefined, - sourceVersion: number, -): boolean { - const workspaceMatches = - (owner.workspaceCwd !== undefined && owner.workspaceCwd === workspaceCwd) || - owner.snapshot.isCurrent(); - return ( - owner.sessionId === sessionId && - owner.sourceVersion === sourceVersion && - workspaceMatches - ); -} - interface CancelledRetryEntry { owner?: CancelledRetryOwner; state: CancelledRetryState; @@ -678,67 +649,6 @@ interface UnknownPromptAdmission { payloadAvailable: boolean; } -function getLatestUserBlockId( - blocks: readonly DaemonTranscriptBlock[], -): string | undefined { - return getLatestUserBlock(blocks)?.id; -} - -function getLatestUserBlock( - blocks: readonly DaemonTranscriptBlock[], -): DaemonTranscriptBlock | undefined { - for (let index = blocks.length - 1; index >= 0; index -= 1) { - const block = blocks[index]; - if ( - block?.kind === 'user' && - block.meta?.['source'] !== 'background_notification' - ) { - return block; - } - } - return undefined; -} - -function matchesUserMessageIdentity( - block: DaemonTranscriptBlock | undefined, - identity: TranscriptUserMessageIdentity | undefined, - allowLocalId = false, -): boolean { - if (!identity) return block === undefined; - if (!block || block.kind !== 'user' || identity.block.kind !== 'user') { - return false; - } - if (block === identity.block) return true; - if (allowLocalId && block.id === identity.block.id) return true; - const expectedRecords = identity.block.sourceRecordIds; - const currentRecords = block.sourceRecordIds; - return ( - expectedRecords !== undefined && - expectedRecords.length > 0 && - currentRecords !== undefined && - currentRecords.length === expectedRecords.length && - currentRecords.every((record, index) => record === expectedRecords[index]) - ); -} - -function findUserMessageByIdentity( - blocks: readonly DaemonTranscriptBlock[], - identity: TranscriptUserMessageIdentity, - allowLocalId = false, -): DaemonTranscriptBlock | undefined { - for (let index = blocks.length - 1; index >= 0; index -= 1) { - const block = blocks[index]; - if ( - block?.kind === 'user' && - block.meta?.['source'] !== 'background_notification' && - matchesUserMessageIdentity(block, identity, allowLocalId) - ) { - return block; - } - } - return undefined; -} - function getLogicalSessionKey( sessionId: string | undefined, workspaceCwd: string | undefined, @@ -746,62 +656,6 @@ function getLogicalSessionKey( return sessionId ? `${workspaceCwd ?? ''}\0${sessionId}` : undefined; } -function getRetryableTurnError( - blocks: readonly DaemonTranscriptBlock[], -): DaemonTranscriptBlock | undefined { - for (let i = blocks.length - 1; i >= 0; i--) { - const block = blocks[i]; - if (block?.kind === 'user') { - if (block.meta?.['source'] === 'background_notification') continue; - break; - } - if (block?.kind === 'error' && block.source === 'turn_error') { - return block; - } - if (block?.kind !== 'debug') break; - } - return undefined; -} - -function matchesTurnErrorIdentity( - block: DaemonTranscriptBlock | undefined, - identity: TranscriptTurnErrorIdentity, -): boolean { - if ( - !block || - block.kind !== 'error' || - block.source !== 'turn_error' || - identity.block.kind !== 'error' || - identity.block.source !== 'turn_error' - ) { - return false; - } - if (block === identity.block) return true; - const expectedPromptId = identity.block.promptId; - if (expectedPromptId) return block.promptId === expectedPromptId; - return ( - identity.block.eventId !== undefined && - block.eventId === identity.block.eventId - ); -} - -function retryTranscriptIdentityMatches( - blocks: readonly DaemonTranscriptBlock[], - transcriptIdentity: FailedPromptRetry['transcriptIdentity'], - allowLocalUserId = false, -): boolean { - return transcriptIdentity.kind === 'failed-prompt' - ? matchesUserMessageIdentity( - getLatestUserBlock(blocks), - transcriptIdentity.identity, - allowLocalUserId, - ) - : matchesTurnErrorIdentity( - getRetryableTurnError(blocks), - transcriptIdentity.identity, - ); -} - interface LocalAnchoredMessage { anchorAfterId?: string; anchorIndex: number; @@ -6455,8 +6309,7 @@ export function App({ } else if (opts?.clearComposerOnPromptStart) { editorRef.current?.clear(); } - let admissionStarted = false; - let admitted = false; + const admissionAttempt = new PromptAdmissionAttempt(); const promptOptions: SendPromptOptionsWithRetry = { images, files, @@ -6467,13 +6320,13 @@ export function App({ optimisticUserMessage: opts?.optimisticUserMessage, retry: opts?.retry, onAdmissionStarted: () => { - admissionStarted = true; + admissionAttempt.markStarted(); opts?.onAdmissionStarted?.( connectionRef.current.sessionId ?? allocatedSessionId, ); }, onAdmitted: () => { - admitted = true; + admissionAttempt.markAdmitted(); if (sessionIdAfterEnsure && promptWorkspaceCwd) { sessionCatalogController.promptAdmitted( promptWorkspaceCwd, @@ -6527,9 +6380,9 @@ export function App({ return await resultPromise; } catch (error) { if ( - admissionStarted && - !admitted && - !isDefinitelyRejectedPromptAdmission(error) && + admissionAttempt.classifyFailure( + isDefinitelyRejectedPromptAdmission(error), + ) === 'unknown' && promptWorkspaceCwd ) { sessionCatalogController.promptAdmissionUncertain(promptWorkspaceCwd); @@ -6844,18 +6697,19 @@ export function App({ owner: retryOwner, transcriptIdentity: retryTranscriptIdentity, }); - let admitted = false; - let admissionStarted = false; + const admissionAttempt = new PromptAdmissionAttempt(() => + retryOwnerIsCurrent(retryOwner), + ); sendPrompt(failed.text, failed.images, failed.files, { optimisticUserMessage: false, skipPrepareSubmit: true, inputAnnotations: failed.inputAnnotations, onAdmissionStarted: () => { - admissionStarted = true; + admissionAttempt.markStarted(); }, onAdmitted: () => { - admitted = true; - if (!retryOwnerIsCurrent(retryOwner)) return; + admissionAttempt.markAdmitted(); + if (!admissionAttempt.isCurrent()) return; setFailedPromptRetry((current) => current?.transcriptIdentity === retryTranscriptIdentity ? retryTranscriptIsCurrent() @@ -6873,9 +6727,11 @@ export function App({ }, }) .catch((error: unknown) => { - if (!retryOwnerIsCurrent(retryOwner)) return; - const definitelyRejected = isDefinitelyRejectedPromptAdmission(error); - if (admissionStarted && !admitted && !definitelyRejected) { + if (!admissionAttempt.isCurrent()) return; + const failure = admissionAttempt.classifyFailure( + isDefinitelyRejectedPromptAdmission(error), + ); + if (failure === 'unknown') { if (!retryTranscriptIsCurrent()) return; updateUnknownPromptAdmission({ sessionId: failed.sessionId, @@ -6893,7 +6749,7 @@ export function App({ ); return; } - if (!admitted) { + if (failure !== 'after-admission') { restoreOrDeferCancelledRetry(retryOwner, { kind: 'failed-prompt', attemptId: retryAttemptId, @@ -6905,7 +6761,7 @@ export function App({ } }) .finally(() => { - if (!retryOwnerIsCurrent(retryOwner)) return; + if (!admissionAttempt.isCurrent()) return; setFailedPromptRetry((current) => current?.transcriptIdentity === retryTranscriptIdentity ? retryTranscriptIsCurrent() @@ -9627,6 +9483,9 @@ export function App({ (admissionOwner.sessionId === undefined || (connectionRef.current.sessionId === admissionOwner.sessionId && getComposerWorkspaceCwd() === admissionOwner.workspaceCwd)); + const admissionAttempt = new PromptAdmissionAttempt( + admissionOwnerIsCurrent, + ); const { trackSendFailure = false, ...sendOptions } = opts ?? {}; const startedWithoutSession = !connectionRef.current.sessionId; const deferComposerCommit = @@ -9637,8 +9496,6 @@ export function App({ let optimisticUserMessage: OptimisticUserMessage | undefined; let submittedPromptText = promptText; let submittedInputAnnotations = sendOptions.inputAnnotations; - let admitted = false; - let admissionStarted = false; let admissionSessionId: string | undefined; sendPrompt(promptText, promptImages, promptFiles, { ownerRef: admissionAttachment, @@ -9654,11 +9511,11 @@ export function App({ ? commitComposerAccepted : undefined, onAdmissionStarted: (sessionId) => { - admissionStarted = true; + admissionAttempt.markStarted(); admissionSessionId = sessionId; }, onAdmitted: () => { - admitted = true; + admissionAttempt.markAdmitted(); }, ...(trackSendFailure ? { @@ -9668,10 +9525,12 @@ export function App({ } : {}), }).catch((error: unknown) => { - if (!admissionOwnerIsCurrent()) return; + if (!admissionAttempt.isCurrent()) return; const failedMessage = optimisticUserMessage; - const definitelyRejected = isDefinitelyRejectedPromptAdmission(error); - if (admissionStarted && !admitted && !definitelyRejected) { + const failure = admissionAttempt.classifyFailure( + isDefinitelyRejectedPromptAdmission(error), + ); + if (failure === 'unknown') { updateFailedPrompt(null); const uncertainSessionId = failedMessage?.sessionId ?? @@ -9697,7 +9556,7 @@ export function App({ } if ( trackSendFailure && - !admitted && + failure !== 'after-admission' && failedMessage && failedMessage.sessionId === connectionRef.current.sessionId && matchesUserMessageIdentity( @@ -9720,7 +9579,7 @@ export function App({ inputAnnotations: submittedInputAnnotations, }); } - if (startedWithoutSession && !admissionStarted) { + if (startedWithoutSession && !admissionAttempt.isStarted()) { const editor = editorRef.current; if (editor && !editor.hasInput()) { editor.setText(submittedPromptText); @@ -10920,18 +10779,19 @@ export function App({ owner: retryOwner, transcriptIdentity: retryTranscriptIdentity, }); - let admissionStarted = false; - let admitted = false; + const admissionAttempt = new PromptAdmissionAttempt(() => + retryOwnerIsCurrent(retryOwner), + ); sendPrompt(retryText, retryImages, retryFiles, { optimisticUserMessage: false, retry: true, inputAnnotations: retryInputAnnotations, onAdmissionStarted: () => { - admissionStarted = true; + admissionAttempt.markStarted(); }, onAdmitted: () => { - admitted = true; - if (!retryOwnerIsCurrent(retryOwner)) return; + admissionAttempt.markAdmitted(); + if (!admissionAttempt.isCurrent()) return; setFailedPromptRetry((current) => current?.transcriptIdentity === retryTranscriptIdentity ? retryTranscriptIsCurrent() @@ -10956,9 +10816,11 @@ export function App({ }, }) .catch((error: unknown) => { - if (!retryOwnerIsCurrent(retryOwner)) return; - const definitelyRejected = isDefinitelyRejectedPromptAdmission(error); - if (admissionStarted && !admitted && !definitelyRejected) { + if (!admissionAttempt.isCurrent()) return; + const failure = admissionAttempt.classifyFailure( + isDefinitelyRejectedPromptAdmission(error), + ); + if (failure === 'unknown') { if (!retryTranscriptIsCurrent()) return; updateUnknownPromptAdmission({ sessionId: retrySessionId, @@ -10976,7 +10838,7 @@ export function App({ ); return; } - if (!admitted) { + if (failure !== 'after-admission') { restoreOrDeferCancelledRetry(retryOwner, { kind: 'turn-error', attemptId: retryAttemptId, @@ -11024,7 +10886,7 @@ export function App({ } }) .finally(() => { - if (!retryOwnerIsCurrent(retryOwner)) return; + if (!admissionAttempt.isCurrent()) return; setFailedPromptRetry((current) => current?.transcriptIdentity === retryTranscriptIdentity ? retryTranscriptIsCurrent() diff --git a/packages/web-shell/client/prompt-admission.test.ts b/packages/web-shell/client/prompt-admission.test.ts new file mode 100644 index 00000000000..2157d781ee6 --- /dev/null +++ b/packages/web-shell/client/prompt-admission.test.ts @@ -0,0 +1,81 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it, vi } from 'vitest'; +import type { DaemonTranscriptBlock } from '@qwen-code/sdk/daemon'; +import { + PromptAdmissionAttempt, + getLatestUserBlock, + matchesUserMessageIdentity, + retryOwnerMatchesCurrent, +} from './prompt-admission'; + +function userBlock( + id: string, + sourceRecordIds?: string[], +): DaemonTranscriptBlock { + return { + id, + kind: 'user', + text: id, + sourceRecordIds, + } as DaemonTranscriptBlock; +} + +describe('PromptAdmissionAttempt', () => { + it('distinguishes rejection, unknown outcome, and post-admission failure', () => { + const attempt = new PromptAdmissionAttempt(); + expect(attempt.classifyFailure(false)).toBe('rejected'); + attempt.markStarted(); + expect(attempt.classifyFailure(true)).toBe('rejected'); + expect(attempt.classifyFailure(false)).toBe('unknown'); + attempt.markAdmitted(); + expect(attempt.classifyFailure(false)).toBe('after-admission'); + }); + + it('owns the current attachment predicate', () => { + const isCurrent = vi.fn(() => false); + expect(new PromptAdmissionAttempt(isCurrent).isCurrent()).toBe(false); + expect(isCurrent).toHaveBeenCalledOnce(); + }); +}); + +describe('prompt transcript identity', () => { + it('skips background notifications when selecting the latest user block', () => { + const expected = userBlock('expected'); + const background = { + ...userBlock('background'), + meta: { source: 'background_notification' }, + } as DaemonTranscriptBlock; + expect(getLatestUserBlock([expected, background])).toBe(expected); + }); + + it('matches replayed blocks by source record identity', () => { + const expected = userBlock('local', ['record-1']); + const replayed = userBlock('server', ['record-1']); + expect(matchesUserMessageIdentity(replayed, { block: expected })).toBe( + true, + ); + }); +}); + +describe('retryOwnerMatchesCurrent', () => { + it('rejects a stale source version even when the attachment still matches', () => { + expect( + retryOwnerMatchesCurrent( + { + sessionId: 'session-1', + workspaceCwd: '/workspace', + sourceVersion: 1, + snapshot: { isCurrent: () => true }, + }, + 'session-1', + '/workspace', + 2, + ), + ).toBe(false); + }); +}); diff --git a/packages/web-shell/client/prompt-admission.ts b/packages/web-shell/client/prompt-admission.ts new file mode 100644 index 00000000000..e9b9ed0d105 --- /dev/null +++ b/packages/web-shell/client/prompt-admission.ts @@ -0,0 +1,201 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { DaemonSessionOwnerSnapshot } from '@qwen-code/webui/daemon-react-sdk'; +import type { DaemonTranscriptBlock } from '@qwen-code/sdk/daemon'; + +export interface TranscriptUserMessageIdentity { + block: DaemonTranscriptBlock; +} + +export interface TranscriptTurnErrorIdentity { + block: DaemonTranscriptBlock; +} + +export interface CancelledRetryOwner { + sessionId?: string; + workspaceCwd?: string; + sessionKey?: string; + sourceVersion: number; + snapshot: DaemonSessionOwnerSnapshot; +} + +export interface FailedPromptRetry { + sessionId: string; + messageId: string; + startedAt: number; + admitted: boolean; + settled: boolean; + owner: CancelledRetryOwner; + transcriptIdentity: + | { kind: 'failed-prompt'; identity: TranscriptUserMessageIdentity } + | { kind: 'turn-error'; identity: TranscriptTurnErrorIdentity }; +} + +export type PromptAdmissionFailureKind = + | 'rejected' + | 'unknown' + | 'after-admission'; + +export class PromptAdmissionAttempt { + private started = false; + private admitted = false; + + constructor(private readonly ownerIsCurrent: () => boolean = () => true) {} + + markStarted(): void { + this.started = true; + } + + markAdmitted(): void { + this.admitted = true; + } + + isStarted(): boolean { + return this.started; + } + + isCurrent(): boolean { + return this.ownerIsCurrent(); + } + + classifyFailure(definitelyRejected: boolean): PromptAdmissionFailureKind { + if (this.admitted) return 'after-admission'; + if (this.started && !definitelyRejected) return 'unknown'; + return 'rejected'; + } +} + +export function retryOwnerMatchesCurrent( + owner: CancelledRetryOwner, + sessionId: string | undefined, + workspaceCwd: string | undefined, + sourceVersion: number, +): boolean { + const workspaceMatches = + (owner.workspaceCwd !== undefined && owner.workspaceCwd === workspaceCwd) || + owner.snapshot.isCurrent(); + return ( + owner.sessionId === sessionId && + owner.sourceVersion === sourceVersion && + workspaceMatches + ); +} + +export function getLatestUserBlockId( + blocks: readonly DaemonTranscriptBlock[], +): string | undefined { + return getLatestUserBlock(blocks)?.id; +} + +export function getLatestUserBlock( + blocks: readonly DaemonTranscriptBlock[], +): DaemonTranscriptBlock | undefined { + for (let index = blocks.length - 1; index >= 0; index -= 1) { + const block = blocks[index]; + if ( + block?.kind === 'user' && + block.meta?.['source'] !== 'background_notification' + ) { + return block; + } + } + return undefined; +} + +export function matchesUserMessageIdentity( + block: DaemonTranscriptBlock | undefined, + identity: TranscriptUserMessageIdentity | undefined, + allowLocalId = false, +): boolean { + if (!identity) return block === undefined; + if (!block || block.kind !== 'user' || identity.block.kind !== 'user') { + return false; + } + if (block === identity.block) return true; + if (allowLocalId && block.id === identity.block.id) return true; + const expectedRecords = identity.block.sourceRecordIds; + const currentRecords = block.sourceRecordIds; + return ( + expectedRecords !== undefined && + expectedRecords.length > 0 && + currentRecords !== undefined && + currentRecords.length === expectedRecords.length && + currentRecords.every((record, index) => record === expectedRecords[index]) + ); +} + +export function findUserMessageByIdentity( + blocks: readonly DaemonTranscriptBlock[], + identity: TranscriptUserMessageIdentity, + allowLocalId = false, +): DaemonTranscriptBlock | undefined { + for (let index = blocks.length - 1; index >= 0; index -= 1) { + const block = blocks[index]; + if ( + block?.kind === 'user' && + block.meta?.['source'] !== 'background_notification' && + matchesUserMessageIdentity(block, identity, allowLocalId) + ) { + return block; + } + } + return undefined; +} + +export function getRetryableTurnError( + blocks: readonly DaemonTranscriptBlock[], +): DaemonTranscriptBlock | undefined { + for (let index = blocks.length - 1; index >= 0; index--) { + const block = blocks[index]; + if (block?.kind === 'user') { + if (block.meta?.['source'] === 'background_notification') continue; + break; + } + if (block?.kind === 'error' && block.source === 'turn_error') return block; + if (block?.kind !== 'debug') break; + } + return undefined; +} + +export function matchesTurnErrorIdentity( + block: DaemonTranscriptBlock | undefined, + identity: TranscriptTurnErrorIdentity, +): boolean { + if ( + !block || + block.kind !== 'error' || + block.source !== 'turn_error' || + identity.block.kind !== 'error' || + identity.block.source !== 'turn_error' + ) { + return false; + } + if (block === identity.block) return true; + const expectedPromptId = identity.block.promptId; + if (expectedPromptId) return block.promptId === expectedPromptId; + return ( + identity.block.eventId !== undefined && + block.eventId === identity.block.eventId + ); +} + +export function retryTranscriptIdentityMatches( + blocks: readonly DaemonTranscriptBlock[], + transcriptIdentity: FailedPromptRetry['transcriptIdentity'], + allowLocalUserId = false, +): boolean { + return transcriptIdentity.kind === 'failed-prompt' + ? matchesUserMessageIdentity( + getLatestUserBlock(blocks), + transcriptIdentity.identity, + allowLocalUserId, + ) + : matchesTurnErrorIdentity( + getRetryableTurnError(blocks), + transcriptIdentity.identity, + ); +} diff --git a/packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx b/packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx index 572c9b4e536..9829a7fefe0 100644 --- a/packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx +++ b/packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx @@ -15117,6 +15117,96 @@ describe('DaemonSessionProvider', () => { await flushPromises(); }); } + + // R9-1 regression fixture: a superseded restore run must not settle the + // successor's pending load. The stale run's `client.resume()` settles in the + // microtask queued BEFORE the successor registers its own load, so the stale + // run reaches the adoption point while `attachmentLifecycle.pendingLoad` is + // already the successor's. Without the attempted-load guard it resolves the + // successor's promise even though the successor's own restore never settled. + it('does not let a superseded restore settle the successor pending load', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response(null, { status: 204 })), + ); + const sessionA = createMockSession({ + sessionId: 'session-a', + events: createIdleEvents(), + }); + sdkMocks.sessions.push(sessionA); + let actions: DaemonSessionActions | undefined; + function Harness() { + actions = useDaemonActions(); + return null; + } + await renderWithProvider(, { autoConnect: true }); + await act(async () => { + await flushPromises(); + }); + + const staleRestore = createDeferred(); + const neverSettles = createDeferred(); + sdkMocks.MockDaemonSessionClient.resume + .mockImplementationOnce(async () => staleRestore.promise) + .mockImplementationOnce(async () => neverSettles.promise); + + let firstOutcome: string | undefined; + let secondOutcome: string | undefined; + act(() => { + void requireActions(actions) + .resumeSession('session-b') + .then( + () => { + firstOutcome = 'resolved'; + }, + () => { + firstOutcome = 'rejected'; + }, + ); + }); + await act(async () => { + await flushPromises(); + }); + + const sessionB = createMockSession({ + sessionId: 'session-b', + events: createIdleEvents(), + replaySnapshot: createTextReplaySnapshot('session b transcript'), + }); + + await act(async () => { + // Queue the stale run's continuation first, then register the successor + // load synchronously — the stale run wakes up with pendingLoad already + // replaced. + staleRestore.resolve(sessionB); + void requireActions(actions) + .resumeSession('session-b') + .then( + () => { + secondOutcome = 'resolved'; + }, + () => { + secondOutcome = 'rejected'; + }, + ); + await flushPromises(); + await flushPromises(); + await flushPromises(); + }); + + // The first switch was superseded, so it rejects with AbortError. + expect(firstOutcome).toBe('rejected'); + // The successor's own restore never settled, so nothing may settle it. + expect(secondOutcome).toBeUndefined(); + + await act(async () => { + neverSettles.reject(new Error('cleanup')); + root?.unmount(); + root = null; + await flushPromises(); + }); + }); + }); function requireActions(actions: T | undefined): T { diff --git a/packages/webui/src/daemon/session/DaemonSessionProvider.tsx b/packages/webui/src/daemon/session/DaemonSessionProvider.tsx index 614d4807155..cb697d3624e 100644 --- a/packages/webui/src/daemon/session/DaemonSessionProvider.tsx +++ b/packages/webui/src/daemon/session/DaemonSessionProvider.tsx @@ -48,6 +48,7 @@ import { normalizeWorkspaceIdentity, resolveSessionRestoreTimeouts, } from './actions.js'; +import { SessionAttachmentLifecycle } from './attachment-lifecycle.js'; import { eventPromptId, findLiveJournalRepairSuffix, @@ -948,10 +949,9 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { const lastSessionIdRef = useRef(undefined); const activePromptsRef = useRef>(new Map()); const settledPromptsRef = useRef>(new Map()); - const pendingSessionLoadRef = useRef( - undefined, + const [attachmentLifecycle] = useState( + () => new SessionAttachmentLifecycle(), ); - const pendingSessionLoadIdRef = useRef(0); const liveJournalRepairRef = useRef( undefined, ); @@ -966,10 +966,6 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { const heartbeatFailureStateRef = useRef({ consecutiveFailures: 0, }); - const manualSessionClearRef = useRef(false); - const skipNextCleanupDetachSessionRef = useRef< - DaemonSessionClient | undefined - >(undefined); const settledRestoredActivePromptSessionsRef = useRef< WeakSet >(new WeakSet()); @@ -1105,8 +1101,8 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { restoreMode === 'load' && restoreSessionId !== undefined && restoreSessionId === sessionRef.current?.sessionId && - sessionRef.current === skipNextCleanupDetachSessionRef.current; - const effectPendingSessionLoad = pendingSessionLoadRef.current; + attachmentLifecycle.isCleanupDetachPreserved(sessionRef.current); + const effectPendingSessionLoad = attachmentLifecycle.pendingLoad; let runnerSession = sessionRef.current; // ── Batched transcript dispatch ──────────────────────────────── @@ -1193,7 +1189,7 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { !repair || repair.attempted || !repair.terminalSeen || - pendingSessionLoadRef.current || + attachmentLifecycle.pendingLoad || transcriptHistoryRef.current.loading || sessionRef.current?.sessionId !== repair.sessionId || hasCurrentSessionActivePromptRef.current() @@ -1236,7 +1232,7 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { | undefined; let reconnectSessionId = restoreSessionId; let shouldCreateFreshSession = - !manualSessionClearRef.current && + !attachmentLifecycle.isManuallyCleared && !restoreSessionId && newSessionNonce > 0; let reconnectAttempt = 0; @@ -1318,6 +1314,11 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { attachedExistingSession = true; } } + // R9-1: the load the current loop iteration attempted. Reassigned + // per iteration that captures a load (iteration-scoped, not + // run-scoped), lifted out of the !session block so the post-restore + // success path below can still see it. + let attemptedLoadForIteration: PendingSessionLoad | undefined; if (!session) { if (!preservingTranscriptDuringLoad) { setConnection((current) => ({ @@ -1362,7 +1363,7 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { capabilityFeatures.includes(WORKSPACE_ACP_STATUS_FEATURE); if ( (shouldDeferInitialSessionCreation || - manualSessionClearRef.current) && + attachmentLifecycle.isManuallyCleared) && !restoreSessionId && !reconnectSessionId && !shouldCreateFreshSession @@ -1420,7 +1421,7 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { ); const preserveClearedSessionCommands = skillsResult.status === 'rejected' || - (manualSessionClearRef.current && + (attachmentLifecycle.isManuallyCleared && deferredSkillCommands.length === 0); setConnection((current) => ({ ...current, @@ -1516,9 +1517,10 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { })); } const attemptedLoad = - pendingSessionLoadRef.current?.sessionId === targetSessionId - ? pendingSessionLoadRef.current + attachmentLifecycle.pendingLoad?.sessionId === targetSessionId + ? attachmentLifecycle.pendingLoad : undefined; + attemptedLoadForIteration = attemptedLoad; const restoreRequestTimeoutMs = attemptedLoad?.requestTimeoutMs ?? resolveSessionRestoreTimeouts(capabilities).requestTimeoutMs; @@ -1600,7 +1602,7 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { preservingTranscriptDuringLoad && attemptedLoad?.sessionId === nextSession.sessionId && (attemptedLoad.signal?.aborted || - pendingSessionLoadRef.current !== attemptedLoad) + attachmentLifecycle.pendingLoad !== attemptedLoad) ) { const previousSession = sessionRef.current; if (nextSession !== previousSession) { @@ -1611,18 +1613,17 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { ); }); } - if (pendingSessionLoadRef.current === attemptedLoad) { - pendingSessionLoadRef.current = undefined; - if (attemptedLoad.timeout !== undefined) { - clearTimeout(attemptedLoad.timeout); - } - attemptedLoad.reject( - new DOMException('Session load cancelled', 'AbortError'), - ); - } - if (skipNextCleanupDetachSessionRef.current === previousSession) { - skipNextCleanupDetachSessionRef.current = undefined; - } + attachmentLifecycle.rejectPendingLoad( + attemptedLoad, + new DOMException('Session load cancelled', 'AbortError'), + ); + // R9-1: pass the load this run attempted — without it this + // release matches a successor's re-preserved exemption by + // session identity alone and clears it. + attachmentLifecycle.releaseCleanupDetachExemption( + previousSession, + attemptedLoad, + ); loadingRequestedSession = false; if (previousSession?.sessionId === nextSession.sessionId) { session = previousSession; @@ -1660,24 +1661,19 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { ); }); } - if (pendingSessionLoadRef.current === attemptedLoad) { - pendingSessionLoadRef.current = undefined; - if (attemptedLoad.timeout !== undefined) { - clearTimeout(attemptedLoad.timeout); - } - attemptedLoad.reject( - new Error( - nextSession.replayDegraded === true - ? 'Fresh replay is degraded' - : 'Fresh replay does not contain the complete target turn', - ), - ); - } - if ( - skipNextCleanupDetachSessionRef.current === previousSession - ) { - skipNextCleanupDetachSessionRef.current = undefined; - } + attachmentLifecycle.rejectPendingLoad( + attemptedLoad, + new Error( + nextSession.replayDegraded === true + ? 'Fresh replay is degraded' + : 'Fresh replay does not contain the complete target turn', + ), + ); + // R9-1: same load-identity guard as the tail-cancel twin. + attachmentLifecycle.releaseCleanupDetachExemption( + previousSession, + attemptedLoad, + ); if (previousSession?.sessionId === nextSession.sessionId) { session = previousSession; reconnectSessionId = previousSession.sessionId; @@ -1777,9 +1773,17 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { hasCurrentSessionActivePromptRef.current = hasSessionActivePrompt; setPromptStatus(hasSessionActivePrompt() ? 'streaming' : 'idle'); - const pendingLoad = pendingSessionLoadRef.current; + const pendingLoad = attachmentLifecycle.pendingLoad; + // R9-1: a superseded load attempt must not adopt the successor's + // re-preserved pending load — session identity alone cannot tell + // them apart. Iterations without a load capture keep the pre-R9-1 + // adoption behaviour; that bypass is what lets creation flows + // resolve, since they register the load only after the capture + // point above. const pendingLoadToResolve = - pendingLoad?.sessionId === activeSession.sessionId + pendingLoad?.sessionId === activeSession.sessionId && + (attemptedLoadForIteration === undefined || + pendingLoad === attemptedLoadForIteration) ? pendingLoad : undefined; @@ -2254,14 +2258,11 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { lastHandledSessionIdRef.current = activeSession.sessionId; lastHandledWorkspaceRef.current = activeSession.workspaceCwd; lastHandledClientIdRef.current = undefined; - pendingSessionLoadRef.current = undefined; - if (pendingLoadToResolve.timeout !== undefined) { - clearTimeout(pendingLoadToResolve.timeout); - } - if (skipNextCleanupDetachSessionRef.current === activeSession) { - skipNextCleanupDetachSessionRef.current = undefined; - } - pendingLoadToResolve.resolve(); + attachmentLifecycle.resolvePendingLoad(pendingLoadToResolve); + attachmentLifecycle.releaseCleanupDetachExemption( + activeSession, + pendingLoadToResolve, + ); } const canReuseSessionMetadata = @@ -2970,7 +2971,7 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { })); return; } - if (manualSessionClearRef.current) { + if (attachmentLifecycle.isManuallyCleared) { session = undefined; sessionRef.current = undefined; hasCurrentSessionActivePromptRef.current = () => false; @@ -3032,7 +3033,7 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { const message = error instanceof Error ? error.message : String(error); const errorStatus = extractHttpStatus(error); - const pendingLoad = pendingSessionLoadRef.current; + const pendingLoad = attachmentLifecycle.pendingLoad; const restoreRetryDelayMs = getRestoreInProgressRetryDelayMs(error); const pendingLoadMatches = pendingLoad === undefined || @@ -3060,7 +3061,7 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { ); if ( pendingLoad !== undefined && - pendingSessionLoadRef.current !== pendingLoad + attachmentLifecycle.pendingLoad !== pendingLoad ) { return; } @@ -3086,17 +3087,7 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { (pendingLoad.sessionId === restoreSessionId || pendingLoad.sessionId === reconnectSessionId) ) { - if ( - skipNextCleanupDetachSessionRef.current?.sessionId === - pendingLoad.sessionId - ) { - skipNextCleanupDetachSessionRef.current = undefined; - } - pendingSessionLoadRef.current = undefined; - if (pendingLoad.timeout !== undefined) { - clearTimeout(pendingLoad.timeout); - } - pendingLoad.reject(error); + attachmentLifecycle.rejectPendingLoad(pendingLoad, error); } if (isAuthFailure || isTerminal) { // Auth failures (401/403) and terminal session errors (404/410) @@ -3271,7 +3262,7 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { session === undefined && sessionRef.current === undefined; const keepSessionForNextEffect = ownsCurrentSession && - session === skipNextCleanupDetachSessionRef.current; + attachmentLifecycle.isCleanupDetachPreserved(session); const isUnmounting = !mountedRef.current; if (ownsCurrentSession || ownsEmptyState) { // A same-attachment effect restart must flush events already yielded by @@ -3289,17 +3280,14 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { } if ( effectPendingSessionLoad !== undefined && - pendingSessionLoadRef.current === effectPendingSessionLoad && + attachmentLifecycle.isCurrentLoad(effectPendingSessionLoad) && (ownsCurrentSession || ownsEmptyState) && (!keepSessionForNextEffect || isUnmounting) ) { - if (pendingSessionLoadRef.current.timeout !== undefined) { - clearTimeout(pendingSessionLoadRef.current.timeout); - } - pendingSessionLoadRef.current.reject( + attachmentLifecycle.rejectPendingLoad( + effectPendingSessionLoad, new DOMException('Session load interrupted by cleanup', 'AbortError'), ); - pendingSessionLoadRef.current = undefined; } if ( ownsCurrentSession && @@ -3320,6 +3308,7 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { } }; }, [ + attachmentLifecycle, autoConnect, autoReconnect, resolvedBaseUrl, @@ -3452,7 +3441,7 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { setPromptStatus('idle'); if (sessionRef.current === session) { if (missingSession) { - manualSessionClearRef.current = true; + attachmentLifecycle.markManuallyCleared(); } sessionRef.current = undefined; } @@ -3489,6 +3478,7 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { clearInterval(timer); }; }, [ + attachmentLifecycle, connection.sessionId, connection.status, heartbeatFailureThreshold, @@ -3502,12 +3492,9 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { sessionRef, activePromptsRef, settledPromptsRef, - pendingSessionLoadRef, - pendingSessionLoadIdRef, + attachmentLifecycle, sessionConfigGeneration: sessionConfigGenerationRef.current, heartbeatSupportedRef, - manualSessionClearRef, - skipNextCleanupDetachSessionRef, passiveAssistantDoneTimerRef, hasSessionActivePrompt: () => hasCurrentSessionActivePromptRef.current(), @@ -3597,6 +3584,7 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { }), [ addNotice, + attachmentLifecycle, clientId, resolvedBaseUrl, resolvedToken, diff --git a/packages/webui/src/daemon/session/actions.test.ts b/packages/webui/src/daemon/session/actions.test.ts index 4af5cdab8f6..4d1314f0fb0 100644 --- a/packages/webui/src/daemon/session/actions.test.ts +++ b/packages/webui/src/daemon/session/actions.test.ts @@ -18,6 +18,7 @@ import type { PendingSessionLoad, SettledPrompt, } from './types'; +import { SessionAttachmentLifecycle } from './attachment-lifecycle'; describe('getConnectionAfterSessionClear', () => { it('clears session fields for the session being detached', () => { @@ -528,13 +529,12 @@ describe('createDaemonSessionActions', () => { it('does not restore a detached session after the session was cleared', async () => { const nextSession = createMockSession('session-b'); const deferred = createDeferred(); - const manualSessionClearRef = { current: false }; const createDetachedSession = vi.fn(() => deferred.promise); - const { actions, sessionRef, getConnection } = createActionsHarness({ - connection: { status: 'connected' }, - createDetachedSession, - manualSessionClearRef, - }); + const { actions, sessionRef, getConnection, manualSessionClearRef } = + createActionsHarness({ + connection: { status: 'connected' }, + createDetachedSession, + }); const createPromise = actions.createSession(); manualSessionClearRef.current = true; @@ -704,6 +704,45 @@ describe('createDaemonSessionActions', () => { expect(store.reset).not.toHaveBeenCalled(); }); + it('keeps the successor exempted when a same-session reload supersedes', async () => { + const existingSession = createMockSession('session-a'); + const existingClient = existingSession as unknown as DaemonSessionClient; + const controller = new AbortController(); + const { actions, attachmentLifecycle, pendingSessionLoadRef } = + createActionsHarness({ + connection: { status: 'connected', sessionId: 'session-a' }, + session: existingSession, + }); + + const first = actions.reloadSession(controller.signal); + const firstSettled = first.catch((error: unknown) => error); + expect(attachmentLifecycle.isCleanupDetachPreserved(existingClient)).toBe( + true, + ); + + // A second reload of the same session supersedes the first while it is + // still pending. Both switches capture the same session object, so the + // superseded switch's deferred release must not clear the exemption the + // successor re-preserved — otherwise the runner-effect cleanup detaches + // the session mid-reload. + const second = actions.reloadSession(controller.signal); + const secondSettled = second.catch((error: unknown) => error); + + await expect(firstSettled).resolves.toMatchObject({ name: 'AbortError' }); + expect(attachmentLifecycle.isCleanupDetachPreserved(existingClient)).toBe( + true, + ); + // A superseded load cannot detach: only the surviving reload may. + expect(existingSession.detach).not.toHaveBeenCalled(); + + attachmentLifecycle.resolvePendingLoad(pendingSessionLoadRef.current!); + await expect(secondSettled).resolves.toBeUndefined(); + expect(attachmentLifecycle.isCleanupDetachPreserved(existingClient)).toBe( + false, + ); + expect(existingSession.detach).toHaveBeenCalledOnce(); + }); + it('keeps the reload abort signal with the pending load', () => { const controller = new AbortController(); const clearLiveJournalRepair = vi.fn(); @@ -856,14 +895,13 @@ describe('createDaemonSessionActions', () => { vi.useFakeTimers(); try { const existingSession = createMockSession('session-a'); - const manualSessionClearRef = { current: false }; const setRestoreSessionId = vi.fn(); - const { actions, getConnection } = createActionsHarness({ - connection: { status: 'connected', sessionId: 'session-a' }, - manualSessionClearRef, - session: existingSession, - setRestoreSessionId, - }); + const { actions, getConnection, manualSessionClearRef } = + createActionsHarness({ + connection: { status: 'connected', sessionId: 'session-a' }, + session: existingSession, + setRestoreSessionId, + }); const loadPromise = actions.loadSession('session-b'); expect(getConnection()).toMatchObject({ @@ -1131,33 +1169,28 @@ describe('createDaemonSessionActions', () => { it('aborts active prompts and rejects pending session loads when clearing', async () => { const controller = new AbortController(); const session = createMockSession('session-a'); - const pendingReject = vi.fn(); - const pendingSessionLoadRef = { - current: { - id: 1, - sessionId: 'session-a', - mode: 'attach' as const, - timeout: setTimeout(() => undefined, 30_000), - resolve: vi.fn(), - reject: pendingReject, - }, - }; + const attachmentLifecycle = new SessionAttachmentLifecycle(); + const pendingLoad = attachmentLifecycle.startPendingLoad({ + sessionId: 'session-a', + mode: 'attach', + onTimeout: () => new Error('timed out'), + }); const { actions } = createActionsHarness({ activePrompts: new Map([['session-a', { controller } as ActivePrompt]]), - pendingSessionLoadRef, + attachmentLifecycle, session, }); await actions.clearSession(); expect(controller.signal.aborted).toBe(true); - expect(pendingReject).toHaveBeenCalledWith( + await expect(pendingLoad).rejects.toEqual( expect.objectContaining({ name: 'AbortError', message: 'Session cleared', }), ); - expect(pendingSessionLoadRef.current).toBeUndefined(); + expect(attachmentLifecycle.pendingLoad).toBeUndefined(); }); it('restarts the event stream after prompt admission', async () => { @@ -2608,11 +2641,10 @@ function createActionsHarness( opts: { activePrompts?: Map; addNotice?: ReturnType; + attachmentLifecycle?: SessionAttachmentLifecycle; clearLiveJournalRepair?: ReturnType; connection?: DaemonConnectionState; createDetachedSession?: ReturnType; - manualSessionClearRef?: { current: boolean }; - pendingSessionLoadRef?: { current: PendingSessionLoad | undefined }; restartEventStream?: ReturnType; session?: ReturnType; setAttachSessionNonce?: ReturnType; @@ -2633,11 +2665,29 @@ function createActionsHarness( const activePromptsRef = { current: opts.activePrompts ?? new Map(), }; - const pendingSessionLoadRef = - opts.pendingSessionLoadRef ?? - ({ current: undefined } as { - current: PendingSessionLoad | undefined; - }); + const attachmentLifecycle = + opts.attachmentLifecycle ?? new SessionAttachmentLifecycle(); + const pendingSessionLoadRef = { + get current(): PendingSessionLoad | undefined { + return attachmentLifecycle.pendingLoad; + }, + set current(value: PendingSessionLoad | undefined) { + if (value !== undefined) { + throw new Error('Tests may only release the current pending load'); + } + const pendingLoad = attachmentLifecycle.pendingLoad; + if (pendingLoad) attachmentLifecycle.releasePendingLoad(pendingLoad); + }, + }; + const manualSessionClearRef = { + get current(): boolean { + return attachmentLifecycle.isManuallyCleared; + }, + set current(value: boolean) { + if (value) attachmentLifecycle.markManuallyCleared(); + else attachmentLifecycle.allowAutomaticAttachment(); + }, + }; const store = { reset: vi.fn(), appendLocalUserMessage: vi.fn(), @@ -2648,12 +2698,9 @@ function createActionsHarness( sessionRef, activePromptsRef, settledPromptsRef: { current: new Map() }, - pendingSessionLoadRef, - pendingSessionLoadIdRef: { current: 0 }, + attachmentLifecycle, sessionConfigGeneration: new WeakMap(), heartbeatSupportedRef: { current: false }, - manualSessionClearRef: opts.manualSessionClearRef ?? { current: false }, - skipNextCleanupDetachSessionRef: { current: undefined }, passiveAssistantDoneTimerRef: { current: undefined }, getCreateSessionRequest: () => ({ workspaceCwd: '/workspace' }), createDetachedSession: (opts.createDetachedSession ?? @@ -2683,7 +2730,9 @@ function createActionsHarness( return { actions, activePromptsRef, + attachmentLifecycle, getConnection: () => connection, + manualSessionClearRef, pendingSessionLoadRef, replaceConnection, sessionRef, diff --git a/packages/webui/src/daemon/session/actions.ts b/packages/webui/src/daemon/session/actions.ts index 3ff439e353f..c2c2e8f4fca 100644 --- a/packages/webui/src/daemon/session/actions.ts +++ b/packages/webui/src/daemon/session/actions.ts @@ -64,6 +64,7 @@ import { getPersistedClientId, persistStableClientId, } from './clientLifecycle.js'; +import type { SessionAttachmentLifecycle } from './attachment-lifecycle.js'; import type { ActivePrompt, AddDaemonSessionNotice, @@ -161,10 +162,6 @@ export function resolveSessionRestoreTimeouts( }; } -function clearPendingLoadTimeout(load: PendingSessionLoad): void { - if (load.timeout !== undefined) clearTimeout(load.timeout); -} - export function normalizeWorkspaceIdentity(value: string | undefined): string { return value ? value.replace(/\\/g, '/').replace(/\/+$/, '') || '/' : ''; } @@ -174,12 +171,9 @@ export interface CreateDaemonSessionActionsArgs { sessionRef: RefBox; activePromptsRef: RefBox>; settledPromptsRef: RefBox>; - pendingSessionLoadRef: RefBox; - pendingSessionLoadIdRef: RefBox; + attachmentLifecycle: SessionAttachmentLifecycle; sessionConfigGeneration: WeakMap; heartbeatSupportedRef: RefBox; - manualSessionClearRef: RefBox; - skipNextCleanupDetachSessionRef: RefBox; passiveAssistantDoneTimerRef: TimerRef; getCreateSessionRequest: () => CreateSessionRequest; createDetachedSession: ( @@ -257,12 +251,9 @@ export function createDaemonSessionActions({ sessionRef, activePromptsRef, settledPromptsRef, - pendingSessionLoadRef, - pendingSessionLoadIdRef, + attachmentLifecycle, sessionConfigGeneration, heartbeatSupportedRef, - manualSessionClearRef, - skipNextCleanupDetachSessionRef, passiveAssistantDoneTimerRef, getCreateSessionRequest, createDetachedSession, @@ -461,19 +452,9 @@ export function createDaemonSessionActions({ settledPromptsRef.current.clear(); setPromptStatus('idle'); clearPassiveAssistantDoneTimer(passiveAssistantDoneTimerRef); - if (pendingSessionLoadRef.current) { - if ( - skipNextCleanupDetachSessionRef.current?.sessionId === - pendingSessionLoadRef.current.sessionId - ) { - skipNextCleanupDetachSessionRef.current = undefined; - } - clearPendingLoadTimeout(pendingSessionLoadRef.current); - pendingSessionLoadRef.current.reject( - new DOMException('Session cleared', 'AbortError'), - ); - pendingSessionLoadRef.current = undefined; - } + attachmentLifecycle.cancelPendingLoad( + new DOMException('Session cleared', 'AbortError'), + ); store.reset(); setRestoreSessionId(undefined); setRestoreWorkspaceCwd(undefined); @@ -485,74 +466,48 @@ export function createDaemonSessionActions({ signal?: AbortSignal, replaySource?: PendingSessionLoad['replaySource'], ): Promise { - const loadId = pendingSessionLoadIdRef.current + 1; - pendingSessionLoadIdRef.current = loadId; - if (pendingSessionLoadRef.current) { - clearPendingLoadTimeout(pendingSessionLoadRef.current); - pendingSessionLoadRef.current.reject( - new DOMException( - `Session ${mode} superseded by a newer request`, - 'AbortError', - ), - ); - } - const loadPromise = new Promise((resolve, reject) => { - const restoreTimeouts = resolveSessionRestoreTimeouts( - getConnection().capabilities, - ); - const watchdogTimeoutMs = + const restoreTimeouts = resolveSessionRestoreTimeouts( + getConnection().capabilities, + ); + return attachmentLifecycle.startPendingLoad({ + sessionId, + mode, + watchdogTimeoutMs: mode === 'attach' ? ATTACH_WATCHDOG_TIMEOUT_MS - : restoreTimeouts.watchdogTimeoutMs; - const timeout = - watchdogTimeoutMs === undefined - ? undefined - : setTimeout(() => { - if (pendingSessionLoadRef.current?.id === loadId) { - pendingSessionLoadRef.current = undefined; - if (sessionRef.current?.sessionId !== sessionId) { - manualSessionClearRef.current = true; - setRestoreSessionId(undefined); - setRestoreWorkspaceCwd(undefined); - setConnection((current) => { - if ( - current.status !== 'connecting' || - current.sessionId !== sessionId - ) { - return current; - } - return { - ...getConnectionAfterSessionClear(current, sessionId), - status: 'disconnected', - sessionId: undefined, - }; - }); - } - reject( - dispatchActionError( - addNotice, - `${capitalize(mode)} session failed`, - new Error(`Session ${mode} timed out`), - getSessionLoadNoticeOperation(mode), - ), - ); - } - }, watchdogTimeoutMs); - pendingSessionLoadRef.current = { - id: loadId, - sessionId, - mode, - timeout, - ...(mode !== 'attach' - ? { requestTimeoutMs: restoreTimeouts.requestTimeoutMs } - : {}), - resolve, - reject, - ...(signal ? { signal } : {}), - ...(replaySource ? { replaySource } : {}), - }; + : restoreTimeouts.watchdogTimeoutMs, + ...(mode !== 'attach' + ? { requestTimeoutMs: restoreTimeouts.requestTimeoutMs } + : {}), + ...(signal ? { signal } : {}), + ...(replaySource ? { replaySource } : {}), + onTimeout: () => { + if (sessionRef.current?.sessionId !== sessionId) { + attachmentLifecycle.markManuallyCleared(); + setRestoreSessionId(undefined); + setRestoreWorkspaceCwd(undefined); + setConnection((current) => { + if ( + current.status !== 'connecting' || + current.sessionId !== sessionId + ) { + return current; + } + return { + ...getConnectionAfterSessionClear(current, sessionId), + status: 'disconnected', + sessionId: undefined, + }; + }); + } + return dispatchActionError( + addNotice, + `${capitalize(mode)} session failed`, + new Error(`Session ${mode} timed out`), + getSessionLoadNoticeOperation(mode), + ); + }, }); - return loadPromise; } function startSessionSwitch( @@ -570,14 +525,14 @@ export function createDaemonSessionActions({ new DOMException('Session load cancelled', 'AbortError'), ); } - manualSessionClearRef.current = false; + attachmentLifecycle.allowAutomaticAttachment(); const loadPromise = startPendingSessionLoad( sessionId, mode, signal, replaySource, ); - const pendingLoad = pendingSessionLoadRef.current; + const pendingLoad = attachmentLifecycle.pendingLoad; const currentSession = sessionRef.current; const currentSessionId = currentSession?.sessionId; const activePrompt = currentSessionId @@ -610,13 +565,14 @@ export function createDaemonSessionActions({ ); }); if (reloadingCurrentSession) { - skipNextCleanupDetachSessionRef.current = currentSession; + attachmentLifecycle.preserveCleanupDetach(currentSession, pendingLoad); void loadPromise .then(detachCurrentSession, () => undefined) .finally(() => { - if (skipNextCleanupDetachSessionRef.current === currentSession) { - skipNextCleanupDetachSessionRef.current = undefined; - } + attachmentLifecycle.releaseCleanupDetachExemption( + currentSession, + pendingLoad, + ); }); } else { void detachCurrentSession(); @@ -656,8 +612,8 @@ export function createDaemonSessionActions({ // replaced the connecting state. if ( !isAbortError(error) && - (pendingSessionLoadRef.current === undefined || - pendingSessionLoadRef.current === pendingLoad) + (attachmentLifecycle.pendingLoad === undefined || + attachmentLifecycle.pendingLoad === pendingLoad) ) { setConnection((current) => ({ ...current, @@ -1327,7 +1283,7 @@ export function createDaemonSessionActions({ return request; }; try { - manualSessionClearRef.current = false; + attachmentLifecycle.allowAutomaticAttachment(); // Fold the initial approval mode into the create request so the daemon // applies it atomically at spawn (`POST /session` → // `spawnOrAttach({ approvalMode })`), avoiding a follow-up @@ -1380,7 +1336,7 @@ export function createDaemonSessionActions({ ), 'Create session timed out', ); - if (manualSessionClearRef.current) { + if (attachmentLifecycle.isManuallyCleared) { try { await withActionTimeout( nextSession.detach(), @@ -1396,7 +1352,7 @@ export function createDaemonSessionActions({ } persistStableClientId(nextSession.clientId, nextSession.sessionId); sessionRef.current = nextSession; - skipNextCleanupDetachSessionRef.current = nextSession; + attachmentLifecycle.preserveCleanupDetach(nextSession); setConnection((current) => ({ ...current, status: 'connected', @@ -1436,7 +1392,7 @@ export function createDaemonSessionActions({ async clearSession() { const session = sessionRef.current; - manualSessionClearRef.current = true; + attachmentLifecycle.markManuallyCleared(); clearActiveSessionState(); sessionRef.current = undefined; setConnection((current) => @@ -1452,7 +1408,7 @@ export function createDaemonSessionActions({ }, async newSession() { - manualSessionClearRef.current = false; + attachmentLifecycle.allowAutomaticAttachment(); clearActiveSessionState(); setConnection((current) => ({ ...current, @@ -2171,7 +2127,7 @@ export function createDaemonSessionActions({ 'branch_session', ); const sourceSessionId = session.sessionId; - const loadGeneration = pendingSessionLoadIdRef.current; + const loadGeneration = attachmentLifecycle.generation; branchInFlight = true; try { const branchRequest: Promise = @@ -2189,7 +2145,7 @@ export function createDaemonSessionActions({ const result = await branchRequest; const switchStarted = sessionRef.current === session && - pendingSessionLoadIdRef.current === loadGeneration; + attachmentLifecycle.generation === loadGeneration; const restored = atRecordId === undefined ? (result as DaemonBranchedSession) diff --git a/packages/webui/src/daemon/session/attachment-lifecycle.test.ts b/packages/webui/src/daemon/session/attachment-lifecycle.test.ts new file mode 100644 index 00000000000..8ce6d00cb27 --- /dev/null +++ b/packages/webui/src/daemon/session/attachment-lifecycle.test.ts @@ -0,0 +1,329 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; +import type { DaemonSessionClient } from '@qwen-code/sdk/daemon'; +import { SessionAttachmentLifecycle } from './attachment-lifecycle.js'; + +describe('SessionAttachmentLifecycle', () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it('rejects the previous load when a newer load supersedes it', async () => { + const lifecycle = new SessionAttachmentLifecycle(); + const first = lifecycle.startPendingLoad({ + sessionId: 'session-a', + mode: 'load', + onTimeout: () => new Error('timed out'), + }); + + const second = lifecycle.startPendingLoad({ + sessionId: 'session-b', + mode: 'resume', + onTimeout: () => new Error('timed out'), + }); + + await expect(first).rejects.toMatchObject({ name: 'AbortError' }); + expect(lifecycle.pendingLoad?.sessionId).toBe('session-b'); + lifecycle.resolvePendingLoad(lifecycle.pendingLoad!); + await expect(second).resolves.toBeUndefined(); + }); + + it('does not let a stale load settle the current load', async () => { + const lifecycle = new SessionAttachmentLifecycle(); + const first = lifecycle.startPendingLoad({ + sessionId: 'session-a', + mode: 'load', + onTimeout: () => new Error('timed out'), + }); + const staleLoad = lifecycle.pendingLoad!; + const second = lifecycle.startPendingLoad({ + sessionId: 'session-b', + mode: 'load', + onTimeout: () => new Error('timed out'), + }); + + await expect(first).rejects.toMatchObject({ name: 'AbortError' }); + expect(lifecycle.resolvePendingLoad(staleLoad)).toBe(false); + expect(lifecycle.pendingLoad?.sessionId).toBe('session-b'); + lifecycle.resolvePendingLoad(lifecycle.pendingLoad!); + await expect(second).resolves.toBeUndefined(); + }); + + it('advances the load generation on every startPendingLoad', async () => { + // branchSession's stale-switch guard samples `generation` before its + // await and compares it afterwards (actions.ts): the guard can only + // notice an in-flight reload if every startPendingLoad actually + // advances the counter. Two supersedes must therefore produce three + // distinct, increasing generations. + const lifecycle = new SessionAttachmentLifecycle(); + expect(lifecycle.generation).toBe(0); + + const first = lifecycle.startPendingLoad({ + sessionId: 'session-a', + mode: 'load', + onTimeout: () => new Error('timed out'), + }); + expect(lifecycle.generation).toBe(1); + expect(lifecycle.pendingLoad?.id).toBe(1); + + const second = lifecycle.startPendingLoad({ + sessionId: 'session-a', + mode: 'load', + onTimeout: () => new Error('timed out'), + }); + expect(lifecycle.generation).toBe(2); + expect(lifecycle.pendingLoad?.id).toBe(2); + + const third = lifecycle.startPendingLoad({ + sessionId: 'session-a', + mode: 'load', + onTimeout: () => new Error('timed out'), + }); + expect(lifecycle.generation).toBe(3); + expect(lifecycle.pendingLoad?.id).toBe(3); + + await expect(first).rejects.toMatchObject({ name: 'AbortError' }); + await expect(second).rejects.toMatchObject({ name: 'AbortError' }); + lifecycle.resolvePendingLoad(lifecycle.pendingLoad!); + await expect(third).resolves.toBeUndefined(); + }); + + it('releases the current load before running its timeout callback', async () => { + vi.useFakeTimers(); + const lifecycle = new SessionAttachmentLifecycle(); + const onTimeout = vi.fn(() => new Error('timed out')); + const pending = lifecycle.startPendingLoad({ + sessionId: 'session-a', + mode: 'attach', + watchdogTimeoutMs: 10, + onTimeout, + }); + const settled = pending.then( + () => ({ status: 'resolved' as const, error: undefined }), + (error: unknown) => ({ status: 'rejected' as const, error }), + ); + + await vi.advanceTimersByTimeAsync(10); + + expect(await settled).toEqual({ + status: 'rejected', + error: expect.objectContaining({ message: 'timed out' }), + }); + expect(onTimeout).toHaveBeenCalledOnce(); + expect(lifecycle.pendingLoad).toBeUndefined(); + }); + + it('owns the manual-clear and cleanup-detach states', () => { + const lifecycle = new SessionAttachmentLifecycle(); + const session = { + sessionId: 'session-a', + } as DaemonSessionClient; + + lifecycle.markManuallyCleared(); + lifecycle.preserveCleanupDetach(session); + + expect(lifecycle.isManuallyCleared).toBe(true); + expect(lifecycle.isCleanupDetachPreserved(session)).toBe(true); + + lifecycle.allowAutomaticAttachment(); + lifecycle.releaseCleanupDetachExemption(session); + + expect(lifecycle.isManuallyCleared).toBe(false); + expect(lifecycle.isCleanupDetachPreserved(session)).toBe(false); + }); + + it('releases the cleanup-detach exemption when a pending load is rejected', async () => { + const lifecycle = new SessionAttachmentLifecycle(); + const session = { + sessionId: 'session-a', + } as DaemonSessionClient; + const pending = lifecycle.startPendingLoad({ + sessionId: 'session-a', + mode: 'load', + onTimeout: () => new Error('timed out'), + }); + lifecycle.preserveCleanupDetach(session); + expect(lifecycle.isCleanupDetachPreserved(session)).toBe(true); + + expect( + lifecycle.rejectPendingLoad( + lifecycle.pendingLoad!, + new Error('load failed'), + ), + ).toBe(true); + + await expect(pending).rejects.toThrow('load failed'); + // A failed load must not keep the session exempted: the next + // effect-cleanup pass has to perform the prompt-state reset/detach. + expect(lifecycle.isCleanupDetachPreserved(session)).toBe(false); + }); + + it('releases the cleanup-detach exemption when a pending load is cancelled', async () => { + const lifecycle = new SessionAttachmentLifecycle(); + const session = { + sessionId: 'session-a', + } as DaemonSessionClient; + const pending = lifecycle.startPendingLoad({ + sessionId: 'session-a', + mode: 'resume', + onTimeout: () => new Error('timed out'), + }); + lifecycle.preserveCleanupDetach(session); + + expect(lifecycle.cancelPendingLoad(new Error('cancelled'))).toBe(true); + + await expect(pending).rejects.toThrow('cancelled'); + expect(lifecycle.isCleanupDetachPreserved(session)).toBe(false); + }); + + it("keeps another session's exemption when rejecting a load", async () => { + const lifecycle = new SessionAttachmentLifecycle(); + const otherSession = { + sessionId: 'session-b', + } as DaemonSessionClient; + const pending = lifecycle.startPendingLoad({ + sessionId: 'session-a', + mode: 'load', + onTimeout: () => new Error('timed out'), + }); + lifecycle.preserveCleanupDetach(otherSession); + + expect( + lifecycle.rejectPendingLoad( + lifecycle.pendingLoad!, + new Error('load failed'), + ), + ).toBe(true); + + await expect(pending).rejects.toThrow('load failed'); + expect(lifecycle.isCleanupDetachPreserved(otherSession)).toBe(true); + }); + + it("keeps the successor's exemption when a stale load is rejected", async () => { + const lifecycle = new SessionAttachmentLifecycle(); + const session = { + sessionId: 'session-a', + } as DaemonSessionClient; + const stale = lifecycle.startPendingLoad({ + sessionId: 'session-a', + mode: 'load', + onTimeout: () => new Error('timed out'), + }); + const staleLoad = lifecycle.pendingLoad!; + // A same-session reload supersedes the failed load; the second switch + // re-preserves the cleanup-detach exemption for the successor. + const successor = lifecycle.startPendingLoad({ + sessionId: 'session-a', + mode: 'load', + onTimeout: () => new Error('timed out'), + }); + await expect(stale).rejects.toMatchObject({ name: 'AbortError' }); + lifecycle.preserveCleanupDetach(session); + expect(lifecycle.isCleanupDetachPreserved(session)).toBe(true); + + // The late reject of the superseded load must return false and must NOT + // release the exemption: releasePendingLoad fails on the stale load + // before any release runs, so the successor keeps its exemption. + expect(lifecycle.rejectPendingLoad(staleLoad, new Error('late'))).toBe( + false, + ); + expect(lifecycle.isCleanupDetachPreserved(session)).toBe(true); + + let successorSettled = false; + void successor.then( + () => { + successorSettled = true; + }, + () => { + successorSettled = true; + }, + ); + await Promise.resolve(); + expect(successorSettled).toBe(false); + + lifecycle.resolvePendingLoad(lifecycle.pendingLoad!); + await expect(successor).resolves.toBeUndefined(); + }); + + it("releases a superseded load's exemption while keeping the successor's", async () => { + const lifecycle = new SessionAttachmentLifecycle(); + const supersededSession = { + sessionId: 'session-a', + } as DaemonSessionClient; + const successorSession = { + sessionId: 'session-b', + } as DaemonSessionClient; + + const first = lifecycle.startPendingLoad({ + sessionId: 'session-a', + mode: 'load', + onTimeout: () => new Error('timed out'), + }); + lifecycle.preserveCleanupDetach(supersededSession); + expect(lifecycle.isCleanupDetachPreserved(supersededSession)).toBe(true); + + const second = lifecycle.startPendingLoad({ + sessionId: 'session-b', + mode: 'load', + onTimeout: () => new Error('timed out'), + }); + // The supersede runs cancelPendingLoad → rejectPendingLoad → + // releaseCleanupDetachExemptionForSession('session-a') synchronously + // inside startPendingLoad, so the superseded load's exemption must + // already be gone by the time the successor re-preserves its own. + lifecycle.preserveCleanupDetach(successorSession); + + await expect(first).rejects.toMatchObject({ name: 'AbortError' }); + expect(lifecycle.isCleanupDetachPreserved(supersededSession)).toBe(false); + expect(lifecycle.isCleanupDetachPreserved(successorSession)).toBe(true); + + lifecycle.resolvePendingLoad(lifecycle.pendingLoad!); + await expect(second).resolves.toBeUndefined(); + }); + + it("keeps the successor's exemption when a superseded same-session release runs late", async () => { + const lifecycle = new SessionAttachmentLifecycle(); + const session = { + sessionId: 'session-a', + } as DaemonSessionClient; + + const first = lifecycle.startPendingLoad({ + sessionId: 'session-a', + mode: 'load', + onTimeout: () => new Error('timed out'), + }); + const firstLoad = lifecycle.pendingLoad!; + lifecycle.preserveCleanupDetach(session, firstLoad); + + // A same-session reload supersedes the first load: the supersede clears + // the exemption synchronously, then the second switch re-preserves it for + // its own load. + const second = lifecycle.startPendingLoad({ + sessionId: 'session-a', + mode: 'load', + onTimeout: () => new Error('timed out'), + }); + const secondLoad = lifecycle.pendingLoad!; + await expect(first).rejects.toMatchObject({ name: 'AbortError' }); + lifecycle.preserveCleanupDetach(session, secondLoad); + expect(lifecycle.isCleanupDetachPreserved(session)).toBe(true); + + // The superseded switch's deferred release arrives after the successor + // re-preserved. Both preserved the same session object, so only the + // recorded load distinguishes the two exemptions. + lifecycle.releaseCleanupDetachExemption(session, firstLoad); + expect(lifecycle.isCleanupDetachPreserved(session)).toBe(true); + + // The successor's own release clears the exemption once it settles. + lifecycle.releaseCleanupDetachExemption(session, secondLoad); + expect(lifecycle.isCleanupDetachPreserved(session)).toBe(false); + + lifecycle.resolvePendingLoad(secondLoad); + await expect(second).resolves.toBeUndefined(); + }); +}); diff --git a/packages/webui/src/daemon/session/attachment-lifecycle.ts b/packages/webui/src/daemon/session/attachment-lifecycle.ts new file mode 100644 index 00000000000..01a8f368dd6 --- /dev/null +++ b/packages/webui/src/daemon/session/attachment-lifecycle.ts @@ -0,0 +1,160 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { DaemonSessionClient } from '@qwen-code/sdk/daemon'; +import type { PendingSessionLoad } from './types.js'; + +interface StartPendingSessionLoad { + sessionId: string; + mode: PendingSessionLoad['mode']; + watchdogTimeoutMs?: number; + requestTimeoutMs?: number; + signal?: AbortSignal; + replaySource?: PendingSessionLoad['replaySource']; + onTimeout: () => unknown; +} + +export class SessionAttachmentLifecycle { + private loadGeneration = 0; + private currentLoad: PendingSessionLoad | undefined; + private manuallyCleared = false; + private cleanupDetachExemption: + | { session: DaemonSessionClient; load: PendingSessionLoad | undefined } + | undefined; + + get pendingLoad(): PendingSessionLoad | undefined { + return this.currentLoad; + } + + get generation(): number { + return this.loadGeneration; + } + + get isManuallyCleared(): boolean { + return this.manuallyCleared; + } + + startPendingLoad({ + sessionId, + mode, + watchdogTimeoutMs, + requestTimeoutMs, + signal, + replaySource, + onTimeout, + }: StartPendingSessionLoad): Promise { + const id = ++this.loadGeneration; + this.cancelPendingLoad( + new DOMException( + `Session ${mode} superseded by a newer request`, + 'AbortError', + ), + ); + + return new Promise((resolve, reject) => { + const load: PendingSessionLoad = { + id, + sessionId, + mode, + resolve, + reject, + ...(requestTimeoutMs !== undefined ? { requestTimeoutMs } : {}), + ...(signal ? { signal } : {}), + ...(replaySource ? { replaySource } : {}), + }; + if (watchdogTimeoutMs !== undefined) { + load.timeout = setTimeout(() => { + if (!this.releasePendingLoad(load)) return; + let timeoutError: unknown; + try { + timeoutError = onTimeout(); + } catch (error) { + timeoutError = error; + } + reject(timeoutError); + }, watchdogTimeoutMs); + } + this.currentLoad = load; + }); + } + + isCurrentLoad(load: PendingSessionLoad | undefined): boolean { + return load !== undefined && this.currentLoad === load; + } + + resolvePendingLoad(load: PendingSessionLoad): boolean { + if (!this.releasePendingLoad(load)) return false; + load.resolve(); + return true; + } + + rejectPendingLoad(load: PendingSessionLoad, error: unknown): boolean { + if (!this.releasePendingLoad(load)) return false; + this.releaseCleanupDetachExemptionForSession(load.sessionId); + load.reject(error); + return true; + } + + cancelPendingLoad(error: unknown): boolean { + const load = this.currentLoad; + return load ? this.rejectPendingLoad(load, error) : false; + } + + releasePendingLoad(load: PendingSessionLoad): boolean { + if (this.currentLoad !== load) return false; + this.currentLoad = undefined; + if (load.timeout !== undefined) clearTimeout(load.timeout); + return true; + } + + markManuallyCleared(): void { + this.manuallyCleared = true; + } + + allowAutomaticAttachment(): void { + this.manuallyCleared = false; + } + + preserveCleanupDetach( + session: DaemonSessionClient, + load?: PendingSessionLoad, + ): void { + this.cleanupDetachExemption = { session, load }; + } + + isCleanupDetachPreserved(session: DaemonSessionClient | undefined): boolean { + return ( + session !== undefined && this.cleanupDetachExemption?.session === session + ); + } + + releaseCleanupDetachExemption( + session?: DaemonSessionClient | undefined, + load?: PendingSessionLoad, + ): void { + const exemption = this.cleanupDetachExemption; + if (exemption === undefined) return; + if (session !== undefined && exemption.session !== session) return; + // Two overlapping reloads of the same session share the session object, + // so identity alone cannot tell a switch's own exemption from the one a + // superseding switch re-preserved. A release tied to a superseded load + // must not clear the successor's exemption. + if ( + load !== undefined && + exemption.load !== undefined && + exemption.load !== load + ) { + return; + } + this.cleanupDetachExemption = undefined; + } + + releaseCleanupDetachExemptionForSession(sessionId: string): void { + if (this.cleanupDetachExemption?.session.sessionId === sessionId) { + this.cleanupDetachExemption = undefined; + } + } +}