One dead ticket id no longer voids the whole Linear batch

lib/__tests__/linear-batch.test.ts

@@ -0,0 +1,101 @@
+import { describe, expect, test } from "bun:test";
+import { fetchTicketsBatch, LinearGraphqlError } from "../linear.ts";
+
+/**
+ * Linear answers a batched alias query with HTTP 200, an `errors` array and an
+ * EMPTY `data` payload as soon as any single alias names an issue it cannot
+ * resolve — a deleted ticket, or one the key cannot see. One dead id therefore
+ * takes every other ticket in the batch down with it, which on this machine
+ * left all 203 cached ids resolving to null.
+ *
+ * A transport failure (timeout, 5xx, 401, 429) looks nothing like that and must
+ * NOT be read as "these ids are gone": `enrichBranches` preserves its cached
+ * tickets only while this function rejects, so swallowing an outage is what
+ * would overwrite good tickets with null.
+ */
+const ticket = (id: string) => ({
+  id: `uuid-${id}`,
+  identifier: id,
+  title: `title ${id}`,
+  url: `https://linear.app/acme/issue/${id}`,
+  state: { name: "In Progress", color: "#fff" },
+});
+
+/** Stands in for Linear: any batch containing a dead id resolves nothing. */
+function fakeGraphql(dead: Set<string>, transportFailure?: Error) {
+  const calls: string[][] = [];
+  const run = async (_key: string, query: string) => {
+    const ids = [...query.matchAll(/issue\(id: "([^"]+)"\)/g)].map(m => m[1]!);
+    calls.push(ids);
+    if (transportFailure) throw transportFailure;
+    if (ids.some(id => dead.has(id))) {
+      throw new LinearGraphqlError([{ message: "Entity not found: Issue" }]);
+    }
+    const data: Record<string, unknown> = {};
+    ids.forEach((id, i) => { data[`i${i}`] = ticket(id); });
+    return data;
+  };
+  return { run, calls };
+}
+
+describe("fetchTicketsBatch survives an unresolvable id", () => {
+  test("resolves every live ticket even when one id in the batch is dead", async () => {
+    const { run } = fakeGraphql(new Set(["ACME-2"]));
+    const got = await fetchTicketsBatch("key", ["ACME-1", "ACME-2", "ACME-3"], run);
+    expect([...got.keys()].sort()).toEqual(["ACME-1", "ACME-3"]);
+  });
+
+  test("a wholly live batch still costs exactly one query", async () => {
+    const { run, calls } = fakeGraphql(new Set());
+    const got = await fetchTicketsBatch("key", ["ACME-1", "ACME-2", "ACME-3"], run);
+    expect(got.size).toBe(3);
+    expect(calls).toHaveLength(1);
+  });
+
+  test("isolating one dead id does not degrade to one query per id", async () => {
+    const ids = Array.from({ length: 16 }, (_, i) => `ACME-${i}`);
+    const { run, calls } = fakeGraphql(new Set(["ACME-9"]));
+    const got = await fetchTicketsBatch("key", ids, run);
+    expect(got.size).toBe(15);
+    // Halving isolates the bad id; a per-id fallback would be 16+ calls.
+    expect(calls.length).toBeLessThan(12);
+  });
+
+  test("all ids dead resolves nothing without throwing", async () => {
+    const { run } = fakeGraphql(new Set(["ACME-1", "ACME-2"]));
+    const got = await fetchTicketsBatch("key", ["ACME-1", "ACME-2"], run);
+    expect(got.size).toBe(0);
+  });
+});
+
+describe("fetchTicketsBatch rejects rather than mistake an outage for dead ids", () => {
+  test("a timeout rejects instead of resolving empty", async () => {
+    const { run } = fakeGraphql(new Set(), new DOMException("The operation timed out.", "TimeoutError"));
+    await expect(fetchTicketsBatch("key", ["ACME-1", "ACME-2"], run)).rejects.toThrow(/timed out/i);
+  });
+
+  test("a non-200 response rejects", async () => {
+    const { run } = fakeGraphql(new Set(), new Error("Linear API 503"));
+    await expect(fetchTicketsBatch("key", ["ACME-1", "ACME-2"], run)).rejects.toThrow("Linear API 503");
+  });
+
+  test("a rate limit rejects rather than halving into per-id drops", async () => {
+    const { run, calls } = fakeGraphql(new Set(), new Error("Linear API 429"));
+    await expect(fetchTicketsBatch("key", ["ACME-1", "ACME-2", "ACME-3"], run)).rejects.toThrow("Linear API 429");
+    // Rejected on the first failure — never halved looking for a culprit.
+    expect(calls).toHaveLength(1);
+  });
+
+  test("a transport failure after a healthy chunk still rejects, never returns partial", async () => {
+    const ids = Array.from({ length: 120 }, (_, i) => `ACME-${i}`);
+    let seen = 0;
+    const run = async (_key: string, query: string) => {
+      if (++seen > 1) throw new Error("Linear API 500");
+      const chunk = [...query.matchAll(/issue\(id: "([^"]+)"\)/g)].map(m => m[1]!);
+      const data: Record<string, unknown> = {};
+      chunk.forEach((id, i) => { data[`i${i}`] = ticket(id); });
+      return data;
+    };
+    await expect(fetchTicketsBatch("key", ids, run)).rejects.toThrow("Linear API 500");
+  });
+});

lib/linear.ts

@@ -139,6 +139,22 @@ export interface LinearTicket {
   branchName: string | null;
 }
 
+/**
+ * Linear answered, and answered with GraphQL errors — HTTP 200 plus an
+ * `errors` array. Transport failures (timeout, 5xx, 401, 429) stay plain
+ * Errors, and `fetchTicketsBatch` relies on the distinction: only this class
+ * may be read as "an id in that chunk is unresolvable".
+ */
+export class LinearGraphqlError extends Error {
+  readonly errors: Array<{ message: string }>;
+
+  constructor(errors: Array<{ message: string }>) {
+    super(errors[0]?.message ?? "Linear GraphQL error");
+    this.name = "LinearGraphqlError";
+    this.errors = errors;
+  }
+}
+
 async function linearGraphql(apiKey: string, query: string, variables: Record<string, unknown>): Promise<unknown> {
   const response = await fetch(GRAPHQL_URL, {
     method: "POST",
@@ -150,7 +166,7 @@ async function linearGraphql(apiKey: string, query: string, variables: Record<st
   if (!response.ok) throw new Error(`Linear API ${response.status}`);
 
   const json = (await response.json()) as { data?: unknown; errors?: Array<{ message: string }> };
-  if (json.errors?.length) throw new Error(json.errors[0]!.message);
+  if (json.errors?.length) throw new LinearGraphqlError(json.errors);
   return json.data;
 }
 
@@ -174,36 +190,71 @@ function toTicket(raw: Record<string, unknown>): LinearTicket {
  *
  * Returns a Map of uppercase identifier → LinearTicket.
  */
+/** Injected in tests; defaults to the real Linear GraphQL call. */
+export type GraphqlRunner = (apiKey: string, query: string, variables: Record<string, unknown>) => Promise<unknown>;
+
+/** One query per this many ids while everything resolves. Above it the query
+    text grows without buying anything; below it a single dead id costs more
+    halvings than it saves. */
+const TICKET_BATCH_SIZE = 50;
+
+function batchQuery(identifiers: string[]): string {
+  const fields = identifiers.map(
+    (id, idx) => `i${idx}: issue(id: "${id}") { id identifier title description url branchName state { name color } }`,
+  );
+  return `query Batch { ${fields.join("\n")} }`;
+}
+
+/**
+ * Resolve as many identifiers as Linear will admit to.
+ *
+ * Linear answers an aliased batch with HTTP 200, an `errors` array and an
+ * EMPTY `data` payload the moment ONE alias names an issue it cannot resolve —
+ * a deleted ticket, or one this key cannot see. The whole batch resolves to
+ * nothing, and since callers write `fetchedAt` regardless, a single dead id
+ * can hold an entire branch cache at `ticket: null` indefinitely.
+ *
+ * So a failed chunk is halved and retried rather than abandoned: live tickets
+ * still land, and a dead id costs log2(chunk) extra queries to isolate instead
+ * of taking its neighbours with it. A chunk of one that still fails IS the
+ * dead id, and is dropped.
+ *
+ * Only a `LinearGraphqlError` earns that treatment. A timeout or an HTTP
+ * failure would otherwise halve down to singletons and drop every id, handing
+ * callers an empty map that is indistinguishable from "none of these tickets
+ * exist" — and `enrichBranches` keeps its cached tickets only on a REJECTION,
+ * so swallowing an outage here is what overwrites good tickets with null.
+ */
 export async function fetchTicketsBatch(
   apiKey: string,
   identifiers: string[],
+  run: GraphqlRunner = linearGraphql,
 ): Promise<Map<string, LinearTicket>> {
   const results = new Map<string, LinearTicket>();
   if (!identifiers.length) return results;
 
-  // Build a single query with aliased fields:
-  //   query Batch {
-  //     i0: issue(id: "ACME-1403") { id identifier title description url branchName state { name color } }
-  //     i1: issue(id: "ACME-1386") { id identifier title description url branchName state { name color } }
-  //     ...
-  //   }
-  const fields = identifiers.map(
-    (id, idx) => `i${idx}: issue(id: "${id}") { id identifier title description url branchName state { name color } }`,
-  );
-  const query = `query Batch { ${fields.join("\n")} }`;
-
-  try {
-    const data = (await linearGraphql(apiKey, query, {})) as Record<string, Record<string, unknown> | null>;
-
-    for (let idx = 0; idx < identifiers.length; idx++) {
-      const raw = data[`i${idx}`];
-      if (raw && raw.id) {
-        const ticket = toTicket(raw);
-        results.set(ticket.identifier.toUpperCase(), ticket);
+  const collect = async (chunk: string[]): Promise<void> => {
+    if (chunk.length === 0) return;
+    try {
+      const data = (await run(apiKey, batchQuery(chunk), {})) as Record<string, Record<string, unknown> | null>;
+      for (let idx = 0; idx < chunk.length; idx++) {
+        const raw = data?.[`i${idx}`];
+        if (raw && raw.id) {
+          const ticket = toTicket(raw);
+          results.set(ticket.identifier.toUpperCase(), ticket);
+        }
       }
+    } catch (err) {
+      if (!(err instanceof LinearGraphqlError)) throw err;
+      if (chunk.length === 1) return; // this id is the unresolvable one
+      const mid = Math.ceil(chunk.length / 2);
+      await collect(chunk.slice(0, mid));
+      await collect(chunk.slice(mid));
     }
-  } catch {
-    // Batch fetch failed — caller will use cached data gracefully
+  };
+
+  for (let i = 0; i < identifiers.length; i += TICKET_BATCH_SIZE) {
+    await collect(identifiers.slice(i, i + TICKET_BATCH_SIZE));
   }
 
   return results;