From 868451ba4932573426bf1d017de318b4bd647eb9 Mon Sep 17 00:00:00 2001 From: vemboy20 Date: Wed, 11 Nov 2026 09:19:38 -0800 Subject: [PATCH 1/2] feat(github): require the PR template sections on core Adds a generic pr-template rule that fails when the PR description drops one of the configured template sections or leaves a required one empty. Enabled on core for Proposed change, Type of change and Checklist. Co-Authored-By: Claude Opus 5.5 --- .../manifests/home-assistant-core/index.ts | 7 ++ src/github/rules/pr-template.ts | 116 ++++++++++++++++++ .../pull_request.edited.body-updated.yaml | 4 + .../pull_request.opened.all-change-types.yaml | 4 + ...uest.opened.bugfix-single-integration.yaml | 4 + .../pull_request.opened.dependency-bump.yaml | 4 + ...equest.opened.new-integration-no-docs.yaml | 4 + ...est.opened.new-integration-transports.yaml | 4 + .../pull_request.opened.release.yaml | 4 + .../pull_request.reopened.yaml | 4 + ...ll_request.synchronize.merge-conflict.yaml | 4 + tests/github/rules/pr-template.test.ts | 113 +++++++++++++++++ 12 files changed, 272 insertions(+) create mode 100644 src/github/rules/pr-template.ts create mode 100644 tests/github/rules/pr-template.test.ts diff --git a/src/github/manifests/home-assistant-core/index.ts b/src/github/manifests/home-assistant-core/index.ts index 0b64089..c170d6e 100644 --- a/src/github/manifests/home-assistant-core/index.ts +++ b/src/github/manifests/home-assistant-core/index.ts @@ -14,6 +14,7 @@ import { mentionCodeOwners } from "../../rules/code-owner-mention.js"; import { forbiddenFiles } from "../../rules/forbidden-files.js"; import { mergeConflict } from "../../rules/merge-conflict.js"; import { mergeTarget } from "../../rules/merge-target.js"; +import { prTemplate } from "../../rules/pr-template.js"; import { setIntegration } from "../../rules/set-integration.js"; import { HomeAssistantRepository, @@ -23,6 +24,7 @@ import { import type { RepoManifest } from "../types.js"; import { coreForbiddenFiles } from "./forbidden-files.js"; import { itemIntegrationDomains } from "./helpers/integration-domains.js"; +import { isReleasePR } from "./helpers/release-pr.js"; import { coreIssueContext } from "./issue-context.js"; import { changeType } from "./rules/change-type.js"; import { dependencyBump } from "./rules/dependency-bump.js"; @@ -81,6 +83,11 @@ export const homeAssistantCore: RepoManifest = { platinumApproval, // Classification & process + prTemplate({ + sections: ["Proposed change", "Type of change", "Checklist"], + requireContent: ["Proposed change"], + skip: async (pr) => ((await isReleasePR(pr)) ? "Not checked on release PRs." : undefined), + }), changeType, fileShape, dependencyBump, diff --git a/src/github/rules/pr-template.ts b/src/github/rules/pr-template.ts new file mode 100644 index 0000000..a8bccc8 --- /dev/null +++ b/src/github/rules/pr-template.ts @@ -0,0 +1,116 @@ +import { EventType } from "../engine/event.js"; +import type { PullRequest } from "../engine/model/pull-request.js"; +import type { RuleContext } from "../engine/model/rule-context.js"; +import { type CheckOutcome, check } from "../engine/rule.js"; +import type { Rule } from "../engine/types.js"; + +type HandledEvent = + | EventType.PULL_REQUEST_OPENED + | EventType.PULL_REQUEST_REOPENED + | EventType.PULL_REQUEST_EDITED + | EventType.PULL_REQUEST_SYNCHRONIZE + | EventType.ON_DEMAND; + +const COMMENT_PATTERN = //g; +const HEADING_PATTERN = /^#{1,6}\s+(.+?)\s*#*\s*$/; +const FENCE_PATTERN = /^\s*(```|~~~)/; + +/** Case, surrounding whitespace and a trailing colon don't make a heading different. */ +function normalize(heading: string): string { + return heading.trim().replace(/:$/, "").trim().toLowerCase(); +} + +/** + * Splits a PR body into its headed sections, keyed by normalized heading. + * Template comments are stripped first: they hold the template's own + * instructions, so a section with only its comment left is still empty. + * Lines inside fenced code blocks never start a section. + */ +export function parseSections(body: string | null): Map { + const sections = new Map(); + if (!body) return sections; + + let current: string | undefined; + let content: string[] = []; + let inFence = false; + const flush = () => { + if (current !== undefined && !sections.has(current)) { + sections.set(current, content.join("\n").trim()); + } + }; + + for (const line of body.replace(COMMENT_PATTERN, "").split(/\r?\n/)) { + if (FENCE_PATTERN.test(line)) inFence = !inFence; + const heading = inFence ? null : HEADING_PATTERN.exec(line); + if (heading) { + flush(); + current = normalize(heading[1]); + content = []; + } else { + content.push(line); + } + } + flush(); + return sections; +} + +export function prTemplate(config: { + /** Headings the PR description must keep, as written in the repo's template. */ + sections: string[]; + /** Headings (a subset of `sections`) that must also hold the contributor's own text. */ + requireContent?: string[]; + /** Repo policy for PRs that don't use the template; return the skip reason. */ + skip?: (pr: PullRequest) => Promise; +}): Rule { + async function evaluate(ctx: RuleContext): Promise { + // allowBots stops bot-sent events; this also covers a human editing a bot's PR. + if ((await ctx.target.authorLogin()).endsWith("[bot]")) { + return { status: "skip", message: "Not checked on PRs opened by bots." }; + } + const reason = await config.skip?.(ctx.target); + if (reason) return { status: "skip", message: reason }; + + const sections = parseSections(await ctx.target.body()); + const issues: string[] = []; + for (const heading of config.sections) { + const content = sections.get(normalize(heading)); + if (content === undefined) { + issues.push(`The **${heading}** section is missing.`); + } else if (config.requireContent?.includes(heading) && content === "") { + issues.push(`The **${heading}** section is empty.`); + } + } + + if (issues.length === 0) { + return { status: "pass", message: "The PR description follows the template." }; + } + return { + status: "fail", + message: [ + "Please fill in the PR description using the template:", + ...issues.map((i) => `- ${i}`), + ].join("\n"), + }; + } + + return check({ + id: "pr-template", + title: "PR description", + description: + `Requires the PR description to keep the template's ${config.sections + .map((s) => `**${s}**`) + .join(", ")} sections` + + (config.requireContent?.length + ? `, with ${config.requireContent.map((s) => `**${s}**`).join(", ")} filled in.` + : "."), + allowBots: false, + events: [ + EventType.PULL_REQUEST_OPENED, + EventType.PULL_REQUEST_REOPENED, + EventType.PULL_REQUEST_EDITED, + EventType.PULL_REQUEST_SYNCHRONIZE, + EventType.ON_DEMAND, + ], + evaluate, + }); +} diff --git a/tests/github/manifests/snapshots/home-assistant-core/pull_request.edited.body-updated.yaml b/tests/github/manifests/snapshots/home-assistant-core/pull_request.edited.body-updated.yaml index b64b7fb..dc384c3 100644 --- a/tests/github/manifests/snapshots/home-assistant-core/pull_request.edited.body-updated.yaml +++ b/tests/github/manifests/snapshots/home-assistant-core/pull_request.edited.body-updated.yaml @@ -31,6 +31,10 @@ statuses: title: Code owner approval status: pending message: Platinum `hue` — needs approval from a code owner before merging. + - id: pr-template + title: PR description + status: pass + message: The PR description follows the template. - id: type-of-change title: Type of change status: pass diff --git a/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.all-change-types.yaml b/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.all-change-types.yaml index e9bf90b..55951ce 100644 --- a/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.all-change-types.yaml +++ b/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.all-change-types.yaml @@ -35,6 +35,10 @@ statuses: title: Code owner approval status: skip message: Touches no platinum integration — no code-owner approval required. + - id: pr-template + title: PR description + status: pass + message: The PR description follows the template. - id: type-of-change title: Type of change status: pass diff --git a/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.bugfix-single-integration.yaml b/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.bugfix-single-integration.yaml index b635d4a..cbda33b 100644 --- a/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.bugfix-single-integration.yaml +++ b/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.bugfix-single-integration.yaml @@ -35,6 +35,10 @@ statuses: title: Code owner approval status: pending message: Platinum `hue` — needs approval from a code owner before merging. + - id: pr-template + title: PR description + status: pass + message: The PR description follows the template. - id: type-of-change title: Type of change status: pass diff --git a/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.dependency-bump.yaml b/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.dependency-bump.yaml index 028465f..fadfee2 100644 --- a/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.dependency-bump.yaml +++ b/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.dependency-bump.yaml @@ -35,6 +35,10 @@ statuses: title: Code owner approval status: skip message: Touches no platinum integration — no code-owner approval required. + - id: pr-template + title: PR description + status: pass + message: The PR description follows the template. - id: type-of-change title: Type of change status: pass diff --git a/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.new-integration-no-docs.yaml b/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.new-integration-no-docs.yaml index 9bd8896..2c2f2bd 100644 --- a/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.new-integration-no-docs.yaml +++ b/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.new-integration-no-docs.yaml @@ -35,6 +35,10 @@ statuses: title: Code owner approval status: skip message: Touches no platinum integration — no code-owner approval required. + - id: pr-template + title: PR description + status: pass + message: The PR description follows the template. - id: type-of-change title: Type of change status: pass diff --git a/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.new-integration-transports.yaml b/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.new-integration-transports.yaml index fb69f95..e907fc6 100644 --- a/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.new-integration-transports.yaml +++ b/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.new-integration-transports.yaml @@ -35,6 +35,10 @@ statuses: title: Code owner approval status: skip message: Touches no platinum integration — no code-owner approval required. + - id: pr-template + title: PR description + status: pass + message: The PR description follows the template. - id: type-of-change title: Type of change status: pass diff --git a/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.release.yaml b/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.release.yaml index 86d8b8e..886f376 100644 --- a/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.release.yaml +++ b/tests/github/manifests/snapshots/home-assistant-core/pull_request.opened.release.yaml @@ -35,6 +35,10 @@ statuses: title: Code owner approval status: skip message: Touches no platinum integration — no code-owner approval required. + - id: pr-template + title: PR description + status: skip + message: Not checked on release PRs. - id: type-of-change title: Type of change status: skip diff --git a/tests/github/manifests/snapshots/home-assistant-core/pull_request.reopened.yaml b/tests/github/manifests/snapshots/home-assistant-core/pull_request.reopened.yaml index af4677a..40db2f7 100644 --- a/tests/github/manifests/snapshots/home-assistant-core/pull_request.reopened.yaml +++ b/tests/github/manifests/snapshots/home-assistant-core/pull_request.reopened.yaml @@ -31,6 +31,10 @@ statuses: title: Code owner approval status: pending message: Platinum `hue` — needs approval from a code owner before merging. + - id: pr-template + title: PR description + status: pass + message: The PR description follows the template. effects: - type: addLabels labels: diff --git a/tests/github/manifests/snapshots/home-assistant-core/pull_request.synchronize.merge-conflict.yaml b/tests/github/manifests/snapshots/home-assistant-core/pull_request.synchronize.merge-conflict.yaml index c0baaa3..106a47e 100644 --- a/tests/github/manifests/snapshots/home-assistant-core/pull_request.synchronize.merge-conflict.yaml +++ b/tests/github/manifests/snapshots/home-assistant-core/pull_request.synchronize.merge-conflict.yaml @@ -27,6 +27,10 @@ statuses: title: Code owner approval status: pending message: Platinum `hue` — needs approval from a code owner before merging. + - id: pr-template + title: PR description + status: pass + message: The PR description follows the template. - id: type-of-change title: Type of change status: pass diff --git a/tests/github/rules/pr-template.test.ts b/tests/github/rules/pr-template.test.ts new file mode 100644 index 0000000..31de700 --- /dev/null +++ b/tests/github/rules/pr-template.test.ts @@ -0,0 +1,113 @@ +import { describe, expect, it } from "vitest"; +import { EventType } from "../../../src/github/engine/event.js"; +import { parseSections, prTemplate } from "../../../src/github/rules/pr-template.js"; +import { createMockContext, runRule } from "../helpers/mock-context.js"; +import { loadPRTemplate, renderPRTemplate } from "../manifests/pr-template.js"; + +const CORE_TEMPLATE = loadPRTemplate("home-assistant-core"); + +const rule = prTemplate({ + sections: ["Proposed change", "Type of change", "Checklist"], + requireContent: ["Proposed change"], + skip: async (pr) => + (await pr.baseRef()) === "master" ? "Not checked on release PRs." : undefined, +}); + +function filledCoreBody(): string { + return renderPRTemplate(CORE_TEMPLATE, { + check: ["Bugfix (non-breaking change which fixes an issue)"], + sections: { "Proposed change": "Fix the thing that was broken." }, + }); +} + +function run(pull_request: Record) { + const context = createMockContext({ + eventType: EventType.PULL_REQUEST_OPENED, + payload: { pull_request }, + }); + return runRule(rule, context); +} + +describe("pr-template", () => { + it("passes a filled-in core template", async () => { + const result = await run({ body: filledCoreBody() }); + expect(result?.section).toMatchObject({ id: "pr-template", status: "pass" }); + }); + + it("fails and names a deleted section", async () => { + const body = filledCoreBody().replace("## Checklist", ""); + const result = await run({ body }); + expect(result?.section?.status).toBe("fail"); + expect(result?.section?.message).toContain("The **Checklist** section is missing."); + expect(result?.section?.message).not.toContain("Type of change"); + }); + + it("fails when Proposed change only holds the template comment", async () => { + const result = await run({ body: CORE_TEMPLATE }); + expect(result?.section?.status).toBe("fail"); + expect(result?.section?.message).toContain("The **Proposed change** section is empty."); + }); + + it("lists every problem at once", async () => { + const result = await run({ body: "Just a one-line description." }); + expect(result?.section?.status).toBe("fail"); + for (const heading of ["Proposed change", "Type of change", "Checklist"]) { + expect(result?.section?.message).toContain(`The **${heading}** section is missing.`); + } + }); + + it("fails on an empty body", async () => { + const result = await run({ body: null }); + expect(result?.section?.status).toBe("fail"); + }); + + it("does not require content in sections outside requireContent", async () => { + const body = renderPRTemplate(CORE_TEMPLATE, { + sections: { "Proposed change": "Something useful." }, + }); + const result = await run({ body }); + expect(result?.section?.status).toBe("pass"); + }); + + it("skips release PRs through the repo's skip hook", async () => { + const result = await run({ body: "Release notes", base: { ref: "master" } }); + expect(result?.section).toMatchObject({ + status: "skip", + message: "Not checked on release PRs.", + }); + }); + + it("skips PRs authored by bots", async () => { + const result = await run({ body: "Bumps foo from 1 to 2", user: { login: "dependabot[bot]" } }); + expect(result?.section?.status).toBe("skip"); + }); + + it("does not run on bot-sent events", () => { + expect(rule.allowBots).toBe(false); + }); +}); + +describe("parseSections", () => { + it("matches headings regardless of case, level and trailing colon", () => { + const sections = parseSections("# PROPOSED CHANGE:\nhello\n### checklist\n- [x] done"); + expect(sections.get("proposed change")).toBe("hello"); + expect(sections.get("checklist")).toBe("- [x] done"); + }); + + it("ignores headings inside template comments", () => { + const sections = parseSections("\n## Shown\ntext"); + expect(sections.has("hidden")).toBe(false); + expect(sections.get("shown")).toBe("text"); + }); + + it("ignores headings inside fenced code blocks", () => { + const sections = parseSections("## Proposed change\n```yaml\n# Example config\n```"); + expect(sections.has("example config")).toBe(false); + expect(sections.get("proposed change")).toContain("# Example config"); + }); + + it("keeps the first section when a heading repeats", () => { + const sections = parseSections("## Checklist\nfirst\n## Checklist\nsecond"); + expect(sections.get("checklist")).toBe("first"); + }); +}); From d8174bdea7c6ff397bc4b4ac6835f2df7e2ad4ef Mon Sep 17 00:00:00 2001 From: vemboy20 Date: Wed, 11 Nov 2026 10:39:15 -0800 Subject: [PATCH 2/2] fix(github): count subheading text as part of its template section A `### Details` heading inside `## Proposed change` ended the section, so a filled-in description read as empty. Sections now run until the next heading of the same or a higher level. Co-Authored-By: Claude Opus 5.5 --- src/github/rules/pr-template.ts | 45 ++++++++++++++------------ tests/github/rules/pr-template.test.ts | 26 ++++++++++++++- 2 files changed, 50 insertions(+), 21 deletions(-) diff --git a/src/github/rules/pr-template.ts b/src/github/rules/pr-template.ts index a8bccc8..94e7be8 100644 --- a/src/github/rules/pr-template.ts +++ b/src/github/rules/pr-template.ts @@ -12,7 +12,7 @@ type HandledEvent = | EventType.ON_DEMAND; const COMMENT_PATTERN = //g; -const HEADING_PATTERN = /^#{1,6}\s+(.+?)\s*#*\s*$/; +const HEADING_PATTERN = /^(#{1,6})\s+(.+?)\s*#*\s*$/; const FENCE_PATTERN = /^\s*(```|~~~)/; /** Case, surrounding whitespace and a trailing colon don't make a heading different. */ @@ -20,37 +20,42 @@ function normalize(heading: string): string { return heading.trim().replace(/:$/, "").trim().toLowerCase(); } +type Line = { text: string; heading?: { level: number; name: string } }; + /** * Splits a PR body into its headed sections, keyed by normalized heading. * Template comments are stripped first: they hold the template's own * instructions, so a section with only its comment left is still empty. - * Lines inside fenced code blocks never start a section. + * A section runs until the next heading of the same or a higher level, so + * text under a subheading (`### Details` inside `## Proposed change`) still + * counts as the parent's content; the subheading lines themselves don't. + * Lines inside fenced code blocks are never headings. */ export function parseSections(body: string | null): Map { const sections = new Map(); if (!body) return sections; - let current: string | undefined; - let content: string[] = []; let inFence = false; - const flush = () => { - if (current !== undefined && !sections.has(current)) { - sections.set(current, content.join("\n").trim()); - } - }; + const lines: Line[] = body + .replace(COMMENT_PATTERN, "") + .split(/\r?\n/) + .map((text) => { + if (FENCE_PATTERN.test(text)) inFence = !inFence; + const match = inFence ? null : HEADING_PATTERN.exec(text); + return match + ? { text, heading: { level: match[1].length, name: normalize(match[2]) } } + : { text }; + }); - for (const line of body.replace(COMMENT_PATTERN, "").split(/\r?\n/)) { - if (FENCE_PATTERN.test(line)) inFence = !inFence; - const heading = inFence ? null : HEADING_PATTERN.exec(line); - if (heading) { - flush(); - current = normalize(heading[1]); - content = []; - } else { - content.push(line); + lines.forEach(({ heading }, start) => { + if (!heading || sections.has(heading.name)) return; + const content: string[] = []; + for (const line of lines.slice(start + 1)) { + if (line.heading && line.heading.level <= heading.level) break; + if (!line.heading) content.push(line.text); } - } - flush(); + sections.set(heading.name, content.join("\n").trim()); + }); return sections; } diff --git a/tests/github/rules/pr-template.test.ts b/tests/github/rules/pr-template.test.ts index 31de700..952e57e 100644 --- a/tests/github/rules/pr-template.test.ts +++ b/tests/github/rules/pr-template.test.ts @@ -48,6 +48,15 @@ describe("pr-template", () => { expect(result?.section?.message).toContain("The **Proposed change** section is empty."); }); + it("passes when Proposed change is written under a subheading", async () => { + const body = filledCoreBody().replace( + "Fix the thing that was broken.", + "### Details\nFix the thing that was broken.", + ); + const result = await run({ body }); + expect(result?.section?.status).toBe("pass"); + }); + it("lists every problem at once", async () => { const result = await run({ body: "Just a one-line description." }); expect(result?.section?.status).toBe("fail"); @@ -89,9 +98,24 @@ describe("pr-template", () => { describe("parseSections", () => { it("matches headings regardless of case, level and trailing colon", () => { - const sections = parseSections("# PROPOSED CHANGE:\nhello\n### checklist\n- [x] done"); + const sections = parseSections("# PROPOSED CHANGE:\nhello\n# checklist\n- [x] done"); expect(sections.get("proposed change")).toBe("hello"); expect(sections.get("checklist")).toBe("- [x] done"); + expect(parseSections("### Checklist").has("checklist")).toBe(true); + }); + + it("counts text under a subheading as the parent section's content", () => { + const sections = parseSections( + "## Proposed change\n### Details\nthe change\n## Type of change\n- [x] Bugfix", + ); + expect(sections.get("proposed change")).toBe("the change"); + expect(sections.get("details")).toBe("the change"); + expect(sections.get("type of change")).toBe("- [x] Bugfix"); + }); + + it("does not count a bare subheading as content", () => { + const sections = parseSections("## Proposed change\n### Details\n\n## Checklist"); + expect(sections.get("proposed change")).toBe(""); }); it("ignores headings inside template comments", () => {