Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,14 @@ import {
DialogTrigger,
} from "~/components/ui/dialog";
import { ConfigEntry } from "~/components/config-entry";
import { Skeleton } from "~/components/ui/skeleton";
import { useJobAgent } from "../_hooks/job-agents";

type DeploymentJobAgent = {
deploymentId: string;
jobAgentId: string;
type JobAgent = {
id: string;
name: string;
type: string;
config: Record<string, any>;
};

type DeploymentAgentCardProps = {
deploymentAgent: DeploymentJobAgent;
};
function TypeIcon({ type }: { type: string }) {
if (type === "github-app") return <SiGithub className="size-8" />;
if (type === "argo-cd")
Expand All @@ -44,26 +40,6 @@ function TypeIcon({ type }: { type: string }) {
return <PlayIcon className="size-8" />;
}

function SkeletonCard() {
return (
<Card>
<CardHeader>
<CardTitle>
<Skeleton className="h-4 w-24" />
</CardTitle>
<CardDescription>
<Skeleton className="h-4 w-24" />
</CardDescription>
</CardHeader>

<CardContent className="space-y-2">
<Skeleton className="h-4 w-full" />
<Skeleton className="h-4 w-full" />
</CardContent>
</Card>
);
}

function ConfigExpanded({
config,
label,
Expand Down Expand Up @@ -120,29 +96,19 @@ function Config({
);
}

export function DeploymentAgentCard({
deploymentAgent,
}: DeploymentAgentCardProps) {
const { jobAgent, isLoading } = useJobAgent(deploymentAgent.jobAgentId);
if (isLoading) return <SkeletonCard />;
if (jobAgent == null) return null;

export function DeploymentAgentCard({ agent }: { agent: JobAgent }) {
return (
<Card>
<CardHeader>
<CardTitle className="flex items-center gap-2">
<TypeIcon {...jobAgent} />
{jobAgent.name}
<TypeIcon type={agent.type} />
{agent.name}
</CardTitle>
<CardDescription>{jobAgent.type}</CardDescription>
<CardDescription>{agent.type}</CardDescription>
</CardHeader>

<CardContent className="space-y-6 text-xs">
<Config config={jobAgent.config} label="Job Agent Config" />
<Config
config={deploymentAgent.config}
label="Deployment Agent Config"
/>
<Config config={agent.config} label="Agent Config" />
</CardContent>
</Card>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,53 @@
import { trpc } from "~/api/trpc";
import { Skeleton } from "~/components/ui/skeleton";
import { useDeployment } from "../_components/DeploymentProvider";
import { DeploymentAgentCard } from "./_components/DeploymentAgentCard";

export default function DeploymentJobAgentPage() {
const { deployment } = useDeployment();
const { data: agents, isLoading } = trpc.deployment.jobAgents.useQuery({
deploymentId: deployment.id,
});
Comment on lines +8 to +10
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.


return (
<div className="m-8 max-w-3xl justify-center space-y-6">
<div className="space-y-2">
<h2 className="text-2xl font-bold">Job Agents</h2>
<p className="text-sm text-muted-foreground">
Job agents are used to dispatch jobs to the correct service. Without
an agent new deployment versions will not take any action.
Job agents matched by this deployment's selector. Without a matching
agent, new deployment versions will not take any action.
</p>
{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>
)}
Comment on lines +20 to +28
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
</div>

{isLoading && (
<div className="grid gap-4">
<Skeleton className="h-32 w-full" />
<Skeleton className="h-32 w-full" />
</div>
)}

{!isLoading && agents != null && agents.length > 0 && (
<div className="grid gap-4">
{agents.map((agent) => (
<DeploymentAgentCard key={agent.id} agent={agent} />
))}
</div>
)}

{!isLoading && (agents == null || agents.length === 0) && (
<p className="text-sm text-muted-foreground">
No job agents match the current selector.
</p>
)}
</div>
);
}
60 changes: 60 additions & 0 deletions apps/workspace-engine/oapi/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -3180,6 +3180,66 @@
},
"openapi": "3.0.0",
"paths": {
"/v1/deployments/{deploymentId}/job-agents": {
"get": {
"operationId": "getJobAgentsForDeployment",
"parameters": [
{
"description": "ID of the deployment",
"in": "path",
"name": "deploymentId",
"required": true,
"schema": {
"type": "string"
}
}
],
"responses": {
"200": {
"content": {
"application/json": {
"schema": {
"properties": {
"items": {
"items": {
"$ref": "#/components/schemas/JobAgent"
},
"type": "array"
}
},
"required": [
"items"
],
"type": "object"
}
}
},
"description": "Job agents matching the deployment selector"
},
"400": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/ErrorResponse"
}
}
},
"description": "Invalid request"
},
"404": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/ErrorResponse"
}
}
},
"description": "Resource not found"
}
},
"summary": "Get job agents matching a deployment selector"
}
},
"/v1/deployments/{deploymentId}/release-targets": {
"get": {
"operationId": "listReleaseTargets",
Expand Down
3 changes: 2 additions & 1 deletion apps/workspace-engine/oapi/spec/main.jsonnet
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
(import 'paths/release_targets.jsonnet') +
(import 'paths/jobs.jsonnet') +
(import 'paths/validate.jsonnet') +
(import 'paths/workflows.jsonnet'),
(import 'paths/workflows.jsonnet') +
(import 'paths/deployment.jsonnet'),

components: {
parameters: (import 'parameters/core.jsonnet'),
Expand Down
25 changes: 25 additions & 0 deletions apps/workspace-engine/oapi/spec/paths/deployment.jsonnet
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
local openapi = import '../lib/openapi.libsonnet';

{
'/v1/deployments/{deploymentId}/job-agents': {
get: {
summary: 'Get job agents matching a deployment selector',
operationId: 'getJobAgentsForDeployment',
parameters: [
openapi.deploymentIdParam(),
],
responses: openapi.okResponse({
type: 'object',
properties: {
items: {
type: 'array',
items: openapi.schemaRef('JobAgent'),
},
},
required: ['items'],
}, 'Job agents matching the deployment selector')
+ openapi.badRequestResponse()
+ openapi.notFoundResponse(),
},
},
}
28 changes: 28 additions & 0 deletions apps/workspace-engine/pkg/oapi/oapi.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package deployments

import (
"errors"
"net/http"

"github.com/gin-gonic/gin"
"github.com/google/uuid"
"github.com/jackc/pgx/v5"
"workspace-engine/pkg/db"
"workspace-engine/pkg/oapi"
"workspace-engine/pkg/selector"
)

type Deployments struct{}

func (d *Deployments) GetJobAgentsForDeployment(c *gin.Context, deploymentId string) {
ctx := c.Request.Context()
queries := db.GetQueries(ctx)

deploymentUUID, err := uuid.Parse(deploymentId)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid deployment ID"})
return
}

deployment, err := queries.GetDeploymentByID(ctx, deploymentUUID)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
c.JSON(http.StatusNotFound, gin.H{"error": "Deployment not found"})
return
}
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to get deployment"})
return
}
Comment on lines +27 to +35
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

allAgents, err := queries.ListJobAgentsByWorkspaceID(ctx, deployment.WorkspaceID)
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to list job agents"})
return
}

