Skip to content

feat(hardware-button): press phone volume and power keys once - #3859

Merged
thymikee merged 2 commits into
mainfrom
feat/hardware-button
Oct 21, 2026
Merged

thymikee merged 2 commits into
mainfrom
feat/hardware-button

Conversation

@thymikee

@thymikee thymikee commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

Closes #3848

Summary

Adds hardware-button <volume-up|volume-down|power>: one press of a phone hardware key, never foregrounding the session app. Single press; no aliases, hold, or raw codes; an unknown/missing positional refuses INVALID_ARGS before device dispatch. The buttons join the existing SYSTEM_BUTTONS family, so every owner's typed family denial covers them until it declares a cell. Android (ADB/helper; Appium via mobile: pressKey 24/25/26; Limrun shares the same phone/TV rule) presses all three on phones/tablets. A physical iPhone/iPad presses volume via the local runner (XCUIDevice) or Appium mobile: pressButton; power is unsupported on every Apple leaf; iOS Simulators, Android TV and tvOS refuse (TV: use tv-remote — #3817's merged TV buttons stay the TV route). Only scope edit: registry line-cap ratchet 2179→2192 (chore(gates) commit, additive descriptor).

agent-device hardware-button volume-down
client.command.hardwareButton({ button: 'power' })

Latent provider bug, untouched: Android webdriver home/appSwitcher legs call mobile: pressButton, which appium-uiautomator2-driver does not implement.

Validation

Head 066cb0d742 (rebased onto 71eae2adcd; round-5: shared Limrun/local phone-TV fact builder, shared positionals parser). device-SDK build-for-testing AgentDeviceRunner -destination 'generic/platform=iOS' → TEST BUILD SUCCEEDED (exit 0). Exact-head check:affected --run receipt and live emulator transcript: evidence comment on this PR. This PR's Swift delta is unchanged since its unit+UI run (449 tests, 0 failures). Not verified live: physical iPhone volume and Appium legs (no such device/session here) — fail-closed and fact/unit-covered. CI device lanes are the remote obligation.

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-21 12:32 UTC

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.48 MB 5.49 MB +10.2 kB
Package (unpacked) 5.48 MB 5.49 MB +10.2 kB
Package (download) 1.66 MB 1.66 MB +2.5 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 38.9 ms 39.4 ms +0.5 ms
CLI --help 80.6 ms 79.9 ms -0.6 ms

@thymikee
thymikee force-pushed the feat/hardware-button branch from 51f3fb4 to 06c07c3 Compare October 20, 2026 22:21
@thymikee

thymikee commented Oct 20, 2026 •

Copy link
Copy Markdown
Member Author

PR #3859 validation evidence — hardware-button (#3848) — round 5

Current head 066cb0d742 (two commits on origin/main 71eae2adcd: 28d22579f1 feat + 066cb0d742 chore(gates)). Supersedes the receipts at 06c07c3e75. This PR's own Swift delta is unchanged since that head's Swift unit+UI run (449 tests, 0 failures); the rebased tree also carries upstream's merged Apple change 702048a474, which CI's Apple lanes re-run.

Round-5 review items and dispositions

