Skip to content

Commit 9a2e2f6

Browse files
committed
feat: warn when @refs outlive the session snapshot they came from
Refs are positional indexes into the latest stored session tree; any selector-based command's resolution capture silently reshuffles them (#1076). Track an honest marker on the daemon session (snapshotRefsStale): set wherever the stored snapshot is replaced by a response that does not hand the new refs to the client, cleared only where the client demonstrably receives them (snapshot responses, find ref outputs). Commands consuming @ref arguments while the marker is set (press/click/longpress/fill/get/wait) attach a warning and still execute — the geometric guards keep catching detectable drift. Closes #1076
1 parent 415f599 commit 9a2e2f6

12 files changed

Lines changed: 560 additions & 9 deletions

src/daemon/handlers/__tests__/find.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -730,3 +730,30 @@ test('handleFindCommands wait captures fresh snapshots while polling', async ()
730730
}
731731
expect(mockDispatch).toHaveBeenCalledTimes(2);
732732
});
733+
734+
test('handleFindCommands click re-issues a fresh ref and clears the stale-refs marker (#1076)', async () => {
735+
const sessionName = 'default';
736+
const session = makeSession(sessionName);
737+
// As set by an earlier selector-resolution capture that replaced the tree.
738+
session.snapshotRefsStale = true;
739+
740+
const { response, session: storedSession } = await runFindClickScenario({
741+
positionals: ['Increment', 'click'],
742+
nodes: [
743+
{
744+
index: 0,
745+
type: 'Button',
746+
label: 'Increment',
747+
hittable: true,
748+
rect: { x: 50, y: 0, width: 100, height: 100 },
749+
depth: 0,
750+
},
751+
],
752+
session,
753+
});
754+
755+
expect(response.ok).toBe(true);
756+
// The response returns a ref minted from the freshly stored snapshot, so
757+
// the marker clears before the internal click @ref sub-invocation runs.
758+
expect(storedSession.snapshotRefsStale).toBe(false);
759+
});

src/daemon/handlers/__tests__/interaction.test.ts

Lines changed: 211 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type { CommandFlags } from '../../../core/dispatch.ts';
66
import { attachRefs, type SnapshotBackend } from '../../../kernel/snapshot.ts';
77
import { AppError } from '../../../kernel/errors.ts';
88
import { buildSnapshotState } from '../snapshot-capture.ts';
9+
import { setSessionSnapshot, STALE_SNAPSHOT_REFS_WARNING } from '../../session-snapshot.ts';
910
import { makeSessionStore } from '../../../__tests__/test-utils/store-factory.ts';
1011
import {
1112
makeIosSession,
@@ -106,7 +107,9 @@ async function emulateCaptureSnapshotForSession(
106107
contextFromFlags(effectiveFlags, session.appBundleId, session.trace?.outPath),
107108
)) as { nodes?: never[]; truncated?: boolean; backend?: SnapshotBackend };
108109
const snapshot = buildSnapshotState(snapshotData ?? {}, effectiveFlags);
109-
session.snapshot = snapshot;
110+
// Mirror the real captureSnapshotForSession: session snapshot writes go
111+
// through setSessionSnapshot so snapshotRefsStale tracking (#1076) applies.
112+
setSessionSnapshot(session, snapshot);
110113
sessionStore.set(session.name, session);
111114
return snapshot;
112115
}
@@ -3101,3 +3104,210 @@ test('is reports Android permission dialog blocker when app content assertion fa
31013104
});
31023105
}
31033106
});
3107+
3108+
// --- Stale @ref warnings (#1076) ---
3109+
3110+
function makeTwoButtonNodes() {
3111+
return [
3112+
{
3113+
index: 0,
3114+
type: 'Application',
3115+
rect: { x: 0, y: 0, width: 390, height: 844 },
3116+
},
3117+
{
3118+
index: 1,
3119+
parentIndex: 0,
3120+
type: 'XCUIElementTypeButton',
3121+
label: 'Continue',
3122+
rect: { x: 10, y: 20, width: 100, height: 40 },
3123+
enabled: true,
3124+
hittable: true,
3125+
},
3126+
{
3127+
index: 2,
3128+
parentIndex: 0,
3129+
type: 'XCUIElementTypeButton',
3130+
label: 'Cancel',
3131+
rect: { x: 10, y: 80, width: 100, height: 40 },
3132+
enabled: true,
3133+
hittable: true,
3134+
},
3135+
];
3136+
}
3137+
3138+
function makeStaleRefSession(sessionName: string): SessionState {
3139+
const session = makeSession(sessionName);
3140+
session.snapshot = {
3141+
nodes: attachRefs(makeTwoButtonNodes() as never),
3142+
createdAt: Date.now(),
3143+
backend: 'xctest',
3144+
};
3145+
// As if the snapshot command just returned these refs to the client.
3146+
session.snapshotRefsStale = false;
3147+
return session;
3148+
}
3149+
3150+
async function runInteraction(
3151+
sessionStore: SessionStore,
3152+
sessionName: string,
3153+
command: string,
3154+
positionals: string[],
3155+
flags: Record<string, unknown> = {},
3156+
) {
3157+
return await handleInteractionCommands({
3158+
req: { token: 't', session: sessionName, command, positionals, flags },
3159+
sessionName,
3160+
sessionStore,
3161+
contextFromFlags,
3162+
});
3163+
}
3164+
3165+
test('press selector then press @ref warns that refs outlived the stored snapshot', async () => {
3166+
const sessionStore = makeSessionStore();
3167+
const sessionName = 'stale-ref-warns';
3168+
const session = makeStaleRefSession(sessionName);
3169+
sessionStore.set(sessionName, session);
3170+
mockDispatch.mockImplementation(async (_device, command) =>
3171+
command === 'snapshot' ? { nodes: makeTwoButtonNodes(), backend: 'xctest' } : {},
3172+
);
3173+
3174+
// Selector press: its resolution capture replaces the stored snapshot
3175+
// without handing the new refs back to the client.
3176+
const selectorPress = await runInteraction(sessionStore, sessionName, 'press', [
3177+
'label=Continue',
3178+
]);
3179+
if (!selectorPress?.ok) {
3180+
throw new Error(`selector press failed: ${JSON.stringify(selectorPress)}`);
3181+
}
3182+
expect(selectorPress.data?.warning).toBeUndefined();
3183+
expect(sessionStore.get(sessionName)?.snapshotRefsStale).toBe(true);
3184+
3185+
const refPress = await runInteraction(sessionStore, sessionName, 'press', ['@e1']);
3186+
expect(refPress?.ok).toBe(true);
3187+
if (refPress?.ok) {
3188+
expect(refPress.data?.warning).toBe(STALE_SNAPSHOT_REFS_WARNING);
3189+
}
3190+
});
3191+
3192+
test('press @ref directly after refs were issued does not warn', async () => {
3193+
const sessionStore = makeSessionStore();
3194+
const sessionName = 'fresh-ref-no-warning';
3195+
const session = makeStaleRefSession(sessionName);
3196+
sessionStore.set(sessionName, session);
3197+
3198+
const response = await runInteraction(sessionStore, sessionName, 'press', ['@e1']);
3199+
expect(response?.ok).toBe(true);
3200+
if (response?.ok) {
3201+
expect(response.data?.warning).toBeUndefined();
3202+
}
3203+
});
3204+
3205+
test('re-issuing refs clears the stale marker so press @ref does not warn', async () => {
3206+
const sessionStore = makeSessionStore();
3207+
const sessionName = 'reissued-refs-no-warning';
3208+
const session = makeStaleRefSession(sessionName);
3209+
sessionStore.set(sessionName, session);
3210+
mockDispatch.mockImplementation(async (_device, command) =>
3211+
command === 'snapshot' ? { nodes: makeTwoButtonNodes(), backend: 'xctest' } : {},
3212+
);
3213+
3214+
const selectorPress = await runInteraction(sessionStore, sessionName, 'press', [
3215+
'label=Continue',
3216+
]);
3217+
expect(selectorPress?.ok).toBe(true);
3218+
expect(sessionStore.get(sessionName)?.snapshotRefsStale).toBe(true);
3219+
3220+
// Simulate the snapshot command re-issuing refs (its handler clears the
3221+
// marker through buildNextSnapshotSession; covered in snapshot-handler tests).
3222+
const stored = sessionStore.get(sessionName)!;
3223+
stored.snapshotRefsStale = false;
3224+
sessionStore.set(sessionName, stored);
3225+
3226+
const refPress = await runInteraction(sessionStore, sessionName, 'press', ['@e1']);
3227+
expect(refPress?.ok).toBe(true);
3228+
if (refPress?.ok) {
3229+
expect(refPress.data?.warning).toBeUndefined();
3230+
}
3231+
});
3232+
3233+
test('fill @ref warns while refs are stale', async () => {
3234+
const sessionStore = makeSessionStore();
3235+
const sessionName = 'stale-ref-fill';
3236+
const session = makeStaleRefSession(sessionName);
3237+
session.snapshot = {
3238+
nodes: attachRefs([
3239+
{
3240+
index: 0,
3241+
type: 'XCUIElementTypeTextField',
3242+
label: 'Email',
3243+
rect: { x: 10, y: 20, width: 200, height: 40 },
3244+
enabled: true,
3245+
hittable: true,
3246+
},
3247+
]),
3248+
createdAt: Date.now(),
3249+
backend: 'xctest',
3250+
};
3251+
session.snapshotRefsStale = true;
3252+
sessionStore.set(sessionName, session);
3253+
3254+
const response = await runInteraction(sessionStore, sessionName, 'fill', [
3255+
'@e1',
3256+
'hello@example.com',
3257+
]);
3258+
expect(response?.ok).toBe(true);
3259+
if (response?.ok) {
3260+
expect(response.data?.warning).toBe(STALE_SNAPSHOT_REFS_WARNING);
3261+
}
3262+
});
3263+
3264+
test('get text @ref warns while refs are stale', async () => {
3265+
const sessionStore = makeSessionStore();
3266+
const sessionName = 'stale-ref-get-text';
3267+
const session = makeStaleRefSession(sessionName);
3268+
session.snapshotRefsStale = true;
3269+
sessionStore.set(sessionName, session);
3270+
mockDispatch.mockRejectedValue(
3271+
new Error('dispatch should not be called for snapshot-derived get text'),
3272+
);
3273+
3274+
const response = await runInteraction(sessionStore, sessionName, 'get', ['text', '@e1']);
3275+
expect(response?.ok).toBe(true);
3276+
if (response?.ok) {
3277+
expect(response.data?.warning).toBe(STALE_SNAPSHOT_REFS_WARNING);
3278+
expect(response.data?.ref).toBe('e1');
3279+
}
3280+
});
3281+
3282+
test('stale-ref warning appends to an existing interaction warning', async () => {
3283+
const sessionStore = makeSessionStore();
3284+
const sessionName = 'stale-ref-appends';
3285+
const session = makeStaleRefSession(sessionName);
3286+
session.snapshot = {
3287+
nodes: attachRefs([
3288+
{
3289+
index: 0,
3290+
// Non-fillable type produces the runtime fill warning the stale-ref
3291+
// warning must append to, not replace.
3292+
type: 'XCUIElementTypeStaticText',
3293+
label: 'Read-only',
3294+
rect: { x: 10, y: 20, width: 200, height: 40 },
3295+
enabled: true,
3296+
hittable: true,
3297+
},
3298+
]),
3299+
createdAt: Date.now(),
3300+
backend: 'xctest',
3301+
};
3302+
session.snapshotRefsStale = true;
3303+
sessionStore.set(sessionName, session);
3304+
3305+
const response = await runInteraction(sessionStore, sessionName, 'fill', ['@e1', 'nope']);
3306+
expect(response?.ok).toBe(true);
3307+
if (response?.ok) {
3308+
const warning = String(response.data?.warning ?? '');
3309+
expect(warning).toContain('attempting fill anyway');
3310+
expect(warning).toContain(STALE_SNAPSHOT_REFS_WARNING);
3311+
expect(warning.endsWith(STALE_SNAPSHOT_REFS_WARNING)).toBe(true);
3312+
}
3313+
});

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,54 @@ test('snapshot on iOS runs when the session tracks an app', async () => {
402402
);
403403
});
404404

405+
test('snapshot clears the stale-refs marker; diff leaves client refs stale (#1076)', async () => {
406+
const sessionStore = makeSessionStore();
407+
const sessionName = 'android-stale-refs-marker';
408+
const session = makeSession(sessionName, androidDevice);
409+
session.snapshot = {
410+
nodes: [{ ref: 'e1', index: 0, depth: 0, type: 'android.widget.Button', label: 'Old' }],
411+
createdAt: Date.now(),
412+
backend: 'android',
413+
};
414+
// As set by a selector-resolution capture that replaced the stored tree.
415+
session.snapshotRefsStale = true;
416+
sessionStore.set(sessionName, session);
417+
mockDispatch.mockResolvedValue({
418+
nodes: [{ index: 0, depth: 0, type: 'android.widget.Button', label: 'Fresh' }],
419+
truncated: false,
420+
backend: 'android',
421+
});
422+
423+
const snapshotResponse = await handleSnapshotCommands({
424+
req: { token: 't', session: sessionName, command: 'snapshot', positionals: [], flags: {} },
425+
sessionName,
426+
logPath: '/tmp/daemon.log',
427+
sessionStore,
428+
});
429+
430+
// The snapshot response hands every stored node's ref to the client.
431+
expect(snapshotResponse?.ok).toBe(true);
432+
expect(sessionStore.get(sessionName)?.snapshotRefsStale).toBe(false);
433+
434+
const diffResponse = await handleSnapshotCommands({
435+
req: {
436+
token: 't',
437+
session: sessionName,
438+
command: 'diff',
439+
positionals: ['snapshot'],
440+
flags: {},
441+
},
442+
sessionName,
443+
logPath: '/tmp/daemon.log',
444+
sessionStore,
445+
});
446+
447+
// diff replaces the stored tree but only returns a summary, so the refs the
448+
// client holds go stale again.
449+
expect(diffResponse?.ok).toBe(true);
450+
expect(sessionStore.get(sessionName)?.snapshotRefsStale).toBe(true);
451+
});
452+
405453
test('snapshot surfaces filtered-to-zero Android guidance for interactive snapshots', async () => {
406454
const sessionStore = makeSessionStore();
407455
const sessionName = 'android-empty-interactive';

src/daemon/handlers/find.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { errorResponse, noActiveSessionError } from './response.ts';
2323
import { recordSessionAction } from './handler-utils.ts';
2424
import { stripInternalInteractionFlags } from '../interaction-outcome-policy.ts';
2525
import { dispatchFindReadOnlyViaRuntime } from '../selector-runtime.ts';
26+
import { markSessionSnapshotRefsIssued } from '../session-snapshot.ts';
2627
import { createSelectorCaptureRuntime } from '../selector-capture-runtime.ts';
2728
import {
2829
isSparseSnapshotQualityVerdict,
@@ -156,6 +157,15 @@ export async function handleFindCommands(params: {
156157
const ref = `@${resolvedNode.ref}`;
157158
const actionFlags = { ...(req.flags ?? {}), noRecord: true };
158159
const match: ResolvedMatch = { node, resolvedNode, ref, nodes, actionFlags };
160+
if (session) {
161+
// #1076 clear choke point: every action below returns `match.ref`, minted
162+
// from the snapshot the capture runtime just stored, so the client leaves
163+
// with a ref that matches the stored tree. Clearing here also keeps the
164+
// internal `click/fill @ref` sub-invocations from attaching a spurious
165+
// stale-refs warning to the find response.
166+
markSessionSnapshotRefsIssued(session);
167+
sessionStore.set(sessionName, session);
168+
}
159169

160170
const actionHandlers: Record<string, () => Promise<DaemonResponse | null>> = {
161171
exists: () => handleFindExists(ctx),

src/daemon/handlers/interaction-touch-response.ts

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type {
55
PressCommandResult,
66
} from '../../contracts/interaction.ts';
77
import { successText } from '../../utils/success-text.ts';
8+
import { STALE_SNAPSHOT_REFS_WARNING } from '../session-snapshot.ts';
89
import { interactionResultExtra, stripAtPrefix } from './interaction-touch-targets.ts';
910

1011
/**
@@ -55,6 +56,13 @@ export function buildInteractionResponseData(params: {
5556
* button tags for click, selector/maestro details for the direct path.
5657
*/
5758
extra?: Record<string, unknown>;
59+
/**
60+
* The command consumed an `@ref` argument while the session's stored
61+
* snapshot had been replaced without re-issuing refs to the client
62+
* (`session.snapshotRefsStale`, #1076). Appends
63+
* STALE_SNAPSHOT_REFS_WARNING to the response warning.
64+
*/
65+
staleRefs?: boolean;
5866
}): InteractionResponsePayloads {
5967
const { source, referenceFrame, extra } = params;
6068
if (source.kind === 'runner-payload') {
@@ -89,13 +97,27 @@ export function buildInteractionResponseData(params: {
8997
...interactionResultExtra(result),
9098
}
9199
: visualization;
92-
if ('warning' in result && result.warning) {
93-
visualization.warning = result.warning;
94-
responseData.warning = result.warning;
100+
const warning = composeResponseWarning(
101+
'warning' in result ? result.warning : undefined,
102+
params.staleRefs,
103+
);
104+
if (warning) {
105+
visualization.warning = warning;
106+
responseData.warning = warning;
95107
}
96108
return { result: visualization, responseData };
97109
}
98110

111+
function composeResponseWarning(
112+
resultWarning: string | undefined,
113+
staleRefs: boolean | undefined,
114+
): string | undefined {
115+
if (staleRefs !== true) return resultWarning;
116+
return resultWarning
117+
? `${resultWarning} ${STALE_SNAPSHOT_REFS_WARNING}`
118+
: STALE_SNAPSHOT_REFS_WARNING;
119+
}
120+
99121
function buildTouchVisualizationResult(params: {
100122
data: Record<string, unknown> | undefined;
101123
fallbackX?: number;

0 commit comments

Comments
 (0)