mirror of
https://github.com/BaaRai/ARTEX.git
synced 2026-11-13 11:40:53 +08:00
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
This commit is contained in:
+16
-2
@@ -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 == "" {
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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())
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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()})
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -404,6 +404,19 @@ export default function MCPPage() {
|
||||
<Badge variant="outline" className="uppercase">
|
||||
{s.transport}
|
||||
</Badge>
|
||||
{s.last_connect_ok === false ? (
|
||||
<Badge variant="destructive" className="max-w-55 truncate font-normal" title={s.last_connect_err ?? "connect failed"}>
|
||||
down: {s.last_connect_err ? s.last_connect_err.slice(0, 60) : "connect failed"}
|
||||
</Badge>
|
||||
) : s.last_connect_ok === true ? (
|
||||
<Badge variant="secondary" className="font-normal">
|
||||
ok {s.last_connect_at ? new Date(s.last_connect_at).toLocaleTimeString() : ""}
|
||||
</Badge>
|
||||
) : (
|
||||
<Badge variant="outline" className="text-muted-foreground font-normal">
|
||||
not connected yet
|
||||
</Badge>
|
||||
)}
|
||||
<div className="ml-auto flex items-center gap-2" onClick={(e) => e.stopPropagation()}>
|
||||
<Switch
|
||||
checked={s.enabled}
|
||||
|
||||
@@ -1179,6 +1179,9 @@ export interface MCPServer {
|
||||
enabled: boolean;
|
||||
insecure?: boolean; // http: skip TLS cert verification (self-signed servers)
|
||||
timeout_seconds?: number; // http/sse: per-request timeout in seconds (default 120)
|
||||
last_connect_at?: string | null; // last connect attempt (ISO time)
|
||||
last_connect_ok?: boolean | null; // outcome of the last connect attempt
|
||||
last_connect_err?: string | null; // failure reason when last_connect_ok is false
|
||||
tools?: string[]; // mcp_tools_cache (names only, for the count)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user