diff --git a/src/daemon/__tests__/request-lock-policy.test.ts b/src/daemon/__tests__/request-lock-policy.test.ts index d6b54505d..49ac826f5 100644 --- a/src/daemon/__tests__/request-lock-policy.test.ts +++ b/src/daemon/__tests__/request-lock-policy.test.ts @@ -32,24 +32,25 @@ const ANDROID_SESSION: SessionState = { }, }; -test('rejects fresh-session selector conflicts under request lock policy', () => { - assert.throws( - () => - applyRequestLockPolicy({ - token: 'token', - session: 'qa-ios', - command: 'snapshot', - positionals: [], - flags: { - device: 'Pixel 9', - }, - meta: { - lockPolicy: 'reject', - lockPlatform: 'ios', - }, - }), - /--device=Pixel 9/i, - ); +test('allows compatible fresh-session selectors under request lock policy', () => { + const req = applyRequestLockPolicy({ + token: 'token', + session: 'qa-ios', + command: 'snapshot', + positionals: [], + flags: { + device: 'iPhone 16', + udid: 'SIM-001', + }, + meta: { + lockPolicy: 'reject', + lockPlatform: 'ios', + }, + }); + + assert.equal(req.flags?.platform, 'ios'); + assert.equal(req.flags?.device, 'iPhone 16'); + assert.equal(req.flags?.udid, 'SIM-001'); }); test('allows open to choose a fresh-session target under request lock policy', () => { @@ -74,7 +75,7 @@ test('allows open to choose a fresh-session target under request lock policy', ( assert.equal(req.flags?.udid, 'SIM-001'); }); -test('strips fresh-session selector conflicts and restores lock platform', () => { +test('strips only fresh-session selector conflicts and restores lock platform', () => { const req = applyRequestLockPolicy({ token: 'token', session: 'qa-ios', @@ -92,10 +93,32 @@ test('strips fresh-session selector conflicts and restores lock platform', () => }); assert.equal(req.flags?.platform, 'ios'); - assert.equal(req.flags?.target, undefined); + assert.equal(req.flags?.target, 'tv'); assert.equal(req.flags?.serial, undefined); }); +test('strips iOS selectors while preserving compatible macOS platform under Apple lock', () => { + const req = applyRequestLockPolicy({ + token: 'token', + session: 'qa-macos', + command: 'snapshot', + positionals: [], + flags: { + platform: 'macos', + udid: 'SIM-001', + iosSimulatorDeviceSet: '/tmp/tenant-a/set', + }, + meta: { + lockPolicy: 'strip', + lockPlatform: 'apple', + }, + }); + + assert.equal(req.flags?.platform, 'macos'); + assert.equal(req.flags?.udid, undefined); + assert.equal(req.flags?.iosSimulatorDeviceSet, undefined); +}); + test('rejects existing-session selector conflicts under request lock policy', () => { assert.throws( () => diff --git a/src/daemon/__tests__/request-router-lock-policy.test.ts b/src/daemon/__tests__/request-router-lock-policy.test.ts index 3070fb1fb..5477da9b0 100644 --- a/src/daemon/__tests__/request-router-lock-policy.test.ts +++ b/src/daemon/__tests__/request-router-lock-policy.test.ts @@ -7,6 +7,13 @@ vi.mock('../../core/dispatch.ts', async (importOriginal) => { return { ...actual, dispatchCommand: vi.fn(async () => ({})) }; }); +vi.mock('../../platforms/ios/runner-client.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, stopIosRunnerSession: vi.fn(async () => {}) }; +}); + +vi.mock('../device-ready.ts', () => ({ ensureDeviceReady: vi.fn(async () => {}) })); + import { dispatchCommand } from '../../core/dispatch.ts'; import { createRequestHandler } from '../request-router.ts'; import type { SessionState } from '../types.ts'; @@ -34,9 +41,42 @@ function makeIosSession(name: string): SessionState { beforeEach(() => { mockDispatch.mockReset(); - mockDispatch.mockResolvedValue({}); + mockDispatch.mockResolvedValue({ nodes: [] }); }); +function installGatedDispatch(): { + order: string[]; + getMaxActive: () => number; + releaseNext: () => void; +} { + const order: string[] = []; + const gates: Array<() => void> = []; + let active = 0; + let maxActive = 0; + + mockDispatch.mockImplementation(async (device, command) => { + order.push(`start-${command}-${device.id}`); + active += 1; + maxActive = Math.max(maxActive, active); + await new Promise((resolve) => { + gates.push(() => { + active -= 1; + order.push(`end-${command}-${device.id}`); + resolve(); + }); + }); + return { nodes: [] }; + }); + + return { + order, + getMaxActive: () => maxActive, + releaseNext: () => { + gates.shift()?.(); + }, + }; +} + test('direct daemon requests cannot bypass reject lock policy for existing sessions', async () => { const sessionStore = makeSessionStore('agent-device-router-lock-'); sessionStore.set('qa-ios', makeIosSession('qa-ios')); @@ -72,6 +112,290 @@ test('direct daemon requests cannot bypass reject lock policy for existing sessi } }); +test('fresh named sessions with matching explicit udid bind and serialize on the selected device', async () => { + const sessionStore = makeSessionStore('agent-device-router-lock-'); + const dispatchGate = installGatedDispatch(); + + const handler = createRequestHandler({ + logPath: path.join(os.tmpdir(), 'daemon.log'), + token: 'test-token', + sessionStore, + leaseRegistry: new LeaseRegistry(), + deviceInventoryProvider: async () => [makeIosSession('inventory').device], + trackDownloadableArtifact: () => 'artifact-id', + }); + + const first = handler({ + token: 'test-token', + session: 'qa-ios-a', + command: 'snapshot', + positionals: [], + flags: { + udid: 'SIM-001', + }, + meta: { + requestId: 'req-fresh-lock-a', + lockPolicy: 'reject', + lockPlatform: 'ios', + }, + }); + + await vi.waitFor(() => { + expect(dispatchGate.order).toEqual(['start-snapshot-SIM-001']); + }); + + const second = handler({ + token: 'test-token', + session: 'qa-ios-b', + command: 'snapshot', + positionals: [], + flags: { + udid: 'SIM-001', + }, + meta: { + requestId: 'req-fresh-lock-b', + lockPolicy: 'reject', + lockPlatform: 'ios', + }, + }); + + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(dispatchGate.order).toEqual(['start-snapshot-SIM-001']); + + dispatchGate.releaseNext(); + + await vi.waitFor(() => { + expect(dispatchGate.order).toEqual([ + 'start-snapshot-SIM-001', + 'end-snapshot-SIM-001', + 'start-snapshot-SIM-001', + ]); + }); + + dispatchGate.releaseNext(); + + const [firstResponse, secondResponse] = await Promise.all([first, second]); + + expect(firstResponse.ok).toBe(true); + expect(secondResponse.ok).toBe(true); + expect(dispatchGate.getMaxActive()).toBe(1); + expect(sessionStore.get('qa-ios-a')?.device.id).toBe('SIM-001'); + expect(sessionStore.get('qa-ios-b')?.device.id).toBe('SIM-001'); +}); + +test('fresh named sessions with the same name serialize first binding before rejecting another device', async () => { + const sessionStore = makeSessionStore('agent-device-router-lock-'); + const firstDevice = makeIosSession('inventory').device; + const secondDevice: SessionState['device'] = { + ...firstDevice, + id: 'SIM-002', + name: 'iPhone 17', + }; + const dispatchGate = installGatedDispatch(); + + const handler = createRequestHandler({ + logPath: path.join(os.tmpdir(), 'daemon.log'), + token: 'test-token', + sessionStore, + leaseRegistry: new LeaseRegistry(), + deviceInventoryProvider: async () => [firstDevice, secondDevice], + trackDownloadableArtifact: () => 'artifact-id', + }); + + const first = handler({ + token: 'test-token', + session: 'qa-ios', + command: 'snapshot', + positionals: [], + flags: { + udid: 'SIM-001', + }, + meta: { + requestId: 'req-fresh-same-session-a', + lockPolicy: 'reject', + lockPlatform: 'ios', + }, + }); + + await vi.waitFor(() => { + expect(dispatchGate.order).toEqual(['start-snapshot-SIM-001']); + }); + + const second = handler({ + token: 'test-token', + session: 'qa-ios', + command: 'snapshot', + positionals: [], + flags: { + udid: 'SIM-002', + }, + meta: { + requestId: 'req-fresh-same-session-b', + lockPolicy: 'reject', + lockPlatform: 'ios', + }, + }); + + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(dispatchGate.order).toEqual(['start-snapshot-SIM-001']); + + dispatchGate.releaseNext(); + + const [firstResponse, secondResponse] = await Promise.all([first, second]); + + expect(firstResponse.ok).toBe(true); + expect(secondResponse.ok).toBe(false); + if (!secondResponse.ok) { + expect(secondResponse.error.code).toBe('INVALID_ARGS'); + expect(secondResponse.error.message).toMatch(/--udid=SIM-002/i); + } + expect(dispatchGate.order).toEqual(['start-snapshot-SIM-001', 'end-snapshot-SIM-001']); + expect(dispatchGate.getMaxActive()).toBe(1); + expect(mockDispatch).toHaveBeenCalledTimes(1); + expect(sessionStore.get('qa-ios')?.device.id).toBe('SIM-001'); +}); + +test('fresh named sessions with only lock platform default serialize on the selected device', async () => { + const sessionStore = makeSessionStore('agent-device-router-lock-'); + const dispatchGate = installGatedDispatch(); + + const handler = createRequestHandler({ + logPath: path.join(os.tmpdir(), 'daemon.log'), + token: 'test-token', + sessionStore, + leaseRegistry: new LeaseRegistry(), + deviceInventoryProvider: async () => [makeIosSession('inventory').device], + trackDownloadableArtifact: () => 'artifact-id', + }); + + const first = handler({ + token: 'test-token', + session: 'qa-default-a', + command: 'snapshot', + positionals: [], + flags: {}, + meta: { + requestId: 'req-fresh-default-lock-a', + lockPolicy: 'reject', + lockPlatform: 'ios', + }, + }); + + await vi.waitFor(() => { + expect(dispatchGate.order).toEqual(['start-snapshot-SIM-001']); + }); + + const second = handler({ + token: 'test-token', + session: 'qa-default-b', + command: 'snapshot', + positionals: [], + flags: {}, + meta: { + requestId: 'req-fresh-default-lock-b', + lockPolicy: 'reject', + lockPlatform: 'ios', + }, + }); + + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(dispatchGate.order).toEqual(['start-snapshot-SIM-001']); + + dispatchGate.releaseNext(); + + await vi.waitFor(() => { + expect(dispatchGate.order).toEqual([ + 'start-snapshot-SIM-001', + 'end-snapshot-SIM-001', + 'start-snapshot-SIM-001', + ]); + }); + + dispatchGate.releaseNext(); + + const [firstResponse, secondResponse] = await Promise.all([first, second]); + + expect(firstResponse.ok).toBe(true); + expect(secondResponse.ok).toBe(true); + expect(dispatchGate.getMaxActive()).toBe(1); + expect(sessionStore.get('qa-default-a')?.device.id).toBe('SIM-001'); + expect(sessionStore.get('qa-default-b')?.device.id).toBe('SIM-001'); +}); + +test('fresh named sessions reject incompatible selector combinations before binding', async () => { + const cases = [ + { + name: 'ios-serial', + flags: { serial: 'emulator-5554' }, + meta: { lockPolicy: 'reject', lockPlatform: 'ios' }, + conflict: /--serial=emulator-5554/i, + }, + { + name: 'ios-android-platform', + flags: { platform: 'android', udid: 'SIM-001' }, + meta: { lockPolicy: 'reject', lockPlatform: 'ios' }, + conflict: /--platform=android/i, + }, + { + name: 'ios-desktop-target', + flags: { target: 'desktop' }, + meta: { lockPolicy: 'reject', lockPlatform: 'ios' }, + conflict: /--target=desktop/i, + }, + { + name: 'macos-udid', + flags: { udid: 'SIM-001', iosSimulatorDeviceSet: '/tmp/tenant-a/set' }, + meta: { lockPolicy: 'reject', lockPlatform: 'macos' }, + conflict: /--udid=SIM-001/i, + }, + { + name: 'apple-macos-udid', + flags: { platform: 'macos', udid: 'SIM-001' }, + meta: { lockPolicy: 'reject', lockPlatform: 'apple' }, + conflict: /--udid=SIM-001/i, + }, + { + name: 'apple-macos-simulator-set', + flags: { platform: 'macos', iosSimulatorDeviceSet: '/tmp/tenant-a/set' }, + meta: { lockPolicy: 'reject', lockPlatform: 'apple' }, + conflict: /--ios-simulator-device-set=\/tmp\/tenant-a\/set/i, + }, + ] as const; + + for (const testCase of cases) { + const sessionStore = makeSessionStore('agent-device-router-lock-'); + const handler = createRequestHandler({ + logPath: path.join(os.tmpdir(), 'daemon.log'), + token: 'test-token', + sessionStore, + leaseRegistry: new LeaseRegistry(), + deviceInventoryProvider: async () => [makeIosSession('inventory').device], + trackDownloadableArtifact: () => 'artifact-id', + }); + + const response = await handler({ + token: 'test-token', + session: testCase.name, + command: 'snapshot', + positionals: [], + flags: testCase.flags, + meta: { + requestId: `req-${testCase.name}`, + ...testCase.meta, + }, + }); + + expect(response.ok).toBe(false); + if (!response.ok) { + expect(response.error.code).toBe('INVALID_ARGS'); + expect(response.error.message).toMatch(testCase.conflict); + } + expect(mockDispatch).not.toHaveBeenCalled(); + expect(sessionStore.get(testCase.name)).toBeUndefined(); + mockDispatch.mockClear(); + } +}); + test('batch steps cannot bypass reject lock policy on nested direct requests', async () => { const sessionStore = makeSessionStore('agent-device-router-lock-'); sessionStore.set('qa-ios', makeIosSession('qa-ios')); diff --git a/src/daemon/device-selector-intent.ts b/src/daemon/device-selector-intent.ts new file mode 100644 index 000000000..ca46ac11e --- /dev/null +++ b/src/daemon/device-selector-intent.ts @@ -0,0 +1,35 @@ +import type { CommandFlags } from '../core/dispatch.ts'; + +const EXPLICIT_DEVICE_SELECTOR_KEYS: ReadonlyArray = [ + 'platform', + 'target', + 'device', + 'udid', + 'serial', +]; + +const LOCKABLE_DEVICE_SELECTOR_KEYS: ReadonlyArray = [ + ...EXPLICIT_DEVICE_SELECTOR_KEYS, + 'iosSimulatorDeviceSet', + 'androidDeviceAllowlist', +]; + +export function hasExplicitDeviceSelector(flags: CommandFlags | undefined): boolean { + return hasAnySelectorValue(flags, EXPLICIT_DEVICE_SELECTOR_KEYS); +} + +export function hasLockableDeviceSelector(flags: CommandFlags | undefined): boolean { + return hasAnySelectorValue(flags, LOCKABLE_DEVICE_SELECTOR_KEYS); +} + +export function hasSelectorValue(value: unknown): value is string { + return typeof value === 'string' && value.trim().length > 0; +} + +function hasAnySelectorValue( + flags: CommandFlags | undefined, + keys: ReadonlyArray, +): boolean { + if (!flags) return false; + return keys.some((key) => hasSelectorValue(flags[key])); +} diff --git a/src/daemon/handlers/__tests__/session-device-utils.test.ts b/src/daemon/handlers/__tests__/session-device-utils.test.ts index 9d9e9a9ba..3a043d5e0 100644 --- a/src/daemon/handlers/__tests__/session-device-utils.test.ts +++ b/src/daemon/handlers/__tests__/session-device-utils.test.ts @@ -1,7 +1,10 @@ import { test, expect } from 'vitest'; import type { SessionState } from '../../types.ts'; -import { refreshSessionDeviceIfNeeded } from '../session-device-utils.ts'; +import { + refreshSessionDeviceIfNeeded, + selectorTargetsSessionDevice, +} from '../session-device-utils.ts'; const iosSimulatorSession: SessionState = { name: 'ios-sim', @@ -33,3 +36,20 @@ test('refreshSessionDeviceIfNeeded keeps iOS simulator session device on non-mac expect(device).toBe(iosSimulatorSession.device); }); + +test('selectorTargetsSessionDevice uses session selector conflicts for simulator set selectors', () => { + const session: SessionState = { + ...iosSimulatorSession, + device: { + ...iosSimulatorSession.device, + simulatorSetPath: '/tmp/session-set', + }, + }; + + expect(selectorTargetsSessionDevice({ iosSimulatorDeviceSet: '/tmp/session-set' }, session)).toBe( + true, + ); + expect(selectorTargetsSessionDevice({ iosSimulatorDeviceSet: '/tmp/other-set' }, session)).toBe( + false, + ); +}); diff --git a/src/daemon/handlers/session-device-utils.ts b/src/daemon/handlers/session-device-utils.ts index 21f5c719c..96cd6abc1 100644 --- a/src/daemon/handlers/session-device-utils.ts +++ b/src/daemon/handlers/session-device-utils.ts @@ -1,12 +1,10 @@ -import { - matchesPlatformSelector, - normalizePlatformSelector, - type DeviceInfo, -} from '../../utils/device.ts'; +import type { DeviceInfo } from '../../utils/device.ts'; import { AppError } from '../../utils/errors.ts'; import { ensureDeviceReady } from '../device-ready.ts'; import { resolveTargetDevice } from '../../core/dispatch.ts'; import type { DaemonRequest, DaemonResponse, SessionState } from '../types.ts'; +import { hasExplicitDeviceSelector } from '../device-selector-intent.ts'; +import { listSessionSelectorConflicts } from '../session-selector.ts'; import { errorResponse } from './response.ts'; export const IOS_SIMULATOR_POST_CLOSE_SETTLE_MS = 300; @@ -27,10 +25,6 @@ export function requireSessionOrExplicitSelector( ); } -export function hasExplicitDeviceSelector(flags: DaemonRequest['flags'] | undefined): boolean { - return Boolean(flags?.platform || flags?.target || flags?.device || flags?.udid || flags?.serial); -} - export function hasExplicitSessionFlag(flags: DaemonRequest['flags'] | undefined): boolean { return typeof flags?.session === 'string' && flags.session.trim().length > 0; } @@ -115,16 +109,5 @@ export function selectorTargetsSessionDevice( session: SessionState | undefined, ): boolean { if (!session) return false; - if (!hasExplicitDeviceSelector(flags)) return true; - const normalizedPlatform = normalizePlatformSelector(flags?.platform); - if (normalizedPlatform && !matchesPlatformSelector(session.device.platform, normalizedPlatform)) { - return false; - } - if (flags?.target && flags.target !== (session.device.target ?? 'mobile')) return false; - if (flags?.udid && flags.udid !== session.device.id) return false; - if (flags?.serial && flags.serial !== session.device.id) return false; - if (flags?.device) { - return flags.device.trim().toLowerCase() === session.device.name.trim().toLowerCase(); - } - return true; + return listSessionSelectorConflicts(session, flags).length === 0; } diff --git a/src/daemon/request-admission.ts b/src/daemon/request-admission.ts index 0750e819f..6b3932f50 100644 --- a/src/daemon/request-admission.ts +++ b/src/daemon/request-admission.ts @@ -1,11 +1,8 @@ import { DAEMON_COMMAND_GROUPS } from '../command-catalog.ts'; -import { resolveTargetDevice } from '../core/dispatch-resolve.ts'; import { AppError } from '../utils/errors.ts'; import { normalizeTenantId, resolveSessionIsolationMode } from './config.ts'; -import { hasExplicitDeviceSelector } from './handlers/session-device-utils.ts'; import { resolveLeaseScope } from './lease-context.ts'; import type { LeaseRegistry } from './lease-registry.ts'; -import { SessionStore } from './session-store.ts'; import type { DaemonRequest } from './types.ts'; const selectorValidationExemptCommands = DAEMON_COMMAND_GROUPS.selectorValidationExempt; @@ -79,24 +76,3 @@ export function shouldValidateSessionSelector(command: string): boolean { export function shouldLockSessionExecution(command: string): boolean { return !sessionExecutionExemptCommands.has(command); } - -export async function resolveExecutionLockKey(params: { - req: DaemonRequest; - sessionName: string; - sessionStore: SessionStore; -}): Promise { - const { req, sessionName, sessionStore } = params; - const existingSession = sessionStore.get(sessionName); - if (existingSession) { - return `device:${existingSession.device.id}`; - } - if (req.command === 'open' || hasExplicitDeviceSelector(req.flags)) { - try { - const device = await resolveTargetDevice(req.flags ?? {}); - return `device:${device.id}`; - } catch { - // Fall back to session scoping when device resolution is not yet available. - } - } - return `session:${sessionName}`; -} diff --git a/src/daemon/request-binding.ts b/src/daemon/request-binding.ts new file mode 100644 index 000000000..fad4b83f6 --- /dev/null +++ b/src/daemon/request-binding.ts @@ -0,0 +1,87 @@ +import { resolveTargetDevice } from '../core/dispatch-resolve.ts'; +import { hasExplicitDeviceSelector } from './device-selector-intent.ts'; +import { applyRequestLockPolicy } from './request-lock-policy.ts'; +import type { SessionStore } from './session-store.ts'; +import type { DaemonRequest, SessionState } from './types.ts'; + +export type RequestExecutionLockKey = `session:${string}` | `device:${string}`; + +export type LockedRequestBinding = { + req: DaemonRequest; + existingSession: SessionState | undefined; +}; + +export async function resolveRequestExecutionLockKeys(params: { + req: DaemonRequest; + sessionName: string; + sessionStore: SessionStore; +}): Promise { + const { req, sessionName, sessionStore } = params; + const existingSession = sessionStore.get(sessionName); + if (existingSession) { + return [deviceExecutionLockKey(existingSession.device.id)]; + } + + const keys = new Set([sessionExecutionLockKey(sessionName)]); + const bindingReq = resolveFreshSessionBindingRequest(req); + if (shouldResolveFreshSessionDeviceLock(bindingReq)) { + try { + // This is advisory lock selection before the request enters the lock; the + // locked request still resolves and binds the target device authoritatively. + const device = await resolveTargetDevice(bindingReq.flags ?? {}); + keys.add(deviceExecutionLockKey(device.id)); + } catch { + // Fall back to session scoping when device resolution is not yet available. + } + } + return orderRequestExecutionLockKeys(keys); +} + +export function prepareLockedRequestBinding(params: { + req: DaemonRequest; + sessionName: string; + sessionStore: SessionStore; +}): LockedRequestBinding { + const existingSession = params.sessionStore.get(params.sessionName); + return { + req: applyRequestLockPolicy(params.req, existingSession), + existingSession, + }; +} + +function resolveFreshSessionBindingRequest(req: DaemonRequest): DaemonRequest { + if (!req.meta?.lockPolicy) return req; + try { + return applyRequestLockPolicy(req); + } catch { + // The request will be rejected during locked binding preparation. Keep lock + // selection best-effort so invalid selectors do not block unrelated work. + return req; + } +} + +function shouldResolveFreshSessionDeviceLock(req: DaemonRequest): boolean { + return req.command === 'open' || hasExplicitDeviceSelector(req.flags); +} + +function sessionExecutionLockKey(sessionName: string): RequestExecutionLockKey { + return `session:${sessionName}`; +} + +function deviceExecutionLockKey(deviceId: string): RequestExecutionLockKey { + return `device:${deviceId}`; +} + +function orderRequestExecutionLockKeys( + keys: Iterable, +): RequestExecutionLockKey[] { + return Array.from(keys).sort((left, right) => { + const categoryOrder = lockKeyCategoryOrder(left) - lockKeyCategoryOrder(right); + if (categoryOrder !== 0) return categoryOrder; + return left.localeCompare(right); + }); +} + +function lockKeyCategoryOrder(key: RequestExecutionLockKey): number { + return key.startsWith('session:') ? 0 : 1; +} diff --git a/src/daemon/request-execution-scope.ts b/src/daemon/request-execution-scope.ts index f0fdad2b9..fb9abd14e 100644 --- a/src/daemon/request-execution-scope.ts +++ b/src/daemon/request-execution-scope.ts @@ -5,15 +5,18 @@ import { applyCommandDefaults } from '../utils/command-schema.ts'; import type { DaemonCommandContext } from './context.ts'; import { contextFromFlags as contextFromFlagsWithLog } from './context.ts'; import { assertSessionSelectorMatches } from './session-selector.ts'; -import { applyRequestLockPolicy } from './request-lock-policy.ts'; import { resolveEffectiveSessionName } from './session-routing.ts'; import { assertRequestLeaseAdmission, - resolveExecutionLockKey, scopeRequestSession, shouldLockSessionExecution, shouldValidateSessionSelector, } from './request-admission.ts'; +import { + prepareLockedRequestBinding, + resolveRequestExecutionLockKeys, + type RequestExecutionLockKey, +} from './request-binding.ts'; import { throwIfRequestCanceled } from './request-cancel.ts'; import { finalizeDaemonResponse } from './request-finalization.ts'; import { @@ -79,9 +82,9 @@ export async function createRequestExecutionScope(params: { assertRequestLeaseAdmission(scopedReq, leaseRegistry); const sessionName = resolveEffectiveSessionName(scopedReq, sessionStore); - const executionLockKey = shouldLockSessionExecution(command) - ? await resolveExecutionLockKey({ req: scopedReq, sessionName, sessionStore }) - : null; + const executionLockKeys = shouldLockSessionExecution(command) + ? await resolveRequestExecutionLockKeys({ req: scopedReq, sessionName, sessionStore }) + : []; const executionLocks = getLeaseRegistryExecutionLocks(leaseRegistry); const scope: RequestExecutionScope = { @@ -91,8 +94,8 @@ export async function createRequestExecutionScope(params: { throwIfCanceled: () => throwIfRequestCanceled(scopedReq.meta?.requestId), runLocked: async (task) => { throwIfRequestCanceled(scopedReq.meta?.requestId); - if (!executionLockKey) return await task(); - return await withKeyedLock(executionLocks, executionLockKey, async () => { + if (executionLockKeys.length === 0) return await task(); + return await withRequestExecutionLocks(executionLocks, executionLockKeys, async () => { throwIfRequestCanceled(scopedReq.meta?.requestId); return await task(); }); @@ -101,6 +104,20 @@ export async function createRequestExecutionScope(params: { return scope; } +async function withRequestExecutionLocks( + locks: Map>, + keys: RequestExecutionLockKey[], + task: () => Promise, +): Promise { + const [key, ...remainingKeys] = keys; + if (!key) return await task(); + return await withKeyedLock( + locks, + key, + async () => await withRequestExecutionLocks(locks, remainingKeys, task), + ); +} + function applyRequestCommandDefaults(req: DaemonRequest): DaemonRequest { const flags = { ...(req.flags ?? {}) }; const changed = applyCommandDefaults(req.command, flags); @@ -124,13 +141,19 @@ export function prepareLockedRequestScope(params: { }): LockedRequestScopeResult { const { scope, logPath, sessionStore, trackDownloadableArtifact } = params; scope.throwIfCanceled(); - const existingSession = sessionStore.get(scope.sessionName); + let existingSession = sessionStore.get(scope.sessionName); if (existingSession) { // Called under runLocked: refreshRecordingHealth may mutate session recording state. refreshRecordingHealth(existingSession); sessionStore.set(scope.sessionName, existingSession); } - const lockedReq = applyRequestLockPolicy(scope.req, existingSession); + const binding = prepareLockedRequestBinding({ + req: scope.req, + sessionName: scope.sessionName, + sessionStore, + }); + const lockedReq = binding.req; + existingSession = binding.existingSession; const finalize = (response: DaemonResponse): DaemonResponse => finalizeDaemonResponse(lockedReq, response, trackDownloadableArtifact); diff --git a/src/daemon/request-lock-policy.ts b/src/daemon/request-lock-policy.ts index 7aaa8867b..05fccfb9c 100644 --- a/src/daemon/request-lock-policy.ts +++ b/src/daemon/request-lock-policy.ts @@ -11,17 +11,10 @@ import { import { isApplePlatform, normalizePlatformSelector } from '../utils/device.ts'; import { buildSessionRecoveryHint, describeSessionDevice } from './session-recovery-hints.ts'; import { shellQuoteIfNeeded } from '../utils/shell-quote.ts'; +import { hasLockableDeviceSelector, hasSelectorValue } from './device-selector-intent.ts'; type LockPlatform = NonNullable['lockPlatform']; - -const LOCKABLE_SELECTOR_KEYS: Array = [ - 'target', - 'device', - 'udid', - 'serial', - 'iosSimulatorDeviceSet', - 'androidDeviceAllowlist', -]; +type NormalizedLockPlatform = NonNullable>; const SELECTOR_OVERRIDE_LOCK_POLICY_COMMANDS: ReadonlySet = new Set([ PUBLIC_COMMANDS.apps, @@ -43,7 +36,7 @@ export function applyRequestLockPolicy( ? [] : existingSession ? listSessionSelectorConflicts(existingSession, nextFlags) - : listFreshSessionConflicts(nextFlags, req.meta?.lockPlatform, req.command); + : listFreshSessionConflicts(nextFlags, req.meta?.lockPlatform); const lockPlatform = req.meta?.lockPlatform; if (conflicts.length === 0) { @@ -125,7 +118,7 @@ function shouldApplyLockPlatformDefault( if (!canOverrideSelector) { return true; } - return !LOCKABLE_SELECTOR_KEYS.some((key) => hasSelectorValue(flags[key])); + return !hasLockableDeviceSelector(flags); } function applyStripLockPolicy( @@ -139,14 +132,19 @@ function applyStripLockPolicy( flags.platform = existingSession.device.platform; return; } - stripFreshSessionConflicts(flags, lockPlatform); + stripSessionConflicts(flags, conflicts); + if (lockPlatform && flags.platform === undefined) { + flags.platform = lockPlatform; + } } function listFreshSessionConflicts( flags: CommandFlags, lockPlatform: LockPlatform, - command: DaemonRequest['command'], ): SessionSelectorConflict[] { + // Fresh sessions are not device-bound yet. Reject only selectors that cannot + // participate in the first binding for the locked platform; concrete device + // existence and identity remain the target resolver's job. const conflicts: SessionSelectorConflict[] = []; const normalizedLockPlatform = normalizePlatformSelector(lockPlatform); if ( @@ -156,22 +154,14 @@ function listFreshSessionConflicts( ) { conflicts.push({ key: 'platform', value: flags.platform }); } - if (command === 'open') { + if (!normalizedLockPlatform) { return conflicts; } - for (const key of LOCKABLE_SELECTOR_KEYS) { - const value = flags[key]; - if (hasSelectorValue(value)) { - conflicts.push({ key: key as SessionSelectorConflictKey, value }); - } - } + appendFreshSessionTargetConflict(conflicts, flags, normalizedLockPlatform); + appendFreshSessionDeviceSelectorConflicts(conflicts, flags, normalizedLockPlatform); return conflicts; } -function hasSelectorValue(value: unknown): value is string { - return typeof value === 'string' && value.trim().length > 0; -} - function platformSelectorsConflict( requested: ReturnType, locked: ReturnType, @@ -183,17 +173,81 @@ function platformSelectorsConflict( return true; } -function stripFreshSessionConflicts(flags: CommandFlags, lockPlatform: LockPlatform): void { - for (const key of LOCKABLE_SELECTOR_KEYS) { - delete flags[key]; +function appendFreshSessionTargetConflict( + conflicts: SessionSelectorConflict[], + flags: CommandFlags, + lockPlatform: NormalizedLockPlatform, +): void { + const target = flags.target; + if (!target) return; + if (targetSelectorsConflict(target, lockPlatform)) { + conflicts.push({ key: 'target', value: target }); } - if (lockPlatform) { - flags.platform = lockPlatform; +} + +function targetSelectorsConflict( + target: NonNullable, + lockPlatform: NormalizedLockPlatform, +): boolean { + switch (lockPlatform) { + case 'android': + case 'ios': + return target === 'desktop'; + case 'macos': + case 'linux': + return target !== 'desktop'; + case 'apple': + return false; + default: + return assertNever(lockPlatform); } } +function appendFreshSessionDeviceSelectorConflicts( + conflicts: SessionSelectorConflict[], + flags: CommandFlags, + lockPlatform: NormalizedLockPlatform, +): void { + for (const key of freshSessionSelectorKeysForPlatform(lockPlatform, flags)) { + const value = flags[key]; + if (hasSelectorValue(value)) { + conflicts.push({ key, value }); + } + } +} + +function freshSessionSelectorKeysForPlatform( + lockPlatform: NormalizedLockPlatform, + flags: CommandFlags, +): SessionSelectorConflictKey[] { + switch (lockPlatform) { + case 'android': + return ['udid', 'iosSimulatorDeviceSet']; + case 'ios': + return ['serial', 'androidDeviceAllowlist']; + case 'apple': + return isAppleDesktopSelector(flags) + ? ['udid', 'serial', 'androidDeviceAllowlist', 'iosSimulatorDeviceSet'] + : ['serial', 'androidDeviceAllowlist']; + case 'macos': + return ['udid', 'serial', 'iosSimulatorDeviceSet', 'androidDeviceAllowlist']; + case 'linux': + return ['udid', 'serial', 'iosSimulatorDeviceSet', 'androidDeviceAllowlist']; + default: + return assertNever(lockPlatform); + } +} + +function isAppleDesktopSelector(flags: CommandFlags): boolean { + return flags.target === 'desktop' || normalizePlatformSelector(flags.platform) === 'macos'; +} + function stripSessionConflicts(flags: CommandFlags, conflicts: SessionSelectorConflict[]): void { for (const conflict of conflicts) { delete flags[conflict.key]; } } + +function assertNever(value: never): never { + throw new Error(`Unhandled lock platform: ${String(value)}`); +} diff --git a/src/daemon/request-platform-providers.ts b/src/daemon/request-platform-providers.ts index 8a3247c1e..8b71d684b 100644 --- a/src/daemon/request-platform-providers.ts +++ b/src/daemon/request-platform-providers.ts @@ -11,7 +11,7 @@ import type { import type { LinuxToolProvider } from '../platforms/linux/tool-provider.ts'; import { isApplePlatform, type DeviceInfo } from '../utils/device.ts'; import type { AppLogProvider } from './app-log.ts'; -import { hasExplicitDeviceSelector } from './handlers/session-device-utils.ts'; +import { hasExplicitDeviceSelector } from './device-selector-intent.ts'; import type { RecordingProvider } from './recording-provider.ts'; import type { DaemonRequest, SessionState } from './types.ts'; import { PUBLIC_COMMANDS } from '../command-catalog.ts';