fix: ability to exclude rule ids from eligible versions endpoint#1148
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
📝 WalkthroughWalkthroughThis PR extends the eligible-versions endpoint with an optional ChangesPolicy rule ID exclusion for eligible-versions endpoint
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/workspace-engine/svc/controllers/desiredrelease/policyeval/policyeval.go`:
- Around line 184-187: The doc for FilterEvaluatorsByRuleID is inconsistent: it
claims the returned slice never shares backing storage but currently returns the
input slice directly when excludeIDs is empty; change the implementation of
FilterEvaluatorsByRuleID so that even in the empty-excludeIDs path it returns a
shallow copy of evals (e.g., allocate a new slice of len(evals) and copy
elements) to avoid aliasing, preserving existing element pointers but ensuring
independent backing storage; update any early-return that returns evals directly
to instead return the copied slice while keeping the function signature and
behavior otherwise unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f69949aa-3f58-4b77-9b8f-52b8af257e7a
📒 Files selected for processing (13)
apps/api/openapi/openapi.jsonapps/api/openapi/paths/release-targets.jsonnetapps/api/src/routes/v1/workspaces/release-targets.tsapps/api/src/types/openapi.tsapps/workspace-engine/oapi/openapi.jsonapps/workspace-engine/oapi/spec/paths/release_targets.jsonnetapps/workspace-engine/pkg/oapi/oapi.gen.goapps/workspace-engine/svc/controllers/desiredrelease/policyeval/policyeval.goapps/workspace-engine/svc/controllers/desiredrelease/policyeval/policyeval_test.goapps/workspace-engine/svc/http/server/openapi/release_targets/eligible_versions.goe2e/api/schema.tse2e/tests/api/release-target-eligible-versions.spec.tspackages/workspace-engine-sdk/src/schema.ts
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for excluding specific policy rule IDs when computing the “eligible versions” set for a release target, enabling callers to “what-if” ignore a particular constraint (e.g., a versionSelector pin) while keeping all other policy evaluation intact.
Changes:
- Extends the eligible-versions request body with
excludeRuleIdsacross OpenAPI specs and generated SDK/types. - Implements evaluator filtering in workspace-engine before policy evaluation runs.
- Adds e2e coverage for bypassing a versionSelector pin, plus unit tests for the evaluator-filter helper.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/workspace-engine-sdk/src/schema.ts | Adds excludeRuleIds to the SDK request schema for eligible versions. |
| e2e/tests/api/release-target-eligible-versions.spec.ts | Adds an e2e test validating that excluding a rule ID bypasses a versionSelector pin. |
| e2e/api/schema.ts | Updates e2e client schema to include excludeRuleIds. |
| apps/workspace-engine/svc/http/server/openapi/release_targets/eligible_versions.go | Parses excludeRuleIds and filters evaluators before evaluating deployable versions. |
| apps/workspace-engine/svc/controllers/desiredrelease/policyeval/policyeval.go | Introduces FilterEvaluatorsByRuleID helper used by the endpoint. |
| apps/workspace-engine/svc/controllers/desiredrelease/policyeval/policyeval_test.go | Adds unit tests for FilterEvaluatorsByRuleID. |
| apps/workspace-engine/pkg/oapi/oapi.gen.go | Regenerates OAPI types to include ExcludeRuleIds in the request body struct. |
| apps/workspace-engine/oapi/spec/paths/release_targets.jsonnet | Updates workspace-engine OpenAPI jsonnet path spec to define excludeRuleIds. |
| apps/workspace-engine/oapi/openapi.json | Updates rendered workspace-engine OpenAPI JSON with excludeRuleIds. |
| apps/api/src/types/openapi.ts | Updates API service OpenAPI-derived TS types to include excludeRuleIds. |
| apps/api/src/routes/v1/workspaces/release-targets.ts | Wires excludeRuleIds through the API layer to workspace-engine. |
| apps/api/openapi/paths/release-targets.jsonnet | Updates API OpenAPI jsonnet path spec to define excludeRuleIds. |
| apps/api/openapi/openapi.json | Updates rendered API OpenAPI JSON with excludeRuleIds. |
Files not reviewed (1)
- apps/workspace-engine/pkg/oapi/oapi.gen.go: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| excludeIDs []string, | ||
| ) []evaluator.Evaluator { | ||
| if len(excludeIDs) == 0 { | ||
| return evals |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests