Skip to content

[Phase 1] Surface the MCP knob to the review workflow without breaking template sync #679

Closed

Description

@github-actions

Story

As a repo operator enabling MCP review on a repo,
I want provide a convention (a committed MCP config file plus the engine env knob) that turns MCP review on for a repo,
so that MCP can be enabled per-repo without editing org-template-synced workflow files in a way that a standards sync would clobber, and with no change to default behavior.

Acceptance Criteria

  1. engine.sh / the review scripts resolve REVIEW_MCP_CONFIG from the environment, defaulting to a conventional committed path only when that file exists; an absent file means MCP is off.
  2. Enabling MCP requires no change to default review behavior for repos that do not opt in.
  3. If pr-review.yml must pass the knob through, the change is limited to its env: block and recorded as a documented repo-specific exception per AGENTS.md; otherwise the knob is sourced entirely from the committed file with no workflow edit. The chosen approach is justified in the PR.
  4. A sample MCP config (Context7, no auth) is committed as documentation-only and is not active unless the file/knob convention selects it.

Tasks / Subtasks

Dev Notes

  • CONSTRAINT: AGENTS.md 'Workflow Files' states all non-exempt workflow changes must use the org templates in standards/workflows verbatim; pr-review.yml is org-template-synced, so arbitrary edits risk being reverted by standards-sync. Prefer sourcing the knob from a committed config file the scripts discover (no workflow edit). If an env passthrough is unavoidable, add it as a documented exception alongside the existing token-report.yml / pr-review-sweep.yml exception entries.
  • pr-review.yml's env: block is at ~lines 123-145 (CLAUDE_CODE_OAUTH_TOKEN, GH_TOKEN, etc.). Any MCP secret (e.g. a GitHub MCP token) would flow here — but Context7 needs none, so start auth-free to avoid touching the template at all.
  • review-one-pr.sh:27 and review-batch.sh:66 both source engine.sh; the resolved knob must be exported so the subprocess that actually runs the review sees it.
  • Keep no secrets in committed files (AGENTS.md Scripts) — the sample config must be no-auth (Context7).

Project Structure Notes

Adds a committed sample MCP config (documentation-only) plus small resolution logic in scripts. Touch pr-review.yml only if strictly necessary and only as a documented exception.

References

  • AGENTS.md#workflow-files (template-sync constraint + exception precedent)
  • .github/workflows/pr-review.yml:123 (env: block)
  • scripts/review-one-pr.sh:27 (sources engine.sh)
  • scripts/review-batch.sh:66 (sources engine.sh)

Likely target surface

  • scripts/engine.sh
  • scripts/review-one-pr.sh
  • .github/workflows/pr-review.yml

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

Activity

  1. don-petry commented on Jun 14, 2026

    @don-petry
    Collaborator

    Dev-Lead Implementation Plan

    Issue: #679 — [Phase 1] Surface the MCP knob to the review workflow without breaking template sync

    Scope

    Resolve REVIEW_MCP_CONFIG in engine.sh to fall back to a conventional committed path (.github/review-mcp.json) only when that file exists, and commit an inert documentation-only Context7 sample — no edit to the org-template-synced pr-review.yml.

    Chosen enablement path (AC #3)

    Committed-file-only (no workflow edit). pr-review.yml's env: block (lines 123-145) is org-template-synced; editing it risks being clobbered by standards-sync. Instead, engine.sh auto-discovers a committed config at the conventional path when REVIEW_MCP_CONFIG is unset. No workflow change, so no AGENTS.md exception is required. To enable, an operator commits .github/review-mcp.json (e.g. by copying the sample). Context7 needs no auth, so no secret has to flow through the template.

    Implementation Checklist

    Tests to Write

    Test Command

    bats tests/dev-lead/unit/test_engine_mcp.bats (and bash .dev-lead/scripts/dev-lead-lint.sh)

  2. don-petry commented on Jun 14, 2026

    @don-petry
    Collaborator

    Dev-Lead: Implementation Complete

    Plan Execution

    Enablement-path justification (AC #3)

    pr-review.yml's env: block is org-template-synced; arbitrary edits risk being clobbered by standards-sync. The knob is therefore sourced entirely from a committed config the scripts auto-discover — no workflow edit, so no documented exception is required. Context7 needs no auth, so no secret has to flow through the template. An MCP server requiring a secret would still need an env-passthrough exception, which is called out in the docs.

    Test Results

    1..15
    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 pointing to a directory → no MCP flags
    ok 5 agentic: REVIEW_MCP_CONFIG set → MCP flags appended to claude call
    ok 6 agentic: REVIEW_MCP_ALLOWED_TOOLS merged into --allowed-tools
    ok 7 agentic: MCP config set but no allowed-tools knob → base allowed-tools kept
    ok 8 duck: REVIEW_MCP_CONFIG set → MCP flags appended to claude duck call
    ok 9 duck: no MCP knob → no MCP flags on claude duck call
    ok 10 default path: no env + conventional file present → MCP flags use conventional path
    ok 11 default path: no env + conventional file absent → no MCP flags (default unchanged)
    ok 12 default path: explicit REVIEW_MCP_CONFIG takes precedence over conventional path
    ok 13 sample: committed Context7 sample is valid JSON
    ok 14 sample: conventional discovery path is absent in-repo (MCP off by default)
    ok 15 triage: MCP knob set → triage still uses --disallowed-tools only, no MCP
    

    Files Changed

    • scripts/engine.sh: resolve REVIEW_MCP_CONFIG from env, else conventional path .github/review-mcp.json (via overridable REVIEW_MCP_CONFIG_DEFAULT_PATH) only if present; export the resolved knobs.
    • .github/review-mcp.json.sample: documentation-only, no-auth Context7 MCP config — inert because the scripts only discover the suffix-less path.
    • docs/pr-review-agent/pr-review-agent.md: new "MCP review" tuning section (resolution order + how to enable).
    • tests/dev-lead/unit/test_engine_mcp.bats: +5 tests (conventional-path present/absent/precedence; sample valid JSON; discovery path absent in-repo).

    Notes

    • Full dev-lead unit suite: 468/468 passing. dev-lead-lint.sh (shellcheck + agent frontmatter) and markdownlint on the changed doc both pass.
    • Resolution is relative to the cascade's CWD (repo root), matching how the scripts reference prompts/.
    • No _comment key in the sample so a plain cp .github/review-mcp.json.sample .github/review-mcp.json yields a directly-usable config.
  3. added a commit that references this issue on Jun 14, 2026
  4. added 4 commits that reference this issue on Jun 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    dev-leadFor dev-lead agent pickupinitiativeEpic / initiative tracking issue

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions