fix(server): bound editor discovery during config loading#4291
fix(server): bound editor discovery during config loading#4291maria-rcks wants to merge 1 commit 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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4bd5bcf. Configure here.
| yield* Deferred.await(discoveryInterrupted); | ||
| assert.deepEqual(availableEditors, []); | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Timeout test missing yieldNow
Medium Severity
The new editor-discovery timeout test forks resolveAvailableEditorsForConfig over Effect.never, then calls TestClock.adjust without first yielding so the child fiber can register its timer. That diverges from the repo’s forkChild + TestClock.adjust pattern and can leave Fiber.join waiting forever, hanging CI.
Reviewed by Cursor Bugbot for commit 4bd5bcf. Configure here.
ApprovabilityVerdict: Needs human review The production code change is a low-risk defensive timeout addition. However, there's an unresolved comment about a potential test issue (missing yieldNow before TestClock.adjust) that could cause the test to hang, which warrants human attention. You can customize Macroscope's approvability policy. Learn more. |


What Changed
availableEditorslist when discovery times out.Why
server.getConfigwaits for optional editor discovery. On affected machines, discovery consumed almost the entire 15-second connection setup window, so the client timed out and entered a reconnect loop even though the server was running.This keeps the fix at the config-loading seam and avoids the process-diagnostics and platform-specific changes proposed in #3911. Related symptom report: #3746.
Closes #3610
Checklist
Note
Bound editor discovery in
makeWsRpcLayerto a 5-second timeoutresolveAvailableEditorsForConfigin ws.ts, which wraps editor discovery with a 5-second timeout usingEffect.timeoutOption.availableEditorsfield in the server config response now returns an empty array on timeout instead of blocking indefinitely.availableEditorslist after 5 seconds.Macroscope summarized 4bd5bcf.
Note
Low Risk
Localized change to optional config metadata with graceful degradation; no auth or persistence impact.
Overview
server.getConfigno longer waits indefinitely on optional editor discovery. Discovery is capped at 5 seconds via newresolveAvailableEditorsForConfig(Effect.timeoutOption); on timeoutavailableEditorsis[]instead of holding the response.This addresses clients timing out during connection setup when slow discovery consumed most of the setup window. A regression test asserts that after 5s the result is empty and the stuck discovery fiber is interrupted.
Reviewed by Cursor Bugbot for commit 4bd5bcf. Bugbot is set up for automated code reviews on this repo. Configure here.