fix: normalize invalid MCP required flags in MCP schemas#6077
fix: normalize invalid MCP required flags in MCP schemas#6077stablegenius49 wants to merge 1 commit intoAstrBotDevs:masterfrom
Conversation
dosubot
bot
added
size:L
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并且给出了一些整体性的反馈：
_normalize_mcp_input_schema中使用的seen: set[str] = set()/if not (name in seen or seen.add(name))这种模式有点晦涩；建议采用更易读的去重方式（例如在构建新列表时使用if name not in seen: seen.add(name); result.append(name)），以使意图更加清晰。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- `_normalize_mcp_input_schema` 中使用的 `seen: set[str] = set()` / `if not (name in seen or seen.add(name))` 这种模式有点晦涩；建议采用更易读的去重方式（例如在构建新列表时使用 `if name not in seen: seen.add(name); result.append(name)`），以使意图更加清晰。
## Individual Comments
### Comment 1
<location path="astrbot/core/agent/mcp_client.py" line_range="146-150" />
<code_context>
+ required_list.append(prop_name)
+
+ if required_list:
+ seen: set[str] = set()
+ normalized["required"] = [
+ name
+ for name in required_list
+ if not (name in seen or seen.add(name))
+ ]
+ elif isinstance(required, list):
</code_context>
<issue_to_address>
**nitpick:** 在推导式中使用 `seen.add` 做去重的小技巧会影响可读性。
这种习惯用法对很多读者来说不太好理解，而 `required_list` 的规模很小且使用频率不高，因此这类微优化没有必要。建议采用更清晰的方式——例如显式循环，或者使用 `normalized["required"] = list(dict.fromkeys(required_list))`——在保持顺序的同时完成去重，对性能影响可以忽略不计。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The
seen: set[str] = set()/if not (name in seen or seen.add(name))pattern in_normalize_mcp_input_schemais a bit opaque; consider a more readable deduplication approach (e.g., building a new list while checkingif name not in seen: seen.add(name); result.append(name)) to make the intent clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `seen: set[str] = set()` / `if not (name in seen or seen.add(name))` pattern in `_normalize_mcp_input_schema` is a bit opaque; consider a more readable deduplication approach (e.g., building a new list while checking `if name not in seen: seen.add(name); result.append(name)`) to make the intent clearer.
## Individual Comments
### Comment 1
<location path="astrbot/core/agent/mcp_client.py" line_range="146-150" />
<code_context>
+ required_list.append(prop_name)
+
+ if required_list:
+ seen: set[str] = set()
+ normalized["required"] = [
+ name
+ for name in required_list
+ if not (name in seen or seen.add(name))
+ ]
+ elif isinstance(required, list):
</code_context>
<issue_to_address>
**nitpick:** The deduplication trick using `seen.add` in a comprehension impacts readability.
This idiom is hard to parse for many readers, and the small size / infrequent use of `required_list` means the micro-optimization isn’t needed. Consider a clearer approach—e.g. an explicit loop or `normalized["required"] = list(dict.fromkeys(required_list))`—to deduplicate while preserving order with negligible performance impact.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| seen: set[str] = set() | ||
| normalized["required"] = [ | ||
| name | ||
| for name in required_list | ||
| if not (name in seen or seen.add(name)) |
There was a problem hiding this comment.
nitpick: 在推导式中使用 seen.add 做去重的小技巧会影响可读性。
这种习惯用法对很多读者来说不太好理解，而 required_list 的规模很小且使用频率不高，因此这类微优化没有必要。建议采用更清晰的方式——例如显式循环，或者使用 normalized["required"] = list(dict.fromkeys(required_list))——在保持顺序的同时完成去重，对性能影响可以忽略不计。
Original comment in English
nitpick: The deduplication trick using seen.add in a comprehension impacts readability.
This idiom is hard to parse for many readers, and the small size / infrequent use of required_list means the micro-optimization isn’t needed. Consider a clearer approach—e.g. an explicit loop or normalized["required"] = list(dict.fromkeys(required_list))—to deduplicate while preserving order with negligible performance impact.
|
Related Documentation 1 document(s) may need updating based on files changed in this PR: AstrBotTeam's Space pr4697的改动View Suggested Changes@@ -413,6 +413,20 @@
- 环境变量 `ASTRBOT_MCP_INIT_STRICT` 可配置为 `1`、`true`、`yes` 或 `on` 启用严格模式
- 严格模式下，如果所有 MCP 服务初始化失败，系统会中止启动
- 非严格模式下（默认），MCP 初始化失败仅记录警告，系统继续启动
+
+**工具 Schema 规范化（PR #6077）：**
+
+[PR #6077](https://github.com/AstrBotDevs/AstrBot/pull/6077) 新增了 MCP 工具 JSON Schema 规范化功能，提升与非标准 MCP 服务器的兼容性（[Issue #5972](https://github.com/AstrBotDevs/AstrBot/issues/5972)）：
+
+- **问题背景**：部分 MCP 服务器提供的 JSON Schema 不符合 Draft 2020-12 规范，在属性级别使用布尔值 `required: true/false` 标记必填字段，而非在父对象中声明 `required` 数组。这种非标准格式会导致 schema 验证失败。
+- **规范化逻辑**：新增 `_normalize_mcp_input_schema()` 函数，在 `MCPTool` 对象初始化前自动规范化 JSON Schema：
+ - 递归检测属性级别的 `required` 布尔标志
+ - 将 `required: true` 的属性名提升到父对象的 `required` 数组
+ - 移除属性自身的 `required` 布尔值
+ - 自动去重，确保 `required` 数组中无重复项
+ - 支持嵌套对象的 schema 规范化
+- **验证策略**：保持严格的 schema 验证，而非完全禁用验证，确保对真正的 schema 错误仍能正确报告
+- **兼容性提升**：修复后，系统可以正常处理来自非标准 MCP 服务器的工具定义，无需手动修改 schema 或放宽验证规则
#### 后台任务执行流程（PR #5722）
Note: You must be authenticated to accept/decline updates. How did I do? Any feedback? |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where certain MCP servers provide JSON schemas that use a non-standard boolean Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a normalization step for non-standard required flags in MCP input schemas, which is a good enhancement for compatibility. The implementation uses a recursive function to traverse and correct the schema, and it's well-supported by new unit tests. My review includes a couple of suggestions to improve the efficiency and readability of the new normalization function.
| seen: set[str] = set() | ||
| normalized["required"] = [ | ||
| name | ||
| for name in required_list | ||
| if not (name in seen or seen.add(name)) | ||
| ] |
There was a problem hiding this comment.
The method used for de-duplicating required_list while preserving order is a bit verbose. For Python 3.7+, you can use dict.fromkeys() which is a more modern, concise, and idiomatic way to achieve the same result.
| seen: set[str] = set() | |
| normalized["required"] = [ | |
| name | |
| for name in required_list | |
| if not (name in seen or seen.add(name)) | |
| ] | |
| normalized["required"] = list(dict.fromkeys(required_list)) |
|
|
||
| return normalized | ||
|
|
||
| return _normalize(copy.deepcopy(schema)) |
There was a problem hiding this comment.
The _normalize function is pure and does not mutate its input; it returns a new, modified copy of the schema. Therefore, the copy.deepcopy() call here is redundant. Removing it will improve performance by avoiding an unnecessary full copy of the schema object, which can be significant for large schemas.
| return _normalize(copy.deepcopy(schema)) | |
| return _normalize(schema) |
1 participant
Summary
required: true/falseflags into the parent object'srequiredarray before validating tool schemasTesting
Closes #5972
Summary by Sourcery
规范化 MCP 工具的输入模式，以在保留严格校验的同时处理非标准的
required标记。Bug 修复：
required数组中进行修正。测试：
required标记时与 MCPTool 的集成测试。Original summary in English
Summary by Sourcery
Normalize MCP tool input schemas to handle non-standard required flags while preserving strict validation.
Bug Fixes:
Tests: