[Phase 1] Thread an optional MCP config into the Claude agentic review tiers (engine.sh)

[github-actions]

Story

As a maintainer of the pr-review engine,
I want add an opt-in MCP config knob that engine.sh threads into the claude --print agentic and rubber-duck invocations,
so that reviews can draw on real-time MCP context while the default (knob unset) behavior stays byte-for-byte unchanged for every existing adopter.

Acceptance Criteria

1. A new env knob (e.g. REVIEW_MCP_CONFIG, a path to an MCP-servers JSON config) is read at engine.sh init; when unset or empty, the claude invocations gain no new flags and behavior is unchanged.
2. When REVIEW_MCP_CONFIG points to a readable file, the claude branch of run_agentic and run_duck append the MCP config flags (verified for the pinned CLI: --mcp-config <file> and --strict-mcp-config) to the claude --print call.
3. MCP tool names are merged into the existing --allowed-tools list for those tiers via a second knob (e.g. REVIEW_MCP_ALLOWED_TOOLS, default empty), so configured MCP tools (e.g. mcp__context7__*) are actually permitted.
4. The triage tier (which restricts tools via --disallowed-tools) is NOT given MCP config and stays fast/restricted.
5. The exact MCP flag spelling is verified against the pinned @anthropic-ai/claude-code CLI version before merge (claude --help / docs), not guessed, and recorded in a code comment.
6. shellcheck scripts/engine.sh passes.

Tasks / Subtasks

• Add a config block reading REVIEW_MCP_CONFIG and REVIEW_MCP_ALLOWED_TOOLS near the other engine knobs (engine.sh ~lines 30-47). (AC: test issue from agent #1)
• Add a small helper that yields the MCP flag array (--mcp-config/--strict-mcp-config) plus the merged allowed-tools value, emitting nothing when the config file is absent/empty. (AC: test issue from agent #1, Go-live improvements for PR review agent #2, Add @claude delegation, auto-merge, and rebase handling #3)
• Wire the helper into the claude branch of run_agentic (engine.sh ~705-718) and run_duck (~807-823), appended after the existing --permission-mode/--allowed-tools args. (AC: Go-live improvements for PR review agent #2, Add @claude delegation, auto-merge, and rebase handling #3)
• Confirm the triage branch (engine.sh ~613-622) is untouched and still passes --disallowed-tools only. (AC: Optimize review: small-PR and incremental fast paths #4)
• Verify --mcp-config/--strict-mcp-config against the installed CLI version and document the verified flags in a comment. (AC: feat: add Copilot engine support via REVIEW_ENGINE toggle #5)
• Run shellcheck on engine.sh and add/extend a bats test asserting MCP flags appear only when the knob is set. (AC: feat: feature ideation framework — adversarial proposer/challenger/synthesizer #6)

Dev Notes

• This repo has NO claude-code-reusable.yml and no mcp_servers input — the idea's named surface does not exist. The real Claude review engine is scripts/engine.sh, invoked by .github/workflows/pr-review.yml -> review-batch.sh/review-one-pr.sh. Target engine.sh, not a new reusable workflow.
• Invocations to extend: the claude branch of run_agentic (engine.sh:705-718) and run_duck (engine.sh:807-823). Both call _claude_chain_invoke ... --permission-mode acceptEdits --allowed-tools "Bash,Read,Grep,Glob". Append the MCP flags here, matching that bash arg-array style.
• _claude_chain_invoke (engine.sh:410) forwards its extra_args verbatim to claude --print --model X "${extra_args[@]}" (engine.sh:453,462) — passing MCP flags as extra args needs no signature change.
• Do NOT touch the triage tier (engine.sh:613-622): it intentionally restricts tools via --disallowed-tools for speed/cost. MCP belongs only on the deep (run_agentic) and duck (run_duck) agentic tiers.
• The dev-lead writer path (engine.sh ~965-977, --allowed-tools includes Write,Edit,WebFetch) is the issue/CI-fixing pipeline, not PR review — out of scope here.
• CLI is @anthropic-ai/claude-code@${CLAUDE_CODE_VERSION} (pr-review.yml:177, default 'latest'). MCP flags can differ across CLI versions; verify before merge — CLAUDE.md forbids guessing flags/SHAs.
• Standards: scripts are #!/usr/bin/env bash with set -euo pipefail, must pass shellcheck (CLAUDE.md Commands), and carry no hardcoded secrets (AGENTS.md Scripts).

Project Structure Notes

Change is confined to scripts/engine.sh plus a bats test under tests/. No new files or workflows required for the plumbing itself.

References

• scripts/engine.sh:705 (run_agentic claude branch)
• scripts/engine.sh:807 (run_duck claude branch)
• scripts/engine.sh:410 (_claude_chain_invoke forwards extra_args)
• scripts/engine.sh:613 (triage tier, --disallowed-tools)
• .github/workflows/pr-review.yml:177 (CLAUDE_CODE_VERSION pin)
• AGENTS.md#scripts
• CLAUDE.md#commands

Likely target surface

• scripts/engine.sh
• tests/

Story prepared by the BMAD Scrum Master (Bob) for epic #676. Status: ready-for-dev.

[don-petry]

Dev-Lead Implementation Plan

Issue: #677 — [Phase 1] Thread an optional MCP config into the Claude agentic review tiers (engine.sh)

Scope

Add two opt-in env knobs (REVIEW_MCP_CONFIG, REVIEW_MCP_ALLOWED_TOOLS) that engine.sh threads into the claude --print calls of the deep (run_agentic) and rubber-duck (run_duck) tiers, leaving default behavior byte-for-byte unchanged when the config knob is unset.

Implementation Checklist

• scripts/engine.sh: add config block reading REVIEW_MCP_CONFIG/REVIEW_MCP_ALLOWED_TOOLS near the other engine knobs (~lines 30-47)
• scripts/engine.sh: add _mcp_review_flags <base_allowed_tools> helper that populates _MCP_FLAGS (--mcp-config <file> --strict-mcp-config) and _MCP_ALLOWED_TOOLS (base + merged MCP tools), emitting nothing when the config file is absent/unreadable
• scripts/engine.sh: wire the helper into the claude branch of run_agentic (~705-718), appending after --permission-mode/--allowed-tools
• scripts/engine.sh: wire the helper into the claude branch of run_duck (~807-823)
• Confirm the triage branch (~613-622) stays untouched (--disallowed-tools only)
• Document the verified flags (against installed @anthropic-ai/claude-code 2.1.138) in a code comment

Tests to Write

• tests/dev-lead/unit/test_engine_mcp.bats — assert MCP flags absent when knob unset (default), present in run_agentic/run_duck when knob set to a readable file, allowed-tools merged, no flags when file unreadable, and triage tier never gets MCP flags
• Extend tests/dev-lead/fixtures/engines/stub-claude with an opt-in STUB_ENGINE_RECORD_ARGS arg-capture (additive, no behavior change when unset)

Test Command

bats tests/dev-lead/unit/ (plus bash .dev-lead/scripts/dev-lead-lint.sh for shellcheck)

[don-petry]

Dev-Lead: Implementation Complete

Plan Execution

• Added REVIEW_MCP_CONFIG / REVIEW_MCP_ALLOWED_TOOLS config block near the other engine knobs (scripts/engine.sh)
• Added _mcp_review_flags <base_allowed_tools> helper — populates _MCP_FLAGS (--mcp-config <file> --strict-mcp-config) and _MCP_ALLOWED_TOOLS (base + merged MCP tools), no-op when config absent/unreadable
• Wired the helper into the claude branch of run_agentic (deep tier), appended after --permission-mode/--allowed-tools
• Wired the helper into the claude branch of run_duck (rubber-duck tier), preserving --max-turns 25
• Confirmed the triage tier is untouched (still --disallowed-tools only) — verified by test
• Verified --mcp-config/--strict-mcp-config against installed @anthropic-ai/claude-code 2.1.138 (claude --help) and recorded the verified spelling in a code comment
• shellcheck (warning severity) + dev-lead-lint pass; added tests/dev-lead/unit/test_engine_mcp.bats

Acceptance Criteria

1. ✅ New knob read at init; unset/empty → no new flags, behavior unchanged
2. ✅ Readable REVIEW_MCP_CONFIG → --mcp-config <file> --strict-mcp-config appended to deep/duck claude calls
3. ✅ REVIEW_MCP_ALLOWED_TOOLS merged into --allowed-tools for those tiers
4. ✅ Triage tier NOT given MCP config (stays --disallowed-tools only)
5. ✅ Flag spelling verified against pinned CLI (2.1.138), documented in comment — not guessed
6. ✅ shellcheck passes

Test Results

1..9
ok 1 agentic: no MCP knob → no MCP flags, default allowed-tools unchanged
ok 2 agentic: empty REVIEW_MCP_CONFIG → no MCP flags
ok 3 agentic: REVIEW_MCP_CONFIG pointing to unreadable file → no MCP flags
ok 4 agentic: REVIEW_MCP_CONFIG set → MCP flags appended to claude call
ok 5 agentic: REVIEW_MCP_ALLOWED_TOOLS merged into --allowed-tools
ok 6 agentic: MCP config set but no allowed-tools knob → base allowed-tools kept
ok 7 duck: REVIEW_MCP_CONFIG set → MCP flags appended to claude duck call
ok 8 duck: no MCP knob → no MCP flags on claude duck call
ok 9 triage: MCP knob set → triage still uses --disallowed-tools only, no MCP

Full dev-lead unit suite: 462 tests passing, 0 failing.

Files Changed

• scripts/engine.sh: MCP config knobs, _mcp_review_flags helper, wiring into run_agentic + run_duck claude branches
• tests/dev-lead/unit/test_engine_mcp.bats: new — 9 cases covering default-unchanged, flags-on-when-set, allowed-tools merge, unreadable-file no-op, and triage-never-MCP
• tests/dev-lead/fixtures/engines/stub-claude: additive STUB_ENGINE_RECORD_ARGS argv-capture hook (no behavior change when unset)

Notes

• New test lives under tests/dev-lead/unit/ which test-dev-lead.yml auto-discovers via bats tests/dev-lead/unit/ — no workflow bats-list edit required.
• Unreadable REVIEW_MCP_CONFIG is treated as unset (no flags) and emits a one-line stderr notice; phrasing avoids rate-limit tokens so it cannot be misclassified by downstream is_rate_limited scans.
• Empty-array expansion "${_MCP_FLAGS[@]}" follows the existing fmt_args pattern already in _claude_chain_invoke.