When a loom branch has previously had merges from the parent branch, or if more than 20 commits would be rebased, a "rebase" should merge from the parent branch instead of rebasing

docs/iloom-commands.md

@@ -323,8 +323,8 @@ Behavior depends on the `mergeBehavior.mode` setting in your iloom configuration
 1. Detects uncommitted changes and auto-commits
 2. Runs validation pipeline: typecheck, lint, tests
 3. If failures occur, launches Claude to help fix issues
-4. Rebases on main branch
-5. Validates fast-forward merge is possible
+4. Synchronizes with main branch (rebase or merge -- see [Smart Rebase/Merge Strategy](#smart-rebasemerge-strategy))
+5. Validates fast-forward merge is possible (or uses `--no-ff` if step 4 used merge)
 6. Merges to main
 7. Installs dependencies in main
 8. Runs post-merge build verification
@@ -414,9 +414,51 @@ il rebase [options]
 **Workflow:**
 
 1. Fetches latest changes from main branch
-2. Attempts to rebase current branch
-3. If conflicts occur, launches Claude to help resolve
-4. Validates resolution and completes rebase
+2. Detects the optimal synchronization strategy (see [Smart Rebase/Merge Strategy](#smart-rebasemerge-strategy) below)
+3. Rebases or merges from the main branch depending on the detected strategy
+4. If conflicts occur, launches Claude to help resolve
+5. Validates resolution and completes the operation
+
+#### Smart Rebase/Merge Strategy
+
+iloom automatically detects when a standard `git rebase` would be problematic and falls back to `git merge` from the parent branch instead. This applies to both `il rebase` and the rebase step within `il finish`.
+
+A merge strategy is used when either condition is met:
+
+- **Merge commits from parent detected:** The branch contains merge commits that originated from the parent branch (e.g., from a previous `git merge main`). Rebasing a branch with such merge commits can produce duplicate commits and confusing history.
+- **Commit count exceeds threshold:** The branch has more commits ahead of the parent than the configured threshold (default: 20). Large rebases are slow, error-prone, and produce many individual conflict points.
+
+When neither condition is met, iloom uses the standard rebase strategy.
+
+The strategy is logged so you can see which approach was chosen:
+
+```
+Found merge commits from main on this branch -- using merge instead of rebase
+```
+
+or:
+
+```
+Branch has 47 commits ahead of main (threshold: 20) -- using merge instead of rebase
+```
+
+**Configuration:**
+
+The commit count threshold is controlled by the `rebase.maxCommitsForRebase` setting:
+
+```json
+{
+  "rebase": {
+    "maxCommitsForRebase": 20
+  }
+}
+```
+
+| Value | Behavior |
+|-------|----------|
+| Positive (e.g., `20`) | Use merge when the branch has this many or more commits ahead of the parent |
+| `0` | Always use merge (never rebase by commit count) |
+| Negative (e.g., `-1`) | Disable the commit count check entirely (merge-commit detection still applies) |
 
 **Examples:**
 
@@ -435,6 +477,7 @@ il rebase --dry-run
 - Useful when main branch has advanced and you need to sync
 - Claude provides context-aware conflict resolution assistance
 - Safe to run multiple times
+- When merge strategy is used during `il finish` local mode, the final merge to main uses `--no-ff` instead of `--ff-only` since the branch is no longer a clean fast-forward
 
 ---
 

src/commands/finish.pr-workflow.test.ts

@@ -119,7 +119,7 @@ describe('FinishCommand - PR State Detection', () => {
 		} as unknown as CommitManager
 
 		mockMergeManager = {
-			rebaseOnMain: vi.fn().mockResolvedValue(undefined),
+			rebaseOnMain: vi.fn().mockResolvedValue({ strategy: 'rebase', conflictsDetected: false, claudeLaunched: false, conflictsResolved: false }),
 			performFastForwardMerge: vi.fn().mockResolvedValue(undefined),
 		} as unknown as MergeManager
 
@@ -287,7 +287,7 @@ describe('FinishCommand - Open PR Workflow', () => {
 		} as unknown as CommitManager
 
 		mockMergeManager = {
-			rebaseOnMain: vi.fn().mockResolvedValue(undefined),
+			rebaseOnMain: vi.fn().mockResolvedValue({ strategy: 'rebase', conflictsDetected: false, claudeLaunched: false, conflictsResolved: false }),
 			performFastForwardMerge: vi.fn().mockResolvedValue(undefined),
 		} as unknown as MergeManager
 
@@ -502,7 +502,7 @@ describe('FinishCommand - Child Loom GitHub PR Workflow', () => {
 		} as unknown as CommitManager
 
 		mockMergeManager = {
-			rebaseOnMain: vi.fn().mockResolvedValue(undefined),
+			rebaseOnMain: vi.fn().mockResolvedValue({ strategy: 'rebase', conflictsDetected: false, claudeLaunched: false, conflictsResolved: false }),
 			performFastForwardMerge: vi.fn().mockResolvedValue(undefined),
 		} as unknown as MergeManager
 
@@ -626,7 +626,7 @@ describe('FinishCommand - Closed PR Workflow', () => {
 		} as unknown as CommitManager
 
 		mockMergeManager = {
-			rebaseOnMain: vi.fn().mockResolvedValue(undefined),
+			rebaseOnMain: vi.fn().mockResolvedValue({ strategy: 'rebase', conflictsDetected: false, claudeLaunched: false, conflictsResolved: false }),
 			performFastForwardMerge: vi.fn().mockResolvedValue(undefined),
 		} as unknown as MergeManager
 
@@ -834,7 +834,7 @@ describe('FinishCommand - Merged PR Workflow', () => {
 		} as unknown as CommitManager
 
 		mockMergeManager = {
-			rebaseOnMain: vi.fn().mockResolvedValue(undefined),
+			rebaseOnMain: vi.fn().mockResolvedValue({ strategy: 'rebase', conflictsDetected: false, claudeLaunched: false, conflictsResolved: false }),
 			performFastForwardMerge: vi.fn().mockResolvedValue(undefined),
 		} as unknown as MergeManager
 
@@ -921,7 +921,7 @@ describe('FinishCommand - Dry-Run Mode for PRs', () => {
 		} as unknown as CommitManager
 
 		mockMergeManager = {
-			rebaseOnMain: vi.fn().mockResolvedValue(undefined),
+			rebaseOnMain: vi.fn().mockResolvedValue({ strategy: 'rebase', conflictsDetected: false, claudeLaunched: false, conflictsResolved: false }),
 			performFastForwardMerge: vi.fn().mockResolvedValue(undefined),
 		} as unknown as MergeManager
 
@@ -1084,7 +1084,7 @@ describe('FinishCommand - Issue Workflow (Regression Tests)', () => {
 		} as unknown as CommitManager
 
 		mockMergeManager = {
-			rebaseOnMain: vi.fn().mockResolvedValue(undefined),
+			rebaseOnMain: vi.fn().mockResolvedValue({ strategy: 'rebase', conflictsDetected: false, claudeLaunched: false, conflictsResolved: false }),
 			performFastForwardMerge: vi.fn().mockResolvedValue(undefined),
 		} as unknown as MergeManager
 
@@ -1200,7 +1200,7 @@ describe('FinishCommand - Linear issue tracker with GitHub PRs', () => {
 		} as unknown as CommitManager
 
 		mockMergeManager = {
-			rebaseOnMain: vi.fn().mockResolvedValue(undefined),
+			rebaseOnMain: vi.fn().mockResolvedValue({ strategy: 'rebase', conflictsDetected: false, claudeLaunched: false, conflictsResolved: false }),
 			performFastForwardMerge: vi.fn().mockResolvedValue(undefined),
 		} as unknown as MergeManager
 

src/commands/finish.test.ts

@@ -186,7 +186,12 @@ describe('FinishCommand', () => {
 		vi.mocked(mockCommitManager.commitChanges).mockResolvedValue(undefined)
 
 		// Mock MergeManager methods to succeed by default
-		vi.mocked(mockMergeManager.rebaseOnMain).mockResolvedValue(undefined)
+		vi.mocked(mockMergeManager.rebaseOnMain).mockResolvedValue({
+			conflictsDetected: false,
+			claudeLaunched: false,
+			conflictsResolved: false,
+			strategy: 'rebase',
+		})
 		vi.mocked(mockMergeManager.performFastForwardMerge).mockResolvedValue(undefined)
 
 		// Mock ResourceCleanup.cleanupWorktree to succeed by default
@@ -486,6 +491,7 @@ describe('FinishCommand', () => {
 
 				vi.mocked(mockMergeManager.rebaseOnMain).mockImplementation(async () => {
 					executionOrder.push('rebase')
+					return { conflictsDetected: false, claudeLaunched: false, conflictsResolved: false, strategy: 'rebase' as const }
 				})
 
 				vi.mocked(installDependencies).mockImplementationOnce(async () => {
@@ -642,6 +648,50 @@ describe('FinishCommand', () => {
 				)
 			})
 
+			describe('smart rebase/merge strategy in local merge', () => {
+				it('should pass noFf: true to performFastForwardMerge when rebaseOnMain used merge strategy', async () => {
+					vi.mocked(mockMergeManager.rebaseOnMain).mockResolvedValue({
+						conflictsDetected: false,
+						claudeLaunched: false,
+						conflictsResolved: false,
+						strategy: 'merge',
+					})
+
+					await command.execute({
+						identifier: '123',
+						options: {},
+					})
+
+					// performFastForwardMerge should receive noFf: true when strategy was 'merge'
+					expect(mockMergeManager.performFastForwardMerge).toHaveBeenCalledWith(
+						mockWorktree.branch,
+						mockWorktree.path,
+						expect.objectContaining({ noFf: true })
+					)
+				})
+
+				it('should not pass noFf when rebaseOnMain used rebase strategy', async () => {
+					vi.mocked(mockMergeManager.rebaseOnMain).mockResolvedValue({
+						conflictsDetected: false,
+						claudeLaunched: false,
+						conflictsResolved: false,
+						strategy: 'rebase',
+					})
+
+					await command.execute({
+						identifier: '123',
+						options: {},
+					})
+
+					// performFastForwardMerge should NOT have noFf when strategy was 'rebase'
+					expect(mockMergeManager.performFastForwardMerge).toHaveBeenCalledWith(
+						mockWorktree.branch,
+						mockWorktree.path,
+						expect.not.objectContaining({ noFf: true })
+					)
+				})
+			})
+
 			it('should pass jsonStream through mergeOptions to rebaseOnMain', async () => {
 				await command.execute({
 					identifier: '123',

src/commands/finish.ts

@@ -26,7 +26,7 @@ import { createNeonProviderFromSettings } from '../utils/neon-helpers.js'
 import { getConfiguredRepoFromSettings, hasMultipleRemotes } from '../utils/remote.js'
 import { promptConfirmation } from '../utils/prompt.js'
 import { UserAbortedCommitError, type FinishResult } from '../types/index.js'
-import type { FinishOptions, GitWorktree, CommitOptions, MergeOptions, PullRequest } from '../types/index.js'
+import type { FinishOptions, GitWorktree, CommitOptions, MergeOptions, PullRequest, RebaseOutcome } from '../types/index.js'
 import type { ResourceCleanupOptions, CleanupResult } from '../types/cleanup.js'
 import type { ParsedInput } from './start.js'
 import { TelemetryService } from '../lib/TelemetryService.js'
@@ -734,19 +734,26 @@ export class FinishCommand {
 			}
 		}
 
+		// Track rebase outcome across the method — when rebaseOnMain falls back to merge strategy,
+		// the local-mode merge step needs to use --no-ff instead of --ff-only.
+		// undefined when skipToPr is set (rebase is skipped entirely).
+		let rebaseOutcome: RebaseOutcome | undefined
+
 		// Skip rebase/validation/commit steps if --skip-to-pr flag is set (debug mode)
 		if (options.skipToPr) {
 			getLogger().info('Skipping rebase/validation/commit (--skip-to-pr flag)')
 		} else {
 			// Step 1: Rebase branch on main FIRST (Issue #344)
 			// This ensures validation runs against the rebased code (with latest main changes)
-			getLogger().info('Rebasing branch on main...')
+			getLogger().info('Synchronizing with parent branch...')
 
-			await this.mergeManager.rebaseOnMain(worktree.path, mergeOptions)
-			getLogger().success('Branch rebased successfully')
+			rebaseOutcome = await this.mergeManager.rebaseOnMain(worktree.path, mergeOptions)
+			getLogger().success(rebaseOutcome?.strategy === 'merge' ? 'Branch synchronized via merge' : 'Branch rebased successfully')
 			result.operations.push({
 				type: 'rebase',
-				message: 'Branch rebased on main',
+				message: rebaseOutcome.strategy === 'merge'
+					? `Branch synchronized with ${rebaseOutcome.targetBranch} via merge`
+					: `Branch rebased on ${rebaseOutcome.targetBranch}`,
 				success: true,
 			})
 
@@ -996,13 +1003,18 @@ export class FinishCommand {
 			return
 		}
 
-		// Step 6: Perform fast-forward merge
-		getLogger().info('Performing fast-forward merge...')
-		await this.mergeManager.performFastForwardMerge(worktree.branch, worktree.path, mergeOptions)
-		getLogger().success('Fast-forward merge completed successfully')
+		// Step 6: Perform merge into main
+		// When rebaseOnMain used merge strategy (instead of rebase), the branch can't fast-forward.
+		// Pass noFf: true so performFastForwardMerge uses --no-ff instead of --ff-only.
+		const localMergeOptions: MergeOptions = rebaseOutcome?.strategy === 'merge'
+			? { ...mergeOptions, noFf: true }
+			: mergeOptions
+		getLogger().info(localMergeOptions.noFf ? 'Performing no-ff merge...' : 'Performing fast-forward merge...')
+		await this.mergeManager.performFastForwardMerge(worktree.branch, worktree.path, localMergeOptions)
+		getLogger().success(localMergeOptions.noFf ? 'No-ff merge completed successfully' : 'Fast-forward merge completed successfully')
 		result.operations.push({
 			type: 'merge',
-			message: 'Fast-forward merge completed',
+			message: localMergeOptions.noFf ? 'No-ff merge completed' : 'Fast-forward merge completed',
 			success: true,
 		})
 

src/commands/ignite.test.ts

@@ -132,7 +132,7 @@ describe('IgniteCommand', () => {
 					expect.objectContaining({
 						headless: false,
 						addDir: '/path/to/feat/issue-70__description',
-						model: 'opus',
+						model: expect.any(String),
 						permissionMode: 'acceptEdits',
 					})
 				)
@@ -208,7 +208,7 @@ describe('IgniteCommand', () => {
 					expect.objectContaining({
 						headless: false,
 						addDir: '/path/to/some-worktree',
-						model: 'opus',
+						model: expect.any(String),
 						permissionMode: 'acceptEdits',
 					})
 				)
@@ -345,7 +345,7 @@ describe('IgniteCommand', () => {
 					expect.any(String),
 					expect.objectContaining({
 						headless: false,
-						model: 'opus',
+						model: expect.any(String),
 						permissionMode: 'acceptEdits',
 					})
 				)
@@ -805,7 +805,7 @@ describe('IgniteCommand', () => {
 					expect.objectContaining({
 						headless: false,
 						addDir: '/path/to/feat/issue-50__terminal-test',
-						model: 'opus', // issue workflow model (default from spin.model)
+						model: expect.any(String), // issue workflow model (from settings)
 						permissionMode: 'acceptEdits', // issue workflow permission mode
 					})
 				)
@@ -927,7 +927,7 @@ describe('IgniteCommand', () => {
 					'Guide the user through the iloom workflow!', // User prompt
 					expect.objectContaining({
 						headless: false,
-						model: 'opus',
+						model: expect.any(String),
 						permissionMode: 'acceptEdits',
 						appendSystemPromptFile: '/path/to/feat/issue-82__test/.claude/iloom-system-prompt.md',
 					})