Add Virtual MCP Server routing interfaces and domain model

[JAORMX]

Summary

Implements the routing interfaces and domain model for Virtual MCP Server as defined in the proposal. This PR provides the foundational interfaces needed to enable parallel development of the vMCP implementation.

What does this PR do?

Adds comprehensive interfaces and domain types for the Virtual MCP Server routing layer:

• pkg/vmcp/types.go: Shared domain types (BackendTarget, Tool, Resource, Prompt, RoutingTable)
• pkg/vmcp/errors.go: Common domain errors
• pkg/vmcp/router/: Request routing interfaces (Router, RoutingStrategy, SessionAffinityProvider)
• pkg/vmcp/aggregator/: Capability aggregation interfaces (Aggregator, ConflictResolver, BackendDiscoverer)
• pkg/vmcp/auth/: Authentication interfaces for both incoming (client → vMCP) and outgoing (vMCP → backends)
• pkg/vmcp/composer/: Workflow engine interfaces for composite tools with elicitation support
• pkg/vmcp/config/: Platform-agnostic configuration model (works for both CLI and Kubernetes)
• pkg/vmcp/cache/: Token caching interfaces with pluggable backends

Key Design Decisions

• Platform Independence: All interfaces work for both CLI (Docker/Podman) and Kubernetes deployments
• DDD Architecture: Clear bounded contexts with proper separation of concerns
• Interface Segregation: Small, focused interfaces enable independent implementation
• Extensibility: Plugin architecture for routing strategies, auth strategies, and cache providers
• Type Safety: Strong typing with enums for states, no magic strings

Architecture

Following Domain-Driven Design principles with bounded contexts:

pkg/vmcp/
├── types.go              # Shared domain types (avoid circular deps)
├── errors.go             # Domain errors
├── router/               # Request routing
├── aggregator/           # Capability aggregation  
├── auth/                 # Authentication (2 boundaries)
├── composer/             # Composite tool workflows
├── config/               # Platform-agnostic config
└── cache/                # Token caching

Benefits

• Enables parallel implementation by multiple developers
• Clear contracts between components
• Testable through interface mocking
• Works seamlessly for both CLI and Kubernetes deployments
• Extensible for future enhancements

Testing

This PR contains interface definitions only. Implementations and tests will follow in subsequent PRs as outlined in the implementation plan.

Related Documentation

• Proposal: Add Virtual MCP Server proposal #2106
• MCP Specification: https://modelcontextprotocol.io/specification

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

[Copilot]

Pull Request Overview

This PR establishes the foundational interfaces and domain model for the Virtual MCP Server, enabling parallel development of the vMCP implementation. It provides a platform-agnostic architecture following Domain-Driven Design principles that works seamlessly for both CLI (Docker/Podman) and Kubernetes deployments.

Key Changes:

• Defines comprehensive domain types and error handling for Virtual MCP Server
• Establishes routing, aggregation, authentication, composition, caching, and configuration interfaces
• Creates a clear bounded context architecture with proper separation of concerns

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pkg/vmcp/types.go	Core domain types including BackendTarget, Tool, Resource, Prompt, and routing table structures
pkg/vmcp/errors.go	Common domain errors following DDD principles for use across vmcp subpackages
pkg/vmcp/doc.go	Comprehensive package documentation with architecture overview and usage examples
pkg/vmcp/router/router.go	Request routing interfaces for directing MCP protocol requests to backend workloads
pkg/vmcp/aggregator/aggregator.go	Capability aggregation interfaces for discovering and merging backend capabilities
pkg/vmcp/auth/auth.go	Authentication interfaces for both incoming (client→vMCP) and outgoing (vMCP→backends) boundaries
pkg/vmcp/composer/composer.go	Workflow engine interfaces for executing multi-step composite tools with elicitation support
pkg/vmcp/config/config.go	Platform-agnostic configuration model compatible with both CLI YAML and Kubernetes CRDs
pkg/vmcp/cache/cache.go	Token caching interfaces with pluggable backend support (memory, Redis)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.26%. Comparing base (0c44157) to head (fe78c46).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/vmcp/cache/cache.go	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2333      +/-   ##
==========================================
- Coverage   54.28%   54.26%   -0.02%     
==========================================
  Files         240      241       +1     
  Lines       23594    23598       +4     
