mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-08-22 08:23:53 +00:00
Duy /zuey/andGitHub
bc3bc25c98
* feat(mcp): filter tools at registration + detect FastMCP session reset with force-reconnect Two foundational MCP reliability improvements: (1) Tool allow/deny filtering at BridgeTool registration: Previously the runtime grant-check at execute time surfaced "grant revoked" errors when the LLM called a tool it wasn't allowed to call. Filter upfront in both the per-agent registration path (manager_connect.go) and per-user registration path (loop_mcp_user.go). Adds tool_filter.go with IsToolAllowed + tests. This eliminates the "registered then runtime-denied" loop. (2) FastMCP session reset detection + force-reconnect: FastMCP/Python mcp servers reject tools/call as "invalid during session initialization" when the upstream session lifecycle resets but our pool still holds the old Mcp-Session-Id. Detector matches three known phrasings (FastMCP, mcp-go, mcp-go transport ErrSessionTerminated) in session_reset.go. On detection, BridgeTool.Execute requests a force-reconnect via atomic CAS dedup so N concurrent failing calls collapse to one reconnect. Health loops skip ping while pending so a server answering ping in "initializing" state cannot clobber connected=true before the fresh Initialize completes. Includes 30s timeout, structured slog telemetry, concurrent CAS dedup test. Files: tool_filter.go + tool_filter_test.go (new), session_reset.go + session_reset_test.go (new), manager_connect.go (connectServer/connectViaPool signatures + registerBridgeTools/registerPoolBridgeTools filter logic + reconnPending skip), manager.go (connectAndFilter + connectServer call signature changes), loop_mcp_user.go (filter-at-register block + WithForceReconnect wiring), pool.go (reconnPending skip), bridge_tool.go (session reset detection + WithForceReconnect callback). * fix(mcp): self-heal grant cache + bypass per-user grant for system/empty userID Two production fixes for "MCP tool: grant revoked" recurring on song-nhi-v2. (1) System-user bypass in ListAccessible: Registration uses LoadForAgent(ctx, agentID, "") while execute uses IsAllowed(ctx, agentID, "system", ...). The LEFT JOIN on mcp_user_grants could match a stale disabled row keyed user_id='system' and silently filter the server out only at execute. Skip the join entirely for synthetic owner identities (userID="" or "system") so registration and execute see the same set. Applied to both PostgreSQL (mcp_servers_access.go) and SQLite (mcp_servers_access.go). (2) Grant-checker no-cache on empty allowByServer: grant_checker.loadEntry now skips the cache write when allowByServer is empty. Without this, a single transient empty result pinned permanent denial until a bus invalidate fired. Re-queries until the empty condition clears, then caches normally. Includes TestStoreGrantChecker_EmptyEntryNotCached.
85 lines
3.1 KiB
Go
85 lines
3.1 KiB
Go
package mcp
|
|
|
|
import (
|
|
"context"
|
|
"log/slog"
|
|
"strings"
|
|
"time"
|
|
)
|
|
|
|
// forceReconnectTimeout bounds the synchronous Initialize during a
|
|
// session-reset recovery so a slow server cannot wedge the goroutine.
|
|
const forceReconnectTimeout = 30 * time.Second
|
|
|
|
// isSessionUninitializedErr detects server-side responses that indicate the
|
|
// MCP session lifecycle was reset and the cached client must re-Initialize
|
|
// before further tool calls succeed.
|
|
//
|
|
// Background: FastMCP / mcp-spec stateful HTTP servers track an "initializing
|
|
// → initialized" state per Mcp-Session-Id. When the server restarts, GCs idle
|
|
// sessions, or scales down, the in-memory state vanishes; the client keeps
|
|
// reusing the same SID and the server treats it as a fresh "initializing"
|
|
// session that rejects `tools/call` until `notifications/initialized` arrives.
|
|
//
|
|
// The mcp-go transport only maps HTTP 404 → ErrSessionTerminated. FastMCP-
|
|
// style servers return HTTP 200 with a JSON-RPC error body instead, which
|
|
// surfaces here as a plain Go error. We string-match because there is no
|
|
// dedicated error code in the JSON-RPC spec for this lifecycle violation.
|
|
//
|
|
// Known phrasings (extend as new servers surface):
|
|
// - "method <X> is invalid during session initialization" (FastMCP / Python mcp)
|
|
// - "session not initialized" (mcp-go server, some Node implementations)
|
|
// - "session terminated" (catch-all for mcp-go ErrSessionTerminated text)
|
|
func isSessionUninitializedErr(err error) bool {
|
|
if err == nil {
|
|
return false
|
|
}
|
|
msg := strings.ToLower(err.Error())
|
|
return strings.Contains(msg, "invalid during session initialization") ||
|
|
strings.Contains(msg, "session not initialized") ||
|
|
strings.Contains(msg, "session terminated")
|
|
}
|
|
|
|
// requestForceReconnect kicks off a fresh Initialize handshake out-of-band
|
|
// because a BridgeTool detected the server lost session state. Concurrent
|
|
// calls are deduped via reconnPending CAS so N failing tool calls in flight
|
|
// trigger exactly one reconnect.
|
|
//
|
|
// Runs asynchronously: the caller (BridgeTool.Execute) returns the original
|
|
// error to the agent loop without waiting. By the next tool call attempt the
|
|
// fresh client will be in place via the atomic clientPtr swap inside
|
|
// fullReconnect.
|
|
func (ss *serverState) requestForceReconnect(reason string) {
|
|
if !ss.reconnPending.CompareAndSwap(false, true) {
|
|
slog.Debug("mcp.session_reset.dedup",
|
|
"server", ss.name, "reason", reason)
|
|
return
|
|
}
|
|
|
|
slog.Warn("mcp.session_reset.detected",
|
|
"server", ss.name,
|
|
"transport", ss.transport,
|
|
"reason", reason,
|
|
"action", "force_reconnect")
|
|
|
|
go func() {
|
|
defer ss.reconnPending.Store(false)
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), forceReconnectTimeout)
|
|
defer cancel()
|
|
|
|
ss.connected.Store(false)
|
|
start := time.Now()
|
|
if fullReconnect(ctx, ss) {
|
|
slog.Info("mcp.session_reset.recovered",
|
|
"server", ss.name,
|
|
"latency_ms", time.Since(start).Milliseconds())
|
|
return
|
|
}
|
|
slog.Warn("mcp.session_reset.recovery_failed",
|
|
"server", ss.name,
|
|
"latency_ms", time.Since(start).Milliseconds(),
|
|
"hint", "next health-loop tick will retry via standard reconnectWithBackoff")
|
|
}()
|
|
}
|