Skip to content

Commit bd62502

Browse files
authored
fix(android): bound the scroll-hint dumpsys probe and skip it in wait polling (#1270) (#1288)
* fix(android): bound the scroll-hint dumpsys probe and skip it in wait polling (#1270) deriveScrollableContentHintsIfNeeded's dumpsys activity top probe ran with an 8s timeout equal to a whole wait/get budget, and measured latency ranges from 37ms to ~5.9s under contention. Cap it at 1.5s so one pathological call can't starve a caller's timeout, and skip hint derivation entirely during find ... wait polling, since a presence check never consumes scroll hints. The wait polling loop actually lives in commands/interaction/runtime/selector-read.ts's waitForFindMatch (daemon/handlers/find.ts's own handleFindWait is unreachable for wait today — dispatchFindReadOnlyViaRuntime always intercepts read-only find actions first), so the skip-hints option threads through that capture path down to snapshotAndroid via the existing flags -> contextFromFlags -> dispatchCommand -> interactor.snapshot channel. * fix(android): skip scroll-hint derivation in standalone wait polling too (#1270) The issue's motivating repro — wait 'label="Battery"' 8000 — polls waitForSelector, not the find-wait loop, and text/ref waits poll waitForText (backend.findText -> captureWaitSnapshot on the daemon, or the snapshotContainsText fallback). All three presence-only polling captures now disable hidden-content-hint derivation, matching the Android alert-wait capture which already did. Stable wait keeps full snapshot semantics. Adds daemon-route regressions for the exact repro shape on both the selector-wait and text-wait routes, asserting every per-poll snapshot dispatch carries snapshotIncludeHiddenContentHints: false.
1 parent 1a1ef7c commit bd62502

13 files changed

Lines changed: 214 additions & 9 deletions

File tree

src/backend.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ export type BackendSnapshotResult = {
6363

6464
export type BackendSnapshotOptions = SnapshotOptions & {
6565
includeRects?: boolean;
66+
includeHiddenContentHints?: boolean;
6667
outPath?: string;
6768
};
6869

src/commands/interaction/runtime/selector-read-shared.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ export async function captureSelectorSnapshot(
5151
scope?: string;
5252
includeRects?: boolean;
5353
interactiveOnly?: boolean;
54+
includeHiddenContentHints?: boolean;
5455
} = {
5556
updateSession: true,
5657
},
@@ -67,6 +68,9 @@ export async function captureSelectorSnapshot(
6768
scope: captureOptions.scope ?? options.scope,
6869
raw: options.raw,
6970
includeRects: captureOptions.includeRects,
71+
...(captureOptions.includeHiddenContentHints !== undefined
72+
? { includeHiddenContentHints: captureOptions.includeHiddenContentHints }
73+
: {}),
7074
});
7175
const snapshot =
7276
result.snapshot ??

src/commands/interaction/runtime/selector-read.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,50 @@ test('runtime find wait reports sparse snapshot verdicts on the selector-read ro
406406
assert.equal(session.snapshot, initialSnapshot);
407407
});
408408

409+
test('runtime find wait skips hidden-content hint derivation on every poll (#1270)', async () => {
410+
// A `wait`'s presence check never consumes scroll hints, so every poll must ask the capture
411+
// layer to skip deriving them — otherwise a single pathological `dumpsys activity top` call
412+
// can eat the whole wait budget from inside this loop.
413+
const snapshot = selectorReadSnapshot();
414+
const captureOptionsCalls: Array<BackendSnapshotOptions | undefined> = [];
415+
let elapsed = 0;
416+
const device = createAgentDevice({
417+
backend: {
418+
platform: 'android',
419+
captureSnapshot: async (_context, options) => {
420+
captureOptionsCalls.push(options);
421+
return { snapshot };
422+
},
423+
} satisfies AgentDeviceBackend,
424+
artifacts: createLocalArtifactAdapter(),
425+
sessions: createMemorySessionStore([{ name: 'default', snapshot }]),
426+
policy: localCommandPolicy(),
427+
clock: {
428+
now: () => elapsed,
429+
sleep: async () => {
430+
elapsed += 300;
431+
},
432+
},
433+
});
434+
435+
await assert.rejects(
436+
() =>
437+
device.selectors.find({
438+
session: 'default',
439+
locator: 'text',
440+
query: 'Never appears',
441+
action: 'wait',
442+
timeoutMs: 500,
443+
}),
444+
/find wait timed out/,
445+
);
446+
447+
assert.equal(captureOptionsCalls.length >= 2, true);
448+
for (const options of captureOptionsCalls) {
449+
assert.equal(options?.includeHiddenContentHints, false);
450+
}
451+
});
452+
409453
test('runtime wait can use backend text search', async () => {
410454
const device = createSelectorDevice(selectorReadSnapshot(), {
411455
findText: true,

src/commands/interaction/runtime/selector-read.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,12 @@ async function waitForFindMatch(
448448
const timeout = options.timeoutMs ?? DEFAULT_TIMEOUT_MS;
449449
const start = now(runtime);
450450
while (now(runtime) - start < timeout) {
451-
const { match } = await findFirstLocatorMatch(runtime, options, locator);
451+
// A presence check never consumes scroll hints, so every poll skips deriving them —
452+
// otherwise a single pathological `dumpsys activity top` call can eat the whole wait
453+
// budget from inside this loop (#1270).
454+
const { match } = await findFirstLocatorMatch(runtime, options, locator, {
455+
includeHiddenContentHints: false,
456+
});
452457
if (match) return { kind: 'found', found: true, waitedMs: now(runtime) - start };
453458
await sleep(runtime, POLL_INTERVAL_MS);
454459
}
@@ -459,11 +464,13 @@ async function findFirstLocatorMatch(
459464
runtime: AgentDeviceRuntime,
460465
options: FindReadCommandOptions,
461466
locator: FindLocator,
467+
captureOverrides?: { includeHiddenContentHints?: boolean },
462468
): Promise<{ capture: CapturedSnapshot; match: SnapshotNode | undefined }> {
463469
const selectorChain = parseFindSelectorExpression(locator, options.query);
464470
const capture = await captureSelectorSnapshot(runtime, options, {
465471
updateSession: true,
466472
scope: findSnapshotScope(runtime, locator, options.query, selectorChain),
473+
includeHiddenContentHints: captureOverrides?.includeHiddenContentHints,
467474
...deriveSelectorCapturePolicy({ selectorChain }),
468475
});
469476
if (isSparseSnapshotQualityVerdict(capture.snapshot.snapshotQuality)) {
@@ -512,8 +519,10 @@ async function waitForSelector(
512519
const chain = parseSelectorChain(selectorExpression);
513520
const capturePolicy = deriveSelectorCapturePolicy({ selectorChain: chain });
514521
while (now(runtime) - start < timeout) {
522+
// Presence-only poll: skip scroll-hint derivation (#1270), same as waitForFindMatch.
515523
const capture = await captureSelectorSnapshot(runtime, options, {
516524
updateSession: true,
525+
includeHiddenContentHints: false,
517526
...capturePolicy,
518527
});
519528
const match = findSelectorChainMatch(capture.snapshot.nodes, chain, {
@@ -549,7 +558,11 @@ async function snapshotContainsText(
549558
options: WaitCommandOptions,
550559
text: string,
551560
): Promise<boolean> {
552-
const capture = await captureSelectorSnapshot(runtime, options, { updateSession: true });
561+
// Presence-only poll: skip scroll-hint derivation (#1270), same as waitForFindMatch.
562+
const capture = await captureSelectorSnapshot(runtime, options, {
563+
updateSession: true,
564+
includeHiddenContentHints: false,
565+
});
553566
return Boolean(findNodeByLabel(capture.snapshot.nodes, text));
554567
}
555568

src/core/dispatch-context.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ export type CommandFlags = Omit<CliFlags, DaemonExcludedCliFlag> & {
2626
kind?: string;
2727
maestro?: MaestroRuntimeFlags;
2828
postGestureStabilization?: boolean;
29+
snapshotIncludeHiddenContentHints?: boolean;
2930
leaseProvider?: string;
3031
provider?: string;
3132
deviceKey?: string;
@@ -61,6 +62,7 @@ export type DispatchContext = ScreenshotDispatchFlags & {
6162
snapshotScope?: string;
6263
snapshotRaw?: boolean;
6364
snapshotIncludeRects?: boolean;
65+
snapshotIncludeHiddenContentHints?: boolean;
6466
skipIosSimulatorBootCheck?: boolean;
6567
count?: number;
6668
intervalMs?: number;

src/core/dispatch.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,7 @@ async function handleSnapshotCommand(
636636
scope: context?.snapshotScope,
637637
raw: context?.snapshotRaw,
638638
includeRects: context?.snapshotIncludeRects,
639+
includeHiddenContentHints: context?.snapshotIncludeHiddenContentHints,
639640
surface: context?.surface,
640641
});
641642
}

src/core/interactor-types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ export const MAESTRO_NON_HITTABLE_FALLBACK_MESSAGE = 'tapped via non-hittable co
7272
export type SnapshotOptions = BaseSnapshotOptions & {
7373
appBundleId?: string;
7474
includeRects?: boolean;
75+
includeHiddenContentHints?: boolean;
7576
surface?: SessionSurface;
7677
};
7778

src/core/interactors/android.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ export function createAndroidInteractor(device: DeviceInfo): Interactor {
6666
depth: options?.depth,
6767
scope: options?.scope,
6868
raw: options?.raw,
69+
includeHiddenContentHints: options?.includeHiddenContentHints,
6970
// appBundleId is present for app-backed daemon sessions; keep the helper warm there,
7071
// but release it after standalone device snapshots so UiAutomation is not squatted.
7172
helperSessionScope: options?.appBundleId ? 'daemon-session' : 'command',

src/daemon/context.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ export function contextFromFlags(
4141
snapshotDepth: flags?.snapshotDepth,
4242
snapshotScope: flags?.snapshotScope,
4343
snapshotRaw: flags?.snapshotRaw,
44+
snapshotIncludeHiddenContentHints: flags?.snapshotIncludeHiddenContentHints,
4445
...screenshotFlagsFromOptions(flags),
4546
count: flags?.count,
4647
intervalMs: flags?.intervalMs,

src/daemon/handlers/__tests__/snapshot-handler.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1543,6 +1543,79 @@ test('wait selector timeout includes compact current-surface details', async ()
15431543
}
15441544
});
15451545

1546+
test('wait selector polling skips hidden-content hint derivation on every poll (#1270)', async () => {
1547+
// The #1270 repro: `wait 'label="Battery"' 8000` on Android. A presence-only wait never
1548+
// consumes scroll hints, so every per-poll snapshot capture must disable hint derivation —
1549+
// otherwise a pathological `dumpsys activity top` call is charged against the wait budget.
1550+
const sessionName = 'android-wait-selector-skips-hints';
1551+
const withoutBattery = {
1552+
nodes: locationRequiredNodes,
1553+
truncated: false,
1554+
backend: 'android',
1555+
analysis: { rawNodeCount: 2, maxDepth: 0 },
1556+
};
1557+
const withBattery = {
1558+
nodes: [
1559+
{
1560+
index: 0,
1561+
depth: 0,
1562+
type: 'android.widget.TextView',
1563+
label: 'Battery',
1564+
rect: { x: 252, y: 780, width: 153, height: 65 },
1565+
},
1566+
],
1567+
truncated: false,
1568+
backend: 'android',
1569+
analysis: { rawNodeCount: 1, maxDepth: 0 },
1570+
};
1571+
mockDispatch.mockResolvedValueOnce(withoutBattery).mockResolvedValueOnce(withBattery);
1572+
1573+
const response = await runWaitCommand(sessionName, androidDevice, ['label="Battery"', '8000']);
1574+
1575+
expect(response?.ok).toBe(true);
1576+
const snapshotCalls = mockDispatch.mock.calls.filter(([, command]) => command === 'snapshot');
1577+
expect(snapshotCalls.length).toBe(2);
1578+
for (const call of snapshotCalls) {
1579+
const context = call[4] as { snapshotIncludeHiddenContentHints?: boolean } | undefined;
1580+
expect(context?.snapshotIncludeHiddenContentHints).toBe(false);
1581+
}
1582+
});
1583+
1584+
test('wait text polling skips hidden-content hint derivation on every poll (#1270)', async () => {
1585+
const sessionName = 'android-wait-text-skips-hints';
1586+
const withoutBattery = {
1587+
nodes: locationRequiredNodes,
1588+
truncated: false,
1589+
backend: 'android',
1590+
analysis: { rawNodeCount: 2, maxDepth: 0 },
1591+
};
1592+
const withBattery = {
1593+
nodes: [
1594+
{
1595+
index: 0,
1596+
depth: 0,
1597+
type: 'android.widget.TextView',
1598+
label: 'Battery',
1599+
rect: { x: 252, y: 780, width: 153, height: 65 },
1600+
},
1601+
],
1602+
truncated: false,
1603+
backend: 'android',
1604+
analysis: { rawNodeCount: 1, maxDepth: 0 },
1605+
};
1606+
mockDispatch.mockResolvedValueOnce(withoutBattery).mockResolvedValueOnce(withBattery);
1607+
1608+
const response = await runWaitCommand(sessionName, androidDevice, ['Battery', '8000']);
1609+
1610+
expect(response?.ok).toBe(true);
1611+
const snapshotCalls = mockDispatch.mock.calls.filter(([, command]) => command === 'snapshot');
1612+
expect(snapshotCalls.length).toBe(2);
1613+
for (const call of snapshotCalls) {
1614+
const context = call[4] as { snapshotIncludeHiddenContentHints?: boolean } | undefined;
1615+
expect(context?.snapshotIncludeHiddenContentHints).toBe(false);
1616+
}
1617+
});
1618+
15461619
test('wait timeout summary prefers content labels over chrome and identifier noise', async () => {
15471620
const sessionName = 'ios-wait-timeout-surface-summary';
15481621
mockRunnerCommand.mockResolvedValue({ found: false });

0 commit comments

Comments
 (0)