feat: add wda session caching #111
Conversation
|
Warning Rate limit exceeded@gmegidish has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 40 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThe changes refactor the device API to use configuration objects instead of positional parameters for Changes
Sequence DiagramsequenceDiagram
participant CLI as Command/CLI
participant Device as Device
participant Agent as Agent Service
participant WDA as WebDriver Agent
Note over CLI,WDA: Old Flow (Positional Parameters)
CLI->>Device: StartAgent()
Device->>Agent: Initialize
Agent-->>Device: Ready
Device-->>CLI: Success
Note over CLI,WDA: New Flow (Config Objects + Callbacks)
CLI->>Device: StartAgent(config{OnProgress})
Device->>Device: OnProgress("Installing...")
Device->>Agent: Initialize
Device->>Device: OnProgress("Launching...")
Agent-->>Device: Ready
Device->>Device: OnProgress("Waiting...")
Device-->>CLI: Success
Note over CLI,WDA: Screen Capture - Before (Positional)
CLI->>Device: StartScreenCapture(fmt, quality, scale, callback)
Device->>WDA: Stream MJPEG
WDA-->>Device: Data chunk
Device->>CLI: callback(data)
Note over CLI,WDA: Screen Capture - After (Config)
CLI->>Device: StartScreenCapture(config{Format, Quality, Scale, OnData, OnProgress})
Device->>Device: OnProgress("Starting stream...")
Device->>WDA: Stream MJPEG
WDA-->>Device: Data chunk
Device->>Device: config.OnData(chunk)
Device->>CLI: Data + Progress updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
devices/android.go (1)
638-694: Potential nil pointer dereference ifconfig.OnDatais nil.Unlike
OnProgresswhich is nil-checked before use (lines 643, 658),config.OnDatais called directly at line 686 without a nil check. If a caller passes a config without anOnDatacallback, this will panic.Apply this diff to add a nil check:
if n > 0 { // Send bytes to callback, break if it returns false + if config.OnData == nil { + break + } if !config.OnData(buffer[:n]) { break } }Alternatively, validate
OnDatais non-nil at the start of the function:func (d *AndroidDevice) StartScreenCapture(config ScreenCaptureConfig) error { if config.Format != "mjpeg" { return fmt.Errorf("unsupported format: %s, only 'mjpeg' is supported", config.Format) } + if config.OnData == nil { + return fmt.Errorf("OnData callback is required for screen capture") + }devices/simulator.go (1)
651-669: Document thatQualityandScaleinScreenCaptureConfigare Android-only features.The
QualityandScalefields are not utilized in the simulator'sStartScreenCaptureimplementation because the underlyingWdaMjpegClientdoes not support these parameters. While the Android implementation passes these to its customMjpegServer(line 663 in devices/android.go), iOS and simulator implementations ignore them. Consider adding documentation toScreenCaptureConfigclarifying thatQualityandScaleare only supported on Android devices, or refactor the config structure to avoid confusion about which fields apply to which platform.
🧹 Nitpick comments (8)
devices/wda/appium-settings.go (1)
6-10: Update comment and error message to reflect get-or-create semantics.The comment on line 6 and error message on line 9 state "create wda session," but the operation now gets or creates a session.
Apply this diff to improve accuracy:
- // create wda session + // get or create wda session sessionId, err := c.GetOrCreateSession() if err != nil { - return fmt.Errorf("failed to create wda session: %w", err) + return fmt.Errorf("failed to get or create wda session: %w", err) }devices/wda/press-button.go (1)
25-28: Update error message to reflect get-or-create semantics.The error message states "failed to create session," but the operation now gets or creates a session.
Apply this diff:
sessionId, err := c.GetOrCreateSession() if err != nil { - return fmt.Errorf("failed to create session: %v", err) + return fmt.Errorf("failed to get or create session: %v", err) }commands/screenshot.go (1)
11-11: StartAgent config wiring looks correct (optionally surface progress later)Using
targetDevice.StartAgent(devices.StartAgentConfig{})matches the updated API and keeps behavior unchanged. Later, you might want to pass anOnProgresscallback here to surface “starting agent…” feedback in the screenshot flow, but that’s optional.Also applies to: 57-57
commands/input.go (1)
7-7: Input commands correctly migrated; consider deduplicating StartAgent boilerplateAll input‑related commands now invoke
StartAgent(devices.StartAgentConfig{})before issuing WDA operations, which is consistent with the new API and existing error‑handling style.Given how many identical
StartAgentblocks there are now, you might consider a small helper in this file (e.g.,startAgentOrError(targetDevice)returning*CommandResponse/error) to reduce duplication and make it easier to add things likeOnProgresslater in a single place.Also applies to: 63-63, 89-89, 115-115, 141-141, 167-167, 208-208
cli/screenshot.go (1)
97-97: Quality is hardcoded; consider adding CLI flag for consistency.The
--scaleflag is exposed but quality usesDefaultMJPEGQuality. If this is intentional, no action needed. Otherwise, consider adding a--qualityflag for thescreencapturecommand similar to thescreenshotcommand.devices/ios.go (1)
347-363: Remove deadif trueconditional.The
if trueblock always executes, making it dead code. This appears to be leftover debugging or placeholder logic. Remove the conditional wrapper.- if true { - portMjpeg, err := findAvailablePort() + portMjpeg, err := findAvailablePort() + if err != nil { + return fmt.Errorf("failed to find available port for mjpeg: %w", err) + } + + portForwarderMjpeg := ios.NewPortForwarder(d.ID()) + err = portForwarderMjpeg.Forward(portMjpeg, 9100) + if err != nil { + return fmt.Errorf("failed to forward port for mjpeg: %w", err) + } + + mjpegUrl := fmt.Sprintf("http://localhost:%d/", portMjpeg) + d.mjpegClient = mjpeg.NewWdaMjpegClient(mjpegUrl) + utils.Verbose("Mjpeg client set up on %s", mjpegUrl) - if err != nil { - return fmt.Errorf("failed to find available port for mjpeg: %w", err) - } - - portForwarderMjpeg := ios.NewPortForwarder(d.ID()) - err = portForwarderMjpeg.Forward(portMjpeg, 9100) - if err != nil { - return fmt.Errorf("failed to forward port for mjpeg: %w", err) - } - - mjpegUrl := fmt.Sprintf("http://localhost:%d/", portMjpeg) - d.mjpegClient = mjpeg.NewWdaMjpegClient(mjpegUrl) - utils.Verbose("Mjpeg client set up on %s", mjpegUrl) - }server/server.go (2)
716-724: Consider handling write errors in progress callback.Write errors in
progressCallback(line 720) are silently ignored, meaning client disconnection during the agent startup phase won't be detected untilOnDatais invoked. Consider returning an error or signaling termination if the write fails.
739-751: Use structured logging instead offmt.Println.Line 742 uses
fmt.Printlnfor error logging. Use the project's logger (utils.Verboseorlog.Printf) for consistency with the rest of the codebase.OnData: func(data []byte) bool { _, writeErr := w.Write(data) if writeErr != nil { - fmt.Println("Error writing data:", writeErr) + utils.Verbose("Error writing data: %v", writeErr) return false }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
cli/device.go(2 hunks)cli/screenshot.go(3 hunks)commands/dump.go(1 hunks)commands/info.go(1 hunks)commands/input.go(7 hunks)commands/orientation.go(3 hunks)commands/screenshot.go(2 hunks)commands/url.go(2 hunks)devices/android.go(4 hunks)devices/common.go(3 hunks)devices/ios.go(4 hunks)devices/simulator.go(7 hunks)devices/wda/appium-settings.go(1 hunks)devices/wda/gesture.go(1 hunks)devices/wda/longpress.go(1 hunks)devices/wda/press-button.go(1 hunks)devices/wda/requests.go(1 hunks)devices/wda/send-keys.go(1 hunks)devices/wda/swipe.go(1 hunks)devices/wda/tap.go(1 hunks)devices/wda/types.go(1 hunks)devices/wda/windowsize.go(1 hunks)server/server.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
commands/url.go (1)
devices/common.go (1)
StartAgentConfig(30-32)
commands/orientation.go (1)
devices/common.go (1)
StartAgentConfig(30-32)
commands/info.go (1)
devices/common.go (1)
StartAgentConfig(30-32)
commands/dump.go (1)
devices/common.go (1)
StartAgentConfig(30-32)
cli/device.go (1)
devices/common.go (1)
StartAgentConfig(30-32)
devices/wda/requests.go (2)
devices/wda/types.go (1)
WdaClient(9-13)utils/logger.go (1)
Verbose(19-23)
devices/ios.go (2)
devices/common.go (3)
StartAgentConfig(30-32)ScreenCaptureConfig(21-27)DefaultMJPEGFramerate(17-17)utils/logger.go (1)
Verbose(19-23)
devices/android.go (2)
devices/common.go (2)
StartAgentConfig(30-32)ScreenCaptureConfig(21-27)utils/logger.go (1)
Verbose(19-23)
commands/input.go (1)
devices/common.go (1)
StartAgentConfig(30-32)
devices/simulator.go (2)
devices/common.go (2)
StartAgentConfig(30-32)ScreenCaptureConfig(21-27)devices/wda/mjpeg/types.go (1)
NewWdaMjpegClient(7-11)
server/server.go (1)
devices/common.go (2)
StartAgentConfig(30-32)ScreenCaptureConfig(21-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint
🔇 Additional comments (21)
devices/wda/longpress.go (1)
9-12: LGTM: Session caching implementation.The switch to GetOrCreateSession() and removal of DeleteSession enables session reuse across operations, improving performance by avoiding redundant session creation overhead.
devices/wda/send-keys.go (1)
7-10: LGTM: Consistent session management.The change correctly adopts the session caching pattern, consistent with other WDA operations.
devices/wda/tap.go (1)
9-12: LGTM: Session caching adopted.Correctly implements the session caching pattern.
commands/info.go (1)
20-23: LGTM: Updated to new config-based signature.The call site correctly adopts the new StartAgentConfig-based API. Passing an empty config is appropriate for this use case where no progress callback is needed.
devices/wda/windowsize.go (1)
18-21: LGTM: Session caching applied.Correctly implements the session caching pattern consistent with other WDA operations.
devices/wda/gesture.go (1)
9-9: Gesture now uses cached WDA sessionSwitching to
GetOrCreateSession()is consistent with the new session‑caching model and keeps the method simple. Just confirm that there is a clear strategy elsewhere for eventually clearing/rotating the cached session if you ever need to change capabilities or reset state on the WDA side.devices/wda/swipe.go (1)
9-9: Swipe aligned with session caching
Swipenow also usesGetOrCreateSession(), matching the new cached‑session flow used by other WDA actions. This keeps behavior consistent across gestures.commands/dump.go (1)
27-27: DumpUI StartAgent call updated correctlyThe change to
targetDevice.StartAgent(devices.StartAgentConfig{})cleanly adapts to the new signature without altering control flow or error handling.commands/url.go (1)
5-7: URL command correctly migrated to StartAgent config APIAdding the
devicesimport and callingStartAgent(devices.StartAgentConfig{})brings this command in line with the new device API, while preserving existing validation and error messages.Also applies to: 26-26
commands/orientation.go (1)
5-7: Orientation commands correctly wired to StartAgent configBoth orientation commands now call
device.StartAgent(devices.StartAgentConfig{}), matching the new API and keeping the existing error handling intact. This looks consistent with the rest of the command layer.Also applies to: 33-33, 63-63
cli/device.go (1)
7-7: LGTM!The import and invocation changes correctly align with the updated
StartAgentinterface. Passing an emptyStartAgentConfig{}is valid sinceOnProgressis optional.Also applies to: 51-51
devices/common.go (2)
20-32: LGTM! Well-structured configuration types.The config structs properly support optional callbacks via nil-checks at call sites. The
OnDatacallback returningboolis a clean pattern for controlling stream termination.
55-55: Interface changes look good.The signature updates for
StartAgentandStartScreenCapturealign with the new config-based approach used across the codebase.Also applies to: 65-65
cli/screenshot.go (2)
77-81: Good use of progress callbacks.Writing progress messages to stderr is the correct approach since stdout is reserved for the MJPEG data stream.
95-110: Clean config-based screen capture implementation.The
OnDatacallback correctly returnsfalseon write errors to terminate the stream. The error is also logged to stderr for visibility.devices/simulator.go (2)
304-320: Good progress reporting for WDA installation.The optional
onProgresscallback is nil-checked before use, providing visibility into download and installation steps.
486-499: Session caching implementation looks correct.The logic properly checks for an existing WDA process, validates it via
GetStatus(), and reuses the session if valid. This avoids unnecessary agent restarts.devices/android.go (2)
387-395: LGTM!Android devices don't require an agent for online devices. Accepting
StartAgentConfigmaintains interface consistency across device types.
643-660: Good progress callbacks with proper nil checks.The
OnProgresscallbacks are correctly nil-checked before invocation, making them truly optional.devices/ios.go (1)
286-344: LGTM!The progress callback invocations are properly nil-checked and the messages reflect the actual operations being performed at each step.
server/server.go (1)
533-536: LGTM!Empty
StartAgentConfig{}is appropriate here sincedevice_infodoesn't require progress streaming.
| func (d IOSDevice) StartScreenCapture(config ScreenCaptureConfig) error { | ||
| // configure mjpeg framerate | ||
| err := d.wdaClient.SetMjpegFramerate(DefaultMJPEGFramerate) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return d.mjpegClient.StartScreenCapture(format, callback) | ||
| if config.OnProgress != nil { | ||
| config.OnProgress("Starting video stream") | ||
| } | ||
|
|
||
| return d.mjpegClient.StartScreenCapture(config.Format, config.OnData) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Script executed:
#!/bin/bash
# First, find files related to mjpeg and wda clients
fd -type f -name "*.go" | grep -E "(mjpeg|wda)" | head -20Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stderr:
error: 'ype' isn't a valid value for '--type <filetype>'
[possible values: file, directory, symlink, executable, empty, socket, pipe]
For more information try '--help'
</shell_result>
Script executed:
#!/bin/bash
# Find files related to mjpeg and wda
find . -name "*.go" | grep -iE "(mjpeg|wda)" | head -20Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
./devices/wda/mjpeg/health.go
./devices/wda/mjpeg/screencapture.go
./devices/wda/mjpeg/types.go
./devices/wda/open-url.go
./devices/wda/orientation.go
./devices/wda/screenshot.go
./devices/wda/source.go
./devices/wda/status.go
./devices/wda/appium-settings.go
./devices/wda/gesture.go
./devices/wda/longpress.go
./devices/wda/press-button.go
./devices/wda/requests.go
./devices/wda/send-keys.go
./devices/wda/swipe.go
./devices/wda/tap.go
./devices/wda/types.go
./devices/wda/windowsize.go
</shell_result>
Script executed:
#!/bin/bash
# Check the StartScreenCapture method signature in mjpeg client
cat -n ./devices/wda/mjpeg/screencapture.goRepository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
1 package mjpeg
2
3 import (
4 "context"
5 "fmt"
6 "io"
7 "net/http"
8 )
9
10 func (c *WdaMjpegClient) StartScreenCapture(format string, callback func([]byte) bool) error {
11
12 client := &http.Client{
13 Timeout: 0, // No timeout for long-lived streaming requests
14 }
15
16 req, err := http.NewRequestWithContext(context.Background(), "GET", c.baseURL, nil)
17 if err != nil {
18 return fmt.Errorf("failed to create request: %w", err)
19 }
20
21 req.Header.Set("Accept", "*/*")
22
23 // Make the request
24 resp, err := client.Do(req)
25 if err != nil {
26 return fmt.Errorf("failed to make request: %w", err)
27 }
28
29 defer func() { _ = resp.Body.Close() }()
30
31 if resp.StatusCode != http.StatusOK {
32 return fmt.Errorf("unexpected status code: %d", resp.StatusCode)
33 }
34
35 buffer := make([]byte, 65536)
36 for {
37 n, err := resp.Body.Read(buffer)
38 if n > 0 {
39 if !callback(buffer[:n]) {
40 // client wants to stop the stream
41 fmt.Println("Screen capture ended by client")
42 break
43 }
44 }
45
46 if err != nil {
47 if err == io.EOF {
48 // Normal end of stream
49 break
50 }
51
52 return fmt.Errorf("error reading response body: %w", err)
53 }
54 }
55
56 fmt.Println("Screen capture ended")
57 return nil
58 }
</shell_result>
Script executed:
#!/bin/bash
# Check ScreenCaptureConfig definition in devices/common.go
cat -n ./devices/common.go | head -50Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
1 package devices
2
3 import (
4 "fmt"
5
6 "github.com/mobile-next/mobilecli/devices/wda"
7 "github.com/mobile-next/mobilecli/types"
8 "github.com/mobile-next/mobilecli/utils"
9 )
10
11 const (
12 // Default MJPEG streaming quality (1-100)
13 DefaultMJPEGQuality = 80
14 // Default MJPEG streaming scale (0.1-1.0)
15 DefaultMJPEGScale = 1.0
16 // Default MJPEG streaming framerate (frames per second)
17 DefaultMJPEGFramerate = 30
18 )
19
20 // ScreenCaptureConfig contains configuration for screen capture operations
21 type ScreenCaptureConfig struct {
22 Format string
23 Quality int
24 Scale float64
25 OnProgress func(message string) // optional progress callback
26 OnData func([]byte) bool // data callback - return false to stop
27 }
28
29 // StartAgentConfig contains configuration for agent startup operations
30 type StartAgentConfig struct {
31 OnProgress func(message string) // optional progress callback
32 }
33
34 // ScreenElementRect represents the rectangle coordinates and dimensions
35 // Re-export types for backward compatibility
36 type ScreenElementRect = types.ScreenElementRect
37 type ScreenElement = types.ScreenElement
38
39 type ControllableDevice interface {
40 ID() string
41 Name() string
42 Platform() string // e.g., "ios", "android"
43 DeviceType() string // e.g., "real", "simulator", "emulator"
44 Version() string // OS version
45 State() string // e.g., "online", "offline"
46
47 TakeScreenshot() ([]byte, error)
48 Reboot() error
49 Boot() error // boot simulator/emulator
50 Shutdown() error // shutdown simulator/emulator
</shell_result>
Script executed:
#!/bin/bash
# Verify the ios.go implementation
sed -n '645,657p' ./devices/ios.goRepository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
func (d IOSDevice) StartScreenCapture(config ScreenCaptureConfig) error {
// configure mjpeg framerate
err := d.wdaClient.SetMjpegFramerate(DefaultMJPEGFramerate)
if err != nil {
return err
}
if config.OnProgress != nil {
config.OnProgress("Starting video stream")
}
return d.mjpegClient.StartScreenCapture(config.Format, config.OnData)
}
</shell_result>
Script executed:
#!/bin/bash
# Search for SetMjpeg methods in wda client files
rg -n "SetMjpeg" ./devices/wda/ -A 3Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
./devices/wda/appium-settings.go:24:func (c *WdaClient) SetMjpegFramerate(framerate int) error {
./devices/wda/appium-settings.go-25- err := c.SetAppiumSettings(map[string]interface{}{
./devices/wda/appium-settings.go-26- "mjpegServerFramerate": framerate,
./devices/wda/appium-settings.go-27- })
</shell_result>
Script executed:
#!/bin/bash
# Search for TODO/FIXME comments related to quality or scale
rg -in "todo|fixme" ./devices/ -A 2 | grep -iE "(quality|scale|mjpeg)" -B 2 -A 2Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 1
</shell_result>
Script executed:
#!/bin/bash
# Check if there's an Android device implementation
find . -name "*.go" | xargs grep -l "type AndroidDevice"Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
./devices/android.go
</shell_result>
Script executed:
#!/bin/bash
# Check Android StartScreenCapture implementation
rg -n "StartScreenCapture" ./devices/android.go -A 15Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
638:func (d *AndroidDevice) StartScreenCapture(config ScreenCaptureConfig) error {
639- if config.Format != "mjpeg" {
640- return fmt.Errorf("unsupported format: %s, only 'mjpeg' is supported", config.Format)
641- }
642-
643- if config.OnProgress != nil {
644- config.OnProgress("Installing DeviceKit")
645- }
646-
647- utils.Verbose("Ensuring DeviceKit is installed...")
648- err := d.EnsureDeviceKitInstalled()
649- if err != nil {
650- return fmt.Errorf("failed to ensure DeviceKit is installed: %v", err)
651- }
652-
653- appPath, err := d.GetAppPath("com.mobilenext.devicekit")
</shell_result>
Script executed:
#!/bin/bash
# Get more of Android StartScreenCapture
sed -n '638,700p' ./devices/android.goRepository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
func (d *AndroidDevice) StartScreenCapture(config ScreenCaptureConfig) error {
if config.Format != "mjpeg" {
return fmt.Errorf("unsupported format: %s, only 'mjpeg' is supported", config.Format)
}
if config.OnProgress != nil {
config.OnProgress("Installing DeviceKit")
}
utils.Verbose("Ensuring DeviceKit is installed...")
err := d.EnsureDeviceKitInstalled()
if err != nil {
return fmt.Errorf("failed to ensure DeviceKit is installed: %v", err)
}
appPath, err := d.GetAppPath("com.mobilenext.devicekit")
if err != nil {
return fmt.Errorf("failed to get app path: %v", err)
}
if config.OnProgress != nil {
config.OnProgress("Starting MJPEG server")
}
utils.Verbose("Starting MJPEG server with app path: %s", appPath)
cmdArgs := append([]string{"-s", d.getAdbIdentifier()}, "exec-out", fmt.Sprintf("CLASSPATH=%s", appPath), "app_process", "/system/bin", "com.mobilenext.devicekit.MjpegServer", "--quality", fmt.Sprintf("%d", config.Quality), "--scale", fmt.Sprintf("%.2f", config.Scale))
utils.Verbose("Running command: %s %s", getAdbPath(), strings.Join(cmdArgs, " "))
cmd := exec.Command(getAdbPath(), cmdArgs...)
stdout, err := cmd.StdoutPipe()
if err != nil {
return fmt.Errorf("failed to create stdout pipe: %v", err)
}
if err := cmd.Start(); err != nil {
return fmt.Errorf("failed to start MJPEG server: %v", err)
}
// Read bytes from the command output and send to callback
buffer := make([]byte, 65536)
for {
n, err := stdout.Read(buffer)
if err != nil {
break
}
if n > 0 {
// Send bytes to callback, break if it returns false
if !config.OnData(buffer[:n]) {
break
}
}
}
_ = cmd.Process.Kill()
return nil
}
func (d *AndroidDevice) installPackage(apkPath string) error {
output, err := d.runAdbCommand("install", apkPath)
if err != nil {
return fmt.Errorf("failed to install package: %v\nOutput: %s", err, string(output))
}
</shell_result>
Quality and Scale parameters are silently ignored in iOS but actively used in Android.
The review comment is correct. While ScreenCaptureConfig includes Quality and Scale fields (devices/common.go lines 20-26) with defined defaults, the iOS implementation doesn't pass these to the mjpegClient. In contrast, the Android implementation (devices/android.go line 664) properly passes both parameters:
cmdArgs := append([]string{...}, "--quality", fmt.Sprintf("%d", config.Quality), "--scale", fmt.Sprintf("%.2f", config.Scale))The iOS mjpegClient.StartScreenCapture method signature (devices/wda/mjpeg/screencapture.go line 10) only accepts format and callback parameters, and no corresponding SetMjpegQuality or SetMjpegScale methods exist on the wdaClient to configure these via appium settings (only SetMjpegFramerate exists).
This creates platform-inconsistent behavior where callers providing quality/scale values for iOS devices will have those values silently ignored.
🤖 Prompt for AI Agents
In devices/ios.go around lines 645-657, the iOS StartScreenCapture ignores
ScreenCaptureConfig.Quality and .Scale causing silent platform inconsistency;
update the call to mjpegClient.StartScreenCapture to accept and forward
config.Quality and config.Scale (e.g., StartScreenCapture(format, quality,
scale, callback)), then update the mjpeg client/wda implementation
(devices/wda/mjpeg/screencapture.go and any mjpeg client interfaces) to accept
those parameters and apply them when starting the MJPEG stream (or alternatively
add SetMjpegQuality/SetMjpegScale on the wdaClient and call them before
StartScreenCapture); ensure signatures and callers are updated accordingly so
iOS honors Quality and Scale like Android.
| type WdaClient struct { | ||
| baseURL string | ||
| httpClient *http.Client | ||
| sessionId string |
There was a problem hiding this comment.
🧩 Analysis chain
Script executed:
#!/bin/bash
# First, let's examine the WdaClient struct definition and the sessionId field
cat -n devices/wda/types.go | head -50Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
1 package wda
2
3 import (
4 "net/http"
5 "strings"
6 "time"
7 )
8
9 type WdaClient struct {
10 baseURL string
11 httpClient *http.Client
12 sessionId string
13 }
14
15 func NewWdaClient(hostPort string) *WdaClient {
16 baseURL := hostPort
17 if !strings.HasPrefix(baseURL, "http://") && !strings.HasPrefix(baseURL, "https://") {
18 baseURL = "http://" + baseURL
19 }
20 baseURL = strings.TrimSuffix(baseURL, "/")
21
22 return &WdaClient{
23 baseURL: baseURL,
24 httpClient: &http.Client{
25 Timeout: 5 * time.Second,
26 },
27 }
28 }
29
30 type TapAction struct {
31 Type string `json:"type"`
32 Duration int `json:"duration,omitempty"`
33 X int `json:"x,omitempty"`
34 Y int `json:"y,omitempty"`
35 Button int `json:"button,omitempty"`
36 }
37
38 type PointerParameters struct {
39 PointerType string `json:"pointerType"`
40 }
41
42 type Pointer struct {
43 Type string `json:"type"`
44 ID string `json:"id"`
45 Parameters PointerParameters `json:"parameters"`
46 Actions []TapAction `json:"actions"`
47 }
48
49 type ActionsRequest struct {
50 Actions []Pointer `json:"actions"`
</shell_result>
Script executed:
#!/bin/bash
# Search for GetOrCreateSession method
rg -nP --type=go 'GetOrCreateSession|func.*sessionId'Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
devices/wda/send-keys.go:7: sessionId, err := c.GetOrCreateSession()
devices/wda/windowsize.go:18: sessionId, err := c.GetOrCreateSession()
devices/wda/tap.go:9: sessionId, err := c.GetOrCreateSession()
devices/wda/swipe.go:9: sessionId, err := c.GetOrCreateSession()
devices/wda/longpress.go:9: sessionId, err := c.GetOrCreateSession()
devices/wda/gesture.go:9: sessionId, err := c.GetOrCreateSession()
devices/wda/requests.go:150:func (c *WdaClient) isSessionStillValid(sessionId string) bool {
devices/wda/requests.go:156:// GetOrCreateSession returns cached session or creates a new one
devices/wda/requests.go:157:func (c *WdaClient) GetOrCreateSession() (string, error) {
devices/wda/requests.go:177:func (c *WdaClient) DeleteSession(sessionId string) error {
devices/wda/orientation.go:13: defer func() { _ = c.DeleteSession(sessionId) }()
devices/wda/orientation.go:51: defer func() { _ = c.DeleteSession(sessionId) }()
devices/wda/appium-settings.go:7: sessionId, err := c.GetOrCreateSession()
devices/wda/press-button.go:25: sessionId, err := c.GetOrCreateSession()
devices/wda/open-url.go:11: defer func() { _ = c.DeleteSession(sessionId) }()
</shell_result>
Script executed:
#!/bin/bash
# Read the GetOrCreateSession method implementation
sed -n '150,180p' devices/wda/requests.goRepository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
func (c *WdaClient) isSessionStillValid(sessionId string) bool {
endpoint := fmt.Sprintf("session/%s", sessionId)
_, err := c.GetEndpoint(endpoint)
return err == nil
}
// GetOrCreateSession returns cached session or creates a new one
func (c *WdaClient) GetOrCreateSession() (string, error) {
// if we have a cached session, validate it first
if c.sessionId != "" {
if c.isSessionStillValid(c.sessionId) {
return c.sessionId, nil
}
// session is invalid, clear it and create a new one
utils.Verbose("cached session %s is invalid, creating new session", c.sessionId)
c.sessionId = ""
}
sessionId, err := c.CreateSession()
if err != nil {
return "", err
}
c.sessionId = sessionId
return sessionId, nil
}
func (c *WdaClient) DeleteSession(sessionId string) error {
url := fmt.Sprintf("session/%s", sessionId)
_, err := c.DeleteEndpoint(url)
if err != nil {
</shell_result>
Script executed:
#!/bin/bash
# Search for goroutine patterns - looking for 'go ' keyword
rg -nP --type=go '\bgo\s+' -B2 -A2
# Also search for concurrent access patterns
echo "=== Searching for goroutine launches ==="
rg -nP --type=go 'go\s+func' -B1 -A3Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
devices/ios.go-382-
devices/ios.go-383- // start WDA in background using testmanagerd similar to go-ios runwda command
devices/ios.go:384: go func() {
devices/ios.go-385- defer cancel()
devices/ios.go-386- _, err := testmanagerd.RunTestWithConfig(ctx, testmanagerd.TestConfig{
--
devices/android.go-460-
devices/android.go-461- // monitor context cancellation to clean up the process only on timeout
devices/android.go:462: go func() {
devices/android.go-463- <-ctx.Done()
devices/android.go-464- if cmd.Process != nil && ctx.Err() == context.DeadlineExceeded {
--
devices/ios/tunnel-manager.go-65-
devices/ios/tunnel-manager.go-66- // Start the tunnel manager update loop for this device
devices/ios/tunnel-manager.go:67: go func() {
devices/ios/tunnel-manager.go-68- defer func() {
devices/ios/tunnel-manager.go-69- tm.tunnelMutex.Lock()
=== Searching for goroutine launches ===
devices/android.go-461- // monitor context cancellation to clean up the process only on timeout
devices/android.go:462: go func() {
devices/android.go-463- <-ctx.Done()
devices/android.go-464- if cmd.Process != nil && ctx.Err() == context.DeadlineExceeded {
devices/android.go-465- utils.Verbose("Boot timeout exceeded, killing emulator process")
--
devices/ios.go-383- // start WDA in background using testmanagerd similar to go-ios runwda command
devices/ios.go:384: go func() {
devices/ios.go-385- defer cancel()
devices/ios.go-386- _, err := testmanagerd.RunTestWithConfig(ctx, testmanagerd.TestConfig{
devices/ios.go-387- BundleId: bundleID,
--
devices/ios/tunnel-manager.go-66- // Start the tunnel manager update loop for this device
devices/ios/tunnel-manager.go:67: go func() {
devices/ios/tunnel-manager.go-68- defer func() {
devices/ios/tunnel-manager.go-69- tm.tunnelMutex.Lock()
devices/ios/tunnel-manager.go-70- tm.updateCtx = nil
</shell_result>
Script executed:
#!/bin/bash
# Search for WdaClient instantiation and usage
echo "=== WdaClient instantiation ==="
rg -nP --type=go 'NewWdaClient|&WdaClient' -B1 -A1
echo "=== WdaClient field/variable declarations ==="
rg -nP --type=go '\bwdaClient|*WdaClient' -B1 -A1 | head -50Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
=== WdaClient instantiation ===
devices/ios.go-84- device.tunnelManager = tunnelManager
devices/ios.go:85: device.wdaClient = wda.NewWdaClient("localhost:8100")
devices/ios.go-86- return device, nil
--
devices/ios.go-307-
devices/ios.go:308: d.wdaClient = wda.NewWdaClient(fmt.Sprintf("http://localhost:%d", port))
devices/ios.go-309-
--
devices/simulator.go-490- // update our instance with new client
devices/simulator.go:491: s.wdaClient = wda.NewWdaClient(fmt.Sprintf("localhost:%d", currentPort))
devices/simulator.go-492- if _, err := s.wdaClient.GetStatus(); err == nil {
--
devices/simulator.go-544- // update WDA client to use the actual port
devices/simulator.go:545: s.wdaClient = wda.NewWdaClient(fmt.Sprintf("localhost:%d", usePort))
devices/simulator.go-546-
--
devices/wda/types.go-14-
devices/wda/types.go:15:func NewWdaClient(hostPort string) *WdaClient {
devices/wda/types.go-16- baseURL := hostPort
--
devices/wda/types.go-21-
devices/wda/types.go:22: return &WdaClient{
devices/wda/types.go-23- baseURL: baseURL,
=== WdaClient field/variable declarations ===
stderr:
PCRE2: error compiling pattern at offset 12: quantifier does not follow a repeatable item
</shell_result>
Script executed:
#!/bin/bash
# Search for device usage patterns and look for concurrent access
echo "=== Search for device handlers/operations ==="
rg -nP --type=go 'func.*\(.*[Dd]evice.*\)' | grep -E '(Tap|Swipe|SendKeys|PressButton|GetWindowSize)' | head -20
echo "=== Search for device receiver patterns ==="
rg -nP --type=go 'func\s*\([a-z]\s+\*Device' | head -20Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
=== Search for device handlers/operations ===
devices/simulator.go:559:func (s SimulatorDevice) PressButton(key string) error {
devices/simulator.go:563:func (s SimulatorDevice) SendKeys(text string) error {
devices/simulator.go:567:func (s SimulatorDevice) Tap(x, y int) error {
devices/simulator.go:575:func (s SimulatorDevice) Swipe(x1, y1, x2, y2 int) error {
devices/simulator.go:579:func (s SimulatorDevice) Gesture(actions []wda.TapAction) error {
devices/ios.go:138:func (d IOSDevice) Tap(x, y int) error {
devices/ios.go:146:func (d IOSDevice) Swipe(x1, y1, x2, y2 int) error {
devices/ios.go:150:func (d IOSDevice) Gesture(actions []wda.TapAction) error {
devices/ios.go:405:func (d IOSDevice) PressButton(key string) error {
devices/ios.go:584:func (d IOSDevice) SendKeys(text string) error {
devices/android.go:231:func (d *AndroidDevice) Tap(x, y int) error {
devices/android.go:251:func (d *AndroidDevice) Swipe(x1, y1, x2, y2 int) error {
devices/android.go:261:func (d *AndroidDevice) Gesture(actions []wda.TapAction) error {
devices/android.go:501:func (d *AndroidDevice) PressButton(key string) error {
devices/android.go:531:func (d *AndroidDevice) SendKeys(text string) error {
=== Search for device receiver patterns ===
</shell_result>
Script executed:
#!/bin/bash
# Find IOSDevice and SimulatorDevice struct definitions
echo "=== IOSDevice struct ==="
rg -nP --type=go 'type IOSDevice struct' -A10
echo "=== SimulatorDevice struct ==="
rg -nP --type=go 'type SimulatorDevice struct' -A10Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
=== IOSDevice struct ===
devices/ios.go:29:type IOSDevice struct {
devices/ios.go-30- Udid string `json:"UniqueDeviceID"`
devices/ios.go-31- DeviceName string `json:"DeviceName"`
devices/ios.go-32- OSVersion string `json:"Version"`
devices/ios.go-33-
devices/ios.go-34- tunnelManager *ios.TunnelManager
devices/ios.go-35- wdaClient *wda.WdaClient
devices/ios.go-36- mjpegClient *mjpeg.WdaMjpegClient
devices/ios.go-37-}
devices/ios.go-38-
devices/ios.go-39-func (d IOSDevice) ID() string {
=== SimulatorDevice struct ===
devices/simulator.go:43:type SimulatorDevice struct {
devices/simulator.go-44- Simulator
devices/simulator.go-45- wdaClient *wda.WdaClient
devices/simulator.go-46-}
devices/simulator.go-47-
devices/simulator.go-48-// parseSimulatorVersion parses iOS version from simulator runtime string
devices/simulator.go-49-// e.g., "com.apple.CoreSimulator.SimRuntime.iOS-18-6" -> "18.6"
devices/simulator.go-50-func parseSimulatorVersion(runtime string) string {
devices/simulator.go-51- // Use regex to extract iOS version from runtime string
devices/simulator.go-52- re := regexp.MustCompile(`iOS-(\d+)-(\d+)`)
devices/simulator.go-53- matches := re.FindStringSubmatch(runtime)
</shell_result>
Script executed:
#!/bin/bash
# Search for concurrent calls to device methods
echo "=== Search for handler or operation patterns ==="
rg -nP --type=go 'http.Handle|Handler|goroutine|concurrent' -B1 -A1 | head -40
echo "=== Search for device method calls in goroutines ==="
rg -nP --type=go '\.Tap\(|\.Swipe\(|\.SendKeys\(|\.GetStatus\(' -B5 | grep -E '(go |device|d\.)' | head -30Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
=== Search for handler or operation patterns ===
utils/download_test.go-33- // Create test server that returns 404
utils/download_test.go:34: server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
utils/download_test.go-35- w.WriteHeader(http.StatusNotFound)
--
server/server.go-76-// corsMiddleware handles CORS preflight requests and adds CORS headers to responses.
server/server.go:77:func corsMiddleware(next http.Handler) http.Handler {
server/server.go:78: return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
server/server.go-79- w.Header().Set("Access-Control-Allow-Origin", "*")
--
server/server.go-108-
server/server.go:109: var handler http.Handler = mux
server/server.go-110- if enableCORS {
--
server/server.go-115- Addr: addr,
server/server.go:116: Handler: handler,
server/server.go-117- ReadTimeout: ReadTimeout,
=== Search for device method calls in goroutines ===
devices/ios.go-134-func (d IOSDevice) Shutdown() error {
devices/ios.go-135- return fmt.Errorf("shutdown is not supported for real iOS devices")
devices/ios.go-136-}
devices/ios.go-137-
devices/ios.go-138-func (d IOSDevice) Tap(x, y int) error {
devices/ios.go:139: return d.wdaClient.Tap(x, y)
devices/ios.go-142-func (d IOSDevice) LongPress(x, y int) error {
devices/ios.go-143- return d.wdaClient.LongPress(x, y)
devices/ios.go-144-}
devices/ios.go-145-
devices/ios.go-146-func (d IOSDevice) Swipe(x1, y1, x2, y2 int) error {
devices/ios.go:147: return d.wdaClient.Swipe(x1, y1, x2, y2)
devices/ios.go-254- // 4. we need to set up a forward proxy to port 8100 on the device ✅
devices/ios.go-255- // 5. we need to set up a forward proxy to port 9100 on the device for MJPEG screencapture
devices/ios.go-256- // 6. we need to wait for the agent to be ready ✅
devices/ios.go-257- // 7. just in case, click HOME button ✅
devices/ios.go-258-
devices/ios.go:259: _, err := d.wdaClient.GetStatus()
devices/ios.go-306- }
devices/ios.go-307-
devices/ios.go-308- d.wdaClient = wda.NewWdaClient(fmt.Sprintf("http://localhost:%d", port))
devices/ios.go-309-
devices/ios.go-310- // check if wda is already running, now that we have a port forwarder set up
devices/ios.go:311: _, err = d.wdaClient.GetStatus()
devices/ios.go-580-
devices/ios.go-581- return fmt.Errorf("process of %s not found", bundleID)
devices/ios.go-582-}
devices/ios.go-583-
devices/ios.go-584-func (d IOSDevice) SendKeys(text string) error {
devices/ios.go:585: return d.wdaClient.SendKeys(text)
</shell_result>
Script executed:
#!/bin/bash
# Find the server handlers and how they use devices
echo "=== Server setup and handlers ==="
cat -n server/server.go | head -150Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
=== Server setup and handlers ===
1 package server
2
3 import (
4 "encoding/json"
5 "fmt"
6 "log"
7 "net/http"
8 "strconv"
9 "strings"
10 "time"
11
12 "github.com/mobile-next/mobilecli/commands"
13 "github.com/mobile-next/mobilecli/devices"
14 "github.com/mobile-next/mobilecli/utils"
15 )
16
17 const (
18 // Parse error: Invalid JSON was received by the server
19 ErrCodeParseError = -32700
20
21 // Invalid Request: The JSON sent is not a valid Request object
22 ErrCodeInvalidRequest = -32600
23
24 // Method not found: The method does not exist / is not available
25 ErrCodeMethodNotFound = -32601
26
27 // Server error: Internal JSON-RPC error
28 ErrCodeServerError = -32000
29
30 // Invalid params: Invalid method parameters
31 ErrCodeInvalidParams = -32602
32
33 // Internal error: Internal JSON-RPC error
34 ErrCodeInternalError = -32603
35 )
36
37 // Server timeouts
38 const (
39 ReadTimeout = 10 * time.Second
40 WriteTimeout = 10 * time.Second
41 IdleTimeout = 120 * time.Second
42 )
43
44 var okResponse = map[string]interface{}{"status": "ok"}
45
46 type JSONRPCRequest struct {
47 // these fields are all omitempty, so we can report back to client if they are missing
48 JSONRPC string `json:"jsonrpc,omitempty"`
49 Method string `json:"method,omitempty"`
50 Params json.RawMessage `json:"params,omitempty"`
51 ID interface{} `json:"id,omitempty"`
52 }
53
54 // JSONRPCResponse represents a JSON-RPC response
55 type JSONRPCResponse struct {
56 JSONRPC string `json:"jsonrpc"`
57 Result interface{} `json:"result,omitempty"`
58 Error interface{} `json:"error,omitempty"`
59 ID interface{} `json:"id"`
60 }
61
62 // ScreenshotParams represents the parameters for the screenshot request
63 type ScreenshotParams struct {
64 DeviceID string `json:"deviceId"`
65 Format string `json:"format,omitempty"` // "png" or "jpeg"
66 Quality int `json:"quality,omitempty"` // 1-100, only used for JPEG
67 }
68
69 // DevicesParams represents the parameters for the devices request
70 type DevicesParams struct {
71 IncludeOffline bool `json:"includeOffline,omitempty"`
72 Platform string `json:"platform,omitempty"`
73 Type string `json:"type,omitempty"`
74 }
75
76 // corsMiddleware handles CORS preflight requests and adds CORS headers to responses.
77 func corsMiddleware(next http.Handler) http.Handler {
78 return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
79 w.Header().Set("Access-Control-Allow-Origin", "*")
80 w.Header().Set("Access-Control-Allow-Methods", "POST, GET, OPTIONS")
81 w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization")
82
83 if r.Method == http.MethodOptions {
84 w.WriteHeader(http.StatusOK)
85 return
86 }
87
88 next.ServeHTTP(w, r)
89 })
90 }
91
92 func StartServer(addr string, enableCORS bool) error {
93 mux := http.NewServeMux()
94
95 mux.HandleFunc("/", sendBanner)
96 mux.HandleFunc("/rpc", handleJSONRPC)
97
98 // if host is missing, default to localhost
99 if !strings.Contains(addr, ":") {
100 // convert addr to integer
101 port, err := strconv.Atoi(addr)
102 if err != nil {
103 return fmt.Errorf("invalid port: %v", err)
104 }
105
106 addr = fmt.Sprintf(":%d", port)
107 }
108
109 var handler http.Handler = mux
110 if enableCORS {
111 handler = corsMiddleware(mux)
112 }
113
114 server := &http.Server{
115 Addr: addr,
116 Handler: handler,
117 ReadTimeout: ReadTimeout,
118 WriteTimeout: WriteTimeout,
119 IdleTimeout: IdleTimeout,
120 }
121
122 utils.Info("Starting server on http://%s...", server.Addr)
123 return server.ListenAndServe()
124 }
125
126 func handleJSONRPC(w http.ResponseWriter, r *http.Request) {
127 if r.Method != http.MethodPost {
128 http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
129 return
130 }
131
132 var req JSONRPCRequest
133 if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
134 sendJSONRPCError(w, nil, ErrCodeParseError, "Parse error", "expecting jsonrpc payload")
135 return
136 }
137
138 if req.JSONRPC != "2.0" {
139 sendJSONRPCError(w, req.ID, ErrCodeInvalidRequest, "Invalid Request", "'jsonrpc' must be '2.0'")
140 return
141 }
142
143 if req.ID == nil {
144 sendJSONRPCError(w, nil, ErrCodeInvalidRequest, "Invalid Request", "'id' field is required")
145 return
146 }
147
148 utils.Info("Request ID: %v, Method: %s, Params: %s", req.ID, req.Method, string(req.Params))
149
150 var result interface{}
</shell_result>
Script executed:
#!/bin/bash
# Read the rest of handleJSONRPC and find device usage
sed -n '150,250p' server/server.goRepository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
var result interface{}
var err error
switch req.Method {
case "devices":
result, err = handleDevicesList(req.Params)
case "screenshot":
result, err = handleScreenshot(req.Params)
case "screencapture":
err = handleScreenCapture(w, req.Params)
case "io_tap":
result, err = handleIoTap(req.Params)
case "io_longpress":
result, err = handleIoLongPress(req.Params)
case "io_text":
result, err = handleIoText(req.Params)
case "io_button":
result, err = handleIoButton(req.Params)
case "io_swipe":
result, err = handleIoSwipe(req.Params)
case "io_gesture":
result, err = handleIoGesture(req.Params)
case "url":
result, err = handleURL(req.Params)
case "device_info":
result, err = handleDeviceInfo(req.Params)
case "io_orientation_get":
result, err = handleIoOrientationGet(req.Params)
case "io_orientation_set":
result, err = handleIoOrientationSet(req.Params)
case "device_boot":
result, err = handleDeviceBoot(w, req.Params)
case "device_shutdown":
result, err = handleDeviceShutdown(req.Params)
case "device_reboot":
result, err = handleDeviceReboot(req.Params)
case "":
err = fmt.Errorf("'method' is required")
default:
sendJSONRPCError(w, req.ID, ErrCodeMethodNotFound, "Method not found", fmt.Sprintf("Method '%s' not found", req.Method))
return
}
if err != nil {
log.Printf("Error decoding JSON-RPC request: %v", err)
sendJSONRPCError(w, req.ID, ErrCodeServerError, "Server error", err.Error())
return
}
sendJSONRPCResponse(w, req.ID, result)
}
func sendJSONRPCResponse(w http.ResponseWriter, id interface{}, result interface{}) {
response := JSONRPCResponse{
JSONRPC: "2.0",
Result: result,
ID: id,
}
w.Header().Set("Content-Type", "application/json")
_ = json.NewEncoder(w).Encode(response)
}
func handleDevicesList(params json.RawMessage) (interface{}, error) {
// default to showing all devices if no params provided
opts := devices.DeviceListOptions{
IncludeOffline: false,
Platform: "",
DeviceType: "",
}
// parse params if provided
if len(params) > 0 {
var devicesParams DevicesParams
if err := json.Unmarshal(params, &devicesParams); err != nil {
return nil, fmt.Errorf("invalid parameters: %w", err)
}
opts.IncludeOffline = devicesParams.IncludeOffline
opts.Platform = devicesParams.Platform
opts.DeviceType = devicesParams.Type
}
response := commands.DevicesCommand(opts)
if response.Status == "error" {
return nil, fmt.Errorf("%s", response.Error)
}
return response.Data, nil
}
func handleScreenshot(params json.RawMessage) (interface{}, error) {
var screenshotParams ScreenshotParams
if err := json.Unmarshal(params, &screenshotParams); err != nil {
return nil, fmt.Errorf("invalid parameters: %v", err)
}
req := commands.ScreenshotRequest{
DeviceID: screenshotParams.DeviceID,
Format: screenshotParams.Format,
Quality: screenshotParams.Quality,
</shell_result>
Script executed:
#!/bin/bash
# Search for handleIoTap and similar handlers
echo "=== Handler implementations ==="
rg -nP --type=go 'func handleIo(Tap|Swipe|Text|Button)' -A15Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
=== Handler implementations ===
server/server.go:290:func handleIoTap(params json.RawMessage) (interface{}, error) {
server/server.go-291- if len(params) == 0 {
server/server.go-292- return nil, fmt.Errorf("'params' is required with fields: deviceId, x, y")
server/server.go-293- }
server/server.go-294-
server/server.go-295- var ioTapParams IoTapParams
server/server.go-296- if err := json.Unmarshal(params, &ioTapParams); err != nil {
server/server.go-297- return nil, fmt.Errorf("invalid parameters: %v. Expected fields: deviceId, x, y", err)
server/server.go-298- }
server/server.go-299-
server/server.go-300- req := commands.TapRequest{
server/server.go-301- DeviceID: ioTapParams.DeviceID,
server/server.go-302- X: ioTapParams.X,
server/server.go-303- Y: ioTapParams.Y,
server/server.go-304- }
server/server.go-305-
--
server/server.go:338:func handleIoSwipe(params json.RawMessage) (interface{}, error) {
server/server.go-339- if len(params) == 0 {
server/server.go-340- return nil, fmt.Errorf("'params' is required with fields: deviceId, x1, y1, x2, y2")
server/server.go-341- }
server/server.go-342-
server/server.go-343- var ioSwipeParams IoSwipeParams
server/server.go-344- if err := json.Unmarshal(params, &ioSwipeParams); err != nil {
server/server.go-345- return nil, fmt.Errorf("invalid parameters: %v. Expected fields: deviceId, x1, y1, x2, y2", err)
server/server.go-346- }
server/server.go-347-
server/server.go-348- if ioSwipeParams.DeviceID == "" {
server/server.go-349- return nil, fmt.Errorf("'deviceId' is required")
server/server.go-350- }
server/server.go-351-
server/server.go-352- // validate that coordinates are provided (x1,y1,x2,y2 must be present)
server/server.go-353- var rawParams map[string]interface{}
--
server/server.go:386:func handleIoText(params json.RawMessage) (interface{}, error) {
server/server.go-387- if len(params) == 0 {
server/server.go-388- return nil, fmt.Errorf("'params' is required with fields: deviceId, text")
server/server.go-389- }
server/server.go-390-
server/server.go-391- var ioTextParams IoTextParams
server/server.go-392- if err := json.Unmarshal(params, &ioTextParams); err != nil {
server/server.go-393- return nil, fmt.Errorf("invalid parameters: %v. Expected fields: deviceId, text", err)
server/server.go-394- }
server/server.go-395-
server/server.go-396- req := commands.TextRequest{
server/server.go-397- DeviceID: ioTextParams.DeviceID,
server/server.go-398- Text: ioTextParams.Text,
server/server.go-399- }
server/server.go-400-
server/server.go-401- response := commands.TextCommand(req)
--
server/server.go:449:func handleIoButton(params json.RawMessage) (interface{}, error) {
server/server.go-450- if len(params) == 0 {
server/server.go-451- return nil, fmt.Errorf("'params' is required with fields: deviceId, button")
server/server.go-452- }
server/server.go-453-
server/server.go-454- var ioButtonParams IoButtonParams
server/server.go-455- if err := json.Unmarshal(params, &ioButtonParams); err != nil {
server/server.go-456- return nil, fmt.Errorf("invalid parameters: %v. Expected fields: deviceId, button", err)
server/server.go-457- }
server/server.go-458-
server/server.go-459- req := commands.ButtonRequest{
server/server.go-460- DeviceID: ioButtonParams.DeviceID,
server/server.go-461- Button: ioButtonParams.Button,
server/server.go-462- }
server/server.go-463-
server/server.go-464- response := commands.ButtonCommand(req)
</shell_result>
Script executed:
#!/bin/bash
# Search for TapCommand and device retrieval
echo "=== Search for TapCommand and device lookup ==="
rg -nP --type=go 'func TapCommand|GetDevice|LookupDevice' -A10 | head -50Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
=== Search for TapCommand and device lookup ===
commands/devices.go:9: deviceInfoList, err := devices.GetDeviceInfoList(opts)
commands/devices.go-10- if err != nil {
commands/devices.go-11- return NewErrorResponse(err)
commands/devices.go-12- }
commands/devices.go-13-
commands/devices.go-14- return NewSuccessResponse(map[string]interface{}{
commands/devices.go-15- "devices": deviceInfoList,
commands/devices.go-16- })
commands/devices.go-17-}
--
devices/ios.go:421: device1, err := goios.GetDeviceWithAddress(udid, address, rsdProvider)
devices/ios.go-422- if err != nil {
devices/ios.go-423- return goios.DeviceEntry{}, fmt.Errorf("error getting device with address: %w", err)
devices/ios.go-424- }
devices/ios.go-425-
devices/ios.go-426- device1.UserspaceTUN = device.UserspaceTUN
devices/ios.go-427- device1.UserspaceTUNHost = device.UserspaceTUNHost
devices/ios.go-428- device1.UserspaceTUNPort = device.UserspaceTUNPort
devices/ios.go-429-
devices/ios.go-430- return device1, nil
devices/ios.go-431-}
--
devices/ios.go:437: device, err := goios.GetDevice(d.Udid)
devices/ios.go-438- if err != nil {
devices/ios.go-439- return goios.DeviceEntry{}, fmt.Errorf("device not found: %s: %w", d.Udid, err)
devices/ios.go-440- }
devices/ios.go-441-
devices/ios.go-442- // Get tunnel info directly from our tunnel manager first
devices/ios.go-443- tunnelMgr := d.tunnelManager.GetTunnelManager()
devices/ios.go-444- tunnelInfo, err := tunnelMgr.FindTunnel(d.Udid)
devices/ios.go-445- if err == nil && tunnelInfo.Udid != "" {
devices/ios.go-446- // We have tunnel info from our tunnel manager
devices/ios.go-447- device.UserspaceTUNPort = tunnelInfo.UserspaceTUNPort
--
commands/input.go:53:func TapCommand(req TapRequest) *CommandResponse {
commands/input.go-54- if req.X < 0 || req.Y < 0 {
commands/input.go-55- return NewErrorResponse(fmt.Errorf("x and y coordinates must be non-negative, got x=%d, y=%d", req.X, req.Y))
commands/input.go-56- }
commands/input.go-57-
commands/input.go-58- targetDevice, err := FindDeviceOrAutoSelect(req.DeviceID)
commands/input.go-59- if err != nil {
commands/input.go-60- return NewErrorResponse(fmt.Errorf("error finding device: %v", err))
commands/input.go-61- }
commands/input.go-62-
commands/input.go-63- err = targetDevice.StartAgent(devices.StartAgentConfig{})
--
devices/ios/port-forwarding.go:45: device, err := goios.GetDevice(pf.udid)
devices/ios/port-forwarding.go-46- if err != nil {
devices/ios/port-forwarding.go-47- return fmt.Errorf("failed to get device %s: %w", pf.udid, err)
devices/ios/port-forwarding.go-48- }
</shell_result>
Script executed:
#!/bin/bash
# Search for FindDeviceOrAutoSelect
rg -nP --type=go 'func FindDeviceOrAutoSelect' -A15Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
commands/commands.go:64:func FindDeviceOrAutoSelect(deviceID string) (devices.ControllableDevice, error) {
commands/commands.go-65- // if deviceID is provided, use existing logic
commands/commands.go-66- if deviceID != "" {
commands/commands.go-67- return FindDevice(deviceID)
commands/commands.go-68- }
commands/commands.go-69-
commands/commands.go-70- // Get all devices for auto-selection
commands/commands.go-71- allDevices, err := devices.GetAllControllableDevices(false)
commands/commands.go-72- if err != nil {
commands/commands.go-73- return nil, fmt.Errorf("error getting devices: %w", err)
commands/commands.go-74- }
commands/commands.go-75-
commands/commands.go-76- // filter to only online devices for auto-selection
commands/commands.go-77- var onlineDevices []devices.ControllableDevice
commands/commands.go-78- for _, d := range allDevices {
commands/commands.go-79- if d.State() == "online" {
</shell_result>
Script executed:
#!/bin/bash
# Search for FindDevice function
rg -nP --type=go 'func FindDevice\(' -A10Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
commands/commands.go:37:func FindDevice(deviceID string) (devices.ControllableDevice, error) {
commands/commands.go-38- if deviceID == "" {
commands/commands.go-39- return nil, fmt.Errorf("device ID is required")
commands/commands.go-40- }
commands/commands.go-41-
commands/commands.go-42- // Check cache first
commands/commands.go-43- if device, exists := deviceCache[deviceID]; exists {
commands/commands.go-44- return device, nil
commands/commands.go-45- }
commands/commands.go-46-
commands/commands.go-47- // Get all devices including offline ones and find the one we want
</shell_result>
Script executed:
#!/bin/bash
# Search for deviceCache declaration
rg -nP --type=go 'deviceCache|var.*Cache.*=' -B2 -A2Repository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 0
stdout:
commands/commands.go-32-
commands/commands.go-33-// DeviceCache provides a simple cache for devices to avoid repeated lookups
commands/commands.go:34:var deviceCache = make(map[string]devices.ControllableDevice)
commands/commands.go-35-
commands/commands.go-36-// FindDevice finds a device by ID, using cache when possible
--
commands/commands.go-41-
commands/commands.go-42- // Check cache first
commands/commands.go:43: if device, exists := deviceCache[deviceID]; exists {
commands/commands.go-44- return device, nil
commands/commands.go-45- }
--
commands/commands.go-53- for _, d := range allDevices {
commands/commands.go-54- if d.ID() == deviceID {
commands/commands.go:55: deviceCache[deviceID] = d
commands/commands.go-56- return d, nil
commands/commands.go-57- }
--
commands/commands.go-89- device := onlineDevices[0]
commands/commands.go-90- // Cache the device for future use
commands/commands.go:91: deviceCache[device.ID()] = device
commands/commands.go-92- return device, nil
commands/commands.go-93- }
</shell_result>
Script executed:
#!/bin/bash
# Check if there's any mutex or synchronization on WdaClient or the cache
rg -nP --type=go 'sync.Mutex|RWMutex|Lock\(\)|Unlock\(\)' devices/wda/types.go commands/commands.goRepository: mobile-next/mobilecli
<shell_result>
Repository: mobile-next/mobilecli
Exit code: 1
</shell_result>
Verify thread-safety of sessionId field access and deviceCache sharing pattern.
The concern is confirmed. WdaClient instances are shared across concurrent goroutines through an unsynchronized deviceCache map in commands/commands.go (line 34). Multiple concurrent HTTP requests return the same cached device instance and call its methods, leading to concurrent access to sessionId in GetOrCreateSession() (devices/wda/requests.go:157-176) without any synchronization. The sessionId field is read at line 161, written at lines 165 and 175 without mutex protection. Additionally, the deviceCache map itself has concurrent read/write access (lines 43, 55, 91) which violates Go's concurrency guarantees.
This creates two data race conditions:
- Unsynchronized map access on
deviceCache - Unsynchronized field access on
WdaClient.sessionId
Both require immediate fix with proper synchronization (sync.RWMutex or sync.Mutex).
🤖 Prompt for AI Agents
In devices/wda/types.go around line 12, the WdaClient.sessionId field is
accessed concurrently and must be protected; add a sync.RWMutex (or sync.Mutex)
to the WdaClient struct and use RLock/RUnlock for reads and Lock/Unlock for
writes in GetOrCreateSession (devices/wda/requests.go lines ~157-176) so all
reads at line 161 and writes at 165/175 are synchronized. Also fix the
unsynchronized deviceCache map in commands/commands.go (around lines 34, 43, 55,
91) by guarding it with a package-level sync.RWMutex (or convert to sync.Map)
and wrap all map reads/writes with the appropriate locks; ensure lock
granularity is minimal (per-cache mutex or per-client mutex+cache mutex) to
avoid deadlocks.
Summary by CodeRabbit
Release Notes
New Features
--scaleflag for configurable screenshot scalingImprovements
✏️ Tip: You can customize this high-level summary in your review settings.