Skip to content

fix(queries): clamp agent and candidate page values #335

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
ralyodio merged 4 commits into from
May 29, 2026
+56 −2
Merged

fix(queries): clamp agent and candidate page values #335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f746dc2
fix(queries): clamp invalid page values
Jorel97 May 29, 2026
c2b5008
test(queries): cover invalid page ranges
Jorel97 May 29, 2026
cbba87f
fix(queries): clamp invalid page values
Jorel97 May 29, 2026
ab78847
test(queries): cover invalid page ranges
Jorel97 May 29, 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
18 changes: 18 additions & 0 deletions src/lib/queries/agents.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,22 @@ describe("buildAgentsQuery", () => {

expect(mock.chain.range).toHaveBeenCalledWith(40, 59);
});

it("clamps negative page values to page 1", () => {
buildAgentsQuery(mock.client, { page: "-1" });

expect(mock.chain.range).toHaveBeenCalledWith(0, 19);
});

it("falls back to page 1 for non-numeric values", () => {
buildAgentsQuery(mock.client, { page: "abc" });

expect(mock.chain.range).toHaveBeenCalledWith(0, 19);
});

it("caps very large page values before calculating the range", () => {
buildAgentsQuery(mock.client, { page: "999999999" });

expect(mock.chain.range).toHaveBeenCalledWith(1999980, 1999999);
});
});
11 changes: 10 additions & 1 deletion src/lib/queries/agents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { SupabaseClient } from "@supabase/supabase-js";

const MAX_PAGE = 100_000;

export interface AgentsQueryParams {
q?: string;
sort?: string;
Expand All @@ -8,6 +10,13 @@ export interface AgentsQueryParams {
tags?: string[];
}

function parsePage(value?: string) {
const parsed = parseInt(value || "1", 10);
return Number.isFinite(parsed)
? Math.min(Math.max(parsed, 1), MAX_PAGE)
: 1;
}
Comment on lines +13 to +18
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot May 29, 2026

Choose a reason for hiding this comment

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

P2 Duplicated parsePage helper

parsePage is now defined identically in both agents.ts and candidates.ts. If the clamping logic ever needs to change (e.g. a different MAX_PAGE, or switching from parseInt to Number), it will need to be updated in both places. Extracting it to a shared utility (e.g. src/lib/queries/utils.ts) would eliminate this drift risk.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


export function buildAgentsQuery(
supabase: SupabaseClient,
params: AgentsQueryParams
Expand Down Expand Up @@ -49,7 +58,7 @@ export function buildAgentsQuery(
query = query.order("created_at", { ascending: false });
}

const pageNum = parseInt(page || "1");
const pageNum = parsePage(page);
const limit = 20;
const offset = (pageNum - 1) * limit;
query = query.range(offset, offset + limit - 1);
Expand Down
18 changes: 18 additions & 0 deletions src/lib/queries/candidates.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,22 @@ describe("buildCandidatesQuery", () => {

expect(mock.chain.range).toHaveBeenCalledWith(20, 39);
});

it("clamps negative page values to page 1", () => {
buildCandidatesQuery(mock.client, { page: "-1" });

expect(mock.chain.range).toHaveBeenCalledWith(0, 19);
});

it("falls back to page 1 for non-numeric values", () => {
buildCandidatesQuery(mock.client, { page: "abc" });

expect(mock.chain.range).toHaveBeenCalledWith(0, 19);
});

it("caps very large page values before calculating the range", () => {
buildCandidatesQuery(mock.client, { page: "999999999" });

expect(mock.chain.range).toHaveBeenCalledWith(1999980, 1999999);
});
});
11 changes: 10 additions & 1 deletion src/lib/queries/candidates.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { SupabaseClient } from "@supabase/supabase-js";

const MAX_PAGE = 100_000;

export interface CandidatesQueryParams {
q?: string;
sort?: string;
Expand All @@ -8,6 +10,13 @@ export interface CandidatesQueryParams {
tags?: string[];
}

function parsePage(value?: string) {
const parsed = parseInt(value || "1", 10);
return Number.isFinite(parsed)
? Math.min(Math.max(parsed, 1), MAX_PAGE)
: 1;
}

export function buildCandidatesQuery(
supabase: SupabaseClient,
params: CandidatesQueryParams
Expand Down Expand Up @@ -49,7 +58,7 @@ export function buildCandidatesQuery(
query = query.order("created_at", { ascending: false });
}

const pageNum = parseInt(page || "1");
const pageNum = parsePage(page);
const limit = 20;
const offset = (pageNum - 1) * limit;
query = query.range(offset, offset + limit - 1);
Expand Down
Loading