diff --git a/.agents/skills/work-issue/references/ci-and-merge.md b/.agents/skills/work-issue/references/ci-and-merge.md index 24fc8cc31..b9e9aeaf5 100644 --- a/.agents/skills/work-issue/references/ci-and-merge.md +++ b/.agents/skills/work-issue/references/ci-and-merge.md @@ -32,6 +32,9 @@ runtimes, failures/cancellations and unexpected skips leave proof unfinished. Keep drafts cheap and batch pushes. Ready synchronize runs fast feedback and leaves `proof` nonpassing; convert to draft and mark ready after the final push to request candidate confirmation. Open/reopen ready events also request it. +Draft runs report `draft gates`, not `proof`. A ready PR without a passing +`proof` from its confirmation run is unconfirmed, even if `draft gates` passed +or failed because its live read found the PR ready; request confirmation again. The candidate gate checks actual merge parents/tree against the live head/base. A changed base requires a fresh confirmation event even without a rebase; valid immutable browser/layout retention remains available. Endurance is fresh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9776188d1..394f1906f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -470,7 +470,7 @@ jobs: retention-days: 7 proof: - name: ${{ github.event_name == 'pull_request' && 'proof' || 'integrated or exhaustive proof' }} + name: ${{ github.event_name != 'pull_request' && 'integrated or exhaustive proof' || github.event.pull_request.draft && 'draft gates' || 'proof' }} # Evaluate even after failures or skips; never interpret missing selected proof as green. needs: [changeset, verify, pack, unit, runtime, runtime_host, browser, layout, captures, endurance] if: ${{ always() && !cancelled() }} diff --git a/docs/decisions/ci-utility.md b/docs/decisions/ci-utility.md index e9979186e..8d81ecaa3 100644 --- a/docs/decisions/ci-utility.md +++ b/docs/decisions/ci-utility.md @@ -103,6 +103,13 @@ proof and endurance wait for confirmation. Its `proof` check deliberately fails with the action needed: convert to draft, finish pushes, then mark ready again. Fast feedback being green is useful, but never sufficient merge evidence. The request belongs to one event, not a label that can approve later commits. +A draft run's aggregate is named `draft gates`, never `proof`, so its cheap +success cannot stand as a ready PR's proof. GitHub can queue a draft push's run +after the `ready_for_review` run and cancel it, leaving no `proof` at all. The +draft aggregate also reads the live PR and fails when that read finds it ready, +closed or at another head, which turns the cancelled request into a visible +failure; a transition after the read is still covered by the distinct name +([#2363](https://github.com/schlessera/brain-kit/issues/2363)). Main pushes still execute complete affected proof. Scheduled and manual runs execute exhaustive discovery with a distinct aggregate check name, so they cannot impersonate a PR's candidate confirmation. Title/label edits run the separate contract gate, not long diff --git a/scripts/candidate-proof.ts b/scripts/candidate-proof.ts index d92a28b86..9f3c26585 100644 --- a/scripts/candidate-proof.ts +++ b/scripts/candidate-proof.ts @@ -3,10 +3,13 @@ import { planFromEnvironment, proofCadence, proofSelections } from "./ci-plan"; import { checkout, type Checkout } from "./proof-inputs"; import { GitHubProofAPI, type API } from "./proof-reuse"; +const CONFIRMATION_REQUIRED = "Candidate confirmation required: convert this PR to draft, then mark ready for review after the final push."; + export async function validateCandidate(root: string, event: any, env: NodeJS.ProcessEnv, api?: API, actual: Checkout = checkout(root)): Promise { const cadence = proofCadence(env, event); - if (cadence === "feedback") throw new Error("Candidate confirmation required: convert this PR to draft, then mark ready for review after the final push."); + if (cadence === "feedback") throw new Error(CONFIRMATION_REQUIRED); + if (cadence === "draft") return requireLiveDraft(event, api); if (cadence !== "confirmation") return; const pr = event.pull_request; if (!Number.isSafeInteger(pr?.number) || pr.number < 1 || !pr.base?.ref || !pr.head?.sha || !pr.base?.sha || @@ -28,6 +31,19 @@ export async function validateCandidate(root: string, event: any, env: NodeJS.Pr console.log(`BRAIN_CANDIDATE_RECEIPT ${JSON.stringify({ event: event.action, head: pr.head.sha, base: pr.base.sha, checkout: actual })}`); } +/** Draft gates pass only while the PR is still a draft at this head. A draft run + * queued after the ready event cancels its confirmation run, so the payload alone + * cannot tell a cheap draft from a ready PR (#2363). */ +async function requireLiveDraft(event: any, api?: API): Promise { + const pr = event.pull_request; + if (!Number.isSafeInteger(pr?.number) || pr.number < 1 || !pr.head?.sha) throw new Error("Draft proof is missing its PR identity"); + if (!api) throw new Error("Draft proof requires live read-only GitHub readiness verification"); + const live = await api.json(`pulls/${pr.number}`); + if (live.state !== "open" || live.draft !== true || live.head?.sha !== pr.head.sha) { + throw new Error(`Draft proof is stale: the live PR is ready, closed or at another head. ${CONFIRMATION_REQUIRED}`); + } +} + export function validateSelections(root: string, event: any, env: NodeJS.ProcessEnv, outputs: Record | undefined): void { const cadence = proofCadence(env, event); if (outputs?.cadence !== cadence) throw new Error("Candidate selection cadence is missing or inconsistent"); diff --git a/scripts/check-ci-workflows.ts b/scripts/check-ci-workflows.ts index d0de95bc1..e66067b8b 100644 --- a/scripts/check-ci-workflows.ts +++ b/scripts/check-ci-workflows.ts @@ -64,7 +64,7 @@ export function workflowProblems(text: string, file: string): string[] { !job?.steps?.some((step: any) => step.run === `bun scripts/ci-proof.ts ${domain} --shard=\${{ matrix.shard }}/${PROOF_SHARDS[domain]}`)) fail(`${domain}: matrix and shard divisor must match PROOF_SHARDS`); } const proof = workflow.jobs?.proof; - if (proof?.name !== "${{ github.event_name == 'pull_request' && 'proof' || 'integrated or exhaustive proof' }}") fail("proof: manual or main runs must not impersonate candidate confirmation"); + if (proof?.name !== "${{ github.event_name != 'pull_request' && 'integrated or exhaustive proof' || github.event.pull_request.draft && 'draft gates' || 'proof' }}") fail("proof: draft, manual or main runs must not impersonate candidate confirmation"); if (JSON.stringify(proof?.needs) !== JSON.stringify(["changeset", ...names])) fail("proof: aggregate must observe every selected category"); if (proof?.if !== "${{ always() && !cancelled() }}") fail("proof: aggregate must observe failures/skips and reject cancellation"); for (const category of ["unit", "runtime", "runtime_host", "browser", "layout", "captures", "endurance"]) { diff --git a/tests/candidate-proof.test.ts b/tests/candidate-proof.test.ts index ef2d4838c..e4d564823 100644 --- a/tests/candidate-proof.test.ts +++ b/tests/candidate-proof.test.ts @@ -1,6 +1,6 @@ import { expect, test } from "bun:test"; import { proofCadence, planChecks, proofSelections } from "../scripts/ci-plan"; -import { validateCandidate } from "../scripts/candidate-proof"; +import { requireCandidate, validateCandidate } from "../scripts/candidate-proof"; import type { API } from "../scripts/proof-reuse"; const actual = { sha: "a".repeat(40), tree: "b".repeat(40), parents: ["c".repeat(40), "d".repeat(40)] }; @@ -36,6 +36,25 @@ test("intermediate green feedback cannot satisfy final candidate confirmation", await validateCandidate("", event, env, api(), actual); }); +test("a draft-mode run cannot pass once the live PR is ready, even when queued after the ready event", async () => { + const draft = { ...event, action: "synchronize", pull_request: { ...event.pull_request, draft: true } }; + const live = { state: "open", draft: true, head: event.pull_request.head }; + for (const action of ["opened", "reopened", "synchronize", "converted_to_draft"]) { + await validateCandidate("", { ...draft, action }, env, api({ "pulls/7": live }), actual); + await expect(validateCandidate("", { ...draft, action }, env, api(), actual)).rejects.toThrow("Draft proof is stale"); + } + await expect(validateCandidate("", draft, env, api(), actual)).rejects.toThrow("Candidate confirmation required"); + for (const change of [{ head: { sha: "e".repeat(40) } }, { state: "closed" }, { state: undefined }, { state: "merged" }, { draft: undefined }]) + await expect(validateCandidate("", draft, env, api({ "pulls/7": { ...live, ...change } }), actual)).rejects.toThrow("Draft proof is stale"); + const unreadable: API = { async json() { throw new Error("Proof API refused 503"); }, async log() { throw new Error("Unexpected log"); } }; + await expect(validateCandidate("", draft, env, unreadable, actual)).rejects.toThrow("Proof API refused 503"); + await expect(validateCandidate("", draft, env, undefined, actual)).rejects.toThrow("live read-only GitHub readiness"); + await expect(validateCandidate("", { ...draft, pull_request: { ...draft.pull_request, number: 0 } }, env, api({ "pulls/7": live }), actual)).rejects.toThrow("missing its PR identity"); + await expect(validateCandidate("", { ...draft, pull_request: { ...draft.pull_request, head: {} } }, env, api({ "pulls/7": live }), actual)).rejects.toThrow("missing its PR identity"); + // The hosted aggregate's entry point reaches the same live check; without a token it cannot pass. + await expect(requireCandidate("", draft, { ...env, GITHUB_ACTIONS: "true", GH_TOKEN: "" })).rejects.toThrow("live read-only GitHub readiness"); +}); + test("candidate gate rejects changed head, base, readiness, immutable tree and missing evidence", async () => { for (const live of [ { state: "open", draft: false, head: { sha: "e".repeat(40) } }, @@ -48,3 +67,23 @@ test("candidate gate rejects changed head, base, readiness, immutable tree and m await expect(validateCandidate("", event, env, api(), { ...actual, parents: [...actual.parents].reverse() })).rejects.toThrow("checkout does not match"); await expect(validateCandidate("", event, { ...env, GITHUB_SHA: "e".repeat(40) }, api(), actual)).rejects.toThrow("checkout does not match"); }); + +test("the hosted entry point reads live readiness through its own API client", async () => { + const draft = { ...event, action: "synchronize", pull_request: { ...event.pull_request, draft: true } }; + const hosted = { ...env, GITHUB_ACTIONS: "true", GH_TOKEN: "fixture-token" }; + const descriptor = Object.getOwnPropertyDescriptor(globalThis, "fetch")!; + const reads: string[] = []; + let live: unknown = { state: "open", draft: true, head: event.pull_request.head }; + Object.defineProperty(globalThis, "fetch", { ...descriptor, value: async (url: string) => { + reads.push(url); + return new Response(JSON.stringify(live), { status: 200 }); + } }); + try { + await requireCandidate("", draft, hosted); + live = { state: "open", draft: false, head: event.pull_request.head }; + await expect(requireCandidate("", draft, hosted)).rejects.toThrow("Draft proof is stale"); + } finally { + Object.defineProperty(globalThis, "fetch", descriptor); + } + expect(reads).toEqual(Array(2).fill(`https://api.github.com/repos/${env.GITHUB_REPOSITORY}/pulls/7`)); +}); diff --git a/tests/ci-workflows.test.ts b/tests/ci-workflows.test.ts index fbb794b9d..c3b0322c0 100644 --- a/tests/ci-workflows.test.ts +++ b/tests/ci-workflows.test.ts @@ -342,10 +342,12 @@ test("workflow guard rejects an omitted complete proof command and aggregate pre }); -test("workflow cannot check out a different tree or impersonate candidate proof from manual runs", () => { +test("workflow cannot check out a different tree or impersonate candidate proof from draft or manual runs", () => { const text = readFileSync(join(ROOT, ".github/workflows/ci.yml"), "utf8"); expect(workflowProblems(text.replace("persist-credentials: false", "persist-credentials: false\n ref: main"), "ci.yml").join("\n")).toContain("proof checkout must use the event candidate"); - expect(workflowProblems(text.replace("name: ${{ github.event_name == 'pull_request' && 'proof' || 'integrated or exhaustive proof' }}", "name: proof"), "ci.yml").join("\n")).toContain("must not impersonate candidate confirmation"); + expect(workflowProblems(text.replace("name: ${{ github.event_name != 'pull_request' && 'integrated or exhaustive proof' || github.event.pull_request.draft && 'draft gates' || 'proof' }}", "name: proof"), "ci.yml").join("\n")).toContain("must not impersonate candidate confirmation"); + // A draft run's aggregate keeps a distinct name, so its cheap success never stands as a ready PR's proof (#2363). + expect(workflowProblems(text.replace("github.event.pull_request.draft && 'draft gates' || ", ""), "ci.yml").join("\n")).toContain("must not impersonate candidate confirmation"); }); test("browser and layout matrices match the shared shard table (#2414)", () => {