fix(lsp): bound tool execution time (#205)
* fix(lsp): bound tool execution time * fix(lsp): harden timeout cancellation
- commit
- f7eb31459d487beb80f4c559a44581dc4c3feca4
- author
- bnema · · 2026-09-12 14:55 UTC
- committer
- GitHub · · 2026-09-12 14:55 UTC
- parent
- 413a9a5
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown
5 files · +87 -13
M docs/lsp.md +3 -2
| @@ -74,8 +74,9 @@ before parsing. | |||
| 74 | 74 | ||
| 75 | 75 | ## Tool actions | |
| 76 | 76 | ||
| 77 | - | The model calls `lsp` with an `action` and an optional `path`, `line`, `column`, | |
| 78 | - | `server`, or `query`: | |
| 77 | + | The model calls `lsp` with an `action` and optional arguments such as `path`, | |
| 78 | + | `line`, `column`, `server`, or `query`. Every action has an overall time budget: | |
| 79 | + | `timeout_ms` defaults to 30,000 milliseconds and is capped at 120,000 milliseconds. | |
| 79 | 80 | ||
| 80 | 81 | - `diagnostics` — merge LSP and CLI diagnostics for a file, glob, or the whole workspace. `run_cli:false` skips command-line linters. | |
| 81 | 82 | - `definition`, `type_definition`, `implementation`, `references`, `hover` — query a language server at a 1-based line and column. | |
| @@ -74,8 +74,9 @@ before parsing. | |||||
| 74 | 74 | ||||
| 75 | ## Tool actions | 75 | ## Tool actions | ||
| 76 | 76 | ||||
| 77 | - | The model calls `lsp` with an `action` and an optional `path`, `line`, `column`, | 77 | + | The model calls `lsp` with an `action` and optional arguments such as `path`, |
| 78 | - | `server`, or `query`: | 78 | + | `line`, `column`, `server`, or `query`. Every action has an overall time budget: |
| 79 | + | `timeout_ms` defaults to 30,000 milliseconds and is capped at 120,000 milliseconds. | |||
| 79 | 80 | ||||
| 80 | - `diagnostics` — merge LSP and CLI diagnostics for a file, glob, or the whole workspace. `run_cli:false` skips command-line linters. | 81 | - `diagnostics` — merge LSP and CLI diagnostics for a file, glob, or the whole workspace. `run_cli:false` skips command-line linters. | ||
| 81 | - `definition`, `type_definition`, `implementation`, `references`, `hover` — query a language server at a 1-based line and column. | 82 | - `definition`, `type_definition`, `implementation`, `references`, `hover` — query a language server at a 1-based line and column. | ||
M packages/agent/lsp/lsp_test.go +24 -0
| @@ -5,6 +5,7 @@ import ( | |||
| 5 | 5 | "bytes" | |
| 6 | 6 | "context" | |
| 7 | 7 | "encoding/json" | |
| 8 | + | "errors" | |
| 8 | 9 | "io" | |
| 9 | 10 | "os" | |
| 10 | 11 | "path/filepath" | |
| @@ -31,6 +32,9 @@ func TestFakeLSPProcess(t *testing.T) { | |||
| 31 | 32 | } | |
| 32 | 33 | switch message.Method { | |
| 33 | 34 | case "initialize": | |
| 35 | + | if os.Getenv("ZUT_FAKE_LSP_HANG_INITIALIZE") == "1" { | |
| 36 | + | continue | |
| 37 | + | } | |
| 34 | 38 | response := []byte(`{"jsonrpc":"2.0","id":` + string(message.ID) + `,"result":{"capabilities":{"definitionProvider":true}}}`) | |
| 35 | 39 | _ = WriteMessage(os.Stdout, response) | |
| 36 | 40 | case "test/echo": | |
| @@ -53,6 +57,26 @@ func TestFakeLSPProcess(t *testing.T) { | |||
| 53 | 57 | } | |
| 54 | 58 | } | |
| 55 | 59 | ||
| 60 | + | func TestManagerCapabilitiesHonorsContextDuringInitialize(t *testing.T) { | |
| 61 | + | root := t.TempDir() | |
| 62 | + | config := `{"autoDetect":false,"servers":{"fake":{"kind":"lsp","command":` + strconv.Quote(os.Args[0]) + `,"args":["-test.run=TestFakeLSPProcess"],"env":{"ZUT_FAKE_LSP":"1","ZUT_FAKE_LSP_HANG_INITIALIZE":"1"}}}}` | |
| 63 | + | if err := os.WriteFile(filepath.Join(root, "lsp.json"), []byte(config), 0o644); err != nil { | |
| 64 | + | t.Fatal(err) | |
| 65 | + | } | |
| 66 | + | manager := NewManager() | |
| 67 | + | t.Cleanup(func() { _ = manager.Close() }) | |
| 68 | + | ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) | |
| 69 | + | defer cancel() | |
| 70 | + | started := time.Now() | |
| 71 | + | _, err := manager.Capabilities(ctx, root, "", "fake") | |
| 72 | + | if !errors.Is(err, context.DeadlineExceeded) { | |
| 73 | + | t.Fatalf("Capabilities error = %v, want context deadline exceeded", err) | |
| 74 | + | } | |
| 75 | + | if elapsed := time.Since(started); elapsed > time.Second { | |
| 76 | + | t.Fatalf("Capabilities returned after %s, want prompt context cancellation", elapsed) | |
| 77 | + | } | |
| 78 | + | } | |
| 79 | + | ||
| 56 | 80 | func TestClientFakeStdioRPC(t *testing.T) { | |
| 57 | 81 | root := t.TempDir() | |
| 58 | 82 | script := os.Args[0] | |
| @@ -5,6 +5,7 @@ import ( | |||||
| 5 | "bytes" | 5 | "bytes" | ||
| 6 | "context" | 6 | "context" | ||
| 7 | "encoding/json" | 7 | "encoding/json" | ||
| 8 | + | "errors" | |||
| 8 | "io" | 9 | "io" | ||
| 9 | "os" | 10 | "os" | ||
| 10 | "path/filepath" | 11 | "path/filepath" | ||
| @@ -31,6 +32,9 @@ func TestFakeLSPProcess(t *testing.T) { | |||||
| 31 | } | 32 | } | ||
| 32 | switch message.Method { | 33 | switch message.Method { | ||
| 33 | case "initialize": | 34 | case "initialize": | ||
| 35 | + | if os.Getenv("ZUT_FAKE_LSP_HANG_INITIALIZE") == "1" { | |||
| 36 | + | continue | |||
| 37 | + | } | |||
| 34 | response := []byte(`{"jsonrpc":"2.0","id":` + string(message.ID) + `,"result":{"capabilities":{"definitionProvider":true}}}`) | 38 | response := []byte(`{"jsonrpc":"2.0","id":` + string(message.ID) + `,"result":{"capabilities":{"definitionProvider":true}}}`) | ||
| 35 | _ = WriteMessage(os.Stdout, response) | 39 | _ = WriteMessage(os.Stdout, response) | ||
| 36 | case "test/echo": | 40 | case "test/echo": | ||
| @@ -53,6 +57,26 @@ func TestFakeLSPProcess(t *testing.T) { | |||||
| 53 | } | 57 | } | ||
| 54 | } | 58 | } | ||
| 55 | 59 | ||||
| 60 | + | func TestManagerCapabilitiesHonorsContextDuringInitialize(t *testing.T) { | |||
| 61 | + | root := t.TempDir() | |||
| 62 | + | config := `{"autoDetect":false,"servers":{"fake":{"kind":"lsp","command":` + strconv.Quote(os.Args[0]) + `,"args":["-test.run=TestFakeLSPProcess"],"env":{"ZUT_FAKE_LSP":"1","ZUT_FAKE_LSP_HANG_INITIALIZE":"1"}}}}` | |||
| 63 | + | if err := os.WriteFile(filepath.Join(root, "lsp.json"), []byte(config), 0o644); err != nil { | |||
| 64 | + | t.Fatal(err) | |||
| 65 | + | } | |||
| 66 | + | manager := NewManager() | |||
| 67 | + | t.Cleanup(func() { _ = manager.Close() }) | |||
| 68 | + | ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) | |||
| 69 | + | defer cancel() | |||
| 70 | + | started := time.Now() | |||
| 71 | + | _, err := manager.Capabilities(ctx, root, "", "fake") | |||
| 72 | + | if !errors.Is(err, context.DeadlineExceeded) { | |||
| 73 | + | t.Fatalf("Capabilities error = %v, want context deadline exceeded", err) | |||
| 74 | + | } | |||
| 75 | + | if elapsed := time.Since(started); elapsed > time.Second { | |||
| 76 | + | t.Fatalf("Capabilities returned after %s, want prompt context cancellation", elapsed) | |||
| 77 | + | } | |||
| 78 | + | } | |||
| 79 | + | ||||
| 56 | func TestClientFakeStdioRPC(t *testing.T) { | 80 | func TestClientFakeStdioRPC(t *testing.T) { | ||
| 57 | root := t.TempDir() | 81 | root := t.TempDir() | ||
| 58 | script := os.Args[0] | 82 | script := os.Args[0] | ||
M packages/agent/lsp/manager.go +20 -2
| @@ -179,6 +179,12 @@ func (m *Manager) ensureClient(ctx context.Context, ws *workspace, spec ServerCo | |||
| 179 | 179 | m.mu.Unlock() | |
| 180 | 180 | return nil, errManagerClosed | |
| 181 | 181 | } | |
| 182 | + | if ctx.Err() != nil { | |
| 183 | + | ws.statuses[spec.ID] = "stopped" | |
| 184 | + | delete(ws.errors, spec.ID) | |
| 185 | + | m.mu.Unlock() | |
| 186 | + | return nil, err | |
| 187 | + | } | |
| 182 | 188 | ws.statuses[spec.ID] = "missing" | |
| 183 | 189 | ws.errors[spec.ID] = err.Error() | |
| 184 | 190 | m.mu.Unlock() | |
| @@ -573,7 +579,12 @@ func (m *Manager) Status(cwd, path string) ([]ServerStatus, error) { | |||
| 573 | 579 | return out, nil | |
| 574 | 580 | } | |
| 575 | 581 | ||
| 576 | - | func (m *Manager) Capabilities(cwd, path, server string) (map[string]json.RawMessage, error) { | |
| 582 | + | // Capabilities starts matching servers when needed. The context bounds server | |
| 583 | + | // startup and initialization. | |
| 584 | + | func (m *Manager) Capabilities(ctx context.Context, cwd, path, server string) (map[string]json.RawMessage, error) { | |
| 585 | + | if ctx == nil { | |
| 586 | + | ctx = context.Background() | |
| 587 | + | } | |
| 577 | 588 | ws, err := m.workspace(cwd) | |
| 578 | 589 | if err != nil { | |
| 579 | 590 | return nil, err | |
| @@ -588,7 +599,7 @@ func (m *Manager) Capabilities(cwd, path, server string) (map[string]json.RawMes | |||
| 588 | 599 | if client != nil { | |
| 589 | 600 | return client, nil | |
| 590 | 601 | } | |
| 591 | - | return m.ensureClient(context.Background(), ws, spec) | |
| 602 | + | return m.ensureClient(ctx, ws, spec) | |
| 592 | 603 | } | |
| 593 | 604 | if server != "" { | |
| 594 | 605 | spec, ok := ws.config.Servers[server] | |
| @@ -608,6 +619,12 @@ func (m *Manager) Capabilities(cwd, path, server string) (map[string]json.RawMes | |||
| 608 | 619 | out := make(map[string]json.RawMessage) | |
| 609 | 620 | var firstErr error | |
| 610 | 621 | for _, status := range statuses { | |
| 622 | + | if err := ctx.Err(); err != nil { | |
| 623 | + | if firstErr == nil { | |
| 624 | + | firstErr = err | |
| 625 | + | } | |
| 626 | + | break | |
| 627 | + | } | |
| 611 | 628 | spec := ws.config.Servers[status.ID] | |
| 612 | 629 | if spec.Kind != "lsp" || spec.IsLinter { | |
| 613 | 630 | continue | |
| @@ -745,6 +762,7 @@ func (m *Manager) RunLinters(ctx context.Context, cwd, path string) error { | |||
| 745 | 762 | } | |
| 746 | 763 | commandCtx, cancel := context.WithTimeout(ctx, m.options.LinterTimeout) | |
| 747 | 764 | cmd := exec.CommandContext(commandCtx, command, args...) | |
| 765 | + | cmd.WaitDelay = 2 * time.Second | |
| 748 | 766 | cmd.Dir = ws.cwd | |
| 749 | 767 | cmd.Env = mergedEnvironment(spec.Env) | |
| 750 | 768 | var stdout, stderr limitedBuffer | |
| @@ -179,6 +179,12 @@ func (m *Manager) ensureClient(ctx context.Context, ws *workspace, spec ServerCo | |||||
| 179 | m.mu.Unlock() | 179 | m.mu.Unlock() | ||
| 180 | return nil, errManagerClosed | 180 | return nil, errManagerClosed | ||
| 181 | } | 181 | } | ||
| 182 | + | if ctx.Err() != nil { | |||
| 183 | + | ws.statuses[spec.ID] = "stopped" | |||
| 184 | + | delete(ws.errors, spec.ID) | |||
| 185 | + | m.mu.Unlock() | |||
| 186 | + | return nil, err | |||
| 187 | + | } | |||
| 182 | ws.statuses[spec.ID] = "missing" | 188 | ws.statuses[spec.ID] = "missing" | ||
| 183 | ws.errors[spec.ID] = err.Error() | 189 | ws.errors[spec.ID] = err.Error() | ||
| 184 | m.mu.Unlock() | 190 | m.mu.Unlock() | ||
| @@ -573,7 +579,12 @@ func (m *Manager) Status(cwd, path string) ([]ServerStatus, error) { | |||||
| 573 | return out, nil | 579 | return out, nil | ||
| 574 | } | 580 | } | ||
| 575 | 581 | ||||
| 576 | - | func (m *Manager) Capabilities(cwd, path, server string) (map[string]json.RawMessage, error) { | 582 | + | // Capabilities starts matching servers when needed. The context bounds server |
| 583 | + | // startup and initialization. | |||
| 584 | + | func (m *Manager) Capabilities(ctx context.Context, cwd, path, server string) (map[string]json.RawMessage, error) { | |||
| 585 | + | if ctx == nil { | |||
| 586 | + | ctx = context.Background() | |||
| 587 | + | } | |||
| 577 | ws, err := m.workspace(cwd) | 588 | ws, err := m.workspace(cwd) | ||
| 578 | if err != nil { | 589 | if err != nil { | ||
| 579 | return nil, err | 590 | return nil, err | ||
| @@ -588,7 +599,7 @@ func (m *Manager) Capabilities(cwd, path, server string) (map[string]json.RawMes | |||||
| 588 | if client != nil { | 599 | if client != nil { | ||
| 589 | return client, nil | 600 | return client, nil | ||
| 590 | } | 601 | } | ||
| 591 | - | return m.ensureClient(context.Background(), ws, spec) | 602 | + | return m.ensureClient(ctx, ws, spec) |
| 592 | } | 603 | } | ||
| 593 | if server != "" { | 604 | if server != "" { | ||
| 594 | spec, ok := ws.config.Servers[server] | 605 | spec, ok := ws.config.Servers[server] | ||
| @@ -608,6 +619,12 @@ func (m *Manager) Capabilities(cwd, path, server string) (map[string]json.RawMes | |||||
| 608 | out := make(map[string]json.RawMessage) | 619 | out := make(map[string]json.RawMessage) | ||
| 609 | var firstErr error | 620 | var firstErr error | ||
| 610 | for _, status := range statuses { | 621 | for _, status := range statuses { | ||
| 622 | + | if err := ctx.Err(); err != nil { | |||
| 623 | + | if firstErr == nil { | |||
| 624 | + | firstErr = err | |||
| 625 | + | } | |||
| 626 | + | break | |||
| 627 | + | } | |||
| 611 | spec := ws.config.Servers[status.ID] | 628 | spec := ws.config.Servers[status.ID] | ||
| 612 | if spec.Kind != "lsp" || spec.IsLinter { | 629 | if spec.Kind != "lsp" || spec.IsLinter { | ||
| 613 | continue | 630 | continue | ||
| @@ -745,6 +762,7 @@ func (m *Manager) RunLinters(ctx context.Context, cwd, path string) error { | |||||
| 745 | } | 762 | } | ||
| 746 | commandCtx, cancel := context.WithTimeout(ctx, m.options.LinterTimeout) | 763 | commandCtx, cancel := context.WithTimeout(ctx, m.options.LinterTimeout) | ||
| 747 | cmd := exec.CommandContext(commandCtx, command, args...) | 764 | cmd := exec.CommandContext(commandCtx, command, args...) | ||
| 765 | + | cmd.WaitDelay = 2 * time.Second | |||
| 748 | cmd.Dir = ws.cwd | 766 | cmd.Dir = ws.cwd | ||
| 749 | cmd.Env = mergedEnvironment(spec.Env) | 767 | cmd.Env = mergedEnvironment(spec.Env) | ||
| 750 | var stdout, stderr limitedBuffer | 768 | var stdout, stderr limitedBuffer | ||
M packages/agent/tools/lsp.go +20 -9
| @@ -15,7 +15,12 @@ import ( | |||
| 15 | 15 | "github.com/bnema/zut/packages/provider" | |
| 16 | 16 | ) | |
| 17 | 17 | ||
| 18 | - | const maxLSPToolOutput = 60 * 1024 | |
| 18 | + | const ( | |
| 19 | + | maxLSPToolOutput = 60 * 1024 | |
| 20 | + | defaultLSPTimeout = 30 * time.Second | |
| 21 | + | maxLSPTimeout = 2 * time.Minute | |
| 22 | + | maxLSPTimeoutMillis = int(maxLSPTimeout / time.Millisecond) | |
| 23 | + | ) | |
| 19 | 24 | ||
| 20 | 25 | // LSPTool exposes diagnostics, language navigation, and server management to | |
| 21 | 26 | // the model. Manager owns processes and is normally shared by all tool calls | |
| @@ -51,7 +56,7 @@ type lspArgs struct { | |||
| 51 | 56 | RunCLI *bool `json:"run_cli,omitempty"` | |
| 52 | 57 | } | |
| 53 | 58 | ||
| 54 | - | const lspSchema = `{"type":"object","properties":{"action":{"type":"string","enum":["diagnostics","definition","references","hover","symbols","rename","code_actions","type_definition","implementation","status","reload","capabilities","request"]},"path":{"type":"string","description":"File path relative to the workspace, required for document actions."},"line":{"type":"integer","minimum":1,"description":"1-based line for a document action."},"column":{"type":"integer","minimum":1,"description":"1-based column for a document action."},"query":{"type":"string","description":"Workspace symbol query; symbols uses document symbols when omitted."},"new_name":{"type":"string","description":"New identifier for rename."},"server":{"type":"string","description":"Optional server id for request or capabilities."},"method":{"type":"string","description":"Raw LSP method for request."},"params":{"type":"object","description":"Raw JSON-RPC params for request."},"apply":{"type":"boolean","description":"Apply a returned rename/code-action WorkspaceEdit. Only files inside the workspace are accepted."},"timeout_ms":{"type":"integer","minimum":1},"max":{"type":"integer","minimum":1,"maximum":50},"run_cli":{"type":"boolean","description":"Run configured CLI linters for diagnostics (default true)."}},"required":["action"]}` | |
| 59 | + | const lspSchema = `{"type":"object","properties":{"action":{"type":"string","enum":["diagnostics","definition","references","hover","symbols","rename","code_actions","type_definition","implementation","status","reload","capabilities","request"]},"path":{"type":"string","description":"File path relative to the workspace, required for document actions."},"line":{"type":"integer","minimum":1,"description":"1-based line for a document action."},"column":{"type":"integer","minimum":1,"description":"1-based column for a document action."},"query":{"type":"string","description":"Workspace symbol query; symbols uses document symbols when omitted."},"new_name":{"type":"string","description":"New identifier for rename."},"server":{"type":"string","description":"Optional server id for request or capabilities."},"method":{"type":"string","description":"Raw LSP method for request."},"params":{"type":"object","description":"Raw JSON-RPC params for request."},"apply":{"type":"boolean","description":"Apply a returned rename/code-action WorkspaceEdit. Only files inside the workspace are accepted."},"timeout_ms":{"type":"integer","minimum":1,"maximum":120000,"description":"Request timeout in milliseconds (default 30000, maximum 120000)."},"max":{"type":"integer","minimum":1,"maximum":50},"run_cli":{"type":"boolean","description":"Run configured CLI linters for diagnostics (default true)."}},"required":["action"]}` | |
| 55 | 60 | ||
| 56 | 61 | func (t *LSPTool) Name() string { return "lsp" } | |
| 57 | 62 | func (t *LSPTool) Description() string { | |
| @@ -91,12 +96,8 @@ func (t *LSPTool) Execute(ctx context.Context, raw json.RawMessage, _ func(strin | |||
| 91 | 96 | } | |
| 92 | 97 | } | |
| 93 | 98 | } | |
| 94 | - | runCtx := ctx | |
| 95 | - | if args.Timeout > 0 { | |
| 96 | - | var cancel context.CancelFunc | |
| 97 | - | runCtx, cancel = context.WithTimeout(ctx, time.Duration(args.Timeout)*time.Millisecond) | |
| 98 | - | defer cancel() | |
| 99 | - | } | |
| 99 | + | runCtx, cancel := context.WithTimeout(ctx, lspToolTimeout(args.Timeout)) | |
| 100 | + | defer cancel() | |
| 100 | 101 | var text string | |
| 101 | 102 | var details any | |
| 102 | 103 | var actionErr error | |
| @@ -148,7 +149,7 @@ func (t *LSPTool) Execute(ctx context.Context, raw json.RawMessage, _ func(strin | |||
| 148 | 149 | text = "Reloaded LSP and linter processes and configuration." | |
| 149 | 150 | case "capabilities": | |
| 150 | 151 | var capabilities map[string]json.RawMessage | |
| 151 | - | capabilities, actionErr = t.Manager.Capabilities(absCWD, path, args.Server) | |
| 152 | + | capabilities, actionErr = t.Manager.Capabilities(runCtx, absCWD, path, args.Server) | |
| 152 | 153 | text = prettyJSON(capabilities) | |
| 153 | 154 | details = capabilities | |
| 154 | 155 | case "request": | |
| @@ -381,6 +382,16 @@ func hasGlobMeta(value string) bool { | |||
| 381 | 382 | return strings.ContainsAny(value, "*?[") | |
| 382 | 383 | } | |
| 383 | 384 | ||
| 385 | + | func lspToolTimeout(milliseconds int) time.Duration { | |
| 386 | + | if milliseconds <= 0 { | |
| 387 | + | return defaultLSPTimeout | |
| 388 | + | } | |
| 389 | + | if milliseconds > maxLSPTimeoutMillis { | |
| 390 | + | return maxLSPTimeout | |
| 391 | + | } | |
| 392 | + | return time.Duration(milliseconds) * time.Millisecond | |
| 393 | + | } | |
| 394 | + | ||
| 384 | 395 | func boundedMax(value int) int { | |
| 385 | 396 | if value <= 0 || value > lsp.DefaultDiagnosticCap { | |
| 386 | 397 | return lsp.DefaultDiagnosticCap | |
| @@ -15,7 +15,12 @@ import ( | |||||
| 15 | "github.com/bnema/zut/packages/provider" | 15 | "github.com/bnema/zut/packages/provider" | ||
| 16 | ) | 16 | ) | ||
| 17 | 17 | ||||
| 18 | - | const maxLSPToolOutput = 60 * 1024 | 18 | + | const ( |
| 19 | + | maxLSPToolOutput = 60 * 1024 | |||
| 20 | + | defaultLSPTimeout = 30 * time.Second | |||
| 21 | + | maxLSPTimeout = 2 * time.Minute | |||
| 22 | + | maxLSPTimeoutMillis = int(maxLSPTimeout / time.Millisecond) | |||
| 23 | + | ) | |||
| 19 | 24 | ||||
| 20 | // LSPTool exposes diagnostics, language navigation, and server management to | 25 | // LSPTool exposes diagnostics, language navigation, and server management to | ||
| 21 | // the model. Manager owns processes and is normally shared by all tool calls | 26 | // the model. Manager owns processes and is normally shared by all tool calls | ||
| @@ -51,7 +56,7 @@ type lspArgs struct { | |||||
| 51 | RunCLI *bool `json:"run_cli,omitempty"` | 56 | RunCLI *bool `json:"run_cli,omitempty"` | ||
| 52 | } | 57 | } | ||
| 53 | 58 | ||||
| 54 | - | const lspSchema = `{"type":"object","properties":{"action":{"type":"string","enum":["diagnostics","definition","references","hover","symbols","rename","code_actions","type_definition","implementation","status","reload","capabilities","request"]},"path":{"type":"string","description":"File path relative to the workspace, required for document actions."},"line":{"type":"integer","minimum":1,"description":"1-based line for a document action."},"column":{"type":"integer","minimum":1,"description":"1-based column for a document action."},"query":{"type":"string","description":"Workspace symbol query; symbols uses document symbols when omitted."},"new_name":{"type":"string","description":"New identifier for rename."},"server":{"type":"string","description":"Optional server id for request or capabilities."},"method":{"type":"string","description":"Raw LSP method for request."},"params":{"type":"object","description":"Raw JSON-RPC params for request."},"apply":{"type":"boolean","description":"Apply a returned rename/code-action WorkspaceEdit. Only files inside the workspace are accepted."},"timeout_ms":{"type":"integer","minimum":1},"max":{"type":"integer","minimum":1,"maximum":50},"run_cli":{"type":"boolean","description":"Run configured CLI linters for diagnostics (default true)."}},"required":["action"]}` | 59 | + | const lspSchema = `{"type":"object","properties":{"action":{"type":"string","enum":["diagnostics","definition","references","hover","symbols","rename","code_actions","type_definition","implementation","status","reload","capabilities","request"]},"path":{"type":"string","description":"File path relative to the workspace, required for document actions."},"line":{"type":"integer","minimum":1,"description":"1-based line for a document action."},"column":{"type":"integer","minimum":1,"description":"1-based column for a document action."},"query":{"type":"string","description":"Workspace symbol query; symbols uses document symbols when omitted."},"new_name":{"type":"string","description":"New identifier for rename."},"server":{"type":"string","description":"Optional server id for request or capabilities."},"method":{"type":"string","description":"Raw LSP method for request."},"params":{"type":"object","description":"Raw JSON-RPC params for request."},"apply":{"type":"boolean","description":"Apply a returned rename/code-action WorkspaceEdit. Only files inside the workspace are accepted."},"timeout_ms":{"type":"integer","minimum":1,"maximum":120000,"description":"Request timeout in milliseconds (default 30000, maximum 120000)."},"max":{"type":"integer","minimum":1,"maximum":50},"run_cli":{"type":"boolean","description":"Run configured CLI linters for diagnostics (default true)."}},"required":["action"]}` |
| 55 | 60 | ||||
| 56 | func (t *LSPTool) Name() string { return "lsp" } | 61 | func (t *LSPTool) Name() string { return "lsp" } | ||
| 57 | func (t *LSPTool) Description() string { | 62 | func (t *LSPTool) Description() string { | ||
| @@ -91,12 +96,8 @@ func (t *LSPTool) Execute(ctx context.Context, raw json.RawMessage, _ func(strin | |||||
| 91 | } | 96 | } | ||
| 92 | } | 97 | } | ||
| 93 | } | 98 | } | ||
| 94 | - | runCtx := ctx | 99 | + | runCtx, cancel := context.WithTimeout(ctx, lspToolTimeout(args.Timeout)) |
| 95 | - | if args.Timeout > 0 { | 100 | + | defer cancel() |
| 96 | - | var cancel context.CancelFunc | |||
| 97 | - | runCtx, cancel = context.WithTimeout(ctx, time.Duration(args.Timeout)*time.Millisecond) | |||
| 98 | - | defer cancel() | |||
| 99 | - | } | |||
| 100 | var text string | 101 | var text string | ||
| 101 | var details any | 102 | var details any | ||
| 102 | var actionErr error | 103 | var actionErr error | ||
| @@ -148,7 +149,7 @@ func (t *LSPTool) Execute(ctx context.Context, raw json.RawMessage, _ func(strin | |||||
| 148 | text = "Reloaded LSP and linter processes and configuration." | 149 | text = "Reloaded LSP and linter processes and configuration." | ||
| 149 | case "capabilities": | 150 | case "capabilities": | ||
| 150 | var capabilities map[string]json.RawMessage | 151 | var capabilities map[string]json.RawMessage | ||
| 151 | - | capabilities, actionErr = t.Manager.Capabilities(absCWD, path, args.Server) | 152 | + | capabilities, actionErr = t.Manager.Capabilities(runCtx, absCWD, path, args.Server) |
| 152 | text = prettyJSON(capabilities) | 153 | text = prettyJSON(capabilities) | ||
| 153 | details = capabilities | 154 | details = capabilities | ||
| 154 | case "request": | 155 | case "request": | ||
| @@ -381,6 +382,16 @@ func hasGlobMeta(value string) bool { | |||||
| 381 | return strings.ContainsAny(value, "*?[") | 382 | return strings.ContainsAny(value, "*?[") | ||
| 382 | } | 383 | } | ||
| 383 | 384 | ||||
| 385 | + | func lspToolTimeout(milliseconds int) time.Duration { | |||
| 386 | + | if milliseconds <= 0 { | |||
| 387 | + | return defaultLSPTimeout | |||
| 388 | + | } | |||
| 389 | + | if milliseconds > maxLSPTimeoutMillis { | |||
| 390 | + | return maxLSPTimeout | |||
| 391 | + | } | |||
| 392 | + | return time.Duration(milliseconds) * time.Millisecond | |||
| 393 | + | } | |||
| 394 | + | ||||
| 384 | func boundedMax(value int) int { | 395 | func boundedMax(value int) int { | ||
| 385 | if value <= 0 || value > lsp.DefaultDiagnosticCap { | 396 | if value <= 0 || value > lsp.DefaultDiagnosticCap { | ||
| 386 | return lsp.DefaultDiagnosticCap | 397 | return lsp.DefaultDiagnosticCap | ||
M packages/agent/tools/lsp_test.go +20 -0
| @@ -8,6 +8,7 @@ import ( | |||
| 8 | 8 | "runtime" | |
| 9 | 9 | "strings" | |
| 10 | 10 | "testing" | |
| 11 | + | "time" | |
| 11 | 12 | "unicode/utf8" | |
| 12 | 13 | ||
| 13 | 14 | "github.com/bnema/zut/packages/agent/lsp" | |
| @@ -62,6 +63,25 @@ func TestLSPToolGlobRespectsJailForSymlinkMatches(t *testing.T) { | |||
| 62 | 63 | } | |
| 63 | 64 | } | |
| 64 | 65 | ||
| 66 | + | func TestLSPToolTimeoutIsAlwaysBounded(t *testing.T) { | |
| 67 | + | cases := []struct { | |
| 68 | + | milliseconds int | |
| 69 | + | want time.Duration | |
| 70 | + | }{ | |
| 71 | + | {milliseconds: 0, want: defaultLSPTimeout}, | |
| 72 | + | {milliseconds: -1, want: defaultLSPTimeout}, | |
| 73 | + | {milliseconds: 25, want: 25 * time.Millisecond}, | |
| 74 | + | {milliseconds: maxLSPTimeoutMillis, want: maxLSPTimeout}, | |
| 75 | + | {milliseconds: maxLSPTimeoutMillis + 1, want: maxLSPTimeout}, | |
| 76 | + | {milliseconds: 1 << 62, want: maxLSPTimeout}, | |
| 77 | + | } | |
| 78 | + | for _, tc := range cases { | |
| 79 | + | if got := lspToolTimeout(tc.milliseconds); got != tc.want { | |
| 80 | + | t.Errorf("lspToolTimeout(%d) = %s, want %s", tc.milliseconds, got, tc.want) | |
| 81 | + | } | |
| 82 | + | } | |
| 83 | + | } | |
| 84 | + | ||
| 65 | 85 | func TestBoundLSPText(t *testing.T) { | |
| 66 | 86 | value := boundLSPText(strings.Repeat("x", maxLSPToolOutput+10)) | |
| 67 | 87 | if len(value) > maxLSPToolOutput || !strings.Contains(value, "truncated") { | |
| @@ -8,6 +8,7 @@ import ( | |||||
| 8 | "runtime" | 8 | "runtime" | ||
| 9 | "strings" | 9 | "strings" | ||
| 10 | "testing" | 10 | "testing" | ||
| 11 | + | "time" | |||
| 11 | "unicode/utf8" | 12 | "unicode/utf8" | ||
| 12 | 13 | ||||
| 13 | "github.com/bnema/zut/packages/agent/lsp" | 14 | "github.com/bnema/zut/packages/agent/lsp" | ||
| @@ -62,6 +63,25 @@ func TestLSPToolGlobRespectsJailForSymlinkMatches(t *testing.T) { | |||||
| 62 | } | 63 | } | ||
| 63 | } | 64 | } | ||
| 64 | 65 | ||||
| 66 | + | func TestLSPToolTimeoutIsAlwaysBounded(t *testing.T) { | |||
| 67 | + | cases := []struct { | |||
| 68 | + | milliseconds int | |||
| 69 | + | want time.Duration | |||
| 70 | + | }{ | |||
| 71 | + | {milliseconds: 0, want: defaultLSPTimeout}, | |||
| 72 | + | {milliseconds: -1, want: defaultLSPTimeout}, | |||
| 73 | + | {milliseconds: 25, want: 25 * time.Millisecond}, | |||
| 74 | + | {milliseconds: maxLSPTimeoutMillis, want: maxLSPTimeout}, | |||
| 75 | + | {milliseconds: maxLSPTimeoutMillis + 1, want: maxLSPTimeout}, | |||
| 76 | + | {milliseconds: 1 << 62, want: maxLSPTimeout}, | |||
| 77 | + | } | |||
| 78 | + | for _, tc := range cases { | |||
| 79 | + | if got := lspToolTimeout(tc.milliseconds); got != tc.want { | |||
| 80 | + | t.Errorf("lspToolTimeout(%d) = %s, want %s", tc.milliseconds, got, tc.want) | |||
| 81 | + | } | |||
| 82 | + | } | |||
| 83 | + | } | |||
| 84 | + | ||||
| 65 | func TestBoundLSPText(t *testing.T) { | 85 | func TestBoundLSPText(t *testing.T) { | ||
| 66 | value := boundLSPText(strings.Repeat("x", maxLSPToolOutput+10)) | 86 | value := boundLSPText(strings.Repeat("x", maxLSPToolOutput+10)) | ||
| 67 | if len(value) > maxLSPToolOutput || !strings.Contains(value, "truncated") { | 87 | if len(value) > maxLSPToolOutput || !strings.Contains(value, "truncated") { | ||