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
68 changes: 67 additions & 1 deletion apps/api/openapi/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -1849,6 +1849,72 @@
},
"type": "object"
},
"ReleaseTargetStateResponse": {
"properties": {
"currentRelease": {
"$ref": "#/components/schemas/Release"
},
"desiredRelease": {
"$ref": "#/components/schemas/Release"
},
"latestJob": {
"properties": {
"job": {
"$ref": "#/components/schemas/Job"
},
"verifications": {
"items": {
"properties": {
"createdAt": {
"format": "date-time",
"type": "string"
},
"id": {
"type": "string"
},
"jobId": {
"type": "string"
},
"message": {
"type": "string"
},
"metrics": {
"items": {
"$ref": "#/components/schemas/VerificationMetricStatus"
},
"type": "array"
},
"status": {
"description": "Computed aggregate status of this verification",
"enum": [
"passed",
"running",
"failed"
],
"type": "string"
}
},
"required": [
"id",
"jobId",
"metrics",
"createdAt",
"status"
],
"type": "object"
},
"type": "array"
}
},
"required": [
"job",
"verifications"
],
"type": "object"
}
},
"type": "object"
},
"ReleaseTargetWithState": {
"properties": {
"releaseTarget": {
Expand Down Expand Up @@ -6815,7 +6881,7 @@
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/ReleaseTargetState"
"$ref": "#/components/schemas/ReleaseTargetStateResponse"
}
}
},
Expand Down
2 changes: 1 addition & 1 deletion apps/api/openapi/paths/release-targets.jsonnet
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ local openapi = import '../lib/openapi.libsonnet';
},
],
responses: openapi.okResponse(
openapi.schemaRef('ReleaseTargetState'),
openapi.schemaRef('ReleaseTargetStateResponse'),
'The state for the release target',
) + openapi.notFoundResponse() + openapi.badRequestResponse(),
},
Expand Down
37 changes: 37 additions & 0 deletions apps/api/openapi/schemas/release.jsonnet
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,41 @@ local openapi = import '../lib/openapi.libsonnet';
state: openapi.schemaRef('ReleaseTargetState'),
},
},

ReleaseTargetStateResponse: {
type: 'object',
properties: {
desiredRelease: openapi.schemaRef('Release'),
currentRelease: openapi.schemaRef('Release'),
latestJob: {
type: 'object',
required: ['job', 'verifications'],
properties: {
job: openapi.schemaRef('Job'),
verifications: {
type: 'array',
items: {
type: 'object',
required: ['id', 'jobId', 'metrics', 'createdAt', 'status'],
properties: {
id: { type: 'string' },
jobId: { type: 'string' },
metrics: {
type: 'array',
items: openapi.schemaRef('VerificationMetricStatus'),
},
message: { type: 'string' },
createdAt: { type: 'string', format: 'date-time' },
status: {
type: 'string',
enum: ['passed', 'running', 'failed'],
description: 'Computed aggregate status of this verification',
},
},
},
},
},
},
},
},
}
168 changes: 30 additions & 138 deletions apps/api/src/routes/v1/workspaces/release-targets.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { Router } from "express";

import { and, count, desc, eq, inArray, sql } from "@ctrlplane/db";
import { and, count, desc, eq, inArray } from "@ctrlplane/db";
import { db } from "@ctrlplane/db/client";
import * as schema from "@ctrlplane/db/schema";
import { getClientFor } from "@ctrlplane/workspace-engine-sdk";

import type { AsyncTypedHandler } from "../../../types/api.js";
import { ApiError, asyncHandler } from "../../../types/api.js";
Expand Down Expand Up @@ -101,16 +102,6 @@ const buildReleaseResponse = async (
};
};

const getMetadataForJob = async (jobId: string) => {
const rows = await db
.select()
.from(schema.jobMetadata)
.where(eq(schema.jobMetadata.jobId, jobId));
const metadata: Record<string, string> = {};
for (const row of rows) metadata[row.key] = row.value;
return metadata;
};

