Repository navigation
[agent-cli] Add dev:stop --release for local worktree devices - #147
Conversation
b0c27d9 to
bc10ba7
Compare
bc10ba7 to
f7033ac
Compare
41d45c1 to
656b3f5
Compare
6176fa5 to
c9520d6
Compare
| } | ||
| if (exitCode !== EXIT_OK) { | ||
| Log.error(explainFailure(report, options)); | ||
| Log.error(report.deviceError ?? explainFailure(report, options)); |
There was a problem hiding this comment.
[bug] When dev:stop --release fails to stop the server, deviceError is set and this line prints it instead of explainFailure. The stop's How line is dropped. still-running loses the longer --timeout / --signal SIGKILL line. foreign-dev-server loses the --force line. deviceError is one sentence and has no How line. JSON still has detail, but stderr is what carries the How line for both human and --json runs.
Suggestion: If the stop itself failed, print explainFailure and then the device sentence. Use deviceError alone only when the server stop succeeded and the release is what failed, and give that sentence its own How line.
| if (options.release) { | ||
| // A requested port may be idle while this worktree's server runs on another port. | ||
| // Read again after stopping: a replacement server may now hold the same lock. | ||
| if (stop.reason === 'foreign-dev-server') { |
There was a problem hiding this comment.
[bug] reason === 'foreign-dev-server' is not "a dev server answered". stopUnlockedDevServerAsync sets it for any busy --port once it refuses to signal, including a listener that did not answer packager-status:running. The lifecycle test stubs that case (fetch refused, isPortInUseAsync true) and still expects release not to run. This branch then says a dev server is on that port and returns before readDevServerLockAsync. A worktree whose own server is already gone cannot --release while --port is merely occupied. A worktree whose server is still on another port is not told that. The plan says to recheck this worktree's lock and to release when that server is stopped or absent. The own-server message in the next branch never runs.
Suggestion: Recheck readDevServerLockAsync first, for every reason. If that lock is held, keep the devices and say this worktree's server is still running. If it is not held, release. If a real foreign Metro should still block release, gate that on the port answering as an Expo dev server and say so. Do not treat a non-dev listener as a dev server.
| return false; | ||
| } | ||
| if (origin.kind === 'spawned') { | ||
| if (origin.kind === 'spawned' || listing?.kind === 'missing') { |
There was a problem hiding this comment.
[bug] An explicit Android binding is deleted when adb get-state stderr matches "not found". That is what adb prints for a shut-down emulator and for an unplugged phone, not only for a serial that no longer exists. offline and unauthorized keep the file. Powered off does not. The next dev cannot reuse the named device. iOS does not do this: a Shutdown simulator stays listed and the file is only expired. The RFC's risk 8 assumes the explicit binding is still there after the instance exits. The unit test only covers get-state answering device.
Suggestion: For explicit, expire the lease and keep the file whether or not get-state finds the serial. Remove the file for spawned, or only when something other than adb's "not found" shows the device was deleted.
| id: serial, | ||
| name: origin.kind === 'spawned' ? `${origin.avd} on ${serial}` : serial, | ||
| released: true, | ||
| released: !killed?.failed, |
There was a problem hiding this comment.
[bug] A spawned binding's file is removed under the lock (line 127) before killEmulatorIfOurs. If kill throws, failed is set and this line reports released: false, but the record is already gone. stopDevices exits 20. A retry finds no binding, releases nothing, and exits 0. The emulator is still running and the port stays taken. no-free-port tells the user to run dev:stop --release, which now succeeds without killing anything.
Suggestion: If kill throws and the pid is still alive, write the binding back before returning released: false, so a retry still has the pid, avd, and port. A pid that is already dead can drop the file and report released: true.
…ce that is gone loses its file (llp/0033 §Release) A release reported `released: true` even when the shutdown or the kill failed; `released` is now false then. An `explicit` Android binding whose serial is gone loses its file instead of keeping an expired lease. `AcquireResult` carries the binding that the acquisition wrote. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ain dev:stop keeps them (llp/0033 §Release) Nothing gave a device back. A created simulator stayed booted and a spawned emulator kept its console port after the lease expired, and the `no-free-port` refusal told the user to stop an emulator by hand. `dev:stop --release` now releases the worktree's local bindings once its dev server is gone: it parks a created simulator and kills a spawned emulator instance. It keeps the devices when this worktree's dev server still runs, including a replacement server that took the lock during the stop. The `no-free-port` refusal names `dev:stop --release`. A plain `dev:stop` keeps the devices, because the next step is usually `dev` again, and reports them with `released: false`. `devices` and `deviceError` are in every `dev:stop` JSON object. A failed release sets `deviceError` and exit 20. The live runner adds `--release` to every `dev:stop`, so a live run leaves no device behind. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… (llp/0033 §Release) `stopLifecycle-test.ts` did not stub `fetch`, so `devStopAsync` asked `http://127.0.0.1:<port>/status` on the real machine. A live Metro on 8082 answered and the first test failed. The test now replaces `globalThis.fetch` with a refused connection and restores it after each test, as `stopAsync-test.ts` does. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ign dev server (llp/0033 §Release) With no dev server of this worktree running, `dev:stop --release --port N` on a port another dev server answers on kept the devices, which is right, but said "this worktree's dev server is still running", which is not. The release now gets the stop result and names the port for `foreign-dev-server`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s earlier releases (llp/0033 §Release) Review of #147. An own binding file that does not parse was skipped by both the release and the kept list, so `dev:stop --release` exited 0 with no devices and no error while a device might still run; it now reports the `unreadable` refusal, which names the file. The release ran both platforms in one call, so an Android failure discarded the iOS result and reported a parked simulator as kept; each platform now releases on its own, and a failure keeps only that platform's device. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… file whatever adb says (llp/0035 §Records) Review of #147. Release asked `adb get-state` about an `explicit` serial and removed the file when adb answered "not found". adb answers that for a phone that is off and for a stopped emulator, so the next `dev` could not reuse the named device. Release no longer asks adb: an `explicit` binding always keeps its file with an expired lease. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…kill throws and the emulator still runs (llp/0035 §Records) Review of #147. Release removes a `spawned` binding's file under the lock and kills the emulator after it. When the kill threw, release reported `released: false` with no file left, so a retry found nothing, exited 0, and the emulator kept its port. Release now writes the binding back with an expired lease when the pid is still alive, through `restoreBindingIfGoneAsync`, which leaves a newer binding alone. A pid that is dead after a failed kill keeps the removal and reports `released: true`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tree's lock, and keeps devices only for a real dev server (llp/0035 §Release) Review of #147. Release kept the devices for every `foreign-dev-server`, and that reason covers any busy `--port`, also a listener that does not answer as a dev server. The own lock was never read on that path, so a worktree whose server was gone could not release, and a worktree whose server ran on another port was not told so. Release now reads the own lock first for every stop result. When the lock is held, it keeps the devices and says this worktree's server is still running. When it is not, a port that answered as an Expo dev server keeps the devices with its own sentence and How line; any other listener does not block the release. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…p's or the release's (llp/0035 §Output and errors) Review of #147. With `--release`, a failed stop also sets `deviceError`, and stderr printed that sentence instead of the stop's What/Why/How, so the next action was lost. The release sentences had no How line either. A failed stop now prints the device sentence first and the stop's What/Why/How after it. When only the release failed, stderr is the device sentence with its own How line: run `dev:stop --release` again, or with no `--port` while this worktree's server runs elsewhere. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e means a release that failed, in JSON and on the human line (llp/0035 §Output and errors) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…l (llp/0035 §Output and errors) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
c9520d6 to
87ada2c
Compare
…b uses it (llp/0035 §iOS binding) (#134) PR 3a of llp/0035 (one device per worktree), based on main after the prep in #133 merged. Review bottom-up in stack #150: #134 → #137 → #146 → #147 → #148 → #149 → #151 → #152. EAS session binding (#139) is separate from this stack. The five design files `llp/0035` to `llp/0035 §EAS session binding` land in the first commit. ## The question How does a worktree get an iOS simulator nobody else has, and how do the other verbs find it? ## What changes for an agent - `dev --ios` binds a simulator this CLI created for the worktree, named `agent-cli <digest>`, boots it with `simctl bootstatus -b`, and runs every `expo run:ios` and install step with `--device <udid>`. The binding is a JSON file under `<expo home>/agent-cli/bindings/`, held by a 60 min lease that use renews; every write runs under one machine-wide lock. Two worktrees get two simulators. - `navigate`, `runtime:reload`, `runtime:stop`, `status` and the installed-app readers read the binding and never list devices to choose one. Without a platform flag, any `up` binding wins. A worktree with no usable binding is refused with `NO_BOUND_DEVICE` and the one `dev` command that fixes it. Main's `NO_DEVICE` and `NO_IOS_DEVICE` are gone. - `dev --detach` on a running server boots the worktree's own parked simulator and refuses `not-reusable` otherwise. A device refusal in the detached child reaches the agent with its own code, exit and How line, through the child verdict. - `smoke --ios` takes its simulator from the binding and leaves it bound; it no longer shuts down the simulator it booted. - Android is unchanged until 3b: its first-`adb`-device rung is injected into the rung loop. - The EAS fallback without `--eas` accepts the `.env.eas-simulator` id only, until PR 5 binds sessions. ## Commits 1. The module `src/deviceBinding/` with its unit tests and the five LLPs. No caller. 2. The wiring: the resolver callback, `withDevice`, the detach relay, the rung loop in every reader, smoke, the deletions. 3. The e2e suite, the LLP sentences that said "first booted", and the changelog. ## Decisions taken while implementing (to fold back into llp/0035 §iOS binding) - The child verdict carries the error code and exit (`Error code <CODE> exit <N>`), written only when `__EXPO_AGENT_CLI_DETACHED=1`. - A failed boot is `DEVICE_UNAVAILABLE` reason `boot-failed`, exit 20, "run the command again". - `withDevice` runs after the `--plan` return and the refusals; main's port block stays before `--plan`, because the plan prints the port. - Smoke's install phase now honours `installNeeded` from the boot result: a created simulator's disk holds nothing. - `status`'s next action still names the plan's `dev --ios` command, not the `--detach --wait-ready` form; changing it touches every status snapshot. - `src/deviceBinding/` has `ios.ts`, `errors.ts`, `events.ts` and `tools.ts` beyond the RFC's file list, all within the 250-line budget. ## Verification - Typecheck, lint (no new warnings), `bun run format:check`. - Unit: 229 files, 4842 tests passed. Every test named in llp/0035 §iOS tests exists under that name. - Build, then e2e: 41 files, 835 tests passed; the device cases run with `AGENT_CLI_NO_DEVICE=0` on macOS. - Not yet run: the live checks of llp/0035 §iOS tests (two real worktrees, `navigate` wall time, a cold `simctl create`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…137) Each worktree gets its own read-only Android emulator instance on a free console port. `dev --android` acquires it, build/install steps receive its serial, and device commands use the recorded binding instead of whichever emulator happens to be first. The registry reserves ports held by every listed emulator, including booting/offline instances. Cleanup checks whole `-avd` and `-ports` values before killing an owned process, and validates the binding identity so release or failed-boot cleanup cannot remove a replacement binding. Windows install-test coverage is restored. ## Stack Review bottom-up in stack #150: #134 → #137 → #146 → #147 → #148 → #149 → #151 → #152. EAS session binding (#139) is separate from this stack. Based on #134. The four review-fix commits are included in this PR, ending at `95a1184a`. ## Validation The split was previously checked with typechecks, formatting, lint and unit tests on each layer, plus focused lifecycle/dev/stop/smoke integration tests and local-device dogfooding. This publication preserves the reviewed branch tip; tests were not rerun for the stack and PR-description changes. --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Devices have a 60-minute lease, but a long-running Metro or native build may go longer without a device command. Renew the worktree's leases every five minutes while the dev-server or build runner is active, including bindings created after the runner starts. Renewal stops with the runner; a lost lease is reported once. The wrappers cover `dev`, detached `dev`, `start`, and build steps. `AGENT_CLI_NO_DEVICE=1` disables renewal. Timer and wiring tests cover lease renewal. The design documents describe lifecycle behavior and dependencies. ## Stack Review bottom-up in stack #150: #134 → #137 → #146 → #147 → #148 → #149 → #151 → #152. EAS session binding (#139) is separate from this stack. Based on #137 (`vonovak/android-device-instance`). ## Validation - Unit: 5,006 pass, including timer tests and two wiring tests that fail if either wrapper is removed. - E2E: `dev-test` and `dev-detach-test`, 73 of 73. - Dogfooding: with Metro running and no verb in between, the binding's `expiresAt` moved forward at +5 and +10 minutes (read from the binding file). - Documentation update: repository formatting, document links and stale-reference checks pass. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
An unattended smoke run should return the device resources it acquired. Release a device bound by smoke's started dev server through `dev:stop --release`, or release the boot phase's own binding when reusing an existing server. A device already bound and running before the run stays available; `--no-start` acquires and releases nothing. Name the bound device and its cleanup result in human output and `environment.boundDevice`. Follow-up commands account for whether the run stopped its server or released its device, and preserve the caller's port. Tests cover replacement bindings, reused servers, kept devices and iOS/Android restart guidance. ## Stack Review bottom-up in stack #150: #134 → #137 → #146 → #147 → #148 → #149 → #151 → #152. EAS session binding (#139) is separate from this stack. Based on #147 (B, `vonovak/lifecycle-2-release`). ## Validation This layer is rebased over the stop-reporting correction in #147; its smoke implementation is unchanged. At the integrated #152 tip: 5,138 unit tests across 241 files and 136 built-CLI dev-detach/status/device-lifecycle/stop tests passed. Build, typecheck, lint (existing warnings only), and formatting passed. The selected cold simulator creation/install/release smoke test passed in 127.55 seconds. The full smoke suite was not completed; partial bounded runs are not a full-suite pass. These fix regressions use subprocess/device stubs; native acceptance was not repeated for this source-fix pass. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
A deleted worktree cannot run `dev:stop --release`, and expired leases leave device resources behind. Device acquisition and `dev:stop` reap other worktrees' stale bindings: delete created simulators for deleted roots, shut down expired simulators when an iOS inventory is available, and end owned Android emulator instances. Explicit devices are preserved. Registry changes happen under the lock; device actions run afterward, including when acquisition refuses. Results appear under `reaped` in `dev:stop --json`. Cross-worktree results identify `projectRoot` and report `devServerCleanup: "not-checked"`; deleted-root human output also explains that CLI/Metro processes may remain. A late last-build metadata write skips an already deleted project root. ## Scope and limits Reaping uses lease expiry as its liveness signal. After a long host sleep it can park a simulator while Metro still runs. This PR cleans device resources; it does not terminate orphan CLI/Metro processes after worktree deletion. Safe server cleanup needs durable server ownership information and remains a separate follow-up. The metadata guard does not make the existing write sequence atomic against simultaneous directory deletion. ## Stack Review bottom-up in stack #150: #134 → #137 → #146 → #147 → #148 → #149 → #151 → #152. EAS session binding (#139) is separate from this stack. Based on #148. The first commit extracts iOS acquisition; subsequent changes implement reaping and clarify its reporting. ## Validation - 152 device-binding/last-build/stop-lifecycle unit tests and two built-CLI device-lifecycle tests passed. Reporting and deleted-root write regressions failed before the fixes. - At the integrated #152 tip: 5,138 unit tests across 241 files and 136 built-CLI dev-detach/status/device-lifecycle/stop tests passed. Build, typecheck, lint (existing warnings only), and formatting passed. The selected cold simulator creation/install/release smoke test passed in 127.55 seconds. The full smoke suite was not completed; partial bounded runs are not a full-suite pass. These fix regressions use subprocess/device stubs; native acceptance was not repeated for this source-fix pass. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…evice on dev) (#151) ## Why A person wants the agent on their own simulator or USB phone. Without this PR there is no way to name one. Two worktrees must not share it: if worktree A holds `dogfood-explicit`, `dev --device dogfood-explicit` in worktree B must refuse. No release or reap may shut it down. ## What changes - `dev --device <name|udid|serial>` binds that device as `explicit`. Release and reap never shut it down, delete it or kill it. - It refuses a device another live worktree holds (exit 20), an ambiguous name (lists the matches), and a second device while this worktree's dev server runs (names both). - It takes over another worktree's expired binding, and deletes the own created simulator it replaces. - An Android serial needs only `adb devices -l`, not the emulator package. - `dev --plan --device` prints a `Device:` line (bind, take over, delete, or refuse), and JSON `explicitDevice`. A refusing plan offers the refusal's How and exits 0. - A How that repeats the caller's `dev` command keeps its flags and adds `--detach --wait-ready`. ## Review guide 1. llp/0033 §--device on dev. 2. `src/deviceBinding/explicit.ts`. Look closely at `readExplicitChoice` and `bindExplicitSection` (transfer and delete under the lock). 3. `index.ts`, `preview.ts`, `emulator.ts`, `src/plan/rerun.ts`, `resolveOptions.ts` (reads `__EXPO_AGENT_CLI_CALLER_ARGV` once). 4. Tests: `explicit-test.ts`, `resolveOptions-test.ts`, `devAsync-test.ts`. ## Stack Review bottom-up in stack #150: #134 → #137 → #146 → #147 → #148 → #149 → #151 → #152. EAS session binding (#139) is separate from this stack. Stacked on PR D (`vonovak/lifecycle-4-reap`). ## Validation - Unit: 5076 pass. The adb-only test failed in preview and acquisition before the fix. - E2E on macOS: `device-lifecycle-test` gains the plan block; the log records e2e green, without counts. - Dogfood: sessions 1, 2, 3, 5 and 7. A concurrent `--device` race had one winner. Session 7 checked the How lines. - Not run: no USB phone. Live tier with `AGENT_CLI_LIVE_UDID` not run. ## Notes - Commit 2 fixes Kudo's finding (explicit Android needed the emulator package). - `explicitDevice` gives the device facts as a sentence, not fields (M3, open). - General changes: plan follow-ups drop repeated commands, and `PORT_TAKEN_AFTER_RETRY` uses `rerunDevCommand`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…tatus sends an unbound worktree to dev (llp/0035 §Lifecycle output) (#152) `status` can recommend the wrong platform or lose a chosen Metro port when a startup has not published its lock or its server has stopped. It can also recommend device commands for a worktree whose device lease has expired. This PR reports the worktree's binding and produces recovery commands using the known development context. ## Behavior - `status --json` exposes each binding's platform, backend, identity, origin, state and expiry. Detached `dev` names its bound device in human output and under `device` in JSON. - A real `dev` invocation retains only parsed platform, requested port, device, build backend and app target in `.expo/agent-cli-last-dev.json`. Dry runs do not write it. This record is a recovery hint, not process-liveness evidence. - Status chooses the explicit internal platform override, then the saved request, then a sole bound platform, then the project default. A different explicit platform discards the other saved choices. No new public platform flags are added to `status`. - Recovery preserves an explicit local status port, the current own-server port, or the saved requested port. A port observed serving another project is omitted. Remote URL ports are not treated as local Metro requests. - Recovery uses #151's quoted retry builder and restores the dev plan steps when replacing a verification action. `rule` and `target` retain their documented plan meaning. - Rebinding guidance preserves the connected-app, web, EAS-session and unsupported-device-host exceptions. A worktree without a binding but with its own running server is told to stop that server before restarting to bind a device. ## Stack Review bottom-up in stack #150: #134 → #137 → #146 → #147 → #148 → #149 → #151 → #152. EAS session binding (#139) is separate from this stack. Based on #151. Review `src/dev/lastRequest.ts`, `src/status/statusAsync.ts`, `src/deviceBinding/summary.ts`, and detached dev's device reporting. ## Validation - Android startup without a lock, sole Android binding, stopped-request recovery, explicit overrides, device-name quoting, corrupt/unwritable context, dry-run behavior, foreign ports and restored plan steps have regression coverage. - A built-CLI Android start → stop → status test verifies retained platform, port and target choices. - At the integrated #152 tip: 5,138 unit tests across 241 files and 136 built-CLI dev-detach/status/device-lifecycle/stop tests passed. Build, typecheck, lint (existing warnings only), and formatting passed. The selected cold simulator creation/install/release smoke test passed in 127.55 seconds. The full smoke suite was not completed; partial bounded runs are not a full-suite pass. These fix regressions use subprocess/device stubs; native acceptance was not repeated for this source-fix pass. The saved request does not establish that a native build is running or fix separate startup-ownership issues. Device origin describes how it became bound, not what the current invocation did. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Stopping Metro can leave the worktree's devices allocated.
dev:stop --releaseshuts down and parks a created iOS simulator or ends a spawned Android emulator after confirming this worktree's server has stopped. Plaindev:stopkeeps devices; explicitly selected devices are preserved. A remaining or replacement server prevents release.When stop refuses because ownership cannot be verified, JSON now reports the observed process liveness and port occupancy instead of falsely reporting that both are gone. Unmeasured facts are
null. Missing lock evidence no longer claims the server was never started by the CLI. Signal authorization and device-release policy are unchanged.Stack
Review bottom-up in stack #150: #134 → #137 → #146 → #147 → #148 → #149 → #151 → #152. EAS session binding (#139) is separate from this stack.
Based on #146. Binding-identity and failed-boot cleanup protections come from #137.
Validation