From ed113b4909a250f6754e9d11754b19c50787bb58 Mon Sep 17 00:00:00 2001 From: SivanCola <32437197+SivanCola@users.noreply.github.com> Date: Tue, 14 Jul 2026 12:17:57 +0800 Subject: [PATCH] fix(mcp): quarantine tools with invalid schemas --- desktop/app.go | 5 +- desktop/app_test.go | 10 ++++ .../capabilities-panel-actions.test.ts | 4 ++ .../src/components/CapabilitiesPanel.tsx | 24 ++++++--- desktop/frontend/src/lib/types.ts | 1 + desktop/frontend/src/locales/en.ts | 2 + desktop/frontend/src/locales/zh-TW.ts | 2 + desktop/frontend/src/locales/zh.ts | 2 + desktop/frontend/src/styles.css | 10 ++++ desktop/go.mod | 1 + desktop/go.sum | 4 ++ go.mod | 1 + go.sum | 4 ++ internal/cli/mcp_manager_view.go | 6 ++- internal/cli/mcp_test.go | 20 +++++++ internal/cli/mcp_view.go | 27 ++++++++++ internal/plugin/cache.go | 14 +++++ internal/plugin/cache_test.go | 28 ++++++++++ internal/plugin/plugin.go | 31 ++++++++++- internal/plugin/plugin_test.go | 52 ++++++++++++++++++ internal/provider/schema_canonicalize.go | 6 +++ internal/provider/schema_canonicalize_test.go | 1 + internal/provider/schema_validate.go | 43 +++++++++++++++ internal/provider/schema_validate_test.go | 54 +++++++++++++++++++ 24 files changed, 341 insertions(+), 11 deletions(-) create mode 100644 internal/provider/schema_validate.go create mode 100644 internal/provider/schema_validate_test.go diff --git a/desktop/app.go b/desktop/app.go index 8b606d3308..fde85e20d6 100644 --- a/desktop/app.go +++ b/desktop/app.go @@ -5826,6 +5826,7 @@ type ToolView struct { Name string `json:"name"` Description string `json:"description"` ReadOnlyHint bool `json:"readOnlyHint,omitempty"` + SchemaError string `json:"schemaError,omitempty"` } // SkillView is one discoverable skill for the drawer. Also backs the @@ -7203,7 +7204,9 @@ func pluginToolsToView(tools []plugin.ToolInfo) []ToolView { } out := make([]ToolView, 0, len(tools)) for _, t := range tools { - out = append(out, ToolView{Name: t.Name, Description: t.Description, ReadOnlyHint: t.ReadOnlyHint}) + out = append(out, ToolView{ + Name: t.Name, Description: t.Description, ReadOnlyHint: t.ReadOnlyHint, SchemaError: t.SchemaError, + }) } return out } diff --git a/desktop/app_test.go b/desktop/app_test.go index 74b0ccd664..6ac3a28278 100644 --- a/desktop/app_test.go +++ b/desktop/app_test.go @@ -36,6 +36,16 @@ import ( "reasonix/internal/tool" ) +func TestPluginToolsToViewPreservesSchemaError(t *testing.T) { + got := pluginToolsToView([]plugin.ToolInfo{{ + Name: "generate_yso_bytes", Description: "Generate payload", ReadOnlyHint: true, + SchemaError: "invalid input schema: bad nested type", + }}) + if len(got) != 1 || got[0].SchemaError != "invalid input schema: bad nested type" { + t.Fatalf("tool views = %+v", got) + } +} + func desktopMCPHTTPServer(t *testing.T) *httptest.Server { t.Helper() return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/desktop/frontend/src/__tests__/capabilities-panel-actions.test.ts b/desktop/frontend/src/__tests__/capabilities-panel-actions.test.ts index a89b3f61c3..d2d299eea4 100644 --- a/desktop/frontend/src/__tests__/capabilities-panel-actions.test.ts +++ b/desktop/frontend/src/__tests__/capabilities-panel-actions.test.ts @@ -207,6 +207,7 @@ console.log("capabilities panel MCP actions"); toolList: [ { name: "issue_read", description: "Read issues.", readOnlyHint: true }, { name: "issue_write", description: "Write issues." }, + { name: "broken_read", description: "Broken tool.", readOnlyHint: true, schemaError: "invalid input schema: bad nested type" }, ], trustedReadOnlyTools: [], }]; @@ -269,6 +270,9 @@ console.log("capabilities panel MCP actions"); }); await waitFor("trusted badge", () => Boolean(document.querySelector(".cap-tool-trust")?.textContent?.includes("Trusted"))); + await waitFor("unavailable tool", () => Boolean(document.querySelector(".cap-tool-hint--error")?.textContent?.includes("Unavailable"))); + ok(document.body.textContent?.includes("1 unavailable"), "server summary reports one quarantined tool"); + ok(document.body.textContent?.includes("invalid input schema: bad nested type"), "tool list shows the schema diagnostic"); const untrust = findButton("Untrust"); if (!untrust) throw new Error("missing Untrust button"); await act(async () => { diff --git a/desktop/frontend/src/components/CapabilitiesPanel.tsx b/desktop/frontend/src/components/CapabilitiesPanel.tsx index 3aa8ef9be4..b71942ca40 100644 --- a/desktop/frontend/src/components/CapabilitiesPanel.tsx +++ b/desktop/frontend/src/components/CapabilitiesPanel.tsx @@ -1,5 +1,5 @@ import { useCallback, useEffect, useMemo, useState } from "react"; -import { ShieldCheck, ShieldOff } from "lucide-react"; +import { CircleAlert, ShieldCheck, ShieldOff } from "lucide-react"; import { asArray } from "../lib/array"; import { app, openExternal } from "../lib/bridge"; import { useT } from "../lib/i18n"; @@ -894,6 +894,7 @@ function ServerRow({ const actionLabel = serverActionLabel(s, t); const lifecycle = mcpServerLifecycleActions(s); const tools = s.toolList ?? []; + const schemaIssueCount = tools.filter((tool) => tool.schemaError).length; let sub = s.status === "failed" ? s.error || t("caps.failed") @@ -906,6 +907,9 @@ function ServerRow({ ? t("caps.disabledAutoStart") : t("caps.disabled") : t("caps.counts", { tools: s.tools, prompts: s.prompts, resources: s.resources }); + if (schemaIssueCount > 0) { + sub = `${sub} · ${t("caps.schemaIssues", { count: schemaIssueCount })}`; + } if (s.managedByPlugin) { sub = `${sub} · ${t("caps.managedByPlugin", { plugin: s.managedByPlugin })}`; } @@ -1034,7 +1038,7 @@ function ServerDetails({ const trustedReadOnlyTools = s.trustedReadOnlyTools ?? []; const trustedReadOnlyToolNames = new Set(trustedReadOnlyTools); const canTrustTool = canMutateConfig; - const reportedReadOnlyToolNames = (tools ?? []).filter((tool) => tool.readOnlyHint).map((tool) => tool.name); + const reportedReadOnlyToolNames = (tools ?? []).filter((tool) => tool.readOnlyHint && !tool.schemaError).map((tool) => tool.name); const bulkTrustToolNames = reportedReadOnlyToolNames.filter((name) => !trustedReadOnlyToolNames.has(name)); if (editing && canEditConfig) { return ( @@ -1144,19 +1148,25 @@ function ServerDetails({
{t("caps.tools")}
{tools.map((tool) => { const trusted = trustedReadOnlyToolNames.has(tool.name); + const unavailable = Boolean(tool.schemaError); return ( -
+
{tool.name}
- {tool.description} - {tool.readOnlyHint && ( + {unavailable ? tool.schemaError : tool.description} + {unavailable ? ( + + + {t("caps.toolUnavailable")} + + ) : tool.readOnlyHint ? ( {t("caps.reportedReadOnly")} - )} + ) : null}
- {canTrustTool ? ( + {!unavailable && canTrustTool ? ( trusted ? (
diff --git a/desktop/frontend/src/lib/types.ts b/desktop/frontend/src/lib/types.ts index c51a81a006..9d0f3bcd24 100644 --- a/desktop/frontend/src/lib/types.ts +++ b/desktop/frontend/src/lib/types.ts @@ -696,6 +696,7 @@ export interface MCPToolView { name: string; description: string; readOnlyHint?: boolean; + schemaError?: string; } export interface SkillView { name: string; diff --git a/desktop/frontend/src/locales/en.ts b/desktop/frontend/src/locales/en.ts index b5de962b5d..42a1b505c6 100644 --- a/desktop/frontend/src/locales/en.ts +++ b/desktop/frontend/src/locales/en.ts @@ -234,6 +234,7 @@ export const en = { "caps.rawLog": "Raw log", "caps.copyLog": "Copy log", "caps.counts": "{tools} tools · {prompts} prompts · {resources} resources", + "caps.schemaIssues": "{count} unavailable", "caps.connected": "connected", "caps.failed": "failed to connect", "caps.tools": "Tools", @@ -307,6 +308,7 @@ export const en = { "caps.expandTools": "Expand tools", "caps.collapseTools": "Collapse tools", "caps.noToolDetails": "This connection did not return tool details.", + "caps.toolUnavailable": "Unavailable", "caps.trustReadOnly": "Pre-trust", "caps.trustedReadOnly": "Trusted", "caps.trustedReadOnlyTools": "Remembered read-only", diff --git a/desktop/frontend/src/locales/zh-TW.ts b/desktop/frontend/src/locales/zh-TW.ts index 0a35676174..6008ea0ab7 100644 --- a/desktop/frontend/src/locales/zh-TW.ts +++ b/desktop/frontend/src/locales/zh-TW.ts @@ -150,6 +150,7 @@ export const zhTW: Record = { "caps.rawLog": "原始日誌", "caps.copyLog": "複製日誌", "caps.counts": "{tools} 工具 · {prompts} 提示 · {resources} 資源", + "caps.schemaIssues": "{count} 個不可用", "caps.connected": "已連線", "caps.failed": "連線失敗", "caps.tools": "工具", @@ -223,6 +224,7 @@ export const zhTW: Record = { "caps.expandTools": "展開工具列表", "caps.collapseTools": "收起工具列表", "caps.noToolDetails": "當前連線未返回工具明細。", + "caps.toolUnavailable": "不可用", "caps.trustReadOnly": "預先信任", "caps.trustedReadOnly": "已信任", "caps.trustedReadOnlyTools": "已記住唯讀", diff --git a/desktop/frontend/src/locales/zh.ts b/desktop/frontend/src/locales/zh.ts index 1ffea558cb..ca58c578a4 100644 --- a/desktop/frontend/src/locales/zh.ts +++ b/desktop/frontend/src/locales/zh.ts @@ -235,6 +235,7 @@ export const zh: Record = { "caps.rawLog": "原始日志", "caps.copyLog": "复制日志", "caps.counts": "{tools} 工具 · {prompts} 提示 · {resources} 资源", + "caps.schemaIssues": "{count} 个不可用", "caps.connected": "已连接", "caps.failed": "连接失败", "caps.tools": "工具", @@ -308,6 +309,7 @@ export const zh: Record = { "caps.expandTools": "展开工具列表", "caps.collapseTools": "收起工具列表", "caps.noToolDetails": "当前连接未返回工具明细。", + "caps.toolUnavailable": "不可用", "caps.trustReadOnly": "预先信任", "caps.trustedReadOnly": "已信任", "caps.trustedReadOnlyTools": "已记住只读", diff --git a/desktop/frontend/src/styles.css b/desktop/frontend/src/styles.css index 48351c6fb3..bb8cdbe598 100644 --- a/desktop/frontend/src/styles.css +++ b/desktop/frontend/src/styles.css @@ -10863,6 +10863,16 @@ body > .mermaid-diagram--fullscreen { line-height: 1; white-space: nowrap; } +.cap-tool-hint--error { + gap: 3px; + border-color: color-mix(in srgb, var(--warn) 34%, transparent); + color: var(--warn); + background: color-mix(in srgb, var(--warn) 8%, transparent); +} +.cap-tool--unavailable .cap-tool__name, +.cap-tool--unavailable .cap-tool__desc { + color: var(--fg-faint); +} .cap-tool-trust-stack { display: flex; flex-wrap: wrap; diff --git a/desktop/go.mod b/desktop/go.mod index fe4400bc95..2f87d51792 100644 --- a/desktop/go.mod +++ b/desktop/go.mod @@ -52,6 +52,7 @@ require ( github.com/rivo/uniseg v0.4.7 // indirect github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06 // indirect github.com/samber/lo v1.53.0 // indirect + github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 // indirect github.com/tkrajina/go-reflector v0.5.8 // indirect github.com/tree-sitter/go-tree-sitter v0.25.0 // indirect github.com/tree-sitter/tree-sitter-javascript v0.25.0 // indirect diff --git a/desktop/go.sum b/desktop/go.sum index 2d7e4ff7d8..d39fb33449 100644 --- a/desktop/go.sum +++ b/desktop/go.sum @@ -20,6 +20,8 @@ github.com/danieljoos/wincred v1.2.3/go.mod h1:6qqX0WNrS4RzPZ1tnroDzq9kY3fu1KwE7 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI= +github.com/dlclark/regexp2 v1.11.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= github.com/go-ole/go-ole v1.3.0 h1:Dt6ye7+vXGIKZ7Xtk4s6/xVdGDQynvom7xCFEdWr6uE= github.com/go-ole/go-ole v1.3.0/go.mod h1:5LS6F96DhAwUc7C+1HLexzMXY1xGRSryjyPPKW6zv78= github.com/go-quicktest/qt v1.101.0 h1:O1K29Txy5P2OK0dGo59b7b0LR6wKfIhttaAhHUyn7eI= @@ -89,6 +91,8 @@ github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06 h1:OkMGxebDj github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06/go.mod h1:+ePHsJ1keEjQtpvf9HHw0f4ZeJ0TLRsxhunSI2hYJSs= github.com/samber/lo v1.53.0 h1:t975lj2py4kJPQ6haz1QMgtId2gtmfktACxIXArw3HM= github.com/samber/lo v1.53.0/go.mod h1:4+MXEGsJzbKGaUEQFKBq2xtfuznW9oz/WrgyzMzRoM0= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 h1:KRzFb2m7YtdldCEkzs6KqmJw4nqEVZGK7IN2kJkjTuQ= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= diff --git a/go.mod b/go.mod index 1f458f1927..4851ad40f7 100644 --- a/go.mod +++ b/go.mod @@ -20,6 +20,7 @@ require ( github.com/mattn/go-runewidth v0.0.24 github.com/rivo/uniseg v0.4.7 github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06 + github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 github.com/spf13/pflag v1.0.6 github.com/tree-sitter/go-tree-sitter v0.25.0 github.com/tree-sitter/tree-sitter-javascript v0.25.0 diff --git a/go.sum b/go.sum index 8797860568..05c332151f 100644 --- a/go.sum +++ b/go.sum @@ -45,6 +45,8 @@ github.com/danieljoos/wincred v1.2.3/go.mod h1:6qqX0WNrS4RzPZ1tnroDzq9kY3fu1KwE7 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI= +github.com/dlclark/regexp2 v1.11.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= github.com/dlclark/regexp2/v2 v2.2.1 h1:mf4KkFUj0gJuarK8P+LgiS+Lit7m9N1yAwEfPbee7R0= github.com/dlclark/regexp2/v2 v2.2.1/go.mod h1:avUrQvPaLz2DrFNHJF0taWAFFX2C1GMSSoeiqFjcBmU= github.com/go-ole/go-ole v1.3.0 h1:Dt6ye7+vXGIKZ7Xtk4s6/xVdGDQynvom7xCFEdWr6uE= @@ -87,6 +89,8 @@ github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0t github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06 h1:OkMGxebDjyw0ULyrTYWeN0UNCCkmCWfjPnIA2W6oviI= github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06/go.mod h1:+ePHsJ1keEjQtpvf9HHw0f4ZeJ0TLRsxhunSI2hYJSs= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 h1:KRzFb2m7YtdldCEkzs6KqmJw4nqEVZGK7IN2kJkjTuQ= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU= github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/internal/cli/mcp_manager_view.go b/internal/cli/mcp_manager_view.go index 9115f79bd4..c77067766c 100644 --- a/internal/cli/mcp_manager_view.go +++ b/internal/cli/mcp_manager_view.go @@ -180,7 +180,11 @@ func (p *mcpManager) renderTools(width int) string { limit = mcpToolMaxRows } for _, t := range v.ToolList[:limit] { - desc := viewCompactText(t.Description, viewBudget(width, 24)) + desc := t.Description + if t.SchemaError != "" { + desc = "unavailable: " + t.SchemaError + } + desc = viewCompactText(desc, viewBudget(width, 24)) fmt.Fprintf(&b, " %-20s %s\n", t.Name, viewMeta(desc)) } if extra := len(v.ToolList) - limit; extra > 0 { diff --git a/internal/cli/mcp_test.go b/internal/cli/mcp_test.go index 62ce3fb3b9..f2bf7ec61c 100644 --- a/internal/cli/mcp_test.go +++ b/internal/cli/mcp_test.go @@ -230,6 +230,26 @@ func TestRenderMCPStatusCapsLongSections(t *testing.T) { } } +func TestRenderMCPStatusShowsQuarantinedTools(t *testing.T) { + got := renderMCPStatus(200, + []plugin.ServerStatus{{ + Name: "yakit", Transport: "stdio", Tools: 1, + ToolList: []plugin.ToolInfo{ + {Name: "echo", Description: "available"}, + {Name: "generate_yso_bytes", SchemaError: "invalid input schema: bad type at /properties/options/items/type"}, + }, + }}, + nil, + nil, + nil, + ) + for _, want := range []string{"1 tool", "1 unavailable tool", "unavailable tools", "generate_yso_bytes", "invalid input schema"} { + if !strings.Contains(got, want) { + t.Fatalf("rendered MCP status missing %q:\n%s", want, got) + } + } +} + func TestMCPCapabilitiesTextUsesAdvertisedTools(t *testing.T) { if got := mcpCapabilitiesText(mcpServerView{HasTools: true}); got != "tools" { t.Fatalf("mcpCapabilitiesText = %q, want tools", got) diff --git a/internal/cli/mcp_view.go b/internal/cli/mcp_view.go index bb9376d2fe..054ba61fc7 100644 --- a/internal/cli/mcp_view.go +++ b/internal/cli/mcp_view.go @@ -63,6 +63,10 @@ func writeMCPServer(b *strings.Builder, width int, s plugin.ServerStatus, prompt transport = "unknown" } meta := fmt.Sprintf("(%s) %s · %s · %s", transport, countText(s.Tools, "tool"), countText(len(prompts), "prompt"), countText(len(resources), "resource")) + invalidTools := invalidMCPTools(s.ToolList) + if len(invalidTools) > 0 { + meta += " · " + countText(len(invalidTools), "unavailable tool") + } name := viewCompactText(s.Name, viewBudget(width, 4+2+1+visibleWidth(meta))) fmt.Fprintf(b, " %s %s %s\n", accent("✓"), bold(name), viewMeta(meta)) if len(prompts) > 0 { @@ -71,6 +75,29 @@ func writeMCPServer(b *strings.Builder, width int, s plugin.ServerStatus, prompt if len(resources) > 0 { writeMCPResourceList(b, width, resources) } + if len(invalidTools) > 0 { + b.WriteString(viewSubhead(" unavailable tools") + "\n") + limit := len(invalidTools) + if limit > mcpMaxItemsPerSection { + limit = mcpMaxItemsPerSection + } + for _, t := range invalidTools[:limit] { + writeMCPItem(b, width, " ", t.Name, t.SchemaError) + } + if extra := len(invalidTools) - limit; extra > 0 { + fmt.Fprintf(b, " %s\n", viewMore(extra, "unavailable tools")) + } + } +} + +func invalidMCPTools(tools []plugin.ToolInfo) []plugin.ToolInfo { + out := make([]plugin.ToolInfo, 0) + for _, t := range tools { + if t.SchemaError != "" { + out = append(out, t) + } + } + return out } func writeMCPFailure(b *strings.Builder, width int, f plugin.Failure) { diff --git a/internal/plugin/cache.go b/internal/plugin/cache.go index fc631d0a2e..cf788a1517 100644 --- a/internal/plugin/cache.go +++ b/internal/plugin/cache.go @@ -131,9 +131,23 @@ func LoadCachedSchemaAny(name, expectedHash string) (cs *CachedSchema, ok bool, if out.Version != cacheVersion { return nil, false, false } + out.Tools = filterValidCachedTools(out.Tools) return &out, true, out.SpecHash == expectedHash } +func filterValidCachedTools(tools []CachedTool) []CachedTool { + out := make([]CachedTool, 0, len(tools)) + for _, t := range tools { + schema, err := normalizeAndValidateToolSchema(t.Schema) + if err != nil { + continue + } + t.Schema = schema + out = append(out, t) + } + return out +} + // SaveCachedSchema atomically writes cs under name. Best-effort: an error // is logged at debug level and dropped. Uses tmpfile + os.Rename in the // parent dir so a crash mid-write can't leave a half-written JSON behind diff --git a/internal/plugin/cache_test.go b/internal/plugin/cache_test.go index 5f2028f8f8..35e911214e 100644 --- a/internal/plugin/cache_test.go +++ b/internal/plugin/cache_test.go @@ -75,6 +75,34 @@ func TestCacheRoundTrip(t *testing.T) { } } +func TestCacheLoadQuarantinesMalformedToolSchema(t *testing.T) { + redirectCache(t) + spec := sampleSpec() + hash := SpecFingerprint(spec) + cs := sampleCachedSchema(hash) + cs.Tools = append(cs.Tools, CachedTool{ + Name: "generate_yso_bytes", + Schema: json.RawMessage(`{ + "type":"object", + "properties":{"options":{"type":"array","items":{"key":{"type":"string"},"type":{"type":"string"},"value":{"type":"string"}}}} + }`), + }) + + if err := SaveCachedSchema(spec.Name, cs); err != nil { + t.Fatalf("SaveCachedSchema: %v", err) + } + got, ok := LoadCachedSchema(spec.Name, hash) + if !ok { + t.Fatal("LoadCachedSchema: miss after save") + } + if len(got.Tools) != 1 || got.Tools[0].Name != "do_thing" { + t.Fatalf("cached tools = %+v, want only valid do_thing", got.Tools) + } + if schema := string(got.Tools[0].Schema); schema != `{"properties":{},"type":"object"}` { + t.Fatalf("valid cached schema = %s", schema) + } +} + func TestCacheInvalidatesOnSpecHashMismatch(t *testing.T) { redirectCache(t) spec := sampleSpec() diff --git a/internal/plugin/plugin.go b/internal/plugin/plugin.go index d627ee5c6c..c7c9143a50 100644 --- a/internal/plugin/plugin.go +++ b/internal/plugin/plugin.go @@ -22,6 +22,7 @@ import ( "time" "reasonix/internal/event" + "reasonix/internal/provider" "reasonix/internal/tool" ) @@ -554,6 +555,7 @@ type ToolInfo struct { Name string Description string ReadOnlyHint bool + SchemaError string } // ServerStatus summarises one connected server for the /mcp command. @@ -1169,18 +1171,25 @@ func (c *Client) listTools(ctx context.Context) ([]tool.Tool, error) { tools := make([]tool.Tool, 0, len(out)) for _, t := range out { hinted := t.Annotations != nil && t.Annotations.ReadOnlyHint + info := ToolInfo{Name: t.Name, Description: t.Description, ReadOnlyHint: hinted} + schema, err := normalizeAndValidateToolSchema(t.InputSchema) + if err != nil { + info.SchemaError = schemaValidationError(err) + toolInfos = append(toolInfos, info) + continue + } visibleName := t.Name if c.spec.StripRawPrefix != "" { visibleName = strings.TrimPrefix(visibleName, c.spec.StripRawPrefix) } - toolInfos = append(toolInfos, ToolInfo{Name: t.Name, Description: t.Description, ReadOnlyHint: hinted}) + toolInfos = append(toolInfos, info) trusted := c.spec.toolReadOnlyTrusted(t.Name, visibleName) tools = append(tools, &remoteTool{ client: c, name: toolName(c.name, visibleName), rawName: t.Name, desc: t.Description, - schema: canonicalizeSchema(t.InputSchema), + schema: schema, readOnly: c.spec.toolReadOnly(t.Name, visibleName, hinted), readOnlyTrusted: trusted, }) @@ -1193,6 +1202,24 @@ func (c *Client) listTools(ctx context.Context) ([]tool.Tool, error) { return append([]tool.Tool(nil), sortedTools...), nil } +func normalizeAndValidateToolSchema(raw json.RawMessage) (json.RawMessage, error) { + schema := canonicalizeSchema(raw) + if err := provider.ValidateToolSchema(schema); err != nil { + return nil, err + } + return schema, nil +} + +func schemaValidationError(err error) string { + const maxRunes = 512 + msg := strings.TrimSpace(err.Error()) + runes := []rune(msg) + if len(runes) > maxRunes { + msg = string(runes[:maxRunes]) + "..." + } + return "invalid input schema: " + msg +} + func (c *Client) listToolsRaw(ctx context.Context) ([]mcpTool, error) { res, err := c.call(ctx, "tools/list", map[string]any{}) if err != nil { diff --git a/internal/plugin/plugin_test.go b/internal/plugin/plugin_test.go index 2026ad4452..03bebc9139 100644 --- a/internal/plugin/plugin_test.go +++ b/internal/plugin/plugin_test.go @@ -365,6 +365,58 @@ func TestClientListToolsRetriesAdvertisedEmptyToolList(t *testing.T) { } } +func TestClientListToolsQuarantinesMalformedSchema(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + tr := &countingToolsTransport{raw: json.RawMessage(`{ + "tools":[ + {"name":"echo","description":"Still available.","inputSchema":{"type":"object","properties":{"msg":{"type":"string"}}}}, + {"name":"generate_yso_bytes","description":"Broken nested schema.","inputSchema":{"type":"object","properties":{"options":{"type":"array","items":{"key":{"type":"string"},"type":{"type":"string"},"value":{"type":"string"}}}}}} + ] + }`)} + c := &Client{name: "yakit", t: tr, spec: Spec{Name: "yakit"}, transport: "stdio"} + + tools, err := c.listTools(ctx) + if err != nil { + t.Fatalf("listTools: %v", err) + } + if len(tools) != 1 || tools[0].Name() != "mcp__yakit__echo" { + t.Fatalf("tools = %v, want only mcp__yakit__echo", names(tools)) + } + if got := string(tools[0].Schema()); got != `{"properties":{"msg":{"type":"string"}},"type":"object"}` { + t.Fatalf("valid sibling schema changed: %s", got) + } + if len(c.tools) != 2 { + t.Fatalf("tool status count = %d, want both advertised tools", len(c.tools)) + } + if c.tools[0].Name != "echo" || c.tools[0].SchemaError != "" { + t.Fatalf("valid tool status = %+v", c.tools[0]) + } + if c.tools[1].Name != "generate_yso_bytes" || !strings.Contains(c.tools[1].SchemaError, "/properties/options/items/type") { + t.Fatalf("quarantined tool status = %+v", c.tools[1]) + } +} + +func TestClientListToolsValidatesAfterCompatibilityNormalization(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + tr := &countingToolsTransport{raw: json.RawMessage(`{"tools":[{"name":"legacy","inputSchema":{"type":"object","properties":{"query":{"type":"string","required":true}}}}]}`)} + c := &Client{name: "legacy", t: tr, spec: Spec{Name: "legacy"}, transport: "stdio"} + + tools, err := c.listTools(ctx) + if err != nil { + t.Fatalf("listTools: %v", err) + } + if len(tools) != 1 { + t.Fatalf("tools = %v, want normalized legacy tool", names(tools)) + } + if got := string(tools[0].Schema()); got != `{"properties":{"query":{"type":"string"}},"type":"object"}` { + t.Fatalf("normalized schema = %s", got) + } +} + func TestSpecReadOnlyToolNamesMarksUnhintedToolsReadOnly(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() diff --git a/internal/provider/schema_canonicalize.go b/internal/provider/schema_canonicalize.go index ae745a188d..20e697d09f 100644 --- a/internal/provider/schema_canonicalize.go +++ b/internal/provider/schema_canonicalize.go @@ -19,6 +19,12 @@ func CanonicalizeSchema(raw json.RawMessage) json.RawMessage { if err := json.Unmarshal(raw, &v); err != nil { return raw } + if v == nil { + // A nil RawMessage persists as JSON null in the MCP schema cache. Treat + // both forms as the same no-argument schema so old cache entries remain + // usable and never reach a strict provider as parameters: null. + return json.RawMessage(`{"properties":{},"type":"object"}`) + } canon := canonicalizeSchemaValue(v) ensureRootObjectProperties(canon) b, err := json.Marshal(canon) diff --git a/internal/provider/schema_canonicalize_test.go b/internal/provider/schema_canonicalize_test.go index bf76344231..ff3a5e4e47 100644 --- a/internal/provider/schema_canonicalize_test.go +++ b/internal/provider/schema_canonicalize_test.go @@ -25,6 +25,7 @@ func TestCanonicalizeSchemaDropsNonArrayRequired(t *testing.T) { func TestCanonicalizeSchemaAddsEmptyPropertiesForNoArgumentObject(t *testing.T) { for _, raw := range []json.RawMessage{ nil, + json.RawMessage(`null`), json.RawMessage(`{"type":"object"}`), } { got := string(CanonicalizeSchema(raw)) diff --git a/internal/provider/schema_validate.go b/internal/provider/schema_validate.go new file mode 100644 index 0000000000..f7f45adff7 --- /dev/null +++ b/internal/provider/schema_validate.go @@ -0,0 +1,43 @@ +package provider + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + + jsonschema "github.com/santhosh-tekuri/jsonschema/v6" +) + +const toolSchemaResource = "urn:reasonix:tool-schema" + +// ValidateToolSchema compiles a provider-visible tool parameter schema without +// resolving external resources. MCP schemas default to draft-07 when they do +// not declare a dialect; explicit $schema declarations still take precedence. +func ValidateToolSchema(raw json.RawMessage) error { + decoder := json.NewDecoder(bytes.NewReader(raw)) + decoder.UseNumber() + var doc any + if err := decoder.Decode(&doc); err != nil { + return fmt.Errorf("invalid JSON: %w", err) + } + if err := decoder.Decode(&struct{}{}); err != io.EOF { + if err == nil { + return fmt.Errorf("invalid JSON: multiple values") + } + return fmt.Errorf("invalid JSON: %w", err) + } + if _, ok := doc.(map[string]any); !ok { + return fmt.Errorf("root must be an object") + } + + compiler := jsonschema.NewCompiler() + compiler.DefaultDraft(jsonschema.Draft7) + if err := compiler.AddResource(toolSchemaResource, doc); err != nil { + return fmt.Errorf("load schema: %w", err) + } + if _, err := compiler.Compile(toolSchemaResource); err != nil { + return fmt.Errorf("compile schema: %w", err) + } + return nil +} diff --git a/internal/provider/schema_validate_test.go b/internal/provider/schema_validate_test.go new file mode 100644 index 0000000000..12b086ba31 --- /dev/null +++ b/internal/provider/schema_validate_test.go @@ -0,0 +1,54 @@ +package provider + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestValidateToolSchemaAcceptsValidDrafts(t *testing.T) { + for _, raw := range []json.RawMessage{ + json.RawMessage(`{"type":"object","properties":{"query":{"type":"string"}}}`), + json.RawMessage(`{"$schema":"https://json-schema.org/draft/2020-12/schema","type":"object","properties":{"values":{"type":"array","prefixItems":[{"type":"string"}]}}}`), + } { + if err := ValidateToolSchema(raw); err != nil { + t.Fatalf("ValidateToolSchema(%s): %v", raw, err) + } + } +} + +func TestValidateToolSchemaRejectsMalformedNestedArrayItems(t *testing.T) { + raw := json.RawMessage(`{ + "type":"object", + "properties":{ + "options":{ + "type":"array", + "items":{ + "key":{"description":"option key","type":"string"}, + "type":{"description":"option type","type":"string"}, + "value":{"description":"option value","type":"string"} + } + } + } + }`) + err := ValidateToolSchema(raw) + if err == nil { + t.Fatal("malformed Yakit-style array item schema was accepted") + } + if msg := err.Error(); !strings.Contains(msg, "/properties/options/items/type") { + t.Fatalf("error does not identify the malformed nested type: %v", err) + } +} + +func TestValidateToolSchemaRejectsNonObjectRootAndExternalRefs(t *testing.T) { + for name, raw := range map[string]json.RawMessage{ + "array root": json.RawMessage(`[]`), + "external ref": json.RawMessage(`{"type":"object","properties":{"x":{"$ref":"https://example.com/schema.json"}}}`), + } { + t.Run(name, func(t *testing.T) { + if err := ValidateToolSchema(raw); err == nil { + t.Fatalf("ValidateToolSchema(%s) returned nil", raw) + } + }) + } +}