-
Notifications
You must be signed in to change notification settings - Fork 581
feat: Add tool annotations for improved LLM tool understanding #480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Add readOnlyHint and destructiveHint annotations to all 23 tools
to help LLMs better understand tool behavior and make safer decisions.
Changes:
- Added readOnlyHint: true to 5 read-only tools:
- debug_request_context, find_in_file, validate_script,
manage_script_capabilities, get_sha
- Added destructiveHint: true to 18 tools that modify data:
- All CRUD tools, execution tools, and state-modifying tools
- Added title annotations for human-readable display
- Imported ToolAnnotations from mcp.types in all tool files
This improves tool safety metadata for MCP clients.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Reviewer's GuideAdds MCP ToolAnnotations (title, readOnlyHint, destructiveHint) across all Unity MCP tools to improve client understanding of tool behavior and safety characteristics, plus a uv.lock update for the newer MCP dependency. Sequence diagram for LLM client using tool annotations for safety decisionssequenceDiagram
actor User
participant LlmClient
participant McpServer
participant UnityEditor
User->>LlmClient: Request Unity project change
LlmClient->>McpServer: List available tools
McpServer-->>LlmClient: Tool metadata with ToolAnnotations
LlmClient->>LlmClient: Classify tools by readOnlyHint and destructiveHint
LlmClient->>User: Present plan using read-only tools first
User->>LlmClient: Approve read-only steps
LlmClient->>McpServer: Call FindInFileTool (readOnlyHint true)
McpServer->>UnityEditor: Read file contents
UnityEditor-->>McpServer: File matches
McpServer-->>LlmClient: Read-only results
LlmClient->>LlmClient: Auto-approve further read-only tools
LlmClient->>User: Request confirmation for destructive tools
User->>LlmClient: Confirm destructive actions
LlmClient->>McpServer: Call ApplyTextEditsTool (destructiveHint true)
McpServer->>UnityEditor: Apply code edits
UnityEditor-->>McpServer: Edit success
McpServer-->>LlmClient: Operation result
LlmClient-->>User: Summarize changes and status
Class diagram for MCP tools using ToolAnnotationsclassDiagram
class ToolAnnotations {
+string title
+bool readOnlyHint
+bool destructiveHint
}
class ApplyTextEditsTool {
+Context ctx
+string uri
+string edits
+ToolAnnotations annotations
}
class CreateScriptTool {
+Context ctx
+string path
+string contents
+ToolAnnotations annotations
}
class DeleteScriptTool {
+Context ctx
+string uri
+ToolAnnotations annotations
}
class ValidateScriptTool {
+Context ctx
+string uri
+ToolAnnotations annotations
}
class ManageScriptTool {
+Context ctx
+string action
+ToolAnnotations annotations
}
class ManageScriptCapabilitiesTool {
+Context ctx
+ToolAnnotations annotations
}
class GetShaTool {
+Context ctx
+string uri
+ToolAnnotations annotations
}
class DebugRequestContextTool {
+Context ctx
+ToolAnnotations annotations
}
class FindInFileTool {
+Context ctx
+string uri
+string pattern
+ToolAnnotations annotations
}
class ExecuteMenuItemTool {
+Context ctx
+string menu_path
+ToolAnnotations annotations
}
class ManageAssetTool {
+Context ctx
+string action
+ToolAnnotations annotations
}
class ManageEditorTool {
+Context ctx
+string action
+ToolAnnotations annotations
}
class ManageGameobjectTool {
+Context ctx
+string action
+ToolAnnotations annotations
}
class ManageMaterialTool {
+Context ctx
+string action
+ToolAnnotations annotations
}
class ManagePrefabsTool {
+Context ctx
+string action
+ToolAnnotations annotations
}
class ManageSceneTool {
+Context ctx
+string action
+ToolAnnotations annotations
}
class ManageShaderTool {
+Context ctx
+string action
+ToolAnnotations annotations
}
class ReadConsoleTool {
+Context ctx
+string action
+ToolAnnotations annotations
}
class SetActiveInstanceTool {
+Context ctx
+string instance
+ToolAnnotations annotations
}
class BatchExecuteTool {
+Context ctx
+list batch
+ToolAnnotations annotations
}
class ExecuteCustomToolTool {
+Context ctx
+string tool_name
+dict parameters
+ToolAnnotations annotations
}
class RunTestsTool {
+Context ctx
+string filter
+ToolAnnotations annotations
}
class ScriptApplyEditsTool {
+Context ctx
+string name
+list edits
+ToolAnnotations annotations
}
ToolAnnotations <.. ApplyTextEditsTool : uses
ToolAnnotations <.. CreateScriptTool : uses
ToolAnnotations <.. DeleteScriptTool : uses
ToolAnnotations <.. ValidateScriptTool : uses
ToolAnnotations <.. ManageScriptTool : uses
ToolAnnotations <.. ManageScriptCapabilitiesTool : uses
ToolAnnotations <.. GetShaTool : uses
ToolAnnotations <.. DebugRequestContextTool : uses
ToolAnnotations <.. FindInFileTool : uses
ToolAnnotations <.. ExecuteMenuItemTool : uses
ToolAnnotations <.. ManageAssetTool : uses
ToolAnnotations <.. ManageEditorTool : uses
ToolAnnotations <.. ManageGameobjectTool : uses
ToolAnnotations <.. ManageMaterialTool : uses
ToolAnnotations <.. ManagePrefabsTool : uses
ToolAnnotations <.. ManageSceneTool : uses
ToolAnnotations <.. ManageShaderTool : uses
ToolAnnotations <.. ReadConsoleTool : uses
ToolAnnotations <.. SetActiveInstanceTool : uses
ToolAnnotations <.. BatchExecuteTool : uses
ToolAnnotations <.. ExecuteCustomToolTool : uses
ToolAnnotations <.. RunTestsTool : uses
ToolAnnotations <.. ScriptApplyEditsTool : uses
class McpForUnityToolDecorator {
+ToolAnnotations annotations
+string description
+string name
}
McpForUnityToolDecorator o-- ToolAnnotations
McpForUnityToolDecorator <.. ApplyTextEditsTool
McpForUnityToolDecorator <.. ValidateScriptTool
McpForUnityToolDecorator <.. DebugRequestContextTool
McpForUnityToolDecorator <.. BatchExecuteTool
McpForUnityToolDecorator <.. ScriptApplyEditsTool
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR adds ToolAnnotations metadata (titles and readOnly/destructive hints) to many Changes
Sequence Diagram(s)(Skipped — changes are metadata and signature additions without new multi-component control flow.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 1 issue, and left some high level feedback:
- Several tools that can be either read-only or mutating depending on the action (e.g.,
manage_*routers,read_console,batch_execute) are marked unconditionally withdestructiveHint=True; consider whether you want more granular signaling (e.g., per-action parameters or separate tools) so clients can auto-approve genuinely read-only invocations. - The
ToolAnnotationsblocks repeat very similartitle/readOnlyHint/destructiveHintdefinitions across many tools; consider centralizing these (e.g., constants or a small helper wrapper aroundmcp_for_unity_tool) to reduce duplication and make future changes to annotation conventions easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Several tools that can be either read-only or mutating depending on the action (e.g., `manage_*` routers, `read_console`, `batch_execute`) are marked unconditionally with `destructiveHint=True`; consider whether you want more granular signaling (e.g., per-action parameters or separate tools) so clients can auto-approve genuinely read-only invocations.
- The `ToolAnnotations` blocks repeat very similar `title`/`readOnlyHint`/`destructiveHint` definitions across many tools; consider centralizing these (e.g., constants or a small helper wrapper around `mcp_for_unity_tool`) to reduce duplication and make future changes to annotation conventions easier.
## Individual Comments
### Comment 1
<location> `Server/src/services/tools/batch_execute.py:25` </location>
<code_context>
from transport.plugin_hub import PluginHub
@mcp_for_unity_tool(
- description="Return the current FastMCP request context details (client_id, session_id, and meta dump)."
+ description="Return the current FastMCP request context details (client_id, session_id, and meta dump).",
</code_context>
<issue_to_address>
**suggestion:** Clarify the destructive nature of `batch_execute`, since it can also wrap read-only calls.
Labeling `batch_execute` as destructive is reasonable for safety, but it will cause all uses to be treated as risky, even read-only batches. Consider adjusting the description to clarify that *batches may be destructive depending on the tools they contain*, so clients and agents can reflect that nuance instead of assuming every `batch_execute` call mutates state.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Address Sourcery AI review feedback by updating descriptions for batch_execute and manage_* tools to clarify their safety characteristics depend on the specific actions/operations being performed. Updated tools: - batch_execute: clarifies safety depends on contained tools - read_console: clarifies 'get' is read-only, 'clear' is destructive - manage_asset: lists read-only vs destructive actions - manage_gameobject: lists read-only vs destructive actions - manage_scene: lists read-only vs destructive actions - manage_editor: lists read-only vs destructive actions - manage_material: lists read-only vs destructive actions - manage_shader: lists read-only vs destructive actions - manage_script: lists read-only vs destructive actions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Thank you for the review feedback! Re: Dual-purpose tools: I've updated the descriptions for I've kept Re: Centralization: Valid point for larger codebases. For this PR's scope (23 tools), I prefer keeping annotations explicit per-tool for clarity and auditability. The repetitive pattern is intentional—it makes each tool self-documenting and easy to audit. Happy to refactor if the maintainers prefer centralized constants. Changes pushed in commit 79c5ec7. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Server/src/services/tools/manage_script.py (1)
512-519: Consolidate multiple annotation strings into single descriptions for consistency.Lines 513-519 contain multiple string metadata values in
Annotatedtypes (name,path,contents,script_type). The codebase convention uses single description strings. Combine the metadata into a single, more informative description string—for example:name: Annotated[str, "Script name without .cs extension"]andpath: Annotated[str, "Path under Assets/ to create the script, e.g., 'Assets/Scripts/My.cs'"].
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Server/src/services/tools/batch_execute.pyServer/src/services/tools/manage_asset.pyServer/src/services/tools/manage_editor.pyServer/src/services/tools/manage_gameobject.pyServer/src/services/tools/manage_material.pyServer/src/services/tools/manage_scene.pyServer/src/services/tools/manage_script.pyServer/src/services/tools/manage_shader.pyServer/src/services/tools/read_console.py
🚧 Files skipped from review as they are similar to previous changes (5)
- Server/src/services/tools/batch_execute.py
- Server/src/services/tools/manage_shader.py
- Server/src/services/tools/manage_asset.py
- Server/src/services/tools/manage_editor.py
- Server/src/services/tools/read_console.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 301
File: docs/CUSTOM_TOOLS.md:54-62
Timestamp: 2025-10-03T22:11:46.002Z
Learning: In Unity MCP, the `description` parameter in the `mcp_for_unity_tool` decorator is technically optional but should always be included as a best practice. Without it, there's a higher chance that MCP clients will not parse the tool correctly. All Unity MCP tools should include the description in the decorator for compatibility.
📚 Learning: 2025-10-03T22:11:46.002Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 301
File: docs/CUSTOM_TOOLS.md:54-62
Timestamp: 2025-10-03T22:11:46.002Z
Learning: In Unity MCP, the `description` parameter in the `mcp_for_unity_tool` decorator is technically optional but should always be included as a best practice. Without it, there's a higher chance that MCP clients will not parse the tool correctly. All Unity MCP tools should include the description in the decorator for compatibility.
Applied to files:
Server/src/services/tools/manage_gameobject.pyServer/src/services/tools/manage_material.pyServer/src/services/tools/manage_script.pyServer/src/services/tools/manage_scene.py
📚 Learning: 2025-11-05T18:23:12.349Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 368
File: MCPForUnity/UnityMcpServer~/src/resources/menu_items.py:15-15
Timestamp: 2025-11-05T18:23:12.349Z
Learning: In Unity MCP, the `name` parameter in the `mcp_for_unity_resource` decorator is the external API name exposed to MCP clients (LLMs, AI agents). The command string passed to `async_send_command_with_retry` or `async_send_with_unity_instance` (e.g., "get_menu_items") is the internal command identifier that must match the C# side. These are decoupled, allowing external API naming to evolve independently of internal command routing.
Applied to files:
Server/src/services/tools/manage_script.py
⏰ 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). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (5)
Server/src/services/tools/manage_gameobject.py (1)
15-21: Annotations and description correctly applied.The
destructiveHint=Trueis appropriate since this tool supports destructive operations (create, modify, delete, etc.), even though some actions are read-only. The description clearly documents which actions fall into each category, enabling LLMs to make informed decisions.Server/src/services/tools/manage_material.py (1)
17-23: Annotations properly configured.The tool annotation correctly marks this as destructive since most operations modify material state. The description comprehensively categorizes all 7 actions (ping, get_material_info as read-only; the rest as destructive).
Server/src/services/tools/manage_scene.py (1)
12-18: Annotations correctly applied with accurate action classification.The description properly categorizes screenshot as read-only (captures without modifying) and scene-modifying operations (create, load, save) as destructive. Good alignment between the action literals and documented behavior.
Server/src/services/tools/manage_script.py (2)
67-86: Well-documented tool with appropriate destructive hint.The multi-line description provides excellent guidance for safe usage, including the recommended workflow and notes about alternative tools. The
destructiveHint=Truecorrectly reflects that this tool modifies script files.
377-383: All tool annotations correctly configured.Each tool has appropriate hints:
create_script,delete_script,manage_script→destructiveHint=Truevalidate_script,manage_script_capabilities,get_sha→readOnlyHint=TrueThe descriptions clearly document tool purposes. Based on learnings, including
descriptionin the decorator is a best practice for MCP client compatibility.Also applies to: 428-434, 456-462, 503-509, 577-590, 615-621
|
Read console can't do anything destructive can it? |
Address maintainer feedback from dsarno. Console clearing affects ephemeral UI state, not project data - it's not "destructive" in the MCP sense (irreversible data loss/modification). Co-Authored-By: Claude <[email protected]>
|
You're right! Clearing the console affects ephemeral UI state, not project data – it's not "destructive" in the MCP sense (irreversible data loss/modification). Changed to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Server/src/services/tools/read_console.py (1)
15-21: Annotations and description look good!The
readOnlyHint=Trueannotation accurately reflects that this tool doesn't modify persistent project state, and the updated description provides helpful guidance about thecountparameter format. The defensive coercion code (lines 67-82) already handles both int and string inputs correctly.Optional: Consider clarifying "read-only" scope in the description
Since the tool supports both
action='get'(read-only) andaction='clear'(modifies ephemeral console UI), you might optionally expand the description to clarify that "read-only" refers to project/persistent state, not the console buffer itself. For example:- description="Gets messages from or clears the Unity Editor console. Note: For maximum client compatibility, pass count as a quoted string (e.g., '5').", + description="Gets messages from or clears the Unity Editor console (clears ephemeral UI state only, no project modifications). Note: For maximum client compatibility, pass count as a quoted string (e.g., '5').",This is purely for explicitness and not required given the maintainer's deliberate decision on the hint semantics.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Server/src/services/tools/read_console.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 301
File: docs/CUSTOM_TOOLS.md:54-62
Timestamp: 2025-10-03T22:11:46.002Z
Learning: In Unity MCP, the `description` parameter in the `mcp_for_unity_tool` decorator is technically optional but should always be included as a best practice. Without it, there's a higher chance that MCP clients will not parse the tool correctly. All Unity MCP tools should include the description in the decorator for compatibility.
📚 Learning: 2025-10-03T22:11:46.002Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 301
File: docs/CUSTOM_TOOLS.md:54-62
Timestamp: 2025-10-03T22:11:46.002Z
Learning: In Unity MCP, the `description` parameter in the `mcp_for_unity_tool` decorator is technically optional but should always be included as a best practice. Without it, there's a higher chance that MCP clients will not parse the tool correctly. All Unity MCP tools should include the description in the decorator for compatibility.
Applied to files:
Server/src/services/tools/read_console.py
🧬 Code graph analysis (1)
Server/src/services/tools/read_console.py (2)
Server/src/transport/unity_transport.py (1)
send_with_unity_instance(78-103)Server/src/transport/legacy/unity_connection.py (1)
async_send_command_with_retry(753-785)
⏰ 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). (2)
- GitHub Check: Sourcery review
- GitHub Check: Sourcery review
🔇 Additional comments (1)
Server/src/services/tools/read_console.py (1)
7-8: LGTM!The
ToolAnnotationsimport is correctly added and necessary for the annotations block below.
Summary
Adds MCP tool annotations (
readOnlyHint,destructiveHint,title) to all 23 tools to help LLMs better understand tool behavior and make safer decisions about tool execution.Changes
Added
readOnlyHint: trueto 5 read-only tools:debug_request_context- Returns current FastMCP request contextfind_in_file- Searches files with regex patternsvalidate_script- Validates C# scripts and returns diagnosticsmanage_script_capabilities- Gets supported operations and limitsget_sha- Gets SHA256 and metadata for scriptsAdded
destructiveHint: trueto 18 tools that modify state:manage_asset,manage_gameobject,manage_material,manage_prefabs,manage_scene,manage_shader,manage_editorapply_text_edits,create_script,delete_script,manage_script,script_apply_editsexecute_menu_item,execute_custom_tool,batch_execute,run_testsset_active_instance,read_console(can clear console)Added
titleannotations for human-readable displayImported
ToolAnnotationsfrommcp.typesin all tool filesWhy This Matters
Tool annotations are advisory hints for MCP clients that enable:
readOnlyHint: truewithout user confirmationTesting
Tool Annotation Summary
🤖 Generated with Claude Code
Summary by Sourcery
Add MCP ToolAnnotations metadata to Unity MCP tools to indicate read-only vs destructive behavior and provide human-readable titles for safer client-side usage.
New Features:
Enhancements:
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.