diff --git a/lib/daemon/__tests__/branch-enrich-heal.test.ts b/lib/daemon/__tests__/branch-enrich-heal.test.ts new file mode 100644 index 00000000..8061215a --- /dev/null +++ b/lib/daemon/__tests__/branch-enrich-heal.test.ts @@ -0,0 +1,60 @@ +import { describe, test, expect } from "bun:test"; +import { createCacheHandlers } from "../handlers/cache.ts"; +import { fakeStore } from "./fake-cache-store.ts"; + +const HOUR = 60 * 60 * 1000; + +function makeCtx(entries: Record) { + const ctx = { cache: fakeStore(entries), refreshCache: async () => {} } as any; + return ctx; +} + +describe("branch:enrich heals an entry whose ticket never resolved", () => { + test("a complete entry is served from cache without re-enriching", async () => { + const ctx = makeCtx({ + b: { linearId: "ACME-1", ticket: { identifier: "ACME-1" }, mr: null, fetchedAt: Date.now() - HOUR }, + }); + const res = await createCacheHandlers(ctx)["branch:enrich"]!({ branch: "b", repoPath: "/tmp/x" }); + expect(res.ok).toBe(true); + expect(res.source).toBe("cache"); + }); + + test("an entry with no linear id at all stays a cache hit", async () => { + // Nothing to resolve: re-enriching would spend a lookup per read forever. + const ctx = makeCtx({ b: { linearId: null, ticket: null, mr: null, fetchedAt: 1 } }); + const res = await createCacheHandlers(ctx)["branch:enrich"]!({ branch: "b", repoPath: "/tmp/x" }); + expect(res.source).toBe("cache"); + }); + + test("an id resolved but no ticket is INCOMPLETE, and re-enriches", async () => { + const ctx = makeCtx({ + b: { linearId: "ACME-1", ticket: null, mr: null, fetchedAt: Date.now() - HOUR }, + }); + const res = await createCacheHandlers(ctx)["branch:enrich"]!({ + branch: "b", + repoPath: "/tmp/x", + // The enricher is injected so the test never reaches the network. + enrich: async () => { + ctx.cache.entries.b.ticket = { identifier: "ACME-1", title: "t", url: "u" }; + }, + }); + expect(res.source).toBe("fresh"); + expect(res.data.ticket.identifier).toBe("ACME-1"); + }); + + test("a recent incomplete entry is not retried, so a genuinely missing ticket costs one lookup", async () => { + let calls = 0; + const ctx = makeCtx({ + b: { linearId: "ACME-1", ticket: null, mr: null, fetchedAt: Date.now() - 1_000 }, + }); + const res = await createCacheHandlers(ctx)["branch:enrich"]!({ + branch: "b", + repoPath: "/tmp/x", + enrich: async () => { + calls++; + }, + }); + expect(calls).toBe(0); + expect(res.source).toBe("cache"); + }); +}); diff --git a/lib/daemon/handlers/cache.ts b/lib/daemon/handlers/cache.ts index 15a44f83..95e382b9 100644 --- a/lib/daemon/handlers/cache.ts +++ b/lib/daemon/handlers/cache.ts @@ -11,6 +11,26 @@ import type { HandlerContext, HandlerMap, CacheEntry } from "./types.ts"; +/** How long an entry that resolved a ticket id but never got the ticket is + left alone before another lookup is spent on it. Short enough that a key + that was missing at first write heals on the next read; long enough that a + ticket id which genuinely resolves to nothing costs one lookup an hour, + not one per request. */ +const INCOMPLETE_RETRY_MS = 10 * 60 * 1000; + +/** + * A cached entry is INCOMPLETE, not a hit, when it extracted a ticket id but + * carries no ticket: that pairing only happens when the lookup failed or was + * skipped (no API key at write time), and the old code's plain existence + * check meant such an entry never got another chance for the life of the + * cache. Entries with no id at all are complete by definition — there is + * nothing left to resolve, and retrying them would spend a lookup per read. + */ +function isIncomplete(entry: CacheEntry, now: number = Date.now()): boolean { + if (!entry.linearId || entry.ticket) return false; + return now - (entry.fetchedAt ?? 0) >= INCOMPLETE_RETRY_MS; +} + export function createCacheHandlers(ctx: HandlerContext): HandlerMap { return { "cache:read": async (payload) => { @@ -49,18 +69,25 @@ export function createCacheHandlers(ctx: HandlerContext): HandlerMap { const branch = payload?.branch as string; const repoPath = payload?.repoPath as string; const remoteUrl = payload?.remoteUrl as string | undefined; + // Test seam: the enricher, so a test never reaches Linear or the forge. + const inject = payload?.enrich as (() => Promise) | undefined; if (!branch) return { ok: false, error: "missing branch" }; - if (ctx.cache.entries[branch]) { - return { ok: true, data: ctx.cache.entries[branch], source: "cache" }; + const cached = ctx.cache.entries[branch]; + if (cached && !isIncomplete(cached)) { + return { ok: true, data: cached, source: "cache" }; } if (!repoPath) return { ok: false, error: "missing repoPath for cold enrichment" }; try { - const { enrichBranches } = await import("../../enrich.ts"); - await enrichBranches([{ path: repoPath, branch }], remoteUrl, { silent: true }); + if (inject) { + await inject(); + } else { + const { enrichBranches } = await import("../../enrich.ts"); + await enrichBranches([{ path: repoPath, branch }], remoteUrl, { silent: true }); + } // enrichBranches wrote through the same singleton store in this // process, so the map is already current; reload() is kept because