Skip to content

feat(android): bring back the shipped plugins - #31

Open
easyhooon wants to merge 9 commits into
toss:feature/android-supportfrom
easyhooon:feature/android-plugins
Open

easyhooon wants to merge 9 commits into
toss:feature/android-supportfrom
easyhooon:feature/android-plugins

Conversation

@easyhooon

Copy link
Copy Markdown

Summary

Refs #17. Follows #30 and brings back the shipped Android plugins, as agreed there. It's rebased onto feature/android-support, so Necto now finds the app through ADB.

Review comment Commit
Files: recursive delete follows nested symlinks f80a763
Preferences: 2147483648 turns an Int key into Long 3335d37
OkHttp: a failed body read is recorded as COMPLETED 6a25077
Control: a cancelled tap or swipe sends no ACTION_CANCEL 6293634, plus a swipe test in 95d85af

Three other fixes:

  • 76dbfb2: the cURL command now quotes the method, URL and header names. Before, a quote in a URL could make a pasted command run what followed it.
  • 67d8c42: NectoAndroid.start(context) no longer creates the default plugins in a build that isn't debuggable. The default argument was evaluated before the debuggable check.
  • f0f7622: verification notes now match the rebase.

Questions before I go further

I've left these alone until you decide:

  1. Inbound frame limit. I asked this in feat(android): add the Android SDK under SDK/Android #30 and it's still open. The limit is 64 KiB on Android and 32 MiB on iOS. With the plugins back, two requests can reach it: files.write, and preferences.set when it carries a large string. Today a request over the limit closes the session and Necto reconnects. Both are documented as limitations. Should I raise the limit to match iOS, or keep 64 KiB and have the panels refuse larger values up front?
  2. Where the plugins live. On iOS, NectoDefaultPlugins is a target separate from NectoSDK. Here the plugins ship inside necto-core and necto-android, and start(context) registers all of them by default. As a result, every app that adopts the SDK gets the DataStore dependency. Should I split them into their own modules (for example necto-plugins, with necto-okhttp already separate) so that apps opt in? I'd do that in this PR or the next one, whichever you prefer.
  3. The CSP follow-up from feat(android): add the Android SDK under SDK/Android #30. It affects every panel, iOS included. Should it target main or feature/android-support? I'll check the built-in panels against their allowedOrigins there.

Changelog

Bring back the shipped Android plugins and fix the issues found in review.

Test Plan

  • cd SDK/Android && ./gradlew build: 31 necto-core tests, 8 necto-okhttp, 15 necto-android.
  • script/test: Swift, Mac app, web and release checks pass.
  • script/test-android-artifacts: no SDK permissions, 24 panel files match iOS, only the debug build is debuggable.
  • Over USB on a Galaxy S25+ (Android 17), after the last code change: script/test-android-sample and script/test-android-security both pass.
  • Android 17 emulator: both pass after the rebase. After the last change (67d8c42), test-android-sample passes. test-android-security timed out at its 10 s am start. The host Mac had a load average near 200 at the time, and a cold start took 9–18 s. It passed on USB with the same APK.

Not verified: Android E2E still isn't in CI. The manual plugin checks in SDK/Android/docs/verification.md ("Earlier device checks") predate the ADB connection and weren't repeated.

The Events, Network, Performance, Files, Preferences and UI Control
plugins, necto-okhttp and the sample screens that exercise them, as they
were before they moved out of the SDK change.
A symlink under a deleted folder led deleteRecursively into its target,
which can lie outside the configured root. Links are now removed as
links.
Closing the body after a failed read still reported the request as
completed. The read error now ends the record as FAILED with that error.
Writing 2147483648 to an Int key stored it as a Long, and the app then
threw ClassCastException reading it as an Int. A value outside the Int
range is now refused before the key is touched.
A gesture cancelled between down and up left the view pressed, and a tap
held that way turned into a long press.
The method, URL and header names went into the command unescaped, so a
quote in a URL the app requested ended the argument, and pasting the
command ran what followed.
A default argument is evaluated before the function body, so a build that
is not debuggable still created every plugin: UI Control registered its
activity callbacks and Files created the external directories. The
defaults are now built after the check.
Test counts and coverage match the fixes. The emulator and USB runs were
repeated over the ADB connection. preferences.set joins files.write under
the 64 KiB inbound limit.
@easyhooon

Copy link
Copy Markdown
Author

Emulator check over ADB

I repeated the plugin checks from "Earlier device checks" in SDK/Android/docs/verification.md over the new ADB connection. The setup was f0f7622 on a Pixel 10 emulator (API 37, arm64), started with script/android-sample emulator-5554. Each operation went through necto-cli, which uses the same schemas and permissions as the panels.

Automated

  • script/test-android-security: all 6 cases pass.
  • script/test-android-sample: passes.

Per plugin

