subprocess.TimeoutExpired not caught in rollback handler

_roll_back_migration calls subprocess.run(..., timeout=60, check=True), which can raise either subprocess.CalledProcessError (non-zero exit) or subprocess.TimeoutExpired (if the command takes >60 s on a slow/loaded database). The new try/except only catches CalledProcessError, so a TimeoutExpired will still escape, skip _resolve_specific_migration, and cause the recovery to fail — the exact bug this PR aims to fix.

The equivalent rollback in the permission-error path (line ~494) correctly uses except Exception to guard against all failure modes.

Continue migration deploy after resolving one failed step

Returning True immediately after _resolve_specific_migration() marks only the currently failing migration as applied and exits setup_database, so any later pending migrations are never deployed in this startup run. This occurs when prisma migrate deploy hits an idempotent P3018 on migration N but N+1+ migrations are still pending (common on version-skipping upgrades), causing startup to report success while the schema remains partially migrated.

Useful? React with 👍 / 👎.

_UNSAFE_PATTERNS is dead code

The _UNSAFE_PATTERNS list is defined here but never referenced anywhere — each of the four TestMigrationSQLIdempotency test methods re-implements its own inline regex check independently. The variable serves no purpose and can mislead future maintainers into thinking it drives the tests.

Additionally, there's a subtle inconsistency: the DROP INDEX entry uses (?!.*ON) to try to exclude MySQL-style DROP INDEX name ON table syntax, and the CREATE INDEX entry uses (?!CONCURRENTLY) to skip CONCURRENTLY indexes — but neither of these exclusions appears in the corresponding test methods, meaning _UNSAFE_PATTERNS and the actual enforcement logic are already diverged.

Either delete the variable, or replace the four duplicate inline-regex blocks in the test methods with a single parametrized test driven by _UNSAFE_PATTERNS.

Hard assert in fixture will fail instead of skip when path is wrong

The recent_migrations fixture raises AssertionError (i.e., a test failure, not a skip) when no migration files are found. If _MIGRATIONS_DIR is incorrect in some CI environment — or the migrations directory is cleaned — all four tests will be reported as failures rather than skipped/xfailed, which makes it harder to distinguish a configuration problem from a real regression.

Consider using pytest.skip when the directory is missing or empty:

RENAME COLUMN is not idempotent on pre-existing instances

This PR makes every ADD COLUMN, DROP COLUMN, and index DDL statement idempotent with IF [NOT] EXISTS, but the bare RENAME COLUMN "call_policy" TO "input_policy" on line 2 is left unguarded. On any pre-existing instance where call_policy was already renamed (e.g., the migration ran once and was then re-attempted), PostgreSQL raises ERROR: column "call_policy" does not exist, which would trip the same failure mode this PR is fixing.

The migration 20250806095134_rename_alias_to_server_name_mcp_table already shows the correct pattern:

DO $$
BEGIN
    IF EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'LiteLLM_ToolTable' AND column_name = 'call_policy') THEN
        ALTER TABLE "LiteLLM_ToolTable" RENAME COLUMN "call_policy" TO "input_policy";
    END IF;
END $$;

This guards the rename with an existence check so re-running it is safe.

TestMigrationSQLIdempotency has no coverage for RENAME COLUMN

The four test methods enforce idempotency for ADD COLUMN, DROP COLUMN, DROP INDEX, and CREATE INDEX, but there is no test for bare RENAME COLUMN statements — which are also non-idempotent. The migration 20260303000000_update_tool_table_policies contains an unguarded RENAME COLUMN that would fail on a pre-existing instance.

Adding a fifth method would catch regressions of this class:

def test_rename_column_uses_do_block(self, all_migrations):
    """RENAME COLUMN must be wrapped in a conditional DO $$ block"""
    violations = []
    for migration_name, sql in all_migrations:
        for line_num, line in enumerate(sql.splitlines(), 1):
            if re.search(r"RENAME\s+COLUMN\s+", line, re.IGNORECASE):
                # Bare RENAME COLUMN outside a DO block is not idempotent
                # Check that it appears inside a DO $$ ... END $$ block
                if not re.search(r"^\s*(IF\s+EXISTS|DO\s+\$)", line, re.IGNORECASE):
                    violations.append(f"  {migration_name}:{line_num}: {line.strip()}")
    assert not violations, (
        "RENAME COLUMN outside a conditional DO $$ block found in migrations:\n"
        + "\n".join(violations)
    )

Default behavior changed from fail-safe to fail-open

The proxy previously exited with code 1 whenever setup_database() returned False. This PR flips the default: migration failures are now silently swallowed and startup continues unless the operator explicitly passes --require_db_migration.

This is a backwards-incompatible behavior change that could leave production deployments in a broken state:

• The database schema may be incomplete (missing columns, tables, or indexes that the application code expects)
• The proxy starts up and serves traffic, but runtime errors then surface for requests that hit the affected schema objects
• Automated deployments that relied on the old fail-fast behavior to catch migration problems now silently proceed

Operators who aren't aware of this flag change won't know to add --require_db_migration to their startup command. Consider keeping the old fail-fast behavior as the default and instead adding a --ignore_db_migration_failure flag for operators who explicitly want the lenient behavior.

Rule Used: What: avoid backwards-incompatible changes without... (source)

return True skips remaining pending migrations

After resolving a single idempotent migration, the code returns True immediately. But _resolve_specific_migration only marks that one migration as applied — it does not apply any other pending migrations. If there are N migrations pending, resolving one and returning True causes the proxy to start up thinking the schema is fully up-to-date, when migrations 2…N have never actually run.

continue would be the correct replacement: it lets the for attempt in range(3) loop execute another prisma migrate deploy, which will now skip the resolved migration and proceed with the remaining ones:

should instead be:

                                        logger.info("✅ Migration resolved, retrying migrate deploy for remaining migrations.")
                                        # break inner error-handling and retry the outer migrate deploy loop
                                        break  # or continue the outer loop

More concretely, replacing return True with continue (if the outer for attempt in range(3) loop is accessible) or restructuring so the outer loop retries would ensure all pending migrations are applied before the function signals success.

Non-idempotent DROP CONSTRAINT left unguarded

This PR wraps ADD CONSTRAINT in DO $$ IF NOT EXISTS blocks for idempotency, but the DROP CONSTRAINT directly above (line 2) was not updated. If this migration partially executes — e.g., the DROP CONSTRAINT succeeds but something fails before the migration is marked applied — then any retry attempt will fail with ERROR: constraint "LiteLLM_TeamMembership_budget_id_fkey" of relation "LiteLLM_TeamMembership" does not exist.

The same issue exists in 20250625145206_cascade_budget_and_loosen_managed_file_json/migration.sql:2.

The fix is to wrap the drop in a DO $$ block with an existence check, mirroring the pattern used for the re-add:

-- DropForeignKey
DO $$
BEGIN
    IF EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'LiteLLM_TeamMembership_budget_id_fkey') THEN
        ALTER TABLE "LiteLLM_TeamMembership" DROP CONSTRAINT "LiteLLM_TeamMembership_budget_id_fkey";
    END IF;
END $$;

This matches the same pg_constraint lookup used for the idempotent ADD CONSTRAINT block below it.

No test coverage for DROP CONSTRAINT idempotency

TestMigrationSQLIdempotency now covers ADD COLUMN, DROP COLUMN, DROP INDEX, CREATE INDEX, RENAME COLUMN, and ADD CONSTRAINT, but there is no test for bare DROP CONSTRAINT without an IF EXISTS guard or a DO $$ block wrapper. Two migration files changed in this PR — 20250603210143_cascade_budget_changes and 20250625145206_cascade_budget_and_loosen_managed_file_json — contain unguarded ALTER TABLE ... DROP CONSTRAINT ... statements that a new test method would immediately catch:

def test_drop_constraint_is_guarded(self, all_migrations):
    """DROP CONSTRAINT must use IF EXISTS or be wrapped in a DO $$ block"""
    violations = []
    for migration_name, sql in all_migrations:
        lines = sql.splitlines()
        in_do_block = False
        for line_num, line in enumerate(lines, 1):
            if re.search(r"DO\s+\$\$", line, re.IGNORECASE):
                in_do_block = True
            if re.search(r"END\s+\$\$", line, re.IGNORECASE):
                in_do_block = False
            if (
                re.search(r"DROP\s+CONSTRAINT\s+", line, re.IGNORECASE)
                and not re.search(r"DROP\s+CONSTRAINT\s+IF\s+EXISTS", line, re.IGNORECASE)
                and not in_do_block
            ):
                violations.append(f"  {migration_name}:{line_num}: {line.strip()}")
    assert not violations, (
        "DROP CONSTRAINT without IF EXISTS or DO $$ guard found in migrations:\n"
        + "\n".join(violations)
    )