refactor: extract deployment job agents into separate table#908
Merged
adityachoudhari26 merged 6 commits intomainfrom Apr 2, 2026
Merged
refactor: extract deployment job agents into separate table#908adityachoudhari26 merged 6 commits intomainfrom
adityachoudhari26 merged 6 commits intomainfrom
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 (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR refactors the deployment-to-job-agent relationship from embedded JSON fields in the deployment table to a proper many-to-many join table ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
jsbroks
approved these changes
Apr 2, 2026
mleonidas
approved these changes
Apr 2, 2026
There was a problem hiding this comment.
Pull request overview
Refactors how deployments reference job agents by moving deployment-specific agent configuration out of the deployment table and into a new join table, then updates TRPC/API/workspace-engine/web consumers accordingly.
Changes:
- Add
deployment_job_agentjoin table and remove legacy deployment job-agent columns/JSON. - Update TRPC and workspace-engine DB queries/controllers to read/write deployment job agents via the new table.
- Adjust web UI components/types to consume the new
deployment.jobAgentsshape and job-agent deployment listing query shape.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/trpc/src/routes/job-agents.ts | Switch deployments lookup to join via deploymentJobAgent table. |
| packages/trpc/src/routes/deployments.ts | Include jobAgents relation on get; change mutation to upsert into deploymentJobAgent. |
| packages/db/src/schema/deployment.ts | Remove legacy columns and add deploymentJobAgent table + relations. |
| packages/db/drizzle/meta/0182_snapshot.json | Drizzle snapshot updated for schema change. |
| packages/db/drizzle/meta/_journal.json | Drizzle journal updated for migration 0182. |
| packages/db/drizzle/0182_rainy_changeling.sql | Migration creates deployment_job_agent, backfills, drops legacy columns. |
| apps/workspace-engine/svc/controllers/relationshipeval/getters_postgres.go | Remove legacy job-agent fields from relationship eval payload. |
| apps/workspace-engine/svc/controllers/jobeligibility/getters_postgres.go | Use new query/conversion that includes job agents. |
| apps/workspace-engine/svc/controllers/jobdispatch/getters_postgres.go | Use new query/conversion that includes job agents. |
| apps/workspace-engine/svc/controllers/deploymentplan/getters_postgres.go | Use new query/conversion that includes job agents. |
| apps/workspace-engine/pkg/db/queries/schema.sql | Add deployment_job_agent table; remove legacy deployment columns. |
| apps/workspace-engine/pkg/db/queries/deployments.sql | Add GetDeploymentWithJobAgents and DML for deployment_job_agent. |
| apps/workspace-engine/pkg/db/queries/computed_relationships.sql | Remove job-agent columns from relationship-eval deployment queries. |
| apps/workspace-engine/pkg/db/models.go | Update Deployment model; add DeploymentJobAgent model. |
| apps/workspace-engine/pkg/db/deployments.sql.go | Regenerated sqlc code for updated deployment queries and new DML. |
| apps/workspace-engine/pkg/db/convert.go | Add ToOapiDeploymentWithJobAgents conversion helper. |
| apps/workspace-engine/pkg/db/convert_test.go | Update deployment conversion tests + add job-agents conversion tests. |
| apps/workspace-engine/pkg/db/computed_relationships.sql.go | Regenerated sqlc code for updated relationship-eval queries. |
| apps/web/app/routes/ws/runners/JobAgentCard.tsx | Update rendering for new TRPC jobAgents.deployments return shape. |
| apps/web/app/routes/ws/deployments/settings/page.$deploymentId.job-agent.tsx | Update list keying to use jobAgentId. |
| apps/web/app/routes/ws/deployments/settings/_components/DeploymentAgentCard.tsx | Update deployment job-agent type and lookup to use jobAgentId. |
| apps/web/app/routes/ws/deployments/_components/types.ts | Remove legacy jobAgentId/jobAgentConfig fields from Deployment type. |
| apps/web/app/routes/ws/deployments/_components/DeploymentProvider.tsx | Update deployment context typing for new jobAgents shape. |
| apps/api/src/routes/v1/workspaces/systems.ts | Remove legacy job-agent fields from deployment formatting in systems endpoints. |
| apps/api/src/routes/v1/workspaces/resources.ts | Remove legacy job-agent fields from deployment items in resource endpoints. |
| apps/api/src/routes/v1/workspaces/deployments.ts | Remove legacy job-agent fields from deployment formatting and create/upsert persistence. |
Comments suppressed due to low confidence (1)
apps/api/src/routes/v1/workspaces/deployments.ts:211
- postDeployment/upsertDeployment previously persisted job-agent related fields from the request. With the new schema, those request fields (if still present in the API contract) are now silently ignored. Consider rejecting unknown job-agent fields, or add dedicated endpoints to manage deployment_job_agent mappings so clients have a clear migration path.
await db.insert(schema.deployment).values({
id,
name: body.name,
description: body.description ?? "",
resourceSelector: body.resourceSelector ?? "false",
metadata: body.metadata ?? {},
workspaceId,
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| jsonb_array_elements(d.job_agents) AS agent | ||
| WHERE d.job_agents IS NOT NULL | ||
| AND jsonb_array_length(d.job_agents) > 0 | ||
| AND EXISTS (SELECT 1 FROM job_agent ja WHERE ja.id = (agent->>'ref')::uuid) |
There was a problem hiding this comment.
The backfill into deployment_job_agent only migrates rows from the legacy deployment.job_agents JSON array. Any deployments that only used the legacy job_agent_id/job_agent_config columns (and had an empty job_agents array) will lose their agent association when those columns are dropped. Consider inserting an additional row for (d.job_agent_id, d.job_agent_config) when job_agent_id is set and exists in job_agent.
Suggested change
| AND EXISTS (SELECT 1 FROM job_agent ja WHERE ja.id = (agent->>'ref')::uuid) | |
| AND EXISTS (SELECT 1 FROM job_agent ja WHERE ja.id = (agent->>'ref')::uuid) | |
| UNION ALL | |
| SELECT d.id, d.job_agent_id, COALESCE(d.job_agent_config, '{}'::jsonb) | |
| FROM deployment d | |
| WHERE d.job_agent_id IS NOT NULL | |
| AND EXISTS (SELECT 1 FROM job_agent ja WHERE ja.id = d.job_agent_id) |
Member
Author
There was a problem hiding this comment.
these were already previously migrated, so we dont have to worry about those
| "config" jsonb DEFAULT '{}' NOT NULL, | ||
| CONSTRAINT "deployment_job_agent_deployment_id_job_agent_id_pk" PRIMARY KEY("deployment_id","job_agent_id") | ||
| ); | ||
| --> statement-breakpoint |
There was a problem hiding this comment.
deployment_job_agent only has the composite PK (deployment_id, job_agent_id). Queries that filter by job_agent_id (e.g. listing deployments for a job agent) won't be able to use that index efficiently because job_agent_id isn't the leading column. Add a separate index on job_agent_id (and consider one on deployment_id if you expect frequent lookups both ways).
Suggested change
| --> statement-breakpoint | |
| --> statement-breakpoint | |
| CREATE INDEX "deployment_job_agent_job_agent_id_idx" ON "deployment_job_agent" USING btree ("job_agent_id");--> statement-breakpoint |
Member
Author
There was a problem hiding this comment.
can add later if this actually becomes a problem
Comment on lines
93
to
104
| deployments: protectedProcedure | ||
| .input(z.object({ workspaceId: z.uuid(), jobAgentId: z.string() })) | ||
| .query(({ input, ctx }) => | ||
| ctx.db | ||
| .select() | ||
| .from(schema.deployment) | ||
| .where(eq(schema.deployment.jobAgentId, input.jobAgentId)), | ||
| .select({ deployment: schema.deployment }) | ||
| .from(schema.deploymentJobAgent) | ||
| .innerJoin( | ||
| schema.deployment, | ||
| eq(schema.deploymentJobAgent.deploymentId, schema.deployment.id), | ||
| ) | ||
| .where(eq(schema.deploymentJobAgent.jobAgentId, input.jobAgentId)), | ||
| ), |
There was a problem hiding this comment.
This procedure accepts workspaceId but does not use it to filter results, and it also lacks an authorizationCheck meta like the deployments router. As written, any authenticated user could query deployments for an arbitrary jobAgentId across workspaces. Filter by deployment.workspaceId = input.workspaceId (and/or jobAgent.workspaceId) and add an authorization check tied to the workspace.
Comment on lines
+229
to
+250
| const { workspaceId, deploymentId, jobAgentId, config } = input; | ||
|
|
||
| const [updated] = await ctx.db | ||
| .update(schema.deployment) | ||
| .set({ jobAgentId, jobAgentConfig: typedConfig }) | ||
| .where(eq(schema.deployment.id, deploymentId)) | ||
| .returning(); | ||
| const deployment = await ctx.db.query.deployment.findFirst({ | ||
| where: eq(schema.deployment.id, deploymentId), | ||
| }); | ||
|
|
||
| if (updated == null) | ||
| if (deployment == null) | ||
| throw new TRPCError({ | ||
| code: "NOT_FOUND", | ||
| message: "Deployment not found", | ||
| }); | ||
|
|
||
| await ctx.db | ||
| .insert(schema.deploymentJobAgent) | ||
| .values({ deploymentId, jobAgentId, config }) | ||
| .onConflictDoUpdate({ | ||
| target: [ | ||
| schema.deploymentJobAgent.deploymentId, | ||
| schema.deploymentJobAgent.jobAgentId, | ||
| ], | ||
| set: { config }, | ||
| }); |
There was a problem hiding this comment.
updateJobAgent uses workspaceId only for enqueuing follow-up work; it does not verify that the deployment belongs to that workspace, nor that the jobAgentId belongs to the same workspace as the deployment. This allows cross-workspace associations if a user can call this mutation. Validate deployment.workspaceId === workspaceId and ensure the referenced job agent is in the same workspace (or enforce it at the DB level).
| ]); | ||
|
|
||
| return updated; | ||
| return deployment; |
There was a problem hiding this comment.
This mutation returns the deployment record fetched before the insert/update into deployment_job_agent, so callers won’t see the updated jobAgents/config reflected in the response. Consider returning the upserted deploymentJobAgent row, or re-fetching the deployment with with: { jobAgents: true } after the write.
Suggested change
| return deployment; | |
| const updatedDeployment = await ctx.db.query.deployment.findFirst({ | |
| where: eq(schema.deployment.id, deploymentId), | |
| with: { jobAgents: true }, | |
| }); | |
| if (updatedDeployment == null) | |
| throw new TRPCError({ | |
| code: "NOT_FOUND", | |
| message: "Deployment not found", | |
| }); | |
| return updatedDeployment; |
Comment on lines
27
to
34
| const formatDeployment = (dep: typeof schema.deployment.$inferSelect) => ({ | ||
| id: dep.id, | ||
| name: dep.name, | ||
| slug: dep.name, | ||
| description: dep.description, | ||
| jobAgentId: dep.jobAgentId ?? undefined, | ||
| jobAgentConfig: dep.jobAgentConfig, | ||
| jobAgents: dep.jobAgents, | ||
| resourceSelector: parseSelector(dep.resourceSelector), | ||
| metadata: dep.metadata, | ||
| }); |
There was a problem hiding this comment.
These v1 endpoints no longer include jobAgentId/jobAgentConfig/jobAgents in the deployment representation. That is a breaking response-shape change for existing clients and also means any job-agent fields sent in create/upsert requests will be silently ignored. Either update the v1 API contract/OpenAPI accordingly (and communicate the breaking change) or keep these fields by joining deployment_job_agent (or providing a dedicated endpoint) during a transition period.
Comment on lines
35
to
42
| const formatDeployment = (dep: typeof schema.deployment.$inferSelect) => ({ | ||
| id: dep.id, | ||
| name: dep.name, | ||
| slug: dep.name, | ||
| description: dep.description, | ||
| jobAgentId: dep.jobAgentId ?? undefined, | ||
| jobAgentConfig: dep.jobAgentConfig, | ||
| jobAgents: dep.jobAgents, | ||
| resourceSelector: parseSelector(dep.resourceSelector), | ||
| metadata: dep.metadata, | ||
| }); |
There was a problem hiding this comment.
formatDeployment for the Systems API no longer includes jobAgentId/jobAgentConfig/jobAgents. If those fields were part of the published v1 contract, this is a breaking change for clients consuming system deployments. Consider either keeping job-agent info by joining deployment_job_agent (or exposing it via a dedicated endpoint) or updating/versioning the contract explicitly.
Comment on lines
290
to
297
| const items = rows.map((r) => ({ | ||
| id: r.deployment.id, | ||
| name: r.deployment.name, | ||
| slug: r.deployment.name, | ||
| description: r.deployment.description, | ||
| jobAgentId: r.deployment.jobAgentId ?? undefined, | ||
| jobAgentConfig: r.deployment.jobAgentConfig, | ||
| jobAgents: r.deployment.jobAgents, | ||
| resourceSelector: parseSelector(r.deployment.resourceSelector), | ||
| metadata: r.deployment.metadata, | ||
| })); |
There was a problem hiding this comment.
This deployments-for-resource response drops jobAgentId/jobAgentConfig/jobAgents from each deployment item. If clients relied on those fields in v1, this is a breaking change. Either include job-agent info by joining deployment_job_agent (or provide a dedicated endpoint) or ensure the API contract is updated/versioned accordingly.
![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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/trpc/src/routes/job-agents.ts (1)
93-103:⚠️ Potential issue | 🟠 MajorReapply workspace scoping to this deployments query.
The new join only filters by
jobAgentId;workspaceIdis now ignored. That means this endpoint can return deployments from another workspace when given a foreign agent ID, and it weakens any authorization flow that keys off the requested workspace.🔧 Suggested fix
-import { eq, takeFirst } from "@ctrlplane/db"; +import { and, eq, takeFirst } from "@ctrlplane/db"; ... deployments: protectedProcedure .input(z.object({ workspaceId: z.uuid(), jobAgentId: z.string() })) .query(({ input, ctx }) => ctx.db .select({ deployment: schema.deployment }) .from(schema.deploymentJobAgent) .innerJoin( schema.deployment, eq(schema.deploymentJobAgent.deploymentId, schema.deployment.id), ) - .where(eq(schema.deploymentJobAgent.jobAgentId, input.jobAgentId)), + .innerJoin( + schema.jobAgent, + eq(schema.deploymentJobAgent.jobAgentId, schema.jobAgent.id), + ) + .where( + and( + eq(schema.deploymentJobAgent.jobAgentId, input.jobAgentId), + eq(schema.deployment.workspaceId, input.workspaceId), + eq(schema.jobAgent.workspaceId, input.workspaceId), + ), + ), ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/trpc/src/routes/job-agents.ts` around lines 93 - 103, The deployments query in the protectedProcedure (deployments) currently only filters by input.jobAgentId and ignores input.workspaceId; update the ctx.db query that selects from schema.deploymentJobAgent and joins schema.deployment so it also enforces the workspace scope by adding a filter that deployment.workspaceId equals input.workspaceId (e.g., include eq(schema.deployment.workspaceId, input.workspaceId) in the where clause or join predicate). Ensure the check references the existing symbols schema.deployment, schema.deploymentJobAgent, and input.workspaceId so the endpoint only returns deployments for the requested workspace.apps/workspace-engine/pkg/db/convert.go (1)
23-38:⚠️ Potential issue | 🟠 MajorPropagate job-agent decode failures instead of hiding them.
Line 34 swallows
json.Unmarshalerrors, so bad aggregated data is returned asJobAgents == nilwith anilerror. That makes corrupt payloads indistinguishable from a deployment that genuinely has no agent associations. Since callers already returnerror, this helper should surface the decode failure.🛠️ Suggested fix
import ( "encoding/json" + "fmt" "github.com/charmbracelet/log" "github.com/google/uuid" "workspace-engine/pkg/oapi" ) @@ -func ToOapiDeploymentWithJobAgents(row GetDeploymentWithJobAgentsRow) *oapi.Deployment { +func ToOapiDeploymentWithJobAgents(row GetDeploymentWithJobAgentsRow) (*oapi.Deployment, error) { d := ToOapiDeployment(Deployment{ ID: row.ID, Name: row.Name, Description: row.Description, ResourceSelector: row.ResourceSelector, Metadata: row.Metadata, WorkspaceID: row.WorkspaceID, }) if row.JobAgents != nil { var jobAgents []oapi.DeploymentJobAgent - if err := json.Unmarshal(row.JobAgents, &jobAgents); err == nil { - d.JobAgents = &jobAgents - } + if err := json.Unmarshal(row.JobAgents, &jobAgents); err != nil { + return nil, fmt.Errorf("unmarshal deployment job agents: %w", err) + } + d.JobAgents = &jobAgents } - return d + return d, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/pkg/db/convert.go` around lines 23 - 38, The helper ToOapiDeploymentWithJobAgents currently swallows json.Unmarshal errors and must surface them: change its signature to return (*oapi.Deployment, error) instead of just *oapi.Deployment, attempt to json.Unmarshal row.JobAgents into jobAgents and if err != nil return nil and the error (wrap with context mentioning ToOapiDeploymentWithJobAgents / JobAgents decode), otherwise set d.JobAgents and return d, nil; update all callers to handle the error accordingly.
🧹 Nitpick comments (1)
apps/workspace-engine/pkg/db/queries/deployments.sql (1)
7-13: Stabilize thejob_agentsarray order.
json_aggwithoutORDER BYis plan-dependent, so the same deployment can serialize itsjob_agentsin different orders across reads. If this payload is diffed or cached downstream, that turns into noisy churn for semantically identical data.♻️ Minimal SQL tweak
SELECT d.*, - COALESCE(json_agg(json_build_object('ref', dja.job_agent_id, 'config', dja.config)) - FILTER (WHERE dja.job_agent_id IS NOT NULL), '[]') AS job_agents + COALESCE( + json_agg( + json_build_object('ref', dja.job_agent_id, 'config', dja.config) + ORDER BY dja.job_agent_id + ) FILTER (WHERE dja.job_agent_id IS NOT NULL), + '[]' + ) AS job_agents FROM deployment d LEFT JOIN deployment_job_agent dja ON dja.deployment_id = d.id WHERE d.id = $1 GROUP BY d.id;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/pkg/db/queries/deployments.sql` around lines 7 - 13, The job_agents array produced by json_agg in the SELECT for deployment (query using deployment d and deployment_job_agent dja, grouped by d.id) is unordered and plan-dependent; modify the aggregation to produce a stable order by adding an ORDER BY inside the aggregate (e.g., order by a stable column such as dja.job_agent_id or another deterministic column) so json_agg(json_build_object(...)) always emits job_agents in a consistent order across reads while keeping the GROUP BY d.id intact.
🤖 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/workspace-engine/pkg/db/computed_relationships.sql.go`:
- Around line 85-95: The new projection removed job-agent fields and breaks
backward compatibility for saved CEL rules; update the SELECT and
GetDeploymentForRelEvalRow to include jobAgentId and jobAgentConfig (join
deployment_job_agent on deployment.id) so the projection still returns
deployment.jobAgentId and deployment.jobAgentConfig (nullable) alongside id,
workspace_id, name, description, metadata; ensure the struct adds corresponding
fields (types matching stored values) and apply the same change to the other
projection instance (the one around the later block) so both evaluation shapes
remain compatible.
In `@apps/workspace-engine/pkg/db/queries/schema.sql`:
- Around line 25-30: The deployment_job_agent table references job_agent(id)
before job_agent is created, causing a foreign-key creation error on a clean DB;
fix by reordering the schema so the CREATE TABLE job_agent definition appears
before the CREATE TABLE deployment_job_agent (or alternatively create job_agent
first or add the referenced table beforehand) so the foreign key reference in
deployment_job_agent can be resolved during CREATE TABLE execution.
In `@packages/db/drizzle/0182_rainy_changeling.sql`:
- Around line 10-21: The migration currently only backfills from
deployment.job_agents and will lose rows that used the legacy
deployment.job_agent_id and deployment.job_agent_config; update the INSERT into
deployment_job_agent to also insert rows derived from the legacy columns (e.g.,
a second SELECT or UNION ALL SELECT d.id, d.job_agent_id,
COALESCE(d.job_agent_config, '{}'::jsonb) FROM deployment d WHERE d.job_agent_id
IS NOT NULL) before dropping columns, preserve JSONB config with COALESCE, and
keep the existing EXISTS check and ON CONFLICT DO NOTHING behavior to avoid
duplicates (targeting the deployment_job_agent table and the legacy columns
job_agent_id and job_agent_config on the deployment table).
In `@packages/trpc/src/routes/deployments.ts`:
- Around line 231-257: The handler currently reads `deployment` before doing the
upsert and returns that stale snapshot; instead, after the call to
`ctx.db.insert(...).values(...).onConflictDoUpdate(...)` you should re-query for
the post-upsert state and return it (either re-fetch the `deployment` including
its `deployment_job_agent` relation or directly read the affected
`deployment_job_agent` via `ctx.db.query.deploymentJobAgent.findFirst({ where:
...deploymentId & jobAgentId })`) rather than returning the original
`deployment` variable; keep the existing enqueues
(`enqueueDeploymentSelectorEval`/`enqueueReleaseTargetsForDeployment`) in place.
---
Outside diff comments:
In `@apps/workspace-engine/pkg/db/convert.go`:
- Around line 23-38: The helper ToOapiDeploymentWithJobAgents currently swallows
json.Unmarshal errors and must surface them: change its signature to return
(*oapi.Deployment, error) instead of just *oapi.Deployment, attempt to
json.Unmarshal row.JobAgents into jobAgents and if err != nil return nil and the
error (wrap with context mentioning ToOapiDeploymentWithJobAgents / JobAgents
decode), otherwise set d.JobAgents and return d, nil; update all callers to
handle the error accordingly.
In `@packages/trpc/src/routes/job-agents.ts`:
- Around line 93-103: The deployments query in the protectedProcedure
(deployments) currently only filters by input.jobAgentId and ignores
input.workspaceId; update the ctx.db query that selects from
schema.deploymentJobAgent and joins schema.deployment so it also enforces the
workspace scope by adding a filter that deployment.workspaceId equals
input.workspaceId (e.g., include eq(schema.deployment.workspaceId,
input.workspaceId) in the where clause or join predicate). Ensure the check
references the existing symbols schema.deployment, schema.deploymentJobAgent,
and input.workspaceId so the endpoint only returns deployments for the requested
workspace.
---
Nitpick comments:
In `@apps/workspace-engine/pkg/db/queries/deployments.sql`:
- Around line 7-13: The job_agents array produced by json_agg in the SELECT for
deployment (query using deployment d and deployment_job_agent dja, grouped by
d.id) is unordered and plan-dependent; modify the aggregation to produce a
stable order by adding an ORDER BY inside the aggregate (e.g., order by a stable
column such as dja.job_agent_id or another deterministic column) so
json_agg(json_build_object(...)) always emits job_agents in a consistent order
across reads while keeping the GROUP BY d.id intact.
🪄 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: 224f52e9-aa1c-40fe-b1c7-e942b2770943
📒 Files selected for processing (26)
apps/api/src/routes/v1/workspaces/deployments.tsapps/api/src/routes/v1/workspaces/resources.tsapps/api/src/routes/v1/workspaces/systems.tsapps/web/app/routes/ws/deployments/_components/DeploymentProvider.tsxapps/web/app/routes/ws/deployments/_components/types.tsapps/web/app/routes/ws/deployments/settings/_components/DeploymentAgentCard.tsxapps/web/app/routes/ws/deployments/settings/page.$deploymentId.job-agent.tsxapps/web/app/routes/ws/runners/JobAgentCard.tsxapps/workspace-engine/pkg/db/computed_relationships.sql.goapps/workspace-engine/pkg/db/convert.goapps/workspace-engine/pkg/db/convert_test.goapps/workspace-engine/pkg/db/deployments.sql.goapps/workspace-engine/pkg/db/models.goapps/workspace-engine/pkg/db/queries/computed_relationships.sqlapps/workspace-engine/pkg/db/queries/deployments.sqlapps/workspace-engine/pkg/db/queries/schema.sqlapps/workspace-engine/svc/controllers/deploymentplan/getters_postgres.goapps/workspace-engine/svc/controllers/jobdispatch/getters_postgres.goapps/workspace-engine/svc/controllers/jobeligibility/getters_postgres.goapps/workspace-engine/svc/controllers/relationshipeval/getters_postgres.gopackages/db/drizzle/0182_rainy_changeling.sqlpackages/db/drizzle/meta/0182_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/deployment.tspackages/trpc/src/routes/deployments.tspackages/trpc/src/routes/job-agents.ts
💤 Files with no reviewable changes (4)
- apps/api/src/routes/v1/workspaces/systems.ts
- apps/web/app/routes/ws/deployments/_components/types.ts
- apps/api/src/routes/v1/workspaces/deployments.ts
- apps/api/src/routes/v1/workspaces/resources.ts
Comment on lines
+10
to
+21
| INSERT INTO deployment_job_agent (deployment_id, job_agent_id, config) | ||
| SELECT d.id, (agent->>'ref')::uuid, COALESCE(agent->'config', '{}'::jsonb) | ||
| FROM deployment d, | ||
| jsonb_array_elements(d.job_agents) AS agent | ||
| WHERE d.job_agents IS NOT NULL | ||
| AND jsonb_array_length(d.job_agents) > 0 | ||
| AND EXISTS (SELECT 1 FROM job_agent ja WHERE ja.id = (agent->>'ref')::uuid) | ||
| ON CONFLICT DO NOTHING; | ||
| --> statement-breakpoint | ||
| ALTER TABLE "deployment" DROP COLUMN "job_agent_id";--> statement-breakpoint | ||
| ALTER TABLE "deployment" DROP COLUMN "job_agent_config";--> statement-breakpoint | ||
| ALTER TABLE "deployment" DROP COLUMN "job_agents"; |
Contributor
There was a problem hiding this comment.
Backfill the legacy singular job-agent fields before dropping them.
This migration only copies deployment.job_agents. Any deployment that still stores its association in job_agent_id / job_agent_config reaches Lines 19-21 with no replacement row, so the job-agent binding is silently lost during upgrade. Merge both legacy sources into the backfill before dropping the old columns.
🛠️ Suggested migration shape
INSERT INTO deployment_job_agent (deployment_id, job_agent_id, config)
-SELECT d.id, (agent->>'ref')::uuid, COALESCE(agent->'config', '{}'::jsonb)
-FROM deployment d,
- jsonb_array_elements(d.job_agents) AS agent
-WHERE d.job_agents IS NOT NULL
- AND jsonb_array_length(d.job_agents) > 0
- AND EXISTS (SELECT 1 FROM job_agent ja WHERE ja.id = (agent->>'ref')::uuid)
+SELECT d.id, d.job_agent_id, COALESCE(d.job_agent_config, '{}'::jsonb)
+FROM deployment d
+WHERE d.job_agent_id IS NOT NULL
+ AND EXISTS (SELECT 1 FROM job_agent ja WHERE ja.id = d.job_agent_id)
+UNION ALL
+SELECT d.id, (agent->>'ref')::uuid, COALESCE(agent->'config', '{}'::jsonb)
+FROM deployment d
+CROSS JOIN LATERAL jsonb_array_elements(COALESCE(d.job_agents, '[]'::jsonb)) AS agent
+WHERE EXISTS (SELECT 1 FROM job_agent ja WHERE ja.id = (agent->>'ref')::uuid)
ON CONFLICT DO NOTHING;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| INSERT INTO deployment_job_agent (deployment_id, job_agent_id, config) | |
| SELECT d.id, (agent->>'ref')::uuid, COALESCE(agent->'config', '{}'::jsonb) | |
| FROM deployment d, | |
| jsonb_array_elements(d.job_agents) AS agent | |
| WHERE d.job_agents IS NOT NULL | |
| AND jsonb_array_length(d.job_agents) > 0 | |
| AND EXISTS (SELECT 1 FROM job_agent ja WHERE ja.id = (agent->>'ref')::uuid) | |
| ON CONFLICT DO NOTHING; | |
| --> statement-breakpoint | |
| ALTER TABLE "deployment" DROP COLUMN "job_agent_id";--> statement-breakpoint | |
| ALTER TABLE "deployment" DROP COLUMN "job_agent_config";--> statement-breakpoint | |
| ALTER TABLE "deployment" DROP COLUMN "job_agents"; | |
| INSERT INTO deployment_job_agent (deployment_id, job_agent_id, config) | |
| SELECT d.id, d.job_agent_id, COALESCE(d.job_agent_config, '{}'::jsonb) | |
| FROM deployment d | |
| WHERE d.job_agent_id IS NOT NULL | |
| AND EXISTS (SELECT 1 FROM job_agent ja WHERE ja.id = d.job_agent_id) | |
| UNION ALL | |
| SELECT d.id, (agent->>'ref')::uuid, COALESCE(agent->'config', '{}'::jsonb) | |
| FROM deployment d | |
| CROSS JOIN LATERAL jsonb_array_elements(COALESCE(d.job_agents, '[]'::jsonb)) AS agent | |
| WHERE EXISTS (SELECT 1 FROM job_agent ja WHERE ja.id = (agent->>'ref')::uuid) | |
| ON CONFLICT DO NOTHING; | |
| --> statement-breakpoint | |
| ALTER TABLE "deployment" DROP COLUMN "job_agent_id";--> statement-breakpoint | |
| ALTER TABLE "deployment" DROP COLUMN "job_agent_config";--> statement-breakpoint | |
| ALTER TABLE "deployment" DROP COLUMN "job_agents"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/db/drizzle/0182_rainy_changeling.sql` around lines 10 - 21, The
migration currently only backfills from deployment.job_agents and will lose rows
that used the legacy deployment.job_agent_id and deployment.job_agent_config;
update the INSERT into deployment_job_agent to also insert rows derived from the
legacy columns (e.g., a second SELECT or UNION ALL SELECT d.id, d.job_agent_id,
COALESCE(d.job_agent_config, '{}'::jsonb) FROM deployment d WHERE d.job_agent_id
IS NOT NULL) before dropping columns, preserve JSONB config with COALESCE, and
keep the existing EXISTS check and ON CONFLICT DO NOTHING behavior to avoid
duplicates (targeting the deployment_job_agent table and the legacy columns
job_agent_id and job_agent_config on the deployment table).
Member
Author
There was a problem hiding this comment.
those were already migrated to use the newer arrays, so we dont need to worry about those
Contributor
There was a problem hiding this comment.
@adityachoudhari26, thanks for the clarification! That makes sense — if a prior migration already normalized all job_agent_id/job_agent_config rows into the job_agents array, then reading only from job_agents for the backfill is correct and complete. I'll note this for future reference.
✏️ Learnings added
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 908
File: packages/db/drizzle/0182_rainy_changeling.sql:10-21
Timestamp: 2026-04-02T22:38:27.059Z
Learning: In `packages/db/drizzle/0182_rainy_changeling.sql` (PR `#908`), the legacy `deployment.job_agent_id` and `deployment.job_agent_config` columns were already migrated to the `deployment.job_agents` JSONB array in a prior migration before this PR ran. Therefore, the backfill INSERT only needs to read from `job_agents` — no need to also handle the singular `job_agent_id`/`job_agent_config` columns. Do not flag the absence of those columns in the backfill query.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 908
File: apps/workspace-engine/pkg/db/computed_relationships.sql.go:85-95
Timestamp: 2026-04-02T22:36:20.973Z
Learning: In `apps/workspace-engine/pkg/db/computed_relationships.sql.go`, `GetDeploymentForRelEval` and `ListDeploymentsByWorkspace` intentionally omit `job_agent_id` and `job_agent_config` from their projections after the refactor that moved job-agent data to a separate `deployment_job_agent` table. No existing CEL relationship-eval rules reference these fields, so backward compatibility is not a concern. Do not flag the absence of job-agent fields in these projections.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 637
File: packages/events/src/kafka/client.ts:10-16
Timestamp: 2025-08-01T04:41:41.345Z
Learning: User adityachoudhari26 prefers not to add null safety checks for required environment variables when they are guaranteed to be present in their deployment configuration, similar to their preference for simplicity over defensive programming in test code.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 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
Repo: ctrlplanedev/ctrlplane PR: 818
File: apps/workspace-engine/pkg/db/queries/schema.sql:252-282
Timestamp: 2026-02-26T23:01:30.641Z
Learning: User adityachoudhari26 prefers to defer adding database indexes on foreign keys until there's evidence they're needed, considering such additions premature optimization at the initial schema design stage.
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 #906
Summary by CodeRabbit