From e1bce95bac779d2f7d8bd62365c37c3b059c886f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Thu, 15 Jan 2026 19:05:16 +0000 Subject: [PATCH] feat: golang linter --- .github/workflows/backend-tests.yml | 8 +- .github/workflows/frontend-tests.yml | 43 ++++++++++ .pre-commit-config.yaml | 6 ++ apps/backend/.golangci.yml | 2 + apps/backend/cmd/agentctl/main.go | 5 +- apps/backend/cmd/kandev/main.go | 23 ++++-- apps/backend/internal/agent/agentctl/acp.go | 40 ++++++++-- .../backend/internal/agent/agentctl/client.go | 36 ++++++--- .../agent/agentctl/launcher/launcher.go | 12 ++- .../internal/agent/agentctl/standalone.go | 23 ++++-- .../agent/credentials/manager_test.go | 67 ++++++++-------- apps/backend/internal/agent/docker/client.go | 36 ++++++--- .../internal/agent/lifecycle/manager_test.go | 21 ----- .../internal/agent/settings/store/sqlite.go | 16 +++- .../internal/agent/worktree/manager.go | 18 +++-- apps/backend/internal/agent/worktree/store.go | 6 +- .../agentctl/adapter/codex_adapter.go | 40 ++++++---- apps/backend/internal/agentctl/api/acp.go | 60 +++++++------- .../internal/agentctl/api/control_server.go | 80 ++++++++++--------- .../internal/agentctl/api/workspace.go | 22 ++++- .../internal/agentctl/instance/manager.go | 5 +- .../internal/agentctl/process/manager.go | 13 ++- .../agentctl/process/workspace_tracker.go | 13 ++- apps/backend/internal/events/bus/memory.go | 4 +- .../internal/events/bus/memory_test.go | 45 ++++++++--- .../internal/gateway/websocket/client.go | 45 ++++++++--- .../internal/integration/integration_test.go | 66 +++++++++++---- .../internal/integration/orchestrator_test.go | 30 +++++-- .../orchestrator/scheduler/scheduler_test.go | 10 ++- apps/backend/internal/orchestrator/service.go | 6 +- .../orchestrator/watcher/watcher_test.go | 21 +++-- .../internal/task/repository/memory.go | 9 --- .../internal/task/repository/sqlite.go | 63 +++++---------- .../internal/task/repository/sqlite_test.go | 18 ++++- .../task/service/repository_discovery_test.go | 6 +- .../internal/task/service/service_test.go | 6 +- apps/backend/internal/user/store/sqlite.go | 8 +- apps/backend/pkg/codex/client.go | 5 +- 38 files changed, 601 insertions(+), 336 deletions(-) create mode 100644 .github/workflows/frontend-tests.yml create mode 100644 apps/backend/.golangci.yml diff --git a/.github/workflows/backend-tests.yml b/.github/workflows/backend-tests.yml index fa572eba7b..b7769be268 100644 --- a/.github/workflows/backend-tests.yml +++ b/.github/workflows/backend-tests.yml @@ -33,6 +33,13 @@ jobs: - name: Run go vet run: go vet ./... + - name: Run golangci-lint + uses: golangci/golangci-lint-action@v6 + with: + version: v1.61.0 + args: ./... + working-directory: apps/backend + - name: Run tests with coverage run: | go test -v -race -coverprofile=coverage.out -covermode=atomic -json ./... > test-results.json 2>&1 || true @@ -85,4 +92,3 @@ jobs: - name: Build run: go build -v ./cmd/kandev - diff --git a/.github/workflows/frontend-tests.yml b/.github/workflows/frontend-tests.yml new file mode 100644 index 0000000000..63d00c767d --- /dev/null +++ b/.github/workflows/frontend-tests.yml @@ -0,0 +1,43 @@ +name: Frontend Tests + +on: + push: + branches: [main] + pull_request: + branches: [main] + +jobs: + frontend: + name: Run Frontend Lint, Tests, and Build + runs-on: ubuntu-latest + defaults: + run: + working-directory: apps + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + cache: 'pnpm' + cache-dependency-path: apps/pnpm-lock.yaml + + - name: Set up pnpm + uses: pnpm/action-setup@v4 + with: + version: '9' + + - name: Install dependencies + run: pnpm install + + - name: Run lint + run: pnpm --filter @kandev/web lint + + - name: Run tests + run: pnpm --filter @kandev/web test + + - name: Build + run: pnpm --filter @kandev/web build diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 66ebc77291..842f8484eb 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -7,6 +7,12 @@ repos: language: system pass_filenames: false + - id: go-lint + name: Go Lint (backend) + entry: make lint-backend + language: system + pass_filenames: false + - id: web-tests name: Web tests entry: make test-web diff --git a/apps/backend/.golangci.yml b/apps/backend/.golangci.yml new file mode 100644 index 0000000000..0894db6ef6 --- /dev/null +++ b/apps/backend/.golangci.yml @@ -0,0 +1,2 @@ +version: "2" +# Default golangci-lint configuration (uses built-in defaults). diff --git a/apps/backend/cmd/agentctl/main.go b/apps/backend/cmd/agentctl/main.go index f8a86946d2..6f79df2622 100644 --- a/apps/backend/cmd/agentctl/main.go +++ b/apps/backend/cmd/agentctl/main.go @@ -68,7 +68,9 @@ func main() { fmt.Fprintf(os.Stderr, "failed to initialize logger: %v\n", err) os.Exit(1) } - defer log.Sync() + defer func() { + _ = log.Sync() + }() // Determine mode mode := detectMode(cfg.Mode) @@ -290,4 +292,3 @@ func collectAgentEnv() []string { } return env } - diff --git a/apps/backend/cmd/kandev/main.go b/apps/backend/cmd/kandev/main.go index 68fa80f2e1..b5dcbd2c53 100644 --- a/apps/backend/cmd/kandev/main.go +++ b/apps/backend/cmd/kandev/main.go @@ -69,7 +69,6 @@ var ( flagAgentRuntime = flag.String("agent-runtime", "", "Agent runtime mode: 'docker' or 'standalone'") flagPort = flag.Int("port", 0, "HTTP server port (default: 8080)") flagLogLevel = flag.String("log-level", "", "Log level: debug, info, warn, error") - flagConfigPath = flag.String("config", "", "Path to config file") flagHelp = flag.Bool("help", false, "Show help message") flagVersion = flag.Bool("version", false, "Show version information") ) @@ -127,7 +126,9 @@ func main() { fmt.Fprintf(os.Stderr, "Failed to initialize logger: %v\n", err) os.Exit(1) } - defer log.Sync() + defer func() { + _ = log.Sync() + }() logger.SetDefault(log) log.Info("Starting Kandev (unified mode)...") @@ -158,7 +159,9 @@ func main() { log.Warn("Failed to initialize Docker client - agent features will be disabled", zap.Error(err)) dockerClient = nil } else { - defer dockerClient.Close() + defer func() { + _ = dockerClient.Close() + }() if err := dockerClient.Ping(ctx); err != nil { log.Warn("Docker daemon not available - agent features will be disabled", zap.Error(err)) dockerClient = nil @@ -184,20 +187,26 @@ func main() { if err != nil { log.Fatal("Failed to initialize SQLite database", zap.Error(err), zap.String("db_path", dbPath)) } - defer taskRepo.Close() + defer func() { + _ = taskRepo.Close() + }() log.Info("SQLite database initialized", zap.String("db_path", dbPath)) agentSettingsRepo, err := settingsstore.NewSQLiteRepository(dbPath) if err != nil { log.Fatal("Failed to initialize agent settings store", zap.Error(err), zap.String("db_path", dbPath)) } - defer agentSettingsRepo.Close() + defer func() { + _ = agentSettingsRepo.Close() + }() userRepo, err := userstore.NewSQLiteRepository(dbPath) if err != nil { log.Fatal("Failed to initialize user store", zap.Error(err), zap.String("db_path", dbPath)) } - defer userRepo.Close() + defer func() { + _ = userRepo.Close() + }() discoveryRegistry, err := discovery.LoadRegistry() if err != nil { @@ -1041,7 +1050,7 @@ func (a *messageCreatorAdapter) CreateToolCallMessage(ctx context.Context, taskI } // Add args if provided (contains kind, path, locations, raw_input) - if args != nil && len(args) > 0 { + if len(args) > 0 { metadata["args"] = args // Extract kind as tool_name for icon selection in the frontend diff --git a/apps/backend/internal/agent/agentctl/acp.go b/apps/backend/internal/agent/agentctl/acp.go index ce20759723..cdfd6de47d 100644 --- a/apps/backend/internal/agent/agentctl/acp.go +++ b/apps/backend/internal/agent/agentctl/acp.go @@ -51,7 +51,11 @@ func (c *Client) Initialize(ctx context.Context, clientName, clientVersion strin if err != nil { return nil, err } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + c.logger.Debug("failed to close initialize response body", zap.Error(err)) + } + }() var result InitializeResponse if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { @@ -91,7 +95,11 @@ func (c *Client) NewSession(ctx context.Context, cwd string) (string, error) { if err != nil { return "", err } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + c.logger.Debug("failed to close session response body", zap.Error(err)) + } + }() var result NewSessionResponse if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { @@ -137,7 +145,11 @@ func (c *Client) Prompt(ctx context.Context, text string) (*PromptResponse, erro if err != nil { return nil, err } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + c.logger.Debug("failed to close prompt response body", zap.Error(err)) + } + }() var result PromptResponse if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { @@ -230,7 +242,9 @@ func (c *Client) StreamUpdates(ctx context.Context, handler func(SessionUpdate)) c.mu.Lock() c.acpConn = nil c.mu.Unlock() - conn.Close() + if err := conn.Close(); err != nil { + c.logger.Debug("failed to close updates websocket", zap.Error(err)) + } }() for { @@ -268,7 +282,9 @@ func (c *Client) CloseACPStream() { defer c.mu.Unlock() if c.acpConn != nil { - c.acpConn.Close() + if err := c.acpConn.Close(); err != nil { + c.logger.Debug("failed to close ACP stream", zap.Error(err)) + } c.acpConn = nil } } @@ -294,7 +310,9 @@ func (c *Client) StreamPermissions(ctx context.Context, handler func(*Permission c.mu.Lock() c.permissionConn = nil c.mu.Unlock() - conn.Close() + if err := conn.Close(); err != nil { + c.logger.Debug("failed to close permissions websocket", zap.Error(err)) + } }() for { @@ -327,7 +345,9 @@ func (c *Client) ClosePermissionStream() { defer c.mu.Unlock() if c.permissionConn != nil { - c.permissionConn.Close() + if err := c.permissionConn.Close(); err != nil { + c.logger.Debug("failed to close permission stream", zap.Error(err)) + } c.permissionConn = nil } } @@ -355,7 +375,11 @@ func (c *Client) RespondToPermission(ctx context.Context, pendingID, optionID st if err != nil { return err } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + c.logger.Debug("failed to close permission response body", zap.Error(err)) + } + }() var result PermissionRespondResponse if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { diff --git a/apps/backend/internal/agent/agentctl/client.go b/apps/backend/internal/agent/agentctl/client.go index 7a9d87dea5..9c93b89af5 100644 --- a/apps/backend/internal/agent/agentctl/client.go +++ b/apps/backend/internal/agent/agentctl/client.go @@ -62,7 +62,7 @@ func (c *Client) Health(ctx context.Context) error { if err != nil { return err } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { return fmt.Errorf("health check failed: %d", resp.StatusCode) @@ -81,7 +81,7 @@ func (c *Client) GetStatus(ctx context.Context) (*StatusResponse, error) { if err != nil { return nil, err } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() var status StatusResponse if err := json.NewDecoder(resp.Body).Decode(&status); err != nil { @@ -101,7 +101,7 @@ func (c *Client) Start(ctx context.Context) error { if err != nil { return err } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() var result struct { Success bool `json:"success"` @@ -127,7 +127,7 @@ func (c *Client) Stop(ctx context.Context) error { if err != nil { return err } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() var result struct { Success bool `json:"success"` @@ -205,7 +205,9 @@ func (c *Client) StreamGitStatus(ctx context.Context, handler func(*GitStatusUpd c.mu.Lock() c.gitStatusConn = nil c.mu.Unlock() - conn.Close() + if err := conn.Close(); err != nil { + c.logger.Debug("failed to close git status websocket", zap.Error(err)) + } }() for { @@ -238,7 +240,9 @@ func (c *Client) CloseGitStatusStream() { defer c.mu.Unlock() if c.gitStatusConn != nil { - c.gitStatusConn.Close() + if err := c.gitStatusConn.Close(); err != nil { + c.logger.Debug("failed to close git status stream", zap.Error(err)) + } c.gitStatusConn = nil } } @@ -264,7 +268,9 @@ func (c *Client) StreamFileChanges(ctx context.Context, handler func(*FileChange c.mu.Lock() c.fileChangesConn = nil c.mu.Unlock() - conn.Close() + if err := conn.Close(); err != nil { + c.logger.Debug("failed to close file changes websocket", zap.Error(err)) + } }() for { @@ -304,7 +310,11 @@ func (c *Client) RequestFileTree(ctx context.Context, path string, depth int) (* if err != nil { return nil, fmt.Errorf("failed to request file tree: %w", err) } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + c.logger.Debug("failed to close file tree response body", zap.Error(err)) + } + }() var response FileTreeResponse if err := json.NewDecoder(resp.Body).Decode(&response); err != nil { @@ -331,7 +341,11 @@ func (c *Client) RequestFileContent(ctx context.Context, path string) (*FileCont if err != nil { return nil, fmt.Errorf("failed to request file content: %w", err) } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + c.logger.Debug("failed to close file content response body", zap.Error(err)) + } + }() var response FileContentResponse if err := json.NewDecoder(resp.Body).Decode(&response); err != nil { @@ -351,7 +365,9 @@ func (c *Client) CloseFileChangesStream() { defer c.mu.Unlock() if c.fileChangesConn != nil { - c.fileChangesConn.Close() + if err := c.fileChangesConn.Close(); err != nil { + c.logger.Debug("failed to close file changes stream", zap.Error(err)) + } c.fileChangesConn = nil } } diff --git a/apps/backend/internal/agent/agentctl/launcher/launcher.go b/apps/backend/internal/agent/agentctl/launcher/launcher.go index 435ba7140a..fc141b4c4f 100644 --- a/apps/backend/internal/agent/agentctl/launcher/launcher.go +++ b/apps/backend/internal/agent/agentctl/launcher/launcher.go @@ -164,7 +164,9 @@ func (l *Launcher) Start(ctx context.Context) error { // Wait for health check to pass if err := l.waitForHealthy(ctx); err != nil { // Kill the process if health check fails - l.cmd.Process.Kill() + if killErr := l.cmd.Process.Kill(); killErr != nil { + l.logger.Warn("failed to kill agentctl process after failed health check", zap.Error(killErr)) + } return fmt.Errorf("agentctl failed to become healthy: %w", err) } @@ -230,7 +232,9 @@ func (l *Launcher) checkPortAvailable() error { if err != nil { return err } - ln.Close() + if err := ln.Close(); err != nil { + return err + } return nil } @@ -255,7 +259,9 @@ func (l *Launcher) waitForHealthy(ctx context.Context) error { resp, err := client.Get(healthURL) if err == nil { - resp.Body.Close() + if closeErr := resp.Body.Close(); closeErr != nil { + l.logger.Debug("failed to close health response body", zap.Error(closeErr)) + } if resp.StatusCode == http.StatusOK { return nil } diff --git a/apps/backend/internal/agent/agentctl/standalone.go b/apps/backend/internal/agent/agentctl/standalone.go index 841c5176d7..68dca7f09b 100644 --- a/apps/backend/internal/agent/agentctl/standalone.go +++ b/apps/backend/internal/agent/agentctl/standalone.go @@ -72,7 +72,9 @@ func (s *StandaloneCtl) Health(ctx context.Context) error { if err != nil { return err } - defer resp.Body.Close() + defer func() { + _ = resp.Body.Close() + }() if resp.StatusCode != http.StatusOK { return fmt.Errorf("health check failed: %d", resp.StatusCode) @@ -97,13 +99,15 @@ func (s *StandaloneCtl) CreateInstance(ctx context.Context, req *CreateInstanceR if err != nil { return nil, fmt.Errorf("failed to create instance: %w", err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusOK { var errResp struct { Error string `json:"error"` } - json.NewDecoder(resp.Body).Decode(&errResp) + if err := json.NewDecoder(resp.Body).Decode(&errResp); err != nil { + return nil, fmt.Errorf("failed to decode error response: %w", err) + } return nil, fmt.Errorf("failed to create instance: %s (status %d)", errResp.Error, resp.StatusCode) } @@ -130,13 +134,17 @@ func (s *StandaloneCtl) DeleteInstance(ctx context.Context, instanceID string) e if err != nil { return fmt.Errorf("failed to delete instance: %w", err) } - defer resp.Body.Close() + defer func() { + _ = resp.Body.Close() + }() if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusNoContent { var errResp struct { Error string `json:"error"` } - json.NewDecoder(resp.Body).Decode(&errResp) + if err := json.NewDecoder(resp.Body).Decode(&errResp); err != nil { + return fmt.Errorf("failed to decode error response: %w", err) + } return fmt.Errorf("failed to delete instance: %s (status %d)", errResp.Error, resp.StatusCode) } @@ -155,7 +163,7 @@ func (s *StandaloneCtl) GetInstance(ctx context.Context, instanceID string) (*In if err != nil { return nil, fmt.Errorf("failed to get instance: %w", err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() if resp.StatusCode == http.StatusNotFound { return nil, fmt.Errorf("instance %q not found", instanceID) @@ -182,7 +190,7 @@ func (s *StandaloneCtl) ListInstances(ctx context.Context) ([]*InstanceInfo, err if err != nil { return nil, fmt.Errorf("failed to list instances: %w", err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("failed to list instances: status %d", resp.StatusCode) @@ -196,4 +204,3 @@ func (s *StandaloneCtl) ListInstances(ctx context.Context) ([]*InstanceInfo, err } return result.Instances, nil } - diff --git a/apps/backend/internal/agent/credentials/manager_test.go b/apps/backend/internal/agent/credentials/manager_test.go index f42f307772..724a337971 100644 --- a/apps/backend/internal/agent/credentials/manager_test.go +++ b/apps/backend/internal/agent/credentials/manager_test.go @@ -47,8 +47,7 @@ func TestManager_GetCredential_FromEnv(t *testing.T) { // Set up test environment variable testKey := "TEST_CREDENTIAL_KEY_12345" testValue := "test-secret-value" - os.Setenv(testKey, testValue) - defer os.Unsetenv(testKey) + t.Setenv(testKey, testValue) log := newTestLogger() mgr := NewManager(log) @@ -74,8 +73,7 @@ func TestManager_GetCredential_FromEnv(t *testing.T) { func TestManager_GetCredential_Cached(t *testing.T) { testKey := "TEST_CACHED_KEY" testValue := "cached-value" - os.Setenv(testKey, testValue) - defer os.Unsetenv(testKey) + t.Setenv(testKey, testValue) log := newTestLogger() mgr := NewManager(log) @@ -90,7 +88,9 @@ func TestManager_GetCredential_Cached(t *testing.T) { } // Remove from env - should still get cached value - os.Unsetenv(testKey) + if err := os.Unsetenv(testKey); err != nil { + t.Fatalf("failed to unset env var: %v", err) + } cred2, err := mgr.GetCredential(ctx, testKey) if err != nil { @@ -117,12 +117,8 @@ func TestManager_GetCredential_NotFound(t *testing.T) { func TestManager_GetCredentials(t *testing.T) { testKey1 := "TEST_MULTI_KEY_1" testKey2 := "TEST_MULTI_KEY_2" - os.Setenv(testKey1, "value1") - os.Setenv(testKey2, "value2") - defer func() { - os.Unsetenv(testKey1) - os.Unsetenv(testKey2) - }() + t.Setenv(testKey1, "value1") + t.Setenv(testKey2, "value2") log := newTestLogger() mgr := NewManager(log) @@ -147,8 +143,7 @@ func TestManager_GetCredentials(t *testing.T) { func TestManager_GetCredentials_PartialFailure(t *testing.T) { testKey := "TEST_PARTIAL_KEY" - os.Setenv(testKey, "value") - defer os.Unsetenv(testKey) + t.Setenv(testKey, "value") log := newTestLogger() mgr := NewManager(log) @@ -169,8 +164,7 @@ func TestManager_GetCredentials_PartialFailure(t *testing.T) { func TestManager_BuildEnvVars(t *testing.T) { testKey := "TEST_BUILD_ENV_KEY" testValue := "build-env-value" - os.Setenv(testKey, testValue) - defer os.Unsetenv(testKey) + t.Setenv(testKey, testValue) log := newTestLogger() mgr := NewManager(log) @@ -219,8 +213,7 @@ func TestManager_BuildEnvVars_MissingRequired(t *testing.T) { func TestManager_HasCredential(t *testing.T) { testKey := "TEST_HAS_CREDENTIAL" - os.Setenv(testKey, "value") - defer os.Unsetenv(testKey) + t.Setenv(testKey, "value") log := newTestLogger() mgr := NewManager(log) @@ -238,8 +231,7 @@ func TestManager_HasCredential(t *testing.T) { func TestManager_ListAvailable(t *testing.T) { testKey := "TEST_LIST_AVAILABLE_API_KEY" - os.Setenv(testKey, "value") - defer os.Unsetenv(testKey) + t.Setenv(testKey, "value") log := newTestLogger() mgr := NewManager(log) @@ -263,8 +255,7 @@ func TestManager_ListAvailable(t *testing.T) { func TestManager_ClearCache(t *testing.T) { testKey := "TEST_CLEAR_CACHE" - os.Setenv(testKey, "original-value") - defer os.Unsetenv(testKey) + t.Setenv(testKey, "original-value") log := newTestLogger() mgr := NewManager(log) @@ -279,7 +270,9 @@ func TestManager_ClearCache(t *testing.T) { } // Update the env and clear cache - os.Setenv(testKey, "new-value") + if err := os.Setenv(testKey, "new-value"); err != nil { + t.Fatalf("failed to update env var: %v", err) + } mgr.ClearCache() // Should get new value @@ -301,8 +294,7 @@ func TestEnvProvider_Name(t *testing.T) { func TestEnvProvider_GetCredential(t *testing.T) { testKey := "TEST_ENV_PROVIDER_KEY" testValue := "env-provider-value" - os.Setenv(testKey, testValue) - defer os.Unsetenv(testKey) + t.Setenv(testKey, testValue) provider := NewEnvProvider("") ctx := context.Background() @@ -323,8 +315,7 @@ func TestEnvProvider_GetCredential_WithPrefix(t *testing.T) { prefix := "KANDEV_" testKey := "MY_SECRET" testValue := "prefixed-value" - os.Setenv(prefix+testKey, testValue) - defer os.Unsetenv(prefix + testKey) + t.Setenv(prefix+testKey, testValue) provider := NewEnvProvider(prefix) ctx := context.Background() @@ -343,8 +334,7 @@ func TestEnvProvider_GetCredential_WithPrefix(t *testing.T) { func TestEnvProvider_ListAvailable(t *testing.T) { testKey := "ANTHROPIC_API_KEY" - os.Setenv(testKey, "test-value") - defer os.Unsetenv(testKey) + t.Setenv(testKey, "test-value") provider := NewEnvProvider("") ctx := context.Background() @@ -412,7 +402,9 @@ func TestFileProvider_GetCredential_NotFound(t *testing.T) { creds := map[string]string{"EXISTING": "value"} data, _ := json.Marshal(creds) - os.WriteFile(configPath, data, 0644) + if err := os.WriteFile(configPath, data, 0644); err != nil { + t.Fatalf("failed to write test file: %v", err) + } provider := NewFileProvider(configPath) ctx := context.Background() @@ -438,7 +430,9 @@ func TestFileProvider_InvalidJSON(t *testing.T) { tmpDir := t.TempDir() configPath := filepath.Join(tmpDir, "invalid.json") - os.WriteFile(configPath, []byte("invalid json"), 0644) + if err := os.WriteFile(configPath, []byte("invalid json"), 0644); err != nil { + t.Fatalf("failed to write test file: %v", err) + } provider := NewFileProvider(configPath) ctx := context.Background() @@ -458,7 +452,9 @@ func TestFileProvider_ListAvailable(t *testing.T) { "KEY_2": "value2", } data, _ := json.Marshal(creds) - os.WriteFile(configPath, data, 0644) + if err := os.WriteFile(configPath, data, 0644); err != nil { + t.Fatalf("failed to write test file: %v", err) + } provider := NewFileProvider(configPath) ctx := context.Background() @@ -480,7 +476,9 @@ func TestFileProvider_Reload(t *testing.T) { // Initial content creds := map[string]string{"KEY": "original"} data, _ := json.Marshal(creds) - os.WriteFile(configPath, data, 0644) + if err := os.WriteFile(configPath, data, 0644); err != nil { + t.Fatalf("failed to write test file: %v", err) + } provider := NewFileProvider(configPath) ctx := context.Background() @@ -494,7 +492,9 @@ func TestFileProvider_Reload(t *testing.T) { // Update file creds["KEY"] = "updated" data, _ = json.Marshal(creds) - os.WriteFile(configPath, data, 0644) + if err := os.WriteFile(configPath, data, 0644); err != nil { + t.Fatalf("failed to write test file: %v", err) + } // Reload err := provider.Reload() @@ -508,4 +508,3 @@ func TestFileProvider_Reload(t *testing.T) { t.Errorf("expected updated, got %q", cred.Value) } } - diff --git a/apps/backend/internal/agent/docker/client.go b/apps/backend/internal/agent/docker/client.go index b9331ccfbd..2e29a857b7 100644 --- a/apps/backend/internal/agent/docker/client.go +++ b/apps/backend/internal/agent/docker/client.go @@ -107,7 +107,11 @@ func (c *Client) PullImage(ctx context.Context, imageName string) error { c.logger.Error("Failed to pull image", zap.String("image", imageName), zap.Error(err)) return fmt.Errorf("failed to pull image %s: %w", imageName, err) } - defer reader.Close() + defer func() { + if err := reader.Close(); err != nil { + c.logger.Warn("Failed to close image pull reader", zap.Error(err)) + } + }() // Read the output to ensure the image is fully pulled _, err = io.Copy(io.Discard, reader) @@ -303,14 +307,8 @@ func (c *Client) GetContainerIP(ctx context.Context, containerID string) (string return "", err } - // Try to get IP from default bridge network first if inspect.NetworkSettings != nil { - // Check bridge network - if inspect.NetworkSettings.IPAddress != "" { - return inspect.NetworkSettings.IPAddress, nil - } - - // Check other networks + // Check available networks for an IP address. for netName, netSettings := range inspect.NetworkSettings.Networks { if netSettings.IPAddress != "" { c.logger.Debug("Found container IP", @@ -538,7 +536,9 @@ func (c *Client) AttachContainer(ctx context.Context, containerID string) (*Atta // Start goroutine to copy from pipe to container go func() { - io.Copy(resp.Conn, stdinReader) + if _, err := io.Copy(resp.Conn, stdinReader); err != nil { + c.logger.Debug("failed to copy stdin to container", zap.Error(err)) + } }() // Create a pipe for demultiplexed stdout @@ -547,7 +547,11 @@ func (c *Client) AttachContainer(ctx context.Context, containerID string) (*Atta // Start goroutine to demultiplex stdout/stderr go func() { - defer stdoutWriter.Close() + defer func() { + if err := stdoutWriter.Close(); err != nil { + c.logger.Debug("failed to close stdout writer", zap.Error(err)) + } + }() c.demultiplexStream(resp.Reader, stdoutWriter) }() @@ -594,7 +598,9 @@ func (c *Client) demultiplexStream(reader io.Reader, writer io.Writer) { // Write stdout (type 1) and stderr (type 2) to writer // We want both for ACP since errors should be visible if streamType == 1 || streamType == 2 { - writer.Write(data) + if _, err := writer.Write(data); err != nil { + c.logger.Debug("failed to write demultiplexed data", zap.Error(err)) + } } } } @@ -603,10 +609,14 @@ func (c *Client) demultiplexStream(reader io.Reader, writer io.Writer) { // Close closes the attach result func (a *AttachResult) Close() error { if a.Stdin != nil { - a.Stdin.Close() + if err := a.Stdin.Close(); err != nil { + return err + } } if a.Conn != nil { - a.Conn.Close() + if err := a.Conn.Close(); err != nil { + return err + } } return nil } diff --git a/apps/backend/internal/agent/lifecycle/manager_test.go b/apps/backend/internal/agent/lifecycle/manager_test.go index 110c2a964c..313517c4c7 100644 --- a/apps/backend/internal/agent/lifecycle/manager_test.go +++ b/apps/backend/internal/agent/lifecycle/manager_test.go @@ -135,26 +135,6 @@ func newTestRegistry() *registry.Registry { return reg } -// managerWithMock creates a manager with a mock docker client -// We need to use reflection or a wrapper to inject the mock -type testManager struct { - *Manager - mockDocker *MockDockerClient -} - -func newTestManager(mockDocker *MockDockerClient, eventBus bus.EventBus) *testManager { - log := newTestLogger() - reg := newTestRegistry() - - // Create a real manager first - mgr := NewManager(nil, reg, eventBus, testAgentConfig(), log) - - return &testManager{ - Manager: mgr, - mockDocker: mockDocker, - } -} - func TestNewManager(t *testing.T) { log := newTestLogger() reg := newTestRegistry() @@ -519,4 +499,3 @@ func TestManager_StartStop(t *testing.T) { t.Fatalf("unexpected error stopping manager: %v", err) } } - diff --git a/apps/backend/internal/agent/settings/store/sqlite.go b/apps/backend/internal/agent/settings/store/sqlite.go index f0e4771509..94cd03ae13 100644 --- a/apps/backend/internal/agent/settings/store/sqlite.go +++ b/apps/backend/internal/agent/settings/store/sqlite.go @@ -38,7 +38,9 @@ func NewSQLiteRepository(dbPath string) (*SQLiteRepository, error) { repo := &SQLiteRepository{db: db} if err := repo.initSchema(); err != nil { - db.Close() + if closeErr := db.Close(); closeErr != nil { + return nil, fmt.Errorf("failed to close database after schema error: %w", closeErr) + } return nil, fmt.Errorf("failed to initialize schema: %w", err) } return repo, nil @@ -181,7 +183,9 @@ func (r *SQLiteRepository) ListAgents(ctx context.Context) ([]*models.Agent, err if err != nil { return nil, err } - defer rows.Close() + defer func() { + _ = rows.Close() + }() var result []*models.Agent for rows.Next() { @@ -258,7 +262,9 @@ func (r *SQLiteRepository) ListAgentProfiles(ctx context.Context, agentID string if err != nil { return nil, err } - defer rows.Close() + defer func() { + _ = rows.Close() + }() var result []*models.AgentProfile for rows.Next() { @@ -338,7 +344,9 @@ func columnExists(db *sql.DB, table, column string) (bool, error) { if err != nil { return false, err } - defer rows.Close() + defer func() { + _ = rows.Close() + }() for rows.Next() { var cid int var name, ctype string diff --git a/apps/backend/internal/agent/worktree/manager.go b/apps/backend/internal/agent/worktree/manager.go index d3274cddba..2197cea737 100644 --- a/apps/backend/internal/agent/worktree/manager.go +++ b/apps/backend/internal/agent/worktree/manager.go @@ -240,7 +240,9 @@ func (m *Manager) createWorktree(ctx context.Context, req CreateRequest) (*Workt if m.store != nil { if err := m.store.CreateWorktree(ctx, wt); err != nil { // Cleanup on failure - m.removeWorktreeDir(ctx, worktreePath, req.RepositoryPath) + if cleanupErr := m.removeWorktreeDir(ctx, worktreePath, req.RepositoryPath); cleanupErr != nil { + m.logger.Warn("failed to cleanup worktree after persist failure", zap.Error(cleanupErr)) + } return nil, fmt.Errorf("failed to persist worktree: %w", err) } } @@ -561,7 +563,9 @@ func (m *Manager) removeWorktreeDir(ctx context.Context, worktreePath, repoPath // Prune stale worktree entries cmd = exec.CommandContext(ctx, "git", "worktree", "prune") cmd.Dir = repoPath - cmd.Run() // Ignore errors + if err := cmd.Run(); err != nil { + m.logger.Debug("git worktree prune failed", zap.Error(err)) + } } return nil } @@ -570,13 +574,17 @@ func (m *Manager) removeWorktreeDir(ctx context.Context, worktreePath, repoPath func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRequest) (*Worktree, error) { // Clean up existing directory if present if existing.Path != "" { - os.RemoveAll(existing.Path) + if err := os.RemoveAll(existing.Path); err != nil { + m.logger.Debug("failed to remove existing worktree path", zap.Error(err)) + } } // Remove from git worktree list cmd := exec.CommandContext(ctx, "git", "worktree", "prune") cmd.Dir = req.RepositoryPath - cmd.Run() // Ignore errors + if err := cmd.Run(); err != nil { + m.logger.Debug("git worktree prune failed", zap.Error(err)) + } // Get repository lock repoLock := m.getRepoLock(req.RepositoryPath) @@ -623,4 +631,4 @@ func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRe zap.String("path", worktreePath)) return existing, nil -} \ No newline at end of file +} diff --git a/apps/backend/internal/agent/worktree/store.go b/apps/backend/internal/agent/worktree/store.go index d28466a299..a97971254e 100644 --- a/apps/backend/internal/agent/worktree/store.go +++ b/apps/backend/internal/agent/worktree/store.go @@ -151,7 +151,7 @@ func (s *SQLiteStore) GetWorktreesByTaskID(ctx context.Context, taskID string) ( if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() return s.scanWorktrees(rows) } @@ -166,7 +166,7 @@ func (s *SQLiteStore) GetWorktreesByRepositoryID(ctx context.Context, repoID str if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() return s.scanWorktrees(rows) } @@ -217,7 +217,7 @@ func (s *SQLiteStore) ListActiveWorktrees(ctx context.Context) ([]*Worktree, err if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() return s.scanWorktrees(rows) } diff --git a/apps/backend/internal/agentctl/adapter/codex_adapter.go b/apps/backend/internal/agentctl/adapter/codex_adapter.go index a5c0acb2a6..ca1147472f 100644 --- a/apps/backend/internal/agentctl/adapter/codex_adapter.go +++ b/apps/backend/internal/agentctl/adapter/codex_adapter.go @@ -93,10 +93,10 @@ func (a *CodexAdapter) Initialize(ctx context.Context) error { }, }) if err != nil { - return fmt.Errorf("Codex initialize handshake failed: %w", err) + return fmt.Errorf("codex initialize handshake failed: %w", err) } if resp.Error != nil { - return fmt.Errorf("Codex initialize error: %s", resp.Error.Message) + return fmt.Errorf("codex initialize error: %s", resp.Error.Message) } // Parse initialize result @@ -563,7 +563,9 @@ func (a *CodexAdapter) handleRequest(id interface{}, method string, params json. var p codex.CommandApprovalParams if err := json.Unmarshal(params, &p); err != nil { a.logger.Warn("failed to parse command approval request", zap.Error(err)) - a.client.SendResponse(id, nil, &codex.Error{Code: codex.InvalidParams, Message: "invalid params"}) + if err := a.client.SendResponse(id, nil, &codex.Error{Code: codex.InvalidParams, Message: "invalid params"}); err != nil { + a.logger.Warn("failed to send invalid params response", zap.Error(err)) + } return } a.handleApprovalRequest(id, handler, threadID, p.ItemID, types.ActionTypeCommand, p.Command, map[string]interface{}{ @@ -576,7 +578,9 @@ func (a *CodexAdapter) handleRequest(id interface{}, method string, params json. var p codex.FileChangeApprovalParams if err := json.Unmarshal(params, &p); err != nil { a.logger.Warn("failed to parse file change approval request", zap.Error(err)) - a.client.SendResponse(id, nil, &codex.Error{Code: codex.InvalidParams, Message: "invalid params"}) + if err := a.client.SendResponse(id, nil, &codex.Error{Code: codex.InvalidParams, Message: "invalid params"}); err != nil { + a.logger.Warn("failed to send invalid params response", zap.Error(err)) + } return } a.handleApprovalRequest(id, handler, threadID, p.ItemID, types.ActionTypeFileWrite, p.Path, map[string]interface{}{ @@ -587,7 +591,9 @@ func (a *CodexAdapter) handleRequest(id interface{}, method string, params json. default: a.logger.Warn("unhandled request", zap.String("method", method)) - a.client.SendResponse(id, nil, &codex.Error{Code: codex.MethodNotFound, Message: "method not found"}) + if err := a.client.SendResponse(id, nil, &codex.Error{Code: codex.MethodNotFound, Message: "method not found"}); err != nil { + a.logger.Warn("failed to send method not found response", zap.Error(err)) + } } } @@ -606,9 +612,10 @@ func (a *CodexAdapter) handleApprovalRequest( options := make([]PermissionOption, len(optionStrings)) for i, opt := range optionStrings { kind := "allow_once" - if opt == "approveAlways" { + switch opt { + case "approveAlways": kind = "allow_always" - } else if opt == "reject" { + case "reject": kind = "reject_once" } options[i] = PermissionOption{ @@ -638,10 +645,12 @@ func (a *CodexAdapter) handleApprovalRequest( if handler == nil { // Auto-approve if no handler a.logger.Info("auto-approving request (no handler)", zap.String("item_id", itemID)) - a.client.SendResponse(id, &codex.ApprovalResponse{ + if err := a.client.SendResponse(id, &codex.ApprovalResponse{ ItemID: itemID, Decision: "approve", - }, nil) + }, nil); err != nil { + a.logger.Warn("failed to send approval response", zap.Error(err)) + } return } @@ -650,10 +659,12 @@ func (a *CodexAdapter) handleApprovalRequest( resp, err := handler(ctx, req) if err != nil { a.logger.Error("permission handler error", zap.Error(err)) - a.client.SendResponse(id, &codex.ApprovalResponse{ + if err := a.client.SendResponse(id, &codex.ApprovalResponse{ ItemID: itemID, Decision: "reject", - }, nil) + }, nil); err != nil { + a.logger.Warn("failed to send reject response", zap.Error(err)) + } return } @@ -664,12 +675,13 @@ func (a *CodexAdapter) handleApprovalRequest( decision = resp.OptionID } - a.client.SendResponse(id, &codex.ApprovalResponse{ + if err := a.client.SendResponse(id, &codex.ApprovalResponse{ ItemID: itemID, Decision: decision, - }, nil) + }, nil); err != nil { + a.logger.Warn("failed to send approval response", zap.Error(err)) + } } // Verify interface implementation var _ AgentAdapter = (*CodexAdapter)(nil) - diff --git a/apps/backend/internal/agentctl/api/acp.go b/apps/backend/internal/agentctl/api/acp.go index 0e23a510be..9a14456385 100644 --- a/apps/backend/internal/agentctl/api/acp.go +++ b/apps/backend/internal/agentctl/api/acp.go @@ -196,7 +196,11 @@ func (s *Server) handleACPStreamWS(c *gin.Context) { s.logger.Error("WebSocket upgrade failed", zap.Error(err)) return } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + s.logger.Debug("failed to close ACP stream websocket", zap.Error(err)) + } + }() s.logger.Info("ACP stream WebSocket connected") @@ -204,22 +208,15 @@ func (s *Server) handleACPStreamWS(c *gin.Context) { updatesCh := s.procMgr.GetUpdates() // Stream session notifications to WebSocket - for { - select { - case notification, ok := <-updatesCh: - if !ok { - return - } + for notification := range updatesCh { + data, err := json.Marshal(notification) + if err != nil { + s.logger.Error("failed to marshal notification", zap.Error(err)) + continue + } - data, err := json.Marshal(notification) - if err != nil { - s.logger.Error("failed to marshal notification", zap.Error(err)) - continue - } - - if err := conn.WriteMessage(websocket.TextMessage, data); err != nil { - return - } + if err := conn.WriteMessage(websocket.TextMessage, data); err != nil { + return } } } @@ -318,7 +315,11 @@ func (s *Server) handlePermissionStreamWS(c *gin.Context) { s.logger.Error("WebSocket upgrade failed", zap.Error(err)) return } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + s.logger.Debug("failed to close permission websocket", zap.Error(err)) + } + }() s.logger.Info("Permission stream WebSocket connected") @@ -326,23 +327,16 @@ func (s *Server) handlePermissionStreamWS(c *gin.Context) { permissionCh := s.procMgr.GetPermissionRequests() // Stream permission notifications to WebSocket - for { - select { - case notification, ok := <-permissionCh: - if !ok { - return - } + for notification := range permissionCh { + data, err := json.Marshal(notification) + if err != nil { + s.logger.Error("failed to marshal permission notification", zap.Error(err)) + continue + } - data, err := json.Marshal(notification) - if err != nil { - s.logger.Error("failed to marshal permission notification", zap.Error(err)) - continue - } - - if err := conn.WriteMessage(websocket.TextMessage, data); err != nil { - s.logger.Debug("WebSocket write error", zap.Error(err)) - return - } + if err := conn.WriteMessage(websocket.TextMessage, data); err != nil { + s.logger.Debug("WebSocket write error", zap.Error(err)) + return } } } diff --git a/apps/backend/internal/agentctl/api/control_server.go b/apps/backend/internal/agentctl/api/control_server.go index 06cc48aaaf..470942e154 100644 --- a/apps/backend/internal/agentctl/api/control_server.go +++ b/apps/backend/internal/agentctl/api/control_server.go @@ -400,28 +400,25 @@ func (m *ControlServer) handleACPStreamWS(c *gin.Context) { m.logger.Error("WebSocket upgrade failed", zap.Error(err)) return } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + m.logger.Debug("failed to close ACP stream websocket", zap.Error(err)) + } + }() m.logger.Info("ACP stream WebSocket connected") updatesCh := m.singleProcMgr.GetUpdates() - for { - select { - case notification, ok := <-updatesCh: - if !ok { - return - } + for notification := range updatesCh { + data, err := json.Marshal(notification) + if err != nil { + m.logger.Error("failed to marshal notification", zap.Error(err)) + continue + } - data, err := json.Marshal(notification) - if err != nil { - m.logger.Error("failed to marshal notification", zap.Error(err)) - continue - } - - if err := conn.WriteMessage(websocket.TextMessage, data); err != nil { - return - } + if err := conn.WriteMessage(websocket.TextMessage, data); err != nil { + return } } } @@ -458,29 +455,26 @@ func (m *ControlServer) handlePermissionStreamWS(c *gin.Context) { m.logger.Error("WebSocket upgrade failed", zap.Error(err)) return } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + m.logger.Debug("failed to close permission websocket", zap.Error(err)) + } + }() m.logger.Info("Permission stream WebSocket connected") permissionCh := m.singleProcMgr.GetPermissionRequests() - for { - select { - case notification, ok := <-permissionCh: - if !ok { - return - } + for notification := range permissionCh { + data, err := json.Marshal(notification) + if err != nil { + m.logger.Error("failed to marshal permission notification", zap.Error(err)) + continue + } - data, err := json.Marshal(notification) - if err != nil { - m.logger.Error("failed to marshal permission notification", zap.Error(err)) - continue - } - - if err := conn.WriteMessage(websocket.TextMessage, data); err != nil { - m.logger.Debug("WebSocket write error", zap.Error(err)) - return - } + if err := conn.WriteMessage(websocket.TextMessage, data); err != nil { + m.logger.Debug("WebSocket write error", zap.Error(err)) + return } } } @@ -523,7 +517,11 @@ func (m *ControlServer) handleGitStatusStreamWS(c *gin.Context) { m.logger.Error("WebSocket upgrade failed", zap.Error(err)) return } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + m.logger.Debug("failed to close git status websocket", zap.Error(err)) + } + }() sub := m.singleProcMgr.GetWorkspaceTracker().SubscribeGitStatus() defer m.singleProcMgr.GetWorkspaceTracker().UnsubscribeGitStatus(sub) @@ -565,7 +563,11 @@ func (m *ControlServer) handleFilesStreamWS(c *gin.Context) { m.logger.Error("WebSocket upgrade failed", zap.Error(err)) return } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + m.logger.Debug("failed to close files websocket", zap.Error(err)) + } + }() sub := m.singleProcMgr.GetWorkspaceTracker().SubscribeFiles() defer m.singleProcMgr.GetWorkspaceTracker().UnsubscribeFiles(sub) @@ -607,7 +609,11 @@ func (m *ControlServer) handleFileChangesStreamWS(c *gin.Context) { m.logger.Error("WebSocket upgrade failed", zap.Error(err)) return } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + m.logger.Debug("failed to close file changes websocket", zap.Error(err)) + } + }() sub := m.singleProcMgr.GetWorkspaceTracker().SubscribeFileChanges() defer m.singleProcMgr.GetWorkspaceTracker().UnsubscribeFileChanges(sub) @@ -675,4 +681,4 @@ func (m *ControlServer) handleFileContent(c *gin.Context) { } c.JSON(200, process.FileContentResponse{Path: path, Content: content, Size: size}) -} \ No newline at end of file +} diff --git a/apps/backend/internal/agentctl/api/workspace.go b/apps/backend/internal/agentctl/api/workspace.go index 44c9be2b0f..5317289ad7 100644 --- a/apps/backend/internal/agentctl/api/workspace.go +++ b/apps/backend/internal/agentctl/api/workspace.go @@ -16,7 +16,11 @@ func (s *Server) handleGitStatusStreamWS(c *gin.Context) { s.logger.Error("WebSocket upgrade failed", zap.Error(err)) return } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + s.logger.Debug("failed to close git status websocket", zap.Error(err)) + } + }() // Subscribe to git status updates sub := s.procMgr.GetWorkspaceTracker().SubscribeGitStatus() @@ -62,7 +66,11 @@ func (s *Server) handleFilesStreamWS(c *gin.Context) { s.logger.Error("WebSocket upgrade failed", zap.Error(err)) return } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + s.logger.Debug("failed to close files websocket", zap.Error(err)) + } + }() // Subscribe to file updates sub := s.procMgr.GetWorkspaceTracker().SubscribeFiles() @@ -108,7 +116,11 @@ func (s *Server) handleFileChangesStreamWS(c *gin.Context) { s.logger.Error("WebSocket upgrade failed", zap.Error(err)) return } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + s.logger.Debug("failed to close file changes websocket", zap.Error(err)) + } + }() // Subscribe to file change notifications sub := s.procMgr.GetWorkspaceTracker().SubscribeFileChanges() @@ -186,6 +198,8 @@ func (s *Server) handleFileContent(c *gin.Context) { // mustParseInt parses a string to int, returns 0 on error func mustParseInt(s string) int { var n int - json.Unmarshal([]byte(s), &n) + if err := json.Unmarshal([]byte(s), &n); err != nil { + return 0 + } return n } diff --git a/apps/backend/internal/agentctl/instance/manager.go b/apps/backend/internal/agentctl/instance/manager.go index f575d4ca4c..248e02b499 100644 --- a/apps/backend/internal/agentctl/instance/manager.go +++ b/apps/backend/internal/agentctl/instance/manager.go @@ -124,7 +124,9 @@ func (m *Manager) CreateInstance(ctx context.Context, req *CreateRequest) (*Crea // Default to a simple handler if no factory is set handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusServiceUnavailable) - w.Write([]byte("server factory not configured")) + if _, err := w.Write([]byte("server factory not configured")); err != nil { + m.logger.Debug("failed to write default response", zap.Error(err)) + } }) } @@ -277,4 +279,3 @@ func collectEnvForInstance(env map[string]string) []string { } return result } - diff --git a/apps/backend/internal/agentctl/process/manager.go b/apps/backend/internal/agentctl/process/manager.go index 141a3057fc..ffed190bf8 100644 --- a/apps/backend/internal/agentctl/process/manager.go +++ b/apps/backend/internal/agentctl/process/manager.go @@ -312,7 +312,9 @@ func (m *Manager) Stop(ctx context.Context) error { // Close the adapter if m.adapter != nil { - m.adapter.Close() + if err := m.adapter.Close(); err != nil { + m.logger.Debug("failed to close adapter", zap.Error(err)) + } } // Close stop channel to signal readers @@ -322,7 +324,9 @@ func (m *Manager) Stop(ctx context.Context) error { // Close stdin to signal EOF to agent if m.stdin != nil { - m.stdin.Close() + if err := m.stdin.Close(); err != nil { + m.logger.Debug("failed to close stdin", zap.Error(err)) + } } // Wait for process to exit with timeout @@ -339,7 +343,9 @@ func (m *Manager) Stop(ctx context.Context) error { // Force kill if m.cmd != nil && m.cmd.Process != nil { m.logger.Warn("force killing agent process") - m.cmd.Process.Kill() + if err := m.cmd.Process.Kill(); err != nil { + m.logger.Warn("failed to kill agent process", zap.Error(err)) + } } } @@ -563,4 +569,3 @@ func (m *Manager) GetPendingPermissions() []*PendingPermission { return result } - diff --git a/apps/backend/internal/agentctl/process/workspace_tracker.go b/apps/backend/internal/agentctl/process/workspace_tracker.go index f3e4f31362..a18fbc92dd 100644 --- a/apps/backend/internal/agentctl/process/workspace_tracker.go +++ b/apps/backend/internal/agentctl/process/workspace_tracker.go @@ -86,7 +86,9 @@ func (wt *WorkspaceTracker) Start(ctx context.Context) { func (wt *WorkspaceTracker) Stop() { close(wt.stopCh) if wt.watcher != nil { - wt.watcher.Close() + if err := wt.watcher.Close(); err != nil { + wt.logger.Debug("failed to close watcher", zap.Error(err)) + } } wt.wg.Wait() wt.logger.Info("workspace tracker stopped") @@ -611,7 +613,9 @@ func (wt *WorkspaceTracker) watchFilesystem(ctx context.Context) { // If a directory was created, watch it if event.Op&fsnotify.Create == fsnotify.Create { if info, err := os.Stat(event.Name); err == nil && info.IsDir() { - wt.addDirectoryRecursive(event.Name) + if err := wt.addDirectoryRecursive(event.Name); err != nil { + wt.logger.Debug("failed to watch new directory", zap.Error(err)) + } } } @@ -752,7 +756,9 @@ func (wt *WorkspaceTracker) GetFileContent(reqPath string) (string, int64, error if err != nil { return "", 0, fmt.Errorf("failed to open file: %w", err) } - defer file.Close() + defer func() { + _ = file.Close() + }() content, err := io.ReadAll(file) if err != nil { @@ -763,4 +769,3 @@ func (wt *WorkspaceTracker) GetFileContent(reqPath string) (string, int64, error } - diff --git a/apps/backend/internal/events/bus/memory.go b/apps/backend/internal/events/bus/memory.go index 395cd8576e..526286e83d 100644 --- a/apps/backend/internal/events/bus/memory.go +++ b/apps/backend/internal/events/bus/memory.go @@ -226,7 +226,9 @@ func (b *MemoryEventBus) Request(ctx context.Context, subject string, event *Eve if err != nil { return nil, fmt.Errorf("failed to create reply subscription: %w", err) } - defer sub.Unsubscribe() + defer func() { + _ = sub.Unsubscribe() + }() // Add reply subject to event data if event.Data == nil { diff --git a/apps/backend/internal/events/bus/memory_test.go b/apps/backend/internal/events/bus/memory_test.go index 4f9e5a2cbe..1d92bac7ac 100644 --- a/apps/backend/internal/events/bus/memory_test.go +++ b/apps/backend/internal/events/bus/memory_test.go @@ -49,7 +49,9 @@ func TestMemoryEventBus_PublishSubscribe(t *testing.T) { if err != nil { t.Fatalf("Subscribe failed: %v", err) } - defer sub.Unsubscribe() + defer func() { + _ = sub.Unsubscribe() + }() event := NewEvent("test.type", "test-source", map[string]interface{}{"key": "value"}) if err := bus.Publish(ctx, "test.subject", event); err != nil { @@ -86,7 +88,9 @@ func TestMemoryEventBus_MultipleSubscribers(t *testing.T) { if err != nil { t.Fatalf("Subscribe %d failed: %v", i, err) } - defer sub.Unsubscribe() + defer func() { + _ = sub.Unsubscribe() + }() } event := NewEvent("test.type", "test-source", nil) @@ -159,7 +163,9 @@ func TestMemoryEventBus_SingleTokenWildcard(t *testing.T) { if err != nil { t.Fatalf("Subscribe failed: %v", err) } - defer sub.Unsubscribe() + defer func() { + _ = sub.Unsubscribe() + }() // Should match - "user" fills the * slot event1 := NewEvent("user.created", "test", nil) @@ -201,7 +207,9 @@ func TestMemoryEventBus_MultiTokenWildcard(t *testing.T) { if err != nil { t.Fatalf("Subscribe failed: %v", err) } - defer sub.Unsubscribe() + defer func() { + _ = sub.Unsubscribe() + }() // Should match - single remaining token event1 := NewEvent("email", "test", nil) @@ -238,7 +246,9 @@ func TestMemoryEventBus_WildcardNoMatch(t *testing.T) { if err != nil { t.Fatalf("Subscribe failed: %v", err) } - defer sub.Unsubscribe() + defer func() { + _ = sub.Unsubscribe() + }() // This should NOT match - missing middle token event := NewEvent("test", "test", nil) @@ -269,7 +279,9 @@ func TestMemoryEventBus_ExactMatch(t *testing.T) { if err != nil { t.Fatalf("Subscribe failed: %v", err) } - defer sub.Unsubscribe() + defer func() { + _ = sub.Unsubscribe() + }() // Should match exactly event1 := NewEvent("test", "test", nil) @@ -312,7 +324,9 @@ func TestMemoryEventBus_QueueSubscribe(t *testing.T) { if err != nil { t.Fatalf("QueueSubscribe %d failed: %v", i, err) } - defer sub.Unsubscribe() + defer func() { + _ = sub.Unsubscribe() + }() } // Publish multiple events @@ -338,6 +352,7 @@ func TestMemoryEventBus_ConcurrentAccess(t *testing.T) { ctx := context.Background() var receivedCount int32 + var publishErrorCount int32 var wg sync.WaitGroup // Subscribe @@ -348,7 +363,9 @@ func TestMemoryEventBus_ConcurrentAccess(t *testing.T) { if err != nil { t.Fatalf("Subscribe failed: %v", err) } - defer sub.Unsubscribe() + defer func() { + _ = sub.Unsubscribe() + }() // Publish concurrently from multiple goroutines numGoroutines := 10 @@ -360,12 +377,17 @@ func TestMemoryEventBus_ConcurrentAccess(t *testing.T) { defer wg.Done() for j := 0; j < eventsPerGoroutine; j++ { event := NewEvent("test.type", "test-source", nil) - bus.Publish(ctx, "test.concurrent", event) + if err := bus.Publish(ctx, "test.concurrent", event); err != nil { + atomic.AddInt32(&publishErrorCount, 1) + } } }() } wg.Wait() + if publishErrorCount > 0 { + t.Errorf("publish errors: %d", publishErrorCount) + } time.Sleep(200 * time.Millisecond) // Allow handlers to complete expectedCount := int32(numGoroutines * eventsPerGoroutine) @@ -426,7 +448,9 @@ func TestMemoryEventBus_Request(t *testing.T) { if err != nil { t.Fatalf("Subscribe failed: %v", err) } - defer sub.Unsubscribe() + defer func() { + _ = sub.Unsubscribe() + }() // Make a request request := NewEvent("echo.request", "requester", map[string]interface{}{ @@ -484,4 +508,3 @@ func TestNewEvent(t *testing.T) { t.Error("Expected timestamp to be set correctly") } } - diff --git a/apps/backend/internal/gateway/websocket/client.go b/apps/backend/internal/gateway/websocket/client.go index 284dad2cb2..4c73a5531d 100644 --- a/apps/backend/internal/gateway/websocket/client.go +++ b/apps/backend/internal/gateway/websocket/client.go @@ -57,14 +57,17 @@ func NewClient(id string, conn *websocket.Conn, hub *Hub, log *logger.Logger) *C func (c *Client) ReadPump(ctx context.Context) { defer func() { c.hub.Unregister(c) - c.conn.Close() + if err := c.conn.Close(); err != nil { + c.logger.Debug("failed to close websocket connection", zap.Error(err)) + } }() c.conn.SetReadLimit(maxMessageSize) - c.conn.SetReadDeadline(time.Now().Add(pongWait)) + if err := c.conn.SetReadDeadline(time.Now().Add(pongWait)); err != nil { + c.logger.Debug("failed to set read deadline", zap.Error(err)) + } c.conn.SetPongHandler(func(string) error { - c.conn.SetReadDeadline(time.Now().Add(pongWait)) - return nil + return c.conn.SetReadDeadline(time.Now().Add(pongWait)) }) for { @@ -310,16 +313,22 @@ func (c *Client) WritePump() { ticker := time.NewTicker(pingPeriod) defer func() { ticker.Stop() - c.conn.Close() + if err := c.conn.Close(); err != nil { + c.logger.Debug("failed to close websocket connection", zap.Error(err)) + } }() for { select { case message, ok := <-c.send: - c.conn.SetWriteDeadline(time.Now().Add(writeWait)) + if err := c.conn.SetWriteDeadline(time.Now().Add(writeWait)); err != nil { + c.logger.Debug("failed to set write deadline", zap.Error(err)) + } if !ok { // Hub closed the channel - c.conn.WriteMessage(websocket.CloseMessage, []byte{}) + if err := c.conn.WriteMessage(websocket.CloseMessage, []byte{}); err != nil { + c.logger.Debug("failed to write close message", zap.Error(err)) + } return } @@ -327,13 +336,25 @@ func (c *Client) WritePump() { if err != nil { return } - w.Write(message) + if _, err := w.Write(message); err != nil { + c.logger.Debug("failed to write websocket message", zap.Error(err)) + _ = w.Close() + return + } // Batch additional queued messages n := len(c.send) for i := 0; i < n; i++ { - w.Write([]byte{'\n'}) - w.Write(<-c.send) + if _, err := w.Write([]byte{'\n'}); err != nil { + c.logger.Debug("failed to write websocket delimiter", zap.Error(err)) + _ = w.Close() + return + } + if _, err := w.Write(<-c.send); err != nil { + c.logger.Debug("failed to write queued websocket message", zap.Error(err)) + _ = w.Close() + return + } } if err := w.Close(); err != nil { @@ -341,7 +362,9 @@ func (c *Client) WritePump() { } case <-ticker.C: - c.conn.SetWriteDeadline(time.Now().Add(writeWait)) + if err := c.conn.SetWriteDeadline(time.Now().Add(writeWait)); err != nil { + c.logger.Debug("failed to set write deadline", zap.Error(err)) + } if err := c.conn.WriteMessage(websocket.PingMessage, nil); err != nil { return } diff --git a/apps/backend/internal/integration/integration_test.go b/apps/backend/internal/integration/integration_test.go index 43e5e4414d..a2570d8d69 100644 --- a/apps/backend/internal/integration/integration_test.go +++ b/apps/backend/internal/integration/integration_test.go @@ -61,7 +61,11 @@ func NewTestServer(t *testing.T) *TestServer { if err != nil { t.Fatalf("failed to create test repository: %v", err) } - t.Cleanup(func() { taskRepo.Close() }) + t.Cleanup(func() { + if err := taskRepo.Close(); err != nil { + t.Errorf("failed to close task repo: %v", err) + } + }) // Initialize task service taskSvc := taskservice.NewService(taskRepo, eventBus, log, taskservice.RepositoryDiscoveryConfig{}) @@ -109,7 +113,9 @@ func NewTestServer(t *testing.T) *TestServer { func (ts *TestServer) Close() { ts.cancelFunc() ts.Server.Close() - ts.TaskRepo.Close() + if err := ts.TaskRepo.Close(); err != nil { + ts.Logger.Error("failed to close task repo: " + err.Error()) + } ts.EventBus.Close() } @@ -216,7 +222,9 @@ func (c *WSClient) readPump() { // Close closes the WebSocket connection func (c *WSClient) Close() { - c.conn.Close() + if err := c.conn.Close(); err != nil { + c.t.Logf("failed to close websocket: %v", err) + } <-c.done } @@ -476,7 +484,9 @@ func TestTaskCRUD(t *testing.T) { // Debug: print error if there is one if resp.Type == ws.MessageTypeError { var errPayload ws.ErrorPayload - resp.ParsePayload(&errPayload) + if err := resp.ParsePayload(&errPayload); err != nil { + t.Fatalf("failed to parse error payload: %v", err) + } t.Logf("Error response: %+v", errPayload) } @@ -587,7 +597,9 @@ func TestTaskStateTransitions(t *testing.T) { "name": "State Test Board", }) var boardPayload map[string]interface{} - boardResp.ParsePayload(&boardPayload) + if err := boardResp.ParsePayload(&boardPayload); err != nil { + t.Fatalf("failed to parse board payload: %v", err) + } boardID := boardPayload["id"].(string) colResp, _ := client.SendRequest("col-1", ws.ActionColumnCreate, map[string]interface{}{ @@ -596,7 +608,9 @@ func TestTaskStateTransitions(t *testing.T) { "position": 0, }) var colPayload map[string]interface{} - colResp.ParsePayload(&colPayload) + if err := colResp.ParsePayload(&colPayload); err != nil { + t.Fatalf("failed to parse column payload: %v", err) + } columnID := colPayload["id"].(string) taskResp, _ := client.SendRequest("task-1", ws.ActionTaskCreate, map[string]interface{}{ @@ -609,7 +623,9 @@ func TestTaskStateTransitions(t *testing.T) { "base_branch": "main", }) var taskPayload map[string]interface{} - taskResp.ParsePayload(&taskPayload) + if err := taskResp.ParsePayload(&taskPayload); err != nil { + t.Fatalf("failed to parse task payload: %v", err) + } taskID := taskPayload["id"].(string) // Test state transitions @@ -665,7 +681,9 @@ func TestTaskMove(t *testing.T) { "name": "Move Test Board", }) var boardPayload map[string]interface{} - boardResp.ParsePayload(&boardPayload) + if err := boardResp.ParsePayload(&boardPayload); err != nil { + t.Fatalf("failed to parse board payload: %v", err) + } boardID := boardPayload["id"].(string) col1Resp, _ := client.SendRequest("col-1", ws.ActionColumnCreate, map[string]interface{}{ @@ -674,7 +692,9 @@ func TestTaskMove(t *testing.T) { "position": 0, }) var col1Payload map[string]interface{} - col1Resp.ParsePayload(&col1Payload) + if err := col1Resp.ParsePayload(&col1Payload); err != nil { + t.Fatalf("failed to parse column payload: %v", err) + } col1ID := col1Payload["id"].(string) col2Resp, _ := client.SendRequest("col-2", ws.ActionColumnCreate, map[string]interface{}{ @@ -683,7 +703,9 @@ func TestTaskMove(t *testing.T) { "position": 1, }) var col2Payload map[string]interface{} - col2Resp.ParsePayload(&col2Payload) + if err := col2Resp.ParsePayload(&col2Payload); err != nil { + t.Fatalf("failed to parse column payload: %v", err) + } col2ID := col2Payload["id"].(string) // Create task in first column @@ -696,7 +718,9 @@ func TestTaskMove(t *testing.T) { "base_branch": "main", }) var taskPayload map[string]interface{} - taskResp.ParsePayload(&taskPayload) + if err := taskResp.ParsePayload(&taskPayload); err != nil { + t.Fatalf("failed to parse task payload: %v", err) + } taskID := taskPayload["id"].(string) // Move task to second column @@ -913,7 +937,9 @@ func TestTaskSubscription(t *testing.T) { "name": "Sub Test Board", }) var boardPayload map[string]interface{} - boardResp.ParsePayload(&boardPayload) + if err := boardResp.ParsePayload(&boardPayload); err != nil { + t.Fatalf("failed to parse board payload: %v", err) + } boardID := boardPayload["id"].(string) colResp, _ := client.SendRequest("col-1", ws.ActionColumnCreate, map[string]interface{}{ @@ -922,7 +948,9 @@ func TestTaskSubscription(t *testing.T) { "position": 0, }) var colPayload map[string]interface{} - colResp.ParsePayload(&colPayload) + if err := colResp.ParsePayload(&colPayload); err != nil { + t.Fatalf("failed to parse column payload: %v", err) + } columnID := colPayload["id"].(string) taskResp, _ := client.SendRequest("task-1", ws.ActionTaskCreate, map[string]interface{}{ @@ -934,7 +962,9 @@ func TestTaskSubscription(t *testing.T) { "base_branch": "main", }) var taskPayload map[string]interface{} - taskResp.ParsePayload(&taskPayload) + if err := taskResp.ParsePayload(&taskPayload); err != nil { + t.Fatalf("failed to parse task payload: %v", err) + } taskID := taskPayload["id"].(string) // Subscribe to the task @@ -989,7 +1019,9 @@ func TestConcurrentRequests(t *testing.T) { "name": "Concurrent Test Board", }) var boardPayload map[string]interface{} - boardResp.ParsePayload(&boardPayload) + if err := boardResp.ParsePayload(&boardPayload); err != nil { + t.Fatalf("failed to parse board payload: %v", err) + } boardID := boardPayload["id"].(string) colResp, _ := client.SendRequest("col-1", ws.ActionColumnCreate, map[string]interface{}{ @@ -998,7 +1030,9 @@ func TestConcurrentRequests(t *testing.T) { "position": 0, }) var colPayload map[string]interface{} - colResp.ParsePayload(&colPayload) + if err := colResp.ParsePayload(&colPayload); err != nil { + t.Fatalf("failed to parse column payload: %v", err) + } columnID := colPayload["id"].(string) // Create multiple tasks concurrently diff --git a/apps/backend/internal/integration/orchestrator_test.go b/apps/backend/internal/integration/orchestrator_test.go index f193bb4522..0ec36c9c95 100644 --- a/apps/backend/internal/integration/orchestrator_test.go +++ b/apps/backend/internal/integration/orchestrator_test.go @@ -340,7 +340,9 @@ func (s *SimulatedAgentManagerClient) PromptAgent(ctx context.Context, agentInst event := bus.NewEvent(events.ACPMessage, "simulated-agent", msgData) subject := events.BuildACPSubject(instance.taskID) - s.eventBus.Publish(context.Background(), subject, event) + if err := s.eventBus.Publish(context.Background(), subject, event); err != nil { + s.logger.Warn("failed to publish simulated ACP message", zap.Error(err)) + } }() return &executor.PromptResult{ @@ -474,7 +476,11 @@ func NewOrchestratorTestServer(t *testing.T) *OrchestratorTestServer { if err != nil { t.Fatalf("failed to create test repository: %v", err) } - t.Cleanup(func() { taskRepo.Close() }) + t.Cleanup(func() { + if err := taskRepo.Close(); err != nil { + t.Errorf("failed to close task repo: %v", err) + } + }) // Initialize task service taskSvc := taskservice.NewService(taskRepo, eventBus, log, taskservice.RepositoryDiscoveryConfig{}) @@ -554,11 +560,15 @@ func NewOrchestratorTestServer(t *testing.T) *OrchestratorTestServer { // Close shuts down the test server func (ts *OrchestratorTestServer) Close() { - ts.OrchestratorSvc.Stop() + if err := ts.OrchestratorSvc.Stop(); err != nil { + ts.Logger.Warn("failed to stop orchestrator", zap.Error(err)) + } ts.AgentManager.Close() ts.cancelFunc() ts.Server.Close() - ts.TaskRepo.Close() + if err := ts.TaskRepo.Close(); err != nil { + ts.Logger.Warn("failed to close task repo", zap.Error(err)) + } ts.EventBus.Close() } @@ -657,8 +667,7 @@ func createOrchestratorWorkspace(t *testing.T, client *OrchestratorWSClient) str require.NoError(t, err) var payload map[string]interface{} - err = resp.ParsePayload(&payload) - require.NoError(t, err) + require.NoError(t, resp.ParsePayload(&payload)) return payload["id"].(string) } @@ -693,7 +702,9 @@ func (c *OrchestratorWSClient) readPump() { // Close closes the WebSocket connection func (c *OrchestratorWSClient) Close() { - c.conn.Close() + if err := c.conn.Close(); err != nil { + c.t.Logf("failed to close websocket: %v", err) + } <-c.done } @@ -1020,7 +1031,10 @@ func TestOrchestratorConcurrentTasks(t *testing.T) { } var payload map[string]interface{} - resp.ParsePayload(&payload) + if err := resp.ParsePayload(&payload); err != nil { + t.Logf("failed to parse start response: %v", err) + return + } results <- payload }(i, taskID) } diff --git a/apps/backend/internal/orchestrator/scheduler/scheduler_test.go b/apps/backend/internal/orchestrator/scheduler/scheduler_test.go index b8ade5b7cc..bb397e6b9c 100644 --- a/apps/backend/internal/orchestrator/scheduler/scheduler_test.go +++ b/apps/backend/internal/orchestrator/scheduler/scheduler_test.go @@ -126,7 +126,11 @@ func createTestExecutor(t *testing.T, agentMgr *mockAgentManager, log *logger.Lo if err != nil { t.Fatalf("failed to create test repository: %v", err) } - t.Cleanup(func() { repo.Close() }) + t.Cleanup(func() { + if err := repo.Close(); err != nil { + t.Errorf("failed to close repo: %v", err) + } + }) cfg := executor.ExecutorConfig{ MaxConcurrent: maxConcurrent, WorktreeEnabled: true, @@ -215,7 +219,9 @@ func TestStartAlreadyRunning(t *testing.T) { s := NewScheduler(q, exec, taskRepo, log, DefaultSchedulerConfig()) _ = s.Start(context.Background()) - defer s.Stop() + defer func() { + _ = s.Stop() + }() err := s.Start(context.Background()) if err != ErrSchedulerAlreadyRunning { diff --git a/apps/backend/internal/orchestrator/service.go b/apps/backend/internal/orchestrator/service.go index 3bc9f02d06..14a3912c22 100644 --- a/apps/backend/internal/orchestrator/service.go +++ b/apps/backend/internal/orchestrator/service.go @@ -204,7 +204,9 @@ func (s *Service) Start(ctx context.Context) error { // Start the scheduler processing loop if err := s.scheduler.Start(ctx); err != nil { - s.watcher.Stop() + if stopErr := s.watcher.Stop(); stopErr != nil { + s.logger.Warn("failed to stop watcher after scheduler start failure", zap.Error(stopErr)) + } s.mu.Lock() s.running = false s.mu.Unlock() @@ -640,7 +642,7 @@ func buildACPMetadata(msg *protocol.Message) map[string]interface{} { "provider_type": string(msg.Type), "provider_agent": msg.AgentID, } - if msg.Data != nil && len(msg.Data) > 0 { + if len(msg.Data) > 0 { metadata["provider_data"] = msg.Data } return metadata diff --git a/apps/backend/internal/orchestrator/watcher/watcher_test.go b/apps/backend/internal/orchestrator/watcher/watcher_test.go index bea6f2af76..a7628419d2 100644 --- a/apps/backend/internal/orchestrator/watcher/watcher_test.go +++ b/apps/backend/internal/orchestrator/watcher/watcher_test.go @@ -59,7 +59,9 @@ func (b *mockEventBus) Publish(ctx context.Context, subject string, event *bus.E // Call handlers for matching subjects if handlers, ok := b.subscriptions[subject]; ok { for _, handler := range handlers { - go handler(ctx, event) + go func(handler bus.EventHandler) { + _ = handler(ctx, event) + }(handler) } } @@ -67,7 +69,9 @@ func (b *mockEventBus) Publish(ctx context.Context, subject string, event *bus.E for _, queueGroup := range b.queuedSubs { if handlers, ok := queueGroup[subject]; ok && len(handlers) > 0 { // Deliver to first handler (simplified) - go handlers[0](ctx, event) + go func(handler bus.EventHandler) { + _ = handler(ctx, event) + }(handlers[0]) } } @@ -217,7 +221,9 @@ func TestTaskEventHandling(t *testing.T) { w := NewWatcher(eventBus, handlers, log) _ = w.Start(context.Background()) - defer w.Stop() + defer func() { + _ = w.Stop() + }() // Simulate publishing a task state changed event oldState := v1.TaskStateTODO @@ -262,7 +268,9 @@ func TestAgentEventHandling(t *testing.T) { w := NewWatcher(eventBus, handlers, log) _ = w.Start(context.Background()) - defer w.Stop() + defer func() { + _ = w.Stop() + }() // Simulate publishing an agent completed event exitCode := 0 @@ -312,7 +320,9 @@ func TestACPMessageHandling(t *testing.T) { w := NewWatcher(eventBus, handlers, log) _ = w.Start(context.Background()) - defer w.Stop() + defer func() { + _ = w.Stop() + }() // Simulate publishing an ACP message event := bus.NewEvent(events.BuildACPSubject("task-789"), "test", map[string]interface{}{ @@ -407,4 +417,3 @@ func TestIsRunning(t *testing.T) { t.Error("watcher should not be running after Stop") } } - diff --git a/apps/backend/internal/task/repository/memory.go b/apps/backend/internal/task/repository/memory.go index 98bf01ed32..3de956a8fc 100644 --- a/apps/backend/internal/task/repository/memory.go +++ b/apps/backend/internal/task/repository/memory.go @@ -626,15 +626,6 @@ func isSessionActive(status models.AgentSessionState) bool { status == models.AgentSessionStateStarting } -func (r *MemoryRepository) getTaskPlacement(taskID, boardID string) (*taskPlacement, bool) { - placements, ok := r.taskPlacements[taskID] - if !ok { - return nil, false - } - placement, ok := placements[boardID] - return placement, ok -} - // Message operations // CreateMessage creates a new message diff --git a/apps/backend/internal/task/repository/sqlite.go b/apps/backend/internal/task/repository/sqlite.go index db61f2f7bd..e69f38c8bb 100644 --- a/apps/backend/internal/task/repository/sqlite.go +++ b/apps/backend/internal/task/repository/sqlite.go @@ -48,7 +48,9 @@ func NewSQLiteRepository(dbPath string) (*SQLiteRepository, error) { // Initialize schema if err := repo.initSchema(); err != nil { - db.Close() + if closeErr := db.Close(); closeErr != nil { + return nil, fmt.Errorf("failed to close database after schema error: %w", closeErr) + } return nil, fmt.Errorf("failed to initialize schema: %w", err) } @@ -342,7 +344,7 @@ func (r *SQLiteRepository) columnExists(table, column string) (bool, error) { if err != nil { return false, err } - defer rows.Close() + defer func() { _ = rows.Close() }() for rows.Next() { var cid int @@ -589,26 +591,6 @@ func (r *SQLiteRepository) ensureWorkspaceIndexes() error { return nil } -func (r *SQLiteRepository) backfillTaskMappings() error { - ctx := context.Background() - - _, err := r.db.ExecContext(ctx, ` - INSERT OR IGNORE INTO task_boards (task_id, board_id) - SELECT id, board_id FROM tasks WHERE board_id != '' - `) - if err != nil { - return err - } - - _, err = r.db.ExecContext(ctx, ` - INSERT OR IGNORE INTO task_columns (task_id, board_id, column_id, position) - SELECT id, board_id, column_id, position - FROM tasks - WHERE board_id != '' AND column_id != '' - `) - return err -} - // Close closes the database connection func (r *SQLiteRepository) Close() error { return r.db.Close() @@ -733,7 +715,7 @@ func (r *SQLiteRepository) ListWorkspaces(ctx context.Context) ([]*models.Worksp if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var result []*models.Workspace for rows.Next() { @@ -794,7 +776,9 @@ func (r *SQLiteRepository) CreateTask(ctx context.Context, task *models.Task) er VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `, task.ID, task.WorkspaceID, task.BoardID, task.ColumnID, task.Title, task.Description, task.State, task.Priority, task.RepositoryID, task.BaseBranch, task.AssignedTo, task.Position, string(metadata), task.CreatedAt, task.UpdatedAt) if err != nil { - tx.Rollback() + if rollbackErr := tx.Rollback(); rollbackErr != nil { + return fmt.Errorf("failed to rollback task insert: %w", rollbackErr) + } return err } @@ -875,7 +859,7 @@ func (r *SQLiteRepository) ListTasks(ctx context.Context, boardID string) ([]*mo if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() return r.scanTasks(rows) } @@ -890,7 +874,7 @@ func (r *SQLiteRepository) ListTasksByColumn(ctx context.Context, columnID strin if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() return r.scanTasks(rows) } @@ -960,13 +944,6 @@ func (r *SQLiteRepository) RemoveTaskFromBoard(ctx context.Context, taskID, boar return err } -func (r *SQLiteRepository) addTaskToBoardTx(ctx context.Context, tx *sql.Tx, taskID, boardID, columnID string, position int) error { - _, err := tx.ExecContext(ctx, ` - UPDATE tasks SET board_id = ?, column_id = ?, position = ?, updated_at = ? WHERE id = ? - `, boardID, columnID, position, time.Now().UTC(), taskID) - return err -} - // Board operations // CreateBoard creates a new board @@ -1049,7 +1026,7 @@ func (r *SQLiteRepository) ListBoards(ctx context.Context, workspaceID string) ( if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var result []*models.Board for rows.Next() { @@ -1153,7 +1130,7 @@ func (r *SQLiteRepository) ListColumns(ctx context.Context, boardID string) ([]* if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var result []*models.Column for rows.Next() { @@ -1242,7 +1219,7 @@ func (r *SQLiteRepository) ListMessages(ctx context.Context, sessionID string) ( if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var result []*models.Message for rows.Next() { @@ -1327,7 +1304,7 @@ func (r *SQLiteRepository) ListMessagesPaginated(ctx context.Context, sessionID if err != nil { return nil, false, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var result []*models.Message for rows.Next() { @@ -1516,7 +1493,7 @@ func (r *SQLiteRepository) ListRepositories(ctx context.Context, workspaceID str if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var result []*models.Repository for rows.Next() { @@ -1604,7 +1581,7 @@ func (r *SQLiteRepository) ListRepositoryScripts(ctx context.Context, repository if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var result []*models.RepositoryScript for rows.Next() { @@ -1949,7 +1926,7 @@ func (r *SQLiteRepository) ListAgentSessions(ctx context.Context, taskID string) if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() return r.scanAgentSessions(rows) } @@ -1966,7 +1943,7 @@ func (r *SQLiteRepository) ListActiveAgentSessions(ctx context.Context) ([]*mode if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() return r.scanAgentSessions(rows) } @@ -2194,7 +2171,7 @@ func (r *SQLiteRepository) ListExecutors(ctx context.Context) ([]*models.Executo if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var result []*models.Executor for rows.Next() { @@ -2313,7 +2290,7 @@ func (r *SQLiteRepository) ListEnvironments(ctx context.Context) ([]*models.Envi if err != nil { return nil, err } - defer rows.Close() + defer func() { _ = rows.Close() }() var result []*models.Environment for rows.Next() { diff --git a/apps/backend/internal/task/repository/sqlite_test.go b/apps/backend/internal/task/repository/sqlite_test.go index fba67d16df..8bde2714d4 100644 --- a/apps/backend/internal/task/repository/sqlite_test.go +++ b/apps/backend/internal/task/repository/sqlite_test.go @@ -22,8 +22,12 @@ func createTestSQLiteRepo(t *testing.T) (*SQLiteRepository, func()) { } cleanup := func() { - repo.Close() - os.Remove(dbPath) + if err := repo.Close(); err != nil { + t.Errorf("failed to close repo: %v", err) + } + if err := os.Remove(dbPath); err != nil && !os.IsNotExist(err) { + t.Errorf("failed to remove db path: %v", err) + } } return repo, cleanup @@ -475,14 +479,20 @@ func TestSQLiteRepository_Persistence(t *testing.T) { _ = repo1.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace"}) board := &models.Board{ID: "persist-board", WorkspaceID: "ws-1", Name: "Persistent Board"} _ = repo1.CreateBoard(ctx, board) - repo1.Close() + if err := repo1.Close(); err != nil { + t.Fatalf("failed to close repo: %v", err) + } // Reopen repository and verify data persisted repo2, err := NewSQLiteRepository(dbPath) if err != nil { t.Fatalf("failed to create second repository: %v", err) } - defer repo2.Close() + defer func() { + if err := repo2.Close(); err != nil { + t.Errorf("failed to close repo: %v", err) + } + }() retrieved, err := repo2.GetBoard(ctx, "persist-board") if err != nil { diff --git a/apps/backend/internal/task/service/repository_discovery_test.go b/apps/backend/internal/task/service/repository_discovery_test.go index 9e88e993db..25e527cfbf 100644 --- a/apps/backend/internal/task/service/repository_discovery_test.go +++ b/apps/backend/internal/task/service/repository_discovery_test.go @@ -277,7 +277,11 @@ func newDiscoveryService(t *testing.T, root string) *Service { if err != nil { t.Fatalf("failed to create test repository: %v", err) } - t.Cleanup(func() { repo.Close() }) + t.Cleanup(func() { + if err := repo.Close(); err != nil { + t.Errorf("failed to close repo: %v", err) + } + }) log, _ := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json", OutputPath: "stdout"}) eventBus := bus.NewMemoryEventBus(log) return NewService(repo, eventBus, log, RepositoryDiscoveryConfig{ diff --git a/apps/backend/internal/task/service/service_test.go b/apps/backend/internal/task/service/service_test.go index 6f0ef28957..a037cd0cc2 100644 --- a/apps/backend/internal/task/service/service_test.go +++ b/apps/backend/internal/task/service/service_test.go @@ -74,7 +74,11 @@ func createTestService(t *testing.T) (*Service, *MockEventBus, repository.Reposi if err != nil { t.Fatalf("failed to create test repository: %v", err) } - t.Cleanup(func() { repo.Close() }) + t.Cleanup(func() { + if err := repo.Close(); err != nil { + t.Errorf("failed to close repo: %v", err) + } + }) eventBus := NewMockEventBus() log, _ := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json", OutputPath: "stdout"}) svc := NewService(repo, eventBus, log, RepositoryDiscoveryConfig{}) diff --git a/apps/backend/internal/user/store/sqlite.go b/apps/backend/internal/user/store/sqlite.go index be7114d677..7bf323f603 100644 --- a/apps/backend/internal/user/store/sqlite.go +++ b/apps/backend/internal/user/store/sqlite.go @@ -42,7 +42,9 @@ func NewSQLiteRepository(dbPath string) (*SQLiteRepository, error) { repo := &SQLiteRepository{db: db} if err := repo.initSchema(); err != nil { - db.Close() + if closeErr := db.Close(); closeErr != nil { + return nil, fmt.Errorf("failed to close database after schema error: %w", closeErr) + } return nil, fmt.Errorf("failed to initialize schema: %w", err) } return repo, nil @@ -112,7 +114,9 @@ func (r *SQLiteRepository) columnExists(table, column string) (bool, error) { if err != nil { return false, err } - defer rows.Close() + defer func() { + _ = rows.Close() + }() for rows.Next() { var cid int diff --git a/apps/backend/pkg/codex/client.go b/apps/backend/pkg/codex/client.go index 076dabc280..a80e60f138 100644 --- a/apps/backend/pkg/codex/client.go +++ b/apps/backend/pkg/codex/client.go @@ -228,7 +228,8 @@ func (c *Client) handleRequest(id interface{}, method string, params json.RawMes c.onRequest(id, method, params) } else { c.logger.Warn("received request but no handler registered", zap.String("method", method)) - c.SendResponse(id, nil, &Error{Code: MethodNotFound, Message: "Method not found"}) + if err := c.SendResponse(id, nil, &Error{Code: MethodNotFound, Message: "Method not found"}); err != nil { + c.logger.Warn("failed to send method not found response", zap.Error(err)) + } } } -