Skip to content

Commit 685bd4d

Browse files
thymikeethymikee
committed
fix(daemon): request-boundary provider runner scope + per-request interactor context
Review findings on #1389 (P1/P2): P1 — daemon routes that issue Apple runner commands outside interactor methods (keyboard, native alert, point read, iOS sequence chunks) escaped to the local XCTest runtime for provider devices. ProviderDeviceRuntime now exposes getAppleRunnerProvider; createProviderDeviceRuntimeRequestProviders composes it into an appleRunnerProvider request resolver and the daemon runtime wires it, so the existing request-boundary scope covers those routes with the provider transport. P2 — per-request RunnerContext was discarded for provider devices: getInteractor threads it through getProviderDeviceInteractor into ProviderDeviceRuntime.getInteractor, so runtimes composing the shared Apple interactor keep requestId (cancellation/accounting), appBundleId, and log paths per request. Lease-route commands (lease_allocate/heartbeat/release, artifacts) now skip sessionless provider-device resolution: they manage lease lifecycle, not a device session, and resolving a default device there spuriously triggered local device discovery before any lease existed. Integration coverage: keyboardDismiss reaches the provider transport via the request scope; every runner call in a request carries that request's id.
1 parent 2451658 commit 685bd4d

5 files changed

Lines changed: 154 additions & 36 deletions

File tree

‎src/core/command-descriptor/registry.ts‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,13 @@ const isShardedTestRequest = (req: DaemonRequest): boolean =>
9090
req.command === 'test' &&
9191
(typeof req.flags?.shardAll === 'number' || typeof req.flags?.shardSplit === 'number');
9292

93+
// Lease-route requests manage lease lifecycle/artifacts, not a device session:
94+
// resolving a default device for provider scoping would spuriously trigger
95+
// local device discovery before any lease exists.
96+
const LEASE_ROUTE_NO_PROVIDER_DEVICE = {
97+
skipSessionlessProviderDevice: () => true,
98+
} as const;
99+
93100
// ADR 0014 request-sensitive ref-frame resolvers. The action is the leading
94101
// positional (see keyboard/alert daemon writers in src/commands/system/index.ts
95102
// and src/commands/capture/alert.ts). Only the read-only status probes preserve
@@ -297,7 +304,12 @@ export const RAW_COMMAND_DESCRIPTORS = [
297304
...(ownerFilesEnabled ? { ownerFiles: ['src/daemon/handlers/lease.ts'] as const } : {}),
298305
catalog: { group: 'internal', key: 'leaseAllocate' },
299306
recordsSessionAction: false,
300-
daemon: { route: 'lease', refFrameEffect: 'preserve', ...ADMISSION_AND_LOCK_EXEMPT },
307+
daemon: {
308+
route: 'lease',
309+
refFrameEffect: 'preserve',
310+
...ADMISSION_AND_LOCK_EXEMPT,
311+
...LEASE_ROUTE_NO_PROVIDER_DEVICE,
312+
},
301313
timeoutPolicy: DEFAULT_TIMEOUT_POLICY,
302314
batchable: false,
303315
},
@@ -306,7 +318,12 @@ export const RAW_COMMAND_DESCRIPTORS = [
306318
...(ownerFilesEnabled ? { ownerFiles: ['src/daemon/handlers/lease.ts'] as const } : {}),
307319
catalog: { group: 'internal', key: 'leaseHeartbeat' },
308320
recordsSessionAction: false,
309-
daemon: { route: 'lease', refFrameEffect: 'preserve', ...ADMISSION_AND_LOCK_EXEMPT },
321+
daemon: {
322+
route: 'lease',
323+
refFrameEffect: 'preserve',
324+
...ADMISSION_AND_LOCK_EXEMPT,
325+
...LEASE_ROUTE_NO_PROVIDER_DEVICE,
326+
},
310327
timeoutPolicy: DEFAULT_TIMEOUT_POLICY,
311328
batchable: false,
312329
},
@@ -315,7 +332,12 @@ export const RAW_COMMAND_DESCRIPTORS = [
315332
...(ownerFilesEnabled ? { ownerFiles: ['src/daemon/handlers/lease.ts'] as const } : {}),
316333
catalog: { group: 'internal', key: 'leaseRelease' },
317334
recordsSessionAction: false,
318-
daemon: { route: 'lease', refFrameEffect: 'preserve', ...ADMISSION_AND_LOCK_EXEMPT },
335+
daemon: {
336+
route: 'lease',
337+
refFrameEffect: 'preserve',
338+
...ADMISSION_AND_LOCK_EXEMPT,
339+
...LEASE_ROUTE_NO_PROVIDER_DEVICE,
340+
},
319341
timeoutPolicy: DEFAULT_TIMEOUT_POLICY,
320342
batchable: false,
321343
},
@@ -324,7 +346,12 @@ export const RAW_COMMAND_DESCRIPTORS = [
324346
...(ownerFilesEnabled ? { ownerFiles: ['src/commands/management/artifacts.ts'] as const } : {}),
325347
catalog: { group: 'public' },
326348
recordsSessionAction: false,
327-
daemon: { route: 'lease', refFrameEffect: 'preserve', ...ADMISSION_AND_LOCK_EXEMPT },
349+
daemon: {
350+
route: 'lease',
351+
refFrameEffect: 'preserve',
352+
...ADMISSION_AND_LOCK_EXEMPT,
353+
...LEASE_ROUTE_NO_PROVIDER_DEVICE,
354+
},
328355
timeoutPolicy: DEFAULT_TIMEOUT_POLICY,
329356
batchable: false,
330357
},

