Skip to content

Commit 628ad7a

Browse files
committed
fix(web): paginate change history beyond the demo scan budget
## Summary ### Why? Change history failed once the receipt window contained more than 500 queue requests, even when the selected change had only one submission. ### What? Return bounded history pages with gateway continuations rather than a resource-exhausted error. Keep page-scoped models, notices, empty states, and navigation in the reusable library; the Next host signs queue/change/version-scoped cursors, preserves snapshot bounds, and resets older pages to a live window on refresh. Preserve exact-version lookup and reject looping gateway cursors. ## Test Plan Passed change-branch web checks, package-consumer checks, production Next standalone build, formatting, lint, module tidy, and Gazelle checks. Added loader/component/reader regression tests for partial history, empty chunks, continuation, and repeated cursors.
1 parent 1533633 commit 628ad7a

9 files changed

Lines changed: 138 additions & 26 deletions

File tree

‎web/service/submitqueue/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ GitHub, Phabricator, and git change pages mirror the canonical change URI's auth
1010

1111
The default queue and change views use a rolling 24-hour window recalculated on every refresh; there are no `from`/`to` URL parameters. Older queue pages carry the stable gateway window inside a signed, queue-scoped `page` cursor, and their Refresh button returns to the live first page.
1212

13-
Pinned GitHub/Phabricator pages use exact-URI summary lookup. Across-version and git demo pages scan queue receipts in the displayed rolling window, up to ten gateway pages. An exhausted or looping scan fails rather than returning partial history. Git scans also match raw demo URIs carrying file hints without requiring those hints in the browser URL. This bounded adapter is not an unlimited logical-change history API.
13+
Pinned GitHub/Phabricator pages use exact-URI summary lookup. Across-version and git demo pages scan queue receipts in the displayed rolling window, up to ten gateway pages per history page. When more queue requests remain, the host marks the results as page-scoped and offers **Continue history scan** instead of failing or claiming complete history. Signed cursors bind the stable receipt window and gateway continuation to the queue, change, and pinned version. Older pages pause automatic refresh; Refresh and **Latest history** return to a fresh live window. A looping gateway cursor still fails. Git scans also match raw demo URIs carrying file hints without requiring those hints in the browser URL. This bounded adapter is not an unlimited logical-change history API.
1414

1515
Required configuration:
1616

‎web/service/submitqueue/src/app/(protected)/[queue]/change/[...reference]/page.tsx‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,15 @@ import { readChangeSubmissions } from "../../../../../server/change-reader";
1010
import { resolveGatewayClient } from "../../../../../server/gateway";
1111
import { requireAuthorization } from "../../../../../server/request-auth";
1212
import { gatewayDiagnostics } from "../../../../../server/diagnostics";
13-
import { defaultRequestWindow } from "../../../../../server/window";
13+
import { loadAuthConfiguration } from "../../../../../server/auth";
14+
import { defaultRequestWindow, decodeRequestPage, encodeRequestPage } from "../../../../../server/window";
1415

1516
export const dynamic = "force-dynamic";
1617
export const revalidate = 0;
1718

