fix(ci): read the toolchain directive, run -race per PR, bump otel (agent-os-7tl) - #7
Merged
Merged
Conversation
Acts on an independent QC review of the two session-1 commits. Both workflows resolved the Go version with `grep '^go ' backend/go.mod`, which returns 1.25.0 — the `go` directive, not the `toolchain go1.25.12` line below it. setup-go therefore installed 1.25.0, and the step was even named "Set up Go 1.25.0". CI did still compile against 1.25.12: GOTOOLCHAIN defaults to `auto`, so the go command silently re-execs into the pinned toolchain. The cache-miss run on PR #6 shows it — `GOTOOLCHAIN='auto'` in the setup step, then `go: downloading go1.25.12 (linux/amd64)` in the build. So there was no live exposure. But nothing in the workflow expressed that, and nothing enforced it: GOTOOLCHAIN=local anywhere would have quietly reverted CI to go1.25.0 and reinstated GO-2026-5856 with no failing check. `go-version-file: backend/go.mod` reads the toolchain directive directly and takes it over the `go` directive when both are present. A `go version` step now records what actually compiled, so the log answers this instead of leaving it to inference. Also from the review: - docker/Dockerfile joins backend.yml's paths filter. It holds one half of the Go version lockstep, and a Dockerfile-only drift previously triggered no backend job at all — the exact drift the lockstep exists to prevent. - -race moves to every PR. The nightly decision rested on an estimate of 4-6 minutes on a two-core runner; measured on the first real run it is 101s for the step and 119s for the job, against 39s for the unit job. It runs concurrently with unit, and integration.yml takes ~4.5min on the same PR, so it is not on the critical path. Removes the schedule trigger and both `if:` gates. - otel v1.41.0 -> v1.42.0, clearing GO-2026-5158 (baggage parsing no longer caps raw header length). Not reachable, but the previous commit's own rule — take the higher patch carrying further security work — was applied to go-git and go-billy and not to otel. Three comments stated things the evidence contradicts, now corrected: - The -race cost figure above. - manualremote_test.go claimed to exercise go-git's worktree.Status. It does not: agent-os-r1a kills the go-git path on every real clone, so everything below step 2a is served by the git CLI. The header now says so explicitly and lists what is never executed, and step 2a asserts the go-git surface that IS reachable (openRepo, repo.Head) so the file carries an honest, non-zero go-git claim. The behind-count is now derived from git rev-list rather than loosened to >= 1. - integration.yml claimed build+vet is what catches a build-constraint leak. True for internal/integrationtest/, whose harness is a non-test file behind the tag. False for internal/truth/, whose integration tests are self-contained: they vet clean and pass on a Docker-equipped runner. What catches those is backend.yml's Docker-free `go test`. Gates: go build, go vet, 657 tests across 9 packages, govulncheck down to the same 3 unfixable docker/docker advisories, manualremote harness green on both HTTPS and SSH. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n directive The previous commit switched both workflows to go-version-file: backend/go.mod on the strength of setup-go's README, which states that the toolchain directive wins when both are present. That is true of v6+; the workflows were still on @v5, which reads only the `go` directive. The observed result on PR #7 was unchanged from before the fix: Set up Go Setup go version spec 1.25.0 Set up Go Successfully set up Go version 1.25.0 Report Go version go: downloading go1.25.12 (linux/amd64) Report Go version go version go1.25.12 linux/amd64 so the comment claiming CI installed the pinned toolchain was false, while the new `go version` step correctly showed the GOTOOLCHAIN=auto re-exec still doing the work. Improved toolchain handling landed in setup-go v6.0.0 as a breaking change (PR actions/setup-go#460); v7.0.0 adds only an ESM migration on top. Pinning v6. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Acts on an independent QC review of the two session-1 commits. Nothing merged was broken; nothing is reverted.
The one that matters
Both workflows resolved the Go version with
grep '^go ' backend/go.mod, which returns 1.25.0 — thegodirective, not thetoolchain go1.25.12line below it.setup-gotherefore installed 1.25.0, and the step was even named "Set up Go 1.25.0".CI did still compile against 1.25.12.
GOTOOLCHAINdefaults toauto, so the go command silently re-execs into the pinned toolchain — visible on PR #6's cache-miss run:So there was no live exposure. But nothing in the workflow expressed that and nothing enforced it:
GOTOOLCHAIN=localanywhere would have quietly reverted CI to go1.25.0 and reinstated GO-2026-5856 with no failing check.go-version-file: backend/go.modreads the toolchain directive directly and prefers it over thegodirective. Ago versionstep now records what actually compiled.Also
docker/Dockerfilejoinsbackend.yml's paths filter. It holds one half of the Go version lockstep, and a Dockerfile-only drift previously triggered no backend job at all — the exact drift the lockstep exists to prevent.-racemoves to every PR. The nightly decision rested on an estimate of 4-6 minutes; measured on the first real run it is 101s for the step, 119s for the job, against 39s forunit. It runs concurrently, andintegration.ymltakes ~4.5min on the same PR, so it is not on the critical path. Removes the schedule trigger and bothif:gates.Three comments that stated things the evidence contradicts
-racecost figure above.manualremote_test.goclaimed to exercise go-git'sworktree.Status. It does not — agent-os-r1a kills the go-git path on every real clone, so everything below step 2a is served by the git CLI. The header now says so and lists what is never executed; step 2a asserts the go-git surface that is reachable (openRepo,repo.Head) so the file carries an honest non-zero claim; the behind-count is derived fromgit rev-listinstead of loosened to>= 1.integration.ymlclaimed build+vet is what catches a build-constraint leak. True forinternal/integrationtest/, whose harness is a non-test file behind the tag. False forinternal/truth/— those integration tests are self-contained, vet clean, and pass on a Docker-equipped runner. What catches them isbackend.yml's Docker-freego test.Gates: build, vet, 657 tests across 9 packages, govulncheck back to the same 3 unfixable
docker/dockeradvisories, manualremote harness green on HTTPS and SSH.What to look for in this PR's own checks: the Go setup step should name 1.25.12, and Race detector should run on a
pull_requestevent.Closes agent-os-7tl.
🤖 Generated with Claude Code