feat: ability to match agents to specific resources#963
Merged
adityachoudhari26 merged 3 commits intomainfrom Apr 9, 2026
Merged
feat: ability to match agents to specific resources#963adityachoudhari26 merged 3 commits intomainfrom
adityachoudhari26 merged 3 commits intomainfrom
Conversation
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 9 minutes and 25 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR introduces resource-aware job-agent selectors by extending the CEL evaluation environment to expose resource fields, creating a new Changes
Sequence DiagramsequenceDiagram
participant Controller as Controller
participant Getter as Getter (DB)
participant Selector as Selector
participant CEL as CEL Engine
Controller->>Getter: GetResource(resourceID)
Getter-->>Controller: *oapi.Resource
Controller->>Selector: MatchJobAgentsWithResource(selector, agents, resource)
Selector->>Selector: Compile selector expression
Selector->>CEL: Evaluate with jobAgent & resource variables
CEL-->>Selector: matched agents
Selector-->>Controller: []oapi.JobAgent
Controller->>Controller: Process matched agents
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Pull request overview
Adds support for resource-aware job-agent selection so job agents can be matched based on properties of the target resource (config/metadata) in addition to job-agent fields.
Changes:
- Extend CEL selector evaluation to include a
resourcevariable alongsidejobAgent. - Update job eligibility, job dispatch, and deployment plan flows to fetch the target resource and evaluate selectors with resource context.
- Add/adjust mocks and tests to cover resource-aware selector behavior and GetResource plumbing.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| apps/workspace-engine/pkg/selector/jobagents.go | Adds MatchJobAgentsWithResource and resource mapping for CEL selector evaluation. |
| apps/workspace-engine/svc/controllers/jobeligibility/reconcile.go | Fetches resource and evaluates selectors using resource-aware matching before creating jobs. |
| apps/workspace-engine/svc/controllers/jobeligibility/reconcile_test.go | Adds test coverage for selectors referencing resource.config and resource.metadata. |
| apps/workspace-engine/svc/controllers/jobdispatch/getters.go | Extends Getter interface to include GetResource. |
| apps/workspace-engine/svc/controllers/jobdispatch/getters_postgres.go | Implements GetResource in the Postgres getter. |
| apps/workspace-engine/svc/controllers/jobdispatch/reconcile.go | Fetches resource from release target and evaluates selectors with resource-aware matching. |
| apps/workspace-engine/svc/controllers/jobdispatch/reconcile_test.go | Updates controller test getter mock to support GetResource. |
| apps/workspace-engine/svc/controllers/deploymentplan/controller.go | Moves selector matching into per-target processing using resource-aware matching. |
| apps/workspace-engine/test/controllers/harness/mocks.go | Updates harness mock to implement GetResource. |
Comments suppressed due to low confidence (1)
apps/workspace-engine/svc/controllers/deploymentplan/controller.go:87
- Deployment plans are no longer completed when there are job agents in the workspace but none match the selector for any release target.
Processonly callsCompletePlanwhenlen(allAgents)==0, andprocessTargetjust returns nil whenlen(matchedAgents)==0, so the plan can remain stuck with no enqueued results. Track whether any target/agent result was enqueued and callCompletePlanat the end when none were, or reintroduce a plan-level fast-path when the selector matches zero agents across all targets.
allAgents, err := c.getter.ListJobAgentsByWorkspaceID(ctx, plan.WorkspaceID)
if err != nil {
return reconcile.Result{}, fmt.Errorf("list job agents: %w", err)
}
if len(allAgents) == 0 {
if err := c.setter.CompletePlan(ctx, planID); err != nil {
return reconcile.Result{}, fmt.Errorf("mark plan completed: %w", err)
}
return reconcile.Result{}, nil
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
106
to
+111
| vars := map[string]any{ | ||
| "jobAgent": jobAgentToMap(&agents[i]), | ||
| } | ||
| if resource != nil { | ||
| vars["resource"] = resourceToMap(resource) | ||
| } |
There was a problem hiding this comment.
resourceToMap(resource) is recomputed for every agent during selector evaluation, allocating and populating the same map repeatedly. Precompute the mapped resource once (outside the agent loop) and reuse it in vars to reduce allocations and CPU when evaluating selectors against many agents.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/workspace-engine/pkg/selector/jobagents.go (2)
76-83: Makeresourcecontract explicit inMatchJobAgentsWithResourceOn Line 80,
resourceis pointer-typed but treated as optional implicitly. Returning a clear early error fornilavoids ambiguous per-agent CEL evaluation failures later.Proposed change
func MatchJobAgentsWithResource( _ context.Context, selector string, agents []oapi.JobAgent, resource *oapi.Resource, ) ([]oapi.JobAgent, error) { + if resource == nil { + return nil, fmt.Errorf("match job agents with resource: resource is required") + } return matchJobAgents(jobAgentWithResourceEnv, selector, agents, resource) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/pkg/selector/jobagents.go` around lines 76 - 83, Make the resource pointer contract explicit in MatchJobAgentsWithResource: detect if the incoming resource argument is nil at the start of MatchJobAgentsWithResource and return a clear error instead of proceeding; this prevents ambiguous per-agent CEL evaluation failures later in matchJobAgents and in jobAgentWithResourceEnv which expect a non-nil resource. Ensure the returned error message clearly states that resource is required (e.g., "resource required") and update callers/tests as needed.
109-111: Avoid recomputingresourceToMapfor every agentLine 110 rebuilds the same resource map on each iteration. Compute it once before the loop and reuse it.
Proposed change
var matched []oapi.JobAgent + var resourceVars map[string]any + if resource != nil { + resourceVars = resourceToMap(resource) + } for i := range agents { vars := map[string]any{ "jobAgent": jobAgentToMap(&agents[i]), } if resource != nil { - vars["resource"] = resourceToMap(resource) + vars["resource"] = resourceVars } ok, err := celutil.EvalBool(prg, vars)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/pkg/selector/jobagents.go` around lines 109 - 111, The code currently calls resourceToMap(resource) inside the per-agent loop, rebuilding the same map for every agent; compute the map once before the loop (e.g., resourceMap := resourceToMap(resource) when resource != nil) and then set vars["resource"] = resourceMap inside the loop, reusing that single map instead of recomputing it each iteration (reference: resourceToMap, vars, resource).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/workspace-engine/pkg/selector/jobagents.go`:
- Around line 76-83: Make the resource pointer contract explicit in
MatchJobAgentsWithResource: detect if the incoming resource argument is nil at
the start of MatchJobAgentsWithResource and return a clear error instead of
proceeding; this prevents ambiguous per-agent CEL evaluation failures later in
matchJobAgents and in jobAgentWithResourceEnv which expect a non-nil resource.
Ensure the returned error message clearly states that resource is required
(e.g., "resource required") and update callers/tests as needed.
- Around line 109-111: The code currently calls resourceToMap(resource) inside
the per-agent loop, rebuilding the same map for every agent; compute the map
once before the loop (e.g., resourceMap := resourceToMap(resource) when resource
!= nil) and then set vars["resource"] = resourceMap inside the loop, reusing
that single map instead of recomputing it each iteration (reference:
resourceToMap, vars, resource).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b86878e0-a9ac-4566-a80a-6622db39d8ce
📒 Files selected for processing (9)
apps/workspace-engine/pkg/selector/jobagents.goapps/workspace-engine/svc/controllers/deploymentplan/controller.goapps/workspace-engine/svc/controllers/jobdispatch/getters.goapps/workspace-engine/svc/controllers/jobdispatch/getters_postgres.goapps/workspace-engine/svc/controllers/jobdispatch/reconcile.goapps/workspace-engine/svc/controllers/jobdispatch/reconcile_test.goapps/workspace-engine/svc/controllers/jobeligibility/reconcile.goapps/workspace-engine/svc/controllers/jobeligibility/reconcile_test.goapps/workspace-engine/test/controllers/harness/mocks.go
jsbroks
reviewed
Apr 9, 2026
Comment on lines
+64
to
+83
| func MatchJobAgents( | ||
| ctx context.Context, | ||
| _ context.Context, | ||
| selector string, | ||
| agents []oapi.JobAgent, | ||
| ) ([]oapi.JobAgent, error) { | ||
| return matchJobAgents(jobAgentEnv, selector, agents, nil) | ||
| } | ||
|
|
||
| // MatchJobAgentsWithResource evaluates a CEL job agent selector against a list | ||
| // of job agents with the resource available in the CEL context, and returns | ||
| // those that match. This allows selectors to reference resource properties, | ||
| // e.g. "jobAgent.config.server == resource.config.argocd.serverUrl". | ||
| func MatchJobAgentsWithResource( | ||
| _ context.Context, | ||
| selector string, | ||
| agents []oapi.JobAgent, | ||
| resource *oapi.Resource, | ||
| ) ([]oapi.JobAgent, error) { | ||
| return matchJobAgents(jobAgentWithResourceEnv, selector, agents, resource) | ||
| } |
Member
There was a problem hiding this comment.
seems weird to wrap this in a function. Also why do we need MatchJobAgents?
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.
Resolves #962
Summary by CodeRabbit
resource.config.*,resource.metadata.*) for more sophisticated agent matching logic.