From a51ad98182ee50323296c2ec287a4362e6838329 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 2 Jan 2026 16:07:33 +0900 Subject: [PATCH] fix(skill-mcp): always inherit process.env for MCP servers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Always merge parent process.env when spawning MCP child processes - Overlay config.env on top if present (for skill-specific overrides) - Fixes issue where skills without explicit env: block started with zero environment variables - Adds 2 tests for env inheritance behavior 🤖 Generated with assistance of OhMyOpenCode (https://github.com/code-yeongyu/oh-my-opencode) --- .../skill-mcp-manager/manager.test.ts | 50 +++++++++++++++++++ src/features/skill-mcp-manager/manager.ts | 10 ++-- 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/src/features/skill-mcp-manager/manager.test.ts b/src/features/skill-mcp-manager/manager.test.ts index ab77f04..2313e22 100644 --- a/src/features/skill-mcp-manager/manager.test.ts +++ b/src/features/skill-mcp-manager/manager.test.ts @@ -106,4 +106,54 @@ describe("SkillMcpManager", () => { expect(manager.getConnectedServers()).toEqual([]) }) }) + + describe("environment variable handling", () => { + it("always inherits process.env even when config.env is undefined", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "test-server", + skillName: "test-skill", + sessionID: "session-1", + } + const configWithoutEnv: ClaudeCodeMcpServer = { + command: "node", + args: ["-e", "process.exit(0)"], + } + + // #when - attempt connection (will fail but exercises env merging code path) + // #then - should not throw "undefined" related errors for env + try { + await manager.getOrCreateClient(info, configWithoutEnv) + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + expect(message).not.toContain("env") + expect(message).not.toContain("undefined") + } + }) + + it("overlays config.env on top of inherited process.env", async () => { + // #given + const info: SkillMcpClientInfo = { + serverName: "test-server", + skillName: "test-skill", + sessionID: "session-2", + } + const configWithEnv: ClaudeCodeMcpServer = { + command: "node", + args: ["-e", "process.exit(0)"], + env: { + CUSTOM_VAR: "custom_value", + }, + } + + // #when - attempt connection + // #then - should not throw, env merging should work + try { + await manager.getOrCreateClient(info, configWithEnv) + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + expect(message).toContain("Failed to connect") + } + }) + }) }) diff --git a/src/features/skill-mcp-manager/manager.ts b/src/features/skill-mcp-manager/manager.ts index 967f06a..027edaf 100644 --- a/src/features/skill-mcp-manager/manager.ts +++ b/src/features/skill-mcp-manager/manager.ts @@ -55,18 +55,20 @@ export class SkillMcpManager { const command = config.command const args = config.args || [] + // Always inherit parent process environment const mergedEnv: Record = {} + for (const [key, value] of Object.entries(process.env)) { + if (value !== undefined) mergedEnv[key] = value + } + // Overlay with skill-specific env vars if present if (config.env) { - for (const [key, value] of Object.entries(process.env)) { - if (value !== undefined) mergedEnv[key] = value - } Object.assign(mergedEnv, config.env) } const transport = new StdioClientTransport({ command, args, - env: config.env ? mergedEnv : undefined, + env: mergedEnv, }) const client = new Client(