@@ -2,6 +2,8 @@ package mcp
22
33import (
44 "context"
5+ "maps"
6+ "os"
57 "testing"
68
79 "github.com/charmbracelet/crush/internal/config"
@@ -11,6 +13,17 @@ import (
1113 "go.uber.org/goleak"
1214)
1315
16+// shellResolverWithPath builds a shell resolver whose env carries PATH
17+// plus any caller-supplied overrides. Without PATH, $(cat), $(echo),
18+// etc. can't find their binaries in a test process where the shell env
19+// is otherwise empty.
20+func shellResolverWithPath(t *testing.T, overrides map[string]string) config.VariableResolver {
21+ t.Helper()
22+ m := map[string]string{"PATH": os.Getenv("PATH")}
23+ maps.Copy(m, overrides)
24+ return config.NewShellVariableResolver(env.NewFromMap(m))
25+}
26+
1427func TestMCPSession_CancelOnClose(t *testing.T) {
1528 defer goleak.VerifyNone(t)
1629
@@ -132,3 +145,285 @@ func TestCreateTransport_URLResolution(t *testing.T) {
132145 require.Equal(t, tmpl, sct.Endpoint)
133146 })
134147}
148+
149+// TestCreateTransport_StdioResolution pins that command, args, and env
150+// for stdio MCPs go through the same resolver seam as the other
151+// transports. Covers both success (expansion produced the expected
152+// exec.Cmd) and failure (any one field erroring prevents transport
153+// creation).
154+func TestCreateTransport_StdioResolution(t *testing.T) {
155+ t.Parallel()
156+
157+ t.Run("success expands command, args, and env", func(t *testing.T) {
158+ t.Parallel()
159+ r := shellResolverWithPath(t, map[string]string{
160+ "MY_TOKEN": "hunter2",
161+ })
162+ m := config.MCPConfig{
163+ Type: config.MCPStdio,
164+ Command: "forgejo-mcp",
165+ Args: []string{"--token", "$MY_TOKEN", "--host", "$(echo example.com)"},
166+ Env: map[string]string{
167+ "SECRET": "$(echo shh)",
168+ "PLAIN": "literal",
169+ "REFERENCE": "$MY_TOKEN",
170+ },
171+ }
172+ tr, err := createTransport(t.Context(), m, r)
173+ require.NoError(t, err)
174+ require.NotNil(t, tr)
175+
176+ ct, ok := tr.(*mcp.CommandTransport)
177+ require.True(t, ok, "expected CommandTransport, got %T", tr)
178+
179+ // exec.Cmd.Args[0] is the command name; the rest are positional
180+ // args as passed.
181+ require.Equal(t, []string{"forgejo-mcp", "--token", "hunter2", "--host", "example.com"}, ct.Command.Args)
182+
183+ // Env is os.Environ() + resolved entries (sorted). Check the
184+ // resolved entries are present with their expanded values.
185+ require.Contains(t, ct.Command.Env, "SECRET=shh")
186+ require.Contains(t, ct.Command.Env, "PLAIN=literal")
187+ require.Contains(t, ct.Command.Env, "REFERENCE=hunter2")
188+ })
189+
190+ t.Run("env resolution failure surfaces error, no transport created", func(t *testing.T) {
191+ t.Parallel()
192+ r := shellResolverWithPath(t, nil)
193+ m := config.MCPConfig{
194+ Type: config.MCPStdio,
195+ Command: "forgejo-mcp",
196+ Env: map[string]string{"TOKEN": "$(false)"},
197+ }
198+ tr, err := createTransport(t.Context(), m, r)
199+ require.Error(t, err)
200+ require.Nil(t, tr)
201+ require.Contains(t, err.Error(), "env TOKEN")
202+ })
203+
204+ t.Run("unset env var is a hard error, not silent empty", func(t *testing.T) {
205+ t.Parallel()
206+ // The regression at the heart of PLAN.md: unset vars used to
207+ // silently substitute "" and hand an empty credential to the
208+ // child process. Now they must error out before exec.
209+ r := shellResolverWithPath(t, nil)
210+ m := config.MCPConfig{
211+ Type: config.MCPStdio,
212+ Command: "forgejo-mcp",
213+ Env: map[string]string{"FORGEJO_ACCESS_TOKEN": "$FORGJO_TOKEN"},
214+ }
215+ tr, err := createTransport(t.Context(), m, r)
216+ require.Error(t, err)
217+ require.Nil(t, tr)
218+ require.Contains(t, err.Error(), "env FORGEJO_ACCESS_TOKEN")
219+ })
220+
221+ t.Run("args resolution failure surfaces error, no transport created", func(t *testing.T) {
222+ t.Parallel()
223+ r := shellResolverWithPath(t, nil)
224+ m := config.MCPConfig{
225+ Type: config.MCPStdio,
226+ Command: "forgejo-mcp",
227+ Args: []string{"--token", "$(false)"},
228+ }
229+ tr, err := createTransport(t.Context(), m, r)
230+ require.Error(t, err)
231+ require.Nil(t, tr)
232+ require.Contains(t, err.Error(), "arg 1")
233+ })
234+
235+ t.Run("command resolution failure surfaces error, no transport created", func(t *testing.T) {
236+ t.Parallel()
237+ r := shellResolverWithPath(t, nil)
238+ m := config.MCPConfig{
239+ Type: config.MCPStdio,
240+ Command: "$(false)",
241+ }
242+ tr, err := createTransport(t.Context(), m, r)
243+ require.Error(t, err)
244+ require.Nil(t, tr)
245+ require.Contains(t, err.Error(), "invalid mcp command")
246+ })
247+
248+ t.Run("identity resolver round-trips templates verbatim", func(t *testing.T) {
249+ t.Parallel()
250+ // Client mode: no local expansion, no error on unset vars.
251+ m := config.MCPConfig{
252+ Type: config.MCPStdio,
253+ Command: "forgejo-mcp",
254+ Args: []string{"--token", "$MCP_MISSING"},
255+ Env: map[string]string{"TOKEN": "$(vault read -f token)"},
256+ }
257+ tr, err := createTransport(t.Context(), m, config.IdentityResolver())
258+ require.NoError(t, err)
259+ ct, ok := tr.(*mcp.CommandTransport)
260+ require.True(t, ok)
261+ require.Equal(t, []string{"forgejo-mcp", "--token", "$MCP_MISSING"}, ct.Command.Args)
262+ require.Contains(t, ct.Command.Env, "TOKEN=$(vault read -f token)")
263+ })
264+}
265+
266+// TestCreateTransport_HeadersResolution pins that a single failing
267+// header aborts HTTP/SSE transport creation and that the successful
268+// resolver passes every expanded header through to the round tripper.
269+func TestCreateTransport_HeadersResolution(t *testing.T) {
270+ t.Parallel()
271+
272+ t.Run("http headers success expands $(cmd)", func(t *testing.T) {
273+ t.Parallel()
274+ r := shellResolverWithPath(t, map[string]string{
275+ "GITHUB_TOKEN": "gh-secret",
276+ })
277+ m := config.MCPConfig{
278+ Type: config.MCPHttp,
279+ URL: "https://mcp.example.com/api",
280+ Headers: map[string]string{
281+ "Authorization": "$(echo Bearer $GITHUB_TOKEN)",
282+ "X-Static": "kept",
283+ },
284+ }
285+ tr, err := createTransport(t.Context(), m, r)
286+ require.NoError(t, err)
287+
288+ sct, ok := tr.(*mcp.StreamableClientTransport)
289+ require.True(t, ok)
290+ rt, ok := sct.HTTPClient.Transport.(*headerRoundTripper)
291+ require.True(t, ok, "expected headerRoundTripper, got %T", sct.HTTPClient.Transport)
292+ require.Equal(t, map[string]string{
293+ "Authorization": "Bearer gh-secret",
294+ "X-Static": "kept",
295+ }, rt.headers)
296+ })
297+
298+ t.Run("http failing header surfaces error, no transport", func(t *testing.T) {
299+ t.Parallel()
300+ r := shellResolverWithPath(t, nil)
301+ m := config.MCPConfig{
302+ Type: config.MCPHttp,
303+ URL: "https://mcp.example.com/api",
304+ Headers: map[string]string{"Authorization": "$(false)"},
305+ }
306+ tr, err := createTransport(t.Context(), m, r)
307+ require.Error(t, err)
308+ require.Nil(t, tr)
309+ require.Contains(t, err.Error(), "header Authorization")
310+ })
311+
312+ t.Run("sse failing header surfaces error, no transport", func(t *testing.T) {
313+ t.Parallel()
314+ r := shellResolverWithPath(t, nil)
315+ m := config.MCPConfig{
316+ Type: config.MCPSSE,
317+ URL: "https://mcp.example.com/events",
318+ Headers: map[string]string{"Authorization": "Bearer $MISSING_TOKEN"},
319+ }
320+ tr, err := createTransport(t.Context(), m, r)
321+ require.Error(t, err)
322+ require.Nil(t, tr)
323+ require.Contains(t, err.Error(), "header Authorization")
324+ require.Contains(t, err.Error(), "$MISSING_TOKEN")
325+ })
326+}
327+
328+// TestCreateSession_ResolutionFailureUpdatesState pins the user-visible
329+// half of the regression fix: when any of command/args/env/headers/url
330+// fails to resolve, createSession must publish StateError to the state
331+// map so crush_info and the TUI's MCP status card can render a real
332+// error instead of the MCP silently sitting in "starting" or being
333+// spawned with an empty credential.
334+//
335+// These subtests cannot run in parallel: `states` is a package-level
336+// csync.Map and each assertion reads the entry written by the call
337+// under test. They do use unique MCP names per subtest to keep them
338+// independent regardless of ordering.
339+func TestCreateSession_ResolutionFailureUpdatesState(t *testing.T) {
340+ r := shellResolverWithPath(t, nil)
341+
342+ tests := []struct {
343+ name string
344+ mcpName string
345+ cfg config.MCPConfig
346+ wantErrContains string
347+ }{
348+ {
349+ name: "stdio env failure",
350+ mcpName: "test-stdio-env-fail",
351+ cfg: config.MCPConfig{
352+ Type: config.MCPStdio,
353+ Command: "echo",
354+ Env: map[string]string{"FORGEJO_ACCESS_TOKEN": "$(false)"},
355+ },
356+ wantErrContains: "env FORGEJO_ACCESS_TOKEN",
357+ },
358+ {
359+ name: "stdio args failure",
360+ mcpName: "test-stdio-args-fail",
361+ cfg: config.MCPConfig{
362+ Type: config.MCPStdio,
363+ Command: "echo",
364+ Args: []string{"--token", "$MCP_UNSET_TOKEN"},
365+ },
366+ wantErrContains: "arg 1",
367+ },
368+ {
369+ name: "http url failure",
370+ mcpName: "test-http-url-fail",
371+ cfg: config.MCPConfig{
372+ Type: config.MCPHttp,
373+ URL: "https://$MCP_MISSING_HOST/api",
374+ },
375+ wantErrContains: "url:",
376+ },
377+ {
378+ name: "http header failure",
379+ mcpName: "test-http-header-fail",
380+ cfg: config.MCPConfig{
381+ Type: config.MCPHttp,
382+ URL: "https://mcp.example.com/api",
383+ Headers: map[string]string{"Authorization": "$(false)"},
384+ },
385+ wantErrContains: "header Authorization",
386+ },
387+ {
388+ name: "sse url failure",
389+ mcpName: "test-sse-url-fail",
390+ cfg: config.MCPConfig{
391+ Type: config.MCPSSE,
392+ URL: "https://$(false)/events",
393+ },
394+ wantErrContains: "url:",
395+ },
396+ {
397+ name: "sse header failure",
398+ mcpName: "test-sse-header-fail",
399+ cfg: config.MCPConfig{
400+ Type: config.MCPSSE,
401+ URL: "https://mcp.example.com/events",
402+ Headers: map[string]string{"Authorization": "Bearer $MISSING_SSE_TOKEN"},
403+ },
404+ wantErrContains: "header Authorization",
405+ },
406+ }
407+
408+ for _, tc := range tests {
409+ t.Run(tc.name, func(t *testing.T) {
410+ // Guarantee a clean slate on the shared state map so a
411+ // stale entry from another test can't satisfy the
412+ // assertion.
413+ states.Del(tc.mcpName)
414+ t.Cleanup(func() { states.Del(tc.mcpName) })
415+
416+ sess, err := createSession(t.Context(), tc.mcpName, tc.cfg, r)
417+ require.Error(t, err)
418+ require.Nil(t, sess)
419+ require.Contains(t, err.Error(), tc.wantErrContains)
420+
421+ info, ok := GetState(tc.mcpName)
422+ require.True(t, ok, "state entry must be written for %q", tc.mcpName)
423+ require.Equal(t, StateError, info.State, "expected StateError, got %s", info.State)
424+ require.Error(t, info.Error, "state must carry the failure error")
425+ require.Contains(t, info.Error.Error(), tc.wantErrContains)
426+ require.Nil(t, info.Client, "no client session on failure")
427+ })
428+ }
429+}