From 09de44720afad1d162fe1e9a908083bc628415ed Mon Sep 17 00:00:00 2001 From: Jordi Gil Date: Fri, 8 May 2026 13:28:08 -0400 Subject: [PATCH 1/5] ci: add MCP tool name validation against live servers Adds a CI check that starts each pack's MCP servers (from mcps.json) and cross-references the `allowed-tools` declared in SKILL.md frontmatter against the actual tools exposed by the server. This catches tool name mismatches (e.g., pod_list vs pods_list) that would silently break skills at runtime but pass all static linters. Components: - scripts/validate-mcp-tools.sh: bash script that starts container- based MCP servers via podman, sends JSON-RPC initialize + tools/list, and validates each skill's allowed-tools against the response - .github/workflows/mcp-tool-validation.yml: GitHub Actions workflow using Kind cluster + podman, triggers on changes to mcps.json or SKILL.md files Co-authored-by: Cursor --- .github/workflows/mcp-tool-validation.yml | 77 +++++++ scripts/validate-mcp-tools.sh | 268 ++++++++++++++++++++++ 2 files changed, 345 insertions(+) create mode 100644 .github/workflows/mcp-tool-validation.yml create mode 100755 scripts/validate-mcp-tools.sh diff --git a/.github/workflows/mcp-tool-validation.yml b/.github/workflows/mcp-tool-validation.yml new file mode 100644 index 00000000..43f59415 --- /dev/null +++ b/.github/workflows/mcp-tool-validation.yml @@ -0,0 +1,77 @@ +name: MCP Tool Validation + +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + paths: + - '**/mcps.json' + - '**/skills/*/SKILL.md' + push: + branches: [main] + paths: + - '**/mcps.json' + - '**/skills/*/SKILL.md' + workflow_dispatch: + +jobs: + mcp-tool-check: + if: github.event.pull_request.draft == false || github.event_name != 'pull_request' + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.11' + + - name: Install podman + run: | + sudo apt-get update -qq + sudo apt-get install -y -qq podman + + - name: Create Kind cluster + uses: helm/kind-action@v1 + with: + cluster_name: mcp-validation + wait: 60s + + - name: Detect changed packs + if: github.event_name == 'pull_request' + id: detect + env: + GITHUB_EVENT_NAME: ${{ github.event_name }} + GITHUB_BASE_REF: ${{ github.base_ref }} + run: | + chmod +x scripts/detect-changed-packs.sh + CHANGED_PACKS=$(./scripts/detect-changed-packs.sh || true) + if [ -z "$CHANGED_PACKS" ]; then + echo "changed=false" >> $GITHUB_OUTPUT + else + echo "changed=true" >> $GITHUB_OUTPUT + echo "packs=$(echo $CHANGED_PACKS | tr '\n' ' ')" >> $GITHUB_OUTPUT + echo "Changed packs: $CHANGED_PACKS" + fi + + - name: Validate MCP tools + env: + KUBECONFIG: ${{ env.KUBECONFIG }} + run: | + chmod +x scripts/validate-mcp-tools.sh + + if [ "${{ github.event_name }}" = "pull_request" ]; then + if [ "${{ steps.detect.outputs.changed }}" = "true" ]; then + echo "Validating changed packs: ${{ steps.detect.outputs.packs }}" + ./scripts/validate-mcp-tools.sh ${{ steps.detect.outputs.packs }} + else + echo "No packs changed, skipping" + exit 0 + fi + else + echo "Validating all packs..." + ./scripts/validate-mcp-tools.sh + fi diff --git a/scripts/validate-mcp-tools.sh b/scripts/validate-mcp-tools.sh new file mode 100755 index 00000000..aa5acd17 --- /dev/null +++ b/scripts/validate-mcp-tools.sh @@ -0,0 +1,268 @@ +#!/usr/bin/env bash +# Validate that `allowed-tools` declared in SKILL.md frontmatter match +# the actual tools exposed by the MCP server defined in mcps.json. +# +# Prerequisites: +# - podman (to run MCP server containers) +# - A valid KUBECONFIG (e.g., from a Kind cluster) +# - python3 with PyYAML +# +# Usage: +# ./scripts/validate-mcp-tools.sh [pack1] [pack2] ... +# No args: validates all packs that have mcps.json +# With args: validates only specified packs + +set -euo pipefail + +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[0;33m' +BLUE='\033[0;34m' +BOLD='\033[1m' +NC='\033[0m' + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" + +TOTAL_SKILLS=0 +PASSED_SKILLS=0 +FAILED_SKILLS=0 +SKIPPED_SKILLS=0 +HAS_ERRORS=false + +echo "" +echo -e "${BOLD}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" +echo -e "${BOLD} MCP Tool Validation Report${NC}" +echo -e "${BOLD} Verifying allowed-tools against live MCP servers${NC}" +echo -e "${BOLD}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" +echo "" + +if [ -z "${KUBECONFIG:-}" ]; then + if [ -f "$HOME/.kube/config" ]; then + export KUBECONFIG="$HOME/.kube/config" + else + echo -e "${RED}ERROR: KUBECONFIG not set and ~/.kube/config not found${NC}" + exit 1 + fi +fi + +query_mcp_tools() { + local command="$1" + shift + local args=("$@") + + local expanded_args=() + for arg in "${args[@]}"; do + expanded_args+=("$(echo "$arg" | sed "s|\${KUBECONFIG}|${KUBECONFIG}|g")") + done + + local init_msg='{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"mcp-tool-validator","version":"0.1"}}}' + local list_msg='{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}' + + local tools_json + tools_json=$( + (echo "$init_msg"; sleep 3; echo "$list_msg"; sleep 3) \ + | "$command" "${expanded_args[@]}" 2>/dev/null \ + | python3 -c " +import sys, json +for line in sys.stdin: + line = line.strip() + if not line: + continue + try: + msg = json.loads(line) + if msg.get('id') == 2: + tools = msg.get('result', {}).get('tools', []) + print(json.dumps([t['name'] for t in tools])) + break + except json.JSONDecodeError: + continue +" 2>/dev/null + ) || true + + echo "$tools_json" +} + +extract_allowed_tools() { + local skill_file="$1" + python3 -c " +import sys + +in_frontmatter = False +for line in open('$skill_file'): + stripped = line.strip() + if stripped == '---': + if not in_frontmatter: + in_frontmatter = True + continue + else: + break + if in_frontmatter and stripped.startswith('allowed-tools:'): + value = stripped[len('allowed-tools:'):].strip() + if value: + print(value) + break +" 2>/dev/null +} + +if [ $# -eq 0 ]; then + PACKS=() + for mcps_file in "$REPO_ROOT"/*/mcps.json; do + pack_dir="$(dirname "$mcps_file")" + pack_name="$(basename "$pack_dir")" + if [ -d "$pack_dir/skills" ]; then + PACKS+=("$pack_name") + fi + done +else + PACKS=("$@") +fi + +if [ ${#PACKS[@]} -eq 0 ]; then + echo -e "${BLUE}No packs with mcps.json found${NC}" + exit 0 +fi + +echo -e "${BLUE}Packs to validate: ${PACKS[*]}${NC}" +echo "" + +EXIT_STATUS=0 + +for pack in "${PACKS[@]}"; do + PACK_DIR="$REPO_ROOT/$pack" + MCPS_FILE="$PACK_DIR/mcps.json" + + if [ ! -f "$MCPS_FILE" ]; then + echo -e "${YELLOW}⚠️ $pack: mcps.json not found, skipping${NC}" + continue + fi + + echo -e "${BOLD}── $pack ──${NC}" + + MCP_SERVERS=$(python3 -c " +import json +with open('$MCPS_FILE') as f: + cfg = json.load(f) +for name, server in cfg.get('mcpServers', {}).items(): + cmd = server.get('command', '') + if cmd in ('podman', 'docker'): + print(name) +" 2>/dev/null) + + declare -A SERVER_TOOLS + + for server_name in $MCP_SERVERS; do + echo -n " Starting MCP server '$server_name'... " + + read -r cmd args_json < <(python3 -c " +import json +with open('$MCPS_FILE') as f: + cfg = json.load(f) +server = cfg['mcpServers']['$server_name'] +print(server['command'], json.dumps(server['args'])) +" 2>/dev/null) + + mapfile -t args < <(python3 -c " +import json, sys +for a in json.loads(sys.argv[1]): + print(a) +" "$args_json" 2>/dev/null) + + tools_json=$(query_mcp_tools "$cmd" "${args[@]}") + + if [ -z "$tools_json" ] || [ "$tools_json" = "[]" ]; then + echo -e "${YELLOW}no tools returned (server may require credentials)${NC}" + SERVER_TOOLS[$server_name]="" + continue + fi + + tool_count=$(python3 -c "import json; print(len(json.loads('$tools_json')))") + echo -e "${GREEN}$tool_count tools available${NC}" + SERVER_TOOLS[$server_name]="$tools_json" + done + + ALL_AVAILABLE_TOOLS=$(python3 -c " +import json +all_tools = set() +$(for sn in $MCP_SERVERS; do + tools="${SERVER_TOOLS[$sn]:-[]}" + if [ -n "$tools" ]; then + echo "all_tools.update(json.loads('$tools'))" + fi + done) +print(json.dumps(sorted(all_tools))) +" 2>/dev/null) + + echo -e " ${BLUE}All available tools: $(python3 -c "import json; print(', '.join(json.loads('$ALL_AVAILABLE_TOOLS')))")${NC}" + echo "" + + for skill_dir in "$PACK_DIR"/skills/*/; do + skill_file="$skill_dir/SKILL.md" + if [ ! -f "$skill_file" ]; then + continue + fi + + skill_name="$(basename "$skill_dir")" + TOTAL_SKILLS=$((TOTAL_SKILLS + 1)) + + allowed=$(extract_allowed_tools "$skill_file") + + if [ -z "$allowed" ]; then + SKIPPED_SKILLS=$((SKIPPED_SKILLS + 1)) + echo -e " ${YELLOW}⚠️ $pack/$skill_name: no allowed-tools declared, skipping${NC}" + continue + fi + + missing="" + for tool in $allowed; do + is_present=$(python3 -c " +import json +tools = json.loads('$ALL_AVAILABLE_TOOLS') +print('yes' if '$tool' in tools else 'no') +" 2>/dev/null) + if [ "$is_present" = "no" ]; then + missing="$missing $tool" + fi + done + + if [ -z "$missing" ]; then + PASSED_SKILLS=$((PASSED_SKILLS + 1)) + echo -e " ✅ ${GREEN}$pack/$skill_name${NC}" + else + FAILED_SKILLS=$((FAILED_SKILLS + 1)) + HAS_ERRORS=true + echo -e " ❌ ${RED}$pack/$skill_name${NC}" + echo -e " ${RED}Missing tools:${missing}${NC}" + echo -e " ${RED}Declared: $allowed${NC}" + fi + done + + unset SERVER_TOOLS + declare -A SERVER_TOOLS + echo "" +done + +echo -e "${BOLD}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" +echo "" +echo -e "${BOLD}VALIDATION SUMMARY${NC}" +echo "────────────────────────────────────────────────────────────────" +printf "%-42s${BLUE}%s${NC}\n" "Total Skills:" "$TOTAL_SKILLS" +printf "✅ %-39s${GREEN}%s${NC}\n" "Passed:" "$PASSED_SKILLS" +printf "⚠️ %-39s${YELLOW}%s${NC}\n" "Skipped (no allowed-tools):" "$SKIPPED_SKILLS" +printf "❌ %-39s${RED}%s${NC}\n" "Failed:" "$FAILED_SKILLS" +echo "" + +if [ "$HAS_ERRORS" = true ]; then + echo -e "${RED}${BOLD}❌ VALIDATION FAILED - TOOL NAME MISMATCHES DETECTED${NC}" + echo -e "${RED}Skills declare tools not available in their MCP server.${NC}" + echo -e "${RED}Check mcps.json for the correct tool names.${NC}" + EXIT_STATUS=1 +elif [ "$SKIPPED_SKILLS" -gt 0 ]; then + echo -e "${YELLOW}${BOLD}⚠️ PASSED WITH WARNINGS${NC}" + echo -e "${YELLOW}Some skills have no allowed-tools declaration.${NC}" +else + echo -e "${GREEN}${BOLD}✅ ALL SKILLS PASSED${NC}" +fi + +echo "" +exit $EXIT_STATUS From 60e115aac0c1a13afa2e9216f45b8b50b55d18b5 Mon Sep 17 00:00:00 2001 From: Jordi Gil Date: Fri, 8 May 2026 13:59:51 -0400 Subject: [PATCH 2/5] ci: rewrite MCP tool validator in Python and harden workflow Replace bash script with a pure-Python implementation for cross-OS portability. Key improvements: - Response-based MCP communication via subprocess + select (no sleeps) - Explicit image pulling with error handling - JSON-RPC pagination support (follows nextCursor) - Levenshtein-based "did you mean?" suggestions for mismatched tools - Graceful handling of servers that exit immediately (missing creds) - File path and line number in error output - Logging of skipped non-container MCP servers Workflow hardening: - Pin all GitHub Actions to SHA - Add permissions: contents: read - Add concurrency group with cancel-in-progress - Add timeout-minutes: 10 - Explicit KUBECONFIG capture from Kind - Add workflow_dispatch pack input for manual runs - Inline pack detection (remove dependency on detect-changed-packs.sh) Co-authored-by: Cursor --- .github/workflows/mcp-tool-validation.yml | 54 ++- scripts/validate-mcp-tools.sh | 268 ------------- scripts/validate_mcp_tools.py | 468 ++++++++++++++++++++++ 3 files changed, 503 insertions(+), 287 deletions(-) delete mode 100755 scripts/validate-mcp-tools.sh create mode 100644 scripts/validate_mcp_tools.py diff --git a/.github/workflows/mcp-tool-validation.yml b/.github/workflows/mcp-tool-validation.yml index 43f59415..923cf355 100644 --- a/.github/workflows/mcp-tool-validation.yml +++ b/.github/workflows/mcp-tool-validation.yml @@ -12,20 +12,33 @@ on: - '**/mcps.json' - '**/skills/*/SKILL.md' workflow_dispatch: + inputs: + pack: + description: 'Pack name to validate (empty = all packs)' + required: false + type: string + +permissions: + contents: read + +concurrency: + group: mcp-tool-validation-${{ github.head_ref || github.ref }} + cancel-in-progress: true jobs: mcp-tool-check: if: github.event.pull_request.draft == false || github.event_name != 'pull_request' runs-on: ubuntu-latest + timeout-minutes: 10 steps: - name: Checkout repository - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: fetch-depth: 0 - name: Set up Python - uses: actions/setup-python@v5 + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 with: python-version: '3.11' @@ -35,43 +48,46 @@ jobs: sudo apt-get install -y -qq podman - name: Create Kind cluster - uses: helm/kind-action@v1 + uses: helm/kind-action@0b5c88445f37b7e12a2ce1ecca7805b9046a74db # v1 with: cluster_name: mcp-validation - wait: 60s + wait: 120s + + - name: Capture KUBECONFIG + run: | + echo "KUBECONFIG=$(kind get kubeconfig-path --name mcp-validation 2>/dev/null || echo ${HOME}/.kube/config)" >> "$GITHUB_ENV" - name: Detect changed packs if: github.event_name == 'pull_request' id: detect - env: - GITHUB_EVENT_NAME: ${{ github.event_name }} - GITHUB_BASE_REF: ${{ github.base_ref }} run: | - chmod +x scripts/detect-changed-packs.sh - CHANGED_PACKS=$(./scripts/detect-changed-packs.sh || true) - if [ -z "$CHANGED_PACKS" ]; then - echo "changed=false" >> $GITHUB_OUTPUT + CHANGED=$(git diff --name-only "origin/${{ github.base_ref }}...HEAD" \ + | grep -oP '^[^/]+(?=/(?:mcps\.json|skills/))' \ + | sort -u \ + | tr '\n' ' ') + if [ -z "$CHANGED" ]; then + echo "changed=false" >> "$GITHUB_OUTPUT" else - echo "changed=true" >> $GITHUB_OUTPUT - echo "packs=$(echo $CHANGED_PACKS | tr '\n' ' ')" >> $GITHUB_OUTPUT - echo "Changed packs: $CHANGED_PACKS" + echo "changed=true" >> "$GITHUB_OUTPUT" + echo "packs=$CHANGED" >> "$GITHUB_OUTPUT" + echo "Changed packs: $CHANGED" fi - name: Validate MCP tools env: KUBECONFIG: ${{ env.KUBECONFIG }} run: | - chmod +x scripts/validate-mcp-tools.sh - - if [ "${{ github.event_name }}" = "pull_request" ]; then + if [ "${{ github.event_name }}" = "workflow_dispatch" ] && [ -n "${{ inputs.pack }}" ]; then + python scripts/validate_mcp_tools.py "${{ inputs.pack }}" + elif [ "${{ github.event_name }}" = "pull_request" ]; then if [ "${{ steps.detect.outputs.changed }}" = "true" ]; then echo "Validating changed packs: ${{ steps.detect.outputs.packs }}" - ./scripts/validate-mcp-tools.sh ${{ steps.detect.outputs.packs }} + python scripts/validate_mcp_tools.py ${{ steps.detect.outputs.packs }} else echo "No packs changed, skipping" exit 0 fi else echo "Validating all packs..." - ./scripts/validate-mcp-tools.sh + python scripts/validate_mcp_tools.py fi diff --git a/scripts/validate-mcp-tools.sh b/scripts/validate-mcp-tools.sh deleted file mode 100755 index aa5acd17..00000000 --- a/scripts/validate-mcp-tools.sh +++ /dev/null @@ -1,268 +0,0 @@ -#!/usr/bin/env bash -# Validate that `allowed-tools` declared in SKILL.md frontmatter match -# the actual tools exposed by the MCP server defined in mcps.json. -# -# Prerequisites: -# - podman (to run MCP server containers) -# - A valid KUBECONFIG (e.g., from a Kind cluster) -# - python3 with PyYAML -# -# Usage: -# ./scripts/validate-mcp-tools.sh [pack1] [pack2] ... -# No args: validates all packs that have mcps.json -# With args: validates only specified packs - -set -euo pipefail - -RED='\033[0;31m' -GREEN='\033[0;32m' -YELLOW='\033[0;33m' -BLUE='\033[0;34m' -BOLD='\033[1m' -NC='\033[0m' - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" - -TOTAL_SKILLS=0 -PASSED_SKILLS=0 -FAILED_SKILLS=0 -SKIPPED_SKILLS=0 -HAS_ERRORS=false - -echo "" -echo -e "${BOLD}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" -echo -e "${BOLD} MCP Tool Validation Report${NC}" -echo -e "${BOLD} Verifying allowed-tools against live MCP servers${NC}" -echo -e "${BOLD}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" -echo "" - -if [ -z "${KUBECONFIG:-}" ]; then - if [ -f "$HOME/.kube/config" ]; then - export KUBECONFIG="$HOME/.kube/config" - else - echo -e "${RED}ERROR: KUBECONFIG not set and ~/.kube/config not found${NC}" - exit 1 - fi -fi - -query_mcp_tools() { - local command="$1" - shift - local args=("$@") - - local expanded_args=() - for arg in "${args[@]}"; do - expanded_args+=("$(echo "$arg" | sed "s|\${KUBECONFIG}|${KUBECONFIG}|g")") - done - - local init_msg='{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"mcp-tool-validator","version":"0.1"}}}' - local list_msg='{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}' - - local tools_json - tools_json=$( - (echo "$init_msg"; sleep 3; echo "$list_msg"; sleep 3) \ - | "$command" "${expanded_args[@]}" 2>/dev/null \ - | python3 -c " -import sys, json -for line in sys.stdin: - line = line.strip() - if not line: - continue - try: - msg = json.loads(line) - if msg.get('id') == 2: - tools = msg.get('result', {}).get('tools', []) - print(json.dumps([t['name'] for t in tools])) - break - except json.JSONDecodeError: - continue -" 2>/dev/null - ) || true - - echo "$tools_json" -} - -extract_allowed_tools() { - local skill_file="$1" - python3 -c " -import sys - -in_frontmatter = False -for line in open('$skill_file'): - stripped = line.strip() - if stripped == '---': - if not in_frontmatter: - in_frontmatter = True - continue - else: - break - if in_frontmatter and stripped.startswith('allowed-tools:'): - value = stripped[len('allowed-tools:'):].strip() - if value: - print(value) - break -" 2>/dev/null -} - -if [ $# -eq 0 ]; then - PACKS=() - for mcps_file in "$REPO_ROOT"/*/mcps.json; do - pack_dir="$(dirname "$mcps_file")" - pack_name="$(basename "$pack_dir")" - if [ -d "$pack_dir/skills" ]; then - PACKS+=("$pack_name") - fi - done -else - PACKS=("$@") -fi - -if [ ${#PACKS[@]} -eq 0 ]; then - echo -e "${BLUE}No packs with mcps.json found${NC}" - exit 0 -fi - -echo -e "${BLUE}Packs to validate: ${PACKS[*]}${NC}" -echo "" - -EXIT_STATUS=0 - -for pack in "${PACKS[@]}"; do - PACK_DIR="$REPO_ROOT/$pack" - MCPS_FILE="$PACK_DIR/mcps.json" - - if [ ! -f "$MCPS_FILE" ]; then - echo -e "${YELLOW}⚠️ $pack: mcps.json not found, skipping${NC}" - continue - fi - - echo -e "${BOLD}── $pack ──${NC}" - - MCP_SERVERS=$(python3 -c " -import json -with open('$MCPS_FILE') as f: - cfg = json.load(f) -for name, server in cfg.get('mcpServers', {}).items(): - cmd = server.get('command', '') - if cmd in ('podman', 'docker'): - print(name) -" 2>/dev/null) - - declare -A SERVER_TOOLS - - for server_name in $MCP_SERVERS; do - echo -n " Starting MCP server '$server_name'... " - - read -r cmd args_json < <(python3 -c " -import json -with open('$MCPS_FILE') as f: - cfg = json.load(f) -server = cfg['mcpServers']['$server_name'] -print(server['command'], json.dumps(server['args'])) -" 2>/dev/null) - - mapfile -t args < <(python3 -c " -import json, sys -for a in json.loads(sys.argv[1]): - print(a) -" "$args_json" 2>/dev/null) - - tools_json=$(query_mcp_tools "$cmd" "${args[@]}") - - if [ -z "$tools_json" ] || [ "$tools_json" = "[]" ]; then - echo -e "${YELLOW}no tools returned (server may require credentials)${NC}" - SERVER_TOOLS[$server_name]="" - continue - fi - - tool_count=$(python3 -c "import json; print(len(json.loads('$tools_json')))") - echo -e "${GREEN}$tool_count tools available${NC}" - SERVER_TOOLS[$server_name]="$tools_json" - done - - ALL_AVAILABLE_TOOLS=$(python3 -c " -import json -all_tools = set() -$(for sn in $MCP_SERVERS; do - tools="${SERVER_TOOLS[$sn]:-[]}" - if [ -n "$tools" ]; then - echo "all_tools.update(json.loads('$tools'))" - fi - done) -print(json.dumps(sorted(all_tools))) -" 2>/dev/null) - - echo -e " ${BLUE}All available tools: $(python3 -c "import json; print(', '.join(json.loads('$ALL_AVAILABLE_TOOLS')))")${NC}" - echo "" - - for skill_dir in "$PACK_DIR"/skills/*/; do - skill_file="$skill_dir/SKILL.md" - if [ ! -f "$skill_file" ]; then - continue - fi - - skill_name="$(basename "$skill_dir")" - TOTAL_SKILLS=$((TOTAL_SKILLS + 1)) - - allowed=$(extract_allowed_tools "$skill_file") - - if [ -z "$allowed" ]; then - SKIPPED_SKILLS=$((SKIPPED_SKILLS + 1)) - echo -e " ${YELLOW}⚠️ $pack/$skill_name: no allowed-tools declared, skipping${NC}" - continue - fi - - missing="" - for tool in $allowed; do - is_present=$(python3 -c " -import json -tools = json.loads('$ALL_AVAILABLE_TOOLS') -print('yes' if '$tool' in tools else 'no') -" 2>/dev/null) - if [ "$is_present" = "no" ]; then - missing="$missing $tool" - fi - done - - if [ -z "$missing" ]; then - PASSED_SKILLS=$((PASSED_SKILLS + 1)) - echo -e " ✅ ${GREEN}$pack/$skill_name${NC}" - else - FAILED_SKILLS=$((FAILED_SKILLS + 1)) - HAS_ERRORS=true - echo -e " ❌ ${RED}$pack/$skill_name${NC}" - echo -e " ${RED}Missing tools:${missing}${NC}" - echo -e " ${RED}Declared: $allowed${NC}" - fi - done - - unset SERVER_TOOLS - declare -A SERVER_TOOLS - echo "" -done - -echo -e "${BOLD}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" -echo "" -echo -e "${BOLD}VALIDATION SUMMARY${NC}" -echo "────────────────────────────────────────────────────────────────" -printf "%-42s${BLUE}%s${NC}\n" "Total Skills:" "$TOTAL_SKILLS" -printf "✅ %-39s${GREEN}%s${NC}\n" "Passed:" "$PASSED_SKILLS" -printf "⚠️ %-39s${YELLOW}%s${NC}\n" "Skipped (no allowed-tools):" "$SKIPPED_SKILLS" -printf "❌ %-39s${RED}%s${NC}\n" "Failed:" "$FAILED_SKILLS" -echo "" - -if [ "$HAS_ERRORS" = true ]; then - echo -e "${RED}${BOLD}❌ VALIDATION FAILED - TOOL NAME MISMATCHES DETECTED${NC}" - echo -e "${RED}Skills declare tools not available in their MCP server.${NC}" - echo -e "${RED}Check mcps.json for the correct tool names.${NC}" - EXIT_STATUS=1 -elif [ "$SKIPPED_SKILLS" -gt 0 ]; then - echo -e "${YELLOW}${BOLD}⚠️ PASSED WITH WARNINGS${NC}" - echo -e "${YELLOW}Some skills have no allowed-tools declaration.${NC}" -else - echo -e "${GREEN}${BOLD}✅ ALL SKILLS PASSED${NC}" -fi - -echo "" -exit $EXIT_STATUS diff --git a/scripts/validate_mcp_tools.py b/scripts/validate_mcp_tools.py new file mode 100644 index 00000000..459fba75 --- /dev/null +++ b/scripts/validate_mcp_tools.py @@ -0,0 +1,468 @@ +#!/usr/bin/env python3 +"""Validate that allowed-tools declared in SKILL.md frontmatter match +the actual tools exposed by the MCP servers defined in mcps.json. + +Starts each container-based MCP server via podman, performs JSON-RPC +initialize + tools/list, then cross-references against each skill's +allowed-tools declaration. + +Usage: + python scripts/validate_mcp_tools.py [pack1] [pack2] ... + No args: validates all packs that have mcps.json +""" + +from __future__ import annotations + +import json +import os +import re +import select +import subprocess +import sys +import time +from dataclasses import dataclass, field +from pathlib import Path + +JSONRPC_TIMEOUT = 30 +SERVER_START_TIMEOUT = 15 +PODMAN_PULL_TIMEOUT = 120 + +INIT_MSG = { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": "2024-11-05", + "capabilities": {}, + "clientInfo": {"name": "mcp-tool-validator", "version": "1.0.0"}, + }, +} + +INITIALIZED_NOTIFICATION = { + "jsonrpc": "2.0", + "method": "notifications/initialized", +} + +TOOLS_LIST_MSG = { + "jsonrpc": "2.0", + "id": 2, + "method": "tools/list", + "params": {}, +} + + +@dataclass +class Finding: + skill: str + pack: str + tool: str + file_path: str + line_number: int | None + suggestion: str | None = None + + def __str__(self) -> str: + loc = f"{self.file_path}" + if self.line_number: + loc += f":{self.line_number}" + hint = f' (did you mean "{self.suggestion}"?)' if self.suggestion else "" + return f'{loc}: tool "{self.tool}" not found in MCP servers{hint}' + + +@dataclass +class ValidationResult: + total_skills: int = 0 + passed: int = 0 + skipped: int = 0 + failed: int = 0 + findings: list[Finding] = field(default_factory=list) + skipped_servers: list[str] = field(default_factory=list) + + @property + def success(self) -> bool: + return self.failed == 0 + + +def levenshtein(s1: str, s2: str) -> int: + if len(s1) < len(s2): + return levenshtein(s2, s1) + if len(s2) == 0: + return len(s1) + prev = list(range(len(s2) + 1)) + for i, c1 in enumerate(s1): + curr = [i + 1] + for j, c2 in enumerate(s2): + curr.append(min(prev[j + 1] + 1, curr[j] + 1, prev[j] + (c1 != c2))) + prev = curr + return prev[len(s2)] + + +def suggest_tool(name: str, available: set[str], max_distance: int = 3) -> str | None: + candidates = [(t, levenshtein(name, t)) for t in available] + candidates = [(t, d) for t, d in candidates if d <= max_distance] + if not candidates: + return None + candidates.sort(key=lambda x: x[1]) + return candidates[0][0] + + +def read_response(proc: subprocess.Popen, expected_id: int, timeout: int = JSONRPC_TIMEOUT) -> dict | None: + """Read JSON-RPC responses from proc.stdout until one matches expected_id.""" + deadline = time.time() + timeout + while time.time() < deadline: + remaining = deadline - time.time() + if remaining <= 0: + break + + if proc.poll() is not None: + for line in proc.stdout: + line = line.strip() + if not line: + continue + try: + msg = json.loads(line) + if msg.get("id") == expected_id: + return msg + except json.JSONDecodeError: + continue + return None + + ready, _, _ = select.select([proc.stdout], [], [], min(remaining, 0.5)) + if not ready: + continue + + line = proc.stdout.readline() + if not line: + continue + line = line.strip() + if not line: + continue + try: + msg = json.loads(line) + except json.JSONDecodeError: + continue + if msg.get("id") == expected_id: + return msg + return None + + +def send_jsonrpc(proc: subprocess.Popen, msg: dict) -> None: + proc.stdin.write(json.dumps(msg) + "\n") + proc.stdin.flush() + + +def pull_image(image: str) -> bool: + """Pull a container image, returning True on success.""" + print(f" Pulling image: {image[:80]}...") + try: + result = subprocess.run( + ["podman", "pull", image], + capture_output=True, text=True, timeout=PODMAN_PULL_TIMEOUT, + ) + if result.returncode != 0: + print(f" WARNING: podman pull failed: {result.stderr.strip()[:200]}") + return False + return True + except subprocess.TimeoutExpired: + print(f" WARNING: podman pull timed out after {PODMAN_PULL_TIMEOUT}s") + return False + + +def extract_image_from_args(args: list[str]) -> str | None: + """Find the container image reference in podman run args.""" + skip_next = False + for i, arg in enumerate(args): + if skip_next: + skip_next = False + continue + if arg.startswith("-"): + if arg in ("-v", "-e", "--env", "--entrypoint", "--name", "--network", + "--userns", "--user", "-p", "--publish", "-w", "--workdir"): + skip_next = True + continue + if arg == "run": + continue + if arg in ("--rm", "-i", "-t", "--interactive", "--tty", "-d", "--detach"): + continue + return arg + return None + + +def query_mcp_tools(command: str, args: list[str], kubeconfig: str) -> list[str] | None: + """Start an MCP server and query its tools via JSON-RPC.""" + expanded_args = [a.replace("${KUBECONFIG}", kubeconfig) for a in args] + + proc = subprocess.Popen( + [command] + expanded_args, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + bufsize=1, + ) + + try: + # Check for immediate exit (missing credentials, bad image, etc.) + time.sleep(0.5) + if proc.poll() is not None: + stderr = proc.stderr.read()[:300] if proc.stderr else "" + print(f" Server exited immediately (code {proc.returncode}): {stderr.strip()}") + return None + + send_jsonrpc(proc, INIT_MSG) + resp = read_response(proc, expected_id=1, timeout=SERVER_START_TIMEOUT) + if resp is None: + print(" WARNING: no response to initialize") + return None + + if "error" in resp: + print(f" WARNING: initialize error: {resp['error']}") + return None + + server_info = resp.get("result", {}).get("serverInfo", {}) + print(f" Server: {server_info.get('name', 'unknown')}") + + send_jsonrpc(proc, INITIALIZED_NOTIFICATION) + + all_tools: list[str] = [] + cursor = None + page = 0 + + while True: + page += 1 + tools_msg = { + "jsonrpc": "2.0", + "id": 100 + page, + "method": "tools/list", + "params": {}, + } + if cursor: + tools_msg["params"]["cursor"] = cursor + + send_jsonrpc(proc, tools_msg) + resp = read_response(proc, expected_id=100 + page) + if resp is None: + print(f" WARNING: no response to tools/list (page {page})") + return all_tools if all_tools else None + + if "error" in resp: + print(f" WARNING: tools/list error: {resp['error']}") + return all_tools if all_tools else None + + result = resp.get("result", {}) + tools = result.get("tools", []) + all_tools.extend(t["name"] for t in tools) + + next_cursor = result.get("nextCursor") + if next_cursor: + cursor = next_cursor + else: + break + + return all_tools + + finally: + proc.stdin.close() + try: + proc.terminate() + proc.wait(timeout=5) + except subprocess.TimeoutExpired: + proc.kill() + proc.wait() + + +def parse_frontmatter(skill_path: Path) -> tuple[str, int | None]: + """Extract allowed-tools value and its line number from SKILL.md frontmatter.""" + in_fm = False + with open(skill_path) as f: + for line_num, line in enumerate(f, 1): + stripped = line.strip() + if stripped == "---": + if not in_fm: + in_fm = True + continue + else: + break + if in_fm and stripped.startswith("allowed-tools:"): + value = stripped[len("allowed-tools:"):].strip() + return value, line_num + return "", None + + +def find_packs(repo_root: Path) -> list[str]: + """Find all packs that have both mcps.json and a skills directory.""" + packs = [] + for mcps_file in sorted(repo_root.glob("*/mcps.json")): + pack_dir = mcps_file.parent + if (pack_dir / "skills").is_dir(): + packs.append(pack_dir.name) + return packs + + +def validate_pack(pack: str, repo_root: Path, kubeconfig: str) -> ValidationResult: + """Validate all skills in a single pack against its MCP servers.""" + result = ValidationResult() + pack_dir = repo_root / pack + mcps_file = pack_dir / "mcps.json" + + if not mcps_file.exists(): + print(f" WARNING: {pack}/mcps.json not found, skipping pack") + return result + + with open(mcps_file) as f: + config = json.load(f) + + servers = config.get("mcpServers", {}) + all_available_tools: set[str] = set() + + for server_name, server_config in servers.items(): + command = server_config.get("command", "") + + if command not in ("podman", "docker"): + reason = f"non-container command '{command}'" + print(f" SKIP server '{server_name}': {reason}") + result.skipped_servers.append(f"{server_name} ({reason})") + continue + + args = server_config.get("args", []) + print(f" Starting MCP server '{server_name}'...") + + image = extract_image_from_args(args) + if image: + if not pull_image(image): + print(f" WARNING: could not pull image, attempting to start anyway") + + tools = query_mcp_tools(command, args, kubeconfig) + + if tools is None: + print(f" WARNING: no tools returned (server may require credentials)") + result.skipped_servers.append(f"{server_name} (no tools returned)") + continue + + print(f" {len(tools)} tools available") + all_available_tools.update(tools) + + if all_available_tools: + print(f" Combined tool pool: {len(all_available_tools)} unique tools") + + skills_dir = pack_dir / "skills" + if not skills_dir.exists(): + return result + + for skill_dir in sorted(skills_dir.iterdir()): + skill_file = skill_dir / "SKILL.md" + if not skill_file.exists(): + continue + + skill_name = skill_dir.name + result.total_skills += 1 + + allowed_tools_str, line_number = parse_frontmatter(skill_file) + + if not allowed_tools_str: + result.skipped += 1 + print(f" SKIP {pack}/{skill_name}: no allowed-tools declared") + continue + + declared_tools = allowed_tools_str.split() + missing = [t for t in declared_tools if t not in all_available_tools] + + if not missing: + result.passed += 1 + print(f" PASS {pack}/{skill_name}: all {len(declared_tools)} tools validated") + else: + result.failed += 1 + rel_path = str(skill_file.relative_to(repo_root)) + for tool in missing: + suggestion = suggest_tool(tool, all_available_tools) + finding = Finding( + skill=skill_name, + pack=pack, + tool=tool, + file_path=rel_path, + line_number=line_number, + suggestion=suggestion, + ) + result.findings.append(finding) + print(f" FAIL {pack}/{skill_name}: {finding}") + + return result + + +def main(packs: list[str] | None = None) -> int: + repo_root = Path(__file__).resolve().parent.parent + + kubeconfig = os.environ.get("KUBECONFIG", "") + if not kubeconfig: + default = Path.home() / ".kube" / "config" + if default.exists(): + kubeconfig = str(default) + else: + print("ERROR: KUBECONFIG not set and ~/.kube/config not found") + return 1 + + if not packs: + packs = find_packs(repo_root) + + if not packs: + print("No packs with mcps.json found") + return 0 + + print() + print("=" * 66) + print(" MCP Tool Validation Report") + print(" Verifying allowed-tools against live MCP servers") + print("=" * 66) + print() + print(f"Packs to validate: {', '.join(packs)}") + print(f"KUBECONFIG: {kubeconfig}") + print() + + combined = ValidationResult() + + for pack in packs: + print(f"-- {pack} --") + result = validate_pack(pack, repo_root, kubeconfig) + combined.total_skills += result.total_skills + combined.passed += result.passed + combined.skipped += result.skipped + combined.failed += result.failed + combined.findings.extend(result.findings) + combined.skipped_servers.extend(result.skipped_servers) + print() + + print("=" * 66) + print() + print("VALIDATION SUMMARY") + print("-" * 66) + print(f" Total skills: {combined.total_skills}") + print(f" Passed: {combined.passed}") + print(f" Skipped (no allowed-tools): {combined.skipped}") + print(f" Failed: {combined.failed}") + print() + + if combined.skipped_servers: + print("Skipped MCP servers:") + for s in combined.skipped_servers: + print(f" - {s}") + print() + + if combined.findings: + print("FINDINGS:") + for f in combined.findings: + print(f" - {f}") + print() + + if not combined.success: + print("VALIDATION FAILED - tool name mismatches detected") + print("Check mcps.json for the correct tool names.") + return 1 + elif combined.skipped > 0: + print("PASSED WITH WARNINGS - some skills have no allowed-tools") + else: + print("ALL SKILLS PASSED") + + return 0 + + +if __name__ == "__main__": + sys.exit(main(packs=sys.argv[1:] if len(sys.argv) > 1 else None)) From 527a2f73525bed6f5e86c29fbc87e20b5b051ea7 Mon Sep 17 00:00:00 2001 From: Jordi Gil Date: Tue, 12 May 2026 11:32:49 -0400 Subject: [PATCH 3/5] ci: integrate MCP tool validation into make validate with graceful skip - Add validate_mcp_tools.py to the main `validate` Makefile target - Add standalone `validate-mcp-tools` target with PACK= filter support - Add graceful prerequisite detection: script exits 0 with a SKIP message when podman or KUBECONFIG is unavailable, so `make validate` never breaks in environments without container tooling - Improve tool name suggestions with substring/prefix matching and component overlap scoring alongside the existing Levenshtein distance Co-authored-by: Cursor --- Makefile | 10 +++++- scripts/validate_mcp_tools.py | 57 +++++++++++++++++++++++++++++------ 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index 3cd91f1b..62d06cfe 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: help install validate validate-collection-schema validate-collection-compliance catalog-mirror-json validate-skill-design validate-skill-design-changed generate serve clean test test-full check-uv +.PHONY: help install validate validate-collection-schema validate-collection-compliance catalog-mirror-json validate-skill-design validate-skill-design-changed validate-mcp-tools generate serve clean test test-full check-uv help: @echo "agentic-collections Documentation Generator" @@ -11,6 +11,7 @@ help: @echo " catalog-mirror-json - Regenerate all .catalog/collection.json from YAML" @echo " validate-skill-design - Validate all skills (use PACK=rh-sre for a specific pack)" @echo " validate-skill-design-changed - Validate only changed skills (staged + unstaged, for local dev)" + @echo " validate-mcp-tools - Validate allowed-tools against live MCP servers (requires podman)" @echo " generate - Generate docs/data.json" @echo " serve - Start local server on http://localhost:8000" @echo " test - Quick test (validate + generate + verify)" @@ -42,6 +43,8 @@ validate: check-uv @uv run python scripts/validate_structure.py @echo "Validating collection compliance (.catalog/)..." @uv run python scripts/validate_collection_compliance.py + @echo "Validating MCP tool references..." + @uv run python scripts/validate_mcp_tools.py @echo "✓ Validation passed!" validate-collection-schema: check-uv @@ -59,6 +62,11 @@ validate-skill-design: check-uv validate-skill-design-changed: check-uv @VALIDATE_INCLUDE_UNCOMMITTED=1 ./scripts/ci-validate-changed-skills.sh +validate-mcp-tools: check-uv + @echo "Validating MCP tool references against live servers..." + @uv run python scripts/validate_mcp_tools.py $(if $(PACK),$(PACK)) + @echo "✓ MCP tool validation complete!" + generate: check-uv @echo "Generating documentation..." @uv run python scripts/build_website.py diff --git a/scripts/validate_mcp_tools.py b/scripts/validate_mcp_tools.py index 459fba75..4d1b704b 100644 --- a/scripts/validate_mcp_tools.py +++ b/scripts/validate_mcp_tools.py @@ -97,12 +97,30 @@ def levenshtein(s1: str, s2: str) -> int: def suggest_tool(name: str, available: set[str], max_distance: int = 3) -> str | None: - candidates = [(t, levenshtein(name, t)) for t in available] - candidates = [(t, d) for t, d in candidates if d <= max_distance] - if not candidates: - return None - candidates.sort(key=lambda x: x[1]) - return candidates[0][0] + best: tuple[str, float] | None = None + + for tool in available: + lev = levenshtein(name, tool) + if lev <= max_distance: + score = lev + if best is None or score < best[1]: + best = (tool, score) + + name_lower, tool_lower = name.lower(), tool.lower() + if name_lower in tool_lower or tool_lower in name_lower: + score = 0.5 if name_lower == tool_lower else 1.0 + if best is None or score < best[1]: + best = (tool, score) + + name_parts = set(name_lower.replace("-", "_").split("_")) + tool_parts = set(tool_lower.replace("-", "_").split("_")) + overlap = name_parts & tool_parts + if overlap and len(overlap) >= len(name_parts) * 0.5: + score = 2.0 - len(overlap) / max(len(tool_parts), 1) + if best is None or score < best[1]: + best = (tool, score) + + return best[0] if best else None def read_response(proc: subprocess.Popen, expected_id: int, timeout: int = JSONRPC_TIMEOUT) -> dict | None: @@ -388,17 +406,38 @@ def validate_pack(pack: str, repo_root: Path, kubeconfig: str) -> ValidationResu return result +def check_prerequisites() -> str | None: + """Return an error message if prerequisites are missing, or None if all OK.""" + try: + subprocess.run( + ["podman", "--version"], capture_output=True, text=True, timeout=5, + ) + except FileNotFoundError: + return "podman not found on PATH" + except subprocess.TimeoutExpired: + return "podman --version timed out" + + kubeconfig = os.environ.get("KUBECONFIG", "") + if not kubeconfig: + default = Path.home() / ".kube" / "config" + if not default.exists(): + return "KUBECONFIG not set and ~/.kube/config not found" + return None + + def main(packs: list[str] | None = None) -> int: repo_root = Path(__file__).resolve().parent.parent + skip_reason = check_prerequisites() + if skip_reason: + print(f"SKIP: {skip_reason} -- MCP tool validation requires podman and a cluster") + return 0 + kubeconfig = os.environ.get("KUBECONFIG", "") if not kubeconfig: default = Path.home() / ".kube" / "config" if default.exists(): kubeconfig = str(default) - else: - print("ERROR: KUBECONFIG not set and ~/.kube/config not found") - return 1 if not packs: packs = find_packs(repo_root) From 116742974848ec895abfabb0d644b03300cfd510 Mon Sep 17 00:00:00 2001 From: Jordi Gil Date: Tue, 19 May 2026 11:37:58 -0400 Subject: [PATCH 4/5] ci: harden workflow security and remove unnecessary Kind cluster - Fix shell injection: route all ${{ }} expressions through env vars (inputs.pack, github.base_ref, steps.detect.outputs.packs) - Remove Kind cluster: tools/list doesn't need cluster connectivity, use a dummy kubeconfig instead (~2 min CI savings) - Remove unused `import re` and dead `TOOLS_LIST_MSG` constant - Add error handling for malformed mcps.json files Co-authored-by: Cursor --- .github/workflows/mcp-tool-validation.yml | 49 ++++++++++++++++------- scripts/validate_mcp_tools.py | 17 ++++---- 2 files changed, 41 insertions(+), 25 deletions(-) diff --git a/.github/workflows/mcp-tool-validation.yml b/.github/workflows/mcp-tool-validation.yml index 923cf355..39b2b804 100644 --- a/.github/workflows/mcp-tool-validation.yml +++ b/.github/workflows/mcp-tool-validation.yml @@ -47,21 +47,36 @@ jobs: sudo apt-get update -qq sudo apt-get install -y -qq podman - - name: Create Kind cluster - uses: helm/kind-action@0b5c88445f37b7e12a2ce1ecca7805b9046a74db # v1 - with: - cluster_name: mcp-validation - wait: 120s - - - name: Capture KUBECONFIG + - name: Create dummy kubeconfig run: | - echo "KUBECONFIG=$(kind get kubeconfig-path --name mcp-validation 2>/dev/null || echo ${HOME}/.kube/config)" >> "$GITHUB_ENV" + mkdir -p "$HOME/.kube" + cat > "$HOME/.kube/config" <<'KUBECONFIG' + apiVersion: v1 + kind: Config + clusters: + - cluster: + server: https://localhost:6443 + name: mcp-validation + contexts: + - context: + cluster: mcp-validation + user: mcp-validation + name: mcp-validation + current-context: mcp-validation + users: + - name: mcp-validation + user: + token: dummy-token-for-tool-listing + KUBECONFIG + echo "KUBECONFIG=$HOME/.kube/config" >> "$GITHUB_ENV" - name: Detect changed packs if: github.event_name == 'pull_request' id: detect + env: + BASE_REF: ${{ github.base_ref }} run: | - CHANGED=$(git diff --name-only "origin/${{ github.base_ref }}...HEAD" \ + CHANGED=$(git diff --name-only "origin/$BASE_REF...HEAD" \ | grep -oP '^[^/]+(?=/(?:mcps\.json|skills/))' \ | sort -u \ | tr '\n' ' ') @@ -76,13 +91,17 @@ jobs: - name: Validate MCP tools env: KUBECONFIG: ${{ env.KUBECONFIG }} + EVENT_NAME: ${{ github.event_name }} + INPUT_PACK: ${{ inputs.pack }} + CHANGED_PACKS: ${{ steps.detect.outputs.packs }} + PACKS_CHANGED: ${{ steps.detect.outputs.changed }} run: | - if [ "${{ github.event_name }}" = "workflow_dispatch" ] && [ -n "${{ inputs.pack }}" ]; then - python scripts/validate_mcp_tools.py "${{ inputs.pack }}" - elif [ "${{ github.event_name }}" = "pull_request" ]; then - if [ "${{ steps.detect.outputs.changed }}" = "true" ]; then - echo "Validating changed packs: ${{ steps.detect.outputs.packs }}" - python scripts/validate_mcp_tools.py ${{ steps.detect.outputs.packs }} + if [ "$EVENT_NAME" = "workflow_dispatch" ] && [ -n "$INPUT_PACK" ]; then + python scripts/validate_mcp_tools.py "$INPUT_PACK" + elif [ "$EVENT_NAME" = "pull_request" ]; then + if [ "$PACKS_CHANGED" = "true" ]; then + echo "Validating changed packs: $CHANGED_PACKS" + python scripts/validate_mcp_tools.py $CHANGED_PACKS else echo "No packs changed, skipping" exit 0 diff --git a/scripts/validate_mcp_tools.py b/scripts/validate_mcp_tools.py index 4d1b704b..039e667c 100644 --- a/scripts/validate_mcp_tools.py +++ b/scripts/validate_mcp_tools.py @@ -15,7 +15,6 @@ import json import os -import re import select import subprocess import sys @@ -43,13 +42,6 @@ "method": "notifications/initialized", } -TOOLS_LIST_MSG = { - "jsonrpc": "2.0", - "id": 2, - "method": "tools/list", - "params": {}, -} - @dataclass class Finding: @@ -326,8 +318,13 @@ def validate_pack(pack: str, repo_root: Path, kubeconfig: str) -> ValidationResu print(f" WARNING: {pack}/mcps.json not found, skipping pack") return result - with open(mcps_file) as f: - config = json.load(f) + try: + with open(mcps_file) as f: + config = json.load(f) + except (json.JSONDecodeError, OSError) as e: + print(f" ERROR: failed to parse {pack}/mcps.json: {e}") + result.failed += 1 + return result servers = config.get("mcpServers", {}) all_available_tools: set[str] = set() From 979ca54e9a5960151514029c9a7f5ab179b4e385 Mon Sep 17 00:00:00 2001 From: Jordi Gil Date: Tue, 19 May 2026 11:43:21 -0400 Subject: [PATCH 5/5] ci: address medium-severity audit findings - Fix race condition: replace time.sleep(0.5) with poll loop (10x100ms) for more reliable server startup detection - Document platform requirement (Linux/macOS) in script docstring - Document fetch-depth: 0 rationale in workflow comment - Simplify workflow dispatch logic with early-exit for unchanged PRs - Update Makefile messaging to clarify graceful skip behavior Co-authored-by: Cursor --- .github/workflows/mcp-tool-validation.yml | 15 +++++++-------- Makefile | 4 ++-- scripts/validate_mcp_tools.py | 14 ++++++++------ 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/.github/workflows/mcp-tool-validation.yml b/.github/workflows/mcp-tool-validation.yml index 39b2b804..c1908697 100644 --- a/.github/workflows/mcp-tool-validation.yml +++ b/.github/workflows/mcp-tool-validation.yml @@ -35,7 +35,7 @@ jobs: - name: Checkout repository uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: - fetch-depth: 0 + fetch-depth: 0 # full history required for git diff in pack detection - name: Set up Python uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 @@ -96,16 +96,15 @@ jobs: CHANGED_PACKS: ${{ steps.detect.outputs.packs }} PACKS_CHANGED: ${{ steps.detect.outputs.changed }} run: | + # Dispatch: manual with pack > PR changed packs > all packs if [ "$EVENT_NAME" = "workflow_dispatch" ] && [ -n "$INPUT_PACK" ]; then + echo "Manual dispatch: validating pack '$INPUT_PACK'" python scripts/validate_mcp_tools.py "$INPUT_PACK" + elif [ "$EVENT_NAME" = "pull_request" ] && [ "$PACKS_CHANGED" != "true" ]; then + echo "No packs changed in this PR, skipping" elif [ "$EVENT_NAME" = "pull_request" ]; then - if [ "$PACKS_CHANGED" = "true" ]; then - echo "Validating changed packs: $CHANGED_PACKS" - python scripts/validate_mcp_tools.py $CHANGED_PACKS - else - echo "No packs changed, skipping" - exit 0 - fi + echo "Validating changed packs: $CHANGED_PACKS" + python scripts/validate_mcp_tools.py $CHANGED_PACKS else echo "Validating all packs..." python scripts/validate_mcp_tools.py diff --git a/Makefile b/Makefile index 62d06cfe..60eac6e1 100644 --- a/Makefile +++ b/Makefile @@ -43,9 +43,9 @@ validate: check-uv @uv run python scripts/validate_structure.py @echo "Validating collection compliance (.catalog/)..." @uv run python scripts/validate_collection_compliance.py - @echo "Validating MCP tool references..." + @echo "Validating MCP tool references (skips gracefully without podman)..." @uv run python scripts/validate_mcp_tools.py - @echo "✓ Validation passed!" + @echo "✓ Validation complete!" validate-collection-schema: check-uv @uv run python scripts/validate_collection_schema.py diff --git a/scripts/validate_mcp_tools.py b/scripts/validate_mcp_tools.py index 039e667c..c1339d14 100644 --- a/scripts/validate_mcp_tools.py +++ b/scripts/validate_mcp_tools.py @@ -6,6 +6,8 @@ initialize + tools/list, then cross-references against each skill's allowed-tools declaration. +Platform: Linux and macOS only (uses select.select for non-blocking I/O). + Usage: python scripts/validate_mcp_tools.py [pack1] [pack2] ... No args: validates all packs that have mcps.json @@ -211,12 +213,12 @@ def query_mcp_tools(command: str, args: list[str], kubeconfig: str) -> list[str] ) try: - # Check for immediate exit (missing credentials, bad image, etc.) - time.sleep(0.5) - if proc.poll() is not None: - stderr = proc.stderr.read()[:300] if proc.stderr else "" - print(f" Server exited immediately (code {proc.returncode}): {stderr.strip()}") - return None + for _ in range(10): + if proc.poll() is not None: + stderr = proc.stderr.read()[:300] if proc.stderr else "" + print(f" Server exited immediately (code {proc.returncode}): {stderr.strip()}") + return None + time.sleep(0.1) send_jsonrpc(proc, INIT_MSG) resp = read_response(proc, expected_id=1, timeout=SERVER_START_TIMEOUT)