Repository navigation
Conversation
nwparker
force-pushed
the
oom/13-b-daemon-pty
branch
from
July 24, 2026 07:48
c2346d5 to
ed074fa
Compare
nwparker
force-pushed
the
oom/13-b-daemon-pty
branch
from
July 25, 2026 02:21
ed074fa to
7f5cd50
Compare
nwparker
force-pushed
the
oom/13-b-daemon-pty
branch
from
July 25, 2026 02:40
7f5cd50 to
8714ad3
Compare
nwparker
force-pushed
the
oom/13-b-daemon-pty
branch
from
July 25, 2026 03:10
8714ad3 to
bf5147e
Compare
Files: 48 applied, 0 deleted (from 6eb70d8) Co-authored-by: Orca <help@stably.ai>
nwparker
force-pushed
the
oom/13-b-daemon-pty
branch
from
July 25, 2026 03:34
bf5147e to
7ce7037
Compare
Contributor
Author
|
Superseded by #10548, which re-scopes this work to the bound-at-ingress doctrine (see #10533 and An audit found the original #10179 scope was ~50x the size of the fix for the crash it targeted, and that bounds had landed ahead of their callers. Every file in these slices was re-adjudicated individually: ingress bounds (transport frames, PTY flow control, on-disk transcripts, parsing, enumeration, media decode, user-keyed accumulators) were kept; interior ceilings behind an already-bounded input were dropped. 1,882 files → 1,155, full suite green, and no No memory protection was lost — only speculation. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OOM hardening slice 13/29 — bound daemon admission & PTY delivery credits
Summary
This slice re-lands OOM ceilings across the terminal daemon and PTY layer: it caps daemon transport sockets/clients/requests/response bytes, bounds NDJSON/binary-frame line and payload sizes, admits and byte-limits every daemon request field, streams overlay-tree removal and config-overlay mirroring under entry/depth caps, and bounds terminal-history logs, pre-ready stdin, pending output, shell-startup rc reads, and terminal size (500x500).
ELI5
Imagine the app's terminal engine as a mailroom that used to accept letters of any size, any number, from anyone, and would pile them up until the building collapsed. This change adds sensible rules: letters can't be huge, only so many can be in flight, envelopes must be addressed correctly, and cleanup crews stop after a reasonable amount of work instead of running forever. Almost nobody in normal use ever bumps these limits; they only kick in when something is broken or hostile.
Proof of OOM fix
Regressions
Normal-path (ordinary use, below the new limits): none — behavior is unchanged below the ceilings.
Overload-only (extreme input / sustained backpressure) — intended, documented degradations:
Build / CI
mainsince the revert; git auto-merges them (no conflict):daemon-server.ts.✅ Inherited defect FIXED in this slice
src/main/daemon/daemon-response-admission.ts— Each create reserves the full 16 MiB response max against a 64 MiB per-client budget, so 4 pending terminal creates can exhaust it and reject the 5th even though real replies are tiny; DaemonPtyAdapter retries daemon-loss but not the capacity error.Adversarial review
CONCERN 1 - Terminal size cap 500x500, HARD-rejected with no clamp (src/shared/terminal-size-limits.ts, wired via daemon-admission-limits.ts + daemon-pty-size.ts). Confirmed the renderer does NOT import terminal-size-limits (git grep over src/renderer is empty), so it does not pre-clamp. createOrAttach with cols>500 -> createOrAttachPayloadError -> spawn rejected outright (NOT clamped to fallback). resize runs terminalSizeAdmissionError with NO allowMissing, so cols>500 -> writeRequestError, PTY keeps old size (renderer/PTY mismatch). Reachability: a maximized full-width terminal on a 49-inch super-ultrawide (5120px) at a ~10px cell font computes
500-512 cols - a real if uncommon dev config, not purely abusive. Rows=500 unreachable; cols500 is the edge. Normal panes/displays (80-300 cols) unaffected. Low-medium; human should confirm Orca clamps fitted dims to 500 in renderer/adapter (out of this slice).CONCERN 2 - Response-reservation model (daemon-response-admission.ts + daemon-server.ts dispatchRequest). createOrAttach/getSnapshot/listSessions/takePendingOutput each reserve a FIXED 16MB regardless of actual size; per-client cap 64MB => only 4 concurrent per app instance (test daemon-server-admission.test.ts:217 confirms 64MB/16MB=4; 5th throws 'request capacity exceeded'); global 128MB => 8. Reservation held for full preparePtySpawn + host.createOrAttach duration, released in .finally(). If the renderer fires 5+ createOrAttach concurrently during multi-terminal restore, requests 5+ get 'Daemon request capacity exceeded; retry'. Multi-terminal restore is a normal power-user scenario. Could NOT confirm it triggers: restore orchestration/retry lives in the renderer/adapter (daemon-pty-adapter.ts is OUT of this slice and unchanged), and the 'retry' error text plus full-CI shipping strongly imply restore serializes or retries. Human should confirm restore path serializes createOrAttach or handles 'capacity exceeded; retry'.
VERIFIED OVERLOAD-ONLY (ceilings far above ordinary use): pending-output record cap 4096 (session.ts) dominated by pre-existing 2MB byte cap and only matters while detached; pre-ready stdin 16MB/4096 segments (throw is caught by write RPC handler, daemon survives); history log cap unchanged at 5MB, checkpoint 16MB (JsonStringifyByteLimitError caught in checkpoint try/catch); shell-startup rc 1MB read + 64KB env-value cap; config-overlay mirroring 4096 entries/1MB names only affects mimo/opencode/relay agent hook services (callers confirmed: src/main/mimo, src/main/opencode, src/relay only), never ordinary spawns; overlay-tree removal 100k entries/256 depth; daemon 128 sockets/32 control clients/24 stream attachments; NDJSON 16MB line/128 depth/1M tokens; 10s handshake timeout.
CLEANUP/CORRECTNESS: No leak. dispatchRequest increments then decrements in .finally() (runs on throw/cancel); admission failures return BEFORE incrementing. writeRequestError early-returns for null/notify ids. DAEMON_MAX_STREAM_ATTACHMENTS(24) < DAEMON_MAX_CONTROL_CLIENTS(32) asymmetry is intentional, only reachable with >24 concurrent app instances.
OOM EVIDENCE: strong. Concrete caps in daemon-admission-limits.ts, daemon-response-admission.ts, terminal-history-file-limits.ts, shell-startup-env.ts, config-overlay-mirroring.ts, overlay-mirror.ts, plus regression tests daemon-server-admission.test.ts, history-manager-memory.test.ts, session-pending-output.test.ts, session-pre-ready-input-memory.test.ts, headless-emulator-size-limits.test.ts, shell-startup-env-memory.test.ts, config-overlay-mirroring.test.ts, overlay-mirror.test.ts.
/Users/nwparker/orca/workspaces/orca/oom-followup/src/main/daemon/daemon-response-admission.ts: The large-response reservation is too coarse for an ordinary restore burst: every createOrAttach reserves the full 16 MiB response maximum, while the per-client reservation ceiling is 64 MiB. Consequently, four pending terminal creates consume the entire budget and the fifth is rejected even though normal create replies are tiny. The existing admission test proves this exact rejection, and DaemonPtyAdapter only retries daemon-loss errors, not the returned capacity error. Request counters, byte reservations, pending spawn preparations, sockets, timers, and directory handles otherwise release on the reviewed success/error/disconnect paths; exact-limit tests did not reveal an inclusive/exclusive error.Files
48 files changed (+2716 / −358).
🤖 Decomposed, reviewed & assembled by Claude Code from the reverted #10179. Draft — do not merge out of order.