diff --git a/CHANGELOG.md b/CHANGELOG.md index 16929f5e0..5a8d7f3b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ - `agent-device help workflow` is now a compact ~8KB card instead of a ~41KB dump; the same depth still exists, split into `help scripting` (save-script, secret-safe fills, batch JSON, replay divergence/repair, recording) and `help gestures` (multi-touch shapes and platform quirks), plus a few paragraphs folded into the topics that already owned the subject (`help debugging`, `help physical-device`, `help validate`). Every `help ` first line is now `agent-device — ` so an agent can read the installed version from its mandatory first help read instead of a separate `agent-device --version` call. - Cloud iOS (BrowserStack, AWS Device Farm): `snapshot` and `diff` no longer fail with `SESSION_NOT_FOUND` on a live provider session (#1658). The app-session guard they ran belongs to the local XCUITest runner, which must attach to a target app; a cloud capture reads the provider's own driver session and needs no app identity, so the guard now applies to local Apple targets only. Relatedly, a cloud iOS `open com.example.app` now records that bundle id on the session — the provider path skips local app resolution (no simctl/devicectl reaches a hosted device), and used to drop an explicitly spelled bundle id along with it, leaving the session with no app identity at all. Opening a second bundle id replaces the first, matching the local path, where an explicitly spelled target always wins over the session's current app; deep links, display names, and bare `open` still keep the app already tracked. -- Cloud `fill` (BrowserStack, AWS Device Farm) waits for the tapped field to take text-entry focus before sending its keys, instead of dispatching both in back-to-back requests (#1658). A WebView input — an OAuth/SSO page in a Safari view controller, for example — takes first responder asynchronously, so the keys used to land with nothing focused while `fill` still answered "Filled N chars"; tapping and filling as two separate commands worked only because the round trip between them gave the field time to focus. The response now discloses `textEntryReadiness`: `keyboard-shown`, `settled-keyboard-up` (back-to-back fills into one form, where keyboard visibility cannot witness the new field taking focus, so the fill waits out the same readiness budget the Apple runner spends there), `settled-unknown` (the driver reports no keyboard state), or `not-observed` (the keys were sent, but no keyboard ever appeared). +- Cloud `fill` (BrowserStack, AWS Device Farm) waits for the tapped field to take text-entry focus before sending its keys, instead of dispatching both in back-to-back requests (#1658). A WebView input — an OAuth/SSO page in a Safari view controller, for example — takes first responder asynchronously, so the keys used to land with nothing focused while `fill` still answered "Filled N chars"; tapping and filling as two separate commands worked only because the round trip between them gave the field time to focus. The response now discloses `textEntryReadiness`: `keyboard-shown`, `settled-keyboard-up` (back-to-back fills into one form, where keyboard visibility cannot witness the new field taking focus, so the fill waits out the same readiness budget the Apple runner spends there), or `settled-unknown` (the driver implements no keyboard route at all). Breaking: when the tap raises no keyboard within that budget, cloud `fill` now FAILS with `COMMAND_FAILED` / `text_entry_focus_not_observed` and sends no keys, instead of typing into whatever holds first responder and answering "Filled N chars" — a fill with no witness must not read as a filled field. A driver that cannot report keyboard state at all is unaffected (it blind-settles as before); only a positively classified unimplemented route counts as such, so a dead session, an auth rejection, or a grid outage now surfaces instead of degrading into a blind text entry. - `scroll` and `back` now accept `--settle` (with `--settle-quiet` and `--timeout`), collapsing scroll-then-observe and back-then-observe into one call (#1638). The response carries the same settled payload the touch commands return — verdict, changed-lines diff with fresh refs on added lines, the unchanged-interactive tail, and `refsGeneration` when the settled tree was stored — and is best-effort: it never fails the action. One difference is deliberate: `scroll`/`back` resolve no element, so the diff baseline is the session's stored pre-action tree ("the last tree you observed") rather than a freshly resolved pre-action capture. Both commands now also preserve the daemon on timeout, like the other settle-capable commands. - Security: repository `./agent-device.json` now accepts only project-safe automation defaults. It rejects daemon endpoint/auth/transport/server settings, tenant/run/lease selectors, provider/cloud and Metro connection fields, headers, executable reporter modules, local write destinations, and other operator-controlled values before local module loading or any daemon health/RPC request. Put remote endpoint and token together in protected CI environment variables, user config, an explicit `--config` file, or the existing `connect`/`--remote-config` workflow. Daemon auth tokens no longer travel in serialized command flags. - `viewport` is now rejected during capability admission on Apple targets instead of reaching the device and failing inside dispatch. No Apple backend can resize a screen — simulator and device geometry is fixed by the selected device type — so `viewport` on iOS/iPadOS/tvOS/macOS now fails with `UNSUPPORTED_OPERATION`, `viewport is not supported on this device`, and a hint pointing at `--platform web` and at picking a different simulator. `capabilities` no longer advertises `viewport` on Apple targets. Web viewport resizing (`agent-device viewport 1280 900 --platform web`) is unchanged, and Android was already denied. diff --git a/packages/contracts/src/interactor-types.ts b/packages/contracts/src/interactor-types.ts index 9100c9d23..a80d9e5d8 100644 --- a/packages/contracts/src/interactor-types.ts +++ b/packages/contracts/src/interactor-types.ts @@ -110,10 +110,13 @@ export type TypeTextBackendResult = { * - `settled-keyboard-up`: the keyboard was ALREADY up (back-to-back fills into * one form), so its visibility says nothing about the NEW field. Entry waited * the full readiness budget, the same answer the Apple runner gives here. - * - `settled-unknown`: the driver reports no keyboard state at all, so nothing - * could be waited for. Entry followed a short blind settle. - * - `not-observed`: the keyboard was down before the tap and never came up. The - * keys were still sent, but nothing corroborates that a field received them. + * - `settled-unknown`: the driver implements no keyboard route at all, so + * nothing could be waited for. Entry followed a short blind settle. + * + * Every value describes a fill that DID send its keys. The fourth outcome — a + * tap that raised no keyboard at all — is deliberately absent: reporting it + * here would recreate #1658's silent success, so the interactor throws + * (`text_entry_focus_not_observed`) without typing instead. * * Closed set: the cloud interactor is its only producer and the boundary that * narrows it (readFillBackendResult, src/core/dispatch-interactions.ts) drops a @@ -123,7 +126,6 @@ export const CLOUD_TEXT_ENTRY_READINESS = [ 'keyboard-shown', 'settled-keyboard-up', 'settled-unknown', - 'not-observed', ] as const; export type CloudTextEntryReadiness = (typeof CLOUD_TEXT_ENTRY_READINESS)[number]; diff --git a/packages/provider-webdriver/src/webdriver-client.test.ts b/packages/provider-webdriver/src/webdriver-client.test.ts new file mode 100644 index 000000000..144444c00 --- /dev/null +++ b/packages/provider-webdriver/src/webdriver-client.test.ts @@ -0,0 +1,98 @@ +import assert from 'node:assert/strict'; +import { afterEach, test } from 'vitest'; +import { AppError } from '@agent-device/kernel/errors'; +import { WebDriverClient } from './webdriver-client.ts'; + +const realFetch = globalThis.fetch; + +afterEach(() => { + globalThis.fetch = realFetch; +}); + +// #1658: `fill` blind-settles and types when the keyboard route is unsupported. +// Anything else reaching that branch would degrade a real provider failure into +// a blind text entry, so the split is pinned against the actual wire shapes. +test('is_keyboard_shown reports an unimplemented route as unsupported', async () => { + // Appium's own answer for a route the driver does not have. + const client = await connectedClient( + webDriverResponse(404, { value: { error: 'unknown command', message: 'Unknown command' } }), + ); + + assert.equal(await client.isKeyboardShown(), 'unsupported'); +}); + +test('is_keyboard_shown reports a non-boolean answer as unsupported', async () => { + const client = await connectedClient(webDriverResponse(200, { value: null })); + + assert.equal(await client.isKeyboardShown(), 'unsupported'); +}); + +// The status is identical to an unimplemented route's, so a status-only test +// would read a dead session as a missing feature and keep typing into it. +test('is_keyboard_shown propagates a dead session, which shares the 404', async () => { + const client = await connectedClient( + webDriverResponse(404, { + value: { + error: 'invalid session id', + message: 'A session is either terminated or not started', + }, + }), + ); + + await assert.rejects(client.isKeyboardShown(), (error: AppError) => { + assert.match(error.message, /terminated or not started/); + return true; + }); +}); + +test('is_keyboard_shown propagates a grid failure', async () => { + const client = await connectedClient( + webDriverResponse(503, { value: { message: 'provider backend unavailable' } }), + ); + + await assert.rejects(client.isKeyboardShown(), (error: AppError) => { + assert.match(error.message, /provider backend unavailable/); + return true; + }); +}); + +// The readiness poll advertises a 2s budget; without the override each probe +// would inherit the client's 30s default and one hung request could hold it +// there. A stub that answers only the abort proves the caller's bound is the +// one in force — were it ignored, this test would hang to its own timeout. +test('is_keyboard_shown honors a caller timeout shorter than the client default', async () => { + const client = await connectedClient( + async (_input, init) => + await new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => reject(init.signal?.reason as Error)); + }), + ); + + await assert.rejects(client.isKeyboardShown(50), (error: Error) => { + assert.match(`${error.name} ${error.message}`, /timeout|abort/i); + return true; + }); +}); + +/** A client with a live session id, so `isKeyboardShown` reaches the session route. */ +async function connectedClient(respond: typeof globalThis.fetch): Promise { + globalThis.fetch = webDriverResponse(200, { + value: { sessionId: 'wd-1', capabilities: {} }, + }); + const client = new WebDriverClient({ + clientVersion: '0.0.0-test', + endpoint: 'http://cloud-webdriver.test/wd/hub/', + requestPolicy: { timeoutMs: 30_000, retryAttempts: 0 }, + }); + await client.createSession({ platformName: 'iOS' }); + globalThis.fetch = respond; + return client; +} + +function webDriverResponse(status: number, body: unknown): typeof globalThis.fetch { + return async () => + new Response(JSON.stringify(body), { + status, + headers: { 'Content-Type': 'application/json' }, + }); +} diff --git a/packages/provider-webdriver/src/webdriver-client.ts b/packages/provider-webdriver/src/webdriver-client.ts index 60e22123b..78251961d 100644 --- a/packages/provider-webdriver/src/webdriver-client.ts +++ b/packages/provider-webdriver/src/webdriver-client.ts @@ -65,6 +65,12 @@ type WebDriverResponse = { type WebDriverRequestOverrides = { retryAttempts?: number; + /** + * Per-request transport bound, for callers whose own budget is far shorter + * than the client's default. Without it a caller waiting 2s on a poll can be + * held for the full default timeout by one hung request. + */ + timeoutMs?: number; }; export class WebDriverClient { @@ -143,22 +149,29 @@ export class WebDriverClient { } /** - * Whether the software keyboard is up. Polled between a text field tap and - * the keys it should receive, so it must stay a single cheap round trip: no - * retries, and a driver that does not implement the route reports through the - * thrown error rather than a guessed boolean (callers decide what an - * unanswerable keyboard state means for them). + * Whether the software keyboard is up, or `unsupported` when this driver has + * no such route. Polled between a text field tap and the keys it should + * receive, so it stays one cheap round trip — no retries, and `timeoutMs` + * lets the caller bound it by its own budget rather than the client default. + * + * The unsupported/failed split matters: a caller that treats every error as + * "this driver cannot answer" would silently degrade a dead session, an auth + * rejection, or a grid outage into a blind text entry. Only a route the + * driver does not implement — or one that answers with a non-boolean — is + * reported as unsupported; everything else throws. */ - async isKeyboardShown(): Promise { - const value = await this.sessionRequest('GET', '/appium/device/is_keyboard_shown', undefined, { - retryAttempts: 0, - }); - if (typeof value !== 'boolean') { - throw new AppError('COMMAND_FAILED', 'WebDriver keyboard-shown response was not a boolean', { - valueType: typeof value, + async isKeyboardShown(timeoutMs?: number): Promise { + let value: unknown; + try { + value = await this.sessionRequest('GET', '/appium/device/is_keyboard_shown', undefined, { + retryAttempts: 0, + ...(timeoutMs === undefined ? {} : { timeoutMs }), }); + } catch (error) { + if (isUnimplementedWebDriverRoute(error)) return 'unsupported'; + throw error; } - return value; + return typeof value === 'boolean' ? value : 'unsupported'; } async back(): Promise { @@ -228,9 +241,10 @@ export class WebDriverClient { ): Promise { let lastError: unknown; const retryAttempts = overrides?.retryAttempts ?? this.requestPolicy.retryAttempts; + const timeoutMs = overrides?.timeoutMs ?? this.requestPolicy.timeoutMs; for (let attempt = 0; attempt <= retryAttempts; attempt += 1) { try { - return await this.requestValueOnce(method, path, body); + return await this.requestValueOnce(method, path, body, timeoutMs); } catch (error) { lastError = error; if (!isRetriableWebDriverError(error) || attempt >= retryAttempts) { @@ -242,7 +256,12 @@ export class WebDriverClient { throw lastError; } - private async requestValueOnce(method: string, path: string, body?: unknown): Promise { + private async requestValueOnce( + method: string, + path: string, + body: unknown, + timeoutMs: number, + ): Promise { const response = await fetch(new URL(trimLeadingSlash(path), this.endpoint), { method, headers: { @@ -251,7 +270,7 @@ export class WebDriverClient { ...this.headers, }, body: body === undefined ? undefined : JSON.stringify(body), - signal: AbortSignal.timeout(this.requestPolicy.timeoutMs), + signal: AbortSignal.timeout(timeoutMs), }); const text = await response.text(); const payload = text ? parseJsonResponse(text) : {}; @@ -348,6 +367,34 @@ function webdriverError(status: number, payload: unknown): AppError { return new AppError('COMMAND_FAILED', message, { status, response: payload }); } +/** + * A route this driver does not implement, as opposed to one that failed. + * + * Classified from the W3C error code, NOT from the HTTP status: `unknown + * command` and `invalid session id` are both 404, so a status test would read a + * dead session as a missing feature — exactly the confusion that turns a broken + * session into a blind text entry. Only 405/501 are unambiguous enough to stand + * on their own. A 5xx, an auth rejection, or a timeout is a real failure. + */ +const UNIMPLEMENTED_WEBDRIVER_STATUSES = new Set([405, 501]); +const UNIMPLEMENTED_WEBDRIVER_ERRORS = new Set(['unknown command', 'unknown method']); + +function isUnimplementedWebDriverRoute(error: unknown): boolean { + if (!(error instanceof AppError)) return false; + const status = error.details?.status; + if (typeof status === 'number' && UNIMPLEMENTED_WEBDRIVER_STATUSES.has(status)) return true; + return UNIMPLEMENTED_WEBDRIVER_ERRORS.has(readWebDriverErrorCode(error).toLowerCase()); +} + +function readWebDriverErrorCode(error: AppError): string { + const response = error.details?.response; + const value = + response && typeof response === 'object' ? (response as { value?: unknown }).value : undefined; + const code = + value && typeof value === 'object' ? (value as { error?: unknown }).error : undefined; + return typeof code === 'string' ? code : ''; +} + function isRetriableWebDriverError(error: unknown): boolean { if (error instanceof AppError) { const status = error.details?.status; diff --git a/packages/provider-webdriver/src/webdriver-interactor.test.ts b/packages/provider-webdriver/src/webdriver-interactor.test.ts index cf9a8ce5c..4afe7c39b 100644 --- a/packages/provider-webdriver/src/webdriver-interactor.test.ts +++ b/packages/provider-webdriver/src/webdriver-interactor.test.ts @@ -40,25 +40,58 @@ test('fill settles when the keyboard was already up before the tap', async () => assert.deepEqual(world.transcript, ['keyboard', 'tap', 'keys']); }); -test('fill settles when the driver cannot report keyboard state', async () => { +test('fill settles when the driver implements no keyboard route', async () => { const world = createTextEntryWorld(); - world.keyboardSupported = false; + world.keyboardRoute = 'unimplemented'; const result = await runFill(world); - // Evidence, never a precondition: an unanswerable keyboard must not fail the fill. + // A route the driver does not have is not a failure — it just leaves nothing to wait for. assert.equal(result?.textEntryReadiness, 'settled-unknown'); assert.deepEqual(world.transcript, ['keyboard', 'tap', 'keys']); }); -test('fill still types when no keyboard ever appears, and says so', async () => { +// The #1658 report is "fill answered Filled N chars while the field kept its +// placeholder". Reporting a readiness value nobody renders would preserve that, +// so a tap that raises no keyboard refuses instead — and sends no keys, leaving +// the field untouched rather than half-written. +test('fill refuses without typing when no keyboard ever appears', async () => { const world = createTextEntryWorld(); world.keyboardShown = [false]; - const result = await runFill(world); + await assert.rejects(runFill(world), (error: AppError) => { + assert.equal(error.code, 'COMMAND_FAILED'); + assert.match(error.message, /no keyboard appeared, so the text was not sent/); + assert.equal(error.details?.reason, 'text_entry_focus_not_observed'); + return true; + }); - assert.equal(result?.textEntryReadiness, 'not-observed'); - assert.equal(world.transcript.at(-1), 'keys'); + assert.equal(world.transcript.includes('keys'), false); +}); + +// A probe that FAILED is not a driver without the feature. Swallowing a dead +// session or a grid outage as "unsupported" would degrade it into a blind type. +test('fill propagates a failing keyboard probe instead of typing blind', async () => { + const world = createTextEntryWorld(); + world.keyboardRoute = 'failing'; + + await assert.rejects(runFill(world), (error: AppError) => { + assert.match(error.message, /invalid session id/); + return true; + }); + + // It threw on the pre-tap probe, so the device was never touched at all. + assert.deepEqual(world.transcript, ['keyboard']); +}); + +test('fill bounds each keyboard probe well under its readiness budget', async () => { + const world = createTextEntryWorld(); + world.keyboardShown = [false, true]; + + await runFill(world); + + // Left at the client default, one hung probe could hold a 2s wait for 30s. + assert.deepEqual(world.probeTimeouts, [1_500, 1_500]); }); /** @@ -75,9 +108,16 @@ async function runFill(world: ReturnType) { backend: 'xctest', capabilities: createCloudWebDriverCapabilities({ provider: 'test', platform: 'ios' }), }); - const pending = interactor.fill(12, 24, 'user@example.com'); + // Settled-shaped from the start: a rejection that lands while the clock is + // being advanced would otherwise be unhandled until the await below. + const pending = interactor.fill(12, 24, 'user@example.com').then( + (value) => ({ rejected: false, value }) as const, + (error: unknown) => ({ rejected: true, error }) as const, + ); await vi.advanceTimersByTimeAsync(5_000); - return await pending; + const settled = await pending; + if (settled.rejected) throw settled.error; + return settled.value; } finally { vi.useRealTimers(); } @@ -90,8 +130,10 @@ async function runFill(world: ReturnType) { function createTextEntryWorld() { const world = { transcript: [] as string[], + probeTimeouts: [] as Array, keyboardShown: [] as boolean[], - keyboardSupported: true, + /** How the driver answers the keyboard route: normally, not at all, or with a real failure. */ + keyboardRoute: 'ok' as 'ok' | 'unimplemented' | 'failing', client: undefined as unknown as WebDriverClient, }; world.client = { @@ -102,10 +144,15 @@ function createTextEntryWorld() { sendKeys: async () => { world.transcript.push('keys'); }, - isKeyboardShown: async () => { + isKeyboardShown: async (timeoutMs?: number) => { world.transcript.push('keyboard'); - if (!world.keyboardSupported) { - throw new AppError('COMMAND_FAILED', 'Unknown command: is_keyboard_shown'); + world.probeTimeouts.push(timeoutMs); + // The real client classifies the wire error; this stands in for its verdict. + if (world.keyboardRoute === 'unimplemented') return 'unsupported' as const; + if (world.keyboardRoute === 'failing') { + // A dead session: 404, same status as an unimplemented route, which is + // why the client classifies on the W3C error code instead. + throw new AppError('COMMAND_FAILED', 'invalid session id', { status: 404 }); } // The last programmed reading is what the keyboard stays at. return world.keyboardShown.length > 1 diff --git a/packages/provider-webdriver/src/webdriver-interactor.ts b/packages/provider-webdriver/src/webdriver-interactor.ts index 502a4fc56..cd9e60079 100644 --- a/packages/provider-webdriver/src/webdriver-interactor.ts +++ b/packages/provider-webdriver/src/webdriver-interactor.ts @@ -26,12 +26,18 @@ import { parseWebDriverSource } from './webdriver-source.ts'; import { setWebDriverOrientation } from './webdriver-orientation.ts'; /** - * How long a tapped field gets to take text entry focus before `fill` types - * anyway. Mirrors the Apple runner's `TextEntryTiming.readinessTimeout` + * How long a tapped field gets to take text entry focus before `fill` gives up. + * Mirrors the Apple runner's `TextEntryTiming.readinessTimeout` * (RunnerTests+TextEntry.swift), which owns this discipline locally. */ const TEXT_ENTRY_READINESS_TIMEOUT_MS = 2_000; const TEXT_ENTRY_READINESS_POLL_MS = 100; +/** + * One keyboard probe's own transport bound. Well under the readiness budget it + * serves, so a single hung probe cannot stretch that budget out toward the + * WebDriver client's much longer default request timeout. + */ +const TEXT_ENTRY_PROBE_TIMEOUT_MS = 1_500; /** The blind wait when the driver reports no keyboard state at all — see `awaitTextEntryReadiness`. */ const TEXT_ENTRY_BLIND_SETTLE_MS = 350; @@ -40,6 +46,26 @@ function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } +/** + * The tap raised no keyboard within the readiness budget, so nothing focused a + * text input and `sendKeys` would go to whatever else holds first responder — + * or nowhere. #1658 is precisely the report of that being answered with + * "Filled N chars", so this fails instead of typing: the caller gets a refusal + * it must handle, and the field is left untouched rather than half-written. + */ +function textEntryFocusNotObservedError(x: number, y: number): AppError { + return new AppError( + 'COMMAND_FAILED', + `fill tapped (${x}, ${y}) but no keyboard appeared, so the text was not sent`, + { + reason: 'text_entry_focus_not_observed', + x, + y, + hint: 'The tap most likely missed the field. Re-check the target with snapshot -i and fill the element it reports, rather than retrying the same coordinates.', + }, + ); +} + export type WebDriverInteractorOptions = { client: WebDriverClient; backend: Extract; @@ -165,10 +191,12 @@ class WebDriverInteractor implements Interactor { this.requireSupport('fill'); // #1658: read the keyboard BEFORE the tap. Only a hidden -> shown // transition proves that OUR tap moved focus; a keyboard that was already - // up says nothing about which field owns first responder now. - const keyboardBeforeTap = await this.tryReadKeyboardShown(); + // up says nothing about which field owns first responder now. A probe + // failure throws here, before the tap, so a broken session fails a fill + // that has not touched the device rather than typing into the dark. + const keyboardBeforeTap = await this.readKeyboardShown(); await this.tap(x, y); - const textEntryReadiness = await this.awaitTextEntryReadiness(keyboardBeforeTap); + const textEntryReadiness = await this.awaitTextEntryReadiness(keyboardBeforeTap, x, y); await this.type(text); return { backend: 'webdriver', x, y, text, textEntryReadiness }; } @@ -288,11 +316,15 @@ class WebDriverInteractor implements Interactor { * only the round trip between them gave the field time to focus. */ private async awaitTextEntryReadiness( - keyboardBeforeTap: boolean | undefined, + keyboardBeforeTap: boolean | 'unsupported', + x: number, + y: number, ): Promise { - // The driver answers nothing about the keyboard, so no wait can learn - // anything. Keep a blind path cheap: settle briefly, then type. - if (keyboardBeforeTap === undefined) { + // The driver implements no keyboard route, so no wait can learn anything. + // Keep a blind path cheap: settle briefly, then type. This is the ONE case + // that types without evidence, and it takes a positively classified + // unsupported answer to reach — never a probe that merely failed. + if (keyboardBeforeTap === 'unsupported') { await sleep(TEXT_ENTRY_BLIND_SETTLE_MS); return 'settled-unknown'; } @@ -310,19 +342,14 @@ class WebDriverInteractor implements Interactor { const deadline = Date.now() + TEXT_ENTRY_READINESS_TIMEOUT_MS; while (Date.now() < deadline) { await sleep(TEXT_ENTRY_READINESS_POLL_MS); - if ((await this.tryReadKeyboardShown()) === true) return 'keyboard-shown'; + if ((await this.readKeyboardShown()) === true) return 'keyboard-shown'; } - // The tap opened no keyboard. Still send the keys — a text view without a - // software keyboard (hardware keyboard attached, or a non-editable target - // the caller means to type at) is a real case, and refusing would turn a - // recoverable miss into a hard failure — but say so, so the caller is not - // told "filled" by a fill that had no witness. - return 'not-observed'; + throw textEntryFocusNotObservedError(x, y); } - /** Keyboard state is evidence, never a precondition: a driver that cannot answer must not fail the fill. */ - private async tryReadKeyboardShown(): Promise { - return await this.client.isKeyboardShown().catch(() => undefined); + /** Bounded by the readiness budget it serves, not the client's default request timeout. */ + private async readKeyboardShown(): Promise { + return await this.client.isKeyboardShown(TEXT_ENTRY_PROBE_TIMEOUT_MS); } private async pointerGesture(name: string, actions: W3CPointerAction[]): Promise { diff --git a/test/integration/provider-scenarios/cloud-webdriver-ios-text-entry.test.ts b/test/integration/provider-scenarios/cloud-webdriver-ios-text-entry.test.ts index 6fa62d0ac..648a33771 100644 --- a/test/integration/provider-scenarios/cloud-webdriver-ios-text-entry.test.ts +++ b/test/integration/provider-scenarios/cloud-webdriver-ios-text-entry.test.ts @@ -67,11 +67,15 @@ test('cloud iOS snapshot captures through the provider session after open', asyn * first responder asynchronously, so the keys arrived with nothing focused and * the command still answered "Filled N chars" — which is why tapping and * filling as two separate commands was the only reliable route. + * + * The device model, not just the request order, is what makes this a + * regression test: the fake drops keys that arrive while the keyboard is down, + * exactly as an unfocused field does, so the assertion below is the field's own + * value. */ -test('cloud iOS fill sends keys only after the field raises the keyboard', async () => { +test('cloud iOS fill lands text in the field it tapped', async () => { await withProviderScenarioResource(createCloudIosWorld, async ({ daemon, server }) => { const lease = await openCloudIosSession(daemon); - server.keyboardShownReadings = [false, true]; const response = await daemon.callCommand( 'fill', @@ -83,7 +87,48 @@ test('cloud iOS fill sends keys only after the field raises the keyboard', async const data = assertRpcOk<{ text?: string; textEntryReadiness?: string }>(response); assert.equal(data.text, 'user@example.com'); assert.equal(data.textEntryReadiness, 'keyboard-shown'); + // The keys reached the focused field, not the void. + assert.equal(server.fieldValue, 'user@example.com'); assert.deepEqual(textEntryTranscript(server), ['keyboard', 'tap', 'keyboard', 'keys']); + + // And the device agrees: the field reports the typed value back. + const snapshot = await daemon.callCommand( + 'snapshot', + [], + { ...leaseFlags(lease.leaseId), snapshotInteractiveOnly: true }, + { meta: leaseMeta(lease.leaseId) }, + ); + const nodes = assertRpcOk<{ nodes?: Array<{ identifier?: string; value?: string }> }>( + snapshot, + ).nodes; + assert.equal(nodes?.find((node) => node.identifier === 'email')?.value, 'user@example.com'); + }); +}, 15_000); + +/** + * #1658 again, from the other side: when the tap raises no keyboard at all, + * nothing focused a field and `sendKeys` would go nowhere. Answering that with + * "Filled N chars" is the original bug, so the fill refuses — and sends no keys, + * leaving the field untouched rather than half-written. + */ +test('cloud iOS fill refuses, without typing, when the tap raises no keyboard', async () => { + await withProviderScenarioResource(createCloudIosWorld, async ({ daemon, server }) => { + const lease = await openCloudIosSession(daemon); + server.keyboardPollsUntilShown = Number.POSITIVE_INFINITY; + + const response = await daemon.callCommand( + 'fill', + ['editable=true', 'user@example.com'], + leaseFlags(lease.leaseId), + { meta: leaseMeta(lease.leaseId) }, + ); + + const error = response.json.error; + assert.ok(error, 'expected fill to refuse'); + assert.match(error.message ?? '', /no keyboard appeared, so the text was not sent/); + assert.equal(error.data?.details?.reason, 'text_entry_focus_not_observed'); + assert.equal(server.fieldValue, ''); + assert.equal(textEntryTranscript(server).includes('keys'), false); }); }, 15_000); @@ -142,9 +187,20 @@ async function openCloudIosSession(daemon: ProviderScenarioHarness): Promise> { return await startCloudWebDriverTestServer(new FakeIosWebDriverServer()); @@ -161,28 +217,50 @@ class FakeIosWebDriverServer extends CloudWebDriverTestServer { cloudWebDriverTestJson({ value: { sessionId: 'wd-1', capabilities: { platformName: 'iOS' } }, }), - 'GET /wd/hub/session/wd-1/source': () => - cloudWebDriverTestJson({ value: fakeIosWebDriverSource() }), + 'GET /wd/hub/session/wd-1/source': () => cloudWebDriverTestJson({ value: this.source() }), 'GET /wd/hub/session/wd-1/window/rect': () => cloudWebDriverTestJson({ value: { x: 0, y: 0, width: 390, height: 844 } }), + 'POST /wd/hub/session/wd-1/actions': () => this.tap(), + 'POST /wd/hub/session/wd-1/keys': () => this.sendKeys(), 'GET /wd/hub/session/wd-1/appium/device/is_keyboard_shown': () => - cloudWebDriverTestJson({ value: this.nextKeyboardShown() }), + cloudWebDriverTestJson({ value: this.keyboardShown() }), }; } - private nextKeyboardShown(): boolean { - return this.keyboardShownReadings.length > 1 - ? this.keyboardShownReadings.shift()! - : this.keyboardShownReadings[0]!; + private tap(): CloudWebDriverTestResponse { + this.pollsRemaining = this.keyboardPollsUntilShown; + return cloudWebDriverTestJson({ value: null }); } -} -function fakeIosWebDriverSource(): string { - return ( - '' + - '' + - '' - ); + /** Focus is not instant: it lands only after the configured number of probes. */ + private keyboardShown(): boolean { + if (this.pollsRemaining <= 0) return true; + this.pollsRemaining -= 1; + return this.pollsRemaining <= 0; + } + + private sendKeys(): CloudWebDriverTestResponse { + // Unfocused keys are accepted by the grid and land nowhere — the bug's shape. + if (this.pollsRemaining <= 0) { + const text = this.lastSentKeys(); + if (text !== undefined) this.fieldValue = text; + } + return cloudWebDriverTestJson({ value: null }); + } + + private lastSentKeys(): string | undefined { + const body = this.calls.at(-1)?.body as { value?: unknown } | undefined; + return Array.isArray(body?.value) ? body.value.join('') : undefined; + } + + private source(): string { + return ( + '' + + `' + + '' + ); + } } function leaseFlags(leaseId?: string): DaemonRequest['flags'] { diff --git a/website/docs/docs/device-clouds.md b/website/docs/docs/device-clouds.md index 2e80f3870..f75eeed9c 100644 --- a/website/docs/docs/device-clouds.md +++ b/website/docs/docs/device-clouds.md @@ -355,4 +355,5 @@ BrowserStack can return session video, Appium logs, device logs, dashboard URL, - If AWS connect fails, use its reported `aws devicefarm get-*` error to check the credential chain, ARN, region, resource platform, or upload readiness. Allocation has not happened yet. - If Limrun connect fails, check `LIMRUN_API_KEY` and the optional `LIMRUN_REGION`. A successful connect verifies the selected instance service without creating an instance. - If artifact lookup is pending immediately after `close`, retry `agent-device artifacts --json`. Some providers finalize video/log URLs asynchronously after the hosted session stops. -- On hosted WebDriver sessions (BrowserStack, AWS Device Farm), `fill` waits for the tapped field to take text entry focus before sending its keys, and reports what it observed as `textEntryReadiness`: `keyboard-shown` (the keyboard came up in response to the tap), `settled-keyboard-up` (the keyboard was already up — back-to-back fills into one form — so it could not witness the new field taking focus and the fill waited out its full readiness budget), `settled-unknown` (the driver reports no keyboard state), or `not-observed` (no keyboard ever appeared — the keys were still sent, but nothing corroborates that a field received them). A `not-observed` fill on a field you expected to focus usually means the tap missed: re-check the target with `snapshot -i` rather than retrying the same coordinates. +- On hosted WebDriver sessions (BrowserStack, AWS Device Farm), `fill` waits for the tapped field to take text entry focus before sending its keys, and reports what it observed as `textEntryReadiness`: `keyboard-shown` (the keyboard came up in response to the tap), `settled-keyboard-up` (the keyboard was already up — back-to-back fills into one form — so it could not witness the new field taking focus and the fill waited out its full readiness budget), or `settled-unknown` (the driver implements no keyboard route, so there was nothing to wait for). +- If the tap raises no keyboard at all, `fill` **fails** with `text_entry_focus_not_observed` and sends no keys, rather than reporting a filled field that never received the text. That almost always means the tap missed: re-check the target with `snapshot -i` and fill the element it reports instead of retrying the same coordinates.