Skip to content

Implement mcp-scripts.dependencies end-to-end with runtime-manager install flow and pinned release-tag validation#39739

Merged
pelikhan merged 10 commits into
mainfrom
copilot/implement-dependencies-field
Jun 17, 2026
Merged

Implement mcp-scripts.dependencies end-to-end with runtime-manager install flow and pinned release-tag validation#39739
pelikhan merged 10 commits into
mainfrom
copilot/implement-dependencies-field

Conversation

Copilot AI commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

mcp-scripts.dependencies was schema-valid and documented but dropped at parse/runtime, causing late tool failures (module/package not found). This change wires the field through parser → generated tools.json → runtime execution, with validation and install semantics aligned to the spec.

  • Parser + model propagation

    • Added Dependencies []string to MCPScriptToolConfig.
    • Parsed dependencies from tool config (string entries only; non-strings ignored defensively).
    • Preserved dependencies through imported mcp-scripts merge paths.
  • Manifest generation (tools.json)

    • Added dependencies to MCPScriptsToolJSON.
    • Emitted sorted dependency lists for stable output.
  • Runtime-manager integration for installs

    • Added actions/setup/js/mcp_dependencies_manager.cjs.
    • Installs dependencies before first invocation, with per-tool/per-dependency deduping.
    • Package manager inferred from handler type:
      • .cjs/JS → npm install --ignore-scripts --no-save
      • .pypython3 -m pip install
      • .gogo mod init (if needed) + go get
      • .sh → system manager (apt-get/yum/dnf)
    • Failure semantics:
      • deterministic failures: fail fast
      • transient failures: bounded retry (max 2 retries), then terminal tool error
    • Uses the existing MCP logger for dependency-install logging.
    • Removed unnecessary direct require("./shim.cjs") from dependency manager and script runner.
    • Ensured mcp_dependencies_manager.cjs is included in setup.sh required file lists (MCP_SCRIPTS_FILES and SAFE_OUTPUTS_FILES) so runtime assets are always copied.
  • Dependency validation

    • Added compile-time mcp-scripts dependency validation in workflow validation path.
    • Reused existing npm/pip validators where applicable; added Go/shell patterns.
    • Enforced exact version pinning (release tags only) for .md workflow sources; floating dependencies are rejected.
      • JS: name@1.2.3 / @scope/name@1.2.3
      • Python: name==1.2.3
      • Go: module@v1.2.3
      • Shell/system: pinned package form (for example name=1.2.3)
    • Error style follows required template:
      • [what's wrong]. [what's expected]. [example]
  • Runtime detection updates

    • mcp-scripts now contribute runtime requirements:
      • script → node
      • py → python
      • go → go
  • Docs sync

    • Corrected spec sync notes to match actual implementation path.
    • Updated reference/spec language and examples to require pinned dependency versions.
  • Targeted tests

    • Parser tests for single/multi/empty/non-string dependencies and merge preservation.
    • Generator tests for tools.json dependency emission + sort stability.
    • New runtime install-path tests for dependency manager behavior including retry/fail-fast.
    • Added validation tests for rejecting floating dependencies and requiring pinned versions.
    • Added setup file-list coverage to ensure transitive runtime dependencies (including mcp_dependencies_manager.cjs) are included in setup.sh.
mcp-scripts:
  fetch-url:
    description: Fetch URL with requests
    py: |
      import requests, json
      r = requests.get(inputs["url"])
      print(json.dumps({"status": r.status_code}))
    dependencies:
      - requests==2.32.3

@github-actions

Copy link
Copy Markdown
Contributor

Hey @Copilot 👋 — great start on the mcp-scripts dependencies implementation! The issue analysis is thorough and the 7-step plan covers all the right layers (parser → generator → runtime install → validation → tests → docs). A few things to address before this is ready for review:

  • No code yet — this is still a draft with 0 changed files. The plan is solid; the next step is to push the actual implementation.
  • Tests are required — the issue plan calls for mcp_scripts_parser_test.go, mcp_scripts_generator_test.go, and at least one language install-path test covering spec conformance IDs T-DEP-001..006. These must land in the same PR.
  • Spec sync note — remember to correct the inaccurate §4 Sync Note in mcp-scripts-specification.md that falsely claims parseMCPScriptToolConfig already handles dependencies.

