Skip to content

ci: require every PR to close an issue or say why it doesn't#4798

Merged
Hmbown merged 1 commit into
mainfrom
chore/pr-issue-link
Jul 26, 2026
Merged

ci: require every PR to close an issue or say why it doesn't#4798
Hmbown merged 1 commit into
mainfrom
chore/pr-issue-link

Conversation

@Hmbown

@Hmbown Hmbown commented Jul 24, 2026

Copy link
Copy Markdown
Owner

No-Issue: repo-hygiene automation, filed from a backlog measurement rather than a report.

What the numbers said

342 issues open, 329 of them touched this month. Nothing is rotting — the backlog isn't stale, it's unclosed. stale.yml runs green daily but is scoped to only-labels: needs-info, which is 8 issues out of 342.

The leak: only 8 of 35 open PRs carry a closing keyword. The other 27 will merge and leave their issues open. That's how 121 issues land on one milestone and why the backlog stopped saying which work is actually done.

What this does

Reads the PR body for a GitHub closing keyword attached to an issue number, or a one-line opt-out:

Closes #1234
No-Issue: dependency bump

Either satisfies it. It's a prompt, not a wall.

Body only, deliberately. GitHub resolves closing keywords from the PR description — Closes #123 in the title auto-closes nothing. Accepting the title would pass PRs that close no issue: a green check guarding nothing, which is the shape this repo already has in cache_guard.rs and in the protected_invariants nothing enforces.

The body is read through env: rather than interpolated into the script, since it's attacker-controlled.

Verification

Matcher tested against 13 cases. All six closing-keyword spellings match (close/closes/closed/fix/fixes/fixed/resolve/resolves/resolved), multiline bodies match, indented opt-outs match. Correctly rejected: bare #1234 with no keyword, prose "this fixes the parser" with no number, an empty No-Issue:, and an unmarked chore. YAML validated.

Also changed, outside this diff

delete_branch_on_merge and allow_auto_merge are now enabled on the repo — merged branches stop accumulating (4 were sitting merged-but-undeleted).

Note on existing PRs

pull_request fires on opened/edited/reopened/synchronize, so the 27 unlinked PRs stay green until someone pushes to them. No retroactive red wall.

342 issues are open and 329 of them were touched this month, so nothing is
rotting -- the backlog is not stale, it is unclosed. Only 8 of 35 open PRs
carried a closing keyword. The other 27 will merge and leave their issues
open, which is how 121 issues accumulate on a single milestone and why the
backlog no longer says which work is actually done.

The check reads the PR body for one of GitHub's closing keywords attached to
an issue number, or a one-line `No-Issue: <why>` opt-out for chores, reverts
and dependency bumps. Either satisfies it, so this is a prompt rather than a
wall.

Body only, deliberately: GitHub resolves closing keywords from the PR
description, so a "Closes #123" sitting in the title auto-closes nothing.
Accepting the title would have passed PRs that close no issue -- the same
false-assurance shape as a green check that guards nothing.

The body is read through env rather than interpolated into the script, since
it is attacker-controlled text.

Verified the matcher against 13 cases: all six closing-keyword spellings and
a multiline body match; indented and inline opt-outs match; bare `#1234`,
prose "this fixes the parser" with no number, an empty `No-Issue:`, and an
unmarked chore correctly fail.

Repo settings changed alongside this: delete_branch_on_merge and
allow_auto_merge are now on, so merged branches stop accumulating.
Copilot AI review requested due to automatic review settings July 24, 2026 22:26
@cursor

cursor Bot commented Jul 24, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 4m 33s —— View job


Review: pr-issue-link.yml

  • Read the workflow diff and repo CLAUDE.md/AGENTS.md guidance
  • Check correctness of the bash matcher (regex, heredoc, quoting)
  • Check security (script-injection surface, token permissions)
  • Check test coverage for the new logic
  • Post review

Small, self-contained addition — one new workflow file, no other code touched. Overall this is solid: it correctly reads PR_BODY through env: instead of interpolating it into the script body (the standard fix for the GitHub Actions script-injection class of bug), scopes permissions: contents: read to least privilege, and triggers on plain pull_request rather than pull_request_target, so forked PRs never get a token with write access. I traced the heredoc at lines 48–60 closely since indented <<'MSG' terminators are a classic gotcha — confirmed the terminator line has the same indentation as the block's base line, so YAML's block-scalar dedent strips it correctly and the heredoc closes as intended. No bug there.

Two minor, non-blocking observations:

1. The #N match misses GitHub's other closing-reference forms (.github/workflows/pr-issue-link.yml:36). The regex requires #[0-9]+ directly after the keyword, but GitHub also resolves closing keywords against owner/repo#123 and full issue URLs (Closes https://github.com/Hmbown/CodeWhale/issues/123). A PR body using the URL form — plausible if someone pastes a link instead of typing #N — would actually close its issue on merge but still get flagged by this check as "neither closes nor opts out." Given this repo is single-repo, the URL form is the realistic case worth covering; cross-repo is unlikely to matter here.

2. The "tested against 13 cases" verification isn't checked into the repo. The PR description lists solid manual coverage (six keyword spellings, multiline, indented opt-out, and the four rejection cases), but none of it exists as a script in the diff. A small assertion loop (e.g. a tests/pr-issue-link-cases.sh invoked with bash -n/local grep before this is trusted as a required check) would let the regex be re-verified automatically if it's ever edited, rather than relying on a comment claim.

Neither blocks merge — the design is explicitly "a prompt, not a wall," and both gaps fail closed (worst case: a valid closing reference has to be restated as #N, not silently ignored).

@Hmbown
Hmbown merged commit 3584f18 into main Jul 26, 2026
24 checks passed
@Hmbown
Hmbown deleted the chore/pr-issue-link branch July 26, 2026 07:02
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