Fix provider runtime readiness and browser attachment#546
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
arul28
changed the title
📝 WalkthroughWalkthroughThis PR refactors Cursor and Droid SDK module loading to support both ESM dynamic import and packaged-runtime fallback resolution, tightens runtime availability gating to depend on health-state readiness, improves SDK worker pool IPC lifecycle reliability, and updates consumer code to align with the new runtime-verified availability model. ChangesSDK Loading and Worker Lifecycle Integration
Runtime Availability and Model Discovery
Browser Service and Consumer Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
June 9, 2026 18:43
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/main/services/ai/authDetector.ts (1)
802-830:⚠️ Potential issue | 🟠 MajorDon’t mark Cursor runtime
"ready"afterCursor.me()
verifyCursorApiKey()callsreportProviderRuntimeReady("cursor")immediately afterCursor.me({ apiKey: key })inapps/desktop/src/main/services/ai/authDetector.ts(around lines 802-830).Cursor.me()validates the API key’s identity, but it doesn’t prove access to the models/catalog. SinceproviderConnectionStatus.tsuseshealth.state === "ready"to setruntimeAvailable(and unblock renderer/chat), a key that later fails the models-list/access checks can temporarily enable UI incorrectly. Gate"ready"on a successful model/access discovery step (e.g., afterCursor.models.list({ apiKey })), or split auth-only vs model-ready health states.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/ai/authDetector.ts` around lines 802 - 830, verifyCursorApiKey() currently calls reportProviderRuntimeReady("cursor") immediately after Cursor.me(...) which only verifies identity and can falsely flip health.state === "ready"; remove that ready call and instead perform a models/access discovery (e.g., call Cursor.models.list({ apiKey: key }) or another model-access check) and only call reportProviderRuntimeReady("cursor") after that succeeds, or alternatively emit a distinct auth-only status and defer setting health.state === "ready" until model discovery completes; update references in verifyCursorApiKey(), Cursor.me, and any logic that relies on providerConnectionStatus.ts / health.state === "ready" accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/main/services/ai/cursorSdkLoader.ts`:
- Around line 7-9: The fallback anchor for createRequire currently uses
process.cwd(), allowing workspace CWD to influence requireFromRuntime
resolution; change the fallback to an application-owned/module-relative path
instead. Update the createRequire call in cursorSdkLoader.ts (the
requireFromRuntime initialization) to compute a module file path anchored to
this module (e.g., derive a file path from import.meta.url using fileURLToPath
and path.dirname, then join that dirname with "package.json") so when __filename
is unavailable you use the module/app package.json location rather than
process.cwd().
In `@apps/desktop/src/main/services/ai/droidSdkLoader.ts`:
- Around line 7-9: The current requireFromRuntime uses process.cwd() when
__filename is missing, which can cause a workspace-installed `@factory/droid-sdk`
to be resolved into the privileged process; change the fallback to an
app/module-relative anchor instead. Update the createRequire call in
droidSdkLoader.ts (the requireFromRuntime creation) to use a path anchored to
this module (e.g., derive a package.json path from __dirname or import.meta.url
/ path.dirname of this file) rather than path.join(process.cwd(),
"package.json"), so the fallback require is resolved from the app/module
directory you control and not the current working directory.
In `@apps/desktop/src/main/services/chat/cursorModelsDiscovery.ts`:
- Around line 630-637: The current failure TTL check that returns null for
recent sdkLastFailure (variables: sdkLastFailure, keyHash, TTL_MS) is bypassed
by the warm path because discoverCursorSdkModelDescriptors(..., { mode:
"cached-or-fallback" }) treats null as a cold cache and calls
warmCursorModelsFromSdk(), causing repeated probes; change the logic so the
failure state is propagated to callers instead of returning null (or
alternatively add the same TTL check inside warmCursorModelsFromSdk) so
discoverCursorSdkModelDescriptors can short-circuit warmCursorModelsFromSdk when
sdkLastFailure exists and is within TTL; update code paths that call
discoverCursorSdkModelDescriptors to handle a distinct failure sentinel (or
thrown/returned failure object) rather than null, and document use of
sdkLastFailure, keyHash and TTL_MS in that flow.
---
Outside diff comments:
In `@apps/desktop/src/main/services/ai/authDetector.ts`:
- Around line 802-830: verifyCursorApiKey() currently calls
reportProviderRuntimeReady("cursor") immediately after Cursor.me(...) which only
verifies identity and can falsely flip health.state === "ready"; remove that
ready call and instead perform a models/access discovery (e.g., call
Cursor.models.list({ apiKey: key }) or another model-access check) and only call
reportProviderRuntimeReady("cursor") after that succeeds, or alternatively emit
a distinct auth-only status and defer setting health.state === "ready" until
model discovery completes; update references in verifyCursorApiKey(), Cursor.me,
and any logic that relies on providerConnectionStatus.ts / health.state ===
"ready" accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d796334a-adc7-4cc6-9ca0-658fc51bc47c
📒 Files selected for processing (30)
apps/ade-cli/src/cursorCloud.tsapps/ade-cli/src/tuiClient/components/ModelPicker/modelPickerLayout.tsapps/ade-cli/tsup.config.tsapps/desktop/src/main/main.tsapps/desktop/src/main/services/ai/aiIntegrationService.tsapps/desktop/src/main/services/ai/authDetector.tsapps/desktop/src/main/services/ai/cursorSdkLoader.tsapps/desktop/src/main/services/ai/droidSdkLoader.tsapps/desktop/src/main/services/ai/providerConnectionStatus.test.tsapps/desktop/src/main/services/ai/providerConnectionStatus.tsapps/desktop/src/main/services/ai/providerTaskRunner.tsapps/desktop/src/main/services/builtInBrowser/builtInBrowserService.test.tsapps/desktop/src/main/services/builtInBrowser/builtInBrowserService.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/main/services/chat/cursorModelsDiscovery.test.tsapps/desktop/src/main/services/chat/cursorModelsDiscovery.tsapps/desktop/src/main/services/chat/cursorSdkPool.test.tsapps/desktop/src/main/services/chat/cursorSdkPool.tsapps/desktop/src/main/services/chat/cursorSdkWorker.tsapps/desktop/src/main/services/chat/droidSdkPool.test.tsapps/desktop/src/main/services/chat/droidSdkPool.tsapps/desktop/src/main/services/chat/droidSdkWorker.tsapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/onboarding/AiRuntimesBand.tsxapps/desktop/src/renderer/components/settings/ProvidersSection.test.tsxapps/desktop/src/renderer/components/settings/ProvidersSection.tsxapps/desktop/src/renderer/components/shared/ModelPicker/providerEmptyState.test.tsxapps/desktop/src/renderer/components/shared/ModelPicker/providerEmptyState.tsxapps/desktop/src/renderer/lib/modelOptions.test.tsapps/desktop/src/renderer/lib/modelOptions.ts
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
June 9, 2026 19:16
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
June 9, 2026 19:22
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
1 participant
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.
Summary
Validation
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests
Greptile Summary
This PR hardens Cursor and Droid SDK loading in packaged/CLI runtimes with a
require-based fallback, gatesruntimeAvailablebehind explicit verification (SDK load + model-list check), and fixes built-in browser views being detached on project-scoped window resolution.cursorSdkLoader/droidSdkLoadermodules add acreateRequire-based fallback for packaged runtimes;cursorSdkPool/droidSdkPoolget asendWorkerMessagehelper with closed-channel guards, an"error"event handler, and sharedrejectPending/cleanupPoolEntryhelpers used by both"error"and"exit"handlers.providerConnectionStatusnow setsruntimeAvailableonly whenproviderRuntimeHealth.state === "ready";authDetector.verifyCursorApiKeyvalidates model access (models.list) in addition to identity (Cursor.me) and reports both auth and non-auth failures;modelOptionsand the model picker only surface Cursor models after runtime is verified.builtInBrowserService.serviceForProjectRootnow callsattachToWindowimmediately on every project-scoped resolution, andmain.tscallsattachToWindowinbindWindowToProjectso views are attached on project switches without waiting for a window-focus event.Confidence Score: 5/5
Safe to merge; all changed paths are well-guarded and covered by the new tests.
The runtime-readiness gating, IPC hardening, and browser-attachment fix are each narrow, self-contained changes with direct test coverage. No pre-existing guarantees are removed, and the fallback loader degrades gracefully when packaged-runtime resolution fails.
No files require special attention.
Important Files Changed
Comments Outside Diff (1)
apps/desktop/src/renderer/components/settings/ProvidersSection.tsx, line 514-538 (link)noticeanderrorbanners can stack simultaneouslyNeither
setNoticenorsetErrorclears its counterpart before setting a value. A user who successfully verifies a key (notice appears, they don't dismiss it) and then immediately tries re-verification that fails will see both the green "Cursor connection verified." banner and the new red error banner at the same time. The same applies in theverifyApiKeypath. AddingsetError(null)beforesetNotice(...)andsetNotice(null)beforesetError(...)in each branch would prevent the overlap.Prompt To Fix With AI
Reviews (2): Last reviewed commit: "Stabilize background CLI draft test" | Re-trigger Greptile