fix: v0.19.1 audit DB upgrade safety, run migrations before SCHEMA_SQL#93
Conversation
Opening an existing audit DB at any schema version older than the current one crashed on first MCP-server boot with no such column: tenant_id. The init path ran SCHEMA_SQL before migrations, and SCHEMA_SQL contains indexes on columns that later migrations add. Init now runs migrations from the stored version (or from v0 for pre-versioned DBs that have no audit_meta row yet) before running SCHEMA_SQL idempotently. Fresh DBs continue to use the existing single-pass SCHEMA_SQL path. Tests added for the v0 (pre-versioning), v1, and current-version open paths. PyPI 0.19.0 + npm 0.19.0 lockstep maintained.
📝 WalkthroughWalkthroughThis patch release (0.19.1) fixes MCP-server startup crashes when opening existing audit databases with older schema versions. The fix reorders initialization to run schema migrations based on stored version before applying SCHEMA_SQL, with special handling for pre-versioned databases, while preserving single-pass behavior for fresh databases. ChangesAudit DB Upgrade Safety
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_sqlite_backend.py (1)
222-237: ⚡ Quick winAssert the post-migration
SCHEMA_SQLartifacts too.
_assert_current()only verifies the version row and migrated columns. That still passes if the final idempotentSCHEMA_SQLcall is dropped, so upgraded DBs could silently missgdpr_redactions,api_keys, oridx_tenant_id. Please assert those as well in this helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_sqlite_backend.py` around lines 222 - 237, The helper _assert_current currently only checks schema_version and migrated columns but must also assert that final SCHEMA_SQL artifacts exist; update _assert_current to, after connecting to the DB (in the same function), verify presence of the gdpr_redactions and api_keys tables (or expected objects created by SCHEMA_SQL) and that the index idx_tenant_id exists (use PRAGMA table_info / sqlite_master queries to confirm table and index names), and raise assertions if any are missing so the post-migration SCHEMA_SQL call cannot be omitted silently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_sqlite_backend.py`:
- Around line 222-237: The helper _assert_current currently only checks
schema_version and migrated columns but must also assert that final SCHEMA_SQL
artifacts exist; update _assert_current to, after connecting to the DB (in the
same function), verify presence of the gdpr_redactions and api_keys tables (or
expected objects created by SCHEMA_SQL) and that the index idx_tenant_id exists
(use PRAGMA table_info / sqlite_master queries to confirm table and index
names), and raise assertions if any are missing so the post-migration SCHEMA_SQL
call cannot be omitted silently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 830b5add-1ad0-44c0-8eb6-a2ed2b11b7aa
📒 Files selected for processing (6)
CHANGELOG.mdclients/ts/package.jsonpyproject.tomlsrc/vaara/__init__.pysrc/vaara/audit/sqlite_backend.pytests/test_sqlite_backend.py
Summary
no such column: tenant_id.SCHEMA_SQLran before migrations, andSCHEMA_SQLcontains indexes on columns that later migrations add.audit_metarow yet) before runningSCHEMA_SQLidempotently. Fresh DBs continue to use the existing single-pass path.Why this is a patch, not minor
@vaara/client0.19.0 → 0.19.1 per release-PR rule.Test plan
TestSchemaUpgrade: pre-versioning (v0), v1, and current-version reopen-is-idempotent.Summary by CodeRabbit
Bug Fixes
Tests