feat: gradual rollouts #572
Conversation
WalkthroughThis update introduces the "Gradual Rollout" policy rule across the backend, API, database schema, and end-to-end tests. It adds new database tables and relations, OpenAPI schema extensions, route handlers, and core rule engine logic to support gradual rollout of releases. Comprehensive tests and documentation for these features are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant DB
participant RuleEngine
Client->>API: PATCH /v1/policies/{policyId} (with gradualRollout)
API->>DB: Update policy and gradualRollout rule
DB-->>API: Confirmation
API-->>Client: Updated policy
Client->>API: GET /v1/deployment-versions/{deploymentVersionId}/releases
API->>DB: Query releases and variables for deployment version
DB-->>API: Release data
API-->>Client: Release list (with gradual rollout info)
Note over RuleEngine,DB: On policy evaluation:
RuleEngine->>DB: Fetch policy (with gradualRollout)
RuleEngine->>DB: Fetch release targets and approvals
RuleEngine->>RuleEngine: Compute rollout start, position, offset
RuleEngine-->>DB: Update release status as per rollout rule
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
e2e/api/schema.ts (1)
1368-1386:⚠️ Potential issue
versionAnyApprovalschanged to an object, but endpoints are inconsistent
Policy1.versionAnyApprovalsis now a single object (good), yet:
updatePolicy(lines 2693-2696) still expects an arrayupsertPolicy(lines 2801-2802) correctly expects a single objectThis mismatch will generate incompatible server & client stubs and fail runtime validation.
- versionAnyApprovals?: { requiredApprovalsCount?: number; }[]; + versionAnyApprovals?: components["schemas"]["VersionAnyApproval"];Please align the request bodies across all operations and add contract tests.
🧹 Nitpick comments (13)
packages/rule-engine/src/db/create-policy.ts (1)
95-97: Return the new rule to keep the API symmetric
gradualRolloutis stripped out ofrest(👍) but never added back to the object that the function returns, whereas every other rule block echoes its input back.
Caller code that relies on the create API to “round-trip” the full payload will now receiveundefined.versionUserApprovals, versionRoleApprovals, + gradualRollout, };apps/jobs/src/policy-checker/index.ts (1)
50-57: Provide a stablejobIdto BullMQ to avoid queuing duplicates
addBulkdoes not deduplicate onname; without ajobIdidentical jobs can be queued multiple times (especially given the duplication risk above).releaseTargets.map((rt) => ({ - name: `${rt.resourceId}-${rt.environmentId}-${rt.deploymentId}`, + name: "evaluate-release-target", + jobId: `${rt.resourceId}:${rt.environmentId}:${rt.deploymentId}`, data: rt, priority: 10, })),apps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/releases/openapi.ts (2)
10-21: Declare required properties for stronger client contractsAll fields are currently optional, which weakens generated typings. Mark them as required:
variables: { type: "object", additionalProperties: true }, - }, + }, + required: [ + "resource", + "environment", + "deployment", + "version", + "variables", + ],
24-78: Addtagsfor better grouping in Swagger / Redocget: { summary: "Get all releases for a deployment version", + tags: ["Releases"],e2e/tests/api/policies/gradual-rollout.spec.ts (2)
53-73: Consider testing edge cases for gradual rollout configuration.The test only covers a simple case with deployRate=1 and windowSizeMinutes=1. Consider adding tests with different values to verify the feature works across various configurations.
86-104: Test might increase CI pipeline duration.The test waits for 60 seconds between checks and runs 5 iterations, adding at least 4 minutes to the test runtime. Consider parameterizing these values for faster execution in CI environments.
- await page.waitForTimeout(60_000); + const waitTime = process.env.CI ? 1_000 : 60_000; // Use shorter wait time in CI + await page.waitForTimeout(waitTime);apps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/releases/route.ts (1)
98-109: Avoid O(n) re-serialization of static version metadata
deploymentVersion.metadatais identical for every row inreleases.
Re-computing the object inside themapincurs unnecessary CPU & memory allocation.- const fullReleases = releases.map((release) => ({ - resource: release.resource, - environment: release.environment, - deployment: release.deployment, - version: { - ...deploymentVersion, - metadata: Object.fromEntries( - deploymentVersion.metadata.map((m) => [m.key, m.value]), - ), - }, - variables: release.variableRelease.variables, - })); + const versionMetadata = Object.fromEntries( + deploymentVersion.metadata.map((m) => [m.key, m.value]), + ); + + const fullReleases = releases.map((release) => ({ + resource: release.resource, + environment: release.environment, + deployment: release.deployment, + version: { ...deploymentVersion, metadata: versionMetadata }, + variables: release.variableRelease.variables, + }));Single calculation keeps the response identical while trimming per-row overhead.
packages/rule-engine/src/rules/gradual-rollout-rule.ts (1)
40-57: Run eligibility checks in parallel to cut latency
filtercurrently awaitsgetDeploymentTimeserially for every candidate.
When the helpers perform DB/IO, the total latency scales linearly with the
number of versions.- for (const candidate of candidates) { - const deploymentTime = await this.getDeploymentTime(candidate); - const isEligible = - isAfter(now, deploymentTime) || isEqual(now, deploymentTime); - if (!isEligible) rejectionReasons.set(candidate.id, skip); - } + const evaluated = await Promise.all( + candidates.map(async (candidate) => ({ + candidate, + deploymentTime: await this.getDeploymentTime(candidate), + })), + ); + + for (const { candidate, deploymentTime } of evaluated) { + const isEligible = + isAfter(now, deploymentTime) || isEqual(now, deploymentTime); + if (!isEligible) rejectionReasons.set(candidate.id, skip); + }This keeps complexity O(n) but reduces wall-clock time to the slowest single
getDeploymentTimecall.packages/rule-engine/src/manager/version-manager-rules.ts (1)
106-137: Reduce duplicate DB work ingetReleaseTargetPosition
getReleaseTargetPositionperforms two queries per version.
Within a singlefilterinvocation this can explode into dozens of hits.Suggestions:
- Cache the
releaseTarget(it is constant across versions) in the outer
closure.- Memoise
positionlook-ups perversion.id– the deterministicmd5
expression guarantees identical results when called twice for the same
version.Indicative refactor:
const releaseTargetPromise = db.query.releaseTarget.findFirst({ where: eq(schema.releaseTarget.id, releaseTargetId), }).then(takeFirst); const positionCache = new Map<string, number>(); const getReleaseTargetPosition = async (version: Version) => { if (positionCache.has(version.id)) return positionCache.get(version.id)!; const releaseTarget = await releaseTargetPromise; // ... build orderedTargetsSubquery as today const position = await db .select() .from(orderedTargetsSubquery) .where(eq(orderedTargetsSubquery.id, releaseTargetId)) .then(takeFirst) .then((r) => r.position); positionCache.set(version.id, position); return position; };This keeps correctness while dramatically cutting DB round-trips for large
candidate sets.e2e/api/schema.ts (1)
1089-1126: Naming collision (ReleasevsRelease1) will confuse every typed consumerAuto-generated suffixes (
Release1) are a code smell that leaks generation details into the public contract.
Pick deterministic, semantically-meaningful names (e.g.ReleaseDetailed,ReleaseSummary) and regenerate the spec so downstream packages don’t end up with unstable import paths on every minor change.openapi.v1.json (3)
2189-2202: MakegradualRolloutnullable in POST upsertPolicy
In the POST/v1/policiesrequest schema you addedgradualRolloutbut omitted"nullable": true. To match the PATCH endpoint and allow callers to omit or explicitly null this field, please add nullability:"gradualRollout": { "$ref": "#/components/schemas/GradualRollout" + ,"nullable": true }
5232-5276: Renamed oldReleaseschema toRelease1—consider a more descriptive name
Release1works technically but is not self-descriptive. Consider renaming to something likeReleaseBaseorReleaseDetailto convey intent. Don’t forget to update all$refusages accordingly.
5907-5928: DefineGradualRolloutschema—add examples and descriptions
The core properties (deployRate,windowSizeMinutes,name,description) are in place. For better client guidance, consider adding"description"and an"example"to each property. For example:"GradualRollout": { "type": "object", "properties": { "deployRate": { "type": "number", + "description": "Proportion of targets to deploy per window (0–1)", + "example": 0.1 }, "windowSizeMinutes": { "type": "number", + "description": "Duration of each rollout window in minutes", + "example": 15 }, "name": { "type": "string", + "description": "Identifier for this rollout rule", + "example": "canary-10-percent" }, "description": { "type": "string", + "description": "Optional human-readable detail about this rollout" } }, "required": ["deployRate","windowSizeMinutes","name"] }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (24)
apps/jobs/src/policy-checker/index.ts(3 hunks)apps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/releases/openapi.ts(1 hunks)apps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/releases/route.ts(1 hunks)apps/webservice/src/app/api/v1/policies/[policyId]/openapi.ts(1 hunks)apps/webservice/src/app/api/v1/policies/openapi.ts(4 hunks)e2e/api/schema.ts(8 hunks)e2e/tests/api/policies/gradual-rollout.spec.ts(1 hunks)e2e/tests/api/policies/gradual-rollout.spec.yaml(1 hunks)openapi.v1.json(8 hunks)packages/api/src/router/policy/evaluate.ts(1 hunks)packages/db/drizzle/0106_wonderful_iron_fist.sql(1 hunks)packages/db/drizzle/meta/_journal.json(1 hunks)packages/db/src/schema/policy-relations.ts(2 hunks)packages/db/src/schema/policy.ts(3 hunks)packages/db/src/schema/rules/gradual-rollout.ts(1 hunks)packages/db/src/schema/rules/index.ts(1 hunks)packages/db/src/schema/rules/rule-relations.ts(2 hunks)packages/rule-engine/src/db/create-policy.ts(2 hunks)packages/rule-engine/src/db/get-applicable-policies.ts(1 hunks)packages/rule-engine/src/db/update-policy.ts(4 hunks)packages/rule-engine/src/manager/version-manager-rules.ts(2 hunks)packages/rule-engine/src/rules/gradual-rollout-rule.ts(1 hunks)packages/rule-engine/src/rules/version-approval-rule.ts(1 hunks)packages/rule-engine/src/types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error...
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
packages/api/src/router/policy/evaluate.tspackages/rule-engine/src/types.tspackages/rule-engine/src/db/get-applicable-policies.tspackages/rule-engine/src/rules/version-approval-rule.tspackages/db/src/schema/policy-relations.tsapps/webservice/src/app/api/v1/policies/[policyId]/openapi.tspackages/db/src/schema/rules/index.tsapps/jobs/src/policy-checker/index.tspackages/db/src/schema/policy.tspackages/db/src/schema/rules/rule-relations.tsapps/webservice/src/app/api/v1/policies/openapi.tspackages/rule-engine/src/db/create-policy.tse2e/tests/api/policies/gradual-rollout.spec.tspackages/rule-engine/src/db/update-policy.tsapps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/releases/route.tspackages/db/src/schema/rules/gradual-rollout.tspackages/rule-engine/src/rules/gradual-rollout-rule.tsapps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/releases/openapi.tse2e/api/schema.tspackages/rule-engine/src/manager/version-manager-rules.ts
🧬 Code Graph Analysis (8)
packages/db/src/schema/policy-relations.ts (1)
packages/db/src/schema/rules/gradual-rollout.ts (1)
policyRuleGradualRollout(7-19)
apps/jobs/src/policy-checker/index.ts (2)
packages/db/src/client.ts (1)
db(15-15)packages/events/src/index.ts (1)
getQueue(28-34)
packages/db/src/schema/policy.ts (1)
packages/db/src/schema/rules/gradual-rollout.ts (1)
createPolicyRuleGradualRollout(21-28)
e2e/tests/api/policies/gradual-rollout.spec.ts (3)
e2e/tests/fixtures.ts (1)
test(11-26)e2e/api/yaml-loader.ts (3)
ImportedEntities(73-123)importEntitiesFromYaml(133-368)cleanupImportedEntities(375-423)packages/db/src/schema/workspace.ts (1)
workspace(18-27)
packages/rule-engine/src/db/update-policy.ts (2)
packages/db/src/common.ts (1)
Tx(22-22)packages/db/src/schema/policy.ts (2)
UpdatePolicy(185-185)policy(43-63)
packages/db/src/schema/rules/gradual-rollout.ts (1)
packages/db/src/schema/policy.ts (1)
policy(43-63)
packages/rule-engine/src/rules/gradual-rollout-rule.ts (2)
packages/rule-engine/src/manager/version-rule-engine.ts (1)
Version(11-17)packages/rule-engine/src/types.ts (2)
FilterRule(31-36)RuleEngineRuleResult(23-26)
packages/rule-engine/src/manager/version-manager-rules.ts (7)
packages/rule-engine/src/types.ts (2)
Policy(66-73)FilterRule(31-36)packages/rule-engine/src/manager/version-rule-engine.ts (1)
Version(11-17)packages/rule-engine/src/rules/version-approval-rule.ts (3)
getAnyApprovalRecords(68-81)getUserApprovalRecords(98-111)getRoleApprovalRecords(83-96)packages/db/src/schema/release.ts (1)
releaseTarget(20-42)packages/db/src/client.ts (1)
db(15-15)packages/db/src/common.ts (1)
takeFirst(9-13)packages/rule-engine/src/rules/gradual-rollout-rule.ts (1)
GradualRolloutRule(14-59)
🪛 Checkov (3.2.334)
openapi.v1.json
[MEDIUM] 350-355: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint
🔇 Additional comments (36)
packages/db/src/schema/rules/index.ts (1)
12-12: LGTM! Clean export addition for the gradual rollout module.This export makes the new gradual rollout functionality accessible to other parts of the application, which is essential for implementing the gradual rollout feature.
packages/api/src/router/policy/evaluate.ts (1)
62-62: LGTM! Correctly includes gradual rollout data in policy evaluation.Adding the
gradualRollout: trueproperty ensures that the gradual rollout relation is included when policies are fetched for evaluation. This is necessary for the policy evaluation logic to access and apply gradual rollout rules.packages/db/drizzle/meta/_journal.json (1)
746-753: LGTM! Properly added migration for the gradual rollout feature.The new migration entry with tag "0106_wonderful_iron_fist" is correctly added to the journal to track the database schema changes for the gradual rollout feature.
packages/rule-engine/src/rules/version-approval-rule.ts (1)
15-15: LGTM! Added necessary timestamp for approval tracking.Adding the
approvedAtfield to the Record type is a good enhancement that enables tracking when approvals occur. This timestamp is essential for implementing time-based gradual rollout logic that depends on approval timing.packages/rule-engine/src/db/get-applicable-policies.ts (1)
43-43: LGTM! Gradual rollout data is now included in policy queries.The addition of
gradualRollout: trueto the query ensures that when fetching applicable policies, the gradual rollout configuration will be included. This change properly integrates the new feature into the existing query structure.packages/rule-engine/src/types.ts (1)
72-72: LGTM! Type definition properly updated for gradual rollout.The
Policytype has been correctly extended to include the optionalgradualRolloutfield of typeschema.PolicyGradualRollout | null. This change ensures type safety when working with policies that may include gradual rollout configurations.packages/db/src/schema/policy-relations.ts (2)
13-13: LGTM! Import for gradual rollout schema added.The import of
policyRuleGradualRolloutfrom the rules index has been properly added to support the new relation.
28-29: LGTM! One-to-one relation established for gradual rollout.The one-to-one relation between policy and gradual rollout has been correctly defined. The blank line after the relation maintains consistent code formatting with other similar relations.
apps/webservice/src/app/api/v1/policies/[policyId]/openapi.ts (1)
80-83: LGTM! OpenAPI schema updated to include gradual rollout.The PATCH endpoint for updating policies has been properly extended to include the
gradualRolloutfield, with the correct reference to theGradualRolloutschema component and appropriate nullable setting.packages/db/src/schema/rules/rule-relations.ts (2)
20-20: Import for gradual rollout added correctly.The import is properly placed among other rule imports, maintaining the organization of the file.
132-140: Relation definition follows established pattern.The
policyRuleGradualRolloutRelationsfollows the same pattern as other rule relations, correctly establishing a one-to-one relationship between the gradual rollout rule and its parent policy.packages/db/drizzle/0106_wonderful_iron_fist.sql (2)
1-9: Table structure is well-designed with appropriate constraints.The
policy_rule_gradual_rollouttable has all necessary fields for implementing gradual rollout functionality:
- Primary key with UUID generation
- One-to-one relationship with policy (via unique constraint)
- Required fields for deployment rate and window size
- Optional description field
This structure supports the gradual rollout feature requirements effectively.
11-11: Foreign key constraint properly configured.The foreign key to the policy table includes cascade delete, which ensures that when a policy is deleted, its associated gradual rollout rule is also removed. This maintains referential integrity in the database.
packages/db/src/schema/policy.ts (3)
40-40: Import added in appropriate location.The import statement for the gradual rollout schema is correctly placed with other rule imports.
151-155: Schema extension properly implements gradual rollout in create policy.The
gradualRolloutfield is correctly added to the create policy schema:
- Properly omits the
policyIdas it will be set during creation- Made optional and nullable since not all policies will use gradual rollout
- Follows the same pattern used for other rule types
This implementation aligns with the established pattern in the codebase.
180-184: Schema extension properly implements gradual rollout in update policy.The
gradualRolloutfield is correctly added to the update policy schema with the same configuration as in the create policy schema, maintaining consistency between the two.e2e/tests/api/policies/gradual-rollout.spec.yaml (2)
1-14: Test system and environment setup is well structured.The test fixture correctly sets up a system and environment with appropriate naming using template variables. The resource selector is properly configured to select resources by identifier, which will work well with the test resources defined below.
15-104: Comprehensive test resource setup for gradual rollout testing.The test fixture creates 10 service resources with:
- Systematic naming using the prefix variable
- Consistent versioning (1.0.0)
- Alternating environments (prod/qa) in metadata
- Enabled status for all services
This provides an excellent foundation for testing various gradual rollout scenarios with different resource subsets.
e2e/tests/api/policies/gradual-rollout.spec.ts (3)
12-12: Good use of constants for readability.The constant
TEN_MINUTESclearly communicates the timeout value and its purpose.
16-33: Test setup follows good practices.The test setup loads entities from a YAML file and cleans them up afterward, ensuring proper test isolation.
41-51: Good use of randomization for test isolation.Using a random deployment slug prevents potential conflicts with other tests.
packages/db/src/schema/rules/gradual-rollout.ts (3)
7-19: Well-structured database schema design.The table schema follows best practices with proper foreign key constraints, uniqueness, and field definitions.
21-28: Appropriate validation constraints.The validation ensures that deployRate and windowSizeMinutes are at least 1, preventing invalid configurations.
30-30: Good type export pattern.Exporting the inferred type promotes type safety throughout the application.
packages/rule-engine/src/db/update-policy.ts (4)
139-154: Well-implemented update function.The updateGradualRollout function follows the same pattern as other update functions in the file, handling all cases (undefined, null, update) correctly.
168-169: Consistent parameter destructuring.The gradualRollout parameter is properly added to the destructured input parameters.
193-194: Parallel update execution.Including updateGradualRollout in the Promise.all call maintains the efficient parallel update pattern.
205-206: Complete query response.Including gradualRollout in the with clause ensures it's included in the query response.
apps/webservice/src/app/api/v1/policies/openapi.ts (4)
70-79: Well-defined OpenAPI schema for GradualRollout.The schema properly defines all required properties and types, matching the database schema constraints.
112-116: Correctly referenced GradualRollout schema.The reference to the GradualRollout schema component is properly set up with nullable option.
128-129: Properly added to required fields.Including gradualRollout in the required fields ensures API contract consistency.
184-187: Request body schema properly updated.The POST endpoint schema is correctly updated to accept the gradualRollout property.
e2e/api/schema.ts (2)
2064-2073: Good simplification ofvaluespayloadSwitching to
VariableValue[]removes an unnecessary nested selector union and keeps the request body leaner. Nice clean-up!
61-120:❓ Verification inconclusive
Release path returns a different “Release” representation than the rest of the API
/v1/deployment-versions/{deploymentVersionId}/releasesadvertises a 200 payload of
components["schemas"]["Release"][], but elsewhere PATCH/POST/v1/releasesstill rely onRelease1.Because
openapi-typescripthad to auto-rename the original schema toRelease1, client code that was written against the earlier contract (Release) will now silently break (type import no longer exists) or, worse, accept the new abbreviated object withoutid,name,status, etc.Action items
- Keep only one canonical “Release” schema—rename the newly introduced lightweight object (e.g.
ReleaseSummary) or revert the original and adapt this path.- Run
rg -t ts '"Release1"'(or a similar query) to update service / UI code that still imports the old name.- Add an integration test that fetches this endpoint and asserts the presence of an
idfield so regressions are caught early.
#!/bin/bash # Search for occurrences of Release1 in the repository rg -n "Release1"
#!/usr/bin/env bash set -e # Search for occurrences of the auto-renamed Release1 schema in TypeScript files rg -n '"Release1"' --glob '*.{ts,tsx}' # Search for occurrences of the original Release schema in TypeScript files rg -n '"Release"' --glob '*.{ts,tsx}' # Search for TypeScript type definitions of Release1 and Release rg -n 'type Release1' --glob '*.{ts,tsx}' rg -n 'type Release =' --glob '*.{ts,tsx}'
#!/usr/bin/env bash set -e # Search for occurrences of the auto-renamed Release1 schema in TypeScript files rg -n '"Release1"' --glob '*.{ts,tsx}' # Search for usage of the original Release schema in the OpenAPI client rg -n '"Release"' e2e/api/schema.ts # Check for any lightweight summary type introduced alongside Release rg -n 'ReleaseSummary' --glob '*.{ts,tsx}'
🏁 Script executed:
#!/usr/bin/env bash set -e # Locate occurrences of the auto-renamed Release1 schema rg -n '"Release1"' --glob '*.{ts,tsx}' # Check for any lightweight summary type alongside Release rg -n 'ReleaseSummary' --glob '*.{ts,tsx}' # Extract the releases endpoint definition for verification sed -n '61,120p' e2e/api/schema.tsLength of output: 49
#!/usr/bin/env bash # Search for any occurrences of the auto-renamed Release1 schema rg -n '"Release1"' --glob '*.{ts,tsx}' || true # Search for any lightweight summary type like ReleaseSummary rg -n 'ReleaseSummary' --glob '*.{ts,tsx}' || true # Show the /v1/releases endpoint definition in schema.ts rg -n '"/v1/releases"' -C 5 e2e/api/schema.ts || true # Re-extract the /v1/deployment-versions/{deploymentVersionId}/releases snippet to reconfirm sed -n '60,140p' e2e/api/schema.ts
Double-check “Release” schema consistency across endpoints
It looks like the
/v1/deployment-versions/{deploymentVersionId}/releasesendpoint is returningcomponents["schemas"]["Release"][], while elsewhere the client may be importing aRelease1type (the auto-renamed original schema). This mismatch can lead to missing fields (id,name,status, etc.) or broken imports.Action items
- Verify whether
openapi-typescripthas generated bothReleaseandRelease1types by searching your codebase forRelease1.- Decide on a single canonical schema:
• Rename the lightweight version to something likeReleaseSummaryand update the path definition accordingly
• Or revert the change and ensure/v1/deployment-versions/.../releasesreturns the fullReleaseobject.- Search for existing imports of
Release1(e.g.rg '"Release1"') and update consumer code to use the chosen schema name.- Add an integration test for this endpoint that asserts the presence of required fields (e.g.
id) to prevent silent regressions.openapi.v1.json (2)
1971-1975: Gradual rollout property added to PATCH policy schema
Good addition of thegradualRolloutfield to the PATCH/v1/policies/{policyId}request schema. This aligns the API contract with the new feature.
5985-5989: EnsuregradualRolloutis returned in policy responses
You’ve addedgradualRolloutto thePolicy1schema and marked it nullable. Please verify that the backend controllers and rule-engine logic populate this field in the responses (and in the E2E tests), otherwise clients will receivenullor miss the property entirely.
| if (gradualRollout != null) | ||
| await tx | ||
| .insert(SCHEMA.policyRuleGradualRollout) | ||
| .values({ ...gradualRollout, policyId }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Upsert & cleanup logic missing – duplicate rows or constraint errors ahead
All other rule inserts use onConflictDoUpdate, making the operation idempotent.
policyRuleGradualRollout is the sole exception, so:
- Re-creating an existing policy will either violate a unique/PK constraint or create duplicates (depending on the table definition).
- Passing
gradualRollout: nullnever removes the stale row, unlike the behaviour implemented inupdate-policy.ts.
Suggested fix:
- if (gradualRollout != null)
- await tx
- .insert(SCHEMA.policyRuleGradualRollout)
- .values({ ...gradualRollout, policyId });
+ if (gradualRollout != null) {
+ await tx
+ .insert(SCHEMA.policyRuleGradualRollout)
+ .values({ ...gradualRollout, policyId })
+ .onConflictDoUpdate({
+ target: [SCHEMA.policyRuleGradualRollout.id],
+ set: buildConflictUpdateColumns(
+ SCHEMA.policyRuleGradualRollout,
+ Object.keys(gradualRollout as Record<string, unknown>),
+ ),
+ });
+ } else {
+ // remove an existing gradual-rollout rule when the caller omits it
+ await tx
+ .delete(SCHEMA.policyRuleGradualRollout)
+ .where(eq(SCHEMA.policyRuleGradualRollout.policyId, policyId));
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (gradualRollout != null) | |
| await tx | |
| .insert(SCHEMA.policyRuleGradualRollout) | |
| .values({ ...gradualRollout, policyId }); | |
| if (gradualRollout != null) { | |
| await tx | |
| .insert(SCHEMA.policyRuleGradualRollout) | |
| .values({ ...gradualRollout, policyId }) | |
| .onConflictDoUpdate({ | |
| target: [SCHEMA.policyRuleGradualRollout.id], | |
| set: buildConflictUpdateColumns( | |
| SCHEMA.policyRuleGradualRollout, | |
| Object.keys(gradualRollout as Record<string, unknown>), | |
| ), | |
| }); | |
| } else { | |
| // remove an existing gradual-rollout rule when the caller omits it | |
| await tx | |
| .delete(SCHEMA.policyRuleGradualRollout) | |
| .where(eq(SCHEMA.policyRuleGradualRollout.policyId, policyId)); | |
| } |
🤖 Prompt for AI Agents
In packages/rule-engine/src/db/create-policy.ts around lines 181 to 185, the
insert into policyRuleGradualRollout lacks upsert logic, risking duplicate rows
or constraint violations and does not handle removal when gradualRollout is
null. Modify the code to use onConflictDoUpdate for idempotent inserts and add
logic to delete the existing row if gradualRollout is null, matching the
behavior in update-policy.ts.
| const ct = alias(schema.computedPolicyTargetReleaseTarget, "ct"); | ||
|
|
||
| const releaseTargets = await db | ||
| .select() | ||
| .from(schema.policy) | ||
| .innerJoin( | ||
| schema.policyTarget, | ||
| eq(schema.policyTarget.policyId, schema.policy.id), | ||
| ) | ||
| .innerJoin(ct, eq(ct.policyTargetId, schema.policyTarget.id)) | ||
| .innerJoin( | ||
| schema.releaseTarget, | ||
| eq(ct.releaseTargetId, schema.releaseTarget.id), | ||
| ) | ||
| .innerJoin( | ||
| schema.policyRuleGradualRollout, | ||
| eq(schema.policyRuleGradualRollout.policyId, schema.policy.id), | ||
| ) | ||
| .limit(PAGE_SIZE) | ||
| .offset(offset) | ||
| .then((rows) => rows.map((row) => row.release_target)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Non-deterministic pagination + join fan-out risks missing or duplicate work
The query paginates with limit/offset but has no orderBy, so PostgreSQL can change row order between calls, causing duplicates or gaps.
Because the joins create a one-to-many relation (multiple policyTargets per policy, etc.) a single release_target can emerge multiple times.
- const releaseTargets = await db
- .select()
+ const releaseTargets = await db
+ .selectDistinctOn([schema.releaseTarget.id])
.from(schema.policy)
...
- .offset(offset)
+ .orderBy(schema.releaseTarget.id)
+ .offset(offset)
.then((rows) => rows.map((row) => row.release_target));This makes pagination deterministic and collapses duplicates.
🤖 Prompt for AI Agents
In apps/jobs/src/policy-checker/index.ts around lines 17 to 37, the query uses
limit and offset for pagination without an orderBy clause, causing
non-deterministic results and potential duplicates due to join fan-out. To fix
this, add a deterministic orderBy clause on a unique or stable column (e.g.,
primary key) to ensure consistent row ordering, and modify the query to collapse
duplicates by using distinct or grouping on release_target to avoid processing
the same release_target multiple times.
| const releases = await db | ||
| .select() | ||
| .from(SCHEMA.release) | ||
| .innerJoin( | ||
| SCHEMA.versionRelease, | ||
| eq(SCHEMA.release.versionReleaseId, SCHEMA.versionRelease.id), | ||
| ) | ||
| .innerJoin( | ||
| variableReleaseSubquery, | ||
| eq( | ||
| SCHEMA.release.variableReleaseId, | ||
| variableReleaseSubquery.variableSetReleaseId, | ||
| ), | ||
| ) | ||
| .innerJoin( | ||
| SCHEMA.releaseTarget, | ||
| eq(SCHEMA.versionRelease.releaseTargetId, SCHEMA.releaseTarget.id), | ||
| ) | ||
| .innerJoin( | ||
| SCHEMA.resource, | ||
| eq(SCHEMA.releaseTarget.resourceId, SCHEMA.resource.id), | ||
| ) | ||
| .innerJoin( | ||
| SCHEMA.environment, | ||
| eq(SCHEMA.releaseTarget.environmentId, SCHEMA.environment.id), | ||
| ) | ||
| .innerJoin( | ||
| SCHEMA.deployment, | ||
| eq(SCHEMA.releaseTarget.deploymentId, SCHEMA.deployment.id), | ||
| ) | ||
| .where(eq(SCHEMA.versionRelease.versionId, deploymentVersionId)) | ||
| .limit(500); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add deterministic ordering & cursor-friendly pagination to the releases query
Today the query limits the result set to 500 rows, but returns them in an undefined order.
This can cause:
- Non-deterministic responses across identical requests (hard to cache & page).
- Starvation of older releases if new ones constantly push them out of the first 500.
Consider ordering by a monotonic column (createdAt, id, etc.) and exposing limit/cursor query-params so consumers can reliably traverse the full history.
- .where(eq(SCHEMA.versionRelease.versionId, deploymentVersionId))
- .limit(500);
+ .where(eq(SCHEMA.versionRelease.versionId, deploymentVersionId))
+ .orderBy(sql`${SCHEMA.release.createdAt} DESC`) // deterministic
+ .limit(limit) // value from searchParams
+ .offset(offset); // cursor/offsetThis small change prevents pagination gaps and improves cacheability.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const releases = await db | |
| .select() | |
| .from(SCHEMA.release) | |
| .innerJoin( | |
| SCHEMA.versionRelease, | |
| eq(SCHEMA.release.versionReleaseId, SCHEMA.versionRelease.id), | |
| ) | |
| .innerJoin( | |
| variableReleaseSubquery, | |
| eq( | |
| SCHEMA.release.variableReleaseId, | |
| variableReleaseSubquery.variableSetReleaseId, | |
| ), | |
| ) | |
| .innerJoin( | |
| SCHEMA.releaseTarget, | |
| eq(SCHEMA.versionRelease.releaseTargetId, SCHEMA.releaseTarget.id), | |
| ) | |
| .innerJoin( | |
| SCHEMA.resource, | |
| eq(SCHEMA.releaseTarget.resourceId, SCHEMA.resource.id), | |
| ) | |
| .innerJoin( | |
| SCHEMA.environment, | |
| eq(SCHEMA.releaseTarget.environmentId, SCHEMA.environment.id), | |
| ) | |
| .innerJoin( | |
| SCHEMA.deployment, | |
| eq(SCHEMA.releaseTarget.deploymentId, SCHEMA.deployment.id), | |
| ) | |
| .where(eq(SCHEMA.versionRelease.versionId, deploymentVersionId)) | |
| .limit(500); | |
| const releases = await db | |
| .select() | |
| .from(SCHEMA.release) | |
| .innerJoin( | |
| SCHEMA.versionRelease, | |
| eq(SCHEMA.release.versionReleaseId, SCHEMA.versionRelease.id), | |
| ) | |
| .innerJoin( | |
| variableReleaseSubquery, | |
| eq( | |
| SCHEMA.release.variableReleaseId, | |
| variableReleaseSubquery.variableSetReleaseId, | |
| ), | |
| ) | |
| .innerJoin( | |
| SCHEMA.releaseTarget, | |
| eq(SCHEMA.versionRelease.releaseTargetId, SCHEMA.releaseTarget.id), | |
| ) | |
| .innerJoin( | |
| SCHEMA.resource, | |
| eq(SCHEMA.releaseTarget.resourceId, SCHEMA.resource.id), | |
| ) | |
| .innerJoin( | |
| SCHEMA.environment, | |
| eq(SCHEMA.releaseTarget.environmentId, SCHEMA.environment.id), | |
| ) | |
| .innerJoin( | |
| SCHEMA.deployment, | |
| eq(SCHEMA.releaseTarget.deploymentId, SCHEMA.deployment.id), | |
| ) | |
| .where(eq(SCHEMA.versionRelease.versionId, deploymentVersionId)) | |
| .orderBy(sql`${SCHEMA.release.createdAt} DESC`) // deterministic order | |
| .limit(limit) // from request params | |
| .offset(offset); // for cursor/offset pagination |
🤖 Prompt for AI Agents
In
apps/webservice/src/app/api/v1/deployment-versions/[deploymentVersionId]/releases/route.ts
between lines 65 and 97, the releases query limits results to 500 without any
ordering, causing non-deterministic and inconsistent pagination. Fix this by
adding an orderBy clause on a monotonic column like createdAt or id to ensure
deterministic ordering. Additionally, modify the API to accept limit and cursor
query parameters to enable cursor-based pagination, allowing consumers to
reliably page through the full release history without gaps or starvation.
| const latestRecord = _.chain([ | ||
| ...anyApprovalRecords, | ||
| ...userApprovalRecords, | ||
| ...roleApprovalRecords, | ||
| ]) | ||
| .filter((r) => r.approvedAt != null) | ||
| .maxBy((r) => r.approvedAt) | ||
| .value(); | ||
|
|
||
| return latestRecord.approvedAt ?? version.createdAt; | ||
| }; |
There was a problem hiding this comment.
Guard against missing approval records to avoid runtime crash
When a policy has approval rules but zero approval records (e.g., a rule
requires 0 approvals or DB inconsistency), latestRecord is undefined,
causing latestRecord.approvedAt to throw.
- const latestRecord = _.chain([
+ const latestRecord: { approvedAt: Date } | undefined = _.chain([
...anyApprovalRecords,
...userApprovalRecords,
...roleApprovalRecords,
])
.filter((r) => r.approvedAt != null)
.maxBy((r) => r.approvedAt)
.value();
-
- return latestRecord.approvedAt ?? version.createdAt;
+ return latestRecord?.approvedAt ?? version.createdAt;This preserves the intended fallback to version.createdAt without risking a
TypeError.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const latestRecord = _.chain([ | |
| ...anyApprovalRecords, | |
| ...userApprovalRecords, | |
| ...roleApprovalRecords, | |
| ]) | |
| .filter((r) => r.approvedAt != null) | |
| .maxBy((r) => r.approvedAt) | |
| .value(); | |
| return latestRecord.approvedAt ?? version.createdAt; | |
| }; | |
| const latestRecord: { approvedAt: Date } | undefined = _.chain([ | |
| ...anyApprovalRecords, | |
| ...userApprovalRecords, | |
| ...roleApprovalRecords, | |
| ]) | |
| .filter((r) => r.approvedAt != null) | |
| .maxBy((r) => r.approvedAt) | |
| .value(); | |
| return latestRecord?.approvedAt ?? version.createdAt; | |
| }; |
🤖 Prompt for AI Agents
In packages/rule-engine/src/manager/version-manager-rules.ts around lines 94 to
104, the code assumes latestRecord is always defined, but if there are no
approval records, latestRecord is undefined causing a runtime error when
accessing latestRecord.approvedAt. Fix this by adding a guard to check if
latestRecord exists before accessing approvedAt, and if not, return
version.createdAt as the fallback to avoid the TypeError.
| GradualRollout: { | ||
| deployRate: number; | ||
| windowSizeMinutes: number; | ||
| name: string; | ||
| description?: string; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
GradualRollout needs domain constraints
The schema currently allows any positive/negative/NaN number:
deployRate– is this 0-100 %? 0-1? integer only?windowSizeMinutes– should be a positive integer.name– max length? pattern?
Consider adding minimum, maximum, and pattern/maxLength to prevent invalid policies from reaching the backend.
🤖 Prompt for AI Agents
In e2e/api/schema.ts around lines 1362 to 1367, the GradualRollout schema
properties lack domain constraints, allowing invalid values. Add validation
rules: set deployRate to a number between 0 and 100 (or clarify if 0-1),
windowSizeMinutes to a positive integer with minimum 1, and add maxLength and
optionally a pattern to name to restrict its format and length. Use JSON schema
keywords like minimum, maximum, maxLength, and pattern to enforce these
constraints.
| Release: { | ||
| resource?: components["schemas"]["Resource"]; | ||
| environment?: components["schemas"]["Environment"]; | ||
| deployment?: components["schemas"]["Deployment"]; | ||
| version?: components["schemas"]["DeploymentVersion"]; | ||
| variables?: { | ||
| [key: string]: unknown; | ||
| }; | ||
| }; | ||
| BaseVariableValue: { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
New Release schema is probably incomplete
A release returned from any endpoint normally needs at least an identifier and timestamps so that UI & automation can reference it.
The newly added schema only contains optional relations + a variables map – no id, name, createdAt, or status.
Unless this is intentional (e.g. “embedded release snippet”), please extend the schema or introduce a derivative type that reflects its truncated shape explicitly.
+id: string; // uuid
+name: string;
+createdAt: string; // date-time
+status?: "ready" | "building" | "failed";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Release: { | |
| resource?: components["schemas"]["Resource"]; | |
| environment?: components["schemas"]["Environment"]; | |
| deployment?: components["schemas"]["Deployment"]; | |
| version?: components["schemas"]["DeploymentVersion"]; | |
| variables?: { | |
| [key: string]: unknown; | |
| }; | |
| }; | |
| BaseVariableValue: { | |
| Release: { | |
| id: string; // uuid | |
| name: string; | |
| createdAt: string; // date-time | |
| status?: "ready" | "building" | "failed"; | |
| resource?: components["schemas"]["Resource"]; | |
| environment?: components["schemas"]["Environment"]; | |
| deployment?: components["schemas"]["Deployment"]; | |
| version?: components["schemas"]["DeploymentVersion"]; | |
| variables?: { | |
| [key: string]: unknown; | |
| }; | |
| }; |
🤖 Prompt for AI Agents
In e2e/api/schema.ts around lines 973 to 982, the Release schema lacks essential
fields like id, name, createdAt, and status, which are typically required for UI
and automation references. To fix this, add these missing fields to the Release
schema with appropriate types, or create a new derived type that explicitly
represents this truncated version if the omission is intentional. Ensure the
schema accurately reflects the data structure returned by the endpoints.
| "Release": { | ||
| "type": "object", | ||
| "properties": { | ||
| "resource": { | ||
| "$ref": "#/components/schemas/Resource" | ||
| }, | ||
| "environment": { | ||
| "$ref": "#/components/schemas/Environment" | ||
| }, | ||
| "deployment": { | ||
| "$ref": "#/components/schemas/Deployment" | ||
| }, | ||
| "version": { | ||
| "$ref": "#/components/schemas/DeploymentVersion" | ||
| }, | ||
| "variables": { | ||
| "type": "object", | ||
| "additionalProperties": true | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
New Release schema introduced—ensure references update correctly
You’ve defined a fresh Release schema for the new endpoint, but the existing endpoints still reference a Release model (formerly the old one). Because the old schema was renamed to Release1, you must update all endpoint schemas (e.g., POST /v1/releases, PATCH /v1/releases/{releaseId}) to reference Release1 instead of Release. Otherwise, clients will receive the wrong payload structure.
🤖 Prompt for AI Agents
In openapi.v1.json around lines 4819 to 4839, you introduced a new Release
schema but renamed the old Release schema to Release1. Update all endpoint
schemas that reference the old Release model, such as POST /v1/releases and
PATCH /v1/releases/{releaseId}, to use Release1 instead of Release to ensure
clients receive the correct payload structure.
| "/v1/deployment-versions/{deploymentVersionId}/releases": { | ||
| "get": { | ||
| "summary": "Get all releases for a deployment version", | ||
| "parameters": [ | ||
| { | ||
| "name": "deploymentVersionId", | ||
| "in": "path", | ||
| "required": true, | ||
| "schema": { | ||
| "type": "string", | ||
| "format": "uuid" | ||
| } | ||
| } | ||
| ], | ||
| "responses": { | ||
| "200": { | ||
| "description": "OK", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "array", | ||
| "items": { | ||
| "$ref": "#/components/schemas/Release" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "404": { | ||
| "description": "Not Found", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "object", | ||
| "properties": { | ||
| "error": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "error" | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "500": { | ||
| "description": "Internal Server Error", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "object", | ||
| "properties": { | ||
| "error": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "error" | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing operationId and tagging for the new endpoint
The GET /v1/deployment-versions/{deploymentVersionId}/releases operation is missing an operationId (and a tags array for grouping). Having a consistent operationId helps code generation and future maintenance.
Apply this diff to introduce an operationId and group under a logical tag:
"/v1/deployment-versions/{deploymentVersionId}/releases": {
"get": {
+ "operationId": "getDeploymentVersionReleases",
+ "tags": ["DeploymentVersions"],
"summary": "Get all releases for a deployment version",
"parameters": [
{
"name": "deploymentVersionId",
"in": "path",
"required": true,
"schema": {
"type": "string",
"format": "uuid"
}
}
],
"responses": {
"200": { ... }
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "/v1/deployment-versions/{deploymentVersionId}/releases": { | |
| "get": { | |
| "summary": "Get all releases for a deployment version", | |
| "parameters": [ | |
| { | |
| "name": "deploymentVersionId", | |
| "in": "path", | |
| "required": true, | |
| "schema": { | |
| "type": "string", | |
| "format": "uuid" | |
| } | |
| } | |
| ], | |
| "responses": { | |
| "200": { | |
| "description": "OK", | |
| "content": { | |
| "application/json": { | |
| "schema": { | |
| "type": "array", | |
| "items": { | |
| "$ref": "#/components/schemas/Release" | |
| } | |
| } | |
| } | |
| } | |
| }, | |
| "404": { | |
| "description": "Not Found", | |
| "content": { | |
| "application/json": { | |
| "schema": { | |
| "type": "object", | |
| "properties": { | |
| "error": { | |
| "type": "string" | |
| } | |
| }, | |
| "required": [ | |
| "error" | |
| ] | |
| } | |
| } | |
| } | |
| }, | |
| "500": { | |
| "description": "Internal Server Error", | |
| "content": { | |
| "application/json": { | |
| "schema": { | |
| "type": "object", | |
| "properties": { | |
| "error": { | |
| "type": "string" | |
| } | |
| }, | |
| "required": [ | |
| "error" | |
| ] | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } | |
| }, | |
| "/v1/deployment-versions/{deploymentVersionId}/releases": { | |
| "get": { | |
| "operationId": "getDeploymentVersionReleases", | |
| "tags": ["DeploymentVersions"], | |
| "summary": "Get all releases for a deployment version", | |
| "parameters": [ | |
| { | |
| "name": "deploymentVersionId", | |
| "in": "path", | |
| "required": true, | |
| "schema": { | |
| "type": "string", | |
| "format": "uuid" | |
| } | |
| } | |
| ], | |
| "responses": { | |
| "200": { | |
| "description": "OK", | |
| "content": { | |
| "application/json": { | |
| "schema": { | |
| "type": "array", | |
| "items": { | |
| "$ref": "#/components/schemas/Release" | |
| } | |
| } | |
| } | |
| } | |
| }, | |
| "404": { | |
| "description": "Not Found", | |
| "content": { | |
| "application/json": { | |
| "schema": { | |
| "type": "object", | |
| "properties": { | |
| "error": { | |
| "type": "string" | |
| } | |
| }, | |
| "required": [ | |
| "error" | |
| ] | |
| } | |
| } | |
| } | |
| }, | |
| "500": { | |
| "description": "Internal Server Error", | |
| "content": { | |
| "application/json": { | |
| "schema": { | |
| "type": "object", | |
| "properties": { | |
| "error": { | |
| "type": "string" | |
| } | |
| }, | |
| "required": [ | |
| "error" | |
| ] | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } | |
| }, |
🧰 Tools
🪛 Checkov (3.2.334)
[MEDIUM] 350-355: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🤖 Prompt for AI Agents
In openapi.v1.json between lines 331 and 397, the GET endpoint for
/v1/deployment-versions/{deploymentVersionId}/releases lacks an operationId and
tags array. Add a unique operationId to identify this operation clearly and
include a tags array with a relevant tag name to group this endpoint logically
for better code generation and maintenance.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/rule-engine/src/manager/version-manager-rules.ts (1)
106-111:⚠️ Potential issueFix potential null reference exception
Similar to a previously identified issue, if there are no approval records or none with an
approvedAtvalue,latestApprovalRecordwill be undefined, causing a runtime error when accessinglatestApprovalRecord.approvedAt.Apply this diff to fix the issue:
- const latestApprovalRecord = _.chain(allApprovalRecords) + const latestApprovalRecord: { approvedAt: Date } | undefined = _.chain(allApprovalRecords) .filter((record) => record.approvedAt != null) .maxBy((record) => record.approvedAt) .value(); - return latestApprovalRecord.approvedAt ?? version.createdAt; + return latestApprovalRecord?.approvedAt ?? version.createdAt;
🧹 Nitpick comments (2)
packages/rule-engine/src/manager/version-manager-rules.ts (2)
139-145: Add error handling for the database queryThe
takeFirstfunction will throw an error if the query returns zero or multiple results. While this shouldn't happen with a properly configured database, it's good practice to add error handling.Consider adding more specific error handling:
return db .select() .from(orderedTargetsSubquery) .where(eq(orderedTargetsSubquery.id, releaseTargetId)) - .then(takeFirst) - .then((r) => r.position); + .then(results => { + try { + return takeFirst(results).position; + } catch (error) { + throw new Error(`Failed to determine position for release target ${releaseTargetId}: ${error.message}`); + } + });
126-128: Consider adding a comment explaining the deterministic ordering logicThe SQL query uses an MD5 hash of concatenated IDs to create a deterministic but pseudo-random ordering of release targets. A comment explaining this design choice would help future maintainers understand the reasoning.
position: sql<number>`ROW_NUMBER() OVER (ORDER BY md5(id || ${version.id}) ASC) - 1`.as( "position", + // Creates a deterministic but pseudo-random ordering of release targets + // based on a hash of their IDs combined with the version ID ),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
packages/rule-engine/src/manager/version-manager-rules.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error...
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
packages/rule-engine/src/manager/version-manager-rules.ts
🧬 Code Graph Analysis (1)
packages/rule-engine/src/manager/version-manager-rules.ts (7)
packages/rule-engine/src/types.ts (2)
Policy(66-73)FilterRule(31-36)packages/rule-engine/src/manager/version-rule-engine.ts (1)
Version(11-17)packages/rule-engine/src/rules/version-approval-rule.ts (3)
getAnyApprovalRecords(68-81)getUserApprovalRecords(98-111)getRoleApprovalRecords(83-96)packages/db/src/schema/release.ts (1)
releaseTarget(20-42)packages/db/src/client.ts (1)
db(15-15)packages/db/src/common.ts (1)
takeFirst(9-13)packages/rule-engine/src/rules/gradual-rollout-rule.ts (1)
GradualRolloutRule(14-67)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build (linux/amd64)
- GitHub Check: Typecheck
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
🔇 Additional comments (3)
packages/rule-engine/src/manager/version-manager-rules.ts (3)
83-97: Good implementation of approval rules validationThe approach to verifying that a version passes all approval rules is well-implemented. Using
reducewith an async function to short-circuit at the first failing rule is efficient and clean.
161-162: Well-integrated gradual rollout ruleThe gradual rollout rule is correctly added to the array of rules and properly filtered to remove any null entries. This maintains backward compatibility with policies that don't have gradual rollout configured.
147-152: Clean implementation of GradualRolloutRule instantiationThe GradualRolloutRule is instantiated with the policy's gradual rollout configuration and the necessary helper functions, following the dependency injection pattern. This separation of concerns enhances testability and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/rule-engine/src/manager/version-manager-rules.ts (1)
109-115:⚠️ Potential issueGuard against missing approval records to avoid runtime crash
When a policy has approval rules but zero approval records (e.g., a rule requires 0 approvals or DB inconsistency),
latestApprovalRecordcould beundefined, causinglatestApprovalRecord.approvedAtto throw.const latestApprovalRecord = _.chain(allApprovalRecords) .filter((record) => record.approvedAt != null) .maxBy((record) => record.approvedAt) .value(); -return latestApprovalRecord.approvedAt ?? version.createdAt; +return latestApprovalRecord?.approvedAt ?? version.createdAt;
🧹 Nitpick comments (5)
packages/rule-engine/src/rules/environment-version-rollout-rule.ts (3)
8-22: Consider adding input validation for deployment offset functionsThe linear and exponential deployment offset functions don't validate their inputs. Negative values for
positionGrowthFactorortimeScaleMinutescould lead to unexpected behavior.export const linearDeploymentOffset = ( positionGrowthFactor: number, timeScaleMinutes: number, ): GetDeploymentOffsetMinutes => - (x: number) => - timeScaleMinutes * (x / positionGrowthFactor); + (x: number) => { + if (positionGrowthFactor <= 0 || timeScaleMinutes < 0) { + throw new Error("Invalid parameters: positionGrowthFactor must be positive and timeScaleMinutes must be non-negative"); + } + return timeScaleMinutes * (x / positionGrowthFactor); + }; export const exponentialDeploymentOffset = ( positionGrowthFactor: number, timeScaleMinutes: number, ): GetDeploymentOffsetMinutes => - (x: number) => - timeScaleMinutes * (Math.exp(x / positionGrowthFactor) - 1); + (x: number) => { + if (positionGrowthFactor <= 0 || timeScaleMinutes < 0) { + throw new Error("Invalid parameters: positionGrowthFactor must be positive and timeScaleMinutes must be non-negative"); + } + return timeScaleMinutes * (Math.exp(x / positionGrowthFactor) - 1); + };
46-73: The filter method lacks error handling for async operationsThe filter method contains several await operations but doesn't handle potential exceptions from the async calls to
getRolloutStartTime,getDeploymentTime, orgetReleaseTargetPosition. Consider adding try/catch blocks.async filter(candidates: Version[]): Promise<RuleEngineRuleResult<Version>> { const now = this.getCurrentTime(); const rejectionReasons = new Map<string, string>(); const skip = this.options.skipReason ?? "Version not eligible for deployment in the current time window"; for (const candidate of candidates) { + try { const startTime = await this.options.getRolloutStartTime(candidate); if (startTime == null) { rejectionReasons.set( candidate.id, "Rollout has not started for this version", ); continue; } const deploymentTime = await this.getDeploymentTime(candidate, startTime); const isEligible = isAfter(now, deploymentTime) || isEqual(now, deploymentTime); if (!isEligible) rejectionReasons.set(candidate.id, skip); + } catch (error) { + rejectionReasons.set( + candidate.id, + `Error determining eligibility: ${error instanceof Error ? error.message : String(error)}` + ); + } } return { allowedCandidates: candidates.filter((c) => !rejectionReasons.has(c.id)), rejectionReasons, }; }
24-74: Add JSDoc comments to improve code documentationThe class and its methods lack documentation comments. Adding JSDoc comments would improve readability and make the purpose of each component clearer.
+/** + * Options for configuring the environment version rollout rule. + */ type EnvironmentVersionRolloutRuleOptions = { getRolloutStartTime: (version: Version) => Date | Promise<Date | null> | null; getReleaseTargetPosition: (version: Version) => number | Promise<number>; getDeploymentOffsetMinutes: GetDeploymentOffsetMinutes; skipReason?: string; }; +/** + * Rule that filters versions based on their calculated deployment time. + * Versions are allowed if the current time is after or equal to their deployment time. + */ export class EnvironmentVersionRolloutRule implements FilterRule<Version> { public readonly name = "EnvironmentVersionRolloutRule"; constructor(private readonly options: EnvironmentVersionRolloutRuleOptions) {} + /** + * Returns the current time. Protected for easier testing. + */ protected getCurrentTime() { return new Date(); } + /** + * Calculates the deployment time for a version based on its start time and target position. + * @param version The version to calculate deployment time for + * @param startTime The rollout start time + * @returns The calculated deployment time + */ async getDeploymentTime(version: Version, startTime: Date) { const targetPosition = await this.options.getReleaseTargetPosition(version); const minutes = this.options.getDeploymentOffsetMinutes(targetPosition); return addMinutes(startOfMinute(startTime), minutes); }packages/rule-engine/src/manager/version-manager-rules.ts (2)
117-148: Improve error handling in database operationsThe database query in
getReleaseTargetPositionhas minimal error handling. Consider adding try/catch blocks to provide more context in case of database errors.const getReleaseTargetPosition = async (version: Version) => { + try { const releaseTarget = await db.query.releaseTarget.findFirst({ where: eq(schema.releaseTarget.id, releaseTargetId), }); if (releaseTarget == null) throw new Error(`Release target ${releaseTargetId} not found`); const orderedTargetsSubquery = db .select({ id: schema.releaseTarget.id, position: sql<number>`ROW_NUMBER() OVER (ORDER BY md5(id || ${version.id}) ASC) - 1`.as( "position", ), }) .from(schema.releaseTarget) .where( and( eq(schema.releaseTarget.environmentId, releaseTarget.environmentId), eq(schema.releaseTarget.deploymentId, releaseTarget.deploymentId), ), ) .as("ordered_targets"); return db .select() .from(orderedTargetsSubquery) .where(eq(orderedTargetsSubquery.id, releaseTargetId)) .then(takeFirst) .then((r) => r.position); + } catch (error) { + throw new Error(`Failed to determine release target position: ${error instanceof Error ? error.message : String(error)}`); + } };
86-115: Consider refactoring the approval check logicThe logic to check if all approval rules pass (lines 91-98) can be simplified using Promise.all or a more straightforward approach.
-// Check if version passes all approval rules -const allApprovalsPassed = await versionApprovalRules.reduce( - async (passedSoFar, rule) => { - if (!(await passedSoFar)) return false; - const { allowedCandidates } = await rule.filter([version]); - return allowedCandidates.length > 0; - }, - Promise.resolve(true), -); +// Check if version passes all approval rules +const approvalResults = await Promise.all( + versionApprovalRules.map(rule => rule.filter([version])) +); +const allApprovalsPassed = approvalResults.every( + result => result.allowedCandidates.length > 0 +);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/rule-engine/src/manager/version-manager-rules.ts(2 hunks)packages/rule-engine/src/rules/environment-version-rollout-rule.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error...
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
packages/rule-engine/src/rules/environment-version-rollout-rule.tspackages/rule-engine/src/manager/version-manager-rules.ts
| const getDeploymentOffsetMinutes = linearDeploymentOffset( | ||
| policy.environmentVersionRollou.positionGrowthFactor, | ||
| policy.environmentVersionRollou.timeScaleMinutes, | ||
| ); |
There was a problem hiding this comment.
Fix typo in property name
There's a typo in the property name environmentVersionRollou which should be environmentVersionRollout.
const getDeploymentOffsetMinutes = linearDeploymentOffset(
- policy.environmentVersionRollou.positionGrowthFactor,
- policy.environmentVersionRollou.timeScaleMinutes,
+ policy.environmentVersionRollout.positionGrowthFactor,
+ policy.environmentVersionRollout.timeScaleMinutes,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const getDeploymentOffsetMinutes = linearDeploymentOffset( | |
| policy.environmentVersionRollou.positionGrowthFactor, | |
| policy.environmentVersionRollou.timeScaleMinutes, | |
| ); | |
| const getDeploymentOffsetMinutes = linearDeploymentOffset( | |
| policy.environmentVersionRollout.positionGrowthFactor, | |
| policy.environmentVersionRollout.timeScaleMinutes, | |
| ); |
🤖 Prompt for AI Agents
In packages/rule-engine/src/manager/version-manager-rules.ts around lines 150 to
153, fix the typo in the property name by changing all instances of
`environmentVersionRollou` to `environmentVersionRollout` to correctly reference
the intended property.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/rule-engine/src/manager/version-manager-rules.ts (1)
114-114:⚠️ Potential issueGuard against missing approval records to avoid runtime crash
When there are no approval records found (either empty list or all records have null
approvedAt),latestApprovalRecordwill beundefined, causing a TypeError when accessinglatestApprovalRecord.approvedAt.Apply this fix to prevent runtime errors:
- return latestApprovalRecord.approvedAt ?? version.createdAt; + return latestApprovalRecord?.approvedAt ?? version.createdAt;
🧹 Nitpick comments (3)
packages/rule-engine/src/manager/version-manager-rules.ts (3)
91-98: Consider simplifying approval validation logicThe current approval validation uses a reduce with an async callback, which can be harder to reason about. Consider using a more straightforward approach with a for loop.
- const allApprovalsPassed = await versionApprovalRules.reduce( - async (passedSoFar, rule) => { - if (!(await passedSoFar)) return false; - const { allowedCandidates } = await rule.filter([version]); - return allowedCandidates.length > 0; - }, - Promise.resolve(true), - ); + let allApprovalsPassed = true; + for (const rule of versionApprovalRules) { + const { allowedCandidates } = await rule.filter([version]); + if (allowedCandidates.length === 0) { + allApprovalsPassed = false; + break; + } + }
103-107: Optimize approval records fetchingThe current implementation fetches approval records from three separate sources, which could be inefficient. Consider combining these queries if possible.
- const allApprovalRecords = [ - ...(await getAnyApprovalRecords([version.id])), - ...(await getUserApprovalRecords([version.id])), - ...(await getRoleApprovalRecords([version.id])), - ]; + // Fetch all approval records in parallel for better performance + const [anyApprovals, userApprovals, roleApprovals] = await Promise.all([ + getAnyApprovalRecords([version.id]), + getUserApprovalRecords([version.id]), + getRoleApprovalRecords([version.id]), + ]); + + const allApprovalRecords = [ + ...anyApprovals, + ...userApprovals, + ...roleApprovals, + ];
109-113: Add type annotation for latestApprovalRecordAdding a type annotation would make it clearer that
latestApprovalRecordmight be undefined and help prevent potential bugs.- const latestApprovalRecord = _.chain(allApprovalRecords) + const latestApprovalRecord: { approvedAt: Date | null } | undefined = _.chain(allApprovalRecords) .filter((record) => record.approvedAt != null) .maxBy((record) => record.approvedAt) .value();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/rule-engine/src/manager/version-manager-rules.ts(2 hunks)packages/rule-engine/src/rules/environment-version-rollout-rule.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rule-engine/src/rules/environment-version-rollout-rule.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error...
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
packages/rule-engine/src/manager/version-manager-rules.ts
🧬 Code Graph Analysis (1)
packages/rule-engine/src/manager/version-manager-rules.ts (7)
packages/rule-engine/src/types.ts (2)
Policy(66-73)FilterRule(31-36)packages/rule-engine/src/manager/version-rule-engine.ts (1)
Version(11-17)packages/rule-engine/src/rules/version-approval-rule.ts (3)
getAnyApprovalRecords(68-81)getUserApprovalRecords(98-111)getRoleApprovalRecords(83-96)packages/db/src/schema/release.ts (1)
releaseTarget(20-42)packages/db/src/client.ts (1)
db(15-15)packages/db/src/common.ts (1)
takeFirst(9-13)packages/rule-engine/src/rules/environment-version-rollout-rule.ts (2)
linearDeploymentOffset(8-14)EnvironmentVersionRolloutRule(31-74)
🔇 Additional comments (3)
packages/rule-engine/src/manager/version-manager-rules.ts (3)
169-171: LGTM: Properly integrates the gradual rollout ruleThe gradual rollout rule is correctly added to the list of rules and filtered to remove any nulls using
isPresent. This is a clean integration with the existing rule system.
86-115: Comprehensive rollout start time determinationThe implementation to determine the rollout start time is well-structured:
- First checks if all approval rules are satisfied
- If so, finds the latest approval timestamp
- Falls back to version creation time if no approvals exist
This ensures gradual rollouts begin at an appropriate time based on the approval workflow.
143-147: 🛠️ Refactor suggestionAdd error handling for the takeFirst function
The
takeFirstfunction will throw an error if the query doesn't return exactly one result, according to its implementation inpackages/db/src/common.ts. Consider adding error handling to gracefully handle this case.- return db - .select() - .from(orderedTargetsSubquery) - .where(eq(orderedTargetsSubquery.id, releaseTargetId)) - .then(takeFirst) - .then((r) => r.position); + return db + .select() + .from(orderedTargetsSubquery) + .where(eq(orderedTargetsSubquery.id, releaseTargetId)) + .then((results) => { + if (results.length !== 1) { + throw new Error(`Expected exactly one result for release target position, got ${results.length}`); + } + return results[0].position; + });⛔ Skipped due to learnings
Learnt from: adityachoudhari26 PR: ctrlplanedev/ctrlplane#395 File: packages/api/src/router/environment-page/resources/router.ts:40-45 Timestamp: 2025-03-24T18:46:38.894Z Learning: The `takeFirst` utility function in the codebase (from `@ctrlplane/db`) throws an Error with message "Found non unique or inexistent value" if the result array doesn't contain exactly one element, making additional null/undefined checks unnecessary after its use.
| sql<number>`ROW_NUMBER() OVER (ORDER BY md5(id || ${version.id}) ASC) - 1`.as( | ||
| "position", | ||
| ), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Review the deterministic ordering mechanism
The SQL query is using an MD5 hash of the release target ID and version ID to create a deterministic random ordering. Make sure this approach meets your requirements for consistent but unpredictable ordering.
This deterministic ordering is important for gradual rollouts to ensure:
- The same targets get the same position for a given version
- Different versions produce different orderings of targets
🏁 Script executed:
#!/bin/bash
# Check if there are any tests for this deterministic ordering
rg -A 5 -B 5 "md5\(id \|\| " --type tsLength of output: 971
Add tests for the MD5-based deterministic ordering
I didn’t find any existing tests covering the ROW_NUMBER() OVER (ORDER BY md5(id || ${version.id}) ASC) logic. To ensure gradual rollouts remain consistent and unpredictable as intended, please add unit tests in packages/rule-engine/src/manager/version-manager-rules.ts that verify:
- For a fixed set of release-target IDs and a given
version.id, the computedpositionarray is identical on repeated runs. - Using the same target IDs with a different
version.idproduces a different ordering. - (Optional) Edge cases, e.g., single target or empty set, behave as expected.
Targets for new tests:
- File: packages/rule-engine/src/manager/version-manager-rules.ts
- SQL snippet at lines 129–131:
sql<number>`ROW_NUMBER() OVER (ORDER BY md5(id || ${version.id}) ASC) - 1`.as( "position", ),
🤖 Prompt for AI Agents
In packages/rule-engine/src/manager/version-manager-rules.ts around lines 129 to
131, add unit tests to verify the deterministic ordering logic using the MD5
hash of target IDs concatenated with version.id. Write tests that confirm the
position array remains consistent for the same set of target IDs and a fixed
version.id across multiple runs, and that changing the version.id results in a
different ordering. Also include tests for edge cases like a single target or an
empty target set to ensure the ordering behaves as expected in those scenarios.
| const getDeploymentOffsetMinutes = linearDeploymentOffset( | ||
| policy.environmentVersionRollout.positionGrowthFactor, | ||
| policy.environmentVersionRollout.timeScaleInterval, | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify property path policy.environmentVersionRollout
The property environmentVersionRollout is accessed but doesn't appear in the Policy type definition from the context provided. Make sure this property exists and is properly defined in the schema.
🏁 Script executed:
#!/bin/bash
# Check if environmentVersionRollout property exists in the Policy type or schema
rg -A 3 -B 3 "environmentVersionRollout" --type tsLength of output: 747
I’ll verify the Policy definition and its use in version-manager-rules.ts to confirm whether environmentVersionRollout is declared.
🏁 Script executed:
#!/bin/bash
# 1. Locate the Policy interface or type across the repo
rg -n "interface Policy" --type ts
rg -n "type Policy" --type ts
# 2. Show how `policy` is typed where used in version-manager-rules.ts
rg -n "policy\s*:" -C3 packages/rule-engine/src/manager/version-manager-rules.tsLength of output: 3952
I’ll inspect the Policy type in packages/rule-engine/src/types.ts to confirm whether environmentVersionRollout is declared.
🏁 Script executed:
#!/bin/bash
# 1. Check for environmentVersionRollout in the Policy extension
rg -n "environmentVersionRollout" -C5 packages/rule-engine/src/types.ts
# 2. Show the definition of Policy in this file
sed -n '1,120p' packages/rule-engine/src/types.tsLength of output: 2573
Missing environmentVersionRollout on Policy
The code in packages/rule-engine/src/manager/version-manager-rules.ts is calling
policy.environmentVersionRollout.positionGrowthFactorbut your Policy extension (in packages/rule-engine/src/types.ts) only includes:
denyWindowsdeploymentVersionSelectorversionAnyApprovalsversionUserApprovalsversionRoleApprovalsgradualRollout
There is no environmentVersionRollout property declared. Please address this by:
- Adding the new field
Inpackages/rule-engine/src/types.ts:export type Policy = schema.Policy & { denyWindows: schema.PolicyRuleDenyWindow[]; deploymentVersionSelector: schema.PolicyDeploymentVersionSelector | null; versionAnyApprovals: schema.PolicyRuleAnyApproval | null; versionUserApprovals: schema.PolicyRuleUserApproval[]; versionRoleApprovals: schema.PolicyRuleRoleApproval[]; gradualRollout: schema.PolicyGradualRollout | null;
- environmentVersionRollout: schema.PolicyEnvironmentVersionRollout; // ← add this
};
- **Verifying the DB schema**
Make sure `@ctrlplane/db/schema.Policy` (in `packages/db/src/schema/policy.ts`) actually defines `environmentVersionRollout` (or rename your code to use an existing field, e.g. `gradualRollout`).
- **Running validators/schemas** to ensure TypeScript picks up the new property.
…so that `policy.environmentVersionRollout` compiles and reflects the intended rollout settings.
<details>
<summary>🤖 Prompt for AI Agents</summary>
In packages/rule-engine/src/manager/version-manager-rules.ts around lines 150 to
153, the code accesses policy.environmentVersionRollout, but this property is
missing from the Policy type definition in packages/rule-engine/src/types.ts. To
fix this, add the environmentVersionRollout field with the correct type to the
Policy interface in packages/rule-engine/src/types.ts. Also verify that the
database schema in packages/db/src/schema/policy.ts includes this field or
rename the code to use an existing property like gradualRollout. Finally, run
any schema validators or TypeScript checks to ensure the new property is
recognized and the code compiles correctly.
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
Summary by CodeRabbit