Skip to content

fix(memory): bound relay protocol accumulation - #11061

Closed
nwparker wants to merge 6 commits into
oom-slice-10-rate-limitsfrom
oom-slice-11-relay-protocol
Closed

nwparker wants to merge 6 commits into
oom-slice-10-rate-limitsfrom
oom-slice-11-relay-protocol

Conversation

@nwparker

Copy link
Copy Markdown
Contributor

Slice 11 of the re-landed OOM work. Base is oom-slice-10-rate-limits (#10995) — review only the top commit.

ELI5

The relay sits between Orca and work running on another host. Several parts of that connection could keep accumulating memory without a stopping point: fragmented protocol frames, parsed requests waiting for handlers, and large Git responses parked while a client acknowledged streamed chunks. A slow or hostile peer could keep enough of those objects alive to exhaust the relay process.

This slice gives every retained collection and payload an explicit ceiling. Oversized work is rejected with a typed error, while anything reported as successful remains complete and untruncated.

What changed

src/relay/protocol.ts

  • Discards an already-invalid frame advertising more than 16 MiB as its payload arrives instead of retaining the advertised payload before rejecting it.
  • Coalesces adversarial tiny frame fragments and caps retained chunk wrappers at 4,096.
  • Uses bounded JSON parsing and serialization so intermediate protocol objects cannot bypass frame-size limits.

src/relay/client-request-aborts.ts and src/relay/dispatcher.ts

  • Tracks active requests by client and globally, including the bytes retained by parsed request payloads.
  • Rejects duplicate or over-budget requests with a typed admission error and releases every count and byte budget on completion, disconnect, or shutdown.

src/relay/git-handler.ts, src/relay/git-response-stream.ts, and src/relay/git-response-utf8-chunks.ts

  • Bounds serialized Git responses at 64 MiB before they are parked for streaming.
  • Caps parked responses at 16 streams and 128 MiB total retained data.
  • Plans UTF-8 boundaries without allocating a second full payload buffer, and base64-encodes only the chunk currently being sent.
  • Accepts acknowledgements only for chunks that were actually sent and releases retained-byte accounting on every exit path.

Relay tests

  • Cover fragmented and oversized frames, bounded JSON encoding, request admission and recovery, stream ownership and byte accounting, lazy serialization, and UTF-8 chunk boundaries.

Proof the OOM fix works

Each bound was mutation-tested by replacing its constant with Number.MAX_SAFE_INTEGER and rerunning the relay suite. Each named test below fails under that mutation. The tests assert the literal ceiling rather than seeding state relative to the constant, so they cannot pass merely because the implementation and test moved together.

Constant Value Test that kills the mutation
MAX_BUFFERED_FRAME_CHUNKS 4,096 handshake framing > bounds retained chunk wrappers for an adversarially fragmented frame
MAX_COALESCED_TINY_FRAME_CHUNK_BYTES 8 KiB handshake framing > does not coalesce ordinary transport chunks while a frame is incomplete
MAX_GIT_RESPONSE_STREAM_BYTES 64 MiB GitResponseStreamRegistry client ownership > rejects a response one byte past the stream ceiling
MAX_CONCURRENT_GIT_RESPONSE_STREAMS 16 GitResponseStreamRegistry client ownership > caps parked streams and defers base64 expansion until the pump runs
MAX_ACTIVE_GIT_RESPONSE_STREAM_BYTES 128 MiB GitResponseStreamRegistry client ownership > caps aggregate bytes retained by concurrent parked responses
MAX_ACTIVE_RELAY_REQUESTS_PER_CLIENT 256 ClientRequestAborts > accepts the per-client request boundary, rejects overflow, and recovers
MAX_ACTIVE_RELAY_REQUESTS 1,024 ClientRequestAborts > accepts the aggregate request boundary without evicting active clients
MAX_ACTIVE_RELAY_REQUEST_BYTES_PER_CLIENT 32 MiB ClientRequestAborts > bounds retained payload bytes per client and releases the budget on delete
MAX_ACTIVE_RELAY_REQUEST_BYTES 64 MiB ClientRequestAborts > bounds aggregate retained payload bytes and recovers after client abort

The former MAX_GIT_RESPONSE_STREAM_CHUNKS ceiling of 4,096 was deleted. A 64 MiB response split into 128 KiB chunks reaches at most 512 chunks, so the byte ceiling always binds first. Removing it removes a bound that could never fire.

Proof of zero regressions

  • Full suite: 3,478 files / 37,167 tests pass. The two failures in src/relay/agent-exec-handler.test.ts are pre-existing and reproduce on the base.
  • Relay suite after rebase: 78 files / 926 tests pass; only those same 2 known tests fail.
  • Mobile: 342 files / 2,486 tests pass, with 0 type errors.
  • Typecheck: node clean with 0 errors; web has exactly the 6 pre-existing errors (worktree-logic ×4, worktree-metadata-merge ×1, worktree-name-suggestions.test ×1).
  • Lint: 27 warnings / 0 errors, identical to the base.
  • Format: clean across 8,432 files.
  • Integration: git merge-tree --write-tree --name-only origin/main HEAD exits 0, so the rebased commit merges cleanly into real main.

User-facing behavior changes

Below the ceilings, successful results are unchanged: there is no silent truncation and no data is dropped from anything reported as successful.

Above the ceilings, behavior now changes explicitly:

  • An already-invalid frame advertising a payload larger than 16 MiB is discarded as it arrives rather than retained until its advertised length has arrived.
  • A Git response larger than 64 MiB is rejected with a typed stream-protocol error.
  • More than 16 parked Git response streams, or more than 128 MiB of parked Git response data, is rejected with a typed stream-admission error.
  • More than 256 active requests for one client, more than 1,024 active requests total, more than 32 MiB of active request payloads for one client, or more than 64 MiB total is rejected with a typed request-admission error.

Made with Orca 🐋

@OrcaWin

OrcaWin commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

SSH still works: Orca connects to the host over SSH and routes workspace operations through the managed remote Orca server (orcad). These merges remove the legacy SSH relay, not SSH support.

Closing as obsolete after the merged SSH relay retirement in #27214 and #27215. This PR concerns the SSH relay server and its relay-only handlers. #27214 removed the desktop relay connection/fallback path; #27215 deleted the SSH relay server, deployment assets, and relay-only tests. SSH hosts now use the managed Orca server (orcad), and unsupported hosts fail explicitly, so this relay-specific work is no longer planned. This closes the proposed relay implementation; it does not claim that any independent local, WSL, or managed-server behavior was fixed.

@OrcaWin OrcaWin closed this Oct 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants