on main help sign in
fix(mcp/lsp): expand variable in commands (#1116)
09d8e75Carlos Alexandro Becker
internal/app/lsp.go+1−1
@@ -36,7 +36,7 @@ func (app *App) createAndStartLSPClient(ctx context.Context, name string, config
36 updateLSPState(name, lsp.StateStarting, nil, nil, 0)
36 updateLSPState(name, lsp.StateStarting, nil, nil, 0)
37
37
38 // Create LSP client.
38 // Create LSP client.
39− lspClient, err := lsp.New(ctx, name, config)
39+ lspClient, err := lsp.New(ctx, name, config, app.config.Resolver())
40 if err != nil {
40 if err != nil {
41 slog.Error("Failed to create LSP client for", name, err)
41 slog.Error("Failed to create LSP client for", name, err)
42 updateLSPState(name, lsp.StateError, err, nil, 0)
42 updateLSPState(name, lsp.StateError, err, nil, 0)
internal/llm/agent/mcp-tools.go+13−8
@@ -149,7 +149,8 @@ func getOrRenewClient(ctx context.Context, name string) (*client.Client, error)
149 return nil, fmt.Errorf("mcp '%s' not available", name)
149 return nil, fmt.Errorf("mcp '%s' not available", name)
150 }
150 }
151
151
152− m := config.Get().MCP[name]
152+ cfg := config.Get()
153+ m := cfg.MCP[name]
153 state, _ := mcpStates.Get(name)
154 state, _ := mcpStates.Get(name)
154
155
155 timeout := mcpTimeout(m)
156 timeout := mcpTimeout(m)
@@ -161,7 +162,7 @@ func getOrRenewClient(ctx context.Context, name string) (*client.Client, error)
161 }
162 }
162 updateMCPState(name, MCPStateError, maybeTimeoutErr(err, timeout), nil, state.ToolCount)
163 updateMCPState(name, MCPStateError, maybeTimeoutErr(err, timeout), nil, state.ToolCount)
163
164
164− c, err = createAndInitializeClient(ctx, name, m)
165+ c, err = createAndInitializeClient(ctx, name, m, cfg.Resolver())
165 if err != nil {
166 if err != nil {
166 return nil, err
167 return nil, err
167 }
168 }
@@ -313,7 +314,7 @@ func doGetMCPTools(ctx context.Context, permissions permission.Service, cfg *con
313
314
314 ctx, cancel := context.WithTimeout(ctx, mcpTimeout(m))
315 ctx, cancel := context.WithTimeout(ctx, mcpTimeout(m))
315 defer cancel()
316 defer cancel()
316− c, err := createAndInitializeClient(ctx, name, m)
317+ c, err := createAndInitializeClient(ctx, name, m, cfg.Resolver())
317 if err != nil {
318 if err != nil {
318 return
319 return
319 }
320 }
@@ -328,8 +329,8 @@ func doGetMCPTools(ctx context.Context, permissions permission.Service, cfg *con
328 return slices.Collect(result.Seq())
329 return slices.Collect(result.Seq())
329}
330}
330
331
331−func createAndInitializeClient(ctx context.Context, name string, m config.MCPConfig) (*client.Client, error) {
332+func createAndInitializeClient(ctx context.Context, name string, m config.MCPConfig, resolver config.VariableResolver) (*client.Client, error) {
332− c, err := createMcpClient(name, m)
333+ c, err := createMcpClient(name, m, resolver)
333 if err != nil {
334 if err != nil {
334 updateMCPState(name, MCPStateError, err, nil, 0)
335 updateMCPState(name, MCPStateError, err, nil, 0)
335 slog.Error("error creating mcp client", "error", err, "name", name)
336 slog.Error("error creating mcp client", "error", err, "name", name)
@@ -367,14 +368,18 @@ func maybeTimeoutErr(err error, timeout time.Duration) error {
367 return err
368 return err
368}
369}
369
370
370−func createMcpClient(name string, m config.MCPConfig) (*client.Client, error) {
371+func createMcpClient(name string, m config.MCPConfig, resolver config.VariableResolver) (*client.Client, error) {
371 switch m.Type {
372 switch m.Type {
372 case config.MCPStdio:
373 case config.MCPStdio:
373− if strings.TrimSpace(m.Command) == "" {
374+ command, err := resolver.ResolveValue(m.Command)
375+ if err != nil {
376+ return nil, fmt.Errorf("invalid mcp command: %w", err)
377+ }
378+ if strings.TrimSpace(command) == "" {
374 return nil, fmt.Errorf("mcp stdio config requires a non-empty 'command' field")
379 return nil, fmt.Errorf("mcp stdio config requires a non-empty 'command' field")
375 }
380 }
376 return client.NewStdioMCPClientWithOptions(
381 return client.NewStdioMCPClientWithOptions(
377− home.Long(m.Command),
382+ home.Long(command),
378 m.ResolvedEnv(),
383 m.ResolvedEnv(),
379 m.Args,
384 m.Args,
380 transport.WithCommandLogger(mcpLogger{name: name}),
385 transport.WithCommandLogger(mcpLogger{name: name}),
internal/lsp/client.go+7−2
@@ -45,7 +45,7 @@ type Client struct {
45}
45}
46
46
47// New creates a new LSP client using the powernap implementation.
47// New creates a new LSP client using the powernap implementation.
48−func New(ctx context.Context, name string, config config.LSPConfig) (*Client, error) {
48+func New(ctx context.Context, name string, config config.LSPConfig, resolver config.VariableResolver) (*Client, error) {
49 // Convert working directory to file URI
49 // Convert working directory to file URI
50 workDir, err := os.Getwd()
50 workDir, err := os.Getwd()
51 if err != nil {
51 if err != nil {
@@ -54,9 +54,14 @@ func New(ctx context.Context, name string, config config.LSPConfig) (*Client, er
54
54
55 rootURI := string(protocol.URIFromPath(workDir))
55 rootURI := string(protocol.URIFromPath(workDir))
56
56
57+ command, err := resolver.ResolveValue(config.Command)
58+ if err != nil {
59+ return nil, fmt.Errorf("invalid lsp command: %w", err)
60+ }
61+
57 // Create powernap client config
62 // Create powernap client config
58 clientConfig := powernap.ClientConfig{
63 clientConfig := powernap.ClientConfig{
59− Command: home.Long(config.Command),
64+ Command: home.Long(command),
60 Args: config.Args,
65 Args: config.Args,
61 RootURI: rootURI,
66 RootURI: rootURI,
62 Environment: func() map[string]string {
67 Environment: func() map[string]string {
internal/lsp/client_test.go+6−3
@@ -5,14 +5,15 @@ import (
5 "testing"
5 "testing"
6
6
7 "github.com/charmbracelet/crush/internal/config"
7 "github.com/charmbracelet/crush/internal/config"
8+ "github.com/charmbracelet/crush/internal/env"
8)
9)
9
10
10−func TestPowernapClient(t *testing.T) {
11+func TestClient(t *testing.T) {
11 ctx := context.Background()
12 ctx := context.Background()
12
13
13 // Create a simple config for testing
14 // Create a simple config for testing
14 cfg := config.LSPConfig{
15 cfg := config.LSPConfig{
15− Command: "echo", // Use echo as a dummy command that won't fail
16+ Command: "$THE_CMD", // Use echo as a dummy command that won't fail
16 Args: []string{"hello"},
17 Args: []string{"hello"},
17 FileTypes: []string{"go"},
18 FileTypes: []string{"go"},
18 Env: map[string]string{},
19 Env: map[string]string{},
@@ -20,7 +21,9 @@ func TestPowernapClient(t *testing.T) {
20
21
21 // Test creating a powernap client - this will likely fail with echo
22 // Test creating a powernap client - this will likely fail with echo
22 // but we can still test the basic structure
23 // but we can still test the basic structure
23− client, err := New(ctx, "test", cfg)
24+ client, err := New(ctx, "test", cfg, config.NewEnvironmentVariableResolver(env.NewFromMap(map[string]string{
25+ "THE_CMD": "echo",
26+ })))
24 if err != nil {
27 if err != nil {
25 // Expected to fail with echo command, skip the rest
28 // Expected to fail with echo command, skip the rest
26 t.Skipf("Powernap client creation failed as expected with dummy command: %v", err)
29 t.Skipf("Powernap client creation failed as expected with dummy command: %v", err)