[janjankowski-cs]

One decision on what a device spec names

Problem

Two parts of the code decided independently whether a request named a device spec, and they disagreed:

Code	Old rule	osVersion: ""
CLI (readLeaseDeviceSpec)	blank strings and physical: false don't count	dropped → open proceeds
Daemon gate (namedLeaseDeviceSpecKeys)	anything not undefined counts	refused

The CLI cleaned the values before sending, so it never hit the difference. The SDK and MCP send values unchanged, so the daemon refused "" and physical: false with device-spec-unsupported.

The rule

readLeaseDeviceSpec in lease-scope.ts is now the only place that decides. Each key gets exactly one of three outcomes:

Value	Outcome
"18", " >=26 " (sent as given), physical: true	named
missing, "", " ", physical: false	not named
osVersion: 26, physical: "yes", imageTag: null	invalid (wrong type)

Changes

1. readLeaseDeviceSpec (lease-scope.ts)

for (const key of LEASE_DEVICE_SPEC_KEYS) {
  const value = source?.[key];
  if (value === undefined) continue;                       // missing → not named
  if (typeof value !== (key === 'physical' ? 'boolean' : 'string'))
    invalidKeys.push(key);                                 // wrong type → invalid
  else if (value === true) spec[key] = true;               // physical: true → named
  else if (typeof value === 'string' && value.trim())
    spec[key] = value;                                     // non-blank string → named, as given
}                                                          // anything left ("" or false) → not named

It returns { spec, keys, invalidKeys }. The old version returned only spec and silently ignored wrong types. namedLeaseDeviceSpecKeys is removed.

2. deviceSpecInvalidError (lease-scope.ts)

A wrong type is a caller mistake, not a missing feature, so it is refused with INVALID_ARGS and the typed reason device-spec-invalid, with details.keys, instead of device-spec-unsupported.

3. requireLeaseDeviceSpec (lease-scope.ts)

The same decision for client-side sources: it throws device-spec-invalid on an invalid key, so the CLI fails before any request is sent. The CLI callers use it:

• proxy-lease-device.ts: deciding whether to allocate by spec, and the spec check on a live lease.
• connection-runtime.ts: the spec sent on lease.allocate.
• lease-device-spec.ts: the command readers for open, install, reinstall and uninstall.

4. Daemon gate (request-router.ts)

const { keys, invalidKeys } = readLeaseDeviceSpec(req.flags);
if (invalidKeys.length > 0) return invalid(invalidKeys);  // wrong type → INVALID_ARGS
if (keys.length === 0) return undefined;                  // "" / false → allowed through
return unsupported(keys);                                 // real spec → refused as before

This fixes the SDK and MCP: the daemon now ignores "" and false the same way the CLI does.

5. Lease request params (readLeaseAllocateProviderFlags)

	Before	After
Named key	sent as given	sent as given
"" or false	sent	dropped
Wrong type	sent	sent unchanged, so the daemon still refuses it

An invalid value is sent unchanged on purpose: dropping it here would let a bad request through without an error.

6. Tests and docs

• lease-scope.test.ts: covers all three outcomes. The old assertion that physical: 'yes' names nothing is replaced, because that behaviour changed.
• agent-device-client-lease-device-spec.test.ts: the SDK against a real daemon. Blank values and physical: false allocate a lease; physical: 'yes' is refused with device-spec-invalid and leaves no lease.
• help remote, remote-proxy.md and ADR 0007 each state the rule in one sentence.

Update: CI and module loading

