fix(db): backfill turn_progress_text columns for v15 migration collision
Deployments that shipped reviewer_failure_count as a local migration numbered 15 record schema_migrations version 15 under that name, so the version-number-only runner skips the canonical turn_progress_text migration (also version 15) and never creates paired_turns.progress_text / progress_updated_at, which the runtime reads. Add an idempotent compat migration (020) that re-adds the columns when missing, plus a regression test reproducing the collided-version-15 database. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
This commit is contained in:
@@ -6,8 +6,17 @@ import { afterEach, describe, expect, it } from 'vitest';
|
|||||||
|
|
||||||
import { applyBaseSchema } from './base-schema.js';
|
import { applyBaseSchema } from './base-schema.js';
|
||||||
import { initializeDatabaseSchema } from './bootstrap.js';
|
import { initializeDatabaseSchema } from './bootstrap.js';
|
||||||
|
import { applyVersionedSchemaMigrations } from './migrations/index.js';
|
||||||
import { applyLegacySchemaMigrations } from './schema.js';
|
import { applyLegacySchemaMigrations } from './schema.js';
|
||||||
|
|
||||||
|
function tableColumns(database: Database, table: string): string[] {
|
||||||
|
return (
|
||||||
|
database.prepare(`PRAGMA table_info(${table})`).all() as Array<{
|
||||||
|
name: string;
|
||||||
|
}>
|
||||||
|
).map((column) => column.name);
|
||||||
|
}
|
||||||
|
|
||||||
function getAppliedSchemaMigrations(
|
function getAppliedSchemaMigrations(
|
||||||
database: Database,
|
database: Database,
|
||||||
): Array<{ version: number; name: string }> {
|
): Array<{ version: number; name: string }> {
|
||||||
@@ -44,6 +53,7 @@ function getExpectedSchemaMigrations(): Array<{
|
|||||||
{ version: 17, name: 'scheduled_task_room_role' },
|
{ version: 17, name: 'scheduled_task_room_role' },
|
||||||
{ version: 18, name: 'paired_turn_output_attachments' },
|
{ version: 18, name: 'paired_turn_output_attachments' },
|
||||||
{ version: 19, name: 'reviewer_failure_count' },
|
{ version: 19, name: 'reviewer_failure_count' },
|
||||||
|
{ version: 20, name: 'turn_progress_text_compat' },
|
||||||
];
|
];
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -114,4 +124,55 @@ describe('initializeDatabaseSchema', () => {
|
|||||||
reopened.close();
|
reopened.close();
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('backfills turn_progress_text columns when version 15 was recorded under a different name', () => {
|
||||||
|
const database = new Database(':memory:');
|
||||||
|
|
||||||
|
try {
|
||||||
|
applyBaseSchema(database);
|
||||||
|
|
||||||
|
// Reproduce a deployment that first shipped reviewer_failure_count as a
|
||||||
|
// local migration numbered 15, before turn_progress_text claimed that
|
||||||
|
// version upstream. The runner skips by version number, so migration 015
|
||||||
|
// (turn_progress_text) would otherwise never run on this database.
|
||||||
|
database.exec(`
|
||||||
|
CREATE TABLE schema_migrations (
|
||||||
|
version INTEGER PRIMARY KEY,
|
||||||
|
name TEXT NOT NULL,
|
||||||
|
applied_at TEXT NOT NULL DEFAULT (datetime('now'))
|
||||||
|
)
|
||||||
|
`);
|
||||||
|
const collidedNames: Record<number, string> = {
|
||||||
|
15: 'reviewer_failure_count',
|
||||||
|
};
|
||||||
|
for (let version = 1; version <= 15; version += 1) {
|
||||||
|
database
|
||||||
|
.prepare('INSERT INTO schema_migrations (version, name) VALUES (?, ?)')
|
||||||
|
.run(version, collidedNames[version] ?? `legacy_${version}`);
|
||||||
|
}
|
||||||
|
// Precondition: the collided database is missing the progress columns.
|
||||||
|
expect(tableColumns(database, 'paired_turns')).not.toContain(
|
||||||
|
'progress_text',
|
||||||
|
);
|
||||||
|
|
||||||
|
applyVersionedSchemaMigrations(database, { assistantName: 'Andy' });
|
||||||
|
|
||||||
|
const columns = tableColumns(database, 'paired_turns');
|
||||||
|
expect(columns).toContain('progress_text');
|
||||||
|
expect(columns).toContain('progress_updated_at');
|
||||||
|
|
||||||
|
// The pre-existing version-15 row is left untouched; the compat migration
|
||||||
|
// (version 20) is what reconciles the schema.
|
||||||
|
const version15 = database
|
||||||
|
.prepare('SELECT name FROM schema_migrations WHERE version = 15')
|
||||||
|
.get() as { name: string };
|
||||||
|
expect(version15.name).toBe('reviewer_failure_count');
|
||||||
|
const version20 = database
|
||||||
|
.prepare('SELECT name FROM schema_migrations WHERE version = 20')
|
||||||
|
.get() as { name: string } | null;
|
||||||
|
expect(version20?.name).toBe('turn_progress_text_compat');
|
||||||
|
} finally {
|
||||||
|
database.close();
|
||||||
|
}
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
32
src/db/migrations/020_turn-progress-text-compat.ts
Normal file
32
src/db/migrations/020_turn-progress-text-compat.ts
Normal file
@@ -0,0 +1,32 @@
|
|||||||
|
import type { Database } from 'bun:sqlite';
|
||||||
|
|
||||||
|
import { tableHasColumn } from './helpers.js';
|
||||||
|
import type { SchemaMigrationDefinition } from './types.js';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Compatibility backfill for `turn_progress_text` (migration 015).
|
||||||
|
*
|
||||||
|
* Some deployments first shipped `reviewer_failure_count` as a local migration
|
||||||
|
* numbered 15, so their `schema_migrations` table records version 15 under a
|
||||||
|
* different name than the canonical `turn_progress_text` migration. The runner
|
||||||
|
* skips migrations purely by version number, so on those databases the real
|
||||||
|
* version-15 migration never runs and `paired_turns.progress_text` /
|
||||||
|
* `progress_updated_at` are missing — yet the runtime reads them. This migration
|
||||||
|
* re-adds the columns idempotently so collided databases converge with fresh
|
||||||
|
* ones. On a fresh database migration 015 already created the columns, so the
|
||||||
|
* `tableHasColumn` guards make this a no-op.
|
||||||
|
*/
|
||||||
|
export const TURN_PROGRESS_TEXT_COMPAT_MIGRATION: SchemaMigrationDefinition = {
|
||||||
|
version: 20,
|
||||||
|
name: 'turn_progress_text_compat',
|
||||||
|
apply(database: Database) {
|
||||||
|
if (!tableHasColumn(database, 'paired_turns', 'progress_text')) {
|
||||||
|
database.exec(`ALTER TABLE paired_turns ADD COLUMN progress_text TEXT`);
|
||||||
|
}
|
||||||
|
if (!tableHasColumn(database, 'paired_turns', 'progress_updated_at')) {
|
||||||
|
database.exec(
|
||||||
|
`ALTER TABLE paired_turns ADD COLUMN progress_updated_at TEXT`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
},
|
||||||
|
};
|
||||||
@@ -19,6 +19,7 @@ import { ROOM_SKILL_OVERRIDES_MIGRATION } from './016_room-skill-overrides.js';
|
|||||||
import { SCHEDULED_TASK_ROOM_ROLE_MIGRATION } from './017_scheduled-task-room-role.js';
|
import { SCHEDULED_TASK_ROOM_ROLE_MIGRATION } from './017_scheduled-task-room-role.js';
|
||||||
import { PAIRED_TURN_OUTPUT_ATTACHMENTS_MIGRATION } from './018_paired-turn-output-attachments.js';
|
import { PAIRED_TURN_OUTPUT_ATTACHMENTS_MIGRATION } from './018_paired-turn-output-attachments.js';
|
||||||
import { REVIEWER_FAILURE_COUNT_MIGRATION } from './019_reviewer-failure-count.js';
|
import { REVIEWER_FAILURE_COUNT_MIGRATION } from './019_reviewer-failure-count.js';
|
||||||
|
import { TURN_PROGRESS_TEXT_COMPAT_MIGRATION } from './020_turn-progress-text-compat.js';
|
||||||
import type {
|
import type {
|
||||||
SchemaMigrationArgs,
|
SchemaMigrationArgs,
|
||||||
SchemaMigrationDefinition,
|
SchemaMigrationDefinition,
|
||||||
@@ -46,6 +47,7 @@ const ORDERED_SCHEMA_MIGRATIONS: readonly SchemaMigrationDefinition[] = [
|
|||||||
SCHEDULED_TASK_ROOM_ROLE_MIGRATION,
|
SCHEDULED_TASK_ROOM_ROLE_MIGRATION,
|
||||||
PAIRED_TURN_OUTPUT_ATTACHMENTS_MIGRATION,
|
PAIRED_TURN_OUTPUT_ATTACHMENTS_MIGRATION,
|
||||||
REVIEWER_FAILURE_COUNT_MIGRATION,
|
REVIEWER_FAILURE_COUNT_MIGRATION,
|
||||||
|
TURN_PROGRESS_TEXT_COMPAT_MIGRATION,
|
||||||
];
|
];
|
||||||
|
|
||||||
function ensureSchemaMigrationsTable(database: Database): void {
|
function ensureSchemaMigrationsTable(database: Database): void {
|
||||||
|
|||||||
Reference in New Issue
Block a user