Repository navigation
Conversation
Markdown link completion listed every .md/.mdx/.markdown file under a workspace root with no ceiling on count, aggregate metadata, path length, directories visited, or recursion depth, and the renderer retained one document array per worktree forever. A large monorepo — or a symlink-free but very deep tree — could grow both the main-process listing and the renderer cache without limit. - Add a shared listing budget (20,000 documents / 8 MiB metadata / 64 KiB path / 100,000 visited entries / depth 256) enforced by the local, SSH, and relay producers alike. Caller-supplied limits are clamped down, never up, so the ceiling is enforced rather than advisory. - Stream directory entries with opendir instead of materializing each directory's full entry array. - Bound renderer retention to an LRU of 8 worktree snapshots capped at 32 MiB aggregate. - Surface capacity failures as a typed error (relay code -33008) and a single toast rather than a silent empty list.
The completion retention limits evict by scope, and eviction can drop a scope whose editor is still open. Nothing re-populated it: `provideCompletionItems` only reads the maps, and the sole writer runs from an effect keyed on language, document list, runtime environment, and worktree — none of which change while you type. So an evicted editor offered zero `[[` completions indefinitely, looking exactly like a workspace with no markdown files. Register a refill callback per model. A completion request that misses now re-supplies once from the owning editor and re-reads, so eviction costs a recomputation instead of the feature. The callback reaches current props through a ref, since it outlives the render that registered it, and is released on unmount. Refills are bounded too (512): a closure and a key, no document snapshot. Co-authored-by: Orca <help@stably.ai>
This was referenced Oct 5, 2026
OrcaWin
marked this pull request as draft
October 27, 2026 09:44
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:10:40Z; 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
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.
Slice 9 of the re-landed OOM work (#10179, reverted in #10255 for size). Based on
oom-slice-8-ssh-config(#10887).ELI5
When you type
[[in a Markdown file, Orca offers to link to any other Markdown file in your workspace. To do that it walks the whole folder tree and builds a list of every.mdfile it finds.That walk had no stopping point. In a huge monorepo it would keep going until it had every file, and the editor kept one of those lists per worktree, forever, never throwing any away. Two ways to run out of memory: one workspace that's too big, or a lot of workspaces that each add up.
This adds a ceiling to the walk and a small, self-cleaning cache to the editor.
What changed
A shared listing budget. One module now owns the ceilings, and the three producers that build this list — local filesystem, SSH provider, relay — all enforce the same ones:
Caller-supplied limits are clamped down, never up (
clampLimitreturnsMath.min). Limits arrive from the renderer and across the relay, so an untrusted caller asking for a bigger budget gets the module ceiling instead. There's a test for exactly this — it passesNumber.MAX_SAFE_INTEGERfor all five and asserts the budget comes back at the module values.Streaming directory reads.
readdirmaterialized every entry of a directory as an array before any of it was examined; a single flat directory with a million entries allocated the whole array first. Discovery now usesopendirand consumes entries withfor await, so entries are processed as they arrive.Bounded renderer retention. The editor kept
Record<worktreeId, MarkdownDocument[]>and only ever added keys. It's now an LRU of 8 worktree snapshots capped at 32 MiB aggregate, evicting oldest-first on either bound.Typed capacity failures. Hitting a ceiling raises a typed error (relay code
-33008) rather than silently returning a short list, so the renderer can tell "workspace too large" apart from "listing failed".User-facing changes
Three, stated plainly:
[[link completion is lost entirely for that workspace — not degraded. The listing throws, the renderer shows one toast ("Workspace is too large for Markdown link completion.") and sets an empty list. This is the one real downside of the slice. Truncating instead would silently offer an arbitrary subset with no signal that anything was missing, which is worse to debug; a hard, announced failure was the deliberate call. Worth revisiting if anyone hits it.**in a workspace with more than 256 levels of nesting stops descending at 256. No known real tree comes close.Everything below those ceilings is byte-for-byte unchanged.
Point 3 was only half true when first written, and the follow-up commit is what makes it true. See below.
The bug the follow-up commit fixes
Review of the first commit found that one of the two evictions this slice adds was not in fact "a refetch, not a failure".
What went wrong. There are two caches here. The worktree LRU in
useMarkdownDocumentsdoes re-fetch on a miss — that one was fine. The Monaco completion retention (MARKDOWN_COMPLETION_MAX_SCOPES= 32,MAX_MODELS= 256, 64 MiB) had no such path:provideCompletionItemsonly reads the scope maps. A miss returned[].setMarkdownDocCompletionDocuments, is called from an effect keyed onlanguage,markdownDocuments,runtimeEnvironmentId, andworktreeId.[[cannot trigger a re-supply.So an editor whose scope got evicted — by 32 other scopes appearing, or by the byte ceiling — offered zero
[[completions for as long as it stayed open. Identical on screen to a workspace containing no Markdown files. Unlike case 1 above, there is no toast: it fails silently and looks like a correct empty result.Reachable in ordinary use: open a Markdown file, work across enough other worktrees or runtime environments to push 32 scopes, come back to the still-open tab.
Confirmed chain-introduced, not pre-existing. At the merge base (
f10b6de2c7) the module holds a plain unboundeddocumentsByModelMapwith no eviction at all, so a mounted editor could never lose its documents. The eviction — and therefore the empty-completion state — arrives with this slice.The fix. Each model registers a refill callback alongside its documents. A completion request that misses the cache now calls it once and re-reads:
Eviction becomes a recomputation instead of a dead feature. Details worth a reviewer's attention:
getCompletionDocuments()recursively instead of re-reading gives infinite recursion — the mutation run proves it (RangeError: Maximum call stack size exceeded).Mutation-tested, 7 mutations, all fail:
MonacoEditorstops registering the refillThe last two are the ones that matter for call-site vacuity: they mutate the component, not the completion module. A new test renders the real
MonacoEditor, drives the real registered Monaco provider throughprovideCompletionItems, and asserts suggestions survive eviction — so deleting the wiring fails a test rather than passing silently.Verification at the current tip (after merging current
origin/main):src/renderer/src/components/editor+src/renderer/src/store+src/shared— 6,379 passed, 0 failed (7 skipped). Node and web typecheck clean. Lint 0 warnings / 0 errors.Proof the bounds actually hold
Every constant was rewritten to
Number.MAX_SAFE_INTEGERand the suite re-run, serially, in a scope covering all seven producer/consumer areas. A bound whose removal leaves the suite green isn't proven — it's decoration. Baseline was sampled twice and its intersection taken, so a known load-sensitive flake can't be mistaken for a kill:MAX_DOCUMENTSMAX_METADATA_BYTESMAX_PATH_BYTESMAX_VISITED_ENTRIESMAX_DEPTHWORKTREE_MAX_SNAPSHOTSWORKTREE_MAX_RETAINED_BYTESAn earlier pass had 6 of these surviving. The cause was the same each time: tests that seeded state relative to the constant under test (
budget.visitedEntries = MAX_VISITED_ENTRIES - 1) pass at any constant value, including one large enough to be no bound at all. The new tests assert against literal ceilings (99,998 / 100,000,8 * 1024 * 1024, 256, 8, 32 MiB), so raising a constant now fails the suite.One case worth noting: the 32 MiB retention test first failed because a single snapshot big enough to reach 32 MiB already exceeds the 8 MiB per-listing ceiling. The aggregate cap only binds across several worktrees, which is the case it exists for — so the test now uses 5 snapshots × 8,000 documents, each individually legal.
Proof of no regressions
src/relay/agent-exec-handler.test.ts, which also fail on the base branch — pre-existing, unrelated.worktree-logic×4,worktree-metadata-merge×1,worktree-name-suggestions.test×1).origin/maincheckout, so the 0 is main's and not suppression added here.)Two findings during verification, both chased down rather than waved off:
A real regression, found and fixed.
filesystem.test.tsmocksfs/promises; switching discovery toopendirbroke 2 markdown tests there because the mock had noopendirexport. It was caught only when the mutation scope widened to all ofsrc/main/ipc— a narrower earlier scope had missed it. Fixed by delegatingopendirto the existingreaddirfixtures, keeping one source of directory contents.A flake, correctly classified.
terminal-history-incremental-restore.test.tsfailed twice in one full-suite run. Rather than assume, I traced the import graph — no slice file is reachable from that test across 155 modules — and ran an interleaved A/B under CPU contention, 5 rounds, alternating base and slice. The failure reproduced on base, not on the slice. It's a pre-existingvi.waitFortiming sensitivity, untouched by the slice chain.Not included
The
src/relay/protocol.tsframe-decoder hardening (buffered-chunk caps, oversized-payload streaming discard, bounded JSON stringify/parse) is deliberately left for a later relay slice. This PR takes only the-33008error code from that area.Made with Orca 🐋