Skip to content

Conversation

@yrobla
Copy link
Contributor

@yrobla yrobla commented Jan 30, 2026

Complete the circuit breaker integration by wiring it through the Virtual MCP Server stack for automatic backend failure handling.

Configuration wiring (cmd/vmcp/app/commands.go):

  • Wire circuit breaker config from YAML to health monitor
  • Pass configurable failure threshold and timeout to monitor
  • Log circuit breaker enablement on server startup

Capability filtering (pkg/vmcp/aggregator/default_aggregator.go):

  • Filter tools, resources, and prompts from unhealthy backends
  • Skip capabilities when backend status is Unhealthy, Unknown, or Unauthenticated
  • Keep capabilities from Healthy and Degraded backends (degraded backends are still operational)
  • Add isBackendHealthy helper for status evaluation

Routing protection (pkg/vmcp/router/default_router.go):

  • Check backend health before routing requests
  • Return ErrBackendUnavailable when backend is unhealthy
  • Add isTargetHealthy helper for status evaluation
  • Log warnings when routing fails due to backend unavailability

Testing (pkg/vmcp/router/default_router_test.go):

  • Add HealthStatus to all test fixtures
  • Add test cases for unhealthy backend routing failures
  • Add test cases for unauthenticated backend failures
  • Add test cases for degraded backend success (still operational)
  • Cover all three routing methods: tools, resources, prompts

Related-to: #3036

@github-actions github-actions bot added the size/S Small PR: 100-299 lines changed label Jan 30, 2026
@yrobla yrobla requested a review from ChrisJBurns as a code owner January 30, 2026 10:04
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/S Small PR: 100-299 lines changed labels Jan 30, 2026
@yrobla yrobla requested a review from Copilot January 30, 2026 10:06
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request completes the circuit breaker integration for the Virtual MCP Server by wiring the circuit breaker configuration through the routing and aggregation layers to enable automatic backend failure handling.

Changes:

  • Wire circuit breaker configuration from YAML to health monitor with configurable failure threshold and timeout
  • Filter capabilities (tools, resources, prompts) from unhealthy backends in the aggregator
  • Add health checks before routing requests, returning errors for unavailable backends
  • Add comprehensive test coverage including E2E tests for circuit breaker lifecycle

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
cmd/vmcp/app/commands.go Wires circuit breaker config from YAML to health monitor and adds startup logging
pkg/vmcp/aggregator/default_aggregator.go Filters capabilities from unhealthy backends and adds isBackendHealthy helper
pkg/vmcp/router/default_router.go Checks backend health before routing and adds isTargetHealthy helper
pkg/vmcp/router/default_router_test.go Adds test cases for unhealthy, unauthenticated, and degraded backend scenarios across all routing methods
pkg/vmcp/server/integration_test.go Updates test fixtures to include HealthStatus field
test/e2e/thv-operator/virtualmcp/virtualmcp_circuit_breaker_lifecycle_test.go Adds comprehensive E2E test for complete circuit breaker lifecycle (open → recovery → closed)

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

@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Jan 30, 2026
@yrobla yrobla self-assigned this Jan 30, 2026
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Jan 30, 2026
@codecov
Copy link

codecov bot commented Jan 30, 2026

Codecov Report

❌ Patch coverage is 48.27586% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.40%. Comparing base (3556e04) to head (d5c6c36).

Files with missing lines Patch % Lines
pkg/vmcp/aggregator/default_aggregator.go 47.36% 16 Missing and 4 partials ⚠️
cmd/vmcp/app/commands.go 0.00% 8 Missing ⚠️
pkg/vmcp/types.go 75.00% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##           issue-3036    #3530      +/-   ##
==============================================
- Coverage       65.43%   65.40%   -0.04%     
==============================================
  Files             404      401       -3     
  Lines           39359    39288      -71     
==============================================
- Hits            25755    25695      -60     
+ Misses          11617    11610       -7     
+ Partials         1987     1983       -4     

☔ 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.

@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Jan 30, 2026
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Jan 30, 2026
@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/L Large PR: 600-999 lines changed labels Jan 30, 2026
@yrobla yrobla requested a review from Copilot January 30, 2026 13:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.


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

@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Jan 30, 2026
Complete the circuit breaker integration by wiring it through the Virtual
MCP Server stack for automatic backend failure handling.

Configuration wiring (cmd/vmcp/app/commands.go):
  - Wire circuit breaker config from YAML to health monitor
  - Pass configurable failure threshold and timeout to monitor
  - Log circuit breaker enablement on server startup

Capability filtering (pkg/vmcp/aggregator/default_aggregator.go):
  - Filter tools, resources, and prompts from unhealthy backends
  - Skip capabilities when backend status is Unhealthy, Unknown, or
    Unauthenticated
  - Keep capabilities from Healthy and Degraded backends (degraded
    backends are still operational)
  - Add isBackendHealthy helper for status evaluation

Routing protection (pkg/vmcp/router/default_router.go):
  - Check backend health before routing requests
  - Return ErrBackendUnavailable when backend is unhealthy
  - Add isTargetHealthy helper for status evaluation
  - Log warnings when routing fails due to backend unavailability

Testing (pkg/vmcp/router/default_router_test.go):
  - Add HealthStatus to all test fixtures
  - Add test cases for unhealthy backend routing failures
  - Add test cases for unauthenticated backend failures
  - Add test cases for degraded backend success (still operational)
  - Cover all three routing methods: tools, resources, prompts

Related-to: #3036

Consolidate health check logic into BackendHealthStatus method

Add IsHealthyForRouting() method to BackendHealthStatus type to eliminate
duplicate health checking logic in aggregator and router packages.

Changes:
- pkg/vmcp/types.go: Add IsHealthyForRouting() method
- pkg/vmcp/aggregator/default_aggregator.go: Use method, remove duplicate
- pkg/vmcp/router/default_router.go: Use method, remove duplicate

This consolidation:
- Reduces code duplication (removes 29 lines of duplicate code)
- Centralizes health status logic in one place
- Makes future changes to health checking easier to maintain
- Follows DRY principle for better code organization

All tests pass. Linting clean.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/M Medium PR: 300-599 lines changed labels Jan 30, 2026
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.


This review will be automatically dismissed once you add the justification section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants