Skip to content
Open
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
11 changes: 6 additions & 5 deletions .github/pr-readiness/README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# PR Readiness Helper

A standalone bot ([`pr-readiness.yaml`](../workflows/pr-readiness.yaml)) that lowers maintainer burden: when CI finishes on a PR, it keeps **one sticky comment** telling the contributor exactly how to fix contributor-fixable problems, moves not-ready PRs to **draft**, and gets out of the way once everything is green.
A standalone bot ([`pr-readiness.yaml`](../workflows/pr-readiness.yaml)) that lowers maintainer burden: when CI finishes on a PR, it keeps **one sticky comment** telling the contributor exactly how to fix contributor-fixable problems, flags not-ready PRs with the **`problem/bot-not-ready`** label, and gets out of the way (clearing the label) once everything is green.

## What it covers

Expand All @@ -25,8 +25,8 @@ Tune guidance, add or remove signals in [`checks.config.json`](checks.config.jso
- Fires on `workflow_run: completed` of CI / Docs / PR Title Check / PR Feature Check. Title and feature checks re-run on PR `edited`, so title/description edits re-evaluate too. `/retest` re-runs CI and therefore re-evaluates.
- **Never posts** on a PR that never had a covered issue.
- While issues exist: one comment listing only the failing items, each with a fix command and a log link. Pending checks are not mentioned.
- Blocking issues (any covered check failure, or a description that doesn't follow the template) also **convert the PR to draft**. The bot **never** marks ready-for-review — that's the contributor's call — and it drafts at most once per head SHA, so a human re-marking it ready is respected until new commits arrive.
- Draft conversion needs a **GitHub App token**: the default Actions token cannot toggle draft state (`Resource not accessible by integration` — verified live). Provision an app with **Pull requests: Read & write** only (do not reuse the cherry-pick app, which can push code), install it on the repo, and set the `PR_READINESS_APP_ID` / `PR_READINESS_APP_PRIVATE_KEY` secrets — the same `actions/create-github-app-token` pattern as `cherry-pick-single.yml`. Without the secrets the bot comments but does not draft.
- Blocking issues (any covered check failure, or a description that doesn't follow the template) also apply the **`problem/bot-not-ready`** label. The bot owns the label outright: it is applied while the verdict is blocking and **removed automatically** the moment it isn't (fix pushed, check re-run green, description fixed, or the description check waived by a maintainer editing it into compliance). No app token or extra secrets: labelling a PR works with the default Actions token under the `pull-requests: write` permission the sticky comment already needs. (An earlier version converted PRs to draft instead — that needed a dedicated GitHub App because the default token cannot toggle draft state, and undrafting semantics around human intent were messy. The label sidesteps both.)
- Because the label mirrors the current verdict with no memory, a manually-removed label is re-applied on the next CI completion while checks still fail — maintainers who disagree with a covered failure can simply review anyway; the label is advisory, not a gate.
- When issues are resolved but other covered checks are still running: the comment shows a short "waiting" state.
- When everything is terminal and green: the comment is edited to a short ✅ all-clear.
- Skipped: PRs by anyone in [`OWNERS`](../../OWNERS) (owners/approvers/reviewers) and by bots.
Expand All @@ -41,19 +41,20 @@ A deterministic check ([`template.ts`](template.ts)) compares the description ag

- `workflow_run` workflows execute the **default branch's** definition with the base-repo token — a fork PR cannot alter what runs here.
- The job **never checks out or executes PR-head code**; the checkout step takes the default branch only. Keep it that way.
- `permissions: {}` at the top; the job grants only `pull-requests: write`, `contents: read`, `actions: read`. No secrets beyond `GITHUB_TOKEN` (and the optional draft-app secrets).
- `permissions: {}` at the top; the job grants only `pull-requests: write`, `contents: read`, `actions: read`. No secrets beyond `GITHUB_TOKEN`.
- `workflow_run.pull_requests` is empty for fork PRs, so the PR is found by matching `head_sha` against open PRs; no match → exit (a newer push superseded the run).
- PR title/body/branch are attacker-controlled: they are only ever handled as data, never interpolated into shell or scripts. The comment never echoes contributor-supplied text — only the bot's own guidance, check titles/URLs, and template section names.
- All actions are pinned to full commit SHAs (enforced by repo lint).

## Dry run

Setting `DRY_RUN: "true"` in the workflow renders the would-be comment and decisions to the job's **step summary** instead of commenting or drafting. Use it to test changes to the bot against real PRs (correct PR resolution, author gating, sensible text) without posting.
Setting `DRY_RUN: "true"` in the workflow renders the would-be comment and decisions to the job's **step summary** instead of commenting or labelling. Use it to test changes to the bot against real PRs (correct PR resolution, author gating, sensible text) without posting.

## Maintenance notes

- **Check renamed?** The signal silently stops matching (fail-safe — no false positives) and any failing unmapped check from a covered app is logged as a warning ("unmapped failing check") so you notice. Update `checks.config.json`.
- **Workflow renamed?** Keep `on.workflow_run.workflows` in `pr-readiness.yaml` in sync with the `name:` fields of `ci-build.yaml`, `docs.yaml`, `pr.yaml`, `pr-feature.yaml`.
- **Label renamed?** `NOT_READY_LABEL` in `comment.ts` must match the repo's `problem/bot-not-ready` label exactly — if the label is renamed in repo settings without updating the constant, the add-labels API quietly *recreates* the old name (with a default colour) rather than failing.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
- **Known limitation:** first-time contributors whose workflows need approval get no help until a maintainer approves the run (nothing completes, so nothing fires).

## Code & local development
Expand Down
13 changes: 5 additions & 8 deletions .github/pr-readiness/classify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,13 @@ export function diagnostics(checkRuns: CheckRun[], config: Config): { unmapped:
interface DecideArgs {
signals: ReadonlyArray<{ id: string; state: string }>;
templateVerdict: { compliant: boolean } | null;
existingState: { draftedSha?: string | null } | null;
hasExistingComment: boolean;
pr: { draft: boolean; headSha: string };
}

// The convergence rules. See README.md for the decision table.
export function decide({ signals, templateVerdict, existingState, hasExistingComment, pr }: DecideArgs): Decision {
// The convergence rules. See README.md for the decision table. `blocking`
// drives the not-ready label: the bot owns it outright, so the label is simply
// applied while blocking and removed once not (main.ts does the sync).
export function decide({ signals, templateVerdict, hasExistingComment }: DecideArgs): Decision {
const failing = signals.filter((s) => s.state === 'failure').map((s) => s.id);
const templateBlocking = Boolean(templateVerdict && templateVerdict.compliant === false);
const blocking = failing.length > 0 || templateBlocking;
Expand All @@ -92,10 +92,7 @@ export function decide({ signals, templateVerdict, existingState, hasExistingCom
shouldComment = true;
}

const alreadyDraftedThisSha = Boolean(existingState && existingState.draftedSha === pr.headSha);
const shouldDraft = blocking && !pr.draft && !alreadyDraftedThisSha;

return { variant, shouldComment, shouldDraft, failing, templateBlocking };
return { variant, shouldComment, blocking, failing, templateBlocking };
}

// OWNERS is a small YAML subset: three keys, each a list of logins.
Expand Down
13 changes: 9 additions & 4 deletions .github/pr-readiness/comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ import type { CommentVariant, State, TemplateIssue } from './types.ts';

export const MARKER = '<!-- pr-readiness-bot -->';

// Must match the label that exists in the repo (Settings → Labels):
// "problem/bot-not-ready — Readiness bot declares this as not ready, see
// comment by bot for why".
export const NOT_READY_LABEL = 'problem/bot-not-ready';

const FOOTER =
'\n---\n<sub>🤖 Automated PR-readiness helper — it re-checks each time CI finishes. ' +
'Unit/E2E test results are <b>not</b> covered here. ' +
Expand All @@ -25,11 +30,11 @@ interface RenderArgs {
variant: CommentVariant | null;
failures: ReadonlyArray<FailureItem>;
templateIssues: TemplateIssue[] | null;
drafted: boolean;
labeled: boolean;
state: State;
}

export function renderComment({ variant, failures, templateIssues, drafted, state }: RenderArgs): string {
export function renderComment({ variant, failures, templateIssues, labeled, state }: RenderArgs): string {
const head = [MARKER, stateLine(state), ''];

if (variant === 'allclear') {
Expand Down Expand Up @@ -81,11 +86,11 @@ export function renderComment({ variant, failures, templateIssues, drafted, stat
lines.push('', '_(A maintainer may waive this.)_', '</details>');
}

if (drafted) {
if (labeled) {
lines.push(
'',
'> [!NOTE]',
'> This PR has been moved to **draft** while the items above are addressed. Mark it **Ready for review** once they are fixed.'
`> This PR carries the \`${NOT_READY_LABEL}\` label while the items above are addressed. It is removed automatically once everything passes.`
);
}

Expand Down
74 changes: 28 additions & 46 deletions .github/pr-readiness/main.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Orchestration for the PR Readiness Helper workflow. A single entry point,
// run(), called from one actions/github-script step in pr-readiness.yaml:
// resolve the PR, gate the author, classify checks, check the description
// against the template, convert to draft if blocking, and render the sticky
// against the template, sync the not-ready label, and render the sticky
// comment (or the dry-run summary). All decision logic lives in the
// unit-tested modules this file imports.

Expand All @@ -10,7 +10,7 @@ import * as path from 'node:path';
import { fileURLToPath } from 'node:url';

import { classifySignals, diagnostics, decide, isExemptAuthor, findPullRequest, pickStepGuidance } from './classify.ts';
import { MARKER, renderComment, parseState } from './comment.ts';
import { MARKER, NOT_READY_LABEL, renderComment } from './comment.ts';
import { checkTemplate } from './template.ts';
import type { Config, JobStep } from './types.ts';

Expand Down Expand Up @@ -41,7 +41,11 @@ interface Octokit {
actions: { getJobForWorkflowRun(params: Record<string, unknown>): Promise<{ data: { steps?: JobStep[] } }> };
pulls: { list: unknown };
checks: { listForRef: unknown };
issues: { listComments: unknown };
issues: {
listComments: unknown;
addLabels(params: Record<string, unknown>): Promise<unknown>;
removeLabel(params: Record<string, unknown>): Promise<unknown>;
};
};
}

Expand Down Expand Up @@ -104,14 +108,13 @@ export async function run({ github, context, core }: { github: Octokit; context:
}
}

// Find our existing sticky comment (if any) and recover its state blob.
// Author check matters: anyone can paste our marker into a comment, but
// only the actions bot's comment may be trusted as state.
// Find our existing sticky comment (if any). Author check matters: anyone
// can paste our marker into a comment, but only the actions bot's comment
// may be trusted as ours.
const comments = await github.paginate(github.rest.issues.listComments, { owner, repo, issue_number: pr.number, per_page: 100 });
const existing = comments.find(
(c) => c.user && c.user.login === 'github-actions[bot]' && typeof c.body === 'string' && c.body.includes(MARKER)
);
const existingState = existing ? parseState(existing.body) : null;

// Deterministic PR-description / template check (no model required).
const template = fs.readFileSync('.github/pull_request_template.md', 'utf8');
Expand All @@ -120,55 +123,34 @@ export async function run({ github, context, core }: { github: Octokit; context:
const decision = decide({
signals,
templateVerdict,
existingState,
hasExistingComment: Boolean(existing),
pr: { draft: pr.draft, headSha },
});

// Draft conversion: at most once per head SHA; undrafting is human-only.
// The default Actions token cannot toggle draft state ("Resource not
// accessible by integration"), so this requires the app token minted by
// the workflow. Best-effort: failure never blocks the comment.
let draftedNow = false;
if (decision.shouldDraft && !dryRun) {
const token = process.env.DRAFT_TOKEN;
if (!token) {
core.warning(
`PR #${pr.number} should be drafted, but no draft token is available ` +
'(PR_READINESS_APP_ID / PR_READINESS_APP_PRIVATE_KEY secrets not configured?)'
);
} else {
try {
const res = await fetch('https://api.github.com/graphql', {
method: 'POST',
headers: { authorization: `bearer ${token}`, 'content-type': 'application/json' },
body: JSON.stringify({
query: 'mutation($id: ID!) { convertPullRequestToDraft(input: {pullRequestId: $id}) { pullRequest { isDraft } } }',
variables: { id: pr.node_id },
}),
});
const result = (await res.json()) as { errors?: Array<{ message: string }> };
if (!res.ok || result.errors) {
throw new Error(result.errors ? result.errors.map((e) => e.message).join('; ') : `HTTP ${res.status}`);
}
draftedNow = true;
} catch (e) {
core.warning(`could not convert PR #${pr.number} to draft: ${errMessage(e)}`);
// Sync the not-ready label to the verdict: the bot owns the label, so it is
// applied while blocking and removed once not. Best-effort: a label API
// failure never blocks the comment.
const hadLabel = Array.isArray(pr.labels) && pr.labels.some((l: { name?: string }) => l.name === NOT_READY_LABEL);
let labeled = dryRun ? decision.blocking : hadLabel; // dry run previews the would-be state
if (!dryRun && decision.blocking !== hadLabel) {
try {
if (decision.blocking) {
await github.rest.issues.addLabels({ owner, repo, issue_number: pr.number, labels: [NOT_READY_LABEL] });
} else {
await github.rest.issues.removeLabel({ owner, repo, issue_number: pr.number, name: NOT_READY_LABEL });
}
labeled = decision.blocking;
} catch (e) {
core.warning(`could not ${decision.blocking ? 'add' : 'remove'} label '${NOT_READY_LABEL}' on PR #${pr.number}: ${errMessage(e)}`);
}
}

const state = {
v: 1,
failing: decision.failing,
draftedSha: draftedNow ? headSha : (existingState && existingState.draftedSha) || null,
};
const state = { v: 1, failing: decision.failing };

const commentBody = renderComment({
variant: decision.variant,
failures: signals.filter((s) => decision.failing.includes(s.id)),
templateIssues: decision.templateBlocking ? templateVerdict.issues : null,
drafted: draftedNow,
labeled,
state,
});

Expand All @@ -180,7 +162,7 @@ export async function run({ github, context, core }: { github: Octokit; context:
`PR #${pr.number} by ${pr.user.login} head=${headSha} | signals: ` +
signals.map((s) => `${s.id}=${s.state}`).join(' ') +
` | template=${templateVerdict.compliant ? 'ok' : 'issues'}` +
` | comment=${decision.shouldComment} variant=${decision.variant || 'n/a'} draft=${decision.shouldDraft} draftedNow=${draftedNow}`
` | comment=${decision.shouldComment} variant=${decision.variant || 'n/a'} blocking=${decision.blocking} label: ${hadLabel} -> ${labeled}`
);
if (decision.shouldComment) {
core.startGroup('rendered comment');
Expand All @@ -192,7 +174,7 @@ export async function run({ github, context, core }: { github: Octokit; context:
core.summary
.addHeading('PR Readiness Helper — dry run', 3)
.addRaw(`PR: #${pr.number} · head: \`${headSha}\` · would comment: **${decision.shouldComment}**` +
` (variant: ${decision.variant || 'n/a'}) · would draft: **${decision.shouldDraft}**\n\n`)
` (variant: ${decision.variant || 'n/a'}) · label \`${NOT_READY_LABEL}\`: ${hadLabel} → would be ${decision.blocking}\n\n`)
.addRaw(decision.shouldComment ? '#### Rendered comment\n\n' + commentBody + '\n' : '')
.addTable([
[{ data: 'signal', header: true }, { data: 'state', header: true }],
Expand Down
Loading