From 11c18e8956681d98846a4bb3bf34e1260fbe8089 Mon Sep 17 00:00:00 2001 From: dtoro Date: Sat, 11 Jul 2026 20:30:07 +0200 Subject: [PATCH] perf(agent): cache the MCP tool list per client (F1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix F1 of plans/2026-07-11-nomos-agent-code-review.md, the last item. buildTools called listToolsFull (a tools/list MCP round-trip) at the start of EVERY chat turn, including every auto-continuation resume — the tool list is static for the lifetime of one MCP connection, changing only when the api process re-registers tools (a restart, which this client already detects and reacts to via reconnectLocked). Re-fetching it every single turn was avoidable network+parsing work on the hot path. mcpClient now caches the parsed tool list after its first fetch, guarded by its own mutex (kept separate from the request-serializing mu so a cache check never contends with an in-flight doRequest call). reconnectLocked clears the cache — an api restart may have changed what's registered, so a stale cache would be wrong, not just slow. fleetSnapshot's get_health_summary call is deliberately left uncached — it's meant to be "as of now." Since each session gets its own client (the per-session pool from the concurrency work), this caches per-task-conversation rather than globally: a task's FIRST turn still pays the round-trip, every turn after reuses the cached list — which is exactly the case that mattered (long-running, heavily-autonomous tasks with many auto-continuation resumes). Verified live via the api's request log: a brand-new session's first turn made 3 MCP calls (initialize, tools/list, get_health_summary); a second turn on the SAME session made exactly 1 (only get_health_summary) — tools/list correctly skipped. This completes the implementation order in plans/2026-07-11-nomos-agent-code-review.md — every A/B/D/E/F finding from the review (excluding C1, explicitly deferred per operator instruction) is now fixed, tested, and verified live. Co-Authored-By: Claude Opus 4.8 --- cmd/nomos/agent.go | 20 ++++++++++++++++++++ cmd/nomos/main.go | 18 ++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/cmd/nomos/agent.go b/cmd/nomos/agent.go index 0d69785..aeb4d7f 100644 --- a/cmd/nomos/agent.go +++ b/cmd/nomos/agent.go @@ -721,7 +721,23 @@ func (a *agent) buildTools(sessionID string) ([]openai.ChatCompletionToolParam, return tools, nil } +// listToolsFull returns the MCP server's tool list, cached on this client +// after the first call (see mcpClient.toolsCache). Fix F1 of +// plans/2026-07-11-nomos-agent-code-review.md: buildTools calls this at the +// start of every chat turn, including every auto-continuation resume — the +// tool list is static for the lifetime of one MCP connection, so re-fetching +// it every single time was avoidable network+parsing work on the hot path. +// Cache invalidates on reconnectLocked (an api restart may change what's +// registered). func (c *mcpClient) listToolsFull() ([]toolDef, error) { + c.toolsMu.Lock() + if c.toolsCache != nil { + cached := c.toolsCache + c.toolsMu.Unlock() + return cached, nil + } + c.toolsMu.Unlock() + resp, err := c.doRequest("tools/list", map[string]any{}) if err != nil { return nil, err @@ -744,5 +760,9 @@ func (c *mcpClient) listToolsFull() ([]toolDef, error) { InputSchema: t.InputSchema, } } + + c.toolsMu.Lock() + c.toolsCache = out + c.toolsMu.Unlock() return out, nil } diff --git a/cmd/nomos/main.go b/cmd/nomos/main.go index 0982544..63e0b48 100644 --- a/cmd/nomos/main.go +++ b/cmd/nomos/main.go @@ -481,6 +481,18 @@ type mcpClient struct { http *http.Client nextID int mu sync.Mutex // this client is one stateful MCP session; serialize ITS OWN calls + + // toolsCache holds the last tools/list result. The tool list is static + // for the lifetime of one MCP connection — it only changes when the api + // process (re)registers tools, i.e. on a restart, which this client + // already detects and reacts to via reconnectLocked. Without this, + // buildTools (called at the start of EVERY chat turn, including every + // auto-continuation resume) paid a full tools/list round-trip every + // single time for a list that's almost always identical to the last one. + // Guarded separately from mu (not reused) so a cache check never + // contends with an in-flight doRequest call for a different method. + toolsMu sync.Mutex + toolsCache []toolDef } func newMCPClient(baseURL string) (*mcpClient, error) { @@ -540,6 +552,12 @@ func (c *mcpClient) doRequest(method string, params map[string]any) (*mcpJSONRPC // reconnectLocked re-initializes the MCP session. The caller must hold c.mu. func (c *mcpClient) reconnectLocked() error { c.sessionID = "" + // A reconnect means the api process was restarted (or forgot us) — its + // tool registration may have changed, so the cached list is no longer + // trustworthy. + c.toolsMu.Lock() + c.toolsCache = nil + c.toolsMu.Unlock() resp, err := c.send("initialize", map[string]any{ "protocolVersion": "2024-11-05", "capabilities": map[string]any{},