Conversation
… + errcheck v1.20.0
There was a problem hiding this comment.
I have enough information. Let me compile the findings and produce the final report.
Step 4c-sel CLASSIFY results:
- Judgment candidates matched by glob triggers on {Makefile, claude/task-runner.go, tools.env}:
changelog/conventional-prefix-required→ SKIPPED —CHANGELOG.mdhas no## Unreleasedsection; no bullet to checkgo-context/cancel-check-in-loop→ SKIPPED — loop boundslen(s)(bounded by string length, not external state)go-licensing/copyright-year-discipline→ SKIPPED — no.gocopyright year changes in diffgo-makefile/test-not-build-for-verification→ SKIPPED — Makefile change is only version bumps intools.env
Step 4d-sel ADJUDICATE results (verifying mechanical findings against actual file content):
| Mechanical Finding | Verdict |
|---|---|
changelog/unreleased-entry-required | CONFIRMED — CHANGELOG.md has no ## Unreleased section; autoRelease repo needs one |
go-architecture/counterfeiter-directive-on-interface | DISMISSED — //counterfeiter:generate IS present at line 16, mock exists at mocks/claude-task-runner.go; mechanical funnel missed the directive due to comment block above interface |
go-composition/no-package-function-calls-in-business-logic | CONFIRMED (architectural note) — glog.V(2).Infof, glog.Warningf, fmt.Sprintf, json.Unmarshal are package-level calls inside method bodies; existing pattern in codebase |
go-context/cancel-check-in-loop | DISMISSED — loop iterates len(s) over a string (bounded), not external iteration |
go-cli/slog-not-glog-in-new-projects | DISMISSED — glog used across the entire module; repo is existing project, exempt |
go-testing/counterfeiter-mocks-required | DISMISSED — mock exists at mocks/claude-task-runner.go |
Traceability:
- Candidates: 4 judgment rules from glob; 6 mechanical findings from funnel
- Applicable: 1 (changelog)
- Skipped: 3 judgment rules; 5 mechanical findings dismissed as false positives/exempt
Selector mode review summary:
Must Fix (Critical): None
Should Fix (Important):
CHANGELOG.md—## Unreleasedsection missing. This is an autoRelease repo — the release agent promotes## Unreleasedpost-merge; without it no version ships. Add a conventional-prefixed bullet (e.g.fix: go 1.27 tooling updates) under## Unreleased. (rule:changelog/unreleased-entry-required)
Nice to Have (Optional):
claude/task-runner.go:60,89—glog.V(2).Infofandglog.Warningfare package-level function calls in business logic. Consider wrapping glog behind aLoggerinterface injected via the constructor — this makes the dependency explicit and testable without relying on glog's global V-level state. (rule:go-composition/no-package-function-calls-in-business-logic)
Notes:
- precommit skipped (selector mode) — CI covers lint+test
- The
gofmtordering change in the Makefile (moved to last informattarget) is intentional and correct — ensuresgofmtruns aftergoimports-reviserandgolines, preventing tool conflicts - Tool version bumps (
golangci v2.12.2→v2.13.1,errcheck v1.10.0→v1.20.0) are appropriate for Go 1.27
{
"verdict": "request-changes",
"summary": "PR updates Go tooling versions for Go 1.27 compatibility. One confirmed finding: CHANGELOG.md lacks an `## Unreleased` section needed for autoRelease to ship a version. The counterfeiter/mock findings are false positives — directive and mock both exist. The glog architectural concern is noted as a pre-existing pattern, not introduced by this PR.",
"comments": [
{
"file": "CHANGELOG.md",
"line": 1,
"severity": "major",
"message": "CHANGELOG.md has no `## Unreleased` section. In an autoRelease repo the release agent promotes `## Unreleased` post-merge; without it no version ships. Add a conventional-prefixed bullet (e.g. `fix: go 1.27 tooling updates`) under `## Unreleased`. (rule: changelog/unreleased-entry-required)"
},
{
"file": "claude/task-runner.go",
"line": 60,
"severity": "nit",
"message": "glog.V(2).Infof is a package-level function call in business logic — hidden dependency. Wrap glog behind a Logger interface injected via the constructor for testability. (rule: go-composition/no-package-function-calls-in-business-logic)"
},
{
"file": "claude/task-runner.go",
"line": 89,
"severity": "nit",
"message": "glog.Warningf is a package-level function call in business logic — hidden dependency. Wrap glog behind a Logger interface injected via the constructor for testability. (rule: go-composition/no-package-function-calls-in-business-logic)"
}
],
"concerns_addressed": [
"correctness: deliver method signature changed from (ctx, result) (*T, error) to (ctx, result) *T — verified nil is never returned, callers correctly append , nil",
"correctness: gofmt ordering in Makefile — confirmed intentional (runs last after goimports-reviser and golines)",
"changelog: unreleased entry missing — not addressed, needs addition before merge",
"go-composition: glog package calls in task-runner.go — not an issue in this PR (pre-existing pattern, not introduced by this change)"
]
}superseded by new automated review
There was a problem hiding this comment.
Step 4c-sel: CLASSIFY
Changed files: CHANGELOG.md, Makefile, claude/task-runner.go, tools.env
Mechanical findings candidates from Step 4a are already provided. The mechanical findings include:
go-architecture/counterfeiter-directive-on-interface— line 16 already has the directive, SKIPPEDgo-cli/slog-not-glog-in-new-projects— existing project (glog used elsewhere), SKIPPEDgo-context/cancel-check-in-loop— loop iterateslen(s)(a string),sc.stepis a trivial struct field mutation, no I/O or blocking calls, SKIPPEDgo-testing/counterfeiter-mocks-required— line 16 has the directive, SKIPPEDgo-composition/no-package-function-calls-in-business-logic— APPLICABLE, genuine violations confirmed below
Judgment rule candidates (Step 4b-i): changelog/conventional-prefix-required applies to CHANGELOG.md; all others skipped (no agent/command/ADR/main.go/golangci files changed).
Step 4d-sel: ADJUDICATE
Verified claude/task-runner.go — confirmed violations for go-composition/no-package-function-calls-in-business-logic:
| Line | Call | Verdict |
|---|---|---|
| 46 | strings.TrimSpace(taskContent) | VIOLATION — stdlib package call in business logic |
| 77 | json.Unmarshal([]byte(jsonBlob), &agentResult) | VIOLATION — stdlib package call in business logic |
| 65, 71, 78 | fmt.Sprintf(...) | VIOLATION — stdlib package call in business logic |
Lines 47, 63, 84 (newErrorResult, deliver) are same-package calls, not pkg.Func() — not violations. Lines 59, 89 (glog.*) are exempt (existing project).
changelog/conventional-prefix-required: CHANGELOG has chore: — compliant, no finding.
Step 3a: LICENSE file present — no issue.
Step 5 — Consolidated Report
{
"verdict": "request-changes",
"summary": "The PR makes correct changes to tooling versions and format target ordering, but the refactored `deliver` method signature change (removing `error` return) is correct and the inline `, nil` return values are appropriate. However, `claude/task-runner.go` contains multiple direct stdlib package function calls in business logic (`strings.TrimSpace`, `json.Unmarshal`, `fmt.Sprintf`) that should be injected through interfaces per go-composition rules.",
"comments": [
{
"file": "claude/task-runner.go",
"line": 46,
"severity": "major",
"message": "Method body calls `strings.TrimSpace` directly — hidden stdlib dependency. Wrap each capability in a small interface, inject via the constructor, and call through the injected field. *(rule: go-composition/no-package-function-calls-in-business-logic)*"
},
{
"file": "claude/task-runner.go",
"line": 65,
"severity": "major",
"message": "Method body calls `fmt.Sprintf` directly — hidden stdlib dependency. Wrap each capability in a small interface, inject via the constructor, and call through the injected field. *(rule: go-composition/no-package-function-calls-in-business-logic)*"
},
{
"file": "claude/task-runner.go",
"line": 71,
"severity": "major",
"message": "Method body calls `fmt.Sprintf` directly — hidden stdlib dependency. Wrap each capability in a small interface, inject via the constructor, and call through the injected field. *(rule: go-composition/no-package-function-calls-in-business-logic)*"
},
{
"file": "claude/task-runner.go",
"line": 77,
"severity": "major",
"message": "Method body calls `json.Unmarshal` directly — hidden stdlib dependency. Wrap each capability in a small interface, inject via the constructor, and call through the injected field. *(rule: go-composition/no-package-function-calls-in-business-logic)*"
},
{
"file": "claude/task-runner.go",
"line": 78,
"severity": "major",
"message": "Method body calls `fmt.Sprintf` directly — hidden stdlib dependency. Wrap each capability in a small interface, inject via the constructor, and call through the injected field. *(rule: go-composition/no-package-function-calls-in-business-logic)*"
}
],
"concerns_addressed": [
"correctness: format target ordering verified — gofmt runs last after goimports-reviser and golines, correct sequence",
"security: golangci v2.13.1 and errcheck v1.20.0 version bumps are legitimate/releases, no behavioral concerns",
"go-composition/no-package-function-calls-in-business-logic: confirmed violations at lines 46, 65, 71, 77, 78 — strings.TrimSpace, fmt.Sprintf (×3), json.Unmarshal"
]
}Pre-existing debt findings (go-composition stdlib injection, flag/log style) unrelated to the Go 1.27 tooling PR; operator-approved override per classifier review.
Uh oh!
There was an error while loading. Please reload this page.
Canonical Go 1.27.0 precommit fix (mirrors go-skeleton e7818706). Reorders
formatsogofmt -wruns last; bumps tools.env (golangci v2.13.1, errcheck v1.20.0). Precommit green.