ci: validate initial PR readiness declarations (#3302)

This commit is contained in:
morluto authored and GitHub committed 2026-10-14 16:51:59 +08:00
1 parent 7c962cc70b
commit 5f0065c542
9 files changed
+702 -14

No files matched your search

+4 -3
View File
@@ -100,15 +100,16 @@ limits, security/privacy/containment implications, and meaningful proof gaps. --
The PR author is the owner by default. A different owner must explicitly accept
ownership on this PR.
AI agents must leave these declarations to a human to dictate to them or there could be catastrophic consequences for many users of REA.
Draft PRs may leave them unchecked. After further changes, clear the final checkbox
until the owner reviews the updated diff and refreshes affected validation.
Draft PRs may leave them unchecked. These declarations record initial readiness
when opening the PR for review; later pushes do not require a new human sign-off.
Keep the description and validation evidence current as the work changes.
See CONTRIBUTING.md#pull-request-ownership-and-readiness. -->
Human owner: <!-- @GitHub-handle -->
- [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
- [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
- [ ] I declare the current revision ready for maintainer review.
- [ ] I declare this PR ready for initial maintainer review.
## Before merge
+51
View File
@@ -0,0 +1,51 @@
name: PR readiness
on:
pull_request_target:
branches: [main, "release/*"]
types:
[
opened,
edited,
synchronize,
reopened,
ready_for_review,
converted_to_draft,
]
workflow_dispatch:
inputs:
pull_request_number:
description: Open PR to recheck
required: true
type: string
permissions: {}
concurrency:
group: pr-readiness-${{ github.event.pull_request.number || inputs.pull_request_number }}
cancel-in-progress: true
jobs:
readiness:
name: Validate author declaration
runs-on: ubuntu-24.04
timeout-minutes: 3
permissions:
contents: read
pull-requests: read
statuses: write
steps:
- uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0
with:
ref: ${{ github.event.repository.default_branch }}
persist-credentials: false
- name: Check initial readiness
uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9
with:
script: |
const { pathToFileURL } = require("node:url");
const entry = pathToFileURL(
`${process.env.GITHUB_WORKSPACE}/scripts/ci/pr-readiness.mjs`,
);
const { runReadinessCheck } = await import(entry.href);
await runReadinessCheck({ github, context, core });
+6 -6
View File
@@ -94,7 +94,7 @@ required to open an issue.
Follow [human ownership and readiness](CONTRIBUTING.md#pull-request-ownership-and-readiness)
when preparing a PR. AI-assisted contributions are welcome; a human must review
and own the final diff, check the validation evidence and its limitations, and
explicitly declare the current revision ready for maintainer review.
explicitly declare the PR ready for initial maintainer review.
Keep the PR body concise and current, with the concrete problem, observable
outcome, approach, and evidence. Report only checks that actually ran as executed;
@@ -103,11 +103,11 @@ Address actionable automated review findings and explain disagreements.
Leave the template's **Author readiness** declarations unchecked for the human
owner to complete. Never attest to human review on their behalf. Keep the PR in
draft until the human declares the current diff ready. After further changes,
clear the ready-for-review checkbox and refresh affected evidence so the human
can review and renew the declaration. CI and automated review results support
review; they do not establish human readiness, maintainer scope approval, or
merge permission.
draft until the human declares the PR initially ready. Later pushes do not
invalidate that declaration or require another human sign-off. Keep the PR body
and affected validation current as work changes. The readiness check validates
the filled declaration; CI and automated review results do not establish that
human review occurred, maintainer scope approval, or merge permission.
<!-- BEGIN:turborepo-agent-rules -->
+18 -5
View File
@@ -67,7 +67,7 @@ Before requesting maintainer review, the human owner must:
passing. Follow the [verification requirements](#development-feedback-and-pr-verification)
for required checks and real-provider coverage.
- Complete the **Author readiness** declaration in the PR template, explicitly
stating that the current revision is ready for review.
stating that the PR is ready for initial review.
Keep the PR description concise and current. Explain the problem, observable
outcome, approach, and useful evidence; link long logs or put them in optional
@@ -78,10 +78,23 @@ approval or permission to merge.
Draft PRs may request early feedback with open questions and incomplete checks.
Leave the readiness declaration unchecked until the human owner has completed
it. After further changes, clear the ready-for-review checkbox until the owner
reviews the updated diff, refreshes affected validation and the PR description,
and renews the declaration. AI agents must leave human declarations to the human
owner and must not attest to a human review on their behalf.
it. The declaration records initial readiness; later pushes do not invalidate
it or require another human sign-off. Keep the PR description and affected
validation current as work changes. AI agents must leave human declarations to
the human owner and must not attest to a human review on their behalf.
The `PR readiness` check reports pending for drafts and requires a named human
owner and all three template declarations for non-drafts. It checks that the
declaration is filled, not whether the code or evidence is correct. Existing PRs
can copy the **Author readiness** section from the current template.
Open PRs must have distinct head commits because GitHub shares commit statuses.
Maintainers enable merge enforcement after the workflow is merged: run
`gh workflow run pr-readiness.yml -f pull_request_number=NUMBER` for existing
open PRs, then add `PR readiness` from GitHub Actions to the branch's required
status checks alongside its existing checks. Deploy the workflow to each
protected target branch before requiring it there. The workflow reads PR
metadata using trusted default-branch code and writes only a commit status.
Maintainers may return a PR to the author or close it when the problem is
unsupported, validation is missing, or the owner cannot explain the change.
+1
View File
@@ -10,6 +10,7 @@ const HOST_TEST_OWNERS = ["apple", "package", "sqlite"];
// workflows' release, publication and self-hosted runner safeguards.
const GUARDED_WORKFLOWS = new Set([
".github/workflows/pr-title.yml",
".github/workflows/pr-readiness.yml",
".github/workflows/ci-full-pr.yml",
".github/workflows/release.yml",
".github/workflows/real-ghidra-windows.yml",
+63
View File
@@ -0,0 +1,63 @@
export interface ReadinessPullRequest {
readonly number: number;
readonly state: "open" | "closed";
readonly draft: boolean;
readonly body: string | null;
readonly head: { readonly sha: string };
readonly base: { readonly ref: string };
}
export interface ReadinessResult {
readonly state: "success" | "failure" | "pending";
readonly description: string;
}
export interface ReadinessGitHub {
readonly rest: {
readonly pulls: {
get(input: {
owner: string;
repo: string;
pull_number: number;
}): Promise<{ data: ReadinessPullRequest }>;
};
readonly repos: {
createCommitStatus(
input: ReadinessResult & {
owner: string;
repo: string;
sha: string;
context: string;
target_url: string;
},
): Promise<unknown>;
readonly listPullRequestsAssociatedWithCommit: unknown;
};
};
paginate(
method: unknown,
input: {
owner: string;
repo: string;
commit_sha: string;
per_page: number;
},
): Promise<
readonly Pick<ReadinessPullRequest, "number" | "state" | "head">[]
>;
}
export function evaluateReadiness(
pull: Pick<ReadinessPullRequest, "body" | "draft">,
template: string,
): ReadinessResult;
export function runReadinessCheck(input: {
github: ReadinessGitHub;
context: {
repo: { owner: string; repo: string };
payload: {
pull_request?: { number: number };
inputs?: { pull_request_number?: string };
};
serverUrl: string;
runId: number;
};
core: { info(message: string): void; setFailed(message: string): void };
}): Promise<ReadinessResult | undefined>;
+173
View File
@@ -0,0 +1,173 @@
import { readFile } from "node:fs/promises";
const result = (state, description) => ({ state, description });
function readinessLines(markdown) {
const lines = markdown.split(/\r?\n/u);
const visible = [];
let fence;
let comment = false;
for (const source of lines) {
if (fence) {
if (
new RegExp(
`^ {0,3}${fence.character}{${fence.length},}\\s*$`,
"u",
).test(source)
)
fence = undefined;
continue;
}
const opening = !comment && /^ {0,3}(`{3,}|~{3,})/u.exec(source);
if (opening) {
fence = { character: opening[1][0], length: opening[1].length };
continue;
}
let line = "";
let offset = 0;
while (offset < source.length) {
const marker = comment ? "-->" : "<!--";
const index = source.indexOf(marker, offset);
if (!comment)
line += source.slice(offset, index === -1 ? source.length : index);
if (index === -1) break;
offset = index + marker.length;
comment = !comment;
}
visible.push(line);
}
const starts = visible.flatMap((line, index) =>
/^## Author readiness\s*$/u.test(line) ? [index] : [],
);
if (starts.length !== 1) return [];
const remaining = visible.slice(starts[0] + 1);
const end = remaining.findIndex((line) => /^#{1,2}\s/u.test(line));
return (end === -1 ? remaining : remaining.slice(0, end)).map((line) =>
line.trimEnd(),
);
}
/** Check the initial declaration against the trusted repository template. */
export function evaluateReadiness(pull, template) {
if (pull.draft)
return result(
"pending",
"Draft PR: complete Author readiness before requesting review.",
);
const expected = readinessLines(template).flatMap((line) => {
const match = /^- \[ \] (.+)$/u.exec(line);
return match ? [match[1]] : [];
});
if (expected.length !== 3)
throw new Error(
"Trusted PR template must contain three readiness declarations.",
);
const lines = readinessLines(pull.body ?? "");
const owners = lines.filter((line) => line.startsWith("Human owner:"));
if (
owners.length !== 1 ||
!/^Human owner: @[a-z\d](?:[a-z\d-]*[a-z\d])?$/iu.test(owners[0])
)
return result(
"failure",
"Fill Human owner with one @GitHub-handle in the Author readiness section.",
);
const declarations = lines.flatMap((line) => {
const match = /^- \[([ xX])\] (.+)$/u.exec(line);
return match
? [{ checked: match[1].toLowerCase() === "x", text: match[2] }]
: [];
});
if (
expected.some((text) => {
const matches = declarations.filter((item) => item.text === text);
return matches.length !== 1 || !matches[0].checked;
})
)
return result(
"failure",
"Complete all three Author readiness declarations from the current PR template.",
);
return result(
"success",
"Human owner and all three initial readiness declarations are filled.",
);
}
/** Publish readiness on the PR head without running contributor code. */
export async function runReadinessCheck({ github, context, core }) {
const number =
context.payload.pull_request?.number ??
Number(context.payload.inputs?.pull_request_number);
if (!Number.isSafeInteger(number) || number <= 0)
throw new Error("A positive pull request number is required.");
const repository = context.repo;
const getPull = async () =>
(await github.rest.pulls.get({ ...repository, pull_number: number })).data;
const pull = await getPull();
if (pull.state !== "open" || !/^(?:main|release\/.+)$/u.test(pull.base.ref))
return;
const publish = (sha, verdict) =>
github.rest.repos.createCommitStatus({
...repository,
sha,
context: "PR readiness",
...verdict,
target_url: `${context.serverUrl}/${repository.owner}/${repository.repo}/actions/runs/${context.runId}`,
});
await publish(
pull.head.sha,
result("pending", "Checking the initial author readiness declaration."),
);
const template = await readFile(
new URL("../../.github/pull_request_template.md", import.meta.url),
"utf8",
);
let verdict = evaluateReadiness(pull, template);
if (verdict.state === "success") {
const associated = await github.paginate(
github.rest.repos.listPullRequestsAssociatedWithCommit,
{ ...repository, commit_sha: pull.head.sha, per_page: 100 },
);
if (
!associated.some(
(other) =>
other.number === number &&
other.state === "open" &&
other.head.sha === pull.head.sha,
)
)
verdict = result(
"pending",
"GitHub has not confirmed this PR's head association; rerun PR readiness.",
);
else if (
associated.some(
(other) =>
other.state === "open" &&
other.number !== number &&
other.head.sha === pull.head.sha,
)
)
verdict = result(
"failure",
"Open PRs must have distinct head commits because GitHub shares commit statuses.",
);
}
const latest = await getPull();
if (latest.state !== "open") return;
if (
latest.head.sha !== pull.head.sha ||
latest.body !== pull.body ||
latest.draft !== pull.draft ||
latest.base.ref !== pull.base.ref
)
verdict = result(
"pending",
"PR changed during the check; wait for the latest run or rerun PR readiness.",
);
await publish(latest.head.sha, verdict);
core.info(verdict.description);
if (verdict.state === "failure") core.setFailed(verdict.description);
return verdict;
}
@@ -1009,6 +1009,10 @@ it.each([
".github/workflows/release.yml",
{ workflows: true, code: true, package: false, website: false },
],
[
".github/workflows/pr-readiness.yml",
{ workflows: true, code: true, package: false, website: false },
],
[
".github/workflows/real-ghidra-windows.yml",
{ workflows: true, code: true, package: false, website: false },
@@ -0,0 +1,382 @@
import { execFile } from "node:child_process";
import { readFile } from "node:fs/promises";
import { fileURLToPath } from "node:url";
import { promisify } from "node:util";
import { describe, expect, it, vi } from "vitest";
import { parse } from "yaml";
import { z } from "zod";
import {
evaluateReadiness,
runReadinessCheck,
type ReadinessGitHub,
type ReadinessPullRequest,
} from "../../../scripts/ci/pr-readiness.mjs";
const template = await readFile(
new URL("../../../.github/pull_request_template.md", import.meta.url),
"utf8",
);
const readyBody = template
.slice(
template.indexOf("## Author readiness"),
template.indexOf("## Before merge"),
)
.replace("Human owner: <!-- @GitHub-handle -->", "Human owner: @contributor")
.replaceAll("- [ ]", "- [x]");
const makePull = (
changes: Partial<ReadinessPullRequest> = {},
): ReadinessPullRequest => ({
number: 42,
state: "open",
draft: false,
body: readyBody,
head: { sha: "a".repeat(40) },
base: { ref: "main" },
...changes,
});
function fixture(changes: Partial<ReadinessPullRequest> = {}) {
const pull = makePull(changes);
const get = vi.fn(async () => ({ data: pull }));
const createCommitStatus = vi.fn(
async (
_status: Parameters<
ReadinessGitHub["rest"]["repos"]["createCommitStatus"]
>[0],
) => ({}),
);
const paginate = vi.fn(async () => [pull]);
const github = {
rest: {
pulls: { get },
repos: {
createCommitStatus,
listPullRequestsAssociatedWithCommit: vi.fn(),
},
},
paginate,
};
const context = {
repo: { owner: "morluto", repo: "rea" },
payload: { pull_request: { number: 42 } },
serverUrl: "https://github.com",
runId: 123,
};
const core = { info: vi.fn(), setFailed: vi.fn() };
return { github, context, core, get, createCommitStatus, paginate, pull };
}
describe("initial author declaration", () => {
it("accepts the filled current template and uppercase checkmarks", () => {
expect(evaluateReadiness(makePull(), template).state).toBe("success");
expect(
evaluateReadiness(
makePull({ body: readyBody.replaceAll("[x]", "[X]") }),
template,
).state,
).toBe("success");
expect(
evaluateReadiness(
makePull({ body: readyBody.replaceAll("\n", "\r\n") }),
template,
).state,
).toBe("success");
});
it("allows drafts without a body or declarations", () => {
expect(
evaluateReadiness(makePull({ draft: true, body: null }), template).state,
).toBe("pending");
});
it.each([0, 1, 2])("requires declaration %i to be checked", (index) => {
let position = 0;
const body = readyBody.replaceAll("- [x]", () =>
position++ === index ? "- [ ]" : "- [x]",
);
expect(evaluateReadiness(makePull({ body }), template).state).toBe(
"failure",
);
});
it.each([
null,
readyBody.replace("Human owner: @contributor", "Human owner:"),
readyBody.replace("Human owner: @contributor", "Human owner: @one @two"),
readyBody.replace(
"Human owner: @contributor",
"Human owner: @automation[bot]",
),
readyBody + "\nHuman owner: @other\n",
readyBody.replace("## Author readiness", "## Other"),
readyBody + readyBody,
`<!--\n${readyBody}\n-->`,
`~~~markdown\n${readyBody}\n~~~`,
`\`\`\`markdown\n${readyBody}\n\`\`\``,
readyBody
.split("\n")
.map((line) => `> ${line}`)
.join("\n"),
readyBody.replace(
"- [x] I declare this PR ready for initial maintainer review.",
"",
),
readyBody +
"\n- [x] I declare this PR ready for initial maintainer review.\n",
])(
"rejects missing, hidden, ambiguous, or incomplete declarations (%#)",
(body) => {
expect(evaluateReadiness(makePull({ body }), template).state).toBe(
"failure",
);
},
);
it("ignores checked examples in comments and fenced blocks before the actual declaration", () => {
for (const example of [
`<!--\n${readyBody}\n-->\n`,
`~~~markdown\n${readyBody}\n~~~\n`,
`\`\`\`markdown\n${readyBody}\n\`\`\`\n`,
])
expect(
evaluateReadiness(makePull({ body: example + readyBody }), template)
.state,
).toBe("success");
});
it("does not treat an HTML comment inside a code example as hiding the real declaration", () => {
const body =
"~~~html\n<!-- an intentionally unfinished HTML example\n~~~\n" +
readyBody;
expect(evaluateReadiness(makePull({ body }), template).state).toBe(
"success",
);
});
it("fails closed when the trusted template loses a declaration", () => {
expect(() =>
evaluateReadiness(
makePull(),
template.replace(
"- [ ] I declare this PR ready for initial maintainer review.",
"",
),
),
).toThrow("three readiness declarations");
});
});
describe("GitHub status boundary", () => {
it("publishes failure on the actual PR head when declarations are unchecked", async () => {
const input = fixture({ body: readyBody.replaceAll("[x]", "[ ]") });
expect((await runReadinessCheck(input))?.state).toBe("failure");
expect(
input.createCommitStatus.mock.calls.map(([status]) => status),
).toEqual([
expect.objectContaining({
sha: input.pull.head.sha,
context: "PR readiness",
state: "pending",
}),
expect.objectContaining({
sha: input.pull.head.sha,
context: "PR readiness",
state: "failure",
}),
]);
expect(input.core.setFailed).toHaveBeenCalledOnce();
});
it("keeps drafts pending without failing the workflow", async () => {
const input = fixture({ draft: true, body: null });
expect((await runReadinessCheck(input))?.state).toBe("pending");
expect(input.core.setFailed).not.toHaveBeenCalled();
expect(input.paginate).not.toHaveBeenCalled();
});
it("keeps the initial declaration valid after later pushes", async () => {
for (const sha of ["a".repeat(40), "b".repeat(40)]) {
const input = fixture({ head: { sha } });
expect((await runReadinessCheck(input))?.state).toBe("success");
expect(input.createCommitStatus).toHaveBeenLastCalledWith(
expect.objectContaining({ sha, state: "success" }),
);
expect(input.github.rest.pulls).toEqual({ get: input.get });
}
});
it("does not publish stale success if the PR changes during the check", async () => {
const input = fixture();
input.get
.mockResolvedValueOnce({ data: input.pull })
.mockResolvedValueOnce({
data: makePull({ head: { sha: "b".repeat(40) }, body: null }),
});
expect((await runReadinessCheck(input))?.state).toBe("pending");
expect(input.createCommitStatus).toHaveBeenLastCalledWith(
expect.objectContaining({ sha: "b".repeat(40), state: "pending" }),
);
});
it("prevents another open PR from lending its status to the same head", async () => {
const input = fixture();
input.paginate.mockResolvedValue([input.pull, makePull({ number: 43 })]);
expect((await runReadinessCheck(input))?.state).toBe("failure");
expect(input.core.setFailed).toHaveBeenCalledWith(
expect.stringContaining("distinct head commits"),
);
});
it("leaves the status pending and surfaces API failure", async () => {
const input = fixture();
input.paginate.mockRejectedValue(new Error("GitHub unavailable"));
await expect(runReadinessCheck(input)).rejects.toThrow(
"GitHub unavailable",
);
expect(input.createCommitStatus).toHaveBeenCalledOnce();
expect(input.createCommitStatus).toHaveBeenCalledWith(
expect.objectContaining({ state: "pending" }),
);
});
it("keeps readiness unknown when GitHub has not associated the head with this PR", async () => {
const input = fixture();
input.paginate.mockResolvedValue([]);
expect((await runReadinessCheck(input))?.state).toBe("pending");
expect(input.core.setFailed).not.toHaveBeenCalled();
});
it.each([{ state: "closed" as const }, { base: { ref: "other-branch" } }])(
"ignores closed PRs and unprotected target branches (%#)",
async (changes) => {
const input = fixture(changes);
await runReadinessCheck(input);
expect(input.createCommitStatus).not.toHaveBeenCalled();
},
);
it("supports manually rechecking an existing PR", async () => {
const input = fixture();
await runReadinessCheck({
...input,
context: {
...input.context,
payload: { inputs: { pull_request_number: "42" } },
},
});
expect(input.get).toHaveBeenCalledWith({
owner: "morluto",
repo: "rea",
pull_number: 42,
});
});
});
const readWorkflow = async () =>
z
.object({
on: z.object({
pull_request_target: z.object({
branches: z.array(z.string()),
types: z.array(z.string()),
}),
workflow_dispatch: z.unknown(),
}),
permissions: z.record(z.string(), z.string()),
jobs: z.object({
readiness: z.object({
permissions: z.record(z.string(), z.string()),
steps: z.array(
z.object({
uses: z.string(),
with: z.object({
ref: z.string().optional(),
"persist-credentials": z.boolean().optional(),
script: z.string().optional(),
}),
}),
),
}),
}),
})
.parse(
parse(
await readFile(
new URL(
"../../../.github/workflows/pr-readiness.yml",
import.meta.url,
),
"utf8",
),
),
);
it("executes the production workflow step with trusted code and status-only write authority", async () => {
const workflow = await readWorkflow();
expect(workflow.on.pull_request_target.branches).toEqual([
"main",
"release/*",
]);
expect(workflow.on.pull_request_target.types).toEqual(
expect.arrayContaining([
"opened",
"edited",
"synchronize",
"reopened",
"ready_for_review",
"converted_to_draft",
]),
);
expect(workflow.permissions).toEqual({});
expect(workflow.jobs.readiness.permissions).toEqual({
contents: "read",
"pull-requests": "read",
statuses: "write",
});
const [checkout, check] = workflow.jobs.readiness.steps;
expect(checkout?.with).toEqual({
ref: "${{ github.event.repository.default_branch }}",
"persist-credentials": false,
});
expect(check?.uses).toMatch(/^actions\/github-script@[a-f\d]{40}$/u);
const input = fixture();
const root = fileURLToPath(new URL("../../../", import.meta.url));
const code = `
import { createRequire } from "node:module";
const require = createRequire(${JSON.stringify(new URL("../../../package.json", import.meta.url).href)});
const pull = ${JSON.stringify(input.pull)};
const statuses = [];
const github = {
rest: {
pulls: { get: async () => ({ data: pull }) },
repos: {
createCommitStatus: async status => statuses.push(status),
listPullRequestsAssociatedWithCommit: () => {},
},
},
paginate: async () => [pull],
};
const context = ${JSON.stringify(input.context)};
const core = {
info: () => {},
setFailed: message => { throw new Error(message); },
};
${z.string().parse(check?.with.script)}
console.log(JSON.stringify(statuses));
`;
const { stdout } = await promisify(execFile)(
process.execPath,
["--input-type=module", "-e", code],
{
cwd: root,
env: { ...process.env, GITHUB_WORKSPACE: root },
encoding: "utf8",
timeout: 10_000,
maxBuffer: 1024 * 1024,
},
);
const statuses = z
.array(z.object({ state: z.string() }))
.parse(JSON.parse(stdout));
expect(statuses.at(-1)?.state).toBe("success");
});