Skip to content

Commit f89014c

Browse files
amankoli09claude
andcommitted
fix: resolve merge conflicts and fix Migration Dry-Run / Backend Tests CI
Merged origin/main into this branch: - .github/workflows/ci.yml: main had already independently added the same "zk-tests" job content this branch's diff collided with, which left main with that job duplicated verbatim. Resolved by keeping one copy of zk-tests and appending this branch's new migration-dry-run job after it, as originally authored. - backend/src/db/timescale.ts: kept this branch's new dryRunMigrations and getMigrationStatus functions; main had no competing changes here. Fixed two bugs the merge/CI run surfaced: - dryRunMigrations ran every pending migration in its own BEGIN/ROLLBACK, so a migration's schema changes never persisted for the next one in the same run — any migration depending on an earlier one (e.g. 007_commitments_cursor_indexes.sql referencing commitment_outcomes from 001_timescale_setup.sql) always failed with "relation does not exist", regardless of whether the SQL was actually correct. Switched to one outer transaction with a savepoint per migration, matching what the existing code comment already described. - The Migration Dry-Run job used plain postgres:16, which doesn't have the timescaledb extension 001_timescale_setup.sql requires (unlike the real docker-compose stack, which uses timescale/timescaledb:latest-pg16). Switched the CI service to the same image so the dry-run is actually representative. - This branch's own rename of 004_horizon_sse_cursors.sql -> 006_horizon_sse_cursors.sql (deduplicating a numbering collision with 004_reputation_snapshots.sql) left a stale hardcoded path in sse-indexer.test.ts pointing at the old filename, failing Backend Tests. Updated both references. Verified locally: full backend build, `npm test` (all 3 suites), and migrate-dry-run.js against a real timescale/timescaledb:latest-pg16 container — all 10 migrations now validate successfully. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 8a23624 commit f89014c

4 files changed

Lines changed: 48 additions & 39 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,15 +108,18 @@ jobs:
108108
runs-on: ubuntu-latest
109109
env:
110110
# Custom runner reads TIMESCALEDB_* env vars; point them at the ephemeral
111-
# postgres:16 service container spun up for this job.
111+
# timescaledb service container spun up for this job.
112112
TIMESCALEDB_HOST: localhost
113113
TIMESCALEDB_PORT: 5432
114114
TIMESCALEDB_DATABASE: pactum_migration_test
115115
TIMESCALEDB_USER: postgres
116116
TIMESCALEDB_PASSWORD: postgres
117117
services:
118118
postgres:
119-
image: postgres:16
119+
# Same image docker-compose.yml uses for the real service, so
120+
# CREATE EXTENSION timescaledb (001_timescale_setup.sql) succeeds here
121+
# instead of only in the full docker-compose e2e stack.
122+
image: timescale/timescaledb:latest-pg16
120123
env:
121124
POSTGRES_PASSWORD: postgres
122125
POSTGRES_DB: pactum_migration_test
@@ -158,14 +161,8 @@ jobs:
158161
working-directory: backend
159162
run: npm run build
160163

161-
- name: Run migration dry-run against ephemeral PostgreSQL
164+
- name: Run migration dry-run against ephemeral TimescaleDB
162165
working-directory: backend
163-
# Note: TimescaleDB-specific DDL (CREATE EXTENSION timescaledb,
164-
# create_hypertable, time_bucket) requires the timescaledb extension
165-
# which is not available in plain postgres:16. The dry-run validates
166-
# checksum integrity and sequential ordering; TimescaleDB-specific
167-
# syntax is exercised in the full integration stack (docker compose).
168-
# Any migration that fails basic PostgreSQL parsing will still fail here.
169166
run: node dist/db/migrate-dry-run.js
170167

171168

‎backend/src/db/migrations/007_commitments_cursor_indexes.sql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
-- Migration: 005_commitments_cursor_indexes.sql
1+
-- Migration: 007_commitments_cursor_indexes.sql
22
-- Add template column and composite indexes for high-performance cursor-based pagination and filtering
33

44
ALTER TABLE commitment_outcomes ADD COLUMN IF NOT EXISTS template VARCHAR(50);

‎backend/src/db/timescale.ts‎

Lines changed: 39 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -177,36 +177,48 @@ export const dryRunMigrations = async (
177177
]),
178178
);
179179

