[codex] Extract session broker surfaces#100
Merged
Merged
Conversation
|
To preview the documentation for this pull request, visit the following URL: docs.page/arenukvern/mcp_flutter~100
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds the ChangesAlias, policy, and docs
IntentCall server and toolkit migration
Flutter showcase app integration
Visibility, gesture, and reveal-search behavior
Proof scripts, evals, and build tooling
Estimated code review effort: 5 (Critical) | ~150 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
# Conflicts: # packages/server_capability_core/pubspec.yaml # packages/server_capability_kernel/pubspec.yaml
- Adjusted project indexing in AGENTS.md to reflect updated symbol and relationship counts. - Updated CHANGELOG.md to document breaking changes in `flutter_mcp_core.dart` and the migration of canonical types to `intentcall_core`. - Added a new command in steward.yaml to check for stale local or root path dependencies in hosted IntentCall. - Updated README.md to reflect the new consumer package policy for IntentCall version 0.2.1. - Enhanced the gesture interaction service to handle web-specific scrolling actions and added new utility functions for semantic snapshot services. - Improved tests for registry discovery and semantic snapshot functionalities. - Cleaned up legacy artifacts in the exec sweep script and ensured proper version checks for IntentCall dependencies.
Arenukvern
marked this pull request as ready for review
July 4, 2026 14:44
Arenukvern
enabled auto-merge
July 4, 2026 14:44
Arenukvern
disabled auto-merge
July 4, 2026 14:57
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (22)
flutter_test_app/lib/agent_state.dart (1)
62-70: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a brief doc comment for the new test-only member.
As per coding guidelines, "Document all public members (methods, properties, etc.) with clear, concise explanations of their purpose and usage in Dart."
resetForTest()is@visibleForTesting(effectively public) but has no///comment.📝 Proposed doc comment
+ /// Resets all tracked state to defaults for deterministic tests. `@visibleForTesting` void resetForTest() {🤖 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 `@flutter_test_app/lib/agent_state.dart` around lines 62 - 70, Add a brief Dart doc comment for the test-only public method resetForTest() in AgentState. Document its purpose clearly and concisely with a /// comment placed immediately above the `@visibleForTesting` member, noting that it resets the internal state fields and notifies listeners for tests.Source: Coding guidelines
mcp_server_dart/lib/src/cli/appintents_testing_command.dart (3)
93-149: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffManual JSON casting instead of
from_json_to_jsonhelpers.
_readJsonObjectFileand the innerswitchcasts inreadAppIntentsTestingSampleArguments/readAppIntentsTestingEntityFixtureshand-roll the "decode object, validate shape, throw on invalid input" pattern. As per coding guidelines, Dart files should "Always use thefrom_json_to_jsonpackage for type-safe JSON handling" and "UsejsonDecodeThrowableMap()when you need exceptions on invalid input," which would replace this bespoke casting/validation logic.🤖 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_server_dart/lib/src/cli/appintents_testing_command.dart` around lines 93 - 149, The JSON parsing in readAppIntentsTestingSampleArguments, readAppIntentsTestingEntityFixtures, and _readJsonObjectFile is hand-rolling casts and validation instead of using the approved helpers. Replace the manual jsonDecode/json cast logic with from_json_to_json utilities, using jsonDecodeThrowableMap() for invalid-input exceptions and any needed typed map conversion, while preserving the existing fixture validation for required fields in AppleAppIntentsTestingEntityFixture.Source: Coding guidelines
62-70: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBroad
on Object catchmasks programming errors.Catching
Objectafter the specificFormatExceptioncatch also swallowsErrorsubtypes (e.g.,TypeError, assertion failures), turning real bugs into a generic "appintents-testing generate failed" exit 65 instead of a diagnosable crash.🛡️ Narrow the catch
- } on Object catch (error) { + } on Exception catch (error) { stderr.writeln('appintents-testing generate failed: $error'); return 65; }🤖 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_server_dart/lib/src/cli/appintents_testing_command.dart` around lines 62 - 70, The generic exception handler in the appintents-testing generate flow is too broad and catches programming errors as if they were user failures. In appintentsTestingGenerateCommand (the try/catch around the FormatException block), remove the on Object catch or narrow it to only expected exception types so TypeError, assertion failures, and other Error subtypes can surface normally. Keep the existing FormatException handling for invalid input, and only map truly anticipated runtime failures to the exit 65 path.
73-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing dartdoc on public top-level functions.
emitAppIntentsTestingScaffold,readAppIntentsTestingSampleArguments, andreadAppIntentsTestingEntityFixturesare all public but lack///documentation. As per coding guidelines, "Document all public members (methods, properties, etc.) with clear, concise explanations of their purpose and usage in Dart."🤖 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_server_dart/lib/src/cli/appintents_testing_command.dart` around lines 73 - 135, Add dartdoc comments to the public top-level functions emitAppIntentsTestingScaffold, readAppIntentsTestingSampleArguments, and readAppIntentsTestingEntityFixtures in appintents_testing_command.dart. Each comment should briefly explain the function’s purpose, what inputs it expects, and what it returns or throws, so the public API is documented consistently with the coding guidelines.Source: Coding guidelines
mcp_server_dart/test/appintents_testing_command_test.dart (1)
6-132: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap tests in
group()per test-file conventions.All four tests sit flat under
main(). As per coding guidelines,**/*_test.dartfiles should "Group related tests with group() and individual cases with test()."♻️ Suggested wrap
void main() { + group('appintents-testing scaffold generation', () { test('emits AppIntentsTesting scaffold from manifest and fixtures', () { ... }); + }); }🤖 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_server_dart/test/appintents_testing_command_test.dart` around lines 6 - 132, The tests in appintents_testing_command_test.dart are currently declared flat under main(), which breaks the test-file convention of grouping related cases. Wrap the four existing test() cases in a group() inside main(), using a descriptive group name that matches the AppIntentsTesting scaffold/fixture behavior, and keep the existing test names and assertions unchanged; the relevant symbols to place are main(), group(), and the existing readAppIntentsTestingSampleArguments/readAppIntentsTestingEntityFixtures/emitAppIntentsTestingScaffold tests.Source: Coding guidelines
mcp_server_dart/lib/src/cli/diagnostics/command_snapshot_service.dart (1)
15-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClass doc comment is missing the required
@aiannotation.The new
CommandSnapshotServicedartdoc is otherwise good (concise, states the "why"), but per repo Dart doc guidelines, class-level docs should include an@aiannotation with instructions for AI tools.As per coding guidelines: "Include an
@aiannotation with specific instructions for AI tools on how to interpret and use the class."🤖 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_server_dart/lib/src/cli/diagnostics/command_snapshot_service.dart` around lines 15 - 26, The class-level dartdoc for CommandSnapshotService is missing the required `@ai` annotation. Update the existing documentation on CommandSnapshotService to keep the current summary, and add an `@ai` section that tells AI tools how to interpret and use this class per the repo guideline. Make sure the annotation is part of the class doc comment, not the constructor docs, and keep the wording concise and specific to CommandSnapshotService and its snapshotStore responsibilities.Source: Coding guidelines
flutter_test_app/lib/showcase_screen.dart (3)
88-109: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueMissing
conston_DebugSection()despite a const constructor.
_DebugSectiondeclaresconst _DebugSection();(line 613) but is instantiated as_DebugSection()here withoutconst. Given the surrounding list now rebuilds on everyAgentStatechange (including greeting sync), this avoidably reallocates the widget on each rebuild.⚡ Proposed fix
- _DebugSection(), + const _DebugSection(),🤖 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 `@flutter_test_app/lib/showcase_screen.dart` around lines 88 - 109, The widget list in the showcase screen is instantiating _DebugSection without const even though _DebugSection has a const constructor. Update the _DebugSection() usage in the children list to use const, matching the neighboring const widgets, so the showcase screen avoids unnecessary widget allocations during rebuilds.
43-46: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winGuard against no-op writes on selection-only changes.
_syncGreetingFromControllerfires on everyTextEditingControllervalue change — including cursor-move/selection-only changes with no text delta — and unconditionally reassignsAgentState.instance.greeting. Since the whole section list rebuilds viaAnimatedBuilder(animation: AgentState.instance, ...), this can trigger unnecessary full-tree rebuilds on cursor movement alone.⚡ Proposed fix
void _syncGreetingFromController() { if (_syncingGreetingFromState) return; - AgentState.instance.greeting = _textController.text; + if (_textController.text == AgentState.instance.greeting) return; + AgentState.instance.greeting = _textController.text; }🤖 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 `@flutter_test_app/lib/showcase_screen.dart` around lines 43 - 46, The _syncGreetingFromController method in ShowcaseScreen is writing AgentState.instance.greeting on every TextEditingController change, including selection-only updates, which can cause unnecessary rebuilds. Update this method to compare the controller text against the current greeting and return early when there is no text change, while still preserving the existing _syncingGreetingFromState guard. Keep the fix localized to _syncGreetingFromController and the AgentState.instance.greeting assignment path so AnimatedBuilder only reacts to real content changes.
641-652: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated tap logic between
Semantics.onTapandTextButton.onPressed.The exact same
state.logMessage(...)call is written twice inline. Since the PR's goal is forSemantics.onTapto give automated tooling parity with real taps, keeping these as separate literals risks the two diverging silently over time.♻️ Proposed fix
+ () { + final onEmitLog = () => + state.logMessage('agent hook: log at ${DateTime.now()}'); + } Semantics( identifier: 'emit_log_button', button: true, - onTap: () => - state.logMessage('agent hook: log at ${DateTime.now()}'), + onTap: onEmitLog, child: TextButton( - onPressed: () => - state.logMessage('agent hook: log at ${DateTime.now()}'), + onPressed: onEmitLog,🤖 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 `@flutter_test_app/lib/showcase_screen.dart` around lines 641 - 652, The tap behavior for the Emit log control is duplicated between `Semantics.onTap` and `TextButton.onPressed`, which risks them drifting apart. Refactor the `Semantics`/`TextButton` block in `showcase_screen.dart` to use a single shared handler or callback variable for the `state.logMessage('agent hook: log at ${DateTime.now()}')` action, and wire both `onTap` and `onPressed` to that same symbol so automated semantics taps and real button taps stay identical.tool/contracts/check_no_personal_paths.sh (1)
7-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider broadening path coverage.
The check only matches
/Users/anton. If maintainers also work from Linux (/home/<user>) or other machines, those hardcoded paths would slip through undetected.🤖 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 `@tool/contracts/check_no_personal_paths.sh` around lines 7 - 16, Broaden the personal-path detection in the check_no_personal_paths.sh script, since the current rg pattern only catches /Users/anton and misses other common home-directory paths. Update the logic around the matches/pattern lookup to search for additional platform-specific personal path prefixes such as Linux /home/<user> and any other hardcoded developer home paths you want to guard against, while keeping the existing ROOT scan and exclusions intact.mcp_server_dart/lib/src/cli/session/flutter_session_connector.dart (1)
8-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd required dartdoc to the new public connector class.
FlutterSessionConnectorand its public members (activeEndpointDisplay,lastSelectionDiagnostics,connect,disconnect) have no///documentation. As per coding guidelines,**/*.dart: "Document all public members (methods, properties, etc.) with clear, concise explanations of their purpose and usage in Dart" and "Start class documentation with a brief, single-sentence summary of the class's purpose."📝 Example doc skeleton
/// Bridges [IntentSessionConnector] to the app's [ConnectionContext]. /// /// `@ai` Use this to translate hosted intentcall_session connection /// requests into flutter_mcp_toolkit_core's [ConnectionContext] calls. final class FlutterSessionConnector implements IntentSessionConnector { const FlutterSessionConnector({required this.connectionContext}); /// The underlying [ConnectionContext] used to manage the VM connection. final ConnectionContext connectionContext; ...🤖 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_server_dart/lib/src/cli/session/flutter_session_connector.dart` around lines 8 - 57, Add Dartdoc to FlutterSessionConnector and every public member it exposes. Document the class with a one-sentence summary of its purpose, then add concise /// comments for connectionContext, activeEndpointDisplay, lastSelectionDiagnostics, connect, and disconnect explaining what each returns or does and how it is used. Keep the wording aligned with the existing IntentSessionConnector and ConnectionContext symbols so the public API is properly documented.Source: Coding guidelines
mcp_server_dart/lib/src/capabilities/visual_capture/desktop_capture_recovery.dart (1)
116-129: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAvoid catching
Object— narrow toException.
on Object catch (e)also catchesErrorsubtypes (e.g.TypeError,StateError, assertion failures), turning genuine programming bugs into a silentfocus_faileddiagnostic instead of surfacing them. Prefer catchingException(or the specific platform-channel exception type) so real bugs aren't masked.🛡️ Proposed narrowing
- } on Object catch (e) { + } on Exception catch (e) { focusDetails = <String, Object?>{ 'ok': false, 'error': 'focus_failed', 'message': '$e', }; }🤖 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_server_dart/lib/src/capabilities/visual_capture/desktop_capture_recovery.dart` around lines 116 - 129, The focus recovery block in desktop_capture_recovery.dart is catching too broadly with the focus call inside the recovery flow, which masks real bugs. Update the try/catch around service.focus in the recovery routine to catch Exception only, or a more specific platform-channel exception type if available, and keep the existing focus_failed diagnostic handling for recoverable failures while allowing Error subclasses to surface normally.mcp_server_dart/lib/src/shared_core/command_executor.dart (1)
1682-1701: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate connection-mode mapping across files.
_toIntentSessionMode/_toCoreModehere duplicates the inverse enum mapping already implemented inFlutterSessionConnector._toCoreMode(mcp_server_dart/lib/src/cli/session/flutter_session_connector.dart). Consider extracting a shared bidirectional mapper to avoid maintaining two parallel switch statements over the same enums.🤖 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_server_dart/lib/src/shared_core/command_executor.dart` around lines 1682 - 1701, The session connection-mode enum mapping is duplicated between command_executor’s _toIntentSessionMode and FlutterSessionConnector._toCoreMode, so extract a shared bidirectional mapper and use it from both places. Move the CoreConnectionMode↔IntentSessionConnectionMode switch logic into a common helper or mapper class, then update _toSessionStartRequest and FlutterSessionConnector to call that shared implementation. Keep the existing enum coverage intact so both directions stay consistent in one place.mcp_server_dart/lib/src/cli/webmcp_command.dart (3)
136-149: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify why tool-invocation success requires
fetchCalled == false.
okfor thetoolName != nullpath requirestoolProof?['fetchCalled'] == falsein addition totoolProof?['ok']. This is a reasonable way to prove the tool executed via the in-page WebMCP API rather than a network bridge, but the intent isn't documented and could confuse future maintainers extending this verifier for tools that legitimately callfetch.📝 Suggested doc comment
+ // fetchCalled must be false: a genuine WebMCP tool invocation runs + // in-page via document.modelContext, not through a network call. final ok = toolName == null ? cdpOk || logEvidence : toolProof?['ok'] == true && toolProof?['fetchCalled'] == false;🤖 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_server_dart/lib/src/cli/webmcp_command.dart` around lines 136 - 149, Document the intent behind the `ok` check in `webmcp_command.dart` so future maintainers understand why `toolProof?['fetchCalled'] == false` is required alongside `toolProof?['ok']`. Add a brief comment near the `ok` assignment in the `toolName != null` branch explaining that `_cdpInvokeWebMcpTool` is verifying in-page WebMCP execution and that `fetchCalled` must stay false for this verifier, and note that tools with legitimate network access may need a different success predicate.
184-296: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated fetch-interception logic between the two invocation branches.
The "dart hook" fallback branch (lines 205-246) and the "testing API" branch (lines 247-287) duplicate the fetch-wrapping, expectation-matching, and try/catch/finally structure almost verbatim inside the embedded JS. Consider factoring the shared logic into a single JS helper function within the expression string to reduce duplication and drift risk between the two paths.
🤖 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_server_dart/lib/src/cli/webmcp_command.dart` around lines 184 - 296, The _cdpInvokeWebMcpTool expression duplicates the same fetch-wrapping, execute, expectation-checking, and try/catch/finally flow in both the dart hook fallback and testing API branches. Refactor the embedded JS in _cdpInvokeWebMcpTool by introducing a single shared helper inside the expression string that handles interception, execution, result validation, and error shaping. Then have both the __intentcallWebMcpDartExecute path and the testing.getTools()/tool.execute path call that helper with only the branch-specific execute function and metadata. Keep the returned ok/code/message/toolName/names fields consistent across both paths.
158-166: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNested ternary reduces readability.
The
verdictvalue is computed via a 4-way nested ternary chain. Consider extracting to a small helper function/switch for clarity.🤖 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_server_dart/lib/src/cli/webmcp_command.dart` around lines 158 - 166, The verdict computation in webmcp_command.dart is too hard to read because it uses a nested ternary chain inside the map. Refactor the logic around the verdict field into a small helper method or switch-style branch near the existing cdpOk, toolName, ok, and logEvidence checks, and have the map call that helper instead of inlining the conditionals.mcp_server_dart/lib/src/capabilities/dynamic_registry/dynamic_result_normalizer.dart (1)
10-20: 📐 Maintainability & Code Quality | 🔵 TrivialModel class should be an extension type per project convention.
DynamicTextResourcePayloadis a plainfinal class, but the project guideline requires extension-type const models for DTOs/data classes, backed byfrom_json_to_jsondecoders (already used inside the function). Consider reshaping this as an extension type wrapping the normalizedMap<String, Object?>with computedcontent/message/payloadgetters, consistent with other models in the codebase.As per coding guidelines: "Always use extension type const models for creating or modifying models, DTOs, or data in Dart files."
🤖 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_server_dart/lib/src/capabilities/dynamic_registry/dynamic_result_normalizer.dart` around lines 10 - 20, `DynamicTextResourcePayload` should follow the project’s Dart model convention instead of being a plain final class. Update the DTO in `dynamic_result_normalizer.dart` to use an extension type const model backed by the normalized `Map<String, Object?>`, and expose `content`, `message`, and `payload` as computed getters. Keep the existing normalization logic in `dynamic_result_normalizer` aligned with the `from_json_to_json` decoder pattern used elsewhere so the model construction remains consistent.Source: Coding guidelines
mcp_server_dart/lib/src/capabilities/dynamic_registry/core_dynamic_registry_gateway.dart (1)
55-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated
'intent_not_found'literal into a shared constant.The same magic string is compared against
result.codein bothrunClientToolandrunClientResource. A sharedconstavoids typo-driven divergence if the intentcall_core error code ever changes.♻️ Proposed refactor
+const _intentNotFoundCode = 'intent_not_found'; + final class RegistryBackedDynamicGateway implements CoreDynamicGateway { ... - if (!result.ok && result.code == 'intent_not_found') { + if (!result.ok && result.code == _intentNotFoundCode) {As per coding guidelines: "Do not rely on strings, use enums instead."
Also applies to: 91-121
🤖 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_server_dart/lib/src/capabilities/dynamic_registry/core_dynamic_registry_gateway.dart` around lines 55 - 78, The `runClientTool`/`runClientResource` flow in `CoreDynamicRegistryGateway` compares `result.code` against the repeated `'intent_not_found'` magic string; replace this with a shared enum/constant used consistently in both methods to avoid divergence and align with the no-strings guideline. Update the `intent_not_found` check and the related failure mapping in `CoreDynamicRegistryGateway` so the error code is referenced from a single source of truth instead of duplicated literals.Source: Coding guidelines
mcp_server_dart/makefile (1)
9-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a symlink instead of duplicating the binary.
cpdoubles the on-disk size of the compiled artifact for a simple alias. A symlink (or hardlink) achieves the same CLI-alias goal without duplicating build output.♻️ Proposed change
dart compile exe bin/flutter_mcp_toolkit.dart -o build/flutter-mcp-toolkit && \ - cp build/flutter-mcp-toolkit build/fmtk + ln -sf flutter-mcp-toolkit build/fmtk🤖 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_server_dart/makefile` around lines 9 - 10, The build alias step in the Makefile duplicates the compiled Dart binary with cp, unnecessarily doubling disk usage. Update the alias creation after dart compile exe in the build target to use a symlink or hardlink for the fmtk alias instead of copying the artifact, while keeping the flutter-mcp-toolkit output unchanged.scripts/run_exec_sweep.sh (2)
18-33: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCleanup only targets the legacy flat layout, not the new per-platform
outdir.Prior-run artifacts inside
${outdir}(e.g. attempt-numbered*_N.visibility/scroll_to_*_N.jsonfiles) aren't cleared between runs, so a rerun with fewer attempts can leave stale files mixed with fresh ones, misleading debugging.♻️ Proposed fix
legacy_root="${repo_root}/.showcase/tool_verify/exec_sweep" outdir="${legacy_root}/${platform}" -mkdir -p "${outdir}" cleanup_legacy_root_artifacts() { local artifact for artifact in "${legacy_root}"/sweep_summary.txt \ "${legacy_root}"/*.json \ "${legacy_root}"/*.stderr; do [[ -e "${artifact}" ]] || continue rm -f "${artifact}" done + rm -rf "${outdir}" } cleanup_legacy_root_artifacts +mkdir -p "${outdir}"🤖 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 `@scripts/run_exec_sweep.sh` around lines 18 - 33, The cleanup in run_exec_sweep.sh only removes artifacts from legacy_root and leaves stale files in the per-platform outdir, so reruns can mix old and new results. Update cleanup_legacy_root_artifacts or add a dedicated cleanup step around outdir creation to remove prior-run artifacts there as well, including the attempt-numbered visibility and scroll_to JSON files produced by the exec_sweep flow. Use the existing symbols legacy_root and outdir to locate the cleanup logic and ensure the directory is reset before each run.
178-283: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDuplicate snapshot/visibility helper logic across two eval scripts.
json_ok,capture_snapshot,write_ref_args_for_identifier, andensure_visible_identifier_argshere are near-duplicates of the same-named functions intool/evals/run_runtime_enter_text_greeting.sh, already diverging (this file'swrite_ref_args_for_identifiertakes an extrarequire_visibleparam the other script lacks). Consider extracting a shared sourced helper script for these to avoid drift.🤖 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 `@scripts/run_exec_sweep.sh` around lines 178 - 283, The snapshot/visibility helper functions in this script are duplicated from the runtime enter-text eval script and are already drifting apart, especially `write_ref_args_for_identifier` with its extra `require_visible` parameter. Extract the shared logic used by `json_ok`, `capture_snapshot`, `write_ref_args_for_identifier`, and `ensure_visible_identifier_args` into a common sourced helper script, then update both eval scripts to call that shared implementation so behavior stays consistent.tool/evals/run_runtime_enter_text_greeting.sh (1)
137-169: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winParameterize
write_reveal_fallback_jsoninstead of hardcoding identifier/direction.The function signature is generic (
args_file,match_file,outfile), but the emitted payload hardcodes"query": "greeting_input_field"and"direction": "down". Currently the one call site happens to match, but the function will silently emit incorrect metadata if reused for any other identifier/direction.♻️ Proposed fix
-write_reveal_fallback_json() { - local args_file="$1" - local match_file="$2" - local outfile="$3" - python3 - "${args_file}" "${match_file}" "${outfile}" <<'PY' +write_reveal_fallback_json() { + local args_file="$1" + local match_file="$2" + local outfile="$3" + local identifier="${4:-greeting_input_field}" + local direction="${5:-down}" + python3 - "${args_file}" "${match_file}" "${outfile}" "${identifier}" "${direction}" <<'PY' import json, sys args = json.load(open(sys.argv[1])) match = json.load(open(sys.argv[2])) +identifier = sys.argv[4] +direction = sys.argv[5] payload = { "ok": True, "data": { "message": "Revealed target with semantic snapshot fallback.", "success": True, "error": None, - "query": "greeting_input_field", + "query": identifier, "matchBy": "identifier", - "direction": "down", + "direction": direction,🤖 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 `@tool/evals/run_runtime_enter_text_greeting.sh` around lines 137 - 169, The write_reveal_fallback_json helper is hardcoding query and direction metadata instead of deriving them from its inputs. Update write_reveal_fallback_json to read the relevant identifier and direction from args_file (or the passed match/context data) and use those values in the emitted payload so the function stays correct for any caller. Keep the existing structure intact, but ensure the fields populated in the JSON reflect the actual invocation context rather than the current greeting_input_field/down constants.
🤖 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.
Inline comments:
In `@docs/ai_agents/marketplace_copy.yaml`:
- Line 14: The `marketplace_copy.yaml` marketing copy has an inaccurate `fmt_*`
tool count. Update the text in the marketplace description to reflect the real
`FmtCapability` counts: 29 built-in `fmt_*` tools by default, with the 4
`debug_dump_*` tools included only when `dumps_supported` is enabled, for a
total of 33 in that mode. Keep the wording aligned with the existing description
section that mentions `fmt_* MCP tools`.
In `@mcp_server_dart/bin/flutter_mcp_toolkit.dart`:
- Around line 2251-2255: Align the `sample-arguments` documentation so it uses
one consistent requirement level across the CLI definition and
`_usageCodegenAppIntentsTestingGenerate`: either update the
`addOption('sample-arguments', ...)` help text or the usage string so both
describe the same thing, and keep the wording consistent with the
`qualifiedName`-keyed primitive App Intent fixtures description used in the
codebase.
- Around line 2674-2688: The `runWebmcpVerify` argument parsing currently calls
`jsonDecode` for `sub.option('tool-args')` without error handling, so malformed
JSON can escape as an unhandled `FormatException`. Update the parsing near
`rawToolArgs`/`toolArgs` to mirror `_parseJsonScalarOrString` by catching
`FormatException` and routing it through the CLI’s normal structured error path
before calling `runWebmcpVerify`, so invalid `--tool-args` input fails cleanly
instead of crashing `main()`.
In `@mcp_toolkit/lib/src/services/reveal_search_service.dart`:
- Line 33: The clamped attempt count in RevealSearchService is inferred as num,
so it no longer matches the int parameter expected later. In
reveal_search_service.dart, update the boundedMaxAttempts assignment in the
RevealSearchService logic to convert the result of maxAttempts.clamp(0,
_maxAttemptsLimit) back to int, keeping the type aligned for the downstream
maxAttempts argument.
In `@mcp_toolkit/lib/src/services/semantic_snapshot_service.dart`:
- Around line 44-58: The viewportRect getter currently always uses the first
FlutterView, which can misclassify visibility in multi-window apps. Update
SemanticSnapshotService.viewportRect to resolve the correct view for the active
semantics tree using the same viewId-based approach as
view_introspection_service.dart, or make the main-view assumption explicit and
consistent. Ensure visibilityForRef, visibleSubtreeSignature, and the snapshot
visibility fields all read from the correct FlutterView rather than blindly
using renderViews.first.
In `@mcp_toolkit/pubspec.yaml`:
- Around line 17-19: The default INTENTCALL_VERSION in print_hosted_deps.sh is
stale and no longer matches the pinned intentcall_core, intentcall_platform, and
intentcall_schema constraints in mcp_toolkit. Update the script’s default
version to the same released version used by the pubspec so the generated hosted
dependency guidance stays consistent; locate the change in the
print_hosted_deps.sh logic that sets INTENTCALL_VERSION and align it with the
package constraints.
In `@plugin/.codex-plugin/plugin.json`:
- Line 29: The marketplace longDescription is outdated and still says “30 fmt_*
MCP tools” even though the shared command catalog now exposes 44 default MCP
tools (45 total commands with dynamicRegistryStats CLI-only). Update the copy in
plugin.json to use the current tool count and keep the rest of the description
aligned with the existing product names like
fmt_list_client_tools_and_resources, fmt_client_tool, and fmt_client_resource.
In `@scripts/run_web_showcase.sh`:
- Around line 59-70: The Flutter web showcase script always appends
--no-web-resources-cdn, which prevents the CanvasKit URL path from being used.
Update scripts/run_web_showcase.sh so the flutter_args construction in the
run_web_showcase flow only includes --no-web-resources-cdn when canvaskit_url is
empty, using the existing canvaskit_url branch and flutter_args array to gate it
conditionally. Keep the FLUTTER_WEB_CANVASKIT_URL case and the local resources
case aligned so the log message matches the actual behavior.
In `@tool/intentcall/print_hosted_deps.sh`:
- Line 5: The default INTENTCALL_VERSION in print_hosted_deps.sh is stale and
does not match the new intentcall package versions used elsewhere in this PR.
Update the version fallback in the script so hosted dependency snippets reflect
the same ^0.6.0 release family as intentcall_core, intentcall_platform, and
intentcall_schema, keeping the script’s default in sync with the published
package versions.
---
Nitpick comments:
In `@flutter_test_app/lib/agent_state.dart`:
- Around line 62-70: Add a brief Dart doc comment for the test-only public
method resetForTest() in AgentState. Document its purpose clearly and concisely
with a /// comment placed immediately above the `@visibleForTesting` member,
noting that it resets the internal state fields and notifies listeners for
tests.
In `@flutter_test_app/lib/showcase_screen.dart`:
- Around line 88-109: The widget list in the showcase screen is instantiating
_DebugSection without const even though _DebugSection has a const constructor.
Update the _DebugSection() usage in the children list to use const, matching the
neighboring const widgets, so the showcase screen avoids unnecessary widget
allocations during rebuilds.
- Around line 43-46: The _syncGreetingFromController method in ShowcaseScreen is
writing AgentState.instance.greeting on every TextEditingController change,
including selection-only updates, which can cause unnecessary rebuilds. Update
this method to compare the controller text against the current greeting and
return early when there is no text change, while still preserving the existing
_syncingGreetingFromState guard. Keep the fix localized to
_syncGreetingFromController and the AgentState.instance.greeting assignment path
so AnimatedBuilder only reacts to real content changes.
- Around line 641-652: The tap behavior for the Emit log control is duplicated
between `Semantics.onTap` and `TextButton.onPressed`, which risks them drifting
apart. Refactor the `Semantics`/`TextButton` block in `showcase_screen.dart` to
use a single shared handler or callback variable for the
`state.logMessage('agent hook: log at ${DateTime.now()}')` action, and wire both
`onTap` and `onPressed` to that same symbol so automated semantics taps and real
button taps stay identical.
In
`@mcp_server_dart/lib/src/capabilities/dynamic_registry/core_dynamic_registry_gateway.dart`:
- Around line 55-78: The `runClientTool`/`runClientResource` flow in
`CoreDynamicRegistryGateway` compares `result.code` against the repeated
`'intent_not_found'` magic string; replace this with a shared enum/constant used
consistently in both methods to avoid divergence and align with the no-strings
guideline. Update the `intent_not_found` check and the related failure mapping
in `CoreDynamicRegistryGateway` so the error code is referenced from a single
source of truth instead of duplicated literals.
In
`@mcp_server_dart/lib/src/capabilities/dynamic_registry/dynamic_result_normalizer.dart`:
- Around line 10-20: `DynamicTextResourcePayload` should follow the project’s
Dart model convention instead of being a plain final class. Update the DTO in
`dynamic_result_normalizer.dart` to use an extension type const model backed by
the normalized `Map<String, Object?>`, and expose `content`, `message`, and
`payload` as computed getters. Keep the existing normalization logic in
`dynamic_result_normalizer` aligned with the `from_json_to_json` decoder pattern
used elsewhere so the model construction remains consistent.
In
`@mcp_server_dart/lib/src/capabilities/visual_capture/desktop_capture_recovery.dart`:
- Around line 116-129: The focus recovery block in desktop_capture_recovery.dart
is catching too broadly with the focus call inside the recovery flow, which
masks real bugs. Update the try/catch around service.focus in the recovery
routine to catch Exception only, or a more specific platform-channel exception
type if available, and keep the existing focus_failed diagnostic handling for
recoverable failures while allowing Error subclasses to surface normally.
In `@mcp_server_dart/lib/src/cli/appintents_testing_command.dart`:
- Around line 93-149: The JSON parsing in readAppIntentsTestingSampleArguments,
readAppIntentsTestingEntityFixtures, and _readJsonObjectFile is hand-rolling
casts and validation instead of using the approved helpers. Replace the manual
jsonDecode/json cast logic with from_json_to_json utilities, using
jsonDecodeThrowableMap() for invalid-input exceptions and any needed typed map
conversion, while preserving the existing fixture validation for required fields
in AppleAppIntentsTestingEntityFixture.
- Around line 62-70: The generic exception handler in the appintents-testing
generate flow is too broad and catches programming errors as if they were user
failures. In appintentsTestingGenerateCommand (the try/catch around the
FormatException block), remove the on Object catch or narrow it to only expected
exception types so TypeError, assertion failures, and other Error subtypes can
surface normally. Keep the existing FormatException handling for invalid input,
and only map truly anticipated runtime failures to the exit 65 path.
- Around line 73-135: Add dartdoc comments to the public top-level functions
emitAppIntentsTestingScaffold, readAppIntentsTestingSampleArguments, and
readAppIntentsTestingEntityFixtures in appintents_testing_command.dart. Each
comment should briefly explain the function’s purpose, what inputs it expects,
and what it returns or throws, so the public API is documented consistently with
the coding guidelines.
In `@mcp_server_dart/lib/src/cli/diagnostics/command_snapshot_service.dart`:
- Around line 15-26: The class-level dartdoc for CommandSnapshotService is
missing the required `@ai` annotation. Update the existing documentation on
CommandSnapshotService to keep the current summary, and add an `@ai` section that
tells AI tools how to interpret and use this class per the repo guideline. Make
sure the annotation is part of the class doc comment, not the constructor docs,
and keep the wording concise and specific to CommandSnapshotService and its
snapshotStore responsibilities.
In `@mcp_server_dart/lib/src/cli/session/flutter_session_connector.dart`:
- Around line 8-57: Add Dartdoc to FlutterSessionConnector and every public
member it exposes. Document the class with a one-sentence summary of its
purpose, then add concise /// comments for connectionContext,
activeEndpointDisplay, lastSelectionDiagnostics, connect, and disconnect
explaining what each returns or does and how it is used. Keep the wording
aligned with the existing IntentSessionConnector and ConnectionContext symbols
so the public API is properly documented.
In `@mcp_server_dart/lib/src/cli/webmcp_command.dart`:
- Around line 136-149: Document the intent behind the `ok` check in
`webmcp_command.dart` so future maintainers understand why
`toolProof?['fetchCalled'] == false` is required alongside `toolProof?['ok']`.
Add a brief comment near the `ok` assignment in the `toolName != null` branch
explaining that `_cdpInvokeWebMcpTool` is verifying in-page WebMCP execution and
that `fetchCalled` must stay false for this verifier, and note that tools with
legitimate network access may need a different success predicate.
- Around line 184-296: The _cdpInvokeWebMcpTool expression duplicates the same
fetch-wrapping, execute, expectation-checking, and try/catch/finally flow in
both the dart hook fallback and testing API branches. Refactor the embedded JS
in _cdpInvokeWebMcpTool by introducing a single shared helper inside the
expression string that handles interception, execution, result validation, and
error shaping. Then have both the __intentcallWebMcpDartExecute path and the
testing.getTools()/tool.execute path call that helper with only the
branch-specific execute function and metadata. Keep the returned
ok/code/message/toolName/names fields consistent across both paths.
- Around line 158-166: The verdict computation in webmcp_command.dart is too
hard to read because it uses a nested ternary chain inside the map. Refactor the
logic around the verdict field into a small helper method or switch-style branch
near the existing cdpOk, toolName, ok, and logEvidence checks, and have the map
call that helper instead of inlining the conditionals.
In `@mcp_server_dart/lib/src/shared_core/command_executor.dart`:
- Around line 1682-1701: The session connection-mode enum mapping is duplicated
between command_executor’s _toIntentSessionMode and
FlutterSessionConnector._toCoreMode, so extract a shared bidirectional mapper
and use it from both places. Move the
CoreConnectionMode↔IntentSessionConnectionMode switch logic into a common helper
or mapper class, then update _toSessionStartRequest and FlutterSessionConnector
to call that shared implementation. Keep the existing enum coverage intact so
both directions stay consistent in one place.
In `@mcp_server_dart/makefile`:
- Around line 9-10: The build alias step in the Makefile duplicates the compiled
Dart binary with cp, unnecessarily doubling disk usage. Update the alias
creation after dart compile exe in the build target to use a symlink or hardlink
for the fmtk alias instead of copying the artifact, while keeping the
flutter-mcp-toolkit output unchanged.
In `@mcp_server_dart/test/appintents_testing_command_test.dart`:
- Around line 6-132: The tests in appintents_testing_command_test.dart are
currently declared flat under main(), which breaks the test-file convention of
grouping related cases. Wrap the four existing test() cases in a group() inside
main(), using a descriptive group name that matches the AppIntentsTesting
scaffold/fixture behavior, and keep the existing test names and assertions
unchanged; the relevant symbols to place are main(), group(), and the existing
readAppIntentsTestingSampleArguments/readAppIntentsTestingEntityFixtures/emitAppIntentsTestingScaffold
tests.
In `@scripts/run_exec_sweep.sh`:
- Around line 18-33: The cleanup in run_exec_sweep.sh only removes artifacts
from legacy_root and leaves stale files in the per-platform outdir, so reruns
can mix old and new results. Update cleanup_legacy_root_artifacts or add a
dedicated cleanup step around outdir creation to remove prior-run artifacts
there as well, including the attempt-numbered visibility and scroll_to JSON
files produced by the exec_sweep flow. Use the existing symbols legacy_root and
outdir to locate the cleanup logic and ensure the directory is reset before each
run.
- Around line 178-283: The snapshot/visibility helper functions in this script
are duplicated from the runtime enter-text eval script and are already drifting
apart, especially `write_ref_args_for_identifier` with its extra
`require_visible` parameter. Extract the shared logic used by `json_ok`,
`capture_snapshot`, `write_ref_args_for_identifier`, and
`ensure_visible_identifier_args` into a common sourced helper script, then
update both eval scripts to call that shared implementation so behavior stays
consistent.
In `@tool/contracts/check_no_personal_paths.sh`:
- Around line 7-16: Broaden the personal-path detection in the
check_no_personal_paths.sh script, since the current rg pattern only catches
/Users/anton and misses other common home-directory paths. Update the logic
around the matches/pattern lookup to search for additional platform-specific
personal path prefixes such as Linux /home/<user> and any other hardcoded
developer home paths you want to guard against, while keeping the existing ROOT
scan and exclusions intact.
In `@tool/evals/run_runtime_enter_text_greeting.sh`:
- Around line 137-169: The write_reveal_fallback_json helper is hardcoding query
and direction metadata instead of deriving them from its inputs. Update
write_reveal_fallback_json to read the relevant identifier and direction from
args_file (or the passed match/context data) and use those values in the emitted
payload so the function stays correct for any caller. Keep the existing
structure intact, but ensure the fields populated in the JSON reflect the actual
invocation context rather than the current greeting_input_field/down constants.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 502dcc16-dc8c-4400-8a0e-4eafcc706974
⛔ Files ignored due to path filters (6)
docs/assets/flutter-mcp-toolkit-infographic-clean.pngis excluded by!**/*.pngdocs/assets/flutter-mcp-toolkit-infographic.pngis excluded by!**/*.pngflutter_test_app/ios/Runner/Generated/IntentCallGenerated.swiftis excluded by!**/generated/**flutter_test_app/macos/Podfile.lockis excluded by!**/*.lockflutter_test_app/macos/Runner/Generated/IntentCallGenerated.swiftis excluded by!**/generated/**pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (150)
.agents/skills/mcp-harness-repo-maintainer/evals/cases/portable-invocation-trigger.yaml.github/pull_request_template.mdAGENTS.mdCHANGELOG.mdCONTRIBUTING.mdREADME.mdSECURITY.mddecisions/0012_fmtk_cli_alias_and_harness_boundary.mdxdecisions/README.mddecisions/index.mdxdocs.jsondocs/NORTH_STAR.mddocs/ai_agents/marketplace_copy.yamldocs/ai_agents/marketplace_distribution.mdxdocs/ai_agents/overview.mdxdocs/contributing/contribution_guide.mdxdocs/contributing/contributors.mdxdocs/contributing/marketplace_submission_runbook.mdxdocs/evidence/dogfood/dogfood_web_eval.yamldocs/index.mdxdocs/intentcall/README.mddocs/start_here/cli_quick_recipes.mdxdocs/start_here/cli_vs_mcp.mdxdocs/start_here/migration_mcp_call_entry_to_agent_call_entry.mddocs/superpowers/WHATS_NEXT.mddocs/superpowers/evals/2026-05-26-webmcp-verification.mddocs/superpowers/evals/README.mddocs/superpowers/evals/archive/2026-05-26-dogfood-spec-gap-matrix.mdflutter_test_app/INTENTCALL_PLATFORM.mdflutter_test_app/android/app/src/main/res/xml/intentcall_shortcuts.xmlflutter_test_app/ios/Flutter/AppFrameworkInfo.plistflutter_test_app/ios/Podfileflutter_test_app/ios/Runner.xcodeproj/project.pbxprojflutter_test_app/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcschemeflutter_test_app/ios/Runner/AppDelegate.swiftflutter_test_app/ios/Runner/Info.plistflutter_test_app/lib/agent_state.dartflutter_test_app/lib/main.dartflutter_test_app/lib/showcase_screen.dartflutter_test_app/linux/intentcall_protocol.desktopflutter_test_app/macos/Flutter/GeneratedPluginRegistrant.swiftflutter_test_app/macos/Podfileflutter_test_app/macos/Runner.xcodeproj/project.pbxprojflutter_test_app/macos/Runner/Info.plistflutter_test_app/pubspec.yamlflutter_test_app/test/showcase_reactivity_test.dartflutter_test_app/tool/gen_resolution.dartflutter_test_app/tool/intentcall/appintents_testing_entities.jsonflutter_test_app/tool/intentcall/appintents_testing_samples.jsonflutter_test_app/web/agent_manifest.jsonflutter_test_app/web/intentcall_webmcp.generated.jsflutter_test_app/web/manifest.jsonflutter_test_app/windows/intentcall_protocol.regflutter_test_app/windows/intentcall_protocol_msix.xmlinstall.shmakefilemcp_server_dart/README.mdmcp_server_dart/bin/flutter_mcp_toolkit.dartmcp_server_dart/lib/flutter_mcp_core.dartmcp_server_dart/lib/src/capabilities/dynamic_registry/core_dynamic_registry_gateway.dartmcp_server_dart/lib/src/capabilities/dynamic_registry/dynamic_gateway.dartmcp_server_dart/lib/src/capabilities/dynamic_registry/dynamic_registry.dartmcp_server_dart/lib/src/capabilities/dynamic_registry/dynamic_result_normalizer.dartmcp_server_dart/lib/src/capabilities/dynamic_registry/registry_discovery_service.dartmcp_server_dart/lib/src/capabilities/visual_capture/desktop_capture_recovery.dartmcp_server_dart/lib/src/capabilities/visual_capture/desktop_window_screenshot.dartmcp_server_dart/lib/src/capabilities/visual_capture/visual_capture.dartmcp_server_dart/lib/src/cli/appintents_testing_command.dartmcp_server_dart/lib/src/cli/cli.dartmcp_server_dart/lib/src/cli/cli_daemon_server.dartmcp_server_dart/lib/src/cli/diagnostics/bundle_builder.dartmcp_server_dart/lib/src/cli/diagnostics/command_snapshot_service.dartmcp_server_dart/lib/src/cli/diagnostics/diagnostics.dartmcp_server_dart/lib/src/cli/session/flutter_session_connector.dartmcp_server_dart/lib/src/cli/session/session.dartmcp_server_dart/lib/src/cli/session/session_manager.dartmcp_server_dart/lib/src/cli/session/state_lock_manager.dartmcp_server_dart/lib/src/cli/session/state_store.dartmcp_server_dart/lib/src/cli/sessions_persistence/safe_writes.dartmcp_server_dart/lib/src/cli/sessions_persistence/session_persistence.dartmcp_server_dart/lib/src/cli/webmcp_command.dartmcp_server_dart/lib/src/mcp_toolkit_server/mixins/dynamic_registry_integration.dartmcp_server_dart/lib/src/shared_core/agent_result_mapper.dartmcp_server_dart/lib/src/shared_core/command_executor.dartmcp_server_dart/lib/src/shared_core/vm_connections/preconnect.dartmcp_server_dart/lib/src/skill_assets.g.dartmcp_server_dart/makefilemcp_server_dart/pubspec.yamlmcp_server_dart/test/appintents_testing_command_test.dartmcp_server_dart/test/bundle_builder_test.dartmcp_server_dart/test/codegen_sync_command_test.dartmcp_server_dart/test/command_snapshot_service_test.dartmcp_server_dart/test/core_executor_test.dartmcp_server_dart/test/desktop_capture_recovery_test.dartmcp_server_dart/test/dynamic_registry_input_schema_test.dartmcp_server_dart/test/flutter_mcp_example_app_integration_test.dartmcp_server_dart/test/flutter_tool_machine_discovery_test.dartmcp_server_dart/test/platform_view_capture_flow_test.dartmcp_server_dart/test/preconnect_test.dartmcp_server_dart/test/registry_discovery_service_test.dartmcp_server_dart/test/safe_writes_test.dartmcp_server_dart/test/session_manager_test.dartmcp_server_dart/test/state_lock_manager_test.dartmcp_server_dart/test/state_store_test.dartmcp_toolkit/README.mdmcp_toolkit/lib/mcp_toolkit.dartmcp_toolkit/lib/src/mcp_toolkit_binding.dartmcp_toolkit/lib/src/mcp_toolkit_extensions.dartmcp_toolkit/lib/src/services/gesture_interaction_service.dartmcp_toolkit/lib/src/services/reveal_search_service.dartmcp_toolkit/lib/src/services/semantic_snapshot_service.dartmcp_toolkit/pubspec.yamlmcp_toolkit/test/control_flow_service_test.dartmcp_toolkit/test/mcp_toolkit_bootstrap_test.dartmcp_toolkit/test/semantic_snapshot_surface_test.dartpackages/core/README.mdpackages/core/pubspec.yamlpackages/server_capability_core/README.mdpackages/server_capability_core/pubspec.yamlpackages/server_capability_kernel/README.mdpackages/server_capability_kernel/lib/src/host_service.dartpackages/server_capability_kernel/lib/src/resource_registration.dartpackages/server_capability_kernel/lib/src/resource_template_registration.dartpackages/server_capability_kernel/lib/src/tool_registration.dartpackages/server_capability_kernel/pubspec.yamlplugin/.codex-plugin/plugin.jsonplugin/README.mdplugin/skills/flutter-mcp-boundary-audit/reference.mdplugin/skills/flutter-mcp-toolkit-guide/SKILL.mdplugin/skills/flutter-mcp-toolkit-intentcall-migration/SKILL.mdplugin/skills/flutter-mcp-toolkit-maintain-macos/SKILL.mdplugin/skills/flutter-mcp-toolkit-maintain-web/SKILL.mdplugin/skills/flutter-mcp-toolkit-repo-maintainer/SKILL.mdplugin/skills/flutter-mcp-toolkit-setup/SKILL.mdscripts/run_exec_sweep.shscripts/run_web_showcase.shscripts/run_web_showcase_tests.shsteward.yamlsteward/scenarios/mcp_flutter.intentcall-hosted-cutover.yamltool/contracts/check_cli_alias_surface.shtool/contracts/check_intentcall_integration.shtool/contracts/check_no_personal_paths.shtool/evals/run_dogfood_eval.shtool/evals/run_macos_validate_runtime.shtool/evals/run_runtime_enter_text_greeting.shtool/intentcall/check_no_path_deps.shtool/intentcall/check_no_path_deps_test.shtool/intentcall/print_hosted_deps.shtool/intentcall/publish_all.shtool/release/build_release_artifacts.sh
💤 Files with no reviewable changes (7)
- mcp_server_dart/lib/src/cli/cli.dart
- mcp_server_dart/lib/src/cli/sessions_persistence/session_persistence.dart
- mcp_server_dart/lib/src/cli/sessions_persistence/safe_writes.dart
- mcp_server_dart/lib/src/cli/session/session_manager.dart
- flutter_test_app/ios/Flutter/AppFrameworkInfo.plist
- mcp_server_dart/lib/src/cli/session/state_store.dart
- mcp_server_dart/lib/src/cli/session/state_lock_manager.dart
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
mcp_server_dart/docs/enhancement_suggestions.md (1)
23-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win"Implementation" note still references the old
MCPToolDefinitionAPI.The code sample above was migrated to
AgentCallEntry.tool, but the "Implementation" guidance right below (line 46) still says to extendMCPToolDefinition. Worth updating that reference so the suggestion doc is internally consistent post-migration.🤖 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_server_dart/docs/enhancement_suggestions.md` around lines 23 - 41, The documentation still refers to the removed MCPToolDefinition API even though the example now uses AgentCallEntry.tool. Update the “Implementation” guidance in the same suggestion section to reference the current AgentCallEntry.tool-based flow and related symbols like debugToolEntry and debugToolMetadata so the doc stays consistent after the migration.docs/core/dynamic_tools_registry.mdx (1)
83-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLifecycle diagram / Quick Checklist not updated to match new
bootstrapFlutterflow.This section now recommends
MCPToolkitBinding.instance.bootstrapFlutter(...)as the minimal correct setup, but the Lifecycle (ASCII) diagram and the Quick Checklist elsewhere in this same doc still describe the olderinitialize()+initializeFlutterToolkit()+ manualaddEntries(...)flow as the primary path. Consider updating those sections so readers aren't given two conflicting "correct" setups.🤖 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 `@docs/core/dynamic_tools_registry.mdx` around lines 83 - 90, The Lifecycle diagram and Quick Checklist still describe the old initialize()/initializeFlutterToolkit()/addEntries flow, which conflicts with the new bootstrapFlutter setup. Update those doc sections in dynamic_tools_registry.mdx to make MCPToolkitBinding.instance.bootstrapFlutter(...) the primary recommended path, and revise any references to manual initialization or entry registration so they align with the new bootstrapFlutter flow and no longer present two competing “correct” setups.flutter_test_app/lib/intentcall_showcase_entries.dart (2)
6-181: 📐 Maintainability & Code Quality | 🔵 TrivialAdd dartdoc to new public API surface.
None of the new public top-level declarations (
intentCallProtocolScheme,buildIntentCallBridgePingEntry,buildSetGreetingEntry,buildEnableSwitchEntry,buildShowcaseScreenEntityType,seedIntentCallShowcaseEntities) have///documentation.As per coding guidelines, "Document all public members (methods, properties, etc.) with clear, concise explanations of their purpose and usage in Dart."
🤖 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 `@flutter_test_app/lib/intentcall_showcase_entries.dart` around lines 6 - 181, Add dartdoc comments for every new public top-level declaration in intentcall_showcase_entries.dart: document intentCallProtocolScheme, buildIntentCallBridgePingEntry, buildSetGreetingEntry, buildEnableSwitchEntry, buildShowcaseScreenEntityType, and seedIntentCallShowcaseEntities. Keep each comment concise but specific about the purpose and expected usage of the constant/function so the public API surface is properly documented.Source: Coding guidelines
137-137: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeep-link path segment
app_screenis hardcoded independently of the entity ref fields.Each
deepLinkliterally embedsapp_screen, duplicatingnamespace/typeNamefrom the correspondingAgentEntityRef(app/screen). If those values ever diverge, the deep links silently go stale.Also applies to: 153-153, 169-169
🤖 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 `@flutter_test_app/lib/intentcall_showcase_entries.dart` at line 137, The deepLink values in intentcall_showcase_entries are hardcoding the app_screen path segment instead of deriving it from the associated AgentEntityRef fields. Update the deepLink construction in the affected showcase entries to build the path from the same namespace/typeName values used by AgentEntityRef (for example via the shared entity metadata), so the link stays consistent if those values change.flutter_test_app/lib/intentcall_showcase_bootstrap.dart (1)
7-73: 📐 Maintainability & Code Quality | 🔵 TrivialAdd dartdoc to new public bootstrap API.
intentCallProofRegistry,intentCallHost,buildIntentCallProofEntries, andconfigureIntentCallShowcaseare public top-level declarations without///documentation.As per coding guidelines, "Document all public members (methods, properties, etc.) with clear, concise explanations of their purpose and usage in Dart."
🤖 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 `@flutter_test_app/lib/intentcall_showcase_bootstrap.dart` around lines 7 - 73, The new public top-level API in intentcall_showcase_bootstrap.dart is missing required dartdoc. Add concise /// documentation for intentCallProofRegistry, intentCallHost, buildIntentCallProofEntries, and configureIntentCallShowcase, explaining each symbol’s purpose and how callers should use them; keep the docs aligned with the behavior in the bootstrap setup and registry binding flow.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.
Inline comments:
In `@docs/guides/creating_dynamic_tools.mdx`:
- Around line 84-105: The `registerMyTools()` sample has an unclosed
`AgentCallEntry.tool(...)` invocation, so the `helloTool` initialization in the
dynamic tools example is syntactically incomplete. Update the snippet in
`creating_dynamic_tools.mdx` by closing the `AgentCallEntry.tool` call before
the `await MCPToolkitBinding.instance.addEntries(entries: {helloTool});` line,
keeping the `helloTool` variable assignment and surrounding example structure
intact.
---
Nitpick comments:
In `@docs/core/dynamic_tools_registry.mdx`:
- Around line 83-90: The Lifecycle diagram and Quick Checklist still describe
the old initialize()/initializeFlutterToolkit()/addEntries flow, which conflicts
with the new bootstrapFlutter setup. Update those doc sections in
dynamic_tools_registry.mdx to make
MCPToolkitBinding.instance.bootstrapFlutter(...) the primary recommended path,
and revise any references to manual initialization or entry registration so they
align with the new bootstrapFlutter flow and no longer present two competing
“correct” setups.
In `@flutter_test_app/lib/intentcall_showcase_bootstrap.dart`:
- Around line 7-73: The new public top-level API in
intentcall_showcase_bootstrap.dart is missing required dartdoc. Add concise ///
documentation for intentCallProofRegistry, intentCallHost,
buildIntentCallProofEntries, and configureIntentCallShowcase, explaining each
symbol’s purpose and how callers should use them; keep the docs aligned with the
behavior in the bootstrap setup and registry binding flow.
In `@flutter_test_app/lib/intentcall_showcase_entries.dart`:
- Around line 6-181: Add dartdoc comments for every new public top-level
declaration in intentcall_showcase_entries.dart: document
intentCallProtocolScheme, buildIntentCallBridgePingEntry, buildSetGreetingEntry,
buildEnableSwitchEntry, buildShowcaseScreenEntityType, and
seedIntentCallShowcaseEntities. Keep each comment concise but specific about the
purpose and expected usage of the constant/function so the public API surface is
properly documented.
- Line 137: The deepLink values in intentcall_showcase_entries are hardcoding
the app_screen path segment instead of deriving it from the associated
AgentEntityRef fields. Update the deepLink construction in the affected showcase
entries to build the path from the same namespace/typeName values used by
AgentEntityRef (for example via the shared entity metadata), so the link stays
consistent if those values change.
In `@mcp_server_dart/docs/enhancement_suggestions.md`:
- Around line 23-41: The documentation still refers to the removed
MCPToolDefinition API even though the example now uses AgentCallEntry.tool.
Update the “Implementation” guidance in the same suggestion section to reference
the current AgentCallEntry.tool-based flow and related symbols like
debugToolEntry and debugToolMetadata so the doc stays consistent after the
migration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 09e22dff-310b-4de8-8a32-d955cb078839
📒 Files selected for processing (42)
.github/workflows/intentcall_publish_dry_run.ymlAGENTS.mdARCHITECTURE.mdCHANGELOG.mdQUICK_START.mdREADME.mddocs/ai_agents/overview.mdxdocs/core/dynamic_tools_registry.mdxdocs/core/project_architecture.mdxdocs/guides/creating_dynamic_tools.mdxdocs/intentcall/README.mddocs/start_here/docs_map.mdxdocs/start_here/why_this_repo_matters.mdxflutter_test_app/INTENTCALL_PLATFORM.mdflutter_test_app/lib/intentcall_showcase_bootstrap.dartflutter_test_app/lib/intentcall_showcase_entries.dartflutter_test_app/lib/main.dartmakefilemcp_server_dart/docs/SIMPLIFIED_DYNAMIC_REGISTRATION.mdmcp_server_dart/docs/enhancement_suggestions.mdmcp_server_dart/lib/src/shared_core/commands/commands_catalog.dartmcp_server_dart/lib/src/skill_assets.g.dartmcp_server_dart/test/appintents_testing_command_test.dartmcp_server_dart/tool/build_skill_assets.dartmcp_toolkit/CHANGELOG.mdmcp_toolkit/README.mdmcp_toolkit/lib/mcp_toolkit.dartmcp_toolkit/lib/src/toolkits/interaction_toolkit.dartpackages/core/lib/src/tools/interaction_input_schemas.dartpackages/server_capability_core/lib/src/tools/interaction_tools.dartplugin/README.mdplugin/assets/DESIGN.mdplugin/skills/flutter-mcp-boundary-audit/SKILL.mdplugin/skills/flutter-mcp-toolkit-custom-tools/SKILL.mdplugin/skills/flutter-mcp-toolkit-guide/SKILL.mdplugin/skills/flutter-mcp-toolkit-inspect/SKILL.mdplugin/skills/flutter-mcp-toolkit-intentcall-migration/SKILL.mdplugin/skills/flutter-mcp/SKILL.mdtool/contracts/check_intentcall_hosted_consumer.shtool/contracts/check_intentcall_integration.shtool/intentcall/print_hosted_deps.shtool/intentcall/publish_all.sh
💤 Files with no reviewable changes (1)
- tool/intentcall/publish_all.sh
✅ Files skipped from review due to trivial changes (21)
- packages/server_capability_core/lib/src/tools/interaction_tools.dart
- plugin/skills/flutter-mcp-toolkit-custom-tools/SKILL.md
- .github/workflows/intentcall_publish_dry_run.yml
- plugin/skills/flutter-mcp-boundary-audit/SKILL.md
- plugin/assets/DESIGN.md
- mcp_server_dart/lib/src/shared_core/commands/commands_catalog.dart
- plugin/README.md
- plugin/skills/flutter-mcp-toolkit-intentcall-migration/SKILL.md
- plugin/skills/flutter-mcp-toolkit-inspect/SKILL.md
- AGENTS.md
- mcp_server_dart/docs/SIMPLIFIED_DYNAMIC_REGISTRATION.md
- mcp_toolkit/CHANGELOG.md
- mcp_toolkit/lib/src/toolkits/interaction_toolkit.dart
- docs/start_here/docs_map.mdx
- mcp_toolkit/README.md
- packages/core/lib/src/tools/interaction_input_schemas.dart
- QUICK_START.md
- ARCHITECTURE.md
- README.md
- mcp_toolkit/lib/mcp_toolkit.dart
- mcp_server_dart/lib/src/skill_assets.g.dart
🚧 Files skipped from review as they are similar to previous changes (3)
- tool/contracts/check_intentcall_integration.sh
- mcp_server_dart/test/appintents_testing_command_test.dart
- flutter_test_app/INTENTCALL_PLATFORM.md
The flutter-mcp-boundary-audit skill was missing the mode prelude marker required by init writers and skill_assets tests. Regenerate skill_assets.g.dart to fix skill-assets-drift CI. Co-authored-by: Cursor <cursoragent@cursor.com>
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
1 participant
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.
Summary
mcp_server_dart.validate-runtimefallback reporting so missing host context and helper retry details are visible instead of flattened into a generic screenshot failure.Why
Downstream tools need a reusable realtime broker/session foundation without importing Flutter-VM-shaped internals. This keeps Flutter-specific VM service, screenshots, and extension calls as adapters while consuming the shared session/result/registration surfaces through published IntentCall packages.
Validation
flutter pub getresolvesintentcall_core,intentcall_mcp, andintentcall_sessionfrom hostedhttps://pub.dev.dart analyze packages/server_capability_kernel packages/server_capability_core mcp_server_dart mcp_toolkitflutter test test/control_flow_service_test.dart test/agent_entry_helpers_test.dart test/mcp_toolkit_bootstrap_test.dart test/semantic_snapshot_surface_test.dartfrommcp_toolkitdart test mcp_server_dart/test/core_executor_test.dart mcp_server_dart/test/dynamic_registry_input_schema_test.dart mcp_server_dart/test/registry_discovery_service_test.dart mcp_server_dart/test/session_manager_test.dart mcp_server_dart/test/command_snapshot_service_test.dart mcp_server_dart/test/desktop_capture_recovery_test.dart mcp_server_dart/test/platform_view_capture_flow_test.dart mcp_server_dart/test/desktop_window_screenshot_test.dartdart test packages/server_capability_kernel/test packages/server_capability_core/test mcp_server_dart/test/agent_registry_host_test.dart mcp_server_dart/test/host_test.dart mcp_server_dart/test/host_extras_test.dart mcp_server_dart/test/flutter_inspector_resources_test.dartvalidate-runtimewithout global desktop flags:ok:true,verdict:pass_with_fallback, failure details now reportmissing_flutter_device.validate-runtimewith--flutter-device macos --flutter-project-dir flutter_test_app:ok:true,verdict:pass,captureMode:desktop_window, no fallback.scripts/run_exec_sweep.sh:PASS=35 FAIL=0 SKIP=0.steward probe --json --profile quickgit diff --checkNotes
GitNexus change detection originally reported CRITICAL for the branch because the PR intentionally touches the broader session/broker/dynamic-registry flow. The hosted-package integration itself reports low risk in change detection; the shim impact is medium because it feeds the capability-kernel public surface and is covered by capability/host registration tests.
Summary by CodeRabbit
fmtkas a short alias for the Flutter MCP Toolkit CLI (installed alongside the main binaries).fmt_*tool surface to 30.webmcp verifywith tool-specific assertions.validate-runtimeand capture diagnostics output.intentcall_sessioninstead.