Skip to content

fix: include deployment agent configs in v1 responses #910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
adityachoudhari26 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: include deployment agent configs in v1 responses #910

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
90 changes: 60 additions & 30 deletions apps/api/src/routes/v1/workspaces/deployments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { AsyncTypedHandler } from "@/types/api.js";
import { ApiError, asyncHandler } from "@/types/api.js";
import { Router } from "express";
import _ from "lodash";
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

lodash is imported here, but apps/api/package.json does not declare lodash as a dependency (only @types/lodash). With pnpm's strict node_modules layout this will likely fail at runtime/build. Either add lodash to this package's dependencies or replace the grouping logic with a small local reducer/Map to avoid the extra dependency.

Copilot uses AI. Check for mistakes.
import { v4 as uuidv4 } from "uuid";

import { and, asc, count, desc, eq, inArray, takeFirst } from "@ctrlplane/db";
Expand All @@ -12,14 +13,30 @@ import {
} from "@ctrlplane/db/reconcilers";
import * as schema from "@ctrlplane/db/schema";

const PLAN_TTL_MS = 60 * 60 * 1000; // 1 hour
// 1 hour

import { validResourceSelector } from "../valid-selector.js";
import { listDeploymentVariablesByDeploymentRouter } from "./deployment-variables.js";

Comment on lines +16 to 20
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

The // 1 hour comment is now separated from PLAN_TTL_MS and sits between import blocks, which makes it easy to miss/misread. Move the comment to be adjacent to PLAN_TTL_MS (or inline with the constant) so it documents the value it refers to.

Copilot uses AI. Check for mistakes.
const parseSelector = (
raw: string | null | undefined,
): string | undefined => {
const PLAN_TTL_MS = 60 * 60 * 1000;
Comment on lines +16 to +21
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Comment is detached from the constant it describes.

The // 1 hour comment on line 16 appears before an import statement, making it unclear what it refers to. It should be directly above PLAN_TTL_MS.

📝 Suggested fix 
-// 1 hour
-
 import { validResourceSelector } from "../valid-selector.js";
 import { listDeploymentVariablesByDeploymentRouter } from "./deployment-variables.js";

+// 1 hour
 const PLAN_TTL_MS = 60 * 60 * 1000;
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/api/src/routes/v1/workspaces/deployments.ts` around lines 16 - 21, The
inline comment "// 1 hour" is detached from the constant it documents; move that
comment so it sits directly above the PLAN_TTL_MS declaration to make the intent
clear. Locate the PLAN_TTL_MS constant and reposition the comment immediately
above it (remove or relocate the stray comment before the imports) so the
comment clearly documents PLAN_TTL_MS.


const getJobAgentsByDeploymentId = async (
deploymentIds: string[],
): Promise<Record<string, { ref: string; config: Record<string, any> }[]>> => {
if (deploymentIds.length === 0) return {};
const links = await db
.select()
.from(schema.deploymentJobAgent)
.where(inArray(schema.deploymentJobAgent.deploymentId, deploymentIds));
return _.chain(links)
.groupBy((l) => l.deploymentId)
.mapValues((group) =>
group.map((l) => ({ ref: l.jobAgentId, config: l.config })),
)
.value();
};

const parseSelector = (raw: string | null | undefined): string | undefined => {
if (raw == null || raw === "false") return undefined;
return raw;
};
Expand Down Expand Up @@ -84,16 +101,16 @@ const listDeployments: AsyncTypedHandler<
const systemLinks =
deploymentIds.length > 0
? await db
.select({
deploymentId: schema.systemDeployment.deploymentId,
system: schema.system,
})
.from(schema.systemDeployment)
.innerJoin(
schema.system,
eq(schema.systemDeployment.systemId, schema.system.id),
)
.where(inArray(schema.systemDeployment.deploymentId, deploymentIds))
.select({
deploymentId: schema.systemDeployment.deploymentId,
system: schema.system,
})
.from(schema.systemDeployment)
.innerJoin(
schema.system,
eq(schema.systemDeployment.systemId, schema.system.id),
)
.where(inArray(schema.systemDeployment.deploymentId, deploymentIds))
: [];

const systemsByDeploymentId = new Map<
Expand All @@ -106,8 +123,14 @@ const listDeployments: AsyncTypedHandler<
systemsByDeploymentId.set(link.deploymentId, arr);
}

const agentsByDeploymentId = await getJobAgentsByDeploymentId(deploymentIds);
const items = deployments.map((dep) => ({
deployment: formatDeployment(dep),
deployment: {
...formatDeployment(dep),
jobAgents: (agentsByDeploymentId[dep.id] ?? []).map(
({ ref, config }) => ({ ref, config }),
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

DeploymentJobAgent in the generated OpenAPI types includes a required selector field, but the response objects added here only contain { ref, config }. This makes the v1 response shape inconsistent with the documented schema/clients. Either include a selector value (e.g. default to "true") or update the OpenAPI schema/types to make selector optional/removed to match the persisted model.

Suggested change
({ ref, config }) => ({ ref, config }),
({ ref, config, selector }) => ({
ref,
config,
selector: selector ?? "true",
}),

Copilot uses AI. Check for mistakes.
),
},
systems: (systemsByDeploymentId.get(dep.id) ?? []).map(formatSystem),
}));

Expand All @@ -129,6 +152,8 @@ const getDeployment: AsyncTypedHandler<

if (dep == null) throw new ApiError("Deployment not found", 404);

const agentsByDeploymentId = await getJobAgentsByDeploymentId([deploymentId]);

const systemRows = await db
.select({ system: schema.system })
.from(schema.systemDeployment)
Expand All @@ -147,14 +172,14 @@ const getDeployment: AsyncTypedHandler<
const variableValues =
variableIds.length > 0
? await db
.select()
.from(schema.deploymentVariableValue)
.where(
inArray(
schema.deploymentVariableValue.deploymentVariableId,
variableIds,
),
)
.select()
.from(schema.deploymentVariableValue)
.where(
inArray(
schema.deploymentVariableValue.deploymentVariableId,
variableIds,
),
)
: [];

const valuesByVariableId = new Map<
Expand All @@ -168,7 +193,12 @@ const getDeployment: AsyncTypedHandler<
}

res.status(200).json({
deployment: formatDeployment(dep),
deployment: {
...formatDeployment(dep),
jobAgents: (agentsByDeploymentId[dep.id] ?? []).map(
({ ref, config }) => ({ ref, config }),
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

Same as in listDeployments: the jobAgents objects returned here omit the selector field that DeploymentJobAgent is documented to require in the generated OpenAPI types. Align the runtime response with the schema by including selector (or adjust the schema/types if selector is no longer part of the model).

Suggested change
({ ref, config }) => ({ ref, config }),
({ ref, config, selector }) => ({ ref, config, selector }),

Copilot uses AI. Check for mistakes.
),
},
systems: systemRows.map((r) => formatSystem(r.system)),
variables: variables.map((v) => ({
variable: {
Expand Down Expand Up @@ -448,12 +478,12 @@ const getDeploymentPlan: AsyncTypedHandler<
status === "computing"
? null
: {
total: allResults.length,
changed,
unchanged,
errored,
unsupported,
};
total: allResults.length,
changed,
unchanged,
errored,
unsupported,
};

res.status(200).json({
id: plan.id,
Expand Down
Loading