Skip to content

chore: add PR template and required CI checks (V2-719, V2-720) - #185

Merged
jacderida merged 3 commits into
mainfrom
chrisoneil/v2-719-add-the-standard-pr-template-across-all-six-crate-repos
Jul 27, 2026
Merged

chore: add PR template and required CI checks (V2-719, V2-720)#185
jacderida merged 3 commits into
mainfrom
chrisoneil/v2-719-add-the-standard-pr-template-across-all-six-crate-repos

Conversation

@jacderida

Copy link
Copy Markdown
Collaborator

Linear issue

Risk tier

  • T0 — docs / tooling / CI / pure UX-output. Repo CI only.
  • T1 — client-only, no network-facing behavior change. CI + prod compat smoke.
  • T2 — node/client logic with behavioral surface, no protocol/format/economics change. Dev testnet + ADR.
  • T3 — protocol / storage format / payments / routing. T2 evidence + adversarial testing.

Compatibility

  • Wire: none
  • Storage: none
  • API: none

Semver impact

  • breaking
  • feature
  • fix

Test evidence

check_pr.py was exercised locally across its cases: missing Linear reference
fails linear; a key in the title/body/branch passes; a body without the
template fails template; an rc-* base is a no-op pass; a fully-filled template
passes; and the failure modes (no/duplicate tier or semver box, empty section,
Tier 2/3 without an ADR link) each fail with a specific message. This PR's own
description is validated by the new checks.

New dependency

none

ADR

n/a

Mitigation / rollback

Revert this PR; the template and checks are additive and carry no runtime effect.

Add the standard pull request template (V2-719) and the CI enforcement that
makes a linked Linear issue and the template fields required on every PR
(V2-720). Part of the release-process extensions; identical across the six
crate repos.

- .github/PULL_REQUEST_TEMPLATE.md: eight-field template with Risk tier and
  Semver impact checkboxes, stable headings as grep anchors for the
  train-manifest verification.
- CLAUDE.md: instruct agents to fill the template on PR open.
- .github/workflows/pr-checks.yml + .github/scripts/check_pr.py: the
  linear-link (main + rc-*) and pr-template (main) required status checks.

V2-719
V2-720

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 24, 2026 19:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds repo-level PR hygiene enforcement for the ant-node project by introducing a standardized PR template and GitHub Actions checks that require a Linear reference and a fully completed template (with main-only enforcement for template completeness).

Changes:

  • Added .github/PULL_REQUEST_TEMPLATE.md to standardize PR descriptions.
  • Added pr-checks GitHub Actions workflow to enforce a Linear link and PR template completeness.
  • Added a Python checker script (.github/scripts/check_pr.py) and documented the requirement in CLAUDE.md.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
CLAUDE.md Documents the new requirement to use the standard PR template and notes the CI enforcement checks.
.github/workflows/pr-checks.yml Adds CI jobs intended to enforce “linear-link” and “pr-template” status checks on PRs targeting main / rc-*.
.github/scripts/check_pr.py Implements the Linear reference and PR template validation logic executed by the workflow jobs.
.github/PULL_REQUEST_TEMPLATE.md Introduces the standardized PR template with required sections and checkboxes.
Comments suppressed due to low confidence (1)

.github/workflows/pr-checks.yml:36

  • Same tamper-resistance concern here: ensure the checkout uses the base SHA so the template check script can’t be altered in the PR to force a pass.
    steps:
      - uses: actions/checkout@v4
      - name: Require the PR template fields

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +8 to +12
pull_request:
types: [opened, edited, synchronize, reopened]
branches:
- main
- 'rc-*'
Comment on lines +21 to +23
steps:
- uses: actions/checkout@v4
- name: Require a linked Linear issue
Comment thread .github/scripts/check_pr.py Outdated
Comment on lines +52 to +55
def has_linear_ref(*parts):
haystack = "\n".join(parts)
m = LINEAR_URL.search(haystack) or LINEAR_KEY.search(haystack)
return m.group(0) if m else None
Comment thread .github/scripts/check_pr.py Outdated
Comment on lines +106 to +118
for heading in (
"linear issue",
"risk tier",
"compatibility",
"semver impact",
"test evidence",
"new dependency",
"adr",
"mitigation / rollback",
):
if heading not in secs:
errors.append(f"missing section: ## {heading.title()}")

