Skip to content

fix(ai): tolerate malformed tool parameter schemas - #2864

Open
yyqdbngt wants to merge 1 commit into
apache:rocketmq-studiofrom
yyqdbngt:codex/yy-ai-tool-schema
Open

fix(ai): tolerate malformed tool parameter schemas#2864
yyqdbngt wants to merge 1 commit into
apache:rocketmq-studiofrom
yyqdbngt:codex/yy-ai-tool-schema

Conversation

@yyqdbngt

Copy link
Copy Markdown

Summary

  • Guard buildToolInputTemplate() against tools whose parameters object is missing or not an object (defensive read via the existing isRecord helper)
  • Export buildToolInputTemplate() for unit testing (same precedent as the AiMessage export in this page)
  • Add unit tests covering schema defaults/enums/type fallbacks, missing/broken parameters, and required fields absent from properties

Why

Tool metadata comes from the MCP catalog over the wire, so the McpTool.parameters shape is not guaranteed. buildToolInputTemplate() dereferenced tool.parameters.required without a type check: a tool entry with a missing or malformed parameters field threw a TypeError inside loadTools(), failing the whole catalog load (and the automatic first-tool selection) even though every other tool was fine.

Testing

  • ./node_modules/.bin/vitest run src/pages/ai/__tests__/ → 17 passed (3 new)
  • ./node_modules/.bin/tsc --noEmit → clean
  • ./node_modules/.bin/eslint src/pages/ai/index.tsx src/pages/ai/__tests__/toolInputTemplate.test.ts → 0 errors

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Clean refactor that extracts buildToolInputTemplate as a standalone exported function and adds defensive null checks for tool.parameters. The extraction improves testability.

Findings

  • [Info] ai/index.tsx — The extraction of buildToolInputTemplate is clean and the function signature is well-typed.
  • [Info] The null check on tool.parameters prevents potential NPE when a tool has no parameters defined.
  • [Info] Good test coverage including the edge case of null parameters.

Suggestions

No blocking issues. Nice improvement in code organization and null safety.


Automated review by github-manager-bot

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — defensive fix improving input validation and error handling.


Automated review by "github-manager-bot"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants