[diegosouzapw]

Problem

release/v3.8.8 shipped two migration files with the same numeric prefix:

• 077_api_key_stream_default_mode.sql (commit 99d673fe5)
• 077_quota_pools.sql (entered via merge 1b0d3c75e)

getMigrationFiles() throws a hard Migration version collision detected error on duplicate prefixes (unless allowlisted in SUPERSEDED_DUPLICATE_MIGRATIONS). This pair was not allowlisted, so getDbInstance() threw at every startup — the app would not boot, and every DB-touching unit test was red on release/v3.8.8.

Fix

• Renumber quota_pools 077 → 085. It is dependency-free (no other migration references quota_pools/quota_allocations) and fully idempotent (CREATE TABLE IF NOT EXISTS), so it can safely move to the next free number. The non-idempotent api_key_stream_default_mode ALTER TABLE ADD COLUMN stays at 077.
• Add a retroactive isSchemaAlreadyApplied guard (case 085) so DBs that already applied quota_pools under the old 077 number do not re-run it (mirrors the existing 070/071 precedents for migrations renumbered out of the 068 collision).
• Add regression test tests/unit/db-migration-version-uniqueness.test.ts enforcing unique migration prefixes.

Verification

• db-migration-version-uniqueness: 2/2 pass (new)
• db-quota-migrations-idempotency: 6/6 pass (was red before — confirms getDbInstance() unblocked)
• db-migration-runner 22/22, db-core-migration 3/3, usage-migrations 6/6, request-log-migration 2/2
• typecheck:core: no new errors in touched files (4 pre-existing baseline errors in mcp-server/server.ts + transformersLocal.ts are unrelated)
• lint on touched files: clean

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request resolves a migration version collision at prefix 077 by renumbering the idempotent quota_pools migration to 085, adding a retroactive schema guard, and introducing a regression test to enforce unique migration prefixes. Feedback highlights a critical bug where databases that already applied quota_pools under 077 will silently skip the new 077_api_key_stream_default_mode migration; to fix this, a compatibility entry should be added to rename the migration in the tracking table. Additionally, the new uniqueness test should be updated to filter out superseded duplicate migrations as documented in its comments.

[gemini-code-assist]

🚨 Critical Migration Skip Bug

While adding the retroactive isSchemaAlreadyApplied guard for "085" prevents 085_quota_pools.sql from re-running, it does not free up the "077" slot in the _omniroute_migrations table for databases that already applied quota_pools under the old "077" version.

The Problem:

If a database has already applied quota_pools as version "077", the _omniroute_migrations table contains a row with version = "077" and name = "quota_pools".
When the migration runner checks for pending migrations:

1. It sees "077" is already in the applied versions set.
2. It silently skips the new 077_api_key_stream_default_mode.sql migration because its version ("077") is considered already applied.
3. As a result, the database will be missing the schema changes from 077_api_key_stream_default_mode.sql (e.g., missing columns), leading to runtime crashes.

The Solution:

You must add an entry to RENAMED_MIGRATION_COMPATIBILITY to automatically rename the applied 077_quota_pools migration to 085_quota_pools in the tracking table. This will free up the "077" slot so 077_api_key_stream_default_mode can run.

Please add the following entry to RENAMED_MIGRATION_COMPATIBILITY (around line 173):

  {
    fromVersion: "077",
    fromName: "quota_pools",
    toVersion: "085",
    toName: "quota_pools",
  }

[gemini-code-assist]

⚠️ Test Does Not Filter Superseded Migrations

The test comment states that this test checks for collisions "minus any SUPERSEDED_DUPLICATE_MIGRATIONS allowlisted renamed pairs", but the implementation does not actually perform this filtering.

If any superseded duplicate migrations (such as 041_session_account_affinity) are present on the filesystem, this test will fail. To make the test robust and align with the documented intent, you should filter out the allowlisted superseded migrations.

Since SUPERSEDED_DUPLICATE_MIGRATIONS is not exported from migrationRunner.ts, you can define a local list of allowed duplicates in the test file and skip them during the collision check.

  const SUPERSEDED_DUPLICATE_MIGRATIONS = [
    { version: "041", name: "session_account_affinity" }
  ];

  const byVersion = new Map<string, string[]>();
  for (const f of migrationFiles()) {
    const isSuperseded = SUPERSEDED_DUPLICATE_MIGRATIONS.some(
      (sup) => sup.version === f.version && sup.name === f.name
    );
    if (isSuperseded) continue;

    if (!byVersion.has(f.version)) byVersion.set(f.version, []);
    byVersion.get(f.version)!.push(f.name);
  }
  const collisions = [...byVersion.entries()]
    .filter(([, names]) => names.length > 1)
    .map(([version, names]) => `${version} → [${names.join(", ")}]`);
  assert.deepEqual(
    collisions,
    [],
    `Migration version collision(s) detected: ${collisions.join("; ")}. ` +
      `Each migration must have a unique numeric prefix (rename to the next free number).`
  );

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d5b163558d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Rehome legacy 077 quota marker before skipping 085

For users upgrading from a build where 077_quota_pools was already recorded in _omniroute_migrations, this guard only prevents re-running the renamed quota migration; it does not free the old 077 version slot. Since runMigrations() builds applied as a set of versions only, the current 077_api_key_stream_default_mode.sql is then treated as already applied and its ALTER TABLE api_keys ADD COLUMN stream_default_mode never runs. Add a compatibility/rehome step for 077_quota_pools before computing applied versions so the new 077 migration can execute while 085 is marked/skipped safely.

Useful? React with 👍 / 👎.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

This is a well-executed fix for a critical migration version collision. The changes:

1. migrationRunner.ts - Added case "085" retroactive guard in isSchemaAlreadyApplied() that checks for both quota_pools and quota_allocations tables. This follows the exact same pattern as existing cases 070 and 071 for previously renumbered migrations.

2. migrations/085_quota_pools.sql - Renumbered from 077 to 085 with clear header comments explaining the renumbering reason. The SQL uses CREATE TABLE IF NOT EXISTS making it idempotent.

3. Tests - New regression test correctly validates no duplicate prefixes and verifies quota_pools is at 085 while api_key_stream_default_mode stays at 077.

Verification

• Migration version 085 is the next free number (after 084_playground_presets.sql)
• No other migrations reference quota_pools or quota_allocations tables
• The idempotency check (both tables existence) correctly prevents re-running on DBs that already applied under old 077 number

Files Reviewed (4 files)

• src/lib/db/migrationRunner.ts - 1 change (added case 085 guard)
• src/lib/db/migrations/085_quota_pools.sql - renumbering with comments
• tests/unit/db-migration-version-uniqueness.test.ts - new regression test
• CHANGELOG.md - documentation update

---

_{Reviewed by laguna-m.1-20260312:free · 1,126,989 tokens}