From dee2ad7199b2da238b7e9edea2d9bb9605033456 Mon Sep 17 00:00:00 2001 From: Akira HIGUCHI Date: Fri, 15 May 2026 22:13:21 +0900 Subject: [PATCH 1/3] refactor(sdk): unify TailorDB deploy pipeline on snapshot schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Route the deploy plan/apply phases through SnapshotType — the same canonical shape used by tailordb migrate — so all comparisons and plan inputs share one normalized representation. Decimal scale is now normalized to the platform default (6) at the snapshot boundary (createSnapshotType, loadSnapshot, convertRemoteFieldsToSnapshot), replacing the per-comparison getEffectiveScale workaround. This consolidates schema normalization in one place (preventing recurrences like the decimal scale drift fixed in #1155) and prepares for a future tailordb migration sync command. --- .../unify-tailordb-deploy-on-snapshot.md | 5 + .../src/cli/commands/deploy/deploy.test.ts | 1 + .../commands/deploy/tailordb/index.test.ts | 1 + .../src/cli/commands/deploy/tailordb/index.ts | 164 +++++++++++------- .../tailordb/migrate/snapshot.test.ts | 20 ++- .../cli/commands/tailordb/migrate/snapshot.ts | 68 +++++--- 6 files changed, 165 insertions(+), 94 deletions(-) create mode 100644 .changeset/unify-tailordb-deploy-on-snapshot.md diff --git a/.changeset/unify-tailordb-deploy-on-snapshot.md b/.changeset/unify-tailordb-deploy-on-snapshot.md new file mode 100644 index 000000000..58aa94d34 --- /dev/null +++ b/.changeset/unify-tailordb-deploy-on-snapshot.md @@ -0,0 +1,5 @@ +--- +"@tailor-platform/sdk": patch +--- + +Unify the `tailor deploy` TailorDB pipeline on the snapshot schema. All plan and apply phases now consume the same canonical snapshot-shaped input that is also used by `tailordb migrate`, with decimal scale normalized to the platform default (`6`) at the snapshot boundary. This consolidates normalization in one place instead of relying on per-comparison workarounds (such as the decimal-scale fix in #1155) and prepares for a future `tailordb migration sync` command. No behavior change for SDK users. diff --git a/packages/sdk/src/cli/commands/deploy/deploy.test.ts b/packages/sdk/src/cli/commands/deploy/deploy.test.ts index c43cdd140..3293db900 100644 --- a/packages/sdk/src/cli/commands/deploy/deploy.test.ts +++ b/packages/sdk/src/cli/commands/deploy/deploy.test.ts @@ -47,6 +47,7 @@ function emptyResults(): PlanResults { context: { workspaceId: "ws", application: {} as PlanResults["tailorDB"]["context"]["application"], + tailorDBInputs: [], config: {} as PlanResults["tailorDB"]["context"]["config"], noSchemaCheck: false, }, 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 ee282b4db..0a671ed2c 100644 --- a/packages/sdk/src/cli/commands/deploy/tailordb/index.test.ts +++ b/packages/sdk/src/cli/commands/deploy/tailordb/index.test.ts @@ -769,6 +769,7 @@ describe("applyTailorDB phase separation", () => { name: "test-app", tailorDBServices: [mockTailorDBService], } as unknown as Application, + tailorDBInputs: [], config: mockConfig, noSchemaCheck: true, // Skip migration checks in unit tests }, diff --git a/packages/sdk/src/cli/commands/deploy/tailordb/index.ts b/packages/sdk/src/cli/commands/deploy/tailordb/index.ts index 5f9649565..0280fdee6 100644 --- a/packages/sdk/src/cli/commands/deploy/tailordb/index.ts +++ b/packages/sdk/src/cli/commands/deploy/tailordb/index.ts @@ -53,6 +53,15 @@ import { formatMigrationNumber, compareRemoteWithSnapshot, formatSchemaDrifts, + createSnapshotType, + type SnapshotFieldConfig, + type SnapshotType, + type SnapshotRecordPermission, + type SnapshotActionPermission, + type SnapshotPermissionCondition, + type SnapshotPermissionOperand, + type SnapshotGqlPermission, + type SnapshotGqlPermissionPolicy, } from "@/cli/commands/tailordb/migrate/snapshot"; import { type TailorDBService } from "@/cli/services/tailordb/service"; import { fetchAll, type OperatorClient } from "@/cli/shared/client"; @@ -82,16 +91,6 @@ import type { 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, - StandardGqlPermissionPolicy, - StandardPermissionCondition, - StandardTailorTypeGqlPermission, - StandardTailorTypePermission, - OperatorFieldConfig, - TailorDBType, -} from "@/types/tailordb"; import type { GqlOperations, TailorDBServiceConfig } from "@/types/tailordb.generated"; import type { SetMetadataRequestSchema } from "@tailor-proto/tailor/v1/metadata_pb"; @@ -245,7 +244,7 @@ function formatRemoteVerificationResults(results: RemoteSchemaVerificationResult * Validate migration files and detect pending migrations * @param {OperatorClient} client - Operator client instance * @param {string} workspaceId - Workspace ID - * @param {ReadonlyMap>} typesByNamespace - Types by namespace + * @param {ReadonlyMap>} typesByNamespace - Types by namespace * @param {LoadedConfig} config - Loaded application config (includes path) * @param {boolean} noSchemaCheck - Whether to skip schema diff check * @returns {Promise} List of pending migrations @@ -253,7 +252,7 @@ function formatRemoteVerificationResults(results: RemoteSchemaVerificationResult async function validateAndDetectMigrations( client: OperatorClient, workspaceId: string, - typesByNamespace: ReadonlyMap>, + typesByNamespace: ReadonlyMap>, config: LoadedConfig, noSchemaCheck: boolean, ): Promise { @@ -392,13 +391,10 @@ export async function applyTailorDB( if (phase === "create-update") { // Validate and detect migrations - // Build types by namespace map - const typesByNamespace = new Map>(); - for (const tailordb of migrationContext.application.tailorDBServices) { - const types = tailordb.types; - if (types) { - typesByNamespace.set(tailordb.namespace, types); - } + // Build types by namespace map (snapshot-shaped, the canonical deploy form) + const typesByNamespace = new Map>(); + for (const tailordb of migrationContext.tailorDBInputs) { + typesByNamespace.set(tailordb.namespace, tailordb.types); } const pendingMigrations = await validateAndDetectMigrations( @@ -973,6 +969,36 @@ async function executeSingleMigrationPostPhase( * @param context - Planning context * @returns Planned changes */ +/** + * Canonical input shape consumed by every TailorDB plan/proto step. + * The deploy pipeline funnels `TailorDBService` through `createSnapshotType` so + * that comparison, manifest generation and migration drift checks all read the + * same snapshot-shaped data, keeping platform-side normalization (e.g. decimal + * scale) in one place. + */ +type TailorDBDeployInput = { + namespace: string; + config: TailorDBServiceConfig; + types: Record; +}; + +/** + * Convert a runtime TailorDBService to the snapshot-shaped deploy input. + * @param service - Loaded TailorDB service (after `loadTypes()`) + * @returns The canonical snapshot-shaped deploy input for downstream plan/apply phases. + */ +function toTailorDBDeployInput(service: TailorDBService): TailorDBDeployInput { + const types: Record = {}; + for (const [typeName, type] of Object.entries(service.types)) { + types[typeName] = createSnapshotType(type); + } + return { + namespace: service.namespace, + config: service.config, + types, + }; +} + export async function planTailorDB(context: PlanContext) { const { client, @@ -983,11 +1009,11 @@ export async function planTailorDB(context: PlanContext) { noSchemaCheck, forceApplyAll = false, } = context; - const tailordbs: TailorDBService[] = []; + const tailordbs: TailorDBDeployInput[] = []; if (!forRemoval) { for (const tailordb of application.tailorDBServices) { await tailordb.loadTypes(); - tailordbs.push(tailordb); + tailordbs.push(toTailorDBDeployInput(tailordb)); } } const executors = forRemoval @@ -1018,6 +1044,7 @@ export async function planTailorDB(context: PlanContext) { context: { workspaceId, application, + tailorDBInputs: tailordbs, config, noSchemaCheck: noSchemaCheck ?? false, }, @@ -1134,7 +1161,7 @@ function areTailorDBServicesEqual( namespace?: { name?: string }; defaultTimezone?: string; }, - desired: Readonly, + desired: Readonly, ): boolean { return areNormalizedEqual( normalizeComparableTailorDBService({ @@ -1152,7 +1179,7 @@ async function planServices( client: OperatorClient, workspaceId: string, appName: string, - tailordbs: ReadonlyArray, + tailordbs: ReadonlyArray, ) { const changeSet = createChangeSet( "TailorDB services", @@ -1278,10 +1305,10 @@ type DeleteType = { async function planTypes( client: OperatorClient, workspaceId: string, - tailordbs: ReadonlyArray, + tailordbs: ReadonlyArray, executors: ReadonlyArray, deletedServices: ReadonlyArray, - filteredTypesByNamespace?: Map>, + filteredTypesByNamespace?: Map>, forceApplyAll = false, ) { const changeSet = createChangeSet("TailorDB types"); @@ -1530,19 +1557,23 @@ function isNumericLikeValue(value: string | number | bigint): boolean { // TODO(remiposo): Copied the type-processor / aggregator processing almost as-is. // This will need refactoring later. /** - * Generate a TailorDB type manifest from parsed type - * @param {TailorDBType} type - Parsed TailorDB type + * Generate a TailorDB type manifest from snapshot-shaped type + * @param {SnapshotType} type - Snapshot-shaped TailorDB type * @param {ReadonlySet} executorUsedTypes - Set of types used by executors * @param {GqlOperations} [namespaceGqlOperations] - Default gqlOperations for the namespace (already normalized) * @returns {MessageInitShape} Type manifest */ function generateTailorDBTypeManifest( - type: TailorDBType, + type: SnapshotType, executorUsedTypes: ReadonlySet, namespaceGqlOperations?: GqlOperations, ): MessageInitShape { + // pluralForm is always populated by the parser when building from configure, + // but stored snapshots (e.g. replayed by a future sync command) may omit it. + // Fall back to inflection so this path stays robust either way. + const rawPluralForm = type.pluralForm ?? inflection.pluralize(type.name); // This ensures that explicitly provided pluralForm like "PurchaseOrderList" becomes "purchaseOrderList" - const pluralForm = inflection.camelize(type.pluralForm, true); + const pluralForm = inflection.camelize(rawPluralForm, true); const defaultSettings: { aggregation: boolean; @@ -1593,7 +1624,7 @@ function generateTailorDBTypeManifest( Object.keys(type.fields) .filter((fieldName) => fieldName !== "id") .forEach((fieldName) => { - const fieldConfig = type.fields[fieldName].config; + const fieldConfig = type.fields[fieldName]; const fieldType = fieldConfig.type; const fieldEntry: MessageInitShape = { type: fieldType, @@ -1606,7 +1637,7 @@ function generateTailorDBTypeManifest( foreignKey: fieldConfig.foreignKey || false, foreignKeyType: fieldConfig.foreignKeyType, foreignKeyField: fieldConfig.foreignKeyField, - required: fieldConfig.required !== false, + required: fieldConfig.required, vector: fieldConfig.vector || false, ...toProtoFieldHooks(fieldConfig), ...(fieldConfig.serial && { @@ -1636,7 +1667,7 @@ function generateTailorDBTypeManifest( MessageInitShape > = {}; - for (const [relationName, rel] of Object.entries(type.forwardRelationships)) { + for (const [relationName, rel] of Object.entries(type.forwardRelationships ?? {})) { relationships[relationName] = { refType: rel.targetType, refField: rel.sourceField, @@ -1646,7 +1677,7 @@ function generateTailorDBTypeManifest( }; } - for (const [relationName, rel] of Object.entries(type.backwardRelationships)) { + for (const [relationName, rel] of Object.entries(type.backwardRelationships ?? {})) { relationships[relationName] = { refType: rel.targetType, refField: rel.targetField, @@ -1683,7 +1714,7 @@ function generateTailorDBTypeManifest( update: [], delete: [], }; - const permission = type.permissions.record + const permission = type.permissions?.record ? protoPermission(type.permissions.record) : defaultPermission; @@ -1704,7 +1735,7 @@ function generateTailorDBTypeManifest( } function toProtoFieldValidate( - fieldConfig: OperatorFieldConfig, + fieldConfig: SnapshotFieldConfig, ): MessageInitShape["validate"] { return (fieldConfig.validate || []).map((val) => ({ action: TailorDBType_PermitAction.DENY, @@ -1718,7 +1749,7 @@ function toProtoFieldValidate( } function toProtoFieldHooks( - fieldConfig: OperatorFieldConfig, + fieldConfig: SnapshotFieldConfig, ): Pick, "hooks"> | Record { if (!fieldConfig.hooks) { return {}; @@ -1740,7 +1771,7 @@ function toProtoFieldHooks( } function processNestedFields( - fields: Record, + fields: Record, ): Record> { const nestedFields: Record> = {}; @@ -1754,7 +1785,7 @@ function processNestedFields( allowedValues: nestedFieldConfig.allowedValues || [], description: nestedFieldConfig.description || "", validate: toProtoFieldValidate(nestedFieldConfig), - required: nestedFieldConfig.required ?? true, + required: nestedFieldConfig.required, array: nestedFieldConfig.array ?? false, index: false, unique: false, @@ -1772,7 +1803,7 @@ function processNestedFields( allowedValues: nestedType === "enum" ? nestedFieldConfig.allowedValues || [] : [], description: nestedFieldConfig.description || "", validate: toProtoFieldValidate(nestedFieldConfig), - required: nestedFieldConfig.required ?? true, + required: nestedFieldConfig.required, array: nestedFieldConfig.array ?? false, index: false, unique: false, @@ -1801,17 +1832,18 @@ function processNestedFields( } function protoPermission( - permission: StandardTailorTypePermission, + permission: SnapshotRecordPermission, ): MessageInitShape { - const ret: MessageInitShape = {}; - for (const [key, policies] of Object.entries(permission)) { - ret[key as keyof StandardTailorTypePermission] = policies.map((policy) => protoPolicy(policy)); - } - return ret; + return { + create: permission.create.map((policy) => protoPolicy(policy)), + read: permission.read.map((policy) => protoPolicy(policy)), + update: permission.update.map((policy) => protoPolicy(policy)), + delete: permission.delete.map((policy) => protoPolicy(policy)), + }; } function protoPolicy( - policy: StandardActionPermission<"record">, + policy: SnapshotActionPermission, ): MessageInitShape { let permit: TailorDBType_Permission_Permit; switch (policy.permit) { @@ -1832,7 +1864,7 @@ function protoPolicy( } function protoCondition( - condition: StandardPermissionCondition<"record">, + condition: SnapshotPermissionCondition, ): MessageInitShape { const [left, operator, right] = condition; @@ -1869,46 +1901,44 @@ function protoCondition( } function protoOperand( - operand: PermissionOperand, + operand: SnapshotPermissionOperand, ): MessageInitShape { - if (typeof operand === "object" && !Array.isArray(operand)) { - if ("user" in operand) { + if (typeof operand === "object" && operand !== null && !Array.isArray(operand)) { + if ("user" in operand && typeof operand.user === "string") { return { kind: { case: "userField", value: operand.user, }, }; - } else if ("record" in operand) { + } else if ("record" in operand && typeof operand.record === "string") { return { kind: { case: "recordField", value: operand.record, }, }; - } else if ("newRecord" in operand) { + } else if ("newRecord" in operand && typeof operand.newRecord === "string") { return { kind: { case: "newRecordField", value: operand.newRecord, }, }; - } else if ("oldRecord" in operand) { + } else if ("oldRecord" in operand && typeof operand.oldRecord === "string") { return { kind: { case: "oldRecordField", value: operand.oldRecord, }, }; - } else { - throw new Error(`Unknown operand: ${JSON.stringify(operand)}`); } } return { kind: { case: "value", - value: fromJson(ValueSchema, operand), + value: fromJson(ValueSchema, operand as Parameters[1]), }, }; } @@ -1931,7 +1961,7 @@ type DeleteGqlPermission = { async function planGqlPermissions( client: OperatorClient, workspaceId: string, - tailordbs: ReadonlyArray, + tailordbs: ReadonlyArray, deletedServices: ReadonlyArray, forceApplyAll = false, ) { @@ -1967,7 +1997,7 @@ async function planGqlPermissions( const types = tailordb.types; for (const typeName of Object.keys(types)) { - const gqlPermission = types[typeName].permissions.gql; + const gqlPermission = types[typeName].permissions?.gql; if (!gqlPermission) { continue; } @@ -2054,7 +2084,7 @@ function normalizeComparableGqlPermission(permission: unknown) { } function protoGqlPermission( - permission: StandardTailorTypeGqlPermission, + permission: SnapshotGqlPermission, ): MessageInitShape { return { policies: permission.map((policy) => protoGqlPolicy(policy)), @@ -2062,7 +2092,7 @@ function protoGqlPermission( } function protoGqlPolicy( - policy: StandardGqlPermissionPolicy, + policy: SnapshotGqlPermissionPolicy, ): MessageInitShape { const actions: TailorDBGQLPermission_Action[] = []; for (const action of policy.actions) { @@ -2112,7 +2142,7 @@ function protoGqlPolicy( } function protoGqlCondition( - condition: StandardPermissionCondition<"gql">, + condition: SnapshotPermissionCondition, ): MessageInitShape { const [left, operator, right] = condition; @@ -2149,10 +2179,10 @@ function protoGqlCondition( } function protoGqlOperand( - operand: PermissionOperand, + operand: SnapshotPermissionOperand, ): MessageInitShape { - if (typeof operand === "object" && !Array.isArray(operand)) { - if ("user" in operand) { + if (typeof operand === "object" && operand !== null && !Array.isArray(operand)) { + if ("user" in operand && typeof operand.user === "string") { return { kind: { case: "userField", @@ -2165,7 +2195,7 @@ function protoGqlOperand( return { kind: { case: "value", - value: fromJson(ValueSchema, operand), + value: fromJson(ValueSchema, operand as Parameters[1]), }, }; } @@ -2183,12 +2213,12 @@ interface MigrationCheckResult { /** * Check if there are schema differences between migration snapshots and local definitions - * @param {ReadonlyMap>} typesByNamespace - Types by namespace + * @param {ReadonlyMap>} typesByNamespace - Snapshot-shaped local types by namespace * @param {NamespaceWithMigrations[]} namespacesWithMigrations - Namespaces with migrations config * @returns {Promise} Results for each namespace */ async function checkMigrationDiffs( - typesByNamespace: ReadonlyMap>, + typesByNamespace: ReadonlyMap>, namespacesWithMigrations: NamespaceWithMigrations[], ): Promise { const results: MigrationCheckResult[] = []; 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..3de8922b3 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.test.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.test.ts @@ -374,7 +374,10 @@ describe("snapshot", () => { expect(diff.breakingChanges[0].reason).toContain("Field type changed"); }); - it("treats decimal field with unset scale as equivalent to platform default", () => { + it("treats decimal fields with the same normalized scale as equivalent", () => { + // Both sides are produced through createSnapshotType / loadSnapshot in + // production, which fills in the platform default scale for decimals. + // The comparison contract expects pre-normalized inputs. const previous: SchemaSnapshot = { ...createEmptySnapshot(), types: { @@ -382,7 +385,7 @@ describe("snapshot", () => { name: "Order", fields: { id: { type: "uuid", required: true }, - amount: { type: "decimal", required: true }, + amount: { type: "decimal", required: true, scale: 6 }, }, }, }, @@ -738,7 +741,8 @@ describe("snapshot", () => { }), }; - const diff = compareLocalTypesWithSnapshot(previousSnapshot, localTypes, namespace); + const snapshotTypes = createSnapshotFromLocalTypes(localTypes, namespace).types; + const diff = compareLocalTypesWithSnapshot(previousSnapshot, snapshotTypes, namespace); expect(diff.changes.length).toBe(1); expect(diff.changes[0].kind).toBe("field_added"); @@ -1772,7 +1776,9 @@ describe("snapshot", () => { expect(drifts[0].details).toContain("allowedValues"); }); - it("treats decimal field with unset scale as platform default (no drift)", () => { + it("treats decimal field with normalized platform-default scale as equivalent (no drift)", () => { + // Snapshots loaded through loadSnapshot have scale normalized to the + // platform default (6) for decimal fields without an explicit scale. const snapshot: SchemaSnapshot = { version: SCHEMA_SNAPSHOT_VERSION, namespace, @@ -1782,7 +1788,7 @@ describe("snapshot", () => { name: "Order", fields: { id: { type: "uuid", required: true }, - amount: { type: "decimal", required: true }, + amount: { type: "decimal", required: true, scale: 6 }, }, }, }, @@ -1799,7 +1805,7 @@ describe("snapshot", () => { expect(drifts).toEqual([]); }); - it("detects drift when decimal scale actually differs from platform default", () => { + it("detects drift when decimal scale differs from snapshot", () => { const snapshot: SchemaSnapshot = { version: SCHEMA_SNAPSHOT_VERSION, namespace, @@ -1809,7 +1815,7 @@ describe("snapshot", () => { name: "Order", fields: { id: { type: "uuid", required: true }, - amount: { type: "decimal", required: true }, + amount: { type: "decimal", required: true, scale: 6 }, }, }, }, diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.ts b/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.ts index 0becbf7dd..72482f6aa 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.ts @@ -52,16 +52,31 @@ export const MIGRATION_NUMBER_PATTERN = /^\d{4}$/; export const DEFAULT_DECIMAL_SCALE = 6; /** - * Resolve the effective scale of a field for comparison purposes. - * Decimal fields without an explicit scale are stored on the platform with the - * default scale, so we normalize unset values to the default to avoid false drift. - * @param {SnapshotFieldConfig} field - Field configuration - * @returns {number | undefined} Effective scale, or undefined for non-decimal fields without scale + * Normalize a snapshot field in place so the snapshot becomes the canonical + * form for comparison. Currently fills in the platform default decimal scale + * when omitted, which avoids false drift between local schemas (where scale + * may be omitted) and the platform (which always materializes a scale). + * @param {SnapshotFieldConfig} field - Field configuration to normalize */ -function getEffectiveScale(field: SnapshotFieldConfig): number | undefined { - if (field.scale !== undefined) return field.scale; - if (field.type === "decimal") return DEFAULT_DECIMAL_SCALE; - return undefined; +function normalizeSnapshotField(field: SnapshotFieldConfig): void { + if (field.type === "decimal" && field.scale === undefined) { + field.scale = DEFAULT_DECIMAL_SCALE; + } + if (field.fields) { + for (const nested of Object.values(field.fields)) { + normalizeSnapshotField(nested); + } + } +} + +/** + * Normalize every field in a snapshot type in place. + * @param {SnapshotType} type - Snapshot type to normalize + */ +function normalizeSnapshotTypeFields(type: SnapshotType): void { + for (const field of Object.values(type.fields)) { + normalizeSnapshotField(field); + } } // Re-export SCHEMA_SNAPSHOT_VERSION for convenience @@ -509,7 +524,7 @@ function createSnapshotFieldConfigFromOperatorConfig( * @param {TailorDBType} type - Parsed TailorDB type definition * @returns {SnapshotType} Snapshot type configuration */ -function createSnapshotType(type: TailorDBType): SnapshotType { +export function createSnapshotType(type: TailorDBType): SnapshotType { const fields: Record = {}; for (const [fieldName, field] of Object.entries(type.fields)) { @@ -616,6 +631,7 @@ function createSnapshotType(type: TailorDBType): SnapshotType { } } + normalizeSnapshotTypeFields(snapshotType); return snapshotType; } @@ -669,7 +685,13 @@ export function createSnapshotFromLocalTypes( */ export function loadSnapshot(filePath: string): SchemaSnapshot { const content = fs.readFileSync(filePath, "utf-8"); - return JSON.parse(content) as SchemaSnapshot; + const snapshot = JSON.parse(content) as SchemaSnapshot; + // Older snapshots may omit scale on decimal fields. Normalize on load so all + // downstream comparisons can read field.scale directly without special cases. + for (const type of Object.values(snapshot.types)) { + normalizeSnapshotTypeFields(type); + } + return snapshot; } /** @@ -1041,7 +1063,7 @@ function areFieldsDifferent(oldField: SnapshotFieldConfig, newField: SnapshotFie if ((oldSerial.format ?? "") !== (newSerial.format ?? "")) return true; } - if (getEffectiveScale(oldField) !== getEffectiveScale(newField)) return true; + if (oldField.scale !== newField.scale) return true; const oldFields = oldField.fields ?? {}; const newFields = newField.fields ?? {}; @@ -1574,18 +1596,25 @@ export function compareSnapshots(previous: SchemaSnapshot, current: SchemaSnapsh } /** - * Compare local types with a snapshot and generate a diff + * Compare a snapshot against canonical SnapshotType-shaped local types. + * Callers must pre-convert TailorDBService.types to SnapshotType via + * `createSnapshotType` so that all comparisons read the same normalized shape. * @param {SchemaSnapshot} snapshot - Schema snapshot to compare against - * @param {Record} localTypes - Local type definitions + * @param {Record} localTypes - Local snapshot-shaped types * @param {string} namespace - Namespace for comparison * @returns {MigrationDiff} Migration diff */ export function compareLocalTypesWithSnapshot( snapshot: SchemaSnapshot, - localTypes: Record, + localTypes: Record, namespace: string, ): MigrationDiff { - const currentSnapshot = createSnapshotFromLocalTypes(localTypes, namespace); + const currentSnapshot: SchemaSnapshot = { + version: SCHEMA_SNAPSHOT_VERSION, + namespace, + createdAt: new Date().toISOString(), + types: localTypes, + }; return compareSnapshots(snapshot, currentSnapshot); } @@ -1830,6 +1859,7 @@ function convertRemoteFieldsToSnapshot( // TODO: Add nested field conversion when remote API supports it + normalizeSnapshotField(config); fields[fieldName] = config; } @@ -1921,10 +1951,8 @@ function compareFields( differences.push(`vector: remote=${remoteVector}, expected=${snapshotVector}`); } - const remoteScale = getEffectiveScale(remoteField); - const snapshotScale = getEffectiveScale(snapshotField); - if (remoteScale !== snapshotScale) { - differences.push(`scale: remote=${remoteScale}, expected=${snapshotScale}`); + if (remoteField.scale !== snapshotField.scale) { + differences.push(`scale: remote=${remoteField.scale}, expected=${snapshotField.scale}`); } if (differences.length > 0) { From c14ec0a5941d9cdf54bee72b2abef59c495078a2 Mon Sep 17 00:00:00 2001 From: Akira HIGUCHI Date: Tue, 19 May 2026 23:54:47 +0900 Subject: [PATCH 2/3] refactor(sdk): tighten snapshot types and centralize normalize boundaries Address review feedback on the deploy-via-snapshot refactor: - SnapshotPermissionOperand: replace the collapsing `| unknown` with a proper discriminated union (SnapshotFieldRefOperand | SnapshotValueOperand) and rewrite protoOperand / protoGqlOperand / convertOperandToProto as exhaustive switches with `satisfies never` and explicit throws. The GQL path now rejects non-`{ user }` field-refs, matching the parser's Zod schema. - TailorDBSnapshotType.pluralForm is now required at the type level; legacy snapshots are backfilled via normalizeSnapshotType at the load boundary instead of per-consumer fallbacks. - Centralize normalizeSnapshotField / normalizeSnapshotType at every snapshot construction boundary and add idempotent defense-in-depth normalize at the entries of compareSnapshots and compareRemoteWithSnapshot, so callers building literals never produce silent false drift. - Rename SnapshotType to TailorDBSnapshotType to disambiguate against generic "snapshot type" reading and align with TailorDBType. - Rewrite snapshot tests to exercise the normalization path through createSnapshotFromLocalTypes / loadSnapshot, and add pluralForm to all TailorDBSnapshotType literals (required by the new contract). - Reword the changeset around the user-visible effect: tailor deploy no longer shows spurious drift on decimal fields without an explicit scale. --- .../unify-tailordb-deploy-on-snapshot.md | 2 +- .../src/cli/commands/deploy/tailordb/index.ts | 99 ++++++-------- .../migrate/db-types-generator.test.ts | 1 + .../tailordb/migrate/db-types-generator.ts | 6 +- .../migrate/snapshot-manifest.test.ts | 9 +- .../tailordb/migrate/snapshot-manifest.ts | 60 +++------ .../tailordb/migrate/snapshot.test.ts | 122 +++++++++++++----- .../cli/commands/tailordb/migrate/snapshot.ts | 106 +++++++++++---- .../migrate/template-generator.test.ts | 5 + packages/sdk/src/cli/lib.ts | 2 +- 10 files changed, 239 insertions(+), 173 deletions(-) diff --git a/.changeset/unify-tailordb-deploy-on-snapshot.md b/.changeset/unify-tailordb-deploy-on-snapshot.md index 58aa94d34..81fe46744 100644 --- a/.changeset/unify-tailordb-deploy-on-snapshot.md +++ b/.changeset/unify-tailordb-deploy-on-snapshot.md @@ -2,4 +2,4 @@ "@tailor-platform/sdk": patch --- -Unify the `tailor deploy` TailorDB pipeline on the snapshot schema. All plan and apply phases now consume the same canonical snapshot-shaped input that is also used by `tailordb migrate`, with decimal scale normalized to the platform default (`6`) at the snapshot boundary. This consolidates normalization in one place instead of relying on per-comparison workarounds (such as the decimal-scale fix in #1155) and prepares for a future `tailordb migration sync` command. No behavior change for SDK users. +Fix `tailor deploy` so decimal fields without an explicit `scale` no longer show spurious drift against the platform (which materializes the default `6`). Deploy now plans and applies through the same snapshot pipeline as `tailordb migrate`. diff --git a/packages/sdk/src/cli/commands/deploy/tailordb/index.ts b/packages/sdk/src/cli/commands/deploy/tailordb/index.ts index 0280fdee6..fd3d65e1e 100644 --- a/packages/sdk/src/cli/commands/deploy/tailordb/index.ts +++ b/packages/sdk/src/cli/commands/deploy/tailordb/index.ts @@ -54,8 +54,9 @@ import { compareRemoteWithSnapshot, formatSchemaDrifts, createSnapshotType, + isSnapshotFieldRefOperand, type SnapshotFieldConfig, - type SnapshotType, + type TailorDBSnapshotType, type SnapshotRecordPermission, type SnapshotActionPermission, type SnapshotPermissionCondition, @@ -244,7 +245,7 @@ function formatRemoteVerificationResults(results: RemoteSchemaVerificationResult * Validate migration files and detect pending migrations * @param {OperatorClient} client - Operator client instance * @param {string} workspaceId - Workspace ID - * @param {ReadonlyMap>} typesByNamespace - Types by namespace + * @param {ReadonlyMap>} typesByNamespace - Types by namespace * @param {LoadedConfig} config - Loaded application config (includes path) * @param {boolean} noSchemaCheck - Whether to skip schema diff check * @returns {Promise} List of pending migrations @@ -252,7 +253,7 @@ function formatRemoteVerificationResults(results: RemoteSchemaVerificationResult async function validateAndDetectMigrations( client: OperatorClient, workspaceId: string, - typesByNamespace: ReadonlyMap>, + typesByNamespace: ReadonlyMap>, config: LoadedConfig, noSchemaCheck: boolean, ): Promise { @@ -392,7 +393,7 @@ export async function applyTailorDB( if (phase === "create-update") { // Validate and detect migrations // Build types by namespace map (snapshot-shaped, the canonical deploy form) - const typesByNamespace = new Map>(); + const typesByNamespace = new Map>(); for (const tailordb of migrationContext.tailorDBInputs) { typesByNamespace.set(tailordb.namespace, tailordb.types); } @@ -979,7 +980,7 @@ async function executeSingleMigrationPostPhase( type TailorDBDeployInput = { namespace: string; config: TailorDBServiceConfig; - types: Record; + types: Record; }; /** @@ -988,7 +989,7 @@ type TailorDBDeployInput = { * @returns The canonical snapshot-shaped deploy input for downstream plan/apply phases. */ function toTailorDBDeployInput(service: TailorDBService): TailorDBDeployInput { - const types: Record = {}; + const types: Record = {}; for (const [typeName, type] of Object.entries(service.types)) { types[typeName] = createSnapshotType(type); } @@ -1308,7 +1309,7 @@ async function planTypes( tailordbs: ReadonlyArray, executors: ReadonlyArray, deletedServices: ReadonlyArray, - filteredTypesByNamespace?: Map>, + filteredTypesByNamespace?: Map>, forceApplyAll = false, ) { const changeSet = createChangeSet("TailorDB types"); @@ -1558,22 +1559,18 @@ function isNumericLikeValue(value: string | number | bigint): boolean { // This will need refactoring later. /** * Generate a TailorDB type manifest from snapshot-shaped type - * @param {SnapshotType} type - Snapshot-shaped TailorDB type + * @param {TailorDBSnapshotType} type - Snapshot-shaped TailorDB type * @param {ReadonlySet} executorUsedTypes - Set of types used by executors * @param {GqlOperations} [namespaceGqlOperations] - Default gqlOperations for the namespace (already normalized) * @returns {MessageInitShape} Type manifest */ function generateTailorDBTypeManifest( - type: SnapshotType, + type: TailorDBSnapshotType, executorUsedTypes: ReadonlySet, namespaceGqlOperations?: GqlOperations, ): MessageInitShape { - // pluralForm is always populated by the parser when building from configure, - // but stored snapshots (e.g. replayed by a future sync command) may omit it. - // Fall back to inflection so this path stays robust either way. - const rawPluralForm = type.pluralForm ?? inflection.pluralize(type.name); - // This ensures that explicitly provided pluralForm like "PurchaseOrderList" becomes "purchaseOrderList" - const pluralForm = inflection.camelize(rawPluralForm, true); + // Ensures that explicitly provided pluralForm like "PurchaseOrderList" becomes "purchaseOrderList". + const pluralForm = inflection.camelize(type.pluralForm, true); const defaultSettings: { aggregation: boolean; @@ -1903,43 +1900,25 @@ function protoCondition( function protoOperand( operand: SnapshotPermissionOperand, ): MessageInitShape { - if (typeof operand === "object" && operand !== null && !Array.isArray(operand)) { - if ("user" in operand && typeof operand.user === "string") { - return { - kind: { - case: "userField", - value: operand.user, - }, - }; - } else if ("record" in operand && typeof operand.record === "string") { - return { - kind: { - case: "recordField", - value: operand.record, - }, - }; - } else if ("newRecord" in operand && typeof operand.newRecord === "string") { - return { - kind: { - case: "newRecordField", - value: operand.newRecord, - }, - }; - } else if ("oldRecord" in operand && typeof operand.oldRecord === "string") { - return { - kind: { - case: "oldRecordField", - value: operand.oldRecord, - }, - }; + if (isSnapshotFieldRefOperand(operand)) { + if ("user" in operand) { + return { kind: { case: "userField", value: operand.user } }; } + if ("record" in operand) { + return { kind: { case: "recordField", value: operand.record } }; + } + if ("newRecord" in operand) { + return { kind: { case: "newRecordField", value: operand.newRecord } }; + } + if ("oldRecord" in operand) { + return { kind: { case: "oldRecordField", value: operand.oldRecord } }; + } + operand satisfies never; + throw new Error(`Unknown field-ref operand shape: ${JSON.stringify(operand)}`); } return { - kind: { - case: "value", - value: fromJson(ValueSchema, operand as Parameters[1]), - }, + kind: { case: "value", value: fromJson(ValueSchema, operand) }, }; } @@ -2181,22 +2160,18 @@ function protoGqlCondition( function protoGqlOperand( operand: SnapshotPermissionOperand, ): MessageInitShape { - if (typeof operand === "object" && operand !== null && !Array.isArray(operand)) { - if ("user" in operand && typeof operand.user === "string") { - return { - kind: { - case: "userField", - value: operand.user, - }, - }; + if (isSnapshotFieldRefOperand(operand)) { + if ("user" in operand) { + return { kind: { case: "userField", value: operand.user } }; } + throw new Error( + `Unsupported field-ref operand in GQL permission: ${JSON.stringify(operand)} ` + + `— GQL permissions only support { user } field references`, + ); } return { - kind: { - case: "value", - value: fromJson(ValueSchema, operand as Parameters[1]), - }, + kind: { case: "value", value: fromJson(ValueSchema, operand) }, }; } @@ -2213,12 +2188,12 @@ interface MigrationCheckResult { /** * Check if there are schema differences between migration snapshots and local definitions - * @param {ReadonlyMap>} typesByNamespace - Snapshot-shaped local types by namespace + * @param {ReadonlyMap>} typesByNamespace - Snapshot-shaped local types by namespace * @param {NamespaceWithMigrations[]} namespacesWithMigrations - Namespaces with migrations config * @returns {Promise} Results for each namespace */ async function checkMigrationDiffs( - typesByNamespace: ReadonlyMap>, + typesByNamespace: ReadonlyMap>, namespacesWithMigrations: NamespaceWithMigrations[], ): Promise { const results: MigrationCheckResult[] = []; 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..0d9379b17 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 @@ -38,6 +38,7 @@ function createMockSnapshot( } snapshotTypes[typeName] = { name: typeName, + pluralForm: `${typeName}s`, fields, }; } diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/db-types-generator.ts b/packages/sdk/src/cli/commands/tailordb/migrate/db-types-generator.ts index af31dd5fa..8cb9fee3b 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/db-types-generator.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/db-types-generator.ts @@ -10,7 +10,7 @@ import { getMigrationFilePath, type SchemaSnapshot, type SnapshotFieldConfig, - type SnapshotType, + type TailorDBSnapshotType, } from "./snapshot"; import type { MigrationDiff } from "./diff-calculator"; @@ -203,12 +203,12 @@ function generateEmptyDbTypes(namespace: string): string { /** * Generate table type definition from a snapshot type - * @param {SnapshotType} type - Snapshot type + * @param {TailorDBSnapshotType} type - Snapshot type * @param {BreakingChangeFieldInfo} breakingChangeFields - Breaking change field info * @returns {{ typeDef: string; usedTimestamp: boolean; usedColumnType: boolean }} Generated type and utility type usage */ function generateTableType( - type: SnapshotType, + type: TailorDBSnapshotType, breakingChangeFields: BreakingChangeFieldInfo, ): { typeDef: string; diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/snapshot-manifest.test.ts b/packages/sdk/src/cli/commands/tailordb/migrate/snapshot-manifest.test.ts index 93c29c0b6..a551c0ede 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/snapshot-manifest.test.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/snapshot-manifest.test.ts @@ -5,15 +5,16 @@ import { generateAllTypeManifestsFromSnapshot, compareSnapshotWithRemote, } from "./snapshot-manifest"; -import type { SchemaSnapshot, SnapshotType, SnapshotRecordPermission } from "./snapshot"; +import type { SchemaSnapshot, TailorDBSnapshotType, SnapshotRecordPermission } from "./snapshot"; describe("snapshot-manifest", () => { function createTestSnapshotType( name: string, - overrides: Partial = {}, - ): SnapshotType { + overrides: Partial = {}, + ): TailorDBSnapshotType { return { name, + pluralForm: `${name}s`, fields: { id: { type: "uuid", required: true }, name: { type: "string", required: true }, @@ -23,7 +24,7 @@ describe("snapshot-manifest", () => { } function createTestSnapshot( - types: Record, + types: Record, namespace = "tailordb", ): SchemaSnapshot { return { 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..5bf728a52 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/snapshot-manifest.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/snapshot-manifest.ts @@ -23,11 +23,12 @@ import { type TailorDBTypeSchema, } from "@tailor-proto/tailor/v1/tailordb_resource_pb"; import * as inflection from "inflection"; +import { isSnapshotFieldRefOperand } from "./snapshot"; import type { SchemaSnapshot, SnapshotEnumValue, SnapshotFieldConfig, - SnapshotType, + TailorDBSnapshotType, SnapshotRelationship, SnapshotRecordPermission, SnapshotActionPermission, @@ -53,17 +54,15 @@ export interface GenerateManifestOptions { /** * Generate a TailorDB type manifest from a snapshot type - * @param {SnapshotType} snapshotType - Snapshot type to generate manifest from + * @param {TailorDBSnapshotType} snapshotType - Snapshot type to generate manifest from * @param {GenerateManifestOptions} options - Generation options * @returns {MessageInitShape} Type manifest */ export function generateTailorDBTypeManifestFromSnapshot( - snapshotType: SnapshotType, + snapshotType: TailorDBSnapshotType, options: GenerateManifestOptions = {}, ): MessageInitShape { - const pluralForm = snapshotType.pluralForm - ? inflection.camelize(snapshotType.pluralForm, true) - : inflection.camelize(inflection.pluralize(snapshotType.name), true); + const pluralForm = inflection.camelize(snapshotType.pluralForm, true); // Build settings const defaultSettings: { @@ -455,50 +454,25 @@ function convertConditionToProto( function convertOperandToProto( operand: SnapshotPermissionOperand, ): MessageInitShape { - if (typeof operand === "object" && operand !== null && !Array.isArray(operand)) { - const obj = operand as Record; - if ("user" in obj && typeof obj.user === "string") { - return { - kind: { - case: "userField", - value: obj.user, - }, - }; + if (isSnapshotFieldRefOperand(operand)) { + if ("user" in operand) { + return { kind: { case: "userField", value: operand.user } }; } - if ("record" in obj && typeof obj.record === "string") { - return { - kind: { - case: "recordField", - value: obj.record, - }, - }; + if ("record" in operand) { + return { kind: { case: "recordField", value: operand.record } }; } - if ("newRecord" in obj && typeof obj.newRecord === "string") { - return { - kind: { - case: "newRecordField", - value: obj.newRecord, - }, - }; + if ("newRecord" in operand) { + return { kind: { case: "newRecordField", value: operand.newRecord } }; } - if ("oldRecord" in obj && typeof obj.oldRecord === "string") { - return { - kind: { - case: "oldRecordField", - value: obj.oldRecord, - }, - }; + if ("oldRecord" in operand) { + return { kind: { case: "oldRecordField", value: operand.oldRecord } }; } - // Fall through to value handling for unknown objects + operand satisfies never; + throw new Error(`Unknown field-ref operand shape: ${JSON.stringify(operand)}`); } - // Value operand (primitive or array) - // Cast to JsonValue type for fromJson return { - kind: { - case: "value", - value: fromJson(ValueSchema, operand as Parameters[1]), - }, + kind: { case: "value", value: fromJson(ValueSchema, operand) }, }; } 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 3de8922b3..49e7b2916 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.test.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.test.ts @@ -220,6 +220,7 @@ describe("snapshot", () => { types: { NewType: { name: "NewType", + pluralForm: "NewTypes", fields: { id: { type: "uuid", required: true } }, }, }, @@ -239,6 +240,7 @@ describe("snapshot", () => { types: { OldType: { name: "OldType", + pluralForm: "OldTypes", fields: { id: { type: "uuid", required: true } }, }, }, @@ -258,6 +260,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, }, }, @@ -267,6 +270,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, email: { type: "string", required: false }, @@ -288,6 +292,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, }, }, @@ -297,6 +302,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, requiredField: { type: "string", required: true }, @@ -317,6 +323,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, name: { type: "string", required: true }, @@ -329,6 +336,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, }, }, @@ -347,6 +355,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, age: { type: "string", required: false }, @@ -359,6 +368,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, age: { type: "number", required: false }, @@ -374,34 +384,34 @@ describe("snapshot", () => { expect(diff.breakingChanges[0].reason).toContain("Field type changed"); }); - it("treats decimal fields with the same normalized scale as equivalent", () => { - // Both sides are produced through createSnapshotType / loadSnapshot in - // production, which fills in the platform default scale for decimals. - // The comparison contract expects pre-normalized inputs. + it("normalizes decimal scale at compare entry so missing scale matches platform default", () => { + // Reproduces the production scenario where one snapshot was loaded from + // an older file that omitted `scale` and the other was produced by + // `createSnapshotType` (which materializes the platform default of 6). + // compareSnapshots normalizes both inputs at the entry, so the diff must + // come out empty even though the literal shapes differ. const previous: SchemaSnapshot = { ...createEmptySnapshot(), types: { Order: { name: "Order", + pluralForm: "Orders", fields: { id: { type: "uuid", required: true }, - amount: { type: "decimal", required: true, scale: 6 }, + amount: { type: "decimal", required: true }, }, }, }, }; - const current: SchemaSnapshot = { - ...createEmptySnapshot(), - types: { - Order: { - name: "Order", - fields: { - id: { type: "uuid", required: true }, - amount: { type: "decimal", required: true, scale: 6 }, - }, - }, + const current = createSnapshotFromLocalTypes( + { + Order: createMockType("Order", { + id: { name: "id", config: { type: "uuid", required: true } }, + amount: { name: "amount", config: { type: "decimal", required: true } }, + }), }, - }; + namespace, + ); const diff = compareSnapshots(previous, current); @@ -415,6 +425,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, email: { type: "string", required: false }, @@ -427,6 +438,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, email: { type: "string", required: true }, @@ -447,6 +459,7 @@ describe("snapshot", () => { types: { Post: { name: "Post", + pluralForm: "Posts", fields: { id: { type: "uuid", required: true }, tags: { type: "string", required: false, array: true }, @@ -459,6 +472,7 @@ describe("snapshot", () => { types: { Post: { name: "Post", + pluralForm: "Posts", fields: { id: { type: "uuid", required: true }, tags: { type: "string", required: false, array: false }, @@ -479,6 +493,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, email: { type: "string", required: true, unique: false }, @@ -491,6 +506,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, email: { type: "string", required: true, unique: true }, @@ -511,6 +527,7 @@ describe("snapshot", () => { types: { Task: { name: "Task", + pluralForm: "Tasks", fields: { id: { type: "uuid", required: true }, status: { @@ -532,6 +549,7 @@ describe("snapshot", () => { types: { Task: { name: "Task", + pluralForm: "Tasks", fields: { id: { type: "uuid", required: true }, status: { @@ -557,6 +575,7 @@ describe("snapshot", () => { types: { Task: { name: "Task", + pluralForm: "Tasks", fields: { id: { type: "uuid", required: true }, status: { @@ -573,6 +592,7 @@ describe("snapshot", () => { types: { Task: { name: "Task", + pluralForm: "Tasks", fields: { id: { type: "uuid", required: true }, status: { @@ -598,6 +618,7 @@ describe("snapshot", () => { types: { Task: { name: "Task", + pluralForm: "Tasks", fields: { id: { type: "uuid", required: true }, status: { @@ -614,6 +635,7 @@ describe("snapshot", () => { types: { Task: { name: "Task", + pluralForm: "Tasks", fields: { id: { type: "uuid", required: true }, status: { @@ -640,6 +662,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, }, }, @@ -656,10 +679,12 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, }, Post: { name: "Post", + pluralForm: "Posts", fields: { id: { type: "uuid", required: true }, authorId: { type: "uuid", required: true }, @@ -673,6 +698,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, backwardRelationships: { posts: { @@ -686,6 +712,7 @@ describe("snapshot", () => { }, Post: { name: "Post", + pluralForm: "Posts", fields: { id: { type: "uuid", required: true }, authorId: { type: "uuid", required: true }, @@ -729,6 +756,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, }, }, @@ -913,6 +941,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, }, }, @@ -1002,6 +1031,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, name: { type: "string", required: true }, @@ -1028,6 +1058,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, }, }, @@ -1067,6 +1098,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, }, }, @@ -1125,6 +1157,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, }, }, @@ -1140,6 +1173,7 @@ describe("snapshot", () => { typeName: "Post", after: { name: "Post", + pluralForm: "Posts", fields: { id: { type: "uuid", required: true }, title: { type: "string", required: true }, @@ -1170,10 +1204,12 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, }, OldType: { name: "OldType", + pluralForm: "OldTypes", fields: { id: { type: "uuid", required: true } }, }, }, @@ -1189,6 +1225,7 @@ describe("snapshot", () => { typeName: "OldType", before: { name: "OldType", + pluralForm: "OldTypes", fields: { id: { type: "uuid", required: true } }, }, }, @@ -1220,10 +1257,12 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, }, Post: { name: "Post", + pluralForm: "Posts", fields: { id: { type: "uuid", required: true }, authorId: { type: "uuid", required: true }, @@ -1519,6 +1558,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, name: { type: "string", required: true }, @@ -1546,10 +1586,12 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, }, Post: { name: "Post", + pluralForm: "Posts", fields: { id: { type: "uuid", required: true } }, }, }, @@ -1575,6 +1617,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, }, }, @@ -1603,6 +1646,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, email: { type: "string", required: false }, @@ -1631,6 +1675,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, }, @@ -1659,6 +1704,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, age: { type: "number", required: false }, @@ -1689,6 +1735,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, name: { type: "string", required: false }, @@ -1718,6 +1765,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true }, tags: { type: "string", required: false, array: true }, @@ -1747,6 +1795,7 @@ describe("snapshot", () => { types: { Task: { name: "Task", + pluralForm: "Tasks", fields: { id: { type: "uuid", required: true }, status: { @@ -1776,23 +1825,32 @@ describe("snapshot", () => { expect(drifts[0].details).toContain("allowedValues"); }); - it("treats decimal field with normalized platform-default scale as equivalent (no drift)", () => { - // Snapshots loaded through loadSnapshot have scale normalized to the - // platform default (6) for decimal fields without an explicit scale. - const snapshot: SchemaSnapshot = { - version: SCHEMA_SNAPSHOT_VERSION, - namespace, - createdAt: new Date().toISOString(), - types: { - Order: { - name: "Order", - fields: { - id: { type: "uuid", required: true }, - amount: { type: "decimal", required: true, scale: 6 }, + it("normalizes decimal scale at compare entry so missing scale matches remote default", () => { + // The snapshot is written from disk without an explicit scale (legacy / + // user-authored form). compareRemoteWithSnapshot normalizes the snapshot + // at entry so it becomes equivalent to a remote that has materialized + // the platform-default scale of 6. + const snapshotPath = path.join(testDir, "decimal-default", SCHEMA_FILE_NAME); + fs.mkdirSync(path.dirname(snapshotPath), { recursive: true }); + fs.writeFileSync( + snapshotPath, + JSON.stringify({ + version: SCHEMA_SNAPSHOT_VERSION, + namespace, + createdAt: new Date().toISOString(), + types: { + Order: { + name: "Order", + pluralForm: "Orders", + fields: { + id: { type: "uuid", required: true }, + amount: { type: "decimal", required: true }, + }, }, }, - }, - }; + }), + ); + const snapshot = loadSnapshot(snapshotPath); const remoteTypes = [ createMockRemoteType("Order", { @@ -1813,6 +1871,7 @@ describe("snapshot", () => { types: { Order: { name: "Order", + pluralForm: "Orders", fields: { id: { type: "uuid", required: true }, amount: { type: "decimal", required: true, scale: 6 }, @@ -1842,6 +1901,7 @@ describe("snapshot", () => { types: { User: { name: "User", + pluralForm: "Users", fields: { id: { type: "uuid", required: true } }, }, }, diff --git a/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.ts b/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.ts index 72482f6aa..191370d91 100644 --- a/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.ts +++ b/packages/sdk/src/cli/commands/tailordb/migrate/snapshot.ts @@ -3,6 +3,7 @@ */ import * as fs from "node:fs"; +import * as inflection from "inflection"; import * as path from "pathe"; import { type MigrationDiff, @@ -70,10 +71,21 @@ function normalizeSnapshotField(field: SnapshotFieldConfig): void { } /** - * Normalize every field in a snapshot type in place. - * @param {SnapshotType} type - Snapshot type to normalize + * Normalize a snapshot type in place to the canonical comparison shape. + * Currently fills: + * - `pluralForm` via inflection when missing (legacy snapshots written + * before `pluralForm` became required may omit it) + * - per-field `scale` defaults via {@link normalizeSnapshotField} + * + * Idempotent — safe to call multiple times on the same input. + * @param {TailorDBSnapshotType} type - Snapshot type to normalize */ -function normalizeSnapshotTypeFields(type: SnapshotType): void { +function normalizeSnapshotType(type: TailorDBSnapshotType): void { + // `pluralForm` is typed as required by TailorDBSnapshotType, but JSON.parse'd legacy + // snapshots may have it undefined at runtime — backfill from inflection. + if (!(type as { pluralForm?: string }).pluralForm) { + type.pluralForm = inflection.pluralize(type.name); + } for (const field of Object.values(type.fields)) { normalizeSnapshotField(field); } @@ -168,14 +180,27 @@ export interface SnapshotRelationship { // ============================================================================ /** - * Permission operand types + * Field-reference operand in a permission condition. Always an object with + * exactly one of `user` / `record` / `newRecord` / `oldRecord` keys. */ -export type SnapshotPermissionOperand = +export type SnapshotFieldRefOperand = | { user: string } | { record: string } | { newRecord: string } - | { oldRecord: string } - | unknown; // ValueOperand (primitives, arrays) + | { oldRecord: string }; + +/** + * Literal value operand (right-hand side of a permission condition). Matches + * the SDK-level value operand surface — primitives and their arrays — as + * defined in the Zod parser schema (RecordPermissionOperandSchema / + * GqlPermissionOperandSchema in parser/service/tailordb/schema.ts). + */ +export type SnapshotValueOperand = string | boolean | string[] | boolean[]; + +/** + * Permission operand union. Either a field-ref object or a literal value. + */ +export type SnapshotPermissionOperand = SnapshotFieldRefOperand | SnapshotValueOperand; /** * Permission operators @@ -191,6 +216,17 @@ export type SnapshotPermissionCondition = readonly [ SnapshotPermissionOperand, ]; +/** + * Type guard: is the operand a field-reference (object) operand? + * @param {SnapshotPermissionOperand} operand - Operand to test + * @returns {boolean} True if operand is a field-ref (not a value operand) + */ +export function isSnapshotFieldRefOperand( + operand: SnapshotPermissionOperand, +): operand is SnapshotFieldRefOperand { + return typeof operand === "object" && operand !== null && !Array.isArray(operand); +} + /** * Action permission policy */ @@ -238,11 +274,14 @@ export interface SnapshotGqlPermissionPolicy { export type SnapshotGqlPermission = readonly SnapshotGqlPermissionPolicy[]; /** - * Type definition in schema snapshot + * Type definition in schema snapshot. + * `pluralForm` is always materialized — either set by the SDK user, derived + * via inflection at snapshot construction, or backfilled when loading legacy + * snapshots in `loadSnapshot`. */ -export interface SnapshotType { +export interface TailorDBSnapshotType { name: string; - pluralForm?: string; + pluralForm: string; description?: string; fields: Record; settings?: { @@ -275,7 +314,7 @@ export interface SchemaSnapshot { version: typeof SCHEMA_SNAPSHOT_VERSION; namespace: string; createdAt: string; - types: Record; + types: Record; } /** @@ -445,6 +484,7 @@ function createSnapshotFieldConfig(field: ParsedField): SnapshotFieldConfig { } } + normalizeSnapshotField(config); return config; } @@ -516,27 +556,28 @@ function createSnapshotFieldConfigFromOperatorConfig( } } + normalizeSnapshotField(config); return config; } /** * Create a snapshot type from a parsed type * @param {TailorDBType} type - Parsed TailorDB type definition - * @returns {SnapshotType} Snapshot type configuration + * @returns {TailorDBSnapshotType} Snapshot type configuration */ -export function createSnapshotType(type: TailorDBType): SnapshotType { +export function createSnapshotType(type: TailorDBType): TailorDBSnapshotType { const fields: Record = {}; for (const [fieldName, field] of Object.entries(type.fields)) { fields[fieldName] = createSnapshotFieldConfig(field); } - const snapshotType: SnapshotType = { + const snapshotType: TailorDBSnapshotType = { name: type.name, + pluralForm: type.pluralForm || inflection.pluralize(type.name), fields, }; - if (type.pluralForm) snapshotType.pluralForm = type.pluralForm; if (type.description) snapshotType.description = type.description; if (type.settings) { snapshotType.settings = {}; @@ -631,7 +672,6 @@ export function createSnapshotType(type: TailorDBType): SnapshotType { } } - normalizeSnapshotTypeFields(snapshotType); return snapshotType; } @@ -660,7 +700,7 @@ export function createSnapshotFromLocalTypes( types: Record, namespace: string, ): SchemaSnapshot { - const snapshotTypes: Record = {}; + const snapshotTypes: Record = {}; for (const [typeName, type] of Object.entries(types)) { snapshotTypes[typeName] = createSnapshotType(type); @@ -686,10 +726,8 @@ export function createSnapshotFromLocalTypes( export function loadSnapshot(filePath: string): SchemaSnapshot { const content = fs.readFileSync(filePath, "utf-8"); const snapshot = JSON.parse(content) as SchemaSnapshot; - // Older snapshots may omit scale on decimal fields. Normalize on load so all - // downstream comparisons can read field.scale directly without special cases. for (const type of Object.values(snapshot.types)) { - normalizeSnapshotTypeFields(type); + normalizeSnapshotType(type); } return snapshot; } @@ -781,7 +819,7 @@ function applyDiffToSnapshot(snapshot: SchemaSnapshot, diff: MigrationDiff): Sch for (const change of diff.changes) { switch (change.kind) { case "type_added": - types[change.typeName] = change.after as SnapshotType; + types[change.typeName] = change.after as TailorDBSnapshotType; break; case "type_removed": delete types[change.typeName]; @@ -1203,8 +1241,8 @@ function addChange( function compareTypeFields( ctx: DiffContext, typeName: string, - prevType: SnapshotType, - currType: SnapshotType, + prevType: TailorDBSnapshotType, + currType: TailorDBSnapshotType, ): void { const prevFieldNames = new Set(Object.keys(prevType.fields)); const currFieldNames = new Set(Object.keys(currType.fields)); @@ -1514,6 +1552,13 @@ function comparePermissions( * @returns {MigrationDiff} Migration diff between snapshots */ export function compareSnapshots(previous: SchemaSnapshot, current: SchemaSnapshot): MigrationDiff { + // Defense-in-depth: factory functions (`createSnapshotType`, `loadSnapshot`, + // `convertRemoteFieldsToSnapshot`) are expected to produce normalized + // snapshots, but a caller assembling a SchemaSnapshot literal would otherwise + // produce silent false drift (e.g. decimal scale 6 vs unset). Idempotent. + for (const type of Object.values(previous.types)) normalizeSnapshotType(type); + for (const type of Object.values(current.types)) normalizeSnapshotType(type); + const ctx: DiffContext = { changes: [], breakingChanges: [] }; const previousTypeNames = new Set(Object.keys(previous.types)); @@ -1596,17 +1641,19 @@ export function compareSnapshots(previous: SchemaSnapshot, current: SchemaSnapsh } /** - * Compare a snapshot against canonical SnapshotType-shaped local types. - * Callers must pre-convert TailorDBService.types to SnapshotType via - * `createSnapshotType` so that all comparisons read the same normalized shape. + * Compare a snapshot against canonical TailorDBSnapshotType-shaped local types. + * Callers are expected to pre-convert TailorDBService.types to TailorDBSnapshotType via + * `createSnapshotType`. As a safety net, `compareSnapshots` re-runs idempotent + * normalization on both sides, so a caller that forgets will still get correct + * comparisons (no silent false drift). * @param {SchemaSnapshot} snapshot - Schema snapshot to compare against - * @param {Record} localTypes - Local snapshot-shaped types + * @param {Record} localTypes - Local snapshot-shaped types * @param {string} namespace - Namespace for comparison * @returns {MigrationDiff} Migration diff */ export function compareLocalTypesWithSnapshot( snapshot: SchemaSnapshot, - localTypes: Record, + localTypes: Record, namespace: string, ): MigrationDiff { const currentSnapshot: SchemaSnapshot = { @@ -1982,6 +2029,9 @@ export function compareRemoteWithSnapshot( remoteTypes: ProtoTailorDBType[], snapshot: SchemaSnapshot, ): SchemaDrift[] { + // Defense-in-depth normalize — matches `compareSnapshots`. Idempotent. + for (const type of Object.values(snapshot.types)) normalizeSnapshotType(type); + const drifts: SchemaDrift[] = []; // Build maps for easy lookup 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..4f7d6ec41 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 @@ -46,6 +46,7 @@ describe("template-generator", () => { const snapshot = createTestSnapshot({ User: { name: "User", + pluralForm: "Users", fields: { name: { type: "string", required: true }, email: { type: "string", required: false }, @@ -114,6 +115,7 @@ describe("template-generator", () => { const previousSnapshot = createTestSnapshot({ User: { name: "User", + pluralForm: "Users", fields: { name: { type: "string", required: true }, }, @@ -221,6 +223,7 @@ describe("template-generator", () => { const snapshotWithOldField = createTestSnapshot({ User: { name: "User", + pluralForm: "Users", fields: { name: { type: "string", required: true }, oldField: { type: "string", required: false }, @@ -262,6 +265,7 @@ describe("template-generator", () => { const snapshotWithoutUnique = createTestSnapshot({ User: { name: "User", + pluralForm: "Users", fields: { email: { type: "string", required: true, unique: false }, }, @@ -305,6 +309,7 @@ describe("template-generator", () => { const snapshotWithAllEnumValues = createTestSnapshot({ Task: { name: "Task", + pluralForm: "Tasks", fields: { status: { type: "enum", diff --git a/packages/sdk/src/cli/lib.ts b/packages/sdk/src/cli/lib.ts index 18e50d174..481d6b329 100644 --- a/packages/sdk/src/cli/lib.ts +++ b/packages/sdk/src/cli/lib.ts @@ -206,7 +206,7 @@ export { getMigrationDirPath, getMigrationFilePath, type SchemaSnapshot, - type SnapshotType, + type TailorDBSnapshotType, type SnapshotFieldConfig, type MigrationInfo, } from "./commands/tailordb/migrate/snapshot"; From 961892f7594bb03901a29768845f3176300846fd Mon Sep 17 00:00:00 2001 From: Akira HIGUCHI Date: Thu, 21 May 2026 15:23:13 +0900 Subject: [PATCH 3/3] test(sdk): seed tailorDBInputs in applyTailorDB reconcile fixture --- packages/sdk/src/cli/commands/deploy/tailordb/index.test.ts | 1 + 1 file changed, 1 insertion(+) 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 dd1d6b7e2..dac6db783 100644 --- a/packages/sdk/src/cli/commands/deploy/tailordb/index.test.ts +++ b/packages/sdk/src/cli/commands/deploy/tailordb/index.test.ts @@ -908,6 +908,7 @@ describe("applyTailorDB migration label reconciliation (--no-schema-check)", () name: "test-app", tailorDBServices: [mockTailorDBService], } as unknown as Application, + tailorDBInputs: [], config, noSchemaCheck: true, },