Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 39 additions & 26 deletions pkg/tools/builtin/mcpcatalog/mcpcatalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// streamable-http servers as a single agent-side toolset that supports
// on-demand activation.
//
// The toolset surfaces five meta-tools to the model:
// The toolset surfaces up to five meta-tools to the model:
//
// - search_remote_mcp_servers — case-insensitive fuzzy search over the
// curated catalog (id / title / description / category / tags).
Expand All @@ -11,8 +11,10 @@
// (defers the actual TCP connect / OAuth handshake until Tools() is
// next enumerated).
// - disable_remote_mcp_server — stop the toolset and remove its tools.
// Only exposed once at least one server is enabled.
// - reset_remote_mcp_server_auth — drop persisted OAuth credentials so
// the next enable triggers a fresh authorization flow.
// the next enable triggers a fresh authorization flow. Only exposed
// once at least one server is enabled.
//
// Activated servers' tools are merged into Tools(); tool list changes are
// reported via a tools.ChangeNotifier handler so the runtime refreshes
Expand Down Expand Up @@ -148,10 +150,12 @@ Workflow:
refuse the connection.
3. Use the newly activated tools as you would any other.
4. Call ` + ToolNameDisable + ` to remove a server when no longer needed.
This tool only appears once at least one server is enabled.
5. If a previously authorized OAuth server starts rejecting requests
(token revoked, scopes changed, signed in to the wrong account),
call ` + ToolNameResetAuth + ` to wipe the persisted credentials.
The next enable will trigger a fresh authorization URL.
The next enable will trigger a fresh authorization URL. This tool
also only appears once at least one server is enabled.

Prefer enabling only the servers you actually need — every server adds
tools to the prompt and contributes to context usage.`
Expand Down Expand Up @@ -297,29 +301,6 @@ func (t *Toolset) Tools(ctx context.Context) ([]tools.Tool, error) {
Title: "Enable remote MCP server",
},
},
{
Name: ToolNameDisable,
Category: "mcp_catalog",
Description: "Disable a previously enabled remote MCP server, dropping its tools from the active set.",
Parameters: tools.MustSchemaFor[DisableArgs](),
OutputSchema: tools.MustSchemaFor[string](),
Handler: tools.NewHandler(t.handleDisable),
Annotations: tools.ToolAnnotations{
Title: "Disable remote MCP server",
},
},
{
Name: ToolNameResetAuth,
Category: "mcp_catalog",
Description: "Clear persisted OAuth credentials (access token, refresh token, dynamic-client-registration data) for a catalog server. The next enable will trigger a fresh authorization flow. No-op for api_key/none servers.",
Parameters: tools.MustSchemaFor[ResetAuthArgs](),
OutputSchema: tools.MustSchemaFor[string](),
Handler: tools.NewHandler(t.handleResetAuth),
Annotations: tools.ToolAnnotations{
Title: "Reset remote MCP server auth",
DestructiveHint: new(true),
},
},
}

t.mu.RLock()
Expand All @@ -329,6 +310,38 @@ func (t *Toolset) Tools(ctx context.Context) ([]tools.Tool, error) {
}
t.mu.RUnlock()

// disable_remote_mcp_server and reset_remote_mcp_server_auth only make
// sense once at least one server is enabled. Hiding them otherwise keeps
// the meta-tool surface (and the LLM's prompt) minimal until the model
// has actually activated something.
if len(enabled) > 0 {
result = append(result,
tools.Tool{
Name: ToolNameDisable,
Category: "mcp_catalog",
Description: "Disable a previously enabled remote MCP server, dropping its tools from the active set.",
Parameters: tools.MustSchemaFor[DisableArgs](),
OutputSchema: tools.MustSchemaFor[string](),
Handler: tools.NewHandler(t.handleDisable),
Annotations: tools.ToolAnnotations{
Title: "Disable remote MCP server",
},
},
tools.Tool{
Name: ToolNameResetAuth,
Category: "mcp_catalog",
Description: "Clear persisted OAuth credentials (access token, refresh token, dynamic-client-registration data) for a catalog server. The next enable will trigger a fresh authorization flow. No-op for api_key/none servers.",
Parameters: tools.MustSchemaFor[ResetAuthArgs](),
OutputSchema: tools.MustSchemaFor[string](),
Handler: tools.NewHandler(t.handleResetAuth),
Annotations: tools.ToolAnnotations{
Title: "Reset remote MCP server auth",
DestructiveHint: new(true),
},
},
)
}

// Stable iteration order: handleEnable / handleDisable can run between
// Tools() invocations, but for a given snapshot we want a deterministic
// merged list so model-side prompt caches and TUI rendering don't
Expand Down
67 changes: 63 additions & 4 deletions pkg/tools/builtin/mcpcatalog/mcpcatalog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,18 @@ func TestEnableDisableLifecycle(t *testing.T) {
var changes atomic.Int32
ts.SetToolsChangedHandler(func() { changes.Add(1) })

// Before enabling: only meta-tools.
// Before enabling: only the always-on meta-tools. Disable and reset are
// hidden until at least one server is enabled.
toolList, err := ts.Tools(ctx)
require.NoError(t, err)
names := toolNames(toolList)
assert.ElementsMatch(t, []string{
ToolNameSearch, ToolNameList, ToolNameEnable, ToolNameDisable, ToolNameResetAuth,
ToolNameSearch, ToolNameList, ToolNameEnable,
}, names)
assert.NotContains(t, names, ToolNameDisable,
"disable must not be exposed when no server is enabled")
assert.NotContains(t, names, ToolNameResetAuth,
"reset_auth must not be exposed when no server is enabled")

// Enable: a callback should fire and the underlying mcp.Toolset should
// be present in t.enabled. We deliberately do NOT exercise the network
Expand Down Expand Up @@ -499,8 +504,9 @@ func TestToolsExposesEnabledServerTools(t *testing.T) {
require.NoError(t, err)

names := toolNames(toolList)
// Meta-tools are always there.
for _, meta := range []string{ToolNameSearch, ToolNameList, ToolNameEnable, ToolNameDisable} {
// All five meta-tools must be visible once a server is enabled
// (disable / reset_auth are gated on len(enabled) > 0).
for _, meta := range []string{ToolNameSearch, ToolNameList, ToolNameEnable, ToolNameDisable, ToolNameResetAuth} {
assert.Contains(t, names, meta)
}
// And so is the tool exposed by the fake MCP server.
Expand Down Expand Up @@ -688,6 +694,59 @@ func TestResetAuthNotifiesEvenWhenKeyringFails(t *testing.T) {
// server requiring OAuth that is probed in a non-interactive context
// must not error out. Tools() returns the meta-surface only and the
// server is silently retried on the next interactive turn.

// TestDisableAndResetAuthGatedOnEnabledServers asserts the meta-surface
// optimisation: disable_remote_mcp_server and reset_remote_mcp_server_auth
// are hidden when no server is enabled (so the LLM sees only the actions
// it can usefully perform), revealed once at least one server is enabled,
// and hidden again after the last server is disabled.
func TestDisableAndResetAuthGatedOnEnabledServers(t *testing.T) {
// Use a local auth-required fake server so the test never touches the
// network and is independent of catalog data. The OAuth path makes
// Tools() swallow the AuthorizationRequired error while keeping the
// entry in t.enabled, which is exactly the state we want to assert
// against.
srv := newAuthRequiredMCPServer(t)
defer srv.Close()

ts := New(stubEnv{vars: map[string]string{}})

const id = "gated-meta-server"
ts.catalog.Servers = append(ts.catalog.Servers, Server{
ID: id, Title: "Gated", URL: srv.URL,
Transport: "streamable-http", Auth: Auth{Type: "oauth"},
})
ts.byID[id] = ts.catalog.Servers[len(ts.catalog.Servers)-1]

ctx := t.Context()
defer func() { require.NoError(t, ts.Stop(ctx)) }()

names := toolNames(mustTools(t, ctx, ts))
assert.ElementsMatch(t, []string{ToolNameSearch, ToolNameList, ToolNameEnable}, names,
"with no server enabled, disable/reset_auth must be hidden")

_, err := ts.handleEnable(ctx, EnableArgs{ID: id})
require.NoError(t, err)

names = toolNames(mustTools(t, ctx, ts))
assert.Contains(t, names, ToolNameDisable, "disable must appear once a server is enabled")
assert.Contains(t, names, ToolNameResetAuth, "reset_auth must appear once a server is enabled")

_, err = ts.handleDisable(ctx, DisableArgs{ID: id})
require.NoError(t, err)

names = toolNames(mustTools(t, ctx, ts))
assert.NotContains(t, names, ToolNameDisable, "disable must be hidden again once no server is enabled")
assert.NotContains(t, names, ToolNameResetAuth, "reset_auth must be hidden again once no server is enabled")
}

func mustTools(t *testing.T, ctx context.Context, ts *Toolset) []tools.Tool {
t.Helper()
list, err := ts.Tools(ctx)
require.NoError(t, err)
return list
}

func TestToolsAuthRequiredIsDeferred(t *testing.T) {
srv := newAuthRequiredMCPServer(t)
defer srv.Close()
Expand Down
Loading