diff --git a/apps/dispatcher/src/admission-summary.test.ts b/apps/dispatcher/src/admission-summary.test.ts index 2b3dace..66cedcf 100644 --- a/apps/dispatcher/src/admission-summary.test.ts +++ b/apps/dispatcher/src/admission-summary.test.ts @@ -2,25 +2,86 @@ // formatting, exercised with fixed inputs like failure-summary.test.ts. import { describe, expect, it } from "vitest"; -import { queuedSummary, serialQueuedSummary } from "./admission-summary"; +import { + nextPositionReport, + POSITION_UPDATE_EVERY_MS, + type PositionReport, + positionBucket, + queuedSummary, + serialQueuedSummary, +} from "./admission-summary"; + +describe("positionBucket", () => { + it("places a waiter with nobody ahead in bucket 1", () => { + expect(positionBucket(0)).toBe("1"); + }); + + it("buckets places 2-5, 6-20 and over 20 at their boundaries", () => { + expect([1, 4, 5, 19, 20, 100].map(positionBucket)).toEqual([ + "2-5", + "2-5", + "6-20", + "6-20", + "over 20", + "over 20", + ]); + }); +}); + +describe("nextPositionReport", () => { + const T0 = 1_000_000; + + it("writes the first position it sees", () => { + expect(nextPositionReport(undefined, 7, T0)).toEqual({ bucket: "6-20", at: T0 }); + }); + + it("writes nothing while the bucket holds, however long", () => { + const last = { bucket: "6-20" as const, at: T0 }; + expect(nextPositionReport(last, 6, T0 + 10 * POSITION_UPDATE_EVERY_MS)).toBeUndefined(); + }); + + it("holds a crossed bucket until a minute has passed since the last write", () => { + const last = { bucket: "6-20" as const, at: T0 }; + expect(nextPositionReport(last, 3, T0 + POSITION_UPDATE_EVERY_MS - 1)).toBeUndefined(); + expect(nextPositionReport(last, 3, T0 + POSITION_UPDATE_EVERY_MS)).toEqual({ + bucket: "2-5", + at: T0 + POSITION_UPDATE_EVERY_MS, + }); + }); + + it("bounds a draining queue of 30 polled every 20s to one write a minute at most", () => { + let last: PositionReport | undefined; + let writes = 0; + for (let i = 0; i <= 30; i++) { + const next = nextPositionReport(last, 30 - i, T0 + i * 20_000); + if (next !== undefined) { + writes += 1; + last = next; + } + } + // over 20 → 6-20 → 2-5 → 1: one write per bucket. + expect(writes).toBe(4); + }); +}); describe("queuedSummary", () => { // 2026-06-05T12:34:00.000Z const TIMES_OUT_AT = Date.UTC(2026, 5, 5, 12, 34, 0); - it("renders position, pool usage, and the UTC give-up time", () => { - expect(queuedSummary(3, 16, 16, TIMES_OUT_AT)).toBe( - "Queued — waiting for a sandbox slot behind 3 runs (16/16 in use); times out 12:34 UTC", + it("renders the bucket, pool usage, and the UTC give-up time", () => { + expect(queuedSummary("2-5", 16, 16, TIMES_OUT_AT)).toBe( + "Queued — waiting for a sandbox slot, 2–5 in line (16/16 in use); times out 12:34 UTC", ); }); - it("singularises a one-run queue", () => { - expect(queuedSummary(1, 16, 16, TIMES_OUT_AT)).toContain("behind 1 run ("); + it("names bucket 1 as next in line and the top bucket as over 20th", () => { + expect(queuedSummary("1", 16, 16, TIMES_OUT_AT)).toContain("next in line ("); + expect(queuedSummary("over 20", 16, 16, TIMES_OUT_AT)).toContain("over 20th in line ("); }); it("zero-pads single-digit hours and minutes", () => { // 2026-06-05T03:05:00.000Z - expect(queuedSummary(0, 16, 16, Date.UTC(2026, 5, 5, 3, 5, 0))).toContain( + expect(queuedSummary("1", 16, 16, Date.UTC(2026, 5, 5, 3, 5, 0))).toContain( "times out 03:05 UTC", ); }); diff --git a/apps/dispatcher/src/admission-summary.ts b/apps/dispatcher/src/admission-summary.ts index ff7321f..e25b14d 100644 --- a/apps/dispatcher/src/admission-summary.ts +++ b/apps/dispatcher/src/admission-summary.ts @@ -7,17 +7,54 @@ // and so cannot load under plain Node + Vitest), mirroring // failure-summary.ts. +/** + * A waiter's place in line, coarsened. Place 1 is next in line (no live + * queued run ahead). The check-run shows the bucket, so a waiter writes only + * when its bucket changes. + */ +export type PositionBucket = "1" | "2-5" | "6-20" | "over 20"; + +/** The bucket for a waiter with `ahead` live queued runs ahead of it. */ +export const positionBucket = (ahead: number): PositionBucket => { + const place = Math.max(0, ahead) + 1; + return place === 1 ? "1" : place <= 5 ? "2-5" : place <= 20 ? "6-20" : "over 20"; +}; + +/** The shortest gap between two queue-position writes of one waiter. */ +export const POSITION_UPDATE_EVERY_MS = 60_000; + +/** The last queue position a waiter wrote to its check-run. */ +export type PositionReport = { readonly bucket: PositionBucket; readonly at: number }; + +/** + * The position write a waiter makes at `now`, or `undefined` for none: a write + * happens when the bucket differs from the last one written and at least + * `POSITION_UPDATE_EVERY_MS` has passed since that write. A bucket crossed + * inside the minute is written on the first poll after it. + */ +export const nextPositionReport = ( + last: PositionReport | undefined, + ahead: number, + now: number, +): PositionReport | undefined => { + const bucket = positionBucket(ahead); + if (last !== undefined && (last.bucket === bucket || now - last.at < POSITION_UPDATE_EVERY_MS)) { + return undefined; + } + return { bucket, at: now }; +}; + /** * The "queued" line the admission gate posts to the in-progress check-run — - * only when the position CHANGES, to bound GitHub API writes. + * only when the position bucket changes, to bound GitHub API writes. * - * @param position live queued runs ahead of this one. + * @param bucket the waiter's place in line, coarsened (`positionBucket`). * @param poolBusy admission slots in use in the run's pool. * @param cap the pool's slot cap (`ADMISSION_CAP`). * @param timesOutAt ms-epoch the dispatch-age ceiling lapses (enqueue + max age). */ export const queuedSummary = ( - position: number, + bucket: PositionBucket, poolBusy: number, cap: number, timesOutAt: number, @@ -25,9 +62,11 @@ export const queuedSummary = ( const d = new Date(timesOutAt); const hh = String(d.getUTCHours()).padStart(2, "0"); const mm = String(d.getUTCMinutes()).padStart(2, "0"); + const place = + bucket === "1" ? "next in line" : bucket === "over 20" ? "over 20th in line" : `${bucket.replace("-", "–")} in line`; return ( - `Queued — waiting for a sandbox slot behind ${position} ` + - `run${position === 1 ? "" : "s"} (${poolBusy}/${cap} in use); ` + + `Queued — waiting for a sandbox slot, ${place} ` + + `(${poolBusy}/${cap} in use); ` + `times out ${hh}:${mm} UTC` ); }; diff --git a/apps/dispatcher/src/cron-parity.test.ts b/apps/dispatcher/src/cron-parity.test.ts index f346e1b..e6299e4 100644 --- a/apps/dispatcher/src/cron-parity.test.ts +++ b/apps/dispatcher/src/cron-parity.test.ts @@ -16,6 +16,7 @@ import { readFileSync } from "node:fs"; import { describe, expect, it } from "vitest"; +import { DELIVERY_SWEEP_CRON } from "./delivery-sweep"; import { runNames, schedulesByCron, scheduledRuns } from "./registry"; /** @@ -47,7 +48,15 @@ describe("cron parity", () => { // silently yields no crons passes every assertion below while checking // nothing. expect(armed.length).toBeGreaterThan(0); - expect(armed.filter((cron) => schedulesByCron(cron).length === 0)).toEqual([]); + // The reconcile sweep's cron is the one armed expression that binds no + // run: the scheduled handler routes it to the sweep. + expect( + armed.filter((cron) => cron !== DELIVERY_SWEEP_CRON && schedulesByCron(cron).length === 0), + ).toEqual([]); + }); + + it("arms the verdict reconcile sweep", () => { + expect(readArmedCrons()).toContain(DELIVERY_SWEEP_CRON); }); it("every cron a scheduled run declares is armed unless deliberately left off", () => { diff --git a/apps/dispatcher/src/delivery-sweep.test.ts b/apps/dispatcher/src/delivery-sweep.test.ts new file mode 100644 index 0000000..01e842b --- /dev/null +++ b/apps/dispatcher/src/delivery-sweep.test.ts @@ -0,0 +1,182 @@ +// Tests for the verdict reconcile sweep — driven through in-memory ports. + +import type { CheckConcludeOptions, CheckDelivery } from "@fractalboxdev/flare-dispatch-core"; +import { describe, expect, it, vi } from "vitest"; +import { + conclusionFor, + DELIVERY_GRACE_MS, + DELIVERY_MAX_AGE_MS, + DELIVERY_SWEEP_LIMIT, + type DeliverySweepPorts, + sweepUndelivered, + type UndeliveredRow, +} from "./delivery-sweep"; + +const NOW = Date.UTC(2026, 9, 22, 12, 0, 0); + +const row = (over: Partial = {}): UndeliveredRow => ({ + id: "exec-1", + run: "oxlint", + repo: "owner/name", + sha: "abc123", + status: "success", + attempt: 1, + inputJson: "{}", + checkRunId: "77", + installationId: 1, + completedAt: NOW - DELIVERY_GRACE_MS - 1, + ...over, +}); + +/** In-memory ports over `rows`; `answer` decides each conclude. */ +const makePorts = ( + rows: UndeliveredRow[], + answer: (installationId: number, opts: CheckConcludeOptions) => CheckDelivery = (_, opts) => ({ + _tag: "delivered", + checkRunId: opts.checkRunId === "noop" ? "9001" : opts.checkRunId, + }), +) => { + const concluded: { installationId: number; opts: CheckConcludeOptions }[] = []; + const marked: { id: string; checkRunId: string; at: number }[] = []; + const refusedRows: { id: string; at: number }[] = []; + const listUndelivered = vi.fn( + async ({ completedBefore, completedAfter, limit }) => + rows + .filter((r) => r.completedAt < completedBefore && r.completedAt >= completedAfter) + .sort((a, b) => a.completedAt - b.completedAt) + .slice(0, limit), + ); + const ports: DeliverySweepPorts = { + listUndelivered, + conclude: async (installationId, opts) => { + concluded.push({ installationId, opts }); + return answer(installationId, opts); + }, + markDelivered: async (id, checkRunId, at) => { + marked.push({ id, checkRunId, at }); + }, + markRefused: async (id, at) => { + refusedRows.push({ id, at }); + }, + now: () => NOW, + }; + return { ports, concluded, marked, refusedRows, listUndelivered }; +}; + +describe("sweepUndelivered", () => { + it("asks for rows past the grace period and inside the max age, oldest first, bounded", async () => { + const { ports, listUndelivered } = makePorts([]); + await sweepUndelivered(ports); + expect(listUndelivered).toHaveBeenCalledWith({ + completedBefore: NOW - DELIVERY_GRACE_MS, + completedAfter: NOW - DELIVERY_MAX_AGE_MS, + limit: DELIVERY_SWEEP_LIMIT, + }); + }); + + it("re-sends oldest first and stamps each delivered row", async () => { + const { ports, concluded, marked } = makePorts([ + row({ id: "newer", completedAt: NOW - DELIVERY_GRACE_MS - 1 }), + row({ id: "older", completedAt: NOW - DELIVERY_GRACE_MS - 60_000 }), + ]); + + const result = await sweepUndelivered(ports); + + expect(concluded.map((c) => c.opts.output?.summary.includes("older") ?? false)).toEqual([ + true, + false, + ]); + expect(marked.map((m) => m.id)).toEqual(["older", "newer"]); + expect(result).toEqual({ delivered: 2, undelivered: 0, refused: 0, deferred: 0 }); + }); + + it("records the check-run created at conclusion for a row whose opener degraded", async () => { + const { ports, concluded, marked } = makePorts([row({ checkRunId: "noop" })]); + await sweepUndelivered(ports); + expect(concluded[0]!.opts.checkRunId).toBe("noop"); + expect(marked).toEqual([{ id: "exec-1", checkRunId: "9001", at: NOW }]); + }); + + it("stops an installation at its first refusal and keeps sending for the others", async () => { + const { ports, concluded, marked } = makePorts( + [ + row({ id: "a1", installationId: 1, completedAt: NOW - DELIVERY_GRACE_MS - 3 }), + row({ id: "b1", installationId: 2, completedAt: NOW - DELIVERY_GRACE_MS - 2 }), + row({ id: "a2", installationId: 1, completedAt: NOW - DELIVERY_GRACE_MS - 1 }), + ], + (installationId, opts) => + installationId === 1 + ? { _tag: "undelivered", reason: "installation quota exhausted" } + : { _tag: "delivered", checkRunId: opts.checkRunId }, + ); + vi.spyOn(console, "warn").mockImplementation(() => {}); + + const result = await sweepUndelivered(ports); + + expect(concluded.map((c) => c.installationId)).toEqual([1, 2]); + expect(marked.map((m) => m.id)).toEqual(["b1"]); + expect(result).toEqual({ delivered: 1, undelivered: 1, refused: 0, deferred: 1 }); + }); + + it("takes a refused row out of the queue without holding up its installation", async () => { + const { ports, concluded, marked, refusedRows } = makePorts( + [ + row({ id: "gone", installationId: 1, sha: "dead", completedAt: NOW - DELIVERY_GRACE_MS - 2 }), + row({ id: "next", installationId: 1, completedAt: NOW - DELIVERY_GRACE_MS - 1 }), + ], + (_, opts) => + opts.sha === "dead" + ? { _tag: "refused", reason: "HTTP 404" } + : { _tag: "delivered", checkRunId: opts.checkRunId }, + ); + vi.spyOn(console, "warn").mockImplementation(() => {}); + const result = await sweepUndelivered(ports); + + expect(concluded.map((c) => c.opts.sha)).toEqual(["dead", "abc123"]); + expect(refusedRows).toEqual([{ id: "gone", at: NOW }]); + expect(marked.map((m) => m.id)).toEqual(["next"]); + expect(result).toEqual({ delivered: 1, undelivered: 0, refused: 1, deferred: 0 }); + }); + + it("counts a rejected conclude as undelivered and a failed stamp as delivered", async () => { + const { ports } = makePorts([row({ id: "x", installationId: 1 }), row({ id: "y", installationId: 2 })]); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const result = await sweepUndelivered({ + ...ports, + conclude: async (installationId, opts) => { + if (installationId === 1) throw new Error("boom"); + return { _tag: "delivered", checkRunId: opts.checkRunId }; + }, + markDelivered: async () => { + throw new Error("d1 down"); + }, + }); + expect(result).toEqual({ delivered: 1, undelivered: 1, refused: 0, deferred: 0 }); + expect(warn).toHaveBeenCalled(); + }); +}); + +describe("conclusionFor", () => { + it("names, titles and concludes the check-run as the workflow does", () => { + const opts = conclusionFor( + row({ run: "check", inputJson: JSON.stringify({ checkLabel: "lint" }), attempt: 2, status: "skipped" }), + ); + expect(opts).toMatchObject({ + repo: "owner/name", + sha: "abc123", + name: "flare-dispatch/check:lint", + checkRunId: "77", + conclusion: "neutral", + output: { title: "flare-dispatch/check:lint (attempt 2)" }, + }); + expect(opts.output?.summary).toContain("⊘ check — skipped."); + }); + + it("names a row with unreadable inputs without a label", () => { + expect(conclusionFor(row({ inputJson: "{not json" })).name).toBe("flare-dispatch/oxlint"); + }); + + it("concludes a failed execution as failure", () => { + expect(conclusionFor(row({ status: "failure" })).conclusion).toBe("failure"); + }); +}); diff --git a/apps/dispatcher/src/delivery-sweep.ts b/apps/dispatcher/src/delivery-sweep.ts new file mode 100644 index 0000000..280132e --- /dev/null +++ b/apps/dispatcher/src/delivery-sweep.ts @@ -0,0 +1,166 @@ +// FlareDispatch Dispatcher — the verdict reconcile sweep. +// +// An execution's verdict reaches the pull request only when its concluding +// check-run write lands; `RunWorkflow` stamps `executions.delivered_at` when it +// does. A write that fails — a rate-limited installation, a GitHub outage — +// leaves a finished row with an installation and no `delivered_at`. This sweep +// runs on its own cron and re-sends those conclusions, oldest first: +// +// * only rows that finished at least `DELIVERY_GRACE_MS` ago, so a workflow +// still between its terminal row write and its check-run write is not +// raced; +// * only rows younger than `DELIVERY_MAX_AGE_MS`, so a verdict an outage +// kept from GitHub for a week stops being retried; +// * only rows not yet refused: a write GitHub rejects for the check-run +// itself (404 or 422 — repository, installation or commit gone) stamps +// `delivery_refused_at` and leaves the queue, so it never holds up the +// newer rows behind it; +// * at most `DELIVERY_SWEEP_LIMIT` rows a tick; +// * inside the verdict reserve: `conclude` spends the reserve and refuses +// only an exhausted installation, and the first undelivered (not refused) +// row of an installation stops the sweep for that installation until the +// next tick. +// +// A row whose opener degraded holds the sentinel check-run id; `conclude` +// creates the check-run with the verdict and the sweep records the new id. +// +// Plain module, no `cloudflare:workers` import, so the dispatcher's Node test +// project loads it; routes/scheduled.ts wires the live D1 and GitHub ports. + +import type { CheckConcludeOptions, CheckDelivery } from "@fractalboxdev/flare-dispatch-core"; +import { checkRunNameFor, checkRunTitleFor } from "./check-name"; +import { checkConclusion, type ExecutionVerdict } from "./terminal-verdict"; + +/** The cron the sweep runs on — armed in `wrangler.jsonc` beside the run crons. */ +export const DELIVERY_SWEEP_CRON = "*/10 * * * *"; + +/** How long after finishing a row first becomes eligible. */ +export const DELIVERY_GRACE_MS = 10 * 60_000; + +/** The oldest finished row the sweep still re-sends. */ +export const DELIVERY_MAX_AGE_MS = 7 * 24 * 3_600_000; + +/** The most rows one tick re-sends. */ +export const DELIVERY_SWEEP_LIMIT = 50; + +/** A finished execution that owes GitHub its verdict. */ +export type UndeliveredRow = { + readonly id: string; + readonly run: string; + readonly repo: string; + readonly sha: string; + readonly status: ExecutionVerdict; + readonly attempt: number; + readonly inputJson: string; + readonly checkRunId: string; + readonly installationId: number; + readonly completedAt: number; +}; + +/** The I/O the sweep runs against. */ +export type DeliverySweepPorts = { + /** + * Finished rows with neither `delivered_at` nor `delivery_refused_at`, + * completed in `[after, before)`, oldest first. + */ + readonly listUndelivered: (opts: { + readonly completedBefore: number; + readonly completedAfter: number; + readonly limit: number; + }) => Promise; + /** The concluding write, through the installation's checks client. */ + readonly conclude: (installationId: number, opts: CheckConcludeOptions) => Promise; + /** Stamp `delivered_at` and the check-run that holds the verdict. */ + readonly markDelivered: (id: string, checkRunId: string, at: number) => Promise; + /** Stamp `delivery_refused_at`: GitHub rejects this row's write for good. */ + readonly markRefused: (id: string, at: number) => Promise; + readonly now: () => number; +}; + +/** What one tick did. */ +export type DeliverySweepResult = { + readonly delivered: number; + readonly undelivered: number; + /** Rows GitHub refused for good, taken out of the queue. */ + readonly refused: number; + /** Rows not attempted because their installation already left one undelivered this tick. */ + readonly deferred: number; +}; + +/** Parse a row's stored inputs; a malformed blob names the check without a label. */ +const parseInputs = (json: string): unknown => { + try { + return JSON.parse(json) as unknown; + } catch { + return undefined; + } +}; + +const VERDICT_LINE: Record string> = { + success: (run) => `✓ ${run} — execution succeeded.`, + failure: (run) => `✗ ${run} — execution failed.`, + skipped: (run) => `⊘ ${run} — skipped.`, +}; + +/** The concluding write a row owes: the name, title and conclusion its workflow posts. */ +export const conclusionFor = (row: UndeliveredRow): CheckConcludeOptions => { + const name = checkRunNameFor(row.run, parseInputs(row.inputJson)); + return { + repo: row.repo, + sha: row.sha, + name, + checkRunId: row.checkRunId, + conclusion: checkConclusion(row.status, true), + output: { + title: checkRunTitleFor(name, row.attempt), + summary: + `${VERDICT_LINE[row.status](row.run)}\n\n` + + `Execution \`${row.id}\`. The reconcile sweep delivered this verdict; ` + + `the execution's own write did not reach GitHub.`, + }, + }; +}; + +/** One sweep tick: re-send every eligible undelivered verdict, oldest first. */ +export const sweepUndelivered = async (ports: DeliverySweepPorts): Promise => { + const now = ports.now(); + const rows = await ports.listUndelivered({ + completedBefore: now - DELIVERY_GRACE_MS, + completedAfter: now - DELIVERY_MAX_AGE_MS, + limit: DELIVERY_SWEEP_LIMIT, + }); + const held = new Set(); + let delivered = 0; + let undelivered = 0; + let refused = 0; + let deferred = 0; + for (const row of rows) { + if (held.has(row.installationId)) { + deferred += 1; + continue; + } + const outcome = await ports + .conclude(row.installationId, conclusionFor(row)) + .catch((cause: unknown): CheckDelivery => ({ _tag: "undelivered", reason: String(cause) })); + if (outcome._tag === "delivered") { + delivered += 1; + // GitHub holds the verdict; a failed stamp only means the next tick + // sends the same conclusion again. + await ports.markDelivered(row.id, outcome.checkRunId, ports.now()).catch((cause: unknown) => { + console.warn(`[delivery-sweep] delivered_at not recorded for ${row.id}: ${String(cause)}`); + }); + } else if (outcome._tag === "refused") { + // The installation still answers; only this row's write is dead. + console.warn(`[delivery-sweep] ${row.id} refused: ${outcome.reason}`); + refused += 1; + await ports.markRefused(row.id, ports.now()).catch((cause: unknown) => { + console.warn(`[delivery-sweep] delivery_refused_at not recorded for ${row.id}: ${String(cause)}`); + }); + } else { + console.warn(`[delivery-sweep] ${row.id} undelivered: ${outcome.reason}`); + held.add(row.installationId); + undelivered += 1; + } + } + return { delivered, undelivered, refused, deferred }; +}; diff --git a/apps/dispatcher/src/routes/scheduled.test.ts b/apps/dispatcher/src/routes/scheduled.test.ts index 665f0cc..cd96583 100644 --- a/apps/dispatcher/src/routes/scheduled.test.ts +++ b/apps/dispatcher/src/routes/scheduled.test.ts @@ -12,7 +12,8 @@ // - duplicate-id `create` error → swallowed, NOT propagated (dedup path). import { describe, expect, it, vi } from "vitest"; -import { handleScheduled } from "./scheduled"; +import { DELIVERY_SWEEP_CRON } from "../delivery-sweep"; +import { handleScheduled, liveDeliverySweepPorts } from "./scheduled"; import type { Env } from "../env"; import { makeFakeR2, makeFakeWorkflow } from "../test-helpers"; @@ -157,3 +158,28 @@ describe("handleScheduled", () => { errorLog.mockRestore(); }); }); + +describe("handleScheduled — verdict reconcile sweep", () => { + it("runs the sweep on its cron and fires no run, without the no-match warning", async () => { + const workflow = makeFakeWorkflow(); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const sweep = vi.fn(async () => {}); + + await handleScheduled(makeEnv(workflow), DELIVERY_SWEEP_CRON, FIRED_AT, undefined, sweep); + + expect(sweep).toHaveBeenCalledTimes(1); + expect(workflow.calls).toHaveLength(0); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + + it("leaves the sweep alone on every other cron", async () => { + const sweep = vi.fn(async () => {}); + await handleScheduled(makeEnv(makeFakeWorkflow()), "0 14 * * *", FIRED_AT, undefined, sweep); + expect(sweep).not.toHaveBeenCalled(); + }); + + it("has no ports on a deploy without App credentials, where no verdict is owed", () => { + expect(liveDeliverySweepPorts(makeEnv(makeFakeWorkflow()))).toBeUndefined(); + }); +}); diff --git a/apps/dispatcher/src/routes/scheduled.ts b/apps/dispatcher/src/routes/scheduled.ts index fb99e5b..0ccff9e 100644 --- a/apps/dispatcher/src/routes/scheduled.ts +++ b/apps/dispatcher/src/routes/scheduled.ts @@ -45,6 +45,15 @@ import { readBranchHead, resolveRepoInstallationId, } from "@fractalboxdev/flare-dispatch-github-app"; +import { makeChecksGithubService } from "@fractalboxdev/flare-dispatch-runtime-cf/checks-github"; +import { Effect } from "effect"; +import type { ChecksService } from "@fractalboxdev/flare-dispatch-core"; +import { + DELIVERY_SWEEP_CRON, + type DeliverySweepPorts, + sweepUndelivered, + type UndeliveredRow, +} from "../delivery-sweep"; import { toInstanceId } from "../instance-id"; import { type ScheduleMatch, schedulesByCron } from "../registry"; import type { Env } from "../env"; @@ -82,6 +91,103 @@ export const githubScheduledHead = } }; +/** Runs the verdict reconcile sweep for one tick. */ +export type RunDeliverySweep = () => Promise; + +/** A D1 row as the undelivered-verdict query returns it. */ +type UndeliveredDbRow = { + id: string; + run: string; + repo: string; + sha: string; + status: UndeliveredRow["status"]; + attempt: number; + input_json: string; + check_run_id: string | number; + installation_id: number; + completed_at: number; +}; + +/** The sweep's D1 + GitHub ports for this deploy, or `undefined` without App credentials. */ +export const liveDeliverySweepPorts = (env: Env): DeliverySweepPorts | undefined => { + const appId = env.GITHUB_APP_ID; + const privateKeyPem = env.GITHUB_APP_PRIVATE_KEY; + if (appId === undefined || privateKeyPem === undefined) return undefined; + const db = env.RUNS_METADATA; + // One client per installation for the tick, so its quota reads carry across rows. + const clients = new Map(); + const client = (installationId: number): ChecksService => { + const existing = clients.get(installationId); + if (existing !== undefined) return existing; + const created = makeChecksGithubService({ appId, privateKeyPem, installationId }); + clients.set(installationId, created); + return created; + }; + return { + listUndelivered: async ({ completedBefore, completedAfter, limit }) => { + const { results } = await db + .prepare( + `SELECT id, run, repo, sha, status, attempt, input_json, check_run_id, installation_id, completed_at + FROM executions + WHERE delivered_at IS NULL AND installation_id IS NOT NULL + AND delivery_refused_at IS NULL + AND status IN ('success', 'failure', 'skipped') + AND completed_at < ? AND completed_at >= ? + ORDER BY completed_at ASC + LIMIT ?`, + ) + .bind(completedBefore, completedAfter, limit) + .all(); + return results.map((r) => ({ + id: r.id, + run: r.run, + repo: r.repo, + sha: r.sha, + status: r.status, + attempt: r.attempt, + inputJson: r.input_json, + checkRunId: String(r.check_run_id), + installationId: r.installation_id, + completedAt: r.completed_at, + })); + }, + conclude: (installationId, opts) => Effect.runPromise(client(installationId).conclude(opts)), + markDelivered: async (id, checkRunId, at) => { + await db + .prepare(`UPDATE executions SET delivered_at = ?, check_run_id = ? WHERE id = ?`) + .bind(at, checkRunId, id) + .run(); + }, + markRefused: async (id, at) => { + await db + .prepare(`UPDATE executions SET delivery_refused_at = ? WHERE id = ?`) + .bind(at, id) + .run(); + }, + now: Date.now, + }; +}; + +/** The live sweep: a no-op on a deploy without App credentials, where no verdict is owed. */ +export const liveDeliverySweep = + (env: Env): RunDeliverySweep => + async () => { + const ports = liveDeliverySweepPorts(env); + if (ports === undefined) return; + try { + const result = await sweepUndelivered(ports); + if (result.delivered + result.undelivered + result.refused + result.deferred > 0) { + console.info( + `[delivery-sweep] delivered=${result.delivered} undelivered=${result.undelivered} refused=${result.refused} deferred=${result.deferred}`, + ); + } + } catch (cause) { + console.error( + `[delivery-sweep] tick failed: ${cause instanceof Error ? cause.message : String(cause)}`, + ); + } + }; + /** Trim a long cron to fit inline in a log message. */ const fmt = (cron: string): string => (cron.length > 40 ? `${cron.slice(0, 37)}...` : cron); @@ -95,16 +201,23 @@ const fmt = (cron: string): string => (cron.length > 40 ? `${cron.slice(0, 37)}. * @param cron `controller.cron` — the expression that fired. * @param scheduledTime `controller.scheduledTime` — epoch ms. * @param resolveHead installation + branch-head lookup for runs that target a branch. + * @param deliverySweep the verdict reconcile sweep, run on `DELIVERY_SWEEP_CRON`. */ export const handleScheduled = async ( env: Env, cron: string, scheduledTime: number, resolveHead: ResolveScheduledHead = githubScheduledHead(env), + deliverySweep: RunDeliverySweep = liveDeliverySweep(env), ): Promise => { + // The verdict reconcile sweep owns its own cron (delivery-sweep.ts). + const isSweepTick = cron === DELIVERY_SWEEP_CRON; + if (isSweepTick) await deliverySweep(); + const matches = schedulesByCron(cron); if (matches.length === 0) { + if (isSweepTick) return; // `wrangler.jsonc` declared a cron the registry does not bind any run to. // Most likely a left-over expression after a run was retired, or a typo // between the wrangler entry and the run's `schedules[].cron`. diff --git a/apps/dispatcher/src/workflow.ts b/apps/dispatcher/src/workflow.ts index 3205937..5dd1e38 100644 --- a/apps/dispatcher/src/workflow.ts +++ b/apps/dispatcher/src/workflow.ts @@ -114,7 +114,7 @@ import { WRITEBACK_ARTIFACT } from "@fractalboxdev/flare-dispatch-core"; import { lookupRun } from "./registry"; import { preAssertedApproval, resolveTargets, runGrant, runsOnFacade } from "./grant-catalog"; import { selectAdmission, selectSandboxNs } from "./sandbox-routing"; -import { queuedSummary, serialQueuedSummary } from "./admission-summary"; +import { nextPositionReport, type PositionReport, queuedSummary, serialQueuedSummary } from "./admission-summary"; import { appendFailureSummary, type FailedStepRow, @@ -473,6 +473,9 @@ export class RunWorkflow extends WorkflowEntrypoint { // output title carries the attempt so a re-run reads as one. const attempt = payload.attempt ?? 1; const checkRunTitle = checkRunTitleFor(checkRunName, attempt); + // The installation the check-run posts under — `null` on a deploy or + // dispatch with no live `Checks` Layer, where no verdict is owed. + const deliveryInstallationId = resolveChecksConfig(this.env, payload.github)?.installationId ?? null; // The Cloudflare Workflows instance page for this execution — the "Details" // link on the GitHub check-run + a markdown link in its summary. `undefined` // when CLOUDFLARE_ACCOUNT_ID is unset (BYOC default): the check-run renders @@ -805,9 +808,12 @@ export class RunWorkflow extends WorkflowEntrypoint { // The canceller waits for this id before terminating the Workflow. // If cancellation won during creation, the opener concludes the check. const active = yield* persistCheckRunId(checkRunId, { + // The installation rides beside the id: it marks the row as owing a + // verdict on GitHub, which the reconcile sweep re-sends if the + // concluding write never lands (delivery-sweep.ts). persist: (id) => Effect.tryPromise(() => - db.prepare(`UPDATE executions SET check_run_id = ? WHERE id = ?`) - .bind(id, payload.executionId).run(), + db.prepare(`UPDATE executions SET check_run_id = ?, installation_id = ? WHERE id = ?`) + .bind(id, deliveryInstallationId, payload.executionId).run(), ).pipe(Effect.retry(Schedule.once.pipe(Schedule.addDelay(() => "500 millis"))), Effect.asVoid, Effect.orDie), status: supersessions.status(payload.executionId), cancel: (id) => checks.update({ @@ -973,7 +979,7 @@ export class RunWorkflow extends WorkflowEntrypoint { admissionMaxQueueAgeMs, ADMISSION_POLL_EVERY_MS, ); - let lastReportedPosition: number | undefined; + let lastReport: PositionReport | undefined; for (let i = 0; i < maxAttempts; i++) { // A child reads its parent's row before each claim: once a parent @@ -1062,11 +1068,13 @@ export class RunWorkflow extends WorkflowEntrypoint { } // `wait`: surface the queue position on the in-progress check-run - // — only when it CHANGED, bounding GitHub API writes to at most - // one per drained peer. Best-effort: a GitHub blip while queued - // must never kill the run. - if (decision.position !== lastReportedPosition) { - lastReportedPosition = decision.position; + // — only when its bucket changes, at most once a minute + // (admission-summary.ts), and only while the installation's quota + // is above the verdict reserve (checks-github.ts). Best-effort: a + // GitHub blip while queued must never kill the run. + const report = nextPositionReport(lastReport, decision.position, observed.now); + if (report !== undefined) { + lastReport = report; yield* checks .progress({ repo: payload.github.repo, @@ -1075,7 +1083,7 @@ export class RunWorkflow extends WorkflowEntrypoint { output: { title: checkRunTitle, summary: queuedSummary( - decision.position, + report.bucket, decision.poolBusy, admissionCap, enqueuedAt + admissionMaxQueueAgeMs, @@ -1535,8 +1543,15 @@ export class RunWorkflow extends WorkflowEntrypoint { // has no `summary_json` for `/demos/:execution` to render. const demoSuffix = demoUrl !== undefined ? ` — [▶ view product demo ↗](${demoUrl})` : ""; const writebackSuffix = writebackLine !== undefined ? `\n\n${writebackLine}` : ""; - yield* checks.update({ + // + // The write runs in a durable step: it may create the check-run (an + // opener that degraded left none), and a replay must not create a + // second. `delivered_at` records that GitHub took it; a row without one + // is re-sent by the reconcile sweep (delivery-sweep.ts). + const delivery = yield* stepDo("deliver-verdict", () => Effect.runPromise(checks.conclude({ repo: payload.github.repo, + sha: payload.github.sha, + name: checkRunName, checkRunId, // `skipped` has no check-run conclusion of its own in the Checks // service — it lands as `neutral` (passes branch protection, renders @@ -1562,7 +1577,21 @@ export class RunWorkflow extends WorkflowEntrypoint { ), }), }, - }); + }))); + if (delivery._tag === "delivered") { + yield* Effect.tryPromise(() => + db.prepare(`UPDATE executions SET delivered_at = ?, check_run_id = ? WHERE id = ?`) + .bind(Date.now(), delivery.checkRunId, payload.executionId).run(), + ).pipe( + Effect.catchAll((cause) => + Effect.logWarning(`delivery: delivered_at not recorded — ${String(cause)}`), + ), + ); + } else { + yield* Effect.logWarning( + `delivery: verdict for ${payload.executionId} ${delivery._tag} (${delivery.reason}) — the reconcile sweep re-sends it`, + ); + } // A fan-out parent concludes any child check-run still open — one whose // own terminal update never landed (child-check-runs.ts). Best-effort. diff --git a/infra/migrations/0012_verdict_delivery.sql b/infra/migrations/0012_verdict_delivery.sql new file mode 100644 index 0000000..4c8f2e1 --- /dev/null +++ b/infra/migrations/0012_verdict_delivery.sql @@ -0,0 +1,25 @@ +-- Migration number: 0012 verdict delivery +-- +-- A computed verdict reaches the pull request only when the concluding +-- check-run write lands. These columns record whether it did: +-- +-- * `installation_id` — the GitHub App installation the execution's +-- check-run posts under. NULL when the execution has +-- no live check-run (no App credentials, or a dispatch +-- with no installation): no verdict is owed on GitHub. +-- * `delivered_at` — ms epoch the concluding write succeeded. NULL until +-- then. +-- * `delivery_refused_at` — ms epoch GitHub rejected the sweep's write for +-- the check-run itself (404 or 422). The row leaves +-- the sweep's queue; NULL otherwise. +-- +-- The reconcile sweep re-sends the conclusion of every finished execution with +-- an installation and neither stamp, oldest first (apps/dispatcher +-- delivery-sweep.ts). Rows written before this migration carry no +-- installation, so the sweep leaves them alone. +ALTER TABLE executions ADD COLUMN installation_id INTEGER; +ALTER TABLE executions ADD COLUMN delivered_at INTEGER; +ALTER TABLE executions ADD COLUMN delivery_refused_at INTEGER; +CREATE INDEX IF NOT EXISTS executions_undelivered + ON executions(completed_at) + WHERE delivered_at IS NULL AND installation_id IS NOT NULL AND delivery_refused_at IS NULL; diff --git a/packages/core/src/fakes/checks-fake.ts b/packages/core/src/fakes/checks-fake.ts index 1d34e89..ae3fd28 100644 --- a/packages/core/src/fakes/checks-fake.ts +++ b/packages/core/src/fakes/checks-fake.ts @@ -81,6 +81,12 @@ export const makeChecksFake = (): { Effect.sync(() => { state.updates.push({ checkRunId, repo, conclusion, output, detailsUrl }); }), + + conclude: ({ repo, checkRunId, conclusion, output, detailsUrl }) => + Effect.sync(() => { + state.updates.push({ checkRunId, repo, conclusion, output, detailsUrl }); + return { _tag: "delivered", checkRunId } as const; + }), }; return { layer: Layer.succeed(Checks, service), state }; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 44ef60e..d24d09a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -259,6 +259,8 @@ export { type CheckConclusion, type CheckOutput, type CheckRunState, + type CheckDelivery, + type CheckConcludeOptions, } from "./services/checks"; export { spawnChildRun, diff --git a/packages/core/src/services/checks.ts b/packages/core/src/services/checks.ts index 09acd0b..500c2dc 100644 --- a/packages/core/src/services/checks.ts +++ b/packages/core/src/services/checks.ts @@ -26,6 +26,32 @@ export type CheckOutput = { readonly summary: string; }; +/** + * Whether a concluding write reached GitHub. `delivered` carries the id of the + * check-run that holds the verdict — a fresh one when the opening create never + * landed. `undelivered` is a write worth sending again (a rate limit, an + * outage, missing credentials); `refused` is one GitHub rejected for this + * check-run itself (404 or 422 — the repository, installation or commit is + * gone, or the payload is invalid), so the same write fails again. Both carry + * the reason for the log line beside them. + */ +export type CheckDelivery = + | { readonly _tag: "delivered"; readonly checkRunId: string } + | { readonly _tag: "undelivered"; readonly reason: string } + | { readonly _tag: "refused"; readonly reason: string }; + +/** The options a concluding write takes. */ +export type CheckConcludeOptions = { + repo: string; + /** The head SHA and name — used when the check-run must be created at conclusion. */ + sha: string; + name: string; + checkRunId: string; + conclusion: CheckConclusion; + output?: CheckOutput; + detailsUrl?: string; +}; + /** * The service contract a runtime Layer implements. `create` returns the * GitHub-assigned check-run id, which the caller threads into `update`. @@ -66,6 +92,13 @@ export interface ChecksService { /** Optional "Details" link, preserved on completion. */ detailsUrl?: string; }) => Effect.Effect; + + /** + * Deliver an execution's verdict: complete `checkRunId`, or create the + * check-run already completed when the opening create returned no real id. + * Total — the outcome is data the caller records as `executions.delivered_at`. + */ + readonly conclude: (opts: CheckConcludeOptions) => Effect.Effect; } /** Context.Tag — the GitHub check-run dependency a run carries. */ @@ -100,4 +133,5 @@ export const checks = { output?: CheckOutput; detailsUrl?: string; }) => Effect.flatMap(Checks, (c) => c.update(opts)), + conclude: (opts: CheckConcludeOptions) => Effect.flatMap(Checks, (c) => c.conclude(opts)), } as const; diff --git a/packages/github-app/src/check-runs.test.ts b/packages/github-app/src/check-runs.test.ts index f4a309d..08df859 100644 --- a/packages/github-app/src/check-runs.test.ts +++ b/packages/github-app/src/check-runs.test.ts @@ -99,6 +99,40 @@ describe("createCheckRun", () => { expect(posts[0]!.body).not.toHaveProperty("details_url"); }); + it("creates the check-run already completed when a conclusion is given", async () => { + await createCheckRun({ + token: "ghs_test", + repo: "owner/name", + sha: "abc123", + name: "flare-dispatch/offload-test", + conclusion: "failure", + }); + expect(posts[0]!.body).toMatchObject({ status: "completed", conclusion: "failure" }); + expect(posts[0]!.body.completed_at).toEqual(expect.any(String)); + }); + + it("hands every response's headers to observe, a failed one included", async () => { + const seen: (string | null)[] = []; + server.use( + http.post("https://api.github.com/repos/:owner/:repo/check-runs", () => + HttpResponse.json( + { message: "slow down" }, + { status: 429, headers: { "x-ratelimit-remaining": "0", "retry-after": "5" } }, + ), + ), + ); + await expect( + createCheckRun({ + token: "ghs_test", + repo: "owner/name", + sha: "abc123", + name: "flare-dispatch/offload-test", + observe: (headers) => seen.push(headers.get("x-ratelimit-remaining")), + }), + ).rejects.toBeInstanceOf(GithubApiError); + expect(seen).toEqual(["0"]); + }); + it("surfaces a GithubApiError on a non-2xx response", async () => { server.use( http.post("https://api.github.com/repos/:owner/:repo/check-runs", () => diff --git a/packages/github-app/src/check-runs.ts b/packages/github-app/src/check-runs.ts index 79cd180..7de2a7d 100644 --- a/packages/github-app/src/check-runs.ts +++ b/packages/github-app/src/check-runs.ts @@ -37,6 +37,11 @@ type ClientOptions = { readonly apiBase?: string; /** `fetch` override — defaults to the global `fetch`. */ readonly fetchImpl?: typeof fetch; + /** + * Called with every response's headers, success or failure, before the + * status is checked — the caller reads the installation's quota from them. + */ + readonly observe?: (headers: Headers) => void; }; /** Options for {@link createCheckRun}. */ @@ -55,10 +60,16 @@ export type CreateCheckRunOptions = ClientOptions & { * reviewer jumps straight from the PR check to the execution's step logs. */ readonly detailsUrl?: string; + /** + * Create the check-run already `completed` with this verdict — the + * conclusion of an execution whose opening create never landed. + */ + readonly conclusion?: CheckConclusion; }; /** - * Create a check-run in `in_progress` and resolve to its GitHub-assigned id. + * Create a check-run and resolve to its GitHub-assigned id: `in_progress`, or + * `completed` when `conclusion` is set. * * @throws {GithubApiError} when the API returns non-2xx. */ @@ -66,19 +77,23 @@ export const createCheckRun = async (opts: CreateCheckRunOptions): Promise }), }); + opts.observe?.(res.headers); await assertOk(res, "check-run update failed"); }; @@ -182,6 +199,7 @@ export const getCheckRunStatus = async (opts: GetCheckRunStatusOptions): Promise headers: ghHeaders(opts.token), }); + opts.observe?.(res.headers); await assertOk(res, "check-run read failed"); const body = (await res.json()) as { status?: unknown }; diff --git a/packages/github-app/src/http.test.ts b/packages/github-app/src/http.test.ts index a09b2c0..bfaa16c 100644 --- a/packages/github-app/src/http.test.ts +++ b/packages/github-app/src/http.test.ts @@ -4,7 +4,7 @@ import { describe, expect, it } from "vitest"; import { GithubApiError } from "./errors"; -import { assertOk, retryAfterMsFromHeaders } from "./http"; +import { assertOk, rateLimitFromHeaders, retryAfterMsFromHeaders } from "./http"; describe("retryAfterMsFromHeaders", () => { it("reads Retry-After (whole seconds) as milliseconds", () => { @@ -79,3 +79,32 @@ describe("assertOk", () => { ).resolves.toBeUndefined(); }); }); + +describe("rateLimitFromHeaders", () => { + it("reads remaining, limit and the reset as ms epoch", () => { + const headers = new Headers({ + "x-ratelimit-remaining": "412", + "x-ratelimit-limit": "5000", + "x-ratelimit-reset": "1700000000", + }); + expect(rateLimitFromHeaders(headers)).toEqual({ + remaining: 412, + limit: 5000, + resetAtMs: 1_700_000_000_000, + }); + }); + + it("returns undefined when any of the three headers is missing", () => { + const headers = new Headers({ "x-ratelimit-remaining": "412", "x-ratelimit-limit": "5000" }); + expect(rateLimitFromHeaders(headers)).toBeUndefined(); + }); + + it("returns undefined for a non-numeric value", () => { + const headers = new Headers({ + "x-ratelimit-remaining": "lots", + "x-ratelimit-limit": "5000", + "x-ratelimit-reset": "1700000000", + }); + expect(rateLimitFromHeaders(headers)).toBeUndefined(); + }); +}); diff --git a/packages/github-app/src/http.ts b/packages/github-app/src/http.ts index 3ce6f7d..eab170b 100644 --- a/packages/github-app/src/http.ts +++ b/packages/github-app/src/http.ts @@ -78,6 +78,30 @@ export const retryAfterMsFromHeaders = ( return undefined; }; +/** An installation's primary quota as one response reported it. */ +export type RateLimitSnapshot = { + /** `x-ratelimit-remaining` — calls left in the current window. */ + readonly remaining: number; + /** `x-ratelimit-limit` — the window's size. */ + readonly limit: number; + /** `x-ratelimit-reset` as ms epoch — when the window refills. */ + readonly resetAtMs: number; +}; + +/** + * Read the primary-quota headers GitHub sends on every REST response, or + * `undefined` when any of the three is missing or non-numeric. + */ +export const rateLimitFromHeaders = (headers: Headers): RateLimitSnapshot | undefined => { + const remaining = Number(headers.get("x-ratelimit-remaining") ?? Number.NaN); + const limit = Number(headers.get("x-ratelimit-limit") ?? Number.NaN); + const reset = Number(headers.get("x-ratelimit-reset") ?? Number.NaN); + if (!Number.isFinite(remaining) || !Number.isFinite(limit) || !Number.isFinite(reset)) { + return undefined; + } + return { remaining, limit, resetAtMs: reset * 1000 }; +}; + /** * Throw a `GithubApiError` (status + body text + any rate-limit retry hint * attached) when a response is non-2xx. Each caller keeps its own success-body diff --git a/packages/github-app/src/index.ts b/packages/github-app/src/index.ts index d4157a5..ab3ac60 100644 --- a/packages/github-app/src/index.ts +++ b/packages/github-app/src/index.ts @@ -23,6 +23,7 @@ export { type GetInstallationTokenOptions, type ResolveRepoInstallationOptions, } from "./installation-token"; +export { rateLimitFromHeaders, type RateLimitSnapshot } from "./http"; export { createCheckRun, getCheckRunStatus, diff --git a/packages/runtime-cf/package.json b/packages/runtime-cf/package.json index 94d47d8..51200e4 100644 --- a/packages/runtime-cf/package.json +++ b/packages/runtime-cf/package.json @@ -11,7 +11,8 @@ "./log-names": "./src/log-names.ts", "./admission-cap": "./src/run-admission-d1.ts", "./spend": "./src/spend-d1.ts", - "./run-supersede": "./src/run-supersede-d1.ts" + "./run-supersede": "./src/run-supersede-d1.ts", + "./checks-github": "./src/checks-github.ts" }, "scripts": { "typecheck": "tsc --noEmit", diff --git a/packages/runtime-cf/src/checks-github.test.ts b/packages/runtime-cf/src/checks-github.test.ts index 7744eb7..fe2d5a1 100644 --- a/packages/runtime-cf/src/checks-github.test.ts +++ b/packages/runtime-cf/src/checks-github.test.ts @@ -21,10 +21,11 @@ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from import { checks } from "@fractalboxdev/flare-dispatch-core"; import { TEST_APP_PRIVATE_KEY } from "@fractalboxdev/flare-dispatch-github-app/testing"; import { type ChecksGithubConfig, makeChecksGithubLive, NOOP_CHECK_RUN_ID } from "./checks-github"; +import { __clearQuotas, quotaState } from "./github-quota"; /** Records the GitHub API calls MSW intercepted, for per-test assertions. */ type Recorded = { - posts: { status: unknown }[]; + posts: { status: unknown; conclusion?: unknown }[]; patches: { id: string; status: unknown; conclusion: unknown }[]; tokenExchanges: number; }; @@ -42,7 +43,7 @@ const server = setupServer( }), http.post("https://api.github.com/repos/:owner/:repo/check-runs", async ({ request }) => { const body = (await request.json()) as Record; - recorded.posts.push({ status: body.status }); + recorded.posts.push({ status: body.status, conclusion: body.conclusion }); return HttpResponse.json({ id: GH_CHECK_RUN_ID }, { status: 201 }); }), http.patch( @@ -66,6 +67,7 @@ afterAll(() => server.close()); beforeEach(() => { recorded = { posts: [], patches: [], tokenExchanges: 0 }; __clearTokenCache(); + __clearQuotas(); }); const CONFIG: ChecksGithubConfig = { @@ -217,3 +219,165 @@ describe("ChecksGithubLive — live GitHub App binding", () => { expect(recorded.tokenExchanges).toBe(0); }); }); + +/** Quota headers for a window resetting ten minutes from now. */ +const quotaHeaders = (remaining: number, limit = 5000): Record => ({ + "x-ratelimit-remaining": String(remaining), + "x-ratelimit-limit": String(limit), + "x-ratelimit-reset": String(Math.floor(Date.now() / 1000) + 600), +}); + +const REPO = { repo: "owner/name", sha: "abc123", name: "flare-dispatch/offload-test" }; + +describe("ChecksGithubLive — rate limits and the verdict reserve", () => { + it("waits out a Retry-After before the retry instead of the default backoff", async () => { + const at: number[] = []; + server.use( + http.post("https://api.github.com/repos/:owner/:repo/check-runs", () => { + at.push(Date.now()); + return at.length === 1 + ? HttpResponse.json({ message: "slow down" }, { status: 429, headers: { "retry-after": "1" } }) + : HttpResponse.json({ id: GH_CHECK_RUN_ID }, { status: 201 }); + }), + ); + + const id = await Effect.runPromise(checks.create(REPO).pipe(Effect.provide(makeChecksGithubLive(CONFIG)))); + + expect(id).toBe(String(GH_CHECK_RUN_ID)); + expect(at).toHaveLength(2); + expect(at[1]! - at[0]!).toBeGreaterThanOrEqual(990); + }); + + it("degrades a wait longer than one call allows and stops calling until it lapses", async () => { + let attempts = 0; + server.use( + http.post("https://api.github.com/repos/:owner/:repo/check-runs", () => { + attempts += 1; + return HttpResponse.json( + { message: "quota" }, + { status: 403, headers: { ...quotaHeaders(0), "retry-after": "600" } }, + ); + }), + ); + const layer = makeChecksGithubLive(CONFIG); + + const first = await Effect.runPromise(checks.create(REPO).pipe(Effect.provide(layer))); + expect(first).toBe(NOOP_CHECK_RUN_ID); + expect(attempts).toBe(1); + expect(quotaState(CONFIG.installationId, Date.now())).toBe("exhausted"); + + // The next create does not spend a call on a wait GitHub already named. + const second = await Effect.runPromise(checks.create(REPO).pipe(Effect.provide(layer))); + expect(second).toBe(NOOP_CHECK_RUN_ID); + expect(attempts).toBe(1); + }); + + it("stops position updates inside the reserve while create and conclude proceed", async () => { + server.use( + http.post("https://api.github.com/repos/:owner/:repo/check-runs", () => + HttpResponse.json({ id: GH_CHECK_RUN_ID }, { status: 201, headers: quotaHeaders(500) }), + ), + http.patch("https://api.github.com/repos/:owner/:repo/check-runs/:id", async ({ request, params }) => { + const body = (await request.json()) as Record; + recorded.patches.push({ id: String(params.id), status: body.status, conclusion: body.conclusion }); + return HttpResponse.json({ id: Number(params.id) }, { headers: quotaHeaders(499) }); + }), + ); + const layer = makeChecksGithubLive(CONFIG); + + const delivery = await Effect.runPromise( + Effect.gen(function* () { + const checkRunId = yield* checks.create(REPO); + yield* checks.progress({ + repo: REPO.repo, + checkRunId, + output: { title: "t", summary: "Queued" }, + }); + return yield* checks.conclude({ ...REPO, checkRunId, conclusion: "success" }); + }).pipe(Effect.provide(layer)), + ); + + expect(recorded.patches.map((p) => p.status)).toEqual(["completed"]); + expect(delivery).toEqual({ _tag: "delivered", checkRunId: String(GH_CHECK_RUN_ID) }); + }); + + it("writes position updates while the quota is above the reserve", async () => { + server.use( + http.post("https://api.github.com/repos/:owner/:repo/check-runs", () => + HttpResponse.json({ id: GH_CHECK_RUN_ID }, { status: 201, headers: quotaHeaders(501) }), + ), + ); + await Effect.runPromise( + Effect.gen(function* () { + const checkRunId = yield* checks.create(REPO); + yield* checks.progress({ repo: REPO.repo, checkRunId, output: { title: "t", summary: "Queued" } }); + }).pipe(Effect.provide(makeChecksGithubLive(CONFIG))), + ); + expect(recorded.patches.map((p) => p.status)).toEqual(["in_progress"]); + }); + + it("creates the check-run with the verdict when the opener degraded to the sentinel", async () => { + const delivery = await Effect.runPromise( + checks + .conclude({ ...REPO, checkRunId: NOOP_CHECK_RUN_ID, conclusion: "failure" }) + .pipe(Effect.provide(makeChecksGithubLive(CONFIG))), + ); + + expect(recorded.posts).toEqual([{ status: "completed", conclusion: "failure" }]); + expect(recorded.patches).toHaveLength(0); + expect(delivery).toEqual({ _tag: "delivered", checkRunId: String(GH_CHECK_RUN_ID) }); + }); + + it("reports a conclusion GitHub never took as undelivered instead of failing", async () => { + server.use( + http.patch("https://api.github.com/repos/:owner/:repo/check-runs/:id", () => + HttpResponse.json({ message: "down" }, { status: 503 }), + ), + ); + const delivery = await Effect.runPromise( + checks + .conclude({ ...REPO, checkRunId: "77", conclusion: "success" }) + .pipe(Effect.provide(makeChecksGithubLive(CONFIG))), + ); + expect(delivery).toEqual({ _tag: "undelivered", reason: "HTTP 503" }); + }); + + it.each([404, 422])("reports a conclusion GitHub rejects with %i as refused", async (status) => { + server.use( + http.patch("https://api.github.com/repos/:owner/:repo/check-runs/:id", () => + HttpResponse.json({ message: "no" }, { status }), + ), + ); + const delivery = await Effect.runPromise( + checks + .conclude({ ...REPO, checkRunId: "77", conclusion: "success" }) + .pipe(Effect.provide(makeChecksGithubLive(CONFIG))), + ); + expect(delivery).toEqual({ _tag: "refused", reason: `HTTP ${status}` }); + }); + + it("reports undelivered without a call when the installation is exhausted", async () => { + server.use( + http.post("https://api.github.com/repos/:owner/:repo/check-runs", () => + HttpResponse.json({ id: GH_CHECK_RUN_ID }, { status: 201, headers: quotaHeaders(0) }), + ), + ); + const layer = makeChecksGithubLive(CONFIG); + const checkRunId = await Effect.runPromise(checks.create(REPO).pipe(Effect.provide(layer))); + const delivery = await Effect.runPromise( + checks.conclude({ ...REPO, checkRunId, conclusion: "success" }).pipe(Effect.provide(layer)), + ); + expect(delivery._tag).toBe("undelivered"); + expect(recorded.patches).toHaveLength(0); + }); + + it("the no-op Layer reports every conclusion undelivered", async () => { + const delivery = await Effect.runPromise( + checks + .conclude({ ...REPO, checkRunId: NOOP_CHECK_RUN_ID, conclusion: "success" }) + .pipe(Effect.provide(makeChecksGithubLive(undefined))), + ); + expect(delivery._tag).toBe("undelivered"); + expect(recorded.posts).toHaveLength(0); + }); +}); diff --git a/packages/runtime-cf/src/checks-github.ts b/packages/runtime-cf/src/checks-github.ts index afeb64e..e83dd40 100644 --- a/packages/runtime-cf/src/checks-github.ts +++ b/packages/runtime-cf/src/checks-github.ts @@ -36,6 +36,25 @@ // not fail correctness. A genuine client error (bad App id / 401 / 404 — a // misconfiguration) is not retryable and still `orDie`s loudly. // +// --- Rate limits and the verdict reserve ------------------------------------- +// +// A retry waits at least as long as GitHub asked: the `Retry-After` header, or +// the reset of an exhausted window. A wait longer than `RETRY_AFTER_CAP_MS` +// is not slept through inside a Workflow step; the call degrades instead and +// the installation is marked exhausted until the wait lapses. +// +// Every response's quota headers feed github-quota.ts. Once an installation is +// down to its reserve (10% of the window), `progress` stops and only `create`, +// `update` and `conclude` proceed, so a burst of queue-position updates cannot +// spend the calls that open and conclude check-runs. +// +// `conclude` is the verdict write. It completes the check-run, or creates it +// already completed when the opener degraded to the sentinel id, and returns +// whether GitHub took it; it never fails, so the caller records the outcome +// as `executions.delivered_at` and the reconcile sweep re-sends the rest. A +// 404 or 422 comes back as `refused`: GitHub rejects that write for good, and +// the sweep stops re-sending it. +// // Spec: specs/04-gha-integration.md § Check-runs callback, specs/05-byoc.md // § Secrets / § Security posture, specs/pm/plan.md § PR6 / § 6. @@ -47,8 +66,14 @@ import { progressCheckRun, updateCheckRun, } from "@fractalboxdev/flare-dispatch-github-app"; -import { Effect, Layer, Schedule } from "effect"; -import { Checks, type CheckRunState, type ChecksService } from "@fractalboxdev/flare-dispatch-core"; +import { Clock, Duration, Effect, Layer, Schedule } from "effect"; +import { + type CheckDelivery, + Checks, + type CheckRunState, + type ChecksService, +} from "@fractalboxdev/flare-dispatch-core"; +import { blockQuota, observeQuota, quotaState, type QuotaState } from "./github-quota"; /** * The GitHub App credentials + installation context the live `Checks` Layer @@ -67,6 +92,13 @@ export type ChecksGithubConfig = { /** Sentinel id `create` returns when running degraded (no credentials). */ export const NOOP_CHECK_RUN_ID = "noop"; +/** The longest GitHub-signalled wait a retry sleeps through inside one call. */ +export const RETRY_AFTER_CAP_MS = 30_000; + +/** The wait GitHub signalled on a failed call, when it signalled one. */ +const retryAfterOf = (e: unknown): number | undefined => + e instanceof GithubAppApiError ? e.retryAfterMs : undefined; + /** * A check-run failure worth retrying: a rate limit (429, or a 403 carrying a * `Retry-After`) or a transient 5xx. A client error (401/404/malformed) is not @@ -77,23 +109,42 @@ const isRetryable = (e: unknown): boolean => e instanceof GithubAppApiError && (e.status === 429 || e.status >= 500 || (e.status === 403 && e.retryAfterMs !== undefined)); +/** + * A failure GitHub returns for this check-run itself — 404 (repository, + * installation or commit gone) or 422 (payload rejected). The same write + * fails again, unlike a rate limit, an outage or an App-wide 401. + */ +const isRefusal = (e: unknown): boolean => + e instanceof GithubAppApiError && (e.status === 404 || e.status === 422); + +/** A retryable failure whose signalled wait, if any, fits inside one call. */ +const isRetryableNow = (e: unknown): boolean => + isRetryable(e) && (retryAfterOf(e) ?? 0) <= RETRY_AFTER_CAP_MS; + /** * Bounded exponential backoff (4 retries, jittered) gated to the retryable - * classes. Jitter keeps a burst of concurrent runs from resynchronising onto - * GitHub. `whileInput` reads a data field, not a `._tag`, so it stays within the + * classes, each delay stretched to at least the wait GitHub signalled. Jitter + * keeps a burst of concurrent runs from resynchronising onto GitHub. + * `whileInput` reads a data field, not a `._tag`, so it stays within the * Effect convention for `Schedule` predicates. */ const checksRetry = Schedule.exponential("250 millis").pipe( Schedule.jittered, Schedule.intersect(Schedule.recurs(4)), - Schedule.whileInput(isRetryable), + Schedule.intersect(Schedule.identity()), + Schedule.whileInput(isRetryableNow), + Schedule.modifyDelay(([, e], delay) => + Duration.max(delay, Duration.millis(retryAfterOf(e) ?? 0)), + ), ); +/** A failed call, named for a log line. */ +const describeFailure = (e: unknown): string => + e instanceof GithubAppApiError ? `HTTP ${e.status}` : "transient failure"; + /** Message for the degrade-to-no-op path once retries are exhausted. */ const degradeMsg = (op: string, e: unknown): string => - `checks.${op} degraded after retries (${ - e instanceof GithubAppApiError ? `HTTP ${e.status}` : "transient failure" - }) — check-run not posted, execution unaffected`; + `checks.${op} degraded after retries (${describeFailure(e)}) — check-run not posted, execution unaffected`; /** * A `ChecksService` that does nothing but log — the graceful-degradation path @@ -111,6 +162,11 @@ const makeNoopChecks = (reason: string): ChecksService => ({ status: () => Effect.succeed("unknown"), update: ({ conclusion }) => Effect.logInfo(`checks.update skipped (${reason}) — conclusion "${conclusion}" not posted`), + conclude: ({ conclusion }) => + Effect.as( + Effect.logInfo(`checks.conclude skipped (${reason}) — conclusion "${conclusion}" not posted`), + { _tag: "undelivered", reason } as const, + ), }); /** @@ -120,10 +176,17 @@ const makeNoopChecks = (reason: string): ChecksService => ({ */ export const makeChecksGithubLive = ( config: ChecksGithubConfig | undefined, -): Layer.Layer => { - if (config === undefined) { - return Layer.succeed(Checks, makeNoopChecks("GitHub App credentials not configured")); - } +): Layer.Layer => + Layer.succeed( + Checks, + config === undefined + ? makeNoopChecks("GitHub App credentials not configured") + : makeChecksGithubService(config), + ); + +/** The live GitHub-App-backed `ChecksService` for one installation. */ +export const makeChecksGithubService = (config: ChecksGithubConfig): ChecksService => { + const installationId = config.installationId; // Resolve a fresh-or-cached installation token. The typed `GithubApiError` is // preserved (not flattened to a generic Error) so the per-method retry can @@ -134,7 +197,7 @@ export const makeChecksGithubLive = ( getInstallationToken({ appId: config.appId, privateKeyPem: config.privateKeyPem, - installationId: config.installationId, + installationId, }), catch: (cause) => cause instanceof GithubAppApiError @@ -144,34 +207,59 @@ export const makeChecksGithubLive = ( }), }); - const service: ChecksService = { + /** The calls this installation's quota admits now. */ + const admits: Effect.Effect = Effect.map(Clock.currentTimeMillis, (now) => + quotaState(installationId, now), + ); + + /** + * One GitHub call with the installation token: its response headers feed + * the quota cache, a signalled wait marks the installation exhausted, and a + * retryable failure is retried under `checksRetry`. + */ + const call = ( + what: string, + run: (token: string, observe: (headers: Headers) => void) => Promise, + ): Effect.Effect => + Effect.gen(function* () { + const accessToken = yield* token(); + return yield* Effect.tryPromise({ + try: () => run(accessToken, (headers) => observeQuota(installationId, headers)), + catch: (cause) => + cause instanceof GithubAppApiError + ? cause + : new Error(`ChecksGithubLive: check-run ${what} failed`, { cause }), + }); + }).pipe( + Effect.tapError((e) => { + const waitMs = retryAfterOf(e); + return waitMs === undefined + ? Effect.void + : Effect.flatMap(Clock.currentTimeMillis, (now) => + Effect.sync(() => blockQuota(installationId, now + waitMs)), + ); + }), + Effect.retry(checksRetry), + ); + + return { create: ({ repo, sha, name, output, detailsUrl }) => Effect.gen(function* () { - const accessToken = yield* token(); - return yield* Effect.tryPromise({ - try: () => - createCheckRun({ - token: accessToken, - repo, - sha, - name, - output, - detailsUrl, - }), - catch: (cause) => - cause instanceof GithubAppApiError - ? cause - : new Error("ChecksGithubLive: check-run create failed", { - cause, - }), - }); - }).pipe( - Effect.retry(checksRetry), - Effect.catchIf(isRetryable, (e) => - Effect.as(Effect.logWarning(degradeMsg("create", e)), NOOP_CHECK_RUN_ID), - ), - Effect.orDie, - ), + if ((yield* admits) === "exhausted") { + yield* Effect.logWarning( + `checks.create skipped — installation quota exhausted; the verdict creates check-run "${name}"`, + ); + return NOOP_CHECK_RUN_ID; + } + return yield* call("create", (t, observe) => + createCheckRun({ token: t, repo, sha, name, output, detailsUrl, observe }), + ).pipe( + Effect.catchIf(isRetryable, (e) => + Effect.as(Effect.logWarning(degradeMsg("create", e)), NOOP_CHECK_RUN_ID), + ), + Effect.orDie, + ); + }), progress: ({ repo, checkRunId, output, detailsUrl }) => Effect.gen(function* () { @@ -181,40 +269,30 @@ export const makeChecksGithubLive = ( yield* Effect.logInfo("checks.progress skipped — create returned the no-op sentinel id"); return; } - const accessToken = yield* token(); - yield* Effect.tryPromise({ - try: () => - progressCheckRun({ - token: accessToken, - repo, - checkRunId, - output, - detailsUrl, - }), - catch: (cause) => - cause instanceof GithubAppApiError - ? cause - : new Error("ChecksGithubLive: check-run progress update failed", { cause }), - }); - }).pipe( - Effect.retry(checksRetry), - Effect.catchIf(isRetryable, (e) => Effect.logWarning(degradeMsg("progress", e))), - Effect.orDie, - ), + // Position updates stop first: the reserve belongs to create and conclude. + const state = yield* admits; + if (state !== "open") { + yield* Effect.logInfo(`checks.progress skipped — installation quota ${state}`); + return; + } + yield* call("progress update", (t, observe) => + progressCheckRun({ token: t, repo, checkRunId, output, detailsUrl, observe }), + ).pipe( + Effect.catchIf(isRetryable, (e) => Effect.logWarning(degradeMsg("progress", e))), + Effect.orDie, + ); + }), status: ({ repo, checkRunId }) => checkRunId === NOOP_CHECK_RUN_ID ? Effect.succeed("unknown") : Effect.gen(function* () { - const accessToken = yield* token(); - return yield* Effect.tryPromise({ - try: () => getCheckRunStatus({ token: accessToken, repo, checkRunId }), - // Keep the typed error so `checksRetry` can classify a rate limit. - catch: (cause) => cause, - }); + if ((yield* admits) === "exhausted") return "unknown" as const; + const status = yield* call("read", (t, observe) => + getCheckRunStatus({ token: t, repo, checkRunId, observe }), + ); + return status === "completed" ? ("completed" as const) : ("open" as const); }).pipe( - Effect.retry(checksRetry), - Effect.map((status): CheckRunState => (status === "completed" ? "completed" : "open")), Effect.catchAllCause((cause) => Effect.as( Effect.logWarning(`checks.status unreadable — ${cause}`), @@ -231,30 +309,48 @@ export const makeChecksGithubLive = ( yield* Effect.logInfo("checks.update skipped — create returned the no-op sentinel id"); return; } - const accessToken = yield* token(); - yield* Effect.tryPromise({ - try: () => - updateCheckRun({ - token: accessToken, - repo, - checkRunId, - conclusion, - output, - detailsUrl, - }), - catch: (cause) => - cause instanceof GithubAppApiError - ? cause - : new Error("ChecksGithubLive: check-run update failed", { - cause, - }), - }); - }).pipe( - Effect.retry(checksRetry), - Effect.catchIf(isRetryable, (e) => Effect.logWarning(degradeMsg("update", e))), - Effect.orDie, - ), - }; + if ((yield* admits) === "exhausted") { + yield* Effect.logWarning( + `checks.update skipped — installation quota exhausted; conclusion "${conclusion}" not posted`, + ); + return; + } + yield* call("update", (t, observe) => + updateCheckRun({ token: t, repo, checkRunId, conclusion, output, detailsUrl, observe }), + ).pipe( + Effect.catchIf(isRetryable, (e) => Effect.logWarning(degradeMsg("update", e))), + Effect.orDie, + ); + }), - return Layer.succeed(Checks, service); + conclude: ({ repo, sha, name, checkRunId, conclusion, output, detailsUrl }) => + Effect.gen(function* () { + if ((yield* admits) === "exhausted") { + return { _tag: "undelivered", reason: "installation quota exhausted" } as const; + } + // An opener that degraded left no check-run: create it with the verdict. + const write: Effect.Effect = + checkRunId === NOOP_CHECK_RUN_ID + ? call("create", (t, observe) => + createCheckRun({ token: t, repo, sha, name, conclusion, output, detailsUrl, observe }), + ) + : call("update", (t, observe) => + updateCheckRun({ token: t, repo, checkRunId, conclusion, output, detailsUrl, observe }), + ).pipe(Effect.as(checkRunId)); + return yield* write.pipe( + Effect.map((id): CheckDelivery => ({ _tag: "delivered", checkRunId: id })), + Effect.catchAll((e) => { + const delivery: CheckDelivery = isRefusal(e) + ? { _tag: "refused", reason: describeFailure(e) } + : { _tag: "undelivered", reason: describeFailure(e) }; + return Effect.as( + Effect.logWarning( + `checks.conclude ${delivery._tag} (${delivery.reason}) — conclusion "${conclusion}" not posted`, + ), + delivery, + ); + }), + ); + }), + }; }; diff --git a/packages/runtime-cf/src/github-quota.test.ts b/packages/runtime-cf/src/github-quota.test.ts new file mode 100644 index 0000000..75bc6ff --- /dev/null +++ b/packages/runtime-cf/src/github-quota.test.ts @@ -0,0 +1,57 @@ +// Unit tests for the per-installation quota cache the checks client reads +// before each GitHub call. + +import { beforeEach, describe, expect, it } from "vitest"; +import { __clearQuotas, blockQuota, observeQuota, quotaState } from "./github-quota"; + +const NOW = 1_700_000_000_000; +const RESET_S = String(NOW / 1000 + 600); + +const quotaHeaders = (remaining: number, limit = 5000, reset = RESET_S): Headers => + new Headers({ + "x-ratelimit-remaining": String(remaining), + "x-ratelimit-limit": String(limit), + "x-ratelimit-reset": reset, + }); + +beforeEach(() => __clearQuotas()); + +describe("quotaState", () => { + it("is open for an installation no response has reported on", () => { + expect(quotaState(1, NOW)).toBe("open"); + }); + + it("is open above the reserve and reserve at or under 10% of the window", () => { + observeQuota(1, quotaHeaders(501)); + expect(quotaState(1, NOW)).toBe("open"); + observeQuota(1, quotaHeaders(500)); + expect(quotaState(1, NOW)).toBe("reserve"); + }); + + it("is exhausted at zero remaining until the window resets", () => { + observeQuota(1, quotaHeaders(0)); + expect(quotaState(1, NOW)).toBe("exhausted"); + expect(quotaState(1, NOW + 600_000)).toBe("open"); + }); + + it("is exhausted inside a Retry-After wait whatever the window reports", () => { + observeQuota(1, quotaHeaders(4000)); + blockQuota(1, NOW + 30_000); + expect(quotaState(1, NOW)).toBe("exhausted"); + // A later response inside the wait keeps the wait. + observeQuota(1, quotaHeaders(3999)); + expect(quotaState(1, NOW + 29_999)).toBe("exhausted"); + expect(quotaState(1, NOW + 30_000)).toBe("open"); + }); + + it("keeps installations apart", () => { + observeQuota(1, quotaHeaders(0)); + expect(quotaState(2, NOW)).toBe("open"); + }); + + it("ignores a response without quota headers", () => { + observeQuota(1, quotaHeaders(0)); + observeQuota(1, new Headers()); + expect(quotaState(1, NOW)).toBe("exhausted"); + }); +}); diff --git a/packages/runtime-cf/src/github-quota.ts b/packages/runtime-cf/src/github-quota.ts new file mode 100644 index 0000000..970b0e7 --- /dev/null +++ b/packages/runtime-cf/src/github-quota.ts @@ -0,0 +1,72 @@ +// @fractalboxdev/flare-dispatch-runtime-cf — per-installation GitHub quota. +// +// GitHub reports an installation's primary quota on every REST response +// (`x-ratelimit-remaining` / `-limit` / `-reset`), and a rate-limited response +// names its wait (`Retry-After`, or the reset of an exhausted window). The +// checks client records both here and reads them back before each call: +// +// * `open` — quota above the reserve: every call proceeds. +// * `reserve` — at or under 10% of the window: only create and conclude +// proceed; queue-position updates stop. +// * `exhausted` — nothing left, or inside a `Retry-After` wait: no call +// proceeds until the window resets or the wait lapses. +// +// The quota itself lives on GitHub, so the state here is a cache of GitHub's +// last answer, not a counter. It is per isolate: two isolates each hold the +// value their own last response carried, and each refreshes on its next call. +// Between calls an isolate may spend into the reserve that another isolate +// already saw, which bounds the overshoot at one call per isolate. + +import { rateLimitFromHeaders } from "@fractalboxdev/flare-dispatch-github-app"; + +/** The share of an installation's hourly quota kept for create and conclude calls. */ +export const QUOTA_RESERVE_FRACTION = 0.1; + +/** What the cache knows about one installation's quota. */ +type QuotaEntry = { + readonly remaining: number; + readonly limit: number; + readonly resetAtMs: number; + /** A `Retry-After` wait in force until this ms epoch, when one was signalled. */ + readonly blockedUntilMs?: number; +}; + +/** Which calls an installation's quota currently admits. */ +export type QuotaState = "open" | "reserve" | "exhausted"; + +const quotas = new Map(); + +/** Test hook: forget every installation's quota. */ +export const __clearQuotas = (): void => quotas.clear(); + +/** Record the quota a response's headers report for `installationId`. */ +export const observeQuota = (installationId: number, headers: Headers): void => { + const snapshot = rateLimitFromHeaders(headers); + if (snapshot === undefined) return; + const prior = quotas.get(installationId); + quotas.set(installationId, { + ...snapshot, + ...(prior?.blockedUntilMs !== undefined ? { blockedUntilMs: prior.blockedUntilMs } : {}), + }); +}; + +/** Record a rate-limit wait GitHub signalled for `installationId`. */ +export const blockQuota = (installationId: number, untilMs: number): void => { + const prior = quotas.get(installationId); + quotas.set(installationId, { + remaining: prior?.remaining ?? 0, + limit: prior?.limit ?? 0, + resetAtMs: prior?.resetAtMs ?? untilMs, + blockedUntilMs: Math.max(untilMs, prior?.blockedUntilMs ?? 0), + }); +}; + +/** The calls `installationId`'s quota admits at `nowMs`. */ +export const quotaState = (installationId: number, nowMs: number): QuotaState => { + const entry = quotas.get(installationId); + if (entry === undefined) return "open"; + if (entry.blockedUntilMs !== undefined && nowMs < entry.blockedUntilMs) return "exhausted"; + if (nowMs >= entry.resetAtMs || entry.limit <= 0) return "open"; + if (entry.remaining <= 0) return "exhausted"; + return entry.remaining <= Math.ceil(entry.limit * QUOTA_RESERVE_FRACTION) ? "reserve" : "open"; +}; diff --git a/wrangler.jsonc b/wrangler.jsonc index 8911bcc..59fe104 100644 --- a/wrangler.jsonc +++ b/wrangler.jsonc @@ -44,6 +44,10 @@ "0 7 * * 1", // finops-audit — Mondays "17 2 * * *", // contextful-measures — daily "17 3 * * SUN", // contextful-protocol — Sunday at 03:17 UTC + // The verdict reconcile sweep, not a run: re-sends the conclusion of + // every finished execution GitHub never received (DELIVERY_SWEEP_CRON, + // apps/dispatcher/src/delivery-sweep.ts). + "*/10 * * * *", ], },