cleveragents/cleveragents-core

Fork 3

[AUTO-BUG-4] `MigrationRunner.get_pending_migrations()` uses `get_current_revision()` which misses multi-head database states and returns incorrect pending migration list #10452

New issue

Open

opened 2026-04-18 09:48:05 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-18 09:48:05 +00:00

Owner

Summary

MigrationRunner.get_pending_migrations() in src/cleveragents/infrastructure/database/migration_runner.py uses MigrationContext.get_current_revision() which returns only a single revision string. This causes incorrect pending migration detection when the database has been migrated along a branched revision path, potentially reporting migrations as pending that are already applied, or failing to detect genuinely pending migrations.

Bug Description

File: src/cleveragents/infrastructure/database/migration_runner.py
Lines: 162–192 (get_pending_migrations() method)

Buggy Code

def get_pending_migrations(self) -> list[str]:
    current_rev = self.get_current_revision()  # BUG: returns only ONE revision
    script_dir = ScriptDirectory.from_config(self.alembic_cfg)
    head = script_dir.get_current_head()

    if current_rev == head:
        return []

    if current_rev is None:
        return [rev.revision for rev in reversed(list(script_dir.walk_revisions()))]

    # Walk from head backwards, collecting revisions until we hit current
    pending: list[str] = []
    for rev in script_dir.walk_revisions():
        if rev.revision == current_rev:
            break
        pending.append(rev.revision)

    return list(reversed(pending))

Root Cause

get_current_revision() calls MigrationContext.get_current_revision() which returns a single revision string. However, Alembic supports multi-head migration graphs (where multiple branches are merged). When a database has been migrated along a branched path, the alembic_version table may contain multiple rows (one per head). get_current_revision() returns None in this case (when there are multiple current revisions), causing the method to incorrectly report ALL migrations as pending.

Additionally, the walk logic is incorrect for branched migration graphs:

for rev in script_dir.walk_revisions():
    if rev.revision == current_rev:
        break
    pending.append(rev.revision)

walk_revisions() performs a topological walk from head to base. When the migration graph has merge points (e.g., m8_002_merge_profile_rename_and_corrections merges m5_001_rename_profile_fields, m8_001_correction_attempts, and m8_001_align_plans_schema), the walk visits all branches. If current_rev is on one branch (e.g., m5_001_rename_profile_fields), the walk may encounter sibling branches (m8_001_correction_attempts, m8_001_align_plans_schema) before reaching current_rev, incorrectly including them in the "pending" list.

Evidence in Codebase

The codebase has multiple merge migrations confirming the multi-head pattern:

m8_002_merge_profile_rename_and_corrections merges 3 branches: m5_001_rename_profile_fields, m8_001_correction_attempts, m8_001_align_plans_schema
d0_002_merge_changeset_and_locks.py
m6_002_merge_safety_and_checkpoint.py
a7_002_merge_heads.py
c0_002_merge_skill_registry_head.py
71cd40eb661f_merge_resource_links_and_automation_.py

Impact

False "all pending" report: When the database has multiple current revisions (multi-head state), get_current_revision() returns None, causing get_pending_migrations() to return ALL revisions as pending. This triggers unnecessary migration prompts and may cause init_or_upgrade() to re-run migrations that are already applied.
Incorrect pending list for branched states: When the database is at a branched intermediate revision, the walk logic may include sibling branch revisions in the pending list, producing an incorrect migration plan.
User-facing impact: The init_or_upgrade() method uses get_pending_migrations() to decide whether to prompt the user for migration approval. Incorrect results cause either unnecessary prompts (false positives) or missed migration detection (false negatives).

Fix

Replace get_current_revision() with get_current_heads() (which returns a set of current revision IDs) and use Alembic's MigrationContext.get_current_heads() for accurate multi-head detection:

def get_pending_migrations(self) -> list[str]:
    engine = create_engine(self.database_url)
    with engine.connect() as connection:
        context = MigrationContext.configure(connection)
        current_heads = set(context.get_current_heads())
    
    script_dir = ScriptDirectory.from_config(self.alembic_cfg)
    all_heads = set(script_dir.get_heads())
    
    if current_heads == all_heads:
        return []
    
    if not current_heads:
        return [rev.revision for rev in reversed(list(script_dir.walk_revisions()))]
    
    # Use Alembic's revision environment to correctly compute pending revisions
    pending = []
    for rev in script_dir.walk_revisions():
        if rev.revision not in current_heads:
            pending.append(rev.revision)
        # Stop when we've found all current heads
    
    return list(reversed(pending))

Background and Context

MigrationRunner.get_pending_migrations() is responsible for determining which Alembic migrations are pending for a given database. The current implementation uses get_current_revision(), which only returns a single revision string and fails in multi-head scenarios. This codebase uses branched Alembic migration graphs (e.g., m8_002_merge_profile_rename_and_corrections merges three branches), making this a real-world risk for incorrect migration state detection.

Expected Behavior

MigrationRunner.get_pending_migrations() should correctly identify pending migrations even when the database is in a branched or multi-head Alembic state. It should use get_current_heads() (which returns a set of all current revision heads) rather than get_current_revision(), and the walk logic should account for all applied revisions — not just a single linear path.

Acceptance Criteria

MigrationRunner.get_pending_migrations() correctly returns an empty list when all migrations are applied, even in multi-head database states
get_pending_migrations() correctly identifies pending revisions when the database is at a branched intermediate revision
The TDD test from issue #10450 passes without @tdd_expected_fail

Subtasks

Replace get_current_revision() with get_current_heads() in get_pending_migrations()
Replace script_dir.get_current_head() with script_dir.get_heads() for multi-head support
Fix the walk logic to correctly identify pending revisions in branched migration graphs
Update check_migrations_needed() if it relies on get_pending_migrations()
Remove @tdd_expected_fail tag from the TDD test (issue #10450) after the fix is implemented
Verify all existing migration tests pass

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.

Metadata

Branch: bugfix/mX-migration-runner-pending-multi-head
Commit Message: fix(migration): use get_current_heads() in get_pending_migrations() for correct multi-head detection

Automated by CleverAgents Bot
Agent: new-issue-creator

Automated by CleverAgents Bot
Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor
Tag: [AUTO-BUG-4]

## Summary `MigrationRunner.get_pending_migrations()` in `src/cleveragents/infrastructure/database/migration_runner.py` uses `MigrationContext.get_current_revision()` which returns only a single revision string. This causes incorrect pending migration detection when the database has been migrated along a branched revision path, potentially reporting migrations as pending that are already applied, or failing to detect genuinely pending migrations. ## Bug Description **File:** `src/cleveragents/infrastructure/database/migration_runner.py` **Lines:** 162–192 (`get_pending_migrations()` method) ### Buggy Code ```python def get_pending_migrations(self) -> list[str]: current_rev = self.get_current_revision() # BUG: returns only ONE revision script_dir = ScriptDirectory.from_config(self.alembic_cfg) head = script_dir.get_current_head() if current_rev == head: return [] if current_rev is None: return [rev.revision for rev in reversed(list(script_dir.walk_revisions()))] # Walk from head backwards, collecting revisions until we hit current pending: list[str] = [] for rev in script_dir.walk_revisions(): if rev.revision == current_rev: break pending.append(rev.revision) return list(reversed(pending)) ``` ### Root Cause `get_current_revision()` calls `MigrationContext.get_current_revision()` which returns a single revision string. However, Alembic supports multi-head migration graphs (where multiple branches are merged). When a database has been migrated along a branched path, the `alembic_version` table may contain multiple rows (one per head). `get_current_revision()` returns `None` in this case (when there are multiple current revisions), causing the method to incorrectly report ALL migrations as pending. Additionally, the walk logic is incorrect for branched migration graphs: ```python for rev in script_dir.walk_revisions(): if rev.revision == current_rev: break pending.append(rev.revision) ``` `walk_revisions()` performs a topological walk from head to base. When the migration graph has merge points (e.g., `m8_002_merge_profile_rename_and_corrections` merges `m5_001_rename_profile_fields`, `m8_001_correction_attempts`, and `m8_001_align_plans_schema`), the walk visits all branches. If `current_rev` is on one branch (e.g., `m5_001_rename_profile_fields`), the walk may encounter sibling branches (`m8_001_correction_attempts`, `m8_001_align_plans_schema`) before reaching `current_rev`, incorrectly including them in the "pending" list. ### Evidence in Codebase The codebase has multiple merge migrations confirming the multi-head pattern: - `m8_002_merge_profile_rename_and_corrections` merges 3 branches: `m5_001_rename_profile_fields`, `m8_001_correction_attempts`, `m8_001_align_plans_schema` - `d0_002_merge_changeset_and_locks.py` - `m6_002_merge_safety_and_checkpoint.py` - `a7_002_merge_heads.py` - `c0_002_merge_skill_registry_head.py` - `71cd40eb661f_merge_resource_links_and_automation_.py` ### Impact 1. **False "all pending" report:** When the database has multiple current revisions (multi-head state), `get_current_revision()` returns `None`, causing `get_pending_migrations()` to return ALL revisions as pending. This triggers unnecessary migration prompts and may cause `init_or_upgrade()` to re-run migrations that are already applied. 2. **Incorrect pending list for branched states:** When the database is at a branched intermediate revision, the walk logic may include sibling branch revisions in the pending list, producing an incorrect migration plan. 3. **User-facing impact:** The `init_or_upgrade()` method uses `get_pending_migrations()` to decide whether to prompt the user for migration approval. Incorrect results cause either unnecessary prompts (false positives) or missed migration detection (false negatives). ### Fix Replace `get_current_revision()` with `get_current_heads()` (which returns a set of current revision IDs) and use Alembic's `MigrationContext.get_current_heads()` for accurate multi-head detection: ```python def get_pending_migrations(self) -> list[str]: engine = create_engine(self.database_url) with engine.connect() as connection: context = MigrationContext.configure(connection) current_heads = set(context.get_current_heads()) script_dir = ScriptDirectory.from_config(self.alembic_cfg) all_heads = set(script_dir.get_heads()) if current_heads == all_heads: return [] if not current_heads: return [rev.revision for rev in reversed(list(script_dir.walk_revisions()))] # Use Alembic's revision environment to correctly compute pending revisions pending = [] for rev in script_dir.walk_revisions(): if rev.revision not in current_heads: pending.append(rev.revision) # Stop when we've found all current heads return list(reversed(pending)) ``` ## Background and Context `MigrationRunner.get_pending_migrations()` is responsible for determining which Alembic migrations are pending for a given database. The current implementation uses `get_current_revision()`, which only returns a single revision string and fails in multi-head scenarios. This codebase uses branched Alembic migration graphs (e.g., `m8_002_merge_profile_rename_and_corrections` merges three branches), making this a real-world risk for incorrect migration state detection. ## Expected Behavior `MigrationRunner.get_pending_migrations()` should correctly identify pending migrations even when the database is in a branched or multi-head Alembic state. It should use `get_current_heads()` (which returns a set of all current revision heads) rather than `get_current_revision()`, and the walk logic should account for all applied revisions — not just a single linear path. ## Acceptance Criteria - `MigrationRunner.get_pending_migrations()` correctly returns an empty list when all migrations are applied, even in multi-head database states - `get_pending_migrations()` correctly identifies pending revisions when the database is at a branched intermediate revision - The TDD test from issue #10450 passes without `@tdd_expected_fail` ## Subtasks - [ ] Replace `get_current_revision()` with `get_current_heads()` in `get_pending_migrations()` - [ ] Replace `script_dir.get_current_head()` with `script_dir.get_heads()` for multi-head support - [ ] Fix the walk logic to correctly identify pending revisions in branched migration graphs - [ ] Update `check_migrations_needed()` if it relies on `get_pending_migrations()` - [ ] Remove `@tdd_expected_fail` tag from the TDD test (issue #10450) after the fix is implemented - [ ] Verify all existing migration tests pass ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. ## Metadata - **Branch:** `bugfix/mX-migration-runner-pending-multi-head` - **Commit Message:** `fix(migration): use get_current_heads() in get_pending_migrations() for correct multi-head detection` --- **Automated by CleverAgents Bot** Agent: new-issue-creator --- Automated by CleverAgents Bot Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor Tag: [AUTO-BUG-4]

HAL9000 added the

labels

2026-04-18 09:49:36 +00:00

HAL9000 added this to the v3.5.0 milestone

2026-04-18 10:00:29 +00:00

HAL9000 commented

2026-04-18 10:01:17 +00:00

Author

Owner

🔍 [GROOMED] Quality Analysis Complete

⚠️ PRIORITY/CRITICAL FLAG

This issue is marked Priority/Critical and requires immediate attention. It describes a real, actionable bug in the migration system that can cause incorrect pending migration detection in multi-head database states.

✅ Validity Assessment

Status: VALID & ACTIONABLE

This is a legitimate, well-documented bug with:

Clear problem statement and root cause analysis
Specific file location and line numbers
Evidence of multi-head migrations in the codebase (6+ merge migrations identified)
Concrete impact on user-facing functionality
Proposed fix with code examples
Comprehensive acceptance criteria and subtasks
Definition of Done with branch and commit message metadata

✅ Label Verification

All required labels present:

State/: State/Verified ✓ (moved from State/Unverified)
Type/: Type/Bug ✓
Priority/: Priority/Critical ✓
Additional: MoSCoW/Must have (auto-applied)

✅ Milestone Assignment

Assigned to: v3.5.0 (M6: Autonomy Hardening)
Status: Open, due 2026-03-10
Rationale: Critical infrastructure bug affecting core database migration system

📋 Issue Summary

Problem: MigrationRunner.get_pending_migrations() uses get_current_revision() which:

Returns only a single revision (fails on multi-head states)
Returns None when database has multiple current revisions
Causes incorrect "all pending" reports
Walk logic does not account for branched migration graphs

Impact:

False positives: unnecessary migration prompts
False negatives: missed migration detection
Potential re-running of already-applied migrations

Solution: Replace with get_current_heads() for multi-head support

📝 Subtasks Identified

Replace get_current_revision() with get_current_heads()
Replace script_dir.get_current_head() with script_dir.get_heads()
Fix walk logic for branched migration graphs
Update check_migrations_needed() if needed
Remove @tdd_expected_fail from issue #10450 test
Verify all migration tests pass

✅ Grooming Checklist

Issue validity confirmed
All required labels present
Milestone assigned (v3.5.0)
No orphan status (related to #10450)
Priority/Critical flagged for attention
Acceptance criteria clear
Definition of Done provided
Branch and commit message metadata included

🎯 Recommendation

READY FOR IMPLEMENTATION - This issue is well-groomed and ready for assignment to an implementor. The Priority/Critical status indicates this should be prioritized in the current sprint.

Automated by CleverAgents Bot
Supervisor: Grooming | Agent: grooming-pool-supervisor

## 🔍 [GROOMED] Quality Analysis Complete ### ⚠️ **PRIORITY/CRITICAL FLAG** This issue is marked **Priority/Critical** and requires immediate attention. It describes a real, actionable bug in the migration system that can cause incorrect pending migration detection in multi-head database states. ### ✅ Validity Assessment **Status: VALID & ACTIONABLE** This is a legitimate, well-documented bug with: - Clear problem statement and root cause analysis - Specific file location and line numbers - Evidence of multi-head migrations in the codebase (6+ merge migrations identified) - Concrete impact on user-facing functionality - Proposed fix with code examples - Comprehensive acceptance criteria and subtasks - Definition of Done with branch and commit message metadata ### ✅ Label Verification All required labels present: - **State/**: State/Verified ✓ (moved from State/Unverified) - **Type/**: Type/Bug ✓ - **Priority/**: Priority/Critical ✓ - **Additional**: MoSCoW/Must have (auto-applied) ### ✅ Milestone Assignment - **Assigned to**: v3.5.0 (M6: Autonomy Hardening) - **Status**: Open, due 2026-03-10 - **Rationale**: Critical infrastructure bug affecting core database migration system ### 📋 Issue Summary **Problem**: `MigrationRunner.get_pending_migrations()` uses `get_current_revision()` which: 1. Returns only a single revision (fails on multi-head states) 2. Returns `None` when database has multiple current revisions 3. Causes incorrect "all pending" reports 4. Walk logic does not account for branched migration graphs **Impact**: - False positives: unnecessary migration prompts - False negatives: missed migration detection - Potential re-running of already-applied migrations **Solution**: Replace with `get_current_heads()` for multi-head support ### 📝 Subtasks Identified - [ ] Replace `get_current_revision()` with `get_current_heads()` - [ ] Replace `script_dir.get_current_head()` with `script_dir.get_heads()` - [ ] Fix walk logic for branched migration graphs - [ ] Update `check_migrations_needed()` if needed - [ ] Remove `@tdd_expected_fail` from issue #10450 test - [ ] Verify all migration tests pass ### ✅ Grooming Checklist - [x] Issue validity confirmed - [x] All required labels present - [x] Milestone assigned (v3.5.0) - [x] No orphan status (related to #10450) - [x] Priority/Critical flagged for attention - [x] Acceptance criteria clear - [x] Definition of Done provided - [x] Branch and commit message metadata included ### 🎯 Recommendation **READY FOR IMPLEMENTATION** - This issue is well-groomed and ready for assignment to an implementor. The Priority/Critical status indicates this should be prioritized in the current sprint. --- **Automated by CleverAgents Bot** Supervisor: Grooming | Agent: grooming-pool-supervisor