chore: errcheck cleanup + flip lint job to blocking#71
Merged
Conversation
golangci-lint reports 0 issues across the project (default-standard linters,
5 minute timeout). The lint job in CI lost its continue-on-error: true and
is now a blocking gate.
What changed:
- 44 silent event-store / projection-store sites (Append + Project) across
internal/cli + internal/engine now log with full story-ID context.
Lost audit-trail events were the load-bearing risk here — every story
state transition now has a non-silent failure path.
- 15 dangerous data-write sites now return wrapped errors:
- improve/discovery.go, improve/opportunities.go, improve/repolearn.go:
f.Write to opportunity/learning JSONL files
- memory/server.go: revenue/opportunity/source persistence
- state/sqlite.go: projectStoryRewritten Exec calls (title/desc/AC/complexity)
- engine/executor.go: launch-config artifact + log-dir creation
- engine/monitor_post_execution.go: review/QA artifact writes
- cli/init.go: store close errors propagate
- cli/{approve,reject,resume}.go: event append/project errors propagate
- ~30 best-effort cleanup sites carry explicit `_ =` discards with a
one-line rationale (deferred Close, git rebase --abort after a failed
rebase, exec.Cmd.Run() for stale-branch cleanup, fmt.Sscanf with valid
zero-fallback semantics).
- .golangci.yml gains exclude-functions for benign noise: fmt.Fprint*
writes to stdout/stderr, (io.Closer).Close, tabwriter.Flush, HTTP body
close. Test-file exemption widened to cover all linters (not just
errcheck) so test refactor hints (QF1011, QF1003, etc.) don't fail CI.
- staticcheck QF1012 / QF1002 / QF1001 / SA9003 fixes across production
code (WriteString(fmt.Sprintf) → fmt.Fprintf, tagged switch, De Morgan).
- CRLF→LF normalised in metrics.go (was the reason a perl rewrite missed
half the file).
Verified: go build, go vet, full test suite (30 packages) pass.
v2.5.0 was built with Go 1.25 and rejects the Go 1.26.4 toolchain pinned in PR #66. v2.12.2 is built with the current Go release line.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
`golangci-lint` reports 0 issues across the project (default-standard linters, 5 minute timeout). The `lint` job in CI lost its `continue-on-error: true` and is now a blocking gate.
Before: 473 errcheck hits + 9 staticcheck QF/SA warnings. After: 0.
What changed
Silent event-store failures fixed (44 sites) in `internal/cli/{approve,reject,resume}.go` + `internal/engine/{conflict_resolver,monitor_dispatch,monitor_escalation,monitor_post_execution,reaper,supervisor,watchdog}.go`. Every story state transition now logs append/project failures with full story-ID context. This is the load-bearing part of the PR — lost audit-trail events were the real risk.
Dangerous data-write sites now return wrapped errors (15):
Best-effort cleanup sites (~30) carry explicit `_ =` discards with one-line rationale (deferred Close, `git rebase --abort` after failed rebase, exec.Cmd.Run for stale-branch cleanup, `fmt.Sscanf` with valid zero-fallback semantics).
Config-level noise suppression in `.golangci.yml`: `exclude-functions` covers benign sites (`fmt.Fprint*` to stdout/stderr, `(io.Closer).Close`, `tabwriter.Flush`, HTTP body close). Test-file exemption widened from errcheck-only to all linters so test refactor hints (QF1011/QF1003) don't fail CI.
Staticcheck QF/SA fixes in production code:
Misc: `metrics.go` CRLF→LF normalised (it was the reason a perl rewrite missed half the file).
Test plan