Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cli/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ var (
platform string
deviceType string

// for screenrecord command
screenrecordBitRate string
screenrecordTimeLimit int
screenrecordOutput string

// for fleet allocate command
fleetType string
fleetVersions []string
Expand Down
39 changes: 39 additions & 0 deletions cli/screenrecord.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package cli

import (
"fmt"

"github.com/mobile-next/mobilecli/commands"
"github.com/spf13/cobra"
)

var screenrecordCmd = &cobra.Command{
Use: "screenrecord",
Short: "Record video from a connected device",
Long: `Records video from a specified device and saves it as an MP4 file. Supports iOS (real/simulator) and Android (real/emulator).`,
RunE: func(cmd *cobra.Command, args []string) error {
req := commands.ScreenRecordRequest{
DeviceID: deviceId,
BitRate: screenrecordBitRate,
TimeLimit: screenrecordTimeLimit,
OutputPath: screenrecordOutput,
}

response := commands.ScreenRecordCommand(req)

printJson(response)
if response.Status == "error" {
return fmt.Errorf("%s", response.Error)
}
return nil
},
}

func init() {
rootCmd.AddCommand(screenrecordCmd)

screenrecordCmd.Flags().StringVar(&deviceId, "device", "", "ID of the device to record from")
screenrecordCmd.Flags().StringVar(&screenrecordBitRate, "bit-rate", "8M", "Video bit rate (e.g., 4M, 500K, 8000000)")
screenrecordCmd.Flags().IntVar(&screenrecordTimeLimit, "time-limit", 300, "Maximum recording time in seconds (max 300)")
screenrecordCmd.Flags().StringVarP(&screenrecordOutput, "output", "o", "", "Output file path (default: screenrecord-<deviceID>-<timestamp>.mp4)")
}
123 changes: 123 additions & 0 deletions commands/screenrecord.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
package commands

import (
"fmt"
"path/filepath"
"strconv"
"strings"
"time"

"github.com/mobile-next/mobilecli/devices"
)

// ScreenRecordRequest represents the parameters for recording video
type ScreenRecordRequest struct {
DeviceID string `json:"deviceId"`
BitRate string `json:"bitRate,omitempty"`
TimeLimit int `json:"timeLimit,omitempty"`
OutputPath string `json:"outputPath,omitempty"`
}

// ScreenRecordResponse represents the response for a screenrecord command
type ScreenRecordResponse struct {
FilePath string `json:"filePath"`
}

// parseBitRate parses a bit-rate string like "4M", "500K", or "8000000" into an integer
func parseBitRate(s string) (int, error) {
s = strings.TrimSpace(s)
if s == "" {
return 0, fmt.Errorf("empty bit-rate string")
}

upper := strings.ToUpper(s)

if strings.HasSuffix(upper, "M") {
num, err := strconv.ParseFloat(upper[:len(upper)-1], 64)
if err != nil {
return 0, fmt.Errorf("invalid bit-rate: %w", err)
}
return int(num * 1_000_000), nil
}

if strings.HasSuffix(upper, "K") {
num, err := strconv.ParseFloat(upper[:len(upper)-1], 64)
if err != nil {
return 0, fmt.Errorf("invalid bit-rate: %w", err)
}
return int(num * 1_000), nil
}

num, err := strconv.Atoi(s)
if err != nil {
return 0, fmt.Errorf("invalid bit-rate: %w", err)
}
return num, nil
}
Comment on lines +27 to +56
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

No validation that parsed bit-rate is positive.

parseBitRate successfully parses negative values (e.g., "-5M"-5000000) and zero. These are passed directly to adb screenrecord --bit-rate, which will either reject them with a confusing error or produce unexpected behavior. Consider adding a positivity check after parsing.

Proposed fix
 	num, err := strconv.Atoi(s)
 	if err != nil {
 		return 0, fmt.Errorf("invalid bit-rate: %w", err)
 	}
+	if num <= 0 {
+		return 0, fmt.Errorf("bit-rate must be a positive value, got %d", num)
+	}
 	return num, nil
 }

The same check should be applied after the M and K suffix branches as well, or consolidated after all branches:

