Skip to content

fix(ci): read the toolchain directive, run -race per PR, bump otel (agent-os-7tl) - #7

Merged
thinkbig1979 merged 2 commits into
mainfrom
fix/ci-go-version-and-otel
Jul 30, 2026
Merged

fix(ci): read the toolchain directive, run -race per PR, bump otel (agent-os-7tl)#7
thinkbig1979 merged 2 commits into
mainfrom
fix/ci-go-version-and-otel

Conversation

@thinkbig1979

Copy link
Copy Markdown
Owner

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 — 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 — visible on PR #6's cache-miss run:

Set up Go 1.25.0   GOTOOLCHAIN='auto'
Build              go: downloading go1.25.12 (linux/amd64)

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 prefers it over the go directive. A go version step now records what actually compiled.

Also

  • 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; measured on the first real run it is 101s for the step, 119s for the job, against 39s for unit. It runs concurrently, 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. 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 that stated things the evidence contradicts

  • 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 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 from git rev-list instead of 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/ — those integration tests are self-contained, vet clean, and pass on a Docker-equipped runner. What catches them is backend.yml's Docker-free go test.

Gates: build, vet, 657 tests across 9 packages, govulncheck back to the same 3 unfixable docker/docker advisories, 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_request event.

Closes agent-os-7tl.

🤖 Generated with Claude Code

thinkbig1979 and others added 2 commits July 30, 2026 21:56
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>
@thinkbig1979
thinkbig1979 merged commit 4284189 into main Jul 30, 2026
5 checks passed
@thinkbig1979
thinkbig1979 deleted the fix/ci-go-version-and-otel branch July 30, 2026 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant