feat(licenses): support child plan propagation#2295
Conversation
Add license parent preview and propagation controls, preserve customer configurations across catalog updates, and organize core license catalog coverage.
|
Automatic Review Skipped Too many files for automatic review. If you would still like a review, you can trigger one manually by commenting: |
|
Capy auto-review is paused for this organization because the usage-cycle auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
Mock the license-reference collaborators added to product item updates so existing atomicity and lock tests exercise their intended paths.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
| if (childWasVersioned) { | ||
| for (const target of prepared.allParents) { | ||
| if (target.selected && !target.hasCustomers) continue; | ||
| await updateParent({ | ||
| parent: target.parent, | ||
| licenses: buildPreservedLicenses({ target, newChild, newChildPlan }), | ||
| versioning: "in_place", | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| for (const target of prepared.selectedParents) { | ||
| if (!childWasVersioned && !target.hasCustomers && !forceVersion) continue; | ||
| await updateParent({ | ||
| parent: target.parent, | ||
| licenses: buildPropagatedLicenses({ target, newChild }), | ||
| versioning: target.isLatest ? "inherit" : "in_place", | ||
| }); | ||
| } |
There was a problem hiding this comment.
Double update for
selected + hasCustomers when child was versioned
When childWasVersioned is true, any parent that is both selected and hasCustomers falls through the first loop (the continue only skips selected && !hasCustomers) and is also processed in the second loop. This causes two sequential updateProduct calls: the first applies buildPreservedLicenses in_place (rebased customize) and the second immediately overwrites it with buildPropagatedLicenses (original snapshot customize). The first write is wasted and triggers an unnecessary Stripe resource sync and catalog rebase. For latest parents the effect is worse — the first call modifies the old version in place and the second call creates a new version, leaving the old version with a stale rebased customize. The guard in the first loop should likely be if (target.selected) continue so all selected parents are handled exclusively by the second loop.
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/internal/licenses/actions/propagation/applyLicenseParentPropagation.ts
Line: 80-98
Comment:
**Double update for `selected + hasCustomers` when child was versioned**
When `childWasVersioned` is `true`, any parent that is both `selected` and `hasCustomers` falls through the first loop (the `continue` only skips `selected && !hasCustomers`) and is also processed in the second loop. This causes two sequential `updateProduct` calls: the first applies `buildPreservedLicenses` in_place (rebased customize) and the second immediately overwrites it with `buildPropagatedLicenses` (original snapshot customize). The first write is wasted and triggers an unnecessary Stripe resource sync and catalog rebase. For latest parents the effect is worse — the first call modifies the old version in place and the second call creates a new version, leaving the old version with a stale rebased customize. The guard in the first loop should likely be `if (target.selected) continue` so all selected parents are handled exclusively by the second loop.
How can I resolve this? If you propose a fix, please make it concise.| editedBasePlan: editedChildPlan, | ||
| diff: childDiff, | ||
| variantPlan: currentEffectivePlan, | ||
| features: ctx.features, | ||
| }), | ||
| license_changes: [licenseChange], | ||
| }); |
There was a problem hiding this comment.
previews is already typed as PlanUpdatePreviewLicenseParent[] — every element is a parsed Zod value, so the preview !== null predicate is always true. Removing the filter avoids a misleading dead-code guard.
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/internal/product/actions/previewUpdatePlan/previewAffectedLicenseParents.ts
Line: 197-203
Comment:
**Redundant `!== null` type guard**
`previews` is already typed as `PlanUpdatePreviewLicenseParent[]` — every element is a parsed Zod value, so the `preview !== null` predicate is always `true`. Removing the filter avoids a misleading dead-code guard.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
7 issues found and verified against the latest diff
Confidence score: 2/5
server/src/internal/product/actions/deleteProduct.tscan allow deleting a license that is still referenced by customized/retired parent-license links, leaving preserved customer configurations pointing at deleted data and risking broken behavior in downstream flows — expand the reference lookup beyond catalog links and block deletion until those references are handled.server/src/internal/catalog/actions/updateCatalog/updateCatalog.tsandvite/src/views/products/plan/versioning/PlanChangeDialog.tsxboth risk silently applying less than requested: parent-only propagation targets can be skipped, and metadata-only “all versions” saves can update only the current version — align the update gating and keep the effectiveall_versionsintent in metadata-only submissions before merging.server/src/internal/billing/v2/compute/customerLicenseTransitions/pairCustomerLicensesByLicensePlan.tscan let dangling customer-license rows preempt a valid live-license transition, which may produce incorrect transition outcomes for real customers — filter dead-link rows before successor matching so fallback IDs cannot claim incoming licenses.server/tests/integration/licenses/catalog-update/child-plan/preserve/versioned-child.test.tscurrently has an unreachable assertion block due to an earlyreturn, so the test passes without validating the behavior it claims to cover — remove the early return and rerun to ensure regressions in child-plan preserve/versioning logic are actually caught.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/tests/integration/licenses/catalog-update/child-plan/preserve/versioned-child.test.ts">
<violation number="1" location="server/tests/integration/licenses/catalog-update/child-plan/preserve/versioned-child.test.ts:51">
P0: Unreachable `return;` on line 64 bypasses all test assertions — the entire block after it (`getFullLicenseProduct`, `expectLicenseDefinitionCorrect`, `plans.update`, all `expect()` calls) never runs. The test passes trivially regardless of behavior. Remove the early return so the assertions execute.</violation>
</file>
<file name="server/src/internal/product/actions/deleteProduct.ts">
<violation number="1" location="server/src/internal/product/actions/deleteProduct.ts:87">
P1: Deleting a license still succeeds when it is referenced by a customized or retired parent-license link, because this lookup only returns catalog links. Those preserved customer configurations retain the deleted license's internal product ID; include all referencing `planLicenses` rows (or reject deletion when any exists).</violation>
</file>
<file name="server/src/internal/catalog/actions/updateCatalog/updateCatalog.ts">
<violation number="1" location="server/src/internal/catalog/actions/updateCatalog/updateCatalog.ts:255">
P1: Catalog updates that only select license-parent propagation targets are skipped before `updateProduct`, so explicit parent propagation has no effect unless the child also has a plan or variant edit. Treat nonempty `update_license_parents` as work to process, and ensure the no-product-update path in `updateProduct` applies the prepared parent propagation.</violation>
</file>
<file name="vite/src/views/products/plan/versioning/PlanChangeDialog.tsx">
<violation number="1" location="vite/src/views/products/plan/versioning/PlanChangeDialog.tsx:273">
P1: Metadata-only saves now submit an in-place update without `all_versions`, so settings advertised as applying to every version can update only the current version. Preserve the `"all"` effective choice for `isMetadataOnly` so the existing all-version request path runs.</violation>
</file>
<file name="server/src/internal/product/actions/inPlaceUpdateUtils.ts">
<violation number="1" location="server/src/internal/product/actions/inPlaceUpdateUtils.ts:107">
P3: Catalog updates fetch every matching license reference merely to decide whether any exists. A heavily reused item can return thousands of rows; add an existence/`LIMIT 1` repo query here, leaving full ID collection to retirement.</violation>
</file>
<file name="vite/src/views/products/plan/versioning/buildMigrateTargets.ts">
<violation number="1" location="vite/src/views/products/plan/versioning/buildMigrateTargets.ts:143">
P2: Parent review can label a new version when this parent has customers but its propagated change is not versionable. Derive the status from `entry.versionable`, matching the backend preview and strategy logic.</violation>
</file>
<file name="server/src/internal/billing/v2/compute/customerLicenseTransitions/pairCustomerLicensesByLicensePlan.ts">
<violation number="1" location="server/src/internal/billing/v2/compute/customerLicenseTransitions/pairCustomerLicensesByLicensePlan.ts:31">
P1: A dangling customer-license row can now suppress a valid live-license transition. Filter dead-link rows before calling `matchCustomerLicenseSuccessors`; filtering after matching lets their fallback id claim the incoming pool, so its match is discarded and the live pool is left unpropagated.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| }), | ||
| ], | ||
| }); | ||
| return; |
There was a problem hiding this comment.
P0: Unreachable return; on line 64 bypasses all test assertions — the entire block after it (getFullLicenseProduct, expectLicenseDefinitionCorrect, plans.update, all expect() calls) never runs. The test passes trivially regardless of behavior. Remove the early return so the assertions execute.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/tests/integration/licenses/catalog-update/child-plan/preserve/versioned-child.test.ts, line 51:
<comment>Unreachable `return;` on line 64 bypasses all test assertions — the entire block after it (`getFullLicenseProduct`, `expectLicenseDefinitionCorrect`, `plans.update`, all `expect()` calls) never runs. The test passes trivially regardless of behavior. Remove the early return so the assertions execute.</comment>
<file context>
@@ -0,0 +1,125 @@
+ }),
+ ],
+ });
+ return;
+
+ const before = await getFullLicenseProduct({
</file context>
| orgId: org.id, | ||
| env, | ||
| }), | ||
| planLicenseRepo.listCatalogByLicenseInternalProductIds({ |
There was a problem hiding this comment.
P1: Deleting a license still succeeds when it is referenced by a customized or retired parent-license link, because this lookup only returns catalog links. Those preserved customer configurations retain the deleted license's internal product ID; include all referencing planLicenses rows (or reject deletion when any exists).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/internal/product/actions/deleteProduct.ts, line 87:
<comment>Deleting a license still succeeds when it is referenced by a customized or retired parent-license link, because this lookup only returns catalog links. Those preserved customer configurations retain the deleted license's internal product ID; include all referencing `planLicenses` rows (or reject deletion when any exists).</comment>
<file context>
@@ -71,8 +84,18 @@ export const deleteProduct = async ({
orgId: org.id,
env,
}),
+ planLicenseRepo.listCatalogByLicenseInternalProductIds({
+ db,
+ licenseInternalProductIds: deletionInternalProductIds,
</file context>
| outgoingCustomerLicenses: outgoingCustomerProduct.customer_licenses ?? [], | ||
| incomingCustomerLicenses: incomingCustomerProduct.customer_licenses ?? [], |
There was a problem hiding this comment.
P1: A dangling customer-license row can now suppress a valid live-license transition. Filter dead-link rows before calling matchCustomerLicenseSuccessors; filtering after matching lets their fallback id claim the incoming pool, so its match is discarded and the live pool is left unpropagated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/internal/billing/v2/compute/customerLicenseTransitions/pairCustomerLicensesByLicensePlan.ts, line 31:
<comment>A dangling customer-license row can now suppress a valid live-license transition. Filter dead-link rows before calling `matchCustomerLicenseSuccessors`; filtering after matching lets their fallback id claim the incoming pool, so its match is discarded and the live pool is left unpropagated.</comment>
<file context>
@@ -16,60 +17,37 @@ export type CustomerLicensePair = {
-
- return pairs;
+ const { matches } = matchCustomerLicenseSuccessors({
+ outgoingCustomerLicenses: outgoingCustomerProduct.customer_licenses ?? [],
+ incomingCustomerLicenses: incomingCustomerProduct.customer_licenses ?? [],
+ });
</file context>
| outgoingCustomerLicenses: outgoingCustomerProduct.customer_licenses ?? [], | |
| incomingCustomerLicenses: incomingCustomerProduct.customer_licenses ?? [], | |
| outgoingCustomerLicenses: ( | |
| outgoingCustomerProduct.customer_licenses ?? [] | |
| ).filter((customerLicense) => customerLicense.planLicense), | |
| incomingCustomerLicenses: ( | |
| incomingCustomerProduct.customer_licenses ?? [] | |
| ).filter((customerLicense) => customerLicense.planLicense), |
| updates: updateParams, | ||
| initialFullProduct: current, | ||
| propagateToVariants: update_variant_ids ?? [], | ||
| licenseParentUpdates: update_license_parents, |
There was a problem hiding this comment.
P1: Catalog updates that only select license-parent propagation targets are skipped before updateProduct, so explicit parent propagation has no effect unless the child also has a plan or variant edit. Treat nonempty update_license_parents as work to process, and ensure the no-product-update path in updateProduct applies the prepared parent propagation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/internal/catalog/actions/updateCatalog/updateCatalog.ts, line 255:
<comment>Catalog updates that only select license-parent propagation targets are skipped before `updateProduct`, so explicit parent propagation has no effect unless the child also has a plan or variant edit. Treat nonempty `update_license_parents` as work to process, and ensure the no-product-update path in `updateProduct` applies the prepared parent propagation.</comment>
<file context>
@@ -251,6 +252,7 @@ const upsertPlans = async ({
updates: updateParams,
initialFullProduct: current,
propagateToVariants: update_variant_ids ?? [],
+ licenseParentUpdates: update_license_parents,
variantUpdates,
});
</file context>
| const hasVariants = variants.length > 0; | ||
| const showVersionStrategy = | ||
| !isMetadataOnly && !!preview && previewHasVersionableTargets(preview); | ||
| const effectiveVersionChoice = showVersionStrategy ? versionChoice : "update"; |
There was a problem hiding this comment.
P1: Metadata-only saves now submit an in-place update without all_versions, so settings advertised as applying to every version can update only the current version. Preserve the "all" effective choice for isMetadataOnly so the existing all-version request path runs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At vite/src/views/products/plan/versioning/PlanChangeDialog.tsx, line 273:
<comment>Metadata-only saves now submit an in-place update without `all_versions`, so settings advertised as applying to every version can update only the current version. Preserve the `"all"` effective choice for `isMetadataOnly` so the existing all-version request path runs.</comment>
<file context>
@@ -245,28 +268,15 @@ export default function PlanChangeDialog({
const hasVariants = variants.length > 0;
+ const showVersionStrategy =
+ !isMetadataOnly && !!preview && previewHasVersionableTargets(preview);
+ const effectiveVersionChoice = showVersionStrategy ? versionChoice : "update";
// Only main-plan changes propagate to variants; license-link edits stay on
// the selected parent version.
</file context>
| const effectiveVersionChoice = showVersionStrategy ? versionChoice : "update"; | |
| const effectiveVersionChoice = showVersionStrategy | |
| ? versionChoice | |
| : isMetadataOnly | |
| ? "all" | |
| : "update"; |
| const targetId = getLicenseParentTargetId(entry); | ||
| if (!selectedLicenseParentIdSet.has(targetId)) continue; | ||
| const planChanges = entry.license_changes[0]?.plan_changes; | ||
| const createsNewVersion = isNewVersion && entry.has_customers; |
There was a problem hiding this comment.
P2: Parent review can label a new version when this parent has customers but its propagated change is not versionable. Derive the status from entry.versionable, matching the backend preview and strategy logic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At vite/src/views/products/plan/versioning/buildMigrateTargets.ts, line 143:
<comment>Parent review can label a new version when this parent has customers but its propagated change is not versionable. Derive the status from `entry.versionable`, matching the backend preview and strategy logic.</comment>
<file context>
@@ -0,0 +1,164 @@
+ const targetId = getLicenseParentTargetId(entry);
+ if (!selectedLicenseParentIdSet.has(targetId)) continue;
+ const planChanges = entry.license_changes[0]?.plan_changes;
+ const createsNewVersion = isNewVersion && entry.has_customers;
+ targets.push({
+ id: `license-parent:${targetId}`,
</file context>
| const createsNewVersion = isNewVersion && entry.has_customers; | |
| const createsNewVersion = isNewVersion && entry.versionable; |
| priceIds: currentFullProduct.prices.map((price) => price.id), | ||
| priceIds, | ||
| }), | ||
| licenseItemRepo.listReferencedEntitlementIds({ db, entitlementIds }), |
There was a problem hiding this comment.
P3: Catalog updates fetch every matching license reference merely to decide whether any exists. A heavily reused item can return thousands of rows; add an existence/LIMIT 1 repo query here, leaving full ID collection to retirement.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/internal/product/actions/inPlaceUpdateUtils.ts, line 107:
<comment>Catalog updates fetch every matching license reference merely to decide whether any exists. A heavily reused item can return thousands of rows; add an existence/`LIMIT 1` repo query here, leaving full ID collection to retirement.</comment>
<file context>
@@ -85,20 +86,34 @@ export const productItemsHaveCustomerReferences = async ({
- priceIds: currentFullProduct.prices.map((price) => price.id),
+ priceIds,
}),
+ licenseItemRepo.listReferencedEntitlementIds({ db, entitlementIds }),
+ licenseItemRepo.listReferencedPriceIds({ db, priceIds }),
]);
</file context>
There was a problem hiding this comment.
1 issue found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/tests/integration/crud/plans/variants/core-contract.test.ts">
<violation number="1" location="server/tests/integration/crud/plans/variants/core-contract.test.ts:34">
P3: Line 9 of the doc comment lists `preview_update is idempotent, no writes` as a contract item under test, but the corresponding test (section 7) was removed in this diff. Readers scanning the contract list will assume idempotency is still tested.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| @@ -31,12 +31,12 @@ import { | |||
| type PlanUpdatePreview, | |||
There was a problem hiding this comment.
P3: Line 9 of the doc comment lists preview_update is idempotent, no writes as a contract item under test, but the corresponding test (section 7) was removed in this diff. Readers scanning the contract list will assume idempotency is still tested.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/tests/integration/crud/plans/variants/core-contract.test.ts, line 34:
<comment>Line 9 of the doc comment lists `preview_update is idempotent, no writes` as a contract item under test, but the corresponding test (section 7) was removed in this diff. Readers scanning the contract list will assume idempotency is still tested.</comment>
<file context>
@@ -31,12 +31,12 @@ import {
} from "@autumn/shared";
-import { TestFeature, getFeatures } from "@tests/setup/v2Features";
-import ctx from "@tests/utils/testInitUtils/createTestContext.js";
+import { getFeatures, TestFeature } from "@tests/setup/v2Features";
import { expectAutumnError } from "@tests/utils/expectUtils/expectErrUtils.js";
-import { initScenario, s } from "@tests/utils/testInitUtils/initScenario.js";
</file context>
Summary
Validation
Full integration suite was not run.
Summary by cubic
Adds child license plan propagation with preview and explicit parent targets. Preserves customer license configurations across catalog updates, hardens immediate-switch safeguards, and improves attach-target validation and customer item-reference detection.
New Features
plans.preview_updateincludeslicense_parents; request withinclude_license_parents.plans.updateacceptsupdate_license_parentsto update selected parent plan versions.versionpin).Migration
update_license_parentsto propagate child license changes to selected parent versions.include_license_parentsand readlicense_parentsin the response.licenses[].versionon parent links; links resolve to the latest child.Written for commit 291bcb9. Summary will update on new commits.
Greptile Summary
This PR adds end-to-end support for child-plan propagation in the license system: when a child (license) plan is edited, its parent plans can now receive a preview of the impact, and the user can explicitly select which parent versions to update alongside the child. It also hardens plan-deletion guards and attach-target validation for license plans.
update_license_parentsparameter andlicense_parentspreview field; theversionpin field is removed fromPlanLicenseParams.prepareLicenseParentPropagation/applyLicenseParentPropagationsnapshot all parent contexts before any writes and apply preserve-vs-propagate semantics;PlanChangeDialogreusesPropagationTargetsStepfor both variant and license-parent selection.Confidence Score: 4/5
The change is safe to merge. The one structural concern in applyLicenseParentPropagation triggers two updateProduct calls on the same parent when the child is versioned, but the final written state is the intended propagated result.
Most new paths are well-scoped and the propagation snapshot+apply split is sound. The double-update issue wastes a Stripe sync round-trip and leaves the old parent version with a stale rebased customize, but does not corrupt the final state. The full integration suite was not run, which is a gap given the breadth of the change.
server/src/internal/licenses/actions/propagation/applyLicenseParentPropagation.ts — the first-loop guard for selected parents when the child is versioned.
Important Files Changed
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "test(licenses): 💍 update product item r..." | Re-trigger Greptile