Skip to content

perf: speed up iOS agent recovery and streamline CLI guidance - #1700

Merged
thymikee merged 9 commits into
mainfrom
agent/safer-parent-touch-targets
Aug 10, 2026
Merged

perf: speed up iOS agent recovery and streamline CLI guidance#1700
thymikee merged 9 commits into
mainfrom
agent/safer-parent-touch-targets

Conversation

@thymikee

@thymikee thymikee commented Aug 9, 2026

Copy link
Copy Markdown
Member

Summary

  • preserve parent selector/ref semantics while choosing a parent-owned touch point outside independently interactive descendants; fully tiled targets fail closed with the caller's original selector/ref context
  • recover promptly from sparse iOS accessibility trees by rejecting shell-only trees, skipping an optional XCTest text-input probe after that channel is penalized, and directing agents to screenshot/coordinate recovery
  • make the default help a short actionable workflow, keep exhaustive reference behind topic help, and make the optional skill mirror that default help while preserving @ref and ~sN syntax

These are generic interaction and agent-guidance changes. Product code and help contain no benchmark task, app, or model-specific strings.

Review expanded the original interaction/help scope into its owning ADR 0011 guarantee classification and geometry property coverage. The final PR touches 34 files but does not add another product command family.

Validation

  • pnpm check:affected --base origin/main --head HEAD --run on Node 24 with unit tests serialized (maxWorkers: 1 during validation)
    • 424/424 test files and 4,138/4,138 tests passed
    • changed-line coverage: 84/85 (98.82%); changed-branch coverage: 54/64 (84.38%)
    • build, package, provider integration, Node integration, progress, and replay-compatibility checks passed
  • AGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTS=1 pnpm build:xcuitest:ios passed
  • red-before-green regression proofs:
    • parent-touch test initially dispatched the child image center, then selected the parent-owned text band after the fix
    • sparse-tree classifier initially accepted a full-screen shell identifier, then rejected it after the fix
    • the fully tiled selector contract initially failed because public output said Ref @e2; it now preserves Selector label=Card, details.selector, and omits details.ref
  • the new parentOwnedTouchPoint guarantee classifies all seven dispatch paths; runtime selector/ref/target-drag cells have contract scenarios, while direct-runner gaps are explicit and issue-owned
  • fast-check properties prove resolved points stay inside target/bounds, outside competing descendants, and invariant under node-array permutations
  • live iOS coordinate interaction completed in about 2.2s with the penalized XCTest text-input probe skipped

Local Bluesky results use the same app/model setup and correct final states. Repeats are reported in full rather than selecting the fastest result; the sample is exploratory (n=2 for fixed agent-device, n=1 for local Argent bsky-09).

Task Original outlier Fixed agent-device repeats Fixed median Local Argent
bsky-06 266.7s / 41 tools 45.2s / 3, 50.8s / 3 48.0s / 3
bsky-09 735.1s / 88 tools 99.4s / 11, 64.8s / 7 82.1s / 9 162.3s / 16

Across the four fixed repeats, median time is 57.8s versus 500.9s for the two original outliers (88.5% lower). On the directly comparable bsky-09 lane, the two-run agent-device mean/median is 49% lower than the single local Argent run. A no-skill Element check also completed correctly in 41.7s / 6 tools.

The outliers came from three compounding generic failure modes: a parent ref could activate a child-owned region, shell-only accessibility output looked healthier than it was, and guidance did not move the model to visual coordinate recovery quickly enough. The compact default help and skill now present the same first-screen workflow; the skill remains primarily a discoverability entry in registries such as skills.sh.

Benchmark setup ran doctor to prepare the documented runner cache. Benchmark-runner setup and prompt experiments are intentionally excluded from this product PR. Native/device CI lanes remain GitHub-authoritative.

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.13 MB 2.13 MB +425 B
JS gzip 695.2 kB 695.5 kB +273 B
npm tarball 826.8 kB 826.8 kB -51 B
npm unpacked 2.88 MB 2.88 MB -268 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.3 ms 26.6 ms +0.3 ms
CLI --help 65.1 ms 65.3 ms +0.2 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/prepare-kind.js +3.3 kB +1.0 kB
dist/src/cli-help.js -2.9 kB -782 B
dist/src/version.js +110 B +25 B

@thymikee

