Skip to content

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#1017

Merged
acreeger merged 2 commits into
mainfrom
feat/issue-1016__smart-rebase-merge
May 21, 2026

Conversation

@acreeger

Copy link
Copy Markdown
Collaborator

Fixes #1016

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


This PR was created automatically by iloom.

@acreeger

acreeger commented May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Enhancement Analysis — Issue #1016

  • Fetch issue details
  • Read project documentation (CLAUDE.md, README.md)
  • Analyze requirements and identify gaps
  • Structure enhanced specification
  • Post final analysis

Enhancement Analysis

Questions for Reporter

Question Answer
Should the 20-commit threshold be a configurable setting (e.g., rebase.maxCommits), or is 20 always the right cutoff? @acreeger Assumed: yes, it should be configurable with 20 as the default, since different teams have different tolerances for rebase complexity.
Does this apply to all rebase surfaces -- il rebase, il finish, and swarm orchestrator (child-to-epic merge) -- or only a subset? Confirmed by reporter: il rebase and il finish (all modes: local, PR, draft-PR) only. Swarm orchestrator is excluded — it's more controlled.
When the system falls back to merge-from-parent instead of rebase, should il finish in local mode still attempt a fast-forward merge to the parent, or should it switch to a regular merge commit? Currently local mode expects linear history after rebase. Assumed: when merge-from-parent is used, the final merge to the parent branch should also use a merge commit (non-fast-forward), since a fast-forward is impossible after introducing merge commits.
Should the user be informed when the system chooses merge over rebase (e.g., a log message explaining why), or should it be silent? Assumed: the system should log a clear message explaining which strategy was chosen and why (e.g., "Detected merge commits from parent branch; using merge instead of rebase").
For the merge-commit detection, does "previously had merges from the parent branch" mean specifically merge commits that brought parent branch changes into the loom branch (i.e., git merge main while on the feature branch), or any merge commit at all? Assumed: specifically merge commits that brought parent-branch changes into the loom branch, not unrelated merges (e.g., merges from other feature branches).

Problem Summary

When a loom branch either (a) already contains merge commits from the parent branch or (b) has accumulated a large number of commits (more than 20), the current il rebase behavior can fail or produce complex conflicts. The system should detect these conditions and automatically fall back to merging from the parent branch instead of rebasing.

User Impact

Users working on long-lived branches or branches where they have previously merged from main encounter painful rebase conflicts, duplicated history, or outright rebase failures during il rebase and il finish workflows.

Enhancement Goal

Automatically detect when a rebase would be problematic (merge history present, or commit count exceeds threshold) and seamlessly fall back to a merge-from-parent strategy, preserving a safe and predictable branch synchronization experience without requiring manual intervention.

Next Steps

  • Reporter to confirm or adjust answers to the questions above
  • Technical analysis to identify all rebase call sites and determine detection logic
  • Implementation planning and execution

Complete Context and Details (click to expand)

Current Behavior

iloom performs rebases in two contexts relevant to this issue:

  1. il rebase -- Standalone command to sync a loom branch with its parent (main or epic branch). Always performs git rebase.
  2. il finish (all modes: local, PR, draft-PR) -- Rebase step that syncs the loom branch with the parent before finishing. In local mode, this is followed by a fast-forward merge to the parent.

Out of Scope: Swarm orchestrator child-to-epic merge flow is excluded — it's more controlled and doesn't need this fallback.

Scenarios Where Rebase Is Problematic

  1. Prior merge commits from parent: If a user has run git merge main on their feature branch (or iloom itself merged parent changes earlier), rebasing replays those merge commits, potentially creating duplicate changes and complex conflicts.
  2. High commit count (>20): Rebasing many commits increases the chance of repeated conflicts at each commit boundary, making the process slow and error-prone. A single merge commit resolves all differences at once.

Affected Surfaces

  • il rebase command
  • il finish (the rebase step in local, PR, and draft-PR modes)

Relationship to Existing Configuration

The mergeBehavior setting currently controls what happens after rebase (local merge vs PR vs draft-PR). This enhancement adds a new decision point before that: whether to rebase at all, or merge from parent instead. This is orthogonal to mergeBehavior.mode.

History Model Implications

When merge-from-parent is chosen instead of rebase, the branch will contain merge commits. This means:

  • Fast-forward merge to the parent branch is no longer possible
  • The final merge (in il finish local mode) would need to use --no-ff or equivalent
  • Teams that enforce linear history (e.g., GitHub branch protection with "Require linear history") would not be able to use local mode with merge-from-parent -- they would need to use PR mode instead, where the platform handles squash/merge strategy

@acreeger

acreeger commented May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Deep Technical Analysis — Issue #1016

  • Map all rebase code paths (commands, lib, swarm orchestrator)
  • Trace GitProvider rebase implementation and git operations
  • Analyze il finish merge flow and fast-forward requirements
  • Analyze swarm orchestrator merge/rebase flow
  • Identify detection logic needed (merge commit detection, commit counting)
  • Map cross-cutting data flow for rebase strategy decision
  • Document all affected interfaces and types
  • Post final analysis with findings

Status: Mapped all rebase call sites. Now analyzing merge flows and detection logic requirements.
ETA: ~5 minutes for complete analysis

@acreeger

acreeger commented May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Deep Technical Analysis -- Issue #1016 (Smart Rebase/Merge)

  • Review existing PR comments for prior questions/answers
  • Map all rebase code paths (commands, lib, swarm orchestrator prompts)
  • Trace GitProvider rebase implementation and git operations
  • Analyze il finish merge flow and fast-forward requirements
  • Swarm orchestrator DESCOPED per user constraint
  • Identify detection logic needed (merge commit detection, commit counting)
  • Map cross-cutting data flow for rebase strategy decision
  • Document all affected interfaces, types, and files
  • Post final analysis with complete findings

Executive Summary

MergeManager.rebaseOnMain() is the single entry point for all rebase operations, called from both il rebase and il finish (all modes: local, PR, draft-PR). The method already counts commits before rebasing (line 133-143), making the threshold check straightforward to add. The critical complication is that il finish in local mode uses --ff-only merge after the rebase -- if the rebase step is replaced by merge-from-parent, the fast-forward merge will fail because merge commits make fast-forward impossible. This requires the strategy decision to propagate back to the caller so il finish can switch to --no-ff merge in local mode.

Scope (per user): Applies to il rebase and all il finish rebase paths (local, PR, draft-PR). Does NOT apply to swarm orchestrator child-to-epic merges. Default commit threshold: 20.

Questions and Key Decisions