+func parseBitRate(s string) (int, error) {
+	// ... existing parsing logic ...
+	// After computing the final int value:
+	if result <= 0 {
+		return 0, fmt.Errorf("bit-rate must be a positive value")
+	}
+	return result, nil
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@commands/screenrecord.go` around lines 27 - 56, parseBitRate currently
accepts zero and negative values (e.g., "-5M"), which are passed to adb; after
parsing the numeric value in parseBitRate (both in the "M" and "K" branches and
the plain-number branch), validate that the resulting bit-rate (num or computed
value) is > 0 and return an error like "bit-rate must be positive" if not; you
can either perform the check in each branch after computing the int value or
consolidate by converting all branches to a single positive-int validation
before returning.


// ScreenRecordCommand records video from the specified device
func ScreenRecordCommand(req ScreenRecordRequest) *CommandResponse {
targetDevice, err := FindDeviceOrAutoSelect(req.DeviceID)
if err != nil {
return NewErrorResponse(fmt.Errorf("error finding device: %w", err))
}

// parse bit-rate (default 8M)
bitRateStr := req.BitRate
if bitRateStr == "" {
bitRateStr = "8M"
}

bitRate, err := parseBitRate(bitRateStr)
if err != nil {
return NewErrorResponse(fmt.Errorf("invalid bit-rate '%s': %w", req.BitRate, err))
}

// validate time-limit (default 300, max 300)
timeLimit := req.TimeLimit
if timeLimit <= 0 {
timeLimit = 300
}
if timeLimit > 300 {
return NewErrorResponse(fmt.Errorf("time-limit must be at most 300 seconds, got %d", timeLimit))
}

// determine output path
outputPath := req.OutputPath
if outputPath == "" {
timestamp := time.Now().Format("20060102150405")
safeDeviceID := strings.ReplaceAll(targetDevice.ID(), ":", "_")
fileName := fmt.Sprintf("screenrecord-%s-%s.mp4", safeDeviceID, timestamp)
outputPath, err = filepath.Abs("./" + fileName)
if err != nil {
return NewErrorResponse(fmt.Errorf("error creating default path: %w", err))
}
} else {
outputPath, err = filepath.Abs(outputPath)
if err != nil {
return NewErrorResponse(fmt.Errorf("invalid output path: %w", err))
}
}

// start agent
err = targetDevice.StartAgent(devices.StartAgentConfig{
Hook: GetShutdownHook(),
})
if err != nil {
return NewErrorResponse(fmt.Errorf("failed to start agent on device %s: %w", targetDevice.ID(), err))
}

// record video
err = targetDevice.RecordVideo(devices.RecordVideoConfig{
BitRate: bitRate,
TimeLimit: timeLimit,
OutputPath: outputPath,
})
if err != nil {
return NewErrorResponse(fmt.Errorf("error recording video: %w", err))
}

return NewSuccessResponse(ScreenRecordResponse{
FilePath: outputPath,
})
}
74 changes: 74 additions & 0 deletions devices/android.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ import (
"fmt"
"os"
"os/exec"
"os/signal"
"path/filepath"
"regexp"
"runtime"
"strconv"
"strings"
"syscall"
"time"

"github.com/mobile-next/mobilecli/devices/wda"
Expand Down Expand Up @@ -1235,3 +1237,75 @@ func (d *AndroidDevice) SetOrientation(orientation string) error {

return nil
}

func (d *AndroidDevice) RecordVideo(config RecordVideoConfig) error {
remotePath := fmt.Sprintf("/sdcard/Download/mobilecli-rec-%d.mp4", time.Now().UnixNano())

// build adb shell screenrecord command
args := []string{
"shell", "screenrecord",
"--bit-rate", strconv.Itoa(config.BitRate),
"--time-limit", strconv.Itoa(config.TimeLimit),
remotePath,
}

if config.OnProgress != nil {
config.OnProgress("Recording video")
}
utils.Verbose("Running: adb %s", strings.Join(append([]string{"-s", d.getAdbIdentifier()}, args...), " "))

// run screenrecord; it blocks until time-limit or the process is killed
cmdArgs := append([]string{"-s", d.getAdbIdentifier()}, args...)
cmd := exec.Command(getAdbPath(), cmdArgs...)

// capture stderr for diagnostics
cmd.Stderr = os.Stderr

// set up signal handling so Ctrl+C stops recording gracefully
sigChan := make(chan os.Signal, 1)
signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM)
defer signal.Stop(sigChan)

if err := cmd.Start(); err != nil {
return fmt.Errorf("failed to start screenrecord: %w", err)
}

// wait for either process exit or signal
done := make(chan error, 1)
go func() {
done <- cmd.Wait()
}()

select {
case <-sigChan:
// user interrupted — send SIGINT to adb so screenrecord finalizes the mp4
utils.Verbose("Interrupt received, stopping recording...")
_ = cmd.Process.Signal(syscall.SIGINT)
<-done // wait for process to finish writing
case err := <-done:
if err != nil {
// screenrecord may exit non-zero on some devices; log but continue
utils.Verbose("screenrecord exited with: %v", err)
}
}
Comment on lines +1264 to +1290
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's look for all signal handling in the codebase
rg -n 'signal\.Notify' --type=go -C3

Repository: mobile-next/mobilecli

Length of output: 1461


🏁 Script executed:

# Check for GetShutdownHook usage
rg -n 'GetShutdownHook\|shutdown' --type=go

Repository: mobile-next/mobilecli

Length of output: 47


🏁 Script executed:

# Look at the RecordVideo function context around the signal handling
fd -e go | xargs grep -l "RecordVideo\|signal.Notify" | head -5

Repository: mobile-next/mobilecli

Length of output: 159


🏁 Script executed:

# Check the specific file context
wc -l devices/android.go

Repository: mobile-next/mobilecli

Length of output: 88


🏁 Script executed:

# Check the ScreenRecordCommand to see if it uses shutdown hooks
rg -n 'ScreenRecordCommand\|GetShutdownHook' --type=go -C5

Repository: mobile-next/mobilecli

Length of output: 47


🏁 Script executed:

# Look at the specific signal handling line in android.go to see exact error handling
sed -n '1280,1290p' devices/android.go

Repository: mobile-next/mobilecli

Length of output: 478


🏁 Script executed:

# Search for Windows-specific handling or comments about Windows
rg -n 'windows\|Windows\|SIGINT' --type=go | grep -i windows

Repository: mobile-next/mobilecli

Length of output: 47


🏁 Script executed:

# Check if there's any build tags or Windows-specific signal handling
rg -n '//\s*\+build\|GOOS' --type=go

Repository: mobile-next/mobilecli

Length of output: 47


🏁 Script executed:

# Look for where RecordVideo is called from
rg -n 'RecordVideo' --type=go

Repository: mobile-next/mobilecli

Length of output: 722


🏁 Script executed:

# Check if there's a shutdown hook pattern in the codebase
rg -n 'shutdown\|Shutdown' --type=go | head -20

Repository: mobile-next/mobilecli

Length of output: 47


🏁 Script executed:

# Look at main.go to see the full shutdown pattern
cat -n main.go | head -60

Repository: mobile-next/mobilecli

Length of output: 1522


🏁 Script executed:

# Check what ShutdownHook does
rg -n 'NewShutdownHook\|type.*ShutdownHook' --type=go -A 15

Repository: mobile-next/mobilecli

Length of output: 47


🏁 Script executed:

# Look at SetShutdownHook in commands package
rg -n 'SetShutdownHook' --type=go -C 5

Repository: mobile-next/mobilecli

Length of output: 2619


🏁 Script executed:

# Check if the hook kills child processes or just cleans up
rg -n 'Shutdown\(\)' --type=go -C 3 | head -40

Repository: mobile-next/mobilecli

Length of output: 1665


🏁 Script executed:

# Look at the full RecordVideo function to understand the complete flow
sed -n '1241,1311p' devices/android.go

Repository: mobile-next/mobilecli

Length of output: 2260


🏁 Script executed:

# Search for how other RecordVideo implementations (iOS, simulator) handle signals
sed -n '1174,1250p' devices/ios.go

Repository: mobile-next/mobilecli

Length of output: 2818


🏁 Script executed:

# Check if there's any cleanup registered in ScreenRecordCommand
cat -n commands/screenrecord.go | head -150

Repository: mobile-next/mobilecli

Length of output: 4187


🏁 Script executed:

# Check if context.Context is used elsewhere in RecordVideo interface or similar functions
rg -n 'context.Context' --type=go | head -20

Repository: mobile-next/mobilecli

Length of output: 257


🏁 Script executed:

# Verify Windows behavior of syscall.SIGINT - check documentation comments
rg -n 'windows\|Windows\|signal' devices/android.go -C 2

Repository: mobile-next/mobilecli

Length of output: 47


Process-wide signal interception creates a race condition that can prevent file download, and graceful stop is broken on Windows.

Two critical issues with the signal handling:

  1. Race condition with process shutdown: signal.Notify in RecordVideo registers for SIGINT/SIGTERM while main.go already has a process-wide handler registered for the same signals. In Go, both handlers receive the signal simultaneously. When Ctrl+C is pressed, main's signal handler (line 41 of main.go) calls hook.Shutdown() and os.Exit(0) immediately, terminating the process before RecordVideo can complete the file pull (lines 1292–1296). This leaves the recording on the device or partially downloaded.

  2. Windows incompatibility: cmd.Process.Signal(syscall.SIGINT) is not supported on Windows and silently fails (the error is discarded). Users on Windows cannot gracefully stop recording early and must wait for the full --time-limit duration, or kill the process (which may corrupt the MP4).

Fix: Remove the local signal handler in RecordVideo and instead pass a context.Context (tied to the main shutdown hook) to allow cancellation. Or, coordinate with main's handler to prevent process exit while the goroutine is still pulling the file. For Windows, fallback to cmd.Process.Kill() if Signal() fails.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@devices/android.go` around lines 1264 - 1290, RecordVideo's local signal
handling (sigChan + signal.Notify) creates a race with the global shutdown and
is Windows-unsafe; remove the local signal registration and instead accept a
context.Context (or use the existing shutdown context) in RecordVideo so
cancellation is driven by main's hook, then when cancelling wait for cmd.Wait()
to finish before pulling the file; also replace the unhandled call to
cmd.Process.Signal(syscall.SIGINT) in the interrupt path with a platform-safe
attempt that checks the error and falls back to cmd.Process.Kill() on failure
(or when runtime.GOOS == "windows") so Windows users can stop recording cleanly.


if config.OnProgress != nil {
config.OnProgress("Pulling recorded video")
}

// pull the file from device
pullOutput, err := d.runAdbCommand("pull", remotePath, config.OutputPath)
if err != nil {
return fmt.Errorf("failed to pull recorded file: %w\n%s", err, string(pullOutput))
}

utils.Verbose("Pulled recording to %s", config.OutputPath)

// remove temp file from device
rmOutput, err := d.runAdbCommand("shell", "rm", remotePath)
if err != nil {
utils.Verbose("Warning: failed to remove temp file on device: %v\n%s", err, string(rmOutput))
}

return nil
}
9 changes: 9 additions & 0 deletions devices/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ const (
DefaultFramerate = 30
)

