mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
fix/android-clear-verify-selectall
332 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Ignacy Łątka
|
496b38c94d |
fix(keyboard): don't delete a field's own placeholder as clear residue
uiautomator renders an empty field's hint into the same `text` attribute it
uses for a value, and `measureFocusedTextLength` reads that attribute raw.
That was harmless while the reading was only a COUNT — over-measuring costs
a few no-op backspaces — but the select-all's read-back turned it into a
PREDICATE, where an over-measurement is a delete run against a field the
clear already emptied. Reproduced on the exact shape this file documents for
API 36 (`text="Search settings" … hint="Search settings"`, on a level that
HAS `keycombination`, so squarely on this path): a clear that fully worked
emitted `input keyevent 123` plus 23 backspaces into the empty box. Past
MAX_DELETE_COUNT it is worse than wasteful — a 171-character placeholder
turned the same successful clear into a 400 with the `{ clear, text }` call's
text never sent.
`hint` is the discriminator and `parseUiAutomatorXml` already surfaces it, so
the read-back now counts a `text` equal to its own `hint` as empty. Only the
read-back: the sizing read wants the opposite error, since under-measuring
there truncates the field, and the levels it serves emit no `hint` anyway.
That made three rules that all fork the same way, so `maxDumps` and
`unmeasurableIsUnreadable` collapse into one `readBack` flag naming the
caller rather than each rule.
Also, from the same pass:
- The tool description promised "a rejected request never changes the field …
a 400 leaves the field exactly as it was". The rescue arm's over-length
refusal is a 400 raised after a select-all and a DEL have gone out, which
the refusal's own text admits. The paragraph now names that one exception.
- The refusal said the DEL "removes one character where it lands" — true only
where the chord was swallowed. A chord that took makes the same DEL remove
the whole selection; both outcomes are named now.
- The `cleared` docstring listed three escapes from the repair. There are
four, and one is wider than stated: an unmeasurable focused editable
ANYWHERE on screen discards the reading, not only one in the target field.
- `devtoolsHierarchyReader` and `AndroidClearOptions.readHierarchy` still
described a legacy-only consumer and the wrong failure mode for the modern
one, which is now this reader's dominant caller.
- The per-device chain's rationale said a character arriving after the DEL
"replaces the whole selection". The DEL consumes the selection; what a
character landing in the read-back window actually gets is measured as
residue and backspaced away.
Mutation-checked: dropping the placeholder rule, applying it to the sizing
read, matching on `hint`'s presence instead of its value, and each of
`readBack`'s three rules all fail their own tests and nothing else.
|
||
|
Ignacy Łątka
|
56d03bf788 |
fix(keyboard): say what the read-back does not repair, and stop asserting what it cannot know
Seven reviewer findings on the verify leg, all of them about a claim the code does not hold up. The read-back leaves two shapes exactly as broken as the fast path does, and neither was named anywhere. `measureFocusedTextLength` counts focused `EditText` nodes only, so a focused WebView input or a custom widget reads as unreadable and gets no rescue at all — reproduced in the unit harness: a focused `android.webkit.WebView` carrying `Monda` sends the chord and the DEL and nothing else, under `cleared: true`. And the rescue is `clearByDeleting`, whose `KEYCODE_MOVE_END` is end-of-LINE, so a multi-line residue is emptied only down to the end of the caret's line — reproduced likewise: `AAAA\nBB\nC` minus the DEL's character emits `input keyevent 123` plus seventeen backspaces and completes clean. The `clear` parameter, the tool description and the `cleared` docstring each said the repair without either qualifier. Two messages still stated device state they cannot establish. The rescue-path over-length refusal opened with "The select-all did not take on this field" and then conceded, three sentences later, that the count can be another window's field or an empty field's placeholder; it now reports what it actually observed — the field read back non-empty. The interrupted-DEL report enumerated two states, but a delete already handed to the device keeps landing after adb is killed, so on a widget that swallowed the chord the field is one character short: a third state, now named. The tool description's 158-backspace fallback sentence had lost its older-levels scoping to a sentence inserted before it, and reads as though a modern level could fire it; it says "on those older levels" now. Also covers the verify leg's `options.readHierarchy` source. Every positive-residue test seeded the read-back through the exec-out dump, which on the ordinary describe -> tap -> keyboard order is the source that cannot answer — dropping `options.readHierarchy` from the verify read left all 202 tests green while disabling the repair whenever the devtools helper holds the UiAutomation connection. Mutation-checked: restoring the flat cause, the two-state enumeration, or the undefined `readHierarchy` each fails exactly its own test. |
||
|
Ignacy Łątka
|
b605e1e509 |
fix(keyboard): don't assert a device state the clear cannot know
Two error messages stated as fact what the code has just finished establishing it cannot know. The interrupted-DEL report said the select-all landed and survives — but `input` exits 0 whether or not the selection took, which is this branch's whole premise, so on a swallowed-chord widget that message sent a replacement into an intact, unselected value while promising it would replace a live selection. The rescue-path over-length refusal said the field HAS been modified and is one character down — but the reading that reached it is the longest focused EditText on screen, which can be another window's field or an empty field's placeholder, so what the DEL actually took is unknowable from here. Both now hedge and point at reading the field, which was already their remedy. The tool description also scoped the over-150 refusal to levels without `input keycombination`; through the verify leg the same refusal is reachable on a level that has the subcommand, so the sentence says so now. |
||
|
Ignacy Łątka
|
b6e903800d |
fix(keyboard): don't read a password field's blind floor as clear residue
The select-all read-back inherited the sizing read's BLIND_DELETE_COUNT floor, so every successful clear of an unmeasurable field — a focused password box — read 150 back and fired a 158-keyevent run into the field just emptied. The read-back now reports an unmeasurable focused editable as unreadable, which leaves the fast path alone. Also refreshes the keyboard tool and result docs that still said Android never reads the field back. |
||
|
Ignacy Łątka
|
f9acc326fc |
fix(keyboard): read the field back, instead of trusting a select-all that exits 0
`input keycombination 113 29` reports exit 0 whether or not the selection took. On a React Native `TextInput` it repeatedly does not: 15 of 85 clears driven through the tool on a Pixel 6 / API 34 came back short by exactly the character the DEL removed, each returning `cleared: true` over a value the `text` was then appended to. The same 85 against a native `EditText` ran clean, so nothing about the call distinguishes the two. No hold duration fixes it — `-t 100` still missed 2/30 and `-t 300` 7/30 — so the clear reads the field back and hands a residue to the selection-free delete run, which needs no selection to be correct and cleared the same field 50/50. Only a positive reading redirects: an unreadable one leaves the fast path as it was rather than spending a blind run on a clear that likely worked, and the read-back does not retry the way the sizing read does, whose stake is a truncated field rather than a skipped rescue. The rescue carries its measurement down so the delete run does not dump again, and the over-length refusal it can reach no longer tells that caller the field is untouched — reached from the select-all path, one character is already gone and the level does have `keycombination`. Also mocks `adbExecOutBinary` in keyboard-android.test.ts, which addressed `emulator-5554` for real: with an emulator attached those tests dumped a live device, so a clear's command stream depended on what was on its screen. Verified: 85/85 clean through the tool-server against the field that broke it, against 15/85 corrupt on the same path unfixed. |
||
|
Hubert Gancarczyk
|
75e4e04c08 |
docs(flow): stop offering two clears a TV cannot do, and an assert that does not exist
`flow-yaml.md` ended its three-TV-family paragraph with "Delete an existing value with repeated backspaces, or use the field's own clear affordance". The `keyboard` tool rejects `key` outright on a TV target, so repeated backspaces are available on Vega alone; on Apple TV and Android TV the field's own affordance is the only way. Both halves were offered to all three families. The clear-only parse test justified itself with "empty the box, then assert the empty state", but there is no such assert: `equals: ""` and `contains: ""` are rejected at parse time and `matches: '^$'` never fires, because absent or empty text is not a haystack — as `references/asserting-field-values.md` already says in words. The comment now names the reasons a clear-only step is legitimate on its own, and points at that reference for what can actually be asserted afterwards. |
||
|
Hubert Gancarczyk
|
2e508c02d9 |
fix(flow): report a type step cancelled mid-Enter as a skip, not a pass
The post-dispatch abort re-check sits before the submit gate, so nothing stood
between the Enter dispatch and `return { ok: true }`. `dispatchOrAbort`
reclassifies only a dispatch that REJECTS under an aborted signal, and the
Android backend takes no signal at all, so a cancel landing mid-Enter left the
call to resolve and handed back `true`. The same cancellation window therefore
flipped a step between `pass` and `skip` on which of its two keyboard calls
happened to be in flight — and the `pass` arm is the one where the submit really
was sent after the caller gave up, which is what the re-check above exists to
prevent.
Driven through the real runner with a signal-less keyboard mock: `{ text }` with
`submit` defaulting on reported `type:pass` with both dispatches issued, and
reports `type:skip` now. An explicit `submit: true` clear-only step, where the
Enter is likewise the last dispatch, is pinned alongside it.
Also corrects two claims the same block makes:
- the backends that take no signal. Chromium now honours one, so Android is the
only one that takes none — and the two that DO only narrow the window rather
than close it, since each checks between keystrokes.
- the `runType` parenthetical's premise. `scrollIncrement` is not "one call per
step": a `scroll-to` calls it up to MAX_SCROLL_ITERATIONS times through
`scrollToVisible`, which guards the signal at the head of every iteration
itself — so the dispatch is safe to leave bare, but not for the stated reason.
|
||
|
Hubert Gancarczyk
|
d91bf6ee51 |
fix(flow): report a cancelled clear-only step as a skip, not a pass
`dispatchOrAbort` reclassifies only a dispatch that REJECTS while the
signal is aborted, and the Android and Chromium keyboard backends take
no signal at all — so a cancel landing mid-call leaves the call to
resolve and returns `true`. The re-check that catches that lived inside
the `if (step.submit ?? step.text !== undefined)` block, which is false
for a clear-only step, so the two shapes diverged: cancel a run during a
`{ into, clear: true }` step's one dispatch and it reported `pass`, while
the identical cancellation on a step with text reported `skip`.
The check moves out of the submit gate. The submitting path is unchanged
— it still runs before the Enter dispatch — and the clear-only path
gains it.
`flow-abort.test.ts` grows the twin of its existing clear-only case: the
existing one mocks the keyboard to throw, which `if (!sent)` already
caught; this one resolves inside the cancelled window, and asserts the
clear DID go out so it cannot pass by the step never reaching the
keyboard. It fails with `type:pass` before this change.
Two review findings on the same file that are documentation, not
behaviour:
- The `FocusOutcome` docblock named the FOCUS_REPORTING_SOURCES guard as
a producer of "unobservable". No flow can reach it: `type` runs only on
ios, android and chromium, and each hard-codes a source already in the
set. It is kept as a forward guard — a new platform would otherwise
poll the full timeout on every type step — and the docblock now says
which of the two producers is live.
- `focusedFromInside`'s containment conjunct was pinned by nothing: every
fixture pairing an out-of-target focused node with a small enough frame
is refused by the namesake counts first, so dropping `frameWithin`
reddened no test. The new fixture puts a same-`resource-id` sibling
outside the container's box, where both counts hold and containment is
the only thing left to refuse on — the ordinary Android shape, since a
resource-id names the layout slot. Verified by mutation: dropping the
conjunct now fails that test and only that test.
|
||
|
Hubert Gancarczyk
|
da61986dec |
test: pin this round's fixes, and the gaps the review's sweep found
New coverage for each fix, every one of them mutation-checked (revert the fix,
the named test goes red):
- an editing host holding identified or password content beside the named
paragraph refuses, while the bare `<div contenteditable><p>…</p></div>`
still clears;
- the tapped element unmounting while a namesake holds focus refuses, and
the same page with it mounted still clears;
- the three ways a focused node borrowed another element's evidence — a
namesake the tap creates, a container that grows onto a field the tap never
reached, a focused field that renames itself out of the namesake set — plus
the control that keeps the name;
- a slow-but-answering tree source is not reported as an outage;
- the chromium walker keeps an invisible element that holds the caret (and
one that contains it), still prunes when the caret is elsewhere, never
resurrects a display:none subtree, and keeps a box-less focused node.
And the gaps the sweep listed:
- no test parsed a non-object `type` body, so the message could name one of
the two valid shapes and stay green;
- no test covered a non-abort `gesture-tap` rejection in `runType`;
- nothing bounded the focus window from ABOVE;
- the skill docs' `clear` YAML examples were outside the parse guard's slice;
- `carriesFocus`'s recursion fixture put the flag at depth 0, where a
non-recursive check reads the same — give the shadow wrapper an id;
- four weak assertions: `toBeDefined()` on a `findById` that returns null,
an `?.identifier` that reads undefined when nothing resolved, a `toBe`
between two strings that pass as `"" === ""`, and a flat-leaf invariant
omitting `focused`, which is now producible.
Also drops the wall-clock half of the early-exit test: `elapsed < 3000` timed
the whole step — temp dir, device resolution, the settle, two keyboard
dispatches — so it measured the host and failed at 3066ms and 3388ms under
load, while the read count beside it pins the exit exactly at any speed.
|
||
|
Hubert Gancarczyk
|
2b9e7b8de8 |
fix: the LOW findings around the gate, the parser and the walker
- `focusedFromInside`'s trailing `underTap.every(...)` is unreachable as false once the `controls` rule holds, so drop it and state why it is implied. - A `type:` step with no `into` rendered its entry as the bare word `undefined`, which in a multi-step flow does not say which step is broken. Catch it in the type arm and echo the step, the way the `claer` typo path already does. - The two clear refusals blamed an overlay or a sibling for what is routinely a field OVERHANGING the container the selector names — default content-box sizing does it for a few pixels, and neither message admitted the possibility. Reproduced and re-read on Chrome 151. - `chromium.ts` claimed the focus flag reaches every describe consumer: `hasContent` does not list `focused`, so a node whose only signal is focus is dropped from the agent-facing render. Verified — an anonymous empty focused contenteditable is absent, the same editor holding a `<p>` is shown. - It also claimed focus inside an iframe leaves the tree with no flag at all. True in the light DOM; inside an OPEN shadow root the HOST is flagged, since the excluded iframe is the root's activeElement. Verified both ways. - The zero-frame sibling drop now consults `focusedSelf` like the two structural collapses beside it — same class of drop, same consequence. |
||
|
Hubert Gancarczyk
|
7076b5f0f8 |
test: pin every fix in this round, and the gaps the review's sweep found
Each of these was verified by mutation: apply the pre-fix code, the named test
goes red; revert, green. Thirteen mutations, thirteen killed.
New coverage for the fixes:
- a focus read that HANGS past the window rather than throwing (H1), asserting
both the outage refusal and that the step answered while the read was still
outstanding - the bound is what keeps "within 3000ms" true;
- a container with a GAP down the middle, once with two controls and once with
one (H3), so what decides is the number of controls and not the gap;
- a focused wrapper around one anonymous FIELD (M3), the editing host's twin;
- `type: { clear: true }` with no `into` as a validation failure, not a crash (L1);
- a replaced getRootNode still reporting focus (L3), and a page describing in
full with ShadowRoot set to a non-constructor (L5).
And the gaps the review named:
- T1 the enclosing-pad sweep now starts at 1px, where the defect lived;
- T4 an UNIDENTIFIED field's draft hoisting into its container's text and
out-ranking a real control for a tap - the three tests beside it all give
their field a placeholder or an id, so they pinned the shielded case while
reading as proof of the general property;
- T5 the new asserting-field-values reference gets a parser guard: `equals: ""`
and `contains: ""` really are rejected, `matches: "^$"` really does parse;
- T6/T7 the ambiguity fixtures now exercise the gates they name. The anonymous
row put focus on the field the tap MISSED, which later gates refuse on their
own, so both arms passed with the ambiguity gate deleted; it now differs in
the colliding name alone, and the same-id re-layout gets the tree order where
"first match wins" would hand back the focused row.
|
||
|
Hubert Gancarczyk
|
dff8420aec |
fix: the LOW findings around the focus read, the walker and the parser
L1 - `type: { clear: true }` with no `into` crashed with "Cannot read
properties of undefined" instead of failing validation: JSON.stringify returns
undefined for undefined, and badEntry read .length off it. The crash predates
the branch, but `clear` in the allowlist is a new door onto it - at the merge
base the same flow failed cleanly as an unknown key. Both `clear` and `text`
forms now return FLOW_ENTRY_UNRECOGNIZED naming `type.into`.
L3 - a page that REPLACES Element.prototype.getRootNode (ShadyDOM,
webcomponents.js) hands back an object that is neither a ShadowRoot nor the
document, and the focus read answered null for it: verified on Chrome 151, the
flag vanished from every node on the page, which is the no-focus tree a
destructive clear goes through on. It falls back to the document's
activeElement - the read this had at the merge base. Focus survives the
replacement now, and an open shadow root still flags the inner element only.
L4 - the iframe descent is dead in a renderer (walk() bails on an inner
documentElement, whose constructor belongs to the inner realm), so it is gone
along with the file header's claim to walk subdocuments. Verified before and
after: an <iframe srcdoc> leaves no trace either way. Its only test passed
because the harness is single-realm, so it went too (T8).
L5 - `typeof ShadowRoot === "undefined"` does not cover a page assigning a
non-constructor. With window.ShadowRoot = null, ShadowRoot.prototype threw
before a single node was walked and describe failed for the whole page
(reproduced). Both that read and the instanceof test now require a function.
L6, L7, L8 - three comments corrected against what the code does: preTapTree is
the last read that SUCCEEDED and can predate the tap by a failed read's whole
timeout; the plain-type early exit deliberately does not fire on a tree that
flags focus nowhere, so that shape still pays the full wait; and no flow report
is written to disk - the reason travels to stdout and the agent's context,
which is reason enough to name no element text.
|
||
|
Hubert Gancarczyk
|
3385468cbd |
fix(flow): size the containment test exactly, so no pad hides inside its slack
frameNoLargerThan carried the same per-dimension slack frameWithin applies per edge, so a focused node padded symmetrically around the target by half the threshold satisfied both and took the clear. On a 900px window that is a trap one pixel larger on every edge: reproduced on Chrome 151, where a focusin trap holding DRAFT-do-not-erase over an <input> passed the step, was emptied and rewritten with the new address, and left the named input holding its old value. There is no tolerance that fixes it. A trap enclosing the field by 1px and a child overhanging its wrapper by 1px are the same geometry, and the flow trees carry no ancestry to tell them apart - flattenHoisting returns leaves under one root on every platform. So the size test is exact: equal frames still confirm (the everyday wrapper-around-input shape, verified live), and any overhang refuses, which fails the step loudly instead of emptying a field the step never named. The overhang sweep moves with the contract: pad 0 is now the admitting case and 1-3px refuse, where the 1-2px arm previously pinned the tolerance this defect lived in. |
||
|
Hubert Gancarczyk
|
2b77ccf42a |
test: close the gaps the review's mutation sweep left open
Every pin below was mutation-checked: the pre-fix code was restored in `flow-actions.ts` / `chromium.ts` and each named test went red. - `sameElement`'s LABEL fallback had no coverage, and it is the whole name-is-not-an-identity defect. Every fixture in the runner file gives its nodes a resource-id, so the anonymous path — the everyday React Native and plain-DOM shape — never ran. `anonymousRowXml` runs it both ways: a namesake and a differently named sibling. - `trackTarget`'s proximity tie-break had none either. `twoSameIdRowsXml` is the re-layout that made it adopt the wrong namesake. - `focusedOwnsTargetText` needed the trap whose OWN text equals the target's; the existing editor fixture only ever exercised the arm that should confirm. - `requireEvidence`'s safety half was unpinned AND the test written for it did not reach it: `overlayFocusXml` carries no `email-wrapper`, so every overlay read was consumed by selector resolution before the tap went out and the focus poll's first read already confirmed. `overlayOverWrapperXml` carries the wrapper, so the clear really has to poll past an overlapping read. - The textarea change had no coverage past the walker itself. Four cases now run the real walker THROUGH the real flow adapter and matcher: the container hoist, the page-wide `visible` check, the tap ranking, and the clear-only proof that still has to work on an unlabelled composer — plus an `<input>`-vs-`<textarea>` spelling case. - `describe-chromium-script.test.ts` certified behaviour that cannot happen: it asserted focus inside a same-origin iframe is carried by the inner element. The walker never descends — `el instanceof Element` rejects the inner `documentElement`, whose constructor belongs to the OUTER realm — and the mock passed only because it is single-realm. Verified live on Chrome 151: an `<iframe srcdoc>` with a unique marker leaves no trace in describe, and focusing the inner input leaves the tree with NO focus flag at all. That is the "unobservable" state a destructive clear dispatches on, so the test now pins it as such. - The refusal tests ran at ~75% of vitest's 5000ms default (measured 3757-3760ms) on the one file whose names would make a load-induced timeout look like a real destructive-clear regression. They get their own testTimeout. |
||
|
Hubert Gancarczyk
|
991b228256 |
fix: four LOW findings around the clear's focus wait and the shadow-root read
- A page that removes `Element.prototype.shadowRoot` took describe down entirely. `protoGetter` falls back to a direct property read when the descriptor is absent — this file's own documented threat model — so a `<form>` holding `<input name="shadowRoot">` hands the CONTROL element to the new ShadowRoot `activeElement` accessor, and Blink throws `Illegal invocation` for the whole page. Verified live: `CHROMIUM_DESCRIBE_FAILED` for every element until the descriptor was restored, and clean with the `isShadowRoot` guard the same function already applies to `root` two lines down. The base could not throw here — it only ever read `shadow.children`. The mock ShadowRoot's accessor is brand-checked now, so a test can see this at all. - A focused node whose frame EQUALS the target's was classified as "encloses". `frameWithin`'s slack is per-edge, so identical frames satisfy it, and the refusal then tells the author to point the selector at the input itself when it already does. `frameNoLargerThan` — added by this PR for exactly this distinction — is now consulted here as it is in `focusedOwnsTargetText`. - The focus wait could overrun its stated budget by a whole tree read. The deadline was checked only AFTER `fetchFlowTree`, while the sleep is capped to land exactly on it, so one more read always started past it — 1-2s of a `uiautomator dump` beyond the 3000ms every refusal message claims the window was. - The `FlowStep` union admitted a `type` step with neither `text` nor `clear`: it serializes to YAML the parser rejects, labels itself `⇐ (clear only)` in a recording summary, and dispatches no keyboard call at all — a silently passing no-op. Pairing the two fields in the union keeps the parser the only thing that can build one. The remaining LOW findings need no change: a labelled `<textarea>`'s 200-char truncation and its live contents feeding the settle fingerprint both went with the textarea fix, which stops the contents becoming node text at all. |
||
|
Hubert Gancarczyk
|
645771dd55 |
fix(describe): spell a <textarea>'s contents the way the field holds them
`accessibleName` whitespace-normalized a `<textarea>`'s value where the `<input>` path deliberately did not, so the same typed text was assertable back in one element type and not the other: `equals: " hello "` matched at the base and stopped matching, and a two-line value came back spelled as one line while the identical string in an `<input>` came back raw. Nothing warned an author which spelling applied to which element. The normalization was there because the walker also emitted the value as node text, and two spellings of one value cannot both be quoted by a failing assert. The contents now reach the tree as exactly one string, so that string is the one the field holds. Verified live: an `equals` on `" hello "` passes against a textarea and an input alike. |
||
|
Hubert Gancarczyk
|
c46eb09844 |
fix(flow): bound the focus wait's dark tail, instead of failing open on it
`lastRead` is only ever written by a read that SUCCEEDED, so the "unreadable" outcome fired only when EVERY read in the window threw. One successful poll disarmed it permanently — including the first, which is the one most likely to report "no focus yet", because the app's focus round-trip has not finished. That maps to "unobservable", the one non-confirmed outcome `runType` deliberately lets a destructive clear through on. A transient tree-source outage in the tail of the window therefore converted a correct refusal into a blind clear. Reproduced on Chrome 151 against this branch's tool-server: nothing focused at the first poll, the tree read poisoned from t+600ms, focus landing on `#other` at t+900ms where no read could see it. The step passed and wrote `WIPED` into `#other`, a field it never named; the same page with the reads left alone refused, and so did the same page poisoned before the first read. The file already states the rule 700 lines up — `CONDITION_DARK_TAIL_TOLERANCE_MS`, "a verdict narrated from the reads before the darkness would describe a screen nobody saw at the deadline". This applies it to the one place where that stale verdict authorises a destructive action, counted in consecutive failed polls rather than in milliseconds: one poll costs a whole `uiautomator dump` on Android, so an elapsed-time budget would read a slow device as an outage. One failure is still the last-poll blip the tolerance exists to absorb. The refusal now says the tree stopped answering rather than that it could not be read at all, which was only ever true of the every-read-failed case. |
||
|
Hubert Gancarczyk
|
9743fbfcaf |
fix(flow): stop a read that cannot SEE focus erasing one that found it
The focus wait classified an empty `focused[]` as the observation "no-focus", which `giveUp` maps to "unobservable" — the one non-confirmed outcome `runType` lets a destructive clear through on. `lastRead` is latest-read-wins with no floor, so a single read in which the focused element's leaf is simply MISSING erased everything the earlier reads had established. Nothing throws; the tree is complete and still holds the target. Both flow adapters produce exactly that for a frame clipping to zero area, which is what keyboard avoidance does to a field it scrolls away, and on Android what a healthy `uiautomator` dump reports for a focused view whose bounds have collapsed. Reproduced on Chrome 151 against this branch's tool-server: `#other` holds the caret and never loses it; the tap on the target does not move focus; collapsing `#other`'s box 1500ms into the wait took the step from "focus never reached" to a pass that emptied `#other` and wrote `WIPED` into it — with `#other` still `document.activeElement` at the end. The same page without the collapse refuses. So a read that saw nothing no longer overwrites one that saw where focus is. Latest-read-wins still decides between sightings, which is what keeps a blur on the focusing tap from being answered with history, and a window in which NO read ever saw focus is still "unobservable" — verified live that a page with nothing focused still dispatches the clear, which is what keeps the directive working on an iOS build whose injected framework omits `firstResponder`. The residual `runType` documents narrows accordingly: an app that blurs, or a field that scrolled away, now only reads as unobservable when it happened before the first read. |
||
|
Hubert Gancarczyk
|
85fa54a06a |
fix(describe): stop a <textarea>'s draft reading as text the page displays
The walker gave a `<textarea>` its live `el.value` as the node's own text. That
fixed the real problem it was aimed at — child text is the markup DEFAULT and
never tracks the value — but node text is what the page DISPLAYS: it hoists into
an ancestor's `subtreeText`, a `text` selector matches it, and the resolver
ranks an exact field match on it above a substring hit. A draft sitting in a
composer therefore behaved like text the page was showing.
Measured on Chrome 151 against this branch's tool-server, each against the same
page with only this hunk changed:
- `assert: { text: { in: { id: chat }, contains: "hello team" } }` PASSED with
the message list empty and the draft still in the box, so a saved regression
test written that way can never go red;
- `assert: { visible: "Alpha" }` PASSED on the composer holding "Alpha" — the
very "assert the consequence instead" pattern the new reference prescribes;
- `tap: { text: "Save" }` landed in a note whose draft was the word "Save",
not on the Save button, whose label is the only "Save" on the page.
A field's contents are not the page's text and there is no third channel to tell
them apart in, so a `<textarea>` now reads its contents back exactly as an
`<input>` does: through `accessibleName`'s value fallback, which an aria-label
or a placeholder pre-empts. All three repros are refusals again, a `tap` lands
on the button, and the clear-only proof still works on an unlabelled composer
(`hidden` fails before the clear, passes after).
Two MEDIUMs fall out with it: a >200-character value can no longer be reported
twice (the dedup compared an unsliced text to an already-sliced name), and a
labelled textarea's contents no longer reach a failing assert's quoted text, so
a secret typed into one stays out of the flow report.
|
||
|
Hubert Gancarczyk
|
591c8ea4fd |
docs: correct nine claims this PR's own code or platforms disprove
Comments and skill pages that describe something other than what happens. No
behaviour change.
- `live-authoring.md` told an agent to KEEP a recorded `tool: keyboard` clear
raw, on the false premise that `type:` accepts only `into`/`text`/`submit`
(`flow-utils.ts` allowlists `clear`) and pointing at a table exception this PR
deleted. It is the raw form that is unsafe: it replays with no focus tap and
no focus check. Converting it is the whole mitigation, and that paragraph
said not to.
- The same page claimed an unconfirmed poll falls through to typing. It does
for a plain `type`; a `clear` hard-fails, which is what `flow-yaml.md`
already says.
- `dispatchOrAbort`'s docstring said `keyboard` discards the `ToolContext`. All
three platforms `clear` supports forward the signal and check it between
keypresses, so an abort-driven rejection is the designed common case there —
which is what makes the wrapper load-bearing rather than defensive.
- The `requireEvidence` fast path was justified by uiautomator flagging a
WebView instead of the EditText inside it. Surveyed across eight screens on
API 36 / WebView 151, including a real in-app WebView, the WebView is
`focused="false"` and the leaf EditText carries the flag. The exit stays; its
stated reason does not.
- `FOCUS_REPORTING_SOURCES` said a source outside the set never reports focus,
naming Vega. Vega's source-parser does set it and `flow-vega-tree` preserves
it; it is excluded because `runDirective` refuses `type` there first.
- The Android TV comment had a `clear` step failing at the first keyboard
dispatch, which presupposes the focus gate passed. `android-devtools` is in
the reporting set, so the wait reports "unconfirmed" and the refusal returns
before any dispatch — as `flow-yaml.md` says.
- `flow-run.ts` cited `(down)` as a sibling option echo twenty lines above the
branch that suppresses exactly that value.
- `asserting-field-values.md` described the textarea case as either/or, gave an
over-broad rule for when an `<input>` exposes its value (`accessibleName`
gates on truthy/resolving, so `aria-label=""` still falls through), and
recommended `assert: { hidden: … }` as working on every platform when it
passes vacuously on exactly the platforms where the contents never reach the
tree. It now also states the secret exposure a `<textarea>` carries.
- `flow-keyboard-clear.test.ts`'s header still gave the allowlist as the reason
the round trip is worth pinning. The round trip is; that reason is not.
|
||
|
Hubert Gancarczyk
|
d1e28a412d |
test(flow): pin the shapes the static fixtures could not express
Every focus fixture held the tree constant across polls and split its rows unevenly, which is what let five defects through a suite that was otherwise green. Each test below fails against the code as it stood one commit before its fix, and each ships with the control that keeps the fix from being a blanket refusal. - an EVENLY split row whose focused half is not the one the tap hit refuses, and the same row with focus on the half the tap DID hit still clears; - a focused input overhanging its wrapper by 1-2px still clears, and 3px — six pixels of extra width on a 1080px screen — refuses, which pins both sides of the containment epsilon instead of only the far one; - an overlay appearing over the tap point AFTER the tap refuses, and the same overlay already there at tap time still clears; - a named container that GROWS between the tap and the poll still clears; - a selector that re-resolves to a better-ranked node after the tap still clears; - a rich-text composer whose only text sits in a child node clears, and the same editor holding a second paragraph refuses; - a `<textarea>` holding a newline reports its contents once, not once raw and once normalized. Also pinned, all three of which a mutation survived: a `clear` keeps polling past an intermediate overlap read instead of refusing on it (the safety half of `requireEvidence`, whose perf half was already covered), the focus flag is suppressed on a shadow host when the flag sits NESTED inside its subtree rather than one level down, and no refusal reason quotes the focused element's own text — the invariant that keeps a field's value out of a report on disk, until now asserted only in a comment. |
||
|
Hubert Gancarczyk
|
fd4aa70e56 |
docs(test): drop the append model from the two tests still teaching it
`1ab12bd2` corrected "typing appends" everywhere in this branch except here. The test NAME - which is what shows up in suite output, so it is read outside the file - said "instead of silently appending", and its comment "an append, which fails much later at an assert". A third copy in `flow-tools.test.ts` glossed `←` as "(append)" while documenting a production comment that says the opposite: `flow-finish-recording.ts` renders it as "types into whatever is already there - NOT necessarily appending". The measured behaviour is per-platform: on Android and Chromium the focusing tap lands the caret mid-field and the new text is spliced INTO the old value; only on iOS does the caret jump to the end and append. |
||
|
Hubert Gancarczyk
|
6552874e66 |
fix(describe): give a textarea its LIVE value, not an empty string
`2f8d2ffe` stopped a `<textarea>` emitting its markup default - correct,
since child text is the authored content and never tracks `el.value` -
but did it by dropping the text entirely, on the grounds that "the
accessible name already carries the live value". That only holds for a
textarea with no aria-label, no aria-labelledby and no placeholder:
`accessibleName` returns those first and only then falls through to
`el.value`. With any of them the node carried NEITHER - the name was the
label and the text was "" - so the field's contents reached no describe
consumer at all.
Reading `el.value` fixes both: the live contents are what the field
shows, which is what every other element's own text already means, and
an unlabelled textarea still de-duplicates against its name exactly as
an `<input>` does.
On Chrome 151 through the branch tool-server:
textarea "Notes" id="t1" <- before, contents gone
textarea "Notes" value="initial content one" id="t1" <- after
assert { text: { in: { id: t1 }, contains: "initial content" } }
before -> fail, 'its text was "Notes"'
after -> pass
and the case `2f8d2ffe` exists for still holds: clear + type "final" on
`<textarea id=t3>initial content three</textarea>` leaves an
`equals: "final"` assert passing, with the default gone.
The test added with that commit exercised the labelled case specifically -
the one where nothing was left behind - and could not see it, because the
mock built a plain element that the script's `instanceof
HTMLTextAreaElement` branches never recognised.
|
||
|
Hubert Gancarczyk
|
e163ff6114 |
fix(describe): stop a collapsed element taking the focus flag with it
The focus block ran at the END of walk() - after both collapse returns -
so an element the walker collapses never reached it. The host was already
suppressed on the assumption that the inner element would carry the flag,
and when the shadow root's activeElement was collapsible BOTH candidates
disappeared and the tree carried no focus at all. `runType` reads that as
"nothing was focused", the one non-confirmed outcome it dispatches a
destructive clear on.
The collapsible-and-focusable shape is the standard rich-text composer: a
bare `<div contenteditable><p>…</p></div>`, which is what Quill /
ProseMirror / Lexical render on a single-paragraph document - box-less,
one child, no id, role or own text. Put it in an open shadow root (any
Lit/Polymer component) and both halves apply at once.
Two changes. The flag is computed ahead of the collapses and keeps the
node alive like a name or an id does - being the caret's element is
targeting information in its own right. And the host is suppressed only
once its shadow subtree has ACTUALLY produced the flag, so a pruned or
collapsed inner element leaves the host reporting rather than nobody.
On Chrome 151 with a `focusin` trap holding the shadow editor:
1 paragraph, before -> pass, editor's draft replaced, #q untouched
1 paragraph, after -> describe shows `div [focused]` inside #shost,
step fails, both intact
2 paragraphs -> refused either way (the editor survives collapse)
|
||
|
Hubert Gancarczyk
|
e206645e76 |
fix(describe): route the focus reads back through the captured getters
`getActiveElement` was captured for a stated hazard - Document's named
getter is [LegacyOverrideBuiltIns], so a `<form name="activeElement">`
shadows the property - and then referenced nowhere. `root.activeElement`
is a raw read, and `getRootNode()` returns the Document for every
light-DOM element, so for the whole page that WAS a raw
`document.activeElement`. `el.shadowRoot` was read raw too, three lines
from the descent that goes through `getShadowRoot` under a comment naming
that same hazard.
On Chrome 151 through the branch tool-server, with a
`<form name="activeElement">` wrapping the fields:
document.activeElement (raw) -> FORM#signin
Document.prototype accessor -> BODY (nothing focused)
describe -> form id="signin" [focused]
and `type: { into: { id: email }, clear: true }` on that page hard-failed
with "the only element reporting focus CONTAINS id=email" - a focus trap
that was not there. After the fix the form is not flagged and the clear
passes, replacing #email.
The Document read now goes through `getActiveElement`, the ShadowRoot
read through a `ShadowRoot.prototype` accessor captured the same way, and
the delegation check reuses the `shadow` the descent already captured.
Also pins the hunk `7323eb45` added, which nothing reached: the mock DOM
defined no Document constructor (so protoGetter degraded to direct reads),
no ShadowRoot, and no `getRootNode`, which made both halves of the focus
block unreachable from a test. Three single-line mutations now fail -
reading the document instead of the element's own root, `delegates =
false`, and the raw `root.activeElement` - where all three left the suite
green before.
|
||
|
Hubert Gancarczyk
|
7af7161e14 |
fix(flow): stop the containment slack admitting an ENCLOSING focused node
`frameWithin`'s epsilon is per-EDGE, so a frame overhanging the target by under the tolerance on EVERY side satisfies "sits inside it" while being a full epsilon larger in each dimension - which is the `encloses` shape the same file refuses as "not evidence", and gives `clearRefusalReason` its own arm to refuse. A focused `android.webkit.WebView` clearing its target by 4px on a 1080x1920 screen passed containment and took the clear, with the step reporting a pass on the field it never touched. The confirm arm now also compares EXTENTS: a node that stands for the target is not bigger than it. That catches a symmetric pad at half the per-edge threshold while still admitting the overhang the epsilon exists for (a child a border's width wider than the box it sits in). The docblock's rationale was wrong too - it described absorbing disagreement between "three different adapters", but both frames come out of one tree read by one adapter, so there is nothing cross-adapter to absorb. It now gives the reason that applies, matching the identical `WITHIN_EPS` in `ui-tree-match` and `EDGE_EPS` in this file. Pinned at pad 4 / 5 / 12 / 100: the guard's removal fails 4 and 5, and widening FRAME_CONTAINMENT_EPSILON to 0.05 fails 4, 5 and 12 - the value was previously unpinned in either direction. |
||
|
Hubert Gancarczyk
|
0d7a50d457 |
perf(flow): stop a plain type polling for a verdict it never reads
Only "confirmed" returned early, and only a `clear` reads the outcome - so a plain `type` polled to the full TYPE_FOCUS_TIMEOUT_MS on every shape that base returned from immediately. Base ended the wait as soon as ANY focus-flagged node overlapped the target, and the shape that lost it is the ordinary hybrid one: uiautomator puts focused="true" on the enclosing `android.webkit.WebView`, not on the `EditText` inside it, and an enclosing node can no longer confirm. A WebView-hosted form of n fields paid n x 3s on the path with no clear at all. `waitForFocus` now takes `requireEvidence`. A caller that will not act on the verdict stops at the first read that settles it; a `clear` polls on, unchanged. Pinned by a test that counts tree reads (3, not ~13) and asserts the step finishes inside the focus budget - it fails at 3773ms with the early exit removed. On Chrome 151 a plain type into a field under a focused-form shape runs in 1.08s. |
||
|
Hubert Gancarczyk
|
2feb70f123 |
fix(flow): confirm a clear from the input the tap hit, not any inside the box
The containment arm accepted ANY focus-flagged node inside the target,
with no check that it was the node under the point `runType` just tapped.
Its only discriminator was a role test on the TARGET - "skip this arm when
the target is itself a text input" - and that vocabulary misses in both
directions against the roles the adapters really emit:
- over-match, so a legitimate wrapper clear is REFUSED. Material's
`TextInputLayout` is the non-editable wrapper carrying the app's
resource-id, and `deriveUiAutomatorRole` matches "textinput" on the
short class name, so it derives `TextField`; ARIA 1.1 puts `combobox`
on the wrapper around the input, which is in the anchored group.
- under-match, so an overlay is ADMITTED. A `contenteditable` composer
is role `div` and React Native's iOS host views come through as
`AXStaticText`, so the arm fires for exactly the shape it was scoped
to exclude.
And on a container holding more than one input - a currency/amount row,
an OTP row, a card number/expiry/cvc row - there was no discriminator at
all: the step cleared whichever sibling reported focus and passed on the
container.
The arm now asks whether the focused node covers where the tap LANDS (the
centre of the target's current frame, recomputed per round so keyboard
avoidance cannot stale it). Structure would be sharper, but both the
Android and the iOS full-hierarchy adapters flatten to leaves under one
root, so a wrapper and its input arrive as siblings and no ancestry is
available. `TEXT_INPUT_ROLE` / `isTextInput` are gone.
Reproduced on Chrome 151 through the branch tool-server, all four before
and after:
composer + focused #mention overlay -> was pass, #mention overwritten
-> now fail, both intact
#amount-row over currency + amount -> was pass, #currency overwritten
-> now fail, both intact
role=combobox wrapper -> was fail ("a focus trap")
-> now pass, replaced
plain div wrapper (control) -> pass either way
|
||
|
Hubert Gancarczyk
|
24d8b73d49 |
fix(flow): resolve a type step's two halves with one ranked resolver
`flowSelectorToNode` picked its node with a bare `firstInReadingOrder`
while `flowSelectorToFrame` went through `selectorToFrame`, which RANKS
matches (exact field hits first, then smallest area). The two feed
opposite halves of one `type` step: the frame is what gets TAPPED, the
node is what the focus check judges identity against.
On the everyday label-above-input shape they disagree. A `{ text: Email }`
selector matches the label as a substring and the field exactly, so the
tap landed on the field while the identity check resolved to the label -
and could then never match. A `clear` hard-failed with "focus never
reached ...", sending the author after a selector that was already
resolving to the input; a plain `type` burned the whole focus timeout on
every such step.
Reproduced on Chrome 151 through the branch tool-server, `<div id=hdr>
Search results</div>` above `<input id=q aria-label=Search value="OLD
QUERY">`: the no-clear control typed into `#q` and passed in 4.8s, the
same selector with `clear: true` failed and left `#q` untouched. After
the fix the clear passes and `#q` reads "REPLACED", and the control drops
to 1.3s.
`selectorToFrame` is now `selectorToNode(...)?.frame`, so there is one
ranking to keep in step instead of two.
|
||
|
Hubert Gancarczyk
|
035afe4bb4 |
fix(flow): judge focus by identity, and flag the element that really holds it
Review drove two more shapes through the branch build against Chrome 150,
in opposite directions.
A focused node INSIDE the target's frame was accepted as proof, so a
suggestion popover overlaying a composer had its own field cleared and
overwritten while the step reported a pass on the composer it never
touched (`#body` byte-identical, `#mention` replaced, 3/3). Only the
enclosing direction was guarded.
And with focus inside an open shadow root `document.activeElement` is the
HOST, so the describe walker flagged the host - which contains the inner
input by construction. Naming the shadow's own input therefore failed
with "the only element reporting focus CONTAINS it", advice the flow was
already following, while the `keyboard` tool it was about to call handles
that shape correctly. Naming the host instead passed and cleared a
different child.
So the walker now flags the activeElement of the element's own ROOT, not
its document, and excludes a host that delegates into its shadow root -
one flag, on the element holding the keys. And the focus wait resolves
the target NODE, not just its frame: identity is confirmation, while
geometry is accepted only for a target that is not itself a text input,
which is what separates "the input inside the testID container I named"
from "an overlay sitting on the field I named". The two now behave:
`into: { id: sinput }` clears the shadow input and leaves its sibling
alone; the popover case fails and touches nothing; the focus trap and the
ordinary replace are unchanged.
An overlap that is neither is its own outcome with its own advice, rather
than being told it hit a shadow host.
Also here: the committed merge conflict in `argent-create-flow` (prettier
had rewritten the markers into a list bullet and a blockquote, so neither
git nor the formatter saw it, and the two surviving copies gave opposite
instructions about carrying a recorded Enter across); the TV rejection
reason, which stated a transport limit that does not hold for Android TV;
`runPinch` described as a single dispatch when it chains several; and the
per-platform assert table moved to `references/`, which is where the
skill guide puts reference material an agent consults rather than reads
every time.
Three invariants a mutation sweep found unpinned now have a test each,
each verified by re-applying the mutation: the `<textarea>` own-text
guard (which shipped with no test), the per-round target-frame
re-resolve, and the overlay-vs-wrapper distinction.
|
||
|
Hubert Gancarczyk
|
ab7f3412b4 |
docs: correct the type directive's docs where the code stopped matching them
`(cleared)` in the recording summary is past tense for something that has
not happened: no step the recorder produces is a `type` — `flow-add-step`
emits only tap/launch/run/tool and the only `kind: "type"` construction in
`src/` is the YAML parser — so every `type` in a summary arrived by a hand
edit and did not run live. It reads `(clear only)` now, matching the run
report's present-tense `(clear first)`.
`type needs { into, text }` names one of two valid shapes since `clear`
made `text` optional, and it is the message an author sees after
extrapolating the bare-scalar form the other directives accept.
"`submit` serializes only the opt-out" stopped being true two lines below
itself, where a clear-only step serializes `submit: true`.
`dispatchOrAbort`'s docblock described a mechanism that does not exist —
cancelling a run does not tear down the transport under an in-flight call
— which made its real trigger look far broader than it is. It now says
what each of its two callers actually needs it for.
The per-platform caret split omitted Chromium in three places, which
matters because "appended" is the half that leads to leaving `clear` out;
re-measured, Chromium behaves like Android. The Android assert bullet
gained the password exception its Chromium sibling already carried — both
adapters redact identically, so `equals` on the typed value is false
either way. And `flow-ios-tree`'s "leaves the poll unconfirmed" named the
value that now refuses a clear, where the case it describes is the one
that must NOT.
|
||
|
Hubert Gancarczyk
|
aafd9899bc |
fix(flow): require focus INSIDE the target before a clear, not merely overlapping it
The overlap test confirmed a clear by construction whenever a
focus-flagged node was large enough to cover the target, and every shape
that produces one can hide a different element holding the keys.
Reproduced on a live Chromium page through this branch's tool-server, an
ordinary `focusin` focus trap bouncing focus back to a `<textarea>` whose
box contains the target input:
before: #notes = "DRAFT do not erase me" #email = "old.remembered.login"
- type: { into: { id: email }, text: "new@example.com", clear: true }
-> pass, "into id=email (clear first)"
after : #notes = "new@example.com" #email = "old.remembered.login"
The user's draft is cleared and overwritten, the named target is never
touched, and the step reports a pass. The same shape occurs with an open
shadow root (`document.activeElement` is the host, never the inner
element) and with a hybrid app's focused WebView.
Confirmation now requires a focus-flagged node INSIDE the target's frame,
which keeps the case the overlap test was written for — a testID wrapper
whose input reports the focus. A node that merely encloses the target is
a new outcome of its own, refused with the reason that fits it. The same
run against the same page now fails with "the only element reporting
focus CONTAINS id=email rather than sitting inside it" and leaves both
fields alone; an ordinary field replace still passes.
A window in which every tree read threw is also no longer treated as
"nothing was focused". Nothing was OBSERVED, which is not evidence — the
convention `settleTree` already applies to the same condition — so it
refuses rather than clearing on it.
Also uniform now: all three of the step's device calls classify a
cancelled run the same way. The focus tap was bare while the two keyboard
dispatches were wrapped, so one step reported `error` or `skip` depending
on which dispatch was in flight when the caller gave up.
Four guards this stack added were removable with the whole flow suite
still green — the pre-Enter abort re-check, the clear/text dispatch's
return, the Enter dispatch's wrapper, and now the tap's. Each has a test
that fails when it is taken out.
|
||
|
Hubert Gancarczyk
|
b7d68d767f |
fix(flow): judge focus by the latest read, not by any read that ever saw it
The focus outcome accumulated "saw focus on something" across every poll round, which made the verdict a race. An app that blurs when the focusing tap lands reports the PREVIOUS field as focused for however many rounds precede the blur, so the same flow against the same app refused the clear or passed it depending on whether round 1 beat the blur — one being a hard step failure and the other a silent no-op. The question the clear is about to act on is whether something else holds focus NOW, so the outcome now reflects the most recent successful read. An app that blurs lands in the documented residual deterministically, and a target that never takes focus while another field keeps it is still refused. A window in which every read throws still reports "unobservable" rather than claiming focus was seen, and the type's doc no longer says that bucket means only "the tree cannot report focus" — it also covers "the tree could not be read". Also records, where the decision is made, that the overlap test confirms by construction against a focus-flagged node large enough to cover the target (an Android host WebView, an iOS first-responder container, a Chromium focus-trap div). On Chromium that is benign — one activeElement per document, so a focused container means no input holds focus and the clear no-ops. The destructive shape needs two focused nodes at once, which only the Android/iOS adapters can emit; it has not been observed on a real device, so it is documented rather than guessed at. |
||
|
Hubert Gancarczyk
|
2b2ef6ea9b |
fix(flow): pin the abort/focus arms review found unpinned, and cut the tap wrap
Second review pass, all four items proven by mutation rather than reading. Two arms of the new code were asserted by nothing. `dispatchOrAbort` returning false on ANY error — turning every backend rejection into a silent "run aborted" skip — passed all 3131 tests; `runRotate` already carried that positive control, `runType` did not, so it has one now. Likewise the focus-wait's source gate: mapping a non-focus-reporting source to "unconfirmed" instead of "unobservable" passed everything, while refusing `clear` on every target behind such a source — the same platform-wide outage the outcome split exists to prevent. Both mutations now fail. The `gesture-tap` wrap is reverted. It was not needed by any of the fixes, nothing asserted it, and it singled the focus tap out for a guard its five siblings (runTap, runLongPress, scrollIncrement, runPinch) do not have. `runRotate` now shares `dispatchOrAbort` rather than keeping the inline copy the helper was written from, and the helper's doc no longer claims `runLaunch` as a precedent: `runLaunch` returns a failure outcome instead of rethrowing, so it deliberately is not a caller. The clear-only cancellation test keeps its case but drops the rationale about reaching the dispatch "by a different branch" — after the collapse both shapes take the same one. Per-platform assert guidance corrected again: on Chromium an assert on the typed value hard-FAILS like iOS (it reads the accessible name); it is the label-matching assert that passes vacuously. The name also falls back through aria-labelledby, skips password fields, and covers `<textarea>`, and Android only pairs hint with value when a contentDescription is present — without one, most React Native `TextInput`s report the contents alone and `equals` is correct. |
||
|
Hubert Gancarczyk
|
bf28bdef56 |
fix(flow): make clear atomic with the text, and never clear blind
Review of #581 surfaced two ways the `type` directive's `clear` could destroy data it was meant to replace. Both reproduced on a Pixel 3a against Bluesky, and both re-verified fixed on the same device. Clear and text now ride ONE `keyboard` call. Every backend validates the whole request before touching the device precisely so a rejected call leaves no trace — `assertTypeableAndroidText`, and the per-character resolves in the chromium and simulator-server backends, all run ahead of the clear. Issuing them separately stepped outside that guarantee: `{ into: field, text: "José", clear: true }` cleared, then rejected the text, leaving the field EMPTY where the identical args as a single call left it holding its original value. Enter stays a separate call — the `typeTv` argument for splitting applies to `key`, which it rejects outright, and not to `clear`, which it rejects just as early. `waitForFocus` now reports its outcome instead of discarding it, and a clear whose focus landed elsewhere fails the step rather than emptying whatever else holds it — previously reported as a pass, labelled as a clear, on a field it never touched. The outcome keys off whether any node in the tree reported focus, not off the source: an iOS build whose injected framework omits `firstResponder` reports none at all, and conflating that with "focus is elsewhere" refused every clear on the platform. `runType` also reclassifies a dispatch that rejects mid-cancellation as an aborted skip, matching `runRotate` and `runLaunch`; a cancelled run no longer surfaces a raw backend message as a step failure. Docs: `type` does not "append" — the focusing tap lands the caret mid field, so on Android the new text is spliced INTO the old value (forty `A`s typed with `ZZZ` gives `AAA…ZZZ…AAA`), while on iOS it appends. The suggested `assert` on a field's text does not work everywhere either: iOS never exposes a field's contents to the flow tree, and a Chromium `<input>` only does so without an aria-label or placeholder. Both stated per-platform now, with the Vega rejection attributed to the directive gate that actually performs it, and the recording-cleanup rule carrying a recorded Enter across as `submit: true`. The run report says `(clear first)` rather than `(cleared first)`: the label is stamped before the step runs and rides every outcome, so the past tense claimed a clear on steps that failed, errored or were skipped. |
||
|
Hubert Gancarczyk
|
00774162d8 |
fix(flow): drop the post-clear read-back check; it was blind or wrong
The `type` directive verified a `clear` by re-reading the focused node and
failing the step when it still held text. Review plus device probes showed the
check is blind exactly where it matters and wrong where it fires, so it is
removed rather than patched.
Blind:
- iOS never carries a value. `flow-ios-tree` projects leaves as
{role, frame, children, label, identifier, focused} and does not even request
a text field from `getFullHierarchy` — the check was dead on the platform.
- A Chromium `<input>` has no value either: the DOM walker fills `value` from
the element's own child text nodes, and an input has none.
- On Android a field with no contentDescription puts its contents in `label`
(`labelOf` falls back to `text`), so no value again — i.e. most React Native
TextInputs.
Wrong where it fires:
- An emptied Android field WITH a contentDescription reports its hint as the
value, so a successful clear reads as leftover text and fails the step.
`<TextInput accessibilityLabel="Email" placeholder="you@example.com"/>` is
the ordinary shape that hits this.
- A Chromium `<textarea>` exposes its default content once `el.value` empties,
producing the same false failure only after the clear succeeds.
Demonstrating both halves: on an API 30 emulator whose clear genuinely failed
("wifi" -> "wif"), the check returned undefined and passed the step — the exact
append it existed to prevent — while the shapes above would have failed steps
that worked.
The silent-no-op risk lives at the tool layer, which now rejects `clear`
outright on every backend that cannot perform it. Flows wanting proof should
`assert` the field's value, which works on every platform. The skill docs said
the step "fails immediately rather than appending"; that claim is removed and
replaced with the assert guidance, since leaving it would stop an agent adding
the check that actually works.
Removing the read-back also removes its abort-guard gap (a run cancelled
between the clear and the read was reported as a step failure rather than a
skip) and its unsettled single-read race.
Tests: dropped the verification cases (one of which was vacuous — the android
adapter strips `value` from password nodes, so the password guard was
unreachable), pinned that a clear adds no extra tree read, and added coverage
for the run-report label and the recording summary, including a clear-only step
that previously rendered as `<- "undefined"` when the summary regressed.
Docs: corrected the platform scope on the argent-device-interact capability row
(Chromium was missing), softened "one device round trip" to "a single tool
call", noted that a clear-only `type` sends no Enter, dropped the API-30
framing now that the Android guard probes the device rather than a version, and
updated the argent-test-ui-flow login example, which still demonstrated the
bare-append pattern the other skills now warn against. `flow-execute`'s
description also documents `type`'s options for the first time.
|
||
|
Hubert Gancarczyk
|
37515fc610 |
feat(flow): clear on the type directive
A flow that replays against a non-empty field types on top of the old value.
It does not fail at that step — it fails later at an `assert`, pointing at the
wrong place. `type` can now empty the field first:
- type:
into: { text: Email }
text: "new@example.com"
clear: true
Threaded through the step union, the YAML shape, parse, serialize, the runner,
the run-report label and the recording summary.
Two semantics decisions:
- `text` becomes optional when `clear` is set. `type: { into: search, clear:
true }` — empty the box, then assert the empty state — is a legitimate step.
A missing/empty `text` without `clear` still rejects as before.
- `submit` defaults to false when there is no text. It defaults to true
otherwise, and is stored (and serialized) only when it differs from that
default, so YAML round-trips unchanged. Firing Enter into a field the step
just emptied would run an empty search or submit a blank form.
`clear` joins the `rejectUnknownKeys` allowlist, so a misspelled `claer` fails
loudly instead of silently reverting the step to an append.
The clear is dispatched as its own `keyboard` call, before the text. It is NOT
folded into a combined `{ clear, text, key }` call even though the tool now
supports that ordering: `typeTv` rejects `key` outright and does so before it
types, so a combined call on an Apple TV / Android TV target would throw with
the field still empty.
`runType` already holds a settled tree, so it verifies the clear landed and
fails the step when the field is still populated — a flow that silently fails
to clear is far worse than one that fails loudly in the right place. The check
reads the focused node's `value` only: a field's placeholder surfaces as its
LABEL (Android exposes the hint that way), and an emptied field reports no
`value` at all, so treating label text as content failed every successful
clear — observed on a real device before the check was narrowed. Password
fields are skipped outright, since uiautomator reports their text as empty and
"looks empty" there is a false pass rather than evidence.
Skill docs: the `type` row and `submit` paragraph in argent-create-flow, the
keyboard section and capability row in argent-device-interact (including the
"use triple-tap to select all" line, which steered agents at the workaround
this replaces), and the Vega/TV rejection in argent-tv-interact.
The load-bearing one is argent-create-flow's recording-cleanup rule: it now
carries `clear` from a recorded `tool: keyboard` step into the `type:`
directive. Without that, the polish step drops it and the cleaned flow reverts
to appending — a docs omission that produces a data bug.
|
||
|
Hubert Gancarczyk
|
b781dc0e91 |
fix(tool-server): catch a Chromium page that refuses the typed characters
`delivered` counts ARRIVALS, from a capture listener that sees each `beforeinput`
before the element's own handler can cancel it. That is what makes it the right
evidence for "did focus move", and it is why it cannot say whether anything took
EFFECT: a field that refuses every insertion in place reads as fully delivered
while holding nothing. Both halves of the split rule then agree the wrong way —
the value is wrong, the arrivals are complete — so neither guard fires.
Reproduced against a real Chromium renderer at this head, `<input
value="old-value-seeded">` whose `beforeinput` handler `preventDefault()`s every
`insert*`, focus retained: `{ clear: true, text: "abc" }` returned 200
`{ typed: "abc", keys: 3, cleared: true }` over an EMPTY field.
A second capture listener counts the `input` events, which Blink dispatches only
for an insertion it applied, and the new guard fires on the corner where the two
disagree completely: every character arrived, none took effect, the field holds
nothing. A page refusing SOME insertions is normalising, and a page that cancels
the event and writes the value itself leaves the field non-empty — both keep
passing, verified against the same renderer.
Reported under its own code: focus never moved, so nothing was split across
fields, and the caller needs to know the field is empty rather than go looking in
a neighbour.
Three further fixes in the same two files:
- The abort signal reached the head of the per-device chain and stopped there.
`runChromium` took no signal and its dispatch loop slept plainly, so a client
that hung up mid-call left Chromium typing out every remaining character and
holding the chain for the whole run — the same abandonment window that made the
chain-head check necessary. It is now checked between characters with the
cadence wait yielding to it, as the iOS backend already does, and kept out of a
character's own three events and out of the clear's select-all/delete pair,
which must stay atomic. Verified over HTTP: hanging up 2s into a 20-character
run at a 300ms cadence stopped it at 6 characters and left the rest untyped,
while the identical call held open typed all 20.
- `nodes` is dropped from the focused-element probe. It was computed, declared on
`FocusedEditable` and shipped, but nothing on the server read it back — the
after-probe reports the surviving stamps as `residue`. `stampEmbeds` stays, for
the side effect that count was a by-product of, and the test that used the field
to prove a form control is never asked for its embeds now counts the calls
instead: the probe swallows a throwing `querySelectorAll`, so the throw alone
was never the pin.
- `delayMs`'s ceiling is pinned AT the boundary. `5000`-accepted plus
`600000`-rejected left the number free — raising `.max(5000)` to `.max(10000)`
kept the whole keyboard suite green — so a `5001` probe now holds it.
|
||
|
Hubert Gancarczyk
|
c2f3d0fd24 |
fix(tool-server): fit clear to the text/key exclusivity below it
The base now rejects a keyboard call carrying both `text` and `key`, so
the legal shapes are `{}`, `{text}`, `{key}`, `{clear}`, `{clear,text}`
and `{clear,key}`. `clear` is unaffected — it is the one parameter that
still combines — but a lot of this branch was written against the
three-part `{ clear, text, key }` call and no longer describes what runs.
Behaviour:
- Vega's clear refusal ended every message with "send the same call
without `clear`". For `{ clear: true }` alone — the empty-the-field
call the parameter exists for — and for `{ clear: true, text: "" }`,
which the injection guard no-ops, re-sending does nothing at all, so
the field is left neither emptied nor typed into. It now discriminates
the three shapes the way the TV twin does, and says the key press is
what to re-send on `{ clear, key }`, which Vega does accept.
- The TV refusal had the same hole one arm over: `{ clear: true, text: "" }`
named `text`, so a `!== undefined` check sent the caller to
`{ text: "" }`, which types nothing. It now falls in with a clear-only
request, which is the shape it actually is.
Budget: the worst case was `{ clear, text, key }` at 26s + 15s + 15s.
It is now `{ clear, text }` at 26s + 15s. Still past the MCP adapter's
30s per-request fetch timeout, so `longRunning` stays — with the
arithmetic corrected in all four places that carried the old sum.
Comments: three backends justified their resolve-before-inject hoist as
"so an unknown key name fails after the text has already been typed".
A key can no longer follow text, so the surviving reason is `clear`,
which empties the field before either. Android's tie-break for a request
"bad in BOTH halves" is gone with it — that needs `text` and `key`
together. Chromium's key block said it ran after the text; it runs after
the clear.
Tests: the three "orders clear → text → key" backend tests become
"orders clear → key", the other combination a caller can still send and
the one nothing else pins — pressing Enter before the delete run would
submit the field's OLD contents. The chromium read-back test moves from
"reads the field back BEFORE the named key" to what the gate actually
does now: a key-only call never reaches the split check, so a submit
handler that empties and blurs the field cannot be reported as a split.
`interaction-messages` keeps the three-part shape on `startedMsg` only,
which is what an event log really shows for a request about to 400.
The TV ordering test's justification was wrong on its own terms:
`blueprints/android-tv-control.ts` is `typeTv`'s callee, not a second
caller, and its API takes a bare string. `typeTv` has exactly two call
sites, both below the guard, so no caller can produce the shape — the
test now says so and stands as an internal-order pin.
|
||
|
Hubert Gancarczyk
|
b86862876c |
fix(tool-server): declare flow-add-step long-running, so recording a slow step is not retried
`flow-add-step` dispatches an arbitrary tool by name, so its worst case
is the worst case of whatever it wraps — and several of those are
`longRunning` themselves. This branch raises `keyboard`'s worst case to
~56s (clear 26s + text 15s + key 15s) and declares the flag for exactly
that reason, and the recorder that wraps it is how a clear gets into a
flow at all: the skill copy instructs authors to record the step raw
rather than fold it into `type:`.
Without the flag the MCP adapter applies its 30s FETCH_TIMEOUT_MS to the
RECORDING of such a step and, on abort, re-POSTs the identical body up to
MAX_RETRIES more times. None of that work is cancellable, so each retry
is a fresh device action AND a fresh appended step.
Reproduced through the REAL MCP adapter against a real tool-server,
recording one `await-ui-element` with `timeoutMs: 40000` (a deterministic
>30s step on this hardware, the same mechanism a >30s `keyboard` takes):
before: 153s, "This operation was aborted" — so the agent concludes
nothing was recorded — and FIVE identical steps in the YAML,
which the replay then performs five times.
after: 40s, no error, ONE step.
The flag itself pre-dates this branch; what is new is that a >30s
recorded step is reachable through it.
|
||
|
Hubert Gancarczyk
|
6b8ab07c68 |
fix(tool-server): chain a keyboard run per device on android and chromium too
The iOS backend queues its runs per device because a clear holds state
across awaits and a concurrent call lands inside it. The other two
backends hold state across awaits in exactly the same way and got no
equivalent: android's modern clear sends the select-all
(`input keycombination`) and the delete (`input keyevent`) as two
separate adb invocations, leaving the field fully SELECTED in between,
with the text a third; chromium spreads the clear, the settle, the
read-back and one dispatch per character over many CDP round trips.
Where iOS loses the intruding keystroke to the modifier, these two type
it OVER a full selection, which destroys the value — and both calls
still return 200 with their own text as `typed`. It needs one caller
overlapping two calls, not two agents.
Measured on this branch before the fix, `{clear, text:"AAAA"}` against
`{clear, text:"BBBB"}`, where the only clean outcomes are AAAA or BBBB:
Android API 36, 10/20/30/50ms — 4 of 4 corrupt:
ABABABAB AABABBB AAABABBB AAAABBBB
Chromium, 0/30/80ms — corrupt at every offset, ABABABAB at 0ms.
And the narrower window that destroys the value the caller was replacing
— `{clear:true}` against `{text:"hello"}` 20ms later, over a field
holding `user@example.com` — left `hell`: the text replaced the whole
selection and the clear's delete then ate a character.
`serializePerDevice` moves out of `simulator-server-keys.ts` into
`device-chain.ts` unchanged, with the case-folding rationale that was
inline at its iOS call site lifted alongside it as `deviceChainKey`, and
both other backends now enter the same chain. The abort check comes with
it: without it a queue of hung-up calls still runs every one of them out
at a device nobody is reading, and `adb shell input` is not cancellable,
so chain entry is the only point a hang-up can be honoured. The Android
TV path stays outside the queue — it cannot `clear`, so it holds no
selection across awaits.
Re-run after the fix, every case a clean serial outcome:
Android API 36, 10/20/30/50ms — 4/4 BBBB
Android API 30 (legacy path), 50ms — BBBB, and a serial
{clear, text} still lands `legacy-ok`
Chromium, 0/30/80ms — 3/3 BBBB
{clear} vs {text:"hello"} on API 36, 20ms, 6 runs — `hello` or empty,
both legitimate orders, no `hell`
iOS, three overlapping calls ({clear,AAA}, {BBB}, {CCC}) — the field
holds AAABBBCCC, so the chain still holds after the move
Apple TV — `clear` still refused before anything reaches the device
|
||
|
Hubert Gancarczyk
|
94abfbc0fe |
fix(tool-server): hold a clear's text to the characters before an Enter, not to none of them
The split check excluded any `text` carrying `\n`, `\r` or `\t`, because
those dispatch as physical Enter/Tab and can move focus by definition —
the same reason a named `key` is excluded. That is right for what follows
such a character, but `descs.some` tested the WHOLE string and the check
was then skipped for EVERY character, so one newline in a `<textarea>`,
where a newline is ordinary content that moves nothing, switched the
guarantee off for a value of any length.
Measured on Chrome 151 against an exact control pair differing only by
that one character, in a textarea whose 4th `input` moves focus to a
neighbour:
{ clear, text: "aaaabbbb" } -> 500, "only 3 of the 8 character(s)"
{ clear, text: "aaaa\nbbbb"} -> 200 { cleared: true }
Both left the same ["aaa", "abbbb"] behind — the second reported a split
as a clean replacement.
The exclusion now ends the guarantee at the first Enter/Tab instead of
dropping it: the characters before it are still held to, the ones after
it are not, since no evidence here separates "the page split the value"
from "the key I was asked to send moved focus". The failure counts
against that prefix and says so, rather than quoting the request's own
length for characters it never had an opinion about.
The whole-call exclusion survives where there is nothing to replace it
with: a delivery count that cannot be read leaves the focus sample as the
only signal, and a blurred field after an Enter is exactly what the
requested key does — so that combination stays excluded.
E2E on the same harness after the fix: the newline split is now reported,
the no-newline control still is, and all four benign shapes still pass —
a search box that submits/empties/blurs on `{clear, text: "query\n"}`
(the false positive the exclusion was added for), an Enter-to-send
textarea composer, a plain two-line textarea, and `text: "ab\tcd"` across
two inputs.
|
||
|
Hubert Gancarczyk
|
db2e95ec3d |
fix(tool-server): stop the designMode clear parking <html> and reporting a focus loss
The editing-host climb is meant to stop at the element that declares
`contenteditable` itself, and — with nothing declaring it, which is what
`document.designMode` produces — at that document's body. Its own comment
said so. It did not: under designMode `documentElement.isContentEditable`
is `true` as well, so the `parent.isContentEditable !== true` test let the
walk take one more step and park <html>.
`document.activeElement` never reports <html> (it falls back to <body>),
so the post-clear check compared the parked node against a value it could
never equal and concluded focus was lost every time, whatever the page
did. The destructive half has already run by then: the document is
emptied and the caller gets a 500 saying "the page moved focus in
response to the clear", which never happened, and is told to tap the
field again.
Reproduced on Chrome 151 in an Electron harness:
`document.designMode = "on"` over `<h3>h</h3><div>alpha beta</div>`, then
`{ clear: true, text: "replacement" }` → the body text went to "" and the
call raised KEYBOARD_CLEAR_FOCUS_LOST naming HTML, with
`document.activeElement` BODY both before and after. A bare
`{ clear: true }` was unaffected, so only the combined form — the form
the parameter exists for — hit it. After the fix the same call returns
`{ typed: "replacement", keys: 11, cleared: true }` with "replacement"
actually in the document.
Guards re-run on the same harness: `<body contenteditable="true">` with
designMode off still clears and types, and focus on an inherited-editable
`<span>` inside a composer still parks the declaring DIV rather than the
span.
The probe test that covered this asserted the host is the BODY while
giving its fake body no `parentElement`, so the walk stopped for a reason
the real DOM does not supply and the <html> hop could not be expressed.
It now models the <html> above it, reporting `isContentEditable: true` as
a browser does.
|
||
|
Hubert Gancarczyk
|
2f8d4cae3a |
fix(tool-server): stop counting an embed the chord can never delete as residue
The Chromium clear verifies two things: no text left, and none of the embedded elements it stamped before the clear still inside the field. Commit 2c813f77 established that Blink neither selects nor deletes non-rendered content, so counting it makes a clear that WORKED report failure — but the rule was applied only to the text measurement (`isRendered`), not to the element one, which matches purely structurally through `querySelectorAll`. So a `display: none` embed inside the editing host is stamped, survives for exactly the reason `isRendered` exists, is counted afterwards, and raises KEYBOARD_CLEAR_INEFFECTIVE — permanently, since every retry re-stamps the same node, with a message that blames the page for cancelling the key and names "an image, a video or similar". Reproduced on Chrome 151 against a matched control, both shapes: `hello` plus a `display: none` <img> in a contenteditable failed while the same page without the style cleared and typed; and `body[contenteditable]` holding one `<input type=hidden>` failed while the same body without it passed. That second shape is what makes this reachable rather than exotic — `input[type=hidden]` is `display: none` by the UA stylesheet and is in EMBED_TAGS, so on a `body[contenteditable]` or designMode page one CSRF token or hidden analytics frame was enough to make every clear a hard failure. `isRendered` moves out of `EDITABLE_TEXT_FN` into its own page-side constant so both probes can declare it exactly once, and the ancestors are walked up to the queried editable: the element count is a FLAT query while the text walk prunes from the top, and `display: none` does not compute onto a child, so asking the embed alone would miss every nested one. Verified against the same Electron harness after the fix: both shapes return 200, and the guards still fail — a visible <img> whose delete the page cancels is still reported, text residue still is, and an ordinary img-only editor still clears. |
||
|
Hubert Gancarczyk
|
d254a50069 |
test(tool-server): pin the parser fix's effect on a saved flow, and stop a test
claiming a guarantee it cannot give `flows/flow-android-tree.ts` shares `parseUiAutomatorXml`, so reading a single-quoted attribute correctly changes the verdict of a saved `assert` / `await` step that was written against the field reading blank. That direction — the one that alters an existing flow's outcome — had no test. And the `commands` payload test claimed to pin the `if (event.commands !== undefined)` guard. It cannot: the payload is sent through `JSON.stringify`, which drops an `undefined`-valued key, so `payload.commands = undefined` and "no key at all" are byte-identical to the fake CDP server — as they are for every sibling guard in that builder. Renamed and re-commented to say what it does pin (the wire shape) and why the guard itself is not observable from there. |
||
|
Hubert Gancarczyk
|
9a1f6f8ecb |
test(tool-server): pin the clear's budget algebra, and let the probe harness
express the selector it rules out The three branches that split the Android clear's 26s all survived mutation: the `Math.min` clamp on the helper read, the `> 0` guard that decides whether to issue it at all, and the retry loop's attempt-0 arm. Each now has a test that fails on the corresponding mutation — verified by applying all three: dropping the clamp fails 3, removing the guard fails 1, charging the backoff on the first attempt fails 28. The guard's test asserts the RPC is not even ISSUED once its share is gone, which is the part that matters beyond timing: that read serialises on the same chain `describe` uses, so one nobody can wait for still holds it. And the probe harness's attribute-selector matcher required an `=`, so it could not express a bare `[contenteditable]` at all — the "a nested editor is not an atomic embed" assertion was comparing 0 against 0 rather than against the 2 a browser returns. The matcher now accepts the bare form and the test shows the two spellings differing. |
||
|
Hubert Gancarczyk
|
8b2eadc16c |
fix(tool-server): stop the interrupted clear asserting a state it cannot know
`KEYBOARD_CLEAR_INTERRUPTED` said the focused field "is PARTLY emptied — N backspaces were sent", but the catch it comes from covers more than the timeout the one existing test exercises: a cause that stopped the run before anything went out (the device went offline, adb lost authorisation) leaves the field untouched and nothing sent. The remedy is the same either way — read the field, do not assume — so the message keeps its warning and drops the assertion: "may be PARTLY emptied", "up to N backspaces were sent". Pinned for both causes now, with the non-timeout `error_kind` carried through. Verified on the API 30 emulator that the ordinary legacy clear still replaces a field exactly (screencap, since the devtools helper makes a concurrent dump serve stale text). |
||
|
Hubert Gancarczyk
|
0b3a407cc3 |
test(tool-server): pin longRunning's serialization, the split's code, and STARTING
Three gaps the review's mutation sweep and reading found, none of which any existing assertion covered: - `http.ts`'s `if (def.longRunning) entry.longRunning = true` was untested. That one line is what the whole no-timeout mechanism rides on — including, positively, stopping the MCP adapter re-sending a timed-out, non-idempotent `keyboard` POST up to five times. - the during-typing split's error code survived being swapped for INEFFECTIVE. The two have to stay distinguishable on the wire: one means "re-clear required", the other "the clear worked, the characters went elsewhere", and `failure_stage` is not serialized. - the helper gate was only ever driven at RUNNING and IDLE, though `isLiveServiceState` also admits STARTING — a helper mid-start holds the UiAutomation connection just as a running one does. |
||
|
Hubert Gancarczyk
|
2a6fa4746b |
test(tool-server): pin the describe verdict that FLIPPED, and cap the dump it quotes
The capture condition went from `/^ERROR:/i.test(trimmed) ||
(!trimmed.includes("<hierarchy") && /error/i.test(trimmed))` to plain
`!trimmed.includes("<hierarchy")`, which moves two real device outputs in
opposite directions. The `Killed` / empty half is pinned; the other half was
not — an `ERROR:` line that `adb exec-out` folds in AHEAD of a usable hierarchy
(a `waitForIdle` timeout, which does not stop the dump) used to throw and now
parses. The existing noise fixture uses `WARNING:`, which the old condition also
let through, so it could not catch that.
Also pinned: the serial the dump is asked for. Mutation showed the whole suite
staying green while this function dumped an entirely DIFFERENT device — it would
answer every describe with whatever screen that device happened to show.
And the failing dump's own bytes were interpolated uncapped into an agent-facing
message. The chromium clear caps its element label and the TV blueprint caps this
same dump output; this one now caps at the same 200.
Verified live: `describe` still works on both the API 30 and API 36 emulators.
|
||
|
Hubert Gancarczyk
|
83de723710 |
docs: correct the two clear figures that no longer match the code
The Android clear budget: ANDROID_CLEAR_BUDGET_MS now derives as ADB_INPUT_TIMEOUT_MS + DELETE_RUN_RESERVE_MS = 26s, and four places still said 20s — the tool's own note plus the three test comments a reader would check it against, which is what made the stale figure look corroborated. And the blind delete run: it sends BLIND_DELETE_COUNT + DELETE_MARGIN = 158 keys, so a 151-158 character field is fully cleared. Both the tool description and the skill said a field "past that same 150 limit keeps its head", which understates what the fallback covers. |