From b64fe385bdec531943ce02802a8702146b0a62c4 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 22:53:07 -0500 Subject: [PATCH 1/2] linear: one dead ticket id no longer voids the whole batch Linear answers an aliased batch query with HTTP 200, an errors array and an EMPTY data payload as soon as ONE alias names an issue it cannot resolve -- a deleted ticket, or one the key cannot see. fetchTicketsBatch swallowed that in a bare catch and returned an empty map, and callers write fetchedAt regardless, so a single dead id held the entire branch cache at ticket:null indefinitely. Measured on this machine: 203 cached ids resolved 0 tickets; the batch of 2 worked and the batch of 3 did not. Failed chunks are now halved and retried, so live tickets still land and a dead id costs log2(chunk) queries to isolate instead of taking its neighbours with it. Same 203 ids now resolve 173, the remaining 30 being genuinely unresolvable. The GraphQL runner is injectable so the tests pin the behaviour without touching the network. Co-Authored-By: Claude Fable 5 --- bun.lock | 2 +- lib/__tests__/linear-batch.test.ts | 61 ++++++++++++++++++++++++++ lib/linear.ts | 70 +++++++++++++++++++++--------- 3 files changed, 111 insertions(+), 22 deletions(-) create mode 100644 lib/__tests__/linear-batch.test.ts 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..f5ea40ad --- /dev/null +++ b/lib/__tests__/linear-batch.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, test } from "bun:test"; +import { fetchTicketsBatch } 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. + */ +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) { + 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 (ids.some(id => dead.has(id))) throw new Error("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); + }); +}); diff --git a/lib/linear.ts b/lib/linear.ts index ae454455..3263dc71 100644 --- a/lib/linear.ts +++ b/lib/linear.ts @@ -174,36 +174,64 @@ 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. + */ 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 { + 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; From 0b6526fc4eee06737a0a364bad7bb62eaf779c88 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Tue, 25 Aug 2026 23:24:22 -0500 Subject: [PATCH 2/2] linear: only a GraphQL error means the id is dead, never a timeout Halving caught every error, so an outage resolved to an empty map that looks exactly like "none of these tickets exist". enrichBranches keeps its cached tickets only on a rejection, so that would overwrite good tickets with null on every Linear blip. LinearGraphqlError marks the HTTP-200-plus-errors answer. Only it halves; timeouts, 5xx, 401 and 429 rethrow. --- lib/__tests__/linear-batch.test.ts | 46 ++++++++++++++++++++++++++++-- lib/linear.ts | 27 ++++++++++++++++-- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/lib/__tests__/linear-batch.test.ts b/lib/__tests__/linear-batch.test.ts index f5ea40ad..b6ccce04 100644 --- a/lib/__tests__/linear-batch.test.ts +++ b/lib/__tests__/linear-batch.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { fetchTicketsBatch } from "../linear.ts"; +import { fetchTicketsBatch, LinearGraphqlError } from "../linear.ts"; /** * Linear answers a batched alias query with HTTP 200, an `errors` array and an @@ -7,6 +7,11 @@ import { fetchTicketsBatch } from "../linear.ts"; * 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}`, @@ -17,12 +22,15 @@ const ticket = (id: string) => ({ }); /** Stands in for Linear: any batch containing a dead id resolves nothing. */ -function fakeGraphql(dead: Set) { +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 (ids.some(id => dead.has(id))) throw new Error("Entity not found: Issue"); + 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; @@ -59,3 +67,35 @@ describe("fetchTicketsBatch survives an unresolvable id", () => { 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 3263dc71..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; } @@ -202,6 +218,12 @@ function batchQuery(identifiers: string[]): string { * 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, @@ -222,7 +244,8 @@ export async function fetchTicketsBatch( results.set(ticket.identifier.toUpperCase(), ticket); } } - } catch { + } 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));