Skip to content

Commit d268b79

Browse files
authored
fix: report blockers on wait timeout (#546)
1 parent d67e988 commit d268b79

3 files changed

Lines changed: 349 additions & 1 deletion

File tree

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

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,125 @@ beforeEach(() => {
8484
mockRunnerCommand.mockResolvedValue({});
8585
});
8686

87+
async function runWaitCommand(
88+
sessionName: string,
89+
device: SessionState['device'],
90+
positionals: string[],
91+
) {
92+
const sessionStore = makeSessionStore();
93+
sessionStore.set(sessionName, makeSession(sessionName, device));
94+
return await handleSnapshotCommands({
95+
req: {
96+
token: 't',
97+
session: sessionName,
98+
command: 'wait',
99+
positionals,
100+
flags: {},
101+
},
102+
sessionName,
103+
logPath: '/tmp/daemon.log',
104+
sessionStore,
105+
});
106+
}
107+
108+
const locationPermissionNodes = [
109+
{
110+
index: 0,
111+
depth: 0,
112+
type: 'android.widget.FrameLayout',
113+
label: 'Location permission',
114+
rect: { x: 0, y: 0, width: 390, height: 844 },
115+
},
116+
{
117+
index: 1,
118+
depth: 1,
119+
parentIndex: 0,
120+
type: 'android.widget.TextView',
121+
label: 'Allow location access?',
122+
rect: { x: 24, y: 210, width: 342, height: 40 },
123+
},
124+
{
125+
index: 2,
126+
depth: 1,
127+
parentIndex: 0,
128+
type: 'android.widget.Button',
129+
label: 'Not now',
130+
rect: { x: 24, y: 320, width: 140, height: 48 },
131+
hittable: true,
132+
},
133+
{
134+
index: 3,
135+
depth: 1,
136+
parentIndex: 0,
137+
type: 'android.widget.Button',
138+
label: 'Continue',
139+
rect: { x: 180, y: 320, width: 160, height: 48 },
140+
hittable: true,
141+
},
142+
];
143+
144+
const locationRequiredNodes = [
145+
{
146+
index: 0,
147+
depth: 0,
148+
type: 'android.widget.TextView',
149+
label: 'Location required',
150+
rect: { x: 24, y: 180, width: 342, height: 40 },
151+
},
152+
{
153+
index: 1,
154+
depth: 0,
155+
type: 'android.widget.Button',
156+
label: 'Dismiss',
157+
rect: { x: 24, y: 260, width: 342, height: 48 },
158+
},
159+
];
160+
161+
const iosSurfaceSummaryNodes = [
162+
{
163+
index: 0,
164+
depth: 0,
165+
type: 'XCUIElementTypeApplication',
166+
label: 'Expo Go',
167+
rect: { x: 0, y: 0, width: 393, height: 852 },
168+
},
169+
{
170+
index: 1,
171+
depth: 1,
172+
type: 'XCUIElementTypeImage',
173+
label: 'gearshape.fill',
174+
rect: { x: 12, y: 54, width: 24, height: 24 },
175+
},
176+
{
177+
index: 2,
178+
depth: 1,
179+
type: 'XCUIElementTypeOther',
180+
label: 'Tab Bar',
181+
rect: { x: 0, y: 760, width: 393, height: 92 },
182+
},
183+
{
184+
index: 3,
185+
depth: 1,
186+
type: 'XCUIElementTypeStaticText',
187+
label: 'Confirm catalog refresh',
188+
rect: { x: 48, y: 280, width: 297, height: 36 },
189+
},
190+
{
191+
index: 4,
192+
depth: 1,
193+
type: 'XCUIElementTypeButton',
194+
label: 'Keep browsing',
195+
rect: { x: 48, y: 360, width: 297, height: 48 },
196+
},
197+
{
198+
index: 5,
199+
depth: 1,
200+
type: 'XCUIElementTypeButton',
201+
identifier: 'host.exp.exponent:id/reload_button',
202+
rect: { x: 260, y: 54, width: 48, height: 48 },
203+
},
204+
];
205+
87206
test('snapshot rejects @ref scope without existing session snapshot', async () => {
88207
const sessionStore = makeSessionStore();
89208
const sessionName = 'ios-sim';
@@ -574,6 +693,98 @@ test('wait text on Android uses freshness-aware capture instead of one-shot snap
574693
);
575694
});
576695

696+
test('wait text timeout includes compact current-surface labels and buttons', async () => {
697+
const sessionName = 'android-wait-timeout-surface';
698+
mockDispatch.mockResolvedValue({
699+
nodes: locationPermissionNodes,
700+
truncated: false,
701+
backend: 'android',
702+
analysis: { rawNodeCount: 4, maxDepth: 1 },
703+
});
704+
705+
const response = await runWaitCommand(sessionName, androidDevice, ['Receipt uploaded', '0']);
706+
707+
expect(response?.ok).toBe(false);
708+
if (response && !response.ok) {
709+
expect(response.error.message).toBe(
710+
'wait timed out for text: Receipt uploaded. Current surface: Location permission, Allow location access?, Not now, Continue.',
711+
);
712+
expect(response.error.details?.currentSurface).toEqual({
713+
labels: ['Location permission', 'Allow location access?', 'Not now', 'Continue'],
714+
buttons: ['Not now', 'Continue'],
715+
});
716+
}
717+
});
718+
719+
test('wait selector timeout includes compact current-surface details', async () => {
720+
const sessionName = 'android-wait-selector-timeout-surface';
721+
mockDispatch.mockResolvedValue({
722+
nodes: locationRequiredNodes,
723+
truncated: false,
724+
backend: 'android',
725+
analysis: { rawNodeCount: 2, maxDepth: 0 },
726+
});
727+
728+
const response = await runWaitCommand(sessionName, androidDevice, ['id=receipt-uploaded', '0']);
729+
730+
expect(response?.ok).toBe(false);
731+
if (response && !response.ok) {
732+
expect(response.error.message).toBe(
733+
'wait timed out for selector: id=receipt-uploaded. Current surface: Location required, Dismiss.',
734+
);
735+
expect(response.error.details?.currentSurface).toEqual({
736+
labels: ['Location required', 'Dismiss'],
737+
buttons: ['Dismiss'],
738+
});
739+
}
740+
});
741+
742+
test('wait timeout summary prefers content labels over chrome and identifier noise', async () => {
743+
const sessionName = 'ios-wait-timeout-surface-summary';
744+
mockRunnerCommand.mockResolvedValue({ found: false });
745+
mockDispatch.mockResolvedValue({
746+
nodes: iosSurfaceSummaryNodes,
747+
truncated: false,
748+
backend: 'xctest',
749+
});
750+
751+
const response = await runWaitCommand(sessionName, iosSimulatorDevice, [
752+
'Impossible success text',
753+
'0',
754+
]);
755+
756+
expect(response?.ok).toBe(false);
757+
if (response && !response.ok) {
758+
expect(response.error.message).toBe(
759+
'wait timed out for text: Impossible success text. Current surface: Confirm catalog refresh, Keep browsing.',
760+
);
761+
expect(response.error.details?.currentSurface).toEqual({
762+
labels: [
763+
'Confirm catalog refresh',
764+
'Keep browsing',
765+
'host.exp.exponent:id/reload_button',
766+
'Expo Go',
767+
'gearshape.fill',
768+
'Tab Bar',
769+
],
770+
buttons: ['Keep browsing', 'host.exp.exponent:id/reload_button'],
771+
});
772+
}
773+
});
774+
775+
test('wait timeout preserves current behavior when current-surface inspection fails', async () => {
776+
const sessionName = 'android-wait-timeout-surface-fails';
777+
mockDispatch.mockRejectedValue(new Error('snapshot unavailable'));
778+
779+
const response = await runWaitCommand(sessionName, androidDevice, ['Receipt uploaded', '0']);
780+
781+
expect(response?.ok).toBe(false);
782+
if (response && !response.ok) {
783+
expect(response.error.message).toBe('wait timed out for text: Receipt uploaded');
784+
expect(response.error.details).toBeUndefined();
785+
}
786+
});
787+
577788
test('settings rejects unsupported iOS physical devices', async () => {
578789
const sessionStore = makeSessionStore();
579790
const sessionName = 'ios-device';

src/daemon/selector-runtime.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
} from './selector-recording.ts';
4444
import { createDaemonRuntimePolicy } from './runtime-policy.ts';
4545
import { createDaemonRuntimeSessionStore } from './runtime-session.ts';
46+
import { maybeWaitTimeoutSurfaceResponse } from './wait-current-surface.ts';
4647

4748
type SelectorRuntimeParams = {
4849
req: DaemonRequest;
@@ -208,7 +209,12 @@ export async function dispatchWaitViaRuntime(
208209
recordIfSession(sessionStore, sessionName, req, result);
209210
return toDaemonWaitData(result);
210211
});
211-
return await maybeAndroidForegroundBlockerResponse(params, response, 'wait');
212+
const enrichedResponse = await maybeWaitTimeoutSurfaceResponse(
213+
{ req, logPath: params.logPath, session, device },
214+
response,
215+
);
216+
// Keep generic wait-surface details first so Android blocker detection can own the top-level message.
217+
return await maybeAndroidForegroundBlockerResponse(params, enrichedResponse, 'wait');
212218
};
213219
if (parsed.kind === 'sleep') return await execute();
214220
return await withSessionlessRunnerCleanup(session, device, execute);

src/daemon/wait-current-surface.ts

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
import type { SnapshotNode } from '../utils/snapshot.ts';
2+
import type { DaemonRequest, DaemonResponse, SessionState } from './types.ts';
3+
import { captureSnapshot } from './handlers/snapshot-capture.ts';
4+
import { errorResponse } from './handlers/response.ts';
5+
import { normalizeType } from './snapshot-processing.ts';
6+
7+
type WaitCurrentSurfaceParams = {
8+
req: DaemonRequest;
9+
logPath?: string;
10+
session: SessionState | undefined;
11+
device: SessionState['device'];
12+
};
13+
14+
type CurrentSurfaceDetails = {
15+
labels: string[];
16+
buttons?: string[];
17+
};
18+
19+
const CHROME_ROLE_MARKERS = ['application', 'window', 'tabbar', 'scrollbar', 'image'] as const;
20+
const CHROME_LABELS = new Set(['tab bar']);
21+
22+
export async function maybeWaitTimeoutSurfaceResponse(
23+
params: WaitCurrentSurfaceParams,
24+
response: DaemonResponse,
25+
): Promise<DaemonResponse> {
26+
if (response.ok || !isWaitTimeoutMessage(response.error.message)) return response;
27+
const currentSurface = await inspectCurrentSurface(params).catch(() => null);
28+
if (!currentSurface) return response;
29+
return errorResponse(
30+
response.error.code,
31+
`${response.error.message}. Current surface: ${currentSurface.summary}.`,
32+
{
33+
...(response.error.details ?? {}),
34+
currentSurface: currentSurface.details,
35+
},
36+
);
37+
}
38+
39+
function isWaitTimeoutMessage(message: string): boolean {
40+
return /^wait timed out for (?:selector|text): /i.test(message);
41+
}
42+
43+
async function inspectCurrentSurface(
44+
params: WaitCurrentSurfaceParams,
45+
): Promise<{ summary: string; details: CurrentSurfaceDetails } | null> {
46+
const capture = await captureSnapshot({
47+
device: params.device,
48+
session: params.session,
49+
flags: {
50+
...params.req.flags,
51+
snapshotInteractiveOnly: true,
52+
snapshotCompact: true,
53+
},
54+
logPath: params.logPath ?? '',
55+
});
56+
const orderedNodes = [...capture.snapshot.nodes].sort(compareSurfacePriority);
57+
const labels = topSurfaceTexts(orderedNodes, 6, { includeIdentifiers: true });
58+
if (labels.length === 0) return null;
59+
const contentNodes = orderedNodes.filter((node) => !isChromeLikeNode(node));
60+
const summaryLabels = topSurfaceTexts(contentNodes, 4, { includeIdentifiers: false });
61+
const buttons = topSurfaceTexts(orderedNodes.filter(isButtonLikeNode), 4, {
62+
includeIdentifiers: true,
63+
});
64+
const summary = (summaryLabels.length > 0 ? summaryLabels : labels.slice(0, 4)).join(', ');
65+
return {
66+
summary,
67+
details: {
68+
labels,
69+
...(buttons.length > 0 ? { buttons } : {}),
70+
},
71+
};
72+
}
73+
74+
function topSurfaceTexts(
75+
nodes: SnapshotNode[],
76+
limit: number,
77+
options: { includeIdentifiers: boolean },
78+
): string[] {
79+
const seen = new Set<string>();
80+
const result: string[] = [];
81+
for (const node of nodes) {
82+
const text = extractSurfaceText(node, options);
83+
if (!text || seen.has(text)) continue;
84+
seen.add(text);
85+
result.push(text);
86+
if (result.length >= limit) break;
87+
}
88+
return result;
89+
}
90+
91+
function compareSurfacePriority(a: SnapshotNode, b: SnapshotNode): number {
92+
return surfacePriority(a) - surfacePriority(b) || compareSurfaceOrder(a, b);
93+
}
94+
95+
function surfacePriority(node: SnapshotNode): number {
96+
const hasHumanText = Boolean(extractSurfaceText(node, { includeIdentifiers: false }));
97+
const chromePenalty = isChromeLikeNode(node) ? 2 : 0;
98+
return chromePenalty + (hasHumanText ? 0 : 1);
99+
}
100+
101+
function compareSurfaceOrder(a: SnapshotNode, b: SnapshotNode): number {
102+
if (a.rect && b.rect) return a.rect.y - b.rect.y || a.rect.x - b.rect.x;
103+
if (a.rect) return -1;
104+
if (b.rect) return 1;
105+
return (a.depth ?? 0) - (b.depth ?? 0) || a.index - b.index;
106+
}
107+
108+
function extractSurfaceText(node: SnapshotNode, options: { includeIdentifiers: boolean }): string {
109+
const candidates = options.includeIdentifiers
110+
? [node.label, node.value, node.identifier]
111+
: [node.label, node.value];
112+
const value = candidates
113+
.map((candidate) => (typeof candidate === 'string' ? candidate.trim() : ''))
114+
.find((candidate) => candidate.length > 0);
115+
return value ? value.replace(/\s+/g, ' ').slice(0, 80) : '';
116+
}
117+
118+
function isChromeLikeNode(node: SnapshotNode): boolean {
119+
const roleText = normalizeType(`${node.type ?? ''} ${node.role ?? ''} ${node.subrole ?? ''}`);
120+
const label = `${node.label ?? ''} ${node.value ?? ''}`.trim().toLowerCase();
121+
return (
122+
CHROME_ROLE_MARKERS.some((marker) => roleText.includes(marker)) ||
123+
CHROME_LABELS.has(label) ||
124+
label.endsWith('.fill')
125+
);
126+
}
127+
128+
function isButtonLikeNode(node: SnapshotNode): boolean {
129+
const roleText = `${node.type ?? ''} ${node.role ?? ''} ${node.subrole ?? ''}`;
130+
return normalizeType(roleText).includes('button');
131+
}

0 commit comments

Comments
 (0)