Skip to content

Commit dc6cf94

Browse files
DumbrisDumbris
authored
Merge pull request #332 from smart-mcp-proxy/quarantine-ux-improvements
Improve quarantine UX: auto-approve, dashboard visibility, doctor stats, log noise
2 parents b4df1de + 89e671a commit dc6cf94

14 files changed

Lines changed: 624 additions & 104 deletions

File tree

‎cmd/mcpproxy-tray/internal/api/client.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@ import (
2828
// This matches the contracts.HealthStatus struct from the core.
2929
// Spec 013: Health is the single source of truth for server status.
3030
type HealthStatus struct {
31-
Level string `json:"level"` // "healthy", "degraded", "unhealthy"
32-
AdminState string `json:"admin_state"` // "enabled", "disabled", "quarantined"
33-
Summary string `json:"summary"` // e.g., "Connected (5 tools)"
34-
Detail string `json:"detail,omitempty"` // Optional longer explanation
35-
Action string `json:"action,omitempty"` // "login", "restart", "enable", "approve", "set_secret", "configure", "view_logs", ""
31+
Level string `json:"level"` // "healthy", "degraded", "unhealthy"
32+
AdminState string `json:"admin_state"` // "enabled", "disabled", "quarantined"
33+
Summary string `json:"summary"` // e.g., "Connected (5 tools)"
34+
Detail string `json:"detail,omitempty"` // Optional longer explanation
35+
Action string `json:"action,omitempty"` // "login", "restart", "enable", "approve", "set_secret", "configure", "view_logs", ""
3636
}
3737

3838
// Server represents a server from the API
@@ -550,7 +550,7 @@ func (c *Client) GetServers() ([]Server, error) {
550550
"base_url", c.baseURL)
551551
} else if stateChanged {
552552
// Only log when server states actually change
553-
c.logger.Infow("Server state changed",
553+
c.logger.Debugw("Server state changed",
554554
"count", len(result),
555555
"connected", countConnected(result),
556556
"with_health", withHealthCount,

‎cmd/mcpproxy/doctor_cmd.go‎

Lines changed: 118 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,13 @@ func shouldUseDoctorDaemon(dataDir string) bool {
8888
return socket.IsSocketAvailable(socketPath)
8989
}
9090

91+
// quarantineServerStats holds quarantine stats for a single server.
92+
type quarantineServerStats struct {
93+
ServerName string
94+
PendingCount int
95+
ChangedCount int
96+
}
97+
9198
func runDoctorClientMode(ctx context.Context, dataDir string, logger *zap.Logger) error {
9299
socketPath := socket.DetectSocketPath(dataDir)
93100
client := cliclient.NewClient(socketPath, logger.Sugar())
@@ -105,10 +112,63 @@ func runDoctorClientMode(ctx context.Context, dataDir string, logger *zap.Logger
105112
return fmt.Errorf("failed to get diagnostics from daemon: %w", err)
106113
}
107114

108-
return outputDiagnostics(diag, info)
115+
// Collect quarantine stats from servers
116+
quarantineStats := collectQuarantineStats(ctx, client, logger)
117+
118+
return outputDiagnostics(diag, info, quarantineStats)
119+
}
120+
121+
// collectQuarantineStats queries each server's tool approvals to find pending tools.
122+
func collectQuarantineStats(ctx context.Context, client *cliclient.Client, logger *zap.Logger) []quarantineServerStats {
123+
servers, err := client.GetServers(ctx)
124+
if err != nil {
125+
logger.Debug("Failed to get servers for quarantine check", zap.Error(err))
126+
return nil
127+
}
128+
129+
var stats []quarantineServerStats
130+
for _, srv := range servers {
131+
name := getStringField(srv, "name")
132+
enabled := getBoolField(srv, "enabled")
133+
if name == "" || !enabled {
134+
continue
135+
}
136+
137+
approvals, err := client.GetToolApprovals(ctx, name)
138+
if err != nil {
139+
logger.Debug("Failed to get tool approvals", zap.String("server", name), zap.Error(err))
140+
continue
141+
}
142+
143+
pending := 0
144+
changed := 0
145+
for _, a := range approvals {
146+
switch a.Status {
147+
case "pending":
148+
pending++
149+
case "changed":
150+
changed++
151+
}
152+
}
153+
154+
if pending > 0 || changed > 0 {
155+
stats = append(stats, quarantineServerStats{
156+
ServerName: name,
157+
PendingCount: pending,
158+
ChangedCount: changed,
159+
})
160+
}
161+
}
162+
163+
// Sort by server name for consistent output
164+
sort.Slice(stats, func(i, j int) bool {
165+
return stats[i].ServerName < stats[j].ServerName
166+
})
167+
168+
return stats
109169
}
110170

111-
func outputDiagnostics(diag map[string]interface{}, info map[string]interface{}) error {
171+
func outputDiagnostics(diag map[string]interface{}, info map[string]interface{}, quarantineStats []quarantineServerStats) error {
112172
switch doctorOutput {
113173
case "json":
114174
// Combine diagnostics with info for JSON output
@@ -118,6 +178,9 @@ func outputDiagnostics(diag map[string]interface{}, info map[string]interface{})
118178
if info != nil {
119179
combined["info"] = info
120180
}
181+
if len(quarantineStats) > 0 {
182+
combined["quarantine"] = quarantineStats
183+
}
121184
output, err := json.MarshalIndent(combined, "", " ")
122185
if err != nil {
123186
return fmt.Errorf("failed to format output: %w", err)
@@ -160,6 +223,9 @@ func outputDiagnostics(diag map[string]interface{}, info map[string]interface{})
160223
fmt.Println("✅ All systems operational! No issues detected.")
161224
fmt.Println()
162225

226+
// Show quarantine stats even when no other issues
227+
displayQuarantineStats(quarantineStats)
228+
163229
// Show deprecated config warnings even when no issues
164230
displayDeprecatedConfigs(diag)
165231

@@ -320,6 +386,9 @@ func outputDiagnostics(diag map[string]interface{}, info map[string]interface{})
320386
fmt.Println()
321387
}
322388

389+
// 5. Tools Pending Quarantine Approval
390+
displayQuarantineStats(quarantineStats)
391+
323392
// Deprecated Configuration warnings
324393
displayDeprecatedConfigs(diag)
325394

@@ -486,3 +555,50 @@ func joinStrings(items []string, sep string) string {
486555
}
487556
return result
488557
}
558+
559+
// displayQuarantineStats shows tools pending quarantine approval in the doctor output.
560+
func displayQuarantineStats(stats []quarantineServerStats) {
561+
if len(stats) == 0 {
562+
return
563+
}
564+
565+
totalPending := 0
566+
totalChanged := 0
567+
for _, s := range stats {
568+
totalPending += s.PendingCount
569+
totalChanged += s.ChangedCount
570+
}
571+
572+
fmt.Println("⚠️ Tools Pending Quarantine Approval")
573+
fmt.Println("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━")
574+
for _, s := range stats {
575+
total := s.PendingCount + s.ChangedCount
576+
detail := ""
577+
if s.PendingCount > 0 && s.ChangedCount > 0 {
578+
detail = fmt.Sprintf(" (%d new, %d changed)", s.PendingCount, s.ChangedCount)
579+
} else if s.ChangedCount > 0 {
580+
detail = " (changed)"
581+
}
582+
fmt.Printf(" %s: %d tool%s pending%s\n", s.ServerName, total, pluralSuffix(total), detail)
583+
}
584+
fmt.Println()
585+
586+
totalTools := totalPending + totalChanged
587+
fmt.Printf(" Total: %d tool%s across %d server%s\n",
588+
totalTools, pluralSuffix(totalTools),
589+
len(stats), pluralSuffix(len(stats)))
590+
fmt.Println()
591+
fmt.Println("💡 Remediation:")
592+
fmt.Println(" • Review and approve tools in Web UI: Server Detail → Tools tab")
593+
fmt.Println(" • Approve via CLI: mcpproxy upstream approve <server-name>")
594+
fmt.Println(" • Inspect tools: mcpproxy upstream inspect <server-name>")
595+
fmt.Println()
596+
}
597+
598+
// pluralSuffix returns "s" if count != 1, "" otherwise.
599+
func pluralSuffix(count int) string {
600+
if count == 1 {
601+
return ""
602+
}
603+
return "s"
604+
}

‎cmd/mcpproxy/doctor_cmd_test.go‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func TestOutputDiagnostics_JSONFormat(t *testing.T) {
3030
defer func() { os.Stdout = oldStdout }()
3131

3232
doctorOutput = "json"
33-
err := outputDiagnostics(diag, nil)
33+
err := outputDiagnostics(diag, nil, nil)
3434

3535
w.Close()
3636
var buf bytes.Buffer
@@ -69,7 +69,7 @@ func TestOutputDiagnostics_PrettyFormat_NoIssues(t *testing.T) {
6969
defer func() { os.Stdout = oldStdout }()
7070

7171
doctorOutput = "pretty"
72-
err := outputDiagnostics(diag, nil)
72+
err := outputDiagnostics(diag, nil, nil)
7373

7474
w.Close()
7575
var buf bytes.Buffer
@@ -111,7 +111,7 @@ func TestOutputDiagnostics_PrettyFormat_WithUpstreamErrors(t *testing.T) {
111111
defer func() { os.Stdout = oldStdout }()
112112

113113
doctorOutput = "pretty"
114-
err := outputDiagnostics(diag, nil)
114+
err := outputDiagnostics(diag, nil, nil)
115115

116116
w.Close()
117117
var buf bytes.Buffer
@@ -167,7 +167,7 @@ func TestOutputDiagnostics_PrettyFormat_WithOAuthRequired(t *testing.T) {
167167
defer func() { os.Stdout = oldStdout }()
168168

169169
doctorOutput = "pretty"
170-
err := outputDiagnostics(diag, nil)
170+
err := outputDiagnostics(diag, nil, nil)
171171

172172
w.Close()
173173
var buf bytes.Buffer
@@ -213,7 +213,7 @@ func TestOutputDiagnostics_PrettyFormat_WithMissingSecrets(t *testing.T) {
213213
defer func() { os.Stdout = oldStdout }()
214214

215215
doctorOutput = "pretty"
216-
err := outputDiagnostics(diag, nil)
216+
err := outputDiagnostics(diag, nil, nil)
217217

218218
w.Close()
219219
var buf bytes.Buffer
@@ -255,7 +255,7 @@ func TestOutputDiagnostics_PrettyFormat_WithRuntimeWarnings(t *testing.T) {
255255
defer func() { os.Stdout = oldStdout }()
256256

257257
doctorOutput = "pretty"
258-
err := outputDiagnostics(diag, nil)
258+
err := outputDiagnostics(diag, nil, nil)
259259

260260
w.Close()
261261
var buf bytes.Buffer
@@ -309,7 +309,7 @@ func TestOutputDiagnostics_PrettyFormat_MultipleIssueTypes(t *testing.T) {
309309
defer func() { os.Stdout = oldStdout }()
310310

311311
doctorOutput = "pretty"
312-
err := outputDiagnostics(diag, nil)
312+
err := outputDiagnostics(diag, nil, nil)
313313

314314
w.Close()
315315
var buf bytes.Buffer
@@ -356,7 +356,7 @@ func TestOutputDiagnostics_PrettyFormat_SingleIssue(t *testing.T) {
356356
defer func() { os.Stdout = oldStdout }()
357357

358358
doctorOutput = "pretty"
359-
err := outputDiagnostics(diag, nil)
359+
err := outputDiagnostics(diag, nil, nil)
360360

361361
w.Close()
362362
var buf bytes.Buffer
@@ -386,7 +386,7 @@ func TestOutputDiagnostics_EmptyFormat(t *testing.T) {
386386

387387
// Empty string should default to pretty format
388388
doctorOutput = ""
389-
err := outputDiagnostics(diag, nil)
389+
err := outputDiagnostics(diag, nil, nil)
390390

391391
w.Close()
392392
var buf bytes.Buffer
@@ -551,7 +551,7 @@ func TestOutputDiagnostics_WarningWithoutTitle(t *testing.T) {
551551
defer func() { os.Stdout = oldStdout }()
552552

553553
doctorOutput = "pretty"
554-
err := outputDiagnostics(diag, nil)
554+
err := outputDiagnostics(diag, nil, nil)
555555

556556
w.Close()
557557
var buf bytes.Buffer
@@ -586,7 +586,7 @@ func TestOutputDiagnostics_HighSeverityWarning(t *testing.T) {
586586
defer func() { os.Stdout = oldStdout }()
587587

588588
doctorOutput = "pretty"
589-
err := outputDiagnostics(diag, nil)
589+
err := outputDiagnostics(diag, nil, nil)
590590

591591
w.Close()
592592
var buf bytes.Buffer
@@ -624,7 +624,7 @@ func TestOutputDiagnostics_SecretWithoutOptionalFields(t *testing.T) {
624624
defer func() { os.Stdout = oldStdout }()
625625

626626
doctorOutput = "pretty"
627-
err := outputDiagnostics(diag, nil)
627+
err := outputDiagnostics(diag, nil, nil)
628628

629629
w.Close()
630630
var buf bytes.Buffer
@@ -665,7 +665,7 @@ func TestOutputDiagnostics_MissingSecretsRealJSON(t *testing.T) {
665665
defer func() { os.Stdout = oldStdout }()
666666

667667
doctorOutput = "pretty"
668-
err := outputDiagnostics(diag, nil)
668+
err := outputDiagnostics(diag, nil, nil)
669669

670670
w.Close()
671671
var buf bytes.Buffer

0 commit comments

Comments
 (0)