fix: clarify proxy runner ownership (#882)

* fix: clarify proxy runner ownership

* fix: tighten proxy runner review follow-ups

Author: thymikee

src/commands/management/prepare.ts

@@ -32,7 +32,7 @@ const prepareCliSchema = {
   usageOverride: 'prepare ios-runner --platform ios|macos [--timeout <ms>]',
   listUsageOverride: 'prepare',
   helpDescription:
-    'Prepare platform helper infrastructure. ios-runner builds/reuses, starts, and health-checks the XCTest runner so later Apple snapshots and interactions do not pay first-use startup cost. In CI, run it after boot/install and before replay/test; if replay/test starts a separate daemon, run clean:daemon after prepare to release the prepared runner lease. Runner build/start output is written to the session runner.log; daemon.log is for daemon lifecycle/startup issues.',
+    'Prepare platform helper infrastructure. ios-runner builds/reuses, starts, and health-checks the XCTest runner so later Apple snapshots and interactions do not pay first-use startup cost. In CI, run it after boot/install and before replay/test; if replay/test starts a separate daemon, run clean:daemon after prepare to release the prepared runner lease. It is not a recovery step for "runner already owned by another agent-device daemon"; stop or clean the owning daemon on the Mac with simulator access instead. Runner build/start output is written to the session runner.log; daemon.log is for daemon lifecycle/startup issues.',
   summary:
     'Pre-warm platform helpers, especially the iOS/macOS XCTest runner before Apple automation',
   positionalArgs: ['ios-runner'],

src/daemon-client-lifecycle.ts

@@ -32,7 +32,12 @@ import {
   type DaemonInfo,
   type DaemonStartupCleanupResult,
 } from './daemon-client-metadata.ts';
-import { canConnect, readRemoteDaemonHealth } from './daemon-client-transport.ts';
+import {
+  canConnect,
+  DAEMON_HTTP_ENDPOINT_UNAVAILABLE_MESSAGE,
+  DAEMON_SOCKET_ENDPOINT_UNAVAILABLE_MESSAGE,
+  readRemoteDaemonHealth,
+} from './daemon-client-transport.ts';
 
 export type DaemonClientSettings = {
   paths: DaemonPaths;
@@ -166,7 +171,7 @@ async function readReusableLocalDaemon(settings: DaemonClientSettings): Promise<
   const existing = readDaemonInfo(settings.paths.infoPath);
   if (!existing) return null;
 
-  const existingReachable = await canConnect(existing, settings.transportPreference);
+  const existingReachable = await canConnectReusableDaemon(existing, settings.transportPreference);
   if (isReusableDaemonInfo(existing, existingReachable)) return existing;
 
   emitDaemonTakeoverNotice(existing, existingReachable, settings.paths.baseDir);
@@ -175,6 +180,27 @@ async function readReusableLocalDaemon(settings: DaemonClientSettings): Promise<
   return null;
 }
 
+async function canConnectReusableDaemon(
+  info: DaemonInfo,
+  preference: DaemonTransportPreference,
+): Promise<boolean> {
+  try {
+    return await canConnect(info, preference);
+  } catch (error) {
+    if (isDaemonTransportUnavailableError(error)) return false;
+    throw error;
+  }
+}
+
+function isDaemonTransportUnavailableError(error: unknown): boolean {
+  return (
+    error instanceof AppError &&
+    error.code === 'COMMAND_FAILED' &&
+    (error.message === DAEMON_HTTP_ENDPOINT_UNAVAILABLE_MESSAGE ||
+      error.message === DAEMON_SOCKET_ENDPOINT_UNAVAILABLE_MESSAGE)
+  );
+}
+
 function isReusableDaemonInfo(info: DaemonInfo, reachable: boolean): boolean {
   return (
     info.version === readVersion() &&

src/daemon-client-transport.ts

@@ -22,6 +22,8 @@ type ResolvedDaemonTransport = 'socket' | 'http';
 
 const LOCAL_DAEMON_HEALTHCHECK_TIMEOUT_MS = 500;
 const REMOTE_DAEMON_HEALTHCHECK_TIMEOUT_MS = 3000;
+export const DAEMON_HTTP_ENDPOINT_UNAVAILABLE_MESSAGE = 'Daemon HTTP endpoint is unavailable';
+export const DAEMON_SOCKET_ENDPOINT_UNAVAILABLE_MESSAGE = 'Daemon socket endpoint is unavailable';
 
 export type RemoteDaemonHealth = {
   reachable: boolean;
@@ -254,8 +256,8 @@ function requireDaemonTransport(
   throw new AppError(
     'COMMAND_FAILED',
     transport === 'http'
-      ? 'Daemon HTTP endpoint is unavailable'
-      : 'Daemon socket endpoint is unavailable',
+      ? DAEMON_HTTP_ENDPOINT_UNAVAILABLE_MESSAGE
+      : DAEMON_SOCKET_ENDPOINT_UNAVAILABLE_MESSAGE,
   );
 }
 
@@ -294,7 +296,7 @@ async function sendSocketRequest(
   timeoutMs: number | undefined,
 ): Promise<DaemonResponse> {
   const port = info.port;
-  if (!port) throw new AppError('COMMAND_FAILED', 'Daemon socket endpoint is unavailable');
+  if (!port) throw new AppError('COMMAND_FAILED', DAEMON_SOCKET_ENDPOINT_UNAVAILABLE_MESSAGE);
   return new Promise((resolve, reject) => {
     let requestWritten = false;
     const socket = net.createConnection({ host: '127.0.0.1', port }, () => {
@@ -360,7 +362,7 @@ async function sendHttpRequest(
     : info.httpPort
       ? new URL(`http://127.0.0.1:${info.httpPort}/rpc`)
       : null;
-  if (!rpcUrl) throw new AppError('COMMAND_FAILED', 'Daemon HTTP endpoint is unavailable');
+  if (!rpcUrl) throw new AppError('COMMAND_FAILED', DAEMON_HTTP_ENDPOINT_UNAVAILABLE_MESSAGE);
   const rpcPayload = JSON.stringify(buildHttpRpcPayload(req, { includeTokenParam: !info.baseUrl }));
   const headers: Record<string, string | number> = {
     'content-type': 'application/json',

src/platforms/ios/__tests__/runner-session.test.ts

@@ -662,25 +662,80 @@ test('runner session startup kills legacy ownerless xcodebuild before launching
 
 test('runner session startup rejects live foreign runner lease', async () => {
   const device = { ...IOS_SIMULATOR, id: 'runner-session-busy-lease-sim' };
+  const previousStateDir = process.env.AGENT_DEVICE_STATE_DIR;
+  process.env.AGENT_DEVICE_STATE_DIR = '/tmp/agent-device-current';
   writeRunnerLease(
     makeRunnerLease({
       deviceId: device.id,
       ownerToken: 'owner-foreign-live',
       ownerPid: process.pid,
       ownerStartTime: RUNNER_OWNER_START_TIME,
+      ownerStateDir: '/tmp/agent-device-owner',
     }),
   );
 
-  await assert.rejects(
-    () => ensureRunnerSession(device, {}),
-    /already owned by another agent-device daemon/,
-  );
+  try {
+    let thrown: unknown;
+    await assert.rejects(async () => {
+      try {
+        await ensureRunnerSession(device, {});
+      } catch (error) {
+        thrown = error;
+        throw error;
+      }
+    }, /already owned by another agent-device daemon/);
+
+    assert.equal(
+      (thrown as { details?: Record<string, unknown> }).details?.ownerStateDir,
+      '/tmp/agent-device-owner',
+    );
+    assert.match(
+      String((thrown as { details?: Record<string, unknown> }).details?.hint),
+      /Do not run prepare ios-runner/,
+    );
+    assert.equal(mockRunCmdBackground.mock.calls.length, 0);
+    assert.equal(
+      mockRunAppleToolCommand.mock.calls.some((call) => call[0] === 'pkill'),
+      false,
+    );
+  } finally {
+    if (previousStateDir === undefined) delete process.env.AGENT_DEVICE_STATE_DIR;
+    else process.env.AGENT_DEVICE_STATE_DIR = previousStateDir;
+  }
+});
 
-  assert.equal(mockRunCmdBackground.mock.calls.length, 0);
-  assert.equal(
-    mockRunAppleToolCommand.mock.calls.some((call) => call[0] === 'pkill'),
-    false,
+test('runner session startup reclaims live foreign runner lease from same state dir', async () => {
+  const device = { ...IOS_SIMULATOR, id: 'runner-session-same-state-lease-sim' };
+  const previousStateDir = process.env.AGENT_DEVICE_STATE_DIR;
+  const stateDir = '/tmp/agent-device-proxy-state';
+  process.env.AGENT_DEVICE_STATE_DIR = stateDir;
+  writeRunnerLease(
+    makeRunnerLease({
+      deviceId: device.id,
+      ownerToken: 'owner-foreign-same-state',
+      ownerPid: process.pid,
+      ownerStartTime: RUNNER_OWNER_START_TIME,
+      ownerStateDir: stateDir,
+      runnerPid: 4_321,
+    }),
   );
+
+  try {
+    const session = await ensureRunnerSession(device, {});
+
+    assert.equal(session.deviceId, device.id);
+    assert.equal(mockRunCmdBackground.mock.calls.length, 1);
+    assert.deepEqual(mockCleanupTempFile.mock.calls, [
+      [`/tmp/AgentDeviceRunner.env.session-${device.id}-owner-foreign-same-state-8123.xctestrun`],
+      [`/tmp/AgentDeviceRunner.env.session-${device.id}-owner-foreign-same-state-8123.json`],
+    ]);
+    const pkillCalls = mockRunAppleToolCommand.mock.calls.filter(isXcodebuildPkillCall);
+    assert.ok(pkillCalls.length >= 2);
+    assert.match(String(pkillCalls[0]?.[1]?.[2] ?? ''), /owner-foreign-same-state/);
+  } finally {
+    if (previousStateDir === undefined) delete process.env.AGENT_DEVICE_STATE_DIR;
+    else process.env.AGENT_DEVICE_STATE_DIR = previousStateDir;
+  }
 });
 
 test('runner session startup reclaims dead foreign runner lease before launching', async () => {

src/platforms/ios/runner-lease.ts

@@ -22,6 +22,7 @@ export type RunnerLease = {
   ownerToken: string;
   ownerPid: number;
   ownerStartTime: string | null;
+  ownerStateDir?: string;
   sessionId: string;
   runnerPid: number | null;
   port: number;
@@ -61,6 +62,7 @@ export function buildRunnerLease(params: {
     ownerToken: RUNNER_OWNER_TOKEN,
     ownerPid: RUNNER_OWNER_PID,
     ownerStartTime: RUNNER_OWNER_START_TIME,
+    ownerStateDir: readCurrentStateDir(),
     sessionId: params.sessionId,
     runnerPid: params.runnerPid ?? null,
     port: params.port,
@@ -115,22 +117,47 @@ export async function prepareRunnerLeaseForStartup(
     return;
   }
   if (state.type === 'busy') {
+    if (isSameStateDirRunnerLease(state.lease)) {
+      await cleanupLeasedRunnerProcesses(state.lease, 'same-state-dir', cleanup);
+      return;
+    }
     throw new AppError(
       'COMMAND_FAILED',
       `iOS runner for ${deviceId} is already owned by another agent-device daemon`,
       {
         deviceId,
         ownerPid: state.lease.ownerPid,
         ownerStartTime: state.lease.ownerStartTime,
+        ownerStateDir: state.lease.ownerStateDir,
         ownerToken: state.lease.ownerToken,
         sessionId: state.lease.sessionId,
-        hint: 'Use a different simulator/session, wait for the other run to finish, or stop the owning daemon before retrying.',
+        hint: buildBusyRunnerLeaseHint(state.lease),
       },
     );
   }
   await cleanupLeasedRunnerProcesses(state.lease, state.type, cleanup);
 }
 
+function isSameStateDirRunnerLease(lease: RunnerLease): boolean {
+  // Same-state reclaim assumes callers sharing AGENT_DEVICE_STATE_DIR are the same logical daemon owner.
+  const currentStateDir = readCurrentStateDir();
+  if (!currentStateDir || !lease.ownerStateDir) return false;
+  return path.resolve(currentStateDir) === path.resolve(lease.ownerStateDir);
+}
+
+function readCurrentStateDir(): string | undefined {
+  return process.env.AGENT_DEVICE_STATE_DIR?.trim() || undefined;
+}
+
+function buildBusyRunnerLeaseHint(lease: RunnerLease): string {
+  const owner = `PID ${lease.ownerPid}`;
+  const stateDir = lease.ownerStateDir ? ` with AGENT_DEVICE_STATE_DIR=${lease.ownerStateDir}` : '';
+  return [
+    `The Mac operator must stop the owning daemon (${owner}${stateDir}) or wait for that run to finish, then retry.`,
+    'Do not run prepare ios-runner from another daemon/client to recover this; a live foreign runner lease cannot be released by the remote client.',
+  ].join(' ');
+}
+
 export async function cleanupOwnedRunnerLease(
   deviceId: string,
   cleanup: RunnerLeaseCleanupAdapter,
@@ -240,6 +267,7 @@ function normalizeRunnerLease(value: unknown, deviceId: string): RunnerLease | n
     deviceId,
     ...fields,
     ownerStartTime: readOptionalString(raw.ownerStartTime),
+    ownerStateDir: readOptionalString(raw.ownerStateDir) ?? undefined,
     runnerPid: readPositiveInteger(raw.runnerPid),
   };
 }
@@ -286,15 +314,16 @@ function isRunnerLeaseOwnerAlive(lease: RunnerLease): boolean {
 
 async function cleanupLeasedRunnerProcesses(
   lease: RunnerLease,
-  reason: 'owned' | 'stale',
+  reason: 'owned' | 'stale' | 'same-state-dir',
   cleanup: RunnerLeaseCleanupAdapter,
 ): Promise<void> {
   emitDiagnostic({
-    level: reason === 'stale' ? 'warn' : 'debug',
+    level: reason === 'stale' || reason === 'same-state-dir' ? 'warn' : 'debug',
     phase: 'ios_runner_lease_cleanup',
     data: {
       deviceId: lease.deviceId,
       ownerPid: lease.ownerPid,
+      ownerStateDir: lease.ownerStateDir,
       ownerToken: lease.ownerToken,
       runnerPid: lease.runnerPid,
       sessionId: lease.sessionId,

src/utils/__tests__/args.test.ts

@@ -1096,6 +1096,10 @@ test('usage includes only global flags in the top-level global flags section', (
 
 test('usage includes agent workflows, config, environment, and examples footers', () => {
   const usageText = usage();
+  assert.ok(
+    usageText.indexOf('Agent Workflows:') < usageText.indexOf('Commands:'),
+    'Agent workflows should appear before the command list for agents that only read the top of help.',
+  );
   assert.match(usageText, /Agent Quickstart:/);
   assert.match(usageText, /Default loop: devices\/apps -> open -> snapshot -i/);
   assert.match(usageText, /Use selectors or refs as positional targets/);
@@ -1122,7 +1126,12 @@ test('usage includes agent workflows, config, environment, and examples footers'
   assert.match(usageText, /verify the action with diff snapshot -i or snapshot --diff/);
   assert.match(usageText, /Sparse or AX-unavailable snapshot/);
   assert.match(usageText, /macOS context menus use click <ref> --button secondary/);
-  assert.match(usageText, /Remote workflow profiles use --remote-config/);
+  assert.match(usageText, /Direct proxy: Cloud\/Linux clients can use iOS simulators/);
+  assert.match(usageText, /A proxy URL\/token means direct proxy mode/);
+  assert.match(usageText, /Direct proxy sessions: choose one explicit --session/);
+  assert.match(usageText, /do not use connect, --remote-config, tenant, run, or lease flags/);
+  assert.match(usageText, /Cloud\/remote-config profiles are separate from direct proxy/);
+  assert.match(usageText, /Do not substitute --config/);
   assert.match(usageText, /app-owned back uses back/);
   assert.match(usageText, /Web browser sessions: read help web/);
   assert.match(
@@ -1230,6 +1239,10 @@ test('usageForCommand documents prepare ios-runner', () => {
   assert.match(help, /XCTest runner/);
   assert.match(help, /separate daemon/);
   assert.match(help, /clean:daemon after prepare/);
+  assert.match(
+    help,
+    /not a recovery step for "runner already owned by another agent-device daemon"/,
+  );
   assert.match(help, /Runner build\/start output is written to the session runner\.log/);
 });
 
@@ -1321,6 +1334,10 @@ test('usageForCommand resolves workflow help topic', () => {
   assert.match(help, /metro prepare --kind expo/);
   assert.match(help, /agent-device prepare ios-runner --platform ios --timeout 240000/);
   assert.match(help, /prepare ios-runner builds\/reuses the XCTest runner/);
+  assert.match(
+    help,
+    /not a recovery step for "runner already owned by another agent-device daemon"/,
+  );
   assert.match(help, /prepared runner does not keep a live lease/);
   assert.match(help, /help react-devtools/);
   assert.match(help, /help react-native/);
@@ -1443,7 +1460,11 @@ test('usageForCommand resolves remote help topic', () => {
   const help = usageForCommand('remote');
   if (help === null) throw new Error('Expected remote help text');
   assert.match(help, /agent-device connect/);
-  assert.match(help, /without --remote-config/);
+  assert.match(help, /There are two different remote modes/);
+  assert.match(help, /Direct proxy: agent-device proxy exposes a Mac you control/);
+  assert.match(help, /A cloud\/Linux client can use iOS simulators through that proxied Mac/);
+  assert.match(help, /Use one explicit --session across open, snapshot, interactions, and close/);
+  assert.match(help, /Do not use connect, --remote-config, tenant, run, or lease flags/);
   assert.match(help, /agent-device open com\.example\.app --remote-config \.\/remote-config\.json/);
   assert.match(help, /disconnect --remote-config \.\/remote-config\.json/);
   assert.match(help, /Script flow, per-command config/);
@@ -1453,11 +1474,15 @@ test('usageForCommand resolves remote help topic', () => {
     help,
     /--daemon-base-url https:\/\/example\.trycloudflare\.com\/agent-device --daemon-auth-token <token>/,
   );
+  assert.match(help, /agent-device open Maps --session maps/);
+  assert.match(help, /agent-device snapshot -i --session maps/);
+  assert.match(help, /agent-device close --session maps/);
   assert.match(help, /store daemonBaseUrl and daemonAuthToken in normal agent-device\.json/);
-  assert.match(help, /Keep platform selection on each command or workflow/);
+  assert.match(help, /keep the same explicit --session until close/);
+  assert.match(help, /do not run prepare ios-runner from the remote client/);
+  assert.match(help, /same-proxy-state stale runner leases are reclaimed/);
   assert.match(help, /same --remote-config to every operational command/);
-  assert.match(help, /do not use agent-device auth for this direct proxy flow/);
-  assert.match(help, /Do not use --remote-config unless you are using the tenant\/run\/lease/);
+  assert.match(help, /do not use agent-device auth, connect, disconnect, --remote-config/);
   assert.match(help, /Do not use --config as a remote profile flag/);
   assert.match(help, /install-from-source --github-actions-artifact org\/repo:artifact/);
 });