@@ -91,17 +91,40 @@ func TestCreateTransport_URLResolution(t *testing.T) {
9191 require.Equal(t, "https://mcp.example.com/events", sse.Endpoint)
9292 })
9393
94− t.Run("http unset var surfaces error, no transport created", func(t *testing.T) {
94+ t.Run("http failing $(cmd) surfaces error, no transport created", func(t *testing.T) {
9595 t.Parallel()
96+ // Under lenient nounset, unset $VAR expands to "" silently,
97+ // so the only way a URL resolution *errors* is a failing
98+ // $(cmd). Mirror the SSE subtest so both transports share
99+ // coverage for the url-resolve-failure path.
96100 m := config.MCPConfig{
97101 Type: config.MCPHttp,
98− URL: "https://$MCP_MISSING_HOST/api",
102+ URL: "https://$(false)/api",
99103 }
100− tr, err := createTransport(t.Context(), m, shell)
104+ tr, err := createTransport(t.Context(), m, shellResolverWithPath(t, nil))
101105 require.Error(t, err)
102106 require.Nil(t, tr)
103107 require.Contains(t, err.Error(), "url:")
104− require.Contains(t, err.Error(), "$MCP_MISSING_HOST")
108+ require.Contains(t, err.Error(), "$(false)")
109+ })
110+
111+ t.Run("http unset var expands empty", func(t *testing.T) {
112+ t.Parallel()
113+ // Pinning test for the new lenient-nounset default: an
114+ // unset bare $VAR in the URL is *not* an error. It
115+ // expands to "" and, here, leaves a syntactically weird
116+ // but non-empty URL that the existing non-empty guard
117+ // still lets through. Guards against a future regression
118+ // that flips strict-by-default back on.
119+ m := config.MCPConfig{
120+ Type: config.MCPHttp,
121+ URL: "https://$MCP_MISSING_HOST/api",
122+ }
123+ tr, err := createTransport(t.Context(), m, shell)
124+ require.NoError(t, err)
125+ sct, ok := tr.(*mcp.StreamableClientTransport)
126+ require.True(t, ok)
127+ require.Equal(t, "https:///api", sct.Endpoint)
105128 })
106129
107130 t.Run("sse failing $(cmd) surfaces error, no transport created", func(t *testing.T) {
@@ -201,16 +224,18 @@ func TestCreateTransport_StdioResolution(t *testing.T) {
201224 require.Contains(t, err.Error(), "env TOKEN")
202225 })
203226
204− t.Run("unset env var is a hard error, not silent empty", func(t *testing.T) {
227+ t.Run("failing env command is a hard error", func(t *testing.T) {
205228 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.
229+ // Under lenient nounset a bare $UNSET expands to ""
230+ // silently — see the pinning subtest below. The remaining
231+ // failure mode for env resolution is a $(cmd) that exits
232+ // non-zero, which must still error out and prevent exec so
233+ // we never hand a broken credential to the child process.
209234 r := shellResolverWithPath(t, nil)
210235 m := config.MCPConfig{
211236 Type: config.MCPStdio,
212237 Command: "forgejo-mcp",
213− Env: map[string]string{"FORGEJO_ACCESS_TOKEN": "$FORGJO_TOKEN"},
238+ Env: map[string]string{"FORGEJO_ACCESS_TOKEN": "$(exit 5)"},
214239 }
215240 tr, err := createTransport(t.Context(), m, r)
216241 require.Error(t, err)
@@ -218,6 +243,28 @@ func TestCreateTransport_StdioResolution(t *testing.T) {
218243 require.Contains(t, err.Error(), "env FORGEJO_ACCESS_TOKEN")
219244 })
220245
246+ t.Run("unset env var expands empty", func(t *testing.T) {
247+ t.Parallel()
248+ // Pinning test for the lenient-nounset default: a bare
249+ // $UNSET in an env value expands to "" without error, and
250+ // the empty entry is kept on the resulting exec.Cmd (env
251+ // entries, unlike headers, are not dropped — see design
252+ // decision #18). Guards against a regression that flips
253+ // strict-by-default back on and silently breaks users
254+ // with configs like FORGEJO_ACCESS_TOKEN=$FORGEJO_TOKEN.
255+ r := shellResolverWithPath(t, nil)
256+ m := config.MCPConfig{
257+ Type: config.MCPStdio,
258+ Command: "forgejo-mcp",
259+ Env: map[string]string{"FORGEJO_ACCESS_TOKEN": "$FORGEJO_TOKEN_UNSET"},
260+ }
261+ tr, err := createTransport(t.Context(), m, r)
262+ require.NoError(t, err)
263+ ct, ok := tr.(*mcp.CommandTransport)
264+ require.True(t, ok)
265+ require.Contains(t, ct.Command.Env, "FORGEJO_ACCESS_TOKEN=")
266+ })
267+
221268 t.Run("args resolution failure surfaces error, no transport created", func(t *testing.T) {
222269 t.Parallel()
223270 r := shellResolverWithPath(t, nil)
@@ -311,17 +358,43 @@ func TestCreateTransport_HeadersResolution(t *testing.T) {
311358
312359 t.Run("sse failing header surfaces error, no transport", func(t *testing.T) {
313360 t.Parallel()
361+ // Under lenient nounset a bare $MISSING expands to "",
362+ // which ResolvedHeaders drops — no error. The failing
363+ // $(cmd) path is the remaining way this can fail loudly;
364+ // cover it on the SSE branch to mirror the HTTP subtest.
314365 r := shellResolverWithPath(t, nil)
315366 m := config.MCPConfig{
316367 Type: config.MCPSSE,
317368 URL: "https://mcp.example.com/events",
318− Headers: map[string]string{"Authorization": "Bearer $MISSING_TOKEN"},
369+ Headers: map[string]string{"Authorization": "$(false)"},
319370 }
320371 tr, err := createTransport(t.Context(), m, r)
321372 require.Error(t, err)
322373 require.Nil(t, tr)
323374 require.Contains(t, err.Error(), "header Authorization")
324− require.Contains(t, err.Error(), "$MISSING_TOKEN")
375+ })
376+
377+ t.Run("sse unset var header drops silently", func(t *testing.T) {
378+ t.Parallel()
379+ // Pinning test for design decision #18 + lenient nounset:
380+ // a header whose value resolves to "" (here because the
381+ // bare $VAR is unset) is omitted from the round tripper
382+ // rather than sent as "X-Header:". Guards against a
383+ // regression that either re-introduces strict-by-default
384+ // or stops dropping empty headers.
385+ r := shellResolverWithPath(t, nil)
386+ m := config.MCPConfig{
387+ Type: config.MCPSSE,
388+ URL: "https://mcp.example.com/events",
389+ Headers: map[string]string{"Authorization": "$MISSING_TOKEN"},
390+ }
391+ tr, err := createTransport(t.Context(), m, r)
392+ require.NoError(t, err)
393+ sse, ok := tr.(*mcp.SSEClientTransport)
394+ require.True(t, ok)
395+ rt, ok := sse.HTTPClient.Transport.(*headerRoundTripper)
396+ require.True(t, ok)
397+ require.NotContains(t, rt.headers, "Authorization")
325398 })
326399}
327400
@@ -356,24 +429,44 @@ func TestCreateSession_ResolutionFailureUpdatesState(t *testing.T) {
356429 wantErrContains: "env FORGEJO_ACCESS_TOKEN",
357430 },
358431 {
432+ // Args that reference an unset bare $VAR no longer
433+ // error out under lenient nounset; the only remaining
434+ // failure mode for arg resolution is a failing $(cmd).
359435 name: "stdio args failure",
360436 mcpName: "test-stdio-args-fail",
361437 cfg: config.MCPConfig{
362438 Type: config.MCPStdio,
363439 Command: "echo",
364− Args: []string{"--token", "$MCP_UNSET_TOKEN"},
440+ Args: []string{"--token", "$(false)"},
365441 },
366442 wantErrContains: "arg 1",
367443 },
368444 {
445+ // Likewise for URL: bare $UNSET expands to ""
446+ // silently, so we need a failing $(cmd) to exercise
447+ // the "url:" wrap from ResolvedURL.
369448 name: "http url failure",
370449 mcpName: "test-http-url-fail",
371450 cfg: config.MCPConfig{
372451 Type: config.MCPHttp,
373− URL: "https://$MCP_MISSING_HOST/api",
452+ URL: "https://$(false)/api",
374453 },
375454 wantErrContains: "url:",
376455 },
456+ {
457+ // A URL whose shell expansion yields the empty
458+ // string (here via ${VAR:-}) is not a ResolvedURL
459+ // error, but the non-empty guard in createTransport
460+ // must still reject it so the state card renders an
461+ // error instead of spawning a transport against "".
462+ name: "http empty-resolved url",
463+ mcpName: "test-http-url-empty",
464+ cfg: config.MCPConfig{
465+ Type: config.MCPHttp,
466+ URL: "${MCP_URL_EMPTY:-}",
467+ },
468+ wantErrContains: "non-empty 'url'",
469+ },
377470 {
378471 name: "http header failure",
379472 mcpName: "test-http-header-fail",
@@ -394,12 +487,17 @@ func TestCreateSession_ResolutionFailureUpdatesState(t *testing.T) {
394487 wantErrContains: "url:",
395488 },
396489 {
490+ // Bare $MISSING in a header resolves to "" silently
491+ // and is then dropped (design decision #18). The
492+ // "header Authorization" wrap only surfaces on a
493+ // $(cmd) failure; that is what this subtest now
494+ // pins for the SSE path.
397495 name: "sse header failure",
398496 mcpName: "test-sse-header-fail",
399497 cfg: config.MCPConfig{
400498 Type: config.MCPSSE,
401499 URL: "https://mcp.example.com/events",
402− Headers: map[string]string{"Authorization": "Bearer $MISSING_SSE_TOKEN"},
500+ Headers: map[string]string{"Authorization": "$(false)"},
403501 },
404502 wantErrContains: "header Authorization",
405503 },