(TML-2397) M2 — Contract-space framework: codec hooks, db init/update, aggregate refactor, uniform on-disk layout#438
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds contract-space aggregate loader/planner/verifier, multi-space runner protocol and type-guard, per-space CLI orchestration (db init/update/verify) with app-scoped migrations paths, family/adapter runner and adapter updates (readAllMarkers, schemaVerifyAgainstSchema), field-event codec hooks and RawSql pass-through, many unit/integration tests, and updated examples/fixtures. ChangesContract-space & Multi-space Core
CLI Orchestration
Family / Targets / Adapters
Tests, Examples, Docs
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
4 tasks
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@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: |
wmadden
force-pushed
the
tml-2397-contract-spaces-first-class-schema-contributions-from
branch
from
6eeed25 to
a210c79
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
packages/2-sql/9-family/src/core/migrations/field-event-planner.ts (1)
162-170: 💤 Low valueDocumentation/implementation alignment check for
isAlterationpredicate.The docstring states "
'altered'is suppressed when onlycodecIddiffers," but the implementation at line 168 suppressesalteredwhencodecIddiffers at all—even if other properties also changed. This is a more conservative approach that may be intentional (codec rotation being a v1 non-goal), but the docstring should reflect the actual behavior.📝 Suggested docstring clarification
/** - * `'altered'` predicate: returns `true` when any property other than - * `codecId` differs. Pure-`codecId` changes are excluded — codec rotation - * is a v1 non-goal (project spec § Non-goals). + * `'altered'` predicate: returns `true` when `codecId` is unchanged and + * any other property differs. Any `codecId` change (alone or combined + * with other changes) suppresses the event — codec rotation is a v1 + * non-goal (project spec § Non-goals). */🤖 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/2-sql/9-family/src/core/migrations/field-event-planner.ts` around lines 162 - 170, The docstring for isAlteration and its predicate behavior are inconsistent with the implementation: the function currently returns false whenever codecId differs (suppressing 'altered' even if other fields also changed). Update the docstring to state that any difference in codecId will suppress the 'altered' predicate (i.e., codecId changes always cause isAlteration to return false), and reference the function name isAlteration and helper sameStorageColumn/StorageColumn so reviewers can verify the described behavior matches the implementation.
🤖 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/db-init.ts`:
- Around line 125-134: runContractSpaceVerifierPrecheck(...) is called before
the try/finally that closes the database client, so if it throws the client
isn't closed; move the precheck call inside the existing try block (or wrap it
in a new try/finally) so that client.close() in the finally always runs, and
preserve the existing handling that returns notOk(precheckResult.failure) when
precheckResult.ok is false; reference runContractSpaceVerifierPrecheck,
client.close, and notOk to locate the code to adjust.
In
`@packages/1-framework/3-tooling/cli/src/control-api/operations/db-apply-per-space.ts`:
- Around line 176-184: The MigrationPlan being built after
readPinnedSpaceContract omits destination.profileHash and uses a placeholder
cast for targetId; update the plan construction so destination includes
destinationContract.profileHash (and origin includes currentMarkerProfileHash
when present) instead of only storageHash, and remove the '' as unknown as
MigrationPlan['targetId'] cast by deferring targetId assignment via a pending
context (e.g., PendingExtensionContext / perSpaceContexts) so appPlan.targetId
is injected later; ensure markerMatchesDestination and
ensurePlanMatchesDestinationContract will then correctly validate profile
hashes; apply the same fix for the similar construction at the other occurrence
around the 237–240 region.
In `@packages/1-framework/3-tooling/migration/src/io.ts`:
- Around line 121-123: The code joins pkg.dirName into targetDir without
validating it, allowing directory traversal like "../x" or multi-segment names;
before calling join(targetDir, pkg.dirName) (used to compute dir for
writeMigrationPackage and writeFile), validate/sanitize pkg.dirName to ensure it
is a single safe path segment: reject or sanitize values that contain "..", path
separators ("/" or "\"), absolute paths, or percent-encoded traversal, or
alternatively compute dir = resolve(targetDir, pkg.dirName) and verify
dir.startsWith(resolve(targetDir) + path.sep) and throw an error if the check
fails; apply this check where dir is constructed so writeMigrationPackage(dir,
...) and writeFile(join(dir, 'contract.json'), ...) cannot escape the intended
root.
In `@packages/1-framework/3-tooling/migration/src/verify-contract-spaces.ts`:
- Around line 211-220: The current check only computes missing = pinned -
marker, allowing extra invariants in marker to go unnoticed; update verify logic
around pinnedInvariants/markerInvariants so you perform a symmetric set
comparison: compute missing = pinned \ marker and extra = marker \ pinned (use
pinnedInvariants and markerInvariants Sets), and treat either non-empty as an
invariantsMismatch (include both missing and extra in the violation payload and
remediation message); also add a regression test that creates a marker row
containing an unexpected extra invariant to assert the new extra-path triggers a
violation (refer to pinnedInvariants, markerInvariants, missing, and the
violations push site).
In `@packages/2-sql/9-family/src/core/control-instance.ts`:
- Around line 606-610: The call in readAllMarkers currently invokes
getControlAdapter().readAllMarkers but getControlAdapter only validates the
presence of introspect/readMarker, so adapters missing readAllMarkers will throw
at runtime; update the validation to ensure the adapter exposes readAllMarkers
(or perform an explicit capability check in readAllMarkers before calling the
method) — e.g., extend getControlAdapter's capability checks to include
'readAllMarkers' (in addition to 'introspect'/'readMarker') or add a guard in
the readAllMarkers function that asserts the adapter implements readAllMarkers
and throws a clear error if not.
- Around line 317-323: The assertion call to assertDescriptorSelfConsistency is
using a blind double-cast for the storage field; remove the "as unknown as
Record<string, unknown>" and pass contractJson.storage directly (SqlStorage is
structurally assignable to Record<string, unknown>) or, if a runtime check is
needed, add a small type-narrowing helper that validates/returns the storage as
Record<string, unknown> before calling assertDescriptorSelfConsistency; update
the call site referencing assertDescriptorSelfConsistency and
contractJson.storage accordingly.
In `@packages/3-extensions/test-contract-space/README.md`:
- Around line 3-5: The README links to a transient projects path
(`projects/extension-contract-spaces`) which may move; update the README for
packages/3-extensions/test-contract-space to reference a durable documentation
location (for example a stable docs site URL, a canonical spec, or an internal
docs/ file) instead of the `projects/` path—locate the phrase "contract-space
mechanism" and the markdown link that points to
`projects/extension-contract-spaces` and replace it with the permanent reference
or remove the link while keeping the explanatory text.
In `@packages/3-extensions/test-contract-space/src/exports/control.ts`:
- Around line 29-40: The descriptor object testContractSpaceExtensionDescriptor
is missing the required manifest property (type ExtensionPackManifest); add
manifest: { id: TEST_SPACE_ID, version: '0.0.1' } to the descriptor so it
matches other control plane descriptors and the expected shape used by
ExtensionPackManifest (ensure you reference TEST_SPACE_ID and the version string
exactly as shown).
In `@packages/3-targets/3-targets/sqlite/src/core/migrations/runner.ts`:
- Around line 208-225: The FK integrity check currently hard-codes failingSpace:
'app' after the per-space loop in SqlMigrationRunner (around fkWasEnabled and
verifyForeignKeyIntegrity); instead, track the last processed space while
iterating perSpaceOptions (e.g., set a local lastProcessedSpace variable when
you set space from spaceOptions and after executeOnConnection succeeds) and use
that lastProcessedSpace as the failingSpace in the notOk return for
fkIntegrityCheck; update references in the block containing fkWasEnabled,
verifyForeignKeyIntegrity, perSpaceOptions, and executeOnConnection to use the
tracked lastProcessedSpace rather than the literal 'app'.
In
`@packages/3-targets/6-adapters/sqlite/test/migrations/db-apply-per-space.cli.test.ts`:
- Line 2: Replace the POSIX-only path handling: stop using
db.path.replace(/\/test\.db$/, '') to compute tmpDir and instead compute tmpDir
with dirname(db.path); also swap imports from 'node:path' to 'pathe' (update the
import of join and add dirname from 'pathe') and update any other occurrences
(see usages around db.path.replace and the tmpDir variable and the join(...)
calls in the test and at lines ~214-220) so path operations are cross-platform.
---
Nitpick comments:
In `@packages/2-sql/9-family/src/core/migrations/field-event-planner.ts`:
- Around line 162-170: The docstring for isAlteration and its predicate behavior
are inconsistent with the implementation: the function currently returns false
whenever codecId differs (suppressing 'altered' even if other fields also
changed). Update the docstring to state that any difference in codecId will
suppress the 'altered' predicate (i.e., codecId changes always cause
isAlteration to return false), and reference the function name isAlteration and
helper sameStorageColumn/StorageColumn so reviewers can verify the described
behavior matches the implementation.
🪄 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: 40ba1083-dfeb-4fe0-b641-6226dc5dd977
⛔ Files ignored due to path filters (5)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/extension-contract-spaces/plan.mdis excluded by!projects/**projects/extension-contract-spaces/spec.mdis excluded by!projects/**projects/extension-contract-spaces/specs/cipherstash-migration.spec.mdis excluded by!projects/**projects/extension-contract-spaces/specs/framework-mechanism.spec.mdis excluded by!projects/**
📒 Files selected for processing (128)
examples/react-router-demo/test/react-router.smoke.e2e.test.tspackages/1-framework/1-core/framework-components/src/control/control-capabilities.tspackages/1-framework/1-core/framework-components/src/control/control-instances.tspackages/1-framework/1-core/framework-components/src/control/control-migration-types.tspackages/1-framework/1-core/framework-components/src/exports/control.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/db-verify.tspackages/1-framework/3-tooling/cli/src/commands/migration-plan.tspackages/1-framework/3-tooling/cli/src/control-api/client.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-apply-per-space.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-init.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-update.tspackages/1-framework/3-tooling/cli/src/control-api/types.tspackages/1-framework/3-tooling/cli/src/exports/control-api.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-extension-migrations-pass.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-migrate-pass.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-verifier-marker-check.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-verifier-precheck.tspackages/1-framework/3-tooling/cli/test/config-types.test.tspackages/1-framework/3-tooling/cli/test/utils/contract-space-extension-migrations-pass.test.tspackages/1-framework/3-tooling/cli/test/utils/contract-space-migrate-pass.test.tspackages/1-framework/3-tooling/cli/test/utils/contract-space-verifier-marker-check.test.tspackages/1-framework/3-tooling/cli/test/utils/contract-space-verifier-precheck.test.tspackages/1-framework/3-tooling/migration/package.jsonpackages/1-framework/3-tooling/migration/src/assert-descriptor-self-consistency.tspackages/1-framework/3-tooling/migration/src/compute-extension-space-apply-path.tspackages/1-framework/3-tooling/migration/src/concatenate-space-apply-inputs.tspackages/1-framework/3-tooling/migration/src/detect-space-contract-drift.tspackages/1-framework/3-tooling/migration/src/emit-pinned-space-artefacts.tspackages/1-framework/3-tooling/migration/src/errors.tspackages/1-framework/3-tooling/migration/src/exports/errors.tspackages/1-framework/3-tooling/migration/src/exports/io.tspackages/1-framework/3-tooling/migration/src/exports/spaces.tspackages/1-framework/3-tooling/migration/src/gather-disk-contract-space-state.tspackages/1-framework/3-tooling/migration/src/io.tspackages/1-framework/3-tooling/migration/src/plan-all-spaces.tspackages/1-framework/3-tooling/migration/src/read-pinned-contract-hash.tspackages/1-framework/3-tooling/migration/src/read-pinned-head-ref.tspackages/1-framework/3-tooling/migration/src/read-pinned-space-contract.tspackages/1-framework/3-tooling/migration/src/space-layout.tspackages/1-framework/3-tooling/migration/src/verify-contract-spaces.tspackages/1-framework/3-tooling/migration/test/assert-descriptor-self-consistency.test.tspackages/1-framework/3-tooling/migration/test/concatenate-space-apply-inputs.test.tspackages/1-framework/3-tooling/migration/test/deletable-node-modules.test.tspackages/1-framework/3-tooling/migration/test/detect-space-contract-drift.test.tspackages/1-framework/3-tooling/migration/test/emit-pinned-space-artefacts.test.tspackages/1-framework/3-tooling/migration/test/gather-disk-contract-space-state.test.tspackages/1-framework/3-tooling/migration/test/plan-all-spaces.test.tspackages/1-framework/3-tooling/migration/test/read-pinned-contract-hash.test.tspackages/1-framework/3-tooling/migration/test/read-pinned-head-ref.test.tspackages/1-framework/3-tooling/migration/test/space-layout.test.tspackages/1-framework/3-tooling/migration/test/verify-contract-spaces.test.tspackages/1-framework/3-tooling/migration/test/write-extension-migration-package.test.tspackages/1-framework/3-tooling/migration/tsdown.config.tspackages/2-mongo-family/9-family/src/core/control-instance.tspackages/2-sql/5-runtime/src/exports/index.tspackages/2-sql/5-runtime/src/sql-marker.tspackages/2-sql/5-runtime/test/intercept-decoding.test.tspackages/2-sql/5-runtime/test/marker-vs-intercept-ordering.test.tspackages/2-sql/5-runtime/test/sql-family-adapter.test.tspackages/2-sql/5-runtime/test/sql-marker.test.tspackages/2-sql/5-runtime/test/sql-runtime.test.tspackages/2-sql/5-runtime/test/utils.tspackages/2-sql/9-family/src/core/control-adapter.tspackages/2-sql/9-family/src/core/control-instance.tspackages/2-sql/9-family/src/core/migrations/field-event-planner.tspackages/2-sql/9-family/src/core/migrations/types.tspackages/2-sql/9-family/src/exports/control.tspackages/2-sql/9-family/test/control-instance.descriptor-self-consistency.test.tspackages/2-sql/9-family/test/field-event-planner.test.tspackages/2-sql/9-family/test/migrations.types.test-d.tspackages/3-extensions/sql-orm-client/test/integration/runtime-helpers.tspackages/3-extensions/test-contract-space/README.mdpackages/3-extensions/test-contract-space/biome.jsoncpackages/3-extensions/test-contract-space/package.jsonpackages/3-extensions/test-contract-space/src/core/constants.tspackages/3-extensions/test-contract-space/src/core/contract.tspackages/3-extensions/test-contract-space/src/core/migrations.tspackages/3-extensions/test-contract-space/src/exports/control.tspackages/3-extensions/test-contract-space/test/descriptor.test.tspackages/3-extensions/test-contract-space/tsconfig.jsonpackages/3-extensions/test-contract-space/tsconfig.prod.jsonpackages/3-extensions/test-contract-space/tsdown.config.tspackages/3-extensions/test-contract-space/vitest.config.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner.tspackages/3-targets/3-targets/postgres/src/core/migrations/runner.tspackages/3-targets/3-targets/postgres/src/core/migrations/statement-builders.tspackages/3-targets/3-targets/postgres/src/exports/statement-builders.tspackages/3-targets/3-targets/postgres/test/migrations/statement-builders.test.tspackages/3-targets/3-targets/sqlite/src/core/migrations/op-factory-call.tspackages/3-targets/3-targets/sqlite/src/core/migrations/operations/raw.tspackages/3-targets/3-targets/sqlite/src/core/migrations/planner.tspackages/3-targets/3-targets/sqlite/src/core/migrations/runner.tspackages/3-targets/3-targets/sqlite/src/core/migrations/statement-builders.tspackages/3-targets/3-targets/sqlite/src/exports/migration.tspackages/3-targets/3-targets/sqlite/src/exports/op-factory-call.tspackages/3-targets/3-targets/sqlite/src/exports/statement-builders.tspackages/3-targets/6-adapters/postgres/src/core/adapter.tspackages/3-targets/6-adapters/postgres/src/core/control-adapter.tspackages/3-targets/6-adapters/postgres/test/adapter.test.tspackages/3-targets/6-adapters/postgres/test/migrations/marker-schema-migration.integration.test.tspackages/3-targets/6-adapters/postgres/test/migrations/planner.codec-field-event.test.tspackages/3-targets/6-adapters/postgres/test/migrations/runner.basic.integration.test.tspackages/3-targets/6-adapters/postgres/test/migrations/runner.errors.integration.test.tspackages/3-targets/6-adapters/postgres/test/migrations/runner.idempotency.integration.test.tspackages/3-targets/6-adapters/postgres/test/migrations/runner.multi-space.integration.test.tspackages/3-targets/6-adapters/sqlite/package.jsonpackages/3-targets/6-adapters/sqlite/src/core/adapter.tspackages/3-targets/6-adapters/sqlite/src/core/control-adapter.tspackages/3-targets/6-adapters/sqlite/test/migrations/db-apply-per-space.cli.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/marker-schema-migration.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/planner.codec-field-event.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/render-typescript.roundtrip.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/runner.basic.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/runner.errors.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/runner.idempotency.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/runner.multi-space.test.tstest/e2e/framework/test/sqlite/utils.tstest/integration/test/cli-journeys/drift-marker.e2e.test.tstest/integration/test/cli-journeys/greenfield-setup.e2e.test.tstest/integration/test/cli-journeys/invariant-routing.e2e.test.tstest/integration/test/cli-journeys/migration-apply-edge-cases.e2e.test.tstest/integration/test/cli.db-init.e2e.errors.test.tstest/integration/test/cli.db-init.e2e.test.tstest/integration/test/cli.db-sign.e2e.test.tstest/integration/test/cli.migration-apply.e2e.test.tstest/integration/test/family.sign-database.test.ts
wmadden
force-pushed
the
tml-2397-codec-hooks-and-db-init-update
branch
from
c2bdd3d to
4448c05
Compare
wmadden
force-pushed
the
tml-2397-contract-spaces-first-class-schema-contributions-from
branch
2 times, most recently
from
474b69f to
7adb376
Compare
wmadden
force-pushed
the
tml-2397-contract-spaces-first-class-schema-contributions-from
branch
from
7adb376 to
3e85bcb
Compare
Base automatically changed from
tml-2397-contract-spaces-first-class-schema-contributions-from
to
main
May 9, 2026 17:54
wmadden
force-pushed
the
tml-2397-codec-hooks-and-db-init-update
branch
from
a7aa92d to
f395e88
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/1-framework/3-tooling/cli/src/control-api/client.ts (1)
65-71: ⚡ Quick winUse a type predicate to narrow the extension-pack shape instead of casting.
Replace the structural cast with a type predicate that explicitly declares the
contractSpaceproperty shape. This keeps the filtering logic type-safe and avoids silently masking future descriptor changes.Suggested refactor
+function hasContractSpace( + pack: { readonly id: string; readonly contractSpace?: unknown }, +): pack is { readonly id: string; readonly contractSpace: unknown } { + return pack.contractSpace !== undefined; +} + function collectExtensionContractSpaceInputs( - extensionPacks: ReadonlyArray<{ readonly id: string }> | undefined, + extensionPacks: + | ReadonlyArray<{ readonly id: string; readonly contractSpace?: unknown }> + | undefined, ): readonly PerSpaceExtensionInput[] { if (!extensionPacks) return []; - return extensionPacks - .filter((pack) => (pack as { readonly contractSpace?: unknown }).contractSpace !== undefined) - .map((pack) => ({ id: pack.id })); + return extensionPacks.filter(hasContractSpace).map((pack) => ({ id: pack.id })); }🤖 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/client.ts` around lines 65 - 71, The filter currently uses a structural cast to check for contractSpace in collectExtensionContractSpaceInputs; replace that cast with a proper type predicate (type guard) function such as hasContractSpace(pack): pack is { id: string; contractSpace: unknown } and use it in the .filter(...) call so TypeScript narrows the pack shape safely before .map(({ id }) => ({ id })); this keeps the check type-safe and avoids using (pack as ...) casts.packages/1-framework/3-tooling/cli/src/commands/db-update.ts (1)
90-94: 💤 Low valueRemove transient project artifact references from comments.
Same issue as in
db-init.ts: comments reference "sub-spec § 4", "AC-13", "AM11" which are transient project artifacts. Describe the behavior directly instead.Also applies to: 105-109
🤖 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/db-update.ts` around lines 90 - 94, Update the explanatory comment block in db-update.ts (same pattern as in db-init.ts) to remove transient project artifact references like "sub-spec § 4", "AC-13", and "AM11" and replace them with a direct description of the behavior being enforced (e.g., explain that the precheck validates per-space layout to catch declared-but-unmigrated schemas and orphaned pinned directories before any DB connection, and that it mirrors the wiring used by db init / db verify so a single-command invocation cannot bypass layout invariants). Apply the same change for the later comment span (around the block noted as lines 105-109).packages/1-framework/3-tooling/cli/src/commands/db-init.ts (1)
120-122: 💤 Low valueRemove transient project artifact references from comments.
The comments reference "sub-spec § 4", "AC-13", and "AM11" which are milestone-task IDs and acceptance criteria. Per coding guidelines, source-code comments should describe the constraint or behavior itself rather than reference transient project artifacts.
Suggested comment revision
- // Per-space layout precheck (sub-spec § 4): catches the - // `declaredButUnmigrated` / `orphanPinnedDir` cases at the CLI surface - // before any database connection. + // Per-space layout precheck: catches `declaredButUnmigrated` / `orphanPinnedDir` + // cases at the CLI surface before any database connection.- // Marker-aware verifier (sub-spec § 4): connect explicitly so we - // can read marker rows before any apply work runs. Locks AC-13 + - // AM11 at the `db init` integration surface — `orphanMarker`, - // marker-vs-pinned `hashMismatch`, and `invariantsMismatch` — that - // the layout-only precheck cannot detect on its own. Mirrors the - // wiring already in place in `db verify`. + // Marker-aware verifier: connect explicitly so we can read marker rows + // before any apply work runs. Detects `orphanMarker`, marker-vs-pinned + // `hashMismatch`, and `invariantsMismatch` that the layout-only precheck + // cannot detect. Mirrors the wiring in `db verify`.As per coding guidelines: "Source-code comments must not reference transient project artifacts, including... milestone-task IDs (e.g. T1.7, T2.5.3, R4), milestone-named acceptance criteria (e.g. AM12, AC-13)."
Also applies to: 134-139
🤖 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/db-init.ts` around lines 120 - 122, Replace the transient artifact references in the comment above the per-space layout precheck in db-init.ts (the comment that mentions "sub-spec § 4", "AC-13", "AM11") with a self-contained description of the behavior: explain that this precheck validates the on-disk project/layout state and catches the declaredButUnmigrated and orphanPinnedDir conditions before any database connection is established, and remove all milestone/task/acceptance IDs so the comment documents the constraint itself rather than project artifacts.
🤖 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/db-update.ts`:
- Around line 95-102: The precheck call to runContractSpaceVerifierPrecheck is
currently outside the try/finally and could leak the client if it throws; move
the await runContractSpaceVerifierPrecheck({...}) call inside the same try block
that later ensures client.close() in the finally, then keep the existing failure
handling (calling client.close() and returning notOk(precheckResult.failure)) or
convert that path to return after closing within the try so all exit paths
(success, precheck failure, or unexpected throw) go through the finally cleanup;
update references to runContractSpaceVerifierPrecheck, client.close, and notOk
accordingly.
In `@packages/1-framework/3-tooling/migration/src/space-layout.ts`:
- Around line 16-24: Remove the duplicate type declaration for ValidSpaceId:
locate the second exported type alias "export type ValidSpaceId = string & {
readonly __brand: 'ValidSpaceId' };" and delete it so only the original
declaration remains; ensure references to assertValidSpaceId and
spaceMigrationDirectory continue to use the single ValidSpaceId type and run the
build to confirm the duplicate-identifier error is resolved.
In `@packages/2-mongo-family/9-family/src/core/control-instance.ts`:
- Around line 277-282: The call in readAllMarkers is missing the required space
field for readMarker; update readAllMarkers to call this.readMarker({ driver:
options.driver, space: APP_SPACE_ID }) and return a Map keyed by APP_SPACE_ID
instead of 'app' so the Map becomes new Map([[APP_SPACE_ID, appMarker]]); ensure
you reference APP_SPACE_ID, readAllMarkers, and
readMarker/ControlDriverInstance/ContractMarkerRecord when making the change.
---
Nitpick comments:
In `@packages/1-framework/3-tooling/cli/src/commands/db-init.ts`:
- Around line 120-122: Replace the transient artifact references in the comment
above the per-space layout precheck in db-init.ts (the comment that mentions
"sub-spec § 4", "AC-13", "AM11") with a self-contained description of the
behavior: explain that this precheck validates the on-disk project/layout state
and catches the declaredButUnmigrated and orphanPinnedDir conditions before any
database connection is established, and remove all milestone/task/acceptance IDs
so the comment documents the constraint itself rather than project artifacts.
In `@packages/1-framework/3-tooling/cli/src/commands/db-update.ts`:
- Around line 90-94: Update the explanatory comment block in db-update.ts (same
pattern as in db-init.ts) to remove transient project artifact references like
"sub-spec § 4", "AC-13", and "AM11" and replace them with a direct description
of the behavior being enforced (e.g., explain that the precheck validates
per-space layout to catch declared-but-unmigrated schemas and orphaned pinned
directories before any DB connection, and that it mirrors the wiring used by db
init / db verify so a single-command invocation cannot bypass layout
invariants). Apply the same change for the later comment span (around the block
noted as lines 105-109).
In `@packages/1-framework/3-tooling/cli/src/control-api/client.ts`:
- Around line 65-71: The filter currently uses a structural cast to check for
contractSpace in collectExtensionContractSpaceInputs; replace that cast with a
proper type predicate (type guard) function such as hasContractSpace(pack): pack
is { id: string; contractSpace: unknown } and use it in the .filter(...) call so
TypeScript narrows the pack shape safely before .map(({ id }) => ({ id })); this
keeps the check type-safe and avoids using (pack as ...) casts.
🪄 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: a2b4f5ba-2940-489b-bcd7-3f9676eceb01
📒 Files selected for processing (49)
.cursor/rules/doc-maintenance.mdcexamples/prisma-next-demo/test/utils/control-client.tspackages/1-framework/1-core/framework-components/src/control/control-capabilities.tspackages/1-framework/1-core/framework-components/src/control/control-instances.tspackages/1-framework/1-core/framework-components/src/control/control-migration-types.tspackages/1-framework/1-core/framework-components/src/exports/control.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/db-verify.tspackages/1-framework/3-tooling/cli/src/commands/init/index.tspackages/1-framework/3-tooling/cli/src/commands/migration-plan.tspackages/1-framework/3-tooling/cli/src/control-api/client.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-apply-per-space.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-init.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-update.tspackages/1-framework/3-tooling/cli/src/control-api/types.tspackages/1-framework/3-tooling/cli/src/exports/control-api.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-extension-migrations-pass.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-migrate-pass.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-verifier-marker-check.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-verifier-precheck.tspackages/1-framework/3-tooling/cli/test/config-types.test.tspackages/1-framework/3-tooling/cli/test/control-api/client.test.tspackages/1-framework/3-tooling/cli/test/control-api/db-apply-per-space.test.tspackages/1-framework/3-tooling/cli/test/control-api/db-init.test.tspackages/1-framework/3-tooling/cli/test/control-api/db-update.test.tspackages/1-framework/3-tooling/cli/test/control-api/progress.test.tspackages/1-framework/3-tooling/cli/test/utils/contract-space-extension-migrations-pass.test.tspackages/1-framework/3-tooling/cli/test/utils/contract-space-migrate-pass.test.tspackages/1-framework/3-tooling/cli/test/utils/contract-space-verifier-marker-check.test.tspackages/1-framework/3-tooling/cli/test/utils/contract-space-verifier-precheck.test.tspackages/1-framework/3-tooling/migration/src/assert-descriptor-self-consistency.tspackages/1-framework/3-tooling/migration/src/compute-extension-space-apply-path.tspackages/1-framework/3-tooling/migration/src/detect-space-contract-drift.tspackages/1-framework/3-tooling/migration/src/emit-pinned-space-artefacts.tspackages/1-framework/3-tooling/migration/src/errors.tspackages/1-framework/3-tooling/migration/src/exports/errors.tspackages/1-framework/3-tooling/migration/src/exports/spaces.tspackages/1-framework/3-tooling/migration/src/gather-disk-contract-space-state.tspackages/1-framework/3-tooling/migration/src/plan-all-spaces.tspackages/1-framework/3-tooling/migration/src/read-pinned-contract-hash.tspackages/1-framework/3-tooling/migration/src/read-pinned-head-ref.tspackages/1-framework/3-tooling/migration/src/read-pinned-space-contract.tspackages/1-framework/3-tooling/migration/src/space-layout.tspackages/1-framework/3-tooling/migration/src/verify-contract-spaces.tspackages/1-framework/3-tooling/migration/test/assert-descriptor-self-consistency.test.tspackages/1-framework/3-tooling/migration/test/gather-disk-contract-space-state.test.tspackages/1-framework/3-tooling/migration/test/read-pinned-head-ref.test.tspackages/2-mongo-family/9-family/src/core/control-instance.ts
💤 Files with no reviewable changes (2)
- packages/1-framework/3-tooling/migration/src/read-pinned-contract-hash.ts
- packages/1-framework/3-tooling/migration/src/emit-pinned-space-artefacts.ts
✅ Files skipped from review due to trivial changes (3)
- .cursor/rules/doc-maintenance.mdc
- packages/1-framework/3-tooling/migration/test/gather-disk-contract-space-state.test.ts
- packages/1-framework/3-tooling/migration/src/exports/spaces.ts
🚧 Files skipped from review as they are similar to previous changes (24)
- packages/1-framework/3-tooling/cli/test/config-types.test.ts
- packages/1-framework/3-tooling/cli/src/control-api/types.ts
- packages/1-framework/1-core/framework-components/src/control/control-migration-types.ts
- packages/1-framework/3-tooling/cli/test/utils/contract-space-migrate-pass.test.ts
- packages/1-framework/3-tooling/cli/src/utils/contract-space-verifier-marker-check.ts
- packages/1-framework/1-core/framework-components/src/control/control-capabilities.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts
- packages/1-framework/3-tooling/migration/test/read-pinned-head-ref.test.ts
- packages/1-framework/3-tooling/cli/src/commands/db-verify.ts
- packages/1-framework/3-tooling/cli/src/utils/contract-space-extension-migrations-pass.ts
- packages/1-framework/3-tooling/migration/src/gather-disk-contract-space-state.ts
- packages/1-framework/3-tooling/migration/test/assert-descriptor-self-consistency.test.ts
- packages/1-framework/3-tooling/cli/src/utils/contract-space-verifier-precheck.ts
- packages/1-framework/3-tooling/migration/src/read-pinned-head-ref.ts
- packages/1-framework/3-tooling/cli/src/utils/contract-space-migrate-pass.ts
- packages/1-framework/3-tooling/cli/src/control-api/operations/db-apply-per-space.ts
- packages/1-framework/3-tooling/cli/test/utils/contract-space-verifier-precheck.test.ts
- packages/1-framework/1-core/framework-components/src/exports/control.ts
- packages/1-framework/3-tooling/migration/src/compute-extension-space-apply-path.ts
- packages/1-framework/3-tooling/migration/src/read-pinned-space-contract.ts
- packages/1-framework/3-tooling/cli/test/utils/contract-space-extension-migrations-pass.test.ts
- packages/1-framework/3-tooling/cli/src/exports/control-api.ts
- packages/1-framework/3-tooling/migration/src/assert-descriptor-self-consistency.ts
- packages/1-framework/3-tooling/migration/src/verify-contract-spaces.ts
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/integration/test/cli.migration-apply.e2e.test.ts (1)
57-65:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFilter
migrations/appentries to real migration directories before resolvingmigration.ts.Line 57 now points at a directory that can include non-migration entries (
refs, contract artefacts). Picking “latest” by mtime across all entries can resolve to a non-migration path and break/flap self-emit.💡 Suggested hardening
function getLatestMigrationDir(testDir: string): string | undefined { const migrationsDir = join(testDir, 'migrations', 'app'); - const dirs = readdirSync(migrationsDir).filter((d) => !d.startsWith('.')); + const dirs = readdirSync(migrationsDir).filter((d) => { + if (d.startsWith('.')) return false; + const full = join(migrationsDir, d); + return statSync(full).isDirectory() && d !== 'refs'; + }); if (dirs.length === 0) return undefined;Also applies to: 76-77
🤖 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 `@test/integration/test/cli.migration-apply.e2e.test.ts` around lines 57 - 65, The selection of the "newest" migration directory currently scans migrationsDir and uses mtime across all entries (dirs -> newest/newestMtime), which can pick non-migration items; update the logic to first filter dirs to only real migration directories (e.g., those containing a migration.ts file or matching your migration naming convention) before computing newest/newestMtime and looping; apply the same filter/hardening where the code later resolves migration.ts (the similar block around the referenced 76-77 behavior) so you never pick refs/artefacts as the migration target.test/integration/test/cli-journeys/invariant-routing.mongo.e2e.test.ts (1)
160-168:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
getLatestMigrationDirshould exclude non-migration entries inmigrations/app.Line 160 now targets a directory that may contain
refsand artefact files. Selecting by mtime without directory/type filtering can choose a non-migration entry and breakmigrationEmitpath resolution.💡 Suggested fix
function getLatestMigrationDir(ctx: JourneyCtx): string { const migrationsDir = join(ctx.testDir, 'migrations', 'app'); - const dirs = readdirSync(migrationsDir).filter((d) => !d.startsWith('.')); + const dirs = readdirSync(migrationsDir).filter((d) => { + if (d.startsWith('.')) return false; + const full = join(migrationsDir, d); + return statSync(full).isDirectory() && d !== 'refs'; + }); if (dirs.length === 0) throw new Error('No migration directory found');🤖 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 `@test/integration/test/cli-journeys/invariant-routing.mongo.e2e.test.ts` around lines 160 - 168, The selection logic in getLatestMigrationDir (using migrationsDir and dirs) currently picks the newest entry by mtime but may pick non-migration files like refs or artefacts; update the code that builds dirs to filter to only actual migration directories (e.g., check statSync(...).isDirectory() and/or match your migration name pattern) before computing newest/newestMtime so migrationEmit path resolution only considers valid migration folders.
🧹 Nitpick comments (4)
packages/1-framework/3-tooling/cli/test/utils/contract-space-migrate-pass.test.ts (1)
95-100: ⚡ Quick winUse a fail-fast assertion instead of a conditional guard in the drift path.
Line 95 currently gates assertions behind
if (...), which is defensive in a path already validated by prior expectations. Make it explicit and fail-fast.As per coding guidelines, "`{packages,test,examples}/**/*.ts`: Prefer assertions over defensive checks when data is guaranteed to be valid."Proposed patch
- const drift = out.drifts[0]; - if (drift && drift.kind === 'drift') { - const warning = formatContractSpaceDriftWarning(drift); - expect(warning).toContain('cipherstash'); - expect(warning).toContain(HASH_A); - expect(warning).toContain(HASH_B); - } + const drift = out.drifts[0]!; + if (drift.kind !== 'drift') { + throw new Error(`Expected drift result, got: ${drift.kind}`); + } + const warning = formatContractSpaceDriftWarning(drift); + expect(warning).toContain('cipherstash'); + expect(warning).toContain(HASH_A); + expect(warning).toContain(HASH_B);🤖 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/utils/contract-space-migrate-pass.test.ts` around lines 95 - 100, Replace the defensive guard around the drift assertions with fail-fast expectations: assert that the variable drift exists and that drift.kind === 'drift' (e.g., expect(drift).toBeDefined(); expect(drift.kind).toBe('drift')) before calling formatContractSpaceDriftWarning(drift), then keep the existing expectations that the warning contains 'cipherstash', HASH_A and HASH_B; this removes the conditional branch and makes the test fail fast when drift is not present or wrong.packages/1-framework/3-tooling/migration/test/aggregate/loader.test.ts (1)
148-150: 💤 Low valueRemove milestone reference from comment.
The comment references "M2 R6 R3 verifier behaviour" which per coding guidelines should not appear in source code. Describe the behavior itself instead.
♻️ Proposed fix
// Both offences are surfaced in one error rather than fixed one - // commit at a time (preserves the M2 R6 R3 verifier behaviour). + // commit at a time (all layout violations are reported together). expect([...failure.violations].sort((a, b) => a.spaceId.localeCompare(b.spaceId))).toEqual([🤖 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/migration/test/aggregate/loader.test.ts` around lines 148 - 150, Edit the inline comment above the assertion that inspects failure.violations (the expect([...failure.violations]... ) line) to remove the "M2 R6 R3 verifier behaviour" milestone reference and instead describe the behavior in plain terms — e.g., state that both offences are reported together in a single aggregated error rather than being fixed one commit at a time, preserving the current verifier behavior; keep the rest of the test untouched.packages/1-framework/3-tooling/migration/test/deletable-node-modules.test.ts (1)
302-306: 💤 Low valueConsider removing the unused
APP_HEAD_HASHvariable.The variable is declared but only referenced via
typeofto suppress an unused-variable warning. If it's not needed for the test assertions, it can be removed entirely.♻️ Cleanup suggestion
describe('aggregate pipeline (loader → planner → verifier) against deleted node_modules', () => { const HEAD_HASH = 'sha256:abc123'; - const APP_HEAD_HASH = 'sha256:appHead'; let projectRoot: string; let migrationsDir: string;And remove lines 302-306.
🤖 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/migration/test/deletable-node-modules.test.ts` around lines 302 - 306, Remove the unused APP_HEAD_HASH artifact and its only reference: delete the APP_HEAD_HASH declaration and the expect(typeof APP_HEAD_HASH).toBe('string') assertion in the deletable-node-modules.test.ts test; ensure no other code references APP_HEAD_HASH, and run the test suite to confirm no lint or unused-variable warnings remain (if the variable was only used to suppress linting, remove any now-unnecessary eslint-disable comments as well).packages/1-framework/3-tooling/cli/src/utils/combine-schema-results.ts (1)
30-46: 💤 Low valueConsider using
pushinstead of array spreading in the loop.The pattern
issues = [...issues, ...result.schema.issues]creates a new array on each iteration, leading to O(n²) complexity in the total number of issues. While this is unlikely to be a problem in practice, collecting into a mutable array and then optionally freezing would be more efficient.♻️ Optional optimization
- let issues: VerifyDatabaseSchemaResult['schema']['issues'] = []; + const issues: VerifyDatabaseSchemaResult['schema']['issues'][number][] = []; const counts = { pass: 0, warn: 0, fail: 0, totalNodes: 0 }; const childRoots: Array<VerifyDatabaseSchemaResult['schema']['root']> = []; for (const [, result] of perSpace) { if (!result.ok) { okAll = false; if (firstFailure === undefined) firstFailure = result; } - issues = [...issues, ...result.schema.issues]; + issues.push(...result.schema.issues); counts.pass += result.schema.counts.pass;🤖 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/combine-schema-results.ts` around lines 30 - 46, The loop in combine-schema-results.ts builds the issues array by recreating it each iteration with issues = [...issues, ...result.schema.issues], which is O(n²); instead, mutate the existing array by replacing that line with issues.push(...result.schema.issues) (or a simple for-of push for compatibility) inside the for (const [, result] of perSpace) loop to collect issues in linear time, keeping the rest of the aggregation (okAll, firstFailure, counts, childRoots) unchanged and optionally freeze the array afterwards if immutability is desired.
🤖 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/control-api/client.ts`:
- Around line 413-416: In the dbVerify method change the action name passed to
connectWithProgress from 'schemaVerify' to 'dbVerify' so progress reporting
matches the invoked operation; locate the dbVerify function and update the
connectWithProgress call (currently connectWithProgress(options.connection,
'schemaVerify', onProgress)) to use 'dbVerify' instead.
In
`@packages/1-framework/3-tooling/cli/test/utils/contract-space-migrate-pass.test.ts`:
- Around line 21-23: The teardown in the afterEach hook calls rm(migrationsDir,
...) unconditionally which can throw if migrationsDir was never assigned during
setup; update the afterEach (the afterEach function) to guard the cleanup by
checking that migrationsDir is defined/truthy before calling rm (or otherwise
wrap the rm call in a safe conditional/try-catch) so setup failures are not
masked by teardown errors.
In `@packages/1-framework/3-tooling/migration/test/space-layout.test.ts`:
- Line 67: Update the inline comment in space-layout.test.ts that currently
reads "// M2.5b: the app no longer has a special-case layout. AC1." to remove
the milestone/task IDs and acceptance criterion tags; keep only the behavioral
statement such as "the app no longer has a special-case layout." so the comment
describes the constraint/behavior without referencing transient project
artifacts (look for the comment string containing "M2.5b" / "AC1" near the test
for space layout).
In `@packages/2-sql/9-family/src/core/migrations/types.ts`:
- Around line 378-384: The optional space property in the runner options should
not silently default to 'app'; change the contract so either (A) remove the
optional space and derive the space value from the plan's required plan.spaceId
(use plan.spaceId wherever runner code currently reads options.space), or (B)
make options.space required and add a runtime assertion that options.space ===
plan.spaceId (fail fast on mismatch). Locate the declaration of readonly space?:
string in types.ts and update the type and downstream callers (runner code that
reads options.space) to use plan.spaceId (or validate equality) so markers are
always written to the correct space.
In
`@packages/2-sql/9-family/test/control-instance.descriptor-self-consistency.test.ts`:
- Around line 67-75: The returned control-plane descriptor object (the literal
with fields kind, id, familyId, targetId, version, contractSpace, create) is
missing the required manifest property; update this fixture and the similar ones
referenced (lines ~82-113) to include a manifest field of type
ExtensionPackManifest by creating or importing a minimal valid manifest and
adding manifest: <that manifest> to the descriptor object so the fixture
satisfies the control descriptor contract (refer to the descriptor literal and
the create() factory to locate the spots to change).
In `@test/integration/test/cli.db-verify.aggregate-schema.test.ts`:
- Around line 29-47: Update the block comment that begins with "F23 lock — `db
verify` against a multi-member aggregate..." in the
cli.db-verify.aggregate-schema.test.ts test header to remove transient
milestone/spec shorthand (e.g., "F23", "M2 R6 R1", "M2.5", "sub-spec") and
instead state the behavior and constraints directly: describe that the test
verifies db verify against an aggregate of app+extension where both claim live
tables, that the verifier pre-projects the live schema per member so each member
only sees owned elements, and list the setup (app claims `user`, extension
claims `test_box`, both tables present and markers match) and expected result
(exit 0, ok: true, zero schema issues); apply the same rewrite to the similar
inline comment around the second comment block referenced (the content around
lines 59-65).
In `@test/integration/test/cli.loader.drift.test.ts`:
- Around line 22-24: The comment currently embeds planning identifiers
("sub-spec", "AC11", "M2.5") and should be rewritten to be behavior-focused;
update the multi-line comment that begins "This is the integration-level lock
for..." to remove all milestone/task references and instead state the purpose
(e.g., "Integration-level lock for the Loader step; the unit-level equivalent
exists elsewhere") while preserving the note about the unit-level lock location
concept so readers know there is a corresponding test. Ensure only the transient
artifact references are removed and the intent/purpose of the comment remains
clear.
In `@test/integration/test/utils/journey-test-helpers.ts`:
- Around line 533-536: Remove the transient milestone token "M2.5b" from the
comment that currently reads "M2.5b layout: `migrations/app/`" so the comment
only describes the path (e.g., "layout: `migrations/app/`" or simply
"`migrations/app/`"); edit the comment block containing the sentence starting
with "Path of the app subspace's migrations directory under the journey test
root" to eliminate the milestone reference and leave a neutral description of
the directory layout.
---
Outside diff comments:
In `@test/integration/test/cli-journeys/invariant-routing.mongo.e2e.test.ts`:
- Around line 160-168: The selection logic in getLatestMigrationDir (using
migrationsDir and dirs) currently picks the newest entry by mtime but may pick
non-migration files like refs or artefacts; update the code that builds dirs to
filter to only actual migration directories (e.g., check
statSync(...).isDirectory() and/or match your migration name pattern) before
computing newest/newestMtime so migrationEmit path resolution only considers
valid migration folders.
In `@test/integration/test/cli.migration-apply.e2e.test.ts`:
- Around line 57-65: The selection of the "newest" migration directory currently
scans migrationsDir and uses mtime across all entries (dirs ->
newest/newestMtime), which can pick non-migration items; update the logic to
first filter dirs to only real migration directories (e.g., those containing a
migration.ts file or matching your migration naming convention) before computing
newest/newestMtime and looping; apply the same filter/hardening where the code
later resolves migration.ts (the similar block around the referenced 76-77
behavior) so you never pick refs/artefacts as the migration target.
---
Nitpick comments:
In `@packages/1-framework/3-tooling/cli/src/utils/combine-schema-results.ts`:
- Around line 30-46: The loop in combine-schema-results.ts builds the issues
array by recreating it each iteration with issues = [...issues,
...result.schema.issues], which is O(n²); instead, mutate the existing array by
replacing that line with issues.push(...result.schema.issues) (or a simple
for-of push for compatibility) inside the for (const [, result] of perSpace)
loop to collect issues in linear time, keeping the rest of the aggregation
(okAll, firstFailure, counts, childRoots) unchanged and optionally freeze the
array afterwards if immutability is desired.
In
`@packages/1-framework/3-tooling/cli/test/utils/contract-space-migrate-pass.test.ts`:
- Around line 95-100: Replace the defensive guard around the drift assertions
with fail-fast expectations: assert that the variable drift exists and that
drift.kind === 'drift' (e.g., expect(drift).toBeDefined();
expect(drift.kind).toBe('drift')) before calling
formatContractSpaceDriftWarning(drift), then keep the existing expectations that
the warning contains 'cipherstash', HASH_A and HASH_B; this removes the
conditional branch and makes the test fail fast when drift is not present or
wrong.
In `@packages/1-framework/3-tooling/migration/test/aggregate/loader.test.ts`:
- Around line 148-150: Edit the inline comment above the assertion that inspects
failure.violations (the expect([...failure.violations]... ) line) to remove the
"M2 R6 R3 verifier behaviour" milestone reference and instead describe the
behavior in plain terms — e.g., state that both offences are reported together
in a single aggregated error rather than being fixed one commit at a time,
preserving the current verifier behavior; keep the rest of the test untouched.
In
`@packages/1-framework/3-tooling/migration/test/deletable-node-modules.test.ts`:
- Around line 302-306: Remove the unused APP_HEAD_HASH artifact and its only
reference: delete the APP_HEAD_HASH declaration and the expect(typeof
APP_HEAD_HASH).toBe('string') assertion in the deletable-node-modules.test.ts
test; ensure no other code references APP_HEAD_HASH, and run the test suite to
confirm no lint or unused-variable warnings remain (if the variable was only
used to suppress linting, remove any now-unnecessary eslint-disable comments as
well).
🪄 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: ebd28738-4bf3-4f5c-8289-fba9b2fd4076
⛔ Files ignored due to path filters (6)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/extension-contract-spaces/e2e-verification.mdis excluded by!projects/**projects/extension-contract-spaces/plan.mdis excluded by!projects/**projects/extension-contract-spaces/specs/contract-space-aggregate-spec.mdis excluded by!projects/**projects/extension-contract-spaces/specs/contract-space-on-disk-shape.spec.mdis excluded by!projects/**projects/extension-contract-spaces/specs/migration-cli-aggregate.spec.mdis excluded by!projects/**
📒 Files selected for processing (157)
examples/mongo-demo/migrations/app/20260409T1030_migration/end-contract.d.tsexamples/mongo-demo/migrations/app/20260409T1030_migration/end-contract.jsonexamples/mongo-demo/migrations/app/20260409T1030_migration/migration.jsonexamples/mongo-demo/migrations/app/20260409T1030_migration/migration.tsexamples/mongo-demo/migrations/app/20260409T1030_migration/ops.jsonexamples/mongo-demo/migrations/app/20260415_add-posts-author-index/end-contract.d.tsexamples/mongo-demo/migrations/app/20260415_add-posts-author-index/end-contract.jsonexamples/mongo-demo/migrations/app/20260415_add-posts-author-index/migration.jsonexamples/mongo-demo/migrations/app/20260415_add-posts-author-index/migration.tsexamples/mongo-demo/migrations/app/20260415_add-posts-author-index/ops.jsonexamples/mongo-demo/test/manual-migration.test.tsexamples/prisma-next-demo/migrations/app/20260422T0720_initial/end-contract.d.tsexamples/prisma-next-demo/migrations/app/20260422T0720_initial/end-contract.jsonexamples/prisma-next-demo/migrations/app/20260422T0720_initial/migration.jsonexamples/prisma-next-demo/migrations/app/20260422T0720_initial/migration.tsexamples/prisma-next-demo/migrations/app/20260422T0720_initial/ops.jsonexamples/prisma-next-demo/migrations/app/20260422T0742_migration/end-contract.d.tsexamples/prisma-next-demo/migrations/app/20260422T0742_migration/end-contract.jsonexamples/prisma-next-demo/migrations/app/20260422T0742_migration/migration.jsonexamples/prisma-next-demo/migrations/app/20260422T0742_migration/migration.tsexamples/prisma-next-demo/migrations/app/20260422T0742_migration/ops.jsonexamples/prisma-next-demo/migrations/app/20260422T0742_migration/start-contract.d.tsexamples/prisma-next-demo/migrations/app/20260422T0742_migration/start-contract.jsonexamples/prisma-next-demo/migrations/app/20260422T0748_migration/end-contract.d.tsexamples/prisma-next-demo/migrations/app/20260422T0748_migration/end-contract.jsonexamples/prisma-next-demo/migrations/app/20260422T0748_migration/migration.jsonexamples/prisma-next-demo/migrations/app/20260422T0748_migration/migration.tsexamples/prisma-next-demo/migrations/app/20260422T0748_migration/ops.jsonexamples/prisma-next-demo/migrations/app/20260422T0748_migration/start-contract.d.tsexamples/prisma-next-demo/migrations/app/20260422T0748_migration/start-contract.jsonexamples/retail-store/migrations/app/20260413T0314_migration/end-contract.d.tsexamples/retail-store/migrations/app/20260413T0314_migration/end-contract.jsonexamples/retail-store/migrations/app/20260413T0314_migration/migration.jsonexamples/retail-store/migrations/app/20260413T0314_migration/migration.tsexamples/retail-store/migrations/app/20260413T0314_migration/ops.jsonexamples/retail-store/migrations/app/20260415_add-product-validation/end-contract.d.tsexamples/retail-store/migrations/app/20260415_add-product-validation/end-contract.jsonexamples/retail-store/migrations/app/20260415_add-product-validation/migration.jsonexamples/retail-store/migrations/app/20260415_add-product-validation/migration.tsexamples/retail-store/migrations/app/20260415_add-product-validation/ops.jsonexamples/retail-store/migrations/app/20260416_backfill-product-status/end-contract.d.tsexamples/retail-store/migrations/app/20260416_backfill-product-status/end-contract.jsonexamples/retail-store/migrations/app/20260416_backfill-product-status/migration.jsonexamples/retail-store/migrations/app/20260416_backfill-product-status/migration.tsexamples/retail-store/migrations/app/20260416_backfill-product-status/ops.jsonexamples/retail-store/test/manual-migration.test.tspackages/1-framework/1-core/framework-components/src/control/control-instances.tspackages/1-framework/1-core/framework-components/src/control/control-spaces.tspackages/1-framework/1-core/framework-components/test/codec-call-context.types.test-d.tspackages/1-framework/1-core/framework-components/test/codec.test.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/db-verify.tspackages/1-framework/3-tooling/cli/src/commands/init/init.tspackages/1-framework/3-tooling/cli/src/commands/migration-apply.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-apply-aggregate.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-init.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-update.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-verify.tspackages/1-framework/3-tooling/cli/src/control-api/types.tspackages/1-framework/3-tooling/cli/src/exports/control-api.tspackages/1-framework/3-tooling/cli/src/utils/combine-schema-results.tspackages/1-framework/3-tooling/cli/src/utils/command-helpers.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-aggregate-loader.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-extension-migrations-pass.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-migrate-pass.tspackages/1-framework/3-tooling/cli/test/commands/migration-invariants.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-tamper.test.tspackages/1-framework/3-tooling/cli/test/config-types.test.tspackages/1-framework/3-tooling/cli/test/control-api/client.test.tspackages/1-framework/3-tooling/cli/test/control-api/db-init.test.tspackages/1-framework/3-tooling/cli/test/control-api/db-update.test.tspackages/1-framework/3-tooling/cli/test/control-api/progress.test.tspackages/1-framework/3-tooling/cli/test/utils/combine-schema-results.test.tspackages/1-framework/3-tooling/cli/test/utils/contract-space-migrate-pass.test.tspackages/1-framework/3-tooling/migration/package.jsonpackages/1-framework/3-tooling/migration/src/aggregate/loader.tspackages/1-framework/3-tooling/migration/src/aggregate/marker-types.tspackages/1-framework/3-tooling/migration/src/aggregate/planner-types.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/assert-descriptor-self-consistency.tspackages/1-framework/3-tooling/migration/src/compute-extension-space-apply-path.tspackages/1-framework/3-tooling/migration/src/concatenate-space-apply-inputs.tspackages/1-framework/3-tooling/migration/src/detect-space-contract-drift.tspackages/1-framework/3-tooling/migration/src/emit-contract-space-artefacts.tspackages/1-framework/3-tooling/migration/src/errors.tspackages/1-framework/3-tooling/migration/src/exports/aggregate.tspackages/1-framework/3-tooling/migration/src/exports/spaces.tspackages/1-framework/3-tooling/migration/src/gather-disk-contract-space-state.tspackages/1-framework/3-tooling/migration/src/plan-all-spaces.tspackages/1-framework/3-tooling/migration/src/read-contract-space-contract.tspackages/1-framework/3-tooling/migration/src/read-contract-space-head-ref.tspackages/1-framework/3-tooling/migration/src/read-pinned-contract-hash.tspackages/1-framework/3-tooling/migration/src/space-layout.tspackages/1-framework/3-tooling/migration/src/verify-contract-spaces.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/detect-space-contract-drift.test.tspackages/1-framework/3-tooling/migration/test/emit-contract-space-artefacts.test.tspackages/1-framework/3-tooling/migration/test/gather-disk-contract-space-state.test.tspackages/1-framework/3-tooling/migration/test/read-contract-space-head-ref.test.tspackages/1-framework/3-tooling/migration/test/read-pinned-contract-hash.test.tspackages/1-framework/3-tooling/migration/test/space-layout.test.tspackages/1-framework/3-tooling/migration/test/verify-contract-spaces.test.tspackages/1-framework/3-tooling/migration/tsdown.config.tspackages/2-mongo-family/9-family/src/core/control-instance.tspackages/2-sql/9-family/src/core/control-adapter.tspackages/2-sql/9-family/src/core/control-instance.tspackages/2-sql/9-family/src/core/migrations/types.tspackages/2-sql/9-family/test/control-instance.descriptor-self-consistency.test.tspackages/3-targets/3-targets/postgres/src/core/migrations/runner.tspackages/3-targets/3-targets/sqlite/src/core/migrations/runner.tspackages/3-targets/6-adapters/postgres/package.jsonpackages/3-targets/6-adapters/postgres/test/migrations/db-init-update.cli.integration.test.tspackages/3-targets/6-adapters/postgres/test/migrations/planner.codec-field-event.test.tspackages/3-targets/6-adapters/postgres/test/migrations/runner.multi-space.integration.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/db-init-update.cli.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/planner.codec-field-event.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/render-typescript.roundtrip.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/runner.multi-space.test.tstest/integration/package.jsontest/integration/test/cli-journeys/data-transform-enum-rebuild.e2e.test.tstest/integration/test/cli-journeys/data-transform-not-null-backfill.e2e.test.tstest/integration/test/cli-journeys/data-transform-nullable-tightening.e2e.test.tstest/integration/test/cli-journeys/data-transform-type-change.e2e.test.tstest/integration/test/cli-journeys/drift-deleted-root.e2e.test.tstest/integration/test/cli-journeys/drift-migration-dag.e2e.test.tstest/integration/test/cli-journeys/invariant-routing.e2e.test.tstest/integration/test/cli-journeys/invariant-routing.mongo.e2e.test.tstest/integration/test/cli-journeys/migration-plan-details.e2e.test.tstest/integration/test/cli-journeys/migration-round-trip.e2e.test.tstest/integration/test/cli-journeys/mongo-migration.e2e.test.tstest/integration/test/cli-journeys/schema-evolution-migrations.e2e.test.tstest/integration/test/cli.db-verify.aggregate-schema.test.tstest/integration/test/cli.loader.drift.test.tstest/integration/test/cli.migration-apply.e2e.test.tstest/integration/test/contract-space-fixture/constants.tstest/integration/test/contract-space-fixture/contract.tstest/integration/test/contract-space-fixture/descriptor.test.tstest/integration/test/contract-space-fixture/migrations.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-init-with-contract-space/prisma-next.config.with-db.tstest/integration/test/utils/journey-test-helpers.ts
💤 Files with no reviewable changes (2)
- packages/1-framework/3-tooling/migration/src/read-pinned-contract-hash.ts
- packages/1-framework/3-tooling/migration/test/read-pinned-contract-hash.test.ts
✅ Files skipped from review due to trivial changes (20)
- packages/3-targets/6-adapters/postgres/package.json
- test/integration/package.json
- packages/1-framework/3-tooling/migration/src/aggregate/marker-types.ts
- test/integration/test/contract-space-fixture/descriptor.test.ts
- test/integration/test/cli-journeys/data-transform-not-null-backfill.e2e.test.ts
- packages/1-framework/1-core/framework-components/src/control/control-spaces.ts
- packages/1-framework/3-tooling/cli/src/commands/init/init.ts
- packages/1-framework/3-tooling/migration/tsdown.config.ts
- packages/1-framework/1-core/framework-components/test/codec.test.ts
- packages/1-framework/3-tooling/migration/src/aggregate/project-schema-to-space.ts
- packages/1-framework/3-tooling/migration/src/aggregate/types.ts
- packages/1-framework/3-tooling/migration/src/concatenate-space-apply-inputs.ts
- packages/1-framework/1-core/framework-components/test/codec-call-context.types.test-d.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-init-with-contract-space/prisma-next.config.with-db.ts
- packages/1-framework/3-tooling/migration/src/exports/aggregate.ts
- packages/1-framework/3-tooling/cli/test/config-types.test.ts
- test/integration/test/contract-space-fixture/migrations.ts
- packages/2-sql/9-family/src/core/control-adapter.ts
- packages/1-framework/3-tooling/migration/src/plan-all-spaces.ts
- packages/1-framework/3-tooling/cli/test/control-api/client.test.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- packages/1-framework/3-tooling/cli/src/utils/contract-space-migrate-pass.ts
- packages/2-sql/9-family/src/core/control-instance.ts
- packages/1-framework/3-tooling/cli/src/utils/contract-space-extension-migrations-pass.ts
- packages/3-targets/6-adapters/postgres/test/migrations/runner.multi-space.integration.test.ts
- packages/3-targets/6-adapters/sqlite/test/migrations/runner.multi-space.test.ts
- packages/1-framework/3-tooling/cli/test/control-api/db-init.test.ts
- packages/1-framework/3-tooling/cli/test/control-api/progress.test.ts
- packages/3-targets/6-adapters/postgres/test/migrations/planner.codec-field-event.test.ts
- packages/1-framework/3-tooling/migration/src/assert-descriptor-self-consistency.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts
- packages/2-mongo-family/9-family/src/core/control-instance.ts
- packages/3-targets/6-adapters/sqlite/test/migrations/planner.codec-field-event.test.ts
- packages/1-framework/3-tooling/cli/test/control-api/db-update.test.ts
wmadden
changed the title
wmadden
added a commit
that referenced
this pull request
May 10, 2026
…ry branch Lifts combine-schema-results.ts from 80% to 100% branch coverage by adding tests for the empty-perSpace wiring-bug throw, the app-id-absent fallback to the first iterator value, the default 'PN-RUN-3010' code when a failing app result has no code, and the second-failure preserved (firstFailure latching) path. Also tightens the summary computation by replacing the unreachable '?? appResult.summary' fallback (the mirror shape app-fails/all-pass is impossible because appResult always participates in okAll) with a typed assertion plus the rationale in a comment. Closes the M2 Coverage CI failure on PR #438.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/1-framework/3-tooling/cli/test/utils/combine-schema-results.test.ts (1)
52-54: ⚡ Quick winPrefer grouped object assertions for combined-result checks
For each case where you assert multiple related fields on
combined, use onetoMatchObject(...)to keep intent compact and aligned with the test style rules.Example refactor pattern
- expect(combined.ok).toBe(false); - expect(combined.summary).toBe('Database schema does not satisfy contract (1 failure)'); - expect(combined.schema.counts.fail).toBe(1); - expect(combined.code).toBe('PN-RUN-3010'); + expect(combined).toMatchObject({ + ok: false, + summary: 'Database schema does not satisfy contract (1 failure)', + schema: { counts: { fail: 1 } }, + code: 'PN-RUN-3010', + });As per coding guidelines: "
**/*.test.ts: Prefer object matchers (toMatchObject) over multiple individual expect().toBe() calls when checking 2 or more related values in tests."Also applies to: 70-72, 93-97, 134-137, 154-157, 174-176
🤖 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/utils/combine-schema-results.test.ts` around lines 52 - 54, Replace the multiple related assertions on the test result object `combined` with a single grouped object matcher: instead of separate expect(combined.ok).toBe(...) and expect(combined.summary).toBe(...), use expect(combined).toMatchObject({ ok: true, summary: 'Database schema satisfies contract' }) (and analogous replacements for the other cases listed). Update the other occurrences that assert 2+ related fields (the blocks around lines referenced: 70-72, 93-97, 134-137, 154-157, 174-176) to use toMatchObject with the appropriate expected keys/values so the assertions are compact and consistent.
🤖 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/load-ts-contract.ts`:
- Around line 179-182: The thrown Error for disallowed imports hardcodes “Only
`@prisma-next/`* packages are allowed…”, which is incorrect when a custom
allowlist is passed; update the Error construction in the block that checks
disallowedFromEntry (referencing disallowedFromEntry and allowlist) to remove
the hardcoded package text and instead describe allowed packages dynamically
(e.g., include allowlist.join(', ') or a human-friendly representation of
allowlist) so the message accurately reflects the provided allowlist and lists
the disallowed imports.
---
Nitpick comments:
In
`@packages/1-framework/3-tooling/cli/test/utils/combine-schema-results.test.ts`:
- Around line 52-54: Replace the multiple related assertions on the test result
object `combined` with a single grouped object matcher: instead of separate
expect(combined.ok).toBe(...) and expect(combined.summary).toBe(...), use
expect(combined).toMatchObject({ ok: true, summary: 'Database schema satisfies
contract' }) (and analogous replacements for the other cases listed). Update the
other occurrences that assert 2+ related fields (the blocks around lines
referenced: 70-72, 93-97, 134-137, 154-157, 174-176) to use toMatchObject with
the appropriate expected keys/values so the assertions are compact and
consistent.
🪄 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: ae7d41e0-4804-4922-8623-47a29ed929e5
📒 Files selected for processing (4)
packages/1-framework/3-tooling/cli/src/load-ts-contract.tspackages/1-framework/3-tooling/cli/src/utils/combine-schema-results.tspackages/1-framework/3-tooling/cli/test/fixtures/disallowed-import.tspackages/1-framework/3-tooling/cli/test/utils/combine-schema-results.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/1-framework/3-tooling/cli/src/utils/combine-schema-results.ts
wmadden
commented
May 10, 2026
assertValidSpaceId previously asserted the input was string, which is a no-op since it already is. Introduce a branded ValidSpaceId type, narrow isValidSpaceId to a type predicate, and update the assertion signature so the compiler can track validated space ids through downstream filesystem helpers. No runtime behavior change.
Adds the synchronous `onFieldEvent` API to `CodecControlHooks` plus a target-agnostic `planFieldEventOperations` helper that diffs two contracts and dispatches to each field codec's hook on `added` / `dropped` / `altered` events. Codec-id-only changes do not fire (v1 non-goal). Hook ordering follows sub-spec § 5: events are grouped (added → dropped → altered), within a group sorted by `(tableName, fieldName)`. Hook-returned ops are appended in their returned order; the result is byte-stable across re-emits. Refs: TML-2397; projects/extension-contract-spaces/specs/framework-mechanism.spec.md § 5
…(T2.2) After structural DDL planning succeeds, the postgres migration planner now calls `planFieldEventOperations` and inlines each codec-emitted op via `RawSqlCall`. Sub-spec § 5 fixes the ordering as `structural → added → dropped → altered`, with within-group sorting by `(tableName, fieldName)` for byte-stable re-emits. Also relaxes `planFieldEventOperations` to return `SqlMigrationPlanOperation<unknown>[]` so target planners cast back to their own target-details specialisation at the integration boundary (matching the trust boundary documented in the helper). Codec authors implementing the hook for a specific lane already produce ops that match that lane's target-details shape. Refs: TML-2397; AM8
Adds a SQLite-side `RawSqlCall` (mirroring postgres's) plus a `rawSql` factory export so codec-emitted `SqlMigrationPlanOperation` instances can be carried through the planner alongside structured call IR. The sqlite migration planner now calls `planFieldEventOperations` after structural DDL planning succeeds and inlines each emitted op via `RawSqlCall`. Same ordering contract as postgres: `structural → added → dropped → altered` with within-group sorting by `(tableName, fieldName)`, giving byte-stable re-emits across lanes. Refs: TML-2397; AM8
… types + cast-at-site Brings sub-spec § 5 + plan into alignment with what M2 R1 actually shipped (commits f7b083c + 5967853 + 7e53525) so M2 R2 wiring reads the source of truth. Two amendments accumulated from M2 R1 review (orchestrator decision 9): - § 5 FieldEventContext uses concrete StorageTable / StorageColumn from @prisma-next/sql-contract/types (not placeholder TableIR / FieldIR), matching the convention already used by other CodecControlHooks methods. - § 5 documents the planFieldEventOperations helper signature: non-generic return type at the helper boundary because extractCodecControlHooks erases target-details to unknown (pre-existing); each target planner casts back at the integration site with scoped per-line .map(...) cast + explanatory comment per AGENTS.md typesafety rules. No public-API surface change for codec authors. Plan T2.1 + T2.2 marked [x] with M2 R1 commit references and AM/AC notes. No code change; closes the M2 R1 sub-spec drift surfaced by the review loop.
Mirrors the postgres render-typescript round-trip test for the sqlite adapter to lock byte-stable preservation of `RawSqlCall` ops through render → execute. Exports `RawSqlCall` from the sqlite target so the test can construct fixtures using the public surface, matching the postgres exports. Closes M2 R1 review finding F4 (write carve-out for implementer follow-ups).
…at family-create Adds `assertDescriptorSelfConsistency` (target-agnostic helper in `@prisma-next/migration-tools/spaces`) and wires it into `createSqlFamilyInstance` so every extension that publishes a `contractSpace` is checked at family-instantiation time: `headRef.hash` must equal the canonical hash recomputed from `contractJson.storage` (after stripping `storage.storageHash` so the hash function does not consume its own output). A mismatch surfaces `MIGRATION.DESCRIPTOR_HEAD_HASH_MISMATCH` naming the offending extension and pointing the user at re-running the extension authoring pipeline. Failing fast at descriptor-load time turns "extension author shipped a stale descriptor" into an explicit, actionable error rather than a confusing mismatch surfacing several layers downstream (drift detection, pinned artefact emission, runner marker writes). Closes the M1 R5 attention point carried into M2 (sub-spec § 3 — Drift detection).
…/ db verify Adds three new helpers to `@prisma-next/migration-tools/spaces`: - `readPinnedHeadRef(projectMigrationsDir, spaceId)` reads the full `(hash, invariants)` pair from `migrations/<space>/refs/head.json`. - `gatherDiskContractSpaceState(...)` collects pinned dirs + head refs off the user repo for every loaded space, returning the input shape `verifyContractSpaces` expects. - `runContractSpaceVerifierPrecheck` (cli util) composes both with `verifyContractSpaces`, surfaces every offending kind in a single structured error envelope (so users see the full picture rather than fixing them across re-runs), and is invoked at the top of `db init` / `db verify` before any database work happens. The marker-dependent verifier kinds (`orphanMarker`, `hashMismatch`, `invariantsMismatch`) are deferred until the runner-side single-tx slice threads marker rows through; the existing helper accepts an empty marker map and silently skips those kinds, so this slice locks the structural-layout part of AC-13 + AC-16 (declaredButUnmigrated, orphanPinnedDir) without committing to a marker-read API change. Closes the structural half of AC-13 + AC-16 (verifier wiring). Sub-spec § 4 — Verifier (T1.5).
At each `prisma-next migrate` invocation, runs a per-space pass over every loaded extension that exposes a `contractSpace`: - Reads the pinned head hash from `migrations/<space>/refs/head.json`. - Compares against the descriptor `headRef.hash` via `detectSpaceContractDrift`. Surfaces a non-fatal warning naming the extension and the diff direction when the hashes diverge. - Always re-emits `contract.json`, `contract.d.ts`, and `refs/head.json` for the space (the framework owns these files; the helper is idempotent). Runs before the app-space no-op check so an extension bump alone (with no structural app-space change) still re-pins extension artefacts on disk. Locks AM7 (drift warning surfaces extension name + diff direction). The `.d.ts` rendering for extension contracts is a placeholder `@ts-nocheck` stub in this slice — full typed rendering for extension contracts is tracked under sub-spec Open Question 3 and will land in a follow-on round once the SQL-family renderer is threaded through the per-space pipeline. Extension migration package materialisation (descriptor `migrations[]` → disk) is also deferred; pairing it with the runner-side single-tx wiring keeps the apply-side ordering and the on-disk materialisation cohesive. Sub-spec § 3 — Drift detection (T1.9).
…ementation The previous docstring read as if the predicate excluded only 'pure-codecId' diffs, but the implementation returns false on *any* codecId difference (including diffs where another property also changed). Update the docstring to describe the actual behaviour so readers don't expect altered events for mixed codec/non-codec diffs.
…, drop redundant cast Two changes wired together because they touch the same call site: - Extend isSqlControlAdapter to require a readAllMarkers method so adapters missing it fail fast at descriptor-load validation rather than at runtime when the per-space verifier asks for it. - Widen DescriptorSelfConsistencyInputs.storage to `unknown` so the family-bound `SqlStorage` value can be passed without an inline double-cast. The helper was already canonicalising through JSON.stringify and only needs a record-shaped value at runtime; the single internal cast keeps the public type family-agnostic.
SqlMigrationRunnerExecuteOptions.space documented itself as defaulting to 'app' but the runners actually fell back to plan.spaceId. The stale docstring also masked the real risk: a caller that supplies a space disagreeing with plan.spaceId would silently write the marker to the wrong row. Update the docstring to describe the runtime behaviour and add a runtime assertion in both Postgres and SQLite runners so a plan/space mismatch fails fast at executeOnConnection. Also include the FK-integrity attribution improvement so the post-loop integrity error surfaces the last successfully-applied space rather than hard-coding APP_SPACE_ID.
…rectories getLatestMigrationDir(...) walked every entry under migrations/app and picked the newest by mtime, so the refs/ subdirectory (or any non-directory artefact) could be selected and break/flap migration.ts resolution. Filter to entries that are directories and drop the refs sibling explicitly.
The constant and its placeholder were dead code — the test no longer relies on a stand-in hash for the app member because the loader synthesises it from the user contract. Removing both leaves the assertions in this test focused on the behaviour the test is named after.
Where two or more related fields are asserted on the same value, collapse them into a single toMatchObject so a regression points at the failing object as a whole rather than at one of several sequential expectations.
Pulls the duplicated `(pack) => cs ? { id, contractSpace } : { id }`
projection scattered across the CLI utilities into one helper as part
of M6. Adds AC11 (single descriptor-import boundary), updates the
implementation footprint, and resequences M6 so the helper lands first
and every command rewires onto it from the start.
wmadden
force-pushed
the
tml-2397-codec-hooks-and-db-init-update
branch
from
18d9e73 to
104d09a
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/e2e/framework/test/utils.ts (1)
103-116:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid shared hardcoded migrations temp directory across e2e runs.
Line 115 and Line 139 both use the same global
/tmp/__e2e-test-migrationspath. This introduces shared mutable state across tests and leaves artifacts behind, which can cause flaky parallel runs and cross-test contamination.Suggested fix (per-run temp dir + cleanup)
-import { readFile } from 'node:fs/promises'; +import { mkdtemp, readFile, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'pathe'; export async function runDbInit(options: { readonly connectionString: string; readonly contractJsonPath: string; + readonly migrationsDir: string; }): Promise<void> { - const { connectionString, contractJsonPath } = options; + const { connectionString, contractJsonPath, migrationsDir } = options; const contractJson = await loadRawContractFromDisk(contractJsonPath); const controlClient = createControlClientForTests(connectionString); try { const result = await controlClient.dbInit({ contract: contractJson, mode: 'apply', - migrationsDir: '/tmp/__e2e-test-migrations', + migrationsDir, }); @@ async function getPlannedDdlSql(options: { readonly connectionString: string; readonly contract: Record<string, unknown>; + readonly migrationsDir: string; }): Promise<string> { - const { connectionString, contract } = options; + const { connectionString, contract, migrationsDir } = options; const controlClient = createControlClientForTests(connectionString); try { const result = await controlClient.dbInit({ contract, mode: 'plan', connection: connectionString, - migrationsDir: '/tmp/__e2e-test-migrations', + migrationsDir, }); @@ export async function withTestRuntime<TContract extends Contract<SqlStorage>>( @@ ): Promise<void> { const contractJson = await loadRawContractFromDisk(contractJsonPath); const contract = validateContract<TContract>(contractJson, emptyCodecLookup); + const migrationsDir = await mkdtemp(join(tmpdir(), '__e2e-test-migrations-')); - await withDevDatabase(async ({ connectionString }) => { - const sql = await getPlannedDdlSql({ connectionString, contract: contractJson }); - await runDbInit({ connectionString, contractJsonPath }); + try { + await withDevDatabase(async ({ connectionString }) => { + const sql = await getPlannedDdlSql({ connectionString, contract: contractJson, migrationsDir }); + await runDbInit({ connectionString, contractJsonPath, migrationsDir }); @@ - }); + }); + } finally { + await rm(migrationsDir, { recursive: true, force: true }); + } }As per coding guidelines, "
{test/integration/**,test/e2e/**}/**/*.{js,ts,mjs,mts,cjs,cts}: Cleanup logic in tests should remove temporary directories after each run."Also applies to: 127-140
🤖 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 `@test/e2e/framework/test/utils.ts` around lines 103 - 116, The test uses a hardcoded shared migrations dir "/tmp/__e2e-test-migrations" in runDbInit when calling controlClient.dbInit, which risks cross-test interference; change this to create a unique per-run temp directory (e.g., using OS temp + a random/uuid suffix or fs.mkdtemp) and pass that path to controlClient.dbInit, then ensure you remove the temp directory in a finally block (or use a scoped cleanup helper) to delete artifacts even on error; apply the same change to the other dbInit invocation in this test file so both usages (runDbInit and the second dbInit call) use per-run temp dirs and perform cleanup.
♻️ Duplicate comments (1)
packages/1-framework/3-tooling/migration/src/aggregate/verifier.ts (1)
104-109:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove transient artifact reference from source comment.
The inline phrase
closes F23is a transient tracking reference and should be replaced with a behavior-only statement.As per coding guidelines, “Source-code comments must not reference transient project artifacts including milestone-task IDs ... and sub-spec references.”
🤖 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/migration/src/aggregate/verifier.ts` around lines 104 - 109, Update the comment block describing schemaCheck: remove the transient tracking phrase "closes F23" and replace it with a behavior-only sentence that explains the effect (e.g., that pre-projection prevents the family's single-contract verifier from seeing other members' tables as `extras`). Ensure the comment still references the relevant symbols `projectSchemaToSpace` and `verifySchemaForMember` and focuses only on observable behavior without any milestone/task IDs.
🧹 Nitpick comments (3)
packages/1-framework/3-tooling/cli/src/exports/control-api.ts (1)
24-25: ⚡ Quick winRemove legacy-transition phrasing from the source comment.
Please reword this to describe current behavior only, without “legacy vs new” framing.
Suggested edit
-// Standalone operations (for tooling that doesn't need full client). The -// aggregate-pipeline operations replace the legacy per-space helpers. +// Standalone operations for tooling that doesn't need the full client. +// These operations run through the aggregate pipeline.As per coding guidelines, "Do not add backward-compatibility shims, migration scaffolding, deprecation warnings, or comments explaining legacy vs. new approaches."
🤖 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/exports/control-api.ts` around lines 24 - 25, Update the top-of-file comment that currently reads "Standalone operations (for tooling that doesn't need full client). The aggregate-pipeline operations replace the legacy per-space helpers." to remove any "legacy vs new" framing; instead describe only current behavior — e.g., state that this module exports standalone aggregate-pipeline operations for tooling that does not require the full client. Edit the comment surrounding the "Standalone operations" and "aggregate-pipeline operations" phrases so it concisely documents current functionality without referencing legacy migration or replacements.test/integration/test/cli.db-init.e2e.test.ts (1)
297-301: 💤 Low valueRemove milestone reference from comment.
The comment references "(M2)" as a milestone identifier. Per coding guidelines, comments must not reference transient project artifacts like milestone names. Describe the behavior directly instead.
Suggested comment rewording
- // Verify structure - should be noop with existing marker. - // After the per-space consolidation (M2) the noop case routes - // through `executeAcrossSpaces` with an empty plan; the summary - // reflects the multi-space envelope rather than the legacy - // single-space "already at target" string. + // Verify structure - should be noop with existing marker. + // The noop case routes through `executeAcrossSpaces` with an + // empty plan; the summary reflects the multi-space envelope + // rather than the legacy single-space "already at target" string.🤖 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 `@test/integration/test/cli.db-init.e2e.test.ts` around lines 297 - 301, Update the inline comment that currently includes the milestone tag "(M2)" to remove the transient milestone reference and instead describe the behavior directly: change the comment around the noop verification so it explains that after the per-space consolidation the noop path goes through executeAcrossSpaces with an empty plan and that the summary reports a multi-space envelope rather than the legacy single-space "already at target" string; update the comment near the noop verification in test/integration/test/cli.db-init.e2e.test.ts accordingly.packages/1-framework/3-tooling/migration/test/deletable-node-modules.test.ts (1)
1-23: ⚡ Quick winRemove transient AC/spec references from these comments.
These blocks bake AC/TC/M2.5/T1.5 labels and spec-path references into source comments, which the repo guidelines explicitly ask us not to keep in code. Please reword them in terms of the behavior being locked, or use a durable reference like
TML-2397if you need traceability.As per coding guidelines,
Source-code comments must not reference transient project artifacts including: projects/<x>/... paths, milestone-task IDs ..., milestone-named acceptance criteria ....Also applies to: 181-199
🤖 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/migration/test/deletable-node-modules.test.ts` around lines 1 - 23, Remove the transient AC/TC/M2.5/T1.5 labels and the projects/... spec-path references from the top comment; instead state the concrete behaviour being locked (e.g., that the per-space verifier and runner read only on-disk artefacts under migrations/<space-id> and the live marker rows and do not import extension descriptor modules), or cite a durable tracker like TML-2397 if traceability is required; apply the same rewrite to the other, similar comment block in this file that documents the same fixture so both comments reference behavior and durable IDs only (see the mention of emitContractSpaceArtefacts, listContractSpaceDirectories, verifyContractSpaces, concatenateSpaceApplyInputs and the 'test-contract-space' note to locate the blocks).
🤖 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/migration/src/aggregate/verifier.ts`:
- Around line 117-180: verifyAggregate always returns ok(...) even when
projectSchemaToSpace or verifySchemaForMember can throw, breaking the
Result<..., AggregateVerifierError> contract; wrap the schema-check block (the
loop that calls projectSchemaToSpace and verifySchemaForMember, and the
subsequent call to detectOrphanElements) in a try/catch and on any exception
return err(...) with an AggregateVerifierError (e.g. kind:
'introspectionFailure' with the caught error/details). Ensure you use the
existing err(...) helper and keep the rest of the marker checks logic unchanged
so failures from projectSchemaToSpace, verifySchemaForMember, or
detectOrphanElements produce an err result instead of throwing.
In
`@packages/1-framework/3-tooling/migration/test/deletable-node-modules.test.ts`:
- Around line 181-200: The test describes an “loader → planner → verifier”
pipeline but only calls loadContractSpaceAggregate(...) and
verifyAggregate(...), so either invoke the real planner entrypoint between them
(call the planner function, e.g. planContractSpaceAggregate(...) or the
project’s planner method that consumes the loader output) so the planner logic
actually runs, or change the test name/commentary to “loader → verifier” to
reflect current behavior; update the test body to call
planContractSpaceAggregate(...) with the aggregate produced by
loadContractSpaceAggregate before passing its output to verifyAggregate, or
rename the describe string and comments to remove “planner” if you intentionally
skip planning.
In `@packages/3-targets/3-targets/postgres/src/core/migrations/planner.ts`:
- Around line 168-173: Update the comment in planner.ts that documents the codec
lifecycle hook (the block mentioning "Codec lifecycle hook (T2.2): ... Sub-spec
§ 5 ... (M2 R2)") to remove transient project artifact identifiers (T2.2,
Sub-spec § 5, M2 R2) and instead describe the behavior directly: state that
onFieldEvent emitted operations are inlined after structural DDL, that ordering
is structural → added → dropped → altered, and that within-group sorting is
deterministic by (tableName, fieldName) for byte-stable re-emits; keep the note
that the hook fires only at the application emitter and not during
extension-space planning.
In `@test/integration/test/cli-journeys/db-update-workflows.e2e.test.ts`:
- Around line 11-20: Update the comment block beginning "Note: Journey O
("re-init conflict")..." to remove the milestone label "M2" and any other
milestone/task identifiers; instead, describe the historical change in neutral
terms (e.g., "a subsequent refactor" or "during refactor") and keep the
technical details intact: explain that the legacy MARKER_ORIGIN_MISMATCH gate
was removed when the dual `db init` path was consolidated into the per-space
flow, that the per-space planner now reconciles schema drift and treats the
marker as bookkeeping, and that marker-related issues are now handled by the
contract-space verifier (referencing the existing test names
`cli.db-init.contract-space-verifier.test.ts` and
`cli.db-update.contract-space-verifier.test.ts`) so readers get the
behavior/history without any milestone naming.
In `@test/integration/test/cli.db-init.contract-space-verifier.test.ts`:
- Around line 12-20: The comment block in the test referencing transient
identifiers ("sub-spec § 4", "AC-13", "AC-16") must be rewritten to remove those
artifacts; edit the comment near the db init description (mentions `db init`,
`extensionPacks`, and `migrations/<space-id>/`) to retain the rationale about
rejecting orphan marker rows and declared-but-unmigrated extensions but without
milestone or acceptance-criteria labels—use plain language like "rejects when an
orphan marker row exists" and "rejects when an extension is declared but its
migrations directory is missing" so the intent remains clear while avoiding
transient identifiers.
In `@test/integration/test/cli.db-update.contract-space-verifier.test.ts`:
- Around line 83-84: The test currently parses the entire captured console
output (consoleOutput.join('\n')) into JSON which is brittle because non-JSON
lines may be present; implement a small extractor used at both parse sites that
scans consoleOutput (or the joined string) and finds the first or last line that
is valid JSON (e.g., iterate lines and try JSON.parse until one succeeds, or
locate the substring between the first '{' and matching '}' and parse that) and
then use that extracted JSON string to create errorJson; update the places that
set errorText/errorJson to call this extractor so only the JSON envelope is
parsed.
- Around line 12-20: The file-level comment in
cli.db-update.contract-space-verifier.test.ts contains transient references
("sub-spec § 4", "AC-13", "AC-16") — replace them with stable, behavior-oriented
text describing the expected verifier behavior for `db update` (e.g., state that
`db update` must reject when an orphan marker row exists for a non-declared
space and must reject when an extension is declared in `extensionPacks` but no
pinned `migrations/<space-id>/` directory exists), and remove all numeric/spec
identifiers; keep the existing explanatory bullets and the `db update` /
"Verifier wiring contract" wording but only with concrete behavior descriptions.
In `@test/integration/test/control-api.test.ts`:
- Around line 323-328: Remove the transient milestone reference in the inline
comment and rephrase it to describe behavior only: state that applying a second
time is idempotent because the per-space flow relies on planner returning an
empty plan and runner being a no-op on empty plans; note that the legacy
single-space path’s “marker-matches-destination” short-circuit was removed, but
do not include any milestone or slice names. Locate the comment near the
per-space flow test where planner, runner, and marker-matches-destination are
mentioned and replace the historical/milestone text with the concise behavioral
explanation.
---
Outside diff comments:
In `@test/e2e/framework/test/utils.ts`:
- Around line 103-116: The test uses a hardcoded shared migrations dir
"/tmp/__e2e-test-migrations" in runDbInit when calling controlClient.dbInit,
which risks cross-test interference; change this to create a unique per-run temp
directory (e.g., using OS temp + a random/uuid suffix or fs.mkdtemp) and pass
that path to controlClient.dbInit, then ensure you remove the temp directory in
a finally block (or use a scoped cleanup helper) to delete artifacts even on
error; apply the same change to the other dbInit invocation in this test file so
both usages (runDbInit and the second dbInit call) use per-run temp dirs and
perform cleanup.
---
Duplicate comments:
In `@packages/1-framework/3-tooling/migration/src/aggregate/verifier.ts`:
- Around line 104-109: Update the comment block describing schemaCheck: remove
the transient tracking phrase "closes F23" and replace it with a behavior-only
sentence that explains the effect (e.g., that pre-projection prevents the
family's single-contract verifier from seeing other members' tables as
`extras`). Ensure the comment still references the relevant symbols
`projectSchemaToSpace` and `verifySchemaForMember` and focuses only on
observable behavior without any milestone/task IDs.
---
Nitpick comments:
In `@packages/1-framework/3-tooling/cli/src/exports/control-api.ts`:
- Around line 24-25: Update the top-of-file comment that currently reads
"Standalone operations (for tooling that doesn't need full client). The
aggregate-pipeline operations replace the legacy per-space helpers." to remove
any "legacy vs new" framing; instead describe only current behavior — e.g.,
state that this module exports standalone aggregate-pipeline operations for
tooling that does not require the full client. Edit the comment surrounding the
"Standalone operations" and "aggregate-pipeline operations" phrases so it
concisely documents current functionality without referencing legacy migration
or replacements.
In
`@packages/1-framework/3-tooling/migration/test/deletable-node-modules.test.ts`:
- Around line 1-23: Remove the transient AC/TC/M2.5/T1.5 labels and the
projects/... spec-path references from the top comment; instead state the
concrete behaviour being locked (e.g., that the per-space verifier and runner
read only on-disk artefacts under migrations/<space-id> and the live marker rows
and do not import extension descriptor modules), or cite a durable tracker like
TML-2397 if traceability is required; apply the same rewrite to the other,
similar comment block in this file that documents the same fixture so both
comments reference behavior and durable IDs only (see the mention of
emitContractSpaceArtefacts, listContractSpaceDirectories, verifyContractSpaces,
concatenateSpaceApplyInputs and the 'test-contract-space' note to locate the
blocks).
In `@test/integration/test/cli.db-init.e2e.test.ts`:
- Around line 297-301: Update the inline comment that currently includes the
milestone tag "(M2)" to remove the transient milestone reference and instead
describe the behavior directly: change the comment around the noop verification
so it explains that after the per-space consolidation the noop path goes through
executeAcrossSpaces with an empty plan and that the summary reports a
multi-space envelope rather than the legacy single-space "already at target"
string; update the comment near the noop verification in
test/integration/test/cli.db-init.e2e.test.ts accordingly.
🪄 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: 0c79745a-276c-47b6-889b-34116c0c7493
⛔ Files ignored due to path filters (8)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlprojects/extension-contract-spaces/e2e-verification.mdis excluded by!projects/**projects/extension-contract-spaces/plan.mdis excluded by!projects/**projects/extension-contract-spaces/specs/contract-space-aggregate-spec.mdis excluded by!projects/**projects/extension-contract-spaces/specs/contract-space-on-disk-shape.spec.mdis excluded by!projects/**projects/extension-contract-spaces/specs/framework-mechanism.spec.mdis excluded by!projects/**projects/extension-contract-spaces/specs/m2-orchestrator-consolidation-spec.mdis excluded by!projects/**projects/extension-contract-spaces/specs/migration-cli-aggregate.spec.mdis excluded by!projects/**
📒 Files selected for processing (191)
.cursor/rules/doc-maintenance.mdcexamples/mongo-demo/migrations/app/20260409T1030_migration/end-contract.d.tsexamples/mongo-demo/migrations/app/20260409T1030_migration/end-contract.jsonexamples/mongo-demo/migrations/app/20260409T1030_migration/migration.jsonexamples/mongo-demo/migrations/app/20260409T1030_migration/migration.tsexamples/mongo-demo/migrations/app/20260409T1030_migration/ops.jsonexamples/mongo-demo/migrations/app/20260415_add-posts-author-index/end-contract.d.tsexamples/mongo-demo/migrations/app/20260415_add-posts-author-index/end-contract.jsonexamples/mongo-demo/migrations/app/20260415_add-posts-author-index/migration.jsonexamples/mongo-demo/migrations/app/20260415_add-posts-author-index/migration.tsexamples/mongo-demo/migrations/app/20260415_add-posts-author-index/ops.jsonexamples/mongo-demo/test/manual-migration.test.tsexamples/prisma-next-demo/migrations/app/20260422T0720_initial/end-contract.d.tsexamples/prisma-next-demo/migrations/app/20260422T0720_initial/end-contract.jsonexamples/prisma-next-demo/migrations/app/20260422T0720_initial/migration.jsonexamples/prisma-next-demo/migrations/app/20260422T0720_initial/migration.tsexamples/prisma-next-demo/migrations/app/20260422T0720_initial/ops.jsonexamples/prisma-next-demo/migrations/app/20260422T0742_migration/end-contract.d.tsexamples/prisma-next-demo/migrations/app/20260422T0742_migration/end-contract.jsonexamples/prisma-next-demo/migrations/app/20260422T0742_migration/migration.jsonexamples/prisma-next-demo/migrations/app/20260422T0742_migration/migration.tsexamples/prisma-next-demo/migrations/app/20260422T0742_migration/ops.jsonexamples/prisma-next-demo/migrations/app/20260422T0742_migration/start-contract.d.tsexamples/prisma-next-demo/migrations/app/20260422T0742_migration/start-contract.jsonexamples/prisma-next-demo/migrations/app/20260422T0748_migration/end-contract.d.tsexamples/prisma-next-demo/migrations/app/20260422T0748_migration/end-contract.jsonexamples/prisma-next-demo/migrations/app/20260422T0748_migration/migration.jsonexamples/prisma-next-demo/migrations/app/20260422T0748_migration/migration.tsexamples/prisma-next-demo/migrations/app/20260422T0748_migration/ops.jsonexamples/prisma-next-demo/migrations/app/20260422T0748_migration/start-contract.d.tsexamples/prisma-next-demo/migrations/app/20260422T0748_migration/start-contract.jsonexamples/prisma-next-demo/test/utils/control-client.tsexamples/retail-store/migrations/app/20260413T0314_migration/end-contract.d.tsexamples/retail-store/migrations/app/20260413T0314_migration/end-contract.jsonexamples/retail-store/migrations/app/20260413T0314_migration/migration.jsonexamples/retail-store/migrations/app/20260413T0314_migration/migration.tsexamples/retail-store/migrations/app/20260413T0314_migration/ops.jsonexamples/retail-store/migrations/app/20260415_add-product-validation/end-contract.d.tsexamples/retail-store/migrations/app/20260415_add-product-validation/end-contract.jsonexamples/retail-store/migrations/app/20260415_add-product-validation/migration.jsonexamples/retail-store/migrations/app/20260415_add-product-validation/migration.tsexamples/retail-store/migrations/app/20260415_add-product-validation/ops.jsonexamples/retail-store/migrations/app/20260416_backfill-product-status/end-contract.d.tsexamples/retail-store/migrations/app/20260416_backfill-product-status/end-contract.jsonexamples/retail-store/migrations/app/20260416_backfill-product-status/migration.jsonexamples/retail-store/migrations/app/20260416_backfill-product-status/migration.tsexamples/retail-store/migrations/app/20260416_backfill-product-status/ops.jsonexamples/retail-store/test/manual-migration.test.tspackages/1-framework/1-core/framework-components/src/control/control-capabilities.tspackages/1-framework/1-core/framework-components/src/control/control-instances.tspackages/1-framework/1-core/framework-components/src/control/control-migration-types.tspackages/1-framework/1-core/framework-components/src/control/control-spaces.tspackages/1-framework/1-core/framework-components/src/exports/control.tspackages/1-framework/1-core/framework-components/test/codec-call-context.types.test-d.tspackages/1-framework/1-core/framework-components/test/codec.test.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/db-verify.tspackages/1-framework/3-tooling/cli/src/commands/init/index.tspackages/1-framework/3-tooling/cli/src/commands/init/init.tspackages/1-framework/3-tooling/cli/src/commands/migration-apply.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-apply-aggregate.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-init.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-update.tspackages/1-framework/3-tooling/cli/src/control-api/operations/db-verify.tspackages/1-framework/3-tooling/cli/src/control-api/types.tspackages/1-framework/3-tooling/cli/src/exports/control-api.tspackages/1-framework/3-tooling/cli/src/load-ts-contract.tspackages/1-framework/3-tooling/cli/src/utils/combine-schema-results.tspackages/1-framework/3-tooling/cli/src/utils/command-helpers.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-aggregate-loader.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-extension-migrations-pass.tspackages/1-framework/3-tooling/cli/src/utils/contract-space-migrate-pass.tspackages/1-framework/3-tooling/cli/test/commands/migration-invariants.test.tspackages/1-framework/3-tooling/cli/test/commands/migration-tamper.test.tspackages/1-framework/3-tooling/cli/test/config-types.test.tspackages/1-framework/3-tooling/cli/test/control-api/client.test.tspackages/1-framework/3-tooling/cli/test/control-api/db-init.test.tspackages/1-framework/3-tooling/cli/test/control-api/db-update.test.tspackages/1-framework/3-tooling/cli/test/control-api/progress.test.tspackages/1-framework/3-tooling/cli/test/fixtures/disallowed-import.tspackages/1-framework/3-tooling/cli/test/utils/combine-schema-results.test.tspackages/1-framework/3-tooling/cli/test/utils/contract-space-extension-migrations-pass.test.tspackages/1-framework/3-tooling/cli/test/utils/contract-space-migrate-pass.test.tspackages/1-framework/3-tooling/cli/vitest.config.tspackages/1-framework/3-tooling/migration/package.jsonpackages/1-framework/3-tooling/migration/src/aggregate/loader.tspackages/1-framework/3-tooling/migration/src/aggregate/marker-types.tspackages/1-framework/3-tooling/migration/src/aggregate/planner-types.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/assert-descriptor-self-consistency.tspackages/1-framework/3-tooling/migration/src/compute-extension-space-apply-path.tspackages/1-framework/3-tooling/migration/src/concatenate-space-apply-inputs.tspackages/1-framework/3-tooling/migration/src/detect-space-contract-drift.tspackages/1-framework/3-tooling/migration/src/emit-contract-space-artefacts.tspackages/1-framework/3-tooling/migration/src/errors.tspackages/1-framework/3-tooling/migration/src/exports/aggregate.tspackages/1-framework/3-tooling/migration/src/exports/errors.tspackages/1-framework/3-tooling/migration/src/exports/spaces.tspackages/1-framework/3-tooling/migration/src/gather-disk-contract-space-state.tspackages/1-framework/3-tooling/migration/src/plan-all-spaces.tspackages/1-framework/3-tooling/migration/src/read-contract-space-contract.tspackages/1-framework/3-tooling/migration/src/read-contract-space-head-ref.tspackages/1-framework/3-tooling/migration/src/read-pinned-contract-hash.tspackages/1-framework/3-tooling/migration/src/space-layout.tspackages/1-framework/3-tooling/migration/src/verify-contract-spaces.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/assert-descriptor-self-consistency.test.tspackages/1-framework/3-tooling/migration/test/deletable-node-modules.test.tspackages/1-framework/3-tooling/migration/test/detect-space-contract-drift.test.tspackages/1-framework/3-tooling/migration/test/emit-contract-space-artefacts.test.tspackages/1-framework/3-tooling/migration/test/gather-disk-contract-space-state.test.tspackages/1-framework/3-tooling/migration/test/read-contract-space-head-ref.test.tspackages/1-framework/3-tooling/migration/test/read-pinned-contract-hash.test.tspackages/1-framework/3-tooling/migration/test/space-layout.test.tspackages/1-framework/3-tooling/migration/test/verify-contract-spaces.test.tspackages/1-framework/3-tooling/migration/tsdown.config.tspackages/2-mongo-family/9-family/src/core/control-instance.tspackages/2-sql/9-family/src/core/control-adapter.tspackages/2-sql/9-family/src/core/control-instance.tspackages/2-sql/9-family/src/core/migrations/field-event-planner.tspackages/2-sql/9-family/src/core/migrations/types.tspackages/2-sql/9-family/src/exports/control.tspackages/2-sql/9-family/test/control-instance.descriptor-self-consistency.test.tspackages/2-sql/9-family/test/field-event-planner.test.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner.tspackages/3-targets/3-targets/postgres/src/core/migrations/runner.tspackages/3-targets/3-targets/sqlite/src/core/migrations/op-factory-call.tspackages/3-targets/3-targets/sqlite/src/core/migrations/operations/raw.tspackages/3-targets/3-targets/sqlite/src/core/migrations/planner.tspackages/3-targets/3-targets/sqlite/src/core/migrations/runner.tspackages/3-targets/3-targets/sqlite/src/exports/migration.tspackages/3-targets/3-targets/sqlite/src/exports/op-factory-call.tspackages/3-targets/6-adapters/postgres/package.jsonpackages/3-targets/6-adapters/postgres/src/core/control-adapter.tspackages/3-targets/6-adapters/postgres/test/migrations/db-init-update.cli.integration.test.tspackages/3-targets/6-adapters/postgres/test/migrations/planner.codec-field-event.test.tspackages/3-targets/6-adapters/postgres/test/migrations/runner.multi-space.integration.test.tspackages/3-targets/6-adapters/sqlite/package.jsonpackages/3-targets/6-adapters/sqlite/src/core/control-adapter.tspackages/3-targets/6-adapters/sqlite/test/migrations/db-init-update.cli.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/planner.codec-field-event.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/render-typescript.roundtrip.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/runner.multi-space.test.tstest/e2e/framework/test/utils.tstest/integration/package.jsontest/integration/test/authoring/psl.pgvector-dbinit.test.tstest/integration/test/cli-journeys/data-transform-enum-rebuild.e2e.test.tstest/integration/test/cli-journeys/data-transform-not-null-backfill.e2e.test.tstest/integration/test/cli-journeys/data-transform-nullable-tightening.e2e.test.tstest/integration/test/cli-journeys/data-transform-type-change.e2e.test.tstest/integration/test/cli-journeys/db-update-workflows.e2e.test.tstest/integration/test/cli-journeys/drift-deleted-root.e2e.test.tstest/integration/test/cli-journeys/drift-migration-dag.e2e.test.tstest/integration/test/cli-journeys/invariant-routing.e2e.test.tstest/integration/test/cli-journeys/invariant-routing.mongo.e2e.test.tstest/integration/test/cli-journeys/migration-plan-details.e2e.test.tstest/integration/test/cli-journeys/migration-round-trip.e2e.test.tstest/integration/test/cli-journeys/mongo-migration.e2e.test.tstest/integration/test/cli-journeys/schema-evolution-migrations.e2e.test.tstest/integration/test/cli.db-init.contract-space-verifier.test.tstest/integration/test/cli.db-init.e2e.errors.test.tstest/integration/test/cli.db-init.e2e.test.tstest/integration/test/cli.db-update.contract-space-verifier.test.tstest/integration/test/cli.db-verify.aggregate-schema.test.tstest/integration/test/cli.loader.drift.test.tstest/integration/test/cli.migration-apply.e2e.test.tstest/integration/test/contract-space-fixture/constants.tstest/integration/test/contract-space-fixture/contract.tstest/integration/test/contract-space-fixture/descriptor.test.tstest/integration/test/contract-space-fixture/migrations.tstest/integration/test/control-api.test.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-init-with-contract-space/contract.tstest/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-init-with-contract-space/prisma-next.config.with-db.tstest/integration/test/fixtures/cli/cli-e2e-test-app/package.jsontest/integration/test/utils/journey-test-helpers.ts
💤 Files with no reviewable changes (3)
- packages/1-framework/3-tooling/migration/src/read-pinned-contract-hash.ts
- test/integration/test/cli.db-init.e2e.errors.test.ts
- packages/1-framework/3-tooling/migration/test/read-pinned-contract-hash.test.ts
✅ Files skipped from review due to trivial changes (26)
- packages/1-framework/3-tooling/cli/vitest.config.ts
- test/integration/test/fixtures/cli/cli-e2e-test-app/package.json
- packages/3-targets/3-targets/sqlite/src/core/migrations/operations/raw.ts
- packages/1-framework/3-tooling/migration/src/exports/errors.ts
- packages/1-framework/1-core/framework-components/test/codec-call-context.types.test-d.ts
- packages/1-framework/1-core/framework-components/test/codec.test.ts
- test/integration/test/cli-journeys/drift-deleted-root.e2e.test.ts
- packages/3-targets/3-targets/sqlite/src/exports/migration.ts
- packages/1-framework/3-tooling/migration/src/exports/aggregate.ts
- test/integration/test/contract-space-fixture/descriptor.test.ts
- test/integration/test/cli-journeys/data-transform-nullable-tightening.e2e.test.ts
- packages/1-framework/1-core/framework-components/src/control/control-spaces.ts
- test/integration/test/cli-journeys/data-transform-enum-rebuild.e2e.test.ts
- packages/1-framework/3-tooling/migration/src/space-layout.ts
- test/integration/test/contract-space-fixture/migrations.ts
- packages/1-framework/3-tooling/migration/src/concatenate-space-apply-inputs.ts
- .cursor/rules/doc-maintenance.mdc
- packages/3-targets/3-targets/sqlite/src/exports/op-factory-call.ts
- packages/1-framework/3-tooling/migration/tsdown.config.ts
- packages/1-framework/3-tooling/cli/src/commands/init/init.ts
- packages/3-targets/6-adapters/sqlite/test/migrations/planner.codec-field-event.test.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-invariants.test.ts
- packages/3-targets/6-adapters/postgres/test/migrations/runner.multi-space.integration.test.ts
- packages/1-framework/3-tooling/migration/src/gather-disk-contract-space-state.ts
- packages/1-framework/3-tooling/migration/src/plan-all-spaces.ts
- packages/1-framework/3-tooling/migration/src/aggregate/types.ts
🚧 Files skipped from review as they are similar to previous changes (92)
- test/integration/test/contract-space-fixture/constants.ts
- packages/1-framework/1-core/framework-components/src/control/control-capabilities.ts
- packages/1-framework/3-tooling/migration/src/aggregate/marker-types.ts
- packages/1-framework/3-tooling/migration/package.json
- test/integration/test/fixtures/cli/cli-e2e-test-app/fixtures/db-init-with-contract-space/prisma-next.config.with-db.ts
- packages/1-framework/3-tooling/cli/test/config-types.test.ts
- test/integration/test/cli.migration-apply.e2e.test.ts
- packages/1-framework/3-tooling/migration/test/aggregate/strategies/synth.test.ts
- test/integration/test/cli-journeys/migration-round-trip.e2e.test.ts
- packages/1-framework/3-tooling/migration/src/read-contract-space-contract.ts
- packages/1-framework/1-core/framework-components/src/exports/control.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-show.ts
- packages/1-framework/3-tooling/migration/src/aggregate/project-schema-to-space.ts
- test/integration/test/cli-journeys/migration-plan-details.e2e.test.ts
- packages/1-framework/3-tooling/cli/src/commands/init/index.ts
- packages/2-sql/9-family/test/control-instance.descriptor-self-consistency.test.ts
- packages/1-framework/3-tooling/cli/src/utils/combine-schema-results.ts
- packages/3-targets/6-adapters/postgres/test/migrations/planner.codec-field-event.test.ts
- packages/1-framework/3-tooling/migration/test/aggregate/project-schema-to-space.test.ts
- packages/1-framework/3-tooling/cli/src/commands/db-update.ts
- packages/1-framework/3-tooling/migration/src/read-contract-space-head-ref.ts
- packages/1-framework/3-tooling/migration/test/gather-disk-contract-space-state.test.ts
- examples/retail-store/test/manual-migration.test.ts
- packages/1-framework/3-tooling/migration/src/assert-descriptor-self-consistency.ts
- packages/2-sql/9-family/src/core/migrations/field-event-planner.ts
- test/integration/test/cli-journeys/drift-migration-dag.e2e.test.ts
- packages/1-framework/3-tooling/migration/src/aggregate/strategies/synth.ts
- packages/1-framework/3-tooling/migration/test/assert-descriptor-self-consistency.test.ts
- test/integration/package.json
- packages/3-targets/3-targets/sqlite/src/core/migrations/planner.ts
- packages/1-framework/3-tooling/migration/src/aggregate/planner-types.ts
- packages/1-framework/3-tooling/cli/src/control-api/types.ts
- packages/1-framework/3-tooling/cli/test/utils/contract-space-migrate-pass.test.ts
- packages/1-framework/3-tooling/cli/src/utils/contract-space-aggregate-loader.ts
- packages/1-framework/3-tooling/cli/test/utils/contract-space-extension-migrations-pass.test.ts
- examples/mongo-demo/test/manual-migration.test.ts
- packages/1-framework/3-tooling/cli/src/commands/db-init.ts
- packages/1-framework/3-tooling/cli/src/control-api/operations/db-apply-aggregate.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts
- packages/3-targets/6-adapters/sqlite/test/migrations/render-typescript.roundtrip.test.ts
- packages/1-framework/3-tooling/cli/test/commands/migration-tamper.test.ts
- test/integration/test/contract-space-fixture/contract.ts
- packages/1-framework/3-tooling/migration/src/compute-extension-space-apply-path.ts
- test/integration/test/cli.loader.drift.test.ts
- test/integration/test/cli-journeys/invariant-routing.e2e.test.ts
- packages/1-framework/3-tooling/migration/src/errors.ts
- test/integration/test/cli-journeys/schema-evolution-migrations.e2e.test.ts
- packages/1-framework/3-tooling/cli/test/utils/combine-schema-results.test.ts
- packages/2-sql/9-family/src/core/migrations/types.ts
- packages/2-sql/9-family/src/core/control-adapter.ts
- packages/1-framework/3-tooling/migration/test/detect-space-contract-drift.test.ts
- packages/1-framework/3-tooling/migration/test/space-layout.test.ts
- packages/1-framework/3-tooling/migration/src/emit-contract-space-artefacts.ts
- packages/1-framework/3-tooling/migration/test/aggregate/strategies/graph-walk.test.ts
- packages/1-framework/3-tooling/migration/src/aggregate/loader.ts
- examples/prisma-next-demo/test/utils/control-client.ts
- test/integration/test/cli-journeys/mongo-migration.e2e.test.ts
- packages/1-framework/3-tooling/cli/src/control-api/operations/db-init.ts
- packages/1-framework/3-tooling/migration/test/aggregate/verifier.test.ts
- packages/1-framework/3-tooling/cli/src/utils/contract-space-migrate-pass.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-apply.ts
- packages/1-framework/3-tooling/cli/test/control-api/progress.test.ts
- packages/1-framework/3-tooling/cli/test/control-api/db-init.test.ts
- packages/3-targets/3-targets/sqlite/src/core/migrations/op-factory-call.ts
- test/integration/test/utils/journey-test-helpers.ts
- packages/1-framework/3-tooling/migration/src/detect-space-contract-drift.ts
- packages/1-framework/3-tooling/cli/src/utils/command-helpers.ts
- packages/1-framework/3-tooling/cli/src/commands/db-verify.ts
- packages/3-targets/6-adapters/sqlite/test/migrations/runner.multi-space.test.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-status.ts
- packages/1-framework/3-tooling/migration/src/aggregate/strategies/graph-walk.ts
- packages/2-sql/9-family/src/core/control-instance.ts
- packages/1-framework/3-tooling/cli/src/utils/contract-space-extension-migrations-pass.ts
- packages/1-framework/3-tooling/migration/src/aggregate/planner.ts
- packages/1-framework/3-tooling/cli/test/control-api/client.test.ts
- packages/1-framework/3-tooling/migration/test/read-contract-space-head-ref.test.ts
- packages/2-sql/9-family/test/field-event-planner.test.ts
- packages/3-targets/6-adapters/sqlite/test/migrations/db-init-update.cli.test.ts
- packages/1-framework/3-tooling/migration/test/aggregate/planner.test.ts
- packages/1-framework/3-tooling/cli/test/control-api/db-update.test.ts
- packages/1-framework/3-tooling/cli/src/control-api/client.ts
- packages/1-framework/3-tooling/migration/test/emit-contract-space-artefacts.test.ts
- packages/3-targets/6-adapters/postgres/test/migrations/db-init-update.cli.integration.test.ts
- test/integration/test/cli-journeys/invariant-routing.mongo.e2e.test.ts
- test/integration/test/cli-journeys/data-transform-type-change.e2e.test.ts
- packages/1-framework/3-tooling/cli/src/load-ts-contract.ts
- packages/1-framework/3-tooling/migration/test/aggregate/loader.test.ts
- packages/1-framework/3-tooling/migration/src/verify-contract-spaces.ts
- packages/1-framework/3-tooling/migration/test/verify-contract-spaces.test.ts
- packages/1-framework/3-tooling/cli/src/control-api/operations/db-verify.ts
- packages/1-framework/3-tooling/cli/src/control-api/operations/db-update.ts
- packages/3-targets/3-targets/sqlite/src/core/migrations/runner.ts
…ON parse against Node warnings Two unrelated CI fixes for the M2 integration suite: - cli.db-init.e2e.errors: drop the "marker mismatch > does not reference non-existent db migrate command" block. It was a regression guard added in c140946 against a stale `prisma-next db migrate` reference in the MARKER_ORIGIN_MISMATCH fix string. M2.collapse-dual-paths (4bd5c99) intentionally removed it; the prior rebase resolution re-introduced it by mistake. The locked string is gone, and the new aggregate flow no longer surfaces this exact error path here, so the guard is obsolete. - cli.db-update.contract-space-verifier: replace `JSON.parse` with the shared `parseJsonObjectFromCliCapture` helper (already used by the sibling tests). Node intermittently emits the SQLite ExperimentalWarning to stderr, which `setupCommandMocks` captures into `consoleOutput` ahead of the JSON envelope, breaking `JSON.parse` non-deterministically.
…tion throws
Wrap the verifyAggregate body so a throw from projectSchemaToSpace or
the caller-supplied verifySchemaForMember surfaces as
notOk({ kind: introspectionFailure, detail }) instead of escaping the
typed Result channel callers depend on. Adds verifier.test.ts coverage
for both throw sites.
The second describe block in deletable-node-modules.test.ts only invokes loadContractSpaceAggregate and verifyAggregate; planner coverage lives in the dedicated planner tests. Rename the describe + docstring so the fixture description no longer overstates the pipeline scope.
The hardcoded /tmp/__e2e-test-migrations path made parallel e2e runs contaminate each other. Allocate a fresh mkdtemp directory inside withTestRuntime (and as a default for runDbInit / getPlannedDdlSql) and remove it in a finally block. Both the plan and apply invocations share one directory per run so the apply step still sees what the plan step wrote.
Replaces milestone IDs (M2, M2.5), task/AC/TC labels (T1.5, T2.2, AC-13, AC-15, AC-16, TC-26), sub-spec section pointers, projects/<x>/ paths, and "legacy vs new" framing in comments across the postgres planner, the aggregate verifier, the cli control-api re-export, several integration tests and the deletable-node-modules tests. Behavioural context is preserved in plain prose; durable references (TML-2397) remain.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/migration/src/aggregate/verifier.ts`:
- Line 1: The import mixes values and a type; change the single statement
importing notOk, ok, and type Result into two statements: keep value imports
(notOk, ok) in the existing import and add a dedicated top-level `import type {
Result } from '@prisma-next/utils/result'`; update any references if necessary
but no behavioral changes are needed (target symbols: notOk, ok, Result in
verifier.ts).
In `@test/e2e/framework/test/utils.ts`:
- Line 4: Replace the import of join from 'node:path' with the cross-runtime
'pathe' package: locate the import statement that imports join (currently
"import { join } from 'node:path'") and change it to import join from 'pathe'
(or the named import form supported by pathe in your codebase), so all usages of
join in utils.ts continue to work across Node, Deno, Bun and edge runtimes.
🪄 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: a08e812d-b8cc-444e-856c-14bc94096374
📒 Files selected for processing (11)
packages/1-framework/3-tooling/cli/src/exports/control-api.tspackages/1-framework/3-tooling/migration/src/aggregate/verifier.tspackages/1-framework/3-tooling/migration/test/aggregate/verifier.test.tspackages/1-framework/3-tooling/migration/test/deletable-node-modules.test.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner.tstest/e2e/framework/test/utils.tstest/integration/test/cli-journeys/db-update-workflows.e2e.test.tstest/integration/test/cli.db-init.contract-space-verifier.test.tstest/integration/test/cli.db-init.e2e.test.tstest/integration/test/cli.db-update.contract-space-verifier.test.tstest/integration/test/control-api.test.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/1-framework/3-tooling/cli/src/exports/control-api.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/planner.ts
- test/integration/test/cli.db-update.contract-space-verifier.test.ts
- packages/1-framework/3-tooling/migration/test/aggregate/verifier.test.ts
- test/integration/test/cli.db-init.contract-space-verifier.test.ts
- test/integration/test/control-api.test.ts
- test/integration/test/cli-journeys/db-update-workflows.e2e.test.ts
- packages/1-framework/3-tooling/migration/test/deletable-node-modules.test.ts
Separate the inline `type Result` modifier into a dedicated `import type` statement, consistent with the rest of the file and the biome useImportType rule.
wmadden
added a commit
that referenced
this pull request
May 10, 2026
…ry branch Lifts combine-schema-results.ts from 80% to 100% branch coverage by adding tests for the empty-perSpace wiring-bug throw, the app-id-absent fallback to the first iterator value, the default 'PN-RUN-3010' code when a failing app result has no code, and the second-failure preserved (firstFailure latching) path. Also tightens the summary computation by replacing the unreachable '?? appResult.summary' fallback (the mirror shape app-fails/all-pass is impossible because appResult always participates in okAll) with a typed assertion plus the rationale in a comment. Closes the M2 Coverage CI failure on PR #438.
wmadden
added a commit
that referenced
this pull request
May 10, 2026
wmadden
added a commit
that referenced
this pull request
May 10, 2026
wmadden
added a commit
that referenced
this pull request
May 10, 2026
wmadden
added a commit
that referenced
this pull request
May 10, 2026
wmadden
added a commit
that referenced
this pull request
May 10, 2026
This was referenced May 10, 2026
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.
Summary
Milestones 2 + 2.5 + 2.5b of TML-2397 — Contract spaces. Stacked on top of #434 (M1).
This PR is the full framework-side contract-space foundation, before any in-tree extension migrates to a contract space (M3 cipherstash, M4 pgvector). Three milestones folded into one PR because they're a single coherent architectural unit — splitting them post-hoc would invent a meaningless review boundary at the loader/aggregate seam:
db init/db update, SQL-family runner restructured for multi-space outer transactions, verifier wired intodb verify/db init, pinned-artefact materialisation, drift detection.(contract, extensionContractSpaces, migrationsDir)triple flowing through M2's per-space orchestrator is replaced with a typedContractSpaceAggregateproduced once by a loader. Three new components in@prisma-next/migration-tools/aggregate— loader, aggregate planner, aggregate verifier — consume the aggregate uniformly. CLI commands collapse to ~10-line pipelines.migrations/<appSpaceId>/(i.e.migrations/app/), uniform with extensions; theif (spaceId === APP_SPACE_ID) return projectMigrationsDirasymmetry inspaceMigrationDirectoryis gone; the antonym-less "pinned" qualifier is dropped from the artefact-emitter helper / type / file-name surface (emitPinnedSpaceArtefacts→emitContractSpaceArtefacts, three sibling readers renamed,errorPinnedArtefactsAppSpacedeleted).Also in this PR (planning-only, no code): M6 spec + e2e-verification + plan for promoting the user-facing
migrationCLI surface (plan/status/apply) to operate on the contract-space aggregate. Promoted to a real milestone after end-to-end testing on a separate cipherstash-integration branch revealed the migration commands aren't multi-space-aware. M6 is sequenced before M5 (close-out) becausemigration applyis the production deploy path; M3 / M4 / M5 cascade-rebase onto this PR's tip.What lands
M2 R1 — codec hooks (4 commits)
onFieldEventlifecycle hook (T2.1)StorageTable/StorageColumntypes + cast-at-integration-siteM2 R2 — verifier + drift detection + descriptor self-consistency (5 commits)
RawSqlCallround-trip fixture (F4 fix)TEST_HEAD_HASHcontent-addressedassertDescriptorSelfConsistencyat family-createdb init/db verifyM2 R3 — marker-aware verifier + materialisation (3 commits)
db verifyplanAllSpacesM2 R4 — runner restructure (2 commits)
executeOnConnection+executeAcrossSpacesfor multi-space outer-txM2 R5 — per-space db init/update + close-out (5 commits)
db init/db updateviaexecuteAcrossSpaces(T2.3, T2.4)onFieldEventfires through per-space db init + on field dropM2 R6 — code-review polish (cosmetic)
edgesByHashmap dropped, span-id constants extracted, post-consolidation comments refreshed,pruneSchemaByOtherSpaceContractsduck-typing pinned.M2 R7 — source-comment hygiene
@see specs/…,sub-spec § N,T1.x/T2.x,AMxx) from 30 source files inpackages/(F29). New.cursor/rules/doc-maintenance.mdcclause locks the rule for future review.M2.5 — aggregate refactor (8 commits)
216968a30ContractSpaceAggregate types and loader (loader folds layout precheck, integrity checks, drift detection, disjointness check into one step; drift is fatal).c87064eb0aggregate planner with graph-walk and synth strategies; strategy selection iscallerPolicy.ignoreGraphFor-driven; explicitextensionPathUnsatisfiable/policyConflicterror variants.a0291aa7caggregate verifier (markerCheck+schemaCheckbundled); per-member pre-projection of the live schema.494cf8b46rewiredb init/db update/db verifyasloader → planner → runner(orloader → verifier) pipelines.26dc050c9delete deprecated per-space CLI surfaces (db-apply-per-space.ts,contract-space-verifier-{precheck,marker-check}.ts,contract-space-migrate-pass.ts's db-init/update/verify-facing portions,concatenateSpaceApplyInputsre-export).43073f18dre-add orphan-marker preflight to aggregate apply pipeline (regression caught by AC13 gate; now lives indb-apply-aggregate.tsasdetectOrphanMarkers, error code5002).6a1096d61integration tests locking aggregate schema verification, drift-as-fatal, and Postgres CLI per-space e2e.08bd5823ftightenexecuteDbVerify, addschemaCheck.orphanElementsdiscriminated union, fix mixed-pass summary (AC5 LOC budget + per-family summary preserved when consistent / fall back to first failing member when mixed).32c6c6bf4walk loader → verifier with deletednode_modules(AC15 pipeline lock; loader's no-descriptor-import boundary).M2.5 cleanup — adopt M1-cleanup conventions on M2.5 surfaces (1 commit)
467d0d4c1Replaces remainingExtension*types fromfamily-sql/controlwith the canonical types fromframework-components/control(ContractSpaceHeadRef,MigrationPackage,ContractSpace); rebases-only fix-up.M2.5b — uniform on-disk layout (1 commit + plan/spec docs)
f16d817d9flattenspaceMigrationDirectory(drop theif (spaceId === APP_SPACE_ID) return projectMigrationsDirbranch); renameemit-pinned-space-artefacts.ts→emit-contract-space-artefacts.ts(and identifier renamesemitPinnedSpaceArtefacts→emitContractSpaceArtefacts,PinnedSpaceArtefactInputs→ContractSpaceArtefactInputs); rename three sibling readers (read-pinned-space-contract.ts,read-pinned-head-ref.ts); foldread-pinned-contract-hash.tsintoread-contract-space-head-ref.ts; deleteerrorPinnedArtefactsAppSpace; replace localPinnedSpaceHeadRefmirror with direct import ofContractSpaceHeadReffrom@prisma-next/framework-components/control; loader's space-discovery walk filtersAPP_SPACE_IDfrom orphan / declared-but-unmigrated checks.cce305685add the on-disk-shape sub-spec + M5 T5.10 close-out spelling sweep (artefact → artifact) deferred from this slice.M6 (planning-only — no code) (3 commits)
547688fadintroduce M6: promote migration CLI to the contract-space aggregate. Addsspecs/migration-cli-aggregate.spec.md, places M6 before M5 close-out, expands T5.3 (Contract Spaces ADR) and T5.7 (Migration System subsystem docs) to reflect M6's CLI surface.projects/extension-contract-spaces/e2e-verification.md(UX findings F1–F8 from end-to-end testing of cipherstash-integration that motivated M6).2fc19bbabadd the M2.5b plan entry;6966259a0record the M3 / M4 / M5 cascade-rebase fixture-writer heads-up as Open Item Adddb verify schemaCLI command #10 (test fixtures hardcodingmigrations/<package>/paths broke silently during M2.5b — heads-up for downstream re-rebase).Architectural decisions
executeAcrossSpacesopens one outer transaction and fans out toexecuteOnConnectionper space. Failure on any space rolls back every space's writes. Existing single-space callers see no behavior change (execute(...)wrapsexecuteOnConnection).verifySqlSchemarestricted to the'app'space only. Extension contract spaces don't own user-facing tables in the live schema; per-space verifier integrity (orphan-marker / hash-mismatch / invariants-mismatch) is covered byverifyContractSpacesindependently.db updateplans against the live schema, the consumer prunes extension-owned tables from the introspected schema before passing to the app-space planner — mirrorsstrictVerification: falseat the runner-side per-space verifier; both layers enforce disjoint per-space ownership.ContractSpaceAggregateis the single typed lens (M2.5). Loader produces it once from(contract, descriptors, projectMigrationsDir); aggregate planner / aggregate verifier consume it uniformly. CLI commands become ~10-line pipelines (loader → planner → runnerorloader → verifier). Closes M2 R6 R2 review concerns F00 / F05 / F09 / F15 / F23 by deleting the surfaces they were filed against rather than rephrasing them; subsumes F30 (PerSpaceevolutionary naming) by deletion.migrateis the remediation.migrations/<spaceId>/. The loop-control branch in the loader (filterAPP_SPACE_IDfrom orphan / declared-but-unmigrated checks) is intentional: it expresses "the app member is implicitly declared", not a path asymmetry. This locks the on-disk contract before public launch (per the project's pre-launch shape rationale captured in the on-disk-shape sub-spec).*ContractSpace*form. ADR-text-only references to "pinned" intentionally retained where they're historical.AC promotions
node_moduleswalk.projects/extension-contract-spaces/specs/contract-space-aggregate-spec.md§ Acceptance criteria).Test plan
pnpm test:packages125+ tasks green.pnpm test:integration(incl.cli.db-verify.aggregate-schema.test.ts,cli.loader.drift.test.ts, Postgres CLI per-space e2e, deletable-node_moduleswalk).pnpm test:e2e(CLI snapshot tests intest/e2e/exercise migration-directory layouts; M2.5b moves them undermigrations/app/).executeAcrossSpaces.pnpm lint:deps(extended audits enforced by M2.5b validation gate:rg -i 'pinned' packages/ --type ts,rg 'spaceId === APP_SPACE_ID' packages/).Open items
db verify schemaCLI command #10 (M2.5b cascade heads-up): test fixtures hardcodingmigrations/<package>/paths broke silently during M2.5b — relocated five sites tomigrations/app/<package>/. M3 / M4 / M5 cascade re-rebase audit pattern (in plan):rg -n "migrations/[^<{\${]" test/ examples/ --type tsper branch before resolving conflicts.Refs: TML-2397.
Summary by CodeRabbit
New Features
Improvements
Tests