Skip to content
Draft
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
Original file line number Diff line number Diff line change
Expand Up @@ -909,6 +909,7 @@ func (a *adkApiTranslator) translateMCPServerTarget(ctx context.Context, agent *
}

func (a *adkApiTranslator) translateRemoteMCPServerTarget(ctx context.Context, agent *adk.AgentConfig, remoteMcpServer *v1alpha2.RemoteMCPServer, mcpServerTool *v1alpha2.McpServerTool, agentHeaders map[string]string, proxyURL string) error {
toolNames := resolveToolNames(mcpServerTool.ToolNames, remoteMcpServer.Status.DiscoveredTools)
switch remoteMcpServer.Spec.Protocol {
case v1alpha2.RemoteMCPServerProtocolSse:
tool, err := a.translateSseHttpTool(ctx, remoteMcpServer, agentHeaders, proxyURL)
Expand All @@ -917,7 +918,7 @@ func (a *adkApiTranslator) translateRemoteMCPServerTarget(ctx context.Context, a
}
agent.SseTools = append(agent.SseTools, adk.SseMcpServerConfig{
Params: *tool,
Tools: mcpServerTool.ToolNames,
Tools: toolNames,
AllowedHeaders: mcpServerTool.AllowedHeaders,
RequireApproval: mcpServerTool.RequireApproval,
})
Expand All @@ -928,14 +929,32 @@ func (a *adkApiTranslator) translateRemoteMCPServerTarget(ctx context.Context, a
}
agent.HttpTools = append(agent.HttpTools, adk.HttpMcpServerConfig{
Params: *tool,
Tools: mcpServerTool.ToolNames,
Tools: toolNames,
AllowedHeaders: mcpServerTool.AllowedHeaders,
RequireApproval: mcpServerTool.RequireApproval,
})
}
return nil
}

// resolveToolNames returns the explicit toolNames filter when set; otherwise it
// expands to the discovered tool list from the RemoteMCPServer status. The
// returned slice is always non-nil so it serializes as a JSON array (the Python
// ADK runtime rejects a null `tools` field with a pydantic validation error).
func resolveToolNames(explicit []string, discovered []*v1alpha2.MCPTool) []string {
if len(explicit) > 0 {
return explicit
}
out := make([]string, 0, len(discovered))
for _, t := range discovered {
if t == nil {
continue
}
out = append(out, t.Name)
}
return out
}

// Helper functions

// isInternalK8sURL checks if a URL points to an internal Kubernetes service.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package agent_test

