diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index 59b5136ab2..149b769b2c 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -11061,11 +11061,21 @@ "remediation" ] } + }, + "offset": { + "type": "integer", + "minimum": 0 + }, + "total": { + "type": "integer", + "minimum": 0 } }, "required": [ "generatedAt", "limit", + "offset", + "total", "hasMore", "filters", "items" @@ -17892,10 +17902,20 @@ "example": "50" }, "required": false, - "description": "Maximum rows to return, clamped from 1 to 100.", + "description": "Maximum rows to return per page, clamped from 1 to 100.", "name": "limit", "in": "query" }, + { + "schema": { + "type": "string", + "example": "0" + }, + "required": false, + "description": "Zero-based row offset for pagination. Advance by `limit` (or the previous page's item count) and append; do not grow `limit` to page.", + "name": "offset", + "in": "query" + }, { "schema": { "type": "string", @@ -17909,6 +17929,7 @@ { "schema": { "type": "string", + "example": "not_official_gittensor_miner", "enum": [ "surface_off", "missing_author", @@ -17917,8 +17938,7 @@ "maintainer_author", "miner_detection_unavailable", "not_official_gittensor_miner" - ], - "example": "not_official_gittensor_miner" + ] }, "required": false, "description": "Optional PR skip reason filter.", diff --git a/apps/loopover-ui/src/components/site/audit-feed-model.ts b/apps/loopover-ui/src/components/site/audit-feed-model.ts index 065ab331b6..340e633eb9 100644 --- a/apps/loopover-ui/src/components/site/audit-feed-model.ts +++ b/apps/loopover-ui/src/components/site/audit-feed-model.ts @@ -17,6 +17,8 @@ export type SkippedPrAuditItem = { export type SkippedPrAuditExport = { generatedAt: string; limit: number; + offset: number; + total: number; hasMore: boolean; filters: { repoFullName: string | null; @@ -36,14 +38,21 @@ export const SKIP_REASON_OPTIONS: Array<{ value: "" | SkippedPrAuditReason; labe { value: "not_official_gittensor_miner", label: "Not official Gittensor miner" }, ]; +/** Page size for each skipped-PR audit request. Load more advances `offset` by this amount (#7438). */ +export const AUDIT_FEED_PAGE_SIZE = 50; +/** Cap on how many rows the UI will accumulate client-side before asking the user to narrow filters. */ +export const AUDIT_FEED_MAX_ITEMS = 100; + export function buildSkippedPrAuditPath(options: { limit: number; + offset?: number; repoFullName?: string; reason?: SkippedPrAuditReason; since?: string; }): string { const params = new URLSearchParams(); params.set("limit", String(options.limit)); + params.set("offset", String(Math.max(0, options.offset ?? 0))); if (options.repoFullName?.trim()) params.set("repoFullName", options.repoFullName.trim()); if (options.reason) params.set("reason", options.reason); if (options.since?.trim()) params.set("since", options.since.trim()); @@ -83,6 +92,8 @@ export function normalizeSkippedPrAuditExport(data: unknown): SkippedPrAuditExpo return { generatedAt: raw.generatedAt, limit: typeof raw.limit === "number" ? raw.limit : items.length, + offset: typeof raw.offset === "number" ? raw.offset : 0, + total: typeof raw.total === "number" ? raw.total : items.length, hasMore: Boolean(raw.hasMore), filters: { repoFullName: @@ -97,6 +108,18 @@ export function normalizeSkippedPrAuditExport(data: unknown): SkippedPrAuditExpo }; } +/** Append a newly fetched page onto the already-rendered list without re-deriving prior rows (#7438). */ +export function appendSkippedPrAuditPage( + current: SkippedPrAuditExport | null, + nextPage: SkippedPrAuditExport, +): SkippedPrAuditExport { + if (!current || nextPage.offset === 0) return nextPage; + return { + ...nextPage, + items: [...current.items, ...nextPage.items], + }; +} + export function formatSkipReason(reason: string): string { const match = SKIP_REASON_OPTIONS.find((option) => option.value === reason); if (match && match.value) return match.label; diff --git a/apps/loopover-ui/src/components/site/audit-feed.test.tsx b/apps/loopover-ui/src/components/site/audit-feed.test.tsx index 3f0fd03697..04c6df0d69 100644 --- a/apps/loopover-ui/src/components/site/audit-feed.test.tsx +++ b/apps/loopover-ui/src/components/site/audit-feed.test.tsx @@ -6,6 +6,7 @@ vi.mock("@/lib/api/request", () => ({ apiFetch: (...args: unknown[]) => apiFetch vi.mock("@/lib/api/origin", () => ({ getApiOrigin: () => "https://api.test" })); import { + appendSkippedPrAuditPage, buildSkippedPrAuditPath, formatSkipReason, normalizeSinceInput, @@ -17,6 +18,8 @@ import { AuditFeed } from "@/components/site/audit-feed"; const SAMPLE: { generatedAt: string; limit: number; + offset: number; + total: number; hasMore: boolean; filters: { repoFullName: null; reason: null; since: null }; items: Array<{ @@ -29,6 +32,8 @@ const SAMPLE: { } = { generatedAt: "2026-05-28T00:00:05.000Z", limit: 50, + offset: 0, + total: 1, hasMore: false, filters: { repoFullName: null, reason: null, since: null }, items: [ @@ -43,17 +48,23 @@ const SAMPLE: { }; describe("audit feed helpers", () => { - it("builds query paths for skipped PR audit filters", () => { - expect(buildSkippedPrAuditPath({ limit: 25 })).toBe("/v1/app/skipped-pr-audit?limit=25"); + it("builds query paths for skipped PR audit filters including offset (#7438)", () => { + expect(buildSkippedPrAuditPath({ limit: 25 })).toBe( + "/v1/app/skipped-pr-audit?limit=25&offset=0", + ); expect( buildSkippedPrAuditPath({ limit: 50, + offset: 50, repoFullName: "repo-owner/owned-repo", reason: "bot_author", since: "2026-05-28T00:00:00.000Z", }), ).toBe( - "/v1/app/skipped-pr-audit?limit=50&repoFullName=repo-owner%2Fowned-repo&reason=bot_author&since=2026-05-28T00%3A00%3A00.000Z", + "/v1/app/skipped-pr-audit?limit=50&offset=50&repoFullName=repo-owner%2Fowned-repo&reason=bot_author&since=2026-05-28T00%3A00%3A00.000Z", + ); + expect(buildSkippedPrAuditPath({ limit: 10, offset: -3 })).toBe( + "/v1/app/skipped-pr-audit?limit=10&offset=0", ); }); @@ -78,6 +89,13 @@ describe("audit feed helpers", () => { expect(normalizeSkippedPrAuditExport({ ...SAMPLE, items: [] })).toMatchObject({ items: [] }); expect(normalizeSkippedPrAuditExport(null)).toBeNull(); expect(normalizeSkippedPrAuditExport({ generatedAt: "2026-05-28T00:00:05.000Z" })).toBeNull(); + expect( + normalizeSkippedPrAuditExport({ + generatedAt: SAMPLE.generatedAt, + hasMore: true, + items: SAMPLE.items, + }), + ).toMatchObject({ offset: 0, total: 1, limit: 1 }); expect( normalizeSkippedPrAuditExport({ ...SAMPLE, @@ -95,6 +113,35 @@ describe("audit feed helpers", () => { }), ).toMatchObject({ items: [{ repoFullName: "x/y", pullNumber: 1 }] }); }); + + it("REGRESSION (#7438): appends the next offset page instead of replacing already-visible rows", () => { + const page1 = { + ...SAMPLE, + hasMore: true, + total: 2, + items: [SAMPLE.items[0]!], + }; + const page2 = { + ...SAMPLE, + offset: 50, + hasMore: false, + total: 2, + items: [ + { + repoFullName: "repo-owner/owned-repo", + pullNumber: 5, + reason: "bot_author", + timestamp: "2026-05-28T00:00:03.000Z", + remediation: "Ignore bot authors in repository settings.", + }, + ], + }; + expect(appendSkippedPrAuditPage(null, page1)).toEqual(page1); + expect(appendSkippedPrAuditPage(page1, { ...page1, offset: 0, items: [] }).items).toEqual([]); + expect(appendSkippedPrAuditPage(page1, page2).items.map((item) => item.pullNumber)).toEqual([ + 6, 5, + ]); + }); }); describe("AuditFeed", () => { @@ -111,7 +158,7 @@ describe("AuditFeed", () => { "https://github.com/repo-owner/owned-repo/pull/6", ); expect(apiFetch).toHaveBeenCalledWith( - "https://api.test/v1/app/skipped-pr-audit?limit=50", + "https://api.test/v1/app/skipped-pr-audit?limit=50&offset=0", expect.objectContaining({ credentials: "include" }), ); }); @@ -131,7 +178,7 @@ describe("AuditFeed", () => { }); it("shows an empty state when the audit export has no items", async () => { - apiFetch.mockResolvedValue({ ok: true, data: { ...SAMPLE, items: [] } }); + apiFetch.mockResolvedValue({ ok: true, data: { ...SAMPLE, items: [], total: 0 } }); render(); expect(await screen.findByText("No skipped PR events")).toBeTruthy(); }); @@ -160,6 +207,7 @@ describe("AuditFeed", () => { expect.any(Object), ), ); + expect(apiFetch.mock.calls.some(([url]) => String(url).includes("offset=0"))).toBe(true); }); it("ignores invalid since values when applying filters", async () => { @@ -194,24 +242,45 @@ describe("AuditFeed", () => { expect(apiFetch).not.toHaveBeenCalled(); }); - it("loads more rows until the maximum page size", async () => { - apiFetch.mockResolvedValue({ ok: true, data: { ...SAMPLE, hasMore: true } }); + it("REGRESSION (#7438): Load more advances offset and appends rows instead of growing limit", async () => { + const firstPage = { + ...SAMPLE, + hasMore: true, + total: 2, + items: [SAMPLE.items[0]!], + }; + const secondPage = { + ...SAMPLE, + offset: 50, + hasMore: false, + total: 2, + items: [ + { + repoFullName: "repo-owner/owned-repo", + pullNumber: 5, + reason: "bot_author", + timestamp: "2026-05-28T00:00:03.000Z", + remediation: "Ignore bot authors in repository settings.", + }, + ], + }; + apiFetch.mockResolvedValueOnce({ ok: true, data: firstPage }); render(); - await screen.findByText("repo-owner/owned-repo"); + expect(await screen.findByText("#6")).toBeTruthy(); apiFetch.mockClear(); - apiFetch.mockResolvedValue({ ok: true, data: { ...SAMPLE, hasMore: true, limit: 100 } }); + apiFetch.mockResolvedValueOnce({ ok: true, data: secondPage }); fireEvent.click(screen.getByRole("button", { name: /load more/i })); await waitFor(() => expect(apiFetch).toHaveBeenCalledWith( - "https://api.test/v1/app/skipped-pr-audit?limit=100", + "https://api.test/v1/app/skipped-pr-audit?limit=50&offset=50", expect.any(Object), ), ); - - expect(screen.getByText(/maximum page size \(100\)/i)).toBeTruthy(); - expect(screen.queryByRole("button", { name: /load more/i })).toBeNull(); + expect(await screen.findByText("#5")).toBeTruthy(); + expect(screen.getByText("#6")).toBeTruthy(); + expect(apiFetch.mock.calls.some(([url]) => String(url).includes("limit=100"))).toBe(false); }); it("shows an error state when the audit response is malformed", async () => { diff --git a/apps/loopover-ui/src/components/site/audit-feed.tsx b/apps/loopover-ui/src/components/site/audit-feed.tsx index 81842c1074..c8fe508058 100644 --- a/apps/loopover-ui/src/components/site/audit-feed.tsx +++ b/apps/loopover-ui/src/components/site/audit-feed.tsx @@ -1,7 +1,10 @@ -import { useCallback, useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { ExternalLink } from "lucide-react"; import { + appendSkippedPrAuditPage, + AUDIT_FEED_MAX_ITEMS, + AUDIT_FEED_PAGE_SIZE, buildSkippedPrAuditPath, formatAuditTimestamp, formatSkipReason, @@ -28,9 +31,6 @@ import { apiFetch } from "@/lib/api/request"; const fieldClass = "mt-1 w-full rounded-token border border-border bg-background/40 px-3 py-2 text-token-sm text-foreground focus-ring"; -const DEFAULT_LIMIT = 50; -const MAX_LIMIT = 100; - type AuditFeedProps = { enabled?: boolean; }; @@ -41,20 +41,23 @@ export function AuditFeed({ enabled = true }: AuditFeedProps) { const [repoFullName, setRepoFullName] = useState(""); const [sinceInput, setSinceInput] = useState(""); const [sinceIso, setSinceIso] = useState(""); - const [limit, setLimit] = useState(DEFAULT_LIMIT); + // #7438: page by advancing offset and appending — never grow `limit` and re-fetch from the start. + const [offset, setOffset] = useState(0); const [status, setStatus] = useState<"loading" | "ready" | "error">("loading"); const [error, setError] = useState(null); const [data, setData] = useState(null); + const loadGeneration = useRef(0); const queryPath = useMemo( () => buildSkippedPrAuditPath({ - limit, + limit: AUDIT_FEED_PAGE_SIZE, + offset, repoFullName: repoFullName || undefined, reason: reason || undefined, since: sinceIso || undefined, }), - [limit, reason, repoFullName, sinceIso], + [offset, reason, repoFullName, sinceIso], ); const load = useCallback(async () => { @@ -64,6 +67,7 @@ export function AuditFeed({ enabled = true }: AuditFeedProps) { setData(null); return; } + const generation = ++loadGeneration.current; setStatus("loading"); setError(null); const origin = getApiOrigin().replace(/\/$/, ""); @@ -72,31 +76,34 @@ export function AuditFeed({ enabled = true }: AuditFeedProps) { credentials: "include", headers: { Accept: "application/json" }, }); + if (generation !== loadGeneration.current) return; if (result.ok) { const normalized = normalizeSkippedPrAuditExport(result.data); if (!normalized) { - setData(null); + if (offset === 0) setData(null); setError("The skipped PR audit endpoint returned an unexpected response."); setStatus("error"); return; } - setData(normalized); + setData((current) => appendSkippedPrAuditPage(current, normalized)); setStatus("ready"); return; } - setData(null); + if (offset === 0) setData(null); setError(result.message); setStatus("error"); - }, [enabled, queryPath]); + }, [enabled, offset, queryPath]); useEffect(() => { void load(); }, [load]); + const resetPaging = () => setOffset(0); + const applyFilters = () => { setSinceIso(normalizeSinceInput(sinceInput)); setRepoFullName(repoDraft.trim()); - setLimit(DEFAULT_LIMIT); + resetPaging(); }; const resetFilters = () => { @@ -105,11 +112,20 @@ export function AuditFeed({ enabled = true }: AuditFeedProps) { setRepoFullName(""); setSinceInput(""); setSinceIso(""); - setLimit(DEFAULT_LIMIT); + resetPaging(); }; const loadMore = () => { - setLimit((current) => Math.min(current + DEFAULT_LIMIT, MAX_LIMIT)); + setOffset((current) => current + AUDIT_FEED_PAGE_SIZE); + }; + + const refresh = () => { + if (offset === 0) { + void load(); + return; + } + // Drop back to the first page so refresh replaces rather than appending a duplicate page. + resetPaging(); }; if (status === "loading" && !data) { @@ -140,7 +156,7 @@ export function AuditFeed({ enabled = true }: AuditFeedProps) { sinceInput={sinceInput} onReasonChange={(value) => { setReason(value); - setLimit(DEFAULT_LIMIT); + resetPaging(); }} onRepoDraftChange={setRepoDraft} onSinceInputChange={setSinceInput} @@ -150,7 +166,7 @@ export function AuditFeed({ enabled = true }: AuditFeedProps) { void load()}>Refresh} + action={Refresh} /> ); @@ -158,6 +174,9 @@ export function AuditFeed({ enabled = true }: AuditFeedProps) { if (!data) return null; + const canLoadMore = + data.hasMore && data.items.length < AUDIT_FEED_MAX_ITEMS && status !== "loading"; + return (
@@ -177,7 +196,7 @@ export function AuditFeed({ enabled = true }: AuditFeedProps) { sinceInput={sinceInput} onReasonChange={(value) => { setReason(value); - setLimit(DEFAULT_LIMIT); + resetPaging(); }} onRepoDraftChange={setRepoDraft} onSinceInputChange={setSinceInput} @@ -249,15 +268,14 @@ export function AuditFeed({ enabled = true }: AuditFeedProps) {
- {data.hasMore && limit < MAX_LIMIT ? ( - Load more - ) : null} - {data.hasMore && limit >= MAX_LIMIT ? ( + {canLoadMore ? Load more : null} + {data.hasMore && data.items.length >= AUDIT_FEED_MAX_ITEMS ? (

- Showing the maximum page size ({MAX_LIMIT}). Narrow filters to inspect older events. + Showing the maximum accumulated page size ({AUDIT_FEED_MAX_ITEMS}). Narrow filters to + inspect older events.

) : null} - void load()}>Refresh + Refresh
); diff --git a/src/api/routes.ts b/src/api/routes.ts index 0eda0c41a8..1fac60d0e7 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -672,6 +672,7 @@ const selfhostDeadLetterQueueQuerySchema = z const skippedPrAuditQuerySchema = z .object({ limit: z.coerce.number().int().optional(), + offset: z.coerce.number().int().optional(), repoFullName: z.string().trim().min(3).max(200).optional(), reason: z.enum(PUBLIC_SURFACE_SKIP_REASONS).optional(), since: z.string().trim().min(1).max(64).optional(), @@ -1725,6 +1726,7 @@ export function createApp() { if (repoFullNames instanceof Response) return repoFullNames; const page = await listPrVisibilitySkipAuditEvents(c.env, { limit: clampInteger(parsed.data.limit ?? 50, 1, 100), + offset: Math.max(0, parsed.data.offset ?? 0), repoFullNames, reason: parsed.data.reason, sinceIso, @@ -1732,6 +1734,8 @@ export function createApp() { return c.json({ generatedAt: nowIso(), limit: page.limit, + offset: page.offset, + total: page.total, hasMore: page.hasMore, filters: { repoFullName: requestedRepo ?? null, diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 0aa1f4fb4f..c8a0512498 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -3191,7 +3191,9 @@ export type PrVisibilitySkipAuditEvent = { export type PrVisibilitySkipAuditPage = { limit: number; + offset: number; hasMore: boolean; + total: number; items: PrVisibilitySkipAuditEvent[]; }; @@ -3199,14 +3201,20 @@ export async function listPrVisibilitySkipAuditEvents( env: Env, options: { limit?: number | undefined; + offset?: number | undefined; repoFullNames?: string[] | undefined; reason?: string | undefined; sinceIso?: string | undefined; } = {}, ): Promise { const limit = clampInteger(options.limit ?? 50, 1, 100); + // #7438: real offset pagination (not a growing limit). Floor at 0 so a negative query param cannot + // rewind into undefined territory; the UI advances by page size and appends. + const offset = Math.max(0, Math.floor(options.offset ?? 0)); const scopedRepoNames = options.repoFullNames === undefined ? undefined : uniqueRepoNames(options.repoFullNames.map((name) => name.trim()).filter(Boolean)); - if (scopedRepoNames !== undefined && scopedRepoNames.length === 0) return { limit, hasMore: false, items: [] }; + if (scopedRepoNames !== undefined && scopedRepoNames.length === 0) { + return { limit, offset, hasMore: false, total: 0, items: [] }; + } const conditions: SQL[] = [eq(auditEvents.eventType, "github_app.pr_visibility_skipped")]; if (options.reason) conditions.push(eq(auditEvents.detail, options.reason)); @@ -3221,7 +3229,11 @@ export async function listPrVisibilitySkipAuditEvents( if (repoFilter) conditions.push(repoFilter); } - const rowLimit = Math.min(500, limit * 5 + 20); + const whereClause = and(...conditions); + + // Over-fetch past offset+limit so unparsable targetKey rows (dropped below) don't shrink the page, and so + // we can tell whether another page exists after slicing. + const rowLimit = Math.min(500, (offset + limit) * 5 + 20); const rows = await getDb(env.DB) .select({ targetKey: auditEvents.targetKey, @@ -3230,7 +3242,7 @@ export async function listPrVisibilitySkipAuditEvents( createdAt: auditEvents.createdAt, }) .from(auditEvents) - .where(and(...conditions)) + .where(whereClause) .orderBy(desc(auditEvents.createdAt), desc(auditEvents.id)) .limit(rowLimit); const items = rows.flatMap((row) => { @@ -3246,7 +3258,13 @@ export async function listPrVisibilitySkipAuditEvents( }, ]; }); - return { limit, hasMore: items.length > limit, items: items.slice(0, limit) }; + const pageItems = items.slice(offset, offset + limit); + // hasMore is based on the parseable over-fetch window (not a raw audit-event COUNT, which can include + // unparsable targetKey rows and would leave Load more stuck on forever). + const hasMore = items.length > offset + limit; + // Lower-bound total when another page exists; exact when this is the last page (#7438 / DLQ-shaped response). + const total = hasMore ? offset + pageItems.length + 1 : offset + pageItems.length; + return { limit, offset, hasMore, total, items: pageItems }; } /** Repo-scoped rollups of gate-outcome audit rows for the maintainer dashboard (#2203). Counts only diff --git a/src/mcp/server.ts b/src/mcp/server.ts index e5b1237821..5c0586333e 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -946,11 +946,14 @@ const skippedPrAuditShape = { reason: z.enum(PUBLIC_SURFACE_SKIP_REASONS).optional(), since: z.string().trim().min(1).max(64).optional(), limit: z.number().int().positive().optional(), + offset: z.number().int().min(0).optional(), }; const skippedPrAuditOutputSchema = { generatedAt: z.string().optional(), limit: z.number().optional(), + offset: z.number().optional(), + total: z.number().optional(), hasMore: z.boolean().optional(), filters: z.unknown().optional(), items: z.array(z.unknown()).optional(), @@ -3449,6 +3452,7 @@ export class LoopoverMcp { reason?: PublicSurfaceSkipReason | undefined; since?: string | undefined; limit?: number | undefined; + offset?: number | undefined; }): Promise { const repoFullNames = await this.requireSkippedPrAuditAccess(input.repoFullName); let sinceIso: string | undefined; @@ -3457,12 +3461,20 @@ export class LoopoverMcp { if (!Number.isFinite(timestamp)) throw new Error(`Invalid since: "${input.since}" is not a parseable date.`); sinceIso = new Date(timestamp).toISOString(); } - const page = await listPrVisibilitySkipAuditEvents(this.env, { limit: input.limit, repoFullNames, reason: input.reason, sinceIso }); + const page = await listPrVisibilitySkipAuditEvents(this.env, { + limit: input.limit, + offset: input.offset, + repoFullNames, + reason: input.reason, + sinceIso, + }); return { - summary: `LoopOver skipped-PR audit: ${page.items.length} event(s) (limit ${page.limit}${page.hasMore ? ", more available" : ""}).`, + summary: `LoopOver skipped-PR audit: ${page.items.length} event(s) (limit ${page.limit}, offset ${page.offset}${page.hasMore ? ", more available" : ""}).`, data: { generatedAt: nowIso(), limit: page.limit, + offset: page.offset, + total: page.total, hasMore: page.hasMore, filters: { repoFullName: input.repoFullName ?? null, diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index daf774cd7d..fc83eb3cd0 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -1132,6 +1132,8 @@ export const SkippedPrAuditExportSchema = z .object({ generatedAt: z.string(), limit: z.number().int().min(1).max(100), + offset: z.number().int().min(0), + total: z.number().int().min(0), hasMore: z.boolean(), filters: z.object({ repoFullName: z.string().nullable(), diff --git a/src/openapi/spec.ts b/src/openapi/spec.ts index 574e765773..4ff7e7c353 100644 --- a/src/openapi/spec.ts +++ b/src/openapi/spec.ts @@ -1199,9 +1199,13 @@ export function buildOpenApiSpec() { request: { query: z.object({ limit: z.string().optional().openapi({ - param: { description: "Maximum rows to return, clamped from 1 to 100." }, + param: { description: "Maximum rows to return per page, clamped from 1 to 100." }, example: "50", }), + offset: z.string().optional().openapi({ + param: { description: "Zero-based row offset for pagination. Advance by `limit` (or the previous page's item count) and append; do not grow `limit` to page." }, + example: "0", + }), repoFullName: z.string().optional().openapi({ param: { description: "Optional repository filter. Browser sessions must have control-panel access to this repo." }, example: "JSONbored/loopover", diff --git a/test/integration/api.test.ts b/test/integration/api.test.ts index cf1b2be154..6f84c2bfe7 100644 --- a/test/integration/api.test.ts +++ b/test/integration/api.test.ts @@ -4476,11 +4476,15 @@ describe("api routes", () => { expect(bounded.status).toBe(200); const boundedBody = (await bounded.json()) as { limit: number; + offset: number; + total: number; hasMore: boolean; items: Array<{ repoFullName: string; pullNumber: number; reason: string; timestamp: string; remediation: string }>; }; expect(boundedBody.limit).toBe(3); + expect(boundedBody.offset).toBe(0); expect(boundedBody.hasMore).toBe(true); + expect(boundedBody.total).toBeGreaterThan(3); expect(boundedBody.items).toEqual([ expect.objectContaining({ repoFullName: "victim-org/secret-repo", pullNumber: 7, reason: "maintainer_author" }), expect.objectContaining({ repoFullName: "repo-owner/owned-repo", pullNumber: 6, reason: "surface_off" }), @@ -4489,6 +4493,23 @@ describe("api routes", () => { expect(boundedBody.items[1]?.remediation).toContain("repository settings"); expect(JSON.stringify(boundedBody)).not.toMatch(/private-author|bot-secret|detector-secret|surface-secret|victim-secret|delivery-secret|github_pat|wallet|hotkey|raw trust/i); + // #7438: offset pages into older rows without re-fetching the head of the feed. + const page2 = await app.request("/v1/app/skipped-pr-audit?limit=3&offset=3", { headers: apiHeaders(env) }, env); + expect(page2.status).toBe(200); + const page2Body = (await page2.json()) as { + offset: number; + hasMore: boolean; + items: Array<{ pullNumber: number }>; + }; + expect(page2Body.offset).toBe(3); + expect(page2Body.items).toHaveLength(3); + expect(page2Body.items.map((item) => item.pullNumber)).not.toEqual( + boundedBody.items.map((item) => item.pullNumber), + ); + const omittedOffset = await app.request("/v1/app/skipped-pr-audit?limit=1", { headers: apiHeaders(env) }, env); + expect(omittedOffset.status).toBe(200); + await expect(omittedOffset.json()).resolves.toMatchObject({ offset: 0, limit: 1 }); + const reasonFiltered = await app.request("/v1/app/skipped-pr-audit?reason=bot_author&limit=500", { headers: apiHeaders(env) }, env); expect(reasonFiltered.status).toBe(200); const reasonFilteredBody = (await reasonFiltered.json()) as { limit: number; hasMore: boolean; items: Array<{ reason: string; pullNumber: number }> }; diff --git a/test/unit/mcp-skipped-pr-audit.test.ts b/test/unit/mcp-skipped-pr-audit.test.ts index bc111319f1..3f6d163d58 100644 --- a/test/unit/mcp-skipped-pr-audit.test.ts +++ b/test/unit/mcp-skipped-pr-audit.test.ts @@ -31,6 +31,8 @@ async function seedOwnedRepo(env: Env, installationId: number, owner: string, na type SkippedPrAuditData = { generatedAt: string; limit: number; + offset: number; + total: number; hasMore: boolean; filters: { repoFullName: string | null; reason: string | null; since: string | null }; items: Array<{ repoFullName: string; pullNumber: number; reason: string; timestamp: string; remediation: string }>; @@ -68,6 +70,7 @@ describe("MCP loopover_get_skipped_pr_audit (#5825)", () => { expect(data.items).toEqual([expect.objectContaining({ repoFullName: "repo-owner/owned-repo", pullNumber: 4, reason: "bot_author" })]); expect(data.items[0]?.remediation).toContain("intentionally kept quiet"); expect(data.limit).toBe(50); + expect(data.offset).toBe(0); expect(data.hasMore).toBe(false); expect(data.filters).toEqual({ repoFullName: null, reason: null, since: null }); expect(JSON.stringify(result.content)).not.toContain("github_pat_should_not_export"); diff --git a/test/unit/skipped-pr-audit.test.ts b/test/unit/skipped-pr-audit.test.ts index 82bbdb6434..38ba70aa59 100644 --- a/test/unit/skipped-pr-audit.test.ts +++ b/test/unit/skipped-pr-audit.test.ts @@ -49,13 +49,14 @@ describe("skipped PR audit repository export", () => { }); const emptyScope = await listPrVisibilitySkipAuditEvents(env, { repoFullNames: [] }); - expect(emptyScope).toMatchObject({ limit: 50, hasMore: false, items: [] }); + expect(emptyScope).toMatchObject({ limit: 50, offset: 0, hasMore: false, total: 0, items: [] }); const scoped = await listPrVisibilitySkipAuditEvents(env, { limit: Number.NaN, repoFullNames: ["owner/re_po", "OWNER/re_po"], }); expect(scoped.limit).toBe(1); + expect(scoped.offset).toBe(0); expect(scoped.items).toEqual([ { repoFullName: "owner/re_po", @@ -68,6 +69,28 @@ describe("skipped PR audit repository export", () => { const unscoped = await listPrVisibilitySkipAuditEvents(env); expect(unscoped.limit).toBe(50); + expect(unscoped.offset).toBe(0); expect(unscoped.items.map((item) => item.pullNumber)).toEqual([8, 7]); + expect(unscoped.total).toBeGreaterThanOrEqual(2); + + // #7438: offset advances into the next page instead of growing limit from the start. + const page0 = await listPrVisibilitySkipAuditEvents(env, { limit: 1, offset: 0 }); + const page1 = await listPrVisibilitySkipAuditEvents(env, { limit: 1, offset: 1 }); + expect(page0.items).toHaveLength(1); + expect(page0.hasMore).toBe(true); + expect(page0.offset).toBe(0); + expect(page1.items).toHaveLength(1); + expect(page1.offset).toBe(1); + expect(page1.items[0]?.pullNumber).not.toBe(page0.items[0]?.pullNumber); + expect(page1.items[0]?.pullNumber).toBe(7); + + const pastEnd = await listPrVisibilitySkipAuditEvents(env, { limit: 10, offset: 50 }); + expect(pastEnd.items).toEqual([]); + expect(pastEnd.hasMore).toBe(false); + expect(pastEnd.offset).toBe(50); + + const negativeOffset = await listPrVisibilitySkipAuditEvents(env, { limit: 10, offset: -5 }); + expect(negativeOffset.offset).toBe(0); + expect(negativeOffset.items.map((item) => item.pullNumber)).toEqual([8, 7]); }); }); diff --git a/test/unit/support/mcp-cli-harness.ts b/test/unit/support/mcp-cli-harness.ts index 792adf4199..6541bb5eaf 100644 --- a/test/unit/support/mcp-cli-harness.ts +++ b/test/unit/support/mcp-cli-harness.ts @@ -771,7 +771,7 @@ export async function startFixtureServer( response.end(JSON.stringify({ error: "skipped_pr_audit_unavailable" })); return; } - response.end(JSON.stringify({ generatedAt: "2026-05-30T00:00:00.000Z", limit: 20, hasMore: false, items: [{ prNumber: 7, reason: "duplicate", remediation: "link the canonical PR" }] })); + response.end(JSON.stringify({ generatedAt: "2026-05-30T00:00:00.000Z", limit: 20, offset: 0, total: 1, hasMore: false, items: [{ prNumber: 7, reason: "duplicate", remediation: "link the canonical PR" }] })); return; } // #6750: the boundary-tests lint mirror proxies here; echo a fired finding + spec.