thymikee commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

Exact head b1d9f3a is not ready. P2: the new blocked-target branch in resolveNodeTouchPoint hardcodes Ref @${node.ref} for both callers. A selector activation whose interactive children fully tile the target therefore reports a ref the user never supplied, while the successful runtime test covers only selector/ref point parity. Pass the caller’s original target label/kind into the blocked error (preserving selector-specific recovery context) and add a route-level fully-tiled selector regression that asserts the public error shape. The parent-owned point regression itself is load-bearing and exact-head Bluesky evidence exercises the intended iOS route. Separately, Coverage is red only because Vitest timed out terminating the unchanged scripts/fuzz/corpus-replay.test.ts worker, with no failed assertion; rerun it green after the code fix. No ready-for-human label while the finding/red check remains.

@thymikee thymikee changed the title Avoid interactive children in parent taps Avoid child controls in parent taps and streamline CLI help Aug 10, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Exact-head re-review at eae87aebd: not ready. (1) Prior P2 remains: the fully tiled selector path still hardcodes Ref @${node.ref} in resolveNodeTouchPoint, so selector users receive a ref-shaped error they never supplied; add a route-level selector regression and preserve the original target context. (2) New P2: parent-owned touch-point selection/refusal is now a cross-dispatch interaction guarantee, but interaction-guarantees.ts is unchanged and its occlusion cells still cite only isSnapshotNodeInteractionBlocked, which excludes ancestor/descendant pairs. Classify the behavior for every path and add the required interaction-contract scenario instead of leaving direct-iOS/native-ref/drag/Maestro parity implicit. (3) The new pure geometry kernel has example-only tests; add fast-check properties proving returned points remain inside target/bounds, outside competing descendants, and are permutation-invariant. Most checks are green; iOS smoke was pending at review. PR is still draft and its title lacks a conventional prefix. No ready label applied.

@thymikee thymikee changed the title Avoid child controls in parent taps and streamline CLI help Speed up iOS agent recovery and streamline CLI guidance Aug 10, 2026
@thymikee
thymikee marked this pull request as ready for review August 10, 2026 13:05
@thymikee

Copy link
Copy Markdown
Member Author

Exact-head re-review at 3e123b1f1: not ready. The three prior P2s remain: the fully tiled selector path still reports hardcoded Ref @${node.ref} without a route-level selector error regression; parent-owned touch/refusal remains unclassified in interaction-guarantees.ts with no interaction-contract scenario; and the geometry kernel still has example-only coverage rather than fast-check inside-bounds/outside-descendants/permutation properties.

There is also a new behavior blocker in the penalized-XCTest optimization: coordinate press skips textInputAt, passes nil to rememberTextEntryTap, and clears the one-shot text-input witness. With the software keyboard hidden (notably hardware-keyboard simulator mode), the immediately following bare type depends on that witness and otherwise fails NOT_FOCUSED, breaking the documented coordinate press → type flow exactly in the sparse-tree recovery state. The new boolean test and generic coordinate timing do not cover that production contract; preserve scoped text-entry behavior and add an exact-route regression/live check.

The sparse-tree classifier itself looks correct, but its new Swift coverage and the probe test are absent from CI's explicit -only-testing list, so the authoritative lane only compiles them. Execute the load-bearing regressions in that lane. The current iOS alert wait 5000 failure matches the identical pre-branch main flake from run 31242736650, so it is infrastructure/fixture-shaped rather than a confirmed code regression; exact head is still red/UNSTABLE and needs a green rerun. No ready label applied.

@thymikee thymikee changed the title Speed up iOS agent recovery and streamline CLI guidance perf: speed up iOS agent recovery and streamline CLI guidance Aug 10, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Addressed all review findings in 984524fb6:

  1. Fully tiled runtime targets now receive the caller context explicitly. Selector failures report Selector label=Card with details.selector and no invented details.ref; ref failures retain their ref shape. Runtime selector/ref and target-drag contract scenarios assert refusal before dispatch.
  2. Added the parentOwnedTouchPoint ADR 0011 guarantee and classified all seven dispatch paths. Runtime selector/ref/target-drag cells point to the shared kernel and have contract coverage; direct iOS/Maestro gaps are explicit, pinned, and linked to Interaction guarantee gaps (ADR 0011 umbrella) #1081; native-ref/coordinate are documented inapplicable.
  3. Added bounded fast-check properties proving resolved points remain inside target/bounds, outside every competing descendant, and invariant under node-array permutations.

