Skip to content

Commit acb3731

Browse files
committed
fix(android): let the launch settle window end re-captures, never helper work
The open's 6 s window reached the snapshot helper as an abort, so a window that closed during a cold helper start or a borrowed capture tore the helper down and left the next read to recover it. The window is now a settle deadline on the transient capture: the content re-capture loop starts no attempt after it, while helper start, capture and teardown keep their own budgets and only the caller's signal cancels them. The transient read replaces the injected port, the borrow session scope and the interactor-side install mapping: the capture itself keeps a session it found, releases one it started, installs no helper and does not retire it after a content verdict. Refs #1571
1 parent f11a928 commit acb3731

13 files changed

Lines changed: 308 additions & 224 deletions

‎CHANGELOG.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,13 @@
4040
React Native app can still show a splash or an empty root at that time, so the first `snapshot`,
4141
`wait` or replay step after a cold `open --relaunch` captured an app without content. Its content
4242
re-capture usually recovered, but inside that command's own budget. The open now captures the
43-
launched app once, with the same content check and up to 3 re-captures, in a fixed 6 s window.
44-
This capture never installs the snapshot helper, does not reset it after a content verdict, and
43+
launched app once, with the same content check and up to 3 re-captures, and starts no re-capture
44+
after 6 s. The 6 s never cancels work: the helper start and each capture keep their own
45+
timeouts, and only cancelling the open stops them. This capture never installs the snapshot helper, does not reset it after a content verdict, and
4546
stops a helper session only if it started that session. So an open-only session does not keep
4647
UiAutomation, and the first read after the open starts the helper as before. `open_timing` reports
4748
`postOpenObservation` (see `PostOpenObservation`): `observable`; `unobservable` for a content
48-
verdict, a system surface over the app, or the window running out; `probe-failed` with a typed
49+
verdict or a system surface over the app; `probe-failed` with a typed
4950
`postOpenObservationFailure` when the capture could not run, for example before the helper is
5051
installed on a new device; and `app-unidentified` when the launched package could not be read. A
5152
URL or deep-link open reports none. The open succeeds in all of these cases, and a cancelled open

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,10 @@ export type SnapshotOptions = BaseSnapshotOptions & {
177177
acquisitionIntent?: 'full' | 'surface-observation';
178178
/**
179179
* A one-off read, such as an open's launch observation. It may use a capture host the session
180-
* already keeps warm, but it never installs one or leaves running one that it started.
180+
* already keeps warm, but it never installs one or leaves running one that it started. It starts
181+
* no re-capture after `settleBy` (epoch ms); only `signal` cancels work already started.
181182
*/
182-
transient?: boolean;
183+
transient?: Readonly<{ settleBy: number }>;
183184
};
184185

185186
/**

‎packages/platform-android/src/__tests__/snapshot-helper-session.fixtures.ts‎

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ export type PersistentSnapshotHelperProviderOptions = {
6262
oneShotXml?: string;
6363
/** Make the device-side stop fail the way an unhealthy transport answers. */
6464
runtimeStopFailure?: boolean;
65+
/** How long a started session takes to report ready. */
66+
sessionReadyDelayMs?: number;
67+
/** How long a session takes to answer its `snapshotCount`th capture. */
68+
captureResponseDelayMs?: (snapshotCount: number) => number;
6569
};
6670

6771
export function createPersistentSnapshotHelperProvider(
@@ -104,36 +108,40 @@ export function createPersistentSnapshotHelperProvider(
104108
const body = options.sessionXml
105109
? options.sessionXml(sessionIndex, snapshotCount)
106110
: `<hierarchy><node text="persistent helper snapshot ${snapshotCount}" bounds="[0,0][10,10]" /></hierarchy>`;
107-
socket.end(
108-
sessionResponse({
109-
requestId,
110-
body,
111-
metadata: {
112-
waitForIdleTimeoutMs: '500',
113-
waitForIdleQuietMs: '100',
114-
timeoutMs: '5000',
115-
maxDepth: '128',
116-
maxNodes: '5000',
117-
rootPresent: 'true',
118-
captureMode: 'interactive-windows',
119-
windowCount: '1',
120-
nodeCount: '1',
121-
truncated: 'false',
122-
elapsedMs: '8',
123-
},
124-
}),
125-
);
111+
const answer = () =>
112+
socket.end(
113+
sessionResponse({
114+
requestId,
115+
body,
116+
metadata: {
117+
waitForIdleTimeoutMs: '500',
118+
waitForIdleQuietMs: '100',
119+
timeoutMs: '5000',
120+
maxDepth: '128',
121+
maxNodes: '5000',
122+
rootPresent: 'true',
123+
captureMode: 'interactive-windows',
124+
windowCount: '1',
125+
nodeCount: '1',
126+
truncated: 'false',
127+
elapsedMs: '8',
128+
},
129+
}),
130+
);
131+
setTimeout(answer, options.captureResponseDelayMs?.(snapshotCount) ?? 0);
126132
});
127133
});
128134
server.listen(port, '127.0.0.1', () => {
129-
process.stdout.write(
130-
[
131-
'INSTRUMENTATION_STATUS: agentDeviceProtocol=android-snapshot-helper-v1',
132-
'INSTRUMENTATION_STATUS: sessionReady=true',
133-
'INSTRUMENTATION_STATUS_CODE: 2',
134-
'',
135-
].join('\n'),
136-
);
135+
setTimeout(() => {
136+
process.stdout.write(
137+
[
138+
'INSTRUMENTATION_STATUS: agentDeviceProtocol=android-snapshot-helper-v1',
139+
'INSTRUMENTATION_STATUS: sessionReady=true',
140+
'INSTRUMENTATION_STATUS_CODE: 2',
141+
'',
142+
].join('\n'),
143+
);
144+
}, options.sessionReadyDelayMs ?? 0);
137145
});
138146
process.onKill = () => {
139147
server.close(() => process.emitExit(0, null));

‎packages/platform-android/src/__tests__/snapshot.test.ts‎

Lines changed: 94 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,7 @@ test('snapshotAndroid keeps daemon-session helper alive for reuse until session
538538
);
539539
});
540540

