fix(tailordb): apply per-migration schema from migration's own snapshot#1207
Draft
toiroakr wants to merge 6 commits into
Draft
fix(tailordb): apply per-migration schema from migration's own snapshot#1207toiroakr wants to merge 6 commits into
toiroakr wants to merge 6 commits into
Conversation
executeSingleMigrationPrePhase (deploy/tailordb/index.ts) sends
changeSet.type.updates[].request verbatim for any type that is
"affected" by the current migration. Because the changeSet is built
from the local source-of-truth types (post-all-migrations), any field
that LATER migrations remove from the same type is already missing
from the request, so the removal silently happens during an earlier
migration's prePhase.
This contradicts docs/services/tailordb-migration.md §"Per-migration
phases", which guarantees that field/type deletions happen in the
owning migration's postPhase only.
Symptom in real deploys: a data-migration script in an intermediate
migration that reads a field removed by a later migration fails with
`field 'X' not found`, because the field was dropped by the very first
prePhase that touched the affected type.
This commit adds a failing unit test that:
1. Stages two pending migrations on the same type User
- #1 adds User.permissions
- #5 removes User.roles
2. Asserts that during #1's prePhase, the request sent to the platform
still contains the roles field (per docs).
3. Currently fails: the request only contains [id, name, permissions]
because roles was already dropped by the local source-of-truth.
Run:
pnpm --filter @tailor-platform/sdk exec vitest run \
src/cli/commands/deploy/tailordb/migration-bulk-schema-bug.test.ts
Expected output: 1 failed, 1 passed.
Note: lefthook hooks are skipped because (1) example#generate fails on
this Windows clone due to pnpm bin symlink issues unrelated to this
change, and (2) the post-commit GPG signature requirement is not set
up on this machine.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
executeSingleMigrationPrePhase/PostPhase sent the FINAL changeSet request for any affected type, so a field that later migrations remove was already missing during an earlier migration's phases. An intermediate migration's data script reading such a field then failed at runtime with `field 'X' not found`. Load each migration's own schema.json snapshot via a cached buildSnapshotTypeManifest helper and submit that intermediate shape instead of the FINAL request. Aligns with services/tailordb-migration.md §"Per-migration phases", which guarantees field/type deletions happen only in the owning migration's postPhase. The repro test cherry-picked from the bug reporter now passes.
🦋 Changeset detectedLatest commit: e697f2c The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
⚡ pkg.pr.new@tailor-platform/sdk@tailor-platform/create-sdk
|
Per-migration pre/post phase was loading <N>/schema.json directly, but the migrate generator only writes the initial 0000/schema.json plus per-migration diff.json. Every real deploy (and CI e2e) therefore hit ENOENT on migrations/0001/schema.json. Use the existing reconstructSnapshotFromMigrations(migrationsDir, N) helper to derive the state AFTER migration N by replaying the initial baseline through all diffs up to N. Cache the result per (namespace, N) for the apply run, as before. Update the bulk-schema-bug reproduction test mock from loadSnapshot to reconstructSnapshotFromMigrations to match.
Drop the long bug-narrative header and the inline FAILS/'current implementation' wording. The test now asserts the spec directly without referring to the pre-fix state.
Trim background-story JSDoc and inline comments left over from the bug investigation: shorten buildSnapshotTypeManifest's doc to its contract, drop the "future migrations' removals" inline narration, and remove restated-assertion comments and trivial fixture descriptions in the migration-bulk-schema-bug test.
Comment on lines
+745
to
+797
| /** | ||
| * Snapshot cache for per-migration schema lookups during a single apply run. | ||
| * | ||
| * Only the initial baseline `0000/schema.json` is stored on disk; later migrations | ||
| * ship `diff.json` only. To get the schema state AFTER migration N we replay the | ||
| * initial snapshot through all diffs up to N via `reconstructSnapshotFromMigrations`. | ||
| * Results are memoized per (namespace, migration number) for the apply run. | ||
| */ | ||
| const migrationSnapshotCache = { | ||
| cache: new Map<string, SchemaSnapshot>(), | ||
| reset() { | ||
| this.cache.clear(); | ||
| }, | ||
| load(migration: PendingMigration): SchemaSnapshot { | ||
| const key = `${migration.namespace}/${migration.number}`; | ||
| let snapshot = this.cache.get(key); | ||
| if (!snapshot) { | ||
| const reconstructed = reconstructSnapshotFromMigrations( | ||
| migration.migrationsDir, | ||
| migration.number, | ||
| ); | ||
| if (!reconstructed) { | ||
| throw new Error( | ||
| `Cannot reconstruct snapshot for ${migration.namespace} migration ${migration.number}: no migrations found in ${migration.migrationsDir}`, | ||
| ); | ||
| } | ||
| snapshot = reconstructed; | ||
| this.cache.set(key, snapshot); | ||
| } | ||
| return snapshot; | ||
| }, | ||
| }; | ||
|
|
||
| /** | ||
| * Build the TailorDBType manifest for `typeName` from migration N's snapshot. | ||
| * @param migration - The pending migration whose snapshot to consult | ||
| * @param typeName - The type name to look up in the snapshot | ||
| * @param tailorDBInputs - Deploy inputs, used to resolve namespace gqlOperations | ||
| * @param executorUsedTypes - Types used by executors (drives publishRecordEvents default) | ||
| * @returns The manifest, or undefined if `typeName` is not in that snapshot. | ||
| */ | ||
| function buildSnapshotTypeManifest( | ||
| migration: PendingMigration, | ||
| typeName: string, | ||
| tailorDBInputs: ReadonlyArray<TailorDBDeployInput>, | ||
| executorUsedTypes: ReadonlySet<string>, | ||
| ): MessageInitShape<typeof TailorDBTypeSchema> | undefined { | ||
| const snapshot = migrationSnapshotCache.load(migration); | ||
| const snapshotType = snapshot.types[typeName]; | ||
| if (!snapshotType) return undefined; | ||
| const input = tailorDBInputs.find((i) => i.namespace === migration.namespace); | ||
| return generateTailorDBTypeManifest(snapshotType, executorUsedTypes, input?.config.gqlOperations); | ||
| } |
|
|
||
| expect(fieldNames).toContain("permissions"); | ||
| expect(fieldNames).toContain("name"); | ||
| expect(fieldNames).toContain("roles"); |
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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacked on #1184 (
refactor/deploy-via-snapshot). Do not merge until #1184 lands; re-target this PR tomainafter that.executeSingleMigration{Pre,Post}PhasesentchangeSet.type.updates[].requestverbatim for any affected type. The changeSet is built from the local source-of-truth (post-all-migrations), so a field that LATER migrations remove was already missing from the request, causing the removal to silently happen during an earlier migration's pre-phase. An intermediate migration's data script reading such a field then failed at runtime withfield 'X' not found.This PR submits the intermediate schema state reconstructed up to migration N (initial
0000/schema.jsonbaseline + diffs through N) via a cachedbuildSnapshotTypeManifesthelper, instead of the FINAL request. Only the initial baseline is stored on disk; later migrations shipdiff.jsononly, so we replay them with the existingreconstructSnapshotFromMigrationshelper. The behavior now matchesservices/tailordb-migration.md§"Per-migration phases", which guarantees field/type deletions happen only in the owning migration's post-phase.index.ts—executeSingleMigration{Pre,Post}Phasenow builds requests from snapshot[N] viabuildSnapshotTypeManifest;migrationSnapshotCachederives snapshot[N] by replaying diffs from the initial baseline;planTypesnow receivesexecutorUsedTypesso the snapshot manifest can defaultpublishRecordEventscorrectly.migration-bulk-schema-bug.test.ts— cherry-picked repro from the bug reporter; extended withreconstructSnapshotFromMigrationsfixtures andexecutorUsedTypescontext to exercise the snapshot pipeline.deploy.test.ts— addedexecutorUsedTypesto the mock context to match the new shape.