180-
for (const file of migrationFiles) {
181-
const filePath = path.join(migrationsDir, file);
182-
const sql = fs.readFileSync(filePath, 'utf8');
183-
const checksum = calculateMigrationChecksum(sql);
184-
185-
if (appliedMap.has(file)) {
186-
const recordedChecksum = appliedMap.get(file);
187-
if (recordedChecksum !== checksum) {
188-
const msg = `Immutable migration '${file}' has been altered! Recorded: ${recordedChecksum}, Current: ${checksum}`;
189-
logger.error(`[dry-run] TAMPERED: ${msg}`);
190-
report.push({ file, status: 'tampered', message: msg });
180+
// Pending migrations are applied inside one outer transaction, each under its
181+
// own savepoint, so a migration sees the cumulative schema left by the pending
182+
// migrations before it — exactly as a real sequential apply would. Rolling back
183+
// the outer transaction at the end (rather than each migration individually)
184+
// means nothing is ever persisted, while dependencies between migrations (e.g.
185+
// an index migration referencing a table an earlier migration creates) still
186+
// validate correctly instead of spuriously failing with "relation does not
187+
// exist".
188+
await client.query('BEGIN');
189+
try {
190+
for (const file of migrationFiles) {
191+
const filePath = path.join(migrationsDir, file);
192+
const sql = fs.readFileSync(filePath, 'utf8');
193+
const checksum = calculateMigrationChecksum(sql);
194+
195+
if (appliedMap.has(file)) {
196+
const recordedChecksum = appliedMap.get(file);
197+
if (recordedChecksum !== checksum) {
198+
const msg = `Immutable migration '${file}' has been altered! Recorded: ${recordedChecksum}, Current: ${checksum}`;
199+
logger.error(`[dry-run] TAMPERED: ${msg}`);
200+
report.push({ file, status: 'tampered', message: msg });
201+
} else {
202+
logger.info(`[dry-run] OK (applied): ${file}`);
203+
report.push({ file, status: 'applied' });
204+
}
191205
} else {
192-
logger.info(`[dry-run] OK (applied): ${file}`);
193-
report.push({ file, status: 'applied' });
194-
}
195-
} else {
196-
// Validate the SQL parses cleanly using a savepoint that we always roll back.
197-
await client.query('BEGIN');
198-
try {
199-
await client.query(sql);
200-
logger.info(`[dry-run] OK (pending, syntax valid): ${file}`);
201-
report.push({ file, status: 'pending' });
202-
} catch (err) {
203-
const msg = err instanceof Error ? err.message : String(err);
204-
logger.error(`[dry-run] SYNTAX ERROR in ${file}: ${msg}`);
205-
report.push({ file, status: 'error', message: msg });
206-
} finally {
207-
await client.query('ROLLBACK');
206+
await client.query('SAVEPOINT dry_run_migration');
207+
try {
208+
await client.query(sql);
209+
await client.query('RELEASE SAVEPOINT dry_run_migration');
210+
logger.info(`[dry-run] OK (pending, syntax valid): ${file}`);
211+
report.push({ file, status: 'pending' });
212+
} catch (err) {
213+
await client.query('ROLLBACK TO SAVEPOINT dry_run_migration');
214+
const msg = err instanceof Error ? err.message : String(err);
215+
logger.error(`[dry-run] SYNTAX ERROR in ${file}: ${msg}`);
216+
report.push({ file, status: 'error', message: msg });
217+
}
208218
}
209219
}
220+
} finally {
221+
await client.query('ROLLBACK');
210222
}
211223

212224
return report;

‎backend/src/indexer/sse-indexer.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ test(
397397
try {
398398
await pool.query(`CREATE SCHEMA "${schema}"`);
399399
const migration = await readFile(
400-
path.join(process.cwd(), 'src/db/migrations/004_horizon_sse_cursors.sql'),
400+
path.join(process.cwd(), 'src/db/migrations/006_horizon_sse_cursors.sql'),
401401
'utf8',
402402
);
403403
// Run migration on a dedicated client with search_path locked so the
@@ -444,7 +444,7 @@ test(
444444
try {
445445
await pool.query(`CREATE SCHEMA "${schema}"`);
446446
const migration = await readFile(
447-
path.join(process.cwd(), 'src/db/migrations/004_horizon_sse_cursors.sql'),
447+
path.join(process.cwd(), 'src/db/migrations/006_horizon_sse_cursors.sql'),
448448
'utf8',
449449
);
450450
// Run migration on a dedicated client with search_path locked so the

0 commit comments

Comments
 (0)