Skip to content

fix: prevent $defs mutation in Tool.from_tool transforms#2493

Merged
jlowin merged 1 commit intoPrefectHQ:mainfrom
jtanningbed:fix/tool-transform-defs-mutation
Nov 26, 2025
Merged

fix: prevent $defs mutation in Tool.from_tool transforms#2493
jlowin merged 1 commit intoPrefectHQ:mainfrom
jtanningbed:fix/tool-transform-defs-mutation

Conversation

@jtanningbed
Copy link
Copy Markdown
Contributor

Fixes #2492

Description

_create_forwarding_transform passes a reference to the parent tool's $defs dict to compress_schema, which mutates it in-place when pruning unused definitions. This corrupts the parent's schema when child tools hide parameters that remove all $ref usage.

The fix deep copies parent_defs before passing to compress_schema.

Contributors Checklist

Review Checklist

  • I have self-reviewed my changes
  • My Pull Request is ready for review

Deep copy parent_defs before passing to compress_schema to prevent
mutation from affecting parent tool schemas when child tools hide
parameters that remove all $ref usage.
@marvin-context-protocol marvin-context-protocol Bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. server Related to FastMCP server implementation or server-side functionality. labels Nov 26, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 26, 2025

Walkthrough

The change modifies src/fastmcp/tools/tool_transform.py to fix schema corruption when creating transformed tools. It adds a deep copy of the parent tool's $defs dictionary when constructing the transformed tool's schema. This prevents in-place mutations by compress_schema from modifying the parent tool's original $defs. A deepcopy import is introduced to support this behavior. No public API or exported entities are altered.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main fix: preventing $defs mutation during Tool.from_tool transformations, which is the core issue addressed in the PR.
Description check ✅ Passed The PR description includes all required sections from the template: references #2492, provides clear context of the problem and fix, completes all checklist items, and demonstrates self-review and readiness.
Linked Issues check ✅ Passed The PR fix directly addresses issue #2492 by implementing the required solution: deep copying parent_defs before passing to compress_schema prevents in-place mutation of the parent tool's schema.
Out of Scope Changes check ✅ Passed The changes are narrowly scoped to fix the identified issue: adding deepcopy import and deep copying parent_defs in _create_forwarding_transform to prevent schema corruption.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fd02970 and ae0b44f.

📒 Files selected for processing (1)
  • src/fastmcp/tools/tool_transform.py (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Run tests: Python 3.10 on ubuntu-latest
  • GitHub Check: Run tests with lowest-direct dependencies
  • GitHub Check: Run tests: Python 3.10 on windows-latest
🔇 Additional comments (2)
src/fastmcp/tools/tool_transform.py (2)

7-7: LGTM! Import added to support the fix.

The deepcopy import is correctly placed and necessary for preventing mutation of the parent tool's $defs dictionary.


624-625: Excellent fix! Prevents schema corruption via defensive copying.

The deepcopy prevents compress_schema from mutating the parent tool's $defs in-place when pruning unused definitions. This directly addresses the root cause described in issue #2492, where hiding parameters that reference $defs entries would corrupt the parent's schema.

Using deepcopy rather than a shallow copy is the right choice here to ensure complete isolation of nested schema structures, and the performance impact is negligible for setup-time operations.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jlowin jlowin merged commit d770a76 into PrefectHQ:main Nov 26, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. server Related to FastMCP server implementation or server-side functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tool.from_tool with hidden parameters corrupts parent tool's $defs schema

2 participants