Red proof: the new selector contract failed against the previous resolver because it returned Ref @e2 has no parent-owned touch point...; it passes with the selector-specific public shape now.

Validation: serialized Node 24 affected gate passed (424/424 files, 4,138/4,138 tests), changed-line coverage 84/85 (98.82%), plus build/package/provider/Node integration/progress/replay-compat. The earlier timeout-only Coverage failure has therefore been rerun green locally; GitHub checks are restarting on the pushed head. The PR title now has the conventional perf: prefix.

@thymikee

Copy link
Copy Markdown
Member Author

Addressed the remaining exact-head feedback in 268301b3c.

  • Penalized coordinate taps now retain a one-shot, identity-bound unresolved-first-responder witness only when the active bundle and a positive PID are known.
  • The immediately following bare type can use the existing synthesized-first-responder route; the witness is consumed before validation, so a second bare type is refused.
  • Added an exact live-simulator regression for successful coordinate tap → first bare type succeeds → second bare type fails without changing the field.
  • Added that regression, the penalized-probe regression, and the sparse-payload policy matrix to the iOS workflow's explicit -only-testing list.

Red proof: against the pre-fix implementation, the exact simulator test returned TEXT_INPUT_NOT_FOCUSED, reported no route, and left the field empty.

Green proof: the three selected runner tests passed on the simulator (3/3, 0 failures); the exact test used synthesized-first-responder for the first type and refused the second after consuming the witness. The required affected gate also passed: 424/424 files, 4,138/4,138 tests, 84/85 changed lines (98.82%), plus package, build, provider integration, Node integration, progress, and replay-compat checks.

I also tried the now-allowed two-worker unit setting. It reproduced the documented request-router-replay-scope contention timeout (5.01s against a 5s budget) with no substantive failure, so I reran the required push gate serialized; the same test passed in 4.64s and the complete gate was green.

@thymikee

Copy link
Copy Markdown
Member Author

Exact-head re-review at 268301b3c: not ready. The prior selector-context, ADR 0011 guarantee/contract, and geometry-property findings are fixed, and the new Swift regressions are now executed by the authoritative iOS lane.

One P1 target-safety blocker remains. rememberCoordinateTextEntryTap(nil, xCTestProbeSkipped: true) turns any successful penalized coordinate tap into a bundle/PID-only unresolved-first-responder witness, including taps on non-text controls. typeTextReliably then permits synthesized typing with no resolved element, and its inner dispatch no longer requires the concrete tap-witness target to still exist. That can type into a stale process first responder instead of failing closed.

The exact-head iOS failure is substantive, not the prior alert-wait infrastructure flake: testBareDelayedTypeFailsWhenTappedInputDisappearsMidCommand now returns success/code nil instead of TEXT_INPUT_NOT_FOCUSED, and testPenalizedCoordinateTapPreservesBareTypeWitnessWhenSoftwareKeyboardIsHidden reports an OK synthesized route while the field contains only coordinate-witne, proving the nil-target route cannot verify the full commit. Preserve element/responder identity before dispatch, add a negative penalized coordinate-tap-on-non-text-control regression with a stale responder, and require the positive route to verify the complete committed value. Rerun the authoritative targeted XCTest lane and exact-head iOS changed-route evidence green. No ready label applied.

@thymikee

Copy link
Copy Markdown
Member Author

Addressed the three follow-up review findings in f9ae545c4.

  1. Parent-edge clearance now adapts to the target's short axis, while clearance from independently interactive descendants remains the full 12px. A 22px dense row with a side control resolves to its safe center instead of failing closed.
  2. Blocked-target diagnostics still bound competitorRefs to five entries and now disclose the full competitorCount. The contract fixture has 10 tiling children and asserts five refs plus competitorCount: 10.
  3. Root help now has a structural test that extracts every command from the hand-written overview groups and checks membership in listCliCommandNames().

Red evidence:

  • the 22px row returned blocked before the clearance split;
  • the selector contract received competitorCount: undefined;
  • a planted traces rename failed with root help names unknown command: traces.

