Repository navigation
[Phase 2] Regression + graceful-degradation guard for the MCP path #662
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 reviewer of this initiative,
I want have a bats test that locks the no-MCP path to today's behavior and proves graceful degradation,
so that adding MCP can never silently change existing reviews nor fail closed.
Acceptance Criteria
- A bats test asserts that with
MCP_CONFIGunset, the assembled claude args contain neither--mcp-confignor anymcp__tool, and--allowed-toolsequals exactly "Bash,Read,Grep,Glob". - A test asserts that with an invalid/unreadable
MCP_CONFIG, the engine emits a::warning::and still runs the review (degraded, not aborted, not fabricated). - A test asserts that with a valid
MCP_CONFIG,--mcp-config,--strict-mcp-config, and at least onemcp__<server>__*tool grant appear in the invocation. - The new test file is registered in the bats list in
.github/workflows/lint.ymland passes in CI.
Tasks / Subtasks
- Add tests/test_engine_mcp.bats that stubs the
claudebinary on PATH to capture the invoked args, exercising the unset / invalid / valid MCP_CONFIG cases. (AC: test issue from agent #1, Go-live improvements for PR review agent #2, Add @claude delegation, auto-merge, and rebase handling #3) - Append the new test file to the bats list in lint.yml (do not reset the repo-specific extension). (AC: Optimize review: small-PR and incremental fast paths #4)
Dev Notes
- Reuse the existing engine-test harness: tests/test_rate_limit.bats and tests/test_cli_error_routing.bats stub the engine CLI and assert on args/exit routing -- copy that PATH-stub approach to capture
claudeargs without real network/auth. - The regression baseline string to assert verbatim is the current allowed-tools value "Bash,Read,Grep,Glob" (scripts/engine.sh agentic branch). If story 1 changed it, this test must still prove the unset-MCP case keeps it intact.
- The bats list in lint.yml is a documented repo-specific extension (AGENTS.md) -- APPEND the new file; template syncs must not reset it.
- Keep tests offline and deterministic: the stub records argv to a temp file and returns canned JSON; assert presence/absence of flags rather than invoking a real MCP server.
Project Structure Notes
Adds one test file under tests/ and one line to lint.yml's bats list -- matches how engine behavior is already guarded in this repo.
References
- tests/test_rate_limit.bats
- tests/test_cli_error_routing.bats
- .github/workflows/lint.yml (bats list)
- scripts/engine.sh
Likely target surface
tests/test_engine_mcp.bats.github/workflows/lint.yml
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