Commit Graph

849 Commits

Author SHA1 Message Date
Hubert Gancarczyk 6e862a60b3 test(keyboard,flow): pin the arms mutation showed unpinned, and the new warning route
Each of these was written against a mutant that kept the suite green, and each
was re-run with that mutant applied to confirm it now fails.

keyboard-clear:
- the atomic cooldown EXPIRING, at the boundary. Arming and not-arming were
  covered; nothing advanced the clock, so widening the constant to a permanent
  skip stayed green.
- `WEDGED_RPC_MS` driven through `ping`. Its rationale names both legs and only
  the write drove it — and `ping` is the leg a helper that goes silent after
  accepting the socket actually spends its time in.
- a helper reporting a protocol NEWER than the gate still takes the atomic path
  (`>=`, not `===`).
- the `sized` tie-break: an unmeasurable focused field tying the longest reading
  keeps the note's BLIND clause, in the walk order where only that arm decides it.
- the read-back arm outranking `no_focused_input`, and the read-back clause
  staying off the paths that never took one.
- a `{{secret:…}}` request on the no-writable-field arm is not told to read the
  field back.
- the cross-platform result SHAPE, which a destructure had stopped checking.

flow runner:
- a `keyboard` clear's warning carried out of a composed `flow-execute`, its
  repeats collapsed, its depth composed, and a green sub-run left warning-free.
- a tool answering `note: ""` raises no warning.

Two gaps from the review are left open, both reported rather than closed: the
blueprint's `setText` wire call (there is no android-devtools blueprint harness
to hang it on, and the call is exercised end to end against a real protocol-2
helper), and `download-native-binaries.sh`'s gate (no shell-test harness in the
repo; both arms were driven by hand against the live release).
2026-08-24 20:50:02 +02:00
Hubert Gancarczyk 0fa956a965 docs(keyboard,flow): scope the "no note" promise, and correct three claims about the code
- The `keyboard` tool description ended "so no note means the clear was the
  verified one" in a sentence describing every platform. Only Android can set
  one: on iOS the same absence means a fire-and-forget chord, on Chromium "seen
  empty, or not observable". The sibling JSDoc in `types.ts` was already scoped
  this way; the agent-facing copy was not, nor was the same ending in
  `argent-create-flow/references/flow-yaml.md` — which also now names the fourth
  route a clear's warning can reach a step report by.

- `KeyboardResult.cleared` said the injected clear-only path "finishes with a
  delete run if the chord left a residue". It finishes with one when the reading
  can be told apart from the field's own placeholder, which is what the run is
  now gated on.

- `ATOMIC_CLEAR_BUDGET_MS` said a helper that "accepts the socket and then goes
  silent" costs up to 20s. That state costs 5s: `ping` is awaited first and
  rejects there, so `setText` is never sent. 20s needs a slow `ping` that
  ANSWERS and a write that then goes silent.

- `AndroidClearOptions.atomicWriteApplied` said the caller sets it for the two
  `setText` outcomes. The caller sets it from the helper's own `applied` flag and
  deliberately not from the reason's name — the same decision, for the same
  reason, that `androidClearNote`'s `DOUBLED` documents at length.

Verified the tool-description quality gate locally after the description edit:
average 9.01 / 10 against a threshold of 9.0.
2026-08-24 20:35:11 +02:00
Hubert Gancarczyk 60c8edecbd fix(flow): surface a weak clear a composed flow buried in its sub-run
`execLeafStep` promoted a weak-pass note for `keyboard` and `run-sequence`
steps. `flow-execute` is the third registered nested orchestrator and was not
read: it keeps its sub-run's reports inside `result`, `flowExecuteOutcome`
answers `undefined` on a green sub-run, and the CLI's `StepReport` has no
`result` field — so a composed flow whose clear took an unverified path
reported a clean green with the warning where nothing renders or counts it.
`flow-add-step` records a raw `tool: flow-execute` step on four branches, so
this is a spelling flows really carry.

Reproduced on an API 36 emulator with the helper too old for the atomic path,
against two real flows (an inner `tool: keyboard` clear, an outer
`tool: flow-execute`):

  before  OUTER step 0 pass flow-execute  warning=NONE
  after   OUTER step 0 pass flow-execute  warning=keyboard clear: the atomic …

Nested `warning`s are taken whole rather than filtered to `keyboard` notes:
unlike a `note`, that field already means one thing — this step passed in a way
that weakens it as proof — and the sub-run's own `execLeafStep` has applied the
tool rule at its level, so composition to any depth needs no second rule.
Repeats are dropped.

The enumeration now lives in one function, since three docblocks name it: both
`StepReport.warning`s and the `flow-execute` tool description said the note
reaches a report three ways, and this is the fourth.
2026-08-24 20:32:53 +02:00
Hubert Gancarczyk cdfa9fba98 fix(android-devtools): check the downloaded helper before it becomes the bundled one
The versionCode check ran after `mv -f "${TMP_APK}" "${ANDROID_TARGET}"` and
after the EXIT trap was cleared, so a release that fails it aborted having
installed the APK it had just diagnosed — at exactly the path
`bundledHelperApkPath()` reads, with the temp copy kept as well.

Reproduced against the live `argent-main` release (versionCode 1, manifest
requires 2) with a sentinel standing in for a previously downloaded helper: the
sentinel was replaced by the rejected APK before the error printed.

The check now reads the temp file and the move happens after it. Same run
against the same release: exit 1, the previous APK still in place, no temp left
behind. A matching versionCode still installs it (verified by lowering the
manifest to 1 and re-running).
2026-08-24 20:24:56 +02:00
Hubert Gancarczyk edcb502a2c fix(android-devtools): refuse to stamp a versionCode the pinned helper source has not earned
`build:android-binaries` compiles the helper from the `packages/argent-private`
submodule and stamps `--version-code` from this repo's manifest. Nothing tied
those two together, so bumping the manifest to 2 for the `setText` protocol —
without moving the submodule pointer, which is still v0.19.0 and
`PROTOCOL_VERSION = "1"` — mints an APK that claims versionCode 2 and cannot
answer the method.

That fails silently and permanently. `ensureAndroidDevtoolsInstalled` compares
`probe.versionCode >= manifest.versionCode`, `2 >= 2` is satisfied, and the real
v2 helper is never pushed. Reproduced on an API 36 emulator: with such an APK
installed and a GENUINE protocol-2 APK bundled, the install is skipped, the
device still reports versionCode 2, and every clear comes back
`helper_outdated`. `adb uninstall` is the only recovery.

The manifest now states the protocol it promises, and `build.sh` compares it
against the source's own `PROTOCOL_VERSION` before compiling anything. Kept as
its own field rather than read off the versionCode: the versionCode is the
install bar and a rebuild for a plain bug fix moves it alone.

Verified both ways on the pinned tree: the build now stops with the pointer's
protocol named and `bin/` untouched, and the same extraction against the
protocol-2 helper source answers 2, which is what the manifest declares.
2026-08-24 20:22:51 +02:00
Hubert Gancarczyk e5e34899f2 fix(keyboard): do not backspace over a reading that could be a placeholder
A clear-only call reads the field back after the DEL and sends a delete run
over whatever it still reports. An empty field reports its PLACEHOLDER in the
same `text` attribute as a value, so the ordinary case — a search box, a login
field — measured the hint and ran `KEYCODE_MOVE_END` plus hint + 8 backspaces
into a field that was already empty.

Those keys are not absorbed. `BaseKeyListener` leaves `KEYCODE_DEL` unconsumed
when there is nothing to delete, so each one reaches the app, and a widget that
reads backspace-on-empty as "remove the item before the caret" — a chip or
recipient field, a segmented OTP box, a React Native `TextInput` with an
`onKeyPress` handler — is emptied by a call that then returns `cleared: true`.
The docblock's justification ("backspace on an empty field is a no-op") holds
for the field's text and not for what the app receives.

Measured on an API 36 emulator against a one-`EditText` app whose key listener
drops a chip on backspace-with-an-empty-buffer, with the helper too old for the
atomic path — which is every device until the protocol-2 release ships:

  before  keys=17  emptyBksp=13  chips=0  value=[]
  after   keys=3   emptyBksp=0   chips=3  value=[]

The measurement now reads the `hint` out where the source carries one, so a
field showing its placeholder measures 0 and the read-back CONFIRMS the clear
instead of rescuing it. Where no hint is carried — API 34's dump, and the
devtools helper's `captureXml` on every level — a positive reading can be
either, and the run no longer goes out on it: the note reports the chord as
unconfirmed, which is what the base did before the read-back existed.

The rescue still fires on a residue the source can prove, verified on the same
device with a widget that swallows the Ctrl+A chord: the field ended empty
after a sized run.
2026-08-24 20:18:31 +02:00
Hubert Gancarczyk 679c78a7b8 docs(keyboard,flow): correct what warns, what a password box verifies, and how a helper goes stale
Six statements this PR left contradicting the code it shipped:

- `StepReport.warning` and `argent-cli`'s copy enumerated the raisers without
  the `run-sequence` route, and without saying that `keyboard` is the only
  tool read this way.
- `reliability-and-recovery.md` said raw `tool:` steps never warn. A raw
  `keyboard` clear does, and the same file's own table lists that row.
- `live-authoring.md` and `argent-qa-flows` both said a password box can never
  produce a verified clear. The helper reads one back whenever the write is
  empty, and says so in its own comment: only `{ clear: true, text: "..." }` on
  a password box is unverifiable.
- `flow-yaml.md`'s canonical `clear` section, which both new passages link back
  to, never mentioned the note or the warning at all.
- The `flow-execute` description listed only the idle and selector-less-gesture
  warning families, so an agent reading it could not learn that a clear warns.
- `helper_outdated` was blamed on "whatever started first, possibly from
  another argent install". `spawnHelper` always starts a fresh `am instrument`
  and connects to the port that process announces, so a foreign helper is never
  on our socket. The real routes are a lagging release and a stale install
  cache.

Also: the `keyboard` description now says the verified Android path is one
accessibility edit rather than key events, so `{ clear, text }` fires no
per-key handler there; and the last copy of the pre-atomic "~41s" worst case
is reconciled with the budgets the tool actually declares.
2026-08-24 12:26:12 +02:00
Hubert Gancarczyk e4fbf7c5b7 test(keyboard,flow): pin the warning's scope, the no-field note, and two boundaries
Each gap below was found by editing the production line and watching the
suites stay green.

- Nothing pinned the SCOPE of the note-to-warning promotion. Narrowing it to
  `keyboard` left all 1226 flow tests green, so a tool returning a healthy
  `note` could be promoted for good with nothing to object. Cover both
  directions, at the top level and one level down inside `run-sequence`.
- The `run-sequence` spelling of a clear had no test at all, in either
  direction, nor a case for a nested report whose shape is unrecognised.
- `dispatchForResult`'s `?? null` was unpinned: dropping it turns a tool that
  returned nothing into the skip a hang-up produces, and no test noticed.
- The note's wording for `no_focused_input` / `not_editable` was unpinned -
  only the `why` fragment was asserted - with a contrast case that keeps the
  plain wording for a refusal.
- `WEDGED_RPC_MS` was loosely pinned: both arms had a test, the boundary did
  not, so widening 2s to 14s stayed green.
- `keyboard-android.test.ts` never reset the atomic cooldown, so the first
  clear to fail left every later test in the file on the injected path by a
  different route.
2026-08-24 12:20:06 +02:00
Hubert Gancarczyk 70fad3b77f fix(flow): surface a keyboard clear's warning from inside a run-sequence step
`run-sequence` is a first-class flow tool step, `keyboard` is one of its
allowed tools, and `argent-device-interact` prescribes exactly this spelling
for a clear that types: `{ clear: true, text }`, then `{ key: "enter" }`.

The runner reads a step result's `note` at the top level only, so the note a
weak clear leaves at `result.steps[i].result.note` never reached the step
report. The step passed clean, the CLI printed `✓ 01 tool run-sequence` and
nothing else, and a QA flow went green in CI over a clear nothing verified -
the same vacuous gate that raw tool steps had, left open for this spelling.

Read the nested per-step results for the same tool, and keep every note a
sequence produced. Other tools' nested notes are ignored for the reason the
top-level read ignores them.
2026-08-24 11:45:12 +02:00
Hubert Gancarczyk a567a006b3 fix(keyboard): stop the note asserting a clear on a screen with no field to clear
Two skip reasons say the accessibility layer found nothing writable.
`no_focused_input` is a whole-screen negative - the helper sweeps the active
window and then every other one before it answers. `not_editable` is what a
screen carrying no text field at all answers in practice: a focused node
exists, it refuses the replace, and it reports itself as not editable.