// RecordVideoConfig contains configuration for video recording
type RecordVideoConfig struct {
BitRate int
TimeLimit int
OutputPath string
OnProgress func(message string)
}

// ScreenCaptureConfig contains configuration for screen capture operations
type ScreenCaptureConfig struct {
Format string
Expand Down Expand Up @@ -67,6 +75,7 @@ type ControllableDevice interface {
UninstallApp(packageName string) (*InstalledAppInfo, error)
Info() (*FullDeviceInfo, error)
StartScreenCapture(config ScreenCaptureConfig) error
RecordVideo(config RecordVideoConfig) error
DumpSource() ([]ScreenElement, error)
DumpSourceRaw() (any, error)
GetOrientation() (string, error)
Expand Down
4 changes: 4 additions & 0 deletions devices/ios.go
Original file line number Diff line number Diff line change
Expand Up @@ -1171,6 +1171,10 @@ func (d IOSDevice) SetOrientation(orientation string) error {
return d.wdaClient.SetOrientation(orientation)
}

func (d IOSDevice) RecordVideo(config RecordVideoConfig) error {
return fmt.Errorf("not yet implemented")
}

// DeviceKitInfo contains information about the started DeviceKit session
type DeviceKitInfo struct {
HTTPPort int `json:"httpPort"`
Expand Down
4 changes: 4 additions & 0 deletions devices/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,3 +301,7 @@ func (r *RemoteDevice) UninstallApp(packageName string) (*InstalledAppInfo, erro
func (r *RemoteDevice) StartScreenCapture(config ScreenCaptureConfig) error {
return fmt.Errorf("screen capture is not supported on remote devices")
}

func (r *RemoteDevice) RecordVideo(config RecordVideoConfig) error {
return fmt.Errorf("not yet implemented")
}
4 changes: 4 additions & 0 deletions devices/simulator.go
Original file line number Diff line number Diff line change
Expand Up @@ -959,3 +959,7 @@ func (s SimulatorDevice) GetOrientation() (string, error) {
func (s SimulatorDevice) SetOrientation(orientation string) error {
return s.wdaClient.SetOrientation(orientation)
}

func (s SimulatorDevice) RecordVideo(config RecordVideoConfig) error {
return fmt.Errorf("not yet implemented")
}
Loading