import (
"context"
"encoding/json"
"fmt"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -1413,3 +1415,83 @@ func Test_AdkApiTranslator_SandboxAgent_BYOEmitsSandbox(t *testing.T) {
require.False(t, sawDeploy)
require.False(t, sawService, "sandbox runtime must not include Service; agent-sandbox owns it")
}

// Test_AdkApiTranslator_RemoteMCPServer_NoToolNames asserts that a RemoteMCPServer
// reference without an explicit toolNames filter materializes to a non-null
// `tools` JSON array, expanded from the server's discovered tool list. The
// Python ADK runtime rejects `tools: null` with a pydantic validation error, so
// the controller must emit an explicit list (https://github.com/kagent-dev/kagent/issues/...).
func Test_AdkApiTranslator_RemoteMCPServer_NoToolNames(t *testing.T) {
scheme := schemev1.Scheme
require.NoError(t, v1alpha2.AddToScheme(scheme))

ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-ns"}}
modelConfig := &v1alpha2.ModelConfig{
ObjectMeta: metav1.ObjectMeta{Name: "m1", Namespace: "test-ns"},
Spec: v1alpha2.ModelConfigSpec{
Model: "gpt-4",
Provider: v1alpha2.ModelProviderOpenAI,
},
}
rms := &v1alpha2.RemoteMCPServer{
ObjectMeta: metav1.ObjectMeta{Name: "tools-server", Namespace: "test-ns"},
Spec: v1alpha2.RemoteMCPServerSpec{
Description: "tools server",
URL: "http://tools.example.com/mcp",
},
Status: v1alpha2.RemoteMCPServerStatus{
DiscoveredTools: []*v1alpha2.MCPTool{
{Name: "alpha"},
{Name: "beta"},
nil, // defensive: nil entry must be skipped, not panic
},
},
}
agent := &v1alpha2.Agent{
ObjectMeta: metav1.ObjectMeta{Name: "ag1", Namespace: "test-ns"},
Spec: v1alpha2.AgentSpec{
Type: v1alpha2.AgentType_Declarative,
Description: "agent",
Declarative: &v1alpha2.DeclarativeAgentSpec{
SystemMessage: "test",
ModelConfig: "m1",
Tools: []*v1alpha2.Tool{
{
Type: v1alpha2.ToolProviderType_McpServer,
McpServer: &v1alpha2.McpServerTool{
TypedReference: v1alpha2.TypedReference{
Kind: "RemoteMCPServer",
ApiGroup: "kagent.dev",
Name: "tools-server",
},
// NOTE: deliberately no ToolNames
},
},
},
},
},
}

kubeClient := fake.NewClientBuilder().
WithScheme(scheme).
WithObjects(ns, modelConfig, rms, agent).
Build()

defaultModel := types.NamespacedName{Namespace: "test-ns", Name: "m1"}
trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "", nil)

outputs, err := translator.TranslateAgent(context.Background(), trans, agent)
require.NoError(t, err)
require.NotNil(t, outputs)
require.NotNil(t, outputs.Config)
require.Len(t, outputs.Config.HttpTools, 1)
assert.Equal(t, []string{"alpha", "beta"}, outputs.Config.HttpTools[0].Tools)

// JSON wire format must not contain `"tools":null` (the Python ADK runtime
// rejects it with a pydantic validation error).
raw, err := json.Marshal(outputs.Config)
require.NoError(t, err)
assert.NotContains(t, string(raw), `"tools":null`)
assert.True(t, strings.Contains(string(raw), `"tools":["alpha","beta"]`),
"expected tools to be inlined as a JSON array, got: %s", string(raw))
}
15 changes: 11 additions & 4 deletions python/packages/kagent-adk/src/kagent/adk/types.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import logging
from typing import Any, Callable, Literal, Optional, Union
from typing import Annotated, Any, Callable, Literal, Optional, Union

import httpx
from agentsts.adk import ADKTokenPropagationPlugin
Expand All @@ -11,7 +11,7 @@
from google.adk.models.anthropic_llm import Claude as ClaudeLLM
from google.adk.models.google_llm import Gemini as GeminiLLM
from google.adk.tools.mcp_tool import SseConnectionParams, StreamableHTTPConnectionParams
from pydantic import AliasChoices, BaseModel, Field
from pydantic import AliasChoices, BaseModel, BeforeValidator, Field

from kagent.adk._approval import make_approval_callback, strip_confirmation_parts_callback
from kagent.adk._mcp_toolset import KAgentMcpToolset
Expand Down Expand Up @@ -140,16 +140,23 @@ def _convert_ollama_options(options: dict[str, str] | None) -> dict[str, Any]:
return converted


def _coerce_none_to_empty_list(v: Any) -> Any:
# The kagent controller has historically emitted `tools: null` for
# RemoteMCPServer refs without an explicit toolNames filter (kagent#TBD).
# Tolerate it here so older controller versions don't crash the runtime.
return [] if v is None else v


class HttpMcpServerConfig(BaseModel):
params: StreamableHTTPConnectionParams
tools: list[str] = Field(default_factory=list)
tools: Annotated[list[str], BeforeValidator(_coerce_none_to_empty_list)] = Field(default_factory=list)
allowed_headers: list[str] | None = None # Headers to forward from A2A request to MCP calls
require_approval: list[str] | None = None # Tools requiring human approval before execution


class SseMcpServerConfig(BaseModel):
params: SseConnectionParams
tools: list[str] = Field(default_factory=list)
tools: Annotated[list[str], BeforeValidator(_coerce_none_to_empty_list)] = Field(default_factory=list)
allowed_headers: list[str] | None = None # Headers to forward from A2A request to MCP calls
require_approval: list[str] | None = None # Tools requiring human approval before execution

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
"""Regression test for kagent#1797: the controller has historically emitted
``http_tools[*].tools = null`` for RemoteMCPServer refs without an explicit
``toolNames`` filter. The runtime must tolerate that input rather than crashing
with a pydantic ``list_type`` validation error at startup.
"""

import json

import pytest

from kagent.adk.types import AgentConfig, HttpMcpServerConfig, SseMcpServerConfig


_HTTP_PARAMS = {"url": "http://example/mcp", "headers": {}, "terminate_on_close": True}
_SSE_PARAMS = {"url": "http://example/mcp", "headers": {}}


@pytest.mark.parametrize(
"model, params",
[
(HttpMcpServerConfig, _HTTP_PARAMS),
(SseMcpServerConfig, _SSE_PARAMS),
],
)
def test_tools_none_is_coerced_to_empty_list(model, params):
cfg = model.model_validate({"params": params, "tools": None})
assert cfg.tools == []


@pytest.mark.parametrize(
"model, params",
[
(HttpMcpServerConfig, _HTTP_PARAMS),
(SseMcpServerConfig, _SSE_PARAMS),
],
)
def test_tools_missing_defaults_to_empty_list(model, params):
cfg = model.model_validate({"params": params})
assert cfg.tools == []


@pytest.mark.parametrize(
"model, params",
[
(HttpMcpServerConfig, _HTTP_PARAMS),
(SseMcpServerConfig, _SSE_PARAMS),
],
)
def test_explicit_tools_list_is_preserved(model, params):
cfg = model.model_validate({"params": params, "tools": ["alpha", "beta"]})
assert cfg.tools == ["alpha", "beta"]


def _make_agent_config_blob(http_tools_value, sse_tools_value):
"""Reproduce the exact wire format the kagent controller writes to the
agent's ``config.json`` Secret. Any field-level coercion in
``HttpMcpServerConfig`` / ``SseMcpServerConfig`` must keep this top-level
construction path working — that path is what crashes the runtime pod on
startup."""
return {
"model": {
"type": "openai",
"model": "gpt-4o-mini",
"api_key": "dummy",
},
"description": "repro for #1797",
"instruction": "test",
"http_tools": [
{
"params": {
"url": "http://example/mcp",
"headers": {},
"terminate_on_close": True,
},
"tools": http_tools_value,
}
],
"sse_tools": [
{
"params": {"url": "http://example/sse", "headers": {}},
"tools": sse_tools_value,
}
],
}


def test_agent_config_load_with_null_tools_does_not_crash():
"""End-to-end runtime startup path: load the exact JSON blob the controller
writes when ``toolNames`` is omitted. Before the fix this raised
``ValidationError: Input should be a valid list ... input_value=None``."""
blob = _make_agent_config_blob(http_tools_value=None, sse_tools_value=None)
cfg = AgentConfig.model_validate_json(json.dumps(blob))
assert cfg.http_tools[0].tools == []
assert cfg.sse_tools[0].tools == []


def test_agent_config_load_with_explicit_tools_preserved():
blob = _make_agent_config_blob(
http_tools_value=["alpha", "beta"],
sse_tools_value=["gamma"],
)
cfg = AgentConfig.model_validate_json(json.dumps(blob))
assert cfg.http_tools[0].tools == ["alpha", "beta"]
assert cfg.sse_tools[0].tools == ["gamma"]
Loading