feat(runtime): add pilot runtime scroll and text replay#115
Closed
drown0315 wants to merge 15 commits into
Closed
Conversation
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.
Intent
The developer wanted to implement issue 8, the type action for editable text targets, in the Flutter Pilot Dart CLI while following the repository code conventions. They clarified that type should directly clear existing text using a stable clear operation, then enter the new text one character at a time rather than pasting or setting the whole string at once. They expected non-editable targets to fail with a clear message and asked for the work to be validated with the standard Dart/Flutter analyze and test commands plus a real Pixel smoke run. After the implementation was completed, they asked whether the non-editable-target acceptance criterion was implemented, then requested that the changes be committed.
What Changed
pilot_runtimereplay support for scroll and typed text, including direct editable text handling, stricter Finder matching, descendant target disambiguation, and runtime protocol/client/binding updates.mcp_flutterpath compatible while extending runtime selection, adapter contracts, smoke verification, and runner tests around the new pilot runtime behavior.Risk Assessment
✅ Low: The remaining changes are well-bounded to the runtime adapter/action paths and smoke documentation/verification, and the previously identified regressions appear addressed without introducing a substantiated merge-blocking issue.
Testing
I ran the relevant Dart runner/adapter/CLI tests, reran the Flutter widget/runtime tests with the correct Flutter runner, captured focused widget-test evidence for editable and non-editable Type Action behavior, and performed real macOS
flutter_pilot testsmoke runs showingtypereplacing text before a user-visiblewaitForand non-editable targets failing clearly; no Pixel device was available, and no screenshot artifact was captured becausepilot_runtimescreenshot capture is not implemented yet.Evidence: passing Type Action CLI smoke transcript
Evidence: non-editable Type Action CLI smoke transcript
Evidence: focused pilot_runtime Type Action widget-test transcript
Evidence: passing Type Action run report
Evidence: passing Type Action timeline
Evidence: non-editable Type Action run report
Evidence: non-editable Type Action timeline
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 3 issues found → auto-fixed (9) ✅
packages/pilot_runtime/lib/src/pilot_runtime_binding.dart:130- Real VM service extension parameters arrive as strings, but_handleScrollrequiresdeltaX/deltaYto already beintordouble.PilotRuntimeClient.performScroll()sends numeric args, the VM extension registrar receivesMap<String, String>, and_requiredDoublethen throws before any scroll can run. Parse numeric strings here or normalize the registrar payload before dispatch.lib/src/runtime/runtime_adapter_selector.dart:33- The default runtime path now selectspilot_runtime, butPilotRuntimeAdapterstill throws not-implemented for screenshots, snapshots, and logs. That means existing Scenarios withcapture: {}and automatic failure diagnostics can regress immediately after this switch. This also conflicts with the pilot_runtime PRD in this diff, which still says to keepMcpFlutterRuntimeAdapteras default until parity/calibration.packages/pilot_runtime/lib/src/scroll_performer.dart:117- A targeted scroll handle that resolves to a wrapper subtree with multiple scrollables silently uses the first descendant scrollable. The documented contract says the handle must identify a scrollable or a subtree that contains one scrollable, and the product guardrail is to fail rather than pick arbitrary targets. Collect descendant scrollables and returnnotScrollable/ambiguous failure unless exactly one is present.🔧 Fix: Fix runtime scroll and adapter selection
3 issues (2 errors, 1 warning) still open:
.github/workflows/real-mcp-flutter-smoke.yml:68- The real smoke workflow still runsdart run tool/run_mcp_flutter_smoke.dart, but this change deletes that tool. Manualworkflow_dispatchsmoke runs will fail before exercising any runtime path; either keep the tool or update the workflow to the new pilot_runtime smoke command.lib/src/commands/app_setup_commands.dart:125-initnow runsflutter pub add pilot_runtime, butpackages/pilot_runtime/pubspec.yamlispublish_to: none, so the package is not addable from pub.dev. This also prepares apps forpilot_runtimewhilemcp_flutterremains the default adapter, so a freshly initialized app will not work with the default test path.lib/src/runtime/mcp_flutter_runtime_adapter.dart:309- The defaultmcp_flutteradapter now only reads semantic snapshot nodes fromdata['nodes']. The previous adapter intentionally acceptednodes,widgets, anditems, plus alternate field names such astext/name,valueKey, andbounds; those response shapes now produce zero matches or missing metadata whilemcp_flutteris still the default runtime. Restore the tolerant decoding unless the runtime contract was explicitly narrowed with calibration evidence.🔧 Fix: Restore default runtime setup and smoke compatibility
3 issues (1 error, 2 warnings) still open:
.github/workflows/real-mcp-flutter-smoke.yml:67- The opt-in real smoke workflow still validates and runs themcp_flutterpath, but the smoke app now registers onlyPilotRuntimeBindingand no longer includesmcp_toolkit. That workflow will fail before it can exercise the runtime path; either restore an MCP-enabled smoke entrypoint/dependency or update the workflow/tool to run the intendedpilot_runtimesmoke path explicitly.lib/src/runtime/mcp_flutter_runtime_adapter.dart:417- Screenshot decoding for the defaultmcp_flutteradapter was narrowed: ifimagesexists but its first entry is not a string, this returns null without checking later entries, nested image maps,screenshots, or priorbase64/pngfields. Those shapes were previously accepted, so defaultcapture: {}and failure diagnostics can lose screenshots even though the toolkit returned one.README.md:113- The README now tells users thatinitinstallspilot_runtimeand printsPilotRuntimeBinding.ensureInitialized()guidance, but the actualinit/doctorimplementation still installs and checksmcp_toolkitplusMCPToolkitBinding.instance.bootstrapFlutter. This will send users down the wrong setup path unless the app setup commands are migrated in the same change or the docs stay aligned with the current default setup.🔧 Fix: Restore MCP smoke and screenshot compatibility
2 warnings still open:
packages/pilot_runtime/lib/src/finder_resolver.dart:132- Wrapper finders only compare against the first descendant text/type. A keyed orbyWidgetwrapper with an unrelated leading child will failbyText/byTypeeven if a later descendant satisfies the requested constraint, which breaks the documented subtree-combination behavior. Collect descendant evidence or search for the requested value instead of storing only the first descendant.packages/pilot_runtime/lib/src/scroll_performer.dart:141- Untargeted scroll treats every visibleScrollableas a candidate for the primary scrollable. Common forms with aTextFieldcan include an internal editable-text scrollable, making the page scroll ambiguous and causingscrollwithout a finder to fail even when there is one user-facing page scrollable. Filter out internal/editable scrollables or use a stronger primary-scrollable selection rule.🔧 Fix: Fix finder and scroll target selection
3 issues (1 error, 2 warnings) still open:
packages/pilot_runtime/lib/src/text_performer.dart:58-_editableStateForaccepts anyEditableTextState, so read-only or disabled text fields are still cleared and mutated by the new direct text path. Those are non-editable targets and should return the samenotEditableTextfailure instead of updating the controller; check the resolvedEditableTextstate/widget editability before returning it.packages/pilot_runtime/lib/src/finder_resolver.dart:191- Descendant text/type evidence is collected without applying the resolver's visibility filter. A keyed orbyWidgetwrapper can now satisfybyText/byTypefrom an offstage, hidden, zero-size, or transparent descendant, producing a visible Finder Match for UI the user cannot interact with. Reuse_isVisiblewhile collecting descendant evidence so subtree matching preserves the visible-target contract.examples/smoke_app/README.md:43- The pilot_runtime acceptance commands no longer setFLUTTER_PILOT_RUNTIME=pilot_runtime, butRuntimeAdapterSelectorstill defaults tomcp_flutter. Running these documented tap/scroll demo commands will select the wrong adapter against targets that only registerPilotRuntimeBinding, so the acceptance path fails before exercising pilot_runtime.🔧 Fix: Fix runtime text and finder guards
3 warnings still open:
packages/pilot_runtime/lib/src/text_performer.dart:61- Typing through a wrapper handle can mutate an arbitrary editable descendant. A keyed/byWidget wrapper with multiple editable text fields, or a hidden editable before the visible one, still resolves as one Finder Match, but_editableStateForreturns the first descendantEditableTextStateit finds. Collect visible editable descendants and fail unless exactly one editable target is present.packages/pilot_runtime/lib/src/scroll_performer.dart:127- Targeted scroll descendant collection counts hidden/offstage scrollables. A wrapper with one visible scrollable and one hidden scrollable will fail as ambiguous, and a wrapper with only a hidden scrollable can be selected despite the visible-target contract. Reuse the resolver visibility rules while collecting descendant scrollables.README.md:147- The smoke scenario was moved toexamples/smoke_app/smoke_scenario.yaml, but the README quick-start commands still reference the deletedexamples/smoke_scenario.yamlpath. These documentedvalidateandtestcommands now fail unless the path is updated; the same stale path appears inREADME_zh.md.🔧 Fix: Tighten runtime descendant target selection
2 warnings still open:
examples/smoke_app/smoke_scenario.yaml:22- The real smoke Scenario now ends after the Finder steps, and the verifier was narrowed to those labels, so the opt-in mcp_flutter smoke path no longer exercises capture artifacts. Screenshot/log/widget-tree capture regressions can now pass the smoke workflow; restore a capture step/verifier check or add a separate capture smoke if this coverage was not intentionally removed.docs-internal/scenario-yaml.md:330- The smoke Scenario was moved toexamples/smoke_app/smoke_scenario.yaml, but these internal Scenario YAML examples still reference the deletedexamples/smoke_scenario.yamlpath. Update the exampletest/validatecommands so the documented commands run.🔧 Fix: Restore smoke capture coverage and docs
1 warning still open:
lib/src/smoke_verifier.dart:200- The smoke verifier now only checks that capture artifact records are listed inrun_report.json; it no longer verifies that each listed capture artifact path exists on disk. A run can reportscreenshot,widgetTree, andlogsartifacts whose files were never written and still pass the opt-in smoke workflow, so artifact-writing regressions are no longer caught. Restore the run-directory path existence check for each required capture artifact.🔧 Fix: Verify smoke capture artifact files
2 warnings still open:
lib/src/runtime/mcp_flutter_runtime_adapter.dart:408- The defaultmcp_flutteradapter now sendsscroll.distanceas a double, while the previous implementation rounded it to an int before callingflutter-mcp-toolkit. Sincemcp_flutterremains the default path, non-integer Scenario deltas can regress default scroll replay if the toolkit expects the prior integer distance shape; keeppilot_runtimedeltas as doubles but restore rounding for this adapter command.examples/smoke_app/README.md:3- The smoke-app README says the top-level smoke app is for the realpilot_runtimepath, but the documented command uses the default adapter andlib/main.dartstill registersMCPToolkitBinding, notPilotRuntimeBinding. Following these instructions will exercisemcp_flutter, notpilot_runtime; either relabel this section as the MCP smoke path or document the pilot_runtime env/entrypoint explicitly.🔧 Fix: Restore MCP smoke scroll compatibility
✅ Re-checked - no issues remain.
⏭️ **Test** - skipped
flutter devicesreported only macOS and Chrome, with no Pixel, Android device, or emulator available. I ran the realflutter_pilot testsmoke on the available macOS target instead.dart test test/scenario_runner_test.dart test/pilot_runtime_adapter_test.dart packages/pilot_runtime/test/pilot_runtime_finder_test.dart packages/pilot_runtime/test/pilot_runtime_client_test.dart(initial mixed runner attempt; Flutter tests failed to load under plain Dart, then rerun correctly withflutter test)dart test test/scenario_runner_test.dart test/pilot_runtime_adapter_test.dart test/mcp_flutter_runtime_adapter_test.dart test/cli_test.dartflutter test test/pilot_runtime_binding_test.dart test/pilot_runtime_client_test.dart test/pilot_runtime_finder_test.dartflutter devicesscript -q /var/folders/1b/xbh8qb0d0j11fzyfm0qs0pf40000gn/T/no-mistakes-evidence/01KX3JB72Z0R2W7M73MNM1KY2J/pilot_runtime_type_widget_tests.txt flutter test test/pilot_runtime_finder_test.dart --name "clears and appends editable text|type fails clearly for a non-editable resolved handle|type fails clearly for a read-only text field"script -q /var/folders/1b/xbh8qb0d0j11fzyfm0qs0pf40000gn/T/no-mistakes-evidence/01KX3JB72Z0R2W7M73MNM1KY2J/pilot_runtime_type_success_cli_pass.txt env FLUTTER_PILOT_RUNTIME=pilot_runtime /Users/drown/development/flutter/bin/dart run ../../bin/flutter_pilot.dart test pilot/no_mistakes_type_success.yaml --target lib/no_mistakes_type_demo.dart --device macosscript -q /var/folders/1b/xbh8qb0d0j11fzyfm0qs0pf40000gn/T/no-mistakes-evidence/01KX3JB72Z0R2W7M73MNM1KY2J/pilot_runtime_type_non_editable_cli.txt env FLUTTER_PILOT_RUNTIME=pilot_runtime /Users/drown/development/flutter/bin/dart run ../../bin/flutter_pilot.dart test pilot/no_mistakes_type_non_editable.yaml --target lib/no_mistakes_type_demo.dart --device macosgit status --shortand transient artifact checks after cleanup✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.