Question Answer
Should the commit threshold (default 20) be configurable via settings? Assumed yes, via a new rebase.maxCommitsForRebase setting. The first PR comment suggested this and it aligns with the settings system pattern.
When merge-from-parent is used in local il finish, should the final merge use --no-ff automatically, or should local mode be blocked entirely? Assumed: use --no-ff automatically. Blocking local mode would be overly restrictive. The user likely wants a working workflow regardless of strategy.
Should the merge-commit detection look only for merges from the parent branch specifically, or any merge commit in history? Assumed: any merge commit in the range targetBranch..HEAD. Detecting specifically "from parent" would require complex ancestry analysis and is fragile. Any merge commit makes rebase problematic.

HIGH/CRITICAL Risks

  • Local mode fast-forward failure: performFastForwardMerge at src/lib/MergeManager.ts:261 uses git merge --ff-only. When rebase is replaced by merge-from-parent, this will throw "not a fast-forward". The finish workflow must detect the strategy used and switch to --no-ff merge for local mode.
  • RebaseOutcome type change is cross-cutting: Adding a strategy field to RebaseOutcome (returned by rebaseOnMain) affects both callers (rebase.ts and finish.ts). The finish.ts caller must read this to adjust downstream merge behavior.

Impact Summary

  • 2 files require significant modification: src/lib/MergeManager.ts (detection logic + merge-from-parent execution), src/commands/finish.ts (react to strategy in local mode)
  • 1 type definition to extend: RebaseOutcome in src/types/index.ts
  • 1 settings schema to extend: SettingsManager.ts (add rebase.maxCommitsForRebase)
  • 1 command file with minor changes: src/commands/rebase.ts (log strategy used)
  • Test files: MergeManager.test.ts, rebase.test.ts, finish.test.ts
  • Documentation: docs/iloom-commands.md

Complete Technical Reference (click to expand for implementation details)

Problem Space Research

Problem Understanding

When a loom branch has merge commits from the parent branch (e.g., user ran git merge main manually) or has accumulated many commits, git rebase becomes problematic: it replays merge commits causing duplicate changes and cascading conflicts, or it forces the user through repeated conflict resolution for each commit. A merge-from-parent (git merge targetBranch) resolves all differences in one operation.

Architectural Context

MergeManager is the service responsible for all rebase and merge operations. It is called from two command entry points. The strategy decision (rebase vs merge) must be made inside MergeManager.rebaseOnMain() since that method already handles the pre-checks, commit counting, and git operations. The result must be communicated back to the caller via RebaseOutcome so downstream operations can adapt.

Edge Cases Identified

  • No commits to rebase but main moved forward: Branch has 0 new commits but target has advanced. Currently handled at line 143-147. Merge-from-parent would also work here but is unnecessary (a simple fast-forward rebase is fine).
  • WIP commit interaction: rebaseOnMain creates a WIP commit at line 103-108 for uncommitted changes before rebasing. If strategy is merge-from-parent, the WIP commit still needs to be created and restored afterwards.
  • Dry-run mode: The dry-run path at line 157-163 must also report which strategy would be used.
  • Teams with linear history enforcement: GitHub branch protection with "Require linear history" will reject branches containing merge commits. For these teams, PR/draft-PR mode is the correct path (GitHub handles squash).

Codebase Research Findings

Affected Area: MergeManager.rebaseOnMain()

Entry Point: src/lib/MergeManager.ts:45 - The rebaseOnMain method
Dependencies:

  • Uses: executeGitCommand, fetchOrigin, getMergeTargetBranch, MetadataManager.readMetadata, SettingsManager.loadSettings
  • Used By: src/commands/rebase.ts:134, src/commands/finish.ts:694

Key internal flow (lines 45-213):

  1. Abort in-progress rebase (line 49)
  2. Determine target branch and remote/local mode (lines 51-74)
  3. Verify branch exists (lines 78-96)
  4. Create WIP commit if uncommitted changes (lines 99-108)
  5. Check if rebase needed via merge-base comparison (lines 110-130)
  6. Count commits via git log --oneline targetBranch..HEAD (lines 133-143) -- this is where commit count is already available
  7. Execute git rebase (lines 168-169) -- this is the decision point
  8. Handle conflicts with Claude assistance (lines 177-213)

Affected Area: FinishCommand.executeIssueWorkflow()

Entry Point: src/commands/finish.ts:673
Critical flow:

  • Line 694: Calls rebaseOnMain() -- ALL modes pass through here
  • Lines 802-813: Merge mode branching (local vs PR vs draft-PR) happens AFTER rebase
  • Line 950: performFastForwardMerge() -- only local mode, uses --ff-only

Affected Area: performFastForwardMerge()

Entry Point: src/lib/MergeManager.ts:261

  • Line 347: git merge --ff-only branchName -- will FAIL if branch has merge commits
  • This method is ONLY called for local mergeBehavior.mode (not PR or draft-PR)
  • Must be extended to support --no-ff when the rebase step used merge-from-parent

Detection Logic Required

Two conditions trigger merge-from-parent instead of rebase:

1. Merge commit detection (any merge commits in branch range):

git log --merges --oneline {targetBranch}..HEAD

If output is non-empty, merge commits exist. This is a simple git command.

2. Commit count threshold (default: 20):
Already computed at MergeManager.ts:138 via commitLines.length. Just needs a comparison against the configured threshold.

Architectural Flow Analysis

Data Flow: rebase strategy decision

Entry Point: src/lib/MergeManager.ts:45 - rebaseOnMain() determines strategy internally
Flow Path:

  1. src/lib/MergeManager.ts:133-143 - Counts commits (already exists)
  2. src/lib/MergeManager.ts:~145 (NEW) - Detects merge commits via git log --merges
  3. src/lib/MergeManager.ts:~150 (NEW) - Loads threshold from settings
  4. src/lib/MergeManager.ts:~155 (NEW) - Decision: rebase if (no merge commits AND commitCount <= threshold), else merge
  5. src/lib/MergeManager.ts:168-169 OR ~170 (NEW) - Execute rebase OR git merge targetBranch
  6. src/types/index.ts:304 - RebaseOutcome returns strategy used to caller

Consumption Points:

  • src/commands/rebase.ts:134 - Logs strategy, adjusts user messaging
  • src/commands/finish.ts:694 - Critical: reads strategy to decide ff-merge vs no-ff merge at line 950

Data Flow: rebase threshold configuration

Entry Point: src/lib/SettingsManager.ts:759-769 (git settings block) or new rebase block
Flow Path:

  1. User sets rebase.maxCommitsForRebase: 20 in settings
  2. SettingsManager.loadSettings() validates and returns
  3. MergeManager.rebaseOnMain() reads via this.settingsManager.loadSettings(worktreePath)
  4. Compared against commitLines.length at decision point

