Skip to content

fix(ios): harden Bluesky-class interaction reliability - #1588

Merged
thymikee merged 13 commits into
mainfrom
codex/benchmark-bluesky-reliability
Aug 5, 2026
Merged

fix(ios): harden Bluesky-class interaction reliability#1588
thymikee merged 13 commits into
mainfrom
codex/benchmark-bluesky-reliability

Conversation

@thymikee

@thymikee thymikee commented Aug 4, 2026

Copy link
Copy Markdown
Member

Summary

  • keep iOS type and fill usable when React Native's accessibility tree is hostile by routing focused text entry through AX-independent runner synthesis, while retaining the verified XCTest path for healthy apps
  • stop scroll containers from stealing taps intended for their actionable descendants
  • preserve Maestro replay's non-hittable fill behavior after ordinary selector fill moved to shared runtime resolution
  • teach small-model workflows that type does not accept --settle and that an explicit success state is the end of the task

The fixes come from rerunning Software Mansion's AppControlBench Bluesky suite on agent-device 0.20.5. They address three concrete failure classes seen in the benchmark: 20–30s XCTest text-entry stalls, ancestor promotion that redirected child taps to list containers, and post-success agent actions that destroyed otherwise judgeable final state.

The private-AX/settle performance work from the same investigation is in #1587. Landing order: merge #1587 first, then rebase this PR onto it before merge so the shared channel-penalty and settle behavior has one final source of truth.

This PR touches the iOS runner, interaction targeting, typed runner evidence, live simulator coverage, and CLI help/conformance. The scope crosses command families because the benchmark exposed independent type, fill, and tap reliability failures that share the same Bluesky-class AX environment.

Privacy remains explicit on the private synthesis path: typeText:atOffset:typingSpeed:shouldRedact: is invoked with shouldRedact: YES, and diagnostics emit only route, phase, timing, character count, and mode—not entered text.

Validation

Regression tests were mechanically proven red:

  • the tap policy produced six failures before ancestor promotion was constrained
  • healthy-route gating failed before synthesis was limited to XCTest-penalized apps
  • private synthesis unavailable/failed disposition and coordinate-focus fallback tests failed under their counterfactual mappings
  • reverting the real typeTextReliably caller to one-shot replacement produced [replace("abc")] instead of [replace("a"), append("b"), append("c")]; the restored production caller passes
  • the Maestro contract now proves a non-hittable selector fill resolves in the runtime, forwards runner-only fallback permission, reports that the fallback was used, and omits inapplicable resolution disclosure

Live iOS simulator evidence:

  • focused type on authenticated Bluesky completed its runner text work in roughly 0.5–0.8s instead of stalling for about 20s
  • semantic fill replaced the exact focused composer text in 2.1s end to end
  • GPT-5.4-mini low effort published the exact post hello from the benchmark; the final screenshot showed it at the top of Following (227.8s, 23 tool calls)
  • a healthy Settings fill retained the focus/type/verify route; no synthesized route was logged

Final verification for the review fixes:

  • focused TypeScript and interaction-contract suites: 168/168 passed
  • focused production-caller XCTest: passed; its one-shot counterfactual failed as described above
  • iOS runner build: passed
  • pnpm check:affected --run: passed with Vitest serialized to one worker after the parallel run showed unrelated host-contention timeouts; 3,469 affected tests passed and changed-line coverage was 100%

The iOS smoke test now reads the authoritative runnerLogPath returned by open, rather than reconstructing a state-directory path.

The public CLI help and deterministic help-conformance corpus were updated. Skills were not changed because they intentionally route to versioned CLI help rather than duplicating command behavior.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.96 MB 1.96 MB +1.2 kB
JS gzip 627.1 kB 627.3 kB +259 B
npm tarball 749.3 kB 753.4 kB +4.1 kB
npm unpacked 2.62 MB 2.65 MB +21.4 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 30.3 ms 32.8 ms +2.5 ms
CLI --help 67.2 ms 70.0 ms +2.8 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/cli-help.js +546 B +100 B
dist/src/viewport-dimension.js +245 B +65 B
dist/src/context.js +263 B +60 B
dist/src/runtime.js +128 B +46 B
dist/src/interaction.js -132 B -39 B

@thymikee

thymikee commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Review of de23cf1: not ready.

[P1] Preserve --delay-ms on the new AX-independent replacement route. typeTextReliably enters the synthesized first-responder replacement branch, sends the entire string through one replaceText event, and returns before the existing per-character delaySeconds loop. That breaks the documented “delay between typed characters” contract used by debounced/search-as-you-type fields. Pace the synthesized route per character (with replacement/select-all only once) or restrict the fast route to zero delay, and add a counterfactual-red regression.

[P1] Cover the normal simple-selector fill route before claiming semantic iOS fill reliability. Default fill id=... / label=... / text=... / value=... requests without verify/settle still take maybeDispatchDirectIosSelectorFillfillElementSelector, which sends a runner type command without resolvedTextInputTarget; it therefore remains on the old AX-dependent element-resolution path. Deliberately route/disable/handle that fast path and pin its production dispatch behavior.

The focused runtime evidence for refs/coordinate resolution is useful, and static/type/Swift gates are green; several exact-head CI/device lanes were still running during review. No fix work was performed.

@thymikee

thymikee commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Addressed both P1 findings in b588a48 (with the implementation split across 62b47bc, b756f8c, and b588a48):

  • --delay-ms is preserved on the AX-independent synthesized replacement path. Delayed multi-character replacement is now planned as replace/select-all for the first character followed by append steps, with the requested delay between steps. The zero-delay fast path remains a single replacement event.
  • Default iOS selector fill no longer bypasses runtime resolution. I removed fill from the direct selector route, so selector fills capture and resolve through the shared runtime before sending coordinate typing with typed resolvedTextInputTarget evidence. The interaction-guarantee matrix and stale direct-fill expectations were updated accordingly.

Red → green evidence:

  • Selector regression test on the pre-fix path failed with expected ios.runner.snapshot, received ios.runner.type; it now passes and asserts runtime snapshot resolution plus resolvedTextInputTarget: true.
  • Swift policy regression test on the pre-fix planner failed with actual [replace("abc")] versus expected [replace("a"), append("b"), append("c")]; it now passes.

Verification:

  • Focused interaction contract suites: 33/33 passed.
  • Focused handler regression test passed.
  • iOS synthesized-replacement policy test passed on simulator.
  • Live Bluesky check: filled Search with delay-check using --delay-ms 50; the response reported resolution.source=runtime, and readback returned exactly delay-check.
  • pnpm check:affected --run passed before push.

@thymikee

thymikee commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Re-review of b588a48: not ready.

[P1] The cutover of ordinary simple selector fill to the runtime accidentally removes the Maestro replay non-hittable fill path: maestro-non-hittable-fallback still declares commands: ['press', 'fill'], the fill contract cases were deleted, and the runtime fill route does not carry allowNonHittableCoordinateFallback. Preserve equivalent Maestro-only semantics (direct or runtime) and pin the matrix behavior.

[P1] Delayed type synthesis is fixed in production, but the new regression exercises only synthesizedReplacementSteps; reverting the real typeTextReliably caller to its former one-shot behavior still passes. Add a production-caller regression/counterfactual or exact live evidence that goes red under that revert.

Owner-action CI: exact-head iOS Smoke is red because the live E2E reads a newly assumed runner.log path and gets ENOENT; use the authoritative returned/log-store path and rerun.

Ordinary selector fill otherwise correctly moves to the runtime.

@thymikee

thymikee commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Addressed the re-review findings in 7922ba614.

  • Maestro non-hittable fill is preserved through the runtime cutover. Runtime resolution now forwards allowNonHittableCoordinateFallback only when the resolved node is actually non-hittable. The Apple runner reports the structured maestroNonHittableCoordinateFallbackUsed result, and the canonical response builder retains the Maestro allowed/used/reason fields while omitting resolution disclosure when that fallback executed. The interaction contract pins snapshot → runtime resolution → coordinate type, the runner permission, targetHittable: false, and the fallback response markers.
  • Delayed synthesis now has a production-caller regression. typeTextReliably uses a real TextEntrySynthesizing boundary; the test invokes that production function and records replace a, append b, append c. Counterfactual evidence: temporarily reverting the caller to one-shot replacement failed at RunnerTests+TextEntryPolicyTests.swift:147, with actual [replace("abc")] versus expected [replace("a"), append("b"), append("c")]. Restored run passed.
  • iOS Smoke uses the authoritative log path. The cold-launch open response's runnerLogPath is retained in the live context and used for the route assertion; the reconstructed stateDir/sessions/<session>/runner.log assumption is gone.

Additional review fixes in the same commit keep synthesized replacement restricted to XCTest-penalized apps, distinguish private-synthesis unavailable (fallback) from genuine failure (raise), preserve coordinate-focus fallback, cover mislabeled inputs on penalized screens, widen the rect property generator, and accept zero-sized runner reference frames. Private synthesized typing passes shouldRedact: YES.

Verification:

  • focused TS/contract suites: 168/168 passed
  • focused production-caller XCTest: green after the counterfactual-red proof
  • iOS runner build: passed
  • required pnpm check:affected --run && git push: passed serialized after the parallel attempt exposed unrelated host-contention timeouts; 3,469 affected tests passed, changed-line coverage 100%

The PR body now also records that #1587 should land first and this PR should be rebased onto it before merge.

@thymikee

thymikee commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Re-review of 7922ba61: prior code findings are addressed. Maestro non-hittable fill is restored through the production runtime path and contract-pinned; delayed synthesis now tests the real typeTextReliably caller with a meaningful one-shot counterfactual; and the code otherwise looks clean. Not ready yet: the branch does not contain merged prerequisite #1587 (8d526f2), so rebase/resync onto current main and re-review the changed head. Exact-head iOS Smoke is also red: open does not return runnerLogPath; it returns authoritative sessionStateDir, and the new assertion fails before the scenario. Use the session-store-provided response path (or deliberately add a public runner-log path contract if that is the intended product change), then rerun after rebase. Residual risk: no separately authorized cross-vendor review was run.

@thymikee
thymikee force-pushed the codex/benchmark-bluesky-reliability branch from 7922ba6 to 535d08b Compare August 4, 2026 16:43
@thymikee

thymikee commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Follow-up after the size/duplication audit and #1587 landing:

  • Rebasing onto merged perf(ios): halve hostile-screen capture cost under the XCTest-channel penalty #1587 changed the review-fix commit to 658c4c0.
  • Added 535d08b to remove the duplicated Swift mirror of the Objective-C synthesis status/attempt model and isolate the private-XCTest boundary in RunnerTests+SynthesizedTextEntry.swift.
  • Consolidated the synthesizer protocol to one operation and table-driven the repeated policy assertions.
  • RunnerTests+TextEntry.swift is now 440 lines and RunnerTests+TextTyping.swift 497 lines, both below the 500-line tripwire.
  • The raw GitHub count treats the TextEntry split as new code. With copy/move detection the full PR is +1,151/-1,170 (net -19), so the apparent large growth is primarily diff accounting, not duplicated implementation.

Verification on the rebased head:

  • iOS runner build succeeded.
  • Six focused synthesized-entry policy/production-caller XCTest cases passed.
  • pnpm check:affected --run: all runnable checks passed; 366 files / 3,470 tests green; changed-line coverage 100%.

@thymikee

thymikee commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Re-review at b9a134176: the prior runnerLogPath smoke blocker is correctly addressed—apps.open() now preserves the daemon's declared runner/request diagnostic paths and the client test pins them. A new exact-head owner-action blocker remains: Build npm package rejects the unguarded testSynthesizedReplacementRequiresPenalizedXCTestAndCoordinates() in RunnerTests+TextEntryPolicyTests.swift, which would ship as runner source. Wrap the runner unit-test entrypoints with #if AGENT_DEVICE_RUNNER_UNIT_TESTS per #1594, then rerun package and iOS smoke. Do not apply ready-for-human while this check is red.

@thymikee

thymikee commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Re-review at 9200680f5: the runnerLogPath/client fix remains sound and structured text-entry route propagation is coherent, but Build npm package is deterministically blocked. RunnerTests+TextEntryPolicyTests.swift contains four unguarded XCTest entrypoints (the synthesized replacement policy tests plus the two coordinate tests); the packaging gate stops on the first. Wrap the complete test-only extension/content with #if AGENT_DEVICE_RUNNER_UNIT_TESTS (nesting the platform guard as needed), then rerun package and iOS smoke. This is owner action, not infrastructure.

@thymikee

thymikee commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

CI follow-up is complete and the final head is green.

Root causes fixed:

  • b9a134176 preserves runnerLogPath / requestLogPath in the SDK open response.
  • 9200680f5 replaces the invalid assumption that every session owns a runner log with structured textEntryRoute diagnostics propagated from the Swift runner. The live iOS E2E now asserts synthesized-first-responder directly.
  • 6dfd91a70 guards the new Swift unit test with AGENT_DEVICE_RUNNER_UNIT_TESTS, so it executes in the unit-test build but is stripped from the npm runner payload.

Verification:

The unrelated Android viewport flake passed on rerun. All PR checks are now green.

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

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Final review at 6dfd91a7096e38cad93321a94595651431241acc: clean and ready for human review. #1587 (8d526f2) is in this head and the branch is mergeable/CLEAN. The prior diagnostic-path, production-route, delayed-synthesis, Maestro fallback, and packaging findings are resolved; the Swift policy tests are stripped behind AGENT_DEVICE_RUNNER_UNIT_TESTS. All exact-head checks pass, including Build npm package and fixture-backed iOS smoke. Required simulator evidence is adequate. Residual risk only: the Bluesky benchmark run is author-supplied rather than independently reproduced here; no actionable findings.

@thymikee
thymikee merged commit 6118581 into main Aug 5, 2026
31 of 32 checks passed
@thymikee
thymikee deleted the codex/benchmark-bluesky-reliability branch August 5, 2026 06:02
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-05 06:02 UTC

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