Repository navigation
[Phase 1] Add optional MCP-config passthrough to the Claude review engine #660
Copy link
Copy link
Closed as not planned
Closed as not planned
Copy link
Labels
initiativeEpic / initiative tracking issueEpic / initiative tracking issue
Description
Story
As a maintainer of the review engine,
I want have engine.sh optionally pass an MCP server config to the Claude CLI and grant those servers' tools on the agentic review tiers,
so that agentic reviews can draw on real-time external context when configured, with zero change when it is not.
Acceptance Criteria
- When the
MCP_CONFIGenv var is unset or empty, theclaude --printinvocation in_claude_chain_invokeis byte-identical to today: no--mcp-config/--strict-mcp-configflags and--allowed-toolsremains exactly "Bash,Read,Grep,Glob". - When
MCP_CONFIGis set to a readable MCP config (file path or inline JSON), the agentic claude tiers (deep/audit/action/single) add--mcp-config <value>and append the configured servers' tool grants (mcp__<server>__*) to--allowed-toolsso those tools are usable. - When
MCP_CONFIGis set but unreadable or invalid JSON, the engine emits a::warning::annotation naming the problem and proceeds with the review WITHOUT MCP -- it never aborts, never silently drops the setting, and never fabricates an all-clear. --strict-mcp-configis passed wheneverMCP_CONFIGis supplied, so only the explicit config loads and no ambient/user MCP servers leak into CI.- The fast non-agentic triage tier is left untouched (MCP applies only to the agentic tiers), and
shellcheck --severity=warning -x scripts/engine.shpasses.
Tasks / Subtasks
- Add a helper in engine.sh that reads
MCP_CONFIG, validates readability/JSON, and emits the extraclaudeargs (--mcp-config,--strict-mcp-config) plus themcp__<server>__*tool grants to merge into--allowed-tools. (AC: Go-live improvements for PR review agent #2, Optimize review: small-PR and incremental fast paths #4) - Wire the helper into the
claude)branch of run_agentic for the deep/audit/action/single tiers (engine.sh ~L710-717), composing with the existing--permission-mode acceptEdits --allowed-tools "Bash,Read,Grep,Glob". (AC: test issue from agent #1, Go-live improvements for PR review agent #2) - On unreadable/invalid MCP_CONFIG, emit a
::warning::(matching the existing degradation pattern) and continue without MCP. (AC: Add @claude delegation, auto-merge, and rebase handling #3) - Confirm the triage tier invocation is unchanged and run
shellcheck --severity=warning -x scripts/engine.sh. (AC: feat: add Copilot engine support via REVIEW_ENGINE toggle #5)
Dev Notes
- Single point of truth for the CLI call:
_claude_chain_invoke(scripts/engine.sh ~L410) is the ONLY placeclaude --print --model Xruns; the agentic tiers pass their flags from run_agentic'sclaude)branch (scripts/engine.sh ~L710-717:--permission-mode acceptEdits --allowed-tools "Bash,Read,Grep,Glob"). Add MCP args there asextra_args, do not fork the invocation. - Claude Code CLI contract: MCP servers are supplied via
--mcp-config <path-or-json>and isolated with--strict-mcp-config; each server's tools are exposed asmcp__<server>__<tool>and are unusable unless listed in--allowed-tools. So MCP requires editing BOTH the config flag and the allowed-tools list. - Keep the triage tier (scripts/engine.sh ~L612-619,
_claude_chain_invokefor the fast classify) MCP-free: it is a non-agentic classification pass, not a context-gathering review. - Fail-Loud-Never-Fake: reuse the established
::warning::degradation idiom (e.g. scripts/engine.sh:494 throttle warning; scripts/validate-engines.sh emits self-contained::warning::lines and always exits 0). Degraded MCP must never abort the review nor produce a fake clean verdict. - Scripts standard (AGENTS.md#scripts):
#!/usr/bin/env bashwithset -euo pipefail; no hardcoded tokens -- any auth-bearing MCP server takes its token from env, never from a committed config literal.
Project Structure Notes
All changes confined to scripts/engine.sh, the existing review engine. No new files. Aligns with the repo's shell-orchestration-for-Actions structure.
References
- scripts/engine.sh#_claude_chain_invoke
- scripts/engine.sh (run_agentic claude branch, --allowed-tools)
- scripts/validate-engines.sh (warning/degradation pattern)
- AGENTS.md#scripts
Likely target surface
scripts/engine.sh
Story prepared by the BMAD Scrum Master (Bob) for epic #659. Status: ready-for-dev.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
initiativeEpic / initiative tracking issueEpic / initiative tracking issue
Activity
mcp_serversinput to pr-review.yml and thread it to the engine #661