-
Notifications
You must be signed in to change notification settings - Fork 13
feat: Implement 10 major improvements to worktree-cli #31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This commit implements comprehensive improvements to the worktree-cli: 1. Fix Global Path Collisions (#1) - Added repo namespace to global paths to prevent cross-repo collisions - ~/worktrees/auth now becomes ~/worktrees/repo-name/auth 2. Enable Bare Repository Support (#2) - Removed blocking check for bare repos - Skip clean check for bare repos (no working tree) 3. Refactor PR Command to Avoid Context Switching (#3) - Use `git fetch origin refs/pull/ID/head:branch` instead of checkout - No longer requires switching branches in main worktree 4. Interactive TUI for Missing Arguments (#4) - Added prompts library for interactive selection - `wt open`, `wt remove`, `wt pr` now show selection UI without args - `wt purge` uses multi-select for batch removal 5. Handle Dirty States Gracefully (#5) - Offer stash/pop workflow when worktree is dirty - Options: stash, abort, or continue anyway - Automatic stash restore in finally block 6. Replace Regex Security with Trust Model (#6) - Removed brittle regex blocklist for setup commands - Added --trust flag for CI environments - Shows commands and asks for confirmation before execution 7. Centralize Path and Naming Logic (#7) - Created src/utils/paths.ts with resolveWorktreeName() - Standardized on replacing / with - for branch names - Prevents collisions: feature/auth and hotfix/auth are now unique 8. Robust Git Output Parsing (#8) - Created typed WorktreeInfo interface - getWorktrees() parses --porcelain output correctly - Handles locked, prunable, detached, and bare states 9. Atomic Operations and Rollback (#9) - Created AtomicWorktreeOperation class - Automatic rollback on failure (removes worktree, cleans up) - withAtomicRollback() helper for clean error handling 10. Automated Integration Tests (#10) - Created test suite with vitest - 26 tests covering all major functionality - Tests create real git repos and run CLI commands
|
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. WalkthroughThis PR adds atomic, stash-aware worktree operations, interactive TUI flows, branch/path validation, and bare-repo support; introduces new utilities (atomic, git, paths, tui, setup), expands CLI options (--trust), and adds comprehensive unit/integration tests. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI
participant TUI
participant Git
participant Atomic
participant Editor
User->>CLI: invoke command (new/pr/extract)
CLI->>Git: getWorktrees(), isBare?, validateBranchName()
alt main worktree dirty (non-bare)
CLI->>TUI: handleDirtyState()
TUI->>User: [stash / abort / continue]
alt stash
CLI->>Git: stashChanges()
end
end
CLI->>CLI: resolveWorktreePath()
CLI->>Atomic: new AtomicWorktreeOperation()
alt remote-tracking branch
Atomic->>Git: createWorktreeFromRemote(path, branch, remoteRef)
else local branch or createBranch
Atomic->>Git: createWorktree(path, branch, createBranch)
end
Atomic->>Atomic: register rollback actions
alt creation success
Atomic->>Atomic: commit()
Atomic->>Git: runInstall() (optional)
CLI->>Editor: open editor (optional)
else creation failure
Atomic->>Atomic: rollback()
Atomic->>Git: cleanup (git remove, delete path)
end
alt stash was created
CLI->>Git: popStash()
end
CLI-->>User: final success/error message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async function fetchPRBranch(prNumber: string, localBranchName: string, provider: GitProvider): Promise<void> { | ||
| if (provider === 'gh') { | ||
| // Fetch the PR head ref directly into a local branch | ||
| // This doesn't require checking out or changing the current branch | ||
| await execa("git", [ | ||
| "fetch", "origin", | ||
| `refs/pull/${prNumber}/head:${localBranchName}`, | ||
| ]); | ||
| } else { | ||
| // For GitLab, fetch the MR source branch | ||
| // First get the source branch name from the MR | ||
| const branchName = await getBranchNameFromPR(prNumber, provider); | ||
| await execa("git", [ | ||
| "fetch", "origin", | ||
| `${branchName}:${localBranchName}`, | ||
| ]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitLab MR fetch misses forked merge requests
For GitLab the new fetchPRBranch implementation always runs git fetch origin ${branchName}:${localBranchName} using the MR’s source branch (lines 80‑86). That works only when the source branch lives on the origin remote; forked MRs expose refs under refs/merge-requests/<iid>/head instead and are not present on origin, so this fetch will fail and the handler aborts before creating a worktree. The previous glab mr checkout handled forked MRs. Please fetch the MR ref (refs/merge-requests/<iid>/head) or delegate to glab so forked GitLab MRs continue to work.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
build/commands/new.js (1)
143-161: Stashed changes won't be restored on failure due toprocess.exit().In Node.js,
process.exit()terminates synchronously and does not executefinallyblocks. If the atomic operation fails (caught at line 143),process.exit(1)at line 150 will prevent the stash restoration in thefinallyblock from running.Move the stash restoration before calling
process.exit():catch (error) { if (error instanceof Error) { console.error(chalk.red("Failed to create new worktree:"), error.message); } else { console.error(chalk.red("Failed to create new worktree:"), error); } + // Restore stashed changes before exiting + if (stashed) { + console.log(chalk.blue("Restoring your stashed changes...")); + const restored = await popStash("."); + if (restored) { + console.log(chalk.green("Changes restored successfully.")); + } + } process.exit(1); } - finally { - // Restore stashed changes if we stashed them - if (stashed) { - console.log(chalk.blue("Restoring your stashed changes...")); - const restored = await popStash("."); - if (restored) { - console.log(chalk.green("Changes restored successfully.")); - } - } - }Alternatively, avoid
process.exit()entirely and let the error propagate to the CLI framework.src/commands/extract.ts (1)
172-188: Stashed changes won't be restored on failure due toprocess.exit().Same issue as in
build/commands/new.js:process.exit(1)at line 178 terminates synchronously and bypasses thefinallyblock, leaving stashed changes unrestored.Apply the same fix as recommended for
new.js- move stash restoration beforeprocess.exit():} catch (error) { if (error instanceof Error) { console.error(chalk.red("Failed to extract worktree:"), error.message); } else { console.error(chalk.red("Failed to extract worktree:"), error); } + // Restore stashed changes before exiting + if (stashed) { + console.log(chalk.blue("Restoring your stashed changes...")); + const restored = await popStash("."); + if (restored) { + console.log(chalk.green("Changes restored successfully.")); + } + } process.exit(1); - } finally { - // Restore stashed changes if we stashed them - if (stashed) { - console.log(chalk.blue("Restoring your stashed changes...")); - const restored = await popStash("."); - if (restored) { - console.log(chalk.green("Changes restored successfully.")); - } - } } }
♻️ Duplicate comments (1)
test/integration.test.js (1)
1-283: Same concerns as TypeScript version apply.This JavaScript file mirrors
test/integration.test.ts. The same issues apply:
- Atomic Operations test has commented-out rollback assertion (line 224-225)
- Bare Repository Support test uses git directly instead of CLI (lines 193-206)
No additional JS-specific issues found.
🧹 Nitpick comments (27)
test/utils.test.ts (1)
86-130: Consider using static imports for consistency.The
AtomicWorktreeOperationis dynamically imported while path utilities use static imports. For consistency and slightly faster test execution, consider static import:+import { AtomicWorktreeOperation } from '../src/utils/atomic.js'; + describe('Atomic Operations', () => { it('should track rollback actions in order', async () => { - const { AtomicWorktreeOperation } = await import('../src/utils/atomic.js'); - const atomic = new AtomicWorktreeOperation();Otherwise, the test logic correctly validates LIFO rollback behavior and commit semantics.
test/utils.test.js (1)
23-37: Consider adding a test for trailing slash edge case.The
getShortBranchNametests cover common cases, but a trailing slash (e.g.,'feature/') could be worth testing to ensure consistent behavior.it('should handle empty segments', () => { expect(getShortBranchName('feature//auth')).toBe('auth'); expect(getShortBranchName('/leading-slash')).toBe('leading-slash'); }); + it('should handle trailing slash', () => { + expect(getShortBranchName('feature/')).toBe('feature'); + }); });src/commands/remove.ts (2)
77-85: TTY check may not behave as expected in all environments.
process.stdin.isTTYreturnsundefined(notfalse) in non-TTY environments, so!process.stdin.isTTYwould betrueforundefined. This works correctly, but consider making the intent clearer.- const isNonInteractive = !process.stdin.isTTY; + const isNonInteractive = process.stdin.isTTY !== true;
109-116: Physical directory cleanup is redundant but harmless.
git worktree removeautomatically deletes the worktree directory when it's clean (or with--forceflag for dirty worktrees). The manualrm()call here is a defensive fallback that will rarely execute, since the directory should already be deleted by git. While the try-catch prevents errors if the directory is missing, consider removing this block to keep the logic straightforward and rely on git's own cleanup, or add a comment explaining why this fallback exists (e.g., handling edge cases where git removal succeeds but directory remains).build/utils/setup.js (1)
64-77: Consider whether failing fast would be more appropriate for dependent commands.The current behavior continues executing remaining commands after a failure. If setup commands have dependencies on each other (e.g.,
npm installbeforenpm run build), continuing after a failure may produce confusing cascading errors.Consider adding an option to fail fast or at least summarizing all failures at the end:
+ let hasFailures = false; for (const command of commands) { console.log(chalk.gray(`Executing: ${command}`)); try { await execa(command, { shell: true, cwd: worktreePath, env, stdio: "inherit" }); } catch (cmdError) { if (cmdError instanceof Error) { console.error(chalk.red(`Setup command failed: ${command}`), cmdError.message); } else { console.error(chalk.red(`Setup command failed: ${command}`), cmdError); } + hasFailures = true; // Continue with other commands } } - console.log(chalk.green("Setup commands completed.")); - return true; + if (hasFailures) { + console.log(chalk.yellow("Setup completed with some failures.")); + } else { + console.log(chalk.green("Setup commands completed.")); + } + return !hasFailures;src/utils/setup.ts (1)
72-86: Same concern as JS version: consider reporting partial failures.The TypeScript version has the same behavior of continuing after command failures. Consider aligning the return value with actual success state.
Same recommendation as
build/utils/setup.js—track and report failures to give the caller accurate success information.src/commands/purge.ts (1)
40-45: Minor redundancy:excludeMainis unnecessary here.The
purgeWorktreesarray is already filtered to exclude the main worktree (line 21), butselectWorktreeis called withexcludeMain: true. This results ingetWorktrees()being called again and filtered redundantly. Consider passing the pre-filtered list directly or removing the redundant flag.build/commands/pr.js (2)
170-172: Consider usingfindWorktreeByPathfor consistency.Other commands use
findWorktreeByPathfor this pattern. Here,getWorktrees()is called and then filtered manually. Using the existing utility would be more consistent.- const worktrees = await getWorktrees(); - const existingWorktree = worktrees.find(wt => wt.path === resolvedPath); + const existingWorktree = await findWorktreeByPath(resolvedPath);Note: Ensure
findWorktreeByPathis imported from../utils/git.js.
237-246: Consider handlingpopStashfailure.If
popStashreturns false or throws, the user's stashed changes might remain in the stash without clear indication. Consider logging a warning if restoration fails, so users know to manually recover their changes.if (stashed) { console.log(chalk.blue("Restoring your stashed changes...")); const restored = await popStash("."); if (restored) { console.log(chalk.green("Changes restored successfully.")); + } else { + console.warn(chalk.yellow("Could not restore stashed changes. Use 'git stash pop' manually.")); } }src/commands/open.ts (2)
6-7: Unused import:getWorktreesis not used.The
getWorktreesfunction is imported but not used in this file. OnlyfindWorktreeByBranchandfindWorktreeByPathare actually called.-import { getWorktrees, findWorktreeByBranch, findWorktreeByPath, WorktreeInfo } from "../utils/git.js"; +import { findWorktreeByBranch, findWorktreeByPath, WorktreeInfo } from "../utils/git.js";
86-93: Edge case: Emptyheadstring for minimal worktrees.For manually constructed
WorktreeInfoobjects (lines 44-53),headis an empty string. Ifdetachedwere somehow true,head.substring(0, 7)would display an empty string. While the current code setsdetached: falsefor these cases, consider adding a guard for robustness.} else if (targetWorktree.detached) { - console.log(chalk.blue(`Opening detached worktree at ${targetWorktree.head.substring(0, 7)}...`)); + const shortHead = targetWorktree.head ? targetWorktree.head.substring(0, 7) : 'unknown'; + console.log(chalk.blue(`Opening detached worktree at ${shortHead}...`)); } else {src/commands/setup.ts (1)
26-70: Consider extracting duplicated JSON parsing logic.The JSON parsing logic (lines 29-46 and 50-67) is duplicated for both config file locations. Consider extracting this into a helper function to reduce duplication and improve maintainability.
+async function tryLoadSetupFile(filePath: string): Promise<string[] | null> { + try { + await stat(filePath); + const content = await readFile(filePath, "utf-8"); + const data = JSON.parse(content) as WorktreeSetupData | string[]; + + let commands: string[] = []; + if (Array.isArray(data)) { + commands = data; + } else if (data && typeof data === 'object' && Array.isArray(data["setup-worktree"])) { + commands = data["setup-worktree"]; + } + + return commands.length > 0 ? commands : null; + } catch { + return null; + } +} + async function loadSetupCommands(repoRoot: string): Promise<{ commands: string[]; filePath: string } | null> { - // Check for .cursor/worktrees.json first const cursorSetupPath = join(repoRoot, ".cursor", "worktrees.json"); - try { - await stat(cursorSetupPath); - const content = await readFile(cursorSetupPath, "utf-8"); - // ... duplicated logic - } catch { - // Not found, try fallback + const cursorCommands = await tryLoadSetupFile(cursorSetupPath); + if (cursorCommands) { + return { commands: cursorCommands, filePath: cursorSetupPath }; } - // Check for worktrees.json const fallbackSetupPath = join(repoRoot, "worktrees.json"); - // ... duplicated logic + const fallbackCommands = await tryLoadSetupFile(fallbackSetupPath); + if (fallbackCommands) { + return { commands: fallbackCommands, filePath: fallbackSetupPath }; + } return null; }build/utils/atomic.js (1)
116-123: Setup commands lack individual rollback granularity.If a setup command fails midway through execution, there's no way to roll back previously executed commands. The comment acknowledges this ("worktree removal handles cleanup"), which is acceptable since the entire worktree is removed on rollback. However, for long-running setup scripts, consider adding a warning that partial execution may occur.
Consider adding a note in the documentation or a log message indicating that setup commands may partially execute before failure:
async runSetupCommands(commands, cwd, env) { - console.log(chalk.blue("Running setup commands...")); + console.log(chalk.blue(`Running ${commands.length} setup command(s)...`)); for (const command of commands) { console.log(chalk.gray(`Executing: ${command}`)); await execa(command, { shell: true, cwd, env, stdio: "inherit" }); } // No specific rollback - worktree removal handles cleanup }build/utils/git.js (1)
341-350: Consider warning about potential stash conflicts.
popStashmay fail if there are merge conflicts. The error is logged, but callers relying on the boolean return may not adequately inform users how to recover their stashed changes (e.g.,git stash list,git stash show).export async function popStash(cwd = ".") { try { await execa("git", ["-C", cwd, "stash", "pop"]); return true; } catch (error) { console.error(chalk.red("Failed to pop stash:"), error.stderr || error.message); + console.error(chalk.yellow("Tip: Your changes are still saved. Run 'git stash list' to see them.")); return false; } }src/utils/paths.ts (1)
103-125: Refactor branch validation to align with git-check-ref-format rules.The validation misses several actual git branch restrictions:
- Cannot start with
-(when using--branchflag)- Cannot contain consecutive slashes or end with
/- Cannot have path components starting with
.or ending with.lock- Should reject 40-character hex strings that look like commit SHAs
Note: The review's claims about
/,@{, and single@are not actual git restrictions; the function correctly allows/for hierarchical branch names.Refactoring to use
git check-ref-format --branchor implementing the complete ruleset would strengthen validation.build/commands/new.js (1)
16-27: Minor redundancy in branch name validation.The empty check at lines 16-21 duplicates logic already present in
validateBranchName()(which checks for empty/whitespace-only names). Consider removing the redundant check to keep the code DRY.- // Validate branch name is provided - if (!branchName || branchName.trim() === "") { - console.error(chalk.red("Error: Branch name is required.")); - console.error(chalk.yellow("Usage: wt new <branchName> [options]")); - console.error(chalk.cyan("Example: wt new feature/my-feature --checkout")); - process.exit(1); - } // Validate branch name format const validation = validateBranchName(branchName); if (!validation.isValid) { - console.error(chalk.red(`Error: ${validation.error}`)); + console.error(chalk.red(`Error: ${validation.error}`)); + if (validation.error === 'Branch name cannot be empty') { + console.error(chalk.yellow("Usage: wt new <branchName> [options]")); + console.error(chalk.cyan("Example: wt new feature/my-feature --checkout")); + } process.exit(1); }build/utils/paths.js (1)
78-96: Consider extending branch name validation for completeness.The current validation covers common cases, but git has additional restrictions (e.g., no leading/trailing dots, no
@{, no control characters). Since git will reject invalid names anyway, this is defense-in-depth, but extending validation could provide better error messages.export function validateBranchName(branchName) { if (!branchName || branchName.trim() === '') { return { isValid: false, error: 'Branch name cannot be empty' }; } // Check for invalid git branch name characters const invalidChars = /[\s~^:?*\[\]\\]/; if (invalidChars.test(branchName)) { return { isValid: false, error: 'Branch name contains invalid characters' }; } // Check for double dots if (branchName.includes('..')) { return { isValid: false, error: 'Branch name cannot contain ".."' }; } // Check for ending with .lock if (branchName.endsWith('.lock')) { return { isValid: false, error: 'Branch name cannot end with ".lock"' }; } + // Check for leading/trailing dots or slashes + if (/^[./]|[./]$/.test(branchName)) { + return { isValid: false, error: 'Branch name cannot start or end with "." or "/"' }; + } + // Check for @{ sequence (reflog syntax) + if (branchName.includes('@{')) { + return { isValid: false, error: 'Branch name cannot contain "@{"' }; + } return { isValid: true }; }test/integration.test.ts (2)
265-282: Test doesn't exercise CLI bare repo support.This test uses
execa('git', ['worktree', 'add', ...])directly instead of the CLI command. To validate Improvement #2 (bare repository support in the CLI), the test should userunCli(['new', ...], bareRepoDir)instead.it('should work with bare repositories', async () => { const worktreePath = join(ctx.testDir, 'bare-worktree'); - // Create worktree from bare repo - const result = await execa('git', ['worktree', 'add', worktreePath, 'main'], { - cwd: bareRepoDir, - reject: false, - }); + // Create worktree from bare repo using CLI + const result = await runCli( + ['new', 'main', '--path', worktreePath, '--editor', 'none'], + bareRepoDir + ); expect(result.exitCode).toBe(0);
326-340: LGTM with minor suggestion.The test covers the implemented validation rules. Consider using
it.eachfor better failure messages that identify which specific branch name failed:- it('should reject invalid branch names', async () => { - const invalidNames = [ - 'branch with spaces', - 'branch..double-dot', - 'branch.lock', - ]; - - for (const name of invalidNames) { - const result = await runCli( - ['new', name, '--editor', 'none'], - ctx.repoDir - ); - expect(result.exitCode).toBe(1); - } - }); + it.each([ + ['branch with spaces', 'contains spaces'], + ['branch..double-dot', 'contains double dots'], + ['branch.lock', 'ends with .lock'], + ])('should reject invalid branch name: %s', async (name) => { + const result = await runCli( + ['new', name, '--editor', 'none'], + ctx.repoDir + ); + expect(result.exitCode).toBe(1); + });build/utils/tui.js (1)
229-232: Consider defensive error message extraction.The error caught at line 229 may not always be an
Errorinstance with a.messageproperty, especially for non-standard errors fromexeca.catch (error) { - console.error(chalk.red(`Failed to fetch ${isPR ? 'PRs' : 'MRs'}:`), error.message); + const errMsg = error instanceof Error ? error.message : String(error); + console.error(chalk.red(`Failed to fetch ${isPR ? 'PRs' : 'MRs'}:`), errMsg); return null; }src/commands/new.ts (1)
172-181: Stash restoration in finally block ensures changes aren't lost.The
finallyblock correctly restores stashed changes regardless of operation outcome. Consider adding a hint if restoration fails (e.g., "Rungit stash listto see your changes"), sincepopStashfailures due to conflicts may leave users unsure where their changes went.finally { // Restore stashed changes if we stashed them if (stashed) { console.log(chalk.blue("Restoring your stashed changes...")); const restored = await popStash("."); if (restored) { console.log(chalk.green("Changes restored successfully.")); + } else { + console.log(chalk.yellow("Could not automatically restore changes. Run 'git stash list' to find your stash.")); } } }build/commands/setup.js (1)
14-56: Setup file loading with flexible schema support.The function correctly handles both config locations and JSON formats. However,
JSON.parseerrors for malformed JSON are caught by the outer catch, which logs nothing and falls through. Consider logging a warning when JSON parsing fails.try { await stat(cursorSetupPath); const content = await readFile(cursorSetupPath, "utf-8"); - const data = JSON.parse(content); + let data; + try { + data = JSON.parse(content); + } catch (parseErr) { + console.warn(chalk.yellow(`Warning: Failed to parse ${cursorSetupPath}: ${parseErr.message}`)); + // Fall through to try fallback location + } + if (data) { let commands = []; // ... rest of logic + }src/commands/pr.ts (2)
79-87: Minor inefficiency: redundant API call for GitLab MRs.For GitLab,
fetchPRBranchcallsgetBranchNameFromPRagain (line 82) even thoughprWorktreeHandleralready retrievedprBranchNameat line 152. Consider passing the already-fetched branch name to avoid a duplicate API call.-async function fetchPRBranch(prNumber: string, localBranchName: string, provider: GitProvider): Promise<void> { +async function fetchPRBranch(prNumber: string, localBranchName: string, provider: GitProvider, remoteBranchName?: string): Promise<void> { if (provider === 'gh') { // Fetch the PR head ref directly into a local branch // This doesn't require checking out or changing the current branch await execa("git", [ "fetch", "origin", `refs/pull/${prNumber}/head:${localBranchName}`, ]); } else { // For GitLab, fetch the MR source branch - // First get the source branch name from the MR - const branchName = await getBranchNameFromPR(prNumber, provider); + const branchName = remoteBranchName ?? await getBranchNameFromPR(prNumber, provider); await execa("git", [ "fetch", "origin", `${branchName}:${localBranchName}`, ]); } }Then update the call at line 159:
await fetchPRBranch(prNumber, prBranchName, provider, prBranchName);
262-270: Consider logging when stash restoration fails.If
popStashreturnsfalse(line 267), no message is logged. This could leave users unaware that their stashed changes weren't restored. Consider adding an else branch with a warning.if (restored) { console.log(chalk.green("Changes restored successfully.")); + } else { + console.log(chalk.yellow("Could not restore stashed changes. Run 'git stash list' to check.")); }build/commands/extract.js (2)
126-133: Inconsistent editor launch behavior compared topr.ts.This file uses
stdio: "inherit"withoutdetached: true, causing the CLI to block until the editor closes. In contrast,pr.tsusesstdio: "ignore", detached: truefor non-blocking behavior. Based on learnings, the project prioritizes consistency across commands.Consider aligning with
pr.ts:- await execa(editorCommand, [resolvedPath], { stdio: "inherit" }); + await execa(editorCommand, [resolvedPath], { stdio: "ignore", detached: true });
149-158: Same stash restore gap aspr.ts.Like
pr.ts, this lacks logging whenpopStashreturnsfalse. Consider adding consistent warning messaging across both files.src/utils/tui.ts (1)
213-248: Consider static import forexecato maintain consistency.Line 214 uses a dynamic import for
execa, but this module already imports from./git.jswhich statically importsexeca. The dynamic import adds slight overhead and inconsistency without clear benefit here.import prompts from "prompts"; import chalk from "chalk"; +import { execa } from "execa"; import { getWorktrees, WorktreeInfo } from "./git.js";Then at line 214:
- const { execa } = await import('execa');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
build/commands/extract.js(4 hunks)build/commands/list.js(1 hunks)build/commands/new.js(4 hunks)build/commands/open.js(1 hunks)build/commands/pr.js(5 hunks)build/commands/purge.js(1 hunks)build/commands/remove.js(1 hunks)build/commands/setup.js(3 hunks)build/index.js(1 hunks)build/utils/atomic.js(1 hunks)build/utils/git.js(1 hunks)build/utils/paths.js(1 hunks)build/utils/setup.js(3 hunks)build/utils/tui.js(1 hunks)package.json(2 hunks)src/commands/extract.ts(3 hunks)src/commands/list.ts(2 hunks)src/commands/new.ts(4 hunks)src/commands/open.ts(1 hunks)src/commands/pr.ts(4 hunks)src/commands/purge.ts(1 hunks)src/commands/remove.ts(3 hunks)src/commands/setup.ts(3 hunks)src/index.ts(1 hunks)src/utils/atomic.ts(1 hunks)src/utils/git.ts(1 hunks)src/utils/paths.ts(1 hunks)src/utils/setup.ts(3 hunks)src/utils/tui.ts(1 hunks)test/integration.test.js(1 hunks)test/integration.test.ts(1 hunks)test/utils.test.js(1 hunks)test/utils.test.ts(1 hunks)tsconfig.json(1 hunks)vitest.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
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
src/commands/remove.ts
📄 CodeRabbit inference engine (.cursor/rules/project.mdc)
Handle removal of Git worktrees, including support for force deletion, in src/commands/remove.ts.
Files:
src/commands/remove.ts
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/remove.tssrc/commands/pr.tssrc/commands/setup.tssrc/commands/open.tssrc/commands/list.tssrc/commands/new.tssrc/commands/extract.tssrc/commands/purge.ts
tsconfig.json
📄 CodeRabbit inference engine (.cursor/rules/project.mdc)
TypeScript compilation settings must be managed in tsconfig.json.
Files:
tsconfig.json
src/commands/list.ts
📄 CodeRabbit inference engine (.cursor/rules/project.mdc)
Provide functionality to list existing Git worktrees in src/commands/list.ts.
Files:
src/commands/list.ts
src/commands/new.ts
📄 CodeRabbit inference engine (.cursor/rules/project.mdc)
Implement logic for creating new Git worktrees, including options for branch creation, dependency installation, and opening in an editor, in src/commands/new.ts.
Files:
src/commands/new.ts
package.json
📄 CodeRabbit inference engine (.cursor/rules/project.mdc)
Project configuration, including dependencies, scripts, and publishing settings, must be defined in package.json.
Files:
package.json
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
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
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
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
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
Learning: Applies to src/commands/list.ts : Provide functionality to list existing Git worktrees in src/commands/list.ts.
Learnt from: juristr
Repo: johnlindquist/worktree-cli PR: 20
File: src/commands/extract.ts:124-127
Timestamp: 2025-08-04T14:22:29.156Z
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.
📚 Learning: 2025-08-04T13:02:29.847Z
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
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/index.tssrc/commands/remove.tsbuild/commands/list.jsbuild/commands/purge.jssrc/utils/paths.tstest/utils.test.tsbuild/utils/atomic.jsbuild/commands/remove.jssrc/utils/setup.tsbuild/utils/git.jssrc/commands/pr.tssrc/commands/setup.tsbuild/utils/paths.jstest/integration.test.tstest/integration.test.jsbuild/utils/tui.jsbuild/commands/new.jsbuild/commands/open.jssrc/commands/open.tsbuild/index.jsbuild/commands/extract.jssrc/commands/list.tssrc/commands/new.tstest/utils.test.jsbuild/commands/setup.jssrc/commands/extract.tssrc/utils/tui.tssrc/utils/atomic.tssrc/commands/purge.tsbuild/commands/pr.jssrc/utils/git.ts
📚 Learning: 2025-08-04T13:02:29.847Z
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
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/index.tssrc/commands/setup.ts
📚 Learning: 2025-08-04T13:02:29.847Z
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
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/remove.tsbuild/commands/list.jsbuild/commands/purge.jssrc/utils/paths.tstest/utils.test.tsbuild/utils/atomic.jsbuild/commands/remove.jssrc/utils/setup.tsbuild/utils/git.jssrc/commands/pr.tsbuild/utils/setup.jssrc/commands/setup.tsbuild/utils/paths.jstest/integration.test.tstest/integration.test.jsbuild/utils/tui.jsbuild/commands/new.jsbuild/commands/open.jssrc/commands/open.tsbuild/index.jsbuild/commands/extract.jssrc/commands/list.tssrc/commands/new.tstest/utils.test.jsbuild/commands/setup.jssrc/commands/extract.tssrc/utils/tui.tssrc/utils/atomic.tssrc/commands/purge.tsbuild/commands/pr.jssrc/utils/git.ts
📚 Learning: 2025-08-04T13:02:29.847Z
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
Learning: Applies to src/commands/list.ts : Provide functionality to list existing Git worktrees in src/commands/list.ts.
Applied to files:
src/commands/remove.tsbuild/commands/list.jsbuild/commands/purge.jssrc/utils/paths.tstest/utils.test.tsbuild/utils/atomic.jsbuild/commands/remove.jssrc/utils/setup.tsbuild/utils/git.jssrc/commands/pr.tsbuild/utils/setup.jssrc/commands/setup.tsbuild/utils/paths.jstest/integration.test.tstest/integration.test.jsbuild/utils/tui.jsbuild/commands/new.jsbuild/commands/open.jssrc/commands/open.tsbuild/index.jsbuild/commands/extract.jssrc/commands/list.tssrc/commands/new.tstest/utils.test.jsbuild/commands/setup.jssrc/commands/extract.tssrc/utils/tui.tssrc/utils/atomic.tssrc/commands/purge.tsbuild/commands/pr.jssrc/utils/git.ts
📚 Learning: 2025-08-04T14:22:29.156Z
Learnt from: juristr
Repo: johnlindquist/worktree-cli PR: 20
File: src/commands/extract.ts:124-127
Timestamp: 2025-08-04T14:22:29.156Z
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.
Applied to files:
src/commands/remove.tsbuild/commands/list.jsbuild/commands/purge.jssrc/utils/paths.tsbuild/utils/atomic.jsbuild/commands/remove.jssrc/utils/setup.tsbuild/utils/git.jssrc/commands/pr.tssrc/commands/setup.tsbuild/utils/paths.jstest/integration.test.tstest/integration.test.jsbuild/utils/tui.jsbuild/commands/new.jsbuild/commands/open.jssrc/commands/open.tsbuild/index.jsbuild/commands/extract.jssrc/commands/list.tssrc/commands/new.tsbuild/commands/setup.jssrc/commands/extract.tssrc/utils/tui.tssrc/utils/atomic.tssrc/commands/purge.tsbuild/commands/pr.jssrc/utils/git.ts
📚 Learning: 2025-08-04T13:02:29.847Z
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
Learning: Applies to src/commands/*.ts : Leverage Execa to execute Git commands and other external processes.
Applied to files:
build/commands/list.jsbuild/utils/atomic.jssrc/utils/setup.tssrc/commands/pr.tsbuild/utils/setup.jssrc/commands/setup.tstest/integration.test.tstest/integration.test.jsbuild/commands/new.jssrc/commands/open.tsbuild/commands/extract.jssrc/commands/list.tssrc/commands/new.tsbuild/commands/setup.jssrc/commands/extract.tssrc/utils/atomic.tssrc/commands/purge.tsbuild/commands/pr.jssrc/utils/git.ts
📚 Learning: 2025-08-04T13:02:29.847Z
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
Learning: Applies to tsconfig.json : TypeScript compilation settings must be managed in tsconfig.json.
Applied to files:
tsconfig.json
📚 Learning: 2025-08-04T13:02:29.847Z
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
Learning: Code must be written in TypeScript and compiled using tsc.
Applied to files:
tsconfig.json
📚 Learning: 2025-08-04T13:02:29.847Z
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
Learning: Applies to package.json : Project configuration, including dependencies, scripts, and publishing settings, must be defined in package.json.
Applied to files:
tsconfig.json
📚 Learning: 2025-08-04T13:02:29.847Z
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
Learning: Applies to src/commands/*.ts : Provide clear, colored console feedback for success and error messages in CLI commands.
Applied to files:
src/commands/list.tssrc/utils/tui.ts
📚 Learning: 2025-08-04T13:02:29.847Z
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
Learning: Use semantic versioning for version management, automated with semantic-release.
Applied to files:
package.json
📚 Learning: 2025-08-04T13:02:29.847Z
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
Learning: Automate builds and publishing using GitHub Actions and semantic-release.
Applied to files:
package.json
🧬 Code graph analysis (20)
src/commands/remove.ts (2)
src/utils/git.ts (3)
WorktreeInfo(169-190)findWorktreeByPath(289-317)findWorktreeByBranch(277-280)src/utils/tui.ts (2)
selectWorktree(15-76)confirm(122-131)
build/commands/purge.js (2)
src/utils/git.ts (1)
getWorktrees(204-268)src/utils/tui.ts (2)
selectWorktree(15-76)confirm(122-131)
src/utils/paths.ts (1)
src/utils/git.ts (1)
getRepoName(325-349)
test/utils.test.ts (2)
src/utils/paths.ts (3)
resolveWorktreeName(19-23)getShortBranchName(34-37)validateBranchName(103-125)src/utils/atomic.ts (1)
AtomicWorktreeOperation(23-190)
build/utils/atomic.js (4)
src/utils/atomic.ts (3)
AtomicWorktreeOperation(23-190)rollback(159-182)withAtomicRollback(198-211)build/commands/pr.js (4)
execa(16-20)execa(28-28)action(110-110)atomic(187-187)build/commands/extract.js (2)
action(21-21)atomic(98-98)build/commands/new.js (2)
action(36-36)atomic(99-99)
build/commands/remove.js (3)
src/commands/remove.ts (1)
removeWorktreeHandler(8-127)src/utils/tui.ts (2)
selectWorktree(15-76)confirm(122-131)src/utils/git.ts (2)
findWorktreeByPath(289-317)findWorktreeByBranch(277-280)
build/utils/git.js (1)
src/utils/git.ts (7)
getWorktrees(204-268)findWorktreeByBranch(277-280)findWorktreeByPath(289-317)getRepoName(325-349)getRepoRoot(84-92)stashChanges(358-371)popStash(379-387)
build/utils/setup.js (3)
build/commands/setup.js (4)
fallbackSetupPath(36-36)repoRoot(145-145)commands(21-21)commands(41-41)build/utils/git.js (1)
repoRoot(306-306)build/utils/paths.js (1)
process(49-49)
test/integration.test.ts (6)
test/integration.test.js (2)
CLI_PATH(12-12)result(47-55)build/config.js (1)
__dirname(6-6)build/utils/git.js (7)
execa(5-5)execa(19-19)execa(83-83)execa(138-138)execa(173-173)execa(293-293)execa(326-326)build/utils/atomic.js (1)
result(185-185)build/utils/paths.js (1)
process(49-49)build/commands/config.js (1)
worktreePath(16-16)
test/integration.test.js (4)
build/utils/git.js (7)
execa(5-5)execa(19-19)execa(83-83)execa(138-138)execa(173-173)execa(293-293)execa(326-326)build/utils/atomic.js (1)
result(185-185)build/utils/paths.js (1)
process(49-49)build/commands/config.js (1)
worktreePath(16-16)
build/utils/tui.js (2)
src/utils/tui.ts (2)
selectWorktree(15-76)confirm(122-131)src/utils/git.ts (1)
getWorktrees(204-268)
build/commands/new.js (5)
build/utils/tui.js (5)
options(15-15)options(135-135)execa(182-186)execa(196-200)handleDirtyState(153-168)build/utils/git.js (8)
execa(5-5)execa(19-19)execa(83-83)execa(138-138)execa(173-173)execa(293-293)execa(326-326)isMainRepoBare(51-74)build/utils/paths.js (2)
process(49-49)resolveWorktreePath(48-71)build/utils/atomic.js (2)
atomic(183-183)AtomicWorktreeOperation(20-175)build/commands/open.js (2)
configuredEditor(96-96)editorCommand(97-97)
build/commands/open.js (3)
src/commands/open.ts (1)
openWorktreeHandler(9-129)src/utils/tui.ts (1)
selectWorktree(15-76)src/utils/git.ts (2)
findWorktreeByPath(289-317)findWorktreeByBranch(277-280)
src/commands/open.ts (3)
src/utils/git.ts (3)
WorktreeInfo(169-190)findWorktreeByPath(289-317)findWorktreeByBranch(277-280)src/utils/tui.ts (1)
selectWorktree(15-76)src/config.ts (2)
getDefaultEditor(43-45)shouldSkipEditor(68-70)
src/commands/list.ts (1)
src/utils/git.ts (1)
getWorktrees(204-268)
build/commands/setup.js (7)
build/utils/git.js (4)
isMainRepoBare(51-74)stashChanges(320-334)getRepoRoot(81-90)popStash(341-350)build/utils/tui.js (4)
options(15-15)options(135-135)handleDirtyState(153-168)confirmCommands(134-146)build/utils/paths.js (2)
validateBranchName(78-96)resolveWorktreePath(48-71)src/utils/paths.ts (2)
validateBranchName(103-125)resolveWorktreePath(60-95)src/utils/git.ts (4)
isMainRepoBare(52-76)stashChanges(358-371)getRepoRoot(84-92)popStash(379-387)build/config.js (1)
getDefaultEditor(32-34)build/utils/atomic.js (2)
atomic(183-183)AtomicWorktreeOperation(20-175)
src/utils/tui.ts (3)
build/utils/tui.js (10)
selectWorktree(14-60)options(15-15)options(135-135)worktrees(16-16)indicators(79-79)confirm(101-109)inputText(117-126)confirmCommands(134-146)handleDirtyState(153-168)selectPullRequest(175-233)src/utils/git.ts (2)
WorktreeInfo(169-190)getWorktrees(204-268)build/utils/git.js (4)
worktrees(180-180)worktrees(247-247)worktrees(258-258)getWorktrees(171-238)
src/utils/atomic.ts (1)
build/utils/atomic.js (3)
AtomicWorktreeOperation(20-175)withAtomicRollback(182-193)atomic(183-183)
src/commands/purge.ts (2)
src/utils/git.ts (1)
getWorktrees(204-268)src/utils/tui.ts (2)
selectWorktree(15-76)confirm(122-131)
src/utils/git.ts (1)
build/utils/git.js (11)
getWorktrees(171-238)blocks(179-179)worktrees(180-180)worktrees(247-247)worktrees(258-258)lines(183-183)findWorktreeByBranch(246-249)getRepoName(290-312)getRepoRoot(81-90)stashChanges(320-334)popStash(341-350)
| for (const wt of selectedWorktrees) { | ||
| console.log(chalk.blue(`\nRemoving worktree for branch "${wt.branch || '(detached)'}"`)); | ||
| let removedSuccessfully = false; | ||
| try { | ||
| // Handle locked worktrees | ||
| if (wt.locked) { | ||
| console.log(chalk.yellow(`Worktree is locked${wt.lockReason ? `: ${wt.lockReason}` : ''}`)); | ||
| const forceUnlock = await confirm("Force remove this locked worktree?", false); | ||
| if (!forceUnlock) { | ||
| console.log(chalk.yellow(`Skipping locked worktree.`)); | ||
| continue; | ||
| } | ||
| } | ||
| catch (removeError) { // Catch potential errors | ||
| const execaError = removeError; // Type assertion | ||
| const stderr = execaError?.stderr || ''; | ||
| const message = execaError?.message || String(removeError); | ||
| // --- Enhanced Error Handling --- | ||
| if (stderr.includes("modified or untracked files")) { | ||
| // Specific handling for dirty worktrees | ||
| console.log(chalk.yellow(`Worktree contains modified or untracked files.`)); | ||
| const forceAnswer = await askQuestion(`Do you want to force remove this worktree (this may lose changes)? (y/N): `); | ||
| if (forceAnswer.toLowerCase() === "y") { | ||
| try { | ||
| await execa("git", ["worktree", "remove", "--force", wt.path]); | ||
| console.log(chalk.green(`Force removed worktree metadata for ${wt.path}.`)); | ||
| removedSuccessfully = true; // Mark as successfully removed by git command | ||
| } | ||
| catch (forceError) { | ||
| const forceExecaError = forceError; | ||
| console.error(chalk.red(`Failed to force remove worktree metadata for "${wt.branch}":`), forceExecaError.stderr || forceExecaError.message); | ||
| // Do not attempt rm -rf if even force remove failed | ||
| } | ||
| // Try to remove the worktree | ||
| await execa("git", ["worktree", "remove", wt.path]); | ||
| console.log(chalk.green(`Removed worktree metadata for ${wt.path}.`)); | ||
| removedSuccessfully = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Locked worktree removal may silently fail without --force.
When the user confirms force removal of a locked worktree (line 63-67), the code proceeds to the normal git worktree remove (line 70) without the --force flag. This will fail for locked worktrees since git worktree remove requires --force to remove a locked worktree.
Apply this diff to add the force flag when removing locked worktrees:
if (wt.locked) {
console.log(chalk.yellow(`Worktree is locked${wt.lockReason ? `: ${wt.lockReason}` : ''}`));
const forceUnlock = await confirm("Force remove this locked worktree?", false);
if (!forceUnlock) {
console.log(chalk.yellow(`Skipping locked worktree.`));
continue;
}
+ // Remove locked worktree with --force
+ await execa("git", ["worktree", "remove", "--force", wt.path]);
+ console.log(chalk.green(`Removed locked worktree metadata for ${wt.path}.`));
+ removedSuccessfully = true;
+ }
+ else {
+ // Try to remove the worktree normally
+ await execa("git", ["worktree", "remove", wt.path]);
+ console.log(chalk.green(`Removed worktree metadata for ${wt.path}.`));
+ removedSuccessfully = true;
}
- // Try to remove the worktree
- await execa("git", ["worktree", "remove", wt.path]);
- console.log(chalk.green(`Removed worktree metadata for ${wt.path}.`));
- removedSuccessfully = 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.
| for (const wt of selectedWorktrees) { | |
| console.log(chalk.blue(`\nRemoving worktree for branch "${wt.branch || '(detached)'}"`)); | |
| let removedSuccessfully = false; | |
| try { | |
| // Handle locked worktrees | |
| if (wt.locked) { | |
| console.log(chalk.yellow(`Worktree is locked${wt.lockReason ? `: ${wt.lockReason}` : ''}`)); | |
| const forceUnlock = await confirm("Force remove this locked worktree?", false); | |
| if (!forceUnlock) { | |
| console.log(chalk.yellow(`Skipping locked worktree.`)); | |
| continue; | |
| } | |
| } | |
| catch (removeError) { // Catch potential errors | |
| const execaError = removeError; // Type assertion | |
| const stderr = execaError?.stderr || ''; | |
| const message = execaError?.message || String(removeError); | |
| // --- Enhanced Error Handling --- | |
| if (stderr.includes("modified or untracked files")) { | |
| // Specific handling for dirty worktrees | |
| console.log(chalk.yellow(`Worktree contains modified or untracked files.`)); | |
| const forceAnswer = await askQuestion(`Do you want to force remove this worktree (this may lose changes)? (y/N): `); | |
| if (forceAnswer.toLowerCase() === "y") { | |
| try { | |
| await execa("git", ["worktree", "remove", "--force", wt.path]); | |
| console.log(chalk.green(`Force removed worktree metadata for ${wt.path}.`)); | |
| removedSuccessfully = true; // Mark as successfully removed by git command | |
| } | |
| catch (forceError) { | |
| const forceExecaError = forceError; | |
| console.error(chalk.red(`Failed to force remove worktree metadata for "${wt.branch}":`), forceExecaError.stderr || forceExecaError.message); | |
| // Do not attempt rm -rf if even force remove failed | |
| } | |
| // Try to remove the worktree | |
| await execa("git", ["worktree", "remove", wt.path]); | |
| console.log(chalk.green(`Removed worktree metadata for ${wt.path}.`)); | |
| removedSuccessfully = true; | |
| for (const wt of selectedWorktrees) { | |
| console.log(chalk.blue(`\nRemoving worktree for branch "${wt.branch || '(detached)'}"`)); | |
| let removedSuccessfully = false; | |
| try { | |
| // Handle locked worktrees | |
| if (wt.locked) { | |
| console.log(chalk.yellow(`Worktree is locked${wt.lockReason ? `: ${wt.lockReason}` : ''}`)); | |
| const forceUnlock = await confirm("Force remove this locked worktree?", false); | |
| if (!forceUnlock) { | |
| console.log(chalk.yellow(`Skipping locked worktree.`)); | |
| continue; | |
| } | |
| // Remove locked worktree with --force | |
| await execa("git", ["worktree", "remove", "--force", wt.path]); | |
| console.log(chalk.green(`Removed locked worktree metadata for ${wt.path}.`)); | |
| removedSuccessfully = true; | |
| } | |
| else { | |
| // Try to remove the worktree normally | |
| await execa("git", ["worktree", "remove", wt.path]); | |
| console.log(chalk.green(`Removed worktree metadata for ${wt.path}.`)); | |
| removedSuccessfully = true; | |
| } |
🤖 Prompt for AI Agents
In build/commands/purge.js around lines 56 to 72, when a worktree is locked and
the user confirms force removal the code still calls git worktree remove without
the --force flag; change the execa invocation so that it conditionally includes
"--force" when wt.locked and the user chose to force (i.e. when forceUnlock is
true), otherwise call it as before; ensure the modified args array passes the
--force flag before the path so git removes locked worktrees successfully.
| export async function getRepoName(cwd = ".") { | ||
| try { | ||
| // Try to get from remote URL first | ||
| const { stdout } = await execa("git", ["-C", cwd, "remote", "get-url", "origin"]); | ||
| const remoteUrl = stdout.trim(); | ||
| // Extract repo name from URL | ||
| // Handles: git@github.com:user/repo.git, https://github.com/user/repo.git, etc. | ||
| const match = remoteUrl.match(/\/([^\/]+?)(\.git)?$/); | ||
| if (match && match[1]) { | ||
| return match[1]; | ||
| } | ||
| } | ||
| catch { | ||
| // Fall through to directory name | ||
| } | ||
| // Fallback: use the directory name of the repo root | ||
| const repoRoot = await getRepoRoot(cwd); | ||
| if (repoRoot) { | ||
| const { basename } = await import('node:path'); | ||
| return basename(repoRoot); | ||
| } | ||
| return 'repo'; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regex may fail to extract repo name from SSH URLs with colons.
The regex /\/([^\/]+?)(\.git)?$/ expects a / before the repo name, but SSH URLs like git@github.com:user/repo.git use : as the separator. This case will fall through to the directory name fallback.
Consider adding support for SSH URLs:
export async function getRepoName(cwd = ".") {
try {
// Try to get from remote URL first
const { stdout } = await execa("git", ["-C", cwd, "remote", "get-url", "origin"]);
const remoteUrl = stdout.trim();
// Extract repo name from URL
- // Handles: git@github.com:user/repo.git, https://github.com/user/repo.git, etc.
- const match = remoteUrl.match(/\/([^\/]+?)(\.git)?$/);
+ // Handles: git@github.com:user/repo.git, https://github.com/user/repo.git, etc.
+ // Try SSH format first (git@host:user/repo.git)
+ let match = remoteUrl.match(/[:\\/]([^\\/]+?)(\.git)?$/);
if (match && match[1]) {
return match[1];
}
}🤖 Prompt for AI Agents
In build/utils/git.js around lines 290 to 312, the repo-name extraction regex
only handles URLs with a "/" separator and misses SSH URLs like
"git@github.com:user/repo.git"; update the extraction to support both "/" and
":" separators (e.g. use a single regex that matches either ":" or "/" before
the repo name and still strips an optional ".git" suffix), keep the existing
fallback to repoRoot if the regex fails, and ensure remoteUrl is trimmed before
matching.
| // Confirm we're in a git repo | ||
| await execa("git", ["rev-parse", "--is-inside-work-tree"]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's check how git rev-parse behaves in different repo types
# and examine the current implementation and other commands
# Check the current implementation in list.ts
echo "=== Current list.ts implementation ==="
git ls-files src/commands/list.ts
# Find all git repo checks across commands
echo -e "\n=== Git repo validation patterns in commands ==="
rg "rev-parse" src/commands/ --type=ts -B2 -A2
# Check for bare repository mentions in the codebase
echo -e "\n=== Bare repository references ==="
rg -i "bare" --type=ts --type=md
# Check project documentation/rules
echo -e "\n=== Project rules ==="
if [ -f ".cursor/rules/project.mdc" ]; then
head -100 ".cursor/rules/project.mdc"
fiRepository: johnlindquist/worktree-cli
Length of output: 11182
🏁 Script executed:
#!/bin/bash
# Check how commands like new.ts and extract.ts handle bare repos
echo "=== new.ts bare repo handling ==="
sed -n '1,50p' src/commands/new.ts | grep -A 15 "Validate we're in"
echo -e "\n=== extract.ts bare repo handling ==="
sed -n '1,50p' src/commands/extract.ts | grep -A 15 "Validate we're in"
echo -e "\n=== isMainRepoBare implementation ==="
rg -A 20 "export async function isMainRepoBare" src/utils/git.tsRepository: johnlindquist/worktree-cli
Length of output: 2718
🏁 Script executed:
#!/bin/bash
# Check the integration test to see how bare repo testing is structured
echo "=== Bare Repository Support test ==="
rg -A 30 "should work with bare repositories" test/integration.test.tsRepository: johnlindquist/worktree-cli
Length of output: 1002
🏁 Script executed:
#!/bin/bash
# Check if list command tests exist and if they test bare repos
echo "=== List command tests ==="
rg -n "list\|List" test/ --type=ts -B2 -A5 | head -50
echo -e "\n=== Check if there are any CLI integration tests ==="
fd "test|spec" test/ --type=ts -x wc -l {} \; | head -20Repository: johnlindquist/worktree-cli
Length of output: 337
🏁 Script executed:
#!/bin/bash
# Final verification: confirm that git rev-parse --git-dir works in bare repos
# and check the exact difference between the two commands
echo "=== How different git commands behave ==="
echo "Testing pattern differences:"
echo "1. --is-inside-work-tree: checks if inside a work tree (fails in bare repo)"
echo "2. --git-dir: returns git directory path (works in both bare and normal repos)"
echo ""
echo "=== Confirm list.ts structure ==="
sed -n '1,30p' src/commands/list.tsRepository: johnlindquist/worktree-cli
Length of output: 1237
Use --git-dir instead of --is-inside-work-tree to support bare repositories.
The git rev-parse --is-inside-work-tree check fails (exits non-zero) in bare repositories, preventing the list command from working in bare repo setups. Since the project explicitly supports bare repositories (as shown by test cases and the isMainRepoBare utility), this check should be replaced:
- await execa("git", ["rev-parse", "--is-inside-work-tree"]);
+ await execa("git", ["rev-parse", "--git-dir"]);The --git-dir option works in both bare and normal repositories, validating we're in a git repository without requiring a working tree.
📝 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.
| // Confirm we're in a git repo | |
| await execa("git", ["rev-parse", "--is-inside-work-tree"]); | |
| // Confirm we're in a git repo | |
| await execa("git", ["rev-parse", "--git-dir"]); |
🤖 Prompt for AI Agents
In src/commands/list.ts around lines 7 to 8, the code uses `git rev-parse
--is-inside-work-tree` which fails for bare repositories; replace that command
with `git rev-parse --git-dir` so the check succeeds for both bare and non-bare
repos (i.e., call execa("git", ["rev-parse", "--git-dir"]) and keep the existing
error handling/flow).
| try { | ||
| // Handle locked worktrees | ||
| if (wt.locked) { | ||
| console.log(chalk.yellow(`Worktree is locked${wt.lockReason ? `: ${wt.lockReason}` : ''}`)); | ||
| const forceUnlock = await confirm("Force remove this locked worktree?", false); | ||
| if (!forceUnlock) { | ||
| console.log(chalk.yellow(`Skipping locked worktree.`)); | ||
| continue; | ||
| } | ||
| } // End of inner try-catch for git worktree remove | ||
|
|
||
| // --- Optional Folder Deletion (Only if Git remove succeeded) --- | ||
| if (removedSuccessfully) { | ||
| // Optionally remove the physical directory if it still exists *after* git remove succeeded | ||
| try { | ||
| await stat(wt.path); // Check if directory still exists | ||
| console.log(chalk.blue(`Attempting to delete folder ${wt.path}...`)); | ||
| await rm(wt.path, { recursive: true, force: true }); | ||
| console.log(chalk.green(`Deleted folder ${wt.path}.`)); | ||
| } catch (statError: any) { | ||
| // If stat fails with 'ENOENT', the directory doesn't exist, which is fine. | ||
| if (statError.code !== 'ENOENT') { | ||
| console.warn(chalk.yellow(`Could not check or delete folder ${wt.path}: ${statError.message}`)); | ||
| } else { | ||
| // Directory already gone, maybe git remove --force deleted it, or it was never there | ||
| console.log(chalk.grey(`Folder ${wt.path} does not exist or was already removed.`)); | ||
| } | ||
|
|
||
| // Try to remove the worktree | ||
| await execa("git", ["worktree", "remove", wt.path]); | ||
| console.log(chalk.green(`Removed worktree metadata for ${wt.path}.`)); | ||
| removedSuccessfully = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Locked worktree removal doesn't use --force.
When a locked worktree is detected and the user confirms force removal (line 75-76), the subsequent git worktree remove at line 83 is called without --force. This will fail because locked worktrees require the --force flag.
Apply this diff to fix the locked worktree removal:
// Try to remove the worktree
- await execa("git", ["worktree", "remove", wt.path]);
+ const removeArgs = ["worktree", "remove"];
+ if (wt.locked) {
+ removeArgs.push("--force");
+ }
+ removeArgs.push(wt.path);
+ await execa("git", removeArgs);
console.log(chalk.green(`Removed worktree metadata for ${wt.path}.`));📝 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.
| try { | |
| // Handle locked worktrees | |
| if (wt.locked) { | |
| console.log(chalk.yellow(`Worktree is locked${wt.lockReason ? `: ${wt.lockReason}` : ''}`)); | |
| const forceUnlock = await confirm("Force remove this locked worktree?", false); | |
| if (!forceUnlock) { | |
| console.log(chalk.yellow(`Skipping locked worktree.`)); | |
| continue; | |
| } | |
| } // End of inner try-catch for git worktree remove | |
| // --- Optional Folder Deletion (Only if Git remove succeeded) --- | |
| if (removedSuccessfully) { | |
| // Optionally remove the physical directory if it still exists *after* git remove succeeded | |
| try { | |
| await stat(wt.path); // Check if directory still exists | |
| console.log(chalk.blue(`Attempting to delete folder ${wt.path}...`)); | |
| await rm(wt.path, { recursive: true, force: true }); | |
| console.log(chalk.green(`Deleted folder ${wt.path}.`)); | |
| } catch (statError: any) { | |
| // If stat fails with 'ENOENT', the directory doesn't exist, which is fine. | |
| if (statError.code !== 'ENOENT') { | |
| console.warn(chalk.yellow(`Could not check or delete folder ${wt.path}: ${statError.message}`)); | |
| } else { | |
| // Directory already gone, maybe git remove --force deleted it, or it was never there | |
| console.log(chalk.grey(`Folder ${wt.path} does not exist or was already removed.`)); | |
| } | |
| // Try to remove the worktree | |
| await execa("git", ["worktree", "remove", wt.path]); | |
| console.log(chalk.green(`Removed worktree metadata for ${wt.path}.`)); | |
| removedSuccessfully = true; | |
| try { | |
| // Handle locked worktrees | |
| if (wt.locked) { | |
| console.log(chalk.yellow(`Worktree is locked${wt.lockReason ? `: ${wt.lockReason}` : ''}`)); | |
| const forceUnlock = await confirm("Force remove this locked worktree?", false); | |
| if (!forceUnlock) { | |
| console.log(chalk.yellow(`Skipping locked worktree.`)); | |
| continue; | |
| } | |
| } | |
| // Try to remove the worktree | |
| const removeArgs = ["worktree", "remove"]; | |
| if (wt.locked) { | |
| removeArgs.push("--force"); | |
| } | |
| removeArgs.push(wt.path); | |
| await execa("git", removeArgs); | |
| console.log(chalk.green(`Removed worktree metadata for ${wt.path}.`)); | |
| removedSuccessfully = true; |
🤖 Prompt for AI Agents
In src/commands/purge.ts around lines 71 to 85, the code prompts the user to
force-remove a locked worktree but always calls git worktree remove without
--force; update the removal call to include --force when the user confirmed
forceUnlock. Construct the git args (["worktree","remove", wt.path]) and, if
wt.locked and forceUnlock is true, push "--force" onto the args before calling
execa so locked worktrees are removed successfully.
| } finally { | ||
| // Restore stashed changes if we stashed them | ||
| if (stashed) { | ||
| console.log(chalk.blue("Restoring your stashed changes...")); | ||
| const restored = await popStash("."); | ||
| if (restored) { | ||
| console.log(chalk.green("Changes restored successfully.")); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the popStash function implementation
ast-grep --pattern $'async function popStash($_, $_) {
$$$
}'Repository: johnlindquist/worktree-cli
Length of output: 52
🏁 Script executed:
# Also search for popStash definition in case it uses different syntax
rg -t ts "popStash\s*(?:=|:)" --max-count 20Repository: johnlindquist/worktree-cli
Length of output: 52
🏁 Script executed:
# Look for the popStash function by name
rg -t ts "function popStash|const popStash" -A 20Repository: johnlindquist/worktree-cli
Length of output: 526
🏁 Script executed:
# Find the stashChanges function
rg -t ts "function stashChanges|const stashChanges" -A 15Repository: johnlindquist/worktree-cli
Length of output: 940
🏁 Script executed:
# Check the full context in setup.ts around the finally block
sed -n '200,260p' src/commands/setup.tsRepository: johnlindquist/worktree-cli
Length of output: 2108
🏁 Script executed:
# Check if git stash output contains reference info
git stash push --help 2>/dev/null | head -20 || echo "Checking Git stash output format..."Repository: johnlindquist/worktree-cli
Length of output: 919
🏁 Script executed:
# Look at the actual git stash output to understand what info is available
rg -t ts "stdout" src/utils/git.ts -B 2 -A 2 | head -30Repository: johnlindquist/worktree-cli
Length of output: 1704
Provide user feedback when stash restoration fails and offer recovery steps.
The finally block calls popStash() but only logs success; if restoration fails (returns false), the user receives no feedback in the CLI. Additionally, stashChanges() doesn't capture the stash reference, leaving users unable to manually recover their changes. Consider logging the failure with a stash recovery command (e.g., git stash list or git stash pop stash@{0}) so users can retrieve their work if needed.
🤖 Prompt for AI Agents
In src/commands/setup.ts around lines 239 to 248, the finally block currently
only logs successful stash restoration and gives no feedback or recovery
instructions when popStash() fails, and stashChanges() doesn't retain the stash
reference; update the stashing flow to capture and return the stash reference
when creating the stash (e.g., stashChanges() should return the stash name/index
or full ref), then in the finally block check the boolean result from popStash()
and, on failure, log a clear error (chalk.red) that restoration failed and
include actionable recovery steps such as running `git stash list` and `git
stash pop <stashRef>` (substitute the captured ref if available) or `git stash
apply <stashRef>` and how to inspect with `git stash show -p <stashRef>`,
ensuring the message instructs the user how to manually recover their changes;
also keep the existing success log path unchanged.
|
🎉 This PR is included in version 2.13.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/git-utils.test.ts (1)
11-38: Consider extractingcreateTestRepointo a shared test helper
createTestRepo()and the surroundingbeforeAll/afterAllpattern are repeated across multiple describes here, and a similar pattern appears in other test files (e.g.test/integration.test.jsper provided snippet). Centralizing this into a small helper module (e.g.test/helpers/git.ts) would reduce duplication and keep future changes to the test repo setup (extra config, hooks, etc.) in one place.Not urgent, but worth considering as tests grow.
Also applies to: 40-50, 97-107, 124-134, 151-161, 184-194, 203-213, 223-233, 242-252
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(9 hunks)test/git-utils.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
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
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
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
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
Learning: Applies to src/commands/list.ts : Provide functionality to list existing Git worktrees in src/commands/list.ts.
Learnt from: juristr
Repo: johnlindquist/worktree-cli PR: 20
File: src/commands/extract.ts:124-127
Timestamp: 2025-08-04T14:22:29.156Z
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.
📚 Learning: 2025-08-04T13:02:29.847Z
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
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:
test/git-utils.test.tsREADME.md
📚 Learning: 2025-08-04T13:02:29.847Z
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
Learning: Applies to src/commands/list.ts : Provide functionality to list existing Git worktrees in src/commands/list.ts.
Applied to files:
test/git-utils.test.tsREADME.md
📚 Learning: 2025-08-04T13:02:29.847Z
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
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:
test/git-utils.test.tsREADME.md
📚 Learning: 2025-08-04T13:02:29.847Z
Learnt from: CR
Repo: johnlindquist/worktree-cli PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-04T13:02:29.847Z
Learning: Applies to src/commands/*.ts : Leverage Execa to execute Git commands and other external processes.
Applied to files:
test/git-utils.test.ts
📚 Learning: 2025-08-04T14:22:29.156Z
Learnt from: juristr
Repo: johnlindquist/worktree-cli PR: 20
File: src/commands/extract.ts:124-127
Timestamp: 2025-08-04T14:22:29.156Z
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.
Applied to files:
test/git-utils.test.tsREADME.md
🧬 Code graph analysis (1)
test/git-utils.test.ts (1)
test/integration.test.js (2)
testDir(18-18)repoDir(19-19)
🔇 Additional comments (1)
README.md (1)
5-13: Documentation updates clearly surface the new behavior and look consistent with the implementation goalsThe README changes do a good job of:
- Advertising the new capabilities (interactive TUI flows, stash‑aware operations, bare repo support, atomic worktree handling, trust‑based setup).
- Documenting new flags and flows (
--trust, interactivewt pr,wt open,wt list/remove/purge, path namespacing, branch name sanitization).- Explaining the security and atomicity model at a level that matches the utilities and PR description.
I don’t see mismatches or problematic promises based on the context provided; this looks ready to ship.
Also applies to: 42-46, 60-67, 71-104, 106-159, 160-167, 205-222, 224-250, 284-298, 308-333, 350-367
| async function createTestRepo(): Promise<TestContext> { | ||
| const testDir = join(tmpdir(), `wt-git-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); | ||
| const repoDir = join(testDir, 'repo'); | ||
|
|
||
| await mkdir(repoDir, { recursive: true }); | ||
| await execa('git', ['init'], { cwd: repoDir }); | ||
| await execa('git', ['config', 'user.email', 'test@test.com'], { cwd: repoDir }); | ||
| await execa('git', ['config', 'user.name', 'Test User'], { cwd: repoDir }); | ||
| await writeFile(join(repoDir, 'README.md'), '# Test\n'); | ||
| await execa('git', ['add', '.'], { cwd: repoDir }); | ||
| await execa('git', ['commit', '-m', 'Initial'], { cwd: repoDir }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stabilize tests by not assuming Git’s default branch is main
createTestRepo() runs git init without specifying a branch (Line 22), but multiple tests assert the branch name is exactly 'main' (Lines 55–58, 111–114, 198–200). On machines where init.defaultBranch is set differently (e.g. master), these tests will fail even though the git utilities are correct.
To make the suite deterministic and CI‑friendly, force the initial branch in the test repo:
- await execa('git', ['init'], { cwd: repoDir });
+ await execa('git', ['init', '-b', 'main'], { cwd: repoDir });Alternatively, you could detect the actual default branch via git symbolic-ref HEAD and assert against that, but the -b main approach keeps expectations simple.
Also applies to: 51-58, 108-114, 195-200
🤖 Prompt for AI Agents
In test/git-utils.test.ts around lines 17 to 27, createTestRepo() calls `git
init` without specifying a branch which makes tests brittle on systems where the
default branch isn't `main`; update the function to initialize the repo with a
fixed branch (e.g. run `git init -b main`) so the created repo always has `main`
as the initial branch, or alternatively run `git symbolic-ref --short HEAD`
after init to detect the actual initial branch and use that value in subsequent
assertions; apply the same change where createTestRepo is used so tests assert
against the deterministic branch name.
Summary
This PR implements all 10 improvements identified in the codebase review, focusing on data safety, core functionality, and user experience.
Changes
Fix Global Path Collisions - Namespace global paths by repo name to prevent cross-repo collisions (
~/worktrees/repo-name/branch)Enable Bare Repository Support - Removed blocking check for bare repos, enabling the most efficient worktree workflow
Refactor
wt prto Avoid Context Switching - Usegit fetch origin refs/pull/ID/head:branchinstead of checkout, no longer switches branches in main worktreeInteractive TUI for Missing Arguments - Added
promptslibrary for fuzzy-searchable selection when args are missingHandle Dirty States Gracefully - Offer stash/pop workflow with user choice (stash, abort, continue)
Replace Regex Security with Trust Model - Removed brittle regex blocklist, added
--trustflag for CI, shows commands before executionCentralize Path and Naming Logic - Created
src/utils/paths.tswithresolveWorktreeName()using/→-replacementRobust Git Output Parsing - Created typed
WorktreeInfointerface andgetWorktrees()parser handling all edge casesAtomic Operations and Rollback - Created
AtomicWorktreeOperationclass for automatic cleanup on failureAutomated Integration Tests - Added 26 tests with vitest covering all major functionality
New Files
src/utils/paths.ts- Centralized path resolutionsrc/utils/atomic.ts- Atomic operation managersrc/utils/tui.ts- Interactive TUI utilitiestest/integration.test.ts- Integration teststest/utils.test.ts- Unit testsvitest.config.ts- Test configurationNew Dependencies
prompts- Interactive prompts library@types/prompts- TypeScript typesTest plan
pnpm test)pnpm build)Summary by CodeRabbit
New Features
Improvements
Tests & Docs
✏️ Tip: You can customize this high-level summary in your review settings.