refactor(mcp): clarify MCP App tool classification (#2046)

Address review feedback on the MCP UI widgets toolset code:

- Name the previously untyped map[string]bool returned by CreateToolsets as
  MCPAppToolNames, with a doc comment describing what it stores (peterj).
- Replace the isMCPAppOnlyTool/isMCPAppTool booleans with a single mcpToolKind
  classifier (agent / app / app_only) so call sites read clearly and new tool
  kinds can be added later without reworking signatures (peterj).

Behavior is unchanged; registry tests updated to cover mcpToolKindOf.

Signed-off-by: Dmytro Rashko <dmitriy.rashko@amdocs.com>

Author: dimetron

go/adk/pkg/mcp/registry.go

@@ -78,9 +78,18 @@ type mcpServerParams struct {
 	TLSDisableSystemCAs   *bool
 }
 
+// MCPAppToolNames is the set of MCP tool names whose results render as
+// interactive MCP App (UI) widgets in the chat (the tool declares a
+// ui.resourceUri and is visible to the agent). It is used as a set, so the bool
+// value is always true and only key presence is meaningful. The agent attaches
+// the model-result compaction callback (see agent.MakeMCPAppModelResultCallback)
+// only to these tools.
+type MCPAppToolNames map[string]bool
+
 // CreateToolsets creates toolsets from all configured HTTP and SSE MCP servers,
-// returning the accumulated toolsets. Errors on individual servers are logged
-// and skipped.
+// returning the accumulated toolsets and the set of agent-visible tools that
+// render as MCP App widgets (see MCPAppToolNames). Errors on individual servers
+// are logged and skipped.
 //
 // When propagateToken is true, Authorization is forwarded to every MCP server
 // independently of AllowedHeaders, mirroring the Python ADKTokenPropagationPlugin
@@ -94,10 +103,10 @@ func CreateToolsets(
 	sseTools []adk.SseMcpServerConfig,
 	propagateToken bool,
 	headerProvider DynamicHeaderProvider,
-) ([]tool.Toolset, map[string]bool) {
+) ([]tool.Toolset, MCPAppToolNames) {
 	log := logr.FromContextOrDiscard(ctx)
 	var toolsets []tool.Toolset
-	appToolNames := make(map[string]bool)
+	appToolNames := make(MCPAppToolNames)
 
 	log.Info("Processing HTTP MCP tools", "httpToolsCount", len(httpTools))
 	for i, httpTool := range httpTools {
@@ -153,7 +162,7 @@ func CreateToolsets(
 }
 
 // addToolset logs, initializes, and returns a single MCP toolset.
-func addToolset(ctx context.Context, log logr.Logger, params mcpServerParams, tools []string, label string, index int) (tool.Toolset, map[string]bool, error) {
+func addToolset(ctx context.Context, log logr.Logger, params mcpServerParams, tools []string, label string, index int) (tool.Toolset, MCPAppToolNames, error) {
 	if params.Headers == nil {
 		params.Headers = make(map[string]string)
 	}
@@ -263,10 +272,53 @@ func createTransport(ctx context.Context, params mcpServerParams) (mcpsdk.Transp
 	return mcpTransport, nil
 }
 
-// isMCPAppOnlyTool reports whether a tool is callable only from the rendered
-// MCP app (hidden from the agent). A tool is app-only when its visibility
-// declares "app" but not "model". Absent visibility defaults to agent-visible.
-func isMCPAppOnlyTool(meta map[string]any) bool {
+// mcpToolKind classifies how an MCP tool is surfaced to the agent and the UI,
+// derived from the tool's MCP-UI metadata (the "ui" block). Modeling this as a
+// single kind rather than a set of overlapping booleans keeps the call sites
+// readable and leaves room for additional kinds (e.g. new tool surfaces) without
+// reworking signatures.
+type mcpToolKind int
+
+const (
+	// mcpToolKindAgent is a regular tool exposed to the agent/model with no
+	// interactive UI rendering.
+	mcpToolKindAgent mcpToolKind = iota
+	// mcpToolKindApp is an agent-visible tool whose result renders as an
+	// interactive MCP App (UI) widget in the chat (declares a ui.resourceUri).
+	mcpToolKindApp
+	// mcpToolKindAppOnly is hidden from the agent and only callable from within
+	// the rendered MCP App (visibility declares "app" but not "model").
+	mcpToolKindAppOnly
+)
+
+func (k mcpToolKind) String() string {
+	switch k {
+	case mcpToolKindApp:
+		return "app"
+	case mcpToolKindAppOnly:
+		return "app_only"
+	default:
+		return "agent"
+	}
+}
+
+// mcpToolKindOf classifies a tool from its MCP metadata. App-only takes
+// precedence: a tool hidden from the model is never surfaced to the agent even
+// if it also declares a UI resource.
+func mcpToolKindOf(meta map[string]any) mcpToolKind {
+	if isAppOnlyVisibility(meta) {
+		return mcpToolKindAppOnly
+	}
+	if hasUIResource(meta) {
+		return mcpToolKindApp
+	}
+	return mcpToolKindAgent
+}
+
+// isAppOnlyVisibility reports whether a tool's visibility hides it from the
+// agent: it declares "app" but not "model". Absent visibility defaults to
+// agent-visible.
+func isAppOnlyVisibility(meta map[string]any) bool {
 	ui, _ := meta["ui"].(map[string]any)
 	if ui == nil {
 		return false
@@ -283,9 +335,9 @@ func isMCPAppOnlyTool(meta map[string]any) bool {
 	return hasApp
 }
 
-// isMCPAppTool reports whether a tool declares an MCP-UI resourceUri,
-// meaning its result should be rendered as an interactive app in the chat UI.
-func isMCPAppTool(meta map[string]any) bool {
+// hasUIResource reports whether a tool declares an MCP-UI resourceUri, meaning
+// its result should be rendered as an interactive app in the chat UI.
+func hasUIResource(meta map[string]any) bool {
 	if ui, ok := meta["ui"].(map[string]any); ok {
 		if uri, _ := ui["resourceUri"].(string); uri != "" {
 			return true
@@ -318,7 +370,7 @@ func normalizeVisibility(value any) []string {
 // agentVisibleToolFilter lists tools from the MCP server, filters out app-only
 // tools and any not in the configured allow-list, and returns a predicate the
 // toolset can apply plus the set of remaining tools that render as MCP apps.
-func agentVisibleToolFilter(ctx context.Context, params mcpServerParams, configuredFilter map[string]bool) (tool.Predicate, map[string]bool, error) {
+func agentVisibleToolFilter(ctx context.Context, params mcpServerParams, configuredFilter map[string]bool) (tool.Predicate, MCPAppToolNames, error) {
 	mcpTransport, err := createTransport(ctx, params)
 	if err != nil {
 		return nil, nil, fmt.Errorf("failed to create transport for %s: %w", params.URL, err)
@@ -337,20 +389,23 @@ func agentVisibleToolFilter(ctx context.Context, params mcpServerParams, configu
 	}
 
 	allowedTools := make([]string, 0, len(result.Tools))
-	appToolNames := make(map[string]bool)
+	appToolNames := make(MCPAppToolNames)
 	for _, t := range result.Tools {
 		if t == nil || t.Name == "" {
 			continue
 		}
 		if len(configuredFilter) > 0 && !configuredFilter[t.Name] {
 			continue
 		}
-		if isMCPAppOnlyTool(t.Meta) {
+		switch mcpToolKindOf(t.Meta) {
+		case mcpToolKindAppOnly:
+			// Hidden from the agent; only the rendered MCP App calls it.
 			continue
-		}
-		allowedTools = append(allowedTools, t.Name)
-		if isMCPAppTool(t.Meta) {
+		case mcpToolKindApp:
+			allowedTools = append(allowedTools, t.Name)
 			appToolNames[t.Name] = true
+		default: // mcpToolKindAgent
+			allowedTools = append(allowedTools, t.Name)
 		}
 	}
 
@@ -411,7 +466,7 @@ func (rt *headerRoundTripper) RoundTrip(req *http.Request) (*http.Response, erro
 
 // initializeToolSet fetches tools from an MCP server using Google ADK's mcptoolset.
 // Returns the created toolset on success.
-func initializeToolSet(ctx context.Context, params mcpServerParams, toolFilter map[string]bool) (tool.Toolset, map[string]bool, error) {
+func initializeToolSet(ctx context.Context, params mcpServerParams, toolFilter map[string]bool) (tool.Toolset, MCPAppToolNames, error) {
 	mcpTransport, err := createTransport(ctx, params)
 	if err != nil {
 		return nil, nil, fmt.Errorf("failed to create transport for %s: %w", params.URL, err)

go/adk/pkg/mcp/registry_test.go

@@ -177,96 +177,66 @@ func TestAllowedRequestHeaders_EmptyAllowedList(t *testing.T) {
 	}
 }
 
-func TestIsMCPAppOnlyTool(t *testing.T) {
+func TestMCPToolKindOf(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
 		name string
 		meta map[string]any
-		want bool
+		want mcpToolKind
 	}{
 		{
-			name: "app visibility list",
+			name: "app visibility list is app-only",
 			meta: map[string]any{"ui": map[string]any{"visibility": []any{"app"}}},
-			want: true,
+			want: mcpToolKindAppOnly,
 		},
 		{
-			name: "app visibility string",
+			name: "app visibility string is app-only",
 			meta: map[string]any{"ui": map[string]any{"visibility": "app"}},
-			want: true,
+			want: mcpToolKindAppOnly,
 		},
 		{
-			name: "model and app visibility list - not app-only",
-			meta: map[string]any{"ui": map[string]any{"visibility": []any{"model", "app"}}},
-			want: false,
-		},
-		{
-			name: "app and model visibility list - not app-only",
-			meta: map[string]any{"ui": map[string]any{"visibility": []any{"app", "model"}}},
-			want: false,
+			name: "app-only wins over a declared resource uri",
+			meta: map[string]any{"ui": map[string]any{"visibility": []any{"app"}, "resourceUri": "ui://forms/form.html"}},
+			want: mcpToolKindAppOnly,
 		},
 		{
-			name: "model only visibility - not app-only",
-			meta: map[string]any{"ui": map[string]any{"visibility": []any{"model"}}},
-			want: false,
+			name: "model and app visibility without resource is a plain agent tool",
+			meta: map[string]any{"ui": map[string]any{"visibility": []any{"model", "app"}}},
+			want: mcpToolKindAgent,
 		},
 		{
-			name: "assistant visible app tool",
-			meta: map[string]any{"ui": map[string]any{"resourceUri": "ui://forms/form.html"}},
-			want: false,
+			name: "model and app visibility with resource renders as app",
+			meta: map[string]any{"ui": map[string]any{"visibility": []any{"app", "model"}, "resourceUri": "ui://forms/form.html"}},
+			want: mcpToolKindApp,
 		},
 		{
-			name: "plain tool",
-			meta: map[string]any{},
-			want: false,
+			name: "model only visibility is a plain agent tool",
+			meta: map[string]any{"ui": map[string]any{"visibility": []any{"model"}}},
+			want: mcpToolKindAgent,
 		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-			if got := isMCPAppOnlyTool(tt.meta); got != tt.want {
-				t.Fatalf("isMCPAppOnlyTool() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
-func TestIsMCPAppTool(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name string
-		meta map[string]any
-		want bool
-	}{
 		{
-			name: "resource uri in ui object",
+			name: "resource uri in ui object renders as app",
 			meta: map[string]any{"ui": map[string]any{"resourceUri": "ui://forms/form.html"}},
-			want: true,
+			want: mcpToolKindApp,
 		},
 		{
-			name: "legacy resource uri key",
+			name: "legacy resource uri key renders as app",
 			meta: map[string]any{"ui/resourceUri": "ui://forms/form.html"},
-			want: true,
-		},
-		{
-			name: "app only without resource uri",
-			meta: map[string]any{"ui": map[string]any{"visibility": []any{"app"}}},
-			want: false,
+			want: mcpToolKindApp,
 		},
 		{
 			name: "plain tool",
 			meta: map[string]any{},
-			want: false,
+			want: mcpToolKindAgent,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			if got := isMCPAppTool(tt.meta); got != tt.want {
-				t.Fatalf("isMCPAppTool() = %v, want %v", got, tt.want)
+			if got := mcpToolKindOf(tt.meta); got != tt.want {
+				t.Fatalf("mcpToolKindOf() = %v, want %v", got, tt.want)
 			}
 		})
 	}