fix: allow fresh session device binding

thymikee · thymikee · commit 26f3886ae86f · 2026-06-02T17:13:22.000-07:00
diff --git 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,7 +93,7 @@ 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);
 });
 
diff --git 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<typeof import('../../platforms/ios/runner-client.ts')>();
+  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,7 +41,7 @@ function makeIosSession(name: string): SessionState {
 
 beforeEach(() => {
   mockDispatch.mockReset();
-  mockDispatch.mockResolvedValue({});
+  mockDispatch.mockResolvedValue({ nodes: [] });
 });
 
 test('direct daemon requests cannot bypass reject lock policy for existing sessions', async () => {
@@ -72,6 +79,240 @@ 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 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<void>((resolve) => {
+      gates.push(() => {
+        active -= 1;
+        order.push(`end-${command}-${device.id}`);
+        resolve();
+      });
+    });
+    return { nodes: [] };
+  });
+
+  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(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(order).toEqual(['start-snapshot-SIM-001']);
+
+  gates.shift()?.();
+
+  await vi.waitFor(() => {
+    expect(order).toEqual([
+      'start-snapshot-SIM-001',
+      'end-snapshot-SIM-001',
+      'start-snapshot-SIM-001',
+    ]);
+  });
+
+  gates.shift()?.();
+
+  const [firstResponse, secondResponse] = await Promise.all([first, second]);
+
+  expect(firstResponse.ok).toBe(true);
+  expect(secondResponse.ok).toBe(true);
+  expect(maxActive).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 only lock platform default serialize on the selected device', async () => {
+  const sessionStore = makeSessionStore('agent-device-router-lock-');
+  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<void>((resolve) => {
+      gates.push(() => {
+        active -= 1;
+        order.push(`end-${command}-${device.id}`);
+        resolve();
+      });
+    });
+    return { nodes: [] };
+  });
+
+  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(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(order).toEqual(['start-snapshot-SIM-001']);
+
+  gates.shift()?.();
+
+  await vi.waitFor(() => {
+    expect(order).toEqual([
+      'start-snapshot-SIM-001',
+      'end-snapshot-SIM-001',
+      'start-snapshot-SIM-001',
+    ]);
+  });
+
+  gates.shift()?.();
+
+  const [firstResponse, secondResponse] = await Promise.all([first, second]);
+
+  expect(firstResponse.ok).toBe(true);
+  expect(secondResponse.ok).toBe(true);
+  expect(maxActive).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,
+    },
+  ] 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/request-admission.ts b/src/daemon/request-admission.ts
@@ -5,6 +5,7 @@ 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 { applyRequestLockPolicy } from './request-lock-policy.ts';
 import { SessionStore } from './session-store.ts';
 import type { DaemonRequest } from './types.ts';
 
@@ -90,13 +91,25 @@ export async function resolveExecutionLockKey(params: {
   if (existingSession) {
     return `device:${existingSession.device.id}`;
   }
-  if (req.command === 'open' || hasExplicitDeviceSelector(req.flags)) {
+  const lockReq = resolveFreshSessionLockRequest(req);
+  if (lockReq.command === 'open' || hasExplicitDeviceSelector(lockReq.flags)) {
     try {
-      const device = await resolveTargetDevice(req.flags ?? {});
+      const device = await resolveTargetDevice(lockReq.flags ?? {});
       return `device:${device.id}`;
     } catch {
       // Fall back to session scoping when device resolution is not yet available.
     }
   }
   return `session:${sessionName}`;
 }
+
+function resolveFreshSessionLockRequest(req: DaemonRequest): DaemonRequest {
+  if (!req.meta?.lockPolicy) return req;
+  try {
+    return applyRequestLockPolicy(req);
+  } catch {
+    // The request will be rejected during locked scope preparation. Keep lock
+    // selection best-effort so invalid selectors do not block unrelated work.
+    return req;
+  }
+}
diff --git a/src/daemon/request-lock-policy.ts b/src/daemon/request-lock-policy.ts
