diff --git a/README.md b/README.md index c7b5939b..8d4ef483 100644 --- a/README.md +++ b/README.md @@ -1650,7 +1650,13 @@ fitting several, such as a routed and a shadow reviewer at one effort whose sessions no record names, is split evenly among them and reported as `split`. Each risk tier's PRs, how many merged, how many a reviewer raised, and how many a later PR's `## Caused by` names -show whether the tiers predict what breaks. Over the merged +show whether the tiers predict what breaks. Each breakage a merged PR in the +window names under `## Caused by` follows on its own line, by culprit, even one +older than the window, which is read from GitHub: the culprit's tier, size, +writers, and local reviewers, who found it and the incident it closes when +the bullet ends with +`(found by check|reviewer|queue|canary|person|customer; incident )` as the +rules ask, either part alone allowed, and how long after the culprit merged the fix opened. Over the merged PRs come each online reviewer's coverage, final heads reviewed, findings per PR reviewed, and answers. A hand-back counts only while its draft is open, so a merged PR @@ -1757,7 +1763,11 @@ removes it. While no sender runs, stops still queue, and the next stop says so once per outage. Each pass runs token-max on this host from the earliest queued session's first -instant, reads the PRs updated since then, and posts `{"records": [...]}` with +instant, reads the PRs updated since then, or since its last upload for that +endpoint and repository read them, if earlier, so a fix merged after its +sessions were sent still goes with the next one, and posts +`{"records": [...], "breakages": [...]}`, the breakages being those the merged +PRs among them name under `## Caused by`, as `quest usage` reports them, with `Authorization: Bearer `, one upload per endpoint and repository, each queued session with its sub-agents, which stop without the hook naming them. Each upload is checked with `parseUpload` before it is posted. A 2xx drops the @@ -1768,6 +1778,29 @@ endpoint and repository back off, waiting a minute and doubling up to six hours, and sit out the passes meanwhile, so they never widen the others' window. token-max missing is retried each pass. +A deploy or alert reports what it found with `quest records incident`, which +posts one breakage at once to the checkout's endpoint and fails loud on any +answer but a 2xx, since nothing retries it: + +```console +$ quest records incident https://alerts.example/1 --found canary +reported https://alerts.example/1 open, its culprit unknown +$ quest records incident https://alerts.example/1 --found canary --pr 42 +reported https://alerts.example/1 against #42 +``` + +The ID is the reporter's, and a report with the same ID replaces the last. An +incident stays open, and the store counts open ones apart, until it is +reported again with `--pr`, or a merged fix names it on its culprit's +`## Caused by` line, as `(incident https://alerts.example/1)`: the sender then +sends the fix's breakage under the incident's ID, so it replaces the open one +as one record holding the culprit, the fix, and the incident. Where several +fixes name one incident, the one updated last wins. One fix closes several incidents of a +culprit with a line for each. `--found` is one of `check`, `reviewer`, `queue`, +`canary`, `person`, or `customer`, and `--at` sets when it was found, as +an ISO 8601 date and time with a `Z` or offset, by default now. Reverts and red default-branch runs are not reported here: the +store reads them from GitHub itself. + ### Typed messages `quest messages` prints the messages a person typed to agents on this host diff --git a/assets/CONTEXT.md b/assets/CONTEXT.md index f1e8c09b..a24d7574 100644 --- a/assets/CONTEXT.md +++ b/assets/CONTEXT.md @@ -65,7 +65,7 @@ A decision about an open PR never authorizes closing it. Confirm a close as its ## Pull requests Merge the base only when the branch conflicts with it or a landing step, such as `quest merge` or a merge queue, refuses the head, and resolve the conflicts in that merge; a branch only behind needs none: landing tests it on the current base. -Before each push to a PR's branch, confirm the PR is still open, since a push after it merges restores the deleted branch. +Before each push to a PR's branch, confirm it is still open: a push after it merges restores the deleted branch. Before a PR is marked ready for review, a model other than its latest writer reviews the committed branch locally, as `/quest-review` describes; a later push needs a new review unless `/quest-merge` exempts its changes. Anything AI-attributed, such as an online reviewer's findings, is a suggestion, not a maintainer's decision: address findings already posted on the head, reply to feedback you disagree with, and never wait for an online review, since the queue waits for any approval the base requires. @@ -74,9 +74,9 @@ Once you fix or refute an automated reviewer's finding, say so in its thread and The PR body's `## Models` section records its size as `Size: M` (its quest's, or for a skill's own PR, as a quest is sized) and one `Implementer: Codex, gpt-6.1-sol, high effort` line per implementing session, moved last whenever it pushes; write the model's ID, and `unknown` for whatever the harness doesn't report. -Keep pull request CI read-only (a read-only `GITHUB_TOKEN`, write credentials only in environments limited to protected branches): quest branches are pushed to the repository, so their workflows get its secrets. Flag a workflow that breaks this. +Keep pull request CI read-only (a read-only `GITHUB_TOKEN`, write credentials only in environments for protected branches): quest branches live in the repository, so their workflows get its secrets. Flag a workflow that breaks this. -A PR that fixes a bug, or redoes work an earlier PR did badly, lists that PR under `## Caused by` as `- [owner/repo#N](url) - what it got wrong`, found by `git blame` on the lines the fix changes or named in its quest's Plan, and confirmed in that PR's diff to introduce the defect, not merely touch the lines, defer the work, or keep older behavior, else traced further back with blame, or says in one line that no single PR caused it, such as an upstream change or a bug as old as the code. +A PR fixing a bug, or redoing an earlier PR's bad work, lists it under `## Caused by` as `- [owner/repo#N](url) - what it got wrong (found by check|reviewer|queue|canary|person|customer; incident )`, each part when known. Find it by blaming the fixed lines, or in its quest's Plan, and confirm its diff introduced the defect, not merely touched those lines, deferred work, or kept older behavior, else trace further back; or say in one line why no single PR did, such as an upstream change or a bug as old as the code. Review findings fixed in the same PR are not culprits. ## Quests diff --git a/docs/context.md b/docs/context.md index 31a6a257..2f015659 100644 --- a/docs/context.md +++ b/docs/context.md @@ -65,7 +65,7 @@ A decision about an open PR never authorizes closing it. Confirm a close as its ## Pull requests Merge the base only when the branch conflicts with it or a landing step, such as `quest merge` or a merge queue, refuses the head, and resolve the conflicts in that merge; a branch only behind needs none: landing tests it on the current base. -Before each push to a PR's branch, confirm the PR is still open, since a push after it merges restores the deleted branch. +Before each push to a PR's branch, confirm it is still open: a push after it merges restores the deleted branch. Before a PR is marked ready for review, a model other than its latest writer reviews the committed branch locally, as `/quest-review` describes; a later push needs a new review unless `/quest-merge` exempts its changes. Anything AI-attributed, such as an online reviewer's findings, is a suggestion, not a maintainer's decision: address findings already posted on the head, reply to feedback you disagree with, and never wait for an online review, since the queue waits for any approval the base requires. @@ -74,9 +74,9 @@ Once you fix or refute an automated reviewer's finding, say so in its thread and The PR body's `## Models` section records its size as `Size: M` (its quest's, or for a skill's own PR, as a quest is sized) and one `Implementer: Codex, gpt-6.1-sol, high effort` line per implementing session, moved last whenever it pushes; write the model's ID, and `unknown` for whatever the harness doesn't report. -Keep pull request CI read-only (a read-only `GITHUB_TOKEN`, write credentials only in environments limited to protected branches): quest branches are pushed to the repository, so their workflows get its secrets. Flag a workflow that breaks this. +Keep pull request CI read-only (a read-only `GITHUB_TOKEN`, write credentials only in environments for protected branches): quest branches live in the repository, so their workflows get its secrets. Flag a workflow that breaks this. -A PR that fixes a bug, or redoes work an earlier PR did badly, lists that PR under `## Caused by` as `- [owner/repo#N](url) - what it got wrong`, found by `git blame` on the lines the fix changes or named in its quest's Plan, and confirmed in that PR's diff to introduce the defect, not merely touch the lines, defer the work, or keep older behavior, else traced further back with blame, or says in one line that no single PR caused it, such as an upstream change or a bug as old as the code. +A PR fixing a bug, or redoing an earlier PR's bad work, lists it under `## Caused by` as `- [owner/repo#N](url) - what it got wrong (found by check|reviewer|queue|canary|person|customer; incident )`, each part when known. Find it by blaming the fixed lines, or in its quest's Plan, and confirm its diff introduced the defect, not merely touched those lines, deferred work, or kept older behavior, else trace further back; or say in one line why no single PR did, such as an upstream change or a bug as old as the code. Review findings fixed in the same PR are not culprits. ## Quests diff --git a/quest/m0/benchmark/breakage.md b/quest/m0/benchmark/breakage.md deleted file mode 100644 index e7602646..00000000 --- a/quest/m0/benchmark/breakage.md +++ /dev/null @@ -1,53 +0,0 @@ -# [M] Record what each merged PR broke - -## Goal - -Every breakage is recorded against the PR that caused it, with that PR's -tier, size, models, and the guardrails it passed, so the report answers -what broke, which layer should have caught it, and whether more or less -should have been spent on the culprit. A breakage is any of: - -- a later PR naming it under `## Caused by`, which `quest usage` counts - today; -- a revert of it; -- a merge queue entry or default-branch run it turned red; -- an incident a repository reports against it, such as a deploy rolled - back or a page, through a command a deploy or alert can call. - -Each breakage records how it was found (a check, a reviewer, the queue, a -canary, a person, a customer) and how long after the merge, since a -breakage a person finds costs more than one a check finds. - -## Plan - -Decided 2026-11-13 (user, /quest-plan): "keep stats about what we're -shipping to recalibrate. Like attributing risk/cost/effort/model and -identifying what broke. Then we can audit if more or fewer guardrails are -needed." - -[Routing evidence](/quest/m0/routing-evidence/README.md) left runtime -escapes to each repository's gate quests. That stands for fixing them; -this quest only records them. - -Agent's choices, for the implementer to revisit: - -- Reverts and red runs come from the forge, which Kixel Code's index - already reads; send what only `quest` knows, the `## Caused by` links - and reported incidents, as records. -- An incident whose culprit is unknown is recorded open, and the fix's - `## Caused by` closes it. Count open ones separately so they are never - read as zero. -- Fix-forward counts the same as a revert. Augment's write-up warns that - revert rate alone undercounts: "many defects are fixed forward or - discovered later." -- Report a rate per class of change and per tier with its sample size, and - no threshold: the maintainer decides what a rise means. -- Keep names of customers and private repositories out of anything this - public repository's records or PRs show. - -## Related - -- [Mistake audit](/quest/m0/mistake-audit.md) - classifies each escape as - catchable or runtime every week -- [Reviewer test set](/quest/m0/benchmark/reviewer-eval.md) - replays these - culprits against the reviewer diff --git a/quest/m0/benchmark/guardrails.md b/quest/m0/benchmark/guardrails.md index b6844bb2..f344f686 100644 --- a/quest/m0/benchmark/guardrails.md +++ b/quest/m0/benchmark/guardrails.md @@ -79,7 +79,5 @@ automatic merging. ## Related -- [Breakage](/quest/m0/benchmark/breakage.md) - the culprit counts each - class is judged on - [Routing evidence](/quest/m0/routing-evidence/README.md) - its shadow reviewer measures what a deeper review buys diff --git a/quest/m0/benchmark/reviewer-eval.md b/quest/m0/benchmark/reviewer-eval.md index 13f8e177..d0905e9a 100644 --- a/quest/m0/benchmark/reviewer-eval.md +++ b/quest/m0/benchmark/reviewer-eval.md @@ -34,11 +34,6 @@ Agent's choices, for the implementer to revisit: - Report recall by tier and size, so a reviewer effort can be cut where it finds nothing more. -## Required - -- [Record what each merged PR broke](/quest/m0/benchmark/breakage.md) - the - culprits with their cause and how they were found - ## Related - [Model check](/quest/m1/model-check.md) - retunes the reviewer table diff --git a/quest/m0/benchmark/risk-tier.md b/quest/m0/benchmark/risk-tier.md index 8fdd33fc..a278fc8c 100644 --- a/quest/m0/benchmark/risk-tier.md +++ b/quest/m0/benchmark/risk-tier.md @@ -33,5 +33,3 @@ covers only the repositories that have one. - [Development as a benchmark](/quest/m0/benchmark/README.md) - the decisions this builds on -- [Breakage](/quest/m0/benchmark/breakage.md) - records what each tier's - PRs later broke diff --git a/src/cli/breakage.ts b/src/cli/breakage.ts new file mode 100644 index 00000000..42c9002e --- /dev/null +++ b/src/cli/breakage.ts @@ -0,0 +1,92 @@ +// What a PR body says about breakage: the culprits its `## Caused by` names, and, for a culprit, how it was made and +// checked, as its own body records it. +import { type Culprit, FINDERS, type Finder, SIZES, type Seat, type Size, recordedRisk } from "../core"; +import type { Repository } from "../github/client"; +import { recorded } from "./models-record"; +import { parseReview } from "./review-record"; +import type { Pull } from "./usage"; + +/** One culprit a `## Caused by` section names, and who or what found the breakage and its incident, when its line says. */ +export interface Cause { + number: number; + found: Finder | null; + /** The ID of the incident the fix closes, as `quest records incident` reported it. */ + incident: string | null; +} + +/** + * The PRs of `repository` that `body`'s `## Caused by` section names as culprits: each bullet that opens with + * `[owner/repo#N](url) - why`. A link anywhere else on a line, as context or a "fixed by" aside, names no culprit. + * A line may end with a parenthesis of either part or both, as `(found by customer; incident )`: who found it, + * an article allowed, and the incident the fix closes. Each culprit and incident keeps its first line, and a + * culprit whose lines name incidents drops its line naming none. + */ +export function culprits(body: string, repository: Repository): Cause[] { + const heading = /^## Caused by[ \t]*\r?$/m.exec(body); + if (!heading) return []; + const rest = body.slice(heading.index + heading[0].length); + const section = rest.slice(0, /^## /m.exec(rest)?.index ?? rest.length); + const escape = (text: string) => text.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const name = `${escape(repository.owner)}/${escape(repository.repo)}`; + const culprit = new RegExp( + `^- \\[${name}#(\\d+)\\]\\(https://github\\.com/${name}/pull/(\\d+)\\) - \\S[^\\r\\n]*`, + "gim", + ); + const tail = new RegExp( + `\\((?:found by (?:an? |the )?(${FINDERS.join("|")})(?:; |(?=\\))))?(?:incident ([^\\r\\n]+?))?\\)\\s*$`, + "i", + ); + const found = new Map(); + for (const match of section.matchAll(culprit)) { + if (match[1] !== match[2]) continue; + const [, by, named] = tail.exec(match[0]) ?? []; + const cause: Cause = { + number: Number(match[1]), + found: (by?.toLowerCase() as Finder | undefined) ?? null, + incident: named?.trim() || null, + }; + const id = JSON.stringify([cause.number, cause.incident]); + if (!found.has(id)) found.set(id, cause); + } + // A line naming an incident records its culprit's breakage, so the culprit's line naming none would count it twice. + const named = new Set([...found.values()].filter((c) => c.incident !== null).map((c) => c.number)); + return [...found.values()].filter((c) => c.incident !== null || !named.has(c.number)); +} + +/** + * `pull` as a culprit. Its body is its author's text, so a record this version cannot read leaves the field empty + * rather than stopping the records of every other PR. + */ +export function culpritOf(pull: Pull): Culprit { + const own = recorded(pull.body); + let tier: Culprit["tier"] = null; + let reviewers: Seat[] = []; + try { + tier = recordedRisk(pull.body)?.tier ?? null; + } catch { + // Malformed: no tier. + } + try { + reviewers = parseReview(pull.body); + } catch { + // Malformed: no reviewers. + } + return { + number: pull.number, + merged: pull.state === "merged" && pull.closed !== null ? new Date(pull.closed).toISOString() : null, + tier, + size: (own.sizes.find((s) => (SIZES as readonly string[]).includes(s)) as Size | undefined) ?? null, + writers: distinct(own.implementers.flatMap(({ fields }) => (fields ? [fields] : []))), + reviewers: distinct(reviewers), + }; +} + +/** Each seat once, in first-named order, as several Implementer lines or a shadow may repeat one. */ +function distinct(seats: Seat[]): Seat[] { + const seen = new Map(); + for (const { harness, model, effort } of seats) { + const id = JSON.stringify([harness, model, effort]); + if (!seen.has(id)) seen.set(id, { harness, model, effort }); + } + return [...seen.values()]; +} diff --git a/src/cli/main.ts b/src/cli/main.ts index a15bda78..1f409028 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -21,6 +21,8 @@ import { questBranch, RISK_FILE, renderBlocker, + FINDERS, + type Finder, SIZES, type Size, unblocked, @@ -84,7 +86,15 @@ import { import { CHECKED_CONTEXT, USER_ASKED, questRoot } from "./rules"; import { messageLines, messages } from "./messages"; import { logWait, pullWaitKind, readWaits } from "./waits"; -import { CHECKOUT_CONFIG, SEND_INTERVAL, background, runSender, tokensPath } from "./send"; +import { + CHECKOUT_CONFIG, + SEND_INTERVAL, + background, + parseInstant, + reportIncident, + runSender, + tokensPath, +} from "./send"; import { installSender, uninstallSender } from "./service"; import { heal, init, stubFindings, uninstall } from "./setup"; import { Refusal, describe, notUpstream, releaseQuest, startQuest } from "./start"; @@ -1195,7 +1205,7 @@ export function main(argv: string[], out: Output): number | Promise { "declines it. Rollups follow: spend by size, role, harness, model, and " + "effort, accuracy by size and implementer, summing each branch's tallies but the shadow's once, where an Implementer line's own size, as (iterate, XS), wins " + "over the PR's, with its writing cost over the merged PRs it was found on, the writer A/B by size and arm, counting a sized PR whose every Implementer line names the same arm, as (arm B), under its first line's writer with the spend of every session on it, local reviewers by size, harness, model, effort, and shadow, with their " + - "tallies and reviewing cost per PR, each risk tier's PRs, merged PRs, tiers a reviewer raised, and PRs a later one fixed, and each online reviewer's coverage and findings over the merged PRs. Without token-max on PATH, it says how to install it and reports the PRs' records alone. " + + "tallies and reviewing cost per PR, each risk tier's PRs, merged PRs, tiers a reviewer raised, and PRs a later one fixed, each online reviewer's coverage and findings over the merged PRs, and each breakage a merged PR in the window names under ## Caused by: its culprit's tier, size, writers, and local reviewers, read from GitHub when older than the window, who found it and the incident it closes when the line ends with (found by check|reviewer|queue|canary|person|customer; incident ), either part alone allowed, and how long after the culprit merged the fix opened. Without token-max on PATH, it says how to install it and reports the PRs' records alone. " + "--json prints the same report as one object.\n\n" + "Uses GH_TOKEN, GITHUB_TOKEN, or gh auth token. Exits 0 when done, 2 for invalid arguments, and 1 for " + "other failures.", @@ -1234,7 +1244,7 @@ export function main(argv: string[], out: Output): number | Promise { .summary("Run the user's record sender until stopped, as its service does") .description( "Send each session record the stop hook queued, with the records of its sub-agents on this host, to the " + - `endpoint its checkout's ${CHECKOUT_CONFIG} names as records, posting {"records": [...]} with the token ` + + `endpoint its checkout's ${CHECKOUT_CONFIG} names as records, posting {"records": [...], "breakages": [...]}, the breakages the merged PRs among them name under ## Caused by, with the token ` + `${tokensPath()} pairs with that endpoint as a bearer token, one upload per endpoint and repository, ` + `then again every ${SEND_INTERVAL / 1000} seconds until stopped, at low priority. A record is built as quest usage ` + "builds it, from token-max's sessions on this host since the earliest queued session's first instant and " + @@ -1263,6 +1273,44 @@ export function main(argv: string[], out: Output): number | Promise { () => 1, ); }); + recordsCommand + .command("incident") + .summary("Report a breakage a deploy or alert found, open until its culprit is named") + .argument( + "", + "A stable ID for the incident, such as the alert's URL; a later report with it replaces this one", + ) + .description( + "Post one breakage to the endpoint this checkout's " + + `${CHECKOUT_CONFIG} names, with the token ${tokensPath()} pairs with it, as {"records": [], "breakages": [...]}. ` + + "Without --pr the incident is open, its culprit unknown; report the same ID again with --pr to close it on " + + "its culprit, whose tier, size, writers, and local reviewers are read from its PR body, or name it on the " + + "culprit's ## Caused by line of the fix, as (incident ), whose record then replaces it. It posts at once " + + "and retries nothing, for a deploy or alert to call.\n\nUses GH_TOKEN, GITHUB_TOKEN, or gh auth token. " + + "Exits 0 when the endpoint took it, 2 for invalid arguments, and 1 for other failures, refusal included.", + ) + .addOption(new Option("--found ", "Who or what found it").choices(FINDERS).makeOptionMandatory()) + .option("--pr ", "The PR that caused it", positiveInteger("--pr")) + .option("--at ", "When it was found, as an ISO 8601 date and time with a Z or offset (default: now)") + .action((id: string, options: { found: Finder; pr?: number; at?: string }) => { + if (/[\r\n]/.test(id) || id.trim() === "") + throw new UsageError("the incident ID must be one non-empty line"); + const at = options.at === undefined ? Date.now() : parseInstant(options.at); + if (at === null) + throw new UsageError( + `--at ${options.at} is not an ISO 8601 date and time with a Z or offset, on a real calendar day`, + ); + settle( + reportIncident( + { cwd: root(), id, found: options.found, pull: options.pr ?? null, at }, + { env: process.env, github: () => gitHubDependencies(warn), fetch: globalThis.fetch }, + ), + (breakage) => + print( + `reported ${breakage.incident} ${breakage.culprit ? `against #${breakage.culprit.number}` : "open, its culprit unknown"}`, + ), + ); + }); const serviceDeps = () => ({ platform: process.platform, env: process.env, diff --git a/src/cli/send.ts b/src/cli/send.ts index f65fdfff..bfd358d6 100644 --- a/src/cli/send.ts +++ b/src/cli/send.ts @@ -1,20 +1,21 @@ -// Sending each session's record to the Kixel Code endpoint its checkout names. The stop hook only queues the session; +// Sending each session's record, and the breakages its PRs name, to the Kixel Code endpoint its checkout names. The stop hook only queues the session; // one sender per user, run by its service manager, rebuilds each queued session's record, with its sub-agents', and // posts them, keeping what failed for the next pass. A record replaces its earlier copy, so a resend never doubles. import { spawnSync } from "node:child_process"; import { existsSync, mkdirSync, readdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; import { homedir, setPriority } from "node:os"; -import { dirname, join } from "node:path"; +import { dirname, join, resolve } from "node:path"; import { createHash, randomUUID } from "node:crypto"; -import { type SessionRecord, parseUpload } from "../core"; -import { type GitHubDependencies, GitHub } from "../github/client"; +import { type BreakageRecord, type Finder, type RecordUpload, parseUpload } from "../core"; +import { culpritOf } from "./breakage"; +import { CLOCK_SKEW, type GitHubDependencies, GitHub } from "../github/client"; import { baseRepository } from "./claims"; import { discoveryEnv } from "./git"; import { readRemote, remoteRepository } from "./github"; import { eachLine } from "./messages"; import { questRoot } from "./rules"; -import { type Pull, type Session, fold, key, sessionRecords, updatedPulls } from "./usage"; +import { type Pull, type Session, fixBreakages, fold, key, readPull, sessionRecords, updatedPulls } from "./usage"; import { type LoggedWait } from "./waits"; /** The checkout's file naming its record endpoint, as `{"records": "https://..."}`. */ @@ -251,6 +252,7 @@ export interface Sent { endpoint: string; repository: string; records: number; + breakages: number; /** The endpoint refused the token, so the records were dropped. */ refused: boolean; } @@ -293,7 +295,7 @@ export async function runSender(deps: SendDependencies, log: (line: string) => v log( each.refused ? `${each.endpoint} refused the token for ${each.repository}; ${each.records} records dropped` - : `sent ${each.records} records of ${each.repository} to ${each.endpoint}`, + : `sent ${each.records} records and ${each.breakages} breakages of ${each.repository} to ${each.endpoint}`, ); } catch (error) { log(error instanceof Error ? error.message : String(error)); @@ -362,7 +364,14 @@ async function sendOnce(deps: SendDependencies, dir: string): Promise { } const [owner, repo] = origin.split("/") as [string, string]; const repository = await baseRepository(client, { owner, repo }, prefix); - const pulls: Pull[] = await updatedPulls(client, repository, since); + // PRs are read from the last upload's read, if earlier, so a fix that merged after its sessions were sent, + // as through the merge queue, still reaches the store with the next session. + const read = deps.now() - CLOCK_SKEW; + const pulls: Pull[] = await updatedPulls( + client, + repository, + Math.min(since, readCursor(deps.env, id) ?? since), + ); const emit = new Set(); const missing: typeof group = []; for (const q of group) { @@ -431,17 +440,11 @@ async function sendOnce(deps: SendDependencies, dir: string): Promise { emit, since, }); - const upload = [...records.values()]; - try { - parseUpload({ records: upload }); - } catch (error) { - throw new Error(`this quest built an upload its own parseUpload refuses: ${(error as Error).message}`); - } - const response = await deps.fetch(endpoint, { - method: "POST", - headers: { authorization: `Bearer ${token}`, "content-type": "application/json" }, - body: JSON.stringify({ records: upload } satisfies { records: SessionRecord[] }), - }); + const upload: RecordUpload = { + records: [...records.values()], + breakages: await fixBreakages({ client, repository, pulls }), + }; + const response = await post(deps.fetch, endpoint, token, upload); if (response.status === 401 || response.status === 403) { refuse(deps.env, endpoint, token); clearBackoff(deps.env, id); @@ -449,14 +452,27 @@ async function sendOnce(deps: SendDependencies, dir: string): Promise { notes.push( `${endpoint} refused this host's token (${response.status}), so it is sent no more session records until the token changes`, ); - sent.push({ endpoint, repository: key(repository), records: upload.length, refused: true }); + sent.push({ + endpoint, + repository: key(repository), + records: upload.records.length, + breakages: upload.breakages.length, + refused: true, + }); continue; } if (!response.ok) throw new Error(`${endpoint} answered ${response.status}: ${(await response.text()).slice(0, 500)}`); clearBackoff(deps.env, id); + writeCursor(deps.env, id, read); for (const q of group) settle(q); - sent.push({ endpoint, repository: key(repository), records: upload.length, refused: false }); + sent.push({ + endpoint, + repository: key(repository), + records: upload.records.length, + breakages: upload.breakages.length, + refused: false, + }); } catch (error) { const wait = backOff(deps, id); failures.push( @@ -470,6 +486,99 @@ async function sendOnce(deps: SendDependencies, dir: string): Promise { return sent; } +/** + * `text` as epoch ms when it is an ISO 8601 date and time with an explicit `Z` or offset on a real calendar day, else + * null: `Date.parse` alone takes a bare date as UTC midnight, a time without an offset as this host's, and rolls an + * impossible day such as February 30 into the next month. + */ +export function parseInstant(text: string): number | null { + const match = + /^(\d{4})-(\d\d)-(\d\d)T([01]\d|2[0-3]):[0-5]\d(:[0-5]\d(\.\d+)?)?(Z|[+-]([01]\d|2[0-3]):[0-5]\d)$/.exec(text); + if (!match) return null; + const [year, month, day] = [Number(match[1]), Number(match[2]), Number(match[3])]; + const date = new Date(0); + date.setUTCFullYear(year, month - 1, day); + if (date.getUTCMonth() !== month - 1 || date.getUTCDate() !== day) return null; + const at = Date.parse(text); + return Number.isNaN(at) ? null : at; +} + +/** An incident to report, as a deploy or alert hands it to `quest records incident`. */ +export interface Incident { + /** A directory in the checkout whose endpoint and repository it is reported for. */ + cwd: string; + /** The reporter's stable ID for it, such as the alert's URL; reporting the same ID again replaces it. */ + id: string; + found: Finder; + /** The PR that caused it, or null while unknown, which leaves it open. */ + pull: number | null; + /** When it was found, epoch ms. */ + at: number; +} + +/** + * Post `incident` as a breakage to the endpoint the checkout's `${CHECKOUT_CONFIG}` names, with the token the user + * paired with it, and return it. Unlike the sender, it posts at once and fails loud on anything the endpoint does not + * take, refusal included, since a deploy or alert calls it once and nothing retries. + */ +export async function reportIncident( + incident: Incident, + deps: Pick, +): Promise { + const root = questRoot(resolve(incident.cwd)); + if (root === null) throw new Error(`${incident.cwd} is not in a checkout that uses Quest`); + const endpoint = checkoutEndpoint(root); + if (endpoint === null) throw new Error(`${join(root, CHECKOUT_CONFIG)} names no record endpoint`); + const token = tokenFor(endpoint, deps.env); + if (token === null) throw new Error(`${tokensPath(deps.env)} holds no token for ${endpoint}`); + const remote = readRemote(root, "origin"); + const origin = remote === null ? null : remoteRepository(remote).repository; + if (origin === null) throw new Error("origin is not on GitHub"); + const client = new GitHub(deps.github()); + const prefix = spawnSync("git", ["-C", root, "rev-parse", "--show-prefix"], { + encoding: "utf8", + env: discoveryEnv(), + }).stdout.trim(); + const repository = await baseRepository(client, origin, prefix); + let culprit: BreakageRecord["culprit"] = null; + if (incident.pull !== null) { + const found = await readPull(client, repository, incident.pull); + if (found === null) throw new Error(`${key(repository)} has no PR #${incident.pull}`); + culprit = culpritOf(found); + } + const breakage: BreakageRecord = { + repository: key(repository), + culprit, + fix: null, + incident: incident.id, + found: incident.found, + at: new Date(incident.at).toISOString(), + }; + const response = await post(deps.fetch, endpoint, token, { records: [], breakages: [breakage] }); + if (!response.ok) + throw new Error(`${endpoint} answered ${response.status}: ${(await response.text()).slice(0, 500)}`); + return breakage; +} + +/** POST `upload` to `endpoint` with `token`, after checking it is one the store's own `parseUpload` takes. */ +async function post( + fetch: typeof globalThis.fetch, + endpoint: string, + token: string, + upload: RecordUpload, +): Promise { + try { + parseUpload(upload); + } catch (error) { + throw new Error(`this quest built an upload its own parseUpload refuses: ${(error as Error).message}`); + } + return fetch(endpoint, { + method: "POST", + headers: { authorization: `Bearer ${token}`, "content-type": "application/json" }, + body: JSON.stringify(upload), + }); +} + /** The longest a failing group waits between tries. */ const MAX_BACKOFF = 6 * 3_600_000; @@ -479,6 +588,25 @@ interface Backoff { next: number; } +const cursorPath = (env: NodeJS.ProcessEnv, id: string) => join(stateDirectory(env), "read", digest(id)); + +/** When group `id`'s last successful upload read its PRs, epoch ms, or null; an unreadable file reads as none. */ +function readCursor(env: NodeJS.ProcessEnv, id: string): number | null { + try { + const at = Number(readFileSync(cursorPath(env, id), "utf8")); + return Number.isFinite(at) && at > 0 ? at : null; + } catch { + return null; + } +} + +function writeCursor(env: NodeJS.ProcessEnv, id: string, at: number): void { + mkdirSync(join(stateDirectory(env), "read"), { recursive: true, mode: 0o700 }); + const path = cursorPath(env, id); + writeFileSync(`${path}.new`, String(at)); + renameSync(`${path}.new`, path); +} + const backoffPath = (env: NodeJS.ProcessEnv, id: string) => join(stateDirectory(env), "backoff", digest(id)); /** A group's backoff, or null without one; an unreadable file reads as none, so it can never block every group. */ diff --git a/src/cli/usage.ts b/src/cli/usage.ts index 3c433b92..8fa66f5b 100644 --- a/src/cli/usage.ts +++ b/src/cli/usage.ts @@ -8,10 +8,11 @@ import { recorded } from "./models-record"; import { type ClaimDependencies, upstream } from "./claims"; import { repositoryPath } from "../github/client"; import { git } from "./git"; +import { culpritOf, culprits } from "./breakage"; import { fixes } from "./land-fixer"; import { MERGER } from "./land-watch-fix"; import { githubRepository, requireRemote } from "./github"; -import { type GitHub, type ObjectValue, type Repository, link } from "../github/client"; +import { type GitHub, type ObjectValue, type Repository, link, object } from "../github/client"; import { type OnlineReview, readOnlineReviews } from "../github/online-review"; import { type Counts, @@ -23,7 +24,7 @@ import { severityText, } from "./review-record"; import { SEVERITIES } from "./review"; -import { ROLES, type Role, type SessionRecord, WAIT_KINDS } from "../core"; +import { type BreakageRecord, ROLES, breakageKey, type Role, type Seat, type SessionRecord, WAIT_KINDS } from "../core"; import { buildRecords } from "./records"; import { type LoggedWait, readWaits } from "./waits"; @@ -210,6 +211,11 @@ export interface Usage { arms: ArmRow[]; tiers: TierRow[]; online: OnlineRow[]; + /** + * What the merged PRs in the window name under `## Caused by`, with each culprit as its body records it, wherever + * it falls; narrowed, those whose culprit or fix is reported. Incidents reach only the store. + */ + breakages: BreakageRecord[]; /** The sessions on no PR reported here, from every repository and agent token-max reads; null when narrowed. */ unattributed: ({ sessions: number } & Tokens) | null; /** Copies of a session found on several hosts, left out for its costliest copy, and the hosts they came from. */ @@ -402,20 +408,57 @@ export async function updatedPulls(client: GitHub, repository: Repository, since return pulls; } +/** The PR numbers of `repository` that `body`'s `## Caused by` section names as culprits, as {@link culprits} reads them. */ +export const causes = (body: string, repository: Repository): number[] => [ + ...new Set(culprits(body, repository).map((cause) => cause.number)), +]; + +/** PR `number` of `repository`, or null when GitHub has none, as for a mistyped `## Caused by` link. */ +export async function readPull(client: GitHub, repository: Repository, number: number): Promise { + const response = await client.request(`${repositoryPath(repository)}/pulls/${number}`, { allow: [404] }); + return response.status === 404 ? null : pull(object(await response.json())); +} + +/** What {@link fixBreakages} reads. */ +export interface FixSources { + client: GitHub; + repository: Repository; + pulls: Pull[]; + /** The PRs a narrowed report shows; unset, every breakage. */ + only?: Set; +} + /** - * The PR numbers of `repository` that `body`'s `## Caused by` section names as culprits: each bullet that opens with - * `[owner/repo#N](url) - why`. A link anywhere else on a line, as context or a "fixed by" aside, names no culprit. + * A breakage for each culprit the merged PRs among `pulls` name under `## Caused by`, found when the fix opened, with + * the incident its line closes. A culprit outside `pulls` is read from GitHub, once each; a link to no PR names no + * culprit. An incident several lines close keeps the line of the fix updated last, so no body can send one twice. + * Given `only`, it keeps only the breakages whose fix or culprit `only` names. */ -export function causes(body: string, repository: Repository): number[] { - const heading = /^## Caused by[ \t]*\r?$/m.exec(body); - if (!heading) return []; - const rest = body.slice(heading.index + heading[0].length); - const section = rest.slice(0, /^## /m.exec(rest)?.index ?? rest.length); - const escape = (text: string) => text.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - const name = `${escape(repository.owner)}/${escape(repository.repo)}`; - const culprit = new RegExp(`^- \\[${name}#(\\d+)\\]\\(https://github\\.com/${name}/pull/(\\d+)\\) - \\S`, "gim"); - const found = [...section.matchAll(culprit)].filter((match) => match[1] === match[2]); - return [...new Set(found.map((match) => Number(match[1])))]; +export async function fixBreakages({ client, repository, pulls, only }: FixSources): Promise { + const known = new Map(pulls.map((each) => [each.number, each])); + const breakages: BreakageRecord[] = []; + const seen = new Set(); + // GitHub lists the newest update first. + for (const fix of pulls.filter((each) => each.state === "merged")) + for (const cause of culprits(fix.body, repository)) { + if (cause.number === fix.number) continue; + // A narrowed report reads no culprit of a breakage it leaves out. + if (only && !only.has(fix.number) && !only.has(cause.number)) continue; + if (!known.has(cause.number)) known.set(cause.number, await readPull(client, repository, cause.number)); + const culprit = known.get(cause.number); + if (!culprit) continue; + const breakage: BreakageRecord = { + repository: key(repository), + culprit: culpritOf(culprit), + fix: fix.number, + incident: cause.incident, + found: cause.found, + at: new Date(fix.created).toISOString(), + }; + if (!seen.has(breakageKey(breakage))) breakages.push(breakage); + seen.add(breakageKey(breakage)); + } + return breakages; } const sized = (value: string | null | undefined): Size | null => @@ -1041,6 +1084,12 @@ export async function usage(root: string, options: UsageOptions, deps: UsageDepe // `quest ready` and hand edits drop the section before merge, so an earlier body counts too. if (read?.bodies.some(handedBack)) row.handedBack = true; } + const breakages = await fixBreakages({ + client, + repository, + pulls: all, + only: narrowed ? new Set(rows.map((row) => row.number)) : undefined, + }); return { repository: key(repository), since: options.since, @@ -1052,6 +1101,7 @@ export async function usage(root: string, options: UsageOptions, deps: UsageDepe arms: armRows(rows), tiers: tierRows(rows), online: onlineRows(rows), + breakages, unattributed, folded: { copies: folded.length, hosts: [...new Set(folded.map((session) => session.host))] }, incomplete: incomplete.map((session) => ({ @@ -1083,6 +1133,27 @@ const percent = (part: number, whole: number) => (whole ? `${Math.round((100 * p const answers = (r: Omit) => `${r.findings} finding${r.findings === 1 ? "" : "s"}: ${r.accepted} accepted, ${r.declined} declined, ${r.unclassified} unclassified, ${r.unanswered} unanswered`; +const seat = ({ model, effort }: Seat) => `${model} ${effort}`; + +/** A fix's breakage: its culprit's tier, size, writers, and reviewers, then who found it, and when after the merge. */ +function breakageText(b: BreakageRecord): string { + const c = b.culprit!; + const made = [ + c.tier ?? "no tier", + c.size ?? "unsized", + c.writers.length ? `written by ${c.writers.map(seat).join(", ")}` : "no writer recorded", + c.reviewers.length ? `reviewed by ${c.reviewers.map(seat).join(", ")}` : "no local review", + ].join(", "); + const ms = c.merged === null ? null : Date.parse(b.at) - Date.parse(c.merged); + const after = + ms === null + ? "never merged" + : ms < 0 + ? "opened before it merged" + : `${ms >= 172_800_000 ? `${(ms / 86_400_000).toFixed(1)}d` : duration(ms)} after merge`; + return `${made}; fixed by #${b.fix}${b.incident ? `, closing incident ${b.incident}` : ""}, ${b.found ? `found by ${b.found}` : "finder unrecorded"}, ${after}`; +} + /** Seconds under a minute, hours past two, else minutes. */ const duration = (ms: number) => ms < 60_000 @@ -1249,6 +1320,12 @@ export function usageLines(report: Usage): string[] { ]), ), ); + if (report.breakages.length) + lines.push( + "", + "Breakages, by the merged fix naming its culprit under ## Caused by:", + ...table(report.breakages.map((b) => [" ", `#${b.culprit!.number}`, breakageText(b)])), + ); const u = report.unattributed; if (u) lines.push("", `Unattributed: ${u.sessions} sessions on no PR here, any repository or agent: ${tokens(u)}`); const host = (name: string | null) => name ?? "local"; diff --git a/src/core/breakage.ts b/src/core/breakage.ts new file mode 100644 index 00000000..5162fa91 --- /dev/null +++ b/src/core/breakage.ts @@ -0,0 +1,104 @@ +// A breakage: what a merged PR broke, as `quest` sends it to Kixel Code beside the session records. It comes from a +// later PR naming the culprit under `## Caused by`, or from an incident a deploy or alert reports. Reverts and red +// default-branch runs are left to the store, which reads them from the forge itself. +import { SIZES, type Size } from "./doc"; +import { parser } from "./parse"; +import { TIERS, type Tier } from "./risk"; + +/** Who or what found a breakage, cheapest first: a person or customer finding it costs more than a check. */ +export const FINDERS = ["check", "reviewer", "queue", "canary", "person", "customer"] as const; +export type Finder = (typeof FINDERS)[number]; + +/** One writer or reviewer seat as the culprit's PR body records it. */ +export interface Seat { + harness: string; + model: string; + effort: string; +} + +/** The PR that caused a breakage, with what its body records of how it was made and checked. */ +export interface Culprit { + number: number; + /** When it merged, ISO 8601 UTC; null when it never did, as a closed PR whose commits landed elsewhere. */ + merged: string | null; + /** Its risk tier, from its `## Review` record; null when it records none or one this version cannot read. */ + tier: Tier | null; + /** Its size, from its `## Models` record. */ + size: Size | null; + /** The writers its `Implementer:` lines name. */ + writers: Seat[]; + /** The local reviewers its `## Review` record names, the guardrail it passed. */ + reviewers: Seat[]; +} + +/** + * One breakage. One naming an incident is keyed by its repository and that ID, else by its repository, culprit, and + * fix, so a resend replaces it. An open incident closes when it is reported again with its culprit, or when a merged + * fix's `## Caused by` names it: the fix's record, holding culprit, fix, and incident, then replaces it. A store + * replacing an incident keeps its earlier `at`, and its finder when the fix names none, since the fix opened later. + */ +export interface BreakageRecord { + /** The culprit's repository as lowercase `owner/repo`. */ + repository: string; + /** Null only for an incident whose culprit is not known yet. */ + culprit: Culprit | null; + /** The PR naming the culprit under `## Caused by`, or null for an incident a fix has not closed. */ + fix: number | null; + /** The ID its reporter gave an incident, such as the alert's URL, or null for a fix naming no incident. */ + incident: string | null; + /** Null when the fix's `## Caused by` line does not say. */ + found: Finder | null; + /** When it was found, ISO 8601 UTC: the incident's report, else the fix PR's opening. */ + at: string; +} + +const { fail, isObject, text, maybe, instant, oneOf, exact, pull, repository } = parser("breakage record"); + +const seat = (value: unknown): Seat => { + if (!isObject(value)) return fail("a seat"); + exact(value, ["harness", "model", "effort"], "a seat"); + return { + harness: text(value.harness, "a seat's harness"), + model: text(value.model, "a seat's model"), + effort: text(value.effort, "a seat's effort"), + }; +}; + +const seats = (value: unknown, what: string): Seat[] => (Array.isArray(value) ? value.map(seat) : fail(what)); + +function culprit(value: unknown): Culprit { + if (!isObject(value)) return fail("the culprit"); + exact(value, ["number", "merged", "tier", "size", "writers", "reviewers"], "the culprit"); + return { + number: pull(value.number, "the culprit's number"), + merged: value.merged === null ? null : instant(value.merged, "the culprit's merge"), + tier: value.tier === null ? null : oneOf(value.tier, TIERS, "the culprit's tier"), + size: value.size === null ? null : oneOf(value.size, SIZES, "the culprit's size"), + writers: seats(value.writers, "the culprit's writers"), + reviewers: seats(value.reviewers, "the culprit's reviewers"), + }; +} + +/** `value` as a breakage, failing on any missing, extra, or ill-typed field, or one naming neither fix nor incident. */ +export function parseBreakage(value: unknown): BreakageRecord { + if (!isObject(value)) return fail("not an object"); + exact(value, ["repository", "culprit", "fix", "incident", "found", "at"], "the breakage"); + const incident = maybe(value.incident, "the incident"); + if (incident !== null && /[\r\n]/.test(incident)) fail("the incident spans lines"); + const record: BreakageRecord = { + repository: repository(value.repository, "repository"), + culprit: value.culprit === null ? null : culprit(value.culprit), + fix: value.fix === null ? null : pull(value.fix, "the fix"), + incident, + found: value.found === null ? null : oneOf(value.found, FINDERS, "how it was found"), + at: instant(value.at, "when it was found"), + }; + if (record.fix === null && record.incident === null) fail("it names neither a fix nor an incident"); + if (record.fix !== null && record.culprit === null) fail("a fix names no culprit"); + if (record.fix !== null && record.fix === record.culprit?.number) fail("a PR fixes itself"); + return record; +} + +/** The identity a store replaces a breakage by. */ +export const breakageKey = (b: BreakageRecord): string => + JSON.stringify(b.incident === null ? [b.repository, b.culprit?.number, b.fix] : [b.repository, b.incident]); diff --git a/src/core/index.ts b/src/core/index.ts index cd18aee5..7b519732 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -94,6 +94,15 @@ export { latestHold, heldToken, } from "./hold"; +export { + type BreakageRecord, + type Culprit, + FINDERS, + type Finder, + type Seat, + breakageKey, + parseBreakage, +} from "./breakage"; export { type ModelSpend, type RecordUpload, diff --git a/src/core/parse.ts b/src/core/parse.ts new file mode 100644 index 00000000..04abed05 --- /dev/null +++ b/src/core/parse.ts @@ -0,0 +1,38 @@ +// Strict field checks for the records `quest` sends, so a store never keeps one in part. + +/** Checks that throw `malformed : ` on any missing, extra, or ill-typed field. */ +export function parser(what: string) { + const fail = (field: string): never => { + throw new Error(`malformed ${what}: ${field}`); + }; + const isObject = (value: unknown): value is Record => + typeof value === "object" && value !== null && !Array.isArray(value); + const text = (value: unknown, field: string): string => + typeof value === "string" && value !== "" ? value : fail(field); + const maybe = (value: unknown, field: string): string | null => (value === null ? null : text(value, field)); + const count = (value: unknown, field: string): number => + typeof value === "number" && Number.isFinite(value) && value >= 0 ? value : fail(field); + const flag = (value: unknown, field: string): boolean => (typeof value === "boolean" ? value : fail(field)); + const instant = (value: unknown, field: string): string => { + const at = text(value, field); + const match = /^(\d{4})-(\d\d)-(\d\d)T\d\d:\d\d:\d\d(\.\d+)?Z$/.exec(at); + if (!match || Number.isNaN(Date.parse(at))) return fail(field); + const date = new Date(0); + date.setUTCFullYear(Number(match[1]), Number(match[2]) - 1, Number(match[3])); + const valid = date.getUTCMonth() === Number(match[2]) - 1 && date.getUTCDate() === Number(match[3]); + return valid ? at : fail(field); + }; + const oneOf = (value: unknown, options: readonly T[], field: string): T => + options.includes(value as T) ? (value as T) : fail(field); + const exact = (value: Record, keys: string[], field: string) => { + const extra = Object.keys(value).find((key) => !keys.includes(key)); + if (extra !== undefined) fail(`${field} has an unknown field ${extra}`); + }; + const pull = (value: unknown, field: string): number => + Number.isSafeInteger(value) && (value as number) > 0 ? (value as number) : fail(field); + const repository = (value: unknown, field: string): string => { + const name = text(value, field); + return /^[a-z\d._-]+\/[a-z\d._-]+$/.test(name) ? name : fail(`${field} is not a lowercase owner/repo`); + }; + return { fail, isObject, text, maybe, count, flag, instant, oneOf, exact, pull, repository }; +} diff --git a/src/core/record.ts b/src/core/record.ts index 73523210..6a56e421 100644 --- a/src/core/record.ts +++ b/src/core/record.ts @@ -1,3 +1,6 @@ +import { type BreakageRecord, breakageKey, parseBreakage } from "./breakage"; +import { parser } from "./parse"; + // The record of one agent session that `quest` sends to Kixel Code, which stores exactly this shape, and that // `quest usage` prints. One session is one record: a resend under the same harness and session ID replaces it. A // host's one sender uploads a session's copies in order, so the latest upload is the newest copy. @@ -77,31 +80,25 @@ export interface SessionRecord { waits: SessionWait[] | null; } -/** The body of a record upload. */ +/** The body of a record upload: sessions' records, and the breakages the uploading host knows of. */ export interface RecordUpload { records: SessionRecord[]; + breakages: BreakageRecord[]; } -const fail = (what: string): never => { - throw new Error(`malformed session record: ${what}`); -}; -const isObject = (value: unknown): value is Record => - typeof value === "object" && value !== null && !Array.isArray(value); -const text = (value: unknown, what: string): string => (typeof value === "string" && value !== "" ? value : fail(what)); -const maybe = (value: unknown, what: string): string | null => (value === null ? null : text(value, what)); -const count = (value: unknown, what: string): number => - typeof value === "number" && Number.isFinite(value) && value >= 0 ? value : fail(what); -const flag = (value: unknown, what: string): boolean => (typeof value === "boolean" ? value : fail(what)); -const instant = (value: unknown, what: string): string => { - const at = text(value, what); - return /^\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d(\.\d+)?Z$/.test(at) && !Number.isNaN(Date.parse(at)) ? at : fail(what); -}; -const oneOf = (value: unknown, options: readonly T[], what: string): T => - options.includes(value as T) ? (value as T) : fail(what); -const exact = (value: Record, keys: string[], what: string) => { - const extra = Object.keys(value).find((key) => !keys.includes(key)); - if (extra !== undefined) fail(`${what} has an unknown field ${extra}`); -}; +const { + fail, + isObject, + text, + maybe, + count, + flag, + instant, + oneOf, + exact, + pull, + repository: ownerRepo, +} = parser("session record"); /** `value` as a record, failing on any missing, extra, or ill-typed field, so a store never keeps one in part. */ export function parseRecord(value: unknown): SessionRecord { @@ -126,8 +123,7 @@ export function parseRecord(value: unknown): SessionRecord { const pulls = Array.isArray(value.pulls) ? value.pulls : fail("pulls"); const models = Array.isArray(value.models) ? value.models : fail("models"); const waits = value.waits === null ? null : Array.isArray(value.waits) ? value.waits : fail("waits"); - const repository = text(value.repository, "repository"); - if (!/^[a-z\d._-]+\/[a-z\d._-]+$/.test(repository)) fail("repository is not a lowercase owner/repo"); + const repository = ownerRepo(value.repository, "repository"); const quest = maybe(value.quest, "quest"); if (quest !== null && !/^quest\/.+\.md$/.test(quest)) fail("quest is not a quest path"); const record: SessionRecord = { @@ -137,7 +133,7 @@ export function parseRecord(value: unknown): SessionRecord { repository, branch: maybe(value.branch, "branch"), quest, - pulls: pulls.map((n) => (Number.isSafeInteger(n) && (n as number) > 0 ? (n as number) : fail("a PR number"))), + pulls: pulls.map((n) => pull(n, "a PR number")), role: oneOf(value.role, ROLES, "role"), models: models.map((m: unknown) => { if (!isObject(m)) return fail("a model"); @@ -178,14 +174,20 @@ export function parseRecord(value: unknown): SessionRecord { return record; } -/** An upload's body, failing whole on any malformed record, one naming no session, or a session sent twice. */ +/** + * An upload's body, failing whole on any malformed record, one naming no session, or a session or breakage sent + * twice. + */ export function parseUpload(value: unknown): RecordUpload { if (!isObject(value)) return fail("the upload is not an object"); - exact(value, ["records"], "the upload"); + exact(value, ["records", "breakages"], "the upload"); if (!Array.isArray(value.records)) return fail("the upload has no record list"); + if (!Array.isArray(value.breakages)) return fail("the upload has no breakage list"); + const breakages = value.breakages.map(parseBreakage); + if (new Set(breakages.map(breakageKey)).size !== breakages.length) fail("the upload holds a breakage twice"); const records = value.records.map(parseRecord); if (records.some((r) => r.session === null)) fail("an uploaded record names no session"); const ids = new Set(records.map((r) => `${r.harness}\0${r.session}`)); if (ids.size !== records.length) fail("the upload holds a session twice"); - return { records }; + return { records, breakages }; } diff --git a/src/github/client.ts b/src/github/client.ts index 4b5cfe80..cf80a1f1 100644 --- a/src/github/client.ts +++ b/src/github/client.ts @@ -48,8 +48,8 @@ export type ObjectValue = Record; /** Retries of a 5xx or a dropped connection, waiting 1, 2, 4, then 8 seconds unless GitHub says how long. */ const TRANSIENT_RETRIES = 4; const backoff = (failures: number) => 1000 * 2 ** failures; -/** How far GitHub's clock may run behind ours when looking for what a lost request created. */ -const CLOCK_SKEW = 60_000; +/** How far GitHub's clock may run behind ours, as when looking for what a lost request created. */ +export const CLOCK_SKEW = 60_000; /** Statuses whose response has no body. */ const NULL_BODY = [204, 205, 304]; diff --git a/test/records.test.ts b/test/records.test.ts index a419bcc2..fe4cde97 100644 --- a/test/records.test.ts +++ b/test/records.test.ts @@ -3,7 +3,7 @@ import { join } from "node:path"; import { describe, expect, test } from "vitest"; -import { type SessionRecord, parseRecord, parseUpload } from "../src/core"; +import { type BreakageRecord, type SessionRecord, parseBreakage, parseRecord, parseUpload } from "../src/core"; import { buildRecords } from "../src/cli/records"; import { transcriptFacts } from "../src/cli/transcript"; import type { Pull, Session } from "../src/cli/usage"; @@ -97,13 +97,79 @@ describe("parseRecord", () => { }); test("an upload fails whole on one bad record or a session sent twice", () => { - expect(parseUpload({ records: [record(), record({ session: "s2" })] }).records).toHaveLength(2); - expect(() => parseUpload({ records: [record(), record({ session: "s2", cost: Number.NaN })] })).toThrow("cost"); - expect(() => parseUpload({ records: [record(), record()] })).toThrow("twice"); - expect(() => parseUpload({ records: [], more: true })).toThrow("unknown field"); + const upload = (records: unknown[]) => ({ records, breakages: [] }); + expect(parseUpload(upload([record(), record({ session: "s2" })])).records).toHaveLength(2); + expect(() => parseUpload(upload([record(), record({ session: "s2", cost: Number.NaN })]))).toThrow("cost"); + expect(() => parseUpload(upload([record(), record()]))).toThrow("twice"); + expect(() => parseUpload({ ...upload([]), more: true })).toThrow("unknown field"); + expect(() => parseUpload({ records: [] })).toThrow("no breakage list"); // A record may name no session, but no upload carries one, since the store replaces records by session. expect(parseRecord(record({ session: null })).session).toBeNull(); - expect(() => parseUpload({ records: [record({ session: null })] })).toThrow("names no session"); + expect(() => parseUpload(upload([record({ session: null })]))).toThrow("names no session"); + }); +}); + +const breakage = (extra: Partial = {}): BreakageRecord => ({ + repository: "acme/proj", + culprit: { + number: 5, + merged: "2026-11-01T10:00:00.000Z", + tier: "low", + size: "S", + writers: [{ harness: "Claude", model: "claude-opus-5-5", effort: "medium" }], + reviewers: [{ harness: "Codex", model: "gpt-6.1-sol", effort: "high" }], + }, + fix: 9, + incident: null, + found: "customer", + at: "2026-11-03T10:00:00.000Z", + ...extra, +}); + +describe("parseBreakage", () => { + test("keeps a fix's breakage and an open or closed incident exactly", () => { + for (const sent of [ + breakage(), + breakage({ fix: null, incident: "https://alerts.example/1", culprit: null, found: "canary" }), + breakage({ fix: null, incident: "deploy 42", found: null }), + // A fix closing an incident. + breakage({ incident: "deploy 42" }), + // A leap day is a real one. + breakage({ at: "2024-02-29T10:00:00Z" }), + ]) + expect(parseBreakage(JSON.parse(JSON.stringify(sent)))).toEqual(sent); + }); + + test.each([ + ["neither fix nor incident", breakage({ fix: null }), "neither a fix nor an incident"], + ["a fix with no culprit", breakage({ culprit: null }), "names no culprit"], + ["a PR fixing itself", breakage({ fix: 5 }), "fixes itself"], + ["an unknown finder", breakage({ found: "luck" as never }), "how it was found"], + ["an incident spanning lines", breakage({ fix: null, incident: "a\nb" }), "spans lines"], + ["an unknown tier", breakage({ culprit: { ...breakage().culprit!, tier: "severe" as never } }), "tier"], + [ + "an extra culprit field", + breakage({ culprit: { ...breakage().culprit!, cost: 1 } as never }), + "unknown field cost", + ], + ["a mixed-case repository", breakage({ repository: "Acme/proj" }), "lowercase"], + ["an impossible calendar day", breakage({ at: "2026-02-30T10:00:00Z" }), "when it was found"], + ])("refuses %s", (_, value, message) => { + expect(() => parseBreakage(value)).toThrow(message); + }); + + test("an upload holds each breakage once, a fix's by culprit and fix and an incident's by ID", () => { + const closed = breakage({ fix: null, incident: "i1" }); + const open = breakage({ fix: null, incident: "i1", culprit: null }); + expect( + parseUpload({ records: [], breakages: [breakage(), breakage({ fix: 10 }), closed] }).breakages, + ).toHaveLength(3); + expect(() => parseUpload({ records: [], breakages: [breakage(), breakage()] })).toThrow("breakage twice"); + expect(() => parseUpload({ records: [], breakages: [closed, open] })).toThrow("breakage twice"); + // A fix closing the incident replaces it, so the two never travel together. + expect(() => parseUpload({ records: [], breakages: [open, breakage({ incident: "i1" })] })).toThrow( + "breakage twice", + ); }); }); diff --git a/test/send.test.ts b/test/send.test.ts index 7e7b3029..ef3ec316 100644 --- a/test/send.test.ts +++ b/test/send.test.ts @@ -6,7 +6,15 @@ import { expect, test } from "vitest"; import { REFERENCE_LINE, parseUpload } from "../src/core"; import { hook } from "../src/cli/hooks"; -import { type SendDependencies, queue, runSender, sendPass, tokensPath } from "../src/cli/send"; +import { + type SendDependencies, + parseInstant, + queue, + reportIncident, + runSender, + sendPass, + tokensPath, +} from "../src/cli/send"; import type { Session } from "../src/cli/usage"; import { tempDir } from "./fixture"; @@ -101,6 +109,7 @@ function sender( status: number | (() => number), sessions: Session[] | null, during: () => void = () => {}, + pulls: object[] = [], ) { const uploads: { token: string; body: unknown }[] = []; const windows: Date[] = []; @@ -119,8 +128,10 @@ function sender( ssh_url: "git@github.com:acme/proj.git", fork: false, }); + if (url.pathname === "/repos/acme/proj/pulls/3") return json(CULPRIT); if (url.pathname === "/repos/acme/proj/pulls") return json([ + ...pulls, { number: 7, html_url: "https://github.com/acme/proj/pull/7", @@ -170,6 +181,21 @@ function sender( return { deps, uploads, windows, slept, clock }; } +/** A merged PR older than any window, which later PRs name as a culprit. */ +const CULPRIT = { + number: 3, + html_url: "https://github.com/acme/proj/pull/3", + title: "C", + head: { ref: "quest/m0/c", repo: { full_name: "acme/proj" } }, + state: "closed", + draft: false, + merged_at: "2026-10-01T00:00:00Z", + created_at: "2026-09-30T00:00:00Z", + updated_at: "2026-10-01T00:00:00Z", + closed_at: "2026-10-01T00:00:00Z", + body: "## Models\n\n- Size: M\n- Implementer: Claude Code, claude-opus-5-5, medium effort\n", +}; + const pending = (env: NodeJS.ProcessEnv) => readdirSync(join(env.XDG_STATE_HOME!, "quest", "records", "pending")); test("the sender posts each queued session with its sub-agents, from its transcript's start", async () => { @@ -182,7 +208,9 @@ test("the sender posts each queued session with its sub-agents, from its transcr session("other", "quest/m0/a"), ]; const { deps, uploads, windows } = sender(env, 200, sessions); - expect(await sendPass(deps)).toEqual([{ endpoint: ENDPOINT, repository: "acme/proj", records: 2, refused: false }]); + expect(await sendPass(deps)).toEqual([ + { endpoint: ENDPOINT, repository: "acme/proj", records: 2, breakages: 0, refused: false }, + ]); expect(windows).toEqual([new Date("2026-11-06T10:00:00Z")]); expect(uploads).toHaveLength(1); expect(uploads[0]!.token).toBe("Bearer secret"); @@ -208,7 +236,7 @@ test("a failed upload stays queued and is said once; a refused token stops sendi const refusing = sender(env, 403, [session("s1", "quest/m0/a")]); expect(await sendPass(refusing.deps)).toEqual([ - { endpoint: ENDPOINT, repository: "acme/proj", records: 1, refused: true }, + { endpoint: ENDPOINT, repository: "acme/proj", records: 1, breakages: 0, refused: true }, ]); expect(pending(env)).toEqual([]); expect(queue(stopped(root), env)).toEqual({ @@ -288,7 +316,9 @@ test("a session token-max no longer lists is dropped and said, without holding b queue(stopped(root), env); queue({ ...stopped(root), session: "gone" }, env); const { deps, uploads } = sender(env, 200, [session("s1", "quest/m0/a")]); - expect(await sendPass(deps)).toEqual([{ endpoint: ENDPOINT, repository: "acme/proj", records: 1, refused: false }]); + expect(await sendPass(deps)).toEqual([ + { endpoint: ENDPOINT, repository: "acme/proj", records: 1, breakages: 0, refused: false }, + ]); expect(parseUpload(uploads[0]!.body).records.map((r) => r.session)).toEqual(["s1"]); expect(pending(env)).toEqual([]); expect(queue(stopped(root), env).notices).toEqual([ @@ -386,7 +416,10 @@ test("the sender passes every interval, records its PID, and outlives a failed p expect(slept).toEqual([60_000, 60_000, 60_000]); // The failed pass keeps the session; the next sends it, and the last finds the queue empty. expect(uploads).toHaveLength(2); - expect(log).toEqual([expect.stringContaining("answered 500"), `sent 1 records of acme/proj to ${ENDPOINT}`]); + expect(log).toEqual([ + expect.stringContaining("answered 500"), + `sent 1 records and 0 breakages of acme/proj to ${ENDPOINT}`, + ]); expect(pending(env)).toEqual([]); }); @@ -440,3 +473,167 @@ test("a queued session for an endpoint that refused its token is dropped unsent" expect(later.uploads).toEqual([]); expect(pending(env)).toEqual([]); }); + +test("the sender sends the breakages a merged fix in its window names, reading an older culprit", async () => { + const env = home(); + queue(stopped(checkout(ENDPOINT)), env); + const fix = { + ...CULPRIT, + number: 8, + head: { ref: "fix/c", repo: { full_name: "acme/proj" } }, + merged_at: "2026-11-06T10:50:00Z", + created_at: "2026-11-06T10:20:00Z", + updated_at: "2026-11-06T10:50:00Z", + closed_at: "2026-11-06T10:50:00Z", + body: "## Caused by\n\n- [acme/proj#3](https://github.com/acme/proj/pull/3) - lost a frame (found by the queue)\n", + }; + const { deps, uploads } = sender(env, 200, [session("s1", "quest/m0/a")], () => {}, [fix]); + expect(await sendPass(deps)).toEqual([ + { endpoint: ENDPOINT, repository: "acme/proj", records: 1, breakages: 1, refused: false }, + ]); + expect(parseUpload(uploads[0]!.body).breakages).toEqual([ + { + repository: "acme/proj", + culprit: { + number: 3, + merged: "2026-10-01T00:00:00.000Z", + tier: null, + size: "M", + writers: [{ harness: "Claude Code", model: "claude-opus-5-5", effort: "medium" }], + reviewers: [], + }, + fix: 8, + incident: null, + found: "queue", + at: "2026-11-06T10:20:00.000Z", + }, + ]); +}); + +test("an incident is posted at once, open until reported again with its culprit, and fails loud", async () => { + const env = home(); + const root = checkout(ENDPOINT); + const { deps, uploads } = sender(env, 200, []); + const report = (pull: number | null) => + reportIncident( + { + cwd: root, + id: "https://alerts.example/1", + found: "canary", + pull, + at: Date.parse("2026-11-06T12:00:00Z"), + }, + deps, + ); + expect((await report(null)).culprit).toBeNull(); + expect((await report(3)).culprit).toMatchObject({ number: 3, size: "M" }); + expect(uploads.map((u) => parseUpload(u.body))).toEqual([ + { + records: [], + breakages: [ + expect.objectContaining({ + incident: "https://alerts.example/1", + culprit: null, + fix: null, + found: "canary", + at: "2026-11-06T12:00:00.000Z", + }), + ], + }, + { records: [], breakages: [expect.objectContaining({ culprit: expect.objectContaining({ number: 3 }) })] }, + ]); + await expect(report(4)).rejects.toThrow("acme/proj has no PR #4"); + await expect( + reportIncident({ cwd: root, id: "x", found: "person", pull: null, at: 0 }, sender(env, 403, []).deps), + ).rejects.toThrow("answered 403"); + await expect( + reportIncident({ cwd: root, id: "x", found: "person", pull: null, at: 0 }, sender(home(null), 200, []).deps), + ).rejects.toThrow("holds no token"); + await expect( + reportIncident({ cwd: checkout(), id: "x", found: "person", pull: null, at: 0 }, deps), + ).rejects.toThrow("names no record endpoint"); +}); + +test("a fix that merges after its sessions were sent reaches the store with the next session", async () => { + const env = home(); + const root = checkout(ENDPOINT); + queue(stopped(root), env); + const fix = (merged: boolean) => ({ + ...CULPRIT, + number: 8, + head: { ref: "fix/c", repo: { full_name: "acme/proj" } }, + state: merged ? "closed" : "open", + merged_at: merged ? "2026-11-06T12:30:00Z" : null, + closed_at: merged ? "2026-11-06T12:30:00Z" : null, + created_at: "2026-11-06T10:20:00Z", + updated_at: merged ? "2026-11-06T12:30:00Z" : "2026-11-06T10:20:00Z", + body: "## Caused by\n\n- [acme/proj#3](https://github.com/acme/proj/pull/3) - lost a frame\n", + }); + const open = sender(env, 200, [session("s1", "quest/m0/a")], () => {}, [fix(false)]); + expect((await sendPass(open.deps))[0]).toMatchObject({ records: 1, breakages: 0 }); + // The next session starts after the fix merged, so its own window alone would miss the fix. + writeFileSync(join(root, "later.jsonl"), `{"type":"user","timestamp":"2026-11-06T13:00:00Z"}\n`); + queue({ ...stopped(root), session: "s2", transcript: join(root, "later.jsonl") }, env); + const later = session("s2", "main", { + start: Date.parse("2026-11-06T13:00:00Z"), + end: Date.parse("2026-11-06T13:30:00Z"), + branches: [ + { name: "main", first: Date.parse("2026-11-06T13:00:00Z"), last: Date.parse("2026-11-06T13:30:00Z") }, + ], + }); + const merged = sender(env, 200, [later], () => {}, [fix(true)]); + merged.clock.now = Date.parse("2026-11-06T14:00:00Z"); + await sendPass(merged.deps); + expect(parseUpload(merged.uploads[0]!.body).breakages.map((b) => [b.culprit?.number, b.fix])).toEqual([[3, 8]]); +}); + +test("an incident reported from a subdirectory as `.`, as with no --root, finds its checkout", async () => { + const env = home(); + const root = checkout(ENDPOINT); + mkdirSync(join(root, "sub")); + const { deps } = sender(env, 200, []); + const before = process.cwd(); + process.chdir(join(root, "sub")); + try { + const reported = await reportIncident({ cwd: ".", id: "x", found: "person", pull: null, at: 0 }, deps); + expect(reported.repository).toBe("acme/proj"); + } finally { + process.chdir(before); + } +}); + +test("a merged fix naming an open incident sends one record under the incident, once however many name it", async () => { + const env = home(); + queue(stopped(checkout(ENDPOINT)), env); + const fix = (number: number, updated: string) => ({ + ...CULPRIT, + number, + head: { ref: `fix/${number}`, repo: { full_name: "acme/proj" } }, + merged_at: updated, + created_at: "2026-11-06T10:20:00Z", + updated_at: updated, + closed_at: updated, + body: "## Caused by\n\n- [acme/proj#3](https://github.com/acme/proj/pull/3) - lost a frame (found by canary; incident deploy 42)\n", + }); + const pulls = [fix(9, "2026-11-06T10:55:00Z"), fix(8, "2026-11-06T10:50:00Z")]; + const { deps, uploads } = sender(env, 200, [session("s1", "quest/m0/a")], () => {}, pulls); + await sendPass(deps); + expect(parseUpload(uploads[0]!.body).breakages.map((b) => [b.culprit?.number, b.fix, b.incident, b.found])).toEqual( + [[3, 9, "deploy 42", "canary"]], + ); +}); + +test("an incident's --at must be a date and time with a Z or offset, on a real calendar day", () => { + expect(parseInstant("2026-11-06T10:00:00Z")).toBe(Date.parse("2026-11-06T10:00:00Z")); + expect(parseInstant("2026-11-06T10:00+02:00")).toBe(Date.parse("2026-11-06T08:00:00Z")); + expect(parseInstant("2024-02-29T10:00:00.5Z")).toBe(Date.parse("2024-02-29T10:00:00.500Z")); + for (const text of [ + "2026-11-06", + "2026-11-06T10:00:00", + "2026-02-30T10:00:00Z", + "2026-11-06T24:00:00Z", + "2026-11-06T10:00:00+25:00", + "Nov 6 2026 10:00 UTC", + ]) + expect(parseInstant(text), text).toBeNull(); +}); diff --git a/test/usage.test.ts b/test/usage.test.ts index db5493e0..5eff8e83 100644 --- a/test/usage.test.ts +++ b/test/usage.test.ts @@ -8,6 +8,7 @@ import { implementer } from "../src/cli/models-record"; import { fixes } from "../src/cli/land-fixer"; import type { Risk, Round, Tally } from "../src/cli/review"; import { classifierSessions, parseReview, reviewRecord, reviewSessions } from "../src/cli/review-record"; +import { culprits } from "../src/cli/breakage"; import { type Session, causes, fold, parseSessions, share, usage, usageLines } from "../src/cli/usage"; import type { LoggedWait } from "../src/cli/waits"; import { tempDir } from "./fixture"; @@ -166,6 +167,30 @@ describe("causes", () => { ]); expect(causes(`- ${link(7)} - outside the section`, repository)).toEqual([]); }); + + test("reads who found it and the incident it closes from the parenthesis ending the line", () => { + const read = (line: string) => culprits(body(`- ${link(7)} - ${line}`), repository)[0]; + expect(read("lost a frame (found by canary; incident https://alerts.example/1)")).toEqual({ + number: 7, + found: "canary", + incident: "https://alerts.example/1", + }); + expect(read("lost a frame (incident deploy 42)\r")).toMatchObject({ found: null, incident: "deploy 42" }); + expect(read("lost a frame (found by the Queue)")).toMatchObject({ found: "queue", incident: null }); + // Only a parenthesis ending the line counts, and only a known finder. + expect(read("lost a frame (found by canary) on deploy")).toMatchObject({ found: null, incident: null }); + expect(read("lost a frame (found by luck)")).toMatchObject({ found: null, incident: null }); + // One fix closing two incidents of one culprit closes both, its line naming none dropped; the culprit counts once. + const lines = body( + `- ${link(7)} - lost a frame (incident a)\n- ${link(7)} - again (found by person; incident b)\n- ${link(7)} - plain\n- ${link(7)} - dup (incident a)\n- ${link(8)} - other`, + ); + expect(culprits(lines, repository)).toEqual([ + { number: 7, found: null, incident: "a" }, + { number: 7, found: "person", incident: "b" }, + { number: 8, found: null, incident: null }, + ]); + expect(causes(lines, repository)).toEqual([7, 8]); + }); }); describe("PR records", () => { @@ -605,8 +630,13 @@ describe("usage", () => { const unrated = body10.slice(body10.indexOf("## Review")).replace(`${RISK}\n`, ""); const body9 = `${unrated}\n\n## Models\n\n- Size: L\n- Implementer: Claude Code, claude-opus-5-5, high effort\n`; const fork13 = unrated.replaceAll(THREAD, "thread-fork"); + // #11 also names #2, older than the window and read on its own, which a customer found, and #99, which is no PR. const body11 = - "Fix.\n\n## Caused by\n\n- [acme/proj#10](https://github.com/acme/proj/pull/10) - missed a case\n"; + "Fix.\n\n## Caused by\n\n- [acme/proj#10](https://github.com/acme/proj/pull/10) - missed a case\n" + + "- [acme/proj#2](https://github.com/acme/proj/pull/2) - dropped a header (found by a customer; incident page 7)\n" + + "- [acme/proj#99](https://github.com/acme/proj/pull/99) - mistyped\n"; + const body2 = + "Old.\n\n## Models\n\n- Size: XS\n- Implementer: Codex, gpt-6.1-sol, low effort\n\n## Review\n\nRewritten by `quest review` after each round.\n\n- Risk: garbled\n"; // A closed duplicate of #11's fix: it never landed, so it fixed nothing. // Newest updated first, as GitHub lists them, so the report stops at #8. const pulls = [ @@ -757,6 +787,14 @@ describe("usage", () => { fork: false, }); if (url.pathname === "/repos/acme/proj/pulls") return json(pulls); + if (url.pathname === "/repos/acme/proj/pulls/2") + return json( + pr(2, "old/b", body2, "2026-10-19T00:00:00Z", { + merged_at: "2026-10-20T00:00:00Z", + closed_at: "2026-10-20T00:00:00Z", + updated_at: "2026-10-20T00:00:00Z", + }), + ); return new Response("not found", { status: 404 }); }; const calls: [Date, string[]][] = []; @@ -940,6 +978,26 @@ describe("usage", () => { "#10 L high merged: PR 10 (quest/m0/a)\n risk: high, It rewrites the wire format.; raised from medium by a reviewer\n", ); expect(text).toContain("#11 - - merged: PR 11 (fix/a)\n"); + // #11 breaks down by culprit, #2 read from GitHub; #12 never merged, and #99 is no PR. + expect(report.breakages.map((b) => [b.culprit?.number, b.fix, b.found, b.at])).toEqual([ + [10, 11, null, "2026-11-06T20:00:00.000Z"], + [2, 11, "customer", "2026-11-06T20:00:00.000Z"], + ]); + expect(report.breakages.map((b) => b.incident)).toEqual([null, "page 7"]); + expect(report.breakages[1]!.culprit).toEqual({ + number: 2, + merged: "2026-10-20T00:00:00.000Z", + tier: null, + size: "XS", + writers: [{ harness: "Codex", model: "gpt-6.1-sol", effort: "low" }], + reviewers: [], + }); + expect(report.breakages[0]!.culprit).toMatchObject({ tier: "high", size: "L" }); + expect(text).toContain( + "Breakages, by the merged fix naming its culprit under ## Caused by:\n" + + " #10 high, L, written by claude-opus-5-5 high, gpt-6.1-sol medium, reviewed by gpt-6.1-sol high, gemini-3-pro high, claude-opus-5-5 high, gpt-6.1-sol medium; fixed by #11, finder unrecorded, opened before it merged\n" + + " #2 no tier, XS, written by gpt-6.1-sol low, no local review; fixed by #11, closing incident page 7, found by customer, 17.8d after merge\n", + ); // #10 is the one PR with a tier, and a later PR fixed it; #11, #5, and #6 of the rest merged. expect(report.tiers).toEqual([ { tier: "high", pulls: 1, merged: 1, raised: 1, culprits: 1 }, @@ -1116,10 +1174,16 @@ describe("usage", () => { }); test("narrows to one PR, and reports the records alone without token-max", async () => { - const { root, deps, sessions } = setup(); + const { root, deps, sessions, requests } = setup(); const one = await usage(root, { since: "2026-11-05", pr: 10 }, deps(sessions)); expect(one.pulls.map((p) => p.number)).toEqual([10]); expect(one.pulls[0]!.fixedBy).toEqual([11]); + // Only #10's breakage is kept, and #11's other culprits, #2 and #99, are never read. + expect(one.breakages.map((b) => [b.culprit?.number, b.fix])).toEqual([[10, 11]]); + expect(requests.filter((path) => /\/pulls\/\d+$/.test(path))).toEqual([]); + // Narrowed to the fix, every culprit it names is kept. + const fix = await usage(root, { since: "2026-11-05", pr: 11 }, deps(sessions)); + expect(fix.breakages.map((b) => b.culprit?.number)).toEqual([10, 2]); expect(one.unattributed).toBeNull(); // The unreadable lines were an unattributed session's, which a narrowed report leaves out. expect(one.incomplete).toEqual([]);