Skip to content

Commit c6a0d24

Browse files
thymikeethymikee
committed
refactor: collocate iOS record context checks
1 parent d6e1c81 commit c6a0d24

3 files changed

Lines changed: 63 additions & 95 deletions

File tree

‎ios-runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,12 @@ final class RunnerTests: XCTestCase {
625625
else {
626626
return Response(ok: false, error: ErrorPayload(message: "recordStart requires outPath"))
627627
}
628+
let hasAppBundleId = !(command.appBundleId?
629+
.trimmingCharacters(in: .whitespacesAndNewlines)
630+
.isEmpty ?? true)
631+
guard hasAppBundleId else {
632+
return Response(ok: false, error: ErrorPayload(message: "recordStart requires appBundleId"))
633+
}
628634
if activeRecording != nil {
629635
return Response(ok: false, error: ErrorPayload(message: "recording already in progress"))
630636
}

‎src/daemon/handlers/__tests__/record-trace.test.ts‎

Lines changed: 36 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,14 @@ import { SessionStore } from '../../session-store.ts';
88
import type { SessionState } from '../../types.ts';
99

1010
type RecordTraceDeps = NonNullable<Parameters<typeof handleRecordTraceCommands>[0]['deps']>;
11+
type RunnerCall = {
12+
command: string;
13+
outPath?: string;
14+
fps?: number;
15+
appBundleId?: string;
16+
logPath?: string;
17+
traceLogPath?: string;
18+
};
1119

1220
function makeSessionStore(): SessionStore {
1321
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-record-trace-'));
@@ -23,6 +31,20 @@ function makeSession(name: string, device: SessionState['device']): SessionState
2331
};
2432
}
2533

