diff --git a/.changeset/preserve-removed-fields-in-pre-migration.md b/.changeset/preserve-removed-fields-in-pre-migration.md new file mode 100644 index 000000000..5fc565758 --- /dev/null +++ b/.changeset/preserve-removed-fields-in-pre-migration.md @@ -0,0 +1,9 @@ +--- +"@tailor-platform/sdk": minor +--- + +Improve `tailordb migration` handling of data-loss-possible changes: + +- Removed fields (`field_removed`) and removed types (`type_removed`) are now reported as **warnings** during `tailordb migration generate`, not silent changes. They are no longer dropped before `migrate.ts` runs: the field/type stays available during the Pre-migration phase so that scripts can read it (e.g. `innerJoin` through a foreign key that is being dropped in the same migration), then the physical drop happens in the Post-migration phase. +- Add `tailordb migration script ` subcommand to add a `migrate.ts` (and `db.ts`) template to an existing migration directory. Useful for warning-tier changes where you want a custom data migration even though one was not generated automatically. +- `migrate.ts` is now executed whenever the file exists on disk for a pending migration, regardless of whether the diff originally required a script. Breaking changes still hard-require a script as before. diff --git a/packages/sdk/docs/cli-reference.md b/packages/sdk/docs/cli-reference.md index 7412af13b..4435155f3 100644 --- a/packages/sdk/docs/cli-reference.md +++ b/packages/sdk/docs/cli-reference.md @@ -114,6 +114,7 @@ Commands for managing TailorDB tables, data, and schema migrations. | ---------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------- | | [tailordb truncate](./cli/tailordb.md#tailordb-truncate) | Truncate (delete all records from) TailorDB tables. | | [tailordb migration generate](./cli/tailordb.md#tailordb-migration-generate) | Generate migration files by detecting schema differences between current local types and the previous migration snapshot. | +| [tailordb migration script](./cli/tailordb.md#tailordb-migration-script) | Add a migration script (migrate.ts) template to an existing migration directory. | | [tailordb migration set](./cli/tailordb.md#tailordb-migration-set) | Set migration checkpoint to a specific number. | | [tailordb migration status](./cli/tailordb.md#tailordb-migration-status) | Show the current migration status for TailorDB namespaces, including applied and pending migrations. | | [tailordb erd export](./cli/tailordb.md#tailordb-erd-export) | Export Liam ERD dist from applied TailorDB schema. | diff --git a/packages/sdk/docs/cli/tailordb.md b/packages/sdk/docs/cli/tailordb.md index 2706e3a7a..073c196d7 100644 --- a/packages/sdk/docs/cli/tailordb.md +++ b/packages/sdk/docs/cli/tailordb.md @@ -154,6 +154,7 @@ tailor-sdk tailordb migration [command] | Command | Description | | ------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------- | | [`tailordb migration generate`](#tailordb-migration-generate) | Generate migration files by detecting schema differences between current local types and the previous migration snapshot. | +| [`tailordb migration script`](#tailordb-migration-script) | Add a migration script (migrate.ts) template to an existing migration directory. | | [`tailordb migration set`](#tailordb-migration-set) | Set migration checkpoint to a specific number. | | [`tailordb migration status`](#tailordb-migration-status) | Show the current migration status for TailorDB namespaces, including applied and pending migrations. | @@ -204,6 +205,55 @@ tailor-sdk tailordb migration generate [options] See [Global Options](../cli-reference.md#global-options) for options available to all commands. + + +#### tailordb migration script + + + + + +Add a migration script (migrate.ts) template to an existing migration directory. + + + + + +**Usage** + +``` +tailor-sdk tailordb migration script [options] +``` + + + + + +**Arguments** + +| Argument | Description | Required | +| -------- | ----------------------------------------------------- | -------- | +| `number` | Migration number to add a script to (e.g., 0001 or 1) | Yes | + + + + + +**Options** + +| Option | Alias | Description | Required | Default | Env | +| ------------------------- | ----- | ----------------------------------------------------------------- | -------- | -------------------- | --------------------------------- | +| `--config ` | `-c` | Path to SDK config file | No | `"tailor.config.ts"` | `TAILOR_PLATFORM_SDK_CONFIG_PATH` | +| `--namespace ` | `-n` | Target TailorDB namespace (required if multiple namespaces exist) | No | - | - | + + + + + +See [Global Options](../cli-reference.md#global-options) for options available to all commands. + + + #### tailordb migration set diff --git a/packages/sdk/docs/services/tailordb-migration.md b/packages/sdk/docs/services/tailordb-migration.md index 55da47fa2..1da9d6af7 100644 --- a/packages/sdk/docs/services/tailordb-migration.md +++ b/packages/sdk/docs/services/tailordb-migration.md @@ -23,14 +23,14 @@ migrations/ │ └── schema.json ├── 0001/ # First change │ ├── diff.json # Field-level diff from 0000 -│ ├── migrate.ts # Data migration script (only if breaking) +│ ├── migrate.ts # Data migration script (auto-generated for breaking changes; can be added manually via `migration script`) │ └── db.ts # Kysely types for the script (pre-migration shape) ├── 0002/ │ └── diff.json # No script — non-breaking changes only └── ... ``` -`0000` always contains a full snapshot. `0001` and onward contain a diff plus, for breaking changes, a script and its types. **Commit the entire `migrations/` directory to version control.** +`0000` always contains a full snapshot. `0001` and onward contain a diff plus, optionally, a script and its types (auto-generated for breaking changes, or added manually via `tailordb migration script` for warning-tier changes). **Commit the entire `migrations/` directory to version control.** ## Initial Setup @@ -113,6 +113,24 @@ A typical change cycle: ``` The pre-migration phase relaxes the new field to optional, the script runs and populates values, then the post-migration phase enforces `required: true`. +### Warnings and optional migration scripts + +Some non-breaking changes can still cause data loss — most notably removing a field (`field_removed`) or removing a type (`type_removed`). `migration generate` reports these as **warnings**: + +``` +Warning: data loss possible: + + - User.legacyParentId: Field removed (existing data will be dropped in the post-migration phase) +``` + +No `migrate.ts` is generated automatically because the schema change itself is non-breaking, but the existing data is dropped during the post-migration phase. If you need to preserve or transform that data first (for example, copy a column into another table before it disappears), add a script with: + +```bash +tailor-sdk tailordb migration script 0002 +``` + +This writes `migrations/0002/migrate.ts` and `migrations/0002/db.ts` next to the existing `diff.json`. The removed field stays readable inside `migrate.ts` because the pre-migration phase keeps it on the type until the script finishes (see [Per-migration phases](#per-migration-phases)). The next `tailor-sdk deploy` runs the script automatically — `migrate.ts` is executed whenever the file exists on disk, regardless of whether the diff itself required it. + ## Configuration ```typescript @@ -139,12 +157,12 @@ export default defineConfig({ ## Generated Files -| File | When generated | Description | -| ------------------ | -------------------------------- | --------------------------------------------------------------------------------------------------------- | -| `0000/schema.json` | First `migration generate` | Full snapshot of all types in the namespace. | -| `XXXX/diff.json` | Every subsequent migration | Field-level diff against the previous snapshot. | -| `XXXX/migrate.ts` | Only for breaking changes | Data transformation script. The `main` export receives a Kysely `Transaction`. | -| `XXXX/db.ts` | Generated alongside `migrate.ts` | Kysely types reflecting the schema **before** this migration. Re-generated on every `migration generate`. | +| File | When generated | Description | +| ------------------ | ------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------ | +| `0000/schema.json` | First `migration generate` | Full snapshot of all types in the namespace. | +| `XXXX/diff.json` | Every subsequent migration | Field-level diff against the previous snapshot. | +| `XXXX/migrate.ts` | Auto-generated for breaking changes; added manually via `tailordb migration script` for warning-tier changes | Data transformation script. The `main` export receives a Kysely `Transaction`. | +| `XXXX/db.ts` | Generated once when `migrate.ts` is created | Kysely types reflecting the schema **before** this migration. | `db.ts` reflects the pre-migration schema because the script runs after the pre-migration phase has temporarily relaxed breaking constraints (e.g., a new `required` field is added as `optional` first), so the data being read still matches the previous shape. @@ -178,25 +196,25 @@ export async function main(trx: Transaction): Promise { ## Supported Schema Changes -| Change Type | Breaking? | Migration Script? | Notes | -| ------------------------------ | --------- | ----------------- | ------------------------------------------------------------------------------------- | -| Add optional field | No | No | Schema change only | -| Add required field | Yes | Yes | Script populates default values | -| Remove field | No | No | Schema change only — data is preserved server-side | -| Change optional → required | Yes | Yes | Script sets defaults for null values | -| Change required → optional | No | No | Schema change only | -| Add index | No | No | Schema change only | -| Remove index | No | No | Schema change only | -| Add unique constraint | Yes | Yes | Script must resolve duplicate values | -| Remove unique constraint | No | No | Schema change only | -| Add enum value | No | No | Schema change only | -| Remove enum value | Yes | Yes | Script migrates records with removed values | -| Add type | No | No | Schema change only | -| Remove type | No | No | Schema change only — data is preserved server-side | -| Change foreign key target type | Yes | Yes | Script updates references to the new target | -| Change field type | - | - | **Not supported** — see [3-step migration](#3-step-migration-for-unsupported-changes) | -| Change array → single value | - | - | **Not supported** — see [3-step migration](#3-step-migration-for-unsupported-changes) | -| Change single value → array | - | - | **Not supported** — see [3-step migration](#3-step-migration-for-unsupported-changes) | +| Change Type | Breaking? | Migration Script? | Notes | +| ------------------------------ | --------- | ----------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Add optional field | No | No | Schema change only | +| Add required field | Yes | Yes | Script populates default values | +| Remove field | No | Optional | Warning tier — no script is auto-generated, but you can add one with `tailordb migration script` to preserve data before the field is dropped. The field stays readable from `migrate.ts` during Pre-migration and is dropped in Post-migration. | +| Change optional → required | Yes | Yes | Script sets defaults for null values | +| Change required → optional | No | No | Schema change only | +| Add index | No | No | Schema change only | +| Remove index | No | No | Schema change only | +| Add unique constraint | Yes | Yes | Script must resolve duplicate values | +| Remove unique constraint | No | No | Schema change only | +| Add enum value | No | No | Schema change only | +| Remove enum value | Yes | Yes | Script migrates records with removed values | +| Add type | No | No | Schema change only | +| Remove type | No | Optional | Warning tier — no script is auto-generated, but you can add one with `tailordb migration script` to preserve data before the type is dropped. The type stays readable from `migrate.ts` during Pre-migration and is dropped in Post-migration. | +| Change foreign key target type | Yes | Yes | Script updates references to the new target | +| Change field type | - | - | **Not supported** — see [3-step migration](#3-step-migration-for-unsupported-changes) | +| Change array → single value | - | - | **Not supported** — see [3-step migration](#3-step-migration-for-unsupported-changes) | +| Change single value → array | - | - | **Not supported** — see [3-step migration](#3-step-migration-for-unsupported-changes) | ### 3-step migration for unsupported changes @@ -216,11 +234,11 @@ When you run `tailor-sdk deploy`, the SDK detects pending migrations (anything p For each pending migration: -1. **Pre-migration**: Type changes that would be breaking are applied in a relaxed form first. Newly-required fields are added as optional; fields whose `optional → required` transition is breaking are temporarily kept optional. Non-breaking changes that are part of the same migration are also applied here. -2. **Script execution**: If `diff.requiresMigrationScript` is true, `migrate.ts` is bundled and sent to the platform via the script execution API. It runs as the configured machine user inside a transaction. -3. **Post-migration**: Required constraints are enforced; field/type deletions are applied; the `sdk-migration` label is bumped to this migration's number. +1. **Pre-migration**: Type changes that would be breaking are applied in a relaxed form first. Newly-required fields are added as optional; fields whose `optional → required` transition is breaking are temporarily kept optional. Fields that are being removed in this migration are temporarily kept on the type so that `migrate.ts` can still read them (for example, to `innerJoin` through a foreign key that is about to be dropped). Non-breaking changes that are part of the same migration are also applied here. +2. **Script execution**: If `migrate.ts` exists on disk for this migration, it is bundled and sent to the platform via the script execution API and runs as the configured machine user inside a transaction. The script is hard-required for breaking changes (`diff.requiresMigrationScript`) but is also executed when present for warning-tier diffs — see [Warnings and optional migration scripts](#warnings-and-optional-migration-scripts). +3. **Post-migration**: Required constraints are enforced; field and type deletions are applied (the columns/tables are physically dropped here); the `sdk-migration` label is bumped to this migration's number. -This split is what allows existing rows to be backfilled before the database starts rejecting nulls. +This split is what allows existing rows to be backfilled before the database starts rejecting nulls, and what lets `migrate.ts` traverse foreign-key fields that the same migration removes. ### Schema verification diff --git a/packages/sdk/src/cli/commands/deploy/tailordb/index.test.ts b/packages/sdk/src/cli/commands/deploy/tailordb/index.test.ts index 6b4563285..2d4d59206 100644 --- a/packages/sdk/src/cli/commands/deploy/tailordb/index.test.ts +++ b/packages/sdk/src/cli/commands/deploy/tailordb/index.test.ts @@ -2,15 +2,20 @@ import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "pathe"; import { describe, test, expect, vi, beforeEach, afterEach } from "vitest"; +import { applyPreMigrationFieldAdjustments } from "@/cli/commands/tailordb/migrate/pre-migration-schema"; import { sdkNameLabelKey } from "../label"; import { applyTailorDB, formatTailorDBResourceChangeEntries, planTailorDB } from "."; import type { PlanContext } from "../deploy"; +import type { DiffChange } from "@/cli/commands/tailordb/migrate/diff-calculator"; +import type { SnapshotFieldConfig } from "@/cli/commands/tailordb/migrate/snapshot"; import type { Application } from "@/cli/services/application"; import type { ExecutorService } from "@/cli/services/executor/service"; import type { TailorDBService } from "@/cli/services/tailordb/service"; import type { OperatorClient } from "@/cli/shared/client"; import type { LoadedConfig } from "@/cli/shared/config-loader"; import type { TailorDBType } from "@/types/tailordb"; +import type { MessageInitShape } from "@bufbuild/protobuf"; +import type { TailorDBType_FieldConfigSchema } from "@tailor-proto/tailor/v1/tailordb_resource_pb"; // Mock label.ts vi.mock("../label", async (importOriginal) => { @@ -824,6 +829,78 @@ describe("applyTailorDB phase separation", () => { }); }); +describe("applyPreMigrationFieldAdjustments", () => { + type ProtoField = MessageInitShape; + + test("re-inserts removed field so migrate.ts can still read it", () => { + // Simulate the new schema produced by planTailorDB: the removed field + // has already been stripped from `fields`. + const fields: Record = { + name: { type: "string", required: true }, + }; + + const removedFieldBefore: SnapshotFieldConfig = { + type: "uuid", + required: true, + foreignKey: true, + foreignKeyType: "OldParent", + }; + const typeChanges = new Map([ + [ + "oldParentId", + { + kind: "field_removed", + typeName: "Child", + fieldName: "oldParentId", + before: removedFieldBefore, + }, + ], + ]); + + applyPreMigrationFieldAdjustments(fields, typeChanges); + + expect(fields.oldParentId).toBeDefined(); + expect(fields.oldParentId?.type).toBe("uuid"); + expect(fields.oldParentId?.foreignKey).toBe(true); + expect(fields.oldParentId?.foreignKeyType).toBe("OldParent"); + expect(fields.oldParentId?.required).toBe(true); + // Untouched fields are preserved. + expect(fields.name?.type).toBe("string"); + }); + + test("relaxes newly-added required field to optional", () => { + const fields: Record = { + newField: { type: "string", required: true }, + }; + const typeChanges = new Map([ + [ + "newField", + { + kind: "field_added", + typeName: "T", + fieldName: "newField", + after: { type: "string", required: true }, + }, + ], + ]); + + applyPreMigrationFieldAdjustments(fields, typeChanges); + + expect(fields.newField?.required).toBe(false); + }); + + test("does not modify fields that are not in typeChanges", () => { + const fields: Record = { + keep: { type: "string", required: true }, + }; + const typeChanges = new Map(); + + applyPreMigrationFieldAdjustments(fields, typeChanges); + + expect(fields.keep?.required).toBe(true); + }); +}); + describe("applyTailorDB migration label reconciliation (--no-schema-check)", () => { let tmpDir: string; let configPath: string; diff --git a/packages/sdk/src/cli/commands/deploy/tailordb/index.ts b/packages/sdk/src/cli/commands/deploy/tailordb/index.ts index cea9dc745..a49b271ee 100644 --- a/packages/sdk/src/cli/commands/deploy/tailordb/index.ts +++ b/packages/sdk/src/cli/commands/deploy/tailordb/index.ts @@ -44,8 +44,11 @@ import { formatMigrationDiff, formatDiffSummary, type MigrationDiff, - type DiffChange, } from "@/cli/commands/tailordb/migrate/diff-calculator"; +import { + applyPreMigrationFieldAdjustments, + buildPreMigrationChangesMap, +} from "@/cli/commands/tailordb/migrate/pre-migration-schema"; import { reconstructSnapshotFromMigrations, compareLocalTypesWithSnapshot, @@ -83,7 +86,6 @@ import type { } from "@/cli/commands/tailordb/migrate/types"; import type { LoadedConfig } from "@/cli/shared/config-loader"; import type { Executor } from "@/types/executor.generated"; -import type { EnumValue } from "@/types/field-types"; import type { PermissionOperand, StandardActionPermission, @@ -324,9 +326,9 @@ async function validateAndDetectMigrations( if (pendingMigrations.length > 0) { logger.newline(); - // Classify migrations by whether they require migration scripts - const withScripts = pendingMigrations.filter((m) => m.diff.requiresMigrationScript); - const withoutScripts = pendingMigrations.filter((m) => !m.diff.requiresMigrationScript); + // Classify migrations by whether a migrate.ts will run for them. + const withScripts = pendingMigrations.filter((m) => m.hasScript); + const withoutScripts = pendingMigrations.filter((m) => !m.hasScript); logger.info(`Applying ${pendingMigrations.length} migration(s):`); if (withoutScripts.length > 0) { @@ -462,9 +464,7 @@ export async function applyTailorDB( // Step 1: Create/update services once at the beginning (services don't need per-migration handling) await executeServicesCreation(client, changeSet); - const migrationsRequiringScripts = pendingMigrations.filter( - (m) => m.diff.requiresMigrationScript, - ); + const migrationsRequiringScripts = pendingMigrations.filter((m) => m.hasScript); // Step 2: Build migration context for script execution (if any migrations require scripts) const migrationCtx = @@ -482,8 +482,8 @@ export async function applyTailorDB( // Pre-migration phase: Create/update types with breaking fields as optional await executeSingleMigrationPrePhase(client, changeSet, migration); - // Script execution (only if this migration requires a script) - if (migration.diff.requiresMigrationScript && migrationCtx) { + // Script execution (only if migrate.ts exists for this migration) + if (migration.hasScript && migrationCtx) { await executeMigrations(migrationCtx, [migration]); } @@ -614,111 +614,6 @@ function handleOptionalToRequiredError(error: unknown, messages: string[]): neve throw error; } -// ============================================================================ -// Pre-Migration Support -// ============================================================================ - -/** - * Map of breaking changes: typeName -> fieldName -> change kind - */ -type BreakingChangesMap = Map>; - -/** - * Build a map of breaking field changes from pending migrations - * @param {PendingMigration[]} pendingMigrations - Pending migrations - * @returns {BreakingChangesMap} Map of breaking changes - */ -function buildBreakingChangesMap(pendingMigrations: PendingMigration[]): BreakingChangesMap { - const map: BreakingChangesMap = new Map(); - - for (const migration of pendingMigrations) { - for (const change of migration.diff.changes) { - // We care about field changes that affect required status - if ( - change.kind === "field_added" || - change.kind === "field_modified" || - change.kind === "field_removed" - ) { - if (!change.fieldName) continue; - - if (!map.has(change.typeName)) { - map.set(change.typeName, new Map()); - } - map.get(change.typeName)!.set(change.fieldName, change); - } - } - } - - return map; -} - -/** - * Field config type for breaking change detection - */ -interface FieldConfig { - required?: boolean; - unique?: boolean; - allowedValues?: EnumValue[]; -} - -/** - * Apply pre-migration schema adjustments to avoid breaking changes before scripts run. - * @param fields - Field configs to adjust - * @param typeChanges - Breaking changes for a type - */ -function applyPreMigrationFieldAdjustments( - fields: Record>, - typeChanges: Map, -): void { - for (const [fieldName, change] of typeChanges) { - const field = fields[fieldName]; - if (!field) continue; - - const before = change.before as FieldConfig | undefined; - const after = change.after as FieldConfig | undefined; - - if (change.kind === "field_added" && after?.required) { - field.required = false; - } - - if (change.kind !== "field_modified") { - continue; - } - - // Optional to required - if (!before?.required && after?.required) { - field.required = false; - } - - // Unique constraint added - if (!(before?.unique ?? false) && (after?.unique ?? false)) { - field.unique = false; - } - - // Enum values removed: keep old values + add new values (union) - if (before?.allowedValues && after?.allowedValues) { - const afterValues = new Set(after.allowedValues.map((v) => v.value)); - const removedValues = before.allowedValues.filter((v) => !afterValues.has(v.value)); - if (removedValues.length > 0) { - // Create union of all values, preserving descriptions where available - const valueMap = new Map(); - for (const v of before.allowedValues) { - valueMap.set(v.value, v.description ?? ""); - } - for (const v of after.allowedValues) { - if (!valueMap.has(v.value)) { - valueMap.set(v.value, v.description ?? ""); - } - } - field.allowedValues = Array.from(valueMap.entries()).map(([value, description]) => ({ - value, - description, - })); - } - } - } -} - // ============================================================================ // Migration Execution Helpers // ============================================================================ @@ -798,8 +693,10 @@ async function executeSingleMigrationPrePhase( changeSet: TailorDBChangeSet, migration: PendingMigration, ): Promise { - // Build breaking changes map for this single migration - const breakingChanges = buildBreakingChangesMap([migration]); + // Build pre-migration changes map for this single migration. Includes both + // breaking changes (required-add, unique-add, enum value removal) and the + // warning-tier field_removed, since the Pre-phase relaxes both. + const preMigrationChanges = buildPreMigrationChangesMap([migration]); const affectedTypes = getAffectedTypeNames(migration); const createdBeforeMigration = new Set(processedTypes.created); @@ -815,7 +712,7 @@ async function executeSingleMigrationPrePhase( const typeName = create.request.tailordbType?.name; if (typeName) processedTypes.created.add(typeName); - const typeChanges = typeName ? breakingChanges.get(typeName) : undefined; + const typeChanges = typeName ? preMigrationChanges.get(typeName) : undefined; if (!typeChanges || typeChanges.size === 0) { return client.createTailorDBType(create.request); @@ -839,7 +736,7 @@ async function executeSingleMigrationPrePhase( const typeName = create.request.tailordbType?.name; if (typeName) processedTypes.updated.add(typeName); - const typeChanges = typeName ? breakingChanges.get(typeName) : undefined; + const typeChanges = typeName ? preMigrationChanges.get(typeName) : undefined; if (!typeChanges || typeChanges.size === 0) { return client.updateTailorDBType({ @@ -870,7 +767,7 @@ async function executeSingleMigrationPrePhase( const typeName = update.request.tailordbType?.name; if (typeName) processedTypes.updated.add(typeName); - const typeChanges = typeName ? breakingChanges.get(typeName) : undefined; + const typeChanges = typeName ? preMigrationChanges.get(typeName) : undefined; if (!typeChanges || typeChanges.size === 0) { return client.updateTailorDBType(update.request); @@ -952,8 +849,9 @@ async function executeSingleMigrationPostPhase( changeSet: TailorDBChangeSet, migration: PendingMigration, ): Promise { - // Build breaking changes map for this single migration - const breakingChanges = buildBreakingChangesMap([migration]); + // Re-use the pre-migration changes map to know which types were touched in + // this migration (so we send the post-phase final-schema update for them). + const preMigrationChanges = buildPreMigrationChangesMap([migration]); const affectedTypes = getAffectedTypeNames(migration); const deletedTypeNames = getDeletedTypeNames(migration); @@ -961,11 +859,11 @@ async function executeSingleMigrationPostPhase( // Pre-migration used cloned requests, so the original changeSet still has correct values try { await Promise.all([ - // For newly created types that had breaking changes in this migration, send update with final values + // For newly created types that had pre-migration adjustments in this migration, send update with final values ...changeSet.type.creates .filter((create) => { const typeName = create.request.tailordbType?.name; - return typeName && affectedTypes.has(typeName) && breakingChanges.has(typeName); + return typeName && affectedTypes.has(typeName) && preMigrationChanges.has(typeName); }) .map((create) => client.updateTailorDBType({ @@ -978,7 +876,7 @@ async function executeSingleMigrationPostPhase( ...changeSet.type.updates .filter((update) => { const typeName = update.request.tailordbType?.name; - return typeName && affectedTypes.has(typeName) && breakingChanges.has(typeName); + return typeName && affectedTypes.has(typeName) && preMigrationChanges.has(typeName); }) .map((update) => client.updateTailorDBType(update.request)), ]); diff --git a/packages/sdk/src/cli/commands/deploy/tailordb/migration.test.ts b/packages/sdk/src/cli/commands/deploy/tailordb/migration.test.ts index f075998ea..4569e59cd 100644 --- a/packages/sdk/src/cli/commands/deploy/tailordb/migration.test.ts +++ b/packages/sdk/src/cli/commands/deploy/tailordb/migration.test.ts @@ -16,8 +16,11 @@ import { updateMigrationLabel, getMigrationMachineUser, groupMigrationsByNamespace, + executeMigrations, + type MigrationContext, } from "./migration"; import type { NamespaceWithMigrations } from "@/cli/commands/tailordb/migrate/config"; +import type { PendingMigration } from "@/cli/commands/tailordb/migrate/types"; import type { OperatorClient } from "@/cli/shared/client"; // Mock label.ts for trnPrefix @@ -34,12 +37,34 @@ vi.mock("@/cli/shared/logger", () => ({ success: vi.fn(), debug: vi.fn(), newline: vi.fn(), + log: vi.fn(), }, styles: { bold: (s: string) => s, }, })); +// Mock spinner so tests don't render TTY frames +vi.mock("@/cli/shared/spinner", () => ({ + spinner: () => ({ + start: () => ({ + succeed: vi.fn(), + fail: vi.fn(), + }), + }), +})); + +// Mock bundler and script executor so executeMigrations can run without +// touching the network or building real bundles. +const bundleMigrationScriptMock = vi.fn(); +const executeScriptMock = vi.fn(); +vi.mock("@/cli/commands/tailordb/migrate/bundler", () => ({ + bundleMigrationScript: (...args: unknown[]) => bundleMigrationScriptMock(...args), +})); +vi.mock("@/cli/shared/script-executor", () => ({ + executeScript: (...args: unknown[]) => executeScriptMock(...args), +})); + const TEST_MIGRATIONS_BASE = path.join(__dirname, "__test_migrations_service__"); function createMockDiff(options: Partial = {}): MigrationDiff { @@ -50,6 +75,8 @@ function createMockDiff(options: Partial = {}): MigrationDiff { changes: [], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, ...options, }; @@ -141,6 +168,7 @@ describe("migration", () => { number: 1, migrationsDir: "/path/a", scriptPath: "/path/a/0001/migrate.ts", + hasScript: true, diffPath: "/path/a/0001/diff.json", diff: createMockDiff({ namespace: "namespace-a" }), }, @@ -149,6 +177,7 @@ describe("migration", () => { number: 1, migrationsDir: "/path/b", scriptPath: "/path/b/0001/migrate.ts", + hasScript: true, diffPath: "/path/b/0001/diff.json", diff: createMockDiff({ namespace: "namespace-b" }), }, @@ -157,6 +186,7 @@ describe("migration", () => { number: 2, migrationsDir: "/path/a", scriptPath: "/path/a/0002/migrate.ts", + hasScript: true, diffPath: "/path/a/0002/diff.json", diff: createMockDiff({ namespace: "namespace-a" }), }, @@ -183,6 +213,7 @@ describe("migration", () => { number: 1, migrationsDir: "/path", scriptPath: "/path/0001/migrate.ts", + hasScript: true, diffPath: "/path/0001/diff.json", diff: createMockDiff({ namespace: "single" }), }, @@ -191,6 +222,7 @@ describe("migration", () => { number: 2, migrationsDir: "/path", scriptPath: "/path/0002/migrate.ts", + hasScript: true, diffPath: "/path/0002/diff.json", diff: createMockDiff({ namespace: "single" }), }, @@ -451,4 +483,121 @@ describe("migration", () => { }); }); }); + + // ========================================================================== + // executeMigrations + // ========================================================================== + describe("executeMigrations", () => { + const workspaceId = "test-workspace"; + + function createMockContext(): MigrationContext { + return { + client: {} as unknown as OperatorClient, + workspaceId, + authNamespace: "auth", + machineUsers: ["test-machine-user"], + dbConfig: {}, + }; + } + + function createMockMigration(overrides: Partial): PendingMigration { + return { + number: 1, + scriptPath: "/path/0001/migrate.ts", + hasScript: true, + diffPath: "/path/0001/diff.json", + namespace: "tailordb", + migrationsDir: "/path", + diff: createMockDiff(), + ...overrides, + }; + } + + beforeEach(() => { + bundleMigrationScriptMock.mockReset(); + executeScriptMock.mockReset(); + bundleMigrationScriptMock.mockResolvedValue({ + bundledCode: "// bundled", + warnings: [], + }); + executeScriptMock.mockResolvedValue({ + success: true, + logs: "", + result: "", + }); + }); + + it("skips migrations without a script file on disk", async () => { + const migrations = [ + createMockMigration({ number: 1, hasScript: false }), + createMockMigration({ number: 2, hasScript: false }), + ]; + + await executeMigrations(createMockContext(), migrations); + + expect(bundleMigrationScriptMock).not.toHaveBeenCalled(); + expect(executeScriptMock).not.toHaveBeenCalled(); + }); + + it("executes warning-tier migrations whose script exists even when not required", async () => { + // requiresMigrationScript=false but hasScript=true represents the + // warning-tier case (e.g. field_removed) where the user opted in by + // running `tailordb migration script`. The optional script must still + // run during deploy. + const migrations = [ + createMockMigration({ + number: 1, + hasScript: true, + diff: createMockDiff({ + hasWarnings: true, + requiresMigrationScript: false, + }), + }), + createMockMigration({ + number: 2, + hasScript: false, + diff: createMockDiff({ + hasWarnings: true, + requiresMigrationScript: false, + }), + }), + ]; + + await executeMigrations(createMockContext(), migrations); + + expect(bundleMigrationScriptMock).toHaveBeenCalledTimes(1); + expect(executeScriptMock).toHaveBeenCalledTimes(1); + expect(executeScriptMock.mock.calls[0][0]).toMatchObject({ + name: "migration-tailordb-0001.js", + }); + }); + + it("executes only the subset with hasScript=true when mixed with breaking changes", async () => { + const migrations = [ + createMockMigration({ + number: 1, + hasScript: true, + diff: createMockDiff({ hasBreakingChanges: true, requiresMigrationScript: true }), + }), + createMockMigration({ + number: 2, + hasScript: false, + diff: createMockDiff({ hasWarnings: true, requiresMigrationScript: false }), + }), + createMockMigration({ + number: 3, + hasScript: true, + diff: createMockDiff({ hasWarnings: true, requiresMigrationScript: false }), + }), + ]; + + await executeMigrations(createMockContext(), migrations); + + expect(executeScriptMock).toHaveBeenCalledTimes(2); + const executedNames = executeScriptMock.mock.calls.map( + (call) => (call[0] as { name: string }).name, + ); + expect(executedNames).toEqual(["migration-tailordb-0001.js", "migration-tailordb-0003.js"]); + }); + }); }); diff --git a/packages/sdk/src/cli/commands/deploy/tailordb/migration.ts b/packages/sdk/src/cli/commands/deploy/tailordb/migration.ts index 0a624baef..b3939f0ef 100644 --- a/packages/sdk/src/cli/commands/deploy/tailordb/migration.ts +++ b/packages/sdk/src/cli/commands/deploy/tailordb/migration.ts @@ -122,12 +122,15 @@ export async function detectPendingMigrations( continue; } - // Load the diff to check if migration script is required + // Load the diff to inspect breaking/warning classification const diff = loadDiff(diffPath); - // Check for migration script (only required for breaking changes) + // The migration script is executed when migrate.ts exists on disk. + // Breaking changes still hard-require a script; warnings (e.g. field_removed) + // may optionally have one added via `tailordb migration script `. const scriptPath = getMigrationFilePath(migrationsDir, file.number, "migrate"); - if (diff.requiresMigrationScript && !fs.existsSync(scriptPath)) { + const hasScript = fs.existsSync(scriptPath); + if (diff.requiresMigrationScript && !hasScript) { logger.warn( `Migration ${namespace}/${file.number} requires a script but migrate.ts not found`, ); @@ -136,7 +139,8 @@ export async function detectPendingMigrations( pendingMigrations.push({ number: file.number, - scriptPath, // May not exist for non-breaking changes + scriptPath, + hasScript, diffPath, namespace, migrationsDir, @@ -241,8 +245,9 @@ export async function executeMigrations( context: MigrationContext, migrations: PendingMigration[], ): Promise { - // Filter migrations that require script execution - const migrationsWithScripts = migrations.filter((m) => m.diff.requiresMigrationScript); + // Run migrate.ts whenever the file exists on disk. Required for breaking changes, + // optional for warning-tier changes (e.g. field_removed). + const migrationsWithScripts = migrations.filter((m) => m.hasScript); if (migrationsWithScripts.length === 0) { return; diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/cli.test.ts b/packages/sdk/src/cli/commands/tailordb/migrate/cli.test.ts index bc580b281..9b4ce4f7d 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/cli.test.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/cli.test.ts @@ -1,5 +1,11 @@ import { describe, expect, it } from "vitest"; -import { generateCommand, migrationCommand, setCommand, statusCommand } from "./index"; +import { + generateCommand, + migrationCommand, + scriptCommand, + setCommand, + statusCommand, +} from "./index"; describe("migration CLI commands", () => { describe("migrationCommand", () => { @@ -12,6 +18,10 @@ describe("migration CLI commands", () => { expect(migrationCommand.subCommands).toHaveProperty("generate"); }); + it("should have script subcommand", () => { + expect(migrationCommand.subCommands).toHaveProperty("script"); + }); + it("should have set subcommand", () => { expect(migrationCommand.subCommands).toHaveProperty("set"); }); @@ -48,6 +58,19 @@ describe("migration CLI commands", () => { }); }); + describe("scriptCommand", () => { + it("should have correct meta information", () => { + expect(scriptCommand.name).toBe("script"); + expect(scriptCommand.description).toContain("script"); + }); + + it("should have required args schema", () => { + const shape = scriptCommand.args.shape; + expect(shape).toHaveProperty("number"); + expect(shape).toHaveProperty("namespace"); + }); + }); + describe("statusCommand", () => { it("should have correct meta information", () => { expect(statusCommand.name).toBe("status"); diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/db-types-generator.test.ts b/packages/sdk/src/cli/commands/tailordb/migrate/db-types-generator.test.ts index fdaa9ccab..a904c019b 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/db-types-generator.test.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/db-types-generator.test.ts @@ -61,6 +61,8 @@ function createMockDiff( changes, hasBreakingChanges: options.hasBreakingChanges ?? false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: options.requiresMigrationScript ?? false, }; } diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/diff-calculator.test.ts b/packages/sdk/src/cli/commands/tailordb/migrate/diff-calculator.test.ts index 17c06e026..b96d77826 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/diff-calculator.test.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/diff-calculator.test.ts @@ -3,10 +3,12 @@ import { hasChanges, formatMigrationDiff, formatBreakingChanges, + formatWarnings, formatDiffSummary, SCHEMA_SNAPSHOT_VERSION, type MigrationDiff, type BreakingChangeInfo, + type WarningChangeInfo, } from "./diff-calculator"; // Helper to create a MigrationDiff @@ -21,6 +23,8 @@ function createDiff( changes, hasBreakingChanges: breakingChanges.length > 0, breakingChanges, + hasWarnings: false, + warnings: [], requiresMigrationScript: breakingChanges.length > 0, }; } @@ -217,6 +221,52 @@ describe("diff-calculator", () => { }); }); + describe("formatWarnings", () => { + it("should return empty string for no warnings", () => { + const result = formatWarnings([]); + expect(result).toBe(""); + }); + + it("should format warnings with field", () => { + const warnings: WarningChangeInfo[] = [ + { + typeName: "User", + fieldName: "legacyId", + reason: "Field removed (existing data will be dropped in the post-migration phase)", + }, + ]; + const result = formatWarnings(warnings); + expect(result).toContain("Warning: data loss possible:"); + expect(result).toContain( + "User.legacyId: Field removed (existing data will be dropped in the post-migration phase)", + ); + }); + + it("should format warnings without field (type-level)", () => { + const warnings: WarningChangeInfo[] = [ + { + typeName: "OldType", + reason: + "Type removed (all records of this type will be dropped in the post-migration phase)", + }, + ]; + const result = formatWarnings(warnings); + expect(result).toContain( + "OldType: Type removed (all records of this type will be dropped in the post-migration phase)", + ); + }); + + it("should format multiple warnings", () => { + const warnings: WarningChangeInfo[] = [ + { typeName: "User", fieldName: "legacyId", reason: "Field removed" }, + { typeName: "OldType", reason: "Type removed" }, + ]; + const result = formatWarnings(warnings); + expect(result).toContain("User.legacyId: Field removed"); + expect(result).toContain("OldType: Type removed"); + }); + }); + describe("formatDiffSummary", () => { it("should return 'No changes' for empty diff", () => { const diff = createDiff([]); diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/diff-calculator.ts b/packages/sdk/src/cli/commands/tailordb/migrate/diff-calculator.ts index 549908e06..2eff7adf1 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/diff-calculator.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/diff-calculator.ts @@ -70,6 +70,10 @@ export interface MigrationDiff { hasBreakingChanges: boolean; /** List of breaking changes */ breakingChanges: BreakingChangeInfo[]; + /** Whether there are non-breaking changes that may cause data loss (e.g. field/type removal) */ + hasWarnings: boolean; + /** List of non-breaking warnings */ + warnings: WarningChangeInfo[]; /** Whether a migration script is required to handle data migration */ requiresMigrationScript: boolean; } @@ -87,6 +91,20 @@ export interface BreakingChangeInfo { showThreeStepHint?: boolean; } +/** + * Warning change information in migration diff. + * + * Warnings are non-breaking changes that may still cause data loss + * (e.g. removing a field or type). Unlike breaking changes, a migration + * script is not required, but writing one is recommended if you need to + * preserve or transform data before the change applies. + */ +export interface WarningChangeInfo { + typeName: string; + fieldName?: string; + reason: string; +} + /** * Check if a migration diff has any changes * @param {MigrationDiff} diff - Migration diff to check @@ -276,6 +294,26 @@ export function formatBreakingChanges(breakingChanges: BreakingChangeInfo[]): st return lines.join("\n"); } +/** + * Format warning changes for display + * @param {WarningChangeInfo[]} warnings - Warning changes to format + * @returns {string} Formatted warning changes string + */ +export function formatWarnings(warnings: WarningChangeInfo[]): string { + if (warnings.length === 0) { + return ""; + } + + const lines: string[] = ["Warning: data loss possible:", ""]; + + for (const w of warnings) { + const location = w.fieldName ? `${w.typeName}.${w.fieldName}` : w.typeName; + lines.push(` - ${location}: ${w.reason}`); + } + + return lines.join("\n"); +} + const DIFF_CHANGE_LABELS: Record = { type_added: "type(s) added", type_removed: "type(s) removed", diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/generate.ts b/packages/sdk/src/cli/commands/tailordb/migrate/generate.ts index 433a7e489..fa132c0b4 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/generate.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/generate.ts @@ -24,6 +24,7 @@ import { formatMigrationDiff, formatBreakingChanges, formatDiffSummary, + formatWarnings, hasChanges, } from "./diff-calculator"; import { @@ -275,6 +276,12 @@ async function generateDiffFromSnapshot( } } + // Warn about non-breaking but data-loss-possible changes (e.g. field/type removal) + if (diff.hasWarnings) { + logger.newline(); + logger.warn(formatWarnings(diff.warnings)); + } + // Get next migration number const migrationNumber = getNextMigrationNumber(migrationsDir); @@ -320,6 +327,14 @@ async function generateDiffFromSnapshot( } catch { return; } + } else if (diff.hasWarnings) { + logger.newline(); + logger.log( + `Data loss is possible for this migration but no script was generated. To add a custom migrate.ts, run:`, + ); + logger.log( + ` ${styles.bold(`tailor-sdk tailordb migration script ${result.migrationNumber.toString().padStart(4, "0")} --namespace ${diff.namespace}`)}`, + ); } } diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/index.ts b/packages/sdk/src/cli/commands/tailordb/migrate/index.ts index 8601ef8a8..987952742 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/index.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/index.ts @@ -3,12 +3,14 @@ * * Subcommands: * - generate: Generate migration files from schema differences - * - set: Set migration checkpoint to a specific number - * - status: Show migration status for TailorDB namespaces + * - script: Add a migrate.ts template to an existing migration + * - set: Set migration checkpoint to a specific number + * - status: Show migration status for TailorDB namespaces */ import { defineCommand } from "politty"; import { generateCommand } from "./generate"; +import { scriptCommand } from "./script"; import { setCommand } from "./set"; import { statusCommand } from "./status"; @@ -17,6 +19,7 @@ export const migrationCommand = defineCommand({ description: "Manage TailorDB schema migrations.", subCommands: { generate: generateCommand, + script: scriptCommand, set: setCommand, status: statusCommand, }, @@ -24,6 +27,8 @@ export const migrationCommand = defineCommand({ export { generateCommand } from "./generate"; export type { GenerateOptions } from "./generate"; +export { scriptCommand } from "./script"; +export type { ScriptOptions } from "./script"; export { setCommand } from "./set"; export type { SetOptions } from "./set"; export { statusCommand } from "./status"; diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/pre-migration-schema.ts b/packages/sdk/src/cli/commands/tailordb/migrate/pre-migration-schema.ts new file mode 100644 index 000000000..dde430bdc --- /dev/null +++ b/packages/sdk/src/cli/commands/tailordb/migrate/pre-migration-schema.ts @@ -0,0 +1,147 @@ +/** + * Pre-migration field config adjustments + * + * The Pre-phase sends a "relaxed" version of the target schema so that + * `migrate.ts` scripts can still operate on the previous shape of the data. + * This module handles the field-level adjustments: + * + * - `field_removed`: re-insert the removed field so migrate.ts can read it + * (the physical drop happens in Post-phase). + * - `field_added` with `required: true`: relax to `required: false`. + * - `field_modified` optional→required, unique constraint added, enum + * value removed: keep the looser side until Post-phase. + * + * Type-level deletions (`type_removed`) are handled by the deploy flow, + * which retains the type until Post-phase rather than via this module. + * + * Post-phase then sends the final schema, after migrate.ts has had a chance + * to fix up data. + */ + +import { convertFieldConfigToProto } from "./snapshot-manifest"; +import type { DiffChange } from "./diff-calculator"; +import type { SnapshotFieldConfig } from "./snapshot"; +import type { PendingMigration } from "./types"; +import type { EnumValue } from "@/types/field-types"; +import type { MessageInitShape } from "@bufbuild/protobuf"; +import type { TailorDBType_FieldConfigSchema } from "@tailor-proto/tailor/v1/tailordb_resource_pb"; + +/** + * Diff change kinds that require pre-migration schema adjustments. + */ +const PRE_MIGRATION_FIELD_KINDS = new Set([ + "field_added", + "field_modified", + "field_removed", +]); + +/** + * Map of pre-migration field changes: typeName -> fieldName -> change. + * + * Includes both breaking changes (required-add, unique-add, enum value + * removal) and warning changes (field_removed). The Pre-phase needs to + * adjust the schema for both so that migrate.ts can still see the previous + * shape. + */ +export type PreMigrationChangesMap = Map>; + +/** + * Build a map of field changes that require pre-migration schema adjustment. + * @param {PendingMigration[]} pendingMigrations - Pending migrations to scan + * @returns {PreMigrationChangesMap} Map of changes keyed by typeName/fieldName + */ +export function buildPreMigrationChangesMap( + pendingMigrations: PendingMigration[], +): PreMigrationChangesMap { + const map: PreMigrationChangesMap = new Map(); + for (const migration of pendingMigrations) { + for (const change of migration.diff.changes) { + if (!PRE_MIGRATION_FIELD_KINDS.has(change.kind)) continue; + if (!change.fieldName) continue; + const perType = map.get(change.typeName) ?? new Map(); + perType.set(change.fieldName, change); + map.set(change.typeName, perType); + } + } + return map; +} + +/** + * Field config subset for pre-migration adjustment heuristics. + */ +interface FieldConfig { + required?: boolean; + unique?: boolean; + allowedValues?: EnumValue[]; +} + +/** + * Apply pre-migration schema adjustments to a single field map in place. + * + * The fields map is the proto-shape `TailorDBType.schema.fields` that will + * be sent in the Pre-phase. We mutate it so that: + * + * - Removed fields are re-inserted using their pre-migration config. + * - Newly added required fields are relaxed to optional. + * - Modified fields keep the looser side of unique/required/enum. + * + * @param {Record>} fields - Field map to adjust (mutated in place) + * @param {Map} typeChanges - Changes for this type, keyed by fieldName + */ +export function applyPreMigrationFieldAdjustments( + fields: Record>, + typeChanges: Map, +): void { + for (const [fieldName, change] of typeChanges) { + if (change.kind === "field_removed") { + const before = change.before as SnapshotFieldConfig | undefined; + if (before) { + fields[fieldName] = convertFieldConfigToProto(before); + } + continue; + } + + const field = fields[fieldName]; + if (!field) continue; + + const before = change.before as FieldConfig | undefined; + const after = change.after as FieldConfig | undefined; + + if (change.kind === "field_added" && after?.required) { + field.required = false; + continue; + } + + if (change.kind !== "field_modified") { + continue; + } + + if (!before?.required && after?.required) { + field.required = false; + } + + if (!(before?.unique ?? false) && (after?.unique ?? false)) { + field.unique = false; + } + + if (before?.allowedValues && after?.allowedValues) { + const afterValues = new Set(after.allowedValues.map((v) => v.value)); + const removedValues = before.allowedValues.filter((v) => !afterValues.has(v.value)); + if (removedValues.length > 0) { + const valueMap = new Map(); + for (const v of before.allowedValues) { + valueMap.set(v.value, v.description ?? ""); + } + for (const v of after.allowedValues) { + if (!valueMap.has(v.value)) { + valueMap.set(v.value, v.description ?? ""); + } + } + field.allowedValues = Array.from(valueMap.entries()).map(([value, description]) => ({ + value, + description, + })); + } + } + } +} diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/script.ts b/packages/sdk/src/cli/commands/tailordb/migrate/script.ts new file mode 100644 index 000000000..9cfae3bf8 --- /dev/null +++ b/packages/sdk/src/cli/commands/tailordb/migrate/script.ts @@ -0,0 +1,172 @@ +/** + * Script command for TailorDB migrations + * + * Adds a `migrate.ts` (and supporting `db.ts`) template to an existing + * migration directory. Useful for warning-tier changes where users may + * want to write a custom data migration even though the change does not + * automatically require one. + */ + +import * as fs from "node:fs"; +import * as fsPromises from "node:fs/promises"; +import * as path from "pathe"; +import { arg } from "politty"; +import { z } from "zod"; +import { configArg } from "@/cli/shared/args"; +import { logBetaWarning } from "@/cli/shared/beta"; +import { defineAppCommand } from "@/cli/shared/command"; +import { loadConfig } from "@/cli/shared/config-loader"; +import { getConfiguredEditorCommand, openInConfiguredEditor } from "@/cli/shared/editor"; +import { logger, styles } from "@/cli/shared/logger"; +import { getNamespacesWithMigrations, type NamespaceWithMigrations } from "./config"; +import { writeDbTypesFile } from "./db-types-generator"; +import { + getMigrationFilePath, + isValidMigrationNumber, + loadDiff, + reconstructSnapshotFromMigrations, + INITIAL_SCHEMA_NUMBER, +} from "./snapshot"; +import { generateMigrationScript } from "./template-generator"; + +export interface ScriptOptions { + configPath?: string; + number: string; + namespace?: string; +} + +/** + * Add a migrate.ts template to an existing migration directory. + * @param {ScriptOptions} options - Command options + */ +async function script(options: ScriptOptions): Promise { + logBetaWarning("tailordb migration"); + + // Accept either the canonical 4-digit form ("0001") or a bare integer + // ("1"–"9999"). Reject inputs containing non-digit characters, integer + // forms with leading zeros ("00001"), and anything outside the + // 0000-9999 directory range that the migrations system supports. + let migrationNumber: number; + if (isValidMigrationNumber(options.number)) { + migrationNumber = parseInt(options.number, 10); + } else if (/^[1-9]\d*$/.test(options.number)) { + migrationNumber = parseInt(options.number, 10); + if (migrationNumber > 9999) { + throw new Error(`Migration number ${options.number} is out of range. Expected 1-9999.`); + } + } else { + throw new Error( + `Invalid migration number format: ${options.number}. Expected 4-digit format (e.g., 0001) or integer 1-9999 (e.g., 1).`, + ); + } + + if (migrationNumber === INITIAL_SCHEMA_NUMBER) { + throw new Error( + `Migration ${options.number} is the initial schema snapshot and cannot have a migration script.`, + ); + } + + const { config } = await loadConfig(options.configPath); + const configDir = path.dirname(config.path); + + const namespacesWithMigrations = getNamespacesWithMigrations(config, configDir); + if (namespacesWithMigrations.length === 0) { + throw new Error("No TailorDB services with migrations configuration found"); + } + + const targetNamespace = resolveTargetNamespace(namespacesWithMigrations, options.namespace); + const { migrationsDir } = namespacesWithMigrations.find( + (ns) => ns.namespace === targetNamespace, + )!; + + const diffPath = getMigrationFilePath(migrationsDir, migrationNumber, "diff"); + if (!fs.existsSync(diffPath)) { + throw new Error( + `Migration ${options.number} not found in ${migrationsDir}. Expected ${diffPath}.`, + ); + } + + const migratePath = getMigrationFilePath(migrationsDir, migrationNumber, "migrate"); + if (fs.existsSync(migratePath)) { + throw new Error(`Migration script already exists at ${migratePath}.`); + } + + const diff = loadDiff(diffPath); + + // Reconstruct the schema state immediately before this migration so that + // db.ts has Kysely types for the previous shape of the data. + const previousSnapshot = reconstructSnapshotFromMigrations(migrationsDir, migrationNumber - 1); + if (!previousSnapshot) { + throw new Error( + `Could not reconstruct previous schema for migration ${options.number}. Make sure migration ${INITIAL_SCHEMA_NUMBER} exists.`, + ); + } + + const scriptContent = generateMigrationScript(diff); + await fsPromises.writeFile(migratePath, scriptContent); + await writeDbTypesFile(previousSnapshot, migrationsDir, migrationNumber, diff); + + logger.success( + `Added migration script for migration ${styles.bold(options.number)} in namespace ${styles.bold(targetNamespace)}`, + ); + logger.info(` Migration script: ${migratePath}`); + logger.info(` DB types: ${getMigrationFilePath(migrationsDir, migrationNumber, "db")}`); + + logger.newline(); + logger.log("Edit the script to implement your data migration logic."); + logger.log("It will be executed by 'tailor-sdk deploy' between Pre and Post phases."); + + const editor = getConfiguredEditorCommand(); + if (!editor) return; + + logger.newline(); + logger.info(`Opening ${path.basename(migratePath)} in ${editor}...`); + try { + await openInConfiguredEditor(migratePath); + } catch { + return; + } +} + +function resolveTargetNamespace( + namespacesWithMigrations: NamespaceWithMigrations[], + requested?: string, +): string { + if (requested) { + if (!namespacesWithMigrations.some((ns) => ns.namespace === requested)) { + throw new Error(`Namespace "${requested}" not found or does not have migrations configured`); + } + return requested; + } + if (namespacesWithMigrations.length === 1) { + return namespacesWithMigrations[0].namespace; + } + throw new Error( + `Multiple TailorDB services found. Please specify namespace with --namespace flag: ${namespacesWithMigrations.map((ns) => ns.namespace).join(", ")}`, + ); +} + +export const scriptCommand = defineAppCommand({ + name: "script", + description: "Add a migration script (migrate.ts) template to an existing migration directory.", + args: z + .object({ + ...configArg, + number: arg(z.string(), { + positional: true, + description: "Migration number to add a script to (e.g., 0001 or 1)", + }), + namespace: arg(z.string().optional(), { + alias: "n", + description: "Target TailorDB namespace (required if multiple namespaces exist)", + }), + }) + .strict(), + run: async (args) => { + await script({ + configPath: args.config, + number: args.number, + namespace: args.namespace, + }); + }, +}); diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/snapshot-manifest.ts b/packages/sdk/src/cli/commands/tailordb/migrate/snapshot-manifest.ts index 6380b8e27..2d19a7567 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/snapshot-manifest.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/snapshot-manifest.ts @@ -176,7 +176,7 @@ export function generateTailorDBTypeManifestFromSnapshot( * @param {SnapshotFieldConfig} config - Snapshot field config * @returns {MessageInitShape} Proto field config */ -function convertFieldConfigToProto( +export function convertFieldConfigToProto( config: SnapshotFieldConfig, ): MessageInitShape { const fieldEntry: MessageInitShape = { diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.test.ts b/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.test.ts index 955be9986..147c65fdb 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.test.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.test.ts @@ -250,6 +250,14 @@ describe("snapshot", () => { expect(diff.changes[0].kind).toBe("type_removed"); expect(diff.hasBreakingChanges).toBe(false); expect(diff.requiresMigrationScript).toBe(false); + expect(diff.hasWarnings).toBe(true); + expect(diff.warnings).toEqual([ + { + typeName: "OldType", + reason: + "Type removed (all records of this type will be dropped in the post-migration phase)", + }, + ]); }); it("detects field addition (optional - non-breaking)", () => { @@ -339,6 +347,14 @@ describe("snapshot", () => { expect(diff.changes[0].kind).toBe("field_removed"); expect(diff.hasBreakingChanges).toBe(false); expect(diff.requiresMigrationScript).toBe(false); + expect(diff.hasWarnings).toBe(true); + expect(diff.warnings).toEqual([ + { + typeName: "User", + fieldName: "name", + reason: "Field removed (existing data will be dropped in the post-migration phase)", + }, + ]); }); it("detects field type change (breaking change)", () => { @@ -764,6 +780,8 @@ describe("snapshot", () => { changes: [], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -793,6 +811,8 @@ describe("snapshot", () => { changes: [], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -853,6 +873,8 @@ describe("snapshot", () => { changes: [], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -885,6 +907,8 @@ describe("snapshot", () => { changes: [], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -933,6 +957,8 @@ describe("snapshot", () => { changes: [{ kind: "type_added", typeName: "NewType" }], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -944,6 +970,59 @@ describe("snapshot", () => { expect(loaded.changes.length).toBe(1); expect(loaded.changes[0].kind).toBe("type_added"); }); + + it("backfills warnings fields for legacy diff.json", () => { + // Legacy diff.json written before warning-tier support shipped. The file + // has no warnings/hasWarnings keys at all. + const legacyDiff = { + version: SCHEMA_SNAPSHOT_VERSION, + namespace, + createdAt: new Date().toISOString(), + changes: [{ kind: "type_added", typeName: "NewType" }], + hasBreakingChanges: false, + breakingChanges: [], + requiresMigrationScript: false, + }; + + const filePath = path.join(testDir, "legacy_diff.json"); + fs.writeFileSync(filePath, JSON.stringify(legacyDiff, null, 2)); + + const loaded = loadDiff(filePath); + + expect(loaded.warnings).toEqual([]); + expect(loaded.hasWarnings).toBe(false); + expect(loaded.changes.length).toBe(1); + }); + + it("derives hasWarnings from warnings array regardless of stored flag", () => { + // A hand-edited diff.json could end up with mismatched warnings and + // hasWarnings; the loader must reconcile to the array. + const inconsistentDiff = { + version: SCHEMA_SNAPSHOT_VERSION, + namespace, + createdAt: new Date().toISOString(), + changes: [], + hasBreakingChanges: false, + breakingChanges: [], + hasWarnings: false, + warnings: [ + { + kind: "field_removed", + typeName: "Product", + fieldName: "legacyCode", + }, + ], + requiresMigrationScript: false, + }; + + const filePath = path.join(testDir, "inconsistent_diff.json"); + fs.writeFileSync(filePath, JSON.stringify(inconsistentDiff, null, 2)); + + const loaded = loadDiff(filePath); + + expect(loaded.warnings.length).toBe(1); + expect(loaded.hasWarnings).toBe(true); + }); }); describe("writeSnapshot", () => { @@ -976,6 +1055,8 @@ describe("snapshot", () => { changes: [], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -1043,6 +1124,8 @@ describe("snapshot", () => { ], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -1082,6 +1165,8 @@ describe("snapshot", () => { ], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -1099,6 +1184,8 @@ describe("snapshot", () => { ], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -1145,6 +1232,8 @@ describe("snapshot", () => { ], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -1191,6 +1280,8 @@ describe("snapshot", () => { ], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -1261,6 +1352,8 @@ describe("snapshot", () => { ], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -1321,6 +1414,8 @@ describe("snapshot", () => { changes: [], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -1340,6 +1435,8 @@ describe("snapshot", () => { changes: [], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; writeDiffToDir(testDir, 1, diff); @@ -1368,6 +1465,8 @@ describe("snapshot", () => { changes: [], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -1447,6 +1546,8 @@ describe("snapshot", () => { changes: [], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; // Missing 0000/schema.json diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.ts b/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.ts index 0becbf7dd..274d35ceb 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.ts @@ -8,6 +8,7 @@ import { type MigrationDiff, type DiffChange, type BreakingChangeInfo, + type WarningChangeInfo, SCHEMA_SNAPSHOT_VERSION, } from "./diff-calculator"; import type { SchemaDrift } from "./types"; @@ -679,7 +680,17 @@ export function loadSnapshot(filePath: string): SchemaSnapshot { */ export function loadDiff(filePath: string): MigrationDiff { const content = fs.readFileSync(filePath, "utf-8"); - return JSON.parse(content) as MigrationDiff; + const parsed = JSON.parse(content) as MigrationDiff; + // Backfill fields introduced after the initial diff.json schema so that older + // migrations on disk remain readable without manual edits. hasWarnings is + // derived from the warnings array to stay consistent even if a hand-edited + // diff.json sets one side without the other. + const warnings = parsed.warnings ?? []; + return { + ...parsed, + warnings, + hasWarnings: warnings.length > 0, + }; } /** @@ -1155,11 +1166,12 @@ function isBreakingFieldChange( } /** - * Context for collecting diff changes and breaking changes + * Context for collecting diff changes, breaking changes, and warnings */ interface DiffContext { changes: DiffChange[]; breakingChanges: BreakingChangeInfo[]; + warnings: WarningChangeInfo[]; } function addChange( @@ -1170,11 +1182,23 @@ function addChange( ): void { ctx.changes.push(change); - if (change.fieldName) { - const breaking = isBreakingFieldChange(change.typeName, change.fieldName, oldField, newField); - if (breaking) { - ctx.breakingChanges.push(breaking); - } + if (!change.fieldName) return; + + const breaking = isBreakingFieldChange(change.typeName, change.fieldName, oldField, newField); + if (breaking) { + ctx.breakingChanges.push(breaking); + return; + } + + // Non-breaking removal still risks data loss: surface as a warning so users + // can decide whether to add a migration script (e.g. JOIN through a + // soon-to-be-dropped foreign key before it disappears). + if (change.kind === "field_removed") { + ctx.warnings.push({ + typeName: change.typeName, + fieldName: change.fieldName, + reason: "Field removed (existing data will be dropped in the post-migration phase)", + }); } } @@ -1492,7 +1516,7 @@ function comparePermissions( * @returns {MigrationDiff} Migration diff between snapshots */ export function compareSnapshots(previous: SchemaSnapshot, current: SchemaSnapshot): MigrationDiff { - const ctx: DiffContext = { changes: [], breakingChanges: [] }; + const ctx: DiffContext = { changes: [], breakingChanges: [], warnings: [] }; const previousTypeNames = new Set(Object.keys(previous.types)); const currentTypeNames = new Set(Object.keys(current.types)); @@ -1516,6 +1540,11 @@ export function compareSnapshots(previous: SchemaSnapshot, current: SchemaSnapsh typeName, before: previous.types[typeName], }); + ctx.warnings.push({ + typeName, + reason: + "Type removed (all records of this type will be dropped in the post-migration phase)", + }); } } @@ -1569,6 +1598,8 @@ export function compareSnapshots(previous: SchemaSnapshot, current: SchemaSnapsh changes: ctx.changes, hasBreakingChanges: ctx.breakingChanges.length > 0, breakingChanges: ctx.breakingChanges, + hasWarnings: ctx.warnings.length > 0, + warnings: ctx.warnings, requiresMigrationScript: ctx.breakingChanges.length > 0, }; } diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/template-generator.test.ts b/packages/sdk/src/cli/commands/tailordb/migrate/template-generator.test.ts index 481e42d51..dc719a092 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/template-generator.test.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/template-generator.test.ts @@ -135,6 +135,8 @@ describe("template-generator", () => { ], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -173,6 +175,8 @@ describe("template-generator", () => { reason: "Required field added", }, ], + hasWarnings: false, + warnings: [], requiresMigrationScript: true, }; @@ -201,6 +205,8 @@ describe("template-generator", () => { changes: [], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -242,6 +248,8 @@ describe("template-generator", () => { ], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -289,6 +297,8 @@ describe("template-generator", () => { reason: "Unique constraint added to field", }, ], + hasWarnings: false, + warnings: [], requiresMigrationScript: true, }; @@ -349,6 +359,8 @@ describe("template-generator", () => { reason: "Enum values removed: CANCELLED", }, ], + hasWarnings: false, + warnings: [], requiresMigrationScript: true, }; @@ -370,6 +382,8 @@ describe("template-generator", () => { changes: [], hasBreakingChanges: false, breakingChanges: [], + hasWarnings: false, + warnings: [], requiresMigrationScript: false, }; @@ -403,6 +417,8 @@ describe("template-generator", () => { reason: "Required field added", }, ], + hasWarnings: false, + warnings: [], requiresMigrationScript: true, }; diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/template-generator.ts b/packages/sdk/src/cli/commands/tailordb/migrate/template-generator.ts index 06b027695..a9dfb515d 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/template-generator.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/template-generator.ts @@ -150,7 +150,7 @@ export async function generateDiffFiles( * @param {MigrationDiff} diff - Migration diff * @returns {string} Migration script content */ -function generateMigrationScript(diff: MigrationDiff): string { +export function generateMigrationScript(diff: MigrationDiff): string { const updates: string[] = []; for (const change of diff.changes) { @@ -168,8 +168,11 @@ function generateMigrationScript(diff: MigrationDiff): string { return `/** * Migration script for ${diff.namespace} * - * This script handles data migration for breaking schema changes. - * Edit this file to implement your data migration logic. + * This script runs between the Pre-migration and Post-migration phases of + * 'tailor-sdk deploy'. Use it to transform existing data so that the schema + * change can complete safely (for breaking changes, this is hard-required; + * for warning-tier changes it is optional). Edit this file to implement + * your data migration logic. * * The transaction is managed by the deploy command. * If any operation fails, all changes will be rolled back. diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/types.ts b/packages/sdk/src/cli/commands/tailordb/migrate/types.ts index 9f0c7e83b..e3e551b7e 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/types.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/types.ts @@ -49,8 +49,10 @@ export const SCHEMA_ERROR_PATTERNS = [ export interface PendingMigration { /** Migration number */ number: number; - /** Path to migration script file */ + /** Path to migration script file (may not exist on disk) */ scriptPath: string; + /** Whether a migration script file exists on disk for this migration */ + hasScript: boolean; /** Path to diff file */ diffPath: string; /** Namespace this migration belongs to */ diff --git a/packages/sdk/src/cli/shared/readonly-guard.test.ts b/packages/sdk/src/cli/shared/readonly-guard.test.ts index 14e514f95..44fa6b645 100644 --- a/packages/sdk/src/cli/shared/readonly-guard.test.ts +++ b/packages/sdk/src/cli/shared/readonly-guard.test.ts @@ -108,6 +108,7 @@ const READ_OR_LOCAL_COMMAND_PATHS = new Set([ "tailordb/erd/serve.ts", "tailordb/migrate/index.ts", "tailordb/migrate/generate.ts", + "tailordb/migrate/script.ts", "tailordb/migrate/status.ts", // Upgrade (local SDK upgrade) "upgrade/index.ts",