| name | review-rfc |
| description | Review RFCs for the ToolHive ecosystem. Use when the user wants to review, critique, or provide feedback on an RFC for toolhive, toolhive-studio, toolhive-registry, toolhive-registry-server, toolhive-cloud-ui, or dockyard projects. |
| allowed-tools | Read, Glob, Grep, Bash(git:*), mcp__github__*, mcp__fetch__fetch, WebFetch, Task |
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:
| Document | When to Read |
|---|
00-overview.md | Always - understand platform concepts |
01-deployment-modes.md | RFCs affecting deployment, K8s, or local mode |
02-core-concepts.md | RFCs introducing new concepts or terminology |
03-transport-architecture.md | RFCs affecting MCP transports or proxy |
04-secrets-management.md | RFCs involving secrets or credentials |
05-runconfig-and-permissions.md | RFCs affecting configuration or permissions |
06-registry-system.md | RFCs affecting registry functionality |
07-groups.md | RFCs involving server grouping |
08-workloads-lifecycle.md | RFCs affecting workload management |
09-operator-architecture.md | RFCs affecting K8s operator or CRDs |
10-virtual-mcp-architecture.md | RFCs involving aggregation or virtual MCP |
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
B. Security Review (CRITICAL)
The Security Considerations section MUST address all of these:
| Section | Questions to Verify |
|---|
| Threat Model | Are potential threats identified? Are attacker capabilities considered? |
| Authentication | How does this affect auth? Are new auth requirements clear? |
| Authorization | What permission checks are needed? Any new permission models? |
| Data Security | Is sensitive data identified? Is encryption addressed? |
| Input Validation | What user input is accepted? How is it validated? |
| Secrets Management | Are secrets handled properly? Can they be rotated? |
| Audit and Logging | Are security events logged? Compliance considered? |
| Mitigations | Are concrete mitigations proposed for identified threats? |
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
D. Diagrams and Examples
E. Feasibility and Impact
ToolHive Ecosystem Context
Target Repositories
| Repository | Type | Key Considerations |
|---|
toolhive | Go | Core platform, CLI, operator, proxy, virtual MCP |
toolhive-studio | TypeScript | Desktop UI, Electron app |
toolhive-registry-server | Go | Registry API, MCP Registry spec compliance |
toolhive-registry | Go/JSON | Registry data, server definitions |
toolhive-cloud-ui | TypeScript/Next.js | Cloud UI, OIDC integration |
dockyard | Go | Container packaging, security scanning |
Key Architecture Principles to Verify
- Platform, not runner: Does this enhance the platform abstraction?
- Security by default: Does this maintain or improve security posture?
- Middleware composability: Can this be implemented as middleware if appropriate?
- RunConfig portability: Does this preserve configuration portability?
- 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