From 3fc4988a1c63c38567048bd1e7ba0a7ca36c46e6 Mon Sep 17 00:00:00 2001 From: NamesMT Date: Sat, 7 Jun 2025 09:44:07 +0000 Subject: [PATCH 1/5] feat: add `injectVariables()`, support magic variables for MCPs --- src/services/mcp/McpHub.ts | 9 ++++--- src/utils/__tests__/config.spec.ts | 26 ++++++++++++++++--- src/utils/config.ts | 40 +++++++++++++++++++++++------- 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index dd4727213e..3a7252a75a 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -31,7 +31,7 @@ import { } from "../../shared/mcp" import { fileExistsAtPath } from "../../utils/fs" import { arePathsEqual } from "../../utils/path" -import { injectEnv } from "../../utils/config" +import { injectEnv, injectVariables } from "../../utils/config" export type McpConnection = { server: McpServer @@ -579,8 +579,11 @@ export class McpHub { let transport: StdioClientTransport | SSEClientTransport | StreamableHTTPClientTransport - // Inject environment variables to the config - const configInjected = (await injectEnv(config)) as typeof config + // Inject variables to the config (environment, magic variables,...) + const configInjected = (await injectVariables(config, { + env: process.env, + workspaceFolder: vscode.workspace.workspaceFolders?.[0]?.uri.fsPath ?? '""', + })) as typeof config if (configInjected.type === "stdio") { transport = new StdioClientTransport({ diff --git a/src/utils/__tests__/config.spec.ts b/src/utils/__tests__/config.spec.ts index 294d05cc4f..060f632279 100644 --- a/src/utils/__tests__/config.spec.ts +++ b/src/utils/__tests__/config.spec.ts @@ -1,5 +1,6 @@ import { vitest, describe, it, expect, beforeEach, afterAll } from "vitest" -import { injectEnv } from "../config" +import { injectEnv, injectVariables } from "../config" + describe("injectEnv", () => { const originalEnv = process.env @@ -52,7 +53,7 @@ describe("injectEnv", () => { const result = await injectEnv(configString, "NOT_FOUND") expect(result).toBe(expectedString) expect(consoleWarnSpy).toHaveBeenCalledWith( - "[injectEnv] env variable MISSING_VAR referenced but not found in process.env", + "[injectVariables] variable MISSING_VAR referenced but not found in env", ) consoleWarnSpy.mockRestore() }) @@ -64,7 +65,7 @@ describe("injectEnv", () => { const result = await injectEnv(configString) expect(result).toBe(expectedString) expect(consoleWarnSpy).toHaveBeenCalledWith( - "[injectEnv] env variable ANOTHER_MISSING referenced but not found in process.env", + "[injectVariables] variable ANOTHER_MISSING referenced but not found in env", ) consoleWarnSpy.mockRestore() }) @@ -99,3 +100,22 @@ describe("injectEnv", () => { expect(result).toEqual({}) }) }) + +describe("injectVariables", () => { + it("should replace singular variable", async () => { + const result = await injectVariables("Hello ${v}", { v: "Hola" }) + expect(result).toEqual("Hello Hola") + }) + + it("should handle undefined singular variable input", async () => { + const result = await injectVariables("Hello ${v}", { v: undefined }) + expect(result).toEqual("Hello ${v}") + }) + + it("should handle empty string singular variable input", async () => { + const result = await injectVariables("Hello ${v}", { v: "" }) + expect(result).toEqual("Hello ") + }) + + // Variable maps are already tested by `injectEnv` tests above. +}) diff --git a/src/utils/config.ts b/src/utils/config.ts index 50dc7d6386..9dcb41fa00 100644 --- a/src/utils/config.ts +++ b/src/utils/config.ts @@ -6,20 +6,42 @@ * Does not mutate original object */ export async function injectEnv>(config: C, notFoundValue: any = "") { - // Use simple regex replace for now, will see if object traversal and recursion is needed here (e.g: for non-serializable objects) + return injectVariables(config, { env: process.env }, notFoundValue) +} +/** + * Deeply injects variables into a configuration object/string/json + * + * Uses VSCode's variables reference pattern: https://code.visualstudio.com/docs/reference/variables-reference#_environment-variables + * + * Does not mutate original object + * + * There is a special handling for a nested (record-type) variables, where it is replaced by `propNotFoundValue` (if available) if the root key exists but the nested key does not. + * + * Matched keys that have `null` | `undefined` values are treated as not found. + */ +export async function injectVariables>( + config: string | Record, + variables: Record>, + propNotFoundValue?: any, +) { + // Use simple regex replace for now, will see if object traversal and recursion is needed here (e.g: for non-serializable objects) const isObject = typeof config === "object" let _config: string = isObject ? JSON.stringify(config) : config - _config = _config.replace(/\$\{env:([\w]+)\}/g, (_, name) => { - // Check if null or undefined - // intentionally using == to match null | undefined - if (process.env[name] == null) { - console.warn(`[injectEnv] env variable ${name} referenced but not found in process.env`) - } + // Intentionally using `== null` to match null | undefined + for (const [key, value] of Object.entries(variables)) { + if (value == null) continue + + if (typeof value === "string") _config = _config.replace(new RegExp(`\\$\\{${key}\\}`, "g"), value) + else + _config = _config.replace(new RegExp(`\\$\\{${key}:([\\w]+)\\}`, "g"), (match, name) => { + if (value[name] == null) + console.warn(`[injectVariables] variable ${name} referenced but not found in ${key}`) - return process.env[name] ?? notFoundValue - }) + return value[name] ?? propNotFoundValue ?? match + }) + } return (isObject ? JSON.parse(_config) : _config) as C extends string ? string : C } From 763eab6c6bf8e266a33ca106d5367d6c8c19de76 Mon Sep 17 00:00:00 2001 From: Trung Dang Date: Sat, 7 Jun 2025 16:54:44 +0700 Subject: [PATCH 2/5] fix: fallback for `workspaceFolder` should just be an empty string Previously this is intended so that the CLI receives a correct empty path argument, but on a second thought, if the user have added the quotes themselves it might cause error. Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com> --- src/services/mcp/McpHub.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 3a7252a75a..5cdfff798d 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -582,7 +582,7 @@ export class McpHub { // Inject variables to the config (environment, magic variables,...) const configInjected = (await injectVariables(config, { env: process.env, - workspaceFolder: vscode.workspace.workspaceFolders?.[0]?.uri.fsPath ?? '""', + workspaceFolder: vscode.workspace.workspaceFolders?.[0]?.uri.fsPath ?? "", })) as typeof config if (configInjected.type === "stdio") { From 88f3c19935cb7a6b13e3af07a3d64621231f511d Mon Sep 17 00:00:00 2001 From: Trung Dang Date: Sat, 7 Jun 2025 16:55:42 +0700 Subject: [PATCH 3/5] chore: remove unused import --- src/services/mcp/McpHub.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 5cdfff798d..35cdcf1159 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -31,7 +31,7 @@ import { } from "../../shared/mcp" import { fileExistsAtPath } from "../../utils/fs" import { arePathsEqual } from "../../utils/path" -import { injectEnv, injectVariables } from "../../utils/config" +import { injectVariables } from "../../utils/config" export type McpConnection = { server: McpServer From cdfd0d838c2118a65bb313853ac06c2f86f51e90 Mon Sep 17 00:00:00 2001 From: NamesMT Date: Sat, 7 Jun 2025 09:58:27 +0000 Subject: [PATCH 4/5] chore: better log format --- src/utils/__tests__/config.spec.ts | 4 ++-- src/utils/config.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/utils/__tests__/config.spec.ts b/src/utils/__tests__/config.spec.ts index 060f632279..540bb7f5e6 100644 --- a/src/utils/__tests__/config.spec.ts +++ b/src/utils/__tests__/config.spec.ts @@ -53,7 +53,7 @@ describe("injectEnv", () => { const result = await injectEnv(configString, "NOT_FOUND") expect(result).toBe(expectedString) expect(consoleWarnSpy).toHaveBeenCalledWith( - "[injectVariables] variable MISSING_VAR referenced but not found in env", + `[injectVariables] variable "MISSING_VAR" referenced but not found in "env"`, ) consoleWarnSpy.mockRestore() }) @@ -65,7 +65,7 @@ describe("injectEnv", () => { const result = await injectEnv(configString) expect(result).toBe(expectedString) expect(consoleWarnSpy).toHaveBeenCalledWith( - "[injectVariables] variable ANOTHER_MISSING referenced but not found in env", + `[injectVariables] variable "ANOTHER_MISSING" referenced but not found in "env"`, ) consoleWarnSpy.mockRestore() }) diff --git a/src/utils/config.ts b/src/utils/config.ts index 9dcb41fa00..fa1c7b2707 100644 --- a/src/utils/config.ts +++ b/src/utils/config.ts @@ -37,7 +37,7 @@ export async function injectVariables { if (value[name] == null) - console.warn(`[injectVariables] variable ${name} referenced but not found in ${key}`) + console.warn(`[injectVariables] variable "${name}" referenced but not found in "${key}"`) return value[name] ?? propNotFoundValue ?? match }) From 99ebb51c67fd924d0d28f6af611b3fe859a82719 Mon Sep 17 00:00:00 2001 From: NamesMT Date: Sun, 8 Jun 2025 04:38:18 +0000 Subject: [PATCH 5/5] chore: better describe the accepted config type and more extensive test --- src/utils/__tests__/config.spec.ts | 34 ++++++++++++++++++++++++++++-- src/utils/config.ts | 18 +++++++++++++--- 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/src/utils/__tests__/config.spec.ts b/src/utils/__tests__/config.spec.ts index 540bb7f5e6..0f1c8275f4 100644 --- a/src/utils/__tests__/config.spec.ts +++ b/src/utils/__tests__/config.spec.ts @@ -31,14 +31,44 @@ describe("injectEnv", () => { key: "${env:API_KEY}", url: "${env:ENDPOINT}", nested: { - value: "Keep this ${env:API_KEY}", + string: "Keep this ${env:API_KEY}", + number: 123, + boolean: true, + stringArr: ["${env:API_KEY}", "${env:ENDPOINT}"], + numberArr: [123, 456], + booleanArr: [true, false], + }, + deeply: { + nested: { + string: "Keep this ${env:API_KEY}", + number: 123, + boolean: true, + stringArr: ["${env:API_KEY}", "${env:ENDPOINT}"], + numberArr: [123, 456], + booleanArr: [true, false], + }, }, } const expectedObject = { key: "12345", url: "https://example.com", nested: { - value: "Keep this 12345", + string: "Keep this 12345", + number: 123, + boolean: true, + stringArr: ["12345", "https://example.com"], + numberArr: [123, 456], + booleanArr: [true, false], + }, + deeply: { + nested: { + string: "Keep this 12345", + number: 123, + boolean: true, + stringArr: ["12345", "https://example.com"], + numberArr: [123, 456], + booleanArr: [true, false], + }, }, } const result = await injectEnv(configObject) diff --git a/src/utils/config.ts b/src/utils/config.ts index fa1c7b2707..68be8ef1d4 100644 --- a/src/utils/config.ts +++ b/src/utils/config.ts @@ -1,3 +1,15 @@ +export type InjectableConfigType = + | string + | { + [key: string]: + | undefined + | null + | boolean + | number + | InjectableConfigType + | Array + } + /** * Deeply injects environment variables into a configuration object/string/json * @@ -5,7 +17,7 @@ * * Does not mutate original object */ -export async function injectEnv>(config: C, notFoundValue: any = "") { +export async function injectEnv(config: C, notFoundValue: any = "") { return injectVariables(config, { env: process.env }, notFoundValue) } @@ -20,8 +32,8 @@ export async function injectEnv>(con * * Matched keys that have `null` | `undefined` values are treated as not found. */ -export async function injectVariables>( - config: string | Record, +export async function injectVariables( + config: C, variables: Record>, propNotFoundValue?: any, ) {