Skip to content

fix(memory): bound rate-limit fetcher accumulators - #10995

Draft
nwparker wants to merge 9 commits into
oom-slice-9-markdownfrom
oom-slice-10-rate-limits
Draft

nwparker wants to merge 9 commits into
oom-slice-9-markdownfrom
oom-slice-10-rate-limits

Conversation

@nwparker

@nwparker nwparker commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

Slice 10 of the re-landed OOM work (#10179, reverted in #10255 for size). Based on oom-slice-9-markdown (#10990).

ELI5

Orca shows how much of your usage quota each AI provider has left. To find out, it reads credential files, calls provider APIs, and — when there's no API for it — spawns a hidden terminal and scrapes the output.

Every one of those paths accumulated without a ceiling:

  • The hidden terminals were never counted, so a provider that kept failing could leave more and more of them running.
  • Their scraped output was appended to a string forever.
  • Reading credentials off a disconnected WSL distro hangs rather than failing. Each refresh started another hang, and each hang pinned one of Node's small pool of filesystem threads. Enough of them and every file read in the app stalls — not just rate limits.
  • The Gemini CLI credential scan walked a bundle directory with no stopping point.
  • The Codex RPC reader buffered stdout until it saw a newline. A peer that never sends one buffers forever.

This puts a ceiling on each.

What changed

Shared, bounded auth reads. One read is now shared per credential path instead of one per caller, with ceilings on retained entries, coalesced waiters, and path length:

Limit Value Guards against
registry entries 128 many distinct credential paths
waiters per path 256 many concurrent refreshes of one path
path bytes 64 KiB one pathological path
queued WSL operations 128 queue growth while distros hang
concurrent WSL operations 2 libuv threadpool exhaustion

WSL reads serialize by normalized distro, which folds wsl$ / wsl.localhost and case aliases together. Healthy local reads never enter this queue.

Bounded Gemini CLI credential discovery. Newer Gemini CLI versions ship no oauth2.js, so Orca scans the bundle directory as a fallback. That scan now stops at 4,096 entries, 512 .js files, or 128 MiB inspected — whichever comes first — and the directory handle is closed even when a read throws. Source reads cap at 32 MiB and package.json reads at 1 MiB.

Bounded Codex RPC lines. A run is dropped once a single unterminated stdout line would exceed 4 MiB, rather than buffering until the process dies.

Bounded hidden PTYs. At most 16 hidden rate-limit PTYs may be active; an overflow attempt is torn down immediately and raises a typed error. Scraped output retains only a bounded tail, copied through a round trip so the retained string doesn't pin the much larger chunk it was sliced from.

Bounded provider JSON. Provider responses go through the shared bounded-response helper instead of await response.json(). Credential files themselves read through a 1 MiB ceiling.

User-facing changes

I looked specifically for behavior changes a user could notice. There are none. Every ceiling here sits far above real usage:

  • 16 hidden PTYs vs. one per provider refresh.
  • 128 credential paths vs. a handful of configured accounts.
  • 4,096 bundle entries vs. ~50 in a real Gemini CLI install.
  • 4 MiB vs. a rate-limit RPC response measured in kilobytes.

Below those ceilings behavior is unchanged. Above them the failure is a typed error and a status: 'error' rate-limit result — the same shape the UI already handles for an unreachable provider — rather than a hang or a crash.

The WSL serialization is the one case where timing changes: two credential reads on different disconnected distros now run one after another instead of together. Both were going to hang regardless; this stops them consuming the filesystem threadpool that the rest of the app shares.

Proof the bounds actually hold

Every constant was rewritten to Number.MAX_SAFE_INTEGER and the suite re-run serially, with the baseline sampled twice and intersected so a load-sensitive flake can't be mistaken for a kill. A bound whose removal leaves the suite green isn't proven — it's decoration.

Constant Value Result Killed by
MAX_QUEUED_WSL_AUTH_OPERATIONS 128 ✅ killed caps the WSL wait queue and recovers as soon as a queued operation leaves
MAX_CONCURRENT_WSL_AUTH_OPERATIONS 2 ✅ killed caps cross-distro operations and never starts an expired queued probe
MAX_AUTH_FILESYSTEM_OPERATION_WAITERS 256 ✅ killed caps coalesced waiters and admits another after one leaves
MAX_AUTH_FILESYSTEM_OPERATION_PATH_BYTES 64 KiB ✅ killed accepts an exact-limit auth path and rejects one byte more
MAX_AUTH_FILESYSTEM_REGISTRY_ENTRIES 128 ✅ killed 2 tests — registry saturation, stalled backend auth reads
MAX_GEMINI_CLI_OAUTH_SOURCE_BYTES 32 MiB ✅ killed reads an OAuth source under a 32 MiB ceiling
MAX_GEMINI_CLI_PACKAGE_JSON_BYTES 1 MiB ✅ killed reads a package.json under a 1 MiB ceiling
MAX_GEMINI_CLI_BUNDLE_ENTRIES 4,096 ✅ killed stops scanning the bundle directory after 4,096 entries
MAX_GEMINI_CLI_BUNDLE_FILES 512 ✅ killed stops scanning the bundle directory after 512 JavaScript files
MAX_GEMINI_CLI_BUNDLE_BYTES 128 MiB ✅ killed stops scanning after 128 MiB of inspected sources
MAX_RPC_RESPONSE_LINE_BYTES 4 MiB ✅ killed drops the RPC run once one unterminated line exceeds 4 MiB
MAX_ACTIVE_HIDDEN_RATE_LIMIT_PTYS 16 ✅ killed caps active hidden PTYs, tears down overflow, and recovers
MAX_INTEGRATION_CREDENTIAL_FILE_BYTES 1 MiB ✅ killed 2 tests — exact-cap read, oversized sparse file rejected

8 of these survived the first pass, and the cause is worth stating because it recurred from slice 9. Tests that seed state relative to the constant under test (Array.from({ length: MAX_ENTRIES })) pass at any constant value. Worse, at MAX_SAFE_INTEGER some of them fail with RangeError or an OOM'd worker — which looks like a kill while proving nothing about the bound. Every test now asserts against the literal ceiling (128, 256, 64 KiB, 4,096, 512, 16), so raising a constant fails the suite.

The Gemini extractor had no test file at all — it was mocked out at the module boundary everywhere it appeared. Its five bounds are now covered directly.

One bound was deleted rather than tested. MAX_AUTH_FILESYSTEM_REGISTRY_PATH_BYTES (8 MiB aggregate) is exactly 128 × 64 KiB — the entry count times the per-path ceiling. It cannot bind before those two do. Rather than write a test for an unreachable branch, I removed the constant and its byte accounting.

Proof of no regressions

  • Full suite: 3,478 files / 37,167 tests pass. Two failures in src/relay/agent-exec-handler.test.ts are pre-existing — I confirmed via git stash that they fail identically on the base commit.
  • Mobile: 342 files / 2,486 tests pass, 0 type errors.
  • Typecheck: node clean (0); web shows exactly the 6 pre-existing errors present on base (worktree-logic ×4, worktree-metadata-merge ×1, worktree-name-suggestions.test ×1).
  • Lint: 27 warnings / 0 errors — identical to baseline. Format clean across 8,432 files.
  • No test was deleted or weakened. I diffed the it( count of all 13 touched test files against their previous versions: net +24 tests, none lost. grok-auth.test.ts looks heavily rewritten in the diff, but that's mock rewiring for the bounded reader — the same 6 tests remain.

A real defect, caught by CI and fixed. The first push failed typecheck: six fetchers import readIntegrationCredentialFileText / readIntegrationCredentialFileSyncText, but I had taken the importers without the module that defines them — the revert had removed those exports. My local typecheck had run before those edits were staged, so it passed against a stale tree; CI caught it honestly. Fixed in 0336ea4b4c by restoring the module with its 1 MiB read ceiling, plus tests for both readers. The lesson generalizes: taking a file from the reverted branch means checking what it imports, not just what it changes.

Four revert hazards caught during assembly. Files that show 0/0 against the merge base are not safe to take wholesale from the reverted branch — doing so silently reverts fixes that shipped on main after the branch point. Restored to HEAD and re-applied by hand:

  1. service.ts / service.test.ts — would have reverted a weekly-only Codex fix and deleted 2 tests.
  2. claude-pty-reset-parser.ts — a pure revert deleting 24-hour reset parsing and box-glyph stripping (main: 4 tests, branch: 0).
  3. codex-fetcher-pty-settle.test.ts — deleted 223 lines including a shipped test.
  4. codex-fetcher.ts — genuine OOM work mixed with a revert of the PTY status-nudge timings. Rebuilt from HEAD with only the OOM changes applied.

nwparker added 4 commits July 27, 2026 13:45
Slice 10 of the re-landed OOM work. Bounds the unbounded accumulators in the
provider rate-limit fetchers.

- Auth filesystem operations share one read per path, with caps on retained
  entries, coalesced waiters, path bytes, and the WSL wait queue. WSL reads
  serialize by distro so a few disconnected distros cannot occupy libuv's
  filesystem pool.
- Gemini CLI OAuth extraction bounds the source read, package.json read, and
  the bundle-directory scan by entries, files, and aggregate bytes.
- Codex RPC drops a run once one unterminated stdout line exceeds 4 MiB.
- Hidden rate-limit PTYs are capped and torn down on overflow; PTY diagnostic
  output retains a bounded tail detached from the oversized chunk.
- Provider JSON responses read through the shared bounded-response helper.

Every bound is covered by a test asserting the literal ceiling, so raising a
constant fails the suite rather than silently buffering more.
The rate-limit fetchers import readIntegrationCredentialFileText and
readIntegrationCredentialFileSyncText, but the source module was never
brought into the slice, so those exports did not exist. Adds them along
with the 1 MiB read ceiling they wrap, plus tests asserting the literal
cap in both the sync and async readers.
@OrcaWin
OrcaWin marked this pull request as draft October 27, 2026 09:44
@OrcaWin

OrcaWin commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

Moving this PR to draft because it has been untouched for more than 80 days (latest recorded PR activity or commit timestamp: 2026-07-28T05:02:39Z; checked on 2026-10-27). This keeps the review queue focused on active work while preserving the proposal. Please mark it ready for review when work resumes.

This branch has not been deployed

No deployments
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