feat(migration): freeze reversible cutover protocol - #677
Conversation
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Introduce the frozen migration manifest contract with exact-key parsing, bounded identifier and hash validation, ISO timestamp round-trip checks, cross-field phase invariants, and a deterministic canonical SHA-256 seal computed over the payload with the checksum field omitted. Checkpoints and reports are canonically ordered before hashing and before freezing, so the seal is invariant to input collection order. Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
…k lineage Checkpoint ordinals are scoped to their domain, so two distinct domains may share an ordinal. Remove the incorrect global ordinal uniqueness check while keeping domain uniqueness (latest checkpoint per domain). Retain the returnPhase sealed by prepare-rollback once publish-rollback settles: rolled-back manifests now require both a rollback mode and a sealed return phase instead of forcing the return phase back to null. Cover the remaining protocol branches, including valid rolling-back and rolled-back manifests, the legal effect table rows, canonical cycle rejection, comparator equality, identifier length bounds, and createMigrationManifest invalid-input rejection. Fix the missing-key test to delete the key rather than assign undefined. Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
PR Summary by QodoFreeze reversible migration protocol with sealed manifests and durable journal
AI Description
Diagram
High-Level Assessment
Files changed (17)
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5797f91991
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Freezes the reversible SQLite→PostgreSQL cutover contract by introducing a strict, checksum-sealed migration manifest state machine plus a crash-recoverable, immutable revision journal, while extracting shared $HOME lock-topology validation and threading explicit UID policy through filesystem durability primitives.
Changes:
- Add
src/migration/protocol.ts(strict manifest schema, canonical SHA-256 sealing, legal reducers, and deterministic recovery classification) andsrc/migration/manifest-store.ts(immutable revisions + head CAS + bounded one-hop recovery). - Extract retained
$HOME/parent descriptor authentication intosrc/storage/home-lock-topology.tsand refactor backend publication to consume it. - Update docs/changeset and Codecov ownership taxonomy; add exhaustive unit tests for protocol/store and new UID-policy seams.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/security-files.test.ts | Adds regressions for expected-UID threading through directory assertions and durable writes. |
| test/migration/protocol.test.ts | Exhaustive protocol schema/reducer/recovery classification coverage. |
| test/migration/manifest-store.test.ts | End-to-end durable store, CAS, crash boundary, tamper, and recovery tests. |
| test/migration/index.test.ts | Verifies the migration barrel export surface. |
| test/codecov-config.test.ts | Updates component contract (29→30) and production ownership counts (191→195). |
| test/backend-publication.test.ts | Adds regressions for extracted HOME topology helper behavior + restoration on failure. |
| src/storage/home-lock-topology.ts | New shared retained-descriptor HOME topology validator/restorer. |
| src/storage/backend-publication.ts | Refactors HOME topology logic to shared helper without intended behavior changes. |
| src/security-files.ts | Threads explicit expectedUid into retained directory assert + sync + durable publication reads. |
| src/migration/protocol.ts | New sealed manifest protocol types, parsing, reducers, and recovery classifier. |
| src/migration/manifest-store.ts | New private immutable manifest revision journal with head CAS and bounded recovery. |
| src/migration/index.ts | New barrel export for migration protocol + store. |
| docs/superpowers/plans/2026-08-12-issue-621-migration-protocol.md | Implementation plan documentation for issue #621. |
| docs/README.md | Links the new operator guide. |
| docs/migration-cutover.md | Operator guide for the protocol, layout, and recovery semantics. |
| codecov.yml | Adds unit-migration-cutover component and assigns home-lock-topology ownership. |
| .changeset/freeze-reversible-migration-protocol.md | Minor release note for the new protocol foundation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/migration/protocol.ts:188
isSafeNonNegativeIntegertreats-0as a valid integer (Number.isSafeInteger(-0)and-0 >= 0), which allows manifests withrevision: -0and other integer fields to pass parsing. Because JSON serialization and your canonical hashing normalize-0to0, this can produce non-idempotent values (e.g., a manifest accepted with-0will round-trip/persist as0, socreate()/read()can disagree under strict equality). Consider rejecting-0(or normalizing it to0) for all protocol integer fields to keep the in-memory object stable across serialization and storage.
function isSafeNonNegativeInteger(value: unknown): value is number {
return typeof value === "number" && Number.isSafeInteger(value) && value >= 0;
}
src/migration/manifest-store.ts:49
isSafeRevisionaccepts-0for head revisions (Number.isSafeInteger(-0)and-0 >= 0). Since head/manifest content is canonicalized via string serialization (which normalizes-0to0), accepting-0here can lead to non-canonical or non-idempotent in-memory values vs persisted bytes. It’s safer to either reject-0or normalize it to0when validating/parsing head and revision numbers.
function isSafeRevision(revision: unknown): revision is number {
return typeof revision === "number" && Number.isSafeInteger(revision) && revision >= 0;
}
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
| [root, migrations, generation, revisions], | ||
| this.#expectedUid, | ||
| () => { | ||
| const headPath = migrationManifestHeadPath(generationId, this.#homeDir); |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 777b80e26d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const source = parseWitness(input.source, "source", "invalid-input"); | ||
| const destination = parseWitness(input.destination, "destination", "invalid-input"); |
There was a problem hiding this comment.
Reject manifests unless the source witness is sqlite and the destination witness is postgresql. These calls validate each backend only against the shared two-value enum, so reversed or same-backend pairs are accepted and can be durably stored even though docs/migration-cutover.md:35-37 defines their directional roles; later copy and cutover code would consequently operate against incorrectly typed endpoints.
Useful? React with 👍 / 👎.
| consumeBoundedRegularFile(join(revisionDirectory, scratch), { | ||
| allowedRoot: revisionDirectory, | ||
| maxBytes: MAX_MANIFEST_BYTES, | ||
| expectedUid, | ||
| allowedModes: [0o600], |
There was a problem hiding this comment.
When a crash leaves a truncated scratch, or the expected scratch pathname contains unrelated bytes, this consumes it based only on its filename, ownership, mode, and link count. The retry therefore erases ambiguous evidence and replaces it instead of failing closed as required for immutable publication; bind the scratch's raw bytes to the candidate's canonical content before unlinking it.
Useful? React with 👍 / 👎.
| requireSingleLink: true, | ||
| }); | ||
| } | ||
| beforeContentPublication(revisionDirectory); |
There was a problem hiding this comment.
If the process or host crashes after the scratch is unlinked but before the replacement writer synchronizes this directory, the deletion is unordered with the new temporary entry. Recovery may then observe both the resurrected old scratch and a new scratch or manifest on different inodes and permanently reject the revision as ambiguous; fsync the revision directory immediately after consuming the scratch and before this publication callback/write boundary.
Useful? React with 👍 / 👎.
Signed-off-by: Bernardo Donadio <bcdonadio@bcdonadio.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1aba84e911
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| atomicWritePrivateFileDurable(headPath, migrationManifestHeadContent(nextHead), { | ||
| expectedUid: this.#expectedUid, | ||
| expectedContentSha256: sha256(current.headContent), | ||
| maxExistingBytes: MAX_MANIFEST_BYTES, |
There was a problem hiding this comment.
When another same-UID process or operator rewrites head.json after atomicWritePrivateFileDurable performs its initial expected-content check but before publication, this update still overwrites those new bytes. The helper checks the hash only at src/security-files.ts:1074-1089 and later performs an unconditional renameSync at line 1140, so supplying expectedContentSha256 here does not provide the documented exact-head CAS and can erase conflicting journal evidence; bind the replacement atomically to the authenticated head identity or revalidate it immediately before a no-clobber publication.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/migration/manifest-store.ts:1151
recover()will currently throw a raw ENOENT if the generation directory does not exist, instead of returning a deterministic protocol error (e.g., "no recoverable genesis"). This makes recovery behavior depend on host filesystem error strings/codes and diverges from the documented sanitizedMigrationProtocolErrorreasons.
const generation = migrationManifestGenerationDirectory(generationId, this.#homeDir);
const root = lcmRoot(this.#homeDir);
const migrations = migrationRoot(this.#homeDir);
const revisions = join(generation, "revisions");
return withAuthenticatedDirectories(
src/migration/manifest-store.ts:1005
#readOptionalStatetreats a missing generation directory as a raw filesystem ENOENT (fromopenPrivateDirectory) rather than a protocol-level "head is absent" state. This makes the publicread()/update()error contract depend on filesystem error details for the common case of an unknown generation ID.
const generation = migrationManifestGenerationDirectory(generationId, this.#homeDir);
const root = lcmRoot(this.#homeDir);
const migrations = migrationRoot(this.#homeDir);
const revisions = join(generation, "revisions");
return withAuthenticatedDirectories(
Summary
Freeze the reversible SQLite/PostgreSQL migration protocol before any data-movement or cutover implementation begins. This adds strict checksum-sealed manifests, legal effect reducers, deterministic recovery classification, and a private crash-recoverable immutable revision journal.
Motivation / Why
Epic #92 needs one durable, fail-closed contract shared by planning, copy, reconciliation, activation, and rollback. Without this foundation, later stages could disagree about legal transitions, evidence requirements, crash recovery, or which persisted state is authoritative.
Type of change
Release-note title
feat(migration): freeze reversible cutover protocol
What changed
0700/0600modes, immutable checksum-named revisions, exact-head compare-and-swap, revision exhaustion handling, and bounded one-hop recovery.src/migration/index.tsand add exclusive Codecov ownership (unit-migration-cutover, 30 components, 195 production TypeScript files).Testing done
npm run test:cinpm run typechecknpm run lintgit diff --checkxai/grok-4.6max was attempted but provider-rejected its reasoning effort, so the required Luna max fallback APPROVED; Opus second pass APPROVED.Risk / rollout
Related issues
Closes #621
Copilot review focus areas
Checklist
implicit any— all public seams are typedcollectStats()usenpm run test:cipasses at 100% coverage