Skip to content

TEST_REPORT.md

Latest commit

 

History

History
242 lines (179 loc) · 17.9 KB

TEST_REPORT.md

File metadata and controls

242 lines (179 loc) · 17.9 KB

Android SDK Code Review - Test Report

Date: 2026-02-12 Reviewer: QA Code Review (static analysis, no device execution) Files Reviewed:

  • FlutterSkillBridge.kt (931 lines)
  • ViewTraversal.kt (232 lines)
  • build.gradle.kts (46 lines)
  • README.md
  • bridge_protocol.dart (protocol spec)

Overall Score: 78 / 100

The SDK is a solid first implementation with correct protocol structure, good threading design, and a clean API surface. However, there are several bugs ranging from critical (security, potential deadlock) to minor (missing timeout in wait_for_element, incomplete Compose support) that should be addressed before a production release.

Criteria Assessment

1. Protocol Compliance - PASS

All 10 core bridge methods from bridge_protocol.dart are implemented:

Method Implemented Notes
initialize Yes Returns framework, sdk_version, platform, os_version
screenshot Yes PixelCopy (API 26+) with drawing cache fallback
inspect Yes Walks view hierarchy, returns interactive elements
tap Yes Uses performClick() via key lookup
enter_text Yes Sets text on EditText via key lookup
swipe Yes Dispatches synthetic MotionEvent sequence
scroll Yes Handles ScrollView, HorizontalScrollView, RecyclerView
find_element Yes Multi-strategy search (tag, contentDescription, ID, text)
get_text Yes Extracts text from TextView/EditText
wait_for_element Yes Single-check only (see Bug #5 below)

Three extended methods are also implemented: get_logs, clear_logs, go_back.

2. Health Check Endpoint - PASS

GET /.flutter-skill returns JSON with all required fields:

{
  "framework": "android-native",
  "app_name": "<configured>",
  "platform": "android",
  "sdk_version": "1.0.0",
  "capabilities": ["initialize", "screenshot", ...]
}

This matches the BridgeServiceInfo.fromHealthCheck() parser in the protocol spec. All five required fields (framework, app_name, platform, capabilities, sdk_version) are present.

3. WebSocket / JSON-RPC 2.0 - PASS (with issues)

WebSocket (RFC 6455):

  • Correctly computes Sec-WebSocket-Accept using SHA-1 + Base64 of key + magic GUID.
  • Handles text frames (opcode 0x01), close (0x08), ping/pong (0x09/0x0A).
  • Correctly handles masking for client-to-server frames.
  • Supports 7-bit, 16-bit (126), and 64-bit (127) payload lengths.
  • Server-to-client frames are unmasked (correct per RFC 6455).

JSON-RPC 2.0:

  • Includes "jsonrpc": "2.0" in all responses.
  • Propagates id from request to response.
  • Uses standard result / error object structure.
  • Error objects include code and message fields.

Issues: See Bug #7 (missing jsonrpc field validation) and Bug #8 (fragmented frames not supported).

4. Default Port - PASS

Port defaults to 18118, matching bridgeDefaultPort in bridge_protocol.dart (line 13). Configurable via the port parameter on start().

5. Code Quality - FAIL

Several issues identified:

  • Critical: Security issue with ServerSocket binding (Bug #1).
  • Critical: Potential deadlock in runOnMainThreadBlocking (Bug #2).
  • Major: Unsafe Long to Int cast for WebSocket payloads (Bug #3).
  • Major: CopyOnWriteArrayList.removeAt(0) is O(n) in a hot path (Bug #4).
  • Major: CoroutineScope is never properly managed after stop() (Bug #9).

6. Android Specifics - PASS (with issues)

  • Main-thread dispatch: runOnMainThreadBlocking correctly uses Handler(Looper.getMainLooper()) with CountDownLatch. Includes a 10-second timeout. Checks if already on main thread to avoid deadlock in that path.
  • PixelCopy: Correctly uses PixelCopy.request() on API 26+ with a fallback. Uses CountDownLatch with 5-second timeout. Falls back to drawing cache on failure.
  • Activity tracking: Uses Application.ActivityLifecycleCallbacks correctly. Sets currentActivity on onActivityResumed, clears on onActivityPaused.

Issues: See Bug #2 (deadlock potential when PixelCopy callback and runOnMainThreadBlocking compete for main thread).

7. Swipe/Touch Simulation - PASS (with minor issues)

  • dispatchSwipeGesture creates a realistic DOWN -> MOVE (10 steps) -> UP sequence.
  • Uses SystemClock.uptimeMillis() for proper event timing.
  • Events are properly recycled via recycle().
  • MotionEvent coordinates use screen coordinates via getLocationOnScreen().

Issue: Events are dispatched synchronously without delays between steps, which may not trigger velocity-based gesture detectors (fling, etc.) since the timestamps increment but wall-clock time does not. See Bug #6.

8. README Accuracy - FAIL

  • Incorrect claim: README states "The bridge server only listens on localhost (port 18118)". However, ServerSocket(port) binds to 0.0.0.0 (all interfaces), not localhost. See Bug #1.
  • Missing limitation: README mentions Jetpack Compose support ("Kotlin, Jetpack Compose, XML Views, or any combination") but ViewTraversal.kt has no Compose-specific traversal. Standard View-based findViewWithTag and children iteration will not discover @Composable elements unless they are wrapped in AndroidView or ComposeView exposes them as standard views. Compose semantics tree requires SemanticsNode traversal, which is absent. See Bug #10.
  • Missing limitation: wait_for_element is described as "Synchronous check if an element exists (server polls)" but the server-side implementation does a single check with no polling or timeout. See Bug #5.

Bugs Found

Critical

Bug #1: ServerSocket binds to all interfaces, not localhost

  • File: FlutterSkillBridge.kt, line 151
  • Code: serverSocket = ServerSocket(port)
  • Issue: ServerSocket(port) binds to 0.0.0.0 (all network interfaces). Any device on the same network can connect to the bridge and control the app. The README incorrectly claims it only listens on localhost.
  • Fix: Use ServerSocket(port, 50, java.net.InetAddress.getByName("127.0.0.1")) to bind exclusively to the loopback interface.
  • Impact: On a real device connected to WiFi, any attacker on the same network could send commands to the app.

Bug #2: Potential deadlock in screenshot capture

  • File: FlutterSkillBridge.kt, lines 796-818 and 882-906
  • Issue: handleScreenshot() calls runOnMainThreadBlocking { captureScreenshot(activity) }. Inside captureScreenshot, captureWithPixelCopy posts the PixelCopy callback to mainHandler (line 810). However, runOnMainThreadBlocking is already holding the main thread (the block is executing on the main thread via mainHandler.post). The PixelCopy callback is posted to the same mainHandler, but since the main thread is blocked waiting on the latch.await(5, TimeUnit.SECONDS) inside the already-running main-thread block, the callback will never execute during that time window.
  • Analysis: Actually, re-reading more carefully: runOnMainThreadBlocking posts the entire captureScreenshot block to the main thread. Inside that block, captureWithPixelCopy uses latch.await(5, TimeUnit.SECONDS) which would block the main thread. The PixelCopy callback is posted to mainHandler, which means it needs the main thread to execute. Since the main thread is blocked on latch.await(), this is a classic deadlock.
  • Result: PixelCopy will always time out (5 seconds), then fall back to the drawing cache method. The PixelCopy path is effectively dead code.
  • Fix: Run PixelCopy.request() from the IO thread with only the callback dispatched to main, or use a separate HandlerThread for the callback. Alternatively, restructure handleScreenshot to not run entirely on the main thread.

Major

Bug #3: Unsafe Long-to-Int cast for WebSocket payload length

  • File: FlutterSkillBridge.kt, line 327
  • Code: val payload = ByteArray(payloadLen.toInt())
  • Issue: payloadLen is a Long but is cast to Int for ByteArray allocation. For the 64-bit length case (payloadLen == 127), values exceeding Int.MAX_VALUE (2 GB) will silently overflow to a negative number, causing NegativeArraySizeException. Even values approaching Int.MAX_VALUE will cause OutOfMemoryError. There is no upper-bound validation.
  • Fix: Add a maximum payload size check (e.g., 16 MB) before allocation: if (payloadLen > 16 * 1024 * 1024) throw IOException("Payload too large").

Bug #4: O(n) removal on CopyOnWriteArrayList in log trimming

  • File: FlutterSkillBridge.kt, lines 121-123
  • Code: 
    while (logBuffer.size > MAX_LOG_ENTRIES) {
    logBuffer.removeAt(0)
}
  • Issue: CopyOnWriteArrayList.removeAt(0) copies the entire array on every removal. If multiple threads call log() simultaneously and overflow the buffer, each removal is O(n) with a full array copy. Additionally, the while loop has a TOCTOU race: size can change between check and removal when called concurrently, potentially removing more entries than intended.
  • Fix: Use a LinkedBlockingDeque with a capacity limit, or batch trim: check once, then remove excess in a single operation. Alternatively, use a ring buffer.

Bug #5: wait_for_element performs a single check with no waiting

  • File: FlutterSkillBridge.kt, lines 732-752
  • Issue: The method name implies it waits until the element appears (with a timeout), but the implementation does a single synchronous check and returns immediately. The protocol spec lists it as a core method, and the README describes it as "Synchronous check if an element exists (server polls)". The server-side polling design means the SDK implementation is technically acceptable, but a timeout_ms parameter is accepted by convention across other bridge SDKs and is missing here.
  • Recommendation: Add an optional timeout parameter (default e.g., 5000ms) that polls at intervals before giving up. This matches agent expectations.

Bug #9: CoroutineScope not recreated after stop()/start() cycle

  • File: FlutterSkillBridge.kt, lines 59, 103-112
  • Code: private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob())
  • Issue: stop() calls scope.cancel(), which permanently cancels the scope and its SupervisorJob. If start() is called again after stop(), the cancelled scope cannot launch new coroutines. All scope.launch calls will silently fail.
  • Fix: Create a new CoroutineScope in start() and store it in a var, or use a new SupervisorJob each time.

Minor

Bug #6: Swipe gesture has no real-time delays between MotionEvents

  • File: FlutterSkillBridge.kt, lines 577-611
  • Issue: The swipe gesture dispatches all 12 events (1 DOWN + 10 MOVE + 1 UP) synchronously with no Thread.sleep or SystemClock.sleep between them. While the event timestamps are correctly spaced (10ms apart), the actual wall-clock time between events is effectively zero. Some gesture detectors (e.g., VelocityTracker used by FlingHelper in RecyclerView) compute velocity from wall-clock time differences, not event timestamps. This could cause swipe-to-dismiss, fling scrolling, or ViewPager swiping to fail.
  • Fix: Add small delays between move events (e.g., SystemClock.sleep(stepDuration)) or dispatch events via postDelayed on the handler.

Bug #7: No JSON-RPC version validation on incoming requests

  • File: FlutterSkillBridge.kt, line 386
  • Issue: The handleJsonRpc method does not validate that the incoming request contains "jsonrpc": "2.0". Per JSON-RPC 2.0 spec, requests without this field are invalid and should return error code -32600 ("Invalid Request").
  • Impact: Low. Non-compliant clients would still work, which could mask protocol bugs.

Bug #8: WebSocket fragmented frames not supported

  • File: FlutterSkillBridge.kt, line 269
  • Issue: The WebSocket frame reader checks the opcode but does not handle continuation frames (opcode 0x00) or the FIN bit. If a client sends a fragmented message (FIN=0 on the first frame, continuation frames, FIN=1 on the last), the server will misinterpret the data. The FIN bit (b0 and 0x80) is read but never checked.
  • Impact: Low for typical JSON-RPC messages which are small, but could affect large screenshot responses or inspect results if a client library fragments frames.

Bug #10: No Jetpack Compose support despite README claim

  • File: ViewTraversal.kt, README.md
  • Issue: The README claims support for "Kotlin, Jetpack Compose, XML Views, or any combination". However, ViewTraversal only traverses the traditional View hierarchy using ViewGroup.children. Jetpack Compose renders into a single AndroidComposeView that does not expose individual composables as child View objects. To inspect Compose UI, the SDK would need to traverse the SemanticsNode tree via SemanticsOwner, which requires a dependency on androidx.compose.ui:ui-test or reflection into Compose internals.
  • Impact: Inspect, find_element, tap, get_text, and enter_text will not work for pure Compose UIs. Only XML Views and Compose elements wrapped in interop views will be discoverable.
  • Fix: Either add Compose semantics traversal or update the README to document this limitation clearly.

Bug #11: isInteractiveView includes views with any non-null tag

  • File: ViewTraversal.kt, line 92
  • Code: if (view.tag != null) return true
  • Issue: Many Android framework views set tag internally (e.g., RecyclerView item views, CoordinatorLayout behavior tags, Fragment views). This means isInteractiveView will return true for many non-interactive views, producing noisy inspect output.
  • Fix: Only treat tags of type String as developer-set keys: if (view.tag is String) return true.

Bug #12: extractText for ToggleButton checks isChecked but ToggleButton extends CompoundButton extends Button extends TextView

  • File: ViewTraversal.kt, lines 172-176
  • Issue: The when expression checks is EditText first, then is TextView, then is Button, then is ToggleButton. Since Button extends TextView, a Button will match is TextView before reaching is Button. Similarly, ToggleButton extends Button extends TextView, so a ToggleButton will match is TextView first and never reach the is ToggleButton branch. The when expression in Kotlin matches the first applicable branch.
  • Wait: Actually re-reading: the order is is EditText -> is TextView -> is Button -> is ToggleButton. EditText extends TextView, so EditText is correctly caught first. But Button extends TextView, so is TextView will match any Button, and ToggleButton extends Button extends TextView, so is TextView will match ToggleButton. The ToggleButton-specific logic (textOn/textOff) will never execute.
  • Fix: Reorder to most-specific first: is ToggleButton -> is EditText -> is Button -> is TextView.

Bug #13: mapViewType has similar inheritance ordering issue

  • File: ViewTraversal.kt, lines 131-152
  • Issue: ImageButton extends ImageView. The check view is ImageButton (line 134) comes before view is ImageView (line 142), which is correct. However, Switch extends CompoundButton extends Button extends TextView. The check view is Button (line 133) comes before view is Switch (line 137). A Switch will match is Button first and be typed as "button" instead of "switch". Same issue for CheckBox, RadioButton, ToggleButton which all extend Button (or CompoundButton extends Button).
  • Wait: Let me recheck. The order is: EditText -> Button -> ImageButton -> CheckBox -> RadioButton -> Switch -> ToggleButton -> SeekBar -> ... Since CheckBox extends CompoundButton extends Button, view is Button will match a CheckBox before the view is CheckBox branch. Similarly for RadioButton, Switch, ToggleButton.
  • Fix: Reorder to check subclasses first: CheckBox, RadioButton, Switch, ToggleButton should all come before Button.

Summary

# Severity Description
1 Critical ServerSocket binds 0.0.0.0 instead of 127.0.0.1
2 Critical PixelCopy deadlocks on main thread (always falls back to drawing cache)
3 Major No payload size limit on WebSocket frame read (OOM/crash)
4 Major O(n) array copy on every log trim with race condition
5 Major wait_for_element does not actually wait
9 Major CoroutineScope not recreatable after stop()
6 Minor Swipe gesture has no wall-clock delays between events
7 Minor No jsonrpc version validation on requests
8 Minor WebSocket fragmented frames not handled
10 Minor README claims Compose support but implementation is View-only
11 Minor Any non-null tag marks a view as interactive (noisy output)
12 Minor extractText ToggleButton branch unreachable due to inheritance order
13 Minor mapViewType misclassifies Switch/CheckBox/RadioButton as "button"

Recommendations

  1. Immediate (before release): Fix Bug #1 (bind to localhost) and Bug #2 (PixelCopy deadlock). These are the most impactful issues.

  2. High priority: Fix Bug #13 and #12 (type ordering). These cause incorrect element classification which directly affects agent accuracy. Simple reorder fix.

  3. Add payload size guard (Bug #3) to prevent crash on malformed WebSocket frames.

  4. Replace CopyOnWriteArrayList for log buffer (Bug #4) with a bounded ring buffer or ArrayDeque with synchronized access.

  5. Recreate CoroutineScope in start() (Bug #9) to support stop/start cycles.

  6. Clarify Compose support (Bug #10): Either implement SemanticsNode traversal for Compose UIs, or update the README to state that only traditional View-based UIs are supported. Compose support should be a tracked roadmap item.

  7. Add integration tests: Even without device execution, the JSON-RPC dispatch, WebSocket framing, and view traversal logic could be unit-tested with mocked View hierarchies.

Report generated by static code analysis. No code was executed on a device or emulator.