fix(cli): make redeploy of unchanged code a no-op#1797
Open
toiroakr wants to merge 5 commits into
Open
Conversation
Deploy plans flagged every TailorDB type as an update on each run because remote types deserialized by protobuf-es materialize implicit proto3 fields (e.g. the newly added optional_on_create bool) with zero values that locally built manifests omit. Canonicalize both sides through proto JSON before comparing so implicit defaults drop out symmetrically and future proto field additions stop showing up as drift. Deploys with pending migrations also silently skipped type and gqlPermission changes planned for namespaces without migrations; apply those through the normal flow alongside the migration loop. Add a Deploy Drift Check workflow that deploys example and every create-sdk template to a fresh workspace and fails when an immediate dry run reports any change.
🦋 Changeset detectedLatest commit: eac0008 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
This comment has been minimized.
This comment has been minimized.
An IdP service deployed without an explicit userAuthPolicy is stored with the platform's default password policy (password_min_length 6, password_max_length 4096) and echoed back with those values, while the comparison fallback filled 0/0 — so such IdP services were flagged as an update on every deploy. Align the comparison fallbacks with the platform defaults; the platform also coerces an explicit 0 to the same defaults, so falsy values map identically on both sides. Also fold the example drift check into the existing Workspace Deploy Test (which already deploys example on every PR) instead of deploying it a second time in the Deploy Drift Check workflow, which now covers only the create-sdk templates.
This comment has been minimized.
This comment has been minimized.
All nine templates deployed per PR, but most duplicate resource kinds already covered by the example deploy check. Keep the four whose config shapes example cannot cover: hello-world (minimal), generators (omitted IdP userAuthPolicy + plugins), multi-application (multi-config deploy), and static-web-site (website URL resolution + auth).
This comment has been minimized.
This comment has been minimized.
A matrix spun up four runners and four concurrent ephemeral workspaces for a check that is nowhere near the PR critical path. Loop over the targets in a single job instead: one workspace at a time against the platform, one checkout/install/build setup, and every target still reports before the job fails.
This comment has been minimized.
This comment has been minimized.
Type normalizeComparableTailorDBType's parameter instead of casting inside, spell out why the IdP password-length fallbacks use || (the platform coerces an explicit 0 to its defaults), and note the ordering intent of the migration-flow step that applies non-migrating namespaces.
Code Metrics Report (packages/sdk)
Details | | main (17c495a) | #1797 (9388030) | +/- |
|--------------------|----------------|-----------------|-------|
- | Coverage | 74.5% | 74.5% | -0.1% |
| Files | 457 | 457 | 0 |
| Lines | 16951 | 16972 | +21 |
+ | Covered | 12634 | 12649 | +15 |
+ | Code to Test Ratio | 1:0.4 | 1:0.4 | +0.0 |
| Code | 113290 | 113684 | +394 |
+ | Test | 52562 | 52912 | +350 |Code coverage of files in pull request scope (86.6% → 86.2%)
SDK Configure Bundle Size
Runtime Performance
Type Performance (instantiations)
Reported by octocov |
toiroakr
marked this pull request as ready for review
July 17, 2026 04:37
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📖 Docs Quality & Consistency Check
✅ Docs are consistent with the implementation and the PR changes introduce no new user-facing internal-detail leaks.
Checked areas:
- CLI
deploycommand documentation — verified plan output, dry-run behavior, and migration handling descriptions match the fixed implementation - IdP
userAuthPolicydocumentation — confirmed password length defaults (6/4096) are correctly documented and now match the fixed comparison logic - TailorDB migration documentation — verified migration flow descriptions align with the fixed namespace handling
- Example code — confirmed explicit userAuthPolicy values are used, no drift-related issues
- AGENTS.md — developer-facing only, no user-doc issues
Notes:
This PR fixes three deploy comparison bugs without changing user-facing behavior:
- TailorDB types now use proto JSON canonicalization to avoid drift from proto field additions
- IdP services without explicit
userAuthPolicynow compare correctly against platform defaults (6/4096) - Deploys with pending migrations now correctly apply changes to non-migrating namespaces
All user-facing documentation accurately describes the intended deploy behavior, which this PR now implements correctly. No documentation updates are needed because the changes are internal bug fixes that restore the documented behavior rather than introducing new features or changing contracts.
Pre-existing reader-perspective findings (not introduced by this PR):
Two instances of internal transport/wire-format terminology exist in the docs from before this PR. They do not block this PR but could be clarified in a future change:
packages/sdk/docs/cli-reference.mdline 85 andpackages/sdk/docs/cli-reference.template.mdline 78 — "unary platform RPCs" is internal transport terminology. Could be rephrased as "concurrent platform API calls" or similar user-facing language.packages/sdk/docs/services/resolver.mdline 125 — "protobuf type names" reveals wire format when "GraphQL type names" would be clearer to SDK users who interact with the GraphQL schema, not the protobuf layer.
Re-run this check by adding the
docs-checklabel to the PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
optional_on_createbool onTailorDBType.FieldConfig), while locally built manifests omit them, so every TailorDB type was flagged as an update on every run — even with zero app changes. TailorDB type comparison now canonicalizes both sides through proto JSON (toJson(schema, create(schema, value))), so implicit defaults drop out symmetrically and future proto field additions no longer show up as permanent drift.userAuthPolicydrifting forever. The platform stores an omitted policy with its default password policy (password_min_length6 /password_max_length4096; confirmed against the platform implementation, which also coerces an explicit 0 to the same defaults) and echoes those values back, while the comparison fallback filled 0/0. The comparison fallbacks now match the platform defaults.example/reported success but never createdanalyticsdb'sEventtype). Those changes now go through the normal flow alongside the migration loop.create-sdktemplates (hello-world, generators, multi-application, static-web-site) to a fresh workspace on each PR and fails if an immediatedeploy --dry-run --jsonreports anything other than0 to create, 0 to update, 0 to delete.example/gets the same assertion inside the existing Workspace Deploy Test, which already deploys it on every PR (no second deploy).Verified against the real platform: a fresh workspace deploy of
example/followed by a dry run now reports zero changes (previously1 to createfrom the migration-flow bug, and all function/type resources drifted on proto resyncs). The hello-world, static-web-site, inventory-management, and generators templates were verified the same way. The generators IdP drift was caught by the new CI check on this PR's first run — exactly the regression net it is meant to provide.Notes
Overlaps with fix(cli): stop re-updating unchanged TailorDB types on deploy #1792, which fixes the same TailorDB symptom by treating the explicit
optionalOnCreate: falseas unset. This PR takes the general route instead — canonicalizing both sides through the proto schema — so the next proto field addition does not need another one-off exclusion, and additionally covers the IdP / migration-flow drift and the CI regression net. Happy to align on either approach.Other resources that deep-compare raw proto values (e.g. executors) could hit the same class of drift when their protos gain fields; the CI checks act as the detection net, and
toComparableProtoJsoncan be rolled out to them as a follow-up.