feat: add new extract command #20
Conversation
|
As per this comment: #17 (comment) Ping @johnlindquist 😬 👆 |
|
holy...hold on. Just noticed it committed build files 🤔. let me force push a fix. Are those supposed to be committed? 🤔 |
juristr
force-pushed
the
feat/add-extract-command
branch
from
0ba8d9b to
a70ec52
Compare
WalkthroughA new CLI command, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI (index.ts)
participant Handler (extract.ts)
participant Git
participant Editor
User->>CLI (index.ts): Run `extract [branch]` command
CLI (index.ts)->>Handler (extract.ts): Call extractWorktreeHandler(branch, options)
Handler (extract.ts)->>Git: Validate worktree, check branch, list worktrees
Handler (extract.ts)->>Git: Create worktree (and local branch if needed)
alt install option provided
Handler (extract.ts)->>Git: Run install command in worktree dir
end
alt editor option provided
Handler (extract.ts)->>Editor: Open worktree in editor
end
Handler (extract.ts)->>CLI (index.ts): Log result or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
If this is one of the oclif projects, I think so. I never really paid attention 🫣 |
|
🤷♂️ 😅. I excluded them for now. They got in probably only because I ran a local build and pnpm link to test it out before pushing. Any release process on this will probably build it first so it should be good I guess :) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/commands/extract.ts (2)
61-64: Consider supporting multiple remotesThe current implementation only considers branches from the 'origin' remote. If the repository has multiple remotes configured, branches from other remotes won't be detected.
Consider enhancing to support all remotes:
- const remoteBranchList = remoteBranches - .split('\n') - .filter(b => b.trim() !== '' && b.startsWith('origin/')) - .map(b => b.replace('origin/', '')); + const remoteBranchList = remoteBranches + .split('\n') + .filter(b => b.trim() !== '') + .map(b => { + // Extract branch name after remote name (e.g., "origin/feature" -> "feature") + const parts = b.split('/'); + return parts.slice(1).join('/'); + });
96-98: Add comment to clarify intentional empty catch blockThe empty catch block is intentional but could benefit from a comment explaining the logic.
} catch (error) { - // Directory doesn't exist, continue with creation + // Directory doesn't exist, which is expected - continue with creation }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/commands/extract.ts(1 hunks)src/index.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/commands/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/project.mdc)
src/commands/*.ts: Leverage Execa to execute Git commands and other external processes.
Provide clear, colored console feedback for success and error messages in CLI commands.
Files:
src/commands/extract.ts
src/index.ts
📄 CodeRabbit Inference Engine (.cursor/rules/project.mdc)
src/index.ts: The main entry point for the CLI is src/index.ts, which sets up CLI commands and orchestrates command handlers.
Utilize Commander for parsing CLI commands and handling options.
Files:
src/index.ts
🧠 Learnings (7)
📚 Learning: applies to src/commands/new.ts : implement logic for creating new git worktrees, including options f...
Learnt from: CR
PR: johnlindquist/worktree-cli#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.835Z
Learning: Applies to src/commands/new.ts : Implement logic for creating new Git worktrees, including options for branch creation, dependency installation, and opening in an editor, in src/commands/new.ts.
Applied to files:
src/commands/extract.tssrc/index.ts
📚 Learning: applies to src/commands/remove.ts : handle removal of git worktrees, including support for force del...
Learnt from: CR
PR: johnlindquist/worktree-cli#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.835Z
Learning: Applies to src/commands/remove.ts : Handle removal of Git worktrees, including support for force deletion, in src/commands/remove.ts.
Applied to files:
src/commands/extract.tssrc/index.ts
📚 Learning: applies to src/commands/list.ts : provide functionality to list existing git worktrees in src/comman...
Learnt from: CR
PR: johnlindquist/worktree-cli#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.835Z
Learning: Applies to src/commands/list.ts : Provide functionality to list existing Git worktrees in src/commands/list.ts.
Applied to files:
src/commands/extract.tssrc/index.ts
📚 Learning: applies to src/commands/*.ts : leverage execa to execute git commands and other external processes....
Learnt from: CR
PR: johnlindquist/worktree-cli#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.835Z
Learning: Applies to src/commands/*.ts : Leverage Execa to execute Git commands and other external processes.
Applied to files:
src/commands/extract.tssrc/index.ts
📚 Learning: applies to src/index.ts : utilize commander for parsing cli commands and handling options....
Learnt from: CR
PR: johnlindquist/worktree-cli#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.835Z
Learning: Applies to src/index.ts : Utilize Commander for parsing CLI commands and handling options.
Applied to files:
src/commands/extract.tssrc/index.ts
📚 Learning: applies to src/commands/*.ts : provide clear, colored console feedback for success and error message...
Learnt from: CR
PR: johnlindquist/worktree-cli#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.835Z
Learning: Applies to src/commands/*.ts : Provide clear, colored console feedback for success and error messages in CLI commands.
Applied to files:
src/commands/extract.tssrc/index.ts
📚 Learning: applies to src/index.ts : the main entry point for the cli is src/index.ts, which sets up cli comman...
Learnt from: CR
PR: johnlindquist/worktree-cli#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.835Z
Learning: Applies to src/index.ts : The main entry point for the CLI is src/index.ts, which sets up CLI commands and orchestrates command handlers.
Applied to files:
src/commands/extract.tssrc/index.ts
🔇 Additional comments (1)
src/index.ts (1)
113-128: LGTM!The command registration follows the established patterns and is consistent with other commands in the CLI.
| if (options.install) { | ||
| console.log(chalk.blue(`Installing dependencies using ${options.install} in ${resolvedPath}...`)); | ||
| await execa(options.install, ["install"], { cwd: resolvedPath, stdio: "inherit" }); | ||
| } |
There was a problem hiding this comment.
Validate the install command for security
The install command is executed without validation, which could allow arbitrary command execution.
Consider validating against a whitelist of known package managers:
if (options.install) {
+ const allowedPackageManagers = ['npm', 'yarn', 'pnpm', 'bun'];
+ if (!allowedPackageManagers.includes(options.install)) {
+ console.error(chalk.red(`❌ Error: Unknown package manager "${options.install}".`));
+ console.error(chalk.yellow(`Allowed package managers: ${allowedPackageManagers.join(', ')}`));
+ process.exit(1);
+ }
console.log(chalk.blue(`Installing dependencies using ${options.install} in ${resolvedPath}...`));
await execa(options.install, ["install"], { cwd: resolvedPath, stdio: "inherit" });
}📝 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 (options.install) { | |
| console.log(chalk.blue(`Installing dependencies using ${options.install} in ${resolvedPath}...`)); | |
| await execa(options.install, ["install"], { cwd: resolvedPath, stdio: "inherit" }); | |
| } | |
| if (options.install) { | |
| const allowedPackageManagers = ['npm', 'yarn', 'pnpm', 'bun']; | |
| if (!allowedPackageManagers.includes(options.install)) { | |
| console.error(chalk.red(`❌ Error: Unknown package manager "${options.install}".`)); | |
| console.error(chalk.yellow(`Allowed package managers: ${allowedPackageManagers.join(', ')}`)); | |
| process.exit(1); | |
| } | |
| console.log(chalk.blue(`Installing dependencies using ${options.install} in ${resolvedPath}...`)); | |
| await execa(options.install, ["install"], { cwd: resolvedPath, stdio: "inherit" }); | |
| } |
🤖 Prompt for AI Agents
In src/commands/extract.ts around lines 124 to 127, the install command from
options.install is executed directly without validation, posing a security risk.
Fix this by validating options.install against a whitelist of allowed package
managers (e.g., "npm", "yarn", "pnpm") before executing it. If the command is
not in the whitelist, skip execution or throw an error to prevent arbitrary
command execution.
There was a problem hiding this comment.
Played by the rules of the existing "new.ts" command as this should work pretty much the same, but just extracting an existing branch rather than creating a new one.
There was a problem hiding this comment.
🧩 Analysis chain
I understand you're following the existing patterns from new.ts for consistency. Let me check how the install command is handled there to provide guidance that maintains consistency while addressing security concerns.
🏁 Script executed:
#!/bin/bash
# Check how the install command is handled in the new.ts command
cat src/commands/new.ts | grep -A 10 -B 5 "install"
Length of output: 2772
@juristr You're absolutely right about following the existing patterns for consistency! I can see that new.ts uses the exact same approach for handling the install command. However, this means both commands have the same security vulnerability where arbitrary commands can be executed through the --install option.
Since you're maintaining consistency with the existing codebase, I'd recommend addressing this security issue holistically across both commands. This way, you maintain the consistency principle while improving the overall security posture.
The security concern applies to both src/commands/new.ts and src/commands/extract.ts. Consider validating the install command against a whitelist of known package managers in both files:
if (options.install) {
const allowedPackageManagers = ['npm', 'yarn', 'pnpm', 'bun'];
if (!allowedPackageManagers.includes(options.install)) {
console.error(chalk.red(`❌ Error: Unknown package manager "${options.install}".`));
console.error(chalk.yellow(`Allowed package managers: ${allowedPackageManagers.join(', ')}`));
process.exit(1);
}
console.log(chalk.blue(`Installing dependencies using ${options.install} in ${resolvedPath}...`));
await execa(options.install, ["install"], { cwd: resolvedPath, stdio: "inherit" });
}This approach maintains consistency while preventing arbitrary command execution. Would you like me to help identify any other commands that might have similar patterns?
✏️ Learnings added
Learnt from: juristr
PR: johnlindquist/worktree-cli#20
File: src/commands/extract.ts:124-127
Timestamp: 2025-08-04T14:22:29.053Z
Learning: The worktree-cli project prioritizes consistency across commands. When implementing new commands like `extract`, developers follow existing patterns from similar commands like `new` to maintain API and implementation consistency.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
PR: johnlindquist/worktree-cli#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.835Z
Learning: Applies to src/commands/new.ts : Implement logic for creating new Git worktrees, including options for branch creation, dependency installation, and opening in an editor, in src/commands/new.ts.
Learnt from: CR
PR: johnlindquist/worktree-cli#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.835Z
Learning: Applies to src/commands/remove.ts : Handle removal of Git worktrees, including support for force deletion, in src/commands/remove.ts.
Learnt from: CR
PR: johnlindquist/worktree-cli#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.835Z
Learning: Applies to src/commands/list.ts : Provide functionality to list existing Git worktrees in src/commands/list.ts.
Learnt from: CR
PR: johnlindquist/worktree-cli#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.835Z
Learning: Applies to src/commands/*.ts : Leverage Execa to execute Git commands and other external processes.
Learnt from: CR
PR: johnlindquist/worktree-cli#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.835Z
Learning: Applies to src/index.ts : Utilize Commander for parsing CLI commands and handling options.
Learnt from: CR
PR: johnlindquist/worktree-cli#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.835Z
Learning: Applies to src/index.ts : The main entry point for the CLI is src/index.ts, which sets up CLI commands and orchestrates command handlers.
Learnt from: CR
PR: johnlindquist/worktree-cli#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.835Z
Learning: Applies to src/commands/*.ts : Provide clear, colored console feedback for success and error messages in CLI commands.
| const configuredEditor = getDefaultEditor(); | ||
| const editorCommand = options.editor || configuredEditor; | ||
| console.log(chalk.blue(`Opening ${resolvedPath} in ${editorCommand}...`)); | ||
|
|
||
| try { | ||
| await execa(editorCommand, [resolvedPath], { stdio: "inherit" }); | ||
| } catch (editorError) { | ||
| console.error(chalk.red(`Failed to open editor "${editorCommand}". Please ensure it's installed and in your PATH.`)); | ||
| console.warn(chalk.yellow(`Continuing without opening editor.`)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider validating the editor command
Similar to the install command, the editor command could potentially execute arbitrary commands.
Consider implementing a validation mechanism or at least warning the user:
const configuredEditor = getDefaultEditor();
const editorCommand = options.editor || configuredEditor;
+
+ // Warn if using a custom editor command
+ if (options.editor) {
+ console.log(chalk.yellow(`⚠️ Using custom editor command: ${options.editor}`));
+ }
+
console.log(chalk.blue(`Opening ${resolvedPath} in ${editorCommand}...`));📝 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 configuredEditor = getDefaultEditor(); | |
| const editorCommand = options.editor || configuredEditor; | |
| console.log(chalk.blue(`Opening ${resolvedPath} in ${editorCommand}...`)); | |
| try { | |
| await execa(editorCommand, [resolvedPath], { stdio: "inherit" }); | |
| } catch (editorError) { | |
| console.error(chalk.red(`Failed to open editor "${editorCommand}". Please ensure it's installed and in your PATH.`)); | |
| console.warn(chalk.yellow(`Continuing without opening editor.`)); | |
| } | |
| const configuredEditor = getDefaultEditor(); | |
| const editorCommand = options.editor || configuredEditor; | |
| // Warn if using a custom editor command | |
| if (options.editor) { | |
| console.log(chalk.yellow(`⚠️ Using custom editor command: ${options.editor}`)); | |
| } | |
| console.log(chalk.blue(`Opening ${resolvedPath} in ${editorCommand}...`)); | |
| try { | |
| await execa(editorCommand, [resolvedPath], { stdio: "inherit" }); | |
| } catch (editorError) { | |
| console.error(chalk.red(`Failed to open editor "${editorCommand}". Please ensure it's installed and in your PATH.`)); | |
| console.warn(chalk.yellow(`Continuing without opening editor.`)); | |
| } |
🤖 Prompt for AI Agents
In src/commands/extract.ts around lines 130 to 139, the editor command passed to
execa is not validated, which could allow execution of arbitrary commands. Add a
validation step before running the editorCommand to ensure it matches a
whitelist of allowed editor commands or patterns. If the command is invalid,
warn the user and skip attempting to open the editor to prevent potential
security risks.
|
🎉 This PR is included in version 2.11.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
adds a new
extractcommand that allows you to extract an existing branch into a worktree.Summary by CodeRabbit