(TML-2397) M1 — Contract spaces: framework mechanism#434
Open
(TML-2397) M1 — Contract spaces: framework mechanism#434
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR converts the contract marker schema from a single-row ChangesPer-space migrations and marker schema
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
@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
changed the title
4 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/1-framework/3-tooling/migration/src/space-layout.ts (1)
28-32: 💤 Low valueConsider a branded type for stronger type safety.
The assertion
asserts spaceId is stringis semantically a no-op since the parameter is alreadystring. If you want the type system to track validated space IDs, consider a branded type:type ValidSpaceId = string & { readonly __brand: 'ValidSpaceId' }; export function assertValidSpaceId(spaceId: string): asserts spaceId is ValidSpaceId { if (!isValidSpaceId(spaceId)) { throw errorInvalidSpaceId(spaceId); } }However, if the current assertion is purely for runtime validation and the branding isn't needed downstream, the current form is acceptable.
🤖 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/space-layout.ts` around lines 28 - 32, The current runtime assertion assertValidSpaceId(spaceId: string): asserts spaceId is string is a no-op at the type level; introduce a branded type (e.g. type ValidSpaceId = string & { readonly __brand: 'ValidSpaceId' }) and change the assertion signature to assertValidSpaceId(spaceId: string): asserts spaceId is ValidSpaceId so the compiler can track validated IDs; add the ValidSpaceId type near the top of the file, update assertValidSpaceId to use it, and adjust any call sites that need the branded type (or explicitly narrow/cast after calling the assertion) so consumers can rely on the stronger type guarantee.packages/3-targets/3-targets/postgres/test/migrations/statement-builders.test.ts (1)
28-80: Follow-up: retire the known coverage debt before warning expiry.Given the CI warning about
operation-resolver.tsandrunner.tsgaps expiring on July 14, 2026, consider adding a focused integration path that exercises descriptor-pipeline resolution through runner execution to close that debt proactively.🤖 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/3-targets/3-targets/postgres/test/migrations/statement-builders.test.ts` around lines 28 - 80, Add an integration test that exercises descriptor-pipeline resolution end-to-end by invoking the Runner (class Runner or runner.run/runner.execute) with a request that hits operation-resolver (the resolver function in operation-resolver.ts) so the pipeline paths are exercised; specifically, create a new test that bootstraps a minimal runner instance, feeds it a descriptor that requires full pipeline resolution, and asserts the resolved descriptor/outcome (this will exercise operation-resolver and runner logic and close the coverage gap). Ensure the new test references the Runner API (Runner.run or Runner.execute) and the operation-resolver entrypoint so CI measures coverage against those files.packages/1-framework/3-tooling/migration/test/emit-pinned-space-artefacts.test.ts (1)
210-214: 💤 Low valueRedundant dynamic import of
mkdir.
mkdiris already imported at line 1 fromnode:fs/promises. The dynamic import here is unnecessary.🧹 Remove redundant import
await writeFile(`${dir}-marker`, 'noop'); // ensure mkdir creates dir // Pre-create a user-authored migration package directory with a stub manifest. - const { mkdir } = await import('node:fs/promises'); await mkdir(userMigration, { recursive: true });🤖 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/emit-pinned-space-artefacts.test.ts` around lines 210 - 214, The dynamic import that re-imports mkdir is redundant; remove the `const { mkdir } = await import('node:fs/promises');` line and use the already-imported `mkdir` symbol (used alongside `writeFile`) to create `userMigration` (e.g., `await mkdir(userMigration, { recursive: true });`), leaving the surrounding stub manifest creation code unchanged.
🤖 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/3-extensions/test-contract-space/README.md`:
- Around line 3-5: The README links to a transient projects path
[`projects/extension-contract-spaces`] which will break; update the README.md
for the package `test-contract-space` so the "contract-space mechanism" link
points to a durable reference (for example the published package, a stable docs
page, or an internal package path), or remove the `projects/` link and replace
it with a permanent anchor or explanatory text; specifically edit the link text
that currently references `projects/extension-contract-spaces` so it targets a
stable resource (or plain text) to satisfy package/docs durability requirements.
---
Nitpick comments:
In `@packages/1-framework/3-tooling/migration/src/space-layout.ts`:
- Around line 28-32: The current runtime assertion assertValidSpaceId(spaceId:
string): asserts spaceId is string is a no-op at the type level; introduce a
branded type (e.g. type ValidSpaceId = string & { readonly __brand:
'ValidSpaceId' }) and change the assertion signature to
assertValidSpaceId(spaceId: string): asserts spaceId is ValidSpaceId so the
compiler can track validated IDs; add the ValidSpaceId type near the top of the
file, update assertValidSpaceId to use it, and adjust any call sites that need
the branded type (or explicitly narrow/cast after calling the assertion) so
consumers can rely on the stronger type guarantee.
In
`@packages/1-framework/3-tooling/migration/test/emit-pinned-space-artefacts.test.ts`:
- Around line 210-214: The dynamic import that re-imports mkdir is redundant;
remove the `const { mkdir } = await import('node:fs/promises');` line and use
the already-imported `mkdir` symbol (used alongside `writeFile`) to create
`userMigration` (e.g., `await mkdir(userMigration, { recursive: true });`),
leaving the surrounding stub manifest creation code unchanged.
In
`@packages/3-targets/3-targets/postgres/test/migrations/statement-builders.test.ts`:
- Around line 28-80: Add an integration test that exercises descriptor-pipeline
resolution end-to-end by invoking the Runner (class Runner or
runner.run/runner.execute) with a request that hits operation-resolver (the
resolver function in operation-resolver.ts) so the pipeline paths are exercised;
specifically, create a new test that bootstraps a minimal runner instance, feeds
it a descriptor that requires full pipeline resolution, and asserts the resolved
descriptor/outcome (this will exercise operation-resolver and runner logic and
close the coverage gap). Ensure the new test references the Runner API
(Runner.run or Runner.execute) and the operation-resolver entrypoint so CI
measures coverage against those files.
🪄 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: b85a0de2-ec74-4bd8-9119-6bea6e223208
⛔ 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 (77)
examples/react-router-demo/test/react-router.smoke.e2e.test.tspackages/1-framework/3-tooling/migration/package.jsonpackages/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/io.tspackages/1-framework/3-tooling/migration/src/exports/spaces.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/space-layout.tspackages/1-framework/3-tooling/migration/src/verify-contract-spaces.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/plan-all-spaces.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/test/write-extension-migration-package.test.tspackages/1-framework/3-tooling/migration/tsdown.config.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/migrations/types.tspackages/2-sql/9-family/src/exports/control.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/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/runner.tspackages/3-targets/3-targets/sqlite/src/core/migrations/statement-builders.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/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/sqlite/src/core/adapter.tspackages/3-targets/6-adapters/sqlite/src/core/control-adapter.tspackages/3-targets/6-adapters/sqlite/test/migrations/marker-schema-migration.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.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
…utcomes Folds the outcomes of the spec-level design discussion into spec.md so the project plan can be derived from a settled spec rather than a draft. The spec was previously well-shaped but left several load-bearing details ambiguous; each ambiguity was resolved in a focused thread and is now reflected as a concrete decision in the requirements + acceptance criteria. Settled (and reflected in the spec): - Migration JSON shape and on-disk layout. Per-space migration directories under migrations/<space-id>/<migration-name>/ (gamma layout); app-space stays flat at root. Framework emits extension migrations from the descriptors in-memory values, not by copying files from node_modules. Byte-equivalence guaranteed by canonicalization. - Cross-space ordering at apply time. Extensions first, app-space second (convention only, matches the implicit dependency direction); single transaction. - Extension descriptor model. Object-based (in-memory JSON values) exposed via the module dependency graph, not filesystem walking. Survives Yarn PnP, Deno, pnpm symlinks, bundlers. Descriptor consumed only at authoring time (migration plan) and verifier time (runtime aggregation); never at apply time. - Codec lifecycle hook contract. Synchronous; receives prior + new IR for the table containing the changed field (app-space scope only); altered fires for any field property change except codecId; returns MigrationOp[] inlined into the app-space migration, app-space-bound by API shape. - Source of truth. Contract canonical via extensionPacks composition; marker rows must match exactly; orphan rows are errors with a clear remediation hint. Lazy marker row creation per space. - db init / db update behaviour. Per-space applications of ADR 208s findPathWithDecision primitive; concatenated per the cross-space ordering convention; single transaction. App-space synthetic-edge greenfield behaviour preserved. - Project scope expanded. Migrate cipherstash + pgvector + arktype-json to contract spaces; remove databaseDependencies.init at end of project. Single mechanism for schema-contributing extensions. - Marker schema. Gains a space column; primary key by space. The structural acceptance criteria grew (AC10/AC11 for pgvector/arktype-json migration, AC12 for db init per-space, AC13 for orphan handling) to match the expanded scope. Open questions reduced from five to two: invariantId namespacing convention and the cipherstash project integration path remain non-blocking. Refs: TML-2397.
Holds extensions to the same WYSIWYG-on-disk principle as app-space: each loaded extension space gets pinned contract.json, contract.d.ts, and refs/head.json under migrations/<space-id>/, alongside its migrations. The framework owns these files and rewrites them on every migrate. Verifier and runner read only the user repo at apply/verify time; the descriptor is needed only at authoring time. Spec: tightens FR2/FR6/FR10/FR16/NFR2/NFR3, adds FR17 (pinned-artefact emission + drift detection), AC14-AC16, and tweaks AC2/AC3/AC6 to exercise the pinned files and the no-descriptor verify path. Plan: new TC-25..TC-30, splits emission into T1.8 (pinned artefacts) and T1.9 (drift detection), renumbers the synthetic test extension to T1.10 (with a node_modules-deleted fixture for TC-26), and adds T3.7 for the bump-cipherstash diff scenario.
Spike on the in-tree extensions found that arktype-json ships no databaseDependencies (jsonb is built-in) and pgvector is the only workspace consumer; cipherstash never landed as a workspace package and so M3 is greenfield authoring rather than a migration. Narrows FR13/AC11, drops M4 arktype-json tasks, and shrinks M5 to the small real removal blast radius (3 files: type def, re-export, pgvector descriptor). Adds two task specs: - specs/framework-mechanism.spec.md drives M1 + M2 with concrete TS shapes, marker-migration SQL, on-disk file paths, canonicalisation rules, codec hook signature, and per-space db init/update flow. - specs/cipherstash-migration.spec.md drives M3 with cipherstash package layout, IR contents, baseline migration shape (with EQL bundle byte-equivalence), codec hook behaviour, and four E2E scenarios (initial / drop / bump / revert workaround). Locks T1.10 to packages/3-extensions/test-contract-space/ as a private workspace package. Notes Linear elevation declined and sub-spec timing chosen as draft-now in the open-items log.
Introduce the in-memory authoring view a schema-contributing extension
publishes through its descriptor module:
ExtensionContractRef - pinned (hash, invariants) head ref.
ExtensionMigrationPackage - in-memory authored migration package
(manifest + ops; no on-disk path).
ExtensionContractSpace - { contractJson, migrations, headRef }
the framework reads at authoring time
and pins into the user repo on emit.
Add the optional `contractSpace` field to SqlControlExtensionDescriptor
and re-export the new types from family-sql/control. The change is
purely additive: extensions without a contract space (today: pgvector,
arktype-json) continue to typecheck unchanged.
Refs: TML-2397
Project: extension-contract-spaces (M1 R1)
…n (T1.10) Add a private workspace fixture that exercises the contract-space mechanism end-to-end against a real `extensionPacks`-shaped descriptor load — without taking on the baggage (vendored bundle SQL, codec hooks, native extension installs) that real consumers like cipherstash and pgvector carry. The descriptor publishes a `contractSpace` declaring a single `test_box` table, a baseline migration that creates it under invariant `test-contract-space:create-test_box-v1`, and a head ref pointing at the post-baseline state. Future rounds wire this fixture into per-space planner / runner / verifier integration tests (TC-22, TC-25..TC-30) and exercise the deletable-node_modules path. Hashes are placeholders (synthetic-* prefix) so the fixture is clearly distinguishable from authoring-pipeline content hashes; later rounds replace them when the per-space emit pipeline lands. Refs: TML-2397 Project: extension-contract-spaces (M1 R1)
…asts (F1) Replace `<value> as Contract['profileHash']` and `<value> as SqlStorage['storageHash']` casts with calls to the existing `profileHash()` and `coreHash()` helpers from `@prisma-next/contract/types`. The helpers carry the brand without the authoring site needing to write `as` casts, matching the repo's typesafety rule (avoid type casts where avoidable) and the convention used by other in-tree authoring sites (e.g. mongo-contract test fixtures). This fixture is a worked example for future extension authors (pgvector M4, monorepo example M4); establishing the helper-call pattern early is cheaper than removing casts later. Closes F1 from M1 R1 review (low / process). Refs: TML-2397.
Adds the foundation for contract spaces by re-keying `prisma_contract.marker` (Postgres) and `_prisma_marker` (SQLite) from the legacy single-row `id` PK to a per-space `space TEXT PRIMARY KEY DEFAULT 'app'`. Existing single-app deployments boot through an idempotent, framework-internal migration that promotes the row to `(space='app', ...)`; new deployments land directly in the new shape. - Postgres: add `migrateMarkerSchemaStatements`, a sequence of guarded `ALTER TABLE` / `DO $$` blocks. Idempotent on fresh, legacy single-row, and already-migrated databases. Applied during `ensureControlTables`. - SQLite: add `migrateMarkerSchemaSqlite`, a PRAGMA-driven rebuild dance (CREATE _new -> INSERT FROM old -> DROP old -> RENAME). SQLite cannot ALTER PRIMARY KEY in place. Applied during `ensureControlTables`, inside the runner's existing BEGIN EXCLUSIVE. - Shared: `sql-marker.ts` and both adapters now accept an optional `space` (defaulting to `APP_SPACE_ID = 'app'`); existing single-app callers keep working with no source change. - Tests: dedicated idempotency integration tests for both targets cover the three boot states (fresh, legacy, already-migrated) per `framework-mechanism.spec.md § 2`. All other marker reads/writes across the test suite migrated to `WHERE space = 'app'`. Refs: TML-2397.
…1.7)
Adds the framework-neutral authoring-time scaffolding the per-space
`migrate` flow needs:
- **T1.6 — Layout convention (γ).** New `./spaces` subpath export that
ships `APP_SPACE_ID`, `isValidSpaceId` / `assertValidSpaceId` (pattern
`[a-z][a-z0-9_-]{0,63}`), and `spaceMigrationDirectory(...)`. App-space
passes through unchanged (`<projectRoot>/migrations`); extension
spaces land under `<projectRoot>/migrations/<space-id>` with the space
id validated as a filesystem-safe directory name.
- **T1.7 — Migration package emission helper.** New
`writeExtensionMigrationPackage(targetDir, pkg)` in `./io`. Writes
`migration.json`, `ops.json`, and a canonical-JSON `contract.json`
snapshot to `<targetDir>/<pkg.dirName>/`. The contract.json
serialisation is deterministic across runs / machines (same input ->
same bytes), enabling per-space PR review and the byte-equivalence
check called for in `framework-mechanism.spec.md § 3`.
- **T1.3 — Per-space planner iterator.** New `planAllSpaces` helper in
`./spaces` that takes `(spaceId, priorContract, newContract)` tuples
plus a per-space `planSpace` callback, returns one
`SpacePlanOutput` per input. The output is sorted alphabetically by
spaceId regardless of input order (AM3); duplicate spaceIds throw
`MIGRATION.DUPLICATE_SPACE_ID` before any callback runs. Today's
single-app behaviour is preserved verbatim when only `app` is in the
input.
The helpers live in `migration-tools` (not `family-sql`) because the
contract-space concept is target-agnostic — Mongo will reuse the same
scaffolding when its per-space pipeline lands. The SQL family wires
these helpers up at the CLI / emitter layer in subsequent rounds.
Refs: TML-2397.
… any callback (F3) Tighten the duplicate-rejection test for `planAllSpaces` to lock in the "rejection happens before any callback runs" atomicity property already documented in the implementation JSDoc and called out in the R3 report. Wraps `planSpace` in `vi.fn` and asserts `not.toHaveBeenCalled()` after the throw, mirroring the empty-input fast-path test. The implementation is unchanged; only the test gets a sharper assertion that catches future regressions which collapse the dedup pass into the same loop as the per-space callback. Refs: TML-2397 — projects/extension-contract-spaces/reviews/code-review.md F3.
…nd verifier helpers (T1.4 + T1.5 + T1.8 + T1.10b)
Land the consumer-side framework primitives for contract-space migrate
/ apply / verify, all in @prisma-next/migration-tools/exports/spaces so
the helpers stay framework-neutral (lint:deps confirms no target-*
references in packages/1-framework). SQL-family wiring at the
consumption site is the next-round work; this round ships the helpers
those consumers compose against.
T1.8 — emitPinnedSpaceArtefacts(projectMigrationsDir, spaceId, inputs):
Writes the three pinned files (contract.json, contract.d.ts,
refs/head.json) under migrations/<spaceId>/. Always-overwrite (the
framework owns these files). Canonical-JSON contract.json so two
emits produce byte-identical output across machines / runs; head.json
serialises invariants alphabetically sorted for the same reason.
Caller renders contract.d.ts via the SQL family typemap-aware
renderer and passes the string in. Rejects the app space (its pinned
shape lives at the project root, not under migrations/) and invalid
space ids (validated against the [a-z][a-z0-9_-]{0,63} pattern from
T1.6). 12 tests.
T1.4 — concatenateSpaceApplyInputs<TOp>(inputs):
Pure ordering helper for the cross-space apply sequence:
extension spaces alphabetical first, app-space last. Generic over
the per-target operation type so the SQL family binds it to its own
SqlMigrationPlanOperation<TTargetDetails> at the use site.
Determinism (NFR6): two callers with the same set of extensionPacks
see identical apply sequences. Atomicity: rejects duplicate spaceIds
with MIGRATION.DUPLICATE_SPACE_ID before producing any output,
mirroring planAllSpaces from R3 so the planner-side and runner-side
helpers reject malformed inputs the same way. 9 tests.
T1.5 — verifyContractSpaces({ loadedSpaces, pinnedDirsOnDisk,
pinnedHashesBySpace, markerRowsBySpace }):
Pure structural verifier covering all five violation kinds:
declaredButUnmigrated (extensionPacks declares a space without a
pinned dir on disk), orphanMarker (marker row whose space is no
longer in extensionPacks), orphanPinnedDir (pinned dir for a space
not in extensionPacks), hashMismatch / invariantsMismatch
(per-space drift between pinned head and marker row).
Output is deterministic (kind first, spaceId alphabetical) so two
callers see byte-identical violation lists. Every violation carries
a string remediation hint following the messages in spec § 4.
listPinnedSpaceDirectories(projectMigrationsDir) (async I/O) ships
alongside, filtering dot-prefixed directories and timestamp-prefixed
app-space migration directories (^\d{8}T\d{4}_) so what is left is
candidates for space-id matching. Returns sorted alphabetically.
18 tests across the two helpers.
T1.10b — deletable-node-modules fixture:
Locks in AC-15 / TC-26 — verifier and runner-ordering helpers
operate without descriptor access. Builds a tmpdir project with
pinned per-space artefacts via emitPinnedSpaceArtefacts, deletes
node_modules, then exercises listPinnedSpaceDirectories +
verifyContractSpaces + concatenateSpaceApplyInputs. The test
intentionally does *not* import @prisma-next/extension-test-contract-space
— the no-descriptor property is what AC-15 locks in. 4 tests.
Refs: TML-2397.
…completed Update plan.md task entries to reflect M1 R4 deliveries: per-space runner ordering helper (T1.4), verifier helpers (T1.5), pinned artefact emission helper (T1.8), and the deletable-node_modules fixture for AC-15 / TC-26 (T1.10b). Each entry names the shipped APIs and links to the migration-tools subpaths consumers import from. Refs: TML-2397.
…(T1.9)
Land the last M1 framework primitive: drift detection between the
descriptor in-memory contract value and the pinned per-space artefacts
on disk. Same target-agnostic placement pattern as R3/R4 helpers — the
SQL family in M2 R1 composes these into the migrate / dbInit pipelines.
detectSpaceContractDrift(spaceId, { descriptorHash, pinnedHash }) is a
pure discriminator: descriptorHash === pinnedHash → noDrift,
pinnedHash === null → firstEmit (extension just added; this run
creates the pinned files), else drift. Threads spaceId / both hashes
through verbatim so the caller (logger / TerminalUI / strict-mode
envelope) has everything it needs to format the AM7 warning without
re-reading the descriptor or the pinned file.
readPinnedContractHash(projectMigrationsDir, spaceId) is the I/O
counterpart. Reads migrations/<spaceId>/refs/head.json (which
emitPinnedSpaceArtefacts wrote with the descriptor headRef.hash) and
returns the hash field as a string. Returns null when the file does
not exist (the firstEmit signal). Validates the space id with the
existing assertValidSpaceId, rejects the app space with the existing
errorPinnedArtefactsAppSpace, and surfaces MIGRATION.INVALID_JSON /
MIGRATION.INVALID_REF_FILE on a corrupt head.json.
Tests: 7 (pure helper) + 8 (I/O wrapper) = 15 new tests.
Refs: TML-2397.
…ndidate Update plan.md T1.9 to [x] with shipped-API references for detectSpaceContractDrift and readPinnedContractHash. With T1.9 landed, M1 reaches 10 of 10 tasks done at the framework-helper level; SQL-family consumption-site wiring lands in M2 R1. Refs: TML-2397.
… shipped helper APIs
Brings framework-mechanism.spec.md into alignment with what M1 R1-R5 actually
shipped, so M2 R1 wiring reads from the source of truth rather than the
original draft signatures.
Five amendments accumulated across the M1 review loop (orchestrator decisions
4-7 + R5 deviation), each replacing draft API sketches with the resolved
shipped APIs:
- § 1: ExtensionMigrationPackage in-memory shape (no dirPath; per R1).
- § 2: control-DDL preflight scope clarified — ADR 029 covers user-DDL only;
ensureControlTables is validated by idempotency tests instead (per R2).
- § 3: planAllSpaces<TContract, TPackage> generic signature with SQL-family
use site (per R3); writeExtensionMigrationPackage / spaceMigrationDirectory
named by their migration-tools/exports/{io,spaces} subpaths (per R3).
- § 3: emitPinnedSpaceArtefacts framework-neutral primitives signature with
SQL-family generateContractDts wiring (per R4).
- § 3 (drift detection): readPinnedContractHash reads pre-computed hash from
refs/head.json rather than re-hashing pinned contract.json — operationally
equivalent under descriptor self-consistency, immune to canonical-JSON
pipeline evolution (per R5). Adds an M2 R1 wiring note recommending
MIGRATION.DESCRIPTOR_HEAD_HASH_MISMATCH as the descriptor-side guard.
- § 4: helper-location paragraph for runner / verifier helpers; replaced
inline draft signatures with concatenateSpaceApplyInputs<TOp> +
verifyContractSpaces + listPinnedSpaceDirectories shipped APIs (per R4).
No code change; closes the M1 sub-spec drift surfaced by the review loop.
The README linked the contract-space mechanism to projects/extension-contract-spaces/, which is transient and gets deleted at project close-out (per drive-project-workflow). Replace with a stable in-repo reference: the @prisma-next/migration-tools spaces export module.
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.
Reuse the top-level node:fs/promises import instead of dynamically re-importing mkdir mid-test.
wmadden
force-pushed
the
tml-2397-contract-spaces-first-class-schema-contributions-from
branch
from
6eeed25 to
a210c79
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/3-targets/3-targets/postgres/src/core/migrations/runner.ts (1)
47-68:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
cloneAndFreezeRecordstill shares objects nested inside arrays.
Object.freeze([...val])only protects the array shell. Ifoperation.metacontains something like[{ ... }], the lateronOperationComplete()callback can still mutate those nested objects through the original operation after the skip record has been pushed, and that mutated metadata is what will be written toexecutedOperations/the ledger. Recurse through array members before freezing, or use a true deep clone/freeze helper here.🤖 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/3-targets/3-targets/postgres/src/core/migrations/runner.ts` around lines 47 - 68, cloneAndFreezeRecord currently only shallow-freezes arrays with Object.freeze([...val]) which leaves object elements inside arrays mutable; change the array branch in cloneAndFreezeRecord to deep-clone and freeze each array element (recursively call cloneAndFreezeRecord for object/array elements, copy primitives as-is), then freeze the resulting array before assigning, so nested objects inside arrays (e.g., operation.meta entries) are fully immutable when used by onOperationComplete()/executedOperations.
🤖 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/3-extensions/test-contract-space/tsdown.config.ts`:
- Around line 3-5: The tsdown config's defineConfig currently only lists
'src/exports/control.ts'; update the entry array in defineConfig to include all
required entry points: 'src/exports/adapter.ts', 'src/exports/types.ts',
'src/exports/codec-types.ts', 'src/exports/control.ts', and
'src/exports/runtime.ts' so the single tsdown configuration block contains every
adapter/types/codec-types/control/runtime entry required by the rule.
---
Outside diff comments:
In `@packages/3-targets/3-targets/postgres/src/core/migrations/runner.ts`:
- Around line 47-68: cloneAndFreezeRecord currently only shallow-freezes arrays
with Object.freeze([...val]) which leaves object elements inside arrays mutable;
change the array branch in cloneAndFreezeRecord to deep-clone and freeze each
array element (recursively call cloneAndFreezeRecord for object/array elements,
copy primitives as-is), then freeze the resulting array before assigning, so
nested objects inside arrays (e.g., operation.meta entries) are fully immutable
when used by onOperationComplete()/executedOperations.
🪄 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: 76689088-09fe-4bab-ad2e-26648f68faff
⛔ 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 (77)
examples/react-router-demo/test/react-router.smoke.e2e.test.tspackages/1-framework/3-tooling/migration/package.jsonpackages/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/io.tspackages/1-framework/3-tooling/migration/src/exports/spaces.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/space-layout.tspackages/1-framework/3-tooling/migration/src/verify-contract-spaces.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/plan-all-spaces.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/test/write-extension-migration-package.test.tspackages/1-framework/3-tooling/migration/tsdown.config.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/migrations/types.tspackages/2-sql/9-family/src/exports/control.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/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/runner.tspackages/3-targets/3-targets/sqlite/src/core/migrations/statement-builders.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/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/sqlite/src/core/adapter.tspackages/3-targets/6-adapters/sqlite/src/core/control-adapter.tspackages/3-targets/6-adapters/sqlite/test/migrations/marker-schema-migration.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.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
✅ Files skipped from review due to trivial changes (19)
- packages/3-targets/3-targets/postgres/src/exports/statement-builders.ts
- packages/2-sql/5-runtime/test/sql-family-adapter.test.ts
- packages/3-targets/6-adapters/postgres/test/migrations/runner.errors.integration.test.ts
- packages/3-extensions/test-contract-space/biome.jsonc
- packages/3-extensions/test-contract-space/tsconfig.prod.json
- packages/1-framework/3-tooling/migration/src/exports/spaces.ts
- packages/2-sql/5-runtime/src/exports/index.ts
- packages/3-extensions/test-contract-space/test/descriptor.test.ts
- packages/3-extensions/test-contract-space/tsconfig.json
- packages/1-framework/3-tooling/migration/test/write-extension-migration-package.test.ts
- packages/1-framework/3-tooling/migration/src/exports/io.ts
- packages/2-sql/9-family/test/migrations.types.test-d.ts
- packages/3-extensions/test-contract-space/README.md
- packages/1-framework/3-tooling/migration/test/space-layout.test.ts
- packages/1-framework/3-tooling/migration/tsdown.config.ts
- packages/3-extensions/test-contract-space/vitest.config.ts
- test/integration/test/cli.db-init.e2e.errors.test.ts
- packages/3-extensions/test-contract-space/src/core/constants.ts
- packages/1-framework/3-tooling/migration/test/concatenate-space-apply-inputs.test.ts
🚧 Files skipped from review as they are similar to previous changes (47)
- packages/3-targets/3-targets/sqlite/src/exports/statement-builders.ts
- test/integration/test/family.sign-database.test.ts
- packages/3-targets/6-adapters/postgres/src/core/adapter.ts
- packages/3-targets/6-adapters/sqlite/test/migrations/runner.idempotency.test.ts
- packages/3-targets/6-adapters/postgres/test/migrations/runner.idempotency.integration.test.ts
- test/integration/test/cli-journeys/drift-marker.e2e.test.ts
- packages/3-targets/6-adapters/sqlite/src/core/adapter.ts
- packages/3-extensions/test-contract-space/src/core/contract.ts
- packages/3-targets/6-adapters/sqlite/test/migrations/runner.errors.test.ts
- test/integration/test/cli.db-sign.e2e.test.ts
- packages/1-framework/3-tooling/migration/src/concatenate-space-apply-inputs.ts
- test/integration/test/cli.migration-apply.e2e.test.ts
- packages/3-targets/6-adapters/postgres/test/migrations/runner.basic.integration.test.ts
- packages/1-framework/3-tooling/migration/src/io.ts
- packages/2-sql/5-runtime/test/sql-marker.test.ts
- packages/2-sql/5-runtime/test/marker-vs-intercept-ordering.test.ts
- packages/1-framework/3-tooling/migration/src/errors.ts
- packages/2-sql/5-runtime/test/utils.ts
- test/integration/test/cli-journeys/greenfield-setup.e2e.test.ts
- packages/1-framework/3-tooling/migration/test/emit-pinned-space-artefacts.test.ts
- packages/1-framework/3-tooling/migration/package.json
- packages/2-sql/5-runtime/test/intercept-decoding.test.ts
- packages/3-extensions/test-contract-space/src/core/migrations.ts
- packages/3-targets/6-adapters/sqlite/test/migrations/runner.basic.test.ts
- packages/1-framework/3-tooling/migration/src/emit-pinned-space-artefacts.ts
- packages/1-framework/3-tooling/migration/src/plan-all-spaces.ts
- packages/1-framework/3-tooling/migration/src/space-layout.ts
- packages/1-framework/3-tooling/migration/test/read-pinned-contract-hash.test.ts
- packages/3-targets/3-targets/sqlite/src/core/migrations/statement-builders.ts
- packages/3-extensions/test-contract-space/package.json
- packages/1-framework/3-tooling/migration/src/detect-space-contract-drift.ts
- packages/2-sql/5-runtime/test/sql-runtime.test.ts
- packages/1-framework/3-tooling/migration/test/verify-contract-spaces.test.ts
- packages/3-targets/6-adapters/postgres/test/adapter.test.ts
- test/e2e/framework/test/sqlite/utils.ts
- test/integration/test/cli.db-init.e2e.test.ts
- packages/2-sql/5-runtime/src/sql-marker.ts
- packages/1-framework/3-tooling/migration/test/deletable-node-modules.test.ts
- test/integration/test/cli-journeys/invariant-routing.e2e.test.ts
- packages/3-targets/3-targets/postgres/test/migrations/statement-builders.test.ts
- packages/1-framework/3-tooling/migration/test/plan-all-spaces.test.ts
- packages/3-extensions/test-contract-space/src/exports/control.ts
- test/integration/test/cli-journeys/migration-apply-edge-cases.e2e.test.ts
- packages/2-sql/9-family/src/core/migrations/types.ts
- packages/3-targets/6-adapters/postgres/src/core/control-adapter.ts
- packages/3-targets/6-adapters/sqlite/test/migrations/marker-schema-migration.test.ts
- packages/1-framework/3-tooling/migration/src/verify-contract-spaces.ts
wmadden
commented
May 8, 2026
wmadden
commented
May 8, 2026
Promote the manifest filename literal from a module-private const to an exported one so other helpers in the package can discriminate migration directories by manifest presence (the same rule readMigrationsDir uses) without re-declaring the literal.
…, not name shape
listPinnedSpaceDirectories distinguished user-authored migration
directories from pinned per-space subdirectories with a regex on the
directory name (^\d{8}T\d{4}_), encoding the formatMigrationDirName
convention as if it were a contract. Directory names belong to users
and are informative, not structural.
Switch to the same discriminator readMigrationsDir already uses:
presence of migration.json at the directory root. A subdirectory is a
migration directory iff it contains the manifest; otherwise it is a
candidate pinned per-space subdirectory. ENOENT means "no manifest";
other stat errors propagate. Sorted, deterministic output is
preserved.
This frees users to name their migration directories however they
like and removes a brittleness flagged in PR #434 review.
…ture to integration-tests workspace (M1-cleanup T-cleanup.1) Drops the workspace package `@prisma-next/extension-test-contract-space` at packages/3-extensions/test-contract-space/. The fixture existed only as self-test scaffolding; no other workspace consumes it. Hosting it under packages/3-extensions/ alongside production extensions (pgvector, cipherstash, arktype-json) gave it a misleading "real extension" shape (F1). Relocates the descriptor + contract + migrations + descriptor sniff-test to test/integration/test/contract-space-fixture/, where the @prisma-next/integration-tests workspace already provides the @prisma-next/family-sql, @prisma-next/sql-contract, and @prisma-next/contract dependencies the fixture needs. M2 T2.5 (codec hook + per-space db init/update tests) will extend this fixture from its new home. Also updates packages/1-framework/3-tooling/migration/test/deletable-node-modules.test.ts: its node_modules stand-in directory name no longer references the dead package; the comment block points at the new fixture location. Refs: TML-2397 Closes-thread: PRRT_kwDOQM0QJc6AnF1X
…cleanup.0/1/2 closure M1s functional acceptance was met but post-implementation review surfaced six design-quality findings (F0–F5 in reviews/code-review.md). Codify the remediation as Milestone 1-cleanup with explicit task breakdown and validation gates so subsequent rounds have a clean contract. R1 closed F0 / F1 / F5: F0 (manifest-presence migration-dir detection) implemented in c19086d90; F1 (test-contract-space fixture relocation) implemented in db33795e3; F5 (CodeRabbit tsdown nitpick) made moot by F1. Mark T-cleanup.0/1/2 done with their resolution notes. Update spec § 7 + § AM11 + References and the resolved-during-finalisation note to point at the fixtures new home (test/integration/test/ contract-space-fixture/) rather than the dropped packages/3-extensions/ test-contract-space/ — kept the design framing while truing up the canonical path. F2 / F3 / F4 (T-cleanup.3 / T-cleanup.4) remain open; they are the next remediation rounds.
…ol-spaces (M1-cleanup F3)
Hoist APP_SPACE_ID into `@prisma-next/framework-components/control` as
the single source of truth (`packages/1-framework/1-core/framework-components/src/control/control-spaces.ts`).
Drop the four duplicate declarations that previously coexisted —
* packages/1-framework/3-tooling/migration/src/space-layout.ts
* packages/2-sql/5-runtime/src/sql-marker.ts
* packages/3-targets/3-targets/postgres/src/core/migrations/statement-builders.ts
* packages/3-targets/3-targets/sqlite/src/core/migrations/statement-builders.ts
— and replace each with an `import { APP_SPACE_ID } from
"@prisma-next/framework-components/control"` plus a re-export so existing
barrel surfaces (sql-runtime, postgres / sqlite statement-builders,
migration-tools/spaces) keep their public shape.
Eliminate raw `"app"` literals in target / runtime / adapter source code
in favour of `APP_SPACE_ID` (or `${APP_SPACE_ID}` inside SQL templates):
* postgres + sqlite ensureMarkerTableStatement (DEFAULT clause)
* postgres migrateMarkerSchemaStatements (UPDATE / ALTER DEFAULT) —
minimal in-place substitution, structure preserved; the array as a
whole is owned by F2 (M1-cleanup T-cleanup.4) and will be deleted
there.
* sqlite migrateMarkerSchemaSqlite (rebuild-table dance)
* postgres + sqlite adapter.readMarkerStatement (params)
* postgres + sqlite control-adapter readMarker (params)
Add a regression guardrail at `scripts/lint-app-space-id.mjs` and wire
it into `pnpm lint:deps`. Two policed invariants:
1. Exactly one `export const APP_SPACE_ID` declaration under
`packages/`, located at the canonical file.
2. No raw `"app"` / `app` literals in `packages/2-sql/**/src` or
`packages/3-targets/**/src` (test files and JSDoc lines are
excluded — the literal is often the test data or prose).
JSDoc lines that document the literal value of APP_SPACE_ID (e.g.
`{@link APP_SPACE_ID} (`app`)`) are intentionally preserved as prose;
they are not runtime values.
Refs: TML-2397, projects/extension-contract-spaces/reviews/code-review.md F3.
…o control plane (M1-cleanup F4)
Move and rename the contract-space identity / authoring types from the
SQL family up into `@prisma-next/framework-components/control`. Contract
spaces are a framework concept (the project spec is family-agnostic per
FRs 3-6), not a SQL one — a Mongo descriptor will eventually consume the
same types specialised to its own contract.
Renames + new home (`control-spaces.ts`):
* ExtensionContractRef -> ContractSpaceHeadRef
* ExtensionMigrationPackage -> AuthoredMigrationPackage
* ExtensionContractSpace -> AuthoredContractSpace<TContract = Contract>
`AuthoredContractSpace` is now generic over its contract value so the
SQL family pins `AuthoredContractSpace<Contract<SqlStorage>>` on
`SqlControlExtensionDescriptor.contractSpace?:` while the framework
shape stays family-neutral.
Also hoists the migration-package metadata types so
`AuthoredMigrationPackage` can reference them from the framework layer
without an upward import to `migration-tools`:
* MigrationHints / MigrationMetadata move to
`framework-components/control/control-migration-types.ts`.
* `migration-tools/src/metadata.ts` becomes a re-export from
`framework-components/control` so existing
`@prisma-next/migration-tools/metadata` consumers (12 files in CLI
+ tests) continue to work unchanged. The arktype runtime schema
that validates `migration.json` stays in `migration-tools/io.ts`
(it is a write-time concern, not a type definition).
Producer-side rename in `migration-tools`:
* writeExtensionMigrationPackage -> writeAuthoredMigrationPackage
(and its test file renamed alongside).
* Drops the lampshaded `MigrationPackageContents` duplicate in
`migration-tools/src/io.ts` and imports the canonical
`AuthoredMigrationPackage` instead.
Consumer-side updates:
* `2-sql/9-family/src/core/migrations/types.ts` drops the three local
interface declarations and uses `AuthoredContractSpace` from
framework-components.
* `2-sql/9-family/src/exports/control.ts` drops the three Extension*
re-exports (no backward-compat shims per repo convention).
* `2-sql/9-family/test/migrations.types.test-d.ts` updates names.
* `test/integration/test/contract-space-fixture/{control,migrations}.ts`
update imports / type names; the fixture continues to specialise
`AuthoredContractSpace<Contract<SqlStorage>>` for its descriptor.
Spec § 1 of `framework-mechanism.spec.md` updated to reflect the new
type names and locations. § 3 / § 6 / § 7 still mention the old names
in narrative prose; surfacing those for orchestrator review (out of
the spec-edit authorisation for this round).
Refs: TML-2397, projects/extension-contract-spaces/reviews/code-review.md F4.
R2 landed F3 + F4 in 9e39382 + 68ebbeb but the spec/plan referenced old type and helper names in narrative prose outside § 1 of the sub-spec (the implementer was scoped to § 1 only). Update spec § 3 (helper-location note + emission-helper paragraph) and § 6 (SpacePathInput.targetRef type annotation) to use the renamed names; ditto plan T1.2 + T1.7 landed- annotations. Mark T-cleanup.3 done with closure narrative covering both findings, including the MigrationMetadata/MigrationHints hoist that R2 needed to make the layering work cleanly. R3 (T-cleanup.4 / F2) remains, gating M1-cleanup SATISFIED.
…shape migration with structured detection (M1-cleanup F2) The marker table moved from a single-row `id` PK to a per-space-row `space` PK during contract-spaces work. Both targets carried a transitional auto-migration helper that promoted legacy databases on every framework boot — appropriate while the schema change was in flight, but a permanent operational surface for a one-time concern past zero-range. Delete the helpers (`migrateMarkerSchemaStatements` on Postgres, `migrateMarkerSchemaSqlite` on SQLite) and replace them with a runtime detection step at runner boot: - `ensureControlTables` now returns `Result<void, SqlMigrationRunnerFailure>` and runs `detectLegacyMarkerShape` before creating the marker table. - New `LEGACY_MARKER_SHAPE` `SqlMigrationRunnerErrorCode` carries a structured failure with the table name in `meta` and a summary that points the operator at the explicit remediation: drop the marker table and re-run `dbInit` from a clean baseline. - Detection inspects `INFORMATION_SCHEMA.COLUMNS` (Postgres) or `PRAGMA table_info` (SQLite) for the absence of the `space` column on an existing marker table; fresh databases (table absent) and already-migrated databases (table present with `space`) both no-op. - The detection step does not mutate the legacy table — operator intervention is the explicit remediation. The `marker-schema-migration` test files (one per target) that exercised the legacy → per-space promotion are deleted; coverage of the detection step lands in each target adapter`s `runner.errors` test file alongside existing `runnerFailure` scenarios. Audit for analogous in-zero-range transitional migrations in the postgres/sqlite control-plane DDL surfaces turned up no other candidates: the ledger table is created with `IF NOT EXISTS` and has no in-place upgrade helper, and no other `migrate*Schema` / promote-shape paths exist in target sources. References: PRRT_kwDOQM0QJc6AnH2D M1-cleanup T-cleanup.4
…ED at 15e0534 3 implementer rounds + 3 reviewer rounds closed all 6 design-quality findings (F0-F5) surfaced in the post-M1 design review. Final commit SHAs: c19086d90 (F0 manifest-presence detection, pre-cleanup-milestone), db33795e3 (F1 test-fixture relocation), 9e39382 (F3 APP_SPACE_ID canonicalisation + lint guard), 68ebbeb (F4 contract-space type hoist + rename), 15e0534 (F2 transitional marker-migration deletion + structured detection). Mark T-cleanup.4 done with closure narrative (audit for analogous in-zero-range transitional migrations across 2-sql + 3-targets came back empty); promote the milestone heading to SATISFIED with the round sequence and AC scoreboard summary preserved for the audit trail. Project plan now stands at M1 + M1-cleanup SATISFIED; M2-M5 remain.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/3-targets/6-adapters/sqlite/src/core/control-adapter.ts (1)
98-128:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftGuard legacy marker tables before selecting on
space.Lines 98-127 only check that
_prisma_markerexists, then immediately queryWHERE space = ?. On pre-cleanup databases that still have the legacy single-row marker shape, this path now throws SQLite's rawno such column: spaceerror instead of surfacing the structuredLEGACY_MARKER_SHAPEremediation you added in the runner. That leaves standalone marker reads in verify/drift paths with an untyped failure mode.At minimum, mirror the runner’s shape probe here before the select, or translate the missing-column case into the same structured remediation flow.
🤖 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/3-targets/6-adapters/sqlite/src/core/control-adapter.ts` around lines 98 - 128, Before running the filtered select on _prisma_marker, probe/guard for the legacy single-row marker shape and map missing-column errors to the same remediation used in the runner: use driver.query to inspect the table schema or a single-row probe (e.g. PRAGMA table_info/_prisma_marker or SELECT * FROM _prisma_marker LIMIT 1) and verify the presence of the space column; if the space column is absent, return/throw the existing LEGACY_MARKER_SHAPE remediation (or call the same remediation helper used by the runner) instead of performing the `SELECT ... WHERE space = ?` with APP_SPACE_ID, and additionally catch a runtime sqlite "no such column: space" error from the driver.query call and translate it into that same structured remediation.
🤖 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/test/write-authored-migration-package.test.ts`:
- Around line 44-65: The test currently verifies deterministic serialization by
writing the package to two different roots (dirA and dirB) but misses overwrite
idempotency; change the test in write-authored-migration-package.test.ts so both
writes target the same output directory (call writeAuthoredMigrationPackage
twice to the same dir, e.g., dirA) to assert a second write over an existing
<targetDir>/<pkg.dirName> produces byte-identical migration.json, ops.json and
contract.json and does not leave stale files; keep existing readFile+expect
comparisons but use the files from the same directory before-and-after the
second write to validate overwrite idempotency for
writeAuthoredMigrationPackage.
In `@packages/2-sql/5-runtime/src/sql-marker.ts`:
- Around line 12-18: Change the optional `space?: string` property on the SQL
marker type to a required `space: string` (remove the defaulting behavior tied
to APP_SPACE_ID) and then update every call site that relied on implicit
defaults (referenced around the other occurrences you flagged) to pass
APP_SPACE_ID explicitly from app-only callers; specifically edit the declaration
for the `space` field in sql-marker.ts and audit references that construct or
read marker objects so they provide a concrete space value instead of omitting
it.
In `@scripts/lint-app-space-id.mjs`:
- Line 67: The global regex LITERAL_RE (const LITERAL_RE = /(['"])app\1/g) is
reused across file tests so its lastIndex carries over and skips matches; before
each file-level test where LITERAL_RE is used (e.g., the check around the code
that runs per-file at line ~110), reset the regex by setting
LITERAL_RE.lastIndex = 0 so each file starts matching from position 0; update
the loop or function that invokes LITERAL_RE to reset lastIndex immediately
before calling test/exec to ensure no matches are missed.
---
Outside diff comments:
In `@packages/3-targets/6-adapters/sqlite/src/core/control-adapter.ts`:
- Around line 98-128: Before running the filtered select on _prisma_marker,
probe/guard for the legacy single-row marker shape and map missing-column errors
to the same remediation used in the runner: use driver.query to inspect the
table schema or a single-row probe (e.g. PRAGMA table_info/_prisma_marker or
SELECT * FROM _prisma_marker LIMIT 1) and verify the presence of the space
column; if the space column is absent, return/throw the existing
LEGACY_MARKER_SHAPE remediation (or call the same remediation helper used by the
runner) instead of performing the `SELECT ... WHERE space = ?` with
APP_SPACE_ID, and additionally catch a runtime sqlite "no such column: space"
error from the driver.query call and translate it into that same structured
remediation.
🪄 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: 7636fac7-0e33-4c19-9f2b-bafdb0a88733
⛔ Files ignored due to path filters (2)
projects/extension-contract-spaces/plan.mdis excluded by!projects/**projects/extension-contract-spaces/specs/framework-mechanism.spec.mdis excluded by!projects/**
📒 Files selected for processing (28)
package.jsonpackages/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/3-tooling/migration/src/exports/io.tspackages/1-framework/3-tooling/migration/src/io.tspackages/1-framework/3-tooling/migration/src/metadata.tspackages/1-framework/3-tooling/migration/src/space-layout.tspackages/1-framework/3-tooling/migration/test/write-authored-migration-package.test.tspackages/2-sql/5-runtime/src/sql-marker.tspackages/2-sql/9-family/src/core/migrations/types.tspackages/2-sql/9-family/test/migrations.types.test-d.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/runner.tspackages/3-targets/3-targets/sqlite/src/core/migrations/statement-builders.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/migrations/runner.errors.integration.test.tspackages/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/runner.errors.test.tsscripts/lint-app-space-id.mjstest/integration/test/contract-space-fixture/control.tstest/integration/test/contract-space-fixture/migrations.ts
✅ Files skipped from review due to trivial changes (3)
- packages/1-framework/3-tooling/migration/src/exports/io.ts
- packages/1-framework/1-core/framework-components/src/exports/control.ts
- test/integration/test/contract-space-fixture/migrations.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/3-targets/3-targets/postgres/src/exports/statement-builders.ts
- packages/3-targets/6-adapters/postgres/src/core/control-adapter.ts
- packages/3-targets/6-adapters/sqlite/test/migrations/runner.errors.test.ts
- test/integration/test/contract-space-fixture/control.ts
- packages/1-framework/3-tooling/migration/src/space-layout.ts
Comment on lines
+44
to
+65
| it('produces byte-identical output across two writes of the same package (idempotency)', async () => { | ||
| const ops = createTestOps(); | ||
| const metadata = createTestMetadata({}, ops); | ||
| const pkg = { dirName: 'baseline', metadata, ops }; | ||
|
|
||
| const dirA = join(tmpDir, 'a'); | ||
| const dirB = join(tmpDir, 'b'); | ||
| await writeAuthoredMigrationPackage(dirA, pkg); | ||
| await writeAuthoredMigrationPackage(dirB, pkg); | ||
|
|
||
| const aManifest = await readFile(join(dirA, pkg.dirName, 'migration.json'), 'utf-8'); | ||
| const bManifest = await readFile(join(dirB, pkg.dirName, 'migration.json'), 'utf-8'); | ||
| expect(aManifest).toBe(bManifest); | ||
|
|
||
| const aOps = await readFile(join(dirA, pkg.dirName, 'ops.json'), 'utf-8'); | ||
| const bOps = await readFile(join(dirB, pkg.dirName, 'ops.json'), 'utf-8'); | ||
| expect(aOps).toBe(bOps); | ||
|
|
||
| const aContract = await readFile(join(dirA, pkg.dirName, 'contract.json'), 'utf-8'); | ||
| const bContract = await readFile(join(dirB, pkg.dirName, 'contract.json'), 'utf-8'); | ||
| expect(aContract).toBe(bContract); | ||
| }); |
There was a problem hiding this comment.
Exercise overwrite idempotency in the same target directory.
Lines 49-52 currently compare two fresh writes in different roots, so this only proves deterministic serialization. It will miss regressions where a second write to an existing <targetDir>/<pkg.dirName> leaves stale files behind or rewrites content differently.
Suggested change
- const dirA = join(tmpDir, 'a');
- const dirB = join(tmpDir, 'b');
- await writeAuthoredMigrationPackage(dirA, pkg);
- await writeAuthoredMigrationPackage(dirB, pkg);
-
- const aManifest = await readFile(join(dirA, pkg.dirName, 'migration.json'), 'utf-8');
- const bManifest = await readFile(join(dirB, pkg.dirName, 'migration.json'), 'utf-8');
- expect(aManifest).toBe(bManifest);
-
- const aOps = await readFile(join(dirA, pkg.dirName, 'ops.json'), 'utf-8');
- const bOps = await readFile(join(dirB, pkg.dirName, 'ops.json'), 'utf-8');
- expect(aOps).toBe(bOps);
-
- const aContract = await readFile(join(dirA, pkg.dirName, 'contract.json'), 'utf-8');
- const bContract = await readFile(join(dirB, pkg.dirName, 'contract.json'), 'utf-8');
- expect(aContract).toBe(bContract);
+ const targetDir = join(tmpDir, 'a');
+ await writeAuthoredMigrationPackage(targetDir, pkg);
+
+ const manifestBefore = await readFile(join(targetDir, pkg.dirName, 'migration.json'), 'utf-8');
+ const opsBefore = await readFile(join(targetDir, pkg.dirName, 'ops.json'), 'utf-8');
+ const contractBefore = await readFile(join(targetDir, pkg.dirName, 'contract.json'), 'utf-8');
+
+ await writeAuthoredMigrationPackage(targetDir, pkg);
+
+ const manifestAfter = await readFile(join(targetDir, pkg.dirName, 'migration.json'), 'utf-8');
+ const opsAfter = await readFile(join(targetDir, pkg.dirName, 'ops.json'), 'utf-8');
+ const contractAfter = await readFile(join(targetDir, pkg.dirName, 'contract.json'), 'utf-8');
+
+ expect(manifestAfter).toBe(manifestBefore);
+ expect(opsAfter).toBe(opsBefore);
+ expect(contractAfter).toBe(contractBefore);🤖 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/write-authored-migration-package.test.ts`
around lines 44 - 65, The test currently verifies deterministic serialization by
writing the package to two different roots (dirA and dirB) but misses overwrite
idempotency; change the test in write-authored-migration-package.test.ts so both
writes target the same output directory (call writeAuthoredMigrationPackage
twice to the same dir, e.g., dirA) to assert a second write over an existing
<targetDir>/<pkg.dirName> produces byte-identical migration.json, ops.json and
contract.json and does not leave stale files; keep existing readFile+expect
comparisons but use the files from the same directory before-and-after the
second write to validate overwrite idempotency for
writeAuthoredMigrationPackage.
Comment on lines
+12
to
+18
| /** | ||
| * Logical space identifier for this marker row. Defaults to | ||
| * {@link APP_SPACE_ID} (`'app'`) so existing single-app callers keep | ||
| * working without modification; per-space callers pass their space id | ||
| * explicitly. | ||
| */ | ||
| readonly space?: string; |
There was a problem hiding this comment.
Make space explicit instead of defaulting to APP_SPACE_ID.
These fallbacks turn any missed per-space plumbing into a silent read/write against the app marker, which is exactly the class of bug this migration should expose. Requiring space here and passing APP_SPACE_ID explicitly from app-only call sites would fail fast instead of mutating the wrong row.
Suggested direction
export interface WriteMarkerInput {
- readonly space?: string;
+ readonly space: string;
readonly storageHash: string;
...
}
-export function readContractMarker(space: string = APP_SPACE_ID): MarkerStatement {
+export function readContractMarker(space: string): MarkerStatement {
return {
...
params: [space],
};
}
export const ensureTableStatement: SqlStatement = {
sql: `create table if not exists prisma_contract.marker (
- space text not null primary key default '${APP_SPACE_ID}',
+ space text not null primary key,
...
)`,
params: [],
};
export function writeContractMarker(input: WriteMarkerInput): WriteContractMarkerStatements {
...
- const params: readonly unknown[] = [input.space ?? APP_SPACE_ID, ...placed.map((c) => c.param)];
+ const params: readonly unknown[] = [input.space, ...placed.map((c) => c.param)];As per coding guidelines: "Do not add backward-compatibility shims, migration scaffolding, deprecation warnings, or comments explaining legacy vs. new approaches" and "Update all references immediately to use new approaches instead of maintaining parallel implementations."
Also applies to: 54-54, 67-67, 113-121
🤖 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/5-runtime/src/sql-marker.ts` around lines 12 - 18, Change the
optional `space?: string` property on the SQL marker type to a required `space:
string` (remove the defaulting behavior tied to APP_SPACE_ID) and then update
every call site that relied on implicit defaults (referenced around the other
occurrences you flagged) to pass APP_SPACE_ID explicitly from app-only callers;
specifically edit the declaration for the `space` field in sql-marker.ts and
audit references that construct or read marker objects so they provide a
concrete space value instead of omitting it.
…prefix removal) R3 reached SATISFIED at 15e0534 but interactive design-review of R2s output surfaced F6: the Authored* prefix on the relocated types implies a structural distinction between contract spaces that doesnt exist (all spaces are equal at the type level — the app/extension designation lives only in the orchestrator), and AuthoredMigrationPackage forks the type hierarchy from the pre-existing on-disk MigrationPackage rather than extending it. Add T-cleanup.5 with the rename plan: ContractSpace<TContract> (canonical, no prefix); MigrationPackage (canonical structural shape, in framework-components/control); OnDiskMigrationPackage extends MigrationPackage (in migration-tools/src/package.ts; adds dirPath); materialiseMigrationPackage (renamed from writeAuthoredMigrationPackage, not deleted — M3 consumes it). Add AC-D6. Flip milestone heading SATISFIED → REOPENED for R4. R4 is the rename pass; M3 (cipherstash) absorbs the import renames on its next rebase.
…e typology (M1-cleanup F6) Drop the `Authored` prefix from the contract-space types so the canonical structural shape is `ContractSpace<TContract>` / `MigrationPackage` (in `framework-components/control`). The on-disk augmented form lives in `migration-tools/src/package.ts` as `OnDiskMigrationPackage extends MigrationPackage` (adds `dirPath`). Whether a value is the application's space or an extension's space is a control-plane concern — the type carries no such distinction, and the prior `Authored*` prefix wrongly implied a structural one. After this rename, app-space and extension-space pipelines consume the same shape; readers (`readMigrationPackage`, `readMigrationsDir`) return `OnDiskMigrationPackage`; the framework's pinned-artefact emitter takes the canonical `MigrationPackage`. `writeAuthoredMigrationPackage` is renamed to `materialiseMigrationPackage` — distinct verb from the lower-level constituent-taking `writeMigrationPackage(dir, metadata, ops)` so the struct-vs-constituents semantic difference stays visible at the call site. The helper is renamed (not deleted) because the M3 cipherstash branch consumes it; M3 absorbs the rename on its next rebase. Hash and graph helpers (`verifyMigrationHash`, `reconstructGraph`) take `OnDiskMigrationPackage` — `reconstructGraph` reads `dirPath` for diagnostics and the readers'-side callers always have the augmented form already. The barrel `migration-tools/src/exports/package.ts` re-exports both `MigrationPackage` (from framework-components) and `OnDiskMigrationPackage` for ergonomic access at consumption sites. Spec § 1 updated to track the renames; § 3 / § 7 references to `writeAuthoredMigrationPackage` left for the orchestrator (mirroring the R2 split). No GH thread to close — F6 surfaced in interactive review. AC-D6: `rg "Authored(ContractSpace|MigrationPackage)|writeAuthoredMigrationPackage" packages/ test/` returns no matches.
…ps (M1-cleanup R4 SATISFIED-candidate) Implementer R4 landed F6 cleanly at f8649ba. Orchestrator close-ups: - plan.md: T-cleanup.5 marked done with commit SHA + rename map preserved as audit trail; milestone heading flipped REOPENED -> SATISFIED-candidate; R4 outcome added; AC scoreboard candidate 7/7 pending reviewer. - spec.md S 3 helper-location prose: writeAuthoredMigrationPackage -> materialiseMigrationPackage with full rename audit trail. - spec.md S 7 (line 167) emission-helper detail: same rename + sentence on the verb-distinction from writeMigrationPackage(dir, metadata, ops) to keep the snapshot-vs-no-snapshot semantic visible at the call site. S 1 was updated by the implementer in the prior commit per the round split. Reviewer R4 still to verify before flipping milestone to SATISFIED.
…osed) Reviewer R4 verdict SATISFIED. The contract-space typology flatten (F6) closes cleanly: canonical ContractSpace<TContract> and MigrationPackage in framework-components/control; OnDiskMigrationPackage extends MigrationPackage in migration-tools for the deserialised-from-disk variant; materialiseMigrationPackage replaces writeAuthoredMigrationPackage (renamed, not deleted — M3 cipherstash branch consumes it on its next rebase). Trajectory: SATISFIED at 15e0534 post-R3 -> reopened at a9697ba for F6 -> re-SATISFIED at ac2157d post-R4. 7/7 ACs PASS, 15/15 GH PR threads closed, validation gates green. Reviewer surfaced one observational item (recorded in code-review.md, not a finding): design-quality vocabulary findings have systematically surfaced post-implementation in this milestone (F0–F6 cycle pattern). A pre-implementation surface-vocabulary pass on M2/M3/M4/M5 would catch this class earlier. Out of scope for R4; carried forward to next-milestone intake.
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
Milestone 1 of TML-2397 — Contract spaces: first-class schema contributions from extensions and monorepo packages. Establishes the framework primitives for contract spaces — disjoint
(contract.json, migration-graph, head-ref)units owned by extensions or monorepo packages — so extensions become first-class schema contributors using the same planner / runner / verifier / migration shape as application authoring.This PR is the foundation. M2-M5 (later PRs in this stack) consume this surface to ship per-space
db init/db update, the cipherstash and pgvector migrations to the new mechanism, and removal of the legacydatabaseDependencies.initescape hatch.What lands
Project shaping (4 commits)
projects/extension-contract-spaces/spec.md) refined through design discussion.node_modules).framework-mechanism.spec.md(drives M1+M2) andcipherstash-migration.spec.md(drives M3).databaseDependencies); 5 milestones.Framework mechanism (10 implementation commits + 6 doc / fix commits)
prisma_contract.markergains aspacecolumn; one-shot framework-internal migration promotes existing single-row marker to(space='app', …)packages/2-sql/4-framework/contractSpacedescriptor fieldSqlControlExtensionDescriptor.contractSpace?: { contractJson, migrations, headRef }packages/2-sql/9-family/src/core/migrations/types.tsplanAllSpaces({ spaces })— target-agnostic primitive in@prisma-next/migration-tools/exports/spacesconcatenateSpaceApplyInputs— extensions alphabetical, then app-space; single transactionverifyContractSpaces— strict matching of loaded extension contract spaces vs marker rows + on-disk pinned dirsmigrations/<space-id>/{contract.json, contract.d.ts, refs/head.json, <migration-name>/...};emitPinnedSpaceArtefacts+writeExtensionMigrationPackagedetectSpaceContractDrift,readPinnedHeadRef,gatherDiskContractSpaceStatepackages/3-extensions/test-contract-space/— private workspace package exercising the contract-space machinery end-to-end at the helper layerPlus: 3 fixes from review rounds (F1 brand-helper conversion, F2 stale mock SQL, F3 spy-not-called assertion).
Architecture
Two architectural principles are enforced throughout:
migrations/<space-id>/), not just referenced vianode_modules. The verifier and runner read on-disk pinned artefacts at apply time — descriptor imports are authoring-time only.space-id), then app-space migrations, all in a single transaction. Multi-space rollback is a property of the runner (M2).All M1 helpers are framework-neutral primitives in
1-framework/packages; per-target composition lives in the SQL family's consumption sites (M2).Project artefacts
projects/extension-contract-spaces/spec.mdprojects/extension-contract-spaces/specs/framework-mechanism.spec.mdprojects/extension-contract-spaces/specs/cipherstash-migration.spec.mdprojects/extension-contract-spaces/plan.md(The project artefacts under
projects/are transient and will be migrated todocs/architecture docs/in M5 close-out.)Stacked PR series
This is the first of a stack:
1-framework/packages.db init/db update(CLI consumer wiring; SQL-family runner restructure for multi-space outer-tx).databaseDependenciesremoval + ADRs + close-out.Verification
pnpm lint:depsgreen (no layering violations).pnpm test:packagesgreen for all M1-touched packages.3fe5a7637; close-out doc-pass at612153654.Test plan
pnpm test:packages.pnpm test:integrationbeyond the documented baseline (parallel resource contention withmongodb-memory-server).Refs: TML-2397.
Summary by CodeRabbit
New Features
Bug Fixes