Skip to content

Commit 513c06e

Browse files
technicalpicklesclaudeDumbris
authored
feat: Structured Server State - Health as Single Source of Truth (smart-mcp-proxy#205)
* docs: update 013 spec to reflect current implementation state Mark completed items from smart-mcp-proxy#192 (Unified Health Status): - HealthStatus struct and calculator implemented - Health field on Server struct - Frontend HealthStatus interface and component integration - Action buttons working in ServerCard and Dashboard Narrow remaining scope to: - Structured state objects (OAuthState, ConnectionState) - Doctor() aggregation from Health - Dashboard UI consolidation * docs: streamline 013 spec to focus on remaining work Remove completed items (smart-mcp-proxy#192 Unified Health Status): - HealthStatus struct, calculator, and frontend integration - Health field on Server - Action buttons in UI Remaining scope: - OAuthState and ConnectionState structured types - Doctor() aggregation from Health - Dashboard UI consolidation (remove duplicate diagnostics) * docs: redesign 013 spec with Health as single source of truth - Health becomes the single source of truth for per-server issues - Add new Health actions: set_secret, configure - Doctor() aggregates from Health instead of independent detection - UI navigates to fix locations (secrets page, config tab) - Remove OAuthState/ConnectionState structured objects from scope 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * docs: add CLI handling to 013 spec - Add FR-012: upstream list MUST handle new Health actions - Add CLI Hint column to Actions table - Add Phase 4: CLI Updates to plan.md - Add section 8: CLI Updates to quickstart.md * Add tasks * feat: add set_secret and configure health actions Implement Health as single source of truth for server state by adding two new action types that detect and surface configuration issues: - set_secret: Shown when a server references an unresolved secret (e.g., ${env:MISSING_TOKEN}). Detail field contains the secret name. - configure: Shown when OAuth configuration has parameter errors (e.g., missing RFC 8707 resource parameter). Changes: - Add ActionSetSecret/ActionConfigure constants and extraction helpers - Extend HealthCalculatorInput with MissingSecret/OAuthConfigErr fields - Add detection logic in CalculateHealth() with unit tests - Refactor Doctor() to aggregate diagnostics from Health.Action - Add action buttons in ServerCard.vue and Dashboard.vue - Update CLI hints in upstream_cmd.go for new actions - Update TypeScript types and swagger.yaml documentation This ensures CLI (mcpproxy doctor), Web UI, and MCP tools all show consistent health information derived from the same source. * fix: propagate health field to tray for consistent connected count The tray was showing incorrect connected counts (8/15) compared to CLI (13 healthy) because the health field was not being propagated through the API client layer. Changes: - Add HealthStatus struct and Health field to tray API client - Extract health from JSON response in GetServers() - Include health in adapter GetAllServers() output map - Add extractHealthLevel() helper to managers.go - Add debug logging to trace health extraction - Add validation script (scripts/validate-health-api.sh) - Add tests for health propagation and consistency Spec 013: Health is the single source of truth for server status. * fix(frontend): use health.level for connected server count The web UI was showing 8 connected servers instead of 13 because it was using the legacy connected boolean field instead of health.level. Added isServerConnected() helper that uses health.level === healthy as the source of truth (per Spec 013), with fallback to legacy connected field for backward compatibility. This aligns the web UI with the tray and CLI which were fixed in the previous commit. * fix: improve doctor CLI output and health propagation - Fix OAuth display to show server objects with specific auth login hints - Add sortArrayByServerName for consistent doctor output ordering - Fix health extraction to handle both struct pointers and maps - Add health status calculation to GetAllServers() endpoint - Update Action field docs to include set_secret and configure * fix: use correct field names for missing secrets in doctor output Update doctor command to use field names from contracts.MissingSecretInfo: - secret_name instead of name - used_by (array) instead of server Also fix tests to match the actual backend JSON structure. * make swagger * fix(frontend): add Login option to server detail page Actions dropdown Use unified health.action to show Login button consistently with ServerCard.vue, replacing the narrow needsOAuth check that only matched errors containing 'authorization'. * chore(frontend): remove dead needsOAuth computed properties Both ServerCard.vue and ServerDetail.vue now use healthAction from the unified health status. The needsOAuth computed properties were no longer referenced anywhere and can be safely removed. * fix: add build tag to managers_test.go for Linux CI compatibility The test file was missing the same build constraint as managers.go, causing test compilation failures on Linux (ubuntu-latest in CI). The managers.go file has //go:build !nogui && !headless && !linux but managers_test.go had no build tag, leading to undefined: extractHealthLevel errors when running tests on Linux. * chore: remove unused diagnostics functions after Health.Action refactor * 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. * refactor(types): eliminate magic strings for health actions Add health constants generation to cmd/generate-types to create TypeScript types from a single source of truth: - Add HealthLevel, AdminState, and HealthAction const/type exports - Add HealthStatus interface using the typed unions - Fix generate-types output path to frontend/src/types/ - Update Server interface with missing fields (connecting, authenticated, tool_list_token_size, oauth_status, token_expires_at, user_logged_out, health) - Add UpdateInfo and InfoResponse types - Update api.ts to re-export health types from contracts.ts - Add documentation to Go constants noting TypeScript synchronization This prevents drift between Go and TypeScript when adding new health actions and provides compile-time type checking for action strings. * refactor(frontend): consolidate APIResponse type to single source of truth Remove duplicate APIResponse interface from api.ts and re-export it from contracts.ts, which is the generated source of truth for types derived from Go constants. * test(tray): add real unit tests for ServerAdapter with mock client Replace documentation-style tests with 22 actual unit tests that verify adapter behavior through mock dependency injection. Changes include: - Add ClientInterface to enable mock-based testing - Refactor ServerAdapter to depend on interface instead of concrete Client - Add MockClient implementation for controlled test scenarios - Test health data preservation through GetAllServers transformation - Test isServerHealthy helper with all health level and fallback cases - Test GetUpstreamStats correctly uses health.level as source of truth - Test error handling paths for API failures - Add end-to-end health data flow integration test * feat(tray,frontend): use health.action as single source of truth for actions Implements User Stories 4 and 5 from spec 013: Tray (US4 - FR-013 through FR-017): - Remove serverSupportsOAuth() URL heuristic, use health.action instead - Show Login Required for stdio OAuth servers (e.g., npx @anthropic/mcp-gcal) - Add menu items for set_secret and configure health actions - Use health.level for connected count (no legacy fallback) - Use health.detail for tooltip instead of last_error Web UI (US5 - FR-018, FR-019): - Suppress redundant last_error display when health.action conveys the issue - Add shouldShowError computed property - Login/set_secret/configure actions suppress verbose error alert Tests: - TestServerNeedsAction: verifies health.action detection - TestTrayLoginActionForStdioServers: stdio OAuth servers show login - TestConnectedCountHealthLevelOnly: no fallback to legacy connected - TestHealthActionMenuItemSelection: correct menu items per action --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Algis Dumbris <a.dumbris@gmail.com>
1 parent 4f48334 commit 513c06e

38 files changed

Lines changed: 4065 additions & 386 deletions

cmd/generate-types/main.go

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ func main() {
1414
typeDefinitions := generateTypeDefinitions()
1515

1616
// Create output directory if it doesn't exist
17-
outputDir := "web/frontend/src/types"
17+
outputDir := "frontend/src/types"
1818
if err := os.MkdirAll(outputDir, 0755); err != nil {
1919
fmt.Printf("Error creating output directory: %v\n", err)
2020
os.Exit(1)
@@ -46,6 +46,50 @@ func generateTypeDefinitions() string {
4646
error?: string;
4747
}
4848
49+
`)
50+
51+
// Health constants - generated from internal/health/constants.go
52+
// These must stay in sync with the Go constants
53+
sb.WriteString(`// Health constants - generated from internal/health/constants.go
54+
// Health levels
55+
export const HealthLevelHealthy = 'healthy' as const;
56+
export const HealthLevelDegraded = 'degraded' as const;
57+
export const HealthLevelUnhealthy = 'unhealthy' as const;
58+
export type HealthLevel = typeof HealthLevelHealthy | typeof HealthLevelDegraded | typeof HealthLevelUnhealthy;
59+
60+
// Admin states
61+
export const AdminStateEnabled = 'enabled' as const;
62+
export const AdminStateDisabled = 'disabled' as const;
63+
export const AdminStateQuarantined = 'quarantined' as const;
64+
export type AdminState = typeof AdminStateEnabled | typeof AdminStateDisabled | typeof AdminStateQuarantined;
65+
66+
// Health actions - suggested remediation for health issues
67+
export const HealthActionNone = '' as const;
68+
export const HealthActionLogin = 'login' as const;
69+
export const HealthActionRestart = 'restart' as const;
70+
export const HealthActionEnable = 'enable' as const;
71+
export const HealthActionApprove = 'approve' as const;
72+
export const HealthActionViewLogs = 'view_logs' as const;
73+
export const HealthActionSetSecret = 'set_secret' as const;
74+
export const HealthActionConfigure = 'configure' as const;
75+
export type HealthAction =
76+
| typeof HealthActionNone
77+
| typeof HealthActionLogin
78+
| typeof HealthActionRestart
79+
| typeof HealthActionEnable
80+
| typeof HealthActionApprove
81+
| typeof HealthActionViewLogs
82+
| typeof HealthActionSetSecret
83+
| typeof HealthActionConfigure;
84+
85+
export interface HealthStatus {
86+
level: HealthLevel;
87+
admin_state: AdminState;
88+
summary: string;
89+
detail?: string;
90+
action?: HealthAction;
91+
}
92+
4993
`)
5094

5195
// Server types
@@ -63,15 +107,22 @@ func generateTypeDefinitions() string {
63107
enabled: boolean;
64108
quarantined: boolean;
65109
connected: boolean;
110+
connecting?: boolean; // Connection attempt in progress
111+
authenticated?: boolean; // Has stored OAuth token (regardless of validity)
66112
status: string;
67113
last_error?: string;
68114
connected_at?: string; // ISO date string
69115
last_reconnect_at?: string; // ISO date string
70116
reconnect_count: number;
71117
tool_count: number;
118+
tool_list_token_size?: number; // Token size for this server's tools
72119
created: string; // ISO date string
73120
updated: string; // ISO date string
74121
isolation?: IsolationConfig;
122+
oauth_status?: 'authenticated' | 'expired' | 'error' | 'none'; // OAuth authentication status
123+
token_expires_at?: string; // ISO date string when OAuth token expires
124+
user_logged_out?: boolean; // True if user explicitly logged out (prevents auto-reconnection)
125+
health?: HealthStatus; // Unified health status calculated by the backend
75126
}
76127
77128
export interface OAuthConfig {
@@ -307,6 +358,27 @@ export function isAPIError(response: APIResponse): response is APIError {
307358
export function isAPISuccess<T>(response: APIResponse<T>): response is APISuccess<T> {
308359
return response.success;
309360
}
361+
362+
// Update check types (from internal/updatecheck/types.go)
363+
export interface UpdateInfo {
364+
available: boolean;
365+
latest_version?: string;
366+
release_url?: string;
367+
checked_at?: string; // ISO date string
368+
is_prerelease?: boolean;
369+
check_error?: string;
370+
}
371+
372+
export interface InfoResponse {
373+
version: string;
374+
web_ui_url: string;
375+
listen_addr: string;
376+
endpoints: {
377+
http: string;
378+
socket: string;
379+
};
380+
update?: UpdateInfo;
381+
}
310382
`)
311383

312384
return sb.String()

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

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

13+
// ClientInterface defines the methods required by ServerAdapter from the API client.
14+
// This interface allows for testing with mock implementations.
15+
type ClientInterface interface {
16+
GetServers() ([]Server, error)
17+
GetInfo() (map[string]interface{}, error)
18+
EnableServer(serverName string, enabled bool) error
19+
TriggerOAuthLogin(serverName string) error
20+
StatusChannel() <-chan StatusUpdate
21+
}
22+
23+
// isServerHealthy returns true if the server is considered healthy.
24+
// It uses health.level as the source of truth, with a fallback to the legacy
25+
// connected field for backward compatibility when health is nil.
26+
func isServerHealthy(health *HealthStatus, legacyConnected bool) bool {
27+
if health != nil {
28+
return health.Level == "healthy"
29+
}
30+
// Fallback to legacy connected field if health is not available
31+
return legacyConnected
32+
}
33+
1334
// ServerAdapter adapts the API client to the ServerInterface expected by the tray
1435
type ServerAdapter struct {
15-
client *Client
36+
client ClientInterface
1637
}
1738

1839
// NewServerAdapter creates a new server adapter
19-
func NewServerAdapter(client *Client) *ServerAdapter {
40+
func NewServerAdapter(client ClientInterface) *ServerAdapter {
2041
return &ServerAdapter{
2142
client: client,
2243
}
@@ -61,7 +82,7 @@ func (a *ServerAdapter) GetUpstreamStats() map[string]interface{} {
6182
connectedCount := 0
6283
totalTools := 0
6384
for _, server := range servers {
64-
if server.Connected {
85+
if isServerHealthy(server.Health, server.Connected) {
6586
connectedCount++
6687
}
6788
totalTools += server.ToolCount
@@ -114,7 +135,7 @@ func (a *ServerAdapter) GetStatus() interface{} {
114135

115136
connectedCount := 0
116137
for _, server := range servers {
117-
if server.Connected {
138+
if isServerHealthy(server.Health, server.Connected) {
118139
connectedCount++
119140
}
120141
}
@@ -215,7 +236,7 @@ func (a *ServerAdapter) GetAllServers() ([]map[string]interface{}, error) {
215236

216237
var result []map[string]interface{}
217238
for _, server := range servers {
218-
result = append(result, map[string]interface{}{
239+
serverMap := map[string]interface{}{
219240
"name": server.Name,
220241
"url": server.URL,
221242
"command": server.Command,
@@ -230,7 +251,20 @@ func (a *ServerAdapter) GetAllServers() ([]map[string]interface{}, error) {
230251
"should_retry": server.ShouldRetry,
231252
"retry_count": server.RetryCount,
232253
"last_retry_time": server.LastRetry,
233-
})
254+
}
255+
256+
// Spec 013: Include health status as source of truth for connected count
257+
if server.Health != nil {
258+
serverMap["health"] = map[string]interface{}{
259+
"level": server.Health.Level,
260+
"admin_state": server.Health.AdminState,
261+
"summary": server.Health.Summary,
262+
"detail": server.Health.Detail,
263+
"action": server.Health.Action,
264+
}
265+
}
266+
267+
result = append(result, serverMap)
234268
}
235269

236270
return result, nil

0 commit comments

Comments
 (0)