oapiAgents := make([]oapi.JobAgent, len(allAgents))
for i, row := range allAgents {
oapiAgents[i] = *db.ToOapiJobAgent(row)
}

matched, err := selector.MatchJobAgents(ctx, deployment.JobAgentSelector, oapiAgents)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "Failed to evaluate selector: " + err.Error()})
return
}

if matched == nil {
matched = []oapi.JobAgent{}
}

c.JSON(http.StatusOK, gin.H{"items": matched})
}
2 changes: 2 additions & 0 deletions apps/workspace-engine/svc/http/server/openapi/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package openapi
import (
"github.com/jackc/pgx/v5/pgxpool"
"workspace-engine/pkg/oapi"
"workspace-engine/svc/http/server/openapi/deployments"
release_targets "workspace-engine/svc/http/server/openapi/release_targets"
"workspace-engine/svc/http/server/openapi/resources"
"workspace-engine/svc/http/server/openapi/validators"
Expand All @@ -21,6 +22,7 @@ func New(pool *pgxpool.Pool) *Server {
var _ oapi.ServerInterface = &Server{}

type Server struct {
deployments.Deployments
resources.Resources
validators.Validator
workflows.Workflows
Expand Down
24 changes: 24 additions & 0 deletions packages/trpc/src/routes/deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -571,4 +571,28 @@ export const deploymentsRouter = router({
};
});
}),

jobAgents: protectedProcedure
.input(z.object({ deploymentId: z.string() }))
.meta({
Comment on lines +575 to +577
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
authorizationCheck: ({ canUser, input }) =>
canUser
.perform(Permission.DeploymentGet)
.on({ type: "deployment", id: input.deploymentId }),
})
.query(async ({ input }) => {
const result = await getClientFor().GET(
"/v1/deployments/{deploymentId}/job-agents",
{ params: { path: { deploymentId: input.deploymentId } } },
);

if (result.error != null) {
throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: `Failed to list job agents for deployment: ${JSON.stringify(result.error)}`,
});
Comment on lines +589 to +593
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

}

return result.data.items;
}),
});
Loading
Loading