Skip to content

fix: resolve "Cannot delete the default on variation" when migrating flags with variations#11

Open
nljordan wants to merge 1 commit into
launchdarkly-labs:mainfrom
nljordan:fix/variation-handling
Open

fix: resolve "Cannot delete the default on variation" when migrating flags with variations#11
nljordan wants to merge 1 commit into
launchdarkly-labs:mainfrom
nljordan:fix/variation-handling

Conversation

@nljordan
Copy link
Copy Markdown
Contributor

When updating existing flags with changed variations, the LaunchDarkly API can return "Cannot delete the default on variation" if any environment's offVariation or fallthrough points at a variation being removed or replaced.

Changes:

  • Pre-patch defaults and env offVariation/fallthrough before replacing variations when the variation count changes (reducing or replacing with different content)
  • Use targeted updates instead of full array replace:
    • Add: use JSON Patch "add" when adding variations (existing content matches)
    • Value-level: patch /variations/{i}/value and /variations/{i}/name when same count but different content
    • Replace: full replace only when reducing or when add/value strategies don't apply
  • Send defaults in a separate request after variations to avoid validation errors

Incremental sync:

  • Add content hash of migratable flag data (variations, defaults, env config)
  • Compare content hash when versions match; re-sync if hash differs
  • Ensures variation value changes are detected even when LaunchDarkly does not bump _version

…flag variations

When updating existing flags with changed variations, the LaunchDarkly API can
return "Cannot delete the default on variation" if any environment's offVariation
or fallthrough points at a variation being removed or replaced.

Changes:
- Pre-patch defaults and env offVariation/fallthrough before replacing variations
  when the variation count changes (reducing or replacing with different content)
- Use targeted updates instead of full array replace:
  - Add: use JSON Patch "add" when adding variations (existing content matches)
  - Value-level: patch /variations/{i}/value and /variations/{i}/name when same
    count but different content
  - Replace: full replace only when reducing or when add/value strategies don't apply
- Send defaults in a separate request after variations to avoid validation errors

Incremental sync:
- Add content hash of migratable flag data (variations, defaults, env config)
- Compare content hash when versions match; re-sync if hash differs
- Ensures variation value changes are detected even when LaunchDarkly does not
  bump _version
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the LaunchDarkly instance-to-instance migration script to avoid validation errors when changing flag variations (notably “Cannot delete the default on variation”) and to make incremental sync detect variation value-only changes even when LaunchDarkly does not bump _version.

Changes:

  • Add a contentHash to the incremental sync manifest to detect variation/default/env-config changes even when versions don’t change.
  • Update variation migration logic to use targeted JSON Patch ops (add / value-level updates) and pre-patch defaults/env pointers before risky variation replacements.
  • Apply defaults in a follow-up request after variation updates to avoid LaunchDarkly validation constraints.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/scripts/launchdarkly-migrations/migrate_between_ld_instances.ts Adds content hashing for incremental sync and reworks variation/default patch sequencing to prevent LD API validation errors.
deno.lock Records dependency resolution metadata (e.g., @std/cli, jsondiffpatch) for reproducible Deno installs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1623 to +1629
const prePatches: any[] = [buildPatch("defaults", "replace", { onVariation: safeIdx, offVariation: safeIdx })];
for (const [key, env] of Object.entries(destinationFlag.environments || {})) {
const e = env as any;
if (!e) continue;
if (e.offVariation !== safeIdx) prePatches.push(buildPatch(`environments/${key}/offVariation`, "replace", safeIdx));
if (e.fallthrough) prePatches.push(buildPatch(`environments/${key}/fallthrough`, "replace", { variation: safeIdx }));
}
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-patch iterates over all destinationFlag.environments and rewrites offVariation/fallthrough for every destination environment. If the destination project has environments that are not in the selected envkeys (or not mapped via --envMap), those envs will be modified here but never restored later (since the env patch loop only runs for envkeys). Consider restricting pre-patches to only the destination env keys that will be migrated (and/or explicitly restoring the original values after variations are updated).

Copilot uses AI. Check for mistakes.
for (let i = 0; i < newCount; i++) {
const src = newVariations[i], dest = destVarsClean[i];
if (!dest) continue;
if (src?.value !== dest?.value) flagLevelPatches.push({ path: `/variations/${i}/value`, op: "replace", value: src?.value });
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src?.value !== dest?.value will treat any object/array variation values as different even when they are deeply equal (because they’re different references after JSON parse), causing unnecessary PATCHes. Use the existing changed() deep comparison (or another deep-equals) for value comparison, similar to how you detect other changes in this function.

Suggested change
if (src?.value !== dest?.value) flagLevelPatches.push({ path: `/variations/${i}/value`, op: "replace", value: src?.value });
if (changed(src?.value, dest?.value)) flagLevelPatches.push({ path: `/variations/${i}/value`, op: "replace", value: src?.value });

Copilot uses AI. Check for mistakes.
Comment on lines +1644 to +1649
for (let i = 0; i < newCount; i++) {
const src = newVariations[i], dest = destVarsClean[i];
if (!dest) continue;
if (src?.value !== dest?.value) flagLevelPatches.push({ path: `/variations/${i}/value`, op: "replace", value: src?.value });
if (src?.name !== dest?.name && src?.name !== undefined) flagLevelPatches.push({ path: `/variations/${i}/name`, op: "replace", value: src?.name });
}
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The targeted same-count variation updates only patch value and (conditionally) name. If the source variation omits name but the destination has one, this won’t clear it, leaving the destination out of sync even though variationsChanged was true. Also, other fields present in variations (e.g. description) are never reconciled in this branch. Consider handling name removal (JSON Patch remove) and patching any other supported variation fields (or falling back to a full variations replace when differences are outside the supported targeted fields).

Suggested change
for (let i = 0; i < newCount; i++) {
const src = newVariations[i], dest = destVarsClean[i];
if (!dest) continue;
if (src?.value !== dest?.value) flagLevelPatches.push({ path: `/variations/${i}/value`, op: "replace", value: src?.value });
if (src?.name !== dest?.name && src?.name !== undefined) flagLevelPatches.push({ path: `/variations/${i}/name`, op: "replace", value: src?.name });
}
const supportedVariationFields = new Set(["value", "name", "description"]);
const variationPatches: any[] = [];
let requiresFullVariationReplace = false;
for (let i = 0; i < newCount; i++) {
const src = newVariations[i] || {};
const dest = destVarsClean[i] || {};
const srcKeys = Object.keys(src);
const destKeys = Object.keys(dest);
const allKeys = new Set([...srcKeys, ...destKeys]);
for (const key of allKeys) {
if (supportedVariationFields.has(key)) continue;
if (JSON.stringify(src[key]) !== JSON.stringify(dest[key])) {
requiresFullVariationReplace = true;
break;
}
}
if (requiresFullVariationReplace) break;
for (const key of supportedVariationFields) {
const srcHasKey = Object.prototype.hasOwnProperty.call(src, key);
const destHasKey = Object.prototype.hasOwnProperty.call(dest, key);
if (!srcHasKey && !destHasKey) continue;
const srcValue = src[key];
const destValue = dest[key];
if (JSON.stringify(srcValue) === JSON.stringify(destValue)) continue;
if (!srcHasKey && destHasKey) {
variationPatches.push({ path: `/variations/${i}/${key}`, op: "remove" });
} else if (srcHasKey && !destHasKey) {
variationPatches.push({ path: `/variations/${i}/${key}`, op: "add", value: srcValue });
} else {
variationPatches.push({ path: `/variations/${i}/${key}`, op: "replace", value: srcValue });
}
}
}
if (requiresFullVariationReplace) {
flagLevelPatches.push(buildPatch("variations", "replace", newVariations));
} else {
flagLevelPatches.push(...variationPatches);
}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants