diff --git a/contracts/agents-api/execution-tools.md b/contracts/agents-api/execution-tools.md index b19976692..7a074a728 100644 --- a/contracts/agents-api/execution-tools.md +++ b/contracts/agents-api/execution-tools.md @@ -90,7 +90,7 @@ Execution carries the resolved values to the Harness and validates them against } ``` -- `server_label` is nonempty and unique within the Session. The `http` and `stdio` transports are accepted; `server_url` is an absolute HTTP or HTTPS URL without credentials or fragment. Nonempty `request_metadata` is rejected with "Nonempty MCP request_metadata is not supported yet." and an inline `authorization` with "Inline MCP authorization is not supported; use credential_id.". `headers` are non-secret, need valid HTTP names and values with names unique ignoring case, and reach the native Harness unchanged beside any bearer; an `Authorization` header is rejected, because credentials enter only through a Vault credential. Claude rejects `${` in `server_url` or a header value ([native differences](./index.md#differences-from-openai)). +- `server_label` contains only letters, digits, underscores and hyphens and is unique among the tools; a violation is a 400 `invalid_request_error` naming the offending tool's `server_label`. A rejected transport member is named the same way, such as `tools[0].transport.cwd`, and on the inline Session agent the path starts with `agent.`. The `http` and `stdio` transports are accepted; `server_url` is an absolute HTTP or HTTPS URL without credentials or fragment. Nonempty `request_metadata` is rejected with "No engine sends MCP request_metadata." and an inline `authorization` with "Inline MCP authorization is not supported; use credential_id.". `headers` are non-secret, need valid HTTP names and values with names unique ignoring case, and reach the native Harness unchanged beside any bearer; an `Authorization` header is rejected, because credentials enter only through a Vault credential. Claude rejects `${` in `server_url` or a header value ([native differences](./index.md#differences-from-openai)). - [Public MCP connection origin](./environments.md#public-mcp-connection-origin) owns origin defaults, placement and credential authority; [Harness capabilities](./harness-capabilities.md#tools) owns per-Harness support. - Omitted or null `allowed_tools` permits every server tool; `[]` permits none. - `required: true` makes native thread creation and cold resume wait for the server to initialize; a failure stops execution without replacing retained history. It needs the Runtime's `mcp_http_required` capability. Public work can be accepted or queued during the wait. diff --git a/contracts/agents-api/index.md b/contracts/agents-api/index.md index ce1d4aa1d..e8784cfff 100644 --- a/contracts/agents-api/index.md +++ b/contracts/agents-api/index.md @@ -80,7 +80,7 @@ Each item is Core's deliberate or native behavior where the official service beh - A Session's `agent.tools` omits `tool_search` declarations. - Core rejects an inline `authorization` on a Session MCP transport, an `Authorization` entry in `headers` and a nonempty stdio `env`: credentials come only from Vaults (`credential_id`) and environment values only from the Environment (`env_vars`) ([MCP](./execution-tools.md#mcp)). - A `stdio` MCP server needs a workspace placement and the origin `environment`. Claude Code starts the server in its workspace directory, so `cwd` must equal it, with its whole process environment, which also holds the model provider key and other servers' Vault bearer tokens, and it needs `allowed_tools` null. MiniMax Code starts the server in its private home directory, so it rejects every `stdio` server ([MCP](./execution-tools.md#mcp)). -- Core rejects nonempty MCP `request_metadata`, which no Harness can send, as well as a `stdio` server whose `connection_origin` is `service`, a blank `server_label`, an empty `credential_id` and an empty `allowed_tools` name, which the pinned schema allows without defining their meaning (400 with the reason). +- Core rejects nonempty MCP `request_metadata`, which no Harness can send, as well as a `stdio` server whose `connection_origin` is `service`, an empty `credential_id` and an empty `allowed_tools` name, which the pinned schema allows without defining their meaning (400 with the reason). - Claude rejects an MCP `server_url` or `headers` value containing `${`, because the native client expands `${VAR}` in both from a process environment that holds Core-held credentials. - Deleting a Session right after an events 202 returns 409, because Core admits the Turn in the same transaction; the official service returns 200. diff --git a/contracts/agents-api/wire-semantics.md b/contracts/agents-api/wire-semantics.md index 3146bff2b..182915c32 100644 --- a/contracts/agents-api/wire-semantics.md +++ b/contracts/agents-api/wire-semantics.md @@ -172,6 +172,7 @@ Agent create and update bodies and the inline `agent` of Session create are chec | Unsupported enum value | JSON path | `Invalid value: ''. Supported values are: ...` with the pinned values | | Integer below minimum | JSON path | `Invalid '': integer below minimum value. Expected a value >= 1, but got instead.` | | Repeated function name, more than one `web_search`, more than one `tool_search` | null | `duplicate function tool name: `, `duplicate web_search tool`, `duplicate tool_search tool` | +| An MCP `server_label` outside `[A-Za-z0-9_-]`, or one repeated by a later tool | `tools[].server_label` of the offending tool | A sentence stating the cause | | Function `parameters` with a string root `type` other than `object` | null | `Invalid schema for function '': schema must be a JSON Schema of 'type: "object"', got 'type: ""'.` | | `text.format` JSON schema with a string root `type` other than `object` | null | `agent.text.format.schema must have top-level type "object"; got ""`, also on Agent requests | @@ -187,7 +188,6 @@ A Session's effective configuration must also pass execution admission, which ap | --- | --- | | A `reasoning.effort`, `reasoning.summary`, `service_tier`, `web_search` mode or setting, `programmatic_tool_calling` value or `text.verbosity` the Harness does not declare | A message naming the field, such as `reasoning.effort "max" is not supported by this Harness.`; see [tool policy](./execution-tools.md#web-search-and-programmatic-tool-calling) and [Harness capabilities](./harness-capabilities.md#execution-and-input) | | A function name that is blank or repeated | "Function names must be nonempty and unique." | -| Two MCP servers with one label | "Execution requires distinct MCP server labels." | | A Harness that does not declare `retained_native_history`, on a sandbox provider that keeps every Environment on workspace storage ([Harness capabilities](./harness-capabilities.md)) | "This Session's harness does not retain its native history, which the sandbox provider's workspace storage requires. Use a harness that supports retained_native_history, or a sandbox provider with its own disk." | A per-Session `tools` replacement admits a Session whose saved tools would be rejected. Support for each tool and Harness is in [execution and tools](./execution-tools.md). diff --git a/contracts/agents-api/zh/execution-tools.md b/contracts/agents-api/zh/execution-tools.md index cebd341ca..df431e5a0 100644 --- a/contracts/agents-api/zh/execution-tools.md +++ b/contracts/agents-api/zh/execution-tools.md @@ -1,7 +1,7 @@ --- title: "执行工具" source: contracts/agents-api/execution-tools.md -source_hash: 44b540ff6827f818d8bc4cbaa6dc5fc9929bbf82a15c66edeb604909ecb6b110 +source_hash: 7783010b0968214d235d349fd60d1ee6096f0ab9d33d58397d90bb0fd935cded --- Agent 在 `tools` 中声明应用函数、控制项和 MCP 服务器,并可在 `text.format` 中声明输出 schema。本契约说明 Core 如何验证声明、哪些内容跨越 Runtime 边界,以及调用方如何恢复待执行操作。[Harness 能力](harness-capabilities.md)列出各 Harness 在不同部署位置支持的操作。原生工作区工具和 Environment Plugin MCP 属于 [Environment](environments.md#skills-plugins-and-environment-mcp)。 @@ -92,7 +92,7 @@ Core 发送 `PromptRequestPayload.ToolSearch` 和每个 `FunctionTool.DeferLoadi } ``` -- `server_label` 非空且在 Session 中唯一。接受 `http` 和 `stdio` 传输;`server_url` 为不带凭据或片段的绝对 HTTP 或 HTTPS URL。非空 `request_metadata` 被拒绝,消息为 "Nonempty MCP request_metadata is not supported yet.";内联 `authorization` 被拒绝,消息为 "Inline MCP authorization is not supported; use credential_id."。`headers` 为非机密标头,名称和值须为有效的 HTTP 名称和值,且名称不区分大小写唯一,会与 bearer 一同原样传给原生 Harness;`Authorization` 标头被拒绝,因为凭据只通过 Vault 凭据进入。Claude 会拒绝 `server_url` 或标头值中的 `${`([原生差异](index.md#differences-from-openai))。 +- `server_label` 只含字母、数字、下划线和连字符,且在各工具之间唯一;违反时返回 400 `invalid_request_error`,并指出出错工具的 `server_label`。被拒绝的传输成员以同样方式指出,例如 `tools[0].transport.cwd`,Session 内联 Agent 的路径以 `agent.` 开头。接受 `http` 和 `stdio` 传输;`server_url` 为不带凭据或片段的绝对 HTTP 或 HTTPS URL。非空 `request_metadata` 被拒绝,消息为 "No engine sends MCP request_metadata.";内联 `authorization` 被拒绝,消息为 "Inline MCP authorization is not supported; use credential_id."。`headers` 为非机密标头,名称和值须为有效的 HTTP 名称和值,且名称不区分大小写唯一,会与 bearer 一同原样传给原生 Harness;`Authorization` 标头被拒绝,因为凭据只通过 Vault 凭据进入。Claude 会拒绝 `server_url` 或标头值中的 `${`([原生差异](index.md#differences-from-openai))。 - [公开 MCP 连接来源](environments.md#public-mcp-connection-origin)定义来源默认值、部署位置和凭据权限;[Harness 能力](harness-capabilities.md#tools)定义各 Harness 支持范围。 - 省略或 null 的 `allowed_tools` 允许所有服务器工具;`[]` 不允许任何工具。 - `required: true` 使原生线程创建和冷恢复等待服务器初始化;失败会停止执行,不替换保留历史。它要求 Runtime 的 `mcp_http_required` 能力。等待期间公开工作可被接受或排队。 diff --git a/contracts/agents-api/zh/index.md b/contracts/agents-api/zh/index.md index 57a4f2e98..42c5abbee 100644 --- a/contracts/agents-api/zh/index.md +++ b/contracts/agents-api/zh/index.md @@ -1,7 +1,7 @@ --- title: "OpenAI 兼容性" source: contracts/agents-api/index.md -source_hash: d65d6dc5d6a8ec195950a119b667964bf5bb651c469191235779d19a7220ff92 +source_hash: afb241cb659061b9f5c50c5db88c83080d02d07eab37ffb2f1c70d5e2386c96e --- Core 实现下方固定版本的 OpenAI Agents API。本台账列出各项资源的覆盖情况、相对于官方服务的所有差异以及已知缺口。 @@ -83,7 +83,7 @@ Core 自身字段位于 `x_agents_core` 中([Core extensions](../../../docs/zh - Session 的 `agent.tools` 会省略 `tool_search` 声明。 - Core 拒绝 Session MCP 传输中的内联 `authorization`、`headers` 中的 `Authorization` 条目以及非空的 stdio `env`:凭据只来自 Vault(`credential_id`),环境值只来自 Environment(`env_vars`)([MCP](execution-tools.md#mcp))。 - `stdio` MCP 服务器需要工作区部署位置和 `environment` 来源。Claude Code 在其工作区目录中启动服务器,因此 `cwd` 必须与之相同;服务器继承整个进程环境,其中也有模型提供方密钥和其他服务器的 Vault bearer 令牌;且 `allowed_tools` 须为 null。MiniMax Code 在其私有主目录中启动服务器,因此拒绝所有 `stdio` 服务器([MCP](execution-tools.md#mcp))。 -- Core 拒绝非空的 MCP `request_metadata`,因为没有 Harness 能发送它;同样拒绝 `connection_origin` 为 `service` 的 `stdio` 服务器,以及空白的 `server_label`、空的 `credential_id` 和空的 `allowed_tools` 名称,固定版本的 schema 允许这些值但没有定义其含义(400,并说明原因)。 +- Core 拒绝非空的 MCP `request_metadata`,因为没有 Harness 能发送它;同样拒绝 `connection_origin` 为 `service` 的 `stdio` 服务器,以及空的 `credential_id` 和空的 `allowed_tools` 名称,固定版本的 schema 允许这些值但没有定义其含义(400,并说明原因)。 - Claude 会拒绝包含 `${` 的 MCP `server_url` 或 `headers` 值,因为原生客户端会从持有 Core 所持凭据的进程环境中展开这两处的 `${VAR}`。 - 在 events 202 之后立即删除 Session 会返回 409,因为 Core 会在同一事务中准入该 Turn;官方服务返回 200。 diff --git a/contracts/agents-api/zh/wire-semantics.md b/contracts/agents-api/zh/wire-semantics.md index 4c807eddd..978886b12 100644 --- a/contracts/agents-api/zh/wire-semantics.md +++ b/contracts/agents-api/zh/wire-semantics.md @@ -1,7 +1,7 @@ --- title: "API 行为" source: contracts/agents-api/wire-semantics.md -source_hash: 1d087e0f29e20f091416cc7bd3293e754c610f2335e2d66456f5205706d9983a +source_hash: 851e09252847f11040524f329419587687e81d361e4e92c0a3e912d2fd887d45 --- 已锁定版本的 OpenAI Python SDK([upstream.json](https://github.com/MiniMax-AI/OpenAgentCore/blob/main/contracts/agents-api/upstream.json))定义了 `/v1` 路由、字段和类型。本页面说明这些类型未作规定之处 Core 的行为,例如状态码、错误字段、默认值和列表边界,以及 Core 与官方服务存在差异的地方。[OpenAI 兼容性](index.md) 列出了这些差异和尚存缺口;[Sessions, events and history](sessions-events.md)、[message content](message-content.md)、[Vaults](vaults.md)、[source Files and Skills](source-files.md) 和 [Environment files and Artifacts](environment-files.md) 分别负责各自资源的规则。 @@ -174,6 +174,7 @@ Agent 创建和更新正文以及 Session 创建中的内联 `agent`,会在其 | 不支持的枚举值 | JSON 路径 | `Invalid value: ''. Supported values are: ...`,后跟已锁定的值 | | 整数低于最小值 | JSON 路径 | `Invalid '': integer below minimum value. Expected a value >= 1, but got instead.` | | 重复的函数名称、多个 `web_search`、多个 `tool_search` | null | `duplicate function tool name: `、`duplicate web_search tool`、`duplicate tool_search tool` | +| MCP `server_label` 含 `[A-Za-z0-9_-]` 以外的字符,或被后面的工具重复 | 出错工具的 `tools[].server_label` | 说明原因的句子 | | Function `parameters` 的字符串根 `type` 不是 `object` | null | `Invalid schema for function '': schema must be a JSON Schema of 'type: "object"', got 'type: ""'.` | | `text.format` JSON schema 的字符串根 `type` 不是 `object` | null | `agent.text.format.schema must have top-level type "object"; got ""`,Agent 请求上也会返回此消息 | @@ -189,7 +190,6 @@ Session 的有效配置还必须通过执行准入,无论配置来自保存还 | --- | --- | | Harness 未声明的 `reasoning.effort`、`reasoning.summary`、`service_tier`、`web_search` 模式或设置、`programmatic_tool_calling` 取值或 `text.verbosity` | 指出字段的消息,例如 `reasoning.effort "max" is not supported by this Harness.`;见 [tool policy](execution-tools.md#web-search-and-programmatic-tool-calling) 和 [Harness 能力](harness-capabilities.md#execution-and-input) | | 函数名称为空或重复 | "Function names must be nonempty and unique." | -| 两个使用同一标签的 MCP 服务器 | "Execution requires distinct MCP server labels." | | 未声明 `retained_native_history` 的 Harness,运行在把每个 Environment 都放在 workspace 存储上的 sandbox provider 上([Harness 能力](harness-capabilities.md)) | "This Session's harness does not retain its native history, which the sandbox provider's workspace storage requires. Use a harness that supports retained_native_history, or a sandbox provider with its own disk." | 每 Session 的 `tools` 替换可以允许一个其已保存 tools 原本会被拒绝的 Session。每种工具和 Harness 的支持情况见 [execution and tools](execution-tools.md)。 diff --git a/services/core/internal/api/configuration_validation.go b/services/core/internal/api/configuration_validation.go index 2fb0a933f..f78dcd771 100644 --- a/services/core/internal/api/configuration_validation.go +++ b/services/core/internal/api/configuration_validation.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "fmt" + "regexp" "strconv" "strings" ) @@ -82,12 +83,41 @@ func validateAgentConfiguration(path string, raw json.RawMessage, root shape) er if value := fields["text"]; jsonValueKind(value) == "an object" { _ = json.Unmarshal(value, &text) } + if err := checkMCPLabels(path, tools); err != nil { + return err + } if text.Format == nil { return configurationConflict(tools, "", nil) } return configurationConflict(tools, text.Format.Type, text.Format.Schema) } +var mcpServerLabel = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) + +// checkMCPLabels rejects an MCP server_label outside [A-Za-z0-9_-] and a label +// repeated by a later tool, naming the offending tool's server_label. +func checkMCPLabels(path string, tools []json.RawMessage) error { + seen := map[string]bool{} + for i, raw := range tools { + var tool struct { + Type string `json:"type"` + ServerLabel string `json:"server_label"` + } + if json.Unmarshal(raw, &tool) != nil || tool.Type != "mcp" { + continue + } + param := joinPath(path, fmt.Sprintf("tools[%d].server_label", i)) + if !mcpServerLabel.MatchString(tool.ServerLabel) { + return &fieldError{param: param, message: "MCP server_label may contain only letters, digits, underscores and hyphens."} + } + if seen[tool.ServerLabel] { + return &fieldError{param: param, message: "MCP server_label must be unique across the tools."} + } + seen[tool.ServerLabel] = true + } + return nil +} + func checkValue(path string, raw json.RawMessage, s shape) error { got := jsonValueKind(raw) if s.kind == anyValue || got == "null" && s.nullable { diff --git a/services/core/internal/api/configuration_validation_test.go b/services/core/internal/api/configuration_validation_test.go index 06753adbc..361cbecb0 100644 --- a/services/core/internal/api/configuration_validation_test.go +++ b/services/core/internal/api/configuration_validation_test.go @@ -115,6 +115,8 @@ func TestAgentConfigurationProtocolErrorsUseOfficialFields(t *testing.T) { {"http url", `"tools":[{"type":"mcp","server_label":"x","transport":{"type":"http"}}]`, "{p}tools[0].transport.server_url", "Missing required parameter: '{p}tools[0].transport.server_url'."}, {"transport type", `"tools":[{"type":"mcp","server_label":"x","transport":{"type":"ws"}}]`, "{p}tools[0].transport.type", "Invalid value: 'ws'. Supported values are: 'http' and 'stdio'."}, {"header value", `"tools":[{"type":"mcp","server_label":"x","transport":{"type":"http","server_url":"https://example.invalid","headers":{"h":1}}}]`, "{p}tools[0].transport.headers.h", "Invalid type for '{p}tools[0].transport.headers.h': expected a string, but got an integer instead."}, + {"mcp label characters", `"tools":[{"type":"mcp","server_label":"a b","transport":{"type":"http","server_url":"https://example.invalid"}}]`, "{p}tools[0].server_label", "MCP server_label may contain only letters, digits, underscores and hyphens."}, + {"mcp label repeated", `"tools":[{"type":"mcp","server_label":"a","transport":{"type":"http","server_url":"https://example.invalid"}},{"type":"mcp","server_label":"a","transport":{"type":"http","server_url":"https://example.invalid"}}]`, "{p}tools[1].server_label", "MCP server_label must be unique across the tools."}, {"verbosity", `"text":{"verbosity":"verbose"}`, "{p}text.verbosity", "Invalid value: 'verbose'. Supported values are: 'low', 'medium', and 'high'."}, {"text member", `"text":{"unknown":true}`, "{p}text.unknown", "Unknown parameter: '{p}text.unknown'."}, {"format type", `"text":{"format":{}}`, "{p}text.format.type", "Missing required parameter: '{p}text.format.type'."}, @@ -276,6 +278,21 @@ func TestAgentConfigurationLocalLimitsKeepCodes(t *testing.T) { } } +// Errors from the local limits on an MCP transport name the offending member. +func TestMCPTransportErrorsNameTheMember(t *testing.T) { + const http1 = `{"type":"mcp","server_label":"x","transport":{"type":"http","server_url":"https://example.invalid"}}` + for _, tc := range []struct{ tool, param, message string }{ + {`{"type":"mcp","server_label":"x","transport":{"type":"http","server_url":"ftp://example.invalid"}}`, "tools[0].transport.server_url", "MCP server_url must be an absolute HTTP(S) URL without credentials or fragment."}, + {http1 + `,{"type":"mcp","server_label":"y","transport":{"type":"http","server_url":"https://example.invalid","headers":{"authorization":"t"}}}`, "tools[1].transport.headers.authorization", "MCP headers cannot carry Authorization; use credential_id."}, + {`{"type":"mcp","server_label":"x","transport":{"type":"stdio","command":"run","cwd":"relative"}}`, "tools[0].transport.cwd", "MCP stdio servers require a nonempty command, an absolute cwd, variable names in env_vars and no NUL characters."}, + } { + for _, op := range configurationOperations()[:3] { + h, _ := configurationHandler(t, nil) + assertConfigurationError(t, credentialRequest(h, http.MethodPost, op.path, op.body(`"tools":[`+tc.tool+`]`)), "invalid_request_error", ptr(op.prefix+tc.param), tc.message) + } + } +} + // C4 and SES-33: only configuration errors precede the input requirement; // accepted configurations and saved-Agent lookup keep their order. func TestSessionConfigurationValidationOrder(t *testing.T) { diff --git a/services/core/internal/api/mcp_configuration.go b/services/core/internal/api/mcp_configuration.go index 23829dcdb..0710ba2ba 100644 --- a/services/core/internal/api/mcp_configuration.go +++ b/services/core/internal/api/mcp_configuration.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "net/url" + "sort" "strings" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" @@ -11,9 +12,10 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/internal/agentplugin" ) -// resolveMCPTool canonicalizes a declaration whose pinned shape was checked at -// decode. Core keeps its local limits. -func resolveMCPTool(raw json.RawMessage, saved bool) (json.RawMessage, error) { +// resolveMCPTool canonicalizes a declaration whose pinned shape and server_label +// were checked at decode. Core keeps its local limits; a rejected transport +// member is named under at, the declaration's path. +func resolveMCPTool(raw json.RawMessage, at string, saved bool) (json.RawMessage, error) { var input v1.MCPToolInput var transport struct { Type string `json:"type"` @@ -29,8 +31,8 @@ func resolveMCPTool(raw json.RawMessage, saved bool) (json.RawMessage, error) { if err := errors.Join(json.Unmarshal(raw, &input), json.Unmarshal(input.Transport, &transport)); err != nil { return nil, &storedDataError{err} } - if strings.TrimSpace(input.ServerLabel) == "" { - return nil, errors.New("MCP tools require type=mcp and a nonempty server_label.") + reject := func(member, message string) error { + return &fieldError{param: at + "." + member, message: message} } stdio := transport.Type == "stdio" // The official service saves an omitted or null origin on an HTTP server as @@ -66,32 +68,54 @@ func resolveMCPTool(raw json.RawMessage, saved bool) (json.RawMessage, error) { ConnectionOrigin: origin, CredentialID: input.CredentialID, RequestMetadata: map[string]json.RawMessage{}} if stdio { if len(transport.Env) != 0 { - return nil, errors.New("Inline MCP env is not supported; declare env_vars and set the values in the Environment.") + return nil, reject("transport.env", "Inline MCP env is not supported; declare env_vars and set the values in the Environment.") } args, envVars := append([]string{}, transport.Args...), append([]string{}, transport.EnvVars...) - if !(proto.MCPStdio{Command: transport.Command, Args: args, Cwd: transport.Cwd, EnvVars: envVars}).Valid() { - return nil, errors.New("MCP stdio servers require a nonempty command, an absolute cwd, variable names in env_vars and no NUL characters.") + // Valid is the one definition of a startable declaration; each probe + // varies one member over a valid remainder to name the member at fault, + // and the whole declaration is checked last. + declaration := proto.MCPStdio{Command: transport.Command, Args: args, Cwd: transport.Cwd, EnvVars: envVars} + for _, probe := range []struct { + member string + stdio proto.MCPStdio + }{ + {"transport.command", proto.MCPStdio{Command: transport.Command, Cwd: "/"}}, + {"transport.args", proto.MCPStdio{Command: "x", Args: args, Cwd: "/"}}, + {"transport.cwd", proto.MCPStdio{Command: "x", Cwd: transport.Cwd}}, + {"transport.env_vars", proto.MCPStdio{Command: "x", Cwd: "/", EnvVars: envVars}}, + {"transport", declaration}, + } { + if !probe.stdio.Valid() { + return nil, reject(probe.member, "MCP stdio servers require a nonempty command, an absolute cwd, variable names in env_vars and no NUL characters.") + } } tool.Transport = v1.MCPTransport{Type: "stdio", Command: transport.Command, Args: &args, Cwd: transport.Cwd, EnvVars: &envVars} return json.Marshal(tool) } if transport.Type != "http" { - return nil, errors.New("MCP supports the http and stdio transports only.") + return nil, reject("transport.type", "MCP supports the http and stdio transports only.") } u, err := url.Parse(transport.ServerURL) if err != nil || u.Hostname() == "" || (u.Scheme != "http" && u.Scheme != "https") || u.User != nil || u.Fragment != "" { - return nil, errors.New("MCP server_url must be an absolute HTTP(S) URL without credentials or fragment.") + return nil, reject("transport.server_url", "MCP server_url must be an absolute HTTP(S) URL without credentials or fragment.") } if transport.Authorization != nil { - return nil, errors.New("Inline MCP authorization is not supported; use credential_id.") + return nil, reject("transport.authorization", "Inline MCP authorization is not supported; use credential_id.") } + names, seen := make([]string, 0, len(transport.Headers)), map[string]bool{} for name := range transport.Headers { + names = append(names, name) + } + sort.Strings(names) + for _, name := range names { if strings.EqualFold(name, "Authorization") { - return nil, errors.New("MCP headers cannot carry Authorization; use credential_id.") + return nil, reject("transport.headers."+name, "MCP headers cannot carry Authorization; use credential_id.") } - } - if !agentplugin.ValidHTTPHeaders(transport.Headers) { - return nil, errors.New("MCP headers require valid HTTP names and values, with names unique ignoring case.") + lower := strings.ToLower(name) + if seen[lower] || !agentplugin.ValidHTTPHeaders(map[string]string{name: transport.Headers[name]}) { + return nil, reject("transport.headers."+name, "MCP headers require valid HTTP names and values, with names unique ignoring case.") + } + seen[lower] = true } tool.Transport = v1.MCPTransport{Type: "http", ServerURL: transport.ServerURL} if len(transport.Headers) != 0 { diff --git a/services/core/internal/api/mcp_configuration_test.go b/services/core/internal/api/mcp_configuration_test.go index 0d4fe0ab5..98dcb3779 100644 --- a/services/core/internal/api/mcp_configuration_test.go +++ b/services/core/internal/api/mcp_configuration_test.go @@ -17,7 +17,7 @@ const publicMCP = `{"type":"mcp","server_label":"records","connection_origin":"s func TestMCPRequiredSavedAndEffectiveConfiguration(t *testing.T) { for _, value := range []string{"true", "false"} { input := strings.TrimSuffix(publicMCP, "}") + `,"required":` + value + `}` - saved, err := resolveMCPTool(json.RawMessage(input), true) + saved, err := resolveMCPTool(json.RawMessage(input), "tools[0]", true) if err != nil { t.Fatal(err) } @@ -36,7 +36,7 @@ func TestMCPRequiredSavedAndEffectiveConfiguration(t *testing.T) { func TestMCPResourceTransportProjections(t *testing.T) { for _, saved := range []bool{false, true} { - raw, err := resolveMCPTool(json.RawMessage(publicMCP), saved) + raw, err := resolveMCPTool(json.RawMessage(publicMCP), "tools[0]", saved) if err != nil { t.Fatal(err) } @@ -61,13 +61,13 @@ func TestMCPResourceTransportProjections(t *testing.T) { // explicit "service" declaration, in saved Agents and Session configuration. func TestMCPOmittedOriginIsService(t *testing.T) { for _, saved := range []bool{false, true} { - explicit, err := resolveMCPTool(json.RawMessage(publicMCP), saved) + explicit, err := resolveMCPTool(json.RawMessage(publicMCP), "tools[0]", saved) if err != nil { t.Fatal(err) } for _, origin := range []string{"", `,"connection_origin":null`} { input := `{"type":"mcp","server_label":"records"` + origin + `,"transport":{"type":"http","server_url":"https://mcp.example.test/tools"}}` - resolved, err := resolveMCPTool(json.RawMessage(input), saved) + resolved, err := resolveMCPTool(json.RawMessage(input), "tools[0]", saved) if err != nil || string(resolved) != string(explicit) { t.Fatalf("saved=%v origin %q: %s, %v; want %s", saved, origin, resolved, err, explicit) } @@ -84,7 +84,7 @@ func TestMCPStdioDeclaration(t *testing.T) { } for _, saved := range []bool{false, true} { for _, origin := range []string{"", `,"connection_origin":null`, `,"connection_origin":"environment"`} { - raw, err := resolveMCPTool(input(origin, stdio+`,"env":{}}`), saved) + raw, err := resolveMCPTool(input(origin, stdio+`,"env":{}}`), "tools[0]", saved) var tool map[string]any if err != nil || json.Unmarshal(raw, &tool) != nil || tool["connection_origin"] != "environment" || !reflect.DeepEqual(tool["transport"], map[string]any{"type": "stdio", "command": "run", "args": []any{"-x"}, "cwd": "/work", "env_vars": []any{"TOKEN"}}) { @@ -92,7 +92,7 @@ func TestMCPStdioDeclaration(t *testing.T) { } } } - raw, err := resolveMCPTool(input("", `{"type":"stdio","command":"run","args":null,"cwd":"/","env_vars":null}`), true) + raw, err := resolveMCPTool(input("", `{"type":"stdio","command":"run","args":null,"cwd":"/","env_vars":null}`), "tools[0]", true) if err != nil || !strings.Contains(string(raw), `"args":[]`) || !strings.Contains(string(raw), `"env_vars":[]`) { t.Fatalf("null lists: %s, %v", raw, err) } @@ -100,14 +100,14 @@ func TestMCPStdioDeclaration(t *testing.T) { "Inline MCP env is not supported; declare env_vars and set the values in the Environment.": input("", stdio+`,"env":{"TOKEN":"private-marker"}}`), "MCP stdio servers run in the Environment; connection_origin must be environment.": input(`,"connection_origin":"service"`, stdio+`}`), } { - if _, err := resolveMCPTool(raw, false); err == nil || err.Error() != message { + if _, err := resolveMCPTool(raw, "tools[0]", false); err == nil || err.Error() != message { t.Fatalf("want %q, got %v", message, err) } } const invalid = "MCP stdio servers require a nonempty command, an absolute cwd, variable names in env_vars and no NUL characters." for _, transport := range []string{`{"type":"stdio","command":" ","cwd":"/"}`, `{"type":"stdio","command":"run","cwd":"work"}`, `{"type":"stdio","command":"run","cwd":"/","env_vars":["A-B"]}`, `{"type":"stdio","command":"run","args":["\u0000"],"cwd":"/"}`} { - if _, err := resolveMCPTool(input("", transport), true); err == nil || err.Error() != invalid { + if _, err := resolveMCPTool(input("", transport), "tools[0]", true); err == nil || err.Error() != invalid { t.Fatalf("%s: %v", transport, err) } } @@ -123,7 +123,7 @@ func TestMCPAllowedToolsAndOptionalFields(t *testing.T) { input["credential_id"], input["request_metadata"], input["required"] = json.RawMessage("null"), json.RawMessage("null"), json.RawMessage("false") input["transport"] = json.RawMessage(`{"type":"http","server_url":"https://mcp.example.test/tools","authorization":null,"headers":null}`) encoded, _ := json.Marshal(input) - resolved, err := resolveMCPTool(encoded, false) + resolved, err := resolveMCPTool(encoded, "tools[0]", false) if err != nil { t.Fatal(err) } @@ -159,7 +159,7 @@ func TestMCPUnsupportedInputsAreSecretSafe(t *testing.T) { } } encoded, _ := json.Marshal(input) - if _, err := resolveMCPTool(encoded, false); err == nil || strings.Contains(err.Error(), "private-marker") { + if _, err := resolveMCPTool(encoded, "tools[0]", false); err == nil || strings.Contains(err.Error(), "private-marker") { t.Fatalf("unsupported MCP input was accepted or disclosed: %v", err) } }) @@ -169,18 +169,18 @@ func TestMCPUnsupportedInputsAreSecretSafe(t *testing.T) { func TestMCPHeadersAreStoredAsGiven(t *testing.T) { for _, saved := range []bool{false, true} { input := `{"type":"mcp","server_label":"records","transport":{"type":"http","server_url":"https://mcp.example.test","headers":{"X-Tenant":"a"}}}` - raw, err := resolveMCPTool(json.RawMessage(input), saved) + raw, err := resolveMCPTool(json.RawMessage(input), "tools[0]", saved) if err != nil || !strings.Contains(string(raw), `"headers":{"X-Tenant":"a"}`) { t.Fatalf("saved=%v: %s, %v", saved, raw, err) } - if _, err := resolveMCPTool(json.RawMessage(strings.Replace(input, "X-Tenant", "authorization", 1)), saved); err == nil { + if _, err := resolveMCPTool(json.RawMessage(strings.Replace(input, "X-Tenant", "authorization", 1)), "tools[0]", saved); err == nil { t.Fatal("Authorization header accepted") } } } func TestSessionMCPKeepsToolOrderAndStripsSavedHeaders(t *testing.T) { - saved, err := resolveMCPTool(json.RawMessage(publicMCP), true) + saved, err := resolveMCPTool(json.RawMessage(publicMCP), "tools[0]", true) if err != nil { t.Fatal(err) } @@ -192,13 +192,10 @@ func TestSessionMCPKeepsToolOrderAndStripsSavedHeaders(t *testing.T) { if strings.Contains(string(tools[0]), "headers") || string(tools[1]) != string(function) { t.Fatalf("Session transport or declared order changed: %s", tools) } - if _, err := resolveSessionTools([]json.RawMessage{saved, function, saved}); err == nil { - t.Fatal("duplicate MCP server labels were admitted") - } } func TestSessionReadOmitsMCPHeaders(t *testing.T) { - stored, err := resolveMCPTool(json.RawMessage(`{"type":"mcp","server_label":"records","transport":{"type":"http","server_url":"https://mcp.example.test","headers":{"X-Tenant":"a"}}}`), false) + stored, err := resolveMCPTool(json.RawMessage(`{"type":"mcp","server_label":"records","transport":{"type":"http","server_url":"https://mcp.example.test","headers":{"X-Tenant":"a"}}}`), "tools[0]", false) if err != nil { t.Fatal(err) } @@ -216,7 +213,7 @@ func TestPublicEnvironmentMCPPreservesDeclaration(t *testing.T) { for _, saved := range []bool{false, true} { for _, allowed := range []string{"null", "[]", `["prove"]`} { raw := []byte(`{"type":"mcp","server_label":"remote","connection_origin":"environment","transport":{"type":"http","server_url":"https://example.test/mcp"},"allowed_tools":` + allowed + `,"required":true,"credential_id":"selected"}`) - resolved, err := resolveMCPTool(raw, saved) + resolved, err := resolveMCPTool(raw, "tools[0]", saved) if err != nil { t.Fatal(err) } @@ -230,7 +227,7 @@ func TestPublicEnvironmentMCPPreservesDeclaration(t *testing.T) { func TestMCPServerURLKeepsQuery(t *testing.T) { const serverURL = "https://mcp.example.test/tools?tenant=a%20b" - saved, err := resolveMCPTool(json.RawMessage(strings.Replace(publicMCP, "https://mcp.example.test/tools", serverURL, 1)), true) + saved, err := resolveMCPTool(json.RawMessage(strings.Replace(publicMCP, "https://mcp.example.test/tools", serverURL, 1)), "tools[0]", true) if err != nil || !strings.Contains(string(saved), serverURL) { t.Fatalf("query URL was rejected or changed: %s, %v", saved, err) } diff --git a/services/core/internal/api/saved_tools.go b/services/core/internal/api/saved_tools.go index 6cb301e03..04cf2eb66 100644 --- a/services/core/internal/api/saved_tools.go +++ b/services/core/internal/api/saved_tools.go @@ -3,6 +3,7 @@ package api import ( "encoding/json" "errors" + "fmt" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" ) @@ -12,7 +13,7 @@ import ( // resolve yet. func resolveSavedTools(input []json.RawMessage) ([]json.RawMessage, error) { tools := make([]json.RawMessage, 0, len(input)) - for _, raw := range input { + for i, raw := range input { var kind struct { Type string `json:"type"` } @@ -40,7 +41,7 @@ func resolveSavedTools(input []json.RawMessage) ([]json.RawMessage, error) { } value = resolved case "mcp": - resolved, err := resolveMCPTool(raw, true) + resolved, err := resolveMCPTool(raw, fmt.Sprintf("tools[%d]", i), true) if err != nil { return nil, err } diff --git a/services/core/internal/api/session_agent.go b/services/core/internal/api/session_agent.go index aa81f7400..5e8a12ee1 100644 --- a/services/core/internal/api/session_agent.go +++ b/services/core/internal/api/session_agent.go @@ -32,6 +32,12 @@ func resolveSessionAgent(input sessionRequest, saved *v1.SavedAgent) (v1.Agent, } resolved, err := resolveSavedAgent(request) if err != nil { + // Saved-Agent resolution names members from the request root; the inline + // agent lives under agent. + var field *fieldError + if errors.As(err, &field) && field.param != "" && input.Agent != nil { + err = &fieldError{param: "agent." + field.param, message: field.message} + } return v1.Agent{}, err } var override v1.SavedAgentConfiguration diff --git a/services/core/internal/api/session_credentials_test.go b/services/core/internal/api/session_credentials_test.go index d291f6a6f..c39771700 100644 --- a/services/core/internal/api/session_credentials_test.go +++ b/services/core/internal/api/session_credentials_test.go @@ -15,7 +15,7 @@ import ( func TestMCPCredentialReferenceIsSchemaNotAuthorization(t *testing.T) { for _, saved := range []bool{false, true} { input := strings.TrimSuffix(publicMCP, "}") + `,"credential_id":"unresolved-reference"}` - raw, err := resolveMCPTool(json.RawMessage(input), saved) + raw, err := resolveMCPTool(json.RawMessage(input), "tools[0]", saved) var output v1.MCPTool if err != nil || json.Unmarshal(raw, &output) != nil || output.CredentialID == nil || *output.CredentialID != "unresolved-reference" { t.Fatal("saved/effective schema resolved or lost the caller credential reference", err) diff --git a/services/core/internal/api/session_tools.go b/services/core/internal/api/session_tools.go index f2f02d0da..fc744566d 100644 --- a/services/core/internal/api/session_tools.go +++ b/services/core/internal/api/session_tools.go @@ -3,6 +3,7 @@ package api import ( "encoding/json" "errors" + "fmt" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" ) @@ -11,7 +12,6 @@ func resolveSessionTools(input []json.RawMessage) ([]json.RawMessage, error) { tools := make([]json.RawMessage, len(input)) functions := make([]v1.FunctionToolInput, 0, len(input)) positions := make([]int, 0, len(input)) - servers := map[string]bool{} search := false controls := map[string]bool{} for i, raw := range input { @@ -43,18 +43,10 @@ func resolveSessionTools(input []json.RawMessage) ([]json.RawMessage, error) { search = true tools[i], _ = json.Marshal(kind) case "mcp": - resolved, err := resolveMCPTool(raw, false) + resolved, err := resolveMCPTool(raw, fmt.Sprintf("tools[%d]", i), false) if err != nil { return nil, err } - var tool v1.MCPTool - if err := json.Unmarshal(resolved, &tool); err != nil { - return nil, &storedDataError{err} - } - if servers[tool.ServerLabel] { - return nil, errors.New("Execution requires distinct MCP server labels.") - } - servers[tool.ServerLabel] = true tools[i] = resolved case "function": var function v1.FunctionToolInput