From e081a8146892582ad9be66ea38bab10aef76a44e Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 19 Oct 2026 11:47:57 -0700 Subject: [PATCH] feat(triage): add a built-in conventional commits rule Closes CL-10919 --- apps/web/src/lib/triage-view.ts | 1 + .../Corbits Triage User Manual.md | 1 + docs/PRODUCT.md | 9 ++-- packages/triage-contracts/src/check-pack.ts | 28 ++++++++++++ .../triage-contracts/src/policy-export.ts | 10 ++++- packages/triage-contracts/src/policy-rules.ts | 5 ++- packages/triage-contracts/src/rule-prose.ts | 11 ++++- .../triage-workflows/src/logic/checks.test.ts | 28 +++++++++++- packages/triage-workflows/src/logic/checks.ts | 38 ++++++++++++++++ .../triage-workflows/src/pr-triage.test.ts | 19 ++++++-- skills/corbits-triage-setup/reference.md | 44 ++----------------- 11 files changed, 143 insertions(+), 51 deletions(-) diff --git a/apps/web/src/lib/triage-view.ts b/apps/web/src/lib/triage-view.ts index 30b9c903b..d74d2fcdb 100644 --- a/apps/web/src/lib/triage-view.ts +++ b/apps/web/src/lib/triage-view.ts @@ -31,6 +31,7 @@ const FACTS: Record string> = { hasConflicts: (value) => (value ? "has conflicts" : "no conflicts"), touchesForbiddenPaths: (value) => (value ? "touches forbidden paths" : "no forbidden paths"), linkedIssue: (value) => (value ? "issue linked" : "no issue linked"), + conventional: (value) => (value ? "conventional commits" : "not conventional commits"), files: (value) => counted(value, "file"), lines: (value) => counted(value, "line"), approvals: (value) => counted(value, "approval"), diff --git a/docs/Corbits Triage User Manual/Corbits Triage User Manual.md b/docs/Corbits Triage User Manual/Corbits Triage User Manual.md index 05e8cd565..c3198042b 100644 --- a/docs/Corbits Triage User Manual/Corbits Triage User Manual.md +++ b/docs/Corbits Triage User Manual/Corbits Triage User Manual.md @@ -216,6 +216,7 @@ A rule asks one question about a pull request and answers pass, fail or unconfir - **Flag merge conflicts with the base branch.** - **CI must pass before merge.** The repository's automated checks must be green. While they are still running, the rule is unconfirmed rather than passed. - **Never accept changes to** certain paths, such as vendored code. + - **Require a conventional commit title and messages.** The title and the first line of each commit must start with a type, such as `feat: add search` or `fix(api)!: drop v1`. The types are feat, fix, refactor, perf, docs, test, chore, ci, build, style and revert unless the file lists its own, and the file can check only the title or only the commits. Case does not matter. Merges and the `Revert "…"` commits GitHub's revert button makes are skipped, and only the first 100 commits are read. When it fails, Triage names each title or commit that doesn't follow the format. - **Fail when …**, a text rule of your own, such as "Fail when any commit message contains "Co-Authored-By:"". It names where to look (the title, the description, any commit message, or several of them), whether the rule fails when the text **contains** or **does not contain** its phrases, and the phrases. Case does not matter. Only the first 100 commits are read. The rule's evidence quotes the line that matched. Text rules count toward the eight rules of your own. - **Judgement**, questions the decision model answers from the diff and the documentation: - **Check the change is focused on one thing.** On a stacked branch, one another pull request was merged into or one based on another open pull request's branch, this rule is skipped, and the comment says why, such as "stacked branch, includes #1521". diff --git a/docs/PRODUCT.md b/docs/PRODUCT.md index ff333f7d8..a314c8d53 100644 --- a/docs/PRODUCT.md +++ b/docs/PRODUCT.md @@ -63,7 +63,9 @@ The same pull request and policy give the same result. A rule is unconfirmed only while the host has not computed what it needs, such as mergeability or CI. Built in: Max files changed, Max lines changed, Linked issue (GitHub, Linear or -either), No merge conflicts, CI green, No forbidden paths. Two always run: +either), No merge conflicts, CI green, No forbidden paths, Conventional commits +(the title and each of the first 100 commits' first line start +`type(scope)!: `, in any case; merges and reverts skipped). Two always run: Not a draft, and No changes requested (a requested change fails and waits on the author, then on the reviewer once the author pushes or replies). The recommended forbidden paths are `vendor/**`, `node_modules/**` and @@ -242,8 +244,9 @@ rule's id, `model` for a yes/no question about the whole pull request, or `text` for a phrase match) and why it exists: a one-line `reason` and its `evidence` (`#44`, `owner/name#44`, links or `maintainer-stated`). All four are required. A model or text rule names its own `id`. A built-in rule takes -its setting as `max` (size limits), `tracker` (linked issue) or `paths` -(forbidden paths); `enabled: false` keeps it written down but off. +its setting as `max` (size limits), `tracker` (linked issue), `paths` +(forbidden paths) or `types` and `fields` (conventional commits); +`enabled: false` keeps it written down but off. ```yaml rules: diff --git a/packages/triage-contracts/src/check-pack.ts b/packages/triage-contracts/src/check-pack.ts index 896e2ca1c..1dce37772 100644 --- a/packages/triage-contracts/src/check-pack.ts +++ b/packages/triage-contracts/src/check-pack.ts @@ -20,6 +20,7 @@ export const CATALOG_IDS = [ "no-merge-conflicts", "ci-green", "no-forbidden-paths", + "conventional-commits", ] as const; export type CatalogId = (typeof CATALOG_IDS)[number]; @@ -38,6 +39,7 @@ export const CHECK_CATALOG: Record = { id: "no-attribution-trailers", name: "No attribution trailers", @@ -121,6 +128,9 @@ export type CatalogCheck = RuleNote & { maxLines?: number; tracker?: IssueTracker; forbiddenGlobs?: string[]; + /** Conventional commits: the types allowed, and whether the title, the commits or both are read; absent is every default type, and both. */ + types?: string[]; + fields?: ConventionalField[]; }; export const MODEL_SHAPES = ["is-true", "score", "choose"] as const; @@ -366,6 +376,8 @@ const CatalogCheckSchema = type({ "maxLines?": "number", "tracker?": type.enumerated(...ISSUE_TRACKERS), "forbiddenGlobs?": "string[]", + "types?": "string[]", + "fields?": type.enumerated(...CONVENTIONAL_FIELDS).array(), }); export const CustomId = type(CUSTOM_ID).atMostLength(CUSTOM_ID_MAX); @@ -547,9 +559,25 @@ function parseCatalogCheck(id: CatalogId, raw: unknown): CatalogCheck | undefine const globs = parseGlobs(row.forbiddenGlobs, "No forbidden paths globs"); if (globs) check.forbiddenGlobs = globs; } + if (id === "conventional-commits") { + if (row.types !== undefined) check.types = commitTypes(row.types); + if (row.fields !== undefined) check.fields = conventionalFields(row.fields); + } return check; } +function commitTypes(value: unknown): string[] { + const types = texts(value, "Conventional commits types"); + const bad = types.find((name) => !CONVENTIONAL_TYPE.test(name)); + if (bad !== undefined) throw new Error(`Conventional commits types must be single words, not ${bad}.`); + return types; +} + +function conventionalFields(value: unknown): ConventionalField[] { + const fields = new Set(list(value, "Conventional commits fields").map((field) => oneOf(field, CONVENTIONAL_FIELDS, "Conventional commits fields"))); + return CONVENTIONAL_FIELDS.filter((field) => fields.has(field)); +} + /** An old size row that set only one limit never measured the other, so that half loads switched off. */ function sizeLimitMissing(id: string, raw: unknown): boolean { const row = asRecord(raw); diff --git a/packages/triage-contracts/src/policy-export.ts b/packages/triage-contracts/src/policy-export.ts index c1daecfff..5feadfaa4 100644 --- a/packages/triage-contracts/src/policy-export.ts +++ b/packages/triage-contracts/src/policy-export.ts @@ -4,6 +4,8 @@ import { dump, JSON_SCHEMA } from "js-yaml"; import { AUTHOR_GROUPS, CATALOG_IDS, + CONVENTIONAL_FIELDS, + CONVENTIONAL_TYPES, POLICY_EVENTS, VERDICT_LABELS, VERDICTS, @@ -44,6 +46,8 @@ function catalogParams(id: CatalogId, row: CatalogCheck): Record { @@ -180,6 +185,7 @@ const CATALOG_RULES: Partial> = { "max-lines-changed": "Most lines changed", "linked-issue": "Require a linked issue from", "no-forbidden-paths": "Never accept changes to", + "conventional-commits": "Conventional commits", }; // CI green has three states: required, absent (merges still wait for CI) and written off (merges skip it). @@ -199,6 +205,8 @@ function catalogValues(pack: CheckPack, id: CatalogId): string[] { return [ISSUE_TRACKER_NAMES[row.tracker && row.tracker !== "off" ? row.tracker : "either"]]; case "no-forbidden-paths": return row.forbiddenGlobs ?? []; + case "conventional-commits": + return [catalogSentence(id, pack)]; default: return ["on"]; } diff --git a/packages/triage-contracts/src/policy-rules.ts b/packages/triage-contracts/src/policy-rules.ts index ad7d5a243..f9c46eb77 100644 --- a/packages/triage-contracts/src/policy-rules.ts +++ b/packages/triage-contracts/src/policy-rules.ts @@ -3,7 +3,7 @@ // the stored policy's reader stays the one authority on what a policy may hold. import { type } from "arktype"; import { asRecord } from "@corbits/common"; -import { CATALOG_IDS, CustomId, Evidence, ISSUE_TRACKERS, Label, POLICY_EVENTS, stepListSchema, Text, TEXT_FIELDS, TEXT_MATCHES, VERDICTS, withAuthorKinds, type CatalogId } from "./check-pack.js"; +import { CATALOG_IDS, CONVENTIONAL_FIELDS, CONVENTIONAL_TYPE, CustomId, Evidence, ISSUE_TRACKERS, Label, POLICY_EVENTS, stepListSchema, Text, TEXT_FIELDS, TEXT_MATCHES, VERDICTS, withAuthorKinds, type CatalogId } from "./check-pack.js"; const Lines = Text.array().atLeastLength(1).atMostLength(50); const Limit = type("number.integer").atLeast(1); @@ -17,6 +17,7 @@ const SIMPLE_CHECKS = ["focused", "docs", "no-merge-conflicts", "ci-green"] as c const RuleSchema = type({ ...RULE_BASE, check: "'max-files-changed' | 'max-lines-changed'", max: Limit }) .or({ ...RULE_BASE, check: "'linked-issue'", "tracker?": type.enumerated(...ISSUE_TRACKERS.filter((tracker) => tracker !== "off")) }) .or({ ...RULE_BASE, check: "'no-forbidden-paths'", paths: Lines }) + .or({ ...RULE_BASE, check: "'conventional-commits'", "types?": type(CONVENTIONAL_TYPE).describe("a single word").array().atLeastLength(1).atMostLength(50), "fields?": type.enumerated(...CONVENTIONAL_FIELDS).array().atLeastLength(1) }) .or({ ...RULE_BASE, check: type.enumerated(...SIMPLE_CHECKS) }) .or({ ...OWN_RULE, check: "'model'" }) .or({ ...OWN_RULE, check: "'text'", fields: type.enumerated(...TEXT_FIELDS).array().atLeastLength(1), failWhen: type.enumerated(...TEXT_MATCHES), phrases: Lines }); @@ -66,6 +67,8 @@ function catalogRow(rule: Extract): Record = { github: "GitHub", linear: "Linear", either: "GitHub or Linear", off: "Not required" }; @@ -8,6 +8,14 @@ function count(value: number | undefined): string { return value?.toLocaleString() ?? "any number of"; } +const CONVENTIONAL_SUBJECTS = { title: "a conventional commit title", commits: "conventional commit messages", both: "a conventional commit title and messages" } as const; + +function conventionalSentence(row: CatalogCheck | undefined): string { + const fields = row?.fields ?? CONVENTIONAL_FIELDS; + const subject = CONVENTIONAL_SUBJECTS[fields.length === 1 ? fields[0]! : "both"]; + return `Require ${subject}${row?.types ? ` of type ${row.types.join(", ")}` : ""}`; +} + const CATALOG_SENTENCES: Record string> = { "max-files-changed": (row) => `Flag pull requests over ${count(row?.maxFiles)} files changed`, "max-lines-changed": (row) => `Flag pull requests over ${count(row?.maxLines)} lines changed`, @@ -15,6 +23,7 @@ const CATALOG_SENTENCES: Record st "no-merge-conflicts": () => "Flag merge conflicts with the base branch", "ci-green": () => "CI must pass before merge", "no-forbidden-paths": (row) => `Never accept changes to ${(row?.forbiddenGlobs ?? []).join(", ") || "forbidden paths"}`, + "conventional-commits": conventionalSentence, focused: () => "Check the change is focused on one thing", docs: () => "Check docs are updated when behavior changes", }; diff --git a/packages/triage-workflows/src/logic/checks.test.ts b/packages/triage-workflows/src/logic/checks.test.ts index 6d96b2556..b527fa7bd 100644 --- a/packages/triage-workflows/src/logic/checks.test.ts +++ b/packages/triage-workflows/src/logic/checks.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { DEFAULT_REPO_POLICY, emptyPack, readCheckPack, recommendedPack, repoPolicy, type RepoPolicy } from "@corbits/triage-contracts"; +import { DEFAULT_REPO_POLICY, emptyPack, readCheckPack, readPolicyFile, recommendedPack, repoPolicy, type RepoPolicy } from "@corbits/triage-contracts"; import { deriveState, REVIEW_ANSWERED_REASON, type PrFacts } from "./checks.ts"; import { NO_FACTS } from "./item.ts"; @@ -209,3 +209,29 @@ describe("text rules", () => { expect(result.needsJudgment).toBe(false); }); }); + +describe("conventional commits", () => { + const open: PrFacts = { ...noisy, draft: false, mergeable: true, checks: "success" }; + const RULE = "rules:\n - check: conventional-commits\n text: Conventional.\n reason: History.\n evidence: [maintainer-stated]\n"; + + function conventional(title: string, commitMessages: string[], settings = "") { + const pack = readCheckPack(readPolicyFile(`${RULE}${settings}`, "acme/widgets")); + return deriveState({ ...open, title, commitMessages }, undefined, pack).checks.find((check) => check.check === "conventional-commits"); + } + + test("a type in any case, a scope and a breaking mark pass, and merges and reverts are skipped", () => { + expect(conventional("feat(api)!: drop v1", ["Fix: login\n\nBody", "Merge pull request #4 from acme/fix", "Merge pull request #12 in PROJ/widgets from fix to main", "Merge branch 'main' into fix", 'Revert "feat: login"', "chore(deps): bump"])).toMatchObject({ result: "pass", reason: "follows conventional commits" }); + }); + + test("an unknown type, a missing colon or space, and a type the policy leaves out each fail, named", () => { + expect(conventional("feature: login", ["fix login", "fix:login", "fix(): login"])).toMatchObject({ + result: "fail", + reason: "`feature: login` isn't a conventional commit; `fix login` isn't a conventional commit; `fix:login` isn't a conventional commit; and 1 more", + }); + expect(conventional("feat: login", [], " types: [fix]\n")).toMatchObject({ result: "fail", reason: "`feat: login` isn't a conventional commit" }); + }); + + test("title-only reads no commits", () => { + expect(conventional("fix: login", ["Add tests"], " fields: [title]\n")).toMatchObject({ result: "pass" }); + }); +}); diff --git a/packages/triage-workflows/src/logic/checks.ts b/packages/triage-workflows/src/logic/checks.ts index bc4211e84..f40f6978b 100644 --- a/packages/triage-workflows/src/logic/checks.ts +++ b/packages/triage-workflows/src/logic/checks.ts @@ -2,6 +2,8 @@ import { catalogCheckEnabled, CLOSED_FINDING, classificationSources, + CONVENTIONAL_FIELDS, + CONVENTIONAL_TYPES, customCheckEnabled, isTextCheck, parseCheckPack, @@ -12,6 +14,7 @@ import { type CatalogId, type CheckOutput, type CheckPack, + type ConventionalField, type IssueTracker, type TextCustomCheck, type TriageEvent, @@ -225,12 +228,41 @@ function textResult(rule: TextCustomCheck, facts: PrFacts): { failed: boolean; r return { failed: missing.length > 0, reason: missing.length ? `${what} does not contain ${phrases}` : `contains ${phrases}`, evidence: missing.map((item) => item.where).slice(0, TEXT_EVIDENCE_MAX) }; } +// `type(scope)!: subject`; the type is checked against the policy's list once it parses, ignoring case as the spec asks. +const CONVENTIONAL_SUBJECT = /^([^\s():!]+)(?:\([^()]+\))?!?: \S/; +// Subjects git and the hosts write themselves: merges, and the revert button's `Revert "…"`. +const GENERATED_SUBJECT = /^(?:Merge (?:pull request|branch|remote-tracking branch|commit|tag) |Revert ")/; +const OFFENDERS_SHOWN = 3; + +type ConventionalRule = { types: readonly string[]; fields: readonly ConventionalField[] }; + +function isConventional(subject: string, types: readonly string[]): boolean { + const type = CONVENTIONAL_SUBJECT.exec(subject)?.[1]?.toLowerCase(); + return type !== undefined && types.some((allowed) => allowed.toLowerCase() === type); +} + +function offenderReason(subject: string): string { + return `\`${excerpt(subject).replaceAll("`", "'")}\` isn't a conventional commit`; +} + +/** Each title or commit subject that is not `type(scope)!: subject` with an allowed type; merges and reverts are left out. */ +function conventionalResult(facts: PrFacts, { types, fields }: ConventionalRule): { failed: boolean; reason: string } { + const commits = facts.commitMessages.map((message) => message.split("\n", 1)[0]!); + const subjects = [...(fields.includes("title") ? [facts.title] : []), ...(fields.includes("commits") ? commits : [])].map((subject) => subject.trim()); + const offenders = [...new Set(subjects.filter((subject) => !GENERATED_SUBJECT.test(subject) && !isConventional(subject, types)))]; + if (offenders.length === 0) return { failed: false, reason: "follows conventional commits" }; + const shown = offenders.slice(0, OFFENDERS_SHOWN).map(offenderReason); + const more = offenders.length - shown.length; + return { failed: true, reason: [...shown, ...(more > 0 ? [`and ${more} more`] : [])].join("; ") }; +} + interface CheckRules { enabled(id: CatalogId): boolean; maxFiles?: number; maxLines?: number; forbiddenGlobs: string[]; tracker: IssueTracker; + conventional?: ConventionalRule; } function resultOf(failed: boolean | null): CheckResult["result"] { @@ -320,6 +352,11 @@ function evaluate(facts: PrFacts, rules: CheckRules, pack?: CheckPack): Determin values: { linkedIssue: ref !== null }, }); } + if (on("conventional-commits") && rules.conventional) { + const { failed, reason } = conventionalResult(facts, rules.conventional); + if (failed) add("conventional-commits", "needs-author-update", reason); + result("conventional-commits", failed, reason, [], { kind: "values", values: { conventional: !failed } }); + } for (const rule of pack?.custom.filter(isTextCheck).filter(customCheckEnabled) ?? []) { const { failed, reason, evidence } = textResult(rule, facts); if (failed) add(rule.id, "needs-author-update", reason); @@ -337,6 +374,7 @@ function deriveFromPack(facts: PrFacts, pack: CheckPack): DeterministicResult { maxLines: pack.checks["max-lines-changed"]?.maxLines, forbiddenGlobs: pack.checks["no-forbidden-paths"]?.forbiddenGlobs ?? [], tracker: pack.checks["linked-issue"]?.tracker ?? "either", + conventional: { types: pack.checks["conventional-commits"]?.types ?? CONVENTIONAL_TYPES, fields: pack.checks["conventional-commits"]?.fields ?? CONVENTIONAL_FIELDS }, }, pack); } diff --git a/packages/triage-workflows/src/pr-triage.test.ts b/packages/triage-workflows/src/pr-triage.test.ts index 886f432a7..4fa738532 100644 --- a/packages/triage-workflows/src/pr-triage.test.ts +++ b/packages/triage-workflows/src/pr-triage.test.ts @@ -49,9 +49,9 @@ const RUNS: Record = { failure: [{ name: "build", status: "completed", conclusion: "failure" }], }; /** `stacked` is corbitsdev/corbits-code#1520's shape, with #9 to #13 merged into its branch and #14 closed unmerged; the branch is based on open #7's. */ -type GithubSeed = { missing?: boolean; files?: FileRow[]; labels?: string[]; prior?: string; failComments?: boolean; stacked?: boolean }; +type GithubSeed = { missing?: boolean; files?: FileRow[]; labels?: string[]; prior?: string; failComments?: boolean; stacked?: boolean; title?: string; commits?: string[] }; -function fakeGithub({ missing = false, files = [FLAG], labels = [], prior, failComments = false, stacked = false }: GithubSeed = {}) { +function fakeGithub({ missing = false, files = [FLAG], labels = [], prior, failComments = false, stacked = false, title = "Fix #3", commits = [] }: GithubSeed = {}) { const requests: Recorded[] = []; const comments: Comment[] = prior === undefined ? [] : [{ id: 5, user: APP, body: prior }]; const reviews: Review[] = []; @@ -65,10 +65,11 @@ function fakeGithub({ missing = false, files = [FLAG], labels = [], prior, failC if (stacked && path.startsWith(`/repos/${REPO}/pulls?state=open&head=acme%3Aparent&`)) return Response.json([{ number: 7, head: { ref: "parent", repo: { id: 1 } } }]); if (path.startsWith(`/repos/${REPO}/compare/`)) return Response.json({ ahead_by: 2, commits: [{ sha: "merge1521" }, { sha: pr.head }] }); if (path.startsWith(`/repos/${REPO}/pulls?state=closed&base=fix`)) return Response.json([{ number: 1521, merge_commit_sha: "merge1521", merged_at: "2026-10-19T00:43:02Z" }, { number: 1490, merge_commit_sha: "elsewhere", merged_at: "2026-10-01T00:00:00Z" }]); - if (path.startsWith(`/repos/${REPO}/pulls?`)) return Response.json([{ number: 8, title: "Fix #3", user: { login: "octocat" }, draft: pr.draft, head: { sha: pr.head } }]); + if (path.startsWith(`/repos/${REPO}/pulls?`)) return Response.json([{ number: 8, title, user: { login: "octocat" }, draft: pr.draft, head: { sha: pr.head } }]); if (path === `/repos/${REPO}/pulls/8`) { - return Response.json({ number: 8, title: "Fix #3", state: pr.state, draft: pr.draft, user: { login: "octocat" }, head: { sha: pr.head, ref: "fix", repo: { id: 1 } }, base, mergeable: true, labels: labels.map((name) => ({ name })), requested_reviewers: [{ login: "rev" }] }); + return Response.json({ number: 8, title, state: pr.state, draft: pr.draft, user: { login: "octocat" }, head: { sha: pr.head, ref: "fix", repo: { id: 1 } }, base, mergeable: true, labels: labels.map((name) => ({ name })), requested_reviewers: [{ login: "rev" }] }); } + if (path.startsWith(`/repos/${REPO}/pulls/8/commits?`)) return Response.json(commits.map((message, index) => ({ sha: `c${index}`, commit: { message } }))); if (path.includes("/check-runs")) return Response.json({ check_runs: RUNS[pr.ci] }); if (path === `/repos/${REPO}`) return Response.json({ default_branch: "main" }); if (path.startsWith(`/repos/${REPO}/git/trees/`)) return Response.json({ sha: "tree", tree: DOCS.map(([doc, text], index) => ({ path: doc, type: "blob", sha: `blob${index}`, size: bytes(text) })) }); @@ -245,6 +246,16 @@ describe("pr-triage body", () => { expect((verdict as Verdict).request.labels).toEqual(["Triage: Ready"]); }); + test("conventional commits fails a title and commits without a type, naming each", async () => { + const pack: CheckPack = { ...emptyPack(REPO), checks: { "conventional-commits": { enabled: true } } }; + const commits = ["Add tests for live run stream", "Update docs: live run stream", "Merge branch 'main' into chat-live-run"]; + const { path, verdict } = await triage(pack, { title: "Chat: stream live run progress", commits }); + const reason = "`Chat: stream live run progress` isn't a conventional commit; `Add tests for live run stream` isn't a conventional commit; `Update docs: live run stream` isn't a conventional commit"; + expect(path).toEqual(RULES_PATH); + expect(verdict).toMatchObject({ verdict: "fail", reasons: [reason] }); + expect((verdict as Verdict).request.comment).toStartWith(`**Triage: Fail**\n\n@octocat, please address the following:\n- ${reason}`); + }); + test("requested changes wait on the author, then on the reviewer once the author pushes, until the reviewer reviews again", async () => { const github = fakeGithub(); const pack: CheckPack = { ...emptyPack(REPO), checks: { "ci-green": { enabled: true } }, actions: HELLO }; diff --git a/skills/corbits-triage-setup/reference.md b/skills/corbits-triage-setup/reference.md index b8d792f1b..a2361bd7d 100644 --- a/skills/corbits-triage-setup/reference.md +++ b/skills/corbits-triage-setup/reference.md @@ -24,6 +24,7 @@ A built-in rule is named only by `check`, never `id`, and each built-in may appe | `ci-green` | none | no | Fails when checks fail; unconfirmed while they run; passes when there are no checks. | | `linked-issue` | `tracker:` `github`, `linear` or `either` (optional, default `either`) | no | Fails unless the title, description, a commit or the branch name references an issue. `github` reads `#123`, GitHub issue links and branches like `123-title`; `linear` reads any key like `ABC-123` (so Jira keys match too), Linear links and branches like `abc-123-title` in any case. | | `no-forbidden-paths` | `paths:` list of 1–50 globs (required) | no | Fails when any changed file matches; for every author, maintainers too. `*` stays within a folder, `**` crosses folders. | +| `conventional-commits` | `types:` list of words (optional, default `feat`, `fix`, `refactor`, `perf`, `docs`, `test`, `chore`, `ci`, `build`, `style`, `revert`); `fields:` `title`, `commits` or both (optional, default both) | no | Fails unless the title and each commit's first line (first 100 commits) start `type: `, `type(scope): ` or `type!: ` with a listed type, in any case. Merges and `Revert "…"` commits are skipped. | | `focused` | none | yes | The decision model checks the change is focused on one thing. | | `docs` | none | yes | The decision model checks docs are updated when behaviour changes. | @@ -47,7 +48,7 @@ Start from the four starter rules (SKILL.md step 3). Add an extra only when the | --- | --- | | Issue links or `ABC-123` keys in most commits or branches (not squash-merge `(#123)` suffixes) | `linked-issue`: `github` for `#123`, `linear` for `ABC-123` keys from any tracker, `either` for both | | Vendored or generated folders that pull requests should never touch | `no-forbidden-paths` listing them | -| Commits mostly follow conventional commits | `conventional-title` below. Copy its phrases exactly and add `type:`, `type(`, `type!:` for each other type in the history (at most 5 more; 50 phrases in all). It checks the title contains a type, not that it starts with one. | +| Commits mostly follow conventional commits | `conventional-commits`; add `types` only when the history uses a type outside the default list, and `fields: [title]` when pull requests are squash-merged | | The user bars tool attribution | `no-tool-attribution` below. Add the user's tool trailers to its phrases; never a bare `Co-Authored-By:`, which also fails human co-authors. | | A decision model is set up and pull requests often mix changes | `focused` | | A decision model is set up and the repository keeps user docs beside its code | `docs` | @@ -140,45 +141,8 @@ rules: paths: ["vendor/**"] reason: Generated or vendored code is changed by its own process. evidence: [maintainer-stated] - - id: conventional-title - check: text - text: The title contains a conventional commit type such as feat or fix. - fields: [title] - failWhen: not-contains - phrases: - - "feat:" - - "feat(" - - "feat!:" - - "fix:" - - "fix(" - - "fix!:" - - "docs:" - - "docs(" - - "docs!:" - - "style:" - - "style(" - - "style!:" - - "refactor:" - - "refactor(" - - "refactor!:" - - "perf:" - - "perf(" - - "perf!:" - - "test:" - - "test(" - - "test!:" - - "build:" - - "build(" - - "build!:" - - "ci:" - - "ci(" - - "ci!:" - - "chore:" - - "chore(" - - "chore!:" - - "revert:" - - "revert(" - - "revert!:" + - check: conventional-commits + text: Require a conventional commit title and messages. reason: The repository's commits follow conventional commits. evidence: [maintainer-stated] - id: no-tool-attribution