Cursor Sdk Cleanup#360
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@copilot review but do not make fixes |
📝 WalkthroughWalkthroughThis PR consolidates Linear issue parsing across lane and chat services and refactors slash command discovery for Claude, Codex, and Cursor into a unified architecture. The changes introduce shared markdown-based discovery utilities, add Cursor slash command support, integrate all providers into the chat service, and expose lane Linear issue linking via the ADE CLI. ChangesLane Linear Linking & Chat Slash Command Services
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review
Scope: 32 file(s), +1478 / −716
Verdict: Looks good
This PR bumps @cursor/sdk to 1.0.13, extracts shared markdown slash-command discovery, adds Cursor-specific discovery/expansion, relaxes Linear issue linking for project-less issues, and wires CLI/TUI surfaces (link-linear-issue, dynamic Cursor model discovery). The diff is mostly mechanical extraction with targeted behavior changes that are covered by new tests.
Notes
- Cursor slash commands:
getSlashCommandsfor the Cursor provider now lists.cursor/commands, agents, skills, and built-in subagents instead of the generic Claude/Codex filesystem merge; send-time expansion for.cursor/commandsis new and matches the documented split (agents/skills stay native to the SDK). This aligns autocomplete with what Cursor can actually expand. - Linear issues:
parseLaneLinearIssueValue/isLinkableLaneLinearIssueintentionally allow emptyprojectId/projectSlug(seelaneService.test.tsandlaneLinearIssue.test.ts); primaryupsertLaneLinearIssuestill enforces core identity fields vialaneLinearIssueMissingFields. - Cursor worker:
settingSources: ["all"]widens which Cursor config layers load versus["project", "user"]; worth watching in production if team-level rules surprise users, but ADE policy hooks still gate tool execution. - Dependency:
@cursor/sdk^1.0.9 → ^1.0.13 (lockfile pins platform binaries at 1.0.13). No issues spotted in the bump itself. - Validation: Targeted vitest was not run in this environment (
vitestnot installed underapps/desktop); findings are from diff review and existing test coverage in the PR.
Sent by Cursor Automation: BUGBOT in Versic
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Reviewed only (no fixes applied) against commit 6bd85b0. One issue found: Targeted tests passed:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ade-cli/src/cli.ts`:
- Around line 2588-2590: The CLI currently allows both --include-in-pr and
--no-include-in-pr to be passed and the later assignment wins; update the
parsing logic around readFlag(args, ["--no-include-in-pr"]), readFlag(args,
["--include-in-pr"]) and input.includeInPr to detect when both flags are present
and fail fast: if both readFlag(...) calls return true, emit a clear error
message and exit with a non-zero status (or throw an Error) instead of silently
overriding; otherwise continue setting input.includeInPr as currently
implemented.
In `@apps/ade-cli/src/tuiClient/__tests__/adeApi.test.ts`:
- Around line 236-257: The test "includes Cursor command files and subagents"
currently only asserts discoverProjectSlashCommands(projectRoot) results; add
two assertion paths that invoke the CLI-layer verification for both headless and
desktop RPC transports: call the ADE CLI verification entry points used in tests
(the headless-mode verifier and the desktop socket-backed ADE RPC verifier —
e.g., the functions or helpers that simulate headless invocation and the desktop
socket-backed RPC client used elsewhere in tests) using the same temporary
projectRoot created by makeTmpRoot, then assert each returns/produces the
expected commands (matching objects with name "/review-code" and "/verifier" and
the same descriptions). Ensure you reference the same
discoverProjectSlashCommands-derived expectations so each transport path
(headless and desktop socket-backed ADE RPC) has its own
expect(...).toEqual/arrayContaining checks to prevent transport-specific
regressions.
In `@apps/desktop/src/main/services/chat/cursorSdkWorker.ts`:
- Line 343: The change broadens configuration layers by setting settingSources
to ["all"]; revert it to only the intended lane-scoped layers by using
settingSources: ["project","user"] in the Cursor SDK agent options (where the
ADE/Cursor agent is initialized) or, if broader sources must remain, add
explicit validation that filters/whitelists allowed layers and rejects
team/enterprise/plugin overrides before applying settings; update the agent
initialization path that sets settingSources to enforce this constraint and add
a short comment documenting the security rationale.
In `@apps/desktop/src/main/services/lanes/laneService.ts`:
- Around line 2380-2382: linkLinearIssues currently accepts items that pass
isLinkableLaneLinearIssue but later parseLaneLinearIssueLink uses
parseLaneLinearIssueJson which requires additional fields, causing stored links
to be dropped on read; update linkLinearIssues to validate entries against the
same requirements as parseLaneLinearIssueJson by calling
parseLaneLinearIssueJson (or reuse its validation logic) on the finalized item
(finalizeLaneLinearIssue result) before inserting, and skip/log items that fail
parseLaneLinearIssueJson so only round-trippable link rows are stored; ensure
you reference finalizeLaneLinearIssue, isLinkableLaneLinearIssue,
parseLaneLinearIssueJson, and parseLaneLinearIssueLink in the change so the
validation is aligned.
In `@apps/desktop/src/renderer/components/lanes/LinearIssueBadge.tsx`:
- Line 58: The project selection currently uses issue.projectName?.trim() ||
issue.projectSlug || issue.teamKey, which lets a whitespace-only projectSlug
win; update the fallback to trim projectSlug as well by using
issue.projectName?.trim() || issue.projectSlug?.trim() || issue.teamKey so
blank/whitespace projectSlug falls through to teamKey (change the variable
assignment for project in LinearIssueBadge component).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2506e367-9441-4015-88a1-dffa686e6e22
⛔ Files ignored due to path filters (6)
apps/ade-cli/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsondocs/ARCHITECTURE.mdis excluded by!docs/**docs/features/agents/README.mdis excluded by!docs/**docs/features/chat/README.mdis excluded by!docs/**docs/features/chat/agent-routing.mdis excluded by!docs/**docs/features/lanes/README.mdis excluded by!docs/**
📒 Files selected for processing (26)
apps/ade-cli/package.jsonapps/ade-cli/src/cli.test.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/tuiClient/__tests__/adeApi.test.tsapps/ade-cli/src/tuiClient/adeApi.tsapps/desktop/src/main/services/adeActions/registry.tsapps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/main/services/chat/claudeSlashCommandDiscovery.test.tsapps/desktop/src/main/services/chat/claudeSlashCommandDiscovery.tsapps/desktop/src/main/services/chat/codexSlashCommandDiscovery.tsapps/desktop/src/main/services/chat/cursorSdkWorker.tsapps/desktop/src/main/services/chat/cursorSlashCommandDiscovery.test.tsapps/desktop/src/main/services/chat/cursorSlashCommandDiscovery.tsapps/desktop/src/main/services/chat/markdownSlashCommandDiscovery.tsapps/desktop/src/main/services/chat/projectSlashCommandDiscovery.tsapps/desktop/src/main/services/chat/slashCommandPromptExpansion.tsapps/desktop/src/main/services/lanes/laneService.test.tsapps/desktop/src/main/services/lanes/laneService.tsapps/desktop/src/renderer/components/lanes/LinearIssueBadge.tsxapps/desktop/src/shared/agentSkillRoots.test.tsapps/desktop/src/shared/agentSkillRoots.tsapps/desktop/src/shared/chatContextAttachments.test.tsapps/desktop/src/shared/chatContextAttachments.tsapps/desktop/src/shared/laneLinearIssue.test.tsapps/desktop/src/shared/laneLinearIssue.ts
| if (readFlag(args, ["--no-include-in-pr"])) input.includeInPr = false; | ||
| if (readFlag(args, ["--include-in-pr"])) input.includeInPr = true; | ||
| if (readFlag(args, ["--close-on-merge"])) input.closeOnMerge = true; |
There was a problem hiding this comment.
Reject contradictory include-in-pr flags
Passing both --include-in-pr and --no-include-in-pr currently succeeds and silently picks the later assignment. Please fail fast on conflicting flags to avoid ambiguous CLI behavior.
Suggested fix
- if (readFlag(args, ["--no-include-in-pr"])) input.includeInPr = false;
- if (readFlag(args, ["--include-in-pr"])) input.includeInPr = true;
+ const noIncludeInPr = readFlag(args, ["--no-include-in-pr"]);
+ const includeInPr = readFlag(args, ["--include-in-pr"]);
+ if (noIncludeInPr && includeInPr) {
+ throw new CliUsageError(
+ "Use either --include-in-pr or --no-include-in-pr, not both.",
+ );
+ }
+ if (noIncludeInPr) input.includeInPr = false;
+ if (includeInPr) input.includeInPr = true;📝 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.
| if (readFlag(args, ["--no-include-in-pr"])) input.includeInPr = false; | |
| if (readFlag(args, ["--include-in-pr"])) input.includeInPr = true; | |
| if (readFlag(args, ["--close-on-merge"])) input.closeOnMerge = true; | |
| const noIncludeInPr = readFlag(args, ["--no-include-in-pr"]); | |
| const includeInPr = readFlag(args, ["--include-in-pr"]); | |
| if (noIncludeInPr && includeInPr) { | |
| throw new CliUsageError( | |
| "Use either --include-in-pr or --no-include-in-pr, not both.", | |
| ); | |
| } | |
| if (noIncludeInPr) input.includeInPr = false; | |
| if (includeInPr) input.includeInPr = true; | |
| if (readFlag(args, ["--close-on-merge"])) input.closeOnMerge = true; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ade-cli/src/cli.ts` around lines 2588 - 2590, The CLI currently allows
both --include-in-pr and --no-include-in-pr to be passed and the later
assignment wins; update the parsing logic around readFlag(args,
["--no-include-in-pr"]), readFlag(args, ["--include-in-pr"]) and
input.includeInPr to detect when both flags are present and fail fast: if both
readFlag(...) calls return true, emit a clear error message and exit with a
non-zero status (or throw an Error) instead of silently overriding; otherwise
continue setting input.includeInPr as currently implemented.
| it("includes Cursor command files and subagents", () => { | ||
| const projectRoot = makeTmpRoot("ade-code-cursor-command-"); | ||
| const commandsDir = path.join(projectRoot, ".cursor", "commands"); | ||
| const agentsDir = path.join(projectRoot, ".cursor", "agents"); | ||
| fs.mkdirSync(commandsDir, { recursive: true }); | ||
| fs.mkdirSync(agentsDir, { recursive: true }); | ||
| fs.writeFileSync(path.join(commandsDir, "review-code.md"), "Review code.\n"); | ||
| fs.writeFileSync(path.join(agentsDir, "verifier.md"), [ | ||
| "---", | ||
| "description: Verify the change", | ||
| "---", | ||
| "", | ||
| "Verify.", | ||
| "", | ||
| ].join("\n")); | ||
|
|
||
| const commands = discoverProjectSlashCommands(projectRoot); | ||
| expect(commands).toEqual(expect.arrayContaining([ | ||
| expect.objectContaining({ name: "/review-code", description: "Review code." }), | ||
| expect.objectContaining({ name: "/verifier", description: "Verify the change" }), | ||
| ])); | ||
| }); |
There was a problem hiding this comment.
Add dual-path ADE CLI verification for these new API behaviors.
These tests validate payload wiring, but they don’t verify the required headless mode and desktop socket-backed ADE RPC paths. Please add one assertion path per mode for these added behaviors to prevent transport-specific regressions.
As per coding guidelines, "For ADE CLI changes, verify both headless mode and the desktop socket-backed ADE RPC path".
Also applies to: 260-279
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ade-cli/src/tuiClient/__tests__/adeApi.test.ts` around lines 236 - 257,
The test "includes Cursor command files and subagents" currently only asserts
discoverProjectSlashCommands(projectRoot) results; add two assertion paths that
invoke the CLI-layer verification for both headless and desktop RPC transports:
call the ADE CLI verification entry points used in tests (the headless-mode
verifier and the desktop socket-backed ADE RPC verifier — e.g., the functions or
helpers that simulate headless invocation and the desktop socket-backed RPC
client used elsewhere in tests) using the same temporary projectRoot created by
makeTmpRoot, then assert each returns/produces the expected commands (matching
objects with name "/review-code" and "/verifier" and the same descriptions).
Ensure you reference the same discoverProjectSlashCommands-derived expectations
so each transport path (headless and desktop socket-backed ADE RPC) has its own
expect(...).toEqual/arrayContaining checks to prevent transport-specific
regressions.
| local: { | ||
| cwd: init.laneRoot, | ||
| settingSources: ["project", "user"], | ||
| settingSources: ["all"], |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Cursor SDK TypeScript settingSources parameter documentation what does "all" include
💡 Result:
In Cursor SDK TypeScript, local.settingSources: ["all"] is a shorthand that includes all ambient on-disk settings layers for a local agent: project, user, team, mdm, and plugins. [1] Specifically: - "project" →.cursor/ in the workspace [1] - "user" → ~/.cursor/ [1] - "team" → team settings synced from the dashboard [1] - "mdm" → MDM-managed enterprise settings [1] - "plugins" → plugin-provided settings [1] - "all" → shorthand for all of the above [1] Note: local.settingSources only applies to local agents; cloud agents ignore this field and always load project/team/plugins instead. [1]
Citations:
Restrict settingSources to prevent cross-scope config leakage
In apps/desktop/src/main/services/chat/cursorSdkWorker.ts (line 343), changing settingSources from ["project","user"] to ["all"] expands local agent configuration layers to include not only workspace/project and user settings, but also team, MDM-managed enterprise settings, and plugin-provided settings (per Cursor SDK docs). This broadens the effective configuration beyond the lane-scoped project/user boundary and can undermine isolation and policy/hook enforcement. Revert to ["project","user"] or explicitly constrain/validate which layers are allowed for the ADE/Cursor agent and document the security rationale.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/main/services/chat/cursorSdkWorker.ts` at line 343, The
change broadens configuration layers by setting settingSources to ["all"];
revert it to only the intended lane-scoped layers by using settingSources:
["project","user"] in the Cursor SDK agent options (where the ADE/Cursor agent
is initialized) or, if broader sources must remain, add explicit validation that
filters/whitelists allowed layers and rejects team/enterprise/plugin overrides
before applying settings; update the agent initialization path that sets
settingSources to enforce this constraint and add a short comment documenting
the security rationale.
| }) { | ||
| const [copyState, setCopyState] = React.useState<CopyState>("idle"); | ||
| const project = issue.projectName?.trim() || issue.projectSlug; | ||
| const project = issue.projectName?.trim() || issue.projectSlug || issue.teamKey; |
There was a problem hiding this comment.
Trim projectSlug before fallback to avoid blank project display.
A whitespace-only projectSlug will currently win the fallback chain and render as empty text instead of teamKey.
Proposed fix
- const project = issue.projectName?.trim() || issue.projectSlug || issue.teamKey;
+ const project = issue.projectName?.trim() || issue.projectSlug?.trim() || issue.teamKey;📝 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.
| const project = issue.projectName?.trim() || issue.projectSlug || issue.teamKey; | |
| const project = issue.projectName?.trim() || issue.projectSlug?.trim() || issue.teamKey; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/components/lanes/LinearIssueBadge.tsx` at line 58,
The project selection currently uses issue.projectName?.trim() ||
issue.projectSlug || issue.teamKey, which lets a whitespace-only projectSlug
win; update the fallback to trim projectSlug as well by using
issue.projectName?.trim() || issue.projectSlug?.trim() || issue.teamKey so
blank/whitespace projectSlug falls through to teamKey (change the variable
assignment for project in LinearIssueBadge component).
arul28
force-pushed
the
ade/cursor-sdk-cleanup-17f7c5d8
branch
from
6bd85b0 to
27124a5
Compare
|
@copilot review but do not make fixes |
Reviewed only (no fixes applied) at commit 27124a5. Current status looks clean:
No UI changes in this review-only pass, so no screenshot applies. |
|
@copilot review but do not make fixes |
Reviewed only (no fixes applied) at commit b9b8b5b. Findings:
Focused validation I ran locally is green:
No UI changes in this review-only pass, so no screenshot applies. |
Extract Cursor-specific slash command discovery from Claude/Codex implementations into dedicated modules. Pull markdown parsing, project discovery, and prompt expansion into shared services. Extract Linear issue helpers from laneService and chat context attachment logic into standalone shared modules with tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Type SAMPLE_ISSUE as LaneLinearIssue to fix description type mismatch - Remove dead candidateNames.has(target) branch (target never has / prefix) - Restrict activateRuntime to cursor provider only (was unintentionally including droid) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Update cliLaunch.test.ts assertion: .cursor/skills is now first in ancestorSkillDirs ordering (matches agentSkillRoots.ts change) - Use Partial<LaneLinearIssue> instead of Partial<typeof SAMPLE_ISSUE> to allow string description overrides in test helper Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
arul28
force-pushed
the
ade/cursor-sdk-cleanup-17f7c5d8
branch
from
b9b8b5b to
110ce10
Compare
|
@copilot review but do not make fixes |
Reviewed only (no fixes applied) at commit 110ce10. Current findings:
Focused local validation is green:
No UI changes in this review-only pass, so no screenshot applies. |
Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Summary by CodeRabbit
Release Notes
New Features
link-linear-issuecommand)Improvements
Dependencies
Greptile Summary
This PR refactors slash command discovery into a shared
markdownSlashCommandDiscovery.tsmodule, adds Cursor IDE slash command and subagent discovery, relaxes theprojectId/projectSlugrequirement for Linear issue linking, and introduces a newlink-linear-issueCLI subcommand.markdownSlashCommandDiscovery.tsfile; a newcursorSlashCommandDiscovery.tsandslashCommandPromptExpansion.tsconsolidate per-provider expansion into a single dispatch function.projectIdandprojectSlugare no longer required fields inparseLaneLinearIssueValue, andLinearIssueBadgenow falls back toteamKeywhen both are absent; shared helpers (finalizeLaneLinearIssue,laneLinearIssueMissingFields,isLinkableLaneLinearIssue) are extracted intoshared/laneLinearIssue.ts.ade lanes link-linear-issueis added alongside a correspondinglinkLinearIssuesaction registered in the allowlist.Confidence Score: 5/5
Safe to merge; changes are well-structured refactors with tests and no correctness regressions on core paths.
The slash command discovery refactor is a clean extraction with equivalent behavior for Claude and Codex sessions. The Linear issue parsing relaxation (dropping projectId/projectSlug as required fields) is intentional and tested. The new linkLinearIssues CLI command follows existing patterns and is properly allowlisted. The one observation — Cursor built-in subagents surfacing in non-Cursor TUI autocomplete — is a UX polish item that does not corrupt data or break existing flows.
apps/desktop/src/main/services/chat/projectSlashCommandDiscovery.ts — the unified discovery aggregator includes Cursor built-in subagents that only work inside Cursor sessions.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[User types slash command] --> B{providerSlashCommand?} B -- No --> C[Build normal prompt] B -- Yes --> D[resolveProviderSlashCommandPrompt] D --> E{provider} E -- claude --> F{Built-in or runtime command?} F -- Yes --> G[return null - use raw text] F -- No --> H[resolveClaudeSlashCommandInvocation] E -- codex --> I{Built-in or runtime?} I -- Yes --> G I -- No --> J[resolveClaudeSlashCommandInvocation] J -- not found --> K[resolveCodexSlashCommandInvocation] E -- cursor --> L[resolveCursorSlashCommandInvocation] L -- not found built-in subagent --> G E -- other --> G H --> M[promptText] K --> M L --> M M -- null --> N[expandedSlashCommandPrompt ?? trimmed raw input] M -- text --> N subgraph Discovery [TUI autocomplete: discoverAllProjectSlashCommands] P[discoverClaudeSlashCommands] Q[discoverCodexSlashCommands] R[discoverCursorSlashCommands incl. built-in subagents] endPrompt To Fix All With AI
Reviews (4): Last reviewed commit: "ship: iteration 2 — fix test-desktop sha..." | Re-trigger Greptile