refactor: move the adb primitives from localnet to device - #48
Merged
Conversation
|
Warning Review limit reachedNext included review available in 8 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Your 88 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (26)
Comment |
|
adb is a device concern, but its primitives lived in `localnet/data-access` because localnet needed them first. Device then grew its own commands on top and had to reach back into localnet for them — including `device/data-access/device-types.ts`, whose `ConnectedDevice extends AdbDevice` made device's own type file depend on localnet's. The command-factory series made this visible: `playground-feature-serve.ts` alone carried six imports into localnet, five of them for adb. Moved to `device/data-access`: `adb-reverse.ts` and `list-adb-devices.ts` whole, and `openUrlOnDevice` split out of `probe-device-port.ts` into `open-url-on-device.ts`. `probeDevicePort` and `parseProbeExitCode` stay in localnet, since they take a `ResolvedLocalnetPort` and exist to prove a validator port is reachable. The `AdbDependencies`, `AdbDevice`, `AdbDeviceState` and `AdbReverseEntry` contracts move from `localnet-types.ts` into `device-types.ts`, next to the `ConnectedDevice` that extends one of them; localnet imports the two it still references rather than re-exporting anything. The edge count does not drop — this is about direction. `device -> localnet` goes from 15 imports to none, and device no longer depends on localnet at all; localnet, playground and emulator now consume device's adb contracts as ordinary consumers. No dependency cycle is introduced: the only mutual pair in `src/` is `device <-> emulator`, which predates this change and comes from #40's shared tweak UI plus `list-connected-devices.ts` reading emulator AVD names. The seven adb parser tests move from `test/localnet.test.ts` to `test/device.test.ts` with the code. `parseTcpPort` is still imported by the localnet suite, where it drives the fake adb harness rather than being the thing under test. `parseContainerStatus` and `parseProbeExitCode` stay behind. No behaviour change: 487 tests pass with and without a TTY, and `device list --json`, `device open`, and `localnet status --json` were each run against a live emulator, exercising the moved listing, reverse and intent paths end to end.
beeman
force-pushed
the
beeman/move-adb-primitives-to-device
branch
from
September 4, 2026 00:50
6c46831 to
7abeaad
Compare
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #47, now merged; this branch is rebased onto it and carries only the adb move.
adb is a device concern, but its primitives live in
localnet/data-accessbecause localnet needed them first. Device then grew its own commands on top and had to reach back into localnet to get them — includingdevice/data-access/device-types.ts, whoseConnectedDevice extends AdbDevicemade device's own type file depend on localnet's.The command-factory series (#41, #45, #46) made this visible: with each feature's imports in its own file,
playground-feature-serve.tsalone showed six imports into localnet, five of them for adb.What moved
To
device/data-access/:adb-reverse.ts— whole (listAdbReverses,parseAdbReverses,parseTcpPort,createAdbReverse,removeAdbReverse)list-adb-devices.ts— whole (listAdbDevices,parseAdbDevices,isUsableDevice)openUrlOnDevice— split out ofprobe-device-port.tsintoopen-url-on-device.tsAdbDependencies,AdbDevice,AdbDeviceState,AdbReverseEntry— out oflocalnet-types.tsintodevice-types.ts, next to theConnectedDevicethat extends one of themprobeDevicePortandparseProbeExitCodestay in localnet: they take aResolvedLocalnetPortand exist to prove a validator port is reachable from a device, so that file needed splitting rather than moving. localnet imports the two adb types it still references; it re-exports nothing.What this buys — direction, not edge count
Being precise, because the totals do not improve: 35 non-core cross-feature imports before, 39 after, across 7 edges either way. Splitting combined imports adds statements.
What changes is the arrow:
device -> localnetlocalnet -> deviceplayground -> localnetplayground -> deviceDevice no longer depends on localnet at all. adb primitives sit in the feature whose domain they are, and localnet, playground and emulator consume them as ordinary consumers.
No cycle is introduced. The only mutual pair in
src/isdevice <-> emulator, and it is byte-identical onorigin/main— it comes from #40's shared tweak UI pluslist-connected-devices.tsreading emulator AVD names.Tests
The seven adb parser tests move from
test/localnet.test.tstotest/device.test.tswith the code.parseTcpPortis still imported by the localnet suite, where it drives the fake adb harness rather than being the subject under test.parseContainerStatusandparseProbeExitCodestay behind with their code.Verification
bun run ci— build, lint,tsc -b --noEmit, 487 pass / 0 fail. Also 487/0 under a pseudo-TTY.Exercised against a live emulator rather than only through stubs, covering the moved listing, reverse and intent paths:
device list --json→[{ "serial": "emulator-5554", "state": "device", "name": "solana-mobile" }]device open 8081 --no-forward→Opened http://localhost:8081 on solana-mobile (emulator-5554)localnet status --json→ reports enginesurfpooland its rpc urlNo changeset: the
solana-mobilebin and the./templatesexport are unchanged.Noted, not addressed
doctor/data-access/check-android-devices.tshas its ownparseAdbDevicesandAdbDevicetype, unrelated to these and with a different shape (it carries akind). Two independent adb-device parsers is worth a look, but consolidating them is a behavioural question about what doctor reports, not part of this move.