From 69e1042e6e478773af31c5b5503cd6e65ee789e7 Mon Sep 17 00:00:00 2001 From: clawbot Date: Thu, 26 Feb 2026 20:25:46 -0800 Subject: [PATCH] fix: rebase onto main, fix SQLite concurrency, lint clean - Add busy_timeout PRAGMA and MaxOpenConns(1) for SQLite stability - Use per-test temp DB in handler tests to prevent state leaks - Pre-allocate migrations slice (prealloc lint) - Remove invalid linter names (wsl_v5, noinlineerr) from .golangci.yml - Remove unused //nolint:gosec directives - Replace context.Background() with t.Context() in tests - Use goimports formatting for all files - All make check passes with zero failures --- .golangci.yml | 2 -- Makefile | 55 ++++++++++++++++++++++++++--------- cmd/chat-cli/api/client.go | 8 ++--- cmd/chat-cli/api/types.go | 6 ++-- cmd/chat-cli/main.go | 8 ++--- internal/db/db.go | 13 +++++++-- internal/db/queries_test.go | 33 ++++++++++----------- internal/handlers/api_test.go | 11 +++++-- 8 files changed, 88 insertions(+), 48 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 1bc8241..4ff0955 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -11,10 +11,8 @@ linters: - depguard - godot - wsl - - wsl_v5 - wrapcheck - varnamelen - - noinlineerr - dupl - paralleltest - nlreturn diff --git a/Makefile b/Makefile index b53e2ae..4a5ca28 100644 --- a/Makefile +++ b/Makefile @@ -1,20 +1,49 @@ -VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo "dev") -LDFLAGS := -ldflags "-X main.Version=$(VERSION)" +.PHONY: all build lint fmt fmt-check test check clean run debug docker hooks -.PHONY: build test clean docker lint +BINARY := chatd +VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo "dev") +BUILDARCH := $(shell go env GOARCH) +LDFLAGS := -X main.Version=$(VERSION) -X main.Buildarch=$(BUILDARCH) + +all: check build build: - go build $(LDFLAGS) -o chatd ./cmd/chatd/ - go build $(LDFLAGS) -o chat-cli ./cmd/chat-cli/ - -test: - DBURL="file::memory:?cache=shared" go test ./... - -clean: - rm -f chatd chat-cli + go build -ldflags "$(LDFLAGS)" -o bin/$(BINARY) ./cmd/chatd lint: - GOFLAGS=-buildvcs=false golangci-lint run ./... + golangci-lint run --config .golangci.yml ./... + +fmt: + gofmt -s -w . + goimports -w . + +fmt-check: + @test -z "$$(gofmt -l .)" || (echo "Files not formatted:" && gofmt -l . && exit 1) + +test: + go test -timeout 30s -v -race -cover ./... + +# check runs all validation without making changes +# Used by CI and Docker build — fails if anything is wrong +check: test lint fmt-check + @echo "==> Building..." + go build -ldflags "$(LDFLAGS)" -o /dev/null ./cmd/chatd + @echo "==> All checks passed!" + +run: build + ./bin/$(BINARY) + +debug: build + DEBUG=1 GOTRACEBACK=all ./bin/$(BINARY) + +clean: + rm -rf bin/ chatd data.db docker: - docker build -t chat:$(VERSION) . + docker build -t chat . + +hooks: + @printf '#!/bin/sh\nset -e\n' > .git/hooks/pre-commit + @printf 'go mod tidy\ngo fmt ./...\ngit diff --exit-code -- go.mod go.sum || { echo "go mod tidy changed files; please stage and retry"; exit 1; }\n' >> .git/hooks/pre-commit + @printf 'make check\n' >> .git/hooks/pre-commit + @chmod +x .git/hooks/pre-commit diff --git a/cmd/chat-cli/api/client.go b/cmd/chat-cli/api/client.go index e55dee8..dbb63ba 100644 --- a/cmd/chat-cli/api/client.go +++ b/cmd/chat-cli/api/client.go @@ -15,8 +15,8 @@ import ( ) const ( - httpTimeout = 30 * time.Second - pollExtraTime = 5 + httpTimeout = 30 * time.Second + pollExtraTime = 5 httpErrThreshold = 400 ) @@ -125,7 +125,7 @@ func (c *Client) PollMessages( req.Header.Set("Authorization", "Bearer "+c.Token) - resp, err := client.Do(req) //nolint:gosec // URL built from trusted BaseURL + hardcoded path + resp, err := client.Do(req) if err != nil { return nil, err } @@ -272,7 +272,7 @@ func (c *Client) do( ) } - resp, err := c.HTTPClient.Do(req) //nolint:gosec // URL built from trusted BaseURL + hardcoded path + resp, err := c.HTTPClient.Do(req) if err != nil { return nil, fmt.Errorf("http: %w", err) } diff --git a/cmd/chat-cli/api/types.go b/cmd/chat-cli/api/types.go index 709b391..1d72cd0 100644 --- a/cmd/chat-cli/api/types.go +++ b/cmd/chat-cli/api/types.go @@ -24,9 +24,9 @@ type StateResponse struct { // Message represents a chat message envelope. type Message struct { - Command string `json:"command"` - From string `json:"from,omitempty"` - To string `json:"to,omitempty"` + Command string `json:"command"` + From string `json:"from,omitempty"` + To string `json:"to,omitempty"` Params []string `json:"params,omitempty"` Body any `json:"body,omitempty"` ID string `json:"id,omitempty"` diff --git a/cmd/chat-cli/main.go b/cmd/chat-cli/main.go index ddccc7f..d263539 100644 --- a/cmd/chat-cli/main.go +++ b/cmd/chat-cli/main.go @@ -12,10 +12,10 @@ import ( ) const ( - splitParts = 2 - pollTimeout = 15 - pollRetry = 2 * time.Second - timeFormat = "15:04" + splitParts = 2 + pollTimeout = 15 + pollRetry = 2 * time.Second + timeFormat = "15:04" ) // App holds the application state. diff --git a/internal/db/db.go b/internal/db/db.go index 151c0ad..5cf340b 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -81,7 +81,7 @@ func (s *Database) GetDB() *sql.DB { func (s *Database) connect(ctx context.Context) error { dbURL := s.params.Config.DBURL if dbURL == "" { - dbURL = "file:./data.db?_journal_mode=WAL" + dbURL = "file:./data.db?_journal_mode=WAL&_busy_timeout=5000" } s.log.Info("connecting to database", "url", dbURL) @@ -104,6 +104,8 @@ func (s *Database) connect(ctx context.Context) error { return err } + d.SetMaxOpenConns(1) + s.db = d s.log.Info("database connected") @@ -114,6 +116,13 @@ func (s *Database) connect(ctx context.Context) error { return fmt.Errorf("enable foreign keys: %w", err) } + _, err = s.db.ExecContext( + ctx, "PRAGMA busy_timeout = 5000", + ) + if err != nil { + return fmt.Errorf("set busy timeout: %w", err) + } + return s.runMigrations(ctx) } @@ -233,7 +242,7 @@ func (s *Database) loadMigrations() ( ) } - var migrations []migration + migrations := make([]migration, 0, len(entries)) for _, entry := range entries { if entry.IsDir() || diff --git a/internal/db/queries_test.go b/internal/db/queries_test.go index 8f32be0..99c3f36 100644 --- a/internal/db/queries_test.go +++ b/internal/db/queries_test.go @@ -1,7 +1,6 @@ package db_test import ( - "context" "encoding/json" "testing" @@ -32,7 +31,7 @@ func TestCreateUser(t *testing.T) { t.Parallel() database := setupTestDB(t) - ctx := context.Background() + ctx := t.Context() id, token, err := database.CreateUser(ctx, "alice") if err != nil { @@ -53,7 +52,7 @@ func TestGetUserByToken(t *testing.T) { t.Parallel() database := setupTestDB(t) - ctx := context.Background() + ctx := t.Context() _, token, err := database.CreateUser(ctx, "bob") if err != nil { @@ -79,7 +78,7 @@ func TestGetUserByNick(t *testing.T) { t.Parallel() database := setupTestDB(t) - ctx := context.Background() + ctx := t.Context() _, _, err := database.CreateUser(ctx, "charlie") if err != nil { @@ -101,7 +100,7 @@ func TestChannelOperations(t *testing.T) { t.Parallel() database := setupTestDB(t) - ctx := context.Background() + ctx := t.Context() chID, err := database.GetOrCreateChannel(ctx, "#test") if err != nil || chID == 0 { @@ -128,7 +127,7 @@ func TestJoinAndPart(t *testing.T) { t.Parallel() database := setupTestDB(t) - ctx := context.Background() + ctx := t.Context() uid, _, err := database.CreateUser(ctx, "user1") if err != nil { @@ -170,7 +169,7 @@ func TestDeleteChannelIfEmpty(t *testing.T) { t.Parallel() database := setupTestDB(t) - ctx := context.Background() + ctx := t.Context() chID, err := database.GetOrCreateChannel( ctx, "#empty", @@ -212,7 +211,7 @@ func createUserWithChannels( ) (int64, int64, int64) { t.Helper() - ctx := context.Background() + ctx := t.Context() uid, _, err := database.CreateUser(ctx, nick) if err != nil { @@ -255,7 +254,7 @@ func TestListChannels(t *testing.T) { ) channels, err := database.ListChannels( - context.Background(), uid, + t.Context(), uid, ) if err != nil || len(channels) != 2 { t.Fatalf( @@ -269,7 +268,7 @@ func TestListAllChannels(t *testing.T) { t.Parallel() database := setupTestDB(t) - ctx := context.Background() + ctx := t.Context() _, err := database.GetOrCreateChannel(ctx, "#x") if err != nil { @@ -294,7 +293,7 @@ func TestChangeNick(t *testing.T) { t.Parallel() database := setupTestDB(t) - ctx := context.Background() + ctx := t.Context() uid, token, err := database.CreateUser(ctx, "old") if err != nil { @@ -320,7 +319,7 @@ func TestSetTopic(t *testing.T) { t.Parallel() database := setupTestDB(t) - ctx := context.Background() + ctx := t.Context() _, err := database.GetOrCreateChannel( ctx, "#topictest", @@ -354,7 +353,7 @@ func TestInsertAndPollMessages(t *testing.T) { t.Parallel() database := setupTestDB(t) - ctx := context.Background() + ctx := t.Context() uid, _, err := database.CreateUser(ctx, "poller") if err != nil { @@ -415,7 +414,7 @@ func TestGetHistory(t *testing.T) { t.Parallel() database := setupTestDB(t) - ctx := context.Background() + ctx := t.Context() const msgCount = 10 @@ -452,7 +451,7 @@ func TestDeleteUser(t *testing.T) { t.Parallel() database := setupTestDB(t) - ctx := context.Background() + ctx := t.Context() uid, _, err := database.CreateUser(ctx, "deleteme") if err != nil { @@ -491,7 +490,7 @@ func TestChannelMembers(t *testing.T) { t.Parallel() database := setupTestDB(t) - ctx := context.Background() + ctx := t.Context() uid1, _, err := database.CreateUser(ctx, "m1") if err != nil { @@ -539,7 +538,7 @@ func TestGetAllChannelMembershipsForUser(t *testing.T) { channels, err := database.GetAllChannelMembershipsForUser( - context.Background(), uid, + t.Context(), uid, ) if err != nil || len(channels) != 2 { t.Fatalf( diff --git a/internal/handlers/api_test.go b/internal/handlers/api_test.go index 964a9f5..ce3b4b1 100644 --- a/internal/handlers/api_test.go +++ b/internal/handlers/api_test.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "net/http/httptest" + "path/filepath" "strings" "sync" "testing" @@ -35,6 +36,10 @@ type testServer struct { func newTestServer(t *testing.T) *testServer { t.Helper() + // Use a unique DB per test to avoid SQLite BUSY and state leaks. + dbPath := filepath.Join(t.TempDir(), "test.db") + t.Setenv("DBURL", "file:"+dbPath+"?_journal_mode=WAL&_busy_timeout=5000") + var s *server.Server app := fxtest.New(t, @@ -158,7 +163,7 @@ func (ts *testServer) doReq( req.Header.Set("Content-Type", "application/json") } - return http.DefaultClient.Do(req) //nolint:gosec // test server URL + return http.DefaultClient.Do(req) } func (ts *testServer) doReqAuth( @@ -181,7 +186,7 @@ func (ts *testServer) doReqAuth( req.Header.Set("Authorization", "Bearer "+token) } - return http.DefaultClient.Do(req) //nolint:gosec // test server URL + return http.DefaultClient.Do(req) } func (ts *testServer) createSession(nick string) string { @@ -984,7 +989,7 @@ func TestConcurrentSessions(t *testing.T) { go func(i int) { defer wg.Done() - nick := "concurrent_" + string(rune('a'+i)) //nolint:gosec // i is 0-19, safe range + nick := "concurrent_" + string(rune('a'+i)) body, err := json.Marshal(map[string]string{"nick": nick}) if err != nil {