Skip to content

Commit bda96f1

Browse files
authored
fix: allow inventory selectors with session locks (#583)
* fix: allow inventory selectors with session locks * fix: reduce lock policy complexity * refactor: simplify lock policy helpers * refactor: trim lock policy cleanup * fix: preserve explicit inventory selectors
1 parent 47b981c commit bda96f1

4 files changed

Lines changed: 214 additions & 13 deletions

File tree

src/daemon/__tests__/request-lock-policy.test.ts

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,103 @@ test('rejects existing-session selector conflicts under request lock policy', ()
118118
);
119119
});
120120

121+
test.each([
122+
{
123+
command: 'apps',
124+
flags: { platform: 'ios', device: 'iPhone 17' },
125+
expected: { platform: 'ios', device: 'iPhone 17', serial: undefined },
126+
},
127+
{
128+
command: 'devices',
129+
flags: { platform: 'android', serial: 'emulator-5554' },
130+
expected: { platform: 'android', device: undefined, serial: 'emulator-5554' },
131+
},
132+
] as const)(
133+
'allows $command to inspect a different selector under existing-session lock policy',
134+
({ command, flags, expected }) => {
135+
const req = applyRequestLockPolicy(
136+
{
137+
token: 'token',
138+
session: 'qa-ios',
139+
command,
140+
positionals: [],
141+
flags,
142+
meta: {
143+
lockPolicy: 'reject',
144+
},
145+
},
146+
IOS_SESSION,
147+
);
148+
149+
assert.deepEqual(selectedFlags(req), expected);
150+
},
151+
);
152+
153+
test.each([
154+
{
155+
command: 'apps',
156+
flags: { device: 'iPhone 17' },
157+
expected: { platform: undefined, device: 'iPhone 17', serial: undefined },
158+
},
159+
{
160+
command: 'devices',
161+
flags: { serial: 'emulator-5554' },
162+
expected: { platform: undefined, device: undefined, serial: 'emulator-5554' },
163+
},
164+
] as const)(
165+
'allows $command to inspect a fresh selector under session lock policy',
166+
({ command, flags, expected }) => {
167+
const req = applyRequestLockPolicy({
168+
token: 'token',
169+
session: 'qa-ios',
170+
command,
171+
positionals: [],
172+
flags,
173+
meta: {
174+
lockPolicy: 'reject',
175+
lockPlatform: 'ios',
176+
},
177+
});
178+
179+
assert.deepEqual(selectedFlags(req), expected);
180+
},
181+
);
182+
183+
test('allows inventory commands to use explicit Apple selectors under another lock platform', () => {
184+
const req = applyRequestLockPolicy({
185+
token: 'token',
186+
session: 'qa-android',
187+
command: 'apps',
188+
positionals: [],
189+
flags: {
190+
udid: 'SIM-001',
191+
},
192+
meta: {
193+
lockPolicy: 'reject',
194+
lockPlatform: 'android',
195+
},
196+
});
197+
198+
assert.equal(req.flags?.platform, undefined);
199+
assert.equal(req.flags?.udid, 'SIM-001');
200+
});
201+
202+
test('defaults inventory commands without explicit selectors to the lock platform', () => {
203+
const req = applyRequestLockPolicy({
204+
token: 'token',
205+
session: 'qa-ios',
206+
command: 'apps',
207+
positionals: [],
208+
flags: {},
209+
meta: {
210+
lockPolicy: 'reject',
211+
lockPlatform: 'ios',
212+
},
213+
});
214+
215+
assert.equal(req.flags?.platform, 'ios');
216+
});
217+
121218
test('allows matching redundant selectors for existing sessions', () => {
122219
const req = applyRequestLockPolicy(
123220
{
@@ -256,3 +353,15 @@ test('strips only conflicting selectors for existing sessions', () => {
256353
assert.equal(req.flags?.device, 'iPhone 16');
257354
assert.equal(req.flags?.serial, undefined);
258355
});
356+
357+
function selectedFlags(req: ReturnType<typeof applyRequestLockPolicy>): {
358+
platform: string | undefined;
359+
device: string | undefined;
360+
serial: string | undefined;
361+
} {
362+
return {
363+
platform: req.flags?.platform,
364+
device: req.flags?.device,
365+
serial: req.flags?.serial,
366+
};
367+
}

src/daemon/__tests__/request-platform-providers.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,22 @@ import {
66
makeAndroidSession,
77
makeIosSession,
88
} from '../../__tests__/test-utils/index.ts';
9+
import { withTargetDeviceResolutionScope } from '../../core/dispatch-resolve.ts';
910
import { createLocalAppleToolProvider, runXcrun } from '../../platforms/ios/tool-provider.ts';
11+
import type { DeviceInfo } from '../../utils/device.ts';
1012
import { startAppLog } from '../app-log.ts';
1113
import { resolveRecordingProvider } from '../recording-provider.ts';
1214
import { withRequestPlatformProviderScope } from '../request-platform-providers.ts';
1315
import type { DaemonRequest } from '../types.ts';
1416

17+
const OTHER_IOS_SIMULATOR: DeviceInfo = {
18+
platform: 'ios',
19+
id: 'sim-2',
20+
name: 'iPhone 17',
21+
kind: 'simulator',
22+
booted: true,
23+
};
24+
1525
test('request platform provider scope exposes Android executor for Android sessions', async () => {
1626
const calls: string[][] = [];
1727
const response = await withRequestPlatformProviderScope(
@@ -179,6 +189,44 @@ test('request platform provider scope applies Apple tool provider only for Apple
179189
assert.deepEqual(calls, [['list', 'devices', '-j']]);
180190
});
181191

192+
test('request platform provider scope follows explicit apps selector for existing sessions', async () => {
193+
const seenDevices: string[] = [];
194+
195+
const result = await withTargetDeviceResolutionScope(
196+
async () => [OTHER_IOS_SIMULATOR],
197+
async () =>
198+
await withRequestPlatformProviderScope(
199+
{
200+
req: {
201+
...request('apps'),
202+
flags: {
203+
platform: 'ios',
204+
device: 'iPhone 17',
205+
},
206+
},
207+
existingSession: makeIosSession('default'),
208+
providers: {
209+
appleToolProvider: ({ device, session }) => {
210+
seenDevices.push(`${session?.name}:${device.id}`);
211+
return createLocalAppleToolProvider({
212+
runCommand: async (cmd, args) => {
213+
throw new Error(`unexpected generic command: ${cmd} ${args.join(' ')}`);
214+
},
215+
simctl: {
216+
run: async () => ({ exitCode: 0, stdout: 'apps-ok', stderr: '' }),
217+
},
218+
});
219+
},
220+
},
221+
},
222+
async () => await runXcrun(['simctl', 'listapps', OTHER_IOS_SIMULATOR.id]),
223+
),
224+
);
225+
226+
assert.equal(result.stdout, 'apps-ok');
227+
assert.deepEqual(seenDevices, [`default:${OTHER_IOS_SIMULATOR.id}`]);
228+
});
229+
182230
test('request platform provider scopes stay isolated across concurrent requests', async () => {
183231
const androidCalls: string[] = [];
184232
const appleCalls: string[] = [];

src/daemon/request-lock-policy.ts

Lines changed: 52 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { AppError } from '../utils/errors.ts';
22
import type { CommandFlags } from '../core/dispatch.ts';
33
import type { SessionState, DaemonRequest } from './types.ts';
4+
import { PUBLIC_COMMANDS } from '../command-catalog.ts';
45
import {
56
formatSessionSelectorConflict,
67
listSessionSelectorConflicts,
@@ -20,6 +21,11 @@ const LOCKABLE_SELECTOR_KEYS: Array<keyof CommandFlags> = [
2021
'androidDeviceAllowlist',
2122
];
2223

24+
const SELECTOR_OVERRIDE_LOCK_POLICY_COMMANDS: ReadonlySet<string> = new Set([
25+
PUBLIC_COMMANDS.apps,
26+
PUBLIC_COMMANDS.devices,
27+
]);
28+
2329
export function applyRequestLockPolicy(
2430
req: DaemonRequest,
2531
existingSession?: SessionState,
@@ -30,13 +36,19 @@ export function applyRequestLockPolicy(
3036
}
3137

3238
const nextFlags: CommandFlags = { ...(req.flags ?? {}) };
33-
const conflicts = existingSession
34-
? listSessionSelectorConflicts(existingSession, nextFlags)
35-
: listFreshSessionConflicts(nextFlags, req.meta?.lockPlatform, req.command);
39+
const canOverrideSelector = SELECTOR_OVERRIDE_LOCK_POLICY_COMMANDS.has(req.command);
40+
const conflicts = canOverrideSelector
41+
? []
42+
: existingSession
43+
? listSessionSelectorConflicts(existingSession, nextFlags)
44+
: listFreshSessionConflicts(nextFlags, req.meta?.lockPlatform, req.command);
45+
const lockPlatform = req.meta?.lockPlatform;
3646

3747
if (conflicts.length === 0) {
38-
if (!existingSession && req.meta?.lockPlatform && nextFlags.platform === undefined) {
39-
nextFlags.platform = req.meta.lockPlatform;
48+
if (
49+
shouldApplyLockPlatformDefault(canOverrideSelector, existingSession, nextFlags, lockPlatform)
50+
) {
51+
nextFlags.platform = lockPlatform;
4052
}
4153
return {
4254
...req,
@@ -45,12 +57,7 @@ export function applyRequestLockPolicy(
4557
}
4658

4759
if (lockPolicy === 'strip') {
48-
if (existingSession) {
49-
stripSessionConflicts(nextFlags, conflicts);
50-
nextFlags.platform = existingSession.device.platform;
51-
} else {
52-
stripFreshSessionConflicts(nextFlags, req.meta?.lockPlatform);
53-
}
60+
applyStripLockPolicy(nextFlags, conflicts, lockPlatform, existingSession);
5461
return {
5562
...req,
5663
flags: nextFlags,
@@ -64,6 +71,35 @@ export function applyRequestLockPolicy(
6471
);
6572
}
6673

74+
function shouldApplyLockPlatformDefault(
75+
canOverrideSelector: boolean,
76+
existingSession: SessionState | undefined,
77+
flags: CommandFlags,
78+
lockPlatform: LockPlatform,
79+
): boolean {
80+
if (!lockPlatform || existingSession || flags.platform !== undefined) {
81+
return false;
82+
}
83+
if (!canOverrideSelector) {
84+
return true;
85+
}
86+
return !LOCKABLE_SELECTOR_KEYS.some((key) => hasSelectorValue(flags[key]));
87+
}
88+
89+
function applyStripLockPolicy(
90+
flags: CommandFlags,
91+
conflicts: SessionSelectorConflict[],
92+
lockPlatform: LockPlatform,
93+
existingSession: SessionState | undefined,
94+
): void {
95+
if (existingSession) {
96+
stripSessionConflicts(flags, conflicts);
97+
flags.platform = existingSession.device.platform;
98+
return;
99+
}
100+
stripFreshSessionConflicts(flags, lockPlatform);
101+
}
102+
67103
function listFreshSessionConflicts(
68104
flags: CommandFlags,
69105
lockPlatform: LockPlatform,
@@ -83,13 +119,17 @@ function listFreshSessionConflicts(
83119
}
84120
for (const key of LOCKABLE_SELECTOR_KEYS) {
85121
const value = flags[key];
86-
if (typeof value === 'string' && value.trim().length > 0) {
122+
if (hasSelectorValue(value)) {
87123
conflicts.push({ key: key as SessionSelectorConflictKey, value });
88124
}
89125
}
90126
return conflicts;
91127
}
92128

129+
function hasSelectorValue(value: unknown): value is string {
130+
return typeof value === 'string' && value.trim().length > 0;
131+
}
132+
93133
function platformSelectorsConflict(
94134
requested: ReturnType<typeof normalizePlatformSelector>,
95135
locked: ReturnType<typeof normalizePlatformSelector>,

src/daemon/request-platform-providers.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,11 @@ async function resolveScopedProviderDevice(
270270
req: DaemonRequest,
271271
existingSession: SessionState | undefined,
272272
): Promise<DeviceInfo | undefined> {
273-
if (existingSession) return existingSession.device;
273+
if (existingSession) {
274+
return req.command === PUBLIC_COMMANDS.apps && hasExplicitDeviceSelector(req.flags)
275+
? await resolveTargetDevice(req.flags ?? {})
276+
: existingSession.device;
277+
}
274278
if (
275279
req.command !== PUBLIC_COMMANDS.open &&
276280
!hasExplicitDeviceSelector(req.flags) &&

0 commit comments

Comments
 (0)