==========================================
- Hits        12808    12806       -2     
- Misses       9577     9582       +5     
- Partials     1209     1210       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[JAORMX]

@claude please review this.

[claude]

Claude finished @JAORMX's task —— View job

---

Code Review Complete

I've completed a thorough review of the Virtual MCP Server routing interfaces and domain model PR. Overall, this is an excellent foundation for the vMCP implementation with strong architectural decisions and well-designed interfaces. Below is my detailed feedback.

---

Summary

This PR establishes solid foundational interfaces following Domain-Driven Design principles. The code demonstrates strong architectural thinking with clear separation of concerns, platform independence, and extensibility. There are a few areas for improvement around documentation clarity, error handling consistency, and interface design details.

Verdict: Approve with minor suggestions ✅

---

Strengths

1. Excellent DDD Architecture

• Clear bounded contexts with proper separation (router, aggregator, auth, composer, config, cache)
• Shared types at package root to avoid circular dependencies
• Domain errors centralized in pkg/vmcp/errors.go
• Strong type safety with enums instead of magic strings

2. Platform Independence

• All interfaces work seamlessly for both CLI (Docker/Podman) and Kubernetes deployments
• Configuration model abstracts away platform specifics
• Smart use of groupRef and BackendTarget abstractions

3. Interface Segregation

• Small, focused interfaces that follow ISP (Interface Segregation Principle)
• Easy to mock for testing
• Clear contracts between components
• Enables parallel development by multiple developers

4. Comprehensive Documentation

• Excellent package-level documentation in pkg/vmcp/doc.go with usage examples
• Good inline documentation for interfaces and types
• Clear explanation of design decisions

---

Issues & Suggestions

1. Critical: Documentation Clarification (pkg/vmcp/aggregator/aggregator.go:161)

Issue: The comment on ToolFilter interface references "ToolHive's existing mcp.WithToolsFilter() middleware" but it's unclear whether implementations should:

• Wrap/delegate to the existing middleware, or
• Provide a new interface that will be used by vMCP implementations

Current Code:

// ToolFilter filters tools from a backend based on configuration.
// This reuses ToolHive's existing mcp.WithToolsFilter() middleware.
type ToolFilter interface {
    // FilterTools returns only the tools that should be included.
    FilterTools(ctx context.Context, tools []vmcp.Tool) ([]vmcp.Tool, error)
}

Issue Analysis: After reviewing pkg/mcp/middleware.go and pkg/mcp/tool_filter.go, I found that:

• mcp.WithToolsFilter() is a functional option for creating HTTP middleware (returns ToolMiddlewareOption)
• It operates on HTTP request/response streams, not directly on tool lists
• The vMCP ToolFilter interface operates on in-memory tool lists, not HTTP streams

Recommendation: This comment is misleading. The interfaces serve different purposes:

• Existing: HTTP middleware for proxying (operates on JSON-RPC over HTTP)
• New vMCP: In-memory filtering during aggregation (operates on domain objects)

Suggested Fix (pkg/vmcp/aggregator/aggregator.go:160-165):

// ToolFilter filters tools from a backend based on configuration.
// Implementations apply filtering logic similar to ToolHive's existing 
// mcp.WithToolsFilter() middleware, but operate on in-memory tool lists
// during aggregation rather than HTTP request/response streams.
type ToolFilter interface {
    // FilterTools returns only the tools that should be included based on
    // the configured filter list. Returns a filtered copy of the input.
    FilterTools(ctx context.Context, tools []vmcp.Tool) ([]vmcp.Tool, error)
}

Same Issue Applies to (pkg/vmcp/aggregator/aggregator.go:167-172):

// ToolOverride applies renames and description updates to tools.
// Implementations apply override logic similar to ToolHive's existing
// mcp.WithToolsOverride() middleware, but operate on in-memory tool lists
// during aggregation rather than HTTP request/response streams.
type ToolOverride interface {
    // ApplyOverrides modifies tool names and descriptions according to
    // the configured overrides. Modifies tools in place or returns a new slice.
    ApplyOverrides(ctx context.Context, tools []vmcp.Tool) ([]vmcp.Tool, error)
}

This addresses the Copilot review comment and clarifies the relationship between these interfaces.

---

2. Error Handling Consistency (pkg/vmcp/router/router.go:62-77)

Issue: Router package defines its own specific errors (ErrToolNotFound, etc.) while also importing pkg/vmcp which has ErrNotFound. This creates inconsistency.

Current Code:

// pkg/vmcp/router/router.go
var (
    ErrToolNotFound = fmt.Errorf("tool not found")
    ErrResourceNotFound = fmt.Errorf("resource not found")
    ErrPromptNotFound = fmt.Errorf("prompt not found")
    // ...
)

Recommendation: Consider whether these should wrap vmcp.ErrNotFound for consistency with DDD error handling patterns:

var (
    // ErrToolNotFound indicates the requested tool doesn't exist.
    ErrToolNotFound = fmt.Errorf("%w: tool", vmcp.ErrNotFound)
    
    // ErrResourceNotFound indicates the requested resource doesn't exist.
    ErrResourceNotFound = fmt.Errorf("%w: resource", vmcp.ErrNotFound)
    
    // ErrPromptNotFound indicates the requested prompt doesn't exist.
    ErrPromptNotFound = fmt.Errorf("%w: prompt", vmcp.ErrNotFound)
)

Rationale: This allows callers to use errors.Is(err, vmcp.ErrNotFound) to handle all "not found" cases generically, while still preserving specific error types for detailed handling.

Same pattern could apply to: pkg/vmcp/aggregator/aggregator.go:175-187

---

3. Interface Design: HealthChecker Context (pkg/vmcp/types.go:185-189)

Issue: HealthChecker.CheckHealth() returns (BackendHealthStatus, error) which can be ambiguous:

• What if error is non-nil but status is "healthy"?
• What if both are nil/zero values?

Current Code:

type HealthChecker interface {
    // CheckHealth checks if a backend is healthy and responding.
    // Returns the current health status and any error encountered.
    CheckHealth(ctx context.Context, target *BackendTarget) (BackendHealthStatus, error)
}

Recommendation: Clarify the contract in documentation:

type HealthChecker interface {
    // CheckHealth checks if a backend is healthy and responding.
    // Returns the current health status and any error encountered.
    //
    // Contract:
    //   - If error is non-nil, the status indicates the failure reason
    //     (typically BackendUnhealthy or BackendUnknown)
    //   - If error is nil, the status reflects the actual backend health
    //   - Never returns (BackendHealthy, non-nil error)
    CheckHealth(ctx context.Context, target *BackendTarget) (BackendHealthStatus, error)
}

---

4. Type Safety: BackendTarget.AuthMetadata (pkg/vmcp/types.go:33)

Issue: AuthMetadata map[string]any uses any which loses type safety. Consider defining specific strategy types.

Current Code:

type BackendTarget struct {
    // ...
    AuthMetadata map[string]any
}

Recommendation: This is acceptable for v1 given the extensible strategy pattern, but consider documenting expected shapes:

// AuthMetadata contains strategy-specific authentication metadata.
// This is opaque to the router and interpreted by the authenticator.
//
// Expected structure by strategy:
//   - "token_exchange": {"token_endpoint": string, "audience": string, ...}
//   - "client_credentials": {"client_id": string, "client_secret": string, ...}
//   - "header_injection": {"headers": map[string]string}
//
// See pkg/vmcp/auth package documentation for complete schema per strategy.
AuthMetadata map[string]any

Alternative: Define strategy-specific config types in pkg/vmcp/auth/strategies/ (can be done in implementation PRs).

---

5. Minor: Missing Context on CapabilityList (pkg/vmcp/types.go:213-229)

Issue: CapabilityList includes SupportsLogging and SupportsSampling but doesn't document what these capabilities enable.

Suggestion: Add brief documentation:

type CapabilityList struct {
    // Tools available on this backend.
    Tools []Tool

    // Resources available on this backend.
    Resources []Resource

    // Prompts available on this backend.
    Prompts []Prompt

    // SupportsLogging indicates if the backend supports MCP logging protocol.
    // When true, the backend can receive logging/setLevel requests.
    SupportsLogging bool

    // SupportsSampling indicates if the backend supports MCP sampling protocol.
    // When true, the backend can receive sampling/createMessage requests.
    SupportsSampling bool
}

---

6. Interface Completeness: TokenCache (pkg/vmcp/cache/cache.go:18-34)

Issue: The TokenCache interface is missing a Stats() method even though StatsProvider interface is defined.

Observation:

type TokenCache interface {
    Get(ctx context.Context, key string) (*CachedToken, error)
    Set(ctx context.Context, key string, token *CachedToken) error
    Delete(ctx context.Context, key string) error
    Clear(ctx context.Context) error
    Close() error
}

// StatsProvider is defined separately
type StatsProvider interface {
    Stats(ctx context.Context) (*Stats, error)
}

Recommendation: This is intentional separation (not all caches need stats), but consider documenting:

// TokenCache provides caching for exchanged authentication tokens.
// This reduces the number of token exchanges and improves performance.
//
// Cache key format: {backend}:{hash(subject_token)}:{audience}
// This ensures proper token isolation per (user, backend) pair.
//
// Implementations may optionally implement StatsProvider for observability.
type TokenCache interface {
    // ... methods
}

---

7. Workflow Engine: Template Expansion Security (pkg/vmcp/composer/composer.go:308-315)

Issue: TemplateExpander uses Go's text/template syntax but doesn't mention security considerations.

Concern: Template injection vulnerabilities if user-controlled data flows into templates without proper escaping.

Recommendation: Add security note:

// TemplateExpander handles template expansion for workflow arguments.
//
// Security: Implementations must ensure proper escaping/sanitization of
// user-provided data to prevent template injection attacks. Consider using
// html/template for HTML contexts or implementing custom escape functions.
type TemplateExpander interface {
    // Expand evaluates templates in the given data using the workflow context.
    // Template syntax follows Go text/template with access to workflow context.
    Expand(ctx context.Context, data map[string]any, workflowCtx *WorkflowContext) (map[string]any, error)

    // EvaluateCondition evaluates a condition template to a boolean.
    EvaluateCondition(ctx context.Context, condition string, workflowCtx *WorkflowContext) (bool, error)
}

---

8. Config Validation: Missing Required Fields (pkg/vmcp/config/config.go)

Issue: The Config struct doesn't indicate which fields are required vs optional.

Recommendation: Add struct tags or documentation:

type Config struct {
    // Name is the virtual MCP server name (required).
    Name string

    // GroupRef references the ToolHive group containing backend workloads (required).
    GroupRef string

    // IncomingAuth configures how clients authenticate to the virtual MCP server.
    // If nil, defaults to anonymous authentication.
    IncomingAuth *IncomingAuthConfig

    // OutgoingAuth configures how the virtual MCP server authenticates to backends.
    // If nil, defaults to pass-through authentication.
    OutgoingAuth *OutgoingAuthConfig

    // ... rest with similar documentation
}

---

9. Consistency: BackendTarget vs Backend (pkg/vmcp/types.go:8-88)

Observation: Both BackendTarget (lines 10-44) and Backend (lines 64-88) exist with overlapping fields. This could be confusing.

Analysis: After review, this appears intentional:

• Backend: Discovery phase output (from BackendDiscoverer)
• BackendTarget: Routing phase input (includes additional routing metadata)

Recommendation: Document the distinction clearly:

// Backend represents a discovered backend MCP server workload.
// This is the output of the discovery phase and input to capability querying.
// After aggregation, backends are converted to BackendTargets for routing.
type Backend struct {
    // ... fields
}

// BackendTarget identifies a specific backend workload and provides
// the information needed to forward requests to it.
// This is the output of aggregation and input to the routing phase.
type BackendTarget struct {
    // ... fields
}

---

10. Minor: Code Coverage (Codecov Report)

Observation: Codecov reports 0% coverage on 4 lines in pkg/vmcp/cache/cache.go.

Analysis: These are the helper methods IsExpired() and ShouldRefresh() on CachedToken. This is expected since these are pure interface definitions with only trivial helper methods.

Recommendation: Tests will be added in implementation PRs. Consider adding basic tests for the helper methods to document expected behavior:

// Test in future implementation PR
func TestCachedToken_IsExpired(t *testing.T) {
    token := &CachedToken{ExpiresAt: time.Now().Add(-1 * time.Hour)}
    assert.True(t, token.IsExpired())
}

---

Consistency with ToolHive Patterns

✅ Follows ToolHive Conventions

1. Interface-based design: Matches patterns in pkg/workloads/manager.go, pkg/transport/types/
2. Factory pattern: Ready for runtime-specific implementations (Docker/Kubernetes)
3. Context propagation: All interfaces properly accept context.Context
4. Error handling: Uses sentinel errors with errors.Is() pattern
5. Package organization: Follows Go standard layout
6. Documentation: Comprehensive package and interface docs

✅ Platform Abstraction

• Correctly abstracts Docker/Podman vs Kubernetes differences
• groupRef and BackendTarget.BaseURL are platform-agnostic
• Config model works for both YAML (CLI) and CRDs (K8s)

✅ Integration Points

• Auth interfaces align with existing pkg/auth/ and pkg/authz/ patterns
• Middleware concepts match pkg/mcp/middleware.go (with noted distinction)
• Transport abstraction compatible with pkg/transport/types/

---

Architecture Review Checklist

• Read and understand all 9 new files in pkg/vmcp/
• Review architecture and design patterns
• Check consistency with ToolHive patterns
• Analyze interface design quality
• Review documentation completeness
• Address Copilot's feedback about mcp.WithToolsFilter()
• Provide comprehensive review feedback

---

Final Recommendations

Before Merge

1. ✅ Must Fix: Update documentation for ToolFilter and ToolOverride interfaces (Issue fix(typo): corrects readme #1)
2. 🟡 Should Consider: Add documentation for helper method contracts (Issues Bump golangci/golangci-lint-action from 2f856675483cb8b9378ee77ee0beb67955aca9d7 to 4696ba8babb6127d732c3c6dde519db15edab9ea #3, Implement secret injection #5, Figure out ergonomics for exposing directories #6, Start simple hardcoded registry #8, Use an actual logger #9)

Future PRs

3. 🔵 Nice to Have: Wrap router errors with vmcp.ErrNotFound for consistency (Issue Do we want the container monitor? #2)
4. 🔵 Nice to Have: Add security notes for template expansion (Issue Create proxy subcommand #7)
5. 🔵 Nice to Have: Define strategy-specific config types (Issue Implement secret store #4)

---

Summary

This is an excellent foundation for the Virtual MCP Server implementation. The interfaces are well-designed, the architecture follows solid DDD principles, and the code demonstrates thoughtful consideration of extensibility and platform independence.

The main issue to address before merge is clarifying the relationship between the new ToolFilter/ToolOverride interfaces and the existing MCP middleware (Issue #1). The other suggestions are primarily documentation improvements and can be addressed in follow-up PRs.

Great work on this foundational piece! 🎉

---

[yrobla]

just a minor comment