Skip to content

feat(ci): block a PR that declares itself only part of a card while also closing it - #8527

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-8476-partof-blocking-check
Aug 13, 2026
Merged

feat(ci): block a PR that declares itself only part of a card while also closing it#8527
hotlong merged 2 commits into
mainfrom
claude/issue-8476-partof-blocking-check

Conversation

@hotlong

@hotlonghotlong commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Fixes#8476

Promotes the half-state sweep's H7 from a report-only patrol item to a PR-scoped blocking check — after settling, live, the one unknown its predicate carried.

Step 1 first: the fenced-code reading, settled

One controlled reading: three arms in ONE throwaway PR body, at one moment, with that PR open and unmerged (#8523, empty commit, never merged; it and its three targets are now shut).

armspelling in the bodyclosed_by_pull_requests on the target
Afenced block#8520 — empty
Binline code span#8521 — empty
Cplain prose#8522 — one reference, PR #8523, state OPEN

Read seconds after that PR opened, and re-read unchanged ~15 minutes later. Two facts fall out:

  1. GitHub's closing-keyword parser does not fire inside a fence. So stripMarkdownCode's fence rule is correct rather than merely reasonable — it is not a false negative, it needs no revision, and a blocking gate may rely on it.
  2. The closing link is created at PR-open time, not at merge. The dispatch's stated mechanism assumption holds: no merge is needed to read the link, so the contradiction is visible — and cheap to fix — from the moment a PR opens.

Arm C is what makes A and B readable at all. Without a positive control, "no link on the fenced arm" cannot be told apart from "closing links only materialize on merge".

Recorded in scripts/pm/check-half-states.mjs beside the inline-span reading it extends: in the header's reading list, and at stripMarkdownCode itself, where the old "NOT independently measured" caveat stood.

Step 2: the blocking check

scripts/check-partof-closing-keyword.mjs — a new, separate, PR-scoped gate. It fails a PR whose body says it is only Part of some card while also carrying a closing keyword bound to that same card.

  • Reuses the predicate, does not fork it. It imports h7PartOfWithClosingKeyword from the sweep and reuses its finding sentence verbatim, so the gate's advice and the protocol's advice cannot drift. The sentence already carries the three approved rewordings ("card N is not addressed here" / "out of scope: card N" / put the keyword in backticks), and a self-test case pins each one.
  • Separate check, not a mode flip. H1 through H6 stay report-only (they are facts about a shared board, and failing an unrelated PR over board state punishes the wrong actor). H7 also stays in the sweep, for patrol coverage of PRs whose CI predates this gate. Nothing existing is weakened; the sweep's exit-0-always contract is untouched.
  • Blast radius nil for correct PRs. The predicate binds per card number, so Part of one card while genuinely closing another stays green, as does explanatory prose that puts a keyword in backticks.

Wiring

A standalone workflow, modelled on the Duplicate Fix Guard — this repo's other PR-body-scoped blocking check — rather than a new job in the pr-automation family. Measured reasons:

  • edited is load-bearing. The body is the whole input, and the pr-automation family does not subscribe to edited; a re-run replays the frozen payload, so without it an author who reworded the sentence would stay red forever. Adding edited to that shared trigger set would instead have made two label-writing jobs re-run on every body edit, which that file's header argues against at length.
  • The body arrives through env:, never interpolated into a run: line — a PR body is arbitrary author-controlled text. A self-test case pins the spelling.
  • No API read in the judging path, so the whole "API hiccup" failure class does not exist here by construction.
  • The job installs no package manager: checkout, actions/setup-node@v7 pinned to Node 22, one node call. That is the ADR merge-approval gate's recipe, this repo's known-good shape for a dependency-free check script, and it is now matched exactly. This was measured the hard way — see the patch round below.
  • No merge_group: trigger: a queue event carries no pull request and so no body. That is also why this is not added to the required-context registry, whose entries must report on queue builds — and whose entries each carry a maintainer ruling. This publishes a red check run; whether it becomes a required context is a settings change no agent seat can make. The Duplicate Fix Guard sits in exactly the same position.

lint.yml runs the new gate's self-test unconditionally, following the PM dispatch-gates precedent: a self-test that runs only when someone remembers is a check whose coverage is a function of who remembered. That self-test also pins the wiring itself — the workflow still invokes the script, still subscribes to body edits, still passes the body through env:, and still names no package manager — so unwiring the gate reddens rather than going quiet.

Error paths (the fail-open vs fail-closed call, stated)

Sibling checks in this repo treat an unread input as a failure, never a pass. This one splits the two cases that get conflated:

  • exit 1 — judged, contradiction found.
  • exit 2 — NOT WIRED: no PR context at all. A wiring or usage failure that says so in its own words. It must not read as "clean" (a gate that cannot read its input has verified nothing) and equally must not read as "this PR is guilty" (it would be red on every PR at once, for something no author did).
  • exit 0 on an empty body is a verdict, not a skip — an empty body demonstrably carries no Part of declaration. PR_NUMBER is the witness that separates it from "handed nothing", since GitHub renders a null body as an empty value.

Patch round: the runtime step (head 2fe178e)

The first head went red in CI, and the failure is worth recording because it names a tool this workflow never mentions:

##[error]Unable to locate executable file: pnpm.

The job died in Setup Node.js, before the script ran. actions/setup-node@v5 defaults package-manager-cache: true (the run log echoes it as a resolved input); that reads packageManager: pnpm@… out of package.json and shells out to pnpm to locate the store to cache. This job installs no package manager on purpose, so pnpm is not on PATH and the action hard-errors. The misleading first read is that the run: line invoked a package manager — it did not, and never did; the reference is an implicit default, invisible in the workflow source.

Fixed by matching the repo's own known-good recipe exactly (setup-node@v7, node-version: '22', no corepack, no install). A new self-test case pins that the guard job names no package manager in any executable line, so this cannot return quietly.

Verification

  • New gate self-test: 28 cases, green. The sweep's own self-test: 79 cases, still green.
  • All four live paths exercised end to end: contradiction (exit 1), clean (exit 0), empty body (exit 0), unwired (exit 2).
  • Three reverse verifications, each with its direction predicted first, then restored: dropping edited from the trigger reddens exactly the wiring case; reintroducing a package-manager invocation reddens exactly the new package-manager case; disabling fence-stripping in the shared predicate reddens exactly the fenced arm — and leaves the delegation case green, because both sides of that comparison call the same mutated predicate. The delegation case proves "no fork"; the fence case proves the rule.
  • Re-derived gates against the real changed paths and ran the union green, including check:type-check-debt after building the ledgered packages' dependency closure the way lint.yml does.
  • The gate is green on its own PR in real CI, judging this body through the real event payload: ✓ check:partof-closing-keyword: PR #8527 carries no Part-of/closing-keyword contradiction. Whole job, 8 seconds.

No changeset: this PR ships CI tooling and releases nothing, so it carries skip-changeset.


Generated by Claude Code

The half-state sweep's H7 becomes a PR-scoped blocking check, reusing the
sweep's exported predicate rather than forking it. H1-H6 stay report-only
(they are facts about a shared board), and H7 stays in the sweep too for
patrol coverage.
Settles the fence question first, with one live controlled reading: fenced
and inline closing keywords do not create a closing link, plain prose does,
and the link exists at PR-open time rather than at merge. Recorded next to
the inline-span measurement it extends.
Part of the wiring: a standalone workflow subscribed to `edited` so a
reworded body can go green without a push, and a lint.yml step that runs the
new gate's self-test.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018WuTtyckQa1VcXwgd52JpN
@vercel

vercelBot commented Aug 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 13, 2026 5:37pm

Request Review

@github-actionsgithub-actionsBot added size/m ci/cd dependencies Pull requests that update a dependency file labels Aug 13, 2026
@hotlonghotlong added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 13, 2026 — with Claude
The job died in Setup Node.js, before the script ran:
##[error]Unable to locate executable file: pnpm.
actions/setup-node@v5 defaults package-manager-cache: true, reads
packageManager: pnpm@... out of package.json and shells out to pnpm to
locate the store. This job installs no package manager on purpose, so
pnpm is not on PATH. Nothing in the workflow source mentions pnpm, which
is what made the first read of the error misleading.
Now mirrors adr-merge-approval exactly -- checkout, setup-node@v7 with
node-version '22', one node call, no install and no corepack -- which is
the repo's known-good shape for a dependency-free check script. A new
self-test case pins that the guard job names no package manager, so this
cannot come back quietly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018WuTtyckQa1VcXwgd52JpN
@hotlong
hotlong marked this pull request as ready for review August 13, 2026 17:49
@hotlong
hotlong added this pull request to the merge queueAug 13, 2026
Merged via the queue into main with commit abdfd29Aug 13, 2026
29 checks passed
@hotlong
hotlong deleted the claude/issue-8476-partof-blocking-check branch August 13, 2026 18:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cddependenciesPull requests that update a dependency filesize/lskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Promote the Part-of/closing-keyword contradiction (H7) into a PR-scoped blocking check — and settle the fenced-code reading first

2 participants

@hotlong@claude