mirror of
https://github.com/Buriburizaem0n/nezha_domains.git
synced 2026-09-19 17:50:12 +00:00
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.
This commit is contained in:
@@ -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,
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
Reference in New Issue
Block a user