diff --git a/packages/platform-android/src/__tests__/text-input-fill.test.ts b/packages/platform-android/src/__tests__/text-input-fill.test.ts index 8558a505be..74bda6e04b 100644 --- a/packages/platform-android/src/__tests__/text-input-fill.test.ts +++ b/packages/platform-android/src/__tests__/text-input-fill.test.ts @@ -802,3 +802,38 @@ function maskBullets(value: string): string { .map(() => '•') .join(''); } +// The provider-native clear is a device-side key sequence; if it silently did nothing, +// `fill ""` must still report failure, not exit 0 (#3358 review). +test('fillAndroid with an empty text fails, not unconfirmed, when a provider fill leaves the old value', async () => { + const value = 'old value'; + await withAndroidAdbProvider( + { + snapshotHelperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, + exec: createAndroidSnapshotHelperExecutor({ + exec: async (args) => { + throw new Error(`unexpected adb call: ${args.join(' ')}`); + }, + captureXml: () => androidInputXml({ text: value }), + }), + text: async () => { + // The instance keys cleared nothing: the field still holds its old value. + }, + }, + { serial: ANDROID_EMULATOR.id }, + async () => { + await assert.rejects( + () => fillAndroid(ANDROID_EMULATOR, 100, 50, ''), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + // The failure must be the verification reading the stale field back, not an earlier + // dispatch refusal: the mismatch names the empty expectation and the old value. + assert.equal(error.details?.failureReason, 'text_mismatch'); + assert.equal(error.details?.expected, ''); + assert.equal(error.details?.actual, value); + return true; + }, + ); + }, + ); +}); diff --git a/packages/platform-android/src/adb-executor.ts b/packages/platform-android/src/adb-executor.ts index cafb0dbd63..51e3606116 100644 --- a/packages/platform-android/src/adb-executor.ts +++ b/packages/platform-android/src/adb-executor.ts @@ -26,5 +26,6 @@ export type { AndroidAdbProvider, AndroidPortReverseEndpoint, AndroidTextInputAction, + AndroidTextInjectionRequest, AndroidTouchInjector, } from './adb-transport.ts'; diff --git a/packages/platform-android/src/adb-transport.ts b/packages/platform-android/src/adb-transport.ts index 30b65a8ae0..44e1faae17 100644 --- a/packages/platform-android/src/adb-transport.ts +++ b/packages/platform-android/src/adb-transport.ts @@ -140,8 +140,10 @@ export type AndroidTextInjectionRequest = { text: string; delayMs?: number; /** - * Present only for fill. Providers must make this target the focused/replaced - * input for the request, not inject into an unrelated currently focused field. + * Present only for fill. Providers must make this target the focused input AND replace its + * contents with `text`, not inject into an unrelated currently focused field and not insert at + * its cursor. An empty `text` is the clear request: the target must end up empty, and the + * provider may complete it without any text-set call of its own. */ target?: { x: number; diff --git a/packages/platform-android/src/mechanics.ts b/packages/platform-android/src/mechanics.ts index 26cce50693..f88e90f761 100644 --- a/packages/platform-android/src/mechanics.ts +++ b/packages/platform-android/src/mechanics.ts @@ -40,6 +40,7 @@ export { type AndroidAdbProvider, type AndroidPortReverseEndpoint, type AndroidTextInputAction, + type AndroidTextInjectionRequest, type AndroidTouchInjector, } from './adb-executor.ts'; export { diff --git a/packages/provider-limrun/src/android-text-entry.test.ts b/packages/provider-limrun/src/android-text-entry.test.ts new file mode 100644 index 0000000000..94e87027ab --- /dev/null +++ b/packages/provider-limrun/src/android-text-entry.test.ts @@ -0,0 +1,78 @@ +import { expect, test, vi } from 'vitest'; +import type { AndroidTextInjectionRequest } from '@agent-device/platform-android/mechanics'; +import { createLimrunAndroidTextInjector } from './android-text-entry.ts'; + +type ClientCall = 'tap' | 'pressKey:a+ctrl' | 'pressKey:del' | `setText:${string}`; + +function sessionWithClient() { + const calls: ClientCall[] = []; + const client = { + tap: vi.fn(async (target: { x: number; y: number }) => { + calls.push('tap'); + return target; + }), + pressKey: vi.fn(async (key: string, modifiers?: string[]) => { + calls.push( + `pressKey:${key}${modifiers?.length ? `+${modifiers.join('+')}` : ''}` as ClientCall, + ); + }), + setText: vi.fn(async (_target: { x: number; y: number } | undefined, text: string) => { + calls.push(`setText:${text}`); + return { textLength: text.length }; + }), + }; + return { injector: createLimrunAndroidTextInjector(client), client, calls }; +} + +function fillRequest( + overrides: Partial = {}, +): AndroidTextInjectionRequest { + return { action: 'fill', target: { x: 540, y: 220 }, text: 'jane@example.com', ...overrides }; +} + +// The instance's setText inserts at the focused field's cursor, so a fill that only calls +// setText concatenates over the old value (#3358). The injector must clear through the +// instance's own input channel before the text goes in. +test('a fill focuses the target, clears the selection, and only then sets the text', async () => { + const { injector, client, calls } = sessionWithClient(); + + await injector(fillRequest()); + + expect(calls).toEqual(['tap', 'pressKey:a+ctrl', 'pressKey:del', 'setText:jane@example.com']); + expect(client.tap).toHaveBeenCalledWith({ x: 540, y: 220 }); + // The text follows the selection the injector just established, so it goes to the field the + // injector tapped, not to whatever the instance would re-resolve from the coordinates. + expect(client.setText).toHaveBeenCalledWith(undefined, 'jane@example.com'); +}); + +// `fill ""` is the clear request (#2063), and the instance rejects an empty setText ("text is +// required"), so the clear stands on its own and no text is sent. +test('an empty fill clears the field without asking setText to set empty text', async () => { + const { injector, client, calls } = sessionWithClient(); + + await injector(fillRequest({ text: '' })); + + expect(calls).toEqual(['tap', 'pressKey:a+ctrl', 'pressKey:del']); + expect(client.setText).not.toHaveBeenCalled(); +}); + +// `type` appends by contract: neither the focus tap nor the clear may ride that action. +test('a type sends text to the target with no tap and no clear', async () => { + const { injector, client, calls } = sessionWithClient(); + + await injector({ action: 'type', text: 'appended' }); + + expect(calls).toEqual(['setText:appended']); + expect(client.setText).toHaveBeenCalledWith(undefined, 'appended'); + expect(client.tap).not.toHaveBeenCalled(); + expect(client.pressKey).not.toHaveBeenCalled(); +}); + +test('a fill without coordinates still clears the focused field before typing', async () => { + const { injector, client, calls } = sessionWithClient(); + + await injector(fillRequest({ target: undefined })); + + expect(calls).toEqual(['pressKey:a+ctrl', 'pressKey:del', 'setText:jane@example.com']); + expect(client.tap).not.toHaveBeenCalled(); +}); diff --git a/packages/provider-limrun/src/android-text-entry.ts b/packages/provider-limrun/src/android-text-entry.ts new file mode 100644 index 0000000000..95e0c2ed4c --- /dev/null +++ b/packages/provider-limrun/src/android-text-entry.ts @@ -0,0 +1,39 @@ +/** + * Text entry on a Limrun Android instance. The instance's `setText` inserts at the focused + * field's cursor and rejects empty text, so it cannot honor `fill`'s replace contract alone: + * the injector owns the replacement — focus, select-all, delete — and hands `setText` only the + * text that replaces the selection. Selecting through the instance's own key events keeps the + * clear on the same input channel as the typing that follows it; a second channel racing the + * selection (adb keyevents) could lose it. `type` keeps appending: no focus, no clear. + */ +import type { AndroidTextInjectionRequest } from '@agent-device/platform-android/mechanics'; + +type LimrunAndroidTextClient = { + tap: (target: { x: number; y: number }) => Promise; + pressKey: (key: string, modifiers?: string[]) => Promise; + setText: (target: { x: number; y: number } | undefined, text: string) => Promise; +}; + +export function createLimrunAndroidTextInjector( + client: LimrunAndroidTextClient, +): (request: AndroidTextInjectionRequest) => Promise { + return async (request) => { + if (request.action !== 'fill') { + await client.setText(request.target, request.text); + return; + } + if (request.target) { + await client.tap(request.target); + } + // Select what the focused field holds, then remove it: the field is now empty and the + // caret is the insertion point. `del` is the SDK's backspace name, the delete the issue + // reproduction verified against the stock Latin IME. + await client.pressKey('a', ['ctrl']); + await client.pressKey('del'); + // The empty fill is the clear request (#2063), which the clear above just completed; + // the SDK's setText refuses an empty text, so the clear request never sends one. + if (request.text.length > 0) { + await client.setText(undefined, request.text); + } + }; +} diff --git a/packages/provider-limrun/src/android.ts b/packages/provider-limrun/src/android.ts index 9cb2e9c025..a30c243b1b 100644 --- a/packages/provider-limrun/src/android.ts +++ b/packages/provider-limrun/src/android.ts @@ -80,7 +80,9 @@ export async function createLimrunAndroidSession( const adbProvider: LimrunAdbProvider = { exec: async (args, execOptions) => await runLimrunAndroidAdb(session, args, execOptions), text: async (request) => { - await client.setText(request.target, request.text); + // Loaded on the text-entry path to keep this provider's declared import-time closure budget. + const { createLimrunAndroidTextInjector } = await import('./android-text-entry.ts'); + await createLimrunAndroidTextInjector(client)(request); }, }; adbProvider.reverse = await dependencies.android.createPortReverse(adbProvider.exec, { diff --git a/packages/provider-limrun/src/runtime-dependencies.test.ts b/packages/provider-limrun/src/runtime-dependencies.test.ts index fce2b7d5c5..c6dc12612c 100644 --- a/packages/provider-limrun/src/runtime-dependencies.test.ts +++ b/packages/provider-limrun/src/runtime-dependencies.test.ts @@ -22,6 +22,7 @@ import type { const state = vi.hoisted(() => ({ constructorOptions: [] as Array<{ defaultHeaders?: Record }>, androidCreateInputs: [] as unknown[], + clientCalls: [] as string[], assetList: vi.fn(async () => [ { id: 'asset-example', @@ -72,7 +73,17 @@ vi.mock('@limrun/api', () => ({ vi.mock('@limrun/api/instance-client', () => ({ createInstanceClient: vi.fn(async () => ({ disconnect: state.disconnect, - setText: vi.fn(async () => undefined), + tap: vi.fn(async () => { + state.clientCalls.push('tap'); + }), + pressKey: vi.fn(async (key: string, modifiers?: string[]) => { + state.clientCalls.push( + `pressKey:${key}${modifiers?.length ? `+${modifiers.join('+')}` : ''}`, + ); + }), + setText: vi.fn(async (_target: unknown, text: string) => { + state.clientCalls.push(`setText:${text}`); + }), startAdbTunnel: vi.fn(async () => ({ address: { address: '127.0.0.1', port: 62_001 }, close: state.tunnelClose, @@ -128,6 +139,38 @@ test('factory uses the injected Android and host adapters as its construction se assert.equal(state.tunnelClose.mock.calls.length, 1); }); +// The production text seam: `adbProvider.text` is what fillAndroid's provider-native branch +// resolves through. A regression to the raw setText pass-through would leave the injector's own +// tests green, so this pins the routing itself against the mocked instance client (#3358). +test('the session adb text seam replaces the field on fill and appends on type', async () => { + state.clientCalls.length = 0; + const fixture = createContractFixture(); + const runtime = createLimrunRuntime({ apiKey: 'lim_test_key' }, fixture.dependencies); + + try { + const device = await allocateAndroidDevice(runtime); + const deviceSession = runtime.getDeviceSession(device); + assert.equal(deviceSession?.platform, 'android'); + if (deviceSession?.platform !== 'android') throw new Error('Expected Android device session'); + const text = deviceSession.adb.text; + if (!text) throw new Error('Expected the Limrun provider to declare its text injector'); + + await text({ action: 'fill', target: { x: 540, y: 220 }, text: 'jane@example.com' }); + assert.deepEqual(state.clientCalls, [ + 'tap', + 'pressKey:a+ctrl', + 'pressKey:del', + 'setText:jane@example.com', + ]); + + state.clientCalls.length = 0; + await text({ action: 'type', text: 'appended' }); + assert.deepEqual(state.clientCalls, ['setText:appended']); + } finally { + await runtime.shutdown(); + } +}); + test('allocation installs an exact uploaded asset before binding its application id', async () => { state.androidCreateInputs.length = 0; const fixture = createContractFixture(); diff --git a/packages/provider-limrun/src/runtime-dependencies.ts b/packages/provider-limrun/src/runtime-dependencies.ts index 5ea353e70b..41772952fc 100644 --- a/packages/provider-limrun/src/runtime-dependencies.ts +++ b/packages/provider-limrun/src/runtime-dependencies.ts @@ -1,4 +1,7 @@ -import type { AndroidAdbInvocation } from '@agent-device/platform-android/mechanics'; +import type { + AndroidAdbInvocation, + AndroidTextInjectionRequest, +} from '@agent-device/platform-android/mechanics'; import type { AppsFilter } from '@agent-device/contracts/device'; import type { Interactor } from '@agent-device/contracts/interactor-types'; import type { AndroidInputOwner } from '@agent-device/contracts/android-input-ownership'; @@ -49,12 +52,7 @@ export type LimrunPortReverse = { export type LimrunAdbProvider = { exec: LimrunAdbExecutor; reverse?: LimrunPortReverse; - text?: (request: { - action: 'type' | 'fill'; - text: string; - delayMs?: number; - target?: { x: number; y: number }; - }) => Promise; + text?: (request: AndroidTextInjectionRequest) => Promise; }; export type LimrunAndroidKeyboardState = {