TML-2716: route all read commands through ContractSpaceAggregate#644
Merged
Conversation
Land the slice-1 (TML-2715) retro lesson across the drive calibration surfaces: dispatch validation must mirror CI's job matrix, not just typecheck. PR #626 reported green then failed CI on lint (formatter + unused import in a test file) and on behind-main test drift. - failure-modes.md: new F11 (dispatch green / CI red). - calibration/dod.md: pnpm lint becomes an always-run dispatch gate; typecheck must cover the package test tsconfig; sync origin/main before final slice validation + push. - retro/findings.md: trial finding for the 2026-05-30 miss. Signed-off-by: Will Madden <madden@prisma.io>
Slice 2 re-points every CLI command that reads migration packages onto the slice-1 ContractSpaceAggregate via a shared buildReadAggregate helper, then deletes both hand-rolled loaders (loadMigrationPackages + enumerateMigrationSpaces). Scope settled with operator: grounding found loadMigrationPackages had six callers (graph, log, db-sign, db-update, migration-plan, ref), not just list/graph/log, so all seven package-reading commands fold in to make the helper deletable. Plan: 3 dispatches (helper+graph; fan-out+ delete loadMigrationPackages; list+delete enumerateMigrationSpaces). Signed-off-by: Will Madden <madden@prisma.io>
Extract the migration-status offline aggregate assembly into a shared helper with relocated appContractShellForAggregateLoad and loadContractRawSafely. No integrity gate; unit tests cover readable and missing contract paths. Signed-off-by: Will Madden <madden@prisma.io>
Replace loadMigrationPackages, readRefs, and readContractEnvelope with one offline aggregate build. Graph, refs, and contract marker come from aggregate.app; top-level migrationsDir matches migration status. Signed-off-by: Will Madden <madden@prisma.io>
D1 (buildReadAggregate + migration graph) reviewed SATISFIED. Land the two slice-level decisions the reviewer surfaced: golden --json tests required before AC-2 close (incl. backfilling migration graph, which shipped without one), and AC-2 "identical output" recorded as a happy-path guarantee (corruption-path refs now tolerant by design). Signed-off-by: Will Madden <madden@prisma.io>
Re-point migration log, db sign, db update, migration plan, and ref package reads through buildReadAggregate and aggregate.app facets. Delete loadMigrationPackages and update the control-api doc reference. Signed-off-by: Will Madden <madden@prisma.io>
Pin migration graph, log, ref set, and db sign JSON output. Add db update --to dry-run golden and align ref/plan tests with buildReadAggregate config. Signed-off-by: Will Madden <madden@prisma.io>
Re-point migration list through buildReadAggregate and a mapper that preserves on-disk space membership. Move refsByContractHash to refs and delete enumerateMigrationSpaces from migration-tools. Signed-off-by: Will Madden <madden@prisma.io>
Exercise list via loadContractSpaceAggregate in unit tests, add --json golden coverage, and relocate resolveRefsByContractHash tests to refs. Signed-off-by: Will Madden <madden@prisma.io>
Sync with origin/main before adopt-read-commands slice close-out. Signed-off-by: Will Madden <madden@prisma.io> # Conflicts: # drive/calibration/failure-modes.md # packages/1-framework/3-tooling/cli/test/commands/migration-list.test.ts
Export HEAD_REF_NAME for extension-space list decorations, fold headRef into the refs-by-hash map, add doUnmock guards on json golden suites, and wire main's per-space graph test through runMigrationListFromDisk. Signed-off-by: Will Madden <madden@prisma.io>
Contributor
📝 WalkthroughWalkthroughThis PR consolidates CLI migration data loading through a new read-only aggregate loader while refactoring ChangesCLI Aggregate-Based Data Loading Refactor
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
🚥 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
@prisma-next/extension-author-tools
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/extension-cipherstash
@prisma-next/middleware-cache
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/extension-postgis
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/cli-telemetry
@prisma-next/emitter
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.
Inline comments:
In `@drive/calibration/dod.md`:
- Line 82: The markdown link in the "Sync `origin/main` before the final
validation + push" checklist entry points to the wrong failure-mode anchor
(currently
`#f11-dispatch-reports-validation-green-but-ci-is-red-dispatch-gates-didnt-mirror-ci`);
update that link to the correct anchor for F14 (replace the `#f11...` fragment
with the F14 fragment `#f14-...` so the link targets § F14 in failure-modes.md),
ensuring the visible text and surrounding checklist remain unchanged.
- Line 18: Update the cross-reference in drive/calibration/dod.md so the
failure-mode link points to the newly added F14 section instead of F11: change
the reference text/URL segment that reads "failure-modes.md § F11" to
"failure-modes.md § F14" (i.e., point to the F14 anchor added in
failure-modes.md) so the sentence about lint being a separate CI job links to
the correct failure-mode entry.
In `@drive/retro/findings.md`:
- Line 11: Update the incorrect cross-reference that points to "failure-modes.md
§ F11" so it instead points to the newly added failure mode F14; search for
occurrences of the string "failure-modes.md § F11" (including the references
inside drive/retro/findings.md and drive/calibration/dod.md) and replace them
with "failure-modes.md § F14" so the drive/retro/findings and dod references
correctly link to the F14 section.
In
`@packages/1-framework/3-tooling/cli/src/utils/contract-space-aggregate-loader.ts`:
- Around line 339-357: The control stack and family bootstrap
(createControlStack(config) and config.family.create(stack)) are executed before
the error-handling try block so failures throw instead of being returned as a
CliStructuredError; move the creation of stack and familyInstance (and
deserializeContract closure if you prefer) into the existing try that surrounds
the aggregate load so any exceptions from createControlStack or
config.family.create are caught and converted into the Result/CliStructuredError
path used by buildReadAggregate, preserving the appContractShell fallback logic
(appContractShell/appContractForLoad) and keeping the outer API that returns a
Result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: b6a4db92-17c5-456d-a76d-298831513e17
⛔ Files ignored due to path filters (2)
projects/migration-store/plan.mdis excluded by!projects/**projects/migration-store/slices/adopt-read-commands/spec.mdis excluded by!projects/**
📒 Files selected for processing (31)
drive/calibration/dod.mddrive/calibration/failure-modes.mddrive/retro/findings.mdpackages/1-framework/3-tooling/cli/src/commands/db-sign.tspackages/1-framework/3-tooling/cli/src/commands/db-update.tspackages/1-framework/3-tooling/cli/src/commands/migration-graph.tspackages/1-framework/3-tooling/cli/src/commands/migration-list.tspackages/1-framework/3-tooling/cli/src/commands/migration-log.tspackages/1-framework/3-tooling/cli/src/commands/migration-plan.tspackages/1-framework/3-tooling/cli/src/commands/migration-status.tspackages/1-framework/3-tooling/cli/src/commands/ref.tspackages/1-framework/3-tooling/cli/src/control-api/types.tspackages/1-framework/3-tooling/cli/src/utils/command-helpers.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-aggregate-loader.tspackages/1-framework/3-tooling/cli/src/utils/migration-space-list-from-aggregate.tspackages/1-framework/3-tooling/cli/test/commands/db-update-read-aggregate-json-golden.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-list-json-golden.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-list.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-plan-command.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-tamper.test.tspackages/1-framework/3-tooling/cli/test/commands/read-commands-json-golden.test.tspackages/1-framework/3-tooling/cli/test/commands/ref.test.tspackages/1-framework/3-tooling/cli/test/utils/build-read-aggregate.test.tspackages/1-framework/3-tooling/migration/package.jsonpackages/1-framework/3-tooling/migration/src/enumerate-migration-spaces.tspackages/1-framework/3-tooling/migration/src/exports/enumerate-migration-spaces.tspackages/1-framework/3-tooling/migration/src/exports/refs.tspackages/1-framework/3-tooling/migration/src/refs.tspackages/1-framework/3-tooling/migration/test/enumerate-migration-spaces.test.tspackages/1-framework/3-tooling/migration/test/refs.test.tspackages/1-framework/3-tooling/migration/tsdown.config.ts
💤 Files with no reviewable changes (6)
- packages/1-framework/3-tooling/migration/src/enumerate-migration-spaces.ts
- packages/1-framework/3-tooling/migration/src/exports/enumerate-migration-spaces.ts
- packages/1-framework/3-tooling/migration/test/enumerate-migration-spaces.test.ts
- packages/1-framework/3-tooling/migration/tsdown.config.ts
- packages/1-framework/3-tooling/cli/src/utils/command-helpers.ts
- packages/1-framework/3-tooling/migration/package.json
wmadden
reviewed
May 30, 2026
| return ok(loaded.value); | ||
| } | ||
|
|
||
| export function appContractShellForAggregateLoad(args: { |
Contributor
There was a problem hiding this comment.
What on earth does this method name mean?
| readonly targetId: string; | ||
| readonly targetFamily: string; | ||
| }): Contract { | ||
| return blindCast<Contract, 'offline read aggregate without contract.json'>({ |
Contributor
There was a problem hiding this comment.
This reason does not adequately explain why this cast exists, it just names it.
Comment on lines
+317
to
+322
| /** | ||
| * Offline tolerant {@link ContractSpaceAggregate} assembly for read-only | ||
| * CLI commands. No integrity gate — callers query `aggregate.app` (or | ||
| * other facets) without re-reading `migrations/`. | ||
| */ | ||
| export async function buildReadAggregate( |
Contributor
There was a problem hiding this comment.
Isn't the aggregate always loaded from disk - i.e. offline?
Comment on lines
+75
to
+85
| const migrations: MigrationListEntry[] = member.packages | ||
| .map((pkg) => ({ | ||
| dirName: pkg.dirName, | ||
| from: pkg.metadata.from, | ||
| to: pkg.metadata.to, | ||
| migrationHash: pkg.metadata.migrationHash, | ||
| operationCount: pkg.ops.length, | ||
| createdAt: pkg.metadata.createdAt, | ||
| refs: refsByHash.get(pkg.metadata.to) ?? [], | ||
| providedInvariants: pkg.metadata.providedInvariants, | ||
| })) |
Contributor
There was a problem hiding this comment.
This map looks like a smell. Why do we maintain two similar but not identical representations of the same data structure? Can we consolidate and delete MigrationListEntry? Or otherwise, this mapping seems like it belongs in the consumer, migration list.
Contributor
There was a problem hiding this comment.
What is this file? These look like centralized junk methods that convert the canonical contract-space aggregate into other, worse data structures. Looks like a backwards compatibilitiy shim
…t stand-in naming Move the CLI-only migration list presentation cluster (MigrationListEntry / MigrationSpaceListEntry / MigrationListResult + classifyMigrationListGraphTopology + MigrationListGraphTopology + MigrationEdgeKind, plus its test) out of @prisma-next/migration-tools into cli/src/utils/formatters, trimming the two now unused package exports. Inline the aggregate->view mapper into the migration list command and delete migration-space-list-from-aggregate.ts, so no second representation of migration state lives in the shared package. Rename appContractShellForAggregateLoad -> appContractStandInFromIdentity with a cast reason that states read commands consume only storage.storageHash + target (never models), and move buildReadAggregate control-stack/family bootstrap inside the Result error boundary so bootstrap failures surface as CliStructuredError. Fix three F11 -> F14 failure-mode cross-reference links. Records the falsified assumption + Option-A relocation decision in projects/migration-store. Signed-off-by: Will Madden <madden@prisma.io>
…ommands # Conflicts: # drive/retro/findings.md # packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts # packages/1-framework/3-tooling/cli/test/commands/migration-plan-command.test.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/1-framework/3-tooling/cli/test/commands/migration-plan-command.test.ts (1)
355-373: 💤 Low valueDrop the now-stale
command-helpersunmock and comment.Since this file no longer registers
vi.mock('../../src/utils/command-helpers', …), thevi.doUnmock('../../src/utils/command-helpers')at Line 363 is dead, and the comment at Line 358 listingcommand-helpers.loadMigrationPackagesas a leaking mock is now misleading. Removing both keeps the unmock list in sync with the active mocks.♻️ Proposed cleanup
// The repo-wide vitest config uses `isolate: false`, so every `vi.mock(...)` // registered above leaks into the next test file in the same worker (which // breaks anything that does real fs I/O against `node:fs/promises.readFile`, - // `command-helpers.loadMigrationPackages`, or `migration-tools/io.writeMigrationPackage`). + // `node:fs/promises.readFile` or `migration-tools/io.writeMigrationPackage`). // Use `doUnmock` (non-hoisted) here so subsequent files see the real modules. afterAll(() => { vi.doUnmock('node:fs/promises'); vi.doUnmock('../../src/config-loader'); - vi.doUnmock('../../src/utils/command-helpers'); vi.doUnmock('`@prisma-next/migration-tools/refs`');🤖 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 `@packages/1-framework/3-tooling/cli/test/commands/migration-plan-command.test.ts` around lines 355 - 373, Remove the stale unmock and related comment: delete the vi.doUnmock('../../src/utils/command-helpers') call inside the afterAll block and update the preceding comment that lists leaking mocks so it no longer references command-helpers.loadMigrationPackages; ensure the afterAll cleanup now only contains the actual modules that were mocked and still need doUnmock (keep the other vi.doUnmock(...) and vi.resetModules() calls intact).packages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-render.ts (1)
16-22: 💤 Low valueConsider consolidating re-exports into an
exports/module.The re-exports here violate the guideline "Don't reexport from one file in another, except in
exports/folders." While these re-exports appear to have existed prior to this PR (the changes only update their source paths), they should ideally be moved to a dedicatedexports/module if they're intended as a public API surface.This is out of scope for the current PR (which focuses on import path consolidation), but consider consolidating public type/function exports into
cli/src/exports/migration-list-formatters.tsor similar in a follow-up.As per coding guidelines: "Don't reexport from one file in another, except in
exports/folders."🤖 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 `@packages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-render.ts` around lines 16 - 22, This file currently re-exports types GlyphMode, MigrationEdgeKind, MigrationListEntry, MigrationListResult, and MigrationSpaceListEntry directly; move those re-exports into a dedicated exports module (e.g., migration-list-formatters) and replace these re-export statements with imports from that new exports module in any consumers; specifically, create a new exports file that exports the listed types and update migration-list-render.ts to import (not re-export) GlyphMode, MigrationEdgeKind, MigrationListEntry, MigrationListResult, and MigrationSpaceListEntry from that exports module so public API re-exports live only in the exports folder.
🤖 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
`@packages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-render.ts`:
- Around line 16-22: This file currently re-exports types GlyphMode,
MigrationEdgeKind, MigrationListEntry, MigrationListResult, and
MigrationSpaceListEntry directly; move those re-exports into a dedicated exports
module (e.g., migration-list-formatters) and replace these re-export statements
with imports from that new exports module in any consumers; specifically, create
a new exports file that exports the listed types and update
migration-list-render.ts to import (not re-export) GlyphMode, MigrationEdgeKind,
MigrationListEntry, MigrationListResult, and MigrationSpaceListEntry from that
exports module so public API re-exports live only in the exports folder.
In
`@packages/1-framework/3-tooling/cli/test/commands/migration-plan-command.test.ts`:
- Around line 355-373: Remove the stale unmock and related comment: delete the
vi.doUnmock('../../src/utils/command-helpers') call inside the afterAll block
and update the preceding comment that lists leaking mocks so it no longer
references command-helpers.loadMigrationPackages; ensure the afterAll cleanup
now only contains the actual modules that were mocked and still need doUnmock
(keep the other vi.doUnmock(...) and vi.resetModules() calls intact).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 6220245c-819d-4bac-a0ad-0f8b7fe3dab3
⛔ Files ignored due to path filters (4)
projects/migration-store/design-decisions.mdis excluded by!projects/**projects/migration-store/plan.mdis excluded by!projects/**projects/migration-store/slices/adopt-read-commands/spec.mdis excluded by!projects/**projects/migration-store/spec.mdis excluded by!projects/**
📒 Files selected for processing (24)
drive/calibration/dod.mddrive/retro/findings.mdpackages/1-framework/3-tooling/cli/src/commands/migration-list.tspackages/1-framework/3-tooling/cli/src/commands/migration-status.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-aggregate-loader.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-data-column.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-graph-layout.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-graph-render.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-graph-topology.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-render.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-types.tspackages/1-framework/3-tooling/cli/test/commands/migration-list.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-plan-command.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-tamper.test.tspackages/1-framework/3-tooling/cli/test/utils/build-read-aggregate.test.tspackages/1-framework/3-tooling/cli/test/utils/formatters/migration-list-graph-fixtures.tspackages/1-framework/3-tooling/cli/test/utils/formatters/migration-list-graph-layout.test.tspackages/1-framework/3-tooling/cli/test/utils/formatters/migration-list-graph-topology.test.tspackages/1-framework/3-tooling/cli/test/utils/formatters/migration-list-render.test.tspackages/1-framework/3-tooling/cli/test/utils/formatters/migration-list-styler.test.tspackages/1-framework/3-tooling/migration/package.jsonpackages/1-framework/3-tooling/migration/src/exports/migration-list-graph-topology.tspackages/1-framework/3-tooling/migration/src/exports/migration-list-types.tspackages/1-framework/3-tooling/migration/tsdown.config.ts
💤 Files with no reviewable changes (4)
- packages/1-framework/3-tooling/migration/src/exports/migration-list-types.ts
- packages/1-framework/3-tooling/migration/src/exports/migration-list-graph-topology.ts
- packages/1-framework/3-tooling/migration/tsdown.config.ts
- packages/1-framework/3-tooling/migration/package.json
✅ Files skipped from review due to trivial changes (6)
- packages/1-framework/3-tooling/cli/test/commands/migration-tamper.test.ts
- packages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-graph-layout.ts
- packages/1-framework/3-tooling/cli/test/utils/formatters/migration-list-graph-topology.test.ts
- drive/retro/findings.md
- packages/1-framework/3-tooling/cli/test/utils/formatters/migration-list-styler.test.ts
- packages/1-framework/3-tooling/cli/src/utils/formatters/migration-list-graph-render.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/1-framework/3-tooling/cli/test/utils/build-read-aggregate.test.ts
- drive/calibration/dod.md
- packages/1-framework/3-tooling/cli/src/commands/migration-list.ts
wmadden
added a commit
that referenced
this pull request
May 30, 2026
Land the mandatory final retro for TML-2709 and delete the transient project workspace now that both slices (TML-2715 PR #626, TML-2716 PR #644) are merged. - Document the ContractSpaceAggregate read model (tolerant build / lazy query / integrity-as-query split) in the Migration System subsystem doc, cross-linked from ADR 212. - Calibration: add failure-mode F15 (behavioural reports-all/tolerates/ refuses ACs verified by code-read instead of a populated fixture) and a per-package test-invocation note in dod.md. - Record the close-out retro in drive/retro/findings.md. - Remove projects/migration-store/. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 30, 2026
Land the mandatory final retro for TML-2709 and delete the transient project workspace now that both slices (TML-2715 PR #626, TML-2716 PR #644) are merged. - Document the ContractSpaceAggregate read model (tolerant build / lazy query / integrity-as-query split) in the Migration System subsystem doc, cross-linked from ADR 212. - Calibration: add failure-mode F15 (behavioural reports-all/tolerates/ refuses ACs verified by code-read instead of a populated fixture) and a per-package test-invocation note in dod.md. - Record the close-out retro in drive/retro/findings.md. - Remove projects/migration-store/. Signed-off-by: Will Madden <madden@prisma.io>
5 tasks
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
2 participants
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.
Linked issue
Refs TML-2716. Slice 2 of the migration-store project (TML-2709); builds on the tolerant, queryable
ContractSpaceAggregatelanded in Slice 1 (TML-2715).At a glance
Every CLI command that reads migration packages now loads them once through the aggregate instead of its own ad-hoc package walk:
Before this slice, each of these commands called
loadMigrationPackages(and several also calledenumerateMigrationSpaces,readRefs, andreadContractEnvelope) directly, each reconstructing its own view of on-disk migration state.Decision
This PR routes all seven migration-package-reading CLI commands —
migration list,migration graph,migration log,db sign,db update,migration plan, andref set— through a single read path (buildReadAggregate→ContractSpaceAggregate), and deletes both legacy helpers that path replaces:loadMigrationPackages(incli/src/utils/command-helpers.ts)enumerateMigrationSpaces(the wholemigration-tools/enumerate-migration-spacesmodule, its export entry, and its test)The result is net-deletion: one model of migration state for read commands, no per-command package walks. Happy-path human output,
--json, and structured errors are unchanged for every command — this is pure adoption, not a behavior change.How it fits together
buildReadAggregate(incli/src/utils/contract-space-aggregate-loader.ts) loads theContractSpaceAggregatewithout the integrity gate that write commands use, and returns{ aggregate, contractHash }. The offline-fallback helpers (appContractShellForAggregateLoad,loadContractRawSafely) move here frommigration-status.tsso both status and read commands share them.migration graph,migration log,db sign,db update,migration plan, andref setswap theirloadMigrationPackagescalls forbuildReadAggregate, deriving graph/packages/refs/contract-hash from the aggregate.loadMigrationPackagesis then deleted.migration listand delete the enumerator. A new mapper,migrationSpaceListEntriesFromAggregate, derives the render-readyMigrationSpaceListEntry[]from aggregate members, using the same on-disk space-id ordering the old enumerator used (app-first, lexical, reserved-name filtered) so extension-only and empty-dir cases stay identical.enumerateMigrationSpacesis deleted;refsByContractHash/resolveRefsByContractHashmove intomigration-tools/refs.--jsongolden test (back-fillingmigration graph, which had no command-level test), so the "no observable change" claim is enforced rather than asserted in prose.Reviewer notes
migration list. The aggregate always exposes a syntheticappspace even whenmigrations/app/is absent on disk; the old enumerator listed only on-disk directories. The mapper resolves this by driving enumeration from the on-disk space-id list and readingaggregate.space(id)per id, so extension-only disks stay extension-only. Worth a spot-check.member.refsexcludes the structuralrefs/head.json, which the oldlist(viareadRefs) included — so an extension space's tip-migration row would have lost itsheaddecoration. The mapper foldsmember.headRefback into the by-destination-hash decoration to preserve byte-identical output. Whetherheadshould appear inlistdecorations at all is a separate semantics question (follow-up, not this slice).ref list/ref deletekeepreadRefs. Onlyref setreads packages, so only it moves to the aggregate; the other two ref subcommands don't touch package state and are untouched.main. This branch mergedorigin/mainmid-flight (conflicts inmigration-list.test.tsand a drive calibration doc);main's independentmigration listgraph-rendering work and amain-authored list golden test are both preserved on top of the aggregate re-point.Verification
pnpm --filter @prisma-next/cli typecheck && pnpm --filter @prisma-next/cli lint && pnpm --filter @prisma-next/cli test— 1080 casespnpm --filter @prisma-next/migration-tools typecheck && pnpm --filter @prisma-next/migration-tools test— 555 casesgit grep loadMigrationPackages/git grep enumerateMigrationSpacesoverpackages/,examples/,test/— zero hitsFollow-ups
headshould appear inmigration listref decorations at all (semantics question deferred above) — to be filed at project close-out.Alternatives considered
loadMigrationPackagesfor the writer-read commands (db sign,db update,migration plan,ref set) and only migrate the three pure-read commands. Rejected: the writer-read commands' package reads are the same mechanical swap, and leaving the helper alive would block the net-deletion that is the point of the slice — the thesis "every command that reads migration packages goes through the aggregate" only holds if all seven move.appspace whenmigrations/app/is absent. Rejected for this slice: that's a model-semantics change with its own blast radius; the mapper reconciles disk-vs-synthetic locally instead, keeping this slice pure-adoption. The model question is noted as a follow-up.Skill update
n/a — internal only. No CLI flags, public APIs, config fields, error codes, or glossary terms change; this is a read-path refactor with byte-identical command output.
Checklist
git commit -s) per the DCO.--jsontests added for each touched command).TML-NNNN: <sentence-case title>form.Summary by CodeRabbit
Bug Fixes
Refactor