From 09d8e75b7be61fb501ecd28682b5a56717db4465 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Wed, 24 Sep 2025 14:35:12 -0300 Subject: [PATCH] fix(mcp/lsp): expand variable in commands (#1116) closes #806 Signed-off-by: Carlos Alexandro Becker Co-authored-by: taigr --- internal/app/lsp.go | 2 +- internal/llm/agent/mcp-tools.go | 21 +++++++++++++-------- internal/lsp/client.go | 9 +++++++-- internal/lsp/client_test.go | 9 ++++++--- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/internal/app/lsp.go b/internal/app/lsp.go index 057e9ce39363f3fd68c8c980ce22e3e8b0e78154..f4c26af2f4ed369a94c7078600ce9639874dc643 100644 --- a/internal/app/lsp.go +++ b/internal/app/lsp.go @@ -36,7 +36,7 @@ func (app *App) createAndStartLSPClient(ctx context.Context, name string, config updateLSPState(name, lsp.StateStarting, nil, nil, 0) // Create LSP client. - lspClient, err := lsp.New(ctx, name, config) + lspClient, err := lsp.New(ctx, name, config, app.config.Resolver()) if err != nil { slog.Error("Failed to create LSP client for", name, err) updateLSPState(name, lsp.StateError, err, nil, 0) diff --git a/internal/llm/agent/mcp-tools.go b/internal/llm/agent/mcp-tools.go index 4a4435dccbdb48ea6d2d64bf7af9f257e8e3730b..d670a5797548cd52bbfd23c8cd16fea96b021e8a 100644 --- a/internal/llm/agent/mcp-tools.go +++ b/internal/llm/agent/mcp-tools.go @@ -149,7 +149,8 @@ func getOrRenewClient(ctx context.Context, name string) (*client.Client, error) return nil, fmt.Errorf("mcp '%s' not available", name) } - m := config.Get().MCP[name] + cfg := config.Get() + m := cfg.MCP[name] state, _ := mcpStates.Get(name) timeout := mcpTimeout(m) @@ -161,7 +162,7 @@ func getOrRenewClient(ctx context.Context, name string) (*client.Client, error) } updateMCPState(name, MCPStateError, maybeTimeoutErr(err, timeout), nil, state.ToolCount) - c, err = createAndInitializeClient(ctx, name, m) + c, err = createAndInitializeClient(ctx, name, m, cfg.Resolver()) if err != nil { return nil, err } @@ -313,7 +314,7 @@ func doGetMCPTools(ctx context.Context, permissions permission.Service, cfg *con ctx, cancel := context.WithTimeout(ctx, mcpTimeout(m)) defer cancel() - c, err := createAndInitializeClient(ctx, name, m) + c, err := createAndInitializeClient(ctx, name, m, cfg.Resolver()) if err != nil { return } @@ -328,8 +329,8 @@ func doGetMCPTools(ctx context.Context, permissions permission.Service, cfg *con return slices.Collect(result.Seq()) } -func createAndInitializeClient(ctx context.Context, name string, m config.MCPConfig) (*client.Client, error) { - c, err := createMcpClient(name, m) +func createAndInitializeClient(ctx context.Context, name string, m config.MCPConfig, resolver config.VariableResolver) (*client.Client, error) { + c, err := createMcpClient(name, m, resolver) if err != nil { updateMCPState(name, MCPStateError, err, nil, 0) slog.Error("error creating mcp client", "error", err, "name", name) @@ -367,14 +368,18 @@ func maybeTimeoutErr(err error, timeout time.Duration) error { return err } -func createMcpClient(name string, m config.MCPConfig) (*client.Client, error) { +func createMcpClient(name string, m config.MCPConfig, resolver config.VariableResolver) (*client.Client, error) { switch m.Type { case config.MCPStdio: - if strings.TrimSpace(m.Command) == "" { + command, err := resolver.ResolveValue(m.Command) + if err != nil { + return nil, fmt.Errorf("invalid mcp command: %w", err) + } + if strings.TrimSpace(command) == "" { return nil, fmt.Errorf("mcp stdio config requires a non-empty 'command' field") } return client.NewStdioMCPClientWithOptions( - home.Long(m.Command), + home.Long(command), m.ResolvedEnv(), m.Args, transport.WithCommandLogger(mcpLogger{name: name}), diff --git a/internal/lsp/client.go b/internal/lsp/client.go index 226d6c6f3896e29dcbc75c04bee23a34bdc85952..259f6ba8c4876dcbeb441d48839685012c48ac32 100644 --- a/internal/lsp/client.go +++ b/internal/lsp/client.go @@ -45,7 +45,7 @@ type Client struct { } // New creates a new LSP client using the powernap implementation. -func New(ctx context.Context, name string, config config.LSPConfig) (*Client, error) { +func New(ctx context.Context, name string, config config.LSPConfig, resolver config.VariableResolver) (*Client, error) { // Convert working directory to file URI workDir, err := os.Getwd() if err != nil { @@ -54,9 +54,14 @@ func New(ctx context.Context, name string, config config.LSPConfig) (*Client, er rootURI := string(protocol.URIFromPath(workDir)) + command, err := resolver.ResolveValue(config.Command) + if err != nil { + return nil, fmt.Errorf("invalid lsp command: %w", err) + } + // Create powernap client config clientConfig := powernap.ClientConfig{ - Command: home.Long(config.Command), + Command: home.Long(command), Args: config.Args, RootURI: rootURI, Environment: func() map[string]string { diff --git a/internal/lsp/client_test.go b/internal/lsp/client_test.go index 99ef0ca3143e5b8689ba3b63fd5c172456a46c24..7cc9f2f4ba230a4c6896e7ccef367a450c1c55c7 100644 --- a/internal/lsp/client_test.go +++ b/internal/lsp/client_test.go @@ -5,14 +5,15 @@ import ( "testing" "github.com/charmbracelet/crush/internal/config" + "github.com/charmbracelet/crush/internal/env" ) -func TestPowernapClient(t *testing.T) { +func TestClient(t *testing.T) { ctx := context.Background() // Create a simple config for testing cfg := config.LSPConfig{ - Command: "echo", // Use echo as a dummy command that won't fail + Command: "$THE_CMD", // Use echo as a dummy command that won't fail Args: []string{"hello"}, FileTypes: []string{"go"}, Env: map[string]string{}, @@ -20,7 +21,9 @@ func TestPowernapClient(t *testing.T) { // Test creating a powernap client - this will likely fail with echo // but we can still test the basic structure - client, err := New(ctx, "test", cfg) + client, err := New(ctx, "test", cfg, config.NewEnvironmentVariableResolver(env.NewFromMap(map[string]string{ + "THE_CMD": "echo", + }))) if err != nil { // Expected to fail with echo command, skip the rest t.Skipf("Powernap client creation failed as expected with dummy command: %v", err)