From 164d3498ea83b579f5919d7bab58d0f657447aa6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 17:12:44 +0000 Subject: [PATCH 1/2] feat(ci): block a PR whose body says `Part of #N` while also closing #N 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 Claude-Session: https://claude.ai/code/session_018WuTtyckQa1VcXwgd52JpN --- .github/workflows/lint.yml | 17 + .../partof-closing-keyword-guard.yml | 74 ++++ package.json | 1 + scripts/check-partof-closing-keyword.mjs | 343 ++++++++++++++++++ scripts/pm/check-half-states.mjs | 44 ++- 5 files changed, 468 insertions(+), 11 deletions(-) create mode 100644 .github/workflows/partof-closing-keyword-guard.yml create mode 100644 scripts/check-partof-closing-keyword.mjs diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index f2bf7b2be2..09bf34eaa8 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -193,6 +193,23 @@ jobs: - name: PM dispatch-gates self-test run: pnpm check:pm-dispatch-gates + # Part-of/closing-keyword guard self-test (#8476). The guard itself is a + # PR-scoped blocking check in its own workflow — it needs a pull request + # body to judge, which this job does not have — so what runs HERE is its + # self-test, which is the half with a verdict independent of any PR. + # Unconditional for the same reason as the two steps above: a self-test + # that runs only when someone remembers is a check whose coverage is a + # function of who remembered, and the failure it hides is quiet — a break + # in the verdict layer lands green and surfaces later as a card silently + # closed by the sentence written to keep it open. + # + # The self-test also pins the WIRING (the guard workflow still invokes + # the script, still subscribes to `edited`, still passes the body through + # `env:`), so unwiring the gate reddens here rather than going quiet. + # Imports one sibling module, reads two files; ~0.1s. + - name: Part-of closing-keyword guard self-test + run: pnpm check:partof-closing-keyword + # Docs/skills authoring guard (#2035 / ADR-0059): TS code blocks in # Markdown/MDX are not type-checked or ESLinted, so skills/ and # content/docs/ can drift back to teaching the bare `: Page = {}` literal diff --git a/.github/workflows/partof-closing-keyword-guard.yml b/.github/workflows/partof-closing-keyword-guard.yml new file mode 100644 index 0000000000..4ba0026d0b --- /dev/null +++ b/.github/workflows/partof-closing-keyword-guard.yml @@ -0,0 +1,74 @@ +# A PR that declares itself only `Part of #N` must not also tell GitHub to +# close `#N`. GitHub's closing-keyword parser matches the keyword plus the +# number and ignores the surrounding prose entirely — negations and modals +# included — so the sentence an author writes to PREVENT an auto-close is +# exactly what performs it on merge. That happened: a half-delivered card was +# closed `completed` two seconds after its PR merged, by its own warning +# sentence, and a closed card reads as finished, so it was found only by a +# post-merge inventory re-pull. The author wrote the warning correctly and +# still lost the card, which is why this is mechanical and not advice. +# +# The rule, the wording of the failure, and the code-stripping this depends on +# all live in `scripts/check-partof-closing-keyword.mjs` and the predicate it +# reuses; that header is authoritative, this file is the invocation. +# +# Sibling shape, deliberately copied rather than reinvented: the Duplicate Fix +# Guard is this repo's other PR-body-scoped blocking check, and it takes the +# same trigger set for the same measured reason. +name: Part-of Closing-Keyword Guard + +# `edited` is load-bearing, not decoration. The body is this check's whole +# input, and GitHub does not re-deliver a `pull_request` event when a body +# changes under any other activity type — while `rerun_failed_jobs` replays the +# frozen payload, so a re-run of a fixed body stays red forever. With `edited` +# subscribed, rewording the sentence fires a fresh event carrying the fresh +# body and the check goes green with no push and no re-run. +# +# No `merge_group:` trigger, and that is not an oversight: a merge-queue event +# carries no pull request and therefore no body, so this check has nothing to +# judge there. That also keeps it out of the required-context registry, whose +# entries must report on queue builds; see the script header on where branch +# protection fits. +on: + pull_request: + types: [opened, edited, reopened, synchronize] + +permissions: + contents: read + +concurrency: + group: partof-closing-keyword-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + partof-closing-keyword: + name: Part-of PR must not also close its card + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v7 + + - name: Setup Node.js + uses: actions/setup-node@v5 + with: + node-version-file: .nvmrc + + # The body reaches the script through `env:`, never through `${{ }}` + # inside the `run:` script. An expression interpolated into a shell line + # is substituted before bash ever sees it, so a PR body is arbitrary + # attacker-controlled text landing in a command; through `env:` it is + # inert data. The script's self-test pins this spelling. + # + # `PR_NUMBER` is not only for the message: GitHub renders a null body as + # an EMPTY value, so it is the witness that separates "this PR has an + # empty body" (a real, clean verdict) from "this step was handed nothing" + # (a wiring failure, which exits 2 and says so). + # + # No install step: the script imports one sibling module and reads no + # workspace package, so `node` on the pinned runtime is the whole + # toolchain it needs. + - name: A Part-of PR body may not carry a closing keyword for the same card + env: + PR_BODY: ${{ github.event.pull_request.body }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: node scripts/check-partof-closing-keyword.mjs diff --git a/package.json b/package.json index 5f61e8d8a1..c2c33863e3 100644 --- a/package.json +++ b/package.json @@ -47,6 +47,7 @@ "check:pm-skill-ratchet": "node scripts/pm/check-skill-line-ratchet.mjs --self-test && node scripts/pm/check-skill-line-ratchet.mjs", "check:pm-skill-id-lint": "node scripts/pm/check-skill-id-lint.mjs --self-test && node scripts/pm/check-skill-id-lint.mjs", "check:pm-dispatch-gates": "node scripts/pm/check-dispatch-gates.mjs", + "check:partof-closing-keyword": "node scripts/check-partof-closing-keyword.mjs --self-test", "check:adr-anchors": "node scripts/check-adr-anchors.mjs --self-test && node scripts/check-adr-anchors.mjs", "check:adr-links": "node scripts/check-adr-links.mjs --self-test && node scripts/check-adr-links.mjs", "check:adr-merge-approval": "node scripts/check-adr-merge-approval.mjs --self-test && node scripts/check-adr-merge-approval.mjs", diff --git a/scripts/check-partof-closing-keyword.mjs b/scripts/check-partof-closing-keyword.mjs new file mode 100644 index 0000000000..0bdfe6bc91 --- /dev/null +++ b/scripts/check-partof-closing-keyword.mjs @@ -0,0 +1,343 @@ +#!/usr/bin/env node +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * check:partof-closing-keyword — the PR-scoped BLOCKING half of the half-state + * sweep's H7: a pull request may not declare itself only `Part of #N` while + * also telling GitHub to close that same `#N`. + * + * node scripts/check-partof-closing-keyword.mjs # judge this PR (CI) + * node scripts/check-partof-closing-keyword.mjs --self-test # verify it offline + * + * ⚠️ Repo paths are named UNQUOTED in this header on purpose. See the last + * section for the measurement that forces it. + * + * ## The defect this blocks + * + * A half-delivered card was closed `completed` two seconds after its PR merged, + * although that PR body opened with the words Part of and carried an explicit + * warning against auto-closing the card. The warning sentence read, verbatim: + * "…the PM should close #8131 deliberately once #8136 lands." GitHub's + * closing-keyword parser matches the keyword plus the number and ignores every + * bit of the surrounding prose — the modal, the negation in the clause before + * it, the whole paragraph arguing the card must stay open. The sentence written + * to PREVENT the auto-close is what performed it, and a closed card reads as + * finished, so the loss was found only by a post-merge inventory re-pull. + * + * Habit is what failed there: the author wrote the warning correctly and still + * lost the card. Only a mechanical comparison at PR time catches a mistake that + * reads as natural English. + * + * ## Why this is a SEPARATE check and not a mode flip of the sweep + * + * scripts/pm/check-half-states.mjs enumerates H1–H7 and is deliberately + * report-only: H1–H6 are facts about a live, shared BOARD, and failing an + * unrelated PR over board state punishes the wrong actor. H7 is the one item + * that is a fact about THE PR BEING CHECKED, so gating on it here contradicts + * nothing the sweep argues. H7 also stays in the sweep — patrol coverage of the + * same fact, on PRs whose CI predates this gate — and the sweep keeps its own + * exit-0-always contract untouched. Two consumers, one predicate, no fork: the + * verdict below is the sweep's exported h7PartOfWithClosingKeyword, called. + * + * The sweep is also unrunnable in most agent containers (its transport section + * measures three container classes, only one of which can reach the API), so + * patrol alone guards nothing in the place where PR bodies are written. + * + * ## Where the body comes from, and why it is not an API read + * + * The wiring workflow hands this script the body through the ENVIRONMENT, out + * of the pull_request event payload. Two consequences worth stating, because + * the alternative (a live GET of the PR) is the obvious-looking design: + * + * 1. There is no HTTP call in the judging path, so the whole "API hiccup" + * failure class does not exist here — not by policy, by construction. The + * only inputs are two environment variables. + * 2. A payload is a frozen snapshot, and this repo has measured what that + * costs when the fix for a red gate is an edit GitHub does not re-deliver: + * a re-run replays the SAME payload, so the run stays red forever. That is + * why the wiring workflow subscribes to the `edited` activity type — an + * author who rewords the sentence gets a fresh event with a fresh payload + * and a green run, with no push and no re-run. The identical trigger set + * and the identical reason are already in the duplicate-fix guard, the + * repo's other PR-body-scoped blocking check; this follows it rather than + * inventing a second shape. + * + * The residual hole is named rather than hidden: `rerun_failed_jobs` on a run + * whose body has since been fixed replays the stale body and stays red. The + * remedy is to edit the body (which fires a new run), not to re-run. + * + * ## Exit codes — and why an empty body is a VERDICT, not a skip + * + * 0 judged, clean. + * 1 judged, contradiction found. The PR is red until the body is reworded. + * 2 NOT WIRED — no PR context in the environment at all. A usage/wiring + * failure, never a statement about any PR. + * + * The split matters in both directions. A gate that cannot read its input has + * verified nothing, and exiting 0 there is the anti-pattern this repo keeps + * paying for: a check that skips silently reads as "no violations". But the + * inverse is a real cost too — a mis-wired gate must not read as "this PR is + * guilty", because it would be red on every PR at once for something no author + * did. Exit 2 says which of the two it is, in its own words. + * + * A PR with an EMPTY body is neither of those. It demonstrably contains no + * Part-of declaration, so it cannot contain the contradiction, and passing it + * is a real judgment of a real (if terse) input. `PR_NUMBER` is what tells the + * two apart: GitHub renders a null body as an EMPTY environment value, so + * "empty body" and "variable absent" are indistinguishable on their own. When + * either variable is present the run is wired, and an absent body is read as + * the empty string it is. + * + * ## What this gate does NOT do + * + * It does not judge a closing keyword bound to a DIFFERENT card than the one + * the body says it is only part of. `Part of #A` plus a real `Fixes #B` is a + * correct, common shape — the actionable half of one card landing while another + * is genuinely closed — and the predicate binds per issue number precisely so + * that stays green. + * + * It also does not decide branch protection. This publishes a red check run; + * whether that check run becomes a REQUIRED context is a settings change no + * agent seat can make (the protection endpoint answers 403 here) and, per the + * required-context registry convention, one that carries a maintainer ruling. + * The duplicate-fix guard sits in exactly the same position. + * + * ## Why the paths above are unquoted + * + * The dispatch-gates derivation resolves a check family to its script file and + * scans THAT FILE for quoted path literals — its watch hints — and the scan + * reads comments and self-test fixtures too (an open card proposes masking + * them; it has not landed). Written the ordinary way, with each path in + * backticks, a header like this one yields a hint per path and fabricates + * MATCHED leads for cards that touch none of this. So paths are named unquoted + * in prose here, and the only quoted paths in this file are the real inputs + * below. The self-test's fixtures are safe by a different mechanism, worth + * knowing rather than re-deriving: a hint must be a single unbroken token, and + * every fixture string here is prose with spaces, so none of them can become + * one. + */ + +import { existsSync, readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import process from 'node:process'; + +import { h7PartOfWithClosingKeyword } from './pm/check-half-states.mjs'; + +const ROOT = new URL('..', import.meta.url).pathname; + +/** The predicate's home — this gate reuses it and must move when it moves. */ +const PREDICATE_SOURCE = 'scripts/pm/check-half-states.mjs'; + +/** The wiring that gives this gate a PR to judge. */ +const WIRING_WORKFLOW = '.github/workflows/partof-closing-keyword-guard.yml'; + +export const EXIT_CLEAN = 0; +export const EXIT_CONTRADICTION = 1; +export const EXIT_NOT_WIRED = 2; + +/** + * The PR context, or null when this process was handed none. + * + * Presence, not truthiness: an empty body is a legitimate input (see the exit + * codes section) and `PR_NUMBER` is the witness that the workflow really ran + * this step, whatever the body rendered to. + */ +export function readPrContext(env) { + const wired = Object.hasOwn(env, 'PR_BODY') || Object.hasOwn(env, 'PR_NUMBER'); + if (!wired) return null; + return { number: String(env.PR_NUMBER ?? '').trim(), body: env.PR_BODY ?? '' }; +} + +/** + * The verdict: `{ exit, lines }`, pure, so the self-test drives it directly. + * + * The finding sentence is the sweep predicate's own, unedited. It already + * carries the three approved rewordings, and keeping ONE wording source is what + * stops the gate's advice and the protocol's advice from drifting apart. + */ +export function judge(ctx) { + if (ctx === null) { + return { + exit: EXIT_NOT_WIRED, + lines: [ + 'check:partof-closing-keyword: NOT WIRED — neither PR_BODY nor PR_NUMBER is set, so this run', + 'was handed no pull request and judged nothing. This is a wiring or usage failure, NOT a', + 'verdict: it says nothing about whether any PR body contradicts itself, and no author caused it.', + '', + `Fix: run it from the workflow that supplies the context (${WIRING_WORKFLOW}), or locally with`, + ' PR_BODY="$(cat some-body.md)" node scripts/check-partof-closing-keyword.mjs', + ], + }; + } + + const where = ctx.number ? `PR #${ctx.number}` : 'this PR'; + const contradiction = h7PartOfWithClosingKeyword({ body: ctx.body }); + if (!contradiction) { + const what = ctx.body.trim() === '' ? 'has an empty body, which can carry no' : 'carries no'; + return { + exit: EXIT_CLEAN, + lines: [`✓ check:partof-closing-keyword: ${where} ${what} Part-of/closing-keyword contradiction.`], + }; + } + + return { + exit: EXIT_CONTRADICTION, + lines: [ + `::error::${where} contradicts itself: ${contradiction}`, + '', + `✗ check:partof-closing-keyword: ${where} contradicts itself.`, + '', + ` ${contradiction}`, + '', + ' Why this is blocking rather than advisory: the card closes SILENTLY on merge, and a closed', + ' card reads as finished — the one incident behind this gate was found only by a post-merge', + ' inventory re-pull. Editing the body re-runs this check; no push and no re-run are needed.', + ], + }; +} + +// --------------------------------------------------------------------------- +// Self-test — the verdict layer, the exit-code contract, and the wiring. +// +// The predicate itself is the sweep's and is tested there; what is pinned here +// is (a) that this gate really delegates to it rather than carrying a second +// copy of the rule, (b) the three code-formatting arms as MEASURED live, and +// (c) that the wiring which feeds this gate still exists and still carries the +// activity type without which a reworded body can never go green. +// --------------------------------------------------------------------------- + +function selfTest() { + const cases = []; + const t = (name, actual, expected) => cases.push([name, actual, expected]); + const verdict = (body, number = '1') => judge({ number, body }); + + // --- The measured arms. All three were read live on one throwaway PR, in one + // body, at one moment, with the PR OPEN and unmerged: the plain-prose target + // gained a closing link within seconds, and the fenced and inline targets + // gained none. The predicate strips code before scanning; these three cases + // are that measurement, kept executable. + t( + 'plain prose beside a Part-of declaration is a finding (the incident specimen)', + verdict('Part of #8131 — the PM should close #8131 deliberately once #8136 lands.').exit, + EXIT_CONTRADICTION, + ); + t( + 'a closing keyword inside a FENCED block is not a finding (measured 2026-08-13)', + verdict('Part of #8520\n\n```text\nFixes #8520\n```\n').exit, + EXIT_CLEAN, + ); + t( + 'a closing keyword inside an INLINE span is not a finding (measured 2026-08-13)', + verdict('Part of #8521 — the dispatch asked for `Fixes #8521`, which is deliberately not used.').exit, + EXIT_CLEAN, + ); + + // --- The shapes that must stay green, so the gate does not tax correct PRs. + t( + 'Part of one card while genuinely closing another is clean', + verdict('Part of #8247\n\nFixes #8245').exit, + EXIT_CLEAN, + ); + t('an ordinary closing PR with no Part-of declaration is clean', verdict('Fixes #8476').exit, EXIT_CLEAN); + t( + 'a Part-of PR with no closing keyword at all is clean', + verdict('Part of #8476 — the decision half stays open.').exit, + EXIT_CLEAN, + ); + t( + 'the word closing is not a closing keyword', + verdict('Part of #8284 — merging this and closing #8284 would drop the severe half.').exit, + EXIT_CLEAN, + ); + + // --- Delegation, not a second copy of the rule. Every body above must get + // the same verdict from this gate as from the shipped predicate; a fork would + // pass the cases above and drift from the sweep on the next one. + const bodies = [ + 'Part of #1 close #1', + 'Part of #1\n\nFixes #2', + 'Part of #1 — `Fixes #1`', + 'Fixes #1', + '', + 'Part of #1 Part of #2 resolves #2', + ]; + t( + 'the verdict is exactly the shipped predicate over every fixture (no forked rule)', + bodies.every( + (body) => + (judge({ number: '1', body }).exit === EXIT_CONTRADICTION) === + (h7PartOfWithClosingKeyword({ body }) !== null), + ), + true, + ); + + // --- The failure message must carry the approved rewordings. This is the + // card's own requirement and the reason the predicate's sentence is reused + // verbatim: an author reading a red check gets the fix, not just the verdict. + const failed = verdict('Part of #8131 — close #8131 once the rest lands.').lines.join('\n'); + t('the failure names the "not addressed here" rewording', failed.includes('is not addressed here'), true); + t('the failure names the "out of scope" rewording', failed.includes('out of scope: #8131'), true); + t('the failure names the backtick escape', failed.includes('backticks'), true); + t('the failure is annotated for the GitHub UI', failed.includes('::error::'), true); + + // --- Empty body: a verdict about a real input, and it must SAY so rather + // than look like a run that judged nothing. + const empty = verdict(''); + t('an empty body is clean', empty.exit, EXIT_CLEAN); + t('an empty body says it was judged, not skipped', empty.lines.join('\n').includes('empty body'), true); + + // --- Wiring absent: never clean, never a verdict about a PR. + const unwired = judge(readPrContext({})); + t('no PR context at all exits NOT WIRED', unwired.exit, EXIT_NOT_WIRED); + t('NOT WIRED says it judged nothing', unwired.lines.join('\n').includes('judged nothing'), true); + t('NOT WIRED does not read as a clean board', unwired.lines.join('\n').includes('✓'), false); + + // --- Context reading: presence, not truthiness. + t('a present but empty body is still wired', readPrContext({ PR_BODY: '' })?.body, ''); + t('the PR number alone is enough to be wired', readPrContext({ PR_NUMBER: '42' })?.number, '42'); + t('an absent body reads as the empty string', readPrContext({ PR_NUMBER: '42' })?.body, ''); + t('an unset environment is not wired', readPrContext({}), null); + + // --- The wiring itself. A gate whose workflow step is deleted or whose + // trigger loses the edit activity is not a weaker gate, it is a silent one. + const wiringPath = join(ROOT, WIRING_WORKFLOW); + const wiring = existsSync(wiringPath) ? readFileSync(wiringPath, 'utf8') : ''; + t('the wiring workflow exists', wiring !== '', true); + t('the wiring workflow runs this script', wiring.includes('node scripts/check-partof-closing-keyword.mjs'), true); + t( + 'the wiring subscribes to body edits, so a reworded body can go green', + /types:\s*\[[^\]]*\bedited\b[^\]]*\]/.test(wiring), + true, + ); + t('the wiring passes the body through env, never through shell interpolation', /env:[\s\S]{0,200}?PR_BODY:/.test(wiring), true); + t('the wiring passes the PR number too (the wired-ness witness)', /PR_NUMBER:/.test(wiring), true); + + // --- The predicate source this gate reuses must still be there to reuse. + t('the predicate source exists', existsSync(join(ROOT, PREDICATE_SOURCE)), true); + + let failedCount = 0; + for (const [name, actual, expected] of cases) { + const ok = JSON.stringify(actual) === JSON.stringify(expected); + if (!ok) failedCount++; + console.log(` ${ok ? '✓' : '✗'} ${name}${ok ? '' : ` (got ${JSON.stringify(actual)}, want ${JSON.stringify(expected)})`}`); + } + if (failedCount) { + console.error(`✗ check-partof-closing-keyword self-test: ${failedCount} of ${cases.length} case(s) failed.`); + process.exit(1); + } + console.log(`✓ check-partof-closing-keyword self-test: ${cases.length} cases pass.`); +} + +// The basename comparison, as in the sweep: this file is imported by nothing +// today, but a future importer must not trigger a judgment as a side effect. +const isMain = process.argv[1] && import.meta.url.endsWith(process.argv[1].split('/').pop()); +if (isMain) { + if (process.argv.includes('--self-test')) { + selfTest(); + } else { + const result = judge(readPrContext(process.env)); + const emit = result.exit === EXIT_CLEAN ? console.log : console.error; + for (const line of result.lines) emit(line); + process.exit(result.exit); + } +} diff --git a/scripts/pm/check-half-states.mjs b/scripts/pm/check-half-states.mjs index 55b5c19daf..326c8435fd 100644 --- a/scripts/pm/check-half-states.mjs +++ b/scripts/pm/check-half-states.mjs @@ -108,10 +108,25 @@ * flags the exact shape a careful author writes when EXPLAINING that they * deliberately did not use the keyword. * - * Reading 4 is measured for inline spans; fenced blocks are stripped by the - * same rule but were NOT independently measured — stated so a later reading can - * revise it rather than inherit it as fact. Both directions of that choice are - * documented at `stripMarkdownCode`. + * 5. keyword + `#N` inside a FENCED BLOCK does not close either — and the + * closing link is created at PR-OPEN time, not at merge. Both were settled + * by one controlled reading on 2026-08-13 (#8476 step 1). A throwaway PR + * (#8523, empty commit, closed unmerged) carried three arms in ONE body at + * one moment: `Fixes #8520` inside a fenced block, `Fixes #8521` inside an + * inline span, and a plain-prose `Fixes #8522`. Read seconds after that PR + * opened, `closed_by_pull_requests` was EMPTY on #8520 and on #8521, and + * carried #8523 (state OPEN) on #8522. The prose arm is the positive + * control that makes the two nulls readable at all: without it, "no link + * on the fenced arm" cannot be told apart from "closing links only + * materialize on merge". + * + * So the strip rule is a false negative in neither direction, and the merge is + * no part of the mechanism: the contradiction exists, and is fixable, from the + * moment a PR opens. That is what lets the same predicate back a PR-scoped + * BLOCKING gate (`scripts/check-partof-closing-keyword.mjs`, which imports + * `h7PartOfWithClosingKeyword` from here) as well as this report-only sweep. + * H7 stays in the sweep regardless — patrol coverage of PRs whose CI predates + * that gate — and nothing about this file's report-only contract changes. * * Scope of the remedy that lands HERE: this is the report-only detector, not a * suppression. Suppressing at source means telling authors not to put a closing @@ -320,13 +335,20 @@ function partOfRe() { * report every author who correctly explains that they did NOT use the keyword * — turning the guard into noise on precisely the careful PRs. * - * Fenced blocks are stripped by the same rule but were NOT independently - * measured. The choice is deliberate and its cost is stated rather than hidden: - * if GitHub does fire inside fences, this is a false NEGATIVE — the direction - * this card exists to prevent. It is taken because a PR body routinely quotes - * whole other bodies, templates and logs in fences, and scanning those would - * bury real findings under quoted text. A single live reading of a fenced - * `Fixes #N` against `closed_by_pull_requests` settles it either way. + * MEASURED for fenced blocks too, as of 2026-08-13 (#8476 step 1): a throwaway + * PR (#8523) carried `Fixes #8520` inside a fence, `Fixes #8521` inside an + * inline span and a plain-prose `Fixes #8522` in ONE body, and seconds after it + * opened — unmerged — `closed_by_pull_requests` was empty on the fenced and + * inline targets while the prose target already carried the link. The prose arm + * is the positive control: it proves the link mechanism was live and readable + * during the reading, so the two nulls mean "the parser does not fire here" and + * not "links appear only on merge". + * + * That closes the one unknown this doc used to carry (the fence rule was + * previously taken on the argument that PR bodies routinely quote whole other + * bodies, templates and logs, and scanning those would bury real findings under + * quoted text). Both spellings are now measured, so stripping is correct rather + * than merely reasonable, and a blocking gate may rely on it. * * Lines are replaced by empty strings rather than deleted so that nothing is * spliced together across a stripped block into an accidental match. From 2fe178e3737653e969deb0046bf8382eafa28b33 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 17:36:26 +0000 Subject: [PATCH 2/2] fix(ci): the guard job needs node, not a package manager 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 Claude-Session: https://claude.ai/code/session_018WuTtyckQa1VcXwgd52JpN --- .../partof-closing-keyword-guard.yml | 24 +++++++++++++++++-- scripts/check-partof-closing-keyword.mjs | 19 +++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/.github/workflows/partof-closing-keyword-guard.yml b/.github/workflows/partof-closing-keyword-guard.yml index 4ba0026d0b..5351c442d2 100644 --- a/.github/workflows/partof-closing-keyword-guard.yml +++ b/.github/workflows/partof-closing-keyword-guard.yml @@ -48,10 +48,30 @@ jobs: - name: Checkout repository uses: actions/checkout@v7 + # Pinned to the same major and spelling as every other setup-node in this + # repo, and for a measured reason rather than tidiness. The first draft of + # this file used `actions/setup-node@v5` with `node-version-file`, and the + # job died IN THIS STEP, before the script ever ran: + # + # ##[error]Unable to locate executable file: pnpm. + # + # v5 defaults `package-manager-cache: true` (the run log echoes it as an + # input), which 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 — the script is dependency-free and imports + # one sibling module — so pnpm is not on PATH and the action hard-errors. + # The failure is worth naming because nothing in the step that failed + # mentions pnpm: it is an implicit default of the action, invisible in the + # workflow source, and the misleading first read is that the `run:` line + # below invoked a package manager. It does not, and never did. + # + # The adr-merge-approval gate is the known-good shape this now matches + # exactly: checkout, setup-node, one `node scripts/check-*.mjs` call, no + # install and no corepack. It is green on this repo today. - name: Setup Node.js - uses: actions/setup-node@v5 + uses: actions/setup-node@v7 with: - node-version-file: .nvmrc + node-version: '22' # The body reaches the script through `env:`, never through `${{ }}` # inside the `run:` script. An expression interpolated into a shell line diff --git a/scripts/check-partof-closing-keyword.mjs b/scripts/check-partof-closing-keyword.mjs index 0bdfe6bc91..b76b239f0e 100644 --- a/scripts/check-partof-closing-keyword.mjs +++ b/scripts/check-partof-closing-keyword.mjs @@ -312,6 +312,25 @@ function selfTest() { t('the wiring passes the body through env, never through shell interpolation', /env:[\s\S]{0,200}?PR_BODY:/.test(wiring), true); t('the wiring passes the PR number too (the wired-ness witness)', /PR_NUMBER:/.test(wiring), true); + // This gate installs no package manager, and that is a property of the JOB, + // not a preference. Measured the hard way: the first draft used a setup-node + // major whose package-manager-cache default is on, which reads packageManager + // out of package.json and shells out to pnpm to find its store. The job died + // in the setup step with "Unable to locate executable file: pnpm", before the + // script ran at all — a failure that names a tool the workflow source never + // mentions. The wording of the pin is deliberately about the WORKFLOW naming + // a package manager, since a comment explaining the incident has to be able + // to say the word; only the executable lines are scanned. + const wiringCommands = wiring + .split('\n') + .filter((line) => !/^\s*#/.test(line)) + .join('\n'); + t( + 'the guard job invokes no package manager (it needs node and nothing else)', + /\b(pnpm|corepack|yarn|npm)\b/.test(wiringCommands), + false, + ); + // --- The predicate source this gate reuses must still be there to reuse. t('the predicate source exists', existsSync(join(ROOT, PREDICATE_SOURCE)), true);