fix(source-control,review,work-items): paginate every documented GitHub list read and drop the positional-index verifications - #2163
Conversation
…and assert completeness
The repository instructed operators and agents to run
`gh api repos/{owner}/{repo}/commits/<sha>/check-runs` with no
pagination. The endpoint returns 30 results per page by default and
reports nothing when it truncates, so on any PR carrying more than 30
check runs the response is a silent partial list — and "is check X
present?" answers a false *no* for every check that landed on a page
the caller never fetched, indistinguishable from a check that never
attached.
Reproduced deterministically against this repo's own PR heads:
2895890: total_count=33, returned=30
580fd09: total_count=33, returned=30
435b2fe: total_count=33, returned=30
On all three, the dropped set was `GitGuardian Security Checks`,
`pr-issue-linkage / pr-issue-linkage`, and `do-not-merge /
do-not-merge` — the three earliest-started checks. `do-not-merge /
do-not-merge` is a required status context for `main`, and a prior
reading of this query concluded it "never attaches to a head SHA". It
attached and was green on every one of those heads.
Both call sites now use `--paginate` with `per_page=100`, matching the
form `skills/pull-request/scripts/fetch-annotations.sh` already used.
Pagination alone moves the cliff to 100 rather than removing it, so
both sites also document a completeness assertion — `total_count`
against the flattened count across every page — plus the trap that
makes the naive form of that assertion wrong: with `--paginate`, `--jq`
runs per page, so `.check_runs | length` prints one line per page, each
reporting only its own page's count. The published assertion slurps the
page stream before comparing, and was verified against a live head both
at `per_page=100` (single page) and at a forced small page size
(four pages), reporting `total_count=35 returned=35` in both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ub list read and drop the positional-index verifications
Every GitHub REST list endpoint these plugins document returns 30 items
per page by default and reports nothing when it truncates. The repo's
instructions read a dozen of them with no pagination.
check-runs — reproduced deterministically against this repo's own heads:
2895890: total_count=33, returned=30
580fd09: total_count=33, returned=30
435b2fe: total_count=33, returned=30
On all three the dropped set was `GitGuardian Security Checks`,
`pr-issue-linkage / pr-issue-linkage`, and `do-not-merge /
do-not-merge` — the three earliest-started checks. `do-not-merge /
do-not-merge` is a required status context for `main`, and a prior
reading of this query concluded it "never attaches to a head SHA". It
attached and completed `success` in three seconds on every one of those
heads.
Comment and review reads are worse, because they are ordered
OLDEST-FIRST (verified: `issues/<n>/comments` and `pulls/<n>/reviews`
both return ascending `created_at`/`submitted_at`). An unpaginated read
therefore drops exactly the newest items — the only ones a monitoring
poll or a "did my reply post?" check cares about. Two call sites paired
that list with `.[-1]`, which does not omit but answers wrongly:
#657 33 comments | true latest 2026-07-22T18:47:05Z
| '.[-1]' unpaginated 2026-07-22T07:20:46Z
#502 31 comments | true latest 2026-07-23T23:29:21Z
| '.[-1]' unpaginated 2026-07-23T23:28:27Z
Every corrected site uses `--paginate` with `per_page=100`, matching the
form `skills/pull-request/scripts/fetch-annotations.sh` already used.
The `.[-1]` verifications now select on the fix SHA, so the query states
what it asserts and cannot be satisfied by another author's comment.
Pagination alone moves the cliff rather than removing it, so
`readiness.md` gains a `Reading GitHub list APIs` section carrying both
rules — paginate every list read; never pair a positional index with a
list — plus the completeness assertion for endpoints that report a
total. That assertion has a trap: with `--paginate`, `--jq` runs per
page, so `.check_runs | length` prints one line per page, each counting
only its own page. The published form slurps the page stream first and
was verified against a live head at `per_page=100` (one page) and at a
forced small page size (four pages), reporting `total_count=35
returned=35` both times.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ion that endpoint cannot carry The annotation-gap guidance told the auditor to paginate "each check-run's /annotations" the same way, then published only a check-runs-shaped completeness assertion. That assertion is not executable against /annotations: the endpoint returns a bare JSON array with no envelope and no `total_count`, so there is nothing to compare the flattened count to, and concatenated pages combine with `add` rather than through a `.check_runs` wrapper. Both endpoints now carry a runnable form of their own, and the difference between them is stated rather than elided — check-runs asserts `total_count`, /annotations cannot, so there `--paginate` is the only guard. Verified live against a check run carrying one annotation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
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. |
|
Claude finished @kyle-sexton's task in 2m 25s —— View job Review complete
SummaryThis PR does what it says: every documented GitHub list-read call site in The new "Reading GitHub list APIs" section in Everything else checked out:
|
|
Claude finished @kyle-sexton's task in 1m 29s —— View job Security review complete
No security issues found. This PR is documentation-only — 12 files, all Reviewed for the categories in scope:
The substance of the change — fixing silent pagination truncation and replacing an unsound |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8375296eb0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Claude has reviewed this PR 5 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
|
Claude finished @kyle-sexton's task in 3m 39s —— View job Review complete
SummaryI re-diffed Two aggregation-under-
Both are the exact class of bug this PR is otherwise systematically fixing (it says so itself, two paragraphs above the second site: " One correctness gap Codex flagged that I independently verified and extended to a second, identical call site:
Everything else checked out:
I did not re-run the author's |
|
Claude finished @kyle-sexton's task in 1m 27s —— View job Security review complete
No CRITICAL or IMPORTANT security issues found. Scope confirmed via Reviewed against the categories in scope:
One SUGGESTION worth noting (already flagged by @chatgpt-codex-connector on No other authorization, injection, secret-handling, or Actions-trust-boundary issues identified in this diff. |
#2135 bumped source-control to 0.51.5 on main — the number this branch already claimed — so the two 0.51.5 sections collided in the changelog. Resolved by renumbering this branch's entry to 0.51.6 and keeping main's 0.51.5 under its own heading with its own content. Neither section is relabelled, absorbed, or dropped: the heading list of all three touched changelogs differs from main by exactly one addition, and each manifest is exactly one increment above main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 23s —— View job Review complete
SummaryThe diff at the current HEAD ( All three previously-flagged findings are still present, unaddressed:
I posted inline comments confirming all four locations with suggested fixes (items 1–2 use the external- Everything else checked out and is unchanged from the prior pass:
I did not re-run the author's |
|
Claude finished @kyle-sexton's task in 2m 15s —— View job Security review complete
No CRITICAL or IMPORTANT security issues found. Scope confirmed at HEAD Reviewed against the categories in scope:
One SUGGESTION, confirmed independently and posted inline at SKILL.md#L183 and its duplicate at review-discipline.md#L239 (originally raised by @chatgpt-codex-connector, mirrored by prior review passes on this PR — I re-verified it's still present at current HEAD rather than re-reporting on trust): the new D7 issue-level verification ( Two other findings live in review comments on this PR (aggregating No other authorization, injection, secret-handling, or Actions-trust-boundary issues identified in this diff. |
…q, and correct two miscited items
Two commands in this change set broke the rule the change set
publishes, one of them ~90 lines below where the file states it.
With `--paginate`, `gh` applies `--jq` to each page SEPARATELY, so any
expression that folds a whole list answers per page:
* `readiness.md` Gate 5 counted codex comments with
`--jq '[…] | length'`. Forced to four pages that printed
`10 10 10 3` instead of `33`.
* `work-item-tracker/adapters/github/README.md` — a command this
change set itself rewrote — reduced with `--jq '[…] | sort_by(.id)'`
and emitted four separately-sorted arrays, each ordered only within
its own page.
Both now drop `--jq`, slurp the page stream with `jq -s`, and flatten
with `.[][]`. Verified against a live 33-comment item forced to four
pages: both corrected forms return the true total, the originals
reproduce the per-page output above.
The trap is promoted from a sentence inside the completeness-assertion
prose to rule 3 of `Reading GitHub list APIs`, since it is a distinct
failure from the two rules already there and is what both violations
broke. It also draws the line the other two rules do not: element-wise
filters (`select`, `map` over `.[]`) are safe under `--paginate`
because their results concatenate; folds are not.
Also corrected: #657 and #502 are ISSUES, not pull requests
(`gh api …/issues/657` carries no `.pull_request`). They were miscited
as PRs in `readiness.md` and the source-control changelog, in prose
presented as measurement. The endpoint and the mechanism are identical,
but measurement that misnames its subject invites doubt about the rest.
The `33–37 check runs` range is left as written; it is substantiated
across five measured heads — 33, 33, 33, 35, 37.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second merge-forward. main reached d18075e and had taken every number this branch claimed: source-control 0.51.6 and 0.51.7, review 0.18.2, work-items 0.35.1. Renumbered above what main holds — 0.51.8, 0.18.3, 0.35.2 — with every main section kept under its own heading and its own content. work-items needed the most care: both sides wrote under a shared `## [0.35.1]` heading, so main's content stays under 0.35.1 and this branch's moves to a new 0.35.2 above it, rather than the two being merged into one section. Verified per file: the heading list differs from main by exactly one addition, none deleted, renamed, or absorbed; each manifest is exactly one increment above main; no conflict marker of any of the four forms survives anywhere in the tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 40s —— View job Security review complete
No CRITICAL or IMPORTANT security issues found. Scope confirmed via Reviewed against the categories in scope:
One SUGGESTION, still unresolved at current HEAD — — matches on comment content only, with no constraint on No other authorization, injection, secret-handling, or Actions-trust-boundary issues identified in this diff. The two aggregation-under- |
|
Claude finished @kyle-sexton's task in 2m 31s —— View job Review complete
SummaryReviewed the diff at current HEAD Progress since the last review pass: the two more severe correctness bugs are now fixed.
I independently re-verified both fixes by reading the current file contents, and grepped all six changed reference/skill files for any remaining One finding, already raised by Codex and three prior review passes on this PR, is still unresolved — reconfirmed and reposted inline since it wasn't touched by the commits that fixed the two items above:
Everything else checked out and is unchanged from the prior passes:
I did not re-run the author's |
Fourth merge-forward, onto fd7ffc4. No conflict this time: main's versions (source-control 0.51.7, review 0.18.2, work-items 0.35.1) are all still below the numbers this branch already claimed, so 0.51.8 / 0.18.3 / 0.35.2 stand unchanged. Verified rather than assumed, since a merge-forward can silently change a file already signed off: `git diff 9fe2d54 HEAD` over plugins/source-control, plugins/review, and plugins/work-items is EMPTY. Everything main brought in lands in guardrails and markdown-format, which this branch does not touch. Heading lists still differ from main by exactly one addition per file; no conflict marker of any of the four forms survives; all three parity modes pass against the merged tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 29s —— View job Security review complete
No CRITICAL or IMPORTANT security issues found. Scope confirmed via Reviewed against the categories in scope:
One SUGGESTION, still unresolved at current HEAD — — matches on comment content only, with no constraint on No other authorization, injection, secret-handling, or Actions-trust-boundary issues identified in this diff. |
|
Claude finished @kyle-sexton's task in 2m 29s —— View job Review complete
SummaryCurrent HEAD is Progress confirmed since the pre-9fe2d54 passes: both aggregation-under-
I independently re-read both files at this HEAD (not just trusting the prior pass's description), and re-grepped all six changed reference/skill files for any remaining One finding remains open, unchanged across six review passes now (Codex + five Claude passes on this PR):
Everything else checked out, consistent with prior passes:
I did not re-run the author's |
…ting identity, not just the SHA
The reviewers are right, and this is the same defect class the PR
exists to fix.
D7's "did my follow-up reply post?" check selected on
`select(.body | contains("<sha>"))`. That proves the SHA was MENTIONED,
not that this agent posted it. A reviewer quoting the fix commit, or a
bot restating it, satisfies the selector — so the gate reports the
reply as posted while the agent's own write silently failed.
That is precisely the shape of the `.[-1]` bug this line replaced: a
plausible positive instead of a real presence signal, on a control gate
an autonomous agent acts on. Replacing one under-constrained selector
with another under-constrained selector fixed the ordering bug and left
the class intact.
Both copies of the checklist step — SKILL.md and the canonical
review-discipline.md — now pin `.user.login` alongside the SHA.
Verified against live data: SHA-only matches, SHA + correct identity
matches, SHA + wrong identity returns empty. The selector stays
element-wise, so it remains safe under `--paginate` per rule 3.
Rule 2 gains the general form rather than just the instance: where a
query is a control gate you will act on, ask what ELSE could satisfy
the selector and constrain that too — one property is usually not
enough.
The two `--jq` folds these threads also flagged were already fixed in
89646d8; the most recent review pass confirms that on this head.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fifth merge-forward, onto 3ccf7df. No conflict: main's versions (source-control 0.51.7, review 0.18.2, work-items 0.35.1) remain below the numbers this branch holds, so 0.51.8 / 0.18.3 / 0.35.2 stand. Re-verified rather than assumed: `git diff fae6f5c HEAD` over plugins/source-control, plugins/review, and plugins/work-items is EMPTY. Everything main brought in lands in songwriting. Heading lists still differ from main by exactly one addition per file; no conflict marker of any of the four forms survives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 53s —— View job Security review complete
No CRITICAL, IMPORTANT, or SUGGESTION security issues found. Scope confirmed at HEAD The one open finding from every prior security/general review pass on this PR is now fixed. The D7 "verify follow-up reply posted" selector previously matched on SHA-in-body alone with no posting-identity constraint (originally raised by @chatgpt-codex-connector, reconfirmed across six review passes). The new commit on this HEAD adds the identity constraint at both copies of the checklist step:
Both now read: This closes the gap: a reviewer or another bot quoting the fix SHA can no longer satisfy the control-gate check on its own. Re-verified the other categories in scope, unchanged from prior passes:
No other authorization, injection, secret-handling, or Actions-trust-boundary issues identified in this diff. |
Review-thread triage — all 19 threadsRecorded here so the disposition survives the session that produced it. Every thread has an individual threaded reply with its evidence; I resolved none of them — disposition is the reviewer's or the operator's. Nineteen threads (17 at first count, two more arrived on the pre-fix head) collapse to two distinct findings, each restated by Codex and several successive review passes. Neither was wrong; neither was superseded-but-invalid. Both are now fixed.
Finding A — reproduced, then fixedAgainst issue #657 (33 comments) forced to four pages: Corrected forms return Rather than patch the two sites, the trap became rule 3 of Finding B — the sharpest finding on this PR, and it was mine
That is exactly the shape of the Both copies now pin The selector stays element-wise, so it does not reintroduce Finding A. Rule 2 took the general form rather than the instance: where a query is a control gate you will act on, ask what else could satisfy the selector and constrain that too — one property is usually not enough. The narrow fix would have left the next such gate free to repeat it. VerificationReplies enumerated with Two threads remain open on |
#2163 merged and took source-control 0.51.8, so this branch's ruleset-fold fix moves to 0.51.9. main's 0.51.8 section is kept intact under its own heading with its own content. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… per_page=100 on every paginated read (#2262) Closes #2244 ## Summary - **D6 verify-commit-pushed** (`reference/review-discipline.md` and `skills/pull-request/SKILL.md` — the SKILL.md site *is* present on current `main` at line 181, contrary to the issue's grep against older `c1b4c629`): replaces the branch-tip read `commits?sha=<branch>&per_page=1` + `--jq '.[0].sha'` with the single-resource presence read `repos/{owner}/{repo}/commits/<fix-sha>`. The tip form asks a presence question but answers a tip question — any push after the fix (follow-up commit, rebase, sibling lane) makes it report the fix missing while present: a false negative on a control gate, and a positional index on a list, which `readiness.md` rule 2 forbids six lines above D7. The single-resource form echoes the SHA on exit 0 when present and fails HTTP 422 (`No commit found for SHA`) when absent — index-free, identity-bound, cannot be satisfied by the wrong record. - **#2246 (source-control rows only)**: adds `per_page=100` to the six remaining `--paginate` sites in this plugin: `merge.md:14-16`, `SKILL.md:166-168` (C1–C3), `monitor.md:195`, `fetch-all-pr-comments.sh:141`, `telemetry-upsert.md:39`. Not truncation defects — `--paginate` alone is complete — but non-conformant with rule 1 as `readiness.md:55` publishes it, at 3.3x the request cost. - **Deliberately unchanged**: `babysit_gh.py:441` and `request_review.py:186` from the #2246 re-sweep are false positives — the `per_page=100` sits in the endpoint URL on the line adjacent to the `--paginate` flag the line-based sweep matched (`request_review.py:185`; every `fetch_paginated_api` caller URL at `babysit_gh.py:459/478/523/691`). The other-plugin rows (claude-ops, work-items) belong to the parallel lane, not this PR. - Version 0.51.11 + CHANGELOG section (renumbered above the 0.51.10 that #2167 landed on main mid-flight). ## Test plan - Live verification of the replacement form in both directions on this repo: `commits/33f0df51…` → echoes the SHA, exit 0; `commits/0000…0000` → HTTP 422, exit 1. - `plugins/source-control/scripts/fetch-all-pr-comments.test.sh` — all 23 checks pass (the three endpoints the script pages carry no existing query string, so `?per_page=100` composes safely). - `scripts/check-changelog-parity.sh --check` and `--check-bump origin/main` — both pass. - `markdownlint-cli2` on all six changed markdown files — 0 issues. - Conflict-marker sweep (all four forms) — clean. ## Related - #2244 (closes) - #2246 (source-control rows; claude-ops/work-items rows remain for the parallel lane) - #2163 / #2238 / #2239 — prior instances of the same defect class Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…se-safe (#2285) ## Summary Fixes #2245. `readiness.md` rule 3's carve-out incorrectly listed `map` over `.[]` as element-wise-safe. `map(f)` is defined as `[.[] | f]` — it builds an array per page under `--paginate`, so it is a per-page fold unless followed by `| .[]`. The carve-out now names `select` and `.[] | f` as safe, and calls out that bare `map(f)` is not — use `map(f) | .[]` or `.[] | f` instead. ## Test plan - [x] Read the updated rule 3 prose for accuracy against the `gh --paginate` + `--jq` behavior described in the issue - [x] Version bump `source-control` 0.51.12 → 0.51.13 with CHANGELOG entry ## Related - #2245 - #2163 (rule 3 author) - #2238 (same pagination doc family) --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
1 participant
No linked issue
Summary
Every GitHub REST list endpoint this repository's instructions read returns 30 items per page by default and reports nothing when it truncates. A dozen documented call sites read them with no pagination, so the guidance told operators and agents to draw conclusions from silently partial data.
The check-runs case, and the false conclusion it already caused
Reproduced deterministically against this repository's own PR heads with the bare form:
On all three the dropped set was identical — the three earliest-started checks:
do-not-merge / do-not-mergeis a required status context formain(ruleset17989001). It is a metadata-onlypull_request_targetjob that reads label metadata and runs no head code, so it completes in about three seconds — well before the heavypull_requestmatrix. That is exactly why it is always among the first started, and therefore always the first truncated away.A prior reading of this query concluded that
do-not-merge"never attaches to a head SHA" on three separate PRs, and recorded it as an established finding. It was false. On2895890cthe context attached at02:38:04Z, completedsuccessat02:38:07Z, and was green on every one of those heads. The query was truncating.Under 31 total checks nothing truncates, which is why earlier occurrences looked like intermittent flakiness that resolved itself.
The comment and review case, which is worse
issues/<n>/comments,pulls/<n>/comments, andpulls/<n>/reviewsare returned oldest-first (verified — ascendingcreated_at/submitted_at). An unpaginated read therefore drops exactly the newest items: the only ones a monitoring poll or a "did my reply post?" check cares about.Two call sites paired that list with
.[-1]. That shape does not omit — it answers, plausibly, and wrongly, because.[-1]on a truncated oldest-first page is the 30th-oldest item:Both are issues, not pull requests — the endpoint and the mechanism are identical, but an earlier draft of this PR miscited them as PRs in prose presented as measurement, and that is corrected here and in the shipped docs.
Rule 3 was found the same way, by the same class of defect, in this change set itself: two commands here reduced across pages inside
--jq, one of them in the file that states the rule. Gate 5's codex-comment count printed10 10 10 3over four pages instead of33; the work-item-tracker recipe emitted four separately-sorted arrays instead of one sorted list. Both now slurp withjq -sand flatten with.[][]. The rule draws the line that makes it usable: element-wise filters (select,mapover.[]) are safe under--paginatebecause their results concatenate; folds are not.Fix
Every corrected site uses
--paginatewithper_page=100, matching the formplugins/source-control/skills/pull-request/scripts/fetch-annotations.sh:111already used — that script was already correct and is untouched here.The
.[-1]verifications now select on the fix SHA instead, so the query states what it is asserting and cannot be satisfied by another author's comment.The sentinel
Pagination alone moves the cliff from 30 to 100 rather than removing it.
readiness.mdgains aReading GitHub list APIssection stating three rules once, with Gate 1 pointing at it rather than restating:--jq.For endpoints that report a total, assert against it. Rule 3 is why the naive assertion is wrong: with
--paginate,--jqruns per page, so it reports one page at a time —— so the published form slurps the page stream first:
Verified against a live head at
per_page=100(one page) and at a forced small page size (four pages), reportingtotal_count=35 returned=35both times..[0].total_countis sound because every page repeats the same total./annotationsis deliberately given its own form rather than the same one: that endpoint returns a bare array with no envelope and nototal_count(verified), so the completeness assertion is unavailable there,--paginateis the only guard, and its pages combine withaddrather than through a.check_runswrapper.Call sites left unchanged, with reasons
plugins/kindle-dedrm/skills/manage/scripts/check-drift.sh:76and its documented twinreferences/workflow.md:40—Satsuoni/DeDRM_toolshas 20 releases total and the newest is a prerelease (v10.0.28, index 0), so nothing truncates today. It is also the one site where a mechanical--paginatewould be the wrong fix: it would walk every release ever published to find a match that is always on page 1. The right shape there is a boundedper_page=100, which is a different decision from the one this PR makes. Follow-up.plugins/discovery/skills/research/context/discipline.md:91,224— prose examples, andreleases/latestis the correct single-resource form anyway.plugins/source-control/reference/review-discipline.md:108,109— a prose inventory of which endpoints get read, not runnable commands.docs/topics/autonomy-ignition/PLAN.md:127— a sanity-check line inside### Phase 1 … [DONE], a historical record of a completed phase against a single-gate scratch repo, not guidance anyone would copy today./repliesPOSTs andissues/comments/<id>single-resource GETs throughout — neither paginates.plugins/source-control/skills/pull-request/scripts/fetch-annotations.sh:111,157andplugins/source-control/scripts/fetch-all-pr-comments.sh:128,152,177— already correct.Nothing unpaginated remains in
source-control,review, orwork-itemsthat is a list read.Known-red check — RESOLVED, kept for the record
Current state:
changelog-parity-gateis green. #2159 merged, this branch mergedmainforward, and the gate now passes on the same 267 KB changelog that failed deterministically before. The account below is what the red meant while it lasted; it is retained because a reviewer reading this PR's check history will see those failures and deserves to know they were never this diff's.What the red was
changelog-parity-gatefailed on this PR, and the failure was not caused by this diff.scripts/check-changelog-parity.sh'shas_headingisrendered_lines - | awk '…{exit}'underset -o pipefail. The reader exits on the first match — line 6, since the newest heading is at the top — while the producer keeps writing; past the pipe buffer it takes SIGPIPE (141),pipefailpropagates it, and a heading present at line 6 column 1 is reported as absent.plugins/source-control/CHANGELOG.mdis 267 KB, the largest in the repo.It is a race on pipe scheduling rather than a size threshold, and it is sensitive to the awk implementation and host. Measured locally (GNU Awk 5.4.0 under MSYS), only the largest file failed:
The runner is worse. This PR's actual
changelog-parity-gaterun failed all three plugins, including the 44 KB one that passes locally:Every one of those three headings is present at line 6, column 1. Independently confirmed per file: the heading list differs from
mainby exactly one addition, with none deleted, renamed, or absorbed. The gate's own 55 self-tests pass in the same run — no existing fixture is large enough to cross the buffer.ci-statusfails only as the aggregate of that one job. Every other check on this head is green, and all four required contexts attached:Fixed on
mainby #2159.Post-merge validation of #2159, unplanned but worth recording.
plugins/source-control/CHANGELOG.mdis the largest changelog in the repo and the one that failed deterministically rather than as a race. After merging #2159 forward it passes all three gate modes —--check,--check-bump,--check-order— against the merged tree. That is independent evidence the fix closed the class rather than moving the boundary.One correction to the mechanism as first written here: the discriminator is the awk engine and host, not file size. CI resolves
awkto gawk, which chunks its writes, so the reader's early exit strands every later chunk; mawk cannot produce the failure at any size. That is why a 44 KB changelog failed on CI while passing locally under MSYS gawk — and why the local byte numbers above should not be read as a threshold.Test plan
check-runsforms run against live heads2895890c,580fd090,435b2fef,193c9d2e,043d60ce; dropped sets computed by jq set difference, not read off a list.per_page=100and at a forced small page size; multi-page behaviour of--jqexhibited, not asserted./annotationsresponse shape checked directly (type=array,has_total_count=false); published annotations form run against a check run carrying one annotation.#657and#502figures re-derived.scripts/check-changelog-parity.sh --checkand--check-orderpass;--check-bumpfails for the reason above.scripts/check-cross-plugin-source-drift.shrc=0;scripts/check-contract-clause-coverage.pypasses.markdownlint-cli2clean over 78 files..mdand.jsononly — no shell files touched, so no shellcheck run applies.main: exactly one heading added per file, none deleted, renamed, or absorbed.Related
changelog-parity-gateSIGPIPE regression this PR's red check is caused by.plugins/source-control/skills/pull-request/scripts/fetch-annotations.sh:111— the in-repo precedent every corrected call site matches.