From 829005aa8dc79648d80ec87b96fc3df65e5e7700 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 6 Aug 2026 12:38:49 +1200 Subject: [PATCH 1/4] docs(changelog): generation covers 0.6.0.0 now, not "from the next release on" #113 moved the changelog from per-PR entries to release-time generation. The policy is right; the sentence describing it put 0.6.0.0 on the wrong side of the line. "Everything up to and including == 0.6.0.0 is hand-written and now frozen; from the next release on, each section is generated" reads as: v6 is finished, generation starts with v7. Generation is in effect now and 0.6.0.0 is the release it applies to. The misreading is not hypothetical - an agent hit it this week and concluded that whatever currently sits under == 0.6.0.0 is what v6 ships with. The trap is that "frozen" is doing two jobs in one sentence. It is a statement about text in the file (leave it alone) and it reads as a statement about a release (settled, quotable). The rewrite splits them: a section's state follows from whether its release has SHIPPED, and 0.6.0.0 has not, so its contents are working text that release-time generation will replace. Both wrong readings are named and denied explicitly, because a reader who has already formed one will not give it up to an implication. Also states the exception that previously had to be inferred. "Do not add entries in a feature PR" does not obviously permit a correction, and #198 made one - the Kafka client entry said 3.9.1 after the pom moved to 3.9.2. The test is now written down: changing an existing claim to be true is allowed, adding information about a change is the generator's job. Three locations carried the phrase (AGENTS.md's Changelog section and its "where things live" row, and the CHANGELOG header NOTE). A fourth, in docs/inflight/release-0600-blockers.md, had already copied it. README.adoc is unaffected - it stopped embedding the changelog in #113 and only links to it. Separately, the same section claimed the PR Checklist changelog gate "should never fire ... since PRs no longer touch the file". That was false when written: #51, #57, #105 and #106 all modify CHANGELOG.adoc and all predate the policy. The gate also does not enforce the policy in either direction - it only fails UNCITED entries, so #57's cited entries would pass while violating it, and it cannot tell an edit from an addition, so a correction like #198's trips it and needs the changelog-ref: N/A opt-out. The gate is left alone deliberately, with the reasoning in docs/inflight/ci-changelog-gate-scope.md. Rejecting every addition would fail the one edit the policy permits, since "adds an entry" and "corrects an entry" are both +* lines - the gate's own header records that fuzzy-matching removed bullets against added ones was built, and abandoned as its subtlest code. And the strict version's only escape is a self-declared PR-body line, so it would buy no enforcement over the written rule while putting an opt-out on every legitimate correction, and would block four PRs that followed the rule that was live when they were opened. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01RtNUsxokE9g2pSEjBHZqNA --- AGENTS.md | 59 ++++++++++++++++++++---- CHANGELOG.adoc | 2 +- docs/inflight/ci-changelog-gate-scope.md | 48 +++++++++++++++++++ docs/inflight/release-0600-blockers.md | 9 ++-- 4 files changed, 103 insertions(+), 15 deletions(-) create mode 100644 docs/inflight/ci-changelog-gate-scope.md diff --git a/AGENTS.md b/AGENTS.md index 882d7996f..63d84a91a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,7 +22,7 @@ owned that content all along.) | **`docs/SELF_HOSTED_RUNNER.md`** | Setup and operation of the self-hosted highcpu runner | CI policy, which lives in the workflows | | **`src/docs/development/upstream-map.yaml`** | **Source of truth** for fork↔upstream mapping (branch/PR → upstream issue/PR, status) | Editorial opinion - that is the `.adoc` beside it | | **`src/docs/development/upstream-pr-analysis.adoc`** | Editorial analysis of upstream PRs: rankings, verdicts, merge order | Facts - when they disagree, the manifest wins | -| **`CHANGELOG.adoc`** | Release notes. Frozen up to `== 0.6.0.0`; later sections are generated at release time from the commit log. `README.adoc` links to it and no longer embeds it | Anything invisible to users or operators - and **not** a per-PR chore: do not add entries in a feature PR | +| **`CHANGELOG.adoc`** | Release notes. Sections for **shipped** releases are frozen; the section for the release being cut - **`== 0.6.0.0` included** - is regenerated at release time from the commit log, so what is under it today is working text, not the notes v6 will publish. `README.adoc` links to it and no longer embeds it | Anything invisible to users or operators - and **not** a per-PR chore: a PR never *adds* an entry. Correcting a factual error in an existing one is the only edit it may make | Rule of thumb: **is it happening now** → `docs/inflight/`; **should happen later** → `refactoring.md`; **already happened** → `CHANGELOG.adoc` or `docs/solutions/`. @@ -196,12 +196,35 @@ stagnation (Class 2, W4's prey), drain overruns, and record loss/duplication. Ta ## Changelog -`CHANGELOG.adoc` holds the release notes. **Nothing about it is a per-PR chore.** Do not add entries -in a feature PR, and do not maintain an `== Unreleased` section. Everything up to and including -`== 0.6.0.0` is hand-written and now frozen; from the next release on, each section is **generated at -release time from the commit log** and then frozen in turn. +`CHANGELOG.adoc` holds the release notes. **Nothing about it is a per-PR chore.** -This removes the file that every PR used to touch - it appeared in 30 of the last 30 master commits, +**Release-time generation is in effect now, and it covers `0.6.0.0` itself.** "Frozen" below is a +statement about *text already written in the file* - leave it alone - and never a claim that some +release's published notes are settled. What state a section is in follows from whether its release +has **shipped**: + +| Section | State | +|---|---| +| `== 0.5.x` and below | Hand-written legacy from before the fork, and shipped. **Frozen.** | +| `== 0.6.0.0` - the release being cut | **Not shipped, so not settled.** Whatever sits under this heading now is working text. It will be **regenerated at release time from `git log ..HEAD`**, replacing what is there, and frozen only once 0.6.0.0 ships. | +| Every release after it | Same treatment: generated when that release is cut, frozen once it ships. | + +Two readings this is written to rule out. **`0.6.0.0` is not on the hand-written side of the line** - +generation does not start at some later release. And **the current contents of `== 0.6.0.0` are not +what v6 will publish** - do not cite them as the release notes, and do not treat the section as +appendable just because the release has not gone out. It is not yours to add to *or* to trust. + +**In a PR the changelog is never added to.** No new entries, and no `== Unreleased` section - a +shipped section is finished, and the in-flight section belongs to the generator. There is no window +in which a PR contributes an entry. + +**The one edit a PR may make is correcting a factual error in text that is already there.** #198 did +exactly this: a Dependencies entry said the Kafka client stayed on `3.9.1` when the pom had moved to +`3.9.2`. A wrong statement in a published artefact does not get better by waiting for the next +generation pass, so fix it. The test is whether you are *changing an existing claim to be true* +(allowed) or *adding information about a change* (the generator's job, not yours). + +The policy removes the file that every PR used to touch - it appeared in 30 of the last 30 master commits, dragging the generated `README.adoc` with it - and removes the ordering problem where an entry had to cite a PR number that did not exist when the entry was written. @@ -236,10 +259,26 @@ section. The judgement it applies, and that a human should re-apply before freez confluentinc; make issue links explicit (`.../issues/NN[#NN]`), since GitHub numbers issues and PRs from one sequence. -**Still live, and now inert:** the `PR Checklist` gate's changelog rule -(`.github/scripts/changelog-ref-gate.js`) fails a human PR that *adds* a `CHANGELOG.adoc` entry citing -no issue. Since PRs no longer touch the file, it should never fire - it remains a guard for anyone -hand-editing the frozen sections. +### The `PR Checklist` changelog gate is a different, narrower check + +`.github/scripts/changelog-ref-gate.js` fails a human PR that adds a `CHANGELOG.adoc` bullet under +`Breaking`, `Improvements`, `Fixes` or `Examples` without an explicit `/issues/NN` link. **Do not read +a green gate as compliance with the rule above** - it is neither a subset nor a superset of it, and it +is not dormant: + +- It **passes** entries the policy forbids. The gate only cares about the *citation*, so an added + entry that links an issue sails through. #57's entries all cite issues. +- It **fires** on the one edit the policy allows. The gate cannot tell an edit from an addition - its + own header explains that matching removed bullets against added ones was tried and abandoned as the + subtlest code in the file - so a correction like #198's looks like a new entry. That is what + `changelog-ref: N/A - ` on its own line in the PR body is for; the workflow names this case + explicitly. +- PRs **do** still touch the file. #51, #57, #105 and #106 were all open before this policy landed and + all modify `CHANGELOG.adoc`; every PR predating the policy is in the same position. + +So the gate enforces the *citation convention* on entries, and a human author and reviewer enforce +"no entries in a PR". Tightening it to reject every addition was considered and rejected - see +[`docs/inflight/ci-changelog-gate-scope.md`](docs/inflight/ci-changelog-gate-scope.md). ## PR Discipline diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index b56f52058..193075c08 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -9,7 +9,7 @@ NOTE:: Routine/automated dependency bumps (e.g. Dependabot) are not listed here. NOTE:: *Reference convention (this is a fork).* Links and bare `#NN` / commit hashes refer to this fork, https://github.com/astubbs/parallel-consumer[astubbs/parallel-consumer]. Upstream references are written explicitly as `upstream #NN` and link to https://github.com/confluentinc/parallel-consumer[confluentinc/parallel-consumer]. Entries below `0.6.0.0` predate the fork and their `#NN` refer to upstream. -NOTE:: *How this file is maintained.* Everything up to and including `0.6.0.0` is hand-written and frozen. From the next release on, each section is **generated at release time from the commit log** and then frozen in turn - so do not add entries here in a feature PR, and do not open an `== Unreleased` section. The judgement applied when generating (what earns an entry, and how long it may be) is in `AGENTS.md` -> Changelog. +NOTE:: *How this file is maintained.* A section is frozen once its release has **shipped** - `0.5.x` and below are hand-written legacy from before the fork, and are finished. The section for the release currently being cut is **generated at release time from the commit log**, replacing whatever text is under it, and is frozen in turn once that release ships. **That includes `0.6.0.0` below**: it has not shipped, so what you are reading there is working text and not the notes that release will publish. Consequently a PR never adds an entry here and never opens an `== Unreleased` section; the only edit a PR may make is correcting a factual error in text that is already present. The judgement applied when generating (what earns an entry, and how long it may be) is in `AGENTS.md` -> Changelog. // git log --pretty="* %s" 0.3.0.2..HEAD diff --git a/docs/inflight/ci-changelog-gate-scope.md b/docs/inflight/ci-changelog-gate-scope.md new file mode 100644 index 000000000..e6a62699a --- /dev/null +++ b/docs/inflight/ci-changelog-gate-scope.md @@ -0,0 +1,48 @@ +# The changelog gate does not enforce the changelog policy + +**Open.** The wording defect that hid this is fixed (#113's "from the next release on"); the gap in +the gate itself is not, and is deliberately left alone. + +## What the policy says vs what the gate checks + +`AGENTS.md` -> Changelog: **a PR never adds a changelog entry.** The section for the release being cut +is regenerated at release time from the commit log. The only edit a PR may make is correcting a +factual error in text that is already there (as #198 did for the Kafka client version). + +`.github/scripts/changelog-ref-gate.js` checks something else entirely: that an *added* bullet under +`Breaking` / `Improvements` / `Fixes` / `Examples` carries an explicit `/issues/NN` link. The two +overlap only by accident: + +- **False pass.** An added entry that cites an issue is a policy violation the gate waves through. + #57's entries all cite issues. +- **False fail.** The gate cannot distinguish an edit from an addition, so the one edit the policy + *permits* - a correction - reads as a new entry. `changelog-ref: N/A - ` in the PR body is + the intended escape, and the workflow comment names this case. + +## Why it was not tightened to "reject any addition" + +Considered while fixing the wording, and rejected: + +1. **The distinction the policy turns on is not mechanically detectable.** "Adds an entry" vs + "corrects an existing one" both appear as `+*` lines in the patch. The gate's own header records + that fuzzy-matching removed bullets against added ones was implemented, became the largest and + subtlest part of the file, and still mispaired entries built from the same template. Re-deriving + it would repeat known-failed work. +2. **A blanket rule would be enforced by the honour system anyway.** Its only escape is + `changelog-ref: N/A - `, a self-declared line in the PR body. The same line that + legitimises a correction legitimises a policy-violating addition, so the strict gate buys no + enforcement over the written rule - it buys an opt-out line on every legitimate correction. +3. **It would block four PRs opened in good faith.** #51, #57, #105 and #106 all modify + `CHANGELOG.adoc` and all predate the policy. Tightening now fails them for following the rule that + was live when they were written. + +Weakening the existing citation check was never on the table: it still does its own job correctly. + +## If this is revisited + +The mechanically honest version is a **warning, not a gate**: any `CHANGELOG.adoc` change in a PR +posts a comment saying entries are generated at release time and asking the author to confirm this is +a correction. No false failures, and it puts the policy in front of the person who would otherwise +learn it from a merge conflict at release time. Worth doing only if entries actually start +reappearing in PRs after 0.6.0.0 ships - until then the four pre-policy PRs are the entire population, +and they are known. diff --git a/docs/inflight/release-0600-blockers.md b/docs/inflight/release-0600-blockers.md index 0fe40a508..e538f2190 100644 --- a/docs/inflight/release-0600-blockers.md +++ b/docs/inflight/release-0600-blockers.md @@ -32,10 +32,11 @@ Release mechanics live in [`release-0.6.0.0.md`](release-0.6.0.0.md); the tracki regenerate with `./mvnw -N asciidoc-template:build` (or `mvn process-sources`). Every README change is therefore a two-file diff, and a PR that touches only the template has silently not changed the published README. -- **`CHANGELOG.adoc` is frozen up to `== 0.6.0.0`.** It is not a per-PR chore (AGENTS.md → *Changelog*), - so a PR editing it is either fixing a factual error in a frozen section or is wrong. When several - agents work in parallel, exactly one may hold that file - it is the highest-collision file in the - repo and conflicts in it are pure noise. +- **`CHANGELOG.adoc` is not a per-PR chore, and the `== 0.6.0.0` section is not final** (AGENTS.md → + *Changelog*). It is regenerated from the commit log when the release is cut, so the text there now + is working text - do not quote it as the release notes. A PR editing the file is either fixing a + factual error in existing text or is wrong. When several agents work in parallel, exactly one may + hold that file - it is the highest-collision file in the repo and conflicts in it are pure noise. - **Anything asserting a dependency version in prose can drift silently.** The `3.9.1`/`3.9.2` mismatch arose because a Dependabot group bump moved `kafka.version` after the release note was written, and nothing cross-checks prose against `pom.xml`. Re-read the `=== Dependencies` section against the pom From 20e2bcf0bd236403acbe28cf3925d7e99e85d34b Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 6 Aug 2026 12:53:07 +1200 Subject: [PATCH 2/4] docs(changelog): inline the gate-scope reasoning, drop the inflight note The note said what the AGENTS.md subsection above it already said. Its one unique contribution - why tightening the gate to reject every addition was rejected - now sits inline where the gate is described, so a reader gets the reasoning without a second hop. docs/inflight/ tracks work that is open and needs picking up. This is a settled decision with its rationale, which is what AGENTS.md is for. A note recording a decision already made is the stale-marker failure that directory's own charter warns about. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01RtNUsxokE9g2pSEjBHZqNA --- AGENTS.md | 9 ++++- docs/inflight/ci-changelog-gate-scope.md | 48 ------------------------ 2 files changed, 7 insertions(+), 50 deletions(-) delete mode 100644 docs/inflight/ci-changelog-gate-scope.md diff --git a/AGENTS.md b/AGENTS.md index 63d84a91a..67dcf624b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -277,8 +277,13 @@ is not dormant: all modify `CHANGELOG.adoc`; every PR predating the policy is in the same position. So the gate enforces the *citation convention* on entries, and a human author and reviewer enforce -"no entries in a PR". Tightening it to reject every addition was considered and rejected - see -[`docs/inflight/ci-changelog-gate-scope.md`](docs/inflight/ci-changelog-gate-scope.md). +"no entries in a PR". Tightening it to reject *every* addition was considered and rejected: "adds an +entry" and "corrects an existing one" are both `+*` lines and are not mechanically distinguishable - +the gate's own header records that matching removed bullets against added ones was built and then +abandoned as the subtlest code in the file. A blanket rule's only escape hatch would be a +self-declared `changelog-ref: N/A - ` in the PR body, which legitimises a violating addition +exactly as easily as a legitimate correction: no enforcement the written rule does not already have, +at the cost of an opt-out on every correction. ## PR Discipline diff --git a/docs/inflight/ci-changelog-gate-scope.md b/docs/inflight/ci-changelog-gate-scope.md deleted file mode 100644 index e6a62699a..000000000 --- a/docs/inflight/ci-changelog-gate-scope.md +++ /dev/null @@ -1,48 +0,0 @@ -# The changelog gate does not enforce the changelog policy - -**Open.** The wording defect that hid this is fixed (#113's "from the next release on"); the gap in -the gate itself is not, and is deliberately left alone. - -## What the policy says vs what the gate checks - -`AGENTS.md` -> Changelog: **a PR never adds a changelog entry.** The section for the release being cut -is regenerated at release time from the commit log. The only edit a PR may make is correcting a -factual error in text that is already there (as #198 did for the Kafka client version). - -`.github/scripts/changelog-ref-gate.js` checks something else entirely: that an *added* bullet under -`Breaking` / `Improvements` / `Fixes` / `Examples` carries an explicit `/issues/NN` link. The two -overlap only by accident: - -- **False pass.** An added entry that cites an issue is a policy violation the gate waves through. - #57's entries all cite issues. -- **False fail.** The gate cannot distinguish an edit from an addition, so the one edit the policy - *permits* - a correction - reads as a new entry. `changelog-ref: N/A - ` in the PR body is - the intended escape, and the workflow comment names this case. - -## Why it was not tightened to "reject any addition" - -Considered while fixing the wording, and rejected: - -1. **The distinction the policy turns on is not mechanically detectable.** "Adds an entry" vs - "corrects an existing one" both appear as `+*` lines in the patch. The gate's own header records - that fuzzy-matching removed bullets against added ones was implemented, became the largest and - subtlest part of the file, and still mispaired entries built from the same template. Re-deriving - it would repeat known-failed work. -2. **A blanket rule would be enforced by the honour system anyway.** Its only escape is - `changelog-ref: N/A - `, a self-declared line in the PR body. The same line that - legitimises a correction legitimises a policy-violating addition, so the strict gate buys no - enforcement over the written rule - it buys an opt-out line on every legitimate correction. -3. **It would block four PRs opened in good faith.** #51, #57, #105 and #106 all modify - `CHANGELOG.adoc` and all predate the policy. Tightening now fails them for following the rule that - was live when they were written. - -Weakening the existing citation check was never on the table: it still does its own job correctly. - -## If this is revisited - -The mechanically honest version is a **warning, not a gate**: any `CHANGELOG.adoc` change in a PR -posts a comment saying entries are generated at release time and asking the author to confirm this is -a correction. No false failures, and it puts the policy in front of the person who would otherwise -learn it from a merge conflict at release time. Worth doing only if entries actually start -reappearing in PRs after 0.6.0.0 ships - until then the four pre-policy PRs are the entire population, -and they are known. From b4edd0ef8f593ce1312e1a5670d09ba796a83984 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 6 Aug 2026 12:53:07 +1200 Subject: [PATCH 3/4] fix(ci): stop the review gate failing because it FOUND the review check-review-posted.sh:94 ran `printf | grep -q` under `set -o pipefail`. grep exits the instant it matches, printf then dies with EPIPE (141), and pipefail promotes that to the pipeline's status - so a comment citing the run made the check fail. It only bites when more than one pipe buffer (64 KiB) of comment data follows the match, which is why it looked intermittent. Observed on #198, #199, #204 and #210. On #210 the review comment sat 4.7 KB into the stream with a 127 KB similarity report behind it; the job log carries `printf: write error: Broken pipe` directly above the failure. The emitted error blamed the workflow-validation guard, which none of those PRs had tripped. The fix is the herestring this repo already prescribes: the same bug class is guarded against in bin/test-check-copyright-headers.sh, whose comment records it "seen live in CI". check-review-posted.sh shipped in that same commit without the guard. Two tests, both verified to fail against the old line: - functional: match found, then >64 KiB of further comments. The existing cases dance around this - case 5 buries the match but keeps it small, case 6 puts it last so nothing follows to fill the buffer. - structural: the checker may not pipe into grep -q or awk at all, mirroring the copyright scanner's guard, so the next instance of the class is caught rather than the next occurrence of this one. The error text is left alone: it misdiagnosed those four PRs only because of this bug, and becomes accurate again once the SIGPIPE path is gone. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01RtNUsxokE9g2pSEjBHZqNA --- bin/check-review-posted.sh | 6 +++++- bin/test-check-review-posted.sh | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/bin/check-review-posted.sh b/bin/check-review-posted.sh index 7f4722423..937c44f68 100755 --- a/bin/check-review-posted.sh +++ b/bin/check-review-posted.sh @@ -91,7 +91,11 @@ if [ "$review_outcome" != "success" ]; then exit 1 fi -if printf '%s\n' "$comment_bodies" | grep -qE "actions/runs/${run_id}([^0-9]|$)"; then +# Herestring, not `printf | grep -q`: under `set -o pipefail` (above) grep's early +# exit on a match kills printf with EPIPE, and pipefail promotes 141 to the +# pipeline status - so FINDING the review would fail the check. Only bites when +# more than one pipe buffer (64 KiB) of comments follows the match. +if grep -qE "actions/runs/${run_id}([^0-9]|$)" <<<"$comment_bodies"; then echo "Review posted by run ${run_id}." exit 0 fi diff --git a/bin/test-check-review-posted.sh b/bin/test-check-review-posted.sh index c823a4271..2b2820aae 100755 --- a/bin/test-check-review-posted.sh +++ b/bin/test-check-review-posted.sh @@ -19,6 +19,7 @@ # 10. wrong argument count -> usage (2) # 11. non-numeric run id -> usage (2) # 12. empty review outcome -> usage (2) +# 13. match found, then >64 KiB of further comments -> pass (0) # # Case 4 is the one worth keeping honest: a plain substring search for "actions/runs/3096" # matches run 30965089954, which would pass this run's check on a different run's review. @@ -77,6 +78,16 @@ $POSTED_COMMENT" 30965089954 success)" assert "run url at the very end of the stream" \ 0 "$(run_checker "see https://github.com/astubbs/parallel-consumer/actions/runs/30965089954" 30965089954 success)" +# The live failure this script shipped with: the match is found, but >64 KiB of +# comments follow it, so `printf | grep -q` + pipefail turned success into exit 1. +# Cases 5 and 6 missed it - case 6 puts the match LAST, so nothing follows to +# fill the pipe buffer. Observed on #198, #199, #204 and #210; on #210 a 4.7 KB +# review comment was followed by a 127 KB similarity report. +LARGE_TRAILER="$(yes 'padding to exceed the 64 KiB pipe buffer, as a similarity report does' | head -n 2000)" +assert "match found, then >64 KiB of further comments (SIGPIPE regression)" \ + 0 "$(run_checker "$POSTED_COMMENT +$LARGE_TRAILER" 30965089954 success)" + for outcome in failure skipped cancelled; do assert "review step '$outcome' is never a review, matching comment or not" \ 1 "$(run_checker "$POSTED_COMMENT" 30965089954 "$outcome")" @@ -92,6 +103,17 @@ assert "non-numeric run id" \ assert "empty review outcome" \ 2 "$(run_checker "$POSTED_COMMENT" 30965089954 "")" +# --- Structural guard: no SIGPIPE-prone pipes in the checker ------------------- +# The same rule bin/test-check-copyright-headers.sh enforces on the copyright +# scanner, for the same reason. This checker shipped without it and hit the bug +# live on four PRs before anyone noticed. +if grep -vE '^[[:space:]]*#' "$CHECKER" | grep -nE '\|[[:space:]]*grep -q|\|[[:space:]]*awk '; then + echo "FAIL: checker pipes into an early-exiting reader (SIGPIPE + pipefail misclassification risk) - use a herestring" + failures=$((failures + 1)) +else + echo "ok: checker has no SIGPIPE-prone pipes into grep -q / awk" +fi + echo if [ "$failures" -eq 0 ]; then echo "All bin/check-review-posted.sh self-tests passed" From 1dbc80b4ca27313e526b104440cb1b4d1801414d Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 6 Aug 2026 15:35:22 +1200 Subject: [PATCH 4/4] docs: qualify fork references, per the gate that landed after this branch PR Checklist failed with "12 reference(s) below #1000 do not say which repo they mean". The gate arrived in 735b1d3a (astubbs#114), which merged 17 minutes AFTER astubbs#211 - so this branch was written before the rule existed. The fork's numbers sit inside upstream's range, so a bare number is a coin flip: astubbs#114 found 48 numbers that exist in BOTH repos meaning different things. All twelve here are fork PRs, now written astubbs#NN. Pointed, given this PR is about a reference convention. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01RtNUsxokE9g2pSEjBHZqNA --- AGENTS.md | 8 ++++---- bin/test-check-review-posted.sh | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index faf7cc914..6f3d9ba27 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -249,7 +249,7 @@ appendable just because the release has not gone out. It is not yours to add to shipped section is finished, and the in-flight section belongs to the generator. There is no window in which a PR contributes an entry. -**The one edit a PR may make is correcting a factual error in text that is already there.** #198 did +**The one edit a PR may make is correcting a factual error in text that is already there.** astubbs#198 did exactly this: a Dependencies entry said the Kafka client stayed on `3.9.1` when the pom had moved to `3.9.2`. A wrong statement in a published artefact does not get better by waiting for the next generation pass, so fix it. The test is whether you are *changing an existing claim to be true* @@ -298,13 +298,13 @@ a green gate as compliance with the rule above** - it is neither a subset nor a is not dormant: - It **passes** entries the policy forbids. The gate only cares about the *citation*, so an added - entry that links an issue sails through. #57's entries all cite issues. + entry that links an issue sails through. astubbs#57's entries all cite issues. - It **fires** on the one edit the policy allows. The gate cannot tell an edit from an addition - its own header explains that matching removed bullets against added ones was tried and abandoned as the - subtlest code in the file - so a correction like #198's looks like a new entry. That is what + subtlest code in the file - so a correction like astubbs#198's looks like a new entry. That is what `changelog-ref: N/A - ` on its own line in the PR body is for; the workflow names this case explicitly. -- PRs **do** still touch the file. #51, #57, #105 and #106 were all open before this policy landed and +- PRs **do** still touch the file. astubbs#51, astubbs#57, astubbs#105 and astubbs#106 were all open before this policy landed and all modify `CHANGELOG.adoc`; every PR predating the policy is in the same position. So the gate enforces the *citation convention* on entries, and a human author and reviewer enforce diff --git a/bin/test-check-review-posted.sh b/bin/test-check-review-posted.sh index 1331384bc..e4606d6aa 100755 --- a/bin/test-check-review-posted.sh +++ b/bin/test-check-review-posted.sh @@ -81,7 +81,8 @@ assert "run url at the very end of the stream" \ # The live failure this script shipped with: the match is found, but >64 KiB of # comments follow it, so `printf | grep -q` + pipefail turned success into exit 1. # Cases 5 and 6 missed it - case 6 puts the match LAST, so nothing follows to -# fill the pipe buffer. Observed on #198, #199, #204 and #210; on #210 a 4.7 KB +# fill the pipe buffer. Observed on astubbs#198, astubbs#199, astubbs#204 and +# astubbs#210; on astubbs#210 a 4.7 KB # review comment was followed by a 127 KB similarity report. LARGE_TRAILER="$(yes 'padding to exceed the 64 KiB pipe buffer, as a similarity report does' | head -n 2000)" assert "match found, then >64 KiB of further comments (SIGPIPE regression)" \