feat(exec): hold the device claim for an external command's lifetime

[thymikee]

Summary

agent-device exec --platform ios --udid <UDID> -- expo run:ios --device <UDID> holds the host-global device claim for the whole lifetime of an external command, so builds that install through the platform tools directly are fenced against agent-device sessions on the same device. Fixes #3263.

• The claim is taken on the wrapper CLI process's own identity (no daemon route: a build outlasts any request envelope, and owner-liveness proofs already reconcile a dead owner). New honest wrapper-exclusive claim policy; the registry never claims the wrapper goes through daemon admission.
• Claim records gain ownerKind: 'wrapper'; the daemon-supersession proof (which reads a daemon.json that can only ever describe daemons) no longer steals live wrapper claims, and claims under a dedicated <stateDir>/exec dir reconcile as owner-process-dead when the wrapper is killed.
• The typed conflict builder moved to daemon-contracts so exec throws exactly the refusal the daemon admission seam returns.
• Child runs with stdio inherit in its own process group; every received SIGINT, SIGHUP, SIGQUIT or SIGTERM forwards to the group (the first one owns the exit code) and the wrapper releases the claim only after the child exits — so a closed terminal or dropped SSH stops the build instead of stranding the fence. Exit code propagates (128+signum for signal exits, 127 for a missing command), and release is attempted on every exit path. macOS and Linux hosts only — exec refuses to start on win32 (UNSUPPORTED_PLATFORM, reason: unsupported-host-platform).
• 25 files; docs: commands.md, sessions.md, README (matching selectors fence the very device the build targets). No scope expansion beyond the claim-store owner-kind fix the wrapper needs.

agent-device exec --platform ios --udid <udid> -- expo run:ios --device <udid>
agent-device exec --platform android --serial emulator-5554 -- adb install app-debug.apk
agent-device device status  # session "exec:<command>" holds the device

Validation

Tested at 0d139bc (review rounds 1–3, rebased onto 4a8abed7b9).

• pnpm check:affected --run --head 0d139bce0 at the exact commit: all runnable checks passed (receipt .tmp/validation/run-XpKV1T/receipt.json).
• Rebase note: main's claim schema v3 (install and reinstall from another session in the same daemon replace the app on a claimed device #3345, via docs(agents): write agent docs in ASD-STE100 and remove misleading docs #3723-era work) made the owner kind a required claimOwnerKind discriminant, so the wrapper kind now rides that field (claimOwnerKind: 'wrapper') instead of a parallel optional ownerKind, and the supersession guard checks claimOwnerKind !== 'wrapper'. The wrapper-claim tests pin the fence after the merge: live wrapper claim not stealable from a registration, foreign acquire refused while the wrapper runs, release on exit.
• Fail-before/pass-after: with the ownerKind supersession guard reverted, the wrapper-steal regression fails (acquired instead of conflict); with the forwarded-signal list narrowed back to SIGINT/SIGTERM, the real-SIGHUP case fails (claim stranded at a dead pid, child alive). Mutations (no release, no signal code, no forward, drop-second-signal) each fail their test for the named reason.
• Unit: pnpm typecheck, pnpm lint, check:layering, check:doc-prose, help-conformance, command-doc-coverage, full packages/daemon (2770 tests), daemon-contracts, command-registry, exec suites — green. The three POSIX signal cases share one test.skipIf(IS_WINDOWS) condition.
• Live simulator at 2665364 through the real CLI dispatch (iPhone 17 Pro ExecIssue3263R3, iOS 27.0, created and deleted for the check; 5509d9a only adds a Windows test skip): during exec … -- sleep 120, device status showed live session=exec:sleep and a second exec refused with DEVICE_IN_USE naming session "exec:sleep"; SIGTERM → rc 143 and claims store empty; SIGHUP → rc 129, the child sleep in the wrapper's group was gone, and the claims store was empty once the wrapper exited; after exit a fresh exec succeeded (rc 0).
• Fence re-check at the rebased head 0d139bc through the real CLI (fresh iPhone 17 Pro ExecFenceR4, created and deleted for the check): status showed live session=exec:sleep, a second exec refused with DEVICE_IN_USE naming the holder, SIGTERM → rc 143 with the store empty, and a fresh exec after exit succeeded rc 0 — the conflict resolution keeps the claim held for the external command's lifetime.
• Not verified: provider-leased cloud targets only hit the local inventory path (never claimable, by design); Windows is refused by policy, not exercised; the child has no controlling terminal, so tools that open /dev/tty (sudo, credential prompts) may fail — untested; Ctrl-Z stops the wrapper but not the child (job control for a session leader's detached child would need a terminal the child doesn't share).

🤖 Generated with Claude Code

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	5.44 MB	5.44 MB	+5.3 kB
Package (unpacked)	5.44 MB	5.44 MB	+5.3 kB
Package (download)	1.64 MB	1.64 MB	+1.9 kB

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	45.6 ms	42.7 ms	-3.0 ms
CLI --help	84.3 ms	81.5 ms	-2.8 ms

[cubic-dev-ai]

1 issue found across 25 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/cli/commands/exec.ts">

<violation number="1" location="src/cli/commands/exec.ts:139">
P2: Relative Windows executables such as ` .\gradlew.bat` are not resolved because this check recognizes only `/`; the spawn helper then rejects the backslash path. Recognize both path separators before calling `path.resolve`.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[github-actions]

PR Preview Action v1.8.1
🚀 View preview at https://callstack.github.io/agent-device/pr-preview/pr-3738/
Built to branch gh-pages at 2026-10-19 16:15 UTC. Preview will be ready when the GitHub Pages deployment is complete.

[cubic-dev-ai]

All reported issues were addressed across 6 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[thymikee]

The exec wrapper can die on terminal hangup while the build it wraps keeps running, so the device claim does not last as long as the command (reviewed at 06e8600).

The child is spawned with detached: true (exec.ts:165). On POSIX that puts it in a new session with no controlling terminal. The wrapper forwards only SIGINT and SIGTERM (exec.ts:197). If the user closes the terminal or tab, or the SSH connection drops, SIGHUP reaches only the wrapper. The default action kills it before the finally block runs. The claim then names a dead pid, and the next acquirer reconciles it as owner-process-dead. Meanwhile expo run:ios, xcodebuild or adb install keeps mutating the device. Ctrl-\ (SIGQUIT) fails the same way. This is the race #3263 asks to close, and it contradicts the docs, which say the claim lasts exactly as long as the command and that exec releases it on every exit path.

The rule to satisfy: while the child group is alive, every terminating signal the wrapper can catch is forwarded to the group, and the wrapper releases the claim only after the child exits. Could attachSignalForwarding own one list of forwarded signals (SIGINT, SIGTERM, SIGHUP, SIGQUIT) and register and dispose all handlers from it? Please add a SIGHUP case to exec.test.ts next to the SIGTERM case. It should fail on the current code, with the wrapper killed and the child still running.

I did not run the SIGHUP behavior. It follows from Node's detached semantics on POSIX and from how terminal hangup is delivered, so treat it as likely rather than confirmed. Related and unverified: with no controlling terminal, tools that open /dev/tty (sudo or credential prompts) may fail, and Ctrl-Z stops the wrapper but not the child. Two small items: the outcome = 'unattributable' assignment at exec.ts:224 is dead because the function returns right after it, and scripts/help-conformance-sample-outputs.mjs:94 still names the old packages/daemon/src/resources/device/device-claim-conflict.ts path.

The live simulator run was at 55beffd. The later changes in exec.ts (repeated-signal forwarding, Windows refusal) are covered only by unit tests with real child processes. The exec tests call runExecCommand directly, so the cli.ts dispatch, the state dir wiring and the resolveActiveConnectionDefaults exclusion are proven only by that live run. The fail-before claim for the wrapper-steal regression and the mutation claims were checked by reading the code, not by running them.

The failing checks (Lint/Typecheck, Repo Guards, Integration, both Smoke lanes) all fail while loading test/integration/command-coverage/declarations.ts. That file calls linux.gap, a builder removed on main by #3717 and #3720, and the fix is in #3740. This diff does not touch it, so the failures look unrelated. There are no conflicts. Before merge, SIGHUP and SIGQUIT need forwarding with a SIGHUP regression test, and CI stays red until #3740 lands on main.

[cubic-dev-ai]

All reported issues were addressed across 7 files (changes from recent commits).

_{You’re at about 90% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.}

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[thymikee]

Addressed in 2665364 (+ 5509d9a for the Windows test skip), rebased onto main (4c1c786, includes #3740).

Blocking (terminal hangup). attachSignalForwarding now owns one FORWARDED_SIGNALS list (SIGINT, SIGTERM, SIGHUP, SIGQUIT), registers and disposes every handler from it, and replays all signals received before the child existed. The rule holds: while the child group is alive, every catchable terminating signal is forwarded to the group and the claim releases only after the child exits. SIGTSTP is deliberately absent (noted below). New real-process case a real SIGHUP stops the child group and releases the claim before the wrapper exits: a real wrapper process gets a real SIGHUP, and it asserts the claim is gone once the wrapper's exit is observable, the child group stopped, and rc 129. It fails on the previous code — with the list narrowed back to SIGINT/SIGTERM it dies exactly as you described: wrapper killed, claim pointing at a dead pid, child still running.

Small items. The dead outcome = 'unattributable' is gone (the catch now just reports and returns); the stale packages/daemon/src/resources/device/... path in help-conformance-sample-outputs.mjs names the real file. Help text and both docs pages name the full forwarded set.

Live run at the final head (through bin/agent-device.mjs, so cli.ts dispatch and the state-dir wiring are on the proven path; fresh iPhone 17 Pro simulator ExecIssue3263R3, iOS 27.0, deleted afterwards): device status showed live session=exec:sleep during the run and a second exec refused with DEVICE_IN_USE naming exec:sleep; SIGTERM → rc 143, claims store empty; SIGHUP → rc 129, the child sleep in the wrapper's group was gone, claims store empty once the wrapper exited; after exit a fresh exec succeeded rc 0.

Unverified side effects — recorded under "Not verified" in the PR body: the child has no controlling terminal, so /dev/tty openers (sudo, credential prompts) may fail (untested), and Ctrl-Z stops the wrapper but not the child — job control for a session-leader's detached child would need a controlling terminal the child doesn't share, so SIGTSTP stays un-forwarded by design.

The CI-red checks you listed are the #3717/#3720 builder gap on main; this head rebases over #3740, so they should clear.

[thymikee]

This PR is ready at 5509d9a. The hang-up (SIGHUP) problem from the earlier review at 06e8600 is fixed, and a real-process test now covers it.

Not blocking, and fine to take or leave: the comment at src/cli/commands/exec.ts:244 repeats the one at :248-250, so you can delete the first copy; and no test sends SIGQUIT, so you could assert that process.listenerCount returns to baseline for every FORWARDED_SIGNALS entry after the SIGTERM case in src/cli/commands/tests/exec.test.ts.

One open inline thread still applies in spirit, but it is fixed at this head, so you can resolve it: the SIGHUP case is gated by test.skipIf(IS_WINDOWS), defined once at :144 (#3738 (comment)).

All 24 checks pass at 5509d9a. The earlier red lanes came from the linux.gap builder gap on main, which #3740 fixed, and this head is rebased over #3740. There are no conflicts. Nothing else is needed before merge.

I did not run the SIGHUP test or the old-code mutation here. I reasoned that the test fails before the fix from the 06e8600 handler set and Node's default SIGHUP behavior. I also did not reproduce the live simulator run at the final head (SIGHUP exit code 129, empty claims store, child gone), so that result is the author's report. I did not check two side effects of the detached child having no controlling terminal, /dev/tty prompts and Ctrl-Z stopping the wrapper but not the child. The PR body already discloses both.

[thymikee]

The code verdict for 5509d9a is unchanged: clean. The branch now conflicts with main, so I removed ready-for-human until it is rebased. After the rebase, please check that the conflict resolution keeps the device claim held for the external command's lifetime and rerun the affected tests.

[thymikee]

Rebased onto 4a8abed7b9 and repushed; new head 0d139bce0, gate green (pnpm check:affected --run --head 0d139bce0, all runnable checks, receipt .tmp/validation/run-XpKV1T/receipt.json).

Conflict resolution. The real conflict was in the claim record: main's schema v3 (#3345 work) made the owner kind a required claimOwnerKind discriminant and trimmed the daemon package exports. Both sides' intent survives by folding the wrapper into main's model rather than keeping a parallel field: 'wrapper' is now a member of DeviceClaimOwnerKind, the wrapper claim writes claimOwnerKind: 'wrapper', claimHeldDevice takes the kind as a parameter (session/transient acquirers still derive theirs), and the supersession guard checks claimOwnerKind !== 'wrapper'. The transient label helper stays main's (it moved to the record module); only the wrapper label stays local. The daemon package.json and the help-conformance producers take main's trimmed shape — the conflict builder import lives in daemon-contracts on both sides. CONTEXT.md's Device claim entry now names the exec wrapper within the 11000-byte glossary budget (10996).

Check you asked for — the claim stays held for the command's lifetime after the merge. Re-ran the fence at 0d139bce0 through the real CLI dispatch (fresh iPhone 17 Pro ExecFenceR4, created and deleted for the check): during exec … -- sleep 120, device status showed live session=exec:sleep and a second exec refused with DEVICE_IN_USE naming the holder; SIGTERM → rc 143 with the claims store empty; after exit a fresh exec succeeded rc 0. The wrapper claim tests pin the rest on the v3 record: live wrapper claim not stealable from a registration, foreign acquire refused while the wrapper runs, release on exit.

The two notes from your 22:45 verdict are in too (top commit): the duplicated release comment at exec.ts:244 is deleted, and the SIGTERM case snapshots process.listenerCount for every FORWARDED_SIGNALS entry and asserts all return to baseline, so SIGQUIT/SIGHUP cleanup is pinned as well.

[thymikee]

The code is clean at 0d139bc, and the conflict from the earlier review (5509d9a) is resolved. All 24 of 24 checks pass at this commit, including the claim tests in daemon-contracts and src/cli/commands/tests/exec.test.ts that exercise the changed records. Required evidence is satisfied.

Not blocking: FORWARDED_SIGNALS at https://github.com/callstack/agent-device/blob/0d139bc/src/cli/commands/exec.ts#L194 is exported, but only exec.test.ts imports it, so you can keep it if fallow accepts it or assert the four signal names inline in the test, and either choice is fine.

The cubic-dev-ai P2 thread about the SIGHUP test does not apply any more, so please resolve it: the case is gated by test.skipIf(IS_WINDOWS) at exec.test.ts:347, and IS_WINDOWS is defined once at :144 (#3738 (comment)).

I did not run the wrapper or exec tests locally, so regression validity comes from reading the code path. I also did not reproduce the live simulator fence run at 0d139bc (DEVICE_IN_USE while held, SIGTERM rc 143 with the store empty, re-acquire rc 0), so that result is your report. The 516-file delta stat includes upstream main commits outside the logical patch, and I did not review those. No further action is needed before a human merge review.

[thymikee]

Follow-up for human review: the code-readiness state is unchanged at 0d139bce0ea5287d9b1f6ec9bc14cc293747b35a. I checked all 12 inline review threads and their replies; all are resolved. GitHub reports 21 passing checks and 3 skipped/neutral checks, no pending or failing checks, and a clean merge state. ready-for-human is already present.

We also investigated whether the Expo use case needs this command. Live checks on iOS Simulator confirmed that an ordinary agent-device session protects install → launch → testing: competing installs from another session, another daemon, and another workspace using the same session name were refused. Direct simctl and device-targeted Expo can still bypass cooperative claims.

The existing Simlock integration provides another no-exec path. At Simlock ccb503534dbd2eb0933243aec9ee6739f22fc7bb, using its pinned agent-device 0.21.28, six selected worker/gateway tests passed: automation, queue cancellation, idle expiry, two-token isolation, gateway uploads/capture/release, and inventory filtering. An additional exploratory gateway test uploaded our existing Expo Release artifact, verified its UI, refused a foreign device selector, and confirmed release/reclamation. These were gateway/worker processes on one Mac; fresh compilation, direct-local Expo + Metro, Android, and cross-machine networking were not exercised.

For product review, the remaining question is which external device operation cannot be replaced by build-only output plus existing install/open commands. The tested Expo workflow can use those commands, with Simlock allocating devices where pooling is needed. A documentation/help recipe may cover that case; a process-owned wrapper remains a separate decision for unavoidable external operations on fixed local devices. This is a product-scope question for the maintainer, rather than a newly found code regression.

[thymikee]

Rebased onto main and repushed. The new head is 1767adcc3d, and the gate passes on that exact commit (pnpm check:affected --run --head 1767adcc3d, every runnable check).

The rebase needed two fixes beyond the conflict-free replay:

• exec runs on the captured host. feat(host): capture HostEnvironment once at entry points #3763 and refactor(host): read every configuration value from an explicit host (#3794) #3798 made claim storage and recovery read an explicit HostEnvironment. runExecCommand now takes host from the CLI entry, like web does. It passes host.env to the claim lock, clearDeviceClaim and acquireWrapperDeviceClaim (which gained an env parameter), uses createClaimRecoveryGatewayFor(host) and createPlatformDeviceInventoryGateways(host), and spawns the child with host.cwd and host.env. The relative-path test sets cwd through createTestHostEnvironment instead of calling process.chdir.
• registry.ts fits its max-lines cap from chore: enforce file size limits with oxlint #3804. The exec descriptor took the file past its cap of 2179 lines. The 16 local-cli descriptors repeated the same four fields, so LOCAL_CLI_COMMAND_TRAITS now carries them, the same way GENERIC_MUTATING_COMMAND_TRAITS does. The file is now 2159 lines, and the cap is lowered to that.

The claim behaviour is unchanged.

[thymikee]

The PR is ready at 1767adc. Both the exec logic and the host.env plumbing look correct in the diff, and the earlier clean review at 0d139bc still holds because those commits are unchanged. The 12 earlier review threads were all resolved before that review, and none has reopened since. I ran no tests locally and judged the host.env change from the diff only. All 23 checks pass at 1767adc, and there are no conflicts. Not blocking, and you can take or leave it: in src/cli/commands/tests/exec.test.ts (https://github.com/callstack/agent-device/blob/1767adcc3d/src/cli/commands/__tests__/exec.test.ts#L79), createTestHostEnvironment() copies process.env by default and the assertions read the claim store through process.env, so a regression that reads process.env instead of host.env would still pass. Would one case that passes a custom env with its own claims directory, without touching process.env, and asserts the claim lands there cover it? Nothing in code is left before merge, so the PR now waits on maintainer review and merge.