Affected Interfaces (ALL must be updated):

  • RebaseOutcome at src/types/index.ts:304 - Add strategy: 'rebase' | 'merge' field
  • IloomSettings Zod schema in src/lib/SettingsManager.ts:759 - Add rebase config block (or extend git block)
  • MergeOptions at src/types/index.ts:398 - No change needed (strategy is decided internally)

Critical Implementation Note: This is a cross-cutting change affecting the return type of rebaseOnMain(). Both callers must handle the new strategy field. The finish.ts caller must branch on it for local mode.

Affected Files

  • src/lib/MergeManager.ts:45-213 - Core logic: add merge-commit detection, commit threshold check, merge-from-parent execution path, and extend RebaseOutcome return values
  • src/lib/MergeManager.ts:261-359 - performFastForwardMerge: needs --no-ff fallback path when strategy was merge
  • src/types/index.ts:304-308 - RebaseOutcome interface: add strategy field
  • src/lib/SettingsManager.ts:759-769 - Settings schema: add rebase configuration (e.g., under rebase or git block)
  • src/lib/SettingsManager.ts:995-1019 - Override schema: matching partial schema
  • src/commands/finish.ts:694-700 - Read strategy from RebaseOutcome, pass to merge step
  • src/commands/finish.ts:948-956 - Local mode: switch to --no-ff when strategy was merge
  • src/commands/rebase.ts:134-163 - Log which strategy was used
  • src/commands/rebase.ts:8 - Import updated types
  • docs/iloom-commands.md:396-431 - Document new behavior and configuration
  • src/lib/MergeManager.test.ts - Add tests for detection logic and merge fallback
  • src/commands/rebase.test.ts - Add tests for strategy-aware output
  • src/commands/finish.test.ts - Add tests for no-ff merge path

Integration Points

  • MergeManager.rebaseOnMain() returns RebaseOutcome -> consumed by RebaseCommand.execute() and FinishCommand.executeIssueWorkflow()
  • FinishCommand.executeIssueWorkflow() calls MergeManager.performFastForwardMerge() for local mode only (line 950)
  • SettingsManager.loadSettings() provides configuration -> consumed by MergeManager for threshold
  • RebaseOutcome.strategy field bridges the decision in MergeManager to the merge behavior in FinishCommand

Medium Severity Risks

  • PR mode force-push unchanged: In PR/draft-PR modes, finish.ts uses --force-with-lease after rebase (line 905). When merge-from-parent is used instead, the branch history won't have been rewritten, so --force-with-lease is still safe but a regular push would suffice. No action required but worth noting.
  • Claude conflict resolution path: The Claude-assisted conflict resolution (MergeManager:460-543) is only triggered on rebase conflicts. With merge-from-parent, conflicts would need different resolution (merge conflicts vs rebase conflicts). The existing detectConflictedFiles method works for both, but the prompt at line 476 references "git rebase conflicts" specifically.
  • Settings migration: Adding a new rebase settings block doesn't require a data migration since it's optional with defaults, but needs to be added to BOTH the full schema and the override schema.

@acreeger acreeger changed the title When a loom branch has previously had merges from the parent branch, or if more than 10 commits would be rebased, a "rebase" should merge from the parent branch instead of rebasing 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 May 21, 2026
@acreeger

acreeger commented May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Implementation Plan -- Issue #1016

  • Fetch issue and PR details, review all prior analysis
  • Read core files: MergeManager.ts, finish.ts, rebase.ts, types/index.ts, SettingsManager.ts
  • Identify exact line numbers for all changes
  • Check for deduplication opportunities
  • Draft implementation plan with execution phases
  • Post final plan

Status: Drafting the full implementation plan now.
ETA: ~3 minutes

@acreeger

acreeger commented May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Implementation Plan for Issue #1016

Summary

When a loom branch contains merge commits from the parent branch, or has accumulated more commits than a configurable threshold (default: 20), MergeManager.rebaseOnMain() should automatically fall back to merging from the parent branch (git merge targetBranch) instead of rebasing. This applies to both il rebase and all il finish modes (local, PR, draft-PR), and the strategy decision must propagate to il finish's local-mode merge step so it uses --no-ff instead of --ff-only.

Questions and Key Decisions

Question Answer Rationale
Where does the strategy decision live? Inside MergeManager.rebaseOnMain() between commit counting (line 138) and rebase execution (line 168) Single choke point for all callers
How is the setting structured? New rebase.maxCommitsForRebase in both Zod schemas Semantically distinct from git block; follows existing pattern of domain-specific settings
What does maxCommitsForRebase=0 mean? Always use merge (never rebase by commit count) 0 means "rebase zero commits before switching" = always merge
What does maxCommitsForRebase=-1 mean? Disable commit-count check entirely Negative = no threshold, merge-commit detection still applies independently
How does merge-commit detection work? For each merge in git log --merges targetBranch..HEAD, check if any non-first-parent is an ancestor of targetBranch via git merge-base --is-ancestor Avoids false positives from merges of other feature branches
How does local-mode finish handle merge strategy? When RebaseOutcome.strategy === 'merge', pass noFf: true to performFastForwardMerge which skips FF validation and uses --no-ff Reuses all existing worktree-finding, branch-verification, and commit-listing logic
What about Claude conflict resolution for merge conflicts? attemptClaudeConflictResolution gets a conflictType parameter that adjusts prompts and allowed tools Merge conflicts use git commit to finalize, not git rebase --continue

High-Level Execution Phases

  1. Types and Settings: Extend RebaseOutcome type and add rebase settings block to both Zod schemas, add noFf to MergeOptions
  2. Core Logic: Add strategy detection (merge-commit detection + threshold check) and merge-from-parent execution path in MergeManager.rebaseOnMain(); adapt performFastForwardMerge for --no-ff; adapt Claude conflict resolution for merge conflicts
  3. Command Integration: Update finish.ts to capture RebaseOutcome and pass noFf option for local-mode merge; update rebase.ts to log strategy used
  4. Tests: Add tests for detection logic, merge fallback, no-ff merge path, and strategy-aware output
  5. Documentation: Update docs/iloom-commands.md with new behavior and configuration

Quick Stats

  • 0 files for deletion
  • 6 files to modify (MergeManager.ts, finish.ts, rebase.ts, types/index.ts, SettingsManager.ts, docs/iloom-commands.md)
  • 0 new files to create
  • 3 test files to modify (MergeManager.test.ts, finish.test.ts, rebase.test.ts)
  • Dependencies: None
  • Estimated complexity: Medium

Potential Risks (HIGH/CRITICAL only)

  • Local mode fast-forward failure: performFastForwardMerge at src/lib/MergeManager.ts:261 uses git merge --ff-only. When rebase is replaced by merge-from-parent, this will throw "not a fast-forward." The noFf option in MergeOptions addresses this by switching to --no-ff.
  • RebaseOutcome type change is cross-cutting: Adding strategy to RebaseOutcome affects the return type of rebaseOnMain(). Both callers (rebase.ts line 134 and finish.ts line 694) currently handle RebaseOutcome -- finish.ts discards it entirely. Must update both.

Complete Implementation Guide (click to expand for step-by-step details)

Automated Test Cases to Create

Test File: src/lib/MergeManager.test.ts (MODIFY)

Purpose: Test strategy detection logic, merge-from-parent execution, and no-ff merge path.

Click to expand complete test structure (55 lines)
describe('Smart Rebase/Merge Strategy', () => {
  describe('Merge commit detection', () => {
    it('should detect merge commits from parent branch and use merge strategy')
    it('should NOT trigger merge strategy for merge commits from other feature branches')
    it('should use rebase when no merge commits from parent exist')
  })

  describe('Commit count threshold', () => {
    it('should use merge when commit count >= threshold (default 20)')
    it('should use rebase when commit count < threshold')
    it('should use merge when maxCommitsForRebase is 0 (always merge)')
    it('should ignore commit count when maxCommitsForRebase is negative')
    it('should respect custom threshold from settings')
  })

  describe('Strategy execution', () => {
    it('should execute git merge targetBranch when strategy is merge')
    it('should execute git rebase targetBranch when strategy is rebase')
    it('should return strategy field in RebaseOutcome')
    it('should log explanatory message when using merge (merge commits reason)')
    it('should log explanatory message when using merge (threshold reason)')
    it('should handle WIP commit correctly with merge strategy')
    it('should report strategy in dry-run mode')
  })

  describe('Merge conflict resolution', () => {
    it('should use merge-specific Claude prompt when strategy is merge')
    it('should check MERGE_HEAD (not rebase state) after merge conflict resolution')
  })
})

describe('No-FF Merge in performFastForwardMerge', () => {
  it('should use --no-ff when noFf option is true')
  it('should skip FF validation when noFf is true')
  it('should still use --ff-only when noFf is false/undefined')
})

Test File: src/commands/finish.test.ts (MODIFY)

Purpose: Test that finish captures rebase strategy and passes noFf for local-mode merge.

Click to expand complete test structure (25 lines)
describe('Smart Rebase/Merge Strategy in Finish', () => {
  it('should pass noFf: true to performFastForwardMerge when rebase used merge strategy', async () => {
    // Mock rebaseOnMain to return { strategy: 'merge', ... }
    // Verify performFastForwardMerge called with noFf: true in mergeOptions
  })

  it('should not pass noFf when rebase used rebase strategy', async () => {
    // Mock rebaseOnMain to return { strategy: 'rebase', ... }
    // Verify performFastForwardMerge called without noFf
  })

  it('should not affect PR mode when merge strategy is used', async () => {
    // Mock rebaseOnMain to return { strategy: 'merge', ... }
    // Verify PR workflow proceeds normally
  })

  it('should log merge strategy in rebase result operation', async () => {
    // Mock rebaseOnMain to return { strategy: 'merge', ... }
    // Verify result.operations contains strategy info
  })
})

Test File: src/commands/rebase.test.ts (MODIFY)

Purpose: Test that rebase command logs strategy used and includes it in jsonStream result.

describe('Smart Rebase/Merge Strategy in Rebase Command', () => {
  it('should include strategy in jsonStream result')
  it('should return strategy field when merge was used')
})

Files to Modify

1. src/types/index.ts:304-308

Change: Extend RebaseOutcome interface with strategy field.

export interface RebaseOutcome {
  conflictsDetected: boolean
  claudeLaunched: boolean
  conflictsResolved: boolean
  strategy: 'rebase' | 'merge'
}

2. src/types/index.ts:398-403

Change: Add noFf option to MergeOptions interface.

export interface MergeOptions {
  dryRun?: boolean
  force?: boolean
  repoRoot?: string
  jsonStream?: boolean
  noFf?: boolean  // Use --no-ff instead of --ff-only for local merge
}

3. src/lib/SettingsManager.ts:759-769 (full schema)

Change: Add rebase settings block after the git block.

Click to expand settings schema addition (12 lines)
// After the git block (line 769), add:
rebase: z
  .object({
    maxCommitsForRebase: z
      .number()
      .int('maxCommitsForRebase must be an integer')
      .default(20)
      .describe(
        'Maximum number of commits before switching from rebase to merge-from-parent. ' +
        'Set to 0 to always use merge. Set to a negative value (e.g., -1) to disable the commit count check.'
      ),
  })
  .default({})
  .describe('Rebase behavior settings'),

4. src/lib/SettingsManager.ts:1062-1073 (no-defaults/override schema)

Change: Add matching rebase block with optional fields (no defaults).

Click to expand override schema addition (11 lines)
// After the git block (line 1073), add:
rebase: z
  .object({
    maxCommitsForRebase: z
      .number()
      .int('maxCommitsForRebase must be an integer')
      .optional()
      .describe(
        'Maximum number of commits before switching from rebase to merge-from-parent. ' +
        'Set to 0 to always use merge. Set to a negative value (e.g., -1) to disable the commit count check.'
      ),
  })
  .optional()
  .describe('Rebase behavior settings'),

5. src/lib/MergeManager.ts:45-213 - rebaseOnMain() method

Change: Add strategy detection and merge-from-parent execution path. This is the largest change.

Key modifications inside rebaseOnMain():

5a. After commit counting (line 143), add merge-commit detection:

  • Run git log --merges --format=%H targetBranch..HEAD to get merge commit hashes
  • For each merge hash, get its parents via git rev-parse <hash>^2 (second parent)
  • Check if that parent is an ancestor of targetBranch via git merge-base --is-ancestor <parent> targetBranch
  • If any match, the branch has merges from the parent -> strategy = 'merge'
Click to expand detection pseudocode (25 lines)
// After line 143, add strategy detection:
// --- Detect merge commits from parent branch ---
const mergesOutput = await executeGitCommand(
  ['log', '--merges', '--format=%H', `${targetBranch}..HEAD`],
  { cwd: worktreePath }
)
const mergeHashes = mergesOutput.trim().split('\n').filter(h => h.length > 0)

let hasMergesFromParent = false
for (const mergeHash of mergeHashes) {
  // Get all parents of this merge commit
  const parentsOutput = await executeGitCommand(
    ['rev-parse', `${mergeHash}^@`], // ^@ = all parents
    { cwd: worktreePath }
  )
  const parents = parentsOutput.trim().split('\n')
  // Check non-first parents (index 1+) for parent-branch ancestry
  for (const parent of parents.slice(1)) {
    try {
      await executeGitCommand(
        ['merge-base', '--is-ancestor', parent.trim(), targetBranch],
        { cwd: worktreePath }
      )
      hasMergesFromParent = true
      break
    } catch {
      // Not an ancestor, continue
    }
  }
  if (hasMergesFromParent) break
}

// --- Determine strategy ---
const maxCommits = settings.rebase?.maxCommitsForRebase ?? 20
const commitCountExceedsThreshold = maxCommits === 0
  || (maxCommits > 0 && commitLines.length >= maxCommits)

let strategy: 'rebase' | 'merge' = 'rebase'
let strategyReason = ''

if (hasMergesFromParent) {
  strategy = 'merge'
  strategyReason = `Found merge commits from ${targetBranch} on this branch`
} else if (commitCountExceedsThreshold) {
  strategy = 'merge'
  strategyReason = maxCommits === 0
    ? 'rebase.maxCommitsForRebase is set to 0 (always merge)'
    : `Branch has ${commitLines.length} commits ahead of ${targetBranch} (threshold: ${maxCommits})`
}

if (strategy === 'merge') {
  getLogger().info(`${strategyReason} — using merge instead of rebase`)
}

5b. Modify the dry-run path (line 157-163) to report strategy:

  • Log which strategy would be used

5c. Replace the rebase execution block (lines 165-212) with strategy-branching:

  • If strategy === 'rebase': existing rebase logic (unchanged)
  • If strategy === 'merge': execute git merge targetBranch with conflict handling
Click to expand merge execution pseudocode (30 lines)
if (strategy === 'merge') {
  // Merge from parent branch
  try {
    await executeGitCommand(['merge', targetBranch], { cwd: worktreePath })
    getLogger().success('Merge from parent branch completed successfully!')
    if (wipCommitHash) {
      await this.restoreWipCommit(worktreePath, wipCommitHash)
    }
    return { conflictsDetected: false, claudeLaunched: false, conflictsResolved: false, strategy: 'merge' }
  } catch (error) {
    const conflictedFiles = await this.detectConflictedFiles(worktreePath)
    if (conflictedFiles.length > 0) {
      getLogger().info('Merge conflicts detected, attempting Claude-assisted resolution...')
      const resolved = await this.attemptClaudeConflictResolution(
        worktreePath, conflictedFiles, { jsonStream, conflictType: 'merge' }
      )
      if (resolved) {
        getLogger().success('Conflicts resolved with Claude assistance, merge completed')
        if (wipCommitHash) {
          await this.restoreWipCommit(worktreePath, wipCommitHash)
        }
        return { conflictsDetected: true, claudeLaunched: true, conflictsResolved: true, strategy: 'merge' }
      }
      // Claude couldn't resolve - abort merge and fail
      await executeGitCommand(['merge', '--abort'], { cwd: worktreePath }).catch(() => {})
      const conflictError = this.formatConflictError(conflictedFiles)
      throw new Error(conflictError)
    }
    throw new Error(
      `Merge failed: ${error instanceof Error ? error.message : String(error)}\n` +
      'Run: git status for more details\n' +
      'Or: git merge --abort to cancel the merge'
    )
  }
}
// else: existing rebase logic (add strategy: 'rebase' to all return values)

5d. Update all existing return statements in the rebase path to include strategy: 'rebase':

  • Line 129: return { ..., strategy: 'rebase' } (already up to date case)
  • Line 162: return { ..., strategy } (dry-run -- uses detected strategy)
  • Line 176: return { ..., strategy: 'rebase' } (successful rebase)
  • Line 198: return { ..., strategy: 'rebase' } (conflict resolved)

6. src/lib/MergeManager.ts:460-543 - attemptClaudeConflictResolution()

Change: Add conflictType parameter to adapt prompts and tools.

  • Add conflictType: 'rebase' | 'merge' to the options parameter
  • When conflictType === 'merge':
    • System prompt says "git merge conflicts" and instructs "git add . and git commit" instead of "git rebase --continue"
    • Allowed tools include Bash(git commit:*) and Bash(git merge:*) instead of Bash(git rebase:*)
  • After Claude finishes, when conflictType === 'merge', check for MERGE_HEAD existence instead of isRebaseInProgress()
Click to expand conflict resolution changes (20 lines)
// Change method signature:
private async attemptClaudeConflictResolution(
  worktreePath: string,
  conflictedFiles: string[],
  options: { jsonStream?: boolean; conflictType?: 'rebase' | 'merge' } = {}
): Promise<boolean> {
  const conflictType = options.conflictType ?? 'rebase'

  // Adapt prompt based on conflict type
  const systemPrompt = conflictType === 'merge'
    ? 'Please help resolve the git merge conflicts...' // use 'git add .' then 'git commit'
    : 'Please help resolve the git rebase conflicts...' // existing prompt

  // Adapt allowed tools
  const allowedTools = conflictType === 'merge'
    ? [...baseTools, 'Bash(git commit:*)', 'Bash(git merge:*)']
    : [...baseTools, 'Bash(git rebase:*)', 'Bash(GIT_EDITOR=true git rebase:*)']

  // After Claude finishes, check appropriate state:
  if (conflictType === 'merge') {
    const mergeInProgress = await this.isMergeInProgress(worktreePath)
    if (mergeInProgress) { return false }
  } else {
    const rebaseInProgress = await this.isRebaseInProgress(worktreePath)
    if (rebaseInProgress) { return false }
  }
}

7. src/lib/MergeManager.ts - Add isMergeInProgress() helper

Change: Add a new private method to check for in-progress merge (check for MERGE_HEAD file in git dir).

// Pattern identical to isRebaseInProgress but checks for MERGE_HEAD
private async isMergeInProgress(worktreePath: string): Promise<boolean> {
  // resolve gitDir, check for MERGE_HEAD file
}

8. src/lib/MergeManager.ts:261-359 - performFastForwardMerge()

Change: Support noFf option from MergeOptions.

  • Line 266: Extract noFf from options: const { dryRun = false, force = false, noFf = false } = options
  • Line 310: Conditionally skip FF validation: if (!noFf) { await this.validateFastForwardPossible(...) }
  • Line 339: Adjust dry-run message: git merge --no-ff vs git merge --ff-only
  • Line 347: Use ['merge', '--no-ff', branchName] when noFf is true, else ['merge', '--ff-only', branchName]
  • Update log messages to indicate merge type

9. src/commands/finish.ts:694

Change: Capture RebaseOutcome and pass strategy to local merge.

At line 694, change:

// Before:
await this.mergeManager.rebaseOnMain(worktree.path, mergeOptions)
// After:
const rebaseOutcome = await this.mergeManager.rebaseOnMain(worktree.path, mergeOptions)

At line 696-700, update the operation message to include strategy:

result.operations.push({
  type: 'rebase',
  message: rebaseOutcome.strategy === 'merge'
    ? 'Branch synchronized with main via merge'
    : 'Branch rebased on main',
  success: true,
})

At line 948-950, pass noFf based on strategy:

// Before:
await this.mergeManager.performFastForwardMerge(worktree.branch, worktree.path, mergeOptions)
// After:
const localMergeOptions = rebaseOutcome?.strategy === 'merge'
  ? { ...mergeOptions, noFf: true }
  : mergeOptions
await this.mergeManager.performFastForwardMerge(worktree.branch, worktree.path, localMergeOptions)

Note: The rebaseOutcome variable must be declared in the outer scope of the method (before the if (options.skipToPr) block) so it's accessible at the merge step. When skipToPr is true, rebaseOutcome remains undefined, so the ?.strategy optional chain handles that.

10. src/commands/rebase.ts:134-163

Change: Log strategy used and include in jsonStream result.

At line 134, the return value is already captured as outcome. Add logging after:

const outcome = await this.mergeManager.rebaseOnMain(worktreePath, mergeOptions)

// Log strategy used (if not already logged by MergeManager)
if (outcome.strategy === 'merge') {
  logger.info('Synchronized with parent branch via merge (instead of rebase)')
}

At line 156-162, include strategy in jsonStream result:

return {
  success: true,
  conflictsDetected: outcome.conflictsDetected,
  claudeLaunched: outcome.claudeLaunched,
  conflictsResolved: outcome.conflictsResolved,
  strategy: outcome.strategy,  // NEW
}

Also update RebaseResult type at src/types/index.ts:296-302 to include strategy:

export interface RebaseResult {
  success: boolean
  conflictsDetected: boolean
  claudeLaunched: boolean
  conflictsResolved?: boolean
  error?: string
  strategy?: 'rebase' | 'merge'
}

11. docs/iloom-commands.md:396-431 (il rebase section)

Change: Document smart rebase/merge behavior.

Update the Workflow section to mention automatic strategy detection:

  • Explain merge-commit detection and commit threshold
  • Document rebase.maxCommitsForRebase setting
  • Add example showing the setting in config

12. docs/iloom-commands.md:320-330 (il finish local mode section)

Change: Update local mode steps to mention that step 4 may use merge instead of rebase, and step 6 adapts accordingly.

Detailed Execution Order

Phase 1 (parallel): Types and Settings

Step 1a: Extend types

Files: src/types/index.ts
Contract: RebaseOutcome gains strategy: 'rebase' | 'merge'; MergeOptions gains noFf?: boolean; RebaseResult gains strategy?: 'rebase' | 'merge'

  1. Add strategy field to RebaseOutcome at line 304 -> Verify: TypeScript compile passes
  2. Add noFf field to MergeOptions at line 398 -> Verify: TypeScript compile passes
  3. Add strategy field to RebaseResult at line 296 -> Verify: TypeScript compile passes

Step 1b: Add settings schema

Files: src/lib/SettingsManager.ts
Contract: settings.rebase.maxCommitsForRebase available as number (default 20) on IloomSettings type

  1. Add rebase block to IloomSettingsSchema after line 769 -> Verify: existing tests pass
  2. Add matching rebase block to IloomSettingsSchemaNoDefaults after line 1073 -> Verify: settings merge works

Phase 2 (sequential after Phase 1): Core MergeManager Logic

Step 2: Implement strategy detection and merge-from-parent in MergeManager

Files: src/lib/MergeManager.ts
Depends on: Step 1a (RebaseOutcome type), Step 1b (settings schema)

  1. Add isMergeInProgress() private method (mirrors isRebaseInProgress) -> Verify: method exists
  2. Add hasMergesFromParent() private method for merge-commit detection -> Verify: method exists
  3. In rebaseOnMain(), after line 143 add strategy detection (merge-commit check + threshold check) -> Verify: strategy variable determined
  4. Update dry-run path (line 157-163) to report strategy -> Verify: dry-run logs strategy
  5. Add merge-from-parent execution branch before existing rebase block -> Verify: merge path works
  6. Update all existing return statements to include strategy field -> Verify: TypeScript compiles
  7. Update attemptClaudeConflictResolution() to accept and use conflictType parameter -> Verify: prompts differ
  8. In performFastForwardMerge(), add noFf support: skip FF validation, use --no-ff flag -> Verify: no-ff merge works

Phase 3 (parallel): Command Integration + Documentation

Step 3a: Update finish.ts

Files: src/commands/finish.ts
Contract: Uses RebaseOutcome.strategy from rebaseOnMain() return value

  1. At line 694, capture rebaseOutcome return value -> Verify: variable available
  2. Declare rebaseOutcome in outer scope so it survives through to line 950 -> Verify: accessible at merge step
  3. Update operation message at line 696-700 to reflect strategy -> Verify: correct message
  4. At line 948-950, pass noFf: true when rebaseOutcome?.strategy === 'merge' -> Verify: correct merge type

Step 3b: Update rebase.ts

Files: src/commands/rebase.ts
Contract: Uses RebaseOutcome.strategy from outcome variable (already captured at line 134)

  1. Add strategy logging after line 134 -> Verify: log output correct
  2. Add strategy to jsonStream result at line 156-162 -> Verify: result includes strategy

Step 3c: Update documentation

Files: docs/iloom-commands.md

  1. Update il rebase section (lines 396-431) with smart strategy detection docs -> Verify: docs accurate
  2. Update il finish local mode section (lines 320-330) -> Verify: docs accurate
  3. Add rebase.maxCommitsForRebase to configuration reference -> Verify: setting documented

Phase 4 (parallel): Tests

Step 4a: MergeManager tests

Files: src/lib/MergeManager.test.ts

  1. Add 'Smart Rebase/Merge Strategy' describe block with merge-commit detection tests -> Verify: tests pass
  2. Add commit count threshold tests (including 0 and negative values) -> Verify: tests pass
  3. Add strategy execution tests (merge path, return value) -> Verify: tests pass
  4. Add no-ff merge tests for performFastForwardMerge -> Verify: tests pass
  5. Add merge conflict resolution tests (different prompt, MERGE_HEAD check) -> Verify: tests pass
  6. Update existing rebase tests to include strategy: 'rebase' in expected return values -> Verify: existing tests pass

Step 4b: finish.ts and rebase.ts tests

Files: src/commands/finish.test.ts, src/commands/rebase.test.ts

  1. Update default mock for rebaseOnMain to return { ..., strategy: 'rebase' } at finish.test.ts line 189 -> Verify: existing tests pass
  2. Add tests for noFf passthrough in local mode -> Verify: tests pass
  3. Add tests for strategy in rebase command jsonStream result -> Verify: tests pass

Execution Plan

  1. Run Steps 1a, 1b in parallel (types + settings schemas -- different files, shared contract: RebaseOutcome has strategy field, MergeOptions has noFf field, settings.rebase.maxCommitsForRebase exists)
  2. Run Step 2 sequentially (core MergeManager logic -- depends on types and settings from Phase 1)
  3. Run Steps 3a, 3b, 3c in parallel (finish.ts, rebase.ts, docs -- different files, all depend on Phase 2 completing)
  4. Run Steps 4a, 4b in parallel (tests -- different files, depend on Phase 3 completing)

Dependencies and Configuration

None -- no new packages required. The rebase.maxCommitsForRebase setting uses existing Zod schema infrastructure and SettingsManager layering.

@acreeger

acreeger commented May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Implementation Complete

Summary

Implemented smart rebase/merge strategy detection for il rebase and all il finish modes (local, PR, draft-PR). When a loom branch has merge commits from the parent branch or exceeds a configurable commit count threshold (default 20), the system automatically falls back to merge-from-parent instead of rebasing.

Changes Made

  • src/types/index.ts: Added strategy field to RebaseOutcome, noFf field to MergeOptions
  • src/types/telemetry.ts: Added rebase.strategy_selected telemetry event
  • src/lib/SettingsManager.ts: Added rebase.maxCommitsForRebase setting (default 20)
  • src/lib/MergeManager.ts: Core strategy detection, merge-from-parent execution, --no-ff merge support, Claude conflict resolution for merges
  • src/commands/finish.ts: Strategy propagation — passes noFf: true to local merge when merge strategy was used
  • src/commands/rebase.ts: Strategy reporting in jsonStream output and user-facing logs
  • docs/iloom-commands.md: Documentation for smart rebase/merge behavior and new setting

Validation Results

  • ✅ Tests: 5226 passed / 5227 total (1 pre-existing skip)
  • ✅ Typecheck: Passed
  • ✅ Lint: Passed
  • ✅ Build: Passed
  • ✅ CI: Passed

Detailed Changes by File (click to expand)

src/lib/MergeManager.ts

Changes: Core smart rebase/merge logic

  • Added hasMergesFromParent() — precise parent-ancestry check for merge commits
  • Added isMergeInProgress() — checks for MERGE_HEAD file
  • Added strategy detection in rebaseOnMain() with two triggers: merge commits from parent, commit count threshold
  • Added merge-from-parent execution path with conflict handling
  • Updated attemptClaudeConflictResolution() to support merge-type conflicts
  • Updated performFastForwardMerge() to support --no-ff mode
  • Added telemetry for strategy selection
  • User-facing logging explains why merge was chosen over rebase

src/lib/SettingsManager.ts

Changes: New rebase.maxCommitsForRebase setting

  • Added to full schema (default: 20) and override schema (optional)
  • Semantics: positive = threshold, 0 = always merge, negative = disable count check

src/commands/finish.ts

Changes: Strategy-aware local merge

  • Captures RebaseOutcome from rebaseOnMain()
  • Passes noFf: true to performFastForwardMerge() when merge strategy was used
  • Updated log messages to reflect merge type

src/commands/rebase.ts

Changes: Strategy reporting

  • Includes strategy field in jsonStream result
  • Logs merge strategy when used

src/lib/MergeManager.test.ts

Changes: 19 new tests + fixture updates

  • Smart Rebase/Merge Strategy: merge commit detection, commit count threshold, 0/negative values, strategy execution, user-facing logging
  • No-FF Merge in performFastForwardMerge: --no-ff flag, FF validation skip, dry-run

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

Changes: Mock updates and new tests

  • Updated all rebaseOnMain mocks to return RebaseOutcome with strategy field
  • Added tests for noFf propagation in finish and strategy in rebase jsonStream output

@acreeger acreeger force-pushed the feat/issue-1016__smart-rebase-merge branch from 769a97e to c219bdb Compare May 21, 2026 22:47
@acreeger

acreeger commented May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Implementation Complete - Issue #1016

Summary

Fixed three bugs in the smart rebase/merge feature: reordered strategy detection before the "already up to date" early-return check, replaced all hardcoded "rebase" and "main" messages with strategy-aware and target-branch-aware text, and added targetBranch to the RebaseOutcome interface so callers can reference the actual target branch.

Changes Made

  • src/types/index.ts: Added targetBranch: string to RebaseOutcome interface
  • src/lib/MergeManager.ts: Reordered steps so strategy detection (commit count + merge-commit checks) runs before the "already up to date" check; made early-return messages strategy-aware; added targetBranch to all return paths; fixed starting message
  • src/commands/finish.ts: Replaced hardcoded "Rebasing branch on main..." with generic pre-call message; made success message strategy-aware; used rebaseOutcome.targetBranch instead of hardcoded "main" in result operations
  • src/commands/rebase.ts: Made post-build success/failure messages strategy-aware by passing strategy to runPostRebaseBuild()
  • src/lib/MergeManager.test.ts: Updated 9 tests to add mocks for the reordered git commands (git log --oneline, git log --merges) and added targetBranch to expected RebaseOutcome assertions

Validation Results

  • ✅ Tests: 5226 passed / 5227 total (1 skipped)
  • ✅ Typecheck: Passed
  • ✅ Lint: Passed

📋 Detailed Changes by File (click to expand)

Files Modified

src/types/index.ts

Changes: Added targetBranch: string field to RebaseOutcome interface

src/lib/MergeManager.ts

Changes: Reordered rebaseOnMain() to compute strategy before the "already up to date" check

  • Moved git log --oneline (commit enumeration) and hasMergesFromParent() detection BEFORE the merge-base comparison
  • Made "already up to date" message strategy-aware ("Already synchronized with..." for merge, "Branch is already up to date..." for rebase)
  • Made commit listing and "moved forward" messages strategy-aware
  • Changed starting message from Starting rebase on ${targetBranch} to Starting synchronization with ${targetBranch}
  • Added targetBranch to all 6 return paths in rebaseOnMain()

src/commands/finish.ts

Changes: Removed hardcoded "rebase" and "main" references

  • Line 697: 'Rebasing branch on main...''Synchronizing with parent branch...'
  • Line 700: Success message uses rebaseOutcome.strategy to select appropriate text
  • Lines 704-705: Result operation message uses rebaseOutcome.targetBranch instead of hardcoded "main"

src/commands/rebase.ts

Changes: Made post-build messages strategy-aware

  • Added strategy parameter to runPostRebaseBuild() method
  • Success message: Post-${strategy} build completed successfully
  • Failure message: Post-${strategy} build failed: ...

src/lib/MergeManager.test.ts

Changes: Updated 9 failing tests

  • 4 "already up to date" tests: Added mocks for git log --oneline and git log --merges that now execute before the early-return check
  • 5 RebaseOutcome assertion tests: Added targetBranch: 'origin/main' to expected objects

Dependencies Added

None

acreeger added 2 commits May 21, 2026 19:18
When a loom branch has merge commits from the parent branch or exceeds
a configurable commit count threshold (default 20), automatically fall
back to merge-from-parent instead of rebasing. Applies to `il rebase`
and all `il finish` modes (local, PR, draft-PR).

- Detect merge commits from parent via precise ancestry check
- Add `rebase.maxCommitsForRebase` setting (0=always merge, <0=disable)
- Propagate strategy through RebaseOutcome so finish uses --no-ff merge
- Add telemetry, tests (19 new MergeManager + 4 command tests), and docs
- Fix pre-existing ignite test failures (model string was env-dependent)
…messaging

Strategy detection (merge-commit detection + commit count threshold) now
runs before the "already up to date" early return so branches with prior
merges from parent are correctly handled. All user-facing messages are
now strategy-aware and use the actual target branch name instead of
hardcoding "main". Added targetBranch to RebaseOutcome for callers.
@acreeger acreeger marked this pull request as ready for review May 21, 2026 23:19
@acreeger acreeger force-pushed the feat/issue-1016__smart-rebase-merge branch from afefee6 to c40c18d Compare May 21, 2026 23:19
@acreeger

Copy link
Copy Markdown
Collaborator Author

iloom Session Summary

Key Themes:

  • MergeManager.rebaseOnMain() is the single choke point for all rebase operations — any change to rebase behavior needs to happen here and propagate via RebaseOutcome
  • The "already up to date" early return uses merge-base semantics, which is true after a git merge even though the branch was never rebased — strategy detection must run before this check
  • Rebase and merge conflicts require different Claude resolution prompts, tool permissions, and git completion commands

Session Details (click to expand)

Key Insights

  • MergeManager.rebaseOnMain() (src/lib/MergeManager.ts) is the only entry point for all rebase operations in the codebase. Both il rebase and il finish (all modes: local, PR, draft-PR) call this method. Changes here affect all surfaces. Swarm orchestrator has a separate path and was explicitly excluded from this feature.
  • The "already up to date" early return at line 111 uses git merge-base --is-ancestor, which returns true when the target's commits are present in the branch via a prior git merge. This is semantically correct for merge strategy but incorrect for rebase strategy — the branch was never linearized. Strategy detection must run before this check, not after.
  • performFastForwardMerge uses --ff-only, which hard-fails when the branch contains merge commits from the parent. When merge-from-parent strategy is used, the caller (finish.ts) must pass noFf: true to switch to --no-ff. The strategy field on RebaseOutcome is the bridge.
  • Claude conflict resolution for merge conflicts requires different behavior than for rebase conflicts: the completion step is git commit (not git rebase --continue), and the allowed tool should be Bash(git commit:*) instead of Bash(git rebase:*). The attemptClaudeConflictResolution method takes a conflictType: 'rebase' | 'merge' parameter to handle this.
  • IloomSettings type is fully inferred from the Zod schema via z.infer<typeof IloomSettingsSchema>. Adding a new settings block to the Zod schema propagates the type automatically — no manual interface updates needed.

Decisions Made

  • Scope: Smart rebase-merge applies to il rebase and all il finish modes (local, PR, draft-PR). Swarm orchestrator child-to-epic merge is excluded — it's more controlled.
  • Threshold semantics for rebase.maxCommitsForRebase: positive value = threshold (merge if count ≥ value), 0 = always merge, negative = disable commit-count check entirely. Default is 20.
  • Merge-commit detection: Precise parent-ancestry check — for each merge commit in git log --merges targetBranch..HEAD, verify any non-first-parent is an ancestor of the target branch via git merge-base --is-ancestor. This avoids false positives from merges of unrelated feature branches.
  • noFf over method duplication: Rather than adding a separate performMergeCommit method (causing 15+ test/call-site updates), added noFf?: boolean to MergeOptions. When true, performFastForwardMerge skips FF validation and uses --no-ff. Keeps the refactor minimal.
  • Explanatory logging: User-facing messages must explain why the strategy switched (e.g., "Found merge commits from origin/develop — using merge instead of rebase" or "Branch has 25 commits ahead (threshold: 20) — using merge instead of rebase").

Challenges Resolved

  • Ordering bug: The original implementation placed strategy detection after the "already up to date" early return. This meant the most common trigger for merge strategy (a branch that previously ran git merge <parent>) was the exact scenario that short-circuited before detection. Commit enumeration (needed for threshold check) must happen before the early return, and the early return must use the already-detected strategy.
  • Error swallowing in hasMergesFromParent: git merge-base --is-ancestor exits with code 1 when the answer is "no" (not an error). The catch block must specifically check for GitCommandError with exitCode === 1 and re-throw anything else — otherwise real errors (repo corruption, invalid refs) are silently ignored.
  • merge --abort failure masking: The original .catch(() => {}) on git merge --abort silently ignored abort failures, potentially leaving the repo in a broken MERGING state with no indication. Replaced with a try/catch that logs a warning.
  • Hardcoded "main" in messages: Caller messages in finish.ts and rebase.ts hardcoded "main" as the target branch, but looms can target any branch (e.g., origin/develop for child looms). Added targetBranch: string to RebaseOutcome so callers can display the actual resolved branch name.

Lessons Learned

  • When introducing a strategy/mode decision into an existing method that has early returns, audit all early returns to ensure they run after (not before) the strategy detection. Early returns that hardcode a strategy value are subtle bugs.
  • git merge-base --is-ancestor A B returns exit code 1 for the "false" case — this is not an error. Any catch block around this command must distinguish between "not an ancestor" (exit 1, expected) and a real failure (other exit codes or error classes).
  • Pre-call user-facing messages (printed before an async operation) should use generic language ("Synchronizing with parent branch...") since the strategy is unknown until the call returns. Post-call messages can use the specific strategy from the returned result.
  • The RebaseOutcome return type is the contract between MergeManager and its callers. Any behavioral divergence between rebase and merge paths (different merge flags, different log messages, different Claude prompts) must be expressed through this type, not handled ad-hoc in callers.

Generated with 🤖❤️ by iloom.ai

@acreeger acreeger merged commit bc6a7b2 into main May 21, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

1 participant