feat: add Cursor Agent CLI as alternative AI provider #273
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a new Cursor provider that runs the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Client/UI
participant Server as App Server
participant CursorAgent as cursor-agent CLI
UI->>Server: POST /api/query (model: cursor-*)
Server->>Server: ProviderFactory -> CursorProvider selected
Server->>CursorAgent: spawn `cursor-agent` (--print --stream-json --model ...)
CursorAgent-->>Server: stream newline-delimited JSON
Server->>Server: buffer & parse JSON lines → convertToProviderMessage
rect rgb(220,240,230)
note over Server: map thinking→assistant, maintain session_id, handle aborts
end
Server-->>UI: stream ProviderMessage events
UI->>Server: optional abort
Server->>CursorAgent: terminate process / cleanup
sequenceDiagram
autonumber
participant UI as Settings UI
participant Server as App Server
participant System as Local System
UI->>Server: GET /api/setup/cursor-status
Server->>System: run `cursor-agent -v` and `cursor-agent status` (or check CURSOR_API_KEY)
System-->>Server: version / installed / auth info
Server-->>UI: CursorStatus JSON
UI->>UI: render CursorCliStatusCard
UI->>Server: GET /api/setup/cursor-status (on refresh)
Server-->>UI: updated status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @tony-nekola-silk, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's AI capabilities by integrating the Cursor Agent CLI as a new, alternative AI provider. This allows users to utilize their existing Cursor subscriptions for AI tasks directly within the application, providing flexibility and reducing dependency on separate Anthropic API keys. The changes encompass both the server-side, with a new provider implementation and CLI status detection, and the user interface, with updated model selection and settings to seamlessly incorporate the new provider. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Cursor Agent CLI as a new AI provider, which is a great addition. The implementation is comprehensive, covering the provider logic on the server, UI updates for model selection and status checking, and extensive testing.
I've identified a few areas for improvement:
- There is duplicated logic for detecting the
cursor-agentCLI. This should be centralized within theCursorProviderto ensure consistency between status checks and command execution. - The installation instructions for the Cursor CLI in the UI appear to have an incorrect package name.
- There's a minor typo in the default model definition for Cursor.
Overall, this is a solid contribution. Addressing these points will improve the robustness and user experience of the new feature.
| async detectInstallation(): Promise<InstallationStatus> { | ||
| try { | ||
| // Try to run cursor-agent --version | ||
| const result = await this.runCommand(['-v']); | ||
|
|
||
| const hasApiKey = !!process.env.CURSOR_API_KEY || !!this.config.apiKey; | ||
|
|
||
| // Check auth status | ||
| let authenticated = false; | ||
| try { | ||
| const statusResult = await this.runCommand(['status']); | ||
| authenticated = !statusResult.includes('not logged in') && !statusResult.includes('error'); | ||
| } catch { | ||
| // Status check failed, assume not authenticated | ||
| } | ||
|
|
||
| return { | ||
| installed: true, | ||
| method: 'cli', | ||
| version: result.trim(), | ||
| hasApiKey, | ||
| authenticated, | ||
| }; | ||
| } catch (error) { | ||
| return { | ||
| installed: false, | ||
| method: 'cli', | ||
| error: (error as Error).message, | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
There's a significant duplication of logic for detecting the cursor-agent CLI. A more robust detection mechanism, which checks common installation paths, exists in apps/server/src/routes/setup/get-cursor-status.ts. However, this provider's detectInstallation and executeQuery methods only work if cursor-agent is in the system's PATH.
This can lead to a situation where the UI shows the CLI as installed (thanks to the logic in get-cursor-status.ts), but agent operations fail because executeQuery can't find the executable.
To fix this and improve robustness, the CLI detection logic should be centralized within this CursorProvider.
I suggest the following refactoring:
- Create a private method in
CursorProvider, e.g.,_findCliPath(), that incorporates the path-finding logic fromget-cursor-status.ts. This method should cache the result. - Update
runCommand()to use this discovered path when spawning processes. - Update
detectInstallation()to use this new centralized logic. - The route handler in
get-cursor-status.tsshould then be simplified to useCursorProvider.detectInstallation().
This will ensure that the CLI path is discovered once and used consistently for both status checks and command execution.
There was a problem hiding this comment.
Fixed in 656dde2 - centralized CLI detection logic in CursorProvider.findCliPath() which checks PATH first, then common installation locations. The get-cursor-status.ts route now delegates to the provider.
| export async function getCursorStatus(): Promise<CursorStatus> { | ||
| let installed = false; | ||
| let version = ''; | ||
| let cliPath = ''; | ||
| let method = 'none'; | ||
|
|
||
| const isWindows = process.platform === 'win32'; | ||
|
|
||
| // Try to find cursor-agent CLI using platform-specific command | ||
| try { | ||
| const findCommand = isWindows ? 'where cursor-agent' : 'which cursor-agent'; | ||
| const { stdout } = await execAsync(findCommand); | ||
| cliPath = stdout.trim().split(/\r?\n/)[0]; | ||
| installed = true; | ||
| method = 'path'; | ||
|
|
||
| // Get version | ||
| try { | ||
| const { stdout: versionOut } = await execAsync('cursor-agent -v'); | ||
| version = versionOut.trim(); | ||
| } catch { | ||
| // Version command might not be available | ||
| } | ||
| } catch { | ||
| // Not in PATH, try common locations based on platform | ||
| const commonPaths = isWindows | ||
| ? (() => { | ||
| const appData = process.env.APPDATA || path.join(os.homedir(), 'AppData', 'Roaming'); | ||
| return [ | ||
| path.join(os.homedir(), '.local', 'bin', 'cursor-agent.exe'), | ||
| path.join(appData, 'npm', 'cursor-agent.cmd'), | ||
| path.join(appData, 'npm', 'cursor-agent'), | ||
| ]; | ||
| })() | ||
| : [ | ||
| path.join(os.homedir(), '.local', 'bin', 'cursor-agent'), | ||
| '/usr/local/bin/cursor-agent', | ||
| path.join(os.homedir(), '.npm-global', 'bin', 'cursor-agent'), | ||
| ]; | ||
|
|
||
| for (const p of commonPaths) { | ||
| try { | ||
| await fs.access(p); | ||
| cliPath = p; | ||
| installed = true; | ||
| method = 'local'; | ||
|
|
||
| // Get version from this path | ||
| try { | ||
| const { stdout: versionOut } = await execAsync(`"${p}" -v`); | ||
| version = versionOut.trim(); | ||
| } catch { | ||
| // Version command might not be available | ||
| } | ||
| break; | ||
| } catch { | ||
| // Not found at this path | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Check authentication | ||
| let auth = { | ||
| authenticated: false, | ||
| method: 'none' as string, | ||
| hasApiKey: !!process.env.CURSOR_API_KEY, | ||
| }; | ||
|
|
||
| // If installed, try to check auth status using cursor-agent status command | ||
| if (installed) { | ||
| try { | ||
| const statusCommand = cliPath ? `"${cliPath}" status` : 'cursor-agent status'; | ||
| const { stdout: statusOut } = await execAsync(statusCommand); | ||
|
|
||
| // Parse status output - if it doesn't contain "not logged in" or error, assume authenticated | ||
| if ( | ||
| !statusOut.toLowerCase().includes('not logged in') && | ||
| !statusOut.toLowerCase().includes('error') && | ||
| !statusOut.toLowerCase().includes('not authenticated') | ||
| ) { | ||
| auth.authenticated = true; | ||
| auth.method = 'oauth'; | ||
| } | ||
| } catch { | ||
| // Status check failed - might not be authenticated | ||
| } | ||
| } | ||
|
|
||
| // Environment variable API key overrides | ||
| if (process.env.CURSOR_API_KEY) { | ||
| auth.authenticated = true; | ||
| auth.hasApiKey = true; | ||
| auth.method = 'api_key_env'; | ||
| } | ||
|
|
||
| return { | ||
| status: installed ? 'installed' : 'not_installed', | ||
| installed, | ||
| method, | ||
| version, | ||
| path: cliPath, | ||
| auth, | ||
| }; | ||
| } |
There was a problem hiding this comment.
This file contains complex logic to detect the cursor-agent CLI installation path. This logic is duplicated and more advanced than the detection logic within CursorProvider itself.
As mentioned in my other comment on cursor-provider.ts, this can cause inconsistencies where the CLI is detected for status checks but not found for execution.
This detection logic should be moved into the CursorProvider class to act as a single source of truth for interacting with the cursor-agent CLI. This file's getCursorStatus function could then be a thin wrapper around a method on the CursorProvider.
There was a problem hiding this comment.
Fixed in 656dde2 - this file now uses CursorProvider.detectInstallation() as the single source of truth. Reduced from 129 lines to 41 lines.
| npm (global) | ||
| </p> | ||
| <code className="text-xs text-foreground/80 font-mono break-all"> | ||
| npm install -g @anthropic-ai/cursor-agent |
There was a problem hiding this comment.
The installation instructions for the Cursor Agent CLI seem to contain an incorrect package name. The package @anthropic-ai/cursor-agent is unlikely to be correct for a Cursor product.
Based on public information, the correct package name is likely @cursor/agent-cli.
Please verify and correct the package name to avoid confusing users.
| npm install -g @anthropic-ai/cursor-agent | |
| npm install -g @cursor/agent-cli |
There was a problem hiding this comment.
Fixed in e20d320 - updated to the correct installation command:
curl https://cursor.com/install -fsS | bash
libs/types/src/model.ts
Outdated
| */ | ||
| export const DEFAULT_MODELS = { | ||
| claude: 'claude-opus-4-5-20251101', | ||
| cursor: 'cursor-sonnet-4', |
There was a problem hiding this comment.
There appears to be a typo in the default model for Cursor. It's set to 'cursor-sonnet-4', but based on other parts of the code (like CURSOR_MODEL_MAP and cursor-provider.ts), it should probably be the alias 'cursor-sonnet' which maps to 'cursor-sonnet-4.5'.
Using an incorrect model string could lead to unexpected behavior or errors when a default model is required.
| cursor: 'cursor-sonnet-4', | |
| cursor: 'cursor-sonnet', |
There was a problem hiding this comment.
Fixed in bc66689 - corrected to cursor-sonnet-4.5 and also added a new cursor-composer model.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
apps/ui/src/components/views/agent-view.tsx (1)
550-552: Consider a more robust label transformation.The chained
.replace()calls work but could fail silently if a model label doesn't contain either prefix. Consider extracting this to a helper or using a regex pattern for clarity.🔎 Optional improvement
- {ALL_MODELS.find((m) => m.id === selectedModel) - ?.label.replace('Claude ', '') - .replace('Cursor ', '') || 'Sonnet'} + {ALL_MODELS.find((m) => m.id === selectedModel) + ?.label.replace(/^(Claude|Cursor)\s+/, '') || 'Sonnet'}apps/server/src/routes/setup/get-cursor-status.ts (3)
35-48: Consider adding a timeout to exec commands.The
execAsynccalls forwhich/whereand version retrieval have no timeout. If the CLI hangs or is unresponsive, these calls could block indefinitely.🔎 Proposed fix to add timeout
try { const findCommand = isWindows ? 'where cursor-agent' : 'which cursor-agent'; - const { stdout } = await execAsync(findCommand); + const { stdout } = await execAsync(findCommand, { timeout: 5000 }); cliPath = stdout.trim().split(/\r?\n/)[0]; installed = true; method = 'path'; // Get version try { - const { stdout: versionOut } = await execAsync('cursor-agent -v'); + const { stdout: versionOut } = await execAsync('cursor-agent -v', { timeout: 5000 }); version = versionOut.trim(); } catch { // Version command might not be available }
73-79: Add timeout for version command in local path detection.Same concern as above - the version command should have a timeout to prevent hanging.
🔎 Proposed fix
// Get version from this path try { - const { stdout: versionOut } = await execAsync(`"${p}" -v`); + const { stdout: versionOut } = await execAsync(`"${p}" -v`, { timeout: 5000 }); version = versionOut.trim(); } catch { // Version command might not be available }
94-111: Add timeout and consider more robust auth status parsing.
- The status command should have a timeout.
- The auth detection relies on absence of specific error strings, which could yield false positives if the CLI output format changes.
🔎 Proposed fix for timeout
if (installed) { try { const statusCommand = cliPath ? `"${cliPath}" status` : 'cursor-agent status'; - const { stdout: statusOut } = await execAsync(statusCommand); + const { stdout: statusOut } = await execAsync(statusCommand, { timeout: 10000 }); // Parse status output - if it doesn't contain "not logged in" or error, assume authenticatedConsider documenting the expected
cursor-agent statusoutput format or adding positive indicators (e.g., "logged in", "authenticated") alongside the negative checks for more robust detection.apps/server/tests/unit/providers/cursor-provider.test.ts (1)
269-296: Consider strengthening the abort signal test.The current test only verifies that
spawnwas called but doesn't actually test the abort behavior. Consider triggering the abort and verifying thatproc.killis called.🔎 Suggested improvement
it('should register abort signal listener on process', async () => { - const mockProcess = createMockProcess([ - JSON.stringify({ type: 'result', subtype: 'success', result: 'test', session_id: 'test' }), - ]); - mockProcess.kill = vi.fn(); - vi.mocked(spawn).mockReturnValue(mockProcess); - const abortController = new AbortController(); - - // Verify the abort controller signal has no listeners initially - expect(abortController.signal.aborted).toBe(false); - + + // Create a process that won't auto-complete + const mockStdout = new Readable({ read() {} }); + const mockStderr = new Readable({ read() { this.push(null); } }); + const mockProcess = new EventEmitter() as any; + mockProcess.stdout = mockStdout; + mockProcess.stderr = mockStderr; + mockProcess.kill = vi.fn(() => { + mockStdout.push(null); + mockProcess.emit('close', 0); + }); + vi.mocked(spawn).mockReturnValue(mockProcess); + const generator = provider.executeQuery({ prompt: 'Test', model: 'cursor-sonnet-4', cwd: '/test', abortController, }); - // Consume the generator - for await (const _ of generator) { - // Just consume - } + // Start consuming, then abort + const iterPromise = generator.next(); + abortController.abort(); + + try { + await iterPromise; + for await (const _ of generator) {} + } catch { + // Expected if abort causes error + } - // The abort signal listener should have been added (we can't easily test this without - // actually aborting, but we verify that the code path ran without error) - expect(spawn).toHaveBeenCalled(); + expect(mockProcess.kill).toHaveBeenCalledWith('SIGTERM'); });apps/server/src/providers/cursor-provider.ts (1)
113-118: Abort listener is never removed — potential memory leak.The
addEventListenercall on line 115 adds a listener that's never cleaned up. If the process completes normally before abort, the listener remains attached to theAbortSignal. While minor for short-lived signals, it's good practice to remove the listener after the process exits.🔎 Proposed fix
// Handle abort if (abortController) { - abortController.signal.addEventListener('abort', () => { + const onAbort = () => { proc.kill('SIGTERM'); - }); + }; + abortController.signal.addEventListener('abort', onAbort); + proc.on('close', () => { + abortController.signal.removeEventListener('abort', onAbort); + }); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
apps/server/src/providers/cursor-provider.tsapps/server/src/providers/provider-factory.tsapps/server/src/routes/setup/get-cursor-status.tsapps/server/src/routes/setup/index.tsapps/server/src/routes/setup/routes/cursor-status.tsapps/server/tests/unit/providers/cursor-provider.test.tsapps/server/tests/unit/providers/provider-factory.test.tsapps/ui/src/components/views/agent-view.tsxapps/ui/src/components/views/board-view/shared/model-constants.tsapps/ui/src/components/views/board-view/shared/model-selector.tsxapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/cli-status/cursor-cli-status.tsxapps/ui/src/components/views/settings-view/cli-status/index.tsapps/ui/src/components/views/settings-view/config/navigation.tsapps/ui/src/components/views/settings-view/hooks/use-cli-status.tsapps/ui/src/components/views/settings-view/hooks/use-settings-view.tsapps/ui/src/components/views/settings-view/shared/types.tsapps/ui/src/lib/electron.tsapps/ui/src/lib/http-api-client.tslibs/types/src/index.tslibs/types/src/model.tslibs/types/src/settings.ts
🧰 Additional context used
🧬 Code graph analysis (7)
apps/server/src/routes/setup/get-cursor-status.ts (1)
init.mjs (1)
isWindows(33-33)
apps/server/tests/unit/providers/cursor-provider.test.ts (2)
apps/server/src/providers/cursor-provider.ts (1)
CursorProvider(46-435)apps/server/src/lib/events.ts (1)
EventEmitter(10-13)
apps/server/src/routes/setup/index.ts (1)
apps/server/src/routes/setup/routes/cursor-status.ts (1)
createCursorStatusHandler(9-22)
apps/ui/src/components/views/settings-view.tsx (3)
apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts (1)
useCliStatus(39-161)apps/ui/src/components/views/settings-view/cli-status/index.ts (2)
ClaudeCliStatus(1-1)CursorCliStatusCard(2-2)apps/ui/src/components/views/settings-view/cli-status/cursor-cli-status.tsx (1)
CursorCliStatusCard(12-134)
apps/server/src/routes/setup/routes/cursor-status.ts (1)
apps/server/src/routes/setup/get-cursor-status.ts (1)
getCursorStatus(26-129)
apps/server/src/providers/cursor-provider.ts (3)
libs/types/src/index.ts (4)
ExecuteOptions(10-10)ProviderMessage(12-12)InstallationStatus(13-13)ModelDefinition(15-15)init.mjs (1)
lines(83-83)apps/ui/src/lib/electron.ts (1)
ModelDefinition(73-73)
apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts (2)
apps/ui/src/store/setup-store.ts (1)
useSetupStore(125-182)apps/ui/src/lib/electron.ts (1)
getElectronAPI(721-730)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (40)
libs/types/src/settings.ts (1)
70-71: LGTM!The
ModelProvidertype extension cleanly adds Cursor support while maintaining backward compatibility with existing Claude configurations.apps/ui/src/lib/electron.ts (2)
554-567: LGTM!The
getCursorStatusmethod follows the established pattern of existing CLI status methods (getClaudeStatus,getGhStatus). The auth object structure is appropriately simplified for Cursor's authentication model.
1248-1260: LGTM!The mock implementation returns appropriate defaults for development/testing scenarios, matching the structure defined in the interface.
libs/types/src/model.ts (1)
30-33: Verify the default Cursor model value.
DEFAULT_MODELS.cursoris set to'cursor-sonnet-4', but this value doesn't appear inCURSOR_MODEL_MAP(which has'cursor-sonnet'mapping to'cursor-sonnet-4.5'). Please verify whether this should be'cursor-sonnet'(the alias) or if'cursor-sonnet-4'is intentionally a direct model ID.apps/server/src/routes/setup/index.ts (1)
7-7: LGTM!The Cursor status route follows the established pattern of other setup routes, using the factory function approach for handler creation.
Also applies to: 21-21
apps/ui/src/components/views/settings-view/cli-status/index.ts (1)
1-2: LGTM!The barrel export cleanly extends the module's public API to include the new Cursor CLI status component.
apps/ui/src/components/views/agent-view.tsx (1)
556-576: LGTM - Well-structured model selector with provider badges.The model selector UI cleanly displays both Claude and Cursor models with distinct provider badges. The widened dropdown and flex layout accommodate the additional information well.
apps/ui/src/components/views/settings-view/shared/types.ts (1)
22-34: LGTM!The
CursorCliStatusinterface appropriately models Cursor's authentication structure with a nestedauthobject, differentiating it from Claude's flatter auth model inCliStatus.apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts (1)
3-12: All references to the old 'claude' view ID have been properly updated.No remaining references to
'claude'found in the settings-view directory. The type definition correctly includes'providers'and the migration from the old view ID has been completed without breaking references.apps/ui/src/components/views/settings-view/config/navigation.ts (1)
24-24: LGTM!The navigation item rename from "Claude" to "AI Providers" correctly reflects the expanded provider support and maintains the appropriate Terminal icon for CLI-based provider configuration.
apps/server/src/routes/setup/routes/cursor-status.ts (1)
1-22: LGTM!Clean route handler implementation that follows the established patterns (consistent with
claude-status.ts). Proper error handling with logging and appropriate HTTP status codes.apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts (3)
21-33: LGTM!The
CursorCliStatusResultinterface correctly mirrors the server-sideCursorStatustype structure, including the auth object withauthenticated,method, andhasApiKeyfields.
63-71: LGTM!Cursor CLI status check follows the same pattern as Claude CLI, with proper optional chaining and error handling. Consistent with the existing implementation style.
137-151: LGTM!The refresh handler follows the same pattern as
handleRefreshClaudeCliwith proper loading state management and error handling.apps/ui/src/lib/http-api-client.ts (1)
444-457: LGTM!The
getCursorStatusmethod follows the established pattern for setup API methods, with a well-typed response that matches the server-sideCursorStatusstructure.apps/server/src/routes/setup/get-cursor-status.ts (1)
121-128: LGTM!The return object correctly constructs the status with all required fields. The logic correctly prioritizes API key authentication when
CURSOR_API_KEYis set.apps/server/src/providers/provider-factory.ts (3)
29-32: LGTM!Clean integration of CursorProvider with consistent prefix-based routing pattern (
cursor-*).
47-48: LGTM!Including CursorProvider in
getAllProviders()ensures it's included in status checks and model enumeration.
83-84: LGTM!Case-insensitive lookup for 'cursor' provider follows the established pattern.
libs/types/src/index.ts (1)
36-45: LGTM!Clean expansion of the type exports to include Cursor-related model maps and type aliases while maintaining backward compatibility with existing exports.
apps/ui/src/components/views/board-view/shared/model-constants.ts (2)
61-61: LGTM!Clean combination of model arrays using spread operator provides a unified list for UI components.
36-59: Model IDs align correctly with CURSOR_MODEL_MAP; documentation inconsistency in types.The UI model IDs (
cursor-opus-thinking,cursor-sonnet,cursor-gpt5) match the keys inCURSOR_MODEL_MAPand are properly handled byCursorProvider.mapModelToCursorFormat(). However, theAgentModeltype documentation inlibs/types/src/model.ts:42incorrectly lists"cursor-sonnet-thinking"instead of"cursor-opus-thinking". Update the comment to reflect the actual available models.apps/ui/src/components/views/settings-view/cli-status/cursor-cli-status.tsx (2)
12-13: LGTM: Good defensive null check.The early return pattern when
statusis null prevents rendering an empty card and ensures the component only displays when there's actual status data to show.
55-91: LGTM: Clean conditional rendering for installed state.The installed state display is well-structured with appropriate optional chaining for
status.method,status.version,status.path, andstatus.authproperties. The auth status indicator using checkmark vs "Not authenticated" text provides clear visual feedback.apps/server/tests/unit/providers/cursor-provider.test.ts (4)
1-10: LGTM: Well-structured test setup.Clean imports and proper mock setup for
child_process.spawn. The mock allows comprehensive testing of the provider without actual CLI execution.
32-59: Good helper function for creating mock processes.The
createMockProcesshelper encapsulates the complexity of mocking Node.js streams and EventEmitter behavior. The 10ms setTimeout for the close event simulates async process completion appropriately.
135-199: Good coverage for model alias mappings.Tests verify all documented mappings:
cursor-sonnet→sonnet-4.5,cursor-gpt5→gpt-5.2,cursor-opus-thinking→opus-4.5-thinking. This ensures the UI aliases correctly translate to cursor-agent CLI format.
358-424: Comprehensive installation detection tests.Good coverage for both success and failure paths in
detectInstallation, including version extraction, CLI not found scenarios, and API key detection via environment variable.apps/ui/src/components/views/settings-view.tsx (2)
12-12: LGTM: Clean import addition.The
CursorCliStatusCardis properly imported from the barrel export alongsideClaudeCliStatus.
89-96: LGTM: Well-integrated Cursor CLI status display.The changes properly:
- Destructure the new cursor-related state from
useCliStatus()- Rename the case to
'providers'to reflect the broader "AI Providers" section- Render
CursorCliStatusCardwith the correct props matching the component interfaceThe Cursor status card is appropriately placed alongside the Claude CLI status for a unified provider management experience.
Also applies to: 107-107, 115-119
apps/server/tests/unit/providers/provider-factory.test.ts (4)
4-4: LGTM: CursorProvider import added for test assertions.
69-89: Good test coverage for Cursor model routing.Tests verify that models with
cursor-*prefix are correctly routed toCursorProvider, including case-insensitivity. The test cases cover representative model variants (cursor-sonnet-4,cursor-sonnet-4-thinking,cursor-gpt-5).
140-148: LGTM: Provider collection tests updated correctly.The tests now verify:
getAllProviders()returns exactly 2 providers (Claude + Cursor)checkAllProviders()returns status for bothclaudeandcursorkeys- Each provider's
installedflag is properly checkedAlso applies to: 164-164, 171-171, 179-180
195-207: LGTM: Name-based lookup and model aggregation tests.Tests verify
getProviderByName('cursor')returnsCursorProviderwith case-insensitivity, andgetAllAvailableModels()includes models withcursorin their IDs.Also applies to: 260-268
apps/server/src/providers/cursor-provider.ts (4)
1-16: LGTM: Clean imports and module setup.Proper imports from
child_process, base provider, and shared types. The docstring provides good context for the provider's purpose.
212-292: LGTM: Comprehensive message type mapping.The
convertToProviderMessagemethod handles all cursor-agent message types appropriately:
system: Filtered out (init messages)user: Passed through with content mappingthinking: Delta messages converted to assistant with thinking block; completed messages filteredassistant: Direct mappingresult/error: Mapped to respective ProviderMessage types
366-406: LGTM: Model definitions are well-structured.Three Cursor models defined with appropriate metadata:
cursor-opus-4.5-thinking: Premium tier with extended thinkingcursor-sonnet-4.5: Standard tier, marked as defaultcursor-gpt-5.2: Premium tier (GPT-5.2 via Cursor)All models correctly specify
provider: 'cursor'and include vision/tools support.
420-434: LGTM: Model mapping handles both aliases and full IDs.The mapping correctly translates:
- UI aliases (
cursor-sonnet,cursor-gpt5,cursor-opus-thinking)- Full model IDs (
cursor-sonnet-4.5,cursor-gpt-5.2,cursor-opus-4.5-thinking)Falls back to the original model string if no mapping exists, which provides forward compatibility.
apps/ui/src/components/views/board-view/shared/model-selector.tsx (2)
14-87: LGTM: Clean extraction of ModelGroup component.The internal
ModelGroupcomponent effectively encapsulates the rendering logic for model option groups with:
- Parameterized icon and color theming (
primaryfor Claude,cyanfor Cursor)- Consistent layout and styling
- Short name extraction that removes common prefixes for compact button labels
This reduces duplication and makes adding future provider groups straightforward.
89-124: LGTM: ModelSelector properly integrates both provider groups.The refactored
ModelSelector:
- Renders Claude models unconditionally as the primary/native provider
- Conditionally renders Cursor models based on
showCursorprop (defaulting totrue)- Passes appropriate theming (
badgeColor) and labels to distinguish the providersThe
showCursorprop provides flexibility for contexts where Cursor models shouldn't be displayed.
| <code className="text-xs text-foreground/80 font-mono break-all"> | ||
| npm install -g @anthropic-ai/cursor-agent | ||
| </code> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@anthropic-ai/cursor-agent npm package
💡 Result:
I couldn't find an npm package named "@anthropic-ai/cursor-agent". Related/alternative packages and projects you may find useful:
- Anthropic official npm packages (TypeScript SDK etc.) — @anthropic-ai/sdk. [1]
- Anthropic's tokenizer repo / package references (@anthropic-ai/tokenizer). [2]
- cursor-tools (npm) — a Cursor-related skills/tools package. [3]
- cursor-agent (community) on GitHub (civai-technologies/cursor-agent) — a Python-based Cursor-style agent project. [4]
If you want, I can (a) search npm for any similarly named packages again, (b) check a specific registry/version, or (c) show install/usage examples for one of the packages above.
🌐 Web query:
Cursor Agent CLI npm official Anthropic package installation
💡 Result:
Short answer — steps and commands:
-
Install official Cursor Agent CLI (recommended):
curl https://cursor.com/install -fsS | bash
Verify: cursor-agent --version. [1] -
If you want the Cursor TypeScript library (task builders) in a Node project:
npm install cursor-agent (or pnpm add cursor-agent). [2] -
Official Anthropic JS/TS SDK (to call Claude from Node):
npm install @anthropic-ai/sdk
Example usage: import Anthropic from '@anthropic-ai/sdk' and set ANTHROPIC_API_KEY. [3]
Sources:
[1] Cursor CLI installation/docs.
[2] cursor-agent npm package (Cursor task-sequence library).
[3] @anthropic-ai/sdk npm (official Anthropic TypeScript/JS SDK).
Fix incorrect npm package name for cursor-agent installation.
The package name @anthropic-ai/cursor-agent does not exist on npm. The correct package name is cursor-agent. Update the installation command to npm install -g cursor-agent to prevent users from encountering installation failures.
🤖 Prompt for AI Agents
In apps/ui/src/components/views/settings-view/cli-status/cursor-cli-status.tsx
around lines 115 to 117, the displayed npm install command uses a non-existent
package name "@anthropic-ai/cursor-agent"; update the text to show the correct
package name by replacing that string with "cursor-agent" so the instruction
reads "npm install -g cursor-agent".
There was a problem hiding this comment.
Fixed in e20d320 - Changed to curl https://cursor.com/install -fsS | bash.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
libs/types/src/model.ts (1)
40-44: JSDoc lists non-existent aliascursor-sonnet-thinking.The comment references
"cursor-sonnet-thinking"butCURSOR_MODEL_MAPdefines'cursor-opus-thinking'instead. Update the doc to match the actual aliases.🔎 Suggested fix
/** * AgentModel - Represents all available models across providers * Claude models: "opus" | "sonnet" | "haiku" - * Cursor models: "cursor-sonnet" | "cursor-sonnet-thinking" | "cursor-gpt5" + * Cursor models: "cursor-opus-thinking" | "cursor-sonnet" | "cursor-gpt5" | "cursor-composer" */apps/server/src/providers/cursor-provider.ts (1)
252-262: Potential race:closelistener attached after stream iteration completes.If the spawned process exits very quickly (e.g., on immediate error), the
closeevent may fire before this Promise's listener is attached, causing the Promise to hang indefinitely. Attach thecloseanderrorlisteners immediately after spawning, before iterating the stream.🔎 Suggested approach
private async *processStream(proc: ChildProcess): AsyncGenerator<ProviderMessage> { let buffer = ''; let sessionId: string | undefined; + + // Set up close promise early to avoid missing the event + const closePromise = new Promise<void>((resolve, reject) => { + proc.on('close', (code) => { + if (code !== 0 && code !== null) { + reject(new Error(`cursor-agent exited with code ${code}`)); + } else { + resolve(); + } + }); + proc.on('error', reject); + }); const stdout = proc.stdout; // ... existing streaming logic ... - // Wait for process to exit - await new Promise<void>((resolve, reject) => { - proc.on('close', (code) => { - if (code !== 0 && code !== null) { - reject(new Error(`cursor-agent exited with code ${code}: ${stderrOutput}`)); - } else { - resolve(); - } - }); - proc.on('error', reject); - }); + // Wait for process to exit + await closePromise;
🧹 Nitpick comments (2)
apps/server/src/providers/cursor-provider.ts (2)
503-518: Consider importingCURSOR_MODEL_MAPfrom shared types.This mapping duplicates the logic in
libs/types/src/model.ts. Using the shared constant would ensure consistency and reduce maintenance burden.🔎 Suggested approach
+import { CURSOR_MODEL_MAP } from '@automaker/types'; + private mapModelToCursorFormat(model: string): string { - const modelMap: Record<string, string> = { - // UI alias -> cursor-agent format - 'cursor-opus-thinking': 'opus-4.5-thinking', - 'cursor-sonnet': 'sonnet-4.5', - 'cursor-gpt5': 'gpt-5.2', - 'cursor-composer': 'composer', - // Full IDs -> cursor-agent format - 'cursor-opus-4.5-thinking': 'opus-4.5-thinking', - 'cursor-sonnet-4.5': 'sonnet-4.5', - 'cursor-gpt-5.2': 'gpt-5.2', - }; + // Build lookup from shared map: alias -> CLI format (strip 'cursor-' prefix from value) + const modelMap: Record<string, string> = {}; + for (const [alias, fullId] of Object.entries(CURSOR_MODEL_MAP)) { + const cliFormat = fullId.replace(/^cursor-/, ''); + modelMap[alias] = cliFormat; + modelMap[fullId] = cliFormat; + } const lowerModel = model.toLowerCase(); return modelMap[lowerModel] || model; }
179-184: Consider removing abort listener on completion to prevent memory leak.The abort event listener is never removed. If the
abortControlleroutlives the process, the listener remains attached. Use{ once: true }or manually remove.🔎 Suggested fix
// Handle abort if (abortController) { - abortController.signal.addEventListener('abort', () => { + abortController.signal.addEventListener('abort', () => { proc.kill('SIGTERM'); - }); + }, { once: true }); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/server/src/providers/cursor-provider.tsapps/server/src/routes/setup/get-cursor-status.tsapps/ui/src/components/views/board-view/shared/model-constants.tsapps/ui/src/components/views/settings-view/cli-status/cursor-cli-status.tsxlibs/types/src/model.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/ui/src/components/views/settings-view/cli-status/cursor-cli-status.tsx
- apps/ui/src/components/views/board-view/shared/model-constants.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/server/src/providers/cursor-provider.ts (1)
libs/types/src/index.ts (4)
ExecuteOptions(10-10)ProviderMessage(12-12)InstallationStatus(13-13)ModelDefinition(15-15)
🪛 GitHub Actions: Test Suite
apps/server/src/providers/cursor-provider.ts
[error] 21-21: Vitest: No "exec" export defined on the mocked 'child_process' module. Cursor-provider.ts uses promisified exec (const execAsync = promisify(exec)) and the unit test for this provider fails due to an invalid mock setup. Update the mock to export/mock exec (e.g., use vi.mock with importOriginal or provide a mock implementation for exec).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (5)
libs/types/src/model.ts (2)
10-18: LGTM!The CURSOR_MODEL_MAP is well-structured with clear alias-to-model mappings. The
as constassertion ensures type safety.
20-26: LGTM!Combining the model maps into
ALL_MODEL_MAPprovides a convenient unified lookup.apps/server/src/providers/cursor-provider.ts (2)
63-108: LGTM!The
findCliPath()implementation is well-designed with proper caching, cross-platform support (Windows vs Unix paths), and graceful fallback through common installation locations.
437-489: LGTM!The
getAvailableModels()provides comprehensive model definitions with appropriate metadata including context windows, capabilities, and tiers.apps/server/src/routes/setup/get-cursor-status.ts (1)
21-41: LGTM!Clean delegation to
CursorProvider.detectInstallation()as the single source of truth for CLI detection. The auth method derivation correctly distinguishes between API key and OAuth authentication.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/server/tests/unit/providers/cursor-provider.test.ts (1)
296-323: The abort signal test doesn't actually verify abort behavior.This test only checks that
spawnwas called, but doesn't verify that the abort signal listener was registered or that aborting kills the process. The comment on lines 320-322 acknowledges this limitation.Consider strengthening this test:
🔎 Proposed improvement to actually test abort behavior
it('should register abort signal listener on process', async () => { - const mockProcess = createMockProcess([ - JSON.stringify({ type: 'result', subtype: 'success', result: 'test', session_id: 'test' }), - ]); - mockProcess.kill = vi.fn(); - vi.mocked(spawn).mockReturnValue(mockProcess); - - const abortController = new AbortController(); - - // Verify the abort controller signal has no listeners initially - expect(abortController.signal.aborted).toBe(false); - - const generator = provider.executeQuery({ - prompt: 'Test', - model: 'cursor-sonnet-4', - cwd: '/test', - abortController, - }); - - // Consume the generator - for await (const _ of generator) { - // Just consume - } - - // The abort signal listener should have been added (we can't easily test this without - // actually aborting, but we verify that the code path ran without error) - expect(spawn).toHaveBeenCalled(); + const mockProcess = new EventEmitter() as any; + mockProcess.stdout = new Readable({ read() { this.push(null); } }); + mockProcess.stderr = new Readable({ read() { this.push(null); } }); + mockProcess.kill = vi.fn(); + vi.mocked(spawn).mockReturnValue(mockProcess); + + const abortController = new AbortController(); + + const generator = provider.executeQuery({ + prompt: 'Test', + model: 'cursor-sonnet-4', + cwd: '/test', + abortController, + }); + + // Start consuming but abort before close + const iterPromise = generator.next(); + abortController.abort(); + + // Allow abort handler to run + await new Promise((r) => setTimeout(r, 0)); + + expect(mockProcess.kill).toHaveBeenCalled(); + + // Clean up - emit close to end the generator + mockProcess.emit('close', 0); + await iterPromise.catch(() => {}); // May throw due to abort });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/server/tests/unit/providers/cursor-provider.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/server/tests/unit/providers/cursor-provider.test.ts (1)
apps/server/src/providers/cursor-provider.ts (1)
CursorProvider(52-519)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (7)
apps/server/tests/unit/providers/cursor-provider.test.ts (7)
1-30: LGTM!The imports and mocks are well-structured. The
osmock correctly handles both named and default exports, which is important for ESM compatibility.
35-50: LGTM!Good test isolation with
vi.clearAllMocks(), fresh provider instance, and environment cleanup inbeforeEach. The default mock forexecprovides sensible baseline behavior for CLI path resolution.
52-56: LGTM!Simple identity test for the provider name.
477-523: LGTM!Good coverage of model availability tests. The tests verify count, specific models, default marker, and feature support flags. Minor note:
cursor-composeris the 4th model but doesn't have an explicit test like the others—this is fine since line 481 validates the count.
525-545: LGTM!Comprehensive feature support tests with clear documentation in the test name explaining why
thinkingreturnsfalse.
547-554: LGTM!Basic validation test confirming the config validates successfully by default.
556-572: LGTM!Good tests for config get/set and merge behavior, verifying that subsequent
setConfigcalls preserve existing keys.
tony-nekola-silk
force-pushed
the
feature/cursor-agent-provider
branch
from
f13cb9b to
b632654
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/ui/src/components/views/board-view/shared/model-selector.tsx (2)
6-6: Remove unused imports.
CLAUDE_MODELSandCURSOR_MODELSare imported but not used directly—models are accessed viaPROVIDERS[].models. This creates dead code.🔎 Proposed fix
-import { CLAUDE_MODELS, CURSOR_MODELS, ModelOption } from './model-constants'; +import type { ModelOption } from './model-constants';
65-72: Short name extraction may need adjustment for future models.The string replacement chain (
replace('Claude ', '').replace('Cursor ', '')...) works for current model names but may produce unexpected results for future models with different naming patterns. Consider extracting this to a helper or adding ashortNameproperty toModelOption.This is a low-priority suggestion for future maintainability—the current approach works fine for the existing models.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
apps/server/src/providers/cursor-provider.tsapps/server/src/providers/provider-factory.tsapps/server/src/routes/setup/get-cursor-status.tsapps/server/src/routes/setup/index.tsapps/server/src/routes/setup/routes/cursor-status.tsapps/server/tests/unit/providers/cursor-provider.test.tsapps/server/tests/unit/providers/provider-factory.test.tsapps/ui/src/components/views/agent-view.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsxapps/ui/src/components/views/board-view/shared/model-constants.tsapps/ui/src/components/views/board-view/shared/model-selector.tsxapps/ui/src/components/views/board-view/shared/provider-constants.tsapps/ui/src/components/views/board-view/shared/provider-selector.tsxapps/ui/src/components/views/profiles-view.tsxapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/cli-status/cursor-cli-status.tsxapps/ui/src/components/views/settings-view/cli-status/index.tsapps/ui/src/components/views/settings-view/config/navigation.tsapps/ui/src/components/views/settings-view/hooks/use-cli-status.tsapps/ui/src/components/views/settings-view/hooks/use-settings-view.tsapps/ui/src/components/views/settings-view/shared/types.tsapps/ui/src/lib/electron.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/store/app-store.tsapps/ui/tests/features/add-feature-provider-selection.spec.tsapps/ui/tests/features/add-feature-to-backlog.spec.tsapps/ui/tests/utils/core/constants.tsapps/ui/tests/utils/project/setup.tsapps/ui/tests/utils/views/board.tslibs/types/src/index.tslibs/types/src/model.tslibs/types/src/settings.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/ui/src/components/views/settings-view/cli-status/index.ts
- apps/ui/src/lib/http-api-client.ts
- apps/ui/src/components/views/agent-view.tsx
- apps/server/src/routes/setup/get-cursor-status.ts
- apps/server/src/routes/setup/index.ts
- apps/ui/src/lib/electron.ts
- apps/ui/src/components/views/settings-view/config/navigation.ts
- apps/server/tests/unit/providers/provider-factory.test.ts
- libs/types/src/settings.ts
- apps/server/src/routes/setup/routes/cursor-status.ts
🧰 Additional context used
🧬 Code graph analysis (12)
apps/ui/src/components/views/profiles-view.tsx (1)
apps/ui/src/components/views/board-view/shared/provider-constants.ts (1)
PROVIDERS(13-30)
apps/server/src/providers/provider-factory.ts (1)
apps/server/src/providers/cursor-provider.ts (1)
CursorProvider(52-522)
apps/ui/src/components/views/board-view/shared/provider-selector.tsx (1)
apps/ui/src/components/views/board-view/shared/provider-constants.ts (2)
ProviderConfig(4-11)PROVIDERS(13-30)
apps/ui/src/components/views/board-view/shared/provider-constants.ts (1)
apps/ui/src/components/views/board-view/shared/model-constants.ts (3)
ModelOption(4-11)CLAUDE_MODELS(13-35)CURSOR_MODELS(37-67)
apps/ui/src/components/views/settings-view/cli-status/cursor-cli-status.tsx (4)
apps/ui/src/components/views/settings-view/shared/types.ts (1)
CursorCliStatus(22-34)apps/ui/src/components/views/settings-view/cli-status/index.ts (1)
CursorCliStatusCard(2-2)apps/ui/src/lib/utils.ts (1)
cn(5-7)apps/ui/src/components/ui/button.tsx (1)
Button(108-108)
apps/ui/tests/features/add-feature-provider-selection.spec.ts (4)
apps/ui/tests/utils/git/worktree.ts (2)
createTempDirPath(52-55)cleanupTempDir(149-153)apps/ui/tests/utils/project/setup.ts (1)
setupRealProject(97-160)apps/ui/tests/utils/core/waiting.ts (1)
waitForNetworkIdle(9-11)apps/ui/tests/utils/views/board.ts (1)
clickAddFeature(98-103)
apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts (2)
apps/ui/src/store/setup-store.ts (1)
useSetupStore(125-182)apps/ui/src/lib/electron.ts (1)
getElectronAPI(721-730)
apps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsx (4)
libs/types/src/index.ts (3)
ModelProvider(63-63)AgentModel(44-44)ThinkingLevel(62-62)libs/types/src/model.ts (1)
AgentModel(45-45)apps/ui/src/components/views/board-view/shared/provider-constants.ts (1)
PROVIDERS(13-30)apps/ui/src/components/views/board-view/shared/model-constants.ts (1)
getModelForProvider(92-107)
apps/server/src/providers/cursor-provider.ts (1)
libs/types/src/index.ts (4)
ExecuteOptions(10-10)ProviderMessage(12-12)InstallationStatus(13-13)ModelDefinition(15-15)
apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx (5)
libs/types/src/index.ts (3)
ModelProvider(63-63)AgentModel(44-44)ThinkingLevel(62-62)libs/types/src/model.ts (1)
AgentModel(45-45)apps/ui/src/components/views/board-view/shared/provider-constants.ts (1)
PROVIDERS(13-30)apps/ui/src/components/views/board-view/shared/model-constants.ts (1)
getModelForProvider(92-107)apps/ui/src/components/views/board-view/shared/model-selector.tsx (1)
ModelSelector(94-134)
libs/types/src/model.ts (1)
libs/types/src/index.ts (7)
CURSOR_MODEL_MAP(38-38)ALL_MODEL_MAP(39-39)CLAUDE_MODEL_MAP(37-37)DEFAULT_MODELS(40-40)ClaudeModelAlias(41-41)CursorModelAlias(42-42)ModelAlias(43-43)
apps/ui/src/components/views/settings-view.tsx (3)
apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts (1)
useCliStatus(39-161)apps/ui/src/components/views/settings-view/cli-status/index.ts (2)
ClaudeCliStatus(1-1)CursorCliStatusCard(2-2)apps/ui/src/components/views/settings-view/cli-status/cursor-cli-status.tsx (1)
CursorCliStatusCard(12-126)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (60)
libs/types/src/model.ts (5)
10-18: LGTM - Cursor model mappings are well-defined.The CURSOR_MODEL_MAP follows the same pattern as CLAUDE_MODEL_MAP and provides clear alias-to-model mappings for the Cursor provider. The const assertion ensures type safety.
20-26: LGTM - Combined model map provides provider-agnostic access.ALL_MODEL_MAP effectively combines both provider maps using the spread operator, enabling unified model resolution across providers.
31-34: LGTM - Cursor default model is appropriately set.The cursor default model is set to 'cursor-opus-4.5-thinking', which aligns with the Claude default pattern of using the full model identifier (not the alias).
36-38: LGTM - Type aliases provide excellent type safety.The new type aliases extract keys from the model maps and provide a unified ModelAlias union type, enabling type-safe provider-agnostic model handling throughout the codebase.
40-44: LGTM - JSDoc documentation is accurate.The JSDoc comment correctly enumerates all available model aliases for both Claude and Cursor providers, matching the actual type definitions. Previous documentation inconsistencies have been addressed.
apps/ui/tests/features/add-feature-to-backlog.spec.ts (1)
74-77: LGTM - Provider selector verification enhances test coverage.The addition verifies that the provider selector is present in the Model tab before proceeding with the feature creation. This is a good sanity check for the new provider UI without disrupting the existing test flow.
apps/ui/src/components/views/settings-view/shared/types.ts (1)
22-34: LGTM - CursorCliStatus interface is well-defined.The CursorCliStatus interface provides a clear type definition for Cursor CLI status information. The nested auth object structure appropriately reflects the Cursor-specific authentication model.
apps/ui/tests/utils/project/setup.ts (2)
794-825: LGTM - Claude profile descriptions updated for provider-agnostic approach.Removing the "Claude" prefix from built-in profile descriptions makes them more provider-agnostic while maintaining clarity through the provider field. The added comment clearly marks the Claude profiles section.
826-857: LGTM - Cursor profiles mirror Claude structure effectively.The three new Cursor profiles (heavy-task, balanced, quick-edit) follow the same structure as Claude profiles with appropriate model selections (opus-thinking, gpt5, composer) and thinking levels. This provides consistent built-in profile coverage across both providers.
apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts (1)
3-12: LGTM - Settings view ID updated for provider-aware navigation.The rename from 'claude' to 'providers' in the SettingsViewId union correctly reflects the expanded settings section that now covers multiple AI providers (Claude and Cursor).
apps/ui/tests/utils/core/constants.ts (1)
103-105: LGTM - Test IDs for provider selection are well-defined.The new test IDs follow existing naming conventions and provide clear selectors for testing provider selection UI (provider dropdown and provider options). These support the provider-aware test scenarios introduced in the PR.
apps/ui/tests/utils/views/board.ts (2)
179-185: LGTM - selectProvider helper is clean and focused.The selectProvider helper provides a simple, type-safe way to select a provider in tests. The 300ms wait allows the UI to update after selection.
187-243: LGTM - fillAddFeatureDialogWithProvider provides comprehensive test utility.The helper effectively extends the original fillAddFeatureDialog with provider and model selection support. The tab switching logic is clean, and the optional handling of provider, model, branch, and category provides flexibility for various test scenarios.
apps/ui/src/components/views/profiles-view.tsx (4)
15-15: LGTM - Imports support provider-aware profiles UI.The new imports (Tabs components, ModelProvider type, and PROVIDERS constant) provide the necessary UI and type support for implementing provider filtering in the profiles view.
Also applies to: 29-30
46-46: LGTM - Provider filtering logic is well-implemented.The selectedProvider state and memoized profile filtering correctly separate built-in and custom profiles by the selected provider. The default to 'claude' is appropriate, and useMemo optimizes performance.
Also applies to: 58-65
147-159: LGTM - Provider tabs UI is clean and maintainable.The Tabs implementation provides a clear provider selection interface. The controlled component pattern with selectedProvider state ensures proper synchronization, and mapping over PROVIDERS makes it maintainable as provider support expands.
67-81: Note: Drag-and-drop maintains global ordering across providers.The handleDragEnd callback operates on the full aiProfiles array rather than the filtered lists. This means reordering profiles within a provider tab affects the global profile order across all providers. This is likely intentional to maintain consistent ordering, but consider whether provider-specific ordering would be more intuitive for users.
apps/ui/src/store/app-store.ts (2)
862-892: LGTM! Good refactoring of profile descriptions.The removal of "Claude" from profile descriptions makes them more generic and reusable. The added comment clearly delineates the Claude profiles section, improving code organization.
893-923: No action required. The Cursor profile model identifiers are correct.The codebase uses short aliases (
cursor-opus-thinking,cursor-gpt5,cursor-composer) in the UI layer intentionally. These are properly mapped to their full identifiers viaCURSOR_MODEL_MAPinlibs/types/src/model.tsand handled consistently by the backend cursor provider. All three models are defined in the type system, model constants, and tested to work correctly.Likely an incorrect or invalid review comment.
libs/types/src/index.ts (1)
36-45: LGTM! Clean API expansion for Cursor model support.The expanded exports properly surface Cursor-related model mappings and types alongside the existing Claude definitions. This maintains backward compatibility while enabling the new Cursor provider functionality.
apps/ui/src/components/views/board-view/shared/provider-selector.tsx (1)
19-51: LGTM! Well-implemented provider selector component.The component follows React best practices with proper TypeScript typing, sensible defaults, and good testability via data-testid attributes. The fallback text handling for missing providers is a nice touch.
apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts (3)
21-33: LGTM! Well-structured interface for Cursor CLI status.The
CursorCliStatusResultinterface appropriately mirrors the Claude CLI status structure while adding Cursor-specific fields like theauthobject. Good consistency in the codebase.
63-71: LGTM! Consistent CLI status checking pattern.The Cursor CLI check follows the same pattern as the Claude CLI check, with proper error handling and state management. The parallel structure makes the code easy to understand and maintain.
137-151: LGTM! Well-implemented refresh functionality.The
handleRefreshCursorClicallback follows the same pattern ashandleRefreshClaudeCli, with proper loading state management and error handling. The use ofuseCallbackis appropriate here.apps/server/src/providers/provider-factory.ts (3)
29-32: LGTM! Clean routing for Cursor models.The cursor-* prefix routing follows the same pattern as Claude model routing, making it consistent and easy to maintain. This provides good extensibility for future providers.
48-48: LGTM! CursorProvider properly integrated.Adding
CursorProviderto the provider list enables it to be discovered and used throughout the application. This is a necessary change for the provider integration.
83-84: LGTM! Direct provider lookup support added.Adding 'cursor' case to the switch statement enables direct provider lookup by name, which is useful for API endpoints and configuration scenarios.
apps/ui/src/components/views/board-view/shared/provider-constants.ts (1)
4-30: LGTM! Well-designed provider configuration system.The
ProviderConfiginterface andPROVIDERSconstant provide a clean abstraction for provider metadata. The use of distinct badge colors (primary for SDK, cyan for CLI) helps users quickly identify provider types. The descriptions are clear and informative.apps/ui/tests/features/add-feature-provider-selection.spec.ts (5)
24-51: LGTM! Thorough test setup.The
beforeAllhook creates a realistic project structure including package.json, Automaker directories, and configuration files. This provides a solid foundation for testing provider selection in a real-world scenario.
57-74: LGTM! Good verification of provider dropdown.The test properly verifies that the provider dropdown is visible and contains the Claude SDK option. The use of
waitForNetworkIdleensures the UI is stable before assertions.
76-97: LGTM! Comprehensive model visibility testing.The test properly verifies that:
- Selecting Claude provider shows Claude models (opus, sonnet, haiku)
- Cursor models are not visible when Claude is selected
This validates the provider-specific model filtering logic.
124-152: LGTM! Excellent test for per-provider model persistence.This test validates a critical UX feature: model selections are remembered per provider. Users can switch between providers without losing their model choice for each. The verification using the
bg-primaryclass is a practical way to check selection state.
154-177: LGTM! Good test for dialog state persistence.This test verifies that the provider selection persists when the dialog is closed and reopened, which is important for user experience. Using keyboard escape to close the dialog adds realism to the test.
apps/ui/src/components/views/settings-view/cli-status/cursor-cli-status.tsx (4)
12-13: LGTM! Proper null handling.Early return when status is null prevents unnecessary rendering and potential errors. This is a good defensive programming practice.
34-48: LGTM! Well-implemented refresh functionality.The refresh button is properly disabled during checking, includes a spinning animation for visual feedback, and has appropriate accessibility attributes (title, data-testid). Good UX considerations.
55-92: LGTM! Comprehensive status display.The installed state display shows all relevant information:
- Detection method
- Version
- Path (with truncation for long paths)
- Authentication status with method
The cyan-themed styling appropriately distinguishes Cursor from Claude. Good attention to detail.
108-119: LGTM! Clear installation instructions.The installation instructions use the correct curl command (as corrected in previous review feedback). The command is displayed in a code block with proper formatting, making it easy for users to copy and execute.
apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx (4)
126-134: Provider memory and ref pattern looks good.The combination of
providerModelMemorystate anduserChangedProviderRefprovides good UX by remembering the user's last selected model per provider and avoiding unwanted provider resets when the dialog reopens. This is a solid pattern for multi-provider selection.
178-184: Provider initialization logic is correct.The initialization correctly derives the provider from the initial model and only resets when the user hasn't explicitly changed the provider. This prevents jarring UX where switching providers gets reverted on dialog reopen.
355-371: Provider selection handler correctly restores model memory.The
handleProviderSelectfunction properly marks user intent, restores the remembered model, and falls back to the first available model if needed. The logic is clean and handles edge cases.
373-388: Profile selection correctly maps to current provider.Using
getModelForProviderensures that when a user selects a profile, they get the equivalent model for their currently selected provider rather than being forced to switch providers.apps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsx (3)
122-131: Provider initialization from feature model is appropriate for edit flow.Unlike the add dialog, the edit dialog correctly re-derives the provider from the feature's model each time. This ensures the UI reflects the actual saved state of the feature being edited.
228-242: Provider switch logic handles model restoration correctly.The
handleProviderSelectfunction restores the remembered model or falls back to the first model of the provider, maintaining consistency with the add dialog pattern.
244-261: Profile selection with model mapping maintains provider context.The
handleProfileSelectcorrectly usesgetModelForProviderto map the profile's base model to the appropriate model for the current provider, then updates the memory for that provider.apps/ui/src/components/views/settings-view.tsx (2)
89-96: Clean hook integration for cursor CLI status.The destructuring pattern follows the existing Claude CLI status pattern, maintaining consistency. All necessary state and handlers are extracted.
107-119: Provider section correctly renders both CLI status cards.The navigation case change from
'claude'to'providers'appropriately reflects the expanded scope. Both Claude and Cursor CLI status cards are rendered with consistent prop patterns.apps/ui/src/components/views/board-view/shared/model-constants.ts (3)
37-67: Cursor models array is well-structured.The CURSOR_MODELS array follows the same pattern as CLAUDE_MODELS with appropriate badges and provider designation. The default model (cursor-opus-thinking) is clearly marked.
75-86: Verify the profile-to-cursor model mappings are intentional.The
sonnet → cursor-gpt5andhaiku → cursor-composermappings may confuse users expecting Claude model equivalents. Consider whether users selecting a "Sonnet profile" would expect GPT-5.2 or Cursor Sonnet 4.5 when switching to the Cursor provider.If this is intentional (e.g., based on capability tiers rather than model families), adding a brief comment explaining the mapping rationale would help maintainability:
🔎 Suggested documentation
export const PROFILE_MODEL_MAP: Record<ModelProvider, Record<string, AgentModel>> = { claude: { opus: 'opus', sonnet: 'sonnet', haiku: 'haiku', }, + // Cursor mappings based on capability tier rather than model family: + // - opus (premium) → cursor-opus-thinking (premium) + // - sonnet (balanced) → cursor-gpt5 (premium tier, different family) + // - haiku (speed) → cursor-composer (different capability focus) cursor: { opus: 'cursor-opus-thinking', sonnet: 'cursor-gpt5', haiku: 'cursor-composer', }, };
92-107: Model resolution function handles edge cases correctly.The
getModelForProviderfunction has good fallback logic: it checks the mapping first, then preserves cursor models for cursor provider, preserves claude models for claude provider, and finally falls back to the original model. This defensive approach prevents runtime errors.apps/server/tests/unit/providers/cursor-provider.test.ts (4)
58-122: Streaming and model mapping tests are comprehensive.The
createMockProcesshelper cleanly encapsulates mock process creation, and the tests verify JSON streaming, message filtering (system messages excluded), and proper message type handling.
162-226: Model alias mapping tests cover all documented aliases.Tests verify mappings for
cursor-sonnet → sonnet-4.5,cursor-gpt5 → gpt-5.2, andcursor-opus-thinking → opus-4.5-thinking, aligning with the implementation'smapModelToCursorFormat.
385-431: Installation detection tests handle multiple spawn calls correctly.The test at lines 419-425 properly uses separate mock processes for version and status commands, avoiding the race condition issue identified in past reviews.
477-522: Model availability tests verify structure and capabilities.Tests confirm 4 models are returned, default model is marked, and all models support vision and tools. Good coverage of the
getAvailableModelscontract.apps/server/src/providers/cursor-provider.ts (5)
63-108: CLI path discovery with caching is well-implemented.The
findCliPathmethod efficiently caches results, checks PATH first, then falls back to common installation locations. Platform-specific paths for Windows and Unix are handled correctly.
119-188: executeQuery correctly constructs CLI arguments and handles abort.The method properly builds CLI arguments including workspace, model mapping, force mode, and resume support. Abort signal handling via
SIGTERMis appropriate for graceful termination.
193-275: Stream processing handles partial JSON lines and errors correctly.The buffering logic for incomplete JSON lines, the early
closePromisesetup (addressing the race condition), and stderr collection for error reporting are all implemented correctly.
497-500: Consider whetherthinkingfeature should be supported for opus-thinking model.The provider exposes
cursor-opus-4.5-thinkingwith extended thinking capabilities, butsupportsFeature('thinking')returnsfalse. If the CLI doesn't expose thinking control as noted in the comment, this is correct. However, users might expect thinking support given the model name.Verify that the cursor-agent CLI doesn't provide thinking budget control. If it does, update
supportsFeatureto returntruefor'thinking'.
506-521: Model mapping is comprehensive and handles case-insensitivity.The
mapModelToCursorFormatfunction covers both UI aliases and full model IDs, with lowercase normalization for robustness. The fallback to the original model prevents errors for unknown models.apps/ui/src/components/views/board-view/shared/model-selector.tsx (2)
19-92: ModelGroup component is well-structured with flexible theming.The internal
ModelGroupcomponent cleanly encapsulates model button rendering with color theming based on provider. The color class mapping for primary/cyan provides clear visual differentiation between Claude and Cursor models.
94-134: ModelSelector correctly integrates ProviderSelector and ModelGroup.The component properly filters providers based on
showCursor, derives available models from the current provider, and passes all necessary props to child components.
Add support for cursor-agent CLI as an alternative AI provider alongside Claude SDK. - Add CursorProvider that wraps cursor-agent CLI with streaming JSON output - Add cursor model types (opus-4.5-thinking, sonnet-4.5, gpt-5.2) - Add model mapping from UI aliases to cursor-agent format - Add cursor CLI status detection endpoint and UI component - Update model selector to show both Claude and Cursor models - Rename "Claude" settings section to "AI Providers" - Add HTTP API client method for cursor status - Add 28 unit tests for CursorProvider
- Add findCliPath() method that checks PATH then common locations - Cache CLI path to avoid repeated filesystem checks - Update executeQuery() and runCommand() to use discovered path - Simplify get-cursor-status.ts to delegate to CursorProvider - Ensures consistent detection for both status checks and execution
Change from incorrect npm package to official curl installer: curl https://cursor.com/install -fsS | bash
…king - Add cursor-composer model across all layers - Change default cursor model from sonnet to opus-thinking - Fix model name mismatch (cursor-sonnet-4 -> cursor-sonnet-4.5)
- Add exec mock for findCliPath method - Add fs/promises and os mocks for path detection - Update spawn expectations to use full CLI path - Update model count from 3 to 4 (added composer) - Update default model test (sonnet -> opus-thinking)
Add ability to select between Claude and Cursor providers in the add/edit feature dialogs with model memory per provider. The profiles view now shows provider tabs to filter profiles. Built-in Cursor profiles are added to match Claude's Heavy Task, Balanced, and Quick Edit profiles.
Rename the display name from "Cursor Agent" to "Cursor CLI" across the UI and code comments while keeping the binary name as cursor-agent.
Add E2E tests for: - Profiles view provider tabs (Claude SDK / Cursor CLI switching) - Agent view model selector (all Claude and Cursor models) Also adds test utilities for provider tab and model selector interactions, and data-testid attributes to profiles view tabs for reliable selection.
Only show profiles matching the selected provider (Claude/Cursor) in the ProfileQuickSelect component for both Add and Edit feature dialogs.
tony-nekola-silk
force-pushed
the
feature/cursor-agent-provider
branch
from
b632654 to
21d5aff
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/types/src/settings.ts (1)
159-159: Update outdated documentation comment.Line 159's comment states
"Provider (currently only "claude")", but theModelProvidertype now includes both'claude'and'cursor'. The comment should be updated to reflect this.Suggested fix
- /** Provider (currently only "claude") */ + /** AI provider (claude or cursor) */ provider: ModelProvider;
♻️ Duplicate comments (1)
apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx (1)
127-132: Hardcoded defaults differ from DEFAULT_MODELS.Same as in edit-feature-dialog, the
providerModelMemoryuses 'cursor-sonnet' whileDEFAULT_MODELS.cursoris 'cursor-opus-4.5-thinking'. Consider using the centralized DEFAULT_MODELS constant for consistency.
🧹 Nitpick comments (8)
apps/ui/src/components/views/agent-view.tsx (2)
556-558: Extract label processing logic to avoid duplication.The model label processing logic (stripping "Claude " and "Cursor " prefixes, defaulting to "Sonnet") is duplicated between the header selector (lines 556-558) and input area selector (lines 965-967). This creates maintenance burden and increases the risk of inconsistencies.
🔎 Suggested refactor to centralize label processing
Extract the label processing to a helper function or compute it once at the component level:
+ // Compute display label once + const selectedModelLabel = useMemo(() => { + const model = ALL_MODELS.find((m) => m.id === selectedModel); + if (!model) return 'Sonnet'; + return model.label.replace('Claude ', '').replace('Cursor ', ''); + }, [selectedModel]); + return ( <div className="flex-1 flex overflow-hidden bg-background" data-testid="agent-view">Then use
selectedModelLabelin both selectors:<Button /* ... */> <Bot className="w-3.5 h-3.5" /> - {ALL_MODELS.find((m) => m.id === selectedModel) - ?.label.replace('Claude ', '') - .replace('Cursor ', '') || 'Sonnet'} + {selectedModelLabel} <ChevronDown className="w-3 h-3 opacity-50" /> </Button>Apply the same change to the input area selector.
Also applies to: 965-967
545-582: Consider extracting the model selector to a reusable component.The model selector dropdown implementation is duplicated between the header (lines 545-582) and input area (lines 957-991). The only differences are the
data-testidvalues and button styling. This duplication makes the component harder to maintain and increases the chance of divergence.Suggested approach
Consider extracting a
<ModelSelectorDropdown>component that accepts:
selectedModelonModelSelecttestIdPrefix(to differentiate testids: "model-selector" vs "model-selector-input")buttonClassName(for styling differences)disabledThis would consolidate the dropdown logic and ensure consistency across both locations.
Also applies to: 957-991
apps/ui/src/components/views/board-view/shared/model-selector.tsx (1)
65-72: Consider consolidating label transformations.The short name extraction uses multiple chained
.replace()calls, which is brittle and could lead to unexpected results if label formats change.Suggested alternatives
Option 1: Store short names in model constants alongside labels:
// In model-constants.ts export const CLAUDE_MODELS: ModelOption[] = [ { id: 'sonnet', label: 'Claude Sonnet 4', shortLabel: 'Sonnet', // ... }, ];Option 2: Use a more robust transformation function:
const getShortName = (label: string): string => { return label .replace(/^(Claude|Cursor)\s+/i, '') .replace(/\s+4$/, '') .replace(/\s+Thinking$/, ' Think'); };Either approach would make the transformation more maintainable and testable.
apps/ui/tests/utils/views/board.ts (1)
187-243: Provider-aware dialog helper looks good, but note the duplication.The
fillAddFeatureDialogWithProviderfunction extends the existingfillAddFeatureDialogwith provider/model support. The implementation is correct and follows established patterns.However, there's significant code duplication between this function and the existing
fillAddFeatureDialog(lines 108-153), particularly for branch and category handling (lines 216-242). Consider consolidating these in a future refactor to reduce maintenance burden.Potential consolidation approach
You could refactor
fillAddFeatureDialogto internally callfillAddFeatureDialogWithProviderwith no provider/model options, or extract the branch/category logic into separate helper functions that both can use.apps/server/src/providers/provider-factory.ts (1)
39-41: Consider explicit error for unknown model prefixes.The current implementation defaults to ClaudeProvider for unknown models with just a console warning. While this provides graceful degradation, it may mask configuration errors or typos in model names. Consider whether throwing an error or returning null for truly unknown models would better surface issues during development.
Alternative approach
- // Default to Claude for unknown models - console.warn(`[ProviderFactory] Unknown model prefix for "${modelId}", defaulting to Claude`); - return new ClaudeProvider(); + // Throw error for unknown models to surface configuration issues + throw new Error( + `[ProviderFactory] Unknown model prefix for "${modelId}". ` + + `Supported prefixes: claude-, cursor-` + );apps/ui/tests/profiles/profiles-provider-selection.spec.ts (1)
74-76: Consider replacingwaitForTimeoutwith deterministic waits.The tests use
page.waitForTimeout(300)after provider tab switches. While this may work, fixed timeouts make tests brittle and unnecessarily slow. Consider waiting for specific UI state changes or data-testid attributes to appear/update instead.Example improvement
// Switch to Cursor CLI await selectProviderTab(page, 'cursor'); - await page.waitForTimeout(300); + // Wait for profile cards to update + await page.locator('[data-testid^="profile-card-"]').first().waitFor();Also applies to: 114-116
apps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsx (1)
126-131: Use DEFAULT_MODELS for cursor default or document the discrepancy.The
providerModelMemoryinitializes cursor to'cursor-sonnet'(which resolves to'cursor-sonnet-4.5'), butDEFAULT_MODELS.cursorinlibs/types/src/model.tsspecifies'cursor-opus-4.5-thinking'. The Claude default ('opus'→'claude-opus-4-5-20251101') is consistent withDEFAULT_MODELS.claude, but the Cursor default differs. Import and useDEFAULT_MODELSfor both providers to maintain consistency with backend defaults, or add a comment explaining why the UI uses a different Cursor model.apps/server/tests/unit/providers/provider-factory.test.ts (1)
69-89: Consider using documented Cursor model IDs in tests.The test models
cursor-sonnet-4,cursor-sonnet-4-thinking, andcursor-gpt-5don't match any documented Cursor model IDs or aliases defined inCursorProvider. While these tests correctly verify thatcursor-*models route toCursorProvider, using undocumented model names reduces test clarity and realism.🔎 Suggested improvement
Use documented model IDs that users would actually use:
describe('Cursor models (cursor-* prefix)', () => { - it('should return CursorProvider for cursor-sonnet-4', () => { - const provider = ProviderFactory.getProviderForModel('cursor-sonnet-4'); + it('should return CursorProvider for cursor-sonnet-4.5', () => { + const provider = ProviderFactory.getProviderForModel('cursor-sonnet-4.5'); expect(provider).toBeInstanceOf(CursorProvider); }); - it('should return CursorProvider for cursor-sonnet-4-thinking', () => { - const provider = ProviderFactory.getProviderForModel('cursor-sonnet-4-thinking'); + it('should return CursorProvider for cursor-opus-thinking', () => { + const provider = ProviderFactory.getProviderForModel('cursor-opus-thinking'); expect(provider).toBeInstanceOf(CursorProvider); }); - it('should return CursorProvider for cursor-gpt-5', () => { - const provider = ProviderFactory.getProviderForModel('cursor-gpt-5'); + it('should return CursorProvider for cursor-gpt-5.2', () => { + const provider = ProviderFactory.getProviderForModel('cursor-gpt-5.2'); expect(provider).toBeInstanceOf(CursorProvider); }); it('should be case-insensitive for cursor models', () => { - const provider = ProviderFactory.getProviderForModel('CURSOR-SONNET-4'); + const provider = ProviderFactory.getProviderForModel('CURSOR-SONNET-4.5'); expect(provider).toBeInstanceOf(CursorProvider); }); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
apps/server/src/providers/cursor-provider.tsapps/server/src/providers/provider-factory.tsapps/server/src/routes/setup/get-cursor-status.tsapps/server/src/routes/setup/index.tsapps/server/src/routes/setup/routes/cursor-status.tsapps/server/tests/unit/providers/cursor-provider.test.tsapps/server/tests/unit/providers/provider-factory.test.tsapps/ui/src/components/views/agent-view.tsxapps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsxapps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsxapps/ui/src/components/views/board-view/shared/model-constants.tsapps/ui/src/components/views/board-view/shared/model-selector.tsxapps/ui/src/components/views/board-view/shared/provider-constants.tsapps/ui/src/components/views/board-view/shared/provider-selector.tsxapps/ui/src/components/views/profiles-view.tsxapps/ui/src/components/views/settings-view.tsxapps/ui/src/components/views/settings-view/cli-status/cursor-cli-status.tsxapps/ui/src/components/views/settings-view/cli-status/index.tsapps/ui/src/components/views/settings-view/config/navigation.tsapps/ui/src/components/views/settings-view/hooks/use-cli-status.tsapps/ui/src/components/views/settings-view/hooks/use-settings-view.tsapps/ui/src/components/views/settings-view/shared/types.tsapps/ui/src/lib/electron.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/store/app-store.tsapps/ui/tests/agent/agent-model-selection.spec.tsapps/ui/tests/features/add-feature-provider-selection.spec.tsapps/ui/tests/features/add-feature-to-backlog.spec.tsapps/ui/tests/profiles/profiles-provider-selection.spec.tsapps/ui/tests/utils/core/constants.tsapps/ui/tests/utils/project/setup.tsapps/ui/tests/utils/views/agent.tsapps/ui/tests/utils/views/board.tsapps/ui/tests/utils/views/profiles.tslibs/types/src/index.tslibs/types/src/model.tslibs/types/src/settings.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- apps/server/src/routes/setup/index.ts
- apps/ui/src/components/views/settings-view/cli-status/cursor-cli-status.tsx
- apps/ui/tests/features/add-feature-to-backlog.spec.ts
- apps/ui/tests/utils/project/setup.ts
- apps/server/src/routes/setup/get-cursor-status.ts
- apps/ui/src/lib/electron.ts
- apps/server/src/routes/setup/routes/cursor-status.ts
- apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts
- apps/ui/src/components/views/board-view/shared/provider-selector.tsx
- apps/ui/src/store/app-store.ts
- apps/ui/tests/features/add-feature-provider-selection.spec.ts
- apps/ui/src/components/views/settings-view/cli-status/index.ts
- apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts
- apps/ui/src/components/views/settings-view/config/navigation.ts
- apps/ui/src/components/views/board-view/shared/provider-constants.ts
- apps/ui/tests/utils/core/constants.ts
🧰 Additional context used
🧬 Code graph analysis (9)
apps/server/tests/unit/providers/cursor-provider.test.ts (2)
apps/server/src/providers/cursor-provider.ts (1)
CursorProvider(52-522)apps/server/src/lib/events.ts (1)
EventEmitter(10-13)
apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx (5)
libs/types/src/index.ts (2)
ModelProvider(63-63)AgentModel(44-44)libs/types/src/model.ts (1)
AgentModel(45-45)apps/ui/src/components/views/board-view/shared/provider-constants.ts (1)
PROVIDERS(13-30)apps/ui/src/components/views/board-view/shared/model-constants.ts (1)
getModelForProvider(92-107)apps/ui/src/components/views/board-view/shared/model-selector.tsx (1)
ModelSelector(94-134)
apps/server/src/providers/provider-factory.ts (2)
apps/server/src/providers/cursor-provider.ts (1)
CursorProvider(52-522)apps/server/src/providers/claude-provider.ts (1)
ClaudeProvider(17-180)
apps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsx (4)
libs/types/src/index.ts (2)
ModelProvider(63-63)AgentModel(44-44)libs/types/src/model.ts (1)
AgentModel(45-45)apps/ui/src/components/views/board-view/shared/provider-constants.ts (1)
PROVIDERS(13-30)apps/ui/src/components/views/board-view/shared/model-constants.ts (1)
getModelForProvider(92-107)
apps/ui/src/components/views/board-view/shared/model-constants.ts (4)
libs/types/src/index.ts (4)
ModelOption(86-86)AgentModel(44-44)ModelProvider(63-63)CLAUDE_MODELS(88-88)libs/types/src/model-display.ts (2)
ModelOption(13-24)CLAUDE_MODELS(41-63)libs/types/src/model.ts (1)
AgentModel(45-45)libs/types/src/settings.ts (2)
AgentModel(12-12)ModelProvider(71-71)
libs/types/src/settings.ts (2)
libs/types/src/index.ts (1)
ModelProvider(63-63)apps/server/src/types/settings.ts (1)
ModelProvider(14-14)
apps/ui/src/components/views/agent-view.tsx (2)
apps/ui/src/components/views/board-view/shared/model-constants.ts (1)
ALL_MODELS(69-69)apps/ui/src/lib/utils.ts (1)
cn(5-7)
apps/server/tests/unit/providers/provider-factory.test.ts (3)
apps/server/src/providers/provider-factory.ts (1)
ProviderFactory(14-109)apps/server/src/providers/cursor-provider.ts (1)
CursorProvider(52-522)apps/server/src/providers/claude-provider.ts (1)
ClaudeProvider(17-180)
apps/ui/tests/agent/agent-model-selection.spec.ts (5)
apps/ui/tests/utils/git/worktree.ts (2)
createTempDirPath(52-55)cleanupTempDir(149-153)apps/ui/tests/utils/project/setup.ts (1)
setupRealProject(97-160)apps/ui/tests/utils/core/waiting.ts (1)
waitForNetworkIdle(9-11)apps/ui/tests/utils/navigation/views.ts (1)
navigateToAgent(77-84)apps/ui/tests/utils/views/agent.ts (3)
clickModelSelector(11-16)selectAgentModel(22-28)getSelectedModelLabel(33-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (19)
apps/ui/src/lib/http-api-client.ts (1)
444-457: LGTM! The newgetCursorStatusmethod correctly follows the existing API pattern and integrates well with the Setup API surface. The implementation is clean and straightforward.The type definition accurately reflects the server-side behavior: the
hasApiKeyproperty is intentionally required (not optional) in Cursor's auth object, as the server always returns this value. This differs fromgetClaudeStatus, where most auth properties are optional, which is by design—Cursor has a simpler auth model that consistently includeshasApiKey.apps/ui/src/components/views/agent-view.tsx (1)
53-53: LGTM!The import change from
CLAUDE_MODELStoALL_MODELScorrectly aligns with the multi-provider architecture, now including both Claude and Cursor models.apps/ui/tests/utils/views/profiles.ts (1)
7-37: LGTM!The new provider tab utilities are well-designed:
- Clear, single-purpose functions with good naming
- Appropriate use of
data-testidselectors for test stability- Proper wait conditions for tab visibility and state changes
These helpers will make provider-related tests more maintainable and readable.
apps/ui/src/components/views/board-view/shared/model-selector.tsx (2)
19-92: LGTM! Clean extraction of ModelGroup component.The
ModelGroupcomponent is well-structured with:
- Clear separation of concerns for rendering provider-specific model lists
- Flexible theming via
badgeColorparameter- Proper TypeScript typing for all props
The component enables provider-aware UI presentation while maintaining consistency across different providers.
94-134: LGTM! Provider-aware ModelSelector implementation.The refactored
ModelSelectorcorrectly integrates:
- Provider selection via
ProviderSelectorcomponent- Dynamic model list based on selected provider
- Conditional provider filtering with
showCursorflag- Provider-specific icons and theming
This provides a solid foundation for multi-provider support in the UI.
apps/ui/tests/utils/views/agent.ts (1)
4-44: LGTM! Well-designed test utilities.The new model selector utilities provide a clean API for test interactions:
- Clear function names and JSDoc documentation
- Appropriate use of
data-testidselectors- Consistent patterns with existing test utilities
The
waitForTimeoutcalls (lines 15, 27) are acceptable for waiting on dropdown animations in E2E tests, though ideally we'd wait for specific UI state changes. Given the simplicity and performance of these operations, the current approach is pragmatic.apps/ui/tests/agent/agent-model-selection.spec.ts (1)
1-145: LGTM! Comprehensive E2E test coverage.This test suite provides excellent coverage for agent model selection:
- Proper test isolation with temporary project setup/teardown
- Tests verify visibility of all models (both Claude and Cursor)
- Provider badge verification ensures UI correctness
- Model switching tests validate state management
- Good use of the new test utilities for consistent interactions
The tests follow best practices with clear AAA structure and meaningful assertions.
apps/ui/tests/utils/views/board.ts (1)
179-185: LGTM! Simple and effective provider selection utility.The
selectProviderfunction provides a clean interface for switching providers in tests, with appropriate wait time for UI updates.libs/types/src/settings.ts (1)
71-71: LGTM! ModelProvider type correctly extended.The addition of
'cursor'to theModelProviderunion type properly supports the new Cursor provider integration.apps/ui/src/components/views/settings-view/shared/types.ts (1)
22-34: LGTM! CursorCliStatus interface is well-structured.The new
CursorCliStatusinterface provides appropriate typing for Cursor CLI status information:
- Clean separation from the existing
CliStatusinterface- The nested
authobject provides better organization for authentication-related fields- All fields are properly typed with optional markers where appropriate
This interface will support the Cursor CLI status card and related UI components effectively.
apps/ui/src/components/views/profiles-view.tsx (1)
46-46: LGTM: Clean provider-aware filtering implementation.The provider selection state and filtering logic are well-implemented. The use of
useMemofor filtered profile lists ensures efficient re-renders, and the default 'claude' provider provides a sensible starting state. The UI integration with proper test IDs supports the E2E test coverage mentioned in the PR.Also applies to: 57-65, 147-163
apps/server/src/providers/provider-factory.ts (1)
29-32: LGTM: Cursor model routing implemented correctly.The cursor-* model routing follows the established pattern and integrates cleanly with the existing provider factory design. The lowercase comparison ensures case-insensitive matching.
apps/ui/tests/profiles/profiles-provider-selection.spec.ts (1)
22-42: LGTM: Comprehensive provider selection test coverage.The test suite effectively validates provider tabs display, switching behavior, and per-provider profile filtering. The use of proper test IDs and helper utilities makes the tests maintainable and readable.
Also applies to: 44-61, 63-84
libs/types/src/index.ts (1)
36-45: LGTM: Clean public API expansion for Cursor models.The expanded exports properly expose the new Cursor model mappings and type aliases while maintaining consistency with the existing Claude exports.
apps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsx (1)
144-148: LGTM: Well-structured provider memory and model mapping.The provider selection logic with per-provider model memory is cleanly implemented. The initialization from the feature's model, the memory updates during selection, and the restoration when switching providers all work together cohesively.
Also applies to: 209-226, 228-242
apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx (2)
133-134: Consider scope of userChangedProviderRef persistence.The
userChangedProviderReftracks whether the user explicitly changed the provider, preventing automatic resets when the dialog reopens. Once set totrue, it persists across all future dialog opens for the entire component lifetime. This means:
- User opens dialog (provider auto-selects based on default profile)
- User manually switches to Cursor
- User closes dialog
- User opens dialog again → provider stays as Cursor even if default profile is Claude
Is this the intended behavior? If users expect each new feature to start with the default profile's provider, the ref might need to reset when the dialog closes or when
openchanges tofalse.Also applies to: 357-358
337-353: LGTM: Consistent provider memory implementation.The provider selection and model memory logic follows the same clean pattern as the edit dialog, with the added benefit of tracking explicit user provider changes via
userChangedProviderRef.Also applies to: 355-371
libs/types/src/model.ts (1)
10-18: LGTM: Clean Cursor model mapping implementation.The Cursor model mappings follow the established pattern from Claude models. The ALL_MODEL_MAP properly combines both provider maps, and the DEFAULT_MODELS cursor entry uses the full model ID consistently with the Claude default.
Also applies to: 20-26, 33-33
apps/ui/src/components/views/settings-view.tsx (1)
107-119: LGTM: Clean Cursor CLI status integration.The addition of CursorCliStatusCard alongside ClaudeCliStatus properly expands the settings to support both providers. The rename from 'claude' to 'providers' for the switch case improves semantic clarity.
| export const PROFILE_MODEL_MAP: Record<ModelProvider, Record<string, AgentModel>> = { | ||
| claude: { | ||
| opus: 'opus', | ||
| sonnet: 'sonnet', | ||
| haiku: 'haiku', | ||
| }, | ||
| cursor: { | ||
| opus: 'cursor-opus-thinking', | ||
| sonnet: 'cursor-gpt5', | ||
| haiku: 'cursor-composer', | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Profile model mappings don't preserve capability tiers.
The PROFILE_MODEL_MAP cursor mappings don't align with capability tiers, which could lead to unexpected behavior and costs:
opus→cursor-opus-thinking: ✅ Premium → Premium (correct)sonnet→cursor-gpt5:⚠️ Balanced → Premium (tier mismatch)haiku→cursor-composer:⚠️ Speed → Premium (tier mismatch)
When users select a profile with sonnet (balanced tier) and switch to the Cursor provider, they'll get cursor-gpt5 (premium tier). Similarly, haiku (speed/basic) maps to premium. This breaks the expected cost/capability relationship.
🔎 Recommended mapping to preserve tiers
cursor: {
opus: 'cursor-opus-thinking',
- sonnet: 'cursor-gpt5',
- haiku: 'cursor-composer',
+ sonnet: 'cursor-sonnet', // Balanced → Balanced
+ haiku: 'cursor-composer', // or 'cursor-sonnet' if no fast option
},Note: Based on CURSOR_MODELS definitions (lines 37-67), cursor-sonnet is marked as "Balanced" and would be a more appropriate match for Claude's sonnet.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const PROFILE_MODEL_MAP: Record<ModelProvider, Record<string, AgentModel>> = { | |
| claude: { | |
| opus: 'opus', | |
| sonnet: 'sonnet', | |
| haiku: 'haiku', | |
| }, | |
| cursor: { | |
| opus: 'cursor-opus-thinking', | |
| sonnet: 'cursor-gpt5', | |
| haiku: 'cursor-composer', | |
| }, | |
| }; | |
| export const PROFILE_MODEL_MAP: Record<ModelProvider, Record<string, AgentModel>> = { | |
| claude: { | |
| opus: 'opus', | |
| sonnet: 'sonnet', | |
| haiku: 'haiku', | |
| }, | |
| cursor: { | |
| opus: 'cursor-opus-thinking', | |
| sonnet: 'cursor-sonnet', // Balanced → Balanced | |
| haiku: 'cursor-composer', // or 'cursor-sonnet' if no fast option | |
| }, | |
| }; |
🤖 Prompt for AI Agents
In apps/ui/src/components/views/board-view/shared/model-constants.ts around
lines 75 to 86, the Cursor entries in PROFILE_MODEL_MAP do not preserve
capability tiers (sonnet and haiku currently map to premium Cursor models), so
update the mappings to pick Cursor model IDs that match the same capability tier
as the Claude profiles: change sonnet → 'cursor-sonnet' (Balanced) and replace
'cursor-composer' for haiku with the Cursor model ID that is marked as "Speed"
in CURSOR_MODELS; ensure each mapping uses the Cursor model whose tier label
matches the original profile tier.
- Change Balanced profile to use cursor-sonnet instead of cursor-gpt5 - Make cursor-sonnet the default Cursor model - Update cursor-composer badge from Premium to Fast - Fix PROFILE_MODEL_MAP to preserve tier mappings Addresses CodeRabbit feedback on PR AutoMaker-Org#273.
|
@coderabbitai Addressed in 27edc13 Fixed the PROFILE_MODEL_MAP tier mappings:
|
|
Tip For best results, initiate chat on the files or code changes. @tony-nekola-silk, thank you for the detailed update! The tier mapping fixes look great: ✅ Balanced tier alignment: Mapping ✅ Speed tier preserved: Keeping ✅ Sensible default: Making ✅ Profile consistency: Updating the Balanced Cursor profile to use These changes improve the provider parity and should make the model selection experience more intuitive for users switching between providers. 🎯 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/ui/src/components/views/board-view/shared/model-constants.ts (2)
37-67: Inconsistent badge terminology for equivalent tiers.The
cursor-composermodel (line 64) uses the badge "Fast", while the equivalent Claude modelhaiku(line 18) uses "Speed". Since these models map to each other viaPROFILE_MODEL_MAP(line 84), they represent the same capability tier but use different labels.Consider standardizing on a single term (either "Speed" or "Fast") for consistency across providers.
🔎 Suggested alignment
Option 1: Align Cursor to use "Speed"
{ id: 'cursor-composer', label: 'Cursor Composer', description: 'Cursor Composer model.', - badge: 'Fast', + badge: 'Speed', provider: 'cursor', },Option 2: Align Claude to use "Fast"
{ id: 'haiku', label: 'Claude Haiku', description: 'Fast and efficient for simple tasks.', - badge: 'Speed', + badge: 'Fast', provider: 'claude', },
92-107: Consider more robust model identification.The function uses string prefix checking (
startsWith('cursor-')) to identify cursor models. While this works for the current model set, it makes assumptions that could break if:
- Cursor models without the
cursor-prefix are added- Additional providers are introduced
- Model naming conventions change
A more maintainable approach would check model existence in the
CURSOR_MODELSorCLAUDE_MODELSarrays.🔎 Alternative implementation using array lookups
export function getModelForProvider(profileModel: AgentModel, provider: ModelProvider): AgentModel { const mapping = PROFILE_MODEL_MAP[provider]; if (mapping && mapping[profileModel]) { return mapping[profileModel]; } - // If it's already a cursor model and provider is cursor, keep it - if (provider === 'cursor' && profileModel.startsWith('cursor-')) { + // If it's already a model for the target provider, keep it + const isCursorModel = CURSOR_MODELS.some(m => m.id === profileModel); + const isClaudeModel = CLAUDE_MODELS.some(m => m.id === profileModel); + + if (provider === 'cursor' && isCursorModel) { return profileModel; } - // If it's a claude model and provider is claude, keep it - if (provider === 'claude' && !profileModel.startsWith('cursor-')) { + if (provider === 'claude' && isClaudeModel) { return profileModel; } // Fallback to the original model return profileModel; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/ui/src/components/views/board-view/shared/model-constants.tsapps/ui/src/store/app-store.tsapps/ui/tests/utils/project/setup.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/ui/src/store/app-store.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (3)
apps/ui/src/components/views/board-view/shared/model-constants.ts (1)
75-86: Tier preservation concern from past review has been addressed.The profile model mappings now correctly preserve capability tiers:
opus→cursor-opus-thinking: Premium → Premium ✅sonnet→cursor-sonnet: Balanced → Balanced ✅haiku→cursor-composer: Speed → Fast ✅This addresses the previous concern about tier misalignment where
sonnetwas mapping tocursor-gpt5(Premium tier).apps/ui/tests/utils/project/setup.ts (2)
794-825: LGTM! Description updates improve clarity.The removal of the "Claude" prefix from profile descriptions is a good refactor, as the
providerfield already distinguishes the provider. The added comment helps organize the profiles array.
827-857: Profile model types are properly defined—no action required, but note the duplicate names.The model types
cursor-opus-thinking,cursor-sonnet, andcursor-composerare correctly defined inapps/ui/src/components/views/board-view/shared/model-constants.tsand are part of the application's type system. The Cursor profiles follow the same naming pattern as Claude profiles ("Heavy Task", "Balanced", "Quick Edit"), but since profile IDs and provider fields are distinct, and test code does not select profiles by name alone, this should not cause ambiguity in practice.
I don't see you adding the cursor CLI setup to the setup wizard. Is that something that maybe I missed or is that not added in this port request? |
|
Hey, I checked out your branch. Can you add some console logs to the cursor provider and verify this is actually getting ran when you select it in the feature model tab? Because I've tried this and it never actually ran GPT 5.2. I had to do a bunch of code changes to try to get this working and still it's not outputting any logs after it ran GPT 5. Verify your code actually runs the cursor stuff because I don't think it's working. |
webdevcody
added
Do Not Merge
2 participants
Summary
Add support for Cursor Agent CLI as an alternative AI provider alongside the existing Claude SDK. This enables users to leverage their Cursor subscription for AI tasks without requiring a separate Anthropic API key.
Key Changes
Test Plan
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.