fix(tool-server): press named key after text in keyboard tool#486
Open
j-piasecki wants to merge 2 commits into
Open
fix(tool-server): press named key after text in keyboard tool#486j-piasecki wants to merge 2 commits into
j-piasecki wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes the keyboard tool’s combined { text, key } behavior so it types the text first and presses the named key afterward, aligning with the intended “type, then submit” workflow across simulator-server, Chromium (CDP), and Vega implementations. It also documents this ordering in tool/run-sequence help and adds regression tests to prevent reintroducing the prior key-first behavior.
Changes:
- Reordered keyboard injection so
{ text, key }executes as text → key across simulator-server, Chromium, and Vega, while still failing fast on unknown key names. - Added
resolveVegaNamedKeycodeto validate Vega named keys independently from injection. - Documented the ordering in the keyboard tool and
run-sequence, and added a regression test suite covering ordering + fail-fast behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/tool-server/test/keyboard-key-order.test.ts | Adds regression coverage for “text then key” ordering and fail-fast unknown-key validation across simulator-server, Chromium, and Vega. |
| packages/tool-server/src/utils/vega-input.ts | Introduces resolveVegaNamedKeycode and refactors named-key injection to use it. |
| packages/tool-server/src/tools/run-sequence/index.ts | Updates run-sequence documentation to state key-after-text semantics and shows the single-step “type and submit” example. |
| packages/tool-server/src/tools/keyboard/simulator-server-keys.ts | Resolves named key up-front (fail-fast) and presses it after typing text. |
| packages/tool-server/src/tools/keyboard/platforms/vega.ts | Validates named key before text injection and injects the key after text. |
| packages/tool-server/src/tools/keyboard/platforms/chromium.ts | Resolves named key up-front (fail-fast) and dispatches it after typing text. |
| packages/tool-server/src/tools/keyboard/index.ts | Documents the key-after-text behavior in schema descriptions and tool description. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Problem
A
keyboardcall combiningtextandkeypressed the named key before typing the text — the reverse of the intended "type, then submit". Every platform branch had the same ordering (copied verbatim from the original iOS implementation into each port), and nothing in the schema or description documented it. Ironically, run-sequence's own description lists "typing then pressing enter" as a canonical use case.The observed failure: an agent ran
{ text: "jane@test.dev", key: "enter" }against a freshly-focused email field in a React Native dev build. Enter fired first into the still-empty field, submitting and blurring it; the text was then typed with nothing focused. RN'sRCTKeyCommandsmatches Cmd-registered dev commands even with no modifier held when there is no first responder (_flags == flags || flags == 0inmatchesInput:flags:), so the baredin ".dev" toggled the dev menu mid-sequence, and every subsequent tap landed on the dev-menu overlay.Fix
textfirst and presskeyafter it:simulator-server-keys.ts(iOS simulator / Android emulator)keyboard/platforms/chromium.ts(Electron / CDP)keyboard/platforms/vega.ts(Fire TV)key; unchanged)resolveVegaNamedKeycodeinvega-input.tsso validation is separable from injection.keyparam description and tool description, and in run-sequence's args table. The run-sequence "type text and submit" example now shows the combined single-step form, since that is the pattern agents naturally reach for.Verification
test/keyboard-key-order.test.ts: 6 tests asserting key-after-text ordering and fail-fast on unknown keys across all three branches.await-ui-elementAndroidhiddenedge case) fails identically on cleanmain, as do the twotscerrors from a stale@argent/telemetrybuild.