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
4 changes: 2 additions & 2 deletions .github/scripts/tests/verify-release-notes.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ test("labels, description, author and base come from the pull request as it is n
fails("## Overview\n", ["Heading not allowed", "No release notes"], { labels: ["minor"], live });
});

test("a (#n) inside an HTML comment still closes the issue, so it fails; comments stay ignored for everything else", () => {
test("a (#n) inside an HTML comment closes nothing, so it fails; comments stay ignored for everything else", () => {
for (const body of [
"<!-- follow-up (#7) -->\n\n## Added\n\n- A thing (#1)\n",
"## Added\n\n- A thing (#1) <!-- also (#8) -->\n",
Expand All @@ -487,7 +487,7 @@ test("a (#n) inside an HTML comment still closes the issue, so it fails; comment
"## Added\n\n- A thing (#1) <!-- (#3) -->\r\n",
]) {
const result = fails(body, ["Issue reference position"]);
assert.match(result.errors[0].message, /sits inside an HTML comment.*release-action still closes #\d+.*never put issue references in comments/);
assert.match(result.errors[0].message, /sits inside an HTML comment: release-action removes the comment with it, so #\d+ is not closed.*end of the delivering bullet.*never put issue references in comments/);
}
// Only fenced and inline code hide a reference, as in release-action, and what surrounds removed code joins up.
passes("## Added\n\n- A thing (#1)\n\n```\n<!-- (#9) -->\n```\n- Another `<!-- (#8) -->` (#2)\n");
Expand Down
12 changes: 7 additions & 5 deletions .github/workflows/verify-pull-request-policy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ jobs:
const KEYWORD = /\b(close[sd]?|fix(?:e[sd])?|resolve[sd]?|refs?|references)(?:[ \t]*:[ \t]*|[ \t]+)((?:[\w.-]+\/[\w.-]+)?#(\d+))/gi;
const KEYWORD_URL = /\b(close[sd]?|fix(?:e[sd])?|resolve[sd]?|refs?|references)(?:[ \t]*:[ \t]*|[ \t]+)(https?:\/\/(?:www\.)?github\.com\/([\w.-]+\/[\w.-]+)\/(?:issues|pull)\/(\d+))/gi;
// The pattern release-action closes issues with.
// It sees everything but fenced and inline code: HTML comments are not skipped.
// release-action removes closed HTML comments first; this check still reads them, so the position
// rule can fail a `(#n)` hidden in one.
const CLOSES_ISSUE = /(?<![\w/-])\(#(\d+)\)/g;
// What may follow the delivering `(#n)` and still count as the end of the bullet: more issue references
// (`(part of #n)`, `(see #n)`, `(Cratis/Repo#n)` too), closing emphasis, a trailing `<br>` and sentence
Expand Down Expand Up @@ -462,9 +463,10 @@ jobs:
// The same without fenced code, which does not belong to the bullet text around it.
const uncommented = blank(blank(text, FENCED_CODE, " "), INLINE_CODE, "x");
const prose = blank(uncommented, HTML_COMMENT, " ").split("\n");
// What release-action reads when it closes issues: it removes fenced and inline code, in that order,
// and nothing else, so a `(#n)` hidden in an HTML comment still closes its issue. Each match is
// mapped back to where it sits in the description.
// Every `(#n)` outside fenced and inline code, in that order, including those in HTML comments.
// release-action also removes closed comments, so a `(#n)` hidden in one closes nothing; it is
// collected here so the position rule can fail it. Each match is mapped back to where it sits in
// the description.
const closes = closingReferences(text, prose);

const violations = [];
Expand Down Expand Up @@ -650,7 +652,7 @@ jobs:
}
for (const match of closes.filter(candidate => candidate.line === index)) {
if (match.commented) {
report("Issue reference position", index, `\`(#${match.number})\` sits inside an HTML comment: the release page does not show the comment, but release-action still closes #${match.number}. Delete it from the comment, or write \`(part of #${match.number})\` or \`see #${match.number}\` outside the comment if it must stay open; never put issue references in comments.`);
report("Issue reference position", index, `\`(#${match.number})\` sits inside an HTML comment: release-action removes the comment with it, so #${match.number} is not closed. Write \`(#${match.number})\` at the end of the delivering bullet, outside the comment, or write \`(part of #${match.number})\` or \`see #${match.number}\` outside the comment if it must stay open; never put issue references in comments.`);
continue;
}
const end = bulletEnd[match.endLine];
Expand Down
18 changes: 11 additions & 7 deletions .github/workflows/verify-release-notes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,10 @@ name: Verify Release Notes
# Fenced and inline code are exempt: release-action ignores them when closing
# issues and the release page shows them as code. Closed HTML comments are not
# shown on the release page, so the heading, keyword, link and placeholder rules
# ignore them; release-action does NOT ignore them, so a `(#n)` inside a comment
# still closes the issue and fails the position rule. An unclosed `<!--` is an
# ignore them. release-action removes them before it publishes the release and
# reads the issues to close from what it publishes, so a `(#n)` inside a comment
# closes nothing; it still fails the position rule, because the delivering bullet
# must carry it. An unclosed `<!--` is an
# error: GitHub hides everything after it.
# Whether a summary states a cohesive theme, whether a bullet is user-facing, and
# whether an issue number exists cannot be decided from text; reviewers keep those.
Expand Down Expand Up @@ -192,7 +194,8 @@ jobs:
const KEYWORD = /\b(close[sd]?|fix(?:e[sd])?|resolve[sd]?|refs?|references)(?:[ \t]*:[ \t]*|[ \t]+)((?:[\w.-]+\/[\w.-]+)?#(\d+))/gi;
const KEYWORD_URL = /\b(close[sd]?|fix(?:e[sd])?|resolve[sd]?|refs?|references)(?:[ \t]*:[ \t]*|[ \t]+)(https?:\/\/(?:www\.)?github\.com\/([\w.-]+\/[\w.-]+)\/(?:issues|pull)\/(\d+))/gi;
// The pattern release-action closes issues with.
// It sees everything but fenced and inline code: HTML comments are not skipped.
// release-action removes closed HTML comments first; this check still reads them, so the position
// rule can fail a `(#n)` hidden in one.
const CLOSES_ISSUE = /(?<![\w/-])\(#(\d+)\)/g;
// What may follow the delivering `(#n)` and still count as the end of the bullet: more issue references
// (`(part of #n)`, `(see #n)`, `(Cratis/Repo#n)` too), closing emphasis, a trailing `<br>` and sentence
Expand Down Expand Up @@ -496,9 +499,10 @@ jobs:
// The same without fenced code, which does not belong to the bullet text around it.
const uncommented = blank(blank(text, FENCED_CODE, " "), INLINE_CODE, "x");
const prose = blank(uncommented, HTML_COMMENT, " ").split("\n");
// What release-action reads when it closes issues: it removes fenced and inline code, in that order,
// and nothing else, so a `(#n)` hidden in an HTML comment still closes its issue. Each match is
// mapped back to where it sits in the description.
// Every `(#n)` outside fenced and inline code, in that order, including those in HTML comments.
// release-action also removes closed comments, so a `(#n)` hidden in one closes nothing; it is
// collected here so the position rule can fail it. Each match is mapped back to where it sits in
// the description.
const closes = closingReferences(text, prose);

const violations = [];
Expand Down Expand Up @@ -684,7 +688,7 @@ jobs:
}
for (const match of closes.filter(candidate => candidate.line === index)) {
if (match.commented) {
report("Issue reference position", index, `\`(#${match.number})\` sits inside an HTML comment: the release page does not show the comment, but release-action still closes #${match.number}. Delete it from the comment, or write \`(part of #${match.number})\` or \`see #${match.number}\` outside the comment if it must stay open; never put issue references in comments.`);
report("Issue reference position", index, `\`(#${match.number})\` sits inside an HTML comment: release-action removes the comment with it, so #${match.number} is not closed. Write \`(#${match.number})\` at the end of the delivering bullet, outside the comment, or write \`(part of #${match.number})\` or \`see #${match.number}\` outside the comment if it must stay open; never put issue references in comments.`);
continue;
}
const end = bulletEnd[match.endLine];
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -816,11 +816,11 @@ Both gates read the labels (and the release-notes gate the description, author a
The release-notes gate checks every pull request into the default branch except Dependabot's. One that carries any of `major`, `minor` or `patch` is release-bound and fails on a violation, also when it carries a second intent label by mistake. One labelled `no-release`, or not labelled yet, gets the same checks as warnings and passes, since any pull request may become releasable; a `no-release` description without a change list is fine. A pull request into another base passes with a notice. The check re-runs when a label is added or the description is edited. It enforces the release-note contract:

- An optional summary, first: either a short `## Summary` section of prose or one lead paragraph without a heading, not both. Then only `## Added`, `## Changed`, `## Fixed`, `## Removed`, `## Security` and `## Deprecated`, in that order, each once and only when it has bullets: prose or a code example alone under a section, such as a `None.` filler, fails, and so does a bullet that only says `None`, `N/A` or `Nothing` (also in bold or italics, with closing punctuation), which is a filler and not a change, so a section holding only such bullets is a section without bullets. `###` sub-headings may group bullets inside a section (there even `### Added` is only a sub-heading; before or between sections a `###` named like a section fails as a wrong-level section).
- A delivered issue is `(#n)` at the end of its bullet (only more issue references, including `(part of #n)`, `(see #n)` and `(Cratis/Repo#n)`, closing emphasis, `<br>` and sentence punctuation may follow; a `:` may introduce nested bullets). Write one `(#n)` per issue: `(#56) (#57)`, never `(#56, #57)`, which closes nothing. `(part of #n)` or `see #n` keeps an issue open, and `Cratis/Repo#n` names another repository. `Closes`, `Fixes`, `Refs` and the other closing or linking keywords never appear, with `#n` or an issue URL, also when wrapped in bold, italics or a link (`**Closes** #n`, `Closes [#n](url)`, `[Closes #n](url)`, and `[Closes](url)` where the link starts a clause). A `(#n)` in the summary, in a heading, in the middle of a bullet or inside an HTML comment fails because release-action would still close it. For a `(#n)` in the middle of a bullet the error quotes the text that follows it (`(#12)` is followed by `, see #11 ...`) and asks for that text to move before the `(#n)`.
- A delivered issue is `(#n)` at the end of its bullet (only more issue references, including `(part of #n)`, `(see #n)` and `(Cratis/Repo#n)`, closing emphasis, `<br>` and sentence punctuation may follow; a `:` may introduce nested bullets). Write one `(#n)` per issue: `(#56) (#57)`, never `(#56, #57)`, which closes nothing. `(part of #n)` or `see #n` keeps an issue open, and `Cratis/Repo#n` names another repository. `Closes`, `Fixes`, `Refs` and the other closing or linking keywords never appear, with `#n` or an issue URL, also when wrapped in bold, italics or a link (`**Closes** #n`, `Closes [#n](url)`, `[Closes #n](url)`, and `[Closes](url)` where the link starts a clause). A `(#n)` in the summary, in a heading, in the middle of a bullet or inside an HTML comment fails: in the summary, a heading or the middle of a bullet release-action would still close it, and inside a comment release-action removes it with the comment, so the issue is not closed. For a `(#n)` in the middle of a bullet the error quotes the text that follows it (`(#12)` is followed by `, see #11 ...`) and asks for that text to move before the `(#n)`.
- No Overview, Test plan, Verification, Notes or other headings (also written as `<h2>`, `<b>`, `<i>`, `*italic*` or `<summary>`, which may span lines), no review, verification, test-result or provenance notes (which agent or model wrote it, a stand-alone `Reviewed by` line, CI results), no relative links, no template placeholders, no unclosed `<!--` and no Copilot "Original prompt" transcripts. Headings are checked like any other line for keywords, `(#n)`, relative links, placeholders and provenance. Provenance is recognised at the start of a clause (`Reviewed with a cross-provider review.`, `Review: Opus-only review`, `The review workflow passed.`, `Co-Authored-By:`); inside a bullet, a `## Summary` or the lead paragraph the note must also end the clause (`- Cross-provider review pending (#3)`), and `Reviewed by ...` is not read there at all (`- Reviewed by status is now shown on the dashboard (#4)` passes), so a bullet that merely mentions review or validation as a product change, such as `- Validation: rules now apply to commands (#3)` or `- Adds same-provider review routing to the review workflow (#3)`, passes. Reviewer-facing information goes in a pull request comment.
- No verification or scope notes and no reviewer instructions, recognised by their phrasing wherever they stand (a bullet, the summary, the lead paragraph or a line of their own): how or where a change was checked (`Verified by building ...`, `verified in Storybook`, `Tested locally`), what was not (`not yet verified`, `Storybook only`), results (`specs 492/492`, `9 of 9 runs`, `Tier 1 PASS`, `0 warnings, 0 errors`, a `Local:`, `CI:`, `Checks:` or `Checked locally:` line, a `Checks (local)` heading), provenance (`Verified against Arc.TypeScript main (v0.34.0)`, `Tested against a real kernel`; the error asks for compatibility as a fact instead, such as `Requires @cratis/arc 0.34 or later`), and stacking, retargeting, merge or deploy order and draft status (`Stacked on #207`, `Retarget to main`, `This PR should be deployed separately`, `Draft:`, `Do not merge`), reported as a reviewer instruction. The patterns name the phrasing, not the topic: `Events are now verified against their schema`, `Toasts are stacked on top of each other` and `- CI: builds now cache packages (#3)` pass.

Fenced code and inline code are exempt. Closed HTML comments are not shown on the release page, so the heading, keyword, link and placeholder rules ignore them, but a `(#n)` inside a comment still closes the issue and fails, so never put issue references in comments. Each violation is reported as its own error, naming the rule, the offending line and the fix. Whether a summary states a cohesive theme, whether a bullet is user-facing and whether an issue number exists cannot be decided from text; reviewers keep those.
Fenced code and inline code are exempt. Closed HTML comments are not shown on the release page, so the heading, keyword, link and placeholder rules ignore them, and release-action removes them before publishing, so a `(#n)` inside a comment closes nothing. Such a `(#n)` still fails: write it at the end of the delivering bullet, outside the comment, and never put issue references in comments. Each violation is reported as its own error, naming the rule, the offending line and the fix. Whether a summary states a cohesive theme, whether a bullet is user-facing and whether an issue number exists cannot be decided from text; reviewers keep those.

In the legacy release-notes reusable, a second job, `drift`, compares the notes with the diff and only ever warns: a bullet whose code spans (paths and identifiers) the diff no longer touches, because the change landed on main another way or was dropped in conflict resolution; a version the diff only removes; a registry (PyPI, npm, NuGet, Maven Central, Docker Hub, GHCR) that no workflow publishes to; and a merge from the base branch that changed which files the pull request touches. It checks out the pull request head with full history (blobs on demand), skips Dependabot, and runs on label changes too, because the caller's concurrency group cancels the whole run when a label changes and the check would otherwise stay cancelled.

Expand Down
Loading