Skip to content

Commit 1016f43

Browse files
refactor: extract isHealthy helper pattern for consistent health checks
Add centralized helper functions for checking server health status: Go: - Add IsHealthy() to internal/health/constants.go for core server use - Add isServerHealthy() to tray adapter for local HealthStatus type - Update tray adapter to use helper instead of duplicated inline logic TypeScript: - Create frontend/src/utils/health.ts with isServerConnected() helper - Add HealthLevel, AdminState, HealthAction constants matching backend - Update stores/servers.ts to import from centralized utility Benefits: - Single source of truth for health check logic per language - Consistent fallback behavior when health is nil/undefined - Easier maintenance when updating health check algorithm - Better testability with isolated helper functions Implements suggestion smart-mcp-proxy#1 from PR smart-mcp-proxy#205 review.
1 parent 7a3b711 commit 1016f43

5 files changed

Lines changed: 204 additions & 24 deletions

File tree

cmd/mcpproxy-tray/internal/api/adapter.go

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,17 @@ import (
1010
internalRuntime "mcpproxy-go/internal/runtime"
1111
)
1212

13+
// isServerHealthy returns true if the server is considered healthy.
14+
// It uses health.level as the source of truth, with a fallback to the legacy
15+
// connected field for backward compatibility when health is nil.
16+
func isServerHealthy(health *HealthStatus, legacyConnected bool) bool {
17+
if health != nil {
18+
return health.Level == "healthy"
19+
}
20+
// Fallback to legacy connected field if health is not available
21+
return legacyConnected
22+
}
23+
1324
// ServerAdapter adapts the API client to the ServerInterface expected by the tray
1425
type ServerAdapter struct {
1526
client *Client
@@ -61,11 +72,7 @@ func (a *ServerAdapter) GetUpstreamStats() map[string]interface{} {
6172
connectedCount := 0
6273
totalTools := 0
6374
for _, server := range servers {
64-
// Spec 013: Use health.level as source of truth
65-
if server.Health != nil && server.Health.Level == "healthy" {
66-
connectedCount++
67-
} else if server.Health == nil && server.Connected {
68-
// Fallback to legacy connected field if health not available
75+
if isServerHealthy(server.Health, server.Connected) {
6976
connectedCount++
7077
}
7178
totalTools += server.ToolCount
@@ -118,11 +125,7 @@ func (a *ServerAdapter) GetStatus() interface{} {
118125

119126
connectedCount := 0
120127
for _, server := range servers {
121-
// Spec 013: Use health.level as source of truth
122-
if server.Health != nil && server.Health.Level == "healthy" {
123-
connectedCount++
124-
} else if server.Health == nil && server.Connected {
125-
// Fallback to legacy connected field if health not available
128+
if isServerHealthy(server.Health, server.Connected) {
126129
connectedCount++
127130
}
128131
}

frontend/src/stores/servers.ts

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,26 +2,13 @@ import { defineStore } from 'pinia'
22
import { ref, computed } from 'vue'
33
import type { Server, LoadingState } from '@/types'
44
import api from '@/services/api'
5+
import { isServerConnected } from '@/utils/health'
56

67
export const useServersStore = defineStore('servers', () => {
78
// State
89
const servers = ref<Server[]>([])
910
const loading = ref<LoadingState>({ loading: false, error: null })
1011

11-
// Helper: Check if a server is considered "connected" using health.level as source of truth
12-
// Falls back to legacy connected field for backward compatibility
13-
function isServerConnected(server: Server): boolean {
14-
// Spec 013: health.level is the single source of truth
15-
if (server.health?.level === 'healthy') {
16-
return true
17-
}
18-
// Fallback to legacy connected field if health is not present
19-
if (!server.health) {
20-
return server.connected
21-
}
22-
return false
23-
}
24-
2512
// Computed
2613
const serverCount = computed(() => ({
2714
total: servers.value.length,

frontend/src/utils/health.ts

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/**
2+
* Health status utility functions
3+
* Extracted for consistent health checks across the application
4+
*/
5+
6+
import type { HealthStatus, Server } from '@/types'
7+
8+
/**
9+
* Health level constants matching Go backend (internal/health/constants.go)
10+
*/
11+
export const HealthLevel = {
12+
Healthy: 'healthy',
13+
Degraded: 'degraded',
14+
Unhealthy: 'unhealthy',
15+
} as const
16+
17+
/**
18+
* Admin state constants matching Go backend (internal/health/constants.go)
19+
*/
20+
export const AdminState = {
21+
Enabled: 'enabled',
22+
Disabled: 'disabled',
23+
Quarantined: 'quarantined',
24+
} as const
25+
26+
/**
27+
* Action constants matching Go backend (internal/health/constants.go)
28+
*/
29+
export const HealthAction = {
30+
None: '',
31+
Login: 'login',
32+
Restart: 'restart',
33+
Enable: 'enable',
34+
Approve: 'approve',
35+
ViewLogs: 'view_logs',
36+
SetSecret: 'set_secret',
37+
Configure: 'configure',
38+
} as const
39+
40+
/**
41+
* Check if a health status indicates a healthy server.
42+
* Uses health.level as the source of truth.
43+
*
44+
* @param health - The health status object (may be undefined)
45+
* @param legacyConnected - Fallback value when health is not available
46+
* @returns true if the server is considered healthy
47+
*/
48+
export function isHealthy(health: HealthStatus | undefined, legacyConnected: boolean): boolean {
49+
if (health) {
50+
return health.level === HealthLevel.Healthy
51+
}
52+
// Fallback to legacy connected field if health is not available
53+
return legacyConnected
54+
}
55+
56+
/**
57+
* Check if a server is considered "connected" using health.level as source of truth.
58+
* Falls back to legacy connected field for backward compatibility.
59+
*
60+
* This is the canonical function to use when determining if a server is operational.
61+
*
62+
* @param server - The server object
63+
* @returns true if the server is connected/healthy
64+
*/
65+
export function isServerConnected(server: Server): boolean {
66+
return isHealthy(server.health, server.connected)
67+
}
68+
69+
/**
70+
* Get the appropriate badge class for a health level
71+
*
72+
* @param level - The health level
73+
* @returns DaisyUI badge class
74+
*/
75+
export function getHealthBadgeClass(level: string): string {
76+
switch (level) {
77+
case HealthLevel.Healthy:
78+
return 'badge-success'
79+
case HealthLevel.Degraded:
80+
return 'badge-warning'
81+
case HealthLevel.Unhealthy:
82+
return 'badge-error'
83+
default:
84+
return 'badge-ghost'
85+
}
86+
}
87+
88+
/**
89+
* Get the appropriate badge class for an admin state
90+
*
91+
* @param state - The admin state
92+
* @returns DaisyUI badge class
93+
*/
94+
export function getAdminStateBadgeClass(state: string): string {
95+
switch (state) {
96+
case AdminState.Enabled:
97+
return 'badge-success'
98+
case AdminState.Disabled:
99+
return 'badge-ghost'
100+
case AdminState.Quarantined:
101+
return 'badge-warning'
102+
default:
103+
return 'badge-ghost'
104+
}
105+
}

internal/health/constants.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Package health provides unified health status calculation for upstream MCP servers.
22
package health
33

4+
import "mcpproxy-go/internal/contracts"
5+
46
// Health levels
57
const (
68
LevelHealthy = "healthy"
@@ -26,3 +28,14 @@ const (
2628
ActionSetSecret = "set_secret"
2729
ActionConfigure = "configure"
2830
)
31+
32+
// IsHealthy returns true if the server is considered healthy.
33+
// It uses health.level as the source of truth, with a fallback to the legacy
34+
// connected field for backward compatibility when health is nil.
35+
func IsHealthy(health *contracts.HealthStatus, legacyConnected bool) bool {
36+
if health != nil {
37+
return health.Level == LevelHealthy
38+
}
39+
// Fallback to legacy connected field if health is not available
40+
return legacyConnected
41+
}

internal/health/helpers_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package health
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
8+
"mcpproxy-go/internal/contracts"
9+
)
10+
11+
func TestIsHealthy_WithHealthyLevel(t *testing.T) {
12+
health := &contracts.HealthStatus{
13+
Level: LevelHealthy,
14+
AdminState: StateEnabled,
15+
Summary: "Connected (5 tools)",
16+
}
17+
18+
assert.True(t, IsHealthy(health, false), "should return true when health.level is healthy")
19+
assert.True(t, IsHealthy(health, true), "should return true when health.level is healthy, ignoring legacy")
20+
}
21+
22+
func TestIsHealthy_WithDegradedLevel(t *testing.T) {
23+
health := &contracts.HealthStatus{
24+
Level: LevelDegraded,
25+
AdminState: StateEnabled,
26+
Summary: "Token expiring soon",
27+
}
28+
29+
assert.False(t, IsHealthy(health, false), "should return false when health.level is degraded")
30+
assert.False(t, IsHealthy(health, true), "should return false when health.level is degraded, ignoring legacy")
31+
}
32+
33+
func TestIsHealthy_WithUnhealthyLevel(t *testing.T) {
34+
health := &contracts.HealthStatus{
35+
Level: LevelUnhealthy,
36+
AdminState: StateEnabled,
37+
Summary: "Connection error",
38+
}
39+
40+
assert.False(t, IsHealthy(health, false), "should return false when health.level is unhealthy")
41+
assert.False(t, IsHealthy(health, true), "should return false when health.level is unhealthy, ignoring legacy")
42+
}
43+
44+
func TestIsHealthy_NilHealthFallsBackToLegacy(t *testing.T) {
45+
// When health is nil, should fall back to legacy connected field
46+
assert.True(t, IsHealthy(nil, true), "should return legacy connected value when health is nil")
47+
assert.False(t, IsHealthy(nil, false), "should return legacy connected value when health is nil")
48+
}
49+
50+
func TestIsHealthy_DisabledServerIsHealthy(t *testing.T) {
51+
// Disabled servers are intentionally not running, so they're considered "healthy"
52+
// (admin made a deliberate choice)
53+
health := &contracts.HealthStatus{
54+
Level: LevelHealthy,
55+
AdminState: StateDisabled,
56+
Summary: "Disabled",
57+
}
58+
59+
assert.True(t, IsHealthy(health, false), "disabled servers should be considered healthy")
60+
}
61+
62+
func TestIsHealthy_QuarantinedServerIsHealthy(t *testing.T) {
63+
// Quarantined servers are intentionally blocked, so they're considered "healthy"
64+
// (admin made a deliberate choice)
65+
health := &contracts.HealthStatus{
66+
Level: LevelHealthy,
67+
AdminState: StateQuarantined,
68+
Summary: "Quarantined for review",
69+
}
70+
71+
assert.True(t, IsHealthy(health, false), "quarantined servers should be considered healthy")
72+
}

0 commit comments

Comments
 (0)