diff options
| author | Paul Buetow <paul@buetow.org> | 2026-03-23 08:08:57 +0200 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2026-03-23 08:08:57 +0200 |
| commit | 4958ea5100ebf8d4ff9fd818b7bc59d01989feb4 (patch) | |
| tree | 44bc03cdfa7d58ea948023c87bfbe8a37fe315c9 /internal/lsp | |
| parent | ba929c035c7c74113d061c57cc5b500af0b20b74 (diff) | |
fix: address all HIGH-severity code quality audit findings
- lsp/server.go: track request goroutines in inflight WaitGroup to
prevent use-after-close writes on shutdown
- lsp/llm_client_registry.go: acquire write lock before calling build()
to eliminate TOCTOU race on cache population
- lsp/handlers_codeaction.go: resolveSimplifyCodeAction now uses
PromptCodeActionSimplify{System,User} (was wrongly using rewrite prompts)
- askcli/taskexport.go: remove exported MustParseTaskExport to prevent
panic on malformed external input; move to unexported test helper
- cmd/ask/main.go: print error to stderr before os.Exit
- llm/{openai,ollama,openrouter}.go: add interface satisfaction assertions
- integrationtests/ask_test.go: replace type assertions with errors.As
for robust exec.ExitError unwrapping
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Diffstat (limited to 'internal/lsp')
| -rw-r--r-- | internal/lsp/handlers_codeaction.go | 8 | ||||
| -rw-r--r-- | internal/lsp/llm_client_registry.go | 26 | ||||
| -rw-r--r-- | internal/lsp/server.go | 8 |
3 files changed, 28 insertions, 14 deletions
diff --git a/internal/lsp/handlers_codeaction.go b/internal/lsp/handlers_codeaction.go index 1d8a36f..aba2113 100644 --- a/internal/lsp/handlers_codeaction.go +++ b/internal/lsp/handlers_codeaction.go @@ -266,10 +266,10 @@ func resolveGoTestCodeAction(s *Server, action CodeAction, payload codeActionPay func resolveSimplifyCodeAction(s *Server, action CodeAction, payload codeActionPayload) (CodeAction, bool) { cfg := s.currentConfig() - sys := cfg.PromptCodeActionRewriteSystem - user := renderTemplate(cfg.PromptCodeActionRewriteUser, map[string]string{ - "instruction": "Simplify and improve the code while preserving behavior. Return only the improved code.", - "selection": payload.Selection, + // Use the simplify-specific prompts, not the rewrite prompts. + sys := cfg.PromptCodeActionSimplifySystem + user := renderTemplate(cfg.PromptCodeActionSimplifyUser, map[string]string{ + "selection": payload.Selection, }) return s.completeCodeAction(action, payload.URI, payload.Range, sys, user, 20*time.Second) } diff --git a/internal/lsp/llm_client_registry.go b/internal/lsp/llm_client_registry.go index 6b9c722..a018158 100644 --- a/internal/lsp/llm_client_registry.go +++ b/internal/lsp/llm_client_registry.go @@ -71,17 +71,28 @@ func (r *llmClientRegistry) clientFor(spec requestSpec, cfg appconfig.App, build if modelOverride == "" { modelOverride = strings.TrimSpace(spec.fallbackModel) } + + // Acquire write lock before calling build to prevent concurrent goroutines + // from all passing the read-lock cache-miss check and issuing duplicate + // build calls for the same provider (TOCTOU race). + r.clientsMu.Lock() + defer r.clientsMu.Unlock() + + // Re-check under the write lock; another goroutine may have populated the + // cache between our RUnlock and this Lock. + if provider == r.llmProvider && r.llmClient != nil { + return r.llmClient + } + if existing, ok := r.altClients[provider]; ok { + return existing + } + client, err := build(cfg, provider, modelOverride) if err != nil { logging.Logf("lsp ", "failed to build client for provider=%s: %v", provider, err) - if baseClient != nil { - return baseClient - } - return nil + return baseClient // may be nil; callers must handle nil } - r.clientsMu.Lock() - defer r.clientsMu.Unlock() if provider == r.llmProvider { if r.llmClient == nil { r.llmClient = client @@ -89,9 +100,6 @@ func (r *llmClientRegistry) clientFor(spec requestSpec, cfg appconfig.App, build } return r.llmClient } - if existing, ok := r.altClients[provider]; ok { - return existing - } if r.altClients == nil { r.altClients = make(map[string]llm.Client) } diff --git a/internal/lsp/server.go b/internal/lsp/server.go index c266e91..25c5e5c 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -378,7 +378,13 @@ func (s *Server) Run() error { // A response from client; ignore continue } - go s.handle(req) + // Track every request goroutine so Run's deferred inflight.Wait() + // catches them all and prevents use-after-close writes to s.out. + s.inflight.Add(1) + go func(r Request) { + defer s.inflight.Done() + s.handle(r) + }(req) if s.exited.Load() { return nil } |
