TML-2458: drop vestigial MigrationMetadata.authorship and .signature#500
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR removes optional ChangesMigration Signature and Authorship Field Removal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/extension-cipherstash
@prisma-next/middleware-telemetry
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/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: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/1-framework/3-tooling/migration/README.md`:
- Around line 20-22: Replace the phrase "identity-affecting" with a
non-ambiguous term such as "non-identity" (or "excluded from identity") in the
sentence that currently reads "Strip identity-affecting fields (`migrationHash`,
`fromContract`, `toContract`, `hints`)..." so it becomes e.g. "Strip
non-identity fields (`migrationHash`, `fromContract`, `toContract`, `hints`)..."
and update any nearby occurrences in the same section that refer to identity
inclusion/exclusion (look for the exact string "identity-affecting", the field
names `migrationHash`, `fromContract`, `toContract`, `hints`, and the word
"canonicalize") to keep wording consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 79c032fe-ac90-4a7b-9a0e-f9ebb0112059
📒 Files selected for processing (10)
docs/architecture docs/adrs/ADR 028 - Migration Structure & Operations.mddocs/architecture docs/adrs/ADR 199 - Storage-only migration identity.mddocs/architecture docs/subsystems/7. Migration System.mdpackages/1-framework/1-core/framework-components/src/control/control-migration-types.tspackages/1-framework/3-tooling/migration/README.mdpackages/1-framework/3-tooling/migration/src/hash.tspackages/1-framework/3-tooling/migration/src/io.tspackages/1-framework/3-tooling/migration/src/migration-base.tspackages/1-framework/3-tooling/migration/test/hash.test.tspackages/1-framework/3-tooling/migration/test/io.test.ts
💤 Files with no reviewable changes (5)
- packages/1-framework/1-core/framework-components/src/control/control-migration-types.ts
- packages/1-framework/3-tooling/migration/src/migration-base.ts
- packages/1-framework/3-tooling/migration/test/hash.test.ts
- packages/1-framework/3-tooling/migration/src/io.ts
- packages/1-framework/3-tooling/migration/test/io.test.ts
…tionMetadata.authorship and .signature Both fields predate the contract-spaces work and have no production producer: no CLI command, planner, or runtime path populates either, and the on-disk MigrationMetadataSchema rejects unknown fields, so keeping them around only encoded a feature that did not exist. Removes: - the field declarations on MigrationMetadata - the corresponding optional entries on MigrationMetadataSchema - the signature strip-out in computeMigrationHash (now redundant) - the authorship carry-forward in buildAttestedMetadata - the tests that exercised those paths The SQL marker / db sign flow is unrelated and untouched. Refs: TML-2458 Signed-off-by: Will Madden <madden@prisma.io>
…ocs and ADRs Aligns prose with the corresponding code change: MigrationMetadata no longer carries authorship/signature, computeMigrationHash no longer strips signature, and there is no future "migration sign" command on the roadmap. Updates the migration-tools README, the Migration System subsystem doc, and ADRs 028 and 199 (which both described these fields in their examples and prose). Refs: TML-2458 Signed-off-by: Will Madden <madden@prisma.io>
552bbbf to
e4d8690
Compare
closes TML-2458
At a glance
MigrationMetadatanow has a tighter, fully-wired shape — the two unused optional fields are gone:Before this PR, the same interface also declared
authorship?: { author?; email? }andsignature?: { keyId; value } | null. Neither field had any production producer (no CLI command, planner, or runtime path populated them) and the on-disk arktype schema rejects unknown fields, so they only encoded a feature that did not exist.Decision
Remove
MigrationMetadata.authorshipandMigrationMetadata.signatureand all their plumbing — they predate the contract-spaces work and were never wired. The migration's content-addressed identity already gives us the integrity guarantee that "signature" was speculatively going to provide; "authorship" had no roadmap. If we want signing or attribution later, we'll redesign the schema slot in the context of the contract-spaces / PPg work rather than reuse this one.The SQL marker /
db signflow (theprisma_contract.markerrow) is a separate, fully-wired concept and is untouched.How it fits together
MigrationMetadatainpackages/1-framework/1-core/framework-components/src/control/control-migration-types.ts. The interface is the source of truth that everything else hangs off.MigrationMetadataSchemainpackages/1-framework/3-tooling/migration/src/io.ts. Schema keeps'+': 'reject', so any committedmigration.jsonthat happens to carry these fields now fails to load withMIGRATION.INVALID_MANIFEST— the desired forcing function (zero such files exist in this repo'sexamples/,projects/, or fixtures, so this is a no-op for everyone today).signaturestrip-out incomputeMigrationHash(ADR 199 — Storage-only migration identity). The strip is now redundant: the field doesn't exist, so there's nothing to strip. Hash output for any existing migration is unchanged becausesignaturewas never written in the first place.authorshipcarry-forward inbuildAttestedMetadatainpackages/1-framework/3-tooling/migration/src/migration-base.ts. The onlyifDefineduse in the file went with it, so the import gets cleaned up too.docs/architecture docs/subsystems/7. Migration System.md, and ADRs 028 / 199 all described these fields by name (Subsystem 7 even had a dedicatedSignaturessection and a futureprisma-next migration signcommand); those references are gone, including the lifecycle "optional signature present" bullet and the policy-knob "optionally require a valid signature" line.Behavior changes & evidence
MigrationMetadatalosesauthorship?andsignature?. Type definition: packages/1-framework/1-core/framework-components/src/control/control-migration-types.ts. Schema: packages/1-framework/3-tooling/migration/src/io.ts. Evidence: the now-removedaccepts metadata with optional authorship fieldtest (deletion in packages/1-framework/3-tooling/migration/test/io.test.ts); the existing'+': 'reject'paths in the same suite still cover unknown-field rejection.computeMigrationHashno longer stripssignaturefrom metadata. Implementation: packages/1-framework/3-tooling/migration/src/hash.ts. Evidence: the now-removedignores signature on metadatacase and the framed-tuple test's destructure update in packages/1-framework/3-tooling/migration/test/hash.test.ts — the surviving framed-tuple test still pins the canonical hashing pipeline byte-for-byte against an inline reference computation, so any drift in the hash framing would fail there.buildAttestedMetadatano longer carriesauthorshipforward across re-emits. Implementation: packages/1-framework/3-tooling/migration/src/migration-base.ts. Evidence: every existingmigration-basetest continues to pass against the trimmed metadata shape (no test was deleted for this one — it was always exercising the carry-forward only as a side effect of fixtures that happened to set it).Compatibility / migration / risk
migration.jsonfiles that containauthorshiporsignature. Per the AGENTS.md "no backward-compat shims" rule, this is the desired posture; a repo-wide scan confirmed zero such files exist today (examples/,projects/, demo apps, integration fixtures all clean).computeMigrationHashalready excluded them from the digest, so existingmigrationHashvalues verify identically.MigrationMetadatainterface is technically a breaking change to the type's shape. Practically it's a no-op — the fields were optional, never produced, and downstream consumers (CLI, planners, examples) compile clean (pnpm typecheckgreen across all 134 task targets).pnpm test:packagesis green (596 files / 8229 tests). Nothing else in the suite referenced these fields.Follow-ups
None. This was a self-contained cleanup discovered during PR #434 review.
Alternatives considered
signaturewith a "not yet wired" comment. Considered because Subsystem 7 + ADR 028 had described it as the slot for PPg provenance. Rejected because there is no concrete PPg signing roadmap that would land in this slot, and migration content-addressing already gives us the integrity property a signature was intended to provide; if we ever wire signing, we will redesign the slot in the context of contract spaces / PPg, not reuse this one.'authorship?': 'unknown','signature?': 'unknown') instead of hard-rejecting on read. Rejected because zero on-disk producers means no real risk, and a tolerance shim would silently swallow malformed manifests rather than surfacing them — the opposite of what'+': 'reject'is for.adr-examples-must-match-codeargues for keeping examples honest.Summary by CodeRabbit
Breaking Changes
Chores