Skip to content

Conversation

@yrobla
Copy link
Contributor

@yrobla yrobla commented Jan 2, 2026

Add configurable partial failure mode to control Layer 1 (discovery) health filtering behavior. The mode determines which backend health statuses are included in capability aggregation.

Mode behaviors:

  • "fail" (strict, default): Includes Healthy & Unknown; excludes Degraded, Unhealthy, and Unauthenticated backends
  • "best_effort" (lenient): Includes Healthy, Degraded & Unknown; excludes only Unhealthy and Unauthenticated backends

Add configurable partial failure mode to control Layer 1 (discovery) health
filtering behavior. The mode determines which backend health statuses are
included in capability aggregation.

Mode behaviors:
- "fail" (strict, default): Includes Healthy & Unknown; excludes Degraded,
  Unhealthy, and Unauthenticated backends
- "best_effort" (lenient): Includes Healthy, Degraded & Unknown; excludes
  only Unhealthy and Unauthenticated backends
@github-actions github-actions bot added the size/L Large PR: 600-999 lines changed label Jan 2, 2026
@yrobla yrobla requested a review from Copilot January 2, 2026 15:35
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 PR implements configurable partial failure mode support for vMCP to control Layer 1 (discovery) health filtering behavior. The feature allows operators to choose between strict ("fail") and lenient ("best_effort") filtering of backend health statuses during capability aggregation.

Key changes:

  • Added PartialFailureMode configuration field with two modes: "fail" (strict, excludes degraded backends) and "best_effort" (lenient, includes degraded backends)
  • Updated health filtering logic to respect the configured mode during discovery phase
  • Extended test coverage with unit tests for mode-aware filtering and E2E tests for both modes

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
pkg/vmcp/health/monitor.go Added IsBackendUsableInMode function to enable mode-aware health status filtering; deprecated IsBackendUsable for backward compatibility
pkg/vmcp/health/monitor_test.go Added comprehensive unit tests for IsBackendUsableInMode covering both modes and edge cases
pkg/vmcp/discovery/health_filter.go Updated FilterHealthyBackends to accept mode parameter and apply mode-specific filtering logic with default to "fail" mode
pkg/vmcp/discovery/health_filter_test.go Added unit tests for mode-aware filtering including fail mode, best_effort mode, default mode, and unknown mode behavior
pkg/vmcp/discovery/middleware.go Updated middleware to accept and pass through the partial failure mode parameter to discovery operations
pkg/vmcp/discovery/middleware_test.go Updated all middleware test cases to include the new mode parameter
pkg/vmcp/server/server.go Added PartialFailureMode field to Server config and struct; integrated mode into middleware initialization
pkg/vmcp/server/adapter/handler_factory.go Updated Layer 2 runtime health checks to use "best_effort" mode (hardcoded)
cmd/vmcp/app/commands.go Added extraction of partial failure mode from config with "fail" as default
test/e2e/thv-operator/virtualmcp/virtualmcp_circuit_breaker_test.go Added E2E test suite for partial failure modes with two vMCP servers configured differently

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

@codecov
Copy link

codecov bot commented Jan 2, 2026

Codecov Report

❌ Patch coverage is 90.76923% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.11%. Comparing base (598e21d) to head (083b1e1).

Files with missing lines Patch % Lines
cmd/vmcp/app/commands.go 0.00% 6 Missing ⚠️
Additional details and impacted files
@@                                       Coverage Diff                                       @@
##           feat/issue-3036-healthcheck-circuitbreaker-remove-unhealthy    #3181      +/-   ##
===============================================================================================
+ Coverage                                                        57.06%   57.11%   +0.04%     
===============================================================================================
  Files                                                              343      343              
  Lines                                                            34434    34471      +37     
===============================================================================================
+ Hits                                                             19650    19688      +38     
+ Misses                                                           13167    13161       -6     
- Partials                                                          1617     1622       +5     

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

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 10 out of 10 changed files in this pull request and generated 5 comments.


💡 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 5, 2026
@yrobla yrobla force-pushed the feat/issue-3036-healthcheck-partial-mode branch from 9e0d072 to 083b1e1 Compare January 5, 2026 09:06
@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 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large PR: 600-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants