Repository navigation
feat(settings): add Android mobile-data on|off - #3636
Conversation
`settings mobile-data on|off` switches mobile data for the default data SIM on adb-backed Android targets through `svc data enable|disable`. It then reads the stored switch back until the value matches the request, within a 2 s budget, and reports `mobileData`. On one SIM the switch is read from the global `mobile_data` row. On a multi-SIM device it is read from `mobile_data<subId>` for the default data subscription (`multi_sim_data_call`). A switch that never settles fails with COMMAND_FAILED instead of reporting a change that did not happen. QA automation can use it to move a streaming session from Wi-Fi to cellular and back, and to check how the app warns about streaming on mobile data. On iOS, tvOS, and visionOS, the daemon refuses the setting before admission with UNSUPPORTED_OPERATION (reason setting-unsupported-on-leaf), because agent-device has no mobile-data control for Apple targets and simulators have no cellular radio.
…very non-Android target Mobile-data read-back: - Choose the stored row before `svc data` runs, from the active modem count (`persist.radio.multisim.config`), never from which rows exist. A stale `mobile_data<subId>` row on a single-modem device, or a seeded plain row on a multi-modem device, no longer decides the result. - Each sample reads one row. - Sample through absent or unreadable values, using a `Deadline` and the request's abort signal. - Report `mobile-data-not-applied` or `mobile-data-unreadable` as typed reasons, with `settingKey` and `storedValue` in the details. - A failure before `svc data` discloses `dispatched: no`; every failure after it discloses `unknown`. Refusals: - The daemon refuses mobile-data on every non-Android target before admission, through one shared leaf-refusal response, so a HarmonyOS or Linux session keeps its ref frame. - The Apple and Limrun iOS owners throw the same typed refusal. - `--app` is forwarded and refused with `setting_app_not_consumed`, because the switch is device-wide. - The macOS unsupported list now includes mobile-data. - Help and the commands and Limrun docs describe the refusal accurately for physical iPhones.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 19 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="packages/platform-apple/src/core/app-settings.ts">
<violation number="1" location="packages/platform-apple/src/core/app-settings.ts:66">
P2: macOS exits through the earlier `isMacOs` branch, so this guard never runs there and `mobile-data` gets the generic `INVALID_ARGS` response instead of the shared unsupported-operation refusal. Move the mobile-data guard ahead of the macOS branch.</violation>
</file>
<file name="packages/daemon/src/execution/handlers/__tests__/snapshot-settings-handler.test.ts">
<violation number="1" location="packages/daemon/src/execution/handlers/__tests__/snapshot-settings-handler.test.ts:411">
P3: This list omits the supported `vega` and `web` platforms, so the test does not verify the pre-admission refusal for every non-Android target as its name claims. Add representative devices for both platforms.</violation>
</file>
<file name="packages/platform-apple/src/core/__tests__/app-settings.test.ts">
<violation number="1" location="packages/platform-apple/src/core/__tests__/app-settings.test.ts:193">
P2: This test asserts `dispatched: 'no'` but discards the fake tool's invocation log, so a command could run and be swallowed before the same refusal is returned. Capture `calls` and assert it is empty.
(Based on your team's feedback about typed interaction refusals.) .</violation>
</file>
<file name="packages/contracts/src/settings.ts">
<violation number="1" location="packages/contracts/src/settings.ts:227">
P2: This classifies every state as device-level, so `settings mobile-data cellular --app X` is rejected as `setting_app_not_consumed` before Android can return its invalid-state error. Gate this on `readSettingState(state)` as `locationAppScope` does.</violation>
</file>
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
View guided diff | Re-trigger cubic
| throw new AppError('INVALID_ARGS', getUnsupportedMacOsSettingMessage(setting)); | ||
| } | ||
| const normalized = setting.toLowerCase(); | ||
| if (normalized === 'mobile-data') throw mobileDataUnsupportedError(device.id); |
There was a problem hiding this comment.
P2: macOS exits through the earlier isMacOs branch, so this guard never runs there and mobile-data gets the generic INVALID_ARGS response instead of the shared unsupported-operation refusal. Move the mobile-data guard ahead of the macOS branch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. 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. At packages/platform-apple/src/core/app-settings.ts, line 66:
<comment>macOS exits through the earlier `isMacOs` branch, so this guard never runs there and `mobile-data` gets the generic `INVALID_ARGS` response instead of the shared unsupported-operation refusal. Move the mobile-data guard ahead of the macOS branch.</comment>
<file context>
@@ -61,9 +62,10 @@ export async function setIosSetting(
throw new AppError('INVALID_ARGS', getUnsupportedMacOsSettingMessage(setting));
}
+ const normalized = setting.toLowerCase();
+ if (normalized === 'mobile-data') throw mobileDataUnsupportedError(device.id);
requireSimulatorDevice(device, 'settings');
await ensureBootedSimulator(device);
</file context>
| await withFakeAppleTool(unexpectedArgs, async () => { | ||
| for (const device of [IOS_TEST_SIMULATOR, TVOS_TEST_SIMULATOR]) { | ||
| await assertRejectsAppError(() => setIosSetting(device, 'mobile-data', 'off'), { | ||
| code: 'UNSUPPORTED_OPERATION', | ||
| reason: 'setting-unsupported-on-leaf', | ||
| dispatched: 'no', | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
P2: This test asserts dispatched: 'no' but discards the fake tool's invocation log, so a command could run and be swallowed before the same refusal is returned. Capture calls and assert it is empty.
(Based on your team's feedback about typed interaction refusals.) .
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. 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. At packages/platform-apple/src/core/__tests__/app-settings.test.ts, line 193:
<comment>This test asserts `dispatched: 'no'` but discards the fake tool's invocation log, so a command could run and be swallowed before the same refusal is returned. Capture `calls` and assert it is empty.
(Based on your team's feedback about typed interaction refusals.) .</comment>
<file context>
@@ -189,6 +189,18 @@ test('setIosSetting biometric refuses an Apple TV simulator before posting anyth
});
+test('setIosSetting mobile-data refuses every Apple leaf before spawning anything', async () => {
+ await withFakeAppleTool(unexpectedArgs, async () => {
+ for (const device of [IOS_TEST_SIMULATOR, TVOS_TEST_SIMULATOR]) {
+ await assertRejectsAppError(() => setIosSetting(device, 'mobile-data', 'off'), {
</file context>
| await withFakeAppleTool(unexpectedArgs, async () => { | |
| for (const device of [IOS_TEST_SIMULATOR, TVOS_TEST_SIMULATOR]) { | |
| await assertRejectsAppError(() => setIosSetting(device, 'mobile-data', 'off'), { | |
| code: 'UNSUPPORTED_OPERATION', | |
| reason: 'setting-unsupported-on-leaf', | |
| dispatched: 'no', | |
| }); | |
| } | |
| }); | |
| await withFakeAppleTool(unexpectedArgs, async ({ calls }) => { | |
| for (const device of [IOS_TEST_SIMULATOR, TVOS_TEST_SIMULATOR]) { | |
| await assertRejectsAppError(() => setIosSetting(device, 'mobile-data', 'off'), { | |
| code: 'UNSUPPORTED_OPERATION', | |
| reason: 'setting-unsupported-on-leaf', | |
| dispatched: 'no', | |
| }); | |
| } | |
| assert.deepEqual(calls, []); | |
| }); |
| return family === 'macos-host' ? 'device-level' : 'app-scoped'; | ||
| } | ||
| if (normalizedSetting === 'location') return locationAppScope(family, state); | ||
| if (normalizedSetting === 'mobile-data') return family === 'mobile' ? 'device-level' : 'unknown'; |
There was a problem hiding this comment.
P2: This classifies every state as device-level, so settings mobile-data cellular --app X is rejected as setting_app_not_consumed before Android can return its invalid-state error. Gate this on readSettingState(state) as locationAppScope does.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. 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. At packages/contracts/src/settings.ts, line 227:
<comment>This classifies every state as device-level, so `settings mobile-data cellular --app X` is rejected as `setting_app_not_consumed` before Android can return its invalid-state error. Gate this on `readSettingState(state)` as `locationAppScope` does.</comment>
<file context>
@@ -223,6 +224,7 @@ export function settingsAppScope(
return family === 'macos-host' ? 'device-level' : 'app-scoped';
}
if (normalizedSetting === 'location') return locationAppScope(family, state);
+ if (normalizedSetting === 'mobile-data') return family === 'mobile' ? 'device-level' : 'unknown';
return 'unknown';
}
</file context>
| if (normalizedSetting === 'mobile-data') return family === 'mobile' ? 'device-level' : 'unknown'; | |
| if (normalizedSetting === 'mobile-data') { | |
| return family === 'mobile' && readSettingState(state) !== undefined ? 'device-level' : 'unknown'; | |
| } |
| kind: 'emulator', | ||
| booted: true, | ||
| }, | ||
| { platform: 'linux', id: 'linux-host', name: 'Linux Desktop', kind: 'device', booted: true }, |
There was a problem hiding this comment.
P3: This list omits the supported vega and web platforms, so the test does not verify the pre-admission refusal for every non-Android target as its name claims. Add representative devices for both platforms.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. 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. At packages/daemon/src/execution/handlers/__tests__/snapshot-settings-handler.test.ts, line 411:
<comment>This list omits the supported `vega` and `web` platforms, so the test does not verify the pre-admission refusal for every non-Android target as its name claims. Add representative devices for both platforms.</comment>
<file context>
@@ -372,6 +374,122 @@ test('settings usage hint documents canonical faceid states', async () => {
+ kind: 'emulator',
+ booted: true,
+ },
+ { platform: 'linux', id: 'linux-host', name: 'Linux Desktop', kind: 'device', booted: true },
+];
+
</file context>
| { platform: 'linux', id: 'linux-host', name: 'Linux Desktop', kind: 'device', booted: true }, | |
| { platform: 'linux', id: 'linux-host', name: 'Linux Desktop', kind: 'device', booted: true }, | |
| { platform: 'vega', id: 'vega-tv', name: 'Vega TV', kind: 'device', target: 'tv' }, | |
| { platform: 'web', id: 'web-browser', name: 'Browser', kind: 'device' }, |
|
Thanks for this. The feature is useful, but the PR is large for one Android switch: +752 lines, and only about 60 of them switch mobile data. Here is what I think could be smaller, reviewed at 6b94f11.
None of this changes the behavior. It is about keeping the patch at the size of the feature. |
|
Thanks for the PR. At 6b94f11 I found one problem that should be fixed before merge. The Android single-modem path is covered by the live run in the PR body, which I accepted from the description and did not re-run. Both Smoke Tests jobs were still pending when I checked, so I cannot say yet what CI shows. I have no conflict information. The multi-SIM branch in resolveAndroidMobileDataSettingKey picks Could the feature be much smaller? A Not blocking, and you can take or leave it: the owner-side refusals in app-settings.ts and the Limrun iOS settings code repeat the refusal the daemon already makes at snapshot-settings.ts:347, so three sites now refuse one setting. Keeping the daemon refusal and deleting the copies would be simpler, unless a caller reaches On the open inline threads, the first four hold. The signal is not checked before the key probe or Before merge, remove the unvalidated multi-SIM branch or post the dual-SIM run, then answer the size questions above. |
Read back only the global mobile_data row and document that multi-SIM devices are not covered until a dual-SIM run shows which row to read. Drop the Apple and Limrun owner refusals, mobileDataUnsupportedError, the SettingLeafRefusal rename and withDispatchDisclosure; the daemon's pre-admission check is now the only non-Android refusal. Check the request signal before svc data and pass it, with a per-read timeout bounded by the settle budget, to every adb call. Move the poll into a shared sampleUntilSettled helper that the two alert loops now use too. Leave a bad state with --app to the owner's INVALID_ARGS, cover vega and web in the daemon refusal test, collapse the Android tests into tables, and shorten the help and docs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Get a Slack DM when cubic finishes reviewing your PRs. Connect Slack
View guided diff | Re-trigger cubic
|
Thanks for the update. At 28106dd the checks are green, and the multi-SIM probe and the app-scope guard are gone, so the earlier points on those are fixed. Two things still need work before merge. The read-back loop in The live run in the PR body was taken at 124d05f, before the multi-SIM probe was removed and the read-back loop was rewritten, and the body still describes the dropped Not blocking, take or leave: commit 28106dd made Limrun Android refuse with On the open inline threads, the commands.md:675 macOS wording thread still applies and needs the "macOS answers INVALID_ARGS" line corrected to UNSUPPORTED_OPERATION: #3636 (comment). The Limrun thread (#3636 (comment)) does not apply as a code defect, because the refusal is a deliberate scope cut, and only the limrun.md:76 claim remains. These do not apply and can be resolved: the Apple owner guard (#3636 (comment)) was removed in 6fdbc52; the app-settings.test.ts case (#3636 (comment)) was deleted in 6fdbc52; the cellular app-scope thread (#3636 (comment)) is rejected earlier by I did not run tests or a device. I did not check whether the daemon normalizes a raw AbortError into a typed response. The loop finding rests on Before merge, the read-back needs to go through |
|
The code is clean at d4f476e. The cancellation, probe-timeout and settle-error fixes from the earlier review (#3636 (comment)) are all in, and the Limrun Android and docs gaps are closed too. The single reported check passes, and there are no conflicts. Not blocking, so take or leave: the two cancellation tests in https://github.com/callstack/agent-device/blob/d4f476e/packages/platform-android/src/__tests__/settings.test.ts#L632 use a bare I did not re-run the emulator validation or any local tests. The emulator run is the one reported in the PR body at d4f476e. Limrun Android shares the setAndroidSetting route over the ADB tunnel with wifi and airplane. Multi-SIM read-back stays uncovered by design and is documented. On the open threads: all of the earlier review threads are either fixed or no longer apply, so none still applies to merge. These do not apply, so please resolve them: the Apple owner guard was removed, and app-settings.ts has no mobile-data branch (#3636 (comment)). The app-settings.test.ts mobile-data case was deleted with the owner refusal (#3636 (comment)). parseSettingsMutation rejects the bad state |
thymikee
added
the
ready-for-human
2 participants
Summary
Adds
settings mobile-data on|offfor Android, so a run on a streaming app can move from Wi-Fi to cellular and back and check the "streaming on mobile data" warnings.agent-device settings wifi off agent-device settings mobile-data on # mobileData: enabledNode:
client.settings.update({ setting: 'mobile-data', state: 'off' })svc data enable|disable, then polls the globalmobile_datarow throughpollUntilSettled(2 s deadline, 100 ms interval). A cancelled request fails with the typed request-canceled error, like the alert and fill polls.mobile-data-not-appliedormobile-data-unreadableand says whether the switch was dispatched.UNSUPPORTED_OPERATION(setting-unsupported-on-leaf).--appis refused withINVALID_ARGSandsetting_app_not_consumed.Test plan
Use an Android emulator or phone with a SIM.
Sis its adb serial.pnpm build && pnpm build:android && pnpm clean:daemonpnpm ad settings mobile-data off --platform android --serial $S --jsonreturnsmobileData: disabled, andadb -s $S shell settings get global mobile_dataprints 0.onreturnsenabledand 1.mobile-data off --app com.example.appfails withINVALID_ARGSandsetting_app_not_consumed, and the row stays 1.settings mobile-data on.Validation
Steps 2 and 3 ran on a local Android emulator (
emulator-5554) with the build from d4f476e:adb shell settings get global mobile_datasettings mobile-data off --jsonmobileData: "disabled"settings mobile-data on --jsonmobileData: "enabled"settings mobile-data off --app com.example.app --jsonINVALID_ARGS,reason: setting_app_not_consumed,dispatched: "no"The emulator was left with mobile data on.
pnpm check:affected --runpasses at d4f476e. The device-lane replay suites are GitHub-only and did not run locally.Not covered: multi-SIM devices, and a ping check that traffic actually moves between Wi-Fi and cellular.