Skip to content
Merged
7 changes: 5 additions & 2 deletions src/client/cross-spawn.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { StdioClientTransport } from "./stdio.js";
import { getDefaultEnvironment, StdioClientTransport } from "./stdio.js";
import spawn from "cross-spawn";
import { JSONRPCMessage } from "../types.js";
import { ChildProcess } from "node:child_process";
Expand Down Expand Up @@ -72,7 +72,10 @@ describe("StdioClientTransport using cross-spawn", () => {
"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?

...customEnv,
...getDefaultEnvironment()
}
})
);
});
Expand Down
62 changes: 61 additions & 1 deletion src/client/stdio.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,21 @@
import { JSONRPCMessage } from "../types.js";
import { StdioClientTransport, StdioServerParameters } from "./stdio.js";
import { StdioClientTransport, StdioServerParameters, DEFAULT_INHERITED_ENV_VARS, getDefaultEnvironment } from "./stdio.js";

const serverParameters: StdioServerParameters = {
command: "/usr/bin/tee",
};


let spawnEnv: Record<string, string> | undefined;

jest.mock('cross-spawn', () => {
const originalSpawn = jest.requireActual('cross-spawn');
return jest.fn((command, args, options) => {
spawnEnv = options.env;
return originalSpawn(command, args, options);
});
});

test("should start then close cleanly", async () => {
const client = new StdioClientTransport(serverParameters);
client.onerror = (error) => {
Expand Down Expand Up @@ -59,3 +70,52 @@ test("should read messages", async () => {

await client.close();
});

test("should properly set default environment variables in spawned process", async () => {
const client = new StdioClientTransport(serverParameters);

await client.start();
await client.close();

// Get the default environment variables
const defaultEnv = getDefaultEnvironment();

// Verify that all default environment variables are present
for (const key of DEFAULT_INHERITED_ENV_VARS) {
if (process.env[key] && !process.env[key].startsWith("()")) {
expect(spawnEnv).toHaveProperty(key);
expect(spawnEnv![key]).toBe(process.env[key]);
expect(spawnEnv![key]).toBe(defaultEnv[key]);
}
}
});

test("should override default environment variables with custom ones", async () => {
const customEnv = {
HOME: "/custom/home",
PATH: "/custom/path",
USER: "custom_user"
};

const client = new StdioClientTransport({
...serverParameters,
env: customEnv
});

await client.start();
await client.close();

// Verify that custom environment variables override default ones
for (const [key, value] of Object.entries(customEnv)) {
expect(spawnEnv).toHaveProperty(key);
expect(spawnEnv![key]).toBe(value);
}

// Verify that other default environment variables are still present
for (const key of DEFAULT_INHERITED_ENV_VARS) {
if (!(key in customEnv) && process.env[key] && !process.env[key].startsWith("()")) {
expect(spawnEnv).toHaveProperty(key);
expect(spawnEnv![key]).toBe(process.env[key]);
}
}
});
6 changes: 5 additions & 1 deletion src/client/stdio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,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