1. Limrun Android hardware-button cells (reviewer finding). Added: packages/provider-limrun/src/interaction-operations.ts now declares volumeUp/volumeDown/power cells for Android nodes. Admission is one shared contracts rule, androidHardwareButtonAvailability in packages/contracts/src/system-button-runtime.ts, used by both the local adb runtime and the Limrun provider — no cloned gate (a pnpm check:fallow clone group flagged the first duplicated attempt; sharing collapsed it, fallow is green). Phone/tablet nodes answer available (the Android leg's createAndroidInteractor carries the keyevent); a TV node refuses unsupported-device-kind / "hardware-button is not supported on Android TV targets; use tv-remote."; iOS legs keep the family denial. Facts test pins phone support and TV refusal: packages/provider-limrun/src/interaction-operations.test.ts and the rule's own test in packages/contracts/src/system-button-runtime.test.ts.

2. Device-SDK build (reviewer finding). Run on the round-5 working tree, which is byte-identical to this head under apple/ (git diff HEAD -- apple/ is empty, and it was empty at each intermediate head — the round-5 rework touched no Apple file):

$ xcodebuild build-for-testing \
    -project apple/runner/AgentDeviceRunner/AgentDeviceRunner.xcodeproj \
    -scheme AgentDeviceRunner \
    -destination 'generic/platform=iOS' \
    CODE_SIGNING_ALLOWED=NO \
    -derivedDataPath /private/var/folders/pn/0s6xww5x5tj2brz0nrvlx96w0000gn/T/opencode/runner-dd-device
...
** TEST BUILD SUCCEEDED **

Exit code 0; build products and AgentDeviceRunner_AgentDeviceRunnerUITests_iphoneos27.1-arm64.xctestrun remain under that derived-data path. No physical iPhone/iPad is connected to this host (devicectl list devices shows simulators only), so the device branch compiles but is not executed here — reported as a limitation, same as prior rounds. No Appium driver session was run either.

Prior-round Swift unit+UI bundle run (449 tests, 0 failures) was at the pre-rebase tree 06c07c3e75. This PR's own Swift delta is unchanged since; the tree additionally carries upstream's merged 702048a474 Apple changes, which the exact-head CI lanes re-run.

3. Design simplifications (reviewer suggestion) — measured:

  • Value-import spread (SYSTEM_BUTTONS/labels from one eager module): rejected with numbers. Importing HARDWARE_BUTTONS/labels as values from system-button-runtime.ts made it value-eager; scripts/__tests__/eager-closure-budgets.test.ts went red with 6 findings — system-button-runtime.ts newly joined 5 entry closures (interactor-operation-catalog, local-interactor-operation-set, platform-runtime-unavailable, system-button-runtime, the src/platform-runtime hub) and no existing shared home absorbs the edge. The rule forbids APPROVED_CLOSURE_ADDITIONS as the fix, so the vocabulary stays in device-rotation.ts (already an eager home) with the type-level drift pins in both directions.
  • Dispatcher switch case: rejected with numbers. Inlining the route in resolveGenericRuntimeExecution raised its cyclomatic complexity 21 → 22, which breaches the fallow high-complexity threshold (measured by temporarily editing the working tree and running pnpm check:fallow: 22 = 1 complexity finding; 21 = 0; the experiment edit was then reverted). The committed design (typed guard + delegation to the system-button owner) keeps the dispatcher at 21.
  • Shared positionals parser: adopted. parseHardwareButtonPositionals in device-rotation.ts owns arity + parse for the CLI reader and the daemon resolver. The redundant name→cell map was removed too — it is now derived by inverting HARDWARE_BUTTON_NAME_LABELS, so the table is stated once.

Live Android re-verification

Built-source CLI (node bin/agent-device.mjs, fresh pnpm build) on owned emulator-5554 (ReactNative API 35), isolated AGENT_DEVICE_STATE_DIR.

At the shared-helper head (1d5f1c928d, before the nit cleanup):

$ agent-device hardware-button volume-up   # x3, settings system volume_music_speaker
2 -> 2 -> 3 -> 4                            # moves with the presses
$ agent-device hardware-button mute
Error (INVALID_ARGS): hardware-button button must be one of: volume-up, volume-down, power.
$ agent-device hardware-button volume-up extra
Error (INVALID_ARGS): hardware-button accepts at most 1 positional argument(s), received 2: volume-up extra
$ agent-device hardware-button
Error (INVALID_ARGS): hardware-button requires exactly one button: volume-up, volume-down, power.

At the final head (066cb0d742, after the review-nit cleanup):

$ agent-device hardware-button volume-up
Pressed hardware button volume-up           # MediaSessionService "Adjusting suggestedStream ... by 1"
                                            # logged per press in logcat; the emulator's STREAM_MUSIC
                                            # was at an audio state where neither the CLI press nor a
                                            # raw `adb shell input keyevent 24` moved streamVolume in
                                            # that window (identical behavior for both routes — parity)
$ agent-device hardware-button power
Pressed hardware button power               # mWakefulness Awake -> Asleep
$ agent-device hardware-button power
Pressed hardware button power               # -> Awake again
$ agent-device hardware-button nope
Error (INVALID_ARGS): hardware-button button must be one of: volume-up, volume-down, power.

Sessions closed, daemons stopped gracefully, volume restored, state dirs removed.

Gates at this head

$ pnpm check:affected --run --head 066cb0d742
check:affected: all runnable checks passed.
[passed] exactCommit=true; receipt=.tmp/validation/run-SIkDl4/receipt.json

Base origin/main = 71eae2adcd. Skipped lanes are the device/remote gates CI owns (swift-runner-ios, replay-android, coverage, fuzzing, physical-device replay). An intermediate head (1d5f1c928d, before the review-nit cleanup) passed the same runnable set (run-Oo7DRh); this receipt is the one for the pushed head.

  • Local targeted set green: contracts, daemon execution/core, platform-android, provider-limrun, src/commands/system, session-journal (3,500+ tests), eager-closure budgets (853), layering, fallow, lint, typecheck, format.

Prior Coverage-lane failure — facts and disposition

The prior head's Coverage CI lane recorded 1 failed test out of 16,703 passed (58 skipped). It is not a coverage-threshold or uncovered-line failure. The failure is one assertion in test/integration/provider-scenarios/remote-upload-lease-beat.test.ts:379, test "a lease lost mid-upload aborts the upload and no install request goes out": daemon.releaseUpload() returned 'drained', the test expected 'canceled' — the fake daemon's grace-window race between lease-loss cancellation and upload drain resolved the other way in that run.

Facts, without asserting attribution either way: this diff touches zero daemon-server/daemon-client/upload/lease files (diff stat: none); the test passes solo and the full provider-integration project passes solo (65 files / 204 tests, 44.76s) at this head locally; the daemon quiesce/shutdown seam around this behavior was rewritten upstream after the failed run's base (8fc8fc1e8a, #3850), so latest main may already change the outcome. A solo local pass does not prove attribution. The new head's CI decides what this head does; if the same assertion fails again, it gets a base comparison per docs/agents/testing.md and goes to the #3850 seam owner rather than being asserted away or loosened.

@thymikee

Copy link
Copy Markdown
Member Author

Thanks for the PR. At 06c07c3 I found two problems, and I think both need to be fixed before merge.

Limrun Android refuses the new buttons. limrunSystemButtonOperationFacts sets only home and appSwitcher on the Android leg. So hardware-button volume-down or power on a Limrun Android phone falls to the family default and returns unsupported-provider-mode, "No Limrun session exposes this system button." The Limrun interactor comes from the same createAndroidInteractor factory, which now has all three members. The help text at src/commands/system/index.ts:336 and commands.md:127 say every ADB-backed Android backend presses all three. #3848 also needs Limrun Android to be available. The rule: every Android owner whose interactor comes from createAndroidInteractor declares the three hardware-button cells, using the local owner's phone and TV rule. Please add volumeUp, volumeDown and power to the Limrun Android branch, with a TV refusal if Limrun can lease TV targets, and add a Limrun facts test. If Limrun is out of scope on purpose, please narrow the help and docs to local ADB plus Appium, and drop the "Closes #3848" claim for that row.

The iOS device branch has never been compiled or run. The only branch that presses is #if os(iOS) && !targetEnvironment(simulator) in RunnerTests+Interaction.swift. Every CI lane builds the simulator SDK, and no CI job builds the runner for the device SDK. If that branch fails to compile, the runner build breaks for every physical iPhone session, not only for hardware-button. Please post an xcodebuild build-for-testing of AgentDeviceRunner with -destination 'generic/platform=iOS' at the new head, ending in TEST BUILD SUCCEEDED. Better still, on a physical iPhone, show agent-device hardware-button volume-up returning { action: 'hardware-button', button: 'volume-up' }, and a runner.log where the volumeUp command is answered ok.

Could the design be smaller? Export HARDWARE_BUTTONS and HARDWARE_BUTTON_NAME_LABELS from the vocabulary module, and have system-button-runtime.ts value-import and spread them into SYSTEM_BUTTONS and SYSTEM_BUTTON_LABELS. Only the reverse edge closes the cycle, and that module already type-imports device-rotation. The parse map then comes from inverting the labels, which removes both type-level pins, the inverse map and the long workaround comments. Could case 'hardware-button' dispatch to resolveBoundHardwareButtonRuntime(...) in the generic-runtime-execution switch, like orientation and focus? That would remove SystemButtonRouteCommand, isSystemButtonRouteCommand, the delegation branch and the positionals pass-through. Could one contracts helper, parseHardwareButtonPositionals, own the arity check and the parse for both the CLI reader and the daemon? The remaining production code looks like normal per-surface fan-out. First, please check that a value edge from system-button-runtime.ts to device-rotation.ts stays within scripts/__tests__/eager-closure-budgets.ts. I did not run that gate. No ADR is needed.

On CI, Smoke Tests looks unrelated to this change. The log shows TEST BUILD SUCCEEDED, then "The Apple runner budget ran out before this step began" with no assertion, and the later Swift test bundles ran with 0 failures. Coverage is unclear. The log I read stops mid-run with no failed test or threshold report, so I cannot attribute it.

Two limits on this review. I did not compile the Swift device branch, so I have not confirmed that XCUIDevice.Button.volumeUp and volumeDown exist for device builds. I also did not reproduce your Android transcript or the check:affected receipt. The Appium legs (Android pressKey 24/25/26, iOS pressButton volumeup) are still unverified live, so they remain a residual risk.

Before merge, please declare the hardware-button cells on Limrun Android, or narrow the help, docs and issue claim. Then please post the device-SDK runner build at the new head.

@thymikee
thymikee force-pushed the feat/hardware-button branch from 06c07c3 to 066cb0d Compare October 21, 2026 10:39
@thymikee

Copy link
Copy Markdown
Member Author

Round 5 addressed; head is now 066cb0d742 (rebased onto 71eae2adcd, two commits). Details in the updated evidence comment.

1. Limrun Android cells — added. provider-limrun/src/interaction-operations.ts declares volumeUp/volumeDown/power for Android nodes on the same admission rule the local adb runtime uses. The rule is now one shared contracts builder, androidHardwareButtonAvailability (contracts/system-button-runtime.ts) — my first pass duplicated the gate locally and pnpm check:fallow flagged the resulting clone group, so both owners call the shared rule now (fallow green). Facts tests pin phone support and the TV refusal in both provider-limrun/src/interaction-operations.test.ts and contracts/system-button-runtime.test.ts.

2. Device-SDK build — done at this head. xcodebuild build-for-testing -project apple/runner/AgentDeviceRunner/AgentDeviceRunner.xcodeproj -scheme AgentDeviceRunner -destination 'generic/platform=iOS' CODE_SIGNING_ALLOWED=NO (derived data under the temp path in the evidence comment) exited 0 with ** TEST BUILD SUCCEEDED **. No physical iPhone is attached to this host (devicectl list devices: simulators only), so the device branch compiles but is not executed here — stated as a limitation, not papered over. No Appium driver session was run either.

3. Smaller design — adopted where measurable, rejected with numbers where not.

  • Shared parseHardwareButtonPositionals: adopted; the CLI reader and daemon resolver now share arity + parse, and the redundant name→cell map is gone (derived by inverting HARDWARE_BUTTON_NAME_LABELS).
  • Value-import spread of SYSTEM_BUTTONS/labels from system-button-runtime.ts: measured, rejected. It made that module value-eager; the eager-closure gate went red with 6 findings — it newly joined 5 entry closures (interactor-operation-catalog, local-interactor-operation-set, platform-runtime-unavailable, system-button-runtime, the platform-runtime hub) with no existing shared home to absorb the edge, and the rule forbids passing via APPROVED_CLOSURE_ADDITIONS. The vocabulary already lives in device-rotation.ts, a module those entries evaluate at the merge-base, with compile-time drift pins in both directions.
  • Dispatcher case 'hardware-button': measured, rejected. Inlining raised resolveGenericRuntimeExecution cyclomatic complexity 21 → 22, which is 1 fallow complexity finding (21 is the passing threshold; measured by temporarily editing the working tree, running pnpm check:fallow, then reverting the edit with git checkout HEAD --). The committed guard + delegation keeps the dispatcher at 21.

Prior Coverage-lane failure at 06c07c3e75: one failed assertion (16,703 passed, 58 skipped — not a coverage-threshold failure), remote-upload-lease-beat.test.ts:379 "a lease lost mid-upload aborts the upload and no install request goes out", releaseUpload() answered drained where the test expects canceled. Facts without asserting attribution: this diff touches zero daemon-server/upload/lease files; the test and its whole project pass solo locally at head; the seam was rewritten upstream after that run's base (8fc8fc1e8a, #3850), so latest main may already change the outcome. A solo pass doesn't prove attribution — the new head's CI decides; if it recurs it gets a base comparison and goes to that seam owner.

Local gates at 066cb0d742: exact-head check:affected --run receipt in the evidence comment; live emulator transcript (volume moves, power Awake↔Asleep, exact INVALID_ARGS sentences) re-run after the shared-helper rework.

@thymikee

Copy link
Copy Markdown
Member Author

The PR is ready at 066cb0d. Both earlier issues are fixed: the Limrun Android cells are now declared, and the device-SDK build is posted. All 19 checks pass, and the earlier Coverage failure in remote-upload-lease-beat.test.ts did not recur. This diff does not touch the upload or lease code. No conflicts. It is ready for human review.

Not blocking: the three-line comment after systemButtonUnavailable in https://github.com/callstack/agent-device/blob/066cb0d/packages/provider-limrun/src/interaction-operations.ts#L142 is attached to no declaration, and the limrunSystemButtonOperationFacts doc block already states the rule, so you can delete it or leave it.

I did not run the iOS build-for-testing; the TEST BUILD SUCCEEDED result comes from your transcript. No physical iPhone run exists, so the XCUIDevice volume press is compiled but not exercised. I did not rerun your eager-closure and fallow-complexity measurements. I ran no live presses through the Appium legs or a live Limrun Android session. The Limrun cell uses the same Android interactor and scoped adb path that home and app-switcher already use there.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 21, 2026
@thymikee
thymikee merged commit c41dc3a into main Oct 21, 2026
19 checks passed
@thymikee
thymikee deleted the feat/hardware-button branch October 21, 2026 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(hardware-button): press phone volume and power buttons on Android and iOS

1 participant