chore: display selected agents in UI#944
Merged
adityachoudhari26 merged 2 commits intomainfrom Apr 9, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a backend endpoint and TRPC query to list job agents matching a deployment's selector, updates OpenAPI/SDK, implements the server handler and selector filtering, and refactors the frontend to fetch and render unified Changes
Sequence DiagramsequenceDiagram
participant Client as React UI
participant TRPC as TRPC Router
participant HTTP as HTTP Server
participant DB as Database
participant Selector as selector.MatchJobAgents
Client->>TRPC: jobAgents({ deploymentId })
TRPC->>TRPC: authorize (Permission.DeploymentGet)
TRPC->>HTTP: GET /v1/deployments/{deploymentId}/job-agents
HTTP->>DB: GetDeploymentByID(deploymentId)
DB-->>HTTP: Deployment { id, workspaceId, jobAgentSelector }
HTTP->>DB: ListJobAgentsByWorkspaceID(workspaceId)
DB-->>HTTP: JobAgent[]
HTTP->>Selector: MatchJobAgents(agents, jobAgentSelector)
Selector-->>HTTP: matched JobAgent[]
HTTP-->>TRPC: { items: matched }
TRPC-->>Client: JobAgent[] -> render DeploymentAgentCard*
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Pull request overview
Adds support for displaying the job agents selected by a deployment’s job-agent selector, end-to-end (Workspace Engine OpenAPI endpoint → SDK types → tRPC route → Web UI).
Changes:
- Introduces
GET /v1/deployments/{deploymentId}/job-agentsin Workspace Engine OpenAPI and server implementation. - Adds a new
deployment.jobAgentstRPC query that calls the Workspace Engine endpoint. - Updates the deployment settings “Job Agents” page to show the selector and matched agents using
DeploymentAgentCard.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/workspace-engine-sdk/src/schema.ts | Regenerates SDK types to include the new deployments/job-agents endpoint. |
| packages/trpc/src/routes/deployments.ts | Adds a protected tRPC query to fetch matched job agents for a deployment. |
| apps/workspace-engine/svc/http/server/openapi/server.go | Wires the new deployments OpenAPI handlers into the server. |
| apps/workspace-engine/svc/http/server/openapi/deployments/server.go | Implements the handler that fetches deployment + filters job agents by selector. |
| apps/workspace-engine/pkg/oapi/oapi.gen.go | Regenerates server interface + router bindings for the new endpoint. |
| apps/workspace-engine/oapi/spec/paths/deployment.jsonnet | Defines the new OpenAPI path. |
| apps/workspace-engine/oapi/spec/main.jsonnet | Includes the new deployment paths module in the composed spec. |
| apps/workspace-engine/oapi/openapi.json | Regenerated OpenAPI output including the new endpoint. |
| apps/web/app/routes/ws/deployments/settings/page.$deploymentId.job-agent.tsx | Fetches and renders matched agents + shows selector text. |
| apps/web/app/routes/ws/deployments/settings/_components/DeploymentAgentCard.tsx | Refactors card to render a JobAgent directly (no per-agent fetch). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+25
to
+29
| deployment, err := queries.GetDeploymentByID(ctx, deploymentUUID) | ||
| if err != nil { | ||
| c.JSON(http.StatusNotFound, gin.H{"error": "Deployment not found"}) | ||
| return | ||
| } |
There was a problem hiding this comment.
GetDeploymentByID errors are all mapped to 404. That will incorrectly return “Deployment not found” for transient DB errors. Distinguish pgx.ErrNoRows (404) from other errors (500) so callers can tell not-found vs internal failure.
| }, | ||
| required: ['items'], | ||
| }, 'Job agents matching the deployment selector') | ||
| + openapi.badRequestResponse(), |
There was a problem hiding this comment.
The OpenAPI spec for this endpoint only declares 200/400 responses, but the handler returns 404 when the deployment is missing (and may return 500 on DB failures). Add openapi.notFoundResponse() (and any other explicitly returned statuses) so the contract matches the implementation and generated SDK types.
Suggested change
| + openapi.badRequestResponse(), | |
| + openapi.badRequestResponse() | |
| + openapi.notFoundResponse(), |
Comment on lines
+575
to
+577
| jobAgents: protectedProcedure | ||
| .input(z.object({ deploymentId: z.string() })) | ||
| .meta({ |
There was a problem hiding this comment.
Input validation should match the other deployment procedures: deploymentId is a UUID elsewhere (z.uuid()). Using z.string() here allows invalid IDs through and can bypass early validation / produce confusing downstream errors.
Comment on lines
+20
to
+28
| {deployment.jobAgentSelector != null && | ||
| deployment.jobAgentSelector !== "false" && ( | ||
| <div className="space-y-1"> | ||
| <h4 className="text-sm font-medium">Selector</h4> | ||
| <pre className="rounded-md bg-muted p-3 font-mono text-xs"> | ||
| {deployment.jobAgentSelector} | ||
| </pre> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
jobAgentSelector is typed as a string and can be empty when not set. The current condition will still render an empty “Selector” block for "". Consider also checking for a non-empty selector (and still excluding the sentinel "false") before rendering.
![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: 6
🧹 Nitpick comments (2)
apps/web/app/routes/ws/deployments/settings/_components/DeploymentAgentCard.tsx (1)
27-32: ReplaceanyinJobAgent.configwith a safer explicit type.
Record<string, any>weakens downstream type safety in config rendering. Prefer an interface withunknownvalues.♻️ Suggested typing update
-type JobAgent = { +interface JobAgent { id: string; name: string; type: string; - config: Record<string, any>; -}; + config: Record<string, unknown>; +}As per coding guidelines:
**/*.{ts,tsx}: Use explicit types in TypeScript code; Prefer interfaces for public APIs in TypeScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/routes/ws/deployments/settings/_components/DeploymentAgentCard.tsx` around lines 27 - 32, The JobAgent type uses Record<string, any> for config which weakens type safety; replace it with a safer explicit interface (e.g., declare interface JobAgentConfig { [key: string]: unknown } and update type JobAgent to use config: JobAgentConfig) and adjust any consumers of JobAgent.config to narrow/check values (type guards or runtime casts) where specific keys are accessed; update the exported type to an interface per guidelines and ensure any rendering code casts/validates config values before use.apps/workspace-engine/svc/http/server/openapi/deployments/server.go (1)
13-15: Add Go doc comments for exported symbols
DeploymentsandGetJobAgentsForDeploymentare exported but undocumented.As per coding guidelines: “Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/svc/http/server/openapi/deployments/server.go` around lines 13 - 15, Add Go doc comments for the exported type Deployments and its exported method GetJobAgentsForDeployment: add a comment above the Deployments type explaining its purpose/context (e.g., "Deployments handles HTTP handlers for deployment-related endpoints") and a comment above GetJobAgentsForDeployment describing what the method does, its parameters (c *gin.Context, deploymentId string), expected behavior, and any important details or side effects (e.g., authentication, response format, error handling). Keep comments concise, use proper Go doc style starting with the symbol name, and include any TODOs or non-obvious behavior as needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/web/app/routes/ws/deployments/settings/page`.$deploymentId.job-agent.tsx:
- Around line 8-10: The UI currently treats a failed
trpc.deployment.jobAgents.useQuery the same as an empty result because it only
checks isLoading and the agents array; change the render logic to explicitly
check isError (from the same useQuery result) before showing the empty-state UI
and render an error message or retry UI when isError is true; update the
component code that uses const { data: agents, isLoading } =
trpc.deployment.jobAgents.useQuery(...) to also destructure isError and branch:
if (isLoading) show loader, else if (isError) show error UI, else if (!agents ||
agents.length === 0) show empty-state; apply the exact same fix to the other
identical query/render block in this file that follows the same pattern.
In `@apps/workspace-engine/oapi/spec/paths/deployment.jsonnet`:
- Around line 11-21: The OpenAPI response block for the deployment selector
endpoint only includes ok and badRequest but misses the handler's 404 and 500
responses; update the responses union to include openapi.notFoundResponse() (for
deployment not found) and openapi.internalServerErrorResponse() (for
list/internal failures) alongside openapi.okResponse(...) and
openapi.badRequestResponse() so the spec matches the runtime behavior exposed by
the handler in server.go.
In `@apps/workspace-engine/svc/http/server/openapi/deployments/server.go`:
- Around line 42-49: MatchJobAgents may return nil which serializes as JSON
null; before responding from the handler that calls selector.MatchJobAgents(ctx,
deployment.JobAgentSelector, oapiAgents) ensure the matched value is converted
to an empty slice when nil (e.g., replace nil with an empty []<appropriate
element type>{}) so c.JSON(http.StatusOK, gin.H{"items": matched}) always emits
an array; update the handler around the matched variable check and return the
corrected slice.
- Around line 25-29: The current handler always maps any error from
queries.GetDeploymentByID(ctx, deploymentUUID) to a 404; change it to detect a
true "not found" error (e.g., compare to sql.ErrNoRows or the package's sentinel
not-found error returned by GetDeploymentByID) and only call
c.JSON(http.StatusNotFound, ...) in that case, otherwise log the error and
return c.JSON(http.StatusInternalServerError, ...) for other failures; update
the branch around GetDeploymentByID and use deploymentUUID and c (gin.Context)
as the referenced symbols to locate the code.
In `@packages/trpc/src/routes/deployments.ts`:
- Around line 589-593: The current handler throws a generic
INTERNAL_SERVER_ERROR when result.error is non-null; instead detect and preserve
upstream client-facing errors (e.g., 4xx) and only use INTERNAL_SERVER_ERROR for
true server faults. Update the code around the result check (the block that
throws the TRPCError for listing job agents — referencing the result variable in
deployments.ts) to inspect result.error (status/code/message) and map known
client statuses to corresponding TRPCError codes (e.g., 400 -> BAD_REQUEST, 404
-> NOT_FOUND, 401/403 -> UNAUTHORIZED/FORBIDDEN) and return the upstream message
(result.error.message) rather than JSON.stringify the whole error; fall back to
INTERNAL_SERVER_ERROR only when no client status is present. Ensure you preserve
error context in the TRPCError message and do not collapse 4xx responses into
500.
In `@packages/workspace-engine-sdk/src/schema.ts`:
- Around line 1373-1394: The responses object for the JobAgent listing endpoint
only declares 200 and 400 but must include runtime 404 and 500 outcomes; update
the OpenAPI source to add a 404 response (e.g., deployment not found with
ErrorResponse schema) and a 500 response (internal server error with
ErrorResponse schema) alongside the existing 200/400 entries, then regenerate
the SDK so packages/workspace-engine-sdk/src/schema.ts (the responses block for
the JobAgent listing operation) includes these status codes.
---
Nitpick comments:
In
`@apps/web/app/routes/ws/deployments/settings/_components/DeploymentAgentCard.tsx`:
- Around line 27-32: The JobAgent type uses Record<string, any> for config which
weakens type safety; replace it with a safer explicit interface (e.g., declare
interface JobAgentConfig { [key: string]: unknown } and update type JobAgent to
use config: JobAgentConfig) and adjust any consumers of JobAgent.config to
narrow/check values (type guards or runtime casts) where specific keys are
accessed; update the exported type to an interface per guidelines and ensure any
rendering code casts/validates config values before use.
In `@apps/workspace-engine/svc/http/server/openapi/deployments/server.go`:
- Around line 13-15: Add Go doc comments for the exported type Deployments and
its exported method GetJobAgentsForDeployment: add a comment above the
Deployments type explaining its purpose/context (e.g., "Deployments handles HTTP
handlers for deployment-related endpoints") and a comment above
GetJobAgentsForDeployment describing what the method does, its parameters (c
*gin.Context, deploymentId string), expected behavior, and any important details
or side effects (e.g., authentication, response format, error handling). Keep
comments concise, use proper Go doc style starting with the symbol name, and
include any TODOs or non-obvious behavior as needed.
🪄 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: b141f765-679a-4aa0-8364-76c085874108
📒 Files selected for processing (10)
apps/web/app/routes/ws/deployments/settings/_components/DeploymentAgentCard.tsxapps/web/app/routes/ws/deployments/settings/page.$deploymentId.job-agent.tsxapps/workspace-engine/oapi/openapi.jsonapps/workspace-engine/oapi/spec/main.jsonnetapps/workspace-engine/oapi/spec/paths/deployment.jsonnetapps/workspace-engine/pkg/oapi/oapi.gen.goapps/workspace-engine/svc/http/server/openapi/deployments/server.goapps/workspace-engine/svc/http/server/openapi/server.gopackages/trpc/src/routes/deployments.tspackages/workspace-engine-sdk/src/schema.ts
Comment on lines
+8
to
+10
| const { data: agents, isLoading } = trpc.deployment.jobAgents.useQuery({ | ||
| deploymentId: deployment.id, | ||
| }); |
Contributor
There was a problem hiding this comment.
Handle query errors separately from true empty results.
If the query fails, the current condition falls through to the empty-state message, which is misleading. Add an explicit error branch (isError) before the empty-state branch.
🛠️ Suggested UI-state fix
- const { data: agents, isLoading } = trpc.deployment.jobAgents.useQuery({
+ const {
+ data: agents,
+ isLoading,
+ isError,
+ error,
+ } = trpc.deployment.jobAgents.useQuery({
deploymentId: deployment.id,
});
@@
- {!isLoading && (agents == null || agents.length === 0) && (
+ {!isLoading && isError && (
+ <p className="text-sm text-destructive">
+ Failed to load job agents: {error.message}
+ </p>
+ )}
+
+ {!isLoading && !isError && (agents == null || agents.length === 0) && (
<p className="text-sm text-muted-foreground">
No job agents match the current selector.
</p>
)}Also applies to: 46-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/routes/ws/deployments/settings/page`.$deploymentId.job-agent.tsx
around lines 8 - 10, The UI currently treats a failed
trpc.deployment.jobAgents.useQuery the same as an empty result because it only
checks isLoading and the agents array; change the render logic to explicitly
check isError (from the same useQuery result) before showing the empty-state UI
and render an error message or retry UI when isError is true; update the
component code that uses const { data: agents, isLoading } =
trpc.deployment.jobAgents.useQuery(...) to also destructure isError and branch:
if (isLoading) show loader, else if (isError) show error UI, else if (!agents ||
agents.length === 0) show empty-state; apply the exact same fix to the other
identical query/render block in this file that follows the same pattern.
Comment on lines
+11
to
+21
| responses: openapi.okResponse({ | ||
| type: 'object', | ||
| properties: { | ||
| items: { | ||
| type: 'array', | ||
| items: openapi.schemaRef('JobAgent'), | ||
| }, | ||
| }, | ||
| required: ['items'], | ||
| }, 'Job agents matching the deployment selector') | ||
| + openapi.badRequestResponse(), |
Contributor
There was a problem hiding this comment.
OpenAPI response contract is missing handler error statuses.
The handler in apps/workspace-engine/svc/http/server/openapi/deployments/server.go returns 404 (deployment not found) and 500 (list failure), but this spec only defines 200/400. Please add those responses here so generated SDK/server contracts match runtime behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/workspace-engine/oapi/spec/paths/deployment.jsonnet` around lines 11 -
21, The OpenAPI response block for the deployment selector endpoint only
includes ok and badRequest but misses the handler's 404 and 500 responses;
update the responses union to include openapi.notFoundResponse() (for deployment
not found) and openapi.internalServerErrorResponse() (for list/internal
failures) alongside openapi.okResponse(...) and openapi.badRequestResponse() so
the spec matches the runtime behavior exposed by the handler in server.go.
Comment on lines
+589
to
+593
| if (result.error != null) { | ||
| throw new TRPCError({ | ||
| code: "INTERNAL_SERVER_ERROR", | ||
| message: `Failed to list job agents for deployment: ${JSON.stringify(result.error)}`, | ||
| }); |
Contributor
There was a problem hiding this comment.
Do not collapse upstream 4xx errors into INTERNAL_SERVER_ERROR.
This endpoint can legitimately return caller-facing errors (for example invalid ID / missing deployment). Mapping all failures to 500 makes client behavior and UI states misleading.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/trpc/src/routes/deployments.ts` around lines 589 - 593, The current
handler throws a generic INTERNAL_SERVER_ERROR when result.error is non-null;
instead detect and preserve upstream client-facing errors (e.g., 4xx) and only
use INTERNAL_SERVER_ERROR for true server faults. Update the code around the
result check (the block that throws the TRPCError for listing job agents —
referencing the result variable in deployments.ts) to inspect result.error
(status/code/message) and map known client statuses to corresponding TRPCError
codes (e.g., 400 -> BAD_REQUEST, 404 -> NOT_FOUND, 401/403 ->
UNAUTHORIZED/FORBIDDEN) and return the upstream message (result.error.message)
rather than JSON.stringify the whole error; fall back to INTERNAL_SERVER_ERROR
only when no client status is present. Ensure you preserve error context in the
TRPCError message and do not collapse 4xx responses into 500.
Comment on lines
+1373
to
+1394
| responses: { | ||
| /** @description Job agents matching the deployment selector */ | ||
| 200: { | ||
| headers: { | ||
| [name: string]: unknown; | ||
| }; | ||
| content: { | ||
| "application/json": { | ||
| items: components["schemas"]["JobAgent"][]; | ||
| }; | ||
| }; | ||
| }; | ||
| /** @description Invalid request */ | ||
| 400: { | ||
| headers: { | ||
| [name: string]: unknown; | ||
| }; | ||
| content: { | ||
| "application/json": components["schemas"]["ErrorResponse"]; | ||
| }; | ||
| }; | ||
| }; |
Contributor
There was a problem hiding this comment.
SDK response types are missing runtime 404/500 outcomes
Line 1373 defines only 200 and 400, but the server implementation can also return 404 (deployment not found) and 500 (job-agent listing failure). This leaves client handling under-typed and out of sync with runtime behavior.
Please add these responses in the OpenAPI source and regenerate this file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/workspace-engine-sdk/src/schema.ts` around lines 1373 - 1394, The
responses object for the JobAgent listing endpoint only declares 200 and 400 but
must include runtime 404 and 500 outcomes; update the OpenAPI source to add a
404 response (e.g., deployment not found with ErrorResponse schema) and a 500
response (internal server error with ErrorResponse schema) alongside the
existing 200/400 entries, then regenerate the SDK so
packages/workspace-engine-sdk/src/schema.ts (the responses block for the
JobAgent listing operation) includes these status codes.
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 #936
Summary by CodeRabbit