@dirvine dirvine left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found two gaps in the enforcement, reproduced against the checker on this PR head:

  1. The Linear gate does not reliably require a Linear issue. LINEAR_KEY accepts any <letters>-<digits> token anywhere in the title, body, or branch. For example, a PR containing only UTF-8 as its supposed issue reference passes both linear and template. Common technical strings such as SHA-256, HTTP-2, or RFC-123 can therefore satisfy the required check accidentally. Please require a real linear.app/.../issue/... URL, or constrain keys to the organisation's known Linear team prefixes.

  2. “Fill every field” is not enforced. A Compatibility section with only Wire: none passes while Storage: and API: are blank, and a T0/T1 PR also passes with an empty ADR section. I reproduced that combination locally and received PR template complete. Please require all three compatibility axes and require an explicit ADR value (n/a for T0/T1; a link for T2/T3).

The six PRs currently carry the same checker (SHA-256 89aa9a45c0444cb40726a3e5d3b8cc5e402d75ed4e50341fb2fff84053098649), so the same fix should be applied consistently across them. The template/workflow shape otherwise looks sound, and the new checks themselves are running successfully.

Address review feedback on the release-process PR checks:

- Linear gate: require a real Linear reference — a linear.app URL or an
  issue key with a known team prefix (V2/AUTO/REL/INFRA/QA) — so tokens
  like UTF-8, SHA-256, RFC-123 no longer satisfy it. Strip HTML comments
  before matching so the template's own V2-123 example does not count.
- Template completeness: require all three Compatibility axes
  (Wire/Storage/API) and an explicit ADR value (n/a for T0/T1, a link for
  T2/T3); previously one filled axis and an empty ADR passed.
- Fix a regex that let an empty Compatibility axis match the next line.
- Emit exact template headings in "missing section" errors.
- Run the workflow on ready_for_review too.

V2-719
V2-720

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 27, 2026 18:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

.github/scripts/check_pr.py:97

  • check_linear() says a linear.app URL must be in the PR body, but it currently searches for linear.app/ across title/body/branch via linear_ref(). This makes enforcement inconsistent with the error message (and the PR description). Either allow URLs anywhere and update the message, or enforce URL-in-body only; the patch below implements URL-in-body only while still allowing issue keys in title/body/branch.
def check_linear():
    # Strip comments from the body so the template's own example does not count.
    ref = linear_ref(env("PR_TITLE"), strip_comments(env("PR_BODY")), env("PR_BRANCH"))
    if ref:
        ok(f"✅ Linear reference found: {ref}")

Comment thread .github/scripts/check_pr.py Outdated
LINEAR_KEY = re.compile(
r"\b(?:" + "|".join(LINEAR_TEAM_PREFIXES) + r")-[0-9]+\b", re.IGNORECASE
)
LINEAR_URL = re.compile(r"linear\.app/", re.IGNORECASE)
Comment thread CLAUDE.md
Comment on lines +124 to +125
CI enforces the Linear link and the template fields (`linear-link` and
`pr-template` status checks); a PR that omits them fails its checks.
@jacderida

Copy link
Copy Markdown
Collaborator Author

Thanks — both gaps are fixed, and the same checker (identical across all six PRs) is updated consistently.

1. Linear gate no longer accepts arbitrary tokens. A reference now counts only if it is a linear.app issue URL, or an issue key with a known Linear team prefix (V2 / AUTO / REL / INFRA / QA). UTF-8, SHA-256, HTTP-2, RFC-123 all fail now (verified). The prefix list is a documented constant to update when a team is added. Relatedly, HTML comments are now stripped before matching, so the template's own V2-123 example in the comment no longer satisfies the check.

2. "Fill every field" is enforced. All three Compatibility axes (Wire/Storage/API) must each carry a value (none is fine), and the ADR field must be filled explicitly — n/a for Tier 0/1, a real link for Tier 2/3. A T0 PR with a blank ADR, or a Compatibility block with only Wire: filled, now fails. While fixing this I also caught a regex bug where an empty axis could "borrow" the next line and pass.

Also addressed from the Copilot review: exact template headings in the "missing section" messages (## ADR, ## Mitigation / rollback rather than Adr / Mitigation / Rollback), and the workflow now also triggers on ready_for_review.

One point I did not change yet: Copilot's tamper-resistance note (switch pull_requestpull_request_target so a PR can't edit the checker to force a pass). It's valid, but pull_request_target runs the workflow from the base branch — which won't have this workflow until these PRs merge — so applying it now would stop the checks running on these very PRs. Suggest converting it in a fast-follow once this is on main.

@dirvine dirvine left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compatibility and ADR enforcement is fixed, and the constrained issue-key prefixes correctly reject UTF-8/SHA-256-style tokens. One residual gap remains in the Linear URL path:

LINEAR_URL = re.compile(r"linear\.app/", re.IGNORECASE) accepts any page on the domain. I reproduced both https://linear.app/changelog and https://linear.app/not-an-issue passing the linear and template checks as a linked issue.

Please constrain the URL pattern to an actual issue path, for example linear.app/<workspace>/issue/<key> (allowing the optional issue slug if needed), and include this negative case in the test matrix. Everything else from my previous review is addressed.

Follow-up review: the linear.app URL check accepted any page on the domain
(linear.app/changelog, linear.app/not-an-issue passed). Constrain it to a real
issue path (linear.app/<workspace>/issue/<key>).

- Linear URL must now match linear.app/<workspace>/issue/<key>[/<slug>].
- Clarify the failure message (a reference is accepted in title, body, or
  branch, matching the actual behaviour).
- Add .github/scripts/test_check_pr.py: a 17-case matrix exercising the real
  checker, including the linear.app/changelog and linear.app/not-an-issue
  negatives; run it in CI via a new self-test job.

V2-719
V2-720

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 27, 2026 18:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

.github/scripts/check_pr.py:39

  • The Linear issue URL regex can be bypassed by domains that merely contain linear.app (e.g. https://notlinear.app/...), because the pattern is not anchored/bounded to the real linear.app host. This weakens the Linear-link requirement and can allow false positives.
LINEAR_URL = re.compile(
    r"linear\.app/[^/\s]+/issue/[A-Za-z][A-Za-z0-9]*-[0-9]+", re.IGNORECASE
)

Comment on lines +65 to +69
("linear: UTF-8 only", "linear", {"PR_TITLE": "fix UTF-8", "PR_BRANCH": "fix/utf8"}, 1),
("linear: SHA-256 / RFC-123", "linear", {"PR_TITLE": "SHA-256 RFC-123", "PR_BRANCH": "x"}, 1),
("linear: linear.app/changelog", "linear", {"PR_BODY": "see https://linear.app/changelog", "PR_BRANCH": "x"}, 1),
("linear: linear.app/not-an-issue", "linear", {"PR_BODY": "https://linear.app/not-an-issue", "PR_BRANCH": "x"}, 1),
("linear: unfilled template (example in comment)", "linear", {"PR_BODY": UNFILLED, "PR_BRANCH": "x"}, 1),
@jacderida

Copy link
Copy Markdown
Collaborator Author

Fixed — and thanks for catching the loose URL path.

Linear URL is now constrained to a real issue path. LINEAR_URL matches linear.app/<workspace>/issue/<key> (with an optional trailing slug), so https://linear.app/changelog and https://linear.app/not-an-issue are both rejected now. I also fixed the message inconsistency Copilot flagged: a reference is accepted in the title, body, or branch, and the message now says exactly that.

Both negatives are in a committed test matrix. Added .github/scripts/test_check_pr.py — a 17-case matrix that runs the real checker as a subprocess and asserts the outcome, including linear.app/changelog, linear.app/not-an-issue, UTF-8/SHA-256, the unfilled-template-comment case, and the completeness cases (compat axes, ADR by tier, tier/semver box counts). It runs in CI as a new self-test job, so the matrix is enforced on every PR, not just something I ran locally.

The same checker + tests are identical across all six PRs (check_pr.py sha256 3f88cf70…).

@dirvine dirvine left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up verified. The Linear URL matcher now requires an actual <workspace>/issue/<key> path, the two reported false-positive URLs are covered by regression tests, and the shared test matrix passes 17/17 locally. The required linear-link and pr-template checks are green. Approved.

@jacderida
jacderida merged commit c3ffe5a into main Jul 27, 2026
16 checks passed
@jacderida
jacderida deleted the chrisoneil/v2-719-add-the-standard-pr-template-across-all-six-crate-repos branch July 27, 2026 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants