fix(lint): resolve static analysis baseline#769
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request resolves the repository’s existing unused, ineffassign, and staticcheck baseline by applying safe mechanical refactors, removing confirmed-dead helpers, and adding targeted lint suppressions where error-string text must remain stable for users.
Changes:
- Simplifies boolean/control-flow expressions to address
staticcheckquick-fix findings while preserving behavior. - Removes unused helpers and assignments flagged by
unused/ineffassign. - Preserves established user-facing error text by adding targeted
//nolint:staticchecksuppressions and by using non-nil contexts in tests/commands.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/usercommands/usercommands.go | Simplifies rune validation logic for validCommandName. |
| internal/update/update.go | Adds //nolint:staticcheck to preserve established Termux error text. |
| internal/update/apply.go | Replaces OS if/else with switch to satisfy staticcheck quick-fix guidance. |
| internal/update/apply_test.go | Mirrors apply.go OS switch structure to keep tests aligned and lint-clean. |
| internal/tui/transcript_selection.go | Removes unused layout helper and consolidates viewport-start computation path. |
| internal/tui/streaming_fade.go | Removes an unused helper for age-tick rescheduling. |
| internal/tui/specialist_card.go | Simplifies switch for tool call summaries. |
| internal/tui/rendering.go | Applies De Morgan simplification to collapse-footer gating logic. |
| internal/tui/provider_wizard_test.go | Removes an ineffectual assignment while retaining the type assertion. |
| internal/tui/onboarding.go | Converts stage checks to switch statements for readability and staticcheck fixes. |
| internal/tui/model.go | Removes unused scrollableTranscriptView wrapper. |
| internal/tui/model_test.go | Removes an ineffectual assignment while preserving call/type-check intent. |
| internal/tui/loop.go | Replaces Sprintf+WriteString with Fprintf into a builder. |
| internal/tui/image_attach_test.go | Uses Fprintf into a buffer to avoid intermediate allocations flagged by linters. |
| internal/tui/flush.go | Simplifies negated boolean expression for “settled row” determination. |
| internal/tui/files_git_sweep_test.go | Passes non-nil contexts (context.TODO()) to satisfy staticcheck. |
| internal/tui/command_center.go | Simplifies credential-check boolean expression. |
| internal/specialist/exec.go | Adjusts signal description extraction per staticcheck quick-fix guidance. |
| internal/sandbox/windows_network.go | Adds //nolint:staticcheck to keep exported sentinel error text unchanged. |
| internal/sandbox/runner.go | Removes unused sandbox helper functions flagged by unused. |
| internal/sandbox/landlock_other.go | Adds //nolint:staticcheck to keep exported sentinel error text unchanged. |
| internal/sandbox/engine.go | Simplifies backend capability boolean checks. |
| internal/sandbox/command_prefix.go | Simplifies if/return into a single return expression. |
| internal/sandbox/adapters.go | Applies De Morgan simplification to backend marker checks. |
| internal/repomap/prompt.go | Uses Fprintf into a builder to avoid intermediate string formatting. |
| internal/provideronboarding/localruntime_test.go | Simplifies slice nil/len checks using len(nil)==0 behavior. |
| internal/providerhealth/providerhealth.go | Simplifies endpoint safety boolean checks while preserving semantics. |
| internal/lsp/navigate_test.go | Passes non-nil contexts (context.TODO()) to satisfy staticcheck. |
| internal/imageinput/pdf_test.go | Uses Fprintf into buffers to avoid intermediate allocations flagged by linters. |
| internal/dictation/transcriber_deepgram.go | Adds //nolint:staticcheck to preserve established Deepgram error text. |
| internal/config/mcp_merge.go | Simplifies merge logic boolean condition without semantic change. |
| internal/cli/usage.go | Uses Fprintf into a builder for report formatting. |
| internal/cli/provider_detect.go | Simplifies argument parsing switch to match staticcheck recommendations. |
| internal/cli/mcp_config.go | Removes ineffectual assignments flagged by ineffassign. |
| internal/cli/agent_eval.go | Uses variadic append to simplify list accumulation. |
| internal/agentinit/agentinit.go | Uses Fprintf into a builder to avoid intermediate strings. |
| internal/agenteval/score.go | Uses variadic append to simplify list accumulation. |
| internal/agent/profile_controller.go | Simplifies boolean condition for escalation gating. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (38)
💤 Files with no reviewable changes (5)
WalkthroughThis PR resolves existing lint findings across agent, CLI, configuration, provider health, sandbox, TUI, and update code. It also replaces intermediate string formatting, removes unused helpers, simplifies equivalent control flow, and updates affected tests. ChangesLint baseline cleanup
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
- [P1] Preserve the id-invariant invalid-session recovery path
internal/tools/exec_command.go:778
Missing, nil, non-integer, and< 1IDs now return distinct validation errors instead ofUnknownExecSessionError. The agent’s repeated-failure guard keys its streak on the first 80 characters of the error, so a model can alternate malformed IDs (or transition from one to a guessed unknown ID) and reset the streak rather than receiving the recovery hint and six-failure halt. This reintroduces the probing/thrashing behavior fixed by the immediate base; keep every no-live-session input on the shared recovery error and restore coverage for the invalid forms.
Summary
unused,ineffassign, andstaticcheckbaselineValidation
go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2 run --enable-only unused,ineffassign,staticcheck ./...(0 issues)go vet ./...go test ./...go run ./cmd/zero-release buildgo run ./cmd/zero-release smokego run golang.org/x/vuln/cmd/govulncheck@v1.3.0 ./...Closes #743
Summary by CodeRabbit
Bug Fixes
Refactor