feat: file system support for ls, mkdir, rm, push, pull and get app path

[gmegidish]

No description provided.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds a Filesystem capability and an apps path command. CLI: new top-level fs command with subcommands push, pull, ls, mkdir, rm, plus apps path [bundle_id]. Server: registers device.fs.* and device.apps.path JSON‑RPC methods and implements base64 file transfer with a 1 MB limit. Commands: new Fs* and AppPath commands. Devices: Android implements full FS ops and container path resolution; Simulator implements full FS ops; iOS implements container path and listing (push/pull/mkdir/rm stubbed); Remote device methods stubbed. Tests and README updated for filesystem usage.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.52% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	No description was provided by the author, making it impossible to assess relevance to the changeset.	Add a pull request description explaining the purpose and scope of the filesystem feature implementation, even if brief.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: adding filesystem support with specific operations (ls, mkdir, rm, push, pull) and app path retrieval functionality.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat-working-with-filesystem

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (1)

cli/root.go (1)

124-142: ⚡ Quick win

Add an apps path example in root help for discoverability.

Since app-container workflows are shown here, adding one mobilecli apps path <bundle-id> --device <device-id> example would make the new path capability easier to discover from --help.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cli/root.go` around lines 124 - 142, Add a short example for the new apps
path subcommand to the CLI root help text: locate the help/examples string that
contains the FILESYSTEM block in root.go (the same block showing fs
ls/pull/push/mkdir/rm) and insert a line demonstrating usage of the apps path
command, e.g. "mobilecli apps path <bundle-id> --device <device-id>", so the
apps path capability is discoverable from --help; update the help string near
the FILESYSTEM examples (the root command help definition) to include this new
example.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cli/apps.go`:
- Around line 181-184: Remove the stray blank line between the
appsCmd.AddCommand(appsPathCmd) call and the
appsLaunchCmd.Flags().StringVar(...) call to satisfy gofmt; open the function
containing appsCmd, locate the block where appsCmd.AddCommand(appsPathCmd) and
appsLaunchCmd.Flags().StringVar(&deviceId, "device", "", "ID of the device to
launch app on") appear and delete the extra empty line so the two statements are
adjacent.

In `@devices/ios_fs.go`:
- Around line 108-158: The ListFiles method in IOSDevice never sets
FileEntry.ModTime, causing inconsistency with Android/Simulator; update
IOSDevice.ListFiles to populate ModTime from the AFC file info returned by
client.Stat (and the single-file stat path) — when you call
client.Stat(fullPath) or client.Stat(remotePath) extract the modification time
(e.g., info.ModTime or the appropriate timestamp field on the returned info) and
assign it to FileEntry.ModTime for both the single-file branch and the directory
listing loop so ModTime is consistently populated.

In `@README.md`:
- Line 279: Update the example heading text that currently reads "Example output
for `apps fs ls`" to the correct command label "Example output for `fs ls`" so
the README example matches the actual command; locate and edit the heading
string in the README where the example output for the file-system listing is
shown and replace `apps fs ls` with `fs ls`.
- Around line 227-305: Update the wording in the "Access files on the device..."
section to remove the contradiction by changing the phrase "Pulling binary files
(images, databases, DEX files) is fully supported and binary-safe on all
platforms." to either "…on all supported platforms." or explicitly "…on Android
and iOS Simulator." Locate and edit the notes block (the paragraph starting with
"Pulling binary files...") in the README's file-system section so the platform
claim matches the earlier sentence that support is currently limited to Android
and iOS Simulator.

---

Nitpick comments:
In `@cli/root.go`:
- Around line 124-142: Add a short example for the new apps path subcommand to
the CLI root help text: locate the help/examples string that contains the
FILESYSTEM block in root.go (the same block showing fs ls/pull/push/mkdir/rm)
and insert a line demonstrating usage of the apps path command, e.g. "mobilecli
apps path <bundle-id> --device <device-id>", so the apps path capability is
discoverable from --help; update the help string near the FILESYSTEM examples
(the root command help definition) to include this new example.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 18213652-dcd5-4af8-8273-195639792576

📥 Commits

Reviewing files that changed from the base of the PR and between 732c64c and e178dc4.

📒 Files selected for processing (12)