• Trimming was removed in 1afb732: a named osVersion or imageTag is sent exactly as given. Step 4 of the validation below ran before that change.
• feat/proxy-device-spec (feat(proxy): lease by device spec — --os, --image-tag, --physical honoured or refused by the backend #3865) was behind main, so daemon-wire-compat compared this branch against v0.21.33 without fix(daemon): answer 404 for an expired or unknown resource id #3958. Main is merged into feat(proxy): lease by device spec — --os, --image-tag, --physical honoured or refused by the backend #3865, and feat(proxy): lease by device spec — --os, --image-tag, --physical honoured or refused by the backend #3865 is merged into this branch.
• Coverage failed on eager-closure-budgets (inherited from feat(proxy): lease by device spec — --os, --image-tag, --physical honoured or refused by the backend #3865):
  • flag-groups.ts imported lease-scope.ts at runtime, so it loaded 5 modules instead of 1. It now declares LEASE_DEVICE_SPEC_FLAGS itself, pinned to LEASE_DEVICE_SPEC_KEYS with a type-only satisfies.
  • lease-device-spec.ts was a new eager module in the CLI closure. Its field set moved to command-input.ts as leaseDeviceSpecFields(), next to repeatedFields(). Its reader moved to cli-grammar/common.ts, next to the other flag readers.
• help remote and remote-proxy.md now state each device-spec rule once.

Validation

Steps 2 and 2c of the manual test, re-run with the same inputs on the iPhone 17 Pro simulator against a local daemon:

Input	CLI	SDK before	SDK after
osVersion: "18"	device-spec-unsupported [osVersion]	same	same ✅
physical: true	device-spec-unsupported [physical]	same	same ✅
osVersion: ""	opened	❌ refused	opened ✅
osVersion: " "	opened	❌ refused	opened ✅
physical: false	not expressible¹	❌ refused	opened ✅
physical: "yes"	not expressible¹	device-spec-unsupported	INVALID_ARGS device-spec-invalid ✅
osVersion: 26	not expressible¹	device-spec-unsupported	INVALID_ARGS device-spec-invalid ✅

¹ The CLI parser types the flags, so it can't produce these values. --physical=false fails earlier with Flag --physical does not take a value, and leaving out --physical is how the CLI says false.

---

Step	What was checked	Result
0	pnpm install --frozen-lockfile && pnpm build on pr-3865 plus the change	✅
1	--os, --image-tag and --physical appear in open --help; help remote includes the new device-spec-invalid sentence	✅
2 (CLI)	--os 18 and --physical → device-spec-unsupported; --os "", --os " " and --image-tag "" → open proceeds	✅
2 (SDK)	osVersion: "18" and physical: true → device-spec-unsupported; "", " " and physical: false → open proceeds; physical: "yes" and osVersion: 26 → INVALID_ARGS, device-spec-invalid	✅ (failed before the change)
3	A plain agent-device proxy refuses --os 18; a plain open straight afterwards succeeds, so no lease was left behind	✅
4	Through the stub, the output shows Leased device: Stub Device, 18.6, virtual; --os ' 18.6 ' reaches the backend trimmed to "18.6"; snapshot works and doesn't carry the spec	✅
5a	open with the same --os on a live lease is served	✅
5b	open with a different --os → CONNECTION_DEVICE_CONFLICT, keys: ["osVersion"]	✅
5c	open with a different --udid and no spec	⚠️ LEASE_SCOPE_MISMATCH (unchanged - unrelated to this change)
5c′	open with a different --udid plus --os → CONNECTION_DEVICE_CONFLICT	✅
5d	open naming no spec is served on the existing lease	✅
5+	open with --os "" on a live spec lease is served, because a blank value names nothing	✅ (new case)
6	connect proxy --os 18.6 stores the spec, and a later plain open sends it	✅
7	A backend that doesn't echo lease.device: the lease is released (released: true) and the command fails with device-spec-unsupported; a plain open afterwards succeeds	✅

[vercel]

@janjankowski-cs is attempting to deploy a commit to the Callstack Team on Vercel.

A member of the Team first needs to authorize it.

[cubic-dev-ai]

All reported issues were addressed across 10 files

<sub>Reply to a comment to ask cubic a question or push back. It learns from your replies.

View guided diff | Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 1 file (changes from recent commits).

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

View guided diff | Re-trigger cubic</sub>

[janjankowski-cs]

New test: request-device-spec-gate.test.ts

packages/daemon/src/server/__tests__/request-device-spec-gate.test.ts checks the daemon's device-spec gate directly, on the path the SDK and MCP use.

Why it was needed

The SDK and MCP send command flags to the daemon unchanged. So a blank osVersion, a blank imageTag or physical: false reaches the gate as is. The CLI drops these values before sending.

The existing SDK test (agent-device-client-lease-device-spec.test.ts) uses lease.allocate. On that path, readLeaseAllocateProviderFlags drops the same values on both the client and the daemon side before the gate runs. So it never tested the gate's own rule: with the pre-fix rule put back into the gate, that test still passed.

Setup

The test builds the real daemon request handler with a fake device runtime, following daemon-policy.test.ts, and sends app-switcher. That command runs on the fake runtime without a real device, so the gate decides the outcome by itself.

Tests

Test	Flags sent	Expected
A command whose blank osVersion, blank imageTag and physical: false name no spec runs	osVersion: "", imageTag: " ", physical: false	ok: true, and the command ran once
A command naming a spec is refused with device-spec-unsupported before it runs	osVersion: "18", physical: false	UNSUPPORTED_OPERATION, reason: "device-spec-unsupported", keys: ["osVersion"], and the command never ran

The first test is the regression test for the review finding: values that name nothing must go through the gate, as they do on the CLI.

The second test is the closest negative case. It checks that:

• a named key is still refused when it's mixed with a value that names nothing;
• physical: false doesn't appear in keys;
• the refusal happens before the command runs.

Proof that it catches the bug

This was run in a separate checkout of ff3b3dbee plus the new test file, rebuilt before each run. For the second run, the pre-fix gate rule ("any key that is not undefined is named") was put back into request-router.ts:

Run	Gate test (new)	SDK test file (existing)	Total
Unmodified	2 passed	4 passed	6 passed
Pre-fix gate rule	2 failed	4 passed	2 failed, 4 passed

Both failures are for the intended reason:

• First test: the request was refused (ok: false instead of ok: true). This is the bug the reviewer found.
• Second test: keys was ["osVersion", "physical"] instead of ["osVersion"], because the old rule counted physical: false as named.

The existing SDK test file passed under the old rule, which confirms it doesn't cover this path.

[cubic-dev-ai]

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

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

View guided diff | Re-trigger cubic</sub>

[thymikee]

The code looks good at bc880e1, and the duplicated ADR block from the earlier review is fixed. Not blocking, and you can take or leave it: the inputDescription strings for osVersion, imageTag and physical in https://github.com/callstack/agent-device/blob/bc880e1/packages/command-registry/src/flag-definitions-target.ts#L87 still only mention device-spec-unsupported. They could add one clause each saying that a blank string or physical:false names nothing, and that another type fails with INVALID_ARGS device-spec-invalid. The comment at cli-flags.ts:64-65 could also point at readLeaseDeviceSpec instead of LEASE_DEVICE_SPEC_KEYS.

The earlier Cubic thread on the ADR no longer applies, because the duplicated block is gone and the INVALID_ARGS sentence carries details.keys. Please resolve it: #4009 (comment)

The only failing check is Vercel. Its link is a git-authorization page for a fork PR, so the preview deploy was not authorized. It did not fail a build, and this diff does not cause it. There are no conflicts. I did not re-run the live SDK runs or the manual steps; they come from the PR body, and the latest change is ADR-only. I did not run the unit or daemon tests locally. Before merge, please land this into feat/proxy-device-spec ahead of #3865 reaching main, so #3865 does not ship the CLI/daemon contract mismatch.

[thymikee]

why extra whitespace here?

[janjankowski-cs]

that way we test right away for trimming of the string itself, not necessary

[thymikee]

Good catch. The padding was there because we trimmed these values, and the test pinned that. Nothing actually sends padded values: the CLI, the config file and the SDK all pass strings as given, and we don't trim platform or device either. So the trim is gone in 1afb732: a named osVersion or imageTag is now sent exactly as given, and a blank one still names nothing. This test now uses a plain '34', and a separate test named for the rule covers a padded value being kept as is.

Checking this also showed that connect proxy --os "" wrote an empty value into the stored profile and then failed to read it back. The profile now goes through the same rule, so a blank value is not stored.

[cubic-dev-ai]

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

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

View guided diff | Re-trigger cubic</sub>

[thymikee]

The code in 1afb732 looks good, and I found no new problems since my earlier review (#4009 (comment)). I read the head tree and the delta only. I did not run tests, gates or CI jobs. No live device run is needed, because the delta changes only CLI and contract spec reading plus docs.

The only check that is not passing is Vercel. Its link goes to vercel.com/git/authorize, which is an authorization prompt for a fork author, not a build of this diff. The diff touches no website build input beyond one docs sentence, so I think this is unrelated. I could not see a failure excerpt, so that rests on the URL.

Two Cubic threads on cli-help.ts:568 still apply, both wording only. In #4009 (comment), "them" reads as all three flags after a sentence about a blank value. In #4009 (comment), "refuses the three keys with UNSUPPORTED_OPERATION" is too broad: the router refuses only a named key that way, accepts a blank or false key, and returns INVALID_ARGS for a wrong type. Please fix the text or reply, then resolve them.

The thread at #4009 (comment) no longer applies. The padded osVersion values were replaced with clean ones, and the whitespace left in lease-scope.test.ts is blank-only input or the deliberate no-trim case. Please resolve it.

Once those threads are settled, I see nothing else in the code that must change before merge.

[cubic-dev-ai]

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

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

View guided diff | Re-trigger cubic</sub>

[cubic-dev-ai]

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

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

View guided diff | Re-trigger cubic</sub>

[cubic-dev-ai]

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

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

View guided diff | Re-trigger cubic</sub>