Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,9 @@ Ordered by measured risk, each independently shippable.

**The NDJSON frame reader is the first bounded-container increment.** Before this change, `packages/acp-bridge/src/ndJsonStream.ts` retained every unterminated tail chunk without a count or byte check, then allocated one contiguous copy, a UTF-16 string, and a parsed object — about fivefold amplification over a frame with no upper bound. Its decoded-message `ReadableStream` also ignored `desiredSize`, creating a second unbounded buffer behind a slow consumer. This is the read side of every spawned ACP child's stdout, while `packages/cli/src/serve/large-pipe-frame-observer.ts` only observes frames after parse and enqueue. `createStderrForwarder` (64 KiB with a `[truncated]` marker) and the channel worker's log buffer are the in-repo templates for bounding at the container.

The first Part 3 increment applies that protection only to ACP streams created by `qwen serve`. A complete inbound or outbound frame is limited to 64 MiB including its newline. The decoded inbound queue is limited to 256 messages and 64 MiB of retained wire bytes. `ReadableStream` exposes one scalar queuing cost rather than independent count and byte watermarks, so each message is charged `max(frameBytes, ceil(64 MiB / 256))`. This is deliberately conservative: it proves both upper bounds, although a mixed queue can be rejected before either independent limit is exactly full. Admission is checked against `desiredSize` before decode and parse, so the message that would exceed the queue is never materialized.
The first Part 3 increment applies that protection only to ACP streams created by `qwen serve`. A complete inbound or outbound frame is limited to 64 MiB including its newline. The decoded inbound queue is limited to 256 messages and 64 MiB of retained wire bytes. `ReadableStream` exposes one scalar queuing cost rather than independent count and byte watermarks, so each message is charged `max(frameBytes, ceil(64 MiB / 256))`. This is deliberately conservative: it proves both queue bounds, although a mixed queue can be rejected before either independent limit is exactly full. Admission is checked against `desiredSize` before decode and parse, so the message that would exceed the queue is never materialized. Because the ACP SDK starts request and notification handlers without awaiting the previous handler, the daemon-owned client wrapper separately limits active handlers to 256 and their conservatively estimated retained payloads to 64 MiB. An inbound request keeps a transport slot until its response is written; once its handler returns, the response's actual conservatively estimated representation also occupies a 256-message/64-MiB prepared-response budget until local pipe delivery. Before any daemon-to-child request or notification enters the SDK, a connection wrapper reserves its arguments against the same count/byte shape; notifications release after local write and requests after response or definitive failure. The stream also correlates admitted outbound request ids with inbound responses. Crossing any of these bounds retires the exact channel generation instead of allowing a slow handler, blocked child stdin, or non-responding child to bypass the stream queue.

Crossing either bound is transport-fatal. The reader cancels the child stdout, reports a typed cause through the transport lifecycle hook, and closes its decoded readable; it does not error that readable because the ACP SDK's internal receive loop does not catch `reader.read()` rejection. The spawn channel terminates that exact tracked child, and the bridge's existing channel-exit path tears down only the sessions multiplexed on that workspace generation. An unterminated final frame is also fatal on this daemon-owned path. Parse-error logs on the bounded path contain only an error code, byte length, and SHA-256 digest; they never echo the frame or the parser error, whose message may itself contain input. The public `ndJsonStream` default, in-memory channels, direct embeds, interactive CLI, and IDE companion do not opt in, so they retain the existing eager queue, parse logging, and unterminated-EOF behavior.
Crossing any bound is transport-fatal. The reader cancels the child stdout, reports a typed cause through the transport lifecycle hook, and closes its decoded readable; it does not error that readable because the ACP SDK's internal receive loop does not catch `reader.read()` rejection. Complete frames admitted before the failing frame remain deliverable in wire order, but no bytes from the failing frame are delivered. The spawn channel marks that exact tracked child unavailable, closes both child pipes, and starts termination, so a concurrent create, resume, attach, prompt, or status operation cannot continue on it during the termination grace period; the bridge's channel-exit path later tears down only the sessions multiplexed on that workspace generation. Guard-triggered exits remain unexpected in lifecycle metrics and include only the bounded typed cause code. Unterminated EOF and clean child-stdout EOF are both fatal on this daemon-owned path: a child that closes its protocol output but remains alive must not leave a reusable dead connection or process slot. Parse errors and structurally invalid JSON-RPC envelopes on the bounded path log only an error code, byte length, and SHA-256 digest, then retire the exact channel generation so the ACP SDK cannot retain a pending request for a response that was silently discarded. Bounded messages also stop at depth 64, 10,000 JSON nodes, and 4,096 array elements. Known child-to-client methods are prevalidated against the same ACP schemas before SDK dispatch, preventing schema-error expansion and independent SDK logging from bypassing the handler and response budgets. The daemon subset accepts numeric, null, and at most 256-byte string request ids; a response id must match an admitted outbound request tracked by the same bounded stream, preventing the SDK's unknown-response path from separately logging child-controlled ids. A matching response is itself proof that the child received the request and may race the parent's local write-completion callback. Method and error-message scalars are capped at 1 KiB. Admitted messages install a non-enumerable Node inspection redaction so the ACP SDK's own handler-error logging omits their payload; the daemon-bounded client wrapper also redacts the independently logged response-error object and emits only bounded error text and structured filesystem discriminators. The public `ndJsonStream` default, unguarded in-memory channels, direct embeds, interactive CLI, and IDE companion do not opt in, so they retain the existing eager queue, error wire shape, parse logging, and unterminated-EOF behavior.

The outbound check happens after `JSON.stringify` and UTF-8 encoding. It prevents an oversized frame from entering the child pipe, but it is not a pre-allocation encoder budget; bounded/canonical JSON encoding remains a separate container change rather than being hidden inside this transport PR.

Expand Down Expand Up @@ -187,7 +187,7 @@ Part 1 changes no child spawn arguments, so there is no change to how any child

The compatibility discussion that belongs here is for the child-capacity policy that follows, and it is deferred with it. What can be said now: that policy will lower ceilings and must never raise them, it will be a compatibility change even without refusals, and it needs an admission rule for the case where an already-running child cannot be shrunk.

No new refusals are introduced. The only new boot failure is the existing validation shape for an out-of-range `--memory-budget-mb`. Workspace registration, persisted restoration, and `POST /workspaces` are unchanged.
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.

`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.

Expand Down
Loading
Loading