• README.md
• cli/apps.go
• cli/fs.go
• cli/root.go
• commands/apps.go
• commands/fs.go
• devices/android.go
• devices/android_fs.go
• devices/common.go
• devices/ios_fs.go
• devices/remote_fs.go
• devices/simulator_fs.go

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@server/server.go`:
- Around line 1589-1595: The code currently reads the entire file with
os.ReadFile(tmpPath) before checking fsSizeLimit which can spike memory; change
the flow in the function handling the pulled file so you stat the file first
(use os.Stat on tmpPath), compare fi.Size() to fsSizeLimit and return an error
if too large, and only then read the file into memory; reference tmpPath,
fsSizeLimit and the existing os.ReadFile call so you replace the pre-check with
a size check using os.Stat (or open the file and use io.CopyN/LimitReader)
before loading.
- Around line 1615-1621: The code currently calls
base64.StdEncoding.DecodeString(p.Content) before checking size, so huge base64
input can allocate large memory; instead compute the decoded length using
base64.StdEncoding.DecodedLen(len(p.Content)) and if that decoded length exceeds
fsSizeLimit return the existing "file too large" error before calling
DecodeString; update the block around p.Content, fsSizeLimit and DecodeString to
perform this pre-check and only decode when the decoded size is within limits.

In `@test/emulator.ts`:
- Around line 264-303: mobilecliJson and the related mobilecli call are
constructing shell strings and are vulnerable to injection and breaking on
spaces; replace execSync string-based calls with execFileSync and pass the
mobilecli binary plus each argument as an argv array (e.g., split the command
into explicit args for subcommands like 'apps','path', packageName, '--device',
deviceId or 'fs','ls', remotePath, etc.), preserve options (encoding, timeout,
stdio, env) and keep JSON.parse(result) in mobilecliJson; update fsPush, fsPull,
fsMkdir, fsRm, getAppPath and fsList callers to build and pass proper argv
arrays to mobilecli/ mobilecliJson so all dynamic values (deviceId, remotePath,
packageName, localPath) are passed as arguments rather than interpolated into a
shell string.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 52a136ee-a514-46ae-b554-120c4e8b2167

📥 Commits

Reviewing files that changed from the base of the PR and between e178dc4 and 35fcbbd.

📒 Files selected for processing (5)

• devices/android_fs.go
• devices/ios_fs.go
• server/dispatch.go
• server/server.go
• test/emulator.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• devices/ios_fs.go
• devices/android_fs.go

[coderabbitai]

♻️ Duplicate comments (1)

test/emulator.ts (1)

264-303: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Replace shell-string execution with argv-based execution in CLI helpers.

mobilecliJson and the FS/app helper callers still construct shell strings from dynamic values. This is injection-prone and also brittle for paths containing spaces/quotes.

Suggested fix (execFileSync + argument arrays)

-import { execSync } from 'child_process';
+import { execFileSync, execSync } from 'child_process';

-function mobilecli(args: string): void {
+function mobilecli(args: string[]): void {
 	const mobilecliBinary = path.join(__dirname, '..', 'mobilecli');
-	const command = `${mobilecliBinary} ${args}`;

 	try {
-		const result = execSync(command, {
+		execFileSync(mobilecliBinary, args, {
 			encoding: 'utf8',
 			timeout: 180000,
 			stdio: ['pipe', 'pipe', 'pipe'],
 			env: {
+				...process.env,
 				ANDROID_HOME: process.env.ANDROID_HOME || "",
 			}
 		});
 	} catch (error: any) {
-		console.log(`Command failed: ${command}`);
+		console.log(`Command failed: ${mobilecliBinary} ${args.map((a) => JSON.stringify(a)).join(' ')}`);
 		if (error.stderr) {
 			console.log(`Error stderr: ${error.stderr}`);
 		}
@@
-function mobilecliJson(args: string): any {
+function mobilecliJson(args: string[]): any {
 	const mobilecliBinary = path.join(__dirname, '..', 'mobilecli');
-	const result = execSync(`${mobilecliBinary} ${args}`, {
+	const result = execFileSync(mobilecliBinary, args, {
 		encoding: 'utf8',
 		timeout: 60000,
 		stdio: ['pipe', 'pipe', 'pipe'],
-		env: { ANDROID_HOME: process.env.ANDROID_HOME || '' },
+		env: { ...process.env, ANDROID_HOME: process.env.ANDROID_HOME || '' },
 	});
 	return JSON.parse(result);
 }

 function getAppContainerPath(deviceId: string, packageName: string): string {
-	const response = mobilecliJson(`apps path ${packageName} --device ${deviceId}`);
+	const response = mobilecliJson(['apps', 'path', packageName, '--device', deviceId]);
 	expect(response.status).to.equal('ok');
 	return response.data.path;
 }

 function fsList(deviceId: string, remotePath: string): any[] {
-	const response = mobilecliJson(`fs ls --device ${deviceId} "${remotePath}"`);
+	const response = mobilecliJson(['fs', 'ls', '--device', deviceId, remotePath]);
 	expect(response.status).to.equal('ok');
 	return response.data;
 }

 function fsPush(deviceId: string, localPath: string, remotePath: string): void {
-	mobilecli(`fs push --device ${deviceId} "${localPath}" "${remotePath}"`);
+	mobilecli(['fs', 'push', '--device', deviceId, localPath, remotePath]);
 }

 function fsPull(deviceId: string, remotePath: string, localPath: string): void {
-	mobilecli(`fs pull --device ${deviceId} "${remotePath}" "${localPath}"`);
+	mobilecli(['fs', 'pull', '--device', deviceId, remotePath, localPath]);
 }

 function fsMkdir(deviceId: string, remotePath: string, parents: boolean): void {
-	const flag = parents ? '-p ' : '';
-	mobilecli(`fs mkdir --device ${deviceId} ${flag}"${remotePath}"`);
+	const args = ['fs', 'mkdir', '--device', deviceId];
+	if (parents) args.push('-p');
+	args.push(remotePath);
+	mobilecli(args);
 }

 function fsRm(deviceId: string, remotePath: string, recursive: boolean): void {
-	const flag = recursive ? '-r ' : '';
-	mobilecli(`fs rm --device ${deviceId} ${flag}"${remotePath}"`);
+	const args = ['fs', 'rm', '--device', deviceId];
+	if (recursive) args.push('-r');
+	args.push(remotePath);
+	mobilecli(args);
 }

#!/bin/bash
# Verify remaining string-based shell execution patterns in this file.
rg -nP --type=ts 'execSync\(\s*`|execSync\(\s*["'\''].*\$\{.*["'\'']\s*\)' test/emulator.ts
rg -nP --type=ts 'mobilecli(Json)?\(\s*`' test/emulator.ts
# Expected after fix: no matches for dynamic/interpolated shell commands.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/emulator.ts` around lines 264 - 303, mobilecliJson and the helper
callers (mobilecli, getAppContainerPath, fsList, fsPush, fsPull, fsMkdir, fsRm)
currently build shell-interpolated command strings which is injection-prone;
change mobilecliJson to use execFileSync (or spawnSync) with an argv array (no
shell=true) and update all callers to pass arguments as separate array entries
(e.g., ['apps','path', packageName, '--device', deviceId] or
['fs','push','--device', deviceId, localPath, remotePath]) instead of string
interpolation, ensuring paths with spaces/quotes are passed as single argv
elements and retaining existing timeout/encoding/env options.

