Scaffold ships lint-dirty: fresh repos fail task check on errcheck (deferred shutdown + resp.Body.Close) #1

New Issue

Closed

opened 2026-06-26 14:37:11 +00:00 by mathias · 3 comments

mathias commented 2026-06-26 14:37:11 +00:00

Owner

Copy Link

Copy Source

Problem

The template scaffolds lint-dirty — a fresh repo generated from it fails task check on errcheck from the first commit, before any user code is added. Every new project inherits a red CI run until someone root-causes it.

Evidence

Discovered while building mathias/brain-gardener (scaffolded from this template). The initial scaffold commit's CI went red on the Lint/Vet job. Root cause: two dropped deferred errors flagged by errcheck:

1. shutdown(ctx) in cmd/__PROJECT_NAME__/main.go (deferred shutdown return value ignored).
2. resp.Body.Close() in the litellm client (pkg/litellm/).

Both are the classic errcheck-on-deferred-call pattern.

Fix

Wrap the deferred calls to explicitly discard the error, e.g.:

defer func() { _ = shutdown(ctx) }()
// and
defer func() { _ = resp.Body.Close() }()

(Applied downstream in brain-gardener 4ad4237; should be fixed at the template source so it doesn't recur.)

Acceptance

• A freshly generated repo from this template passes task check (lint + vet + go test -race) green on its first CI run, with no hand-fixing.
• Verify by generating a throwaway repo and confirming run #1 is green.

Impact

Low severity but high friction — every create_project_from_template consumer (brain-gardener, and future Go agents) starts with a red main + red first PR, which masks real failures and erodes the "green main" signal during early dev.

## Problem The template scaffolds **lint-dirty** — a fresh repo generated from it fails `task check` on `errcheck` from the first commit, before any user code is added. Every new project inherits a red CI run until someone root-causes it. ## Evidence Discovered while building `mathias/brain-gardener` (scaffolded from this template). The initial scaffold commit's CI went red on the Lint/Vet job. Root cause: two dropped deferred errors flagged by `errcheck`: 1. `shutdown(ctx)` in `cmd/__PROJECT_NAME__/main.go` (deferred shutdown return value ignored). 2. `resp.Body.Close()` in the litellm client (`pkg/litellm/`). Both are the classic `errcheck`-on-deferred-call pattern. ## Fix Wrap the deferred calls to explicitly discard the error, e.g.: ```go defer func() { _ = shutdown(ctx) }() // and defer func() { _ = resp.Body.Close() }() ``` (Applied downstream in brain-gardener `4ad4237`; should be fixed at the template source so it doesn't recur.) ## Acceptance - A freshly generated repo from this template passes `task check` (lint + vet + `go test -race`) green on its **first** CI run, with no hand-fixing. - Verify by generating a throwaway repo and confirming run #1 is green. ## Impact Low severity but high friction — every `create_project_from_template` consumer (brain-gardener, and future Go agents) starts with a red `main` + red first PR, which masks real failures and erodes the "green main" signal during early dev.

mathias commented 2026-06-26 15:33:31 +00:00

Author

Owner

Copy Link

Copy Source

Confirmed: the two-line fix resolves it downstream — offering to push it to the template source

I'm the brain-gardener build that surfaced this. Confirming the root cause + fix from the consumer side, and offering to close it at the source.

Verified downstream (mathias/brain-gardener):

• Fresh scaffold's CI was red from commit 0 — task check failed errcheck on exactly the two deferred calls named here.
• Fix applied in 4ad4237:

  defer func() { _ = shutdown(ctx) }()      // cmd/<name>/main.go
  defer func() { _ = resp.Body.Close() }()  // pkg/litellm/model.go (line 115)
  

• Result: task check green (lint 0 issues, vet clean, go test -race green), CI run #4 fully green. So the fix is correct and sufficient.

Offer: I can push the same two-line change to this template (cmd/__PROJECT_NAME__/main.go + pkg/litellm/model.go) so the next create_project_from_template consumer starts green on run #1 — satisfying this issue's acceptance ("freshly generated repo passes task check on its first CI run"). It's a trivial, surgical, behavior-preserving change.

Two adjacent hardening ideas — want either?

1. Add errcheck to a pre-commit / template-self-CI so the template itself is gated and can't regress lint-dirty again. (The template has no CI running its own task check today, which is why this shipped.)
2. A .golangci.yml committed in the template with the house lint config, so consumers inherit identical lint behaviour instead of golangci defaults.

Say the word and I'll PR the two-line fix (small-scope, just this issue) — and separately #1/#2 if you want the template hardened against recurrence. Which scope do you want?

## Confirmed: the two-line fix resolves it downstream — offering to push it to the template source I'm the brain-gardener build that surfaced this. Confirming the root cause + fix from the consumer side, and offering to close it at the source. **Verified downstream** (`mathias/brain-gardener`): - Fresh scaffold's CI was red from commit 0 — `task check` failed `errcheck` on exactly the two deferred calls named here. - Fix applied in `4ad4237`: ```go defer func() { _ = shutdown(ctx) }() // cmd/<name>/main.go defer func() { _ = resp.Body.Close() }() // pkg/litellm/model.go (line 115) ``` - Result: `task check` green (lint 0 issues, vet clean, `go test -race` green), CI run #4 fully green. So the fix is correct and sufficient. **Offer:** I can push the same two-line change to this template (`cmd/__PROJECT_NAME__/main.go` + `pkg/litellm/model.go`) so the next `create_project_from_template` consumer starts green on run #1 — satisfying this issue's acceptance ("freshly generated repo passes `task check` on its **first** CI run"). It's a trivial, surgical, behavior-preserving change. **Two adjacent hardening ideas — want either?** 1. **Add `errcheck` to a pre-commit / template-self-CI** so the template itself is gated and can't regress lint-dirty again. (The template has no CI running its own `task check` today, which is why this shipped.) 2. **A `.golangci.yml`** committed in the template with the house lint config, so consumers inherit identical lint behaviour instead of golangci defaults. Say the word and I'll PR the two-line fix (small-scope, just this issue) — and separately #1/#2 if you want the template hardened against recurrence. Which scope do you want?

mathias commented 2026-06-26 21:27:18 +00:00

Author

Owner

Copy Link

Copy Source

Scope: two-line fix + template-self-CI (#1). Defer .golangci.yml (#2).

Take both the fix and #1 — and #1 is the more important half. The root cause isn't the two dropped errors; it's that the template runs no CI on itself, so lint-dirty shipped undetected and will regress again on the next template edit without a gate. The two-line fix treats the symptom; template-self-CI treats the cause and is what actually satisfies this issue's intent ("fresh repos start green") durably. A template that scaffolds every downstream Go agent is high-leverage — it earns its own task check gate.

→ PR both together:

1. The two-line errcheck fix (cmd/__PROJECT_NAME__/main.go + pkg/litellm/model.go).
2. Template-self-CI: a workflow running task check (lint+vet+go test -race) on the template's own pushes/PRs, so it can't ship lint-dirty again. Mind the __PROJECT_NAME__ placeholder — the self-CI needs to either substitute a dummy name or run check in a way that tolerates the placeholder (whichever's cleaner; flag if the placeholder fights the build).

Defer #2 (.golangci.yml) — it's not a bug fix, it's a standardization decision with fan-out: every future consumer would inherit that exact ruleset, and changing it later is a multi-repo edit. That belongs to a deliberate "house Go lint standard" call, not bundled into this PR. File it as a separate follow-up issue here so it's tracked but unblocked from the fix.

Verify acceptance the way you stated it: generate a throwaway repo from the template post-fix and confirm CI run #1 is green with no hand-fixing.

## Scope: two-line fix + template-self-CI (#1). Defer `.golangci.yml` (#2). **Take both the fix and #1 — and #1 is the more important half.** The root cause isn't the two dropped errors; it's that **the template runs no CI on itself**, so lint-dirty shipped undetected and *will* regress again on the next template edit without a gate. The two-line fix treats the symptom; template-self-CI treats the cause and is what actually satisfies this issue's intent ("fresh repos start green") durably. A template that scaffolds every downstream Go agent is high-leverage — it earns its own `task check` gate. → **PR both together:** 1. The two-line `errcheck` fix (`cmd/__PROJECT_NAME__/main.go` + `pkg/litellm/model.go`). 2. Template-self-CI: a workflow running `task check` (lint+vet+`go test -race`) on the template's own pushes/PRs, so it can't ship lint-dirty again. Mind the `__PROJECT_NAME__` placeholder — the self-CI needs to either substitute a dummy name or run check in a way that tolerates the placeholder (whichever's cleaner; flag if the placeholder fights the build). **Defer #2 (`.golangci.yml`)** — it's not a bug fix, it's a *standardization* decision with fan-out: every future consumer would inherit that exact ruleset, and changing it later is a multi-repo edit. That belongs to a deliberate "house Go lint standard" call, not bundled into this PR. **File it as a separate follow-up issue** here so it's tracked but unblocked from the fix. Verify acceptance the way you stated it: generate a throwaway repo from the template post-fix and confirm CI run #1 is green with no hand-fixing.

mathias referenced this issue 2026-07-21 21:19:21 +00:00

fix(ci): Lint / Test / Vet failing since bootstrap — corrects stale "CD-deploy-gap-only" assumption #3

mathias added the risk/lowsize/xstype/fix labels 2026-08-27 05:25:47 +00:00

mathias added the agent/ready label 2026-08-27 05:28:18 +00:00

mathias referenced this issue from a commit 2026-09-17 16:00:07 +00:00

fix(template): children were red because ./... cannot see the entrypoint

mathias closed this issue 2026-09-17 16:00:07 +00:00

mathias commented 2026-09-17 16:01:42 +00:00

Author

Owner

Copy Link

Copy Source

Fixed in 2b3851a (v0.1.0), verified 2026-09-17.

Your acceptance criterion was the right one — "verify by generating a throwaway repo and confirming run #1 is green" — and it is now enforced on every run rather than done once by hand.

The cause was not in the code

The template's own task check has been green this whole time. That is not because the defect was absent; it is because Go ignores directories whose names begin with _ or . when expanding ./....

$ go list ./...
__MODULE_PATH__/internal/agent
__MODULE_PATH__/pkg/litellm          <- no cmd package

cmd/__PROJECT_NAME__ starts with an underscore, so go build ./..., go vet ./..., go test ./... and golangci-lint run ./... all silently skipped the one package every generated repo is built around. After substitution the directory has a real name, becomes visible, and the error appears.

Template green, children red, by construction — and invisible from either repo's CI, which is why this sat open since June.

Demonstrated rather than reasoned about:

template:  golangci-lint run ./...  ->  0 issues
child:     cmd/childproj/main.go:25:16: Error return value is not
           checked (errcheck)  ->  defer shutdown(ctx)

What changed

The live defect: defer shutdown(ctx) dropped an error (SetupTelemetry returns func(context.Context) error). Now wrapped, with the underscore-directory reason recorded inline so the next person does not delete the wrapper as noise.

scripts/tests/generated-child_test.sh, wired into task check: substitutes the tracked tree into a throwaway child and gates go list / build / vet / golangci-lint on the result. Four assertions, the first being that the entrypoint is visible at all — without it the other three would pass for the wrong reason.

Verified in CI, not just locally (run 7509):

task: [check] bash scripts/tests/generated-child_test.sh
  ok   the generated child's entrypoint is visible to go list
  ok   the generated child builds
  ok   the generated child passes go vet
  ok   the generated child passes golangci-lint
4 passed, 0 failed

That log check mattered: the harness skips when golangci-lint is absent, so a green job would otherwise not prove it had run.

Rejected alternative, recorded so it is not retried

golangci-lint run ./cmd/__PROJECT_NAME__ does work — naming a path directly bypasses the pattern exclusion. But it reports a gofumpt violation on the import block that does not exist in any child: the formatter's import grouping depends on the module path, and the template's is a placeholder. Wiring that into check would turn the gate red over an artifact of being a template.

Proven to fail when it should

It went red twice before going green — once on the unfixed entrypoint, and once when it was still reading git archive HEAD and reported a stale pass against an unstaged fix. It now reads tracked files from the working tree, because a check inside task check has to gate what is about to be committed.

---

Created via git-mcp on behalf of @mathias

**Fixed in `2b3851a` (v0.1.0), verified 2026-09-17.** Your acceptance criterion was the right one — *"verify by generating a throwaway repo and confirming run #1 is green"* — and it is now enforced on every run rather than done once by hand. ## The cause was not in the code The template's own `task check` has been green this whole time. That is not because the defect was absent; it is because **Go ignores directories whose names begin with `_` or `.` when expanding `./...`**. ``` $ go list ./... __MODULE_PATH__/internal/agent __MODULE_PATH__/pkg/litellm <- no cmd package ``` `cmd/__PROJECT_NAME__` starts with an underscore, so `go build ./...`, `go vet ./...`, `go test ./...` **and** `golangci-lint run ./...` all silently skipped the one package every generated repo is built around. After substitution the directory has a real name, becomes visible, and the error appears. Template green, children red, by construction — and invisible from either repo's CI, which is why this sat open since June. Demonstrated rather than reasoned about: ``` template: golangci-lint run ./... -> 0 issues child: cmd/childproj/main.go:25:16: Error return value is not checked (errcheck) -> defer shutdown(ctx) ``` ## What changed The live defect: `defer shutdown(ctx)` dropped an error (`SetupTelemetry` returns `func(context.Context) error`). Now wrapped, with the underscore-directory reason recorded inline so the next person does not delete the wrapper as noise. `scripts/tests/generated-child_test.sh`, wired into `task check`: substitutes the tracked tree into a throwaway child and gates `go list` / `build` / `vet` / `golangci-lint` on **the result**. Four assertions, the first being that the entrypoint is visible at all — without it the other three would pass for the wrong reason. Verified in CI, not just locally (run 7509): ``` task: [check] bash scripts/tests/generated-child_test.sh ok the generated child's entrypoint is visible to go list ok the generated child builds ok the generated child passes go vet ok the generated child passes golangci-lint 4 passed, 0 failed ``` That log check mattered: the harness skips when `golangci-lint` is absent, so a green job would otherwise not prove it had run. ## Rejected alternative, recorded so it is not retried `golangci-lint run ./cmd/__PROJECT_NAME__` *does* work — naming a path directly bypasses the pattern exclusion. But it reports a **gofumpt** violation on the import block that does not exist in any child: the formatter's import grouping depends on the module path, and the template's is a placeholder. Wiring that into `check` would turn the gate red over an artifact of being a template. ## Proven to fail when it should It went red twice before going green — once on the unfixed entrypoint, and once when it was still reading `git archive HEAD` and reported a stale pass against an unstaged fix. It now reads tracked files from the working tree, because a check inside `task check` has to gate what is about to be committed. --- _Created via git-mcp on behalf of @mathias_

mathias referenced this issue 2026-09-17 16:01:56 +00:00

Template ships a non-green baseline: missing go.sum + two errcheck lint bugs #2

Sign in to join this conversation.

No Branch/Tag Specified

Branches Tags

main

eval/t1/c1/r0

eval/t1/c1/r0-new

v0.1.2

v0.1.1

v0.1.0

Labels

Clear labels

agent/ready

Meets the agent-ready contract; authorizes unattended execution

blocked/client

Waiting on the client

blocked/decision

Waiting on an unmade architectural decision; do not work until it resolves

blocked/supplier

Waiting on a supplier

blocked/upstream

Waiting on an upstream project or vendor

risk/high

Irreversible, security-relevant, or control plane

risk/low

Reversible, local, test-covered

risk/medium

Touches shared state, deploys or data

size/l

Too big - split it

size/m

Under two days

size/s

Under half a day

size/xs

Under an hour

type/chore

Maintenance, no behaviour change

type/decision

Human decision required; never agent/ready

type/docs

Documentation only

type/feat

New capability

type/fix

Corrects broken behaviour

type/ops

Infrastructure, deploy, runtime

type/research

Investigate; output is a finding, not code

type/test

Test coverage only

No labels agent/ready risk/low size/xs type/fix

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

mathias

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: mathias/template-go-agent#1