Repository navigation
Conversation
leehack
requested review from
jakubfijalkowski and
mateusz-pietras
as code owners
October 24, 2026 23:30
👷 Deploy request for marionette-landing pending review.Visit the deploys page to approve it
|
Author
|
I know it's a pretty big PR. I tried to refactor it to have a better architectural shape, leveraging mcp_dart. If you don't like it, it's ok to reject, but I basically just wanted to show how you could organize the tools. |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The SDK upgrade and custom VM transport lifecycle rewrite warrant human review, particularly with live Flutter/device validation still outstanding.
No open issues 🎉
What changed in this PR
Refactors the MCP server into protocol adapters and independent runtime services, with explicit resource ownership and structured tool results.
Changes:
- Upgrades
mcp_dartto^2.4.3and adds output schemas while retaining existing text, images, and error responses. - Separates tool registration, connection workflows, screenshot persistence, and shutdown; adds public entrypoints and compatibility exports.
- Expands lifecycle and compatibility tests, separates minimum-version CI dependency groups, and updates setup and architecture guidance.
| File | Description |
|---|---|
website/lib/pages/docs/start_page.dart |
Clarifies Flutter SDK and separate-server requirements. |
README.md |
Links contributor architecture guidance. |
pubspec.lock |
Locks the upgraded MCP SDK. |
packages/marionette_mcp/test/vm_service/tools/inspection_tools_test.dart |
Tests screenshot adapters and failure handling. |
packages/marionette_mcp/test/vm_service/tools/extension_tools_test.dart |
Migrates extension tests to typed adapters. |
packages/marionette_mcp/test/vm_service/tools/device_tools_test.dart |
Migrates device configuration adapter tests. |
packages/marionette_mcp/test/vm_service/dynamic_extension_tools_test.dart |
Tests removal, reconnect, and stale discovery. |
packages/marionette_mcp/test/vm_service/connector_lifecycle_test.dart |
Covers VM connection ownership and cancellation. |
packages/marionette_mcp/test/vm_service/app_connection_service_test.dart |
Covers handshake and session transitions. |
packages/marionette_mcp/test/support/test_process.dart |
Adds subprocess communication and cleanup helpers. |
packages/marionette_mcp/test/structured_tool_outputs_test.dart |
Validates schemas and structured success results. |
packages/marionette_mcp/test/stdio_server_test.dart |
Covers shutdown with active or pending connections. |
packages/marionette_mcp/test/sse_server_test.dart |
Covers legacy SSE interaction and shutdown. |
packages/marionette_mcp/test/session/step_logger_test.dart |
Checks session attribution across reconnects. |
packages/marionette_mcp/test/session/screenshot_service_test.dart |
Tests capture, persistence, and session retention. |
packages/marionette_mcp/test/server_runner_test.dart |
Tests startup failures and logging cleanup. |
packages/marionette_mcp/test/server_context_test.dart |
Tests overlapping connection/session operations. |
packages/marionette_mcp/test/server_config_test.dart |
Tests option parsing and invalid ports. |
packages/marionette_mcp/test/server_api_test.dart |
Checks public APIs and protocol compatibility. |
packages/marionette_mcp/test/fixtures/vm_service_app.dart |
Adds a minimal VM service fixture. |
packages/marionette_mcp/test/architecture_test.dart |
Enforces entrypoints and dependency boundaries. |
packages/marionette_mcp/pubspec.yaml |
Updates the MCP SDK constraint. |
packages/marionette_mcp/lib/src/vm_service/vm_service_context.dart |
Removes the former combined context implementation. |
packages/marionette_mcp/lib/src/vm_service/vm_service_connector.dart |
Adds explicit transport ownership and cancellation. |
packages/marionette_mcp/lib/src/vm_service/tools/text_tools.dart |
Removes superseded text tool registration. |
packages/marionette_mcp/lib/src/vm_service/tools/system_tools.dart |
Removes superseded system tool registration. |
packages/marionette_mcp/lib/src/vm_service/tools/keyboard_tools.dart |
Removes superseded keyboard tool registration. |
packages/marionette_mcp/lib/src/vm_service/tools/inspection_tools.dart |
Removes combined inspection and persistence implementation. |
packages/marionette_mcp/lib/src/vm_service/tools/extension_tools.dart |
Removes superseded extension tool registration. |
packages/marionette_mcp/lib/src/vm_service/tools/device_tools.dart |
Removes superseded device tool registration. |
packages/marionette_mcp/lib/src/vm_service/dynamic_extension_tools.dart |
Removes the former pooled registration implementation. |
packages/marionette_mcp/lib/src/vm_service/app_connection_service.dart |
Owns handshakes and report-session transitions. |
packages/marionette_mcp/lib/src/session/screenshot_service.dart |
Extracts MCP-independent capture and persistence. |
packages/marionette_mcp/lib/src/session/report_steps.dart |
Extracts report formatting and persistence. |
packages/marionette_mcp/lib/src/mcp/tools/tools.dart |
Defines the single built-in tool catalog. |
packages/marionette_mcp/lib/src/mcp/tools/tool_runner.dart |
Houses shared adapter error handling. |
packages/marionette_mcp/lib/src/mcp/tools/tool_results.dart |
Defines shared structured result helpers. |
packages/marionette_mcp/lib/src/mcp/tools/text_tools.dart |
Adds the typed text-input adapter. |
packages/marionette_mcp/lib/src/mcp/tools/system_tools.dart |
Adds typed reload and restart adapters. |
packages/marionette_mcp/lib/src/mcp/tools/keyboard_tools.dart |
Adds the typed keyboard adapter. |
packages/marionette_mcp/lib/src/mcp/tools/inspection_tools.dart |
Maps inspection and screenshot results to MCP. |
packages/marionette_mcp/lib/src/mcp/tools/extension_tools.dart |
Adds typed extension discovery and invocation. |
packages/marionette_mcp/lib/src/mcp/tools/dynamic_extension_tools.dart |
Registers connection-scoped tools with structured outputs. |
packages/marionette_mcp/lib/src/mcp/tools/device_tools.dart |
Adds the typed device configuration adapter. |
packages/marionette_mcp/lib/src/mcp/tools/connection_tools.dart |
Maps connection outcomes and logs transitions. |
packages/marionette_mcp/lib/src/mcp/tools/base_tool.dart |
Defines typed tool and registration contracts. |
packages/marionette_mcp/lib/src/mcp/tools/ancestor_keys_description.dart |
Houses shared selector documentation. |
packages/marionette_mcp/lib/src/mcp/step_logging.dart |
Retains invocation sessions for outcome logging. |
packages/marionette_mcp/lib/src/mcp/server_runner.dart |
Centralizes startup, transports, and cleanup. |
packages/marionette_mcp/lib/src/mcp/server_context.dart |
Connects runtime lifecycle to MCP state. |
packages/marionette_mcp/lib/src/mcp/server_config.dart |
Centralizes standalone options and port validation. |
packages/marionette_mcp/lib/src/mcp/mcp.dart |
Defines server composition and tool registration. |
packages/marionette_mcp/lib/src/mcp_server_runner.dart |
Preserves the CLI runner import through an export. |
packages/marionette_mcp/lib/runtime.dart |
Exposes the public VM connector API. |
packages/marionette_mcp/lib/marionette_mcp.dart |
Exposes public server APIs. |
packages/marionette_mcp/example/README.md |
Documents example setup and SDK requirements. |
packages/marionette_mcp/example/pubspec.lock |
Updates example dependencies for the SDK upgrade. |
packages/marionette_mcp/bin/marionette_mcp.dart |
Uses shared configuration and sets process exit codes. |
packages/marionette_flutter/skills/marionette-flutter-drive-app/SKILL.md |
Documents structured outputs and installation compatibility. |
packages/marionette_flutter/lib/src/binding/marionette_configuration.dart |
Updates the session-report documentation URL only. |
docs/architecture.md |
Documents boundaries, ownership, and verification. |
CHANGELOG.md |
Records server changes and updates a documentation link. |
.github/workflows/ci.yaml |
Separates minimum-Dart and minimum-Flutter dependency groups. |
Review effort: Balanced
Member
|
Hello, thank you for the contribution. It's definitely a big PR 😃, but that's fair given the scope of the changes. I had a quick look at the code and overall I like the proposed structure. We still need to review it in depth and coordinate it with the native support work in progress (#137, #138). I'll keep you posted once we get to it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upgrade
mcp_dartto^2.4.3, separate MCP adapters from app workflows, and fix connection/session ownership and shutdown. Tools gainoutputSchemaand structured success results while preserving existing names, input schemas, annotations, text, images, and error behavior.CLI package behavior is unchanged. Streamable HTTP remains deferred.
Architecture
flowchart LR subgraph Public["lib: public entrypoints"] API["marionette_mcp.dart"] RuntimeAPI["runtime.dart"] end subgraph MCP["lib/src/mcp: protocol adapters"] Runner["Bootstrap and shutdown"] Composition["Composition root and context"] Tools["Typed tools: schemas and result mapping"] Runner --> Composition --> Tools end subgraph Runtime["lib/src: runtime implementation"] Services["Connection and screenshot services"] Connector["VM service connector"] Services --> Connector end API --> Runner RuntimeAPI --> Connector Tools --> Services Tools --> Connector Connector --> App["Flutter app"]lib/; implementation lives undersrc/. MCP-independent services own connection, session, and screenshot workflows.createAllToolscatalog replaces redundant factories and invocation wrappers. The bootstrap owns resources and cleanup.The architecture guide explains ownership, structured results, and adding tools. Consumer docs, examples, website guidance, and the agent skill are updated.
Compatibility and CI
No new direct dependencies or declared minimum version increases. Flutter 3.27's test SDK pins
meta1.15;mcp_dart2.4.3 requires 1.16+. Minimum-version CI therefore selects independent Dart/Flutter workspace groups without dependency overrides. Normal and publication jobs match upstreammain.The full workspace and MCP Flutter example require the newer pinned Flutter SDK. Flutter 3.27 apps remain supported through the binding with a separate server; setup docs explain this distinction.
Validation
9aa0071, including analysis, formatting, both toolchains, and website checks.Live Flutter/device testing has not been performed.