fix(ios): grant the text-entry commit wait time against progress - #2035
fix(ios): grant the text-entry commit wait time against progress#2035thymikee wants to merge 4 commits into
Conversation
The synthesized commit wait used a flat 3s deadline, which cannot tell a throttled simulator input pipeline (characters keep landing, slowly) from a wedged one (nothing lands) — it condemned both at the same instant and reported TEXT_INPUT_COMMIT_NOT_OBSERVED over a `type`/`fill` that was still working, on branches touching no iOS code. SynthesizedCommitBudget grants time against progress instead: while the observed value's expected-prefix grows — the same length-only evidence logCommitCadence already emits — the wait continues, up to a 10s ceiling. A pipeline making no progress expires at exactly the 3s the flat deadline used, so a wedge is condemned no later than before. It is a reference type, and the observe/expire coupling carries a structural guard, because as a struct that coupling would rest on Swift boxing one captured var and could revert to the flat deadline silently. Text-entry readiness' hardware-keyboard fallback also stops returning a possibly-unfocused element after 0.35s of "no software keyboard seen"; it now returns only on confirmed focus of the target and re-arms otherwise. And the keyboard-hidden precondition of testBareTypeUsesTappedInputWhenSoftwareKeyboardIsHidden skips rather than fails, so an environment flip cannot read as a product regression. The issue's remaining ask — pinning the simulator keyboard preference — is deliberately not done: measured on a dedicated simulator, per-device ConnectHardwareKeyboard makes no difference to a headless `simctl boot`, which always shows the software keyboard. See the PR body for the A/B. Closes #1874
Size Report
npm unpacked components
Startup median (7 runs, lower is better):
Top changed chunks:
Top changed packed files
|
|
The +6.5 kB npm unpacked is the two new Swift files (the whole UITests bundle ships as source); JS shrinks by 216 B. Itemized in the PR body, with why a smaller design was rejected. 🤖 Addressed by Claude Code |
|
Re-reviewed |
Review follow-up. The budget was a detached object tested in isolation, with a TypeScript parser asserting that two escaping Swift closures happened to share it — a guard that only existed because the seam was in the wrong place. The budget is now a local `var` inside `awaitSynthesizedCommitOutcome` and its replacement counterpart, advanced from the same observation the progress check already reads, with the clock injected alongside the existing observation and pacing seams. Recording progress and asking whether time is up are two statements in one loop, so there is no coupling left to guard. The detached tests and the TypeScript wiring guard are deleted. In their place, four sequence tests drive the shipped waits through a hand-driven clock: a prefix that keeps growing outlives the flat 3s deadline, a frozen prefix is condemned at exactly 3s, an indefinitely throttled pipeline stops at the 10s ceiling, and a value churning between two lengths buys no time. Verified red first — the two progress tests fail against a no-op `record`, and the two unchanged-behavior tests stay green.
|
Addressed in The budget is now a local In their place, four sequence tests drive the shipped waits through a hand-driven clock: a growing prefix outlives the flat 3s deadline (settles at t=8s), a frozen prefix is condemned at exactly 3s, an indefinitely throttled pipeline stops at the 10s ceiling, and a value churning between two lengths buys no time. Red-first verified: the two progress tests fail against a no-op Re-validated on iPhone 17 Pro / iOS 26.2: the full 🤖 Addressed by Claude Code |
thymikee
added
the
ready-for-human
|
Re-review at |
The synthesized commit wait started its clock before reading the field's placeholder, and that read is an AX round-trip which takes seconds on exactly the loaded host this budget exists for. Slow setup therefore spent the budget: with a 3.5s placeholder read the first observation already exceeded the 3s stall budget, so `type` reported TEXT_INPUT_COMMIT_NOT_OBSERVED after a single poll — sooner than the flat deadline this replaced, in the one condition it was written for. The budget is now two durations, and only the poll loop starts it, from its own first `now()`. Passing a pre-loop timestamp is no longer expressible. The poll also takes one clock sample instead of two, so the instant an observation is recorded at is the instant it is judged against. testCommitWaitBudgetStartsAtTheLoopRatherThanBeforeIt pins it: 60s of setup before the wait must still leave the full stall budget. Verified red against a deadline started outside the loop.
|
Max-effort review of the branch found one real bug, now fixed in The budget clock started before the wait, not at it. The budget is now two durations, and only the poll loop starts it, from its own first Two smaller ones fixed alongside: the poll took two clock samples where one was meant, and Re-validated: policy set 28 passed, full 🤖 Addressed by Claude Code |
…very fill The `smoke:form-input` half of #1874 is not the commit deadline. This PR's own iOS lane reproduced it (run 32889322172) and the trace settles it: `wait start expectedLen=12`, then zero `[DEBUG-1874] poll` lines, then `wait outcome=notObserved elapsedMs=3608`. The wait never polled — it returned from the `textMatchesPlaceholder` guard, which refuses before polling because an empty text field renders its placeholder AS its accessibility value, so a match cannot prove a commit. `field-name`'s placeholder was "Ada Lovelace" and every checkout-form suite fills exactly "Ada Lovelace"; `field-email` had the same collision with "ada@example.com". Twelve fills across eight files, so `fill` into those fields is unverifiable by contract. It looked intermittent only because the synthesized-replacement route is gated on `xCTestChannelPenalized` — it fires when the host is loaded — which is also why re-running a failed job on the same commit reproduced it identically. The collision also made the read-back assertions vacuous: `assertJsonContains( name, 'Ada Lovelace')` is satisfied by an empty field rendering the placeholder. Fixed in the fixture rather than in the values, because frozen replay-compat corpora carry the same fills and must not be edited. fixture-fill-placeholder-collision.test.ts guards the class: it fails on any repository fill whose value equals the target field's placeholder.
|
The max-effort review turned up something bigger than the three findings I posted, and it came from this PR's own failing iOS lane. The Zero poll lines. The wait never entered its loop — it returned from the It looked intermittent because the synthesized-replacement route is gated on It also made the read-back assertions vacuous: Fixed in Residual, flagged not fixed: 🤖 Addressed by Claude Code |
thymikee
removed
the
ready-for-human
|
[P2] Remove Current iOS smoke is still running; this is a code-quality blocker independent of that result. |
1 participant
Summary
typeandfillon iOS stop reportingTEXT_INPUT_COMMIT_NOT_OBSERVEDover text that is stilllanding. The synthesized commit wait used a flat 3s deadline, which cannot tell a throttled
simulator input pipeline (characters keep arriving, slowly) from a wedged one (nothing arrives) —
so it condemned both at the same instant. That is what turned a simulator-load episode into a red
Smoke Tests (iOS simulator)/ targeted-XCTest lane on branches touching no iOS code.SynthesizedCommitBudget). The observedvalue's expected-prefix length — the same length-only evidence
logCommitCadencealready emits —decides: while it grows, the wait continues, up to a 10s ceiling. A pipeline making no progress
expires at exactly the 3s the flat deadline used, so nothing that fails today starts passing
merely by waiting longer, and a wedge is condemned no later than before.
waitForTextEntryReadiness'shardware-keyboard fallback returned a possibly-unfocused element after 0.35s of "no software
keyboard seen". It now returns only when the target element itself confirms keyboard focus, and
re-arms that window otherwise; unconfirmed, the wait runs its normal readiness timeout — what it
would have spent with no fallback at all.
testBareTypeUsesTappedInputWhenSoftwareKeyboardIsHiddenis now
XCTSkipIf, notXCTAssertFalse. An environment flip must not read as a productregression on an unrelated PR.
Closes #1874.
Two corrections to the issue's diagnosis
Neither observed failure reaches
waitForTextEntryReadiness:407. Tracing both paths:testBareTypeUsesTappedInputWhenSoftwareKeyboardIsHiddentypes through the tap witness, andrememberedTextEntryTarget()returns before readiness is ever consulted.smoke:form-input→fill full namenever reaches readiness either: it fails insiderunSynthesizedReplacementRoute, whichshouldUseSynthesizedFirstResponderReplacementgates onhasResolvedElement == false— the penalized coordinate route builds that target directly inexecuteTypeCommand, bypassingfocusTextInputForTextEntryentirely.And the smoke half is not the deadline either — it is the fixture. This PR's own iOS lane
reproduced it (run 32889322172),
and the trace settles it:
The wait never polled. It returned from the
textMatchesPlaceholderguard, which refuses beforepolling because an empty text field renders its placeholder as its accessibility value, so a match
cannot prove a commit.
expectedLen=12is"Ada Lovelace"— andfield-name's placeholder was"Ada Lovelace".field-emailhad the same collision withada@example.com: twelve fills acrosseight files, every checkout-form flow in the repository. The 3.6s is just the AX resolution for the
placeholder read that precedes the guard.
It looked intermittent because the synthesized-replacement route is gated on
xCTestChannelPenalized— it fires when the host is loaded. That also explains the datapoint theissue found most puzzling:
rerun_failed_jobson the same commit reproducing identically. Adeterministic guard on a load-gated route looks exactly like a sticky per-device flake.
The collision additionally made the read-back assertions vacuous —
assertJsonContains(name, 'Ada Lovelace')is satisfied by an empty field rendering the placeholder — so fixing it strengthens thesmoke rather than just quieting it. Fixed in the fixture, not in the values, because frozen
replay-compat corpora carry the same fills;
fixture-fill-placeholder-collision.test.tsfails onany repository fill whose value equals its field's placeholder.
So the three changes address three different things: the budget is what the targeted XCTest lane
needs, the fixture is what the smoke lane needs, and the readiness fix closes the latent
sensitivity the issue describes without being what fired in either.
Pinning the simulator keyboard preference would have been a no-op, so this PR does not do it.
Measured on a dedicated simulator, headless
simctl boot, iOS 26.2, then deleted:ConnectHardwareKeyboardtrue(hardware keyboard "connected")falsecom.apple.iphonesimulatoris Simulator.app's preference domain; with no Simulator.app running,CoreSimulator does not consult it. Writing it would have added a step, a guard test and a green
assertion that proved only that
defaults writeran. The useful half of that finding is thereassuring one: a headless lane has no hardware keyboard to inherit, so the software keyboard
always shows and the ambient state the issue names is not a variable in these lanes at all — which
independently agrees with the 2026-08-21 diagnosis comment.
Validation
Local, iPhone 17 Pro / iOS 26.2 simulator, arm64 slice. The Swift tree is byte-identical to the
validated one —
mainmoved under this branch mid-work (#2021) and the rebase touched nothingunder
apple/runner/, and the static gates were re-run onba7e03f885after it:ios.ymlPR-only-testing:list: 75 passed, 0 failures.SynthesizedCommitBudget.recordto a no-op turns
testCommitWaitOutlivesTheFlatDeadlineWhileTheExpectedPrefixGrowsandtestCommitWaitCeilingStopsAnIndefinitelyThrottledPipelinered, whiletestCommitWaitCondemnsAFrozenPipelineAtTheStallBudgetandtestCommitWaitTreatsARetreatingValueAsNoProgressstay green — they encode the behavior thatmust not move.
testHardwareKeyboardResponderConfirmsItsOwnKeyboardFocuswas written red first and caught awrong implementation:
XCUIElement.descendants(matching:)does not include the receiver, soan element-rooted focus query answers
0for a leaf text field. Instrumented on-device(
selfDescendants=0,appFocused=1naming the tapped field atkeyboardVisible=0) and rebuiltagainst the app-wide predicate with an identity check. That XCTest behavior is now pinned by the
test rather than assumed.
com.apple.iphonesimulatorrestored from apre-probe export afterwards, agent-device sessions closed.
that now covers it was planted-red: before the fixture change it lists all twelve collisions.
Not verified locally: the macOS host XCTest lane, blocked on this host by the known
Accessibility/Automation permission gap. The budget tests are platform-neutral and were executed on
the simulator instead, where the same
#if AGENT_DEVICE_RUNNER_UNIT_TESTSbundle runs them. CI onthe head is the authority for the host and smoke lanes.
Unrelated pre-existing red on
mainat the same commit, not introduced here:cli-help-topics.test.ts/cli-help.test.tsfail because theworkflowhelp card is 9003 bytesagainst a 9000-byte cap.
Design notes and residual risk
awaitSynthesizedCommitOutcomeand its replacementcounterpart hold it as a local
varand advance it from the same observation the progress checkalready reads, with the clock injected alongside the existing observation and pacing seams. An
earlier revision made it a detached object shared between two escaping closures and guarded that
sharing with a TypeScript parser; the guard was evidence the seam was wrong, so both are gone.
What replaced them is four sequence tests that drive the real waits.
fillcan now take up to 10s instead of 3s when the value keeps converging withoutever matching (a formatter rewriting input, say). Bounded well inside the daemon's 45s
per-command budget. A dropped-character corruption still fails at ~3s, because its prefix freezes.
fillstill cannot verify a value equal to the field's placeholder, and that asymmetry isroute-dependent: the ordinary
xctest-elementroute reads the raw value back and passes, whilethe penalized route refuses. Both are right on the evidence each has, but the same command can
therefore report differently under load. Worth its own issue against fix(ios): report an unobserved text commit instead of a partial success #1924's three-state
PlaceholderCommitEvidencemodel; not expanded into here.2026-08-21 comment would still exhaust the ceiling and report honestly. Why the simulator input
pipeline throttles at all is unchanged by this PR; the
[DEBUG-1874]cadence traces stay in placefor it.
Size: the growth is Apple runner source, because the whole
AgentDeviceRunnerUITestsbundleships as source; JS shrinks.
SynthesizedCommitBudgetkeeps its own file only to holdRunnerTests+SynthesizedTextEntry.swiftinside the 500-line extraction rule — folding it backwould save nothing packed. The review rework removed the detached test file and the TypeScript
guard, so the final figure is below what the last size report measured.
8 files: the iOS text-entry path, the form fixture's two placeholders, and one guard. No docs change:
TEXT_INPUT_COMMIT_NOT_OBSERVED's meaning and recovery advice inwebsite/docs/docs/commands.mdare unchanged — only how long the runner looks before reporting it.