From 27ac3f5bac9c88b8af7a612080e1d5a82235e057 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 21 Dec 2025 17:33:35 +0000 Subject: [PATCH 1/7] Initial plan From 3b8da9dcaf9ec13433422c98763466496c791141 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 21 Dec 2025 17:49:39 +0000 Subject: [PATCH 2/7] Add golangci-lint configuration and integrate with build Co-authored-by: wham <448809+wham@users.noreply.github.com> --- .golangci.yml | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++ main.md | 37 ++++++++++++++++++++++++++++++++++++ scripts/run | 10 +++++++++- 3 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..521d3a2 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,52 @@ +# golangci-lint configuration +# Documentation: https://golangci-lint.run/usage/configuration/ +version: 2 + +linters: + enable: + # Default/standard linters + - errcheck # Checks for unchecked errors + - govet # Reports suspicious constructs + - ineffassign # Detects ineffectual assignments + - staticcheck # Comprehensive static analysis + - unused # Checks for unused code + + # Additional common linters + - revive # Fast, configurable linter (golint replacement) + - misspell # Finds commonly misspelled English words + - gosec # Inspects for security problems + - bodyclose # Checks HTTP response body is closed + - goconst # Finds repeated strings that could be constants + - gocyclo # Checks cyclomatic complexity + +formatters: + enable: + - gofmt # Format code properly + - goimports # Manage import statements + +linters-settings: + errcheck: + check-type-assertions: false + check-blank: false + + govet: + enable-all: true + + gocyclo: + min-complexity: 15 # Standard threshold + + gosec: + excludes: + - G104 # Audit errors not checked - covered by errcheck + +issues: + exclude-rules: + # Exclude some linters from running on test files + - path: _test\.go + linters: + - gocyclo + - errcheck + - gosec + + max-issues-per-linter: 0 + max-same-issues: 0 diff --git a/main.md b/main.md index 89eb995..582c556 100644 --- a/main.md +++ b/main.md @@ -1163,6 +1163,43 @@ Download the appropriate archive for your platform from [releases](https://githu curl -L https://github.com/wham/github-brain/releases/download/v1.2.3/github-brain-darwin-arm64.tar.gz | tar xz ``` +## Code Quality + +### Linting + +Use **golangci-lint** for code quality checks. Configuration in `.golangci.yml`. + +**Linters enabled:** +- **errcheck** - Checks for unchecked errors +- **govet** - Reports suspicious constructs +- **ineffassign** - Detects ineffectual assignments +- **staticcheck** - Comprehensive static analysis +- **unused** - Checks for unused code +- **revive** - Fast, configurable linter (golint replacement) +- **misspell** - Finds commonly misspelled English words +- **gosec** - Inspects for security problems +- **bodyclose** - Checks HTTP response body is closed +- **goconst** - Finds repeated strings that could be constants +- **gocyclo** - Checks cyclomatic complexity (threshold: 15) + +**Formatters enabled:** +- **gofmt** - Formats code properly +- **goimports** - Manages import statements + +**Running the linter:** +```bash +# Standalone +golangci-lint run --timeout=5m + +# Integrated with build (via scripts/run) +./scripts/run [command] +``` + +**CI Integration:** +- Linting runs automatically on all PRs via `.github/workflows/build.yml` +- Build fails if linter finds issues +- Part of the standard build task in `scripts/run` + ### Release Model Coded in `.github/workflow/release.yml` and `.github/workflow/build.yml`. diff --git a/scripts/run b/scripts/run index 203182f..ed3786b 100755 --- a/scripts/run +++ b/scripts/run @@ -2,8 +2,16 @@ set -e cd "$(dirname "$0")/.." +# Lint the code (non-blocking in development) +echo "Running linter..." +if command -v golangci-lint &> /dev/null; then + golangci-lint run --timeout=5m || echo "Warning: Linter found issues (non-blocking in development)" +else + echo "Warning: golangci-lint not found, skipping linting" +fi + # Build with FTS5 support enabled -CGO_CFLAGS="-DSQLITE_ENABLE_FTS5" go build -gcflags="all=-N -l" -o ./build/github-brain . +CGO_ENABLED=1 CGO_CFLAGS="-DSQLITE_ENABLE_FTS5" CGO_LDFLAGS="-lm" go build -gcflags="all=-N -l" -o ./build/github-brain . # Set home directory to checkout directory CHECKOUT_DIR="$(pwd)" From 0fbd0525d8a251e0005c40ecf8d07f5090352227 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 21 Dec 2025 17:54:50 +0000 Subject: [PATCH 3/7] Fix golangci-lint config - remove invalid formatters section Co-authored-by: wham <448809+wham@users.noreply.github.com> --- .golangci.yml | 5 ----- main.md | 4 ---- 2 files changed, 9 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 521d3a2..c69eb13 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -19,11 +19,6 @@ linters: - goconst # Finds repeated strings that could be constants - gocyclo # Checks cyclomatic complexity -formatters: - enable: - - gofmt # Format code properly - - goimports # Manage import statements - linters-settings: errcheck: check-type-assertions: false diff --git a/main.md b/main.md index 582c556..10fc022 100644 --- a/main.md +++ b/main.md @@ -1182,10 +1182,6 @@ Use **golangci-lint** for code quality checks. Configuration in `.golangci.yml`. - **goconst** - Finds repeated strings that could be constants - **gocyclo** - Checks cyclomatic complexity (threshold: 15) -**Formatters enabled:** -- **gofmt** - Formats code properly -- **goimports** - Manages import statements - **Running the linter:** ```bash # Standalone From 3e6100ce4550f7693a92f94e781b514c6a3cde5f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 21 Dec 2025 17:56:17 +0000 Subject: [PATCH 4/7] Clarify linting behavior in CI vs local development Co-authored-by: wham <448809+wham@users.noreply.github.com> --- main.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main.md b/main.md index 10fc022..8ef8da6 100644 --- a/main.md +++ b/main.md @@ -1193,8 +1193,8 @@ golangci-lint run --timeout=5m **CI Integration:** - Linting runs automatically on all PRs via `.github/workflows/build.yml` -- Build fails if linter finds issues -- Part of the standard build task in `scripts/run` +- Build fails if linter finds issues (blocking) +- In local development (`scripts/run`), linting runs but is non-blocking to allow rapid iteration ### Release Model From 7f99edd58abfa47c4a26e73740b1d986a3dee017 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 23 Dec 2025 07:39:55 +0000 Subject: [PATCH 5/7] Remove .golangci.yml config file and linting integration, keep CGO flags fix Co-authored-by: wham <448809+wham@users.noreply.github.com> --- .golangci.yml | 47 ----------------------------------------------- main.md | 33 --------------------------------- scripts/run | 8 -------- 3 files changed, 88 deletions(-) delete mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml deleted file mode 100644 index c69eb13..0000000 --- a/.golangci.yml +++ /dev/null @@ -1,47 +0,0 @@ -# golangci-lint configuration -# Documentation: https://golangci-lint.run/usage/configuration/ -version: 2 - -linters: - enable: - # Default/standard linters - - errcheck # Checks for unchecked errors - - govet # Reports suspicious constructs - - ineffassign # Detects ineffectual assignments - - staticcheck # Comprehensive static analysis - - unused # Checks for unused code - - # Additional common linters - - revive # Fast, configurable linter (golint replacement) - - misspell # Finds commonly misspelled English words - - gosec # Inspects for security problems - - bodyclose # Checks HTTP response body is closed - - goconst # Finds repeated strings that could be constants - - gocyclo # Checks cyclomatic complexity - -linters-settings: - errcheck: - check-type-assertions: false - check-blank: false - - govet: - enable-all: true - - gocyclo: - min-complexity: 15 # Standard threshold - - gosec: - excludes: - - G104 # Audit errors not checked - covered by errcheck - -issues: - exclude-rules: - # Exclude some linters from running on test files - - path: _test\.go - linters: - - gocyclo - - errcheck - - gosec - - max-issues-per-linter: 0 - max-same-issues: 0 diff --git a/main.md b/main.md index 8ef8da6..89eb995 100644 --- a/main.md +++ b/main.md @@ -1163,39 +1163,6 @@ Download the appropriate archive for your platform from [releases](https://githu curl -L https://github.com/wham/github-brain/releases/download/v1.2.3/github-brain-darwin-arm64.tar.gz | tar xz ``` -## Code Quality - -### Linting - -Use **golangci-lint** for code quality checks. Configuration in `.golangci.yml`. - -**Linters enabled:** -- **errcheck** - Checks for unchecked errors -- **govet** - Reports suspicious constructs -- **ineffassign** - Detects ineffectual assignments -- **staticcheck** - Comprehensive static analysis -- **unused** - Checks for unused code -- **revive** - Fast, configurable linter (golint replacement) -- **misspell** - Finds commonly misspelled English words -- **gosec** - Inspects for security problems -- **bodyclose** - Checks HTTP response body is closed -- **goconst** - Finds repeated strings that could be constants -- **gocyclo** - Checks cyclomatic complexity (threshold: 15) - -**Running the linter:** -```bash -# Standalone -golangci-lint run --timeout=5m - -# Integrated with build (via scripts/run) -./scripts/run [command] -``` - -**CI Integration:** -- Linting runs automatically on all PRs via `.github/workflows/build.yml` -- Build fails if linter finds issues (blocking) -- In local development (`scripts/run`), linting runs but is non-blocking to allow rapid iteration - ### Release Model Coded in `.github/workflow/release.yml` and `.github/workflow/build.yml`. diff --git a/scripts/run b/scripts/run index ed3786b..5e11a9c 100755 --- a/scripts/run +++ b/scripts/run @@ -2,14 +2,6 @@ set -e cd "$(dirname "$0")/.." -# Lint the code (non-blocking in development) -echo "Running linter..." -if command -v golangci-lint &> /dev/null; then - golangci-lint run --timeout=5m || echo "Warning: Linter found issues (non-blocking in development)" -else - echo "Warning: golangci-lint not found, skipping linting" -fi - # Build with FTS5 support enabled CGO_ENABLED=1 CGO_CFLAGS="-DSQLITE_ENABLE_FTS5" CGO_LDFLAGS="-lm" go build -gcflags="all=-N -l" -o ./build/github-brain . From 58a192785382a4e52a716bf17e177b4c24618fe4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 23 Dec 2025 20:33:03 +0000 Subject: [PATCH 6/7] Add back linting integration to scripts/run and lightweight spec to main.md Co-authored-by: wham <448809+wham@users.noreply.github.com> --- main.md | 20 ++++++++++++++++++++ scripts/run | 8 ++++++++ 2 files changed, 28 insertions(+) diff --git a/main.md b/main.md index 89eb995..4d2d138 100644 --- a/main.md +++ b/main.md @@ -1163,6 +1163,26 @@ Download the appropriate archive for your platform from [releases](https://githu curl -L https://github.com/wham/github-brain/releases/download/v1.2.3/github-brain-darwin-arm64.tar.gz | tar xz ``` +## Code Quality + +### Linting + +Use **golangci-lint** with default configuration for code quality checks. + +**Running the linter:** +```bash +# Standalone +golangci-lint run --timeout=5m + +# Integrated with build (via scripts/run) +./scripts/run [command] +``` + +**CI Integration:** +- Linting runs automatically on all PRs via `.github/workflows/build.yml` +- Build fails if linter finds issues (blocking) +- In local development (`scripts/run`), linting runs but is non-blocking to allow rapid iteration + ### Release Model Coded in `.github/workflow/release.yml` and `.github/workflow/build.yml`. diff --git a/scripts/run b/scripts/run index 5e11a9c..ed3786b 100755 --- a/scripts/run +++ b/scripts/run @@ -2,6 +2,14 @@ set -e cd "$(dirname "$0")/.." +# Lint the code (non-blocking in development) +echo "Running linter..." +if command -v golangci-lint &> /dev/null; then + golangci-lint run --timeout=5m || echo "Warning: Linter found issues (non-blocking in development)" +else + echo "Warning: golangci-lint not found, skipping linting" +fi + # Build with FTS5 support enabled CGO_ENABLED=1 CGO_CFLAGS="-DSQLITE_ENABLE_FTS5" CGO_LDFLAGS="-lm" go build -gcflags="all=-N -l" -o ./build/github-brain . From f1f549f53ece9809c35545b1b07b196d9fd50726 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 23 Dec 2025 22:43:06 +0000 Subject: [PATCH 7/7] Fix all golangci-lint issues: check Close() errors and apply staticcheck suggestions Co-authored-by: wham <448809+wham@users.noreply.github.com> --- main.go | 76 +++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 61 insertions(+), 15 deletions(-) diff --git a/main.go b/main.go index e8c9ee3..f49730c 100644 --- a/main.go +++ b/main.go @@ -466,7 +466,7 @@ func visibleLength(s string) int { if runes[i] == '\033' && i+1 < len(runes) && runes[i+1] == '[' { // Skip CSI sequence: ESC [ ... (terminated by a letter) i += 2 - for i < len(runes) && !((runes[i] >= 'A' && runes[i] <= 'Z') || (runes[i] >= 'a' && runes[i] <= 'z')) { + for i < len(runes) && (runes[i] < 'A' || runes[i] > 'Z') && (runes[i] < 'a' || runes[i] > 'z') { i++ } i++ // Skip the terminating letter @@ -933,12 +933,16 @@ func InitDB(dbDir, organization string, progress ProgressInterface) (*DB, error) } // Check if database file exists and if schema version matches - var needsRecreation bool = true + needsRecreation := true if _, err := os.Stat(dbPath); err == nil { // Database exists, check schema version tempDB, err := sql.Open("sqlite3", dbPath) if err == nil { - defer tempDB.Close() + defer func() { + if closeErr := tempDB.Close(); closeErr != nil { + slog.Warn("Failed to close temp database", "error", closeErr) + } + }() schemaMatches, checkErr := checkSchemaVersion(tempDB, progress) if checkErr != nil { if progress != nil { @@ -1061,7 +1065,11 @@ func (db *DB) PopulateSearchTable(currentUsername string, progress ProgressInter if err != nil { slog.Warn("Failed to query user repositories, proceeding with boost=1.0 for all", "error", err) } else { - defer rows.Close() + defer func() { + if closeErr := rows.Close(); closeErr != nil { + slog.Warn("Failed to close rows", "error", closeErr) + } + }() for rows.Next() { var repo string if err := rows.Scan(&repo); err == nil { @@ -1287,7 +1295,11 @@ func (db *DB) GetRepositories() ([]Repository, error) { if err != nil { return nil, fmt.Errorf("failed to query repositories: %w", err) } - defer rows.Close() + defer func() { + if closeErr := rows.Close(); closeErr != nil { + slog.Warn("Failed to close rows", "error", closeErr) + } + }() var repositories []Repository for rows.Next() { @@ -1443,7 +1455,11 @@ func (db *DB) GetDiscussions(repository string, fromDate time.Time, toDate time. if err != nil { return nil, fmt.Errorf("failed to query discussions: %w", err) } - defer rows.Close() + defer func() { + if closeErr := rows.Close(); closeErr != nil { + slog.Warn("Failed to close rows", "error", closeErr) + } + }() var discussions []Discussion for rows.Next() { @@ -1501,7 +1517,11 @@ func (db *DB) GetIssues(repository string, createdFromDate time.Time, createdToD if err != nil { return nil, fmt.Errorf("failed to query issues: %w", err) } - defer rows.Close() + defer func() { + if closeErr := rows.Close(); closeErr != nil { + slog.Warn("Failed to close rows", "error", closeErr) + } + }() var issues []Issue for rows.Next() { @@ -1566,7 +1586,11 @@ func (db *DB) GetPullRequests(repository string, createdFromDate time.Time, crea if err != nil { return nil, fmt.Errorf("failed to query pull requests: %w", err) } - defer rows.Close() + defer func() { + if closeErr := rows.Close(); closeErr != nil { + slog.Warn("Failed to close rows", "error", closeErr) + } + }() var pullRequests []PullRequest for rows.Next() { @@ -3337,7 +3361,11 @@ func (db *DB) GetDiscussionsByRepository(repositoryName string) ([]Discussion, e if err != nil { return nil, fmt.Errorf("failed to query discussions: %w", err) } - defer rows.Close() + defer func() { + if closeErr := rows.Close(); closeErr != nil { + slog.Warn("Failed to close rows", "error", closeErr) + } + }() var discussions []Discussion for rows.Next() { @@ -3506,7 +3534,11 @@ func (se *SearchEngine) searchAllTables(tokens []string, limit int) ([]SearchRes slog.Error("FTS search query failed", "sql", query, "search_table", "search", "fts_query", ftsQuery, "error", err) return nil, fmt.Errorf("FTS search failed: %w", err) } - defer rows.Close() + defer func() { + if closeErr := rows.Close(); closeErr != nil { + slog.Warn("Failed to close rows", "error", closeErr) + } + }() var results []SearchResult for rows.Next() { @@ -3838,7 +3870,7 @@ func ListPullRequestsTool(db *DB) func(context.Context, *mcp.CallToolRequest, Li } } if !found { - return nil, nil, fmt.Errorf("Invalid fields: %s\n\nUse one of the available fields: %s", field, strings.Join(validFields, ", ")) + return nil, nil, fmt.Errorf("invalid fields: %s\n\nUse one of the available fields: %s", field, strings.Join(validFields, ", ")) } } } @@ -4358,7 +4390,11 @@ func main() { logErrorAndReturn(progress, "Error: Failed to initialize database: %v", err) return } - defer db.Close() + defer func() { + if closeErr := db.Close(); closeErr != nil { + slog.Error("Failed to close database", "error", closeErr) + } + }() // Acquire lock to prevent concurrent pull operations if err := db.LockPull(); err != nil { @@ -4539,7 +4575,11 @@ func main() { slog.Error("Failed to initialize database", "error", err) os.Exit(1) } - defer db.Close() + defer func() { + if closeErr := db.Close(); closeErr != nil { + slog.Error("Failed to close database", "error", closeErr) + } + }() if err := RunMCPServer(db); err != nil { slog.Error("MCP server error", "error", err) @@ -5498,7 +5538,11 @@ func requestDeviceCode() (*DeviceCodeResponse, error) { if err != nil { return nil, err } - defer resp.Body.Close() + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil { + slog.Warn("Failed to close response body", "error", closeErr) + } + }() body, err := io.ReadAll(resp.Body) if err != nil { @@ -5547,7 +5591,9 @@ func pollForAccessToken(deviceCode *DeviceCodeResponse) (accessToken string, err } body, err := io.ReadAll(resp.Body) - resp.Body.Close() + if closeErr := resp.Body.Close(); closeErr != nil { + slog.Warn("Failed to close response body", "error", closeErr) + } if err != nil { continue }