Expanding the geometry property generator to thin desktop axes also found a half-pixel clipping edge: rounding could move a candidate outside the effective viewport bound. That shrunk counterexample is now pinned, and rounded candidates outside the search rect are rejected.

Validation: the required affected gate passed with 424/424 files and 4,141/4,141 tests; changed-line coverage is 93/94 (98.94%), with build, package, provider integration, Node integration, progress, and replay-compat checks green. Two workers reproduced the documented replay-scope 5s contention timeout, so the authoritative push gate was rerun serialized and passed.

@thymikee

Copy link
Copy Markdown
Member Author

Exact-head re-review at f9ae545c4: not ready. The new touch-point delta is clean: adaptive parent-edge clearance retains the full child-control buffer, rounded candidates stay inside effective bounds, thin/half-pixel cases and expanded properties are meaningful, competitorCount discloses the total while refs remain capped, and the help/catalog guard is load-bearing.

The Apple text-entry P1 is unchanged—the new commit does not touch that implementation. The authoritative targeted XCTest lane again proves it: testBareDelayedTypeFailsWhenTappedInputDisappearsMidCommand returns success/code nil instead of TEXT_INPUT_NOT_FOCUSED. Penalized coordinate taps can still create a bundle/PID-only unresolved-first-responder witness, and synthesized typing can dispatch after the concrete target disappears or after a non-text tap, potentially reaching a stale responder. Preserve scoped element/responder identity, fail closed when the concrete target is gone, add the negative non-text-coordinate/stale-responder regression, and verify the complete committed value.

The red Coverage job is separate known infrastructure noise: all 6,178 tests passed with no failed assertion before scripts/fuzz/corpus-replay.test.ts hit the Vitest fork teardown timeout/Worker exited unexpectedly; rerun it green. iOS is a substantive code failure and stops before full fixture evidence, so exact-head changed-route device proof is still missing. No ready label applied.

@thymikee

Copy link
Copy Markdown
Member Author

iOS follow-up is pushed.

  • Fixed the coordinate-tap → bare type gap by refusing to mint an unresolved process-scoped text-entry witness; bare typing now requires concrete element identity.
  • Red proof on pre-fix code: the new XCTest failed 4 assertions and typed must-not-type into the stale field.
  • Green proof post-fix: the new non-text coordinate regression, delayed-disappearing-target regression, and hidden-keyboard concrete-target regression all passed (3/3).
  • AGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTS=1 pnpm build:xcuitest:ios passed.
  • Full required local gate passed under Node 24: 61 Node integration tests (53 pass, 8 skipped), 4,138 Vitest tests, 98.94% changed-line coverage.
  • The remaining direct iOS selector/Maestro parent-owned touch-point gap, including selector-first/no-skill users, is tracked in Close parent-owned touch-point gap for direct iOS selectors and Maestro #1718.

Docs/skills were not changed for this bug: the unsafe path was unreleased and removed, while skills remain thin routers to CLI help.

@thymikee
thymikee force-pushed the agent/safer-parent-touch-targets branch from 5a0840b to df7002c Compare August 10, 2026 18:27
@thymikee

Copy link
Copy Markdown
Member Author

Post-rebase validation update: rebased cleanly onto current main, preserving its newer daemon smoke diagnostics and shared Vitest worker cap. The required affected gate passed before the force-push: 63 Node integration tests (55 pass, 8 intentional skips), 4,182 Vitest tests, and 94/95 changed lines covered (98.95%).

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-10 18:43 UTC

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 10, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Exact-head review at df7002ca: clean / ready for human once the two in-progress smoke lanes complete.

The prior bare-type P1 is fixed: a penalized coordinate tap cannot mint an unresolved first-responder witness, and the authoritative iOS lane now executes the live stale-responder/non-text-coordinate regression. Parent-owned touch-point selection/refusal now has complete ADR 0011 classification, runtime selector/ref/drag contracts, and load-bearing geometry/permutation properties; direct iOS/Maestro gaps remain explicitly owned by #1718. All completed checks are green.

Residual note, not a #1700 finding: coordinate fill's unresolved synthesized replacement route predates this branch's base and remains separate technical debt.

@thymikee
thymikee merged commit cdc754e into main Aug 10, 2026
32 of 33 checks passed
@thymikee
thymikee deleted the agent/safer-parent-touch-targets branch August 10, 2026 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant