diff --git a/packages/command-registry/src/__tests__/batch.test.ts b/packages/command-registry/src/__tests__/batch.test.ts index 881e7aa669..d62d8411b0 100644 --- a/packages/command-registry/src/__tests__/batch.test.ts +++ b/packages/command-registry/src/__tests__/batch.test.ts @@ -58,6 +58,31 @@ function batchRequest(commands: string[], responseLevel?: ResponseLevel): BatchR }; } +// #2997: `testIme` joined INHERITED_PARENT_FLAG_KEYS for the flow command's session +// opens; batch shares that list, so its steps inherit the parent's opt-in the same way. +// Pinning it here keeps the shared-list addition deliberate for both consumers. +test('batch steps inherit the parent testIme session-open opt-in', async () => { + const seen: Array | undefined> = []; + const request: BatchRequest = { + token: 't', + command: 'batch', + positionals: [], + flags: { + batchSteps: [{ command: 'open' }, { command: 'open', flags: { testIme: false } }], + testIme: true, + }, + }; + const response = await runBatch(request, 'session', async (req) => { + seen.push(req.flags as Record | undefined); + return { ok: true, data: {} }; + }); + + assert.equal(response.ok, true); + assert.equal(seen[0]?.testIme, true); + // A step's own flag wins; the merge only fills gaps. + assert.equal(seen[1]?.testIme, false); +}); + test('batch preserves typed error recovery signals from a failing step', async () => { const response = await runBatch(batchRequest(['open']), 'session', async () => ({ ok: false, diff --git a/packages/command-registry/src/batch-policy.ts b/packages/command-registry/src/batch-policy.ts index fc5b371f88..595af77f7f 100644 --- a/packages/command-registry/src/batch-policy.ts +++ b/packages/command-registry/src/batch-policy.ts @@ -47,6 +47,9 @@ export const INHERITED_PARENT_FLAG_KEYS = [ 'serial', 'verbose', 'out', + // A session-open opt-in the parent flow request carries down to its dispatched opens + // (replay steps and batch steps alike); a step's own recorded flag wins. + 'testIme', ] as const; /** diff --git a/packages/command-registry/src/flag-definitions-target.ts b/packages/command-registry/src/flag-definitions-target.ts index 7e91aec928..66ef356181 100644 --- a/packages/command-registry/src/flag-definitions-target.ts +++ b/packages/command-registry/src/flag-definitions-target.ts @@ -315,7 +315,7 @@ export const TARGET_FLAG_DEFINITIONS: readonly FlagDefinition[] = [ type: 'boolean', usageLabel: '--test-ime', usageDescription: - 'open: activate the headless Android test IME for deterministic Unicode text entry (default on for emulators; opt-in on real devices)', + 'open/test/replay: activate the headless Android test IME for deterministic Unicode text entry (default on for emulators; opt-in on real devices; on test/replay it applies to the sessions the flow opens)', projectConfig: true, recorded: false, }, @@ -326,7 +326,7 @@ export const TARGET_FLAG_DEFINITIONS: readonly FlagDefinition[] = [ setValue: false, usageLabel: '--no-test-ime', usageDescription: - 'open: keep the real Android keyboard even on emulators (opt out of the headless test IME)', + 'open/test/replay: keep the real Android keyboard even on emulators (opt out of the headless test IME; on test/replay it applies to the sessions the flow opens)', projectConfig: true, recorded: false, }, diff --git a/packages/contracts/package.json b/packages/contracts/package.json index bee7b0f6f9..c0f379b880 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -236,6 +236,10 @@ "types": "./src/interaction.ts", "default": "./src/interaction.ts" }, + "./input-validation": { + "types": "./src/input-validation.ts", + "default": "./src/input-validation.ts" + }, "./is-predicate": { "types": "./src/is-predicate.ts", "default": "./src/is-predicate.ts" diff --git a/packages/contracts/src/client-replay.ts b/packages/contracts/src/client-replay.ts index 6546bd601f..b3e1047dcd 100644 --- a/packages/contracts/src/client-replay.ts +++ b/packages/contracts/src/client-replay.ts @@ -41,6 +41,12 @@ export type ReplayRunOptions = AgentDeviceRequestOverrides & saveScript?: boolean | string; /** #1258: overwrite an existing --save-script target instead of refusing. Alias: --overwrite. */ force?: boolean; + /** + * Activate the headless Android test IME for the sessions this replay opens + * (default on for emulators; opt-in on real devices). `false` keeps the real + * keyboard even on emulators. + */ + testIme?: boolean; }; export type ReplayTestOptions = AgentDeviceRequestOverrides & @@ -61,6 +67,12 @@ export type ReplayTestOptions = AgentDeviceRequestOverrides & reportJunit?: string; shardAll?: number; shardSplit?: number; + /** + * Activate the headless Android test IME for the sessions each suite attempt + * opens (default on for emulators; opt-in on real devices). `false` keeps the + * real keyboard even on emulators. + */ + testIme?: boolean; }; export type BatchStep = { diff --git a/packages/contracts/src/facades/command.ts b/packages/contracts/src/facades/command.ts index 3c31f8b26d..f7b42f682c 100644 --- a/packages/contracts/src/facades/command.ts +++ b/packages/contracts/src/facades/command.ts @@ -13,7 +13,11 @@ export type { CommandExecutionOptions, InternalRequestOptions } from '../request export type { CommandFlags, MaestroRuntimeFlags } from '../command-flags.ts'; export type { DaemonWireRequest, DaemonWireRequestMeta } from '../daemon-wire-request.ts'; export type { DispatchedCommand } from '../dispatched-command.ts'; -export { readOptionalInteger, readOptionalNumber } from '../input-validation.ts'; +export { + ANDROID_SHELL_TEXT_UNSUPPORTED_REASON, + readOptionalInteger, + readOptionalNumber, +} from '../input-validation.ts'; export { IOS_SAFARI_BUNDLE_ID, isDeepLinkTarget, diff --git a/packages/contracts/src/input-validation.ts b/packages/contracts/src/input-validation.ts index 25eee8dd81..6a67c5c7c6 100644 --- a/packages/contracts/src/input-validation.ts +++ b/packages/contracts/src/input-validation.ts @@ -38,3 +38,11 @@ export function readOptionalNumber( } return value; } + +/** + * The typed reason the Android adb-shell text channel reports when it cannot carry the + * requested text. Recovery routing keys on this constant, never on the message: the message + * states the channel limit; the reason names which recovery surfaces apply. It lives here + * because every producer and consumer of the reason already evaluates this module. + */ +export const ANDROID_SHELL_TEXT_UNSUPPORTED_REASON = 'android_shell_text_unsupported' as const; diff --git a/packages/platform-android/src/__tests__/text-input.test.ts b/packages/platform-android/src/__tests__/text-input.test.ts index 353ad6294b..5784c1897c 100644 --- a/packages/platform-android/src/__tests__/text-input.test.ts +++ b/packages/platform-android/src/__tests__/text-input.test.ts @@ -1,6 +1,7 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; -import { fillAndroid, typeAndroid } from '../text-input.ts'; +import { ANDROID_SHELL_TEXT_UNSUPPORTED_REASON } from '@agent-device/contracts/input-validation'; +import { ANDROID_TEST_IME_OPEN_HINT, fillAndroid, typeAndroid } from '../text-input.ts'; import { assertRejectsAppError } from './test-utils/app-error.ts'; import { ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT, @@ -226,9 +227,13 @@ test('typeAndroid reports clear error when unicode input is unsupported', async return { stderr: `unexpected args: ${args.join(' ')}`, exitCode: 1 }; }, async ({ device }) => { + // #2997: consumers route recovery on the typed reason; the hint stays the + // direct-interaction `open --test-ime` route and the replay boundary rewrites it. await assertRejectsAppError(() => typeAndroid(device, '很'), { code: 'COMMAND_FAILED', message: /provider-native text injection/i, + reason: ANDROID_SHELL_TEXT_UNSUPPORTED_REASON, + hint: ANDROID_TEST_IME_OPEN_HINT, }); }, ); diff --git a/packages/platform-android/src/text-input.ts b/packages/platform-android/src/text-input.ts index 138bc05671..8cc1af8ad2 100644 --- a/packages/platform-android/src/text-input.ts +++ b/packages/platform-android/src/text-input.ts @@ -5,6 +5,7 @@ * `fill-verification.ts`. */ import type { FillUnconfirmedVerification } from '@agent-device/contracts/fill-evidence'; +import { ANDROID_SHELL_TEXT_UNSUPPORTED_REASON } from '@agent-device/contracts/command'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { AppError, discloseDispatchAfterSteps } from '@agent-device/kernel/errors'; import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; @@ -463,14 +464,27 @@ function isAndroidInputTextUnsupported(error: unknown): boolean { return false; } +/** + * The direct-interaction route's recovery (`open`, then a failing `fill`/`press`). The replay + * failure boundary replaces it for flow runs off the typed reason below, so a flow caller is + * never sent to a flag only `open` accepts. + */ +export const ANDROID_TEST_IME_OPEN_HINT = + 'On emulators the test IME activates automatically; on real devices pass `open --test-ime` to enable it (see `agent-device doctor` for the current IME state).'; + function unsupportedAndroidShellTextError(text: string, cause?: unknown): AppError { return new AppError( 'COMMAND_FAILED', - 'Android text input requires provider-native text injection or the bundled test IME helper for non-ASCII/control characters; the adb-shell fallback supports ASCII text only. On emulators the test IME activates automatically; on real devices pass `open --test-ime` to enable it (see `agent-device doctor` for the current IME state).', + 'Android text input requires provider-native text injection or the bundled test IME helper for non-ASCII/control characters; the adb-shell fallback supports ASCII text only.', { backend: 'adb-shell', + reason: ANDROID_SHELL_TEXT_UNSUPPORTED_REASON, textLength: Array.from(text).length, textPreview: text.slice(0, 32), + // The direct-interaction route's recovery. The replay failure boundary rewrites it + // off the typed reason (`ANDROID_TEST_IME_FLOW_HINT`), so a flow caller is never + // sent to a flag only `open` accepts. + hint: ANDROID_TEST_IME_OPEN_HINT, }, cause instanceof Error ? cause : undefined, ); diff --git a/packages/replay-port/src/daemon-port/__tests__/session-replay-runtime-failure-response.test.ts b/packages/replay-port/src/daemon-port/__tests__/session-replay-runtime-failure-response.test.ts index c6834d9bcb..ad9f943ca2 100644 --- a/packages/replay-port/src/daemon-port/__tests__/session-replay-runtime-failure-response.test.ts +++ b/packages/replay-port/src/daemon-port/__tests__/session-replay-runtime-failure-response.test.ts @@ -1,5 +1,10 @@ import { test, expect } from 'vitest'; -import { buildReplayDivergenceFailureResponseFromDescriptor } from '../session-replay-runtime-failure-response.ts'; +import { ANDROID_SHELL_TEXT_UNSUPPORTED_REASON } from '@agent-device/contracts/input-validation'; +import { + ANDROID_TEST_IME_FLOW_HINT, + buildReplayDivergenceFailureResponseFromDescriptor, + hoistReplayFailureCauseDiagnosticMeta, +} from '../session-replay-runtime-failure-response.ts'; test('native replay failure metadata keeps machine fields and daemon-owned paths intact', () => { const replayPath = '/tmp/flows/ios-login.ad'; @@ -70,3 +75,36 @@ test('a replay divergence carries the readiness evidence of an exhausted target expect(response.error.code).toBe('REPLAY_DIVERGENCE'); expect(response.error.details).toMatchObject({ reason: 'selector_not_found', readiness }); }); + +// #2997: the Android platform states `open --test-ime` because it sees one dispatched +// session-open. On this surface that advice is unactionable — a flow caller never runs +// `open` — so the cause's hint is replaced off the typed reason, never off the message. +// The fixture carries the production shape: the cause arrives WITH the open-route hint +// already hoisted, so a rewrite that only fills a missing hint would still fail here. +test('an Android shell-text cause gets the flow-owned --test-ime recovery', () => { + // The platform's own hint travels over the wire, so the fixture states it literally the way + // a consumer sees it: replay-port never imports the Android package. + const openRouteHint = + 'On emulators the test IME activates automatically; on real devices pass `open --test-ime` to enable it (see `agent-device doctor` for the current IME state).'; + const cause = hoistReplayFailureCauseDiagnosticMeta({ + code: 'COMMAND_FAILED', + message: + 'Android text input requires provider-native text injection or the bundled test IME helper for non-ASCII/control characters; the adb-shell fallback supports ASCII text only.', + hint: openRouteHint, + details: { reason: ANDROID_SHELL_TEXT_UNSUPPORTED_REASON, hint: openRouteHint }, + }); + + expect(cause.hint).toBe(ANDROID_TEST_IME_FLOW_HINT); + expect(cause.hint).toContain('--test-ime'); + expect(cause.hint).not.toContain('open --test-ime'); +}); + +test('a cause without the Android shell-text reason keeps its own hoisted hint', () => { + const cause = hoistReplayFailureCauseDiagnosticMeta({ + code: 'COMMAND_FAILED', + message: 'Selector did not match', + details: { reason: 'selector_not_found', hint: 'Inspect the latest snapshot.' }, + }); + + expect(cause.hint).toBe('Inspect the latest snapshot.'); +}); diff --git a/packages/replay-port/src/daemon-port/session-replay-maestro-runtime.ts b/packages/replay-port/src/daemon-port/session-replay-maestro-runtime.ts index 348537f0d4..a4a32614d7 100644 --- a/packages/replay-port/src/daemon-port/session-replay-maestro-runtime.ts +++ b/packages/replay-port/src/daemon-port/session-replay-maestro-runtime.ts @@ -350,6 +350,7 @@ function maestroRuntimeDeviceFlags( platform, target: device.target, noRecord: true, + ...(requestedFlags?.testIme === undefined ? {} : { testIme: requestedFlags.testIme }), }; if (platform === 'android') return { ...flags, serial: device.id }; return { @@ -367,6 +368,7 @@ function unresolvedMaestroRuntimeDeviceFlags( platform, target: requestedFlags?.target ?? 'mobile', noRecord: true, + ...(requestedFlags?.testIme === undefined ? {} : { testIme: requestedFlags.testIme }), }; if (requestedFlags?.device) flags.device = requestedFlags.device; return platform === 'android' diff --git a/packages/replay-port/src/daemon-port/session-replay-runtime-failure-response.ts b/packages/replay-port/src/daemon-port/session-replay-runtime-failure-response.ts index d62d42f93d..0b55f409ee 100644 --- a/packages/replay-port/src/daemon-port/session-replay-runtime-failure-response.ts +++ b/packages/replay-port/src/daemon-port/session-replay-runtime-failure-response.ts @@ -1,4 +1,5 @@ import type { SessionAction } from '@agent-device/contracts/session'; +import { ANDROID_SHELL_TEXT_UNSUPPORTED_REASON } from '@agent-device/contracts/input-validation'; import { scrubReplayVarValues, type ReplayVarScrubEntry } from '@agent-device/ad-replay/divergence'; import { formatDivergenceActionLabel } from '@agent-device/ad-script'; import type { SnapshotDiagnosticsSummary } from '@agent-device/contracts/capture'; @@ -7,15 +8,33 @@ import { type DaemonResponse } from '@agent-device/kernel/contracts'; export type ReplayFailureCause = Extract['error']; +/** + * Recovery hint for flow-owned session opens: `replay`/`test` accept `--test-ime` themselves + * and pass the opt-in to the sessions their flow opens. + */ +export const ANDROID_TEST_IME_FLOW_HINT = + 'On emulators the test IME activates automatically; on real devices pass `--test-ime` to this test/replay run to enable it for the sessions the flow opens (see `agent-device doctor` for the current IME state).'; + export function hoistReplayFailureCauseDiagnosticMeta( error: ReplayFailureCause, ): ReplayFailureCause { - return { + const cause: ReplayFailureCause = { ...error, hint: error.hint ?? readStringDetail(error.details, 'hint'), diagnosticId: error.diagnosticId ?? readStringDetail(error.details, 'diagnosticId'), logPath: error.logPath ?? readStringDetail(error.details, 'logPath'), }; + return rewriteAndroidTestImeFlowHint(cause); +} + +/** + * The Android platform states the `open --test-ime` recovery because it sees one + * dispatched session-open; a flow caller cannot run `open`, so on this surface the + * recovery is the `test`/`replay` flag itself. Keyed on the typed reason only. + */ +function rewriteAndroidTestImeFlowHint(error: ReplayFailureCause): ReplayFailureCause { + if (error.details?.reason !== ANDROID_SHELL_TEXT_UNSUPPORTED_REASON) return error; + return { ...error, hint: ANDROID_TEST_IME_FLOW_HINT }; } export function buildReplayDivergenceFailureResponse(params: { diff --git a/scripts/layering/contracts-exports.snapshot.json b/scripts/layering/contracts-exports.snapshot.json index d5f7f69d10..34180299ca 100644 --- a/scripts/layering/contracts-exports.snapshot.json +++ b/scripts/layering/contracts-exports.snapshot.json @@ -55,6 +55,7 @@ "@agent-device/contracts/gesture-plan-types", "@agent-device/contracts/gesture-runtime", "@agent-device/contracts/host-diagnostics", + "@agent-device/contracts/input-validation", "@agent-device/contracts/interaction", "@agent-device/contracts/interaction-guarantees", "@agent-device/contracts/interactor-operation-catalog", diff --git a/src/cli/parser/__tests__/args-parse-session.test.ts b/src/cli/parser/__tests__/args-parse-session.test.ts index 14c87a98f7..f1313bc8e4 100644 --- a/src/cli/parser/__tests__/args-parse-session.test.ts +++ b/src/cli/parser/__tests__/args-parse-session.test.ts @@ -18,24 +18,6 @@ test('parseArgs recognizes command-specific flag combinations', async () => { assert.equal(parsed.flags.relaunch, true); }, }, - { - label: 'open --test-ime forces the Android test IME on', - argv: ['open', 'settings', '--platform', 'android', '--test-ime'], - strictFlags: true, - assertParsed: (parsed) => { - assert.equal(parsed.command, 'open'); - assert.equal(parsed.flags.testIme, true); - }, - }, - { - label: 'open --no-test-ime forces the Android test IME off', - argv: ['open', 'settings', '--platform', 'android', '--no-test-ime'], - strictFlags: true, - assertParsed: (parsed) => { - assert.equal(parsed.command, 'open'); - assert.equal(parsed.flags.testIme, false); - }, - }, { label: 'open --platform ios --target tv', argv: ['open', 'Settings', '--platform', 'ios', '--target', 'tv'], diff --git a/src/cli/parser/__tests__/args-parse-test-ime.test.ts b/src/cli/parser/__tests__/args-parse-test-ime.test.ts new file mode 100644 index 0000000000..b980602cd4 --- /dev/null +++ b/src/cli/parser/__tests__/args-parse-test-ime.test.ts @@ -0,0 +1,54 @@ +/** + * `--test-ime` / `--no-test-ime` parsing across the three surfaces that accept it (#2997). + * Split out of `args-parse-session.test.ts`, which sits at the 1,000-line test-size + * tripwire (AGENTS.md "Module and test topology"). + */ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import { parseArgs } from '../args.ts'; + +const scenarios: Array<{ + label: string; + argv: string[]; + assertParsed: (parsed: ReturnType) => void; +}> = [ + { + label: 'open --test-ime forces the Android test IME on', + argv: ['open', 'settings', '--platform', 'android', '--test-ime'], + assertParsed: (parsed) => { + assert.equal(parsed.command, 'open'); + assert.equal(parsed.flags.testIme, true); + }, + }, + { + label: 'open --no-test-ime forces the Android test IME off', + argv: ['open', 'settings', '--platform', 'android', '--no-test-ime'], + assertParsed: (parsed) => { + assert.equal(parsed.command, 'open'); + assert.equal(parsed.flags.testIme, false); + }, + }, + { + label: 'test --test-ime opts the suite session opens into the Android test IME', + argv: ['test', './suite.ad', '--test-ime'], + assertParsed: (parsed) => { + assert.equal(parsed.command, 'test'); + assert.equal(parsed.flags.testIme, true); + }, + }, + { + label: 'replay --no-test-ime forces the real keyboard for the replay sessions', + argv: ['replay', './flow.ad', '--no-test-ime'], + assertParsed: (parsed) => { + assert.equal(parsed.command, 'replay'); + assert.equal(parsed.flags.testIme, false); + }, + }, +]; + +test('parseArgs admits --test-ime on every surface that accepts it', async () => { + for (const scenario of scenarios) { + const parsed = parseArgs(scenario.argv, { strictFlags: true }); + scenario.assertParsed(parsed); + } +}); diff --git a/src/commands/replay/index.test.ts b/src/commands/replay/index.test.ts index 550ad8d127..c5b9430a72 100644 --- a/src/commands/replay/index.test.ts +++ b/src/commands/replay/index.test.ts @@ -80,6 +80,30 @@ describe('replay command interface', () => { }); }); + test('reads the --test-ime opt-in on both flow surfaces', () => { + expect(replayCliReader(['./checkout.ad'], flags({ testIme: true }))).toMatchObject({ + testIme: true, + }); + expect(replayCliReader(['./checkout.ad'], flags({ testIme: false }))).toMatchObject({ + testIme: false, + }); + expect(testCliReader(['./suite.ad'], flags({ testIme: true }))).toMatchObject({ + testIme: true, + }); + // Absent stays absent: the session-open defaults (emulator on, device off) must survive. + expect(replayCliReader(['./checkout.ad'], flags()).testIme).toBeUndefined(); + expect(testCliReader(['./suite.ad'], flags()).testIme).toBeUndefined(); + }); + + test('admits testIme on the replay and test CLI schemas', () => { + // The schema's allowedFlags is the CLI projection allowlist; the metadata input + // schema is the Node/MCP structured surface. + expect(replayCommandFacet.cliSchema?.allowedFlags).toContain('testIme'); + expect(testCommandFacet.cliSchema?.allowedFlags).toContain('testIme'); + expect(replayCommandMetadata.inputSchema.properties).toHaveProperty('testIme'); + expect(testCommandMetadata.inputSchema.properties).toHaveProperty('testIme'); + }); + test('rejects missing replay path', () => { expectInvalidArgs(() => replayCliReader([], flags()), 'replay requires path'); }); diff --git a/src/commands/replay/index.ts b/src/commands/replay/index.ts index e57dfcad90..9afae42b3d 100644 --- a/src/commands/replay/index.ts +++ b/src/commands/replay/index.ts @@ -76,6 +76,9 @@ export const replayCommandMetadata = defineFieldCommandMetadata( // #1258: overwrite an existing --save-script target (arm-time preflight + // publish) instead of refusing. Alias: --overwrite. force: booleanField(), + testIme: booleanField( + 'Activate the headless Android test IME for the sessions this replay opens (default on for emulators; opt-in on real devices).', + ), }, ); @@ -98,6 +101,9 @@ export const testCommandMetadata = defineFieldCommandMetadata( artifactsDir: stringField(), shardAll: integerField(), shardSplit: integerField(), + testIme: booleanField( + 'Activate the headless Android test IME for the sessions each suite attempt opens (default on for emulators; opt-in on real devices).', + ), }, ); @@ -165,6 +171,7 @@ const replayCliSchema = { 'out', 'saveScript', 'force', + 'testIme', ], // ADR 0012 decision 6: on replay, --save-script arms a repair transaction from step 1 (not the // open/close authoring lifecycle the shared flag description documents) and the healed script @@ -194,6 +201,7 @@ const testCliSchema = { 'reportJunit', 'shardAll', 'shardSplit', + 'testIme', ], } as const satisfies CommandSchemaOverride; @@ -212,6 +220,7 @@ export const replayCliReader: CliReader = (positionals, flags) => ({ timeoutMs: flags.timeoutMs, saveScript: flags.saveScript, force: flags.force, + testIme: flags.testIme, }); export const testCliReader: CliReader = (positionals, flags) => ({ @@ -230,6 +239,7 @@ export const testCliReader: CliReader = (positionals, flags) => ({ artifactsDir: flags.artifactsDir, shardAll: flags.shardAll, shardSplit: flags.shardSplit, + testIme: flags.testIme, }); export const replayDaemonWriter: AsyncDaemonWriter = async (input) => { diff --git a/src/daemon/__tests__/replay-maestro/session-replay-runtime-maestro-dispatch.test.ts b/src/daemon/__tests__/replay-maestro/session-replay-runtime-maestro-dispatch.test.ts index 36fd939d73..b7ce22db68 100644 --- a/src/daemon/__tests__/replay-maestro/session-replay-runtime-maestro-dispatch.test.ts +++ b/src/daemon/__tests__/replay-maestro/session-replay-runtime-maestro-dispatch.test.ts @@ -300,3 +300,43 @@ test('every nested Maestro request keeps the replay envelope and the resolved de assert.equal(nested[0]?.command, 'open'); assert.equal(nested[0]?.flags?.relaunch, true); }); + +// #2997: the flow command's --test-ime/--no-test-ime must reach the session open the +// Maestro runtime dispatches, or a real-device flow cannot opt in to the test IME that +// eraseText/backspace needs. Omitting the flag keeps it absent on the dispatch so the +// session-open defaults (emulator on, device off) still decide. +test('a Maestro replay carries its --test-ime opt-in onto the open it dispatches', async () => { + for (const testIme of [true, false] as const) { + const nested: DaemonRequest[] = []; + const { response } = await runReplayFixture({ + label: `maestro-nested-test-ime-${testIme}`, + script: ['appId: demo.app', '---', '- launchApp', ''].join('\n'), + flags: { replayBackend: 'maestro', platform: 'android', testIme }, + invoke: async (req) => { + nested.push(req); + return { ok: true, data: {} }; + }, + }); + + assert.equal(response.ok, true); + const open = nested.find((req) => req.command === 'open'); + assert.ok(open); + assert.equal(open.flags?.testIme, testIme); + } + + const nested: DaemonRequest[] = []; + const { response } = await runReplayFixture({ + label: 'maestro-nested-test-ime-absent', + script: ['appId: demo.app', '---', '- launchApp', ''].join('\n'), + flags: { replayBackend: 'maestro', platform: 'android' }, + invoke: async (req) => { + nested.push(req); + return { ok: true, data: {} }; + }, + }); + + assert.equal(response.ok, true); + const open = nested.find((req) => req.command === 'open'); + assert.ok(open); + assert.equal(open.flags?.testIme, undefined); +}); diff --git a/src/daemon/__tests__/replay-runtime/session-replay-action-runtime.test.ts b/src/daemon/__tests__/replay-runtime/session-replay-action-runtime.test.ts index e50b53df9f..80a9aeb19a 100644 --- a/src/daemon/__tests__/replay-runtime/session-replay-action-runtime.test.ts +++ b/src/daemon/__tests__/replay-runtime/session-replay-action-runtime.test.ts @@ -139,6 +139,63 @@ test.each([ expect(await replayStepReadinessSchedule(REPLAY_REQUEST.flags, action)).toEqual(dispatchSchedule); }); +// #2997: the replay/test command's own --test-ime opt-in rides the parent flags onto the +// open this step dispatches; an authored step flag wins because mergeParentFlags only +// fills gaps. Without the inheritance the real-device flow open silently defaults off. +test('replay inherits the flow command testIme onto a dispatched open step', async () => { + const action: SessionAction = { + ts: 0, + command: 'open', + positionals: ['com.example.demo'], + flags: {}, + }; + let dispatchedFlags: Record | undefined; + await invokeReplayAction({ + req: { ...REPLAY_REQUEST, flags: { testIme: true } }, + sessionName: 'default', + action, + resolved: action, + filePath: 'flow.ad', + line: 1, + step: 1, + resolvedSessionScope: undefined, + dependencies: replayDaemonDependencies, + invoke: async (request) => { + dispatchedFlags = request.flags; + return { ok: true, data: {} }; + }, + }); + + expect(dispatchedFlags?.testIme).toBe(true); +}); + +test('replay keeps an authored open step testIme over the flow command opt-out', async () => { + const action: SessionAction = { + ts: 0, + command: 'open', + positionals: ['com.example.demo'], + flags: { testIme: true }, + }; + let dispatchedFlags: Record | undefined; + await invokeReplayAction({ + req: { ...REPLAY_REQUEST, flags: { testIme: false } }, + sessionName: 'default', + action, + resolved: action, + filePath: 'flow.ad', + line: 1, + step: 1, + resolvedSessionScope: undefined, + dependencies: replayDaemonDependencies, + invoke: async (request) => { + dispatchedFlags = request.flags; + return { ok: true, data: {} }; + }, + }); + + expect(dispatchedFlags?.testIme).toBe(true); +}); + test('replay keeps a readinessTimeoutMs the step already carries instead of overwriting it', async () => { const action: SessionAction = { ts: 0, diff --git a/src/daemon/__tests__/replay-suite/session-test-suite-command-nested-flags.test.ts b/src/daemon/__tests__/replay-suite/session-test-suite-command-nested-flags.test.ts index 9e5cc3fc2d..77c9773afb 100644 --- a/src/daemon/__tests__/replay-suite/session-test-suite-command-nested-flags.test.ts +++ b/src/daemon/__tests__/replay-suite/session-test-suite-command-nested-flags.test.ts @@ -47,6 +47,27 @@ test('buildNestedReplayFlags threads artifactsDir through even when parent lacks assert.deepEqual(result, { artifactsDir: '/tmp/attempt-1' }); }); +// #2997: the suite command's own --test-ime opt-in must fan out to every attempt's +// nested replay, or a real-device suite cannot opt in to the test IME its eraseText +// steps need. Parent flags ride through untouched, so pin that here. +test('buildNestedReplayFlags fans the parent testIme opt-in onto every attempt', () => { + const result = buildNestedReplayFlags({ + parentFlags: { platform: 'android', testIme: true }, + platform: undefined, + target: undefined, + artifactsDir: '/suite-root/flow/attempt-1', + }); + assert.equal(result?.testIme, true); + + const optedOut = buildNestedReplayFlags({ + parentFlags: { platform: 'android', testIme: false }, + platform: undefined, + target: undefined, + artifactsDir: '/suite-root/flow/attempt-1', + }); + assert.equal(optedOut?.testIme, false); +}); + test('buildNestedReplayFlags overrides a parent artifactsDir with the attempt-level one', () => { const result = buildNestedReplayFlags({ parentFlags: { artifactsDir: '/suite-root' }, diff --git a/src/daemon/__tests__/request-router-replay-test-ime.test.ts b/src/daemon/__tests__/request-router-replay-test-ime.test.ts new file mode 100644 index 0000000000..46d9b70a07 --- /dev/null +++ b/src/daemon/__tests__/request-router-replay-test-ime.test.ts @@ -0,0 +1,104 @@ +/** + * #2997: a `test`/`replay` run on a physical Android device opts into the bundled test + * IME with `--test-ime`, and that opt-in must arrive at the session open the flow itself + * owns. The flag rides the request envelope through the production router into the + * lifecycle binding, where the platform host performs the activation. Physical-device + * defaults stay off: the activation seam fires only for an explicit opt-in. + */ +import { createTestDeviceInventoryGateways } from '../../__tests__/test-utils/device-inventory-gateways.ts'; +import { beforeEach, expect, test, vi } from 'vitest'; +import fs from 'node:fs'; + +import path from 'node:path'; +import { getResolveTargetDeviceMock } from './request-router-dispatch-mocks.ts'; + +vi.mock('../device/device-ready.ts', () => ({ ensureDeviceReady: vi.fn(async () => {}) })); + +vi.mock('../../platform-runtime-runtime-hints.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, applyRuntimeHintValues: vi.fn(async () => {}) }; +}); + +// The IME seam itself talks adb; the router test asserts it is reached with the resolved +// device, so the mechanics module answers without a device attached. +const activateAndroidTestIme = vi.hoisted(() => vi.fn()); +const restoreAndroidTestIme = vi.hoisted(() => vi.fn()); +vi.mock('@agent-device/platform-android/mechanics', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + activateAndroidTestIme, + restoreAndroidTestIme, + resolveAndroidPackageForOpen: vi.fn(async () => undefined), + }; +}); + +import { ANDROID_DEVICE } from '../../__tests__/test-utils/device-fixtures.ts'; +import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts'; +import { replayScriptSourceBundleFor } from '../../__tests__/test-utils/replay-script-source.ts'; +import { LeaseRegistry } from '../lease-registry.ts'; +import { + createRequestHandler, + lifecycleDeviceRuntimeGateway, +} from './test-device-runtime-gateway.ts'; +import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; + +const mockActivateAndroidTestIme = vi.mocked(activateAndroidTestIme); +const mockRestoreAndroidTestIme = vi.mocked(restoreAndroidTestIme); +const mockResolveTargetDevice = vi.mocked(getResolveTargetDeviceMock()); + +beforeEach(() => { + mockActivateAndroidTestIme.mockReset(); + mockActivateAndroidTestIme.mockResolvedValue({ outcome: 'settled' }); + mockRestoreAndroidTestIme.mockReset(); + mockRestoreAndroidTestIme.mockResolvedValue({ restored: false, reason: 'no-record' }); + mockResolveTargetDevice.mockReset(); + // A physical device, which is the #1198 default-off route this opt-in exists for. + mockResolveTargetDevice.mockResolvedValue(ANDROID_DEVICE); +}); + +async function runAndroidFlowReplay(session: string, flags: Record) { + const root = mkdtempForTestSync('agent-device-replay-test-ime-'); + const replayPath = path.join(root, 'flow.ad'); + fs.writeFileSync(replayPath, 'open com.example.demo\n'); + const handler = createRequestHandler({ + logPath: path.join(mkdtempForTestSync('daemon'), 'daemon.log'), + token: 'test-token', + sessionStore: makeSessionStore('agent-device-replay-test-ime-'), + leaseRegistry: new LeaseRegistry(), + deviceRuntimeGateway: lifecycleDeviceRuntimeGateway, + deviceInventoryGateways: createTestDeviceInventoryGateways(), + trackDownloadableArtifact: () => 'artifact-id', + }); + const response = await handler({ + token: 'test-token', + session, + command: 'replay', + positionals: [replayPath], + flags: { + platform: 'android', + replayScriptSource: replayScriptSourceBundleFor(replayPath), + ...flags, + }, + meta: { cwd: root, requestId: `replay-test-ime-${session}` }, + }); + return { response }; +} + +test('a replay run opted into --test-ime activates the test IME on the device its flow opens', async () => { + const { response } = await runAndroidFlowReplay('replay-test-ime-on', { testIme: true }); + + expect(response).toMatchObject({ ok: true }); + expect(mockActivateAndroidTestIme).toHaveBeenCalledTimes(1); + expect(mockActivateAndroidTestIme.mock.calls[0]?.[0]).toMatchObject({ + id: ANDROID_DEVICE.id, + platform: 'android', + }); +}); + +test('a replay run without --test-ime leaves the physical device on the real keyboard', async () => { + const { response } = await runAndroidFlowReplay('replay-test-ime-default', {}); + + expect(response).toMatchObject({ ok: true }); + expect(mockActivateAndroidTestIme).not.toHaveBeenCalled(); +}); diff --git a/website/docs/docs/configuration.md b/website/docs/docs/configuration.md index 9e524a5da8..79631bb2c3 100644 --- a/website/docs/docs/configuration.md +++ b/website/docs/docs/configuration.md @@ -127,6 +127,7 @@ These env vars are the supported user-facing configuration surface. Other `AGENT | --- | --- | --- | | CLI defaults and config | `AGENT_DEVICE_HOME`, `AGENT_DEVICE_CONFIG`, `AGENT_DEVICE_SESSION`, `AGENT_DEVICE_PLATFORM`, `AGENT_DEVICE_SCREENSHOT_SCALE`, `AGENT_DEVICE_SESSION_LOCK`, `AGENT_DEVICE_DAEMON_BASE_URL`, `AGENT_DEVICE_DAEMON_AUTH_TOKEN`, `AGENT_DEVICE_CLOUD_BASE_URL` | Public | | Device scoping | `AGENT_DEVICE_ANDROID_DEVICE_ALLOWLIST` | Public | +| Android test IME | `AGENT_DEVICE_TEST_IME` | Public. Same setting as `--test-ime` / `--no-test-ime` on `open`, `test`, and `replay`; see known limitations. | | Local daemon storage | `AGENT_DEVICE_STATE_DIR` | Public | | Metro and install helpers | `AGENT_DEVICE_METRO_BEARER_TOKEN`, `AGENT_DEVICE_BUNDLETOOL_JAR` | Public | | App hooks and logs | `AGENT_DEVICE_APP_EVENT_URL_TEMPLATE`, `AGENT_DEVICE_IOS_APP_EVENT_URL_TEMPLATE`, `AGENT_DEVICE_MACOS_APP_EVENT_URL_TEMPLATE`, `AGENT_DEVICE_ANDROID_APP_EVENT_URL_TEMPLATE`, `AGENT_DEVICE_APP_LOG_MAX_BYTES`, `AGENT_DEVICE_APP_LOG_MAX_FILES`, `AGENT_DEVICE_APP_LOG_REDACT_PATTERNS`, `AGENT_DEVICE_EVENT_LOG_MAX_BYTES` | Public. Byte caps take whole integers (`5242880`), not `5MB`. | diff --git a/website/docs/docs/known-limitations.md b/website/docs/docs/known-limitations.md index 81ada8f29a..37915a7cf3 100644 --- a/website/docs/docs/known-limitations.md +++ b/website/docs/docs/known-limitations.md @@ -34,7 +34,7 @@ through `simctl pbcopy`. `adb shell input text` (the local ASCII-only fallback) cannot inject non-ASCII text (for example Chinese characters or emoji) on any Android system image. `agent-device` ships its own headless test IME (`android-ime-helper`) that handles this natively — it also removes the visible system keyboard from snapshots entirely, which the manual-ADBKeyBoard workaround this section used to describe never did. - **Emulators**: the test IME activates automatically on `open`; non-ASCII `fill`/`type` just work, no setup needed. -- **Real devices**: pass `--test-ime` to `open` to opt in (off by default on real hardware, since a stuck helper IME leaves the real keyboard unavailable until restored — `agent-device` restores the previous IME on session close and on daemon startup if a prior session crashed, and `agent-device doctor` flags a stuck test IME with the exact `adb shell ime set ` command to fix it manually if needed). +- **Real devices**: pass `--test-ime` to `open` to opt in (off by default on real hardware, since a stuck helper IME leaves the real keyboard unavailable until restored — `agent-device` restores the previous IME on session close and on daemon startup if a prior session crashed, and `agent-device doctor` flags a stuck test IME with the exact `adb shell ime set ` command to fix it manually if needed). `test` and `replay` accept the same setting on the flow command itself (`--test-ime` / `--no-test-ime`, or `testIme` in config), which applies it to the sessions each flow run opens — this is the route a Maestro `eraseText` or non-ASCII `fill` step needs when the flow owns the session. If the helper cannot be installed (locked-down managed devices, some cloud providers), text entry falls back to the existing ASCII-only `adb shell input text` path and non-ASCII `fill`/`type` reports the gap.