From 28e6618c71f4f28c1b715520655b74a79d13b007 Mon Sep 17 00:00:00 2001 From: BaaRai <130491490+BaaRai@users.noreply.github.com> Date: Sun, 11 Oct 2026 22:50:00 +0200 Subject: [PATCH] feat: surface MCP server connect health to operators and agents Every connect attempt (discovery, refresh, and the per-agent-run wiring) now records its outcome on the server row: last_connect_at/ok/err, written through a change-deduped update so per-run wiring does not amplify writes. The MCP page shows a health chip per card (ok with time, or the failure reason), and a new read-only mcp_status tool (bound to auto, worker and pentest) lets a worker notice that a server it expected is down and say so instead of silently degrading while the cached tool count keeps the card looking healthy. Closes #38 --- db/config.go | 18 ++++++- db/schema.sql | 5 ++ server/assembly.go | 3 ++ server/mcp_health_test.go | 65 ++++++++++++++++++++++++++ server/mcpdiscover.go | 3 ++ server/orchestration.go | 7 ++- server/platform_tools.go | 51 ++++++++++++++++++++ web/src/app/(main)/system/mcp/page.tsx | 13 ++++++ web/src/lib/types.ts | 3 ++ 9 files changed, 165 insertions(+), 3 deletions(-) create mode 100644 server/mcp_health_test.go diff --git a/db/config.go b/db/config.go index ba75a7d..43e5262 100644 --- a/db/config.go +++ b/db/config.go @@ -822,11 +822,14 @@ type MCPServer struct { Enabled bool `json:"enabled"` Insecure bool `json:"insecure"` // http: skip TLS cert verification (self-signed servers, issue #108) TimeoutSeconds int `json:"timeout_seconds"` // http: per-request timeout in seconds; 0 = default 120 + LastConnectAt *time.Time `json:"last_connect_at,omitempty"` + LastConnectOK *bool `json:"last_connect_ok,omitempty"` + LastConnectErr *string `json:"last_connect_err,omitempty"` Tools []string `json:"tools,omitempty"` // cached tool names (mcp_tools_cache) } func (d *DB) ListMCP() ([]*MCPServer, error) { - rows, err := d.Query(`SELECT id,name,transport,COALESCE(command,''),args,env,COALESCE(url,''),enabled,insecure,COALESCE(timeout_seconds,120) FROM mcp_servers ORDER BY id`) + rows, err := d.Query(`SELECT id,name,transport,COALESCE(command,''),args,env,COALESCE(url,''),enabled,insecure,COALESCE(timeout_seconds,120),last_connect_at,last_connect_ok,last_connect_err FROM mcp_servers ORDER BY id`) if err != nil { return nil, err } @@ -834,7 +837,7 @@ func (d *DB) ListMCP() ([]*MCPServer, error) { for rows.Next() { var m MCPServer var args, env []byte - if err := rows.Scan(&m.ID, &m.Name, &m.Transport, &m.Command, &args, &env, &m.URL, &m.Enabled, &m.Insecure, &m.TimeoutSeconds); err != nil { + if err := rows.Scan(&m.ID, &m.Name, &m.Transport, &m.Command, &args, &env, &m.URL, &m.Enabled, &m.Insecure, &m.TimeoutSeconds, &m.LastConnectAt, &m.LastConnectOK, &m.LastConnectErr); err != nil { rows.Close() return nil, err } @@ -914,6 +917,17 @@ ON CONFLICT (server_id, tool_name) DO UPDATE SET description=EXCLUDED.descriptio return tx.Commit() } +// UpdateMCPHealth records the outcome of one connect attempt. The WHERE clause +// skips the write when nothing changed, so per-run wiring does not amplify writes. +func (d *DB) UpdateMCPHealth(id int64, ok bool, errMsg string) error { + _, err := d.Exec(`UPDATE mcp_servers + SET last_connect_at = now(), last_connect_ok = $2, last_connect_err = NULLIF($3, '') + WHERE id = $1 + AND (last_connect_ok IS DISTINCT FROM $2 OR last_connect_err IS DISTINCT FROM NULLIF($3, '') OR last_connect_at IS NULL)`, + id, ok, errMsg) + return err +} + func (d *DB) SaveMCP(m *MCPServer) (int64, error) { args, env := string(m.Args), string(m.Env) if args == "" { diff --git a/db/schema.sql b/db/schema.sql index acc2cb1..6938bcb 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -863,6 +863,11 @@ ALTER TABLE mcp_servers ADD COLUMN IF NOT EXISTS insecure BOOLEAN NOT NULL DEFAU -- servers whose tools legitimately run minutes (e.g. binary reverse engineering) need room -- beyond the default without forcing every other server to wait as long. ALTER TABLE mcp_servers ADD COLUMN IF NOT EXISTS timeout_seconds INT NOT NULL DEFAULT 120; +-- Connect health: updated on every connect attempt (discovery, refresh, per-agent-run +-- wiring). NULL = never attempted. The WHERE-deduped update avoids write amplification. +ALTER TABLE mcp_servers ADD COLUMN IF NOT EXISTS last_connect_at TIMESTAMPTZ; +ALTER TABLE mcp_servers ADD COLUMN IF NOT EXISTS last_connect_ok BOOLEAN; +ALTER TABLE mcp_servers ADD COLUMN IF NOT EXISTS last_connect_err TEXT; DROP TRIGGER IF EXISTS trg_mcp_upd ON mcp_servers; CREATE TRIGGER trg_mcp_upd BEFORE UPDATE ON mcp_servers FOR EACH ROW EXECUTE FUNCTION set_updated_at(); diff --git a/server/assembly.go b/server/assembly.go index eda9eb8..c27d548 100644 --- a/server/assembly.go +++ b/server/assembly.go @@ -86,14 +86,17 @@ func wireAgentAugment(pg *db.DB, skillDir string, hostTools func() ([]actool.Cor cl, err := connectMCP(ctx, m) if err != nil { log.Printf("[mcp] %s connect failed: %v", m.Name, err) + _ = pg.UpdateMCPHealth(m.ID, false, "connect: "+err.Error()) continue } closers = append(closers, cl) ts, err := cl.Tools(ctx) if err != nil { log.Printf("[mcp] %s tools/list failed: %v", m.Name, err) + _ = pg.UpdateMCPHealth(m.ID, false, "tools/list: "+err.Error()) continue } + _ = pg.UpdateMCPHealth(m.ID, true, "") for _, t := range ts { extra = append(extra, t) allNames = append(allNames, t.Name()) diff --git a/server/mcp_health_test.go b/server/mcp_health_test.go new file mode 100644 index 0000000..b19be49 --- /dev/null +++ b/server/mcp_health_test.go @@ -0,0 +1,65 @@ +package server + +import ( + "strings" + "testing" + + "github.com/Autumn-27/artex/db" +) + +// The health columns must reflect the outcome of every connect attempt and +// transition cleanly down -> ok, so the UI badge and the mcp_status tool can +// show operators why a server's tools are missing (#38). +func TestMCPHealthStateTransitions(t *testing.T) { + s, _, _ := trafficEvidenceServer(t) + pg := s.m.pg + + row := &db.MCPServer{Name: "health-test", Transport: "http", URL: "http://127.0.0.1:1/mcp", Enabled: true} + id, err := pg.SaveMCP(row) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = pg.DeleteMCP(id) }) + + found := listMCPByName(t, pg, "health-test") + if found.LastConnectAt != nil || found.LastConnectOK != nil || found.LastConnectErr != nil { + t.Fatalf("fresh server should have no health state, got %+v", found) + } + + if err := pg.UpdateMCPHealth(id, false, "connect: no route to host"); err != nil { + t.Fatal(err) + } + found = listMCPByName(t, pg, "health-test") + if found.LastConnectOK == nil || *found.LastConnectOK { + t.Fatal("expected last_connect_ok = false after a failed connect") + } + if found.LastConnectErr == nil || !strings.Contains(*found.LastConnectErr, "no route") { + t.Fatalf("expected the failure reason, got %+v", found.LastConnectErr) + } + + if err := pg.UpdateMCPHealth(id, true, ""); err != nil { + t.Fatal(err) + } + found = listMCPByName(t, pg, "health-test") + if found.LastConnectOK == nil || !*found.LastConnectOK { + t.Fatal("expected last_connect_ok = true after a successful connect") + } + if found.LastConnectErr != nil { + t.Fatalf("expected the error cleared on success, got %q", *found.LastConnectErr) + } +} + +func listMCPByName(t *testing.T, pg *db.DB, name string) *db.MCPServer { + t.Helper() + rows, err := pg.ListMCP() + if err != nil { + t.Fatal(err) + } + for _, r := range rows { + if r.Name == name { + return r + } + } + t.Fatalf("server %s not found", name) + return nil +} diff --git a/server/mcpdiscover.go b/server/mcpdiscover.go index b61545d..fbb5818 100644 --- a/server/mcpdiscover.go +++ b/server/mcpdiscover.go @@ -49,13 +49,16 @@ func connectMCP(ctx context.Context, m *db.MCPServer) (mcpClient, error) { func (s *Server) discoverAndCacheMCP(ctx context.Context, m *db.MCPServer) error { cl, err := connectMCP(ctx, m) if err != nil { + _ = s.m.pg.UpdateMCPHealth(m.ID, false, err.Error()) return err } defer cl.Close() ts, err := cl.Tools(ctx) if err != nil { + _ = s.m.pg.UpdateMCPHealth(m.ID, false, err.Error()) return err } + _ = s.m.pg.UpdateMCPHealth(m.ID, true, "") tools := make([]db.MCPTool, 0, len(ts)) for _, t := range ts { tools = append(tools, db.MCPTool{Name: t.Name(), Description: t.Description()}) diff --git a/server/orchestration.go b/server/orchestration.go index 696b28c..2251a2c 100644 --- a/server/orchestration.go +++ b/server/orchestration.go @@ -484,7 +484,12 @@ func (s *Server) seedOrchestrationTools() { } for _, t := range s.platformTools() { schema, _ := json.Marshal(t.InputSchema()) - _ = s.m.PG().SeedTool(t.Name(), t.Description(), schema, autoAgents) + bindings := autoAgents + if t.Name() == "mcp_status" { + // health visibility is for the executors too, not just the platform agent + bindings = json.RawMessage(`["auto","worker","pentest"]`) + } + _ = s.m.PG().SeedTool(t.Name(), t.Description(), schema, bindings) } s.refreshBuiltinToolSchemas() s.seedAutoDefaultBindings() diff --git a/server/platform_tools.go b/server/platform_tools.go index 4f99dff..f37cef6 100644 --- a/server/platform_tools.go +++ b/server/platform_tools.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "strings" + "time" "github.com/Autumn-27/artex/db" actool "github.com/Autumn-27/norma/tool" @@ -17,6 +18,7 @@ import ( func (s *Server) platformTools() []actool.CoreTool { return []actool.CoreTool{ + s.toolMCPStatus(), s.toolCreateSkill(), s.toolUpdateSkillFile(), s.toolCreateCustomTool(), @@ -243,6 +245,55 @@ func (s *Server) toolUpdateCustomTool() actool.CoreTool { }) } +// ---- MCP health ---- + +// toolMCPStatus reports each MCP server's last connect outcome so a worker can +// notice a missing server (and say so) instead of silently degrading when the +// catalog is cached but the server is unreachable. +func (s *Server) toolMCPStatus() actool.CoreTool { + return wrTool("mcp_status", + "Report the health of every configured MCP server: name, transport, enabled, cached tool count, and the outcome of the last connect attempt (time, ok/error). Use this when tools you expect are missing or before relying on a skill-gated server.", + map[string]any{"type": "object", "properties": map[string]any{}}, + func(ctx context.Context, in json.RawMessage) (actool.Result, error) { + servers, err := s.m.pg.ListMCP() + if err != nil { + return actool.Errorf("Error: " + err.Error()), nil + } + var b strings.Builder + fmt.Fprintf(&b, "%d MCP server(s):\n", len(servers)) + for _, m := range servers { + status := "never connected" + switch { + case m.LastConnectOK != nil && *m.LastConnectOK: + status = "connected ok" + case m.LastConnectOK != nil && !*m.LastConnectOK: + status = "DOWN: " + strings.TrimSpace(coalesceStr(m.LastConnectErr, "connect failed")) + } + fmt.Fprintf(&b, "- %s [%s, %s, %d cached tools]: %s", m.Name, m.Transport, enabledWord(m.Enabled), len(m.Tools), status) + if m.LastConnectAt != nil { + fmt.Fprintf(&b, " (last attempt %s)", m.LastConnectAt.UTC().Format(time.RFC3339)) + } + b.WriteByte('\n') + } + b.WriteString("If a server you need is DOWN, tell the operator in your final report instead of working around it.") + return actool.Text(b.String()), nil + }) +} + +func coalesceStr(p *string, fallback string) string { + if p == nil { + return fallback + } + return *p +} + +func enabledWord(b bool) string { + if b { + return "enabled" + } + return "disabled" +} + // ---- MCP ---- type mcpToolInput struct { diff --git a/web/src/app/(main)/system/mcp/page.tsx b/web/src/app/(main)/system/mcp/page.tsx index 5dce909..69683c5 100644 --- a/web/src/app/(main)/system/mcp/page.tsx +++ b/web/src/app/(main)/system/mcp/page.tsx @@ -404,6 +404,19 @@ export default function MCPPage() { {s.transport} + {s.last_connect_ok === false ? ( + + down: {s.last_connect_err ? s.last_connect_err.slice(0, 60) : "connect failed"} + + ) : s.last_connect_ok === true ? ( + + ok {s.last_connect_at ? new Date(s.last_connect_at).toLocaleTimeString() : ""} + + ) : ( + + not connected yet + + )}
e.stopPropagation()}>