fix(server): run editor discovery concurrently and cache the result - #5050
fix(server): run editor discovery concurrently and cache the result#5050capad-xyz wants to merge 2 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ApprovabilityVerdict: Needs human review This PR introduces caching and concurrent execution for editor discovery, changing state management and interruption semantics. While tests are included, the new caching layer and detached fiber pattern represent meaningful runtime behavior changes that warrant human review. You can customize Macroscope's approvability policy. Learn more. |
Effect.cached memoizes the inner effect's exit, so an interrupted first scan poisoned the cache permanently. Cache the fork instead and let callers join a detached fiber, so a timed-out caller cancels only its own wait.
Problem
server.getConfigblocks on external editor discovery, which probes ~20 editor commands sequentially, each walking everyPATHentry against everyPATHEXTvariant. On hosts with long PATHs this exceeds the 5sEDITOR_DISCOVERY_TIMEOUTon every call, so discovery is interrupted, callers get an empty editor list, and the next call starts over from scratch. Every client connection pays the full 5s on its first RPC.Fixes #4210. Also addresses the empty-list symptom in #4697.
Fix
Two changes in
ExternalLauncher:Effect.forEach(..., { concurrency: EDITORS.length })instead of a sequential loop. Wall time becomes the slowest single probe rather than the sum.Effect.cachedaroundEffect.forkDetach, so callers share one scan viaFiber.join.The second point is deliberate. Caching the scan directly is unsafe here:
Effect.cachediscachedWithTTL(self, Duration.infinity), andcachedInvalidateWithTTLmemoizes the exit throughonExit(self, ...)— including an interrupted exit — and with an infinite TTL never recomputes. A caller hitting the 5s timeout mid-scan would pin that interrupted exit for the life of the process. Caching the fork instead memoizes a fiber handle, which is produced instantly and cannot carry an interrupted exit; a caller that times out cancels only its own wait while the scan runs to completion in the background. On a #4210-class machine (15-99s scans) editors now resolve on a subsequentgetConfiginstead of never.EDITOR_DISCOVERY_TIMEOUTinws.tsis untouched and still bounds the cold call.Results are unchanged.
Effect.forEachpreserves input order, and per-editor command preference (resolveAvailableCommand) is untouched, so the returned list is identical to what the sequential loop produced — same editors, same order. There are no per-editor timeouts, so a slow-but-present editor cannot be dropped from the result.Relation to #4739
#4739 targeted the same issue and was closed unmerged. This takes a different approach: it adds caching rather than only bounding latency, and it avoids per-editor timeouts, which is what changed selection behaviour there. Happy to close this if you would rather solve it another way — the measurements below may still be useful for whatever lands.
Measurements
Windows 11, 46-entry PATH, 12-entry PATHEXT, desktop nightly
0.0.32-nightly.20260730.958(patched bundle):ws.rpc.server.getConfigexternalLauncher.buildAvailableEditorsDiscovery now completes, so clients receive a real editor list instead of a permanently empty one.
Tests
vp test run src/process/externalLauncher.test.ts— 6 passed. Two added:All fibers interrupted without error.Not covered
This does not address the Android store app disconnect (#4901); that reproduces with a fast
getConfigtoo and is protocol skew on the client build. This only removes the server-side stall.Diagnosed and implemented with Claude Code (Fable 5).