feat(data): add fail-closed schema migration ledger - #500
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough스키마 세대와 Changes스키마 마이그레이션 상태 관리
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟠 High · up to Startup can currently treat a valid legacy or canonical schema with an unexpected extra table as trusted, allowing an unverified database state to pass migration checks and potentially serve requests under the wrong schema assumptions. This concrete correctness risk should be fixed before merge, and the required independent approval after the latest push is still missing. Sequence Diagram(s)sequenceDiagram
participant DatabaseStartup
participant schema_migration
participant SQLiteDatabase
participant QueryLayer
DatabaseStartup->>schema_migration: inspectSchemaBootstrapState(database)
schema_migration->>SQLiteDatabase: 스키마와 schema_migrations 조회
SQLiteDatabase-->>schema_migration: 스키마 세대와 원장 상태
schema_migration-->>DatabaseStartup: 검증된 상태 또는 SchemaMigrationStateError
DatabaseStartup->>schema_migration: ensureSchemaMigrationState(database)
schema_migration->>SQLiteDatabase: 세대 원장 레코드 멱등 삽입
DatabaseStartup->>QueryLayer: canonical 세대 초기화
QueryLayer-->>DatabaseStartup: 현재 세대 미지원 오류
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 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 |
|
@opencode-agent @cwl-noema-review Please perform a fresh independent formal review of exact current head |
Pull request was converted to draft
|
@opencode-agent Request independent review for exact current head |
|
@coderabbitai review |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent Fresh exact-head review requested for |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
opencode-agent
Bot
added
area: data
|
Caution Review failedAn error occurred during the review process. Please try again later. 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough스키마 세대와 Changes스키마 마이그레이션 상태 관리
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔵 Low · up to The PR adds durable schema-generation checks and transactional first bootstrap, improving protection against partial or mixed databases. A stale restore with conflicting history may still receive schema mutations before startup rejects it, and simultaneous first starts may contend without a demonstrated wait/retry policy; the service remains fail-closed, so this is mergeable with explicit owner awareness of these bounded risks. Sequence Diagram(s)sequenceDiagram
participant ApplicationStartup
participant db.mjs
participant schema_migration
participant SQLiteDatabase
ApplicationStartup->>db.mjs: 데이터베이스 모듈 초기화
db.mjs->>schema_migration: inspectSchemaBootstrapState(database)
schema_migration->>SQLiteDatabase: 카탈로그와 schema_migrations 조회
SQLiteDatabase-->>schema_migration: 스키마 세대와 원장 상태
alt 빈 데이터베이스
db.mjs->>schema_migration: runAtomicLegacySchemaBootstrap(database, bootstrapSql)
schema_migration->>SQLiteDatabase: BEGIN IMMEDIATE 및 레거시 DDL
SQLiteDatabase-->>schema_migration: COMMIT 또는 DDL 오류
else canonical 세대
db.mjs->>schema_migration: ensureSchemaMigrationState(database)
schema_migration-->>db.mjs: canonical_ready
db.mjs-->>ApplicationStartup: SchemaMigrationStateError
else 레거시 세대
db.mjs->>schema_migration: ensureLegacyCompatibilityColumns(database)
schema_migration->>SQLiteDatabase: PRAGMA table_info 및 필요한 ALTER TABLE
db.mjs->>schema_migration: ensureSchemaMigrationState(database)
schema_migration-->>db.mjs: legacy_ready
db.mjs-->>ApplicationStartup: 시작 완료
end
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 41.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 8 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 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 |
Dismissed as stale predecessor-head evidence after a fresh exact-state refetch. This OpenCode CHANGES_REQUESTED review is explicitly bound to ad4166b and its then-failing coverage-evidence run 32123439123. The live contributor head is 6bab0a5 on protected develop@2c328875e00e86537df3e965170be80532571cad; its current repository workflow labels are terminal success, while known exact-head checkout/control repairs remain separately required. Dismissal removes predecessor review state only; it does not approve this PR or manufacture current-head evidence. A qualifying independent current-head/latest-push approval and corrected exact-head gates remain required.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
1 participant
Buyer and operator impact
Issue #433 requires a zero-downtime rename of ScopeWeave's legacy single-word SQLite tables. Before any rename can be safe, startup needs durable migration identity and must refuse to serve a database left half old-schema and half canonical-schema after an interrupted cutover or stale restore.
This bounded prerequisite adds that fail-closed migration boundary without renaming production tables yet. It also replaces the earlier blanket
ALTER TABLEexception swallowing with catalog-first compatibility-column validation, makes first bootstrap transactional, and preserves additive legacy schema self-healing for compliant existing databases.Exact current identity and bounded delta
develop@2c328875e00e86537df3e965170be80532571cad;6bab0a547f484307073d0e3f7e9c3a15963ed1fe;refactor/schema-migration-ledger-433;developas the exact merge base;CHANGELOG.md;docs/doctoring/schema-migration-ledger.md;package.json;server/db.mjs;server/schema_migration.mjs;tests/unit/coverage-script-contract.test.mjs;tests/unit/legacy-column-migration.test.mjs;tests/unit/schema-migration-concurrent-startup.test.mjs;tests/unit/schema-migration-ledger-contract.test.mjs;tests/unit/schema-migration-state.test.mjs;tests/unit/schema-migration-streaming.test.mjs.Any contributor-head or protected-base movement invalidates revision-sensitive evidence until the exact revisions, semantic delta, reviews, checks and checkout attestations are freshly revalidated.
Current production contract
schema_migrationsledger idempotently;audit_log,api_tokens,project_revisions,project_seen,share_tokens, andwebhook_deliveries) rather than misclassifying them as migration-generation objects;BEGIN IMMEDIATE/commit/rollback so an interrupted initial CREATE sequence does not leave an accidentally serviceable partial generation;users.token_version,projects.archived, andprojects.methodologydeclarations before accepting them, adds them only when absent, and propagates real compatibility-DDL failures;server/db.mjsandserver/schema_migration.mjsin the canonical c8 owned-production coverage set.TDD and review-driven repair chronology
The branch began with the schema-state regression before implementation. Subsequent current-tree repairs were test-first or directly coupled to realistic regressions:
6bab0a547f484307073d0e3f7e9c3a15963ed1fe(fix(data): preserve additive schema bootstrap), which repairs the discovered regression where an early return for an established legacy generation skipped idempotent creation of non-rename tables/indexes.The current head therefore preserves both safety properties: a brand-new interrupted bootstrap rolls back, while a complete legacy database can still receive additive idempotent schema objects.
Current unresolved Devin threads are informational operating-boundary observations rather than demonstrated current defects: canonical-state ledger recording occurs before the intentional unsupported-canonical fail-close; strict compatibility-column declarations may reject hand-modified self-hosted schemas by design; a future dual-generation expand executor needs its own controlled transition; genuine additive self-heal can still contend for a write lock when it actually creates a missing object; ledger creation/default-text checks depend on documented SQLite behavior; and the compatibility-column step intentionally runs only on the legacy path. They remain visible rather than being resolved for status cleanliness.
A CodeRabbit suggestion to reject every table outside the ten-table rename set is not adopted because #433 explicitly preserves existing compliant application tables outside that rename set; treating those tables as migration corruption would make valid ScopeWeave databases unstartable. The bounded catalog sentinel exists to distinguish a non-empty unrelated database from a pristine bootstrap, not to outlaw ScopeWeave's other owned tables.
Current exact-head evidence
For exact contributor head
6bab0a547f484307073d0e3f7e9c3a15963ed1fe, all six currently associated pull-request workflow runs are terminal GitHub-success:32502265208;32502265219;32502265195;32502265327;32502265731; and32502265282.These success labels are useful product/regression evidence but are not promoted to immutable contributor-head merge authority while the known workflow checkout defects remain unresolved. ScopeWeave #523 owns repository-native exact-head Server Tests/coverage and Dependency Review integrity.
ContextualWisdomLab/.github#1222owns the centrally reusable SAST/Security exact-head repair through the central repository's existing dedicated writer path.The formal OpenCode
CHANGES_REQUESTEDreview is bound to predecessor headad4166b0652ecf204c4876770b69265ab9c6b2d8and its then-failing coverage evidence; it does not transfer to current head6bab0a5.... CodeRabbit/Devin COMMENTED reviews are supplementary rather than qualifying independent approval. No qualifying independent current-head/last-push approval is present.Standards, recovery and remaining #433 scope
docs/doctoring/schema-migration-ledger.mdrecords the pre-bootstrap boundary, catalog-first compatibility validation, monotonic ledger semantics, transactional first-bootstrap behavior, rollback/recovery constraints, and APA 7 references to ISO/IEC 9075-1:2023 and current SQLite ALTER TABLE/PRAGMA documentation.A partial rename or schema/ledger generation mismatch is never auto-healed. Later #433 slices still own the reviewed expand/verify/contract rename executor, complete foreign-key/index/trigger/view/query verification, populated-database migration/restart/kill-point acceptance, PostgreSQL adapter parity, compatibility-view decisions where genuinely needed, restore rehearsal, and final canonical-object enforcement. This PR does not close #433 and does not claim canonical schema shipment.
Merge gate
Do not merge or enable auto-merge until this unchanged exact head remains reconciled to freshly resolved protected
develop, the repository and central exact-head evidence controls are protected-shipped and regenerated, every applicable CI/browser/owned statement-branch-function-line coverage/docstring/CodeQL/SAST/security/dependency/supply-chain/package/SBOM/provenance/migration-recovery gate is substantively terminal-passing, valid unresolved current-head defects are zero, and a qualifying independent current-head/last-push approval genuinely exists under live rulesets.Pending, queued, skipped-required, cancelled, absent, neutral-required, failed, stale, predecessor, synthetic-only, status-only, author-only, model-only, rate-limited, or infrastructure-only evidence is non-passing.
Refs #433
Summary by CodeRabbit
개선 사항
문서