Conversation
- Remove restrictive rootDir setting that prevented TypeScript from following @elizaos/core imports - Add skipLibCheck to improve build performance and avoid external dependency type checking issues Co-authored-by: Cursor <cursoragent@cursor.com>
- Add create/switch support for GitHub PR and GitLab MR URLs - Add src/utils: prUrlParser, messageParser, nicknameGenerator - Workspace service: findWorkspacesBySourceUrl, generateUniqueName - Add .gitignore for dist, node_modules, .turbo, env, coverage Made-with: Cursor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (11)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR improves workspace creation from Pull Request / Merge Request URLs on GitHub and GitLab, and adds natural language nickname generation for workspaces using LLMs to enable more intuitive workspace inference from user messages.
Changes:
- New utility files:
prUrlParser.ts(parses GitHub PR / GitLab MR URLs),nicknameGenerator.ts(LLM-powered workspace nicknames),messageParser.ts(unified workspace inference from messages), andutils/index.tsbarrel export WorkspaceMetadatatype extended withnicknames?: string[];WorkspaceServicegainsupdateWorkspaceMetadata;SWITCH_WORKSPACEandCREATE_WORKSPACEactions updated to use the new utilities and generate/migrate nicknamestsconfig.jsonupdated: removesrootDir, addsskipLibCheck: true
Reviewed changes
Copilot reviewed 8 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
tsconfig.json |
Removes rootDir (still inferred from include), adds skipLibCheck: true to suppress type errors in .d.ts files |
src/utils/prUrlParser.ts |
New parser for GitHub PR and GitLab MR URLs, including workspace name generation |
src/utils/nicknameGenerator.ts |
LLM-based nickname generation with rule-based fallback |
src/utils/messageParser.ts |
Unified workspace inference (explicit commands, implicit mention, URL extraction) |
src/utils/index.ts |
Barrel export for all new utilities |
src/types/index.ts |
Adds nicknames field to WorkspaceMetadata |
src/services/workspace.service.ts |
Adds updateWorkspaceMetadata and debug logging for execCommand |
src/providers/index.ts |
Minor: fixes missing line number on workspaceSettingsProvider export |
src/actions/switch.ts |
Uses unified inferWorkspace; background nickname migration on switch |
src/actions/create.ts |
PR/MR URL detection and fetchPRBranch; nickname generation on create |
.gitignore |
New file added |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Strategy 3: Single workspace auto-selection (60% confidence) | ||
| const workspaces = workspaceService.listWorkspaces(); | ||
| if (workspaces.length === 1) { | ||
| return { | ||
| name: workspaces[0].name, | ||
| method: 'auto-single', | ||
| confidence: 60, | ||
| context: 'Only one workspace available', | ||
| }; | ||
| } | ||
|
|
||
| // Strategy 4: Extract from URL if present | ||
| const url = extractRepositoryUrl(text); | ||
| if (url) { | ||
| const derivedName = deriveWorkspaceNameFromUrl(url); | ||
| return { | ||
| name: derivedName, | ||
| method: 'url-extracted', | ||
| confidence: 80, | ||
| context: `Derived from URL: ${url}`, | ||
| }; | ||
| } |
There was a problem hiding this comment.
The inference strategies in inferWorkspace are executed out of confidence order: Strategy 3 (auto-single, 60% confidence) is evaluated before Strategy 4 (url-extracted, 80% confidence). The docstring comment says they're ordered by confidence, but the implementation contradicts this.
As a result, if the user sends a message with a URL and there happens to be only one workspace available, the system will select the single workspace (60% confidence) and ignore the URL-derived workspace name (80% confidence), which is a less accurate outcome. Strategy 4 should be checked before Strategy 3.
| * import { extractWorkspaceName, inferWorkspace } from './utils/messageParser'; | ||
| * | ||
| * // In an action handler | ||
| * const name = extractWorkspaceName(message.content.text); |
There was a problem hiding this comment.
The code example in the module-level docstring references extractWorkspaceName (line 17, 20), but this function does not exist in this file. The actual exported function is extractExplicitWorkspaceName. The example import and usage should be updated to use the correct function name.
| * import { extractWorkspaceName, inferWorkspace } from './utils/messageParser'; | |
| * | |
| * // In an action handler | |
| * const name = extractWorkspaceName(message.content.text); | |
| * import { extractExplicitWorkspaceName, inferWorkspace } from './utils/messageParser'; | |
| * | |
| * // In an action handler | |
| * const name = extractExplicitWorkspaceName(message.content.text); |
| if (pr) { | ||
| return { | ||
| branch: pr.head?.ref || null, | ||
| title: pr.title, | ||
| }; |
There was a problem hiding this comment.
The return type of fetchPRBranch is declared as Promise<{ branch: string; title?: string } | null>, but at line 69, branch is assigned pr.head?.ref || null. If pr.head is undefined, pr.head?.ref evaluates to undefined, and undefined || null evaluates to null, which does not match the string type declared for branch. This would cause a TypeScript type error and could result in a { branch: null } object being returned instead of null, which the caller at line 199 (if (prDetails?.branch)) does handle since null is falsy — but it's still a type violation. The return should guard against this case, e.g., by returning null if pr.head?.ref is falsy.
| // Check if this is a PR/MR URL | ||
| prInfo = parsePRUrl(sourceUrl); |
There was a problem hiding this comment.
The PR URL detection in create.ts is broken because extractRepositoryUrl is called on the raw message text first, and its HTTPS regex strips the /pull/123 portion before parsePRUrl is called.
For a URL like https://github.com/owner/repo/pull/123, the regex /https?:\/\/[^\s]+?\/([a-zA-Z0-9_-]+\/[a-zA-Z0-9_-]+)(?:\.git)?/i with a lazy quantifier will match and return https://github.com/owner/repo (stopping at the first valid owner/repo pattern). The truncated URL is then assigned to sourceUrl (line 186), and when parsePRUrl(sourceUrl) is called at line 189, it receives the repo URL without /pull/123 and returns null — so the PR detection logic is never triggered.
To fix this, parsePRUrl should be called on the original text (or the full, raw URL extracted without truncation) before the general-purpose extractRepositoryUrl. For example, you could call parsePRUrl(text) directly, or restructure the URL extraction so PR URLs are detected first as a distinct case in create.ts.
| // Check if this is a PR/MR URL | |
| prInfo = parsePRUrl(sourceUrl); | |
| // Check if this is a PR/MR URL using the full message text | |
| prInfo = parsePRUrl(lowerText); |
| let matched = false; | ||
|
|
||
| // Check workspace name | ||
| const escapedName = ws.name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | ||
| const nameRegex = new RegExp(`\\b${escapedName}\\b`, 'i'); | ||
|
|
||
| if (nameRegex.test(text)) { | ||
| mentioned.push(ws.name); | ||
| matched = true; | ||
| continue; // Move to next workspace | ||
| } | ||
|
|
||
| // Check nicknames (if available) | ||
| if (ws.metadata.nicknames && ws.metadata.nicknames.length > 0) { | ||
| for (const nickname of ws.metadata.nicknames) { | ||
| const escapedNickname = nickname.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | ||
| const nicknameRegex = new RegExp(`\\b${escapedNickname}\\b`, 'i'); | ||
|
|
||
| if (nicknameRegex.test(lowerText)) { | ||
| mentioned.push(ws.name); | ||
| matched = true; | ||
| break; // Found a match, no need to check other nicknames | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The matched variable in findMentionedWorkspaces is declared and assigned but never read after the loop body. It serves no functional purpose and can be removed. The continue and break statements in the loop already handle the control flow correctly without needing this variable.
No description provided.