Skip to content

fix: version list endpoint respects limit param #1119

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
May 11, 2026
Merged

fix: version list endpoint respects limit param #1119

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
27af3b1
fix: version list endpoint respects limit param
adityachoudhari26 May 11, 2026
783916e
cleanup
adityachoudhari26 May 11, 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
91 changes: 72 additions & 19 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,11 +1,11 @@
import type { AsyncTypedHandler } from "@/types/api.js";
import type { Tx } from "@ctrlplane/db";
import { ApiError, asyncHandler } from "@/types/api.js";
import { evaluate } from "cel-js";
import { Router } from "express";
import { v4 as uuidv4 } from "uuid";
import { z } from "zod";

import type { Tx } from "@ctrlplane/db";
import { and, asc, count, desc, eq, inArray, takeFirst } from "@ctrlplane/db";
import { db } from "@ctrlplane/db/client";
import {
Expand Down Expand Up @@ -316,22 +316,78 @@ function filterDeploymentVersions(
});
}

const CEL_BATCH_SIZE = 500;
const CEL_MAX_SCAN = 100_000;

Comment on lines +319 to +321
async function listDeploymentVersionsMatchingCel(
deploymentId: string,
cel: string,
order: "asc" | "desc",
offset: number,
limit: number,
) {
const orderBy =
order === "asc"
? [
asc(schema.deploymentVersion.createdAt),
asc(schema.deploymentVersion.tag),
]
: [
desc(schema.deploymentVersion.createdAt),
desc(schema.deploymentVersion.tag),
];

Comment on lines +329 to +339
const pageEnd = offset + limit;
const items: (typeof schema.deploymentVersion.$inferSelect)[] = [];
let scanned = 0;
let matched = 0;

while (scanned < CEL_MAX_SCAN) {
const batch = await db
.select()
.from(schema.deploymentVersion)
.where(eq(schema.deploymentVersion.deploymentId, deploymentId))
.orderBy(...orderBy)
.limit(CEL_BATCH_SIZE)
.offset(scanned);
if (batch.length === 0) break;

for (const version of filterDeploymentVersions(batch, cel)) {
if (matched >= offset && matched < pageEnd) items.push(version);
matched++;
}

scanned += batch.length;
if (batch.length < CEL_BATCH_SIZE) break;
}

return { items, total: matched };
}
Comment on lines +345 to +365
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

CEL_MAX_SCAN can silently return partial total and incomplete pages.

When the scan cap is reached, the function returns total: matched as if complete, even though more matching rows may exist beyond the cap. That breaks total correctness and can hide valid results at higher offsets. Please surface truncation explicitly (or adopt a continuation strategy that preserves correctness).

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/src/routes/v1/workspaces/deployments.ts` around lines 339 - 359, The
loop that pages through deploymentVersion using CEL_MAX_SCAN can stop early and
return an incorrect total (it currently returns { items, total: matched
})—update the function that iterates over schema.deploymentVersion (the while
loop using CEL_MAX_SCAN, CEL_BATCH_SIZE, scanned, matched and
filterDeploymentVersions) to detect when the scan cap was hit (scanned >=
CEL_MAX_SCAN or batch.length === CEL_BATCH_SIZE on final iteration) and surface
truncation instead of returning a potentially wrong total: either return a
nullable/unknown total (e.g., total: null) or add a truncated flag (e.g.,
truncated: true) alongside total so callers know the count is incomplete; ensure
the change is applied where the function currently returns { items, total:
matched } and document the new contract for consumers.


const listDeploymentVersions: AsyncTypedHandler<
"/v1/workspaces/{workspaceId}/deployments/{deploymentId}/versions",
"get"
> = async (req, res) => {
const { deploymentId } = req.params;
const { workspaceId, deploymentId } = req.params;
const limit = req.query.limit ?? 50;
const offset = req.query.offset ?? 0;
const order = req.query.order ?? "desc";
const { cel } = req.query;

const orderBy =
order === "asc"
? asc(schema.deploymentVersion.createdAt)
: desc(schema.deploymentVersion.createdAt);
await assertDeploymentExistsInWorkspace(workspaceId, deploymentId);

if (cel == null) {
const orderBy =
order === "asc"
? [
asc(schema.deploymentVersion.createdAt),
asc(schema.deploymentVersion.tag),
]
: [
desc(schema.deploymentVersion.createdAt),
desc(schema.deploymentVersion.tag),
];

const { total } = await db
.select({ total: count() })
.from(schema.deploymentVersion)
Expand All @@ -342,7 +398,7 @@ const listDeploymentVersions: AsyncTypedHandler<
.select()
.from(schema.deploymentVersion)
.where(eq(schema.deploymentVersion.deploymentId, deploymentId))
.orderBy(orderBy)
.orderBy(...orderBy)
.limit(limit)
.offset(offset);

Expand All @@ -358,20 +414,17 @@ const listDeploymentVersions: AsyncTypedHandler<
if (!validResourceSelector(cel))
throw new ApiError("Invalid CEL expression", 400);

// CEL is evaluated in-memory, so cap the candidate set to bound cost.
// Filtering applies to the 1000 most-recent (or oldest, for asc) versions.
const candidates = await db
.select()
.from(schema.deploymentVersion)
.where(eq(schema.deploymentVersion.deploymentId, deploymentId))
.orderBy(orderBy)
.limit(1000);

const filtered = filterDeploymentVersions(candidates, cel);
const { items, total } = await listDeploymentVersionsMatchingCel(
deploymentId,
cel,
order,
offset,
limit,
);

res.status(200).json({
items: filtered.slice(offset, offset + limit).map(formatDeploymentVersion),
total: filtered.length,
items: items.map(formatDeploymentVersion),
total,
limit,
offset,
});
Expand Down
2 changes: 2 additions & 0 deletions e2e/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
"test:policies": "pnpm exec playwright test tests/api/policies/",
"test:deployments": "pnpm exec playwright test --project=api-tests tests/api/deployments.spec.ts",
"test:deployment-version-deps": "pnpm exec playwright test --project=api-tests tests/api/deployment-version-dependencies.spec.ts",
"test:deployment-version-list": "pnpm exec playwright test --project=api-tests tests/api/deployment-version-list.spec.ts",
"test:deployment-version-list:heavy": "RUN_HEAVY_TESTS=1 pnpm exec playwright test --project=api-tests tests/api/deployment-version-list.spec.ts",
"test:release-targets": "pnpm exec playwright test tests/api/release-targets.spec.ts",
"test:yaml": "pnpm exec playwright test tests/api/yaml-import.spec.ts",
"test:yaml-prefixed": "pnpm exec playwright test tests/api/random-prefix-yaml.spec.ts",
Expand Down
Loading
Loading