Skip to content

fix(lint): resolve static analysis baseline#769

Open
PierrunoYT wants to merge 1 commit into
Gitlawb:mainfrom
PierrunoYT:fix/issue-743-golangci-lint-baseline
Open

fix(lint): resolve static analysis baseline#769
PierrunoYT wants to merge 1 commit into
Gitlawb:mainfrom
PierrunoYT:fix/issue-743-golangci-lint-baseline

Conversation

@PierrunoYT

@PierrunoYT PierrunoYT commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • resolve the existing unused, ineffassign, and staticcheck baseline
  • remove confirmed-dead helpers and simplify equivalent control flow
  • preserve established error text with targeted lint suppressions

Validation

  • 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 build
  • go run ./cmd/zero-release smoke
  • go run golang.org/x/vuln/cmd/govulncheck@v1.3.0 ./...

Closes #743

Summary by CodeRabbit

  • Bug Fixes

    • Strengthened provider connectivity checks to consistently block unsafe private or loopback destinations unless explicitly permitted.
    • Refined tool-result card collapsing behavior for clearer transcript presentation.
    • Improved handling when disabling built-in MCP servers.
    • Preserved more accurate signal information when commands terminate unexpectedly.
  • Refactor

    • Simplified internal formatting, navigation, sandbox, update, and command-processing logic without changing expected workflows.

Copilot AI review requested due to automatic review settings July 20, 2026 10:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 staticcheck quick-fix findings while preserving behavior.
  • Removes unused helpers and assignments flagged by unused/ineffassign.
  • Preserves established user-facing error text by adding targeted //nolint:staticcheck suppressions 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.

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6901440e-fdc5-481f-8909-e3596a176797

📥 Commits

Reviewing files that changed from the base of the PR and between da9fb50 and c7bdd34.

📒 Files selected for processing (38)
  • internal/agent/profile_controller.go
  • internal/agenteval/score.go
  • internal/agentinit/agentinit.go
  • internal/cli/agent_eval.go
  • internal/cli/mcp_config.go
  • internal/cli/provider_detect.go
  • internal/cli/usage.go
  • internal/config/mcp_merge.go
  • internal/dictation/transcriber_deepgram.go
  • internal/imageinput/pdf_test.go
  • internal/lsp/navigate_test.go
  • internal/providerhealth/providerhealth.go
  • internal/provideronboarding/localruntime_test.go
  • internal/repomap/prompt.go
  • internal/sandbox/adapters.go
  • internal/sandbox/command_prefix.go
  • internal/sandbox/engine.go
  • internal/sandbox/landlock_other.go
  • internal/sandbox/runner.go
  • internal/sandbox/windows_network.go
  • internal/specialist/exec.go
  • internal/tui/command_center.go
  • internal/tui/files_git_sweep_test.go
  • internal/tui/flush.go
  • internal/tui/image_attach_test.go
  • internal/tui/loop.go
  • internal/tui/model.go
  • internal/tui/model_test.go
  • internal/tui/onboarding.go
  • internal/tui/provider_wizard_test.go
  • internal/tui/rendering.go
  • internal/tui/specialist_card.go
  • internal/tui/streaming_fade.go
  • internal/tui/transcript_selection.go
  • internal/update/apply.go
  • internal/update/apply_test.go
  • internal/update/update.go
  • internal/usercommands/usercommands.go
💤 Files with no reviewable changes (5)
  • internal/tui/transcript_selection.go
  • internal/tui/model.go
  • internal/cli/mcp_config.go
  • internal/sandbox/runner.go
  • internal/tui/streaming_fade.go

Walkthrough

This 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.

Changes

Lint baseline cleanup

Layer / File(s) Summary
Formatting and control-flow cleanup
internal/agent/*, internal/agenteval/*, internal/cli/*, internal/lsp/*, internal/imageinput/*, internal/repomap/*, internal/specialist/*, internal/usercommands/*
Staticcheck-driven changes simplify boolean expressions, switches, slice appends, builder writes, context usage, validation assertions, and signal descriptions.
Configuration, provider, and sandbox paths
internal/config/*, internal/providerhealth/*, internal/sandbox/*
MCP merge checks and provider blocking conditions are rewritten, exported error strings receive lint suppressions, and three unused sandbox helpers are removed.
TUI interaction and transcript paths
internal/tui/*
TUI navigation, rendering, transcript geometry, scrollback settlement, test state handling, and formatting code are refactored to address lint findings.
Platform update branching
internal/update/*
Update platform selection and its test use switch runtime.GOOS; unsupported-platform error handling receives a lint suppression.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • Gitlawb/zero#706: Removes the same unused sandbox helper functions in internal/sandbox/runner.go.
  • Gitlawb/zero#167: Also changes SSRF blocking conditions in internal/providerhealth/providerhealth.go.
  • Gitlawb/zero#158: Overlaps with the TUI flush and rendering paths updated here.

Suggested reviewers: copilot, vasanthdev2004

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes unrelated edits in files like profile_controller.go, providerhealth.go, and tui/rendering.go that are not in #743. Remove or split the unrelated edits so this PR only contains fixes for the #743 lint findings.
Docstring Coverage ⚠️ Warning Docstring coverage is 36.84% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately describes the lint-baseline fix.
Linked Issues check ✅ Passed The changes address the listed unused, ineffassign, and staticcheck findings in #743.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 < 1 IDs now return distinct validation errors instead of UnknownExecSessionError. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Resolve existing golangci-lint baseline (35 findings)

3 participants