diff --git a/bun.lock b/bun.lock index 442600fd..60a0f300 100644 --- a/bun.lock +++ b/bun.lock @@ -33,7 +33,7 @@ }, "packages/rt-client": { "name": "@mattstack/rt-client", - "version": "0.5.0", + "version": "0.6.0", "dependencies": { "jsonc-parser": "^3.3.1", }, diff --git a/lib/__tests__/linear-batch.test.ts b/lib/__tests__/linear-batch.test.ts new file mode 100644 index 00000000..b6ccce04 --- /dev/null +++ b/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, 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 = {}; + 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 = {}; + chunk.forEach((id, i) => { data[`i${i}`] = ticket(id); }); + return data; + }; + await expect(fetchTicketsBatch("key", ids, run)).rejects.toThrow("Linear API 500"); + }); +}); diff --git a/lib/linear.ts b/lib/linear.ts index ae454455..8ad54ba6 100644 --- a/lib/linear.ts +++ b/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): Promise { const response = await fetch(GRAPHQL_URL, { method: "POST", @@ -150,7 +166,7 @@ async function linearGraphql(apiKey: string, query: string, variables: Record }; - 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): 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) => Promise; + +/** 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> { const results = new Map(); 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 | 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 => { + if (chunk.length === 0) return; + try { + const data = (await run(apiKey, batchQuery(chunk), {})) as Record | 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;