Fix Flutter Toolkit Commands Failing With Unknown property "isolateId"#95
Fix Flutter Toolkit Commands Failing With Unknown property "isolateId"#95drown0315 wants to merge 1 commit into
Conversation
|
To view this pull requests documentation preview, visit the following URL: docs.page/arenukvern/mcp_flutter~95 Documentation is deployed and generated using docs.page. |
📝 WalkthroughWalkthroughThis PR fixes a tool validation failure in Flutter debug mode by filtering VM-service-injected ChangesisolateId metadata filtering in service extensions
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
🧹 Nitpick comments (1)
mcp_toolkit/lib/src/mcp_toolkit_extensions.dart (1)
194-201: 💤 Low valueInline comments correctly explain the VM transport metadata filtering.
The helper function correctly strips
isolateIdbefore schema coercion/validation, preventingAgentValidationExceptionon unknown properties. The inline comments clearly explain WHY this filtering is necessary (VM service transport injects routing metadata that isn't an MCP tool argument).💬 Optional: Consider dartdoc style for consistency
While inline comments work well for private functions, the coding guidelines prefer
///for documentation comments. If you want stricter consistency with project conventions:-// Flutter's VM service extension transport injects isolateId into callback -// parameters. It routes the VM service call to the target isolate; it is not -// an MCP tool argument. Strip it before schema coercion/validation because -// tool schemas intentionally reject unknown properties. -Map<String, Object?> _extractToolArguments( - final Map<String, String> parameters, -) => Map<String, Object?>.from(parameters)..remove('isolateId'); +/// Flutter's VM service extension transport injects isolateId into callback +/// parameters. It routes the VM service call to the target isolate; it is not +/// an MCP tool argument. Strip it before schema coercion/validation because +/// tool schemas intentionally reject unknown properties. +Map<String, Object?> _extractToolArguments( + final Map<String, String> parameters, +) => Map<String, Object?>.from(parameters)..remove('isolateId');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp_toolkit/lib/src/mcp_toolkit_extensions.dart` around lines 194 - 201, Summary: convert the inline block comment explaining VM service transport metadata filtering into a dartdoc comment for consistency. Replace the existing /* ... *// // block above the private helper _extractToolArguments with a /// dartdoc comment that preserves the same explanation (that Flutter VM service injects isolateId into callback parameters and it must be stripped before schema coercion/validation), leaving the implementation Map<String, Object?> _extractToolArguments(Map<String, String> parameters) => Map<String, Object?>.from(parameters)..remove('isolateId'); unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@mcp_toolkit/lib/src/mcp_toolkit_extensions.dart`:
- Around line 194-201: Summary: convert the inline block comment explaining VM
service transport metadata filtering into a dartdoc comment for consistency.
Replace the existing /* ... *// // block above the private helper
_extractToolArguments with a /// dartdoc comment that preserves the same
explanation (that Flutter VM service injects isolateId into callback parameters
and it must be stripped before schema coercion/validation), leaving the
implementation Map<String, Object?> _extractToolArguments(Map<String, String>
parameters) => Map<String, Object?>.from(parameters)..remove('isolateId');
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da41ee07-e7d6-4ad2-8566-aedda7ab0334
📒 Files selected for processing (2)
mcp_toolkit/lib/src/mcp_toolkit_extensions.dartmcp_toolkit/test/mcp_toolkit_bootstrap_test.dart
Fix Flutter Toolkit Commands Failing With
Unknown property "isolateId"Summary
isolateIdmetadata beforemcp_toolkitvalidates tool arguments.isolateId.Closes #94
Problem
Flutter VM Service injects
isolateIdinto service extension callback parameters as routing metadata.mcp_toolkitwas passing the full callback parameter map into schema coercion and validation. Because toolkit schemas are strict, commands could fail with:This affected app-side toolkit commands such as:
get_view_detailssemantic_snapshotget_app_errorsFix
Filter VM Service metadata before schema coercion and validation:
_extractToolArgumentsremovesisolateIdwhile preserving actual tool arguments.Validation
flutter test test/mcp_toolkit_bootstrap_test.dartflutter testflutter analyzeSummary by CodeRabbit
Bug Fixes
Tests