fix: isolate review MCP config from project .mcp.json

[DGouron]

Summary

• Use --mcp-config + --strict-mcp-config CLI flags to inject only the review-progress MCP server during automated reviews
• Removes ensureProjectMcpConfig() call from invokeClaudeReview() — the project's .mcp.json is no longer read or modified
• Adds buildMcpConfigJson() function with 3 dedicated tests

Root cause

MR 4883 reported "outils MCP indisponibles" during an initial review. Investigation revealed the project's .mcp.json contained a remote gitnexus MCP server (manually added, unrelated to ReviewFlow). When that server was unreachable, Claude Code's MCP initialization timed out entirely, making all MCP tools unavailable — including review-progress.

Test plan

• 3 new tests for buildMcpConfigJson() (valid JSON, single server, throws on missing path)
• 5 existing ensureProjectMcpConfig tests still pass
• Full suite: 945 tests pass
• Typecheck + lint clean
• Deploy and verify next review produces inline comments on a code MR

🤖 Generated with Claude Code

[DGouron]

Code Review - PR #114 (fix: isolate review MCP config from project .mcp.json)

Date: 2026-03-06 | Reviewer: Claude Code (Mentor Mode) | Branch: fix/mcp-isolation-strict-config | Modified files: 2 (+62/-3 lines)

---

Executive Summary

Audit	Score	Verdict
Clean Architecture	9/10	Correct layer placement, good isolation improvement
Strategic DDD	8/10	N/A for this scope - pure infrastructure change
SOLID	8/10	SRP respected, function is focused
Testing	7/10	New function tested, but dead code left with tests
Security	9/10	No secrets, no sensitive data exposed

Overall Score: 8/10 - Clean, focused fix that solves a real operational problem. Minor cleanup needed.

---

Analysis

This MR solves a concrete operational problem: third-party MCP servers (e.g. gitnexus) defined in the project's .mcp.json were causing initialization timeouts during reviews.

Solution: Instead of writing to the project's .mcp.json, the review now builds a self-contained MCP config via buildMcpConfigJson(), passes it via --mcp-config + --strict-mcp-config CLI flags. This isolates reviews from project MCP configuration.

The approach is correct and clean.

---

Important Corrections (before merge)

1. Dead code: ensureProjectMcpConfig is now unused

Files: claudeInvoker.ts:61-91, test file lines 42-133

The ensureProjectMcpConfig() function is still defined, exported, and tested (5 tests), but its only call site was removed. No other production file references it.

"Dead code is a maintenance burden. It confuses readers, must be maintained, and consumes cognitive load. Delete it."
— Robert C. Martin, Clean Code, Chapter 17 (G9)

Fix: Remove the function, its tests, and the .gitignore comment at line 42. Consider renaming the test file to buildMcpConfigJson.test.ts.

---

2. Relative import in test file

File: ensureProjectMcpConfig.test.ts:2

// Current (violates project convention):
import { ... } from '../../../../frameworks/claude/claudeInvoker.js';

// Expected:
import { ... } from '@/frameworks/claude/claudeInvoker.js';

Pre-existing issue, but the line was modified in this MR.

---

Positive Observations

Aspect	Observation
Problem solving	--strict-mcp-config is the correct fix - isolates without side effects
Test coverage	buildMcpConfigJson tested with 3 focused cases (happy path, structure, error)
Code clarity	JSDoc + inline comments explain the rationale
Minimal diff	Focused change, minimal blast radius

---

Pre-Merge Checklist

• Remove dead ensureProjectMcpConfig function and its tests
• Fix relative import to use @/ alias in test file
• Clean up .gitignore comment referencing ensureProjectMcpConfig
• yarn verify passes without error

[REVIEW_STATS:blocking=0:warnings=2:suggestions=1:score=8]

[DGouron]

🔄 Follow-up Review - PR #114

Date: 2026-03-06
Previous review: Initial review on PR #114

---

Blocking Issues Verification

#	Issue	Status	Comment
1	Dead code: ensureProjectMcpConfig unused after refactor	✅ FIXED	Function removed, readFileSync import cleaned up, .gitignore comment removed, tests rewritten for new buildMcpConfigJson()
2	Relative import violates @/ alias convention in test file	✅ FIXED	Changed to @/frameworks/claude/claudeInvoker.js

---

New Problems Detected

No new problems detected in the modifications.

The refactor is clean:

• buildMcpConfigJson() is a pure function returning a JSON string — simpler and more testable than the old ensureProjectMcpConfig which had filesystem side effects
• --mcp-config + --strict-mcp-config flags properly isolate review MCP config from project .mcp.json
• Tests cover valid output, single-server constraint, and error case (MCP server not found)
• No any, no type assertions, no Law of Demeter violations, imports follow @/ convention

---

Final Verdict

Criterion	Status
Blocking issues fixed	2/2
New blockers	0
Ready to merge	✅ Yes

[REVIEW_STATS:blocking=0:warnings=0:suggestions=0:score=9]

✅ READY TO MERGE