fix: stale version does not evict newer version #628

adityachoudhari26 · 2025-07-25T01:59:28Z

Summary by CodeRabbit

New Features
- Added support for defining agents in the release system configuration.
Bug Fixes
- Improved release creation logic to prevent releases for versions older than the currently deployed version.
- Enhanced validation to ensure only newer versions are considered for deployment.
Tests
- Added a test case verifying that releases are not created for older versions than those already deployed.

coderabbitai · 2025-07-25T01:59:36Z

Walkthrough

The changes introduce a new agents section to a YAML release configuration, add a test to ensure releases are not created for versions older than the current deployment, and update the version manager logic to only consider versions newer than the latest deployed (including in-progress jobs) when prevalidating versions for release.

Changes

File(s)	Change Summary
e2e/tests/api/releases/release.spec.yaml	Added a new top-level `agents` section with a templated agent definition.
e2e/tests/api/releases/version-release.spec.ts	Added a fixture and a new test verifying that releases are not created for versions older than the current deployed.
packages/rule-engine/src/manager/version-manager.ts	Updated logic to consider in-progress jobs as deployed and filter prevalidation to only versions newer than current.

Sequence Diagram(s)

sequenceDiagram
    participant Test as E2E Test
    participant API as API Server
    participant DB as Database
    participant JobQ as Job Queue

    Test->>API: Create deployment with agent
    Test->>API: Create version (newer)
    API->>DB: Store deployment & version
    Test->>API: Create release for newer version
    API->>DB: Validate version is newer than deployed
    API->>JobQ: Enqueue job for release
    Test->>API: Create version (older)
    Test->>API: Attempt release for older version
    API->>DB: Validate version (filtered out as older)
    API-->>Test: No release created for older version

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~15 minutes

Possibly related PRs

fix: Deploy release if it is after latest non-pending #200: Modifies version filtering and validation logic to enforce deployment only of releases newer than the last active one, directly relating to the updated prevalidation and filtering logic in this PR.

Suggested reviewers

jsbroks

Poem

A rabbit hops through YAML fields,
With agents new, the config yields.
Versions checked, both old and new—
Only the freshest will make it through!
With logic crisp and tests in place,
This code now leaps with agile grace.
🐇✨

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d45f3bf and c7e537b.

📒 Files selected for processing (3)

e2e/tests/api/releases/release.spec.yaml (1 hunks)
e2e/tests/api/releases/version-release.spec.ts (2 hunks)
packages/rule-engine/src/manager/version-manager.ts (4 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (CLAUDE.md)

**/*.{ts,tsx}: Use TypeScript with explicit types (prefer interfaces for public APIs)
Import styles: Use named imports, group imports by source (std lib > external > internal)
Consistent type imports: import type { Type } from "module"
Prefer async/await over raw promises
Handle errors explicitly (use try/catch and typed error responses)

Files:

packages/rule-engine/src/manager/version-manager.ts
e2e/tests/api/releases/version-release.spec.ts

⚙️ CodeRabbit Configuration File

**/*.{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.

Files:

packages/rule-engine/src/manager/version-manager.ts
e2e/tests/api/releases/version-release.spec.ts

**/*.{js,jsx,ts,tsx,json,md,yml,yaml}

📄 CodeRabbit Inference Engine (CLAUDE.md)

Formatting: Prettier is used with @ctrlplane/prettier-config

Files:

packages/rule-engine/src/manager/version-manager.ts
e2e/tests/api/releases/release.spec.yaml
e2e/tests/api/releases/version-release.spec.ts

🧠 Learnings (3)

📓 Common learnings

Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#601
File: e2e/tests/api/policies/retry-policy.spec.ts:23-24
Timestamp: 2025-06-24T23:52:50.732Z
Learning: The user adityachoudhari26 prefers not to add null safety checks or defensive programming in test code, particularly in e2e tests, as they prioritize simplicity and focus on the main functionality being tested rather than comprehensive error handling within the test itself.

Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#601
File: e2e/tests/api/policies/retry-policy.spec.ts:117-130
Timestamp: 2025-06-24T23:53:25.398Z
Learning: User adityachoudhari26 prefers to keep non-null assertions in e2e test code without extensive null safety checks, reasoning that test failures serve the same purpose of catching issues and the extra validation doesn't add much value in test contexts.

packages/rule-engine/src/manager/version-manager.ts (10)

Learnt from: adityachoudhari26
PR: #604
File: packages/rule-engine/src/manager/version-manager.ts:124-139
Timestamp: 2025-06-30T21:19:43.866Z
Learning: In packages/rule-engine/src/manager/version-manager.ts, the findDesiredVersion method should use takeFirst (not takeFirstOrNull) because if a desiredVersionId is set but the query fails to find exactly one matching version, it indicates a data integrity or configuration issue that should fail loudly rather than be handled silently.

Learnt from: adityachoudhari26
PR: #181
File: packages/api/src/router/deployment.ts:116-131
Timestamp: 2024-10-29T02:04:50.312Z
Learning: In packages/api/src/router/deployment.ts, the list.byDeploymentId procedure requires multiple database queries due to limitations of the releaseMatchesCondition function.

Learnt from: adityachoudhari26
PR: #237
File: packages/api/src/router/job.ts:362-365
Timestamp: 2024-11-27T23:18:42.055Z
Learning: In the file packages/api/src/router/job.ts, the function releaseMatchesCondition returns undefined if the filter parameter is null. This behavior ensures that when constructing the query with and(...), the condition is omitted, allowing the query to function correctly even if there is no release channel associated with the environment.

Learnt from: adityachoudhari26
PR: #579
File: packages/rule-engine/src/rules/concurrency-rule.ts:8-11
Timestamp: 2025-06-01T19:10:47.122Z
Learning: In packages/rule-engine/src/rules/concurrency-rule.ts, the ConcurrencyRule should remain simple without additional validation since database and Zod schemas already handle concurrency validation. The user prefers this rule to be "dumb" and just perform the comparison check rather than duplicating validation logic.

Learnt from: adityachoudhari26
PR: #627
File: apps/event-worker/src/workers/job-update/trigger-dependendent-targets.ts:0-0
Timestamp: 2025-07-23T22:42:20.903Z
Learning: In apps/event-worker/src/workers/job-update/trigger-dependendent-targets.ts, when implementing error handling for the triggerDependentTargets function, only a try-catch wrapper is needed around the entire function body. Additional null checks after takeFirst() calls are unnecessary since takeFirst throws an error if no record is found.

Learnt from: adityachoudhari26
PR: #237
File: apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/DeploymentResourceDrawer.tsx:43-50
Timestamp: 2024-11-27T23:16:35.580Z
Learning: In DeploymentResourceDrawer.tsx, the isOpen variable already checks whether deploymentId, environmentId, and resourceId are non-null, so additional null checks in query enabled conditions are not necessary.

Learnt from: adityachoudhari26
PR: #593
File: packages/rule-engine/src/manager/version-manager-rules/version-approval.ts:55-70
Timestamp: 2025-06-18T21:46:51.459Z
Learning: The takeFirst utility function from the database layer throws an error if no record is found, rather than returning undefined. This means functions using takeFirst already have error handling built-in and don't require additional null checks.

Learnt from: adityachoudhari26
PR: #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.

Learnt from: adityachoudhari26
PR: #500
File: packages/db/src/schema/job.ts:117-120
Timestamp: 2025-04-21T18:34:54.764Z
Learning: In the system's job schema, the relationship between job and releaseJob is a true one-to-one relationship - a release job should only ever point to one job and vice versa. The implementation uses one(releaseJob, ...) in the jobRelations to reflect this business rule.

Learnt from: adityachoudhari26
PR: #515
File: apps/event-worker/src/workers/compute-systems-release-targets.ts:86-110
Timestamp: 2025-04-28T18:38:21.163Z
Learning: In SQL queries that use inArray() with arrays like deploymentIds or environmentIds, if these arrays are empty, it will generate an invalid IN () clause that PostgreSQL rejects. Adding condition checks (e.g., if (array.length > 0)) before executing such queries prevents SQL syntax errors.

e2e/tests/api/releases/version-release.spec.ts (7)

Learnt from: adityachoudhari26
PR: #181
File: packages/api/src/router/deployment.ts:116-131
Timestamp: 2024-10-29T02:04:50.312Z
Learning: In packages/api/src/router/deployment.ts, the list.byDeploymentId procedure requires multiple database queries due to limitations of the releaseMatchesCondition function.

Learnt from: adityachoudhari26
PR: #187
File: apps/jobs/src/ephemeral-env-checker/index.ts:57-0
Timestamp: 2024-10-30T23:10:58.869Z
Learning: In the codebase, deployments are decoupled from environments. When deleting environments (e.g., in apps/jobs/src/ephemeral-env-checker/index.ts), associated deployments should not be deleted.

Learnt from: adityachoudhari26
PR: #627
File: apps/event-worker/src/workers/job-update/trigger-dependendent-targets.ts:0-0
Timestamp: 2025-07-23T22:42:20.903Z
Learning: In apps/event-worker/src/workers/job-update/trigger-dependendent-targets.ts, when implementing error handling for the triggerDependentTargets function, only a try-catch wrapper is needed around the entire function body. Additional null checks after takeFirst() calls are unnecessary since takeFirst throws an error if no record is found.

Learnt from: adityachoudhari26
PR: #601
File: e2e/tests/api/policies/retry-policy.spec.ts:96-99
Timestamp: 2025-06-24T23:53:36.327Z
Learning: In e2e test files, prefer explicit null safety checks and validation over non-null assertions (!). When validating API responses in tests, use type guards and throw descriptive errors rather than assuming values are always present.

Learnt from: adityachoudhari26
PR: #237
File: packages/api/src/router/job.ts:382-390
Timestamp: 2024-11-27T23:18:59.456Z
Learning: In the codebase, releases are not scoped to a specific resource.

Learnt from: adityachoudhari26
PR: #517
File: e2e/tests/api/deployment-variable.spec.ts:70-76
Timestamp: 2025-04-28T21:59:04.723Z
Learning: In the ctrlplane e2e tests, tests can be isolated between runs using prefixed systems with random IDs in YAML templates, but tests within the same file may still need to handle isolation between test cases if they operate on the same resources.

Learnt from: adityachoudhari26
PR: #237
File: packages/api/src/router/job.ts:362-365
Timestamp: 2024-11-27T23:18:42.055Z
Learning: In the file packages/api/src/router/job.ts, the function releaseMatchesCondition returns undefined if the filter parameter is null. This behavior ensures that when constructing the query with and(...), the condition is omitted, allowing the query to function correctly even if there is no release channel associated with the environment.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

GitHub Check: build (linux/amd64)
GitHub Check: build (linux/amd64)
GitHub Check: Typecheck
GitHub Check: Lint

🔇 Additional comments (7)

e2e/tests/api/releases/release.spec.yaml (1)

26-28: LGTM! Agent configuration added correctly.

The new agents section follows the established templating pattern and provides the necessary fixture for the E2E test scenarios.

e2e/tests/api/releases/version-release.spec.ts (2)

18-18: LGTM! Agent fixtures setup added correctly.

The addition of upsertAgentFixtures() is necessary to support the new test case that requires job agents for deployment processing.

107-239: Comprehensive test case validates the stale version fix.

The test correctly verifies that older versions don't create releases when newer versions are already deployed. The logic is sound:

Creates deployment with job agent

Creates newer version and verifies release creation

Completes the job successfully

Creates older version and confirms no release is created

The use of explicit createdAt timestamps ensures deterministic version ordering, and the test properly handles async operations with appropriate wait times.

packages/rule-engine/src/manager/version-manager.ts (4)

9-9: LGTM! Import added for new OR logic.

The or import is correctly added to support the expanded job status condition.

106-109: Correct expansion of job status consideration.

Including InProgress jobs alongside Successful jobs when determining the latest deployed version is the right approach. This prevents older versions from being deployed while a deployment is still in progress.

186-193: Core fix correctly filters out stale versions.

The filtering logic properly addresses the PR objective by ensuring only versions created after the latest deployed version are considered for release. The fallback to epoch date (new Date(0)) when no latest deployed version exists is appropriate and allows the first deployment to proceed.

206-210: Variable updates correctly use filtered versions.

The logic correctly uses versionsNewerThanLatest instead of the original versions array, ensuring consistency with the filtering applied above.

✨ Finishing Touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch stale-version-evicts-new-bug

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.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai generate unit tests to generate unit tests for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

fix: stale version does not evict newer version

c7e537b

adityachoudhari26 merged commit 00aa5da into main Jul 25, 2025
7 checks passed

adityachoudhari26 deleted the stale-version-evicts-new-bug branch July 25, 2025 08:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: stale version does not evict newer version #628

fix: stale version does not evict newer version #628

Uh oh!

adityachoudhari26 commented Jul 25, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jul 25, 2025 •

edited

Loading

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: stale version does not evict newer version #628

fix: stale version does not evict newer version #628

Uh oh!

Conversation

adityachoudhari26 commented Jul 25, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

adityachoudhari26 commented Jul 25, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jul 25, 2025 •

edited

Loading