Detailed changes
@@ -401,6 +401,77 @@ func (m MCPConfig) ResolvedHeaders(r VariableResolver) (map[string]string, error
return out, nil
}
+// ResolvedArgs returns l.Args with every element expanded through the
+// given resolver. A fresh slice is allocated; l.Args is never mutated.
+// On the first resolution failure it returns nil and an error
+// identifying the offending positional index; the inner resolver error
+// is already sanitized by ResolveValue and is wrapped with %w so
+// errors.Is/As continues to work.
+//
+// Empty resolved values are kept (a deliberate "empty positional arg"
+// like --flag "" is sometimes valid), matching MCPConfig.ResolvedArgs;
+// see PLAN.md Phase 2 design decision #18.
+//
+// The resolver choice matters: in server mode pass the shell resolver
+// so $VAR / $(cmd) expand; in client mode pass IdentityResolver so the
+// template is forwarded verbatim. See PLAN.md Phase 2 design decision
+// #13.
+func (l LSPConfig) ResolvedArgs(r VariableResolver) ([]string, error) {
+ if len(l.Args) == 0 {
+ return nil, nil
+ }
+ out := make([]string, len(l.Args))
+ for i, a := range l.Args {
+ v, err := r.ResolveValue(a)
+ if err != nil {
+ return nil, fmt.Errorf("arg %d: %w", i, err)
+ }
+ out[i] = v
+ }
+ return out, nil
+}
+
+// ResolvedEnv returns l.Env with every value expanded through the
+// given resolver. A fresh map is allocated; l.Env is never mutated.
+// On the first resolution failure it returns nil and an error that
+// identifies the offending key; the inner resolver error is already
+// sanitized by ResolveValue and is wrapped with %w so errors.Is/As
+// continues to work.
+//
+// Empty resolved values are kept ("FOO=" is a legitimate request;
+// opt out via ${VAR:+...}), matching MCPConfig.ResolvedEnv; see
+// PLAN.md Phase 2 design decision #18.
+//
+// Shape note: this returns map[string]string rather than the []string
+// shape MCPConfig.ResolvedEnv uses because the consumer
+// (powernap.ClientConfig.Environment in internal/lsp/client.go) takes
+// a map directly — returning a []string here would only force a
+// round-trip back to a map at the call site. See PLAN.md Phase 2
+// design decision #13.
+//
+// See ResolvedArgs for guidance on picking a resolver.
+func (l LSPConfig) ResolvedEnv(r VariableResolver) (map[string]string, error) {
+ if len(l.Env) == 0 {
+ return map[string]string{}, nil
+ }
+ out := make(map[string]string, len(l.Env))
+ // Sort keys so failures are reported deterministically when more
+ // than one value would fail.
+ keys := make([]string, 0, len(l.Env))
+ for k := range l.Env {
+ keys = append(keys, k)
+ }
+ slices.Sort(keys)
+ for _, k := range keys {
+ v, err := r.ResolveValue(l.Env[k])
+ if err != nil {
+ return nil, fmt.Errorf("env %q: %w", k, err)
+ }
+ out[k] = v
+ }
+ return out, nil
+}
+
type Agent struct {
ID string `json:"id,omitempty"`
Name string `json:"name,omitempty"`
@@ -359,6 +359,118 @@ func TestResolvedArgs_RealShell(t *testing.T) {
})
}
+// TestLSPConfig_ResolvedArgs_RealShell exercises both success and
+// failure for l.Args symmetrically with MCP args. Args are ordered so
+// error messages must identify a positional index, not a key.
+func TestLSPConfig_ResolvedArgs_RealShell(t *testing.T) {
+ t.Parallel()
+
+ t.Run("success expands $VAR in each element", func(t *testing.T) {
+ t.Parallel()
+ l := LSPConfig{Args: []string{"--root", "$HOME", "--flag", "literal"}}
+ r := realShellResolver(map[string]string{"HOME": "/home/tester"})
+ got, err := l.ResolvedArgs(r)
+ require.NoError(t, err)
+ require.Equal(t, []string{"--root", "/home/tester", "--flag", "literal"}, got)
+ })
+
+ t.Run("failure identifies offending index", func(t *testing.T) {
+ t.Parallel()
+ l := LSPConfig{Args: []string{"--root", "$(false)"}}
+ orig := slices.Clone(l.Args)
+
+ got, err := l.ResolvedArgs(realShellResolver(nil))
+ require.Error(t, err)
+ require.Nil(t, got)
+ require.Contains(t, err.Error(), "arg 1")
+ require.Equal(t, orig, l.Args, "receiver Args must be preserved")
+ })
+
+ t.Run("nil args returns nil, no error", func(t *testing.T) {
+ t.Parallel()
+ l := LSPConfig{}
+ got, err := l.ResolvedArgs(realShellResolver(nil))
+ require.NoError(t, err)
+ require.Nil(t, got)
+ })
+}
+
+// TestLSPConfig_ResolvedEnv_RealShell pins the LSP env contract:
+// success expands $VAR, failure wraps with the key name, and the
+// receiver map is never mutated. The shape is map[string]string
+// (not the MCP []string form) because powernap.ClientConfig.Environment
+// takes a map directly.
+func TestLSPConfig_ResolvedEnv_RealShell(t *testing.T) {
+ t.Parallel()
+
+ t.Run("success expands $VAR", func(t *testing.T) {
+ t.Parallel()
+ l := LSPConfig{Env: map[string]string{"GOPATH": "$HOME/go"}}
+ r := realShellResolver(map[string]string{"HOME": "/home/tester"})
+ got, err := l.ResolvedEnv(r)
+ require.NoError(t, err)
+ require.Equal(t, map[string]string{"GOPATH": "/home/tester/go"}, got)
+ })
+
+ t.Run("failure identifies offending key", func(t *testing.T) {
+ t.Parallel()
+ l := LSPConfig{Env: map[string]string{
+ "GOPATH": "$(false)",
+ "OTHER": "literal",
+ }}
+ orig := maps.Clone(l.Env)
+
+ got, err := l.ResolvedEnv(realShellResolver(nil))
+ require.Error(t, err)
+ require.Nil(t, got)
+ require.Contains(t, err.Error(), `env "GOPATH"`)
+ require.Equal(t, orig, l.Env, "receiver Env must be preserved")
+ })
+
+ t.Run("idempotent and non-mutating", func(t *testing.T) {
+ t.Parallel()
+ l := LSPConfig{Env: map[string]string{
+ "A": "$(echo one)",
+ "B": "literal",
+ }}
+ orig := maps.Clone(l.Env)
+ r := realShellResolver(nil)
+
+ first, err := l.ResolvedEnv(r)
+ require.NoError(t, err)
+ second, err := l.ResolvedEnv(r)
+ require.NoError(t, err)
+ require.Equal(t, first, second)
+ require.Equal(t, orig, l.Env, "receiver Env must be preserved")
+ })
+}
+
+// TestLSPConfig_IdentityResolver pins the client-mode contract: both
+// ResolvedArgs and ResolvedEnv round-trip the template verbatim under
+// IdentityResolver and never error on unset variables. Local
+// expansion would double-expand when the server does its own — this
+// has to stay a pure pass-through.
+func TestLSPConfig_IdentityResolver(t *testing.T) {
+ t.Parallel()
+
+ l := LSPConfig{
+ Args: []string{"--root", "$LSP_ROOT", "$(vault read -f lsp)"},
+ Env: map[string]string{
+ "GOPATH": "$HOME/go",
+ "TOKEN": "$(cat /run/secrets/x)",
+ },
+ }
+ r := IdentityResolver()
+
+ args, err := l.ResolvedArgs(r)
+ require.NoError(t, err)
+ require.Equal(t, l.Args, args)
+
+ envs, err := l.ResolvedEnv(r)
+ require.NoError(t, err)
+ require.Equal(t, l.Env, envs)
+}
+
// TestMCPConfig_IdentityResolver pins the client-mode contract: every
// Resolved* method round-trips the template verbatim and never errors
// on unset variables. Local expansion would double-expand when the
@@ -5,7 +5,6 @@ import (
"encoding/json"
"fmt"
"log/slog"
- "maps"
"os"
"path/filepath"
"slices"
@@ -169,11 +168,21 @@ func (c *Client) createPowernapClient() error {
return fmt.Errorf("invalid lsp command: %w", err)
}
+ args, err := c.config.ResolvedArgs(c.resolver)
+ if err != nil {
+ return fmt.Errorf("invalid lsp args: %w", err)
+ }
+
+ envs, err := c.config.ResolvedEnv(c.resolver)
+ if err != nil {
+ return fmt.Errorf("invalid lsp env: %w", err)
+ }
+
clientConfig := powernap.ClientConfig{
Command: home.Long(command),
- Args: c.config.Args,
+ Args: args,
RootURI: rootURI,
- Environment: maps.Clone(c.config.Env),
+ Environment: envs,
Settings: c.config.Options,
InitOptions: c.config.InitOptions,
WorkspaceFolders: []protocol.WorkspaceFolder{
@@ -61,6 +61,42 @@ func TestClient(t *testing.T) {
}
}
+// TestNew_ExpansionFailure_Args pins that a failing $(cmd) in LSP
+// args surfaces as a load error prefixed "invalid lsp args:" and that
+// no client is returned. Mirrors the MCP contract where expansion
+// failure hard-stops transport creation rather than silently running
+// with an empty or literal value.
+func TestNew_ExpansionFailure_Args(t *testing.T) {
+ t.Parallel()
+
+ cfg := config.LSPConfig{
+ Command: "echo",
+ Args: []string{"--root", "$(false)"},
+ }
+ resolver := config.NewShellVariableResolver(env.NewFromMap(map[string]string{}))
+
+ client, err := New(t.Context(), "test-args-fail", cfg, resolver, ".", false)
+ require.Error(t, err)
+ require.Nil(t, client, "client must not start when args expansion fails")
+ require.Contains(t, err.Error(), "invalid lsp args")
+}
+
+// TestNew_ExpansionFailure_Env pins the same contract for env values.
+func TestNew_ExpansionFailure_Env(t *testing.T) {
+ t.Parallel()
+
+ cfg := config.LSPConfig{
+ Command: "echo",
+ Env: map[string]string{"BAD": "$(false)"},
+ }
+ resolver := config.NewShellVariableResolver(env.NewFromMap(map[string]string{}))
+
+ client, err := New(t.Context(), "test-env-fail", cfg, resolver, ".", false)
+ require.Error(t, err)
+ require.Nil(t, client, "client must not start when env expansion fails")
+ require.Contains(t, err.Error(), "invalid lsp env")
+}
+
func TestNilClient(t *testing.T) {
t.Parallel()