Move network diagnostics from MCP tools to /debug REST endpoints + sam-node debug CLI - #330
Conversation
There was a problem hiding this comment.
Code Review
This pull request migrates operator diagnostics and debug utilities (such as mesh info, connectivity stats, token info, network info, and peer connection) from MCP tools to dedicated HTTP endpoints served over a Unix socket. The corresponding MCP tools have been removed, and integration tests have been updated to use the new REST endpoints. The review feedback highlights several potential nil pointer dereference panics across the new debug handler methods (meshInfo, connectivityStats, tokenInfo, networkInfo, and connectPeer) where n.Host, n.DHT, or n.Store are accessed without prior nil checks.
2de3a7c to
d53f636
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request moves operator diagnostics (such as mesh-info, connectivity, network-info, token-info, and logs) from MCP tools to dedicated /debug HTTP endpoints served over a Unix socket, making them invisible to agents and accessible even when the mesh is unreachable. Feedback focuses on adding defensive checks across the new debug handler methods to prevent potential nil pointer dereferences on uninitialized fields (such as Host, DHT, or Store). Additionally, it is recommended to enforce timeouts on connection contexts to prevent indefinite blocking and to validate that the socket path is not empty before dialing.
|
it needs rebase |
d53f636 to
e35a905
Compare
|
/gemini review |
e35a905 to
42da772
Compare
There was a problem hiding this comment.
Code Review
This pull request moves operator diagnostics (such as logs, connectivity, network/token info, and peer connections) out of the Model Context Protocol (MCP) tools list and into dedicated /debug/ REST endpoints. This prevents agents from seeing these diagnostic tools in their tool list. A new sam-node debug CLI command has been introduced to interact with these endpoints, and tests and documentation have been updated accordingly. Feedback includes pre-allocating the connectedPeers slice in debug_handlers.go to avoid serializing to null when empty, and using a lightweight curlimages/curl container instead of python:3.12 in E2E tests to improve execution speed.
Remove the five operator-diagnostic MCP tools (connect_peer, check_connectivity, get_network_info, get_token_info, get_recent_logs) from the agent-facing MCP server and re-expose them as /debug REST endpoints on the sidecar mux, behind withAuth but not withMeshConnection so they keep answering while the mesh is unreachable. get_mesh_info stays as an MCP tool and gains GET /debug/mesh-info for operators. Add a 'sam-node debug' command group with one subcommand per endpoint, talking to the node over its Unix socket so no token is needed. Fixes google#318
42da772 to
1f984a0
Compare
|
now A2A and we are moving to beta , go go go 💪 |
Fixes #318.
The node's MCP server exposed 16 tools to every connected agent, five of which are operator diagnostics, not mesh usage. This PR removes those five from the agent-facing tool list and re-exposes them as REST endpoints under
/debugon the sidecar mux (following the/debugnaming discussed in the issue), plus asam-node debugcommand group that reaches them over the node's Unix socket.Endpoints
GET /debug/mesh-infoget_mesh_infostays an MCP tool; this is the operator's view of the same data)sam-node debug mesh-infoHost.Network().Peers()(in-memory connection table),DHT.RoutingTable().Size(),RouterPeerID,BoundSocketPathGET /debug/connectivity[?peer_id=…]check_connectivitysam-node debug connectivity [peer-id]Host.Connectand measures latencyrouter_error_msg,ping_latency_ms)GET /debug/network-infoget_network_infosam-node debug network-infoHost.Network().ListenAddresses(),Host.Addrs()GET /debug/token-infoget_token_infosam-node debug token-infoStore.LoadIdentity()/LoadIdentityExpiration()(local store)GET /debug/logsget_recent_logssam-node debug logsPOST /debug/connect-peerconnect_peersam-node debug connect-peer <multiaddr>Host.Connectto the given multiaddr; revoked-peer and banned-peer checks kept from the tool handlerAuth and gating
withAuth(token, allowAuthorizationFallback=true, …)withMeshConnection: four endpoints are pure local reads, and the two that dial take a concrete address and do one bounded attempt whose error is the diagnostic output. Gating them 503s exactly when an operator needs them.withMeshConnectiononly incidentally, because all MCP traffic shares the"/"route on the sidecar mux — diagnostics were unusable on a disconnected node. Moving them to/debugmakes that gating decision explicit.CLI
sam-node debug <subcommand>talks HTTP over the Unix socket (--socket-path, defaulting to<data-dir>/sam.socklikerun), so the socket's0600permissions are the credential and no token appears anywhere. Output is the endpoint's raw JSON, so it composes withjq. Root help grows by one entry regardless of how many diagnostics are added later.