Skip to content

fix(tests-runner): install go.mod's toolchain release, resolved in the Go job - #309

Merged
topcoder1 merged 1 commit into
mainfrom
fix/tests-runner-go-toolchain
Oct 26, 2026
Merged

topcoder1 merged 1 commit into
mainfrom
fix/tests-runner-go-toolchain

Conversation

@topcoder1

Copy link
Copy Markdown
Owner

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 fix(coverage-floor): install go.mod's toolchain release, not its go-line floor #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 fix(coverage-floor): install go.mod's toolchain release, not its go-line floor #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 fix(coverage-floor): install go.mod's toolchain release, not its go-line floor #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 fix(coverage-floor): install go.mod's toolchain release, not its go-line floor #308's head, apart from the two conditions that don't apply here (language and pre-measured).

🤖 Generated with Claude Code

…e Go job

tests-runner.yml had the go-line-only resolver coverage-floor.yml had, so
a repo pinning `toolchain go1.26.10` over `go 1.26.1` would run its Go
tests on go1.26.1: setup-go exports GOTOOLCHAIN=local, which skips the
toolchain line. No fleet repo calls this reusable with Go yet.

Port coverage-floor's resolver (#308): the newer of
the toolchain pin and the go line, compared in Go's order; comments and CR
stripped; release candidates in setup-go's spelling; a toolchain line that
names no Go release fails. It runs in run-go, just before setup-go, not in
detect: a failure in detect would skip run-go, and a skipped required check
passes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@topcoder1 topcoder1 added the manual-merge Durable hold: never auto-merge; a human must click. Outranks all automerge bypasses. label Oct 26, 2026
@github-actions github-actions Bot added the risk:blocked Risk class: blocked label Oct 26, 2026
@github-actions

Copy link
Copy Markdown

Risk class: blocked — manual merge required.

This PR touches one of the blocked path categories from .github/risk-paths.yml (Dockerfiles, docker-compose, .github/workflows/**, **/.env*, **/secrets*, infra/, terraform/, k8s/, or the classifier config itself).

Auto-merge is refused by claude-author-automerge.yml. A maintainer should review the diff and click "Squash and merge" themselves.

(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.")

@github-actions

Copy link
Copy Markdown

The review is complete. The logic is sound:

  • Version resolution moved to run-go: Correct fix — a failure in detect would skip run-go, and a skipped required check passes, so the resolution must live in the job itself.
  • directive() awk function: Correctly strips CRLF and // comments (both spaced and unspaced) before extracting field $2; uses exit on first match so later duplicate lines don't interfere.
  • sort -V for version comparison: GNU sort places rc identifiers before the corresponding release (e.g. 1.27rc2 < 1.27.0), matching Go's own ordering. The comment acknowledges this and the tests verify it.
  • RC format conversion (1.27rc2 → 1.27.0-rc.2): Applied after the sort, covering both a go line with an RC and a toolchain pin with an RC.
  • Invalid toolchain gating: The regex TOOLCHAIN_RE rejects go1.26.10x, bare 1.26.10, and arbitrary words like banana; the elif block emits a structured ::error:: and exits 1 with the offending value quoted (no injection risk — the echo uses double-quotes but does not eval).
  • Fallback GO_VER="1.23" is correctly placed outside the if block, covering the no-go.mod and empty-result cases.
  • Tests are comprehensive: parametrised happy-path cases, structural test that enforces ordering of checkout/resolve/setup-go and wires the correct output expression into setup-go.

No issues found. Fix is correct, well-scoped, and pinned by a self-test that will catch future drift.

Claude Review · workflow run

@topcoder1
topcoder1 merged commit bbf5851 into main Oct 26, 2026
18 checks passed
@topcoder1
topcoder1 deleted the fix/tests-runner-go-toolchain branch October 26, 2026 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual-merge Durable hold: never auto-merge; a human must click. Outranks all automerge bypasses. risk:blocked Risk class: blocked

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant