Skip to content

Commit efda424

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/agent-device-2027-9d286e
* origin/main: feat(interaction): accept `fill <target> ""` as the clear-field primitive (#2066) fix(ci): spawn the differential's agent-device CLI as argv, not one option (#2069) fix(daemon): keep the recovery hint on Maestro replay errors (#2075) # Conflicts: # src/commands/command-input.ts
2 parents 8c0cbac + 4b8bcac commit efda424

42 files changed

Lines changed: 990 additions & 153 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/workflows/conformance-differential.yml‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,11 @@ concurrency:
3232
cancel-in-progress: true
3333

3434
env:
35-
AGENT_DEVICE_CLI: '--experimental-strip-types src/bin.ts'
35+
# Entry script and node flags are separate on purpose: one variable cannot
36+
# encode both without either corrupting a path that contains spaces or
37+
# spawning the whole line as a single node option.
38+
AGENT_DEVICE_CLI: 'src/bin.ts'
39+
AGENT_DEVICE_CLI_NODE_FLAGS: '--experimental-strip-types'
3640
DIFFERENTIAL_ONLY: ${{ github.event.inputs.only || '' }}
3741
# CI should not phone home, and it keeps `maestro --version` to just the
3842
# version instead of prefixing an analytics notice.

‎.github/workflows/ios.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ jobs:
144144
-only-testing:AgentDeviceRunnerUITests/RunnerTests/testPenalizedCoordinateTapOnNonTextControlDoesNotAuthorizeBareType \
145145
-only-testing:AgentDeviceRunnerUITests/RunnerTests/testBareDelayedTypeFailsWhenTappedInputDisappearsMidCommand \
146146
-only-testing:AgentDeviceRunnerUITests/RunnerTests/testSynthesizedTextCommitProgressWalksExpectedPrefixOnly \
147+
-only-testing:AgentDeviceRunnerUITests/RunnerTests/testEmptyReplacementWithoutResolvableTargetFailsClosed \
147148
-only-testing:AgentDeviceRunnerUITests/RunnerTests/testTextEntryTapWitnessIsBoundToTargetIdentity \
148149
-only-testing:AgentDeviceRunnerUITests/RunnerTests/testCoordinateTapTextInputProbeSkipsPenalizedXCTestChannel \
149150
-only-testing:AgentDeviceRunnerUITests/RunnerTests/testCoordinateTextInputCandidateMustBeEnabledAndContainTheTouchPoint \

‎android/snapshot-helper/src/main/java/com/callstack/agentdevice/snapshothelper/AccessibilityTreeXml.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,13 @@ static void appendNode(
3535
appendWindowMetadata(xml, windowMetadata);
3636
}
3737
appendNonEmptyAttribute(xml, "text", node.getText());
38+
// getText() returns the HINT for an empty field on modern Android, so `text` alone cannot
39+
// distinguish a cleared field from one whose value equals its hint; only this flag can
40+
// (#2063 empty-fill verification).
41+
appendTrueAttribute(
42+
xml,
43+
"hint-showing",
44+
Build.VERSION.SDK_INT >= Build.VERSION_CODES.O && node.isShowingHintText());
3845
appendNonEmptyAttribute(xml, "resource-id", node.getViewIdResourceName());
3946
appendAttribute(xml, "class", node.getClassName());
4047
appendNonEmptyAttribute(xml, "package", node.getPackageName());

‎apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+TextTyping.swift‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,36 @@ extension RunnerTests {
1515
) -> TextEntryResult {
1616
let totalStartedAt = Date()
1717
guard !text.isEmpty else {
18+
// Replacing a field WITH the empty string is the clear-field primitive (#2063): the typing
19+
// is vacuous but the clear is not. Returning here unconditionally left the old value in
20+
// place while the response reported "typed". Append (`type`) and unrepaired entry stay
21+
// no-ops, which is what an empty payload means for them.
22+
if repairMode == .replacement {
23+
guard let clearTarget = resolveTextEntryElement(app: app, target: target) else {
24+
// No resolvable input means no clear happened — including the synthesized
25+
// first-responder route, whose target carries no element. Success here would claim a
26+
// clear the runner never performed.
27+
logTextEntryPhase(commandId: commandId, phase: "total", startedAt: totalStartedAt, chars: 0, mode: repairMode)
28+
return TextEntryResult(
29+
verified: nil,
30+
repaired: false,
31+
expectedText: "",
32+
observedText: nil,
33+
failure: .notFocused
34+
)
35+
}
36+
clearTextInput(clearTarget)
37+
// nil means the value is unreadable (secure fields), not "not empty": leave `verified`
38+
// nil there rather than reporting a mismatch the runner cannot actually observe.
39+
let observed = editableTextValue(for: clearTarget, treatingPlaceholderAsEmpty: true)
40+
logTextEntryPhase(commandId: commandId, phase: "total", startedAt: totalStartedAt, chars: 0, mode: repairMode)
41+
return TextEntryResult(
42+
verified: observed.map { $0.isEmpty },
43+
repaired: false,
44+
expectedText: "",
45+
observedText: observed
46+
)
47+
}
1848
logTextEntryPhase(commandId: commandId, phase: "total", startedAt: totalStartedAt, chars: 0, mode: repairMode)
1949
return TextEntryResult(verified: true, repaired: false, expectedText: "", observedText: "")
2050
}

‎apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+TextEntryPolicyTests.swift‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,26 @@ extension RunnerTests {
445445
XCTAssertEqual(result.failure, .commitNotObserved)
446446
}
447447

448+
// `fill <target> ""` is the clear-field primitive (#2063). When no clear target resolves —
449+
// Springboard's home screen has no focused text input — the empty-text replacement path must
450+
// fail closed: it used to fall through to the vacuous-typing early return and report
451+
// `verified: true` for a clear that never ran.
452+
func testEmptyReplacementWithoutResolvableTargetFailsClosed() {
453+
let result = typeTextReliably(
454+
app: springboard,
455+
target: TextEntryTarget(element: nil, refreshPoint: nil, prefersFocusedElement: false),
456+
text: "",
457+
delaySeconds: 0,
458+
repairMode: .replacement,
459+
xCTestChannelPenalized: false,
460+
synthesizer: RecordingTextEntrySynthesizer()
461+
)
462+
463+
XCTAssertEqual(result.failure, .notFocused)
464+
XCTAssertNil(result.verified)
465+
XCTAssertNil(result.observedText)
466+
}
467+
448468
// Companion to the above: text carrying a submit key must skip the wait entirely, same as the
449469
// append route (`awaitSynthesizedFirstResponderCommit`) — the app may clear or rewrite the field
450470
// on submit, so there is nothing meaningful to poll toward.

‎packages/contracts/src/interactor-types.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,12 @@ export type Interactor = {
285285
hoverRef?(ref: string): Promise<Record<string, unknown> | void>;
286286
focus(x: number, y: number): Promise<Record<string, unknown> | void>;
287287
type(text: string, delayMs?: number): Promise<TypeTextBackendResult | void>;
288+
/**
289+
* Replace the target's text with `text`. The empty string is the clear request (#2063), not a
290+
* no-op: an implementation must empty the field or fail — never report success over an
291+
* untouched value. A backend with no clear mechanism refuses the empty text up front (see the
292+
* webdriver interactor).
293+
*/
288294
fill(
289295
x: number,
290296
y: number,

‎packages/maestro/src/internal/export-flow.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,20 @@ function convertFillAction(
266266
}
267267
const tapTarget = readTapTarget(target, undefined, resolveSelector);
268268
if (!tapTarget) return { kind: 'unsupported', message: 'fill target is not Maestro-compatible' };
269+
if (text.length === 0) {
270+
// The clear request (#2063): `inputText: ""` types nothing in Maestro, so the recorded
271+
// clear would silently become a no-op. `eraseText` is Maestro's clear verb; without a
272+
// count it erases up to its 50-character default, which is a bound this export cannot
273+
// recover the real length for.
274+
return {
275+
kind: 'commands',
276+
commands: [{ tapOn: tapTarget }, 'eraseText'],
277+
warnings: [
278+
...readLabelSelectorWarnings(target),
279+
'fill "" exports as tapOn + eraseText; Maestro erases at most its 50-character default',
280+
],
281+
};
282+
}
269283
return {
270284
kind: 'commands',
271285
commands: [{ tapOn: tapTarget }, { inputText: text }],

‎packages/maestro/test/conformance/differential/engine-process.test.ts‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@ import fs from 'node:fs';
33
import os from 'node:os';
44
import path from 'node:path';
55
import { test } from 'node:test';
6-
import { classifyAgentDeviceFailure, runAgentDeviceEngine } from './engine-process.ts';
6+
import {
7+
classifyAgentDeviceFailure,
8+
resolveAgentDeviceCliArgv,
9+
runAgentDeviceEngine,
10+
} from './engine-process.ts';
711

812
test('agent-device JSON distinguishes infrastructure from behavioral failures', () => {
913
const result = (infrastructure?: true) =>
@@ -29,7 +33,7 @@ test('agent-device execution accepts a CLI path containing spaces', () => {
2933
const cliPath = path.join(root, 'agent device.mjs');
3034
try {
3135
fs.writeFileSync(cliPath, '');
32-
assert.deepEqual(runAgentDeviceEngine(cliPath, []), {
36+
assert.deepEqual(runAgentDeviceEngine([cliPath], []), {
3337
engine: 'agent-device',
3438
outcome: 'pass',
3539
exitCode: 0,
@@ -38,3 +42,25 @@ test('agent-device execution accepts a CLI path containing spaces', () => {
3842
fs.rmSync(root, { recursive: true, force: true });
3943
}
4044
});
45+
46+
test('the CLI argv keeps node flags and the entry script apart', () => {
47+
// The default is the source CLI the device workflows name explicitly.
48+
assert.deepEqual(resolveAgentDeviceCliArgv(undefined, undefined), [
49+
'--experimental-strip-types',
50+
'src/bin.ts',
51+
]);
52+
// An entry path is never split, so spaces in it survive.
53+
assert.deepEqual(resolveAgentDeviceCliArgv('/tmp/agent device.mjs', ''), [
54+
'/tmp/agent device.mjs',
55+
]);
56+
assert.deepEqual(resolveAgentDeviceCliArgv('/tmp/agent device.mjs', undefined), [
57+
'--experimental-strip-types',
58+
'/tmp/agent device.mjs',
59+
]);
60+
// Flags are split, which is exact because a node flag cannot contain a space.
61+
assert.deepEqual(resolveAgentDeviceCliArgv('bin/x.mjs', '--no-warnings --enable-source-maps'), [
62+
'--no-warnings',
63+
'--enable-source-maps',
64+
'bin/x.mjs',
65+
]);
66+
});

‎packages/maestro/test/conformance/differential/engine-process.ts‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,41 @@ export function classifyAgentDeviceFailure(stdout: string): 'behavioral' | 'infr
3232
}
3333
}
3434

35-
export function runAgentDeviceEngine(cliPath: string, args: string[]): EngineResult {
36-
const result = spawnSync(process.execPath, [cliPath, ...args, '--json'], {
35+
/**
36+
* The agent-device CLI the differential drives, as two variables that cannot be
37+
* confused for one another:
38+
*
39+
* AGENT_DEVICE_CLI the entry script — ONE path, never split, so a
40+
* path containing spaces survives verbatim
41+
* AGENT_DEVICE_CLI_NODE_FLAGS node flags — split on whitespace, which is exact
42+
* because a node flag cannot contain a space
43+
*
44+
* One variable holding `--experimental-strip-types src/bin.ts` cannot express
45+
* both: splitting it corrupts `/tmp/agent device.mjs`, and not splitting it
46+
* spawns the whole line as a single node option — the bug that infrastructure-
47+
* failed every scenario from 2026-08-25. Set the flags variable to the empty
48+
* string to run an entry that needs none (a built `bin/agent-device.mjs`).
49+
*/
50+
const DEFAULT_CLI_ENTRY = 'src/bin.ts';
51+
const DEFAULT_CLI_NODE_FLAGS = '--experimental-strip-types';
52+
53+
export function resolveAgentDeviceCliArgv(
54+
entry: string | undefined,
55+
nodeFlags: string | undefined,
56+
): string[] {
57+
const flags = (nodeFlags ?? DEFAULT_CLI_NODE_FLAGS).split(/\s+/).filter(Boolean);
58+
return [...flags, entry?.trim() || DEFAULT_CLI_ENTRY];
59+
}
60+
61+
/**
62+
* `cliArgv` is a node argv — flags and entry script as separate elements, built
63+
* by `resolveAgentDeviceCliArgv`. Never a command line: spawning one as a single
64+
* argument aborts node before the CLI loads ("bad option: --experimental-strip-
65+
* types src/bin.ts"), which every scenario then reports as an infrastructure
66+
* failure. An entry path reaching this array is already its own element.
67+
*/
68+
export function runAgentDeviceEngine(cliArgv: readonly string[], args: string[]): EngineResult {
69+
const result = spawnSync(process.execPath, [...cliArgv, ...args, '--json'], {
3770
cwd: process.cwd(),
3871
encoding: 'utf8',
3972
});

‎packages/maestro/test/conformance/differential/run.test.ts‎

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ test('an ordinary Maestro process failure cannot satisfy a behavioral waiver', (
174174
{
175175
dryRun: false,
176176
maestroBin: `${process.execPath} ${maestroCli}`,
177-
agentDeviceCli,
177+
agentDeviceCliArgv: [agentDeviceCli],
178178
},
179179
);
180180

@@ -186,6 +186,99 @@ test('an ordinary Maestro process failure cannot satisfy a behavioral waiver', (
186186
}
187187
});
188188

189+
// The production route the P1 review asked for: environment -> parseRunnerArgs ->
190+
// runScenario -> spawn. Calling runAgentDeviceEngine with a hand-built argv proves
191+
// the spawn, but not that the two variables reach it intact — and it is the
192+
// environment contract that broke the nightly and that a whitespace-split
193+
// AGENT_DEVICE_CLI would break again in the other direction.
194+
describe('the agent-device CLI environment route', () => {
195+
const ROUTE_SCENARIO = {
196+
id: 'cli-env-route',
197+
flow: 'differential/flows/settle-after-tap.yaml',
198+
comparesAcrossEngines: 'test fixture',
199+
expect: 'pass',
200+
divergenceMeans: 'test fixture',
201+
} as const;
202+
203+
/** Run one scenario with AGENT_DEVICE_CLI* set, restoring the environment after. */
204+
function reportForEnvironment(entry: string, nodeFlags: string, maestroCli: string) {
205+
const previous = {
206+
entry: process.env.AGENT_DEVICE_CLI,
207+
flags: process.env.AGENT_DEVICE_CLI_NODE_FLAGS,
208+
};
209+
process.env.AGENT_DEVICE_CLI = entry;
210+
process.env.AGENT_DEVICE_CLI_NODE_FLAGS = nodeFlags;
211+
try {
212+
const options = parseRunnerArgs([]);
213+
return {
214+
argv: options.agentDeviceCliArgv,
215+
report: runScenario(ROUTE_SCENARIO, {
216+
...options,
217+
maestroBin: `${process.execPath} ${maestroCli}`,
218+
}),
219+
};
220+
} finally {
221+
restoreEnv('AGENT_DEVICE_CLI', previous.entry);
222+
restoreEnv('AGENT_DEVICE_CLI_NODE_FLAGS', previous.flags);
223+
}
224+
}
225+
226+
function restoreEnv(name: string, value: string | undefined): void {
227+
if (value === undefined) delete process.env[name];
228+
else process.env[name] = value;
229+
}
230+
231+
/**
232+
* `spaced` is a subdirectory whose NAME carries the space, so the entry path
233+
* holds one whatever the file is called. The maestro stub deliberately stays
234+
* out of it: `runMaestroEngine` still splits its command on spaces, and a
235+
* spaced stub path would fail this test for the other engine's reason.
236+
*/
237+
function withFixtureRoot(run: (spaced: string, maestroCli: string) => void): void {
238+
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-env-route-'));
239+
const spaced = path.join(root, 'agent device');
240+
fs.mkdirSync(spaced);
241+
const maestroCli = path.join(root, 'maestro.mjs');
242+
fs.writeFileSync(maestroCli, 'process.exit(0);\n');
243+
try {
244+
run(spaced, maestroCli);
245+
} finally {
246+
fs.rmSync(root, { recursive: true, force: true });
247+
}
248+
}
249+
250+
test('a CLI path containing spaces reaches the spawn unsplit', () => {
251+
withFixtureRoot((spaced, maestroCli) => {
252+
const entry = path.join(spaced, 'cli.mjs');
253+
fs.writeFileSync(entry, 'process.exit(0);\n');
254+
255+
const { argv, report } = reportForEnvironment(entry, '', maestroCli);
256+
257+
assert.deepEqual(argv, [entry], 'the entry path must stay one argument');
258+
assert.equal(report.agentDevice.outcome, 'pass');
259+
assert.equal(report.failed, false);
260+
});
261+
});
262+
263+
test('node flags stay separate arguments alongside a spaced path', () => {
264+
withFixtureRoot((spaced, maestroCli) => {
265+
const entry = path.join(spaced, 'cli.ts');
266+
fs.writeFileSync(entry, 'const code: number = 0;\nprocess.exit(code);\n');
267+
268+
const { argv, report } = reportForEnvironment(
269+
entry,
270+
'--experimental-strip-types',
271+
maestroCli,
272+
);
273+
274+
// Neither property is expressible in a single whitespace-joined variable.
275+
assert.deepEqual(argv, ['--experimental-strip-types', entry]);
276+
assert.equal(report.agentDevice.outcome, 'pass');
277+
assert.equal(report.failed, false);
278+
});
279+
});
280+
});
281+
189282
test('every device flow targets the fixture app the workflow installs', () => {
190283
for (const scenario of DIFFERENTIAL_SCENARIOS) {
191284
const flowPath = path.join(CONFORMANCE_DIR, scenario.flow);

0 commit comments

Comments
 (0)