Fix post-migration-first: update tests, Dockerfile, and docs to match new schema approach#34
Merged
Fix post-migration-first: update tests, Dockerfile, and docs to match new schema approach#34
Conversation
Merged
Co-authored-by: alvagante <283804+alvagante@users.noreply.github.com>
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
[WIP] [082] Consolidate backend SQLite schema management
Fix post-migration-first: update tests, Dockerfile, and docs to match new schema approach
Mar 14, 2026
alvagante
approved these changes
Mar 14, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aligns tests, Docker packaging, and internal docs with the repository’s migration-first SQLite schema approach (after removal of the legacy schema.sql/hybrid layout).
Changes:
- Updated DB test setup to use migration files (and in one case
MigrationRunner) instead of the removedschema.sql. - Adjusted migration/index expectations in tests to match the actual set of migrations and indexes.
- Updated Docker runtime image to copy only SQL migrations, and refreshed the
.kiroprompt’s schema file structure section.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| backend/test/performance/database-performance.test.ts | Uses MigrationRunner to apply all migrations for in-memory performance setup. |
| backend/test/database/performance-test.ts | Switches schema bootstrap to 000_initial_schema.sql (but currently not enough for ExecutionRepository). |
| backend/test/database/migration-integration.test.ts | Updates expectations to include migration 000. |
| backend/test/database/index-verification.test.ts | Removes expectations for non-existent composite indexes; validates real junction-table indexing. |
| backend/test/database/006_add_batch_executions.test.ts | Bootstraps from migration 000 instead of deleted schema.sql. |
| Dockerfile | Copies only src/database/migrations into runtime image. |
| .kiro/database-cleanup-prompt.md | Updates “Current File Structure” and fixes a truncated checklist line. |
Comments suppressed due to low confidence (1)
backend/test/database/performance-test.ts:34
ExecutionRepository.create()insertsbatch_id/batch_positioncolumns unconditionally, but this script only applies migration 000, which does not add those columns (they’re introduced in migration 006). As a result, the firstrepo.create()call will fail with “no such column: batch_id”. Apply migrations through 006 (preferably viaMigrationRunner) or update the setup to create the latestexecutionsschema before usingExecutionRepository.
// Load and execute migration 000 (initial schema)
const schemaPath = join(__dirname, '../../src/database/migrations/000_initial_schema.sql');
const schema = readFileSync(schemaPath, 'utf-8');
// Split by semicolon and execute each statement
const statements = schema.split(';').filter(s => s.trim().length > 0);
for (const statement of statements) {
await runAsync(db, statement);
}
Comment on lines
14
to
+19
| backend/src/database/ | ||
| ├── schema.sql # Base schema: executions table | ||
| ├── rbac-schema.sql # Base schema: RBAC tables (DUPLICATE of 001) | ||
| ├── audit-schema.sql # ⚠️ ORPHANED - never loaded, duplicates 004 | ||
| ├── migrations.sql # ⚠️ ORPHANED - never loaded, superseded | ||
| ├── DatabaseService.ts # Loads schema.sql + rbac-schema.sql, then runs migrations | ||
| ├── DatabaseService.ts # Runs only MigrationRunner (migration-first approach) | ||
| ├── MigrationRunner.ts # Runs numbered migrations from migrations/ | ||
| └── migrations/ | ||
| ├── 001_initial_rbac.sql # Creates RBAC tables (duplicates rbac-schema.sql) | ||
| ├── 000_initial_schema.sql # Initial schema: executions + revoked_tokens tables | ||
| ├── 001_initial_rbac.sql # Creates RBAC tables |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After consolidating to a migration-first schema approach, several artifacts still referenced the removed
schema.sqlbase file and the old hybrid layout. This PR addresses review feedback from #32.Test fixes
Three test files loaded the deleted
schema.sqldirectly for in-memory DB setup:006_add_batch_executions.test.ts— loadmigrations/000_initial_schema.sqlinsteaddatabase-performance.test.ts— useMigrationRunnerto apply all migrations (removes the manualALTER TABLEhack for batch columns)performance-test.ts— loadmigrations/000_initial_schema.sqlinsteadTwo tests also had stale expectations from before migration 000 existed:
migration-integration.test.ts— expected 6 migrations (001–006); updated to 7 (000–006)index-verification.test.ts— expected composite indexes (idx_user_roles_composite, etc.) that were never added to any migration; updated to verify the single-column junction table indexes that do existDockerfile
Changed the runtime
COPYfrom the entiresrc/database/tree (TypeScript sources included) to just the migrations directory:Documentation
Updated
.kiro/database-cleanup-prompt.md"Current File Structure" section to reflect the post-cleanup state (removed references to deletedschema.sql,rbac-schema.sql,audit-schema.sql,migrations.sql) and fixed a truncated sentence.✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.