From 26ba5d2c6031975ef757c78be962a73127fa06c8 Mon Sep 17 00:00:00 2001 From: Ryan Simmen Date: Mon, 27 Apr 2026 08:50:52 -0400 Subject: [PATCH 1/4] chore: tighten golangci lint checks --- .golangci.yml | 3 ++- AGENTS.md | 2 +- CONTRIBUTING.md | 2 +- cmd/mato/main_test.go | 2 +- internal/config/config.go | 5 +++-- internal/doctor/check_tasks.go | 3 ++- internal/frontmatter/frontmatter.go | 3 ++- internal/graph/graph.go | 2 +- internal/merge/merge.go | 2 +- internal/merge/squash.go | 4 ++-- internal/queue/cancel.go | 2 +- internal/queue/claim.go | 6 +++--- internal/queue/manifest_test.go | 3 ++- internal/queue/queue.go | 2 +- internal/queue/resolve_test.go | 3 ++- internal/queue/runnable_backlog_test.go | 3 ++- internal/runner/task.go | 4 ++-- 17 files changed, 29 insertions(+), 22 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 1fbe84ea..d9af905a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,6 +3,7 @@ linters: default: none enable: - errcheck + - errorlint - govet - ineffassign - staticcheck @@ -15,13 +16,13 @@ linters: presets: - comments - common-false-positives - - legacy - std-error-handling paths: - vendor formatters: enable: - gofmt + - goimports exclusions: generated: lax paths: diff --git a/AGENTS.md b/AGENTS.md index 33028266..652a16d2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,7 +13,7 @@ make verify # run build, vet, lint, deadcode, and un # Lint go vet ./... # built-in static analysis -make lint # golangci-lint (errcheck, gosimple, govet, ineffassign, staticcheck, unused, gofmt) +make lint # golangci-lint (errcheck, errorlint, govet, ineffassign, staticcheck, unused, gofmt, goimports) make deadcode # detect unreachable exported code and unused symbols # Format diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 823ba196..c0f02dda 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -76,7 +76,7 @@ Coverage measurement and regression-test evidence are documented in [Testing](do ## Code Style -- Format Go code with `gofmt`; `make verify` runs `golangci-lint`, which includes `gofmt` checks. +- Format Go code with `gofmt`; `make verify` runs `golangci-lint`, which includes `gofmt` and `goimports` checks. - Follow idiomatic Go naming and structure, including the conventions in [Effective Go](https://go.dev/doc/effective_go) and [Go Code Review Comments](https://go.dev/wiki/CodeReviewComments). - Keep imports grouped and sorted by Go tooling conventions. - Prefer small, minimal changes over broad refactors unless the refactor is the point of the work. diff --git a/cmd/mato/main_test.go b/cmd/mato/main_test.go index 3cfc186c..ad43536e 100644 --- a/cmd/mato/main_test.go +++ b/cmd/mato/main_test.go @@ -1247,7 +1247,7 @@ func TestDoctorCmd_HardFailurePropagated(t *testing.T) { if err == nil { t.Fatal("expected hard failure error, got nil") } - if err != hardErr { + if !errors.Is(err, hardErr) { t.Errorf("error = %v, want %v", err, hardErr) } diff --git a/internal/config/config.go b/internal/config/config.go index 799c7320..454ca513 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -3,6 +3,7 @@ package config import ( "bytes" + "errors" "fmt" "io" "os" @@ -94,7 +95,7 @@ func LoadFile(path string) (Config, error) { dec := yaml.NewDecoder(bytes.NewReader(data)) dec.KnownFields(true) if err := dec.Decode(&cfg); err != nil { - if err == io.EOF { + if errors.Is(err, io.EOF) { return Config{}, nil } return Config{}, fmt.Errorf("parse config file %s: %w", path, err) @@ -102,7 +103,7 @@ func LoadFile(path string) (Config, error) { // Reject multi-document YAML: a second Decode must return io.EOF. var extra interface{} - if err := dec.Decode(&extra); err != io.EOF { + if err := dec.Decode(&extra); !errors.Is(err, io.EOF) { return Config{}, fmt.Errorf("parse config file %s: contains multiple YAML documents", path) } diff --git a/internal/doctor/check_tasks.go b/internal/doctor/check_tasks.go index 9783d12e..db1683eb 100644 --- a/internal/doctor/check_tasks.go +++ b/internal/doctor/check_tasks.go @@ -2,9 +2,10 @@ package doctor import ( "fmt" - "github.com/ryansimmen/mato/internal/dirs" "path/filepath" "strings" + + "github.com/ryansimmen/mato/internal/dirs" ) // ---------- E. Task Parsing ---------- diff --git a/internal/frontmatter/frontmatter.go b/internal/frontmatter/frontmatter.go index e991850c..d5b8add1 100644 --- a/internal/frontmatter/frontmatter.go +++ b/internal/frontmatter/frontmatter.go @@ -3,6 +3,7 @@ package frontmatter import ( "crypto/sha256" + "errors" "fmt" "io" "os" @@ -129,7 +130,7 @@ func decodeTaskMeta(block string, meta *TaskMeta) error { dec := yaml.NewDecoder(strings.NewReader(block)) dec.KnownFields(true) if err := dec.Decode(meta); err != nil { - if err == io.EOF { + if errors.Is(err, io.EOF) { return nil } return err diff --git a/internal/graph/graph.go b/internal/graph/graph.go index c1499847..631ca404 100644 --- a/internal/graph/graph.go +++ b/internal/graph/graph.go @@ -367,7 +367,7 @@ func ShowTo(w io.Writer, repoRoot, format string, showAll bool) error { // Fail on directory-level read errors only; skip glob warnings. for _, bw := range idx.BuildWarnings() { if !isGlobWarning(bw) { - return fmt.Errorf("incomplete index: %s: %v", bw.State, bw.Err) + return fmt.Errorf("incomplete index: %s: %w", bw.State, bw.Err) } } diff --git a/internal/merge/merge.go b/internal/merge/merge.go index 18575ed6..0a7cc80a 100644 --- a/internal/merge/merge.go +++ b/internal/merge/merge.go @@ -149,7 +149,7 @@ func loadMergeTaskBranch(path string) (string, error) { return "", errTaskBranchMarkerMissing } if err := git.ValidateBranch(taskBranch); err != nil { - return "", fmt.Errorf("%w: %v", errTaskBranchMarkerInvalid, err) + return "", fmt.Errorf("%w: %w", errTaskBranchMarkerInvalid, err) } return taskBranch, nil } diff --git a/internal/merge/squash.go b/internal/merge/squash.go index 23f1ae0d..21e886f1 100644 --- a/internal/merge/squash.go +++ b/internal/merge/squash.go @@ -46,7 +46,7 @@ func mergeReadyTask(repoRoot, branch string, task mergeQueueTask) (*mergeResult, } if _, err := gitOutput(cloneDir, "merge", "--squash", "origin/"+taskBranch); err != nil { - return nil, fmt.Errorf("%w: %s: %v", errSquashMergeConflict, taskBranch, err) + return nil, fmt.Errorf("%w: %s: %w", errSquashMergeConflict, taskBranch, err) } // If the squash produced no staged changes, the task branch is already @@ -63,7 +63,7 @@ func mergeReadyTask(repoRoot, branch string, task mergeQueueTask) (*mergeResult, return nil, fmt.Errorf("commit squash merge: %w", err) } if _, err := gitOutput(cloneDir, "push", "origin", branch); err != nil { - return nil, fmt.Errorf("%w: push %s: %v", errPushAfterSquashFailed, branch, err) + return nil, fmt.Errorf("%w: push %s: %w", errPushAfterSquashFailed, branch, err) } // Capture merge result for completion detail. diff --git a/internal/queue/cancel.go b/internal/queue/cancel.go index 5e8e5126..d71df71b 100644 --- a/internal/queue/cancel.go +++ b/internal/queue/cancel.go @@ -126,7 +126,7 @@ func cancelResolvedTask(tasksDir string, idx *PollIndex, match TaskMatch) (Cance if err := appendCancelledRecordFn(failedPath); err != nil { if rollbackErr := AtomicMove(failedPath, match.Path); rollbackErr != nil { fmt.Fprintf(os.Stderr, "error: cancelled marker write failed and rollback to %s/ also failed: %v\n", match.State, rollbackErr) - return CancelResult{}, fmt.Errorf("write cancelled marker: %w (rollback failed: %v)", err, rollbackErr) + return CancelResult{}, fmt.Errorf("write cancelled marker: %w (rollback failed: %w)", err, rollbackErr) } return CancelResult{}, fmt.Errorf("write cancelled marker to %s: %w (rolled back to %s/)", failedPath, err, match.State) } diff --git a/internal/queue/claim.go b/internal/queue/claim.go index 969bafac..a113be3d 100644 --- a/internal/queue/claim.go +++ b/internal/queue/claim.go @@ -283,7 +283,7 @@ func handleRetryExhaustedTask(name, dst, src, failedDir string) error { // Move back to backlog so the task is not left orphaned // in in-progress/ without a claimed-by marker. if rbErr := retryExhaustedRollback(dst, src); rbErr != nil { - return fmt.Errorf("retry-exhausted rollback failed for %s (task stranded in in-progress): move-to-failed: %v, rollback: %w", name, err, rbErr) + return fmt.Errorf("retry-exhausted rollback failed for %s (task stranded in in-progress): move-to-failed: %w, rollback: %w", name, err, rbErr) } // Rollback succeeded, but the task is now back in backlog/ // while still retry-exhausted. Return a hard error so the @@ -300,7 +300,7 @@ func handleRetryExhaustedTask(name, dst, src, failedDir string) error { // meaning the task is stranded in in-progress/ without ownership metadata. func rollbackClaimToBacklog(name, dst, src string, claimErr error) error { if rbErr := claimRollbackFn(dst, src); rbErr != nil { - return fmt.Errorf("claim rollback failed for %s (task stranded in in-progress): prepend: %v, rollback: %w", name, claimErr, rbErr) + return fmt.Errorf("claim rollback failed for %s (task stranded in in-progress): prepend: %w, rollback: %w", name, claimErr, rbErr) } return nil } @@ -399,7 +399,7 @@ func SelectAndClaimTask(tasksDir, agentID string, candidates []string, cooldown if err != nil { ui.Warnf("warning: could not assign branch for %s: %v\n", name, err) if restoreErr := restoreClaimedTaskContents(dst, originalData); restoreErr != nil { - return nil, fmt.Errorf("assign branch for %s: %w (also failed to restore task contents: %v)", name, err, restoreErr) + return nil, fmt.Errorf("assign branch for %s: %w (also failed to restore task contents: %w)", name, err, restoreErr) } if rbErr := rollbackClaimToBacklog(name, dst, src, err); rbErr != nil { return nil, rbErr diff --git a/internal/queue/manifest_test.go b/internal/queue/manifest_test.go index 2fe6958e..546bad88 100644 --- a/internal/queue/manifest_test.go +++ b/internal/queue/manifest_test.go @@ -1,11 +1,12 @@ package queue import ( - "github.com/ryansimmen/mato/internal/dirs" "os" "path/filepath" "strings" "testing" + + "github.com/ryansimmen/mato/internal/dirs" ) func TestComputeQueueManifest_PriorityOrder(t *testing.T) { diff --git a/internal/queue/queue.go b/internal/queue/queue.go index 3e85a1bc..86a312b8 100644 --- a/internal/queue/queue.go +++ b/internal/queue/queue.go @@ -490,7 +490,7 @@ func finalizeAtomicMove(src, dst, mode string) error { } cleanupErr := removeFn(dst) if cleanupErr != nil && !os.IsNotExist(cleanupErr) { - return fmt.Errorf("atomic move %s → %s: remove source after %s: %w (also failed to remove destination during rollback: %v)", src, dst, mode, err, cleanupErr) + return fmt.Errorf("atomic move %s → %s: remove source after %s: %w (also failed to remove destination during rollback: %w)", src, dst, mode, err, cleanupErr) } return fmt.Errorf("atomic move %s → %s: remove source after %s: %w", src, dst, mode, err) } diff --git a/internal/queue/resolve_test.go b/internal/queue/resolve_test.go index 759fc95f..ef26ecd6 100644 --- a/internal/queue/resolve_test.go +++ b/internal/queue/resolve_test.go @@ -1,9 +1,10 @@ package queue import ( - "github.com/ryansimmen/mato/internal/dirs" "strings" "testing" + + "github.com/ryansimmen/mato/internal/dirs" ) func TestResolveTask(t *testing.T) { diff --git a/internal/queue/runnable_backlog_test.go b/internal/queue/runnable_backlog_test.go index edcd899a..f1c7ac74 100644 --- a/internal/queue/runnable_backlog_test.go +++ b/internal/queue/runnable_backlog_test.go @@ -1,9 +1,10 @@ package queue import ( - "github.com/ryansimmen/mato/internal/dirs" "strings" "testing" + + "github.com/ryansimmen/mato/internal/dirs" ) func TestFormatDependencyBlocks(t *testing.T) { diff --git a/internal/runner/task.go b/internal/runner/task.go index 17144f77..95b37eee 100644 --- a/internal/runner/task.go +++ b/internal/runner/task.go @@ -351,9 +351,9 @@ func moveTaskToReviewWithMarker(tasksDir string, claimed *queue.ClaimedTask, bra detail := fmt.Sprintf("write branch marker to %s: %v (rollback failed: %v)", readyPath, err, rollbackErr) if quarantineErr := queue.QuarantinePushedTaskHandoff(tasksDir, claimed.Filename, readyPath, detail); quarantineErr != nil { fmt.Fprintf(os.Stderr, "error: branch marker write failed, rollback to in-progress/ also failed, and quarantine to failed/ also failed: %v\n", quarantineErr) - return fmt.Errorf("write branch marker to %s: %w (rollback failed: %v; quarantine to failed/ also failed: %v)", readyPath, err, rollbackErr, quarantineErr) + return fmt.Errorf("write branch marker to %s: %w (rollback failed: %w; quarantine to failed/ also failed: %w)", readyPath, err, rollbackErr, quarantineErr) } - return fmt.Errorf("write branch marker to %s: %w (rollback failed: %v; moved task to failed/)", readyPath, err, rollbackErr) + return fmt.Errorf("write branch marker to %s: %w (rollback failed: %w; moved task to failed/)", readyPath, err, rollbackErr) } return fmt.Errorf("write branch marker to %s: %w (rolled back to in-progress/)", readyPath, err) } From 7720b11a25a9e916c26971e15944b3c1b4c0f6b4 Mon Sep 17 00:00:00 2001 From: Ryan Simmen Date: Mon, 27 Apr 2026 09:21:23 -0400 Subject: [PATCH 2/4] chore: add more lint checks --- .golangci.yml | 3 +++ AGENTS.md | 2 +- internal/merge/taskops_test.go | 1 - internal/queue/cancel.go | 1 - internal/queueview/resolve.go | 1 - 5 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index d9af905a..41f8a5f4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -2,10 +2,13 @@ version: "2" linters: default: none enable: + - bodyclose + - copyloopvar - errcheck - errorlint - govet - ineffassign + - misspell - staticcheck - unused settings: diff --git a/AGENTS.md b/AGENTS.md index 652a16d2..9a8b0835 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,7 +13,7 @@ make verify # run build, vet, lint, deadcode, and un # Lint go vet ./... # built-in static analysis -make lint # golangci-lint (errcheck, errorlint, govet, ineffassign, staticcheck, unused, gofmt, goimports) +make lint # golangci-lint (bodyclose, copyloopvar, errcheck, errorlint, govet, ineffassign, misspell, staticcheck, unused, gofmt, goimports) make deadcode # detect unreachable exported code and unused symbols # Format diff --git a/internal/merge/taskops_test.go b/internal/merge/taskops_test.go index 382f902a..92ddb89f 100644 --- a/internal/merge/taskops_test.go +++ b/internal/merge/taskops_test.go @@ -126,7 +126,6 @@ func TestAppendTaskRecord(t *testing.T) { errCh := make(chan error, len(records)) var wg sync.WaitGroup for _, record := range records { - record := record wg.Add(1) go func() { defer wg.Done() diff --git a/internal/queue/cancel.go b/internal/queue/cancel.go index d71df71b..a66a689d 100644 --- a/internal/queue/cancel.go +++ b/internal/queue/cancel.go @@ -72,7 +72,6 @@ func ListCancellableTasks(tasksDir string) []TaskMatch { if _, ok := stateSet[pf.State]; !ok { continue } - pf := pf matches = append(matches, TaskMatch{ Filename: pf.Filename, State: pf.State, diff --git a/internal/queueview/resolve.go b/internal/queueview/resolve.go index bbc6b4ad..8c1141af 100644 --- a/internal/queueview/resolve.go +++ b/internal/queueview/resolve.go @@ -70,7 +70,6 @@ func CollectTaskMatches(idx *PollIndex, taskRef string, states []string) (string if _, ok := allowed[pf.State]; !ok { continue } - pf := pf match := TaskMatch{ Filename: pf.Filename, State: pf.State, From d9aef8cb18a4872a3e248aff8b7ef34b05166449 Mon Sep 17 00:00:00 2001 From: Ryan Simmen Date: Mon, 27 Apr 2026 09:36:30 -0400 Subject: [PATCH 3/4] chore: expand lint coverage --- .golangci.yml | 4 ++++ AGENTS.md | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 41f8a5f4..cc3b625b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -4,12 +4,16 @@ linters: enable: - bodyclose - copyloopvar + - durationcheck - errcheck - errorlint - govet - ineffassign + - makezero - misspell + - nolintlint - staticcheck + - unconvert - unused settings: errcheck: diff --git a/AGENTS.md b/AGENTS.md index 9a8b0835..e24e468c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,7 +13,7 @@ make verify # run build, vet, lint, deadcode, and un # Lint go vet ./... # built-in static analysis -make lint # golangci-lint (bodyclose, copyloopvar, errcheck, errorlint, govet, ineffassign, misspell, staticcheck, unused, gofmt, goimports) +make lint # golangci-lint (bodyclose, copyloopvar, durationcheck, errcheck, errorlint, govet, ineffassign, makezero, misspell, nolintlint, staticcheck, unconvert, unused, gofmt, goimports) make deadcode # detect unreachable exported code and unused symbols # Format From eb1a4e00d50d1e81be760317fa8cfa1a28b71d58 Mon Sep 17 00:00:00 2001 From: Ryan Simmen Date: Mon, 27 Apr 2026 09:41:53 -0400 Subject: [PATCH 4/4] chore: enable nil error lint checks --- .golangci.yml | 11 +++++++++++ AGENTS.md | 2 +- internal/configresolve/validate.go | 2 +- internal/merge/merge.go | 10 ++-------- internal/runner/runner_poll.go | 8 ++++++-- internal/runner/task.go | 3 +++ 6 files changed, 24 insertions(+), 12 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index cc3b625b..257657d2 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -11,10 +11,12 @@ linters: - ineffassign - makezero - misspell + - nilerr - nolintlint - staticcheck - unconvert - unused + - wastedassign settings: errcheck: check-type-assertions: true @@ -26,6 +28,15 @@ linters: - std-error-handling paths: - vendor + rules: + - linters: + - nilerr + path: internal/integration/bounded_run_test\.go + text: "error is not nil" + - linters: + - nilerr + path: internal/pause/pause\.go + text: "error is not nil" formatters: enable: - gofmt diff --git a/AGENTS.md b/AGENTS.md index e24e468c..3805c52f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,7 +13,7 @@ make verify # run build, vet, lint, deadcode, and un # Lint go vet ./... # built-in static analysis -make lint # golangci-lint (bodyclose, copyloopvar, durationcheck, errcheck, errorlint, govet, ineffassign, makezero, misspell, nolintlint, staticcheck, unconvert, unused, gofmt, goimports) +make lint # golangci-lint (bodyclose, copyloopvar, durationcheck, errcheck, errorlint, govet, ineffassign, makezero, misspell, nilerr, nolintlint, staticcheck, unconvert, unused, wastedassign, gofmt, goimports) make deadcode # detect unreachable exported code and unused symbols # Format diff --git a/internal/configresolve/validate.go b/internal/configresolve/validate.go index 4685ea35..08233ddf 100644 --- a/internal/configresolve/validate.go +++ b/internal/configresolve/validate.go @@ -192,7 +192,7 @@ func validateDurationResolved(resolved Resolved[string], settingName, configPath if err == nil && d > 0 { return nil } - message := "" + var message string if err != nil { if resolved.Source == SourceEnv { message = fmt.Sprintf("invalid %s %q: %v", resolved.EnvVar, resolved.Value, err) diff --git a/internal/merge/merge.go b/internal/merge/merge.go index 0a7cc80a..b177b50c 100644 --- a/internal/merge/merge.go +++ b/internal/merge/merge.go @@ -228,25 +228,19 @@ func executeMergeRound(ctx context.Context, repoRoot, tasksDir, branch string, t if !detailWritten { continue } - bookkeepingComplete := false if err := moveTaskWithRetry(ctx, task.path, completedPath); err != nil { if _, statErr := os.Stat(completedPath); statErr == nil { if removeErr := removeTaskFileFn(task.path); removeErr != nil { ui.Warnf("warning: could not remove duplicate ready-to-merge task %s: %v\n", task.name, removeErr) continue } - bookkeepingComplete = true } else { ui.Warnf("warning: merged task %s but could not move to completed: %v\n", task.name, err) continue } - } else { - bookkeepingComplete = true - } - if bookkeepingComplete { - runtimedata.DeleteRuntimeArtifacts(tasksDir, task.name) - cleanupTaskBranch(repoRoot, taskBranchName(task)) } + runtimedata.DeleteRuntimeArtifacts(tasksDir, task.name) + cleanupTaskBranch(repoRoot, taskBranchName(task)) merged++ } diff --git a/internal/runner/runner_poll.go b/internal/runner/runner_poll.go index acb03c62..540a1135 100644 --- a/internal/runner/runner_poll.go +++ b/internal/runner/runner_poll.go @@ -467,8 +467,10 @@ func pollLoop(ctx context.Context, env envConfig, run runContext, repoRoot, task boundedErrorCount := 0 priorPaused := false for { - if ctx.Err() != nil { + select { + case <-ctx.Done(): return nil + default: } result := pollIterate(ctx, env, run, repoRoot, tasksDir, branch, agentID, cooldown, &heartbeat, failedDirExcluded, priorPaused) @@ -478,8 +480,10 @@ func pollLoop(ctx context.Context, env envConfig, run runContext, repoRoot, task // immediately. This is unconditional — regardless of whether a // task was claimed — to avoid starting new work with a cancelled // context. - if ctx.Err() != nil { + select { + case <-ctx.Done(): return nil + default: } cycleHadError := result.pollHadError diff --git a/internal/runner/task.go b/internal/runner/task.go index 95b37eee..95d77a1b 100644 --- a/internal/runner/task.go +++ b/internal/runner/task.go @@ -230,6 +230,9 @@ func runOnce(ctx context.Context, env envConfig, run runContext, claimed *queue. func postAgentPush(env envConfig, agentID string, claimed *queue.ClaimedTask, cloneDir, startingTip string) error { // Task must still be in in-progress/ (agent no longer moves files). if _, err := os.Stat(claimed.TaskPath); err != nil { + if !os.IsNotExist(err) { + return fmt.Errorf("stat claimed task %s: %w", claimed.TaskPath, err) + } return nil }