const getMetadataForJobs = async (jobIds: string[]) => {
if (jobIds.length === 0) return new Map<string, Record<string, string>>();
const rows = await db
Expand All @@ -126,106 +117,6 @@ const getMetadataForJobs = async (jobIds: string[]) => {
return map;
};

const computeReleaseTargetState = async (
resourceId: string,
environmentId: string,
deploymentId: string,
) => {
const [desiredReleaseRow, latestSuccessfulJob, latestJob] = await Promise.all(
[
db
.select({
desiredReleaseId: schema.releaseTargetDesiredRelease.desiredReleaseId,
})
.from(schema.releaseTargetDesiredRelease)
.where(
and(
eq(schema.releaseTargetDesiredRelease.resourceId, resourceId),
eq(schema.releaseTargetDesiredRelease.environmentId, environmentId),
eq(schema.releaseTargetDesiredRelease.deploymentId, deploymentId),
),
)
.then((rows) => rows[0]),
db
.select({ job: schema.job, releaseId: schema.releaseJob.releaseId })
.from(schema.job)
.innerJoin(
schema.releaseJob,
eq(schema.releaseJob.jobId, schema.job.id),
)
.innerJoin(
schema.release,
eq(schema.release.id, schema.releaseJob.releaseId),
)
.where(
and(
eq(schema.release.resourceId, resourceId),
eq(schema.release.environmentId, environmentId),
eq(schema.release.deploymentId, deploymentId),
eq(schema.job.status, "successful"),
sql`${schema.job.completedAt} IS NOT NULL`,
),
)
.orderBy(desc(schema.job.completedAt))
.limit(1)
.then((rows) => rows[0]),
db
.select({ job: schema.job, releaseId: schema.releaseJob.releaseId })
.from(schema.job)
.innerJoin(
schema.releaseJob,
eq(schema.releaseJob.jobId, schema.job.id),
)
.innerJoin(
schema.release,
eq(schema.release.id, schema.releaseJob.releaseId),
)
.where(
and(
eq(schema.release.resourceId, resourceId),
eq(schema.release.environmentId, environmentId),
eq(schema.release.deploymentId, deploymentId),
),
)
.orderBy(desc(schema.job.createdAt))
.limit(1)
.then((rows) => rows[0]),
],
);

const state: Record<string, unknown> = {};

if (desiredReleaseRow?.desiredReleaseId != null) {
const release = await db
.select()
.from(schema.release)
.where(eq(schema.release.id, desiredReleaseRow.desiredReleaseId))
.then((rows) => rows[0]);
if (release != null)
state.desiredRelease = await buildReleaseResponse(release);
}

if (latestSuccessfulJob != null) {
const release = await db
.select()
.from(schema.release)
.where(eq(schema.release.id, latestSuccessfulJob.releaseId))
.then((rows) => rows[0]);
if (release != null)
state.currentRelease = await buildReleaseResponse(release);
}

if (latestJob != null) {
const metadata = await getMetadataForJob(latestJob.job.id);
state.latestJob = {
job: toJobResponseForState(latestJob.job, latestJob.releaseId, metadata),
verifications: [],
};
}

return state;
};

const getReleaseTargetJobs: AsyncTypedHandler<
"/v1/workspaces/{workspaceId}/release-targets/{releaseTargetKey}/jobs",
"get"
Expand Down Expand Up @@ -327,37 +218,29 @@ const getReleaseTargetState: AsyncTypedHandler<
"/v1/workspaces/{workspaceId}/release-targets/{releaseTargetKey}/state",
"get"
> = async (req, res) => {
const { releaseTargetKey } = req.params;
const { resourceId, environmentId, deploymentId } =
parseReleaseTargetKey(releaseTargetKey);
const { workspaceId, releaseTargetKey } = req.params;

const rtExists = await db
.select({ id: schema.releaseTargetDesiredRelease.id })
.from(schema.releaseTargetDesiredRelease)
.where(
and(
eq(schema.releaseTargetDesiredRelease.resourceId, resourceId),
eq(schema.releaseTargetDesiredRelease.environmentId, environmentId),
eq(schema.releaseTargetDesiredRelease.deploymentId, deploymentId),
),
)
.then((rows) => rows[0]);

if (rtExists == null) throw new ApiError("Release target not found", 404);

const state = await computeReleaseTargetState(
resourceId,
environmentId,
deploymentId,
const { data, error, response } = await getClientFor(workspaceId).GET(
"/v1/workspaces/{workspaceId}/release-targets/{releaseTargetKey}/state",
{
params: { path: { workspaceId, releaseTargetKey } },
},
Comment on lines +221 to +227
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

bypassCache is silently ignored here.

The public handler still accepts bypassCache, but this proxy request never forwards it or rejects it. Clients can ask for a cache bypass and still get the default behavior with no signal. Please either pass the flag upstream or remove it from the public contract.

Comment on lines +223 to +227
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

getClientFor(workspaceId) is unsafe with the current SDK cache.

packages/workspace-engine-sdk/src/index.ts caches a single client and only sets x-workspace-id on first construction. After the first request, both this call and the loop below can proxy later workspaces with a stale header, causing cross-workspace state lookups or false failures. Key the cache by workspaceId or attach the header per request before routing state reads through this client.

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

In `@apps/api/src/routes/v1/workspaces/release-targets.ts` around lines 223 - 227,
The SDK's getClientFor function currently returns a single cached client that
keeps the initial x-workspace-id header, causing cross-workspace requests (seen
where getClientFor(workspaceId).GET is called to fetch
/v1/workspaces/{workspaceId}/release-targets/{releaseTargetKey}/state and in the
loop below). Fix by making the client cache keyed by workspaceId (so each
workspace gets its own client) or by ensuring x-workspace-id is set per-request
before calling GET; update the client creation logic in
packages/workspace-engine-sdk/src/index.ts to include workspaceId in the cache
key or add a method to attach/override the x-workspace-id header on each request
and use that here (refer to getClientFor and the GET call above).

);

res.status(200).json(state);
if (error != null)
throw new ApiError(
error.error ?? "Failed to get release target state",
response.status >= 400 && response.status < 500 ? response.status : 502,
);

res.status(200).json(data);
};

const getReleaseTargetStates: AsyncTypedHandler<
"/v1/workspaces/{workspaceId}/release-targets/state",
"post"
> = async (req, res) => {
const { workspaceId } = req.params;
const { limit: rawLimit, offset: rawOffset } = req.query;
const { deploymentId, environmentId } = req.body;

Expand Down Expand Up @@ -385,18 +268,27 @@ const getReleaseTargetStates: AsyncTypedHandler<

const items = await Promise.all(
releaseTargets.map(async (rt) => {
const state = await computeReleaseTargetState(
rt.resourceId,
rt.environmentId,
rt.deploymentId,
const releaseTargetKey = `${rt.resourceId}-${rt.environmentId}-${rt.deploymentId}`;
const { data, error, response } = await getClientFor(workspaceId).GET(
"/v1/workspaces/{workspaceId}/release-targets/{releaseTargetKey}/state",
{ params: { path: { workspaceId, releaseTargetKey } } },
);

if (error != null)
throw new ApiError(
error.error ?? "Failed to get release target state",
response.status >= 400 && response.status < 500
? response.status
: 502,
);

return {
releaseTarget: {
resourceId: rt.resourceId,
environmentId: rt.environmentId,
deploymentId: rt.deploymentId,
},
state,
state: data,
};
}),
);
Comment on lines 269 to 294
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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check if workspace-engine has or could have a batch endpoint for release target states

# Search for batch-related endpoints in workspace-engine
rg -n 'release.?target' --glob '*.go' --glob '*.jsonnet' apps/workspace-engine/ | grep -i 'batch\|bulk\|multi' || echo "No batch endpoint found"

# Check the current endpoint implementation
rg -nA 5 'GetReleaseTargetState' apps/workspace-engine/

Repository: ctrlplanedev/ctrlplane

Length of output: 3184


🏁 Script executed:

# First, let's verify the actual code at those lines
head -300 apps/api/src/routes/v1/workspaces/release-targets.ts | tail -60

Repository: ctrlplanedev/ctrlplane

Length of output: 1817


🏁 Script executed:

# Check the SDK client initialization and timeout configuration
rg -n 'getClientFor|timeout|HttpClient' apps/api/src/routes/v1/workspaces/release-targets.ts -B 3 -A 3

Repository: ctrlplanedev/ctrlplane

Length of output: 1162


🏁 Script executed:

# Search more broadly for batch/bulk endpoints in the API
rg -n 'batch|bulk|multi' apps/workspace-engine/ --glob '*.go' | grep -i 'release.*target\|endpoint\|handler' | head -20

Repository: ctrlplanedev/ctrlplane

Length of output: 462


🏁 Script executed:

# Check if there's a similar pattern elsewhere in the codebase that might indicate this is intentional
rg -n 'Promise.all.*map.*async.*GET' apps/api/src/routes/ -A 5 | head -40

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Check the workspace-engine SDK client configuration for timeouts
find apps -name '*workspace-engine-sdk*' -o -name '*sdk*' | grep -i workspace | head -20

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Look for the SDK client source/types
rg -l 'workspace-engine-sdk' apps/api/src --glob '*.ts' | head -5

Repository: ctrlplanedev/ctrlplane

Length of output: 188


🏁 Script executed:

# Check if there's any timeout or concurrency configuration in the SDK
rg -n 'timeout|Timeout|concurrent|Concurrent' apps/api/src/routes/v1/workspaces/release-targets.ts -B 2 -A 2

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Check the workspace-engine API definition to understand its capacity/design
find apps/workspace-engine -name '*.yaml' -o -name '*.yml' -o -name '*.json' | xargs grep -l 'release.*target\|openapi' 2>/dev/null | head -5

Repository: ctrlplanedev/ctrlplane

Length of output: 144


🏁 Script executed:

# Search for any load testing, performance notes, or architectural decisions about the GetReleaseTargetState endpoint
rg -n 'GetReleaseTargetState|release.*target.*state' apps/workspace-engine/svc/http/server/openapi/release_targets/ -A 10 | head -50

Repository: ctrlplanedev/ctrlplane

Length of output: 1179


🏁 Script executed:

# Look at the SDK package to understand timeout behavior
find . -path '*/workspace-engine-sdk*' -type f | head -20

Repository: ctrlplanedev/ctrlplane

Length of output: 389


🏁 Script executed:

# Check workspace-engine-sdk package.json or exports
rg -l 'workspace-engine-sdk' packages/ --type json | head -5

Repository: ctrlplanedev/ctrlplane

Length of output: 135


🏁 Script executed:

# Look for the actual SDK source
fd -e ts -e tsx 'workspace-engine-sdk' packages/ | head -10

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Check what packages/api depends on or exports for workspace-engine-sdk
cat packages/api/package.json | grep -A 5 -B 5 workspace-engine

Repository: ctrlplanedev/ctrlplane

Length of output: 123


🏁 Script executed:

# Search for how getClientFor is implemented
rg -n 'export.*getClientFor\|function getClientFor' packages/ apps/ -A 10 | head -60

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Check the SDK source code
cat packages/workspace-engine-sdk/src/index.ts

Repository: ctrlplanedev/ctrlplane

Length of output: 978


🏁 Script executed:

# Check the SDK config
cat packages/workspace-engine-sdk/src/config.ts

Repository: ctrlplanedev/ctrlplane

Length of output: 344


🏁 Script executed:

# Look for the actual client implementation and any timeout settings
rg -n 'timeout|Timeout' packages/workspace-engine-sdk/src/ -A 2 -B 2

Repository: ctrlplanedev/ctrlplane

Length of output: 1371


🏁 Script executed:

# Check if there are any architectural decisions or comments about the N+1 pattern
rg -n 'N\+1|n\+1|batch' apps/api/src/routes/v1/workspaces/release-targets.ts -B 5 -A 5

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Check if there's pagination or limits already in place
rg -n 'limit.*50\|limitVal' apps/api/src/routes/v1/workspaces/release-targets.ts -B 2 -A 2

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Check if there's any documentation about workspace-engine capacity or performance
rg -n 'capacity|concurrent|load|perform' apps/workspace-engine/README.md apps/workspace-engine/docs/ 2>/dev/null | head -20

Repository: ctrlplanedev/ctrlplane

Length of output: 326


🏁 Script executed:

# Verify if this endpoint is frequently called and if there are any issues filed about performance
git log --oneline -n 50 -- apps/api/src/routes/v1/workspaces/release-targets.ts 2>/dev/null | head -20

Repository: ctrlplanedev/ctrlplane

Length of output: 98


🏁 Script executed:

# Check if timeout is being set anywhere in the API routes
rg -n 'timeout' apps/api/src/routes/v1/workspaces/ --glob '*.ts' -B 2 -A 2

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# See what options are available in ClientOptions for openapi-fetch
rg -n 'ClientOptions|interface.*Options' packages/workspace-engine-sdk/src/ -A 5

Repository: ctrlplanedev/ctrlplane

Length of output: 1036


🏁 Script executed:

# Check openapi-fetch documentation or types in node_modules if available
rg -n 'ClientOptions|export.*interface' packages/workspace-engine-sdk/node_modules/openapi-fetch 2>/dev/null | head -20

Repository: ctrlplanedev/ctrlplane

Length of output: 48


🏁 Script executed:

# Look for any timeout handling in the fetch wrapper
cat packages/workspace-engine-sdk/src/index.ts | grep -A 10 "fetch:"

Repository: ctrlplanedev/ctrlplane

Length of output: 355


🏁 Script executed:

# Check if there's any middleware or request interceptor that handles timeouts
rg -n 'AbortController|timeout|abort' apps/api/src/ --glob '*.ts' | grep -i 'client\|fetch\|request' | head -20

Repository: ctrlplanedev/ctrlplane

Length of output: 175


🏁 Script executed:

# Verify the actual endpoint route handler in workspace-engine to understand its performance characteristics
cat apps/workspace-engine/svc/http/server/openapi/release_targets/server.go | head -100

Repository: ctrlplanedev/ctrlplane

Length of output: 2795


N+1 HTTP calls pattern creates unnecessary load on workspace-engine.

This implementation makes one HTTP call to workspace-engine for each release target (up to 50 by default). With Promise.all, all calls run in parallel, and since each GetReleaseTargetState call triggers multiple database queries (desired release, current release, latest job, and verifications), this pattern puts significant load on workspace-engine under concurrent traffic.

Additionally, if any single call fails, Promise.all rejects immediately, discarding successful results from other targets.

Consider:

  • Adding a batch endpoint to workspace-engine that accepts multiple release target keys
  • Using Promise.allSettled to return partial results when individual calls fail
  • Adding concurrency limits (e.g., p-limit) to avoid overwhelming the service
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/api/src/routes/v1/workspaces/release-targets.ts` around lines 269 - 292,
This code performs per-target GETs via releaseTargets.map + Promise.all calling
getClientFor(...).GET which causes N+1 load and brittle all-or-nothing failure;
change this to use a batched request to workspace-engine (e.g., add/consume an
endpoint that accepts an array of releaseTargetKeys and returns states) by
constructing the list of keys and calling a single batch endpoint instead of
per-target GETs, and if a batch endpoint is not yet available: replace
Promise.all with Promise.allSettled or add concurrency limiting (p-limit) around
the per-target getClientFor(...).GET calls to avoid flooding the service; ensure
you still map results back to releaseTarget objects and convert errors into
ApiError only when appropriate (aggregate partial failures or surface per-item
errors) so successful states are returned rather than dropped.

Expand Down
22 changes: 21 additions & 1 deletion apps/api/src/types/openapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1621,6 +1621,26 @@ export interface components {
desiredRelease?: components["schemas"]["Release"];
latestJob?: components["schemas"]["Job"];
};
ReleaseTargetStateResponse: {
currentRelease?: components["schemas"]["Release"];
desiredRelease?: components["schemas"]["Release"];
latestJob?: {
job: components["schemas"]["Job"];
verifications: {
/** Format: date-time */
createdAt: string;
id: string;
jobId: string;
message?: string;
metrics: components["schemas"]["VerificationMetricStatus"][];
/**
* @description Computed aggregate status of this verification
* @enum {string}
*/
status: "passed" | "running" | "failed";
}[];
};
};
Comment on lines +1624 to +1643
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

ReleaseTargetStateResponse is still detached from the public API.

This type is added here, but getReleaseTargetState still returns ReleaseTargetState at Line 4474 and ReleaseTargetWithState.state still points at the old type at Line 1646. Generated API clients therefore still cannot read latestJob.verifications[].status. Please wire the new schema into the source OpenAPI response and regenerate.

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

In `@apps/api/src/types/openapi.ts` around lines 1624 - 1643, The new schema
ReleaseTargetStateResponse was added but not wired into the public API: update
the OpenAPI response for getReleaseTargetState so the operation's response
schema uses ReleaseTargetStateResponse (replace the old ReleaseTargetState), and
change ReleaseTargetWithState.state to reference ReleaseTargetStateResponse so
latestJob.verifications[].status is exposed; after updating the OpenAPI source
definitions, regenerate the API client and types so clients can read
latestJob.verifications[].status.

ReleaseTargetWithState: {
releaseTarget: components["schemas"]["ReleaseTarget"];
state: components["schemas"]["ReleaseTargetState"];
Expand Down Expand Up @@ -4451,7 +4471,7 @@ export interface operations {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["ReleaseTargetState"];
"application/json": components["schemas"]["ReleaseTargetStateResponse"];
};
};
/** @description Invalid request */
Expand Down
Loading
Loading