Skip to content

Map mcp-go sentinel errors and poll registry version for timely status reports#4317

Merged
yrobla merged 1 commit intomainfrom
issue-4278-v1
Mar 23, 2026
Merged

Map mcp-go sentinel errors and poll registry version for timely status reports#4317
yrobla merged 1 commit intomainfrom
issue-4278-v1

Conversation

@yrobla
Copy link
Contributor

@yrobla yrobla commented Mar 23, 2026

Summary

  • Map transport.ErrUnauthorized to ErrAuthenticationFailed and transport.ErrLegacySSEServer to ErrBackendUnavailable in wrapBackendError, before the string-based fallback. This ensures 401 responses from mcp-go reach health monitoring as BackendUnauthenticated rather than BackendUnhealthy.
  • Add "unauthorized (401)" pattern to IsAuthenticationError to match mcp-go's ErrUnauthorized string format (reversed order vs "401 unauthorized").
  • Poll DynamicRegistry.Version() every 2s in periodicStatusReporting and trigger an immediate status report when the version changes, so backend additions/removals are reflected without waiting for the full 30s interval.

Fixes #4278

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe):

Test plan

  • Unit tests (task test)
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)

Changes

File Change

Does this introduce a user-facing change?

Special notes for reviewers

@yrobla yrobla requested a review from Copilot March 23, 2026 10:36
@github-actions github-actions bot added the size/S Small PR: 100-299 lines changed label Mar 23, 2026
@codecov
Copy link

codecov bot commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.12%. Comparing base (d96dbac) to head (d32dacb).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
pkg/vmcp/server/status_reporting.go 88.23% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4317      +/-   ##
==========================================
+ Coverage   69.09%   69.12%   +0.02%     
==========================================
  Files         477      477              
  Lines       48074    48099      +25     
==========================================
+ Hits        33216    33247      +31     
+ Misses      12276    12268       -8     
- Partials     2582     2584       +2     

☔ 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

This PR improves vMCP health/status accuracy when using mcp-go by mapping specific transport-layer sentinel errors to vmcp sentinel errors and by making status reporting react quickly to dynamic backend registry changes (fixing cases where backends could remain “unhealthy” after startup races or registry removals).

Changes:

  • Map mcp-go transport sentinel errors (ErrUnauthorized, ErrLegacySSEServer) to vmcp sentinel errors in wrapBackendError prior to string-based fallbacks.
  • Expand IsAuthenticationError matching to include mcp-go’s "unauthorized (401)" string format.
  • Add registry-version polling (every 2s) to trigger immediate status reports when dynamic backend registries change, plus new unit tests for the behavior.

Reviewed changes

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

Show a summary per file
File Description
pkg/vmcp/client/client.go Maps mcp-go transport sentinel errors to vmcp sentinel errors before string fallbacks.
pkg/vmcp/client/client_test.go Adds unit tests validating the new wrapBackendError sentinel mappings.
pkg/vmcp/errors.go Extends auth error string detection to match "unauthorized (401)".
pkg/vmcp/health/checker_test.go Adds test coverage for the new auth error string format and health categorization expectations.
pkg/vmcp/server/status_reporting.go Polls dynamic registry version to trigger immediate status reports on backend add/remove changes.
pkg/vmcp/server/status_reporting_test.go Adds unit test to ensure version changes trigger a status report without waiting for the full interval.

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

…s reports

- Map transport.ErrUnauthorized to ErrAuthenticationFailed and
  transport.ErrLegacySSEServer to ErrBackendUnavailable in wrapBackendError,
  before the string-based fallback. This ensures 401 responses from mcp-go
  reach health monitoring as BackendUnauthenticated rather than BackendUnhealthy.
- Add "unauthorized (401)" pattern to IsAuthenticationError to match
  mcp-go's ErrUnauthorized string format (reversed order vs "401 unauthorized").
- Poll DynamicRegistry.Version() every 2s in periodicStatusReporting and
  trigger an immediate status report when the version changes, so backend
  additions/removals are reflected without waiting for the full 30s interval.

Related-to: #4278
@github-actions github-actions bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Mar 23, 2026
@yrobla yrobla requested a review from Copilot March 23, 2026 11:01
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 6 out of 6 changed files in this pull request and generated 1 comment.


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

Copy link
Contributor

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

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

Code review comment on version-polling interval configuration.

Copy link
Contributor

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

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

that was really just a nit

@yrobla yrobla merged commit e561e9c into main Mar 23, 2026
50 of 51 checks passed
@yrobla yrobla deleted the issue-4278-v1 branch March 23, 2026 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vMCP health monitor permanently marks backends unhealthy after startup race

4 participants