Skip to content

fix(toolchain): build the Go analysis battery with the go1.27 toolchain (RIG-3303) - #904

Merged
trunk-sudo[bot] merged 2 commits into
mainfrom
compass-repo/rig-3303-go127-analysis-battery
Sep 5, 2026
Merged

fix(toolchain): build the Go analysis battery with the go1.27 toolchain (RIG-3303)#904
trunk-sudo[bot] merged 2 commits into
mainfrom
compass-repo/rig-3303-go127-analysis-battery

Conversation

@rigel-mintaka

@rigel-mintaka rigel-mintaka commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Renovate #650 bumped the go compiler to 1.27.1, but the Go analysis battery (golangci-lint, govulncheck, go-licenses, nilaway) came from the bare nixpkgs attributes, built with go1.26. A go1.26-built analyzer cannot parse the go1.27 stdlib and fails every run with file requires newer Go version go1.27 (application built with go1.26). nilaway runs first in the compass-go:ci aggregate and failed fast, so only its skew showed on main — but golangci-lint and govulncheck are the same class. This reddened moon (go) -> rollup (the required check) and gated the whole merge queue since 2026-09-05T17:27Z.

This is the fix-forward Matt ruled: keep go 1.27.1 and fix the skewed side, so the compiler and every analyzer stay on one Go version.

What changed

  • tools/toolchain/versions/go-analysis.nix — new pins for the battery. Two need a release past the nixpkgs pin to understand go1.27, not just a rebuild: nilaway (nixpkgs ships x/tools v0.31.0, which cannot parse go1.27; pinned to an upstream rev carrying v0.45.0) and golangci-lint (nixpkgs ships 2.12.2, whose bundled staticcheck panics on the go1.27 IR; 2.13.0 added go1.27 support, pinned to 2.13.2).
  • tools/toolchain/go-analysis.nix — new shared builder: rebuilds all four with the go-overlay toolchain the code compiles against (each package.nix takes a different buildGo*Module arg — buildGoModule / buildGo126Module / buildGoLatestModule — so the override names them per tool). go-licenses additionally wraps its binary's GOROOT with its own top-level go arg, so that arg gets the toolchain too (a passGo flag). Both the dev shell and the CI parity gate import this one module and pass the same goToolchain, so they resolve one store path per tool.
  • devenv.nix / tools/toolchain/gate-tools.nix — the four tools move out of the parsed with pkgs packages literal (no longer bare nixpkgs attrs) into the appended toolchain list and the store-path-checked langs set, exactly as goToolchain is handled. The parity gate now verifies CI's battery is byte-identical to the dev shell's.
  • go/internal/runner/host.go — the newer nilaway (struct-init-v2) re-flags the Resolve()-receiver cluster in agent.go that the old build guarded. All 15 findings trace to one unguarded callsite (host.go:320); a handle == nil guard there clears them, matching the existing pattern at :593/:706/:840/:947.
  • go/.golangci.yml — golangci-lint 2.13 renamed exhaustruct -> exhaustruct_v5 (deprecating the old name); disable both so the idiom-fighting linter stays off under default: all, mirroring the existing wsl/wsl_v5 pair.
  • go/server/service.go — drop a //nolint:unparam on forward that 2.13's nolintlint reports as unused (the newer unparam no longer flags it).

Review outcome