541-
test('a borrowed capture releases a helper session it had to start', async () => {
541+
test('a transient capture releases a helper session it had to start', async () => {
542542
const adbCalls: (readonly string[])[] = [];
543543
const spawnArgs: (readonly string[])[] = [];
544544
const processes: FakeAndroidProcess[] = [];
@@ -551,7 +551,7 @@ test('a borrowed capture releases a helper session it had to start', async () =>
551551
await snapshotAndroid(device, {
552552
helperAdb: provider,
553553
helperArtifact,
554-
helperSessionScope: 'borrow',
554+
transient: openSettleWindow(),
555555
});
556556

557557
assert.equal(spawnArgs.length, 1);
@@ -562,7 +562,7 @@ test('a borrowed capture releases a helper session it had to start', async () =>
562562
);
563563
});
564564

565-
test('a borrowed capture leaves a warm daemon-session helper running', async () => {
565+
test('a transient capture leaves a warm daemon-session helper running', async () => {
566566
const adbCalls: (readonly string[])[] = [];
567567
const spawnArgs: (readonly string[])[] = [];
568568
const processes: FakeAndroidProcess[] = [];
@@ -580,7 +580,7 @@ test('a borrowed capture leaves a warm daemon-session helper running', async ()
580580
const borrowed = await snapshotAndroid(device, {
581581
helperAdb: provider,
582582
helperArtifact,
583-
helperSessionScope: 'borrow',
583+
transient: openSettleWindow(),
584584
});
585585

586586
assert.equal(borrowed.androidSnapshot.helperSessionReused, true);
@@ -662,7 +662,7 @@ test('snapshotAndroid retires content-invalid daemon helper before the next requ
662662
);
663663
});
664664

665-
test('a borrowed capture reports its content verdict without retiring the warm helper', async () => {
665+
test('a transient capture reports its content verdict without retiring the warm helper', async () => {
666666
const adbCalls: (readonly string[])[] = [];
667667
const spawnArgs: (readonly string[])[] = [];
668668
const processes: FakeAndroidProcess[] = [];
@@ -682,7 +682,7 @@ test('a borrowed capture reports its content verdict without retiring the warm h
682682
});
683683

684684
await assert.rejects(
685-
snapshotAndroid(device, { helperAdb: provider, helperArtifact, helperSessionScope: 'borrow' }),
685+
snapshotAndroid(device, { helperAdb: provider, helperArtifact, transient: openSettleWindow() }),
686686
(error: unknown) => isUnreadableCaptureContentError(error),
687687
);
688688

@@ -691,6 +691,90 @@ test('a borrowed capture reports its content verdict without retiring the warm h
691691
assert.equal(spawnArgs.length, 1);
692692
});
693693

694+
test('a settle window that passes during the helper start never cancels the start', async () => {
695+
const adbCalls: (readonly string[])[] = [];
696+
const spawnArgs: (readonly string[])[] = [];
697+
const processes: FakeAndroidProcess[] = [];
698+
const provider = createPersistentSnapshotHelperProvider({
699+
calls: adbCalls,
700+
spawnArgs,
701+
processes,
702+
sessionXml: () => androidSystemWindowOnlyXml(),
703+
sessionReadyDelayMs: 150,
704+
});
705+
706+
await assert.rejects(
707+
snapshotAndroid(device, {
708+
helperAdb: provider,
709+
helperArtifact,
710+
transient: { settleBy: Date.now() + 30 },
711+
}),
712+
(error: unknown) =>
713+
isUnreadableCaptureContentError(error) && (error as AppError).details?.attempts === 1,
714+
);
715+
716+
assert.equal(spawnArgs.length, 1);
717+
assert.equal(processes[0]?.killed, false, 'the started helper was not signalled');
718+
assert.equal(processes[0]?.exitCode, 0, 'the started helper quit on its own');
719+
assert.equal(adbCalls.some(isHelperRuntimeReset), false);
720+
});
721+
722+
test('a settle window that passes during a warm session capture leaves the session running', async () => {
723+
const adbCalls: (readonly string[])[] = [];
724+
const spawnArgs: (readonly string[])[] = [];
725+
const processes: FakeAndroidProcess[] = [];
726+
const provider = createPersistentSnapshotHelperProvider({
727+
calls: adbCalls,
728+
spawnArgs,
729+
processes,
730+
sessionXml: (_sessionIndex, snapshotCount) =>
731+
snapshotCount === 1
732+
? '<hierarchy><node text="warm helper" bounds="[0,0][10,10]" /></hierarchy>'
733+
: androidSystemWindowOnlyXml(),
734+
captureResponseDelayMs: (snapshotCount) => (snapshotCount === 1 ? 0 : 150),
735+
});
736+
await snapshotAndroid(device, {
737+
helperAdb: provider,
738+
helperArtifact,
739+
helperSessionScope: 'daemon-session',
740+
});
741+
742+
await assert.rejects(
743+
snapshotAndroid(device, {
744+
helperAdb: provider,
745+
helperArtifact,
746+
transient: { settleBy: Date.now() + 30 },
747+
}),
748+
(error: unknown) =>
749+
isUnreadableCaptureContentError(error) && (error as AppError).details?.attempts === 1,
750+
);
751+
752+
assert.equal(spawnArgs.length, 1);
753+
assert.equal(processes[0]?.killed, false);
754+
assert.equal(processes[0]?.exitCode, null, 'the session helper is still running');
755+
assert.equal(adbCalls.some(isHelperRuntimeReset), false);
756+
});
757+
758+
test('a transient capture on a device without the current helper installs nothing', async () => {
759+
const adbCalls: (readonly string[])[] = [];
760+
const helperAdb: AndroidAdbExecutor = async (args) => {
761+
adbCalls.push(args);
762+
if (isHelperVersionProbe(args)) return { exitCode: 1, stdout: '', stderr: 'not found' };
763+
return { exitCode: 0, stdout: '', stderr: '' };
764+
};
765+
766+
await assert.rejects(
767+
snapshotAndroid(device, { helperAdb, helperArtifact, transient: openSettleWindow() }),
768+
(error: unknown) =>
769+
(error as AppError).details?.reason === 'android-snapshot-helper-not-current',
770+
);
771+
772+
assert.equal(
773+
adbCalls.some((args) => args.includes('install') || args.includes('instrument')),
774+
false,
775+
);
776+
});
777+
694778
test('content-invalid daemon helper retirement force-stops the helper runtime', async () => {
695779
// Retirement after a content failure is a recovery path, not a release: the helper answered with
696780
// output we could not trust, so the next capture must meet a runtime that was reset. A clean quit
@@ -1514,3 +1598,7 @@ test('buildUiHierarchySnapshot derives hidden content hints from can-scroll-* on
15141598
assert.equal(scrollArea.hiddenContentAbove, true);
15151599
assert.equal(scrollArea.hiddenContentBelow, true);
15161600
});
1601+
1602+
function openSettleWindow(): { settleBy: number } {
1603+
return { settleBy: Date.now() + 60_000 };
1604+
}

0 commit comments

Comments
 (0)