(TML-2397) feat(cli): show every contract space in migration plan/status/apply output#474
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR refactors migration execution from per-step runner dispatch to descriptor-driven multi-space aggregate execution. It introduces applyAggregate as a shared control-plane operation to run ordered per-space plans, rewrites migration apply to build contract-space aggregates and plan per-space targets, updates CLI commands/formatters to render per-space breakdowns, and implements Mongo as a single-space multi-space shim. ChangesMulti-Space Aggregate Execution
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
@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-telemetry
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@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/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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/1-framework/3-tooling/cli/src/commands/migration-apply.ts (2)
141-147:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThread the resolved ref name into
migrationApply, not just its hash.We resolve
refNamehere, but only passrefHashandrefInvariantsdownstream. That means the aggregate apply path can no longer report the actual user-supplied ref inpathDecision.refNameor invariant-path failures; it has to fall back to synthetic labels instead. Please carry the resolved name through the control-api call as well.Also applies to: 258-264
🤖 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-apply.ts` around lines 141 - 147, The code resolves a user ref into refEntry (via readRefs and resolveRef) but only forwards its hash/invariants downstream, losing the original resolved ref name; update the call(s) to migrationApply (and the other call site around the 258-264 block) to include the resolved ref name (e.g., refEntry.name or refEntry.refName) alongside refHash and refInvariants so pathDecision.refName and invariant-path errors can show the actual user-supplied ref; locate usages of refEntry, refName, refHash, refInvariants and add the resolved name parameter through the control-api call(s).
145-153:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap pre-flight filesystem failures in structured CLI results.
Both catch blocks rethrow unexpected errors, so a permissions/corruption/read failure while resolving refs or loading migration packages will bypass
handleResult()and crash the command instead of returning a CLI envelope. These paths should map toerrorUnexpected(...)just like the other pre-flight I/O in this command.As per coding guidelines, CLI commands must use structured errors (
CliStructuredError), the Result pattern, and should never throw errors except for unhandled failures that fail fast.Also applies to: 207-214
🤖 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-apply.ts` around lines 145 - 153, The catch blocks around readRefs/resolveRef (where refEntry is assigned) and the later block around loading migration packages currently rethrow non-MigrationToolsError errors; change these to return a structured CLI Result by wrapping unexpected filesystem errors with errorUnexpected(...) and returning notOk(...) so the command always yields a CLI envelope instead of throwing. Concretely, in the try/catch that calls readRefs and resolveRef, and in the similar try/catch around loadMigrationPackages, replace the `throw error` path with `return notOk(errorUnexpected(error))` (or map the error into a CliStructuredError via errorUnexpected) while preserving the existing MigrationToolsError handling that calls notOk(mapMigrationToolsError(...)); keep using the same symbols: readRefs, resolveRef, loadMigrationPackages, MigrationToolsError.is, mapMigrationToolsError, notOk, and errorUnexpected so handleResult receives a Result rather than an exception.packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts (1)
280-289:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve extension-space emissions in the no-op and placeholder result paths.
These branches now carry
emittedExtensionDirs, but they still classify the run as plainnoOp/ placeholder-only success.formatMigrationPlanOutput()short-circuits on those flags, so extension-only bumps can write newmigrations/<space>/...directories without ever showing them to the user or surfacing the canonical follow-up apply step. That makesmigration plansilently mutate disk in the exact cross-space case this PR is trying to expose.Also applies to: 417-428
🤖 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-plan.ts` around lines 280 - 289, The no-op and placeholder-only result branches must not short-circuit output when extension emissions exist: when building the MigrationPlanResult in the fromHash === toStorageHash branch (and the similar placeholder-only branch around the block that creates a placeholder result), if extensionMigrationsResult.emitted is non-empty set noOp and placeholder to false (or otherwise clear the short-circuit flags) while still including emittedExtensionDirs in the result object; this ensures formatMigrationPlanOutput (which short-circuits on noOp/placeholder) will surface the emittedExtensionDirs and the follow-up apply step instead of silently mutating disk.
🧹 Nitpick comments (1)
packages/1-framework/3-tooling/cli/src/control-api/operations/apply-aggregate.ts (1)
91-95: ⚡ Quick winDrop the milestone shorthand from this doc comment.
M6 AC4 / AC5is a transient planning reference, so it will go stale in source and violates the repo comment rule. Please describe the behavior directly instead. As per coding guidelines, "Source-code comments must not reference transient project artifacts including ... milestone-task IDs ... [and] milestone-named acceptance criteria."🤖 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/control-api/operations/apply-aggregate.ts` around lines 91 - 95, The doc comment on the readonly field perSpace (type AggregatePerSpaceExecutionEntry[]) uses the transient shorthand "M6 AC4 / AC5"; remove that shorthand and replace it with a plain description of behavior: state that the perSpace entries contain populated markers indicating per-space execution status and any metadata required to build action-specific success envelopes (e.g., success/failure flags, timestamps, and identifiers). Update the comment above the perSpace declaration to describe those markers and their purpose without referencing milestone or acceptance-criteria IDs.
🤖 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/1-core/framework-components/src/control/control-capabilities.ts`:
- Around line 80-87: The comment block mentioning migration behavior contains a
transient milestone ID "TML-2397"; remove that ID and leave a descriptive note
instead (e.g., keep "per the extension-contract-spaces project spec" or similar)
so the comment describes behavior without project artifact IDs; update the
comment near the references to SqlMigrationRunner, Mongo per-space, and the
CLI's applyAggregate primitive accordingly and do not add any other transient
identifiers.
In `@packages/1-framework/3-tooling/cli/src/commands/migration-status.ts`:
- Around line 739-747: Replace the ephemeral empty stack when creating the
validation family: instead of calling config.family.create({} as any), build the
control stack properly and pass a real driver per the guideline (always pass
driver to ControlFamilyDescriptor.create). Instantiate the family via
config.family.create(driver) (or create a minimal/control driver object
appropriate for CLI context) so validateContract uses a real control stack; keep
the subsequent call to familyInstance.validateContract(json) unchanged.
- Around line 757-760: The current reduce unconditionally sums
spaces[].pendingCount into totalPendingAcrossSpaces and converts unknown
(undefined) counts to 0; change the logic to first check that every
aggregateSpaces entry has a defined pendingCount (e.g., Array.prototype.every(s
=> s.pendingCount !== undefined)) and only then compute totalPendingAcrossSpaces
via a reduce; if any pendingCount is undefined, leave totalPendingAcrossSpaces
undefined (or omit it from the emitted object). Apply the same fix to the other
occurrence referenced around lines 1026-1027 so both places only emit the total
when all pendingCount values are known.
- Around line 707-714: The catch currently sets allMarkers = new Map(), which
loadAggregateStatusSpaces() treats as "no marker" for every space; change the
catch to set allMarkers = null (or the equivalent sentinel) when
client.readAllMarkers() is unsupported so the per-space logic preserves the
"marker unknown" fallback; update any type annotations around allMarkers if
needed and ensure client.readAllMarkers() remains invoked in the try block and
loadAggregateStatusSpaces() receives the null value.
In
`@packages/1-framework/3-tooling/cli/src/control-api/operations/apply-aggregate.ts`:
- Around line 144-149: The progress span label is hardcoded to "Applying
migration plan across spaces", which is misleading for other actions; update the
onProgress call that emits kind:'spanStart' (spanId: APPLY_SPAN_ID) to use an
action-specific or neutral label determined from the AggregateApplyAction value
passed in (e.g., add a helper like progressLabelForAction(action) that maps
'dbInit' -> 'Initializing database across spaces', 'dbUpdate' -> 'Updating
database across spaces', 'migrationApply' -> 'Applying migration plan across
spaces', or return a neutral label), then call onProgress with that computed
label so the structured renderer matches the actual command.
- Around line 151-170: The base interface MultiSpaceRunnerPerSpaceOptions is
missing common optional fields used by runners (strictVerification and context),
causing a cast when building perSpaceOptions and hiding type errors; update the
MultiSpaceRunnerPerSpaceOptions interface to include optional properties
strictVerification?: boolean and context?: unknown (or the correct context type
used by runners), then remove the unnecessary casts where perSpaceOptions is
constructed and where runner is narrowed (after hasMultiSpaceRunner()) so
executeAcrossSpaces is called with a correctly typed perSpaceOptions list; keep
family-specific fields (e.g., SQL schemaName) out of the base interface.
In
`@packages/1-framework/3-tooling/cli/src/control-api/operations/migration-apply.ts`:
- Around line 150-160: The current early-continue branch in migration-apply.ts
drops a member from perSpacePlans when its member.migrations.graph.nodes.size
=== 0 and the live marker already equals the target (or live undefined and
target === EMPTY_SENTINEL); instead of continue, append a zero-op resolution for
that space into perSpacePlans (e.g., a per-space plan entry marking zero
migrations applied and status "already-at-head") so the aggregate result and
migrationsTotal count that member, then continue; apply the same change in the
symmetric empty-graph block around the 214-233 region; locate symbols: member,
perSpacePlans, liveMarker.storageHash, targetHash, EMPTY_SENTINEL, and
buildNeverPlannedFailure to implement this behavior.
In
`@packages/1-framework/3-tooling/cli/src/utils/contract-space-aggregate-loader.ts`:
- Line 193: The JSDoc near the function in contract-space-aggregate-loader.ts is
out of date: it claims app-side migration packages are intentionally not
threaded but the code now passes appMigrationPackages via the inputs (see the
appMigrationPackages: inputs.appMigrationPackages ?? [] assignment). Update the
nearby JSDoc to state that appMigrationPackages are now forwarded/propagated
into the aggregate loader (or to describe the new wiring and any implications),
and mention the exact parameter name appMigrationPackages so readers can
correlate docs with the code.
In `@packages/1-framework/3-tooling/cli/src/utils/extension-pack-inputs.ts`:
- Line 11: Remove transient milestone/task identifiers from the comment that
currently reads "This is the AC11 helper for the M6 (extension-contract-spaces)
milestone." (and the similar reference later containing "AC11"/"M6") and replace
them with behavior-only wording describing what the helper does; update the
top-level comment in extension-pack-inputs.ts (and the other inline comment that
mentions AC11/M6) to a concise description of the helper’s purpose and behavior
without any milestone or acceptance-criteria IDs.
In `@packages/2-mongo-family/9-family/src/core/mongo-target-descriptor.ts`:
- Around line 68-74: The inline comment in mongo-target-descriptor.ts contains a
transient milestone ID ("TML-2397"); remove that ID and rephrase the comment to
be behavior-focused—e.g., describe that Mongo has no per-space extension
contracts, the aggregate is always single-member, and that executeAcrossSpaces
is a degenerate shim asserting length === 1 and delegating to execute so
applyAggregate can route through Mongo like the SQL family; update the comment
around the executeAcrossSpaces / applyAggregate explanation (referencing
executeAcrossSpaces and applyAggregate) accordingly.
In `@packages/3-extensions/cipherstash/test/cipherstash-codec.test.ts`:
- Line 157: Locate the inline comment that contains the text "pre-M6" in
cipherstash-codec.test.ts and replace it with a behavior-only statement (for
example: "Legacy wording must not reappear.") so the comment does not reference
a transient milestone; also scan the surrounding comments in the same test to
ensure no other milestone/task identifiers remain.
---
Outside diff comments:
In `@packages/1-framework/3-tooling/cli/src/commands/migration-apply.ts`:
- Around line 141-147: The code resolves a user ref into refEntry (via readRefs
and resolveRef) but only forwards its hash/invariants downstream, losing the
original resolved ref name; update the call(s) to migrationApply (and the other
call site around the 258-264 block) to include the resolved ref name (e.g.,
refEntry.name or refEntry.refName) alongside refHash and refInvariants so
pathDecision.refName and invariant-path errors can show the actual user-supplied
ref; locate usages of refEntry, refName, refHash, refInvariants and add the
resolved name parameter through the control-api call(s).
- Around line 145-153: The catch blocks around readRefs/resolveRef (where
refEntry is assigned) and the later block around loading migration packages
currently rethrow non-MigrationToolsError errors; change these to return a
structured CLI Result by wrapping unexpected filesystem errors with
errorUnexpected(...) and returning notOk(...) so the command always yields a CLI
envelope instead of throwing. Concretely, in the try/catch that calls readRefs
and resolveRef, and in the similar try/catch around loadMigrationPackages,
replace the `throw error` path with `return notOk(errorUnexpected(error))` (or
map the error into a CliStructuredError via errorUnexpected) while preserving
the existing MigrationToolsError handling that calls
notOk(mapMigrationToolsError(...)); keep using the same symbols: readRefs,
resolveRef, loadMigrationPackages, MigrationToolsError.is,
mapMigrationToolsError, notOk, and errorUnexpected so handleResult receives a
Result rather than an exception.
In `@packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts`:
- Around line 280-289: The no-op and placeholder-only result branches must not
short-circuit output when extension emissions exist: when building the
MigrationPlanResult in the fromHash === toStorageHash branch (and the similar
placeholder-only branch around the block that creates a placeholder result), if
extensionMigrationsResult.emitted is non-empty set noOp and placeholder to false
(or otherwise clear the short-circuit flags) while still including
emittedExtensionDirs in the result object; this ensures
formatMigrationPlanOutput (which short-circuits on noOp/placeholder) will
surface the emittedExtensionDirs and the follow-up apply step instead of
silently mutating disk.
---
Nitpick comments:
In
`@packages/1-framework/3-tooling/cli/src/control-api/operations/apply-aggregate.ts`:
- Around line 91-95: The doc comment on the readonly field perSpace (type
AggregatePerSpaceExecutionEntry[]) uses the transient shorthand "M6 AC4 / AC5";
remove that shorthand and replace it with a plain description of behavior: state
that the perSpace entries contain populated markers indicating per-space
execution status and any metadata required to build action-specific success
envelopes (e.g., success/failure flags, timestamps, and identifiers). Update the
comment above the perSpace declaration to describe those markers and their
purpose without referencing milestone or acceptance-criteria IDs.
🪄 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: 404c6f38-8d2f-4a1d-ada4-1d76c802b2ac
⛔ Files ignored due to path filters (2)
projects/extension-contract-spaces/plan.mdis excluded by!projects/**projects/extension-contract-spaces/specs/migration-cli-aggregate.spec.mdis excluded by!projects/**
📒 Files selected for processing (28)
packages/1-framework/1-core/framework-components/src/control/control-capabilities.tspackages/1-framework/1-core/framework-components/src/control/control-migration-types.tspackages/1-framework/3-tooling/cli/src/commands/db-init.tspackages/1-framework/3-tooling/cli/src/commands/db-update.tspackages/1-framework/3-tooling/cli/src/commands/migration-apply.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/control-api/client.tspackages/1-framework/3-tooling/cli/src/control-api/operations/apply-aggregate.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-apply-aggregate.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/contract-space-aggregate-loader.tspackages/1-framework/3-tooling/cli/src/utils/extension-pack-inputs.tspackages/1-framework/3-tooling/cli/src/utils/formatters/migrations.tspackages/1-framework/3-tooling/cli/test/commands/migration-invariants.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-show.test.tspackages/1-framework/3-tooling/cli/test/control-api/client.test.tspackages/1-framework/3-tooling/cli/test/output.db-update.test.tspackages/1-framework/3-tooling/cli/test/output.json-shapes.test.tspackages/1-framework/3-tooling/cli/test/output.migration-commands.test.tspackages/1-framework/3-tooling/migration/src/aggregate/planner-types.tspackages/1-framework/3-tooling/migration/src/aggregate/strategies/graph-walk.tspackages/1-framework/3-tooling/migration/src/exports/aggregate.tspackages/2-mongo-family/9-family/src/core/mongo-target-descriptor.tspackages/3-extensions/cipherstash/src/core/cipherstash-codec.tspackages/3-extensions/cipherstash/test/cipherstash-codec.test.tstest/integration/test/cli.db-update.e2e.test.ts
💤 Files with no reviewable changes (1)
- packages/1-framework/3-tooling/cli/test/control-api/client.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/test/control-api/db-update.test.ts`:
- Around line 610-619: The test uses optional chaining and a fallback when
inspecting executeAcrossSpaces.mock.calls[0]?.[0], which leaves defensive checks
after an assertion; replace this with a double-cast pattern to coerce the mock
return into the expected shape (e.g., cast the call argument using "as unknown
as { perSpaceOptions: ReadonlyArray<{ executionChecks?: unknown }> }"), assert
callArg is defined and that callArg.perSpaceOptions is non-empty (e.g.,
expect(callArg.perSpaceOptions.length).toBeGreaterThan(0)), then iterate over
callArg.perSpaceOptions without optional chaining and assert each
opts.executionChecks is undefined; refer to executeAcrossSpaces, callArg,
perSpaceOptions, and opts.executionChecks when making the changes.
🪄 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: 933e91d4-bc3b-4c3e-9843-e03a3ed8d8ed
📒 Files selected for processing (2)
packages/1-framework/3-tooling/cli/test/control-api/db-update.test.tstest/integration/test/cli.db-init.contract-space-verifier.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/test/utils/extension-pack-inputs.test.ts`:
- Around line 2-8: The import on the tested file mixes a type with value
imports; split the type-only symbol into a top-level "import type" statement and
keep the runtime imports separate: move ExtensionPackInput into an "import type
{ ExtensionPackInput } from '../../src/utils/extension-pack-inputs';" and keep
toDeclaredExtensions, toExtensionInputs, toExtensionMigrationsInputs,
toMigratePassInputs in a plain import from the same module so the file conforms
to the repo TS rule against inline type imports.
🪄 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: d8c4908d-548a-47c2-aee9-85428c4f7ebd
📒 Files selected for processing (2)
packages/1-framework/3-tooling/cli/test/utils/contract-space-migrate-pass.test.tspackages/1-framework/3-tooling/cli/test/utils/extension-pack-inputs.test.ts
wmadden
changed the title
Contributor
Author
Triage of CodeRabbit review summary (review)Decomposed into four sub-findings:
Tracking: |
Contributor
Author
|
Done — A14a (originally deferred to TML-2477 during triage) was implemented in this PR before merge:
TML-2477 will be cancelled (work has landed here). |
Contributor
Author
|
Done on two outside-diff findings from the CodeRabbit review body (no per-thread anchor available):
|
…ive] tag (M6 T6.6) Reword cipherstash codec lifecycle hook labels to action-first / column-first form so first-time users can read them without extension- domain knowledge: Register cipherstash search config for X.Y → Enable cipherstash search on X.Y Remove cipherstash search config for X.Y → Disable cipherstash search on X.Y Rotate cipherstash search config for X.Y → Rotate cipherstash search on X.Y Drop the inline [additive] / [widening] / [destructive] / [mutative] / [data] operationClass tag from default human-readable migration output (migration plan, db init, db update, migration show). The destructive warning still surfaces — both as a per-line "(destructive)" marker and as the existing footer warning — but the additive/widening tags belonged to the planner, not the user reviewing a plan. Closes F2 from projects/extension-contract-spaces/e2e-verification.md; satisfies AC7 + AC8 (M6 sub-spec § Operation labels). Open question 1 (per sub-spec) is settled by dropping the operationClass tag entirely rather than gating it behind --verbose.
…6 follow-up) The two formatter unit tests asserted on the inline [additive] / [destructive] tags M6 T6.6 dropped from default human-readable output. Update the expectations to reflect the new shape: tags are gone from the per-op line; destructive ops keep a "(destructive)" marker on the line itself; the existing footer warning still surfaces. Test fixture update only — no source change. Caught by `pnpm --filter @prisma-next/cli test` after the T6.6 source landed.
Add cli/utils/extension-pack-inputs.ts as the single descriptor-import
boundary for CLI consumers of `Config.extensionPacks`. Every CLI
command / utility that reads an extension descriptor for its
`contractSpace` projection now goes through:
toExtensionInputs(extensionPacks) — canonical projection
-> toDeclaredExtensions(canonical) — aggregate loader
-> toMigratePassInputs(canonical) — migrate-time pass
-> toExtensionMigrationsInputs(canonical) — extension migrations
The structural cast `pack as { contractSpace?: ... }` lives only inside
the canonical helper. AC11 grep:
rg "as \{[^}]*contractSpace\?" packages/1-framework/3-tooling/cli/src/
returns matches in the canonical helper only (one match — the JSDoc
example).
Inline projections in migration-plan.ts (two sites at ~237 and ~263)
go away; the local toDeclaredExtensions / ExtensionPackForAggregate
duplication in contract-space-aggregate-loader.ts goes away. Per-pass
input types continue to live with the consumer (MigrateExtensionInput,
ExtensionMigrationsExtensionInput) — they are the function-parameter
types; the helper imports them.
This is the M6 sub-spec § Required changes 7 work and is a prerequisite
for the per-command rewires later in M6: every command that joins the
aggregate-by-default surface inherits the same projection rather than
re-implementing it.
…/ AC4 / AC5) Replace the single ambiguous `Signature: <hash>` line on db init / db update success with a per-space breakdown that surfaces every space involved in the run, in canonical schedule order (extensions alphabetically, then app), each with its applied operations and its post-apply marker hash. Pipeline: - Add `AggregatePerSpaceExecutionEntry` to control-api types and a `perSpace` field on `DbInitSuccess` / `DbUpdateSuccess`. - `executeAggregateApply` projects its already-ordered `orderedResolutions` into the new shape (`buildPerSpaceBreakdown`) and threads it through the plan / apply success wrappers. - The CLI `db init` / `db update` commands forward `perSpace` onto `MigrationCommandResult`. - Add `formatPerSpaceBlock` to the shared formatter; `formatMigrationApplyOutput` + `formatMigrationPlanOutput` render it in place of the single-line marker block when present. Single-space callers fall back to a labelled `App-space marker:` line (renamed from the ambiguous `Signature:`) — AC4 holds in the degenerate case too. - Top-line summary names the space count: "Applied N operation(s) across M contract spaces"; success footer points at `prisma-next migration status` (AC6). Closes F7 from projects/extension-contract-spaces/e2e-verification.md. Locks AC4 (per-space markers observable) and AC5 (canonical schedule order observable) at the unit level for db init / db update apply mode. The shared formatter is what migration apply (T6.4) will also consume once it walks the aggregate.
… summary (M6 T6.2 / AC3)
Today the multi-space pass already materialises extension-space migration
packages onto disk during `prisma-next migration plan`, but the success
summary names only the app-space directory; the cross-space side effect
(per e2e finding F1) is buried in `ui.step` log lines that scroll above
the success block.
Carry the per-space side effect through into the result and the
success block:
- `MigrationPlanResult.emittedExtensionDirs` is the canonical record of
every extension-space migration package this run wrote to disk
under `migrations/<spaceId>/<dirName>/`. Empty when the project
has no extension packs declaring a contract space, or when every
extension-space package was already on disk.
- The summary line records the cross-space side effect:
"Planned N operation(s); materialised M extension-space migration(s)".
- The success block now lists each space explicitly:
App space → migrations/app/<dirName>
Extension space pgvector → migrations/pgvector/<dirName>
- The "Next:" hint always points at the canonical
`prisma-next migration apply` regardless of how many spaces were
materialised — `db update` is a dev-time convenience, not the
canonical replay step.
A reader of the success block can now tell at a glance which spaces
were touched and which directories were emitted, satisfying AC3 at
both the JSON-result level (`emittedExtensionDirs`) and the human-
readable summary level.
R1 surfaced four orchestrator decisions that translate into spec/plan amendments before R2 is delegated: - T6.4 (`migration apply`): clarify apply path is graph-walk-only (no `planAggregate`, no synth, no introspection); factor the runner-driving tail of `executeAggregateApply` into a shared `applyAggregate` primitive consumed by both `migration apply` and the `db init` / `db update` family. Pin `--ref <hash>` as app-space-only — extensions always advance to head (they own their own ref control). - T6.3 (`migration status`): full rewrite, not the layered alternative. `MigrationStatusResult` becomes aggregate-shaped; per-space rendering throughout; `--ref` / `--graph` / `--limit` interpret consistently for multi-space. - T6.7 (cipherstash example script parity): dropped from M6 scope. The example directory does not exist on this lineage; AC9 demoted to OUT OF SCOPE on the M6 scoreboard. - Pgvector dist subpath import escapee: M4-lineage failure surfaced under M6 R1 `pnpm test:packages`. M6 declares `test:packages` green-modulo- this-escapee, same posture M2 used for `cli.emit-cli-process.e2e`. Plan tasks T6.2 / T6.5 / T6.6 + AC11 are marked landed in R1.
…eApply (M6 T6.4 prep)
Extract the runner-driving tail of `db-apply-aggregate.ts:executeAggregateApply`
into a shared `applyAggregate(aggregate, perSpacePlans, applyOrder, ...)`
primitive that any aggregate apply caller can consume.
The primitive owns:
- the multi-space-runner capability check + dispatch
- the `apply` span emission (start/end with action attribution)
- per-space totals aggregation (`operationsPlanned` / `operationsExecuted`)
- the per-space breakdown projection (`AggregatePerSpaceExecutionEntry[]`)
Each caller still wraps the result into its own action-specific
success envelope (`DbInitSuccess` / `DbUpdateSuccess` /
`MigrationApplySuccess`) — the primitive returns a neutral
`{ orderedResolutions, totalOpsPlanned, totalOpsExecuted, perSpace }`.
`executeAggregateApply` now does:
1. load aggregate
2. read live state (markers + introspect)
3. planAggregate (synth for app, graph-walk for extensions)
4. wrapPlanResult OR applyAggregate → wrapApplyResult
No behaviour change for `db init` / `db update` (validated by full
CLI test suite green at 819/819). The primitive is the seam M6 T6.4
needs so `migration apply` can build `perSpacePlans` via
`graphWalkStrategy` directly (no synth, no introspection — replay
semantics) and reuse the same runner-driving tail.
Sub-spec § Required changes 1: "factor-out commit lands first; all
three callers refactor to consume it in the same slice."
…4 / AC1) `migration apply` is rewired to consume the contract-space aggregate and graph-walk every member (extension spaces alphabetically, then the app member). Replay-only: no introspection, no synth — the user has already planned and committed migration directories; this is the prod-time replay step. Pipeline (M6 sub-spec § migration apply semantics): 1. Load aggregate from disk (M2.5 loader, with the app-space migration packages threaded through to hydrate the app graph). 2. Read live marker rows per space (`familyInstance.readAllMarkers`). 3. Per member: `graphWalkStrategy(member, currentMarker)` plots a path. Empty-graph members fail loudly — a "never planned" space is a user-error condition for replay. 4. Hand off to the shared `applyAggregate` primitive (factored out in fc1764105) — same runner-driving tail `db init` / `db update` already use. Output is per-space (M6 AC4 / AC5): top line names cross-space totals, then a per-space block in canonical schedule order with operations + post-apply marker hash, then a Next: hint pointing at `migration status`. `--ref <hash>` is app-space-only (sub-spec). When provided, the app members graph-walk targets the named hash instead of the contracts `storage.storageHash`. Extension members always walk to their own `headRef.hash` — extensions own their own ref control via `refs/head.json`. `<space>:<hash>` syntax is rejected (out of scope for M6). Other changes: - `BuildAggregateInputs.appMigrationPackages` (optional) lets `migration apply` thread the users authored app-space packages through. `db init` / `db update` continue to pass `[]` (their app-member synth path doesnt walk the app graph). - `MigrationApplyOptions` reshape: removes the legacy `originHash` / `destinationHash` / `pendingMigrations` triple in favour of `contract` / `migrationsDir` / `appMigrationPackages` / `refHash?`. Removes the `MigrationApplyAppliedEntry.dirName` field in favour of `spaceId` (the per-space aggregation). - `MigrationApplyResult.perSpace` always present — same `AggregatePerSpaceExecutionEntry` shape as `db init` / `db update`. - Unknown-invariant pre-check moves post-connect, folding the app-space markers invariants into the union-of-known set (matches todays `(declared by graph) ∪ (already on marker)` behaviour from the legacy command). - `graphWalkStrategy` is now exported from `@prisma-next/migration-tools/aggregate` so the CLI operation can call it without going through the planner. Tests: - `output.json-shapes.test.ts`: pin the new `MigrationApplyResult` wire shape (per-space breakdown + cross-space marker fields). - `output.migration-commands.test.ts`: pin the per-space-block formatter output for the new aggregate-walking apply. - `client.test.ts`: drop the obsolete single-space `migrationApply` describe block — those tests pinned a unit-testable surface that no longer exists (the new operation requires real on-disk fixtures + a multi-space-capable runner). Aggregate-walking coverage moves to T6.8 (pgvector E2E snapshot). - `migration-invariants.test.ts`: stub `readAllMarkers` on the mock family instance so the post-connect pre-check can run; relax the retired-invariant assertion to focus on the pre-check contract (UNKNOWN_INVARIANT must not surface) rather than the downstream apply outcome (the mock environment doesnt wire a runner). Sub-spec § Required changes 1.
Extend MigrationStatusResult with optional `spaces[]` and `totalPendingAcrossSpaces` fields enumerating every on-disk contract space in canonical schedule order (extensions alphabetically, then app). Per-space rows surface the head hash, live marker hash (when online), and pending-migration count computed via graphWalkStrategy. The legacy top-level fields continue to describe the app member specifically; per-space detail for extension members lives only on the new `spaces[]` list. This is the data-shape change for T6.3 (M6 AC2). The formatter update follows in the next commit.
When the result enumerates extension spaces, append a `spaces` section to the human-readable status output: one line per space with status glyph, kind tag ([app]/[ext]), space id, head hash, marker hash, and pending count. When any space has pending work, emit a cross-space pending total + apply hint. Single-space-degenerate parity: when the result lists only the app member, the section is suppressed and the output is identical to the pre-aggregate format. Existing format-status-summary tests continue to pass unchanged. Closes part of T6.3 (M6 AC2).
…gration apply The aggregate-walking migration apply was reporting one applied entry per space (and migrationsApplied = space count), regressing existing JSON-shape consumers (integration tests asserted migrationsApplied === migration-edge count and applied[] === one entry per migration directory). Restore the per-edge view by exposing the chain edges from graphWalkStrategy via a new AggregatePerSpacePlan.migrationEdges field (dirName, migrationHash, from, to, operationCount). The synth strategy leaves it absent. migration apply now flatMaps the edges into applied[] (one entry per authored migration) while keeping the per-space aggregate breakdown on perSpace[]. Also tweak human output to: (a) name both migration count and op count in the summary line so e2e assertions on the word migration(s) keep matching, and (b) render per-space markers as `marker:` instead of `marker →` for parity with the existing single-space `App-space marker:` line.
… apply Restore back-compat with single-space invariant-routing journeys that the M6 T6.4 graph-walk-only rewrite regressed: - Surface pathDecision (requiredInvariants, satisfiedInvariants, selectedPath with per-edge invariants) on MigrationApplySuccess for the app member, derived from the graph-walk strategy. Tests in cli-journeys/invariant-routing.e2e.test.ts read it directly to validate which path the planner chose and why. - Thread refInvariants through the migrationApply control-api so the operation walks against the user-supplied refs ref invariant set rather than the on-disk file head. Previously --ref <name> used the ref hash but ignored its required invariants, so the graph-walk computed required = []. Now the planner honours the ref declaration end-to-end. - Replace the bespoke invariantsUnsatisfiable failure envelope with errorNoInvariantPath so the canonical MIGRATION.NO_INVARIANT_PATH meta (code, required, missing, structuralPath) flows out unchanged — the cli-journeys suite asserts on that exact shape. Threads the new fields through AggregatePerSpacePlan, the client.migrationApply options, and the CLI commands result type.
The M6 T6.5/T6.6 work renamed the single-space Signature: line to per-space marker: (or App-space marker: when only one space is visible). Update the integration assertion to match the new wording.
…(M6)
The aggregate apply primitive was passing
`executionChecks: { prechecks: false, postchecks: false, idempotencyChecks: false }`
to every per-space runner invocation, suppressing the framework's
postcondition-pre-check idempotency mechanism (ADR 038 — operations
whose postconditions already hold are skipped on replay). The
suppression was authored back in M2.5's multi-space `db init` /
`db update` slice (commit 43453b5 — "feat(cli,migration-tools):
per-space db init/update via executeAcrossSpaces") with no rationale
in the commit message. It went undetected in those flows because
`db init` runs greenfield (postchecks irrelevant) and `db update`
synth-generates ops that genuinely need to fire.
M6 T6.4 routes `migration apply` through the same primitive, where
the suppression breaks ADR 038's replay semantics: a re-applied
addColumn op fails with `column already exists` instead of being
skipped after the runner observes that the postcondition is already
satisfied.
Removing the override lets each runner fall back to its defaults
(SQL family: all three checks enabled) so the idempotency probe
fires as designed.
Verified by:
- `cli-journeys/invariant-routing.e2e.test.ts > Journey R`
(rollback marker.storageHash → re-apply → marker advances back)
fails before the patch with PN-RUN-3000 "column already exists";
passes after.
- `cli-journeys/data-transform-enum-rebuild.e2e.test.ts`
(enum rebuild + data transform ordering) fails before the patch
with the same root cause; passes after.
- All 6/6 invariant-routing journeys pass after the patch.
3 mongo invariant-routing journeys still fail with a different root
cause (Mongo target's runner pathway through the new `migration apply`)
and need separate triage.
…ce aggregates M6 routes `migration apply` / `db init` / `db update` through the shared `applyAggregate` primitive, which requires `MultiSpaceCapableRunner`. Mongo per-space is a non-goal per the extension-contract-spaces project spec (TML-2397), so its aggregate is always single-member -- but it still needs to participate in the aggregate dispatch path uniformly with the SQL family. Adds a degenerate `executeAcrossSpaces` to the Mongo target descriptor: asserts perSpaceOptions.length === 1 and delegates to the existing single-space `execute()`. Restores the 3 Mongo invariant-routing journeys that M6 regressed (apply init was failing before any DB activity with "Runner for target mongo does not implement executeAcrossSpaces"). Updates the docstrings on `MultiSpaceCapableRunner` and `hasMultiSpaceRunner` to reflect that Mongo now implements the capability via a single-space shim.
Two stale assertions surfaced under M6 CI: 1. db-update unit test asserted executeAcrossSpaces was called with prechecks/postchecks/idempotencyChecks all disabled. That codified the M2.5-lineage bug that commit e6c2c87 removed. Flip the test to assert the inverse: applyAggregate does NOT opt out of runner checks, so re-apply remains idempotent per ADR 038. 2. db-init contract-space verifier integration test did naive JSON.parse(consoleOutput.join) on the merged stdout/stderr capture. M6 per-space output emits extra decoration on the same stream, making the merged blob occasionally non-JSON in CI. Switch to the robust parseJsonObjectFromCliCapture helper (same pattern as the companion db-update verifier test).
- New test file covers all four functions in extension-pack-inputs.ts (toExtensionInputs, toDeclaredExtensions, toMigratePassInputs, toExtensionMigrationsInputs) including both contractSpace-present and contractSpace-absent branches per pack. AC11 helper goes from 41% statement coverage to 100%. - Add two cases to contract-space-migrate-pass.test.ts covering the priorHeadHash present and absent (first-run drift) branches of formatContractSpaceDriftWarning. Closes the pre-existing 87.5% branch gap that surfaced after the AC11 helper inflated the package report.
…upported When client.readAllMarkers() throws (older family without support), migration-status now leaves the per-space marker map as null so the aggregate loader can render rows in the marker-unknown shape rather than the marker-empty shape, which previously misreported every space as "(no marker)" with concrete pending counts. Refs: TML-2397
Replace the `config.family.create({} as any)` stub with a properly
composed control stack so the validateContract callback runs against a
fully-typed family instance. Drops the three `biome-ignore noExplicitAny`
suppressions in the aggregate-status block. Descriptors that read stack
members during construction now see a consistent view.
Refs: TML-2397
Summing per-space `pendingCount` with `?? 0` silently turned offline / marker-unknown rows into "zero pending", which converted the JSON output for `migration status` from "unknown" into "none". Extract a `computeTotalPendingAcrossSpaces` helper that returns `undefined` whenever any space lacks a defined `pendingCount`, and omit the field from the result envelope in that case so consumers can tell the two conditions apart. Refs: TML-2397
`applyAggregate` is shared by `db init`, `db update`, and `migration apply`, but the `spanStart` label was hardcoded to "Applying migration plan across spaces" — which read incorrectly in structured-progress output for the non-migration surfaces. Switch over the action union to emit an action-appropriate label per surface. Extends progress coverage with focused per-action assertions on the label. Refs: TML-2397
The hardcoded "Applying migration plan across spaces" label was inaccurate for the db init / db update surfaces, which now share the same primitive. Introduce progressLabelForAction so each action emits an action-appropriate spanStart label, and cover the mapping with a dedicated unit test plus the existing dbInit progress test. Refs: TML-2397
migration apply was silently dropping empty-graph members whose live marker already matched the target hash, so the success envelope did not include them in perSpace[] and the summary undercounted loaded contract spaces. Track those resolutions in a separate map and merge them back into the canonical order so every loaded member surfaces in the result with a zero-op breakdown. Refs: TML-2397
…loader The JSDoc still claimed app-side migration packages were intentionally not threaded through, but `migration apply` now forwards `inputs.appMigrationPackages` so the graph-walk strategy can plot a path through them. Update the comment to match. Refs: TML-2397
Drop the AC11 / M6 milestone shorthand from the file-level and ExtensionPackLike comments and describe the boundary in behavioural terms instead. Refs: TML-2397
…/ placeholder branches When `migration plan` short-circuits (true no-op or placeholder-only emission) but extension contracts have bumped, the human renderer was hiding the new `migrations/<spaceId>/<dirName>/` directories the framework just wrote. Users would see "No changes detected" and skip running apply, even though extension migrations had appeared on disk. Render the emitted extension block + canonical apply hint in both short-circuit branches alongside the full-plan branch. JSON output already included `emittedExtensionDirs`; no schema change.
…te suite Use the double-cast pattern to declare the call-arg shape up front, then assert presence + non-empty perSpaceOptions before iterating. Drops the post-assertion optional chaining the previous shape required.
…ding The cli package runs vitest with isolate: false, so an earlier test in the control-api directory (client.test.ts) was evaluating migration-apply with the real contract-space-aggregate-loader bound. The at-head test's vi.mock was hoisted but did not replace the already-resolved binding, causing the real loader to run and fail with a contract target mismatch. Reset modules and re-import executeMigrationApply in beforeEach so this file's mock is the active binding when the test runs.
…ision The strategy already forwards required-invariant state to findPathWithDecision; extend it to forward the optional refName decoration too. Pure decoration — the path-finding algorithm itself does not interpret the name. Lets callers that walk an app-space ref (today: executeMigrationApply) name the user-supplied --ref in the resulting PathDecision and in any unsatisfiable invariant-path error instead of a synthetic placeholder.
When a user runs migration apply --ref prod and the on-disk graph cannot satisfy a required invariant, the MIGRATION.NO_INVARIANT_PATH envelope was naming the ref as the synthetic literal "app-ref" instead of "prod". Same substitution happened in pathDecision.refName surfaced via structured progress. Thread the resolved ref name from the CLI command through migrationApply into executeMigrationApply, then pass it to graphWalkStrategy for the app member only (extensions own their own ref control per the contract-spaces spec). The error envelope and pathDecision now carry the literal the user typed. Refs: TML-2477
…llution) The cli package runs vitest with isolate: false (forced by process.chdir usage), which shares the module graph across test files in the same worker. The earlier client.test.ts evaluates control-api/operations/migration-apply transitively with the real contract-space-aggregate-loader bound; the at-head test would then hoist its own vi.mock too late to replace the captured binding. Various rescue attempts (resetModules at module top, isolateModulesAsync, beforeEach reset) either failed to replace the binding or polluted sibling files. The empty-graph at-head branch is small and obvious, and the behavior surfaces in the success envelope perSpace[] list that the contract-space cli-journey suites already exercise end-to-end.
wmadden
force-pushed
the
tml-2397-migration-cli-aggregate
branch
from
d87e5fa to
e437113
Compare
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
1 participant
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.
At a glance
Before this PR,
migration applycollapsed everything into one line:The user couldn't tell that two contract spaces (an extension's and the app's) had moved, or in what order, or where each one's marker ended up. After this PR:
migration planandmigration statusget the same per-space treatment.db init/db updatealready moved to this shape internally in M2.5; this PR finishes the migration of the user-facing surface.The decision
The repo already has a contract-space aggregate type (one app space + zero-or-more extension spaces) and an aggregate-aware planner that the
dbcommands use. Themigrationcommands hadn't caught up — they each had their own app-space-only code path.We collapse the apply path to one shared primitive so all three commands route through it:
executeAggregateApply(used bydb init/db update) intoapplyAggregate(aggregate, perSpacePlans, …).migration applyto load the aggregate, rungraphWalkStrategyper member to plot the on-disk path from the current marker to head, then hand off toapplyAggregate. NoplanAggregatecall on this path — see "Why noplanAggregateonmigration apply" below.migration planandmigration statusresults to be aggregate-shaped (spaces[]plus cross-space totals) and render per-space blocks.One apply primitive, two callers (
dbfamily andmigration apply); they only differ in how each builds itsperSpacePlans.Walkthrough
Why no
planAggregateonmigration applyplanAggregateis the aggregate-aware planner. For each member it picks one of two strategies —synthStrategy(introspect the live DB, diff against the contract IR, generate ops) orgraphWalkStrategy(plot a path through committed on-disk migration directories). Thedbfamily needs the synth path because that's how it generates the migration from scratch.migration applyis the prod-time replay step. The user has already runmigration plan; the directories are committed; there's nothing to synthesize. CallingplanAggregatehere would either accidentally invoke synth (and thus introspection) or require the planner to learn a "replay-only" flag that gates half its behavior off. Cleaner to callgraphWalkStrategydirectly per member and skip the planner entirely.This keeps a clean planner-vs-replay boundary in the call graph: introspection lives behind
planAggregate;applyAggregateis pure dispatch over already-resolved plans.--ref <hash>is app-space-onlyWhen the user passes
--ref prod, the app-space graph-walk targets the named hash instead of head; extension spaces always advance to their ownheadRef.hash. Extensions own their own ref control viarefs/head.json. This is consistent with how extensions own their contract-space lifecycle generally.Operation labels lose the
[additive]tagThe cipherstash extension's labels were the canary — they read
[additive] adds search config column to user.email, which is double-wrapping: the user can tell something is additive from the verb. Reworded to action-first / column-first (Add encrypted column "email" to "user") and the[additive]/[mutative]/[destructive]tag is dropped from the default human-readable line entirely. Destructive operations still get(destructive)highlighted because that's a different signal — "this will delete things", not "this falls in category X".Two regressions surfaced once the surface lit up
Wiring
migration applythroughapplyAggregateimmediately surfaced two existing-but-hidden bugs. Fixed in this same PR rather than deferred, because the new tests would have stayed red:applyAggregatewas explicitly settingexecutionChecks: { prechecks: false, postchecks: false, idempotencyChecks: false }on every per-space invocation — an override that pre-dated this PR but had been invisible because no caller exercised re-apply through it. With the override removed, the runner's ADR-038 postcondition-already-satisfied probe runs again, so re-applying a migration whose ops are already on disk silently skips them instead of re-issuing the DDL (which then blew up withcolumn "name" of relation "user" already exists).MultiSpaceCapableRunner. Mongo per-space is a non-goal — its aggregate is always single-member — butapplyAggregatestill wants to dispatch through one uniform interface, so it threw upfront on every Mongomigration apply. Added a degenerate single-spaceexecuteAcrossSpacesshim on the Mongo target descriptor that assertsperSpaceOptions.length === 1and delegates to existingexecute(). Cleaner than special-casing insideapplyAggregate, since the constraint belongs to the target.Verification
The behavior that's worth eyeballing on a checkout:
prisma-next migration planon an app that uses pgvector or cipherstash now shows one block per space.prisma-next migration applyshows per-space markers + applied directories at the bottom; noSignature:line.prisma-next migration apply --ref <hash>(single-space PG/SQLite) is unchanged — extensions advance to head, app targets the named hash.prisma-next migration applyon Mongo behaves identically to before.Suite-level:
pnpm typecheck/pnpm lint:deps/pnpm build: green.mongo-migration.e2e,data-transform-enum-rebuild.e2e: green.adopt-migrations,brownfield-adoption,drift-schema,ref-routing,migration-status-diagnostics) flake under the full parallel sweep but pass in isolation — pre-existing suite-level pollution on shared PG connections, not introduced here.Compatibility / migration / risk
Signature:line will need to look atmarker:per space. Anything consuming--jsonshould be unaffected — the JSON envelope shape ondb init/db updatealready moved tospaces[]under M2.5;migration applyandmigration statusJSON now match.MigrationRunnerResultMongo always returned.Alternatives considered
migration statusrewrite (wrap the existing single-space view and add an extensions section underneath). Rejected: the single-space view assumes a single graph, a single head ref, and one set of pending counts. Wrapping it meant either rendering the app twice (once in the wrap, once in the extensions list) or maintaining two divergent render paths. Full rewrite ofMigrationStatusResultto be aggregate-shaped was cleaner.migration applycallplanAggregatewith a "replay-only" flag. Rejected: it forces the planner to grow a mode that disables half its behavior, and keeps the temptation to introspect-on-apply alive. CallinggraphWalkStrategydirectly keeps the planner-vs-replay boundary explicit in the call graph.applyAggregatefor runners withoutexecuteAcrossSpaces. Rejected: it puts a target-specific constraint ("Mongo aggregates are always single-member") in a target-neutral primitive. Putting the shim on the Mongo target descriptor keeps the constraint where it belongs.[additive]tag behind--verbose. Rejected: there's no real use case for the tag after the labels were reworded to action-first; surfacing it only with a flag is dead weight in the formatter.Follow-ups
--graph/--limitflag semantics onmigration status(single-space behavior preserved; per-space rendering doesn't depend on this).main; not in this PR to keep the diff focused).examples/cipherstash-integration/script parity — the example directory doesn't exist on this branch's lineage; if it surfaces under TML-2373, the script work belongs there.Non-goals
hasMultiSpaceRunnercapability guard (TML-2464, once every target is aggregate-native).