Skip to content

fix(client): Some mcp server need default env(#393, #196) #394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jul 7, 2025
Merged

fix(client): Some mcp server need default env(#393, #196) #394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
cd359ed
fix(client): No such file or directory in StdioClientTransport(#393, …
sunrabbit123 Apr 24, 2025
9cca201
fix: modify test code about stdio.test
sunrabbit123 Apr 29, 2025
47204f2
Merge branch 'main' into fix/1
sunrabbit123 Apr 29, 2025
c5dd44c
Merge branch 'main' into fix/1
sunrabbit123 May 9, 2025
823fc43
Merge branch 'main' into fix/1
sunrabbit123 May 26, 2025
bd61d21
Merge branch 'main' into fix/1
sunrabbit123 Jun 30, 2025
f43bfcc
test: fix test code for save test case principal
sunrabbit123 Jun 30, 2025
afb128d
fix: should pass environment variables correctly
sunrabbit123 Jun 30, 2025
f31cefe
Merge branch 'main' into fix/1
sunrabbit123 Jul 2, 2025
4607f37
Merge branch 'main' into fix/1
sunrabbit123 Jul 7, 2025
d4bfe55
fix: format error with merge
sunrabbit123 Jul 7, 2025
0ff7ae0
simplify tests
ihrpr Jul 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions src/client/cross-spawn.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { StdioClientTransport } from "./stdio.js";
import { StdioClientTransport, getDefaultEnvironment } from "./stdio.js";
import spawn from "cross-spawn";
import { JSONRPCMessage } from "../types.js";
import { ChildProcess } from "node:child_process";
Expand Down Expand Up @@ -67,12 +67,33 @@ describe("StdioClientTransport using cross-spawn", () => {

await transport.start();

// verify environment variables are passed correctly
// verify environment variables are merged correctly
expect(mockSpawn).toHaveBeenCalledWith(
"test-command",
[],
expect.objectContaining({
env: customEnv
env: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but.. i don't understand this comment, can i ask you to explain with detail?

...getDefaultEnvironment(),
...customEnv
}
})
);
});

test("should use default environment when env is undefined", async () => {
const transport = new StdioClientTransport({
command: "test-command",
env: undefined
});

await transport.start();

// verify default environment is used
expect(mockSpawn).toHaveBeenCalledWith(
"test-command",
[],
expect.objectContaining({
env: getDefaultEnvironment()
})
);
});
Expand Down
6 changes: 5 additions & 1 deletion src/client/stdio.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,11 @@ export class StdioClientTransport implements Transport {
this._serverParams.command,
this._serverParams.args ?? [],
{
env: this._serverParams.env ?? getDefaultEnvironment(),
// merge default env with server env because mcp server needs some env vars
env: {
...getDefaultEnvironment(),
...this._serverParams.env,
},
stdio: ["pipe", "pipe", this._serverParams.stderr ?? "inherit"],
shell: false,
signal: this._abortController.signal,
Expand Down