feat: webview support for ios simulators and android real/emulators

[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

This PR implements WebView inspection: CI builds native Android and iOS agents and embeds them as Go assets; Android uses a JVMTI agent plus Java RPC server/bridge, iOS provides a dylib agent with a MobileServer and bridge. Device drivers (ADB and simulator/LLDB flows) install/attach agents and perform JSON-RPC calls to list, navigate, evaluate JS, fetch content, and wait for load states. CLI commands, command-layer types, server handlers, Makefile/CI wiring, and tests are included.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.45% 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 pull request description was provided by the author, making it impossible to assess relevance to the changeset.	Add a description explaining the webview feature implementation, agent setup, and supported operations for reviewers.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main feature: adding webview support for both iOS simulators and Android real/emulator devices.
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/webview-v2

---

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

[coderabbitai]

Actionable comments posted: 18

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

agents/agents.go (1)

14-14: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove trailing formatting drift causing CI failure.

Line 14 introduces a gofmt mismatch; CI is already failing on formatting. Run gofmt (or remove the extra trailing blank line) before merge.

🤖 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 `@agents/agents.go` at line 14, Remove the trailing formatting drift in
agents.go by running gofmt (or manually deleting the extra blank line at the end
of the file around line 14) so the file matches gofmt formatting and CI passes;
verify with gofmt -w agents.go or gofmt -l to confirm no differences remain.

🧹 Nitpick comments (6)

agents/android/jvmti_agent.c (1)

31-36: 💤 Low value

Local reference parent is never deleted.

j_dex and j_opt are properly cleaned up, but the parent jobject from CallStaticObjectMethod leaks. While JNI local references are auto-freed on thread detach, consistently managing them prevents issues if the agent is loaded multiple times or the function logic changes.

♻️ Proposed fix

     (*env)->DeleteLocalRef(env, j_dex);
     (*env)->DeleteLocalRef(env, j_opt);
+    (*env)->DeleteLocalRef(env, parent);

🤖 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 `@agents/android/jvmti_agent.c` around lines 31 - 36, The local jobject parent
returned by CallStaticObjectMethod (stored in variable parent) is not released;
after you finish using parent (after NewObject creating loader and before
returning/exiting the function), call (*env)->DeleteLocalRef(env, parent) to
free the local JNI reference (mirror how j_dex and j_opt are deleted); ensure
DeleteLocalRef is invoked even on error paths so parent cannot leak if
CallStaticObjectMethod succeeded but later calls fail.

agents/ios/agent.m (1)

15-26: 💤 Low value

Server object lifetime relies on block capture - consider documenting or making explicit.

The server object is only kept alive by being captured in the dispatch_async block. While this works with ARC, the lifetime management is implicit. If run ever returns or the block completes, the server could be deallocated.

If run is designed to block indefinitely, this is fine. If not, the server could be prematurely deallocated.

🤖 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 `@agents/ios/agent.m` around lines 15 - 26, The MobileServer instance (server)
is only retained by the dispatch_async block capture, which makes its lifetime
implicit; ensure server is kept alive explicitly by storing it in a long-lived
strong variable or property (e.g., a static/global or an owning ivar) or by
documenting that MobileServer::run blocks indefinitely; update the code that
creates MobileServer (and the dispatch_async call) to assign server to a named
strong owner (or add a comment documenting that run never returns) so the
instance is not prematurely deallocated, referencing MobileServer, server, bind,
run, and the dispatch_async block.

agents/ios/Makefile (1)

9-9: ⚡ Quick win

Add .PHONY declarations for non-file targets.

all and clean are not actual files. Without .PHONY, Make could skip them if files with those names exist.

♻️ Proposed fix

+.PHONY: all clean
+
 all: agent-sim.dylib agent-dev.dylib

Also applies to: 30-31

🤖 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 `@agents/ios/Makefile` at line 9, Add a .PHONY declaration for the non-file
targets so make won't treat them as files: declare .PHONY including the targets
"all" and "clean" (and any other non-file targets on lines 30-31) in the
Makefile so that the "all" and "clean" rules are always executed regardless of
existing files named "all" or "clean".

agents/ios/dispatcher.m (1)

94-104: ⚖️ Poor tradeoff

Blocking polling loop in waitForLoadState could tie up the RPC handler thread.

The polling loop blocks with sleepForTimeInterval for up to timeoutMs (default 30 seconds). If the server is single-threaded, this blocks all other RPC requests during the wait.

Consider whether async/callback-based waiting is feasible, or document that this operation is blocking.

🤖 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 `@agents/ios/dispatcher.m` around lines 94 - 104, The blocking polling loop
inside waitForLoadState (which calls [IosBridge evaluateJS:inWebView:] and
returns via rpc_result/rpc_error) ties up the RPC handler thread; move the
polling off the RPC thread by performing the loop asynchronously (e.g.,
dispatch_async to a background/global queue) so the RPC thread can return
immediately and then invoke rpc_result/rpc_error back on the RPC/main queue when
the condition or timeout is reached; also replace the current
sleepForTimeInterval blocking wait with a non-blocking wait pattern (timer,
usleep on background thread, or dispatch_after) and preserve the same
timeout/error logic and messages.

agents/ios/bridge.m (1)

7-18: 💤 Low value

Same timeout race pattern in runOnMainThread.

Similar to evaluateJS, if the 5-second timeout fires, the block dispatched to main queue may still execute after the caller has returned. If the block captures and modifies state the caller depends on, this could cause issues. Callers using this helper should be aware of this behavior.

🤖 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 `@agents/ios/bridge.m` around lines 7 - 18, The helper runOnMainThread uses a
5s semaphore wait but dispatches a normal block that may still run after
timeout; change it to create a cancellable dispatch block (use
dispatch_block_create) and dispatch that to the main queue, then wait with
dispatch_semaphore_wait; if the wait times out, call dispatch_block_cancel on
the created block so the block will not execute after the caller returns (mirror
the safer pattern used for evaluateJS). Ensure the dispatched block signals the
semaphore when it runs, and replace the current dispatch_async usage with
dispatch_block_create + dispatch_async + dispatch_block_cancel on timeout.

agents/ios/devicekit.m (1)

22-28: ⚡ Quick win

Guard/remove the legacy devicekit dylib side effects in agents/ios/devicekit.m

agents/ios/devicekit.m’s __attribute__((constructor)) writes to /tmp/gilm.txt and swizzles NSURLSession (dataTaskWithURL:completionHandler:). While agents/ios/Makefile labels this as a “legacy test dylib” and builds it as devicekit.dylib, mobilecli’s iOS injection path only embeds and dlopens ios/agent-sim.dylib (agents/agents.go + devices/ios_webview.go), not devicekit.dylib. If this legacy dylib isn’t used by shipped flows, remove it or gate these load-time side effects behind a debug/test build flag.

🤖 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 `@agents/ios/devicekit.m` around lines 22 - 28, The constructor function in
devicekit.m (the __attribute__((constructor)) block) performs load-time side
effects — writing to /tmp/gilm.txt and swizzling NSURLSession’s
dataTaskWithURL:completionHandler: (see originalDataTaskWithURL and the wrapped
completion handler) — which must be removed or gated; either delete this legacy
devicekit dylib logic entirely if it’s unused, or wrap the constructor and any
swizzle/writing logic in a debug/test-only preprocessor guard (e.g. `#ifdef` DEBUG
or a TEST_DYLIB flag) so that the write to /tmp and the swizzling only occur in
test builds and are never executed in production/injection builds. Ensure the
guarded symbols include the constructor block, any calls that write to
/tmp/gilm.txt, and the swizzle implementation that replaces
dataTaskWithURL:completionHandler:.

🤖 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 @.github/workflows/build.yml:
- Line 24: The workflow uses actions/checkout@v5 without disabling credential
persistence, exposing tokens across artifact-producing/consuming jobs; update
the checkout steps (actions/checkout@v5) in the artifact-related jobs to include
persist-credentials: false in the with: block, and apply the same change to the
other occurrence referenced (the second actions/checkout@v5 instance) so
credentials are not persisted between jobs.
- Line 24: Replace all tag-pinned checkout steps (e.g., uses:
actions/checkout@v5) with immutable commit SHA pins and add the checkout input
persist-credentials: false; specifically locate each use of actions/checkout@v5
(occurrences at the lines noted in the review) and change to the corresponding
full commit SHA for that release, and add with: persist-credentials: false under
the checkout step to prevent exposing the workflow token.

In `@agents/android/java/AndroidBridge.java`:
- Around line 73-93: The runOnMainThread helper can deadlock when invoked from
the main thread; add a fast-path at the top of runOnMainThread that checks
Looper.myLooper() == Looper.getMainLooper() and, if true, invokes task.call()
directly (propagating any thrown Exception) and returns the result instead of
posting to sMainHandler and awaiting the latch; keep the existing posted-path
behavior for non-main threads and preserve existing timeout/exception semantics
for that path.

In `@agents/android/java/HttpRpcServer.java`:
- Around line 30-50: The request-body handling in HttpRpcServer is incorrect: it
reads bytes via a BufferedReader/InputStreamReader into a char[] (body) with a
single in.read(...) call and ignores the returned count, which corrupts payloads
and mishandles byte-based Content-Length; replace the body-read logic to read
exactly contentLength bytes from the raw InputStream (client.getInputStream())
using a loop that accumulates until all bytes are read, then decode those bytes
into a String with an explicit charset (e.g., UTF-8) to produce bodyStr; ensure
you stop when contentLength==0 and handle partial reads by checking the read
byte count each iteration instead of using the BufferedReader/char[] approach.

In `@agents/android/java/JsonRpcDispatcher.java`:
- Around line 22-27: Change how the JSON-RPC "id" is handled in
JsonRpcDispatcher.dispatch: parse and store id as an Object (e.g., Object id =
req.has("id") ? req.opt("id") : /* no-id marker */) so numeric ids are not
coerced to String, and use req.has("id") (or the same presence marker) when
building responses in result(...) and error(...) to always echo the id field
(including explicit null via JSONObject.NULL or equivalent) when the request
contained an "id" key; update the error(...) and result(...) call sites to
accept and emit this preserved id object instead of relying on id != null.

In `@agents/android/jvmti_agent.c`:
- Around line 136-141: Check the return value of (*vm)->AttachCurrentThread
before using env: if AttachCurrentThread fails, do not call bootstrap(env,
dex_path, opt_dir) with a possibly NULL env—log an error (including the failing
return code and dex_path), detach/cleanup if necessary, and return an
appropriate JNI error (e.g., JNI_ERR) instead of proceeding; update the function
around AttachCurrentThread/bootstrap to validate the jint result and handle
failure paths safely.

In `@agents/ios/bridge.m`:
- Around line 100-119: The evaluateJS block can race if dispatch_semaphore_wait
times out because the evaluateJavaScript completionHandler may later write
jsResult/jsError; add an __block BOOL timedOut (or similar) and set it to NO
before dispatch_async, have the completionHandler check timedOut and return
without touching jsResult/jsError if timedOut is YES, and when
dispatch_semaphore_wait returns due to timeout set timedOut = YES before reading
jsResult/jsError (and avoid double-signaling the semaphore); update references
to jsResult/jsError/sem in the evaluateJavaScript completionHandler and the
timeout branch to use this flag to prevent the race.

In `@agents/ios/devicekit.m`:
- Around line 14-16: The code unsafely casts resp to NSHTTPURLResponse; update
the logic around the variables resp, http, url, and body to first verify resp is
an NSHTTPURLResponse (e.g., using isKindOfClass:) before casting, and handle the
non-HTTP case (set a safe status value or include a descriptive placeholder like
"non-HTTP response" in the out string) so you don't read an invalid statusCode
when building the out NSString from url.absoluteString, http.statusCode, and
body.

In `@agents/ios/dispatcher.m`:
- Around line 22-29: The requireParam function currently builds an rpc_error
with a nil reqId; change the signature of requireParam to accept the caller's
reqId (e.g., add NSString *reqId parameter), and when creating the error call
rpc_error(reqId, -32602, ...) instead of passing nil; then update every call
site of requireParam in this file to pass the request ID variable supplied by
the caller so errors include the original reqId (refer to requireParam and
rpc_error to locate the changes and update all usages).

In `@agents/ios/server.m`:
- Around line 48-59: In bind, when bindPort succeeds but listen(_serverFd, ...)
returns < 0 you must close the opened socket to avoid a FD leak: call
close(_serverFd) (and optionally set _serverFd = -1) before returning NO; update
the error log in the bind method to perform cleanup on listen failure so the
socket created by bindPort is properly released.

In `@devices/adb.go`:
- Around line 255-267: The ctx-waiter started in TrackDevices can leak if the
read loop exits for non-context reasons because it only waits on ctx.Done(); fix
by adding a local done channel (e.g., doneCh := make(chan struct{})) and modify
closeConn (and its closeOnce) to close(doneCh) when invoked, then change the
waiter goroutine to select between ctx.Done() and doneCh so it unblocks when
closeConn runs; reference symbols: TrackDevices, closeOnce, closeConn,
ctx.Done(), doneCh.

In `@devices/android_webview.go`:
- Around line 225-268: The current ensureAgentReady/getWebViewPort flow
re-installs the kit and re-creates port forwards on every call (via
installWebViewKit and forwardWebViewSocket), causing stale forwards and repeated
work; change AndroidDevice to cache per-package agent state (e.g.,
map[string]{port,pid,agentDir,lastChecked}) and update ensureAgentReady to: 1)
check the cache first and verify the cached port is still serving (use
isAgentReady) and the process PID is unchanged (getProcessPID) before skipping
install/forward; 2) only call installWebViewKit and forwardWebViewSocket when
the cache is missing or invalid; and 3) remove/cleanup stale forwards when the
target process dies or when replacing a forward. Use the existing symbols
ensureAgentReady, getWebViewPort, installWebViewKit, forwardWebViewSocket,
getProcessPID and attachJVMTIAgent to implement the cache and cleanup logic.

In `@devices/ios_webview.go`:
- Around line 136-146: The lldb injection can hang because exec.Command("lldb",
...) has no deadline; replace the direct exec.Command usage with a context with
timeout and exec.CommandContext to enforce a deadline (e.g., ctx, cancel :=
context.WithTimeout(context.Background(), timeout); defer cancel()), call
exec.CommandContext(ctx, "lldb", ...), run CombinedOutput on that cmd, and
handle context.DeadlineExceeded by returning a clear timeout error; update
references to cmd and the CombinedOutput error handling in ios_webview.go so the
lldb process is killed when the timeout elapses.
- Around line 52-69: The current ps aux parsing (loop using strings.Split,
strings.Fields and fields[10]) is brittle when the command/executable contains
spaces; update the parsing logic in ios_webview.go to reliably extract PID and
the full command path by replacing the strings.Fields-based split with a regex
or explicit substring extraction: capture the PID and the full command portion
(e.g., use a regexp that matches user, whitespace, pid, other columns and then
captures the rest of the line as the command or match the PID with ^\S+\s+(\d+)
and take the remainder of the line as the command), then append candidates using
the parsed pid and the full command string instead of fields[10]; reference the
existing loop that uses strings.Split(string(out), "\n"), strings.Fields,
fields[10], and the candidates append to locate and change the code.

In `@go.mod`:
- Line 10: The go.mod currently pins github.com/danielpaulus/go-ios to a
pseudo-version that skips v1.0.208–v1.0.211; either confirm that skipping those
fixes is intentional for this change or bump the dependency to include the fixes
(e.g., run go get github.com/danielpaulus/go-ios@v1.0.211, then go mod tidy to
update go.mod and go.sum) and run the iOS-related unit/integration tests and any
TUN/DTX flows to verify no regressions; also update the PR description or
changelog to document the decision (module: github.com/danielpaulus/go-ios,
version spec in go.mod).

In `@Makefile`:
- Around line 5-7: The agents target unconditionally invokes agents/ios making
make build macOS-only; change the Makefile so build and build-cover do not
always call agents/ios — either split agents into separate targets (e.g.,
agents-android and agents-ios) and have agents depend only on android, or guard
the ios invocation with an OS check (e.g., ifeq ($(shell uname),Darwin)) or a
user flag (e.g., BUILD_IOS=1) so targets like build, build-cover, and the agents
target call agents/android by default and only call agents/ios when on macOS or
when BUILD_IOS is set; update references to agents, agents/android, agents/ios,
build and build-cover accordingly.
- Around line 38-39: Update the Makefile's clean target so it also removes iOS
agent artifacts; currently it only runs $(MAKE) -C agents/android clean and then
rm -f mobilecli coverage.out coverage.html — add a step to invoke the iOS agent
clean (e.g. run the make clean in agents/ios or call the top-level agents clean
if that aggregates platforms) so both agents/android and agents/ios are cleaned
when the clean target runs.

In `@server/dispatch.go`:
- Around line 15-57: The map literal in server/dispatch.go has inconsistent
key/value alignment (e.g., entries around
"device.webview.waitForLoadState"/handleWebViewWaitForLoadState vs
"device.apps.path"/handleAppsPath), causing gofmt failures; run gofmt -w
server/dispatch.go (or gofmt on the repo) to normalize spacing and alignment
across all entries (ensure handlers like handleScreenshot,
handleScreenCaptureSession, handleWebViewEvaluate, handleAppsPath, handleFsLs,
etc. align consistently), then commit the reformatted file.

---

Outside diff comments:
In `@agents/agents.go`:
- Line 14: Remove the trailing formatting drift in agents.go by running gofmt
(or manually deleting the extra blank line at the end of the file around line
14) so the file matches gofmt formatting and CI passes; verify with gofmt -w
agents.go or gofmt -l to confirm no differences remain.

---

Nitpick comments:
In `@agents/android/jvmti_agent.c`:
- Around line 31-36: The local jobject parent returned by CallStaticObjectMethod
(stored in variable parent) is not released; after you finish using parent
(after NewObject creating loader and before returning/exiting the function),
call (*env)->DeleteLocalRef(env, parent) to free the local JNI reference (mirror
how j_dex and j_opt are deleted); ensure DeleteLocalRef is invoked even on error
paths so parent cannot leak if CallStaticObjectMethod succeeded but later calls
fail.

In `@agents/ios/agent.m`:
- Around line 15-26: The MobileServer instance (server) is only retained by the
dispatch_async block capture, which makes its lifetime implicit; ensure server
is kept alive explicitly by storing it in a long-lived strong variable or
property (e.g., a static/global or an owning ivar) or by documenting that
MobileServer::run blocks indefinitely; update the code that creates MobileServer
(and the dispatch_async call) to assign server to a named strong owner (or add a
comment documenting that run never returns) so the instance is not prematurely
deallocated, referencing MobileServer, server, bind, run, and the dispatch_async
block.

In `@agents/ios/bridge.m`:
- Around line 7-18: The helper runOnMainThread uses a 5s semaphore wait but
dispatches a normal block that may still run after timeout; change it to create
a cancellable dispatch block (use dispatch_block_create) and dispatch that to
the main queue, then wait with dispatch_semaphore_wait; if the wait times out,
call dispatch_block_cancel on the created block so the block will not execute
after the caller returns (mirror the safer pattern used for evaluateJS). Ensure
the dispatched block signals the semaphore when it runs, and replace the current
dispatch_async usage with dispatch_block_create + dispatch_async +
dispatch_block_cancel on timeout.

In `@agents/ios/devicekit.m`:
- Around line 22-28: The constructor function in devicekit.m (the
__attribute__((constructor)) block) performs load-time side effects — writing to
/tmp/gilm.txt and swizzling NSURLSession’s dataTaskWithURL:completionHandler:
(see originalDataTaskWithURL and the wrapped completion handler) — which must be
removed or gated; either delete this legacy devicekit dylib logic entirely if
it’s unused, or wrap the constructor and any swizzle/writing logic in a
debug/test-only preprocessor guard (e.g. `#ifdef` DEBUG or a TEST_DYLIB flag) so
that the write to /tmp and the swizzling only occur in test builds and are never
executed in production/injection builds. Ensure the guarded symbols include the
constructor block, any calls that write to /tmp/gilm.txt, and the swizzle
implementation that replaces dataTaskWithURL:completionHandler:.

In `@agents/ios/dispatcher.m`:
- Around line 94-104: The blocking polling loop inside waitForLoadState (which
calls [IosBridge evaluateJS:inWebView:] and returns via rpc_result/rpc_error)
ties up the RPC handler thread; move the polling off the RPC thread by
performing the loop asynchronously (e.g., dispatch_async to a background/global
queue) so the RPC thread can return immediately and then invoke
rpc_result/rpc_error back on the RPC/main queue when the condition or timeout is
reached; also replace the current sleepForTimeInterval blocking wait with a
non-blocking wait pattern (timer, usleep on background thread, or
dispatch_after) and preserve the same timeout/error logic and messages.

In `@agents/ios/Makefile`:
- Line 9: Add a .PHONY declaration for the non-file targets so make won't treat
them as files: declare .PHONY including the targets "all" and "clean" (and any
other non-file targets on lines 30-31) in the Makefile so that the "all" and
"clean" rules are always executed regardless of existing files named "all" or
"clean".

🪄 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: caec56f7-6a7c-4b82-962f-91e2c809da0b

📥 Commits

Reviewing files that changed from the base of the PR and between fc64db4 and 8379735.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (40)

