Skip to content

Commit ff6031b

Browse files
committed
fix(android): resolve touch helper artifact from the ADB provider like snapshots do (PR #1281 re-review)
- prepareAndroidTouchHelper now uses the same artifact precedence as snapshot capture: the scoped adbProvider's snapshotHelperArtifact when present, otherwise the bundled resolver (whose strict unavailable error is preserved). The provider artifact drives both the install decision and the instrumentationRunner used for one-shot commands, so an ADB-backed provider that supplies a helper artifact but no native touch override runs snapshots and gestures against the same single helper (issue #1275). Regression: 'a provider-supplied snapshotHelperArtifact overrides the bundled artifact for touch' pins the provider packageName on the install probe, the provider apkPath on the install call, the provider instrumentationRunner on the am instrument args, and that the bundled resolver is never invoked. - ADR 0002 now states explicitly that one-shot retry applies only to idempotent reads (viewport) after the failed session is stopped; non-idempotent gesture failures surface directly. - Helper README session transport corrected: a persistent process serving one short-lived socket connection per request (the server closes each accepted connection), not a single long-lived connection.
1 parent 9c13215 commit ff6031b

4 files changed

Lines changed: 72 additions & 7 deletions

File tree

‎android/snapshot-helper/README.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,10 @@ The `gesture` payload is a base64-encoded JSON object using protocol `android-to
7575
## Persistent Session
7676

7777
Passing `-e sessionPort <port>` keeps the instrumentation alive after startup and serves repeated
78-
commands over a local TCP socket on `127.0.0.1:<port>`, instead of exiting after one snapshot. This
79-
avoids paying UiAutomation connect/teardown cost per call. Each line sent to the socket is one
80-
command; the instrumentation replies once per command and keeps the connection open for the next
81-
one:
78+
commands over a local TCP server on `127.0.0.1:<port>`, instead of exiting after one snapshot. This
79+
avoids paying UiAutomation connect/teardown cost per call. Each command uses one short-lived
80+
connection: the client connects, sends a single command line, reads the response, and the server
81+
closes that connection; the process stays alive for the next connection:
8282

8383
- `snapshot <requestId>` — capture and return an XML snapshot, same semantics as the default mode
8484
- `viewport <requestId>` — return interactive-window viewport bounds

‎docs/adr/0002-persistent-platform-helper-sessions.md‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,11 @@ automation helper: a live persistent helper session executes touch commands dire
5959
session socket, and without one the same helper runs one-shot (amended 2026-07, issue #1275,
6060
consistent with ADR 0013; previously touch synthesis shipped as a separate instrumentation helper,
6161
so the daemon had to stop the persistent snapshot session before every gesture and let the next
62-
snapshot restart it lazily). Helper reuse must never turn process ownership into cross-command
63-
interference.
62+
snapshot restart it lazily). One-shot retry after a failed session command applies only to
63+
idempotent reads such as viewport resolution, and only after the failed session has been stopped.
64+
Gesture injection is not idempotent — events may already be partially injected — so a session
65+
gesture failure surfaces directly instead of retrying one-shot. Helper reuse must never turn
66+
process ownership into cross-command interference.
6467

6568
For iOS, keep the XCTest runner session as the reference implementation for lifecycle and
6669
invalidation behavior. Android does not need to copy iOS internals, but it should reuse the same

‎src/platforms/android/__tests__/touch-helper.test.ts‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
readAndroidTouchHelperFinalRecord,
1212
readAndroidTouchHelperViewport,
1313
} from '../touch-helper.ts';
14+
import { resolveAndroidHelperArtifact } from '../helper-package-install.ts';
1415
import { ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT } from '../../../__tests__/test-utils/android-snapshot-helper.ts';
1516
import {
1617
ANDROID_TOUCH_HELPER_MANIFEST as manifest,
@@ -189,6 +190,64 @@ test('one-shot gesture instruments the snapshot-helper runner with the touch-pla
189190
assert.equal(result.elapsedMs, 12);
190191
});
191192

193+
test('a provider-supplied snapshotHelperArtifact overrides the bundled artifact for touch', async () => {
194+
const device = makeIsolatedDevice();
195+
const providerArtifact = {
196+
...ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT,
197+
manifest: {
198+
...ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT.manifest,
199+
packageName: 'com.example.provider.snapshothelper',
200+
instrumentationRunner: 'com.example.provider.snapshothelper/.SnapshotInstrumentation',
201+
},
202+
};
203+
vi.mocked(resolveAndroidHelperArtifact).mockClear();
204+
205+
let probeArgs: string[] | undefined;
206+
let installArgs: string[] | undefined;
207+
let instrumentArgs: string[] | undefined;
208+
const result = await withAndroidAdbProvider(
209+
{
210+
// ADB-backed provider carrying a helper artifact but no native touch implementation:
211+
// gestures must install and run the provider's helper, same as snapshots do, never the
212+
// bundled artifact resolver.
213+
exec: async (args) => {
214+
if (args.includes('--show-versioncode')) {
215+
probeArgs = [...args];
216+
return {
217+
exitCode: 0,
218+
stdout: `package:${providerArtifact.manifest.packageName} versionCode:1`,
219+
stderr: '',
220+
};
221+
}
222+
if (args[0] === 'install') {
223+
installArgs = [...args];
224+
return { exitCode: 0, stdout: 'Success', stderr: '' };
225+
}
226+
instrumentArgs = [...args];
227+
return {
228+
exitCode: 0,
229+
stdout: [
230+
resultRecord({ ok: 'true', kind: 'swipe', injectedEvents: '4', elapsedMs: '12' }),
231+
'INSTRUMENTATION_CODE: 0',
232+
].join('\n'),
233+
stderr: '',
234+
};
235+
},
236+
snapshotHelperArtifact: providerArtifact,
237+
},
238+
{ serial: device.id },
239+
async () => await executeAndroidTouchHelperPlan(device, flingPlan()),
240+
);
241+
242+
assert.equal(vi.mocked(resolveAndroidHelperArtifact).mock.calls.length, 0);
243+
assert.ok(probeArgs?.includes(providerArtifact.manifest.packageName));
244+
assert.equal(installArgs?.at(-1), providerArtifact.apkPath);
245+
assert.equal(instrumentArgs?.at(-1), providerArtifact.manifest.instrumentationRunner);
246+
assert.equal(result.installReason, 'outdated');
247+
assert.equal(result.helperVersion, providerArtifact.manifest.version);
248+
assert.equal(result.helperTransport, 'instrumentation');
249+
});
250+
192251
test('one-shot gesture timeout extends beyond the plan duration for long-running gestures', async () => {
193252
const device = makeIsolatedDevice();
194253
let capturedTimeoutMs: number | undefined;

‎src/platforms/android/touch-helper.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,10 @@ export async function readAndroidTouchHelperViewport(device: DeviceInfo): Promis
134134

135135
async function prepareAndroidTouchHelper(device: DeviceInfo): Promise<PreparedAndroidTouchHelper> {
136136
const adbProvider = resolveAndroidAdbProvider(device);
137-
const artifact = await resolveAndroidTouchHelperArtifact();
137+
// Same artifact precedence as snapshot capture: a provider-supplied helper artifact overrides
138+
// the bundled one so both transports install and run the single shared helper (issue #1275).
139+
const artifact =
140+
adbProvider.snapshotHelperArtifact ?? (await resolveAndroidTouchHelperArtifact());
138141
const deviceKey = getAndroidSnapshotHelperSessionDeviceKey(device);
139142
const install = await withDiagnosticTimer(
140143
'android_touch_helper_install',

0 commit comments

Comments
 (0)