Repository navigation
fix(coverage-floor): install go.mod's toolchain release, not its go-line floor - #308
Conversation
…ine floor The detect step read only go.mod's `go` directive and handed it to setup-go as an exact version. setup-go then exports GOTOOLCHAIN=local, so a repo pinning `toolchain go1.26.10` over `go 1.26.1` had its required coverage lane measured on go1.26.1, green and with no warning (whois-api-llc/techrecon#1213, job 120625436528). Prefer a `toolchain goV` line, fall back to the `go` line, and fail on a toolchain line that names no Go release instead of measuring the floor again. A caller's go_version input still wins. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… does Codex pre-review of the toolchain fix found three valid go.mod files it mishandled, each reproduced: - `go 1.26.1` + `toolchain go1.26.0` installed 1.26.0, which cannot build the module under GOTOOLCHAIN=local (`go: go.mod requires go >= 1.26.1`). Install the newer of the two, as Go treats a lower pin as stale. - `toolchain go1.26.10//pin` failed the job, though Go ends the token at `//`. Strip comments and CR before reading either directive. - `toolchain go1.27rc2` went to setup-go as 1.27rc2, which it cannot match (it compares against 1.27.0-rc.2). Fail it with a message that names the go_version form instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex pre-review: a caller passing pre_measured_coverage_artifact installs no Go, yet a toolchain line the detect step will not install from (go1.27rc2, valid Go) failed that caller's job before the artifact download. Resolve the Go release only when the run measures coverage itself, and pin the env wiring that tells detect which run it is. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Independent review: failing an rc toolchain sent owners to a go_version override, and that override outlives the pin, the same silent mismatch this series removes. An rc go line (`go 1.27rc1`) also already reached setup-go as 1.27rc1, which it cannot match. Compare in Go's spelling, which sort -V orders as Go does (1.27rc2 < 1.27.0, checked on GNU and BSD sort), then hand the winner to setup-go as 1.27.0-rc.2, the form its README documents and its matcher accepts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Risk class: This PR touches one of the blocked path categories from Auto-merge is refused by (This is a policy notice, not a code-quality failure. The classify job itself does not fail — required CI checks remain authoritative for "is the code green.") |
|
The diff is small and focused. I've reviewed both changed files:
No issues found. Diff is well-tested and the logic is correct. Claude Review · workflow run |
…e Go job (#309) ## What was wrong `tests-runner.yml`'s `detect` job resolved the Go version the same way `coverage-floor.yml` did. It read only go.mod's `go` directive and gave it to `actions/setup-go`, which then exports `GOTOOLCHAIN=local`, so `go` skips go.mod's `toolchain` line. A Go repo that pins a patched toolchain would run its tests on the unpatched floor, green and with no warning. Reproduced against this branch's base: on techrecon's go.mod (`go 1.26.1` + `toolchain go1.26.10`), the shipped `detect` step outputs `go_version=1.26.1`. #308 fixes the same code in coverage-floor.yml, where it measured techrecon's coverage on go1.26.1. This PR ports that fix. ## The fix - The resolver from #308, unchanged in its rules: - A caller's `go_version` input wins, used as given. - Otherwise it installs the newer of the `toolchain goV` pin and the `go` line, compared in Go's order (`1.27rc2 < 1.27.0`). - `//` comments and CR are stripped first. - A release candidate goes to setup-go as `1.27.0-rc.2`, the only spelling setup-go matches. - A toolchain line that names no Go release fails with an `::error::` quoting the line. - `toolchain default`, or no toolchain line, installs the `go` line; no go.mod installs `1.23`. - **It runs in `run-go`, in a new step between checkout and setup-go, not in `detect`.** If the step failed in `detect`, `run-go` would be skipped through `needs`, and a skipped required check passes. Today a line like `toolchain banana` reaches `go test`, which rejects go.mod and turns the Go check red. Failing in `detect` would make that a skip. In `run-go`, the failure lands on the Go check itself. - `detect` loses its `go_version` output, its `INPUT_GO_VERSION` env and its go.mod read. Nothing else consumed them: `run-go` was the only reader, and `workflow_call` exposes no outputs. - The `go_version` input's description now states the default. The resolver is copied, not shared. A shared action would have to be pinned by SHA and land in a separate merge first, as `pinned-gate-tests` is. If #308 changes in review, keep the two copies in step. ## Fleet repos whose behavior changes None. - No repo with a go.mod calls `tests-runner.yml`. That comes from a tree survey of all 145 non-archived repos in `topcoder1` and `whois-api-llc`. - The 7 callers code search finds (attaxion_dev, crawl-infra, dotclaude, wxa-jake-ai, wxa-secrets, wxa_marketing, wxa_sanctions) are Python or JS. For them `run-go` never runs, and `detect` only drops an output they cannot read. ## Tests `selftest/test_tests_runner_go_version.py` runs the shipped resolver step, in its working directory, against hand-written go.mod files. Every expected version is a literal. It has 22 cases: - 17 resolve cases, the same set as #308's. - 3 toolchain lines that must fail the Go job: `1.26.10`, `go1.26.10x`, `banana`. - A negative control: a commented-out `// toolchain`, a misspelled `toolchian` and a `require …/toolchain` path must not move the version. A positive control through the same harness then finds a real directive. - A wiring check: - The resolver sits in `run-go`, after checkout and before setup-go. - setup-go installs its output. - It sees `inputs.working_directory` and `inputs.go_version`. Results: - **RED on `main`:** 22 failed, because `run-go` has no resolver step. The old `detect` step separately reproduced `1.26.1` on techrecon's go.mod. - **GREEN after:** 22 passed, under bash 5.3, under macOS `/bin/bash` 3.2, and with GNU `sort` first on PATH. - **Mutation check:** 20 of 20 mutants killed. They were the 15 resolver mutants from #308's check, setup-go reading `detect`'s old output, no `working-directory`, `go_version` not wired, the resolver moved after setup-go, and the resolver moved back into `detect`. - **Existing guards:** `test_tests_runner_pinned_gates.py`, which drives `detect`, still passes (24 tests). - **Lint:** pinned ruff 0.15.10 check and format are clean, actionlint 1.7.12 is clean, and shellcheck is clean on the new step and on `detect`. - **Full selftest suite on `2c49b0c`:** 1302 passed, 1 skipped, in 9m56s. The skip is environmental and predates this change. Auto-merge rationale: manual merge. PRs to topcoder1/ci-workflows are always click-merged, and this changes a reusable that fleet repos call for their required test lanes. Codex rounds: 2, run in parallel on `2c49b0c`. Both were clean. The resolver itself already went through 8 rounds on #308. An independent code-reviewer agent approved `2c49b0c` with no finding over the bar: - Python, JS and pinned-gate callers are unaffected. - No `needs.detect.outputs.go_version` remains, and actionlint would flag a dangling one. - The step order is checkout → resolve → setup-go → tests. - The step's executable lines match coverage-floor.yml's at #308's head, apart from the two conditions that don't apply here (language and pre-measured). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
What was wrong
coverage-floor.yml's detect step read only go.mod'sgodirective and gave it toactions/setup-goas an exact version. setup-go then exportsGOTOOLCHAIN=local(itssetGoToolchain()), sogoruns whatever was installed and skips go.mod'stoolchainline. A repo that pins a patched toolchain had its required coverage lane measured on the unpatched floor, green and with no warning.Seen on whois-api-llc/techrecon, which pins
toolchain go1.26.10for GO-2026-6809 (crypto/x509Certificate.Verify) overgo 1.26.1:Setup go version spec 1.26.1,go version go1.26.1,GOTOOLCHAIN='local'.b853a3a44:ci.yml's test job (it usesgo-version-file: go.mod) runsgo1.26.10. Coverage-floor run 40261908797 runsgo1.26.1.The fix
When the caller passes no
go_versionand the run measures coverage itself, the detect step resolves the Go release this way:go_versioninput wins, used as given. Unchanged.//comment. Go ends a token at//even with no space before it (toolchain go1.26.10//pin).toolchain goVpin, orgoV-suffixfor a custom build of V, installs the newer of V and thegoline, compared in Go's own order. A pin below thegoline is stale: Go ignores it, and underGOTOOLCHAIN=localthe older release cannot build the module (go: go.mod requires go >= 1.26.2 (running go 1.26.1; GOTOOLCHAIN=local)).toolchain default, or no toolchain line, installs thegoline. Unchanged.1.27.0-rc.2. setup-go'sfindMatchnormalizes the candidate versions, not the requested one, so Go's1.27rc2matches nothing; that was already true for an rcgoline. The comparison happens before this mapping, in Go's spelling.sort -Vorders that spelling as Go does (1.27 < 1.27rc1 < 1.27rc2 < 1.27.0 < 1.27.1, checked on GNU coreutils 9.11 and macOS sort), so a stale rc pin still loses to its release.toolchain 1.26.10,toolchain go1.26.10x) fails the job with an::error::that quotes the line, instead of measuring the floor again unseen.1.23. Unchanged.A pre-measured run (
pre_measured_coverage_artifact) installs no Go, so it never reads go.mod, and no toolchain line can fail it. Thego_versioninput gets a description that states the default.Why not
go-version-file: go.mod: setup-go v7's own version-file mode has these same gaps. It readstoolchainonly whenGOTOOLCHAINisn'tlocalat parse time. It falls back to thegoline silently on a toolchain line its regex can't match. It takes a pin below thegoline as given. And it returns rc toolchains as1.27rc2, which its own matcher then can't find. Resolving in the detect step also keeps the chosen version visible as the step's output, and lets the selftest run the shipped step with literal expectations.Fleet repos whose behavior changes
I enumerated all 145 non-archived repos in
topcoder1andwhois-api-llcthrough the tree API (no truncated trees), not code search alone. 22 have a go.mod, and 2 have atoolchainline. Code search across all owners found no callers anywhere else; its positive controls (techrecon, wxa-graph) appeared.language: gogo 1.26.1+toolchain go1.26.10language: gogo 1.26.10language: gogo 1.26.10language: pythongo 1.26.1+toolchain go1.26.10techrecon is the only repo that changes. Every other Go lane there already runs 1.26.10: they read go.mod through
go-version-file, anddeploy.ymlpins1.26.10exactly. Itsci.ymltest job is green on main on 1.26.10, so the coverage lane joins them.Not in this PR
tests-runner.yml's detect step has the same go-line-only resolver it shared with this file. No fleet repo callstests-runner.ymlwith Go today, so it changes nothing yet. It's a follow-up.Tests
selftest/test_coverage_floor_go_version.pyruns the shipped detect step against hand-written go.mod files, and every expected version is a literal. It has 24 cases:go 1.26.9+go1.26.10), toolchain above the go line, a spaced and an unspaced//comment, two stale pins below the go line, an rc pin, a stale rc pin below its release, an rc go line, CRLF,default,-suffix, caller input wins (twice), no go.mod.1.26.10,go1.26.10x,banana.// toolchain, a misspelledtoolchianand arequire …/toolchainmodule path must not move the version. A positive control through the same harness then finds a real directive.go_versionandpre_measured_coverage_artifact.Results:
/bin/bash3.2, and with GNUsortfirst on PATH.14f6780: 1304 passed, 1 skipped, in 9m18s. The skip is environmental and predates this change.gobinary, or from setup-go's source and README, before it was fixed.Auto-merge rationale: manual merge. PRs to topcoder1/ci-workflows are always click-merged, and this changes a reusable that every Go repo's required coverage lane calls.
Codex rounds: 8, run in pairs on each commit, and every finding was reproduced before it was fixed:
edce29b: 3 P2s. A stale pin below the go line won, an unspaced//comment failed the job, and the rc spelling was one setup-go can't install. Fixed in2ccd64f; the rc part was revised in14f6780.2ccd64f: 1 P2. Pre-measured runs failed on go.mod. Fixed in5740d56.5740d56: clean.14f6780: clean.An independent code-reviewer agent separately found the stale-pin and rc issues on
edce29b. It approvededce29b..5740d56with one pre-existing P3 (an rc go line) and recommended normalizing rc rather than failing it. That is14f6780, which also fixes the P3. Its check of14f6780: approve, with no finding over the bar. Its probes under bash 5.3 and 3.2, with both BSD and GNU sort, all agree.🤖 Generated with Claude Code