From bb38a9d4d28ca74e787eebfd26d6c1bfc3dd10e7 Mon Sep 17 00:00:00 2001 From: naiba Date: Sun, 31 May 2026 16:18:57 +0000 Subject: [PATCH] fix(mcp): accept agents reporting version without v prefix and stop masking tool errors compareSemver fell back to lexical string ordering when the numeric segments were equal, so an agent reporting "2.1.0" compared as older than the gate constant "v2.1.0" (0x32 < 0x76). This wrongly flagged every agent as unsupported_agent, breaking server.exec and fs.* on all online servers. Drop the fallback: semverParts already strips the v prefix, so equal numeric segments mean equal versions. The error was further masked because tool error responses shipped a structuredContent {error_code,error} that violates the tool outputSchema (which requires exit_code/stdout/...). Strict MCP clients validated it and rejected the whole response with -32602, hiding the real isError text. Omit structuredContent on error; the cause stays in content[].text. --- cmd/dashboard/controller/mcp.go | 7 +- cmd/dashboard/controller/mcp_capability.go | 17 ++--- .../controller/mcp_capability_test.go | 25 +++++++ .../controller/mcp_error_structured_test.go | 67 +++++++++++++++++++ 4 files changed, 100 insertions(+), 16 deletions(-) create mode 100644 cmd/dashboard/controller/mcp_error_structured_test.go diff --git a/cmd/dashboard/controller/mcp.go b/cmd/dashboard/controller/mcp.go index 9788b56a..e35d6908 100644 --- a/cmd/dashboard/controller/mcp.go +++ b/cmd/dashboard/controller/mcp.go @@ -327,13 +327,12 @@ func handleToolsCall(c *gin.Context, req *jsonRPCRequest, tok *model.APIToken) { }) return } + // 错误结果不带 structuredContent：严格客户端会拿它去校验工具声明的 + // outputSchema（要求 exit_code/stdout/... 等成功字段），缺字段就整条 + // 响应报 -32602，把真正的 isError 文本掩盖掉。错误信息走 content[].text。 writeJSONRPCResult(c, req.ID, mcpToolCallResult{ Content: []mcpContent{{Type: "text", Text: errMsg}}, IsError: true, - StructuredContent: map[string]string{ - "error_code": errCode, - "error": errMsg, - }, }) } diff --git a/cmd/dashboard/controller/mcp_capability.go b/cmd/dashboard/controller/mcp_capability.go index e743f530..a7a137cd 100644 --- a/cmd/dashboard/controller/mcp_capability.go +++ b/cmd/dashboard/controller/mcp_capability.go @@ -27,13 +27,12 @@ func requireAgentSupportsMCP(server *model.Server) error { return nil } -// compareSemver 比较两个 "MAJOR.MINOR.PATCH[-suffix]" 字符串。 -// 返回 -1/0/1。无法解析时按字符串字典序比较，保证全序但可能不精确—— -// 对于 "agent 太老" 的快速失败用途已经足够。 +// compareSemver 比较两个 "MAJOR.MINOR.PATCH[-suffix]" 字符串，返回 -1/0/1。 +// semverParts 已剥掉可选的 "v" 前缀与 "-/+" 后缀，所以只按三段数字定序： +// 数字段相等即视为相等版本。绝不能回退到字符串字典序——agent 上报 "2.1.0" +// 而门槛常量是 "v2.1.0"，'2'(0x32) < 'v'(0x76) 会把相等版本误判为更旧， +// 导致所有 agent 被错误地判为不支持 MCP。 func compareSemver(a, b string) int { - if a == b { - return 0 - } aparts := semverParts(a) bparts := semverParts(b) for i := 0; i < 3; i++ { @@ -44,12 +43,6 @@ func compareSemver(a, b string) int { return 1 } } - if a < b { - return -1 - } - if a > b { - return 1 - } return 0 } diff --git a/cmd/dashboard/controller/mcp_capability_test.go b/cmd/dashboard/controller/mcp_capability_test.go index b828c34b..41d04f91 100644 --- a/cmd/dashboard/controller/mcp_capability_test.go +++ b/cmd/dashboard/controller/mcp_capability_test.go @@ -50,3 +50,28 @@ func TestRequireAgentSupportsMCPDefersWhenAgentNeverReported(t *testing.T) { require.NoError(t, requireAgentSupportsMCP(&model.Server{Host: nil}), "Host==nil means agent never reported its build; defer the version decision to the CallAgent timeout layer") } + +func TestCompareSemverIgnoresVPrefixMismatch(t *testing.T) { + cases := []struct { + a, b string + want int + }{ + {"2.1.0", "v2.1.0", 0}, + {"v2.1.0", "2.1.0", 0}, + {"2.1.0", "2.1.0", 0}, + {"2.1.0", "v2.1.1", -1}, + {"v2.1.2", "2.1.0", 1}, + {"2.2.0", "v2.1.9", 1}, + } + for _, c := range cases { + require.Equalf(t, c.want, compareSemver(c.a, c.b), + "compareSemver(%q,%q): the only difference is the optional 'v' prefix and/or numeric ordering; "+ + "a bare lexical fallback wrongly orders %q < %q", c.a, c.b, c.a, c.b) + } +} + +func TestRequireAgentSupportsMCPAcceptsBarePrefixReport(t *testing.T) { + agent := &model.Server{Host: &model.Host{Version: "2.1.0"}} + require.NoError(t, requireAgentSupportsMCP(agent), + "agents report Host.Version without the 'v' prefix (e.g. \"2.1.0\"); it must compare equal to MCPMinAgentVersion \"v2.1.0\" and be accepted") +} diff --git a/cmd/dashboard/controller/mcp_error_structured_test.go b/cmd/dashboard/controller/mcp_error_structured_test.go new file mode 100644 index 00000000..fd457c20 --- /dev/null +++ b/cmd/dashboard/controller/mcp_error_structured_test.go @@ -0,0 +1,67 @@ +package controller + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/nezhahq/nezha/model" + "github.com/nezhahq/nezha/service/singleton" +) + +// A tool error must not ship structuredContent: strict clients validate it +// against the tool's outputSchema (which requires exec/fs result fields) and +// reject the whole response with -32602, masking the real isError text. +func TestServerExec_ErrorResult_OmitsStructuredContent(t *testing.T) { + cleanup, uid := setupMCPTest(t) + defer cleanup() + + srv, _ := singleton.ServerShared.Get(7) + srv.SetTaskStream(&execErrorStream{errMsg: "agent disabled command execution"}) + + tok, _ := mkToken(t, uid, []string{model.ScopeServerExec}, nil) + + c, w := mcpCallCtx(t, tok, uid, jsonRPCRequest{ + JSONRPC: "2.0", ID: json.RawMessage("1"), Method: "tools/call", + Params: jsonObj(t, toolCallParams{ + Name: "server.exec", + Arguments: jsonRaw(map[string]any{ + "server_id": 7, + "cmd": "whoami", + "timeout_seconds": 2, + }), + }), + }) + mcpEndpoint(c) + + _, tcr := decodeRPC(w) + require.NotNil(t, tcr) + require.True(t, tcr.IsError) + require.Contains(t, tcr.Content[0].Text, "agent disabled command execution", + "error text must carry the real cause") + require.Nil(t, tcr.StructuredContent, + "error responses must omit structuredContent so strict clients don't validate it against outputSchema and mask the real error") +} + +func TestScopeDenied_OmitsStructuredContent(t *testing.T) { + cleanup, uid := setupMCPTest(t) + defer cleanup() + + tok, _ := mkToken(t, uid, []string{model.ScopeServerRead}, nil) + + c, w := mcpCallCtx(t, tok, uid, jsonRPCRequest{ + JSONRPC: "2.0", ID: json.RawMessage("1"), Method: "tools/call", + Params: jsonObj(t, toolCallParams{ + Name: "server.exec", + Arguments: jsonRaw(map[string]any{"server_id": 7, "cmd": "echo"}), + }), + }) + mcpEndpoint(c) + + _, tcr := decodeRPC(w) + require.NotNil(t, tcr) + require.True(t, tcr.IsError) + require.Nil(t, tcr.StructuredContent, + "scope-denied error must also omit structuredContent") +}