ci: touched packages runs in parallel legs and re-checks a failure before blaming the change - #1734
Merged
Merged
Conversation
…fore blaming the change Most red `touched packages` runs were not caused by the pull request: in the month audited, about 40% were a test already failing on dev, a flaky test, or the selector itself (#1332). Each one cost a person or an agent a full investigate-and-rerun cycle. A failing test is now run once more on the head. If it passes it is reported as flaky; if it fails again it is run at the base commit, and if it fails there too it is reported as already failing on the base. Both warn, stay green and are recorded on one standing issue, because a flaky test is still a bug with an owner. A failure that passes at the base, a test absent at the base, build failures, timeouts, panics outside a test, killed processes and more than five failures in a leg stay red, and the last group is never re-run. Go's JSON events drive the attribution while the log keeps the text Go would have printed. The touched packages run as separate legs (tui3, session, codeaf, rest) on separate runners instead of one after another, so a change spanning the heavy packages waits for the slowest leg rather than the sum. The rest leg uses -p 2 on the 16 GB public runner and every leg prints its capacity. scripts/touched-packages.sh is now the one selector for CI and `make test-touched`. The Go build cache refreshes. setup-go keyed it on go.sum, so every run restored the snapshot from the last dependency change (2026-09-10) and never saved a newer one. Caches are now keyed by UTC day, saved by the first dev push of the day that misses, restored by pull requests from the newest one, and left to GitHub's eviction. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…facts that stopped being true docs/rules/ci.md describes the legs, the one re-run, the base comparison, the standing issue and the daily build cache, and replaces the "never a retry" rule with the reason it changed. The seven-gigabyte, two-core and `-p 1` reasoning is marked as the private-runner history it is; the repository is public and its standard runner has 4 vCPU and 16 GB. CLAUDE.md stops describing the known-red ledger as live (it was deleted in #1012) and states the touched job's behaviour in a paragraph. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… under go test -json TestARenameNoticeNeverReachesTheJSONOnStdout compared renameNotice with os.Stderr. Under `go test -json` the testing package replaces os.Stderr with a pipe of its own after package variables are initialized, so the comparison failed although the notice still went to standard error. The touched-packages classifier and `make test-report` both run that way; the first live run of the new legs reported it as already failing on the base. It now asserts the writer is the file on the standard-error descriptor. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… else gets The first live runs of the legs showed that `go test -json` changes how tests behave. Under it Go 1.26 replaces os.Stderr after package init, and the binary receives -test.v=test2json, which helper processes that re-run the test binary inherit (internal/seniordev/util's four process tests fail only that way). A failure that exists only under -json fails at the base under -json too, so it would be reported as already failing forever and a real regression in that test could never turn the job red. The first run, the head retry and the base probe now run exactly as `make test` runs them, without -json, and the classifier parses Go's plain package blocks and scripts/shard-test.sh's summaries. Anything it cannot attribute from text stays red, and every panic stays red without a retry, because text alone cannot prove which test a panic belongs to. Each leg also runs `go mod download` before testing: cmd/codeaf-notices lists modules with GOPROXY=off, and a leg only downloads what it compiles, so with a cold module cache that test failed in the rest leg. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Collaborator
Author
Live proof on throwaway branches (workflow_dispatch, this branch's workflow at c3f3e98)
Runs: 36940090816, 36940093594, 36940096225, 36940099149, 36940101663. All of these ran with no cache (only a dev push saves the new keys), so they are the worst case. The light gate ran 3m36s–4m36s cold; it should drop once the first dev push saves its daily cache, and I'll check that on the first PR after merge. What the first round of live runs caught and this branch fixed before the round above:
The standing issue the proofs created (#1735) is closed: its comments are proof noise, and after merge the job opens a fresh one on the first real finding. |
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
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.
Why
A month of
touched packageshistory (2 Sep – 1 Oct, 357 red runs, each classified against the commits that followed) found:dev(12%), a flaky test (16%), or the selector itself (8%, fixed in ci: a changed directory in a nested module is not a package of the root module #1332);Every one of those not-your-fault reds cost a person or an agent a full investigate-and-rerun cycle. Separately, the job ran touched packages one after another, and its Go build cache had not been refreshed since 2026-09-10.
What changes
A failure is re-checked before it blames the change. Each named failing test is run once more on the head. If it passes, it is reported as flaky. If it fails again, it is run at the base commit (PR base SHA, previous push tip, or
HEAD~1on a dispatch); if it fails there too, it is reported as already failing on the base. Both warn, stay green, appear in the step summary, and get one comment on a standing issue titled touched packages: flaky or already-failing tests, so a flaky test still has an owner. Red, as before:Nothing is skipped by name and every test still runs. The log keeps the text Go would have printed (the attribution reads Go's JSON events behind it).
Parallel legs. Touched packages run as separate legs (
tui3,session,codeaf,rest) on separate runners, so a change spanning the heavy packages waits for the slowest leg instead of the sum.restuses-p 2on the 16 GB public runner; each leg printsnproc/free -g.cmd/codeafstays unsharded: sharding it was tried and its sharded runs failed. A job still namedtouched packagesaggregates the legs, andcheckis unchanged.A build cache that refreshes.
setup-gokeyed its cache ongo.sum, so every run restored the snapshot from the last dependency change and logged "not saving cache". Caches are now keyed by UTC day, saved by the firstdevpush of the day that misses, restored by PRs from the newest one, and left to GitHub's eviction (about 2 GB a day at most, against the 10 GB limit).One selector.
scripts/touched-packages.shis now the single selector for CI andmake test-touched, which also runs the same classifier locally (no issue calls).Rule change
docs/rules/ci.mdsaid a failure is never retried. This replaces that with one re-run of the failing tests plus a base comparison, because a flaky or inherited failure is a real bug that belongs on the standing issue, not on the PR that happened to touch the package.Validation
scripts/touched-packages_test.sh(no-Go change, each named leg, mixed,go.mod/go.sum→ whole tree, nested modules, emptied dirs) andscripts/touched-verdict_test.sh(flaky, inherited, own regression, absent test/package at base, unbuildable/skipped base, build failure, timeout, panic outside a test, over the cap, subtests, readable log, issue find-or-create/fork/API error). Both are inmake test-tooling.make pr-readygreen on this branch; YAML parses;go vet, gofmt, laws.devpush saves).🤖 Generated with Claude Code