34+
function makeIosDeviceSession(name: string, appBundleId?: string): SessionState {
35+
const session = makeSession(name, {
36+
platform: 'ios',
37+
id: 'ios-device-1',
38+
name: 'My iPhone',
39+
kind: 'device',
40+
booted: true,
41+
});
42+
if (appBundleId) {
43+
session.appBundleId = appBundleId;
44+
}
45+
return session;
46+
}
47+
2648
async function runRecordCommand(params: {
2749
sessionStore: SessionStore;
2850
sessionName: string;
@@ -49,7 +71,7 @@ async function runRecordCommand(params: {
4971
}
5072

5173
function makeIosDeviceRunnerDeps(
52-
runnerCalls: Array<{ command: string; outPath?: string; fps?: number; appBundleId?: string; logPath?: string; traceLogPath?: string }>,
74+
runnerCalls: RunnerCall[],
5375
runCmdCalls: Array<{ cmd: string; args: string[] }>,
5476
): RecordTraceDeps {
5577
const runIosRunnerCommand: RecordTraceDeps['runIosRunnerCommand'] = async (_device, command, options) => {
@@ -78,17 +100,10 @@ function makeIosDeviceRunnerDeps(
78100
test('record start/stop uses iOS runner on physical iOS devices', async () => {
79101
const sessionStore = makeSessionStore();
80102
const sessionName = 'ios-device';
81-
const session = makeSession(sessionName, {
82-
platform: 'ios',
83-
id: 'ios-device-1',
84-
name: 'My iPhone',
85-
kind: 'device',
86-
booted: true,
87-
});
88-
session.appBundleId = 'com.atebits.Tweetie2';
103+
const session = makeIosDeviceSession(sessionName, 'com.atebits.Tweetie2');
89104
sessionStore.set(sessionName, session);
90105

91-
const runnerCalls: Array<{ command: string; outPath?: string; fps?: number; appBundleId?: string; logPath?: string; traceLogPath?: string }> = [];
106+
const runnerCalls: RunnerCall[] = [];
92107
const runCmdCalls: Array<{ cmd: string; args: string[] }> = [];
93108
const deps = makeIosDeviceRunnerDeps(runnerCalls, runCmdCalls);
94109
const finalOut = path.join(os.tmpdir(), `agent-device-test-record-${Date.now()}.mp4`);
@@ -153,17 +168,10 @@ test('record start/stop uses iOS runner on physical iOS devices', async () => {
153168
test('record start resolves relative output path from request cwd', async () => {
154169
const sessionStore = makeSessionStore();
155170
const sessionName = 'ios-device-cwd';
156-
const session = makeSession(sessionName, {
157-
platform: 'ios',
158-
id: 'ios-device-1',
159-
name: 'My iPhone',
160-
kind: 'device',
161-
booted: true,
162-
});
163-
session.appBundleId = 'com.atebits.Tweetie2';
171+
const session = makeIosDeviceSession(sessionName, 'com.atebits.Tweetie2');
164172
sessionStore.set(sessionName, session);
165173

166-
const runnerCalls: Array<{ command: string; outPath?: string; fps?: number; appBundleId?: string; logPath?: string; traceLogPath?: string }> = [];
174+
const runnerCalls: RunnerCall[] = [];
167175
const runCmdCalls: Array<{ cmd: string; args: string[] }> = [];
168176
const deps = makeIosDeviceRunnerDeps(runnerCalls, runCmdCalls);
169177
const cwd = '/tmp/agent-device-cwd-test';
@@ -198,17 +206,10 @@ test('record start resolves relative output path from request cwd', async () =>
198206
test('record start forwards explicit fps to iOS runner', async () => {
199207
const sessionStore = makeSessionStore();
200208
const sessionName = 'ios-device-fps';
201-
const session = makeSession(sessionName, {
202-
platform: 'ios',
203-
id: 'ios-device-1',
204-
name: 'My iPhone',
205-
kind: 'device',
206-
booted: true,
207-
});
208-
session.appBundleId = 'com.atebits.Tweetie2';
209+
const session = makeIosDeviceSession(sessionName, 'com.atebits.Tweetie2');
209210
sessionStore.set(sessionName, session);
210211

211-
const runnerCalls: Array<{ command: string; outPath?: string; fps?: number; appBundleId?: string; logPath?: string; traceLogPath?: string }> = [];
212+
const runnerCalls: RunnerCall[] = [];
212213
const runCmdCalls: Array<{ cmd: string; args: string[] }> = [];
213214
const deps = makeIosDeviceRunnerDeps(runnerCalls, runCmdCalls);
214215
const response = await runRecordCommand({
@@ -228,13 +229,7 @@ test('record start forwards explicit fps to iOS runner', async () => {
228229
test('record start rejects invalid fps value', async () => {
229230
const sessionStore = makeSessionStore();
230231
const sessionName = 'ios-device-invalid-fps';
231-
sessionStore.set(sessionName, makeSession(sessionName, {
232-
platform: 'ios',
233-
id: 'ios-device-1',
234-
name: 'My iPhone',
235-
kind: 'device',
236-
booted: true,
237-
}));
232+
sessionStore.set(sessionName, makeIosDeviceSession(sessionName));
238233

239234
const response = await runRecordCommand({
240235
sessionStore,
@@ -260,13 +255,7 @@ test('record start rejects invalid fps value', async () => {
260255
test('record start on iOS device requires active app session context', async () => {
261256
const sessionStore = makeSessionStore();
262257
const sessionName = 'ios-device-no-app';
263-
sessionStore.set(sessionName, makeSession(sessionName, {
264-
platform: 'ios',
265-
id: 'ios-device-1',
266-
name: 'My iPhone',
267-
kind: 'device',
268-
booted: true,
269-
}));
258+
sessionStore.set(sessionName, makeIosDeviceSession(sessionName));
270259

271260
const response = await runRecordCommand({
272261
sessionStore,
@@ -291,14 +280,7 @@ test('record start on iOS device requires active app session context', async ()
291280
test('record start returns structured error when iOS runner start fails', async () => {
292281
const sessionStore = makeSessionStore();
293282
const sessionName = 'ios-device-start-fail';
294-
const session = makeSession(sessionName, {
295-
platform: 'ios',
296-
id: 'ios-device-1',
297-
name: 'My iPhone',
298-
kind: 'device',
299-
booted: true,
300-
});
301-
session.appBundleId = 'com.atebits.Tweetie2';
283+
const session = makeIosDeviceSession(sessionName, 'com.atebits.Tweetie2');
302284
sessionStore.set(sessionName, session);
303285

304286
const response = await runRecordCommand({
@@ -325,14 +307,7 @@ test('record start returns structured error when iOS runner start fails', async
325307
test('record start recovers from stale iOS runner recording state', async () => {
326308
const sessionStore = makeSessionStore();
327309
const sessionName = 'ios-device-runner-desync';
328-
const session = makeSession(sessionName, {
329-
platform: 'ios',
330-
id: 'ios-device-1',
331-
name: 'My iPhone',
332-
kind: 'device',
333-
booted: true,
334-
});
335-
session.appBundleId = 'com.atebits.Tweetie2';
310+
const session = makeIosDeviceSession(sessionName, 'com.atebits.Tweetie2');
336311
sessionStore.set(sessionName, session);
337312

338313
const commands: string[] = [];
@@ -367,14 +342,7 @@ test('record start recovers from stale iOS runner recording state', async () =>
367342
test('record start does not stop recording owned by another session during desync recovery', async () => {
368343
const sessionStore = makeSessionStore();
369344
const ownerSessionName = 'ios-device-owner';
370-
const ownerSession = makeSession(ownerSessionName, {
371-
platform: 'ios',
372-
id: 'ios-device-1',
373-
name: 'My iPhone',
374-
kind: 'device',
375-
booted: true,
376-
});
377-
ownerSession.appBundleId = 'com.example.owner';
345+
const ownerSession = makeIosDeviceSession(ownerSessionName, 'com.example.owner');
378346
ownerSession.recording = {
379347
platform: 'ios-device-runner',
380348
outPath: '/tmp/owner.mp4',
@@ -383,14 +351,7 @@ test('record start does not stop recording owned by another session during desyn
383351
sessionStore.set(ownerSessionName, ownerSession);
384352

385353
const sessionName = 'ios-device-requester';
386-
const requesterSession = makeSession(sessionName, {
387-
platform: 'ios',
388-
id: 'ios-device-1',
389-
name: 'My iPhone',
390-
kind: 'device',
391-
booted: true,
392-
});
393-
requesterSession.appBundleId = 'com.example.requester';
354+
const requesterSession = makeIosDeviceSession(sessionName, 'com.example.requester');
394355
sessionStore.set(sessionName, requesterSession);
395356

396357
const commands: string[] = [];
@@ -424,13 +385,7 @@ test('record stop clears iOS runner recording state when runner stop fails', asy
424385
const sessionStore = makeSessionStore();
425386
const sessionName = 'ios-device-stop-fail';
426387
sessionStore.set(sessionName, {
427-
...makeSession(sessionName, {
428-
platform: 'ios',
429-
id: 'ios-device-1',
430-
name: 'My iPhone',
431-
kind: 'device',
432-
booted: true,
433-
}),
388+
...makeIosDeviceSession(sessionName),
434389
recording: { platform: 'ios-device-runner', outPath: '/tmp/device.mp4', remotePath: 'tmp/device.mp4' },
435390
});
436391

‎src/daemon/handlers/record-trace.ts‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ function isRunnerRecordingAlreadyInProgressError(error: unknown): boolean {
3939
return errorMessage(error).toLowerCase().includes('recording already in progress');
4040
}
4141

42+
function normalizeAppBundleId(session: SessionState): string | undefined {
43+
const trimmed = session.appBundleId?.trim();
44+
return trimmed && trimmed.length > 0 ? trimmed : undefined;
45+
}
46+
4247
function findOtherActiveIosRunnerRecording(
4348
sessionStore: SessionStore,
4449
deviceId: string,
@@ -114,27 +119,28 @@ export async function handleRecordTraceCommands(params: {
114119
};
115120
}
116121
const outPath = req.positionals?.[1] ?? `./recording-${Date.now()}.mp4`;
117-
const resolvedOut = SessionStore.expandHome(outPath, req.meta?.cwd);
118-
const outDir = path.dirname(resolvedOut);
119-
fs.mkdirSync(outDir, { recursive: true });
120122
if (!isCommandSupportedOnDevice('record', device)) {
121123
return {
122124
ok: false,
123125
error: { code: 'UNSUPPORTED_OPERATION', message: 'record is not supported on this device' },
124126
};
125127
}
128+
const iosDeviceAppBundleId =
129+
device.platform === 'ios' && device.kind === 'device' ? normalizeAppBundleId(activeSession) : undefined;
130+
if (device.platform === 'ios' && device.kind === 'device' && !iosDeviceAppBundleId) {
131+
return {
132+
ok: false,
133+
error: {
134+
code: 'INVALID_ARGS',
135+
message: 'record on physical iOS devices requires an active app session; run open <app> first',
136+
},
137+
};
138+
}
139+
const resolvedOut = SessionStore.expandHome(outPath, req.meta?.cwd);
140+
fs.mkdirSync(path.dirname(resolvedOut), { recursive: true });
126141
const runnerOptions = getRunnerOptions(req, logPath, activeSession);
127142
if (device.platform === 'ios' && device.kind === 'device') {
128-
const appBundleId = activeSession.appBundleId?.trim();
129-
if (!appBundleId) {
130-
return {
131-
ok: false,
132-
error: {
133-
code: 'INVALID_ARGS',
134-
message: 'record on physical iOS devices requires an active app session; run open <app> first',
135-
},
136-
};
137-
}
143+
const appBundleId = iosDeviceAppBundleId;
138144
const recordingFileName = `agent-device-recording-${Date.now()}.mp4`;
139145
const remotePath = `tmp/${recordingFileName}`;
140146
const startRunnerRecording = async () => {
@@ -223,10 +229,11 @@ export async function handleRecordTraceCommands(params: {
223229
}
224230
const recording = activeSession.recording;
225231
if (recording.platform === 'ios-device-runner') {
232+
const appBundleId = normalizeAppBundleId(activeSession);
226233
try {
227234
await deps.runIosRunnerCommand(
228235
device,
229-
{ command: 'recordStop', appBundleId: activeSession.appBundleId },
236+
{ command: 'recordStop', appBundleId },
230237
getRunnerOptions(req, logPath, activeSession),
231238
);
232239
} catch (error) {

0 commit comments

Comments
 (0)