Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .agents/skills/work-issue/references/ci-and-merge.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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() }}
Expand Down
7 changes: 7 additions & 0 deletions docs/decisions/ci-utility.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 17 additions & 1 deletion scripts/candidate-proof.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
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 ||
Expand All @@ -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<void> {
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<string, string> | undefined): void {
const cadence = proofCadence(env, event);
if (outputs?.cadence !== cadence) throw new Error("Candidate selection cadence is missing or inconsistent");
Expand Down
2 changes: 1 addition & 1 deletion scripts/check-ci-workflows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"]) {
Expand Down
41 changes: 40 additions & 1 deletion tests/candidate-proof.test.ts
Original file line number Diff line number Diff line change
@@ -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)] };
Expand Down Expand Up @@ -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) } },
Expand All @@ -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`));
});
6 changes: 4 additions & 2 deletions tests/ci-workflows.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)", () => {
Expand Down
Loading