• .github/workflows/build.yml
• .gitignore
• Makefile
• README.md
• agents/agents.go
• agents/android/Makefile
• agents/android/java/AndroidBridge.java
• agents/android/java/HttpRpcServer.java
• agents/android/java/JsonRpcDispatcher.java
• agents/android/java/MobileCliAgent.java
• agents/android/java/RpcException.java
• agents/android/java/WebViewAgent.java
• agents/android/jvmti_agent.c
• agents/ios/Makefile
• agents/ios/agent.m
• agents/ios/bridge.h
• agents/ios/bridge.m
• agents/ios/devicekit.m
• agents/ios/dispatcher.h
• agents/ios/dispatcher.m
• agents/ios/server.h
• agents/ios/server.m
• cli/flags.go
• cli/root.go
• cli/webview.go
• commands/apps.go
• commands/boot.go
• commands/commands.go
• commands/input.go
• commands/reboot.go
• commands/url.go
• commands/webview.go
• devices/adb.go
• devices/adb_test.go
• devices/android_webview.go
• devices/common.go
• devices/ios_webview.go
• go.mod
• server/dispatch.go
• server/webview_handlers.go

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Compare github.com/danielpaulus/go-ios v1.0.211 with v1.0.207-0.20260326100139-5d5f0d1129b8. Were there bug fixes or security fixes in versions after 1.0.207 that would be lost by this pin?

💡 Result:

No specific security fixes (or explicit “security”/CVE bug fixes) are clearly indicated in the sources we found for versions after 1.0.207; however, there are clear bugfixes / reliability improvements and functional changes in releases later than 1.0.207. What changed after your pinned version - Your pin: v1.0.207-0.20260326100139-5d5f0d1129b8 (published 2026-03-26) [1]. - Later releases in the same version family include v1.0.208 (published 2026-04-15), v1.0.209 (2026-04-17), v1.0.210 (2026-04-20), v1.0.211 (2026-04-22), and v1.0.211-0.20260422071638-fcb349a334ec (2026-04-22) [1]. Bugfix evidence we can cite - The project’s GitHub releases page shows multiple “fix” and “move” type entries around the timeframe following your pin (e.g., “Fix panic in DTX dispatcher when device sends incomplete outputReceived …” and other maintenance changes) [2]. - One of the later release notes we can see (from the releases page content) describes a tunnel-related improvement: “tunnel: platform-split TUN setup and permission check (#707)” and explicitly mentions that setupTunnelInterface no longer shells out to ifconfig and related capability/permission/error-handling changes [2]. This is the kind of change typically associated with bugfixing/reliability, not a documented security patch. Security-fix evidence we can cite - In the retrieved sources, we do not see any release note that explicitly labels changes as a security fix (e.g., “security fix”, “CVE”, “vulnerability”, etc.) specifically for versions after 1.0.207 [2][1]. Answer to your question (“Were there bug fixes or security fixes in versions after 1.0.207 that would be lost by this pin?”) - Yes, bugfixes/reliability improvements appear to have occurred in versions after 1.0.207 (the releases page includes explicit fix/panic-related items and other maintenance changes) [2]. If you only pin to 1.0.207-0.20260326100139-5d5f0d1129b8, you would miss those improvements. - We cannot confirm from the sources retrieved that any of the later versions contained explicit security fixes (no explicit security advisories/CVEs are shown in the snippets we collected) [2][1]. If you want, I can do a second pass specifically extracting the release notes for every version between 1.0.207-0.20260326100139-5d5f0d1129b8 and v1.0.211 (and 1.0.208/1.0.209/1.0.210) and check for any mention of “security”, “CVE”, “vulnerability”, “auth bypass”, etc.

Citations:

• 1: https://pkg.go.dev/github.com/danielpaulus/go-ios
• 2: https://github.com/danielpaulus/go-ios/releases

---

Confirm the go-ios pseudo-version pin doesn’t drop required fixes (go.mod line 10).

github.com/danielpaulus/go-ios is pinned to v1.0.207-0.20260326100139-5d5f0d1129b8 instead of v1.0.211. The project’s release notes show multiple follow-on bugfix/reliability changes in v1.0.208–v1.0.211 (e.g., a “Fix panic in DTX dispatcher…” and tunnel/TUN setup permission/error-handling changes), so this pin would skip them. Ensure the downgrade is intentional and doesn’t regress the iOS flows needed by this PR (or bump back to v1.0.211/later and document why).

🤖 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 `@go.mod` at line 10, The go.mod currently pins github.com/danielpaulus/go-ios
to a pseudo-version that skips v1.0.208–v1.0.211; either confirm that skipping
those fixes is intentional for this change or bump the dependency to include the
fixes (e.g., run go get github.com/danielpaulus/go-ios@v1.0.211, then go mod
tidy to update go.mod and go.sum) and run the iOS-related unit/integration tests
and any TUN/DTX flows to verify no regressions; also update the PR description
or changelog to document the decision (module: github.com/danielpaulus/go-ios,
version spec in go.mod).

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

agents/android/java/HttpRpcServer.java (1)

32-33: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix HTTP Content-Length mismatch in agents/android/java/HttpRpcServer.java
bytes.length is computed from UTF-8, but the response (and headers) are written through PrintWriter, which uses its own/default encoding—so non-ASCII JSON can have a different byte length than the advertised Content-Length (lines 56-63).

Minimal fix

-import java.io.PrintWriter;
+import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
@@
-				PrintWriter out = new PrintWriter(client.getOutputStream(), false)
+				OutputStream out = client.getOutputStream()
@@
-				byte[] bytes = response.getBytes("UTF-8");
-				out.print("HTTP/1.1 200 OK\r\n");
-				out.print("Content-Type: application/json\r\n");
-				out.print("Content-Length: " + bytes.length + "\r\n");
-				out.print("Connection: close\r\n");
-				out.print("\r\n");
-				out.print(response);
+				byte[] bytes = response.getBytes(StandardCharsets.UTF_8);
+				out.write(("HTTP/1.1 200 OK\r\n"
+					+ "Content-Type: application/json; charset=UTF-8\r\n"
+					+ "Content-Length: " + bytes.length + "\r\n"
+					+ "Connection: close\r\n"
+					+ "\r\n").getBytes(StandardCharsets.US_ASCII));
+				out.write(bytes);
 				out.flush();

🤖 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 `@agents/android/java/HttpRpcServer.java` around lines 32 - 33, The response
and Content-Length must be written with the same encoding used to compute
bytes.length; replace the PrintWriter usage so headers and body are written as
bytes in UTF-8 (or use a writer explicitly set to UTF-8). Specifically in
HttpRpcServer where BufferedReader and PrintWriter are created for client, stop
using a platform-default PrintWriter for the response: compute byte[] body =
json.getBytes(StandardCharsets.UTF_8), build the HTTP header string and write
header.getBytes(StandardCharsets.US_ASCII) followed by body using
client.getOutputStream().write(...), then flush and close the stream (or
alternatively create new PrintWriter(new
OutputStreamWriter(client.getOutputStream(), StandardCharsets.UTF_8), false) and
ensure Content-Length uses body.length). Ensure you reference and update the
PrintWriter/client.getOutputStream() usage so Content-Length matches the actual
UTF-8 byte length.

♻️ Duplicate comments (1)

agents/android/java/HttpRpcServer.java (1)

32-52: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Read the request body as bytes and loop until full.

Content-Length is byte-based, but this code reads decoded characters through BufferedReader and assumes a single read(...) fills the buffer. UTF-8 payloads and partial reads can still truncate or corrupt JSON-RPC requests. This is the same parser bug already flagged on the earlier revision.

In Java, does `Reader.read(char[], off, len)` guarantee filling the requested length, and is HTTP `Content-Length` defined in bytes rather than decoded characters?

🤖 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 `@agents/android/java/HttpRpcServer.java` around lines 32 - 52, The request
body is being read via a Reader into a char[] which can mis-handle byte-based
Content-Length and partial reads; change the logic in HttpRpcServer (the socket
handling block that currently uses BufferedReader in, PrintWriter out, and reads
into char[] body) to read raw bytes from client.getInputStream(), loop until you
have read exactly contentLength bytes (handling short/partial reads), accumulate
into a byte buffer (or ByteArrayOutputStream), then decode that byte array to a
String using UTF-8 for JSON-RPC parsing; keep existing header parsing but stop
using Reader.read(...) to fill the body buffer.

🤖 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 `@agents/android/java/HttpRpcServer.java`:
- Around line 15-21: The server currently accepts LocalSocket connections and
forwards them directly to handleClient / JsonRpcDispatcher.dispatch without
authentication; after server.accept() obtain the peer credentials via
client.getPeerCredentials() and enforce an allowlist (UID, package name or
shared secret) before calling handleClient or dispatch; update the
LocalServerSocket accept loop(s) (the block creating LocalServerSocket and any
other accept sites that call handleClient/JsonRpcDispatcher.dispatch) to
validate credentials, log and close/ignore unauthorized sockets, and only
proceed to read/parse requests for allowed peers.

---

Outside diff comments:
In `@agents/android/java/HttpRpcServer.java`:
- Around line 32-33: The response and Content-Length must be written with the
same encoding used to compute bytes.length; replace the PrintWriter usage so
headers and body are written as bytes in UTF-8 (or use a writer explicitly set
to UTF-8). Specifically in HttpRpcServer where BufferedReader and PrintWriter
are created for client, stop using a platform-default PrintWriter for the
response: compute byte[] body = json.getBytes(StandardCharsets.UTF_8), build the
HTTP header string and write header.getBytes(StandardCharsets.US_ASCII) followed
by body using client.getOutputStream().write(...), then flush and close the
stream (or alternatively create new PrintWriter(new
OutputStreamWriter(client.getOutputStream(), StandardCharsets.UTF_8), false) and
ensure Content-Length uses body.length). Ensure you reference and update the
PrintWriter/client.getOutputStream() usage so Content-Length matches the actual
UTF-8 byte length.

---

Duplicate comments:
In `@agents/android/java/HttpRpcServer.java`:
- Around line 32-52: The request body is being read via a Reader into a char[]
which can mis-handle byte-based Content-Length and partial reads; change the
logic in HttpRpcServer (the socket handling block that currently uses
BufferedReader in, PrintWriter out, and reads into char[] body) to read raw
bytes from client.getInputStream(), loop until you have read exactly
contentLength bytes (handling short/partial reads), accumulate into a byte
buffer (or ByteArrayOutputStream), then decode that byte array to a String using
UTF-8 for JSON-RPC parsing; keep existing header parsing but stop using
Reader.read(...) to fill the body buffer.

🪄 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: 5b1462b9-a98a-4645-832d-3bcf23183c6d

📥 Commits

Reviewing files that changed from the base of the PR and between 8379735 and a53c2c4.

📒 Files selected for processing (7)

• agents/android/java/HttpRpcServer.java
• agents/android/java/JsonRpcDispatcher.java
• agents/android/java/MobilecliAgent.java
• agents/android/java/RpcException.java
• agents/android/java/WebViewAgent.java
• agents/android/jvmti_agent.c
• agents/ios/agent.m

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

• agents/android/java/MobilecliAgent.java

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

• agents/ios/agent.m
• agents/android/jvmti_agent.c
• agents/android/java/RpcException.java
• agents/android/java/JsonRpcDispatcher.java
• agents/android/java/WebViewAgent.java

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

agents/ios/devicekit.m (1)

23-23: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Undefined variable http causes compilation failure.

The variable http is never declared. The status code is now extracted into statusStr (lines 14-16), but this log statement wasn't updated to match.

🐛 Proposed fix

-            NSLog(@"[gadget] swizzle fired for %@ -> %ld, wrote to /tmp/gilm.txt", url, (long)http.statusCode);
+            NSLog(@"[gadget] swizzle fired for %@ -> %@, wrote to /tmp/gilm.txt", url, statusStr);

🤖 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 `@agents/ios/devicekit.m` at line 23, The NSLog call in devicekit.m references
an undefined variable `http` causing a compile error; update the NSLog in the
swizzle/logging site to use the already-extracted `statusStr` instead of
`http.statusCode`, and change the format specifier from "%ld" to "%@" so the
call becomes NSLog(@"[gadget] swizzle fired for %@ -> %@, wrote to
/tmp/gilm.txt", url, statusStr) (locate the NSLog statement near the swizzle
code that currently mentions `http`).

🤖 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.

Outside diff comments:
In `@agents/ios/devicekit.m`:
- Line 23: The NSLog call in devicekit.m references an undefined variable `http`
causing a compile error; update the NSLog in the swizzle/logging site to use the
already-extracted `statusStr` instead of `http.statusCode`, and change the
format specifier from "%ld" to "%@" so the call becomes NSLog(@"[gadget] swizzle
fired for %@ -> %@, wrote to /tmp/gilm.txt", url, statusStr) (locate the NSLog
statement near the swizzle code that currently mentions `http`).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7a4b898a-a9c8-4775-b30f-c4ea475a1845

📥 Commits

Reviewing files that changed from the base of the PR and between a53c2c4 and a65145e.

📒 Files selected for processing (9)

• .github/workflows/build.yml
• .gitignore
• Makefile
• agents/agents.go
• agents/android/Makefile
• agents/android/jvmti_agent.c
• agents/ios/bridge.m
• agents/ios/devicekit.m
• devices/android_webview.go

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

• .gitignore

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

• agents/agents.go
• Makefile
• .github/workflows/build.yml
• agents/ios/bridge.m