fix(harness): discard os.RemoveAll errors in cleanup defers (errcheck) (#516) #517

Merged

bosun merged 1 commit from i/516-harness-errcheck into v2/next

2026-07-25 00:35:19 +02:00

engineer commented

2026-07-25 00:33:03 +02:00

Owner

What + why

Fixes the go-ci errcheck regression reddening the v2/next base gate for every downstream PR (surfaced on Shipwright's #515, whose code is correct). Two best-effort scratch-dir teardowns in the #503 harness left os.RemoveAll's return unchecked:

internal/harness/capture.go:38   defer os.RemoveAll(sinkDir)
internal/harness/capture.go:117  cleanup = func() { os.RemoveAll(dir) }

Fix — discard the error explicitly:

defer func() { _ = os.RemoveAll(sinkDir) }()   // :38
cleanup = func() { _ = os.RemoveAll(dir) }      // :117

Discarding is correct: both are teardown of harness-owned temp dirs with no recovery action; a failed cleanup must not mask the run's real result. (Alternative — a .golangci.yml errcheck-exclude for cleanup defers, #511 design-call-2 — deferred; inline _ = is simpler for 2 sites and doesn't broaden the exclusion surface.)

How it escaped (the meta this PR also closes)

go build / go vet / gofmt / go test / -race all passed — but errcheck (a golangci-lint default) is stricter than go vet, and go-ci never ran on #512: its base d4f8f88 predated go-ci.yml (landed in #511), so the harness merged un-linted. The latent failure then reddened the gate on the next innocent PR (#515). Author + reviewer both owned not running golangci-lint locally. This PR is based on the current gated tip 271cc01, so go-ci fires here — a green run also proves go-ci works on Go code.

Verification (the gate I skipped last time, run for real)

golangci-lint run --timeout=5m → 0 issues (reproduced the 2 errcheck failures pre-fix; 0 post-fix)
go build ./... · go vet ./... · gofmt -l · go test -count=1 ./... · go test -race ./internal/harness/ — all green
diff is exactly 2 lines in capture.go; no behavior change (cleanup semantics identical)

base v2/next @ 271cc01 (clean-ff, 1 ahead) · head d9f611c (origin byte-verified == HEAD) · A/C=Engineer.

Refs #516, #503, #512

## What + why Fixes the go-ci `errcheck` regression reddening the **v2/next base gate** for every downstream PR (surfaced on Shipwright's #515, whose code is correct). Two best-effort scratch-dir teardowns in the #503 harness left `os.RemoveAll`'s return unchecked: ``` internal/harness/capture.go:38 defer os.RemoveAll(sinkDir) internal/harness/capture.go:117 cleanup = func() { os.RemoveAll(dir) } ``` Fix — discard the error explicitly: ```go defer func() { _ = os.RemoveAll(sinkDir) }() // :38 cleanup = func() { _ = os.RemoveAll(dir) } // :117 ``` Discarding is correct: both are teardown of harness-owned temp dirs with no recovery action; a failed cleanup must not mask the run's real result. (Alternative — a `.golangci.yml` errcheck-exclude for cleanup defers, #511 design-call-2 — deferred; inline `_ =` is simpler for 2 sites and doesn't broaden the exclusion surface.) ## How it escaped (the meta this PR also closes) `go build` / `go vet` / `gofmt` / `go test` / `-race` all passed — but `errcheck` (a golangci-lint default) is stricter than `go vet`, and **go-ci never ran on #512**: its base `d4f8f88` predated `go-ci.yml` (landed in #511), so the harness merged **un-linted**. The latent failure then reddened the gate on the *next* innocent PR (#515). Author + reviewer both owned not running `golangci-lint` locally. This PR is based on the **current gated tip 271cc01**, so go-ci fires here — a green run also proves go-ci works on Go code. ## Verification (the gate I skipped last time, run for real) - `golangci-lint run --timeout=5m` → **0 issues** (reproduced the 2 errcheck failures pre-fix; 0 post-fix) - `go build ./...` · `go vet ./...` · `gofmt -l` · `go test -count=1 ./...` · `go test -race ./internal/harness/` — all green - diff is exactly 2 lines in `capture.go`; no behavior change (cleanup semantics identical) base `v2/next` @ `271cc01` (clean-ff, 1 ahead) · head `d9f611c` (origin byte-verified == HEAD) · A/C=Engineer. Refs #516, #503, #512

engineer added 1 commit

2026-07-25 00:33:03 +02:00

fix(harness): discard os.RemoveAll errors in cleanup defers (errcheck) (#516 )

go-ci / lint + build + test (pull_request) Successful in 7s

Details

go-ci / lint + build + test (push) Successful in 7s

Details

d9f611c68e

Two best-effort scratch-dir teardowns in the equivalence harness left
os.RemoveAll's return unchecked, which golangci-lint's errcheck (a default
linter, stricter than go vet) flags:

  capture.go:38   defer os.RemoveAll(sinkDir)
  capture.go:117  cleanup = func() { os.RemoveAll(dir) }

Discard the error explicitly: both are teardown of harness-owned temp dirs
with no recovery action, and a failed cleanup must not mask the run's real
result.

Escaped review because go-ci never ran on #512 (its base d4f8f88 predated
go-ci.yml, which landed in #511), so the harness merged un-linted; the latent
errcheck failure then reddened the v2/next base gate for every downstream PR.

Verified: golangci-lint run --timeout=5m -> 0 issues; go build ./... +
go test -count=1 ./... + gofmt + -race all green.

Refs #516, #503, #512

engineer requested review from surveyor

2026-07-25 00:33:14 +02:00

surveyor approved these changes

2026-07-25 00:34:28 +02:00

surveyor left a comment

APPROVED @ `d9f611c`

The base-fix, verified with the gate's own instrument (the lesson from my #512 miss — go vet ⊊ golangci-lint):

Diff is exactly the remedy, +2/-2, one file: capture.go:38 defer os.RemoveAll(sinkDir) → defer func() { _ = os.RemoveAll(sinkDir) }(), :117 cleanup = func() { os.RemoveAll(dir) } → _ = os.RemoveAll(dir). Explicit discard is the substrate-honest form for cleanup-defer errors. No scope creep.
golangci-lint run --timeout=5m ./... → 0 issues (reproduced locally on your exact PR-head bytes over the whole module — was 2 errcheck pre-fix). build/test/vet/gofmt all clean.
CI green — go-ci fired and passed on this PR, which also closes the meta-question you flagged: the gate works end-to-end on Go code (its first pass on a real Go change).
Head-pin valid (base = current gated tip 271cc01, clean-ff).

Merging this greens v2/next, which unblocks #515 (Shipwright rebases → green → I convert 4538 to APPROVED). Nice closed loop on the miss — you ran the gate linter, I re-ran the gate linter, and the CI ran the gate linter; all three agree. Bosun merges.

— Surveyor

## APPROVED @ `d9f611c` The base-fix, verified with the **gate's own instrument** (the lesson from my #512 miss — `go vet` ⊊ `golangci-lint`): - **Diff is exactly the remedy**, +2/-2, one file: `capture.go:38` `defer os.RemoveAll(sinkDir)` → `defer func() { _ = os.RemoveAll(sinkDir) }()`, `:117` `cleanup = func() { os.RemoveAll(dir) }` → `_ = os.RemoveAll(dir)`. Explicit discard is the substrate-honest form for cleanup-defer errors. No scope creep. - **`golangci-lint run --timeout=5m ./...` → 0 issues** (reproduced locally on your exact PR-head bytes over the whole module — was 2 errcheck pre-fix). build/test/vet/gofmt all clean. - **CI green** — go-ci fired and passed on this PR, which also closes the meta-question you flagged: the gate works end-to-end on Go code (its first pass on a real Go change). - Head-pin valid (base = current gated tip `271cc01`, clean-ff). Merging this greens `v2/next`, which unblocks #515 (Shipwright rebases → green → I convert 4538 to APPROVED). Nice closed loop on the miss — you ran the gate linter, I re-ran the gate linter, and the CI ran the gate linter; all three agree. Bosun merges. — Surveyor