From ca3b059a62c10c42392470157e12b26fdf5d91cb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C5=ABdolfs=20O=C5=A1i=C5=86=C5=A1?= <rudolfs@osins.org>
Date: Wed, 30 Sep 2026 15:04:50 +0200
Subject: [PATCH] Move CI job grouping into lib

The status chip's grouping, dedupe and summary rules were untested inside
the component, and a wrong CI status is very visible.
---
 src/components/JobCob.svelte | 164 ++---------------------------
 src/lib/ciJobs.ts            | 161 ++++++++++++++++++++++++++++
 tests/unit/ciJobs.test.ts    | 197 +++++++++++++++++++++++++++++++++++
 3 files changed, 366 insertions(+), 156 deletions(-)
 create mode 100644 src/lib/ciJobs.ts
 create mode 100644 tests/unit/ciJobs.test.ts

diff --git a/src/components/JobCob.svelte b/src/components/JobCob.svelte
index 0774962ed..c77efd370 100644
--- a/src/components/JobCob.svelte
+++ b/src/components/JobCob.svelte
@@ -1,12 +1,15 @@
 <script lang="ts">
-  import type { Author } from "@bindings/cob/Author";
-  import type { Job } from "@bindings/repo/Job";
-  import type { Run } from "@bindings/repo/Run";
-
   import { SvelteSet } from "svelte/reactivity";
 
+  import type { RunView, Status } from "@app/lib/ciJobs";
+  import {
+    aggregateStatus,
+    groupJobs,
+    statusLabel,
+    totalCounts,
+  } from "@app/lib/ciJobs";
   import { cachedListJobs } from "@app/lib/invoke";
-  import { authorForNodeId, safeHttpUrl } from "@app/lib/utils";
+  import { authorForNodeId } from "@app/lib/utils";
 
   import Button from "@app/components/Button.svelte";
   import Icon from "@app/components/Icon.svelte";
@@ -22,160 +25,9 @@
 
   const { commit, rid, styleHeight = "2rem" }: Props = $props();
 
-  type Status = Run["status"];
-
-  type RunView = {
-    run: Run;
-    label: string;
-    safeLog: string | undefined;
-  };
-
-  type HostGroup = {
-    host: string;
-    runs: RunView[];
-  };
-
-  type Counts = { succeeded: number; failed: number; started: number };
-
-  type NodeGroup = {
-    nodeKey: string;
-    author: Author;
-    hosts: HostGroup[];
-    flatRuns?: RunView[];
-    inlineHost?: string;
-    counts: Counts;
-    status: Status;
-  };
-
   const collapsed = new SvelteSet<string>();
   let popoverExpanded: boolean = $state(false);
 
