Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-deploy-drift.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@tailor-platform/sdk": patch
---

Fix deploy reporting spurious updates on every run: TailorDB types no longer drift when the platform proto gains new fields, IdP services without an explicit userAuthPolicy no longer drift against the platform's default password policy, and deploys with pending migrations no longer silently skip type changes in namespaces that have no migrations
176 changes: 176 additions & 0 deletions .github/workflows/deploy-drift-check.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
name: "Deploy Drift Check"

# Deploys create-sdk templates to a fresh workspace and verifies that an
# immediate dry run reports "Plan: 0 to create, 0 to update, 0 to delete" — a
# redeploy of unchanged code must be a no-op. Guards against comparison drift
# such as newly added proto fields being reported as updates on every deploy.
# example/ gets the same check inside the existing Workspace Deploy Test
# (deploy.yml), which already deploys it on every PR.
#
# The target list intentionally covers config-shape diversity rather than
# every template: example configures everything explicitly, so drift caused by
# the platform filling defaults for OMITTED sections (e.g. IdP userAuthPolicy)
# only reproduces on minimal configs like these.
#
# Targets run as a serial loop in a single job, not a matrix: it keeps the
# platform load at one ephemeral workspace at a time and pays the
# checkout/install/build setup only once. The job is nowhere near the PR
# critical path (Workspace Deploy Test and the e2e suites take far longer),
# so the extra wall-clock over a parallel matrix costs nothing. A failing
# target does not stop the loop — every target reports before the job fails.

on:
pull_request:

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: false

permissions: {}

jobs:
changes:
name: Detect changes
# Skip re-runs triggered by tailor-pr-trigger bot (changeset/generate commits)
if: github.event.action != 'synchronize' || github.event.sender.login != 'tailor-pr-trigger[bot]'
runs-on: ubuntu-latest
permissions:
contents: read
timeout-minutes: 5
outputs:
should_run: ${{ steps.filter.outputs.changes }}
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
persist-credentials: "false"
- uses: dorny/paths-filter@7b450fff21473bca461d4b92ce414b9d0420d706 # v4.0.2
id: filter
with:
filters: |
changes:
- .github/workflows/deploy-drift-check.yml
- .github/actions/**
- packages/**
- package.json
- tsconfig.json
- pnpm-workspace.yaml
- pnpm-lock.yaml

drift-check:
needs: changes
if: needs.changes.outputs.should_run == 'true'
runs-on: ubuntu-latest
name: Drift check (templates)
permissions:
contents: read
timeout-minutes: 30
steps:
- name: checkout
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
persist-credentials: "false"

- name: Install deps
uses: ./.github/actions/install-deps

- name: Login as machine user
run: pnpm tailor-sdk login --machineuser
env:
TAILOR_PLATFORM_MACHINE_USER_CLIENT_ID: ${{ secrets.PLATFORM_MACHINE_USER_CLIENT_ID }}
TAILOR_PLATFORM_MACHINE_USER_CLIENT_SECRET: ${{ secrets.PLATFORM_MACHINE_USER_CLIENT_SECRET }}

- name: Deploy each template and verify a dry run reports no changes
env:
TAILOR_PLATFORM_ORGANIZATION_ID: ${{ secrets.TAILOR_PLATFORM_ORGANIZATION_ID }}
TAILOR_PLATFORM_FOLDER_ID: ${{ secrets.TAILOR_PLATFORM_FOLDER_ID }}
# Fresh checkout deploys a throwaway app; per-run id injection is intended here.
TAILOR_PLATFORM_SDK_ALLOW_CI_ID_INJECTION: "true"
RUN_ID: ${{ github.run_id }}
run: |
failed=()

check_target() {
local target="$1" dir="$2" config="$3"
local workspace_id summary

workspace_id=$(cd "$dir" && pnpm exec tailor-sdk workspace create -n "e2e-ws-${RUN_ID}-drift-${target}" -r us-west --json | tail -1 | jq -r '.id') || workspace_id=""
if [[ -z "$workspace_id" || "$workspace_id" == "null" ]]; then
echo "::error::${target}: failed to create workspace"
return 1
fi

local result=0
if ! (cd "$dir" && pnpm exec tailor-sdk deploy -c "$config" -w "$workspace_id" --yes); then
echo "::error::${target}: deploy failed"
result=1
else
summary=$(cd "$dir" && pnpm exec tailor-sdk deploy -c "$config" -w "$workspace_id" --dry-run --yes --json | tail -1 | jq -c '.summary') || summary=""
echo "${target} plan summary: $summary"
if ! echo "$summary" | jq -e '.create == 0 and .update == 0 and .delete == 0 and .replace == 0' > /dev/null; then
echo "::error::${target}: dry run after deploy reported drift: $summary"
echo "Re-running without --json for a readable diff:"
(cd "$dir" && pnpm exec tailor-sdk deploy -c "$config" -w "$workspace_id" --dry-run --yes) || true
result=1
fi
fi

pnpm exec tailor-sdk workspace delete -w "$workspace_id" -y \
|| echo "::warning::${target}: workspace delete failed (the cleanup step sweeps it)"
return "$result"
}

while IFS='|' read -r target dir config; do
echo "::group::${target}"
check_target "$target" "$dir" "$config" || failed+=("$target")
echo "::endgroup::"
done <<'TARGETS'
hello-world|packages/create-sdk/templates/hello-world|tailor.config.ts
generators|packages/create-sdk/templates/generators|tailor.config.ts
multi-application|packages/create-sdk/templates/multi-application|apps/user/tailor.config.ts,apps/admin/tailor.config.ts
static-web-site|packages/create-sdk/templates/static-web-site|tailor.config.ts
TARGETS

if (( ${#failed[@]} > 0 )); then
echo "::error::Deploy drift check failed for: ${failed[*]}"
exit 1
fi
echo "All templates redeploy as a no-op"

- name: Cleanup workspaces
if: always()
working-directory: packages/sdk
env:
RUN_ID: ${{ github.run_id }}
run: pnpm exec tsx scripts/cleanup-e2e-workspaces.ts --run-id="${RUN_ID}-drift"

drift-check-result:
name: Deploy drift check result
needs: [changes, drift-check]
if: >-
always() &&
(github.event.action != 'synchronize' || github.event.sender.login != 'tailor-pr-trigger[bot]')
runs-on: ubuntu-latest
permissions: {}
timeout-minutes: 5
steps:
- name: Check result
env:
CHANGES_RESULT: ${{ needs.changes.result }}
SHOULD_RUN: ${{ needs.changes.outputs.should_run }}
DRIFT_CHECK_RESULT: ${{ needs.drift-check.result }}
run: |
# Fail closed: if the paths-filter job itself failed, should_run is
# empty and must not be mistaken for "no relevant changes".
if [[ "$CHANGES_RESULT" != "success" ]]; then
echo "Changes job did not succeed (result=$CHANGES_RESULT)"
exit 1
fi
if [[ "$SHOULD_RUN" != "true" ]]; then
echo "Skipped - no relevant changes"
exit 0
fi
if [[ "$DRIFT_CHECK_RESULT" != "success" ]]; then
echo "Drift check job failed"
exit 1
fi
echo "Deploy drift check succeeded"
16 changes: 16 additions & 0 deletions .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,22 @@ jobs:

echo "✅ All migrations applied successfully"

# A redeploy of unchanged code must be a no-op; anything else is
# comparison drift (e.g. new proto fields read as updates on every run).
- name: Dry run must report no changes
if: runner.os != 'Windows'
working-directory: example
shell: bash
run: |
summary=$(pnpm --silent tailor-sdk deploy -c tailor.config.ts --dry-run --yes --json | tail -1 | jq -c '.summary')
echo "Plan summary: $summary"
if ! echo "$summary" | jq -e '.create == 0 and .update == 0 and .delete == 0 and .replace == 0' > /dev/null; then
echo "::error::Dry run after deploy reported drift: $summary"
echo "Re-running without --json for a readable diff:"
pnpm tailor-sdk deploy -c tailor.config.ts --dry-run --yes
exit 1
fi

- name: Validate seed data
if: runner.os == 'Windows'
working-directory: example
Expand Down
29 changes: 28 additions & 1 deletion packages/sdk/src/cli/commands/deploy/compare.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import { create } from "@bufbuild/protobuf";
import { TailorDBTypeSchema } from "@tailor-platform/tailor-proto/tailordb_resource_pb";
import { describe, expect, test } from "vitest";
import { areNormalizedEqual, normalizeProtoConfig, stableStringify } from "./compare";
import {
areNormalizedEqual,
normalizeProtoConfig,
stableStringify,
toComparableProtoJson,
} from "./compare";

describe("compare policy", () => {
// Generic compare preserves type distinctions.
Expand All @@ -15,4 +22,24 @@ describe("compare policy", () => {
expect(normalizeProtoConfig({ seconds: 1n })).toEqual({ seconds: "1" });
expect(normalizeProtoConfig({ seconds: 1 })).toEqual({ seconds: 1 });
});

test("toComparableProtoJson equates an init shape with its materialized message", () => {
const init = {
name: "Invoice",
schema: { fields: { code: { type: "string", required: true } } },
};
// Deserialized messages materialize implicit proto3 fields (e.g. bools
// added to the proto later) with zero values that the init shape omits.
// optionalOnCreate is deliberately named as the canary: it is a field the
// SDK never sets, so it proves materialization happens.
const materialized = create(TailorDBTypeSchema, init);
expect(materialized.schema?.fields.code?.optionalOnCreate).toBe(false);

expect(
areNormalizedEqual(
toComparableProtoJson(TailorDBTypeSchema, init),
toComparableProtoJson(TailorDBTypeSchema, materialized),
),
).toBe(true);
});
});
21 changes: 21 additions & 0 deletions packages/sdk/src/cli/commands/deploy/compare.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,24 @@
import { create, toJson, type DescMessage, type MessageInitShape } from "@bufbuild/protobuf";

/**
* Canonicalize a proto message (or init shape) into proto JSON for comparison.
*
* Deserialized messages materialize implicit proto3 fields with their zero
* values while locally built init shapes omit them, so comparing the raw
* objects reports a diff for every field the SDK does not set — including
* fields newly added to the proto. Proto JSON omits implicit zero values on
* both sides, which matches wire semantics (unset == zero value).
* @param schema - Message schema describing the value
* @param value - Message or init shape to canonicalize
* @returns Proto JSON representation of the value
*/
export function toComparableProtoJson<Desc extends DescMessage>(
schema: Desc,
value: MessageInitShape<Desc>,
): unknown {
return toJson(schema, create(schema, value));
}
Comment on lines +1 to +20

/**
* Stable JSON-like serialization that sorts object keys and ignores proto runtime metadata.
* @param value - Value to serialize
Expand Down
43 changes: 43 additions & 0 deletions packages/sdk/src/cli/commands/deploy/idp.plan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ type MockIdpServiceOpts = {
clients?: string[];
publishUserEvents?: boolean | undefined;
gqlOperations?: Record<string, boolean | undefined>;
omitUserAuthPolicy?: boolean;
};

function createMockApplication(opts?: {
Expand Down Expand Up @@ -97,6 +98,9 @@ function createMockApplication(opts?: {
} else {
result.publishUserEvents = true;
}
if (service.omitUserAuthPolicy) {
delete result.userAuthPolicy;
}
return result;
}),
} as unknown as Application;
Expand Down Expand Up @@ -218,6 +222,45 @@ describe("planIdP", () => {
expect(result.changeSet.service.unchanged).toHaveLength(0);
});

test("marks idp service unchanged when userAuthPolicy is omitted and remote returns server defaults", async () => {
// The platform fills an omitted userAuthPolicy with its own defaults
// (password_min_length 6 / password_max_length 4096, everything else zero)
// and echoes them back; that must not read as drift.
const client = createMockClient({
services: [
createMatchingRemoteService({
userAuthPolicy: {
useNonEmailIdentifier: false,
allowSelfPasswordReset: false,
passwordRequireUppercase: false,
passwordRequireLowercase: false,
passwordRequireNonAlphanumeric: false,
passwordRequireNumeric: false,
passwordMinLength: 6,
passwordMaxLength: 4096,
allowedEmailDomains: [],
allowGoogleOauth: false,
disablePasswordAuth: false,
allowMicrosoftOauth: false,
enableMfa: false,
requireMfa: false,
allowedReturnOrigins: [],
mfaIssuer: "",
},
}),
],
clients: defaultIdpClientSecret,
});

const result = await planIdP({
...createContext(client),
application: createMockApplication({ idpServices: [{ omitUserAuthPolicy: true }] }),
});

expect(result.changeSet.service.updates).toHaveLength(0);
expect(result.changeSet.service.unchanged).toHaveLength(1);
});

test("marks idp service updated when config matches but ownership metadata is missing", async () => {
const client = createMockClient({
services: [createMatchingRemoteService({ label: undefined })],
Expand Down
8 changes: 6 additions & 2 deletions packages/sdk/src/cli/commands/deploy/idp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,12 @@ function normalizeComparableUserAuthPolicy(
passwordRequireLowercase: policy?.passwordRequireLowercase ?? false,
passwordRequireNonAlphanumeric: policy?.passwordRequireNonAlphanumeric ?? false,
passwordRequireNumeric: policy?.passwordRequireNumeric ?? false,
passwordMinLength: policy?.passwordMinLength ?? 0,
passwordMaxLength: policy?.passwordMaxLength ?? 0,
// The platform fills an omitted policy with password_min_length 6 and
// password_max_length 4096 and echoes those back; it also coerces an
// explicit 0 to the same defaults, which is why these use || (not ??) —
// every falsy local value must compare equal to the stored defaults.
passwordMinLength: policy?.passwordMinLength || 6,
passwordMaxLength: policy?.passwordMaxLength || 4096,
allowedEmailDomains: (policy?.allowedEmailDomains ?? []).toSorted(),
allowGoogleOauth: policy?.allowGoogleOauth ?? false,
disablePasswordAuth: policy?.disablePasswordAuth ?? false,
Expand Down
Loading
Loading