feat(interaction): accept fill <target> "" as the clear-field primitive - #2066
Conversation
Emptying an input was not expressible: `fill` refused the empty string
("Expected text to be a non-empty string"), `type` only appends, and `keyboard`
has no delete verb. Clearing a field before typing is a routine QA step, so the
only route was the app's own clear button or N locale-dependent keyboard delete
presses read out of a snapshot.
`fill <target> ""` now means "replace with nothing". Both platforms already own
the clear half of replace, so this is the validation and reporting that stood in
front of it, not a new interaction:
- `stringField` takes an opt-in `allowEmpty`, used only by `fill`'s `text`.
`requiredField` still refuses a MISSING text, so `fill @e57` stays an error
rather than silently erasing the field — `readFillTargetFromPositionals` now
reports `undefined` for "no text argument" instead of collapsing it to `''`.
`type` keeps refusing an empty text: appending nothing is not a clear.
- The Apple runner's empty-text early return skipped the clear while reporting
"typed". For a replacement it now runs `clearTextInput` and verifies the field
came back empty (secure fields stay unverifiable, as elsewhere).
- Android already clears before typing and skips an empty shell/IME write, but
its verifier read a cleared field's absent `text` attribute as a mismatch
against `''`. An empty expectation now accepts null or "".
Whitespace-only text keeps its established per-shape rules; only `''` is new.
Closes callstack#2063
|
[P1] Empty replacement must fail closed when no input is actually observed. On Android, |
Addresses the P1 review on callstack#2066, then closes the same fail-open class on the backends the PR did not reach: - Android: an empty expectation no longer matches when the verification scan observed NO input node at all — actual is null both for a cleared field and for a wrong point/lost focus, and three empty samples of nothing were a stable success for a clear that never touched a field. - Apple runner: when the empty-replacement path cannot resolve a clear target (including the synthesized first-responder route, whose target carries no element), it returns the typed TEXT_INPUT_NOT_FOCUSED failure instead of falling through to the vacuous-typing verified-success return. Regression runs in the ios.yml XCTest lane. - webdriver: fill is tap + sendKeys and owns no clear mechanism, so an empty fill refuses as UNSUPPORTED_OPERATION before touching the device, instead of reporting a clear it cannot perform. - linux + web coordinate fill: typing zero characters over the select-all selection left the old value intact; the empty fill now deletes the selection. - recording: an empty --record-as literal matches inside every string; it now parameterizes only the fill's own text field instead of rewriting every empty field and empty evidence label in the entry. (The session-wide echo registry already excluded empty literals.) - help: the text-entry topic taught agents that fill "" is not a clear-field command; it now states the new contract. Each new test was observed red against the pre-fix code.
…rifies Live Pixel 9 emulator, adb-shell channel: clearing the Settings search field succeeded on the device but reported 'Android fill verification failed', because a cleared EditText dumps its HINT as text — getText() returns the hint for an empty field on modern Android, so 'Search settings' read back as a residual value. This is the same placeholder-as-value trap the Apple runner already handles with treatingPlaceholderAsEmpty. The helper now emits hint-showing (isShowingHintText, API 26+), the hierarchy parser carries it, and fill verification matches against the field's VALUE — hint-only text is an empty value, for empty and non-empty expectations alike. A field whose real value equals its hint string keeps failing the clear check: only the authoritative flag, never the text, says it is a hint. Raw uiautomator dumps carry no such fact and keep the fail-closed behavior. Live evidence, both admission channels, after this fix: test-ime and adb-shell clears both report Filled 0 chars with the field back on its placeholder; the pre-fix adb-shell run failed closed (never a false success).
…fill clear - android adb-shell: the delete burst is sized from the value being REMOVED (pre-mutation read; the attempt's cap when unreadable), not from the empty incoming text, which sent the 12/24-delete minimums and could never empty a field longer than 36 characters. - android: the unconfirmed soft-success no longer applies to an empty expectation — nothing app-formats the empty value, so residue after a clear is a failed clear, and the soft-success also skipped the second, bigger delete burst. - android masked fields: an empty expectation accepts an observed masked node with no dump text (a masked field WITH content dumps its bullet run), so clearing a password field no longer fails after the clear worked — matching iOS, where a secure-field clear succeeds unverified. - find: 'find <q> fill ""' now reaches the fill leaf as the clear request on both the CLI reader and the daemon positional parse; a MISSING value keeps its refusal at each producer, so the typed value: string contract is unchanged. - maestro export: a recorded clear exports as tapOn + eraseText instead of a vacuous inputText: "" (with the 50-character-default warning). - the missing-text refusals teach the clear form: (use "" to clear the field). Full unit suite green (1061 files); each behavioral fix carries a test observed red against the prior code.
|
Addressed the P1 and everything an adversarial pass + live runs surfaced, in three maintainer commits (95a5238, 120e689, f9c8555): The two P1 fail-opens
Found while collecting the live Android evidence
Backends the PR's "every backend" claim missed
Live evidence (iPhone 17 Pro sim + own Pixel 9 Pro XL emulator, isolated state dir):
Known bounds (documented, fail-closed, follow-up material): the clear has no repair pass on iOS, so fields beyond the 120-delete cap fail honestly; a cleared non-EditText-classed custom input that drops out of the dump fails verification rather than passing; web Validation: full unit-core green (1061 files / 8110 tests), |
|
Re-review at |
…pt branches The review commits pushed parseFillTarget and fillAndroid over the complexity gate (13 cyclomatic each). Each fill target shape parses in its own function sharing one missing-text response, and the adb-shell attempt (clear sizing + clear + type + verify) moves out of the fill loop. Behavior-preserving; the existing tests cover every branch.
thymikee
added
the
ready-for-human
|
Re-reviewed exact head The later maintainer follow-up supplies Android adb-shell and test-IME live-clear evidence, and its growth accounting supersedes the stale PR body: the diff is about +329 non-test production lines, below the size-escalation threshold. CI remains in progress. Residual risk is declared: Maestro export uses bounded |
Design pass after review: the missing-vs-empty rule and the observed- value rule each had several owners; now each has one. - parseFillTarget decodes ONCE through readFillTargetFromPositionals — which already owns shape detection and documents the undefined-vs-'' contract on DecodedFillTarget — and keeps only what the wire owns: versioned-ref admission, the selector whitespace rule, and the daemon responses. This deletes the point branch's duplicated slicing, the hasFillText guard, and the three per-shape parse functions. - observedAndroidValue() is the single statement of Android's value rule (absent attribute and hint-only text are the empty value); the text branch, the match rule, and the masked branch all consume it. The masked branch thereby gains the hint-showing collapse it was missing, and isAcceptableAndroidFillMatch narrows to plain strings. - The empty-text-is-clear contract is stated once, on Interactor.fill in contracts, instead of implied per backend. Behavior-preserving except the masked+hint gain; the existing tests cover every branch (494 Android, 15 fill-target).
|
Clean delta review: the parse and Android observation refactors preserve the fail-closed empty-fill semantics; prior live evidence remains valid. All relevant CI is green except the pending iOS smoke lane. No new finding; the ready-for-human label remains appropriate. |
…027-9d286e * origin/main: feat(interaction): accept `fill <target> ""` as the clear-field primitive (#2066) fix(ci): spawn the differential's agent-device CLI as argv, not one option (#2069) fix(daemon): keep the recovery hint on Maestro replay errors (#2075) # Conflicts: # src/commands/command-input.ts
2 participants
Closes #2063
Summary
Emptying a text input was not expressible.
fillrefused the empty string,typeonly appends,keyboardhas no delete verb, andget valueis not a thing — so clearing a field before typing,a routine QA step, was reachable only through the app's own clear button or N presses of a
locale-dependent keyboard delete key read out of a snapshot first.
fill <target> ""now means "replace with nothing". Both platforms already own the clear half ofreplace — this change is mostly the validation and reporting standing in front of it, not a new
interaction:
stringFieldtakes an opt-inallowEmpty, used by exactly one field:fill'stext.typekeeps refusing an empty text (appending nothing is not a clear), and everyother string field keeps the non-empty rule.
requiredFieldstill refuses a missingtext, andreadFillTargetFromPositionalsnow reportsundefinedfor "no text argument" instead ofcollapsing it to
''. Sofill @e57staysExpected text to be set.rather than silentlyerasing the field — the one hazard in relaxing this check.
typeTextReliablyearly-returned on empty text and reportedverified: true,which would have made the whole feature a no-op that claims success. For
repairMode == .replacementit now runs the existingclearTextInputand verifies the field came back empty;secure fields, whose value is unreadable, stay unverified rather than reporting a mismatch.
type(append) and unrepaired entry keep the old no-op return.fillAndroidalready clears before typing and already skips an empty shell/IMEwrite, so the device behaviour was correct — but
isAcceptableAndroidFillMatchread a clearedEditText's absenttextattribute (null) as a mismatch against'', so a successful clearwould have been reported
text_mismatch. An empty expectation now acceptsnullor"".Whitespace-only text keeps its established per-shape rules (payload on ref/coordinate fills,
refused on selector fills); only
''is new.Tests
New/updated, each observed red against the pre-fix code:
src/commands/interaction/metadata.test.ts(new) —fillaccepts'', still refuses a missingtextand a non-stringtext;typestill refuses''.src/commands/interaction/interactions.test.ts— the CLI reader distinguishes an empty textpositional from an absent one on all three target shapes, and the daemon writer projects
''asits own positional.
src/daemon/handlers/__tests__/interaction-touch-targets.test.ts—parseFillTargetaccepts''after a ref, a selector and coordinates; still refuses a missing text on all three; keepsthe per-shape whitespace rules.
src/platforms/android/__tests__/text-input-fill.test.ts— a cleared field verifies against anempty expectation whether the dump carries
text=""or notextattribute, and a non-emptyfield still fails one.
pnpm check:quick,pnpm test:unit(8100 passed) andpnpm check:command-docsare green.Live check
iOS 26.5 simulator (iPhone 17), DemoApp with a text field, runner rebuilt via
pnpm build:xcuitest:ios, CLI from this clone:fill @e53 "hello world""hello world"fill @e53 """Text input"(placeholder — empty)fill @e53 ""againfill @e53INVALID_ARGS: Expected text to be set.fill 'id="form.textField"' "second value""second value"fill 'id="form.textField"' ""Session closed and daemon stopped afterwards.
Android is not live-verified — no emulator was available. Its half is a verification-predicate
fix plus paths that already short-circuit on empty text, and it carries unit coverage, but a
maintainer should run one
fill <target> ""on an emulator (both the adb-shell and test-IMEchannels) before merging.
What a maintainer might push back on
clearas its own command was the issue's other option. Not taken: it would add a registrydescriptor, a capability admission entry, an MCP tool, help, and recording/replay handling for
behaviour
fillalready performs.fill <target> ""is the same primitive with no new surface.reuses
clearTextInput; the non-replacement path is byte-identical. It is the one piece with nounit coverage (the runner has none), so the live table above is its only evidence.
"Filled 0 chars"is accurate but reads oddly for a clear. Changing it means touching twomessage builders in different daemon modules (
touch-runtime.ts,handlers/interaction-touch-payload.ts); left out to keep the diff on the primitive. Happy to addit if preferred.
""is accepted where" "is still refused on selector fills. Thatis deliberate — an explicit clear versus an accidentally blank argument — but it is a rule worth
agreeing on.
optimizeFillActionskips the selector optimisation for empty text, so a recordedclear falls back to the ref form. Out of scope here; flag it if it should be part of this change.