1819
export default async function ChangePage({ params, searchParams }: {
1920
params: Promise<{ queue: string; reference: string[] }>;
20-
searchParams: Promise<{ from?: string | string[]; to?: string | string[] }>;
21+
searchParams: Promise<{ from?: string | string[]; to?: string | string[]; page?: string | string[] }>;
2122
}) {
2223
await connection();
2324
await requireAuthorization();
@@ -28,25 +29,44 @@ export default async function ChangePage({ params, searchParams }: {
2829
notFound();
2930
}
3031
const search = await searchParams;
31-
if (search.from !== undefined || search.to !== undefined) {
32-
redirect(changeHref(queue, reference, reference.version !== null));
32+
const latestHref = changeHref(queue, reference, reference.version !== null);
33+
const scansQueue = reference.version === null || reference.scheme === "git";
34+
if (search.from !== undefined || search.to !== undefined ||
35+
(search.page !== undefined && (typeof search.page !== "string" || !scansQueue))) {
36+
redirect(latestHref);
3337
}
34-
const requestWindow = defaultRequestWindow();
38+
const secret = loadAuthConfiguration().token;
39+
const cursorScope = JSON.stringify([queue, reference.logicalPath, reference.version]);
40+
const pageWindow = typeof search.page === "string" ? decodeRequestPage(cursorScope, search.page, secret) : undefined;
41+
if (search.page !== undefined && pageWindow === undefined) {
42+
redirect(latestHref);
43+
}
44+
const requestWindow = pageWindow ?? defaultRequestWindow();
3545
const result = await loadChangeSubmissions(
36-
() => readChangeSubmissions(resolveGatewayClient(), queue, reference, requestWindow),
46+
async () => {
47+
const page = await readChangeSubmissions(resolveGatewayClient(), queue, reference, requestWindow);
48+
return {
49+
submissions: page.submissions,
50+
pagination: page.nextPageToken || pageWindow ? {
51+
nextHref: page.nextPageToken
52+
? `${latestHref}?page=${encodeRequestPage(cursorScope, requestWindow, page.nextPageToken, secret)}` : null,
53+
latestHref: pageWindow ? latestHref : null,
54+
} : undefined,
55+
};
56+
},
3757
{
3858
queue, provider: reference.scheme === "github" ? "GitHub" : reference.scheme === "phab" ? "Phabricator" : "Git",
3959
host: reference.host, repository: reference.repository, review: reference.review,
4060
pinnedVersion: reference.version,
4161
logicalHref: changeHref(queue, reference),
42-
window: reference.version === null || reference.scheme === "git" ? { fromMs: requestWindow.fromMs, toMs: requestWindow.toMs } : null,
62+
window: scansQueue ? { fromMs: requestWindow.fromMs, toMs: requestWindow.toMs } : null,
4363
},
4464
{ diagnostics: gatewayDiagnostics },
4565
);
4666
return <main className="shell">
4767
{result.ok ? <ChangeView model={result.data} /> : <ErrorState error={result.error} />}
48-
<div className="page-actions"><NextRefresh
49-
terminal={!result.ok && !result.error.retryable}
68+
<div className="page-actions"><NextRefresh refreshHref={pageWindow ? latestHref : undefined}
69+
terminal={pageWindow !== undefined || (!result.ok && !result.error.retryable)}
5070
transientFailureCount={!result.ok && result.error.retryable ? 1 : 0}
5171
/></div>
5272
</main>;

‎web/service/submitqueue/src/server/change-reader.test.ts‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ describe("demo bounded change reader", () => {
1616
const client = { list } as unknown as SubmitQueueGatewayClient;
1717
const reference = parseChangePath(["github", "github.com", "uber", "submitqueue", "pull", "123"])!;
1818
const result = await readChangeSubmissions(client, "demo-queue", reference, window);
19-
expect(result.map(value => value.request.sqid)).toEqual(["42", "38"]);
19+
expect(result.submissions.map(value => value.request.sqid)).toEqual(["42", "38"]);
20+
expect(result.nextPageToken).toBeNull();
2021
expect(list).toHaveBeenLastCalledWith(expect.objectContaining({ pageToken: "next", receivedAtOrAfterMs: 100n, receivedBeforeMs: 200n }));
2122
});
2223

@@ -35,12 +36,43 @@ describe("demo bounded change reader", () => {
3536
const getRequestSummaryByChangeURI = vi.fn();
3637
const client = { list, getRequestSummaryByChangeURI } as unknown as SubmitQueueGatewayClient;
3738
const result = await readChangeSubmissions(client, "demo-queue", parseChangeUri(uri)!, window);
38-
expect(result[0]?.request.sqid).toBe("42");
39-
expect(result[0]?.versionHref).not.toContain("?");
39+
expect(result.submissions[0]?.request.sqid).toBe("42");
40+
expect(result.submissions[0]?.versionHref).not.toContain("?");
4041
expect(getRequestSummaryByChangeURI).not.toHaveBeenCalled();
4142
});
4243

43-
it("fails a truncated or looping scan instead of presenting partial history", async () => {
44+
it.each([true, false])("continues a bounded scan without losing older matches (first page has matches: %s)", async (hasMatches) => {
45+
const list = vi.fn().mockImplementation(async ({ pageToken }: { pageToken: string }) => {
46+
const page = pageToken ? Number(pageToken.slice(5)) : 0;
47+
return {
48+
requests: page === 10 ? [request("38")] : page === 0 && hasMatches ? [request("42")] : [],
49+
nextPageToken: page < 10 ? `page-${page + 1}` : "",
50+
};
51+
});
52+
const client = { list } as unknown as SubmitQueueGatewayClient;
53+
const reference = parseChangePath(["github", "github.com", "uber", "submitqueue", "pull", "123"])!;
54+
const first = await readChangeSubmissions(client, "demo-queue", reference, window);
55+
expect(first.submissions.map(value => value.request.sqid)).toEqual(hasMatches ? ["42"] : []);
56+
expect(first.nextPageToken).toBe("page-10");
57+
expect(list).toHaveBeenCalledTimes(10);
58+
const second = await readChangeSubmissions(client, "demo-queue", reference, { ...window, pageToken: first.nextPageToken! });
59+
expect(second.submissions.map(value => value.request.sqid)).toEqual(["38"]);
60+
expect(second.nextPageToken).toBeNull();
61+
expect(list).toHaveBeenLastCalledWith(expect.objectContaining({
62+
pageToken: "page-10", receivedAtOrAfterMs: 100n, receivedBeforeMs: 200n,
63+
}));
64+
});
65+
66+
it("rejects a cursor that loops back to the starting page", async () => {
67+
const client = {
68+
list: vi.fn().mockResolvedValue({ requests: [], nextPageToken: "start" }),
69+
} as unknown as SubmitQueueGatewayClient;
70+
const reference = parseChangePath(["github", "github.com", "uber", "submitqueue", "pull", "123"])!;
71+
await expect(readChangeSubmissions(client, "demo-queue", reference, { ...window, pageToken: "start" }))
72+
.rejects.toMatchObject({ code: Code.Internal });
73+
});
74+
75+
it("fails a looping scan instead of presenting duplicate history", async () => {
4476
const client = {
4577
list: vi.fn().mockResolvedValue({ requests: [request("42")], nextPageToken: "repeated" }),
4678
} as unknown as SubmitQueueGatewayClient;

‎web/service/submitqueue/src/server/change-reader.ts‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,17 @@ import { REQUEST_PAGE_SIZE, type RequestWindow } from "./window";
77

88
const MAX_CHANGE_SCAN_PAGES = 10;
99

10+
export interface ChangeSubmissionPage {
11+
submissions: GatewayChangeSubmission[];
12+
nextPageToken: string | null;
13+
}
14+
1015
export async function readChangeSubmissions(
1116
client: SubmitQueueGatewayClient,
1217
queue: string,
1318
reference: ChangeReference,
1419
window: RequestWindow,
15-
): Promise<GatewayChangeSubmission[]> {
20+
): Promise<ChangeSubmissionPage> {
1621
const submissions: GatewayChangeSubmission[] = [];
1722
const appendSubmissions = (requests: readonly GatewayRequestSummary[]) => {
1823
for (const request of requests) {
@@ -30,10 +35,10 @@ export async function readChangeSubmissions(
3035
if (reference.uri !== null && reference.scheme !== "git") {
3136
const response = await client.getRequestSummaryByChangeURI({ queue, changeUri: reference.uri });
3237
appendSubmissions(response.requests);
33-
return submissions;
38+
return { submissions, nextPageToken: null };
3439
}
35-
let pageToken = "";
36-
const seenTokens = new Set<string>();
40+
let pageToken = window.pageToken ?? "";
41+
const seenTokens = new Set<string>(pageToken ? [pageToken] : []);
3742
for (let page = 0; page < MAX_CHANGE_SCAN_PAGES; page += 1) {
3843
const response = await client.list({
3944
queue,
@@ -44,14 +49,13 @@ export async function readChangeSubmissions(
4449
});
4550
appendSubmissions(response.requests);
4651
if (!response.nextPageToken) {
47-
return submissions;
52+
return { submissions, nextPageToken: null };
4853
}
4954
if (seenTokens.has(response.nextPageToken)) {
5055
throw new ConnectError("Repeated queue page token", Code.Internal);
5156
}
5257
pageToken = response.nextPageToken;
5358
seenTokens.add(pageToken);
5459
}
55-
// Never present a truncated queue scan as complete change history.
56-
throw new ConnectError("Change receipt window exceeds demo scan budget", Code.ResourceExhausted);
60+
return { submissions, nextPageToken: pageToken };
5761
}

‎web/submitqueue/src/components.test.tsx‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,27 @@ describe("request components", () => {
170170
expect(screen.getByRole("link", { name: version }).getAttribute("href")).toBe(`/logical/${version}`);
171171
});
172172

173+
it.each([
174+
{ nextHref: "/change?page=next", latestHref: null },
175+
{ nextHref: null, latestHref: "/change" },
176+
])("renders page-scoped history and host-provided navigation: %j", pagination => {
177+
render(<ChangeSubmissions model={{
178+
queue: "demo-queue", provider: "Git", host: "git.example.com",
179+
repository: "demo", review: "refs/heads/main", pinnedVersion: null,
180+
window: { fromMs: 1, toMs: 2 }, submissions: [], pagination,
181+
}} />);
182+
expect(screen.getByText("No submissions were found on this page.")).toBeTruthy();
183+
expect(screen.queryByText("No submissions were found in this scope.")).toBeNull();
184+
expect(screen.getByRole("status").textContent).toContain("Showing matches from this page");
185+
if (pagination.nextHref) {
186+
expect(screen.getByRole("link", { name: "Continue history scan" }).getAttribute("href")).toBe(pagination.nextHref);
187+
expect(screen.getByRole("status").textContent).toContain("More queue requests remain");
188+
} else {
189+
expect(screen.queryByRole("link", { name: "Continue history scan" })).toBeNull();
190+
expect(screen.getByRole("link", { name: "Latest history" }).getAttribute("href")).toBe(pagination.latestHref);
191+
}
192+
});
193+
173194
it("shows unknown statuses safely", () => {
174195
render(<RequestStatus status="new_pipeline_step" />);
175196
expect(screen.getByText("New Pipeline Step").getAttribute("data-tone")).toBe("neutral");

‎web/submitqueue/src/components.tsx‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,10 @@ export function ChangeSubmissions({ model, basePath, onVersionChange }: ChangeSu
227227
return <section className="sq-change-detail">
228228
<nav className="sq-breadcrumb" aria-label="Breadcrumb"><a href={paths.directory()}>Queues</a><span>›</span><a href={paths.requests(model.queue)}>{model.queue}</a><span>› Change</span></nav>
229229
<h1>{model.review}</h1><p className="sq-secondary">{model.pinnedVersion ? `Submitted version: ${model.pinnedVersion}` : "Submissions across versions"}</p>
230+
{model.pagination ? <p className="sq-secondary" role="status">
231+
Showing matches from this page of queue requests only.
232+
{model.pagination.nextHref ? " More queue requests remain to be searched." : " The history scan is complete for this receipt window."}
233+
</p> : null}
230234
<dl className="sq-change-identity"><div><dt>Provider</dt><dd>{model.provider}</dd></div><div><dt>Host</dt><dd>{model.host}</dd></div><div><dt>Repository</dt><dd>{model.repository || "—"}</dd></div><div><dt>Queue</dt><dd>{model.queue}</dd></div></dl>
231235
<div className="sq-toolbar"><h2>Submission history</h2>
232236
{model.logicalHref && onVersionChange && versions.length > 0 ?
@@ -238,11 +242,15 @@ export function ChangeSubmissions({ model, basePath, onVersionChange }: ChangeSu
238242
model.pinnedVersion !== null && model.logicalHref ? <a href={model.logicalHref}>All submitted versions</a> : null}
239243
</div>
240244
{model.window ? <p className="sq-secondary">Receipt window: <Timestamp value={model.window.fromMs} /> — <Timestamp value={model.window.toMs} />. This is not unlimited retained history.</p> : <p className="sq-secondary">Retained submissions for this exact version</p>}
241-
{model.submissions.length === 0 ? <p className="sq-empty">No submissions were found in this scope.</p> :
245+
{model.submissions.length === 0 ? <p className="sq-empty">{model.pagination ? "No submissions were found on this page." : "No submissions were found in this scope."}</p> :
242246
<div className="sq-table-scroll"><table aria-label="Change submissions"><thead><tr><th>Request</th><th>Submitted version</th><th>Received · UTC</th><th>Status</th></tr></thead>
243247
<tbody>{model.submissions.map(item => <tr key={item.request.sqid}><td><a href={paths.request(model.queue, item.request.sqid)}>{item.request.sqid}</a></td>
244248
<td><a className="sq-version" aria-label={item.version} title={item.version} href={item.versionHref}>{item.version.length > 20 ? `${item.version.slice(0, 8)}…${item.version.slice(-4)}` : item.version}</a></td><td><Timestamp value={item.request.receivedAtMs} /></td>
245249
<td><RequestStatus status={item.request.status} />{item.request.lastError ? <p className="sq-row-error">{item.request.lastError}</p> : null}</td></tr>)}</tbody></table></div>}
246-
<p className="sq-secondary">{model.submissions.length} requests · newest received first · repeated submissions remain separate</p>
250+
<p className="sq-secondary">{model.submissions.length} requests{model.pagination ? " on this page" : ""} · newest received first · repeated submissions remain separate</p>
251+
{model.pagination ? <nav className="page-actions" aria-label="Change history pagination">
252+
{model.pagination.nextHref ? <a href={model.pagination.nextHref}>Continue history scan</a> : null}
253+
{model.pagination.latestHref ? <a href={model.pagination.latestHref}>Latest history</a> : null}
254+
</nav> : null}
247255
</section>;
248256
}

‎web/submitqueue/src/models.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ export interface ChangeDetailModel {
6464
logicalHref?: string;
6565
submissions: ChangeSubmissionModel[];
6666
window: { fromMs: number; toMs: number } | null;
67+
pagination?: { nextHref: string | null; latestHref: string | null };
6768
}
6869

6970
export interface WebError {

‎web/submitqueue/src/server.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { createFakeGatewayReader, gatewayHistoryFixture, gatewayRequestFixture }
44

55
import {
66
classifyGatewayError,
7+
loadChangeSubmissions,
78
loadRequestDetail,
89
loadRequestList,
910
requestDetailIsComplete,
@@ -14,6 +15,24 @@ import {
1415
describe("server presentation mapping", () => {
1516
beforeEach(() => vi.clearAllMocks());
1617

18+
it.each([false, true])("maps change submissions with optional host pagination (paged: %s)", async paged => {
19+
const request = gatewayRequestFixture();
20+
const submissions = [{
21+
request, version: "sha", versionHref: "/change/sha",
22+
}];
23+
const pagination = { nextHref: "/change?page=next", latestHref: null };
24+
const result = await loadChangeSubmissions(async () => paged ? { submissions, pagination } : submissions, {
25+
queue: "demo-queue", provider: "Git", host: "git.example.com",
26+
repository: "demo", review: "refs/heads/main", pinnedVersion: null, window: { fromMs: 100, toMs: 200 },
27+
});
28+
expect(result).toMatchObject({
29+
ok: true, data: { submissions: [{ request: { sqid: request.sqid }, version: "sha" }] },
30+
});
31+
if (result.ok) {
32+
expect(result.data.pagination).toEqual(paged ? pagination : undefined);
33+
}
34+
});
35+
1736
it("converts int64 only within JavaScript's safe range", () => {
1837
expect(safeInt64ToNumber(1_700_000_000_000n)).toBe(1_700_000_000_000);
1938
expect(safeInt64ToNumber(BigInt(Number.MAX_SAFE_INTEGER) + 1n)).toBeNull();

‎web/submitqueue/src/server.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -370,24 +370,31 @@ export interface GatewayChangeSubmission {
370370
versionHref: string;
371371
}
372372

373+
export interface GatewayChangeSubmissionPage {
374+
submissions: readonly GatewayChangeSubmission[];
375+
pagination?: ChangeDetailModel["pagination"];
376+
}
377+
373378
export async function loadChangeSubmissions(
374-
read: () => Promise<readonly GatewayChangeSubmission[]>,
379+
read: () => Promise<readonly GatewayChangeSubmission[] | GatewayChangeSubmissionPage>,
375380
context: Omit<ChangeDetailModel, "submissions">,
376381
options: GatewayLoadOptions = {},
377382
): Promise<LoadResult<ChangeDetailModel>> {
378383
if (!context.queue) {
379384
return { ok: false, error: INVALID_INPUT };
380385
}
381386
try {
387+
const response = await read();
388+
const page: GatewayChangeSubmissionPage = "submissions" in response ? response : { submissions: response };
382389
const submissions: ChangeDetailModel["submissions"] = [];
383-
for (const item of await read()) {
390+
for (const item of page.submissions) {
384391
const request = mapRequestSummary(item.request);
385392
if (!request) {
386393
return { ok: false, error: INTERNAL_ERROR };
387394
}
388395
submissions.push({ request, version: item.version, versionHref: item.versionHref });
389396
}
390-
return { ok: true, data: { ...context, submissions } };
397+
return { ok: true, data: { ...context, submissions, ...(page.pagination ? { pagination: page.pagination } : {}) } };
391398
} catch (error) {
392399
reportGatewayError(options, "change", context.queue, null, error);
393400
return { ok: false, error: classifyGatewayError(error) };

0 commit comments

Comments
 (0)