Skip to content

chore: wire together deployment planning #953

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

Merged
adityachoudhari26 merged 2 commits into from
Apr 9, 2026
Merged

chore: wire together deployment planning #953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b23f39
chore: wire together deployment planning
adityachoudhari26 Apr 9, 2026
1e83d45
cleanup
adityachoudhari26 Apr 9, 2026
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
63 changes: 47 additions & 16 deletions apps/api/openapi/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,32 +565,61 @@
},
"DeploymentPlanTarget": {
"properties": {
"contentHash": {
"description": "Hash of the rendered output for change detection",
"environmentId": {
"type": "string"
},
"current": {
"description": "Full rendered output of the currently deployed state",
"environmentName": {
"type": "string"
},
"environmentId": {
"hasChanges": {
"description": "True if any result for this target has changes",
"type": "boolean"
},
"resourceId": {
"type": "string"
},
"environmentName": {
"resourceName": {
"type": "string"
},
"results": {
"items": {
"$ref": "#/components/schemas/DeploymentPlanTargetResult"
},
"type": "array"
}
},
"required": [
"environmentId",
"environmentName",
"resourceId",
"resourceName",
"hasChanges",
"results"
Comment on lines +574 to +597
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Keep state-dependent plan fields nullable/variant by status.

DeploymentPlanTargetResult still allows "computing", "errored", and "unsupported", but the schema now requires hasChanges, contentHash, current, proposed, and message for every result, and DeploymentPlanTarget.hasChanges is derived from those results. That forces the API to invent false / empty-string placeholders for states where those values are unknown or not applicable, especially for the 202 accepted plan response. Clients then can't distinguish “unchanged” from “not computed yet” or “empty output” from “no output”.

Prefer either:

  • keeping these fields nullable/optional outside "completed", or
  • splitting the result schema with oneOf keyed by status.

Also applies to: 603-623, 635-642

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/api/openapi/openapi.json` around lines 574 - 597, The OpenAPI schema
forces state-dependent fields to be present for all DeploymentPlanTargetResult
entries (fields: hasChanges, contentHash, current, proposed, message) and
derives DeploymentPlanTarget.hasChanges from them, which prevents distinguishing
“computing/accepted” states; update the schema so those fields are optional/null
when status != "completed" or replace DeploymentPlanTargetResult with a
discriminated oneOf union keyed by status (e.g., completed vs
computing/errored/unsupported) so completed results require
hasChanges/contentHash/current/proposed/message while other statuses omit or
allow nulls, and ensure DeploymentPlanTarget.hasChanges is computed only from
completed-result variants.

],
"type": "object"
},
"DeploymentPlanTargetResult": {
"properties": {
"contentHash": {
"description": "Hash of the rendered output for change detection",
"type": "string"
},
"current": {
"description": "Full rendered output of the currently deployed state",
"type": "string"
},
"hasChanges": {
"nullable": true,
"type": "boolean"
},
"proposed": {
"description": "Full rendered output of the proposed version",
"id": {
"type": "string"
},
"resourceId": {
"message": {
"description": "Agent message (e.g. error explanation or summary)",
"type": "string"
},
"resourceName": {
"proposed": {
"description": "Full rendered output of the proposed version",
"type": "string"
},
"status": {
Expand All @@ -604,11 +633,13 @@
}
},
"required": [
"environmentId",
"environmentName",
"resourceId",
"resourceName",
"status"
"id",
"status",
"hasChanges",
"contentHash",
"current",
"proposed",
"message"
],
"type": "object"
},
Expand Down
26 changes: 20 additions & 6 deletions apps/api/openapi/schemas/deployments.jsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,19 +108,33 @@ local jobAgentConfig = {
},
},

DeploymentPlanTargetResult: {
type: 'object',
required: ['id', 'status', 'hasChanges', 'contentHash', 'current', 'proposed', 'message'],
properties: {
id: { type: 'string' },
status: { type: 'string', enum: ['computing', 'completed', 'errored', 'unsupported'] },
hasChanges: { type: 'boolean' },
contentHash: { type: 'string', description: 'Hash of the rendered output for change detection' },
current: { type: 'string', description: 'Full rendered output of the currently deployed state' },
proposed: { type: 'string', description: 'Full rendered output of the proposed version' },
message: { type: 'string', description: 'Agent message (e.g. error explanation or summary)' },
},
},

DeploymentPlanTarget: {
type: 'object',
required: ['environmentId', 'environmentName', 'resourceId', 'resourceName', 'status'],
required: ['environmentId', 'environmentName', 'resourceId', 'resourceName', 'hasChanges', 'results'],
properties: {
environmentId: { type: 'string' },
environmentName: { type: 'string' },
resourceId: { type: 'string' },
resourceName: { type: 'string' },
status: { type: 'string', enum: ['computing', 'completed', 'errored', 'unsupported'] },
hasChanges: { type: 'boolean', nullable: true },
contentHash: { type: 'string', description: 'Hash of the rendered output for change detection' },
current: { type: 'string', description: 'Full rendered output of the currently deployed state' },
proposed: { type: 'string', description: 'Full rendered output of the proposed version' },
hasChanges: { type: 'boolean', description: 'True if any result for this target has changes' },
results: {
type: 'array',
items: openapi.schemaRef('DeploymentPlanTargetResult'),
},
},
},

Expand Down
29 changes: 13 additions & 16 deletions apps/api/src/routes/v1/workspaces/deployments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ const createDeploymentPlan: AsyncTypedHandler<
res.status(202).json({
id: planId,
status: "computing",
summary: null,
summary: { total: 0, changed: 0, unchanged: 0, errored: 0, unsupported: 0 },
targets: [],
});
};
Expand Down Expand Up @@ -437,11 +437,11 @@ const getDeploymentPlan: AsyncTypedHandler<
results: t.results.map((r) => ({
id: r.id,
status: r.status,
hasChanges: r.hasChanges,
current: r.current,
proposed: r.proposed,
contentHash: r.contentHash,
message: r.message,
hasChanges: r.hasChanges ?? false,
contentHash: r.contentHash ?? "",
current: r.current ?? "",
proposed: r.proposed ?? "",
message: r.message ?? "",
Comment on lines +440 to +444
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.

Coalescing nullable DB fields to false/"" here changes semantics and can make the response internally inconsistent: if r.hasChanges is NULL while status is computing, the API will return hasChanges: false for that result, but the summary.unchanged count is computed from the raw DB rows (r.hasChanges === false) and will not include NULLs. This can mislead clients into thinking there are unchanged results and also makes targets[].results[].hasChanges disagree with summary. Prefer omitting unknown fields (e.g., hasChanges: r.hasChanges ?? undefined, contentHash/current/proposed/message: ... ?? undefined) or alternatively compute the summary using the same coerced values so the counts match the returned results.

Suggested change
hasChanges: r.hasChanges ?? false,
contentHash: r.contentHash ?? "",
current: r.current ?? "",
proposed: r.proposed ?? "",
message: r.message ?? "",
hasChanges: r.hasChanges ?? undefined,
contentHash: r.contentHash ?? undefined,
current: r.current ?? undefined,
proposed: r.proposed ?? undefined,
message: r.message ?? undefined,

Copilot uses AI. Check for mistakes.
})),
Comment on lines 437 to 445
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don’t serialize unknown result state as concrete values.

For rows that are still computing, hasChanges is still unknown in the database. Converting that to false here makes pending results look unchanged, while summary is still derived from the raw nullable value. contentHash/current/proposed/message have the same problem: "" is no longer distinguishable from “not produced yet”. Preserve undefined until those fields are actually populated, or compute the summary from the same normalized object.

Possible fix 
     results: t.results.map((r) => ({
       id: r.id,
       status: r.status,
-      hasChanges: r.hasChanges ?? false,
-      contentHash: r.contentHash ?? "",
-      current: r.current ?? "",
-      proposed: r.proposed ?? "",
-      message: r.message ?? "",
+      hasChanges: r.hasChanges ?? undefined,
+      contentHash: r.contentHash ?? undefined,
+      current: r.current ?? undefined,
+      proposed: r.proposed ?? undefined,
+      message: r.message ?? undefined,
     })),
🤖 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 437 - 445, The
map that builds the serialized result objects (the results: t.results.map block)
currently forces nullable fields to concrete values (hasChanges => false,
contentHash/current/proposed/message => ""), which hides "not yet produced"
state for rows still computing; change the mapper to preserve undefined for
those fields (omit the null-coalescing defaults) so hasChanges stays undefined
when DB value is null, and likewise leave contentHash, current, proposed,
message as undefined until populated, or alternatively compute the derived
summary from the same normalized object so both summary and exported fields
reflect the same nullable/undefined semantics; update the results mapping code
(the mapper that returns id, status, hasChanges, contentHash, current, proposed,
message) accordingly.

}));

Expand All @@ -455,16 +455,13 @@ const getDeploymentPlan: AsyncTypedHandler<

const status =
computing > 0 ? "computing" : errored > 0 ? "failed" : "completed";
const summary =
status === "computing"
? null
: {
total: allResults.length,
changed,
unchanged,
errored,
unsupported,
};
const summary = {
total: allResults.length,
changed,
unchanged,
errored,
unsupported,
};

res.status(200).json({
id: plan.id,
Expand Down
22 changes: 15 additions & 7 deletions apps/api/src/types/openapi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1254,17 +1254,25 @@ export interface components {
unsupported?: number;
};
DeploymentPlanTarget: {
/** @description Hash of the rendered output for change detection */
contentHash?: string;
/** @description Full rendered output of the currently deployed state */
current?: string;
environmentId: string;
environmentName: string;
hasChanges?: boolean | null;
/** @description Full rendered output of the proposed version */
proposed?: string;
/** @description True if any result for this target has changes */
hasChanges: boolean;
resourceId: string;
resourceName: string;
results: components["schemas"]["DeploymentPlanTargetResult"][];
};
DeploymentPlanTargetResult: {
/** @description Hash of the rendered output for change detection */
contentHash: string;
/** @description Full rendered output of the currently deployed state */
current: string;
hasChanges: boolean;
id: string;
/** @description Agent message (e.g. error explanation or summary) */
message: string;
/** @description Full rendered output of the proposed version */
proposed: string;
/** @enum {string} */
status: "computing" | "completed" | "errored" | "unsupported";
};
Expand Down
9 changes: 9 additions & 0 deletions packages/workspace-engine-sdk/src/schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1391,6 +1391,15 @@ export interface operations {
"application/json": components["schemas"]["ErrorResponse"];
};
};
/** @description Resource not found */
404: {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["ErrorResponse"];
};
};
};
};
listReleaseTargets: {
Expand Down
Loading