If you'd like a hand continuing the implementation, here's a ready-to-use prompt:

Implement the `dependencies` field for mcp-scripts end-to-end in the github/gh-aw repo, following the plan in PR #39739:

1. Add `Dependencies []string` to `MCPScriptToolConfig` in `pkg/workflow/mcp_scripts_parser.go` and parse it from `toolMap` in `parseMCPScriptToolConfig`. Preserve it in `mergeMCPScripts`.
2. Add `Dependencies []string` (json:"dependencies,omitempty") to `MCPScriptsToolJSON` in `pkg/workflow/mcp_scripts_generator.go` and populate it (sorted) in `GenerateMCPScriptsToolsConfig`.
3. In the appropriate mcp_handler_*.cjs and/or mcp_setup_generator.go, install declared dependencies before first tool invocation using the language-inferred package manager (npm/pip/go get/apt). Fail-fast on deterministic errors; bounded retry (max 2) on transient ones.
4. Add dependency-name validation following the patterns in existing pip_validation.go / npm_validation.go, using the project error-message style guide.
5. Add tests in `pkg/workflow/mcp_scripts_parser_test.go` (parse single, multiple, empty, non-string entries) and `pkg/workflow/mcp_scripts_generator_test.go` (dependencies appear in tools.json), covering spec conformance IDs T-DEP-001..006.
6. Correct the inaccurate Sync Note in `docs/src/content/docs/specs/mcp-scripts-specification.md` §4 to reflect the real implementation.
7. Run `make agent-finish` (build, test, recompile, format, lint) before marking the PR ready for review.

Generated by ✅ Contribution Check ·

Copilot AI and others added 2 commits June 17, 2026 06:52
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement dependencies field for mcp-scripts Implement mcp-scripts.dependencies end-to-end with runtime-manager install flow Jun 17, 2026
Copilot AI requested a review from pelikhan June 17, 2026 06:59
@github-actions github-actions Bot mentioned this pull request Jun 17, 2026
@pelikhan pelikhan marked this pull request as ready for review June 17, 2026 12:08
Copilot AI review requested due to automatic review settings June 17, 2026 12:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR completes the end-to-end implementation of mcp-scripts.dependencies in gh-aw, wiring the field from workflow parsing through tools.json generation into MCP runtime execution, and adding compile-time validation and targeted tests to prevent late “module/package not found” failures.

Changes:

  • Propagates dependencies through the mcp-scripts model/parser and emits them into generated tools.json (sorted for stable output).
  • Adds compile-time validation for dependency names (npm/pip/go/system packages) and extends runtime detection for mcp-scripts language requirements.
  • Introduces a runtime dependency installer/gate in the JS MCP server, with retry behavior and tests.
Show a summary per file
File Description
pkg/workflow/validation_test.go Adds a runtime package validation test covering invalid mcp-scripts dependency names.
pkg/workflow/runtime_validation.go Hooks mcp-scripts dependency validation into the runtime package validation path.
pkg/workflow/runtime_setup_test.go Adds test coverage for runtime detection from mcp-scripts tool definitions.
pkg/workflow/runtime_detection.go Detects node/python/go runtime requirements based on mcp-scripts tool language fields.
pkg/workflow/mcp_scripts_parser.go Parses dependencies from tool config and stores them in MCPScriptToolConfig.
pkg/workflow/mcp_scripts_parser_test.go Adds parser tests for dependencies and verifies dependency preservation across merges.
pkg/workflow/mcp_scripts_generator.go Emits dependencies into tools.json and sorts them for stable generation.
pkg/workflow/mcp_scripts_generator_test.go Verifies tools.json includes dependencies and that ordering is stable/sorted.
pkg/workflow/mcp_scripts_dependencies_validation.go Implements compile-time dependency name validation by inferred package manager.
pkg/workflow/mcp_scripts_dependencies_validation_wasm.go Stubs dependency validation for js/wasm builds.
pkg/workflow/mcp_scripts_dependencies_validation_test.go Unit tests for dependency name validation across managers.
docs/src/content/docs/specs/mcp-scripts-specification.md Updates spec sync notes to reflect actual dependency propagation + install path.
docs/src/content/docs/reference/mcp-scripts.md Updates author guidance to use dependencies: for third-party Python packages.
actions/setup/js/mcp_server_core.cjs Wraps tool handlers with a dependency-install gate before first invocation.
actions/setup/js/mcp_scripts_tool_factory.cjs Documents the dependencies field in tool config typedefs.
actions/setup/js/mcp_scripts_mcp_server.cjs Documents the dependencies field in server typedefs.
actions/setup/js/mcp_scripts_config_loader.cjs Documents the dependencies field in config loader typedefs.
actions/setup/js/mcp_dependencies_manager.test.cjs Adds vitest coverage for manager inference, gating, retries, and fail-fast behavior.
actions/setup/js/mcp_dependencies_manager.cjs Implements dependency install logic (npm/pip/go/system) with retry and dedupe gates.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 19/19 changed files
  • Comments generated: 3

Comment on lines +96 to +104
} 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 {
Comment on lines +120 to +140
return async () => {
if (perToolInstallPromises.has(toolKey)) {
return perToolInstallPromises.get(toolKey);
}

const installPromise = (async () => {
for (const dependency of depList) {
const key = `${manager}:${dependency}`;
if (!installedDependencyPromises.has(key)) {
installedDependencyPromises.set(
key,
Promise.resolve().then(() => installDependency(logger, toolName, dependency, manager, basePath))
);
}
await installedDependencyPromises.get(key);
}
})();

perToolInstallPromises.set(toolKey, installPromise);
return installPromise;
};
Comment thread pkg/workflow/validation_test.go Outdated
@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel failed during test quality analysis.

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check.

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot compiler must enforce pinning of dependencies in .md source to release tags. no floating dependencies.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@copilot compiler must enforce pinning of dependencies in .md source to release tags. no floating dependencies.

Addressed in 6f9c344: the compiler now rejects floating mcp-scripts.dependencies values and requires exact release-version pins per runtime (name@1.2.3, name==1.2.3, module@v1.2.3, name=1.2.3). I also added/updated validation tests and docs examples to use pinned dependencies.

Copilot AI changed the title Implement mcp-scripts.dependencies end-to-end with runtime-manager install flow Implement mcp-scripts.dependencies end-to-end with runtime-manager install flow and pinned release-tag validation Jun 17, 2026

@pelikhan pelikhan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot add extensive logging to core.* , require shim.cjs for shims

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot run pr-finisher skill

Copilot AI and others added 2 commits June 17, 2026 14:48
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot revert logging with core and use existing logger

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@copilot add extensive logging to core.* , require shim.cjs for shims

Implemented in 980a20f (plus 58f5439): dependency install flow now emits extensive core.* logs (debug/info/warning/error) via shim-backed logging, and mcp-scripts-runner.cjs now explicitly require("./shim.cjs") so shim globals are always present for mcp-scripts runtime paths.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@copilot revert logging with core and use existing logger

Updated in d5bcd6b: removed core.* dependency-install logging in actions/setup/js/mcp_dependencies_manager.cjs and now use only the existing passed logger methods.

@copilot run pr-finisher skill

Done. I ran the PR-finisher flow for this iteration: triaged PR feedback, applied the requested logging revert in d5bcd6b, ran local validation (make fmt, make lint, make test-unit, make test, make agent-report-progress), and pushed the update.

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot

  • remove require shim.cjs . Not needed
  • ensure all required files are listed in setup.sh

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@copilot

  • remove require shim.cjs . Not needed
  • ensure all required files are listed in setup.sh

Updated in c3505d4: removed the unnecessary require("./shim.cjs") lines from mcp_dependencies_manager.cjs and mcp-scripts-runner.cjs, and added mcp_dependencies_manager.cjs to both MCP_SCRIPTS_FILES and SAFE_OUTPUTS_FILES in actions/setup/setup.sh so required runtime files are fully listed.

@pelikhan pelikhan merged commit 625a9e2 into main Jun 17, 2026
37 checks passed
@pelikhan pelikhan deleted the copilot/implement-dependencies-field branch June 17, 2026 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants