From b62d46e50ee3d98157ea64721adc8a09ac58e9bc Mon Sep 17 00:00:00 2001 From: viettranx Date: Sat, 7 Mar 2026 20:51:39 +0700 Subject: [PATCH] refactor(lint): apply Go best practices across codebase - Use errors.Is() instead of direct sentinel comparison (13 instances) - Convert if/else-if chains to switch/case for same-variable comparisons - Remove redundant bitwise OR with zero - Add post-implementation checklist to CLAUDE.md --- CLAUDE.md | 16 ++++++++++++++++ cmd/gateway_providers.go | 11 ++++++----- cmd/gateway_tsnet.go | 3 ++- cmd/migrate.go | 7 ++++--- cmd/upgrade.go | 4 ++-- internal/channels/feishu/feishu.go | 3 ++- internal/channels/feishu/larkws_proto.go | 5 +++-- internal/gateway/server.go | 3 ++- internal/http/providers.go | 11 ++++++----- internal/oauth/openai.go | 3 ++- internal/store/pg/agents_context.go | 3 ++- internal/store/pg/teams.go | 5 +++-- internal/tools/create_image.go | 7 ++++--- internal/tools/shell.go | 3 ++- 14 files changed, 56 insertions(+), 28 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 0f8e427b..efb3da4a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -61,3 +61,19 @@ go test -v ./tests/integration/ # Integration tests cd ui/web && pnpm install && pnpm dev # Web dashboard (dev) ``` + +## Post-Implementation Checklist + +After implementing or modifying Go code, run these checks: + +```bash +go build ./... # Compile check +go vet ./... # Static analysis +go test -race ./tests/integration/ # Integration tests with race detector +``` + +Go conventions to follow: +- Use `errors.Is(err, sentinel)` instead of `err == sentinel` +- Use `switch/case` instead of `if/else if` chains on the same variable +- Use `append(dst, src...)` instead of loop-based append +- Always handle errors; don't ignore return values diff --git a/cmd/gateway_providers.go b/cmd/gateway_providers.go index c5a41e19..c81fee76 100644 --- a/cmd/gateway_providers.go +++ b/cmd/gateway_providers.go @@ -170,21 +170,22 @@ func registerProvidersFromDB(registry *providers.Registry, provStore store.Provi if p.APIKey == "" { continue } - if p.ProviderType == store.ProviderChatGPTOAuth { + switch p.ProviderType { + case store.ProviderChatGPTOAuth: ts := oauth.NewDBTokenSource(provStore, secretStore, p.Name) registry.Register(providers.NewCodexProvider(p.Name, ts, p.APIBase, "")) - } else if p.ProviderType == store.ProviderAnthropicNative { + case store.ProviderAnthropicNative: registry.Register(providers.NewAnthropicProvider(p.APIKey, providers.WithAnthropicBaseURL(p.APIBase))) - } else if p.ProviderType == store.ProviderDashScope { + case store.ProviderDashScope: registry.Register(providers.NewDashScopeProvider(p.APIKey, p.APIBase, "")) - } else if p.ProviderType == store.ProviderBailian { + case store.ProviderBailian: base := p.APIBase if base == "" { base = "https://coding-intl.dashscope.aliyuncs.com/v1" } registry.Register(providers.NewOpenAIProvider(p.Name, p.APIKey, base, "qwen3.5-plus")) - } else { + default: prov := providers.NewOpenAIProvider(p.Name, p.APIKey, p.APIBase, "") if p.ProviderType == store.ProviderMiniMax { prov.WithChatPath("/text/chatcompletion_v2") diff --git a/cmd/gateway_tsnet.go b/cmd/gateway_tsnet.go index cffab73c..03a6fc6f 100644 --- a/cmd/gateway_tsnet.go +++ b/cmd/gateway_tsnet.go @@ -4,6 +4,7 @@ package cmd import ( "context" + "errors" "log/slog" "net" "net/http" @@ -59,7 +60,7 @@ func initTailscale(ctx context.Context, cfg *config.Config, mux http.Handler) fu httpSrv := &http.Server{Handler: mux} go func() { - if err := httpSrv.Serve(ln); err != nil && err != http.ErrServerClosed { + if err := httpSrv.Serve(ln); err != nil && !errors.Is(err, http.ErrServerClosed) { slog.Warn("Tailscale HTTP server error", "error", err) } }() diff --git a/cmd/migrate.go b/cmd/migrate.go index cffb47ec..c691f98a 100644 --- a/cmd/migrate.go +++ b/cmd/migrate.go @@ -3,6 +3,7 @@ package cmd import ( "context" "database/sql" + "errors" "fmt" "log/slog" "os" @@ -93,7 +94,7 @@ func migrateUpCmd() *cobra.Command { } defer m.Close() - if err := m.Up(); err != nil && err != migrate.ErrNoChange { + if err := m.Up(); err != nil && !errors.Is(err, migrate.ErrNoChange) { return fmt.Errorf("migrate up: %w", err) } @@ -139,7 +140,7 @@ func migrateDownCmd() *cobra.Command { if steps <= 0 { steps = 1 } - if err := m.Steps(-steps); err != nil && err != migrate.ErrNoChange { + if err := m.Steps(-steps); err != nil && !errors.Is(err, migrate.ErrNoChange) { return fmt.Errorf("migrate down: %w", err) } @@ -226,7 +227,7 @@ func migrateGotoCmd() *cobra.Command { } defer m.Close() - if err := m.Migrate(uint(version)); err != nil && err != migrate.ErrNoChange { + if err := m.Migrate(uint(version)); err != nil && !errors.Is(err, migrate.ErrNoChange) { return fmt.Errorf("migrate goto: %w", err) } slog.Info("migrated to version", "version", version) diff --git a/cmd/upgrade.go b/cmd/upgrade.go index 972623fc..2109a7d4 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -168,7 +168,7 @@ func runUpgrade(dryRun bool) error { } defer m.Close() - if err := m.Up(); err != nil && err != migrate.ErrNoChange { + if err := m.Up(); err != nil && !errors.Is(err, migrate.ErrNoChange) { fmt.Println("FAILED") return fmt.Errorf("migrate up: %w", err) } @@ -240,7 +240,7 @@ func checkSchemaOrAutoUpgrade(dsn string) error { } defer m.Close() - if mErr := m.Up(); mErr != nil && mErr != migrate.ErrNoChange { + if mErr := m.Up(); mErr != nil && !errors.Is(mErr, migrate.ErrNoChange) { return fmt.Errorf("auto-upgrade: migrate up: %w", mErr) } diff --git a/internal/channels/feishu/feishu.go b/internal/channels/feishu/feishu.go index 1a701872..e7d49e7b 100644 --- a/internal/channels/feishu/feishu.go +++ b/internal/channels/feishu/feishu.go @@ -6,6 +6,7 @@ package feishu import ( "context" "encoding/json" + "errors" "fmt" "log/slog" "net/http" @@ -278,7 +279,7 @@ func (c *Channel) startWebhook(ctx context.Context) error { } go func() { - if err := c.httpServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { + if err := c.httpServer.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { slog.Error("feishu webhook server error", "error", err) } }() diff --git a/internal/channels/feishu/larkws_proto.go b/internal/channels/feishu/larkws_proto.go index 455bbad1..f60ece89 100644 --- a/internal/channels/feishu/larkws_proto.go +++ b/internal/channels/feishu/larkws_proto.go @@ -3,6 +3,7 @@ package feishu import ( "bytes" "encoding/binary" + "errors" "fmt" "io" ) @@ -83,7 +84,7 @@ func unmarshalFrame(data []byte) (*wsFrame, error) { for r.Len() > 0 { tag, err := binary.ReadUvarint(r) if err != nil { - if err == io.EOF { + if errors.Is(err, io.EOF) { break } return nil, fmt.Errorf("read tag: %w", err) @@ -181,7 +182,7 @@ func unmarshalHeader(data []byte) (wsHeader, error) { // --- Protobuf encoding helpers --- func pbWriteVarintField(w *bytes.Buffer, fieldNum int, val uint64) { - tag := uint64(fieldNum<<3 | 0) // wire type 0 = varint + tag := uint64(fieldNum << 3) // wire type 0 = varint pbWriteUvarint(w, tag) pbWriteUvarint(w, val) } diff --git a/internal/gateway/server.go b/internal/gateway/server.go index 6e53c2ef..d169a09f 100644 --- a/internal/gateway/server.go +++ b/internal/gateway/server.go @@ -2,6 +2,7 @@ package gateway import ( "context" + "errors" "fmt" "log/slog" "net" @@ -247,7 +248,7 @@ func (s *Server) Start(ctx context.Context) error { s.httpServer.Shutdown(shutdownCtx) }() - if err := s.httpServer.ListenAndServe(); err != http.ErrServerClosed { + if err := s.httpServer.ListenAndServe(); !errors.Is(err, http.ErrServerClosed) { return fmt.Errorf("gateway server: %w", err) } return nil diff --git a/internal/http/providers.go b/internal/http/providers.go index d07aedf4..f8ad9de9 100644 --- a/internal/http/providers.go +++ b/internal/http/providers.go @@ -94,21 +94,22 @@ func (h *ProvidersHandler) registerInMemory(p *store.LLMProviderData) { if p.APIKey == "" { return } - if p.ProviderType == store.ProviderChatGPTOAuth { + switch p.ProviderType { + case store.ProviderChatGPTOAuth: ts := oauth.NewDBTokenSource(h.store, h.secretStore, p.Name) h.providerReg.Register(providers.NewCodexProvider(p.Name, ts, p.APIBase, "")) - } else if p.ProviderType == store.ProviderAnthropicNative { + case store.ProviderAnthropicNative: h.providerReg.Register(providers.NewAnthropicProvider(p.APIKey, providers.WithAnthropicBaseURL(p.APIBase))) - } else if p.ProviderType == store.ProviderDashScope { + case store.ProviderDashScope: h.providerReg.Register(providers.NewDashScopeProvider(p.APIKey, p.APIBase, "")) - } else if p.ProviderType == store.ProviderBailian { + case store.ProviderBailian: base := p.APIBase if base == "" { base = "https://coding-intl.dashscope.aliyuncs.com/v1" } h.providerReg.Register(providers.NewOpenAIProvider(p.Name, p.APIKey, base, "qwen3.5-plus")) - } else { + default: prov := providers.NewOpenAIProvider(p.Name, p.APIKey, p.APIBase, "") if p.ProviderType == store.ProviderMiniMax { prov.WithChatPath("/text/chatcompletion_v2") diff --git a/internal/oauth/openai.go b/internal/oauth/openai.go index a97db74c..120924c3 100644 --- a/internal/oauth/openai.go +++ b/internal/oauth/openai.go @@ -4,6 +4,7 @@ package oauth import ( "context" "crypto/rand" + "errors" "crypto/sha256" "encoding/base64" "encoding/json" @@ -182,7 +183,7 @@ func StartLoginOpenAI() (*PendingLogin, error) { srv := &http.Server{Handler: mux} go func() { - if err := srv.Serve(listener); err != nil && err != http.ErrServerClosed { + if err := srv.Serve(listener); err != nil && !errors.Is(err, http.ErrServerClosed) { errCh <- fmt.Errorf("callback server: %w", err) } }() diff --git a/internal/store/pg/agents_context.go b/internal/store/pg/agents_context.go index f9d5e0ee..7ee00048 100644 --- a/internal/store/pg/agents_context.go +++ b/internal/store/pg/agents_context.go @@ -3,6 +3,7 @@ package pg import ( "context" "database/sql" + "errors" "path/filepath" "time" @@ -118,7 +119,7 @@ func (s *PGAgentStore) GetUserOverride(ctx context.Context, agentID uuid.UUID, u agentID, userID, ).Scan(&d.AgentID, &d.UserID, &d.Provider, &d.Model) if err != nil { - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return nil, nil // not found = no override } return nil, nil diff --git a/internal/store/pg/teams.go b/internal/store/pg/teams.go index ee98c9e8..9b4ff042 100644 --- a/internal/store/pg/teams.go +++ b/internal/store/pg/teams.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "encoding/json" + "errors" "time" "github.com/google/uuid" @@ -156,7 +157,7 @@ func (s *PGTeamStore) GetTeamForAgent(ctx context.Context, agentID uuid.UUID) (* LIMIT 1`, agentID, store.TeamStatusActive) d, err := scanTeamRow(row) - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return nil, nil } return d, err @@ -220,7 +221,7 @@ func (s *PGTeamStore) GetHandoffRoute(ctx context.Context, channel, chatID strin &d.ID, &d.Channel, &d.ChatID, &d.FromAgentKey, &d.ToAgentKey, &d.Reason, &d.CreatedBy, &d.CreatedAt, ) - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return nil, nil } if err != nil { diff --git a/internal/tools/create_image.go b/internal/tools/create_image.go index 3ca8c520..96467d13 100644 --- a/internal/tools/create_image.go +++ b/internal/tools/create_image.go @@ -98,15 +98,16 @@ func (t *CreateImageTool) Execute(ctx context.Context, args map[string]interface // - others (openai, etc.): /images/generations var imageBytes []byte var usage *providers.Usage - if providerName == "gemini" { + switch providerName { + case "gemini": var genErr error imageBytes, usage, genErr = t.callGeminiNativeImageGen(ctx, cp.APIKey(), cp.APIBase(), model, prompt) err = genErr - } else if providerName == "openrouter" { + case "openrouter": var genErr error imageBytes, usage, genErr = t.callImageGenAPI(ctx, cp.APIKey(), cp.APIBase(), model, prompt, aspectRatio) err = genErr - } else { + default: var genErr error imageBytes, usage, genErr = t.callStandardImageGenAPI(ctx, cp.APIKey(), cp.APIBase(), model, prompt) err = genErr diff --git a/internal/tools/shell.go b/internal/tools/shell.go index e97d881d..a1cdc1d1 100644 --- a/internal/tools/shell.go +++ b/internal/tools/shell.go @@ -3,6 +3,7 @@ package tools import ( "bytes" "context" + "errors" "fmt" "log/slog" "os/exec" @@ -297,7 +298,7 @@ func (t *ExecTool) executeOnHost(ctx context.Context, command, cwd string) *Resu func (t *ExecTool) executeInSandbox(ctx context.Context, command, cwd, sandboxKey string) *Result { sb, err := t.sandboxMgr.Get(ctx, sandboxKey, t.workingDir) if err != nil { - if err == sandbox.ErrSandboxDisabled { + if errors.Is(err, sandbox.ErrSandboxDisabled) { return t.executeOnHost(ctx, command, cwd) } // Docker unavailable (binary missing, daemon down) → fallback to host