diff --git a/.qwen/e2e-tests/2026-08-12-daemon-acp-http-pre-attach-bounds.md b/.qwen/e2e-tests/2026-08-12-daemon-acp-http-pre-attach-bounds.md new file mode 100644 index 00000000000..15358de58d1 --- /dev/null +++ b/.qwen/e2e-tests/2026-08-12-daemon-acp-http-pre-attach-bounds.md @@ -0,0 +1,31 @@ +# Daemon ACP HTTP pre-attach bounds + +## Scope + +Verify that connection/session responses produced before an ACP HTTP SSE or WebSocket owner is ready are bounded by serialized bytes and frame count across every workspace mount. The test does not claim to bound ordinary live transport queues or transient `JSON.stringify` amplification. + +## Baseline + +Run the harness against the parent of this change. Initialize one ACP HTTP connection without attaching its response stream, then make the fake bridge produce 128 distinct 1 MiB results. Confirm retained heap/RSS grows with every payload and that the connection remains registered. Repeat with primary and dynamic workspace connections to confirm their retained buffers add together without a daemon-global boundary. + +## Verification + +1. Start `qwen serve` with ACP HTTP enabled, one primary workspace, and one dynamically registered trusted workspace. +2. For each workspace, initialize a logical connection but delay its connection/session stream attachment. +3. Produce distinct large responses until the per-connection 64 MiB boundary is crossed. Expect only the admitting connection to close; a shared WebSocket must receive close code 1013. Confirm the other workspace can still initialize, open a stream, and complete a small request. +4. With several connections below their individual limits, compete for the shared 4,096-frame/256-MiB budget. Expect the connection attempting the global N+1 admission to close without evicting frames from another connection. +5. Attach a deliberately stalled SSE writer after frames are buffered. Confirm status moves the frames from buffered to pending delivery while `usedFrames` and `usedBytes` remain charged. Close the socket, settle the write, and confirm all counters return to the pre-test baseline. +6. Buffer several successful `session/new`, `session/load`, `session/resume`, or `session/fork` results, then close or overflow the connection before delivery. Confirm fresh sessions and persisted forks are removed, newly attached clients are detached, existing ownership remains intact, and none of the provisional sessions accept a prompt before response delivery. +7. Send notification forms of `session/new`, `session/load`, `session/resume`, and `session/fork`. Confirm no session is created, restored, attached, or forked. +8. Read `GET /daemon/status?detail=full`. Verify fixed limits, global current/high-water count and bytes, pending-delivery frames, guard failures, per-mount failure attribution, and per-connection owned count/bytes. +9. Remove the dynamic workspace and close all test connections. Confirm global budget usage returns to the primary baseline. + +## Commands + +```bash +(cd packages/acp-bridge && npx vitest run src/bridge.test.ts src/spawnChannel.test.ts) +(cd packages/cli && npx vitest run src/serve/acp-http/pre-attach-budget.test.ts src/serve/acp-http/connection-registry.test.ts src/serve/acp-http/sse-stream.test.ts src/serve/acp-http/ws-stream.test.ts src/serve/acp-http/transport.test.ts src/serve/daemon-status.test.ts) +(cd packages/sdk-typescript && npx vitest run test/unit/daemon-public-surface.test.ts) +npm run build && npm run typecheck && npm run lint +git diff --check +``` diff --git a/docs/design/2026-07-31-daemon-capacity-model-and-memory-bounds.md b/docs/design/2026-07-31-daemon-capacity-model-and-memory-bounds.md index 1d3017f2317..17691f31c5b 100644 --- a/docs/design/2026-07-31-daemon-capacity-model-and-memory-bounds.md +++ b/docs/design/2026-07-31-daemon-capacity-model-and-memory-bounds.md @@ -153,7 +153,9 @@ The outbound check happens after `JSON.stringify` and UTF-8 encoding. It prevent **Workspace-supplied config files are read without a size gate.** `fs.readFileSync(path, 'utf-8')` on workspace `.qwen/settings.json` (`packages/cli/src/config/settings.ts:557,733`), trusted folders, the serve fast path (synchronous, so it also blocks the event loop), and every discovered `QWEN.md`, twenty concurrently (`packages/core/src/utils/memoryDiscovery.ts:225,245`). Registering a workspace containing a two-gigabyte `settings.json` exhausts the daemon with no session, no prompt, and no agent — the cheapest attack in the set, and the one furthest from anything a heap ledger would notice. -Recorded and deferred with evidence: SSE and WebSocket write chains respect backpressure but do not bound queued bytes (`acp-http/sse-stream.ts:110-128`, `ws-stream.ts:58-82`); ACP pre-attach frame buffers mirror the EventBus's `maxQueued` but not its `maxQueuedBytes` (`connection-registry.ts:18,30`); the organized session list materializes 50,000 summaries; several per-workspace caches outlive their workspace. +**ACP HTTP pre-attach buffers are the next bounded-container increment.** Connection and session replies are serialized once at production time and retained only as UTF-8 `Buffer`s. Each stream owns at most 256 buffered frames, each logical connection owns at most 1,024 frames and 64 MiB, and one process-global budget shared by primary and dynamic workspace registries owns at most 4,096 frames and 256 MiB. Attach transfers a lease to pending delivery; it is released only after the SSE write chain or WebSocket send callback settles. Count or byte overflow does not evict an older frame: it retires the exact session, or the whole logical connection when the queue is connection-scoped or shares a WebSocket. Fresh and newly attached session ownership remains provisional until the granting response is locally delivered, so teardown or overflow can roll back every definitively undelivered grant without exposing a session the client never learned it owned. If SSE accepts a complete ownership-granting frame but closes before its final write callback, the outcome is unknown and the daemon preserves the session rather than deleting it: a live logical connection conservatively commits ownership, while connection teardown detaches the client but leaves persisted state available for resume. Server response serialization failures are contained to the offending frame instead of being classified as resource exhaustion for the whole connection. Existing live SSE and WebSocket frames, and transient single-frame serialization amplification, remain separate container work. + +Recorded and deferred with evidence: live SSE and WebSocket write chains respect backpressure but do not bound queued bytes (`acp-http/sse-stream.ts`, `ws-stream.ts`); the organized session list materializes 50,000 summaries; several per-workspace caches outlive their workspace. ### Part 4 — Small aggregate quotas where multiplicity matters @@ -189,6 +191,8 @@ The compatibility discussion that belongs here is for the child-capacity policy Workspace registration, persisted restoration, and `POST /workspaces` are unchanged. The daemon-owned ACP transport now refuses a complete frame above 64 MiB; a decoded queue, active-handler set, pre-SDK outbound operation set, outstanding request set, or prepared-response set above its 256-message/64-MiB charge; an incomplete or clean protocol EOF while the child is still owned; string request ids above 256 bytes; response ids that do not match an admitted outstanding request; method or error-message scalars above 1 KiB; and JSON structures above the documented depth/node/array limits. Parse, envelope, and known-method schema violations are also transport-fatal after metadata-only logging, so every refusal retires only that workspace channel generation instead of leaving an SDK request pending or an SDK write queue growing. Standalone and public `ndJsonStream`/bridge callers remain opt-in and keep their previous transport and error-wire behavior when no limits or transport guard are supplied. +ACP HTTP pre-attach queues no longer silently evict their oldest frame. The 257th frame on one stream, or a connection/global count or byte refusal, closes the exact owner; a shared WebSocket closes with code 1013. Buffered frames are serialized at production time, so later mutation of the source object no longer changes the wire result. `session/new`, `session/load`, `session/resume`, and `session/fork` notifications no longer mutate state, and request-form ownership is usable only after its response is locally delivered. Clients observe an overload through the SSE/WS close because a full queue cannot safely enqueue its own error response. Public standalone ACP behavior and the workspace/session count defaults are unchanged. + `maxSessions` and `maxTotalSessions` keep their current defaults and derivation, and this change gives them no new bound. An earlier draft claimed `maxTotalSessions` was transitively bounded because `workspaceCount` would be capped by the budget; that is false against this PR, where the workspace cap remains the fixed `MAX_REGISTERED_WORKSPACES = 25` and nothing derives a limit from the budget at all. Sessions still multiplex onto one child per workspace, so per-session memory sits inside a child heap that nothing currently bounds beyond V8's own ceiling. The documentation for `maxSessions` should be read as a fairness and file-descriptor lever, not a memory one. `limits.memory` and `runtime.memory` on `GET /daemon/status` are additive and optional in the SDK mirror, so older daemons parse against newer clients. diff --git a/docs/design/daemon-acp-http/README.md b/docs/design/daemon-acp-http/README.md index 5ff7735f9eb..14fbd7b3133 100644 --- a/docs/design/daemon-acp-http/README.md +++ b/docs/design/daemon-acp-http/README.md @@ -392,7 +392,7 @@ All fixes verified by the expanded vitest suite (**18 tests**) + a fresh live sm | R3 | **P1** | **No connection→session ownership**: any authenticated connection could open the session SSE for, or prompt, _any_ sessionId in the workspace (read-eavesdrop; prompt was only blocked incidentally by the unregistered-clientId error). | `AcpConnection.ownedSessions` populated by `session/new`/`load`/`resume`; session stream returns `403` and per-session POSTs return `INVALID_PARAMS` for unowned ids (`requireOwned`). | | R4 | **P1** | `mountAcpHttp` handle was discarded → TTL sweep timer + live SSE streams leaked on shutdown. | Handle parked on `app.locals`; `runQwenServe` close hook calls `dispose()` before `bridge.shutdown()` (mirrors the device-flow registry). | | R5 | **P1** | **Pending permission leak**: closing a session/connection with a permission outstanding left the bridge blocked awaiting a vote. | `closeSessionStream`/`destroy` cancel matching pending requests via an injected `onAbandonPending` → `cancelAbandonedPermission`. | -| R6 | **P1** | Pre-attach frame buffers (`connBuffer`/`binding.buffer`) were unbounded. | Capped at 256 frames (drop-oldest), matching the EventBus `maxQueued`. | +| R6 | **P1** | Pre-attach frame buffers (`connBuffer`/`binding.buffer`) were unbounded. | Initially capped at 256 frames; current behavior also enforces connection/global count and byte budgets and closes the exact owner instead of silently dropping an older frame. | | R7 | **P2** | `initialize` ignored the client's requested `protocolVersion`. | Negotiates `min(requested, 1)`. | | R8 | **P2** | No `Acp-Session-Id` ↔ `params.sessionId` cross-check (RFD §2.3). | POST asserts they agree; mismatch → `INVALID_PARAMS`. | | R9 | **P2** | `session/cancel` request-form (with id) never answered; duplicate top-level `_meta.qwen`. | Reply when an id is present; single `agentCapabilities._meta.qwen`. | diff --git a/docs/design/daemon-acp-http/sse-resumable-stream.md b/docs/design/daemon-acp-http/sse-resumable-stream.md index 5492051bfac..bdfbed617e3 100644 --- a/docs/design/daemon-acp-http/sse-resumable-stream.md +++ b/docs/design/daemon-acp-http/sse-resumable-stream.md @@ -99,10 +99,11 @@ the monotonic sequence the client resumes from. WebSocket is a stateful connection, no SSE replay (consistent with `AcpWsTransport.supportsReplay = false`). 4. **`connection-registry.ts`** — `sendSession(sessionId, frame, id?)` - threads `id` to `stream.send`. The per-session pre-attach **buffer** - stores `{ frame, id? }` pairs so a buffered frame keeps its cursor when - flushed on attach. (The connection-scoped buffer is unchanged — those - frames are JSON-RPC responses with no bus id.) + threads `id` to the transport. The per-session pre-attach **buffer** + stores one serialized UTF-8 payload with its optional cursor and budget + lease, so a buffered frame keeps its cursor without retaining the source + object or serializing it again on attach. Connection-scoped replies use the + same representation. 5. **`dispatch.ts`** - `translateEvent` passes `event.id` through every `sendSession` / `binding.stream.send` call for bus events. @@ -183,6 +184,21 @@ operator logging can't drift. ## Backward compatibility +Pre-attach queues are bounded by both count and serialized payload bytes. One +stream owns at most 256 frames, one logical connection at most 1,024 frames and +64 MiB, and all ACP HTTP mounts share a process-global 4,096-frame/256-MiB +budget. A fresh attach transfers the lease to the transport writer and releases +it only after local delivery or definitive failure. If SSE accepts a complete +frame but closes before its final write callback, delivery is outcome-unknown; +an ownership-granting response preserves the session rather than deleting it. +If the logical connection is still live, ownership is conservatively +committed; during connection teardown, the client is detached while persisted +session data remains available for resume. Resume still discards +id-bearing buffered events in favor of authoritative ring replay and preserves +id-less reply ordering, but that discard now releases the retained byte lease. +Overflow closes the exact session; connection-scoped or shared-WebSocket +overflow closes the logical connection instead of evicting an older frame. + - **Old clients that don't send `Last-Event-ID`** → `lastEventId` is `undefined` → `subscribeEvents` starts live, exactly as today. - **Adding `id:` lines is backward-compatible SSE** — a client that ignores diff --git a/packages/acp-bridge/src/json-string-bytes.test.ts b/packages/acp-bridge/src/json-string-bytes.test.ts new file mode 100644 index 00000000000..0936d36ce20 --- /dev/null +++ b/packages/acp-bridge/src/json-string-bytes.test.ts @@ -0,0 +1,71 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it, vi } from 'vitest'; +import { estimateJsonStringBytes } from './json-string-bytes.js'; + +describe('estimateJsonStringBytes', () => { + it('matches JSON.stringify UTF-8 bytes for every UTF-16 code unit', () => { + for (let code = 0; code <= 0xffff; code++) { + const value = String.fromCharCode(code); + expect(estimateJsonStringBytes(value, Number.MAX_SAFE_INTEGER)).toBe( + Buffer.byteLength(JSON.stringify(value)), + ); + } + }); + + it('matches JSON.stringify for paired surrogates and mixed escaping', () => { + const samples = [ + '"\\\b\f\n\r\t', + '\u0000\u001f', + '\ud800', + '\udc00', + '\ud83d\ude00', + 'ASCII é 中 \ud83d\ude00 \ud800', + ]; + for (const value of samples) { + expect(estimateJsonStringBytes(value, Number.MAX_SAFE_INTEGER)).toBe( + Buffer.byteLength(JSON.stringify(value)), + ); + } + }); + + it('matches JSON.stringify for deterministic random strings', () => { + let state = 0x5eed1234; + const nextCodeUnit = () => { + state = (Math.imul(state, 1664525) + 1013904223) >>> 0; + return state & 0xffff; + }; + for (let sample = 0; sample < 1000; sample++) { + const length = nextCodeUnit() % 128; + let value = ''; + for (let index = 0; index < length; index++) { + value += String.fromCharCode(nextCodeUnit()); + } + expect(estimateJsonStringBytes(value, Number.MAX_SAFE_INTEGER)).toBe( + Buffer.byteLength(JSON.stringify(value)), + ); + } + }); + + it('returns limit + 1 as soon as the encoded string exceeds the limit', () => { + expect(estimateJsonStringBytes('\u0001'.repeat(100), 20)).toBe(21); + }); + + it('uses native byte counting for large strings that need no escaping', () => { + const charCodeAt = vi.spyOn(String.prototype, 'charCodeAt'); + try { + const value = 'x'.repeat(8 * 1024 * 1024); + expect(estimateJsonStringBytes(value, 1024)).toBe(1025); + expect(estimateJsonStringBytes(value, Number.MAX_SAFE_INTEGER)).toBe( + value.length + 2, + ); + expect(charCodeAt).not.toHaveBeenCalled(); + } finally { + charCodeAt.mockRestore(); + } + }); +}); diff --git a/packages/acp-bridge/src/json-string-bytes.ts b/packages/acp-bridge/src/json-string-bytes.ts new file mode 100644 index 00000000000..a7a577a8988 --- /dev/null +++ b/packages/acp-bridge/src/json-string-bytes.ts @@ -0,0 +1,50 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +export function estimateJsonStringBytes( + value: string, + limitBytes: number, +): number { + const unescapedBytes = Buffer.byteLength(value, 'utf8') + 2; + if (unescapedBytes > limitBytes) return limitBytes + 1; + if (!/["\\]|[^ -\ud7ff\ue000-\uffff]/u.test(value)) { + return unescapedBytes; + } + let bytes = 2; + for (let index = 0; index < value.length; index++) { + const code = value.charCodeAt(index); + if (code === 0x22 || code === 0x5c) { + bytes += 2; + } else if (code <= 0x1f) { + bytes += + code === 0x08 || + code === 0x09 || + code === 0x0a || + code === 0x0c || + code === 0x0d + ? 2 + : 6; + } else if (code >= 0xd800 && code <= 0xdbff) { + const next = value.charCodeAt(index + 1); + if (next >= 0xdc00 && next <= 0xdfff) { + bytes += 4; + index++; + } else { + bytes += 6; + } + } else if (code >= 0xdc00 && code <= 0xdfff) { + bytes += 6; + } else if (code <= 0x7f) { + bytes++; + } else if (code <= 0x7ff) { + bytes += 2; + } else { + bytes += 3; + } + if (bytes > limitBytes) return limitBytes + 1; + } + return bytes; +} diff --git a/packages/acp-bridge/src/spawnChannel.test.ts b/packages/acp-bridge/src/spawnChannel.test.ts index ca3fc3686df..1c82addbec7 100644 --- a/packages/acp-bridge/src/spawnChannel.test.ts +++ b/packages/acp-bridge/src/spawnChannel.test.ts @@ -416,6 +416,28 @@ describe('createSpawnChannelFactory env policy', () => { }); }); + it('charges JSON string escaping before admitting prepared responses', async () => { + const child = createFakeChildProcess(); + mockSpawn.mockReturnValue(child); + const channel = await createSpawnChannelFactory({ + pipeLimits: { + maxFrameBytes: 64_000, + maxQueuedMessages: 2, + maxQueuedBytes: 6_000, + }, + })('/tmp/project'); + const response = { + content: '\u0001'.repeat(700), + }; + + expect(() => + channel.transportGuard?.reservePreparedResponse(response), + ).toThrow('NDJSON decoded queue is full'); + await expect(channel.transportFailed).resolves.toMatchObject({ + code: 'ndjson_queue_limit_exceeded', + }); + }); + it('keeps the default factory unbounded and validates opt-in limits early', () => { expect(DAEMON_ACP_NDJSON_LIMITS).toEqual({ maxFrameBytes: 64 * 1024 * 1024, diff --git a/packages/acp-bridge/src/spawnChannel.ts b/packages/acp-bridge/src/spawnChannel.ts index 9dffacdfda6..c9579bf9e29 100644 --- a/packages/acp-bridge/src/spawnChannel.ts +++ b/packages/acp-bridge/src/spawnChannel.ts @@ -37,6 +37,7 @@ 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 = @@ -253,7 +254,10 @@ function estimatePreparedResponseBytes(value: unknown, limitBytes: number) { if (!descriptor || descriptor.get || descriptor.set) { return limitBytes + 1; } - bytes += (frame.first ? 0 : 1) + Buffer.byteLength(next.value) + 3; + 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 }); @@ -265,7 +269,10 @@ function estimatePreparedResponseBytes(value: unknown, limitBytes: number) { } else if (current === undefined) { bytes += 4; } else if (typeof current === 'string') { - bytes += Buffer.byteLength(current) + 2; + bytes += estimateJsonStringBytes( + current, + Math.max(0, limitBytes - bytes), + ); } else if (typeof current === 'number') { bytes += 24; } else if (typeof current === 'boolean') { diff --git a/packages/cli/src/serve/acp-http/connection-registry.test.ts b/packages/cli/src/serve/acp-http/connection-registry.test.ts index f0f73fd6fac..e424572071e 100644 --- a/packages/cli/src/serve/acp-http/connection-registry.test.ts +++ b/packages/cli/src/serve/acp-http/connection-registry.test.ts @@ -5,8 +5,11 @@ */ import { describe, expect, it, vi } from 'vitest'; +import { EventEmitter } from 'node:events'; import { ConnectionRegistry } from './connection-registry.js'; -import type { TransportStream } from './transport-stream.js'; +import { AcpPreAttachBudget } from './pre-attach-budget.js'; +import type { DeliveryResult, TransportStream } from './transport-stream.js'; +import { WsStream } from './ws-stream.js'; class FakeStream implements TransportStream { isClosed = false; @@ -19,11 +22,82 @@ class FakeStream implements TransportStream { this.sent.push({ message, id }); } + async sendSerialized(payload: Buffer, id?: number): Promise { + this.sent.push({ message: JSON.parse(payload.toString('utf8')), id }); + return this.isClosed ? 'closed' : 'delivered'; + } + + close(): void { + this.isClosed = true; + } +} + +class ControlledStream implements TransportStream { + isClosed = false; + private settle: ((result: DeliveryResult) => void) | undefined; + + constructor(readonly kind: 'sse' | 'ws' = 'sse') {} + + async send(): Promise {} + + sendSerialized(): Promise { + return new Promise((resolve) => { + this.settle = resolve; + }); + } + + complete(result: DeliveryResult): void { + this.settle?.(result); + } + close(): void { this.isClosed = true; } } +class PendingSessionStream implements TransportStream { + isClosed = false; + readonly sent: Array<{ message: unknown; id?: number }> = []; + private readonly settles: Array<(result: DeliveryResult) => void> = []; + + readonly kind = 'sse' as const; + + async send(): Promise {} + + sendSerialized(payload: Buffer, id?: number): Promise { + this.sent.push({ message: JSON.parse(payload.toString('utf8')), id }); + return new Promise((resolve) => this.settles.push(resolve)); + } + + completeAll(result: DeliveryResult): void { + for (const settle of this.settles.splice(0)) settle(result); + } + + close(): void { + this.isClosed = true; + } +} + +class PendingWebSocket extends EventEmitter { + readonly OPEN = 1; + readyState = this.OPEN; + callback: ((err?: Error) => void) | undefined; + + send(_data: unknown, ...args: unknown[]): void { + const callback = args.at(-1); + this.callback = + typeof callback === 'function' + ? (callback as (err?: Error) => void) + : undefined; + } + + ping(): void {} + + close(): void { + this.readyState = 3; + } +} + describe('ConnectionRegistry.getSnapshot', () => { it('counts SSE streams and redacts full connection ids', () => { const registry = new ConnectionRegistry(undefined, undefined, 2); @@ -133,6 +207,237 @@ describe('ConnectionRegistry.getSnapshot', () => { } }); + it('keeps session frames buffered when only the connection SSE stream is live', async () => { + const budget = new AcpPreAttachBudget({ maxFrames: 4, maxBytes: 1024 }); + const registry = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + budget, + ); + try { + const conn = registry.create(true); + if (!conn) return; + const connectionStream = new FakeStream('sse'); + conn.attachConnStream(connectionStream); + conn.ownSession('sess-1'); + conn.getOrCreateSession('sess-1'); + + const eventDelivery = conn.sendSession('sess-1', { event: true }, 7); + const replyDelivery = conn.sendSessionReply('sess-1', { reply: true }); + + expect(connectionStream.sent).toEqual([]); + expect(registry.getSnapshot()).toMatchObject({ + bufferedSessionFrames: 2, + preAttachOwnedFrames: 2, + }); + expect(budget.snapshot().usedFrames).toBe(2); + + const sessionStream = new FakeStream('sse'); + conn.attachSessionStream('sess-1', sessionStream, new AbortController()); + + await expect(eventDelivery).resolves.toBe('delivered'); + await expect(replyDelivery).resolves.toBe('delivered'); + expect(sessionStream.sent).toEqual([ + { message: { event: true }, id: 7 }, + { message: { reply: true }, id: undefined }, + ]); + expect(budget.snapshot().usedFrames).toBe(0); + } finally { + registry.dispose(); + } + }); + + it('hands pending gap replies to a replacement session stream without releasing their leases', async () => { + const budget = new AcpPreAttachBudget({ maxFrames: 4, maxBytes: 1024 }); + const registry = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + budget, + ); + try { + const conn = registry.create(true); + if (!conn) return; + conn.ownSession('sess-1'); + conn.getOrCreateSession('sess-1'); + + const first = conn.sendSessionReply('sess-1', { reply: 1 }); + const second = conn.sendSessionReply('sess-1', { reply: 2 }); + const streamA = new PendingSessionStream(); + conn.attachSessionStream('sess-1', streamA, new AbortController()); + expect(streamA.sent).toEqual([ + { message: { reply: 1 }, id: undefined }, + { message: { reply: 2 }, id: undefined }, + ]); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 2, + pendingDeliveryFrames: 2, + }); + + const streamB = new PendingSessionStream(); + conn.attachSessionStream('sess-1', streamB, new AbortController()); + streamA.completeAll('outcome_unknown'); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 2, + pendingDeliveryFrames: 2, + }); + + const streamC = new PendingSessionStream(); + conn.attachSessionStream('sess-1', streamC, new AbortController()); + streamB.completeAll('outcome_unknown'); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 2, + pendingDeliveryFrames: 2, + }); + streamC.completeAll('delivered'); + + await expect(first).resolves.toBe('delivered'); + await expect(second).resolves.toBe('delivered'); + expect(streamC.sent).toEqual([ + { message: { reply: 1 }, id: undefined }, + { message: { reply: 2 }, id: undefined }, + ]); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 0, + usedBytes: 0, + pendingDeliveryFrames: 0, + }); + } finally { + registry.dispose(); + } + }); + + it('hands a pending live reply to a replacement session stream', async () => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + conn.ownSession('sess-1'); + const streamA = new PendingSessionStream(); + conn.attachSessionStream('sess-1', streamA, new AbortController()); + + const delivery = conn.sendSessionReply('sess-1', { reply: 'live' }); + expect(streamA.sent).toEqual([ + { message: { reply: 'live' }, id: undefined }, + ]); + + const streamB = new PendingSessionStream(); + conn.attachSessionStream('sess-1', streamB, new AbortController()); + streamA.completeAll('outcome_unknown'); + streamB.completeAll('delivered'); + + await expect(delivery).resolves.toBe('delivered'); + expect(streamB.sent).toEqual([ + { message: { reply: 'live' }, id: undefined }, + ]); + expect(registry.getSnapshot()).toMatchObject({ + preAttachOwnedFrames: 0, + preAttachOwnedBytes: 0, + }); + } finally { + registry.dispose(); + } + }); + + it('buffers a pending live reply when disconnect precedes the replacement stream', async () => { + const budget = new AcpPreAttachBudget({ maxFrames: 4, maxBytes: 1024 }); + const registry = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + budget, + ); + try { + const conn = registry.create(true); + if (!conn) return; + conn.ownSession('sess-1'); + const streamA = new PendingSessionStream(); + conn.attachSessionStream('sess-1', streamA, new AbortController()); + + const delivery = conn.sendSessionReply('sess-1', { reply: 'live' }, 7); + streamA.close(); + streamA.completeAll('outcome_unknown'); + conn.detachSessionStream('sess-1', streamA, 10_000); + await Promise.resolve(); + expect(registry.getSnapshot()).toMatchObject({ + bufferedSessionFrames: 1, + pendingDeliveryFrames: 1, + preAttachOwnedFrames: 1, + preAttachOwnedBytes: Buffer.byteLength('{"reply":"live"}'), + }); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 1, + pendingDeliveryFrames: 1, + }); + + const streamB = new PendingSessionStream(); + conn.attachSessionStream('sess-1', streamB, new AbortController(), 5); + expect(streamB.sent).toEqual([]); + conn.releaseDeferredSessionReplies('sess-1', 7); + streamB.completeAll('delivered'); + + await expect(delivery).resolves.toBe('delivered'); + expect(streamB.sent).toEqual([ + { message: { reply: 'live' }, id: undefined }, + ]); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 0, + usedBytes: 0, + pendingDeliveryFrames: 0, + }); + } finally { + registry.dispose(); + } + }); + + it('defers handed-off gap replies behind a replacement stream replay', async () => { + const budget = new AcpPreAttachBudget({ maxFrames: 4, maxBytes: 1024 }); + const registry = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + budget, + ); + try { + const conn = registry.create(true); + if (!conn) return; + conn.ownSession('sess-1'); + conn.getOrCreateSession('sess-1'); + + const delivery = conn.sendSessionReply('sess-1', { reply: true }); + const streamA = new PendingSessionStream(); + conn.attachSessionStream('sess-1', streamA, new AbortController()); + + const streamB = new PendingSessionStream(); + conn.attachSessionStream('sess-1', streamB, new AbortController(), 5); + streamA.completeAll('outcome_unknown'); + expect(streamB.sent).toEqual([]); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 1, + pendingDeliveryFrames: 1, + }); + + conn.endReplayDeferral('sess-1', 5); + streamB.completeAll('delivered'); + await expect(delivery).resolves.toBe('delivered'); + expect(streamB.sent).toEqual([ + { message: { reply: true }, id: undefined }, + ]); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 0, + usedBytes: 0, + pendingDeliveryFrames: 0, + }); + } finally { + registry.dispose(); + } + }); + it('on resume, skips id-bearing buffered frames (ring owns them) AND defers id-less replies until flushBufferedSessionFrames (post-replay order)', () => { // Two regressions in one path: // (1) silent-frame-loss: a frame sent to the dead socket (id below the @@ -201,12 +506,7 @@ describe('ConnectionRegistry.getSnapshot', () => { } }); - it('under a content flood the pre-attach buffer evicts id-bearing (ring-replayable) frames and keeps the irreplaceable id-less reply', () => { - // The buffer cap (256) is shared between id-bearing bus events (the ring - // redelivers them on reconnect) and id-less deferred JSON-RPC replies (the - // ring does NOT track them). A fast model flooding content during a detach - // gap must not evict the `session/prompt` reply — that would hang the - // caller. Eviction must prefer the replayable id-bearing frames. + it('retires the exact session instead of silently evicting a pre-attach frame', async () => { const registry = new ConnectionRegistry(); try { const conn = registry.create(true); @@ -214,104 +514,860 @@ describe('ConnectionRegistry.getSnapshot', () => { conn.ownSession('sess-1'); conn.getOrCreateSession('sess-1'); - // One irreplaceable id-less reply lands first, then a flood of id-bearing - // content frames well past the 256 cap. - conn.sendSessionReply('sess-1', { promptResult: true }); - for (let i = 1; i <= 400; i++) - conn.sendSession('sess-1', { chunk: i }, i); + for (let i = 0; i < 256; i++) { + void conn.sendSession('sess-1', { chunk: i }, i); + } + await expect( + conn.sendSession('sess-1', { chunk: 256 }, 256), + ).resolves.toBe('failed'); + expect(conn.sessions.has('sess-1')).toBe(false); + expect(conn.destroyed).toBe(false); + } finally { + registry.dispose(); + } + }); - // Fresh reconnect flushes whatever survived. The id-less reply must be - // among the flushed frames (id-bearing frames were evicted preferentially). - const s = new FakeStream('sse'); - conn.attachSessionStream('sess-1', s, new AbortController()); - const replyDelivered = s.sent.some( - (x) => - (x.message as { promptResult?: boolean }).promptResult === true && - x.id === undefined, + it('contains a throwing detach callback during session resource failure', async () => { + const onDetach = vi.fn(() => { + throw new Error('detach failed'); + }); + const registry = new ConnectionRegistry(undefined, onDetach); + try { + const conn = registry.create(true); + if (!conn) return; + conn.ownSession('sess-1'); + conn.getOrCreateSession('sess-1'); + for (let i = 0; i < 256; i++) { + void conn.sendSession('sess-1', { chunk: i }, i); + } + + await expect( + conn.sendSession('sess-1', { chunk: 256 }, 256), + ).resolves.toBe('failed'); + expect(onDetach).toHaveBeenCalledOnce(); + expect(conn.sessions.has('sess-1')).toBe(false); + expect(registry.get(conn.connectionId)).toBe(conn); + } finally { + registry.dispose(); + } + }); + + it('retires the connection when its connection-scoped stream reaches the hard frame cap', async () => { + const registry = new ConnectionRegistry(undefined, undefined, 2); + try { + const conn = registry.create(true); + if (!conn) return; + for (let i = 0; i < 256; i++) void conn.sendConn({ reply: i }); + await expect(conn.sendConn({ reply: 256 })).resolves.toBe('failed'); + expect(registry.get(conn.connectionId)).toBeUndefined(); + expect(conn.destroyed).toBe(true); + } finally { + registry.dispose(); + } + }); + + it('discards every provisional receipt when overflow retires a connection', async () => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + const delivered = vi.fn(); + const discarded = vi.fn(); + for (let i = 0; i < 256; i++) { + void conn.sendConn({ reply: i }, { delivered, discarded }); + } + await expect( + conn.sendConn({ reply: 256 }, { delivered, discarded }), + ).resolves.toBe('failed'); + expect(delivered).not.toHaveBeenCalled(); + expect(discarded).toHaveBeenCalledTimes(257); + } finally { + registry.dispose(); + } + }); + + it('bounds retained serialized payload bytes before stream attachment', async () => { + const registry = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + undefined, + 1024, + 1024, + ); + try { + const conn = registry.create(true); + if (!conn) return; + await expect(conn.sendConn({ value: 'x'.repeat(2048) })).resolves.toBe( + 'failed', ); - expect(replyDelivered).toBe(true); + expect(conn.destroyed).toBe(true); } finally { registry.dispose(); } }); - it('never evicts an irreplaceable id-less reply even when the buffer is ENTIRELY id-less replies (no id-bearing frame to drop)', () => { - // Degenerate case wenshao flagged: if the gap buffer fills with only id-less - // deferred replies, there is no replayable frame to evict — dropping one - // would silently hang its caller. So pushCapped must NOT drop; it lets the - // irreplaceable replies exceed the soft cap (they're bounded by real - // in-flight RPC count, not a content flood). Every reply must survive. + it('enforces the connection frame cap across distinct session streams', async () => { + const registry = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + undefined, + 2, + ); + try { + const conn = registry.create(true); + if (!conn) return; + for (const sessionId of ['sess-1', 'sess-2', 'sess-3']) { + conn.ownSession(sessionId); + conn.getOrCreateSession(sessionId); + } + void conn.sendSession('sess-1', { first: true }); + void conn.sendSession('sess-2', { second: true }); + + await expect( + conn.sendSession('sess-3', { overflow: true }), + ).resolves.toBe('failed'); + expect(registry.get(conn.connectionId)).toBeUndefined(); + } finally { + registry.dispose(); + } + }); + + it('accepts the exact byte limit and rejects the next byte', async () => { + const exactFrame = { value: 'exact' }; + const exactBytes = Buffer.byteLength(JSON.stringify(exactFrame)); + const registry = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + undefined, + 1024, + exactBytes, + ); + try { + const conn = registry.create(true); + if (!conn) return; + void conn.sendConn(exactFrame); + expect(registry.getSnapshot().preAttachOwnedBytes).toBe(exactBytes); + await expect(conn.sendConn('x')).resolves.toBe('failed'); + expect(conn.destroyed).toBe(true); + } finally { + registry.dispose(); + } + }); + + it('freezes a buffered frame at serialization time', async () => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + const frame = { value: 'before' }; + void conn.sendConn(frame); + frame.value = 'after'; + const stream = new FakeStream('sse'); + conn.attachConnStream(stream); + expect(stream.sent).toEqual([ + { message: { value: 'before' }, id: undefined }, + ]); + } finally { + registry.dispose(); + } + }); + + it('limits a live session serialization failure to its independent stream', async () => { const registry = new ConnectionRegistry(); - const stderr = vi - .spyOn(process.stderr, 'write') - .mockImplementation(() => true); try { const conn = registry.create(true); if (!conn) return; conn.ownSession('sess-1'); - conn.getOrCreateSession('sess-1'); + conn.attachSessionStream( + 'sess-1', + new FakeStream('sse'), + new AbortController(), + ); + const frame: { self?: unknown } = {}; + frame.self = frame; + await expect(conn.sendSession('sess-1', frame)).resolves.toBe('failed'); + expect(conn.sessions.has('sess-1')).toBe(false); + expect(registry.get(conn.connectionId)).toBe(conn); + } finally { + registry.dispose(); + } + }); - // Far past the 256 soft cap but under the 1024 hard cap: ALL id-less - // replies (no id-bearing frames). - const N = 300; - for (let i = 0; i < N; i++) conn.sendSessionReply('sess-1', { reply: i }); + it.each([ + ['BigInt', { value: 1n }], + ['undefined', undefined], + ])( + 'retires the exact buffered owner for an unserializable %s frame', + async (_, frame) => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + conn.ownSession('sess-1'); + conn.getOrCreateSession('sess-1'); + + await expect(conn.sendSession('sess-1', frame)).resolves.toBe('failed'); + expect(conn.sessions.has('sess-1')).toBe(false); + expect(registry.get(conn.connectionId)).toBe(conn); + } finally { + registry.dispose(); + } + }, + ); + + it('shares the daemon budget across workspace registries', async () => { + const budget = new AcpPreAttachBudget({ maxFrames: 2, maxBytes: 1024 }); + const primary = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + budget, + ); + const secondary = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + budget, + ); + try { + const primaryConn = primary.create(true); + const secondaryConn = secondary.create(true); + if (!primaryConn || !secondaryConn) return; + void primaryConn.sendConn({ source: 'primary' }); + void secondaryConn.sendConn({ source: 'secondary' }); + expect(budget.snapshot().usedFrames).toBe(2); + + await expect( + secondaryConn.sendConn({ source: 'overflow' }), + ).resolves.toBe('failed'); + expect(secondary.get(secondaryConn.connectionId)).toBeUndefined(); + expect(primary.get(primaryConn.connectionId)).toBe(primaryConn); + expect(budget.snapshot().usedFrames).toBe(1); + + primary.dispose(); + expect(budget.snapshot().usedFrames).toBe(0); + } finally { + primary.dispose(); + secondary.dispose(); + } + }); - // Fresh reconnect flushes everything — not one reply was evicted. - const s = new FakeStream('sse'); - conn.attachSessionStream('sess-1', s, new AbortController()); - const replyIds = s.sent - .map((x) => (x.message as { reply?: number }).reply) - .filter((v) => typeof v === 'number'); - expect(replyIds).toHaveLength(N); - expect(new Set(replyIds).size).toBe(N); // all distinct, none lost - - // The soft-cap warning is the operator's only signal it was exceeded — - // assert it fired (exactly once, at the transition, not per push). - const softCapLogs = stderr.mock.calls.filter((c) => - String(c[0]).includes('pre-attach buffer over soft cap'), + it('shares the daemon byte budget across workspace registries', async () => { + const frame = { value: 'payload' }; + const bytes = Buffer.byteLength(JSON.stringify(frame)); + const budget = new AcpPreAttachBudget({ maxFrames: 10, maxBytes: bytes }); + const primary = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + budget, + ); + const secondary = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + budget, + ); + try { + const primaryConn = primary.create(true); + const secondaryConn = secondary.create(true); + if (!primaryConn || !secondaryConn) return; + void primaryConn.sendConn(frame); + + await expect(secondaryConn.sendConn(frame)).resolves.toBe('failed'); + expect(secondary.get(secondaryConn.connectionId)).toBeUndefined(); + expect(primary.get(primaryConn.connectionId)).toBe(primaryConn); + expect(budget.snapshot().usedBytes).toBe(bytes); + } finally { + primary.dispose(); + secondary.dispose(); + } + }); + + it('keeps a lease until an in-flight delivery settles after teardown', async () => { + const budget = new AcpPreAttachBudget({ maxFrames: 4, maxBytes: 1024 }); + const registry = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + budget, + ); + try { + const conn = registry.create(true); + if (!conn) return; + const delivery = conn.sendConn({ buffered: true }); + const stream = new ControlledStream(); + conn.attachConnStream(stream); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 1, + pendingDeliveryFrames: 1, + }); + + registry.delete(conn.connectionId); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 1, + pendingDeliveryFrames: 1, + }); + + stream.complete('closed'); + await expect(delivery).resolves.toBe('closed'); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 0, + usedBytes: 0, + pendingDeliveryFrames: 0, + }); + } finally { + registry.dispose(); + } + }); + + it('requeues a connection reply when its live stream closes before delivery', async () => { + const budget = new AcpPreAttachBudget({ maxFrames: 4, maxBytes: 1024 }); + const registry = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + budget, + ); + try { + const conn = registry.create(true); + if (!conn) return; + const first = new ControlledStream(); + conn.attachConnStream(first); + const delivered = vi.fn(); + const discarded = vi.fn(); + const frame = { id: 42, result: { ok: true } }; + const delivery = conn.sendConn(frame, { delivered, discarded }); + + first.close(); + first.complete('closed'); + await vi.waitFor(() => + expect(registry.getSnapshot().bufferedConnectionFrames).toBe(1), ); - expect(softCapLogs).toHaveLength(1); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 1, + usedBytes: Buffer.byteLength(JSON.stringify(frame)), + }); + expect(delivered).not.toHaveBeenCalled(); + expect(discarded).not.toHaveBeenCalled(); + + const replacement = new FakeStream('sse'); + conn.attachConnStream(replacement); + await expect(delivery).resolves.toBe('delivered'); + expect(replacement.sent).toEqual([ + { message: { id: 42, result: { ok: true } }, id: undefined }, + ]); + expect(delivered).toHaveBeenCalledOnce(); + expect(discarded).not.toHaveBeenCalled(); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 0, + usedBytes: 0, + pendingDeliveryFrames: 0, + }); } finally { - stderr.mockRestore(); registry.dispose(); } }); - it('enforces a HARD ceiling on all-id-less buffer growth (defense-in-depth) — drops oldest and logs loudly past 4× the soft cap', () => { - // The soft cap never drops id-less replies, but an UNBOUNDED heap is worse - // than a hung caller — so past the 1024 hard cap pushCapped drops the oldest - // id-less reply and logs loudly. Bounds a pathological / buggy producer. + it('settles a pending delivery against its original session binding', async () => { + const budget = new AcpPreAttachBudget({ maxFrames: 4, maxBytes: 1024 }); + const registry = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + budget, + ); + try { + const conn = registry.create(true); + if (!conn) return; + conn.ownSession('sess-1'); + const original = conn.getOrCreateSession('sess-1'); + const originalDelivery = conn.sendSession('sess-1', { original: true }); + const stream = new ControlledStream(); + conn.attachSessionStream('sess-1', stream, new AbortController()); + + conn.closeSessionStream('sess-1'); + conn.ownSession('sess-1'); + const replacement = conn.getOrCreateSession('sess-1'); + const replacementDelivery = conn.sendSession('sess-1', { + replacement: true, + }); + expect(original.ownedFrames).toBe(1); + expect(replacement.ownedFrames).toBe(1); + + stream.complete('closed'); + await expect(originalDelivery).resolves.toBe('closed'); + expect(original.ownedFrames).toBe(0); + expect(replacement.ownedFrames).toBe(1); + expect(budget.snapshot().usedFrames).toBe(1); + + conn.closeSessionStream('sess-1'); + await expect(replacementDelivery).resolves.toBe('closed'); + expect(budget.snapshot().usedFrames).toBe(0); + } finally { + registry.dispose(); + } + }); + + it('tombstones session ownership before teardown callbacks can re-enter', () => { const registry = new ConnectionRegistry(); - const stderr = vi - .spyOn(process.stderr, 'write') - .mockImplementation(() => true); try { const conn = registry.create(true); if (!conn) return; conn.ownSession('sess-1'); conn.getOrCreateSession('sess-1'); + const identity = conn.captureSessionOwnershipIdentity('sess-1'); - const N = 1100; // past the 1024 hard cap - for (let i = 0; i < N; i++) conn.sendSessionReply('sess-1', { reply: i }); + conn.closeSessionStream('sess-1'); - const s = new FakeStream('sse'); - conn.attachSessionStream('sess-1', s, new AbortController()); - const replyIds = s.sent - .map((x) => (x.message as { reply?: number }).reply) - .filter((v): v is number => typeof v === 'number'); - - // Buffer bounded at the hard cap (1024); the OLDEST were dropped, so the - // most-recent survive. - expect(replyIds).toHaveLength(1024); - expect(replyIds).toContain(N - 1); // newest kept - expect(replyIds).not.toContain(0); // oldest dropped - expect( - stderr.mock.calls.some((c) => - String(c[0]).includes('HARD buffer cap breached'), - ), - ).toBe(true); + expect(conn.canCommitSessionOwnership('sess-1', identity)).toBe(false); + } finally { + registry.dispose(); + } + }); + + it('rejects a deferred ownership grant after the same session was closed', () => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + const first = conn.captureSessionOwnershipIdentity('sess-1'); + const deferred = conn.captureSessionOwnershipIdentity('sess-1'); + + expect(conn.canCommitSessionOwnership('sess-1', first)).toBe(true); + conn.getOrCreateSession('sess-1'); + conn.ownSession('sess-1'); + conn.releaseSessionOwnershipIdentity('sess-1', first); + conn.closeSessionStream('sess-1'); + + expect(conn.canCommitSessionOwnership('sess-1', deferred)).toBe(false); + conn.releaseSessionOwnershipIdentity('sess-1', deferred); + + const replacement = conn.captureSessionOwnershipIdentity('sess-1'); + expect(conn.canCommitSessionOwnership('sess-1', replacement)).toBe(true); + expect(replacement.generation).toBe(0); + conn.releaseSessionOwnershipIdentity('sess-1', replacement); + } finally { + registry.dispose(); + } + }); + + it('rejects an ownership commit while session/close is in flight', () => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + const identity = conn.captureSessionOwnershipIdentity('sess-1'); + conn.closingSessions.add('sess-1'); + + expect(conn.canCommitSessionOwnership('sess-1', identity)).toBe(false); + } finally { + registry.dispose(); + } + }); + + it('treats session overflow as connection-fatal on a shared WebSocket before lazy attachment', async () => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + conn.ownSession('sess-1'); + conn.getOrCreateSession('sess-1'); + for (let i = 0; i < 256; i++) { + void conn.sendSession('sess-1', { chunk: i }, i); + } + conn.attachConnStream(new ControlledStream('ws')); + + await expect( + conn.sendSession('sess-1', { chunk: 256 }, 256), + ).resolves.toBe('failed'); + expect(registry.get(conn.connectionId)).toBeUndefined(); + expect(conn.destroyed).toBe(true); + } finally { + registry.dispose(); + } + }); + + it('limits an independently attached SSE session without closing its WebSocket connection', async () => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + conn.attachConnStream(new FakeStream('ws')); + conn.ownSession('sess-1'); + const sessionStream = new FakeStream('sse'); + conn.attachSessionStream('sess-1', sessionStream, new AbortController()); + const frame: { self?: unknown } = {}; + frame.self = frame; + + await expect(conn.sendSession('sess-1', frame)).resolves.toBe('failed'); + expect(conn.sessions.has('sess-1')).toBe(false); + expect(registry.get(conn.connectionId)).toBe(conn); + expect(conn.destroyed).toBe(false); + } finally { + registry.dispose(); + } + }); + + it('limits a detached SSE session without closing its WebSocket connection', async () => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + const connectionStream = new FakeStream('ws'); + conn.attachConnStream(connectionStream); + conn.ownSession('sess-1'); + const sessionStream = new FakeStream('sse'); + conn.attachSessionStream('sess-1', sessionStream, new AbortController()); + conn.detachSessionStream('sess-1', sessionStream, 10_000); + const frame: { self?: unknown } = {}; + frame.self = frame; + + await expect(conn.sendSession('sess-1', frame)).resolves.toBe('failed'); + expect(conn.sessions.has('sess-1')).toBe(false); + expect(registry.get(conn.connectionId)).toBe(conn); + expect(connectionStream.isClosed).toBe(false); + expect(conn.destroyed).toBe(false); + } finally { + registry.dispose(); + } + }); + + it('discards a live provisional receipt when teardown beats delivery', async () => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + const stream = new ControlledStream(); + conn.attachConnStream(stream); + const delivered = vi.fn(); + const discarded = vi.fn(); + const send = conn.sendConn( + { sessionId: 'provisional' }, + { delivered, discarded }, + ); + + registry.delete(conn.connectionId); + await vi.waitFor(() => expect(discarded).toHaveBeenCalledOnce()); + expect(delivered).not.toHaveBeenCalled(); + + stream.complete('delivered'); + await expect(send).resolves.toBe('delivered'); + expect(discarded).toHaveBeenCalledOnce(); + expect(delivered).not.toHaveBeenCalled(); + } finally { + registry.dispose(); + } + }); + + it('commits a provisional receipt when local delivery is ambiguous', async () => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + const stream = new ControlledStream(); + conn.attachConnStream(stream); + const delivered = vi.fn(); + const discarded = vi.fn(); + const send = conn.sendConn( + { sessionId: 'provisional' }, + { delivered, discarded }, + ); + + stream.complete('outcome_unknown'); + await expect(send).resolves.toBe('outcome_unknown'); + expect(delivered).toHaveBeenCalledOnce(); + expect(discarded).not.toHaveBeenCalled(); + } finally { + registry.dispose(); + } + }); + + it('lets an ambiguous delivery settle before destroy discards receipts', async () => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + const stream = new ControlledStream(); + conn.attachConnStream(stream); + const delivered = vi.fn(); + const discarded = vi.fn(); + const outcomeUnknown = vi.fn(); + const send = conn.sendConn( + { sessionId: 'provisional' }, + { delivered, discarded, outcomeUnknown }, + ); + + registry.delete(conn.connectionId); + stream.complete('outcome_unknown'); + await expect(send).resolves.toBe('outcome_unknown'); + expect(outcomeUnknown).toHaveBeenCalledOnce(); + expect(delivered).not.toHaveBeenCalled(); + expect(discarded).not.toHaveBeenCalled(); + } finally { + registry.dispose(); + } + }); + + it('preserves an active WebSocket receipt when close makes delivery ambiguous', async () => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + const socket = new PendingWebSocket(); + conn.attachConnStream(new WsStream(socket as never)); + const delivered = vi.fn(); + const discarded = vi.fn(); + const outcomeUnknown = vi.fn(); + const send = conn.sendConn( + { sessionId: 'provisional' }, + { delivered, discarded, outcomeUnknown }, + ); + await vi.waitFor(() => expect(socket.callback).toBeDefined()); + + registry.delete(conn.connectionId); + + await expect(send).resolves.toBe('outcome_unknown'); + expect(outcomeUnknown).toHaveBeenCalledOnce(); + expect(delivered).not.toHaveBeenCalled(); + expect(discarded).not.toHaveBeenCalled(); + socket.callback?.(); + expect(outcomeUnknown).toHaveBeenCalledOnce(); + } finally { + registry.dispose(); + } + }); + + it('preserves an active WebSocket receipt when peer loss reaches the send callback first', async () => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + const socket = new PendingWebSocket(); + conn.attachConnStream( + new WsStream(socket as never, () => registry.delete(conn.connectionId)), + ); + const delivered = vi.fn(); + const discarded = vi.fn(); + const outcomeUnknown = vi.fn(); + const send = conn.sendConn( + { sessionId: 'provisional' }, + { delivered, discarded, outcomeUnknown }, + ); + await vi.waitFor(() => expect(socket.callback).toBeDefined()); + + socket.callback?.(new Error('socket closed')); + socket.readyState = 3; + socket.emit('close'); + + await expect(send).resolves.toBe('outcome_unknown'); + expect(outcomeUnknown).toHaveBeenCalledOnce(); + expect(delivered).not.toHaveBeenCalled(); + expect(discarded).not.toHaveBeenCalled(); + expect(registry.get(conn.connectionId)).toBeUndefined(); + } finally { + registry.dispose(); + } + }); + + it.each([ + ['live', { value: 1n }], + [ + 'buffered', + (() => { + const frame: { self?: unknown } = {}; + frame.self = frame; + return frame; + })(), + ], + ])( + 'contains a %s connection response serialization failure to one frame', + async (mode, frame) => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + for (const sessionId of ['sess-1', 'sess-2']) { + conn.ownSession(sessionId); + conn.getOrCreateSession(sessionId); + } + if (mode === 'live') conn.attachConnStream(new FakeStream('sse')); + + await expect(conn.sendConn(frame)).resolves.toBe('failed'); + expect(registry.get(conn.connectionId)).toBe(conn); + expect(conn.destroyed).toBe(false); + expect(conn.ownedSessions).toEqual(new Set(['sess-1', 'sess-2'])); + expect(conn.sessions.get('sess-1')?.abort.signal.aborted).toBe(false); + expect(conn.sessions.get('sess-2')?.abort.signal.aborted).toBe(false); + } finally { + registry.dispose(); + } + }, + ); + + it('settles delivery when a delivered receipt callback throws', async () => { + const budget = new AcpPreAttachBudget({ maxFrames: 1, maxBytes: 1024 }); + const registry = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + budget, + ); + try { + const conn = registry.create(true); + if (!conn) return; + const delivery = conn.sendConn( + { reply: true }, + { + delivered: () => { + throw new Error('delivered callback failed'); + }, + discarded: vi.fn(), + }, + ); + + conn.attachConnStream(new FakeStream('sse')); + + await expect(delivery).resolves.toBe('delivered'); + expect(budget.snapshot().usedFrames).toBe(0); + } finally { + registry.dispose(); + } + }); + + it('continues teardown when a discarded receipt callback throws', async () => { + const registry = new ConnectionRegistry(); + const conn = registry.create(true); + if (!conn) return; + const delivery = conn.sendConn( + { reply: true }, + { + delivered: vi.fn(), + discarded: () => { + throw new Error('discarded callback failed'); + }, + }, + ); + + expect(() => registry.delete(conn.connectionId)).not.toThrow(); + await expect(delivery).resolves.toBe('closed'); + registry.dispose(); + }); + + it('revalidates connection identity after serialization re-entry', async () => { + const budget = new AcpPreAttachBudget({ maxFrames: 4, maxBytes: 1024 }); + const registry = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + budget, + ); + try { + const conn = registry.create(true); + if (!conn) return; + const frame = { + toJSON: () => { + registry.delete(conn.connectionId); + return { stale: true }; + }, + }; + await expect(conn.sendConn(frame)).resolves.toBe('closed'); + expect(budget.snapshot()).toMatchObject({ usedFrames: 0, usedBytes: 0 }); + expect(registry.get(conn.connectionId)).toBeUndefined(); + } finally { + registry.dispose(); + } + }); + + it('revalidates connection identity after getter re-entry', async () => { + const budget = new AcpPreAttachBudget({ maxFrames: 4, maxBytes: 1024 }); + const registry = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + budget, + ); + try { + const conn = registry.create(true); + if (!conn) return; + const frame = Object.defineProperty({}, 'value', { + enumerable: true, + get: () => { + registry.delete(conn.connectionId); + return 'stale'; + }, + }); + + await expect(conn.sendConn(frame)).resolves.toBe('closed'); + expect(budget.snapshot()).toMatchObject({ usedFrames: 0, usedBytes: 0 }); + expect(registry.get(conn.connectionId)).toBeUndefined(); + } finally { + registry.dispose(); + } + }); + + it('rejects a buffered frame when serialization re-entry replaces its stream', async () => { + const budget = new AcpPreAttachBudget({ maxFrames: 4, maxBytes: 1024 }); + const registry = new ConnectionRegistry( + undefined, + undefined, + 2, + 30 * 60_000, + budget, + ); + try { + const conn = registry.create(true); + if (!conn) return; + const replacement = new FakeStream('sse'); + const frame = Object.defineProperty({}, 'value', { + enumerable: true, + get: () => { + conn.attachConnStream(replacement); + return 'stale'; + }, + }); + + await expect(conn.sendConn(frame)).resolves.toBe('closed'); + expect(replacement.sent).toEqual([]); + expect(budget.snapshot()).toMatchObject({ usedFrames: 0, usedBytes: 0 }); + } finally { + registry.dispose(); + } + }); + + it('does not fail a replacement stream when stale serialization re-entry throws', async () => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + const replacement = new FakeStream('sse'); + const frame = { + toJSON: () => { + conn.attachConnStream(replacement); + throw new Error('stale serialization failed'); + }, + }; + + await expect(conn.sendConn(frame)).resolves.toBe('closed'); + expect(conn.destroyed).toBe(false); + expect(conn.connStream).toBe(replacement); } finally { - stderr.mockRestore(); registry.dispose(); } }); @@ -681,6 +1737,39 @@ describe('ConnectionRegistry.getSnapshot', () => { } }); + it('does not let a later reply overtake an earlier watermark-gated reply after replay completes', () => { + const registry = new ConnectionRegistry(); + try { + const conn = registry.create(true); + if (!conn) return; + conn.ownSession('sess-1'); + conn.getOrCreateSession('sess-1'); + + const stream = new FakeStream('sse'); + conn.attachSessionStream('sess-1', stream, new AbortController(), 5); + + void conn.sendSessionReply('sess-1', { first: true }, 9); + conn.endReplayDeferral('sess-1', 7); + expect(stream.sent).toEqual([]); + + void conn.sendSessionReply('sess-1', { second: true }, 10); + expect(stream.sent).toEqual([]); + + conn.releaseDeferredSessionReplies('sess-1', 9); + expect(stream.sent).toEqual([ + { message: { first: true }, id: undefined }, + ]); + + conn.releaseDeferredSessionReplies('sess-1', 10); + expect(stream.sent).toEqual([ + { message: { first: true }, id: undefined }, + { message: { second: true }, id: undefined }, + ]); + } finally { + registry.dispose(); + } + }); + it('releases ALL deferred replies at replay_complete when the replay evicted frames (state_resync_required) — no cascading freeze on an unreachable anchor', () => { // doudouOUC's cascading-freeze: a reply anchored ABOVE the surviving range // (its anchor event was evicted from the ring on overflow) would otherwise diff --git a/packages/cli/src/serve/acp-http/connection-registry.ts b/packages/cli/src/serve/acp-http/connection-registry.ts index f2c70e9d90b..433a1c6df87 100644 --- a/packages/cli/src/serve/acp-http/connection-registry.ts +++ b/packages/cli/src/serve/acp-http/connection-registry.ts @@ -7,27 +7,18 @@ import { randomUUID } from 'node:crypto'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; import { logSafe } from './json-rpc.js'; -import type { TransportStream } from './transport-stream.js'; - -/** - * Per-stream cap on frames buffered before the client attaches its SSE - * stream. Mirrors the EventBus's `maxQueued` backpressure cap so a client - * that drives requests without ever opening a stream can't grow daemon - * memory without bound. Oldest frames are dropped past the cap. - */ -const MAX_BUFFERED_FRAMES = 256; - -/** - * Defense-in-depth hard ceiling for the degenerate all-id-less buffer case. - * Id-less deferred replies are never evicted at the soft cap (dropping one - * hangs its caller), and they're bounded in practice by the number of in-flight - * session RPCs — but that invariant is convention, not enforced by the type - * system or call sites. So a future id-less producer that ISN'T RPC-bounded, or - * a buggy client, can't grow the buffer without limit: past this hard cap we - * drop the oldest id-less reply and log loudly (its caller may hang, but an - * unbounded daemon heap is worse). 4× the soft cap. - */ -const HARD_BUFFERED_FRAMES_CAP = MAX_BUFFERED_FRAMES * 4; +import { + ACP_PRE_ATTACH_MAX_FRAMES_PER_CONNECTION, + ACP_PRE_ATTACH_MAX_FRAMES_PER_STREAM, + ACP_PRE_ATTACH_MAX_PAYLOAD_BYTES_PER_CONNECTION, + AcpPreAttachBudget, + type AcpPreAttachLease, +} from './pre-attach-budget.js'; +import type { + DeliveryResult, + TransportCloseReason, + TransportStream, +} from './transport-stream.js'; /** Default cap on concurrent live connections (mirrors a bounded resource). */ const DEFAULT_MAX_CONNECTIONS = 64; @@ -54,9 +45,15 @@ export type DetachSessionFn = ( clientId: string | undefined, ) => void; -/** A pre-attach session frame plus its optional bus event id (SSE cursor). */ -interface BufferedSessionFrame { - frame: unknown; +export interface DeliveryReceipt { + delivered(): void; + discarded(): void; + outcomeUnknown?(): void; +} + +interface PreparedFrame { + payload: Buffer; + sequence: number; id?: number; /** * For DEFERRED out-of-band replies only (`sendSessionReply`, always id-less): @@ -67,6 +64,14 @@ interface BufferedSessionFrame { * (§1.8 W1). `undefined` ⇒ release at the next boundary unconditionally. */ anchorId?: number; + receipt?: DeliveryReceipt; + receiptSettled?: boolean; + lease?: AcpPreAttachLease; + binding?: SessionBinding; + requeueOnStreamReplacement?: boolean; + deliveryAttempt?: number; + deliveryStream?: TransportStream; + resolve: (result: DeliveryResult) => void; } /** @@ -87,12 +92,16 @@ export interface SessionBinding { clientId?: string; /** Session-scoped SSE stream (the client's `GET /acp` with both headers). */ stream?: TransportStream; + /** Stable across a transport detach while `stream` is temporarily absent. */ + usesConnectionStream?: boolean; /** * Frames emitted before the session stream attached, flushed on attach. * Each keeps its bus event id (when it has one) so the SSE `id:` resume * cursor survives the buffer → live-stream handoff. */ - buffer: BufferedSessionFrame[]; + buffer: PreparedFrame[]; + ownedFrames: number; + ownedBytes: number; /** * Aborts the bridge event subscription tied to the CURRENT session * stream. Replaced with a fresh controller on every re-attach — a @@ -152,6 +161,16 @@ export interface PendingClientRequest { kind: 'permission'; } +export interface SessionOwnershipIdentity { + binding: SessionBinding | undefined; + generation: number; +} + +interface SessionOwnershipState { + generation: number; + captures: number; +} + export interface PendingClientRequestRef { conn: AcpConnection; id: string; @@ -173,6 +192,9 @@ export interface AcpConnectionDiagnostic { wsStreams: number; bufferedConnectionFrames: number; bufferedSessionFrames: number; + pendingDeliveryFrames: number; + preAttachOwnedFrames: number; + preAttachOwnedBytes: number; } export interface ConnectionRegistrySnapshot { @@ -183,6 +205,12 @@ export interface ConnectionRegistrySnapshot { sseStreams: number; wsStreams: number; pendingClientRequests: number; + bufferedConnectionFrames: number; + bufferedSessionFrames: number; + pendingDeliveryFrames: number; + preAttachOwnedFrames: number; + preAttachOwnedBytes: number; + preAttachGuardFailures: number; connections: AcpConnectionDiagnostic[]; } @@ -193,7 +221,17 @@ export class AcpConnection { private readonly abortController = new AbortController(); readonly abortSignal = this.abortController.signal; /** Frames emitted before the connection stream attached, flushed on attach. */ - private readonly connBuffer: unknown[] = []; + private readonly connBuffer: PreparedFrame[] = []; + private readonly pendingDeliveries = new Set(); + private nextPreparedSequence = 0; + private ownedFrames = 0; + private ownedBytes = 0; + private connOwnedFrames = 0; + private readonly pendingReceipts = new Set(); + private readonly sessionOwnershipStates = new Map< + string, + SessionOwnershipState + >(); readonly sessions = new Map(); /** * Sessions this connection created (`session/new`) or explicitly @@ -259,6 +297,14 @@ export class AcpConnection { fromLoopback: boolean, private readonly onAbandonPending?: AbandonPendingFn, private readonly onDetachSession?: DetachSessionFn, + private readonly preAttachBudget = new AcpPreAttachBudget(), + private readonly onFatalConnection?: ( + connection: AcpConnection, + reason: TransportCloseReason, + ) => void, + private readonly maxFramesPerConnection = ACP_PRE_ATTACH_MAX_FRAMES_PER_CONNECTION, + private readonly maxPayloadBytesPerConnection = ACP_PRE_ATTACH_MAX_PAYLOAD_BYTES_PER_CONNECTION, + private readonly onPreAttachGuardFailure?: () => void, ) { this.connectionId = connectionId ?? randomUUID(); this.clientId = randomUUID(); @@ -288,10 +334,54 @@ export class AcpConnection { return this.ownedSessions.has(sessionId); } + captureSessionOwnershipIdentity(sessionId: string): SessionOwnershipIdentity { + let state = this.sessionOwnershipStates.get(sessionId); + if (!state) { + state = { generation: 0, captures: 0 }; + this.sessionOwnershipStates.set(sessionId, state); + } + state.captures += 1; + return { + binding: this.sessions.get(sessionId), + generation: state.generation, + }; + } + + canCommitSessionOwnership( + sessionId: string, + ownership: SessionOwnershipIdentity, + ): boolean { + return ( + !this.destroyed && + !this.closingSessions.has(sessionId) && + this.sessionOwnershipStates.get(sessionId)?.generation === + ownership.generation && + this.sessions.get(sessionId) === ownership.binding + ); + } + + releaseSessionOwnershipIdentity( + sessionId: string, + ownership: SessionOwnershipIdentity, + ): void { + const state = this.sessionOwnershipStates.get(sessionId); + if (!state || state.generation < ownership.generation) return; + state.captures -= 1; + if (state.captures === 0) { + this.sessionOwnershipStates.delete(sessionId); + } + } + getOrCreateSession(sessionId: string): SessionBinding { let binding = this.sessions.get(sessionId); if (!binding) { - binding = { sessionId, abort: new AbortController(), buffer: [] }; + binding = { + sessionId, + abort: new AbortController(), + buffer: [], + ownedFrames: 0, + ownedBytes: 0, + }; this.sessions.set(sessionId, binding); } return binding; @@ -317,8 +407,14 @@ export class AcpConnection { } let sessionStreams = 0; let bufferedSessionFrames = 0; + let pendingDeliveryFrames = 0; for (const binding of this.sessions.values()) { bufferedSessionFrames += binding.buffer.length; + pendingDeliveryFrames += binding.buffer.filter( + (prepared) => + prepared.lease !== undefined && + prepared.deliveryAttempt !== undefined, + ).length; if (binding.stream && !binding.stream.isClosed) { sessionStreams += 1; liveStreams.add(binding.stream); @@ -346,16 +442,28 @@ export class AcpConnection { wsStreams, bufferedConnectionFrames: this.connBuffer.length, bufferedSessionFrames, + pendingDeliveryFrames: + pendingDeliveryFrames + + [...this.pendingDeliveries].filter( + (prepared) => prepared.lease !== undefined, + ).length, + preAttachOwnedFrames: this.ownedFrames, + preAttachOwnedBytes: this.ownedBytes, }; } /** Send a frame on the connection-scoped stream (buffer until it attaches). */ - sendConn(frame: unknown): void { - if (this.connStream && !this.connStream.isClosed) { - void this.connStream.send(frame); - } else { - pushCapped(this.connBuffer, frame, `conn ${this.connectionId}`); - } + sendConn(frame: unknown, receipt?: DeliveryReceipt): Promise { + const trackedReceipt = this.trackReceipt(receipt); + return this.prepareAndBuffer( + this.connBuffer, + frame, + undefined, + undefined, + trackedReceipt, + undefined, + true, + ); } /** True if any session currently has a live (open) SSE stream. */ @@ -398,11 +506,20 @@ export class AcpConnection { attachConnStream(stream: TransportStream): void { // A reconnect cancels any pending grace-period reap. this.clearGraceTimer(); - // Close any prior connection stream so its heartbeat interval + socket - // don't leak when a client reconnects the connection-scoped GET. - if (this.connStream && this.connStream !== stream) this.connStream.close(); + const previousStream = this.connStream; this.connStream = stream; - for (const frame of this.connBuffer.splice(0)) void stream.send(frame); + // Move unresolved replies off the old stream before closing it. A queued + // SSE write can otherwise resolve `closed` without ever reaching the wire. + if (previousStream && previousStream !== stream) { + if (!this.requeuePendingConnectionFrames(previousStream)) { + previousStream.close(); + return; + } + previousStream.close(); + } + for (const prepared of this.connBuffer.splice(0)) { + this.deliverPrepared(stream, prepared); + } } /** @@ -413,19 +530,24 @@ export class AcpConnection { * creating here would resurrect a ghost binding (no stream, no owner) that * buffers up to 256 late pump/reply frames forever. */ - sendSession(sessionId: string, frame: unknown, id?: number): void { + sendSession( + sessionId: string, + frame: unknown, + id?: number, + ): Promise { const binding = this.sessions.get(sessionId); - if (!binding) return; + if (!binding) return Promise.resolve('closed'); if (binding.stream && !binding.stream.isClosed) { - void binding.stream.send(frame, id); - } else { - pushCapped( - binding.buffer, - { frame, id }, - `session ${sessionId}`, - (e) => e.id, - ); + return this.sendLive(binding.stream, frame, id, undefined, binding); } + return this.prepareAndBuffer( + binding.buffer, + frame, + id, + undefined, + undefined, + binding, + ); } /** @@ -447,26 +569,21 @@ export class AcpConnection { * is done, replies go straight to the wire (steady state, unchanged from a * non-resumed stream). */ - sendSessionReply(sessionId: string, frame: unknown, anchorId?: number): void { + sendSessionReply( + sessionId: string, + frame: unknown, + anchorId?: number, + ): Promise { const binding = this.sessions.get(sessionId); - if (!binding) return; - // Steady state — live stream, no replay in flight, nothing queued ahead — - // goes straight to the wire (same as a never-resumed stream). Otherwise - // defer with the watermark so ordering is preserved while catching up. - if ( - binding.stream && - !binding.stream.isClosed && - !binding.replayPending && - binding.buffer.length === 0 - ) { - void binding.stream.send(frame); - return; - } - pushCapped( + if (!binding) return Promise.resolve('closed'); + return this.prepareAndBuffer( binding.buffer, - { frame, id: undefined, anchorId }, - `session ${sessionId}`, - (e) => e.id, + frame, + undefined, + anchorId, + undefined, + binding, + true, ); } @@ -501,7 +618,7 @@ export class AcpConnection { break; } binding.buffer.shift(); - void binding.stream.send(front.frame); + this.deliverPrepared(binding.stream, front); } } @@ -543,7 +660,12 @@ export class AcpConnection { // the prompt must survive. CONTRACT: that identity guard and this ordering // must stay in lockstep. binding.stream = stream; + binding.usesConnectionStream = stream === this.connStream; if (prevStream && prevStream !== stream && prevStream !== this.connStream) { + if (!this.requeuePendingSessionReplies(binding, prevStream)) { + prevStream.close(); + return binding; + } prevStream.close(); } // Flush buffered pre-attach frames produced during the detach gap. @@ -597,7 +719,7 @@ export class AcpConnection { const gap = binding.buffer.splice(0); for (const entry of gap) { if (!replayingOnAttach) { - void stream.send(entry.frame, entry.id); // fresh connect: flush all now + this.deliverPrepared(stream, entry); // fresh connect: flush all now } else if (entry.id !== undefined) { // Resume: ring replay owns bus events, so drop the buffered copy to // avoid double-delivery (the same id arrives via replay). This branch is @@ -611,6 +733,7 @@ export class AcpConnection { // be ring-evicted before reconnect, which the replay signals to the // client as `state_resync_required` (not a silent gap) — strictly better // than the pre-resume live-only behaviour, which lost every gap frame. + this.settlePrepared(entry, 'closed'); continue; } else { binding.buffer.push(entry); // resume: defer id-less past replay @@ -686,8 +809,8 @@ export class AcpConnection { binding.buffer.length === 0 ) return; - for (const { frame, id } of binding.buffer.splice(0)) { - void binding.stream.send(frame, id); + for (const prepared of binding.buffer.splice(0)) { + this.deliverPrepared(binding.stream, prepared); } } @@ -765,17 +888,38 @@ export class AcpConnection { closeSessionStream(sessionId: string): void { const binding = this.sessions.get(sessionId); - if (!binding) return; - this.teardownBinding(binding); this.sessions.delete(sessionId); this.ownedSessions.delete(sessionId); + const ownershipState = this.sessionOwnershipStates.get(sessionId); + if (ownershipState) { + ownershipState.generation += 1; + if (ownershipState.captures === 0) { + this.sessionOwnershipStates.delete(sessionId); + } + } + if (!binding) return; + this.teardownBinding(binding); } - destroy(): void { + destroy(reason?: TransportCloseReason): void { + if (this.destroyed) return; this.destroyed = true; this.abortController.abort(); this.clearGraceTimer(); - for (const binding of this.sessions.values()) { + const bindings = [...this.sessions.values()]; + this.sessions.clear(); + this.ownedSessions.clear(); + for (const prepared of this.connBuffer.splice(0)) { + this.discardPrepared(prepared); + } + this.connStream?.close(reason); + setImmediate(() => { + for (const prepared of this.pendingDeliveries) { + this.discardPreparedReceipt(prepared); + } + for (const receipt of [...this.pendingReceipts]) receipt.discarded(); + }); + for (const binding of bindings) { try { this.teardownBinding(binding); } catch (err) { @@ -784,10 +928,8 @@ export class AcpConnection { ); } } - this.sessions.clear(); - this.ownedSessions.clear(); + this.sessionOwnershipStates.clear(); this.pending.clear(); - this.connStream?.close(); } private teardownBinding(binding: SessionBinding): void { @@ -795,6 +937,14 @@ export class AcpConnection { clearTimeout(binding.graceTimer); binding.graceTimer = undefined; } + for (const prepared of binding.buffer.splice(0)) { + this.discardPrepared(prepared); + } + for (const prepared of this.pendingDeliveries) { + if (prepared.binding === binding) { + this.discardPreparedReceipt(prepared); + } + } binding.abort.abort(); binding.promptAbort?.abort(); // Don't close the stream if it's the shared connStream (WS reuses @@ -806,6 +956,496 @@ export class AcpConnection { this.onDetachSession?.(binding.sessionId, binding.clientId); } + private sendLive( + stream: TransportStream, + frame: unknown, + id?: number, + receipt?: DeliveryReceipt, + binding?: SessionBinding, + ): Promise { + let payload: Buffer; + try { + const serialized = JSON.stringify(frame); + if (serialized === undefined) throw new Error('not serializable'); + payload = Buffer.from(serialized, 'utf8'); + } catch { + receipt?.discarded(); + if (!this.isCurrentStreamOwner(stream, binding)) { + return Promise.resolve('closed'); + } + this.failSerializationOwner(binding); + return Promise.resolve('failed'); + } + if (!this.isCurrentStreamOwner(stream, binding)) { + receipt?.discarded(); + return Promise.resolve('closed'); + } + const result = stream.sendSerialized(payload, id); + this.observeLiveReceipt(result, receipt); + return result; + } + + private isCurrentStreamOwner( + stream: TransportStream, + binding: SessionBinding | undefined, + ): boolean { + if (this.destroyed || stream.isClosed) return false; + return binding + ? this.sessions.get(binding.sessionId) === binding && + binding.stream === stream + : this.connStream === stream; + } + + private prepareAndBuffer( + buffer: PreparedFrame[], + frame: unknown, + id: number | undefined, + anchorId: number | undefined, + receipt: DeliveryReceipt | undefined, + binding: SessionBinding | undefined, + requeueOnStreamReplacement = false, + ): Promise { + if (this.destroyed) { + receipt?.discarded(); + return Promise.resolve('closed'); + } + const expectedStream = binding ? binding.stream : this.connStream; + if ( + (binding ? binding.ownedFrames : this.connOwnedFrames) >= + ACP_PRE_ATTACH_MAX_FRAMES_PER_STREAM + ) { + receipt?.discarded(); + this.failOwner(binding, 'ACP pre-attach frame limit'); + return Promise.resolve('failed'); + } + let payload: Buffer; + try { + const serialized = JSON.stringify(frame); + if (serialized === undefined) throw new Error('not serializable'); + payload = Buffer.from(serialized, 'utf8'); + } catch { + receipt?.discarded(); + if ( + this.destroyed || + (binding ? binding.stream : this.connStream) !== expectedStream || + (binding !== undefined && + this.sessions.get(binding.sessionId) !== binding) + ) { + return Promise.resolve('closed'); + } + this.failSerializationOwner(binding); + return Promise.resolve('failed'); + } + if ( + this.destroyed || + (binding ? binding.stream : this.connStream) !== expectedStream || + (binding !== undefined && + this.sessions.get(binding.sessionId) !== binding) + ) { + receipt?.discarded(); + return Promise.resolve('closed'); + } + if ( + (binding ? binding.ownedFrames : this.connOwnedFrames) >= + ACP_PRE_ATTACH_MAX_FRAMES_PER_STREAM + ) { + receipt?.discarded(); + this.failOwner(binding, 'ACP pre-attach frame limit'); + return Promise.resolve('failed'); + } + const liveStream = expectedStream; + const deliveryDeferred = binding?.replayPending === true; + const deliverImmediately = + buffer.length === 0 && + liveStream && + !liveStream.isClosed && + !deliveryDeferred; + if (deliverImmediately && !requeueOnStreamReplacement) { + return this.sendSerializedLive(liveStream, payload, id, receipt); + } + let lease: AcpPreAttachLease | undefined; + if (!deliverImmediately) { + if ( + this.ownedFrames >= this.maxFramesPerConnection || + payload.byteLength > this.maxPayloadBytesPerConnection - this.ownedBytes + ) { + receipt?.discarded(); + this.failConnection('ACP pre-attach connection budget'); + return Promise.resolve('failed'); + } + lease = this.preAttachBudget.tryReserve(payload.byteLength); + if (!lease) { + receipt?.discarded(); + this.failConnection('ACP pre-attach daemon budget', false); + return Promise.resolve('failed'); + } + } + let resolve!: (result: DeliveryResult) => void; + const result = new Promise((accept) => { + resolve = accept; + }); + const prepared: PreparedFrame = { + payload, + sequence: this.nextPreparedSequence++, + id, + anchorId, + receipt, + lease, + binding, + requeueOnStreamReplacement, + resolve, + }; + if (deliverImmediately) { + this.deliverPrepared(liveStream, prepared); + return result; + } + buffer.push(prepared); + this.ownedFrames += 1; + this.ownedBytes += payload.byteLength; + if (binding) { + binding.ownedFrames += 1; + binding.ownedBytes += payload.byteLength; + } else { + this.connOwnedFrames += 1; + } + return result; + } + + private sendSerializedLive( + stream: TransportStream, + payload: Buffer, + id: number | undefined, + receipt: DeliveryReceipt | undefined, + ): Promise { + const result = stream.sendSerialized(payload, id); + this.observeLiveReceipt(result, receipt); + return result; + } + + private observeLiveReceipt( + result: Promise, + receipt: DeliveryReceipt | undefined, + ): void { + void result.then( + (outcome) => { + if (outcome === 'delivered') receipt?.delivered(); + else if (outcome === 'outcome_unknown') receipt?.outcomeUnknown?.(); + else receipt?.discarded(); + }, + () => receipt?.discarded(), + ); + } + + private trackReceipt( + receipt: DeliveryReceipt | undefined, + ): DeliveryReceipt | undefined { + if (!receipt) return undefined; + let settled = false; + const tracked: DeliveryReceipt = { + delivered: () => { + if (settled) return; + settled = true; + this.pendingReceipts.delete(tracked); + this.invokeReceipt(receipt, 'delivered'); + }, + discarded: () => { + if (settled) return; + settled = true; + this.pendingReceipts.delete(tracked); + this.invokeReceipt(receipt, 'discarded'); + }, + outcomeUnknown: () => { + if (settled) return; + settled = true; + this.pendingReceipts.delete(tracked); + this.invokeReceipt(receipt, 'outcomeUnknown'); + }, + }; + this.pendingReceipts.add(tracked); + return tracked; + } + + private deliverPrepared( + stream: TransportStream, + prepared: PreparedFrame, + ): void { + const attempt = (prepared.deliveryAttempt ?? 0) + 1; + prepared.deliveryAttempt = attempt; + prepared.deliveryStream = stream; + prepared.lease?.markPendingDelivery(); + this.pendingDeliveries.add(prepared); + void stream.sendSerialized(prepared.payload, prepared.id).then( + (outcome) => this.settlePreparedAttempt(prepared, attempt, outcome), + () => this.settlePreparedAttempt(prepared, attempt, 'failed'), + ); + } + + private requeuePendingSessionReplies( + binding: SessionBinding, + previousStream: TransportStream, + ): boolean { + // These replies have no SSE event id and are absent from ring replay. Use + // at-least-once handoff: a duplicate JSON-RPC response is identifiable by + // its request id, while dropping the only response hangs the caller. + const requeued = [...this.pendingDeliveries].filter( + (prepared) => + prepared.binding === binding && + prepared.deliveryStream === previousStream && + prepared.requeueOnStreamReplacement, + ); + for (const prepared of requeued) { + if (!this.reservePreparedForBuffer(prepared, binding)) return false; + } + for (const prepared of requeued) { + if ( + prepared.binding !== binding || + prepared.deliveryStream !== previousStream || + !prepared.requeueOnStreamReplacement + ) { + continue; + } + prepared.deliveryAttempt = (prepared.deliveryAttempt ?? 0) + 1; + prepared.deliveryStream = undefined; + this.pendingDeliveries.delete(prepared); + this.insertPrepared(binding.buffer, prepared); + } + return true; + } + + private requeuePendingConnectionFrames( + previousStream: TransportStream, + ): boolean { + const requeued = [...this.pendingDeliveries].filter( + (prepared) => + prepared.binding === undefined && + prepared.deliveryStream === previousStream && + prepared.requeueOnStreamReplacement, + ); + for (const prepared of requeued) { + if (!this.reservePreparedForBuffer(prepared)) return false; + } + for (const prepared of requeued) { + if ( + prepared.binding !== undefined || + prepared.deliveryStream !== previousStream || + !prepared.requeueOnStreamReplacement + ) { + continue; + } + prepared.deliveryAttempt = (prepared.deliveryAttempt ?? 0) + 1; + prepared.deliveryStream = undefined; + this.pendingDeliveries.delete(prepared); + this.insertPrepared(this.connBuffer, prepared); + } + return true; + } + + private settlePreparedAttempt( + prepared: PreparedFrame, + attempt: number, + outcome: DeliveryResult, + ): void { + if (prepared.deliveryAttempt !== attempt) return; + const binding = prepared.binding; + const stream = prepared.deliveryStream; + if ( + outcome === 'closed' && + prepared.requeueOnStreamReplacement && + !binding && + stream && + stream.isClosed && + !this.destroyed && + this.connStream === stream + ) { + if (!this.reservePreparedForBuffer(prepared)) return; + prepared.deliveryAttempt = attempt + 1; + prepared.deliveryStream = undefined; + this.pendingDeliveries.delete(prepared); + this.insertPrepared(this.connBuffer, prepared); + return; + } + if ( + outcome !== 'delivered' && + prepared.requeueOnStreamReplacement && + binding && + stream && + stream !== this.connStream && + stream.isClosed && + !this.destroyed && + this.sessions.get(binding.sessionId) === binding && + (binding.stream === stream || binding.stream === undefined) + ) { + if (!this.reservePreparedForBuffer(prepared, binding)) return; + prepared.deliveryAttempt = attempt + 1; + prepared.deliveryStream = undefined; + this.pendingDeliveries.delete(prepared); + this.insertPrepared(binding.buffer, prepared); + return; + } + prepared.deliveryStream = undefined; + this.settlePrepared(prepared, outcome); + } + + private reservePreparedForBuffer( + prepared: PreparedFrame, + binding?: SessionBinding, + ): boolean { + if (prepared.lease) return true; + if ( + (binding ? binding.ownedFrames : this.connOwnedFrames) >= + ACP_PRE_ATTACH_MAX_FRAMES_PER_STREAM + ) { + this.settlePrepared(prepared, 'failed'); + this.failOwner(binding, 'ACP pre-attach frame limit'); + return false; + } + if ( + this.ownedFrames >= this.maxFramesPerConnection || + prepared.payload.byteLength > + this.maxPayloadBytesPerConnection - this.ownedBytes + ) { + this.settlePrepared(prepared, 'failed'); + this.failConnection('ACP pre-attach connection budget'); + return false; + } + const lease = this.preAttachBudget.tryReserve(prepared.payload.byteLength); + if (!lease) { + this.settlePrepared(prepared, 'failed'); + this.failConnection('ACP pre-attach daemon budget', false); + return false; + } + lease.markPendingDelivery(); + prepared.lease = lease; + this.ownedFrames += 1; + this.ownedBytes += prepared.payload.byteLength; + if (binding) { + binding.ownedFrames += 1; + binding.ownedBytes += prepared.payload.byteLength; + } else { + this.connOwnedFrames += 1; + } + return true; + } + + private insertPrepared( + buffer: PreparedFrame[], + prepared: PreparedFrame, + ): void { + const index = buffer.findIndex( + (buffered) => buffered.sequence > prepared.sequence, + ); + if (index === -1) buffer.push(prepared); + else buffer.splice(index, 0, prepared); + } + + private discardPrepared(prepared: PreparedFrame): void { + this.discardPreparedReceipt(prepared); + if (!this.pendingDeliveries.has(prepared)) { + this.settlePrepared(prepared, 'closed'); + } + } + + private settlePrepared( + prepared: PreparedFrame, + outcome: DeliveryResult, + ): void { + const lease = prepared.lease; + lease?.release(); + prepared.lease = undefined; + this.pendingDeliveries.delete(prepared); + if (lease) { + this.ownedFrames -= 1; + this.ownedBytes -= prepared.payload.byteLength; + if (prepared.binding) { + prepared.binding.ownedFrames -= 1; + prepared.binding.ownedBytes -= prepared.payload.byteLength; + } else { + this.connOwnedFrames -= 1; + } + } + if (!prepared.receiptSettled) { + prepared.receiptSettled = true; + if (prepared.receipt) { + this.invokeReceipt( + prepared.receipt, + outcome === 'delivered' + ? 'delivered' + : outcome === 'outcome_unknown' + ? 'outcomeUnknown' + : 'discarded', + ); + } + } + prepared.resolve(outcome); + } + + private discardPreparedReceipt(prepared: PreparedFrame): void { + if (prepared.receiptSettled) return; + prepared.receiptSettled = true; + if (prepared.receipt) this.invokeReceipt(prepared.receipt, 'discarded'); + } + + private invokeReceipt( + receipt: DeliveryReceipt, + outcome: 'delivered' | 'discarded' | 'outcomeUnknown', + ): void { + try { + if (outcome === 'outcomeUnknown') { + (receipt.outcomeUnknown ?? receipt.delivered)(); + } else { + receipt[outcome](); + } + } catch { + writeStderrLine(`qwen serve: /acp ${outcome} receipt callback failed`); + } + } + + private failOwner( + binding: SessionBinding | undefined, + message: string, + ): void { + this.preAttachBudget.recordGuardFailure(); + this.onPreAttachGuardFailure?.(); + writeStderrLine(`qwen serve: /acp resource guard: ${message}`); + if ( + !binding || + binding.usesConnectionStream === true || + (binding.usesConnectionStream === undefined && + binding.stream === undefined && + this.connStream?.kind === 'ws') + ) { + this.retireConnection(message); + return; + } + try { + this.closeSessionStream(binding.sessionId); + } catch (error) { + writeStderrLine( + `qwen serve: /acp resource guard session teardown failed: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + + private failSerializationOwner(binding: SessionBinding | undefined): void { + writeStderrLine('qwen serve: /acp response serialization failed'); + if (binding) this.failOwner(binding, 'ACP response serialization failed'); + } + + private failConnection(message: string, recordFailure = true): void { + if (recordFailure) this.preAttachBudget.recordGuardFailure(); + this.onPreAttachGuardFailure?.(); + writeStderrLine(`qwen serve: /acp resource guard: ${message}`); + this.retireConnection(message); + } + + private retireConnection(_message: string): void { + const reason = { code: 1013, reason: 'Resource limit' }; + if (this.onFatalConnection) this.onFatalConnection(this, reason); + else this.destroy(reason); + } + /** * Cancel + drop any pending agent→client requests for a closing session. * This is the LAST-RESORT recovery path: `resolveClientResponse` retains a @@ -833,57 +1473,6 @@ export class AcpConnection { } } -function pushCapped( - buf: T[], - frame: T, - label = 'stream', - getId?: (entry: T) => number | undefined, -): void { - if (buf.length >= MAX_BUFFERED_FRAMES) { - // Prefer evicting a REPLAYABLE id-bearing frame over an irreplaceable - // id-less one. On the session buffer, id-bearing entries are EventBus - // events the ring redelivers on reconnect, while id-less entries are - // deferred JSON-RPC replies (`sendSessionReply`) the ring does NOT track — - // dropping one would hang the `session/prompt` caller forever. So under a - // content flood during a detach gap, evict the oldest id-bearing frame and - // keep the reply. - const replayable = getId ? buf.findIndex((e) => getId(e) !== undefined) : 0; - if (replayable === -1) { - // Degenerate case: the buffer is ENTIRELY id-less deferred replies, so - // there is nothing replaceable to evict. Dropping one would silently hang - // its caller (the exact failure this guard prevents), so we do NOT drop at - // the soft cap — id-less replies are bounded in practice by in-flight RPC - // count. But enforce a HARD ceiling as defense-in-depth: past it, an - // unbounded daemon heap is the worse failure, so drop the oldest and log - // loudly. - if (buf.length >= HARD_BUFFERED_FRAMES_CAP) { - buf.shift(); - writeStderrLine( - `qwen serve: /acp HARD buffer cap breached (${label}) — dropping ` + - `oldest id-less reply (its caller may hang); buffer was ${ - buf.length + 1 - }`, - ); - } else if (buf.length === MAX_BUFFERED_FRAMES) { - // Log ONCE, at the soft-cap transition — not on every subsequent push, - // which would scale linearly with the over-cap depth. - writeStderrLine( - `qwen serve: /acp pre-attach buffer over soft cap (${label}) — ` + - `id-less replies are irreplaceable, not dropping`, - ); - } - } else { - const [dropped] = buf.splice(replayable, 1); - const droppedId = getId?.(dropped); - writeStderrLine( - `qwen serve: /acp pre-attach buffer full (${label}), dropped frame` + - (droppedId !== undefined ? ` id ${droppedId}` : ''), - ); - } - } - buf.push(frame); -} - /** * Registry of live ACP connections with an idle-TTL sweep. The sweep is * defensive: a well-behaved client `DELETE /acp`s, but a crashed client @@ -892,12 +1481,16 @@ function pushCapped( export class ConnectionRegistry { private readonly byId = new Map(); private readonly sweepTimer: ReturnType; + private preAttachGuardFailures = 0; constructor( private readonly onAbandonPending?: AbandonPendingFn, private readonly onDetachSession?: DetachSessionFn, private readonly maxConnections = DEFAULT_MAX_CONNECTIONS, private readonly idleTtlMs = 30 * 60_000, + private readonly preAttachBudget = new AcpPreAttachBudget(), + private readonly maxFramesPerConnection = ACP_PRE_ATTACH_MAX_FRAMES_PER_CONNECTION, + private readonly maxPayloadBytesPerConnection = ACP_PRE_ATTACH_MAX_PAYLOAD_BYTES_PER_CONNECTION, ) { this.sweepTimer = setInterval(() => this.sweep(), 60_000); this.sweepTimer.unref(); @@ -917,6 +1510,13 @@ export class ConnectionRegistry { fromLoopback, this.onAbandonPending, this.onDetachSession, + this.preAttachBudget, + (failed, reason) => this.deleteExact(failed, reason), + this.maxFramesPerConnection, + this.maxPayloadBytesPerConnection, + () => { + this.preAttachGuardFailures += 1; + }, ); this.byId.set(conn.connectionId, conn); return conn; @@ -990,8 +1590,17 @@ export class ConnectionRegistry { delete(connectionId: string): boolean { const conn = this.byId.get(connectionId); if (!conn) return false; - conn.destroy(); - return this.byId.delete(connectionId); + return this.deleteExact(conn); + } + + private deleteExact( + conn: AcpConnection, + reason?: TransportCloseReason, + ): boolean { + if (this.byId.get(conn.connectionId) !== conn) return false; + this.byId.delete(conn.connectionId); + conn.destroy(reason); + return true; } get size(): number { @@ -1022,6 +1631,27 @@ export class ConnectionRegistry { connections, (conn) => conn.pendingClientRequests, ), + bufferedConnectionFrames: sumBy( + connections, + (conn) => conn.bufferedConnectionFrames, + ), + bufferedSessionFrames: sumBy( + connections, + (conn) => conn.bufferedSessionFrames, + ), + pendingDeliveryFrames: sumBy( + connections, + (conn) => conn.pendingDeliveryFrames, + ), + preAttachOwnedFrames: sumBy( + connections, + (conn) => conn.preAttachOwnedFrames, + ), + preAttachOwnedBytes: sumBy( + connections, + (conn) => conn.preAttachOwnedBytes, + ), + preAttachGuardFailures: this.preAttachGuardFailures, connections, }; } diff --git a/packages/cli/src/serve/acp-http/dispatch.ts b/packages/cli/src/serve/acp-http/dispatch.ts index 84e14a0ace3..13adb3a83ad 100644 --- a/packages/cli/src/serve/acp-http/dispatch.ts +++ b/packages/cli/src/serve/acp-http/dispatch.ts @@ -146,8 +146,10 @@ import { import type { AcpConnection, ConnectionRegistry, + DeliveryReceipt, PendingClientRequestRef, } from './connection-registry.js'; +import type { DeliveryResult } from './transport-stream.js'; import { QWEN_META_KEY, QWEN_METHOD_NS, @@ -349,6 +351,7 @@ const WORKSPACE_GENERATION_MUTATION_METHODS = new Set([ 'session/new', 'session/load', 'session/resume', + 'session/fork', `${QWEN_METHOD_NS}workspace/init`, `${QWEN_METHOD_NS}workspace/trust/request`, `${QWEN_METHOD_NS}workspace/permissions/set`, @@ -1020,12 +1023,93 @@ export class AcpDispatcher { }); } - private killOrphanSession( + private ownershipReceipt( + conn: AcpConnection, sessionId: string, - removePersistedSession = false, - runtime?: AcpSessionRuntimeContext, - ): void { - void this.removeOrphanSession(sessionId, removePersistedSession, runtime); + clientId: string | undefined, + attached: boolean, + runtime: AcpSessionRuntimeContext, + options: { initialReplayPending?: boolean; removePersisted?: boolean } = {}, + ): DeliveryReceipt & { armInitialReplay(): boolean } { + let settled = false; + const ownershipIdentity = conn.captureSessionOwnershipIdentity(sessionId); + const releaseIdentity = () => + conn.releaseSessionOwnershipIdentity(sessionId, ownershipIdentity); + const detachWithoutDeleting = () => { + try { + void runtime.bridge.detachClient(sessionId, clientId).catch(() => {}); + } catch { + // Best-effort settlement; teardown must continue settling receipts. + } + }; + const commit = () => { + settled = true; + const binding = conn.getOrCreateSession(sessionId); + binding.clientId = clientId; + if (options.initialReplayPending) { + conn.markInitialReplayPending(sessionId); + } + conn.ownSession(sessionId); + releaseIdentity(); + }; + const rollback = () => { + if (settled) return; + settled = true; + if (attached) { + try { + void runtime.bridge.detachClient(sessionId, clientId).catch(() => {}); + } catch { + // Best-effort rollback; teardown must continue settling other receipts. + } + } else { + try { + void this.removeOrphanSession( + sessionId, + options.removePersisted === true, + runtime, + ); + } catch { + // Best-effort rollback; teardown must continue settling other receipts. + } + } + releaseIdentity(); + }; + return { + armInitialReplay: () => { + if ( + settled || + !conn.canCommitSessionOwnership(sessionId, ownershipIdentity) + ) { + return false; + } + conn.markInitialReplayPending(sessionId); + return true; + }, + delivered: () => { + if (settled) return; + if (!conn.canCommitSessionOwnership(sessionId, ownershipIdentity)) { + settled = true; + detachWithoutDeleting(); + releaseIdentity(); + return; + } + commit(); + }, + outcomeUnknown: () => { + if (settled) return; + if ( + !conn.destroyed && + conn.canCommitSessionOwnership(sessionId, ownershipIdentity) + ) { + commit(); + } else { + settled = true; + detachWithoutDeleting(); + releaseIdentity(); + } + }, + discarded: rollback, + }; } /** @@ -1515,6 +1599,12 @@ export class AcpDispatcher { return; case 'session/new': { + if (id === undefined) { + writeStderrLine( + 'qwen serve: /acp session/new notification rejected', + ); + return; + } const meta = isObject(params['_meta']) ? params['_meta'] : undefined; const parsedSessionId = parseCallerSuppliedSessionId( meta?.[REQUESTED_SESSION_ID_META_KEY], @@ -1590,77 +1680,67 @@ export class AcpDispatcher { ...source, ...(requestedSessionId ? { sessionId: requestedSessionId } : {}), }); - const rollbackSession = async (): Promise => { - if (session.attached) { - await sessionRuntime.bridge - .detachClient(session.sessionId, session.clientId) - .catch(() => {}); - } else { - await this.removeOrphanSession( + const ownership = this.ownershipReceipt( + conn, + session.sessionId, + session.clientId, + session.attached, + sessionRuntime, + { removePersisted: true }, + ); + try { + assertGenerationOpen?.(); + if ( + requestedSessionId !== undefined && + session.sessionId !== requestedSessionId + ) { + throw new RequestedSessionIdNotHonoredError( + requestedSessionId, session.sessionId, - true, - sessionRuntime, ); } - }; - try { - assertGenerationOpen?.(); - } catch (error) { - await rollbackSession(); - throw error; - } - if ( - requestedSessionId !== undefined && - session.sessionId !== requestedSessionId - ) { - await rollbackSession(); - throw new RequestedSessionIdNotHonoredError( - requestedSessionId, + if (conn.destroyed) { + ownership.discarded(); + return; + } + const configOptions = await this.configOptionsFor( session.sessionId, + sessionRuntime.bridge, ); - } - // Teardown raced the spawn: the connection was destroyed while the - // bridge call was in flight, so nothing will tear this session down. - // Kill the orphan (no other client could have attached yet). - if (conn.destroyed) { - this.killOrphanSession(session.sessionId, true, sessionRuntime); - return; - } - const configOptions = await this.configOptionsFor( - session.sessionId, - sessionRuntime.bridge, - ); - try { assertGenerationOpen?.(); + if (conn.destroyed) { + ownership.discarded(); + return; + } + // Build ACP-standard models/modes from configOptions. + // configOptions carry model/mode as category-tagged entries; + // the standard also expects top-level models/modes objects. + const models = this.extractModelState(configOptions); + const modes = this.extractModeState(configOptions); + this.replyOwnership( + conn, + id, + { + sessionId: session.sessionId, + ...(session.sourceType + ? { sourceType: session.sourceType } + : {}), + ...(session.sourceId !== undefined + ? { sourceId: session.sourceId } + : {}), + ...(session.sourcePersisted !== undefined + ? { sourcePersisted: session.sourcePersisted } + : {}), + ...(configOptions ? { configOptions } : {}), + ...(models ? { models } : {}), + ...(modes ? { modes } : {}), + }, + ownership, + ); } catch (error) { - await rollbackSession(); + ownership.discarded(); throw error; } - if (conn.destroyed) { - this.killOrphanSession(session.sessionId, true, sessionRuntime); - return; - } - conn.getOrCreateSession(session.sessionId).clientId = - session.clientId; - conn.ownSession(session.sessionId); - // Build ACP-standard models/modes from configOptions. - // configOptions carry model/mode as category-tagged entries; - // the standard also expects top-level models/modes objects. - const models = this.extractModelState(configOptions); - const modes = this.extractModeState(configOptions); - this.replyConn(conn, id, { - sessionId: session.sessionId, - ...(session.sourceType ? { sourceType: session.sourceType } : {}), - ...(session.sourceId !== undefined - ? { sourceId: session.sourceId } - : {}), - ...(session.sourcePersisted !== undefined - ? { sourcePersisted: session.sourcePersisted } - : {}), - ...(configOptions ? { configOptions } : {}), - ...(models ? { models } : {}), - ...(modes ? { modes } : {}), - }); return; } finally { reservation?.release(); @@ -1669,6 +1749,10 @@ export class AcpDispatcher { case 'session/load': case 'session/resume': { + if (id === undefined) { + writeStderrLine(`qwen serve: /acp ${method} notification rejected`); + return; + } const sessionId = normalizeSessionIdForLookup( String(params['sessionId'] ?? ''), ); @@ -1819,102 +1903,96 @@ export class AcpDispatcher { return session; }, ); - const rollbackRestore = async (): Promise => { - if (restored.attached) { - await sessionRuntime.bridge - .detachClient(sessionId, restored.clientId) - .catch(() => {}); - } else { - await sessionRuntime.bridge - .killSession(sessionId, { requireZeroAttaches: true }) - .catch(() => {}); - } - }; - try { - assertGenerationOpen?.(); - } catch (error) { - await rollbackRestore(); - throw error; - } - // ACP standard: load/resume response includes configOptions + models + modes - const loadConfigOptions = await this.configOptionsFor( + const initialReplayOnDelivery = + method === 'session/load' && !conn.ownsSession(sessionId); + const ownership = this.ownershipReceipt( + conn, sessionId, - sessionRuntime.bridge, + restored.clientId, + restored.attached, + sessionRuntime, + { initialReplayPending: initialReplayOnDelivery }, ); - const loadModels = this.extractModelState(loadConfigOptions); - const loadModes = this.extractModeState(loadConfigOptions); - const loadState = restored.state ?? {}; - const loadMeta = isObject(loadState._meta) - ? loadState._meta - : undefined; - const loadQwenMeta = isObject(loadMeta?.[QWEN_META_KEY]) - ? loadMeta[QWEN_META_KEY] - : undefined; - const replayStatus = - method === 'session/load' && restored.partial === true - ? { - partial: true as const, - ...(typeof restored.replayError === 'string' - ? { replayError: restored.replayError } - : {}), - } - : undefined; try { assertGenerationOpen?.(); - } catch (error) { - await rollbackRestore(); - throw error; - } - // Teardown raced the restore — EITHER the whole connection was - // destroyed (`conn.destroyed`) OR a `session/close` for this id - // started while the restore response was being assembled. Cleanup - // depends on what restore did: an attach is rolled back, while a - // freshly restored session must be killed by the spawn owner. - const closeRaced = conn.closingSessions.has(sessionId); - if (conn.destroyed || closeRaced) { - void rollbackRestore().catch((err) => - writeStderrLine( - `qwen serve: /acp orphan ${restored.attached ? 'detach' : 'kill'}(${logSafe(sessionId)}) teardown-race: ${logSafe(errMsg(err))}`, - ), + // ACP standard: load/resume response includes configOptions + models + modes + const loadConfigOptions = await this.configOptionsFor( + sessionId, + sessionRuntime.bridge, ); - // Connection-still-alive close race → tell the client to retry. - // Same rationale as the pre-await guard: a transient server-side - // race, so INTERNAL_ERROR (-32603), not INVALID_PARAMS. - if (closeRaced && !conn.destroyed && id !== undefined) { - conn.sendConn( - error( - id, - RPC.INTERNAL_ERROR, - `session ${sessionId} was closed during load; retry`, - ), - ); + const loadModels = this.extractModelState(loadConfigOptions); + const loadModes = this.extractModeState(loadConfigOptions); + const loadState = restored.state ?? {}; + const loadMeta = isObject(loadState._meta) + ? loadState._meta + : undefined; + const loadQwenMeta = isObject(loadMeta?.[QWEN_META_KEY]) + ? loadMeta[QWEN_META_KEY] + : undefined; + const replayStatus = + method === 'session/load' && restored.partial === true + ? { + partial: true as const, + ...(typeof restored.replayError === 'string' + ? { replayError: restored.replayError } + : {}), + } + : undefined; + assertGenerationOpen?.(); + // Teardown raced the restore — the connection was destroyed, a + // close is in flight, or the previously-owned binding was + // replaced while the restore response was being assembled. + const closeRaced = conn.closingSessions.has(sessionId); + const replayArmRaced = + !conn.destroyed && + !closeRaced && + method === 'session/load' && + !initialReplayOnDelivery && + !ownership.armInitialReplay(); + if (conn.destroyed || closeRaced || replayArmRaced) { + ownership.discarded(); + // Connection-still-alive close race → tell the client to retry. + // Same rationale as the pre-await guard: a transient server-side + // race, so INTERNAL_ERROR (-32603), not INVALID_PARAMS. + if ((closeRaced || replayArmRaced) && !conn.destroyed) { + conn.sendConn( + error( + id, + RPC.INTERNAL_ERROR, + `session ${sessionId} was closed during load; retry`, + ), + ); + } + return; } - return; - } - conn.getOrCreateSession(sessionId).clientId = restored.clientId; - if (method === 'session/load') { - conn.markInitialReplayPending(sessionId); + this.replyOwnership( + conn, + id, + { + ...loadState, + ...(replayStatus + ? { + _meta: { + ...(loadMeta ?? {}), + [QWEN_META_KEY]: { + ...(loadQwenMeta ?? {}), + sessionLoadReplay: replayStatus, + }, + }, + } + : {}), + ...(loadConfigOptions + ? { configOptions: loadConfigOptions } + : {}), + ...(loadModels ? { models: loadModels } : {}), + ...(loadModes ? { modes: loadModes } : {}), + }, + ownership, + ); + } catch (error) { + ownership.discarded(); + throw error; } - conn.ownSession(sessionId); - this.replyConn(conn, id, { - ...loadState, - ...(replayStatus - ? { - _meta: { - ...(loadMeta ?? {}), - [QWEN_META_KEY]: { - ...(loadQwenMeta ?? {}), - sessionLoadReplay: replayStatus, - }, - }, - } - : {}), - ...(loadConfigOptions - ? { configOptions: loadConfigOptions } - : {}), - ...(loadModels ? { models: loadModels } : {}), - ...(loadModes ? { modes: loadModes } : {}), - }); return; } finally { reservation.release(); @@ -2119,6 +2197,12 @@ export class AcpDispatcher { // ACP standard: session/fork — create a branched copy of an existing // session. Maps to bridge.branchSession(). case 'session/fork': { + if (id === undefined) { + writeStderrLine( + 'qwen serve: /acp session/fork notification rejected', + ); + return; + } if (this.liveSessionIsolation) { if (id !== undefined) { conn.sendConn( @@ -2142,8 +2226,10 @@ export class AcpDispatcher { return; } await this.withMutableOwned(conn, sessionId, id, async () => { + const sessionRuntime = this.getSessionRuntimeContext(); const ctx = this.sessionCtx(conn, sessionId, loopback); - const result = (await this.bridge.branchSession( + assertGenerationOpen?.(); + const result = (await sessionRuntime.bridge.branchSession( sessionId, { name: @@ -2153,31 +2239,42 @@ export class AcpDispatcher { }, ctx, )) as BridgeBranchedSession; - if (conn.destroyed) { - const cleanup = result.attached - ? this.bridge.detachClient(result.sessionId, result.clientId) - : this.bridge.killSession(result.sessionId, { - requireZeroAttaches: true, - }); - void cleanup.catch((err) => - writeStderrLine( - `qwen serve: /acp orphan ${result.attached ? 'detach' : 'kill'}(${logSafe(result.sessionId)}) fork-race: ${logSafe(errMsg(err))}`, - ), + const ownership = this.ownershipReceipt( + conn, + result.sessionId, + result.clientId, + result.attached, + sessionRuntime, + { removePersisted: true }, + ); + try { + assertGenerationOpen?.(); + if (conn.destroyed) { + ownership.discarded(); + return; + } + const configOptions = await this.configOptionsFor( + result.sessionId, + sessionRuntime.bridge, ); - return; + assertGenerationOpen?.(); + const models = this.extractModelState(configOptions); + const modes = this.extractModeState(configOptions); + this.replyOwnership( + conn, + id, + { + sessionId: result.sessionId, + ...(configOptions ? { configOptions } : {}), + ...(models ? { models } : {}), + ...(modes ? { modes } : {}), + }, + ownership, + ); + } catch (error) { + ownership.discarded(); + throw error; } - conn.getOrCreateSession(result.sessionId).clientId = - result.clientId; - conn.ownSession(result.sessionId); - const configOptions = await this.configOptionsFor(result.sessionId); - const models = this.extractModelState(configOptions); - const modes = this.extractModeState(configOptions); - this.replyConn(conn, id, { - sessionId: result.sessionId, - ...(configOptions ? { configOptions } : {}), - ...(models ? { models } : {}), - ...(modes ? { modes } : {}), - }); }); return; } @@ -4984,11 +5081,12 @@ export class AcpDispatcher { }; // A permission request MUST reach a LIVE session stream. Going // through `sendSession` would (a) silently drop the frame if the - // session was torn down (lookup-only), or (b) buffer it pre-attach - // where `pushCapped` could evict it under event throughput — either - // way the `pending` entry is orphaned and the agent's prompt blocks - // on a vote forever. So deliver DIRECTLY to a live stream, and if - // there is none, cancel (deny-safe) rather than register+stall. + // session was torn down (lookup-only), or (b) put it behind the + // pre-attach resource guard, where owner teardown cannot preserve a + // newly registered pending vote. Either way the `pending` entry could + // outlive its delivery path and block the agent forever. So deliver + // DIRECTLY to a live stream, and if there is none, cancel (deny-safe) + // rather than register+stall. const binding = conn.sessions.get(sessionId); if (!binding?.stream || binding.stream.isClosed) { // KNOWN GAP (tracked as the §1.7 cross-connection permission @@ -5257,9 +5355,33 @@ export class AcpDispatcher { conn: AcpConnection, id: JsonRpcId | undefined, result: unknown, + receipt?: DeliveryReceipt, + ): Promise { + if (id === undefined) { + receipt?.discarded(); + return Promise.resolve('closed'); + } + const delivery = conn.sendConn(success(id, result), receipt); + void delivery.then( + (outcome) => { + if (outcome === 'failed' && !conn.destroyed) { + void conn.sendConn( + error(id, RPC.INTERNAL_ERROR, 'Response delivery failed'), + ); + } + }, + () => undefined, + ); + return delivery; + } + + private replyOwnership( + conn: AcpConnection, + id: JsonRpcId, + result: unknown, + receipt: DeliveryReceipt, ): void { - if (id === undefined) return; - conn.sendConn(success(id, result)); + void this.replyConn(conn, id, result, receipt); } private replySession( @@ -5302,7 +5424,20 @@ export class AcpDispatcher { logSafe(err instanceof Error ? err.message : String(err)), ); } - conn.sendSessionReply(sessionId, frame, anchorId); + const delivery = conn.sendSessionReply(sessionId, frame, anchorId); + void delivery.then( + (outcome) => { + if ( + (outcome === 'failed' || outcome === 'closed') && + !conn.destroyed + ) { + void conn.sendConn( + error(id, RPC.INTERNAL_ERROR, 'Response delivery failed'), + ); + } + }, + () => undefined, + ); } else { // Fallback fired — log it so an operator can correlate "reply arrived on // the connection stream, not the session stream" with a mid-flight diff --git a/packages/cli/src/serve/acp-http/index.ts b/packages/cli/src/serve/acp-http/index.ts index 3a76a01263c..62af241de3a 100644 --- a/packages/cli/src/serve/acp-http/index.ts +++ b/packages/cli/src/serve/acp-http/index.ts @@ -38,6 +38,12 @@ import { type AcpConnection, type AcpConnectionDiagnostic, } from './connection-registry.js'; +import { + ACP_PRE_ATTACH_MAX_FRAMES_GLOBAL, + ACP_PRE_ATTACH_MAX_PAYLOAD_BYTES_GLOBAL, + AcpPreAttachBudget, + type AcpPreAttachBudgetSnapshot, +} from './pre-attach-budget.js'; import { SseStream } from './sse-stream.js'; import { WsStream } from './ws-stream.js'; import type { RateLimitTier } from '../rate-limit.js'; @@ -526,6 +532,7 @@ export interface AcpHttpMountSnapshot { primary: boolean; connectionCount: number; wsStreams: number; + preAttachGuardFailures: number; } export interface AcpHttpConnectionDiagnostic extends AcpConnectionDiagnostic { @@ -542,6 +549,10 @@ export interface AcpHttpSnapshot { sseStreams: number; wsStreams: number; pendingClientRequests: number; + bufferedConnectionFrames: number; + bufferedSessionFrames: number; + pendingDeliveryFrames: number; + preAttach: AcpPreAttachBudgetSnapshot; mounts: AcpHttpMountSnapshot[]; connections: AcpHttpConnectionDiagnostic[]; } @@ -623,6 +634,10 @@ export function mountAcpHttp( ? runtimeEffectiveEnv(opts.workspaceRegistry.primary, daemonEnv) : daemonEnv; const path = opts.path ?? '/acp'; + const preAttachBudget = new AcpPreAttachBudget({ + maxFrames: ACP_PRE_ATTACH_MAX_FRAMES_GLOBAL, + maxBytes: ACP_PRE_ATTACH_MAX_PAYLOAD_BYTES_GLOBAL, + }); const dispatcherRef: { current?: AcpDispatcher } = {}; // Lifecycle gate: once `dispose()` runs, late/in-flight HTTP requests get a // 503 instead of racing torn-down registries (issue #6378 daemon shutdown). @@ -678,6 +693,8 @@ export function mountAcpHttp( }); }, opts.maxConnections, + undefined, + preAttachBudget, ); let cdpMcpRegistered = false; let cdpMcpRegistering: Promise | undefined; @@ -1295,6 +1312,8 @@ export function mountAcpHttp( }); }, opts.maxConnections, + undefined, + preAttachBudget, ); const workspaceRememberLane = new WorkspaceRememberTaskLane( rt.bridge, @@ -2457,6 +2476,7 @@ export function mountAcpHttp( snap: mount.registry.getSnapshot(), }); } + const preAttach = preAttachBudget.snapshot(); return { connectionCount: perMount.reduce( (n, m) => n + m.snap.connectionCount, @@ -2473,11 +2493,22 @@ export function mountAcpHttp( (n, m) => n + m.snap.pendingClientRequests, 0, ), + bufferedConnectionFrames: perMount.reduce( + (n, m) => n + m.snap.bufferedConnectionFrames, + 0, + ), + bufferedSessionFrames: perMount.reduce( + (n, m) => n + m.snap.bufferedSessionFrames, + 0, + ), + pendingDeliveryFrames: preAttach.pendingDeliveryFrames, + preAttach, mounts: perMount.map((m) => ({ workspaceId: m.workspaceId, primary: m.primary, connectionCount: m.snap.connectionCount, wsStreams: m.snap.wsStreams, + preAttachGuardFailures: m.snap.preAttachGuardFailures, })), connections: perMount.flatMap((mount) => mount.snap.connections.map((connection) => ({ diff --git a/packages/cli/src/serve/acp-http/pre-attach-budget.test.ts b/packages/cli/src/serve/acp-http/pre-attach-budget.test.ts new file mode 100644 index 00000000000..bf8ef228b73 --- /dev/null +++ b/packages/cli/src/serve/acp-http/pre-attach-budget.test.ts @@ -0,0 +1,68 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { AcpPreAttachBudget } from './pre-attach-budget.js'; + +describe('AcpPreAttachBudget', () => { + it('atomically enforces frame and byte limits', () => { + const byFrames = new AcpPreAttachBudget({ maxFrames: 2, maxBytes: 100 }); + expect(byFrames.tryReserve(10)).toBeDefined(); + expect(byFrames.tryReserve(10)).toBeDefined(); + expect(byFrames.tryReserve(1)).toBeUndefined(); + expect(byFrames.snapshot()).toMatchObject({ + usedFrames: 2, + usedBytes: 20, + guardFailures: 1, + }); + + const byBytes = new AcpPreAttachBudget({ maxFrames: 10, maxBytes: 20 }); + expect(byBytes.tryReserve(20)).toBeDefined(); + expect(byBytes.tryReserve(1)).toBeUndefined(); + expect(byBytes.snapshot()).toMatchObject({ + usedFrames: 1, + usedBytes: 20, + guardFailures: 1, + }); + }); + + it('tracks delivery ownership and releases leases idempotently', () => { + const budget = new AcpPreAttachBudget({ maxFrames: 2, maxBytes: 100 }); + const lease = budget.tryReserve(40); + expect(lease).toBeDefined(); + lease?.markPendingDelivery(); + lease?.markPendingDelivery(); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 1, + usedBytes: 40, + pendingDeliveryFrames: 1, + highWaterFrames: 1, + highWaterBytes: 40, + }); + + lease?.release(); + lease?.release(); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 0, + usedBytes: 0, + pendingDeliveryFrames: 0, + highWaterFrames: 1, + highWaterBytes: 40, + }); + }); + + it('keeps counters unchanged when pending delivery is marked after release', () => { + const budget = new AcpPreAttachBudget({ maxFrames: 1, maxBytes: 100 }); + const lease = budget.tryReserve(40); + lease?.release(); + lease?.markPendingDelivery(); + expect(budget.snapshot()).toMatchObject({ + usedFrames: 0, + usedBytes: 0, + pendingDeliveryFrames: 0, + }); + }); +}); diff --git a/packages/cli/src/serve/acp-http/pre-attach-budget.ts b/packages/cli/src/serve/acp-http/pre-attach-budget.ts new file mode 100644 index 00000000000..7180ac1a34c --- /dev/null +++ b/packages/cli/src/serve/acp-http/pre-attach-budget.ts @@ -0,0 +1,91 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +export const ACP_PRE_ATTACH_MAX_FRAMES_PER_STREAM = 256; +export const ACP_PRE_ATTACH_MAX_FRAMES_PER_CONNECTION = 1024; +export const ACP_PRE_ATTACH_MAX_FRAMES_GLOBAL = 4096; +export const ACP_PRE_ATTACH_MAX_PAYLOAD_BYTES_PER_CONNECTION = 64 * 1024 * 1024; +export const ACP_PRE_ATTACH_MAX_PAYLOAD_BYTES_GLOBAL = 256 * 1024 * 1024; + +export interface AcpPreAttachBudgetSnapshot { + usedFrames: number; + usedBytes: number; + pendingDeliveryFrames: number; + highWaterFrames: number; + highWaterBytes: number; + guardFailures: number; +} + +export interface AcpPreAttachBudgetLimits { + maxFrames: number; + maxBytes: number; +} + +export interface AcpPreAttachLease { + markPendingDelivery(): void; + release(): void; +} + +export class AcpPreAttachBudget { + private usedFrames = 0; + private usedBytes = 0; + private pendingDeliveryFrames = 0; + private highWaterFrames = 0; + private highWaterBytes = 0; + private guardFailures = 0; + + constructor( + readonly limits: AcpPreAttachBudgetLimits = { + maxFrames: ACP_PRE_ATTACH_MAX_FRAMES_GLOBAL, + maxBytes: ACP_PRE_ATTACH_MAX_PAYLOAD_BYTES_GLOBAL, + }, + ) {} + + tryReserve(bytes: number): AcpPreAttachLease | undefined { + if ( + this.usedFrames >= this.limits.maxFrames || + bytes > this.limits.maxBytes - this.usedBytes + ) { + this.guardFailures += 1; + return undefined; + } + this.usedFrames += 1; + this.usedBytes += bytes; + this.highWaterFrames = Math.max(this.highWaterFrames, this.usedFrames); + this.highWaterBytes = Math.max(this.highWaterBytes, this.usedBytes); + let released = false; + let pendingDelivery = false; + return { + markPendingDelivery: () => { + if (released || pendingDelivery) return; + pendingDelivery = true; + this.pendingDeliveryFrames += 1; + }, + release: () => { + if (released) return; + released = true; + if (pendingDelivery) this.pendingDeliveryFrames -= 1; + this.usedFrames -= 1; + this.usedBytes -= bytes; + }, + }; + } + + recordGuardFailure(): void { + this.guardFailures += 1; + } + + snapshot(): AcpPreAttachBudgetSnapshot { + return { + usedFrames: this.usedFrames, + usedBytes: this.usedBytes, + pendingDeliveryFrames: this.pendingDeliveryFrames, + highWaterFrames: this.highWaterFrames, + highWaterBytes: this.highWaterBytes, + guardFailures: this.guardFailures, + }; + } +} diff --git a/packages/cli/src/serve/acp-http/sse-stream.test.ts b/packages/cli/src/serve/acp-http/sse-stream.test.ts index 3a851571434..9196be0257a 100644 --- a/packages/cli/src/serve/acp-http/sse-stream.test.ts +++ b/packages/cli/src/serve/acp-http/sse-stream.test.ts @@ -9,12 +9,17 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import type { Response } from 'express'; import { SseStream } from './sse-stream.js'; +type WriteCallback = (err?: Error | null) => void; + /** * Minimal Express `Response` mock: an EventEmitter with the `write`/`end`/ * header surface `SseStream` touches. `writeBehavior` lets a test force * `res.write` to return false (backpressure) or throw (socket error). */ -function mockRes(writeBehavior?: () => boolean) { +function mockRes( + writeBehavior?: () => boolean, + writeCallbacks?: WriteCallback[], +) { const ee = new EventEmitter() as unknown as Response & { chunks: string[]; ended: boolean; @@ -26,7 +31,7 @@ function mockRes(writeBehavior?: () => boolean) { status: () => unknown; setHeader: () => void; flushHeaders: () => void; - write: (c: string) => boolean; + write: (c: string, callback?: WriteCallback) => boolean; end: () => void; req: EventEmitter; }; @@ -37,13 +42,18 @@ function mockRes(writeBehavior?: () => boolean) { m.setHeader = () => {}; m.flushHeaders = () => {}; m.req = new EventEmitter(); - m.write = (chunk: string) => { + m.write = (chunk: string, callback?: WriteCallback) => { m.chunks.push(chunk); + if (callback) { + if (writeCallbacks) writeCallbacks.push(callback); + else queueMicrotask(callback); + } return writeBehavior ? writeBehavior() : true; }; m.end = () => { m.ended = true; m.writableEnded = true; + ee.emit('finish'); }; return ee as unknown as Response & { chunks: string[]; ended: boolean }; } @@ -177,4 +187,101 @@ describe('SseStream', () => { await p; expect(settled).toBe(true); }); + + it('waits for every write callback before reporting local delivery', async () => { + const callbacks: WriteCallback[] = []; + const res = mockRes(undefined, callbacks); + const s = new SseStream(res); + const delivery = s.sendSerialized(Buffer.from('{"ok":true}')); + let settled = false; + void delivery.then(() => { + settled = true; + }); + + for (let i = 0; i < 3; i++) { + await vi.waitFor(() => expect(callbacks).toHaveLength(1)); + expect(settled).toBe(false); + callbacks.shift()?.(); + } + + await expect(delivery).resolves.toBe('delivered'); + expect(settled).toBe(true); + }); + + it('reports a write callback error as failed delivery', async () => { + const callbacks: WriteCallback[] = []; + const res = mockRes(undefined, callbacks); + const s = new SseStream(res); + const delivery = s.sendSerialized(Buffer.from('{"ok":true}')); + + await vi.waitFor(() => expect(callbacks).toHaveLength(1)); + callbacks.shift()?.(new Error('EPIPE')); + + await expect(delivery).resolves.toBe('failed'); + await vi.waitFor(() => expect(s.isClosed).toBe(true)); + }); + + it('does not retain drain listeners after a synchronous callback error', async () => { + const res = mockRes(() => false); + const writable = res as unknown as { + write: (chunk: string | Buffer, callback?: WriteCallback) => boolean; + }; + writable.write = (_chunk, callback) => { + callback?.(new Error('EPIPE')); + return false; + }; + const s = new SseStream(res); + + await expect(s.sendSerialized(Buffer.from('{"ok":true}'))).resolves.toBe( + 'failed', + ); + expect((res as unknown as EventEmitter).listenerCount('drain')).toBe(0); + }); + + it('settles an active write as closed before its callback arrives', async () => { + const callbacks: WriteCallback[] = []; + const res = mockRes(undefined, callbacks); + const s = new SseStream(res); + const delivery = s.sendSerialized(Buffer.from('{"ok":true}')); + + await vi.waitFor(() => expect(callbacks).toHaveLength(1)); + s.close(); + await expect(delivery).resolves.toBe('closed'); + + callbacks.shift()?.(); + expect(s.isClosed).toBe(true); + }); + + it('reports an accepted complete frame as outcome unknown on close', async () => { + const callbacks: WriteCallback[] = []; + const res = mockRes(undefined, callbacks); + const s = new SseStream(res); + const delivery = s.sendSerialized(Buffer.from('{"ok":true}')); + + for (let index = 0; index < 2; index++) { + await vi.waitFor(() => expect(callbacks).toHaveLength(1)); + callbacks.shift()?.(); + } + await vi.waitFor(() => expect(callbacks).toHaveLength(1)); + expect((res as unknown as { chunks: string[] }).chunks.join('')).toBe( + 'data: {"ok":true}\n\n', + ); + + (res as unknown as EventEmitter).emit('close'); + await expect(delivery).resolves.toBe('outcome_unknown'); + callbacks.shift()?.(); + }); + + it('reports an incomplete accepted frame as closed', async () => { + const callbacks: WriteCallback[] = []; + const res = mockRes(undefined, callbacks); + const s = new SseStream(res); + const delivery = s.sendSerialized(Buffer.from('{"ok":true}')); + + await vi.waitFor(() => expect(callbacks).toHaveLength(1)); + (res as unknown as EventEmitter).emit('close'); + + await expect(delivery).resolves.toBe('closed'); + callbacks.shift()?.(); + }); }); diff --git a/packages/cli/src/serve/acp-http/sse-stream.ts b/packages/cli/src/serve/acp-http/sse-stream.ts index d8d0d4b836d..175cb99e238 100644 --- a/packages/cli/src/serve/acp-http/sse-stream.ts +++ b/packages/cli/src/serve/acp-http/sse-stream.ts @@ -6,6 +6,7 @@ import type { Response } from 'express'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; +import type { DeliveryResult, TransportStream } from './transport-stream.js'; /** * A long-lived Server-Sent-Events writer for the ACP-over-HTTP transport. @@ -22,13 +23,14 @@ import { writeStderrLine } from '../../utils/stdioHelpers.js'; * including the optional ring-buffer `id:` sequencing that drives * `Last-Event-ID` resume (see `docs/design/daemon-acp-http/sse-resumable-stream.md`). */ -export class SseStream { +export class SseStream implements TransportStream { readonly kind = 'sse' as const; private writeChain: Promise = Promise.resolve(); private heartbeat: ReturnType | undefined; private closed = false; private cleanupFn: (() => void) | undefined; + private readonly activeWriteClosers = new Set<() => void>(); constructor( private readonly res: Response, @@ -60,7 +62,9 @@ export class SseStream { this.cleanupFn = () => this.close(); this.res.req.on('close', this.cleanupFn); + this.res.on('close', this.cleanupFn); this.res.on('error', this.cleanupFn); + this.res.on('finish', this.cleanupFn); } /** @@ -71,8 +75,23 @@ export class SseStream { * terminal frames (no bus id), matching REST `formatSseFrame`. */ send(message: unknown, id?: number): Promise { + const payload = Buffer.from(JSON.stringify(message), 'utf8'); + return this.sendSerialized(payload, id).then(() => undefined); + } + + sendSerialized(payload: Buffer, id?: number): Promise { const idLine = id !== undefined ? `id: ${id}\n` : ''; - return this.writeRaw(`${idLine}data: ${JSON.stringify(message)}\n\n`); + return this.enqueueWrite(async () => { + if (this.closed || this.res.writableEnded) return 'closed'; + if (idLine && (await this.doWrite(idLine)) !== 'delivered') { + return 'closed'; + } + if ((await this.doWrite('data: ')) !== 'delivered') return 'closed'; + if ((await this.doWrite(payload)) !== 'delivered') return 'closed'; + const suffix = await this.doWrite('\n\n'); + if (suffix !== 'delivered') return suffix; + return 'delivered'; + }); } get isClosed(): boolean { @@ -82,10 +101,13 @@ export class SseStream { close(): void { if (this.closed) return; this.closed = true; + for (const settle of this.activeWriteClosers) settle(); if (this.heartbeat) clearInterval(this.heartbeat); if (this.cleanupFn) { this.res.req.off('close', this.cleanupFn); + this.res.off('close', this.cleanupFn); this.res.off('error', this.cleanupFn); + this.res.off('finish', this.cleanupFn); this.cleanupFn = undefined; } try { @@ -107,63 +129,101 @@ export class SseStream { } } - private writeRaw(chunk: string): Promise { - const next = this.writeChain.then(() => this.doWrite(chunk)); + private writeRaw(chunk: string | Buffer): Promise { + return this.enqueueWrite(() => this.doWrite(chunk)).then(() => undefined); + } + + private enqueueWrite( + write: () => Promise, + ): Promise { + const run = () => write(); + const next = this.writeChain.then(run, run); // The stream OWNS write-failure handling: callers fire-and-forget // (`void stream.send(...)`), so a broken socket would otherwise leave a // zombie stream (heartbeats firing, no events delivered, no log). On the // first failure, log once and close so the subscription tears down. - this.writeChain = next.catch((err: unknown) => { - if (!this.closed) { - writeStderrLine( - `qwen serve: /acp SSE write failed, closing stream: ${ - err instanceof Error ? err.message : String(err) - }`, - ); - this.close(); - } - return undefined; - }); - return next; + this.writeChain = next + .then(() => undefined) + .catch((err: unknown) => { + if (!this.closed) { + writeStderrLine( + `qwen serve: /acp SSE write failed, closing stream: ${ + err instanceof Error ? err.message : String(err) + }`, + ); + this.close(); + } + return undefined; + }); + return next.catch(() => 'failed'); } - private doWrite(chunk: string): Promise { - return new Promise((resolve, reject) => { + private doWrite(chunk: string | Buffer): Promise { + return new Promise((resolve, reject) => { if (this.closed || this.res.writableEnded) { - resolve(); - return; - } - let ok: boolean; - try { - ok = this.res.write(chunk); - } catch (err) { - reject(err as Error); - return; - } - if (ok) { - resolve(); + resolve('closed'); return; } + let settled = false; + let callbackDone = false; + let drainDone = false; + let writeReturned: boolean | undefined; const cleanup = () => { + this.activeWriteClosers.delete(onCloseEv); this.res.off('drain', onDrain); this.res.off('close', onCloseEv); + this.res.off('finish', onCloseEv); this.res.off('error', onErrorEv); }; - const onDrain = () => { + const settle = (result: DeliveryResult) => { + if (settled) return; + settled = true; cleanup(); - resolve(); + resolve(result); + }; + const finishIfReady = () => { + if (writeReturned !== undefined && callbackDone && drainDone) { + settle('delivered'); + } + }; + const onDrain = () => { + drainDone = true; + finishIfReady(); }; const onCloseEv = () => { - cleanup(); - resolve(); + settle(writeReturned === undefined ? 'closed' : 'outcome_unknown'); }; const onErrorEv = (err: Error) => { + if (settled) return; + settled = true; cleanup(); reject(err); }; - this.res.once('drain', onDrain); this.res.once('close', onCloseEv); + this.res.once('finish', onCloseEv); this.res.once('error', onErrorEv); + this.activeWriteClosers.add(onCloseEv); + try { + writeReturned = this.res.write(chunk, (err?: Error | null) => { + if (err) { + onErrorEv(err); + return; + } + callbackDone = true; + finishIfReady(); + }); + } catch (err) { + onErrorEv(err as Error); + return; + } + if (settled) return; + if (!writeReturned) { + this.res.once('drain', onDrain); + } else { + drainDone = true; + } + finishIfReady(); + if (this.closed || this.res.writableEnded) onCloseEv(); }); } } diff --git a/packages/cli/src/serve/acp-http/transport-stream.ts b/packages/cli/src/serve/acp-http/transport-stream.ts index f3958afeb0a..ddf920aeb38 100644 --- a/packages/cli/src/serve/acp-http/transport-stream.ts +++ b/packages/cli/src/serve/acp-http/transport-stream.ts @@ -8,6 +8,17 @@ * Transport-agnostic stream interface consumed by `AcpConnection`. * Both `SseStream` (HTTP SSE) and `WsStream` (WebSocket) implement this. */ +export type DeliveryResult = + | 'delivered' + | 'outcome_unknown' + | 'closed' + | 'failed'; + +export interface TransportCloseReason { + code: number; + reason: string; +} + export interface TransportStream { readonly kind: 'sse' | 'ws'; /** @@ -17,6 +28,7 @@ export interface TransportStream { * WebSocket transport ignores it (stateful connection, no SSE replay). */ send(message: unknown, id?: number): Promise; - close(): void; + sendSerialized(payload: Buffer, id?: number): Promise; + close(reason?: TransportCloseReason): void; readonly isClosed: boolean; } diff --git a/packages/cli/src/serve/acp-http/transport.test.ts b/packages/cli/src/serve/acp-http/transport.test.ts index d960085af7c..1bbe9d81512 100644 --- a/packages/cli/src/serve/acp-http/transport.test.ts +++ b/packages/cli/src/serve/acp-http/transport.test.ts @@ -191,6 +191,7 @@ class FakeBridge { historyReplay?: string; clientId?: string; }> = []; + resumeRequests: string[] = []; replaySnapshot: SessionReplaySnapshot | undefined; loadState: Record = { replayed: true }; loadPartial: true | undefined; @@ -200,6 +201,7 @@ class FakeBridge { name?: string; clientId?: string; }> = []; + configGate: Promise | undefined; closedSessions: string[] = []; @@ -273,6 +275,7 @@ class FakeBridge { } async resumeSession(req: { sessionId: string }) { + this.resumeRequests.push(req.sessionId); return { sessionId: req.sessionId, workspaceCwd: TEST_WORKSPACE, @@ -360,6 +363,7 @@ class FakeBridge { // Session config options live in the child's session context state. async getSessionContextStatus(sessionId: string) { + if (this.configGate) await this.configGate; return { v: 1, sessionId, @@ -423,6 +427,7 @@ class FakeBridge { } detached: Array<{ sessionId: string; clientId?: string }> = []; + detachThrowsSynchronously = false; async cancelSession(sessionId: string) { this.cancelled.push(sessionId); @@ -434,8 +439,10 @@ class FakeBridge { if (this.closeError) throw this.closeError; if (this.closeShouldThrow) throw new Error('bridge close failed'); } - async detachClient(sessionId: string, clientId?: string) { + detachClient(sessionId: string, clientId?: string): Promise { + if (this.detachThrowsSynchronously) throw new Error('sync detach failed'); this.detached.push({ sessionId, clientId }); + return Promise.resolve(); } async preheat() {} @@ -887,12 +894,12 @@ function frameReader(res: Response) { } async function waitUntil( - predicate: () => boolean, + predicate: () => boolean | Promise, timeoutMs = 2000, ): Promise { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { - if (predicate()) return; + if (await predicate()) return; await new Promise((r) => setTimeout(r, 10)); } throw new Error('Timed out waiting for condition'); @@ -1117,13 +1124,26 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { // Establish ownership of the fake bridge's session ('sess-1') so the // ownership-gated session stream + per-session POSTs are allowed. async function newSession(connId: string, id = 99): Promise { + const conn = acpHandle?.registry.get(connId); + const needsDeliveryStream = + conn?.connStream === undefined || conn.connStream.isClosed; + const deliveryStream = needsDeliveryStream + ? await openStream(connId) + : undefined; + const delivered = deliveryStream + ? takeFrames(deliveryStream, 1) + : undefined; await post(connId, { jsonrpc: '2.0', id, method: 'session/new', params: {}, }); - await new Promise((r) => setTimeout(r, 30)); // let handle() register ownership + if (delivered) { + await delivered; + } else { + await new Promise((r) => setTimeout(r, 30)); + } } async function withRuntimeDir( @@ -1382,6 +1402,142 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { ]); }); + it('does not grant session ownership until the reply is delivered', async () => { + const connId = await initialize(); + await post(connId, { + jsonrpc: '2.0', + id: 200, + method: 'session/new', + params: { cwd: TEST_WORKSPACE }, + }); + await new Promise((resolve) => setTimeout(resolve, 30)); + + const beforeDelivery = await openStream(connId, 'sess-1'); + expect(beforeDelivery.status).toBe(403); + + const connStream = await openStream(connId); + const [reply] = (await takeFrames(connStream, 1)) as Array<{ + id: number; + result: { sessionId: string }; + }>; + expect(reply).toMatchObject({ + id: 200, + result: { sessionId: 'sess-1' }, + }); + + const afterDelivery = await openStream(connId, 'sess-1'); + expect(afterDelivery.status).toBe(200); + await afterDelivery.body?.cancel(); + }); + + it('does not mutate sessions for ownership-granting notifications', async () => { + const connId = await initialize(); + await post(connId, { + jsonrpc: '2.0', + method: 'session/new', + params: { cwd: TEST_WORKSPACE }, + }); + await new Promise((resolve) => setTimeout(resolve, 30)); + expect(bridge.lastSpawnScope).toBeUndefined(); + expect(acpHandle?.registry.get(connId)?.ownedSessions.size).toBe(0); + }); + + it('does not load, resume, or fork for notification forms', async () => { + const connId = await initialize(); + await newSession(connId); + + await post(connId, { + jsonrpc: '2.0', + method: 'session/load', + params: { sessionId: 'loaded-1' }, + }); + await post(connId, { + jsonrpc: '2.0', + method: 'session/resume', + params: { sessionId: 'resumed-1' }, + }); + await post(connId, { + jsonrpc: '2.0', + method: 'session/fork', + params: { sessionId: 'sess-1' }, + }); + await new Promise((resolve) => setTimeout(resolve, 30)); + + expect(bridge.loadRequests).toEqual([]); + expect(bridge.resumeRequests).toEqual([]); + expect(bridge.branchRequests).toEqual([]); + expect(acpHandle?.registry.get(connId)?.ownedSessions).toEqual( + new Set(['sess-1']), + ); + }); + + it('rolls back an undelivered fresh session when the connection closes', async () => { + const connId = await initialize(); + await post(connId, { + jsonrpc: '2.0', + id: 201, + method: 'session/new', + params: { cwd: TEST_WORKSPACE }, + }); + await new Promise((resolve) => setTimeout(resolve, 30)); + await fetch(`${base}/acp`, { + method: 'DELETE', + headers: { 'acp-connection-id': connId }, + }); + await waitUntil(() => bridge.killed.includes('sess-1')); + expect(acpHandle?.registry.get(connId)).toBeUndefined(); + }); + + it('continues connection teardown when provisional detach throws synchronously', async () => { + bridge.spawnAttached = true; + bridge.detachThrowsSynchronously = true; + const connId = await initialize(); + await post(connId, { + jsonrpc: '2.0', + id: 203, + method: 'session/new', + params: { cwd: TEST_WORKSPACE }, + }); + await new Promise((resolve) => setTimeout(resolve, 30)); + + const response = await fetch(`${base}/acp`, { + method: 'DELETE', + headers: { 'acp-connection-id': connId }, + }); + + expect(response.status).toBe(202); + expect(acpHandle?.registry.get(connId)).toBeUndefined(); + }); + + it('removes an undelivered persistent fork when the connection closes', async () => { + const connId = await initialize(); + await newSession(connId); + await writeStoredSession('branch-1'); + const connStream = acpHandle?.registry.get(connId)?.connStream; + connStream?.close(); + + await post(connId, { + jsonrpc: '2.0', + id: 202, + method: 'session/fork', + params: { sessionId: 'sess-1' }, + }); + await new Promise((resolve) => setTimeout(resolve, 30)); + await fetch(`${base}/acp`, { + method: 'DELETE', + headers: { 'acp-connection-id': connId }, + }); + + await waitUntil(() => bridge.killed.includes('branch-1')); + await waitUntil( + async () => + (await new SessionService(TEST_WORKSPACE).getSessionLocation( + 'branch-1', + )) === undefined, + ); + expect(acpHandle?.registry.get(connId)).toBeUndefined(); + }); + it('session/new rejects the daemon-owned Live Voice source namespace', async () => { const connId = await initialize(); const connStream = await openStream(connId); @@ -2117,7 +2273,6 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { const voterConnStream = await openStream(voterConnId); const voterReader = frameReader(voterConnStream); try { - await voterReader.next(); // buffered session/new response on B await post(streamConnId, { jsonrpc: '2.0', id: 7, @@ -2205,7 +2360,6 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { const sessStream = await openStream(connId, 'sess-1'); const sessReader = frameReader(sessStream); try { - await connReader.next(); // buffered session/new response await post(connId, { jsonrpc: '2.0', id: 43, @@ -2269,7 +2423,6 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { const sessStream = await openStream(connId, 'sess-1'); const sessReader = frameReader(sessStream); try { - await connReader.next(); // buffered session/new response await post(connId, { jsonrpc: '2.0', id: 51, @@ -2332,7 +2485,6 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { const sessStream = await openStream(connId, 'sess-1'); const sessReader = frameReader(sessStream); try { - await connReader.next(); // buffered session/new response // Round 1: object _meta is preserved verbatim (nested shape survives). await post(connId, { jsonrpc: '2.0', @@ -2424,7 +2576,6 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { const sessStream = await openStream(connId, 'sess-1'); const sessReader = frameReader(sessStream); try { - await connReader.next(); // buffered session/new response await post(connId, { jsonrpc: '2.0', id: 49, @@ -2552,7 +2703,6 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { const sessStream = await openStream(connId, 'sess-1'); const sessReader = frameReader(sessStream); try { - await connReader.next(); // buffered session/new response await post(connId, { jsonrpc: '2.0', id: 7, @@ -2674,7 +2824,6 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { const sessStream = await openStream(connId, 'sess-1'); const sessReader = frameReader(sessStream); try { - await connReader.next(); // buffered session/new response await post(connId, { jsonrpc: '2.0', id: 22, @@ -2728,7 +2877,6 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { const sessStream = await openStream(connId, 'sess-1'); const sessReader = frameReader(sessStream); try { - await connReader.next(); // buffered session/new response await post(connId, { jsonrpc: '2.0', id: 24, @@ -2819,7 +2967,6 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { const sessStream = await openStream(connId, 'sess-1'); const sessReader = frameReader(sessStream); try { - await connReader.next(); // buffered session/new response await post(connId, { jsonrpc: '2.0', id: 30, @@ -2892,7 +3039,6 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { const connStream = await openStream(connId); const connReader = frameReader(connStream); try { - await connReader.next(); // buffered session/new response await post(connId, { jsonrpc: '2.0', id: 32, @@ -2943,7 +3089,6 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { const sessStream = await openStream(connId, 'sess-1'); const sessReader = frameReader(sessStream); try { - await connReader.next(); // buffered session/new response await post(connId, { jsonrpc: '2.0', id: 33, @@ -3014,7 +3159,6 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { const sessStream = await openStream(connId, 'sess-1'); const sessReader = frameReader(sessStream); try { - await connReader.next(); // buffered session/new response await post(connId, { jsonrpc: '2.0', id: 35, @@ -3104,7 +3248,6 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { const sessStream = await openStream(connId, 'sess-1'); const sessReader = frameReader(sessStream); try { - await connReader.next(); // buffered session/new response await post(connId, { jsonrpc: '2.0', id: 37, @@ -3657,6 +3800,129 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { }, ); + it.each(['live', 'buffered'] as const)( + 'session/load returns an internal error when its %s response cannot be serialized', + async (mode) => { + bridge.loadState = { value: 1n }; + const connId = await initialize(); + const connStream = mode === 'live' ? await openStream(connId) : undefined; + + await post(connId, { + jsonrpc: '2.0', + id: 21, + method: 'session/load', + params: { sessionId: 'loaded-1' }, + }); + + const stream = connStream ?? (await openStream(connId)); + const reader = frameReader(stream); + try { + const failure = (await reader.next()) as { + id: number; + error: { code: number; message: string }; + }; + expect(failure).toMatchObject({ + id: 21, + error: { + code: -32603, + message: 'Response delivery failed', + }, + }); + expect(acpHandle?.registry.get(connId)?.destroyed).toBe(false); + expect(acpHandle?.registry.get(connId)?.ownsSession('loaded-1')).toBe( + false, + ); + expect(bridge.detached).toContainEqual({ + sessionId: 'loaded-1', + clientId: 'client-load', + }); + + await post(connId, { + jsonrpc: '2.0', + id: 22, + method: 'authenticate', + }); + await expect(reader.next()).resolves.toMatchObject({ + id: 22, + result: {}, + }); + } finally { + reader.close(); + } + }, + ); + + it('answers session replies on the connection stream when the pre-attach queue overflows', async () => { + const connId = await initialize(); + const connStream = await openStream(connId); + const reader = frameReader(connStream); + try { + await post(connId, { + jsonrpc: '2.0', + id: 899, + method: 'session/new', + params: {}, + }); + await expect(reader.next()).resolves.toMatchObject({ id: 899 }); + + const promptIds = Array.from({ length: 257 }, (_, index) => 900 + index); + for (const [index, id] of promptIds.entries()) { + const response = await post(connId, { + jsonrpc: '2.0', + id, + method: 'session/prompt', + params: { + sessionId: 'sess-1', + prompt: [{ type: 'text', text: 'fill detached reply queue' }], + }, + }); + expect(response.status).toBe(202); + if (index < 256) { + await waitUntil( + () => acpHandle?.getSnapshot().bufferedSessionFrames === index + 1, + ); + } + } + + const replies: Array<{ + id: number; + error: { code: number; message: string }; + }> = []; + for (let index = 0; index < promptIds.length; index += 1) { + replies.push( + (await reader.next(5000)) as { + id: number; + error: { code: number; message: string }; + }, + ); + } + + expect( + replies + .map(({ id, error }) => ({ id, error })) + .sort((a, b) => a.id - b.id), + ).toEqual( + promptIds.map((id) => ({ + id, + error: { + code: -32603, + message: 'Response delivery failed', + }, + })), + ); + const conn = acpHandle?.registry.get(connId); + expect(conn?.destroyed).toBe(false); + expect(conn?.sessions.has('sess-1')).toBe(false); + expect(acpHandle?.getSnapshot().preAttach).toMatchObject({ + usedFrames: 0, + usedBytes: 0, + pendingDeliveryFrames: 0, + }); + } finally { + reader.close(); + } + }); + it('session/load reports partial replay status under qwen _meta', async () => { bridge.loadState = { replayed: true, @@ -3803,6 +4069,125 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { }); }); + it('arms initial load replay before an already-owned load reply is delivered', async () => { + bridge.replaySnapshot = { + lastEventId: 1, + compactedTurns: [ + { + v: 1, + id: 1, + type: 'session_update', + data: { + sessionId: 'sess-1', + update: { sessionUpdate: 'agent_message_chunk' }, + }, + } as BridgeEvent, + ], + liveJournal: [], + }; + const connId = await initialize(); + const connStream = await openStream(connId); + const initialReply = takeFrames(connStream, 1); + await post(connId, { + jsonrpc: '2.0', + id: 19, + method: 'session/new', + params: {}, + }); + await initialReply; + acpHandle?.registry.get(connId)?.connStream?.close(); + + await post(connId, { + jsonrpc: '2.0', + id: 20, + method: 'session/load', + params: { sessionId: 'sess-1' }, + }); + await waitUntil( + () => acpHandle?.getSnapshot().bufferedConnectionFrames === 1, + ); + + const sessionStream = await openStream(connId, 'sess-1'); + const sessionReader = frameReader(sessionStream); + const replayed = (await sessionReader.next(1000)) as { + method: string; + params: { update?: { sessionUpdate?: string } }; + }; + expect(replayed).toMatchObject({ + method: 'session/update', + params: { update: { sessionUpdate: 'agent_message_chunk' } }, + }); + await waitUntil(() => bridge.subscribeCalls.length === 1); + bridge.queues.get('sess-1')!.push({ + type: 'replay_complete', + data: { replayedCount: 0 }, + }); + await expect(sessionReader.next()).resolves.toMatchObject({ + method: '_qwen/notify', + params: { kind: 'replay_complete' }, + }); + + const replacementConnStream = await openStream(connId); + const replacementConnReader = frameReader(replacementConnStream); + await expect(replacementConnReader.next()).resolves.toMatchObject({ + id: 20, + }); + + const secondSessionStream = await openStream(connId, 'sess-1'); + const secondSessionReader = frameReader(secondSessionStream); + await waitUntil(() => bridge.subscribeCalls.length === 2); + expect(bridge.subscribeCalls[1]).toEqual({ sessionId: 'sess-1' }); + sessionReader.close(); + secondSessionReader.close(); + replacementConnReader.close(); + }); + + it('does not arm initial replay on a replacement session generation', async () => { + const connId = await initialize(); + const connStream = await openStream(connId); + const connReader = frameReader(connStream); + await post(connId, { + jsonrpc: '2.0', + id: 19, + method: 'session/new', + params: {}, + }); + await expect(connReader.next()).resolves.toMatchObject({ id: 19 }); + + let releaseConfig!: () => void; + bridge.configGate = new Promise((resolve) => { + releaseConfig = resolve; + }); + await post(connId, { + jsonrpc: '2.0', + id: 20, + method: 'session/load', + params: { sessionId: 'sess-1' }, + }); + await waitUntil(() => bridge.loadRequests.length === 1); + + const conn = acpHandle?.registry.get(connId); + expect(conn).toBeDefined(); + conn!.closeSessionStream('sess-1'); + const replacement = conn!.getOrCreateSession('sess-1'); + replacement.clientId = 'new-generation'; + conn!.ownSession('sess-1'); + releaseConfig(); + + await expect(connReader.next()).resolves.toMatchObject({ + id: 20, + error: { code: -32603 }, + }); + expect(replacement.initialReplayPending).not.toBe(true); + expect(replacement.clientId).toBe('new-generation'); + expect(conn!.ownsSession('sess-1')).toBe(true); + expect(bridge.detached).toContainEqual({ + sessionId: 'sess-1', + clientId: 'client-load', + }); + connReader.close(); + }); + it('emits the stderr breadcrumb only when the initial replay snapshot is degraded', async () => { const makeSnapshot = ( sessionId: string, @@ -5029,9 +5414,9 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { await newSession(connId); const connStream = await openStream(connId); const sessStream = await openStream(connId, 'sess-1'); - // conn stream carries: buffered session/new reply (id 99), the close - // ack (id 91), AND the fallback prompt reply (id 90). - const connFrames = takeFrames(connStream, 3); + // The helper already delivered the session/new ownership grant. This + // stream carries the close ack and fallback prompt reply. + const connFrames = takeFrames(connStream, 2); await new Promise((r) => setTimeout(r, 50)); await post(connId, { jsonrpc: '2.0', @@ -5560,8 +5945,7 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { const connId = await initialize(); await newSession(connId); const connStream = await openStream(connId); - // 4 frames: buffered session/new reply (id 99) + the 3 below. - const got = takeFrames(connStream, 4); + const got = takeFrames(connStream, 3); await new Promise((r) => setTimeout(r, 50)); await post(connId, { jsonrpc: '2.0', 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 87d75ac8670..42fca320370 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 @@ -830,6 +830,75 @@ describe('workspace-qualified ACP (/workspaces/:workspace/acp)', () => { }); }); + it('rolls back session/fork through the bridge generation that created it', async () => { + let releaseFork!: () => void; + const forkGate = new Promise((resolve) => { + releaseFork = resolve; + }); + primaryBridge.branchSession = vi.fn(async (sessionId) => { + await forkGate; + return { + sessionId: 'forked-primary-session', + workspaceCwd: '/ws', + attached: false, + clientId: 'forked-primary-client', + state: {}, + displayName: 'Forked primary session', + forkedFrom: { sessionId, displayName: sessionId }, + }; + }); + const pending = sendWsRequests('/acp', [ + { + jsonrpc: '2.0', + id: 2, + method: 'session/new', + params: { workspaceCwd: '/ws' }, + }, + { + jsonrpc: '2.0', + id: 3, + method: 'session/fork', + params: { sessionId: 'primary-session' }, + }, + ]); + await vi.waitFor(() => + expect(primaryBridge.branchSession).toHaveBeenCalledOnce(), + ); + + const replacementBridge = makeBridge(); + const entry = workspaceRegistry.primaryEntry; + expect(workspaceRegistry.beginReplacement(entry, 'policy-2')).toBe(true); + workspaceRegistry.activateReplacement( + entry, + makeRuntime({ + id: 'primary-id', + cwd: '/ws', + primary: true, + trusted: true, + bridge: replacementBridge, + }), + 'policy-2', + ); + releaseFork(); + + const responses = await pending; + expect(responses[1]).toMatchObject({ + error: { + code: -32603, + data: { + httpStatus: 503, + errorKind: 'workspace_runtime_unavailable', + retryable: true, + }, + }, + }); + expect(primaryBridge.killSession).toHaveBeenCalledWith( + 'forked-primary-session', + { requireZeroAttaches: true }, + ); + expect(replacementBridge.killSession).not.toHaveBeenCalled(); + }); + it('uses the registry generation guard for qualified ACP mounts', async () => { const sessionId = '550e8400-e29b-41d4-a716-446655440184'; let releaseContext!: () => void; diff --git a/packages/cli/src/serve/acp-http/ws-stream.test.ts b/packages/cli/src/serve/acp-http/ws-stream.test.ts index a9dd09428e6..c46dad561a3 100644 --- a/packages/cli/src/serve/acp-http/ws-stream.test.ts +++ b/packages/cli/src/serve/acp-http/ws-stream.test.ts @@ -32,6 +32,19 @@ class MockWebSocket extends EventEmitter { } } +class ControlledWebSocket extends MockWebSocket { + callback?: (err?: Error) => void; + + override send(data: string, ...args: unknown[]) { + this.sent.push(data); + const callback = args.at(-1); + this.callback = + typeof callback === 'function' + ? (callback as (err?: Error) => void) + : undefined; + } +} + describe('WsStream', () => { let ws: MockWebSocket; @@ -82,6 +95,56 @@ describe('WsStream', () => { expect(ws.sent).toEqual([]); }); + it('reports queued sends as closed when the stream is already closed', async () => { + const stream = new WsStream(ws as never); + stream.close(); + + await expect( + stream.sendSerialized(Buffer.from('{"ok":true}')), + ).resolves.toBe('closed'); + expect(ws.sent).toEqual([]); + }); + + it('close() marks an active accepted send outcome unknown', async () => { + const controlled = new ControlledWebSocket(); + const stream = new WsStream(controlled as never); + const delivery = stream.sendSerialized(Buffer.from('{"ok":true}')); + await vi.waitFor(() => expect(controlled.callback).toBeDefined()); + + stream.close(); + await expect(delivery).resolves.toBe('outcome_unknown'); + + controlled.callback?.(); + expect(stream.isClosed).toBe(true); + }); + + it('marks an accepted send outcome unknown when peer loss wins the callback race', async () => { + const controlled = new ControlledWebSocket(); + const stream = new WsStream(controlled as never); + const delivery = stream.sendSerialized(Buffer.from('{"ok":true}')); + await vi.waitFor(() => expect(controlled.callback).toBeDefined()); + + controlled.callback?.(new Error('socket closed')); + await expect(delivery).resolves.toBe('outcome_unknown'); + expect(stream.isClosed).toBe(true); + + controlled.readyState = 3; + controlled.emit('close'); + expect(stream.isClosed).toBe(true); + }); + + it('does not submit a queued send after the socket stops being open', async () => { + const controlled = new ControlledWebSocket(); + controlled.readyState = 3; + const stream = new WsStream(controlled as never); + + await expect( + stream.sendSerialized(Buffer.from('{"ok":true}')), + ).resolves.toBe('closed'); + expect(controlled.sent).toEqual([]); + stream.close(); + }); + it('isClosed starts false, becomes true after close()', () => { const stream = new WsStream(ws as never); expect(stream.isClosed).toBe(false); @@ -99,6 +162,12 @@ describe('WsStream', () => { expect(ws.closeCode).toBe(1000); }); + it('uses the supplied resource-fatal close code', () => { + const stream = new WsStream(ws as never); + stream.close({ code: 1013, reason: 'Resource limit' }); + expect(ws.closeCode).toBe(1013); + }); + it('close() calls onClose callback', () => { const onClose = vi.fn(); const stream = new WsStream(ws as never, onClose); @@ -189,4 +258,48 @@ describe('WsStream', () => { expect(onClose).toHaveBeenCalled(); expect(stream.isClosed).toBe(true); }); + + it('closes after a synchronous send failure when stderr is unavailable', async () => { + const onClose = vi.fn(); + const stderrWrite = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => { + throw new Error('write EPIPE'); + }); + ws.send = () => { + throw new Error('socket send failed'); + }; + const stream = new WsStream(ws as never, onClose); + + try { + await expect( + stream.sendSerialized(Buffer.from('{"fail":true}')), + ).resolves.toBe('failed'); + expect(stream.isClosed).toBe(true); + expect(onClose).toHaveBeenCalledTimes(1); + await expect( + stream.sendSerialized(Buffer.from('{"after":"failure"}')), + ).resolves.toBe('closed'); + } finally { + stderrWrite.mockRestore(); + } + }); + + it('closes after a socket error when stderr is unavailable', () => { + const onClose = vi.fn(); + const stderrWrite = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => { + throw new Error('write EPIPE'); + }); + const stream = new WsStream(ws as never, onClose); + + try { + expect(() => ws.emit('error', new Error('socket failed'))).not.toThrow(); + expect(stream.isClosed).toBe(true); + expect(onClose).toHaveBeenCalledTimes(1); + } finally { + stderrWrite.mockRestore(); + } + }); }); diff --git a/packages/cli/src/serve/acp-http/ws-stream.ts b/packages/cli/src/serve/acp-http/ws-stream.ts index 67a0dda2380..71e2292aa53 100644 --- a/packages/cli/src/serve/acp-http/ws-stream.ts +++ b/packages/cli/src/serve/acp-http/ws-stream.ts @@ -5,8 +5,12 @@ */ import type { WebSocket } from 'ws'; -import { writeStderrLine } from '../../utils/stdioHelpers.js'; -import type { TransportStream } from './transport-stream.js'; +import { writeStderrLineSafe } from '../../utils/stdioHelpers.js'; +import type { + DeliveryResult, + TransportCloseReason, + TransportStream, +} from './transport-stream.js'; export class WsStream implements TransportStream { readonly kind = 'ws' as const; @@ -14,6 +18,7 @@ export class WsStream implements TransportStream { private writeChain: Promise = Promise.resolve(); private _closed = false; private heartbeat: ReturnType | undefined; + private readonly activeSendClosers = new Set<() => void>(); constructor( private readonly ws: WebSocket, @@ -22,10 +27,10 @@ export class WsStream implements TransportStream { ) { ws.on('close', () => this.close()); ws.on('error', (err) => { - writeStderrLine( + this.close(); + writeStderrLineSafe( `qwen serve: /acp WS error: ${err instanceof Error ? err.message : String(err)}`, ); - this.close(); }); let alive = true; ws.on('pong', () => { @@ -57,47 +62,80 @@ export class WsStream implements TransportStream { // (matches `AcpWsTransport.supportsReplay = false`). send(message: unknown, _id?: number): Promise { const data = JSON.stringify(message); - const next = this.writeChain.then( - () => - new Promise((resolve, reject) => { - if (this._closed) { - resolve(); - return; - } - this.ws.send(data, (err) => { - if (err) reject(err); - else resolve(); - }); - }), - ); - this.writeChain = next.catch((err: unknown) => { - if (!this._closed) { - writeStderrLine( - `qwen serve: /acp WS write failed: ${err instanceof Error ? err.message : String(err)}`, - ); + return this.enqueueSend(data).then(() => undefined); + } + + sendSerialized(data: Buffer, _id?: number): Promise { + return this.enqueueSend(data, { binary: false }); + } + + private enqueueSend( + data: string | Buffer, + options?: { binary: boolean }, + ): Promise { + const next = this.writeChain + .then( + () => + new Promise((resolve) => { + if (this._closed) { + resolve('closed'); + return; + } + let settled = false; + const settle = (result: DeliveryResult) => { + if (settled) return; + settled = true; + this.activeSendClosers.delete(onSocketClose); + resolve(result); + }; + const onSocketClose = () => settle('outcome_unknown'); + const callback = (err?: Error) => { + settle(err ? 'outcome_unknown' : 'delivered'); + if (err) this.close(); + }; + if (this.ws.readyState !== this.ws.OPEN) { + settle('closed'); + return; + } + this.activeSendClosers.add(onSocketClose); + try { + if (options) this.ws.send(data, options, callback); + else this.ws.send(data, callback); + } catch { + settle('failed'); + } + }), + ) + .catch(() => 'failed' as const); + this.writeChain = next.then((result) => { + if (result === 'failed' && !this._closed) { this.close(); + writeStderrLineSafe('qwen serve: /acp WS write failed'); } }); - return this.writeChain; + return next; } get isClosed(): boolean { return this._closed; } - close(): void { + close(closeReason?: TransportCloseReason): void { if (this._closed) return; this._closed = true; + for (const settle of this.activeSendClosers) settle(); if (this.heartbeat) clearInterval(this.heartbeat); try { - if (this.ws.readyState === this.ws.OPEN) this.ws.close(1000); + if (this.ws.readyState === this.ws.OPEN) { + this.ws.close(closeReason?.code ?? 1000, closeReason?.reason); + } } catch { /* socket gone */ } try { this.onClose?.(); } catch (err) { - writeStderrLine( + writeStderrLineSafe( `qwen serve: /acp WS onClose threw: ${err instanceof Error ? err.message : String(err)}`, ); } diff --git a/packages/cli/src/serve/daemon-status.test.ts b/packages/cli/src/serve/daemon-status.test.ts index 0b84677d4c8..bc0d7e8fa41 100644 --- a/packages/cli/src/serve/daemon-status.test.ts +++ b/packages/cli/src/serve/daemon-status.test.ts @@ -1009,6 +1009,12 @@ describe('buildDaemonStatusResponse', () => { sseStreams: 1, wsStreams: 0, pendingClientRequests: 0, + bufferedConnectionFrames: 0, + bufferedSessionFrames: 0, + pendingDeliveryFrames: 0, + preAttachOwnedFrames: 0, + preAttachOwnedBytes: 0, + preAttachGuardFailures: 0, connections: [], }, rateLimitHits: { prompt: 1, mutation: 2, read: 3 }, @@ -1043,6 +1049,12 @@ describe('buildDaemonStatusResponse', () => { sseStreams: 0, wsStreams: 1, pendingClientRequests: 0, + bufferedConnectionFrames: 0, + bufferedSessionFrames: 0, + pendingDeliveryFrames: 0, + preAttachOwnedFrames: 0, + preAttachOwnedBytes: 0, + preAttachGuardFailures: 0, connections: [primaryDiagnostic], }; @@ -1057,17 +1069,76 @@ describe('buildDaemonStatusResponse', () => { sseStreams: 0, wsStreams: 2, pendingClientRequests: 0, - mounts: [], + bufferedConnectionFrames: 0, + bufferedSessionFrames: 0, + pendingDeliveryFrames: 1, + preAttach: { + usedFrames: 3, + usedBytes: 4096, + pendingDeliveryFrames: 1, + highWaterFrames: 7, + highWaterBytes: 8192, + guardFailures: 4, + }, + mounts: [ + { + workspaceId: null, + primary: true, + connectionCount: 1, + wsStreams: 1, + preAttachGuardFailures: 1, + }, + { + workspaceId: 'secondary-id', + primary: false, + connectionCount: 1, + wsStreams: 1, + preAttachGuardFailures: 3, + }, + ], connections: [primaryDiagnostic, secondaryDiagnostic], }, }), ); expect(response.runtime.transport.acp.connections).toBe(2); + expect(response.runtime.transport.acp.preAttach).toEqual({ + bufferedConnectionFrames: 0, + bufferedSessionFrames: 0, + pendingDeliveryFrames: 1, + usedFrames: 3, + usedBytes: 4096, + highWaterFrames: 7, + highWaterBytes: 8192, + guardFailures: 4, + }); + expect(response.limits).toMatchObject({ + acpPreAttachMaxFramesPerStream: 256, + acpPreAttachMaxFramesPerConnection: 1024, + acpPreAttachMaxFramesGlobal: 4096, + acpPreAttachMaxPayloadBytesPerConnection: 64 * 1024 * 1024, + acpPreAttachMaxPayloadBytesGlobal: 256 * 1024 * 1024, + }); expect(response.full?.acpConnections).toEqual([ primaryDiagnostic, secondaryDiagnostic, ]); + expect(response.full?.acpMounts).toEqual([ + { + workspaceId: null, + primary: true, + connectionCount: 1, + wsStreams: 1, + preAttachGuardFailures: 1, + }, + { + workspaceId: 'secondary-id', + primary: false, + connectionCount: 1, + wsStreams: 1, + preAttachGuardFailures: 3, + }, + ]); }); it('embeds runtime.metrics.series when getMetricsSeries is provided, and omits it otherwise', async () => { @@ -1836,6 +1907,9 @@ function makeAcpDiagnostic( wsStreams: 1, bufferedConnectionFrames: 0, bufferedSessionFrames: 0, + pendingDeliveryFrames: 0, + preAttachOwnedFrames: 0, + preAttachOwnedBytes: 0, workspaceId, workspaceCwd, primary, @@ -1926,12 +2000,27 @@ function makeOptions(input: MakeOptionsInput = {}): BuildDaemonStatusOptions { sseStreams: input.acpSnapshot!.sseStreams, wsStreams: input.acpSnapshot!.wsStreams, pendingClientRequests: input.acpSnapshot!.pendingClientRequests, + bufferedConnectionFrames: + input.acpSnapshot!.bufferedConnectionFrames, + bufferedSessionFrames: input.acpSnapshot!.bufferedSessionFrames, + pendingDeliveryFrames: input.acpSnapshot!.pendingDeliveryFrames, + preAttach: { + usedFrames: input.acpSnapshot!.preAttachOwnedFrames, + usedBytes: input.acpSnapshot!.preAttachOwnedBytes, + pendingDeliveryFrames: + input.acpSnapshot!.pendingDeliveryFrames, + highWaterFrames: input.acpSnapshot!.preAttachOwnedFrames, + highWaterBytes: input.acpSnapshot!.preAttachOwnedBytes, + guardFailures: input.acpSnapshot!.preAttachGuardFailures, + }, mounts: [ { workspaceId: null, primary: true, connectionCount: input.acpSnapshot!.connectionCount, wsStreams: input.acpSnapshot!.wsStreams, + preAttachGuardFailures: + input.acpSnapshot!.preAttachGuardFailures, }, ], connections: [], diff --git a/packages/cli/src/serve/daemon-status.ts b/packages/cli/src/serve/daemon-status.ts index 63917fb95e4..e74a935042b 100644 --- a/packages/cli/src/serve/daemon-status.ts +++ b/packages/cli/src/serve/daemon-status.ts @@ -6,6 +6,13 @@ import type { ServeProtocolVersions } from './capabilities.js'; import type { AcpHttpHandle, AcpHttpSnapshot } from './acp-http/index.js'; +import { + ACP_PRE_ATTACH_MAX_FRAMES_GLOBAL, + ACP_PRE_ATTACH_MAX_FRAMES_PER_CONNECTION, + ACP_PRE_ATTACH_MAX_FRAMES_PER_STREAM, + ACP_PRE_ATTACH_MAX_PAYLOAD_BYTES_GLOBAL, + ACP_PRE_ATTACH_MAX_PAYLOAD_BYTES_PER_CONNECTION, +} from './acp-http/pre-attach-budget.js'; import type { DeviceFlowRegistry } from './auth/device-flow.js'; import type { DaemonLogger, @@ -149,6 +156,7 @@ type WorkspaceStatusSection = DaemonStatusSection; interface FullDaemonStatus { sessions: BridgeDaemonStatusSnapshot['sessions']; + acpMounts: AcpHttpSnapshot['mounts']; acpConnections: AcpHttpSnapshot['connections']; workspace: Record; auth: { @@ -186,6 +194,11 @@ interface DaemonStatusLimits { channelIdleTimeoutMs: number; sessionIdleTimeoutMs: number; acpConnectionCap: number | null; + acpPreAttachMaxFramesPerStream: number | null; + acpPreAttachMaxFramesPerConnection: number | null; + acpPreAttachMaxFramesGlobal: number | null; + acpPreAttachMaxPayloadBytesPerConnection: number | null; + acpPreAttachMaxPayloadBytesGlobal: number | null; /** * The daemon's resolved memory figures. Observed and reported only: nothing * consumes them to size a child. `null` on paths that resolve none, such as @@ -340,6 +353,16 @@ interface DaemonStatusRuntime { sseStreams: number; wsStreams: number; pendingClientRequests: number; + preAttach: { + bufferedConnectionFrames: number; + bufferedSessionFrames: number; + pendingDeliveryFrames: number; + usedFrames: number; + usedBytes: number; + highWaterFrames: number; + highWaterBytes: number; + guardFailures: number; + }; }; }; rateLimit: { @@ -869,6 +892,22 @@ export async function buildDaemonStatusResponse( channelIdleTimeoutMs: bridgeSnapshot.limits.channelIdleTimeoutMs, sessionIdleTimeoutMs: bridgeSnapshot.limits.sessionIdleTimeoutMs, acpConnectionCap: acpSnapshot?.connectionCap ?? null, + acpPreAttachMaxFramesPerStream: + acpSnapshot !== undefined ? ACP_PRE_ATTACH_MAX_FRAMES_PER_STREAM : null, + acpPreAttachMaxFramesPerConnection: + acpSnapshot !== undefined + ? ACP_PRE_ATTACH_MAX_FRAMES_PER_CONNECTION + : null, + acpPreAttachMaxFramesGlobal: + acpSnapshot !== undefined ? ACP_PRE_ATTACH_MAX_FRAMES_GLOBAL : null, + acpPreAttachMaxPayloadBytesPerConnection: + acpSnapshot !== undefined + ? ACP_PRE_ATTACH_MAX_PAYLOAD_BYTES_PER_CONNECTION + : null, + acpPreAttachMaxPayloadBytesGlobal: + acpSnapshot !== undefined + ? ACP_PRE_ATTACH_MAX_PAYLOAD_BYTES_GLOBAL + : null, memory: toDaemonStatusMemoryLimits( memoryBudget, input.getChildHeapPolicySnapshot?.(), @@ -924,6 +963,17 @@ export async function buildDaemonStatusResponse( sseStreams: acpAggregate?.sseStreams ?? 0, wsStreams: acpAggregate?.wsStreams ?? 0, pendingClientRequests: acpAggregate?.pendingClientRequests ?? 0, + preAttach: { + bufferedConnectionFrames: + acpAggregate?.bufferedConnectionFrames ?? 0, + bufferedSessionFrames: acpAggregate?.bufferedSessionFrames ?? 0, + pendingDeliveryFrames: acpAggregate?.pendingDeliveryFrames ?? 0, + usedFrames: acpAggregate?.preAttach.usedFrames ?? 0, + usedBytes: acpAggregate?.preAttach.usedBytes ?? 0, + highWaterFrames: acpAggregate?.preAttach.highWaterFrames ?? 0, + highWaterBytes: acpAggregate?.preAttach.highWaterBytes ?? 0, + guardFailures: acpAggregate?.preAttach.guardFailures ?? 0, + }, }, }, rateLimit: { @@ -1021,6 +1071,7 @@ async function buildFullStatus( return { sessions, + acpMounts: acpSnapshot?.mounts ?? [], acpConnections: acpSnapshot?.connections ?? [], workspace: { mcp, diff --git a/packages/cli/src/serve/run-qwen-serve.test.ts b/packages/cli/src/serve/run-qwen-serve.test.ts index 3bd7f46ce57..5bc9a147942 100644 --- a/packages/cli/src/serve/run-qwen-serve.test.ts +++ b/packages/cli/src/serve/run-qwen-serve.test.ts @@ -7835,6 +7835,7 @@ describe('runQwenServe runtime startup failures', () => { }, full: { sessions: [], + acpMounts: [], acpConnections: [], workspace: {}, auth: { diff --git a/packages/cli/src/serve/run-qwen-serve.ts b/packages/cli/src/serve/run-qwen-serve.ts index adc554aba64..b8ad3bb7f71 100644 --- a/packages/cli/src/serve/run-qwen-serve.ts +++ b/packages/cli/src/serve/run-qwen-serve.ts @@ -1669,6 +1669,11 @@ function createBootstrapServeApp(input: { channelIdleTimeoutMs: channelIdleTimeoutMs(opts.channelIdleTimeoutMs), sessionIdleTimeoutMs: sessionIdleTimeoutMs(opts.sessionIdleTimeoutMs), acpConnectionCap: null, + acpPreAttachMaxFramesPerStream: null, + acpPreAttachMaxFramesPerConnection: null, + acpPreAttachMaxFramesGlobal: null, + acpPreAttachMaxPayloadBytesPerConnection: null, + acpPreAttachMaxPayloadBytesGlobal: null, // No child-heap policy during bootstrap: it is built with the // runtime, so `enforced` is correctly false and `childHeap` null in // this window even when the flag says `enforce`. @@ -1717,6 +1722,16 @@ function createBootstrapServeApp(input: { sseStreams: 0, wsStreams: 0, pendingClientRequests: 0, + preAttach: { + bufferedConnectionFrames: 0, + bufferedSessionFrames: 0, + pendingDeliveryFrames: 0, + usedFrames: 0, + usedBytes: 0, + highWaterFrames: 0, + highWaterBytes: 0, + guardFailures: 0, + }, }, }, rateLimit: { @@ -1740,6 +1755,7 @@ function createBootstrapServeApp(input: { ? { full: { sessions: [], + acpMounts: [], acpConnections: [], workspace: {}, auth: { diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index 75fae8fa22f..4c5eac910eb 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -630,6 +630,11 @@ export interface DaemonStatusReport { channelIdleTimeoutMs: number; sessionIdleTimeoutMs: number; acpConnectionCap: number | null; + acpPreAttachMaxFramesPerStream?: number | null; + acpPreAttachMaxFramesPerConnection?: number | null; + acpPreAttachMaxFramesGlobal?: number | null; + acpPreAttachMaxPayloadBytesPerConnection?: number | null; + acpPreAttachMaxPayloadBytesGlobal?: number | null; compactedReplayMaxBytes: number; maxJournalEvents: number; maxJournalBytes: number; @@ -725,6 +730,16 @@ export interface DaemonStatusReport { sseStreams: number; wsStreams: number; pendingClientRequests: number; + preAttach?: { + bufferedConnectionFrames: number; + bufferedSessionFrames: number; + pendingDeliveryFrames: number; + usedFrames: number; + usedBytes: number; + highWaterFrames: number; + highWaterBytes: number; + guardFailures: number; + }; }; }; rateLimit: { @@ -866,7 +881,26 @@ export interface DaemonStatusReport { /** Present only when requested with `detail=full`. */ full?: { sessions: DaemonStatusReportSession[]; - acpConnections: Array>; + /** Additive; absent when reading full status from an older daemon. */ + acpMounts?: Array<{ + workspaceId: string | null; + primary: boolean; + connectionCount: number; + wsStreams: number; + preAttachGuardFailures: number; + }>; + acpConnections: Array<{ + connectionIdPrefix?: string; + workspaceId?: string | null; + workspaceCwd?: string; + primary?: boolean; + bufferedConnectionFrames?: number; + bufferedSessionFrames?: number; + pendingDeliveryFrames?: number; + preAttachOwnedFrames?: number; + preAttachOwnedBytes?: number; + [key: string]: unknown; + }>; workspace: Record; auth: { supportedDeviceFlowProviders: string[]; diff --git a/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts b/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts index 90dd843f4ee..14a2b3ac997 100644 --- a/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts +++ b/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts @@ -327,9 +327,83 @@ describe('public SDK entry — typed daemon event surface (#4217)', () => { expectTypeOf().not.toBeNever(); expectTypeOf().not.toBeNever(); expectTypeOf().not.toBeNever(); - expectTypeOf().toMatchTypeOf<{ - compactedReplayMaxBytes: number; + expectTypeOf< + DaemonStatusReport['limits']['compactedReplayMaxBytes'] + >().toEqualTypeOf(); + expectTypeOf< + Pick< + DaemonStatusReport['limits'], + | 'acpPreAttachMaxFramesPerStream' + | 'acpPreAttachMaxFramesPerConnection' + | 'acpPreAttachMaxFramesGlobal' + | 'acpPreAttachMaxPayloadBytesPerConnection' + | 'acpPreAttachMaxPayloadBytesGlobal' + > + >().toEqualTypeOf<{ + acpPreAttachMaxFramesPerStream?: number | null; + acpPreAttachMaxFramesPerConnection?: number | null; + acpPreAttachMaxFramesGlobal?: number | null; + acpPreAttachMaxPayloadBytesPerConnection?: number | null; + acpPreAttachMaxPayloadBytesGlobal?: number | null; + }>(); + expectTypeOf< + DaemonStatusReport['limits']['acpPreAttachMaxPayloadBytesGlobal'] + >().toEqualTypeOf(); + expectTypeOf< + Pick + >().toEqualTypeOf<{ + preAttach?: { + bufferedConnectionFrames: number; + bufferedSessionFrames: number; + pendingDeliveryFrames: number; + usedFrames: number; + usedBytes: number; + highWaterFrames: number; + highWaterBytes: number; + guardFailures: number; + }; + }>(); + expectTypeOf().toMatchTypeOf< + DaemonStatusReport['runtime']['transport']['acp']['preAttach'] + >(); + expectTypeOf< + Pick< + NonNullable['acpConnections'][number], + | 'bufferedConnectionFrames' + | 'bufferedSessionFrames' + | 'pendingDeliveryFrames' + | 'preAttachOwnedFrames' + | 'preAttachOwnedBytes' + > + >().toEqualTypeOf<{ + bufferedConnectionFrames?: number; + bufferedSessionFrames?: number; + pendingDeliveryFrames?: number; + preAttachOwnedFrames?: number; + preAttachOwnedBytes?: number; }>(); + expectTypeOf< + NonNullable< + DaemonStatusReport['full'] + >['acpConnections'][number]['preAttachOwnedFrames'] + >().toEqualTypeOf(); + const legacyAcpConnections: NonNullable< + DaemonStatusReport['full'] + >['acpConnections'] = [{}]; + expect(legacyAcpConnections).toHaveLength(1); + expectTypeOf< + NonNullable< + DaemonStatusReport['full'] + >['acpConnections'][number]['connectionIdPrefix'] + >().toEqualTypeOf(); + expectTypeOf().toMatchTypeOf< + NonNullable['acpMounts'] + >(); + expectTypeOf< + NonNullable< + NonNullable['acpMounts'] + >[number]['preAttachGuardFailures'] + >().toEqualTypeOf(); expectTypeOf().toMatchTypeOf<{ runId?: string; logMode?: DaemonLogMode;