From 964c2133cd3ba6b328d87623d66c2441dd9de49a Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Mon, 5 Oct 2026 18:48:19 -0700 Subject: [PATCH] feat(web): discover configured queues through the gateway ## Summary ### Why? The gateway ListQueues API has landed, so the UI should discover configured queues instead of hard-coding demo-queue. ### What? Add a framework-independent queue-directory loader and structural reader interface, fetch the gateway catalog in the Next host, and validate queue/request/change routes against it. Display empty configured queues, preserve discovery failures as errors, and update package-consumer coverage and documentation. Preserve the web stack rooted at main, including the change-history routing and pagination fixes in their owning branch. ## Test Plan Passed all ten web unit/type/lint/generation/package checks and the production Next/OCI build through the real-stack suite. All five Playwright/axe tests passed, covering authentication, queue discovery, request history, readable Git change links, and pagination beyond 500 queue requests. --- doc/rfc/web-library.md | 6 ++-- web/package_test/consumer.ts | 3 +- web/service/submitqueue/README.md | 4 +-- .../[queue]/change/[...reference]/page.tsx | 12 ++++--- .../src/app/(protected)/[queue]/page.tsx | 9 +++--- .../[queue]/request/[...sqid]/page.tsx | 12 ++++--- .../submitqueue/src/app/(protected)/page.tsx | 16 ++++++++-- web/service/submitqueue/src/app/not-found.tsx | 2 +- web/service/submitqueue/src/server/config.ts | 4 --- .../submitqueue/src/server/queues.test.ts | 27 ++++++++++++++++ web/service/submitqueue/src/server/queues.ts | 13 ++++++++ web/submitqueue/README.md | 2 ++ web/submitqueue/src/components.tsx | 4 +-- web/submitqueue/src/server.test.ts | 32 +++++++++++++++++++ web/submitqueue/src/server.ts | 32 ++++++++++++++++++- web/submitqueue/src/testing.ts | 13 ++++++-- web/test/e2e/submitqueue.spec.ts | 17 ++++++++++ 17 files changed, 177 insertions(+), 31 deletions(-) create mode 100644 web/service/submitqueue/src/server/queues.test.ts create mode 100644 web/service/submitqueue/src/server/queues.ts diff --git a/doc/rfc/web-library.md b/doc/rfc/web-library.md index 9b464e333..d1cc4fc02 100644 --- a/doc/rfc/web-library.md +++ b/doc/rfc/web-library.md @@ -10,11 +10,11 @@ Phase one is read-only: request summary, history, and queue receipt list ([statu ## URL reference -The entry point lists the queues configured by the host, with each queue linking to its landing page. The queue is the top-level navigation context. Singular resource segments identify what the user is opening; the ID remains a value supplied by the gateway, not a string the UI interprets. The reference host currently serves only `demo-queue`, so its queue list initially has one entry. +The entry point lists gateway-configured queues through `ListQueues`, including empty queues, with each queue linking to its landing page. The queue is the top-level navigation context; resource IDs remain gateway-supplied values that the UI does not interpret. | Resource or view | Route | Example | Availability | |---|---|---|---| -| Queue directory | `/` | `/` lists configured queues, including a link to `/demo-queue` | Reference host | +| Queue directory | `/` | `/` lists queues returned by `ListQueues` | Reference host | | Queue landing and request list | `/` | `/demo-queue` | Reference host | | Request detail | `//request/` | `/demo-queue/request/10` | Reference host | | Request history tab | `//request/?view=history` | `/demo-queue/request/10?view=history` | Reference host | @@ -22,7 +22,7 @@ The entry point lists the queues configured by the host, with each queue linking | GitHub logical change | `//change/github////pull/` | `/demo-queue/change/github/github.com/uber/submitqueue/pull/123` | Demo receipt-window scan; unbounded lookup deferred | | Phabricator logical change | `//change/phab//D` | `/demo-queue/change/phab/phabricator.example.com/D12345` | Demo receipt-window scan; unbounded lookup deferred | -The queue directory replaces the automatic redirect to `demo-queue`. Queue discovery belongs to the host's configured queue registry; the directory does not infer queue names from requests or require a new gateway discovery API. +The queue directory replaces the automatic redirect to `demo-queue`. The library maps queue models; the host fetches and validates them through the published gateway API, without inferring queues from received requests. The request and batch examples follow [scoped sequential resource IDs](scoped-resource-ids.md): `"10"` and `"4"` are stored string IDs, and the route supplies their queue and resource context. The UI passes each ID unchanged to the gateway. The mockups below retain earlier illustrative slash-containing IDs; those labels do not override the scoped-ID contract. diff --git a/web/package_test/consumer.ts b/web/package_test/consumer.ts index 69b90138e..61d92ed75 100644 --- a/web/package_test/consumer.ts +++ b/web/package_test/consumer.ts @@ -1,5 +1,5 @@ import { RequestList, QueueDirectory, ChangeSubmissions, AutoRefresh } from "@submitqueue/web-submitqueue"; -import { loadRequestList, loadChangeSubmissions } from "@submitqueue/web-submitqueue/server"; +import { loadRequestList, loadChangeSubmissions, loadQueueDirectory } from "@submitqueue/web-submitqueue/server"; import { createFakeGatewayReader } from "@submitqueue/web-submitqueue/testing"; void RequestList; @@ -9,3 +9,4 @@ void QueueDirectory; void ChangeSubmissions; void AutoRefresh; void loadChangeSubmissions; +void loadQueueDirectory; diff --git a/web/service/submitqueue/README.md b/web/service/submitqueue/README.md index ae38b46ca..b30b651fc 100644 --- a/web/service/submitqueue/README.md +++ b/web/service/submitqueue/README.md @@ -2,9 +2,9 @@ This Next.js application demonstrates how to host `@submitqueue/web-submitqueue`. It is intentionally local/demo wiring rather than a production authentication or deployment contract. -The host owns the `/` landing page and `//request/` detail route, the fixed `demo-queue` selection, HTTP Basic authentication, server-side Connect transport, readiness checks, and environment validation. Request IDs remain readable and are passed to the gateway without interpreting their internal format. `/healthz` validates configuration and performs a bounded gateway ping so Compose readiness reflects the host's only upstream. The browser never connects to the gRPC gateway directly. +The host owns the `/` landing page and `//request/` detail route, HTTP Basic authentication, server-side Connect transport, readiness checks, and environment validation. Request IDs remain readable and are passed to the gateway without interpreting their internal format. `/healthz` validates configuration and performs a bounded gateway ping so Compose readiness reflects the host's only upstream. The browser never connects to the gRPC gateway directly. -`/` lists the configured queues instead of redirecting to the demo queue. Request detail defaults to Summary and uses `?view=history` for shareable History navigation. The host supplies Next-specific refresh completion, dynamic rendering, authentication on every protected route, and CSS; none of those framework decisions live in the reusable library. +`/` fetches configured queues through the gateway's `ListQueues` RPC, including queues with no requests. Queue, request, and change routes validate queue names against the same discovered catalog, with no demo-only allowlist. Discovery errors remain errors rather than being reported as missing queues. Request detail defaults to Summary and uses `?view=history` for shareable History navigation. The host supplies Next-specific refresh completion, dynamic rendering, authentication on every protected route, and CSS; none of those framework decisions live in the reusable library. GitHub, Phabricator, and git change pages mirror the canonical change URI's authority/path. The host adapter validates those provider-specific paths; React components do not parse URIs. Fake-provider `sq-files`/`sq-fake` query hints remain in the submitted backend values but are excluded from UI labels and navigation URLs. diff --git a/web/service/submitqueue/src/app/(protected)/[queue]/change/[...reference]/page.tsx b/web/service/submitqueue/src/app/(protected)/[queue]/change/[...reference]/page.tsx index 7ebe3ab35..ad5fd6a2a 100644 --- a/web/service/submitqueue/src/app/(protected)/[queue]/change/[...reference]/page.tsx +++ b/web/service/submitqueue/src/app/(protected)/[queue]/change/[...reference]/page.tsx @@ -4,7 +4,7 @@ import { connection } from "next/server"; import { notFound, redirect } from "next/navigation"; import { NextRefresh } from "../../../../../components/next-refresh"; import { ChangeView } from "../../../../../components/change-view"; -import { DEMO_QUEUE } from "../../../../../server/config"; +import { loadConfiguredQueue } from "../../../../../server/queues"; import { parseChangePath, changeHref } from "../../../../../server/change"; import { readChangeSubmissions } from "../../../../../server/change-reader"; import { resolveGatewayClient } from "../../../../../server/gateway"; @@ -25,7 +25,11 @@ export default async function ChangePage({ params, searchParams }: { const path = await params; const queue = decodePathSegment(decodeURIComponent(path.queue)); const reference = parseChangePath(path.reference.map(decodeURIComponent)); - if (queue !== DEMO_QUEUE || reference === null) { + if (reference === null) { + notFound(); + } + const configured = await loadConfiguredQueue(queue); + if (configured.ok && !configured.data) { notFound(); } const search = await searchParams; @@ -42,7 +46,7 @@ export default async function ChangePage({ params, searchParams }: { redirect(latestHref); } const requestWindow = pageWindow ?? defaultRequestWindow(); - const result = await loadChangeSubmissions( + const result = configured.ok ? await loadChangeSubmissions( async () => { const page = await readChangeSubmissions(resolveGatewayClient(), queue, reference, requestWindow); return { @@ -62,7 +66,7 @@ export default async function ChangePage({ params, searchParams }: { window: scansQueue ? { fromMs: requestWindow.fromMs, toMs: requestWindow.toMs } : null, }, { diagnostics: gatewayDiagnostics }, - ); + ) : configured; return
{result.ok ? : }
decodePathSegment(decodeURIComponent(value))).join("/"); @@ -49,14 +53,14 @@ export default async function RequestPage({ redirect(paths.request(queue, sqid, { view })); } - const result = await loadRequestDetail( + const result = configured.ok ? await loadRequestDetail( resolveDemoGateway, { queue, sqid, }, { diagnostics: gatewayDiagnostics }, - ); + ) : configured; if (!result.ok && result.error.kind === "not-found") { notFound(); diff --git a/web/service/submitqueue/src/app/(protected)/page.tsx b/web/service/submitqueue/src/app/(protected)/page.tsx index 4f2208314..8149c3f53 100644 --- a/web/service/submitqueue/src/app/(protected)/page.tsx +++ b/web/service/submitqueue/src/app/(protected)/page.tsx @@ -1,7 +1,10 @@ import { connection } from "next/server"; -import { QueueDirectory } from "@submitqueue/web-submitqueue"; +import { ErrorState, QueueDirectory } from "@submitqueue/web-submitqueue"; +import { loadQueueDirectory } from "@submitqueue/web-submitqueue/server"; -import { HOST_QUEUES } from "../../server/config"; +import { resolveGatewayClient } from "../../server/gateway"; +import { gatewayDiagnostics } from "../../server/diagnostics"; +import { NextRefresh } from "../../components/next-refresh"; import { requireAuthorization } from "../../server/request-auth"; export const dynamic = "force-dynamic"; @@ -10,5 +13,12 @@ export const revalidate = 0; export default async function HomePage() { await connection(); await requireAuthorization(); - return
; + const result = await loadQueueDirectory(resolveGatewayClient(), { diagnostics: gatewayDiagnostics }); + return
+ {result.ok ? : } +
+
; } diff --git a/web/service/submitqueue/src/app/not-found.tsx b/web/service/submitqueue/src/app/not-found.tsx index 49e5178ab..113f734ae 100644 --- a/web/service/submitqueue/src/app/not-found.tsx +++ b/web/service/submitqueue/src/app/not-found.tsx @@ -6,7 +6,7 @@ export default function NotFound() {

Request lookup

Request not found

-

The request may not exist in the demo queue or may have expired.

+

The queue or request may not exist, or its retained data may have expired.

Return to queues diff --git a/web/service/submitqueue/src/server/config.ts b/web/service/submitqueue/src/server/config.ts index 0d90a6f7d..bd540ffb9 100644 --- a/web/service/submitqueue/src/server/config.ts +++ b/web/service/submitqueue/src/server/config.ts @@ -1,9 +1,5 @@ import "server-only"; -export const DEMO_QUEUE = "demo-queue"; -export const HOST_QUEUES = [ - { name: DEMO_QUEUE, description: "Local read-only demo backed by the SubmitQueue gateway" }, -]; export const GATEWAY_DEADLINE_MS = 5_000; export const GATEWAY_HEALTH_DEADLINE_MS = 1_000; diff --git a/web/service/submitqueue/src/server/queues.test.ts b/web/service/submitqueue/src/server/queues.test.ts new file mode 100644 index 000000000..e6dd8aa90 --- /dev/null +++ b/web/service/submitqueue/src/server/queues.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it, vi } from "vitest"; +import { createFakeGatewayReader } from "@submitqueue/web-submitqueue/testing"; +import { Code, ConnectError } from "@connectrpc/connect"; + +const resolver = vi.hoisted(() => ({ resolveGatewayClient: vi.fn() })); +vi.mock("./gateway", () => resolver); + +import { loadConfiguredQueue } from "./queues"; + +describe("gateway-backed host queue validation", () => { + it("accepts configured queues other than the former demo-only queue", async () => { + resolver.resolveGatewayClient.mockReturnValue(createFakeGatewayReader({ + queues: [{ name: "empty-queue" }], + })); + expect(await loadConfiguredQueue("empty-queue")).toEqual({ ok: true, data: true }); + expect(await loadConfiguredQueue("unknown")).toEqual({ ok: true, data: false }); + }); + + it("does not treat a failed queue discovery as a missing queue", async () => { + resolver.resolveGatewayClient.mockReturnValue(createFakeGatewayReader({ + queuesError: new ConnectError("unavailable", Code.Unavailable), + })); + expect(await loadConfiguredQueue("demo-queue")).toMatchObject({ + ok: false, error: { kind: "transient", retryable: true }, + }); + }); +}); diff --git a/web/service/submitqueue/src/server/queues.ts b/web/service/submitqueue/src/server/queues.ts new file mode 100644 index 000000000..f347387e6 --- /dev/null +++ b/web/service/submitqueue/src/server/queues.ts @@ -0,0 +1,13 @@ +import "server-only"; + +import type { LoadResult } from "@submitqueue/web-submitqueue"; +import { loadQueueDirectory } from "@submitqueue/web-submitqueue/server"; +import { gatewayDiagnostics } from "./diagnostics"; +import { resolveGatewayClient } from "./gateway"; + +export async function loadConfiguredQueue(queue: string): Promise> { + const directory = await loadQueueDirectory(resolveGatewayClient(), { diagnostics: gatewayDiagnostics }); + return directory.ok + ? { ok: true, data: directory.data.some(value => value.name === queue) } + : directory; +} diff --git a/web/submitqueue/README.md b/web/submitqueue/README.md index 3a39f0399..a8270dec0 100644 --- a/web/submitqueue/README.md +++ b/web/submitqueue/README.md @@ -21,4 +21,6 @@ Request list/detail result views retain the last successful snapshot on transien The package-consumer checks typecheck without Next.js and reject Next.js dependencies or imports in the emitted library. +`loadQueueDirectory` maps the gateway's `ListQueues` response into serializable queue models and sanitized errors. It uses a separate structural queue-reader interface so request-only adapters remain valid; the host supplies the client and decides where to render or validate the discovered catalog. + Normal queue/resource paths remain readable. Exceptional dot-only path segments use a visible `~` escape so browsers cannot normalize them away; `decodePathSegment` reverses that transport escape without interpreting an ID. Change-version navigation is an optional host callback over already supplied URLs, keeping router integration out of the library. diff --git a/web/submitqueue/src/components.tsx b/web/submitqueue/src/components.tsx index 6b761b162..d814ce6b4 100644 --- a/web/submitqueue/src/components.tsx +++ b/web/submitqueue/src/components.tsx @@ -40,10 +40,10 @@ function RequestChanges({ request, links = {}, labels = {} }: { export function QueueDirectory({ queues, basePath }: { queues: readonly QueueModel[]; basePath?: string }) { const paths = new WebPaths(basePath === undefined ? {} : { basePath }); return
-

Queues

Queues configured by this host

+

Queues

Available submit queues

{queues.length === 0 ?

No queues are configured.

:
    {queues.map(queue =>
  • -

    {queue.name}

    {queue.description}

    +

    {queue.name}

    {queue.description ?

    {queue.description}

    : null}
    • )}
}
; } diff --git a/web/submitqueue/src/server.test.ts b/web/submitqueue/src/server.test.ts index 874397e74..6b2fe6e7a 100644 --- a/web/submitqueue/src/server.test.ts +++ b/web/submitqueue/src/server.test.ts @@ -7,11 +7,43 @@ import { loadChangeSubmissions, loadRequestDetail, loadRequestList, + loadQueueDirectory, requestDetailIsComplete, requestDetailRefreshState, safeInt64ToNumber, } from "./server"; +describe("queue discovery mapping", () => { + it("lists gateway-configured queues, including ones without requests", async () => { + const reader = createFakeGatewayReader({ queues: [{ name: "demo-queue" }, { name: "empty-queue" }] }); + const result = await loadQueueDirectory(reader); + expect(result).toEqual({ + ok: true, data: [{ name: "demo-queue", description: "" }, { name: "empty-queue", description: "" }], + }); + }); + + it("returns an empty directory when the gateway has no configured queues", async () => { + expect(await loadQueueDirectory(createFakeGatewayReader({ queues: [] }))).toEqual({ ok: true, data: [] }); + }); + + it("classifies discovery failures without leaking the gateway message", async () => { + const onGatewayError = vi.fn(); + const cause = new ConnectError("private upstream detail", Code.Unavailable); + const result = await loadQueueDirectory( + createFakeGatewayReader({ queuesError: cause }), + { diagnostics: { onGatewayError } }, + ); + expect(result).toMatchObject({ ok: false, error: { kind: "transient", retryable: true } }); + expect(JSON.stringify(result)).not.toContain("private upstream detail"); + expect(onGatewayError).toHaveBeenCalledWith(expect.objectContaining({ operation: "queues", cause })); + }); + + it.each([{ queues: [{ name: "" }] }, { queues: [{ name: "same" }, { name: "same" }] }])("rejects malformed discovery data", async ({ queues }) => { + const result = await loadQueueDirectory(createFakeGatewayReader({ queues })); + expect(result).toMatchObject({ ok: false, error: { kind: "internal" } }); + }); +}); + describe("server presentation mapping", () => { beforeEach(() => vi.clearAllMocks()); diff --git a/web/submitqueue/src/server.ts b/web/submitqueue/src/server.ts index 6e852cd5b..677be0fa8 100644 --- a/web/submitqueue/src/server.ts +++ b/web/submitqueue/src/server.ts @@ -9,6 +9,7 @@ import type { RequestDetailModel, RequestListModel, RequestSummaryModel, + QueueModel, WebError, } from "./models.js"; import { isTerminalStatus } from "./status.js"; @@ -71,7 +72,14 @@ export interface GatewayReader { export type GatewayResolver = (queue: string) => GatewayReader; -export type GatewayOperation = "list" | "summary" | "history" | "change"; +export interface GatewayQueueReader { + listQueues( + input: Record, + options?: GatewayCallOptions, + ): Promise<{ queues: readonly { name: string }[] }>; +} + +export type GatewayOperation = "list" | "summary" | "history" | "change" | "queues"; export interface GatewayDiagnostic { operation: GatewayOperation; @@ -195,6 +203,28 @@ function reportGatewayError( } } +export async function loadQueueDirectory( + gateway: GatewayQueueReader, + options: GatewayLoadOptions = {}, +): Promise> { + try { + const response = await gateway.listQueues({}); + const names = new Set(); + const queues: QueueModel[] = []; + for (const queue of response.queues) { + if (!queue.name || names.has(queue.name)) { + return { ok: false, error: INTERNAL_ERROR }; + } + names.add(queue.name); + queues.push({ name: queue.name, description: "" }); + } + return { ok: true, data: queues }; + } catch (error) { + reportGatewayError(options, "queues", "", null, error); + return { ok: false, error: classifyGatewayError(error) }; + } +} + export function requestDetailIsComplete(model: RequestDetailModel): boolean { if (model.historyError !== null || !isTerminalStatus(model.request.status)) { return false; diff --git a/web/submitqueue/src/testing.ts b/web/submitqueue/src/testing.ts index c9e5f5bd8..f55ecd568 100644 --- a/web/submitqueue/src/testing.ts +++ b/web/submitqueue/src/testing.ts @@ -1,6 +1,7 @@ import type { GatewayHistoryEvent, GatewayReader, + GatewayQueueReader, GatewayRequestSummary, } from "./server.js"; @@ -36,6 +37,8 @@ export function gatewayHistoryFixture( } export interface FakeGatewayReaderOptions { + queues?: readonly { name: string }[]; + queuesError?: unknown; requests?: readonly GatewayRequestSummary[]; nextPageToken?: string; summary?: GatewayRequestSummary; @@ -45,9 +48,15 @@ export interface FakeGatewayReaderOptions { historyError?: unknown; } -export function createFakeGatewayReader(options: FakeGatewayReaderOptions = {}): GatewayReader { +export function createFakeGatewayReader(options: FakeGatewayReaderOptions = {}): GatewayReader & GatewayQueueReader { const summary = options.summary ?? options.requests?.[0] ?? gatewayRequestFixture(); return { + async listQueues() { + if (options.queuesError !== undefined) { + throw options.queuesError; + } + return { queues: options.queues ?? [{ name: "demo-queue" }] }; + }, async list() { if (options.listError !== undefined) { throw options.listError; @@ -72,4 +81,4 @@ export function createFakeGatewayReader(options: FakeGatewayReaderOptions = {}): }; } -export type { GatewayHistoryEvent, GatewayReader, GatewayRequestSummary } from "./server.js"; +export type { GatewayHistoryEvent, GatewayReader, GatewayQueueReader, GatewayRequestSummary } from "./server.js"; diff --git a/web/test/e2e/submitqueue.spec.ts b/web/test/e2e/submitqueue.spec.ts index 701049499..d0f95644c 100644 --- a/web/test/e2e/submitqueue.spec.ts +++ b/web/test/e2e/submitqueue.spec.ts @@ -55,6 +55,23 @@ test("challenges anonymous requests", async () => { expect(response.headers.get("www-authenticate")).toContain("Basic"); }); +test("discovers gateway queues including empty queues and rejects unknown queues", async ({ page }) => { + const expected = (await gateway.listQueues({})).queues.map(value => value.name); + expect(expected).toContain(queue); + const emptyQueue = expected.find(name => name !== queue)!; + expect(emptyQueue).toBeDefined(); + await page.goto("/"); + await expect(page.locator(".sq-directory h2 a")).toHaveText(expected); + expect((await new AxeBuilder({ page }).analyze()).violations).toEqual([]); + await page.getByRole("link", { name: emptyQueue, exact: true }).click(); + await expect(page.getByRole("heading", { name: emptyQueue, exact: true })).toBeVisible(); + await expect(page.getByText("No requests were received in this window.")).toBeVisible(); + await page.goto(`/${emptyQueue}/request/999999`); + await expect(page.getByRole("heading", { name: "Request not found", exact: true })).toBeVisible(); + await page.goto("/not-a-configured-queue"); + await expect(page.getByRole("heading", { name: "Request not found", exact: true })).toBeVisible(); +}); + test("shows newest requests and an accessible readable detail history", async ({ page }) => { await page.goto("/"); await expect(page.getByRole("heading", { name: "Queues", exact: true })).toBeVisible();