Skip to content

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

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)#20
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
Contributor

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>

@dirvine dirvine 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.

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>
@jacderida

Copy link
Copy Markdown
Contributor 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

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>
@jacderida

Copy link
Copy Markdown
Contributor 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

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 0cc5fae into main Jul 27, 2026
11 checks passed
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.

2 participants