TML-2715: tolerant queryable ContractSpaceAggregate + checkIntegrity()#626
Merged
wmadden merged 38 commits intoMay 30, 2026
Merged
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughAdds a tolerant ContractSpaceAggregate with lazy ChangesAggregate-based migration integrity and CLI
Sequence Diagram(s) sequenceDiagram
participant User
participant CLI
participant AggregateLoader
participant IntegrityCheck
participant DB
User->>CLI: run migrate/check/show/status/new/plan
CLI->>AggregateLoader: loadContractSpaceAggregate(migrationsDir, appContract)
AggregateLoader-->>CLI: ContractSpaceAggregate (lazy)
CLI->>IntegrityCheck: aggregate.checkIntegrity({declaredExtensions, checkContracts})
IntegrityCheck-->>CLI: IntegrityViolation[]
alt violations exist and command is gating
CLI-->>User: Refuse with PN-MIG-5001 / PN-MIG-5002 or PN-MIG-CHECK-00x
else proceed
CLI->>DB: apply/plan/verify as needed
DB-->>CLI: results
CLI-->>User: ok output
end
Estimated code review effort Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
@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 📦
|
wmadden
force-pushed
the
tml-2709-introduce-migrationstore-load-once-queryable-multi-space
branch
from
61b714b to
2c015f6
Compare
![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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/1-framework/3-tooling/cli/src/commands/migration-show.ts (1)
186-201:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't ignore
readMigrationsDir().problems.These paths now compute “latest” and resolve refs from
packagesalone, so an unreadable package gets silently dropped from the graph. That can makemigration showreport an older migration as latest, or returnmissing/No migrations foundwhen the directory is actually corrupted. Please fail whenproblems.length > 0instead of proceeding with a partial read.💡 Example guard
- const { packages: allPackages } = await readMigrationsDir(spaceDir); + const { packages: allPackages, problems } = await readMigrationsDir(spaceDir); + if (problems.length > 0) { + return notOk( + errorRuntime('Failed to read migrations', { + why: `One or more migration packages under ${relative(process.cwd(), spaceDir)} could not be loaded.`, + fix: 'Repair or remove the unreadable migration package(s) and rerun the command.', + }), + ); + }Also applies to: 305-331, 407-437
🤖 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/commands/migration-show.ts` around lines 186 - 201, The code currently ignores readMigrationsDir().problems and proceeds with a partial set of packages; update the logic after calling readMigrationsDir in migration-show.ts (where you destructure { packages: allPackages }) to check the returned problems array and return a notOk(errorRuntime(...)) if problems.length > 0, providing a clear why/fix message (similar style to existing errorRuntime usage). Do this check before using allPackages/graph/latestMigration (and also apply the same guard to the other similar blocks referenced around the other ranges), ensure you reference readMigrationsDir, allPackages, problems, and use the project's errorRuntime/notOk helpers to fail fast when unreadable packages are detected.
🤖 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 `@packages/1-framework/3-tooling/cli/src/commands/migrate.ts`:
- Around line 231-247: Move the integrity gate (buildContractSpaceAggregate /
gate.ok check) to run before any resolution of the `--to` graph (i.e., before
calls to readRefs(), loadMigrationPackages(), and parseContractRef() / the
options.to branch). Specifically, locate the buildContractSpaceAggregate call
that uses targetId/migrationsDir/appContract/deserializeContract and hoist it
above the code path that executes readRefs(), loadMigrationPackages(), and
parseContractRef(), so that the 5002 integrity failure is returned immediately
(via notOk(gate.failure)) before any graph/package resolution can produce
spurious ref/path errors.
In `@packages/1-framework/3-tooling/cli/src/commands/migration-status.ts`:
- Around line 874-881: Move the package-corruption check earlier so it runs
before any ref-resolution or earlyGraph construction that uses tolerant
loadMigrationPackages; specifically, call detectPackageCorruption (the block
that uses migrationsDir, contractRawForAggregate and deserializeContract via
familyInstance.deserializeContract) before the options.to / options.from branch
builds earlyGraph or invokes loadMigrationPackages, so unloadable on-disk
packages trigger the 5002 corruption envelope instead of downstream ref/path
errors. Ensure you keep the existing parameters (migrationsDir,
contractRawForAggregate, deserializeContract) and return
notOk(corruptionFailure) immediately if corruption is detected.
In
`@packages/1-framework/3-tooling/cli/src/utils/contract-space-aggregate-loader.ts`:
- Around line 90-94: The error message created by the CliStructuredError
instances (e.g., the '5001' envelope built with variables summary and lines,
domain 'MIG', and the existing fix string) points users to `prisma-next migrate`
which cannot repair the broken on-disk artifacts; update the fix text for the
'5001' and related '5002' CliStructuredError constructions so they instruct
users to either restore/re‑emit the affected extension package(s) or correct
their extensionPacks declaration (for example: "Restore or re‑emit the affected
extension package(s) that created the migrations/ directory, or update your
extensionPacks to remove the orphaned package"), and apply the same updated fix
wording to the other similar error construction block in this file.
- Around line 209-210: Remove the unnecessary bare cast on
inputs.extensionPacks: call toDeclaredExtensionsFromRaw with
inputs.extensionPacks directly (no "as ReadonlyArray<unknown>") since
BuildAggregateInputs.extensionPacks already has the correct ReadonlyArray type;
update the invocation that assigns declaredExtensions (the
toDeclaredExtensionsFromRaw(...) call) to use inputs.extensionPacks without the
cast.
In `@packages/1-framework/3-tooling/migration/src/aggregate/loader.ts`:
- Around line 153-163: readHeadRefTolerant is currently catching all exceptions
and converting them into a tolerant result; change it to only swallow expected
non-fatal cases (e.g., missing head ref/file NOT_FOUND or known parse/validation
errors) by inspecting the error (e.g., error.code === 'ENOENT' or error
instanceof SomeParseError) and returning { headRef: null, problem: ... } for
those, but rethrow for catastrophic I/O/system errors (e.g., error.code ===
'EACCES' || 'EIO' || 'EPERM' || other unknown codes) so real filesystem failures
are not masked; apply the same narrowing pattern to the other tolerant helper in
this file (the one in the 175-186 region) and use the same helpers/functions
referenced here (readContractSpaceHeadRef, HEAD_REF_NAME, detailOf) to build the
tolerant result when appropriate.
In `@packages/1-framework/3-tooling/migration/test/io.test.ts`:
- Around line 677-687: Rename the test case description to match its setup:
change the it(...) title that currently reads "returns a packageUnloadable
problem and omits the package when migration.json is missing" to reflect that
migration.json is present but invalid (e.g., "returns a packageUnloadable
problem and omits the package when migration.json is invalid"); keep the
existing setup that writes an invalid migration.json and ops.json and do not
change the test body or file operations in the test function.
In `@packages/3-mongo-target/1-mongo-target/src/core/control-target.ts`:
- Around line 180-182: Replace the two plain double-casts with explicit utility
casts: instead of "opts.destinationContract as unknown as Contract" used in the
contract thunk, call castAs<Contract>(opts.destinationContract) or
blindCast<Contract, 'Reason'>(opts.destinationContract) (choose blindCast if you
want to annotate intent/reason). Do the same for the other occurrence where
something is cast to ContractSpaceMember (replace "as unknown as
ContractSpaceMember" with castAs<ContractSpaceMember>(...) or
blindCast<ContractSpaceMember, 'Reason'>(...)). Import castAs/blindCast from
"`@prisma-next/utils/casts`" and keep the original identifiers (contract,
ContractSpaceMember) unchanged.
---
Outside diff comments:
In `@packages/1-framework/3-tooling/cli/src/commands/migration-show.ts`:
- Around line 186-201: The code currently ignores readMigrationsDir().problems
and proceeds with a partial set of packages; update the logic after calling
readMigrationsDir in migration-show.ts (where you destructure { packages:
allPackages }) to check the returned problems array and return a
notOk(errorRuntime(...)) if problems.length > 0, providing a clear why/fix
message (similar style to existing errorRuntime usage). Do this check before
using allPackages/graph/latestMigration (and also apply the same guard to the
other similar blocks referenced around the other ranges), ensure you reference
readMigrationsDir, allPackages, problems, and use the project's
errorRuntime/notOk helpers to fail fast when unreadable packages are detected.
🪄 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: c6684e22-eb9f-4aba-bfda-85c8306f6e82
⛔ Files ignored due to path filters (5)
projects/migration-store/design-notes.mdis excluded by!projects/**projects/migration-store/learnings.mdis excluded by!projects/**projects/migration-store/plan.mdis excluded by!projects/**projects/migration-store/slices/tolerant-queryable-aggregate/spec.mdis excluded by!projects/**projects/migration-store/spec.mdis excluded by!projects/**
📒 Files selected for processing (57)
docs/architecture docs/subsystems/7. Migration System.mddocs/glossary.mdpackages/1-framework/3-tooling/cli/src/commands/migrate.tspackages/1-framework/3-tooling/cli/src/commands/migration-check.tspackages/1-framework/3-tooling/cli/src/commands/migration-new.tspackages/1-framework/3-tooling/cli/src/commands/migration-plan.tspackages/1-framework/3-tooling/cli/src/commands/migration-show.tspackages/1-framework/3-tooling/cli/src/commands/migration-status.tspackages/1-framework/3-tooling/cli/src/control-api/client.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-verify.tspackages/1-framework/3-tooling/cli/src/control-api/operations/migration-apply.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/test/commands/cross-consumer-integrity.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-apply.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-e2e.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-plan-command.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-plan.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-ref.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-show.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-status-aggregate-spaces.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-tamper.test.tspackages/1-framework/3-tooling/cli/test/control-api/apply-aggregate.test.tspackages/1-framework/3-tooling/migration/package.jsonpackages/1-framework/3-tooling/migration/src/aggregate/aggregate.tspackages/1-framework/3-tooling/migration/src/aggregate/check-integrity.tspackages/1-framework/3-tooling/migration/src/aggregate/loader.tspackages/1-framework/3-tooling/migration/src/aggregate/planner.tspackages/1-framework/3-tooling/migration/src/aggregate/project-schema-to-space.tspackages/1-framework/3-tooling/migration/src/aggregate/strategies/graph-walk.tspackages/1-framework/3-tooling/migration/src/aggregate/strategies/synth.tspackages/1-framework/3-tooling/migration/src/aggregate/types.tspackages/1-framework/3-tooling/migration/src/aggregate/verifier.tspackages/1-framework/3-tooling/migration/src/compute-extension-space-apply-path.tspackages/1-framework/3-tooling/migration/src/enumerate-migration-spaces.tspackages/1-framework/3-tooling/migration/src/exports/aggregate.tspackages/1-framework/3-tooling/migration/src/exports/integrity-violation.tspackages/1-framework/3-tooling/migration/src/exports/io.tspackages/1-framework/3-tooling/migration/src/integrity-violation.tspackages/1-framework/3-tooling/migration/src/io.tspackages/1-framework/3-tooling/migration/src/migration-graph.tspackages/1-framework/3-tooling/migration/src/refs.tspackages/1-framework/3-tooling/migration/test/aggregate/loader.test.tspackages/1-framework/3-tooling/migration/test/aggregate/planner.test.tspackages/1-framework/3-tooling/migration/test/aggregate/project-schema-to-space.test.tspackages/1-framework/3-tooling/migration/test/aggregate/strategies/graph-walk.test.tspackages/1-framework/3-tooling/migration/test/aggregate/strategies/synth.test.tspackages/1-framework/3-tooling/migration/test/aggregate/verifier.test.tspackages/1-framework/3-tooling/migration/test/deletable-node-modules.test.tspackages/1-framework/3-tooling/migration/test/fixtures.tspackages/1-framework/3-tooling/migration/test/io.test.tspackages/1-framework/3-tooling/migration/test/migration-graph.test.tspackages/1-framework/3-tooling/migration/tsdown.config.tspackages/3-mongo-target/1-mongo-target/src/core/control-target.tstest/integration/test/cli-journeys/migration-plan-details.e2e.test.tstest/integration/test/cli.migration-apply.e2e.test.ts
💤 Files with no reviewable changes (3)
- packages/1-framework/3-tooling/cli/src/control-api/types.ts
- packages/1-framework/3-tooling/migration/src/migration-graph.ts
- packages/1-framework/3-tooling/cli/src/control-api/client.ts
Comment on lines
209
to
210
| const declaredExtensions: readonly DeclaredExtensionEntry[] = toDeclaredExtensionsFromRaw( | ||
| inputs.extensionPacks as ReadonlyArray<unknown>, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/1-framework/3-tooling/cli/src/utils/contract-space-aggregate-loader.ts"
# Show the exact snippet with line numbers around 209-210
nl -ba "$FILE" | sed -n '180,235p'Repository: prisma/prisma-next
Length of output: 105
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/1-framework/3-tooling/cli/src/utils/contract-space-aggregate-loader.ts"
# Show lines around 209-210 using cat -n
cat -n "$FILE" | sed -n '190,235p'
# Find where inputs.extensionPacks is declared/typed in this file
rg -n "extensionPacks" "$FILE"
# Locate toDeclaredExtensionsFromRaw definition and its parameter type
rg -n "toDeclaredExtensionsFromRaw" -S .Repository: prisma/prisma-next
Length of output: 4520
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect contract-space-aggregate-loader.ts around BuildAggregateInputs definition
FILE1="packages/1-framework/3-tooling/cli/src/utils/contract-space-aggregate-loader.ts"
echo "---- $FILE1 (around extensionPacks type) ----"
cat -n "$FILE1" | sed -n '140,200p'
# Inspect extension-pack-inputs.ts around toDeclaredExtensionsFromRaw signature
FILE2="packages/1-framework/3-tooling/cli/src/utils/extension-pack-inputs.ts"
echo "---- $FILE2 (around toDeclaredExtensionsFromRaw) ----"
cat -n "$FILE2" | sed -n '120,210p'
# Also show exact callsite signature line for cast snippet
echo "---- call site ----"
cat -n "$FILE1" | sed -n '200,220p'Repository: prisma/prisma-next
Length of output: 6628
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect contract-space-aggregate-loader.ts around BuildAggregateInputs definition
FILE1="packages/1-framework/3-tooling/cli/src/utils/contract-space-aggregate-loader.ts"
echo "---- $FILE1 (around extensionPacks type) ----"
cat -n "$FILE1" | sed -n '140,200p'
# Inspect extension-pack-inputs.ts around toDeclaredExtensionsFromRaw signature
FILE2="packages/1-framework/3-tooling/cli/src/utils/extension-pack-inputs.ts"
echo "---- $FILE2 (around toDeclaredExtensionsFromRaw) ----"
cat -n "$FILE2" | sed -n '120,210p'
echo "---- call site ----"
cat -n "$FILE1" | sed -n '200,220p'Repository: prisma/prisma-next
Length of output: 6628
Drop the redundant bare cast for inputs.extensionPacks
BuildAggregateInputs.extensionPacks is already typed as ReadonlyArray<ControlExtensionDescriptor<TFamilyId, TTargetId>>, and toDeclaredExtensionsFromRaw expects ReadonlyArray<unknown>, so the inputs.extensionPacks as ReadonlyArray<unknown> cast is unnecessary (it’s a bare as in production code).
const declaredExtensions: readonly DeclaredExtensionEntry[] = toDeclaredExtensionsFromRaw(
inputs.extensionPacks as ReadonlyArray<unknown>,
);🤖 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/contract-space-aggregate-loader.ts`
around lines 209 - 210, Remove the unnecessary bare cast on
inputs.extensionPacks: call toDeclaredExtensionsFromRaw with
inputs.extensionPacks directly (no "as ReadonlyArray<unknown>") since
BuildAggregateInputs.extensionPacks already has the correct ReadonlyArray type;
update the invocation that assigns declaredExtensions (the
toDeclaredExtensionsFromRaw(...) call) to use inputs.extensionPacks without the
cast.
wmadden
commented
May 30, 2026
| | `PN-MIG-CHECK-004 DANGLING_REF` | Ref points at a contract hash absent from the graph. | `prisma-next ref set <name> <valid-hash>` or `prisma-next ref delete <name>`. | | ||
| | `PN-MIG-CHECK-005 EDGE_MISMATCH` | Migration's `metadata.to` disagrees with its `end-contract.json` snapshot's `storageHash` — within-migration snapshot drift. | Re-emit the migration package so `migration.json` and `end-contract.json` agree. | | ||
| | `PN-MIG-CHECK-006 SNAPSHOT_UNREADABLE` | Migration's `end-contract.json` snapshot exists but cannot be parsed. | Re-emit the migration package to repair the snapshot file. | | ||
| | `PN-MIG-CHECK-007 SELF_EDGE` | A migration's source contract equals its target (`from === to`) with no data operation — a self-edge that applies nothing. | Delete the no-op self-edge, or add the data operation it was meant to carry. | |
Contributor
Author
There was a problem hiding this comment.
This is technically misleading. The distinguishing characteristic is that it carries an data invariant. Whether that's a schema change or a data change is not relevant; users can use this to maintain schema artifacts that Prisma Next cannot track.
Comment on lines
+231
to
+247
| // Gate the contract-space aggregate before opening a DB connection. | ||
| // `checkIntegrity` is a pure offline check (on-disk migration state + | ||
| // the live app contract — no driver), and the transitional constraint | ||
| // forbids any window where a broken set touches a database. Refusing | ||
| // here preserves the original "refuse before connecting" safety | ||
| // property: a tampered/inconsistent set fails with the `5002` integrity | ||
| // envelope before the driver is ever created. | ||
| const gate = await buildContractSpaceAggregate({ | ||
| targetId: config.target.targetId, | ||
| migrationsDir, | ||
| appContract: contractRaw, | ||
| extensionPacks: config.extensionPacks ?? [], | ||
| deserializeContract: (json) => familyInstance.deserializeContract(json), | ||
| }); | ||
| if (!gate.ok) { | ||
| return notOk(gate.failure); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
I don't like using the verb "gate" here, or the variable name gate. We are loading or constructing the contract-space aggregate, not "gating". Moreover, the comment decsribes checking its integrity, but never calls checkIntegrity(). Why? The whole comment could be omitted with a single call to checkIntegrity(), which already spells out its intention.
Comment on lines
+126
to
+130
| const COVERED_BY_LEGACY_CHECKS: ReadonlySet<IntegrityViolation['kind']> = new Set([ | ||
| 'hashMismatch', | ||
| 'providedInvariantsMismatch', | ||
| 'packageUnloadable', | ||
| ]); |
Contributor
Author
There was a problem hiding this comment.
Don't codify legacy behavior, update it so it's no longer legacy and delete these extensive comments and workarounds
| if (problem.kind === 'packageUnloadable') { | ||
| failures.push({ | ||
| pnCode, | ||
| pnCode: 'PN-MIG-CHECK-002', |
Contributor
Author
There was a problem hiding this comment.
CAn we avoid magic strings and instead use common constructor functions? We already do for errors, what is this failure type?
Comment on lines
+515
to
+516
| // Fold in the aggregate-level integrity view. `check`'s on-disk pass | ||
| // above only walks the app space; `checkIntegrity` reports the full set |
Contributor
Author
There was a problem hiding this comment.
Why does it need two passes? Can we consolidate them?
| } | ||
|
|
||
| try { | ||
| aggregateSpaces = await loadAggregateStatusSpaces({ |
Contributor
Author
There was a problem hiding this comment.
Is this loading the same information again?
| bundles: readonly OnDiskMigrationPackage[]; | ||
| graph: MigrationGraph; | ||
| }> { | ||
| const bundles = await readMigrationsDir(migrationsDir); |
Contributor
Author
There was a problem hiding this comment.
This looks like it should have been replaced by the CS agg. Why do we still have this function?
| const spaceId = 'spaceId' in violation ? violation.spaceId : '*'; | ||
| return `Integrity violation "${violation.kind}" for contract space "${spaceId}".`; | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
This looks like it duplicates the mapping logic in packages/1-framework/3-tooling/cli/src/commands/migration-check.ts
Comment on lines
+144
to
+150
| violations: [ | ||
| { | ||
| kind: 'validation', | ||
| spaceId: contractUnreadable.spaceId, | ||
| detail: contractUnreadable.detail, | ||
| }, | ||
| ], |
Contributor
Author
There was a problem hiding this comment.
We appear to have control plane errors, "failures" and now "violations". Why do we have so many error-shaped objects? Can we consolidate them?
Comment on lines
+113
to
+128
| return { | ||
| kind: 'hashMismatch', | ||
| spaceId, | ||
| dirName: problem.dirName, | ||
| stored: problem.stored, | ||
| computed: problem.computed, | ||
| }; | ||
| case 'providedInvariantsMismatch': | ||
| return { kind: 'providedInvariantsMismatch', spaceId, dirName: problem.dirName }; | ||
| case 'packageUnloadable': | ||
| return { | ||
| kind: 'packageUnloadable', | ||
| spaceId, | ||
| dirName: problem.dirName, | ||
| detail: problem.detail, | ||
| }; |
Contributor
Author
There was a problem hiding this comment.
Why do we have this mapping AGAIN?
wmadden
added a commit
that referenced
this pull request
May 30, 2026
Fold the PR #626 review findings + operator decisions into the slice spec: harden the tolerant-queryable foundation (one integrity vocabulary, one mapping spine, single-engine migration check, load-once across slice-1 consumers) before slice 2 adds list/graph/log readers.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/1-framework/3-tooling/cli/src/utils/contract-space-aggregate-loader.ts (1)
178-182: 💤 Low valueBare cast should use
castAsfor consistency.The cast
extensionPacks as ReadonlyArray<unknown>is a bareasin production code. While widening tounknownis safe, the coding guidelines prefercastAs<T>for consistency.♻️ Suggested fix
+import { castAs } from '`@prisma-next/utils/casts`'; + function declaredExtensionsFromInputs( extensionPacks: BuildAggregateInputs<string, string>['extensionPacks'], ): readonly DeclaredExtensionEntry[] { - return toDeclaredExtensionsFromRaw(extensionPacks as ReadonlyArray<unknown>); + return toDeclaredExtensionsFromRaw(castAs<ReadonlyArray<unknown>>(extensionPacks)); }As per coding guidelines: "No bare
asin production code. UseblindCast<T, "Reason">orcastAs<T>from@prisma-next/utils/casts".🤖 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/contract-space-aggregate-loader.ts` around lines 178 - 182, Replace the bare TypeScript cast in declaredExtensionsFromInputs by using the project's casting helper: import and use castAs (or blindCast) from `@prisma-next/utils/casts` instead of `extensionPacks as ReadonlyArray<unknown>` so the call to toDeclaredExtensionsFromRaw receives castAs<ReadonlyArray<unknown>>(extensionPacks) (referencing function declaredExtensionsFromInputs and toDeclaredExtensionsFromRaw).packages/1-framework/3-tooling/cli/src/commands/migration-check.ts (1)
92-111: 💤 Low valueSilent catch may hide unexpected failures during integrity loading.
The
catch { return []; }silently swallows all errors, including unexpected runtime errors (e.g., OOM, permission issues). Consider logging a debug-level message or at least returning a distinguishing empty result vs. explicitly noting that the aggregate failed to load.That said, given this is supplementary to the existing per-migration checks and the command still reports primary failures, this is acceptable for a tolerant reader pattern.
🤖 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/commands/migration-check.ts` around lines 92 - 111, The silent catch in loadAggregateIntegrityViolations swallows all errors (from readFile, JSON.parse, familyInstance.deserializeContract, loadContractSpaceAggregate, etc.); change the catch to capture the error (catch (err)) and log a debug-level message including the error and context (e.g., migrationsDir and the contract path from resolveContractPath(config)) before returning []; update the handler around loadAggregateIntegrityViolations to use an existing logger or console.debug if no logger is available so failures to load the aggregate are distinguishable in logs while preserving the tolerant empty-result behavior.packages/1-framework/3-tooling/cli/test/commands/migration-show.test.ts (1)
3-3: ⚡ Quick winReplace
node:pathwithpathefor consistency.This test file is in the CLI package test suite (
packages/.../cli/test/), not intest/integration/test/. Per repository guidelines, preferpatheovernode:pathfor all CLI tooling files, including unit tests.📦 Suggested fix
-import { join } from 'node:path'; +import { join } from 'pathe';Based on learnings: "In the Prisma-next repository, prefer importing and using 'pathe' over the built-in 'node:path' module for path operations across the codebase (including CLI commands and tooling files)."
🤖 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-show.test.ts` at line 3, Replace the node:path import with pathe in the test: change the import statement that currently reads "import { join } from 'node:path';" to import join from 'pathe' (e.g., "import { join } from 'pathe';") so the CLI test uses the repository-preferred pathe module; update any uses of join in migration-show.test.ts if necessary to match the pathe import.
🤖 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/commands/migration-check.ts`:
- Around line 92-111: The silent catch in loadAggregateIntegrityViolations
swallows all errors (from readFile, JSON.parse,
familyInstance.deserializeContract, loadContractSpaceAggregate, etc.); change
the catch to capture the error (catch (err)) and log a debug-level message
including the error and context (e.g., migrationsDir and the contract path from
resolveContractPath(config)) before returning []; update the handler around
loadAggregateIntegrityViolations to use an existing logger or console.debug if
no logger is available so failures to load the aggregate are distinguishable in
logs while preserving the tolerant empty-result behavior.
In
`@packages/1-framework/3-tooling/cli/src/utils/contract-space-aggregate-loader.ts`:
- Around line 178-182: Replace the bare TypeScript cast in
declaredExtensionsFromInputs by using the project's casting helper: import and
use castAs (or blindCast) from `@prisma-next/utils/casts` instead of
`extensionPacks as ReadonlyArray<unknown>` so the call to
toDeclaredExtensionsFromRaw receives
castAs<ReadonlyArray<unknown>>(extensionPacks) (referencing function
declaredExtensionsFromInputs and toDeclaredExtensionsFromRaw).
In `@packages/1-framework/3-tooling/cli/test/commands/migration-show.test.ts`:
- Line 3: Replace the node:path import with pathe in the test: change the import
statement that currently reads "import { join } from 'node:path';" to import
join from 'pathe' (e.g., "import { join } from 'pathe';") so the CLI test uses
the repository-preferred pathe module; update any uses of join in
migration-show.test.ts if necessary to match the pathe import.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 2849ff26-3247-486f-8a68-889a5c08de4c
⛔ Files ignored due to path filters (1)
projects/migration-store/slices/tolerant-queryable-aggregate/spec.mdis excluded by!projects/**
📒 Files selected for processing (32)
docs/architecture docs/subsystems/7. Migration System.mddocs/glossary.mdpackages/1-framework/3-tooling/cli/src/commands/migrate.tspackages/1-framework/3-tooling/cli/src/commands/migration-check.tspackages/1-framework/3-tooling/cli/src/commands/migration-new.tspackages/1-framework/3-tooling/cli/src/commands/migration-show.tspackages/1-framework/3-tooling/cli/src/commands/migration-status.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-verify.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-aggregate-loader.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migrations.tspackages/1-framework/3-tooling/cli/src/utils/integrity-violation-to-check-failure.tspackages/1-framework/3-tooling/cli/test/commands/cross-consumer-integrity.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-show.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-status-aggregate-spaces.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-tamper.test.tspackages/1-framework/3-tooling/cli/test/control-api/db-verify.per-member-verifier.test.tspackages/1-framework/3-tooling/cli/test/utils/map-integrity-violations.test.tspackages/1-framework/3-tooling/migration/src/aggregate/check-integrity.tspackages/1-framework/3-tooling/migration/src/aggregate/loader.tspackages/1-framework/3-tooling/migration/src/aggregate/types.tspackages/1-framework/3-tooling/migration/src/exports/aggregate.tspackages/1-framework/3-tooling/migration/src/integrity-violation.tspackages/1-framework/3-tooling/migration/src/migration-graph.tspackages/1-framework/3-tooling/migration/test/aggregate/check-integrity.test.tspackages/1-framework/3-tooling/migration/test/aggregate/loader.catastrophic-io.test.tspackages/1-framework/3-tooling/migration/test/aggregate/loader.test.tspackages/1-framework/3-tooling/migration/test/io.test.tspackages/1-framework/3-tooling/migration/test/migration-graph.test.tspackages/1-framework/3-tooling/migration/test/refs.tolerant-io.test.tspackages/3-mongo-target/1-mongo-target/src/core/control-target.tstest/integration/test/cli-journeys/migration-show-reachability.e2e.test.tstest/integration/test/cli-journeys/schema-evolution-migrations.e2e.test.ts
✅ Files skipped from review due to trivial changes (3)
- packages/1-framework/3-tooling/cli/test/utils/map-integrity-violations.test.ts
- docs/glossary.md
- docs/architecture docs/subsystems/7. Migration System.md
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/1-framework/3-tooling/migration/src/integrity-violation.ts
- packages/3-mongo-target/1-mongo-target/src/core/control-target.ts
- packages/1-framework/3-tooling/migration/test/io.test.ts
- packages/1-framework/3-tooling/migration/src/aggregate/check-integrity.ts
- packages/1-framework/3-tooling/migration/src/aggregate/types.ts
- packages/1-framework/3-tooling/migration/test/aggregate/loader.test.ts
- packages/1-framework/3-tooling/migration/src/aggregate/loader.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-status-aggregate-spaces.test.ts
- packages/1-framework/3-tooling/cli/test/commands/cross-consumer-integrity.test.ts
…actSpaceAggregate Shape TML-2709 as: unify the migration read-commands on the existing ContractSpaceAggregate by separating build / query / judge — tolerant disk-only load, lazy graph()/contract() facets, and integrity as a checkIntegrity() query instead of throws in the load path. Drops the originally-proposed parallel MigrationStore model. Zero open questions. Signed-off-by: Will Madden <madden@prisma.io> Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Slice 1 (TML-2715): tolerant queryable ContractSpaceAggregate + checkIntegrity(); re-point check/apply/verify/status/show. Slice 2 (TML-2716): adopt the model in migration list/graph/log, deleting hand-rolled disk I/O. Stack is forced by the build/judge atomicity constraint. Signed-off-by: Will Madden <madden@prisma.io> Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
tolerant-queryable-aggregate (TML-2715): 4-dispatch sequence — tolerant primitives, the queryable aggregate + checkIntegrity(), re-point the load-throw consumers, cross-consumer integration tests. Signed-off-by: Will Madden <madden@prisma.io> Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Introduces the full IntegrityViolation discriminated union and IntegrityQueryOptions that checkIntegrity() will return. Exposes them via a new ./integrity-violation package export. The union covers three groups: recoverable violations (package retained, surfaced for policy), config/contract-dependent violations (only produced when the matching IntegrityQueryOptions opt is set), and unloadable violations (package omitted from the model). Signed-off-by: Will Madden <madden@prisma.io> Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
…throw A from === to self-edge is now representable in the graph regardless of whether the package has data ops. The sameSourceAndTarget invariant becomes a violation checkIntegrity() will report rather than a fatal thrown during graph construction. Transitional: callers that previously relied on the throw to gate self-edges will add local shims (see aggregate/loader.ts) until checkIntegrity() subsumes the check. Signed-off-by: Will Madden <madden@prisma.io> Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
…f throwing
readMigrationsDir now returns { packages, problems } instead of
throwing on content-level errors:
- hashMismatch: stored migrationHash differs from recomputed value;
package is retained in packages[], problem recorded.
- providedInvariantsMismatch: manifest and ops.json disagree on
invariants; package is retained, problem recorded.
- packageUnloadable: manifest missing, unparseable, or schema-invalid;
package omitted, problem recorded.
Catastrophic I/O errors (e.g. ENOTDIR) still reject.
Exports the new PackageLoadProblem and ReadMigrationsDirResult types
from the ./io entry point. Updates intra-package callers
(enumerate-migration-spaces, compute-extension-space-apply-path) to
destructure the new return shape.
Signed-off-by: Will Madden <madden@prisma.io>
Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Adapts aggregate/loader.ts to the new tolerant primitives:
- readMigrationsDir now returns { packages, problems }; the loader
converts any problem to integrityFailure (first-failure, as before).
- reconstructGraph no longer throws on no-data-op self-edges; the
loader re-acquires that check locally for both extension and app
packages.
Both shims are annotated for removal when the aggregate-refactor
dispatch introduces checkIntegrity() to subsume them.
Signed-off-by: Will Madden <madden@prisma.io>
Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
The aggregate loads migration state from disk; the live app contract always comes from the project central contract (compiled PSL), so it is threaded in as a required appContract rather than pretended to be a disk artifact. Fold the intra-package apply/verify engine re-point into D2 (atomic with the member-shape change); D3 narrows to the CLI consumers. Signed-off-by: Will Madden <madden@prisma.io> Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
…-point [handoff]
Reshape ContractSpaceMember/ContractSpaceAggregate to raw packages + refs
+ nullable headRef + lazy memoised graph()/contract() + query methods
(listSpaces/hasSpace/space/spaces). Rewrite loadContractSpaceAggregate to
{migrationsDir, deserializeContract, appContract}: tolerant (never throws on
disk content), app headRef synthesised from appContract.storage.storageHash,
app + extension packages read from disk. Add checkIntegrity(opts?) returning
the full IntegrityViolation set (no first-failure bail), config/contract
checks gated behind declaredExtensions/requireContracts. Re-point the
intra-package apply/verify engine (planner/verifier/graph-walk/synth/
project-schema-to-space) onto the new shape via requireHeadRef + lazy facets.
Both D1 transitional shims removed. Package typecheck passes; unit tests
updated but the package test run was not yet confirmed green at handoff.
Signed-off-by: Will Madden <madden@prisma.io>
Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
A corrupt/unparseable ref json was silently dropped (refs -> {}), which
violates the cross-cutting invariant that every disk-content integrity
problem is represented. Add a refUnreadable violation kind (symmetric
with packageUnloadable); an absent head ref stays headRefMissing.
Signed-off-by: Will Madden <madden@prisma.io>
Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
A corrupt or schema-invalid user ref json was previously swallowed to {},
breaking the model invariant that every disk-content integrity problem is
represented. Read refs tolerantly per-file: an unparseable named ref is
omitted from member.refs and surfaced as a refUnreadable violation, while
the well-formed refs are still returned. refs/head.json is excluded from
user-ref reading (it has a distinct head-ref shape) and judged via the
head reader: a corrupt head.json becomes refUnreadable (suppressing
headRefMissing), while a genuinely absent head.json stays headRefMissing.
refUnreadable is an always-checked recoverable kind, not gated behind
declaredExtensions/requireContracts. Construction still never throws.
Also pin the loader's extension-enumeration filter: a reserved-named
(refs) directory and a grammar-invalid directory are excluded from
extensions / listSpaces(), with a valid sibling proving non-exclusion.
Signed-off-by: Will Madden <madden@prisma.io>
Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Re-point the CLI-package consumers of the contract-space aggregate off
the old throw-on-load construction onto loadContractSpaceAggregate plus
an explicit checkIntegrity() gate (slice tolerant-queryable-aggregate).
Behaviour preserved modulo the intended self-edge tolerance and
check-reports-all changes; 5001 / 5002 / PN-MIG-CHECK-* envelopes and the
meta.violations[] shape kept verbatim.
- contract-space-aggregate-loader builds the tolerant aggregate and maps
IntegrityViolation[] onto the 5001 / 5002 envelopes.
- migration check reports all violations at once; status / show / db
verify / apply / migrate consume the model via the tolerant
readMigrationsDir ({ packages, problems }).
- migrate gates with checkIntegrity() before client.connect(), preserving
the refuse-before-connecting safety property (the gate is a pure offline
check, so a broken set never touches a database).
- migration new gates before scaffolding, so a tampered set refuses with
the structured 5002 envelope instead of degrading to NO_INITIAL_MIGRATION.
- migration status refuses on the reader-subset package-corruption gate
(hash- / invariants-mismatch, unloadable manifest); self-edge, head-ref,
layout, and contract drift stay tolerated.
- Rewrite the tamper suite around the per-command-class matrix: gating
commands refuse via 5002; explicit show <path> still throws HASH_MISMATCH.
Signed-off-by: Will Madden <madden@prisma.io>
Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
The uniformity assertion it referenced was removed when the tamper suite was rewritten around the per-command-class matrix. Signed-off-by: Will Madden <madden@prisma.io> Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
`migration check` ran an app-space-only, hand-rolled on-disk pass and was
never wired to the contract-space aggregate's `checkIntegrity()`. The
relocated self-edge (`sameSourceAndTarget`) and the cross-space layout /
contract checks the prior throw-on-load loader enforced were therefore
never re-acquired in `check`, so a populated fixture reported only a
subset of its violations (the spec § Blast radius names `check`, and the
DoD requires "reports all at once").
Fold the full `checkIntegrity({ declaredExtensions, requireContracts })`
violation set into the graph-wide check via the tolerant
`loadContractSpaceAggregate` (not `buildContractSpaceAggregate`, which
collapses to a single refusal — `check` must report every violation, not
bail). The existing on-disk pass is preserved verbatim; the three kinds
it already covers (hashMismatch -> 001, providedInvariantsMismatch /
packageUnloadable -> 002) are skipped from the fold so no fault is
reported twice. Relocated / aggregate-level kinds get sequential codes:
007 sameSourceAndTarget 008 orphanSpaceDir 009 declaredButUnmigrated
010 headRefMissing 011 headRefNotInGraph 012 refUnreadable
013 targetMismatch 014 disjointness 015 contractUnreadable
Rendered / `--json` output shape is unchanged. The fold is best-effort:
if the live contract can't be read the file-level checks still report.
Signed-off-by: Will Madden <madden@prisma.io>
Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Drive every contract-space consumer against a single project planted with three independent faults — a from===to self-edge (no data op), a hash-mismatched package, and an orphan contract-space directory — to pin the per-command-class behaviour of the tolerant model end-to-end: - migration check renders ALL THREE violations in one invocation (PN-MIG-CHECK-001 hash + -007 self-edge + -008 orphan-dir). This is the assertion that failed before the check re-point; it now holds. - migrate (apply) refuses the all-three fixture with PN-MIG-5001 (orphan / layout drift takes precedence) and an integrity-only fixture (hash mismatch, no orphan) with PN-MIG-5002 + meta.violations[]. - migration status pin refuses PN-MIG-5002 on package corruption, while the render path tolerates a self-edge and renders (exit 0). - migration show renders a self-edge package without crashing. DB-free: migrate gates before client.connect, so the stub driver is never reached. db verify shares the identical mapIntegrityViolations gate but runs it post-connect (needs a live marker first); its 5001/5002 surface is pinned by the loader unit test and the real-DB aggregate-schema suite. Signed-off-by: Will Madden <madden@prisma.io> Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
The aggregate fold in `migration check` skipped hashMismatch / providedInvariantsMismatch / packageUnloadable by violation kind, on the assumption the legacy on-disk pass already reports them. But that pass reads the app migrations directory alone, so for those kinds in an extension or orphan space neither path reported them — the corruption was silently dropped. Scope the dedupe to the app space (isAppSpaceLegacyCovered, keyed on APP_SPACE_ID): the same kinds in an extension/orphan space now surface through the fold with their PN-MIG-CHECK code, while app-space duplicates stay suppressed. Covering tests: a declared extension carrying a hash-mismatched package is now reported (located in that space), and the all-three fixture asserts the app-space hash mismatch is reported exactly once. Signed-off-by: Will Madden <madden@prisma.io> Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
The integrity-check catalogue in the Migration System subsystem doc and the glossary stopped at 005, but `migration check` now emits 006 (an unparseable end-contract.json snapshot) plus the aggregate, cross-space codes 007-015: the from===to self-edge (007) and the layout, head-ref, ref, target, disjointness, and contract violations (008-015) that the tolerant aggregate surfaces across the app space and every extension. Add a row per code (described by the fault each represents), correct the stale 001-005 range, and add a scope note clarifying which codes are per-package vs aggregate cross-space and that 001/002 now report across every contract space. Signed-off-by: Will Madden <madden@prisma.io> Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Signed-off-by: Will Madden <madden@prisma.io> Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
The migration metadata model dropped the vestigial labels and hints fields; the cross-consumer integrity fixture still set them, which broke typecheck after rebasing onto main. Signed-off-by: Will Madden <madden@prisma.io> Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
…asts The aggregate db-verify path passed the ContractSpaceMember.contract accessor (a function) to the family verifySchema instead of the deserialized contract, so every schema-checking verify (full and --schema-only) threw and surfaced as "Aggregate verifier introspection failed". Invoke the accessor so the family receives the contract. Also convert the bare as casts introduced by this slice (redundant JSON.parse as unknown, the hash-mismatch details narrowing, and the profileHash read) to typed bindings / runtime checks / castAs so the lint:casts ratchet no longer reports a per-PR increase. Signed-off-by: Will Madden <madden@prisma.io> Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
…emaToSpace ContractSpaceMember.contract is a lazy accessor (a method), but the mongo multi-space runner.s toSpaceMember shim set it as a property value. The as unknown as ContractSpaceMember cast hid the mismatch at compile time; at runtime projectSchemaToSpace called other.contract() and threw "other.contract is not a function", failing every multi-space mongo verify/apply. Return the contract from a thunk so the accessor contract holds. Signed-off-by: Will Madden <madden@prisma.io> Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Fold the PR #626 review findings + operator decisions into the slice spec: harden the tolerant-queryable foundation (one integrity vocabulary, one mapping spine, single-engine migration check, load-once across slice-1 consumers) before slice 2 adds list/graph/log readers. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
…iolation reconstructGraph no longer throws on colliding hashes; checkIntegrity reports duplicateMigrationHash per space. headRefPresentInGraph no longer swallows graph() faults — unexpected engine errors propagate. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Rename IntegrityQueryOptions.requireContracts to checkContracts. Re-export IntegrityViolation, IntegrityQueryOptions, and DeclaredExtensionEntry from @prisma-next/migration-tools/aggregate and drop the /integrity-violation subpath. CLI consumers import from /aggregate and map duplicateMigrationHash to PN-MIG-CHECK-016. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Extract integrityViolationToCheckFailure as the sole PN-MIG-CHECK-* mapper. mapIntegrityViolations meta.violations now carries union kinds (not invented validation/integrity strings). Export loadProblemToViolation so the legacy check pass widens PackageLoadProblem through the shared path. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Remove the app-space legacy dedup and redundant PN-001/PN-002 re-derivation; render all checkIntegrity() violations across spaces. Keep check-only PN-003–006, orphan-empty-dir PN-002, and the target branch unchanged so observable output and cross-consumer behaviour stay the same. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
… (new) Original D8 mixed a uniform load-once fan-out with migration new's distinct gate-narrowing; split for dispatch-INVEST. show->D10, tail->D11. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Split aggregate load and checkIntegrity refusal in the loader, hoist the refusal above --to resolution, and serve app graph and packages from aggregate.app facets instead of loadMigrationPackages. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Load the contract-space aggregate once per invocation, refuse package corruption before --to/--from, and reuse the same aggregate for per-space enumeration and the app graph pipeline. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Package-corruption refusal runs only when loadContractRawSafely succeeds, matching pre-D8 detectPackageCorruption placement. Pin the corrupt-packages + unreadable-contract path so it cannot regress. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Load the contract-space aggregate once and refuse only hash/invariants/ unloadable violations; serve --from and latest-migration resolution from app.packages and app.graph(). Cross-space layout drift no longer blocks new — pinned by tests. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
…gate `migration show` now requires a single app-space target and loads the tolerant contract-space aggregate once (ungated) to resolve packages, graph, and refs. Drops no-target enumeration and direct disk reads. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
…e self-edge wording Head-ref tolerant loading now propagates EACCES/EIO-style failures while still surfacing parse errors as refUnreadable. Recovery hints no longer send users to migrate for faults migrate refuses; self-edge copy reflects missing data invariants. Mongo target builds ContractSpaceMember via the factory; db verify test pins member.contract() resolution. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
wmadden-electric
force-pushed
the
tml-2709-introduce-migrationstore-load-once-queryable-multi-space
branch
from
6dda28c to
b887db0
Compare
vi.mock on the aggregate subpath does not override the binding migration-status already captured when another suite file loads first (isolate: false). Use vi.spyOn on the live export instead. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
Avoid deserializing a partial contract stub when contract.json is missing; the aggregate loader only needs storage hash and target ids. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 30, 2026
Fold the PR #626 review findings + operator decisions into the slice spec: harden the tolerant-queryable foundation (one integrity vocabulary, one mapping spine, single-engine migration check, load-once across slice-1 consumers) before slice 2 adds list/graph/log readers. Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
wmadden
deleted the
tml-2709-introduce-migrationstore-load-once-queryable-multi-space
branch
This was referenced May 30, 2026
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>
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-2715 — slice 1 of the TML-2709 project (one tolerant contract-space model for the migration commands). Prerequisite #603 (TML-2697) is merged; this branch is rebased onto
main. Slice 2 (TML-2716) adopts the model inlist/graph/log.Summary
The migration commands each re-derived "what spaces/refs/graph exist on disk" independently, and integrity was judged while loading — the first problem threw, so
migration checkcould only ever report one fault before bailing. This slice makes the existingContractSpaceAggregatetolerant and queryable, and turns integrity into an explicitcheckIntegrity()query, so every command reads one model and applies its own policy (render / report / refuse).At a glance
Loading no longer throws on disk content; integrity is a query that returns the whole set of problems:
Before: a single tampered package made
migration checkexit after reporting one failure. After: it reports all of them in one invocation.Decision
This slice splits the three responsibilities the old loader fused — build, query, judge — onto the one model the codebase already had (
ContractSpaceAggregate/ContractSpaceMember), rather than introducing a second model:loadContractSpaceAggregate({ migrationsDir, deserializeContract, appContract })never throws on disk content. Every integrity problem becomes a representedIntegrityViolation; genuinely unparseable packages are omitted (packageUnloadable), everything else is retained.ContractSpaceMembercarries rawpackages+ userrefs+ a nullableheadRef, withgraph()/contract()computed lazily and memoised.ContractSpaceAggregategainslistSpaces/hasSpace/space/spaces;app+extensionsstay for the existing planner/verifier/runner.reconstructGraphis now pure (no self-edge throw);checkIntegrity(opts?)returns the fullIntegrityViolation[]. Consumers apply policy: readers render,checkreports all, apply/verify/newrefuse on the gating subset.migration check,status,show,new,plan,db verify, and the apply/migratepath all construct the tolerant aggregate and gate viacheckIntegrity()— preserving the existing5001/5002error envelopes andPN-MIG-CHECK-*codes.list/graph/logare intentionally out of scope (slice 2) — they only add consumers and are pure net-deletion follow-up.How it fits together
reconstructGraphbuilds structure without throwing on afrom === toself-edge;readMigrationsDirreturns{ packages, problems }instead of throwing on a hash mismatch / unparseable manifest. TheIntegrityViolationunion is the shared vocabulary for every represented problem.graph()/contract(); the intra-package apply/verify engine (planner, verifier, strategies, project-schema-to-space) moves onto the new shape in the same change, since the member-shape change is atomic with it.checkIntegrity(). It walks the loaded model and returns all violations — config/contract-dependent checks (layout drift, disjointness, target, contract validation) gated behinddeclaredExtensions/checkContracts.migration checkfolds the fullcheckIntegrity()set (re-acquiring the relocated self-edge + orphan-space-dir checks it lost when the throws were removed), reporting every fault at once.checkreports all three, apply/verify refuse with the structured envelope.Behavior changes & evidence
migration checkreports every fault at once (was: bailed after the first). It now foldscheckIntegrity()across all spaces, including extension/orphan spaces. Impl: migration-check.ts, contract-space-aggregate-loader.ts. Evidence: cross-consumer-integrity.test.ts.from === toself-edge is tolerated by readers and reported (not fatal) bycheck. Impl: migration-graph.ts. Evidence: migration-graph.test.ts, the cross-consumer suite.migrate,migration new,plan, apply/verify) refuse on a tampered set via the structured5001/5002envelopes;migrate/newrefuse offline, before connecting. Impl: migrate.ts, migration-new.ts, control-api/operations/db-verify.ts. Evidence: migration-tamper.test.ts.Round 2 — review-driven consolidation (D5–D11)
The first round delivered the behaviour (tolerant + queryable + report-all), but review found the foundation only half consolidated: two integrity vocabularies, duplicated violation→envelope mappers, a two-path
migration check, and several consumers that loaded the aggregate and then re-readmigrations/anyway. Round 2 closes that so the foundation is structurally whole before slice 2 adds three more readers — and so slice 2 stays pure net-deletion. No slice-2 work was pulled forward.duplicateMigrationHashis now a representedIntegrityViolation(reconstructGraphno longer throws on it); the integrity types are re-exported from the/aggregatesubpath (the standalone/integrity-violationexport is dropped);IntegrityQueryOptions.requireContracts→checkContracts.IntegrityViolation → PN-MIG-CHECK-*mapper per catalogue, replacing the duplicated mapping that had drifted betweenmigration checkand the loader wrapper.check(D7).migration checknow runs a singlecheckIntegrity()engine for both the graph-wide and single-package forms — no parallel legacy path.migrate(3→1),status(4→1), andmigration new(2→1) each load the aggregate exactly once and query facets;loadMigrationPackagesis gone from all three; the integrity refusal is hoisted ahead of any DB connect /--to/--fromresolution.migration shownarrowed + routed (D10).shownow requires a<target>; the argument-less "enumerate every space" path is removed, and the surviving single-migration inspect routes through the aggregate (tolerant, ungated) like every other consumer.EACCES/EIO/unknown) instead of masking it asrefUnreadable; recovery hints no longer send users to a command that now refuses on the same fault; the self-edge wording is reframed around the missing data-invariant; the mongotoSpaceMembershim is built viacreateContractSpaceMember(removing twoas unknown ascasts).Behaviour changes worth a second look
migration showwith no argument now errors (<target>is required). The multi-space "latest per space" enumeration is gone —showmeans nothing without a referent.show <ref>now requires a readable contract (it loads the aggregate): missing →PN-CLI-4004, invalid →PN-CLI-4003. It was previously contract-free. The extra load over a single targeted read is an accepted cost; making the loader tolerant of an absent contract is a deferred optimization.show <path>of a hash-tampered-but-loadable package now renders at exit 0 instead of refusing —showis an inspect verb (integrity ismigration check's job), and the render is faithful to current on-disk content. A genuinely unloadable requested package still surfaces as "not found", so corruption is not masked.migration newnow proceeds on cross-space layout / target / contract drift where it previously refused. Its gate is narrowed to the package-corruption subset (hashMismatch/providedInvariantsMismatch/packageUnloadable); scaffolding a new app migration shouldn't be blocked by an unmaterialised extension.--jsonintegrity shape:meta.violations[].kindnow carries the realIntegrityViolationkinds (D6), and a duplicatemigrationHashsurfaces asPN-MIG-CHECK-016(D5) rather than throwing.Notes for the reviewer
PN-MIG-CHECK-*codes (007–015). WiringchecktocheckIntegrity()means it can now surface integrity kinds it previously couldn't reach. EachIntegrityViolationkind maps 1:1 to a code (007 SELF_EDGE,008 ORPHAN_SPACE_DIR, …015 CONTRACT_UNREADABLE);001/002are reused for app-space package corruption (the legacy on-disk pass owns those). The catalogue is documented in subsystem doc 7 and the glossary. This is the largest user-facing surface change — worth a sanity check.statusintegrity policy.statusrefuses (5002) on package-corruption kinds, renders + tolerates self-edges, and tolerates non-corruption extension drift silently. This is deliberate (a status command should report state, not crash on a self-edge); making it report non-corruption drift is a future spec decision, not a defect here.IntegrityViolation → PN-MIG-CHECK-*mapping (gained behaviour). The genuine net-deletion (~−388) lives inmigration-tools/loader.ts, replacing throw-on-load.5001; an integrity-only fault →5002. The cross-consumer suite pins both.react-router-demobuild and a couple of CLI suites can fail on a cold workspace with a staleemitterdist (@prisma-next/ts-renderresolution) — a freshpnpm buildclears it; andcontract-emit.test.ts's "overlapping emits" race + the redirect tests can flake under full-suite parallelism (500 ms budget) but pass in isolation. None touch this slice.projects/migration-store/(spec, plan, slice spec, design notes, learnings) ride on the branch per the project workflow; they're removed at project close-out (after slice 2).Testing performed
Run on the final rebased HEAD:
pnpm typecheck— green (migration-tools + cli).pnpm --filter @prisma-next/migration-tools test— 538 passed.pnpm --filter @prisma-next/cli test— 1019 passed; the only reds are cold-start/parallelism flakes in two non-slice files (removed-verb-redirects,contract-emit) that pass in isolation (7/7, 8/8).pnpm lint:deps— clean (1000 modules, 0 violations;APP_SPACE_IDcanonical-source check passes).Skill update
n/a — internal + CLI diagnostics only. The new
PN-MIG-CHECK-006..015codes are documented in the Migration System subsystem doc and the glossary (see Reviewer notes); no agent skill encodes the check-code catalogue.Follow-ups
list/graph/logand delete their hand-rolled space/ref/graph I/O (net deletion).readMigrationsDir's manifest-existencestatand the deferred contract reader still bare-catch non-ENOENTfaults (e.g.EACCES), diverging from the now-uniform re-throw policy in the ref-family readers. Out of this slice's scope; worth a dedicated pass for full reader-policy uniformity.projects/migration-store/once slice 2 lands.Alternatives considered
MigrationStoremodel. Rejected: it conceptually overlappedContractSpaceAggregate, and a second model for the same on-disk state is exactly what this project set out to avoid. Evolving the existing aggregate keeps one model.list/graph/log) must render a half-broken project without paying for contract deserialization or config reconciliation. SeparatingcheckIntegrity()from construction lets each command pay only for what it queries.migration new/statuson tolerant reads (no gate). Rejected: a mutating command silently degrading toNO_INITIAL_MIGRATIONon a tampered base, orstatusrendering misleading counts off a corrupt graph, is worse than an explicit refusal. Both gate on the package-corruption subset.Checklist
git commit -s).TML-NNNN: <sentence-case title>form.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation