From 43ea03ca047064c9425ed8e721022a703df300ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 25 Sep 2026 16:01:19 +0200 Subject: [PATCH 1/2] refactor(ios-runner): charge the text-entry budget the plan the runner posts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The delivery budget added in #2904 was a parallel projection: projectedSeconds re-derived the spaced split, the warmup peel and the warmup wait, all decided elsewhere, and two pre-merge fix commits were drift this shape caused. It also under-charged the plan it modeled: a post that replaces selects with its own synthesize record (RunnerSynthesizedTextEntry.m), which the estimate counted as one call. SynthesizedTextPlan now carries the posts a command makes (characters, whether the post selects first, the wait after it), the budget charges the sum over those steps, and both synthesized routes execute that same array — the loop sleeps the plan's own pauses instead of re-deriving delaySeconds > 0 twice more. The type route decides the budget once, outside the per-chunk closure, and its budget refusal and synthesizer .fallback arm share one application-wide typing path. The pace moved from the ObjC bridge (whose getter Swift read back) into TextEntryTiming and is passed to the bridge, so no call site can type at a pace its own budget never charged. The test-only acknowledge window moved into the tests, and the fixture's never-asserted edits= counter is gone. fill's admitted length at --delay-ms 0 moves 214 -> 212 (honesty about the select-all record, which the old hint overpromised); the recovery hint now fits the host's 400-character diagnostic bound it previously overflowed by 60, losing its last sentence on the wire. --- .../AgentDeviceRunner/AgentDeviceRunnerApp.m | 5 +- .../RunnerSynthesizedTextEntry.h | 18 +- .../RunnerSynthesizedTextEntry.m | 35 +-- .../RunnerTests+SynthesizedTextEntry.swift | 214 ++++++++++----- .../RunnerTests+TextEntry.swift | 30 ++- .../RunnerTests+TextTyping.swift | 97 ++++--- ...unnerTests+SynthesizedTextEntryTests.swift | 10 +- .../RunnerTests+TextEntryPolicyTests.swift | 246 +++++++++++++----- 8 files changed, 438 insertions(+), 217 deletions(-) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunner/AgentDeviceRunnerApp.m b/apple/runner/AgentDeviceRunner/AgentDeviceRunner/AgentDeviceRunnerApp.m index 6c08c12dde..a1b3dc8f6a 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunner/AgentDeviceRunnerApp.m +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunner/AgentDeviceRunnerApp.m @@ -66,7 +66,6 @@ @interface AgentDeviceRunnerViewController : UIViewController @property(nonatomic, assign) NSUInteger firstAlertActions; @property(nonatomic, assign) NSUInteger replacementAlertActions; @property(nonatomic, strong) UILabel *textEntryWriteBackStatus; -@property(nonatomic, assign) NSUInteger textEntryRenderedEdits; @property(nonatomic, assign) NSUInteger textEntryWriteBacks; @property(nonatomic, copy, nullable) NSString *textEntryRenderedValue; @property(nonatomic, assign) NSTimeInterval textEntryLastEditTime; @@ -187,8 +186,7 @@ - (void)updateAlertActionStatus { - (void)updateTextEntryWriteBackStatus { NSTimeInterval burstSpan = self.textEntryLastEditTime - self.textEntryBurstStartTime; self.textEntryWriteBackStatus.text = [NSString - stringWithFormat:@"edits=%lu write-backs=%lu burst-edits=%lu burst-ms=%lu min-gap-ms=%lu", - (unsigned long)self.textEntryRenderedEdits, + stringWithFormat:@"write-backs=%lu burst-edits=%lu burst-ms=%lu min-gap-ms=%lu", (unsigned long)self.textEntryWriteBacks, (unsigned long)self.textEntryBurstEdits, (unsigned long)llround(burstSpan * 1000), @@ -292,7 +290,6 @@ - (void)agentDeviceTextEntryDidChange:(UITextField *)textField { } } else { self.textEntryRenderedValue = [textField.text copy]; - self.textEntryRenderedEdits += 1; } [self updateTextEntryWriteBackStatus]; } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSynthesizedTextEntry.h b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSynthesizedTextEntry.h index cecbe07ed8..dd01eb9c5d 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSynthesizedTextEntry.h +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSynthesizedTextEntry.h @@ -17,21 +17,21 @@ typedef NS_ENUM(NSInteger, RunnerSynthesizedTextEntryStatus) { @interface RunnerSynthesizedTextEntry : NSObject -// Characters per second the synthesized text-input records are typed at. Declared here, where the -// typing happens, so the delivery budget that bounds a burst is charged the same pace the app sees. -// The edit-acknowledge window that pace is sized for is a separate assumption about the app -// (TextEntryTiming.synthesizedAcknowledgeWindowSeconds), not a value derived from this one. -+ (NSUInteger)typingSpeedCharactersPerSecond; - // Synthesizes keyboard input for the current first responder without resolving an // XCUIElement or serializing the application's accessibility tree. +// +// `charactersPerSecond` is XCTest's `typingSpeed:` argument. The caller declares it because the +// same number is what the caller's delivery budget charges a burst (#2955): a pace owned here and +// read back from Swift would let a call site type at a speed its own budget never charged. + (RunnerSynthesizedTextEntryResult *)synthesizeTextWithApplication:(id)application - text:(NSString *)text; + text:(NSString *)text + charactersPerSecond:(NSUInteger)charactersPerSecond; // Replaces the current first responder's contents with one synthesized Command-A record -// followed by a text-input record, typed at the bounded pace declared in the implementation. +// followed by a text-input record, typed at the pace the caller declares. + (RunnerSynthesizedTextEntryResult *)replaceTextWithApplication:(id)application - text:(NSString *)text; + text:(NSString *)text + charactersPerSecond:(NSUInteger)charactersPerSecond; @end diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSynthesizedTextEntry.m b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSynthesizedTextEntry.m index d76deeb4eb..5edffde88d 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSynthesizedTextEntry.m +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerSynthesizedTextEntry.m @@ -5,19 +5,10 @@ static NSString *const RunnerTextSynthesisSurface = @"text"; -// XCTest's `typingSpeed:` argument is characters per second. At 60 the 11 characters of a `fill` -// arrived at a fixture field across 131 ms (~13 ms per gap), which is faster than an app that owns -// its field's value and re-applies it after the edit (a controlled React Native `TextInput`, an -// async validator) can acknowledge: such a write lands between two characters of the burst and -// erases what was typed while it was in flight, leaving a value that is stable short of the -// request. 12 characters/second spaces them ~83 ms apart on average, which reduces that loss but -// does not remove it: XCTest does not space the characters evenly, and two of them can reach the -// app a few milliseconds apart. Against a fixture app that acknowledges each edit within 40 ms, 60 -// characters/second left 1 of 11 characters in 20 of 20 bursts, and this pace left 10 or 11. The -// command refuses a field left short; back-pressure from the field (#2906) is what would prevent -// it. The app-owned-value lane test pins the average spacing the app sees, and the delivery budget -// in TextEntryTiming bounds what the pace costs a long text. -static const NSUInteger RunnerTextEntryTypingSpeedCharactersPerSecond = 12; +// The pace (`typingSpeed:`) is a caller argument, not a constant here: TextEntryTiming owns it so +// the synthesized delivery budget charges the pace the app actually sees (#2955). XCTest's +// `typingSpeed:` is characters per second, and it spaces them unevenly — two characters of a paced +// burst can reach the app a few milliseconds apart at any pace. typedef id (*RunnerTextMsgSendInit)(id, SEL, NSString *); typedef id (*RunnerTextMsgSendInitPath)(id, SEL); @@ -49,6 +40,7 @@ static RunnerSynthesizedTextEntryResult *RunnerSynthesizeTextWithMode( id application, NSString *text, + NSUInteger charactersPerSecond, BOOL replace ); @@ -65,18 +57,16 @@ @implementation RunnerSynthesizedTextEntryResult @implementation RunnerSynthesizedTextEntry -+ (NSUInteger)typingSpeedCharactersPerSecond { - return RunnerTextEntryTypingSpeedCharactersPerSecond; -} - + (RunnerSynthesizedTextEntryResult *)synthesizeTextWithApplication:(id)application - text:(NSString *)text { - return RunnerSynthesizeTextWithMode(application, text, NO); + text:(NSString *)text + charactersPerSecond:(NSUInteger)charactersPerSecond { + return RunnerSynthesizeTextWithMode(application, text, charactersPerSecond, NO); } + (RunnerSynthesizedTextEntryResult *)replaceTextWithApplication:(id)application - text:(NSString *)text { - return RunnerSynthesizeTextWithMode(application, text, YES); + text:(NSString *)text + charactersPerSecond:(NSUInteger)charactersPerSecond { + return RunnerSynthesizeTextWithMode(application, text, charactersPerSecond, YES); } @end @@ -84,6 +74,7 @@ + (RunnerSynthesizedTextEntryResult *)replaceTextWithApplication:(id)application static RunnerSynthesizedTextEntryResult *RunnerSynthesizeTextWithMode( id application, NSString *text, + NSUInteger charactersPerSecond, BOOL replace ) { @try { @@ -151,7 +142,7 @@ + (RunnerSynthesizedTextEntryResult *)replaceTextWithApplication:(id)application bridge.typeTextSelector, text, 0.0, - RunnerTextEntryTypingSpeedCharactersPerSecond, + charactersPerSecond, YES ); ((RunnerMsgSendAddPath)objc_msgSend)(record, bridge.core.addPathSelector, path); diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift index 16f34d1618..f8ed2ac90a 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift @@ -42,8 +42,16 @@ extension RunnerTests { #if os(iOS) let postStartedAt = Date() let result = replacingExistingText - ? RunnerSynthesizedTextEntry.replaceText(withApplication: app, text: text) - : RunnerSynthesizedTextEntry.synthesizeText(withApplication: app, text: text) + ? RunnerSynthesizedTextEntry.replaceText( + withApplication: app, + text: text, + charactersPerSecond: TextEntryTiming.synthesizedCharactersPerSecond + ) + : RunnerSynthesizedTextEntry.synthesizeText( + withApplication: app, + text: text, + charactersPerSecond: TextEntryTiming.synthesizedCharactersPerSecond + ) NSLog( "[DEBUG-1874] synthesize posted %d chars status=%d tookMs=%.0f", text.count, result.status.rawValue, postStartedAt.timeIntervalSinceNow * -1000 @@ -73,74 +81,144 @@ extension RunnerTests { #endif } - struct SynthesizedReplacementStep: Equatable { - let text: String - let replacesExistingText: Bool - } + /// One synthesized text-entry plan: the posts a command makes, in the order it makes them, each + /// with the wait that follows it. The delivery budget charges this array and the dispatch loop + /// posts this same array, so the ceiling a command is refused against can never be a projection + /// of a plan the runner did not run (#2955). + struct SynthesizedTextPlan: Equatable { + /// One post. `characterCount` characters of the text, taken in order, are what it types. + struct Step: Equatable { + let characterCount: Int + /// True when this post selects the field's existing value away first. That selection is its + /// own synthesize record, so the post costs one call more than typing characters. + var replacesExistingText = false + /// Seconds charged for what follows this post: the `--delay-ms` gap before the next post, or + /// the warmup read-back after a peeled first character. The read-back costs one poll, because + /// the route that needs a budget has no element to read the character back from and its wait + /// cannot be longer than one poll of a value nobody can observe. + var pauseAfterSeconds: TimeInterval = 0 + /// Set on the post whose value the loop waits for before the rest is posted. + var warmsUpField = false - static func synthesizedReplacementSteps( - text: String, - delaySeconds: Double - ) -> [SynthesizedReplacementStep] { - guard synthesizedReplacementIsSpaced(characterCount: text.count, delaySeconds: delaySeconds) - else { - return [SynthesizedReplacementStep(text: text, replacesExistingText: true)] + /// How many private synthesize records this post runs. + var synthesizeCallCount: Int { + replacesExistingText ? 2 : 1 + } } - return Array(text).enumerated().map { index, character in - SynthesizedReplacementStep( - text: String(character), - replacesExistingText: index == 0 - ) + + let steps: [Step] + /// Recorded by the builder so the dispatch loop never re-decides the shape from `delaySeconds`. + let isSpaced: Bool + + /// One post per character, `delaySeconds` charged after every post but the last. A replacement + /// selects once, on its first post; an append never selects. + static func spacedSteps( + characterCount: Int, + delaySeconds: Double, + replacesExistingTextOnFirstPost: Bool = false + ) -> [Step] { + (0.. Bool { + /// Whether a text is posted one character per synthesize call, `delaySeconds` apart, rather than + /// as one burst. The one place this is decided for every synthesized route. + static func synthesizedDeliveryIsSpaced(characterCount: Int, delaySeconds: Double) -> Bool { delaySeconds > 0 && characterCount > 1 } - /// What a synthesized burst costs in wall clock, and the ceiling it has to fit inside before the - /// first character is posted. `synthesizedReplacementSteps` decides how a text is posted; this - /// decides whether the runner may start posting it at all. - enum SynthesizedDeliveryBudget { - /// Seconds between two characters of one synthesized burst. - static var characterInterval: TimeInterval { - 1.0 / Double(RunnerSynthesizedTextEntry.typingSpeedCharactersPerSecond()) + /// The posts a synthesized command makes. + /// + /// - A spaced request posts one character per call with the requested gap between calls; a + /// `fill` selects once, on its first post, and a `type` never selects. + /// - A burst is one post, which a `fill` selects away first. + /// - A burst from a command that repairs peels one character instead, because a field whose app + /// owns its value can reject the whole burst on the first edit's write-back; the rest follows + /// after the warmup read-back. + static func synthesizedTextPlan( + characterCount: Int, + delaySeconds: Double, + selectsExistingText: Bool, + peelsWarmupCharacter: Bool = false + ) -> SynthesizedTextPlan { + if synthesizedDeliveryIsSpaced(characterCount: characterCount, delaySeconds: delaySeconds) { + return SynthesizedTextPlan( + steps: SynthesizedTextPlan.spacedSteps( + characterCount: characterCount, + delaySeconds: delaySeconds, + replacesExistingTextOnFirstPost: selectsExistingText + ), + isSpaced: true + ) } - - /// Seconds the plan spends posting: each synthesize call types its characters at the pace and - /// pays its overhead, a spaced plan sleeps `delaySeconds` between two calls, and a plan that - /// peels one character as a warmup (`typeWarmup`) pays one more call and the wait before the - /// rest is posted. That wait is one poll here because the caller that asks has no element to - /// read the warmup character back from, so `waitForWarmupValue` has no value to wait for. - static func projectedSeconds( - textLength: Int, - delaySeconds: TimeInterval, - typeWarmup: Bool = false - ) -> TimeInterval { - let spaced = synthesizedReplacementIsSpaced(characterCount: textLength, delaySeconds: delaySeconds) - let warmupSplit = typeWarmup && textLength > 1 && !spaced - let calls = spaced ? textLength : (warmupSplit ? 2 : 1) - return Double(textLength) * characterInterval - + Double(calls) * TextEntryTiming.synthesizeCallOverhead - + Double(calls - 1) * delaySeconds - + (warmupSplit ? TextEntryTiming.pollInterval : 0) + guard peelsWarmupCharacter && characterCount > 1 else { + return SynthesizedTextPlan( + steps: [ + SynthesizedTextPlan.Step( + characterCount: characterCount, + replacesExistingText: selectsExistingText + ), + ], + isSpaced: false + ) } + return SynthesizedTextPlan( + steps: [ + SynthesizedTextPlan.Step( + characterCount: 1, + pauseAfterSeconds: TextEntryTiming.pollInterval, + warmsUpField: true + ), + SynthesizedTextPlan.Step(characterCount: characterCount - 1), + ], + isSpaced: false + ) + } - static func exceeds( - textLength: Int, - delaySeconds: TimeInterval, - typeWarmup: Bool = false - ) -> Bool { - projectedSeconds(textLength: textLength, delaySeconds: delaySeconds, typeWarmup: typeWarmup) - > TextEntryTiming.synthesizedDeliveryCeiling + /// What a plan may not cost: more wall clock than the command has left to post characters. It is + /// asked before the first character is dispatched, from the array about to be posted. + enum SynthesizedDeliveryBudget { + static func exceeds(_ plan: SynthesizedTextPlan) -> Bool { + plan.seconds > TextEntryTiming.synthesizedDeliveryCeiling } - /// Longest text `exceeds` admits at `delaySeconds`, which is what the refusal tells the caller. + /// Longest text a replacement at `delaySeconds` still admits, which is the number the refusal + /// tells the caller. It asks the same plan admission judges, so the recovery hint can never + /// promise a length the route then refuses. static func maxTextLength(delaySeconds: TimeInterval) -> Int { var length = 1 - while !exceeds(textLength: length + 1, delaySeconds: delaySeconds) { + while !exceeds( + synthesizedTextPlan( + characterCount: length + 1, + delaySeconds: delaySeconds, + selectsExistingText: true + ) + ) { length += 1 } return length @@ -153,10 +231,12 @@ extension RunnerTests { ) -> SynthesizedReplacementRouteOutcome { #if os(iOS) NSLog("AGENT_DEVICE_RUNNER_TEXT_ENTRY_ROUTE route=synthesized-first-responder-replacement") - if SynthesizedDeliveryBudget.exceeds( - textLength: request.text.count, - delaySeconds: request.delaySeconds - ) { + let plan = Self.synthesizedTextPlan( + characterCount: request.text.count, + delaySeconds: request.delaySeconds, + selectsExistingText: true + ) + if SynthesizedDeliveryBudget.exceeds(plan) { NSLog( "AGENT_DEVICE_RUNNER_TEXT_ENTRY_ROUTE route=synthesized-first-responder-replacement " + "reason=delivery-budget-refused chars=%d budgetChars=%d", @@ -174,14 +254,13 @@ extension RunnerTests { ) ) } - let steps = Self.synthesizedReplacementSteps( - text: request.text, - delaySeconds: request.delaySeconds - ) - for (index, step) in steps.enumerated() { + var postedCount = 0 + let characters = Array(request.text) + for step in plan.steps { + let nextCount = postedCount + step.characterCount switch request.synthesizer.enterText( app: request.app, - text: step.text, + text: String(characters[postedCount.. 0 { + sleepFor(step.pauseAfterSeconds) } } // The private synthesize call returns at post time, not commit time, and this route never diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift index 96f3a3c3e3..75f1b2561a 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift @@ -36,7 +36,12 @@ extension RunnerTests { let recoveryBudget = SynthesizedDeliveryBudget.maxTextLength( delaySeconds: Double(recoveryDelay) / 1000 ) - return "Fill at most \(SynthesizedDeliveryBudget.maxTextLength(delaySeconds: 0)) characters at a time without --delay-ms and append the rest with separate type commands, keeping each command inside that budget. --delay-ms lowers the budget, because each character then gets its own synthesize call and each gap between characters pays the delay: \(recoveryBudget) characters at --delay-ms \(recoveryDelay). A longer timeout does not help: this route is chosen when the accessibility channel is already degraded, and the pace is what makes the text long." + // Kept inside the 400-character diagnostic bound the host applies to every error string + // (`REDACTED_STRING_MAX_LENGTH` in packages/kernel/src/redaction.ts): a hint truncated at + // that boundary looks actionable and is not, which is the failure the iOS open-command hint + // already refuses to produce. The previous wording cost 460 characters and lost its last + // sentence on the wire. + return "Fill at most \(SynthesizedDeliveryBudget.maxTextLength(delaySeconds: 0)) characters per command without --delay-ms and append the rest with separate type commands. --delay-ms lowers the limit: each character then gets its own synthesize call and each gap pays the delay, so \(recoveryDelay) ms fits \(recoveryBudget). This route is chosen when the accessibility channel is already degraded, so a longer timeout does not help." } } } @@ -71,12 +76,23 @@ extension RunnerTests { static let synthesizedDeliveryCeiling: TimeInterval = RunnerTests.mainThreadExecutionTimeout - synthesizedReplacementFocusAllowance - synthesizedCommitCeiling - /// The edit-acknowledge window the synthesized pace is sized for: on average, a burst's - /// characters reach the app at least this far apart. XCTest spaces them unevenly, so an app - /// with this window can still lose a character that arrives early; the command then refuses the - /// short value (#2906 tracks preventing it). The pace policy test and the app-owned-value lane - /// test pin the pace against it. - static let synthesizedAcknowledgeWindowSeconds: TimeInterval = 0.04 + /// XCTest's `typingSpeed:` argument: characters per second a synthesized text-input record is + /// typed at. At 60 the 11 characters of a `fill` arrived at a fixture field across 131 ms + /// (~13 ms per gap), which is faster than an app that owns its field's value and re-applies it + /// after the edit (a controlled React Native `TextInput`, an async validator) can acknowledge: + /// such a write lands between two characters of the burst and erases what was typed while it was + /// in flight, leaving a value that is stable short of the request. 12 characters/second spaces + /// them ~83 ms apart on average, which reduces that loss but does not remove it: XCTest does not + /// space the characters evenly, and two of them can reach the app a few milliseconds apart. + /// Against a fixture app that acknowledges each edit within 40 ms, 60 characters/second left 1 + /// of 11 characters in 20 of 20 bursts, and this pace left 10 or 11. The command refuses a + /// field left short; back-pressure from the field (#2906) is what would prevent it. This is the + /// one pace declaration: it is passed to the bridge that types, and the delivery budget below + /// charges it, so the pace the app sees and the pace the command is refused at cannot drift. It + /// is a `UInt` because that is the bridge's argument type, so no call site converts it. + static let synthesizedCharactersPerSecond: UInt = 12 + /// Seconds two characters of one synthesized burst are typed apart. + static let synthesizedCharacterInterval: TimeInterval = 1.0 / Double(synthesizedCharactersPerSecond) /// What one private synthesize call costs beyond typing its characters, which a `--delay-ms` /// plan pays once per character. One-character calls at the shipped pace took 222 ms on average /// on an iPhone 17 Pro simulator (212–617 ms over 235 calls), 83 ms of it the character. diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextTyping.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextTyping.swift index 18de273f9a..b3008eacad 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextTyping.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextTyping.swift @@ -136,6 +136,39 @@ extension RunnerTests { } } + // The shape this command posts, decided once: a spaced per-character plan, a peeled warmup plus + // the rest, or one burst. The delivery budget below charges this array and the dispatch loops + // below post it, so a refusal cannot come from an estimate of a plan the runner never ran + // (#2955). The warmup peel is charged here even though only the burst route performs it, and + // the spaced route skips it: a spaced plan already posts per character. + let synthesizedTypePlan = Self.synthesizedTextPlan( + characterCount: text.count, + delaySeconds: delaySeconds, + selectsExistingText: false, + peelsWarmupCharacter: repairMode != .none + ) + // One answer for the whole command, not one per post. The synthesized pace is slowed for fields + // whose app owns the value, and a burst that long outlasts the command while the runner is still + // posting it, so the ceiling is charged the whole command: charging the chunk instead made 215 + // characters look like 1 + 214, each inside the budget. + let synthesizedTypeCarriesTheCommand = !SynthesizedDeliveryBudget.exceeds(synthesizedTypePlan) + + // Application-wide typing is the only channel left for this post: it is what the command's budget + // falls back to, and what private synthesis falls back to. The route cannot read the value back + // afterwards, so the text arrives unverified either way. The refusal is charged the whole command + // and the post is a chunk of it, so the log names both lengths. + func typeApplicationWide(_ value: String, reason: String) -> (element: XCUIElement?, failure: TextEntryFailure?) { + NSLog( + "AGENT_DEVICE_RUNNER_TEXT_ENTRY_ROUTE route=xctest-application-fallback reason=%@ chars=%d commandChars=%d", + reason, + value.count, + text.count + ) + textEntryRoute = "xctest-application-fallback" + app.typeText(value) + return (resolveTextEntryElement(app: app, target: activeTarget), nil) + } + func typeIntoCurrentTarget(_ value: String) -> (element: XCUIElement?, failure: TextEntryFailure?) { #if os(iOS) if shouldUseSynthesizedFirstResponderType { @@ -163,39 +196,18 @@ extension RunnerTests { return (currentTarget, nil) } else if activeTarget.prefersFocusedElement && isKeyboardVisible(app: app) { #if os(iOS) - // Text the command budget cannot carry at the synthesized pace goes through application-wide - // typing instead. The synthesizer's pace is slowed for fields whose app owns the value, and a - // burst that long outlasts the command while the runner is still posting it. The ceiling is - // what the command's watchdog leaves, so it is charged the whole command: an append peels its - // first character for warmup and a `--delay-ms` plan dispatches one character at a time, and - // neither chunk would look long on its own. This branch's target has no element to type into, - // so nothing can be read back afterwards: the value arrives unverified, as it does for this - // route's older synthesizer-unavailable fallback. - if SynthesizedDeliveryBudget.exceeds( - textLength: text.count, - delaySeconds: delaySeconds, - typeWarmup: repairMode != .none - ) { - textEntryRoute = "xctest-application-fallback" - NSLog( - "AGENT_DEVICE_RUNNER_TEXT_ENTRY_ROUTE route=xctest-application-fallback " - + "reason=delivery-budget chars=%d", - value.count - ) - app.typeText(value) - return (resolveTextEntryElement(app: app, target: activeTarget), nil) + // Two ways this post leaves the synthesized channel, and both hand the text to + // application-wide typing: the command's own budget refused it, or XCTest's private synthesis + // is unavailable. This branch's target has no element to type into, so nothing can be read + // back afterwards: the value arrives unverified either way. + guard synthesizedTypeCarriesTheCommand else { + return typeApplicationWide(value, reason: "delivery-budget") } textEntryRoute = "synthesized-first-responder" NSLog("AGENT_DEVICE_RUNNER_TEXT_ENTRY_ROUTE route=synthesized-first-responder") - let action = synthesizer.enterText( - app: app, - text: value, - replacingExistingText: false - ) - switch action { + switch synthesizer.enterText(app: app, text: value, replacingExistingText: false) { case .fallback: - textEntryRoute = "xctest-application-fallback" - app.typeText(value) + return typeApplicationWide(value, reason: "synthesis-unavailable") case .raise(let message): NSException( name: NSExceptionName.internalInconsistencyException, @@ -238,17 +250,20 @@ extension RunnerTests { } let characters = Array(text) - if delaySeconds > 0 && characters.count > 1 { + if synthesizedTypePlan.isSpaced { var typedTarget: XCUIElement? let delayedTypeStartedAt = Date() - for (index, character) in characters.enumerated() { - let dispatch = typeIntoCurrentTarget(String(character)) + var postedCount = 0 + for step in synthesizedTypePlan.steps { + let nextCount = postedCount + step.characterCount + let dispatch = typeIntoCurrentTarget(String(characters[postedCount.. 0 { + sleepFor(step.pauseAfterSeconds) } } logTextEntryPhase( @@ -288,8 +303,9 @@ extension RunnerTests { } let typedTarget: XCUIElement? - if repairMode != .none && characters.count > 1 { - let firstCharacter = String(characters[0]) + if let warmupSplit = synthesizedTypePlan.warmupSplit { + let peeledCount = warmupSplit.first.characterCount + let firstCharacter = String(characters[0.. SynthesizedTextEntryAction { - steps.append( - SynthesizedReplacementStep( - text: text, - replacesExistingText: replacingExistingText - ) - ) + posts.append(Post(text: text, replacesExistingText: replacingExistingText)) return .continueTyping } } @@ -226,29 +236,57 @@ extension RunnerTests { } } - func testSynthesizedReplacementPacesCharactersAfterSelectingOnce() { + // The plan is the single source for both the cost and the posts: these cases name the array the + // replacement route will execute, character counts and all, because the budget below charges this + // same array rather than a second estimate of it (#2955). + func testSynthesizedReplacementPlanPacesCharactersAfterSelectingOnce() { XCTAssertEqual( - Self.synthesizedReplacementSteps(text: "abc", delaySeconds: 0.05), - [ - SynthesizedReplacementStep(text: "a", replacesExistingText: true), - SynthesizedReplacementStep(text: "b", replacesExistingText: false), - SynthesizedReplacementStep(text: "c", replacesExistingText: false), - ] + Self.synthesizedTextPlan(characterCount: 3, delaySeconds: 0.05, selectsExistingText: true), + SynthesizedTextPlan( + steps: [ + SynthesizedTextPlan.Step(characterCount: 1, replacesExistingText: true, pauseAfterSeconds: 0.05), + SynthesizedTextPlan.Step(characterCount: 1, pauseAfterSeconds: 0.05), + SynthesizedTextPlan.Step(characterCount: 1), + ], + isSpaced: true + ) + ) + XCTAssertEqual( + Self.synthesizedTextPlan(characterCount: 3, delaySeconds: 0, selectsExistingText: true), + SynthesizedTextPlan( + steps: [SynthesizedTextPlan.Step(characterCount: 3, replacesExistingText: true)], + isSpaced: false + ) + ) + } + + // A select-and-type post runs two synthesize records — the Command-A selection and the text — so a + // burst that replaces costs more than the characters it types. Charging one call per post is the + // projection defect this plan exists to remove, one level down. + func testSynthesizedPlanChargesAReplacingPostTwoSynthesizeCalls() { + let replacing = SynthesizedTextPlan.Step(characterCount: 1, replacesExistingText: true) + XCTAssertEqual(replacing.synthesizeCallCount, 2) + XCTAssertEqual( + SynthesizedTextPlan.Step(characterCount: 1).synthesizeCallCount, + 1 ) XCTAssertEqual( - Self.synthesizedReplacementSteps(text: "abc", delaySeconds: 0), - [SynthesizedReplacementStep(text: "abc", replacesExistingText: true)] + Self.synthesizedTextPlan(characterCount: 1, delaySeconds: 0, selectsExistingText: true).seconds, + TextEntryTiming.synthesizedCharacterInterval + + 2 * TextEntryTiming.synthesizeCallOverhead ) } // The pace is what keeps a field the app owns from losing most of a replacement (#2080), so it // cannot drift on its own: one character interval has to leave that app at least twice the - // acknowledge window the route is sized for. The host lane runs this on every PR; the iOS lane's - // app-owned-value test checks the spacing the app actually receives. + // acknowledge window the route is sized for. The pace is declared once in TextEntryTiming and + // passed to the bridge that types, so the budget cannot charge a speed the app never sees. The + // host lane runs this on every PR; the iOS lane's app-owned-value test checks the spacing the app + // actually receives. func testSynthesizedPaceLeavesRoomForAnAppToAcknowledgeEachEdit() { XCTAssertGreaterThanOrEqual( - SynthesizedDeliveryBudget.characterInterval, - 2 * TextEntryTiming.synthesizedAcknowledgeWindowSeconds + TextEntryTiming.synthesizedCharacterInterval, + 2 * TextEntryTestAssumptions.synthesizedAcknowledgeWindowSeconds ) } @@ -259,59 +297,130 @@ extension RunnerTests { func testSynthesizedDeliveryBudgetRefusesTextThatOutrunsTheCommand() { let fits = SynthesizedDeliveryBudget.maxTextLength(delaySeconds: 0) XCTAssertGreaterThan(fits, 0) - XCTAssertFalse(SynthesizedDeliveryBudget.exceeds(textLength: fits, delaySeconds: 0)) - XCTAssertTrue(SynthesizedDeliveryBudget.exceeds(textLength: fits + 1, delaySeconds: 0)) + XCTAssertFalse( + SynthesizedDeliveryBudget.exceeds( + Self.synthesizedTextPlan(characterCount: fits, delaySeconds: 0, selectsExistingText: true) + ) + ) + XCTAssertTrue( + SynthesizedDeliveryBudget.exceeds( + Self.synthesizedTextPlan(characterCount: fits + 1, delaySeconds: 0, selectsExistingText: true) + ) + ) } - // A spaced plan posts each character in its own synthesize call and sleeps between two of them, - // so a character costs the pace, the call's overhead and the delay together, not the larger of - // pace and delay. The delay checked is the retry TEXT_INPUT_COMMIT_NOT_OBSERVED recommends. - func testSpacedDeliveryBudgetChargesEachCharacterItsCallAndDelay() { + // A spaced plan posts each character in its own synthesize call and waits between two of them, so a + // character costs the pace, the call's overhead and the delay together, not the larger of pace and + // delay. The delay checked is the retry TEXT_INPUT_COMMIT_NOT_OBSERVED recommends. + func testSpacedDeliveryBudgetRefusesSoonerThanABurst() { let delay = Double(TextEntryTiming.recoveryDelayMilliseconds) / 1000 let fits = SynthesizedDeliveryBudget.maxTextLength(delaySeconds: delay) - XCTAssertFalse(SynthesizedDeliveryBudget.exceeds(textLength: fits, delaySeconds: delay)) - XCTAssertTrue(SynthesizedDeliveryBudget.exceeds(textLength: fits + 1, delaySeconds: delay)) - XCTAssertEqual( - SynthesizedDeliveryBudget.projectedSeconds(textLength: 10, delaySeconds: delay) - - SynthesizedDeliveryBudget.projectedSeconds(textLength: 9, delaySeconds: delay), - SynthesizedDeliveryBudget.characterInterval - + TextEntryTiming.synthesizeCallOverhead - + delay, - accuracy: 1e-9 + XCTAssertFalse( + SynthesizedDeliveryBudget.exceeds( + Self.synthesizedTextPlan(characterCount: fits, delaySeconds: delay, selectsExistingText: true) + ) + ) + XCTAssertTrue( + SynthesizedDeliveryBudget.exceeds( + Self.synthesizedTextPlan(characterCount: fits + 1, delaySeconds: delay, selectsExistingText: true) + ) ) XCTAssertLessThan(fits, SynthesizedDeliveryBudget.maxTextLength(delaySeconds: 0)) XCTAssertLessThan(SynthesizedDeliveryBudget.maxTextLength(delaySeconds: 0.2), fits) + // Policy floor, not a formula copy: one more spaced character pays its pace, its own + // synthesize call AND the delay, so the gap between two adjacent lengths cannot come cheaper + // than all three. A builder that dropped `pauseAfterSeconds` (charge the delay nowhere, or + // sleep for nothing) lands under this floor. + let cost = Self.synthesizedTextPlan(characterCount: 10, delaySeconds: delay, selectsExistingText: true).seconds + - Self.synthesizedTextPlan(characterCount: 9, delaySeconds: delay, selectsExistingText: true).seconds + XCTAssertGreaterThanOrEqual( + cost, + TextEntryTiming.synthesizedCharacterInterval + + TextEntryTiming.synthesizeCallOverhead + + 0.8 * delay + ) + } + + // Both builders must describe the SAME text the executor will slice: every step's characters + // accounted for, the spaced plan covering each character exactly once, and the peeled plan's + // rest posting what the warmup did not. An off-by-one here posts a character twice or never, + // and the charged seconds would describe a different command than the one run. + func testSynthesizedPlansCoverTheirTextExactlyOnce() { + for length in [2, 3, 11, 240] { + for delay in [0.0, 0.08] { + let replacement = Self.synthesizedTextPlan( + characterCount: length, delaySeconds: delay, selectsExistingText: true + ) + XCTAssertEqual(replacement.steps.reduce(0) { $0 + $1.characterCount }, length) + let type = Self.synthesizedTextPlan( + characterCount: length, delaySeconds: delay, selectsExistingText: false, peelsWarmupCharacter: true + ) + XCTAssertEqual(type.steps.reduce(0) { $0 + $1.characterCount }, length) + if let split = type.warmupSplit { + XCTAssertEqual(split.first.characterCount + split.rest.characterCount, length) + // A peel only exists on an unspaced burst, and its read-back is the one poll the budget + // route can afford: a spaced plan covers the same text with per-character posts instead. + XCTAssertEqual(split.first.pauseAfterSeconds, TextEntryTiming.pollInterval) + } + } + } } - // A `type` plan peels one character as a warmup and posts the rest afterwards, so the same text - // costs one synthesize call and one wait more than the single burst the replacement route posts. - // Without this the estimate charged a burst, which is what made the over-budget branch of the - // keyboard-visible route unreachable: 215 characters looked like 1 + 214, each inside the budget. - func testTypeWarmupSplitCostsOneMoreCallThanASingleBurst() { + // The recovery tells the caller to fill ≤ N and append the rest with `type`. That only works if + // an appending command with the peeled warmup is admitted at exactly the same lengths a fill is; + // if the peel ever cost more, the hint would send the caller into a refusal on the second chunk. + func testAppendChunksAreAdmittedAtTheFillsOwnBudget() { + for delay in [0.0, Double(TextEntryTiming.recoveryDelayMilliseconds) / 1000] { + let fillFits = SynthesizedDeliveryBudget.maxTextLength(delaySeconds: delay) + XCTAssertFalse( + SynthesizedDeliveryBudget.exceeds( + Self.synthesizedTextPlan( + characterCount: fillFits, + delaySeconds: delay, + selectsExistingText: false, + peelsWarmupCharacter: true + ) + ), + "delay \(delay): the hint's recovery refuses what its own number recommends" + ) + } + } + + // A `type` command peels one character as a warmup and posts the rest, so the same text costs one + // synthesize call and one read-back more than the single burst the replacement route posts. Without + // this the estimate charged a burst, which made the over-budget branch of the keyboard-visible route + // unreachable: 215 characters looked like 1 + 214, each inside the budget. + func testTypePlanPeelsAWarmupCharacterAndChargesItsReadBack() { let length = 20 - let withWarmup = SynthesizedDeliveryBudget.projectedSeconds( - textLength: length, - delaySeconds: 0, - typeWarmup: true + let warmup = TextEntryTiming.pollInterval + XCTAssertEqual( + Self.synthesizedTextPlan(characterCount: length, delaySeconds: 0, selectsExistingText: false, peelsWarmupCharacter: true), + SynthesizedTextPlan( + steps: [ + SynthesizedTextPlan.Step(characterCount: 1, pauseAfterSeconds: warmup, warmsUpField: true), + SynthesizedTextPlan.Step(characterCount: length - 1), + ], + isSpaced: false + ) ) - XCTAssertGreaterThan( - withWarmup, - SynthesizedDeliveryBudget.projectedSeconds(textLength: length, delaySeconds: 0) + // Floor, not arithmetic copy: the peeled plan costs at least one synthesize call AND the + // one-poll read-back more than the same burst unpeeled. Dropping either charge falls under it. + XCTAssertGreaterThanOrEqual( + Self.synthesizedTextPlan(characterCount: length, delaySeconds: 0, selectsExistingText: false, peelsWarmupCharacter: true).seconds + - Self.synthesizedTextPlan(characterCount: length, delaySeconds: 0, selectsExistingText: false, peelsWarmupCharacter: false).seconds, + TextEntryTiming.synthesizeCallOverhead + TextEntryTiming.pollInterval ) - XCTAssertEqual( - withWarmup - SynthesizedDeliveryBudget.projectedSeconds(textLength: length, delaySeconds: 0), - TextEntryTiming.synthesizeCallOverhead + TextEntryTiming.pollInterval, - accuracy: 1e-9 + // A spaced `type` already posts per character, and a single character has no rest to post, so + // neither shape peels. + XCTAssertNil( + Self.synthesizedTextPlan(characterCount: length, delaySeconds: 0.2, selectsExistingText: false, peelsWarmupCharacter: true).warmupSplit ) - // The split mirrors the plan: a spaced `type` already posts per character, and a single - // character has no rest to post. - XCTAssertEqual( - SynthesizedDeliveryBudget.projectedSeconds(textLength: length, delaySeconds: 0.2, typeWarmup: true), - SynthesizedDeliveryBudget.projectedSeconds(textLength: length, delaySeconds: 0.2) + XCTAssertNil( + Self.synthesizedTextPlan(characterCount: 1, delaySeconds: 0, selectsExistingText: false, peelsWarmupCharacter: true).warmupSplit ) XCTAssertEqual( - SynthesizedDeliveryBudget.projectedSeconds(textLength: 1, delaySeconds: 0, typeWarmup: true), - SynthesizedDeliveryBudget.projectedSeconds(textLength: 1, delaySeconds: 0) + Self.synthesizedTextPlan(characterCount: length, delaySeconds: 0.2, selectsExistingText: false, peelsWarmupCharacter: true), + Self.synthesizedTextPlan(characterCount: length, delaySeconds: 0.2, selectsExistingText: false, peelsWarmupCharacter: false) ) } @@ -323,14 +432,21 @@ extension RunnerTests { // The recovery has to tell the caller to split the text: waiting it out or raising a timeout // does nothing, because the pace is what makes the burst long, not the host being slow. A // delayed request fits fewer characters, so the hint names both budgets rather than promising - // the undelayed one to a caller retrying with --delay-ms. + // the undelayed one to a caller retrying with --delay-ms. The numbers come from the same + // admission the route applied, so a hint can never promise a length it then refuses. let hint = TextEntryFailure.synthesisBudgetExceeded.hint - XCTAssertTrue(hint.contains("\(SynthesizedDeliveryBudget.maxTextLength(delaySeconds: 0)) characters at a time")) + XCTAssertTrue( + hint.contains("\(SynthesizedDeliveryBudget.maxTextLength(delaySeconds: 0)) characters per command"), + hint + ) let recoveryDelay = TextEntryTiming.recoveryDelayMilliseconds let recoveryBudget = SynthesizedDeliveryBudget.maxTextLength( delaySeconds: Double(recoveryDelay) / 1000 ) - XCTAssertTrue(hint.contains("\(recoveryBudget) characters at --delay-ms \(recoveryDelay)")) + XCTAssertTrue(hint.contains("\(recoveryDelay) ms fits \(recoveryBudget)"), hint) + // The host redacts any diagnostic string past 400 characters, and a hint cut there reads as + // actionable but is not. This one carried 460 and silently lost its last sentence on the wire. + XCTAssertLessThanOrEqual(hint.count, 400, "hint outlives the host's diagnostic bound: \(hint.count)") } #if os(iOS) @@ -363,11 +479,11 @@ extension RunnerTests { ) XCTAssertEqual( - synthesizer.steps, + synthesizer.posts, [ - SynthesizedReplacementStep(text: "a", replacesExistingText: true), - SynthesizedReplacementStep(text: "b", replacesExistingText: false), - SynthesizedReplacementStep(text: "c", replacesExistingText: false), + RecordingTextEntrySynthesizer.Post(text: "a", replacesExistingText: true), + RecordingTextEntrySynthesizer.Post(text: "b", replacesExistingText: false), + RecordingTextEntrySynthesizer.Post(text: "c", replacesExistingText: false), ] ) XCTAssertNil(result.verified) From 705ef4bc009da2fc5c98ff490e07ea99331be5a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 26 Sep 2026 08:22:44 +0200 Subject: [PATCH 2/2] refactor(ios-runner): post a charged text-entry plan through one executor Both synthesized routes now run the plan through runSynthesizedTextPlan: the executor slices the next characters per step, takes the wait the plan carries, and hands a warmup step to the route's own read-back. The plan records no shape flag; the burst/warmup/paced phase naming derives from the steps. A post that stops the plan answers with its typed failure before the target reads the last element, so a vanished input cannot turn a focused failure into a snapshot error. --- .../RunnerTests+SynthesizedTextEntry.swift | 131 +++++++++---- .../RunnerTests+TextEntry.swift | 3 +- .../RunnerTests+TextTyping.swift | 184 +++++++----------- ...unnerTests+SynthesizedTextEntryTests.swift | 7 +- .../RunnerTests+TextEntryPolicyTests.swift | 103 +++++++--- 5 files changed, 249 insertions(+), 179 deletions(-) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift index f8ed2ac90a..454f309cb1 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SynthesizedTextEntry.swift @@ -107,8 +107,6 @@ extension RunnerTests { } let steps: [Step] - /// Recorded by the builder so the dispatch loop never re-decides the shape from `delaySeconds`. - let isSpaced: Bool /// One post per character, `delaySeconds` charged after every post but the last. A replacement /// selects once, on its first post; an append never selects. @@ -137,12 +135,11 @@ extension RunnerTests { } } - /// The peeled first character and the rest, when the plan warms the field up before posting it. - var warmupSplit: (first: Step, rest: Step)? { - guard steps.count == 2, let first = steps.first, first.warmsUpField else { - return nil - } - return (first, steps[1]) + /// Whether the plan hands the text over one character at a time, which is what a `--delay-ms` + /// request builds. A burst is a single post and a repair peels a warmup character, so both keep + /// a phase log per post; a paced plan logs one phase for the whole delivery instead. + var pacesEveryCharacter: Bool { + steps.count > 1 && !steps.contains(where: \.warmsUpField) } } @@ -172,8 +169,7 @@ extension RunnerTests { characterCount: characterCount, delaySeconds: delaySeconds, replacesExistingTextOnFirstPost: selectsExistingText - ), - isSpaced: true + ) ) } guard peelsWarmupCharacter && characterCount > 1 else { @@ -183,8 +179,7 @@ extension RunnerTests { characterCount: characterCount, replacesExistingText: selectsExistingText ), - ], - isSpaced: false + ] ) } return SynthesizedTextPlan( @@ -195,8 +190,7 @@ extension RunnerTests { warmsUpField: true ), SynthesizedTextPlan.Step(characterCount: characterCount - 1), - ], - isSpaced: false + ] ) } @@ -225,6 +219,56 @@ extension RunnerTests { } } + /// Where one post leaves the plan: move on to the next step, or stop the command with a reason the + /// route has already reported. + enum SynthesizedStepDispatch { + case posted + case stop + } + + struct SynthesizedPlanRun { + let postedCharacterCount: Int + /// True when a post stopped the plan before its last step. + let stoppedEarly: Bool + } + + /// Posts a plan the delivery budget just charged: each step slices the next characters off the + /// text, and every wait the plan carries is taken. Both synthesized routes post through here so the + /// array a command is refused against cannot drift from the posts the command makes (#2955). + /// + /// `waitAfterWarmupCharacter` replaces the charged pause on a warmup step with what the route wants + /// to wait on, handed the characters that post made. The route that has an + /// element reads the peeled character back and waits up to `warmupValueTimeout` for the app to + /// accept it; the route that needed a budget has no element, so its wait is the plan's one poll of + /// a value nobody can observe. Both are waits against the same charge, which is why the read-back + /// belongs to the route and the plan only carries the character it waits for. + @MainActor + func runSynthesizedTextPlan( + _ plan: SynthesizedTextPlan, + text: String, + post: (_ characters: String, _ step: SynthesizedTextPlan.Step) -> SynthesizedStepDispatch, + waitAfterWarmupCharacter: (_ characters: String) -> Void, + didPostStep: (_ step: SynthesizedTextPlan.Step) -> Void = { _ in } + ) -> SynthesizedPlanRun { + let characters = Array(text) + var postedCount = 0 + for step in plan.steps { + let nextCount = postedCount + step.characterCount + if post(String(characters[postedCount.. 0 { + sleepFor(step.pauseAfterSeconds) + } + } + return SynthesizedPlanRun(postedCharacterCount: postedCount, stoppedEarly: false) + } + @MainActor func runSynthesizedReplacementRoute( _ request: SynthesizedReplacementRequest @@ -254,33 +298,38 @@ extension RunnerTests { ) ) } - var postedCount = 0 - let characters = Array(request.text) - for step in plan.steps { - let nextCount = postedCount + step.characterCount - switch request.synthesizer.enterText( - app: request.app, - text: String(characters[postedCount.. 0 { - sleepFor(step.pauseAfterSeconds) - } + // A private synthesis channel that is gone mid-plan leaves the command the same + // point-and-focus fallback it had before the plan existed. + let synthesisAvailable = runSynthesizedTextPlan( + plan, + text: request.text, + post: { slice, step in + switch request.synthesizer.enterText( + app: request.app, + text: slice, + replacingExistingText: step.replacesExistingText + ) { + case .continueTyping: + return .posted + case .fallback: + return .stop + case .raise(let message): + NSException( + name: NSExceptionName.internalInconsistencyException, + reason: message ?? "private XCTest text synthesis failed" + ).raise() + return .stop + } + }, + // A replacement never peels a warmup character, so no step asks for a read-back. + waitAfterWarmupCharacter: { _ in } + ) + if synthesisAvailable.stoppedEarly { + NSLog("AGENT_DEVICE_RUNNER_TEXT_ENTRY_ROUTE route=verified-fallback reason=synthesis-unavailable") + guard let point = request.target.refreshPoint else { return .notApplicable } + return .fallback( + focusTextInputForTextEntry(app: request.app, x: point.x, y: point.y) + ) } // The private synthesize call returns at post time, not commit time, and this route never // resolves an XCUIElement, so without this wait it had no way to notice a dropped or diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift index 75f1b2561a..7a20e82b84 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextEntry.swift @@ -39,8 +39,7 @@ extension RunnerTests { // Kept inside the 400-character diagnostic bound the host applies to every error string // (`REDACTED_STRING_MAX_LENGTH` in packages/kernel/src/redaction.ts): a hint truncated at // that boundary looks actionable and is not, which is the failure the iOS open-command hint - // already refuses to produce. The previous wording cost 460 characters and lost its last - // sentence on the wire. + // already refuses to produce. return "Fill at most \(SynthesizedDeliveryBudget.maxTextLength(delaySeconds: 0)) characters per command without --delay-ms and append the rest with separate type commands. --delay-ms lowers the limit: each character then gets its own synthesize call and each gap pays the delay, so \(recoveryDelay) ms fits \(recoveryBudget). This route is chosen when the accessibility channel is already degraded, so a longer timeout does not help." } } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextTyping.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextTyping.swift index b3008eacad..1e44e388f0 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextTyping.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextTyping.swift @@ -235,138 +235,100 @@ extension RunnerTests { ) } - func waitForWarmupValue(_ expectedValue: String?, target: TextEntryTarget) { - guard let expectedValue else { + func readBackWarmupCharacter(_ peeledCharacter: String) { + // A route with an element reads the peeled character back and waits up to `warmupValueTimeout` + // for the app to accept it. The route that had to be budgeted has no element, so its expected + // value is nil and this is the plan's one charged poll. + let warmupExpectedText = expectedTextEntryValue( + typedText: peeledCharacter, + mode: repairMode, + initialText: initialText + ) + guard let warmupExpectedText else { sleepFor(TextEntryTiming.pollInterval) return } let deadline = Date().addingTimeInterval(TextEntryTiming.warmupValueTimeout) while Date() < deadline { - if editableTextValue(for: resolveTextEntryElement(app: app, target: target)) == expectedValue { + if editableTextValue(for: resolveTextEntryElement(app: app, target: activeTarget)) == warmupExpectedText { return } sleepFor(TextEntryTiming.pollInterval) } } - let characters = Array(text) - if synthesizedTypePlan.isSpaced { - var typedTarget: XCUIElement? - let delayedTypeStartedAt = Date() - var postedCount = 0 - for step in synthesizedTypePlan.steps { - let nextCount = postedCount + step.characterCount - let dispatch = typeIntoCurrentTarget(String(characters[postedCount.. 0 { - sleepFor(step.pauseAfterSeconds) + if step.warmsUpField { + // The read-back wants the element this post just typed into, not a re-resolve from scratch. + activeTarget = activeTarget.withElement(typedTarget) } - } - logTextEntryPhase( - commandId: commandId, - phase: "type-delayed", - startedAt: delayedTypeStartedAt, - chars: characters.count, - mode: repairMode - ) - if repairMode == .none { - logTextEntryPhase(commandId: commandId, phase: "total", startedAt: totalStartedAt, chars: text.count, mode: repairMode) - return TextEntryResult( - verified: nil, - repaired: false, - expectedText: nil, - observedText: nil, - textEntryRoute: textEntryRoute + return .posted + }, + waitAfterWarmupCharacter: { peeledCharacter in + let warmupStartedAt = Date() + readBackWarmupCharacter(peeledCharacter) + logTextEntryPhase( + commandId: commandId, + phase: "warmup", + startedAt: warmupStartedAt, + chars: 1, + mode: repairMode + ) + }, + didPostStep: { step in + guard !synthesizedTypePlan.pacesEveryCharacter else { + return + } + let phase: String + if step.warmsUpField { + phase = "type-first" + sawWarmupPost = true + } else { + phase = sawWarmupPost ? "type-remaining" : "type-all" + } + logTextEntryPhase( + commandId: commandId, + phase: phase, + startedAt: stepStartedAt, + chars: step.characterCount, + mode: repairMode ) } - let verifyStartedAt = Date() - var result = verifyTextEntryWithRepairIfNeeded( - app: app, - target: activeTarget.withElement(typedTarget), - expectedText: expectedText, - repairMode: repairMode - ) + ) + if synthesizedTypePlan.pacesEveryCharacter { + // Paced delivery is one paced burst: the phase covers every post and names the whole text + // rather than its last character. logTextEntryPhase( commandId: commandId, - phase: "verify", - startedAt: verifyStartedAt, - chars: characters.count, + phase: "type-delayed", + startedAt: pacedStartedAt, + chars: text.count, mode: repairMode ) - logTextEntryPhase(commandId: commandId, phase: "total", startedAt: totalStartedAt, chars: text.count, mode: repairMode) - result.textEntryRoute = textEntryRoute - return result } - - let typedTarget: XCUIElement? - if let warmupSplit = synthesizedTypePlan.warmupSplit { - let peeledCount = warmupSplit.first.characterCount - let firstCharacter = String(characters[0.. 0) + if let warmup = type.steps.first, warmup.warmsUpField { + XCTAssertEqual(warmup.characterCount, 1) + XCTAssertEqual(warmup.pauseAfterSeconds, TextEntryTiming.pollInterval) } } } @@ -399,8 +397,7 @@ extension RunnerTests { steps: [ SynthesizedTextPlan.Step(characterCount: 1, pauseAfterSeconds: warmup, warmsUpField: true), SynthesizedTextPlan.Step(characterCount: length - 1), - ], - isSpaced: false + ] ) ) // Floor, not arithmetic copy: the peeled plan costs at least one synthesize call AND the @@ -412,11 +409,13 @@ extension RunnerTests { ) // A spaced `type` already posts per character, and a single character has no rest to post, so // neither shape peels. - XCTAssertNil( - Self.synthesizedTextPlan(characterCount: length, delaySeconds: 0.2, selectsExistingText: false, peelsWarmupCharacter: true).warmupSplit + XCTAssertFalse( + Self.synthesizedTextPlan(characterCount: length, delaySeconds: 0.2, selectsExistingText: false, peelsWarmupCharacter: true) + .steps.contains(where: \.warmsUpField) ) - XCTAssertNil( - Self.synthesizedTextPlan(characterCount: 1, delaySeconds: 0, selectsExistingText: false, peelsWarmupCharacter: true).warmupSplit + XCTAssertFalse( + Self.synthesizedTextPlan(characterCount: 1, delaySeconds: 0, selectsExistingText: false, peelsWarmupCharacter: true) + .steps.contains(where: \.warmsUpField) ) XCTAssertEqual( Self.synthesizedTextPlan(characterCount: length, delaySeconds: 0.2, selectsExistingText: false, peelsWarmupCharacter: true), @@ -424,6 +423,66 @@ extension RunnerTests { ) } + // The executor is where the charged plan and the posted text meet, so it owns the slicing contract: + // every step hands over the next slice in order, a warmup step gets the read-back with exactly its + // own characters instead of the charged pause, and a post that stops the plan reports only what + // arrived. A wrong slice here types a character twice or never, which is the same failure the + // charged-seconds tests above can only see as a number. + @MainActor + func testSynthesizedPlanExecutorSlicesTheTextItsChargedPlanPosts() { + let text = "abcdefgh" + let cases: [(Int, Double, Bool)] = [ + (1, 0, false), + (8, 0, false), + (8, 0.001, false), + (8, 0, true), + (8, 0.001, true), + ] + for (length, delay, peels) in cases { + let plan = Self.synthesizedTextPlan( + characterCount: length, + delaySeconds: delay, + selectsExistingText: false, + peelsWarmupCharacter: peels + ) + var slices: [String] = [] + var reads: [String] = [] + let run = runSynthesizedTextPlan( + plan, + text: String(text.prefix(length)), + post: { slice, _ in + slices.append(slice) + return .posted + }, + waitAfterWarmupCharacter: { reads.append($0) } + ) + XCTAssertEqual(slices.joined(), String(text.prefix(length)), "plan \(length)/\(delay)/peel=\(peels)") + XCTAssertEqual(slices.count, plan.steps.count) + XCTAssertEqual(run.postedCharacterCount, length) + XCTAssertFalse(run.stoppedEarly) + XCTAssertEqual(reads, plan.steps.contains(where: { $0.warmsUpField }) ? ["a"] : []) + } + } + + @MainActor + func testSynthesizedPlanExecutorStopsAtThePostThatRefusesTheText() { + let plan = Self.synthesizedTextPlan(characterCount: 6, delaySeconds: 0.001, selectsExistingText: false) + var slices: [String] = [] + let run = runSynthesizedTextPlan( + plan, + text: "abcdef", + post: { slice, _ in + slices.append(slice) + return slice == "c" ? .stop : .posted + }, + waitAfterWarmupCharacter: { _ in } + ) + XCTAssertEqual(slices, ["a", "b", "c"]) + XCTAssertTrue(run.stoppedEarly) + // The refused post never delivered its character, so the caller learns two arrived. + XCTAssertEqual(run.postedCharacterCount, 2) + } + func testSynthesizedBudgetExceededCarriesItsOwnCodeAndRecovery() { XCTAssertEqual( TextEntryFailure.synthesisBudgetExceeded.rawValue,