diff --git a/actions/setup/js/mcp_dependencies_manager.cjs b/actions/setup/js/mcp_dependencies_manager.cjs new file mode 100644 index 00000000000..2bf806024ef --- /dev/null +++ b/actions/setup/js/mcp_dependencies_manager.cjs @@ -0,0 +1,184 @@ +// @ts-check + +const { execFileSync } = require("child_process"); +const fs = require("fs"); +const path = require("path"); + +const installedDependencyPromises = new Map(); +const perToolInstallPromises = new Map(); +let execFileSyncRunner = execFileSync; + +/** + * Emit dependency-install logs via the provided MCP logger. + * @param {Object} logger + * @param {string} level + * @param {string} message + */ +function logWithCore(logger, level, message) { + if (logger) { + const logMethod = typeof logger[level] === "function" ? logger[level] : logger.debug; + if (typeof logMethod === "function") { + logMethod(message); + } + } +} + +function inferDependencyManager(handlerPath) { + const ext = path.extname(handlerPath).toLowerCase(); + if (ext === ".py") return "pip"; + if (ext === ".go") return "go"; + if (ext === ".sh") return "shell"; + return "npm"; +} + +function resolveShellPackageManager() { + const managers = [ + { command: "apt-get", args: ["install", "-y"] }, + { command: "yum", args: ["install", "-y"] }, + { command: "dnf", args: ["install", "-y"] }, + ]; + + for (const manager of managers) { + try { + execFileSyncRunner("which", [manager.command], { stdio: "pipe" }); + return manager; + } catch { + // Try next manager + } + } + + return null; +} + +function isTransientInstallFailure(message) { + return /(timed out|timeout|temporary|network|econnreset|econnrefused|eai_again|etimedout|429|502|503|504)/i.test(message); +} + +function isDeterministicInstallFailure(message) { + return /(not found|no matching distribution|unable to locate package|invalid requirement|permission denied|forbidden|unauthorized|unknown revision|invalid version)/i.test(message); +} + +function executeInstallWithRetry(logger, toolName, dependency, command, args, cwd) { + const maxRetries = 2; + for (let attempt = 0; attempt <= maxRetries; attempt++) { + try { + logWithCore(logger, "debug", ` [${toolName}] Installing dependency '${dependency}' with: ${command} ${args.join(" ")}`); + execFileSyncRunner(command, args, { + cwd, + stdio: "pipe", + env: process.env, + }); + logWithCore(logger, "info", ` [${toolName}] Installed dependency '${dependency}'`); + return; + } catch (error) { + const stderr = error && error.stderr ? String(error.stderr) : ""; + const stdout = error && error.stdout ? String(error.stdout) : ""; + const details = [stderr.trim(), stdout.trim(), error && error.message ? String(error.message) : ""].filter(Boolean).join("\n"); + + if (isDeterministicInstallFailure(details)) { + logWithCore(logger, "error", ` [${toolName}] Deterministic dependency install failure for '${dependency}'`); + throw new Error(`Dependency installation failed for '${dependency}': ${details || "deterministic failure"}`); + } + + if (!isTransientInstallFailure(details) || attempt === maxRetries) { + logWithCore(logger, "error", ` [${toolName}] Dependency install failed after ${attempt + 1} attempt(s) for '${dependency}'`); + throw new Error(`Dependency installation failed for '${dependency}' after ${attempt + 1} attempt(s): ${details || "unknown error"}`); + } + + logWithCore(logger, "warning", ` [${toolName}] Transient dependency install failure for '${dependency}', retrying (${attempt + 1}/${maxRetries})`); + } + } +} + +function installDependency(logger, toolName, dependency, manager, basePath) { + let command = ""; + let args = []; + let cwd = basePath; + + if (manager === "npm") { + command = "npm"; + args = ["install", "--ignore-scripts", "--no-save", "--", dependency]; + } else if (manager === "pip") { + command = "python3"; + args = ["-m", "pip", "install", "--disable-pip-version-check", dependency]; + } else if (manager === "go") { + const goModPath = path.join(basePath, "go.mod"); + if (!fs.existsSync(goModPath)) { + try { + execFileSyncRunner("go", ["mod", "init", "example.com/mcp-scripts"], { cwd: basePath, stdio: "pipe", env: process.env }); + } catch { + // go.mod may have been created concurrently + } + } + command = "go"; + args = ["get", dependency]; + } else if (manager === "shell") { + const shellPM = resolveShellPackageManager(); + if (!shellPM) { + throw new Error(`Dependency installation failed for '${dependency}': no supported system package manager found (expected apt-get, yum, or dnf)`); + } + command = shellPM.command; + args = [...shellPM.args, dependency]; + cwd = process.cwd(); + } else { + return; + } + + executeInstallWithRetry(logger, toolName, dependency, command, args, cwd); +} + +function createDependencyInstallGate(logger, toolName, handlerPath, dependencies, basePath) { + const depList = Array.isArray(dependencies) ? dependencies.filter(dep => typeof dep === "string" && dep.trim() !== "") : []; + if (depList.length === 0) { + return async () => {}; + } + + const manager = inferDependencyManager(handlerPath); + const toolKey = `${toolName}:${handlerPath}`; + + return async () => { + if (perToolInstallPromises.has(toolKey)) { + logWithCore(logger, "debug", ` [${toolName}] Reusing dependency install gate for ${toolKey}`); + return perToolInstallPromises.get(toolKey); + } + + const installPromise = (async () => { + logWithCore(logger, "debug", ` [${toolName}] Starting dependency install gate (${depList.length} dependency item(s))`); + for (const dependency of depList) { + const key = `${manager}:${dependency}`; + if (!installedDependencyPromises.has(key)) { + logWithCore(logger, "debug", ` [${toolName}] No existing install promise for '${dependency}', creating one`); + installedDependencyPromises.set( + key, + Promise.resolve().then(() => installDependency(logger, toolName, dependency, manager, basePath)) + ); + } else { + logWithCore(logger, "debug", ` [${toolName}] Reusing existing install promise for '${dependency}'`); + } + await installedDependencyPromises.get(key); + } + logWithCore(logger, "debug", ` [${toolName}] Dependency install gate completed`); + })(); + + perToolInstallPromises.set(toolKey, installPromise); + return installPromise; + }; +} + +function resetDependencyInstallStateForTests() { + installedDependencyPromises.clear(); + perToolInstallPromises.clear(); +} + +function setExecFileSyncRunnerForTests(runner) { + execFileSyncRunner = runner || execFileSync; +} + +module.exports = { + createDependencyInstallGate, + inferDependencyManager, + isTransientInstallFailure, + isDeterministicInstallFailure, + resetDependencyInstallStateForTests, + setExecFileSyncRunnerForTests, +}; diff --git a/actions/setup/js/mcp_dependencies_manager.test.cjs b/actions/setup/js/mcp_dependencies_manager.test.cjs new file mode 100644 index 00000000000..20ae8423040 --- /dev/null +++ b/actions/setup/js/mcp_dependencies_manager.test.cjs @@ -0,0 +1,70 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +describe("mcp_dependencies_manager", () => { + beforeEach(() => { + vi.resetModules(); + }); + + it("infers manager from handler extension", async () => { + const { inferDependencyManager } = await import("./mcp_dependencies_manager.cjs"); + expect(inferDependencyManager("/tmp/tool.py")).toBe("pip"); + expect(inferDependencyManager("/tmp/tool.go")).toBe("go"); + expect(inferDependencyManager("/tmp/tool.sh")).toBe("shell"); + expect(inferDependencyManager("/tmp/tool.cjs")).toBe("npm"); + }); + + it("installs python dependencies before first invocation only", async () => { + const execRunner = vi.fn().mockReturnValue(Buffer.from("")); + const { createDependencyInstallGate, resetDependencyInstallStateForTests, setExecFileSyncRunnerForTests } = await import("./mcp_dependencies_manager.cjs"); + resetDependencyInstallStateForTests(); + setExecFileSyncRunnerForTests(execRunner); + + const logger = { debug: vi.fn(), debugError: vi.fn() }; + const gate = createDependencyInstallGate(logger, "fetch-url", "/tmp/fetch.py", ["requests"], "/tmp"); + await gate(); + await gate(); + + const installCalls = execRunner.mock.calls.filter(call => call[0] === "python3" && call[1][0] === "-m"); + expect(installCalls).toHaveLength(1); + expect(installCalls[0][1]).toEqual(["-m", "pip", "install", "--disable-pip-version-check", "requests"]); + }); + + it("retries transient install failures", async () => { + const execRunner = vi + .fn() + .mockImplementationOnce(() => { + const error = new Error("timeout"); + error.stderr = Buffer.from("network timeout"); + throw error; + }) + .mockReturnValueOnce(Buffer.from("")); + const { createDependencyInstallGate, resetDependencyInstallStateForTests, setExecFileSyncRunnerForTests } = await import("./mcp_dependencies_manager.cjs"); + resetDependencyInstallStateForTests(); + setExecFileSyncRunnerForTests(execRunner); + + const logger = { debug: vi.fn(), debugError: vi.fn() }; + const gate = createDependencyInstallGate(logger, "fetch-url", "/tmp/fetch.py", ["requests"], "/tmp"); + await gate(); + + const installCalls = execRunner.mock.calls.filter(call => call[0] === "python3" && call[1][0] === "-m"); + expect(installCalls).toHaveLength(2); + }); + + it("fails fast on deterministic install failures", async () => { + const execRunner = vi.fn().mockImplementation(() => { + const error = new Error("bad package"); + error.stderr = Buffer.from("No matching distribution found for bad package"); + throw error; + }); + const { createDependencyInstallGate, resetDependencyInstallStateForTests, setExecFileSyncRunnerForTests } = await import("./mcp_dependencies_manager.cjs"); + resetDependencyInstallStateForTests(); + setExecFileSyncRunnerForTests(execRunner); + + const logger = { debug: vi.fn(), debugError: vi.fn() }; + const gate = createDependencyInstallGate(logger, "fetch-url", "/tmp/fetch.py", ["bad package"], "/tmp"); + + await expect(gate()).rejects.toThrow("Dependency installation failed"); + const installCalls = execRunner.mock.calls.filter(call => call[0] === "python3" && call[1][0] === "-m"); + expect(installCalls).toHaveLength(1); + }); +}); diff --git a/actions/setup/js/mcp_scripts_config_loader.cjs b/actions/setup/js/mcp_scripts_config_loader.cjs index 9b5a28c9e20..9d1a607d8d8 100644 --- a/actions/setup/js/mcp_scripts_config_loader.cjs +++ b/actions/setup/js/mcp_scripts_config_loader.cjs @@ -17,6 +17,7 @@ const { ERR_SYSTEM, ERR_VALIDATION } = require("./error_codes.cjs"); * @property {Object} inputSchema - JSON Schema for tool inputs * @property {string} [handler] - Path to handler file (.cjs, .sh, or .py) * @property {number} [timeout] - Timeout in seconds for tool execution (default: 60) + * @property {string[]} [dependencies] - Runtime dependencies installed before first invocation */ /** diff --git a/actions/setup/js/mcp_scripts_mcp_server.cjs b/actions/setup/js/mcp_scripts_mcp_server.cjs index b6ce0403d3e..3584812fcf9 100644 --- a/actions/setup/js/mcp_scripts_mcp_server.cjs +++ b/actions/setup/js/mcp_scripts_mcp_server.cjs @@ -30,6 +30,7 @@ const { getErrorMessage } = require("./error_helpers.cjs"); * @property {string} description - Tool description * @property {Object} inputSchema - JSON Schema for tool inputs * @property {string} [handler] - Path to handler file (.cjs, .sh, or .py) + * @property {string[]} [dependencies] - Runtime dependencies installed before first invocation */ /** diff --git a/actions/setup/js/mcp_scripts_tool_factory.cjs b/actions/setup/js/mcp_scripts_tool_factory.cjs index ed6fce9a440..1e48c7e635d 100644 --- a/actions/setup/js/mcp_scripts_tool_factory.cjs +++ b/actions/setup/js/mcp_scripts_tool_factory.cjs @@ -13,6 +13,7 @@ * @property {string} description - Tool description * @property {Object} inputSchema - JSON Schema for tool inputs * @property {string} handler - Path to handler file (.cjs, .sh, or .py) + * @property {string[]} [dependencies] - Runtime dependencies installed before first invocation */ /** diff --git a/actions/setup/js/mcp_server_core.cjs b/actions/setup/js/mcp_server_core.cjs index bf0abf67788..ab85a617c8f 100644 --- a/actions/setup/js/mcp_server_core.cjs +++ b/actions/setup/js/mcp_server_core.cjs @@ -34,6 +34,7 @@ const { ReadBuffer } = require("./read_buffer.cjs"); const { validateRequiredFields, validateStringInputLengths } = require("./mcp_scripts_validation.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { generateEnhancedErrorMessage } = require("./mcp_enhanced_errors.cjs"); +const { createDependencyInstallGate } = require("./mcp_dependencies_manager.cjs"); const encoder = new TextEncoder(); const PARAMETER_SIMILARITY_DISTANCE_BONUS = 2; @@ -53,6 +54,7 @@ const UNKNOWN_PARAMETER_LIST_PREVIEW_MAX = 10; * @property {Function} [handler] - Tool handler function * @property {string} [handlerPath] - Optional file path to handler module (original path from config) * @property {number} [timeout] - Timeout in seconds for tool execution (default: 60) + * @property {string[]} [dependencies] - Runtime dependencies to install before first invocation */ /** @@ -391,7 +393,12 @@ function loadToolHandlers(server, tools, basePath) { // Lazy-load shell handler module const { createShellHandler } = require("./mcp_handler_shell.cjs"); const timeout = tool.timeout || 60; // Default to 60 seconds if not specified - tool.handler = createShellHandler(server, toolName, resolvedPath, timeout); + const baseHandler = createShellHandler(server, toolName, resolvedPath, timeout); + const ensureDependenciesInstalled = createDependencyInstallGate(server, toolName, resolvedPath, tool.dependencies, basePath || process.cwd()); + tool.handler = async args => { + await ensureDependenciesInstalled(); + return baseHandler(args); + }; loadedCount++; server.debug(` [${toolName}] Shell handler created successfully with timeout: ${timeout}s`); @@ -417,7 +424,12 @@ function loadToolHandlers(server, tools, basePath) { // Lazy-load Python handler module const { createPythonHandler } = require("./mcp_handler_python.cjs"); const timeout = tool.timeout || 60; // Default to 60 seconds if not specified - tool.handler = createPythonHandler(server, toolName, resolvedPath, timeout); + const baseHandler = createPythonHandler(server, toolName, resolvedPath, timeout); + const ensureDependenciesInstalled = createDependencyInstallGate(server, toolName, resolvedPath, tool.dependencies, basePath || process.cwd()); + tool.handler = async args => { + await ensureDependenciesInstalled(); + return baseHandler(args); + }; loadedCount++; server.debug(` [${toolName}] Python handler created successfully with timeout: ${timeout}s`); @@ -428,7 +440,12 @@ function loadToolHandlers(server, tools, basePath) { // Lazy-load Go handler module const { createGoHandler } = require("./mcp_handler_go.cjs"); const timeout = tool.timeout || 60; // Default to 60 seconds if not specified - tool.handler = createGoHandler(server, toolName, resolvedPath, timeout); + const baseHandler = createGoHandler(server, toolName, resolvedPath, timeout); + const ensureDependenciesInstalled = createDependencyInstallGate(server, toolName, resolvedPath, tool.dependencies, basePath || process.cwd()); + tool.handler = async args => { + await ensureDependenciesInstalled(); + return baseHandler(args); + }; loadedCount++; server.debug(` [${toolName}] Go handler created successfully with timeout: ${timeout}s`); @@ -439,7 +456,12 @@ function loadToolHandlers(server, tools, basePath) { // Lazy-load JavaScript handler module const { createJavaScriptHandler } = require("./mcp_handler_javascript.cjs"); const timeout = tool.timeout || 60; // Default to 60 seconds if not specified - tool.handler = createJavaScriptHandler(server, toolName, resolvedPath, timeout); + const baseHandler = createJavaScriptHandler(server, toolName, resolvedPath, timeout); + const ensureDependenciesInstalled = createDependencyInstallGate(server, toolName, resolvedPath, tool.dependencies, basePath || process.cwd()); + tool.handler = async args => { + await ensureDependenciesInstalled(); + return baseHandler(args); + }; loadedCount++; server.debug(` [${toolName}] JavaScript handler created successfully with timeout: ${timeout}s`); diff --git a/actions/setup/setup.sh b/actions/setup/setup.sh index b176581372c..4792ff52ed0 100755 --- a/actions/setup/setup.sh +++ b/actions/setup/setup.sh @@ -210,6 +210,7 @@ MCP_SCRIPTS_FILES=( "mcp_scripts_tool_factory.cjs" "mcp_scripts_validation.cjs" "mcp_server_core.cjs" + "mcp_dependencies_manager.cjs" "mcp_logger.cjs" "mcp_http_transport.cjs" "mcp_http_server_runner.cjs" @@ -273,6 +274,7 @@ SAFE_OUTPUTS_FILES=( "allowed_extensions_helpers.cjs" "safe_outputs_append.cjs" "mcp_server_core.cjs" + "mcp_dependencies_manager.cjs" "mcp_logger.cjs" "mcp_http_transport.cjs" "mcp_http_server_runner.cjs" diff --git a/docs/src/content/docs/reference/mcp-scripts.md b/docs/src/content/docs/reference/mcp-scripts.md index 90c9f1b06bd..692f7ebc8b6 100644 --- a/docs/src/content/docs/reference/mcp-scripts.md +++ b/docs/src/content/docs/reference/mcp-scripts.md @@ -131,7 +131,7 @@ mcp-scripts: print(json.dumps(result)) ``` -Python 3.10+ is available with standard library modules. Install additional packages inline using pip if needed. +Python 3.10+ is available with standard library modules. For third-party packages, use the `dependencies:` field with exact version pins (for example, `requests==2.32.3`) so gh-aw installs them before first tool invocation. ## Go Tools (`go:`) diff --git a/docs/src/content/docs/specs/mcp-scripts-specification.md b/docs/src/content/docs/specs/mcp-scripts-specification.md index deb55d04c50..1fe02ea3910 100644 --- a/docs/src/content/docs/specs/mcp-scripts-specification.md +++ b/docs/src/content/docs/specs/mcp-scripts-specification.md @@ -241,7 +241,7 @@ mcp-scripts: run: | echo "$INPUT_JSON" | jq '.data | length' dependencies: - - jq + - jq=1.6-2.1 timeout: 30 ``` @@ -261,12 +261,13 @@ mcp-scripts: response = requests.get(inputs.get('url')) print(json.dumps({"status": response.status_code, "content_length": len(response.text)})) dependencies: - - requests + - requests==2.32.3 timeout: 60 ``` **Requirements**: - Implementations MUST install dependencies before first tool invocation +- Dependencies MUST be pinned to exact release versions (floating references such as `latest`, bare names, or version ranges are not allowed) - Dependencies SHOULD be cached for subsequent invocations - Dependency installation failures MUST result in tool execution errors - Package names MUST be valid for the target package manager @@ -1594,7 +1595,11 @@ and run `go test ./pkg/workflow/...` to verify conformance. - **§4 — Configuration Format**: Frontmatter YAML is parsed by `parseMCPScriptsMap` in `mcp_scripts_parser.go`. The `dependencies` field (added in v1.1.0) is handled in - `parseMCPScriptToolConfig`. JSON Schema validation is provided by + `parseMCPScriptToolConfig` and propagated to `tools.json` by + `GenerateMCPScriptsToolsConfig` in `mcp_scripts_generator.go`. Runtime dependency + installation is performed before first invocation in + `actions/setup/js/mcp_dependencies_manager.cjs` via `loadToolHandlers` in + `actions/setup/js/mcp_server_core.cjs`. JSON Schema validation is provided by `pkg/parser/schemas/mcp-scripts-config.schema.json`. - **§5 — Tool Execution**: Each language's tool script is generated by a dedicated function diff --git a/pkg/workflow/mcp_scripts_dependencies_validation.go b/pkg/workflow/mcp_scripts_dependencies_validation.go new file mode 100644 index 00000000000..62008ad505c --- /dev/null +++ b/pkg/workflow/mcp_scripts_dependencies_validation.go @@ -0,0 +1,161 @@ +//go:build !js && !wasm + +package workflow + +import ( + "errors" + "fmt" + "regexp" + "strings" + + "github.com/github/gh-aw/pkg/semverutil" +) + +var ( + goModuleNameRE = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._/-]*$`) + shellPackageDependencyNameRE = regexp.MustCompile(`^[a-z0-9][a-z0-9+.-]*([:=][A-Za-z0-9.+~:-]+)?$`) +) + +func (c *Compiler) validateMCPScriptDependencies(workflowData *WorkflowData) error { + if workflowData == nil || workflowData.MCPScripts == nil { + return nil + } + + for toolName, tool := range workflowData.MCPScripts.Tools { + if len(tool.Dependencies) == 0 { + continue + } + + manager := inferMCPScriptDependencyManager(tool) + if manager == "" { + continue + } + + for _, dependency := range tool.Dependencies { + dependency = strings.TrimSpace(dependency) + if dependency == "" { + continue + } + if err := validateMCPScriptDependencyName(toolName, manager, dependency); err != nil { + return err + } + } + } + + return nil +} + +func inferMCPScriptDependencyManager(tool *MCPScriptToolConfig) string { + switch { + case tool.Script != "": + return "npm" + case tool.Py != "": + return "pip" + case tool.Go != "": + return "go" + case tool.Run != "": + return "apt" + default: + return "" + } +} + +func validateMCPScriptDependencyName(toolName, manager, dependency string) error { + switch manager { + case "npm": + name, version, err := splitNpmDependency(dependency) + if err != nil { + if nameErr := validateNpmPackageName(dependency); nameErr != nil { + return newInvalidDependencyNameError(toolName, dependency) + } + return newUnpinnedDependencyError(toolName, dependency, "npm", "name@1.2.3", "@scope/name@1.2.3") + } + if err := validateNpmPackageName(name); err != nil { + return newInvalidDependencyNameError(toolName, dependency) + } + if !semverutil.IsValid(version) { + return newUnpinnedDependencyError(toolName, dependency, "npm", "name@1.2.3", "@scope/name@1.2.3") + } + case "pip": + name := dependency + if idx := strings.IndexAny(name, "=<>!~"); idx > 0 { + name = name[:idx] + } + name = strings.TrimSpace(name) + if err := validatePipPackageName(name); err != nil { + return newInvalidDependencyNameError(toolName, dependency) + } + _, version, found := strings.Cut(dependency, "==") + if !found { + return newUnpinnedDependencyError(toolName, dependency, "pip", "name==1.2.3", "requests==2.32.3") + } + version = strings.TrimSpace(version) + if version == "" { + return newUnpinnedDependencyError(toolName, dependency, "pip", "name==1.2.3", "requests==2.32.3") + } + if !semverutil.IsValid(version) { + return newUnpinnedDependencyError(toolName, dependency, "pip", "name==1.2.3", "requests==2.32.3") + } + case "go": + moduleName, version, found := strings.Cut(dependency, "@") + if !found { + if !goModuleNameRE.MatchString(strings.TrimSpace(dependency)) { + return newInvalidDependencyNameError(toolName, dependency) + } + return newUnpinnedDependencyError(toolName, dependency, "go", "module@v1.2.3", "github.com/google/uuid@v1.6.0") + } + moduleName = strings.TrimSpace(moduleName) + version = strings.TrimSpace(version) + if moduleName == "" || version == "" { + return newUnpinnedDependencyError(toolName, dependency, "go", "module@v1.2.3", "github.com/google/uuid@v1.6.0") + } + if !goModuleNameRE.MatchString(moduleName) { + return newInvalidDependencyNameError(toolName, dependency) + } + if !strings.HasPrefix(version, "v") || !semverutil.IsValid(version) { + return newUnpinnedDependencyError(toolName, dependency, "go", "module@v1.2.3", "github.com/google/uuid@v1.6.0") + } + case "apt": + if !shellPackageDependencyNameRE.MatchString(dependency) { + return newInvalidDependencyNameError(toolName, dependency) + } + hasVersionPin := strings.Contains(dependency, "=") || strings.Contains(dependency, ":") + if !hasVersionPin { + return newUnpinnedDependencyError(toolName, dependency, "shell", "name=1.2.3", "jq=1.6-2.1") + } + } + + return nil +} + +func splitNpmDependency(dependency string) (string, string, error) { + atIndex := strings.LastIndex(dependency, "@") + if atIndex <= 0 || atIndex == len(dependency)-1 { + return "", "", errors.New("missing npm package version") + } + name := strings.TrimSpace(dependency[:atIndex]) + version := strings.TrimSpace(dependency[atIndex+1:]) + if name == "" || version == "" { + return "", "", errors.New("missing npm package version") + } + return name, version, nil +} + +func newInvalidDependencyNameError(toolName, dependency string) error { + return fmt.Errorf( + "invalid dependency name %q for tool %q. Expected a valid package name for the inferred package manager. Example: dependencies: [requests]", + dependency, + toolName, + ) +} + +func newUnpinnedDependencyError(toolName, dependency, manager, expected, example string) error { + return fmt.Errorf( + "dependency %q for tool %q is not pinned to a release tag. Expected %s dependency format %q with an exact version. Example: dependencies: [%q]", + dependency, + toolName, + manager, + expected, + example, + ) +} diff --git a/pkg/workflow/mcp_scripts_dependencies_validation_test.go b/pkg/workflow/mcp_scripts_dependencies_validation_test.go new file mode 100644 index 00000000000..552ce961a32 --- /dev/null +++ b/pkg/workflow/mcp_scripts_dependencies_validation_test.go @@ -0,0 +1,78 @@ +//go:build !integration + +package workflow + +import ( + "strings" + "testing" +) + +func TestValidateMCPScriptDependencies(t *testing.T) { + compiler := NewCompiler() + + t.Run("valid dependencies", func(t *testing.T) { + err := compiler.validateMCPScriptDependencies(&WorkflowData{ + MCPScripts: &MCPScriptsConfig{ + Tools: map[string]*MCPScriptToolConfig{ + "js-tool": { + Script: "return { ok: true }", + Dependencies: []string{"lodash@4.17.21", "@scope/pkg@1.0.0"}, + }, + "py-tool": { + Py: "print('ok')", + Dependencies: []string{"requests==2.32.3", "urllib3==2.2.1"}, + }, + "go-tool": { + Go: `fmt.Println("ok")`, + Dependencies: []string{"github.com/google/uuid@v1.6.0"}, + }, + "sh-tool": { + Run: "echo ok", + Dependencies: []string{"jq=1.6-2.1", "curl=8.5.0"}, + }, + }, + }, + }) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + }) + + t.Run("invalid dependency name", func(t *testing.T) { + err := compiler.validateMCPScriptDependencies(&WorkflowData{ + MCPScripts: &MCPScriptsConfig{ + Tools: map[string]*MCPScriptToolConfig{ + "fetch-url": { + Py: "print('ok')", + Dependencies: []string{"re quests"}, + }, + }, + }, + }) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), `invalid dependency name "re quests" for tool "fetch-url"`) { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("floating dependency rejected", func(t *testing.T) { + err := compiler.validateMCPScriptDependencies(&WorkflowData{ + MCPScripts: &MCPScriptsConfig{ + Tools: map[string]*MCPScriptToolConfig{ + "fetch-url": { + Py: "print('ok')", + Dependencies: []string{"requests"}, + }, + }, + }, + }) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), `dependency "requests" for tool "fetch-url" is not pinned to a release tag`) { + t.Fatalf("unexpected error: %v", err) + } + }) +} diff --git a/pkg/workflow/mcp_scripts_dependencies_validation_wasm.go b/pkg/workflow/mcp_scripts_dependencies_validation_wasm.go new file mode 100644 index 00000000000..73b32a19823 --- /dev/null +++ b/pkg/workflow/mcp_scripts_dependencies_validation_wasm.go @@ -0,0 +1,7 @@ +//go:build js || wasm + +package workflow + +func (c *Compiler) validateMCPScriptDependencies(workflowData *WorkflowData) error { + return nil +} diff --git a/pkg/workflow/mcp_scripts_generator.go b/pkg/workflow/mcp_scripts_generator.go index 374cc622807..494d458e78a 100644 --- a/pkg/workflow/mcp_scripts_generator.go +++ b/pkg/workflow/mcp_scripts_generator.go @@ -15,12 +15,13 @@ var mcpScriptsGeneratorLog = logger.New("workflow:mcp_scripts_generator") // MCPScriptsToolJSON represents a tool configuration for the tools.json file type MCPScriptsToolJSON struct { - Name string `json:"name"` - Description string `json:"description"` - InputSchema map[string]any `json:"inputSchema"` - Handler string `json:"handler,omitempty"` - Env map[string]string `json:"env,omitempty"` - Timeout int `json:"timeout,omitempty"` + Name string `json:"name"` + Description string `json:"description"` + InputSchema map[string]any `json:"inputSchema"` + Handler string `json:"handler,omitempty"` + Dependencies []string `json:"dependencies,omitempty"` + Env map[string]string `json:"env,omitempty"` + Timeout int `json:"timeout,omitempty"` } // MCPScriptsConfigJSON represents the tools.json configuration file structure @@ -124,13 +125,20 @@ func GenerateMCPScriptsToolsConfig(mcpScripts *MCPScriptsConfig) string { } } + var dependencies []string + if len(toolConfig.Dependencies) > 0 { + dependencies = append([]string(nil), toolConfig.Dependencies...) + sort.Strings(dependencies) + } + config.Tools = append(config.Tools, MCPScriptsToolJSON{ - Name: toolName, - Description: toolConfig.Description, - InputSchema: inputSchema, - Handler: handler, - Env: envRefs, - Timeout: toolConfig.Timeout, + Name: toolName, + Description: toolConfig.Description, + InputSchema: inputSchema, + Handler: handler, + Dependencies: dependencies, + Env: envRefs, + Timeout: toolConfig.Timeout, }) } diff --git a/pkg/workflow/mcp_scripts_generator_test.go b/pkg/workflow/mcp_scripts_generator_test.go index 6a9f826d568..5b673f5536c 100644 --- a/pkg/workflow/mcp_scripts_generator_test.go +++ b/pkg/workflow/mcp_scripts_generator_test.go @@ -38,6 +38,10 @@ func TestGenerateMCPScriptsMCPServerScript(t *testing.T) { Name: "analyze-data", Description: "Analyze data with Python", Py: "import json\nprint(json.dumps({'result': 'success'}))", + Dependencies: []string{ + "urllib3", + "requests", + }, Inputs: map[string]*MCPScriptParam{ "data": { Type: "string", @@ -127,6 +131,16 @@ func TestGenerateMCPScriptsMCPServerScript(t *testing.T) { t.Error("Tools config should reference Python script handler file") } + if !strings.Contains(toolsJSON, `"dependencies": [`) { + t.Error("Tools config should include dependencies array when configured") + } + if !strings.Contains(toolsJSON, `"requests",`) || !strings.Contains(toolsJSON, `"urllib3"`) { + t.Error("Tools config should include configured dependencies") + } + if strings.Index(toolsJSON, `"requests"`) > strings.Index(toolsJSON, `"urllib3"`) { + t.Error("Tools config should sort dependencies for stable output") + } + // Check for Go tool handler if !strings.Contains(toolsJSON, `"handler": "process-data.go"`) { t.Error("Tools config should reference Go script handler file") diff --git a/pkg/workflow/mcp_scripts_parser.go b/pkg/workflow/mcp_scripts_parser.go index 4da5ec11a64..306a1130b01 100644 --- a/pkg/workflow/mcp_scripts_parser.go +++ b/pkg/workflow/mcp_scripts_parser.go @@ -40,15 +40,16 @@ type MCPScriptsConfig struct { // MCPScriptToolConfig holds the configuration for a single mcp-script tool type MCPScriptToolConfig struct { - Name string // Tool name (key from the config) - Description string // Required: tool description - Inputs map[string]*MCPScriptParam // Optional: input parameters - Script string // JavaScript implementation (mutually exclusive with Run, Py, and Go) - Run string // Shell script implementation (mutually exclusive with Script, Py, and Go) - Py string // Python script implementation (mutually exclusive with Script, Run, and Go) - Go string // Go script implementation (mutually exclusive with Script, Run, and Py) - Env map[string]string // Environment variables (typically for secrets) - Timeout int // Timeout in seconds for tool execution (default: 60) + Name string // Tool name (key from the config) + Description string // Required: tool description + Inputs map[string]*MCPScriptParam // Optional: input parameters + Script string // JavaScript implementation (mutually exclusive with Run, Py, and Go) + Run string // Shell script implementation (mutually exclusive with Script, Py, and Go) + Py string // Python script implementation (mutually exclusive with Script, Run, and Go) + Go string // Go script implementation (mutually exclusive with Script, Run, and Py) + Dependencies []string // Optional runtime dependencies + Env map[string]string // Environment variables (typically for secrets) + Timeout int // Timeout in seconds for tool execution (default: 60) } // MCPScriptParam holds the configuration for a tool input parameter @@ -171,6 +172,17 @@ func parseMCPScriptToolConfig(toolName string, toolMap map[string]any) *MCPScrip } } + // Parse dependencies (optional list of strings) + if dependencies, exists := toolMap["dependencies"]; exists { + if depsList, ok := dependencies.([]any); ok { + for _, dep := range depsList { + if depStr, ok := dep.(string); ok { + toolConfig.Dependencies = append(toolConfig.Dependencies, depStr) + } + } + } + } + // Parse timeout (optional, default is 60 seconds) if timeout, exists := toolMap["timeout"]; exists { switch t := timeout.(type) { diff --git a/pkg/workflow/mcp_scripts_parser_test.go b/pkg/workflow/mcp_scripts_parser_test.go index 05d9fe3ebf1..211c9758be4 100644 --- a/pkg/workflow/mcp_scripts_parser_test.go +++ b/pkg/workflow/mcp_scripts_parser_test.go @@ -3,6 +3,7 @@ package workflow import ( + "reflect" "testing" ) @@ -127,3 +128,74 @@ func TestIsMCPScriptsEnabledWithEnv(t *testing.T) { // TestParseMCPScriptsAndExtractMCPScriptsConfigConsistency verifies that ParseMCPScripts // and extractMCPScriptsConfig produce identical results for the same input. // This ensures both functions use the shared helper correctly. + +func TestParseMCPScriptToolConfigDependencies(t *testing.T) { + t.Run("single dependency", func(t *testing.T) { + tool := parseMCPScriptToolConfig("fetch", map[string]any{ + "description": "fetch data", + "py": "print('ok')", + "dependencies": []any{"requests"}, + }) + if !reflect.DeepEqual(tool.Dependencies, []string{"requests"}) { + t.Fatalf("expected dependencies [requests], got %v", tool.Dependencies) + } + }) + + t.Run("multiple dependencies", func(t *testing.T) { + tool := parseMCPScriptToolConfig("analyze", map[string]any{ + "description": "analyze", + "script": "return { ok: true }", + "dependencies": []any{"lodash", "zod"}, + }) + if !reflect.DeepEqual(tool.Dependencies, []string{"lodash", "zod"}) { + t.Fatalf("expected dependencies [lodash zod], got %v", tool.Dependencies) + } + }) + + t.Run("empty dependencies", func(t *testing.T) { + tool := parseMCPScriptToolConfig("noop", map[string]any{ + "description": "noop", + "run": "echo ok", + "dependencies": []any{}, + }) + if len(tool.Dependencies) != 0 { + t.Fatalf("expected no dependencies, got %v", tool.Dependencies) + } + }) + + t.Run("non-string dependencies are skipped", func(t *testing.T) { + tool := parseMCPScriptToolConfig("mixed", map[string]any{ + "description": "mixed deps", + "py": "print('ok')", + "dependencies": []any{"requests", 123, true, "urllib3"}, + }) + if !reflect.DeepEqual(tool.Dependencies, []string{"requests", "urllib3"}) { + t.Fatalf("expected string dependencies only, got %v", tool.Dependencies) + } + }) +} + +func TestMergeMCPScriptsPreservesDependencies(t *testing.T) { + compiler := NewCompiler() + main := &MCPScriptsConfig{ + Mode: "http", + Tools: map[string]*MCPScriptToolConfig{}, + } + + imported := `{ + "fetch-url": { + "description": "fetch url", + "py": "print('ok')", + "dependencies": ["requests", "urllib3"] + } + }` + + merged := compiler.mergeMCPScripts(main, []string{imported}) + tool, ok := merged.Tools["fetch-url"] + if !ok { + t.Fatal("expected merged tool fetch-url") + } + if !reflect.DeepEqual(tool.Dependencies, []string{"requests", "urllib3"}) { + t.Fatalf("expected dependencies [requests urllib3], got %v", tool.Dependencies) + } +} diff --git a/pkg/workflow/runtime_detection.go b/pkg/workflow/runtime_detection.go index 6af32eb1953..254e965945c 100644 --- a/pkg/workflow/runtime_detection.go +++ b/pkg/workflow/runtime_detection.go @@ -25,6 +25,9 @@ func DetectRuntimeRequirements(workflowData *WorkflowData) []RuntimeRequirement if workflowData.ParsedTools != nil { detectFromMCPConfigs(workflowData.ParsedTools, requirements) } + if workflowData.MCPScripts != nil { + detectFromMCPScripts(workflowData.MCPScripts, requirements) + } // When using a custom image runner, ensure Node.js is set up. // Standard GitHub-hosted runners (ubuntu-*, windows-*) have Node.js pre-installed, @@ -206,6 +209,31 @@ func detectFromMCPConfigs(tools *ToolsConfig, requirements map[string]*RuntimeRe updateRequiredRuntime(runtime, "", requirements) } } + + } +} + +// detectFromMCPScripts scans mcp-scripts tool definitions for language runtimes. +func detectFromMCPScripts(mcpScripts *MCPScriptsConfig, requirements map[string]*RuntimeRequirement) { + if mcpScripts == nil { + return + } + + for _, tool := range mcpScripts.Tools { + switch { + case tool.Script != "": + if runtime := findRuntimeByID("node"); runtime != nil { + updateRequiredRuntime(runtime, "", requirements) + } + case tool.Py != "": + if runtime := findRuntimeByID("python"); runtime != nil { + updateRequiredRuntime(runtime, "", requirements) + } + case tool.Go != "": + if runtime := findRuntimeByID("go"); runtime != nil { + updateRequiredRuntime(runtime, "", requirements) + } + } } } diff --git a/pkg/workflow/runtime_setup_test.go b/pkg/workflow/runtime_setup_test.go index 14a47415338..a58b6988cce 100644 --- a/pkg/workflow/runtime_setup_test.go +++ b/pkg/workflow/runtime_setup_test.go @@ -281,10 +281,36 @@ func TestDetectFromMCPConfigs(t *testing.T) { t.Errorf("Expected runtime %s to be detected", expectedID) } } + }) } } +func TestDetectFromMCPScripts(t *testing.T) { + requirements := make(map[string]*RuntimeRequirement) + detectFromMCPScripts(&MCPScriptsConfig{ + Tools: map[string]*MCPScriptToolConfig{ + "js-tool": {Script: "return { ok: true }"}, + "py-tool": {Py: "print('ok')"}, + "go-tool": {Go: `fmt.Println("ok")`}, + "sh-tool": {Run: "echo ok"}, + }, + }, requirements) + + if _, ok := requirements["node"]; !ok { + t.Fatal("expected node runtime for script tool") + } + if _, ok := requirements["python"]; !ok { + t.Fatal("expected python runtime for py tool") + } + if _, ok := requirements["go"]; !ok { + t.Fatal("expected go runtime for go tool") + } + if _, ok := requirements["shell"]; ok { + t.Fatal("did not expect shell runtime requirement") + } +} + func TestGenerateRuntimeSetupSteps(t *testing.T) { tests := []struct { name string diff --git a/pkg/workflow/runtime_validation.go b/pkg/workflow/runtime_validation.go index 4407cc4d387..c12b1784009 100644 --- a/pkg/workflow/runtime_validation.go +++ b/pkg/workflow/runtime_validation.go @@ -255,6 +255,10 @@ func (c *Compiler) validateRuntimePackages(workflowData *WorkflowData) error { runtimeValidationLog.Printf("Validating runtime packages: found %d runtime requirements", len(requirements)) var errors []string + if err := c.validateMCPScriptDependencies(workflowData); err != nil { + errors = append(errors, err.Error()) + } + for _, req := range requirements { switch req.Runtime.ID { case "node": diff --git a/pkg/workflow/validation_test.go b/pkg/workflow/validation_test.go index 0fa96930b1e..f9fcd78105b 100644 --- a/pkg/workflow/validation_test.go +++ b/pkg/workflow/validation_test.go @@ -4,6 +4,7 @@ package workflow import ( "os/exec" + "strings" "testing" ) @@ -279,6 +280,38 @@ func TestValidateRuntimePackages(t *testing.T) { }, expectError: false, }, + { + name: "invalid mcp-scripts pip dependency name", + workflowData: &WorkflowData{ + MCPScripts: &MCPScriptsConfig{ + Tools: map[string]*MCPScriptToolConfig{ + "fetch-url": { + Name: "fetch-url", + Description: "Fetch URL", + Py: "print('ok')", + Dependencies: []string{"re quests"}, + }, + }, + }, + }, + expectError: true, + }, + { + name: "floating mcp-scripts pip dependency", + workflowData: &WorkflowData{ + MCPScripts: &MCPScriptsConfig{ + Tools: map[string]*MCPScriptToolConfig{ + "fetch-url": { + Name: "fetch-url", + Description: "Fetch URL", + Py: "print('ok')", + Dependencies: []string{"requests"}, + }, + }, + }, + }, + expectError: true, + }, // Note: These tests would fail if npm/uv/pip are available, so we skip them // The actual validation logic is tested by the extraction tests } @@ -295,6 +328,16 @@ func TestValidateRuntimePackages(t *testing.T) { if !tt.expectError && err != nil { t.Errorf("unexpected error: %v", err) } + if tt.expectError && err != nil && tt.name == "invalid mcp-scripts pip dependency name" { + if !strings.Contains(err.Error(), `invalid dependency name "re quests" for tool "fetch-url"`) { + t.Fatalf("expected invalid dependency error message, got: %v", err) + } + } + if tt.expectError && err != nil && tt.name == "floating mcp-scripts pip dependency" { + if !strings.Contains(err.Error(), `dependency "requests" for tool "fetch-url" is not pinned to a release tag`) { + t.Fatalf("expected pinned dependency error message, got: %v", err) + } + } }) } }