git.iko.soy/crush
on main help sign in
test: cover shell expansion in MCP config resolution
240e5c2Christian Rocha
internal/agent/tools/mcp/init_test.go+295
compare with main
unifiedsplit
@@ -2,6 +2,8 @@ package mcp
2
2
3import (
3import (
4 "context"
4 "context"
5+ "maps"
6+ "os"
5 "testing"
7 "testing"
6
8
7 "github.com/charmbracelet/crush/internal/config"
9 "github.com/charmbracelet/crush/internal/config"
@@ -11,6 +13,17 @@ import (
11 "go.uber.org/goleak"
13 "go.uber.org/goleak"
12)
14)
13
15
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+
14func TestMCPSession_CancelOnClose(t *testing.T) {
27func TestMCPSession_CancelOnClose(t *testing.T) {
15 defer goleak.VerifyNone(t)
28 defer goleak.VerifyNone(t)
16
29
@@ -132,3 +145,285 @@ func TestCreateTransport_URLResolution(t *testing.T) {
132 require.Equal(t, tmpl, sct.Endpoint)
145 require.Equal(t, tmpl, sct.Endpoint)
133 })
146 })
134}
147}
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+}
internal/config/load.go+6
@@ -222,6 +222,12 @@ func (c *Config) configureProviders(store *ConfigStore, env env.Env, resolver Va
222 if len(config.ExtraHeaders) > 0 {
222 if len(config.ExtraHeaders) > 0 {
223 maps.Copy(headers, config.ExtraHeaders)
223 maps.Copy(headers, config.ExtraHeaders)
224 }
224 }
225+ // Intentional divergence from MCP env/headers/args/url resolution:
226+ // provider headers that fail to resolve log and keep their literal
227+ // template so providers with optional, env-gated headers still
228+ // load on hosts where those vars are unset. Changing this to a
229+ // hard error would break those configs. See PLAN.md "Design
230+ // decisions" item 4 for the full rationale.
225 for k, v := range headers {
231 for k, v := range headers {
226 resolved, err := resolver.ResolveValue(v)
232 resolved, err := resolver.ResolveValue(v)
227 if err != nil {
233 if err != nil {
internal/config/load_test.go+58
@@ -1589,3 +1589,61 @@ func TestConfig_configureProviders_HyperAPIKeyFromConfigOverrides(t *testing.T)
1589 require.True(t, ok, "Hyper provider should be configured")
1589 require.True(t, ok, "Hyper provider should be configured")
1590 require.Equal(t, "env-api-key", pc.APIKey)
1590 require.Equal(t, "env-api-key", pc.APIKey)
1591}
1591}
1592+
1593+// TestConfig_configureProviders_ProviderHeaderResolveFailure pins the
1594+// intentional divergence at load.go:225: a provider header whose
1595+// expansion fails must NOT fail the whole config load. It logs an
1596+// error, keeps the literal template in the resolved header map, and
1597+// moves on. The MCP contract (hard error on failed expansion) does not
1598+// apply here because many providers ship DefaultHeaders that reference
1599+// env vars which are legitimately unset on most hosts.
1600+//
1601+// If this test ever flips to requiring an error, read PLAN.md "Design
1602+// decisions" item 4 before changing the production code — the
1603+// divergence is deliberate.
1604+func TestConfig_configureProviders_ProviderHeaderResolveFailure(t *testing.T) {
1605+ knownProviders := []catwalk.Provider{
1606+ {
1607+ ID: "openai",
1608+ APIKey: "$OPENAI_API_KEY",
1609+ APIEndpoint: "https://api.openai.com/v1",
1610+ Models: []catwalk.Model{{ID: "test-model"}},
1611+ },
1612+ }
1613+
1614+ cfg := &Config{
1615+ Providers: csync.NewMapFrom(map[string]ProviderConfig{
1616+ "openai": {
1617+ ExtraHeaders: map[string]string{
1618+ // Failing $(...) — inner command exits 1, no stdout.
1619+ "X-Broken": "$(false)",
1620+ // Unset var — nounset makes this an error too.
1621+ "X-Missing": "$PROVIDER_HEADER_NEVER_SET",
1622+ // Happy path: must still be resolved, proving the
1623+ // failure in the other two did not abort the loop.
1624+ "X-Static": "kept-literal",
1625+ },
1626+ },
1627+ }),
1628+ }
1629+ cfg.setDefaults("/tmp", "")
1630+
1631+ testEnv := env.NewFromMap(map[string]string{
1632+ "OPENAI_API_KEY": "test-key",
1633+ "PATH": os.Getenv("PATH"),
1634+ })
1635+ resolver := NewShellVariableResolver(testEnv)
1636+
1637+ err := cfg.configureProviders(testStore(cfg), testEnv, resolver, knownProviders)
1638+ require.NoError(t, err, "provider load must tolerate failing header expansion")
1639+
1640+ pc, ok := cfg.Providers.Get("openai")
1641+ require.True(t, ok, "openai provider must still be configured")
1642+
1643+ // Literal template preserved for the two failing headers; the
1644+ // happy-path header is resolved through the shell (pass-through
1645+ // for a literal value).
1646+ require.Equal(t, "$(false)", pc.ExtraHeaders["X-Broken"])
1647+ require.Equal(t, "$PROVIDER_HEADER_NEVER_SET", pc.ExtraHeaders["X-Missing"])
1648+ require.Equal(t, "kept-literal", pc.ExtraHeaders["X-Static"])
1649+}
internal/config/resolve_real_test.gonew file+294
@@ -0,0 +1,294 @@
1+package config
2+
3+import (
4+ "fmt"
5+ "maps"
6+ "os"
7+ "path/filepath"
8+ "slices"
9+ "testing"
10+
11+ "github.com/charmbracelet/crush/internal/env"
12+ "github.com/stretchr/testify/require"
13+)
14+
15+// These tests exercise the full shell-expansion path (no mocks,
16+// no injected Expander) to catch regressions that only surface when
17+// internal/shell actually runs the value. Table-level unit tests with
18+// fake expanders live in resolve_test.go.
19+
20+// realShellResolver builds a resolver backed by a shell env that
21+// contains PATH + the caller-supplied overrides. Production callers
22+// get PATH for free via env.New(); these tests need it so $(cat ...)
23+// and similar inner commands can resolve.
24+func realShellResolver(vars map[string]string) VariableResolver {
25+ m := map[string]string{"PATH": os.Getenv("PATH")}
26+ maps.Copy(m, vars)
27+ return NewShellVariableResolver(env.NewFromMap(m))
28+}
29+
30+func writeTempFile(t *testing.T, content string) string {
31+ t.Helper()
32+ dir := t.TempDir()
33+ p := filepath.Join(dir, "secret")
34+ require.NoError(t, os.WriteFile(p, []byte(content), 0o600))
35+ return p
36+}
37+
38+// TestResolvedEnv_RealShell_Success covers the shell constructs the
39+// PLAN calls out: $(cat tempfile) with and without trailing newline,
40+// ${VAR:-default} for unset vars, literal-space preservation around
41+// $(...), nested parens, quoted args inside $(echo ...), and a
42+// glob-like literal round-tripping unchanged.
43+func TestResolvedEnv_RealShell_Success(t *testing.T) {
44+ t.Parallel()
45+
46+ withNL := writeTempFile(t, "token-with-nl\n")
47+ noNL := writeTempFile(t, "token-no-nl")
48+
49+ m := MCPConfig{
50+ Env: map[string]string{
51+ // POSIX strips exactly one trailing newline from $(...)
52+ // output, so both forms land on the same value.
53+ "TOK_NL": fmt.Sprintf("$(cat %s)", withNL),
54+ "TOK_NO": fmt.Sprintf("$(cat %s)", noNL),
55+
56+ // ${VAR:-default} must not error on unset: this is the
57+ // opt-in escape hatch for "empty is fine".
58+ "FALLBACK": "${MCP_MISSING:-fallback}",
59+
60+ // Leading/trailing literal spaces around $(...) must be
61+ // preserved — single-value contract, no field splitting.
62+ "PADDED": " $(echo v) ",
63+
64+ // ")" inside a quoted arg to echo is a regression guard
65+ // for the old hand-rolled paren matcher.
66+ "PAREN": `$(echo ")")`,
67+
68+ // Embedded space inside a quoted arg must survive
69+ // verbatim; no word-splitting side effect.
70+ "SPACEY": `$(echo "a b")`,
71+
72+ // Glob-like literals must not expand.
73+ "GLOB": "*.go",
74+ },
75+ }
76+
77+ got, err := m.ResolvedEnv(realShellResolver(nil))
78+ require.NoError(t, err)
79+
80+ // ResolvedEnv returns "KEY=value" sorted by key.
81+ want := []string{
82+ "FALLBACK=fallback",
83+ "GLOB=*.go",
84+ "PADDED= v ",
85+ "PAREN=)",
86+ "SPACEY=a b",
87+ "TOK_NL=token-with-nl",
88+ "TOK_NO=token-no-nl",
89+ }
90+ require.Equal(t, want, got)
91+}
92+
93+// TestResolvedEnv_RealShell_DoesNotMutate pins that both success and
94+// failure paths leave m.Env untouched. Prior behaviour rewrote the
95+// value in place on error; that was the exact mechanism that shipped
96+// empty credentials to MCP servers.
97+func TestResolvedEnv_RealShell_DoesNotMutate(t *testing.T) {
98+ t.Parallel()
99+
100+ t.Run("success path leaves Env untouched", func(t *testing.T) {
101+ t.Parallel()
102+ m := MCPConfig{Env: map[string]string{"TOKEN": "$(echo shh)"}}
103+ orig := maps.Clone(m.Env)
104+
105+ _, err := m.ResolvedEnv(realShellResolver(nil))
106+ require.NoError(t, err)
107+ require.Equal(t, orig, m.Env)
108+ })
109+
110+ t.Run("failure path leaves Env untouched", func(t *testing.T) {
111+ t.Parallel()
112+ m := MCPConfig{Env: map[string]string{"BROKEN": "$(false)"}}
113+ orig := maps.Clone(m.Env)
114+
115+ _, err := m.ResolvedEnv(realShellResolver(nil))
116+ require.Error(t, err)
117+ require.Equal(t, orig, m.Env, "map must be preserved on error")
118+ })
119+}
120+
121+// TestResolvedEnv_RealShell_Idempotent pins the pure-function contract:
122+// two calls on the same config return deeply-equal slices.
123+func TestResolvedEnv_RealShell_Idempotent(t *testing.T) {
124+ t.Parallel()
125+
126+ m := MCPConfig{
127+ Env: map[string]string{
128+ "A": "$(echo one)",
129+ "B": "$(echo two)",
130+ "C": "literal",
131+ },
132+ }
133+ r := realShellResolver(nil)
134+
135+ first, err := m.ResolvedEnv(r)
136+ require.NoError(t, err)
137+ second, err := m.ResolvedEnv(r)
138+ require.NoError(t, err)
139+ require.Equal(t, first, second)
140+}
141+
142+// TestResolvedEnv_RealShell_Deterministic guards against Go's
143+// randomized map iteration leaking into the returned slice order.
144+func TestResolvedEnv_RealShell_Deterministic(t *testing.T) {
145+ t.Parallel()
146+
147+ m := MCPConfig{Env: map[string]string{
148+ "Z": "z",
149+ "A": "a",
150+ "M": "m",
151+ }}
152+
153+ got, err := m.ResolvedEnv(realShellResolver(nil))
154+ require.NoError(t, err)
155+ require.True(t, slices.IsSorted(got), "env slice must be sorted; got %v", got)
156+}
157+
158+// TestResolvedEnv_RealShell_NounsetRegression is the single most
159+// important assertion in this file: an unset variable is an error, not
160+// an empty expansion. Swapping the hand-rolled parser for mvdan's
161+// default-expansion behaviour without nounset would re-introduce
162+// Defect 1 via a typo'd variable name.
163+func TestResolvedEnv_RealShell_NounsetRegression(t *testing.T) {
164+ t.Parallel()
165+
166+ m := MCPConfig{Env: map[string]string{
167+ // Intentional typo: user meant $FORGEJO_TOKEN.
168+ "FORGEJO_ACCESS_TOKEN": "$FORGJO_TOKEN",
169+ }}
170+ got, err := m.ResolvedEnv(realShellResolver(nil))
171+ require.Error(t, err, "unset var must not silently expand to empty")
172+ require.Nil(t, got)
173+ require.Contains(t, err.Error(), "FORGEJO_ACCESS_TOKEN")
174+ require.Contains(t, err.Error(), "$FORGJO_TOKEN")
175+}
176+
177+// TestResolvedEnv_RealShell_FailureDetail pins that a failing inner
178+// command surfaces enough detail (exit code + stderr) to diagnose
179+// without forcing the user to re-run the command by hand. Also
180+// verifies the template is included so they know which Env entry
181+// blew up.
182+func TestResolvedEnv_RealShell_FailureDetail(t *testing.T) {
183+ t.Parallel()
184+
185+ missing := filepath.Join(t.TempDir(), "definitely-not-here")
186+ m := MCPConfig{Env: map[string]string{
187+ "FORGEJO_ACCESS_TOKEN": fmt.Sprintf("$(cat %s)", missing),
188+ }}
189+
190+ _, err := m.ResolvedEnv(realShellResolver(nil))
191+ require.Error(t, err)
192+ msg := err.Error()
193+ require.Contains(t, msg, "FORGEJO_ACCESS_TOKEN", "must identify the failing env var")
194+ require.Contains(t, msg, missing, "must include the template so users see what failed")
195+ require.Contains(t, msg, "exit status", "must surface the inner exit code")
196+}
197+
198+// TestResolvedHeaders_RealShell_FailurePreservesOriginal pins two
199+// invariants simultaneously: on failure the returned map is nil (not
200+// a partially-populated map) and the receiver's Headers map is
201+// unchanged. A test that only asserted on the returned value could
202+// hide an in-place mutation regression.
203+func TestResolvedHeaders_RealShell_FailurePreservesOriginal(t *testing.T) {
204+ t.Parallel()
205+
206+ m := MCPConfig{Headers: map[string]string{
207+ "Authorization": "Bearer $(false)",
208+ "X-Static": "kept",
209+ }}
210+ orig := maps.Clone(m.Headers)
211+
212+ got, err := m.ResolvedHeaders(realShellResolver(nil))
213+ require.Error(t, err)
214+ require.Nil(t, got, "headers map must be nil on failure")
215+ require.Contains(t, err.Error(), "header Authorization")
216+ require.Equal(t, orig, m.Headers, "receiver Headers must be preserved")
217+}
218+
219+// TestResolvedArgs_RealShell exercises both success and failure for
220+// m.Args symmetrically with Env. Args are ordered so error messages
221+// must identify a positional index, not a key.
222+func TestResolvedArgs_RealShell(t *testing.T) {
223+ t.Parallel()
224+
225+ t.Run("success expands each element", func(t *testing.T) {
226+ t.Parallel()
227+ m := MCPConfig{Args: []string{"--token", "$(echo shh)", "--host", "example.com"}}
228+ got, err := m.ResolvedArgs(realShellResolver(nil))
229+ require.NoError(t, err)
230+ require.Equal(t, []string{"--token", "shh", "--host", "example.com"}, got)
231+ })
232+
233+ t.Run("failure identifies offending index", func(t *testing.T) {
234+ t.Parallel()
235+ m := MCPConfig{Args: []string{"--token", "$(false)"}}
236+ orig := slices.Clone(m.Args)
237+
238+ got, err := m.ResolvedArgs(realShellResolver(nil))
239+ require.Error(t, err)
240+ require.Nil(t, got)
241+ require.Contains(t, err.Error(), "arg 1")
242+ require.Equal(t, orig, m.Args, "receiver Args must be preserved")
243+ })
244+
245+ t.Run("nil args returns nil, no error", func(t *testing.T) {
246+ t.Parallel()
247+ m := MCPConfig{}
248+ got, err := m.ResolvedArgs(realShellResolver(nil))
249+ require.NoError(t, err)
250+ require.Nil(t, got)
251+ })
252+}
253+
254+// TestMCPConfig_IdentityResolver pins the client-mode contract: every
255+// Resolved* method round-trips the template verbatim and never errors
256+// on unset variables. Local expansion would double-expand when the
257+// server does its own — this has to stay a pure pass-through.
258+func TestMCPConfig_IdentityResolver(t *testing.T) {
259+ t.Parallel()
260+
261+ m := MCPConfig{
262+ Command: "$CMD",
263+ Args: []string{"--token", "$MCP_MISSING_TOKEN", "$(vault read -f secret)"},
264+ Env: map[string]string{
265+ "TOKEN": "$(cat /run/secrets/x)",
266+ "HOST": "$MCP_MISSING_HOST",
267+ },
268+ Headers: map[string]string{
269+ "Authorization": "Bearer $(vault read -f token)",
270+ },
271+ URL: "https://$MCP_HOST/$(vault read -f path)",
272+ }
273+ r := IdentityResolver()
274+
275+ args, err := m.ResolvedArgs(r)
276+ require.NoError(t, err)
277+ require.Equal(t, m.Args, args)
278+
279+ envs, err := m.ResolvedEnv(r)
280+ require.NoError(t, err)
281+ // Sorted "KEY=value".
282+ require.Equal(t, []string{
283+ "HOST=$MCP_MISSING_HOST",
284+ "TOKEN=$(cat /run/secrets/x)",
285+ }, envs)
286+
287+ headers, err := m.ResolvedHeaders(r)
288+ require.NoError(t, err)
289+ require.Equal(t, m.Headers, headers)
290+
291+ u, err := m.ResolvedURL(r)
292+ require.NoError(t, err)
293+ require.Equal(t, m.URL, u)
294+}