Skip to content

fix(tools): fall back to string for unsupported MCP parameter types#38788

Open
AnnaSuSu wants to merge 1 commit into
langgenius:mainfrom
AnnaSuSu:fix/mcp-unsupported-param-type
Open

fix(tools): fall back to string for unsupported MCP parameter types#38788
AnnaSuSu wants to merge 1 commit into
langgenius:mainfrom
AnnaSuSu:fix/mcp-unsupported-param-type

Conversation

@AnnaSuSu

@AnnaSuSu AnnaSuSu commented Jul 11, 2026

Copy link
Copy Markdown

Important

  1. Make sure you have read our contribution guidelines
  2. Ensure there is an associated issue and you have been assigned to it
  3. Use the correct syntax to link this PR: Fixes #<issue number>.

Summary

No issue (self-reported crash found by reading the MCP tool-conversion path).

Loading or invoking an MCP tool provider crashes with an unhandled ValueError when a
remote MCP server declares a parameter whose JSON-schema type is not one of the supported
parameter types.

A tool's inputSchema is supplied verbatim by a third-party MCP server and persisted as-is
(only description is normalized before storage in MCPToolManageService). When that schema
uses a non-standard type name — e.g. "text", a capitalized "String", or a ["bogus", "null"]
list — convert_mcp_schema_to_parameter passes it straight into
ToolParameter.ToolParameterType(prop_type), which raises:

ValueError: 'text' is not a valid ToolParameter.ToolParameterType

This propagates out of MCPToolProviderController.from_entity / from_db, which sits on every
provider-load path — listing the provider's tools, updating its credentials
(update_provider_credentials), and invoking a tool via
ToolManager.get_mcp_provider_controller. A single malformed tool therefore makes the whole
provider unusable, and because the bad schema is already stored, the crash repeats on every
subsequent load.

resolve_property_type (added in #33258) was designed to always resolve to a usable type,
falling back to "string" for cyclic schemas, over-depth unions, null, and empty type lists.
The two branches that echo the schema's raw type (case str() and the first non-null entry of
case list()) are the only ones that don't validate their result against the supported set, so
an unsupported name slips through to the enum constructor.

The fix extends that same fallback to the one case it missed: after alias mapping
(integer/floatnumber), a type outside ToolParameterType falls back to "string",
mirroring how every other unresolved type is already handled. Standard types are unaffected —
integer still resolves to number, object/array are preserved.

Adds a regression test that is red on an unmodified checkout (with the ValueError above) and
green with this fix, and confirms the existing null / anyOf / oneOf / allOf / depth-limit
cases still pass.

Screenshots

N/A — backend-only change.

Checklist

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran make lint && make type-check (backend) and cd web && pnpm exec vp staged (frontend) to appease the lint gods

From Claude Code

@AnnaSuSu AnnaSuSu requested a review from QuantumGhost as a code owner July 11, 2026 06:20
@dosubot dosubot Bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant