diff --git a/agents/api-security-reviewer.md b/agents/api-security-reviewer.md index 43d31194..c2e7dc83 100644 --- a/agents/api-security-reviewer.md +++ b/agents/api-security-reviewer.md @@ -1,7 +1,8 @@ --- name: api-security-reviewer +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent to review backend/API code for security vulnerabilities. Checks authentication, JWT handling, input validation, output security, and access control.\\n\\n\\nContext: User has added new API endpoints or authentication logic.\\nuser: \"I've added the new user registration endpoint\"\\nassistant: \"Let me invoke the api-security-reviewer agent to check for security issues in your new endpoint.\"\\n\\nNew API endpoints should be reviewed for authentication, input validation, and secure response handling.\\n\\n\\n\\n\\nContext: User has modified JWT or session handling.\\nuser: \"Updated the token refresh logic\"\\nassistant: \"I'll run the api-security-reviewer agent to verify the JWT implementation follows security best practices.\"\\n\\nJWT changes require verification of algorithm, expiry, and secret handling.\\n\\n" -tools: Read, Grep, Glob, Bash +tools: Read, Grep, Glob, Bash, mcp__codebase, mcp__context7, mcp__autonomous_bugs model: haiku color: red --- diff --git a/agents/backend-performance-reviewer.md b/agents/backend-performance-reviewer.md index 23f47188..60dde66b 100644 --- a/agents/backend-performance-reviewer.md +++ b/agents/backend-performance-reviewer.md @@ -1,7 +1,8 @@ --- name: backend-performance-reviewer +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent to review backend code for performance issues. Checks database queries, caching strategies, async patterns, and API response optimization.\\n\\n\\nContext: User has added database queries or data fetching logic.\\nuser: \"Added the query to fetch all user tasks\"\\nassistant: \"Let me invoke the backend-performance-reviewer agent to check for N+1 queries and pagination issues.\"\\n\\nDatabase queries should be reviewed for efficiency, proper indexing, and avoiding N+1 patterns.\\n\\n\\n\\n\\nContext: User has implemented caching or heavy data processing.\\nuser: \"Implemented caching for the dashboard data\"\\nassistant: \"I'll run the backend-performance-reviewer agent to verify cache invalidation and TTL strategies.\"\\n\\nCaching implementations need review for proper invalidation and memory considerations.\\n\\n" -tools: Read, Grep, Glob, Bash +tools: Read, Grep, Glob, Bash, mcp__codebase, mcp__context7, mcp__autonomous_bugs model: haiku color: orange --- diff --git a/agents/commit-guard.md b/agents/commit-guard.md index 073cb4cd..4fd3bc0b 100644 --- a/agents/commit-guard.md +++ b/agents/commit-guard.md @@ -1,7 +1,8 @@ --- name: commit-guard +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent before committing code to guard against unintentional duplication using semantic search.\\n\\n\\nContext: User is about to commit staged changes.\\nuser: \"I'm ready to commit these changes\"\\nassistant: \"Let me invoke the commit-guard agent to check for duplicates before you commit.\"\\n\\ncommit-guard runs semantic duplicate detection against the search index and checks DRY rules per file.\\n\\n\\n\\n\\nContext: User has added new utility functions or components.\\nuser: \"Added a new helper function for date formatting\"\\nassistant: \"I'll run the commit-guard agent to verify this doesn't duplicate an existing utility.\"\\n\\nNew utilities should be checked against the indexed codebase to prevent duplication.\\n\\n" -tools: Read, Grep, Glob, Bash, mcp__codebase__searchCode +tools: Read, Grep, Glob, Bash, mcp__codebase, mcp__context7, mcp__autonomous_bugs, mcp__codebase__searchCode model: haiku color: blue --- diff --git a/agents/conventions-reviewer.md b/agents/conventions-reviewer.md index aa781378..86b77f18 100644 --- a/agents/conventions-reviewer.md +++ b/agents/conventions-reviewer.md @@ -1,7 +1,8 @@ --- name: conventions-reviewer +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent to check a diff against the governing CLAUDE.md files of the repo it's installed in. Flags a violation only when it can quote both the exact rule and the exact offending line; otherwise stays silent. No style opinions.\\n\\n\\nContext: A CLAUDE.md rule says never hand-edit generated files.\\nuser: \"Updated the generated icon exports directly\"\\nassistant: \"I'll run the conventions-reviewer agent to check whether that edit violates the repo's own generated-file rule.\"\\n\\nA written CLAUDE.md rule with an unhedged directive and a concrete offending line is exactly what this reviewer exists to catch.\\n\\n\\n\\n\\nContext: A nested package has its own CLAUDE.md scoping a rule to that package only.\\nuser: \"Added a new file under packages/api\"\\nassistant: \"Let me invoke the conventions-reviewer agent — packages/api's own CLAUDE.md may govern this file, on top of the repo root's.\"\\n\\nScoping matters: a rule in one package's CLAUDE.md never governs a sibling package's files.\\n\\n" -tools: Read, Grep, Glob +tools: Read, Grep, Glob, mcp__codebase, mcp__context7, mcp__autonomous_bugs model: haiku color: cyan --- diff --git a/agents/correctness-reviewer.md b/agents/correctness-reviewer.md index bbc10888..070d4447 100644 --- a/agents/correctness-reviewer.md +++ b/agents/correctness-reviewer.md @@ -1,7 +1,8 @@ --- name: correctness-reviewer +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent to review a finished diff for correctness bugs: concurrency/race conditions, state-machine dead states, writer/reader contract mismatches across modules, broadcast/dedup errors, discarded return values, and classifier/parsing edge cases.\\n\\n\\nContext: User has implemented retry/recovery logic that writes task statuses.\\nuser: \"The task retry flow is done\"\\nassistant: \"Let me invoke the correctness-reviewer agent to trace every status write to its readers and walk the concurrent interleavings.\"\\n\\nStatus writes need CAS guards and every consumer (pollers, filters, queries) must still select the written state.\\n\\n\\n\\n\\nContext: User added an event that is broadcast to multiple listeners.\\nuser: \"chat-ready now fires on resume\"\\nassistant: \"I'll run the correctness-reviewer agent to enumerate the listeners and check for duplicate handling.\"\\n\\nA broadcast consumed by N listeners without targeting/dedup executes its effect N times.\\n\\n" -tools: Read, Grep, Glob, Bash +tools: Read, Grep, Glob, Bash, mcp__codebase, mcp__context7, mcp__autonomous_bugs model: sonnet color: orange --- diff --git a/agents/feature-completeness-reviewer.md b/agents/feature-completeness-reviewer.md index 2b2b33f7..07426602 100644 --- a/agents/feature-completeness-reviewer.md +++ b/agents/feature-completeness-reviewer.md @@ -1,5 +1,6 @@ --- name: feature-completeness-reviewer +mcpServers: [codebase, context7, autonomous_bugs] model: opus description: Use this agent proactively after any significant code or feature implementation to identify missing components, overlooked dependencies, and gaps in the solution. Specifically invoke this agent when:\\n\\n\\nContext: The user has just implemented a new feature for real-time chat functionality.\\nuser: \"I've just finished implementing the core chat messaging system\"\\nassistant: \"Great work on the messaging system! Let me use the Task tool to launch the feature-completeness-reviewer agent to analyze what supporting components might be needed.\"\\n\\nSince a significant feature was implemented, proactively use the feature-completeness-reviewer agent to identify missing pieces like debugging tools, monitoring capabilities, or infrastructure requirements.\\n\\n\\n\\n\\nContext: The user has completed a major refactoring of the authentication system.\\nuser: \"The authentication refactor is done, all tests passing\"\\nassistant: \"Excellent! Now let me invoke the feature-completeness-reviewer agent to examine if we've covered all the necessary touchpoints.\"\\n\\nAfter completing authentication work, use the feature-completeness-reviewer agent to check for missing elements like session management, token refresh mechanisms, or user experience flows.\\n\\n\\n\\n\\nContext: A new API endpoint has been added to the system.\\nuser: \"I've added the new /api/status endpoint\"\\nassistant: \"Perfect! Let me use the feature-completeness-reviewer agent to assess the broader implications and any missing pieces.\"\\n\\nWhen new APIs are created, proactively launch the feature-completeness-reviewer agent to identify missing documentation, error handling, rate limiting, or integration points.\\n\\n --- diff --git a/agents/feature-critique.md b/agents/feature-critique.md index ef8be3ec..479f85de 100644 --- a/agents/feature-critique.md +++ b/agents/feature-critique.md @@ -1,5 +1,6 @@ --- name: feature-critique +mcpServers: [codebase, context7, autonomous_bugs] model: opus description: Pre-implementation critique specialist. Analyzes feature proposals, architectural decisions, and technical approaches BEFORE work begins. Evaluates feasibility, UX implications, security concerns, codebase conflicts, data flow correctness, runtime behavior across user configurations, and missing considerations using evidence-based research. Invoke when a feature proposal, technical approach, or architectural decision needs critical evaluation before implementation starts. --- diff --git a/agents/fix-upstream-reviewer.md b/agents/fix-upstream-reviewer.md index 68d3aa87..89418e33 100644 --- a/agents/fix-upstream-reviewer.md +++ b/agents/fix-upstream-reviewer.md @@ -1,5 +1,6 @@ --- name: fix-upstream-reviewer +mcpServers: [codebase, context7, autonomous_bugs] model: opus description: Lead-engineer code reviewer that identifies fallbacks, workarounds, and patches that mask upstream problems. Use proactively when reviewing code that contains try/catch fallbacks, redundant API calls, defensive null-checks, or multi-path resolution logic. Answers two questions - can we simplify this code, and should we fix upstream (closer to the source of the problem) instead of patching downstream? Here "upstream" means the direction the data flows FROM, not any particular repo. --- diff --git a/agents/frontend-accessibility-reviewer.md b/agents/frontend-accessibility-reviewer.md index b95d10ea..f5d449aa 100644 --- a/agents/frontend-accessibility-reviewer.md +++ b/agents/frontend-accessibility-reviewer.md @@ -1,7 +1,8 @@ --- name: frontend-accessibility-reviewer +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent to review frontend code for accessibility (WCAG AA) issues. Checks semantic HTML, keyboard navigation, ARIA usage, color contrast, form labels, focus management, and motion safety. Advisory only.\\n\\n\\nContext: User has added a new interactive component.\\nuser: \"Added the new dropdown menu\"\\nassistant: \"Let me invoke the frontend-accessibility-reviewer agent to check keyboard navigation, focus management, and ARIA usage.\"\\n\\nNew interactive widgets should be reviewed for keyboard access and correct ARIA roles.\\n\\n\\n\\n\\nContext: User has changed colors or added text on a colored background.\\nuser: \"Updated the badge styles\"\\nassistant: \"I'll run the frontend-accessibility-reviewer agent to verify contrast ratios meet WCAG AA.\"\\n\\nColor changes need a contrast check (4.5:1 normal text, 3:1 large/non-text).\\n\\n" -tools: Read, Grep, Glob, Bash +tools: Read, Grep, Glob, Bash, mcp__codebase, mcp__context7, mcp__autonomous_bugs model: opus color: blue --- diff --git a/agents/frontend-performance-reviewer.md b/agents/frontend-performance-reviewer.md index b8aa1c6e..b841c3f3 100644 --- a/agents/frontend-performance-reviewer.md +++ b/agents/frontend-performance-reviewer.md @@ -1,7 +1,8 @@ --- name: frontend-performance-reviewer +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent to review frontend code for performance issues. Checks bundle size, image optimization, CSS efficiency, and React rendering patterns.\\n\\n\\nContext: User has added new React components or modified rendering logic.\\nuser: \"Added the new dashboard widgets\"\\nassistant: \"Let me invoke the frontend-performance-reviewer agent to check for unnecessary re-renders and bundle size impact.\"\\n\\nNew components should be reviewed for React.memo usage, proper hook dependencies, and lazy loading opportunities.\\n\\n\\n\\n\\nContext: User has added images or modified CSS.\\nuser: \"Added the product images to the catalog page\"\\nassistant: \"I'll run the frontend-performance-reviewer agent to verify image optimization and lazy loading.\"\\n\\nImages need review for proper formats, dimensions, and lazy loading implementation.\\n\\n" -tools: Read, Grep, Glob, Bash +tools: Read, Grep, Glob, Bash, mcp__codebase, mcp__context7, mcp__autonomous_bugs model: haiku color: yellow --- diff --git a/agents/frontend-security-reviewer.md b/agents/frontend-security-reviewer.md index 56f82ca7..c5964f0c 100644 --- a/agents/frontend-security-reviewer.md +++ b/agents/frontend-security-reviewer.md @@ -1,7 +1,8 @@ --- name: frontend-security-reviewer +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent to review frontend code for security vulnerabilities. Checks XSS prevention, CSRF protection, token storage, and input validation.\\n\\n\\nContext: User has added form handling or user input processing.\\nuser: \"Added the comment submission form\"\\nassistant: \"Let me invoke the frontend-security-reviewer agent to check for XSS vulnerabilities and input sanitization.\"\\n\\nUser input handling should be reviewed for proper sanitization and XSS prevention.\\n\\n\\n\\n\\nContext: User has modified authentication or token handling.\\nuser: \"Updated the login flow to store the session\"\\nassistant: \"I'll run the frontend-security-reviewer agent to verify tokens aren't stored in localStorage and are properly secured.\"\\n\\nAuthentication changes need review for secure token storage and CSRF protection.\\n\\n" -tools: Read, Grep, Glob, Bash +tools: Read, Grep, Glob, Bash, mcp__codebase, mcp__context7, mcp__autonomous_bugs model: haiku color: red --- diff --git a/agents/prior-art.md b/agents/prior-art.md index 73300d45..3f1fc69e 100644 --- a/agents/prior-art.md +++ b/agents/prior-art.md @@ -1,5 +1,6 @@ --- name: prior-art +mcpServers: [codebase, context7, autonomous_bugs] model: opus description: Step-0 problem validation BEFORE any plan exists. Given a problem statement, researches whether the problem is already solved (locally cloned reference checkouts, upstream fixes, other consumers of the same dependency, the web), whether it is a red herring, and whether the problem's frame itself should exist. Returns a cited SOLVED_ELSEWHERE / DISSOLVE_FRAME / GENUINE_NEW_WORK / INSUFFICIENT_EVIDENCE verdict with per-leg availability attestation. Invoke when a task is problem-shaped — a bug or pain attributed to a dependency, a missing capability, a limit to work around — before options or plans are drafted. --- diff --git a/cli/lib/install/agent-assets/agent-assets.mts b/cli/lib/install/agent-assets/agent-assets.mts index 7a987905..bb1c82cf 100644 --- a/cli/lib/install/agent-assets/agent-assets.mts +++ b/cli/lib/install/agent-assets/agent-assets.mts @@ -5,6 +5,7 @@ import { type AgentAssetKind, type AgentProvider, isAgentProvider } from './agen const FRONTMATTER_RE = /^(?:\uFEFF)?---[ \t]*\r?\n([\s\S]*?)^---[ \t]*(?:\r?\n|$)/m; const FRONTMATTER_KEY_RE = /^[A-Za-z_][A-Za-z0-9_-]*$/; const AGENT_NAME_RE = /^[A-Za-z0-9][A-Za-z0-9_-]*$/; +const MCP_SERVER_NAME_RE = /^[A-Za-z0-9][A-Za-z0-9_-]*$/; const NUMBER_RE = /^[+-]?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?$/; const PLAIN_COMMENT_RE = /\s+#/; const NULL_RE = /^(?:null)$/i; @@ -144,6 +145,20 @@ function parseFrontmatterScalar(raw: string, key: string): unknown { return parsePlainScalar(raw); } +function parseMcpServers(raw: string): string[] { + if (!raw.startsWith('[') || !raw.endsWith(']')) + throw new Error('Agent frontmatter field "mcpServers" must be an inline string list'); + const names = raw + .slice(1, -1) + .split(',') + .map((name) => name.trim()); + if (names.length === 0 || names.some((name) => !MCP_SERVER_NAME_RE.test(name))) + throw new Error('Agent frontmatter field "mcpServers" contains an invalid server name'); + if (new Set(names).size !== names.length) + throw new Error('Agent frontmatter field "mcpServers" contains a duplicate server name'); + return names; +} + function requiredString(fields: Map, key: string): string { const value = fields.get(key); if (typeof value !== 'string' || !value.trim()) @@ -171,7 +186,8 @@ function parseAgentFrontmatter(markdown: string): AgentFrontmatter { throw new Error(`Malformed agent frontmatter line ${index + 2}: ${line}`); } if (fields.has(key)) throw new Error(`Duplicate agent frontmatter field: ${key}`); - fields.set(key, parseFrontmatterScalar(line.slice(separator + 1).trim(), key)); + const raw = line.slice(separator + 1).trim(); + fields.set(key, key === 'mcpServers' ? parseMcpServers(raw) : parseFrontmatterScalar(raw, key)); } const body = markdown.slice(match[0].length); diff --git a/cli/lib/install/agent-assets/agent-assets.test.mts b/cli/lib/install/agent-assets/agent-assets.test.mts index 7024b25f..d865ee12 100644 --- a/cli/lib/install/agent-assets/agent-assets.test.mts +++ b/cli/lib/install/agent-assets/agent-assets.test.mts @@ -42,6 +42,7 @@ describe('agent asset projections', () => { 'name: quote-reviewer', String.raw`description: "Quotes: \"yes\"; path C:\\tmp; literal \\n marker" # metadata comment`, 'tools: Read, Grep, Bash', + 'mcpServers: [codebase, context7, autonomous_bugs]', 'model: opus', 'color: blue', '---', @@ -64,6 +65,7 @@ Keep C:\tmp and triple quotes """ intact. `, }); expect(toml).not.toContain('tools ='); + expect(toml).not.toContain('mcpServers ='); expect(toml).not.toContain('model ='); expect(toml).not.toContain('color ='); @@ -104,6 +106,14 @@ Keep C:\tmp and triple quotes """ intact. ['tagged indented block scalar', '---\nname: reviewer\ndescription: !!str |2\n---\nbody'], ['tagged anchor', '---\nname: reviewer\ndescription: !!str &label true\n---\nbody'], ['collection description', '---\nname: reviewer\ndescription: [not, a, string]\n---\nbody'], + [ + 'block MCP server list', + '---\nname: reviewer\ndescription: valid\nmcpServers:\n - codebase\n---\nbody', + ], + [ + 'invalid MCP server name', + '---\nname: reviewer\ndescription: valid\nmcpServers: [codebase, ../bad]\n---\nbody', + ], ['empty body', '---\nname: reviewer\ndescription: valid\n---\n'], ['malformed field', '---\nname reviewer\ndescription: valid\n---\nbody'], ['malformed quoted scalar', '---\nname: reviewer\ndescription: "bad\\q"\n---\nbody'], @@ -134,4 +144,21 @@ Keep C:\tmp and triple quotes """ intact. ]); } }); + + it('gives every bundled Claude agent only the shared MCP baseline', () => { + const agentsDir = join(packageDir(), 'agents'); + const files = readdirSync(agentsDir).filter((name) => name.endsWith('.md')); + + for (const file of files) { + const markdown = readFileSync(join(agentsDir, file), 'utf8'); + expect(markdown, file).toMatch(/^mcpServers: \[codebase, context7, autonomous_bugs\]$/m); + const tools = markdown.match(/^tools: (.+)$/m)?.[1]; + if (tools) { + const toolNames = tools.split(',').map((tool) => tool.trim()); + expect(toolNames, file).toEqual( + expect.arrayContaining(['mcp__codebase', 'mcp__context7', 'mcp__autonomous_bugs']), + ); + } + } + }); }); diff --git a/dist/agents/api-security-reviewer.md b/dist/agents/api-security-reviewer.md index 43d31194..c2e7dc83 100644 --- a/dist/agents/api-security-reviewer.md +++ b/dist/agents/api-security-reviewer.md @@ -1,7 +1,8 @@ --- name: api-security-reviewer +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent to review backend/API code for security vulnerabilities. Checks authentication, JWT handling, input validation, output security, and access control.\\n\\n\\nContext: User has added new API endpoints or authentication logic.\\nuser: \"I've added the new user registration endpoint\"\\nassistant: \"Let me invoke the api-security-reviewer agent to check for security issues in your new endpoint.\"\\n\\nNew API endpoints should be reviewed for authentication, input validation, and secure response handling.\\n\\n\\n\\n\\nContext: User has modified JWT or session handling.\\nuser: \"Updated the token refresh logic\"\\nassistant: \"I'll run the api-security-reviewer agent to verify the JWT implementation follows security best practices.\"\\n\\nJWT changes require verification of algorithm, expiry, and secret handling.\\n\\n" -tools: Read, Grep, Glob, Bash +tools: Read, Grep, Glob, Bash, mcp__codebase, mcp__context7, mcp__autonomous_bugs model: haiku color: red --- diff --git a/dist/agents/backend-performance-reviewer.md b/dist/agents/backend-performance-reviewer.md index 23f47188..60dde66b 100644 --- a/dist/agents/backend-performance-reviewer.md +++ b/dist/agents/backend-performance-reviewer.md @@ -1,7 +1,8 @@ --- name: backend-performance-reviewer +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent to review backend code for performance issues. Checks database queries, caching strategies, async patterns, and API response optimization.\\n\\n\\nContext: User has added database queries or data fetching logic.\\nuser: \"Added the query to fetch all user tasks\"\\nassistant: \"Let me invoke the backend-performance-reviewer agent to check for N+1 queries and pagination issues.\"\\n\\nDatabase queries should be reviewed for efficiency, proper indexing, and avoiding N+1 patterns.\\n\\n\\n\\n\\nContext: User has implemented caching or heavy data processing.\\nuser: \"Implemented caching for the dashboard data\"\\nassistant: \"I'll run the backend-performance-reviewer agent to verify cache invalidation and TTL strategies.\"\\n\\nCaching implementations need review for proper invalidation and memory considerations.\\n\\n" -tools: Read, Grep, Glob, Bash +tools: Read, Grep, Glob, Bash, mcp__codebase, mcp__context7, mcp__autonomous_bugs model: haiku color: orange --- diff --git a/dist/agents/commit-guard.md b/dist/agents/commit-guard.md index 073cb4cd..4fd3bc0b 100644 --- a/dist/agents/commit-guard.md +++ b/dist/agents/commit-guard.md @@ -1,7 +1,8 @@ --- name: commit-guard +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent before committing code to guard against unintentional duplication using semantic search.\\n\\n\\nContext: User is about to commit staged changes.\\nuser: \"I'm ready to commit these changes\"\\nassistant: \"Let me invoke the commit-guard agent to check for duplicates before you commit.\"\\n\\ncommit-guard runs semantic duplicate detection against the search index and checks DRY rules per file.\\n\\n\\n\\n\\nContext: User has added new utility functions or components.\\nuser: \"Added a new helper function for date formatting\"\\nassistant: \"I'll run the commit-guard agent to verify this doesn't duplicate an existing utility.\"\\n\\nNew utilities should be checked against the indexed codebase to prevent duplication.\\n\\n" -tools: Read, Grep, Glob, Bash, mcp__codebase__searchCode +tools: Read, Grep, Glob, Bash, mcp__codebase, mcp__context7, mcp__autonomous_bugs, mcp__codebase__searchCode model: haiku color: blue --- diff --git a/dist/agents/conventions-reviewer.md b/dist/agents/conventions-reviewer.md index aa781378..86b77f18 100644 --- a/dist/agents/conventions-reviewer.md +++ b/dist/agents/conventions-reviewer.md @@ -1,7 +1,8 @@ --- name: conventions-reviewer +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent to check a diff against the governing CLAUDE.md files of the repo it's installed in. Flags a violation only when it can quote both the exact rule and the exact offending line; otherwise stays silent. No style opinions.\\n\\n\\nContext: A CLAUDE.md rule says never hand-edit generated files.\\nuser: \"Updated the generated icon exports directly\"\\nassistant: \"I'll run the conventions-reviewer agent to check whether that edit violates the repo's own generated-file rule.\"\\n\\nA written CLAUDE.md rule with an unhedged directive and a concrete offending line is exactly what this reviewer exists to catch.\\n\\n\\n\\n\\nContext: A nested package has its own CLAUDE.md scoping a rule to that package only.\\nuser: \"Added a new file under packages/api\"\\nassistant: \"Let me invoke the conventions-reviewer agent — packages/api's own CLAUDE.md may govern this file, on top of the repo root's.\"\\n\\nScoping matters: a rule in one package's CLAUDE.md never governs a sibling package's files.\\n\\n" -tools: Read, Grep, Glob +tools: Read, Grep, Glob, mcp__codebase, mcp__context7, mcp__autonomous_bugs model: haiku color: cyan --- diff --git a/dist/agents/correctness-reviewer.md b/dist/agents/correctness-reviewer.md index bbc10888..070d4447 100644 --- a/dist/agents/correctness-reviewer.md +++ b/dist/agents/correctness-reviewer.md @@ -1,7 +1,8 @@ --- name: correctness-reviewer +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent to review a finished diff for correctness bugs: concurrency/race conditions, state-machine dead states, writer/reader contract mismatches across modules, broadcast/dedup errors, discarded return values, and classifier/parsing edge cases.\\n\\n\\nContext: User has implemented retry/recovery logic that writes task statuses.\\nuser: \"The task retry flow is done\"\\nassistant: \"Let me invoke the correctness-reviewer agent to trace every status write to its readers and walk the concurrent interleavings.\"\\n\\nStatus writes need CAS guards and every consumer (pollers, filters, queries) must still select the written state.\\n\\n\\n\\n\\nContext: User added an event that is broadcast to multiple listeners.\\nuser: \"chat-ready now fires on resume\"\\nassistant: \"I'll run the correctness-reviewer agent to enumerate the listeners and check for duplicate handling.\"\\n\\nA broadcast consumed by N listeners without targeting/dedup executes its effect N times.\\n\\n" -tools: Read, Grep, Glob, Bash +tools: Read, Grep, Glob, Bash, mcp__codebase, mcp__context7, mcp__autonomous_bugs model: sonnet color: orange --- diff --git a/dist/agents/feature-completeness-reviewer.md b/dist/agents/feature-completeness-reviewer.md index 2b2b33f7..07426602 100644 --- a/dist/agents/feature-completeness-reviewer.md +++ b/dist/agents/feature-completeness-reviewer.md @@ -1,5 +1,6 @@ --- name: feature-completeness-reviewer +mcpServers: [codebase, context7, autonomous_bugs] model: opus description: Use this agent proactively after any significant code or feature implementation to identify missing components, overlooked dependencies, and gaps in the solution. Specifically invoke this agent when:\\n\\n\\nContext: The user has just implemented a new feature for real-time chat functionality.\\nuser: \"I've just finished implementing the core chat messaging system\"\\nassistant: \"Great work on the messaging system! Let me use the Task tool to launch the feature-completeness-reviewer agent to analyze what supporting components might be needed.\"\\n\\nSince a significant feature was implemented, proactively use the feature-completeness-reviewer agent to identify missing pieces like debugging tools, monitoring capabilities, or infrastructure requirements.\\n\\n\\n\\n\\nContext: The user has completed a major refactoring of the authentication system.\\nuser: \"The authentication refactor is done, all tests passing\"\\nassistant: \"Excellent! Now let me invoke the feature-completeness-reviewer agent to examine if we've covered all the necessary touchpoints.\"\\n\\nAfter completing authentication work, use the feature-completeness-reviewer agent to check for missing elements like session management, token refresh mechanisms, or user experience flows.\\n\\n\\n\\n\\nContext: A new API endpoint has been added to the system.\\nuser: \"I've added the new /api/status endpoint\"\\nassistant: \"Perfect! Let me use the feature-completeness-reviewer agent to assess the broader implications and any missing pieces.\"\\n\\nWhen new APIs are created, proactively launch the feature-completeness-reviewer agent to identify missing documentation, error handling, rate limiting, or integration points.\\n\\n --- diff --git a/dist/agents/feature-critique.md b/dist/agents/feature-critique.md index ef8be3ec..479f85de 100644 --- a/dist/agents/feature-critique.md +++ b/dist/agents/feature-critique.md @@ -1,5 +1,6 @@ --- name: feature-critique +mcpServers: [codebase, context7, autonomous_bugs] model: opus description: Pre-implementation critique specialist. Analyzes feature proposals, architectural decisions, and technical approaches BEFORE work begins. Evaluates feasibility, UX implications, security concerns, codebase conflicts, data flow correctness, runtime behavior across user configurations, and missing considerations using evidence-based research. Invoke when a feature proposal, technical approach, or architectural decision needs critical evaluation before implementation starts. --- diff --git a/dist/agents/fix-upstream-reviewer.md b/dist/agents/fix-upstream-reviewer.md index 68d3aa87..89418e33 100644 --- a/dist/agents/fix-upstream-reviewer.md +++ b/dist/agents/fix-upstream-reviewer.md @@ -1,5 +1,6 @@ --- name: fix-upstream-reviewer +mcpServers: [codebase, context7, autonomous_bugs] model: opus description: Lead-engineer code reviewer that identifies fallbacks, workarounds, and patches that mask upstream problems. Use proactively when reviewing code that contains try/catch fallbacks, redundant API calls, defensive null-checks, or multi-path resolution logic. Answers two questions - can we simplify this code, and should we fix upstream (closer to the source of the problem) instead of patching downstream? Here "upstream" means the direction the data flows FROM, not any particular repo. --- diff --git a/dist/agents/frontend-accessibility-reviewer.md b/dist/agents/frontend-accessibility-reviewer.md index b95d10ea..f5d449aa 100644 --- a/dist/agents/frontend-accessibility-reviewer.md +++ b/dist/agents/frontend-accessibility-reviewer.md @@ -1,7 +1,8 @@ --- name: frontend-accessibility-reviewer +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent to review frontend code for accessibility (WCAG AA) issues. Checks semantic HTML, keyboard navigation, ARIA usage, color contrast, form labels, focus management, and motion safety. Advisory only.\\n\\n\\nContext: User has added a new interactive component.\\nuser: \"Added the new dropdown menu\"\\nassistant: \"Let me invoke the frontend-accessibility-reviewer agent to check keyboard navigation, focus management, and ARIA usage.\"\\n\\nNew interactive widgets should be reviewed for keyboard access and correct ARIA roles.\\n\\n\\n\\n\\nContext: User has changed colors or added text on a colored background.\\nuser: \"Updated the badge styles\"\\nassistant: \"I'll run the frontend-accessibility-reviewer agent to verify contrast ratios meet WCAG AA.\"\\n\\nColor changes need a contrast check (4.5:1 normal text, 3:1 large/non-text).\\n\\n" -tools: Read, Grep, Glob, Bash +tools: Read, Grep, Glob, Bash, mcp__codebase, mcp__context7, mcp__autonomous_bugs model: opus color: blue --- diff --git a/dist/agents/frontend-performance-reviewer.md b/dist/agents/frontend-performance-reviewer.md index b8aa1c6e..b841c3f3 100644 --- a/dist/agents/frontend-performance-reviewer.md +++ b/dist/agents/frontend-performance-reviewer.md @@ -1,7 +1,8 @@ --- name: frontend-performance-reviewer +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent to review frontend code for performance issues. Checks bundle size, image optimization, CSS efficiency, and React rendering patterns.\\n\\n\\nContext: User has added new React components or modified rendering logic.\\nuser: \"Added the new dashboard widgets\"\\nassistant: \"Let me invoke the frontend-performance-reviewer agent to check for unnecessary re-renders and bundle size impact.\"\\n\\nNew components should be reviewed for React.memo usage, proper hook dependencies, and lazy loading opportunities.\\n\\n\\n\\n\\nContext: User has added images or modified CSS.\\nuser: \"Added the product images to the catalog page\"\\nassistant: \"I'll run the frontend-performance-reviewer agent to verify image optimization and lazy loading.\"\\n\\nImages need review for proper formats, dimensions, and lazy loading implementation.\\n\\n" -tools: Read, Grep, Glob, Bash +tools: Read, Grep, Glob, Bash, mcp__codebase, mcp__context7, mcp__autonomous_bugs model: haiku color: yellow --- diff --git a/dist/agents/frontend-security-reviewer.md b/dist/agents/frontend-security-reviewer.md index 56f82ca7..c5964f0c 100644 --- a/dist/agents/frontend-security-reviewer.md +++ b/dist/agents/frontend-security-reviewer.md @@ -1,7 +1,8 @@ --- name: frontend-security-reviewer +mcpServers: [codebase, context7, autonomous_bugs] description: "Use this agent to review frontend code for security vulnerabilities. Checks XSS prevention, CSRF protection, token storage, and input validation.\\n\\n\\nContext: User has added form handling or user input processing.\\nuser: \"Added the comment submission form\"\\nassistant: \"Let me invoke the frontend-security-reviewer agent to check for XSS vulnerabilities and input sanitization.\"\\n\\nUser input handling should be reviewed for proper sanitization and XSS prevention.\\n\\n\\n\\n\\nContext: User has modified authentication or token handling.\\nuser: \"Updated the login flow to store the session\"\\nassistant: \"I'll run the frontend-security-reviewer agent to verify tokens aren't stored in localStorage and are properly secured.\"\\n\\nAuthentication changes need review for secure token storage and CSRF protection.\\n\\n" -tools: Read, Grep, Glob, Bash +tools: Read, Grep, Glob, Bash, mcp__codebase, mcp__context7, mcp__autonomous_bugs model: haiku color: red --- diff --git a/dist/agents/prior-art.md b/dist/agents/prior-art.md index 73300d45..3f1fc69e 100644 --- a/dist/agents/prior-art.md +++ b/dist/agents/prior-art.md @@ -1,5 +1,6 @@ --- name: prior-art +mcpServers: [codebase, context7, autonomous_bugs] model: opus description: Step-0 problem validation BEFORE any plan exists. Given a problem statement, researches whether the problem is already solved (locally cloned reference checkouts, upstream fixes, other consumers of the same dependency, the web), whether it is a red herring, and whether the problem's frame itself should exist. Returns a cited SOLVED_ELSEWHERE / DISSOLVE_FRAME / GENUINE_NEW_WORK / INSUFFICIENT_EVIDENCE verdict with per-leg availability attestation. Invoke when a task is problem-shaped — a bug or pain attributed to a dependency, a missing capability, a limit to work around — before options or plans are drafted. --- diff --git a/dist/cli/lib/install/agent-assets/agent-assets.mjs b/dist/cli/lib/install/agent-assets/agent-assets.mjs index 09c928a7..3ab1bebb 100644 --- a/dist/cli/lib/install/agent-assets/agent-assets.mjs +++ b/dist/cli/lib/install/agent-assets/agent-assets.mjs @@ -4,6 +4,7 @@ import { isAgentProvider } from "./agent-providers.mjs"; const FRONTMATTER_RE = /^(?:\uFEFF)?---[ \t]*\r?\n([\s\S]*?)^---[ \t]*(?:\r?\n|$)/m; const FRONTMATTER_KEY_RE = /^[A-Za-z_][A-Za-z0-9_-]*$/; const AGENT_NAME_RE = /^[A-Za-z0-9][A-Za-z0-9_-]*$/; +const MCP_SERVER_NAME_RE = /^[A-Za-z0-9][A-Za-z0-9_-]*$/; const NUMBER_RE = /^[+-]?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?$/; const PLAIN_COMMENT_RE = /\s+#/; const NULL_RE = /^(?:null)$/i; @@ -137,6 +138,19 @@ function parseFrontmatterScalar(raw, key) { return quotedScalar(raw, key, "'"); return parsePlainScalar(raw); } +function parseMcpServers(raw) { + if (!raw.startsWith('[') || !raw.endsWith(']')) + throw new Error('Agent frontmatter field "mcpServers" must be an inline string list'); + const names = raw + .slice(1, -1) + .split(',') + .map((name) => name.trim()); + if (names.length === 0 || names.some((name) => !MCP_SERVER_NAME_RE.test(name))) + throw new Error('Agent frontmatter field "mcpServers" contains an invalid server name'); + if (new Set(names).size !== names.length) + throw new Error('Agent frontmatter field "mcpServers" contains a duplicate server name'); + return names; +} function requiredString(fields, key) { const value = fields.get(key); if (typeof value !== 'string' || !value.trim()) @@ -162,7 +176,8 @@ function parseAgentFrontmatter(markdown) { } if (fields.has(key)) throw new Error(`Duplicate agent frontmatter field: ${key}`); - fields.set(key, parseFrontmatterScalar(line.slice(separator + 1).trim(), key)); + const raw = line.slice(separator + 1).trim(); + fields.set(key, key === 'mcpServers' ? parseMcpServers(raw) : parseFrontmatterScalar(raw, key)); } const body = markdown.slice(match[0].length); if (!body.trim()) diff --git a/dist/gate-engine/judge/mcp/profile.mjs b/dist/gate-engine/judge/mcp/profile.mjs new file mode 100644 index 00000000..d1bab7a3 --- /dev/null +++ b/dist/gate-engine/judge/mcp/profile.mjs @@ -0,0 +1,219 @@ +import { execFileSync } from 'node:child_process'; +import { createHash } from 'node:crypto'; +import { chmodSync, lstatSync, mkdtempSync, readFileSync, realpathSync, rmSync, statSync, writeFileSync, } from 'node:fs'; +import { homedir, tmpdir } from 'node:os'; +import path from 'node:path'; +import { withoutGitEnv } from "../judge-isolation.mjs"; +const BASELINE_SERVER_NAMES = ['codebase', 'context7', 'autonomous_bugs']; +const BASELINE_TOOL_PREFIXES = BASELINE_SERVER_NAMES.map((name) => `mcp__${name}`); +const EMPTY_MCP_CONFIG = '{"mcpServers":{}}'; +const REGISTRY_ENV = 'DEVKIT_JUDGE_MCP_CONFIG'; +const registryCache = new Map(); +const warned = new Set(); +const isRecord = (value) => typeof value === 'object' && value !== null && !Array.isArray(value); +function warnOnce(key, message) { + if (warned.has(key)) + return; + warned.add(key); + console.error(message); +} +function isInside(root, candidate) { + const rel = path.relative(root, candidate); + return rel === '' || (!rel.startsWith('..') && !path.isAbsolute(rel)); +} +function trustedRegistryPath(requested, cwd, explicit) { + try { + if (!path.isAbsolute(requested)) + return null; + const entry = lstatSync(requested); + if (!entry.isFile() || entry.isSymbolicLink()) + return null; + if (typeof process.getuid === 'function' && entry.uid !== process.getuid()) + return null; + if ((entry.mode & 0o022) !== 0) + return null; + const canonical = realpathSync(requested); + const canonicalCwd = realpathSync(cwd); + if (explicit && isInside(canonicalCwd, canonical)) + return null; + return canonical; + } + catch { + return null; + } +} +function readRegistry(file) { + try { + const stat = statSync(file); + const stamp = `${stat.mtimeMs}:${stat.size}`; + const cached = registryCache.get(file); + if (cached?.stamp === stamp) + return cached.value; + const parsed = JSON.parse(readFileSync(file, 'utf8')); + const value = isRecord(parsed) ? parsed : null; + registryCache.set(file, { stamp, value }); + return value; + } + catch { + return null; + } +} +function validServer(value) { + if (!isRecord(value) || value.disabled === true) + return null; + const command = value.command; + const url = value.url; + if (typeof command !== 'string' && typeof url !== 'string') + return null; + const { disabled: _disabled, ...server } = value; + return server; +} +function serverTable(value) { + if (!isRecord(value)) + return {}; + const result = {}; + for (const [name, server] of Object.entries(value)) { + const valid = validServer(server); + if (valid) + result[name] = valid; + } + return result; +} +function primaryCheckoutRoot(cwd, env) { + try { + const common = execFileSync('git', ['rev-parse', '--path-format=absolute', '--git-common-dir'], { cwd, env: withoutGitEnv(env), encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }).trim(); + if (!common || path.basename(common) !== '.git') + return null; + return realpathSync(path.dirname(common)); + } + catch { + return null; + } +} +function projectCandidates(cwd, env, supplied) { + const candidates = supplied ?? [cwd, primaryCheckoutRoot(cwd, env)].filter(Boolean); + const result = []; + for (const candidate of candidates) { + if (!candidate) + continue; + try { + const canonical = realpathSync(candidate); + if (!result.includes(canonical)) + result.push(canonical); + } + catch { + const resolved = path.resolve(candidate); + if (!result.includes(resolved)) + result.push(resolved); + } + } + return result; +} +function selectedServers(registry, serverNames, roots) { + const selected = {}; + const rootServers = serverTable(registry.mcpServers); + const projects = isRecord(registry.projects) ? registry.projects : {}; + const projectRows = roots.map((root) => projects[root]).filter(isRecord); + for (const name of serverNames) { + let server = rootServers[name] ?? null; + for (const project of projectRows) { + const disabled = Array.isArray(project.disabledMcpServers) ? project.disabledMcpServers : []; + if (disabled.includes(name)) { + server = null; + continue; + } + const projectServer = serverTable(project.mcpServers)[name]; + if (projectServer) + server = projectServer; + } + if (server) + selected[name] = server; + } + return selected; +} +export function namedAgentMcpProfile() { + return { + kind: 'named-agent', + serverNames: BASELINE_SERVER_NAMES, + }; +} +export function withNamedAgentMcpTools(tools, ...extraTools) { + const values = [tools, ...BASELINE_TOOL_PREFIXES, ...extraTools] + .flatMap((value) => value.split(',')) + .map((value) => value.trim()) + .filter(Boolean); + return [...new Set(values)].join(','); +} +/** + * Stable, secret-safe cache partition for the capabilities a named judge can actually receive. + * The digest includes the declared tool set, trusted registry location, and selected definitions; + * cache entries therefore never survive a capability change, while secret-bearing config is never + * written to the cache itself. + */ +export function judgeMcpCapabilityFingerprint(profile, allowedTools, options) { + const env = options.env ?? process.env; + const explicit = options.registryPath !== undefined || env[REGISTRY_ENV] !== undefined; + const requested = options.registryPath ?? env[REGISTRY_ENV] ?? path.join(homedir(), '.claude.json'); + const registryPath = trustedRegistryPath(requested, options.cwd, explicit); + const registry = registryPath ? readRegistry(registryPath) : null; + const servers = profile.kind === 'named-agent' && registry + ? selectedServers(registry, profile.serverNames, projectCandidates(options.cwd, env, options.projectRoots)) + : {}; + return createHash('sha256') + .update(JSON.stringify({ allowedTools, profile, registryPath: registryPath ?? requested, servers })) + .digest('hex'); +} +function emptyProfile() { + return { + args: ['--mcp-config', EMPTY_MCP_CONFIG, '--strict-mcp-config'], + serverNames: [], + cleanup: () => { }, + }; +} +export function prepareJudgeMcpProfile(profile, options) { + if (profile.kind === 'none') + return emptyProfile(); + const env = options.env ?? process.env; + const explicit = options.registryPath !== undefined || env[REGISTRY_ENV] !== undefined; + const requested = options.registryPath ?? env[REGISTRY_ENV] ?? path.join(homedir(), '.claude.json'); + const registryPath = trustedRegistryPath(requested, options.cwd, explicit); + if (!registryPath) { + warnOnce(`registry:${requested}`, `guard-review: trusted MCP registry unavailable at ${requested} — named agents continue with strict-empty MCP isolation`); + return emptyProfile(); + } + const registry = readRegistry(registryPath); + if (!registry) { + warnOnce(`registry-json:${registryPath}`, 'guard-review: trusted MCP registry is unreadable — named agents continue with strict-empty MCP isolation'); + return emptyProfile(); + } + const roots = projectCandidates(options.cwd, env, options.projectRoots); + const servers = selectedServers(registry, profile.serverNames, roots); + const present = Object.keys(servers); + const missing = profile.serverNames.filter((name) => !present.includes(name)); + if (missing.length > 0) + warnOnce(`missing:${registryPath}:${missing.join(',')}`, `guard-review: named-agent MCP profile missing ${missing.join(', ')} — continuing with the configured subset under strict isolation`); + if (present.length === 0) + return emptyProfile(); + let directory = null; + try { + directory = mkdtempSync(path.join(options.temporaryRoot ?? tmpdir(), 'devkit-judge-mcp-')); + chmodSync(directory, 0o700); + const file = path.join(directory, 'mcp.json'); + writeFileSync(file, `${JSON.stringify({ mcpServers: servers })}\n`, { + encoding: 'utf8', + mode: 0o600, + flag: 'wx', + }); + return { + args: ['--mcp-config', file, '--strict-mcp-config'], + serverNames: present, + cleanup: () => rmSync(directory, { recursive: true, force: true }), + }; + } + catch { + if (directory) + rmSync(directory, { recursive: true, force: true }); + warnOnce('temporary-config', 'guard-review: private MCP profile file could not be created — named agents continue with strict-empty MCP isolation'); + return emptyProfile(); + } +} diff --git a/dist/gate-engine/judge/run-judge.mjs b/dist/gate-engine/judge/run-judge.mjs index ea02ff19..594f42d9 100644 --- a/dist/gate-engine/judge/run-judge.mjs +++ b/dist/gate-engine/judge/run-judge.mjs @@ -19,6 +19,7 @@ import { execFile, execFileSync } from 'node:child_process'; import { parseJudgeUsage, unwrapClaudeResult, withResultArgs, } from "./claude-result.mjs"; import { emitGateEvent } from "./gate-events.mjs"; import { withoutGitEnv } from "./judge-isolation.mjs"; +import { prepareJudgeMcpProfile } from "./mcp/profile.mjs"; import { composeTranscript, saveTranscriptUnique } from "./transcript-store.mjs"; // Narrow an unknown thrown value to the JudgeError shape; a non-object (or null) reads as {} so every // field access is undefined — matching the original `e?.field` optional-chaining behaviour exactly. @@ -184,8 +185,12 @@ function readJudgeOutput(stdout) { export function execJudge(opts) { const { label, args, input, timeout, cwd, env, onOutage } = opts; const startedAt = Date.now(); + const mcp = prepareJudgeMcpProfile(opts.mcpProfile ?? { kind: 'none' }, { + cwd: cwd ?? process.cwd(), + env, + }); try { - const out = execFileSync('claude', withResultArgs(args), { + const out = execFileSync('claude', withResultArgs([...mcp.args, ...args]), { cwd, // Never the caller's env verbatim: git leaks an ABSOLUTE GIT_INDEX_FILE/GIT_DIR into every // hook run in a linked worktree (how ship commits), and a tool-using judge that touches @@ -216,6 +221,9 @@ export function execJudge(opts) { onOutage?.(kind); return null; } + finally { + mcp.cleanup(); + } } /** * Async twin of execJudge — same contract (raw stdout, or `null` after ONE stderr warning), but @@ -231,6 +239,10 @@ export function execJudge(opts) { export function execJudgeAsync(opts) { const { label, args, input, timeout, cwd, env, onOutage } = opts; const startedAt = Date.now(); + const mcp = prepareJudgeMcpProfile(opts.mcpProfile ?? { kind: 'none' }, { + cwd: cwd ?? process.cwd(), + env, + }); return new Promise((resolve) => { // Shared outage path — a callback error AND a synchronous throw from execFile() itself (e.g. an // out-of-range `timeout` validates and throws before spawn even starts, sc-1317) both resolve @@ -239,6 +251,7 @@ export function execJudgeAsync(opts) { // resolves) for any caller awaiting it outside its own try/catch — the sync execJudge twin // already had this same guard via its enclosing try/catch. const fail = (err) => { + mcp.cleanup(); warnUnavailable(label, err, timeout); const kind = isJudgeTimeout(err) ? 'timeout' : 'transient'; emitJudgeExec(opts, kind, startedAt); @@ -246,7 +259,7 @@ export function execJudgeAsync(opts) { resolve(null); }; try { - const child = execFile('claude', withResultArgs(args), { + const child = execFile('claude', withResultArgs([...mcp.args, ...args]), { cwd, // env: see the execJudge twin — the git-env scrub applies to every judge spawn. env: withoutGitEnv(env), @@ -260,6 +273,7 @@ export function execJudgeAsync(opts) { fail(err); return; } + mcp.cleanup(); if (!stdout || !String(stdout).trim()) { warnNoOutput(label); emitJudgeExec(opts, 'empty', startedAt); diff --git a/dist/gate-engine/review/cascade/reviewer.mjs b/dist/gate-engine/review/cascade/reviewer.mjs index 64ecded3..75e3020b 100644 --- a/dist/gate-engine/review/cascade/reviewer.mjs +++ b/dist/gate-engine/review/cascade/reviewer.mjs @@ -1,4 +1,5 @@ import { JUDGE_ISOLATION } from "../../judge/judge-isolation.mjs"; +import { namedAgentMcpProfile } from "../../judge/mcp/profile.mjs"; import { DEEP_JUDGE_TIMEOUT_MS, execJudgeAsync } from "../../judge/run-judge.mjs"; import { renderGoverningClaudeMd } from "../claude-md.mjs"; import { buildCappedDiffEvidence } from "../diff-evidence.mjs"; @@ -53,6 +54,8 @@ async function cascadeVerdict({ reviewer, files }, { cwd, cfg, exec = execJudgeA ? wrapPrompt(body, reviewer, files, assetRoot, checklistRecoveryReason, promptExtras, checklistRoot) : wrapConventionsPrompt(body, files, renderGoverningClaudeMd(cwd, files), promptExtras); const input = buildCappedDiffEvidence(gitCached(cwd, [], files), stat); + const allowedTools = allowedToolsFor(reviewer, cfg, checklistRoot); + const mcpProfile = namedAgentMcpProfile(); const args = (promptBody, model) => [ '-p', promptBody, @@ -60,7 +63,7 @@ async function cascadeVerdict({ reviewer, files }, { cwd, cfg, exec = execJudgeA model, ...JUDGE_ISOLATION, '--allowedTools', - allowedToolsFor(reviewer, cfg, checklistRoot), + allowedTools, ]; const passModel = reviewer.model ?? firstModel; let firstOutage; @@ -71,6 +74,7 @@ async function cascadeVerdict({ reviewer, files }, { cwd, cfg, exec = execJudgeA timeout: DEEP_JUDGE_TIMEOUT_MS, cwd, transcript: false, + mcpProfile, env, onOutage: (kind) => { firstOutage = kind; @@ -127,6 +131,7 @@ async function cascadeVerdict({ reviewer, files }, { cwd, cfg, exec = execJudgeA timeout: DEEP_JUDGE_TIMEOUT_MS, cwd, transcript: false, + mcpProfile, env, onOutage: (kind) => { secondOutage = kind; diff --git a/dist/gate-engine/review/completeness.mjs b/dist/gate-engine/review/completeness.mjs index 627e817e..f0f1a360 100644 --- a/dist/gate-engine/review/completeness.mjs +++ b/dist/gate-engine/review/completeness.mjs @@ -38,6 +38,7 @@ import { renderTargets } from "./evidence/targets-block.mjs"; export { renderTargets } from "./evidence/targets-block.mjs"; import { emitCacheHit, finishGateTiming } from "../judge/gate-events.mjs"; import { JUDGE_ISOLATION } from "../judge/judge-isolation.mjs"; +import { judgeMcpCapabilityFingerprint, namedAgentMcpProfile, withNamedAgentMcpTools, } from "../judge/mcp/profile.mjs"; import { reportGateInfraFailure } from "../judge/odb-probe.mjs"; import { DEEP_JUDGE_TIMEOUT_MS, execJudgeAsync, strictRemedy } from "../judge/run-judge.mjs"; import { loadCache, savePasses } from "./cache.mjs"; @@ -105,11 +106,16 @@ export async function runCompleteness(msgFile, cwd = process.cwd(), { exec = exe return finish(0); let prompt; let diff; + let allowedTools = withNamedAgentMcpTools(TOOLS); + const mcpProfile = namedAgentMcpProfile(); + let capabilityFingerprint = ''; let stickyKey = ''; try { const cfg = resolveGuardConfig(cwd); if (cfg.noLlm) return finish(0); + allowedTools = withNamedAgentMcpTools(TOOLS, cfg.indexPath ? cfg.searchTool : ''); + capabilityFingerprint = judgeMcpCapabilityFingerprint(mcpProfile, allowedTools, { cwd }); const message = normalizeCommitMessage(readFileSync(path.isAbsolute(msgFile) ? msgFile : path.resolve(cwd, msgFile), 'utf8')); const files = execSync('git diff --cached --name-only', { cwd, encoding: 'utf8' }) .split('\n') @@ -134,7 +140,7 @@ export async function runCompleteness(msgFile, cwd = process.cwd(), { exec = exe // A FAIL is never sticky (only the confident-PASS save below writes this key), so a found gap // must genuinely be re-judged closed. Checked before scopedTargets/diff assembly — a sticky // hit skips the retrieval work too, not just the judge. - stickyKey = cacheKey('completeness-intent', `${verdictBranch(cwd)}\u0000${message}`, body); + stickyKey = cacheKey('completeness-intent', `${verdictBranch(cwd)}\u0000${message}`, `${body}\u0000${capabilityFingerprint}`); const sticky = loadCache(cwd)[stickyKey]; if (sticky) { console.error('guard-review: completeness — cached PASS (same branch + message; a retry-reshaped diff is not re-judged)'); @@ -174,7 +180,7 @@ export async function runCompleteness(msgFile, cwd = process.cwd(), { exec = exe // (docs/decisions/ship-gates-converge-not-restart.md), and completeness was outside it. // Key = every byte the judge reads: the prompt (message, governing Targets, brief) plus the // capped stdin evidence. An amended message or a re-staged hunk therefore MISSES and re-judges. - const key = cacheKey('completeness', diff, prompt); + const key = cacheKey('completeness', diff, `${prompt}\u0000${capabilityFingerprint}`); const hit = loadCache(cwd)[key]; if (hit) { console.error('guard-review: completeness — cached PASS (identical judgement)'); @@ -186,10 +192,11 @@ export async function runCompleteness(msgFile, cwd = process.cwd(), { exec = exe let outage; const raw = await exec({ label: 'review:completeness', - args: ['-p', prompt, '--model', 'opus', ...JUDGE_ISOLATION, '--allowedTools', TOOLS], + args: ['-p', prompt, '--model', 'opus', ...JUDGE_ISOLATION, '--allowedTools', allowedTools], input: diff, timeout: DEEP_JUDGE_TIMEOUT_MS, cwd, + mcpProfile, onOutage: (kind) => { outage = kind; }, diff --git a/dist/gate-engine/review/reviewers.mjs b/dist/gate-engine/review/reviewers.mjs index de711611..5eca357b 100644 --- a/dist/gate-engine/review/reviewers.mjs +++ b/dist/gate-engine/review/reviewers.mjs @@ -12,6 +12,7 @@ import { createHash } from 'node:crypto'; import { normalizeReviewRoots } from '../../skills/_devkit/review-roots.mjs'; import { sourceMatchers } from "../config.mjs"; import { devkitVersion } from "../devkit-version.mjs"; +import { withNamedAgentMcpTools } from "../judge/mcp/profile.mjs"; import { checklistContractFor } from "./lens/split.mjs"; /** Type guard: does this REVIEWERS entry use the checklist workflow? Skill-less reviewers (e.g. * conventions-reviewer) don't — see Reviewer.skill docstring. */ @@ -186,23 +187,21 @@ export function selectReviewers(stagedFiles, cfg) { * Comma-joined --allowedTools value for one reviewer: the read-only base, PLUS its own checklist * script (the one non-git Bash prefix a judge gets — scoped to that exact script path, so the * judge can drive its checklist but still cannot write files, stage, or commit), PLUS the - * consumer's semantic search tool for commit-guard. + * consumer's semantic search tool for commit-guard, plus every named agent's strict MCP baseline. */ export function allowedToolsFor(reviewer, cfg, assetRoot = '.claude') { - // A skill-less reviewer (e.g. conventions-reviewer) has no checklist script to grant Bash for, - // and its AC forbids Bash entirely — Read/Grep/Glob only, full stop, no BASE_TOOLS git-diff Bash - // either (its evidence is pre-rendered onto stdin/prompt instead — see wrapConventionsPrompt). + // A skill-less reviewer has no checklist script; its evidence is pre-rendered onto stdin/prompt. if (!hasChecklist(reviewer)) - return 'Read,Grep,Glob'; + return withNamedAgentMcpTools('Read,Grep,Glob'); const tools = `${BASE_TOOLS},Bash(node ${checklistScriptAt(reviewer, assetRoot)}:*)`; if (reviewer.domain === 'code') - return `${tools},${cfg.searchTool}`; + return withNamedAgentMcpTools(tools, cfg.searchTool); // The correctness reviewer's writer/reader-contract lens benefits from semantic search, but // only when the consumer actually wired an index (indexPath set) — otherwise cfg.searchTool is // a generic default naming an MCP tool the judge doesn't have, and Grep is the core mechanism. if (reviewer.domain === 'all' && cfg.indexPath) - return `${tools},${cfg.searchTool}`; - return tools; + return withNamedAgentMcpTools(tools, cfg.searchTool); + return withNamedAgentMcpTools(tools); } /** Strip a leading YAML frontmatter block from an agent .md. */ export function stripFrontmatter(md) { diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index 4e60df06..1db65e77 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -37,6 +37,7 @@ timeline. New rationale lives in the per-axis file. | [gate-opt-out-is-visible-and-detectable](gate-opt-out-is-visible-and-detectable.md) | Fail-open stays the default; the SILENCE around it does not. Three layers. (1) DETECT: a doctor check (cli/lib/doctor/guard-config-checks.mts) reports DRIFT when the resolved indexPath is null AND either .search-code/index.db is on disk OR .devkit/config.json recorded searchCode. It is gated on the dup guard specifically (the only gate that reads the index), skipped when guard.config.json is itself MISSING/unparseable (same root cause, one line), and treats an EXPLICIT null indexPath value as a declared opt-out — resolveGuardConfig collapses absent and explicit-null, so the raw file is the one place they differ, which yields an escape hatch with zero new config surface. SEARCH_CODE_DB is checked directly because it is matcher-only and never reaches resolveGuardConfig. The fixable flag is CONDITIONAL on the recorded selection, since selectionFlags emits --search-code only for a repo that already has it — a warning promising a repair init cannot perform would never clear. It is a CheckResult in default mode and an ADVISORY line in overlay/self-host, which short-circuit before collectResults; without that the check would be unreachable in the devkit repo itself. (2) REPORT: runDeterministic collects fail-open gates into a skipped list, names them on GREEN runs as loudly as red, and emits one gate_result with status could_not_run per skipped gate so the rate becomes measurable at all. (3) BLOCK: GUARD_DETERMINISTIC_STRICT=1 turns an opt-out into label(could-not-run) and exit 1. Strict does NOT flip failOpen2: that flag is a property of the GATE (exit 2 is an opt-out for this one), strict is a property of the RUN (what we do about an opt-out), which is what keeps an --extra command fatal exit 2 at (unexpected:2) instead of relabelling it as a chosen skip. prefixCacheScope salts on strict for the same anti-laundering reason the coverage salt exists — a non-strict all-green key would otherwise be HIT by a later strict run of the identical tree, skipping every gate. | The co-occurrence matcher fails open when guard.config.json has no … | 2026-08-05 | | [gate-telemetry-self-describing](gate-telemetry-self-describing.md) | Gate telemetry is self-describing. (1) Every emitted event identifies its origin: the ship envelope stamps repo/branch from DEVKIT_SHIP_REPO/DEVKIT_SHIP_BRANCH exported by the ship script, degrading to empty strings rather than guessing. (2) Every judgement outcome emits — INCLUDING the non-outcomes: a cache hit emits cache_hit, a lost cache write emits cache_write_failed. (3) Those cheap outcomes get their OWN event type, never a synthetic row on an existing one, and carry the SAME judge label judge_exec uses — so hit rate is cache_hit/(cache_hit+judge_exec) grouped by judge, with no join and no inference. | Story sc-1239 was filed, triaged and nearly implemented against num… | 2026-07-25 | | [gate-verdict-attribution](gate-verdict-attribution.md) | A verdict-producing gate emits its INPUTS and its non-runs, not only its outcomes. (1) Every SELECTED reviewer emits review_scope BEFORE the judge runs — files, diff_sha256, file count and hash, and cached — so a cache hit is attributable on the same footing as a live run. (2) Every reviewer that did NOT run emits review_skipped with the reason (gate_disabled, no_llm, GUARD_REVIEW_SKIP, not_selected), so non-selection is a row and never an absence. (3) Every verdict carries prompt_identity: the same hash formula as the review-mode packaged preflight, computed over the SYNCED consumer assets on the commit/ship path, so a production rate and a bench baseline are comparable when the bytes match. (4) The per-lens checklist vector rides on review_result INCLUDING the passes, with each failing lens attributed to what the gate did with it (blocking, waived, dropped_out_of_charter). Absent items means no artifact — a different fact from an artifact with zero failures. | Reviewer telemetry recorded verdicts but never what produced them, … | 2026-07-27 | +| [judge-mcp-profiles](judge-mcp-profiles.md) | Every Devkit-spawned claude -p judge uses Claude Code strict MCP configuration. Pure and internal judges receive an empty profile. Named reviewer agents receive trusted machine-local codebase, Context7, and autonomous_bugs definitions when configured, and the full autonomous_bugs server permission includes report and amend. Definitions come only from user-owned Claude configuration or an explicit machine-local override, never repository-controlled .mcp.json. Task-dispatched agents use provider-native named server references and retain role-specific research capabilities. Secret-bearing definitions travel through private temporary files, not process arguments. | Concurrent devkit ship runs launch several independent claude -p ju… | 2026-08-15 | | [judge-verdict-cache-scope](judge-verdict-cache-scope.md) | A confident PASS is cached at the altitude of the question the judge answers, not uniformly on the evidence bytes. Completeness judges the commit MESSAGE's claims against what the change delivers, so its PASS is additionally keyed on branch + normalised message + reviewer brief (version-salted), beside the byte-exact key. A retry that reshapes the diff to satisfy a DIFFERENT reviewer, on the same branch under the same message, is not re-judged. A FAIL is never sticky. Message normalisation mirrors git --cleanup=whitespace so the ship's composed temp file and git's COMMIT_EDITMSG compute one identical key across the two hooks. | The completeness gate is straight opus (mean 263s, max 1803s over 1… | 2026-08-07 | | [new-optional-component-offer](new-optional-component-offer.md) | A single registry, OPTIONAL_COMPONENTS in components.mts, drives a generic upgrade step (3c) that offers every optional component a repo has never been asked about. 'Never asked' is detected by the ABSENCE of the recorded key, not a falsy value: applyInit writes every component key on every run, so a repo that answered — yes OR no — carries the key and is never asked again, while a repo predating the component has no key at all. No per-repo 'offers made' state. Nothing is ever auto-added: non-TTY REPORTS only, matching the step-3 gates policy, because an opt-in component arriving because someone ran upgrade in CI is a defect. Correspondingly, a run where nobody was actually asked (non-TTY, or a cancelled prompt) passes those ids to applyInit as InitPlan.undecided, and EVERY config writer — applyInit and applyOverlay alike — runs dropUndecided, so their keys stay absent; otherwise step 4's broad refresh records the normalized 'false' as a decision nobody made and suppresses the offer permanently. The interactive wizard seeds an already-installed optional component into its defaults for the same reason: accepting the defaults on a re-run must not silently drop one. | devkit ships opt-in components after most consumer repos are alread… | 2026-07-29 | | [non-devkit-asset-collision-preserve](non-devkit-asset-collision-preserve.md) | a sync treats a name as the CONSUMER's (preserve, never clobber) iff it (1) exists under a target surface, (2) is NOT recorded in devkit's prior manifest, AND (3) its on-disk bytes DIVERGE from the bundle. Default everywhere is PRESERVE. `devkit init` interactive offers a per-asset `multiselect` (keyed `${kind}:${name}`) to adopt specific collisions; `--force` (package/standalone/overlay) and the standalone `sync-skills`/`sync-agents --force` adopt all. A preserved name is left off the manifest (devkit never claims a file it didn't write). devkit's OWN copies — manifest-owned, or unmanifested-but-byte-identical to the bundle — keep overwriting, so version-bump propagation and self-dogfood are intact. `clean`'s no-manifest fallback gains the same content/tracked guard so it never deletes a preserved untracked user asset. | the sync step (syncSkills/syncAgents/syncHookScripts) hardcoded `wr… | 2026-06-30 | diff --git a/docs/decisions/judge-mcp-profiles.md b/docs/decisions/judge-mcp-profiles.md new file mode 100644 index 00000000..d0bed163 --- /dev/null +++ b/docs/decisions/judge-mcp-profiles.md @@ -0,0 +1,22 @@ +--- +slug: judge-mcp-profiles +created: 2026-08-15 +--- + +# judge-mcp-profiles + +## Target · 2026-08-15 — Strict role-scoped MCP profiles for automated judges + +**Context:** Concurrent devkit ship runs launch several independent claude -p judges. Each judge currently inherits every globally configured MCP server, multiplying unrelated stdio process trees and driving memory, compression, and CPU contention across the machine. +**Ruling:** Every Devkit-spawned claude -p judge uses Claude Code strict MCP configuration. Pure and internal judges receive an empty profile. Named reviewer agents receive trusted machine-local codebase, Context7, and autonomous_bugs definitions when configured, and the full autonomous_bugs server permission includes report and amend. Definitions come only from user-owned Claude configuration or an explicit machine-local override, never repository-controlled .mcp.json. Task-dispatched agents use provider-native named server references and retain role-specific research capabilities. Secret-bearing definitions travel through private temporary files, not process arguments. +**Consequences:** +- Positive: Ship reviewers retain the three useful agent capabilities while unrelated global MCP servers no longer fan out per judge, reducing machine contention without changing the in-chain review architecture. +- Negative: The approved stdio servers can still start once per named judge, full autonomous bug access intentionally permits external issue writes from headless reviewers, and Devkit must maintain Claude-specific registry resolution plus safe degradation when a server is unavailable. +**Vision-fit:** n/a — internal developer tooling reliability and resource isolation +**Researched:** Claude Code CLI strict-mcp-config and subagent mcpServers documentation; Enso, Paperclip, and Takt headless Claude wrappers; prior-art verdict SOLVED_ELSEWHERE with high confidence; feature-critique PROCEED_WITH_CHANGES. +**Rejected:** A flat MCP ban, because named agents lose codebase/docs/autonomy capabilities. Unrestricted inherited MCP discovery, because it recreates the resource fan-out. Repository-controlled server commands, because a commit gate must not execute untrusted configuration from the repository it judges. A new queue as the first fix, because native per-run isolation removes the unnecessary load at its source. +**Revisit-when:** Claude Code provides lazy or shared MCP server connections across independent sessions, Devkit moves its judges to another provider runtime, or measured reviewer quality/resource evidence supports a different baseline profile. +**Scope:** gate-engine/judge/**,gate-engine/review/**,agents/** +**Category:** commit-gates +**Source:** manual +- 2026-08-15 — **Scope:** gate-engine/judge/**,gate-engine/review/**,agents/**,cli/lib/install/agent-assets/** — Claude agent assets require native mcpServers list frontmatter to survive Devkit projection and installation. diff --git a/gate-engine/judge/__tests__/judge-exec-telemetry.test.mts b/gate-engine/judge/__tests__/judge-exec-telemetry.test.mts index a68735f6..23601996 100644 --- a/gate-engine/judge/__tests__/judge-exec-telemetry.test.mts +++ b/gate-engine/judge/__tests__/judge-exec-telemetry.test.mts @@ -16,7 +16,7 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { execJudge, execJudgeAsync, recordAgentRun } from '../run-judge.mts'; import { DIFF_HEADER, OUTPUT_HEADER, readTranscript } from '../transcript-store.mts'; -const ENV_KEYS = ['DEVKIT_GATE_EVENTS', 'DEVKIT_SHIP_ID', 'PATH']; +const ENV_KEYS = ['DEVKIT_GATE_EVENTS', 'DEVKIT_JUDGE_MCP_CONFIG', 'DEVKIT_SHIP_ID', 'PATH']; const saved: Record = {}; let dir: string; let sink: string; @@ -63,6 +63,16 @@ afterEach(() => { }); describe('judge_exec telemetry', () => { + it('strictly disables inherited MCP servers for an unprofiled judge', () => { + fakeClaude('printf \'%s\\n\' "$*"'); + const out = execJudge({ + label: 'detect', + args: ['-p', 'judge this'], + timeout: 30000, + }); + expect(out).toContain('--mcp-config {"mcpServers":{}} --strict-mcp-config'); + }); + it('success emits one ok event with model/duration/sizes AND a transcript by default', () => { fakeClaude('echo FIT'); const out = execJudge({ diff --git a/gate-engine/judge/__tests__/mcp-profile.test.mts b/gate-engine/judge/__tests__/mcp-profile.test.mts new file mode 100644 index 00000000..299f28c3 --- /dev/null +++ b/gate-engine/judge/__tests__/mcp-profile.test.mts @@ -0,0 +1,155 @@ +import { + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + statSync, + symlinkSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { afterAll, describe, expect, it } from 'vitest'; +import { + judgeMcpCapabilityFingerprint, + namedAgentMcpProfile, + prepareJudgeMcpProfile, + withNamedAgentMcpTools, +} from '../mcp/profile.mts'; + +const root = mkdtempSync(path.join(tmpdir(), 'judge-mcp-profile-')); +const repo = path.join(root, 'repo'); +const registry = path.join(root, 'registry.json'); +mkdirSync(repo); + +afterAll(() => { + rmSync(root, { recursive: true, force: true }); +}); + +function writeRegistry(extra: Record = {}): void { + writeFileSync( + registry, + JSON.stringify({ + mcpServers: { + context7: { type: 'stdio', command: 'context7', args: [] }, + autonomous_bugs: { type: 'stdio', command: 'bugs', env: { TOKEN_FILE: '/secret/path' } }, + unrelated: { type: 'stdio', command: 'heavy-server' }, + }, + projects: { + [realpathSync(repo)]: { + mcpServers: { + codebase: { type: 'stdio', command: 'search-code', args: ['mcp'] }, + alternate: { type: 'http', url: 'https://example.test/mcp' }, + }, + }, + }, + ...extra, + }), + { mode: 0o600 }, + ); +} + +describe('judge MCP profiles', () => { + it('uses a strict empty config without reading any registry for pure judges', () => { + const prepared = prepareJudgeMcpProfile({ kind: 'none' }, { cwd: repo }); + expect(prepared.args).toEqual(['--mcp-config', '{"mcpServers":{}}', '--strict-mcp-config']); + expect(prepared.serverNames).toEqual([]); + }); + + it('selects only baseline servers from a trusted machine registry', () => { + writeRegistry(); + const profile = namedAgentMcpProfile(); + const prepared = prepareJudgeMcpProfile(profile, { + cwd: repo, + registryPath: registry, + projectRoots: [repo], + temporaryRoot: root, + }); + const configPath = prepared.args[1] as string; + const config = JSON.parse(readFileSync(configPath, 'utf8')) as { + mcpServers: Record; + }; + expect(Object.keys(config.mcpServers).sort()).toEqual([ + 'autonomous_bugs', + 'codebase', + 'context7', + ]); + expect(config.mcpServers).not.toHaveProperty('unrelated'); + expect(statSync(path.dirname(configPath)).mode & 0o777).toBe(0o700); + expect(statSync(configPath).mode & 0o777).toBe(0o600); + expect(prepared.args.join(' ')).not.toContain('TOKEN_FILE'); + prepared.cleanup(); + expect(() => statSync(configPath)).toThrow(); + }); + + it('does not let an allowed repository-configured tool activate another MCP server', () => { + writeRegistry(); + const profile = namedAgentMcpProfile(); + const prepared = prepareJudgeMcpProfile(profile, { + cwd: repo, + registryPath: registry, + projectRoots: [repo], + temporaryRoot: root, + }); + const config = JSON.parse(readFileSync(prepared.args[1] as string, 'utf8')) as { + mcpServers: Record; + }; + expect(withNamedAgentMcpTools('Read', 'mcp__alternate__query')).toContain( + 'mcp__alternate__query', + ); + expect(config.mcpServers).not.toHaveProperty('alternate'); + prepared.cleanup(); + }); + + it('changes the capability fingerprint when a selected trusted server definition changes', () => { + writeRegistry(); + const options = { cwd: repo, registryPath: registry, projectRoots: [repo] }; + const first = judgeMcpCapabilityFingerprint(namedAgentMcpProfile(), 'Read', options); + writeRegistry({ + projects: { + [realpathSync(repo)]: { + mcpServers: { codebase: { type: 'stdio', command: 'search-code', args: ['mcp', 'v2'] } }, + }, + }, + }); + expect(judgeMcpCapabilityFingerprint(namedAgentMcpProfile(), 'Read', options)).not.toBe(first); + }); + + it('never trusts a repository-controlled config or a symlinked override', () => { + const repositoryConfig = path.join(repo, '.mcp.json'); + writeFileSync( + repositoryConfig, + JSON.stringify({ mcpServers: { codebase: { command: 'malicious' } } }), + { mode: 0o600 }, + ); + const fromRepo = prepareJudgeMcpProfile(namedAgentMcpProfile(), { + cwd: repo, + registryPath: repositoryConfig, + }); + expect(fromRepo.serverNames).toEqual([]); + + writeRegistry(); + const link = path.join(root, 'registry-link.json'); + symlinkSync(registry, link); + const fromLink = prepareJudgeMcpProfile(namedAgentMcpProfile(), { + cwd: repo, + registryPath: link, + }); + expect(fromLink.serverNames).toEqual([]); + }); + + it('grants the complete autonomous_bugs server namespace to named agents', () => { + const tools = withNamedAgentMcpTools('Read,Grep', 'mcp__alternate__query'); + expect(tools.split(',')).toEqual( + expect.arrayContaining([ + 'Read', + 'Grep', + 'mcp__codebase', + 'mcp__context7', + 'mcp__autonomous_bugs', + 'mcp__alternate__query', + ]), + ); + }); +}); diff --git a/gate-engine/judge/mcp/profile.mts b/gate-engine/judge/mcp/profile.mts new file mode 100644 index 00000000..dd77129a --- /dev/null +++ b/gate-engine/judge/mcp/profile.mts @@ -0,0 +1,294 @@ +import { execFileSync } from 'node:child_process'; +import { createHash } from 'node:crypto'; +import { + chmodSync, + lstatSync, + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + statSync, + writeFileSync, +} from 'node:fs'; +import { homedir, tmpdir } from 'node:os'; +import path from 'node:path'; +import { withoutGitEnv } from '../judge-isolation.mts'; + +const BASELINE_SERVER_NAMES = ['codebase', 'context7', 'autonomous_bugs'] as const; +const BASELINE_TOOL_PREFIXES = BASELINE_SERVER_NAMES.map((name) => `mcp__${name}`); +const EMPTY_MCP_CONFIG = '{"mcpServers":{}}'; +const REGISTRY_ENV = 'DEVKIT_JUDGE_MCP_CONFIG'; + +type JsonRecord = Record; +type McpServers = Record; + +interface RegistryCacheEntry { + stamp: string; + value: JsonRecord | null; +} + +const registryCache = new Map(); +const warned = new Set(); + +export interface NamedAgentMcpProfile { + kind: 'named-agent'; + serverNames: readonly string[]; +} + +export type JudgeMcpProfile = { kind: 'none' } | NamedAgentMcpProfile; + +export interface PreparedJudgeMcpProfile { + args: string[]; + serverNames: string[]; + cleanup: () => void; +} + +export interface PrepareJudgeMcpOptions { + cwd: string; + env?: NodeJS.ProcessEnv; + registryPath?: string; + projectRoots?: readonly string[]; + temporaryRoot?: string; +} + +const isRecord = (value: unknown): value is JsonRecord => + typeof value === 'object' && value !== null && !Array.isArray(value); + +function warnOnce(key: string, message: string): void { + if (warned.has(key)) return; + warned.add(key); + console.error(message); +} + +function isInside(root: string, candidate: string): boolean { + const rel = path.relative(root, candidate); + return rel === '' || (!rel.startsWith('..') && !path.isAbsolute(rel)); +} + +function trustedRegistryPath(requested: string, cwd: string, explicit: boolean): string | null { + try { + if (!path.isAbsolute(requested)) return null; + const entry = lstatSync(requested); + if (!entry.isFile() || entry.isSymbolicLink()) return null; + if (typeof process.getuid === 'function' && entry.uid !== process.getuid()) return null; + if ((entry.mode & 0o022) !== 0) return null; + const canonical = realpathSync(requested); + const canonicalCwd = realpathSync(cwd); + if (explicit && isInside(canonicalCwd, canonical)) return null; + return canonical; + } catch { + return null; + } +} + +function readRegistry(file: string): JsonRecord | null { + try { + const stat = statSync(file); + const stamp = `${stat.mtimeMs}:${stat.size}`; + const cached = registryCache.get(file); + if (cached?.stamp === stamp) return cached.value; + const parsed = JSON.parse(readFileSync(file, 'utf8')) as unknown; + const value = isRecord(parsed) ? parsed : null; + registryCache.set(file, { stamp, value }); + return value; + } catch { + return null; + } +} + +function validServer(value: unknown): JsonRecord | null { + if (!isRecord(value) || value.disabled === true) return null; + const command = value.command; + const url = value.url; + if (typeof command !== 'string' && typeof url !== 'string') return null; + const { disabled: _disabled, ...server } = value; + return server; +} + +function serverTable(value: unknown): McpServers { + if (!isRecord(value)) return {}; + const result: McpServers = {}; + for (const [name, server] of Object.entries(value)) { + const valid = validServer(server); + if (valid) result[name] = valid; + } + return result; +} + +function primaryCheckoutRoot(cwd: string, env: NodeJS.ProcessEnv): string | null { + try { + const common = execFileSync( + 'git', + ['rev-parse', '--path-format=absolute', '--git-common-dir'], + { cwd, env: withoutGitEnv(env), encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }, + ).trim(); + if (!common || path.basename(common) !== '.git') return null; + return realpathSync(path.dirname(common)); + } catch { + return null; + } +} + +function projectCandidates( + cwd: string, + env: NodeJS.ProcessEnv, + supplied?: readonly string[], +): string[] { + const candidates = supplied ?? [cwd, primaryCheckoutRoot(cwd, env)].filter(Boolean); + const result: string[] = []; + for (const candidate of candidates) { + if (!candidate) continue; + try { + const canonical = realpathSync(candidate); + if (!result.includes(canonical)) result.push(canonical); + } catch { + const resolved = path.resolve(candidate); + if (!result.includes(resolved)) result.push(resolved); + } + } + return result; +} + +function selectedServers( + registry: JsonRecord, + serverNames: readonly string[], + roots: readonly string[], +): McpServers { + const selected: McpServers = {}; + const rootServers = serverTable(registry.mcpServers); + const projects = isRecord(registry.projects) ? registry.projects : {}; + const projectRows = roots.map((root) => projects[root]).filter(isRecord); + + for (const name of serverNames) { + let server: JsonRecord | null = rootServers[name] ?? null; + for (const project of projectRows) { + const disabled = Array.isArray(project.disabledMcpServers) ? project.disabledMcpServers : []; + if (disabled.includes(name)) { + server = null; + continue; + } + const projectServer = serverTable(project.mcpServers)[name]; + if (projectServer) server = projectServer; + } + if (server) selected[name] = server; + } + return selected; +} + +export function namedAgentMcpProfile(): NamedAgentMcpProfile { + return { + kind: 'named-agent', + serverNames: BASELINE_SERVER_NAMES, + }; +} + +export function withNamedAgentMcpTools(tools: string, ...extraTools: string[]): string { + const values = [tools, ...BASELINE_TOOL_PREFIXES, ...extraTools] + .flatMap((value) => value.split(',')) + .map((value) => value.trim()) + .filter(Boolean); + return [...new Set(values)].join(','); +} + +/** + * Stable, secret-safe cache partition for the capabilities a named judge can actually receive. + * The digest includes the declared tool set, trusted registry location, and selected definitions; + * cache entries therefore never survive a capability change, while secret-bearing config is never + * written to the cache itself. + */ +export function judgeMcpCapabilityFingerprint( + profile: JudgeMcpProfile, + allowedTools: string, + options: PrepareJudgeMcpOptions, +): string { + const env = options.env ?? process.env; + const explicit = options.registryPath !== undefined || env[REGISTRY_ENV] !== undefined; + const requested = + options.registryPath ?? env[REGISTRY_ENV] ?? path.join(homedir(), '.claude.json'); + const registryPath = trustedRegistryPath(requested, options.cwd, explicit); + const registry = registryPath ? readRegistry(registryPath) : null; + const servers = + profile.kind === 'named-agent' && registry + ? selectedServers( + registry, + profile.serverNames, + projectCandidates(options.cwd, env, options.projectRoots), + ) + : {}; + return createHash('sha256') + .update( + JSON.stringify({ allowedTools, profile, registryPath: registryPath ?? requested, servers }), + ) + .digest('hex'); +} + +function emptyProfile(): PreparedJudgeMcpProfile { + return { + args: ['--mcp-config', EMPTY_MCP_CONFIG, '--strict-mcp-config'], + serverNames: [], + cleanup: () => {}, + }; +} + +export function prepareJudgeMcpProfile( + profile: JudgeMcpProfile, + options: PrepareJudgeMcpOptions, +): PreparedJudgeMcpProfile { + if (profile.kind === 'none') return emptyProfile(); + + const env = options.env ?? process.env; + const explicit = options.registryPath !== undefined || env[REGISTRY_ENV] !== undefined; + const requested = + options.registryPath ?? env[REGISTRY_ENV] ?? path.join(homedir(), '.claude.json'); + const registryPath = trustedRegistryPath(requested, options.cwd, explicit); + if (!registryPath) { + warnOnce( + `registry:${requested}`, + `guard-review: trusted MCP registry unavailable at ${requested} — named agents continue with strict-empty MCP isolation`, + ); + return emptyProfile(); + } + const registry = readRegistry(registryPath); + if (!registry) { + warnOnce( + `registry-json:${registryPath}`, + 'guard-review: trusted MCP registry is unreadable — named agents continue with strict-empty MCP isolation', + ); + return emptyProfile(); + } + + const roots = projectCandidates(options.cwd, env, options.projectRoots); + const servers = selectedServers(registry, profile.serverNames, roots); + const present = Object.keys(servers); + const missing = profile.serverNames.filter((name) => !present.includes(name)); + if (missing.length > 0) + warnOnce( + `missing:${registryPath}:${missing.join(',')}`, + `guard-review: named-agent MCP profile missing ${missing.join(', ')} — continuing with the configured subset under strict isolation`, + ); + if (present.length === 0) return emptyProfile(); + + let directory: string | null = null; + try { + directory = mkdtempSync(path.join(options.temporaryRoot ?? tmpdir(), 'devkit-judge-mcp-')); + chmodSync(directory, 0o700); + const file = path.join(directory, 'mcp.json'); + writeFileSync(file, `${JSON.stringify({ mcpServers: servers })}\n`, { + encoding: 'utf8', + mode: 0o600, + flag: 'wx', + }); + return { + args: ['--mcp-config', file, '--strict-mcp-config'], + serverNames: present, + cleanup: () => rmSync(directory as string, { recursive: true, force: true }), + }; + } catch { + if (directory) rmSync(directory, { recursive: true, force: true }); + warnOnce( + 'temporary-config', + 'guard-review: private MCP profile file could not be created — named agents continue with strict-empty MCP isolation', + ); + return emptyProfile(); + } +} diff --git a/gate-engine/judge/run-judge.mts b/gate-engine/judge/run-judge.mts index fac1bd7b..5b93daa5 100644 --- a/gate-engine/judge/run-judge.mts +++ b/gate-engine/judge/run-judge.mts @@ -25,6 +25,7 @@ import { } from './claude-result.mts'; import { emitGateEvent } from './gate-events.mts'; import { withoutGitEnv } from './judge-isolation.mts'; +import { type JudgeMcpProfile, prepareJudgeMcpProfile } from './mcp/profile.mts'; import { composeTranscript, saveTranscriptUnique } from './transcript-store.mts'; // The error thrown/handed back by a `claude` spawn — a Node exec error augmented with these fields. @@ -170,6 +171,8 @@ interface ExecJudgeOpts { env?: NodeJS.ProcessEnv; onOutage?: (kind: 'timeout' | 'transient' | 'empty') => void; transcript?: boolean; + /** Strict MCP profile. Omitted means a pure/internal judge with no MCP servers. */ + mcpProfile?: JudgeMcpProfile; } /** The `--model ` value from a judge argv, for the telemetry event; null when absent. */ @@ -279,8 +282,12 @@ function readJudgeOutput(stdout: string): { text: string; usage: JudgeUsage | nu export function execJudge(opts: ExecJudgeOpts): string | null { const { label, args, input, timeout, cwd, env, onOutage } = opts; const startedAt = Date.now(); + const mcp = prepareJudgeMcpProfile(opts.mcpProfile ?? { kind: 'none' }, { + cwd: cwd ?? process.cwd(), + env, + }); try { - const out = execFileSync('claude', withResultArgs(args), { + const out = execFileSync('claude', withResultArgs([...mcp.args, ...args]), { cwd, // Never the caller's env verbatim: git leaks an ABSOLUTE GIT_INDEX_FILE/GIT_DIR into every // hook run in a linked worktree (how ship commits), and a tool-using judge that touches @@ -309,6 +316,8 @@ export function execJudge(opts: ExecJudgeOpts): string | null { emitJudgeExec(opts, kind, startedAt); onOutage?.(kind); return null; + } finally { + mcp.cleanup(); } } @@ -326,6 +335,10 @@ export function execJudge(opts: ExecJudgeOpts): string | null { export function execJudgeAsync(opts: ExecJudgeOpts): Promise { const { label, args, input, timeout, cwd, env, onOutage } = opts; const startedAt = Date.now(); + const mcp = prepareJudgeMcpProfile(opts.mcpProfile ?? { kind: 'none' }, { + cwd: cwd ?? process.cwd(), + env, + }); return new Promise((resolve) => { // Shared outage path — a callback error AND a synchronous throw from execFile() itself (e.g. an // out-of-range `timeout` validates and throws before spawn even starts, sc-1317) both resolve @@ -334,6 +347,7 @@ export function execJudgeAsync(opts: ExecJudgeOpts): Promise { // resolves) for any caller awaiting it outside its own try/catch — the sync execJudge twin // already had this same guard via its enclosing try/catch. const fail = (err: unknown) => { + mcp.cleanup(); warnUnavailable(label, err, timeout); const kind = isJudgeTimeout(err) ? 'timeout' : 'transient'; emitJudgeExec(opts, kind, startedAt); @@ -343,7 +357,7 @@ export function execJudgeAsync(opts: ExecJudgeOpts): Promise { try { const child = execFile( 'claude', - withResultArgs(args), + withResultArgs([...mcp.args, ...args]), { cwd, // env: see the execJudge twin — the git-env scrub applies to every judge spawn. @@ -359,6 +373,7 @@ export function execJudgeAsync(opts: ExecJudgeOpts): Promise { fail(err); return; } + mcp.cleanup(); if (!stdout || !String(stdout).trim()) { warnNoOutput(label); emitJudgeExec(opts, 'empty', startedAt); diff --git a/gate-engine/review/__tests__/completeness-capability-cache.test.mts b/gate-engine/review/__tests__/completeness-capability-cache.test.mts new file mode 100644 index 00000000..c2fae9a5 --- /dev/null +++ b/gate-engine/review/__tests__/completeness-capability-cache.test.mts @@ -0,0 +1,83 @@ +import { execSync } from 'node:child_process'; +import { mkdtempSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { runCompleteness } from '../completeness.mts'; +import { + cleanupReviewFixtures, + consumerRepo, + mkExec, + trackReviewFixtureDir, +} from './run-review-fixtures.mts'; + +const ENV_KEYS = ['DEVKIT_JUDGE_MCP_CONFIG', 'DEVKIT_SHIP_BRANCH'] as const; +const saved: Record = {}; + +beforeEach(() => { + for (const key of ENV_KEYS) { + saved[key] = process.env[key]; + delete process.env[key]; + } +}); + +afterEach(() => { + cleanupReviewFixtures(); + for (const key of ENV_KEYS) { + if (saved[key] === undefined) delete process.env[key]; + else process.env[key] = saved[key]; + } +}); + +function messageFile(repo: string, message: string): string { + const file = join(repo, '.git', 'COMMIT_EDITMSG_TEST'); + writeFileSync(file, message); + return file; +} + +function writeTrustedRegistry(file: string, version: string): void { + writeFileSync( + file, + JSON.stringify({ + mcpServers: { + codebase: { type: 'stdio', command: 'search-code', args: [version] }, + context7: { type: 'stdio', command: 'context7' }, + autonomous_bugs: { type: 'stdio', command: 'autonomous-bugs' }, + }, + }), + { mode: 0o600 }, + ); +} + +describe('runCompleteness capability cache partition', () => { + it('invalidates exact and sticky PASSes when the trusted MCP profile changes', async () => { + const repo = consumerRepo({ backend: true }); + const configRoot = trackReviewFixtureDir(mkdtempSync(join(tmpdir(), 'completeness-mcp-'))); + const registry = join(configRoot, 'claude.json'); + process.env.DEVKIT_JUDGE_MCP_CONFIG = registry; + process.env.DEVKIT_SHIP_BRANCH = 'feat/capability-cache'; + const exec = mkExec(async () => 'VERDICT: PASS'); + + writeTrustedRegistry(registry, 'v1'); + expect(await runCompleteness(messageFile(repo, 'feat: capability cache'), repo, { exec })).toBe( + 0, + ); + expect(exec).toHaveBeenCalledTimes(1); + + // New message misses sticky by itself; unchanged diff proves the exact key also includes profile. + writeTrustedRegistry(registry, 'v2-with-different-definition'); + expect( + await runCompleteness(messageFile(repo, 'feat: capability cache amended'), repo, { exec }), + ).toBe(0); + expect(exec).toHaveBeenCalledTimes(2); + + // New diff misses exact by itself; original message proves the sticky key also includes profile. + writeFileSync(join(repo, 'src', 'main', 'db.ts'), 'export const q = 2;\n'); + execSync('git add src/main/db.ts', { cwd: repo }); + writeTrustedRegistry(registry, 'v3-with-another-definition'); + expect(await runCompleteness(messageFile(repo, 'feat: capability cache'), repo, { exec })).toBe( + 0, + ); + expect(exec).toHaveBeenCalledTimes(3); + }); +}); diff --git a/gate-engine/review/__tests__/conventions-evidence.test.mts b/gate-engine/review/__tests__/conventions-evidence.test.mts index 0950ec77..f9f46e97 100644 --- a/gate-engine/review/__tests__/conventions-evidence.test.mts +++ b/gate-engine/review/__tests__/conventions-evidence.test.mts @@ -39,7 +39,7 @@ describe('conventions evidence completeness', () => { const [call] = exec.mock.calls[0]; expect(call.label).toBe('review:conventions-reviewer'); expect(call.input).toContain('OMITTED'); - expect(call.args).toContain('Read,Grep,Glob'); + expect(call.args).toContain('Read,Grep,Glob,mcp__codebase,mcp__context7,mcp__autonomous_bugs'); expect(call.args[1]).toContain('use Read to inspect every available in-scope staged file'); expect(call.args[1]).toContain('must not produce a semantic FAIL'); }); diff --git a/gate-engine/review/__tests__/reviewers.test.mts b/gate-engine/review/__tests__/reviewers.test.mts index 988563a3..f9106498 100644 --- a/gate-engine/review/__tests__/reviewers.test.mts +++ b/gate-engine/review/__tests__/reviewers.test.mts @@ -259,7 +259,8 @@ describe('allowedToolsFor', () => { const tools = allowedToolsFor(REVIEWERS[0], cfg); expect(tools).toBe( 'Read,Grep,Glob,Bash(git diff:*),Bash(git log:*),Bash(git status:*),' + - 'Bash(node .claude/skills/api-security/scripts/checklist.mjs:*)', + 'Bash(node .claude/skills/api-security/scripts/checklist.mjs:*),' + + 'mcp__codebase,mcp__context7,mcp__autonomous_bugs', ); expect(tools).not.toMatch(/(^|,)Bash(,|$)/); // never an unscoped Bash expect(tools).not.toMatch(/Write|Edit/); @@ -275,9 +276,12 @@ describe('allowedToolsFor', () => { expect(tools).toContain('Bash(node .agents/skills/api-security/scripts/checklist.mjs:*)'); expect(tools).not.toContain('Bash(node .claude/skills/api-security/scripts/checklist.mjs:*)'); }); - it('a skill-less reviewer (conventions) gets EXACTLY Read,Grep,Glob — no Bash at all, per its AC', () => { + it('a skill-less reviewer gets read tools plus the three named MCPs, but no Bash', () => { const conv = REVIEWERS.find((r) => r.name === 'conventions-reviewer'); - expect(allowedToolsFor(conv, cfg)).toBe('Read,Grep,Glob'); + expect(allowedToolsFor(conv, cfg)).toBe( + 'Read,Grep,Glob,mcp__codebase,mcp__context7,mcp__autonomous_bugs', + ); + expect(allowedToolsFor(conv, cfg)).not.toContain('Bash'); }); }); diff --git a/gate-engine/review/__tests__/run-review.test.mts b/gate-engine/review/__tests__/run-review.test.mts index 53723a65..f0095a55 100644 --- a/gate-engine/review/__tests__/run-review.test.mts +++ b/gate-engine/review/__tests__/run-review.test.mts @@ -1478,7 +1478,6 @@ describe('runReviewGate — per-completion checkpoints', () => { ); }); }); - describe('runReviewGate — bounded judge concurrency (sc-1050)', () => { // consumerRepo({backend, frontend}) stages one file per domain → all 7 reviewers selected // (backend pair, frontend pair, commit-guard, correctness, conventions). @@ -1487,10 +1486,12 @@ describe('runReviewGate — bounded judge concurrency (sc-1050)', () => { const probe = concurrencyProbe(repo); expect(await runReviewGate(repo, { exec: probe.exec })).toBe(0); expect(probe.exec).toHaveBeenCalledTimes(7); + expect(probe.exec.mock.calls.every(([opts]) => opts.mcpProfile?.kind === 'named-agent')).toBe( + true, + ); expect(probe.maxInflight()).toBe(6); expect(Object.keys(loadCache(repo)).length).toBe(7); }); - it('GUARD_REVIEW_CONCURRENCY=1 fully serializes — never more than 1 in flight', async () => { const repo = consumerRepo({ backend: true, frontend: true }); process.env.GUARD_REVIEW_CONCURRENCY = '1'; @@ -1498,7 +1499,6 @@ describe('runReviewGate — bounded judge concurrency (sc-1050)', () => { expect(await runReviewGate(repo, { exec: probe.exec })).toBe(0); expect(probe.maxInflight()).toBe(1); }); - it('a cap ≥ reviewer count only BOUNDS, never pads — all 7 run at once', async () => { const repo = consumerRepo({ backend: true, frontend: true }); process.env.GUARD_REVIEW_CONCURRENCY = '9'; diff --git a/gate-engine/review/cascade/reviewer.mts b/gate-engine/review/cascade/reviewer.mts index 680de666..24dd8de5 100644 --- a/gate-engine/review/cascade/reviewer.mts +++ b/gate-engine/review/cascade/reviewer.mts @@ -1,5 +1,6 @@ import type { GuardConfig } from '../../config.mts'; import { JUDGE_ISOLATION } from '../../judge/judge-isolation.mts'; +import { namedAgentMcpProfile } from '../../judge/mcp/profile.mts'; import { DEEP_JUDGE_TIMEOUT_MS, execJudgeAsync } from '../../judge/run-judge.mts'; import { renderGoverningClaudeMd } from '../claude-md.mts'; import { buildCappedDiffEvidence } from '../diff-evidence.mts'; @@ -115,6 +116,8 @@ async function cascadeVerdict( ) : wrapConventionsPrompt(body, files, renderGoverningClaudeMd(cwd, files), promptExtras); const input = buildCappedDiffEvidence(gitCached(cwd, [], files), stat); + const allowedTools = allowedToolsFor(reviewer, cfg, checklistRoot); + const mcpProfile = namedAgentMcpProfile(); const args = (promptBody: string, model: string): string[] => [ '-p', promptBody, @@ -122,7 +125,7 @@ async function cascadeVerdict( model, ...JUDGE_ISOLATION, '--allowedTools', - allowedToolsFor(reviewer, cfg, checklistRoot), + allowedTools, ]; const passModel = reviewer.model ?? firstModel; let firstOutage: 'timeout' | 'transient' | 'empty' | undefined; @@ -133,6 +136,7 @@ async function cascadeVerdict( timeout: DEEP_JUDGE_TIMEOUT_MS, cwd, transcript: false, + mcpProfile, env, onOutage: (kind: 'timeout' | 'transient' | 'empty') => { firstOutage = kind; @@ -191,6 +195,7 @@ async function cascadeVerdict( timeout: DEEP_JUDGE_TIMEOUT_MS, cwd, transcript: false, + mcpProfile, env, onOutage: (kind: 'timeout' | 'transient' | 'empty') => { secondOutage = kind; diff --git a/gate-engine/review/completeness.mts b/gate-engine/review/completeness.mts index 1ebc28e4..f24b237b 100644 --- a/gate-engine/review/completeness.mts +++ b/gate-engine/review/completeness.mts @@ -41,6 +41,11 @@ export { renderTargets, type TargetBlock } from './evidence/targets-block.mts'; import { emitCacheHit, finishGateTiming } from '../judge/gate-events.mts'; import { JUDGE_ISOLATION } from '../judge/judge-isolation.mts'; +import { + judgeMcpCapabilityFingerprint, + namedAgentMcpProfile, + withNamedAgentMcpTools, +} from '../judge/mcp/profile.mts'; import { reportGateInfraFailure } from '../judge/odb-probe.mts'; import { DEEP_JUDGE_TIMEOUT_MS, execJudgeAsync, strictRemedy } from '../judge/run-judge.mts'; import { loadCache, savePasses } from './cache.mts'; @@ -123,10 +128,15 @@ export async function runCompleteness( if (envFlag('NO_COMPLETENESS')) return finish(0); let prompt: string; let diff: string; + let allowedTools = withNamedAgentMcpTools(TOOLS); + const mcpProfile = namedAgentMcpProfile(); + let capabilityFingerprint = ''; let stickyKey = ''; try { const cfg = resolveGuardConfig(cwd); if (cfg.noLlm) return finish(0); + allowedTools = withNamedAgentMcpTools(TOOLS, cfg.indexPath ? cfg.searchTool : ''); + capabilityFingerprint = judgeMcpCapabilityFingerprint(mcpProfile, allowedTools, { cwd }); const message = normalizeCommitMessage( readFileSync(path.isAbsolute(msgFile) ? msgFile : path.resolve(cwd, msgFile), 'utf8'), ); @@ -154,7 +164,11 @@ export async function runCompleteness( // A FAIL is never sticky (only the confident-PASS save below writes this key), so a found gap // must genuinely be re-judged closed. Checked before scopedTargets/diff assembly — a sticky // hit skips the retrieval work too, not just the judge. - stickyKey = cacheKey('completeness-intent', `${verdictBranch(cwd)}\u0000${message}`, body); + stickyKey = cacheKey( + 'completeness-intent', + `${verdictBranch(cwd)}\u0000${message}`, + `${body}\u0000${capabilityFingerprint}`, + ); const sticky = loadCache(cwd)[stickyKey]; if (sticky) { console.error( @@ -204,7 +218,7 @@ export async function runCompleteness( // (docs/decisions/ship-gates-converge-not-restart.md), and completeness was outside it. // Key = every byte the judge reads: the prompt (message, governing Targets, brief) plus the // capped stdin evidence. An amended message or a re-staged hunk therefore MISSES and re-judges. - const key = cacheKey('completeness', diff, prompt); + const key = cacheKey('completeness', diff, `${prompt}\u0000${capabilityFingerprint}`); const hit = loadCache(cwd)[key]; if (hit) { console.error('guard-review: completeness — cached PASS (identical judgement)'); @@ -217,10 +231,11 @@ export async function runCompleteness( let outage: 'timeout' | 'transient' | 'empty' | undefined; const raw = await exec({ label: 'review:completeness', - args: ['-p', prompt, '--model', 'opus', ...JUDGE_ISOLATION, '--allowedTools', TOOLS], + args: ['-p', prompt, '--model', 'opus', ...JUDGE_ISOLATION, '--allowedTools', allowedTools], input: diff, timeout: DEEP_JUDGE_TIMEOUT_MS, cwd, + mcpProfile, onOutage: (kind) => { outage = kind; }, diff --git a/gate-engine/review/reviewers.mts b/gate-engine/review/reviewers.mts index 83dc6dd4..92708b44 100644 --- a/gate-engine/review/reviewers.mts +++ b/gate-engine/review/reviewers.mts @@ -13,6 +13,7 @@ import { createHash } from 'node:crypto'; import { normalizeReviewRoots } from '../../skills/_devkit/review-roots.mjs'; import { type GuardConfig, sourceMatchers } from '../config.mts'; import { devkitVersion } from '../devkit-version.mts'; +import { withNamedAgentMcpTools } from '../judge/mcp/profile.mts'; import { checklistContractFor } from './lens/split.mts'; /** The resolved governance-gate config shape (the review cluster reads its `review.*`, `scanRoots`, @@ -245,24 +246,23 @@ export function selectReviewers(stagedFiles: string[], cfg: GuardConfig): Review * Comma-joined --allowedTools value for one reviewer: the read-only base, PLUS its own checklist * script (the one non-git Bash prefix a judge gets — scoped to that exact script path, so the * judge can drive its checklist but still cannot write files, stage, or commit), PLUS the - * consumer's semantic search tool for commit-guard. + * consumer's semantic search tool for commit-guard, plus every named agent's strict MCP baseline. */ export function allowedToolsFor( reviewer: Reviewer, cfg: GuardConfig, assetRoot = '.claude', ): string { - // A skill-less reviewer (e.g. conventions-reviewer) has no checklist script to grant Bash for, - // and its AC forbids Bash entirely — Read/Grep/Glob only, full stop, no BASE_TOOLS git-diff Bash - // either (its evidence is pre-rendered onto stdin/prompt instead — see wrapConventionsPrompt). - if (!hasChecklist(reviewer)) return 'Read,Grep,Glob'; + // A skill-less reviewer has no checklist script; its evidence is pre-rendered onto stdin/prompt. + if (!hasChecklist(reviewer)) return withNamedAgentMcpTools('Read,Grep,Glob'); const tools = `${BASE_TOOLS},Bash(node ${checklistScriptAt(reviewer, assetRoot)}:*)`; - if (reviewer.domain === 'code') return `${tools},${cfg.searchTool}`; + if (reviewer.domain === 'code') return withNamedAgentMcpTools(tools, cfg.searchTool); // The correctness reviewer's writer/reader-contract lens benefits from semantic search, but // only when the consumer actually wired an index (indexPath set) — otherwise cfg.searchTool is // a generic default naming an MCP tool the judge doesn't have, and Grep is the core mechanism. - if (reviewer.domain === 'all' && cfg.indexPath) return `${tools},${cfg.searchTool}`; - return tools; + if (reviewer.domain === 'all' && cfg.indexPath) + return withNamedAgentMcpTools(tools, cfg.searchTool); + return withNamedAgentMcpTools(tools); } /** Strip a leading YAML frontmatter block from an agent .md. */