fix(licenses): reset transition usage unless carry-over is enabled - #3255
Merged
Conversation
Run smaller license transitions synchronously and preserve explicit carry-over semantics across retained and pooled entitlements. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+24
to
+34
| const customerEntityCount = hasTransitions | ||
| ? await countEntitiesByInternalCustomerId({ | ||
| db: ctx.db, | ||
| internalCustomerId: | ||
| customerLicenseTransitions![0].incomingCustomerLicense | ||
| .internal_customer_id, | ||
| cap: SYNC_BATCH_TRANSITION_MAX_ENTITIES, | ||
| }) | ||
| : 0; | ||
| const runSynchronously = | ||
| customerEntityCount < SYNC_BATCH_TRANSITION_MAX_ENTITIES; |
Contributor
There was a problem hiding this comment.
If a customer has fewer than 1,000 live entities but thousands of active reusable seat assignments, this check runs the full transition inside the billing request because it counts entities instead of the assignment rows being updated, causing the request to block through many database batches and potentially time out rather than queueing the work.
Knowledge Base Used: Billing lifecycle and payment flows
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/internal/billing/v2/execute/executeAutumnActions/executeCustomerLicenseTransitions.ts
Line: 24-34
Comment:
**Entity count understates transition size**
If a customer has fewer than 1,000 live entities but thousands of active reusable seat assignments, this check runs the full transition inside the billing request because it counts entities instead of the assignment rows being updated, causing the request to block through many database batches and potentially time out rather than queueing the work.
**Knowledge Base Used:** [Billing lifecycle and payment flows](https://app.greptile.com/autumn-org-2/-/custom-context/knowledge-base/useautumn/autumn/-/docs/billing-lifecycle.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
9 issues found across 31 files
Confidence score: 2/5
computeEntitlementPriceTransitions.ts,computeBatchMigrationOperations.ts, andbatchTransition.tscan drop retained-only or same-definition changes before migration consumers process them, leaving usage unreset or license operations incomplete — propagateretainedthrough transition detection and all migration consumers.computeCustomerEntitlementPatch.tsomitscarryOverUsageson changed-definition migration patches, which can reset live balances to the new grant instead of preserving usage — pass through the migration carry-over contract.computeEntitlementPriceOperations.tshas reset edge cases for duplicate retained IDs and pooled equal-grant transitions, potentially leaving stale usage or resetting contribution cycles to the raw grant — include the target ID in retained resets and preserve carried balances when the grant is unchanged.replaceCustomerEntitlementsBatch.tscan leave a staleeffective_atmarker after a pooledset, while the retained-item test and benchmark fixtures do not reliably validate reset behavior (retained-consumable-item-transition.test.ts,benchReplaceCustomerEntitlementsBatch.ts) — clear the marker and make the tests exercise pre-reset usage with valid pooled-balance data.
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/src/internal/billing/v2/actions/batchTransition/compute/operations/entitlementPriceOperations/computeEntitlementPriceOperations.ts">
<violation number="1" location="server/src/internal/billing/v2/actions/batchTransition/compute/operations/entitlementPriceOperations/computeEntitlementPriceOperations.ts:94">
P2: When a pooled transition carries usage and the incoming grant equals the outgoing grant, `customerEntitlementPatch.balance` is undefined, but this fallback resets both contribution cycles to the raw grant. Preserve the undefined patch instead of overwriting carried pooled contribution state.</violation>
<violation number="2" location="server/src/internal/billing/v2/actions/batchTransition/compute/operations/entitlementPriceOperations/computeEntitlementPriceOperations.ts:328">
P1: When a license has multiple IDs for the same retained definition, this reset omits rows already using `toEntitlement.id`. Include the target ID in retained reset operations instead of adding it only when no other candidate exists.</violation>
</file>
<file name="server/src/internal/billing/v2/actions/batchTransition/execute/sql/replaceCustomerEntitlementsBatch.ts">
<violation number="1" location="server/src/internal/billing/v2/actions/batchTransition/execute/sql/replaceCustomerEntitlementsBatch.ts:51">
P2: When a pooled `set` transition replaces a contribution with a pending `effective_at`, this branch leaves a stale deferred marker after making current and next amounts equal. Clear `effective_at` in the set assignment so the next promotion does not treat the already-applied transition as pending.</violation>
</file>
<file name="server/tests/integration/licenses/billing/transitions/scheduled-switch/item-transitions/retained-consumable-item-transition.test.ts">
<violation number="1" location="server/tests/integration/licenses/billing/transitions/scheduled-switch/item-transitions/retained-consumable-item-transition.test.ts:1">
P3: The header comment says these are "paid→free seat switches," but both parent plans are paid (fromParentPrice 50, toParentPrice 20) and both seat products carry a $10 price. This is a paid→paid downgrade; the paid→free case is the non-consumable sibling test. Update the comment so it doesn't mislead readers about what the test covers.</violation>
<violation number="2" location="server/tests/integration/licenses/billing/transitions/scheduled-switch/item-transitions/retained-consumable-item-transition.test.ts:130">
P2: The 'resets retained consumable usage at activation' assertion runs after the clock is advanced to `billingPeriod.end`, which is exactly when the monthly Messages entitlement resets anyway. So `usage: 0` would pass even if the transition carried usage into the new grant, making this test unable to verify the reset-by-default behavior it claims to cover. Verify the transition semantics independently of the monthly cycle reset (for example, assert at a mid-cycle activation or against a non-cycle-resetting entitlement) so a regression to carried usage would fail the test.</violation>
</file>
<file name="server/src/internal/billing/v2/actions/batchTransition/compute/transitions/computeEntitlementPriceTransitions.ts">
<violation number="1" location="server/src/internal/billing/v2/actions/batchTransition/compute/transitions/computeEntitlementPriceTransitions.ts:82">
P1: Same-definition survivors now land in `retained` instead of `transitions`, but the migration consumers weren't updated to read it. `computeBatchMigrationOperations.ts` (replaceEntitlements, toLicenseOps) and `checkUpdatePlanTransitionEligibility.ts` only iterate `link.transitions.transitions`/`.added`/`.deleted`, so version-bump and customize paths will silently stop emitting replace ops for unchanged entitlements. Fold `retained` into those consumers (with the carry-over gating) so the migration lane matches the batch-transition lane, or this drops the reset/repoint behavior they relied on.</violation>
</file>
<file name="server/tests/perf/benchReplaceCustomerEntitlementsBatch.ts">
<violation number="1" location="server/tests/perf/benchReplaceCustomerEntitlementsBatch.ts:212">
P3: The pooled noise is incomplete: seedNoise inserts pooled_balance_contributions rows whose `pooled_balance_id` (`noise_pool_*`) is never created, and never seeds any `pooled_balances` rows, so the pooled scenarios measure a pooled_balances table holding a single row. Seed real pool rows and link each noise contribution to a pool and to a noise customer_entitlement's `pooled_contribution_id` so the pooled query's contribution/pool CTEs are exercised against realistic volumes; otherwise drop the misleading contribution noise.</violation>
</file>
<file name="server/src/internal/billing/v2/actions/batchTransition/compute/operations/entitlementPriceOperations/computeCustomerEntitlementPatch.ts">
<violation number="1" location="server/src/internal/billing/v2/actions/batchTransition/compute/operations/entitlementPriceOperations/computeCustomerEntitlementPatch.ts:106">
P1: When the migration `groupFilterReplaceRows` path updates a changed entitlement definition, this call resets each live balance to the new grant because it omits `carryOverUsages`. Preserve the migration contract by passing an explicit enabled carry-over config there, or otherwise keep migration callers on delta semantics while applying the reset default only to license transitions.</violation>
</file>
<file name="server/src/internal/billing/v2/actions/batchTransition/batchTransition.ts">
<violation number="1" location="server/src/internal/billing/v2/actions/batchTransition/batchTransition.ts:46">
P1: Retained-only product changes are still discarded before `batchTransition` runs, so their usage is not reset. Include `retained` in the upstream no-op/transition detection as well.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| return; | ||
| } | ||
|
|
||
| retained.push({ fromEntitlementPrice, toEntitlementPrice }); |
Contributor
There was a problem hiding this comment.
P1: Same-definition survivors now land in retained instead of transitions, but the migration consumers weren't updated to read it. computeBatchMigrationOperations.ts (replaceEntitlements, toLicenseOps) and checkUpdatePlanTransitionEligibility.ts only iterate link.transitions.transitions/.added/.deleted, so version-bump and customize paths will silently stop emitting replace ops for unchanged entitlements. Fold retained into those consumers (with the carry-over gating) so the migration lane matches the batch-transition lane, or this drops the reset/repoint behavior they relied on.
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/actions/batchTransition/compute/transitions/computeEntitlementPriceTransitions.ts, line 82:
<comment>Same-definition survivors now land in `retained` instead of `transitions`, but the migration consumers weren't updated to read it. `computeBatchMigrationOperations.ts` (replaceEntitlements, toLicenseOps) and `checkUpdatePlanTransitionEligibility.ts` only iterate `link.transitions.transitions`/`.added`/`.deleted`, so version-bump and customize paths will silently stop emitting replace ops for unchanged entitlements. Fold `retained` into those consumers (with the carry-over gating) so the migration lane matches the batch-transition lane, or this drops the reset/repoint behavior they relied on.</comment>
<file context>
@@ -73,13 +76,16 @@ export const computeEntitlementPriceTransitions = ({
+ return;
}
+
+ retained.push({ fromEntitlementPrice, toEntitlementPrice });
});
</file context>
| const balance = computeBalancePatch({ | ||
| fromInitialState, | ||
| toInitialState, | ||
| carryUsage: shouldCarryOverUsage({ toEntitlement, carryOverUsages }), |
Contributor
There was a problem hiding this comment.
P1: When the migration groupFilterReplaceRows path updates a changed entitlement definition, this call resets each live balance to the new grant because it omits carryOverUsages. Preserve the migration contract by passing an explicit enabled carry-over config there, or otherwise keep migration callers on delta semantics while applying the reset default only to license transitions.
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/actions/batchTransition/compute/operations/entitlementPriceOperations/computeCustomerEntitlementPatch.ts, line 106:
<comment>When the migration `groupFilterReplaceRows` path updates a changed entitlement definition, this call resets each live balance to the new grant because it omits `carryOverUsages`. Preserve the migration contract by passing an explicit enabled carry-over config there, or otherwise keep migration callers on delta semantics while applying the reset default only to license transitions.</comment>
<file context>
@@ -75,7 +100,11 @@ export const computeCustomerEntitlementPatch = ({
+ const balance = computeBalancePatch({
+ fromInitialState,
+ toInitialState,
+ carryUsage: shouldCarryOverUsage({ toEntitlement, carryOverUsages }),
+ });
</file context>
|
|
||
| const hasEntitlementPriceTransitions = | ||
| entitlementPriceTransitions.transitions.length > 0 || | ||
| entitlementPriceTransitions.retained.length > 0 || |
Contributor
There was a problem hiding this comment.
P1: Retained-only product changes are still discarded before batchTransition runs, so their usage is not reset. Include retained in the upstream no-op/transition detection as well.
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/actions/batchTransition/batchTransition.ts, line 46:
<comment>Retained-only product changes are still discarded before `batchTransition` runs, so their usage is not reset. Include `retained` in the upstream no-op/transition detection as well.</comment>
<file context>
@@ -43,6 +43,7 @@ export const batchTransition = async ({
const hasEntitlementPriceTransitions =
entitlementPriceTransitions.transitions.length > 0 ||
+ entitlementPriceTransitions.retained.length > 0 ||
entitlementPriceTransitions.added.length > 0 ||
entitlementPriceTransitions.deleted.length > 0;
</file context>
| continue; | ||
| } | ||
|
|
||
| const operation = computeReplaceOperation({ |
Contributor
There was a problem hiding this comment.
P1: When a license has multiple IDs for the same retained definition, this reset omits rows already using toEntitlement.id. Include the target ID in retained reset operations instead of adding it only when no other candidate exists.
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/actions/batchTransition/compute/operations/entitlementPriceOperations/computeEntitlementPriceOperations.ts, line 328:
<comment>When a license has multiple IDs for the same retained definition, this reset omits rows already using `toEntitlement.id`. Include the target ID in retained reset operations instead of adding it only when no other candidate exists.</comment>
<file context>
@@ -275,10 +295,44 @@ export const computeEntitlementPriceOperations = ({
+ continue;
+ }
+
+ const operation = computeReplaceOperation({
+ candidateOutgoingEntitlements,
+ transition,
</file context>
| } | ||
|
|
||
| const incrementAmount = | ||
| const pooledContributionPatch = |
Contributor
There was a problem hiding this comment.
P2: When a pooled transition carries usage and the incoming grant equals the outgoing grant, customerEntitlementPatch.balance is undefined, but this fallback resets both contribution cycles to the raw grant. Preserve the undefined patch instead of overwriting carried pooled contribution state.
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/actions/batchTransition/compute/operations/entitlementPriceOperations/computeEntitlementPriceOperations.ts, line 94:
<comment>When a pooled transition carries usage and the incoming grant equals the outgoing grant, `customerEntitlementPatch.balance` is undefined, but this fallback resets both contribution cycles to the raw grant. Preserve the undefined patch instead of overwriting carried pooled contribution state.</comment>
<file context>
@@ -82,10 +91,15 @@ const computeReplaceOperation = ({
}
- const incrementAmount =
+ const pooledContributionPatch =
customerEntitlementPatch.balance?.type === "increment"
- ? customerEntitlementPatch.balance.amount
</file context>
| next_cycle_contribution = contribution.next_cycle_contribution + ${patch.amount}` | ||
| : sql` | ||
| current_contribution = ${patch.amount}, | ||
| next_cycle_contribution = ${patch.amount}`; |
Contributor
There was a problem hiding this comment.
P2: When a pooled set transition replaces a contribution with a pending effective_at, this branch leaves a stale deferred marker after making current and next amounts equal. Clear effective_at in the set assignment so the next promotion does not treat the already-applied transition as pending.
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/actions/batchTransition/execute/sql/replaceCustomerEntitlementsBatch.ts, line 51:
<comment>When a pooled `set` transition replaces a contribution with a pending `effective_at`, this branch leaves a stale deferred marker after making current and next amounts equal. Clear `effective_at` in the set assignment so the next promotion does not treat the already-applied transition as pending.</comment>
<file context>
@@ -22,68 +22,107 @@ const assignedSeatFilter = (
+ next_cycle_contribution = contribution.next_cycle_contribution + ${patch.amount}`
+ : sql`
+ current_contribution = ${patch.amount},
+ next_cycle_contribution = ${patch.amount}`;
+ const syntheticBalanceAssignment =
+ patch.type === "increment"
</file context>
Suggested change
| next_cycle_contribution = ${patch.amount}`; | |
| next_cycle_contribution = ${patch.amount}, | |
| effective_at = NULL`; |
| scenario.customerId, | ||
| scenario.entities[index].id, | ||
| ); | ||
| const usage = 0; |
Contributor
There was a problem hiding this comment.
P2: The 'resets retained consumable usage at activation' assertion runs after the clock is advanced to billingPeriod.end, which is exactly when the monthly Messages entitlement resets anyway. So usage: 0 would pass even if the transition carried usage into the new grant, making this test unable to verify the reset-by-default behavior it claims to cover. Verify the transition semantics independently of the monthly cycle reset (for example, assert at a mid-cycle activation or against a non-cycle-resetting entitlement) so a regression to carried usage would fail the test.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/tests/integration/licenses/billing/transitions/scheduled-switch/item-transitions/retained-consumable-item-transition.test.ts, line 130:
<comment>The 'resets retained consumable usage at activation' assertion runs after the clock is advanced to `billingPeriod.end`, which is exactly when the monthly Messages entitlement resets anyway. So `usage: 0` would pass even if the transition carried usage into the new grant, making this test unable to verify the reset-by-default behavior it claims to cover. Verify the transition semantics independently of the monthly cycle reset (for example, assert at a mid-cycle activation or against a non-cycle-resetting entitlement) so a regression to carried usage would fail the test.</comment>
<file context>
@@ -0,0 +1,162 @@
+ scenario.customerId,
+ scenario.entities[index].id,
+ );
+ const usage = 0;
+ expectBalanceCorrect({
+ customer: entity,
</file context>
| @@ -0,0 +1,162 @@ | |||
| /** Scheduled paid→free seat switches keep the same messages entitlement. | |||
Contributor
There was a problem hiding this comment.
P3: The header comment says these are "paid→free seat switches," but both parent plans are paid (fromParentPrice 50, toParentPrice 20) and both seat products carry a $10 price. This is a paid→paid downgrade; the paid→free case is the non-consumable sibling test. Update the comment so it doesn't mislead readers about what the test covers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/tests/integration/licenses/billing/transitions/scheduled-switch/item-transitions/retained-consumable-item-transition.test.ts, line 1:
<comment>The header comment says these are "paid→free seat switches," but both parent plans are paid (fromParentPrice 50, toParentPrice 20) and both seat products carry a $10 price. This is a paid→paid downgrade; the paid→free case is the non-consumable sibling test. Update the comment so it doesn't mislead readers about what the test covers.</comment>
<file context>
@@ -0,0 +1,162 @@
+/** Scheduled paid→free seat switches keep the same messages entitlement.
+ * Activation resolves carry from the org transition rule only — the original
+ * attach's carry_over_usages is not persisted — so default is a usage reset. */
</file context>
Suggested change
| /** Scheduled paid→free seat switches keep the same messages entitlement. | |
| /** Scheduled paid→paid seat downgrades keep the same messages entitlement. |
| ) | ||
| SELECT | ||
| 'noise_contribution_' || i, | ||
| 'noise_pool_' || i, |
Contributor
There was a problem hiding this comment.
P3: The pooled noise is incomplete: seedNoise inserts pooled_balance_contributions rows whose pooled_balance_id (noise_pool_*) is never created, and never seeds any pooled_balances rows, so the pooled scenarios measure a pooled_balances table holding a single row. Seed real pool rows and link each noise contribution to a pool and to a noise customer_entitlement's pooled_contribution_id so the pooled query's contribution/pool CTEs are exercised against realistic volumes; otherwise drop the misleading contribution noise.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/tests/perf/benchReplaceCustomerEntitlementsBatch.ts, line 212:
<comment>The pooled noise is incomplete: seedNoise inserts pooled_balance_contributions rows whose `pooled_balance_id` (`noise_pool_*`) is never created, and never seeds any `pooled_balances` rows, so the pooled scenarios measure a pooled_balances table holding a single row. Seed real pool rows and link each noise contribution to a pool and to a noise customer_entitlement's `pooled_contribution_id` so the pooled query's contribution/pool CTEs are exercised against realistic volumes; otherwise drop the misleading contribution noise.</comment>
<file context>
@@ -0,0 +1,547 @@
+ )
+ SELECT
+ 'noise_contribution_' || i,
+ 'noise_pool_' || i,
+ 'noise_seat_' || i,
+ 'noise_ce_' || i,
</file context>
Unused assignment CPs should receive add/replace/delete, and migration item replaces must keep carrying usage after the default-reset change. Co-authored-by: Cursor <cursoragent@cursor.com>
6 tasks
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
1 participant
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.
Run smaller license transitions synchronously and preserve explicit carry-over semantics across retained and pooled entitlements.
Co-authored-by: Cursor cursoragent@cursor.com
Stack created with GitHub Stacks CLI • Give Feedback 💬
Summary by cubic
Resets license transition usage unless carry-over is enabled, and runs smaller transitions synchronously. Retained (same-definition) entitlements now reset to the incoming grant by default, pooled balances reset to the new grant, and usage carries only when
carry_over_usagesis enabled, the feature is allocated, or the entitlement hascarry_from_previousset.carryOverUsagesresolution and propagation through plan compute, batch transition setup, and the transition record.Written for commit 68c7e4c. Summary will update on new commits.
Greptile Summary
This PR changes license transitions so usage resets unless carry-over is explicitly enabled, while preserving carry-over for allocated features, selected consumables, and migration edits.
Confidence Score: 4/5
The PR does not appear safe to merge until synchronous execution is based on the actual transition workload rather than only the number of live entities.
A customer can have fewer than 1,000 live entities but many active unassigned seats; the new threshold then awaits a transition in the billing request even though replacement and deletion SQL process all of those seat entitlement rows.
Files Needing Attention: server/src/internal/billing/v2/execute/executeAutumnActions/executeCustomerLicenseTransitions.ts, server/src/internal/entities/repos/countEntitiesByInternalCustomerId.ts
Important Files Changed
Reviews (2): Last reviewed commit: "fix(licenses): include unused seats in e..." | Re-trigger Greptile
Context used: