Add MCP/Context7 connectivity assertion to daily-pr-review-health #903

Activity

1. don-petry

   added

   enhancementFeature requestsFeature requests

   dev-leadFor dev-lead agent pickupFor dev-lead agent pickup

   on Jun 22, 2026
2. don-petry

   added a parent issue on Jun 22, 2026

   • MCP-powered review enrichment for the self-hosted Claude review engine (engine.sh) #676
3. don-petry commented on Jun 22, 2026

   

   don-petry

   on Jun 22, 2026

   CollaboratorAuthor

   More actions

   Dev-Lead Implementation Plan

   Issue: #903 — Add MCP/Context7 connectivity assertion to daily-pr-review-health

   Scope

   Add a durable, affirmative MCP-connectivity assertion to the daily PR-review health check: when .github/review-mcp.json is present, run the configured server(s) through the same flags engine.sh uses (--mcp-config … --strict-mcp-config --debug mcp), assert the MCP server "…": Successfully connected handshake, and fail the health check if MCP is configured but unreachable. No-op when MCP isn't configured.

   Implementation Checklist

   • scripts/mcp_connectivity_check.sh — new script following the pure-helper + main() pattern (mirrors scripts/initiative_canary.sh):
     • pure classify_mcp_output (CONNECTED / FAILED / NO_HANDSHAKE), mcp_check_is_failure, mcp_check_report
     • main(): resolve config path (default .github/review-mcp.json), skip/no-op when absent; else derive mcp__<server>__* allowlist from the config, invoke claude --print --debug mcp --mcp-config … --strict-mcp-config --allowed-tools …, classify, emit ::notice::[mcp] on success / ::error:: + exit 1 on failure
     • reuse the REVIEW_MCP_DEBUG knob so the affirmative ::notice::[mcp] is the success signal
   • .github/workflows/daily-pr-review-health.yml — add an additive final step that runs the assertion, guarded by hashFiles('.github/review-mcp.json') so non-MCP repos are unaffected; sets REVIEW_MCP_DEBUG: '1'
   • .github/workflows/lint.yml — register the new bats test file in the bats job list

   Tests to Write

   • tests/test_mcp_connectivity_check.bats:
     • pure classify_mcp_output: success handshake → CONNECTED; explicit failure marker → FAILED; neither → NO_HANDSHAKE
     • mcp_check_is_failure: only CONNECTED is non-failure
     • main() with a mocked claude on PATH: healthy handshake → exit 0 + ::notice::[mcp]; failure/no-handshake → exit 1 + ::error::; missing config → skip/no-op exit 0; passes the engine.sh flags (--mcp-config, --strict-mcp-config, --debug mcp)

   Test Command

   bats tests/test_mcp_connectivity_check.bats (full suite via scripts/run-bats.sh …), plus bash .dev-lead/scripts/dev-lead-lint.sh
4. don-petry commented on Jun 22, 2026

   

   don-petry

   on Jun 22, 2026

   CollaboratorAuthor

   More actions

   Dev-Lead: Implementation Complete

   Plan Execution

   • Added scripts/mcp_connectivity_check.sh — pure helpers (classify_mcp_output, mcp_check_is_failure, derive_allowed_tools, mcp_check_report) + main() that probes the configured server(s) through the same flags engine.sh threads (--mcp-config … --strict-mcp-config --debug mcp) and asserts the MCP server "…": Successfully connected handshake
   • Skips/no-ops (exit 0) when MCP isn't configured; fails (exit 1) when MCP is configured but unreachable — and treats no handshake at all as a failure (a durable monitor must not pass on the mere absence of an error)
   • Reuses the REVIEW_MCP_DEBUG knob (feat(pr-review): add REVIEW_MCP_DEBUG knob to surface the MCP handshake #892): emits the affirmative ::notice::[mcp] in engine.sh's exact format as the success signal
   • Wired into .github/workflows/daily-pr-review-health.yml as an additive final step, guarded by hashFiles('.github/review-mcp.json') so non-MCP repos are unaffected
   • Registered tests/test_mcp_connectivity_check.bats in the lint.yml bats job
   • Repo lint (.dev-lead/scripts/dev-lead-lint.sh) + shellcheck clean; both workflow YAMLs validated

   Test Results

   20 new tests — tests/test_mcp_connectivity_check.bats:
   ok 1..7   classify_mcp_output (CONNECTED / FAILED / NO_HANDSHAKE; case-insensitive; empty→NO_HANDSHAKE)
   ok 8..10  mcp_check_is_failure (only CONNECTED is non-failure)
   ok 11..13 derive_allowed_tools (single/multi server, no-server→empty)
   ok 14..15 mcp_check_report (CONNECTED / FAILED rendering)
   ok 16     main: healthy handshake → exit 0, ::notice::[mcp], engine.sh flags passed
   ok 17     main: connection failure → exit 1, ::error::, MCP_FAILED set
   ok 18     main: no handshake at all → exit 1 (silent outage = failure)
   ok 19     main: MCP not configured → skip/no-op, exit 0
   ok 20     main: REVIEW_MCP_ALLOWED_TOOLS overrides the derived allowlist
   
   dev-lead-lint.sh: shellcheck (75 files) + agent profiles → all checks passed
   shellcheck --severity=warning -x scripts/mcp_connectivity_check.sh → clean
   

   Files Changed

   • scripts/mcp_connectivity_check.sh (new): the connectivity assertion (pure helpers + main()), layout mirrors scripts/initiative_canary.sh
   • tests/test_mcp_connectivity_check.bats (new): 20 unit/integration tests (pure helpers + mocked-claude main())
   • .github/workflows/daily-pr-review-health.yml: added the Assert MCP connectivity step (additive, hashFiles-guarded, REVIEW_MCP_DEBUG: '1')
   • .github/workflows/lint.yml: registered the new bats file in the bats job

   Notes

   • The probe relies on the fact that with --debug mcp --strict-mcp-config the CLI connects to the configured servers at session start (to enumerate tools), so a trivial prompt is enough to force the handshake — matching the feat(pr-review): add REVIEW_MCP_DEBUG knob to surface the MCP handshake #892 verification reference. The allowlist is derived from the config's mcpServers keys (mcp__<server>__*), overridable via REVIEW_MCP_ALLOWED_TOOLS.
   • The assertion is the last step in the workflow so it is purely additive: the existing log-analysis report still runs/posts; the job conclusion now also reflects MCP health.
   • Pre-existing unrelated failure: tests/dev-lead/unit/test_engine_mcp.bats test Compliance: claude-missing-check-run-trigger #64 ("debug: REVIEW_MCP_DEBUG set + healthy handshake → ::notice:: surfaced") fails in this environment even on a clean branch (verified by stashing all of my changes). It is unrelated to Add MCP/Context7 connectivity assertion to daily-pr-review-health #903 and was left untouched per the "do not modify unrelated files" constraint.
5. don-petry

   added 2 commits that reference this issue on Jun 22, 2026

   Harden MCP connectivity check per review feedback (#903)

   ...

   c8e4bd1

   Add MCP/Context7 connectivity assertion to daily PR-review health che…

   ...

   ac05d65
6. don-petry

   closed this as completedin #905on Jun 22, 2026
7. don-petry

   added 4 commits that reference this issue on Jun 23, 2026

   feat: implement issue #903 — Add MCP/Context7 connectivity assertion …

   ...

   a92cb46

   feat: implement issue #903 — Add MCP/Context7 connectivity assertion …

   ...

   2888396

   Add MCP/Context7 connectivity assertion to daily PR-review health che…

   ...

   3f7a22e

   feat: implement issue #903 — Add MCP/Context7 connectivity assertion …

   ...

   aed1c80