From 16e5b9cb137044f3a649bab4ba06e22d5f467590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 25 Aug 2026 17:24:44 +0200 Subject: [PATCH 1/7] fix(ios): grant the text-entry commit wait time against progress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Refs #1874 — not a closing keyword on purpose. This is a mitigation; the unidentified simulator input-throttle mechanism that issue tracks is untouched here, so it stays open. --- .github/workflows/ios.yml | 1 + .../RunnerTests+CommandExecution.swift | 58 ++++++++++++++- .../RunnerTests+SynthesizedCommitBudget.swift | 59 ++++++++++++++++ .../RunnerTests+SynthesizedTextEntry.swift | 15 +++- .../RunnerTests+TextEntry.swift | 61 ++++++++++++++-- ...erTests+SynthesizedCommitBudgetTests.swift | 70 +++++++++++++++++++ .../RunnerTests+TextEntryPolicyTests.swift | 12 ++-- .../apple-runner-commit-budget-wiring.test.ts | 61 ++++++++++++++++ 8 files changed, 323 insertions(+), 14 deletions(-) create mode 100644 apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift create mode 100644 apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SynthesizedCommitBudgetTests.swift create mode 100644 src/__tests__/apple-runner-commit-budget-wiring.test.ts diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index e0d87ed6d..b0726e022 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -140,6 +140,7 @@ jobs: -only-testing:AgentDeviceRunnerUITests/RunnerTests/testSinglePointerFlingFallsBackToXCTestCoordinateDragWhenPrivateSynthesisFails \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testTypeWithoutResolvedInputReturnsTypedFailureBeforeDispatchingText \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testBareTypeUsesTappedInputWhenSoftwareKeyboardIsHidden \ + -only-testing:AgentDeviceRunnerUITests/RunnerTests/testHardwareKeyboardResponderConfirmsItsOwnKeyboardFocus \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testPenalizedCoordinateTapOnNonTextControlDoesNotAuthorizeBareType \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testBareDelayedTypeFailsWhenTappedInputDisappearsMidCommand \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testSynthesizedTextCommitProgressWalksExpectedPrefixOnly \ diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift index ae140fc47..5617f5516 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift @@ -379,9 +379,13 @@ extension RunnerTests { ) let tapResponse = try executeOnMainPrepared(command: tapCommand, activeApp: app) XCTAssertTrue(tapResponse.ok, String(describing: tapResponse.error)) - XCTAssertFalse( + // A precondition, not a product claim. The fixture's empty `inputView` is what keeps the + // keyboard down, but nothing in this bundle owns the simulator's own keyboard settings, so an + // ambient flip that raised one here would be an environment fact — and reporting it as a + // failed assertion is what made this read as a product regression on unrelated PRs (#1874). + try XCTSkipIf( isKeyboardVisible(app: app), - "the test must exercise a focused responder with the software keyboard hidden" + "software keyboard is up: this simulator cannot exercise the hidden-keyboard responder path" ) let failureCountBefore = currentXCTestFailureCount() @@ -407,6 +411,56 @@ extension RunnerTests { XCTAssertEqual(String(describing: textField.value ?? ""), "hardware-keyboard") } + // `waitForTextEntryReadiness`'s hardware-keyboard fallback returns early only on confirmed + // focus (#1874), and `keyboardFocusConfirmed` reads that from the app-wide focus predicate this + // bundle otherwise refuses to trust. Two XCTest facts it rests on, neither a repository + // invariant: the predicate reports a responder that shows NO software keyboard at all, and it + // names the element well enough to tell the tapped field from another one. The fixture field is + // the exact shape the fallback exists for — a real responder with an empty `inputView` — so this + // is where both are observable. If either regressed, readiness would silently stop taking the + // fallback and spend the full readinessTimeout on every hardware-keyboard field, which no other + // assertion would notice. + func testHardwareKeyboardResponderConfirmsItsOwnKeyboardFocus() throws { + app.launchArguments = ["--agent-device-text-entry-regression"] + app.launch() + defer { + invalidateCachedTarget(reason: "unit_test_cleanup") + app.terminate() + } + XCTAssertTrue(app.waitForExistence(timeout: appExistenceTimeout)) + + let textField = app.textFields["agent-device-hardware-keyboard-input"] + XCTAssertTrue(textField.waitForExistence(timeout: appExistenceTimeout)) + let otherElement = app.staticTexts["Agent Device Runner"] + XCTAssertTrue(otherElement.waitForExistence(timeout: appExistenceTimeout)) + XCTAssertFalse( + keyboardFocusConfirmed(app: app, element: textField), + "an untapped field must not confirm focus, or the fallback would fire immediately" + ) + + let tapCommand = try runnerCommandFixture( + #"{"command":"tap","commandId":"tap-focus-confirmation","selectorKey":"id","selectorValue":"agent-device-hardware-keyboard-input"}"# + ) + let tapResponse = try executeOnMainPrepared(command: tapCommand, activeApp: app) + XCTAssertTrue(tapResponse.ok, String(describing: tapResponse.error)) + try XCTSkipIf( + isKeyboardVisible(app: app), + "software keyboard is up: this simulator cannot exercise the hidden-keyboard responder path" + ) + + let deadline = Date().addingTimeInterval(TextEntryTiming.readinessTimeout) + var confirmed = keyboardFocusConfirmed(app: app, element: textField) + while !confirmed && Date() < deadline { + sleepFor(TextEntryTiming.pollInterval) + confirmed = keyboardFocusConfirmed(app: app, element: textField) + } + XCTAssertTrue(confirmed, "a tapped responder must confirm its own keyboard focus") + XCTAssertFalse( + keyboardFocusConfirmed(app: app, element: otherElement), + "focus held by another element must read as a refusal, never as this element's focus" + ) + } + func testBareDelayedTypeFailsWhenTappedInputDisappearsMidCommand() throws { app.launchArguments = [ "--agent-device-text-entry-regression", diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift new file mode 100644 index 000000000..b0fc51402 --- /dev/null +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift @@ -0,0 +1,59 @@ +import XCTest + +// How long the synthesized text-entry commit wait is willing to keep looking. Split out of +// RunnerTests+SynthesizedTextEntry.swift so the policy is a pure, clock-injected value type the +// macOS host lane can exercise without a simulator. +extension RunnerTests { + /// The commit wait's deadline. + /// + /// A synthesized burst can be *throttled* rather than dropped: on a loaded simulator the + /// characters keep landing, just slowly, and everything touching the input system slows with + /// them (#1874). A flat wall-clock budget cannot tell that apart from a wedged pipeline, so it + /// condemned both at the same instant — and the throttled case is a working command reported as + /// `TEXT_INPUT_COMMIT_NOT_OBSERVED`, which is what turned an environment episode into a red + /// lane on branches touching no iOS code. + /// + /// So time is granted against *progress* — the observed value's expected-prefix growing, the + /// same length-only evidence `logCommitCadence` already emits — with an absolute ceiling, so a + /// pipeline that delivers one character per stall window cannot hold a command open forever. + /// + /// A pipeline making no progress at all still expires at exactly the `stallBudget` the flat + /// deadline used, so nothing that fails today starts passing merely by waiting longer: the wait + /// extends only while characters are still arriving. + /// + /// A reference type on purpose. Its two readers are separate escaping closures — the commit + /// wait's `observe` records into it, its `isExpired` reads it — and the whole fix is the + /// coupling between them. As a struct that coupling rests on Swift boxing one captured `var`, + /// which a later refactor could quietly break back into the flat deadline with every test still + /// green. Sharing one instance makes that unrepresentable instead of merely true today. + final class SynthesizedCommitBudget { + let startedAt: Date + let stallBudget: TimeInterval + let ceiling: TimeInterval + private var bestPrefixLength: Int + private var lastProgressAt: Date + + init(startedAt: Date, stallBudget: TimeInterval, ceiling: TimeInterval) { + self.startedAt = startedAt + self.stallBudget = stallBudget + self.ceiling = ceiling + // Nothing has landed yet, and an observation of "still nothing" must not read as progress. + self.bestPrefixLength = 0 + self.lastProgressAt = startedAt + } + + /// Records one observation's expected-prefix length. Only forward movement counts: a shorter + /// read (the app clearing the field mid-flight, an unreadable poll reporting -1) is not + /// evidence the burst is still landing, so it neither buys time nor takes any back. + func record(expectedPrefixLength: Int, at now: Date) { + guard expectedPrefixLength > bestPrefixLength else { return } + bestPrefixLength = expectedPrefixLength + lastProgressAt = now + } + + func isExpired(at now: Date) -> Bool { + now.timeIntervalSince(startedAt) >= ceiling + || now.timeIntervalSince(lastProgressAt) >= stallBudget + } + } +} diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift index 0b75db4d3..3dc3678b2 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift @@ -360,23 +360,32 @@ extension RunnerTests { waitForNextObservation: () -> Void ) { let placeholder = resolveTextEntryElement(app: app, target: target)?.placeholderValue - let deadline = Date().addingTimeInterval(TextEntryTiming.synthesizedCommitTimeout) let waitStartedAt = Date() + // Throttling and a wedge are the same to a flat deadline, so the expected-prefix walk decides + // instead of the wall clock alone — the very distinction the cadence line below was added to + // make visible. See `SynthesizedCommitBudget`. + let budget = SynthesizedCommitBudget( + startedAt: waitStartedAt, + stallBudget: TextEntryTiming.synthesizedCommitStallTimeout, + ceiling: TextEntryTiming.synthesizedCommitCeiling + ) return ( placeholder: placeholder, - isExpired: { Date() >= deadline }, + isExpired: { budget.isExpired(at: Date()) }, observe: { let observedText = self.editableTextValue( for: self.resolveTextEntryElement(app: app, target: target), treatingPlaceholderAsEmpty: true ) + let expectedPrefixLen = observedText.map { Self.commonPrefixLength($0, expectedText) } ?? -1 + budget.record(expectedPrefixLength: expectedPrefixLen, at: Date()) // Cadence evidence stays value-free: the polled value is user content typed through // `type`/`fill` and must never reach runner.log. Lengths and the expected-prefix walk // are enough to distinguish throttling (prefix grows slowly) from a wedge (it freezes). Self.logCommitCadence( elapsedMs: Int(waitStartedAt.timeIntervalSinceNow * -1000), observedLen: observedText?.count ?? -1, - expectedPrefixLen: observedText.map { Self.commonPrefixLength($0, expectedText) } ?? -1 + expectedPrefixLen: expectedPrefixLen ) return observedText }, diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift index d4a849343..b46a1c163 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift @@ -45,7 +45,14 @@ extension RunnerTests { static let pollInterval: TimeInterval = 0.02 static let warmupValueTimeout: TimeInterval = 0.4 static let verificationStabilityWindow: TimeInterval = 0.2 - static let synthesizedCommitTimeout: TimeInterval = 3.0 + /// How long the commit wait tolerates seeing NO further progress toward the expected value. + /// Numerically the flat deadline this replaced, so a pipeline that delivers nothing is + /// condemned at exactly the same instant it always was (see `SynthesizedCommitBudget`). + static let synthesizedCommitStallTimeout: TimeInterval = 3.0 + /// The commit wait's absolute bound, however long characters keep arriving. Sits well inside + /// the daemon's per-command budget (`RUNNER_COMMAND_TIMEOUT_MS`, 45s), which also has to cover + /// focus, clear and verification around this wait. + static let synthesizedCommitCeiling: TimeInterval = 10.0 static let synthesizedCommitPollInterval: TimeInterval = 0.2 } @@ -384,7 +391,7 @@ extension RunnerTests { var latest = resolveTextEntryElement(app: app, target: target) let keyboardVisibleAtEntry = isKeyboardVisible(app: app) let deadline = Date().addingTimeInterval(timeout) - let hardwareKeyboardFallback = Date().addingTimeInterval( + var hardwareKeyboardFallback = Date().addingTimeInterval( min(TextEntryTiming.hardwareKeyboardFallbackTimeout, timeout) ) var sawSoftwareKeyboard = false @@ -404,8 +411,19 @@ extension RunnerTests { return latest } sawSoftwareKeyboard = sawSoftwareKeyboard || keyboardElementExists(app: app) - if !sawSoftwareKeyboard && Date() >= hardwareKeyboardFallback && latest != nil { - return latest + // A responder that takes no software keyboard (hardware keyboard connected, or a custom + // `inputView`) would otherwise burn the whole readinessTimeout waiting for one that is never + // coming. Leaving that window a bare wall-clock guess made readiness a function of ambient + // simulator state (#1874): on a loaded host the keyboard is merely late, and returning here + // handed the caller an element that had not taken focus yet. Ask the target itself instead, + // and re-arm rather than re-asking every poll — the query is cheap, not free. + if !sawSoftwareKeyboard, Date() >= hardwareKeyboardFallback, let candidate = latest { + if keyboardFocusConfirmed(app: app, element: candidate) { + return candidate + } + hardwareKeyboardFallback = Date().addingTimeInterval( + TextEntryTiming.hardwareKeyboardFallbackTimeout + ) } sleepFor(TextEntryTiming.pollInterval) } @@ -464,6 +482,41 @@ extension RunnerTests { return !wasVisibleAtEntry && isKeyboardVisible(app: app) } + /// Positive evidence that this element — the one readiness is about to hand its caller — holds + /// keyboard focus. Readable even with no software keyboard on screen, which is what lets the + /// hardware-keyboard fallback stop guessing from a wall clock. + /// + /// This is the same app-wide predicate `focusedTextInput` refuses to trust on iOS, used the + /// other way round. There, the query PICKS the target, so a stale or unrelated match becomes + /// the field that gets typed into. Here the target is already chosen and the query only + /// corroborates it: at most one element holds keyboard focus, so an answer that is not this + /// element is a refusal, not a substitution. Every way of being wrong therefore ends as `false` + /// and costs the remaining readiness timeout — exactly what the wait would spend with no + /// fallback at all. + func keyboardFocusConfirmed(app: XCUIApplication, element: XCUIElement) -> Bool { +#if os(iOS) + return safely("TEXT_ENTRY_FOCUS_CONFIRMED", false) { + // An element that no longer resolves reads as identifier "" and frame `.zero`, which would + // match any focused element that also reports an empty frame. Require a real frame first, + // so a dead handle cannot corroborate anything. + let frame = element.frame + guard !frame.isEmpty else { + return false + } + let focused = app + .descendants(matching: .any) + .matching(NSPredicate(format: "hasKeyboardFocus == 1")) + .firstMatch + guard focused.exists else { + return false + } + return focused.identifier == element.identifier && focused.frame == frame + } +#else + return false +#endif + } + private func keyboardElementExists(app: XCUIApplication) -> Bool { #if os(iOS) return safely("KEYBOARD_EXISTS", false) { app.keyboards.firstMatch.exists } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SynthesizedCommitBudgetTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SynthesizedCommitBudgetTests.swift new file mode 100644 index 000000000..162540f64 --- /dev/null +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SynthesizedCommitBudgetTests.swift @@ -0,0 +1,70 @@ +import XCTest + +extension RunnerTests { +#if AGENT_DEVICE_RUNNER_UNIT_TESTS + private static func budget( + startedAt: Date, + stallBudget: TimeInterval = 3, + ceiling: TimeInterval = 10 + ) -> SynthesizedCommitBudget { + SynthesizedCommitBudget(startedAt: startedAt, stallBudget: stallBudget, ceiling: ceiling) + } + + // The #1874 regression, stated as the budget sees it: a burst that keeps landing must keep its + // wait alive past the old flat deadline. Before this policy the wait expired at 3s regardless, + // so a throttled-but-working `type`/`fill` reported TEXT_INPUT_COMMIT_NOT_OBSERVED — the red + // that kept appearing on branches touching no iOS code. Reverting `record` to a no-op turns + // this red. + func testProgressKeepsTheCommitWaitAliveBeyondTheFlatDeadline() { + let start = Date(timeIntervalSinceReferenceDate: 0) + let budget = Self.budget(startedAt: start) + // One character per 2 seconds: never idle for 3s, so never expired on the stall rule. + for (index, second) in [2.0, 4.0, 6.0].enumerated() { + budget.record(expectedPrefixLength: index + 1, at: start.addingTimeInterval(second)) + XCTAssertFalse( + budget.isExpired(at: start.addingTimeInterval(second)), + "progress at \(second)s must not be expired" + ) + } + XCTAssertFalse(budget.isExpired(at: start.addingTimeInterval(8))) + } + + // The other half, and the reason the stall budget keeps the flat deadline's number: a pipeline + // that delivers nothing is condemned at exactly the instant it always was. Nothing that fails + // today starts passing by waiting longer. + func testAWedgedPipelineStillExpiresAtTheStallBudget() { + let start = Date(timeIntervalSinceReferenceDate: 0) + let budget = Self.budget(startedAt: start) + XCTAssertFalse(budget.isExpired(at: start.addingTimeInterval(2.99))) + XCTAssertTrue(budget.isExpired(at: start.addingTimeInterval(3))) + } + + // Progress buys time, but not without bound: one character per stall window would otherwise + // hold a command open until the daemon's own 45s timeout killed the runner's request. + func testTheCeilingBoundsAnIndefinitelyThrottledPipeline() { + let start = Date(timeIntervalSinceReferenceDate: 0) + let budget = Self.budget(startedAt: start) + for index in 1...5 { + let now = start.addingTimeInterval(Double(index) * 2) + budget.record(expectedPrefixLength: index, at: now) + XCTAssertEqual(budget.isExpired(at: now), index == 5, "at \(index * 2)s") + } + } + + // Only forward movement is evidence the burst is still landing. A field the app clears + // mid-flight, or an unreadable poll (`observe` reports -1), must not reset the stall clock — + // that would let a wedge disguised as churn hold the wait open to the ceiling every time. + func testBackwardsAndUnreadableObservationsBuyNoTime() { + let start = Date(timeIntervalSinceReferenceDate: 0) + let budget = Self.budget(startedAt: start) + budget.record(expectedPrefixLength: 5, at: start.addingTimeInterval(1)) + for (prefixLength, second) in [(0, 2.0), (-1, 3.0), (5, 3.5)] { + budget.record(expectedPrefixLength: prefixLength, at: start.addingTimeInterval(second)) + } + XCTAssertTrue( + budget.isExpired(at: start.addingTimeInterval(4)), + "the stall clock must still date from the 1s observation that actually advanced" + ) + } +#endif +} diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift index 2635b1ba3..ad056bf63 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift @@ -416,11 +416,13 @@ extension RunnerTests { // poll and the value never becomes "abc" — under the replacement-mode outcome function that is // correctly a failure (see `testSynthesizedReplacementCommitCatchesDroppedMiddleCharacters` for // why it must NOT be waved through as success), so this call runs the real 3-second deadline - // (`TextEntryTiming.synthesizedCommitTimeout`) before returning. That is deliberate here, not a - // flake: this test only runs in the nightly XCUITest lane (see `runner-xctest-local-run-gotchas` - // memory / ios.yml's `-only-testing:` allowlist), where a few extra seconds is a non-issue, and - // the alternative — asserting `nil` on a wiring path that can never actually observe the - // expected text — would silently reintroduce the exact bug this fix closes. + // (`TextEntryTiming.synthesizedCommitStallTimeout`; a nil read never advances the expected + // prefix, so `SynthesizedCommitBudget` grants it no extra time) before returning. That is + // deliberate here, not a flake: this test only runs in the nightly XCUITest lane (see + // `runner-xctest-local-run-gotchas` memory / ios.yml's `-only-testing:` allowlist), where a + // few extra seconds is a non-issue, and the alternative — asserting `nil` on a wiring path + // that can never actually observe the expected text — would silently reintroduce the exact + // bug this fix closes. XCTAssertEqual(result.failure, .commitNotObserved) } diff --git a/src/__tests__/apple-runner-commit-budget-wiring.test.ts b/src/__tests__/apple-runner-commit-budget-wiring.test.ts new file mode 100644 index 000000000..f54360520 --- /dev/null +++ b/src/__tests__/apple-runner-commit-budget-wiring.test.ts @@ -0,0 +1,61 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test } from 'vitest'; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const synthesizedTextEntryPath = path.join( + repoRoot, + 'apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift', +); + +// #1874: the synthesized commit wait grants time against progress instead of a flat deadline, and +// that only works if the two halves stay connected — `observe` must record each poll's +// expected-prefix length into the budget, and `isExpired` must ask that same budget. Delete either +// side and every Swift test still passes: `SynthesizedCommitBudgetTests` drives the budget +// directly, and the one test that runs the real wait observes `nil` on every poll, so it never +// records progress and expires at the stall budget either way. The wait would silently revert to +// the flat 3s deadline the issue is about, with nothing red. +// +// A structural guard rather than an executed one because the coupling lives inside two escaping +// closures built for a live XCUIElement; there is no seam that can be driven from a host lane +// without a simulator and a field whose value grows over several seconds. + +function extractIngredients(source: string): string { + const start = source.indexOf('private func synthesizedCommitPollingIngredients'); + assert.ok(start !== -1, 'the commit wait must keep its polling-ingredients seam'); + const end = source.indexOf('\n /// Blocks until', start); + assert.ok(end !== -1, 'the ingredients function must precede the commit waits that consume it'); + return source.slice(start, end); +} + +test('the commit wait records progress into the same budget its deadline reads', () => { + const ingredients = extractIngredients(fs.readFileSync(synthesizedTextEntryPath, 'utf8')); + + assert.match( + ingredients, + /isExpired: \{ budget\.isExpired\(at: Date\(\)\) \}/, + 'the deadline must be the budget, not a flat wall-clock deadline', + ); + + const observeStart = ingredients.indexOf('observe: {'); + const observeEnd = ingredients.indexOf('waitForNextObservation:', observeStart); + assert.ok(observeStart !== -1 && observeEnd !== -1, 'the wait must keep its observe seam'); + assert.match( + ingredients.slice(observeStart, observeEnd), + /budget\.record\(expectedPrefixLength:/, + 'every observation must record its expected-prefix length, or the budget never extends', + ); + + assert.match( + ingredients, + /stallBudget: TextEntryTiming\.synthesizedCommitStallTimeout/, + 'the no-progress budget must stay the flat deadline it replaced', + ); + assert.match( + ingredients, + /ceiling: TextEntryTiming\.synthesizedCommitCeiling/, + 'progress must stay bounded by an absolute ceiling', + ); +}); From 62e6cc6e598317215f26a4617735506c954b8790 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 25 Aug 2026 19:09:32 +0200 Subject: [PATCH 2/7] refactor(ios): move the commit-wait budget into the wait itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../RunnerTests+SynthesizedCommitBudget.swift | 30 +-- .../RunnerTests+SynthesizedTextEntry.swift | 61 +++--- ...erTests+SynthesizedCommitBudgetTests.swift | 70 ------- .../RunnerTests+TextEntryPolicyTests.swift | 174 ++++++++++++++++-- .../apple-runner-commit-budget-wiring.test.ts | 61 ------ 5 files changed, 214 insertions(+), 182 deletions(-) delete mode 100644 apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SynthesizedCommitBudgetTests.swift delete mode 100644 src/__tests__/apple-runner-commit-budget-wiring.test.ts diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift index b0fc51402..4aa1b46f3 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift @@ -1,10 +1,10 @@ import XCTest -// How long the synthesized text-entry commit wait is willing to keep looking. Split out of -// RunnerTests+SynthesizedTextEntry.swift so the policy is a pure, clock-injected value type the -// macOS host lane can exercise without a simulator. +// How long the synthesized text-entry commit wait keeps looking. Split out of +// RunnerTests+SynthesizedTextEntry.swift only to keep that file inside its size budget; the +// policy is consumed exclusively by the two commit waits there, which is also where it is tested. extension RunnerTests { - /// The commit wait's deadline. + /// One commit wait's deadline. /// /// A synthesized burst can be *throttled* rather than dropped: on a loaded simulator the /// characters keep landing, just slowly, and everything touching the input system slows with @@ -21,12 +21,11 @@ extension RunnerTests { /// deadline used, so nothing that fails today starts passing merely by waiting longer: the wait /// extends only while characters are still arriving. /// - /// A reference type on purpose. Its two readers are separate escaping closures — the commit - /// wait's `observe` records into it, its `isExpired` reads it — and the whole fix is the - /// coupling between them. As a struct that coupling rests on Swift boxing one captured `var`, - /// which a later refactor could quietly break back into the flat deadline with every test still - /// green. Sharing one instance makes that unrepresentable instead of merely true today. - final class SynthesizedCommitBudget { + /// Owned by the wait that creates it: `awaitSynthesizedCommitOutcome` and its replacement + /// counterpart hold it as a local `var` across their own poll loop, so recording progress and + /// asking whether time is up are two statements in one function rather than a coupling between + /// separate closures. + struct SynthesizedCommitBudget { let startedAt: Date let stallBudget: TimeInterval let ceiling: TimeInterval @@ -42,10 +41,19 @@ extension RunnerTests { self.lastProgressAt = startedAt } + /// The budget the shipped `type`/`fill` waits run under. + static func standard(startedAt: Date) -> SynthesizedCommitBudget { + SynthesizedCommitBudget( + startedAt: startedAt, + stallBudget: TextEntryTiming.synthesizedCommitStallTimeout, + ceiling: TextEntryTiming.synthesizedCommitCeiling + ) + } + /// Records one observation's expected-prefix length. Only forward movement counts: a shorter /// read (the app clearing the field mid-flight, an unreadable poll reporting -1) is not /// evidence the burst is still landing, so it neither buys time nor takes any back. - func record(expectedPrefixLength: Int, at now: Date) { + mutating func record(expectedPrefixLength: Int, at now: Date) { guard expectedPrefixLength > bestPrefixLength else { return } bestPrefixLength = expectedPrefixLength lastProgressAt = now diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift index 3dc3678b2..9cc55bc0a 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift @@ -260,13 +260,18 @@ extension RunnerTests { case notObserved } - /// The commit wait's decision, with observation, pacing and the clock injected so the deadline - /// branch is exercisable without a simulator (the macOS host lane runs this; the member wrapper - /// below binds the real XCUI reads). + /// The commit wait's decision, with observation, pacing and the clock injected so both deadline + /// branches are exercisable without a simulator (the macOS host lane runs this; the member + /// wrapper below binds the real XCUI reads). + /// + /// The budget is a local `var`, advanced from the same observation the progress check reads, so + /// "did the burst move" and "is time up" are two statements in one loop rather than a coupling + /// between separately-held state. static func awaitSynthesizedCommitOutcome( expectedText: String, placeholder: String?, - isExpired: () -> Bool, + budget: SynthesizedCommitBudget, + now: () -> Date, observe: () -> String?, waitForNextObservation: () -> Void ) -> SynthesizedTextCommitOutcome { @@ -276,15 +281,21 @@ extension RunnerTests { if Self.textMatchesPlaceholder(expectedText, placeholder: placeholder) { return .notObserved } + var budget = budget // The deadline is checked AFTER an observation, never before one, so the last thing that // happens before condemning a commit is a read. Checking first would condemn a commit that // landed during the final poll sleep — the exact loaded-host timing this wait exists for. while true { - switch synthesizedTextCommitProgress(observedText: observe(), expectedText: expectedText) { + let observedText = observe() + switch synthesizedTextCommitProgress(observedText: observedText, expectedText: expectedText) { case .committed, .diverged: return .settled case .pending: - if isExpired() { return .notObserved } + budget.record( + expectedPrefixLength: Self.commonPrefixLength(observedText ?? "", expectedText), + at: now() + ) + if budget.isExpired(at: now()) { return .notObserved } waitForNextObservation() } } @@ -323,23 +334,33 @@ extension RunnerTests { static func awaitSynthesizedReplacementCommitOutcome( expectedText: String, placeholder: String?, - isExpired: () -> Bool, + budget: SynthesizedCommitBudget, + now: () -> Date, observe: () -> String?, waitForNextObservation: () -> Void ) -> SynthesizedTextCommitOutcome { if Self.textMatchesPlaceholder(expectedText, placeholder: placeholder) { return .notObserved } + var budget = budget while true { - if observe() == expectedText { + let observedText = observe() + if observedText == expectedText { return .settled } - if isExpired() { return .notObserved } + // Prefix growth cannot settle this wait — a value with a hole in the middle is still a + // failure, see the doc comment above — but it is the same evidence that the burst is still + // landing, so it buys the same time here as it does in append mode. + budget.record( + expectedPrefixLength: Self.commonPrefixLength(observedText ?? "", expectedText), + at: now() + ) + if budget.isExpired(at: now()) { return .notObserved } waitForNextObservation() } } - /// The placeholder/deadline/observe/pacing ingredients shared by the append route + /// The placeholder/observe/pacing ingredients shared by the append route /// (`awaitSynthesizedFirstResponderCommit`) and the replacement route /// (`awaitSynthesizedReplacementCommit`). What must NOT be shared is which outcome function /// consumes them: see `awaitSynthesizedReplacementCommitOutcome`'s doc comment for why append @@ -355,37 +376,25 @@ extension RunnerTests { expectedText: String ) -> ( placeholder: String?, - isExpired: () -> Bool, observe: () -> String?, waitForNextObservation: () -> Void ) { let placeholder = resolveTextEntryElement(app: app, target: target)?.placeholderValue let waitStartedAt = Date() - // Throttling and a wedge are the same to a flat deadline, so the expected-prefix walk decides - // instead of the wall clock alone — the very distinction the cadence line below was added to - // make visible. See `SynthesizedCommitBudget`. - let budget = SynthesizedCommitBudget( - startedAt: waitStartedAt, - stallBudget: TextEntryTiming.synthesizedCommitStallTimeout, - ceiling: TextEntryTiming.synthesizedCommitCeiling - ) return ( placeholder: placeholder, - isExpired: { budget.isExpired(at: Date()) }, observe: { let observedText = self.editableTextValue( for: self.resolveTextEntryElement(app: app, target: target), treatingPlaceholderAsEmpty: true ) - let expectedPrefixLen = observedText.map { Self.commonPrefixLength($0, expectedText) } ?? -1 - budget.record(expectedPrefixLength: expectedPrefixLen, at: Date()) // Cadence evidence stays value-free: the polled value is user content typed through // `type`/`fill` and must never reach runner.log. Lengths and the expected-prefix walk // are enough to distinguish throttling (prefix grows slowly) from a wedge (it freezes). Self.logCommitCadence( elapsedMs: Int(waitStartedAt.timeIntervalSinceNow * -1000), observedLen: observedText?.count ?? -1, - expectedPrefixLen: expectedPrefixLen + expectedPrefixLen: observedText.map { Self.commonPrefixLength($0, expectedText) } ?? -1 ) return observedText }, @@ -421,7 +430,8 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: expectedText, placeholder: ingredients.placeholder, - isExpired: ingredients.isExpired, + budget: .standard(startedAt: waitStartedAt), + now: { Date() }, observe: ingredients.observe, waitForNextObservation: ingredients.waitForNextObservation ) @@ -457,7 +467,8 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: expectedText, placeholder: ingredients.placeholder, - isExpired: ingredients.isExpired, + budget: .standard(startedAt: waitStartedAt), + now: { Date() }, observe: ingredients.observe, waitForNextObservation: ingredients.waitForNextObservation ) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SynthesizedCommitBudgetTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SynthesizedCommitBudgetTests.swift deleted file mode 100644 index 162540f64..000000000 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SynthesizedCommitBudgetTests.swift +++ /dev/null @@ -1,70 +0,0 @@ -import XCTest - -extension RunnerTests { -#if AGENT_DEVICE_RUNNER_UNIT_TESTS - private static func budget( - startedAt: Date, - stallBudget: TimeInterval = 3, - ceiling: TimeInterval = 10 - ) -> SynthesizedCommitBudget { - SynthesizedCommitBudget(startedAt: startedAt, stallBudget: stallBudget, ceiling: ceiling) - } - - // The #1874 regression, stated as the budget sees it: a burst that keeps landing must keep its - // wait alive past the old flat deadline. Before this policy the wait expired at 3s regardless, - // so a throttled-but-working `type`/`fill` reported TEXT_INPUT_COMMIT_NOT_OBSERVED — the red - // that kept appearing on branches touching no iOS code. Reverting `record` to a no-op turns - // this red. - func testProgressKeepsTheCommitWaitAliveBeyondTheFlatDeadline() { - let start = Date(timeIntervalSinceReferenceDate: 0) - let budget = Self.budget(startedAt: start) - // One character per 2 seconds: never idle for 3s, so never expired on the stall rule. - for (index, second) in [2.0, 4.0, 6.0].enumerated() { - budget.record(expectedPrefixLength: index + 1, at: start.addingTimeInterval(second)) - XCTAssertFalse( - budget.isExpired(at: start.addingTimeInterval(second)), - "progress at \(second)s must not be expired" - ) - } - XCTAssertFalse(budget.isExpired(at: start.addingTimeInterval(8))) - } - - // The other half, and the reason the stall budget keeps the flat deadline's number: a pipeline - // that delivers nothing is condemned at exactly the instant it always was. Nothing that fails - // today starts passing by waiting longer. - func testAWedgedPipelineStillExpiresAtTheStallBudget() { - let start = Date(timeIntervalSinceReferenceDate: 0) - let budget = Self.budget(startedAt: start) - XCTAssertFalse(budget.isExpired(at: start.addingTimeInterval(2.99))) - XCTAssertTrue(budget.isExpired(at: start.addingTimeInterval(3))) - } - - // Progress buys time, but not without bound: one character per stall window would otherwise - // hold a command open until the daemon's own 45s timeout killed the runner's request. - func testTheCeilingBoundsAnIndefinitelyThrottledPipeline() { - let start = Date(timeIntervalSinceReferenceDate: 0) - let budget = Self.budget(startedAt: start) - for index in 1...5 { - let now = start.addingTimeInterval(Double(index) * 2) - budget.record(expectedPrefixLength: index, at: now) - XCTAssertEqual(budget.isExpired(at: now), index == 5, "at \(index * 2)s") - } - } - - // Only forward movement is evidence the burst is still landing. A field the app clears - // mid-flight, or an unreadable poll (`observe` reports -1), must not reset the stall clock — - // that would let a wedge disguised as churn hold the wait open to the ceiling every time. - func testBackwardsAndUnreadableObservationsBuyNoTime() { - let start = Date(timeIntervalSinceReferenceDate: 0) - let budget = Self.budget(startedAt: start) - budget.record(expectedPrefixLength: 5, at: start.addingTimeInterval(1)) - for (prefixLength, second) in [(0, 2.0), (-1, 3.0), (5, 3.5)] { - budget.record(expectedPrefixLength: prefixLength, at: start.addingTimeInterval(second)) - } - XCTAssertTrue( - budget.isExpired(at: start.addingTimeInterval(4)), - "the stall clock must still date from the 1s observation that actually advanced" - ) - } -#endif -} diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift index ad056bf63..7ea6addf7 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift @@ -90,17 +90,130 @@ extension RunnerTests { ) } + /// A hand-driven clock for the commit waits. Time moves only where the wait sleeps, which is + /// what makes "the burst kept landing" and "the pipeline froze" expressible as two sequences of + /// the same length rather than as wall-clock luck. + final class CommitWaitClock { + private var current = Date(timeIntervalSinceReferenceDate: 0) + let startedAt = Date(timeIntervalSinceReferenceDate: 0) + + var read: () -> Date { { self.current } } + + func advance(_ seconds: TimeInterval) { + current = current.addingTimeInterval(seconds) + } + + /// Defaults to a budget no test can exhaust, so a test that never advances the clock is + /// asking about settling rather than about time. + func budget(stallBudget: TimeInterval = 3600, ceiling: TimeInterval = 3600) -> SynthesizedCommitBudget { + SynthesizedCommitBudget(startedAt: startedAt, stallBudget: stallBudget, ceiling: ceiling) + } + + /// Seconds elapsed on this clock, for asserting *when* a wait gave up. + var elapsed: TimeInterval { current.timeIntervalSince(startedAt) } + } + + // #1874, through the shipped wait rather than a detached policy object: a burst that keeps + // landing must outlive the flat 3s deadline that used to govern it. Each poll advances the + // clock 2s and delivers one more character, so the wait is never idle for a full stall budget + // and must walk all the way to the match at t=6s. Reverting the wait to a flat deadline turns + // this red at the third poll. + func testCommitWaitOutlivesTheFlatDeadlineWhileTheExpectedPrefixGrows() { + let expected = "hardware" + let clock = CommitWaitClock() + var landed = 0 + let outcome = Self.awaitSynthesizedCommitOutcome( + expectedText: expected, + placeholder: nil, + budget: clock.budget(stallBudget: 3, ceiling: 10), + now: clock.read, + observe: { String(expected.prefix(landed * 2)) }, + waitForNextObservation: { + landed += 1 + clock.advance(2) + } + ) + XCTAssertEqual(outcome, .settled) + XCTAssertEqual(clock.elapsed, 8, "the wait must still be polling well past the 3s stall budget") + } + + // The other half, and the reason the stall budget keeps the flat deadline's number: a pipeline + // that delivers nothing is condemned at exactly the instant it always was, so nothing that + // fails today starts passing merely by waiting longer. + func testCommitWaitCondemnsAFrozenPipelineAtTheStallBudget() { + let clock = CommitWaitClock() + let outcome = Self.awaitSynthesizedCommitOutcome( + expectedText: "hardware", + placeholder: nil, + budget: clock.budget(stallBudget: 3, ceiling: 10), + now: clock.read, + observe: { "ha" }, + waitForNextObservation: { clock.advance(1) } + ) + XCTAssertEqual(outcome, .notObserved) + XCTAssertEqual(clock.elapsed, 3, "a frozen prefix must give up on the stall budget, not the ceiling") + } + + // Progress buys time, but not without bound: one character per stall window would otherwise + // hold the command open until the daemon's own 45s budget killed the request. Here every poll + // lands a character, so only the ceiling can stop it. + func testCommitWaitCeilingStopsAnIndefinitelyThrottledPipeline() { + let clock = CommitWaitClock() + var landed = 0 + let outcome = Self.awaitSynthesizedCommitOutcome( + expectedText: String(repeating: "a", count: 100), + placeholder: nil, + budget: clock.budget(stallBudget: 3, ceiling: 10), + now: clock.read, + observe: { String(repeating: "a", count: landed) }, + waitForNextObservation: { + landed += 1 + clock.advance(2) + } + ) + XCTAssertEqual(outcome, .notObserved) + XCTAssertEqual(clock.elapsed, 10, "the ceiling is absolute, however long characters keep arriving") + } + + // Only forward movement is evidence the burst is still landing. A field the app clears + // mid-flight would otherwise reset the stall clock on every poll and hold every wedged wait + // open to the ceiling. Replacement mode, because that is where a non-matching value keeps + // polling rather than settling as `.diverged`. + func testCommitWaitTreatsARetreatingValueAsNoProgress() { + let clock = CommitWaitClock() + let observations = ["ada@", "", "ada@", "", "ada@"] + var index = 0 + let outcome = Self.awaitSynthesizedReplacementCommitOutcome( + expectedText: "ada@example", + placeholder: nil, + budget: clock.budget(stallBudget: 3, ceiling: 10), + now: clock.read, + observe: { observations[min(index, observations.count - 1)] }, + waitForNextObservation: { + index += 1 + clock.advance(1) + } + ) + XCTAssertEqual(outcome, .notObserved) + XCTAssertEqual(clock.elapsed, 3, "churn between two values is not progress and must not buy time") + } + // The regression behind #1874/#1844: the wait used to return Void, so an expired deadline was // indistinguishable from a commit and `type` reported ok over a partially committed field. The // CI signature was a field holding "h" out of "hardware-keyboard" with the command successful. func testSynthesizedCommitDeadlineIsNotReportedAsACommit() { + let clock = CommitWaitClock() var observations = 0 let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware-keyboard", placeholder: nil, - isExpired: { observations >= 3 }, + budget: clock.budget(stallBudget: 3, ceiling: 10), + now: clock.read, observe: { "h" }, - waitForNextObservation: { observations += 1 } + waitForNextObservation: { + observations += 1 + clock.advance(1) + } ) XCTAssertEqual(outcome, .notObserved) XCTAssertEqual(observations, 3, "a pending prefix must keep polling until the deadline") @@ -108,11 +221,13 @@ extension RunnerTests { func testSynthesizedCommitStopsAtTheFirstSettledObservation() { for observed in ["hardware-keyboard", "hardwarX", nil] { + let clock = CommitWaitClock() var polls = 0 let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware-keyboard", placeholder: nil, - isExpired: { false }, + budget: clock.budget(), + now: clock.read, observe: { observed }, waitForNextObservation: { polls += 1 } ) @@ -125,11 +240,13 @@ extension RunnerTests { func testSynthesizedCommitWalksAPrefixToCompletion() { let steps = ["", "hardware-", "hardware-keyboard"] + let clock = CommitWaitClock() var index = 0 let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware-keyboard", placeholder: nil, - isExpired: { false }, + budget: clock.budget(), + now: clock.read, observe: { steps[min(index, steps.count - 1)] }, waitForNextObservation: { index += 1 } ) @@ -141,13 +258,20 @@ extension RunnerTests { // landing during the final poll sleep was condemned as never observed — a false failure under // exactly the loaded-host timing this wait exists for. Red against that ordering. func testCommitLandingDuringTheFinalSleepIsStillObserved() { + let clock = CommitWaitClock() var polls = 0 let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware-keyboard", placeholder: nil, - isExpired: { polls >= 1 }, + budget: clock.budget(stallBudget: 3, ceiling: 10), + now: clock.read, + // The value lands during the sleep that takes the clock past the stall budget: the read + // happens first, so it is still observed. observe: { polls == 0 ? "hardware-" : "hardware-keyboard" }, - waitForNextObservation: { polls += 1 } + waitForNextObservation: { + polls += 1 + clock.advance(9) + } ) XCTAssertEqual(outcome, .settled) } @@ -159,11 +283,13 @@ extension RunnerTests { func testClearAfterDispatchCannotTurnThePlaceholderIntoCommitEvidence() { let textBeforeDispatch = "0" let expectedText = textBeforeDispatch + ".00" + let clock = CommitWaitClock() var observations = 0 let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: expectedText, placeholder: "0.00", - isExpired: { false }, + budget: clock.budget(), + now: clock.read, observe: { observations += 1 return "0.00" @@ -188,10 +314,12 @@ extension RunnerTests { (" ", ""), ] for testCase in cases { + let clock = CommitWaitClock() let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: testCase.expectedText, placeholder: testCase.placeholder, - isExpired: { false }, + budget: clock.budget(), + now: clock.read, observe: { testCase.expectedText }, waitForNextObservation: {} ) @@ -215,24 +343,31 @@ extension RunnerTests { (expected: "ada@example", observedAfterDrop: "aexample"), ] for corruption in corruptions { + let appendClock = CommitWaitClock() XCTAssertEqual( Self.awaitSynthesizedCommitOutcome( expectedText: corruption.expected, placeholder: nil, - isExpired: { false }, + budget: appendClock.budget(), + now: appendClock.read, observe: { corruption.observedAfterDrop }, waitForNextObservation: {} ), .settled, "append-mode's diverge-trusting outcome must stay unchanged by this fix" ) + let clock = CommitWaitClock() var polls = 0 let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: corruption.expected, placeholder: nil, - isExpired: { polls >= 2 }, + budget: clock.budget(stallBudget: 2, ceiling: 10), + now: clock.read, observe: { corruption.observedAfterDrop }, - waitForNextObservation: { polls += 1 } + waitForNextObservation: { + polls += 1 + clock.advance(1) + } ) XCTAssertEqual(outcome, .notObserved, "expected \(corruption.expected), dropped to \(corruption.observedAfterDrop)") XCTAssertEqual(polls, 2, "a settled-but-wrong value must be polled until the deadline, not trusted early") @@ -246,11 +381,13 @@ extension RunnerTests { // does not depend on prefix-walking to keep polling. func testSynthesizedReplacementCommitToleratesLagUntilExactMatch() { let steps = ["", "ad", "ada@example"] + let clock = CommitWaitClock() var index = 0 let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: "ada@example", placeholder: nil, - isExpired: { false }, + budget: clock.budget(), + now: clock.read, observe: { steps[min(index, steps.count - 1)] }, waitForNextObservation: { index += 1 } ) @@ -261,13 +398,18 @@ extension RunnerTests { // Same ordering guarantee as `testCommitLandingDuringTheFinalSleepIsStillObserved`: the deadline // is checked AFTER an observation, so a match landing during the final poll sleep is still caught. func testSynthesizedReplacementCommitLandingDuringTheFinalSleepIsStillObserved() { + let clock = CommitWaitClock() var polls = 0 let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: "ada@example", placeholder: nil, - isExpired: { polls >= 1 }, + budget: clock.budget(stallBudget: 3, ceiling: 10), + now: clock.read, observe: { polls == 0 ? "ada@exampl" : "ada@example" }, - waitForNextObservation: { polls += 1 } + waitForNextObservation: { + polls += 1 + clock.advance(9) + } ) XCTAssertEqual(outcome, .settled) } @@ -275,11 +417,13 @@ extension RunnerTests { // Same placeholder-collision guard as append mode, and for the same reason: a pre-dispatch value // cannot identify what a later placeholder-equal AX value represents, so refuse before polling. func testSynthesizedReplacementCommitPlaceholderGuardRefusesWithoutPolling() { + let clock = CommitWaitClock() var observations = 0 let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: "0.00", placeholder: "0.00", - isExpired: { false }, + budget: clock.budget(), + now: clock.read, observe: { observations += 1 return "0.00" diff --git a/src/__tests__/apple-runner-commit-budget-wiring.test.ts b/src/__tests__/apple-runner-commit-budget-wiring.test.ts deleted file mode 100644 index f54360520..000000000 --- a/src/__tests__/apple-runner-commit-budget-wiring.test.ts +++ /dev/null @@ -1,61 +0,0 @@ -import assert from 'node:assert/strict'; -import fs from 'node:fs'; -import path from 'node:path'; -import { fileURLToPath } from 'node:url'; -import { test } from 'vitest'; - -const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); -const synthesizedTextEntryPath = path.join( - repoRoot, - 'apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift', -); - -// #1874: the synthesized commit wait grants time against progress instead of a flat deadline, and -// that only works if the two halves stay connected — `observe` must record each poll's -// expected-prefix length into the budget, and `isExpired` must ask that same budget. Delete either -// side and every Swift test still passes: `SynthesizedCommitBudgetTests` drives the budget -// directly, and the one test that runs the real wait observes `nil` on every poll, so it never -// records progress and expires at the stall budget either way. The wait would silently revert to -// the flat 3s deadline the issue is about, with nothing red. -// -// A structural guard rather than an executed one because the coupling lives inside two escaping -// closures built for a live XCUIElement; there is no seam that can be driven from a host lane -// without a simulator and a field whose value grows over several seconds. - -function extractIngredients(source: string): string { - const start = source.indexOf('private func synthesizedCommitPollingIngredients'); - assert.ok(start !== -1, 'the commit wait must keep its polling-ingredients seam'); - const end = source.indexOf('\n /// Blocks until', start); - assert.ok(end !== -1, 'the ingredients function must precede the commit waits that consume it'); - return source.slice(start, end); -} - -test('the commit wait records progress into the same budget its deadline reads', () => { - const ingredients = extractIngredients(fs.readFileSync(synthesizedTextEntryPath, 'utf8')); - - assert.match( - ingredients, - /isExpired: \{ budget\.isExpired\(at: Date\(\)\) \}/, - 'the deadline must be the budget, not a flat wall-clock deadline', - ); - - const observeStart = ingredients.indexOf('observe: {'); - const observeEnd = ingredients.indexOf('waitForNextObservation:', observeStart); - assert.ok(observeStart !== -1 && observeEnd !== -1, 'the wait must keep its observe seam'); - assert.match( - ingredients.slice(observeStart, observeEnd), - /budget\.record\(expectedPrefixLength:/, - 'every observation must record its expected-prefix length, or the budget never extends', - ); - - assert.match( - ingredients, - /stallBudget: TextEntryTiming\.synthesizedCommitStallTimeout/, - 'the no-progress budget must stay the flat deadline it replaced', - ); - assert.match( - ingredients, - /ceiling: TextEntryTiming\.synthesizedCommitCeiling/, - 'progress must stay bounded by an absolute ceiling', - ); -}); From bf9a84f3ce614f8acfbbf8784e4b5a2a7ca7c169 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 25 Aug 2026 21:23:11 +0200 Subject: [PATCH 3/7] fix(ios): grant the text-entry commit wait time against progress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../RunnerTests+SynthesizedCommitBudget.swift | 76 ++++++++++--------- .../RunnerTests+SynthesizedTextEntry.swift | 30 ++++---- .../RunnerTests+TextEntryPolicyTests.swift | 69 +++++++++++------ 3 files changed, 104 insertions(+), 71 deletions(-) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift index 4aa1b46f3..581025307 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift @@ -4,7 +4,7 @@ import XCTest // RunnerTests+SynthesizedTextEntry.swift only to keep that file inside its size budget; the // policy is consumed exclusively by the two commit waits there, which is also where it is tested. extension RunnerTests { - /// One commit wait's deadline. + /// How long a commit wait is allowed to keep looking. /// /// A synthesized burst can be *throttled* rather than dropped: on a loaded simulator the /// characters keep landing, just slowly, and everything touching the input system slows with @@ -20,48 +20,54 @@ extension RunnerTests { /// A pipeline making no progress at all still expires at exactly the `stallBudget` the flat /// deadline used, so nothing that fails today starts passing merely by waiting longer: the wait /// extends only while characters are still arriving. - /// - /// Owned by the wait that creates it: `awaitSynthesizedCommitOutcome` and its replacement - /// counterpart hold it as a local `var` across their own poll loop, so recording progress and - /// asking whether time is up are two statements in one function rather than a coupling between - /// separate closures. struct SynthesizedCommitBudget { - let startedAt: Date let stallBudget: TimeInterval let ceiling: TimeInterval - private var bestPrefixLength: Int - private var lastProgressAt: Date - - init(startedAt: Date, stallBudget: TimeInterval, ceiling: TimeInterval) { - self.startedAt = startedAt - self.stallBudget = stallBudget - self.ceiling = ceiling - // Nothing has landed yet, and an observation of "still nothing" must not read as progress. - self.bestPrefixLength = 0 - self.lastProgressAt = startedAt - } /// The budget the shipped `type`/`fill` waits run under. - static func standard(startedAt: Date) -> SynthesizedCommitBudget { - SynthesizedCommitBudget( - startedAt: startedAt, - stallBudget: TextEntryTiming.synthesizedCommitStallTimeout, - ceiling: TextEntryTiming.synthesizedCommitCeiling - ) - } + static let standard = SynthesizedCommitBudget( + stallBudget: TextEntryTiming.synthesizedCommitStallTimeout, + ceiling: TextEntryTiming.synthesizedCommitCeiling + ) - /// Records one observation's expected-prefix length. Only forward movement counts: a shorter - /// read (the app clearing the field mid-flight, an unreadable poll reporting -1) is not - /// evidence the burst is still landing, so it neither buys time nor takes any back. - mutating func record(expectedPrefixLength: Int, at now: Date) { - guard expectedPrefixLength > bestPrefixLength else { return } - bestPrefixLength = expectedPrefixLength - lastProgressAt = now + /// Starts the clock, and only the poll loop may: the wait's setup reads the field's + /// placeholder first, which is an AX round-trip that takes seconds on exactly the loaded host + /// this budget exists for. Time spent there is not time the field was given to commit, and + /// charging it to the budget made the wait give up sooner than the flat deadline it replaced. + func deadline(startedAt: Date) -> Deadline { + Deadline(budget: self, startedAt: startedAt) } - func isExpired(at now: Date) -> Bool { - now.timeIntervalSince(startedAt) >= ceiling - || now.timeIntervalSince(lastProgressAt) >= stallBudget + /// One wait's running deadline. Held as a local `var` by the loop that started it, so + /// recording progress and asking whether time is up are two statements in one function + /// rather than a coupling between separately-held state. + struct Deadline { + private let budget: SynthesizedCommitBudget + private let startedAt: Date + // Nothing has landed yet, and an observation of "still nothing" must not read as progress. + private var bestPrefixLength = 0 + private var lastProgressAt: Date + + init(budget: SynthesizedCommitBudget, startedAt: Date) { + self.budget = budget + self.startedAt = startedAt + self.lastProgressAt = startedAt + } + + /// Records one observation's expected-prefix length. Only forward movement counts: a + /// shorter read — the app clearing the field mid-flight, or a value that could not be read + /// at all, which measures as 0 — is not evidence the burst is still landing, so it neither + /// buys time nor takes any back. + mutating func record(expectedPrefixLength: Int, at now: Date) { + guard expectedPrefixLength > bestPrefixLength else { return } + bestPrefixLength = expectedPrefixLength + lastProgressAt = now + } + + func isExpired(at now: Date) -> Bool { + now.timeIntervalSince(startedAt) >= budget.ceiling + || now.timeIntervalSince(lastProgressAt) >= budget.stallBudget + } } } } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift index 9cc55bc0a..1ae7c972d 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift @@ -264,9 +264,9 @@ extension RunnerTests { /// branches are exercisable without a simulator (the macOS host lane runs this; the member /// wrapper below binds the real XCUI reads). /// - /// The budget is a local `var`, advanced from the same observation the progress check reads, so - /// "did the burst move" and "is time up" are two statements in one loop rather than a coupling - /// between separately-held state. + /// The deadline is a local `var`, started from this loop's own first `now()` and advanced from + /// the same observation the progress check reads, so "did the burst move" and "is time up" are + /// two statements in one loop rather than a coupling between separately-held state. static func awaitSynthesizedCommitOutcome( expectedText: String, placeholder: String?, @@ -281,7 +281,7 @@ extension RunnerTests { if Self.textMatchesPlaceholder(expectedText, placeholder: placeholder) { return .notObserved } - var budget = budget + var deadline = budget.deadline(startedAt: now()) // The deadline is checked AFTER an observation, never before one, so the last thing that // happens before condemning a commit is a read. Checking first would condemn a commit that // landed during the final poll sleep — the exact loaded-host timing this wait exists for. @@ -291,11 +291,14 @@ extension RunnerTests { case .committed, .diverged: return .settled case .pending: - budget.record( + // One clock sample, so the instant an observation is recorded at is the instant it is + // judged against. + let sampledAt = now() + deadline.record( expectedPrefixLength: Self.commonPrefixLength(observedText ?? "", expectedText), - at: now() + at: sampledAt ) - if budget.isExpired(at: now()) { return .notObserved } + if deadline.isExpired(at: sampledAt) { return .notObserved } waitForNextObservation() } } @@ -342,7 +345,7 @@ extension RunnerTests { if Self.textMatchesPlaceholder(expectedText, placeholder: placeholder) { return .notObserved } - var budget = budget + var deadline = budget.deadline(startedAt: now()) while true { let observedText = observe() if observedText == expectedText { @@ -351,11 +354,12 @@ extension RunnerTests { // Prefix growth cannot settle this wait — a value with a hole in the middle is still a // failure, see the doc comment above — but it is the same evidence that the burst is still // landing, so it buys the same time here as it does in append mode. - budget.record( + let sampledAt = now() + deadline.record( expectedPrefixLength: Self.commonPrefixLength(observedText ?? "", expectedText), - at: now() + at: sampledAt ) - if budget.isExpired(at: now()) { return .notObserved } + if deadline.isExpired(at: sampledAt) { return .notObserved } waitForNextObservation() } } @@ -430,7 +434,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: expectedText, placeholder: ingredients.placeholder, - budget: .standard(startedAt: waitStartedAt), + budget: .standard, now: { Date() }, observe: ingredients.observe, waitForNextObservation: ingredients.waitForNextObservation @@ -467,7 +471,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: expectedText, placeholder: ingredients.placeholder, - budget: .standard(startedAt: waitStartedAt), + budget: .standard, now: { Date() }, observe: ingredients.observe, waitForNextObservation: ingredients.waitForNextObservation diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift index 7ea6addf7..412b92595 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift @@ -94,8 +94,8 @@ extension RunnerTests { /// what makes "the burst kept landing" and "the pipeline froze" expressible as two sequences of /// the same length rather than as wall-clock luck. final class CommitWaitClock { + private let origin = Date(timeIntervalSinceReferenceDate: 0) private var current = Date(timeIntervalSinceReferenceDate: 0) - let startedAt = Date(timeIntervalSinceReferenceDate: 0) var read: () -> Date { { self.current } } @@ -103,16 +103,14 @@ extension RunnerTests { current = current.addingTimeInterval(seconds) } - /// Defaults to a budget no test can exhaust, so a test that never advances the clock is - /// asking about settling rather than about time. - func budget(stallBudget: TimeInterval = 3600, ceiling: TimeInterval = 3600) -> SynthesizedCommitBudget { - SynthesizedCommitBudget(startedAt: startedAt, stallBudget: stallBudget, ceiling: ceiling) - } - /// Seconds elapsed on this clock, for asserting *when* a wait gave up. - var elapsed: TimeInterval { current.timeIntervalSince(startedAt) } + var elapsed: TimeInterval { current.timeIntervalSince(origin) } } + /// A budget no test can exhaust, so a test that never advances the clock is asking about + /// settling rather than about time. + static let unboundedCommitBudget = SynthesizedCommitBudget(stallBudget: 3600, ceiling: 3600) + // #1874, through the shipped wait rather than a detached policy object: a burst that keeps // landing must outlive the flat 3s deadline that used to govern it. Each poll advances the // clock 2s and delivers one more character, so the wait is never idle for a full stall budget @@ -125,7 +123,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: expected, placeholder: nil, - budget: clock.budget(stallBudget: 3, ceiling: 10), + budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), now: clock.read, observe: { String(expected.prefix(landed * 2)) }, waitForNextObservation: { @@ -137,6 +135,31 @@ extension RunnerTests { XCTAssertEqual(clock.elapsed, 8, "the wait must still be polling well past the 3s stall budget") } + // Review finding: the wait's setup reads the field's placeholder first, and that is an AX + // round-trip which takes seconds on exactly the loaded host this budget exists for. An earlier + // revision started the clock before that read, so slow setup spent the budget and the wait gave + // up after a single poll — sooner than the flat deadline it replaced, in the one condition it + // was written for. The clock now starts inside the loop, so a slow setup buys nothing and costs + // nothing. + func testCommitWaitBudgetStartsAtTheLoopRatherThanBeforeIt() { + let clock = CommitWaitClock() + clock.advance(60) + var polls = 0 + let outcome = Self.awaitSynthesizedCommitOutcome( + expectedText: "hardware", + placeholder: nil, + budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), + now: clock.read, + observe: { "ha" }, + waitForNextObservation: { + polls += 1 + clock.advance(1) + } + ) + XCTAssertEqual(outcome, .notObserved) + XCTAssertEqual(polls, 3, "the wait must get its whole stall budget however slow its setup was") + } + // The other half, and the reason the stall budget keeps the flat deadline's number: a pipeline // that delivers nothing is condemned at exactly the instant it always was, so nothing that // fails today starts passing merely by waiting longer. @@ -145,7 +168,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware", placeholder: nil, - budget: clock.budget(stallBudget: 3, ceiling: 10), + budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), now: clock.read, observe: { "ha" }, waitForNextObservation: { clock.advance(1) } @@ -163,7 +186,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: String(repeating: "a", count: 100), placeholder: nil, - budget: clock.budget(stallBudget: 3, ceiling: 10), + budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), now: clock.read, observe: { String(repeating: "a", count: landed) }, waitForNextObservation: { @@ -186,7 +209,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: "ada@example", placeholder: nil, - budget: clock.budget(stallBudget: 3, ceiling: 10), + budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), now: clock.read, observe: { observations[min(index, observations.count - 1)] }, waitForNextObservation: { @@ -207,7 +230,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware-keyboard", placeholder: nil, - budget: clock.budget(stallBudget: 3, ceiling: 10), + budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), now: clock.read, observe: { "h" }, waitForNextObservation: { @@ -226,7 +249,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware-keyboard", placeholder: nil, - budget: clock.budget(), + budget: Self.unboundedCommitBudget, now: clock.read, observe: { observed }, waitForNextObservation: { polls += 1 } @@ -245,7 +268,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware-keyboard", placeholder: nil, - budget: clock.budget(), + budget: Self.unboundedCommitBudget, now: clock.read, observe: { steps[min(index, steps.count - 1)] }, waitForNextObservation: { index += 1 } @@ -263,7 +286,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware-keyboard", placeholder: nil, - budget: clock.budget(stallBudget: 3, ceiling: 10), + budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), now: clock.read, // The value lands during the sleep that takes the clock past the stall budget: the read // happens first, so it is still observed. @@ -288,7 +311,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: expectedText, placeholder: "0.00", - budget: clock.budget(), + budget: Self.unboundedCommitBudget, now: clock.read, observe: { observations += 1 @@ -318,7 +341,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: testCase.expectedText, placeholder: testCase.placeholder, - budget: clock.budget(), + budget: Self.unboundedCommitBudget, now: clock.read, observe: { testCase.expectedText }, waitForNextObservation: {} @@ -348,7 +371,7 @@ extension RunnerTests { Self.awaitSynthesizedCommitOutcome( expectedText: corruption.expected, placeholder: nil, - budget: appendClock.budget(), + budget: Self.unboundedCommitBudget, now: appendClock.read, observe: { corruption.observedAfterDrop }, waitForNextObservation: {} @@ -361,7 +384,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: corruption.expected, placeholder: nil, - budget: clock.budget(stallBudget: 2, ceiling: 10), + budget: SynthesizedCommitBudget(stallBudget: 2, ceiling: 10), now: clock.read, observe: { corruption.observedAfterDrop }, waitForNextObservation: { @@ -386,7 +409,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: "ada@example", placeholder: nil, - budget: clock.budget(), + budget: Self.unboundedCommitBudget, now: clock.read, observe: { steps[min(index, steps.count - 1)] }, waitForNextObservation: { index += 1 } @@ -403,7 +426,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: "ada@example", placeholder: nil, - budget: clock.budget(stallBudget: 3, ceiling: 10), + budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), now: clock.read, observe: { polls == 0 ? "ada@exampl" : "ada@example" }, waitForNextObservation: { @@ -422,7 +445,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: "0.00", placeholder: "0.00", - budget: clock.budget(), + budget: Self.unboundedCommitBudget, now: clock.read, observe: { observations += 1 From 837bdd1dd1896d5519297f0bf908d37153095125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 25 Aug 2026 21:52:04 +0200 Subject: [PATCH 4/7] fix(test-app): stop the form fixture placing its own placeholder in every fill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- examples/test-app/src/screens/FormScreen.tsx | 4 +- ...fixture-fill-placeholder-collision.test.ts | 83 +++++++++++++++++++ 2 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 src/__tests__/fixture-fill-placeholder-collision.test.ts diff --git a/examples/test-app/src/screens/FormScreen.tsx b/examples/test-app/src/screens/FormScreen.tsx index 42802194c..e9c8273a6 100644 --- a/examples/test-app/src/screens/FormScreen.tsx +++ b/examples/test-app/src/screens/FormScreen.tsx @@ -108,7 +108,7 @@ export function FormScreen(props: FormScreenProps) { accessibilityLabel="Full name" label="Full name" onChangeText={(value) => props.onChange('name', value)} - placeholder="Ada Lovelace" + placeholder="Type your full name" testID="field-name" value={props.form.name} /> @@ -119,7 +119,7 @@ export function FormScreen(props: FormScreenProps) { label="Email" onChangeText={(value) => props.onChange('email', value)} onSubmitEditing={() => Keyboard.dismiss()} - placeholder="ada@example.com" + placeholder="Type your email" returnKeyType="done" testID="field-email" value={props.form.email} diff --git a/src/__tests__/fixture-fill-placeholder-collision.test.ts b/src/__tests__/fixture-fill-placeholder-collision.test.ts new file mode 100644 index 000000000..2e03e67df --- /dev/null +++ b/src/__tests__/fixture-fill-placeholder-collision.test.ts @@ -0,0 +1,83 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test } from 'vitest'; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const formScreenPath = path.join(repoRoot, 'examples/test-app/src/screens/FormScreen.tsx'); + +// #1874, `smoke:form-input` half: a `fill` whose value equals the target field's placeholder can +// never be confirmed. An empty text field renders its placeholder AS its accessibility value, so +// `element.value` is byte-identical whether the placeholder is showing or the committed text +// happens to match it — `awaitSynthesizedReplacementCommitOutcome` refuses such a wait outright +// (`textMatchesPlaceholder`), which is the documented contract, and the CLI reference says so. +// +// The fixture used to place `placeholder="Ada Lovelace"` on the field every suite fills with +// "Ada Lovelace". That is only reachable on the synthesized-replacement route, which is gated on +// `xCTestChannelPenalized` — i.e. it fires when the host is loaded — so it read as an intermittent +// simulator flake for months rather than as a fixture that asks for the one thing `fill` cannot +// verify. Frozen replay-compat corpora carry the same fill, so the collision has to be kept out of +// the placeholders rather than out of the values. + +type Field = { testID: string; placeholder: string }; + +/** (testID, placeholder) pairs from the fixture's form screen, in source order. */ +function fixtureFields(): Field[] { + const source = fs.readFileSync(formScreenPath, 'utf8'); + const fields: Field[] = []; + const blocks = source.split(' { + const fields = fixtureFields(); + assert.ok(fields.length > 0, 'the form fixture must expose placeheld text fields to check'); + + const placeholderByTestID = new Map(fields.map((field) => [field.testID, field.placeholder])); + const collisions = repositoryFills() + .filter(({ testID, value }) => { + const placeholder = placeholderByTestID.get(testID); + // Mirrors RunnerTests+TextEntry.swift `textMatchesPlaceholder`: trimmed, whole-value equality. + return ( + placeholder !== undefined && placeholder.trim() === value.trim() && value.trim() !== '' + ); + }) + .map(({ source, testID, value }) => `${source}: fill id="${testID}" "${value}"`); + + assert.deepEqual( + collisions, + [], + 'change the fixture placeholder, not the filled value — frozen replay-compat corpora carry these fills', + ); +}); From 3067836f22a5ff66a8dfd15171f0b0597ec63abe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 26 Aug 2026 10:51:16 +0200 Subject: [PATCH 5/7] refactor(ios): drop the fill/placeholder source guard and flatten the commit deadline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review: the 83-line guard was a source-reconstruction test, not a fixture invariant. It regex-parsed JSX and two literal fill spellings and duplicated the Swift trim/equality rule in TypeScript, so it could stay green while its "every fill" claim was false — expressions, variables, typed clients and unlisted roots are all outside what a regex can enumerate. The owning evidence already exists: the Swift tests prove a placeholder-equal AX value is unobservable, and live smoke:form-input failed on the prior head for exactly this collision. Deleted; the two placeholder changes stay. Same pass over the rest of the change, for the same reason. The commit deadline was a budget value type, a nested Deadline type and a factory method; it is now one flat struct the poll loop constructs, with the two durations as defaulted parameters. Production call sites name no budget at all, tests name one only when they are asking about time, and SynthesizedCommitBudget.standard and the tests' unboundedCommitBudget both disappear. --- .../RunnerTests+SynthesizedCommitBudget.swift | 73 ---------------- ...unnerTests+SynthesizedCommitDeadline.swift | 56 +++++++++++++ .../RunnerTests+SynthesizedTextEntry.swift | 15 ++-- .../RunnerTests+TextEntry.swift | 2 +- .../RunnerTests+TextEntryPolicyTests.swift | 49 ++++++----- ...fixture-fill-placeholder-collision.test.ts | 83 ------------------- 6 files changed, 89 insertions(+), 189 deletions(-) delete mode 100644 apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift create mode 100644 apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitDeadline.swift delete mode 100644 src/__tests__/fixture-fill-placeholder-collision.test.ts diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift deleted file mode 100644 index 581025307..000000000 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitBudget.swift +++ /dev/null @@ -1,73 +0,0 @@ -import XCTest - -// How long the synthesized text-entry commit wait keeps looking. Split out of -// RunnerTests+SynthesizedTextEntry.swift only to keep that file inside its size budget; the -// policy is consumed exclusively by the two commit waits there, which is also where it is tested. -extension RunnerTests { - /// How long a commit wait is allowed to keep looking. - /// - /// A synthesized burst can be *throttled* rather than dropped: on a loaded simulator the - /// characters keep landing, just slowly, and everything touching the input system slows with - /// them (#1874). A flat wall-clock budget cannot tell that apart from a wedged pipeline, so it - /// condemned both at the same instant — and the throttled case is a working command reported as - /// `TEXT_INPUT_COMMIT_NOT_OBSERVED`, which is what turned an environment episode into a red - /// lane on branches touching no iOS code. - /// - /// So time is granted against *progress* — the observed value's expected-prefix growing, the - /// same length-only evidence `logCommitCadence` already emits — with an absolute ceiling, so a - /// pipeline that delivers one character per stall window cannot hold a command open forever. - /// - /// A pipeline making no progress at all still expires at exactly the `stallBudget` the flat - /// deadline used, so nothing that fails today starts passing merely by waiting longer: the wait - /// extends only while characters are still arriving. - struct SynthesizedCommitBudget { - let stallBudget: TimeInterval - let ceiling: TimeInterval - - /// The budget the shipped `type`/`fill` waits run under. - static let standard = SynthesizedCommitBudget( - stallBudget: TextEntryTiming.synthesizedCommitStallTimeout, - ceiling: TextEntryTiming.synthesizedCommitCeiling - ) - - /// Starts the clock, and only the poll loop may: the wait's setup reads the field's - /// placeholder first, which is an AX round-trip that takes seconds on exactly the loaded host - /// this budget exists for. Time spent there is not time the field was given to commit, and - /// charging it to the budget made the wait give up sooner than the flat deadline it replaced. - func deadline(startedAt: Date) -> Deadline { - Deadline(budget: self, startedAt: startedAt) - } - - /// One wait's running deadline. Held as a local `var` by the loop that started it, so - /// recording progress and asking whether time is up are two statements in one function - /// rather than a coupling between separately-held state. - struct Deadline { - private let budget: SynthesizedCommitBudget - private let startedAt: Date - // Nothing has landed yet, and an observation of "still nothing" must not read as progress. - private var bestPrefixLength = 0 - private var lastProgressAt: Date - - init(budget: SynthesizedCommitBudget, startedAt: Date) { - self.budget = budget - self.startedAt = startedAt - self.lastProgressAt = startedAt - } - - /// Records one observation's expected-prefix length. Only forward movement counts: a - /// shorter read — the app clearing the field mid-flight, or a value that could not be read - /// at all, which measures as 0 — is not evidence the burst is still landing, so it neither - /// buys time nor takes any back. - mutating func record(expectedPrefixLength: Int, at now: Date) { - guard expectedPrefixLength > bestPrefixLength else { return } - bestPrefixLength = expectedPrefixLength - lastProgressAt = now - } - - func isExpired(at now: Date) -> Bool { - now.timeIntervalSince(startedAt) >= budget.ceiling - || now.timeIntervalSince(lastProgressAt) >= budget.stallBudget - } - } - } -} diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitDeadline.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitDeadline.swift new file mode 100644 index 000000000..921adb4c9 --- /dev/null +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitDeadline.swift @@ -0,0 +1,56 @@ +import XCTest + +// How long the synthesized text-entry commit wait keeps looking. Split out of +// RunnerTests+SynthesizedTextEntry.swift only to keep that file inside its size budget; the two +// commit waits there are the sole users, and where this is tested. +extension RunnerTests { + /// One commit wait's running deadline. + /// + /// A synthesized burst can be *throttled* rather than dropped: on a loaded simulator the + /// characters keep landing, just slowly, and everything touching the input system slows with + /// them (#1874). A flat wall-clock budget cannot tell that apart from a wedged pipeline, so it + /// condemned both at the same instant — and the throttled case is a working command reported as + /// `TEXT_INPUT_COMMIT_NOT_OBSERVED`, which is what turned an environment episode into a red + /// lane on branches touching no iOS code. + /// + /// So time is granted against *progress* — the observed value's expected-prefix growing, the + /// same length-only evidence `logCommitCadence` already emits — with an absolute `ceiling`, so a + /// pipeline delivering one character per stall window cannot hold a command open forever. A + /// pipeline making no progress at all still expires at exactly the `stallBudget` the flat + /// deadline used, so nothing that fails today starts passing merely by waiting longer. + /// + /// Started by the poll loop from its own first `now()`, and held as a local `var` there. The + /// wait's setup reads the field's placeholder first, an AX round-trip that takes seconds on + /// exactly the loaded host this exists for; charging that to the deadline made the wait give up + /// sooner than the flat budget it replaced. + struct SynthesizedCommitDeadline { + private let startedAt: Date + private let stallBudget: TimeInterval + private let ceiling: TimeInterval + // Nothing has landed yet, and an observation of "still nothing" must not read as progress. + private var bestPrefixLength = 0 + private var lastProgressAt: Date + + init(startedAt: Date, stallBudget: TimeInterval, ceiling: TimeInterval) { + self.startedAt = startedAt + self.stallBudget = stallBudget + self.ceiling = ceiling + self.lastProgressAt = startedAt + } + + /// Records one observation's expected-prefix length. Only forward movement counts: a shorter + /// read — the app clearing the field mid-flight, or a value that could not be read at all, + /// which measures as 0 — is not evidence the burst is still landing, so it neither buys time + /// nor takes any back. + mutating func record(expectedPrefixLength: Int, at now: Date) { + guard expectedPrefixLength > bestPrefixLength else { return } + bestPrefixLength = expectedPrefixLength + lastProgressAt = now + } + + func isExpired(at now: Date) -> Bool { + now.timeIntervalSince(startedAt) >= ceiling + || now.timeIntervalSince(lastProgressAt) >= stallBudget + } + } +} diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift index 1ae7c972d..7609bbaab 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift @@ -266,11 +266,13 @@ extension RunnerTests { /// /// The deadline is a local `var`, started from this loop's own first `now()` and advanced from /// the same observation the progress check reads, so "did the burst move" and "is time up" are - /// two statements in one loop rather than a coupling between separately-held state. + /// two statements in one loop. The budget defaults to the shipped one, so only a test that is + /// asking about time has to name it. static func awaitSynthesizedCommitOutcome( expectedText: String, placeholder: String?, - budget: SynthesizedCommitBudget, + stallBudget: TimeInterval = TextEntryTiming.synthesizedCommitStallTimeout, + ceiling: TimeInterval = TextEntryTiming.synthesizedCommitCeiling, now: () -> Date, observe: () -> String?, waitForNextObservation: () -> Void @@ -281,7 +283,7 @@ extension RunnerTests { if Self.textMatchesPlaceholder(expectedText, placeholder: placeholder) { return .notObserved } - var deadline = budget.deadline(startedAt: now()) + var deadline = SynthesizedCommitDeadline(startedAt: now(), stallBudget: stallBudget, ceiling: ceiling) // The deadline is checked AFTER an observation, never before one, so the last thing that // happens before condemning a commit is a read. Checking first would condemn a commit that // landed during the final poll sleep — the exact loaded-host timing this wait exists for. @@ -337,7 +339,8 @@ extension RunnerTests { static func awaitSynthesizedReplacementCommitOutcome( expectedText: String, placeholder: String?, - budget: SynthesizedCommitBudget, + stallBudget: TimeInterval = TextEntryTiming.synthesizedCommitStallTimeout, + ceiling: TimeInterval = TextEntryTiming.synthesizedCommitCeiling, now: () -> Date, observe: () -> String?, waitForNextObservation: () -> Void @@ -345,7 +348,7 @@ extension RunnerTests { if Self.textMatchesPlaceholder(expectedText, placeholder: placeholder) { return .notObserved } - var deadline = budget.deadline(startedAt: now()) + var deadline = SynthesizedCommitDeadline(startedAt: now(), stallBudget: stallBudget, ceiling: ceiling) while true { let observedText = observe() if observedText == expectedText { @@ -434,7 +437,6 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: expectedText, placeholder: ingredients.placeholder, - budget: .standard, now: { Date() }, observe: ingredients.observe, waitForNextObservation: ingredients.waitForNextObservation @@ -471,7 +473,6 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: expectedText, placeholder: ingredients.placeholder, - budget: .standard, now: { Date() }, observe: ingredients.observe, waitForNextObservation: ingredients.waitForNextObservation diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift index b46a1c163..cd333a9f3 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift @@ -47,7 +47,7 @@ extension RunnerTests { static let verificationStabilityWindow: TimeInterval = 0.2 /// How long the commit wait tolerates seeing NO further progress toward the expected value. /// Numerically the flat deadline this replaced, so a pipeline that delivers nothing is - /// condemned at exactly the same instant it always was (see `SynthesizedCommitBudget`). + /// condemned at exactly the same instant it always was (see `SynthesizedCommitDeadline`). static let synthesizedCommitStallTimeout: TimeInterval = 3.0 /// The commit wait's absolute bound, however long characters keep arriving. Sits well inside /// the daemon's per-command budget (`RUNNER_COMMAND_TIMEOUT_MS`, 45s), which also has to cover diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift index 412b92595..97a26ff4a 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift @@ -92,7 +92,8 @@ extension RunnerTests { /// A hand-driven clock for the commit waits. Time moves only where the wait sleeps, which is /// what makes "the burst kept landing" and "the pipeline froze" expressible as two sequences of - /// the same length rather than as wall-clock luck. + /// the same length rather than as wall-clock luck — and a test that never advances it cannot + /// expire any budget, so only a test asking about time names `stallBudget`/`ceiling`. final class CommitWaitClock { private let origin = Date(timeIntervalSinceReferenceDate: 0) private var current = Date(timeIntervalSinceReferenceDate: 0) @@ -107,10 +108,6 @@ extension RunnerTests { var elapsed: TimeInterval { current.timeIntervalSince(origin) } } - /// A budget no test can exhaust, so a test that never advances the clock is asking about - /// settling rather than about time. - static let unboundedCommitBudget = SynthesizedCommitBudget(stallBudget: 3600, ceiling: 3600) - // #1874, through the shipped wait rather than a detached policy object: a burst that keeps // landing must outlive the flat 3s deadline that used to govern it. Each poll advances the // clock 2s and delivers one more character, so the wait is never idle for a full stall budget @@ -123,7 +120,8 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: expected, placeholder: nil, - budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), + stallBudget: 3, + ceiling: 10, now: clock.read, observe: { String(expected.prefix(landed * 2)) }, waitForNextObservation: { @@ -148,7 +146,8 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware", placeholder: nil, - budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), + stallBudget: 3, + ceiling: 10, now: clock.read, observe: { "ha" }, waitForNextObservation: { @@ -168,7 +167,8 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware", placeholder: nil, - budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), + stallBudget: 3, + ceiling: 10, now: clock.read, observe: { "ha" }, waitForNextObservation: { clock.advance(1) } @@ -186,7 +186,8 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: String(repeating: "a", count: 100), placeholder: nil, - budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), + stallBudget: 3, + ceiling: 10, now: clock.read, observe: { String(repeating: "a", count: landed) }, waitForNextObservation: { @@ -209,7 +210,8 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: "ada@example", placeholder: nil, - budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), + stallBudget: 3, + ceiling: 10, now: clock.read, observe: { observations[min(index, observations.count - 1)] }, waitForNextObservation: { @@ -230,7 +232,8 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware-keyboard", placeholder: nil, - budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), + stallBudget: 3, + ceiling: 10, now: clock.read, observe: { "h" }, waitForNextObservation: { @@ -249,8 +252,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware-keyboard", placeholder: nil, - budget: Self.unboundedCommitBudget, - now: clock.read, + now: clock.read, observe: { observed }, waitForNextObservation: { polls += 1 } ) @@ -268,7 +270,6 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware-keyboard", placeholder: nil, - budget: Self.unboundedCommitBudget, now: clock.read, observe: { steps[min(index, steps.count - 1)] }, waitForNextObservation: { index += 1 } @@ -286,7 +287,8 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware-keyboard", placeholder: nil, - budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), + stallBudget: 3, + ceiling: 10, now: clock.read, // The value lands during the sleep that takes the clock past the stall budget: the read // happens first, so it is still observed. @@ -311,7 +313,6 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: expectedText, placeholder: "0.00", - budget: Self.unboundedCommitBudget, now: clock.read, observe: { observations += 1 @@ -341,8 +342,7 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: testCase.expectedText, placeholder: testCase.placeholder, - budget: Self.unboundedCommitBudget, - now: clock.read, + now: clock.read, observe: { testCase.expectedText }, waitForNextObservation: {} ) @@ -371,8 +371,7 @@ extension RunnerTests { Self.awaitSynthesizedCommitOutcome( expectedText: corruption.expected, placeholder: nil, - budget: Self.unboundedCommitBudget, - now: appendClock.read, + now: appendClock.read, observe: { corruption.observedAfterDrop }, waitForNextObservation: {} ), @@ -384,7 +383,8 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: corruption.expected, placeholder: nil, - budget: SynthesizedCommitBudget(stallBudget: 2, ceiling: 10), + stallBudget: 2, + ceiling: 10, now: clock.read, observe: { corruption.observedAfterDrop }, waitForNextObservation: { @@ -409,7 +409,6 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: "ada@example", placeholder: nil, - budget: Self.unboundedCommitBudget, now: clock.read, observe: { steps[min(index, steps.count - 1)] }, waitForNextObservation: { index += 1 } @@ -426,7 +425,8 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: "ada@example", placeholder: nil, - budget: SynthesizedCommitBudget(stallBudget: 3, ceiling: 10), + stallBudget: 3, + ceiling: 10, now: clock.read, observe: { polls == 0 ? "ada@exampl" : "ada@example" }, waitForNextObservation: { @@ -445,7 +445,6 @@ extension RunnerTests { let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: "0.00", placeholder: "0.00", - budget: Self.unboundedCommitBudget, now: clock.read, observe: { observations += 1 @@ -584,7 +583,7 @@ extension RunnerTests { // correctly a failure (see `testSynthesizedReplacementCommitCatchesDroppedMiddleCharacters` for // why it must NOT be waved through as success), so this call runs the real 3-second deadline // (`TextEntryTiming.synthesizedCommitStallTimeout`; a nil read never advances the expected - // prefix, so `SynthesizedCommitBudget` grants it no extra time) before returning. That is + // prefix, so `SynthesizedCommitDeadline` grants it no extra time) before returning. That is // deliberate here, not a flake: this test only runs in the nightly XCUITest lane (see // `runner-xctest-local-run-gotchas` memory / ios.yml's `-only-testing:` allowlist), where a // few extra seconds is a non-issue, and the alternative — asserting `nil` on a wiring path diff --git a/src/__tests__/fixture-fill-placeholder-collision.test.ts b/src/__tests__/fixture-fill-placeholder-collision.test.ts deleted file mode 100644 index 2e03e67df..000000000 --- a/src/__tests__/fixture-fill-placeholder-collision.test.ts +++ /dev/null @@ -1,83 +0,0 @@ -import assert from 'node:assert/strict'; -import fs from 'node:fs'; -import path from 'node:path'; -import { fileURLToPath } from 'node:url'; -import { test } from 'vitest'; - -const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); -const formScreenPath = path.join(repoRoot, 'examples/test-app/src/screens/FormScreen.tsx'); - -// #1874, `smoke:form-input` half: a `fill` whose value equals the target field's placeholder can -// never be confirmed. An empty text field renders its placeholder AS its accessibility value, so -// `element.value` is byte-identical whether the placeholder is showing or the committed text -// happens to match it — `awaitSynthesizedReplacementCommitOutcome` refuses such a wait outright -// (`textMatchesPlaceholder`), which is the documented contract, and the CLI reference says so. -// -// The fixture used to place `placeholder="Ada Lovelace"` on the field every suite fills with -// "Ada Lovelace". That is only reachable on the synthesized-replacement route, which is gated on -// `xCTestChannelPenalized` — i.e. it fires when the host is loaded — so it read as an intermittent -// simulator flake for months rather than as a fixture that asks for the one thing `fill` cannot -// verify. Frozen replay-compat corpora carry the same fill, so the collision has to be kept out of -// the placeholders rather than out of the values. - -type Field = { testID: string; placeholder: string }; - -/** (testID, placeholder) pairs from the fixture's form screen, in source order. */ -function fixtureFields(): Field[] { - const source = fs.readFileSync(formScreenPath, 'utf8'); - const fields: Field[] = []; - const blocks = source.split(' { - const fields = fixtureFields(); - assert.ok(fields.length > 0, 'the form fixture must expose placeheld text fields to check'); - - const placeholderByTestID = new Map(fields.map((field) => [field.testID, field.placeholder])); - const collisions = repositoryFills() - .filter(({ testID, value }) => { - const placeholder = placeholderByTestID.get(testID); - // Mirrors RunnerTests+TextEntry.swift `textMatchesPlaceholder`: trimmed, whole-value equality. - return ( - placeholder !== undefined && placeholder.trim() === value.trim() && value.trim() !== '' - ); - }) - .map(({ source, testID, value }) => `${source}: fill id="${testID}" "${value}"`); - - assert.deepEqual( - collisions, - [], - 'change the fixture placeholder, not the filled value — frozen replay-compat corpora carry these fills', - ); -}); From 44cc2149f2bb2be785b88345beb7cbf5b34a23b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 26 Aug 2026 15:24:04 +0200 Subject: [PATCH 6/7] refactor(ios): split the text-entry readiness and commit-wait seams MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review: the change grew three files past their budgets. Splitting them along the seams they already had, no behavior change. RunnerTests+TextEntry.swift (607) keeps the vocabulary, field clearing and value reading at 259; everything that decides "which element is about to receive text, and has it taken focus" moves to RunnerTests+TextEntryReadiness.swift at 354. RunnerTests+SynthesizedTextEntry.swift (503) keeps the private-XCTest synthesis boundary, the replacement route and the route policies at 356. The commit wait moves next to the deadline that bounds it: the two waits, the observation and pacing they poll through, and the value-free cadence line that path may log now sit together in RunnerTests+SynthesizedCommitDeadline.swift at 206. That also puts every line touching the polled field value in one file, so apple-runner-log-redaction.test.ts guards a single surface — its path constant moves with it. The deadline's clock and sequence tests leave the policy tests (641 -> 494) for a sibling RunnerTests+SynthesizedCommitDeadlineTests.swift, which gains the replacement-route case the review asked for: a growing prefix carries the wait past the 3s stall budget and the 10s ceiling is what ends it. The injected clock is now defaulted, so only a test actually asking about time names it. --- ...unnerTests+SynthesizedCommitDeadline.swift | 156 +++++++- .../RunnerTests+SynthesizedTextEntry.swift | 151 +------- .../RunnerTests+TextEntry.swift | 348 ----------------- .../RunnerTests+TextEntryReadiness.swift | 354 ++++++++++++++++++ ...Tests+SynthesizedCommitDeadlineTests.swift | 164 ++++++++ .../RunnerTests+TextEntryPolicyTests.swift | 147 -------- .../apple-runner-log-redaction.test.ts | 6 +- 7 files changed, 676 insertions(+), 650 deletions(-) create mode 100644 apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntryReadiness.swift create mode 100644 apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SynthesizedCommitDeadlineTests.swift diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitDeadline.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitDeadline.swift index 921adb4c9..b88d2d5aa 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitDeadline.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitDeadline.swift @@ -1,8 +1,11 @@ import XCTest -// How long the synthesized text-entry commit wait keeps looking. Split out of -// RunnerTests+SynthesizedTextEntry.swift only to keep that file inside its size budget; the two -// commit waits there are the sole users, and where this is tested. +// The synthesized text-entry commit wait, end to end: the deadline that bounds it, the two waits +// that run it, the observation/pacing they poll through, and the value-free cadence line that path +// is allowed to log. Split from RunnerTests+SynthesizedTextEntry.swift, which keeps the +// private-XCTest synthesis boundary and the route policies; the pure outcome functions these wrap +// stay there next to the rules they encode. Everything that touches the polled field value now +// lives in this one file, which is the surface apple-runner-log-redaction.test.ts guards. extension RunnerTests { /// One commit wait's running deadline. /// @@ -53,4 +56,151 @@ extension RunnerTests { || now.timeIntervalSince(lastProgressAt) >= stallBudget } } + + /// Blocks until the synthesized bare-type text is observable in the target field, so `type` + /// cannot report ok while trailing characters are still uncommitted on a slow simulator. + /// + /// Observation only. A stalled prefix cannot be told apart from a suffix still queued in the + /// event stream, so re-synthesizing the difference risks committing it twice after the command + /// already reported success (#1676 rejected exactly that repair). Reporting `.notObserved` is + /// what the caller does instead: the partial value is the agent's to resolve, and a named + /// failure beats a success that misdescribes the field. Text carrying a submit key is skipped + /// outright: the app may clear or rewrite the field on submit, so `textBefore + typedText` is + /// not the value to wait for. + func awaitSynthesizedFirstResponderCommit( + app: XCUIApplication, + target: TextEntryTarget, + textBefore: String?, + typedText: String + ) -> SynthesizedTextCommitOutcome { + guard let textBefore, !typedText.contains("\n"), !typedText.contains("\r") else { + return .unobservable + } + let expectedText = textBefore + typedText + let waitStartedAt = Date() + NSLog("[DEBUG-1874] wait start expectedLen=%ld route=append", expectedText.count) + let ingredients = synthesizedCommitPollingIngredients(app: app, target: target, expectedText: expectedText) + let outcome = Self.awaitSynthesizedCommitOutcome( + expectedText: expectedText, + placeholder: ingredients.placeholder, + now: { Date() }, + observe: ingredients.observe, + waitForNextObservation: ingredients.waitForNextObservation + ) + NSLog( + "[DEBUG-1874] wait outcome=%@ elapsedMs=%.0f route=append", + String(describing: outcome), + waitStartedAt.timeIntervalSinceNow * -1000 + ) + return outcome + } + + /// Blocks until the synthesized replacement text (`fill`) is observable in the target field, so + /// `fill` cannot report ok while the select-all-and-retype it posted is still uncommitted — or + /// silently wrong — on a slow or channel-penalized simulator (this route runs only when the + /// XCTest channel is already penalized, and never resolves an `XCUIElement`, so it previously had + /// no verification at all). + /// + /// Unlike the append route, the expected value is the final text itself — replacement mode + /// clears the field first, so there is no `textBefore` prefix to account for — and unlike the + /// append route, a settled mismatch is always reported rather than trusted: see + /// `awaitSynthesizedReplacementCommitOutcome`'s doc comment. + func awaitSynthesizedReplacementCommit( + app: XCUIApplication, + target: TextEntryTarget, + expectedText: String + ) -> SynthesizedTextCommitOutcome { + guard !expectedText.contains("\n"), !expectedText.contains("\r") else { + return .unobservable + } + let waitStartedAt = Date() + NSLog("[DEBUG-1874] wait start expectedLen=%ld route=replacement", expectedText.count) + let ingredients = synthesizedCommitPollingIngredients(app: app, target: target, expectedText: expectedText) + let outcome = Self.awaitSynthesizedReplacementCommitOutcome( + expectedText: expectedText, + placeholder: ingredients.placeholder, + now: { Date() }, + observe: ingredients.observe, + waitForNextObservation: ingredients.waitForNextObservation + ) + NSLog( + "[DEBUG-1874] wait outcome=%@ elapsedMs=%.0f route=replacement", + String(describing: outcome), + waitStartedAt.timeIntervalSinceNow * -1000 + ) + return outcome + } + + /// The emitted cadence line, as a pure function so its output is assertable. Only lengths and + /// a timestamp are representable here; there is no String parameter, so observed field + /// contents cannot reach runner.log through this boundary whatever they contain. + static func commitCadenceLogLine( + elapsedMs: Int, + observedLen: Int, + expectedPrefixLen: Int + ) -> String { + "[DEBUG-1874] poll t=\(elapsedMs)ms observedLen=\(observedLen) expectedPrefixLen=\(expectedPrefixLen)" + } + + /// The typed boundary for commit-wait cadence evidence. The poll path must log through this + /// function and never through a raw NSLog: every parameter is an Int, so the polled value's + /// contents are unrepresentable at the call site. + static func logCommitCadence( + elapsedMs: Int, + observedLen: Int, + expectedPrefixLen: Int + ) { + NSLog( + "%@", + commitCadenceLogLine( + elapsedMs: elapsedMs, + observedLen: observedLen, + expectedPrefixLen: expectedPrefixLen + ) + ) + } + + /// The placeholder/observe/pacing ingredients shared by the append route + /// (`awaitSynthesizedFirstResponderCommit`) and the replacement route + /// (`awaitSynthesizedReplacementCommit`). What must NOT be shared is which outcome function + /// consumes them: see `awaitSynthesizedReplacementCommitOutcome`'s doc comment for why append + /// mode's "trust a diverged value" rule is wrong for replacement mode. Each caller therefore + /// calls its own named outcome function directly, with real argument labels — deliberately not + /// a stored closure/function-value parameter here, which would erase those labels at the call + /// site and make the observe closure unrecognizable to the static content-redaction check in + /// `apple-runner-log-redaction.test.ts` (`extractObserveClosure` locates the labeled closure + /// literal by its text; a closure passed as a plain function value carries no such label). + private func synthesizedCommitPollingIngredients( + app: XCUIApplication, + target: TextEntryTarget, + expectedText: String + ) -> ( + placeholder: String?, + observe: () -> String?, + waitForNextObservation: () -> Void + ) { + let placeholder = resolveTextEntryElement(app: app, target: target)?.placeholderValue + let waitStartedAt = Date() + return ( + placeholder: placeholder, + observe: { + let observedText = self.editableTextValue( + for: self.resolveTextEntryElement(app: app, target: target), + treatingPlaceholderAsEmpty: true + ) + // Cadence evidence stays value-free: the polled value is user content typed through + // `type`/`fill` and must never reach runner.log. Lengths and the expected-prefix walk + // are enough to distinguish throttling (prefix grows slowly) from a wedge (it freezes). + Self.logCommitCadence( + elapsedMs: Int(waitStartedAt.timeIntervalSinceNow * -1000), + observedLen: observedText?.count ?? -1, + expectedPrefixLen: observedText.map { Self.commonPrefixLength($0, expectedText) } ?? -1 + ) + return observedText + }, + // XCUI resolution shares the automation channel with the in-flight synthesized event. + // Sparse reads let the target consume that event instead of continuously interrupting it. + waitForNextObservation: { self.sleepFor(TextEntryTiming.synthesizedCommitPollInterval) } + ) + } } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift index 7609bbaab..357af4f8d 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift @@ -213,35 +213,6 @@ extension RunnerTests { return length } - /// The emitted cadence line, as a pure function so its output is assertable. Only lengths and - /// a timestamp are representable here; there is no String parameter, so observed field - /// contents cannot reach runner.log through this boundary whatever they contain. - static func commitCadenceLogLine( - elapsedMs: Int, - observedLen: Int, - expectedPrefixLen: Int - ) -> String { - "[DEBUG-1874] poll t=\(elapsedMs)ms observedLen=\(observedLen) expectedPrefixLen=\(expectedPrefixLen)" - } - - /// The typed boundary for commit-wait cadence evidence. The poll path must log through this - /// function and never through a raw NSLog: every parameter is an Int, so the polled value's - /// contents are unrepresentable at the call site. - static func logCommitCadence( - elapsedMs: Int, - observedLen: Int, - expectedPrefixLen: Int - ) { - NSLog( - "%@", - commitCadenceLogLine( - elapsedMs: elapsedMs, - observedLen: observedLen, - expectedPrefixLen: expectedPrefixLen - ) - ) - } - /// How the commit wait ended. Distinct from `SynthesizedTextCommitProgress`, which classifies a /// single observation: this is the whole wait's verdict, and it exists so the deadline can be /// told apart from success. The wait used to return `Void`, which made an expired deadline @@ -273,7 +244,7 @@ extension RunnerTests { placeholder: String?, stallBudget: TimeInterval = TextEntryTiming.synthesizedCommitStallTimeout, ceiling: TimeInterval = TextEntryTiming.synthesizedCommitCeiling, - now: () -> Date, + now: () -> Date = { Date() }, observe: () -> String?, waitForNextObservation: () -> Void ) -> SynthesizedTextCommitOutcome { @@ -341,7 +312,7 @@ extension RunnerTests { placeholder: String?, stallBudget: TimeInterval = TextEntryTiming.synthesizedCommitStallTimeout, ceiling: TimeInterval = TextEntryTiming.synthesizedCommitCeiling, - now: () -> Date, + now: () -> Date = { Date() }, observe: () -> String?, waitForNextObservation: () -> Void ) -> SynthesizedTextCommitOutcome { @@ -367,124 +338,6 @@ extension RunnerTests { } } - /// The placeholder/observe/pacing ingredients shared by the append route - /// (`awaitSynthesizedFirstResponderCommit`) and the replacement route - /// (`awaitSynthesizedReplacementCommit`). What must NOT be shared is which outcome function - /// consumes them: see `awaitSynthesizedReplacementCommitOutcome`'s doc comment for why append - /// mode's "trust a diverged value" rule is wrong for replacement mode. Each caller therefore - /// calls its own named outcome function directly, with real argument labels — deliberately not - /// a stored closure/function-value parameter here, which would erase those labels at the call - /// site and make the observe closure unrecognizable to the static content-redaction check in - /// `apple-runner-log-redaction.test.ts` (`extractObserveClosure` locates the labeled closure - /// literal by its text; a closure passed as a plain function value carries no such label). - private func synthesizedCommitPollingIngredients( - app: XCUIApplication, - target: TextEntryTarget, - expectedText: String - ) -> ( - placeholder: String?, - observe: () -> String?, - waitForNextObservation: () -> Void - ) { - let placeholder = resolveTextEntryElement(app: app, target: target)?.placeholderValue - let waitStartedAt = Date() - return ( - placeholder: placeholder, - observe: { - let observedText = self.editableTextValue( - for: self.resolveTextEntryElement(app: app, target: target), - treatingPlaceholderAsEmpty: true - ) - // Cadence evidence stays value-free: the polled value is user content typed through - // `type`/`fill` and must never reach runner.log. Lengths and the expected-prefix walk - // are enough to distinguish throttling (prefix grows slowly) from a wedge (it freezes). - Self.logCommitCadence( - elapsedMs: Int(waitStartedAt.timeIntervalSinceNow * -1000), - observedLen: observedText?.count ?? -1, - expectedPrefixLen: observedText.map { Self.commonPrefixLength($0, expectedText) } ?? -1 - ) - return observedText - }, - // XCUI resolution shares the automation channel with the in-flight synthesized event. - // Sparse reads let the target consume that event instead of continuously interrupting it. - waitForNextObservation: { self.sleepFor(TextEntryTiming.synthesizedCommitPollInterval) } - ) - } - - /// Blocks until the synthesized bare-type text is observable in the target field, so `type` - /// cannot report ok while trailing characters are still uncommitted on a slow simulator. - /// - /// Observation only. A stalled prefix cannot be told apart from a suffix still queued in the - /// event stream, so re-synthesizing the difference risks committing it twice after the command - /// already reported success (#1676 rejected exactly that repair). Reporting `.notObserved` is - /// what the caller does instead: the partial value is the agent's to resolve, and a named - /// failure beats a success that misdescribes the field. Text carrying a submit key is skipped - /// outright: the app may clear or rewrite the field on submit, so `textBefore + typedText` is - /// not the value to wait for. - func awaitSynthesizedFirstResponderCommit( - app: XCUIApplication, - target: TextEntryTarget, - textBefore: String?, - typedText: String - ) -> SynthesizedTextCommitOutcome { - guard let textBefore, !typedText.contains("\n"), !typedText.contains("\r") else { - return .unobservable - } - let expectedText = textBefore + typedText - let waitStartedAt = Date() - NSLog("[DEBUG-1874] wait start expectedLen=%ld route=append", expectedText.count) - let ingredients = synthesizedCommitPollingIngredients(app: app, target: target, expectedText: expectedText) - let outcome = Self.awaitSynthesizedCommitOutcome( - expectedText: expectedText, - placeholder: ingredients.placeholder, - now: { Date() }, - observe: ingredients.observe, - waitForNextObservation: ingredients.waitForNextObservation - ) - NSLog( - "[DEBUG-1874] wait outcome=%@ elapsedMs=%.0f route=append", - String(describing: outcome), - waitStartedAt.timeIntervalSinceNow * -1000 - ) - return outcome - } - - /// Blocks until the synthesized replacement text (`fill`) is observable in the target field, so - /// `fill` cannot report ok while the select-all-and-retype it posted is still uncommitted — or - /// silently wrong — on a slow or channel-penalized simulator (this route runs only when the - /// XCTest channel is already penalized, and never resolves an `XCUIElement`, so it previously had - /// no verification at all). - /// - /// Unlike the append route, the expected value is the final text itself — replacement mode - /// clears the field first, so there is no `textBefore` prefix to account for — and unlike the - /// append route, a settled mismatch is always reported rather than trusted: see - /// `awaitSynthesizedReplacementCommitOutcome`'s doc comment. - func awaitSynthesizedReplacementCommit( - app: XCUIApplication, - target: TextEntryTarget, - expectedText: String - ) -> SynthesizedTextCommitOutcome { - guard !expectedText.contains("\n"), !expectedText.contains("\r") else { - return .unobservable - } - let waitStartedAt = Date() - NSLog("[DEBUG-1874] wait start expectedLen=%ld route=replacement", expectedText.count) - let ingredients = synthesizedCommitPollingIngredients(app: app, target: target, expectedText: expectedText) - let outcome = Self.awaitSynthesizedReplacementCommitOutcome( - expectedText: expectedText, - placeholder: ingredients.placeholder, - now: { Date() }, - observe: ingredients.observe, - waitForNextObservation: ingredients.waitForNextObservation - ) - NSLog( - "[DEBUG-1874] wait outcome=%@ elapsedMs=%.0f route=replacement", - String(describing: outcome), - waitStartedAt.timeIntervalSinceNow * -1000 - ) - return outcome - } - static func shouldUseResolvedCoordinateTextEntryRoute( repairMode: TextTypingRepairMode, hasX: Bool, diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift index cd333a9f3..661550362 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift @@ -134,211 +134,6 @@ extension RunnerTests { element.typeText(deletes) } - func focusedTextInput(app: XCUIApplication) -> XCUIElement? { -#if os(iOS) - // iOS focus predicates can return stale or misleading text-input matches - // under XCUITest, so text entry readiness is driven by tap/keyboard state. - return nil -#else - return safely("FOCUSED_INPUT_QUERY") { - let candidates = app - .descendants(matching: .any) - .matching(NSPredicate(format: "hasKeyboardFocus == 1")) - .allElementsBoundByIndex - for candidate in candidates where candidate.exists { - switch candidate.elementType { - case .textField, .secureTextField, .searchField, .textView: - return candidate - default: - continue - } - } - return nil - } -#endif - } - - func rememberTextEntryTap(_ element: XCUIElement?) { - guard let element, isTextEntryElement(element) else { - clearRememberedTextEntryTap() - return - } - textEntryTapWitness = TextEntryTapWitness( - element: element, - bundleId: currentBundleId, - processIdentifier: currentAppProcessIdentifier - ) - } - - func clearRememberedTextEntryTap() { - textEntryTapWitness = nil - } - - private func rememberedTextEntryTarget() -> TextEntryTarget? { - guard let witness = textEntryTapWitness else { - return nil - } - // The tap is proof for one immediately-following bare type only. Consume it before checking - // the element so a failed or interrupted type cannot reuse stale focus evidence. - clearRememberedTextEntryTap() - guard witness.matches( - bundleId: currentBundleId, - processIdentifier: currentAppProcessIdentifier - ) else { - return nil - } - let element = witness.element - // XCUIElement is query-backed rather than a stable node identity. A same-identifier field - // introduced by app-side navigation between tap and this immediate type can therefore - // re-resolve here; keep the witness one-shot and fail closed on every observable identity - // boundary instead of using frame equality, which would reject legitimate layout changes. - guard safely("LAST_TAPPED_TEXT_INPUT_EXISTS", false, { element.exists }) else { - return nil - } - // Keep the target scoped to the element that the preceding tap actually selected. Do not - // attach a refresh point: if that element disappeared, bare type must fail closed rather - // than rediscovering a different field or dispatching unscoped app.typeText. - return TextEntryTarget( - element: element, - refreshPoint: nil, - prefersFocusedElement: false, - fromTapWitness: true - ) - } - - func stabilizeTextInputBeforeTyping( - app: XCUIApplication, - target: XCUIElement?, - keyboardVisibleBeforeTap: Bool? = nil - ) -> TextEntryStabilization { -#if os(tvOS) - return TextEntryStabilization(element: target, focusConfirmed: true) -#else - let latest = target - let keyboardVisibleAtEntry = keyboardVisibleBeforeTap ?? isKeyboardVisible(app: app) - let deadline = Date().addingTimeInterval(TextEntryTiming.focusTimeout) - while Date() < deadline { - if let focused = focusedTextInput(app: app) { - return TextEntryStabilization(element: focused, focusConfirmed: true) - } - // focusedTextInput is intentionally nil on iOS; treat the keyboard transitioning to - // visible after our tap as the focus-moved signal. Don't fast-path when it was already up. - if keyboardBecameVisible(app: app, wasVisibleAtEntry: keyboardVisibleAtEntry) { - return TextEntryStabilization(element: latest, focusConfirmed: true) - } - sleepFor(TextEntryTiming.pollInterval) - } - return TextEntryStabilization(element: latest, focusConfirmed: false) -#endif - } - - func focusTextInputForTextEntry(app: XCUIApplication, x: Double?, y: Double?) -> TextEntryTarget { - guard let x, let y else { - let softwareKeyboardVisible = isKeyboardVisible(app: app) - if !softwareKeyboardVisible, let rememberedTarget = rememberedTextEntryTarget() { - return rememberedTarget - } - // Bare `type` targets the current first responder. On iOS we intentionally do not trust - // `hasKeyboardFocus`, but an already-visible software keyboard is sufficient evidence that - // app.typeText has a receiver; waiting the full readiness timeout cannot prove a stronger - // target because there is no selector/coordinate focus move to validate. - if softwareKeyboardVisible { - return TextEntryTarget( - element: focusedTextInput(app: app), - refreshPoint: nil, - prefersFocusedElement: true - ) - } - let focused = waitForTextEntryReadiness( - app: app, - target: TextEntryTarget( - element: focusedTextInput(app: app), - refreshPoint: nil, - prefersFocusedElement: true - ) - ) - return TextEntryTarget(element: focused, refreshPoint: nil, prefersFocusedElement: true) - } - - let keyboardVisibleBeforeTap = isKeyboardVisible(app: app) - let target = textInputAt(app: app, x: x, y: y) - let requestedPoint = CGPoint(x: x, y: y) - if let target { - let frame = target.frame - if !frame.isEmpty { - _ = tapAt(app: app, x: frame.midX, y: frame.midY) - } else { - _ = tapAt(app: app, x: x, y: y) - } - } else { - _ = tapAt(app: app, x: x, y: y) - } - // A visible keyboard is not enough evidence for app.typeText, because focus may still - // belong to a previous field. With a concrete target we type through XCUIElement.typeText, - // so after tapping it the iOS readiness timeout cannot prove anything stronger. - if keyboardVisibleBeforeTap, let target { - return TextEntryTarget( - element: target, - refreshPoint: textEntryRefreshPoint(for: target) ?? requestedPoint, - prefersFocusedElement: false - ) - } - let stabilized = stabilizeTextInputBeforeTyping( - app: app, - target: target, - keyboardVisibleBeforeTap: keyboardVisibleBeforeTap - ) - let readyTarget = TextEntryTarget( - element: stabilized.element ?? target, - refreshPoint: requestedPoint, - prefersFocusedElement: false - ) - let concreteTargetReady = keyboardVisibleBeforeTap && readyTarget.element != nil - let element = stabilized.focusConfirmed || concreteTargetReady - ? (stabilized.element ?? target) - : (waitForTextEntryReadiness(app: app, target: readyTarget) ?? stabilized.element ?? target) - return TextEntryTarget( - element: element, - refreshPoint: textEntryRefreshPoint(for: element) ?? requestedPoint, - prefersFocusedElement: false - ) - } - - func focusTextInputForTextEntry(app: XCUIApplication, element: XCUIElement) -> TextEntryTarget { - let point = textEntryRefreshPoint(for: element) - let keyboardVisibleBeforeTap = isKeyboardVisible(app: app) - if let point { - _ = tapAt(app: app, x: point.x, y: point.y) - } - // See the coordinate-target path above: direct element typing keeps this scoped to the - // tapped target, while the first-character warmup and final verify still catch dropped input. - if keyboardVisibleBeforeTap { - return TextEntryTarget( - element: element, - refreshPoint: textEntryRefreshPoint(for: element) ?? point, - prefersFocusedElement: false - ) - } - let stabilized = stabilizeTextInputBeforeTyping( - app: app, - target: element, - keyboardVisibleBeforeTap: keyboardVisibleBeforeTap - ) - let readyTarget = TextEntryTarget( - element: stabilized.element ?? element, - refreshPoint: point, - prefersFocusedElement: false - ) - let resolved = stabilized.focusConfirmed - ? (stabilized.element ?? element) - : (waitForTextEntryReadiness(app: app, target: readyTarget) ?? stabilized.element ?? element) - return TextEntryTarget( - element: resolved, - refreshPoint: textEntryRefreshPoint(for: resolved) ?? point, - prefersFocusedElement: false - ) - } - func isTextEntryElement(_ element: XCUIElement) -> Bool { switch element.elementType { case .textField, .secureTextField, .searchField, .textView: @@ -382,149 +177,6 @@ extension RunnerTests { return nil } - private func waitForTextEntryReadiness( - app: XCUIApplication, - target: TextEntryTarget, - timeout: TimeInterval = TextEntryTiming.readinessTimeout - ) -> XCUIElement? { -#if os(iOS) - var latest = resolveTextEntryElement(app: app, target: target) - let keyboardVisibleAtEntry = isKeyboardVisible(app: app) - let deadline = Date().addingTimeInterval(timeout) - var hardwareKeyboardFallback = Date().addingTimeInterval( - min(TextEntryTiming.hardwareKeyboardFallbackTimeout, timeout) - ) - var sawSoftwareKeyboard = false - while Date() < deadline { - if let focused = focusedTextInput(app: app) { - latest = focused - if isKeyboardVisible(app: app) { - return focused - } - } - // Fast-path on a keyboard hidden->visible transition: our tapped field gained focus, so - // return immediately instead of burning the full readinessTimeout (warmup-first-char echo - // + post-type verify/repair remain as drop safety nets). When the keyboard was ALREADY up - // (back-to-back fills), this isn't a focus signal — fall through to the settle/timeout so - // text isn't sent to the previously-focused field. - if keyboardBecameVisible(app: app, wasVisibleAtEntry: keyboardVisibleAtEntry) { - return latest - } - sawSoftwareKeyboard = sawSoftwareKeyboard || keyboardElementExists(app: app) - // A responder that takes no software keyboard (hardware keyboard connected, or a custom - // `inputView`) would otherwise burn the whole readinessTimeout waiting for one that is never - // coming. Leaving that window a bare wall-clock guess made readiness a function of ambient - // simulator state (#1874): on a loaded host the keyboard is merely late, and returning here - // handed the caller an element that had not taken focus yet. Ask the target itself instead, - // and re-arm rather than re-asking every poll — the query is cheap, not free. - if !sawSoftwareKeyboard, Date() >= hardwareKeyboardFallback, let candidate = latest { - if keyboardFocusConfirmed(app: app, element: candidate) { - return candidate - } - hardwareKeyboardFallback = Date().addingTimeInterval( - TextEntryTiming.hardwareKeyboardFallbackTimeout - ) - } - sleepFor(TextEntryTiming.pollInterval) - } - return focusedTextInput(app: app) ?? latest -#else - return resolveTextEntryElement(app: app, target: target) -#endif - } - - func waitForTextEntryReadinessAfterTap(app: XCUIApplication, element: XCUIElement) { -#if os(iOS) - switch element.elementType { - case .textField, .secureTextField, .searchField, .textView: - if waitForFocusedTextInput(app: app, timeout: TextEntryTiming.readinessTimeout) != nil { - return - } - let frame = element.frame - if !frame.isEmpty { - _ = tapAt(app: app, x: frame.midX, y: frame.midY) - _ = waitForFocusedTextInput(app: app, timeout: TextEntryTiming.readinessTimeout) - } - default: - return - } -#endif - } - - private func waitForFocusedTextInput(app: XCUIApplication, timeout: TimeInterval) -> XCUIElement? { - let deadline = Date().addingTimeInterval(timeout) - while Date() < deadline { - if let focused = focusedTextInput(app: app) { - return focused - } - sleepFor(TextEntryTiming.pollInterval) - } - return focusedTextInput(app: app) - } - - private func textEntryRefreshPoint(for element: XCUIElement?) -> CGPoint? { - guard let element else { - return nil - } - let frame = element.frame - guard !frame.isEmpty else { - return nil - } - return CGPoint(x: frame.midX, y: frame.midY) - } - - /// A focus-moved signal for iOS text entry, where `focusedTextInput` is intentionally nil. - /// The software keyboard TRANSITIONING from hidden (at entry) to visible means the field we - /// just tapped gained first-responder. If the keyboard was ALREADY up (e.g. back-to-back - /// fills into different fields), its visibility is not evidence focus moved to the new field, - /// so callers must keep waiting rather than typing into the previously-focused field. - private func keyboardBecameVisible(app: XCUIApplication, wasVisibleAtEntry: Bool) -> Bool { - return !wasVisibleAtEntry && isKeyboardVisible(app: app) - } - - /// Positive evidence that this element — the one readiness is about to hand its caller — holds - /// keyboard focus. Readable even with no software keyboard on screen, which is what lets the - /// hardware-keyboard fallback stop guessing from a wall clock. - /// - /// This is the same app-wide predicate `focusedTextInput` refuses to trust on iOS, used the - /// other way round. There, the query PICKS the target, so a stale or unrelated match becomes - /// the field that gets typed into. Here the target is already chosen and the query only - /// corroborates it: at most one element holds keyboard focus, so an answer that is not this - /// element is a refusal, not a substitution. Every way of being wrong therefore ends as `false` - /// and costs the remaining readiness timeout — exactly what the wait would spend with no - /// fallback at all. - func keyboardFocusConfirmed(app: XCUIApplication, element: XCUIElement) -> Bool { -#if os(iOS) - return safely("TEXT_ENTRY_FOCUS_CONFIRMED", false) { - // An element that no longer resolves reads as identifier "" and frame `.zero`, which would - // match any focused element that also reports an empty frame. Require a real frame first, - // so a dead handle cannot corroborate anything. - let frame = element.frame - guard !frame.isEmpty else { - return false - } - let focused = app - .descendants(matching: .any) - .matching(NSPredicate(format: "hasKeyboardFocus == 1")) - .firstMatch - guard focused.exists else { - return false - } - return focused.identifier == element.identifier && focused.frame == frame - } -#else - return false -#endif - } - - private func keyboardElementExists(app: XCUIApplication) -> Bool { -#if os(iOS) - return safely("KEYBOARD_EXISTS", false) { app.keyboards.firstMatch.exists } -#else - return false -#endif - } - private func moveCaretToEnd(element: XCUIElement) { #if os(tvOS) return diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntryReadiness.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntryReadiness.swift new file mode 100644 index 000000000..2cb0473c9 --- /dev/null +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntryReadiness.swift @@ -0,0 +1,354 @@ +import XCTest + +// Text-entry readiness: which element is about to receive text, and whether it has actually taken +// focus. Split from RunnerTests+TextEntry.swift, which keeps the vocabulary (failures, timings, +// targets), field clearing and value reading; everything that decides "ready" lives here. +extension RunnerTests { + func focusedTextInput(app: XCUIApplication) -> XCUIElement? { +#if os(iOS) + // iOS focus predicates can return stale or misleading text-input matches + // under XCUITest, so text entry readiness is driven by tap/keyboard state. + return nil +#else + return safely("FOCUSED_INPUT_QUERY") { + let candidates = app + .descendants(matching: .any) + .matching(NSPredicate(format: "hasKeyboardFocus == 1")) + .allElementsBoundByIndex + for candidate in candidates where candidate.exists { + switch candidate.elementType { + case .textField, .secureTextField, .searchField, .textView: + return candidate + default: + continue + } + } + return nil + } +#endif + } + + func rememberTextEntryTap(_ element: XCUIElement?) { + guard let element, isTextEntryElement(element) else { + clearRememberedTextEntryTap() + return + } + textEntryTapWitness = TextEntryTapWitness( + element: element, + bundleId: currentBundleId, + processIdentifier: currentAppProcessIdentifier + ) + } + + func clearRememberedTextEntryTap() { + textEntryTapWitness = nil + } + + private func rememberedTextEntryTarget() -> TextEntryTarget? { + guard let witness = textEntryTapWitness else { + return nil + } + // The tap is proof for one immediately-following bare type only. Consume it before checking + // the element so a failed or interrupted type cannot reuse stale focus evidence. + clearRememberedTextEntryTap() + guard witness.matches( + bundleId: currentBundleId, + processIdentifier: currentAppProcessIdentifier + ) else { + return nil + } + let element = witness.element + // XCUIElement is query-backed rather than a stable node identity. A same-identifier field + // introduced by app-side navigation between tap and this immediate type can therefore + // re-resolve here; keep the witness one-shot and fail closed on every observable identity + // boundary instead of using frame equality, which would reject legitimate layout changes. + guard safely("LAST_TAPPED_TEXT_INPUT_EXISTS", false, { element.exists }) else { + return nil + } + // Keep the target scoped to the element that the preceding tap actually selected. Do not + // attach a refresh point: if that element disappeared, bare type must fail closed rather + // than rediscovering a different field or dispatching unscoped app.typeText. + return TextEntryTarget( + element: element, + refreshPoint: nil, + prefersFocusedElement: false, + fromTapWitness: true + ) + } + + func stabilizeTextInputBeforeTyping( + app: XCUIApplication, + target: XCUIElement?, + keyboardVisibleBeforeTap: Bool? = nil + ) -> TextEntryStabilization { +#if os(tvOS) + return TextEntryStabilization(element: target, focusConfirmed: true) +#else + let latest = target + let keyboardVisibleAtEntry = keyboardVisibleBeforeTap ?? isKeyboardVisible(app: app) + let deadline = Date().addingTimeInterval(TextEntryTiming.focusTimeout) + while Date() < deadline { + if let focused = focusedTextInput(app: app) { + return TextEntryStabilization(element: focused, focusConfirmed: true) + } + // focusedTextInput is intentionally nil on iOS; treat the keyboard transitioning to + // visible after our tap as the focus-moved signal. Don't fast-path when it was already up. + if keyboardBecameVisible(app: app, wasVisibleAtEntry: keyboardVisibleAtEntry) { + return TextEntryStabilization(element: latest, focusConfirmed: true) + } + sleepFor(TextEntryTiming.pollInterval) + } + return TextEntryStabilization(element: latest, focusConfirmed: false) +#endif + } + + func focusTextInputForTextEntry(app: XCUIApplication, x: Double?, y: Double?) -> TextEntryTarget { + guard let x, let y else { + let softwareKeyboardVisible = isKeyboardVisible(app: app) + if !softwareKeyboardVisible, let rememberedTarget = rememberedTextEntryTarget() { + return rememberedTarget + } + // Bare `type` targets the current first responder. On iOS we intentionally do not trust + // `hasKeyboardFocus`, but an already-visible software keyboard is sufficient evidence that + // app.typeText has a receiver; waiting the full readiness timeout cannot prove a stronger + // target because there is no selector/coordinate focus move to validate. + if softwareKeyboardVisible { + return TextEntryTarget( + element: focusedTextInput(app: app), + refreshPoint: nil, + prefersFocusedElement: true + ) + } + let focused = waitForTextEntryReadiness( + app: app, + target: TextEntryTarget( + element: focusedTextInput(app: app), + refreshPoint: nil, + prefersFocusedElement: true + ) + ) + return TextEntryTarget(element: focused, refreshPoint: nil, prefersFocusedElement: true) + } + + let keyboardVisibleBeforeTap = isKeyboardVisible(app: app) + let target = textInputAt(app: app, x: x, y: y) + let requestedPoint = CGPoint(x: x, y: y) + if let target { + let frame = target.frame + if !frame.isEmpty { + _ = tapAt(app: app, x: frame.midX, y: frame.midY) + } else { + _ = tapAt(app: app, x: x, y: y) + } + } else { + _ = tapAt(app: app, x: x, y: y) + } + // A visible keyboard is not enough evidence for app.typeText, because focus may still + // belong to a previous field. With a concrete target we type through XCUIElement.typeText, + // so after tapping it the iOS readiness timeout cannot prove anything stronger. + if keyboardVisibleBeforeTap, let target { + return TextEntryTarget( + element: target, + refreshPoint: textEntryRefreshPoint(for: target) ?? requestedPoint, + prefersFocusedElement: false + ) + } + let stabilized = stabilizeTextInputBeforeTyping( + app: app, + target: target, + keyboardVisibleBeforeTap: keyboardVisibleBeforeTap + ) + let readyTarget = TextEntryTarget( + element: stabilized.element ?? target, + refreshPoint: requestedPoint, + prefersFocusedElement: false + ) + let concreteTargetReady = keyboardVisibleBeforeTap && readyTarget.element != nil + let element = stabilized.focusConfirmed || concreteTargetReady + ? (stabilized.element ?? target) + : (waitForTextEntryReadiness(app: app, target: readyTarget) ?? stabilized.element ?? target) + return TextEntryTarget( + element: element, + refreshPoint: textEntryRefreshPoint(for: element) ?? requestedPoint, + prefersFocusedElement: false + ) + } + + func focusTextInputForTextEntry(app: XCUIApplication, element: XCUIElement) -> TextEntryTarget { + let point = textEntryRefreshPoint(for: element) + let keyboardVisibleBeforeTap = isKeyboardVisible(app: app) + if let point { + _ = tapAt(app: app, x: point.x, y: point.y) + } + // See the coordinate-target path above: direct element typing keeps this scoped to the + // tapped target, while the first-character warmup and final verify still catch dropped input. + if keyboardVisibleBeforeTap { + return TextEntryTarget( + element: element, + refreshPoint: textEntryRefreshPoint(for: element) ?? point, + prefersFocusedElement: false + ) + } + let stabilized = stabilizeTextInputBeforeTyping( + app: app, + target: element, + keyboardVisibleBeforeTap: keyboardVisibleBeforeTap + ) + let readyTarget = TextEntryTarget( + element: stabilized.element ?? element, + refreshPoint: point, + prefersFocusedElement: false + ) + let resolved = stabilized.focusConfirmed + ? (stabilized.element ?? element) + : (waitForTextEntryReadiness(app: app, target: readyTarget) ?? stabilized.element ?? element) + return TextEntryTarget( + element: resolved, + refreshPoint: textEntryRefreshPoint(for: resolved) ?? point, + prefersFocusedElement: false + ) + } + + private func waitForTextEntryReadiness( + app: XCUIApplication, + target: TextEntryTarget, + timeout: TimeInterval = TextEntryTiming.readinessTimeout + ) -> XCUIElement? { +#if os(iOS) + var latest = resolveTextEntryElement(app: app, target: target) + let keyboardVisibleAtEntry = isKeyboardVisible(app: app) + let deadline = Date().addingTimeInterval(timeout) + var hardwareKeyboardFallback = Date().addingTimeInterval( + min(TextEntryTiming.hardwareKeyboardFallbackTimeout, timeout) + ) + var sawSoftwareKeyboard = false + while Date() < deadline { + if let focused = focusedTextInput(app: app) { + latest = focused + if isKeyboardVisible(app: app) { + return focused + } + } + // Fast-path on a keyboard hidden->visible transition: our tapped field gained focus, so + // return immediately instead of burning the full readinessTimeout (warmup-first-char echo + // + post-type verify/repair remain as drop safety nets). When the keyboard was ALREADY up + // (back-to-back fills), this isn't a focus signal — fall through to the settle/timeout so + // text isn't sent to the previously-focused field. + if keyboardBecameVisible(app: app, wasVisibleAtEntry: keyboardVisibleAtEntry) { + return latest + } + sawSoftwareKeyboard = sawSoftwareKeyboard || keyboardElementExists(app: app) + // A responder that takes no software keyboard (hardware keyboard connected, or a custom + // `inputView`) would otherwise burn the whole readinessTimeout waiting for one that is never + // coming. Leaving that window a bare wall-clock guess made readiness a function of ambient + // simulator state (#1874): on a loaded host the keyboard is merely late, and returning here + // handed the caller an element that had not taken focus yet. Ask the target itself instead, + // and re-arm rather than re-asking every poll — the query is cheap, not free. + if !sawSoftwareKeyboard, Date() >= hardwareKeyboardFallback, let candidate = latest { + if keyboardFocusConfirmed(app: app, element: candidate) { + return candidate + } + hardwareKeyboardFallback = Date().addingTimeInterval( + TextEntryTiming.hardwareKeyboardFallbackTimeout + ) + } + sleepFor(TextEntryTiming.pollInterval) + } + return focusedTextInput(app: app) ?? latest +#else + return resolveTextEntryElement(app: app, target: target) +#endif + } + + func waitForTextEntryReadinessAfterTap(app: XCUIApplication, element: XCUIElement) { +#if os(iOS) + switch element.elementType { + case .textField, .secureTextField, .searchField, .textView: + if waitForFocusedTextInput(app: app, timeout: TextEntryTiming.readinessTimeout) != nil { + return + } + let frame = element.frame + if !frame.isEmpty { + _ = tapAt(app: app, x: frame.midX, y: frame.midY) + _ = waitForFocusedTextInput(app: app, timeout: TextEntryTiming.readinessTimeout) + } + default: + return + } +#endif + } + + private func waitForFocusedTextInput(app: XCUIApplication, timeout: TimeInterval) -> XCUIElement? { + let deadline = Date().addingTimeInterval(timeout) + while Date() < deadline { + if let focused = focusedTextInput(app: app) { + return focused + } + sleepFor(TextEntryTiming.pollInterval) + } + return focusedTextInput(app: app) + } + + private func textEntryRefreshPoint(for element: XCUIElement?) -> CGPoint? { + guard let element else { + return nil + } + let frame = element.frame + guard !frame.isEmpty else { + return nil + } + return CGPoint(x: frame.midX, y: frame.midY) + } + + /// A focus-moved signal for iOS text entry, where `focusedTextInput` is intentionally nil. + /// The software keyboard TRANSITIONING from hidden (at entry) to visible means the field we + /// just tapped gained first-responder. If the keyboard was ALREADY up (e.g. back-to-back + /// fills into different fields), its visibility is not evidence focus moved to the new field, + /// so callers must keep waiting rather than typing into the previously-focused field. + private func keyboardBecameVisible(app: XCUIApplication, wasVisibleAtEntry: Bool) -> Bool { + return !wasVisibleAtEntry && isKeyboardVisible(app: app) + } + + /// Positive evidence that this element — the one readiness is about to hand its caller — holds + /// keyboard focus. Readable even with no software keyboard on screen, which is what lets the + /// hardware-keyboard fallback stop guessing from a wall clock. + /// + /// This is the same app-wide predicate `focusedTextInput` refuses to trust on iOS, used the + /// other way round. There, the query PICKS the target, so a stale or unrelated match becomes + /// the field that gets typed into. Here the target is already chosen and the query only + /// corroborates it: at most one element holds keyboard focus, so an answer that is not this + /// element is a refusal, not a substitution. Every way of being wrong therefore ends as `false` + /// and costs the remaining readiness timeout — exactly what the wait would spend with no + /// fallback at all. + func keyboardFocusConfirmed(app: XCUIApplication, element: XCUIElement) -> Bool { +#if os(iOS) + return safely("TEXT_ENTRY_FOCUS_CONFIRMED", false) { + // An element that no longer resolves reads as identifier "" and frame `.zero`, which would + // match any focused element that also reports an empty frame. Require a real frame first, + // so a dead handle cannot corroborate anything. + let frame = element.frame + guard !frame.isEmpty else { + return false + } + let focused = app + .descendants(matching: .any) + .matching(NSPredicate(format: "hasKeyboardFocus == 1")) + .firstMatch + guard focused.exists else { + return false + } + return focused.identifier == element.identifier && focused.frame == frame + } +#else + return false +#endif + } + + private func keyboardElementExists(app: XCUIApplication) -> Bool { +#if os(iOS) + return safely("KEYBOARD_EXISTS", false) { app.keyboards.firstMatch.exists } +#else + return false +#endif + } +} diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SynthesizedCommitDeadlineTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SynthesizedCommitDeadlineTests.swift new file mode 100644 index 000000000..840a6b0ef --- /dev/null +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SynthesizedCommitDeadlineTests.swift @@ -0,0 +1,164 @@ +import XCTest + +extension RunnerTests { +#if AGENT_DEVICE_RUNNER_UNIT_TESTS + /// A hand-driven clock for the commit waits. Time moves only where the wait sleeps, which is + /// what makes "the burst kept landing" and "the pipeline froze" expressible as two sequences of + /// the same length rather than as wall-clock luck — and a test that never advances it cannot + /// expire any budget, so only a test asking about time names `stallBudget`/`ceiling`. + final class CommitWaitClock { + private let origin = Date(timeIntervalSinceReferenceDate: 0) + private var current = Date(timeIntervalSinceReferenceDate: 0) + + var read: () -> Date { { self.current } } + + func advance(_ seconds: TimeInterval) { + current = current.addingTimeInterval(seconds) + } + + /// Seconds elapsed on this clock, for asserting *when* a wait gave up. + var elapsed: TimeInterval { current.timeIntervalSince(origin) } + } + + // #1874, through the shipped wait rather than a detached policy object: a burst that keeps + // landing must outlive the flat 3s deadline that used to govern it. Each poll advances the + // clock 2s and delivers one more character, so the wait is never idle for a full stall budget + // and must walk all the way to the match at t=6s. Reverting the wait to a flat deadline turns + // this red at the third poll. + func testCommitWaitOutlivesTheFlatDeadlineWhileTheExpectedPrefixGrows() { + let expected = "hardware" + let clock = CommitWaitClock() + var landed = 0 + let outcome = Self.awaitSynthesizedCommitOutcome( + expectedText: expected, + placeholder: nil, + stallBudget: 3, + ceiling: 10, + now: clock.read, + observe: { String(expected.prefix(landed * 2)) }, + waitForNextObservation: { + landed += 1 + clock.advance(2) + } + ) + XCTAssertEqual(outcome, .settled) + XCTAssertEqual(clock.elapsed, 8, "the wait must still be polling well past the 3s stall budget") + } + + // Review finding: the wait's setup reads the field's placeholder first, and that is an AX + // round-trip which takes seconds on exactly the loaded host this budget exists for. An earlier + // revision started the clock before that read, so slow setup spent the budget and the wait gave + // up after a single poll — sooner than the flat deadline it replaced, in the one condition it + // was written for. The clock now starts inside the loop, so a slow setup buys nothing and costs + // nothing. + func testCommitWaitBudgetStartsAtTheLoopRatherThanBeforeIt() { + let clock = CommitWaitClock() + clock.advance(60) + var polls = 0 + let outcome = Self.awaitSynthesizedCommitOutcome( + expectedText: "hardware", + placeholder: nil, + stallBudget: 3, + ceiling: 10, + now: clock.read, + observe: { "ha" }, + waitForNextObservation: { + polls += 1 + clock.advance(1) + } + ) + XCTAssertEqual(outcome, .notObserved) + XCTAssertEqual(polls, 3, "the wait must get its whole stall budget however slow its setup was") + } + + // The other half, and the reason the stall budget keeps the flat deadline's number: a pipeline + // that delivers nothing is condemned at exactly the instant it always was, so nothing that + // fails today starts passing merely by waiting longer. + func testCommitWaitCondemnsAFrozenPipelineAtTheStallBudget() { + let clock = CommitWaitClock() + let outcome = Self.awaitSynthesizedCommitOutcome( + expectedText: "hardware", + placeholder: nil, + stallBudget: 3, + ceiling: 10, + now: clock.read, + observe: { "ha" }, + waitForNextObservation: { clock.advance(1) } + ) + XCTAssertEqual(outcome, .notObserved) + XCTAssertEqual(clock.elapsed, 3, "a frozen prefix must give up on the stall budget, not the ceiling") + } + + // Progress buys time, but not without bound: one character per stall window would otherwise + // hold the command open until the daemon's own 45s budget killed the request. Here every poll + // lands a character, so only the ceiling can stop it. + func testCommitWaitCeilingStopsAnIndefinitelyThrottledPipeline() { + let clock = CommitWaitClock() + var landed = 0 + let outcome = Self.awaitSynthesizedCommitOutcome( + expectedText: String(repeating: "a", count: 100), + placeholder: nil, + stallBudget: 3, + ceiling: 10, + now: clock.read, + observe: { String(repeating: "a", count: landed) }, + waitForNextObservation: { + landed += 1 + clock.advance(2) + } + ) + XCTAssertEqual(outcome, .notObserved) + XCTAssertEqual(clock.elapsed, 10, "the ceiling is absolute, however long characters keep arriving") + } + + // Only forward movement is evidence the burst is still landing. A field the app clears + // mid-flight would otherwise reset the stall clock on every poll and hold every wedged wait + // open to the ceiling. Replacement mode, because that is where a non-matching value keeps + // polling rather than settling as `.diverged`. + func testCommitWaitTreatsARetreatingValueAsNoProgress() { + let clock = CommitWaitClock() + let observations = ["ada@", "", "ada@", "", "ada@"] + var index = 0 + let outcome = Self.awaitSynthesizedReplacementCommitOutcome( + expectedText: "ada@example", + placeholder: nil, + stallBudget: 3, + ceiling: 10, + now: clock.read, + observe: { observations[min(index, observations.count - 1)] }, + waitForNextObservation: { + index += 1 + clock.advance(1) + } + ) + XCTAssertEqual(outcome, .notObserved) + XCTAssertEqual(clock.elapsed, 3, "churn between two values is not progress and must not buy time") + } + + // The replacement route earns time the same way, and is bounded the same way — it is the route + // `fill` takes when the XCTest channel is penalized, i.e. the one that runs on a loaded host. + // Here every poll lands one more character of a value that never completes, so the wait can only + // end at the ceiling: it proves the growing prefix carried it past the 3s stall budget (a flat + // deadline stops at t=3) and that the ceiling still stops it. + func testReplacementCommitWaitOutlivesTheFlatDeadlineThenStopsAtTheCeiling() { + let expected = "ada@example.com" + let clock = CommitWaitClock() + var landed = 0 + let outcome = Self.awaitSynthesizedReplacementCommitOutcome( + expectedText: expected, + placeholder: nil, + stallBudget: 3, + ceiling: 10, + now: clock.read, + observe: { String(expected.prefix(landed)) }, + waitForNextObservation: { + landed += 1 + clock.advance(2) + } + ) + XCTAssertEqual(outcome, .notObserved) + XCTAssertEqual(clock.elapsed, 10, "progress must carry the wait past 3s, and the ceiling must end it") + XCTAssertEqual(landed, 5, "one character per poll, five polls inside the ceiling") + } +#endif +} diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift index 97a26ff4a..5d7f61413 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift @@ -90,139 +90,6 @@ extension RunnerTests { ) } - /// A hand-driven clock for the commit waits. Time moves only where the wait sleeps, which is - /// what makes "the burst kept landing" and "the pipeline froze" expressible as two sequences of - /// the same length rather than as wall-clock luck — and a test that never advances it cannot - /// expire any budget, so only a test asking about time names `stallBudget`/`ceiling`. - final class CommitWaitClock { - private let origin = Date(timeIntervalSinceReferenceDate: 0) - private var current = Date(timeIntervalSinceReferenceDate: 0) - - var read: () -> Date { { self.current } } - - func advance(_ seconds: TimeInterval) { - current = current.addingTimeInterval(seconds) - } - - /// Seconds elapsed on this clock, for asserting *when* a wait gave up. - var elapsed: TimeInterval { current.timeIntervalSince(origin) } - } - - // #1874, through the shipped wait rather than a detached policy object: a burst that keeps - // landing must outlive the flat 3s deadline that used to govern it. Each poll advances the - // clock 2s and delivers one more character, so the wait is never idle for a full stall budget - // and must walk all the way to the match at t=6s. Reverting the wait to a flat deadline turns - // this red at the third poll. - func testCommitWaitOutlivesTheFlatDeadlineWhileTheExpectedPrefixGrows() { - let expected = "hardware" - let clock = CommitWaitClock() - var landed = 0 - let outcome = Self.awaitSynthesizedCommitOutcome( - expectedText: expected, - placeholder: nil, - stallBudget: 3, - ceiling: 10, - now: clock.read, - observe: { String(expected.prefix(landed * 2)) }, - waitForNextObservation: { - landed += 1 - clock.advance(2) - } - ) - XCTAssertEqual(outcome, .settled) - XCTAssertEqual(clock.elapsed, 8, "the wait must still be polling well past the 3s stall budget") - } - - // Review finding: the wait's setup reads the field's placeholder first, and that is an AX - // round-trip which takes seconds on exactly the loaded host this budget exists for. An earlier - // revision started the clock before that read, so slow setup spent the budget and the wait gave - // up after a single poll — sooner than the flat deadline it replaced, in the one condition it - // was written for. The clock now starts inside the loop, so a slow setup buys nothing and costs - // nothing. - func testCommitWaitBudgetStartsAtTheLoopRatherThanBeforeIt() { - let clock = CommitWaitClock() - clock.advance(60) - var polls = 0 - let outcome = Self.awaitSynthesizedCommitOutcome( - expectedText: "hardware", - placeholder: nil, - stallBudget: 3, - ceiling: 10, - now: clock.read, - observe: { "ha" }, - waitForNextObservation: { - polls += 1 - clock.advance(1) - } - ) - XCTAssertEqual(outcome, .notObserved) - XCTAssertEqual(polls, 3, "the wait must get its whole stall budget however slow its setup was") - } - - // The other half, and the reason the stall budget keeps the flat deadline's number: a pipeline - // that delivers nothing is condemned at exactly the instant it always was, so nothing that - // fails today starts passing merely by waiting longer. - func testCommitWaitCondemnsAFrozenPipelineAtTheStallBudget() { - let clock = CommitWaitClock() - let outcome = Self.awaitSynthesizedCommitOutcome( - expectedText: "hardware", - placeholder: nil, - stallBudget: 3, - ceiling: 10, - now: clock.read, - observe: { "ha" }, - waitForNextObservation: { clock.advance(1) } - ) - XCTAssertEqual(outcome, .notObserved) - XCTAssertEqual(clock.elapsed, 3, "a frozen prefix must give up on the stall budget, not the ceiling") - } - - // Progress buys time, but not without bound: one character per stall window would otherwise - // hold the command open until the daemon's own 45s budget killed the request. Here every poll - // lands a character, so only the ceiling can stop it. - func testCommitWaitCeilingStopsAnIndefinitelyThrottledPipeline() { - let clock = CommitWaitClock() - var landed = 0 - let outcome = Self.awaitSynthesizedCommitOutcome( - expectedText: String(repeating: "a", count: 100), - placeholder: nil, - stallBudget: 3, - ceiling: 10, - now: clock.read, - observe: { String(repeating: "a", count: landed) }, - waitForNextObservation: { - landed += 1 - clock.advance(2) - } - ) - XCTAssertEqual(outcome, .notObserved) - XCTAssertEqual(clock.elapsed, 10, "the ceiling is absolute, however long characters keep arriving") - } - - // Only forward movement is evidence the burst is still landing. A field the app clears - // mid-flight would otherwise reset the stall clock on every poll and hold every wedged wait - // open to the ceiling. Replacement mode, because that is where a non-matching value keeps - // polling rather than settling as `.diverged`. - func testCommitWaitTreatsARetreatingValueAsNoProgress() { - let clock = CommitWaitClock() - let observations = ["ada@", "", "ada@", "", "ada@"] - var index = 0 - let outcome = Self.awaitSynthesizedReplacementCommitOutcome( - expectedText: "ada@example", - placeholder: nil, - stallBudget: 3, - ceiling: 10, - now: clock.read, - observe: { observations[min(index, observations.count - 1)] }, - waitForNextObservation: { - index += 1 - clock.advance(1) - } - ) - XCTAssertEqual(outcome, .notObserved) - XCTAssertEqual(clock.elapsed, 3, "churn between two values is not progress and must not buy time") - } - // The regression behind #1874/#1844: the wait used to return Void, so an expired deadline was // indistinguishable from a commit and `type` reported ok over a partially committed field. The // CI signature was a field holding "h" out of "hardware-keyboard" with the command successful. @@ -247,12 +114,10 @@ extension RunnerTests { func testSynthesizedCommitStopsAtTheFirstSettledObservation() { for observed in ["hardware-keyboard", "hardwarX", nil] { - let clock = CommitWaitClock() var polls = 0 let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware-keyboard", placeholder: nil, - now: clock.read, observe: { observed }, waitForNextObservation: { polls += 1 } ) @@ -265,12 +130,10 @@ extension RunnerTests { func testSynthesizedCommitWalksAPrefixToCompletion() { let steps = ["", "hardware-", "hardware-keyboard"] - let clock = CommitWaitClock() var index = 0 let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: "hardware-keyboard", placeholder: nil, - now: clock.read, observe: { steps[min(index, steps.count - 1)] }, waitForNextObservation: { index += 1 } ) @@ -308,12 +171,10 @@ extension RunnerTests { func testClearAfterDispatchCannotTurnThePlaceholderIntoCommitEvidence() { let textBeforeDispatch = "0" let expectedText = textBeforeDispatch + ".00" - let clock = CommitWaitClock() var observations = 0 let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: expectedText, placeholder: "0.00", - now: clock.read, observe: { observations += 1 return "0.00" @@ -338,11 +199,9 @@ extension RunnerTests { (" ", ""), ] for testCase in cases { - let clock = CommitWaitClock() let outcome = Self.awaitSynthesizedCommitOutcome( expectedText: testCase.expectedText, placeholder: testCase.placeholder, - now: clock.read, observe: { testCase.expectedText }, waitForNextObservation: {} ) @@ -366,12 +225,10 @@ extension RunnerTests { (expected: "ada@example", observedAfterDrop: "aexample"), ] for corruption in corruptions { - let appendClock = CommitWaitClock() XCTAssertEqual( Self.awaitSynthesizedCommitOutcome( expectedText: corruption.expected, placeholder: nil, - now: appendClock.read, observe: { corruption.observedAfterDrop }, waitForNextObservation: {} ), @@ -404,12 +261,10 @@ extension RunnerTests { // does not depend on prefix-walking to keep polling. func testSynthesizedReplacementCommitToleratesLagUntilExactMatch() { let steps = ["", "ad", "ada@example"] - let clock = CommitWaitClock() var index = 0 let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: "ada@example", placeholder: nil, - now: clock.read, observe: { steps[min(index, steps.count - 1)] }, waitForNextObservation: { index += 1 } ) @@ -440,12 +295,10 @@ extension RunnerTests { // Same placeholder-collision guard as append mode, and for the same reason: a pre-dispatch value // cannot identify what a later placeholder-equal AX value represents, so refuse before polling. func testSynthesizedReplacementCommitPlaceholderGuardRefusesWithoutPolling() { - let clock = CommitWaitClock() var observations = 0 let outcome = Self.awaitSynthesizedReplacementCommitOutcome( expectedText: "0.00", placeholder: "0.00", - now: clock.read, observe: { observations += 1 return "0.00" diff --git a/src/__tests__/apple-runner-log-redaction.test.ts b/src/__tests__/apple-runner-log-redaction.test.ts index 1d72336ec..3cb18245b 100644 --- a/src/__tests__/apple-runner-log-redaction.test.ts +++ b/src/__tests__/apple-runner-log-redaction.test.ts @@ -5,9 +5,9 @@ import { fileURLToPath } from 'node:url'; import { test } from 'vitest'; const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); -const synthesizedTextEntryPath = path.join( +const commitWaitPath = path.join( repoRoot, - 'apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift', + 'apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedCommitDeadline.swift', ); // The synthesized bare-type commit wait polls the target field's live value on the shipped @@ -34,7 +34,7 @@ function extractObserveClosure(source: string): string { } test('commit-wait cadence logging goes through the typed value-free boundary', () => { - const source = fs.readFileSync(synthesizedTextEntryPath, 'utf8'); + const source = fs.readFileSync(commitWaitPath, 'utf8'); assert.match( source, From 4b2c958654208d2845e58d14ed0e0d9d5e60c2cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 26 Aug 2026 15:57:02 +0200 Subject: [PATCH 7/7] refactor(ios): split text-entry target acquisition from readiness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review residual: the readiness extraction was 354 lines and still owned two questions. Acquisition — the one-shot tap witness, post-tap stabilization, both focusTextInputForTextEntry entry points and the refresh point — moves to RunnerTests+TextEntryFocus.swift (206). Readiness keeps the waits, the keyboard signals they read and the focus corroboration (158). The dependency is one-way: acquisition asks readiness, never the reverse, so waitForTextEntryReadiness and keyboardBecameVisible lose file-private scope and nothing else does. --- .../RunnerTests+TextEntryFocus.swift | 206 +++++++++++++++++ .../RunnerTests+TextEntryReadiness.swift | 208 +----------------- 2 files changed, 212 insertions(+), 202 deletions(-) create mode 100644 apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntryFocus.swift diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntryFocus.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntryFocus.swift new file mode 100644 index 000000000..da780c827 --- /dev/null +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntryFocus.swift @@ -0,0 +1,206 @@ +import XCTest + +// Text-entry target acquisition: choosing the element a `type`/`fill` will address and getting +// focus onto it — the one-shot tap witness, the post-tap stabilization, and the two +// `focusTextInputForTextEntry` entry points the command layer calls. Whether that target is ready +// is RunnerTests+TextEntryReadiness.swift's question, and this file asks it rather than answering +// it. +extension RunnerTests { + func rememberTextEntryTap(_ element: XCUIElement?) { + guard let element, isTextEntryElement(element) else { + clearRememberedTextEntryTap() + return + } + textEntryTapWitness = TextEntryTapWitness( + element: element, + bundleId: currentBundleId, + processIdentifier: currentAppProcessIdentifier + ) + } + + func clearRememberedTextEntryTap() { + textEntryTapWitness = nil + } + + private func rememberedTextEntryTarget() -> TextEntryTarget? { + guard let witness = textEntryTapWitness else { + return nil + } + // The tap is proof for one immediately-following bare type only. Consume it before checking + // the element so a failed or interrupted type cannot reuse stale focus evidence. + clearRememberedTextEntryTap() + guard witness.matches( + bundleId: currentBundleId, + processIdentifier: currentAppProcessIdentifier + ) else { + return nil + } + let element = witness.element + // XCUIElement is query-backed rather than a stable node identity. A same-identifier field + // introduced by app-side navigation between tap and this immediate type can therefore + // re-resolve here; keep the witness one-shot and fail closed on every observable identity + // boundary instead of using frame equality, which would reject legitimate layout changes. + guard safely("LAST_TAPPED_TEXT_INPUT_EXISTS", false, { element.exists }) else { + return nil + } + // Keep the target scoped to the element that the preceding tap actually selected. Do not + // attach a refresh point: if that element disappeared, bare type must fail closed rather + // than rediscovering a different field or dispatching unscoped app.typeText. + return TextEntryTarget( + element: element, + refreshPoint: nil, + prefersFocusedElement: false, + fromTapWitness: true + ) + } + + func stabilizeTextInputBeforeTyping( + app: XCUIApplication, + target: XCUIElement?, + keyboardVisibleBeforeTap: Bool? = nil + ) -> TextEntryStabilization { +#if os(tvOS) + return TextEntryStabilization(element: target, focusConfirmed: true) +#else + let latest = target + let keyboardVisibleAtEntry = keyboardVisibleBeforeTap ?? isKeyboardVisible(app: app) + let deadline = Date().addingTimeInterval(TextEntryTiming.focusTimeout) + while Date() < deadline { + if let focused = focusedTextInput(app: app) { + return TextEntryStabilization(element: focused, focusConfirmed: true) + } + // focusedTextInput is intentionally nil on iOS; treat the keyboard transitioning to + // visible after our tap as the focus-moved signal. Don't fast-path when it was already up. + if keyboardBecameVisible(app: app, wasVisibleAtEntry: keyboardVisibleAtEntry) { + return TextEntryStabilization(element: latest, focusConfirmed: true) + } + sleepFor(TextEntryTiming.pollInterval) + } + return TextEntryStabilization(element: latest, focusConfirmed: false) +#endif + } + + func focusTextInputForTextEntry(app: XCUIApplication, x: Double?, y: Double?) -> TextEntryTarget { + guard let x, let y else { + let softwareKeyboardVisible = isKeyboardVisible(app: app) + if !softwareKeyboardVisible, let rememberedTarget = rememberedTextEntryTarget() { + return rememberedTarget + } + // Bare `type` targets the current first responder. On iOS we intentionally do not trust + // `hasKeyboardFocus`, but an already-visible software keyboard is sufficient evidence that + // app.typeText has a receiver; waiting the full readiness timeout cannot prove a stronger + // target because there is no selector/coordinate focus move to validate. + if softwareKeyboardVisible { + return TextEntryTarget( + element: focusedTextInput(app: app), + refreshPoint: nil, + prefersFocusedElement: true + ) + } + let focused = waitForTextEntryReadiness( + app: app, + target: TextEntryTarget( + element: focusedTextInput(app: app), + refreshPoint: nil, + prefersFocusedElement: true + ) + ) + return TextEntryTarget(element: focused, refreshPoint: nil, prefersFocusedElement: true) + } + + let keyboardVisibleBeforeTap = isKeyboardVisible(app: app) + let target = textInputAt(app: app, x: x, y: y) + let requestedPoint = CGPoint(x: x, y: y) + if let target { + let frame = target.frame + if !frame.isEmpty { + _ = tapAt(app: app, x: frame.midX, y: frame.midY) + } else { + _ = tapAt(app: app, x: x, y: y) + } + } else { + _ = tapAt(app: app, x: x, y: y) + } + // A visible keyboard is not enough evidence for app.typeText, because focus may still + // belong to a previous field. With a concrete target we type through XCUIElement.typeText, + // so after tapping it the iOS readiness timeout cannot prove anything stronger. + if keyboardVisibleBeforeTap, let target { + return TextEntryTarget( + element: target, + refreshPoint: textEntryRefreshPoint(for: target) ?? requestedPoint, + prefersFocusedElement: false + ) + } + let stabilized = stabilizeTextInputBeforeTyping( + app: app, + target: target, + keyboardVisibleBeforeTap: keyboardVisibleBeforeTap + ) + let readyTarget = TextEntryTarget( + element: stabilized.element ?? target, + refreshPoint: requestedPoint, + prefersFocusedElement: false + ) + let concreteTargetReady = keyboardVisibleBeforeTap && readyTarget.element != nil + let element = stabilized.focusConfirmed || concreteTargetReady + ? (stabilized.element ?? target) + : (waitForTextEntryReadiness(app: app, target: readyTarget) ?? stabilized.element ?? target) + return TextEntryTarget( + element: element, + refreshPoint: textEntryRefreshPoint(for: element) ?? requestedPoint, + prefersFocusedElement: false + ) + } + + func focusTextInputForTextEntry(app: XCUIApplication, element: XCUIElement) -> TextEntryTarget { + let point = textEntryRefreshPoint(for: element) + let keyboardVisibleBeforeTap = isKeyboardVisible(app: app) + if let point { + _ = tapAt(app: app, x: point.x, y: point.y) + } + // See the coordinate-target path above: direct element typing keeps this scoped to the + // tapped target, while the first-character warmup and final verify still catch dropped input. + if keyboardVisibleBeforeTap { + return TextEntryTarget( + element: element, + refreshPoint: textEntryRefreshPoint(for: element) ?? point, + prefersFocusedElement: false + ) + } + let stabilized = stabilizeTextInputBeforeTyping( + app: app, + target: element, + keyboardVisibleBeforeTap: keyboardVisibleBeforeTap + ) + let readyTarget = TextEntryTarget( + element: stabilized.element ?? element, + refreshPoint: point, + prefersFocusedElement: false + ) + let resolved = stabilized.focusConfirmed + ? (stabilized.element ?? element) + : (waitForTextEntryReadiness(app: app, target: readyTarget) ?? stabilized.element ?? element) + return TextEntryTarget( + element: resolved, + refreshPoint: textEntryRefreshPoint(for: resolved) ?? point, + prefersFocusedElement: false + ) + } + + private func textEntryRefreshPoint(for element: XCUIElement?) -> CGPoint? { + guard let element else { + return nil + } + let frame = element.frame + guard !frame.isEmpty else { + return nil + } + return CGPoint(x: frame.midX, y: frame.midY) + } + + /// A focus-moved signal for iOS text entry, where `focusedTextInput` is intentionally nil. + /// The software keyboard TRANSITIONING from hidden (at entry) to visible means the field we + /// just tapped gained first-responder. If the keyboard was ALREADY up (e.g. back-to-back + /// fills into different fields), its visibility is not evidence focus moved to the new field, + /// so callers must keep waiting rather than typing into the previously-focused field. +} diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntryReadiness.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntryReadiness.swift index 2cb0473c9..3404d0adc 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntryReadiness.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntryReadiness.swift @@ -1,8 +1,9 @@ import XCTest -// Text-entry readiness: which element is about to receive text, and whether it has actually taken -// focus. Split from RunnerTests+TextEntry.swift, which keeps the vocabulary (failures, timings, -// targets), field clearing and value reading; everything that decides "ready" lives here. +// Text-entry readiness: whether the target is actually able to receive text yet. The waits, the +// keyboard signals they read, and the focus corroboration that ends the hardware-keyboard window. +// RunnerTests+TextEntryFocus.swift is the only caller of the two non-private entries here; nothing +// in this file reaches back into it. extension RunnerTests { func focusedTextInput(app: XCUIApplication) -> XCUIElement? { #if os(iOS) @@ -28,188 +29,7 @@ extension RunnerTests { #endif } - func rememberTextEntryTap(_ element: XCUIElement?) { - guard let element, isTextEntryElement(element) else { - clearRememberedTextEntryTap() - return - } - textEntryTapWitness = TextEntryTapWitness( - element: element, - bundleId: currentBundleId, - processIdentifier: currentAppProcessIdentifier - ) - } - - func clearRememberedTextEntryTap() { - textEntryTapWitness = nil - } - - private func rememberedTextEntryTarget() -> TextEntryTarget? { - guard let witness = textEntryTapWitness else { - return nil - } - // The tap is proof for one immediately-following bare type only. Consume it before checking - // the element so a failed or interrupted type cannot reuse stale focus evidence. - clearRememberedTextEntryTap() - guard witness.matches( - bundleId: currentBundleId, - processIdentifier: currentAppProcessIdentifier - ) else { - return nil - } - let element = witness.element - // XCUIElement is query-backed rather than a stable node identity. A same-identifier field - // introduced by app-side navigation between tap and this immediate type can therefore - // re-resolve here; keep the witness one-shot and fail closed on every observable identity - // boundary instead of using frame equality, which would reject legitimate layout changes. - guard safely("LAST_TAPPED_TEXT_INPUT_EXISTS", false, { element.exists }) else { - return nil - } - // Keep the target scoped to the element that the preceding tap actually selected. Do not - // attach a refresh point: if that element disappeared, bare type must fail closed rather - // than rediscovering a different field or dispatching unscoped app.typeText. - return TextEntryTarget( - element: element, - refreshPoint: nil, - prefersFocusedElement: false, - fromTapWitness: true - ) - } - - func stabilizeTextInputBeforeTyping( - app: XCUIApplication, - target: XCUIElement?, - keyboardVisibleBeforeTap: Bool? = nil - ) -> TextEntryStabilization { -#if os(tvOS) - return TextEntryStabilization(element: target, focusConfirmed: true) -#else - let latest = target - let keyboardVisibleAtEntry = keyboardVisibleBeforeTap ?? isKeyboardVisible(app: app) - let deadline = Date().addingTimeInterval(TextEntryTiming.focusTimeout) - while Date() < deadline { - if let focused = focusedTextInput(app: app) { - return TextEntryStabilization(element: focused, focusConfirmed: true) - } - // focusedTextInput is intentionally nil on iOS; treat the keyboard transitioning to - // visible after our tap as the focus-moved signal. Don't fast-path when it was already up. - if keyboardBecameVisible(app: app, wasVisibleAtEntry: keyboardVisibleAtEntry) { - return TextEntryStabilization(element: latest, focusConfirmed: true) - } - sleepFor(TextEntryTiming.pollInterval) - } - return TextEntryStabilization(element: latest, focusConfirmed: false) -#endif - } - - func focusTextInputForTextEntry(app: XCUIApplication, x: Double?, y: Double?) -> TextEntryTarget { - guard let x, let y else { - let softwareKeyboardVisible = isKeyboardVisible(app: app) - if !softwareKeyboardVisible, let rememberedTarget = rememberedTextEntryTarget() { - return rememberedTarget - } - // Bare `type` targets the current first responder. On iOS we intentionally do not trust - // `hasKeyboardFocus`, but an already-visible software keyboard is sufficient evidence that - // app.typeText has a receiver; waiting the full readiness timeout cannot prove a stronger - // target because there is no selector/coordinate focus move to validate. - if softwareKeyboardVisible { - return TextEntryTarget( - element: focusedTextInput(app: app), - refreshPoint: nil, - prefersFocusedElement: true - ) - } - let focused = waitForTextEntryReadiness( - app: app, - target: TextEntryTarget( - element: focusedTextInput(app: app), - refreshPoint: nil, - prefersFocusedElement: true - ) - ) - return TextEntryTarget(element: focused, refreshPoint: nil, prefersFocusedElement: true) - } - - let keyboardVisibleBeforeTap = isKeyboardVisible(app: app) - let target = textInputAt(app: app, x: x, y: y) - let requestedPoint = CGPoint(x: x, y: y) - if let target { - let frame = target.frame - if !frame.isEmpty { - _ = tapAt(app: app, x: frame.midX, y: frame.midY) - } else { - _ = tapAt(app: app, x: x, y: y) - } - } else { - _ = tapAt(app: app, x: x, y: y) - } - // A visible keyboard is not enough evidence for app.typeText, because focus may still - // belong to a previous field. With a concrete target we type through XCUIElement.typeText, - // so after tapping it the iOS readiness timeout cannot prove anything stronger. - if keyboardVisibleBeforeTap, let target { - return TextEntryTarget( - element: target, - refreshPoint: textEntryRefreshPoint(for: target) ?? requestedPoint, - prefersFocusedElement: false - ) - } - let stabilized = stabilizeTextInputBeforeTyping( - app: app, - target: target, - keyboardVisibleBeforeTap: keyboardVisibleBeforeTap - ) - let readyTarget = TextEntryTarget( - element: stabilized.element ?? target, - refreshPoint: requestedPoint, - prefersFocusedElement: false - ) - let concreteTargetReady = keyboardVisibleBeforeTap && readyTarget.element != nil - let element = stabilized.focusConfirmed || concreteTargetReady - ? (stabilized.element ?? target) - : (waitForTextEntryReadiness(app: app, target: readyTarget) ?? stabilized.element ?? target) - return TextEntryTarget( - element: element, - refreshPoint: textEntryRefreshPoint(for: element) ?? requestedPoint, - prefersFocusedElement: false - ) - } - - func focusTextInputForTextEntry(app: XCUIApplication, element: XCUIElement) -> TextEntryTarget { - let point = textEntryRefreshPoint(for: element) - let keyboardVisibleBeforeTap = isKeyboardVisible(app: app) - if let point { - _ = tapAt(app: app, x: point.x, y: point.y) - } - // See the coordinate-target path above: direct element typing keeps this scoped to the - // tapped target, while the first-character warmup and final verify still catch dropped input. - if keyboardVisibleBeforeTap { - return TextEntryTarget( - element: element, - refreshPoint: textEntryRefreshPoint(for: element) ?? point, - prefersFocusedElement: false - ) - } - let stabilized = stabilizeTextInputBeforeTyping( - app: app, - target: element, - keyboardVisibleBeforeTap: keyboardVisibleBeforeTap - ) - let readyTarget = TextEntryTarget( - element: stabilized.element ?? element, - refreshPoint: point, - prefersFocusedElement: false - ) - let resolved = stabilized.focusConfirmed - ? (stabilized.element ?? element) - : (waitForTextEntryReadiness(app: app, target: readyTarget) ?? stabilized.element ?? element) - return TextEntryTarget( - element: resolved, - refreshPoint: textEntryRefreshPoint(for: resolved) ?? point, - prefersFocusedElement: false - ) - } - - private func waitForTextEntryReadiness( + func waitForTextEntryReadiness( app: XCUIApplication, target: TextEntryTarget, timeout: TimeInterval = TextEntryTiming.readinessTimeout @@ -289,23 +109,7 @@ extension RunnerTests { return focusedTextInput(app: app) } - private func textEntryRefreshPoint(for element: XCUIElement?) -> CGPoint? { - guard let element else { - return nil - } - let frame = element.frame - guard !frame.isEmpty else { - return nil - } - return CGPoint(x: frame.midX, y: frame.midY) - } - - /// A focus-moved signal for iOS text entry, where `focusedTextInput` is intentionally nil. - /// The software keyboard TRANSITIONING from hidden (at entry) to visible means the field we - /// just tapped gained first-responder. If the keyboard was ALREADY up (e.g. back-to-back - /// fills into different fields), its visibility is not evidence focus moved to the new field, - /// so callers must keep waiting rather than typing into the previously-focused field. - private func keyboardBecameVisible(app: XCUIApplication, wasVisibleAtEntry: Bool) -> Bool { + func keyboardBecameVisible(app: XCUIApplication, wasVisibleAtEntry: Bool) -> Bool { return !wasVisibleAtEntry && isKeyboardVisible(app: app) }