ci: migrate golangci-lint to v2 and re-enable disabled workflows#11
Conversation
Both .github/workflows/ci.yml and release.yml were renamed to .disabled in the initial commit of this repo, so neither tests nor lint have ever run on PRs. Combined with .golangci.yml being v1 format (incompatible with the installed golangci-lint v2.x), this explained how stale assertions slipped past v0.1.8 (TestSupportedAgents missing AgentTypeGasCity) and v0.1.9 (the AGENT_ENV non-codex Codex case in agents_comprehensive_test.go) without anyone noticing locally or in CI. Changes: - Migrate .golangci.yml to v2 format using `golangci-lint migrate`. Add explicit errcheck exclusions to match the codebase's existing policy: skip _test.go files, exclude common cleanup patterns ((io.Closer).Close, os.Chmod, os.MkdirAll, Registry.Register, Environment.Remove, fmt.Sscanf). Exclude staticcheck QF1011 since `var _ Iface = impl` interface-conformance assertions need the explicit type. With these exclusions, `make check` passes cleanly. - Re-enable ci.yml: drop Go 1.22/1.23 from the matrix (go.mod requires 1.24), bump golangci/golangci-lint-action to v6 (v4 predates golangci-lint v2 config support). - Re-enable release.yml unchanged except for the same lint-action bump. Follow-up: 30 errcheck warnings exist in production paths (e.g. agent.go fmt.Sscanf, agents/register.go Register calls, sessions/claudecode.go defer file.Close) that are now hidden by exclusion patterns. A separate pass should review whether each is genuinely safe to ignore or should be tightened. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughGolangci-lint configuration migrated to v2 format; CI ChangesGo Linting Toolchain Upgrade
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.golangci.yml (1)
24-28: ⚡ Quick winTest-file errcheck blanket exclusion may hide legitimate test bugs.
Excluding all errcheck violations in
_test.gofiles can mask real issues where setup/teardown errors would cause test failures or false positives. For instance, failing to check errors fromos.MkdirAllin test setup could result in misleading test outcomes when subsequent writes fail.Consider narrowing exclusions to specific functions known to be safe in tests (e.g.,
(*testing.T).Cleanupregistration), rather than suppressing all error checks in test files.♻️ Narrower test exclusion example
- # Test files routinely use Close/Chmod/Register without error checks - # for setup and cleanup. Those errors aren't actionable in tests. - - path: _test\.go - linters: - - errcheck + # Allow specific helper calls to ignore errors in tests + # but catch genuine logic errors + # (If you must exclude, list specific functions above in exclude-functions)Alternatively, rely on the function-level
exclude-functionslist (lines 9-16) which already covers Close/Chmod/Register, and remove this blanket test-file exclusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.golangci.yml around lines 24 - 28, The current rule that disables the errcheck linter for all files matching the _test\.go path is too broad; replace this blanket exclusion by removing the path: _test\.go block for errcheck and instead rely on the existing function-level exclusions (the exclude-functions list used for Close/Chmod/Register) or add specific safe test helper functions (e.g., testing.T.Cleanup registration) to exclude-functions; update .golangci.yml to delete the path-based errcheck exclusion and, if needed, add only narrowly scoped function names to exclude-functions so test setup/teardown errors remain checked.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 31-34: Update the GitHub Actions step using golangci-lint to use
golangci-lint-action@v7 (replace golangci-lint-action@v6) so it supports
golangci-lint v2.x; ensure the existing "name: golangci-lint" step and "version:
latest" remain but point to the v7+ action to avoid the "v2 is not supported by
v6" failure.
In @.golangci.yml:
- Around line 7-16: The .golangci.yml excludes suppress error checks for
operations like (io.Closer).Close, (*os.File).Close, os.Chmod, os.MkdirAll,
(github.com/sageox/agentx.Registry).Register,
(*github.com/sageox/agentx.DefaultRegistryImpl).Register,
(github.com/sageox/agentx.Environment).Remove and fmt.Sscanf which can mask real
failures; remove these entries and audit each call site: for each use of
Close/Chmod/MkdirAll/Register/Remove/Sscanf either (a) propagate the error to
the caller, (b) handle and log the error where it occurs, or (c) explicitly
ignore it with `_ = ...` plus a short comment why ignoring is safe; after
changes run golangci-lint to confirm warnings are fixed and keep only deliberate
`_ =` ignores documented inline.
- Around line 1-48: The CI failure comes from using golangci-lint v2 config
(version: "2" in .golangci.yml) while workflows reference
golangci/golangci-lint-action@v6 which only supports v1; either update all
workflow job steps that reference golangci/golangci-lint-action@v6 to use
golangci/golangci-lint-action@v7 (or later) so the action supports v2 config, or
change the config version in .golangci.yml from "2" to the v1-compatible format
(version: "1") to match the existing `@v6` action—search for occurrences of
golangci/golangci-lint-action@v6 in your workflow files or the version: "2" key
in .golangci.yml to locate where to change.
---
Nitpick comments:
In @.golangci.yml:
- Around line 24-28: The current rule that disables the errcheck linter for all
files matching the _test\.go path is too broad; replace this blanket exclusion
by removing the path: _test\.go block for errcheck and instead rely on the
existing function-level exclusions (the exclude-functions list used for
Close/Chmod/Register) or add specific safe test helper functions (e.g.,
testing.T.Cleanup registration) to exclude-functions; update .golangci.yml to
delete the path-based errcheck exclusion and, if needed, add only narrowly
scoped function names to exclude-functions so test setup/teardown errors remain
checked.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 51f9a5fd-953c-4829-be44-fc4a5f0cbc66
📒 Files selected for processing (3)
.github/workflows/ci.yml.github/workflows/release.yml.golangci.yml
| version: "2" | ||
|
|
||
| linters: | ||
| # Default v2 linter set: errcheck, govet, ineffassign, staticcheck, unused. | ||
| # gofmt and goimports moved to formatters in v2 (configured below). | ||
| settings: | ||
| errcheck: | ||
| exclude-functions: | ||
| - (io.Closer).Close | ||
| - (*os.File).Close | ||
| - os.Chmod | ||
| - os.MkdirAll | ||
| - (github.com/sageox/agentx.Registry).Register | ||
| - (*github.com/sageox/agentx.DefaultRegistryImpl).Register | ||
| - (github.com/sageox/agentx.Environment).Remove | ||
| - fmt.Sscanf | ||
| exclusions: | ||
| generated: lax | ||
| paths: | ||
| - third_party$ | ||
| - builtin$ | ||
| - examples$ | ||
| rules: | ||
| # Test files routinely use Close/Chmod/Register without error checks | ||
| # for setup and cleanup. Those errors aren't actionable in tests. | ||
| - path: _test\.go | ||
| linters: | ||
| - errcheck | ||
| # QF1011 flags `var _ Iface = impl` interface-conformance assertions, | ||
| # but the explicit type is the whole point of the pattern. | ||
| - text: "QF1011" | ||
| linters: | ||
| - staticcheck | ||
|
|
||
| formatters: | ||
| enable: | ||
| - errcheck | ||
| - gosimple | ||
| - govet | ||
| - ineffassign | ||
| - staticcheck | ||
| - unused | ||
| - gofmt | ||
| - goimports | ||
|
|
||
| linters-settings: | ||
| goimports: | ||
| local-prefixes: github.com/sageox/agentx | ||
|
|
||
| issues: | ||
| exclude-use-default: false | ||
| settings: | ||
| goimports: | ||
| local-prefixes: | ||
| - github.com/sageox/agentx | ||
| exclusions: | ||
| generated: lax | ||
| paths: | ||
| - third_party$ | ||
| - builtin$ | ||
| - examples$ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the v2 configuration against golangci-lint's schema
# Check if the config is valid by running golangci-lint with --version to see output
golangci-lint --version 2>/dev/null || echo "golangci-lint not installed in sandbox"
# Look for any example v2 configs or migration documentation in the repo
fd -e yml -e yaml golangci | head -20Repository: sageox/agentx
Length of output: 148
🌐 Web query:
Is golangci-lint-action@v6 compatible with golangci-lint configuration version 2 format?
💡 Result:
No, golangci-lint-action@v6 is not compatible with golangci-lint configuration version 2 format. It is restricted to golangci-lint v1.x versions, which use config version 1.[1][2] Attempting to use golangci-lint v2 (which requires config version: "2"[3][4][5]) with v6 will throw an error: "golangci-lint v2 is not supported by golangci-lint-action v6, you must update to golangci-lint-action v7."[1] v7.0.0 and later support golangci-lint v2 only.[6] The action added JSONSchema verification (default true in later v6 releases), which fails on v2 configs with v1 tooling.[7][8]
Citations:
- 1: golangci/golangci-lint-action@2ee514f
- 2: feat: restrict action v6 on golangci-lint v1 golangci/golangci-lint-action#1194
- 3: Use golangci-lint v2 to lint with v1 config file? golangci/golangci-lint#5774
- 4: https://golangci-lint.run/docs/product/migration-guide/
- 5: https://golangci-lint.run/docs/configuration/file
- 6: golangci/golangci-lint-action@v6.5.0...v7.0.0
- 7: The action fails validation on custom linters configuration golangci/golangci-lint-action#1179
- 8: golangci/golangci-lint-action@v6.2.0...v6.5.0
🌐 Web query:
golangci-lint version 2 configuration format syntax for linters.settings and formatters sections
💡 Result:
In golangci-lint version 2, the configuration uses YAML format (also supports TOML/JSON). The top-level sections are linters and formatters, each with a settings subsection for per-linter/formatter options [1][2][3]. linters.settings syntax: Nested map where keys are linter names (lowercase, matching golangci-lint linters output) and values are linter-specific options as maps or primitives. Example: linters: settings: asasalint: exclude: - Append - \.Wrapf use-builtin-exclusions: false depguard: rules: main: list-mode: lax files: - $all allow: - $gostd [2][1] formatters.settings syntax: Nested map where keys are formatter names (e.g., gci, gofmt, gofumpt, goimports, golines, swaggo) and values are formatter-specific options. Example: formatters: settings: gci: sections: - standard - default - prefix(github.com/org/project) no-inline-comments: true custom-order: true goimports: local-prefixes: - github.com/my/project golines: max-len: 120 [3][4][5] Full reference in https://github.com/golangci/golangci-lint/blob/master/.golangci.reference.yml [1][6]. Use golangci-lint linters or golangci-lint formatters for lists. Config starts with version: "2" [1].
Citations:
- 1: https://golangci-lint.run/docs/configuration/file
- 2: https://golangci-lint.run/docs/linters/configuration/
- 3: https://golangci-lint.run/docs/formatters/configuration/
- 4: https://github.com/golangci/golangci-lint/blob/ab192139/pkg/config/formatters_settings.go
- 5: https://github.com/maratori/golangci-lint-config/blob/v2.8.0/.golangci.yml
- 6: https://github.com/golangci/golangci-lint/blob/master/.golangci.reference.yml
🏁 Script executed:
fd -e yml -e yaml | xargs grep -l "golangci-lint-action" 2>/dev/null | head -10Repository: sageox/agentx
Length of output: 39
🏁 Script executed:
find .github/workflows -type f -name "*.yml" -o -name "*.yaml" 2>/dev/null | xargs cat 2>/dev/null | grep -A2 -B2 "golangci" 2>/dev/null || echo "No workflows found or no golangci references"Repository: sageox/agentx
Length of output: 376
Incompatible action version for v2 configuration format.
The .golangci.yml file uses version: "2", but the workflow files reference golangci/golangci-lint-action@v6, which only supports golangci-lint v1.x configuration. This will cause CI failures with the error: "golangci-lint v2 is not supported by golangci-lint-action v6, you must update to golangci-lint-action v7."
Update the workflow files to use golangci/golangci-lint-action@v7 or later, or downgrade .golangci.yml to v1 configuration format.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.golangci.yml around lines 1 - 48, The CI failure comes from using
golangci-lint v2 config (version: "2" in .golangci.yml) while workflows
reference golangci/golangci-lint-action@v6 which only supports v1; either update
all workflow job steps that reference golangci/golangci-lint-action@v6 to use
golangci/golangci-lint-action@v7 (or later) so the action supports v2 config, or
change the config version in .golangci.yml from "2" to the v1-compatible format
(version: "1") to match the existing `@v6` action—search for occurrences of
golangci/golangci-lint-action@v6 in your workflow files or the version: "2" key
in .golangci.yml to locate where to change.
| errcheck: | ||
| exclude-functions: | ||
| - (io.Closer).Close | ||
| - (*os.File).Close | ||
| - os.Chmod | ||
| - os.MkdirAll | ||
| - (github.com/sageox/agentx.Registry).Register | ||
| - (*github.com/sageox/agentx.DefaultRegistryImpl).Register | ||
| - (github.com/sageox/agentx.Environment).Remove | ||
| - fmt.Sscanf |
There was a problem hiding this comment.
Excluding error checks creates technical debt and potential correctness risks.
The exclude-functions list suppresses error checking for operations that can fail in production:
os.Chmod,os.MkdirAll- filesystem permission/creation failures(io.Closer).Close,(*os.File).Close- resource cleanup failuresRegistry.Register- registration failuresfmt.Sscanf- parse failures
While the PR objectives acknowledge ~30 production-path errcheck warnings are hidden, these exclusions can mask real bugs. For example, unhandled MkdirAll failures could lead to subsequent file-write crashes, and ignored Register errors could result in missing agent registrations at runtime.
Consider auditing each suppressed case to either explicitly ignore (with inline _ = ... and comment), propagate errors, or log them. This addresses the technical debt before it compounds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.golangci.yml around lines 7 - 16, The .golangci.yml excludes suppress error
checks for operations like (io.Closer).Close, (*os.File).Close, os.Chmod,
os.MkdirAll, (github.com/sageox/agentx.Registry).Register,
(*github.com/sageox/agentx.DefaultRegistryImpl).Register,
(github.com/sageox/agentx.Environment).Remove and fmt.Sscanf which can mask real
failures; remove these entries and audit each call site: for each use of
Close/Chmod/MkdirAll/Register/Remove/Sscanf either (a) propagate the error to
the caller, (b) handle and log the error where it occurs, or (c) explicitly
ignore it with `_ = ...` plus a short comment why ignoring is safe; after
changes run golangci-lint to confirm warnings are fixed and keep only deliberate
`_ =` ignores documented inline.
The first PR run installed golangci-lint v1.64.8 because action v6 with `version: latest` doesn't pick a v2 release. The v2 config then fails schema verification on v1's binary. Action v7+ supports v2; v9 is the current major. Pin lint to v2.12.2 (latest v2.x at the time of writing) so future v3 releases don't quietly break CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
.golangci.ymlv1 → v2 with errcheck exclusions matching the codebase's existing policy.make checknow passes cleanly (1101 tests, 0 lint findings).ci.ymlandrelease.yml(renamed from.disabledin the initial commit, so neither has ever run). Drop Go 1.22/1.23 from the CI matrix (go.modrequires 1.24). Bumpgolangci/golangci-lint-actionv4 → v6 (v4 predates lint v2 config support).Why
While prepping the v0.1.10 release, two stale test assertions surfaced that should have been caught long ago:
TestSupportedAgentswas missingAgentTypeGasCity(added toSupportedAgentsin v0.1.8).TestAllAgents_Detect/Codex/AGENT_ENV non-codexstill assertedfalseafter v0.1.9'se907e81madeDetectRuntimestrictly authoritative for Codex (the equivalent assertion incodex_test.gowas updated; this one was missed).Both slipped past local verification and CI. Investigation: CI was renamed
.disabledin3efd314(initial commit) and never re-enabled, while.golangci.ymlwas a v1 config unreadable by the installedgolangci-lint v2.11.4— somake checkerrored out on config load before tests ran. Nothing was gating PRs.What's in the new lint config
golangci-lint migrateplus targeted exclusions:errcheckfor_test.gofiles (test setup/cleanup routinely ignores errors that aren't actionable in tests).(io.Closer).Close,(*os.File).Close,os.Chmod,os.MkdirAll,Registry.Register,Environment.Remove,fmt.Sscanffrom errcheck — these match patterns already in production code that are deliberate ignores.QF1011(it flagsvar _ Iface = implinterface-conformance assertions, but the explicit type is the whole point of the pattern).Follow-up (not in this PR)
30 errcheck warnings exist in production paths (
agent.gofmt.Sscanf,agents/register.goRegister calls,sessions/claudecode.godeferredfile.Close, etc.) that are now hidden by the exclusion patterns. A separate pass should audit each — for genuinely safe ones, replace with explicit_ = fn()at the call site; for unsafe ones, propagate or log the error. Tracking left as a known follow-up per maintainer preference.Test plan
make checkpasses locally (1101 tests, 0 lint findings)golangci-lint run ./...reports0 issues.release.ymland produces a GitHub release from CHANGELOG.md🤖 Generated with Claude Code
Summary by CodeRabbit