TML-2536: route on-disk contract reads through ContractSerializer + strict deserializer (v2)#533
Merged
Merged
Conversation
5 tasks
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRoutes on-disk contract reads through familyInstance.deserializeContract; enforces explicit ChangesContract deserialization & storage-type normalization
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
@prisma-next/extension-author-tools
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/extension-cipherstash
@prisma-next/middleware-cache
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/extension-postgis
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/emitter
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
packages/1-framework/3-tooling/cli/src/commands/migrate.ts (1)
176-177: ⚡ Quick winKeep the read-site boundary explicit with
as unknown.This now crosses the serializer seam correctly, but the repo pattern for on-disk reads is
validateContract(JSON.parse(raw) as unknown). Keeping the parsed payload asunknownhere avoids silently reintroducing an untyped boundary at the exact seam this PR is tightening.Suggested fix
- contractRaw = familyInstance.validateContract(JSON.parse(contractContent)); + contractRaw = familyInstance.validateContract(JSON.parse(contractContent) as unknown);As per coding guidelines
packages/**/src/**/*.ts: "All on-disk contract reads must route through the seam: callfamilyInstance.validateContract(JSON.parse(raw) as unknown)instead of castingJSON.parse(...) as Contract".🤖 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/migrate.ts` around lines 176 - 177, The JSON parse at the on-disk boundary is missing the explicit unknown cast; update the call that constructs contractRaw so the parsed payload is treated as unknown before validation: pass JSON.parse(contractContent) as unknown into familyInstance.validateContract (i.e., use the unknown cast at the seam where contractContent is parsed) so the code reads through the serializer seam rather than reintroducing an untyped boundary.packages/1-framework/3-tooling/cli/src/commands/migration-new.ts (1)
99-101: ⚡ Quick winUse the repo’s explicit
unknownseam pattern here too.The command now validates at the read site, which is the right fix. It should still keep the parsed JSON at
unknownwhen crossing that seam so this path matches the workspace rule used elsewhere.Suggested fix
- toContract = familyInstance.validateContract(JSON.parse(contractJsonContent)); + toContract = familyInstance.validateContract(JSON.parse(contractJsonContent) as unknown);As per coding guidelines
packages/**/src/**/*.ts: "All on-disk contract reads must route through the seam: callfamilyInstance.validateContract(JSON.parse(raw) as unknown)instead of castingJSON.parse(...) as Contract".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/1-framework/3-tooling/cli/src/commands/migration-new.ts` around lines 99 - 101, The code currently parses JSON directly into a typed value before validation; instead pass the parse result through the repo's unknown seam: call familyInstance.validateContract(JSON.parse(contractJsonContent) as unknown) to produce toContract. Update the call site referencing familyInstance.validateContract, the toContract assignment, and the contractJsonContent parse so the raw read is cast to unknown before validation.packages/1-framework/3-tooling/cli/src/commands/migration-show.ts (1)
379-381: ⚡ Quick winPreserve the explicit
unknownboundary on the app-contract read.The seam is now in the right place. One small follow-up: keep
JSON.parse(...)wrapped asunknownbefore passing it tovalidateContract(...)so this read site follows the repo’s standard deserialization pattern.Suggested fix
- appContract = familyInstance.validateContract(JSON.parse(contractJsonContent)); + appContract = familyInstance.validateContract(JSON.parse(contractJsonContent) as unknown);As per coding guidelines
packages/**/src/**/*.ts: "All on-disk contract reads must route through the seam: callfamilyInstance.validateContract(JSON.parse(raw) as unknown)instead of castingJSON.parse(...) as Contract".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/1-framework/3-tooling/cli/src/commands/migration-show.ts` around lines 379 - 381, The call to familyInstance.validateContract currently passes JSON.parse(contractJsonContent) directly; wrap the parsed value with the explicit unknown seam before validation (i.e., pass JSON.parse(contractJsonContent) as unknown into familyInstance.validateContract) so appContract is produced via the repository's standard deserialization pattern used by validateContract.packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts (1)
99-100: ⚡ Quick winApply the explicit
unknownboundary at both contract read sites.This file now routes both on-disk contracts through the serializer seam, which is good. To fully match the repo contract-read pattern, both parsed payloads should cross that seam as
unknownrather than as an implicit untyped value.Suggested fix
- return validateContract(JSON.parse(raw)); + return validateContract(JSON.parse(raw) as unknown);- toContract = familyInstance.validateContract(JSON.parse(contractJsonContent)); + toContract = familyInstance.validateContract(JSON.parse(contractJsonContent) as unknown);As per coding guidelines
packages/**/src/**/*.ts: "All on-disk contract reads must route through the seam: callfamilyInstance.validateContract(JSON.parse(raw) as unknown)instead of castingJSON.parse(...) as Contract".Also applies to: 216-218
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts` around lines 99 - 100, The parsed JSON is being passed as an untyped value; change both contract read sites to cross the serializer seam with an explicit unknown boundary by casting the JSON.parse payload to unknown before validating—e.g. replace validateContract(JSON.parse(raw)) with validateContract(JSON.parse(raw) as unknown) (and similarly where familyInstance.validateContract(JSON.parse(raw)) appears around the later block) so both reads use the explicit unknown boundary..cursor/rules/contract-normalization-responsibilities.mdc (1)
1-120: 💤 Low valueConsider extracting the diagram to keep the rule file under the soft limit.
The file is at 120 lines, exceeding the 100-line soft limit for rulecard files. The 32-line ASCII diagram (lines 15-46) is valuable but could be moved to a durable location under
docs/architecture docs/and linked from this rule, per the guideline that says "move long examples todocs/". This would bring the rule file back under the soft limit while keeping the diagram accessible.As per coding guidelines: "Keep rulecard files under 100 lines (soft limit) and never exceed 200 lines (hard limit); move long examples to
docs/"🤖 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 @.cursor/rules/contract-normalization-responsibilities.mdc around lines 1 - 120, The rule file "Contract normalization responsibilities" exceeds the 100-line soft limit because of the large ASCII diagram; extract that diagram block (the on-disk JSON → familyInstance.validateContract → target.contractSerializer.deserializeContract flow) into a new durable doc under docs/architecture (e.g., "contract-serialization-seam.md"), paste the diagram there with any needed caption, then replace the diagram in the rule with a short one-line pointer/link and a brief sentence referencing the moved diagram (and ensure the rule stays under 100 lines); update any internal references that cite the diagram to the new docs/architecture location.
🤖 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
`@skills/extension-author/prisma-next-extension-upgrade/upgrades/0.9-to-0.10/stamp-storage-types-kind.ts`:
- Around line 99-115: The traversal currently collects any CONTRACT_FILES found
anywhere (in walk), which can rewrite non-migration fixtures; limit collection
to files that live under a migrations ancestor by checking the directory
ancestry before pushing to out. Update the walk function (and its call sites if
needed) so that when you encounter a file (the branch using CONTRACT_FILES and
out.push) you first verify the current dir path contains a directory named
"migrations" (e.g., by checking dir path segments or carrying an inMigrations
flag during recursion) and only push join(dir, entry.name) when that check
passes; keep SKIP_DIRS logic and other behavior unchanged.
- Line 80: The import uses join from 'node:path' which violates the TypeScript
guideline; replace the import to use 'pathe' instead and update any references
to the join symbol accordingly (change the import statement that currently
imports join from 'node:path' to import join from 'pathe' so the existing usages
of join in this file—e.g., in functions or constants that build paths—use
pathe's cross-platform implementation).
In `@skills/upgrade/prisma-next-upgrade/upgrades/0.9-to-0.10/instructions.md`:
- Line 77: Update the serializer description to reflect the actual codemod
behavior: replace the claim that it uses JSON.stringify(value, null, 2) + '\n'
with a note that the script uses a custom formatter which inlines short
primitive arrays while otherwise pretty-printing (matching the codemod's
serializer implementation), so that readers understand the inlining behavior and
won't expect a strict JSON.stringify output.
In
`@skills/upgrade/prisma-next-upgrade/upgrades/0.9-to-0.10/stamp-storage-types-kind.ts`:
- Around line 99-115: The walk function currently adds any CONTRACT_FILES found
anywhere; restrict rewrites to only snapshot files under a migrations/ ancestor
by threading an inMigrations boolean through the recursion: change walk(dir:
string) to walk(dir: string, inMigrations = false), set nextInMigrations =
inMigrations || entry.name === 'migrations' before recursing, and only push
join(dir, entry.name) to out when entry.isFile() &&
CONTRACT_FILES.has(entry.name) && inMigrations is true; update the initial call
to walk(root) accordingly and preserve SKIP_DIRS handling.
- Line 80: The file currently imports join from 'node:path' but project TS
tooling expects 'pathe'; update the import to pull join from 'pathe' and keep
all usages of join (seen around the calls that construct paths on the lines
where join is used) unchanged — replace the import statement that references
'node:path' with an import from 'pathe' so the codemod uses the repo-standard
path library.
---
Nitpick comments:
In @.cursor/rules/contract-normalization-responsibilities.mdc:
- Around line 1-120: The rule file "Contract normalization responsibilities"
exceeds the 100-line soft limit because of the large ASCII diagram; extract that
diagram block (the on-disk JSON → familyInstance.validateContract →
target.contractSerializer.deserializeContract flow) into a new durable doc under
docs/architecture (e.g., "contract-serialization-seam.md"), paste the diagram
there with any needed caption, then replace the diagram in the rule with a short
one-line pointer/link and a brief sentence referencing the moved diagram (and
ensure the rule stays under 100 lines); update any internal references that cite
the diagram to the new docs/architecture location.
In `@packages/1-framework/3-tooling/cli/src/commands/migrate.ts`:
- Around line 176-177: The JSON parse at the on-disk boundary is missing the
explicit unknown cast; update the call that constructs contractRaw so the parsed
payload is treated as unknown before validation: pass
JSON.parse(contractContent) as unknown into familyInstance.validateContract
(i.e., use the unknown cast at the seam where contractContent is parsed) so the
code reads through the serializer seam rather than reintroducing an untyped
boundary.
In `@packages/1-framework/3-tooling/cli/src/commands/migration-new.ts`:
- Around line 99-101: The code currently parses JSON directly into a typed value
before validation; instead pass the parse result through the repo's unknown
seam: call familyInstance.validateContract(JSON.parse(contractJsonContent) as
unknown) to produce toContract. Update the call site referencing
familyInstance.validateContract, the toContract assignment, and the
contractJsonContent parse so the raw read is cast to unknown before validation.
In `@packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts`:
- Around line 99-100: The parsed JSON is being passed as an untyped value;
change both contract read sites to cross the serializer seam with an explicit
unknown boundary by casting the JSON.parse payload to unknown before
validating—e.g. replace validateContract(JSON.parse(raw)) with
validateContract(JSON.parse(raw) as unknown) (and similarly where
familyInstance.validateContract(JSON.parse(raw)) appears around the later block)
so both reads use the explicit unknown boundary.
In `@packages/1-framework/3-tooling/cli/src/commands/migration-show.ts`:
- Around line 379-381: The call to familyInstance.validateContract currently
passes JSON.parse(contractJsonContent) directly; wrap the parsed value with the
explicit unknown seam before validation (i.e., pass
JSON.parse(contractJsonContent) as unknown into familyInstance.validateContract)
so appContract is produced via the repository's standard deserialization pattern
used by validateContract.
🪄 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: 8d75736c-2433-42eb-92e8-b8d639bd6a2a
⛔ Files ignored due to path filters (3)
projects/tml-2536-contract-deserializer-seam/manual-qa.mdis excluded by!projects/**projects/tml-2536-contract-deserializer-seam/plans/plan.mdis excluded by!projects/**projects/tml-2536-contract-deserializer-seam/spec.mdis excluded by!projects/**
📒 Files selected for processing (51)
.cursor/rules/as-contract-cast-smell.mdc.cursor/rules/contract-normalization-responsibilities.mdc.cursor/rules/validate-contract-usage.mdc.github/workflows/ci.ymldocs/architecture docs/subsystems/7. Migration System.mdexamples/prisma-next-demo/migrations/app/20260422T0720_initial/end-contract.jsonexamples/prisma-next-demo/migrations/app/20260422T0742_migration/end-contract.jsonexamples/prisma-next-demo/migrations/app/20260422T0742_migration/start-contract.jsonexamples/prisma-next-demo/migrations/app/20260422T0748_migration/end-contract.jsonexamples/prisma-next-demo/migrations/app/20260422T0748_migration/start-contract.jsonexamples/prisma-next-demo/migrations/app/20260518T1701_namespaces_bookend/end-contract.d.tsexamples/prisma-next-demo/migrations/app/20260518T1701_namespaces_bookend/end-contract.jsonexamples/prisma-next-demo/migrations/app/20260518T1701_namespaces_bookend/migration.jsonexamples/prisma-next-demo/migrations/app/20260518T1701_namespaces_bookend/migration.tsexamples/prisma-next-demo/migrations/app/20260518T1701_namespaces_bookend/ops.jsonexamples/prisma-next-demo/migrations/app/20260518T1701_namespaces_bookend/start-contract.d.tsexamples/prisma-next-demo/migrations/app/20260518T1701_namespaces_bookend/start-contract.jsonexamples/prisma-next-demo/package.jsonexamples/prisma-next-demo/scripts/check-migration-plan.tspackage.jsonpackages/1-framework/0-foundation/contract/src/hashing.tspackages/1-framework/3-tooling/cli/src/commands/db-verify.tspackages/1-framework/3-tooling/cli/src/commands/migrate.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/control-api/client.tspackages/1-framework/3-tooling/cli/src/control-api/operations/contract-emit.tspackages/1-framework/3-tooling/cli/src/load-ts-contract.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/migration/src/aggregate/strategies/graph-walk.tspackages/1-framework/3-tooling/migration/src/aggregate/strategies/synth.tspackages/2-sql/1-core/contract/src/ir/sql-storage.tspackages/2-sql/1-core/contract/test/sql-storage.test.tspackages/2-sql/4-lanes/query-builder/src/root.tspackages/2-sql/5-runtime/test/sql-context.codec-context.test.tspackages/3-extensions/pgvector/test/migrations/planner.contract-to-schema-ir.test.tspackages/3-mongo-target/1-mongo-target/src/core/control-target.tspackages/3-targets/3-targets/postgres/src/exports/control.tspackages/3-targets/3-targets/postgres/test/fixtures/snapshot-read-shapes/codec-instance.jsonpackages/3-targets/3-targets/postgres/test/fixtures/snapshot-read-shapes/postgres-enum.jsonpackages/3-targets/3-targets/postgres/test/snapshot-read-shapes.test.tspackages/3-targets/3-targets/sqlite/src/core/control-target.tsscripts/lint-no-contract-cast.mjsskills-contrib/drive-code-review/SKILL.mdskills-contrib/drive-pr-local-review/SKILL.mdskills/extension-author/prisma-next-extension-upgrade/upgrades/0.9-to-0.10/instructions.mdskills/extension-author/prisma-next-extension-upgrade/upgrades/0.9-to-0.10/stamp-storage-types-kind.tsskills/upgrade/prisma-next-upgrade/upgrades/0.9-to-0.10/instructions.mdskills/upgrade/prisma-next-upgrade/upgrades/0.9-to-0.10/stamp-storage-types-kind.ts
wmadden
added a commit
that referenced
this pull request
May 19, 2026
Re-apply the drive-qa-plan skill against the shipped state of PR #533 (branch `tml-2536-contract-deserializer-seam-v2`) — the previous draft was written in D4 against the now-superseded PR #520 and predated D5 (demo CI gate, lint guard, namespaces-bookend migration) and D6 (rationale comments). What changed: - Header now points at PR #533 and frames both audiences (CLI users + extension authors) up-front; the implementation-history bullet list in "What this script is testing" mentions the shipped guardrails by their concrete names (`lint:no-contract-cast`, the `Demo `migration plan` (no-op)` CI job, `stamp-storage-types-kind.ts`). - Added an Isolation column to the TOC and an explicit `Isolation` line to every scenario (and the exploratory charter), tagging each with the strictest tag it actually needs: `workspace` for DB / tree mutators, `read-only` for the rule-coherence pass, `external` for the `gh pr checks` scenario. - Added "Preconditions: no prerequisite scenarios" / "Scenario N completed" lines so the runner has explicit DAG edges (Scenario 2 depends on 1; Scenario 8 depends on 1 or 3; everything else is independent). - Stripped every severity pre-classification (🛑 Blocker /⚠️ High / 📝 Follow-up) from failure-mode lists in Scenarios 9, 10, 11. The script now enumerates failure-mode categories only; severity belongs in the runner report. - Updated Scenario 4 (negative control for the strict deserializer) to target `migrations/app/20260518T1701_namespaces_bookend/end-contract.json` — the head migration that the planner now reads as the predecessor snapshot after D5's bookend addition. The old target (`20260422T0720_initial`) no longer exercises the predecessor-read path on its own. - Updated Scenario 3 to apply the *full* checked-in history (initial plus bookend) and `migration show` both, so the re-enactment touches the literal pre-bug artefact and the new head. - Updated Scenario 7 to reference PR #533 instead of #520 and to name the actual CI job display name (`Demo `migration plan` (no-op)`). - Updated Scenario 10's oracle and step 8 expectations: the demo `migration plan` is now a *clean* no-op end-to-end (D5's namespaces bookend closed the residual TML-2521 drift), so the prior "you may still see PN-CLI-4020" carve-out is gone — any non-zero / non-no-op envelope is now a finding. - Pre-flight now uses the workspace-canonical CLI invocation (`node ../../packages/.../cli.js`) the rest of the script already uses, so the demo-rooted scenarios all read the same way. - "Scenarios deliberately not in this script" now explicitly excludes the static-pass half of AC-2 / AC-8 (re-running the lint over a clean tree) and AC-11; the sign-off coverage map maps AC-2 to Scenario 5 (the gate-of-gate negative control) and points the AC-11 row at the exclusion table. No scenarios were deleted under the litmus test — every existing scenario already fell into one of the six legitimate buckets. Each scenario now names which bucket it falls under so the runner can quickly verify the script's honesty. Scope is script-revision only: no spec / plan / source / CI / skill changes. The runner skill (`drive-qa-run`) drives the script and owns the report.
wmadden
added a commit
that referenced
this pull request
May 19, 2026
Manual-QA pass against PR #533 surfaced four rough-edges in the prisma-next-demo. Mirrored locally per the convention established by `examples/prisma-next-postgis-demo/gotchas.md` (each entry references its upstream Linear ticket in the [PN] Gotchas project). Entries: - TML-2495 (pre-existing): `db init` PN-MIG-5001 declaredButUnmigrated for extension spaces unless `migration plan` is run first. Same bug class the postgis demo already filed; reproduces here against pgvector. - TML-2564 (new): `migration status` reports `never-planned` after `db init` even when markerHash === headHash. Status taxonomy categorises by how the marker was set, not by sync state. - TML-2565 (new): `migration show` (no argument) silently means `migration heads`. Overloaded verb; targetless behaviour reads src/prisma/contract.json and enumerates per-space leaf migrations, which is unrelated to the targeted behaviour's per-migration inspection. - TML-2566 (new, F-7 system bug): Loaded predecessor contract snapshots are not verified against their persisted storageHash at load time. Semantically-inconsistent end-contract.json passes `migration plan` silently. Follow-on from TML-2536's deserializer-seam closure: the load happens; the post-load hash-recomputation-and-compare step does not. All three new tickets reference TML-2536's manual-QA report at `projects/tml-2536-contract-deserializer-seam/manual-qa-reports/2026-05-18-claude-opus-4-7.md` for repro artefacts. Source-of-truth for these entries is the Linear tickets, not this file; this file is the in-repo mirror so a reader bumping into one of these behaviours has a local pointer. Signed-off-by: Will Madden <madden@prisma.io>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/prisma-next-demo/gotchas.md`:
- Around line 29-33: The fenced code block that starts with "PN-MIG-5001 —
Contract-space layout violation" is missing a language identifier and triggers
markdownlint MD040; update the opening fence from ``` to a language-tagged fence
such as ```text or ```console so the block is treated as plain CLI output
(ensure the block still contains the same lines: "PN-MIG-5001 — Contract-space
layout violation", "[declaredButUnmigrated] pgvector", and the pgvector
message).
🪄 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: f066f465-8d2f-4098-ae4c-b197d4737680
⛔ Files ignored due to path filters (7)
projects/tml-2536-contract-deserializer-seam/manual-qa-reports/2026-05-18-claude-opus-4-7.mdis excluded by!projects/**projects/tml-2536-contract-deserializer-seam/manual-qa-reports/artefacts/F-1/migration-show-on-corrupted-end-contract.txtis excluded by!projects/**projects/tml-2536-contract-deserializer-seam/manual-qa-reports/artefacts/F-7/scenario-9-probes.txtis excluded by!projects/**projects/tml-2536-contract-deserializer-seam/manual-qa-reports/artefacts/F-8/scenario-9-probes.txtis excluded by!projects/**projects/tml-2536-contract-deserializer-seam/manual-qa-reports/artefacts/F-9/scenario-10-codemod-format-divergence.txtis excluded by!projects/**projects/tml-2536-contract-deserializer-seam/manual-qa-reports/artefacts/scenario-9-probes.txtis excluded by!projects/**projects/tml-2536-contract-deserializer-seam/manual-qa.mdis excluded by!projects/**
📒 Files selected for processing (2)
examples/prisma-next-demo/gotchas.mdskills/extension-author/prisma-next-extension-upgrade/upgrades/0.9-to-0.10/instructions.md
wmadden
added a commit
that referenced
this pull request
May 19, 2026
Every CLI on-disk contract read now passes the parsed payload as `unknown` into `validateContract`, matching the documented seam pattern (`familyInstance.validateContract(JSON.parse(raw) as unknown)`) in `.cursor/rules/contract-normalization-responsibilities.mdc`. This keeps the read boundary at the serializer seam explicit and avoids silently reintroducing an untyped step inside the validator call. Touches the five read sites flagged by CodeRabbit on PR #533: - `migrate.ts` (contractRaw) - `migration-new.ts` (toContract) - `migration-show.ts` (appContract) - `migration-plan.ts` (validateContract(raw) and familyInstance.validateContract(contractJsonContent))
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…btree The codemod claimed (in the colocated instructions) to "walk the project root [and] descend into every directory named migrations/", but the actual walk recursed into every non-skipped subtree and stamped any `start-contract.json` / `end-contract.json` it found. That could unintentionally rewrite non-migration fixtures (e.g. inline contract test snapshots) in downstream user projects. Thread an `inMigrations` flag through `walk(...)` so: - the recursion still visits every non-skipped subtree (we cannot short-circuit because `migrations/` need not sit at any fixed depth); - but a CONTRACT_FILES match is only collected once we have crossed (or are below) a directory literally named `migrations`. The fix is applied identically to both copies of the codemod (user upgrade skill + extension-author upgrade skill) so they remain byte-identical, per the existing convention. Flagged by CodeRabbit on PR #533.
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…uctions The user-facing instructions claimed the codemod re-serialises with `JSON.stringify(value, null, 2) + '\n'`, but the script actually uses a custom pretty-printer (`formatJson`) that inlines short primitive arrays while otherwise matching `JSON.stringify(null, 2)` indentation. Aligning the prose with the shipped behaviour matches the extension-author copy and prevents misleading diff expectations. Flagged by CodeRabbit on PR #533.
Contributor
Author
|
Follow-ups on CodeRabbit's review body (the five nitpicks bundled in the review summary; not on individual review threads): Addressed:
All five now follow the documented seam pattern ( Not addressing in this PR:
|
wmadden
added a commit
that referenced
this pull request
May 19, 2026
Every CLI on-disk contract read now passes the parsed payload as `unknown` into `validateContract`, matching the documented seam pattern (`familyInstance.validateContract(JSON.parse(raw) as unknown)`) in `.cursor/rules/contract-normalization-responsibilities.mdc`. This keeps the read boundary at the serializer seam explicit and avoids silently reintroducing an untyped step inside the validator call. Touches the five read sites flagged by CodeRabbit on PR #533: - `migrate.ts` (contractRaw) - `migration-new.ts` (toContract) - `migration-show.ts` (appContract) - `migration-plan.ts` (validateContract(raw) and familyInstance.validateContract(contractJsonContent)) Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…btree The codemod claimed (in the colocated instructions) to "walk the project root [and] descend into every directory named migrations/", but the actual walk recursed into every non-skipped subtree and stamped any `start-contract.json` / `end-contract.json` it found. That could unintentionally rewrite non-migration fixtures (e.g. inline contract test snapshots) in downstream user projects. Thread an `inMigrations` flag through `walk(...)` so: - the recursion still visits every non-skipped subtree (we cannot short-circuit because `migrations/` need not sit at any fixed depth); - but a CONTRACT_FILES match is only collected once we have crossed (or are below) a directory literally named `migrations`. The fix is applied identically to both copies of the codemod (user upgrade skill + extension-author upgrade skill) so they remain byte-identical, per the existing convention. Flagged by CodeRabbit on PR #533. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…uctions The user-facing instructions claimed the codemod re-serialises with `JSON.stringify(value, null, 2) + '\n'`, but the script actually uses a custom pretty-printer (`formatJson`) that inlines short primitive arrays while otherwise matching `JSON.stringify(null, 2)` indentation. Aligning the prose with the shipped behaviour matches the extension-author copy and prevents misleading diff expectations. Flagged by CodeRabbit on PR #533. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
force-pushed
the
tml-2536-contract-deserializer-seam-v2
branch
from
a7a7049 to
bf0e7a7
Compare
wmadden
added a commit
that referenced
this pull request
May 19, 2026
Re-apply the drive-qa-plan skill against the shipped state of PR #533 (branch `tml-2536-contract-deserializer-seam-v2`) — the previous draft was written in D4 against the now-superseded PR #520 and predated D5 (demo CI gate, lint guard, namespaces-bookend migration) and D6 (rationale comments). What changed: - Header now points at PR #533 and frames both audiences (CLI users + extension authors) up-front; the implementation-history bullet list in "What this script is testing" mentions the shipped guardrails by their concrete names (`lint:no-contract-cast`, the `Demo `migration plan` (no-op)` CI job, `stamp-storage-types-kind.ts`). - Added an Isolation column to the TOC and an explicit `Isolation` line to every scenario (and the exploratory charter), tagging each with the strictest tag it actually needs: `workspace` for DB / tree mutators, `read-only` for the rule-coherence pass, `external` for the `gh pr checks` scenario. - Added "Preconditions: no prerequisite scenarios" / "Scenario N completed" lines so the runner has explicit DAG edges (Scenario 2 depends on 1; Scenario 8 depends on 1 or 3; everything else is independent). - Stripped every severity pre-classification (🛑 Blocker /⚠️ High / 📝 Follow-up) from failure-mode lists in Scenarios 9, 10, 11. The script now enumerates failure-mode categories only; severity belongs in the runner report. - Updated Scenario 4 (negative control for the strict deserializer) to target `migrations/app/20260518T1701_namespaces_bookend/end-contract.json` — the head migration that the planner now reads as the predecessor snapshot after D5's bookend addition. The old target (`20260422T0720_initial`) no longer exercises the predecessor-read path on its own. - Updated Scenario 3 to apply the *full* checked-in history (initial plus bookend) and `migration show` both, so the re-enactment touches the literal pre-bug artefact and the new head. - Updated Scenario 7 to reference PR #533 instead of #520 and to name the actual CI job display name (`Demo `migration plan` (no-op)`). - Updated Scenario 10's oracle and step 8 expectations: the demo `migration plan` is now a *clean* no-op end-to-end (D5's namespaces bookend closed the residual TML-2521 drift), so the prior "you may still see PN-CLI-4020" carve-out is gone — any non-zero / non-no-op envelope is now a finding. - Pre-flight now uses the workspace-canonical CLI invocation (`node ../../packages/.../cli.js`) the rest of the script already uses, so the demo-rooted scenarios all read the same way. - "Scenarios deliberately not in this script" now explicitly excludes the static-pass half of AC-2 / AC-8 (re-running the lint over a clean tree) and AC-11; the sign-off coverage map maps AC-2 to Scenario 5 (the gate-of-gate negative control) and points the AC-11 row at the exclusion table. No scenarios were deleted under the litmus test — every existing scenario already fell into one of the six legitimate buckets. Each scenario now names which bucket it falls under so the runner can quickly verify the script's honesty. Scope is script-revision only: no spec / plan / source / CI / skill changes. The runner skill (`drive-qa-run`) drives the script and owns the report. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
Manual-QA pass against PR #533 surfaced four rough-edges in the prisma-next-demo. Mirrored locally per the convention established by `examples/prisma-next-postgis-demo/gotchas.md` (each entry references its upstream Linear ticket in the [PN] Gotchas project). Entries: - TML-2495 (pre-existing): `db init` PN-MIG-5001 declaredButUnmigrated for extension spaces unless `migration plan` is run first. Same bug class the postgis demo already filed; reproduces here against pgvector. - TML-2564 (new): `migration status` reports `never-planned` after `db init` even when markerHash === headHash. Status taxonomy categorises by how the marker was set, not by sync state. - TML-2565 (new): `migration show` (no argument) silently means `migration heads`. Overloaded verb; targetless behaviour reads src/prisma/contract.json and enumerates per-space leaf migrations, which is unrelated to the targeted behaviour's per-migration inspection. - TML-2566 (new, F-7 system bug): Loaded predecessor contract snapshots are not verified against their persisted storageHash at load time. Semantically-inconsistent end-contract.json passes `migration plan` silently. Follow-on from TML-2536's deserializer-seam closure: the load happens; the post-load hash-recomputation-and-compare step does not. All three new tickets reference TML-2536's manual-QA report at `projects/tml-2536-contract-deserializer-seam/manual-qa-reports/2026-05-18-claude-opus-4-7.md` for repro artefacts. Source-of-truth for these entries is the Linear tickets, not this file; this file is the in-repo mirror so a reader bumping into one of these behaviours has a local pointer. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
force-pushed
the
tml-2536-contract-deserializer-seam-v2
branch
from
bf0e7a7 to
1b0251b
Compare
wmadden
added a commit
that referenced
this pull request
May 19, 2026
Every CLI on-disk contract read now passes the parsed payload as `unknown` into `validateContract`, matching the documented seam pattern (`familyInstance.validateContract(JSON.parse(raw) as unknown)`) in `.cursor/rules/contract-normalization-responsibilities.mdc`. This keeps the read boundary at the serializer seam explicit and avoids silently reintroducing an untyped step inside the validator call. Touches the five read sites flagged by CodeRabbit on PR #533: - `migrate.ts` (contractRaw) - `migration-new.ts` (toContract) - `migration-show.ts` (appContract) - `migration-plan.ts` (validateContract(raw) and familyInstance.validateContract(contractJsonContent)) Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…btree The codemod claimed (in the colocated instructions) to "walk the project root [and] descend into every directory named migrations/", but the actual walk recursed into every non-skipped subtree and stamped any `start-contract.json` / `end-contract.json` it found. That could unintentionally rewrite non-migration fixtures (e.g. inline contract test snapshots) in downstream user projects. Thread an `inMigrations` flag through `walk(...)` so: - the recursion still visits every non-skipped subtree (we cannot short-circuit because `migrations/` need not sit at any fixed depth); - but a CONTRACT_FILES match is only collected once we have crossed (or are below) a directory literally named `migrations`. The fix is applied identically to both copies of the codemod (user upgrade skill + extension-author upgrade skill) so they remain byte-identical, per the existing convention. Flagged by CodeRabbit on PR #533. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…uctions The user-facing instructions claimed the codemod re-serialises with `JSON.stringify(value, null, 2) + '\n'`, but the script actually uses a custom pretty-printer (`formatJson`) that inlines short primitive arrays while otherwise matching `JSON.stringify(null, 2)` indentation. Aligning the prose with the shipped behaviour matches the extension-author copy and prevents misleading diff expectations. Flagged by CodeRabbit on PR #533. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
commented
May 19, 2026
Comment on lines
+85
to
+110
| demo-migration-plan: | ||
| name: Demo `migration plan` (no-op) | ||
| # Regression gate for TML-2536: the bug crashed `prisma-next | ||
| # migration plan` against `examples/prisma-next-demo` because the | ||
| # demo's predecessor end-contract bypassed the family | ||
| # `ContractSerializer` seam. Closing the bug also requires that | ||
| # the demo's checked-in migration history continues to plan as a | ||
| # no-op end-to-end; this job exercises that path on every PR so | ||
| # the demo stops working = failed CI, not undetected rot. | ||
| needs: build | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| - uses: jdx/mise-action@1648a7812b9aeae629881980618f079932869151 # v4.0.1 | ||
| with: | ||
| cache: true | ||
| - uses: pnpm/action-setup@91ab88e2619ed1f46221f0ba42d1492c02baf788 # v6.0.6 | ||
| - name: Install dependencies (skip bin linking) | ||
| run: pnpm install --frozen-lockfile --ignore-scripts | ||
| - name: Build packages (needed for the CLI binary) | ||
| run: pnpm build | ||
| - name: Link bins | ||
| run: pnpm install --frozen-lockfile | ||
| - name: Run migration plan against checked-in history | ||
| run: pnpm --filter prisma-next-demo migration:plan:check | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Why does this have its own workflow step instead of an e2e test?
Contributor
Author
There was a problem hiding this comment.
Agreed — a workflow job per regression test isn't sustainable. Stripped the demo-migration-plan job in b3f8cbe0e. The check itself belongs inside the CLI — migration check --compiled with an opt-in flag that loads migration.ts files and verifies outputs match inputs — filed as TML-2568 in the [PN] May: Migrations project. Once that lands the regression coverage moves into the existing e2e harness, not a bespoke workflow job.
Contributor
Author
There was a problem hiding this comment.
This is pretty strange. Why does this need a dedicated script? This is the kind of thing a user would want to do in CI too: are my migrations in order? Can't we use prisma-next migration check for this?
Contributor
Author
There was a problem hiding this comment.
Right diagnosis. Confirmed: migration check runs structural integrity (hash recompute over metadata+ops, refs, reachability, stored-string compare of metadata.to vs end-contract.json.storage.storageHash) but does not invoke the planner against live contract.json — so it can't catch the TML-2536 regression class (planner crashing on deserialization) or generic drift. Filed TML-2568 to extend migration check --compiled with an opt-in flag that loads migration.ts and verifies outputs match inputs (opt-in because it loads user code; database-credential-free so it can gate CI before deploy creds). Deleted the dedicated script + workflow in b3f8cbe0e ahead of the proper fix.
Contributor
Author
There was a problem hiding this comment.
Record this as gotchas in Linear and delete the file
Contributor
Author
There was a problem hiding this comment.
Done in ddcd9c9d6. All four gotchas (TML-2495, TML-2564, TML-2565, TML-2566) are already in Linear; deleted the local mirror. Postgis-demo's gotchas.md (the pattern I was mirroring) stays for now — separate cleanup pass.
| @@ -13,6 +13,12 @@ function sha256(content: string): string { | |||
| } | |||
|
|
|||
| function hashContract(section: Record<string, unknown>): string { | |||
Contributor
Author
There was a problem hiding this comment.
Why do we need a function which operates on a portion of a contract?
Contributor
Author
There was a problem hiding this comment.
Right diagnosis, deferred to a follow-up.
hashContract constructs a stubbed full Contract envelope (populating roots:{}, models:{}, extensionPacks:{}, capabilities:{}, meta:{}, profileHash:'' as constant noise) and routes it through canonicalizeContract with a naive JSON.parse(JSON.stringify(c)) serializer. So computeStorageHash doesn't actually hash storage — it hashes a stubbed full envelope with storage embedded inside it. Same collision resistance in practice (the stubbed slots contribute zero entropy), but the function does not do what its name says.
The clean fix is per-slice trivial implementations: each compute*Hash takes only its slice + target identifiers and hashes a canonical envelope containing only those fields. No shared hashContract helper, no stubbed whole-Contract construction, no as unknown as Contract cast.
I attempted this refactor in-PR and discovered a serious test-gap: hashing.test.ts verifies structural properties (sha256 format, stability, sensitivity, key-order independence) but does NOT lock specific hash values. The refactor changed the hashed bytes silently — caught only by prisma-next contract emit against the demo (extension headRef.hash mismatch on pgvector). That hash change cascades into every stored hash in tree + every downstream user's checked-in contract.json / end-contract.json.
Filed as TML-2569. Three deliverables: (1) lock hash values in tests, (2) refactor hashing.ts to per-slice trivial impls, (3) handle the breaking-change rollout (internal regen of pgvector + demo + fixtures; user-facing recipe via 0.9-to-0.10 upgrade instructions). Natural next slice after this PR merges.
Comment on lines
+120
to
+125
| * Codec-triple authors that have an untagged shape on hand can call | ||
| * `toStorageTypeInstance(...)` (which stamps the `'codec-instance'` | ||
| * discriminator) before constructing `SqlStorage`. On-disk reads | ||
| * cross `familyInstance.validateContract` first; the structural | ||
| * arktype schema rejects untagged entries earlier, so this throw | ||
| * only fires for in-memory authoring bugs. |
Contributor
Author
There was a problem hiding this comment.
Didn't we remove this behavior?
Contributor
Author
There was a problem hiding this comment.
Yes — the loose fallthrough was removed in commit bd64120b4; what remains is the strict guard at the construction boundary.
Pre-TML-2536, normaliseTypeEntry was a recovery function: it would normalise untagged codec triples to a tagged StorageTypeInstance shape silently, masking format drift. That's the behaviour the bug landed on top of — bypassing the deserializer was invisible because normaliseTypeEntry would patch up untagged input on the way through.
Post-TML-2536, the recovery branches are gone. The function is now what its name should imply: it asserts structural validity at the SqlStorage constructor boundary, throwing a diagnostic naming the entry + its kind (or "missing kind discriminator") for anything that isn't already a tagged StorageTypeInstance or a PostgresEnumStorageEntry class instance. On-disk reads can't reach this throw — the family seam's arktype schema rejects untagged entries earlier. The throw only fires for in-memory authoring bugs (test fixtures, future construction sites that construct SqlStorage directly without crossing the serializer).
Removing the function entirely is on the table but I'd rather keep the construction-boundary guard as defense in depth — it's the only line of defense if a future code path constructs SqlStorage from a raw object that bypassed the serializer. If that becomes load-bearing surface area, we'd want it asserting loudly rather than silently passing through. Could rename to assertTypeEntryShape if the normalise name reads cold given the behavior change — worth doing if you agree.
Comment on lines
+44
to
+52
| // Blind cast: the framework SPI signature | ||
| // (`control-migration-types.ts § contractToSchema`) types | ||
| // `contract` as the generic `Contract | null`. Inside the | ||
| // sqlite target descriptor we know any contract reaching | ||
| // this method is SQL-family — the family contract resolver | ||
| // would have refused to construct a sqlite target binding | ||
| // otherwise — so we narrow the generic to | ||
| // `Contract<SqlStorage>` for the lowering call. | ||
| return contractToSchemaIR(contract as unknown as Contract<SqlStorage> | null, { |
Contributor
Author
There was a problem hiding this comment.
This seems like it could easily be replaced with a type predicate
Contributor
Author
There was a problem hiding this comment.
Done in b84dc23d3. Added a local isSqlContract(contract): contract is Contract<SqlStorage> | null predicate that checks contract.storage instanceof SqlStorage, and replaced the as unknown as Contract<SqlStorage> | null cast with a runtime guard. The narrowing is now real, not asserted; the SPI signature stays as-is. Typecheck clean; lint:no-contract-cast still clean. Kept the predicate local to control-target.ts since it's the only call site — hoist into @prisma-next/family-sql if it proves useful elsewhere.
Contributor
Author
There was a problem hiding this comment.
Deleted the whole manual-qa-reports/artefacts/ tree in fc81d49a0 (F-1, F-7, F-8, F-9, scenario-9-probes.txt) — the inline narrative in the QA report is the durable record. Updated the two surviving artefact-link references in the report to note the captures have been pruned post-merge.
Contributor
Author
There was a problem hiding this comment.
I'm a bit uncertain we want this lint script
Contributor
Author
There was a problem hiding this comment.
Keeping for now. Three landings on the table for a follow-up: (a) widen the regex to AST-based detection (catches typed annotations like let foo: Contract = JSON.parse(...) which the current regex misses); (b) replace with a biome/eslint rule for better integration into the standard lint flow; (c) drop entirely once architect F-A3 (write-side seam asymmetry) is closed and the convention is complete. Filing as low-priority follow-up; the current guard is small, narrow, and bounded by the allowlist size.
Contributor
Author
There was a problem hiding this comment.
Avoid modifying drive skills. Modify drive/code/README.md instead (introduced on #522 )
Contributor
Author
There was a problem hiding this comment.
Done in 7c25a522e. Reverted the 6-line "Smells to surface" block. Moved the same content into drive/code/README.md (new file, follows the drive/qa/README.md structure). #522 (which introduces the drive/<workflow>/README.md convention) can land independently — no conflict.
Contributor
Author
There was a problem hiding this comment.
Avoid modifying drive skills. Modify drive/code/README.md instead (introduced on #522 )
Contributor
Author
There was a problem hiding this comment.
Same fix as the drive-code-review/SKILL.md sibling: reverted in 7c25a522e, content moved to drive/code/README.md.
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…n.ts Per PR #533 review comments 3263947775 + 3263956074: a per-PR workflow job for every regression test isn't sustainable, and the check belongs inside `prisma-next migration check` (see TML-2568) so downstream users can run it in their own CI without a bespoke script. - Remove `demo-migration-plan` job from `.github/workflows/ci.yml`. - Delete `examples/prisma-next-demo/scripts/check-migration-plan.ts`. - Remove `migration:plan:check` from `examples/prisma-next-demo/package.json` scripts. Until TML-2568 lands and the e2e harness picks up the regression coverage, the TML-2536 regression class is guarded only by the test suite + `lint:no-contract-cast`. Accepted trade-off per review.
wmadden
added a commit
that referenced
this pull request
May 19, 2026
Per PR #533 review comment 3263959905: gotchas are tracked in Linear; no need for a local mirror. Entries previously listed in this file are filed as: - TML-2495 — db init fails for extension spaces unless migration plan is run first - TML-2564 — migration status reports never-planned after db init when markerHash === headHash - TML-2565 — migration show (no argument) silently means migration heads - TML-2566 — predecessor end-contract.json is not verified against its persisted storageHash at load time Postgis-demo's gotchas.md (which this file was mirroring) stays for now; can be cleaned up in a separate pass.
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…act predicate Per PR #533 review comment 3264043123: the `contract as unknown as Contract<SqlStorage> | null` cast in `sqliteControlTargetDescriptor.contractToSchema` encoded an SPI-typing workaround as a blind cast. Replace with a local `isSqlContract` type predicate that checks `contract.storage instanceof SqlStorage` and throws a named diagnostic when the invariant is violated. - Promote `SqlStorage` import from `type` to value so `instanceof` works. - Add `isSqlContract(contract): contract is Contract<SqlStorage> | null` predicate local to `control-target.ts` (small blast radius; hoist if it proves useful elsewhere). - Replace the cast with a runtime guard + throw on violation. The type narrowing is now real, not asserted. Typecheck clean (`pnpm typecheck` 134/134); `lint:no-contract-cast` still clean.
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…eferences Per PR #533 review comment 3264043861: artefact captures from the QA run aren't durable evidence; the inline narrative in the QA report itself is sufficient post-merge. - Delete `projects/tml-2536-contract-deserializer-seam/manual-qa-reports/artefacts/` in its entirety (F-1, F-7, F-8, F-9, scenario-9-probes.txt). - Update the QA report (`2026-05-18-claude-opus-4-7.md`) to drop the two surviving artefact link references; replace with a note that raw captures have been pruned.
wmadden
added a commit
that referenced
this pull request
May 19, 2026
Per PR #533 review comments 3264049516 + 3264049883: project-specific review knowledge belongs in `drive/<workflow>/README.md` (convention introduced on #522), not in generic `skills-contrib/drive-*/SKILL.md` bodies. - Revert the "Smells to surface during the principal-engineer pass" section added to `skills-contrib/drive-code-review/SKILL.md` and `skills-contrib/drive-pr-local-review/SKILL.md`. - Create `drive/code/README.md` (missing category README) carrying the `as Contract` smell entry, structured like `drive/qa/README.md`. #522 (which introduces the `drive/<workflow>/README.md` convention) can land independently — no conflict.
Contributor
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
`@skills/extension-author/prisma-next-extension-upgrade/upgrades/0.9-to-0.10/stamp-storage-types-kind.ts`:
- Around line 64-67: Update the header comment in stamp-storage-types-kind.ts to
accurately state that files are serialized using formatJson(...) (which applies
inline short primitive arrays) rather than JSON.stringify(value, null, 2) +
'\n'; locate the top-of-file comment near the storage.types description and
replace the incorrect serialization text with a note that the transformation
writes via formatJson(...) so diffs may include formatting differences (inline
short primitive arrays) outside storage.types.
In
`@skills/upgrade/prisma-next-upgrade/upgrades/0.9-to-0.10/stamp-storage-types-kind.ts`:
- Around line 64-67: Update the file header comment that currently claims output
is produced by `JSON.stringify(value, null, 2) + '\n'` to instead state that
output is produced by `formatJson(...)` with short primitive-array inlining (the
same serializer used in this transform), so maintenance notes accurately reflect
the actual serialization used for `storage.types` and any snapshots; search for
the header text in this file and replace the JSON.stringify reference with a
description mentioning `formatJson` and short primitive-array inlining (keep
mention of `storage.types` and the CLI formatting behavior intact).
🪄 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: b8b5dfe2-a705-4e68-882c-0d84444468ab
⛔ Files ignored due to path filters (2)
projects/tml-2536-contract-deserializer-seam/manual-qa-reports/2026-05-18-claude-opus-4-7.mdis excluded by!projects/**projects/tml-2536-contract-deserializer-seam/manual-qa.mdis excluded by!projects/**
📒 Files selected for processing (10)
drive/code/README.mdpackages/1-framework/3-tooling/cli/src/commands/migrate.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/3-targets/3-targets/sqlite/src/core/control-target.tsskills/extension-author/prisma-next-extension-upgrade/upgrades/0.9-to-0.10/instructions.mdskills/extension-author/prisma-next-extension-upgrade/upgrades/0.9-to-0.10/stamp-storage-types-kind.tsskills/upgrade/prisma-next-upgrade/upgrades/0.9-to-0.10/instructions.mdskills/upgrade/prisma-next-upgrade/upgrades/0.9-to-0.10/stamp-storage-types-kind.ts
✅ Files skipped from review due to trivial changes (1)
- drive/code/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/1-framework/3-tooling/cli/src/commands/migrate.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-new.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-show.ts
- packages/1-framework/3-tooling/cli/src/commands/migration-plan.ts
Comment on lines
+64
to
+67
| * The transformation re-serialises each affected file via | ||
| * `JSON.stringify(value, null, 2) + '\n'` — the same formatting the | ||
| * CLI uses when authoring snapshots originally, so the diff outside | ||
| * `storage.types` is zero on files the CLI generated. Hand-edited |
Contributor
There was a problem hiding this comment.
Update the header comment to match actual serialization behavior.
The comment still says the rewrite uses JSON.stringify(value, null, 2) + '\n', but the script writes with formatJson(...) (including inline short primitive arrays). Keeping this accurate avoids misleading diff expectations.
🛠️ Proposed fix
- * The transformation re-serialises each affected file via
- * `JSON.stringify(value, null, 2) + '\n'` — the same formatting the
- * CLI uses when authoring snapshots originally, so the diff outside
+ * The transformation re-serialises each affected file via a custom
+ * pretty-printer (`formatJson(...) + '\n'`) that matches
+ * `JSON.stringify(..., null, 2)` indentation while inlining short
+ * primitive arrays, so the diff outside
* `storage.types` is zero on files the CLI generated. Hand-edited📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| * The transformation re-serialises each affected file via | |
| * `JSON.stringify(value, null, 2) + '\n'` — the same formatting the | |
| * CLI uses when authoring snapshots originally, so the diff outside | |
| * `storage.types` is zero on files the CLI generated. Hand-edited | |
| * The transformation re-serialises each affected file via a custom | |
| * pretty-printer (`formatJson(...) + '\n'`) that matches | |
| * `JSON.stringify(..., null, 2)` indentation while inlining short | |
| * primitive arrays, so the diff outside | |
| * `storage.types` is zero on files the CLI generated. Hand-edited |
🤖 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
`@skills/extension-author/prisma-next-extension-upgrade/upgrades/0.9-to-0.10/stamp-storage-types-kind.ts`
around lines 64 - 67, Update the header comment in stamp-storage-types-kind.ts
to accurately state that files are serialized using formatJson(...) (which
applies inline short primitive arrays) rather than JSON.stringify(value, null,
2) + '\n'; locate the top-of-file comment near the storage.types description and
replace the incorrect serialization text with a note that the transformation
writes via formatJson(...) so diffs may include formatting differences (inline
short primitive arrays) outside storage.types.
Comment on lines
+64
to
+67
| * The transformation re-serialises each affected file via | ||
| * `JSON.stringify(value, null, 2) + '\n'` — the same formatting the | ||
| * CLI uses when authoring snapshots originally, so the diff outside | ||
| * `storage.types` is zero on files the CLI generated. Hand-edited |
Contributor
There was a problem hiding this comment.
Correct the serializer description in the file header.
The header states JSON.stringify(value, null, 2) + '\n', but output is produced by formatJson(...) with short primitive-array inlining. Updating this keeps maintenance/debug expectations accurate.
🛠️ Proposed fix
- * The transformation re-serialises each affected file via
- * `JSON.stringify(value, null, 2) + '\n'` — the same formatting the
- * CLI uses when authoring snapshots originally, so the diff outside
+ * The transformation re-serialises each affected file via a custom
+ * pretty-printer (`formatJson(...) + '\n'`) that matches
+ * `JSON.stringify(..., null, 2)` indentation while inlining short
+ * primitive arrays, so the diff outside
* `storage.types` is zero on files the CLI generated. Hand-edited📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| * The transformation re-serialises each affected file via | |
| * `JSON.stringify(value, null, 2) + '\n'` — the same formatting the | |
| * CLI uses when authoring snapshots originally, so the diff outside | |
| * `storage.types` is zero on files the CLI generated. Hand-edited | |
| * The transformation re-serialises each affected file via a custom | |
| * pretty-printer (`formatJson(...) + '\n'`) that matches | |
| * `JSON.stringify(..., null, 2)` indentation while inlining short | |
| * primitive arrays, so the diff outside | |
| * `storage.types` is zero on files the CLI generated. Hand-edited |
🤖 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
`@skills/upgrade/prisma-next-upgrade/upgrades/0.9-to-0.10/stamp-storage-types-kind.ts`
around lines 64 - 67, Update the file header comment that currently claims
output is produced by `JSON.stringify(value, null, 2) + '\n'` to instead state
that output is produced by `formatJson(...)` with short primitive-array inlining
(the same serializer used in this transform), so maintenance notes accurately
reflect the actual serialization used for `storage.types` and any snapshots;
search for the header text in this file and replace the JSON.stringify reference
with a description mentioning `formatJson` and short primitive-array inlining
(keep mention of `storage.types` and the CLI formatting behavior intact).
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…ract Sweep finishes the partial rename in the working tree by: - Fixing two malformed JSDoc blocks (control-instances.ts, sql + mongo family control-instance.ts) whose embedded `/**` glob broke typecheck. - Updating the remaining `validateContract` references (comments, READMEs, lint-script error string, drive/code README) to point at the family-instance method `deserializeContract` and dropping the now- redundant cursor rule `contract-normalization-responsibilities.mdc`. - Realigning `.cursor/rules/as-contract-cast-smell.mdc` on the `familyInstance.deserializeContract(...)` idiom. - Renaming the stale call site `validateContract(JSON.parse(...))` in `migration-plan.ts`'s `readPredecessorEndContract` to the local `deserializeContract` parameter the function already declares. The freestanding framework function `validateContract<T>` keeps its name in this commit; Note 1 follow-up collapses it into the family serializer.
…eferences Per PR #533 review comment 3264043861: artefact captures from the QA run aren't durable evidence; the inline narrative in the QA report itself is sufficient post-merge. - Delete `projects/tml-2536-contract-deserializer-seam/manual-qa-reports/artefacts/` in its entirety (F-1, F-7, F-8, F-9, scenario-9-probes.txt). - Update the QA report (`2026-05-18-claude-opus-4-7.md`) to drop the two surviving artefact link references; replace with a note that raw captures have been pruned. Signed-off-by: Will Madden <madden@prisma.io>
Per PR #533 review comments 3264049516 + 3264049883: project-specific review knowledge belongs in `drive/<workflow>/README.md` (convention introduced on #522), not in generic `skills-contrib/drive-*/SKILL.md` bodies. - Revert the "Smells to surface during the principal-engineer pass" section added to `skills-contrib/drive-code-review/SKILL.md` and `skills-contrib/drive-pr-local-review/SKILL.md`. - Create `drive/code/README.md` (missing category README) carrying the `as Contract` smell entry, structured like `drive/qa/README.md`. #522 (which introduces the `drive/<workflow>/README.md` convention) can land independently — no conflict. Signed-off-by: Will Madden <madden@prisma.io>
…ract Sweep finishes the partial rename in the working tree by: - Fixing two malformed JSDoc blocks (control-instances.ts, sql + mongo family control-instance.ts) whose embedded `/**` glob broke typecheck. - Updating the remaining `validateContract` references (comments, READMEs, lint-script error string, drive/code README) to point at the family-instance method `deserializeContract` and dropping the now- redundant cursor rule `contract-normalization-responsibilities.mdc`. - Realigning `.cursor/rules/as-contract-cast-smell.mdc` on the `familyInstance.deserializeContract(...)` idiom. - Renaming the stale call site `validateContract(JSON.parse(...))` in `migration-plan.ts`'s `readPredecessorEndContract` to the local `deserializeContract` parameter the function already declares. The freestanding framework function `validateContract<T>` keeps its name in this commit; Note 1 follow-up collapses it into the family serializer. Signed-off-by: Will Madden <madden@prisma.io>
…ly serializer Removes the freestanding `validateContract<T>` framework function and its per-family wrappers; the family `ContractSerializerBase` SPI is now the only public seam for hydrating an on-disk JSON envelope into a typed `Contract`. - `@prisma-next/contract`: drop `validate-contract.ts` and its export (`./validate-contract`). `ContractValidationError` migrates to its own module (`./contract-validation-error`), which is the new import path for consumers that need to type-test the error class. - `@prisma-next/sql-contract`: `validateSqlContract` is no longer exported; its body is inlined as `validateSqlContractStructure` (module-private) and reached only through `validateSqlContractFully`, which the family `SqlContractSerializerBase` already routes through. Tests realign on `validateSqlContractFully`. - `@prisma-next/mongo-contract`: `validate-mongo-contract.ts` and its test file are gone. The family `MongoContractSerializerBase` already composed `MongoContractSchema` + `validateContractDomain` + `validateMongoStorage` directly, so no logic migrates — only the freestanding wrapper disappears. The two consumers (mongo-demo cache test, mongo-query-builder result-shape test) move to the family serializer or the fixture-seam cast that `as-contract-cast-smell.mdc` explicitly permits in tests. - Cursor rule `.cursor/rules/validate-contract-usage.mdc` is deleted — the type-parameter discipline it prescribed only applied to the freestanding function. - README and comment touch-ups in `family-sql` / `mongo-contract` / `sql-contract` reflect the seam-only surface. Signed-off-by: Will Madden <madden@prisma.io>
Brings `db verify` into line with every other CLI on-disk read (TML-2536): the contract is hydrated at the read site via `familyInstance.deserializeContract`, and the downstream `dbVerify` client/op now accepts the already-deserialized `Contract` instead of running the SerializerBase pipeline a second time. - `commands/db-verify.ts`: construct the family instance up front, drop the multi-paragraph "we read as raw JSON because the seam happens downstream" rationale comment, and route the JSON through `familyInstance.deserializeContract` (with a structured-error wrap for `ContractValidationError`). - `control-api/types.ts`: narrow `DbVerifyOptions.contract` from `unknown` to `Contract` and document the new precondition. - `control-api/client.ts`: `dbVerify` no longer re-deserializes — the hydrated value flows straight into `executeDbVerify`. `client.verify` and `VerifyOptions.contract` keep their `unknown` surface for now: integration tests still pass raw JSON through that entry point, and re-deserialization of an already-hydrated Contract is a no-op for the SQL/Mongo SerializerBase pipelines. Signed-off-by: Will Madden <madden@prisma.io>
…ance directly, async readFile - F01: drop the inner createControlStack + family.create pair in migration-new.ts; reuse the outer family instance constructed up-front for the seam-of-record read. - F02: readPredecessorEndContract now takes the ControlFamilyInstance directly rather than a (json) => deserializeContract closure. The family-instance is the seam; callers depend on it without an extra abstraction layer. - F09: migration-new.ts now uses await readFile from node:fs/promises to match the other four CLI commands that read the contract file. Signed-off-by: Will Madden <madden@prisma.io>
The classifier (looksLikeUntaggedCodecTriple) now rejects pg/enum@1 entries whose typeParams.values is not a string[], so a single diagnostic shape — "neither stamped nor untagged codec triple — hand-edit required" — covers both missing-codecId and malformed-enum-values cases. The defensive runtime check inside stampEntry stays as an invariant marker so a future loosening of the predicate cannot silently produce a malformed StampedPostgresEnum; it is unreachable under the normal flow. Applied byte-identically to both copies (verified via diff). Signed-off-by: Will Madden <madden@prisma.io>
…itly Documents that the lint:no-contract-cast script only enforces the cast-keyword variant of the smell (as Contract / as Contract<...>); structural-assignment variants are not lint-enforced and rely on review + convention. Avoids the rule's prose over-claiming what the grep catches. Signed-off-by: Will Madden <madden@prisma.io>
lint:no-contract-cast is a code-shape lint, not a dependency-graph lint — chaining it into lint:deps mis-taxonomises the failure (a future as Contract regression would surface as a "Lint dependencies" failure rather than a "Lint code shape" one). - package.json: drop lint:no-contract-cast from the lint:deps chain; add a new lint:code script that runs it (kept lint:no-contract-cast as a standalone entry for ad-hoc local invocation). - ci.yml: add a "Lint code shape" step right after "Lint dependencies" so the CI surface mirrors the script taxonomy. The new lint:code slot is named generically so future workspace-level code-shape lints can chain into it. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
force-pushed
the
tml-2536-contract-deserializer-seam-v2
branch
from
549bfde to
c486e1c
Compare
wmadden
added a commit
that referenced
this pull request
May 19, 2026
Re-apply the drive-qa-plan skill against the shipped state of PR #533 (branch `tml-2536-contract-deserializer-seam-v2`) — the previous draft was written in D4 against the now-superseded PR #520 and predated D5 (demo CI gate, lint guard, namespaces-bookend migration) and D6 (rationale comments). What changed: - Header now points at PR #533 and frames both audiences (CLI users + extension authors) up-front; the implementation-history bullet list in "What this script is testing" mentions the shipped guardrails by their concrete names (`lint:no-contract-cast`, the `Demo `migration plan` (no-op)` CI job, `stamp-storage-types-kind.ts`). - Added an Isolation column to the TOC and an explicit `Isolation` line to every scenario (and the exploratory charter), tagging each with the strictest tag it actually needs: `workspace` for DB / tree mutators, `read-only` for the rule-coherence pass, `external` for the `gh pr checks` scenario. - Added "Preconditions: no prerequisite scenarios" / "Scenario N completed" lines so the runner has explicit DAG edges (Scenario 2 depends on 1; Scenario 8 depends on 1 or 3; everything else is independent). - Stripped every severity pre-classification (🛑 Blocker /⚠️ High / 📝 Follow-up) from failure-mode lists in Scenarios 9, 10, 11. The script now enumerates failure-mode categories only; severity belongs in the runner report. - Updated Scenario 4 (negative control for the strict deserializer) to target `migrations/app/20260518T1701_namespaces_bookend/end-contract.json` — the head migration that the planner now reads as the predecessor snapshot after D5's bookend addition. The old target (`20260422T0720_initial`) no longer exercises the predecessor-read path on its own. - Updated Scenario 3 to apply the *full* checked-in history (initial plus bookend) and `migration show` both, so the re-enactment touches the literal pre-bug artefact and the new head. - Updated Scenario 7 to reference PR #533 instead of #520 and to name the actual CI job display name (`Demo `migration plan` (no-op)`). - Updated Scenario 10's oracle and step 8 expectations: the demo `migration plan` is now a *clean* no-op end-to-end (D5's namespaces bookend closed the residual TML-2521 drift), so the prior "you may still see PN-CLI-4020" carve-out is gone — any non-zero / non-no-op envelope is now a finding. - Pre-flight now uses the workspace-canonical CLI invocation (`node ../../packages/.../cli.js`) the rest of the script already uses, so the demo-rooted scenarios all read the same way. - "Scenarios deliberately not in this script" now explicitly excludes the static-pass half of AC-2 / AC-8 (re-running the lint over a clean tree) and AC-11; the sign-off coverage map maps AC-2 to Scenario 5 (the gate-of-gate negative control) and points the AC-11 row at the exclusion table. No scenarios were deleted under the litmus test — every existing scenario already fell into one of the six legitimate buckets. Each scenario now names which bucket it falls under so the runner can quickly verify the script's honesty. Scope is script-revision only: no spec / plan / source / CI / skill changes. The runner skill (`drive-qa-run`) drives the script and owns the report. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
Manual-QA pass against PR #533 surfaced four rough-edges in the prisma-next-demo. Mirrored locally per the convention established by `examples/prisma-next-postgis-demo/gotchas.md` (each entry references its upstream Linear ticket in the [PN] Gotchas project). Entries: - TML-2495 (pre-existing): `db init` PN-MIG-5001 declaredButUnmigrated for extension spaces unless `migration plan` is run first. Same bug class the postgis demo already filed; reproduces here against pgvector. - TML-2564 (new): `migration status` reports `never-planned` after `db init` even when markerHash === headHash. Status taxonomy categorises by how the marker was set, not by sync state. - TML-2565 (new): `migration show` (no argument) silently means `migration heads`. Overloaded verb; targetless behaviour reads src/prisma/contract.json and enumerates per-space leaf migrations, which is unrelated to the targeted behaviour's per-migration inspection. - TML-2566 (new, F-7 system bug): Loaded predecessor contract snapshots are not verified against their persisted storageHash at load time. Semantically-inconsistent end-contract.json passes `migration plan` silently. Follow-on from TML-2536's deserializer-seam closure: the load happens; the post-load hash-recomputation-and-compare step does not. All three new tickets reference TML-2536's manual-QA report at `projects/tml-2536-contract-deserializer-seam/manual-qa-reports/2026-05-18-claude-opus-4-7.md` for repro artefacts. Source-of-truth for these entries is the Linear tickets, not this file; this file is the in-repo mirror so a reader bumping into one of these behaviours has a local pointer. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
Every CLI on-disk contract read now passes the parsed payload as `unknown` into `validateContract`, matching the documented seam pattern (`familyInstance.validateContract(JSON.parse(raw) as unknown)`) in `.cursor/rules/contract-normalization-responsibilities.mdc`. This keeps the read boundary at the serializer seam explicit and avoids silently reintroducing an untyped step inside the validator call. Touches the five read sites flagged by CodeRabbit on PR #533: - `migrate.ts` (contractRaw) - `migration-new.ts` (toContract) - `migration-show.ts` (appContract) - `migration-plan.ts` (validateContract(raw) and familyInstance.validateContract(contractJsonContent)) Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…btree The codemod claimed (in the colocated instructions) to "walk the project root [and] descend into every directory named migrations/", but the actual walk recursed into every non-skipped subtree and stamped any `start-contract.json` / `end-contract.json` it found. That could unintentionally rewrite non-migration fixtures (e.g. inline contract test snapshots) in downstream user projects. Thread an `inMigrations` flag through `walk(...)` so: - the recursion still visits every non-skipped subtree (we cannot short-circuit because `migrations/` need not sit at any fixed depth); - but a CONTRACT_FILES match is only collected once we have crossed (or are below) a directory literally named `migrations`. The fix is applied identically to both copies of the codemod (user upgrade skill + extension-author upgrade skill) so they remain byte-identical, per the existing convention. Flagged by CodeRabbit on PR #533. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…uctions The user-facing instructions claimed the codemod re-serialises with `JSON.stringify(value, null, 2) + '\n'`, but the script actually uses a custom pretty-printer (`formatJson`) that inlines short primitive arrays while otherwise matching `JSON.stringify(null, 2)` indentation. Aligning the prose with the shipped behaviour matches the extension-author copy and prevents misleading diff expectations. Flagged by CodeRabbit on PR #533. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…n.ts Per PR #533 review comments 3263947775 + 3263956074: a per-PR workflow job for every regression test isn't sustainable, and the check belongs inside `prisma-next migration check` (see TML-2568) so downstream users can run it in their own CI without a bespoke script. - Remove `demo-migration-plan` job from `.github/workflows/ci.yml`. - Delete `examples/prisma-next-demo/scripts/check-migration-plan.ts`. - Remove `migration:plan:check` from `examples/prisma-next-demo/package.json` scripts. Until TML-2568 lands and the e2e harness picks up the regression coverage, the TML-2536 regression class is guarded only by the test suite + `lint:no-contract-cast`. Accepted trade-off per review. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
Per PR #533 review comment 3263959905: gotchas are tracked in Linear; no need for a local mirror. Entries previously listed in this file are filed as: - TML-2495 — db init fails for extension spaces unless migration plan is run first - TML-2564 — migration status reports never-planned after db init when markerHash === headHash - TML-2565 — migration show (no argument) silently means migration heads - TML-2566 — predecessor end-contract.json is not verified against its persisted storageHash at load time Postgis-demo's gotchas.md (which this file was mirroring) stays for now; can be cleaned up in a separate pass. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…act predicate Per PR #533 review comment 3264043123: the `contract as unknown as Contract<SqlStorage> | null` cast in `sqliteControlTargetDescriptor.contractToSchema` encoded an SPI-typing workaround as a blind cast. Replace with a local `isSqlContract` type predicate that checks `contract.storage instanceof SqlStorage` and throws a named diagnostic when the invariant is violated. - Promote `SqlStorage` import from `type` to value so `instanceof` works. - Add `isSqlContract(contract): contract is Contract<SqlStorage> | null` predicate local to `control-target.ts` (small blast radius; hoist if it proves useful elsewhere). - Replace the cast with a runtime guard + throw on violation. The type narrowing is now real, not asserted. Typecheck clean (`pnpm typecheck` 134/134); `lint:no-contract-cast` still clean. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…eferences Per PR #533 review comment 3264043861: artefact captures from the QA run aren't durable evidence; the inline narrative in the QA report itself is sufficient post-merge. - Delete `projects/tml-2536-contract-deserializer-seam/manual-qa-reports/artefacts/` in its entirety (F-1, F-7, F-8, F-9, scenario-9-probes.txt). - Update the QA report (`2026-05-18-claude-opus-4-7.md`) to drop the two surviving artefact link references; replace with a note that raw captures have been pruned. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
Per PR #533 review comments 3264049516 + 3264049883: project-specific review knowledge belongs in `drive/<workflow>/README.md` (convention introduced on #522), not in generic `skills-contrib/drive-*/SKILL.md` bodies. - Revert the "Smells to surface during the principal-engineer pass" section added to `skills-contrib/drive-code-review/SKILL.md` and `skills-contrib/drive-pr-local-review/SKILL.md`. - Create `drive/code/README.md` (missing category README) carrying the `as Contract` smell entry, structured like `drive/qa/README.md`. #522 (which introduces the `drive/<workflow>/README.md` convention) can land independently — no conflict. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…ract Sweep finishes the partial rename in the working tree by: - Fixing two malformed JSDoc blocks (control-instances.ts, sql + mongo family control-instance.ts) whose embedded `/**` glob broke typecheck. - Updating the remaining `validateContract` references (comments, READMEs, lint-script error string, drive/code README) to point at the family-instance method `deserializeContract` and dropping the now- redundant cursor rule `contract-normalization-responsibilities.mdc`. - Realigning `.cursor/rules/as-contract-cast-smell.mdc` on the `familyInstance.deserializeContract(...)` idiom. - Renaming the stale call site `validateContract(JSON.parse(...))` in `migration-plan.ts`'s `readPredecessorEndContract` to the local `deserializeContract` parameter the function already declares. The freestanding framework function `validateContract<T>` keeps its name in this commit; Note 1 follow-up collapses it into the family serializer. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…ly serializer Removes the freestanding `validateContract<T>` framework function and its per-family wrappers; the family `ContractSerializerBase` SPI is now the only public seam for hydrating an on-disk JSON envelope into a typed `Contract`. - `@prisma-next/contract`: drop `validate-contract.ts` and its export (`./validate-contract`). `ContractValidationError` migrates to its own module (`./contract-validation-error`), which is the new import path for consumers that need to type-test the error class. - `@prisma-next/sql-contract`: `validateSqlContract` is no longer exported; its body is inlined as `validateSqlContractStructure` (module-private) and reached only through `validateSqlContractFully`, which the family `SqlContractSerializerBase` already routes through. Tests realign on `validateSqlContractFully`. - `@prisma-next/mongo-contract`: `validate-mongo-contract.ts` and its test file are gone. The family `MongoContractSerializerBase` already composed `MongoContractSchema` + `validateContractDomain` + `validateMongoStorage` directly, so no logic migrates — only the freestanding wrapper disappears. The two consumers (mongo-demo cache test, mongo-query-builder result-shape test) move to the family serializer or the fixture-seam cast that `as-contract-cast-smell.mdc` explicitly permits in tests. - Cursor rule `.cursor/rules/validate-contract-usage.mdc` is deleted — the type-parameter discipline it prescribed only applied to the freestanding function. - README and comment touch-ups in `family-sql` / `mongo-contract` / `sql-contract` reflect the seam-only surface. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
Brings `db verify` into line with every other CLI on-disk read (TML-2536): the contract is hydrated at the read site via `familyInstance.deserializeContract`, and the downstream `dbVerify` client/op now accepts the already-deserialized `Contract` instead of running the SerializerBase pipeline a second time. - `commands/db-verify.ts`: construct the family instance up front, drop the multi-paragraph "we read as raw JSON because the seam happens downstream" rationale comment, and route the JSON through `familyInstance.deserializeContract` (with a structured-error wrap for `ContractValidationError`). - `control-api/types.ts`: narrow `DbVerifyOptions.contract` from `unknown` to `Contract` and document the new precondition. - `control-api/client.ts`: `dbVerify` no longer re-deserializes — the hydrated value flows straight into `executeDbVerify`. `client.verify` and `VerifyOptions.contract` keep their `unknown` surface for now: integration tests still pass raw JSON through that entry point, and re-deserialization of an already-hydrated Contract is a no-op for the SQL/Mongo SerializerBase pipelines. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…ance directly, async readFile - F01: drop the inner createControlStack + family.create pair in migration-new.ts; reuse the outer family instance constructed up-front for the seam-of-record read. - F02: readPredecessorEndContract now takes the ControlFamilyInstance directly rather than a (json) => deserializeContract closure. The family-instance is the seam; callers depend on it without an extra abstraction layer. - F09: migration-new.ts now uses await readFile from node:fs/promises to match the other four CLI commands that read the contract file. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
The classifier (looksLikeUntaggedCodecTriple) now rejects pg/enum@1 entries whose typeParams.values is not a string[], so a single diagnostic shape — "neither stamped nor untagged codec triple — hand-edit required" — covers both missing-codecId and malformed-enum-values cases. The defensive runtime check inside stampEntry stays as an invariant marker so a future loosening of the predicate cannot silently produce a malformed StampedPostgresEnum; it is unreachable under the normal flow. Applied byte-identically to both copies (verified via diff). Signed-off-by: Will Madden <madden@prisma.io>
wmadden
added a commit
that referenced
this pull request
May 19, 2026
…itly Documents that the lint:no-contract-cast script only enforces the cast-keyword variant of the smell (as Contract / as Contract<...>); structural-assignment variants are not lint-enforced and rely on review + convention. Avoids the rule's prose over-claiming what the grep catches. Signed-off-by: Will Madden <madden@prisma.io>
This was referenced May 19, 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.
Re-cut of #520 against current
origin/main(the original branch became unrebaseable due to overlapping CLI refactors). Same scope, cleaner history, planned as an orphan slice usingdrive-domain-model(D0–D7).Resolves TML-2536.
Closes #520.
Summary
migration plan,migration new,migration show,migrate) read contracts off disk viaJSON.parse(...) as Contract, bypassing the family'sContractSerializerand skipping the kind-discriminator canonicalization thatstorage.typesrequires. A silent legacy-shape normalizer inSqlStoragewas masking the divergence in-process, so nothing failed loudly even when the demo's migrations stopped functioning.familyInstance.validateContract) at the read site. TheSqlStorage.normaliseTypeEntryfallthrough is removed so the deserializer is strict. Both behaviours are enforced going forward by a structural lint (lint:no-contract-cast) and a demo-level CI regression gate (demo-migration-plan).storage.typescarrying untagged[name, sql-name, codec-ref]triples) no longer parse. Shipped with codemods + instructions for both end-users (skills/upgrade/prisma-next-upgrade/upgrades/0.9-to-0.10) and extension authors (skills/extension-author/prisma-next-extension-upgrade/upgrades/0.9-to-0.10).projects/tml-2536-contract-deserializer-seam/manual-qa.mdwalks 11 scenarios end-to-end through the demo (including the two upgrade journeys and the re-enactment of the original PN-CLI-4020).What's in the slice (D0–D6)
47763874d+65e18457e(drive-domain-model brief), plus deterministic prerequisite picks (dd054f651,7dbbfe609,a918547ae,ed21bdcb9,d0c8bee5a)9c0eb39bc—migration-plan.ts,migration-new.ts,migration-show.ts,migrate.tsall now go throughvalidateContract;db-verify.tsclarifying comment; test mocks updated607c55115—0.9-to-0.10/instructions.md+stamp-storage-types-kind.tscodemod (preserves CLI compact-array on-disk shape)a86ed0051— mirrors D2 plus source-side rules (toStorageTypeInstance/PostgresEnumType)52b10a906— adds upgrade-journey scenarios for both audiences8d7b3fc4e, demo CI gatedbf1bcae4+--json214554ab7, test hygienebd64120b4+82e7cce44, and demo-state bookend2751bf7cb(absorbs TML-2521 namespaces drift so the gate is green)6c4800d21— documents the remaining tenas unknown as Contractallowlist entriesValidation
pnpm typecheck— greenpnpm lint:no-contract-cast— clean (noas Contractcasts outside the rationale-comment allowlist)pnpm lint:deps— cleanpnpm check:upgrade-coverage --mode pr— cleanpnpm fixtures:check— cleanpnpm --filter @prisma-next/cli test— 27/27pnpm --filter prisma-next-demo migration:plan:check— clean (plan is no-op, no file drift)rg "as Contract" packages/1-framework/3-tooling/cli/src/commands/— zero non-test hitspnpm test:packages— 8719/8733 pass; 7 failures across 3 files are infrastructure-flaky (postgres connection / portal C_17 / cross-file parallelism).client.test.tspasses 27/27 when run in isolation.Test plan
demo-migration-planandlint:no-contract-cast).projects/tml-2536-contract-deserializer-seam/manual-qa.md(Scenarios 10 + 11) on a fresh demo checkout.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests