fix(catalog): create explicit plan versions - #2285
Merged
Merged
Conversation
Contributor
|
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. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Contributor
There was a problem hiding this comment.
All reported issues were addressed
Confidence score: 5/5
- Safe to merge after the addressed issues were fixed.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
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
Verification
Summary by cubic
Fix explicit plan versioning end-to-end. We now enforce sequential versions and process intra-plan versions in order, preventing missing versions and keeping license pins correct.
Bug Fixes
plan.version(default to 1 if missing) and validate sequential targets against existing products.force_version; requirelatest + 1and fail with a clear error if not.sortCatalogPlansByDependenciesnow ensuresvNdepends onvN-1, so versions for the sameplan_idare applied in ascending order within a batch.Dependencies
bun.lockto pick up transitive updates.Written for commit b294ed3. Summary will update on new commits.
Greptile Summary
This PR fixes catalog plan versioning to honour explicit
versionfields supplied in the catalog update payload — previously the preview, preflight, and write paths all silently ignored or reset the caller-supplied version to1.previewCatalogPlanUpdatenow passesversion ?? 1instead of always1to new-plan previews, andcatalogPlanPreflightusesplan.version ?? 1so virtual products for license sync carry the correct version.updateCatalog.upsertPlansgrows a new code path: when a plan with an explicitversionis absent in the DB but a lower version already exists, it validates the gap (version === latest.version + 1) and callsupdateProductwithforce_version: trueinstead of falling through to a plain create.Confidence Score: 3/5
The write path in updateCatalog can silently produce an incomplete catalog if callers submit versions for the same plan_id out of ascending order — v2 params end up creating a v1 product, and the intended v2 is never written.
The core logic in upsertPlans depends on intra-plan plans being processed in ascending version order, but sortCatalogPlansByDependencies only topologically orders by license references — it provides no guarantee about version ordering within the same plan_id. A caller submitting [childId v2, childId v1] would silently get only v1 with correct data and no v2, with no error returned.
server/src/internal/catalog/actions/updateCatalog/updateCatalog.ts — the new explicit-version branch and the interaction with sortCatalogPlansByDependencies need a second look.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[upsertPlans: process planParams] --> B{current = getFull with version?} B -- found --> C[existing update path] B -- not found --> D{version defined?} D -- no --> G[createProduct - fresh plan v1] D -- yes --> E[getFull latest version, allowNotFound] E --> F{latest found?} F -- no --> G F -- yes --> H{version === latest.version + 1?} H -- no --> I[throw ProductNotFoundError] H -- yes --> J[updateProduct force_version: true] J --> K[continue to next plan] G --> K%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[upsertPlans: process planParams] --> B{current = getFull with version?} B -- found --> C[existing update path] B -- not found --> D{version defined?} D -- no --> G[createProduct - fresh plan v1] D -- yes --> E[getFull latest version, allowNotFound] E --> F{latest found?} F -- no --> G F -- yes --> H{version === latest.version + 1?} H -- no --> I[throw ProductNotFoundError] H -- yes --> J[updateProduct force_version: true] J --> K[continue to next plan] G --> KPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "chore: refresh bun lockfile" | Re-trigger Greptile