Repository navigation
ci(fewer-jobs): batch the small per-PR jobs so the sharded suites stop queueing - #457
Merged
Merged
Conversation
…te jobs into repo: hygiene Three per-PR jobs ran gates that `repo: hygiene` already runs in its `bin/check-all.sh --with-tests` sweep: `Copyright header check` (copyright.yml, deleted), `quarantine: audit` and `docs data: audit` (maven.yml, deleted). Every PR push launched them twice. Master run 34066495360 shows all four gates and their self-tests `ok` inside the sweep before this deletion. Why it matters: the repo is public on the free plan, whose cap is 20 concurrent jobs, and a PR push launched 27 of ours plus 3 CodeQL analyzes. The big sharded suites were queueing behind one-minute jobs. This is the first cut of the ce-optimize run `ci-fewer-jobs`: 27 -> 24. What the fold had to carry over so nothing quietly stops running: - COPYRIGHT_CHECK_REQUIRE_FORK_POINT=1 and an explicit PyYAML assertion on the hygiene job, mirroring its shellcheck one. - check-all.sh gains --strict: a CANNOT (exit 2, cannot run) fails the sweep. Without it a missing tool on the hosted image would have read as a pass, because check-all exits 0 with a CANNOT among passes. CI runs strict; the local default is unchanged. - No token added to hygiene: check-quarantine-owners.sh is advisory there, and quarantine-lane.yml (required `tests`) already runs both quarantine gates with a token on every push. The three deleted job names are required checks in the master ruleset. docs/inflight/ci-fewer-jobs-ruleset-edits.md lists the removals and the merge-time ordering; a required context nothing produces leaves every PR pending. Two citations of the deleted workflow were repaired: the live docs/copyright.md reworded, the dated solution record given a history pointer per docs/citations.md. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
`PR Checklist` (pr-checklist.yml, deleted) ran eleven steps in its own job on every PR push: three bin/test-* self-tests that the hygiene sweep already runs, a Node compile-check, the gate-module unit tests, the marker-index check, and five github-script gates that read the PR body and diff. All of them now run as steps of `repo: hygiene`. Second cut of the ce-optimize run `ci-fewer-jobs`: 24 -> 23 jobs. What the fold had to preserve: - Triggers are the union. The checklist ran on `edited` because its gates read the PR body; hygiene now does too, so a body edit still re-evaluates them (and re-runs the macOS lane, an accepted cost). - A concurrency group per PR, keyed on the PR number or the sha so master pushes never cancel each other. - Read-only token. Hygiene held no token by design; the checklist needs contents+pull-requests read. No write scope was added - the sweep runs PR-authored scripts, and the reviewer-isolation rule in docs/ci.md forbids a write grant beside them. - persist-credentials: false on the checkout. Nothing in the sweep uses the checkout credential; gh reads GH_TOKEN and the one fetch is anonymous against a public remote. - The github-script gates are guarded to pull_request events; the tree-only steps also run on master pushes, which is more coverage. Why hygiene and not the other PR-metadata job: `Check PR Dependencies` needs checks: write and a `closed` trigger to unblock child PRs when a parent merges, and PR-authored steps must not sit beside a write grant. The two review-gate jobs each produce a required check with its own semantics. Recorded in docs/inflight/ci-fewer-jobs-ruleset-edits.md, which now lists four required contexts to remove at merge time. The old job name was cited as the authoritative gate across AGENTS.md, bin/, docs/ and the PR template; those live references now name `repo: hygiene`, and the two dated records that cited the deleted file carry a history pointer per docs/citations.md. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
… job `dups: clones`, `dups: similarity` and `deps: vulnerabilities` each took a runner slot for a scan that finishes in well under two minutes, with no Maven build between them. They are now three steps of one `scan: repo` job in maven.yml: dependency review first because it is the fastest signal, then the two duplicate detectors. Third cut of the ce-optimize run `ci-fewer-jobs`: 23 -> 21 jobs per PR push. Fail at the end, report everything: every scanner step is guarded with `!cancelled()`, so a red first scanner never hides the second or the third, and the job conclusion aggregates them - no verdict step, no continue-on-error. One full-depth checkout serves all three (both duplicate tools compare against the base). The job restates the contents: read and pull-requests: write scopes the clones job carried; maven.yml already granted those workflow-wide, so nothing widened. Each step keeps the name of the job it replaces. The log then reads as the checks list used to, and the sentences across docs/inflight/ of the form "dups: clones found X on astubbs#NNN" stay literally true: only the check name changed. The explanatory comment blocks moved with their steps. Three required contexts go and one arrives; the ordering is in docs/inflight/ci-fewer-jobs-ruleset-edits.md - remove at merge time, promote `scan: repo` once a PR against merged master has produced it. bin/check-pr-analysis-surfaces.sh reads annotations by count and comments by bot type, never by these names, so it is unaffected. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
The two build-dependent analysers each waited on the dependency cache job, then restored the same Maven cache with a byte-identical step, to run for about a minute and about six. They are now two steps of one `static: analysis` job: Infer first because it is the cheaper signal, then SpotBugs with its annotation and sticky-comment steps unchanged. Fourth and last cut of the ce-optimize run `ci-fewer-jobs`: 21 -> 20 jobs per PR push, and 27 -> 20 across the run. Fail at the end, report everything: each analyser step is guarded with `!cancelled()`, so a red Infer never hides SpotBugs, and the two reporting steps that were `always()` are now `!cancelled()` for the same reason. The SpotBugs summary step keeps its deliberate `continue-on-error`. The job conclusion aggregates; no verdict step. This is the last safe fold of build-dependent lanes. The two together run to roughly seven minutes against a Unit Tests critical path of about nine, so the headroom is nearly spent - folding the PIT lane in as well would blow through it, because that lane runs to half an hour whenever it has something to mutate. Permissions narrowed rather than widened: neither job declared any, so both inherited the workflow-wide grant. The merged job declares only what the two used - contents read, pull-requests write for the sticky comment, checks write for the annotations - and drops actions: read, which neither analyser touched. A check rename breaks consumers that match by name, even when the step names are preserved. bin/check-pr-analysis-surfaces.sh filters check runs through a name regex, and `racerd` had been dead in it since the job became `static: infer`, dropping that surface from the listing with nothing to say so. The pattern now names the live checks and carries the rule. Nine required contexts now go and two arrive; docs/inflight/ci-fewer-jobs-ruleset-edits.md carries the list and the ordering for both directions. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…n PRs The OSS Index audit took a runner slot of its own on every PR to do about two minutes of work. On PRs it is now the last step of `scan: repo`, after the three scanners have reported. dependency-audit.yml keeps the job for its weekly schedule and its dispatch, which is where the lane earns its keep: a new advisory arrives without anybody pushing. Only the pull_request trigger left. Fifth cut of the ce-optimize run `ci-fewer-jobs`: 20 -> 19 jobs per PR push. The fork and Dependabot guard is carried over exactly. A job-level `if:` cannot express "skip seven of these ten steps", so the condition became a job-level env var that each CVE step reads. A PR from a fork or from Dependabot gets no CVE steps at all, which is what it got before when it got no job, and pays none of the setup cost. A secret that is present but empty still reds the build from the preflight. Cost: `scan: repo` now needs the dependency cache, so it gains `needs: prepare-deps` and runs to roughly four and a half minutes against a Unit Tests critical path of about nine. Two things this leaves behind, both recorded in the workflow headers: the three no-build scanners now start behind the cache job rather than immediately, which the next commit in this series removes along with the cache job itself; and the five audit steps are duplicated across the two workflows, so each header says to change both or neither. Sharing them wants a composite action, which is not this change. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
The PR-scoped mutation lane held a runner slot to do about eleven seconds of work on most pushes, because it usually finds no changed core class in a decidable package to mutate. It is now the last step of `scan: repo`, behind everything that reports: the scanners and the CVE audit have already posted before PIT starts. Sixth cut of the ce-optimize run `ci-fewer-jobs`: 19 -> 18 jobs per PR push. Its semantics are carried over exactly, because they are the whole point of the lane. The exit-code handling is unchanged, including the branch that turns "could not run, or ran and scored nothing" into a failure rather than a green tick. The step keeps continue-on-error, so a PIT verdict still cannot red the required `scan: repo` job - the same reason requiring the old job would have been vacuous. No ruleset edit is owed for this one, and the working note says so, so nobody diffing the checks list before and after goes hunting for a missed removal. The step bound is better than what it replaces. The job used to carry a 30-minute cap, and a run that hit it was cancelled outright, which is one of the sightings recorded against the untracked-flakes note. A 20-minute step bound ends the step and lets the job report instead, and it sits above the longest scored run on record. One guard needed widening. The JDK and cache-restore steps sat behind the CVE credentials condition from the previous commit, and PIT runs on fork and Dependabot pull requests, which is exactly where that condition is false. They now run unconditionally, so PIT has a JDK where the CVE steps skip. Everything touching OSS Index stays guarded. The analysis-surfaces script matched check runs on a `Mutation` alternation that now names nothing, the same way `racerd` did after an earlier rename. Removed, with a note that the survivor table hangs off `scan: repo` now. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
✅ Duplicate Code ReportTwo engines run in parallel for cross-validation. Each has its own thresholds tuned to its baseline - the real safety net is the per-engine "max increase vs base" check. ✅ PMD CPD
No new clones introduced by this PR. ✅ jscpd (language-agnostic)
No new clones introduced by this PR. Powered by astubbs/duplicate-code-cross-check |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #457 +/- ##
============================================
- Coverage 83.07% 82.47% -0.60%
- Complexity 1350 1482 +132
============================================
Files 84 95 +11
Lines 4597 5165 +568
Branches 492 505 +13
============================================
+ Hits 3819 4260 +441
- Misses 594 711 +117
- Partials 184 194 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
[superseded - a quarantined test changed outcome] 🧪🔒 Quarantine Lane Report
🔴 expected while the owner PR is open · 🟡🎲 flapper, pass proves nothing · 🚨 a deterministic quarantined test passing means its fix landed: delete its Updated for Superseded by a newer quarantine lane report. |
🟢 Throughput — OKThis branch measured the same speed as master, on the one test this measures.
Allowable range 🟢 ≥ 0.70 · 🟡 0.50–0.70 (about a 30% loss) · 🔴 < 0.50 (about a 50% loss) What the numbers mean, and what they cannot tell youThe one that gets misread. Why a shape and not a rate. A rate depends on which runner you drew. A shape does not: every test here processes a fixed number of records, so a runner twice as slow doubles the subject and the controls together and leaves their ratio alone. That is the whole trick, and it is why the reported rate is shown last and labelled as this machine only. Reading the comparison. By conservation, not by correction. Every test in this lane processes a fixed number of records, so within one run the ratio of one test's time to another's is invariant under machine speed — a runner twice as slow doubles both terms and leaves the ratio alone. There is no machine-index correction to be wrong, because nothing needed correcting. Per-method times, not class times. A class time is Reference is the median of 10 recent What this still cannot do. It removes machine-to-machine variance. It does not remove this test's own run-to-run variance, measured at about 30% on a single unchanged commit while its controls stayed within 5%. That is a property of the test, not of the comparison, and no arithmetic here can touch it — which is why the reference is a median and the bounds are deliberately coarse. 🟡 means look at this; only 🔴 is outside the measured spread. Runs used: 7a8dd92, df3bd71, 440b9bd, 11e0500, e20e898, 9999144, cc36b64, 1941cdf, 05c02bb, 5706686 Since the previous push: ratio 1.092 -> 1.004, share 1.528 -> 1.662, rate 95453 -> 70242 (-26.4%). One push of difference sits inside this test's measured spread - read it as movement, not as a result. Updated for |
… fold cannot redefine its default The copyright scanner's self-test asserts BOTH arms of the fork-point rule: a missing fork point warns and skips (exit 0) by default, and hard-fails (exit 2) under COPYRIGHT_CHECK_REQUIRE_FORK_POINT=1. The default-behaviour case set no variable at all, so it asserted whatever the ambient environment said the default was - which was fine for as long as nothing set it. The fold of copyright.yml into `repo: hygiene` set it. The deleted workflow put COPYRIGHT_CHECK_REQUIRE_FORK_POINT on the SCAN STEP only, so its self-test step never saw it; repo-hygiene.yml carries it on the JOB-level env block, where every step inherits it - including the sweep step, `bin/check-all.sh --with-tests --strict`, which discovers and runs this self-test by glob. Strict mode became the ambient default, the fixture repos deliberately have no fork point, and the assertion read "expected '0', got '2'". The default case now clears the variable explicitly with `env -u`, and says at the fix site why that is load-bearing. This STRENGTHENS the test rather than relaxing it: a case asserting a default must define the environment it is asserting about, not inherit one. Nothing was removed, nothing accepts exit 2 now that did not before, and the strict-mode case immediately below - which sets the variable explicitly, for the same reason - is untouched. CONTROL ARM. With the `env -u` reverted in place and everything else identical, `COPYRIGHT_CHECK_REQUIRE_FORK_POINT=1 bash bin/test-check-copyright-headers.sh` fails on exactly that one case (expected '0', got '2') and exits 1; with it restored, both arms exit 0. The clean arm passes either way, which is why this was green on a laptop and red only in CI. SAME DEFECT CLASS, SWEPT. The `repo: hygiene` job sets two variables at job level, PR_NUMBER and COPYRIGHT_CHECK_REQUIRE_FORK_POINT. Every `bin/test-*.sh` and `bin/test-*.mjs` was checked for reads of either. Only two self-tests read one at all: this one, and bin/test-check-branch-self-reference.sh, which already sets PR_NUMBER explicitly on all three of its gate invocations and unsets the GITHUB_* variables beside them - with a comment recording this same lesson from the last time it bit. No other self-test is exposed. The wider sweep for uncontrolled ambient reads across all 34 self-tests turned up only locally-assigned names and INFER_JDK / CI in bin/test-check-infer.sh, neither of which the job sets. The stale citation at the fix site is repaired in passing: the authoritative gate it named, copyright.yml, is a file this branch deleted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRMqsJQrGcF6ZH8xyX8AU6
…ns it, not the workflow that named it
QuarantinedAnnotationContractTest asserted that .github/workflows/maven.yml
contained the literal strings `bin/check-quarantine-registry.sh` and
`bin/check-quarantine-owners.sh`. This branch deleted maven.yml's
`quarantine: audit` job, because both gates were already running on every pull
request inside `repo: hygiene`'s `bin/check-all.sh --with-tests --strict` sweep,
which DISCOVERS gates by globbing `bin/check-*.sh` rather than naming them. No
workflow names either script on the per-PR path any more, so the test's
grep-the-workflow mechanism could only fail - it cannot express "runs via a glob
sweep".
THE ASSERTION IS REPLACED, NOT DELETED, and the guarantee it protects is
end-to-end as before. The per-PR audit now has two homes and both are asserted:
(a) The discovering sweep. repo-hygiene.yml runs `bin/check-all.sh --with-tests
--strict` and triggers on `pull_request`; check-all.sh discovers gates by
the `bin/check-*.sh` glob; each gate EXISTS at a path that glob matches; and
neither gate is named in check-all.sh's two exception lists, PR_SCOPED and
NEEDS_ARGS, which are the only ways a globbed gate is skipped. That last
link is what actually proves the glob still covers them - without it, "the
glob covers them" is an assumption rather than a measurement. The lists are
read through a helper that fails loudly if either stops being a plain
double-quoted assignment, because a silently-empty read would turn every
membership check into a pass.
(b) quarantine-lane.yml, which still names both scripts explicitly as fail-fast
steps ahead of the lane run. Those assertions are unchanged.
The assertion that the lane RUN (`bin/quarantined-test.sh`) must NOT be in
maven.yml is kept, and maven.yml is still read for it.
The method is renamed from perPrWorkflowRunsTheAuditAndTheLaneWorkflowRunsTheTests
to theHygieneSweepRunsTheAuditOnEveryPrAndTheLaneWorkflowRunsTheTests, and both
the class javadoc and the method javadoc now state where the audit runs and why
grepping a workflow proves nothing about it. A reader arriving at a failure can
tell the topology from the test alone.
CONTROL ARM. Adding `check-quarantine-registry.sh` to check-all.sh's PR_SCOPED
list - the one change that would silently stop the per-PR sweep running the
audit, with every workflow still green - fails the new case with the message
naming the list and its contents. Restored, the class is 12/12 green. The
previous assertion could not detect that state at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RRMqsJQrGcF6ZH8xyX8AU6
…k, and why no scanner reports it Six jobs in maven.yml open with the same checkout, setup-java and cache-restore steps, and the cache key must agree in all six because the file's header forbids setup-java's own cache. Two of the six arrived with the job folds on this branch. There is no .github/actions/ directory, so nothing shares them. The entry carries the measurement rather than the assertion, because "the duplication scanners are green" reads as evidence and here it is not. PMD CPD never reads a workflow at all: its language detect is winner-take-all, Java wins, and YAML is not mappable by it. jscpd is language-agnostic and does read them - pointed at the workflow directory it reports real clones, including the one pair it still sees at the job's own default threshold, between the two review workflows. Pointed at maven.yml alone it reports nothing at any threshold down to 20 tokens, because each copy of the block has different comments interleaved between its steps: strip the comments and the six are identical, leave them in and the token streams differ. That is the capability half of the 2026-08-12 write-up on the scanners not looking where agents duplicate. Its scope half was fixed when the scan widened to the whole repo; this half survived that fix, and the entry says so where somebody checking a green report will find it. Recorded with the OSS Index audit steps, now duplicated across two workflows behind a "change both, or neither" note, because one composite-action decision settles both. Deliberately not done here: this PR already rewrites every workflow and owes ruleset edits at merge, and its whole risk is a check quietly ceasing to run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRMqsJQrGcF6ZH8xyX8AU6
[superseded - a quarantined test changed outcome] 🧪🔒 Quarantine Lane Report
🔴 expected while the owner PR is open · 🟡🎲 flapper, pass proves nothing · 🚨 a deterministic quarantined test passing means its fix landed: delete its Since the previous push: Updated for Superseded by a newer quarantine lane report. |
[superseded - a quarantined test changed outcome] 🧪🔒 Quarantine Lane Report
🔴 expected while the owner PR is open · 🟡🎲 flapper, pass proves nothing · 🚨 a deterministic quarantined test passing means its fix landed: delete its No quarantined test changed outcome since the previous push. Updated for Superseded by a newer quarantine lane report. |
|
Owner
Author
Not addressing as new work: both are already covered by commits already on this branch. Verified locally against the current head ( |
Owner
Author
|
@claude review this |
|
Claude encountered an error after 9s —— View job I'll analyze this and get back to you. |
1 similar comment
|
Claude encountered an error after 9s —— View job I'll analyze this and get back to you. |
Owner
Author
|
@claude review this |
|
Claude encountered an error after 7s —— View job I'll analyze this and get back to you. |
Owner
Author
|
@claude review this |
|
Claude encountered an error after 9s —— View job I'll analyze this and get back to you. |
Owner
Author
|
@codex review |
![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: 1bfa32b218
ℹ️ 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".
One conflict, in the untracked-CI-flakes note, and both sides kept. The two edits are to adjacent rows of the same table and are about different things: this branch rewrote the PIT lane's row to record that the fold moved its bound from a job cap that cancelled the whole row to a step bound that lets the job report, and master rewrote the row above it to add a second sighting of the lifecycle test along with the prose block explaining what that sighting adds. Neither touches what the other says. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRMqsJQrGcF6ZH8xyX8AU6
…ting them
The note is the merge-time checklist for the ruleset edit the folds owe, and it
was writing derived cardinalities all through its prose - "All ten old names",
"An eleventh", "the four jobs", ordinal step positions, "Add two". Every one of
them is a second copy of a list that sits a few lines away, and
docs/inflight/AGENTS.md forbids exactly this ("Never write down what a command
can answer" - and it says counts of any kind, not just git facts) because the
copy is wrong the first time somebody edits the list and nothing goes red.
Here the drift has an unusually sharp failure mode, which is why this was worth
doing rather than filing: a reader who trusts "ten" over the list can leave a
name required that no run will ever produce, and a required context nothing
produces pends every PR in the repository - the spotbugs incident again. The
count is the one thing in this document that can silently make its own
instructions wrong.
So the name lists are untouched and are now stated to be the authority, the
ordinals are gone, and "how many" is answered by a command instead:
gh api repos/astubbs/parallel-consumer/rules/branches/master \
--jq '.[] | select(.type == "required_status_checks")
| .parameters.required_status_checks[].context'
Run at the merge, that prints what is actually required, which is the thing the
tally was standing in for. Verified against the live ruleset while writing this:
it returns every name on the removal list.
Two counts stay, because they cannot drift apart from what they count - each
names its members in the same sentence ("three of the checklist's named
self-test steps (a, b, c)", "all four self-tests ok" directly under the four
script names). The rule is about a number that can outlive its list.
The `<!-- file-refs: N/A -->` marker moved with the paragraph it excuses: the
gate reads upward and stops at a blank line, so inserting a paragraph between
the citation and its marker silently orphans both. bin/check-file-refs.sh caught
that, which is the second time this run that a gate found something a read-
through did not.
Raised by codex in review on #457.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RRMqsJQrGcF6ZH8xyX8AU6
…ched checks The scanner fold handed `scan: repo` a `needs: prepare-deps` edge that none of the three jobs it replaces had. `dups: clones`, `dups: similarity` and `deps: vulnerabilities` never built anything and never touched the Maven cache; they reported through a failed cache lane. Only the CVE and PIT steps at the tail of the merged job want the cache, and the edge was added for them. With the implicit job-level `success()` still in force, that edge is a new way to wedge the repository. A failed or timed-out `prepare-deps` - the Azure west-US Maven Central timeout that job exists to fix, so not hypothetical - SKIPS the whole job. `scan: repo` is a required check, and a skipped required check does not go red: it stays PENDING, forever, on every PR. So a transient cache failure would have suppressed three scanners that need no cache at all and left the PR waiting on a context that would never arrive. MECHANISM CHOSEN, and why it is this one. The job's `if:` now reads `!cancelled() && github.event_name == 'pull_request'`. Naming a status function replaces the implicit `success()` gate on the `needs:` edge, so the job runs when prepare-deps fails and still reports a conclusion. `!cancelled()` rather than `always()` so a superseded push still cancels it instead of spending a runner on work nobody will read. NOTHING DEGRADES INTO A PASS, which is the constraint that ruled out the alternatives. Moving the dependency onto the Maven-dependent steps alone would have needed a per-step `needs`, which does not exist; dropping the edge would have cost every scan run a cold cache. Running anyway costs nothing false: the scanners need no Maven, the cache step is `actions/cache/restore` with a prefix `restore-keys` fallback so the CVE and PIT steps get an older or cold cache rather than none, and a Maven step that genuinely cannot resolve fails its step and reds the job. A cannot-measure reads as red, never as a tick - the defect class this repo keeps relearning. SAME DEFECT, SECOND SITE, fixed with it. `static: analysis` is the other required context this fold creates and it carries the same edge. There it is not a regression - `static: infer` and `static: spotbugs` were both already required and both already had it - but a required name about to be added to the ruleset can be wedged the same way, so it takes the same guard. Checked and dismissed: `test`, `test-kafka-compat` and `build` also need prepare-deps and are untouched by this PR, so their behaviour is unchanged and out of scope here. Verified: actionlint reports the same three findings as the base and no new ones, and the job-count harness still measures 18 PR jobs with 0 dropped checks, so the `if:` rewrite has not changed what the fold delivers. Raised by codex in review on #457. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRMqsJQrGcF6ZH8xyX8AU6
…ded gates
The dedicated per-gate CI jobs are gone from this branch - copyright, the
quarantine audit, docs-data - and what replaced them is one `repo: hygiene` step
running `bin/check-all.sh --with-tests --strict`. Those jobs installed what they
needed explicitly, so a hosted image that stopped shipping PyYAML or ShellCheck
turned them red. Folded into the sweep, the same image change turns the gate
into a CANNOT, and the sweep's DEFAULT exits 0 on a CANNOT as long as something
else ran. `--strict` is the single line of behaviour standing between that and a
green `repo: hygiene` over gates that measured nothing.
bin/test-check-all.sh never invoked it. Its exit-2 fixture asserted only the
default's exit 0, so a regression accepting a CANNOT under strict would have
been caught by no test in this repository, and the fold's own justification -
"the sweep now takes --strict" - rested on an untested line. bin/AGENTS.md
requires a checker fix to carry a case that goes red against the old code; this
one had been recorded as a follow-up and not done.
Four arms, and the pair of controls that make them mean something:
- exit 2 FAILS under --strict, and says why a CANNOT on a runner is never a
legitimate skip.
- CONTROL, same fixture, one term changed: without --strict the identical
CANNOT still exits 0. So the arm above measures the flag, not the fixture.
- exit 3 stays nothing-in-scope under --strict. That distinction is what makes
--strict safe to run in CI at all - "measured, nothing in scope" is a result
where "could not measure" is not - and pinning it stops a later tightening
quietly redefining one as the other.
- a genuine exit-1 failure is still reported as a FAILURE and not relabelled a
strict-CANNOT. Both exit 1, so only the message separates them.
VERIFIED AGAINST THE PARENT IMPLEMENTATION, twice, because "it did not exist
yet" is the weaker of the two controls:
1. Against origin/master's check-all.sh (pre---strict), the four new arms fail
and every pre-existing arm passes: 20 passed, 4 failed. That proves the
arms are exercising --strict and nothing else, but it fails for the trivial
reason that the flag is an unknown argument.
2. The targeted one. Take the current script, keep --strict parsed and
accepted, delete only the block that turns a CANNOT into exit 1. Exactly
the two arms asserting that behaviour go red - exit 0 where 1 was wanted -
and all 22 others stay green, the exit-3 and genuine-failure arms included.
That is the regression the reviewer described, reproduced and caught.
On this branch, unmodified: 24 passed, 0 failed.
Raised by codex in review on #457.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RRMqsJQrGcF6ZH8xyX8AU6
…e file text
`theHygieneSweepRunsTheAuditOnEveryPrAndTheLaneWorkflowRunsTheTests` asserted
that the quarantine audit reaches every pull request by checking that
repo-hygiene.yml contains the literal `pull_request:`. That file also carries
the literal inside a comment - the roadmap gate's carrier line,
`pull_request: astubbs#NNN` - so the assertion was satisfied by prose. Delete
the real trigger and the test stayed green while the invariant it names, "the
per-PR audit runs on every PR", was false.
That matters more here than it looks. The whole point of the assertion is that
the per-PR audit no longer has a job naming it: the fold left the discovering
`check-all.sh` sweep as the only path, and this test is the chain that proves
the sweep is reached. A link in that chain a comment can satisfy is not a link.
The fix is a `workflowTriggers(String)` helper that walks the top-level `on:`
block: it opens at a column-0 `on:` (or `"on":` - YAML 1.1 reads a bare `on` as
a boolean, so some editors quote it), closes at the next column-0 key, skips
comment and blank lines, and returns keys at the block's own indent, so `types:`
nested under `pull_request:` is not mistaken for a trigger. The inline forms
(`on: push`, `on: [push, pull_request]`) are handled too, so rewriting the block
in flow style will not silently empty the result rather than failing. The
assertion messages print the triggers actually found, which is the thing a
reader needs when this goes red.
CONTROL. Removed the real `pull_request:` trigger and its `types:` line from
repo-hygiene.yml and ran the test:
- the old assertion's evidence survived - `grep 'pull_request:'` still matched,
on the comment line, so the previous form would have passed;
- the new assertion failed, reporting `Declared triggers: [push,
workflow_dispatch]`.
Restored, and the class passes 12/12.
SAME DEFECT, SECOND SITE, in the same file. `laneWorkflowDeclaresItsTriggers`
asserts quarantine-lane.yml's three triggers the same way. It is pre-existing
and happens to be correct today only because that file has no comment carrying
one of those literals WITH its colon - correct by luck, not by construction,
which is precisely the shape that went wrong one method up. It now reads through
the same helper. Checked and dismissed: nothing else in the tree asserts a
workflow trigger by substring (`grep -rn "pull_request\|workflow_dispatch\|
schedule:" --include=*.java` returns only these two methods and unrelated
javadoc).
Raised by codex in review on #457.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RRMqsJQrGcF6ZH8xyX8AU6
Owner
Author
|
@claude review this |
|
Claude finished @astubbs's task in 3m 23s —— View job Review complete
Reviewed at head All four Codex findings are already fixed on this branch, and I independently verified each fix rather than just trusting the reply:
Two other things worth noting, not blocking:
I did not re-run Nothing further to flag. The PR does what it says, the prior automated review's findings were all genuinely fixed (not just replied to), and the fixes carry their own regression coverage and controls rather than just patching the reported symptom. LGTM. |
…at were refuted The experiment record lived in gitignored scratch, so every hypothesis, measurement and conclusion from this work would have died with the machine that ran it. The squash message carries what changed; this carries why, what was tried and rejected, and the traps. The refuted ones are the point, and the cache-warming job is the worked example. It looked like a free job and the only lever that also cut wall clock. It is the recorded fix for a diagnosed Maven Central timeout, and an open branch depends on a step added to it. The failure worth writing down is not the wrong guess but the reason it survived review: the prior-art sweep searched the words the problem was framed in, and the owning document is filed under build errors. It was one grep away on the mechanism word. Also recorded: the target was set against the wrong CodeQL job count, so it was never reachable; the reviewer-failure explanation was refuted by a same-size comparison; and two things a job-count harness cannot see by construction, one of which let two failures reach CI because the local sweep was not the sweep CI runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRMqsJQrGcF6ZH8xyX8AU6
🧪🔒 Quarantine Lane Report
🔴 expected while the owner PR is open · 🟡🎲 flapper, pass proves nothing · 🚨 a deterministic quarantined test passing means its fix landed: delete its Since the previous push: Updated for |
Merged
6 tasks done
astubbs
added a commit
that referenced
this pull request
Sep 7, 2026
…w's job names, and a test says so Un-folding the PIT lane into its own job broke `bin/check-pr-analysis-surfaces.sh`, whose "SURFACES THIS SCRIPT CANNOT READ" listing filters check runs by name. #457 had dropped the `Mutation` alternative because the fold made `scan: repo` the row carrying the PIT job summary; with the job back, nothing matched `Mutation Tests (PIT, PR-scoped)` and its summary silently stopped being listed - no failure, just a row that stops appearing. That is the third instance of one class, and the second self-inflicted: the comment two lines above the regex already warned about it, citing `racerd` outliving the job that became `static: infer`. A warning that has now been walked into twice is not a control. So the class is closed with a test rather than a fourth comment. `bin/test-check-pr-analysis-surfaces.sh` reads the `scan`, `mutation` and `static` job names out of `.github/workflows/maven.yml`, extracts the pattern out of the subject, and asserts each name still matches. Neither side is restated in the test, so a rename or an un-fold flips it red with nobody editing it, and a job that disappears fails loudly instead of passing over an absent name. Control: against the pre-fix regex the new arm reports `the mutation job is named "Mutation Tests (PIT, PR-scoped)" ... which the listing pattern "static: analysis|scan: repo|CVE|Quarantine" does NOT match`, with `scan` and `static` still ok - 9 passed, 1 failed. With the alternative restored, 10 passed, 0 failed. The arm is static where every other arm is end-to-end through a faked `gh`, and deliberately: the filtering happens inside `gh --jq`, so a fake would have to reimplement jq, and a fake that knows the pattern would be testing itself. The invariant is testable without the plumbing. Only maven.yml jobs are asserted, because only their `name:` is the check-run name verbatim. The pattern's other two alternatives match no check run on a PR head today - the quarantine lane's job is named `tests` (`Quarantine Lane` is the workflow, which the check-runs API does not return) and `deps: whole-tree CVE scan` is `schedule`-only since the CVE fold - but deciding their fate is not this change's, so they are left alone. Notes that described the fold as current state are corrected rather than annotated, since they are live notes: `ci-fewer-jobs-ruleset-edits.md` (the fold is undone, the job cap is 25 on `mutation`, and the "second repair" paragraph now records the revert and names the guard), `test-untracked-ci-flakes.md` (the step-level `timeout-minutes: 20` is unchanged and still in force - only its location moved), `ci-orphaned-pit-comment-on-open-prs.md`, and `docs/refactoring.md`, whose setup-block duplication entry gained a seventh copy with the restored job. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRMqsJQrGcF6ZH8xyX8AU6
astubbs
added a commit
that referenced
this pull request
Sep 7, 2026
Picks up the batched per-PR jobs (#457) whose `scan: repo` and `static: analysis` contexts master's ruleset now requires, plus #450's offset-registration ordering in the same shard code this branch's put-back touches. One conflict, docs/inflight/ci-codecov-flags-not-like-for-like.md: both sides appended a dated sighting; both kept, this branch's 2026-09-03 one first. Master's text naming this PR in two notes is marked post-merge-correct rather than rewritten - dated records of runs against this branch while it was open, which read as history once it lands. State and ArchUnit tests green on the merged tree from fresh reports: RetryQueueRequeueWindowTest, RetryQueueRebalancePathTest, RetryQueueTest, RetryQueueIteratorConfinementTest, ShardManagerTest, ShardPopulationRaceTest, ArchitectureTest, RebalanceCallbackRuleControlTest. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rEzrWYFr6oEzy6porczd3
astubbs
added a commit
that referenced
this pull request
Sep 7, 2026
…must not wait on it (#463) #457 batched several small per-PR jobs together to stop the sharded suites queueing behind them. One of those folds was wrong. PIT went in as the last STEP of `scan: repo`, which is a REQUIRED check. A GitHub job emits one check run, and that check does not report until the whole job finishes. PIT's runtime is bimodal - about 11 seconds when no in-scope class changed, about 19 minutes when it actually mutates - so on any PR that touches mutable code the scanners and the CVE audit finished in about three and a half minutes and their verdict was then withheld for another seventeen. A lane deliberately kept non-gating, because its OUTCOME must not block a merge, was blocking the merge with its RUNTIME. So the mutation lane is its own job again. Its two steps move back verbatim, `continue-on-error` stays on the steps rather than the job - a job-level one makes the check-run conclusion success even when the step failed, which is why requiring it would be vacuous - and `timeout-minutes: 20` stays on the run step, where a hit ends the step and lets the job report, rather than on the job where a hit cancelled the whole row. Both job caps come down to fit what each job now holds, and the JDK and cache-restore steps in `scan: repo` go back behind the credentials guard they were only pulled out from for the folded lane. The measurement that hid this is the more useful half. The harness scored each fold on MEDIAN step durations, and a median is meaningless for a bimodal lane: it reported eleven seconds and hid a twenty-minute worst case. Batching is safe for fast, bounded, gating work; a slow, bimodal, deliberately non-gating lane is the one shape that must not be folded into a required job. Recorded in the solutions write-up of that run, which now says so. Un-folding also broke `bin/check-pr-analysis-surfaces.sh`, whose listing matches check runs against a hand-maintained set of job names: the mutation entry was removed when the fold landed, so restoring the job dropped its surface from the listing silently. That is the third instance of one defect class, and the comment directly above the pattern warns about it. The pattern is fixed and its self-test now derives the job names from the workflow, so the two cannot drift - proven by a control that fails against the old pattern. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Sep 8, 2026
56 commits of master, none of which touched this PR's mechanism: master changed neither `.github/workflows/release.yml`, `bin/release-notes.py`, `bin/test-release-notes.sh` nor `.github/workflows/mutation-full-sweep.yml` since the branch's last master merge, so the renderer and the workflow step that calls it merge unchanged. Master's `release.yml` still carries the inline `awk`/`sed` converter this PR replaces. Master also changed six of the files this PR touches. Four auto-merged: `docs/releasing.md` and `bin/AGENTS.md` (#457's job batching, #442's integration sharding), `.gitignore` (#378, #440), and `docs/inflight/release-0.6.0.0.md` (#448, #476). Two conflicted. `docs/inflight/release-0600-blockers.md` - the #197 triage bullet, inside the `post-merge: checked` markers. This branch (2026-09-01, 2026-09-03) rewrote it in post-merge terms as "all four landed"; master's grooming sweep (#476, 2026-09-08, correcting on 2026-09-07) rewrote the same bullet to say the empty-release-body item **was already false when written** - #72 gave the workflow a `--notes-file` built from the `CHANGELOG.adoc` section on 2026-07-29, with `--generate-notes` only as a fallback. Master's correction is right and is the later decision, so it is kept; master's claim that MDC is still open is not - #205 merged 2026-08-27, which is why this branch corrected it, and that correction is kept. Resolved as one bullet carrying both later facts, and it now states what #199 actually does (replace the inline converter, and fail the release on a missing or unrenderable section) rather than the retired claim that it restores a body that was never absent. `docs/refactoring.md` - two unrelated new sections appended at the same point. Both kept: this branch's "`bin/release-notes.py` is Python in a Node-default `bin/`" and master's "JUnit tag resolution is implemented twice". No content from either side dropped. Neither `--ours` nor `--theirs` was used. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XS64Xttx4vF5datYh7fmk
6 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The big suites were queueing behind the small jobs. This repo is public on the
free plan, whose ceiling is 20 concurrent jobs across the whole account, and a
single PR push was launching more than that on its own before any other PR
pushed. Sharding the heavy suites made each one faster and made the crowding
worse.
So this batches the small jobs. Each fold is its own commit, and each keeps
every check that ran before:
Copyright header check,quarantine: audit,docs data: auditrepo: hygienealready ran those gatesPR Checklistrepo: hygienedups: clones,dups: similarity,deps: vulnerabilitiesscan: repojobstatic: infer,static: spotbugsstatic: analysisjobdeps: whole-tree CVE scan(the PR half)scan: repoMutation Tests (PIT, PR-scoped)scan: repoWhat changed is the number of
jobs:entries apull_requestevent starts.Read it off any ref rather than off this sentence:
Every batched job still reports every failure. Each folded step is guarded
if: ${{ !cancelled() }}, so a red first step never hides the ones behind it,and the job conclusion aggregates them. No verdict step, because the platform
already does that job. No new
continue-on-error.The deletions were duplicates, and the fold closed a hole
repo: hygienerunsbin/check-all.sh --with-tests, which discovers gates byglob, so the copyright, quarantine-registry, quarantine-owners and docs-data
gates were each running twice on every push. Evidence before deletion: master
run 34066495360 shows all four, and their self-tests, passing inside the sweep.
The dedicated jobs had been papering over something.
check-all.shexits 0 whena gate reports CANNOT, so a missing tool on the hosted image would have read as
a pass, and those jobs hid it by installing what they needed explicitly. The
sweep now takes
--strict, which fails on a CANNOT, and asserts its tools upfront.
What is owed at merge, and what is not
Required status-check contexts disappear and two arrive. The authoritative
lists, and the ordering in both directions, are in
docs/inflight/ci-fewer-jobs-ruleset-edits.md, which also carries the commandthat derives them from the live ruleset. Removals happen in the same sitting as
the merge; additions only once a PR has produced the new context. Get it
backwards and every PR in the repo pends on a check nothing emits.
One context vanishes with no ruleset edit owed at all. The PIT lane was
deliberately never required, because
continue-on-errormakes requiring itvacuous, and the note says so explicitly so nobody diffing the checks list goes
hunting for a removal that was never needed.
Review, and what changed after it
The
@clauderoute was asked four times and errored out every time withoutproducing a review, so Codex reviewed this instead - against
1bfa32b21,not the current head. It raised four findings, all of them fair, all fixed in
their own commits with a control arm each:
ruleset edit - removed, lists kept, deriving command added
--stricthad no regression case inbin/test-check-all.sh, so a regressionaccepting CANNOT would have left
repo: hygienegreen while folded gatesmeasured nothing - four arms added, checked against the pre-
--strictscriptprepare-depsfailure could skip the requiredscan: repojob, and askipped required check pends forever, wedging the PR - the batched jobs now
name a status function so they run and report when the cache lane fails
did not protect its stated invariant - it now parses the
on:block, provenby deleting the real trigger and watching the old form still pass
Those four fixes are themselves unreviewed, which this repo has a name for.
Master was also merged in after the review, resolving one conflict in
docs/inflight/test-untracked-ci-flakes.mdwhere both sides had edited adjacentrows about different tests; both were kept.
What this does not do
It does not get a single push under the cap. CodeQL's default setup adds four
analyze jobs that compete for the same slots, and both Python and JavaScript
have real source here, so none of them is idle. Closing the remaining gap is no
longer a batching question: it is either a coverage decision or a plan decision,
since GitHub Pro doubles the ceiling.
The
prepare-depscache job was going to be the last fold, and it was refused.It is the recorded fix for the Azure west-US Maven Central timeout, where an
uncached artifact hangs for four minutes per attempt, and
#379 added a step to it that an open branch depends on.
Removing it would have reinstated a diagnosed failure to save one slot.
The duplicated JDK-and-cache setup block across the jobs in
maven.ymlis leftalone and registered in
docs/refactoring.md, together with the measurementshowing why neither duplication engine reports it.
Checklist
docs/ci.md,.github/workflows/README.mdand the affecteddocs/inflight/notes track every fold; the live prose that named a deleted job now names its successordocs/features/-N/A - CI topology, nothing a user of the library can observebin/test-check-all.shgained the--strictregression arms;QuarantinedAnnotationContractTestnow parses the workflow trigger instead of grepping the file text; both changes carry a control arm that fails without the fixdocs/inflight/working note started at the PR's first commit -docs/inflight/ci-fewer-jobs-ruleset-edits.md, opened with the first fold and extended by each one after itce-simplifyandce-code-reviewlocally -N/A - Codex reviewed the PR instead; the @claude route errored on every attempt without producing a review