Reviewed against every lens with the nix mechanics verified by direct build/eval (not read). All four load-bearing invariants — single-derivation, parity-parser contract, host.go guard, golangci config — verified to hold empirically. Findings: 1 high, 2 medium, 3 low.

  • HIGH — fixed (commit 2). go-licenses was not actually de-skewed: it wraps its binary with --set GOROOT '${go}/share/go' via a separate top-level go arg the builder override left at go1.26.5, so it classified a go1.27 build's stdlib against a go1.26 GOROOT — the exact skew this PR removes for the other three. Threading the toolchain through that arg (the passGo flag above) fixes it: verified the wrapper GOROOT now points at go-1.27.1 and the closure carries zero go-1.26 references (was one). This also drops the duplicate go toolchain from the closure (~219 MB).
  • LOW ×3 — fixed (commit 2). rebuild docstring notes pin fully replaces src, plus an assert that a malformed pin names itself at the versions file; the ci.yml and build-and-ci.md battery-provenance comments now say the battery is rebuilt (not nixpkgs-provided) and rides phase one; the parity-core fixture gained the goAnalysis.* appended-list refs + assertion. (The reviewer-named eng-docs mirror copy is gather-generated and gitignored — it regenerates from the canonical docs/ fix, no separate edit.)
  • MEDIUM ×2 — deferred to RIG-3313 (deliberate). (a) Putting the battery in phase-one langs is a measured 4.1x regression on the setup bootstrap step, because the rebuilt derivations are non-substitutable and ~9 jobs build them for a PATH only moon (go) uses. The correct fix — a dedicated gate-tools.nix output the parity gate folds in, so only the go job pays — touches parity.ts/parity-core.ts/ci.yml, real blast radius on the parity gate, so it is out of scope for an urgent MQ-unblock. The HIGH fix already removed ~219 MB of the 304 MB closure growth. Accepting the residual from-source rebuild cost as the price of the fix-forward is a deliberate call, recorded here per the reviewer. (b) The new pin file is not yet Renovate-managed; fix(toolchain): build the Go analysis battery with the go1.27 toolchain (RIG-3303) #904 stamps it MANUALLY MAINTAINED with a bump policy, and RIG-3313 covers wiring the customManagers.

Verification

  • moon run compass-go:ci — the full aggregate (fmt/vet/lint/nilaway/test/build/vuln/licenses) — passes on the go1.27 tree. This is the exact aggregate red on main.
  • The toolchain parity gate passes: all 71 pinned tools match the dev shell; each battery tool store-path-verified as its go1.27.1 build. go-licenses now resolves to the de-skewed 1v7xi932… build.
  • compass-go:lint reports 0 issues; compass-go:nilaway clean; parity-core unit tests 23 pass.

Spec-impact: none. Refs RIG-3303. Refs #650. Follow-up: RIG-3313.

Co-authored-by: Matt Wilkinson matt@rigel.build

…in (RIG-3303)

Renovate #650 bumped the go compiler to 1.27.1, but the Go analysis battery (golangci-lint, govulncheck, go-licenses, nilaway) came from the bare nixpkgs attributes, built with go1.26. A go1.26-built analyzer cannot parse the go1.27 stdlib and fails every run with `file requires newer Go version go1.27 (application built with go1.26)`. nilaway runs first in the `compass-go:ci` aggregate and failed fast, so only its skew showed on main — but golangci-lint and govulncheck are the same class. This reddened `moon (go)` -> `rollup` (the required check) and gated the whole merge queue since 2026-09-05T17:27Z.

This is the fix-forward Matt ruled: keep go 1.27.1 and fix the skewed side, so the compiler and every analyzer stay on one Go version.

## What changed

- `tools/toolchain/versions/go-analysis.nix` — new pins for the battery. Two need a release past the nixpkgs pin to understand go1.27, not just a rebuild: nilaway (nixpkgs ships x/tools v0.31.0, which cannot parse go1.27; pinned to an upstream rev carrying v0.45.0) and golangci-lint (nixpkgs ships 2.12.2, whose bundled staticcheck panics on the go1.27 IR; 2.13.0 added go1.27 support, pinned to 2.13.2).
- `tools/toolchain/go-analysis.nix` — new shared builder: rebuilds all four with the go-overlay toolchain the code compiles against (each package.nix takes a different buildGo*Module arg — buildGoModule / buildGo126Module / buildGoLatestModule — so the override names them per tool). Both the dev shell and the CI parity gate import this one module and pass the same `goToolchain`, so they resolve one store path per tool.
- `devenv.nix` / `tools/toolchain/gate-tools.nix` — the four tools move out of the parsed `with pkgs` `packages` literal (no longer bare nixpkgs attrs) into the appended toolchain list and the store-path-checked `langs` set, exactly as `goToolchain` is handled. The parity gate now verifies CI's battery is byte-identical to the dev shell's.
- `go/internal/runner/host.go` — the newer nilaway (struct-init-v2) re-flags the `Resolve()`-receiver cluster in `agent.go` that the old build guarded. All 15 findings trace to one unguarded callsite (host.go:320); a `handle == nil` guard there clears them, matching the existing pattern at :593/:706/:840/:947.
- `go/.golangci.yml` — golangci-lint 2.13 renamed `exhaustruct` -> `exhaustruct_v5` (deprecating the old name); disable both so the idiom-fighting linter stays off under `default: all`, mirroring the existing `wsl`/`wsl_v5` pair.
- `go/server/service.go` — drop a `//nolint:unparam` on `forward` that 2.13's nolintlint reports as unused (the newer unparam no longer flags it).

## Verification

- `moon run compass-go:ci` — the full aggregate (fmt/vet/lint/nilaway/test/build/vuln/licenses/sqlc-drift) — passes on the go1.27 tree. This is the exact aggregate red on main.
- The toolchain parity gate passes: all 71 pinned tools match the dev shell; each battery tool store-path-verified as its go1.27.1 build.
- `compass-go:lint` reports 0 issues; `compass-go:nilaway` clean.

Spec-impact: none. Refs RIG-3303. Refs #650.

Co-authored-by: Matt Wilkinson <matt@rigel.build>
@linear-code

linear-code Bot commented Sep 5, 2026

Copy link
Copy Markdown

RIG-3303

@trunk-io

trunk-io Bot commented Sep 5, 2026

Copy link
Copy Markdown

😎 Merged successfully via direct-to-queue (branch protection was bypassed for this merge) - details.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Compass engineering docs preview: https://compass-repo-rig-3303-go127.compass-eng-docs.pages.dev

Deployed from compass-repo/rig-3303-go127-analysis-battery at 6138084.

Changed pages:

…303)

Review of #904 found go-licenses was not actually de-skewed: it wraps its
binary with `--set GOROOT '${go}/share/go'` using its own top-level `go` arg,
which the earlier fix left at nixpkgs' go1.26.5 while rebuilding the binary
itself with go1.27. So go-licenses classified a go1.27 build's stdlib against a
go1.26 GOROOT — the exact skew this PR removes for the other three tools.

- **go-analysis.nix**: thread the go1.27 toolchain through a rebuilt tool's own
  `go` arg via a new `passGo` flag, set on go-licenses. Verified: its wrapper
  GOROOT now points at go-1.27.1 and its closure carries zero go-1.26 refs
  (was one). Also add an `assert` that a `pin` carries `hash` + (`tag`|`rev`).
- **versions/go-analysis.nix**: correct the comment that claimed go-licenses
  needs "only the compiler rebuild", and flag the file as manually maintained
  (not yet Renovate-managed — follow-up tracked).
- **ci.yml / build-and-ci.md**: the phase-two nixpkgs-attrs step and the docs
  both listed the Go analysis battery among the tools it installs; it is not —
  the battery is rebuilt against the go toolchain and rides phase one via
  gate-tools.nix `langs`. Corrected both.
- **parity-core.test.ts**: extend the devenv fixture's appended `++ [ … ]` list
  with `goAnalysis.*` refs and assert the parser never yields them, covering the
  real block's shape.

Verified: `moon run compass-go:ci` green on go1.27 (all 10 tasks incl. licenses
+ vuln); parity 71/71; parity-core unit tests 23 pass.

Spec-impact: none. Refs RIG-3303

Co-authored-by: Matt Wilkinson <matt@rigel.build>
@mattwilkinsonn

Copy link
Copy Markdown
Contributor

/trunk merge --direct --priority=urgent

@trunk-sudo
trunk-sudo Bot merged commit ab56c27 into main Sep 5, 2026
17 checks passed
@trunk-sudo
trunk-sudo Bot deleted the compass-repo/rig-3303-go127-analysis-battery branch September 5, 2026 22:23
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.

2 participants