‎src/core/interactors.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export async function getInteractor(
1414
runnerContext: RunnerContext,
1515
): Promise<Interactor> {
1616
if (isActiveProviderDevice(device)) {
17-
const providerInteractor = getProviderDeviceInteractor(device);
17+
const providerInteractor = getProviderDeviceInteractor(device, runnerContext);
1818
if (providerInteractor) return providerInteractor;
1919
throw new AppError(
2020
'UNSUPPORTED_OPERATION',

‎src/daemon/server/daemon-runtime.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ export async function startDaemonRuntime(
198198
leaseLifecycleProvider: providerRuntimeProviders.leaseLifecycleProvider,
199199
cloudArtifactProvider,
200200
deviceInventoryProvider: providerRuntimeProviders.deviceInventoryProvider,
201+
appleRunnerProvider: providerRuntimeProviders.appleRunnerProvider,
201202
providerRuntimeIds: providerRuntimeProviders.providerRuntimeIds,
202203
providerRuntimeRequiredIds: providerRuntimeProviders.providerRuntimeRequiredIds,
203204
providerDeviceRuntimeScope: providerRuntimeProviders.providerDeviceRuntimeScope,

‎src/provider-device-runtime.ts‎

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,13 @@ import type {
44
CloudArtifactsQuery,
55
CloudArtifactsResult,
66
} from './cloud-artifacts.ts';
7-
import type { Interactor } from './core/interactor-types.ts';
7+
import type { Interactor, RunnerContext } from './core/interactor-types.ts';
88
import type { DeviceInventoryProvider } from './core/dispatch-resolve.ts';
9+
import type { AppleRunnerProviderResolver } from './daemon/request-platform-providers.ts';
10+
import type {
11+
AppleRunnerCommandExecutor,
12+
AppleRunnerProvider,
13+
} from './platforms/apple/core/runner/runner-provider.ts';
914
import type { LeaseLifecycleContext, LeaseLifecycleProvider } from './daemon/handlers/lease.ts';
1015
import type { DeviceLease } from './daemon/lease-registry.ts';
1116
import { publicPlatformString, type DeviceInfo } from './kernel/device.ts';
@@ -31,7 +36,23 @@ export type ProviderDeviceRuntime = {
3136
cloudArtifacts?: CloudArtifactProvider;
3237
deviceInventoryProvider: DeviceInventoryProvider;
3338
ownsDevice(device: DeviceInfo): boolean;
34-
getInteractor(device: DeviceInfo): Interactor | undefined;
39+
/**
40+
* Builds the interactor for an owned device. `runnerContext` carries the
41+
* per-request context (requestId for cancellation, appBundleId, log paths);
42+
* runtimes composing the shared Apple interactor should thread it through so
43+
* runner calls stay attributable to the request that issued them.
44+
*/
45+
getInteractor(device: DeviceInfo, runnerContext?: RunnerContext): Interactor | undefined;
46+
/**
47+
* Apple runner transport for an owned device. When present, the daemon scopes
48+
* the WHOLE request with it (via the `appleRunnerProvider` request resolver),
49+
* so daemon routes that issue runner commands outside interactor methods
50+
* (keyboard, native alert, point read, iOS sequences) reach the provider
51+
* transport instead of the local XCTest runtime.
52+
*/
53+
getAppleRunnerProvider?(
54+
device: DeviceInfo,
55+
): AppleRunnerProvider | AppleRunnerCommandExecutor | undefined;
3556
installApp?(
3657
device: DeviceInfo,
3758
app: string,
@@ -65,6 +86,7 @@ export type ProviderDeviceRuntimeRequestProviders = {
6586
recoverExpiredLease?: ProviderExpiredLeaseRecovery;
6687
cloudArtifactProvider?: CloudArtifactProvider;
6788
deviceInventoryProvider?: DeviceInventoryProvider;
89+
appleRunnerProvider?: AppleRunnerProviderResolver;
6890
providerDeviceRuntimeScope?: <T>(task: () => Promise<T>) => Promise<T>;
6991
};
7092

@@ -87,10 +109,13 @@ async function withProviderDeviceRuntimeScope<T>(
87109
return await providerDeviceRuntimeScope.run([...runtimes], task);
88110
}
89111

90-
export function getProviderDeviceInteractor(device: DeviceInfo): Interactor | undefined {
112+
export function getProviderDeviceInteractor(
113+
device: DeviceInfo,
114+
runnerContext?: RunnerContext,
115+
): Interactor | undefined {
91116
for (const runtime of getActiveProviderDeviceRuntimes()) {
92117
if (!runtime.ownsDevice(device)) continue;
93-
const interactor = runtime.getInteractor(device);
118+
const interactor = runtime.getInteractor(device, runnerContext);
94119
if (interactor) return interactor;
95120
}
96121
return undefined;
@@ -168,11 +193,26 @@ export function createProviderDeviceRuntimeRequestProviders(
168193
recoverExpiredLease: composeExpiredLeaseRecovery(runtimes),
169194
cloudArtifactProvider: composeCloudArtifactProvider(runtimes),
170195
deviceInventoryProvider: composeDeviceInventoryProvider(runtimes),
196+
appleRunnerProvider: composeAppleRunnerProviderResolver(runtimes),
171197
providerDeviceRuntimeScope: async (task) =>
172198
await withProviderDeviceRuntimeScope(runtimes, task),
173199
};
174200
}
175201

202+
function composeAppleRunnerProviderResolver(
203+
runtimes: ProviderDeviceRuntime[],
204+
): AppleRunnerProviderResolver | undefined {
205+
if (!runtimes.some((runtime) => runtime.getAppleRunnerProvider !== undefined)) return undefined;
206+
return (context) => {
207+
for (const runtime of runtimes) {
208+
if (!runtime.ownsDevice(context.device)) continue;
209+
const provider = runtime.getAppleRunnerProvider?.(context.device);
210+
if (provider) return provider;
211+
}
212+
return undefined;
213+
};
214+
}
215+
176216
function composeExpiredLeaseRecovery(
177217
runtimes: ProviderDeviceRuntime[],
178218
): ProviderExpiredLeaseRecovery | undefined {

‎test/integration/provider-scenarios/provider-ios-runner-transport.test.ts‎

Lines changed: 77 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,17 @@ import {
55
type ProviderDeviceRuntime,
66
} from '../../../src/provider-device-runtime.ts';
77
import type { DeviceInventoryProvider } from '../../../src/core/dispatch-resolve.ts';
8-
import type { Interactor } from '../../../src/core/interactor-types.ts';
8+
import type { Interactor, RunnerContext } from '../../../src/core/interactor-types.ts';
99
import type { LeaseLifecycleProvider } from '../../../src/daemon/handlers/lease.ts';
1010
import type { DeviceLease } from '../../../src/daemon/lease-registry.ts';
1111
import type { DaemonRequest } from '../../../src/daemon/types.ts';
1212
import type { DeviceInfo } from '../../../src/kernel/device.ts';
1313
import { createAppleInteractor } from '../../../src/platforms/apple/interactor.ts';
1414
import type { RunnerCommand } from '../../../src/platforms/apple/core/runner/runner-contract.ts';
15-
import type { AppleRunnerProvider } from '../../../src/platforms/apple/core/runner/runner-provider.ts';
15+
import type {
16+
AppleRunnerCommandOptions,
17+
AppleRunnerProvider,
18+
} from '../../../src/platforms/apple/core/runner/runner-provider.ts';
1619
import { assertRpcOk } from './assertions.ts';
1720
import { createProviderScenarioHarness, withProviderScenarioResource } from './harness.ts';
1821

@@ -27,6 +30,9 @@ const DEVICE: DeviceInfo = {
2730
booted: true,
2831
};
2932

33+
type RecordedRunnerCall = { command: RunnerCommand; options: AppleRunnerCommandOptions };
34+
type RunnerTransportCalls = { runner: RecordedRunnerCall[]; opens: number };
35+
3036
// Issue #1297 acceptance path: a provider that only supplies an
3137
// AppleRunnerProvider transport (plus its own `open`) reuses the SHARED Apple
3238
// interactor — selector resolution, tap, fill, and snapshot all arrive at the
@@ -70,30 +76,73 @@ test('provider-supplied Apple runner transport reuses the shared interactor stac
7076
assertRpcOk(
7177
await daemon.callCommand('click', ['label="Provider Ready"'], request.flags, request),
7278
);
73-
const tap = calls.commands.find((command) => command.command === 'tap');
79+
const tap = calls.runner.find((call) => call.command.command === 'tap');
7480
assert.ok(tap, 'expected a runner-protocol tap on the provider transport');
7581
// The shared runtime resolved the selector against the transport's snapshot
7682
// and tapped the element center, whichever tap vehicle it picked.
77-
assert.deepEqual({ x: tap.x, y: tap.y }, { x: 60, y: 30 });
83+
assert.deepEqual({ x: tap.command.x, y: tap.command.y }, { x: 60, y: 30 });
7884

7985
assertRpcOk(
8086
await daemon.callCommand('fill', ['label="Provider Input"', 'hello'], request.flags, request),
8187
);
82-
const fill = calls.commands.find(
83-
(command) => command.command === 'type' && command.textEntryMode === 'replace',
88+
const fill = calls.runner.find(
89+
(call) => call.command.command === 'type' && call.command.textEntryMode === 'replace',
8490
);
8591
assert.ok(fill, 'expected a runner-protocol fill on the provider transport');
86-
assert.equal(fill.text, 'hello');
87-
assert.deepEqual({ x: fill.x, y: fill.y }, { x: 60, y: 90 });
92+
assert.equal(fill.command.text, 'hello');
93+
assert.deepEqual({ x: fill.command.x, y: fill.command.y }, { x: 60, y: 90 });
8894

89-
const snapshots = calls.commands.filter((command) => command.command === 'snapshot');
95+
const snapshots = calls.runner.filter((call) => call.command.command === 'snapshot');
9096
assert.ok(
9197
snapshots.length >= 4,
9298
`expected shared snapshot runtime traffic on the transport, got ${snapshots.length}`,
9399
);
94100
});
95101
});
96102

103+
// Daemon routes that issue runner commands OUTSIDE interactor methods (keyboard,
104+
// native alert, point read, iOS sequences) must reach the provider transport via
105+
// the request-boundary `appleRunnerProvider` scope instead of escaping to the
106+
// local XCTest runtime.
107+
test('daemon direct runner routes reach the provider transport through the request scope', async () => {
108+
await withProviderScenarioResource(createRunnerTransportWorld, async ({ daemon, calls }) => {
109+
const lease = await allocateLease(daemon);
110+
const request = { flags: leaseFlags(lease.leaseId), meta: leaseMeta(lease.leaseId) };
111+
assertRpcOk(await daemon.callCommand('open', ['com.example.app'], request.flags, request));
112+
113+
calls.runner.length = 0;
114+
assertRpcOk(await daemon.callCommand('keyboard', ['dismiss'], request.flags, request));
115+
const dismiss = calls.runner.find((call) => call.command.command === 'keyboardDismiss');
116+
assert.ok(dismiss, 'expected keyboardDismiss on the provider transport, not local XCTest');
117+
});
118+
});
119+
120+
// Per-request RunnerContext threading: the daemon builds the interactor per
121+
// request, so runner calls carry the request's id (cancellation/accounting)
122+
// instead of the construction-time context.
123+
test('provider transport runner calls carry the per-request id', async () => {
124+
await withProviderScenarioResource(createRunnerTransportWorld, async ({ daemon, calls }) => {
125+
const lease = await allocateLease(daemon);
126+
const flags = leaseFlags(lease.leaseId);
127+
const openMeta = { ...leaseMeta(lease.leaseId), requestId: 'req-open-1' };
128+
assertRpcOk(await daemon.callCommand('open', ['com.example.app'], flags, { meta: openMeta }));
129+
130+
calls.runner.length = 0;
131+
const clickMeta = { ...leaseMeta(lease.leaseId), requestId: 'req-click-1' };
132+
assertRpcOk(
133+
await daemon.callCommand('click', ['label="Provider Ready"'], flags, { meta: clickMeta }),
134+
);
135+
assert.ok(calls.runner.length >= 1, 'expected runner traffic for the click request');
136+
for (const call of calls.runner) {
137+
assert.equal(
138+
call.options.requestId,
139+
'req-click-1',
140+
`runner ${call.command.command} lost the per-request id`,
141+
);
142+
}
143+
});
144+
});
145+
97146
async function allocateLease(
98147
daemon: Awaited<ReturnType<typeof createProviderScenarioHarness>>,
99148
): Promise<DeviceLease> {
@@ -104,7 +153,7 @@ async function allocateLease(
104153
}
105154

106155
async function createRunnerTransportWorld() {
107-
const calls = { commands: [] as RunnerCommand[], opens: 0 };
156+
const calls: RunnerTransportCalls = { runner: [], opens: 0 };
108157
const runtime = createProviderRuntime(calls);
109158
const providers = createProviderDeviceRuntimeRequestProviders([runtime]);
110159
const daemon = await createProviderScenarioHarness({
@@ -121,11 +170,13 @@ async function createRunnerTransportWorld() {
121170
};
122171
}
123172

124-
function createProviderRuntime(calls: {
125-
commands: RunnerCommand[];
126-
opens: number;
127-
}): ProviderDeviceRuntime {
128-
const interactor = createRunnerTransportInteractor(calls);
173+
function createProviderRuntime(calls: RunnerTransportCalls): ProviderDeviceRuntime {
174+
const transport: AppleRunnerProvider = {
175+
runCommand: async (_device, command, options) => {
176+
calls.runner.push({ command, options });
177+
return runnerResultFor(command);
178+
},
179+
};
129180
const leaseLifecycle: LeaseLifecycleProvider = {
130181
allocate: async (lease) =>
131182
lease.leaseProvider === PROVIDER ? { provider: PROVIDER, deviceId: DEVICE.id } : undefined,
@@ -137,23 +188,22 @@ function createProviderRuntime(calls: {
137188
leaseLifecycle,
138189
deviceInventoryProvider,
139190
ownsDevice: (device) => device.id === DEVICE.id,
140-
getInteractor: (device) => (device.id === DEVICE.id ? interactor : undefined),
191+
getInteractor: (device, runnerContext) =>
192+
device.id === DEVICE.id
193+
? createRunnerTransportInteractor(calls, transport, runnerContext)
194+
: undefined,
195+
getAppleRunnerProvider: (device) => (device.id === DEVICE.id ? transport : undefined),
141196
shutdown: async () => undefined,
142197
};
143198
}
144199

145-
function createRunnerTransportInteractor(calls: {
146-
commands: RunnerCommand[];
147-
opens: number;
148-
}): Interactor {
149-
const transport: AppleRunnerProvider = {
150-
runCommand: async (_device, command) => {
151-
calls.commands.push(command);
152-
return runnerResultFor(command);
153-
},
154-
};
200+
function createRunnerTransportInteractor(
201+
calls: RunnerTransportCalls,
202+
transport: AppleRunnerProvider,
203+
runnerContext: RunnerContext | undefined,
204+
): Interactor {
155205
return {
156-
...createAppleInteractor(DEVICE, {}, transport),
206+
...createAppleInteractor(DEVICE, runnerContext ?? {}, transport),
157207
// App lifecycle stays provider-owned: the transport seam covers runner
158208
// commands only, so the provider composes its own `open` on top.
159209
open: async () => {

0 commit comments

Comments
 (0)