diff --git a/README.md b/README.md index 880f72d..3e075cc 100644 --- a/README.md +++ b/README.md @@ -318,6 +318,12 @@ iloom finish # AI assisted validation, commit, merge steps, as well as loom cleanup (run this from the loom directory) # Alias: dn +iloom rebase +# Rebase current branch on main with Claude-assisted conflict resolution (run this from a loom directory) +# Options: +# -f, --force - Skip confirmation prompts +# -n, --dry-run - Preview actions without executing + iloom cleanup [identifier...] # Remove a loom without merging (safely, by default) diff --git a/src/cli.ts b/src/cli.ts index 7e65f85..65fa9dd 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -212,6 +212,22 @@ program } }) +program + .command('rebase') + .description('Rebase current branch on main with Claude-assisted conflict resolution') + .option('-f, --force', 'Skip confirmation prompts') + .option('-n, --dry-run', 'Preview actions without executing') + .action(async (options: { force?: boolean; dryRun?: boolean }) => { + try { + const { RebaseCommand } = await import('./commands/rebase.js') + const command = new RebaseCommand() + await command.execute(options) + } catch (error) { + logger.error(`Failed to rebase: ${error instanceof Error ? error.message : 'Unknown error'}`) + process.exit(1) + } + }) + program .command('spin') .alias('ignite') diff --git a/src/commands/rebase.test.ts b/src/commands/rebase.test.ts new file mode 100644 index 0000000..3ba3d93 --- /dev/null +++ b/src/commands/rebase.test.ts @@ -0,0 +1,316 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { RebaseCommand, WorktreeValidationError } from './rebase.js' +import type { MergeManager } from '../lib/MergeManager.js' +import type { GitWorktreeManager } from '../lib/GitWorktreeManager.js' +import type { SettingsManager } from '../lib/SettingsManager.js' +import type { GitWorktree } from '../types/worktree.js' + +// Mock dependencies +vi.mock('../lib/MergeManager.js') +vi.mock('../lib/GitWorktreeManager.js') +vi.mock('../lib/SettingsManager.js') +vi.mock('../utils/git.js', () => ({ + isValidGitRepo: vi.fn(), + getRepoRoot: vi.fn(), +})) + +// Import mocked functions +import { isValidGitRepo, getRepoRoot } from '../utils/git.js' + +describe('RebaseCommand', () => { + let command: RebaseCommand + let mockMergeManager: MergeManager + let mockGitWorktreeManager: GitWorktreeManager + let mockSettingsManager: SettingsManager + let originalCwd: typeof process.cwd + + // Helper to create mock worktree + const createMockWorktree = (overrides: Partial = {}): GitWorktree => ({ + path: '/test/worktree', + branch: 'feat/issue-123-test', + commit: 'abc123', + bare: false, + detached: false, + locked: false, + ...overrides, + }) + + beforeEach(() => { + // Save original cwd + originalCwd = process.cwd + + // Create mock MergeManager + mockMergeManager = { + rebaseOnMain: vi.fn().mockResolvedValue(undefined), + } as unknown as MergeManager + + // Create mock GitWorktreeManager + mockGitWorktreeManager = { + listWorktrees: vi.fn(), + isMainWorktree: vi.fn(), + } as unknown as GitWorktreeManager + + // Create mock SettingsManager + mockSettingsManager = { + loadSettings: vi.fn().mockResolvedValue({}), + getProtectedBranches: vi.fn().mockResolvedValue(['main', 'master', 'develop']), + } as unknown as SettingsManager + + // Create command with mocked dependencies + command = new RebaseCommand(mockMergeManager, mockGitWorktreeManager, mockSettingsManager) + }) + + afterEach(() => { + process.cwd = originalCwd + }) + + describe('WorktreeValidationError', () => { + it('creates error with message and suggestion', () => { + const error = new WorktreeValidationError('Test message', 'Test suggestion') + expect(error.message).toBe('Test message') + expect(error.suggestion).toBe('Test suggestion') + expect(error.name).toBe('WorktreeValidationError') + }) + }) + + describe('worktree validation', () => { + it('throws error when not in a git repository', async () => { + process.cwd = vi.fn().mockReturnValue('/tmp/not-a-repo') + vi.mocked(isValidGitRepo).mockResolvedValue(false) + + await expect(command.execute()).rejects.toThrow(WorktreeValidationError) + await expect(command.execute()).rejects.toThrow('Not a git repository.') + }) + + it('throws error when repo root cannot be determined', async () => { + process.cwd = vi.fn().mockReturnValue('/test/worktree') + vi.mocked(isValidGitRepo).mockResolvedValue(true) + vi.mocked(getRepoRoot).mockResolvedValue(null) + + await expect(command.execute()).rejects.toThrow(WorktreeValidationError) + await expect(command.execute()).rejects.toThrow('Could not determine repository root.') + }) + + it('throws error when directory is not a registered worktree', async () => { + process.cwd = vi.fn().mockReturnValue('/test/regular-repo') + vi.mocked(isValidGitRepo).mockResolvedValue(true) + vi.mocked(getRepoRoot).mockResolvedValue('/test/regular-repo') + vi.mocked(mockGitWorktreeManager.listWorktrees).mockResolvedValue([ + createMockWorktree({ path: '/other/worktree' }), + ]) + + await expect(command.execute()).rejects.toThrow(WorktreeValidationError) + await expect(command.execute()).rejects.toThrow('This directory is not an iloom worktree.') + }) + + it('throws error when running from main worktree', async () => { + const mainWorktree = createMockWorktree({ + path: '/test/main-repo', + branch: 'main', + bare: true, + }) + process.cwd = vi.fn().mockReturnValue('/test/main-repo') + vi.mocked(isValidGitRepo).mockResolvedValue(true) + vi.mocked(getRepoRoot).mockResolvedValue('/test/main-repo') + vi.mocked(mockGitWorktreeManager.listWorktrees).mockResolvedValue([mainWorktree]) + vi.mocked(mockGitWorktreeManager.isMainWorktree).mockResolvedValue(true) + + await expect(command.execute()).rejects.toThrow(WorktreeValidationError) + await expect(command.execute()).rejects.toThrow('Cannot rebase from the main worktree.') + }) + + it('works from subdirectory within valid worktree', async () => { + const worktree = createMockWorktree({ path: '/test/worktree' }) + process.cwd = vi.fn().mockReturnValue('/test/worktree/src/components') + vi.mocked(isValidGitRepo).mockResolvedValue(true) + vi.mocked(getRepoRoot).mockResolvedValue('/test/worktree') + vi.mocked(mockGitWorktreeManager.listWorktrees).mockResolvedValue([worktree]) + vi.mocked(mockGitWorktreeManager.isMainWorktree).mockResolvedValue(false) + + await command.execute() + + // Should use repo root, not the subdirectory + expect(mockMergeManager.rebaseOnMain).toHaveBeenCalledWith('/test/worktree', { + dryRun: false, + force: false, + }) + }) + + it('provides helpful suggestion for non-git directory', async () => { + process.cwd = vi.fn().mockReturnValue('/tmp/not-a-repo') + vi.mocked(isValidGitRepo).mockResolvedValue(false) + + try { + await command.execute() + expect.fail('Expected WorktreeValidationError') + } catch (error) { + expect(error).toBeInstanceOf(WorktreeValidationError) + expect((error as WorktreeValidationError).suggestion).toContain("'il start'") + } + }) + + it('provides helpful suggestion for regular git repo', async () => { + process.cwd = vi.fn().mockReturnValue('/test/regular-repo') + vi.mocked(isValidGitRepo).mockResolvedValue(true) + vi.mocked(getRepoRoot).mockResolvedValue('/test/regular-repo') + vi.mocked(mockGitWorktreeManager.listWorktrees).mockResolvedValue([]) + + try { + await command.execute() + expect.fail('Expected WorktreeValidationError') + } catch (error) { + expect(error).toBeInstanceOf(WorktreeValidationError) + expect((error as WorktreeValidationError).suggestion).toContain("'il list'") + } + }) + + it('provides helpful suggestion for main worktree', async () => { + const mainWorktree = createMockWorktree({ + path: '/test/main-repo', + branch: 'main', + }) + process.cwd = vi.fn().mockReturnValue('/test/main-repo') + vi.mocked(isValidGitRepo).mockResolvedValue(true) + vi.mocked(getRepoRoot).mockResolvedValue('/test/main-repo') + vi.mocked(mockGitWorktreeManager.listWorktrees).mockResolvedValue([mainWorktree]) + vi.mocked(mockGitWorktreeManager.isMainWorktree).mockResolvedValue(true) + + try { + await command.execute() + expect.fail('Expected WorktreeValidationError') + } catch (error) { + expect(error).toBeInstanceOf(WorktreeValidationError) + expect((error as WorktreeValidationError).suggestion).toContain('Navigate to a feature worktree') + } + }) + }) + + describe('execute with valid worktree', () => { + beforeEach(() => { + // Setup valid worktree context + const worktree = createMockWorktree({ path: '/test/worktree' }) + process.cwd = vi.fn().mockReturnValue('/test/worktree') + vi.mocked(isValidGitRepo).mockResolvedValue(true) + vi.mocked(getRepoRoot).mockResolvedValue('/test/worktree') + vi.mocked(mockGitWorktreeManager.listWorktrees).mockResolvedValue([worktree]) + vi.mocked(mockGitWorktreeManager.isMainWorktree).mockResolvedValue(false) + }) + + it('calls rebaseOnMain with worktree path', async () => { + await command.execute() + + expect(mockMergeManager.rebaseOnMain).toHaveBeenCalledWith('/test/worktree', { + dryRun: false, + force: false, + }) + }) + + it('succeeds when branch is already up to date', async () => { + vi.mocked(mockMergeManager.rebaseOnMain).mockResolvedValue(undefined) + + await expect(command.execute()).resolves.toBeUndefined() + }) + + it('succeeds when rebase completes without conflicts', async () => { + vi.mocked(mockMergeManager.rebaseOnMain).mockResolvedValue(undefined) + + await expect(command.execute()).resolves.toBeUndefined() + }) + + it('handles rebase conflicts by launching Claude (via MergeManager)', async () => { + // MergeManager.rebaseOnMain handles Claude conflict resolution internally + // It only throws if conflicts cannot be resolved + vi.mocked(mockMergeManager.rebaseOnMain).mockResolvedValue(undefined) + + await expect(command.execute()).resolves.toBeUndefined() + }) + + it('propagates MergeManager errors', async () => { + const mergeError = new Error('Rebase failed: merge conflict') + vi.mocked(mockMergeManager.rebaseOnMain).mockRejectedValue(mergeError) + + await expect(command.execute()).rejects.toThrow('Rebase failed: merge conflict') + }) + + it('handles dry-run mode', async () => { + await command.execute({ dryRun: true }) + + expect(mockMergeManager.rebaseOnMain).toHaveBeenCalledWith('/test/worktree', { + dryRun: true, + force: false, + }) + }) + + it('handles force mode', async () => { + await command.execute({ force: true }) + + expect(mockMergeManager.rebaseOnMain).toHaveBeenCalledWith('/test/worktree', { + dryRun: false, + force: true, + }) + }) + + it('handles both dry-run and force mode together', async () => { + await command.execute({ dryRun: true, force: true }) + + expect(mockMergeManager.rebaseOnMain).toHaveBeenCalledWith('/test/worktree', { + dryRun: true, + force: true, + }) + }) + }) + + describe('edge cases', () => { + it('handles worktree with multiple worktrees registered', async () => { + const mainWorktree = createMockWorktree({ + path: '/test/main', + branch: 'main', + }) + const featureWorktree = createMockWorktree({ + path: '/test/feat-issue-123', + branch: 'feat/issue-123-feature', + }) + process.cwd = vi.fn().mockReturnValue('/test/feat-issue-123') + vi.mocked(isValidGitRepo).mockResolvedValue(true) + vi.mocked(getRepoRoot).mockResolvedValue('/test/feat-issue-123') + vi.mocked(mockGitWorktreeManager.listWorktrees).mockResolvedValue([ + mainWorktree, + featureWorktree, + ]) + vi.mocked(mockGitWorktreeManager.isMainWorktree).mockResolvedValue(false) + + await command.execute() + + expect(mockMergeManager.rebaseOnMain).toHaveBeenCalledWith('/test/feat-issue-123', { + dryRun: false, + force: false, + }) + }) + + it('validates before calling rebaseOnMain', async () => { + process.cwd = vi.fn().mockReturnValue('/tmp/not-a-repo') + vi.mocked(isValidGitRepo).mockResolvedValue(false) + + await expect(command.execute()).rejects.toThrow(WorktreeValidationError) + + // rebaseOnMain should not be called if validation fails + expect(mockMergeManager.rebaseOnMain).not.toHaveBeenCalled() + }) + + it('handles deeply nested subdirectory within worktree', async () => { + const worktree = createMockWorktree({ path: '/test/worktree' }) + process.cwd = vi.fn().mockReturnValue('/test/worktree/src/lib/utils/deep/nested') + vi.mocked(isValidGitRepo).mockResolvedValue(true) + vi.mocked(getRepoRoot).mockResolvedValue('/test/worktree') + vi.mocked(mockGitWorktreeManager.listWorktrees).mockResolvedValue([worktree]) + vi.mocked(mockGitWorktreeManager.isMainWorktree).mockResolvedValue(false) + + await command.execute() + + expect(mockMergeManager.rebaseOnMain).toHaveBeenCalledWith('/test/worktree', { + dryRun: false, + force: false, + }) + }) + }) +}) diff --git a/src/commands/rebase.ts b/src/commands/rebase.ts new file mode 100644 index 0000000..95b29e9 --- /dev/null +++ b/src/commands/rebase.ts @@ -0,0 +1,127 @@ +import { logger } from '../utils/logger.js' +import { MergeManager } from '../lib/MergeManager.js' +import { GitWorktreeManager } from '../lib/GitWorktreeManager.js' +import { SettingsManager } from '../lib/SettingsManager.js' +import { isValidGitRepo, getRepoRoot } from '../utils/git.js' +import type { MergeOptions } from '../types/index.js' + +export interface RebaseOptions { + force?: boolean + dryRun?: boolean +} + +/** + * Error thrown when the rebase command is run from an invalid location + */ +export class WorktreeValidationError extends Error { + constructor( + message: string, + public readonly suggestion: string + ) { + super(message) + this.name = 'WorktreeValidationError' + } +} + +/** + * RebaseCommand: Rebase current branch on main with Claude-assisted conflict resolution + * + * This command: + * 1. Validates the current directory is an iloom-managed worktree + * 2. Detects the worktree root (supports running from subdirectories) + * 3. Delegates to MergeManager.rebaseOnMain() which handles: + * - Checking main branch exists + * - Detecting uncommitted changes (throws if found) + * - Checking if already up-to-date + * - Executing rebase + * - Claude-assisted conflict resolution + * 4. Reports success + */ +export class RebaseCommand { + private mergeManager: MergeManager + private gitWorktreeManager: GitWorktreeManager + private settingsManager: SettingsManager + + constructor(mergeManager?: MergeManager, gitWorktreeManager?: GitWorktreeManager, settingsManager?: SettingsManager) { + this.mergeManager = mergeManager ?? new MergeManager() + this.gitWorktreeManager = gitWorktreeManager ?? new GitWorktreeManager() + this.settingsManager = settingsManager ?? new SettingsManager() + } + + /** + * Validate that the current directory is within an iloom-managed worktree + * Returns the worktree root path if valid + * @throws WorktreeValidationError if validation fails + */ + private async validateWorktreeContext(): Promise { + const currentDir = process.cwd() + + // Step 1: Check if we're in a git repository at all + const isGitRepo = await isValidGitRepo(currentDir) + if (!isGitRepo) { + throw new WorktreeValidationError( + 'Not a git repository.', + "Run 'il rebase' from within an iloom worktree created by 'il start'." + ) + } + + // Step 2: Get the repository root (handles subdirectories) + const repoRoot = await getRepoRoot(currentDir) + if (!repoRoot) { + throw new WorktreeValidationError( + 'Could not determine repository root.', + "Run 'il rebase' from within an iloom worktree created by 'il start'." + ) + } + + // Step 3: Check if this path is a registered git worktree + const worktrees = await this.gitWorktreeManager.listWorktrees() + const currentWorktree = worktrees.find(wt => wt.path === repoRoot) + + if (!currentWorktree) { + throw new WorktreeValidationError( + 'This directory is not an iloom worktree.', + "Run 'il rebase' from within a worktree created by 'il start '. Use 'il list' to see available worktrees." + ) + } + + // Step 4: Check if this is the main worktree (we shouldn't rebase from main) + const isMain = await this.gitWorktreeManager.isMainWorktree(currentWorktree, this.settingsManager) + if (isMain) { + throw new WorktreeValidationError( + 'Cannot rebase from the main worktree.', + "Navigate to a feature worktree created by 'il start ' and run 'il rebase' from there." + ) + } + + return repoRoot + } + + async execute(options: RebaseOptions = {}): Promise { + // Step 1: Validate we're in a valid iloom worktree + let worktreePath: string + try { + worktreePath = await this.validateWorktreeContext() + } catch (error) { + if (error instanceof WorktreeValidationError) { + logger.error(error.message) + logger.info(error.suggestion) + throw error + } + throw error + } + + const mergeOptions: MergeOptions = { + dryRun: options.dryRun ?? false, + force: options.force ?? false, + } + + // MergeManager.rebaseOnMain() handles: + // - Checking main branch exists + // - Detecting uncommitted changes (throws if found) + // - Checking if already up-to-date + // - Executing rebase + // - Claude-assisted conflict resolution + await this.mergeManager.rebaseOnMain(worktreePath, mergeOptions) + } +} diff --git a/src/lib/GitWorktreeManager.test.ts b/src/lib/GitWorktreeManager.test.ts index 8d0bd56..ccbf1d0 100644 --- a/src/lib/GitWorktreeManager.test.ts +++ b/src/lib/GitWorktreeManager.test.ts @@ -4,6 +4,7 @@ import fs from 'fs-extra' import { GitWorktreeManager } from './GitWorktreeManager.js' import { MockFactories } from '../test-utils/mock-factories.js' import * as gitUtils from '../utils/git.js' +import type { SettingsManager } from './SettingsManager.js' // Mock the git utils module vi.mock('../utils/git.js', () => ({ @@ -17,6 +18,7 @@ vi.mock('../utils/git.js', () => ({ getRepoRoot: vi.fn(), hasUncommittedChanges: vi.fn(), getDefaultBranch: vi.fn(), + findMainWorktreePathWithSettings: vi.fn(), })) // Mock fs-extra @@ -31,10 +33,15 @@ vi.mock('fs-extra', () => ({ describe('GitWorktreeManager', () => { let manager: GitWorktreeManager + let mockSettingsManager: SettingsManager const mockRepoPath = '/test/repo' beforeEach(() => { manager = new GitWorktreeManager(mockRepoPath) + mockSettingsManager = { + loadSettings: vi.fn().mockResolvedValue({}), + getProtectedBranches: vi.fn().mockResolvedValue(['main', 'master', 'develop']), + } as unknown as SettingsManager MockFactories.resetAll() vi.clearAllMocks() }) @@ -1046,7 +1053,7 @@ describe('GitWorktreeManager', () => { }) describe('isMainWorktree', () => { - it('should identify the first worktree as the main worktree', async () => { + it('should return true when worktree path matches findMainWorktreePathWithSettings result', async () => { const mainWorktree = { path: '/test/repo', branch: 'main', @@ -1056,6 +1063,15 @@ describe('GitWorktreeManager', () => { locked: false, } + vi.mocked(gitUtils.findMainWorktreePathWithSettings).mockResolvedValue('/test/repo') + + const result = await manager.isMainWorktree(mainWorktree, mockSettingsManager) + + expect(result).toBe(true) + expect(gitUtils.findMainWorktreePathWithSettings).toHaveBeenCalledWith(mainWorktree.path, mockSettingsManager) + }) + + it('should return false when worktree path does not match main worktree path', async () => { const featureWorktree = { path: '/test/worktree-feature', branch: 'feature-branch', @@ -1065,17 +1081,15 @@ describe('GitWorktreeManager', () => { locked: false, } - const mockWorktrees = [mainWorktree, featureWorktree] + vi.mocked(gitUtils.findMainWorktreePathWithSettings).mockResolvedValue('/test/repo') - vi.mocked(gitUtils.executeGitCommand).mockResolvedValue('mock output') - vi.mocked(gitUtils.parseWorktreeList).mockReturnValue(mockWorktrees) - - const result = await manager.isMainWorktree(mainWorktree) + const result = await manager.isMainWorktree(featureWorktree, mockSettingsManager) - expect(result).toBe(true) + expect(result).toBe(false) + expect(gitUtils.findMainWorktreePathWithSettings).toHaveBeenCalledWith(featureWorktree.path, mockSettingsManager) }) - it('should NOT identify non-main worktrees (index > 0) as main', async () => { + it('should correctly identify main vs non-main worktrees', async () => { const mainWorktree = { path: '/test/repo', branch: 'main', @@ -1085,79 +1099,38 @@ describe('GitWorktreeManager', () => { locked: false, } - const featureWorktree = { - path: '/test/worktree-feature', - branch: 'feature-branch', + const featureWorktree1 = { + path: '/test/worktree-1', + branch: 'feature-1', commit: 'def456', bare: false, detached: false, locked: false, } - const bugfixWorktree = { - path: '/test/worktree-bugfix', - branch: 'bugfix-branch', + const featureWorktree2 = { + path: '/test/worktree-2', + branch: 'feature-2', commit: 'ghi789', bare: false, detached: false, locked: false, } - const mockWorktrees = [mainWorktree, featureWorktree, bugfixWorktree] - - vi.mocked(gitUtils.executeGitCommand).mockResolvedValue('mock output') - vi.mocked(gitUtils.parseWorktreeList).mockReturnValue(mockWorktrees) - - const result1 = await manager.isMainWorktree(featureWorktree) - const result2 = await manager.isMainWorktree(bugfixWorktree) - - expect(result1).toBe(false) - expect(result2).toBe(false) - }) - - it('should handle multiple worktrees correctly', async () => { - const worktrees = [ - { - path: '/test/repo', - branch: 'main', - commit: 'abc123', - bare: false, - detached: false, - locked: false, - }, - { - path: '/test/worktree-1', - branch: 'feature-1', - commit: 'def456', - bare: false, - detached: false, - locked: false, - }, - { - path: '/test/worktree-2', - branch: 'feature-2', - commit: 'ghi789', - bare: false, - detached: false, - locked: false, - }, - ] - - vi.mocked(gitUtils.executeGitCommand).mockResolvedValue('mock output') - vi.mocked(gitUtils.parseWorktreeList).mockReturnValue(worktrees) + // All calls return the main worktree path + vi.mocked(gitUtils.findMainWorktreePathWithSettings).mockResolvedValue('/test/repo') - // Only the first worktree should be main - const result0 = await manager.isMainWorktree(worktrees[0]) - const result1 = await manager.isMainWorktree(worktrees[1]) - const result2 = await manager.isMainWorktree(worktrees[2]) + const result0 = await manager.isMainWorktree(mainWorktree, mockSettingsManager) + const result1 = await manager.isMainWorktree(featureWorktree1, mockSettingsManager) + const result2 = await manager.isMainWorktree(featureWorktree2, mockSettingsManager) expect(result0).toBe(true) expect(result1).toBe(false) expect(result2).toBe(false) }) - it('should handle single worktree (main only) scenario', async () => { - const mainWorktree = { + it('should pass settingsManager to findMainWorktreePathWithSettings', async () => { + const worktree = { path: '/test/repo', branch: 'main', commit: 'abc123', @@ -1166,63 +1139,29 @@ describe('GitWorktreeManager', () => { locked: false, } - const mockWorktrees = [mainWorktree] - - vi.mocked(gitUtils.executeGitCommand).mockResolvedValue('mock output') - vi.mocked(gitUtils.parseWorktreeList).mockReturnValue(mockWorktrees) + vi.mocked(gitUtils.findMainWorktreePathWithSettings).mockResolvedValue('/test/repo') - const result = await manager.isMainWorktree(mainWorktree) + await manager.isMainWorktree(worktree, mockSettingsManager) - expect(result).toBe(true) + expect(gitUtils.findMainWorktreePathWithSettings).toHaveBeenCalledWith('/test/repo', mockSettingsManager) }) - it('should work correctly regardless of worktree path', async () => { - // Test that classification is based on order, not path matching - const worktrees = [ - { - path: '/custom/unusual/path', - branch: 'main', - commit: 'abc123', - bare: false, - detached: false, - locked: false, - }, - { - path: '/var/project/feature', - branch: 'feature-branch', - commit: 'def456', - bare: false, - detached: false, - locked: false, - }, - ] - - vi.mocked(gitUtils.executeGitCommand).mockResolvedValue('mock output') - vi.mocked(gitUtils.parseWorktreeList).mockReturnValue(worktrees) - - const result0 = await manager.isMainWorktree(worktrees[0]) - const result1 = await manager.isMainWorktree(worktrees[1]) - - expect(result0).toBe(true) - expect(result1).toBe(false) - }) - - it('should handle empty worktree list gracefully', async () => { - const someWorktree = { - path: '/test/worktree', - branch: 'feature-branch', + it('should use worktree path for settings lookup', async () => { + const worktree = { + path: '/custom/path/worktree', + branch: 'feature', commit: 'abc123', bare: false, detached: false, locked: false, } - vi.mocked(gitUtils.executeGitCommand).mockResolvedValue('') - vi.mocked(gitUtils.parseWorktreeList).mockReturnValue([]) + vi.mocked(gitUtils.findMainWorktreePathWithSettings).mockResolvedValue('/custom/path/main') - const result = await manager.isMainWorktree(someWorktree) + const result = await manager.isMainWorktree(worktree, mockSettingsManager) expect(result).toBe(false) + expect(gitUtils.findMainWorktreePathWithSettings).toHaveBeenCalledWith('/custom/path/worktree', mockSettingsManager) }) }) diff --git a/src/lib/GitWorktreeManager.ts b/src/lib/GitWorktreeManager.ts index 186fa17..f65d81c 100644 --- a/src/lib/GitWorktreeManager.ts +++ b/src/lib/GitWorktreeManager.ts @@ -19,7 +19,9 @@ import { getRepoRoot, hasUncommittedChanges, getDefaultBranch, + findMainWorktreePathWithSettings, } from '../utils/git.js' +import type { SettingsManager } from './SettingsManager.js' /** * Manages Git worktrees for the iloom CLI @@ -65,15 +67,15 @@ export class GitWorktreeManager { /** * Check if a worktree is the main repository worktree - * The main worktree is the first one listed by git worktree list (Git guarantee) - * This cannot be determined by path comparison because --show-toplevel returns - * the same value for all worktrees. + * Uses findMainWorktreePathWithSettings to determine the main worktree based on settings. + * + * @param worktree - The worktree to check + * @param settingsManager - SettingsManager instance for loading settings + * @returns true if the worktree is the main worktree */ - async isMainWorktree(worktree: GitWorktree): Promise { - const worktrees = await this.listWorktrees() - // The first worktree is always the main worktree (Git design) - const mainWorktree = worktrees[0] - return mainWorktree !== undefined && mainWorktree.path === worktree.path + async isMainWorktree(worktree: GitWorktree, settingsManager: SettingsManager): Promise { + const mainWorktreePath = await findMainWorktreePathWithSettings(worktree.path, settingsManager) + return worktree.path === mainWorktreePath } /** @@ -444,9 +446,14 @@ export class GitWorktreeManager { * Remove multiple worktrees * Returns a summary of successes and failures * Automatically filters out the main worktree + * + * @param worktrees - Array of worktrees to remove + * @param settingsManager - SettingsManager instance for determining main worktree + * @param options - Cleanup options */ async removeWorktrees( worktrees: GitWorktree[], + settingsManager: SettingsManager, options: WorktreeCleanupOptions = {} ): Promise<{ successes: Array<{ worktree: GitWorktree }> @@ -459,7 +466,7 @@ export class GitWorktreeManager { for (const worktree of worktrees) { // Skip main worktree - if (await this.isMainWorktree(worktree)) { + if (await this.isMainWorktree(worktree, settingsManager)) { skipped.push({ worktree, reason: 'Cannot remove main worktree' }) continue } diff --git a/src/lib/ResourceCleanup.test.ts b/src/lib/ResourceCleanup.test.ts index 674a5b1..c64f72f 100644 --- a/src/lib/ResourceCleanup.test.ts +++ b/src/lib/ResourceCleanup.test.ts @@ -46,6 +46,7 @@ describe('ResourceCleanup', () => { mockGitWorktree.findWorktreeForIssue = vi.fn() mockGitWorktree.findWorktreeForPR = vi.fn() mockGitWorktree.findWorktreeForBranch = vi.fn() + mockGitWorktree.isMainWorktree = vi.fn() vi.clearAllMocks() }) diff --git a/src/lib/ResourceCleanup.ts b/src/lib/ResourceCleanup.ts index 9ebc313..fd1b9a2 100644 --- a/src/lib/ResourceCleanup.ts +++ b/src/lib/ResourceCleanup.ts @@ -595,7 +595,7 @@ export class ResourceCleanup { const blockers: string[] = [] // Check if main worktree - const isMain = await this.gitWorktree.isMainWorktree(worktree) + const isMain = await this.gitWorktree.isMainWorktree(worktree, this.settingsManager) if (isMain) { blockers.push(`Cannot cleanup main worktree: "${worktree.branch}" @ "${worktree.path}"`) } diff --git a/tests/lib/ResourceCleanup-database-prefetch.test.ts b/tests/lib/ResourceCleanup-database-prefetch.test.ts index b16f582..63ba877 100644 --- a/tests/lib/ResourceCleanup-database-prefetch.test.ts +++ b/tests/lib/ResourceCleanup-database-prefetch.test.ts @@ -21,6 +21,7 @@ vi.mock('../../src/utils/logger.js', () => ({ vi.mock('../../src/utils/git.js', () => ({ executeGitCommand: vi.fn(), findMainWorktreePath: vi.fn().mockResolvedValue('/test/main'), + findMainWorktreePathWithSettings: vi.fn().mockResolvedValue('/test/main'), hasUncommittedChanges: vi.fn().mockResolvedValue(false), }))