-  function parseUrl(s: string): URL | undefined {
-    try {
-      return new URL(s);
-    } catch {
-      return undefined;
-    }
-  }
-
-  function runLabel(run: Run, url: URL | undefined): string {
-    if (url && url.host === "github.com") {
-      const m = url.pathname.match(/\/actions\/runs\/(\d+)/);
-      if (m) return m[1];
-    }
-    return run.runId.slice(0, 8);
-  }
-
-  function isTerminal(status: Status): boolean {
-    return status === "succeeded" || status === "failed";
-  }
-
-  function aggregateStatus(c: Counts): Status {
-    if (c.failed) return "failed";
-    if (c.started) return "started";
-    return "succeeded";
-  }
-
-  function statusLabel(c: Counts): string {
-    const parts: string[] = [];
-    if (c.succeeded) parts.push(`${c.succeeded} passed`);
-    if (c.failed) parts.push(`${c.failed} failed`);
-    if (c.started) parts.push(`${c.started} running`);
-    return parts.join(" · ");
-  }
-
-  function totalCounts(groups: NodeGroup[]): Counts {
-    const total: Counts = { succeeded: 0, failed: 0, started: 0 };
-    for (const g of groups) {
-      total.succeeded += g.counts.succeeded;
-      total.failed += g.counts.failed;
-      total.started += g.counts.started;
-    }
-    return total;
-  }
-
-  function groupJobs(jobs: Job[]): NodeGroup[] {
-    const byNode: Record<
-      string,
-      {
-        author: Author;
-        byHost: Record<string, RunView[]>;
-        seen: Map<string, RunView>;
-      }
-    > = {};
-    const nodeOrder: string[] = [];
-    for (const job of jobs) {
-      for (const run of job.runs) {
-        const url = parseUrl(run.log);
-        const host = url && url.host ? url.host : "(unknown host)";
-        const safeLog =
-          url?.host !== "no.url.example.com" ? safeHttpUrl(run.log) : undefined;
-        const view: RunView = {
-          run,
-          label: runLabel(run, url),
-          safeLog,
-        };
-        const key = run.node.did;
-        let entry = byNode[key];
-        if (!entry) {
-          entry = { author: run.node, byHost: {}, seen: new Map() };
-          byNode[key] = entry;
-          nodeOrder.push(key);
-        }
-        const existing = entry.seen.get(run.runId);
-        if (existing) {
-          // The same run can appear under multiple job COBs. Keep one view
-          // per runId, but let a terminal status override a stale "started"
-          // one when COBs polled at different times disagree. We mutate the
-          // existing view in place because it is the same object referenced
-          // from its host bucket below.
-          if (!isTerminal(existing.run.status) && isTerminal(run.status)) {
-            existing.run = run;
-            existing.label = view.label;
-            existing.safeLog = view.safeLog;
-          }
-          continue;
-        }
-        entry.seen.set(run.runId, view);
-        const bucket = entry.byHost[host] ?? (entry.byHost[host] = []);
-        bucket.push(view);
-      }
-    }
-    return nodeOrder.map(nodeKey => {
-      const { author, byHost } = byNode[nodeKey];
-      const hosts: HostGroup[] = Object.entries(byHost).map(([host, runs]) => ({
-        host,
-        runs,
-      }));
-      const counts: Counts = { succeeded: 0, failed: 0, started: 0 };
-      for (const { runs } of hosts) {
-        for (const v of runs) counts[v.run.status]++;
-      }
-      const group: NodeGroup = {
-        nodeKey,
-        author,
-        hosts,
-        counts,
-        status: aggregateStatus(counts),
-      };
-      if (hosts.length === 1) {
-        group.flatRuns = hosts[0].runs;
-        if (!aliasMatchesHost(author.alias, hosts[0].host)) {
-          group.inlineHost = hosts[0].host;
-        }
-      }
-      return group;
-    });
-  }
-
-  function aliasMatchesHost(alias: string | undefined, host: string): boolean {
-    if (!alias) return false;
-    const a = alias.toLowerCase();
-    if (host === a) return true;
-    if (host.endsWith("." + a)) return true;
-    return host.split(".").includes(a);
-  }
-
   // Held in a `$derived`; see `ReviewCodeThread`. The TTL here means a re-run
   // would refetch and blank the chip.
   const jobsPromise = $derived(cachedListJobs(rid, commit).catch(() => []));
diff --git a/src/lib/ciJobs.ts b/src/lib/ciJobs.ts
new file mode 100644
index 000000000..b6d7c8965
--- /dev/null
+++ b/src/lib/ciJobs.ts
@@ -0,0 +1,161 @@
+import type { Author } from "@bindings/cob/Author";
+import type { Job } from "@bindings/repo/Job";
+import type { Run } from "@bindings/repo/Run";
+
+import { safeHttpUrl } from "@app/lib/utils";
+
+// How the CI status chip groups and summarises a commit's job runs.
+
+export type Status = Run["status"];
+
+export type RunView = {
+  run: Run;
+  label: string;
+  safeLog: string | undefined;
+};
+
+export type HostGroup = {
+  host: string;
+  runs: RunView[];
+};
+
+export type Counts = { succeeded: number; failed: number; started: number };
+
+export type NodeGroup = {
+  nodeKey: string;
+  author: Author;
+  hosts: HostGroup[];
+  flatRuns?: RunView[];
+  inlineHost?: string;
+  counts: Counts;
+  status: Status;
+};
+
+function parseUrl(s: string): URL | undefined {
+  try {
+    return new URL(s);
+  } catch {
+    return undefined;
+  }
+}
+
+export function runLabel(run: Run, url: URL | undefined): string {
+  if (url && url.host === "github.com") {
+    const m = url.pathname.match(/\/actions\/runs\/(\d+)/);
+    if (m) return m[1];
+  }
+  return run.runId.slice(0, 8);
+}
+
+export function isTerminal(status: Status): boolean {
+  return status === "succeeded" || status === "failed";
+}
+
+export function aggregateStatus(c: Counts): Status {
+  if (c.failed) return "failed";
+  if (c.started) return "started";
+  return "succeeded";
+}
+
+export function statusLabel(c: Counts): string {
+  const parts: string[] = [];
+  if (c.succeeded) parts.push(`${c.succeeded} passed`);
+  if (c.failed) parts.push(`${c.failed} failed`);
+  if (c.started) parts.push(`${c.started} running`);
+  return parts.join(" · ");
+}
+
+export function totalCounts(groups: NodeGroup[]): Counts {
+  const total: Counts = { succeeded: 0, failed: 0, started: 0 };
+  for (const g of groups) {
+    total.succeeded += g.counts.succeeded;
+    total.failed += g.counts.failed;
+    total.started += g.counts.started;
+  }
+  return total;
+}
+
+export function groupJobs(jobs: Job[]): NodeGroup[] {
+  const byNode: Record<
+    string,
+    {
+      author: Author;
+      byHost: Record<string, RunView[]>;
+      seen: Map<string, RunView>;
+    }
+  > = {};
+  const nodeOrder: string[] = [];
+  for (const job of jobs) {
+    for (const run of job.runs) {
+      const url = parseUrl(run.log);
+      const host = url && url.host ? url.host : "(unknown host)";
+      const safeLog =
+        url?.host !== "no.url.example.com" ? safeHttpUrl(run.log) : undefined;
+      const view: RunView = {
+        run,
+        label: runLabel(run, url),
+        safeLog,
+      };
+      const key = run.node.did;
+      let entry = byNode[key];
+      if (!entry) {
+        entry = { author: run.node, byHost: {}, seen: new Map() };
+        byNode[key] = entry;
+        nodeOrder.push(key);
+      }
+      const existing = entry.seen.get(run.runId);
+      if (existing) {
+        // The same run can appear under multiple job COBs. Keep one view
+        // per runId, but let a terminal status override a stale "started"
+        // one when COBs polled at different times disagree. We mutate the
+        // existing view in place because it is the same object referenced
+        // from its host bucket below.
+        if (!isTerminal(existing.run.status) && isTerminal(run.status)) {
+          existing.run = run;
+          existing.label = view.label;
+          existing.safeLog = view.safeLog;
+        }
+        continue;
+      }
+      entry.seen.set(run.runId, view);
+      const bucket = entry.byHost[host] ?? (entry.byHost[host] = []);
+      bucket.push(view);
+    }
+  }
+  return nodeOrder.map(nodeKey => {
+    const { author, byHost } = byNode[nodeKey];
+    const hosts: HostGroup[] = Object.entries(byHost).map(([host, runs]) => ({
+      host,
+      runs,
+    }));
+    const counts: Counts = { succeeded: 0, failed: 0, started: 0 };
+    for (const { runs } of hosts) {
+      for (const v of runs) counts[v.run.status]++;
+    }
+    const group: NodeGroup = {
+      nodeKey,
+      author,
+      hosts,
+      counts,
+      status: aggregateStatus(counts),
+    };
+    if (hosts.length === 1) {
+      group.flatRuns = hosts[0].runs;
+      if (!aliasMatchesHost(author.alias, hosts[0].host)) {
+        group.inlineHost = hosts[0].host;
+      }
+    }
+    return group;
+  });
+}
+
+export function aliasMatchesHost(
+  alias: string | undefined,
+  host: string,
+): boolean {
+  if (!alias) return false;
+  const a = alias.toLowerCase();
+  if (host === a) return true;
+  if (host.endsWith("." + a)) return true;
+  return host.split(".").includes(a);
+}
diff --git a/tests/unit/ciJobs.test.ts b/tests/unit/ciJobs.test.ts
new file mode 100644
index 000000000..3795eabcb
--- /dev/null
+++ b/tests/unit/ciJobs.test.ts
@@ -0,0 +1,197 @@
+import type { Job } from "@bindings/repo/Job";
+import type { Run } from "@bindings/repo/Run";
+
+import { describe, expect, test } from "vitest";
+
+import {
+  aggregateStatus,
+  aliasMatchesHost,
+  groupJobs,
+  isTerminal,
+  runLabel,
+  statusLabel,
+  totalCounts,
+} from "@app/lib/ciJobs";
+
+import { author } from "./support/cobs";
+
+const ci = author("ci", "ci.example.com");
+const other = author("other", "other");
+
+function run(runId: string, props: Partial<Run> = {}): Run {
+  return {
+    runId,
+    node: ci,
+    status: "succeeded",
+    log: `https://ci.example.com/runs/${runId}`,
+    ...props,
+  };
+}
+
+function job(runs: Run[]): Job {
+  return { jobId: "job", commit: "c", runs };
+}
+
+describe("run details", () => {
+  test("runLabel uses the GitHub Actions run number when there is one", () => {
+    expect(
+      runLabel(
+        run("abcdefghijk"),
+        new URL("https://github.com/o/r/actions/runs/12345/job/1"),
+      ),
+    ).toBe("12345");
+  });
+
+  test("runLabel falls back to a short run id", () => {
+    expect(runLabel(run("abcdefghijk"), undefined)).toBe("abcdefgh");
+    expect(
+      runLabel(run("abcdefghijk"), new URL("https://github.com/o/r")),
+    ).toBe("abcdefgh");
+    expect(
+      runLabel(
+        run("abcdefghijk"),
+        new URL("https://ci.example.com/actions/runs/12345"),
+      ),
+    ).toBe("abcdefgh");
+  });
+
+  test.each([
+    ["succeeded", true],
+    ["failed", true],
+    ["started", false],
+  ] as const)("isTerminal(%j) is %j", (status, expected) => {
+    expect(isTerminal(status)).toBe(expected);
+  });
+});
+
+describe("summaries", () => {
+  test.each([
+    [{ succeeded: 1, failed: 1, started: 1 }, "failed"],
+    [{ succeeded: 1, failed: 0, started: 1 }, "started"],
+    [{ succeeded: 2, failed: 0, started: 0 }, "succeeded"],
+    [{ succeeded: 0, failed: 0, started: 0 }, "succeeded"],
+  ])("aggregateStatus(%j) is %j", (counts, expected) => {
+    expect(aggregateStatus(counts)).toBe(expected);
+  });
+
+  test("statusLabel names each non-zero count", () => {
+    expect(statusLabel({ succeeded: 2, failed: 1, started: 3 })).toBe(
+      "2 passed · 1 failed · 3 running",
+    );
+    expect(statusLabel({ succeeded: 0, failed: 1, started: 0 })).toBe(
+      "1 failed",
+    );
+    expect(statusLabel({ succeeded: 0, failed: 0, started: 0 })).toBe("");
+  });
+
+  test("totalCounts adds up every group", () => {
+    const groups = groupJobs([
+      job([
+        run("a"),
+        run("b", { status: "failed" }),
+        run("c", { node: other, status: "started" }),
+      ]),
+    ]);
+
+    expect(totalCounts(groups)).toEqual({
+      succeeded: 1,
+      failed: 1,
+      started: 1,
+    });
+  });
+});
+
+describe("aliasMatchesHost", () => {
+  test.each([
+    ["ci.example.com", "ci.example.com", true],
+    ["example.com", "ci.example.com", true],
+    ["ci", "ci.example.com", true],
+    ["CI", "ci.example.com", true],
+    ["other", "ci.example.com", false],
+    ["ample.com", "ci.example.com", false],
+    [undefined, "ci.example.com", false],
+  ])("alias %j on host %j is %j", (alias, host, expected) => {
+    expect(aliasMatchesHost(alias, host)).toBe(expected);
+  });
+});
+
+describe("groupJobs", () => {
+  test("groups runs by node in order of first appearance", () => {
+    const groups = groupJobs([
+      job([run("a"), run("b", { node: other })]),
+      job([run("c")]),
+    ]);
+
+    expect(groups.map(g => [g.nodeKey, g.counts.succeeded])).toEqual([
+      [ci.did, 2],
+      [other.did, 1],
+    ]);
+  });
+
+  test("groups a node's runs by log host", () => {
+    const [group] = groupJobs([
+      job([
+        run("a"),
+        run("b", { log: "https://github.com/o/r/actions/runs/7" }),
+        run("c", { log: "not a url" }),
+        run("d", { log: "mailto:ci@example.com" }),
+      ]),
+    ]);
+
+    expect(
+      group.hosts.map(h => [h.host, h.runs.map(r => r.run.runId)]),
+    ).toEqual([
+      ["ci.example.com", ["a"]],
+      ["github.com", ["b"]],
+      ["(unknown host)", ["c", "d"]],
+    ]);
+    expect(group.flatRuns).toBeUndefined();
+  });
+
+  test("lists a single host's runs flat, naming the host unless the alias does", () => {
+    const [named] = groupJobs([job([run("a")])]);
+    expect(named.flatRuns?.map(r => r.run.runId)).toEqual(["a"]);
+    expect(named.inlineHost).toBeUndefined();
+
+    const [unnamed] = groupJobs([job([run("a", { node: other })])]);
+    expect(unnamed.inlineHost).toBe("ci.example.com");
+  });
+
+  test("keeps one view per run, letting a finished status win", () => {
+    const [group] = groupJobs([
+      job([run("a", { status: "started" })]),
+      job([run("a", { status: "failed" })]),
+      job([run("a", { status: "started" })]),
+    ]);
+
+    expect(group.hosts[0].runs.map(r => r.run.status)).toEqual(["failed"]);
+    expect(group.status).toBe("failed");
+  });
+
+  test("keeps the first finished status of a run", () => {
+    const [group] = groupJobs([
+      job([run("a", { status: "failed" })]),
+      job([run("a", { status: "succeeded" })]),
+    ]);
+
+    expect(group.hosts[0].runs.map(r => r.run.status)).toEqual(["failed"]);
+  });
+
+  test("links only to safe logs, and not to the placeholder host", () => {
+    const [group] = groupJobs([
+      job([
+        run("a"),
+        run("b", { log: "javascript:alert(1)" }),
+        run("c", { log: "https://no.url.example.com/x" }),
+      ]),
+    ]);
+
+    expect(
+      group.hosts.flatMap(h => h.runs.map(r => [r.run.runId, r.safeLog])),
+    ).toEqual([
+      ["a", "https://ci.example.com/runs/a"],
+      ["b", undefined],
+      ["c", undefined],
+    ]);
+  });
+});