Review RFC Skill

This skill helps you thoroughly review RFCs for the ToolHive ecosystem, ensuring they meet quality standards, architectural alignment, and security requirements.

Overview

When reviewing an RFC, you should evaluate it against multiple dimensions: completeness, technical accuracy, architectural alignment, security considerations, and feasibility.

Review Workflow

Step 1: Read the RFC

First, read the RFC document completely. If provided a PR number or file path, fetch and read it.

Step 2: Fetch Architectural Context

Before reviewing, gather context from the ToolHive architecture documentation. Use mcp__github__get_file_contents to read relevant docs from stacklok/toolhive repo's docs/arch/ directory:

Step 3: Check Related Existing RFCs

Search this repository for related RFCs that might:

• Conflict with the proposal
• Be superseded by the proposal
• Provide context or dependencies

Step 4: Verify Against Target Repository

Search the target repository to verify:

• Proposed changes align with existing code patterns
• No conflicts with recent changes
• API changes are compatible with existing interfaces

Review Checklist

A. Structure and Completeness

• Metadata present: Status, Author, Created, Last Updated, Target Repository
• Summary: Clear 2-3 sentence description
• Problem Statement: Clearly articulates the problem, who's affected, why it matters
• Goals: Specific, measurable objectives listed
• Non-Goals: Explicit scope boundaries defined
• Proposed Solution: Detailed design with diagrams where appropriate
• Security Considerations: All required subsections present (see below)
• Alternatives Considered: At least one alternative evaluated
• Compatibility: Backward and forward compatibility addressed
• Implementation Plan: Phased approach with concrete tasks
• Testing Strategy: Multiple test levels covered
• Open Questions: Unresolved items listed (if any)

B. Security Review (CRITICAL)

The Security Considerations section MUST address all of these:

Red flags to watch for:

• Missing or superficial security section
• "N/A" without justification for security subsections
• No threat model for network-exposed features
• Secrets in configuration examples
• Missing input validation for user-provided data
• No audit logging for security-relevant operations

C. Technical Accuracy

• Correct terminology: Uses ToolHive concepts correctly (Workloads, Transports, Middleware, etc.)
• Architecture alignment: Follows established patterns and principles
• Code examples: Syntactically correct, idiomatic for the language
• API design: Consistent with existing APIs in the target repo
• CRD design: Follows Kubernetes conventions if applicable
• Configuration format: Matches existing RunConfig patterns

D. Diagrams and Examples

• Mermaid diagrams: Complex flows illustrated clearly
• Code examples: Concrete, not abstract placeholders
• Configuration examples: Realistic YAML/JSON examples
• Sequence diagrams: For multi-component interactions

E. Feasibility and Impact

• Implementation complexity: Is the phased approach realistic?
• Dependencies: Are external dependencies identified?
• Breaking changes: Are migration paths provided if needed?
• Performance impact: Considered where relevant?
• Cross-repo impact: If multiple repos, are all impacts identified?

ToolHive Ecosystem Context

Target Repositories

Key Architecture Principles to Verify

1. Platform, not runner: Does this enhance the platform abstraction?
2. Security by default: Does this maintain or improve security posture?
3. Middleware composability: Can this be implemented as middleware if appropriate?
4. RunConfig portability: Does this preserve configuration portability?
5. Cloud-native: Is this Kubernetes-friendly where applicable?

CRD Types (for K8s-related RFCs)

• MCPServer - Individual MCP server deployment
• MCPRegistry - Registry configuration
• MCPToolConfig - Tool filtering and configuration
• MCPExternalAuthConfig - External authentication
• MCPGroup - Server grouping
• VirtualMCPServer - Aggregation of multiple servers

Review Output Format

Structure your review as follows:

## RFC Review: [RFC Title]

### Summary
[1-2 sentence summary of your overall assessment]

### Strengths
- [What the RFC does well]

### Areas for Improvement

#### Critical Issues (Must Fix)
- [Issues that must be addressed before acceptance]

#### Suggestions (Should Consider)
- [Improvements that would strengthen the RFC]

#### Minor/Nitpicks (Optional)
- [Small improvements or style suggestions]

### Security Assessment
[Specific feedback on the security section]

### Architectural Alignment
[How well does this align with ToolHive architecture?]

### Questions for the Author
- [Clarifying questions that need answers]

### Recommendation
[ ] Ready to accept
[ ] Accept with minor changes
[ ] Needs revision (address critical issues)
[ ] Major rework needed

Common Issues to Watch For

Problem Statement Issues

• Too vague or abstract
• Doesn't explain who benefits
• Problem already solved elsewhere

Design Issues

• Over-engineered for the problem
• Missing error handling considerations
• Doesn't consider edge cases
• Breaks existing functionality without migration path

Security Issues

• Missing threat model
• Hardcoded credentials in examples
• No input validation
• Missing audit logging
• Overly permissive defaults

Implementation Issues

• Unrealistic phasing
• Missing dependencies
• No rollback plan
• Insufficient testing strategy

Reference Files

• Template: rfcs/0000-template.md
• Contributing guide: CONTRIBUTING.md
• Existing RFCs: rfcs/THV-*.md