p24-infra Docs

lint-pr-closing-keywords-operations

5 min read

lint-pr-closing-keywords — operations

Type: GitHub Actions workflow (.github/workflows/lint-pr-closing-keywords.yml) Trigger: pull_request — opened, edited, synchronize Runner: ubuntu-latest Issues: #4744 (Job 1, follow-up to #4741) · #6413 (Job 2)

The workflow now has two jobs guarding two distinct closing-keyword bugs:

JobClassBlocking?
lint-closing-keywords4881 — GitHub’s native linker closes on a bare close/fix/resolve adjacent to #N, ignoring the Partially opt-outno (sticky warning)
lint-implements-unchecked-items#4677 — a full Implements:/Closes:/Fixes:/Resolves: #N closer paired with an unchecked acceptance box, closing still-open scope on mergeyes (fails the check)

Job 1 — lint-closing-keywords (native-linker contradiction, warning-only)

GitHub’s native issue-linker (a platform feature that runs before our custom close-merged-pr-issues.yml) closes an issue whenever a merged PR body contains a bare closing keyword — close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved — immediately followed by #N, anywhere in the body, even inside a negated sentence. The Partially implements: opt-out (#4677) only neutralises our workflow; the native linker ignores it. So a PR that deliberately ships a subset of an issue’s scope can still silently close the whole issue on merge (this happened in #4741).

This workflow warns the author before that happens:

  1. Reads the PR body (via env var — never inline ${{ }}, to avoid script injection).
  2. Set A = issue numbers matched by the native closing-keyword surface.
  3. Set B = issue numbers in a Partially <keyword>: #N opt-out.
  4. Contradiction = A ∩ B. If non-empty, it posts/refreshes a single sticky warning comment (marker <!-- lint-pr-closing-keywords -->) listing the affected numbers and how to rephrase.
  5. When the contradiction is resolved (e.g. the author edits the body), the sticky comment is deleted on the next edited/synchronize run.

-ing forms (closing #N, fixing #N) are safe — GitHub does not treat them as closers, and the regex cannot match them. Full-scope PRs using Closes #N with no Partially line are never flagged.

Job 2 — lint-implements-unchecked-items (#4677 class, BLOCKING)

The #4677 class is different and Job 1 does not catch it: a PR body that uses a full (non-Partially) closing keyword for #N — most commonly Implements: #N (our own close-merged-pr-issues.yml keyword), but also Closes:/Fixes:/Resolves: #N — while an acceptance-criteria checkbox for that same scope is still unchecked (- [ ]). On merge the issue is auto-closed even though its own checklist shows the work is not done. It recurred three times despite two CLAUDE.md warnings (#4583/#2742 on 2026-07-30; then #6410 → #6406 on 2026-08-18, where Implements: #6406 shipped with an unchecked - [ ] s3-v32-stage dedicated credential and #6406 auto-closed before the real remaining work (#6412) even opened).

This job fails the check (red X, so it must be fixed before merge — not just warned):

  1. Reads the PR body (via env var — never inline ${{ }}).
  2. Pre-scrubs every Partially <kw>: #N construct, then extracts full-closer target numbers. The pre-scrub and keyword surface (closes|fixes|resolves|implements, colon optional) mirror close-merged-pr-issues.yml exactly, so it fires on precisely the keywords that actually auto-close the issue.
  3. Counts unchecked - [ ] / * [ ] boxes. If the body has an acceptance-titled heading, the count is scoped to that section only — a ## Manual test checklist full of unchecked boxes is a normal worker-PR artefact (worker-issue.md Step 9) and must not trip the guard; without an acceptance heading it falls back to the whole body.
  4. If there is at least one full-closer target and at least one (in-scope) unchecked box, it posts/refreshes a sticky comment (marker <!-- lint-implements-unchecked-items -->) and the final gate step exits 1.

Never flagged: Partially implements: #N (the correct #4677 opt-out), a fully-checked acceptance section, or a full closer with no checkboxes at all. To clear a real flag: check the remaining box(es), or switch Implements: #N → Partially implements: #N and note the deferred scope.

The if: failure() notify step is placed before the blocking gate so the intentional exit 1 (a correctly-detected violation) does not raise a Discord ping or bug issue — only a genuine step error does.

Secrets / dependencies

  • P24_DISCORD_INFRA_SCRIPTS_ERRORS_WEBHOOK_URL (GH Actions secret) — used only by the if: failure() step per the p24-infra error-notification standard (Discord embed + bug issue).
  • No other credentials. Uses the default github.token for comment read/write.

Failure handling

Per the error-notification standard, any step failure fires the Notify on failure step: a Discord embed to the infra-scripts errors webhook and a gh issue create --label bug. A failed run means the guardrail did not evaluate that PR — a partially-scoped PR could still silently close its issue on merge until the workflow is fixed.

Manual test

Job 1:

  • Open a PR whose body has both Partially implements: #123 and a raw close #123 → warning posted.
  • A PR with Closes #123 and no Partially line → no warning.
  • Edit the body to separate the keyword from #N → sticky warning is removed on the next run.

Job 2:

  • PR body Implements: #123 + ## Acceptance criteria with an unchecked - [ ] → check fails.
  • Same body but Partially implements: #123 → passes (correct opt-out).
  • Same body but every acceptance box checked → passes.

The detection logic of both jobs is executed verbatim by tests/workflows/test_lint_pr_closing_keywords.py (the marked # --- … (BEGIN/END) --- shell blocks are extracted and run standalone), so a regex/awk edit is caught by the test suite.

  • .github/workflows/close-merged-pr-issues.yml — the custom linker whose opt-out this protects.
  • CLAUDE.md §Branching → Issue-closing keywords — the partial-scope convention (#4677).

Graph View