Skip to content

Commit cb15541

Browse files
committed
```
Implement merge target branch resolution for child looms. Fixes #328 - Remove parentBranchName parameter from copyIloomSettings (now handled via getMergeTargetBranch) - Update MergeManager to find worktree for merge target branch instead of always finding main worktree - Add fallback to findMainWorktreePathWithSettings when merge target branch is not checked out - Inject MetadataManager into MergeManager for parent loom metadata access - Use getMergeTargetBranch utility to resolve merge target (parent > settings.mainBranch > main) - Pre-fetch merge target branch in ResourceCleanup before worktree deletion (before metadata becomes unavailable) - Add comprehensive tests for child loom merge scenarios and merge target resolution priorities ```
1 parent 75c5e4a commit cb15541

File tree

8 files changed

+850
-88
lines changed

8 files changed

+850
-88
lines changed

src/lib/LoomManager.ts

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,7 @@ export class LoomManager {
111111
await this.copyEnvironmentFiles(worktreePath)
112112

113113
// 7. Copy Loom settings (settings.local.json) - ALWAYS done regardless of capabilities
114-
// Pass parent branch name if this is a child loom
115-
await this.copyIloomSettings(worktreePath, input.parentLoom?.branchName)
114+
await this.copyIloomSettings(worktreePath)
116115

117116
// 7.5. Copy Claude settings (.claude/settings.local.json) - ALWAYS done regardless of capabilities
118117
await this.copyClaudeSettings(worktreePath)
@@ -634,9 +633,8 @@ export class LoomManager {
634633
* Copy iloom configuration (settings.local.json) from main repo to worktree
635634
* Always called regardless of project capabilities
636635
* @param worktreePath Path to the worktree
637-
* @param parentBranchName Optional parent branch name for child looms (sets mainBranch)
638636
*/
639-
private async copyIloomSettings(worktreePath: string, parentBranchName?: string): Promise<void> {
637+
private async copyIloomSettings(worktreePath: string): Promise<void> {
640638
const mainSettingsLocalPath = path.join(process.cwd(), '.iloom', 'settings.local.json')
641639

642640
try {
@@ -653,26 +651,6 @@ export class LoomManager {
653651
} else {
654652
await this.environment.copyIfExists(mainSettingsLocalPath, worktreeSettingsLocalPath)
655653
}
656-
657-
// If this is a child loom, update mainBranch setting
658-
if (parentBranchName) {
659-
let existingSettings = {}
660-
661-
try {
662-
const content = await fs.readFile(worktreeSettingsLocalPath, 'utf8')
663-
existingSettings = JSON.parse(content)
664-
} catch {
665-
// File doesn't exist or invalid, start fresh
666-
}
667-
668-
const updatedSettings = {
669-
...existingSettings,
670-
mainBranch: parentBranchName,
671-
}
672-
673-
await fs.writeFile(worktreeSettingsLocalPath, JSON.stringify(updatedSettings, null, 2))
674-
getLogger().info(`Set mainBranch to ${parentBranchName} for child loom`)
675-
}
676654
} catch (error) {
677655
getLogger().warn(`Warning: Failed to copy settings.local.json: ${error instanceof Error ? error.message : 'Unknown error'}`)
678656
}

src/lib/MergeManager.test.ts

Lines changed: 197 additions & 35 deletions
Large diffs are not rendered by default.

src/lib/MergeManager.ts

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import { executeGitCommand, findMainWorktreePathWithSettings } from '../utils/git.js'
1+
import { executeGitCommand, findMainWorktreePathWithSettings, findWorktreeForBranch, getMergeTargetBranch } from '../utils/git.js'
22
import { getLogger } from '../utils/logger-context.js'
33
import { detectClaudeCli, launchClaude } from '../utils/claude.js'
44
import { SettingsManager } from './SettingsManager.js'
5+
import { MetadataManager } from './MetadataManager.js'
56
import type { MergeOptions } from '../types/index.js'
67

78
/**
@@ -12,19 +13,25 @@ import type { MergeOptions } from '../types/index.js'
1213
*/
1314
export class MergeManager {
1415
private settingsManager: SettingsManager
16+
private metadataManager: MetadataManager
1517

16-
constructor(settingsManager?: SettingsManager) {
18+
constructor(settingsManager?: SettingsManager, metadataManager?: MetadataManager) {
1719
this.settingsManager = settingsManager ?? new SettingsManager()
20+
this.metadataManager = metadataManager ?? new MetadataManager()
1821
}
1922

2023
/**
21-
* Get the main branch name from settings (defaults to 'main')
22-
* @param worktreePath - Optional path to load settings from (defaults to process.cwd())
24+
* Get the merge target branch for a loom
25+
* Priority: parent loom metadata > configured main branch > 'main'
26+
* @param worktreePath - Optional path to load settings/metadata from (defaults to process.cwd())
2327
* @private
2428
*/
2529
private async getMainBranch(worktreePath?: string): Promise<string> {
26-
const settings = await this.settingsManager.loadSettings(worktreePath)
27-
return settings.mainBranch ?? 'main'
30+
// Delegate to shared utility function
31+
return getMergeTargetBranch(worktreePath ?? process.cwd(), {
32+
settingsManager: this.settingsManager,
33+
metadataManager: this.metadataManager,
34+
})
2835
}
2936

3037
/**
@@ -217,19 +224,33 @@ export class MergeManager {
217224

218225
getLogger().info('Starting fast-forward merge...')
219226

220-
// Step 1: Find where main branch is checked out
221-
// This copies the bash script approach: find main worktree, run commands from there
222-
const mainWorktreePath = options.repoRoot ??
223-
await findMainWorktreePathWithSettings(worktreePath, this.settingsManager)
224-
225-
// Load mainBranch from settings in the worktree being finished (not cwd)
226-
// This ensures we use the correct settings when finishing a child loom from parent
227+
// Step 1: Get the merge target branch FIRST
228+
// For child looms, this will be the parent branch from metadata
229+
// For regular looms, this falls back to settings.mainBranch or 'main'
227230
const mainBranch = await this.getMainBranch(worktreePath)
228231

229-
// Step 3: No need to checkout main - it's already checked out in mainWorktreePath
232+
// Step 2: Find where the merge target branch is checked out
233+
// CRITICAL: We must find the worktree for the MERGE TARGET, not settings.mainBranch
234+
// This fixes the child loom bug where we'd find the 'main' worktree instead of the parent branch worktree
235+
let mainWorktreePath: string
236+
if (options.repoRoot) {
237+
mainWorktreePath = options.repoRoot
238+
} else {
239+
try {
240+
// First try to find worktree with the exact merge target branch checked out
241+
mainWorktreePath = await findWorktreeForBranch(mainBranch, worktreePath)
242+
} catch {
243+
// Fallback: if no worktree has the branch checked out, use settings-based lookup
244+
// This handles edge cases like bare repos or detached HEAD states
245+
getLogger().debug(`No worktree found for branch '${mainBranch}', falling back to settings-based lookup`)
246+
mainWorktreePath = await findMainWorktreePathWithSettings(worktreePath, this.settingsManager)
247+
}
248+
}
249+
250+
// Step 3: No need to checkout - the merge target branch is already checked out in mainWorktreePath
230251
getLogger().debug(`Using ${mainBranch} branch location: ${mainWorktreePath}`)
231252

232-
// Step 4: Verify on main branch
253+
// Step 4: Verify we're on the correct branch
233254
const currentBranch = await executeGitCommand(['branch', '--show-current'], {
234255
cwd: mainWorktreePath,
235256
})

0 commit comments

Comments
 (0)