fix(mcp): allow inline stdio env assignments

[Shushmitaaaa]

Summary

Fixes STDIO MCP env validation so inline KEY=value entries are treated as explicit environment assignments instead of looking up the full KEY=value string as an environment variable name.

Changes

• Split STDIO env entries on the first =
• Skip host environment lookup for inline KEY=value assignments
• Preserve validation for referenced env vars like API_KEY
• Reject empty env assignment names like =value
• Added regression tests for inline assignments, referenced env vars, and empty names

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (React)
• Docs

How to test

Validated the focused MCP package tests from the core module.

cd core
go test ./mcp

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes

  • Improved environment-variable handling for STDIO clients: inline assignments like NAME=value are accepted, referenced variables must be set (otherwise a clear error is shown), and empty variable names are rejected with a specific error.

• Tests

  • Added unit tests covering inline env assignments, set vs. unset referenced variables, and empty-name validation.

Walkthrough

The PR updates STDIO connection environment handling to parse each StdioConfig.Envs entry as either NAME or NAME=value. Inline assignments are accepted without host checks; plain NAME entries must be non-empty and exist in the host environment. Four tests cover accepted inline assignments, set referenced vars, missing vars (error), and empty-name errors.

Changes

STDIO Environment Variable Validation

Layer / File(s)	Summary
STDIO environment parsing core/mcp/clientmanager.go	createSTDIOConnection now parses env entries as NAME or NAME=value, accepts inline assignments without checking the host environment, rejects empty env names, and errors when a plain NAME entry is unset.
Environment validation test suite core/mcp/clientmanager_test.go	Adds four tests verifying inline env assignments succeed, set referenced env vars succeed, missing referenced env vars produce an error mentioning the variable is not set, and empty env assignment names are rejected with a specific error.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• [Bug]: MCPStdioConfig.Envs validation incorrectly treats KEY=value as literal key name #3791: This change directly addresses parsing KEY=value inline assignments in StdioConfig.Envs which was previously treated as a literal env name.

Poem

I hop through envs both near and far,
Splitting keys and values like a star,
Inline assigns I now allow,
Empty names get a stern bow,
Tests at dawn confirm the spar. 🐰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: allowing inline KEY=value environment assignments in STDIO MCP configuration.
Description check	✅ Passed	The description covers the summary, specific changes, type of change, affected areas, and testing instructions. While the breaking changes section is not explicitly checked and security considerations are not addressed, the core required information is present and complete.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

[greptile-apps]

Confidence Score: 4/5

The change is a focused, low-risk fix to env validation logic with no impact on transport setup or connection lifecycle.

The core fix in clientmanager.go is correct and handles all edge cases well. The only gap is a missing happy-path test for the referenced-var branch, which means a future inversion of that check would not be caught by these tests.

core/mcp/clientmanager_test.go — the referenced-var success path is untested.

Important Files Changed

Filename	Overview
core/mcp/clientmanager.go	Validation loop uses strings.Cut to distinguish inline assignments from plain var-name references; logic is correct for all edge cases including empty names and values with embedded equals signs.
core/mcp/clientmanager_test.go	New file adds three focused regression tests; happy-path coverage for a referenced env var that IS set to a non-empty value is absent.

_{Reviews (1): Last reviewed commit: "fix(mcp): allow inline stdio env assignm..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

core/mcp/clientmanager_test.go (1)

11-25: ⚡ Quick win

Add a regression case where inline env value contains =.

Given the parser now splits on the first =, add a case like Envs: []string{"TEST_STDIO_ENV_ASSIGNMENT=a=b"} and assert success. This guards the exact contract and prevents accidental full-split regressions.

Suggested test addition

 func TestCreateSTDIOConnectionAllowsInlineEnvAssignments(t *testing.T) {
 	t.Parallel()
@@
 	_, _, err := (&MCPManager{}).createSTDIOConnection(context.Background(), config, nil)
 	require.NoError(t, err)
+
+	configWithEqualsInValue := &schemas.MCPClientConfig{
+		Name:           "test-stdio-client",
+		ConnectionType: schemas.MCPConnectionTypeSTDIO,
+		StdioConfig: &schemas.MCPStdioConfig{
+			Command: "echo",
+			Envs:    []string{"TEST_STDIO_ENV_ASSIGNMENT=a=b"},
+		},
+	}
+
+	_, _, err = (&MCPManager{}).createSTDIOConnection(context.Background(), configWithEqualsInValue, nil)
+	require.NoError(t, err)
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/mcp/clientmanager_test.go` around lines 11 - 25, Update the test
TestCreateSTDIOConnectionAllowsInlineEnvAssignments (which exercises
MCPManager.createSTDIOConnection) to include a regression case where an env
value contains an extra '=' (for example Envs:
[]string{"TEST_STDIO_ENV_ASSIGNMENT=a=b"}) and assert the call still returns no
error; this ensures the parser only splits on the first '=' and doesn't drop or
mis-split the value.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@core/mcp/clientmanager_test.go`:
- Around line 11-25: Update the test
TestCreateSTDIOConnectionAllowsInlineEnvAssignments (which exercises
MCPManager.createSTDIOConnection) to include a regression case where an env
value contains an extra '=' (for example Envs:
[]string{"TEST_STDIO_ENV_ASSIGNMENT=a=b"}) and assert the call still returns no
error; this ensures the parser only splits on the first '=' and doesn't drop or
mis-split the value.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 366227f6-cc4b-44f9-89ed-24dd1fc67a6e

📥 Commits

Reviewing files that changed from the base of the PR and between ab74128 and bafa8eb.

📒 Files selected for processing (2)

• core/mcp/clientmanager.go
• core/mcp/clientmanager_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• core/mcp/clientmanager.go

[greptile-apps]

Greptile encountered an error while reviewing this PR. Please reach out to support@greptile.com for assistance.

[Shushmitaaaa]

Added the referenced-env happy path test as suggested.

[Pratham-Mishra04]

Hey @Shushmitaaaa thanks for the PR! It looks good - will be merging in a while

[akshaydeo]

Merge activity

• Jun 1, 5:54 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jun 1, 5:55 AM UTC: Graphite couldn't merge this PR because it failed for an unknown reason (Fast-forward merges are not supported for forked repositories. Please create a branch in the target repository in order to merge).