swarm-ui: keep the issue report's table, errors and repo filter consistent

A failed report dropped its error under the previous repo's rows, the
repo list and the report shared one error slot, a remembered repo that
the list lacked left the select on all repos, and the report URL did not
encode the repo path. Each request now has its own error, a failed report
clears the rows, the remembered repo keeps an option, and the path parts
are encoded.

frontend/packages/swarm-ui/src/pages/IssueReportPage.tsx

@@ -35,6 +35,7 @@ import { useLocalSetting } from "@hive/shared/settings-storage.js";
 import { Panel } from "../ui/panel/Panel.js";
 import { SelectField } from "../ui/select-field/SelectField.js";
 import { Table, type TableColumn } from "../ui/table/Table.js";
+import { ALL_REPOS, reportUrl, repoOptions } from "./issue-report-repo.js";
 import "./IssueReportPage.css";
 
 interface IssueReportRow {
@@ -63,10 +64,6 @@ type SortKey =
   | "transitively_blocks_count";
 type SortDir = "asc" | "desc";
 
-// Sentinel `<select>` value for "every repo" — `''` can't collide with a
-// real `owner/name` value, which always contains a slash.
-const ALL_REPOS = "";
-
 // One key per persisted control, namespaced like the theme/motion keys
 // (`swarm-ui:issue-report:…`) so nothing else on the page — or a future
 // page — collides with these by accident.
@@ -79,12 +76,6 @@ const SORT_DIR_KEY = "swarm-ui:issue-report:sort-dir";
 // e.g. title/assignees/blocked).
 const TABLE_FILTERS_KEY = "swarm-ui:issue-report:table-filters";
 
-function splitRepo(repo: string): { org: string; name: string } | null {
-  const i = repo.indexOf("/");
-  if (i < 0) return null;
-  return { org: repo.slice(0, i), name: repo.slice(i + 1) };
-}
-
 function compareRows(
   a: IssueReportRow,
   b: IssueReportRow,
@@ -143,6 +134,9 @@ export function IssueReportPage() {
     ALL_REPOS,
   );
   const [rows, setRows] = useState<IssueReportRow[] | null>(null);
+  // One slot per request, so a report that loads does not clear a repo
+  // list that failed.
+  const [reposError, setReposError] = useState<ProblemDetails | null>(null);
   const [error, setError] = useState<ProblemDetails | null>(null);
   const [loading, setLoading] = useState(false);
   const [hideBlocked, setHideBlocked] = useLocalSetting(
@@ -160,19 +154,19 @@ export function IssueReportPage() {
   // visits is rare enough that a manual reload covers it. Same
   // `cancelled` guard as the repo-filter effect below, so an unmount
   // mid-flight (navigating away before this resolves) doesn't call
-  // `setRepos`/`setError` on a component that's already gone.
+  // `setRepos`/`setReposError` on a component that's already gone.
   useEffect(() => {
     let cancelled = false;
     (async () => {
       const r = await fetch("/api/repos");
       if (!r.ok) {
-        if (!cancelled) setError(await readApiError(r));
+        if (!cancelled) setReposError(await readApiError(r));
         return;
       }
       const data = (await r.json()) as string[];
       if (!cancelled) setRepos(data);
     })().catch((e: unknown) => {
-      if (!cancelled) setError({ detail: String(e) });
+      if (!cancelled) setReposError({ detail: String(e) });
     });
     return () => {
       cancelled = true;
@@ -185,14 +179,17 @@ export function IssueReportPage() {
   useEffect(() => {
     let cancelled = false;
     setLoading(true);
-    const split = repoFilter ? splitRepo(repoFilter) : null;
-    const url = split
-      ? `/api/repos/${split.org}/${split.name}/issue-report`
-      : "/api/issue-report";
+    // A failed report drops the rows, so the table never shows another
+    // repo's issues under this repo's error.
+    const fail = (problem: ProblemDetails) => {
+      setRows(null);
+      setError(problem);
+    };
     (async () => {
-      const r = await fetch(url);
+      const r = await fetch(reportUrl(repoFilter));
       if (!r.ok) {
-        if (!cancelled) setError(await readApiError(r));
+        const problem = await readApiError(r);
+        if (!cancelled) fail(problem);
         return;
       }
       const data = (await r.json()) as IssueReportRow[];
@@ -201,7 +198,7 @@ export function IssueReportPage() {
       setError(null);
     })()
       .catch((e: unknown) => {
-        if (!cancelled) setError({ detail: String(e) });
+        if (!cancelled) fail({ detail: String(e) });
       })
       .finally(() => {
         if (!cancelled) setLoading(false);
@@ -379,10 +376,7 @@ export function IssueReportPage() {
           label="repo"
           value={repoFilter}
           onChange={setRepoFilter}
-          options={[
-            { value: ALL_REPOS, label: "all repos" },
-            ...(repos ?? []).map((r) => ({ value: r, label: r })),
-          ]}
+          options={repoOptions(repos, repoFilter)}
         />
         <label class="issue-report-toggle">
           <input
@@ -395,6 +389,12 @@ export function IssueReportPage() {
           hide blocked (open dependency)
         </label>
       </div>
+      {reposError ? (
+        <ApiErrorPanel
+          context="failed to load the repo list"
+          problem={reposError}
+        />
+      ) : null}
       {error ? (
         <ApiErrorPanel
           context="failed to load the issue report"

frontend/packages/swarm-ui/src/pages/issue-report-repo.ts

@@ -0,0 +1,42 @@
+// Pure helpers for the issue report's repo filter: the report URL and the
+// select's options. Kept out of the page so the cases are unit tested.
+
+/** Select value for "every repo". It cannot collide with a real
+ * `owner/name`, which always has a slash. */
+export const ALL_REPOS = "";
+
+export interface RepoOption {
+  value: string;
+  label: string;
+}
+
+/** The report URL for a repo filter. The path parts are encoded, so a name
+ * with `?`, `#` or `%` stays one path segment. A filter without a slash
+ * is not a repo, so it reads as all repos. */
+export function reportUrl(repoFilter: string): string {
+  const i = repoFilter.indexOf("/");
+  if (i < 0) return "/api/issue-report";
+  const org = encodeURIComponent(repoFilter.slice(0, i));
+  const name = encodeURIComponent(repoFilter.slice(i + 1));
+  return `/api/repos/${org}/${name}/issue-report`;
+}
+
+/** The select's options. A remembered repo that the list lacks keeps its
+ * own option, so the select shows what the table is filtered to. Once the
+ * list has loaded, the label says why the repo is not in it. */
+export function repoOptions(
+  repos: string[] | null,
+  selected: string,
+): RepoOption[] {
+  const options: RepoOption[] = [
+    { value: ALL_REPOS, label: "all repos" },
+    ...(repos ?? []).map((r) => ({ value: r, label: r })),
+  ];
+  if (selected !== ALL_REPOS && !(repos ?? []).includes(selected)) {
+    options.push({
+      value: selected,
+      label: repos === null ? selected : `${selected} (no open issues)`,
+    });
+  }
+  return options;
+}

frontend/packages/swarm-ui/test/issue-report-repo.test.mjs

@@ -0,0 +1,55 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import {
+  ALL_REPOS,
+  reportUrl,
+  repoOptions,
+} from "../src/pages/issue-report-repo.ts";
+
+test("reportUrl: no repo means the combined report", () => {
+  assert.equal(reportUrl(ALL_REPOS), "/api/issue-report");
+});
+
+test("reportUrl: a filter without a slash is not a repo", () => {
+  assert.equal(reportUrl("nope"), "/api/issue-report");
+});
+
+test("reportUrl: org and name become two path segments", () => {
+  assert.equal(
+    reportUrl("hyperhive/hyperhive"),
+    "/api/repos/hyperhive/hyperhive/issue-report",
+  );
+});
+
+test("reportUrl: reserved characters in a name are encoded", () => {
+  assert.equal(
+    reportUrl("org/a?b#c%d"),
+    "/api/repos/org/a%3Fb%23c%25d/issue-report",
+  );
+});
+
+test("repoOptions: all repos comes first, then the list", () => {
+  assert.deepEqual(repoOptions(["a/x", "b/y"], ALL_REPOS), [
+    { value: "", label: "all repos" },
+    { value: "a/x", label: "a/x" },
+    { value: "b/y", label: "b/y" },
+  ]);
+});
+
+test("repoOptions: a listed selection adds nothing", () => {
+  assert.equal(repoOptions(["a/x"], "a/x").length, 2);
+});
+
+test("repoOptions: a remembered repo keeps an option before the list loads", () => {
+  assert.deepEqual(repoOptions(null, "a/x"), [
+    { value: "", label: "all repos" },
+    { value: "a/x", label: "a/x" },
+  ]);
+});
+
+test("repoOptions: a remembered repo missing from the list says why", () => {
+  assert.deepEqual(repoOptions(["b/y"], "a/x").at(-1), {
+    value: "a/x",
+    label: "a/x (no open issues)",
+  });
+});