🧹 Nitpick comments (1)

test/emulator.ts (1)

105-111: ⚡ Quick win

Ensure temp files are always cleaned up on failing test paths.

Cleanup currently runs only on success. If fsPush, fsPull, or assertions fail, temp files remain and can accumulate across CI retries.

Suggested fix (use try/finally)

 const localFile = writeTempFile('hello from mobilecli');
-fsPush(deviceId, localFile, remoteFile);
-fs.unlinkSync(localFile);
+try {
+	fsPush(deviceId, localFile, remoteFile);
+} finally {
+	if (fs.existsSync(localFile)) fs.unlinkSync(localFile);
+}

 const localDest = path.join(os.tmpdir(), `mobilecli-pull-${Date.now()}.txt`);
-fsPull(deviceId, remoteFile, localDest);
-const contents = fs.readFileSync(localDest, 'utf8');
-expect(contents.trim()).to.equal('hello from mobilecli');
-fs.unlinkSync(localDest);
+try {
+	fsPull(deviceId, remoteFile, localDest);
+	const contents = fs.readFileSync(localDest, 'utf8');
+	expect(contents.trim()).to.equal('hello from mobilecli');
+} finally {
+	if (fs.existsSync(localDest)) fs.unlinkSync(localDest);
+}

Also applies to: 121-129, 172-178, 188-196

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/emulator.ts` around lines 105 - 111, The temp files created by
writeTempFile (localFile) must be removed even if fsPush/fsPull or assertions
throw; wrap the test body that uses localFile in a try/finally so
fs.unlinkSync(localFile) runs in finally (guarded by fs.existsSync(localFile) if
needed) and move fs.unlinkSync out of the success path; apply the same
try/finally pattern to the other tests that create localFile and call
fsPush/fsPull (the blocks around fsPush/fsPull and the tests referenced at
121-129, 172-178, 188-196) so temp files are always cleaned up on failure.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@test/emulator.ts`:
- Around line 264-303: mobilecliJson and the helper callers (mobilecli,
getAppContainerPath, fsList, fsPush, fsPull, fsMkdir, fsRm) currently build
shell-interpolated command strings which is injection-prone; change
mobilecliJson to use execFileSync (or spawnSync) with an argv array (no
shell=true) and update all callers to pass arguments as separate array entries
(e.g., ['apps','path', packageName, '--device', deviceId] or
['fs','push','--device', deviceId, localPath, remotePath]) instead of string
interpolation, ensuring paths with spaces/quotes are passed as single argv
elements and retaining existing timeout/encoding/env options.

---

Nitpick comments:
In `@test/emulator.ts`:
- Around line 105-111: The temp files created by writeTempFile (localFile) must
be removed even if fsPush/fsPull or assertions throw; wrap the test body that
uses localFile in a try/finally so fs.unlinkSync(localFile) runs in finally
(guarded by fs.existsSync(localFile) if needed) and move fs.unlinkSync out of
the success path; apply the same try/finally pattern to the other tests that
create localFile and call fsPush/fsPull (the blocks around fsPush/fsPull and the
tests referenced at 121-129, 172-178, 188-196) so temp files are always cleaned
up on failure.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6ca1cf66-30b7-42e8-bc7a-8c3e75af3dba

📥 Commits

Reviewing files that changed from the base of the PR and between 35fcbbd and 6f4f4a5.

📒 Files selected for processing (5)

• README.md
• cli/fs.go
• commands/apps.go
• devices/android_fs.go
• test/emulator.ts

✅ Files skipped from review due to trivial changes (1)

• README.md

🚧 Files skipped from review as they are similar to previous changes (3)

• commands/apps.go
• cli/fs.go
• devices/android_fs.go