diff --git a/packages/core/src/fakes/github-fake.ts b/packages/core/src/fakes/github-fake.ts index eb4dd4a..8bfc395 100644 --- a/packages/core/src/fakes/github-fake.ts +++ b/packages/core/src/fakes/github-fake.ts @@ -50,6 +50,14 @@ export type GithubFakeState = { state: "open" | "closed" | "all"; labels?: readonly string[]; updatedWithinDays?: number; + maxPages?: number; + /** + * Recorded, never simulated — the fake holds one page, so it can never + * truncate. A dedup read's correctness depends on ASKING for `strict`, and + * that is the half a test can pin here; the truncation itself is pinned + * against the wire in `issues.test.ts`. + */ + strict?: boolean; }>; /** Every `actionRuns` call, in order. */ readonly actionRunsCalls: Array<{ @@ -72,6 +80,20 @@ export type GithubFakeState = { readonly pullReviewCalls: PullReviewRequest[]; /** Every `openDraftPullRequest` call, in order. */ readonly openDraftPullRequestCalls: OpenDraftPullRequest[]; + /** + * Every `openIssue` call, in order — and each one also lands in `issues`. + * + * That second half is the point. A run whose whole job is "do not file what I + * already filed" can only be tested if a filed issue is visible to the next + * read, so the fake appends it rather than merely recording the call. Without + * that, every test would pass against a run that dedups against nothing. + */ + readonly openIssueCalls: Array<{ + repo: string; + title: string; + body: string; + labels?: readonly string[]; + }>; /** Every `createRelease` call, in order — lets a test assert a release published. */ readonly createReleaseCalls: CreateRelease[]; /** Every label ADD, in order. */ @@ -133,6 +155,7 @@ export const makeGithubFake = ( readTextFileCalls: [], pullReviewCalls: [], openDraftPullRequestCalls: [], + openIssueCalls: [], createReleaseCalls: [], addIssueLabelsCalls: [], removeIssueLabelCalls: [], @@ -146,9 +169,9 @@ export const makeGithubFake = ( const openedBranches = new Set(); const service: GithubService = { - issues: ({ repo, state: want = "open", labels, updatedWithinDays }) => + issues: ({ repo, state: want = "open", labels, updatedWithinDays, maxPages, strict }) => Effect.sync(() => { - state.issuesCalls.push({ repo, state: want, labels, updatedWithinDays }); + state.issuesCalls.push({ repo, state: want, labels, updatedWithinDays, maxPages, strict }); const need = labels === undefined ? undefined : new Set(labels); return state.issues.filter((i) => { if (i.repo !== repo) return false; @@ -164,6 +187,34 @@ export const makeGithubFake = ( }); }), + openIssue: ({ repo, title, body, labels }) => + Effect.sync(() => { + state.openIssueCalls.push({ repo, title, body, labels }); + // Numbered above every issue the fake knows about, in this repo or any + // other, because GitHub's numbering is per repo but a test asserting on + // `#3` should not have it mean two different issues. + const number = state.issues.reduce((max, i) => Math.max(max, i.number), 0) + 1; + const url = `https://github.com/${repo}/issues/${number}`; + state.issues = [ + ...state.issues, + { + repo, + number, + title, + body, + state: "open" as const, + labels: [...(labels ?? [])], + author: "flare-dispatch[bot]", + authorAssociation: "OWNER", + url, + commentCount: 0, + createdAt: now, + updatedAt: now, + }, + ]; + return { number, url }; + }), + // The writes record and mutate the seeded issue, so a test can assert both // "the call was made" and "the state machine advanced". addIssueLabels: ({ repo, issue, labels }) => @@ -195,7 +246,9 @@ export const makeGithubFake = ( Effect.sync(() => { state.closeIssueAsDuplicateCalls.push({ repo, issue, duplicateOf }); state.issues = state.issues.map((i) => - i.repo === repo && i.number === issue ? { ...i, state: "closed" as const } : i, + i.repo === repo && i.number === issue + ? { ...i, state: "closed" as const, closedAt: now } + : i, ); }), diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a22e4c0..d2f8807 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -153,6 +153,7 @@ export { github, Github, type GithubService, + type IssueCreated, type IssueRef, type PullRequestHistoryRef, type ReadTextFileRequest, diff --git a/packages/core/src/primitives/automerge-gate.test.ts b/packages/core/src/primitives/automerge-gate.test.ts index e010108..13d83c4 100644 --- a/packages/core/src/primitives/automerge-gate.test.ts +++ b/packages/core/src/primitives/automerge-gate.test.ts @@ -372,6 +372,7 @@ describe("evaluateAutomerge — a self-declared run marker is not authorship", ( const githubService = (over: Partial): GithubService => ({ issues: () => Effect.succeed([]), + openIssue: () => Effect.succeed({ number: 1, url: "" }), addIssueLabels: () => Effect.void, removeIssueLabel: () => Effect.void, commentOnIssue: () => Effect.void, diff --git a/packages/core/src/primitives/suppression.test.ts b/packages/core/src/primitives/suppression.test.ts index a78d75f..a7aa387 100644 --- a/packages/core/src/primitives/suppression.test.ts +++ b/packages/core/src/primitives/suppression.test.ts @@ -296,6 +296,7 @@ describe("decideSuppression", () => { /** A `GithubService` with every method stubbed, overridable per test. */ const githubService = (over: Partial): GithubService => ({ issues: () => Effect.succeed([]), + openIssue: () => Effect.succeed({ number: 1, url: "" }), addIssueLabels: () => Effect.void, removeIssueLabel: () => Effect.void, commentOnIssue: () => Effect.void, diff --git a/packages/core/src/primitives/suppression.ts b/packages/core/src/primitives/suppression.ts index 70d3c6a..69a62da 100644 --- a/packages/core/src/primitives/suppression.ts +++ b/packages/core/src/primitives/suppression.ts @@ -412,8 +412,18 @@ export type CheckSuppressionArgs = { readonly ledgerRef?: string; /** The repo prior proposals were opened against — defaults to `ledgerRepo`. */ readonly proposalRepo?: string; - /** The head-branch prefix every proposal of this kind shares. */ - readonly headBranchPrefix: string; + /** + * The head-branch prefix every proposal of this kind shares. + * + * **Omit it for a consumer that opens no PRs**, and the cooldown half is + * skipped entirely — no PR-history read, no spend, no cooldown. That is not a + * degraded mode: a cooldown dated from a closed PR is meaningless where no PR + * exists, and passing a prefix that matches nothing would answer "no prior + * proposals" every tick for a reason no reader could tell apart from "the + * feature is off". The spec-audit sweep is the first such consumer — its + * memory is the issue it filed, and only the ledger half applies here. + */ + readonly headBranchPrefix?: string; /** Now, in epoch ms — passed in so a run's clock is the one that decides. */ readonly nowMs: number; /** Cooldown length — defaults to {@link COOLDOWN_DAYS_DEFAULT}. */ @@ -464,25 +474,32 @@ export const checkSuppression = (args: CheckSuppressionArgs) => ); } - // 2. The cooldowns. Paginate no further back than the cooldown window — - // `updatedAt >= closedAt`, so nothing closed inside it can be missed. - const priorProposals = yield* github - .pullRequestHistory({ - repo: proposalRepo, - headBranchPrefix: args.headBranchPrefix, - state: "all", - updatedWithinDays: cooldownDays, - }) - .pipe( - Effect.catchTag("GitHubApiError", (err) => - Effect.gen(function* () { - const why = `PR history for ${proposalRepo} (${args.headBranchPrefix}*) unreadable (GitHub ${err.status} ${err.reason}) — cooldowns NOT applied this tick`; - degraded.push(why); - yield* io.log("warn", `suppression: ${why}`); - return [] as readonly PullRequestHistoryRef[]; - }), - ), - ); + // 2. The cooldowns — only for a consumer that opens PRs. With no prefix + // there is no PR to have been closed, so the read is skipped rather than + // made and ignored: an empty history would otherwise be recorded as "no + // prior proposals", which reads the same as a working cooldown finding + // nothing. + const headBranchPrefix = args.headBranchPrefix; + const priorProposals = + headBranchPrefix === undefined + ? ([] as readonly PullRequestHistoryRef[]) + : yield* github + .pullRequestHistory({ + repo: proposalRepo, + headBranchPrefix, + state: "all", + updatedWithinDays: cooldownDays, + }) + .pipe( + Effect.catchTag("GitHubApiError", (err) => + Effect.gen(function* () { + const why = `PR history for ${proposalRepo} (${headBranchPrefix}*) unreadable (GitHub ${err.status} ${err.reason}) — cooldowns NOT applied this tick`; + degraded.push(why); + yield* io.log("warn", `suppression: ${why}`); + return [] as readonly PullRequestHistoryRef[]; + }), + ), + ); const verdicts = decideSuppression({ candidates: args.keys, diff --git a/packages/core/src/services/github.ts b/packages/core/src/services/github.ts index ebcc45e..f62253a 100644 --- a/packages/core/src/services/github.ts +++ b/packages/core/src/services/github.ts @@ -50,6 +50,26 @@ export type IssueRef = { readonly createdAt: number; /** epoch ms. */ readonly updatedAt: number; + /** + * epoch ms when the issue was closed, or `undefined` while it is open. + * + * `updatedAt` cannot stand in: any touch resets it, so a window dated from it + * never expires. This is the column the org store's `pulls` table lacks and + * `issues` has — the whole reason an issue-shaped ledger needs no workaround + * where a PR-shaped one needed a file in git. + */ + readonly closedAt?: number; +}; + +/** The outcome of {@link GithubService.openIssue}. */ +export type IssueCreated = { + readonly number: number; + /** + * The issue's web URL. Never empty: a caller announces the issue by linking + * it, so a create that came back without one fails rather than publishing a + * link to nowhere. + */ + readonly url: string; }; /** @@ -394,9 +414,43 @@ export interface GithubService { labels?: readonly string[]; updatedWithinDays?: number; maxPages?: number; + /** + * Fail rather than return a list the page ceiling cut short. + * + * A triage pass wants the default: the 500 most recently updated issues are + * the tick's work and a longer backlog waits. A **deduplication** read + * cannot — it asks "have I filed this already?", and a truncated list says + * "no" for every issue it did not reach, so the caller duplicates whatever + * fell off the end. Set it wherever an absent row is read as a fact. + */ + strict?: boolean; installationId?: number; }) => Effect.Effect; + /** + * Open one issue — the write the spec-audit sweep files an open question with. + * + * **Fails rather than degrading to a logged no-op**, which is the opposite of + * `openDraftPullRequest` below, and the difference is what the artifact is. A + * PR write that no-ops loses nothing: the branch is idempotent and the content + * is a file that still exists in the commit the next tick will re-derive. Here + * the issue *is* the question — there is no file, no branch, and no second + * copy — so a silent no-op drops it, and the loop then has no record that it + * ever had something to ask. + * + * Narrow on purpose: a title, a body, and labels. No assignee, no milestone, + * no template. What bounds it is the caller — the sweep files into one control + * repo resolved from config, and never files a question it did not first fail + * to find among that repo's existing issues. + */ + readonly openIssue: (req: { + repo: string; + title: string; + body: string; + labels?: readonly string[]; + installationId?: number; + }) => Effect.Effect; + /** Add labels to an issue — the state machine's write (§5). */ readonly addIssueLabels: (req: { repo: string; @@ -497,8 +551,16 @@ export const github = { labels?: readonly string[]; updatedWithinDays?: number; maxPages?: number; + strict?: boolean; installationId?: number; }) => Effect.flatMap(Github, (g) => g.issues(opts)), + openIssue: (req: { + repo: string; + title: string; + body: string; + labels?: readonly string[]; + installationId?: number; + }) => Effect.flatMap(Github, (g) => g.openIssue(req)), addIssueLabels: (req: { repo: string; issue: number; diff --git a/packages/github-app/src/index.ts b/packages/github-app/src/index.ts index e0fa053..8ae3bfb 100644 --- a/packages/github-app/src/index.ts +++ b/packages/github-app/src/index.ts @@ -70,6 +70,7 @@ export { export { addIssueLabels, closeIssueAsDuplicate, + createIssue, createIssueComment, listIssues, removeIssueLabel, @@ -77,6 +78,8 @@ export { type AuthorAssociation, type CloseIssueAsDuplicateOptions, type CreateIssueCommentOptions, + type CreateIssueOptions, + type CreateIssueResult, type IssueListItem, type ListIssuesOptions, type RemoveIssueLabelOptions, diff --git a/packages/github-app/src/issues.test.ts b/packages/github-app/src/issues.test.ts index c77209c..cd680b8 100644 --- a/packages/github-app/src/issues.test.ts +++ b/packages/github-app/src/issues.test.ts @@ -22,6 +22,7 @@ import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; import { addIssueLabels, closeIssueAsDuplicate, + createIssue, createIssueComment, listIssues, removeIssueLabel, @@ -64,6 +65,13 @@ const server = setupServer( await capture(request, "list"); return HttpResponse.json(pages.shift() ?? []); }), + http.post("https://api.github.com/repos/:owner/:repo/issues", async ({ request }) => { + await capture(request, "create"); + return HttpResponse.json( + { number: 41, html_url: "https://github.com/owner/name/issues/41" }, + { status: 201 }, + ); + }), http.post( "https://api.github.com/repos/:owner/:repo/issues/:n/labels", async ({ request, params }) => { @@ -197,6 +205,90 @@ describe("listIssues", () => { ); await expect(listIssues({ ...base })).rejects.toThrow(); }); + + it("reads closed_at, and leaves it empty while the issue is open", async () => { + pages = [[rawIssue({ state: "closed", closed_at: "2026-08-10T09:00:00Z" }), rawIssue()]]; + const out = await listIssues({ ...base, state: "all" }); + expect(out[0]?.closedAt).toBe("2026-08-10T09:00:00Z"); + expect(out[1]?.closedAt).toBe(""); + }); + + // The dedup read's whole job is answering "have I already filed this?", and a + // list the ceiling cut short answers "no" for everything it never reached. + it("throws under `strict` when the page ceiling cut the list short", async () => { + const full = Array.from({ length: 100 }, (_, i) => rawIssue({ number: i + 1 })); + pages = [full, full]; + await expect(listIssues({ ...base, maxPages: 1, strict: true })).rejects.toThrow( + /page ceiling/, + ); + }); + + it("does not throw under `strict` when a short page ended the list", async () => { + pages = [[rawIssue()]]; + await expect(listIssues({ ...base, maxPages: 1, strict: true })).resolves.toHaveLength(1); + }); + + // The boundary case: a full last page that happens to be the last page. GitHub + // cannot say so, and neither can we — a strict caller is told to look again + // rather than shown a list nobody can vouch for. + it("throws under `strict` on an exactly-full last page", async () => { + pages = [Array.from({ length: 100 }, (_, i) => rawIssue({ number: i + 1 })), []]; + await expect(listIssues({ ...base, maxPages: 1, strict: true })).rejects.toThrow(); + }); +}); + +describe("createIssue", () => { + it("POSTs title, body and labels to /issues and returns the number and url", async () => { + const out = await createIssue({ + ...base, + title: "Is ADR-0002 accepted?", + body: "maintenance-key: org-spec-audit/memory-capability", + labels: ["maintenance:open-question", "question:decide"], + }); + + expect(out).toEqual({ number: 41, url: "https://github.com/owner/name/issues/41" }); + expect(calls[0]).toMatchObject({ method: "POST", path: "create" }); + expect(calls[0]?.body).toEqual({ + title: "Is ADR-0002 accepted?", + body: "maintenance-key: org-spec-audit/memory-capability", + labels: ["maintenance:open-question", "question:decide"], + }); + expect(calls[0]?.authorization).toBe("Bearer inst-token-abc"); + }); + + it("omits `labels` entirely when none were given", async () => { + await createIssue({ ...base, title: "t", body: "b" }); + expect(calls[0]?.body).toEqual({ title: "t", body: "b" }); + }); + + it("fails when the create returns no issue number — #0 links nowhere", async () => { + server.use( + http.post("https://api.github.com/repos/:owner/:repo/issues", () => + HttpResponse.json({ html_url: "https://github.com/owner/name/issues/?" }, { status: 201 }), + ), + ); + await expect(createIssue({ ...base, title: "t", body: "b" })).rejects.toThrow( + /no issue number/, + ); + }); + + it("fails when the create returns no url — a notice would link nowhere", async () => { + server.use( + http.post("https://api.github.com/repos/:owner/:repo/issues", () => + HttpResponse.json({ number: 41 }, { status: 201 }), + ), + ); + await expect(createIssue({ ...base, title: "t", body: "b" })).rejects.toThrow(/no html_url/); + }); + + it("surfaces a non-2xx", async () => { + server.use( + http.post("https://api.github.com/repos/:owner/:repo/issues", () => + HttpResponse.json({ message: "Resource not accessible by integration" }, { status: 403 }), + ), + ); + await expect(createIssue({ ...base, title: "t", body: "b" })).rejects.toThrow(); + }); }); describe("addIssueLabels", () => { diff --git a/packages/github-app/src/issues.ts b/packages/github-app/src/issues.ts index 01eb7f0..99bde2e 100644 --- a/packages/github-app/src/issues.ts +++ b/packages/github-app/src/issues.ts @@ -1,6 +1,7 @@ -// @fractalboxdev/flare-dispatch-github-app — the issue surface: one read, four writes. +// @fractalboxdev/flare-dispatch-github-app — the issue surface: one read, five writes. // // GET /repos/{o}/{r}/issues — list, carrying labels +// POST /repos/{o}/{r}/issues — CREATE one issue // POST /repos/{o}/{r}/issues/{n}/labels — add labels // DELETE /repos/{o}/{r}/issues/{n}/labels/{name} — remove one label // POST /repos/{o}/{r}/issues/{n}/comments — comment @@ -9,9 +10,25 @@ // // Why these and not a general issue API: `process/content/maintenance-loop.md` // §5 says **labels are the state machine**, and a classifier whose verdict -// cannot be recorded is spend with no artifact. This is exactly the set §5's -// issue machine names and nothing else — no create, no reopen, no edit, no -// milestone. A capability nobody needs is a capability waiting to be mis-wired. +// cannot be recorded is spend with no artifact. Still no reopen, no edit, no +// milestone, no assignee-clearing — a capability nobody needs is a capability +// waiting to be mis-wired. +// +// --- Why `createIssue` exists now, having deliberately not existed ----------- +// +// This module shipped with "no create" as a stated rule, because the triage desk +// only ever *routes* issues other people open. The spec-audit sweep changed what +// an issue is for: it files **one issue per open question**, and that issue is +// the loop's ledger entry rather than a report of anything. The state a decline +// needs — closed, and still closed a year later — is then the artifact itself, +// where the PR-shaped version needed a key, a second file in git, and a cooldown +// dated from a column the org store does not carry. +// +// The narrowness survives the addition. `createIssue` takes a title, a body and +// labels; there is no assignee, no milestone, no template, and no way to spell +// "create in a repo I was not given". What bounds it is the caller: the sweep +// files into ONE control repo it read from config, and it files nothing it did +// not first fail to find in that repo's existing issues. // // --- `closeIssueAsDuplicate` is not `closeIssue` ------------------------------ // @@ -42,6 +59,7 @@ // Provider-neutral plain `async`, no Effect — the Layer (`makeGithubLive`) wraps // these, the same split every other module in this package keeps. +import { GithubApiError } from "./errors"; import { assertOk, API_BASE_DEFAULT, ghHeaders, resolveClient, splitRepo } from "./http"; /** How GitHub describes the author's standing in the repo. */ @@ -69,6 +87,16 @@ export type IssueListItem = { readonly authorAssociation: AuthorAssociation; readonly createdAt: string; readonly updatedAt: string; + /** + * When the issue was closed, or `""` while it is open. + * + * Read because a cooldown needs it and `updatedAt` cannot substitute — any + * touch resets that one, so a window computed from it never expires. The org + * store's `pulls` table lacks this column entirely, which is why suppression + * reads GitHub rather than the store; `issues` has it, so an issue-shaped + * ledger needs no workaround. + */ + readonly closedAt: string; readonly url: string; readonly commentCount: number; }; @@ -95,6 +123,8 @@ type RawIssue = { readonly author_association?: unknown; readonly created_at?: unknown; readonly updated_at?: unknown; + /** `null` while the issue is open. */ + readonly closed_at?: unknown; readonly html_url?: unknown; readonly comments?: unknown; /** Present iff this "issue" is really a pull request. */ @@ -128,6 +158,7 @@ const normalizeIssue = (raw: RawIssue): IssueListItem | undefined => { authorAssociation: (ASSOCIATIONS.has(association) ? association : "NONE") as AuthorAssociation, createdAt: str(raw.created_at), updatedAt: str(raw.updated_at), + closedAt: str(raw.closed_at), url: str(raw.html_url), commentCount: typeof raw.comments === "number" ? raw.comments : 0, }; @@ -152,6 +183,23 @@ export type ListIssuesOptions = IssueCallBase & { readonly updatedSince?: number; /** Page ceiling — bounds a sweep over a large backlog. Default 5 (500 issues). */ readonly maxPages?: number; + /** + * Throw instead of returning a list the page ceiling cut short. + * + * A triage pass wants the default: reading the 500 most recently updated + * issues is the bound, and a backlog past it is simply not this tick's work. + * A **deduplication** read cannot live with that. It asks "have I already + * filed this question?", and a truncated list answers "no" for everything it + * did not reach — so the caller files a duplicate of every question that fell + * off the last page, which is the exact failure the read exists to prevent. + * + * Two returns are otherwise indistinguishable: the loop stops on a short page + * (the list is complete) or on the ceiling with a full page (there is more), + * and a bare array cannot tell them apart. Rather than return a shape every + * caller must then remember to check, the caller that cannot tolerate a + * partial read asks for `strict` and gets an error it already handles. + */ + readonly strict?: boolean; }; const PER_PAGE = 100; @@ -163,13 +211,16 @@ const MAX_PAGES_DEFAULT = 5; * Paginates to `maxPages` and stops early on a short page. The ceiling is a * bound on a scheduled sweep, not a correctness property: a backlog past it is * simply not read this tick, which is visible rather than silent because the - * caller knows what it asked for. + * caller knows what it asked for. Callers that need the *whole* set — see + * `strict` — get an error instead of a short list. */ export const listIssues = async (opts: ListIssuesOptions): Promise => { const { apiBase, doFetch } = resolveClient(opts); const { owner, name } = splitRepo(opts.repo); const maxPages = opts.maxPages ?? MAX_PAGES_DEFAULT; const out: IssueListItem[] = []; + /** Did the pagination end because GitHub ran out, or because we did? */ + let exhausted = false; for (let page = 1; page <= maxPages; page++) { const url = new URL(`${apiBase ?? API_BASE_DEFAULT}/repos/${owner}/${name}/issues`); @@ -188,16 +239,102 @@ export const listIssues = async (opts: ListIssuesOptions): Promise => { + const { apiBase, doFetch } = resolveClient(opts); + const { owner, name } = splitRepo(opts.repo); + const res = await doFetch(`${apiBase ?? API_BASE_DEFAULT}/repos/${owner}/${name}/issues`, { + method: "POST", + headers: ghHeaders(opts.token, { json: true }), + body: JSON.stringify({ + title: opts.title, + body: opts.body, + ...(opts.labels !== undefined && opts.labels.length > 0 ? { labels: opts.labels } : {}), + }), + }); + await assertOk(res, `issue create failed for ${opts.repo}`); + const created = (await res.json()) as { readonly number?: unknown; readonly html_url?: unknown }; + if (typeof created.number !== "number") { + throw new GithubApiError( + `issue create for ${opts.repo} returned no issue number`, + res.status, + "", + ); + } + // The URL is validated too, not just the number: a caller announces this issue + // by linking it, so `""` would publish a broken link rather than fail. Both + // fields are checked because both are load-bearing for the same caller. + const url = str(created.html_url); + if (url === "") { + throw new GithubApiError( + `issue create for ${opts.repo} returned no html_url (issue #${created.number} exists)`, + res.status, + "", + ); + } + return { number: created.number, url }; +}; + /** The number identifying one issue, on every write below. */ type IssueTarget = IssueCallBase & { readonly issue: number }; diff --git a/packages/runtime-cf/src/deferred.ts b/packages/runtime-cf/src/deferred.ts index d20756e..3b42912 100644 --- a/packages/runtime-cf/src/deferred.ts +++ b/packages/runtime-cf/src/deferred.ts @@ -126,6 +126,11 @@ export const GithubDeferred: Layer.Layer = Layer.succeed( // `issues` is the same class: an empty list reads as "nothing to triage", // which a scheduled run would act on by reporting a clean estate. issues: () => Effect.fail(new GitHubApiError({ status: 0, reason: "unauthorized" })), + // `openIssue` is the one WRITE in the fail class, because the issue it opens + // is the artifact and not a report of one. A logged skip here discards the + // question with nothing left holding it — no branch, no file, no second + // copy — and the run would report a clean sweep having asked nothing. + openIssue: () => Effect.fail(new GitHubApiError({ status: 0, reason: "unauthorized" })), // The state-machine writes degrade to a logged no-op, like `pullReview`. addIssueLabels: ({ repo, issue }) => Effect.logInfo( diff --git a/packages/runtime-cf/src/github-live.ts b/packages/runtime-cf/src/github-live.ts index 38217de..5c9807a 100644 --- a/packages/runtime-cf/src/github-live.ts +++ b/packages/runtime-cf/src/github-live.ts @@ -27,6 +27,7 @@ import { addIssueLabels, closeIssueAsDuplicate, + createIssue, createIssueComment, createPullReview, createRelease, @@ -46,6 +47,7 @@ import { Github, GitHubApiError, type GithubService, + type IssueCreated, type IssueRef, type PullRequestHistoryRef, type ReleaseResult, @@ -267,7 +269,7 @@ export const makeGithubLive = (config: GithubLiveConfig | undefined): Layer.Laye ); }), - issues: ({ repo, state, labels, updatedWithinDays, maxPages, installationId }) => + issues: ({ repo, state, labels, updatedWithinDays, maxPages, strict, installationId }) => Effect.gen(function* () { if (config === undefined) return yield* readNeedsCredentials(); const token = yield* mintToken(config, repo, installationId); @@ -281,22 +283,51 @@ export const makeGithubLive = (config: GithubLiveConfig | undefined): Layer.Laye ? { updatedSince: Date.now() - updatedWithinDays * 86_400_000 } : {}), ...(maxPages !== undefined ? { maxPages } : {}), + ...(strict === true ? { strict: true } : {}), + }), + ); + return raw.map((i): IssueRef => { + // `closed_at` is absent on an open issue and unparseable on a malformed + // one; both leave the field off rather than dating a close at the + // epoch, which would read as "closed in 1970" to anything computing a + // window from it. + const closedAt = i.closedAt === "" ? Number.NaN : Date.parse(i.closedAt); + return { + repo, + number: i.number, + title: i.title, + body: i.body, + state: i.state, + labels: i.labels, + author: i.author, + authorAssociation: i.authorAssociation, + url: i.url, + commentCount: i.commentCount, + createdAt: Date.parse(i.createdAt) || 0, + updatedAt: Date.parse(i.updatedAt) || 0, + ...(Number.isFinite(closedAt) ? { closedAt } : {}), + }; + }); + }), + + // `openIssue` is the exception to the paragraph below: it FAILS without + // credentials rather than logging a skip. The state-machine writes annotate + // an issue that exists whether or not the write lands, and the PR write + // leaves its content in git — but this one *is* the artifact, so a no-op + // discards the question and leaves the loop with no record it had one. + openIssue: ({ repo, title, body, labels, installationId }) => + Effect.gen(function* () { + if (config === undefined) return yield* readNeedsCredentials(); + const token = yield* mintToken(config, repo, installationId); + return yield* ghCall(() => + createIssue({ + token, + repo, + title, + body, + ...(labels !== undefined && labels.length > 0 ? { labels } : {}), }), ); - return raw.map((i): IssueRef => ({ - repo, - number: i.number, - title: i.title, - body: i.body, - state: i.state, - labels: i.labels, - author: i.author, - authorAssociation: i.authorAssociation, - url: i.url, - commentCount: i.commentCount, - createdAt: Date.parse(i.createdAt) || 0, - updatedAt: Date.parse(i.updatedAt) || 0, - })); }), // The four state-machine writes + the one close. Each degrades to a logged diff --git a/runs/org-spec-audit.test.ts b/runs/org-spec-audit.test.ts index 409dd6a..9e51ffd 100644 --- a/runs/org-spec-audit.test.ts +++ b/runs/org-spec-audit.test.ts @@ -1,6 +1,11 @@ // Run-level unit tests for `org-spec-audit` — drive the run against the // in-memory test runtime (`makeCFRuntimeTest`) with seeded config + sandbox + // model fakes. No CF, no Docker, no model provider. +// +// The property most of this file exists to pin: **a question already on file is +// not filed again.** The `github` fake appends every `openIssue` to the same +// list `issues` reads back, so "the second sweep sees the first sweep's issue" +// is expressible here rather than only in production. import { it } from "@effect/vitest"; import { Effect } from "effect"; @@ -9,15 +14,25 @@ import { makeCFRuntimeTest } from "@fractalboxdev/flare-dispatch-core/testing"; import { Github, GitHubApiError, + type IssueRef, type ModelCompletionResult, - type PullRequestHistoryRef, } from "@fractalboxdev/flare-dispatch-core"; import type { SuppressionReport } from "@fractalboxdev/flare-dispatch-core/primitives"; -import { mergeAcrossRepos, orgSpecAudit, parseWindowHours, renderMessage } from "./org-spec-audit"; +import { + firstMaintenanceKey, + indexFiledQuestions, + issueTitle, + mergeAcrossRepos, + orgSpecAudit, + parseLabel, + parsePositiveInt, + parseWindowHours, + renderIssueBody, + renderNotice, +} from "./org-spec-audit"; const firedAt = Date.UTC(2026, 7, 8); // 2026-08-08 const input = { firedAt } as const; -const DAY = 86_400_000; /** Nothing suppressed, nothing broken — the shape most render tests want. */ const noSuppression: SuppressionReport = { allowed: [], suppressed: [], degraded: [] }; @@ -28,6 +43,12 @@ const reported = (questions: unknown[]): ModelCompletionResult => ({ text: "", }); +/** A tools-mode result for the reconcile call — minted key → on-file key. */ +const matched = (matches: Array<{ minted: string; existing: string }>): ModelCompletionResult => ({ + toolCalls: [{ name: "report_key_matches", arguments: { matches } }], + text: "", +}); + const question = (over: Record = {}) => ({ group: "decide", question: "Does the dispatcher still commit to per-run spend caps?", @@ -38,6 +59,30 @@ const question = (over: Record = {}) => ({ ...over, }); +const KEY = "org-spec-audit/per-run-spend-caps"; +const LABEL = "maintenance:open-question"; + +/** A question already on file in the control repo — the ledger, seeded. */ +const filedIssue = (over: Partial & { key?: string } = {}): IssueRef => { + const { key = KEY, ...rest } = over; + return { + repo: "owner/control", + number: 41, + title: "Does the dispatcher still commit to per-run spend caps?", + // The key on the FIRST line, which is where `renderIssueBody` puts it. + body: `maintenance-key: ${key}\n\n\nprose`, + state: "open" as const, + labels: [LABEL, "question:decide"], + author: "flare-dispatch[bot]", + authorAssociation: "OWNER", + url: "https://github.com/owner/control/issues/41", + commentCount: 0, + createdAt: firedAt - 86_400_000, + updatedAt: firedAt - 86_400_000, + ...rest, + }; +}; + const baseConfig = { "org-spec-audit.repos": "owner/alpha owner/beta", "org-spec-audit.control-repo": "owner/control", @@ -65,12 +110,12 @@ describe("org-spec-audit", () => { return Effect.gen(function* () { const out = yield* orgSpecAudit.run(input); expect(out.reposSwept).toBe(0); - expect(out.prOpened).toBe(false); - expect(handles.github.openDraftPullRequestCalls).toHaveLength(0); + expect(out.questionsFiled).toBe(0); + expect(handles.github.openIssueCalls).toHaveLength(0); }).pipe(Effect.provide(layer)); }); - it.effect("merges the same question across repos into one control-plane PR", () => { + it.effect("merges the same question across repos into ONE issue", () => { const { layer, handles } = makeCFRuntimeTest({ config: baseConfig, sandboxProgram: activeSandbox, @@ -78,6 +123,7 @@ describe("org-spec-audit", () => { // Both repos raise the same key — the merge is the point of sweeping. responses: [reported([question()]), reported([question()])], }, + github: { now: firedAt }, }); return Effect.gen(function* () { @@ -85,25 +131,151 @@ describe("org-spec-audit", () => { expect(out.reposSwept).toBe(2); expect(out.questionsRaised).toBe(2); expect(out.questionsAfterMerge).toBe(1); + expect(out.questionsFiled).toBe(1); - const calls = handles.github.openDraftPullRequestCalls; + const calls = handles.github.openIssueCalls; expect(calls).toHaveLength(1); expect(calls[0]!.repo).toBe("owner/control"); - expect(calls[0]!.headBranch).toBe("flare-dispatch/spec-audit-questions-2026-08-08"); - // The neutral default — `questions-dir` is unset in `baseConfig`, and no - // value in this repo names any particular operator's layout. - expect(calls[0]!.files[0]!.path).toBe("maintenance/questions/2026-08-08.md"); - // Both repos are named as sources on the single merged line. - expect(calls[0]!.files[0]!.content).toContain("owner/alpha"); - expect(calls[0]!.files[0]!.content).toContain("owner/beta"); - expect(calls[0]!.body).toContain("auto-merge: never"); + expect(calls[0]!.title).toBe("Does the dispatcher still commit to per-run spend caps?"); + // Both repos are named as sources on the single merged issue. + expect(calls[0]!.body).toContain("owner/alpha"); + expect(calls[0]!.body).toContain("owner/beta"); + // The index label and the lane label, from the defaults. + expect(calls[0]!.labels).toEqual([LABEL, "question:decide"]); + }).pipe(Effect.provide(layer)); + }); + + // The property the whole redesign exists for. #147 and #148 asked three of the + // same questions two days apart, two of them under a byte-identical key, + // because nothing read the key back against a question that was merely OPEN. + it.effect("files nothing when every question is already on file", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: baseConfig, + sandboxProgram: activeSandbox, + modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { now: firedAt, issues: [filedIssue()] }, + }); + + return Effect.gen(function* () { + const out = yield* orgSpecAudit.run(input); + expect(out.questionsAfterMerge).toBe(1); + expect(out.questionsAlreadyFiled).toBe(1); + expect(out.questionsFiled).toBe(0); + expect(handles.github.openIssueCalls).toHaveLength(0); + // Silent in the channel too: re-announcing a standing question is the + // daily file again, one line long. + expect(handles.notice.published).toHaveLength(0); + }).pipe(Effect.provide(layer)); + }); + + it.effect("never re-files or reopens a question that was answered and closed", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: baseConfig, + sandboxProgram: activeSandbox, + modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { + now: firedAt, + issues: [filedIssue({ state: "closed", closedAt: firedAt - 200 * 86_400_000 })], + }, + }); + + return Effect.gen(function* () { + const out = yield* orgSpecAudit.run(input); + // Closed means decided. Not a cooldown, not a 30-day window — a question + // answered 200 days ago is still answered. + expect(out.questionsAlreadyFiled).toBe(1); + expect(out.questionsFiled).toBe(0); + expect(handles.github.openIssueCalls).toHaveLength(0); + // And nothing reopens it: the run has no reopen, and does not comment. + expect(handles.github.addIssueLabelsCalls).toHaveLength(0); + expect(handles.github.commentOnIssueCalls).toHaveLength(0); + expect(handles.github.closeIssueAsDuplicateCalls).toHaveLength(0); + }).pipe(Effect.provide(layer)); + }); + + it.effect("asks for every state, un-windowed, strictly, under the questions label", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: baseConfig, + sandboxProgram: activeSandbox, + modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { now: firedAt }, + }); + + return Effect.gen(function* () { + yield* orgSpecAudit.run(input); + const [call] = handles.github.issuesCalls; + // Each of these is one edit away from silently breaking dedup: `open` would + // re-file every answered question, a window would resurrect the old ones, + // and a non-strict read answers "not filed" for whatever the page ceiling + // cut off. + expect(call).toMatchObject({ repo: "owner/control", state: "all", strict: true }); + expect(call!.labels).toEqual([LABEL]); + expect(call!.updatedWithinDays).toBeUndefined(); + // And a ceiling far above the library default, because `strict` turns an + // outgrown ceiling into a permanent failure rather than a short list. + expect(call!.maxPages).toBeGreaterThanOrEqual(20); + }).pipe(Effect.provide(layer)); + }); + + // The inversion. With one PR a day an unreadable ledger cost one duplicate PR, + // so failing open was right. An unreadable ISSUE SET costs a duplicate of + // every question at once. + it.effect("files nothing at all when it cannot read what it already asked", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: baseConfig, + sandboxProgram: activeSandbox, + modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { now: firedAt }, + }); + + return Effect.gen(function* () { + const fake = yield* Github; + const exit = yield* Effect.exit( + orgSpecAudit.run(input).pipe( + Effect.provideService(Github, { + ...fake, + issues: () => Effect.fail(new GitHubApiError({ status: 500, reason: "transient" })), + }), + ), + ); + + expect(exit._tag).toBe("Failure"); + expect(handles.github.openIssueCalls).toHaveLength(0); + // It loses a day and no facts: the questions are re-derived tomorrow. + expect(handles.notice.published).toHaveLength(0); + }).pipe(Effect.provide(layer)); + }); + + // The read is deliberately ahead of the sweep: it is one cheap call whose + // failure ends the tick, and reading it afterwards would mean paying for an + // estate of model calls and discarding every one. + it.effect("reads the ledger before spending a single model call", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: baseConfig, + sandboxProgram: activeSandbox, + modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { now: firedAt }, + }); + + return Effect.gen(function* () { + const fake = yield* Github; + yield* Effect.exit( + orgSpecAudit.run(input).pipe( + Effect.provideService(Github, { + ...fake, + issues: () => Effect.fail(new GitHubApiError({ status: 500, reason: "transient" })), + }), + ), + ); + expect(handles.modelGateway.requests).toHaveLength(0); + expect(handles.sandbox.clones).toHaveLength(0); }).pipe(Effect.provide(layer)); }); // The run holds no default control repo on purpose: a default is a repo - // somebody else's deployment files pull requests against. Unset must stop the - // run, and stop it BEFORE the sweep — an hour of model calls whose output has - // nowhere to go is the expensive way to learn a key is missing. + // somebody else's deployment files issues on. Unset must stop the run, and + // stop it BEFORE the sweep — an hour of model calls whose output has nowhere + // to go is the expensive way to learn a key is missing. it.effect("fails when no control repo is configured, before sweeping anything", () => { const { layer, handles } = makeCFRuntimeTest({ config: withoutKey(baseConfig, "org-spec-audit.control-repo"), @@ -115,30 +287,43 @@ describe("org-spec-audit", () => { const exit = yield* Effect.exit(orgSpecAudit.run(input)); expect(exit._tag).toBe("Failure"); expect(JSON.stringify(exit)).toContain("org-spec-audit.control-repo"); - // Nothing was cloned, nothing was executed, nothing was proposed. - expect(handles.github.openDraftPullRequestCalls).toHaveLength(0); + // Nothing was cloned, nothing was executed, nothing was filed. + expect(handles.github.openIssueCalls).toHaveLength(0); expect(handles.sandbox.clones).toHaveLength(0); expect(handles.sandbox.execs).toHaveLength(0); }).pipe(Effect.provide(layer)); }); - it.effect("writes where `questions-dir` says, not where the run was born", () => { + it.effect("labels issues the way config says, not the way the run was born", () => { const { layer, handles } = makeCFRuntimeTest({ - config: { ...baseConfig, "org-spec-audit.questions-dir": "infra/loop/open-questions/" }, + config: { + ...baseConfig, + "org-spec-audit.questions-label": "loop:question", + "org-spec-audit.lane-label-prefix": "answer-by-", + }, sandboxProgram: activeSandbox, modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { now: firedAt }, }); return Effect.gen(function* () { yield* orgSpecAudit.run(input); - const calls = handles.github.openDraftPullRequestCalls; - expect(calls[0]!.files[0]!.path).toBe("infra/loop/open-questions/2026-08-08.md"); + expect(handles.github.openIssueCalls[0]!.labels).toEqual([ + "loop:question", + "answer-by-decide", + ]); + // The read filters on the same label it writes — the two being one value + // is what makes dedup work at all. + expect(handles.github.issuesCalls[0]!.labels).toEqual(["loop:question"]); }).pipe(Effect.provide(layer)); }); - it.effect("refuses a questions-dir that escapes the repo root", () => { + // A comma makes the filter and the write two different things: GitHub's list + // query joins labels on commas, so the read would filter on two labels while + // the write applied one — and every question would re-file forever, silently. + it.effect("refuses a questions label carrying a comma, before reading anything", () => { const { layer, handles } = makeCFRuntimeTest({ - config: { ...baseConfig, "org-spec-audit.questions-dir": "../../etc" }, + config: { ...baseConfig, "org-spec-audit.questions-label": "loop:question,bug" }, sandboxProgram: activeSandbox, modelGateway: { responses: [reported([question()])] }, }); @@ -146,36 +331,36 @@ describe("org-spec-audit", () => { return Effect.gen(function* () { const exit = yield* Effect.exit(orgSpecAudit.run(input)); expect(exit._tag).toBe("Failure"); - expect(handles.github.openDraftPullRequestCalls).toHaveLength(0); + expect(JSON.stringify(exit)).toContain("org-spec-audit.questions-label"); + expect(handles.github.issuesCalls).toHaveLength(0); + expect(handles.github.openIssueCalls).toHaveLength(0); }).pipe(Effect.provide(layer)); }); - it.effect("announces the same text it committed, under a use case", () => { + it.effect("announces the delta, links each issue, and names no channel", () => { const { layer, handles } = makeCFRuntimeTest({ config: baseConfig, sandboxProgram: activeSandbox, modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { now: firedAt }, }); return Effect.gen(function* () { yield* orgSpecAudit.run(input); const [notice] = handles.notice.published; - const file = handles.github.openDraftPullRequestCalls[0]!.files[0]!; if (notice === undefined) throw new Error("no notice was published"); - // One rendering, two destinations. A second wording would be a second - // thing to keep true, and the first question a reader asks about a - // digest is which copy is the real one. - expect(notice.text).toBe(file.content); + expect(notice.text).toContain("1 new question(s)"); + expect(notice.text).toContain("1 open"); // A KIND of message, never a room. The receiver maps this to a channel // from its own config; nothing here can name one. expect(notice.useCase).toBe("org-spec-audit"); expect(JSON.stringify(notice)).not.toMatch(/channel/i); - // The PR link rides as a typed entry, because markup inside `text` would - // be escaped by the receiver along with everything else. + // Issue links ride as typed entries, because markup inside `text` would be + // escaped by the receiver along with everything else. expect(notice.links).toEqual([ - { url: "https://github.com/owner/control/pull/1", label: "the questions PR" }, + { url: "https://github.com/owner/control/issues/1", label: "#1" }, ]); }).pipe(Effect.provide(layer)); }); @@ -188,6 +373,7 @@ describe("org-spec-audit", () => { config: baseConfig, sandboxProgram: activeSandbox, modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { now: firedAt }, }); return Effect.gen(function* () { @@ -199,21 +385,21 @@ describe("org-spec-audit", () => { }).pipe(Effect.provide(layer)); }); - it.effect("keeps the file and the verdict when the notice does not land", () => { - // The digest is already in git, which is the copy that has to survive. An - // announcement that failed must not retroactively make the sweep a failure. + it.effect("keeps the issues and the verdict when the notice does not land", () => { + // The questions are already on GitHub, which is the copy that has to + // survive. An announcement that failed must not make the sweep a failure. const { layer, handles } = makeCFRuntimeTest({ config: baseConfig, sandboxProgram: activeSandbox, modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { now: firedAt }, notice: { outcome: "failed" }, }); return Effect.gen(function* () { const out = yield* orgSpecAudit.run(input); - expect(out.prOpened).toBe(true); - expect(out.questionsAfterMerge).toBe(1); - expect(handles.github.openDraftPullRequestCalls).toHaveLength(1); + expect(out.questionsFiled).toBe(1); + expect(handles.github.openIssueCalls).toHaveLength(1); }).pipe(Effect.provide(layer)); }); @@ -222,13 +408,14 @@ describe("org-spec-audit", () => { config: baseConfig, sandboxProgram: { ...activeSandbox, "git log --oneline": { exitCode: 0, stdout: "" } }, modelGateway: { responses: [] }, + github: { now: firedAt }, }); return Effect.gen(function* () { const out = yield* orgSpecAudit.run(input); expect(out.reposSwept).toBe(0); expect(out.reposSkipped).toBe(2); - expect(handles.github.openDraftPullRequestCalls).toHaveLength(0); + expect(handles.github.openIssueCalls).toHaveLength(0); }).pipe(Effect.provide(layer)); }); @@ -237,14 +424,15 @@ describe("org-spec-audit", () => { config: baseConfig, sandboxProgram: activeSandbox, modelGateway: { responses: [reported([]), reported([])] }, + github: { now: firedAt }, }); return Effect.gen(function* () { const out = yield* orgSpecAudit.run(input); expect(out.reposSwept).toBe(2); expect(out.questionsAfterMerge).toBe(0); - expect(out.prOpened).toBe(false); - expect(handles.github.openDraftPullRequestCalls).toHaveLength(0); + expect(out.questionsFiled).toBe(0); + expect(handles.github.openIssueCalls).toHaveLength(0); // Empty means silent in the channel too. A digest that fires whether or // not there is news is one people stop reading, and by then it has // nothing left to spend. @@ -269,7 +457,7 @@ describe("org-spec-audit", () => { // The whole sweep stops: auditing 1 of 2 repos and reporting success is // how a repo drops out of the estate without anyone being told. expect(handles.sandbox.clones).toHaveLength(0); - expect(handles.github.openDraftPullRequestCalls).toHaveLength(0); + expect(handles.github.openIssueCalls).toHaveLength(0); }).pipe(Effect.provide(layer)); }); @@ -298,19 +486,20 @@ describe("org-spec-audit", () => { "specs/*.md": { exitCode: 1, stdout: "", stderr: "not a git repository" }, }, modelGateway: { responses: [] }, + github: { now: firedAt }, }); return Effect.gen(function* () { const out = yield* orgSpecAudit.run(input); expect(out.reposSwept).toBe(0); expect(out.reposSkipped).toBe(2); - expect(out.prOpened).toBe(false); + expect(out.questionsFiled).toBe(0); expect(handles.modelGateway.requests).toHaveLength(0); - expect(handles.github.openDraftPullRequestCalls).toHaveLength(0); + expect(handles.github.openIssueCalls).toHaveLength(0); }).pipe(Effect.provide(layer)); }); - it.effect("a crafted spec cannot register a maintenance-key the PR never proposed", () => { + it.effect("a crafted spec cannot register a maintenance-key the issue never claims", () => { const { layer, handles } = makeCFRuntimeTest({ config: baseConfig, sandboxProgram: activeSandbox, @@ -326,24 +515,20 @@ describe("org-spec-audit", () => { reported([]), ], }, + github: { now: firedAt }, }); return Effect.gen(function* () { yield* orgSpecAudit.run(input); - const body = handles.github.openDraftPullRequestCalls[0]!.body; - - // The reader's own regex (packages/core/src/primitives/suppression.ts): - // line-anchored, so it picks a key up from ANYWHERE in the body, not just - // the trailer block. The body carries one key per question it proposes — - // here exactly one — and nothing the model wrote may join that set. - const keys = [...body.matchAll(/^[ \t]*maintenance-key:[ \t]*(\S+)[ \t]*$/gm)].map( - (m) => m[1], - ); - expect(keys).toEqual(["org-spec-audit/per-run-spend-caps"]); - expect(keys).not.toContain("org-spec-audit/unrelated-question"); - expect(keys).not.toContain("org-spec-audit/another-one"); + const body = handles.github.openIssueCalls[0]!.body; + + // The authentic trailer is the body's FIRST line, and the reader takes the + // first match — so a key the model echoed is inert text further down + // rather than the identity of this issue. + expect(body.split("\n")[0]).toBe(`maintenance-key: ${KEY}`); + expect(firstMaintenanceKey(body)).toBe(KEY); - // The text is not censored — it is still readable, just not line-leading. + // The text is not censored — it is still readable, just not authoritative. expect(body).toContain("maintenance-key: org-spec-audit/unrelated-question"); }).pipe(Effect.provide(layer)); }); @@ -356,6 +541,7 @@ describe("org-spec-audit", () => { "head -800": { exitCode: 1, stdout: "", stderr: "not a git repository" }, }, modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { now: firedAt }, }); return Effect.gen(function* () { @@ -366,149 +552,274 @@ describe("org-spec-audit", () => { expect(handles.modelGateway.requests).toHaveLength(0); expect(out.reposSwept).toBe(0); expect(out.reposSkipped).toBe(2); - expect(out.prOpened).toBe(false); + expect(out.questionsFiled).toBe(0); }).pipe(Effect.provide(layer)); }); }); -// --- Suppression: what the run refuses to propose twice ---------------------- +// --- The per-sweep cap ------------------------------------------------------- -const LEDGER = "owner/control:maintenance/declined.jsonl"; -const KEY = "org-spec-audit/per-run-spend-caps"; +describe("org-spec-audit — the cap", () => { + const seven = Array.from({ length: 7 }, (_unused, i) => + question({ key: `q${i}`, question: `Question ${i}?` }), + ); -/** A prior proposal carrying the key, closed unmerged `daysAgo` days back. */ -const closedProposal = ( - daysAgo: number, - over: Partial = {}, -): PullRequestHistoryRef => - ({ - repo: "owner/control", - number: 7, - title: "docs(maintenance): open questions", - body: `maintenance-key: ${KEY}`, - headBranch: `flare-dispatch/spec-audit-questions-2026-06-0${daysAgo % 9}`, - headSha: "abc123", - state: "closed", - draft: true, - labels: [], - author: "flare-dispatch[bot]", - requestedReviewers: [], - url: "https://github.com/owner/control/pull/7", - createdAt: firedAt - (daysAgo + 5) * DAY, - // Touched today on purpose: a cooldown dated from `updated_at` would never - // expire, which is the whole reason `closed_at` is the field that counts. - updatedAt: firedAt, - closedAt: firedAt - daysAgo * DAY, - ...over, - }) satisfies PullRequestHistoryRef; - -/** The runtime the suppression tests share — one question, one control repo. */ -const suppressionRuntime = ( - github: NonNullable[0]>["github"], -) => - makeCFRuntimeTest({ - config: baseConfig, - sandboxProgram: activeSandbox, - modelGateway: { responses: [reported([question()]), reported([question()])] }, - github: { now: firedAt, ...github }, + it.effect("files up to the cap and says what it held, never truncating silently", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: { ...baseConfig, "org-spec-audit.max-new-questions": "2" }, + sandboxProgram: activeSandbox, + modelGateway: { responses: [reported(seven), reported([])] }, + github: { now: firedAt }, + }); + + return Effect.gen(function* () { + const out = yield* orgSpecAudit.run(input); + expect(out.questionsAfterMerge).toBe(7); + expect(out.questionsFiled).toBe(2); + expect(out.questionsHeldByCap).toBe(5); + expect(handles.github.openIssueCalls).toHaveLength(2); + // A shorter list that does not say it is shorter reads as fewer problems. + expect(handles.notice.published[0]!.text).toContain("Held by the per-sweep cap: 5"); + }).pipe(Effect.provide(layer)); }); -describe("org-spec-audit — suppression", () => { - it.effect("never re-proposes a question the ledger declined", () => { - const { layer, handles } = suppressionRuntime({ - files: { - [LEDGER]: JSON.stringify({ - key: KEY, - reason: "answered in ADR-0011; the spec is right", - by: "@ada", - at: "2026-08-01", - }), + it.effect("files the held-back questions on the next sweep, since none are on file", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: { ...baseConfig, "org-spec-audit.max-new-questions": "2" }, + sandboxProgram: activeSandbox, + // Two ticks: sweeps for run 1, sweeps for run 2, then run 2's reconcile. + // The fake repeats its last entry, so `matched([])` covers that and after. + modelGateway: { + responses: [reported(seven), reported([]), reported(seven), reported([]), matched([])], }, + github: { now: firedAt }, }); return Effect.gen(function* () { + yield* orgSpecAudit.run(input); + expect(handles.github.openIssueCalls).toHaveLength(2); + // Second tick: the two filed are found on file, the other five are not. const out = yield* orgSpecAudit.run(input); - expect(out.questionsAfterMerge).toBe(1); - expect(out.questionsSuppressed).toBe(1); - expect(out.prOpened).toBe(false); - // Nothing left to ask ⇒ no PR at all, and the count still reports why. - expect(handles.github.openDraftPullRequestCalls).toHaveLength(0); + expect(out.questionsAlreadyFiled).toBe(2); + expect(out.questionsFiled).toBe(2); + expect(handles.github.openIssueCalls).toHaveLength(4); }).pipe(Effect.provide(layer)); }); - // The ledger's location is the operator's, like the questions dir. The - // default this repo ships is a placeholder, and an operator who moves the - // file must have the run follow it — including in the sentence the PR body - // prints telling a reviewer where to record a permanent decline. - it.effect("reads the ledger where `declined-path` says, and says so in the body", () => { + it.effect("defaults the cap when the value is nonsense", () => { const { layer, handles } = makeCFRuntimeTest({ - config: { ...baseConfig, "org-spec-audit.declined-path": "infra/loop/declined.jsonl" }, + config: { ...baseConfig, "org-spec-audit.max-new-questions": "-3" }, sandboxProgram: activeSandbox, - modelGateway: { responses: [reported([question()]), reported([question()])] }, - github: { - now: firedAt, - files: { "owner/control:infra/loop/declined.jsonl": "" }, + modelGateway: { responses: [reported(seven), reported([])] }, + github: { now: firedAt }, + }); + + return Effect.gen(function* () { + const out = yield* orgSpecAudit.run(input); + expect(out.questionsFiled).toBe(5); + expect(handles.github.openIssueCalls).toHaveLength(5); + }).pipe(Effect.provide(layer)); + }); +}); + +// --- Key reconciliation: the same question, worded differently --------------- + +describe("org-spec-audit — key reconciliation", () => { + /** The same question as `filedIssue()`, minted under a different verb. */ + const rephrased = question({ + key: "spend-caps-per-run", + question: "Are per-run spend caps still committed to?", + }); + + it.effect("does not re-file a question the model matches onto one on file", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: baseConfig, + sandboxProgram: activeSandbox, + modelGateway: { + responses: [ + reported([rephrased]), + reported([]), + matched([{ minted: "spend-caps-per-run", existing: "per-run-spend-caps" }]), + ], + }, + github: { now: firedAt, issues: [filedIssue()] }, + }); + + return Effect.gen(function* () { + const out = yield* orgSpecAudit.run(input); + expect(out.questionsAlreadyFiled).toBe(1); + expect(out.questionsFiled).toBe(0); + expect(handles.github.openIssueCalls).toHaveLength(0); + expect( + handles.io.logs.some((l) => l.msg.includes("reconciled onto") && l.msg.includes(KEY)), + ).toBe(true); + }).pipe(Effect.provide(layer)); + }); + + // The containment: an answer naming a key that was never read decides nothing. + // A missed match costs a human one click; an accepted hallucination is a + // question that is never asked again. + it.effect("files anyway when the model matches onto a key nobody has on file", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: baseConfig, + sandboxProgram: activeSandbox, + modelGateway: { + responses: [ + reported([rephrased]), + reported([]), + matched([{ minted: "spend-caps-per-run", existing: "a-question-nobody-asked" }]), + ], }, + github: { now: firedAt, issues: [filedIssue()] }, + }); + + return Effect.gen(function* () { + const out = yield* orgSpecAudit.run(input); + expect(out.questionsFiled).toBe(1); + expect(handles.github.openIssueCalls).toHaveLength(1); + }).pipe(Effect.provide(layer)); + }); + + it.effect("files anyway when the reconcile call fails", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: baseConfig, + sandboxProgram: activeSandbox, + // The third call returns a sweep payload, which cannot parse as a + // reconciliation — the same shape a model failure takes here. + modelGateway: { responses: [reported([rephrased]), reported([]), reported([])] }, + github: { now: firedAt, issues: [filedIssue()] }, + }); + + return Effect.gen(function* () { + const out = yield* orgSpecAudit.run(input); + expect(out.questionsFiled).toBe(1); + expect( + handles.io.logs.some((l) => l.level === "warn" && l.msg.includes("all 1 as new")), + ).toBe(true); + }).pipe(Effect.provide(layer)); + }); + + it.effect("spends nothing on reconciliation when nothing is on file", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: baseConfig, + sandboxProgram: activeSandbox, + modelGateway: { responses: [reported([question()]), reported([])] }, + github: { now: firedAt }, }); return Effect.gen(function* () { yield* orgSpecAudit.run(input); - const calls = handles.github.openDraftPullRequestCalls; - expect(calls).toHaveLength(1); - expect(calls[0]!.body).toContain("`infra/loop/declined.jsonl`"); - expect(calls[0]!.body).not.toContain("maintenance/declined.jsonl"); + // Two sweeps, no third call: on a fresh control repo every question is new + // by construction and there is nothing to match against. + expect(handles.modelGateway.requests).toHaveLength(2); + expect( + handles.modelGateway.requests.some((r) => JSON.stringify(r).includes("report_key_matches")), + ).toBe(false); + }).pipe(Effect.provide(layer)); + }); + + it.effect("skips reconciliation for a question whose key already matches exactly", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: baseConfig, + sandboxProgram: activeSandbox, + modelGateway: { responses: [reported([question()]), reported([])] }, + github: { now: firedAt, issues: [filedIssue()] }, + }); + + return Effect.gen(function* () { + const out = yield* orgSpecAudit.run(input); + expect(out.questionsAlreadyFiled).toBe(1); + // An exact key match IS the same question. Paying a model to confirm it + // would be spend with no decision attached. + expect(handles.modelGateway.requests).toHaveLength(2); }).pipe(Effect.provide(layer)); }); +}); + +// --- Suppression: the pre-emptive half, and the half that retired ------------ - it.effect("honours a cooldown dated from when the proposal was closed", () => { - const { layer, handles } = suppressionRuntime({ - pullRequestHistory: [closedProposal(5)], +const LEDGER = "owner/control:maintenance/declined.jsonl"; + +describe("org-spec-audit — suppression", () => { + it.effect("never files a question the ledger declined", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: baseConfig, + sandboxProgram: activeSandbox, + modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { + now: firedAt, + files: { + [LEDGER]: JSON.stringify({ + key: KEY, + reason: "answered in ADR-0011; the spec is right", + by: "@ada", + at: "2026-08-01", + }), + }, + }, }); return Effect.gen(function* () { const out = yield* orgSpecAudit.run(input); + expect(out.questionsAfterMerge).toBe(1); expect(out.questionsSuppressed).toBe(1); - expect(out.prOpened).toBe(false); - expect(handles.github.openDraftPullRequestCalls).toHaveLength(0); - // And nothing is announced either. Suppression runs BEFORE the notice, so - // a question a human declined is not re-broadcast into a channel — which - // is the louder half of re-proposing it, and the one nobody can close. + expect(out.questionsFiled).toBe(0); + expect(handles.github.openIssueCalls).toHaveLength(0); + // Nothing announced either: re-broadcasting a declined question into a + // channel is the louder half of re-proposing it, and the one nobody can + // close. expect(handles.notice.published).toHaveLength(0); }).pipe(Effect.provide(layer)); }); - it.effect("asks again once the cooldown has expired", () => { - const { layer, handles } = suppressionRuntime({ - pullRequestHistory: [closedProposal(45, { updatedAt: firedAt - 45 * DAY })], + // No PRs ⇒ no PR history ⇒ no cooldown. A prefix matching nothing would answer + // "no prior proposals" every tick, for a reason no reader could tell apart + // from the feature being off. + it.effect("reads no PR history at all", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: baseConfig, + sandboxProgram: activeSandbox, + modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { now: firedAt }, }); return Effect.gen(function* () { - const out = yield* orgSpecAudit.run(input); - expect(out.questionsSuppressed).toBe(0); - expect(out.prOpened).toBe(true); - expect(handles.github.openDraftPullRequestCalls).toHaveLength(1); + yield* orgSpecAudit.run(input); + expect(handles.github.pullRequestHistoryCalls).toHaveLength(0); + expect(handles.github.openIssueCalls).toHaveLength(1); }).pipe(Effect.provide(layer)); }); - it.effect("does not suppress on a proposal that was merged", () => { - const { layer } = suppressionRuntime({ - pullRequestHistory: [closedProposal(5, { mergedAt: firedAt - 5 * DAY })], + it.effect("reads the ledger where `declined-path` says, and says so in the issue", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: { ...baseConfig, "org-spec-audit.declined-path": "infra/loop/declined.jsonl" }, + sandboxProgram: activeSandbox, + modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { now: firedAt, files: { "owner/control:infra/loop/declined.jsonl": "" } }, }); return Effect.gen(function* () { - const out = yield* orgSpecAudit.run(input); - expect(out.questionsSuppressed).toBe(0); - expect(out.prOpened).toBe(true); + yield* orgSpecAudit.run(input); + const calls = handles.github.openIssueCalls; + expect(calls).toHaveLength(1); + expect(calls[0]!.body).toContain("`infra/loop/declined.jsonl`"); + expect(calls[0]!.body).not.toContain("maintenance/declined.jsonl"); }).pipe(Effect.provide(layer)); }); it.effect("skips a malformed ledger line and honours the rest", () => { - const { layer, handles } = suppressionRuntime({ - files: { - [LEDGER]: [ - "}}} not json at all", - JSON.stringify({ key: KEY, reason: "settled", by: "@ada", at: "2026-08-01" }), - ].join("\n"), + const { layer, handles } = makeCFRuntimeTest({ + config: baseConfig, + sandboxProgram: activeSandbox, + modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { + now: firedAt, + files: { + [LEDGER]: [ + "}}} not json at all", + JSON.stringify({ key: KEY, reason: "settled", by: "@ada", at: "2026-08-01" }), + ].join("\n"), + }, }, }); @@ -521,12 +832,17 @@ describe("org-spec-audit — suppression", () => { }).pipe(Effect.provide(layer)); }); - it.effect("proposes anyway — and says so — when the ledger cannot be read", () => { - const { layer, handles } = suppressionRuntime({}); + // The ledger read still fails OPEN, and the asymmetry with the issue read is + // the point: one issue a human closes, versus a duplicate of everything. + it.effect("files anyway — and says so — when the ledger cannot be read", () => { + const { layer, handles } = makeCFRuntimeTest({ + config: baseConfig, + sandboxProgram: activeSandbox, + modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { now: firedAt }, + }); return Effect.gen(function* () { - // Wrap the fake so only the ledger read fails; every other `github` call - // (notably the draft-PR write this test asserts on) still records. const fake = yield* Github; const out = yield* orgSpecAudit.run(input).pipe( Effect.provideService(Github, { @@ -536,20 +852,18 @@ describe("org-spec-audit — suppression", () => { ); expect(out.questionsSuppressed).toBe(0); - expect(out.prOpened).toBe(true); - const calls = handles.github.openDraftPullRequestCalls; - expect(calls).toHaveLength(1); - expect(calls[0]!.body).toContain("Suppression degraded"); + expect(out.questionsFiled).toBe(1); + expect(handles.notice.published[0]!.text).toContain("Suppression degraded"); expect(handles.io.logs.some((l) => l.level === "warn" && l.msg.includes("unreadable"))).toBe( true, ); }).pipe(Effect.provide(layer)); }); - it.effect("reports the suppressed count and reason in the PR body and the file", () => { - // Two questions: one declined, one still open — so a PR is opened AND has - // something to explain. A shorter list with no explanation reads as "fewer - // problems", which is the opposite of true. + it.effect("reports the suppressed count and reason in the notice", () => { + // Two questions: one declined, one still open — so something is filed AND + // there is something to explain. A shorter list with no explanation reads as + // "fewer problems", which is the opposite of true. const { layer, handles } = makeCFRuntimeTest({ config: baseConfig, sandboxProgram: activeSandbox, @@ -579,197 +893,265 @@ describe("org-spec-audit — suppression", () => { const out = yield* orgSpecAudit.run(input); expect(out.questionsAfterMerge).toBe(2); expect(out.questionsSuppressed).toBe(1); - expect(out.prOpened).toBe(true); - - const call = handles.github.openDraftPullRequestCalls[0]!; - expect(call.body).toContain("**Suppressed: 1**"); - expect(call.body).toContain("answered in ADR-0011"); - expect(call.body).toContain("suppressed: 1"); - // The message file IS the digest FractalBOT posts — it must say it too. - expect(call.files[0]!.content).toContain("**Suppressed: 1**"); - expect(call.files[0]!.content).toContain("1 suppressed"); + expect(out.questionsFiled).toBe(1); + + const text = handles.notice.published[0]!.text; + expect(text).toContain("**Suppressed: 1**"); + expect(text).toContain("answered in ADR-0011"); + expect(text).toContain("1 declined"); // The surviving question is still asked; the declined one is gone. - expect(call.files[0]!.content).toContain("Who owns egress?"); - expect(call.files[0]!.content).not.toContain("Does the dispatcher still commit"); + expect(text).toContain("Who owns egress?"); + expect(text).not.toContain("Does the dispatcher still commit"); }).pipe(Effect.provide(layer)); }); - it.effect("carries one maintenance-key per question, not one per PR", () => { - // A dated per-PR key would be unique every day and suppress nothing, ever. - const { layer, handles } = suppressionRuntime({}); + it.effect("carries one maintenance-key per issue, keyed on the question", () => { + // A dated key would be unique every day and match nothing, ever. + const { layer, handles } = makeCFRuntimeTest({ + config: baseConfig, + sandboxProgram: activeSandbox, + modelGateway: { responses: [reported([question()]), reported([question()])] }, + github: { now: firedAt }, + }); return Effect.gen(function* () { yield* orgSpecAudit.run(input); - const body = handles.github.openDraftPullRequestCalls[0]!.body; + const body = handles.github.openIssueCalls[0]!.body; expect(body).toContain(`maintenance-key: ${KEY}`); expect(body).not.toContain("maintenance-key: org-spec-audit/2026-08-08"); }).pipe(Effect.provide(layer)); }); - - it.effect("asks about exactly the branch prefix its own proposals use", () => { - const { layer, handles } = suppressionRuntime({}); - - return Effect.gen(function* () { - yield* orgSpecAudit.run(input); - const [call] = handles.github.pullRequestHistoryCalls; - expect(call).toMatchObject({ - repo: "owner/control", - headBranchPrefix: "flare-dispatch/spec-audit-questions-", - state: "all", - }); - expect(handles.github.openDraftPullRequestCalls[0]!.headBranch).toMatch( - new RegExp(`^${call!.headBranchPrefix}`), - ); - }).pipe(Effect.provide(layer)); - }); }); describe("mergeAcrossRepos", () => { - const raised = (over: Record) => - ({ ...question(), ...over }) as Parameters[0][number]; + const raised = (over: Record = {}) => + ({ + repo: "o/a", + group: "decide" as const, + question: "Does X still commit to Y?", + evidence: "spec says A, tree says B", + specPath: "specs/x.md", + assumption: "assume nothing changes", + key: "x-commits-to-y", + ...over, + }) as Parameters[0][number]; - it("merges on a normalized key regardless of the model's punctuation", () => { - const out = mergeAcrossRepos([ - raised({ repo: "o/a", key: "Per-Run Spend Caps" }), - raised({ repo: "o/b", key: "per_run_spend_caps" }), - ]); + it("merges one key raised by two repos into one question with two sources", () => { + const out = mergeAcrossRepos([raised(), raised({ repo: "o/b" })]); expect(out).toHaveLength(1); expect(out[0]!.sources.map((s) => s.repo)).toEqual(["o/a", "o/b"]); }); - it("counts one repo raising the same key twice as one question", () => { - const out = mergeAcrossRepos([raised({ repo: "o/a" }), raised({ repo: "o/a" })]); + it("keeps one repo raising the same key twice as one question", () => { + const out = mergeAcrossRepos([raised(), raised()]); expect(out).toHaveLength(1); expect(out[0]!.sources).toHaveLength(1); }); - it("ranks the most-shared question first", () => { + it("ranks by DISTINCT repos, not by source count", () => { + // One repo raising a question from two specs is still one repo asking. const out = mergeAcrossRepos([ - raised({ repo: "o/a", key: "lonely" }), - raised({ repo: "o/b", key: "shared" }), - raised({ repo: "o/c", key: "shared" }), + raised({ key: "twice-in-one-repo", specPath: "specs/a.md" }), + raised({ key: "twice-in-one-repo", specPath: "specs/b.md" }), + raised({ key: "shared", repo: "o/a" }), + raised({ key: "shared", repo: "o/b" }), ]); expect(out[0]!.key).toBe("shared"); }); - it("ranks by distinct repos, so one repo's two specs can't outrank two repos", () => { - const out = mergeAcrossRepos([ - raised({ repo: "o/a", key: "one-repo-twice", specPath: "specs/x.md" }), - raised({ repo: "o/a", key: "one-repo-twice", specPath: "specs/y.md" }), - raised({ repo: "o/b", key: "two-repos" }), - raised({ repo: "o/c", key: "two-repos" }), - ]); - // Both merge to 2 sources; only `two-repos` is a question the estate shares, - // which is the entire reason this run sweeps rather than running per repo. - expect(out[0]!.key).toBe("two-repos"); + it("strips zero-width and bidi characters from model prose", () => { + const out = mergeAcrossRepos([raised({ repo: "o/a", question: "Does​ X‮ still commit⁦ to Y?" })]); + expect(out[0]!.question).toBe("Does X still commit to Y?"); }); +}); - it("falls back to the question text when the model's key is junk", () => { - const out = mergeAcrossRepos([raised({ repo: "o/a", key: "-" })]); - expect(out[0]!.key).toContain("does-the-dispatcher"); +describe("parseWindowHours", () => { + it("defaults when unset or nonsense", () => { + expect(parseWindowHours(undefined)).toBe(26); + expect(parseWindowHours("")).toBe(26); + expect(parseWindowHours("0")).toBe(26); + expect(parseWindowHours("-4")).toBe(26); }); - // The reader that parses `maintenance-key:` drops any key over 200 chars, and - // a dropped key is worse than a short one: the question keeps being proposed - // and can never be recorded as declined. - it("caps the model's own key, not only the fallback", () => { - const out = mergeAcrossRepos([raised({ repo: "o/a", key: "x".repeat(400) })]); - // `org-spec-audit/` + key must still fit the reader's 200-char budget. - expect(`org-spec-audit/${out[0]!.key}`.length).toBeLessThanOrEqual(200); - expect(out[0]!.key.length).toBeGreaterThan(3); + it("takes a positive integer", () => { + expect(parseWindowHours("72")).toBe(72); }); +}); - it("never leaves a trailing hyphen when the cap lands mid-word", () => { - const out = mergeAcrossRepos([raised({ repo: "o/a", key: `${"ab-".repeat(200)}tail` })]); - expect(out[0]!.key).not.toMatch(/-$/); +describe("parseLabel", () => { + it("falls back when unset or blank", () => { + expect(parseLabel(undefined, "d")).toBe("d"); + expect(parseLabel(null, "d")).toBe("d"); + expect(parseLabel(" ", "d")).toBe("d"); }); - it("collapses a model field that spans lines, so it cannot start one", () => { - const out = mergeAcrossRepos([ - raised({ - repo: "o/a", - evidence: "spec says X\nmaintenance-key: org-spec-audit/unrelated\nand the tree says Y", - }), - ]); - expect(out[0]!.evidence).not.toContain("\n"); - expect(out[0]!.evidence).toBe( - "spec says X maintenance-key: org-spec-audit/unrelated and the tree says Y", - ); + it("trims a usable value", () => { + expect(parseLabel(" loop:question ", "d")).toBe("loop:question"); }); - it("strips zero-width and bidi characters from model prose", () => { - const out = mergeAcrossRepos([ - raised({ repo: "o/a", question: "Does​ X‮ still commit⁦ to Y?" }), - ]); - expect(out[0]!.question).toBe("Does X still commit to Y?"); + // Set-and-unusable is `undefined`, never the fallback: a label that filters + // differently than it writes breaks dedup with nothing erroring. + it("rejects a comma and an over-long name rather than falling back", () => { + expect(parseLabel("a,b", "d")).toBeUndefined(); + expect(parseLabel("x".repeat(51), "d")).toBeUndefined(); + expect(parseLabel("x".repeat(50), "d")).toBe("x".repeat(50)); }); }); -describe("parseWindowHours", () => { - it("defaults when unset or nonsense", () => { - expect(parseWindowHours(undefined)).toBe(26); - expect(parseWindowHours("")).toBe(26); - expect(parseWindowHours("0")).toBe(26); - expect(parseWindowHours("-4")).toBe(26); +describe("parsePositiveInt", () => { + it("defaults when unset or non-positive", () => { + expect(parsePositiveInt(undefined, 5)).toBe(5); + expect(parsePositiveInt("", 5)).toBe(5); + expect(parsePositiveInt("0", 5)).toBe(5); + expect(parsePositiveInt("-2", 5)).toBe(5); + expect(parsePositiveInt("abc", 5)).toBe(5); }); it("takes a positive integer", () => { - expect(parseWindowHours("72")).toBe(72); + expect(parsePositiveInt("12", 5)).toBe(12); + }); +}); + +describe("firstMaintenanceKey / indexFiledQuestions", () => { + it("takes the first key, so a later one cannot claim the issue", () => { + const body = [ + "maintenance-key: org-spec-audit/real", + "", + "evidence mentioning maintenance-key: org-spec-audit/spoofed", + ].join("\n"); + expect(firstMaintenanceKey(body)).toBe("org-spec-audit/real"); + }); + + it("has no key when there is no trailer line", () => { + expect(firstMaintenanceKey("just prose\nand more prose")).toBeUndefined(); + }); + + it("indexes only this run's namespace", () => { + const index = indexFiledQuestions([ + filedIssue({ number: 1, key: "org-spec-audit/mine" }), + filedIssue({ number: 2, key: "some-other-run/theirs" }), + // A human-opened issue carrying the label is not a question this run asked. + filedIssue({ number: 3, body: "no trailer here" }), + ]); + expect([...index.keys()]).toEqual(["org-spec-audit/mine"]); + }); + + it("keeps the first of a duplicated key, deterministically", () => { + const index = indexFiledQuestions([filedIssue({ number: 9 }), filedIssue({ number: 10 })]); + expect(index.get(KEY)?.number).toBe(9); + }); +}); + +describe("issueTitle", () => { + const q = (question: string) => ({ + key: "k", + group: "decide" as const, + question, + assumption: "a", + sources: [{ repo: "o/a", specPath: "specs/x.md" }], + evidence: "e", + }); + + it("is the question as asked", () => { + expect(issueTitle(q("Who owns egress?"))).toBe("Who owns egress?"); + }); + + it("cuts an over-long title where we can see it, not where GitHub does", () => { + const out = issueTitle(q("Q".repeat(400))); + expect(out).toHaveLength(240); + expect(out.endsWith("…")).toBe(true); + }); +}); + +describe("renderIssueBody", () => { + const q = { + key: "spend-caps", + group: "decide" as const, + question: "Who owns egress?", + assumption: "assume nothing changes", + sources: [{ repo: "o/a", specPath: "specs/x.md" }], + evidence: "spec says A, tree says B", + }; + + it("puts the trailer first, then the answer-if-nobody-does and the evidence", () => { + const out = renderIssueBody({ + question: q, + day: "2026-08-08", + declinedPath: "infra/declined.jsonl", + }); + expect(out.split("\n")[0]).toBe("maintenance-key: org-spec-audit/spend-caps"); + expect(out).toContain("**If nobody answers:** assume nothing changes"); + expect(out).toContain("spec says A, tree says B"); + expect(out).toContain("`o/a` (specs/x.md)"); + expect(out).toContain("`infra/declined.jsonl`"); + }); + + it("tells the reader that closing is the record and nothing reopens it", () => { + const out = renderIssueBody({ question: q, day: "2026-08-08", declinedPath: "d.jsonl" }); + expect(out).toContain("Closing is the record"); + expect(out).toContain("never reopen"); }); }); -describe("renderMessage", () => { - const merged = (n: number, group: "decide" | "confirm") => +describe("renderNotice", () => { + const opened = (n: number, group: "decide" | "confirm") => Array.from({ length: n }, (_unused, i) => ({ - key: `k${i}`, - group, - question: `Q${i}?`, - assumption: "assume nothing changes", - sources: [{ repo: "o/a", specPath: "specs/x.md" }], - evidence: "spec says X, tree says Y", + number: 100 + i, + url: `https://github.com/o/c/issues/${100 + i}`, + question: { + key: `k${i}`, + group, + question: `Q${i}?`, + assumption: "assume nothing changes", + sources: [{ repo: "o/a", specPath: "specs/x.md" }], + evidence: "spec says X, tree says Y", + }, })); - it("states what the per-group cap kept out rather than truncating silently", () => { - const out = renderMessage({ - day: "2026-08-08", - merged: merged(7, "decide"), + const base = { + day: "2026-08-08", + openOnFile: 3, + heldByCap: 0, + alreadyFiled: 0, + raised: 1, + suppression: noSuppression, + }; + + it("leads with the delta, and links each new question by number", () => { + const out = renderNotice({ + ...base, + opened: opened(2, "decide"), outcomes: [{ repo: "o/a", skipped: false, questions: [] }], - raised: 7, - suppression: noSuppression, }); - expect(out).toContain("2 more in this group, not shown"); - expect(out).toContain("Below the per-group cap: 2"); + expect(out).toContain("2 new question(s) · 3 open"); + expect(out).toContain("(#100)"); + expect(out).toContain("(#101)"); }); it("names the repos it swept and the ones it skipped", () => { - const out = renderMessage({ - day: "2026-08-08", - merged: merged(1, "confirm"), + const out = renderNotice({ + ...base, + opened: opened(1, "confirm"), outcomes: [ { repo: "o/a", skipped: false, questions: [] }, { repo: "o/dormant", skipped: true, questions: [] }, ], - raised: 1, - suppression: noSuppression, }); expect(out).toContain("Swept: `o/a`"); expect(out).toContain("`o/dormant`"); }); it("separates a repo that failed from one that was quiet", () => { - const out = renderMessage({ - day: "2026-08-08", - merged: merged(1, "confirm"), + const out = renderNotice({ + ...base, + opened: opened(1, "confirm"), outcomes: [ { repo: "o/a", skipped: false, questions: [] }, { repo: "o/dormant", skipped: true, questions: [] }, { repo: "o/broken", skipped: true, failure: "model call failed (429)", questions: [] }, ], - raised: 1, - suppression: noSuppression, }); // A failure counted as "unchanged" turns an outage into good news. - expect(out).toContain("1 unchanged or without specs"); expect(out).toContain("1 failed"); expect(out).toContain("Failed: `o/broken` (model call failed (429))"); expect(out).not.toContain("Failed: none"); @@ -777,15 +1159,24 @@ describe("renderMessage", () => { expect(out.indexOf("could not be swept")).toBeLessThan(out.indexOf("## Confirm")); }); - it("says so explicitly when nothing failed", () => { - const out = renderMessage({ - day: "2026-08-08", - merged: merged(1, "confirm"), + it("says so explicitly when nothing failed and nothing was held", () => { + const out = renderNotice({ + ...base, + opened: opened(1, "confirm"), outcomes: [{ repo: "o/a", skipped: false, questions: [] }], - raised: 1, - suppression: noSuppression, }); expect(out).toContain("Failed: none"); + expect(out).toContain("Nothing held by the cap."); expect(out).not.toContain("could not be swept"); }); + + it("names what the cap held rather than shortening the list in silence", () => { + const out = renderNotice({ + ...base, + opened: opened(2, "decide"), + heldByCap: 5, + outcomes: [{ repo: "o/a", skipped: false, questions: [] }], + }); + expect(out).toContain("Held by the per-sweep cap: 5"); + }); }); diff --git a/runs/org-spec-audit.ts b/runs/org-spec-audit.ts index 7bd9e3f..164399d 100644 --- a/runs/org-spec-audit.ts +++ b/runs/org-spec-audit.ts @@ -3,9 +3,8 @@ // A Schedule-mode run that sweeps every configured repo, reads each one's // `specs/` against its tree, and collects the divergences that CANNOT be // reconciled automatically — the ones where *which side is right* is a -// judgment nobody has made yet. It deduplicates them across the estate, groups -// them by the answer that unblocks them, and opens ONE draft PR against the -// control repo carrying the grouped list as a dated markdown file. +// judgment nobody has made yet. It deduplicates them across the estate and +// files each one as ONE GITHUB ISSUE in the control repo. // // --- Why this is a separate run from `spec-drift-pr` ------------------------- // @@ -19,52 +18,90 @@ // quarters, two specs in different repos that disagree. Per repo those die in // a PR body. Across an estate the same question is usually raised in three // repos, and answering it once closes all three. That merge is the entire -// reason this sweeps instead of running per repo, and it is why the output is -// ONE PR against the control repo rather than one per repo. +// reason this sweeps instead of running per repo, and it is why every question +// lands in ONE control repo rather than in the repo that raised it. // -// --- Delivery: the file is the record, the notice is the announcement -------- +// --- One question, one issue — and why it is not a daily file --------------- +// +// This run shipped filing a dated `.md` in a draft PR: every question it +// found that day, rendered fresh. Two sweeps two days apart then asked three of +// the same questions twice, two of them under a BYTE-IDENTICAL +// `maintenance-key` (`org#147`, `org#148`). +// +// An identical key that re-proposes rules out the obvious cause and leaves the +// real one. The ledger only ever remembered *no*: both suppression rules key +// off a TERMINATED proposal — a decline recorded in the ledger, or a PR closed +// unmerged — and the first PR was neither. It was open, unanswered, sitting in +// the queue, which the design read as no signal at all. +// +// So the unit is now the question, and the artifact is a GitHub issue: +// +// open — asked, unanswered → this run writes NOTHING. Silence is correct. +// closed — answered, or declined → never filed again, and never reopened. +// absent — new → filed. +// +// That collapses the whole apparatus into one field. `open` is a first-class +// answer to "have I raised this?", which no ledger of declines could express, +// and a decline stops needing a second file a human maintains by hand. +// +// Two rules follow, and both are load-bearing: +// +// * NEVER reopen and never re-announce. A run that reopens an issue argues +// with the person who closed it, and a daily "still open" comment is the +// daily file again in miniature. How long a decision has gone unmade is +// already legible from the issue's own age. +// * The dedup read fails CLOSED (below), inverting the suppression +// primitive's posture on purpose. +// +// --- Delivery: the issue is the record, the notice is the announcement ------- // // This run does not post to Slack, and must not be given a way to. Slack bot // tokens live with the Slack ingress and stay there (see // `apps/dispatcher/src/slack-notify.ts`) — a cron run holding a workspace-write // credential is how a token ends up somewhere nobody meant it to be. // -// So it does two things with one rendering. The PR carries the message as a -// dated markdown file: that is the durable artifact and the reviewed record, -// and answering in its thread is how the questions get closed. Then -// `notice.publish` hands the SAME text to the `notice` capability, which names +// The issues are the durable record; answering in a thread and closing is how a +// question ends. `notice.publish` then announces the DELTA — filed today, how +// many stand open, what the cap held — to the `notice` capability, which names // a use case and nothing else; the Slack ingress resolves that to a room and -// posts it with the token it already holds. One direction of trust, no new -// credential here, and no second wording to drift from the first. +// posts it with the token it already holds. +// +// The delta matters as much as the medium. The old message was the day's whole +// file, so its length tracked the sweep rather than the news and a reader who +// had already seen eleven questions was shown eleven questions again. Now most +// days say nothing, and "nothing" means nothing CHANGED rather than nothing +// found — with the open-question count one GitHub query away for anyone who +// wants to tell those apart. // -// Both halves are best-effort in the one direction that matters: a notice that -// did not land is a logged line, never a verdict. The questions are already in -// git, which is the copy that has to survive. +// --- Suppression: two reads, and only one may fail open ---------------------- // -// --- Suppression: the loop's memory ------------------------------------------ +// The issue set is the memory. Before filing, the run reads every issue in the +// control repo carrying the questions label, in ANY state, and matches on the +// `maintenance-key: org-spec-audit/` line in each body. // -// Before it proposes anything, the run asks the `suppression` primitive which -// of today's questions it has already been told no to. A key in the control -// repo's declines ledger is never proposed again; a -// question whose proposal a human closed unmerged waits out a 30-day cooldown -// dated from `closed_at`. Every proposed question carries its own -// `maintenance-key: org-spec-audit/` line in the PR body — that -// line is what both halves match on. +// That read **fails closed**, which reverses what this run used to do. When one +// PR a day carried every question, an unreadable ledger cost one duplicate PR, +// so failing open was right and failing closed would have silenced the loop. +// Now an unreadable issue set costs a duplicate of EVERY question at once — so +// a sweep that cannot enumerate what it has already asked files nothing. It +// loses a day and no facts, because the questions are re-derived tomorrow. +// Same reason the read is `state: "all"`, un-windowed, and `strict` (a list the +// page ceiling cut short answers "not filed" for everything it never reached). // -// The suppressed count and the reason for each appear in the PR body AND in the -// message file, because a silently shorter list reads as "fewer problems", -// which is the opposite of what it means. Both reads fail open: if the ledger -// or the PR history cannot be read, the run proposes anyway and prints the -// warning, since a duplicate PR is a nuisance and a silently disabled loop is -// the failure the mechanism exists to prevent. +// The declines ledger (`declined.jsonl`) is the PRE-EMPTIVE half and still +// fails open: a key there is never filed at all, and if the file cannot be read +// the run files anyway, because the cost is one issue a human closes — and that +// close then suppresses it permanently, which is a better end state than a loop +// silently disabled by an unreachable file. The PR-history half retires with +// the PR: this run opens none, so no cooldown is computed from one. // // --- CONFIG the operator sets (out of band) --------------------------------- // // Every value that names an operator's own estate is a key, not a constant. -// This run is generic machinery — which repos it reads, which repo it writes -// to, and where in that repo the file lands are the operator's business and -// live in their config, never in this file. A default that names somebody's -// repo is a default that files a PR against it. +// This run is generic machinery — which repos it reads, which repo it files +// into, and what it labels the issues are the operator's business and live in +// their config, never in this file. A default that names somebody's repo is a +// default that opens issues on it. // // Unset keys are not uniform, and the split is deliberate. An unset `repos` is // a run nobody has pointed at anything yet: it warns and no-ops, because on a @@ -73,16 +110,18 @@ // opposite — the sweep would do all its work with nowhere to put the answer — // so that one fails the run loudly. // -// CONFIG_KV org-spec-audit.repos comma/space-separated `owner/name` estate to sweep (optional — unset disables the sweep) -// CONFIG_KV org-spec-audit.base base branch to read (default "main") -// CONFIG_KV org-spec-audit.control-repo `owner/name` the questions PR lands in (REQUIRED — no default) -// CONFIG_KV org-spec-audit.questions-dir repo-relative dir for `.md` (default "maintenance/questions") -// CONFIG_KV org-spec-audit.declined-path repo-relative declines ledger (default "maintenance/declined.jsonl") -// CONFIG_KV org-spec-audit.window-hours skip a repo with no commits in this window (default "26") -// CONFIG_KV org-spec-audit.backend "workers-ai" | "anthropic" | "bedrock" (default workers-ai) -// CONFIG_KV org-spec-audit.prompt (optional) override the question-detection system prompt -// CONFIG_KV org-spec-audit.workers-ai.model model id -// CONFIG_KV org-spec-audit.workers-ai.mode "tools" | "json" (default "tools") +// CONFIG_KV org-spec-audit.repos comma/space-separated `owner/name` estate to sweep (optional — unset disables the sweep) +// CONFIG_KV org-spec-audit.base base branch to read (default "main") +// CONFIG_KV org-spec-audit.control-repo `owner/name` the question issues land in (REQUIRED — no default) +// CONFIG_KV org-spec-audit.questions-label label marking a question issue (default "maintenance:open-question") +// CONFIG_KV org-spec-audit.lane-label-prefix prefix + group → the lane label (default "question:") +// CONFIG_KV org-spec-audit.max-new-questions issues filed per sweep; the rest are counted (default 5) +// CONFIG_KV org-spec-audit.declined-path repo-relative declines ledger (default "maintenance/declined.jsonl") +// CONFIG_KV org-spec-audit.window-hours skip a repo with no commits in this window (default "26") +// CONFIG_KV org-spec-audit.backend "workers-ai" | "anthropic" | "bedrock" (default workers-ai) +// CONFIG_KV org-spec-audit.prompt (optional) override the question-detection system prompt +// CONFIG_KV org-spec-audit.workers-ai.model model id +// CONFIG_KV org-spec-audit.workers-ai.mode "tools" | "json" (default "tools") // // Mode: Schedule mode — specs/04-gha-integration.md § Schedule mode. The cron // MUST also be in wrangler.jsonc `triggers.crons`. @@ -99,7 +138,7 @@ import { step, type Container, } from "@fractalboxdev/flare-dispatch-core"; -import type { CheckoutFailed, GitHubApiError } from "@fractalboxdev/flare-dispatch-core"; +import type { CheckoutFailed, GitHubApiError, IssueRef } from "@fractalboxdev/flare-dispatch-core"; import { checkSuppression, DECLINED_LEDGER_PATH, @@ -129,29 +168,51 @@ const key = namespacedKey(NAMESPACE); const REPOS_KEY = key("repos"); const BASE_KEY = key("base"); const CONTROL_REPO_KEY = key("control-repo"); -const QUESTIONS_DIR_KEY = key("questions-dir"); +const QUESTIONS_LABEL_KEY = key("questions-label"); +const LANE_LABEL_PREFIX_KEY = key("lane-label-prefix"); +const MAX_NEW_QUESTIONS_KEY = key("max-new-questions"); const DECLINED_PATH_KEY = key("declined-path"); const WINDOW_HOURS_KEY = key("window-hours"); /** - * Where the dated questions file lands inside the control repo. + * The label every question issue carries — the ledger's INDEX. * - * A directory, not a template: the run appends `.md`, so there is no - * placeholder syntax to get wrong and no way for config to name a single file - * that every day overwrites. + * Machine state, not a human affordance: the dedup read filters on it to find + * every question this run has ever asked, so a question whose label someone + * removes becomes fileable again. The lane labels below are the opposite — + * nothing matches on them, so they are safe to retriage by hand. */ -const QUESTIONS_DIR_DEFAULT = "maintenance/questions"; +const QUESTIONS_LABEL_DEFAULT = "maintenance:open-question"; +const LANE_LABEL_PREFIX_DEFAULT = "question:"; /** - * The `maintenance-key` namespace and the branch prefix every proposal shares. + * How many issues one sweep may open. * - * Both are load-bearing for suppression: the key is what the ledger matches on, - * and the prefix is how a later tick finds the PRs a human already closed - * (each day's proposal gets its own dated branch, so the prefix is all they - * have in common). + * A first sweep of a widened estate can find twenty, and twenty new issues at + * 05:45 is a wall rather than a digest. What the cap holds back is counted and + * named in the notice — never dropped silently, because a shorter list that + * does not say it is shorter reads as fewer problems — and files on the next + * sweep, which finds it un-filed and therefore fresh. */ +const MAX_NEW_QUESTIONS_DEFAULT = 5; + +/** + * Page ceiling for the ledger read — 2,000 label-filtered issues. + * + * Well above the library's default of 5 pages, because this read is `strict` and + * a strict read that outgrows its ceiling fails EVERY tick until someone raises + * it. It costs nothing until the set is that large: pagination stops on the first + * short page, so today it is one request. + * + * Read the number as a canary rather than a limit. Two thousand questions asked + * and never closed is not a pagination problem — it is [§8]'s failure mode + * arrived, the loop having asked more than the team answers, and a run going red + * there is closer to correct than a run quietly deciding on the first 500. + */ +const LEDGER_MAX_PAGES = 20; + +/** The `maintenance-key` namespace every question is suppressed by. */ const MAINTENANCE_SOURCE = "org-spec-audit"; -const BRANCH_PREFIX = "flare-dispatch/spec-audit-questions-"; /** The stable, repo-independent id a question is suppressed by. */ const maintenanceKey = (questionKey: string): string => `${MAINTENANCE_SOURCE}/${questionKey}`; @@ -172,8 +233,10 @@ const MAX_SPECS_CHARS = 40_000; const MAX_TREE_CHARS = 12_000; const QUESTIONS_MAX_TOKENS = 2048; -/** How many questions per group reach the message. The rest are counted, not dropped silently. */ -const PER_GROUP_CAP = 5; +// A per-group rendering cap used to live here, because the message carried every +// standing question and a long group buried the rest. The notice now lists only +// what was FILED this tick, which `max-new-questions` already bounds — one cap +// instead of two, and the one that bounds the writes is the one that matters. /** * The four groups, in message order. @@ -211,12 +274,61 @@ const AuditQuestions = Schema.Struct({ * A stable, repo-INDEPENDENT slug of the underlying question. Two repos * asking the same thing must produce the same key or the cross-repo * merge — the reason this run sweeps at all — silently does nothing. + * + * Stable across DAYS as well as repos, which is the harder half and was + * once wrong here: the same question arrived as `authorize-pipeline-dags` + * one day and `adopt-pipeline-dags` the next, matching nothing. The prompt + * asks for a noun phrase for that reason, and `reconcileKeys` catches what + * prompt discipline misses — a rule with no mechanism behind it is a rule + * that holds until the model rephrases. */ key: Schema.String, }), ), }); +/** + * One verdict per newly-minted key: the key already on file that it means the + * same question as, or `""` for none. + * + * Deliberately not "is this a duplicate, yes/no" — the model has to NAME the + * question it thinks this duplicates, and that name is then checked against the + * set actually read from the control repo. A key it invents matches nothing and + * the question gets filed, which is the safe direction: the cost of a missed + * match is one duplicate issue a human closes, and the cost of an accepted + * hallucination is a question silently never asked. + */ +const KeyReconciliation = Schema.Struct({ + matches: Schema.Array( + Schema.Struct({ + /** The key this sweep minted, echoed back so the mapping is unambiguous. */ + minted: Schema.String, + /** An existing key from the list given, or `""` when this question is new. */ + existing: Schema.String, + }), + ), +}); + +const RECONCILE_PROMPT = `You are matching newly-raised questions against questions already on file. + +For each NEW question you are given, decide whether it is THE SAME QUESTION as +one of the questions already on file — the same decision, needing the same +answer, however differently it is worded. Wording, framing and the verb used +carry no weight; the subject and the decision it needs are what matter. + +Rules: +- Answer with the existing key when it is the same question, and "" when it is + not. Every new key you were given gets exactly one row. +- Only ever answer with a key from the on-file list, verbatim. Never invent one, + never adjust one, and never answer with a new key. +- Narrower or broader is NOT the same question. "Should we support DAGs" and + "should we deprecate the linear model" need different answers; keep them apart. +- When you are unsure, answer "". A duplicate question costs a human one click; + a question wrongly matched away is never asked again.`; + +const RECONCILE_JSON_CONTRACT = `{"matches":[{"minted":string,"existing":string}]}`; +const RECONCILE_MAX_TOKENS = 1024; + /** The question-detection prompt (operator-overridable). */ const QUESTIONS_PROMPT_DEFAULT = `You read a project's specs/ against its file tree and recent commits, and you report ONLY what cannot be reconciled automatically. @@ -236,8 +348,12 @@ Rules, each of which drops a finding when broken: evidence, no question. - State the assumption we should make if nobody answers. A question without one waits for a meeting. -- The key is a short lowercase slug of the UNDERLYING question, with no repo - name in it, so the same question asked in two repos merges into one line. +- The key is a short lowercase slug NAMING THE SUBJECT of the question, as a + noun phrase and never a verb phrase: "pipeline-dags", not + "adopt-pipeline-dags", "authorize-pipeline-dags" or "should-we-adopt-dags". A + verb encodes the action being proposed, which changes with how you phrase it; + the subject of the question does not. No repo name in it, so the same question + asked in two repos merges into one line. - Report nothing rather than padding. An empty array is a good answer.`; const Input = Schema.Struct({ @@ -249,9 +365,20 @@ const Output = Schema.Struct({ reposSkipped: Schema.Number, questionsRaised: Schema.Number, questionsAfterMerge: Schema.Number, - /** Merged questions the ledger or a cooldown kept out of the proposal. */ + /** + * Merged questions that already have an issue, in any state. + * + * The number this whole design exists to make non-zero on a steady estate: on + * a quiet week every question the sweep raises is one already on file, and the + * correct output is no writes at all. + */ + questionsAlreadyFiled: Schema.Number, + /** Merged questions the declines ledger kept from being filed. */ questionsSuppressed: Schema.Number, - prOpened: Schema.Boolean, + /** Issues actually opened this tick. */ + questionsFiled: Schema.Number, + /** Fresh questions the per-sweep cap held back — they file on the next tick. */ + questionsHeldByCap: Schema.Number, }); export const orgSpecAudit = defineRun({ @@ -309,8 +436,10 @@ export const orgSpecAudit = defineRun({ reposSkipped: 0, questionsRaised: 0, questionsAfterMerge: 0, + questionsAlreadyFiled: 0, questionsSuppressed: 0, - prOpened: false, + questionsFiled: 0, + questionsHeldByCap: 0, }; } @@ -341,13 +470,76 @@ export const orgSpecAudit = defineRun({ // model calls whose output has nowhere to go. See // `primitives/control-plane` for the rule and the failure text. const controlRepo = yield* resolveControlRepo(CONTROL_REPO_KEY); - const questionsDir = yield* resolveRepoRelativePath(QUESTIONS_DIR_KEY, QUESTIONS_DIR_DEFAULT); const declinedPath = yield* resolveRepoRelativePath(DECLINED_PATH_KEY, DECLINED_LEDGER_PATH); + // Set-and-unusable fails; unset takes the default. A label that cannot be + // both filtered on and applied would break dedup silently — see + // `parseLabel`. + const questionsLabel = parseLabel( + yield* step("resolve-questions-label", () => config.get(QUESTIONS_LABEL_KEY)), + QUESTIONS_LABEL_DEFAULT, + ); + if (questionsLabel === undefined) { + return yield* Effect.fail( + new StepFailed({ + step: "resolve-questions-label", + cause: `${QUESTIONS_LABEL_KEY} is not a usable GitHub label (no comma, max ${LABEL_MAX_CHARS} chars)`, + }), + ); + } + const lanePrefix = parseLabel( + yield* step("resolve-lane-prefix", () => config.get(LANE_LABEL_PREFIX_KEY)), + LANE_LABEL_PREFIX_DEFAULT, + ); + if (lanePrefix === undefined) { + return yield* Effect.fail( + new StepFailed({ + step: "resolve-lane-prefix", + cause: `${LANE_LABEL_PREFIX_KEY} is not a usable GitHub label prefix (no comma, max ${LABEL_MAX_CHARS} chars)`, + }), + ); + } + const maxNew = parsePositiveInt( + yield* step("resolve-max-new", () => config.get(MAX_NEW_QUESTIONS_KEY)), + MAX_NEW_QUESTIONS_DEFAULT, + ); + const windowHours = parseWindowHours( yield* step("resolve-window", () => config.get(WINDOW_HOURS_KEY)), ); + // 1b. The ledger read — BEFORE the sweep, not after. + // + // It is one cheap call whose failure ends the tick, so it belongs + // where the other deterministic exits are (§7). Reading it after the + // sweep would mean paying for an estate's worth of model calls and + // then discarding every one of them. + // + // `state: "all"` because a question answered a year ago must still + // suppress; un-windowed for the same reason; `strict` because a list + // the page ceiling cut short answers "not filed" for everything it + // never reached, and this read's answer to that question is what + // decides whether anything is written. + // + // No `catchAll`. A failure here fails the run, which is the whole + // inversion: better a day with no questions filed than a day that + // files a duplicate of every question on file. + const onFile = yield* step("read-question-ledger", () => + github.issues({ + repo: controlRepo, + state: "all", + labels: [questionsLabel], + strict: true, + maxPages: LEDGER_MAX_PAGES, + }), + ); + const filed = indexFiledQuestions(onFile); + yield* io.log( + "info", + `org-spec-audit: ${filed.size} question(s) already on file in ${controlRepo} ` + + `(${onFile.filter((i) => i.state === "open").length} open)`, + ); + // 2. The backend, under THIS run's namespace. A misconfigured backend // fails loudly — an audit that silently answers nothing is worse than // one that does not run. @@ -390,7 +582,7 @@ export const orgSpecAudit = defineRun({ const swept = outcomes.filter((o) => !o.skipped).length; const skipped = outcomes.filter((o) => o.skipped).length; - // 4. Empty means silent — no PR and, now, no notice. A digest that fires + // 4. Empty means silent — no issue and no notice. A digest that fires // whether or not there is news is one people learn to skip, and that // is far more expensive in a channel than in a repo: the day it does // have something to say, nobody is reading. @@ -401,108 +593,156 @@ export const orgSpecAudit = defineRun({ reposSkipped: skipped, questionsRaised: raised.length, questionsAfterMerge: 0, + questionsAlreadyFiled: 0, questionsSuppressed: 0, - prOpened: false, + questionsFiled: 0, + questionsHeldByCap: 0, }; } - // 5. Suppression, BEFORE anything is proposed. A question the ledger - // declined is never asked again; one whose proposal a human closed - // unmerged waits out a cooldown dated from the close. Both reads fail - // OPEN and say so — a duplicate PR is a nuisance, a silently disabled - // loop is the failure this whole mechanism exists to prevent. + // 5. Which of today's questions are already on file? Exact keys first, + // deterministically and for free — a matching key IS the same question + // and needs nothing to confirm it. + const exact = merged.filter((q) => filed.has(maintenanceKey(q.key))); + const residue = merged.filter((q) => !filed.has(maintenanceKey(q.key))); + + // 6. Then reconcile the residue, ONCE, for the whole batch. A key is + // minted from prose, so two sweeps can name one question twice — + // `authorize-pipeline-dags` and `adopt-pipeline-dags` were the same + // question on consecutive days. Asking a model to MATCH against what is + // on file is far more stable than asking it to invent the same slug + // twice, and it is one call for the batch rather than one per question. + // + // Skipped entirely when there is nothing on file to match against — + // on a fresh control repo every question is new by construction. + const matchRows = + residue.length > 0 && filed.size > 0 + ? yield* step("reconcile-keys", () => + reconcileKeys({ + residue, + onFile: [...filed.values()], + resolved, + }), + ).pipe( + // A model that cannot answer must not be able to file duplicates + // OR to suppress questions: falling back to "nothing matched" + // files the residue, which is the direction whose worst case is a + // human closing an issue. + Effect.catchAll((err) => + io + .log( + "warn", + `org-spec-audit: key reconciliation failed (${describe(err)}) — treating all ${residue.length} as new`, + ) + .pipe(Effect.as([] as readonly KeyMatch[])), + ), + ) + : ([] as readonly KeyMatch[]); + const reconciled = new Map(matchRows.map((m) => [m.minted, m.existing])); + + const alreadyFiled = [...exact, ...residue.filter((q) => reconciled.has(q.key))]; + const unfiled = residue.filter((q) => !reconciled.has(q.key)); + + for (const q of residue) { + const match = reconciled.get(q.key); + if (match !== undefined) { + yield* io.log( + "info", + `org-spec-audit: "${q.key}" reconciled onto ${match} — already asked, not re-filed`, + ); + } + } + + // 7. The declines ledger — the pre-emptive half, and the only read here + // that still fails OPEN. A key in it is never filed at all; if the file + // cannot be read the run files anyway, because the cost is one issue a + // human closes and that close then suppresses it for good. + // + // No `headBranchPrefix`: this run opens no PRs, so there is no PR + // history to date a cooldown from. The issue's own state is the memory. const suppression = yield* step("check-suppression", () => checkSuppression({ - keys: merged.map((q) => maintenanceKey(q.key)), - // The ledger and the proposals live in the same control repo, so one - // installation covers both reads. + keys: unfiled.map((q) => maintenanceKey(q.key)), ledgerRepo: controlRepo, ledgerPath: declinedPath, - headBranchPrefix: BRANCH_PREFIX, nowMs: input.firedAt, }), ); const allowed = new Set(suppression.allowed); - const proposed = merged.filter((q) => allowed.has(maintenanceKey(q.key))); + const fresh = unfiled.filter((q) => allowed.has(maintenanceKey(q.key))); - // Every question suppressed is a *good* tick, and a silent one — there is - // nothing new to ask. The count still lands in the output so a digest can - // say "0 new, 3 suppressed" rather than implying a quiet estate. - if (proposed.length === 0) { + // 8. Nothing fresh is the STEADY STATE, not a failure — every question + // the sweep raised is one somebody has already been asked. It is also + // the tick that used to open a duplicate PR, so the log line says which + // of the two reasons produced the silence. + if (fresh.length === 0) { yield* io.log( "info", - `org-spec-audit: ${merged.length} question(s), all suppressed — no PR opened`, + `org-spec-audit: ${merged.length} question(s), ${alreadyFiled.length} already on file, ` + + `${suppression.suppressed.length} declined — nothing to file`, ); return { reposSwept: swept, reposSkipped: skipped, questionsRaised: raised.length, questionsAfterMerge: merged.length, + questionsAlreadyFiled: alreadyFiled.length, questionsSuppressed: suppression.suppressed.length, - prOpened: false, + questionsFiled: 0, + questionsHeldByCap: 0, }; } - // 6. One control-plane PR against the configured control repo. The file - // it carries is the durable record — and the same text is what gets - // announced. - const message = renderMessage({ + // 9. File. One issue per question, capped, sequentially — this is a write + // of at most `maxNew`, and a stable order makes the notice's issue + // numbers read in the same order as its lines. + const toFile = fresh.slice(0, maxNew); + const heldByCap = fresh.length - toFile.length; + + const opened = yield* Effect.forEach( + toFile, + (q) => + step(`file-${q.key}`, () => + github.openIssue({ + repo: controlRepo, + title: issueTitle(q), + body: renderIssueBody({ question: q, day, declinedPath }), + labels: [questionsLabel, `${lanePrefix}${q.group}`], + }), + ).pipe(Effect.map((created) => ({ question: q, ...created }))), + { concurrency: 1 }, + ); + + // 10. Say what CHANGED. Not the standing list — a reader who has already + // seen eleven questions is not helped by being shown eleven questions, + // and a message whose length tracks the sweep rather than the news is + // one people stop opening. + const openOnFile = onFile.filter((i) => i.state === "open").length + opened.length; + const notice_ = renderNotice({ day, - merged: proposed, + opened, + openOnFile, + heldByCap, + alreadyFiled: alreadyFiled.length, outcomes, raised: raised.length, suppression, }); - const result = yield* step("open-questions-pr", () => - github.openDraftPullRequest({ - repo: controlRepo, - baseBranch, - headBranch: `${BRANCH_PREFIX}${day}`, - title: `docs(maintenance): open questions from the spec audit sweep (${day})`, - body: renderPrBody({ - day, - merged: proposed, - outcomes, - raised: raised.length, - suppression, - message, - declinedPath, - }), - commitMessage: `docs(maintenance): spec audit open questions (${day})\n\nGenerated by flare-dispatch org-spec-audit.`, - files: [ - { - path: `${questionsDir}/${day}.md`, - content: message, - }, - ], - }), - ); - // 7. Say it out loud. The same `message`, verbatim — the file and the - // announcement are one rendering on purpose, so nobody has to ask - // which of two wordings is the real one. It is the SUPPRESSION-FILTERED - // rendering, so a tick that proposes nothing new announces nothing new - // either. No markup is built here: - // `text` is data the receiver escapes, and the PR link rides in the - // typed `links` field precisely because markup inside `text` would be - // escaped along with everything else. - // - // `dedupeKey` is the day, which is also this run's schedule - // idempotency key. Deterministic per (run, day) and free of any clock - // read, so a retried step re-sends bytes the receiver has already - // claimed and gets a 409 instead of posting the digest twice. yield* step("publish-notice", () => notice.publish({ useCase: NOTICE_USE_CASE, dedupeKey: day, - text: message, - links: [{ url: result.url, label: "the questions PR" }], + text: notice_, + links: opened.map((o) => ({ url: o.url, label: `#${o.number}` })), }), ); yield* io.log( "info", - `org-spec-audit: ${proposed.length} question(s) from ${raised.length} raised (${suppression.suppressed.length} suppressed) — ${result.created ? "opened" : "updated"} PR #${result.number}`, + `org-spec-audit: filed ${opened.length} question(s) (${opened.map((o) => `#${o.number}`).join(", ")}) ` + + `from ${raised.length} raised — ${alreadyFiled.length} already on file, ` + + `${suppression.suppressed.length} declined, ${heldByCap} held by the cap`, ); return { @@ -510,8 +750,10 @@ export const orgSpecAudit = defineRun({ reposSkipped: skipped, questionsRaised: raised.length, questionsAfterMerge: merged.length, + questionsAlreadyFiled: alreadyFiled.length, questionsSuppressed: suppression.suppressed.length, - prOpened: result.created, + questionsFiled: opened.length, + questionsHeldByCap: heldByCap, }; }), }); @@ -722,8 +964,7 @@ const INVISIBLE = * Collapsing rather than escaping keeps the digest readable and costs nothing * real: the prompt already contracts each of these fields to a single sentence. */ -const oneLine = (raw: string): string => - raw.replace(INVISIBLE, "").replace(/\s+/g, " ").trim(); +const oneLine = (raw: string): string => raw.replace(INVISIBLE, "").replace(/\s+/g, " ").trim(); /** * The longest question-key that survives the round trip. @@ -763,6 +1004,166 @@ export const parseWindowHours = (raw: string | undefined | null): number => { return Number.isFinite(n) && n > 0 ? n : WINDOW_HOURS_DEFAULT; }; +/** GitHub's own limit. A longer name is rejected at the API, not truncated here. */ +const LABEL_MAX_CHARS = 50; + +/** + * A label from config. Unset takes the default; **set-and-unusable is + * `undefined`**, which the caller turns into a failed run. + * + * Falling back on a bad value would be the worse of the two behaviours, and not + * by a little: this label is BOTH the filter the dedup read applies and the + * label the write applies. A comma makes those two different things — GitHub's + * list query joins labels on commas, so the read would filter on two labels + * while the write applied one — and the symptom is every question re-filing + * forever with nothing anywhere erroring. Same reasoning as the base ref. + */ +export const parseLabel = ( + raw: string | undefined | null, + fallback: string, +): string | undefined => { + if (raw === undefined || raw === null || raw.trim() === "") return fallback; + const v = raw.trim(); + if (v.includes(",") || v.length > LABEL_MAX_CHARS) return undefined; + return v; +}; + +/** A positive-integer config value, falling back when unset or unparseable. */ +export const parsePositiveInt = (raw: string | undefined | null, fallback: number): number => { + const n = Number.parseInt(raw ?? "", 10); + return Number.isFinite(n) && n > 0 ? n : fallback; +}; + +/** + * The FIRST `maintenance-key` line in an issue body, or `undefined`. + * + * First, not last, and not all of them: an issue body carries model prose + * derived from a swept repo, so a spec can contain a line that looks exactly + * like a trailer. `renderIssueBody` emits the authentic key as the body's first + * line for precisely this reason, so first-match is what makes a spoofed key + * inert rather than authoritative. + */ +export const firstMaintenanceKey = (body: string): string | undefined => { + for (const line of body.split("\n")) { + const m = /^maintenance-key:\s*(\S+)\s*$/.exec(line.trim()); + if (m?.[1] !== undefined) return m[1]; + } + return undefined; +}; + +/** + * Index the questions already on file, by `maintenance-key`. + * + * Keys outside this run's namespace are ignored, so another consumer sharing the + * questions label cannot make this run believe it has already asked something. + * An issue with no key at all is ignored too — a human-opened issue that happens + * to carry the label is a question this run did not ask and cannot match. + * + * On a duplicate key the first wins, which is the most recently updated (GitHub + * lists `sort=updated&direction=desc`). Which one wins does not matter to the + * caller — presence is the whole answer — but it should be deterministic. + */ +export const indexFiledQuestions = (issues: readonly IssueRef[]): Map => { + const byKey = new Map(); + for (const issue of issues) { + const key = firstMaintenanceKey(issue.body); + if (key === undefined || !key.startsWith(`${MAINTENANCE_SOURCE}/`)) continue; + if (!byKey.has(key)) byKey.set(key, issue); + } + return byKey; +}; + +/** Strip the `org-spec-audit/` namespace — the reconcile prompt speaks bare keys. */ +const bareKey = (namespaced: string): string => namespaced.slice(MAINTENANCE_SOURCE.length + 1); + +type ReconcileArgs = { + /** Today's questions with no exact key match — the only ones worth asking about. */ + readonly residue: readonly MergedQuestion[]; + readonly onFile: readonly IssueRef[]; + readonly resolved: { backend: string; model: string; mode: "tools" | "json" }; +}; + +/** One accepted match, as a plain row — see `reconcileKeys` on why not a Map. */ +type KeyMatch = { readonly minted: string; readonly existing: string }; + +/** + * Match today's un-filed questions against the ones already on file, returning + * one row per question that duplicates one. + * + * A plain array rather than a `Map`, because this runs inside `step()` and a + * step's result is checkpointed as JSON — a `Map` serializes to `{}`, which + * would silently mean "nothing matched" on a replay. The caller indexes it. + * + * **Every answer is checked against the set actually read from the control + * repo.** A model fully talked into "this duplicates `org-spec-audit/whatever`" + * produces no match, because `whatever` was never in the list — the same + * containment `closeIssueAsDuplicate`'s `knownNumbers` uses. The failure + * direction is deliberate: an unmatched duplicate costs a human one click, and + * an accepted hallucination is a question that is never asked again. + */ +const reconcileKeys = (args: ReconcileArgs) => + Effect.gen(function* () { + const known = new Map(); + for (const issue of args.onFile) { + const key = firstMaintenanceKey(issue.body); + if (key !== undefined) known.set(bareKey(key), key); + } + const minted = new Set(args.residue.map((q) => q.key)); + + const result = yield* completeStructured({ + backend: args.resolved.backend, + model: args.resolved.model, + mode: args.resolved.mode, + system: RECONCILE_PROMPT, + userBody: renderReconcileBody({ residue: args.residue, onFile: args.onFile }), + jsonContract: RECONCILE_JSON_CONTRACT, + schema: KeyReconciliation, + toolName: "report_key_matches", + toolDescription: 'For each newly-raised key, the on-file key it duplicates, or "".', + surface: "org-spec-audit", + maxTokens: RECONCILE_MAX_TOKENS, + }); + + const matches: KeyMatch[] = []; + const seen = new Set(); + for (const m of result.matches) { + const from = m.minted.trim(); + const onto = m.existing.trim(); + // A row about a key we did not ask about, or an on-file key that does not + // exist, decides nothing. Both are dropped silently rather than logged per + // row: a model listing a stale key is ordinary, and the interesting event + // (a question NOT filed because it matched) is logged by the caller. + if (onto === "" || !minted.has(from) || seen.has(from)) continue; + const resolvedKey = known.get(onto) ?? known.get(bareKey(onto)); + if (resolvedKey === undefined) continue; + seen.add(from); + matches.push({ minted: from, existing: resolvedKey }); + } + return matches; + }); + +/** The reconcile call's data half: what is on file, and what was just raised. */ +const renderReconcileBody = (args: { + readonly residue: readonly MergedQuestion[]; + readonly onFile: readonly IssueRef[]; +}): string => { + const filed = args.onFile.flatMap((issue) => { + const key = firstMaintenanceKey(issue.body); + if (key === undefined) return []; + return [ + `- key: ${bareKey(key)}\n question: ${oneLine(issue.title)}\n status: ${issue.state}`, + ]; + }); + + return [ + "## Questions already on file", + filed.length > 0 ? filed.join("\n") : "(none)", + "", + "## Questions raised just now", + args.residue.map((q) => `- key: ${q.key}\n question: ${q.question}`).join("\n"), + ].join("\n"); +}; + // --- In-container gather scripts (plain `git`, no extra CLI) ----------------- /** @@ -828,38 +1229,101 @@ const renderUserBody = (ctx: { const MARKER = ""; -type RenderArgs = { +/** GitHub truncates a longer title in its own UI; cut it where we can see it. */ +const MAX_TITLE_CHARS = 240; + +/** The issue title — the question as asked, which is what a reader scans. */ +export const issueTitle = (q: MergedQuestion): string => + q.question.length > MAX_TITLE_CHARS ? `${q.question.slice(0, MAX_TITLE_CHARS - 1)}…` : q.question; + +/** + * One question's issue body. + * + * **The trailer block comes first, and that order is load-bearing.** Everything + * below it is model output derived from the contents of a swept repo, so a spec + * crafted to make the model emit `maintenance-key: org-spec-audit/something` + * would — with the trailer last — put a spoofed key ahead of the real one for + * any reader that takes the first match. `indexFiledQuestions` takes exactly + * that first match, so emitting the authentic key first makes anything the model + * echoes inert text further down. + */ +export const renderIssueBody = (args: { + readonly question: MergedQuestion; + readonly day: string; + readonly declinedPath: string; +}): string => { + const q = args.question; + return ( + [ + `maintenance-key: ${maintenanceKey(q.key)}`, + MARKER, + "", + `> 🤖 Filed by \`flare-dispatch/org-spec-audit\` on ${args.day} — a divergence where` + + ` *which side is right* is a judgment nobody has made yet, not drift (\`spec-drift-pr\`` + + ` proposes those).`, + "", + `> **Answer in the thread and close this.** Closing is the record: a later sweep that` + + ` finds this question still unsettled will not re-file it and will never reopen it.` + + ` To keep it from ever being asked again, add its key to \`${args.declinedPath}\`.`, + "", + `**If nobody answers:** ${q.assumption}`, + "", + q.evidence, + "", + `Raised by: ${q.sources.map((s) => `\`${s.repo}\` (${s.specPath})`).join(" · ")}`, + ].join("\n") + "\n" + ); +}; + +/** One filed issue, as the notice reports it. */ +type OpenedQuestion = { + readonly question: MergedQuestion; + readonly number: number; + readonly url: string; +}; + +type NoticeArgs = { readonly day: string; - /** The questions actually being proposed — suppressed ones are already out. */ - readonly merged: readonly MergedQuestion[]; + /** Filed this tick — the news, and the only questions the notice lists. */ + readonly opened: readonly OpenedQuestion[]; + /** Open questions carrying the label after this tick, filed ones included. */ + readonly openOnFile: number; + readonly heldByCap: number; + readonly alreadyFiled: number; readonly outcomes: readonly RepoOutcome[]; readonly raised: number; - /** What suppression kept out, and whether either read degraded. */ readonly suppression: SuppressionReport; }; /** - * The file the PR carries — and the message a Slack consumer posts, verbatim. + * The announcement — the DELTA, not the standing list. + * + * The old rendering was the day's whole file, so its length tracked the sweep + * rather than the news and a reader who had already seen eleven questions was + * shown eleven questions again. This lists what was filed, counts what stands, + * and says what the cap held. * - * Written as GitHub markdown, not Slack mrkdwn: the canonical artifact is the - * reviewed file in git, and the Slack twin is derived at send time by whoever - * holds the token. + * Written as GitHub markdown, not Slack mrkdwn: the receiver holds the token and + * converts at send time, which is where escaping already lives. */ -export const renderMessage = (args: RenderArgs): string => { +export const renderNotice = (args: NoticeArgs): string => { const swept = args.outcomes.filter((o) => !o.skipped).map((o) => o.repo); const failed = args.outcomes.filter((o) => o.failure !== undefined); const quiet = args.outcomes .filter((o) => o.skipped && o.failure === undefined) .map((o) => o.repo); - const dropped = countDropped(args.merged); const lines: string[] = [ `# Spec audit — ${args.day}`, "", - `${swept.length} repo(s) swept · ${quiet.length} unchanged or without specs · ` + - `${failed.length} failed · ${args.merged.length} question(s) from ${args.raised} raised` + + `${args.opened.length} new question(s) · ${args.openOnFile} open · ` + + // The swept / unchanged / failed split stays in the headline: a failure + // counted as "unchanged" turns an outage into a quiet week, which is the + // sentence a reader is least likely to question. + `${swept.length} repo(s) swept · ${quiet.length} unchanged or without specs · ` + + `${failed.length} failed · ${args.raised} raised, ${args.alreadyFiled} already on file` + (args.suppression.suppressed.length > 0 - ? ` · ${args.suppression.suppressed.length} suppressed` + ? ` · ${args.suppression.suppressed.length} declined` : ""), "", // Immediately after the headline count, not in a footer: a reader who sees @@ -879,21 +1343,17 @@ export const renderMessage = (args: RenderArgs): string => { } for (const group of GROUPS) { - const inGroup = args.merged.filter((q) => q.group === group); + const inGroup = args.opened.filter((o) => o.question.group === group); if (inGroup.length === 0) continue; lines.push(`## ${GROUP_HEADING[group]} (${inGroup.length})`, ""); - for (const q of inGroup.slice(0, PER_GROUP_CAP)) { + for (const o of inGroup) { lines.push( - `- **${q.question}**`, - ` - ${q.evidence}`, - ` - raised by: ${q.sources.map((s) => `\`${s.repo}\` (${s.specPath})`).join(" · ")}`, - ` - if nobody answers: ${q.assumption}`, + `- **${o.question.question}** (#${o.number})`, + ` - raised by: ${o.question.sources.map((s) => `\`${s.repo}\` (${s.specPath})`).join(" · ")}`, + ` - if nobody answers: ${o.question.assumption}`, ); } - if (inGroup.length > PER_GROUP_CAP) { - lines.push(`- _${inGroup.length - PER_GROUP_CAP} more in this group, not shown._`); - } lines.push(""); } @@ -903,60 +1363,15 @@ export const renderMessage = (args: RenderArgs): string => { `Swept: ${swept.length > 0 ? swept.map((r) => `\`${r}\``).join(" · ") : "none"}`, `Unchanged or no \`specs/\`: ${quiet.length > 0 ? quiet.map((r) => `\`${r}\``).join(" · ") : "none"}`, `Failed: ${failed.length > 0 ? failed.map((o) => `\`${o.repo}\` (${o.failure})`).join(" · ") : "none"}`, - dropped > 0 ? `Below the per-group cap: ${dropped}` : "Nothing dropped by the cap.", + // Named, never silent: a list the cap shortened reads as fewer problems. + args.heldByCap > 0 + ? `Held by the per-sweep cap: ${args.heldByCap} — they file on the next sweep.` + : "Nothing held by the cap.", ); return `${lines.join("\n")}\n`; }; -/** How many merged questions the per-group cap keeps out of the message. */ -const countDropped = (merged: readonly MergedQuestion[]): number => - GROUPS.reduce((total, group) => { - const n = merged.filter((q) => q.group === group).length; - return total + Math.max(0, n - PER_GROUP_CAP); - }, 0); - -/** - * The PR body. Carries the loop's machine-readable lines plus the message - * itself, so a reviewer decides without opening the diff. - * - * **One `maintenance-key` line per question, not one per PR.** The key is what - * a later tick matches against the ledger and against this PR once it is - * closed, so it has to name the thing a human declines — a question. A dated - * per-PR key would be unique every day and suppress nothing, ever. - */ -const renderPrBody = ( - args: RenderArgs & { message: string; declinedPath: string }, -): string => - [ - "### Spec audit — the questions the sweep could not answer", - "", - "> 🤖 Draft opened by `flare-dispatch/org-spec-audit`. These are divergences where *which side is right* is a judgment nobody has made yet — not drift (`spec-drift-pr` proposes those). Answer in the thread or edit the file; merging records the answers.", - "", - `> Closing this unmerged suppresses every key below for 30 days. To suppress one permanently, add its key to \`${args.declinedPath}\` with a reason.`, - "", - // The trailers precede the message, and that order is load-bearing. Every - // line of `message` below is model output derived from the contents of the - // swept repos, so a spec crafted to make the model emit `auto-merge: yes` - // would, with the trailers last, put a spoofed value ahead of the real one - // for any consumer that reads the first match. Emitted first, the authentic - // trailers win and anything the model echoes is inert text further down. - ...args.merged.map((q) => `maintenance-key: ${maintenanceKey(q.key)}`), - `swept: ${ - args.outcomes - .filter((o) => !o.skipped) - .map((o) => o.repo) - .join(", ") || "none" - }`, - `suppressed: ${args.suppression.suppressed.length}`, - "auto-merge: never (specs are a sensitive path)", - MARKER, - "", - "---", - "", - args.message, - ].join("\n"); - /** The errors `sweepRepo`'s `catchAll` knows how to describe precisely. */ type CaughtError = | BackendUnconfigured