Skip to content

feat(proxy): lease by device spec — --os, --image-tag, --physical honoured or refused by the backend - #3865

Draft
V3RON wants to merge 8 commits into
mainfrom
feat/proxy-device-spec
Draft

V3RON wants to merge 8 commits into
mainfrom
feat/proxy-device-spec

Conversation

@V3RON

@V3RON V3RON commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

open, install, reinstall and uninstall through connect proxy can now lease by OS version, Android image tag, and physical or virtual class. A backend that cannot honour the spec refuses it rather than ignoring it. 38 files. Closes #3864.

Flag Key on agent_device.lease.allocate Echo on data.lease.device
--os <version|range> osVersion (opaque string) model, osVersion (exact)
--image-tag <tag> imageTag imageTag?
--physical physical: true physical (boolean, always present)

Rules:

  • The CLI sends the keys only on a by-spec allocate. devices.list is not called first.
  • With --physical, the backend leases one of its enrolled phones. Only physical: true names the key; physical: 'yes' or 1 names no spec.
  • A daemon refuses any request that names any of the three keys. This covers a local daemon and one behind agent-device proxy. The refusal is UNSUPPORTED_OPERATION with reason: 'device-spec-unsupported' and keys, and it happens before admission, so no lease is recorded.
  • If a lease comes back without lease.device, or with a non-boolean physical, the CLI releases it and fails with the same reason.
  • A live lease serves a command only when every spec key the command names, and any --udid/--serial, matches it. Otherwise the command fails with CONNECTION_DEVICE_CONFLICT.
  • connect proxy stores the keys as defaults. The keys are visible in MCP and the SDK. open --json prints lease.device; the human output shows physical|virtual.
  • Docs: remote-proxy.md, help remote, and an amendment to ADR 0007.
  • --physical is back, per the maintainer. It was dropped in 1270549 while no phone could reach Simlock's daemon. Simlock's phone path is now merged (Physical devices through agent-device callstackincubator/simlock#870): its front takes physical (boolean), strips it before forwarding, and echoes lease.device.physical.
agent-device open MyApp --platform android --os 34 --image-tag google_apis_playstore
agent-device open MyApp --platform ios --physical --os 18.6.2

Validation

At df790b73a, pnpm check:affected --run --head df790b73a ran with exactCommit=true (fail-open, full set): format, lint, typecheck, layering, di-seams, fallow, mcp-metadata, build, package, integration-node and macos-coverage passed. vitest-related had 4989 passed, 1 failed: daemon-client-ensure-drain.test.ts timed out (5 s). The run before, at the previous head, timed out in macos-recording.test.ts instead. Both files pass when run alone (7/7). Neither covers a changed file. CI owns device lanes and coverage.

Live: the --physical shape was proved end to end against Simlock on a Samsung SM-A165F and an iPhone 11 Pro Max (commit 06bb9d1, cherry-picked here).

Review: three independent Opus rounds on the --os/--image-tag version. All findings except the gaps below are fixed. The round-3 fixes and the --physical restore were not reviewed again.

Assumption: a spec key that a command leaves out does not constrain a live lease. This follows the existing --device rule.
Assumption: env vars cannot set the keys; only flags and connect proxy can.

Known gaps:

  • MCP and the SDK never allocate a proxy lease, so through them the keys are always refused.
  • connect forms other than proxy accept the flags but do not store them, as with --device.
  • If a backend returns the run's existing lease without an echo, the CLI releases that lease.
  • If a spec lease expires and a non-allocating command re-allocates it by device, the new lease has no spec. A later open --os then fails with CONNECTION_DEVICE_CONFLICT.

View guided diff Turn on auto-fix

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://callstack.github.io/agent-device/pr-preview/pr-3865/

Built to branch gh-pages at 2026-10-22 11:19 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.54 MB 5.55 MB +8.4 kB
Package (unpacked) 5.54 MB 5.55 MB +8.4 kB
Package (download) 1.68 MB 1.68 MB +2.3 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 41.9 ms 43.1 ms +1.2 ms
CLI --help 88.1 ms 89.4 ms +1.3 ms

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Explicit blank spec values can silently become unconstrained requests instead of being validated or refused.

2 open issues
What changed in this PR

Adds OS-version and Android-image-tag selection to proxy device leases, addressing #3864.

Changes:

  • Exposes device-spec options through CLI, SDK, MCP, and connection defaults.
  • Adds backend confirmation, unsupported-spec refusal, and live-lease conflict checks.
  • Reports granted device details and adds tests and documentation.
File Description
website/​docs/​docs/​remote-proxy.md Documents spec-based leasing.
src/​mcp/​__tests__/​command-tools-operator-inputs.test.ts Checks MCP spec fields.
src/​daemon-client/​daemon-client-rpc.test.ts Tests allocation wire fields.
src/​commands/​schema/​option-schema.ts Excludes ambient environment defaults.
src/​commands/​schema/​command-overrides.ts Allows spec flags on connect.
src/​commands/​schema/​cli-help.ts Explains proxy spec behavior.
src/​commands/​management/​lease-device-spec.ts Shares command spec fields.
src/​commands/​management/​install.ts Adds install-family spec inputs.
src/​commands/​management/​app.ts Adds open spec inputs.
src/​commands/​command-flags.ts Projects spec request flags.
src/​client/​lease-client.ts Reads granted device facts.
src/​cli/​connection/​proxy-profile.ts Stores proxy spec defaults.
src/​cli/​connection/​profile-fields.ts Groups hosted allocation fields.
src/​cli/​commands/​router.ts Accepts granted device metadata.
src/​cli/​commands/​router-types.ts Types granted device metadata.
src/​cli/​commands/​proxy-lease-device.ts Validates and binds spec leases.
src/​cli/​commands/​generic.ts Prints granted device facts.
src/​cli/​commands/​connection-runtime.ts Integrates spec allocation and reporting.
src/​cli/​commands/​__tests__/​proxy-lease-device-spec.test.ts Tests proxy spec lifecycle.
src/​cli/​commands/​__tests__/​generic.test.ts Tests granted-device output.
src/​cli.ts Carries lease facts to output.
src/​__tests__/​agent-device-client-leases.test.ts Tests SDK allocation fields.
src/​__tests__/​agent-device-client-lease-device-spec.test.ts Tests daemon and proxy refusals.
scripts/​integration-progress-model.ts Identifies spec-test ownership.
packages/​remote/​src/​remote-connection-state.ts Persists lease-bound spec metadata.
packages/​remote/​src/​remote-config-schema.ts Adds spec profile fields.
packages/​daemon/​src/​server/​request-router.ts Refuses specs before admission.
packages/​contracts/​src/​request-envelope.ts Adds request spec fields.
packages/​contracts/​src/​remote-config-fields.ts Types profile spec defaults.
packages/​contracts/​src/​lease-scope.ts Defines shared spec handling.
packages/​contracts/​src/​facades/​client.ts Exports spec and device types.
packages/​contracts/​src/​client-lease.ts Extends lease contracts.
packages/​contracts/​src/​client-app.ts Extends app-operation options.
packages/​contracts/​src/​cli-flags.ts Types CLI spec flags.
packages/​contracts/​src/​__tests__/​lease-scope.test.ts Tests spec and echo readers.
packages/​command-registry/​src/​flag-groups.ts Shares supported spec flags.
packages/​command-registry/​src/​flag-definitions-target.ts Defines public spec flags.
docs/​adr/​0007-remote-device-leases.md Records spec-leasing decisions.

Review effort: Balanced

Comment on lines +326 to +328
return stripUndefined({
osVersion: nonEmptyString(source.osVersion),
imageTag: nonEmptyString(source.imageTag),
}
return deviceSpecUnsupportedError(
keys,
'The proxy backend leased a device without confirming the requested device spec, so the lease was released.',
@V3RON
V3RON marked this pull request as draft October 21, 2026 12:34
@thymikee

Copy link
Copy Markdown
Member

I reviewed 4f94d93. The design is sound, and one open Copilot thread still holds and needs a fix before this is ready: --os '' is dropped on the CLI but refused on MCP/SDK. readLeaseDeviceSpec drops empty values, while namedLeaseDeviceSpecKeys refuses them, so the two surfaces disagree. Could both surfaces handle an empty value the same way?

The new tests would fail on the old code, because the flags, keys and guard do not exist there. I did not rerun any tests. The honoured path, where a backend echoes lease.device, is proven only against a test-client fake, since simlock#814 is not available for a live allocate by spec.

Not blocking, and fine to take or leave: the release in src/cli/commands/proxy-lease-device.ts:138 uses .catch(() => undefined), so a failed release disappears, while releaseUnhonouredSpecLease puts released/releaseError in the conflict details. The spec key list has two names in packages/command-registry/src/flag-groups.ts:38, and readLeaseDeviceSpec, leaseDeviceSpecFields and the proxy profile code hand-write osVersion/imageTag instead of mapping over LEASE_DEVICE_SPEC_KEYS. The leaseDevice fact travels through the CLI router and then gets a special case for open twice in src/cli/commands/generic.ts:627, so open's own formatter could merge it once. The other Copilot thread also holds: the release message always says it was released, although details.released is right.

Smoke failed on open com.callstack.agentdevicelab --relaunch with android-app-close-timeout on a local daemon. That route uses no spec flag and no remote config, and the Android close and relaunch path does not overlap this diff, so it looks unrelated. Coverage also failed, but its log is cut off before any failing test, so I could not attribute it. Please rerun both, and if Coverage fails again, share the failing case. I know of no conflicts.

@V3RON V3RON changed the title feat(proxy): lease by device spec — --os, --image-tag honoured or refused by the backend feat(proxy): lease by device spec — --os, --image-tag, --physical honoured or refused by the backend Oct 22, 2026
@thymikee

Copy link
Copy Markdown
Member

The earlier review's main problem is not fixed at df790b7, and the new physical check adds a second value that splits the same way. The CLI reader still drops osVersion: '' and imageTag: ' ', and now also drops physical: false (lease-scope.ts#L326). The daemon router names any value that is not undefined (lease-scope.ts#L316, request-router.ts:575). So SDK/MCP open { physical: false } reaches the daemon as flags.physical=false and fails with UNSUPPORTED_OPERATION device-spec-unsupported, keys ['physical']. A connect proxy profile with physical: false, or CLI --os '', is dropped and the command runs with no spec. The same input gives different results on CLI, MCP and SDK, which breaks the consistency rule in #3864. An MCP agent that sends physical: false to mean a simulator is refused even on a local daemon. The rule to satisfy: one function in lease-scope.ts classifies each supplied spec value as named, not named or invalid, and every surface calls it. That covers leaseDeviceSpecInputFromFlags, hasLeaseDeviceSpec, the connection-runtime allocate and the daemon router's deviceSpecFlagsResponse. Either refuse blank strings and non-true physical everywhere (CLI INVALID_ARGS, daemon device-spec-unsupported), or treat them as not named everywhere. Typing LeaseDeviceSpec.physical and the option field as the literal true would make false impossible to send.

Not blocking, and you can take or leave these: the CLI checks only that lease.device exists, so a physical: true request with an echo of { physical: false } is accepted and bound (it could release through releaseUnhonouredSpecLease with keys ['physical'] when lease.device.physical !== true, see proxy-lease-device.ts#L131); once the rule above is decided, the physical row in remote-proxy.md should state it, and the release sentence there and in ADR 0007 should also cover a lease.device with no boolean physical; and one test that iterates LEASE_DEVICE_SPEC_KEYS and asserts getOptionSpec(key) has no env names would have caught the ENV_EXCLUDED_FLAG_KEYS miss that df790b7 fixed by hand (option-schema.ts#L39).

Could one per-key descriptor in lease-scope.ts, for example LEASE_DEVICE_SPEC_FIELDS = { osVersion: 'string', imageTag: 'string', physical: 'true' }, plus one classifier readLeaseDeviceSpec(source) -> { spec, invalidKeys } replace most of this? The CLI readers, the proxy allocate and the daemon router would all call it. The router would refuse the keys in spec and invalidKeys, and the CLI would raise INVALID_ARGS on invalidKeys. ENV_EXCLUDED_FLAG_KEYS, the remote-config env: false entries, leaseDeviceSpecFields, the proxy profile and the bindLeaseNamedDevice deletion map could then all map over the keys. The physical === true special case and the per-key deletes would go away, and this split could not recur for a fourth key. What would have to change first is the decision on whether a blank string or physical: false is invalid or not named, and then LeaseDeviceSpec.physical is typed to match.

The two open inline threads still stand: the blank osVersion/imageTag and physical mismatch (P2, same root as above) and the "lease was released" message (P3, unchanged and fine to leave). No open thread can be resolved yet.

The Coverage job failed in the Run coverage step, and its log ends without a failing test or a vitest summary. The previous head 4f94d93 failed the same way, and main is green at d217300. The PR touches paths that run exercises (the daemon request-router refusal, the daemon-client allocate flags and the CLI connection runtime), so I cannot rule out an overlap, but no failing case names this diff. Please rerun Coverage after the fix and share the failing case if it fails again.

I did not run any tests. The MCP/SDK route above comes from reading the code, not from an executed call. The honoured path is proven in the repo only against createTestClient fakes. The PR mentions a live Simlock run for --physical, but I could not see its output, and --os and --image-tag have no live run. I also do not know whether Simlock reads physical: false as virtual only or as no preference, and that decides which side of the rule is right.

Before merge, the CLI and the daemon must agree on which spec values name a key, which covers blank strings and now physical: false, and Coverage must pass or show a failing case.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proxy lease by device spec: --os and --image-tag, honoured or refused by the backend

3 participants