From 462184dff3eef32f01f06634305da1355ac1bec2 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Mon, 23 Mar 2026 09:04:17 +0200 Subject: chore: bump version to v0.25.9 Code quality fixes from audit: - Log silently discarded errors in status sinks and stats.Update call sites - Fix json.Marshal errors silently ignored in LSP handlers - Replace time.Sleep in tests with channel signaling (mcp) and fake clock (stats) - Make context cancellation work in production time.Sleep sites (handlers_document, cmdentry) - Remove init()-based provider registration from llm package; use explicit RegisterAllProviders() - Add WaitGroup goroutine tracking to MCP server Run() Co-Authored-By: Claude Sonnet 4.6 --- internal/lsp/handlers.go | 6 ++++-- internal/lsp/handlers_completion.go | 22 ++++++++++++++++------ internal/lsp/handlers_document.go | 22 +++++++++++++++------- internal/lsp/handlers_utils.go | 6 ++++-- internal/lsp/server.go | 8 ++++++-- 5 files changed, 45 insertions(+), 19 deletions(-) (limited to 'internal/lsp') diff --git a/internal/lsp/handlers.go b/internal/lsp/handlers.go index 0f98715..b06b5ea 100644 --- a/internal/lsp/handlers.go +++ b/internal/lsp/handlers.go @@ -158,8 +158,10 @@ func (s *Server) checkTriggerFromContext(p CompletionParams, current string, ope logging.Logf("lsp ", "handleCompletion: unmarshal raw context: %v", err) } } else { - b, _ := json.Marshal(p.Context) - if err := json.Unmarshal(b, &ctx); err != nil { + b, err := json.Marshal(p.Context) + if err != nil { + logging.Logf("lsp ", "handleCompletion: marshal context: %v", err) + } else if err := json.Unmarshal(b, &ctx); err != nil { logging.Logf("lsp ", "handleCompletion: unmarshal context: %v", err) } } diff --git a/internal/lsp/handlers_completion.go b/internal/lsp/handlers_completion.go index d6529de..e6d8951 100644 --- a/internal/lsp/handlers_completion.go +++ b/internal/lsp/handlers_completion.go @@ -93,8 +93,10 @@ func extractTriggerInfo(p CompletionParams) (kind int, ch string) { logging.Logf("lsp ", "extractTriggerInfo: unmarshal raw context: %v", err) } } else { - b, _ := json.Marshal(p.Context) - if err := json.Unmarshal(b, &ctx); err != nil { + b, err := json.Marshal(p.Context) + if err != nil { + logging.Logf("lsp ", "extractTriggerInfo: marshal context: %v", err) + } else if err := json.Unmarshal(b, &ctx); err != nil { logging.Logf("lsp ", "extractTriggerInfo: unmarshal context: %v", err) } } @@ -325,7 +327,10 @@ func (s *Server) executeChatCompletion(ctx context.Context, plan completionPlan, } s.incRecvCounters(len(text)) modelUsed := spec.effectiveModel(client.DefaultModel()) - _ = stats.Update(ctx, client.Name(), modelUsed, sentSize, len(text)) + // Update global stats cache; log but don't fail on stats errors + if err := stats.Update(ctx, client.Name(), modelUsed, sentSize, len(text)); err != nil { + logging.Logf("lsp ", "stats update error: %v", err) + } s.logLLMStats(modelUsed) trimmed := strings.TrimSpace(text) cursorByte := utf16OffsetToByteOffset(plan.current, plan.params.Position.Character) @@ -357,8 +362,10 @@ func parseManualInvoke(ctx any) bool { logging.Logf("lsp ", "parseManualInvoke: unmarshal raw context: %v", err) } } else { - b, _ := json.Marshal(ctx) - if err := json.Unmarshal(b, &c); err != nil { + b, err := json.Marshal(ctx) + if err != nil { + logging.Logf("lsp ", "parseManualInvoke: marshal context: %v", err) + } else if err := json.Unmarshal(b, &c); err != nil { logging.Logf("lsp ", "parseManualInvoke: unmarshal context: %v", err) } } @@ -501,7 +508,10 @@ func (s *Server) tryProviderNativeCompletion(ctx context.Context, plan completio } s.incSentCounters(sentBytes) s.incRecvCounters(len(suggestions[0])) - _ = stats.Update(ctx2, client.Name(), modelUsed, sentBytes, len(suggestions[0])) + // Update global stats cache; log but don't fail on stats errors + if err := stats.Update(ctx2, client.Name(), modelUsed, sentBytes, len(suggestions[0])); err != nil { + logging.Logf("lsp ", "stats update error: %v", err) + } s.logLLMStats(modelUsed) cleaned := s.postProcessNativeCompletion(suggestions[0], current, p.Position.Character) if cleaned == "" { diff --git a/internal/lsp/handlers_document.go b/internal/lsp/handlers_document.go index f69c626..7c59aa8 100644 --- a/internal/lsp/handlers_document.go +++ b/internal/lsp/handlers_document.go @@ -2,6 +2,7 @@ package lsp import ( + "context" "encoding/json" "strings" "time" @@ -400,9 +401,13 @@ func (s *Server) buildChatMessages(uri string, pos Position, prompt string) []ll // clientApplyEdit sends a workspace/applyEdit request to the client. func (s *Server) clientApplyEdit(label string, edit WorkspaceEdit) { params := ApplyWorkspaceEditParams{Label: label, Edit: edit} + b, err := json.Marshal(params) + if err != nil { + logging.Logf("lsp ", "clientApplyEdit: marshal error: %v", err) + return + } id := s.nextReqID() req := Request{JSONRPC: "2.0", ID: id, Method: "workspace/applyEdit"} - b, _ := json.Marshal(params) req.Params = b s.writeMessage(req) } @@ -428,9 +433,13 @@ func (s *Server) clientShowDocument(uri string, sel *Range) { params.URI = uri params.TakeFocus = true params.Selection = sel + b, err := json.Marshal(params) + if err != nil { + logging.Logf("lsp ", "clientShowDocument: marshal error: %v", err) + return + } id := s.nextReqID() req := Request{JSONRPC: "2.0", ID: id, Method: "window/showDocument"} - b, _ := json.Marshal(params) req.Params = b s.writeMessage(req) } @@ -440,14 +449,13 @@ func (s *Server) clientShowDocument(uri string, sel *Range) { // The goroutine respects s.serverCtx so it won't write after shutdown. func (s *Server) deferShowDocument(uri string, sel Range) { ctx := s.serverCtx + if ctx == nil { + // Fallback for tests that don't set a server context. + ctx = context.Background() + } s.inflight.Add(1) go func() { defer s.inflight.Done() - if ctx == nil { - time.Sleep(120 * time.Millisecond) - s.clientShowDocument(uri, &sel) - return - } select { case <-time.After(120 * time.Millisecond): s.clientShowDocument(uri, &sel) diff --git a/internal/lsp/handlers_utils.go b/internal/lsp/handlers_utils.go index e3c65a5..048c2fd 100644 --- a/internal/lsp/handlers_utils.go +++ b/internal/lsp/handlers_utils.go @@ -278,8 +278,10 @@ func (s *Server) chatWithStats(ctx context.Context, surface surfaceKind, spec re return "", err } s.incRecvCounters(len(txt)) - // Update global stats cache - _ = stats.Update(ctx, client.Name(), modelUsed, sent, len(txt)) + // Update global stats cache; log but don't fail on stats errors + if err := stats.Update(ctx, client.Name(), modelUsed, sent, len(txt)); err != nil { + logging.Logf("lsp ", "stats update error: %v", err) + } s.logLLMStats(modelUsed) return txt, nil } diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 25c5e5c..7675d34 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -344,13 +344,17 @@ func (s *Server) cancelRequests() { func (s *Server) emitLLMStartStatus(provider, model string) { if s.statusSink != nil { - _ = s.statusSink.SetLLMStart(provider, model) + if err := s.statusSink.SetLLMStart(provider, model); err != nil { + logging.Logf("lsp ", "status sink SetLLMStart error: %v", err) + } } } func (s *Server) emitGlobalStatus(gs GlobalStatus) { if s.statusSink != nil { - _ = s.statusSink.SetGlobal(gs) + if err := s.statusSink.SetGlobal(gs); err != nil { + logging.Logf("lsp ", "status sink SetGlobal error: %v", err) + } } } -- cgit v1.2.3