Plugin Result
Network "Run every request" records all 10 requests. The 404 and 500 complete with their status, DELETE returns 204, and the unresolvable host is failed. The 293 KB large response arrives whole (isTruncated: false). Detail returns the headers, bodies and cURL.
Preferences settings is listed. Set, detail and remove work. 3000000000 is refused for an existing Int key and for a new key with type: "Int". A Long key (example.lastLaunch) still accepts its value.
Files Roots, list, write, preview, info and delete work. Deleting a directory that holds a link to keep/ removes the directory, and keep/target.txt survives. A link that points outside the root is refused by info, list and preview ("path escapes the root").
Performance The snapshot, metrics and performance.observe stream report every metric. FPS reads 58–59 while scrolling until the host gets loaded.
Events The list returns the boot event. Detail, the stream and resubscribe are covered by test-android-sample.
Control (Views) All 14 fixture steps pass: tap ×2, nav, replace and append Korean text, password (the value is not returned), notes, switch, tap at a position, 2 fingers × 2 taps, swipe scrolls poc.scroll, open detail and back.
Control (Compose) All 14 steps pass. When the keyboard is up, or a target is scrolled out of view, it is not offered until it is visible again, as documented.
Accessibility The disabled button and read-only text are not targets. Count tap works. The sheet hides the background, and back closes it.

Notes

  • Some Control calls hit the 5 s operation timeout: switching into the Control tab, a few inputs, and swipes. The host had a load average of about 9 with other VMs running, and logcat showed Choreographer: Skipped 31–55 frames at those moments. Each timed-out action had still landed when I read the screen afterwards, and the same calls took under 0.1 s once the screen was idle. I read this as host load rather than a code issue, but on a slow device the 600 ms settle can push some calls close to the budget.
  • control.back sends the key to the activity, so with the keyboard up it navigates instead of closing the keyboard. A hardware back press closes the keyboard first. This matches the operation's description; I'm only noting it here.
  • files.info on a link inside the root follows it and reports the target's name (keep), not the link's.
  • The USB path was not part of this run. It still needs a physical device.

@easyhooon

Copy link
Copy Markdown
Author

Device check over USB

This repeats the emulator check on a Galaxy S25+ (SM-S936N, Android 17 / API 37) connected over USB. The build was f0f7622, run with script/android-sample R3CY70P3F4H, so it covers the USB path the emulator could not.

Automated

  • script/test-android-security: all 6 cases pass (17.5 s).
  • script/test-android-sample: passes (6.7 s).

Per plugin

Plugin Result
Network "Run every request" records all 10 requests: 404 and 500 with their status, DELETE 204, and the unresolvable host as failed. The 293 KB large response arrives whole. cURL is returned.
Preferences Set and remove work. 3000000000 is refused for an existing Int key and for type: "Int". The Long key still accepts its value.
Files List, write, preview and delete work. Deleting a directory that holds a link to keep/ leaves keep/target.txt in place. A link out of the root is refused ("path escapes the root").
Performance Every metric is reported. FPS holds at about 120 while scrolling on the 120 Hz display.
Events The list returns the boot event.
Control (Views, Compose) All 14 fixture steps pass on both. Each call took about 0.7–1.2 s, with no timeouts. On Compose, targets below the keyboard are not offered while it is up, as documented.
Accessibility The disabled button and read-only text are not targets. Count tap works. The sheet hides the background, and back closes it.

On the device, none of the 5 s timeouts seen on the loaded emulator host happened. That supports reading them as host load.

Not covered

  • Unplugging the cable. test-android-sample covers disconnecting and reconnecting from the app side only.
  • One small thing I noticed: files.write into a directory that does not exist fails with PROVIDER_FAILED and the temp file's path (…/.a.txt.necto-<n>: open failed: ENOENT). Something like "the parent directory does not exist" would read better. I can change it here or in a follow-up, whichever you prefer.

@easyhooon

Copy link
Copy Markdown
Author

Unplugging the cable

Following up on the device check, I unplugged the Galaxy S25+ while the sample was connected and plugged it back in. Same build (f0f7622), run with script/android-sample R3CY70P3F4H.

What happened

  • On the unplug, Necto dropped the app from necto-cli device list, and the call in flight failed with TARGET_DISCONNECTED.
  • On the replug, ADB was back at once (adb devices lists the phone as device, run-as reads the endpoint). Necto found the app and forwarded its socket again.
  • The app did not reconnect. It stayed out of the list for several minutes, until I restarted it.

Why

Before the restart, /proc/net/unix on the phone showed the app's socket with one connection still established and 16 more waiting in the backlog, never accepted. Those 16 are Necto's probes after the replug.

NectoSdkRuntime.serve accepts one session and handles it to the end before it calls accept() again. The session from before the unplug was never closed on the phone, so the runtime is still blocked reading it and never gets to the new connections. handle() already closes a previous session when a new one arrives, but with the loop written this way a second session never reaches it.

After am force-stop and a relaunch, the app was listed again within 4 s and preferences.suites answered.

Scope

This is in the Android SDK from #30, not in the plugins this PR adds. The iOS runtime has the same loop shape. I did not unplug an iPhone, so I don't know whether usbmuxd closes the socket on the phone there, which would hide it.

Possible fixes: keep accepting while a session runs, and let the newer session replace the older one through the existing path in handle(). Or give the session a read timeout or keepalive. I can do this here or in a follow-up, whichever you prefer.

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.

1 participant