On both, the note opened "so the field was cleared with the select-all chord
...", asserting the one thing the helper had just failed to find. `cleared:
true` and a non-zero `keys` said it again. The clear-only arm then appended
the read-back clause, which offers a screen that would not capture and a
password box, and omits the explanation the reason itself had given. No arm
carried the only remedy that helps: focus a field and retry.

Say the fallback was TRIED, keep what each fallback path cannot promise, and
close with what was sent and what to do. Not a claim of failure - a view the
tree cannot describe, a WebView or a Flutter surface, can still take the keys.
A read-back that saw an empty field outranks the reason and keeps the ordinary
wording, since the fallback is confirmed there.
2026-08-24 11:39:05 +02:00
Hubert Gancarczyk fd878d7f8a fix(android-devtools): fail the download when the helper release lags the manifest
The manifest versionCode is one of three numbers that must move together, and
the only one in this repo. The other two live in argent-private: the helper's
`PROTOCOL_VERSION` and the `aapt2 --version-code` the APK is linked with.
Nothing checked them against each other, so a release cut before the paired
helper lands ships an APK the manifest already outgrew.

That failure is silent. The install gate compares the device's versionCode
against the manifest's, so it can never be satisfied and every tool-server
process reinstalls the helper once per device. Every protocol-gated call - the
new atomic clear among them - reads the older protocol and falls back to its
weaker path, while the build reports nothing.

Read the downloaded APK's versionCode with aapt2 and refuse a mismatch, next
to the dylib platform check that guards the same class of mis-built release.
Hosts with no Android SDK skip the check, like the vtool and apksigner checks
around it.
2026-08-24 11:31:33 +02:00
Hubert Gancarczyk 67d49dd47c fix(flow): warn on a weak keyboard clear, not on every tool that returns a note
`resultNote` is a structural read of any `note` key, and the tool-step pass
branch applied it to every tool. Several tools attach a `note` to a HEALTHY
result: `open-url` explains that an http(s) URL may land in the browser,
`react-profiler-status` declares `note` required and fills it on the
running-session path, and `await-ui-element` reports a `hidden` wait met at
once. Each of those now marked its step `⚠` on every run, counted into the
run summary, and listed the flow under "what needs attention" forever — so
the QA-flow rule to resolve every passing-step warning could not be met.

Key the promotion on the tool instead. The Android `keyboard` clear is the
one whose `note` means the call passed weakly: it is emitted only when the
verified accessibility replace did not run. Nothing is dropped from the other
tools - their `note` still rides along in `result`.
2026-08-24 11:29:41 +02:00
Hubert Gancarczyk 1a0a073723 docs(flow): enumerate the keyboard-clear warning wherever warnings are listed
The runner gained a third family of step warning and nothing that enumerates
warnings, or tells an author how to resolve one, was updated for it. Five sites
still described the raisers as the idle wait plus the selector-less gesture:
`StepReport.warning` in flow-run and in argent-cli, `argent-qa-flows` §5 (whose
"resolve every passing-step warning" is unconditional and whose remedies covered
only those two), `live-authoring.md`, and the classification table in
`reliability-and-recovery.md`.

It is not a rare shape: `{ clear: true, text: … }` into any Android password
field can never verify — the read-back sees bullets — so it warns on every run.
The remedy differs by note: a helper that is missing, too old, or could not start
is environment, while a field that cannot be read back can never produce a
verified clear and has to be gated on an app result instead.
2026-08-20 22:20:55 +02:00
Hubert Gancarczyk 6be9deda58 fix(flow): surface a tool step's note as its warning, not only a directive's
`note` is the one signal saying whether an Android clear was the verified kind,
and only the `type` directive turned it into a step warning. A flow that carries
the same clear as a RAW tool step got nothing: `flow-add-step` has no
`keyboard` → `type` rewrite, so a recorded clear replays as a tool step whose
report put the note inside `result` — and `argent-cli`'s own `StepReport` has no
`result` field, so the CLI printed `✓ NN tool keyboard` and nothing else.

`argent flow run` is the runner the QA-flow skill prescribes for CI, so on the
raw form the warning gate was vacuous: the flow completed clean over an
unverified clear with no on-screen signal.

Verified end to end against a Pixel 7 / API 34: a one-step flow
`- tool: keyboard / args: { clear: true, text: abc }` now reports
`"status": "pass"` with the note in `warning`.
2026-08-20 22:16:21 +02:00
Hubert Gancarczyk aa33dd970d test(android-devtools): pin which RPCs get the long client timeout
Nothing in the repo covered `android-devtools-client.ts` at all, so reverting
`method === "getHierarchy" || method === "setText"` to the single-method ternary
left every keyboard suite green — a one-token edit that reads as tidying and
reintroduces a doubled write: a host that gives up at 5s reports "fall back",
the injected clear runs, and the accessibility `ACTION_SET_TEXT` it stopped
waiting for can still land in the middle of it.

The test drives the real client over a real socket that never answers, and reads
the budget out of the timeout it raises.
2026-08-20 22:06:53 +02:00
Hubert Gancarczyk 37f995ef10 test(keyboard): pin the error_kind carried through the kept-selection rewrap
The accompanying test seeded a cause whose `error_kind` was already
`"subprocess"` — identical to the `??` fallback — so it asserted an incidental
property rather than the propagation. Replacing the whole expression with a
hardcoded `error_kind: "subprocess"` left all three keyboard suites green.

`injectAndroidText` can raise `timeout` and `dependency_missing` as well, and
both sibling rewraps in `android-input.ts` are pinned against exactly that.
2026-08-20 22:05:56 +02:00
Hubert Gancarczyk 0cbb4591a9 docs(skill): put the clear-verification caveat back, with the Android note
The deleted sentence — "A `cleared: true` result does not prove the field is
empty" — is still true everywhere except Android's new atomic path. iOS cannot
read the field at all, and Chromium degrades to "seen empty, or not observable".

After the deletion this file said nothing about verification at all, while still
routing "clear and retype field" to a single call, and the `note` this change
adds as its one "a weaker path ran" signal was invisible from here. Both sibling
documents kept and extended the caveat in the same change.
2026-08-20 22:04:28 +02:00
Hubert Gancarczyk 1c44ce969c docs(flow): correct what the Android keyboard does with a cancelled run
The block reasoning `dispatchOrAbort` and `dispatchForResult` rest on said
"neither `runAndroidPhoneType` nor `runChromium` takes a signal at all — Android's
own comment says so outright", and concluded that a cancelled `type` still types
the whole text out on both. That was true on the base branch. This change gives
`runAndroidPhoneType` a signal and two further checks, so a `{ clear, text }`
cancelled during the atomic leg now types nothing.

Chromium is unchanged and keeps the paragraph's original claim.
2026-08-20 22:04:11 +02:00
Hubert Gancarczyk 216fd113c3 docs(keyboard): scope "no note means verified" to the backend that can set one
`note` sits on the shared `KeyboardResult`, but only the Android backend ever
sets it — the iOS and Chromium returns are plain `{ typed, keys, cleared? }`
literals. So "absent means the clear was the verified one" was true on Android
and structurally false everywhere else, twelve lines under the same docblock
saying iOS "cannot read the field at all".
2026-08-20 22:03:49 +02:00
Hubert Gancarczyk 7d7fbe735d docs(keyboard): stop the tool description contradicting itself about a 400
Three sentences in one paragraph disagreed. "A support probe and a screen dump go
to the device first — neither of which changes it" was true when the over-length
refusal existed only on the legacy path; it is now also reached after the
select-all chord and a real delete. "Either way a 400 leaves the field exactly as
it was" then said the opposite of the exception inserted two sentences above it,
and it is the closing sentence an agent acts on.

The exception as worded also covered only the read-back-mismatch shape, while
`unverifiable` — which a password field always returns for a non-empty write —
reaches the same fallback with the write applied.

Rewritten as one rule with its cases: the pre-send rejections leave the field
untouched; Android's over-length refusal is decided from a read, and what went to
the device before it differs by level and by whether a replace was accepted.
2026-08-20 22:03:34 +02:00
Hubert Gancarczyk 62a1394349 docs(android-input): correct the { clear, text } worst case for the atomic tier
The docblock kept the pre-atomic "~41s" while the two sibling statements moved
in the same change (`keyboard/index.ts` to "well past a minute",
`flow-add-step.ts` to "over a minute"), so three sites disagreed.

Summing the real constants for an Android `{ clear, text }`: 8s
ATOMIC_CLEAR_BUDGET_MS + 5s `ping` + 15s `setText` + 26s ANDROID_CLEAR_BUDGET_MS
+ 15s ADB_INPUT_TIMEOUT_MS = 69s.
2026-08-20 22:02:17 +02:00
Hubert Gancarczyk 81142dbe77 docs(keyboard): correct the RPC budget the atomic clear actually spends
The docblock said both RPCs are bounded by DEFAULT_RPC_TIMEOUT_MS, "so a helper
that accepts the socket and then goes silent costs at most those two timeouts on
top". The same change moves `setText` onto LONG_RPC_TIMEOUT_MS, so the real
worst case on top of the 8s start is 5s + 15s = 20s, not 10s.

The figure is load-bearing — it is the stated rationale both for the 8s ceiling
and for leaving the RPCs unraced — and the sibling statement in `keyboard/index.ts`
had the mirror-image error, calling both RPCs 15s when `ping` is 5s.
2026-08-20 22:02:01 +02:00
Hubert Gancarczyk 33342993df fix(keyboard): do not send the measurement back into a start this call abandoned
`devtoolsHierarchyReader`'s premise is "with it down there is no contention", and
its gate counts STARTING as live. That is right when ANOTHER caller is spawning
the helper — joining that init beats racing the holder it is about to become.
It is wrong for the one caller that started it in THIS call and gave up on it:
the node it finds is its own, and the init it awaits is the one the atomic
attempt already abandoned, so the read waits out its whole share for an answer
that has already failed to arrive once.

That caller now passes no reader, which puts it back on the dump — exactly what
ran before the helper was consulted at all.

Measured on a Pixel 7 / API 34, one `{ clear: true }` against a start that never
settles: 16049 ms before, 11054 ms after, with the field measured (not the blind
run) in both.
2026-08-20 22:01:38 +02:00
Hubert Gancarczyk cd85a9edbc docs(keyboard): say what the cooldown mark's delete actually does
The comment claimed it stops "a device that comes good" being held back for the
rest of the cooldown. It cannot: the guard above returns early for any mark
inside its window, so the only mark reachable at that line has already expired,
and an expired mark gates nothing. What the delete does is keep the map bounded.
2026-08-20 21:54:55 +02:00
Hubert Gancarczyk 788959f8ae fix(keyboard): arm the atomic-clear cooldown for the failures it was written for
The mark was written only when `resolveService` rejected BEFORE the 8s budget.
Every failure ATOMIC_START_COOLDOWN_MS names is slower than that — its own
docblock quotes the blueprint's 60s install plus 30s readiness wait — so the
cooldown armed for the cheap failures and stayed unarmed for the expensive ones.
A start failing half a second past the budget left every clear paying the full
budget and starting a fresh install, forever: the exact outcome the constant
says it prevents.

The race abandons the loser rather than cancelling it, so the rejection arrives
long after this call returned. It is recorded where it actually lands — on the
`catch` attached to the abandoned promise, which until now discarded it. (That
handler was never needed to keep the rejection handled: `Promise.race`
subscribes to every input, so nothing was going unhandled. It has a job now.)

The other half is a helper the registry DOES resolve and that never answers: it
returns `rpc_failed`, recorded nothing, and the successful resolve had already
deleted any mark — so a wedged helper was re-asked on every clear forever, at
5s for a hung `ping` or 15s for a hung `setText`. That is the more expensive
state, not a cheaper one. A failed leg slower than WEDGED_RPC_MS now arms the
same cooldown; a fast rejection still does not, since retrying it costs nothing.

Measured on a Pixel 7 / API 34, three consecutive clears each:
  start rejecting at 8.5s — before 11899/11594/11366 ms, after 11504/2826/2840
  helper up and silent    — after 17307/1303/1289 ms
2026-08-20 21:54:37 +02:00
Hubert Gancarczyk 2e27e08723 fix(keyboard): stop saying nothing was modified after a replace the widget accepted
`unverifiable` and `value_mismatch` fall through with `applied: true`, and on a
level whose `input` has no `keycombination` the fallback is the legacy one — its
refusal says "Nothing was modified and nothing was typed." Nothing ties the
helper's protocol to the API level (the manifest carries no minSdk gate, and the
helper is measured running on API 30), so protocol 2 plus no `keycombination` is
an ordinary configuration.

"Nothing was modified" is the sentence that makes a retry against the original
value look safe, and it contradicts the doubling warning the note gives for the
same two reasons.

Verified on a real API 30 emulator with a 160-character field: the refusal now
reads "Nothing was typed. An accessibility replace was ACCEPTED by the widget
before this ran, so the field may already hold the value you asked for."
2026-08-20 21:46:31 +02:00
Hubert Gancarczyk bd372218c7 fix(keyboard): stop the post-chord refusal asserting what the device did
The over-length message reached from the select-all path said "The select-all
did not take on this field" and "the delete that followed the select-all removed
one character from it". Neither is known at that point. The count comes from the
same whole-screen measurement the message itself warns about two sentences
later: it can be another window's focused field, or this field's own
placeholder, which the view hierarchy reports in the same attribute as a value.

Measured on a Pixel 7 / API 34 against a focused field with a 181-character
placeholder: the chord took, the field ended EMPTY with one text change, and the
call still returned 400 — quoting 181 characters and both device claims, every
one of them false.

The refusal stays: a residue that long really cannot be backspaced away, and
shipping `cleared: true` over it is the failure this whole change exists to
close. What it now says is the pair of states the caller can actually act on.
2026-08-20 21:39:34 +02:00
Hubert Gancarczyk 3daac0618d fix(keyboard): honour a hang-up before pressing the key that follows a clear
The abort check between the tiers guards only the failing arm, so a `key` that
followed a SUCCESSFUL accessibility replace went out with no signal check at
all. Both twins do the opposite — `simulator-server-keys.ts` and
`platforms/chromium.ts` check immediately before the press, the latter with a
comment naming `{ clear, key: "enter" }`.

So a cancelled `{ clear: true, key: "enter" }` submitted the field when the
helper was present and did not when it was absent: same request, same hang-up,
opposite side effect, decided by a path the caller cannot see. The window is the
whole atomic leg — the start budget plus both RPCs.

The injected arm's own key press gets the same check, for the same reason: the
clear and the text are already on the device and cannot be taken back, but the
submit can still be withheld.

Verified on a Pixel 7 / API 34 by hanging up 2s into a 6s replace and sending
`{ clear: true, key: "space" }` — the space is observable in the field. Before:
`value=[Monday ] changes=1`. After: `value=[Monday] changes=0`.
2026-08-20 21:34:07 +02:00
Hubert Gancarczyk be0322805e fix(keyboard): do not tell the agent to read back a field it filled from a secret
The note closed with "Read the field back if the exact value matters" on every
call, including one whose text came from a `{{secret:…}}` placeholder — arriving
in the same response as the notice saying the after-typing screenshot was
skipped, and against `argent-device-interact`, which tells the agent to submit or
navigate away rather than `describe` such a field.

The pairing is not rare: a password field always answers `unverifiable` for a
non-empty write, so a credential entry produces this note on almost every call.

Verified on a Pixel 7 / API 34 with a real `ARGENT_SECRET_` placeholder resolved
through the tool: the note now says not to read the field back, and names the
app's own result instead.
2026-08-20 21:25:18 +02:00
Hubert Gancarczyk e3245c4c4f fix(keyboard): stop reporting a read-back the clear never got
The `select-all` arm asserted "The field was read back afterwards and nothing was
left to remove", but three different read-backs reach that path and only one of
them saw an empty field. A screen the reader could not capture was covered by
the caveat that followed; a field the reader DID capture and could not measure
was not — a focused password box is floored to the blind count rather than
dropped, so it lands here with the note claiming a positive reading.

That is the wrong field to be wrong about: on a password box whose widget
swallows the chord — the failure this change exists to close — the caller was
told the box is clear while the credential is still in it.

The outcome now carries which of the two happened, and the note says so.

Verified on a Pixel 7 / API 34 against a focused password field: the note reads
"Nothing confirmed the chord took".
2026-08-20 21:20:42 +02:00
Hubert Gancarczyk 32aeccad8b fix(keyboard): stop reading a placeholder as proof the select-all failed
The read-back gate feeds on `measureFocusedTextLength`, which reads the dump's
`text` attribute — and an EMPTY field reports its placeholder there. Neither
reader carries a hint attribute to tell the two apart on the levels this path
serves: checked on API 34, `uiautomator` emits no `hint`, and the helper's
`captureXml` does not either.

Measured on a Pixel 7 / API 34 against a hinted native `EditText` holding
"Monday": the chord took, the field ended empty with exactly one text change,
the read-back returned "type here" — and the note came back saying "the chord
did NOT take on this field", contradicting the `cleared: true` it ships with.
Every search box, login field and URL bar reads that way.

The run still goes out: backspace on an empty field is a no-op, while skipping
it on a real residue leaves the value the clear was asked to remove. What
changes is the claim — the arm now reports the reading and what the run did,
and says why the reading cannot separate the two.
2026-08-20 21:16:04 +02:00
Hubert Gancarczyk b69f2ff498 fix(keyboard): stop warning about a doubled value on a clear-only call
The doubling warning needs a SECOND write to exist. A clear-only call has none —
its fallback deletes — and the accepted replace wrote the empty string, which is
what the fallback leaves behind anyway. So `{ clear: true }` came back saying
the field "may now hold that value with the fallback's text added to it" over a
call that carried no text, in the same note that had just said the field read
back empty.

Both pinned cases passed `text: "abc"`, so the clear-only shape was unpinned.
2026-08-20 21:11:27 +02:00
Hubert Gancarczyk b742d2e404 fix(keyboard): decide the doubled-value warning from applied, not the reason's name
`applied` was destructured away and never read, so the note's doubling warning
came from a set of reason NAMES. That answers "nothing was written" for the two
shapes that arrive from outside the table:

  - a reply carrying `applied: true` with no reason at all — `reason` is
    optional on `SetTextResult`, and this is the one shape the `??` fallback
    exists for. It was labelled `action_refused`, so the caller was told the
    widget refused AND lost the warning that the fallback is about to write on
    top of a value the tool itself put in the field.
  - a reason from a helper newer than this build, which renders as "a reason
    this version does not recognise" and was likewise treated as a refusal.

The unnamed miss is now `value_mismatch`, which is what it is: the widget took
the action and the field read back holding something else.

The shared mock derives `applied` the way the helper does, so a case that names
a reason cannot state it wrongly.
2026-08-20 21:06:04 +02:00
Hubert Gancarczyk 1d7f7a4d06 fix(keyboard): read the setText reason as an own property, not through the prototype
`reason` crosses the RPC boundary as a free string from a helper that may be
newer than this build, so `WHY[reason] ?? …` resolved an inherited key rather
than falling back: `constructor` rendered `function Object() { [native code] }`
into the note, and `toString` and `hasOwnProperty` the same way.

The guard was added so an unrecognised reason does not read as "(undefined)";
these three read worse than that. Same own-property check, for the same reason,
as `resolveAndroidNamedKeycode`.
2026-08-20 20:56:51 +02:00
Hubert Gancarczyk a35d843e0b refactor(android-devtools): drop the setText reply's length field
The helper stopped sending it: on a match it was the REQUEST's length - the
caller's own string - and -1 on every other outcome, so no input made it say
anything. This side only ever destructured `{ matched, reason }`.

The two halves are one wire contract, so they move together with
argent-private's "drop the setText reply's length field".

Also names the password case on `unverifiable`, now that the helper reports it
there rather than as `value_mismatch`. A password field publishes one bullet
per character instead of its value, so a non-empty write can never read back
equal to itself; clearing one still verifies, because empty is empty whether or
not it is masked.
2026-08-19 13:32:21 +02:00
Hubert Gancarczyk 8ef553d55f test(keyboard): make the setText mock's length match the helper
The shared mock returned `text.length` for its `matched: false` cases too, a
reply the helper no longer produces. Nothing asserted on it, so this is
fidelity rather than a fix - but a mock that can return what the real thing
cannot is where a future assertion goes wrong.
2026-08-18 20:24:09 +02:00
Hubert Gancarczyk 79f5979628 docs(android-devtools): correct the setText reply contract
Three statements went stale when the helper changed.

`length` is documented as "the value read back". It is the length of the
REQUESTED value on a match and -1 otherwise - the helper never measures what
the field holds, because the one case where the two differ is `value_mismatch`,
where the field holds something the caller did not send and which the threat
model on setText says can be a credential.

"Only a broken RPC throws" is no longer true: a `params.text` that is not a
string is refused by name rather than coerced into the user's field. Not
reachable from this client, whose schema is `z.string().optional()`, but the
sentence claimed more than it could.

The SET_TEXT_MIN_PROTOCOL block described the versionCode not moving, and
offered `adb uninstall` as the way onto a new build. It has moved, so the
paragraph and its workaround go; the race that can still hand back an older
protocol stays.
2026-08-18 20:24:09 +02:00
Hubert Gancarczyk af8da14a56 fix(android-devtools): bump the helper manifest versionCode to 2
`ensureAndroidDevtoolsInstalled` compares the device's version code against
this number, so with both sides at 1 it returned early on every device that
had ever run the Android describe path. `setText` and the PROTOCOL_VERSION 2
that gates it reached only a device that had never had the helper.

Pairs with argent-private's build-native-binaries.yml, which now links the APK
with `--version-code 2`. Both numbers have to move together: bumping only this
one leaves the gate unsatisfiable and reinstalls on every tool-server process.
2026-08-18 20:24:09 +02:00
Hubert Gancarczyk 7df9ed6ef5 feat(keyboard): clear Android fields atomically 2026-08-18 17:10:41 +02:00
Hubert Gancarczyk 160d340bb2 chore: remove bloat from SKILLs 2026-08-17 15:22:54 +02:00
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.
2026-08-17 15:22:54 +02:00
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.
2026-08-17 15:22:54 +02:00
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.
2026-08-17 15:22:54 +02:00
Hubert Gancarczyk 026b36c3ad docs: fit the type directive's polish rules to the two-step recording
Three corrections, all from the same cause: `keyboard` rejects `text`
and `key` in one call, so a recorded type-then-submit is ALWAYS two
steps and the polish rules have to read them as a pair.

`live-authoring.md`'s two carry-across rules contradicted each other on
exactly that shape. "Carry a recorded `key: "enter"` across as
`submit: true`" covers the Enter step; "a recorded step with `text` but
no `key` needs `submit: false`" covers the text step — and always will,
since no recorded text step can carry a key. Following both produced
`submit: false` for a recording that did submit, so the cleaned login
flow never fired the form; following only the first and leaving the raw
Enter step in place submitted twice. The rules are now one table over
the whole run of `keyboard` steps on a field, which is the unit the
conversion actually operates on, with the two failure modes named under
it. The conversion table further down defers to it rather than giving a
second, shorter answer.

`keyboard`'s `clear` parameter said a swallowed select-all leaves the
field "one character shorter … and a combined `text` then appends to
that". Per the caret rule in the same sentence, that holds on iOS only:
on Android the surviving delete acts as a backspace at the caret the tap
left mid-value, and the text is spliced there. `flow-yaml.md` already
documented the splice per platform, so the two disagreed on Android.

`asserting-field-values.md` did not mention that Chromium truncates
every accessible name at 200 characters — Chromium alone. An `equals`
against a longer value cannot match, and a long pre-filled field is one
of the shapes `clear` exists for.
2026-08-17 15:22:54 +02:00
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.
2026-08-17 15:22:54 +02:00
Hubert Gancarczyk 66a67d5f3a docs: correct what the clear's own docs claim about it
- `live-authoring.md`'s Replay section said a `type:` whose focus was never
  confirmed is scored a pass. True of a plain `type`; a `clear` fails.
- `flow-yaml.md` said the clear dispatches on three findings and no others.
  There is a fourth — the window where no read saw focus — which the same
  paragraph goes on to describe.
- The residual behind that fourth is materially bigger than "nothing was
  focused, so nothing can be lost", in both `flow-yaml.md` and `runType`'s
  comment. An audit of the prunes in the adapters gives at least four more
  shapes: an off-screen or scroll-clipped frame, iOS's `hidden`/`alpha` prune,
  Android's system-UI prune (the notification direct-reply field, the
  bouncer), and a truncated tree on any source. Each is a field the step never
  named, and the step reports a pass.
- `dispatchOrAbort`'s docblock said android and chromium check the signal
  between keypresses like iOS. They check once, at their per-device queue, and
  neither `runAndroidPhoneType` nor `runChromium` takes a signal — cancel a run
  mid-`type` there and the whole text is still typed.
- The same block said `gesture-tap`'s platform handlers discard the context.
  It has no platform handlers, and `execute` declares no context parameter.
- `waitForFrameAndTree` said its stale-tree window opens only when a read fails
  outright; `settleTree` also returns best-effort when every read succeeded and
  no two matched.
- `describe/contract.ts` said platforms other than Vega leave `focused` unset.
  All three set it, and the flow focus wait reads nothing else.
- `flow-run.ts`'s description never said a `clear` can hard-fail the step, and
  mentioned `submit` only for the clear-only default.
- `flow-finish-recording.ts` described `type.submit` in the one direction that
  no longer matters.
- `reliability-and-recovery.md` prescribed "assert the committed value" with no
  pointer to the reference that says when it hard-fails.
- `argent-qa-flows` never mentioned `clear`, though running the same YAML twice
  with no manual data reset is exactly what it is for.
2026-08-17 15:22:54 +02:00
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.
2026-08-17 15:22:53 +02:00
Hubert Gancarczyk 6014a56d7b fix(flow): stop the window's own edge being reported as a dead tree source
Every focus poll is given whatever is left of the window as its read budget, so
a read started too late is abandoned mid-flight — and `giveUp` books that
abandonment as darkness. The step then failed telling the author "the device's
tree source is down" while every read in the window had answered, and because
crossing FOCUS_DARK_TAIL_TOLERANCE_MS depends on how the read time divides into
the 3000ms window, the verdict was not monotonic in latency.

Reproduced on Chrome 151 against the branch tool-server, on a page whose focus
the tree cannot report and whose reads were slowed uniformly: 0ms and 150ms
passed, 250ms failed as an outage, 300ms and 400ms passed again. No read failed
in any run. Android's `getHierarchy` sits in that band — 2.85s / 0.29s / 0.46s
/ 0.30s across four consecutive idle describes on this host — which is what
turns it into a flaky step rather than a consistently wrong one.

A read the window cannot afford is no longer started, measured against what the
last successful read on this device cost. The same band now passes at every
latency, 250ms three times over, while a source whose read never completes
inside the window is still reported as an outage.
2026-08-17 15:22:53 +02:00
Hubert Gancarczyk 8e80634389 fix(describe): keep the branch holding the caret through the visibility prune
The walker prunes an invisible subtree before focus is ever considered, so the
field that is invisible BY DESIGN — the capture input under rendered OTP/PIN
boxes, a barcode-scanner sink, a styled file input — left the tree with no
focus flag anywhere. The type directive's focus wait reads that as "nothing is
focused", which is the one non-confirmed outcome it dispatches a destructive
clear on, and the field the caret is in is very often the one holding the
secret.

Reproduced on Chrome 151 against the branch tool-server: rendered OTP boxes
over an opacity:0 capture input holding `SECRET-OTP-123456`, a step naming the
visible box — tree focus flags NONE, step pass, secret rewritten. The identical
page at opacity:1 refused. It now refuses at both, with the secret intact.

opacity:0 does not stop an element being focused, so the prune now asks whether
the element is (or contains) the caret's element first. With nothing focused,
opacity:0 and display:none subtrees are pruned exactly as before.
2026-08-17 15:22:53 +02:00
Hubert Gancarczyk 906a7272fe fix(flow): require the focused node's name to be unambiguous in BOTH trees
`focusedFromInside` joins its two halves by name — the focused node comes from
the current tree and the current container box, the evidence about it from the
pre-tap tree and the pre-tap box — and only the pre-tap side was checked for
ambiguity. A focused node could therefore borrow the evidence collected for a
different element. Three ways in, all reproduced on Chrome 151 against the
branch tool-server:

  - a namesake the TAP CREATES: a wrapper that appends a second `row_input` on
    focus and focuses it passed, and rewrote the field the step never named;
  - a container that GROWS: a 44px row growing to 140px on focus swallowed a
    field that was never a tap candidate, and cleared it;
  - a focused field that RENAMES ITSELF out of the namesake set (a currency
    input dropping its separators), leaving the sibling under the tap as the
    unique pre-tap match — the amount-row mis-target the docs say is refused.

All three leave two namesakes inside the container in the current tree, so
deciding on that too refuses all three, with nothing emptied. The controls that
remove the growth, the late namesake or the rename still refuse, and the
legitimate wrapper clears still pass — a testID box over one input, both with
the tap centre on the input and with it on a tall label.
2026-08-17 15:22:53 +02:00