Skip to content

Commit 80895ae

Browse files
authored
refactor: extract request execution scope (#523)
1 parent 801c2ae commit 80895ae

5 files changed

Lines changed: 443 additions & 135 deletions

File tree

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
import { test, expect } from 'vitest';
2+
import fs from 'node:fs';
3+
import os from 'node:os';
4+
import path from 'node:path';
5+
import { withDiagnosticsScope } from '../../utils/diagnostics.ts';
6+
import {
7+
makeAndroidSession,
8+
makeIosSession,
9+
} from '../../__tests__/test-utils/session-factories.ts';
10+
import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts';
11+
import { LeaseRegistry } from '../lease-registry.ts';
12+
import { clearRequestCanceled, markRequestCanceled } from '../request-cancel.ts';
13+
import {
14+
createRequestExecutionScope,
15+
prepareLockedRequestScope,
16+
} from '../request-execution-scope.ts';
17+
import type { DaemonRequest } from '../types.ts';
18+
19+
const TEST_ROOT = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-request-execution-scope-'));
20+
const LOG_PATH = path.join(TEST_ROOT, 'diagnostics.log');
21+
22+
test('createRequestExecutionScope applies tenant scoping and lease admission', async () => {
23+
const sessionStore = makeSessionStore('agent-device-request-scope-');
24+
const leaseRegistry = new LeaseRegistry();
25+
const lease = leaseRegistry.allocateLease({ tenantId: 'tenant-a', runId: 'run-1' });
26+
27+
const scope = await createRequestExecutionScope({
28+
req: makeRequest({
29+
session: 'default',
30+
command: 'snapshot',
31+
meta: {
32+
tenantId: 'tenant-a',
33+
runId: 'run-1',
34+
leaseId: lease.leaseId,
35+
sessionIsolation: 'tenant',
36+
},
37+
}),
38+
sessionStore,
39+
leaseRegistry,
40+
});
41+
42+
expect(scope.req.session).toBe('tenant-a:default');
43+
expect(scope.req.meta?.tenantId).toBe('tenant-a');
44+
expect(scope.sessionName).toBe('tenant-a:default');
45+
});
46+
47+
test('createRequestExecutionScope rejects tenant requests without an active lease', async () => {
48+
await expect(
49+
createRequestExecutionScope({
50+
req: makeRequest({
51+
session: 'default',
52+
command: 'snapshot',
53+
meta: {
54+
tenantId: 'tenant-a',
55+
runId: 'run-1',
56+
leaseId: '0'.repeat(32),
57+
sessionIsolation: 'tenant',
58+
},
59+
}),
60+
sessionStore: makeSessionStore('agent-device-request-scope-'),
61+
leaseRegistry: new LeaseRegistry(),
62+
}),
63+
).rejects.toThrow(/Lease is not active/);
64+
});
65+
66+
test('prepareLockedRequestScope preserves existing-session selector validation', async () => {
67+
const sessionStore = makeSessionStore('agent-device-request-scope-');
68+
sessionStore.set('default', makeAndroidSession('default'));
69+
const scope = await createRequestExecutionScope({
70+
req: makeRequest({
71+
command: 'snapshot',
72+
flags: {
73+
platform: 'ios',
74+
},
75+
}),
76+
sessionStore,
77+
leaseRegistry: new LeaseRegistry(),
78+
});
79+
80+
expect(() =>
81+
prepareLockedRequestScope({
82+
scope,
83+
logPath: LOG_PATH,
84+
sessionStore,
85+
trackDownloadableArtifact: () => 'artifact-id',
86+
}),
87+
).toThrow(/cannot be used with --platform=ios/i);
88+
});
89+
90+
test('prepareLockedRequestScope blocks commands for invalidated recordings before handlers run', async () => {
91+
const sessionStore = makeSessionStore('agent-device-request-scope-');
92+
sessionStore.set(
93+
'default',
94+
makeIosSession('default', {
95+
recording: {
96+
platform: 'ios-device-runner',
97+
outPath: '/tmp/recording.mp4',
98+
remotePath: '/tmp/remote.mp4',
99+
startedAt: Date.now(),
100+
showTouches: true,
101+
gestureEvents: [],
102+
invalidatedReason: 'iOS runner session restarted during recording',
103+
},
104+
}),
105+
);
106+
const scope = await createRequestExecutionScope({
107+
req: makeRequest({ command: 'snapshot' }),
108+
sessionStore,
109+
leaseRegistry: new LeaseRegistry(),
110+
});
111+
112+
const result = await withDiagnosticsScope({ command: 'snapshot', logPath: LOG_PATH }, async () =>
113+
prepareLockedRequestScope({
114+
scope,
115+
logPath: LOG_PATH,
116+
sessionStore,
117+
trackDownloadableArtifact: () => 'artifact-id',
118+
}),
119+
);
120+
121+
expect(result.type).toBe('response');
122+
if (result.type === 'response') {
123+
expect(result.response.ok).toBe(false);
124+
if (!result.response.ok) {
125+
expect(result.response.error.message).toBe('iOS runner session restarted during recording');
126+
}
127+
}
128+
});
129+
130+
test('runLocked rejects a canceled request before executing work', async () => {
131+
const requestId = 'request-scope-canceled-before-lock';
132+
const scope = await createRequestExecutionScope({
133+
req: makeRequest({ meta: { requestId } }),
134+
sessionStore: makeSessionStore('agent-device-request-scope-'),
135+
leaseRegistry: new LeaseRegistry(),
136+
});
137+
138+
markRequestCanceled(requestId);
139+
try {
140+
await expect(scope.runLocked(async () => 'ran')).rejects.toThrow(/request canceled/);
141+
} finally {
142+
clearRequestCanceled(requestId);
143+
}
144+
});
145+
146+
test('runLocked rejects a request canceled while waiting for its execution lock', async () => {
147+
const requestId = 'request-scope-canceled-after-lock';
148+
const sessionStore = makeSessionStore('agent-device-request-scope-');
149+
sessionStore.set('default', makeIosSession('default'));
150+
const leaseRegistry = new LeaseRegistry();
151+
const first = await createRequestExecutionScope({
152+
req: makeRequest({ command: 'click' }),
153+
sessionStore,
154+
leaseRegistry,
155+
});
156+
const second = await createRequestExecutionScope({
157+
req: makeRequest({ command: 'click', meta: { requestId } }),
158+
sessionStore,
159+
leaseRegistry,
160+
});
161+
let releaseLock: () => void = () => {};
162+
const lockReleased = new Promise<void>((resolve) => {
163+
releaseLock = resolve;
164+
});
165+
const firstRun = first.runLocked(async () => await lockReleased);
166+
const secondRun = second.runLocked(async () => 'ran');
167+
const secondExpectation = expect(secondRun).rejects.toThrow(/request canceled/);
168+
169+
markRequestCanceled(requestId);
170+
releaseLock();
171+
try {
172+
await firstRun;
173+
await secondExpectation;
174+
} finally {
175+
clearRequestCanceled(requestId);
176+
}
177+
});
178+
179+
function makeRequest(overrides: Partial<DaemonRequest> = {}): DaemonRequest {
180+
return {
181+
token: 'test-token',
182+
session: 'default',
183+
command: 'snapshot',
184+
positionals: [],
185+
...overrides,
186+
};
187+
}

src/daemon/request-android-adb.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
type AndroidAdbExecutor,
33
type AndroidAdbProvider,
4+
withAndroidAdbProvider,
45
} from '../platforms/android/adb-executor.ts';
56
import { resolveTargetDevice } from '../core/dispatch-resolve.ts';
67
import type { DeviceInfo } from '../utils/device.ts';
@@ -18,7 +19,7 @@ export type AndroidAdbProviderResolver = (params: {
1819
session?: AndroidAdbProviderRequestSession;
1920
}) => AndroidAdbProvider | AndroidAdbExecutor | undefined;
2021

21-
export async function resolveScopedAndroidAdbProvider(params: {
22+
async function resolveScopedAndroidAdbProvider(params: {
2223
req: DaemonRequest;
2324
existingSession: SessionState | undefined;
2425
androidAdbProvider?: AndroidAdbProviderResolver;
@@ -36,6 +37,26 @@ export async function resolveScopedAndroidAdbProvider(params: {
3637
return { provider, executor, serial: device.id };
3738
}
3839

40+
export async function withRequestAndroidAdbScope<T>(
41+
params: {
42+
req: DaemonRequest;
43+
existingSession: SessionState | undefined;
44+
androidAdbProvider?: AndroidAdbProviderResolver;
45+
},
46+
task: (adb: { executor?: AndroidAdbExecutor }) => Promise<T>,
47+
): Promise<T> {
48+
const requestAdb = await resolveScopedAndroidAdbProvider({
49+
req: params.req,
50+
existingSession: params.existingSession,
51+
androidAdbProvider: params.androidAdbProvider,
52+
});
53+
return await withAndroidAdbProvider(
54+
requestAdb.provider,
55+
{ serial: requestAdb.serial ?? '' },
56+
async () => await task({ executor: requestAdb.executor }),
57+
);
58+
}
59+
3960
async function resolveAndroidAdbProviderDevice(
4061
req: DaemonRequest,
4162
existingSession: SessionState | undefined,

src/daemon/request-cancel.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,12 @@ export function createRequestCanceledError(): AppError {
8787
});
8888
}
8989

90+
export function throwIfRequestCanceled(requestId: string | undefined): void {
91+
if (isRequestCanceled(requestId)) {
92+
throw createRequestCanceledError();
93+
}
94+
}
95+
9096
export function isRequestCanceledError(error: unknown): boolean {
9197
if (!(error instanceof AppError)) return false;
9298
if (error.code !== 'COMMAND_FAILED') return false;

0 commit comments

Comments
 (0)