diff --git a/packages/@expo/agent-cli/CHANGELOG.md b/packages/@expo/agent-cli/CHANGELOG.md index 5b48b28c..27ccf0ba 100644 --- a/packages/@expo/agent-cli/CHANGELOG.md +++ b/packages/@expo/agent-cli/CHANGELOG.md @@ -12,6 +12,7 @@ ### 🎉 New features - `navigate` and `smoke` now suppress onboarding, the floating Tools button, and automatic dev-menu opening when launching a development build. Set `EXPO_NO_DEV_MENU=0` to opt out. +- `dev --device ` binds a simulator or an Android device the user names. The CLI never shuts it down or deletes it, refuses a device bound to another live worktree, refuses a serial `adb` lists as `offline` or `unauthorized` with `explicit-not-ready` (exit 7) and names what readies it, and `dev --plan` says it will refuse. The plan's `explicitDevice` says the same on `--json` and on the `cli:start_plan` event. A run that binds no local device (`--web`, `AGENT_CLI_NO_DEVICE=1`, iOS off macOS, no Android SDK) refuses `--device` with `BAD_ARGS`. ([#TBD](https://github.com/expo/expo-agent-cli/pull/TBD) by [@vonovak](https://github.com/vonovak)) - `dev`, `smoke` and `dev:stop` reap the devices of other worktrees: a deleted worktree's created simulator is deleted, an expired one is shut down on the next iOS acquisition, and a spawned emulator instance whose worktree is gone or whose lease expired is killed, or reported `device-gone` when its pid has exited or runs another program; `dev:stop --json` lists them under `reaped`. A delete or kill that fails keeps the binding, so the next reap tries again, and `dev:stop` exits 20 with a How line naming the command that does it by hand. ([#TBD](https://github.com/expo/expo-agent-cli/pull/TBD) by [@vonovak](https://github.com/vonovak)) - `smoke --ios` and `smoke --android` release the device they bound when the run ends: a child dev server's device through `dev:stop --release`, a device bound next to a reused dev server through the boot phase's cleanup. `smoke --no-start` binds and releases nothing. The report names that device, how the run got it, and what its release did: a `device` line, and `environment.boundDevice` in `--json`. A run that binds nothing and uses the worktree's bound device names it as `already bound · kept` (action `existing`). When no app attached and the run put its dev server and device back, the follow-ups start them again with `dev -- --detach --wait-ready` before `navigate`, and a repeated `smoke` keeps `--port`. ([#TBD](https://github.com/expo/expo-agent-cli/pull/TBD) by [@vonovak](https://github.com/vonovak)) - `dev:stop --release` parks the worktree's created simulator and ends its spawned emulator instance once this worktree's dev server is stopped; plain `dev:stop` keeps them and reports them under `devices` with `released: null`; `false` means a release failed. ([#TBD](https://github.com/expo/expo-agent-cli/pull/TBD) by [@vonovak](https://github.com/vonovak)) diff --git a/packages/@expo/agent-cli/e2e-live/README.md b/packages/@expo/agent-cli/e2e-live/README.md new file mode 100644 index 00000000..3a41d277 --- /dev/null +++ b/packages/@expo/agent-cli/e2e-live/README.md @@ -0,0 +1,19 @@ +# Device bindings and two worktrees + +Each worktree has its own simulator or emulator binding. Local `dev --ios` runs use +`--device "$AGENT_CLI_LIVE_UDID"` when that variable is set; otherwise `dev` creates a +simulator for the worktree. Explicit devices are never shut down or deleted by cleanup. +The live runner uses `dev:stop --release` when cleaning up its servers. + +To exercise isolation manually, open two checkouts and run the following in each, using +8082 for the second checkout: + +```sh +npx expo-agent-cli dev --ios --detach --wait-ready --port 8081 +``` + +In the first checkout, run +`npx expo-agent-cli dev:stop`, then start it again: it should reuse its simulator. Run +`npx expo-agent-cli dev:stop --release --json` in each checkout to park the simulators +(or end spawned Android instances). For explicit selection, assign a different simulator +UDID to each worktree; naming the other worktree's device must refuse. diff --git a/packages/@expo/agent-cli/e2e-live/utils.ts b/packages/@expo/agent-cli/e2e-live/utils.ts index 4a5b5004..ab5e3878 100644 --- a/packages/@expo/agent-cli/e2e-live/utils.ts +++ b/packages/@expo/agent-cli/e2e-live/utils.ts @@ -233,6 +233,16 @@ export async function runLiveAsync( argv: string[], { env, label }: LiveOptions = {} ): Promise { + const udid = env?.AGENT_CLI_LIVE_UDID ?? process.env.AGENT_CLI_LIVE_UDID; + if ( + argv[0] === 'dev' && + argv.includes('--ios') && + !argv.includes('--eas') && + udid && + !argv.some((arg) => arg === '--device' || arg.startsWith('--device=')) + ) { + argv = [...argv, '--device', udid]; + } if (argv[0] === 'dev:stop' && !argv.includes('--release')) argv = [...argv, '--release']; run.spend.commands += 1; const startedAt = Date.now(); diff --git a/packages/@expo/agent-cli/e2e/__tests__/device-lifecycle-test.ts b/packages/@expo/agent-cli/e2e/__tests__/device-lifecycle-test.ts index 0a242af8..86086c71 100644 --- a/packages/@expo/agent-cli/e2e/__tests__/device-lifecycle-test.ts +++ b/packages/@expo/agent-cli/e2e/__tests__/device-lifecycle-test.ts @@ -107,6 +107,28 @@ describe('device lifecycle across two worktrees', () => { .sort() ).toEqual([first.runDevice, second.runDevice].sort()); + const plan = await executeAgentCliAsync( + projects[1]!, + ['dev', '--ios', '--local', '--plan', '--device', first.runDevice!, '--json'], + { env: envFor(projects[1]!) } + ); + expect(plan.exitCode).toBe(0); + expect(JSON.parse(plan.stdout)).toMatchObject({ + explicitDevice: { + refuses: true, + summary: expect.stringContaining(`is bound to the worktree ${first.projectRoot}`), + how: 'npx @expo/agent-cli dev --ios --local --json --detach --wait-ready', + }, + followups: expect.arrayContaining([ + expect.objectContaining({ + id: 'device-refusal', + command: 'npx @expo/agent-cli dev --ios --local --json --detach --wait-ready', + }), + ]), + }); + expect(JSON.parse(plan.stdout).followups.map(({ id }: { id: string }) => id)).not.toContain( + 'dev' + ); expect(bindings()).toHaveLength(2); const kept = await executeAgentCliAsync(projects[0]!, ['dev:stop', '--json'], { diff --git a/packages/@expo/agent-cli/e2e/__tests__/plan-test.ts b/packages/@expo/agent-cli/e2e/__tests__/plan-test.ts index 40b8d133..c1b27bee 100644 --- a/packages/@expo/agent-cli/e2e/__tests__/plan-test.ts +++ b/packages/@expo/agent-cli/e2e/__tests__/plan-test.ts @@ -459,7 +459,7 @@ describe('@expo/agent-cli dev --plan', () => { }); expect(plan.reasons.join('\n')).not.toContain('does not take that route on its own'); expect(plan.followups.map((followup) => followup.command)).toContain( - 'npx @expo/agent-cli dev --ios' + 'npx @expo/agent-cli dev --ios --json --detach --wait-ready' ); }); diff --git a/packages/@expo/agent-cli/llp/0035-one-device-per-worktree.rfc.md b/packages/@expo/agent-cli/llp/0035-one-device-per-worktree.rfc.md index dc6f1a11..b853772e 100644 --- a/packages/@expo/agent-cli/llp/0035-one-device-per-worktree.rfc.md +++ b/packages/@expo/agent-cli/llp/0035-one-device-per-worktree.rfc.md @@ -202,17 +202,17 @@ export type ReleasedDevice = { ## Choice (`choose.ts`, pure) -Inputs: the own binding and the other worktrees' bindings, each marked stale or not; the inventory, read before the lock; `explicit`; `reuseOnly`; `now`; on Android `busyPorts`, computed by `android.ts` in the same section: the running `emulator-NNNN` serials, the Android bindings whose lease is live, and every Android binding whose `emulatorPid` is alive, because a reap kills it only after the section. The free port is the first free even one in 5554..5584. `IosInventory` holds `SimulatorEntry[]` and `newestIosRuntime: { identifier, deviceType } | null`: the newest available iOS runtime that lists a mainstream iPhone, with its highest-numbered `iPhone Pro`, else `iPhone `, else `iPhone Pro Max`, whatever order `simctl` lists them in, because the newest runtime can list only a device no app renders on (iOS 27.1 lists iPhone Duo alone); only when no runtime has one does the newest runtime's first iPhone type apply. `AndroidInventory` holds the first AVD of `emulator -list-avds` and `runningSerials`. Outputs: +Inputs: the own binding and the other worktrees' bindings, each marked stale or not; the inventory, read before the lock; `explicit`; `reuseOnly`; `now`; on Android `busyPorts`, computed by `android.ts` in the same section: the running `emulator-NNNN` serials, the Android bindings whose lease is live, and every Android binding whose `emulatorPid` is alive, because a reap kills it only after the section. The free port is the first free even one in 5554..5584. `IosInventory` holds `SimulatorEntry[]` and `newestIosRuntime: { identifier, deviceType } | null`: the newest available iOS runtime that lists a mainstream iPhone, with its highest-numbered `iPhone Pro`, else `iPhone `, else `iPhone Pro Max`, whatever order `simctl` lists them in, because the newest runtime can list only a device no app renders on (iOS 27.1 lists iPhone Duo alone); only when no runtime has one does the newest runtime's first iPhone type apply. `AndroidInventory` holds the first AVD of `emulator -list-avds`, `runningSerials`, and the listed serials that are not ready, with their state. With `--device`, Android reads only `adb devices -l`: a USB device needs no emulator package. Outputs: ```ts | { kind: 'reuse'; binding } // own binding, live or expired, device present (iOS: listed and available; Android spawned: the instance check; Android explicit: serial listed); also --device naming the own device | { kind: 'ios-create'; runtime; deviceType; name } | { kind: 'android-spawn'; port; avd } | { kind: 'explicit'; device } -| { kind: 'refuse'; reason: 'no-ios-runtime' | 'no-avd' | 'no-free-port' | 'explicit-not-found' | 'explicit-bound' | 'own-explicit-over-owned' | 'not-reusable'; boundBy?: { id: string; root: string }[]; matches?: string[] } +| { kind: 'refuse'; reason: 'no-ios-runtime' | 'no-avd' | 'no-free-port' | 'explicit-not-found' | 'explicit-not-ready' | 'explicit-bound' | 'own-explicit-over-owned' | 'not-reusable'; boundBy?: { id: string; root: string }[]; matches?: string[] } ``` -- `--device` matches a simulator by name or udid, case-insensitive, and an emulator by serial only, because read-only instances share one AVD name. Two or more matches are `explicit-not-found` with `matches`. A device that another worktree's `created` or `spawned` binding names, live or stale, is `explicit-bound`, and so is one its live `explicit` binding names; an expired `explicit` binding of another worktree is removed in the section and the device is bound here, because a named device belongs to whoever names it now. A simulator named `agent-cli …` that no binding names is `explicit-not-found`, so a leaked create is never bound. `--device` naming another device while the own binding is `spawned` is `own-explicit-over-owned`, because one file per worktree and platform cannot hold both and a replaced instance would be orphaned. While the own binding is `created`, the own simulator's file is removed in the same section and the simulator is deleted with `simctl delete` after it, the `explicit` binding is written, and the delete is reported on stderr: the user named the device, so the replacement is what they asked for, and a parked binding would refuse forever. An own `explicit` binding is replaced. +- `--device` matches a simulator by name or udid, case-insensitive, and an emulator by serial only, because read-only instances share one AVD name. Two or more matches are `explicit-not-found` with `matches`. A serial `adb` lists in a state other than `device`, such as `offline` or `unauthorized`, that no candidate matches is `explicit-not-ready`, not `explicit-not-found`. A device that another worktree's `created` or `spawned` binding names, live or stale, is `explicit-bound`, and so is one its live `explicit` binding names; an expired `explicit` binding of another worktree is removed in the section and the device is bound here, because a named device belongs to whoever names it now. A simulator named `agent-cli …` that no binding names is `explicit-not-found`, so a leaked create is never bound. `--device` naming another device while the own binding is `spawned` and the instance check holds is `own-explicit-over-owned`, because one file per worktree and platform cannot hold both and a replaced instance would be orphaned; an own `spawned` binding that fails the instance check counts as gone, and the `explicit` binding replaces its file. While the own binding is `created`, the own simulator's file is removed in the same section and the simulator is deleted with `simctl delete` after it, the `explicit` binding is written, and the delete is reported on stderr: the user named the device, so the replacement is what they asked for, and a parked binding would refuse forever. An own `explicit` binding is replaced. - With `reuseOnly`, any choice but `reuse` is `not-reusable`. - An own binding whose device is gone is removed under the lock and allocation goes on. `no-free-port` carries the serials and the worktrees they are bound to in `boundBy`. There is no cloud variant: the cloud path writes a binding and chooses nothing. @@ -291,8 +291,9 @@ Each reaped device is reported on stderr and in JSON with reason `expired`, `del | `DEVICE_UNAVAILABLE` | `no-ios-runtime` | 7 | "no iOS runtime with an iPhone is installed" | `xcodebuild -downloadPlatform iOS` | | `DEVICE_UNAVAILABLE` | `no-avd` | 7 | "no Android virtual device exists" | create one in Android Studio | | `DEVICE_UNAVAILABLE` | `no-free-port` | 20 | the serials and the worktrees they are bound to; a serial no binding names is listed as "not ours" | ` dev:stop --release` in a listed worktree, or stop the unlisted serial by hand | -| `DEVICE_UNAVAILABLE` | `explicit-bound` | 20 | " is bound to the worktree " | ` dev` without `--device` | -| `DEVICE_UNAVAILABLE` | `explicit-not-found` | 1 | "no device matches , or more than one does" with `matches` | ` status --json` | +| `DEVICE_UNAVAILABLE` | `explicit-bound` | 20 | " is bound to the worktree " | the caller's ` dev` without `--device` | +| `DEVICE_UNAVAILABLE` | `explicit-not-found` | 1 | "no device matches , or more than one does" with `matches` | the same command with `--device `; none: `simctl list`/`adb devices` | +| `DEVICE_UNAVAILABLE` | `explicit-not-ready` | 7 | "adb lists as " | allow USB debugging or reconnect, then the same command with `--device ` | | `DEVICE_UNAVAILABLE` | `own-explicit-over-owned` | 1 | "this worktree runs the emulator instance " | ` dev:stop --release` | | `DEVICE_UNAVAILABLE` | `not-reusable` | 20 | "a dev server is running and this worktree has no device to reuse" | ` dev:stop` | | `DEVICE_UNAVAILABLE` | `create-timeout` | 22 | "the device tool did not answer in time" | run the command again | @@ -558,8 +559,9 @@ What happens to the device of a deleted or expired worktree? How does a person pick a device? -- In `DEV_OWN_FLAGS`; its value is read as `resolvePort` reads `--port`, `--device=` included; the flag and value are stripped from the Expo args as `withoutPortArgs` strips the port; `detachArgv` carries it unchanged; with `--eas` it is `BAD_ARGS`, checked where `assertEasRunFits` runs. The match and the refusals are in §Choice; the origin's rule is Contract 7. `dev --plan` naming a device bound to another worktree carries "will refuse: is bound to ". +- In `DEV_OWN_FLAGS`; its value is read as `resolvePort` reads `--port`, `--device=` included; the flag and value are stripped from the Expo args as `withoutPortArgs` strips the port; `detachArgv` carries it unchanged, and a detached parent gives its child the caller's argv in `__EXPO_AGENT_CLI_CALLER_ARGV`, which `resolveDevOptions` reads once and keeps from the Expo CLI, so every How line that repeats the caller's `dev` command (`rerunDevCommand`, which adds `--detach --wait-ready` to a foreground command) repeats what the caller typed; with `--eas` it is `BAD_ARGS`, checked where `assertEasRunFits` runs, and so is a run that binds no local device (`--web`, `AGENT_CLI_NO_DEVICE`, a host that cannot run the platform's device), checked before the probe. The match and the refusals are in §Choice; the origin's rule is Contract 7. With `--device`, Android reads only `adb devices -l`: a USB device needs no emulator package. `dev --plan --device` prints a `Device:` line under the steps and the plan's `explicitDevice` (on `--json` and `cli:start_plan`) from the same choice, reuse-only while this worktree's dev server runs, read without writes or boots: the device it binds, the expired foreign binding it takes over and the own created simulator it deletes, or "Will refuse — ", whose How command replaces the `dev` follow-up while the plan still exits 0. Only a refusal of the choice is printed so; a device tool error, a timeout or an unreadable binding fails the plan as it fails the run. A plan whose EAS route nobody chose previews no device, because the run stops before it binds one. - Live tier: the live tier passes `--device $AGENT_CLI_LIVE_UDID` to `dev` or lets it create. The runbook in `e2e-live/README` gains the two-worktree setup. +- `explicit.ts` holds the `--device` request, its pure choice and the section that binds it (`readExplicitChoice`, `chooseExplicitDevice`, `bindExplicitSection`); `preview.ts` holds the `dev --plan --device` line (`previewExplicitDeviceAsync`). ### Lifecycle output diff --git a/packages/@expo/agent-cli/src/dev/__tests__/detachAsync-test.ts b/packages/@expo/agent-cli/src/dev/__tests__/detachAsync-test.ts index 4fdbdc10..53578a9a 100644 --- a/packages/@expo/agent-cli/src/dev/__tests__/detachAsync-test.ts +++ b/packages/@expo/agent-cli/src/dev/__tests__/detachAsync-test.ts @@ -20,6 +20,7 @@ import { fetchAdvertisedUrlAsync } from '../advertisedUrl'; import { devDetachAsync, OPEN_PLATFORM_GRACE_MS } from '../detachAsync'; import { detachedLogPath } from '../logFile'; import { isProcessAlive } from '../processLiveness'; +import { CALLER_ARGV_ENV } from '../deviceOption'; import { resolveDevOptions } from '../resolveOptions'; vi.mock('../../devLock', () => ({ readDevServerLockAsync: vi.fn() })); @@ -531,6 +532,8 @@ describe('a lock whose port Metro has not logged', () => { // @ref llp/0035-one-device-per-worktree.rfc.md §How `dev` uses it describe('the bound device', () => { + const platform = process.platform; + afterEach(() => Object.defineProperty(process, 'platform', { value: platform })); const running = { url: 'http://127.0.0.1:8393', port: 8393, @@ -720,6 +723,38 @@ describe('the bound device', () => { }); expect(error.message).toBe(refusal.message); }); + + it("gives the child's --device refusal the caller's flags, --detach and --wait-ready too", async () => { + vi.mocked(readDevServerLockAsync).mockReset().mockResolvedValue(null); + const argv = ['--ios', '--device', 'Sim', '--detach', '--wait-ready', '--port', '8092']; + let spawned: { args: readonly string[]; env: NodeJS.ProcessEnv } | undefined; + vi.mocked(spawn).mockImplementation((( + _command: string, + args: string[], + options: { env: NodeJS.ProcessEnv } + ) => { + spawned = { args, env: options.env }; + const child = Object.assign(new EventEmitter(), { pid: 4242, unref: vi.fn() }); + setTimeout(() => child.emit('exit', 1, null), 50); + return child as unknown as ChildProcess; + }) as typeof spawn); + + const result = devDetachAsync(projectRoot, resolveDevOptions(argv), { print: false }).catch( + () => null + ); + await vi.runAllTimersAsync(); + await result; + + const childArgv = spawned!.args.slice(spawned!.args.indexOf('dev') + 1); + vi.stubEnv('__EXPO_AGENT_CLI_DETACHED', spawned!.env.__EXPO_AGENT_CLI_DETACHED); + vi.stubEnv(CALLER_ARGV_ENV, spawned!.env[CALLER_ARGV_ENV]); + try { + expect(childArgv).not.toContain('--detach'); + expect(resolveDevOptions(childArgv).callerArgv).toEqual(argv); + } finally { + vi.unstubAllEnvs(); + } + }); }); /** The log a detached child writes for `error`, line by line, as `logCmdError` prints it. */ diff --git a/packages/@expo/agent-cli/src/dev/__tests__/devAsync-test.ts b/packages/@expo/agent-cli/src/dev/__tests__/devAsync-test.ts index 64fd1cff..f2766327 100644 --- a/packages/@expo/agent-cli/src/dev/__tests__/devAsync-test.ts +++ b/packages/@expo/agent-cli/src/dev/__tests__/devAsync-test.ts @@ -3,7 +3,12 @@ import os from 'os'; import path from 'path'; import { acquireDeviceAsync, devicesDisabled, inspectBindingAsync } from '../../deviceBinding'; +import { bindingFor, fakeTools, writeBinding } from '../../deviceBinding/__tests__/fakeTools'; +import { bindingPathFor, readBindingFile } from '../../deviceBinding/registry'; +import { deviceUnavailableError, notReusableError } from '../../deviceBinding/errors'; +import { previewExplicitDeviceAsync } from '../../deviceBinding/preview'; import { withLeaseExtendedAsync } from '../../deviceBinding/renew'; +import { readDevServerLockAsync } from '../../devLock/client'; import type { FollowUp } from '../../followups'; import { Log } from '../../log'; import { emitStartPlan } from '../../plan/emit'; @@ -18,6 +23,7 @@ import { isInteractive } from '../../utils/interactive'; import { devAsync } from '../devAsync'; import { findFreePortAsync, formatPortMove, resolvePlannedPortAsync } from '../portCollision'; import { findPortListenerAsync } from '../portListener'; +import { CALLER_ARGV_ENV } from '../deviceOption'; import { resolveDevOptions } from '../resolveOptions'; import { isExpoDevServerAsync } from '../stopAsync'; @@ -51,6 +57,11 @@ vi.mock('../../project/fingerprint', () => ({ clearFingerprintMemo: vi.fn() })); vi.mock('../../project/fingerprintCache', () => ({ clearFingerprintCache: vi.fn() })); vi.mock('../../utils/expoCli', () => ({ runExpoAsync: vi.fn(), spawnExpoAsync: vi.fn() })); vi.mock('../../start/startAsync', () => ({ runDevServerAsync: vi.fn() })); +vi.mock('../../deviceBinding/preview', () => ({ previewExplicitDeviceAsync: vi.fn() })); +vi.mock('../../devLock/client', async (importOriginal) => ({ + ...(await importOriginal()), + readDevServerLockAsync: vi.fn(async () => null), +})); vi.mock('../../deviceBinding/renew', () => ({ withLeaseExtendedAsync: vi.fn((_root: string, work: () => Promise) => work()), })); @@ -179,6 +190,7 @@ function mockLanAddress(address: string | null) { beforeEach(() => { vol.reset(); mockReported.length = 0; + vi.mocked(readDevServerLockAsync).mockReset().mockResolvedValue(null); vi.mocked(readLastBuildRecord).mockReturnValue({}); vi.mocked(runExpoAsync).mockResolvedValue(0); vi.mocked(isInteractive).mockReturnValue(true); @@ -1068,10 +1080,10 @@ describe(devAsync, () => { // The port the retry lost is the one probed for its holder. expect(findPortListenerAsync).toHaveBeenLastCalledWith(retryPort); expect(error.message).toMatch( - /How: start on a free port with ".* dev --ios --json --port \d+"\./ + /How: start on a free port with ".* dev --ios --json --port \d+ --detach --wait-ready"\./ ); expect(error.message).not.toContain('dev:stop'); - expect(error.suggestedCommand).toMatch(/ dev --ios --json --port \d+$/); + expect(error.suggestedCommand).toMatch(/ dev --ios --json --port \d+ --detach --wait-ready$/); expect(error.message).not.toContain("CLI's own"); expect(runDevServerAsync).toHaveBeenCalledTimes(2); expect(Log.log).not.toHaveBeenCalled(); @@ -1098,8 +1110,30 @@ describe(devAsync, () => { ); expect(error.suggestedCommand).toMatch( - / dev --ios --clear --tunnel --json --port \d+ -- --max-workers 2$/ + / dev --ios --clear --tunnel --json --port \d+ --detach --wait-ready -- --max-workers 2$/ + ); + }); + + it(`should suggest, from a detached child, the command its parent was given`, async () => { + mockProjectState(); + vi.mocked(runDevServerAsync).mockImplementation(async (_root, args) => { + const port = args.includes('--port') ? Number(args[args.indexOf('--port') + 1]) : 8081; + return devServerRun({ + exitCode: 1, + stderr: PORT_TAKEN.replace(/Port \d+/, `Port ${port}`), + }); + }); + vi.stubEnv('__EXPO_AGENT_CLI_DETACHED', '1'); + vi.stubEnv(CALLER_ARGV_ENV, JSON.stringify(['--ios', '--detach', '--wait-ready', '--json'])); + + const options = resolveDevOptions(['--ios', '--json']); + vi.unstubAllEnvs(); + const error = await devAsync(projectRoot, options).then( + () => null, + (thrown) => thrown ); + + expect(error.suggestedCommand).toMatch(/ dev --ios --detach --wait-ready --json --port \d+$/); }); // No free port to move to: `dev:stop --force` only when the holder answers as an Expo dev @@ -1422,6 +1456,7 @@ describe(devAsync, () => { }); afterEach(() => { + mockPlatform(realPlatform); vi.mocked(devicesDisabled).mockReturnValue(true); }); @@ -1443,6 +1478,37 @@ describe(devAsync, () => { } ); + it.each(['SIM-1', 'HUMAN'])( + 'protects the running server’s device when naming %s', + async (query) => { + mockProjectState(); + vol.fromJSON({ '/project/package.json': '{}' }); + vi.mocked(readDevServerLockAsync).mockResolvedValueOnce({ pid: 1 } as never); + const tools = fakeTools({ + simulators: [ + { udid: 'SIM-1', name: 'agent-cli own', state: 'Booted' }, + { udid: 'HUMAN', name: 'My iPhone', state: 'Shutdown' }, + ], + }); + const file = bindingPathFor(projectRoot, 'ios', 'local-ios'); + writeBinding(file, bindingFor(projectRoot, 'SIM-1')); + const real = + await vi.importActual('../../deviceBinding'); + vi.mocked(acquireDeviceAsync).mockImplementationOnce((root, platform, options) => + real.acquireDeviceAsync(root, platform, { ...options, tools }) + ); + + const run = devAsync(projectRoot, resolveDevOptions(['--ios', '--device', query])); + if (query === 'SIM-1') await expect(run).resolves.toBe(0); + else { + await expect(run).rejects.toMatchObject({ data: { reason: 'not-reusable' } }); + expect(runDevServerAsync).not.toHaveBeenCalled(); + } + expect(readBindingFile(file)).toMatchObject({ binding: { device: { udid: 'SIM-1' } } }); + expect(tools.calls).not.toContainEqual(['delete', 'SIM-1']); + } + ); + // withDevice-after-forwarded-args it(`binds after the refusals and pins the build step to the simulator, over a forwarded --device`, async () => { mockStaleDevClientState(); @@ -1451,7 +1517,10 @@ describe(devAsync, () => { devAsync(projectRoot, resolveDevOptions(['--ios', '--local', '--', '--device', 'X'])) ).resolves.toBe(0); - expect(acquireDeviceAsync).toHaveBeenCalledWith(projectRoot, 'ios'); + expect(acquireDeviceAsync).toHaveBeenCalledWith(projectRoot, 'ios', { + reuseOnly: false, + explicit: undefined, + }); const [, args] = vi.mocked(runDevServerAsync).mock.calls[0]!; expect(args.filter((arg) => arg === '--device')).toHaveLength(1); expect(args).toEqual(expect.arrayContaining(['run:ios', '--device', 'SIM-1'])); @@ -1506,6 +1575,28 @@ describe(devAsync, () => { ); }); + it.each([ + ['--web', ['--web', '--device', 'X'], () => {}], + [ + 'AGENT_CLI_NO_DEVICE=1', + ['--ios', '--device', 'X'], + () => vi.mocked(devicesDisabled).mockReturnValue(true), + ], + ['--ios off macOS', ['--ios', '--device', 'X'], () => mockPlatform('linux')], + ] as const)( + `refuses --device with BAD_ARGS when the run binds no local device: %s`, + async (_case, argv, arrange) => { + mockStaleDevClientState(); + arrange(); + + await expect(devAsync(projectRoot, resolveDevOptions([...argv]))).rejects.toMatchObject({ + code: 'BAD_ARGS', + }); + + expect(probeProjectStateAsync).not.toHaveBeenCalled(); + } + ); + describe('--plan', () => { it(`pins the steps to the own binding when it is up or down, and acquires nothing`, async () => { mockStaleDevClientState(); @@ -1552,6 +1643,98 @@ describe(devAsync, () => { expect.anything() ); }); + + it(`reads --device as the run does while this worktree's dev server runs: reuse only`, async () => { + mockStaleDevClientState(); + vi.mocked(readDevServerLockAsync).mockResolvedValueOnce({ pid: 1 } as never); + vi.mocked(previewExplicitDeviceAsync).mockRejectedValueOnce( + notReusableError('ios', { bound: 'agent-cli 0000', named: 'Sim' }) + ); + + await expect( + devAsync( + projectRoot, + resolveDevOptions(['--ios', '--local', '--plan', '--device', 'Sim', '--port', '8091']) + ) + ).resolves.toBe(0); + + expect(previewExplicitDeviceAsync).toHaveBeenCalledWith( + projectRoot, + 'ios', + { query: 'Sim', rerun: ['--ios', '--local', '--plan', '--port', '8091'] }, + true + ); + expect(emittedFollowUps()).toContainEqual( + expect.objectContaining({ id: 'device-refusal', command: 'npx @expo/agent-cli dev:stop' }) + ); + }); + + it(`offers a bind plan's run with the caller's --device and --port`, async () => { + mockStaleDevClientState(); + vi.mocked(readDevServerLockAsync).mockResolvedValueOnce(null); + vi.mocked(previewExplicitDeviceAsync).mockResolvedValueOnce({ + device: { ...device, name: 'My Sim', origin: 'explicit' }, + summary: 'Will bind My Sim (SIM-1).', + }); + + await devAsync( + projectRoot, + resolveDevOptions(['--ios', '--local', '--plan', '--device', 'My Sim', '--port', '8091']) + ); + + expect(previewExplicitDeviceAsync).toHaveBeenCalledWith( + projectRoot, + 'ios', + expect.anything(), + false + ); + expect(emittedFollowUps().find((followup) => followup.id === 'dev')!.command).toBe( + "npx @expo/agent-cli dev --ios --local --device 'My Sim' --port 8091 --detach --wait-ready" + ); + }); + + it(`fails --plan --device as the run does when the device tool times out`, async () => { + mockStaleDevClientState(); + vi.mocked(previewExplicitDeviceAsync).mockRejectedValueOnce( + deviceUnavailableError('create-timeout', { platform: 'ios' }) + ); + + await expect( + devAsync( + projectRoot, + resolveDevOptions(['--ios', '--local', '--plan', '--device', 'Sim']) + ) + ).rejects.toMatchObject({ data: { reason: 'create-timeout' }, exitCode: 22 }); + + expect(emitStartPlan).not.toHaveBeenCalled(); + }); + + it(`previews no --device for a plan whose EAS route nobody chose, and offers the --eas run without it`, async () => { + mockStaleDevClientState(); + const { detectToolchainAsync } = await import('../../toolchain'); + vi.mocked(detectToolchainAsync).mockResolvedValue({ + platform: 'ios', + status: 'missing', + detail: 'no Xcode', + requirement: 'Xcode', + caveats: [], + impossible: true, + }); + vi.mocked(previewExplicitDeviceAsync).mockResolvedValueOnce({ + device: { ...device, name: 'My Sim', origin: 'explicit' }, + summary: 'Will bind My Sim (SIM-1).', + }); + + await expect( + devAsync(projectRoot, resolveDevOptions(['--ios', '--plan', '--device', 'My Sim'])) + ).resolves.toBe(0); + + expect(previewExplicitDeviceAsync).not.toHaveBeenCalled(); + expect(vi.mocked(emitStartPlan).mock.calls[0]![0]).not.toHaveProperty('explicitDevice'); + expect(emittedFollowUps().find((followup) => followup.id === 'dev')!.command).toBe( + 'npx @expo/agent-cli dev --ios --eas --detach --wait-ready' + ); + }); }); }); }); diff --git a/packages/@expo/agent-cli/src/dev/__tests__/resolveOptions-test.ts b/packages/@expo/agent-cli/src/dev/__tests__/resolveOptions-test.ts index 2b4d1179..782c7a1f 100644 --- a/packages/@expo/agent-cli/src/dev/__tests__/resolveOptions-test.ts +++ b/packages/@expo/agent-cli/src/dev/__tests__/resolveOptions-test.ts @@ -1,4 +1,5 @@ import { DEFAULT_DETACH_TIMEOUT_MS } from '../detachAsync'; +import { CALLER_ARGV_ENV } from '../deviceOption'; import { resolveDevOptions } from '../resolveOptions'; describe(resolveDevOptions, () => { @@ -19,6 +20,7 @@ describe(resolveDevOptions, () => { waitReady: false, detachTimeoutMs: DEFAULT_DETACH_TIMEOUT_MS, detachArgv: expect.any(Array), + callerArgv: expect.any(Array), // Read and not forwarded: `dev` sets `--port` on every step that serves. port: 8082, }); @@ -41,6 +43,7 @@ describe(resolveDevOptions, () => { waitReady: false, detachTimeoutMs: DEFAULT_DETACH_TIMEOUT_MS, detachArgv: expect.any(Array), + callerArgv: expect.any(Array), port: null, }); }); @@ -66,6 +69,7 @@ describe(resolveDevOptions, () => { waitReady: false, detachTimeoutMs: DEFAULT_DETACH_TIMEOUT_MS, detachArgv: expect.any(Array), + callerArgv: expect.any(Array), port: 8082, }); }); @@ -204,6 +208,7 @@ describe(resolveDevOptions, () => { waitReady: false, detachTimeoutMs: DEFAULT_DETACH_TIMEOUT_MS, detachArgv: expect.any(Array), + callerArgv: expect.any(Array), port: null, }); }); @@ -229,6 +234,7 @@ describe(resolveDevOptions, () => { waitReady: false, detachTimeoutMs: DEFAULT_DETACH_TIMEOUT_MS, detachArgv: expect.any(Array), + callerArgv: expect.any(Array), port: null, }); }); @@ -334,3 +340,57 @@ describe('--eas puts the device on EAS', () => { expect(options.expoArgs).toEqual(['--tunnel']); }); }); + +describe('--device', () => { + it.each([['--device', 'My iPhone'], ['--device=My iPhone']])( + 'owns the selection and carries it into the detached child (%s)', + (...flag) => { + const argv = ['--ios', '--detach', ...flag, '--clear']; + const options = resolveDevOptions(argv); + expect(options.device).toBe('My iPhone'); + expect(options.expoArgs).toEqual(['--clear']); + expect(options.detachArgv).toEqual(argv); + } + ); + it('leaves arguments after the separator alone', () => { + expect(resolveDevOptions(['--ios', '--', '--device', 'elsewhere'])).toMatchObject({ + expoArgs: ['--', '--device', 'elsewhere'], + }); + }); + it.each([ + ['--ios', '--device'], + ['--ios', '--device='], + ['--ios', '--device', '--plan'], + ['--ios', '--device', 'SIM', '--eas'], + ])('refuses invalid selection flags (%s)', (...argv) => { + expect(() => resolveDevOptions(argv)).toThrow(); + }); +}); + +describe('the caller argv of a detached child', () => { + afterEach(() => vi.unstubAllEnvs()); + const child = ['--ios', '--device', 'Sim']; + const caller = ['--ios', '--device', 'Sim', '--detach', '--wait-ready']; + + it('is the parent argv the child was given, which no subprocess of the child inherits', () => { + vi.stubEnv('__EXPO_AGENT_CLI_DETACHED', '1'); + vi.stubEnv(CALLER_ARGV_ENV, JSON.stringify(caller)); + expect(resolveDevOptions(child).callerArgv).toEqual(caller); + expect(process.env[CALLER_ARGV_ENV]).toBeUndefined(); + }); + + it.each(['{', '"--ios"', '[1]'])( + "is the child's own argv when the variable is malformed (%s)", + (value) => { + vi.stubEnv('__EXPO_AGENT_CLI_DETACHED', '1'); + vi.stubEnv(CALLER_ARGV_ENV, value); + expect(resolveDevOptions(child).callerArgv).toEqual(child); + } + ); + + it('is the own argv outside a detached child', () => { + vi.stubEnv('__EXPO_AGENT_CLI_DETACHED', ''); + vi.stubEnv(CALLER_ARGV_ENV, JSON.stringify(caller)); + expect(resolveDevOptions(child).callerArgv).toEqual(child); + }); +}); diff --git a/packages/@expo/agent-cli/src/dev/detachAsync.ts b/packages/@expo/agent-cli/src/dev/detachAsync.ts index a7f4d968..a8acae8a 100644 --- a/packages/@expo/agent-cli/src/dev/detachAsync.ts +++ b/packages/@expo/agent-cli/src/dev/detachAsync.ts @@ -54,6 +54,7 @@ import { type DetachedChildPhase, type DetachedChildVerdict, } from './childVerdict'; +import { CALLER_ARGV_ENV, explicitRequestOf } from './deviceOption'; import { event } from './events'; import { detachedLogPath, @@ -229,7 +230,12 @@ export async function devDetachAsync( hostBindsPlatform(platform) ) { Log.progress( - acquireLine(await acquireDeviceAsync(projectRoot, platform, { reuseOnly: true })) + acquireLine( + await acquireDeviceAsync(projectRoot, platform, { + reuseOnly: true, + explicit: explicitRequestOf(options.device, options.callerArgv), + }) + ) ); } const hasExited = () => !isProcessAlive(running.pid); @@ -305,6 +311,7 @@ export async function devDetachAsync( ...process.env, __EXPO_AGENT_CLI_INTERNAL_INVOCATION: '1', __EXPO_AGENT_CLI_DETACHED: '1', + [CALLER_ARGV_ENV]: JSON.stringify(options.callerArgv), }, }); // The child owns the file now; a descriptor left open here would keep the parent alive. diff --git a/packages/@expo/agent-cli/src/dev/devAsync.ts b/packages/@expo/agent-cli/src/dev/devAsync.ts index 52afd527..7e40f9f5 100644 --- a/packages/@expo/agent-cli/src/dev/devAsync.ts +++ b/packages/@expo/agent-cli/src/dev/devAsync.ts @@ -5,7 +5,10 @@ import type { OpenAppOnEasReport } from './openAppEas'; import { withLeaseExtendedAsync } from '../deviceBinding/renew'; +import { isExplicitChoiceRefusal } from '../deviceBinding/errors'; +import { previewExplicitDeviceAsync } from '../deviceBinding/preview'; import { outputTail } from '../deploy/parseOutput'; +import { readDevServerLockAsync } from '../devLock/client'; import { readLastLoggedDevServerPort, readPortArg } from '../devLock/port'; import { acquireDeviceAsync, @@ -31,10 +34,10 @@ import { classifySubprocessFailure, lastNonEmptyLine } from '../needsHuman/detec import { assertEasProjectConfiguredAsync } from '../needsHuman/easProject'; import { needsHumanErrorFrom } from '../needsHuman/error'; import { emitStartPlan } from '../plan/emit'; +import { rerunDevCommand } from '../plan/rerun'; import { event as planEvent } from '../plan/events'; import { readLastBuildRecord, recordLastBuildFingerprint } from '../plan/lastBuild'; import { resolveStartPlanAsync } from '../plan/resolveAsync'; -import { isPlatformFlag } from '../plan/platformFlags'; import type { NativePlatform, PlanPlatform } from '../plan/types'; import { PROGRAM_NAME, PROGRAM_PREFIX } from '../programName'; import { defaultSmokePlatformAsync, smokeCommand } from '../smoke/suggest'; @@ -64,6 +67,7 @@ import { type WrapperCrashTool, } from '../utils/wrapperCrash'; import { appReachedDevice } from './buildEvidence'; +import { explicitRequestOf, withoutDeviceArgs } from './deviceOption'; import { event as devEvent } from './events'; import { forwardedStepArgs, @@ -91,6 +95,9 @@ import { easCommandPrefix } from '../utils/easCli'; * last step when every step succeeded. */ export async function devAsync(projectRoot: string, options: DevOptions): Promise { + if (options.device !== undefined && boundPlatform(options) == null) { + throw deviceNotBindableError(options); + } if (options.mode !== 'plan' && options.deviceBackend === 'eas') { await assertEasProjectConfiguredAsync(projectRoot); } @@ -178,19 +185,23 @@ export async function devAsync(projectRoot: string, options: DevOptions): Promis // refusals below, so a run that refuses creates no simulator, and `--plan` only inspects. const bindsHere = resolved.buildLocation != null; let acquired = boundEarly; + const implicitEas = + plan.buildLocation?.runsOn === 'eas' && plan.buildLocation.selection?.implicit === true; // @ref llp/0009-smart-followups.rfc.md §Examples per command // `--plan` stops here, so its follow-ups are about the plan itself and the plan object is the // whole answer. if (options.mode === 'plan') { - if (bindsHere) { - acquired = await acquireDevice(plan); + if (!implicitEas) { + if (bindsHere) { + acquired = await acquireDevice(plan); + } + plan = await withPlannedDevice(projectRoot, plan, acquired, options); } - plan = withPlannedDevice(plan, acquired, options); const followups = followUpsEnabled(options.followups) ? // The typed flag, not the resolved platform: this is the plan the caller asked for, and the // command that runs it has to ask for the same one (F103). - buildStartPlanFollowUps(plan, state, options.platform) + buildStartPlanFollowUps(plan, state, options.platform, options.callerArgv) : []; emitStartPlan(plan, { mode: 'plan', json: options.json, followups }); reportFollowUps('dev', followups, { json: options.json }); @@ -200,7 +211,7 @@ export async function devAsync(projectRoot: string, options: DevOptions): Promis // @ref llp/0015-backend-selection-and-config.rfc.md §The selection — named, not taken. // Detection may say the cloud is the only route this host leaves; it may not spend the caller's // EAS credits on that finding. A flag or the config is a person's own choice and runs. - if (plan.buildLocation?.runsOn === 'eas' && plan.buildLocation.selection?.implicit) { + if (implicitEas) { throw implicitEasRouteError(plan, options.platform); } @@ -294,7 +305,9 @@ function deviceCallbackFor( return null; } if (options.mode === 'plan') { - return await inspectOwnDeviceAsync(projectRoot, platform); + return options.device === undefined + ? await inspectOwnDeviceAsync(projectRoot, platform) + : null; } if (options.port != null && draft.steps.some(isDevServerStep)) { const planned = await resolvePlannedPortAsync(options.port); @@ -302,7 +315,10 @@ function deviceCallbackFor( throw await portDemandedError(projectRoot, options.port, options.platform); } } - const acquired = await acquireDeviceAsync(projectRoot, platform); + const acquired = await acquireDeviceAsync(projectRoot, platform, { + reuseOnly: (await readDevServerLockAsync(projectRoot)) != null, + explicit: explicitRequestOf(options.device, options.callerArgv), + }); Log.progress(acquireLine(acquired)); return acquired; }; @@ -323,6 +339,25 @@ function boundPlatform(options: DevOptions): DevicePlatform | null { : null; } +/** `--device` on a run that binds no local device, which would drop the device it names. */ +function deviceNotBindableError(options: DevOptions): CommandError { + const why = + options.platform === 'web' + ? '--web serves the app to a browser, which needs no device' + : devicesDisabled() + ? 'AGENT_CLI_NO_DEVICE=1 turns every device off' + : options.platform === 'ios' + ? 'only macOS runs an iOS simulator' + : 'this machine has no Android SDK'; + const command = rerunDevCommand(options.platform, withoutDeviceArgs(options.callerArgv)); + const error = new CommandError( + 'BAD_ARGS', + `--device names a local device to bind, and ${why}, so nothing would use it.\nHow: remove --device, then run "${command}".` + ); + error.suggestedCommand = command; + return error; +} + /** `--plan`'s read of the own binding: the device as `reused` when it is up or down, else null. */ async function inspectOwnDeviceAsync( projectRoot: string, @@ -335,11 +370,38 @@ async function inspectOwnDeviceAsync( } /** The `--plan` view: the steps pinned to the bound device, or the reason none is pinned yet. */ -function withPlannedDevice( +async function withPlannedDevice( + projectRoot: string, plan: StartPlan, acquired: AcquireResult | null, options: DevOptions -): StartPlan { +): Promise { + const selectedPlatform = boundPlatform(options); + const explicit = explicitRequestOf(options.device, options.callerArgv); + if (selectedPlatform && explicit) { + try { + const { device, summary } = await previewExplicitDeviceAsync( + projectRoot, + selectedPlatform, + explicit, + (await readDevServerLockAsync(projectRoot)) != null + ); + return { + ...withDevice(plan, deviceIdOf(device)), + explicitDevice: { refuses: false, summary }, + }; + } catch (error) { + if (!isExplicitChoiceRefusal(error)) throw error; + return { + ...plan, + explicitDevice: { + refuses: true, + summary: error.message.split('\n')[0]!, + how: error.suggestedCommand ?? null, + }, + }; + } + } if (acquired) { return withDevice(plan, deviceIdOf(acquired.device)); } @@ -606,7 +668,7 @@ async function executePlanAsync( askedPort, movedTo: retry ? readPortArg(retry.args) : null, exitCode: result.exitCode, - callerArgv: options.detachArgv, + callerArgv: options.callerArgv, platform: options.platform, }); } @@ -924,15 +986,8 @@ async function portTakenAfterRetryError(stop: { movedTo != null ? `Why: the step asked for port ${askedPort}, which was taken, and the one retry moved it to port ${movedTo}, which ${holder} bound before the dev server did.` : `Why: ${holder} holds port ${askedPort}, and no free port was found to retry on.`; - // The platform flag first, so the command reads as `dev -- …` wherever it was typed. - const separator = stop.callerArgv.indexOf('--'); - const rest = stop.callerArgv.filter( - (arg, index) => !isPlatformFlag(arg) || (separator !== -1 && index > separator) - ); const onFreePort = - free == null - ? null - : `${PROGRAM_PREFIX} dev --${stop.platform} ${withPortArg(rest, free).join(' ')}`; + free == null ? null : rerunDevCommand(stop.platform, withPortArg(stop.callerArgv, free)); const forceStop = answersAsDevServer && listener != null && looksLikeDevServerProcess(listener) ? `${PROGRAM_PREFIX} dev:stop --port ${taken} --force` diff --git a/packages/@expo/agent-cli/src/dev/deviceOption.ts b/packages/@expo/agent-cli/src/dev/deviceOption.ts new file mode 100644 index 00000000..ff0754b9 --- /dev/null +++ b/packages/@expo/agent-cli/src/dev/deviceOption.ts @@ -0,0 +1,43 @@ +// @ref llp/0035-one-device-per-worktree.rfc.md §--device on dev +import type { ExplicitRequest } from '../deviceBinding/explicit'; +import { CommandError } from '../utils/errors'; + +/** Set by a detached `dev` on its child, whose own argv lacks the parent's `--detach`, `--wait-ready` and `--json`. */ +export const CALLER_ARGV_ENV = '__EXPO_AGENT_CLI_CALLER_ARGV'; + +/** `--device`'s value, and the caller's arguments without it, which a refusal's How repeats. */ +export function explicitRequestOf( + device: string | undefined, + callerArgv: readonly string[] +): ExplicitRequest | undefined { + return device === undefined ? undefined : { query: device, rerun: withoutDeviceArgs(callerArgv) }; +} + +export function resolveDeviceOption(argv: string[]): string | undefined { + const separator = argv.indexOf('--'); + const own = separator < 0 ? argv : argv.slice(0, separator); + let value: string | undefined; + for (const [index, arg] of own.entries()) { + if (arg === '--device' || arg.startsWith('--device=')) { + value = arg === '--device' ? own[index + 1] : arg.slice('--device='.length); + if (!value?.trim() || value.startsWith('-')) + throw new CommandError( + 'BAD_ARGS', + '--device needs a simulator name or ID, or an Android serial.' + ); + } + } + return value; +} + +export function withoutDeviceArgs(argv: readonly string[]): string[] { + const separator = argv.indexOf('--'); + const own = separator < 0 ? argv : argv.slice(0, separator); + const kept: string[] = []; + for (let index = 0; index < own.length; index++) { + const arg = own[index]!; + if (arg === '--device') index++; + else if (!arg.startsWith('--device=')) kept.push(arg); + } + return separator < 0 ? kept : [...kept, ...argv.slice(separator)]; +} diff --git a/packages/@expo/agent-cli/src/dev/index.ts b/packages/@expo/agent-cli/src/dev/index.ts index 27b2056a..3d68076c 100644 --- a/packages/@expo/agent-cli/src/dev/index.ts +++ b/packages/@expo/agent-cli/src/dev/index.ts @@ -8,6 +8,7 @@ export const devRunHelp: CommandHelp = { command: 'dev:run', usage: `${PROGRAM_PREFIX} dev --ios|--android|--web`, options: [ + `--device Bind a local simulator by name/ID or an Android device by serial`, `--detach Run the dev server in the background and give the terminal back`, `--wait-ready With --detach, also wait for the bundler before reporting`, `--plan Print what must run to get this app on a device, then exit`, diff --git a/packages/@expo/agent-cli/src/dev/knownFlags.ts b/packages/@expo/agent-cli/src/dev/knownFlags.ts index e35a786e..1c30c768 100644 --- a/packages/@expo/agent-cli/src/dev/knownFlags.ts +++ b/packages/@expo/agent-cli/src/dev/knownFlags.ts @@ -30,6 +30,7 @@ import { unknownOptionError } from '../utils/unknownOption'; * Go", which is exactly what the preference says. */ export const DEV_OWN_FLAGS: readonly string[] = [ + '--device', '--eas', '--local', '--no-open', diff --git a/packages/@expo/agent-cli/src/dev/resolveOptions.ts b/packages/@expo/agent-cli/src/dev/resolveOptions.ts index 1f71a582..060431f0 100644 --- a/packages/@expo/agent-cli/src/dev/resolveOptions.ts +++ b/packages/@expo/agent-cli/src/dev/resolveOptions.ts @@ -8,6 +8,7 @@ import { PROGRAM_PREFIX } from '../programName'; import type { BuildBackend, RunTarget } from '../settings/types'; import { CommandError } from '../utils/errors'; import { DEFAULT_DETACH_TIMEOUT_MS } from './detachAsync'; +import { CALLER_ARGV_ENV, resolveDeviceOption, withoutDeviceArgs } from './deviceOption'; import { withoutPortArgs } from './forwardedArgs'; import { assertKnownDevFlags } from './knownFlags'; @@ -43,6 +44,7 @@ export type DevMode = export interface DevOptions { mode: DevMode; + device?: string; /** Arguments to append after the plan's own, when spawning the `expo` CLI. */ expoArgs: string[]; /** Sync skills shortly after the dev server starts, cleared by `--no-agent-skills`. */ @@ -130,6 +132,21 @@ export interface DevOptions { * parent's. Recorded here so the resolver stays the one place that reads argv. */ detachArgv: string[]; + /** The command line the caller typed, which a How line repeats; a detached child's parent gives it. */ + callerArgv: string[]; +} + +/** Read once, and removed so the child's own subprocesses never inherit it. */ +function callerArgvOf(argv: string[]): string[] { + const value = process.env[CALLER_ARGV_ENV]; + delete process.env[CALLER_ARGV_ENV]; + if (process.env.__EXPO_AGENT_CLI_DETACHED !== '1' || value === undefined) return argv; + try { + const parsed: unknown = JSON.parse(value); + return Array.isArray(parsed) && parsed.every((arg) => typeof arg === 'string') ? parsed : argv; + } catch { + return argv; + } } /** @@ -169,6 +186,7 @@ export function resolveDevOptions(argv: string[]): DevOptions { const buildBackend = resolveBuildBackend(argv, example); const runTarget = resolveRunTarget(argv, example); const port = resolvePort(argv, example); + const device = resolveDeviceOption(argv); const open = !argv.includes('--no-open'); const deviceBackend: DeviceBackend = buildBackend === 'eas' ? 'eas' : 'local'; if (deviceBackend === 'eas') { @@ -177,6 +195,7 @@ export function resolveDevOptions(argv: string[]): DevOptions { return { mode: argv.includes('--plan') ? 'plan' : 'run', + ...(device === undefined ? {} : { device }), // `--port` is stripped: `dev` owns the port and sets it on every step that serves // (`./portCollision.ts`). A native platform flag is stripped too: it names what the plan is // for, and this command performs the open itself (`./openApp.ts`) — handing it to `expo start` @@ -184,7 +203,9 @@ export function resolveDevOptions(argv: string[]): DevOptions { // `--web` stays: serving the web bundle is `expo start`'s own job. expoArgs: [ ...withoutPortArgs( - argv.filter((arg) => !AGENT_CLI_ONLY_FLAGS.includes(arg) && !isNativePlatformFlag(arg)) + withoutDeviceArgs(argv).filter( + (arg) => !AGENT_CLI_ONLY_FLAGS.includes(arg) && !isNativePlatformFlag(arg) + ) ), // @ref llp/0027-everything-on-eas.rfc.md §The dev server is tunnelled // Implied, never asked for: an EAS Simulator session is a machine in a datacenter, and @@ -207,6 +228,7 @@ export function resolveDevOptions(argv: string[]): DevOptions { // The child's argv is built from the same list, so a flag added to this command reaches the // detached run without a second place to remember. detachArgv: argv, + callerArgv: callerArgvOf(argv), }; } @@ -257,6 +279,11 @@ function namesTunnel(argv: readonly string[]): boolean { * host the session cannot reach (`--lan`, `--localhost`, `--host lan|localhost`). */ function assertEasRunFits(argv: string[]): void { + if (resolveDeviceOption(argv) !== undefined) + throw new CommandError( + 'BAD_ARGS', + '--device selects a local device and cannot be used with --eas. How: remove --device to use EAS Simulator.' + ); if (argv.includes('--web')) { throw opposite( '--eas and --web ask for two different places to run the app, so this run has no plan.', diff --git a/packages/@expo/agent-cli/src/deviceBinding/__tests__/android-test.ts b/packages/@expo/agent-cli/src/deviceBinding/__tests__/android-test.ts index 4dc9d111..8842892f 100644 --- a/packages/@expo/agent-cli/src/deviceBinding/__tests__/android-test.ts +++ b/packages/@expo/agent-cli/src/deviceBinding/__tests__/android-test.ts @@ -124,9 +124,12 @@ describe(acquireDeviceAsync, () => { ]); }); - it('spawns again when the own instance is dead, on the first free port', async () => { + it.each(['spawned', 'explicit'] as const)('replaces missing %s', async (kind) => { const tools = fakeTools(); - writeBinding(file(), androidBindingFor(ROOT, 'emulator-5556', spawned(5556, 77))); + writeBinding( + file(), + androidBindingFor(ROOT, 'emulator-5556', kind === 'spawned' ? spawned(5556, 77) : { kind }) + ); const result = await acquireDeviceAsync(ROOT, 'android', { tools }); diff --git a/packages/@expo/agent-cli/src/deviceBinding/__tests__/explicit-test.ts b/packages/@expo/agent-cli/src/deviceBinding/__tests__/explicit-test.ts new file mode 100644 index 00000000..02b7488d --- /dev/null +++ b/packages/@expo/agent-cli/src/deviceBinding/__tests__/explicit-test.ts @@ -0,0 +1,368 @@ +import { vol } from 'memfs'; + +import { acquireDeviceAsync, releaseWorktreeDevicesAsync } from '..'; +import { bindingPathFor, readBindingFile, removeBindingFile } from '../registry'; +import { AVD } from './fakeAndroidTools'; +import { androidBindingFor, bindingFor, fakeTools, writeBinding } from './fakeTools'; + +const ROOT = '/work/app'; +const OTHER = '/work/other'; +const file = (root = ROOT) => bindingPathFor(root, 'ios', 'local-ios'); +beforeEach(() => { + vol.reset(); + vol.fromJSON({ [`${ROOT}/package.json`]: '{}', [`${OTHER}/package.json`]: '{}' }); +}); +const human = { udid: 'HUMAN', name: 'My iPhone', state: 'Shutdown' as const }; +const named = (query: string, rerun = ['--ios']) => ({ query, rerun }); + +it('device-over-own-created-deletes, while explicit-never-shut-down', async () => { + const tools = fakeTools({ + simulators: [human, { udid: 'OLD', name: 'agent-cli old', state: 'Booted' }], + }); + writeBinding(file(), bindingFor(ROOT, 'OLD')); + expect( + await acquireDeviceAsync(ROOT, 'ios', { explicit: named('my iphone'), tools }) + ).toMatchObject({ + action: 'explicit', + device: { udid: 'HUMAN', origin: 'explicit' }, + }); + expect(tools.calls).toContainEqual(['delete', 'OLD']); + expect(tools.lockedAt[tools.calls.findIndex(([command]) => command === 'delete')]).toBe(false); + await releaseWorktreeDevicesAsync(ROOT, { tools }); + expect(tools.calls).not.toContainEqual(['shutdown', 'HUMAN']); + expect(readBindingFile(file())).toMatchObject({ binding: { device: { udid: 'HUMAN' } } }); +}); + +it.each(['2999-01-01T00:00:00Z', '2020-01-01T00:00:00Z'])( + 'explicit-bound-live-or-stale (%s)', + async (expiresAt) => { + const tools = fakeTools({ simulators: [human] }); + writeBinding(file(OTHER), bindingFor(OTHER, 'HUMAN', { expiresAt })); + await expect( + acquireDeviceAsync(ROOT, 'ios', { explicit: named('HUMAN'), tools }) + ).rejects.toMatchObject({ data: { reason: 'explicit-bound' }, exitCode: 20 }); + expect(readBindingFile(file())).toEqual({ kind: 'none' }); + } +); + +it('transfers an expired foreign explicit device and reuses it on the next dev', async () => { + const tools = fakeTools({ simulators: [human] }); + const binding = bindingFor(OTHER, 'HUMAN', { expiresAt: '2020-01-01T00:00:00Z' }); + if (binding.device.backend === 'local-ios') binding.device.origin = 'explicit'; + writeBinding(file(OTHER), binding); + await acquireDeviceAsync(ROOT, 'ios', { explicit: named('human'), tools }); + expect(readBindingFile(file(OTHER))).toEqual({ kind: 'none' }); + expect(await acquireDeviceAsync(ROOT, 'ios', { tools })).toMatchObject({ + action: 'reused', + device: { udid: 'HUMAN', origin: 'explicit' }, + }); +}); + +it('refuses ambiguous names, leaked creates, and reuseOnly replacement without changing the own binding', async () => { + const tools = fakeTools({ + simulators: [ + human, + { ...human, udid: 'SECOND' }, + { udid: 'LEAK', name: 'agent-cli lost', state: 'Shutdown' }, + ], + }); + await expect( + acquireDeviceAsync(ROOT, 'ios', { explicit: named(human.name), tools }) + ).rejects.toMatchObject({ + data: { reason: 'explicit-not-found', matches: ['HUMAN', 'SECOND'] }, + exitCode: 1, + }); + await expect( + acquireDeviceAsync(ROOT, 'ios', { explicit: named('LEAK'), tools }) + ).rejects.toMatchObject({ + data: { reason: 'explicit-not-found' }, + }); + writeBinding(file(), bindingFor(ROOT, 'SECOND')); + await expect( + acquireDeviceAsync(ROOT, 'ios', { explicit: named('HUMAN'), reuseOnly: true, tools }) + ).rejects.toMatchObject({ + data: { reason: 'not-reusable' }, + exitCode: 20, + message: + 'The running dev server uses agent-cli 0000; --device names My iPhone.\nHow: run "npx @expo/agent-cli dev:stop", then this command again.', + }); + expect(readBindingFile(file())).toMatchObject({ binding: { device: { udid: 'SECOND' } } }); +}); + +it("refusals' How lines repeat the caller's flags, and an ambiguous name names a match", async () => { + const tools = fakeTools({ simulators: [human, { ...human, udid: 'SECOND' }] }); + const rerun = ['--ios', '--detach', '--wait-ready', '--port', '8082']; + writeBinding(file(OTHER), bindingFor(OTHER, 'SECOND')); + await expect( + acquireDeviceAsync(ROOT, 'ios', { explicit: named('SECOND', rerun), tools }) + ).rejects.toMatchObject({ + message: + 'My iPhone is bound to the worktree /work/other.\nHow: remove --device, then run "npx @expo/agent-cli dev --ios --detach --wait-ready --port 8082".', + suggestedCommand: 'npx @expo/agent-cli dev --ios --detach --wait-ready --port 8082', + }); + await expect( + acquireDeviceAsync(ROOT, 'ios', { explicit: named('my iphone', rerun), tools }) + ).rejects.toMatchObject({ + message: + 'No device uniquely matches "my iphone"; matches: HUMAN, SECOND.\nHow: run "npx @expo/agent-cli dev --ios --detach --wait-ready --port 8082 --device HUMAN", or pass another match to --device.', + suggestedCommand: + 'npx @expo/agent-cli dev --ios --detach --wait-ready --port 8082 --device HUMAN', + }); + await expect( + acquireDeviceAsync(ROOT, 'ios', { explicit: named('nope', rerun), tools }) + ).rejects.toMatchObject({ + message: + 'No device uniquely matches "nope".\nHow: run "xcrun simctl list devices available", then pass a listed name or UDID to --device.', + suggestedCommand: 'xcrun simctl list devices available', + }); + await expect( + acquireDeviceAsync(ROOT, 'android', { explicit: named('nope', ['--android']), tools }) + ).rejects.toMatchObject({ + message: + 'No device uniquely matches "nope".\nHow: run "adb devices", then pass a listed serial to --device.', + }); +}); + +it("a refusal's How runs a foreground caller's command detached, without --plan", async () => { + const tools = fakeTools({ simulators: [human] }); + writeBinding(file(OTHER), bindingFor(OTHER, 'HUMAN')); + await expect( + acquireDeviceAsync(ROOT, 'ios', { + explicit: named('HUMAN', ['--local', '--plan', '--ios', '--port', '8082']), + tools, + }) + ).rejects.toMatchObject({ + suggestedCommand: 'npx @expo/agent-cli dev --ios --local --port 8082 --detach --wait-ready', + }); +}); + +it('preview refuses not-reusable as the run does while the own dev server runs', async () => { + const { previewExplicitDeviceAsync } = await import('../preview'); + const tools = fakeTools({ + simulators: [human, { udid: 'OLD', name: 'agent-cli 0000', state: 'Booted' }], + }); + writeBinding(file(), bindingFor(ROOT, 'OLD')); + await expect( + previewExplicitDeviceAsync(ROOT, 'ios', named('HUMAN'), true, tools) + ).rejects.toMatchObject({ + data: { reason: 'not-reusable' }, + message: expect.stringContaining( + 'The running dev server uses agent-cli 0000; --device names My iPhone.' + ), + }); +}); + +it('will not orphan an own spawned emulator when asked to use another serial', async () => { + const tools = fakeTools({ + emulators: [ + { serial: 'emulator-5554', state: 'device', pid: 4001 }, + { serial: 'emulator-5556', state: 'device' }, + ], + }); + const file = bindingPathFor(ROOT, 'android', 'local-android'); + const binding = androidBindingFor(ROOT, 'emulator-5554', { + kind: 'spawned', + avd: AVD, + port: 5554, + emulatorPid: 4001, + }); + writeBinding(file, binding); + await expect( + acquireDeviceAsync(ROOT, 'android', { explicit: named('emulator-5556'), tools }) + ).rejects.toMatchObject({ data: { reason: 'own-explicit-over-owned' }, exitCode: 1 }); + expect(readBindingFile(file)).toEqual({ kind: 'binding', binding }); + expect(tools.killed).toEqual([]); +}); + +it('binds another serial over an own spawned emulator whose pid is dead', async () => { + const tools = fakeTools({ + emulators: [{ serial: 'emulator-5556', state: 'device' }], + alivePids: [], + }); + const file = bindingPathFor(ROOT, 'android', 'local-android'); + writeBinding( + file, + androidBindingFor(ROOT, 'emulator-5554', { + kind: 'spawned', + avd: AVD, + port: 5554, + emulatorPid: 4001, + }) + ); + expect( + await acquireDeviceAsync(ROOT, 'android', { + explicit: named('emulator-5556', ['--android']), + tools, + }) + ).toMatchObject({ action: 'explicit', device: { serial: 'emulator-5556' } }); + expect(readBindingFile(file)).toMatchObject({ + binding: { device: { serial: 'emulator-5556', origin: { kind: 'explicit' } } }, + }); + expect(tools.killed).toEqual([]); +}); + +it('preview reports a foreign owner without changing its expired binding or running cleanup', async () => { + const { previewExplicitDeviceAsync } = await import('../preview'); + const tools = fakeTools({ simulators: [human] }); + const binding = bindingFor(OTHER, 'HUMAN', { expiresAt: '2020-01-01T00:00:00Z' }); + writeBinding(file(OTHER), binding); + await expect( + previewExplicitDeviceAsync(ROOT, 'ios', named('HUMAN'), false, tools) + ).rejects.toThrow('bound to the worktree /work/other'); + expect(readBindingFile(file(OTHER))).toEqual({ kind: 'binding', binding }); + expect(readBindingFile(file())).toEqual({ kind: 'none' }); + expect(tools.calls.every(([command]) => command === 'list')).toBe(true); +}); + +it('preview says which device it binds, whose expired binding it takes, and what it deletes', async () => { + const { previewExplicitDeviceAsync } = await import('../preview'); + const tools = fakeTools({ + simulators: [human, { udid: 'OLD', name: 'agent-cli 0000', state: 'Booted' }], + }); + const own = bindingFor(ROOT, 'OLD'); + const foreign = bindingFor(OTHER, 'HUMAN', { expiresAt: '2020-01-01T00:00:00Z' }); + if (foreign.device.backend === 'local-ios') foreign.device.origin = 'explicit'; + writeBinding(file(), own); + writeBinding(file(OTHER), foreign); + expect( + (await previewExplicitDeviceAsync(ROOT, 'ios', named('my iphone'), false, tools)).summary + ).toBe( + 'Will bind My iPhone (HUMAN), taken over from /work/other (lease expired); will delete agent-cli 0000 (replaced by --device).' + ); + expect(readBindingFile(file())).toEqual({ kind: 'binding', binding: own }); + expect(readBindingFile(file(OTHER))).toEqual({ kind: 'binding', binding: foreign }); + expect(tools.calls.every(([command]) => command === 'list')).toBe(true); +}); + +it('reuses an explicitly named owned Android instance while adb still lists it offline', async () => { + const tools = fakeTools({ + emulators: [{ serial: 'emulator-5554', state: 'offline', pid: 4001 }], + }); + const file = bindingPathFor(ROOT, 'android', 'local-android'); + const binding = androidBindingFor(ROOT, 'emulator-5554', { + kind: 'spawned', + avd: AVD, + port: 5554, + emulatorPid: 4001, + }); + writeBinding(file, binding); + expect( + await acquireDeviceAsync(ROOT, 'android', { explicit: named('emulator-5554'), tools }) + ).toMatchObject({ action: 'reused', device: binding.device }); + expect(tools.spawned).toEqual([]); +}); + +it.each(['offline', 'unauthorized', 'no permissions'] as const)( + 'refuses a serial adb lists %s as not ready, and names it again in the How', + async (state) => { + const { previewExplicitDeviceAsync } = await import('../preview'); + const tools = fakeTools({ emulators: [{ serial: 'R58N123', state }] }); + const explicit = named('R58N123', ['--android']); + const refusal = { + data: { reason: 'explicit-not-ready', state }, + exitCode: 7, + suggestedCommand: 'npx @expo/agent-cli dev --android --device R58N123 --detach --wait-ready', + }; + await expect( + previewExplicitDeviceAsync(ROOT, 'android', explicit, false, tools) + ).rejects.toMatchObject(refusal); + await expect(acquireDeviceAsync(ROOT, 'android', { explicit, tools })).rejects.toMatchObject( + refusal + ); + expect(readBindingFile(bindingPathFor(ROOT, 'android', 'local-android'))).toEqual({ + kind: 'none', + }); + } +); + +it('explicit-android-needs-only-adb', async () => { + const { previewExplicitDeviceAsync } = await import('../preview'); + const tools = fakeTools({ emulators: [{ serial: 'R58N123', state: 'device' }] }); + tools.emulatorList = vi.fn(async () => ({ + stdout: '', + stderr: '', + exitCode: null, + spawnError: Object.assign(new Error('spawn emulator ENOENT'), { code: 'ENOENT' }), + })); + expect( + (await previewExplicitDeviceAsync(ROOT, 'android', named('R58N123'), false, tools)).device + ).toMatchObject({ + serial: 'R58N123', + origin: { kind: 'explicit' }, + }); + expect( + await acquireDeviceAsync(ROOT, 'android', { explicit: named('R58N123'), tools }) + ).toMatchObject({ + action: 'explicit', + device: { serial: 'R58N123' }, + }); + expect(readBindingFile(bindingPathFor(ROOT, 'android', 'local-android'))).toMatchObject({ + binding: { device: { serial: 'R58N123', origin: { kind: 'explicit' } } }, + }); + await releaseWorktreeDevicesAsync(ROOT, { tools }); + for (const reuseOnly of [false, true]) { + expect(await acquireDeviceAsync(ROOT, 'android', { tools, reuseOnly })).toMatchObject({ + action: 'reused', + device: { serial: 'R58N123', origin: { kind: 'explicit' } }, + }); + } + expect(tools.emulatorList).not.toHaveBeenCalled(); + expect(tools.calls.some(([command]) => command === 'emulator')).toBe(false); + tools.emulators.length = 0; + await expect(acquireDeviceAsync(ROOT, 'android', { tools })).rejects.toMatchObject({ + code: 'EMULATOR_NOT_RUNNABLE', + }); +}); + +it('refreshes the AVD inventory if a USB binding is removed during discovery', async () => { + const tools = fakeTools({ emulators: [{ serial: 'USB', state: 'device' }] }); + const file = bindingPathFor(ROOT, 'android', 'local-android'); + writeBinding(file, androidBindingFor(ROOT, 'USB', { kind: 'explicit' })); + const adb = tools.adb; + vi.spyOn(tools, 'adb').mockImplementationOnce(async (args, options) => { + const result = await adb(args, options); + removeBindingFile(file); + return result; + }); + + expect(await acquireDeviceAsync(ROOT, 'android', { tools })).toMatchObject({ + action: 'spawned', + device: { serial: 'emulator-5554' }, + }); + const listedAvds = tools.calls.findIndex( + ([command, arg]) => command === 'emulator' && arg === '-list-avds' + ); + expect(listedAvds).toBeGreaterThanOrEqual(0); + expect(tools.lockedAt[listedAvds]).toBe(false); +}); + +it('reuses an own USB binding adb lists unauthorized, and lists no AVD', async () => { + const tools = fakeTools({ emulators: [{ serial: 'R58N123', state: 'unauthorized' }] }); + writeBinding( + bindingPathFor(ROOT, 'android', 'local-android'), + androidBindingFor(ROOT, 'R58N123', { kind: 'explicit' }) + ); + + expect(await acquireDeviceAsync(ROOT, 'android', { tools })).toMatchObject({ + action: 'reused', + device: { serial: 'R58N123', origin: { kind: 'explicit' } }, + }); + expect(tools.calls.some(([command]) => command === 'emulator')).toBe(false); +}); + +it('refreshes the AVD inventory if an unauthorized USB binding is removed during discovery', async () => { + const tools = fakeTools({ emulators: [{ serial: 'USB', state: 'unauthorized' }] }); + const file = bindingPathFor(ROOT, 'android', 'local-android'); + writeBinding(file, androidBindingFor(ROOT, 'USB', { kind: 'explicit' })); + const adb = tools.adb; + vi.spyOn(tools, 'adb').mockImplementationOnce(async (args, options) => { + const result = await adb(args, options); + removeBindingFile(file); + return result; + }); + + expect(await acquireDeviceAsync(ROOT, 'android', { tools })).toMatchObject({ + action: 'spawned', + device: { serial: 'emulator-5554' }, + }); +}); diff --git a/packages/@expo/agent-cli/src/deviceBinding/__tests__/fakeAndroidTools.ts b/packages/@expo/agent-cli/src/deviceBinding/__tests__/fakeAndroidTools.ts index d767c383..bb8faf76 100644 --- a/packages/@expo/agent-cli/src/deviceBinding/__tests__/fakeAndroidTools.ts +++ b/packages/@expo/agent-cli/src/deviceBinding/__tests__/fakeAndroidTools.ts @@ -9,7 +9,7 @@ export const AVD = 'Pixel_9'; export interface FakeEmulator { serial: string; /** What `adb` lists and `get-state` answers; `offline` is an instance still booting. */ - state: 'device' | 'offline'; + state: 'device' | 'offline' | 'unauthorized' | 'no permissions'; /** The pid `isPidAlive` answers for. Absent for a device that is not an instance of ours. */ pid?: number; /** The command line `commandOf` answers; defaults to an instance of ours on its serial's port. */ diff --git a/packages/@expo/agent-cli/src/deviceBinding/choose.ts b/packages/@expo/agent-cli/src/deviceBinding/choose.ts index 94aa838d..23a29c7d 100644 --- a/packages/@expo/agent-cli/src/deviceBinding/choose.ts +++ b/packages/@expo/agent-cli/src/deviceBinding/choose.ts @@ -67,6 +67,8 @@ export interface AndroidInventory { runningSerials: string[]; /** Every serial `adb devices -l` lists, in any state: a booting instance already holds its port. */ listedSerials: string[]; + /** Serials adb lists in a state other than `device`, with that state. */ + unready: { serial: string; state: string }[]; } /** A serial on a console port and the worktree bound to it; `root` is null for one that is not ours. */ diff --git a/packages/@expo/agent-cli/src/deviceBinding/emulator.ts b/packages/@expo/agent-cli/src/deviceBinding/emulator.ts index 01874b69..012378d4 100644 --- a/packages/@expo/agent-cli/src/deviceBinding/emulator.ts +++ b/packages/@expo/agent-cli/src/deviceBinding/emulator.ts @@ -9,6 +9,7 @@ import type { SpawnCaptureResult } from '../utils/spawnCapture'; import { firstLine } from '../utils/text'; import type { AndroidInventory } from './choose'; import { deviceUnavailableError } from './errors'; +import { bindingPathFor, readBindingFile } from './registry'; import type { DeviceTools } from './types'; /** The budget of every `adb` call that is not a boot wait. */ @@ -42,13 +43,57 @@ function adbFailedError(reason: string): CommandError { ); } +/** Skip AVD discovery for a bound USB device; the caller rechecks the binding under the lock. */ +export async function readAndroidBindingInventoryAsync( + projectRoot: string, + tools: DeviceTools, + explicit: boolean, + forceAvds: boolean +) { + const file = bindingPathFor(projectRoot, 'android', 'local-android'); + const explicitSerial = () => { + const read = readBindingFile(file); + const device = read.kind === 'binding' ? read.binding.device : null; + return device?.backend === 'local-android' && device.origin.kind === 'explicit' + ? device.serial + : undefined; + }; + const reuseSerial = forceAvds ? undefined : explicitSerial(); + const inventory = await readAndroidInventoryAsync(tools, { avds: !explicit, reuseSerial }); + return { + inventory, + isCurrent: () => + reuseSerial === undefined || + !inventory.listedSerials.includes(reuseSerial) || + explicitSerial() === reuseSerial, + }; +} + /** The Android inventory, read before the lock. @throws the tool error, or `create-timeout`. */ -export async function readAndroidInventoryAsync(tools: DeviceTools): Promise { +export async function readAndroidInventoryAsync( + tools: DeviceTools, + { avds: listAvds = true, reuseSerial }: { avds?: boolean; reuseSerial?: string } = {} +): Promise { const devices = await tools.adb(['devices', '-l'], { timeoutMs: ADB_CALL_TIMEOUT_MS }); if (devices.spawnError) { throw adbNotRunnableError(resolveAdb(), devices.spawnError.message); } assertAnswered(devices, 'adb devices -l', adbFailedError); + const runningSerials = parseAndroidDevices(devices.stdout).map((device) => device.deviceId); + const listedSerials = devices.stdout + .split('\n') + .slice(1) + .flatMap((line) => line.trim().split(/\s+/)[0] || []); + const unready = devices.stdout + .split('\n') + .slice(1) + .flatMap((line) => { + const [serial, word, next] = line.trim().split(/\s+/); + const state = word === 'no' && next === 'permissions' ? 'no permissions' : word; + return serial && state && state !== 'device' ? [{ serial, state }] : []; + }); + if (!listAvds || (reuseSerial !== undefined && listedSerials.includes(reuseSerial))) + return { avd: null, runningSerials, listedSerials, unready }; const avds = await tools.emulatorList(['-list-avds'], { timeoutMs: 60_000 }); if (avds.spawnError) { throw emulatorNotRunnableError(avds.spawnError.message); @@ -56,11 +101,9 @@ export async function readAndroidInventoryAsync(tools: DeviceTools): Promise device.deviceId), - listedSerials: devices.stdout - .split('\n') - .slice(1) - .flatMap((line) => line.trim().split(/\s+/)[0] || []), + runningSerials, + listedSerials, + unready, }; } diff --git a/packages/@expo/agent-cli/src/deviceBinding/errors.ts b/packages/@expo/agent-cli/src/deviceBinding/errors.ts index d2fd62b4..5daacfdf 100644 --- a/packages/@expo/agent-cli/src/deviceBinding/errors.ts +++ b/packages/@expo/agent-cli/src/deviceBinding/errors.ts @@ -122,16 +122,37 @@ export function deviceUnavailableError( } } -function notReusableError(platform: DevicePlatform): CommandError { +/** A running dev server allows only a reuse; `devices` names the bound one and the one `--device` asks for. */ +export function notReusableError( + platform: DevicePlatform, + devices?: { bound: string; named: string } +): CommandError { const error = refusal('DEVICE_UNAVAILABLE', 'not-reusable', EXIT_OUTCOME_FAILED, [ - `A dev server is running and this worktree has no ${platform} device to reuse.`, - 'Why: a device created now would carry no app, because the running server already installed nothing on it.', + ...(devices + ? [`The running dev server uses ${devices.bound}; --device names ${devices.named}.`] + : [ + `A dev server is running and this worktree has no ${platform} device to reuse.`, + 'Why: a device created now would carry no app, because the running server already installed nothing on it.', + ]), `How: run "${PROGRAM_PREFIX} dev:stop", then this command again.`, ]); error.suggestedCommand = `${PROGRAM_PREFIX} dev:stop`; return error; } +const CHOICE_REFUSALS = [ + 'explicit-not-found', + 'explicit-not-ready', + 'explicit-bound', + 'own-explicit-over-owned', + 'not-reusable', +]; + +/** A refusal of the choice itself, which `dev --plan --device` prints; any other error fails the plan as it fails the run. */ +export function isExplicitChoiceRefusal(error: unknown): error is CommandError { + return error instanceof CommandError && CHOICE_REFUSALS.includes(error.data?.reason as string); +} + function portHolders(boundBy: BoundSerial[]): string { return ( boundBy.map(({ id, root }) => `${id} (${root ?? 'not ours'})`).join(', ') || diff --git a/packages/@expo/agent-cli/src/deviceBinding/explicit.ts b/packages/@expo/agent-cli/src/deviceBinding/explicit.ts new file mode 100644 index 00000000..8f43136e --- /dev/null +++ b/packages/@expo/agent-cli/src/deviceBinding/explicit.ts @@ -0,0 +1,240 @@ +// @ref llp/0035-one-device-per-worktree.rfc.md §Choice +import { rerunDevCommand } from '../plan/rerun'; +import { canonicalizeExistingPath } from '../utils/dir'; +import { EXIT_NEEDS_HUMAN } from '../exitCodes'; +import { CommandError } from '../utils/errors'; +import { PROGRAM_PREFIX } from '../programName'; +import type { AndroidInventory } from './choose'; +import type { Cleanup } from './cleanup'; +import { deviceUnavailableError, notReusableError } from './errors'; +import { instanceGone } from './emulator'; +import { isStale, leaseFrom } from './lease'; +import { + bindingPathFor, + listBindingFiles, + readBindingFile, + removeBindingFile, + writeBindingFile, +} from './registry'; +import { + deviceIdOf, + deviceNameOf, + localBackendOf, + type Binding, + type BoundDevice, + type DevicePlatform, + type DeviceTools, +} from './types'; + +export type ExplicitCandidate = Extract; +/** `--device`'s value, and the caller's `dev` arguments without it, which a refusal's How repeats (`rerunDevCommand`). */ +export type ExplicitRequest = { query: string; rerun: readonly string[] }; +type Foreign = { file: string; binding: Binding; stale: boolean }; +type Choice = { device: ExplicitCandidate; reused: boolean; transfer: string[] }; +type Unready = AndroidInventory['unready']; +type ExplicitReason = + | 'explicit-not-found' + | 'explicit-not-ready' + | 'explicit-bound' + | 'own-explicit-over-owned'; +type RefusalFacts = { matches?: string[]; root?: string; state?: string; rerun: readonly string[] }; + +/** Pure choice; refusing never changes the current binding. */ +export function chooseExplicitDevice( + own: Binding | null, + others: Foreign[], + candidates: ExplicitCandidate[], + { query, rerun }: ExplicitRequest, + platform: DevicePlatform, + reuseOnly: boolean, + unready: Unready = [] +): Choice { + const matches = candidates.filter( + (device) => + deviceIdOf(device).toLowerCase() === query.toLowerCase() || + (device.backend === 'local-ios' && device.name.toLowerCase() === query.toLowerCase()) + ); + const listed = unready.find(({ serial }) => serial.toLowerCase() === query.toLowerCase()); + if (matches.length === 0 && listed) + throw explicitError('explicit-not-ready', listed.serial, platform, { + state: listed.state, + rerun, + }); + if (matches.length !== 1) + throw explicitError('explicit-not-found', query, platform, { + matches: matches.map(deviceIdOf), + rerun, + }); + const device = matches[0]!; + const id = deviceIdOf(device); + if (own && deviceIdOf(own.device) === id) + return { device: own.device as ExplicitCandidate, reused: true, transfer: [] }; + const holders = others.filter(({ binding }) => deviceIdOf(binding.device) === id); + const blocked = holders.find( + ({ binding, stale }) => + !stale || + (binding.device.backend === 'local-ios' + ? binding.device.origin !== 'explicit' + : binding.device.backend === 'local-android' && binding.device.origin.kind !== 'explicit') + ); + if (blocked) + throw explicitError('explicit-bound', deviceNameOf(device), platform, { + root: blocked.binding.projectRoot, + rerun, + }); + if ( + device.backend === 'local-ios' && + device.name.startsWith('agent-cli ') && + holders.length === 0 + ) + throw explicitError('explicit-not-found', query, platform, { rerun }); + if (reuseOnly) + throw notReusableError( + platform, + own ? { bound: deviceNameOf(own.device), named: deviceNameOf(device) } : undefined + ); + if (own?.device.backend === 'local-android' && own.device.origin.kind === 'spawned') + throw explicitError('own-explicit-over-owned', own.device.serial, platform, { rerun }); + return { device, reused: false, transfer: holders.map(({ file }) => file) }; +} + +/** Read under the lock when acquiring; read-only callers can use the same choice for a plan. */ +export function readExplicitChoice( + projectRoot: string, + platform: DevicePlatform, + candidates: ExplicitCandidate[], + explicit: ExplicitRequest, + reuseOnly: boolean, + tools: DeviceTools, + unready: Unready = [] +) { + const backend = localBackendOf(platform); + const file = bindingPathFor(projectRoot, platform, backend); + const read = readBindingFile(file); + if (read.kind === 'unreadable') + throw deviceUnavailableError('unreadable', { platform, path: file }); + let own = read.kind === 'binding' ? read.binding : null; + if (own?.device.backend === 'local-android' && own.device.origin.kind === 'spawned') { + const device = own.device; + if (instanceGone(tools, own.device.origin) != null) own = null; + // adb can still list the owned instance offline; the instance check is the reuse proof. + else if (!candidates.some((candidate) => deviceIdOf(candidate) === device.serial)) + candidates = [...candidates, device]; + } + const others = listBindingFiles(platform, backend) + .filter((other) => other !== file) + .flatMap((file) => { + const read = readBindingFile(file); + return read.kind === 'binding' + ? [{ file, binding: read.binding, stale: isStale(read.binding, tools.now()) }] + : []; + }); + return { + file, + own, + choice: chooseExplicitDevice(own, others, candidates, explicit, platform, reuseOnly, unready), + }; +} + +/** The own created simulator a new explicit choice deletes, or null. */ +export function replacedByExplicit(own: Binding | null, choice: Choice): Binding | null { + return !choice.reused && own?.device.backend === 'local-ios' && own.device.origin === 'created' + ? own + : null; +} + +export function bindExplicitSection( + projectRoot: string, + platform: DevicePlatform, + candidates: ExplicitCandidate[], + explicit: ExplicitRequest, + reuseOnly: boolean, + tools: DeviceTools, + actions: Cleanup[], + unready: Unready = [] +) { + const { file, own, choice } = readExplicitChoice( + projectRoot, + platform, + candidates, + explicit, + reuseOnly, + tools, + unready + ); + const binding: Binding = { + version: 1, + projectRoot: canonicalizeExistingPath(projectRoot), + device: choice.device, + ...leaseFrom(tools.now()), + }; + writeBindingFile(file, binding); + const replaced = replacedByExplicit(own, choice); + if (replaced) + actions.push({ binding: replaced, action: 'delete', reason: 'replaced by --device' }); + for (const transferred of choice.transfer) removeBindingFile(transferred); + return { binding, action: choice.reused ? ('reused' as const) : ('explicit' as const) }; +} + +function explicitError( + reason: ExplicitReason, + name: string, + platform: DevicePlatform, + { matches = [], root, state, rerun }: RefusalFacts +): CommandError { + const { what, how, line } = refusalText(reason, name, platform, { matches, root, state, rerun }); + const error = new CommandError('DEVICE_UNAVAILABLE', `${what}\nHow: ${line}`); + error.exitCode = + reason === 'explicit-not-ready' ? EXIT_NEEDS_HUMAN : reason === 'explicit-bound' ? 20 : 1; + error.suggestedCommand = how; + error.data = { reason, ...(matches.length ? { matches } : {}), ...(root ? { root } : {}) }; + if (state) error.data.state = state; + return error; +} + +function refusalText( + reason: ExplicitReason, + name: string, + platform: DevicePlatform, + { matches = [], root, state, rerun }: RefusalFacts +): { what: string; how: string; line: string } { + if (reason === 'explicit-not-ready') { + const how = rerunDevCommand(platform, rerun, name); + return { + what: `adb lists ${name} as ${state}, so it cannot be bound.`, + how, + line: `${state === 'unauthorized' ? 'allow USB debugging on the device' : 'reconnect the device'}, then run "${how}".`, + }; + } + if (reason === 'explicit-bound') { + const how = rerunDevCommand(platform, rerun); + return { + what: `${name} is bound to the worktree ${root}.`, + how, + line: `remove --device, then run "${how}".`, + }; + } + if (reason === 'own-explicit-over-owned') { + const how = `${PROGRAM_PREFIX} dev:stop --release`; + return { + what: `This worktree runs the emulator instance ${name}.`, + how, + line: `run "${how}".`, + }; + } + if (matches.length) { + const how = rerunDevCommand(platform, rerun, matches[0]); + return { + what: `No device uniquely matches "${name}"; matches: ${matches.join(', ')}.`, + how, + line: `run "${how}", or pass another match to --device.`, + }; + } + const how = platform === 'ios' ? 'xcrun simctl list devices available' : 'adb devices'; + const listed = platform === 'ios' ? 'name or UDID' : 'serial'; + return { + what: `No device uniquely matches "${name}".`, + how, + line: `run "${how}", then pass a listed ${listed} to --device.`, + }; +} diff --git a/packages/@expo/agent-cli/src/deviceBinding/index.ts b/packages/@expo/agent-cli/src/deviceBinding/index.ts index e30f236f..02a833fc 100644 --- a/packages/@expo/agent-cli/src/deviceBinding/index.ts +++ b/packages/@expo/agent-cli/src/deviceBinding/index.ts @@ -3,11 +3,12 @@ import { BOOT_DEVICE_TIMEOUT_MS } from '../device/bootDevice'; import { bindAndroidSectionAsync, bootEmulatorAsync } from './android'; -import { androidToolsResolve, readAndroidInventoryAsync } from './emulator'; +import { androidToolsResolve, readAndroidBindingInventoryAsync } from './emulator'; import { deviceUnavailableError } from './errors'; import { clearInspectCache } from './inspect'; import { bootSimulatorAsync, readIosInventoryAsync } from './ios'; import { bindingIdentity, bindIosSectionAsync } from './iosAcquire'; +import { bindExplicitSection, type ExplicitRequest } from './explicit'; import { acquireSectionAsync } from './reap'; import { releaseWorktreeDevicesAsync } from './release'; import { bindingPathFor, readBindingFile } from './registry'; @@ -41,17 +42,15 @@ export function devicesDisabled(): boolean { return process.env.AGENT_CLI_NO_DEVICE === '1'; } -/** - * Whether this host can run the platform's local device: simulators exist on macOS only, and an - * emulator instance needs the Android SDK's `adb` and `emulator`. Elsewhere `dev` serves for a - * device somewhere else and binds nothing. - */ +/** Whether local devices are supported: iOS on macOS, Android wherever `adb` resolves. */ export function hostBindsPlatform(platform: DevicePlatform): boolean { return platform === 'ios' ? process.platform === 'darwin' : androidToolsResolve(); } /** The one line a caller prints on stderr: which device the run bound, and how. */ export function acquireLine({ device, action }: AcquireResult): string { + if (action === 'explicit') + return `Bound ${deviceNameOf(device)} (${deviceIdOf(device)}) to this worktree; it will not be shut down or deleted.`; if (device.backend === 'local-android') { const avd = device.origin.kind === 'spawned' ? ` of ${device.origin.avd}` : ''; return action === 'spawned' @@ -68,28 +67,29 @@ export interface AcquireDeviceOptions { /** Any choice but a reuse is refused `not-reusable`. */ reuseOnly?: boolean; tools?: DeviceTools; + explicit?: ExplicitRequest; } +type SectionOptions = { reuseOnly: boolean; tools: DeviceTools; explicit?: ExplicitRequest }; + /** * Bind this worktree's device and boot it. * - * The inventory is read before the lock. Under the lock, in one section: the own binding is - * read, the choice made and written, with `simctl create` or the emulator spawn inside the - * section. iOS revalidates the binding against its inventory snapshot; a changed identity - * retries outside the lock, at most twice. After the section, boot runs on every choice. - * A failed boot lets go of the binding. + * Inventory is read outside the lock; binding reads, choice, writes and allocation run under it. + * A changed binding retries discovery outside the lock. Every choice boots after the section; + * a failed boot lets go of the binding. * * @throws `DEVICE_UNAVAILABLE` with `data.reason`, `DEVICE_REGISTRY_LOCKED`, or the tool error. */ export async function acquireDeviceAsync( projectRoot: string, platform: DevicePlatform, - { reuseOnly = false, tools = defaultTools() }: AcquireDeviceOptions = {} + { reuseOnly = false, tools = defaultTools(), explicit }: AcquireDeviceOptions = {} ): Promise { const section = platform === 'ios' - ? await acquireIosAsync(projectRoot, { reuseOnly, tools }) - : await acquireAndroidAsync(projectRoot, { reuseOnly, tools }); + ? await acquireIosAsync(projectRoot, { reuseOnly, tools, explicit }) + : await acquireAndroidAsync(projectRoot, { reuseOnly, tools, explicit }); clearInspectCache(); const { binding, action, boot } = section; const result = await boot(); @@ -109,9 +109,13 @@ interface Acquired { async function acquireIosAsync( projectRoot: string, - { reuseOnly, tools }: { reuseOnly: boolean; tools: DeviceTools } + { reuseOnly, tools, explicit }: SectionOptions ): Promise { - const { binding, wasBooted, action } = await bindIosAsync(projectRoot, reuseOnly, tools); + const { binding, wasBooted, action } = await bindIosAsync(projectRoot, { + reuseOnly, + tools, + explicit, + }); const udid = deviceIdOf(binding.device); return { binding, @@ -123,12 +127,44 @@ async function acquireIosAsync( async function acquireAndroidAsync( projectRoot: string, - { reuseOnly, tools }: { reuseOnly: boolean; tools: DeviceTools } + { reuseOnly, tools, explicit }: SectionOptions, + forceAvds = false ): Promise { - const inventory = await readAndroidInventoryAsync(tools); - const { binding, action, handle } = await acquireSectionAsync(projectRoot, tools, async () => - bindAndroidSectionAsync(projectRoot, { inventory, reuseOnly, tools }) + const { inventory, isCurrent } = await readAndroidBindingInventoryAsync( + projectRoot, + tools, + explicit !== undefined, + forceAvds ); + const section = await acquireSectionAsync(projectRoot, tools, async (actions) => { + if (explicit === undefined) { + if (!isCurrent()) return null; + return bindAndroidSectionAsync(projectRoot, { inventory, reuseOnly, tools }); + } + const candidates = inventory.runningSerials.map((serial) => ({ + backend: 'local-android' as const, + platform: 'android' as const, + serial, + origin: { kind: 'explicit' as const }, + })); + return { + ...bindExplicitSection( + projectRoot, + 'android', + candidates, + explicit, + reuseOnly, + tools, + actions, + inventory.unready + ), + handle: null, + }; + }); + // Refresh the full inventory outside the lock, at most once. + if (section === null) + return acquireAndroidAsync(projectRoot, { reuseOnly, tools, explicit }, true); + const { binding, action, handle } = section; const serial = deviceIdOf(binding.device); return { binding, @@ -163,7 +199,7 @@ function instanceWatcher( } /** Each inventory belongs to the binding identity observed before the device list was read. */ -async function bindIosAsync(projectRoot: string, reuseOnly: boolean, tools: DeviceTools) { +async function bindIosAsync(projectRoot: string, { reuseOnly, tools, explicit }: SectionOptions) { const file = bindingPathFor(projectRoot, 'ios', 'local-ios'); for (let attempt = 0; attempt < 3; attempt += 1) { const before = readBindingFile(file); @@ -171,13 +207,38 @@ async function bindIosAsync(projectRoot: string, reuseOnly: boolean, tools: Devi const bound = await acquireSectionAsync( projectRoot, tools, - async () => { + async (actions) => { const read = readBindingFile(file); if (read.kind === 'unreadable') { throw deviceUnavailableError('unreadable', { platform: 'ios', path: file }); } if (bindingIdentity(read) !== bindingIdentity(before)) return null; - return bindIosSectionAsync(projectRoot, { inventory, reuseOnly, tools }); + if (explicit === undefined) + return bindIosSectionAsync(projectRoot, { inventory, reuseOnly, tools }); + const candidates = inventory.simulators + .filter((sim) => sim.isAvailable) + .map((sim) => ({ + backend: 'local-ios' as const, + platform: 'ios' as const, + udid: sim.udid, + name: sim.name, + origin: 'explicit' as const, + })); + const result = bindExplicitSection( + projectRoot, + 'ios', + candidates, + explicit, + reuseOnly, + tools, + actions + ); + return { + ...result, + wasBooted: inventory.simulators.some( + (sim) => sim.udid === deviceIdOf(result.binding.device) && sim.state === 'Booted' + ), + }; }, inventory ); diff --git a/packages/@expo/agent-cli/src/deviceBinding/preview.ts b/packages/@expo/agent-cli/src/deviceBinding/preview.ts new file mode 100644 index 00000000..e6e9a5fe --- /dev/null +++ b/packages/@expo/agent-cli/src/deviceBinding/preview.ts @@ -0,0 +1,64 @@ +// @ref llp/0035-one-device-per-worktree.rfc.md §--device on dev +import { readAndroidInventoryAsync } from './emulator'; +import { + readExplicitChoice, + replacedByExplicit, + type ExplicitCandidate, + type ExplicitRequest, +} from './explicit'; +import { readIosInventoryAsync } from './ios'; +import { readBindingFile } from './registry'; +import { defaultTools } from './tools'; +import { deviceIdOf, deviceNameOf, type DevicePlatform, type DeviceTools } from './types'; + +/** + * A plan reads the same choice as acquisition, but never writes, boots or reaps. `reuseOnly` is + * the run's: true while this worktree's dev server runs. + */ +export async function previewExplicitDeviceAsync( + projectRoot: string, + platform: DevicePlatform, + explicit: ExplicitRequest, + reuseOnly: boolean, + tools: DeviceTools = defaultTools() +) { + const android = + platform === 'android' ? await readAndroidInventoryAsync(tools, { avds: false }) : null; + const candidates: ExplicitCandidate[] = !android + ? (await readIosInventoryAsync(tools)).simulators + .filter((sim) => sim.isAvailable) + .map((sim) => ({ + backend: 'local-ios', + platform: 'ios', + udid: sim.udid, + name: sim.name, + origin: 'explicit', + })) + : android.runningSerials.map((serial) => ({ + backend: 'local-android', + platform: 'android', + serial, + origin: { kind: 'explicit' }, + })); + const { own, choice } = readExplicitChoice( + projectRoot, + platform, + candidates, + explicit, + reuseOnly, + tools, + android?.unready ?? [] + ); + const label = `${deviceNameOf(choice.device)} (${deviceIdOf(choice.device)})`; + const parts = [ + choice.reused ? `Will reuse ${label}, already bound to this worktree` : `Will bind ${label}`, + ]; + for (const file of choice.transfer) { + const read = readBindingFile(file); + if (read.kind === 'binding') + parts.push(`, taken over from ${read.binding.projectRoot} (lease expired)`); + } + const replaced = replacedByExplicit(own, choice); + if (replaced) parts.push(`; will delete ${deviceNameOf(replaced.device)} (replaced by --device)`); + return { device: choice.device, summary: `${parts.join('')}.` }; +} diff --git a/packages/@expo/agent-cli/src/followups/__tests__/start-test.ts b/packages/@expo/agent-cli/src/followups/__tests__/start-test.ts index 60217c3c..f07d230e 100644 --- a/packages/@expo/agent-cli/src/followups/__tests__/start-test.ts +++ b/packages/@expo/agent-cli/src/followups/__tests__/start-test.ts @@ -192,13 +192,67 @@ describe(buildStartFollowUps, () => { }); describe(buildStartPlanFollowUps, () => { + it(`should offer the refusal's How command instead of a run of a plan that refuses --device`, () => { + const followups = buildStartPlanFollowUps( + mockPlan({ + explicitDevice: { + refuses: true, + summary: 'agent-cli f5ce27a6 is bound to the worktree /work/other.', + how: 'npx @expo/agent-cli dev --ios --detach --wait-ready', + }, + }), + mockState(), + 'ios' + ); + + expect(ids(followups)).toEqual(['device-refusal']); + expect(followups[0]!.command).toBe('npx @expo/agent-cli dev --ios --detach --wait-ready'); + }); + + it(`should list a refusal's How once when another follow-up carries the same command`, () => { + const followups = buildStartPlanFollowUps( + mockPlan({ + explicitDevice: { + refuses: true, + summary: 'No device uniquely matches "x".', + how: 'npx @expo/agent-cli status --json', + }, + }), + mockState({ + expoGo: { + compatible: false, + reasons: [{ kind: 'custom-native-code', detail: 'Native module.' }], + }, + }), + 'ios' + ); + + expect(ids(followups)).toEqual(['device-refusal']); + }); + + it(`should keep the caller's --device and named --port in the command that runs a bind plan`, () => { + const followups = buildStartPlanFollowUps( + mockPlan({ + explicitDevice: { refuses: false, summary: 'Will bind dogfood s2 (UDID-1).' }, + devServerPort: { port: 8091, movedFrom: null, state: 'named', taken: true }, + }), + mockState(), + 'ios', + ['--ios', '--local', '--plan', '--device', 'dogfood s2', '--port', '8091'] + ); + + expect(followups[0]!.command).toBe( + "npx @expo/agent-cli dev --ios --local --device 'dogfood s2' --port 8091 --detach --wait-ready" + ); + }); + it(`should offer to run the plan that was printed`, () => { const followups = buildStartPlanFollowUps(mockPlan(), mockState()); expect(ids(followups)).toEqual(['dev']); // `dev` requires a platform, so the command states the host's when the caller typed none. expect(followups[0]!.command).toBe( - `npx @expo/agent-cli dev --${process.platform === 'darwin' ? 'ios' : 'android'}` + `npx @expo/agent-cli dev --${process.platform === 'darwin' ? 'ios' : 'android'} --detach --wait-ready` ); }); @@ -214,13 +268,15 @@ describe(buildStartPlanFollowUps, () => { buildStartPlanFollowUps(mockPlan(), mockState(), 'android').find( (followup) => followup.id === 'dev' )!.command - ).toBe('npx @expo/agent-cli dev --android'); + ).toBe('npx @expo/agent-cli dev --android --detach --wait-ready'); expect( buildStartPlanFollowUps(mockPlan(), mockState(), undefined).find( (followup) => followup.id === 'dev' )!.command - ).toBe(`npx @expo/agent-cli dev --${process.platform === 'darwin' ? 'ios' : 'android'}`); + ).toBe( + `npx @expo/agent-cli dev --${process.platform === 'darwin' ? 'ios' : 'android'} --detach --wait-ready` + ); }); it.each(['dev-client-stale', 'bare-stale', 'needs-dev-client'])( @@ -290,7 +346,7 @@ describe(buildStartPlanFollowUps, () => { }); // @ref llp/0015-backend-selection-and-config.rfc.md §The follow-ups of a chosen backend - it(`should offer the account a cloud plan needs, rather than a route it already took`, () => { + it.each([[], ['--device', 'SERIAL'], ['--device=SERIAL']])('cloud run %j', (...deviceArgs) => { const followups = buildStartPlanFollowUps( mockPlan({ rule: 'dev-client-stale', @@ -308,10 +364,17 @@ describe(buildStartPlanFollowUps, () => { }, }), }), - mockState() + mockState(), + 'ios', + ['--ios', '--plan', '--port', '8091', ...deviceArgs] ); expect(ids(followups)).toEqual(['eas-account', 'dev', 'build-freshness']); + expect(followups[1]!.command).toBe( + 'npx @expo/agent-cli dev --ios --eas --port 8091 --detach --wait-ready' + ); + if (deviceArgs.length) + expect(followups[1]!.why).toContain('The named local device is not used.'); expect(followups[0]!.command).toBe('npx @expo/agent-cli whoami'); expect(followups[0]!.why).toContain('an Expo account'); // The plan already went to the cloud, so offering the cloud again says nothing. diff --git a/packages/@expo/agent-cli/src/followups/start.ts b/packages/@expo/agent-cli/src/followups/start.ts index 8672e2d2..baa0ba2c 100644 --- a/packages/@expo/agent-cli/src/followups/start.ts +++ b/packages/@expo/agent-cli/src/followups/start.ts @@ -10,6 +10,8 @@ import type { ProjectState, StartPlan } from '../project/types'; import { localTool, EAS_REQUIREMENT, EAS_WHERE, LOCAL_WHERE } from '../toolchain/runsOn'; import type { ToolchainStatus } from '../toolchain/types'; import { capFollowUps, type FollowUp } from './types'; +import { rerunDevCommand } from '../plan/rerun'; +import { withoutDeviceArgs } from '../dev/deviceOption'; import { easCommandPrefix } from '../utils/easCli'; /** Where `expo start` listens when the command line names no port. */ @@ -309,10 +311,10 @@ export function buildStartPlanFollowUps( * ran a different plan. The *typed* flag rather than the platform the plan settled on, which is * the same split `decideStartPlan` keeps: printing a flag nobody typed would claim they asked. */ - requestedPlatform?: PlanPlatform + requestedPlatform?: PlanPlatform, + /** The caller's `dev` arguments, which the command that runs the plan repeats. */ + callerArgv: readonly string[] = [] ): FollowUp[] { - // `dev` requires a platform, so this flag always states one. - const platformFlag = ` --${requestedPlatform ?? hostPlatform()}`; const followups: FollowUp[] = []; // @ref llp/0015-backend-selection-and-config.rfc.md §The follow-ups of a chosen backend @@ -341,13 +343,28 @@ export function buildStartPlanFollowUps( // is not one a bare `dev` takes: it stops and asks for `--eas`. The line offered here is the one // that runs, and it says what the flag costs. const implicitEas = location?.runsOn === 'eas' && location.selection?.implicit === true; - followups.push({ - id: 'dev', - command: `${PROGRAM_PREFIX} dev${platformFlag}${implicitEas ? ' --eas' : ''}`, - why: implicitEas - ? 'Runs the plan above on EAS — the build there, the app on an EAS Simulator session — which uses EAS credits, so the flag is yours to pass; without it the same command stops and says so.' - : 'Runs the plan above, emitting it again first so nothing runs unannounced.', - }); + const runArgv = implicitEas ? withoutDeviceArgs(callerArgv) : callerArgv; + const device = plan.explicitDevice; + if (device?.refuses) { + if (device.how) + followups.push({ + id: 'device-refusal', + command: device.how, + why: 'Running the plan above stops at the --device refusal printed under its steps; this is the next step that refusal names.', + }); + } else + followups.push({ + id: 'dev', + // `dev` requires a platform, so the command always states one. + command: rerunDevCommand( + requestedPlatform ?? hostPlatform(), + implicitEas ? ['--eas', ...runArgv] : runArgv + ), + why: implicitEas + ? 'Runs the plan above on EAS — the build there, the app on an EAS Simulator session — which uses EAS credits, so the flag is yours to pass; without it the same command stops and says so.' + + (runArgv.length === callerArgv.length ? '' : ' The named local device is not used.') + : 'Runs the plan above, emitting it again first so nothing runs unannounced.', + }); if (BUILDING_RULES.includes(plan.rule)) { followups.push({ @@ -365,7 +382,12 @@ export function buildStartPlanFollowUps( }); } - return capFollowUps(followups); + return capFollowUps( + followups.filter( + (followup, index) => + followups.findIndex((first) => first.command === followup.command) === index + ) + ); } /** diff --git a/packages/@expo/agent-cli/src/plan/__tests__/emit-test.ts b/packages/@expo/agent-cli/src/plan/__tests__/emit-test.ts index bb244239..a2114955 100644 --- a/packages/@expo/agent-cli/src/plan/__tests__/emit-test.ts +++ b/packages/@expo/agent-cli/src/plan/__tests__/emit-test.ts @@ -35,6 +35,7 @@ describe(emitStartPlan, () => { reasons: plan.reasons, buildLocation: null, devServerPort: null, + explicitDevice: null, }); }); @@ -50,6 +51,14 @@ describe(emitStartPlan, () => { ); }); + it(`should carry the --device choice on the event, null without one`, () => { + const explicitDevice = { refuses: true, summary: 'x', how: 'h' } as const; + + emitStartPlan({ ...plan, explicitDevice }, { mode: 'plan' }); + + expect(event).toHaveBeenCalledWith('start_plan', expect.objectContaining({ explicitDevice })); + }); + it(`should print the plan table`, () => { emitStartPlan(plan, { mode: 'smart' }); diff --git a/packages/@expo/agent-cli/src/plan/__tests__/format-test.ts b/packages/@expo/agent-cli/src/plan/__tests__/format-test.ts index 76c2c62d..b6349a98 100644 --- a/packages/@expo/agent-cli/src/plan/__tests__/format-test.ts +++ b/packages/@expo/agent-cli/src/plan/__tests__/format-test.ts @@ -51,6 +51,30 @@ describe(formatTimeClass, () => { }); describe(formatStartPlan, () => { + it(`should print what --device does as its own line under the steps, above Why`, () => { + const lines = formatStartPlan({ + ...plan, + explicitDevice: { refuses: true, summary: 'No device uniquely matches "x".', how: null }, + }) + .split('\n') + .map(strip); + + const device = lines.indexOf(' Device: Will refuse — No device uniquely matches "x".'); + expect(device).toBeGreaterThan(lines.findIndex((line) => line.includes('expo run:ios'))); + expect(device).toBeLessThan(lines.indexOf(' Why')); + }); + + it(`should print a binding plan's summary after Device with no verb of its own`, () => { + const lines = formatStartPlan({ + ...plan, + explicitDevice: { refuses: false, summary: 'Will bind Sim (UDID-1).' }, + }) + .split('\n') + .map(strip); + + expect(lines).toContain(' Device: Will bind Sim (UDID-1).'); + }); + it(`should report the rule and the target`, () => { expect(formatStartPlan(plan)).toMatch(/dev-client-stale/); expect(formatStartPlan(plan)).toMatch(/target: dev-client/); diff --git a/packages/@expo/agent-cli/src/plan/emit.ts b/packages/@expo/agent-cli/src/plan/emit.ts index 5e942c04..10509c62 100644 --- a/packages/@expo/agent-cli/src/plan/emit.ts +++ b/packages/@expo/agent-cli/src/plan/emit.ts @@ -57,6 +57,7 @@ export function emitStartPlan( // the sort of thing it would otherwise go and shell out to find. buildLocation: plan.buildLocation, devServerPort: plan.devServerPort ?? null, + explicitDevice: plan.explicitDevice ?? null, }); if (print === 'none') { return; diff --git a/packages/@expo/agent-cli/src/plan/events.ts b/packages/@expo/agent-cli/src/plan/events.ts index a3c94043..50b62409 100644 --- a/packages/@expo/agent-cli/src/plan/events.ts +++ b/packages/@expo/agent-cli/src/plan/events.ts @@ -28,6 +28,8 @@ declare module '2g' { buildLocation: PlanBuildLocation | null; /** @see StartPlan.devServerPort */ devServerPort: StartPlan['devServerPort'] | null; + /** @see StartPlan.explicitDevice */ + explicitDevice: StartPlan['explicitDevice'] | null; }; /** One plan step is about to be spawned. */ 'cli:start_plan_step': { id: string; argv: string[]; index: number; total: number }; diff --git a/packages/@expo/agent-cli/src/plan/format.ts b/packages/@expo/agent-cli/src/plan/format.ts index 5251879a..cdae46d5 100644 --- a/packages/@expo/agent-cli/src/plan/format.ts +++ b/packages/@expo/agent-cli/src/plan/format.ts @@ -48,6 +48,11 @@ export function formatStartPlan(plan: StartPlan): string { ); } + if (plan.explicitDevice) { + const { refuses, summary } = plan.explicitDevice; + lines.push('', chalk` {bold Device:} ${refuses ? chalk`{red Will refuse} — ` : ''}${summary}`); + } + if (plan.buildLocation) { lines.push('', ` ${formatBuildLocation(plan.buildLocation)}`); } diff --git a/packages/@expo/agent-cli/src/plan/rerun.ts b/packages/@expo/agent-cli/src/plan/rerun.ts new file mode 100644 index 00000000..d4665416 --- /dev/null +++ b/packages/@expo/agent-cli/src/plan/rerun.ts @@ -0,0 +1,25 @@ +// @ref llp/0035-one-device-per-worktree.rfc.md §--device on dev +import { PROGRAM_PREFIX } from '../programName'; +import { quoteArgument } from '../utils/consent'; +import { isPlatformFlag } from './platformFlags'; +import type { PlanPlatform } from './types'; + +/** + * The caller's `dev` command again: the platform first, `--device ` added, `--plan` + * dropped, and `--detach --wait-ready` added to a foreground command, which would block an agent. + */ +export function rerunDevCommand( + platform: PlanPlatform, + callerArgv: readonly string[], + device?: string +): string { + const separator = callerArgv.indexOf('--') < 0 ? callerArgv.length : callerArgv.indexOf('--'); + const own = callerArgv.slice(0, separator); + const args = [ + ...own.filter((arg) => !isPlatformFlag(arg) && arg !== '--plan'), + ...(device === undefined ? [] : ['--device', device]), + ...(own.includes('--detach') ? [] : ['--detach', '--wait-ready']), + ...callerArgv.slice(separator), + ]; + return `${PROGRAM_PREFIX} dev --${platform}${args.map((arg) => ` ${quoteArgument(arg)}`).join('')}`; +} diff --git a/packages/@expo/agent-cli/src/project/types.ts b/packages/@expo/agent-cli/src/project/types.ts index 23dcdf98..67787343 100644 --- a/packages/@expo/agent-cli/src/project/types.ts +++ b/packages/@expo/agent-cli/src/project/types.ts @@ -134,6 +134,13 @@ export interface StartPlan { * @see llp/0004-smart-start-and-project-state.rfc.md §A busy port is not a step only a person can complete */ devServerPort?: DevServerPort; + /** + * What `dev --device` does to devices when this plan runs, or the refusal it will meet, with the + * refusal's How command. Read without writes or boots. Absent without `--device`. + */ + explicitDevice?: + | { refuses: false; summary: string } + | { refuses: true; summary: string; how: string | null }; } /** diff --git a/packages/@expo/agent-cli/src/utils/consent.ts b/packages/@expo/agent-cli/src/utils/consent.ts index 95a9f8be..b00a0885 100644 --- a/packages/@expo/agent-cli/src/utils/consent.ts +++ b/packages/@expo/agent-cli/src/utils/consent.ts @@ -27,7 +27,7 @@ const YES_FLAGS = new Set(['--yes', '-y']); const NEEDS_QUOTING = /[^\w@%+=:,./-]/; /** One argument, as a shell would have to be given it to arrive unchanged. */ -function quoteArgument(argument: string): string { +export function quoteArgument(argument: string): string { if (argument !== '' && !NEEDS_QUOTING.test(argument)) { return argument; }